[MIB-DOCTORS] early MIB Doctor review for draft-ietf-softwire-map-mib-07
"Bert Wijnen (IETF)" <bertietf@bwijnen.net> Mon, 08 May 2017 14:55 UTC
Return-Path: <bertietf@bwijnen.net>
X-Original-To: mib-doctors@ietfa.amsl.com
Delivered-To: mib-doctors@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 948F31294B8 for <mib-doctors@ietfa.amsl.com>; Mon, 8 May 2017 07:55:53 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.62
X-Spam-Level:
X-Spam-Status: No, score=-2.62 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H3=-0.01, RCVD_IN_MSPIKE_WL=-0.01] autolearn=ham autolearn_force=no
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id HvDG26VDWV4J for <mib-doctors@ietfa.amsl.com>; Mon, 8 May 2017 07:55:51 -0700 (PDT)
Received: from lb2-smtp-cloud2.xs4all.net (lb2-smtp-cloud2.xs4all.net [194.109.24.25]) (using TLSv1 with cipher DHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id ECD021294A8 for <mib-doctors@ietf.org>; Mon, 8 May 2017 07:55:50 -0700 (PDT)
Received: from Macintosh-4.fritz.box ([IPv6:2001:981:602b:1:5f4:343c:cbdf:d5d2]) by smtp-cloud2.xs4all.net with ESMTP id Hqvn1v00P327dWa01qvo8V; Mon, 08 May 2017 16:55:49 +0200
To: draft-ietf-softwire-map-mib.all@ietf.org, MIB Doctors <mib-doctors@ietf.org>
From: "Bert Wijnen (IETF)" <bertietf@bwijnen.net>
Message-ID: <d7384d87-7a25-408d-86e7-9073a6c38278@bwijnen.net>
Date: Mon, 08 May 2017 16:55:47 +0200
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:45.0) Gecko/20100101 Thunderbird/45.8.0
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"; format="flowed"
Content-Transfer-Encoding: 7bit
Archived-At: <https://mailarchive.ietf.org/arch/msg/mib-doctors/vluaNkcNPtIivKdmE_wnswttqFs>
Subject: [MIB-DOCTORS] early MIB Doctor review for draft-ietf-softwire-map-mib-07
X-BeenThere: mib-doctors@ietf.org
X-Mailman-Version: 2.1.22
Precedence: list
List-Id: MIB Doctors list <mib-doctors.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/mib-doctors>, <mailto:mib-doctors-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/mib-doctors/>
List-Post: <mailto:mib-doctors@ietf.org>
List-Help: <mailto:mib-doctors-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/mib-doctors>, <mailto:mib-doctors-request@ietf.org?subject=subscribe>
X-List-Received-Date: Mon, 08 May 2017 14:55:54 -0000
[not sure if draft-ietf-softwire-map-mib.all@ietf.org also includes the softwire WG mailing list. Anyways, I am not subscribed to that WG list, so I probably can't post there. Pls WG chairs forward if needed/wanted]. I did an early MIB doctor review for this document. The syntax check with SMICNG is clean. Great. My comments/questions: - section 4 states: The MAP-E MIB provides a way to configure and monitor the MAP devices in MAP encapsulation mode through SNMP. Yet there are no read-write or read-create objects defined in the MIB module. So I don't think you can "configure" anything on the MAP devices via SNMP with this MIB module - section 4.1.1 states: The mapRule subtree describes managed objects used for managing the multiple mapping rules in the MAP encapsulation mode. So, since the MIB module is read-only, I think I would change s/managing/monitoring/. But maybe that is just a wording choice. - section 4.1.2 - The BR MUST perform a validation of the consistency of the source IPv6 address and source port number for the packet using BMR. - The Customer Edge (CE) SHOULD check that MAP received packets' transport-layer destination port number is in the range configured by MAP for the CE. Mmmm... what is BR ? Please expand the acronym the first time you use it in your document. Same for BMR. I am not sure that the MIB document is the place to tell (normatively (MUST/SHOULD) a BR that it MUST perform a certain protocol function. That is (I assume) specified in the RFC7597 itself, right? Maybe you mean to say that the BR (and CE) MUST and SHOULD report any invalid packets as found per the rules in RFC7579. They MUST/SHOULD report them via this MIB module (as per your MODULE Conformance). Oh well, again this may be a wording issue. If my interpretation here is correct, I can live with your wording although I think it can be clearer and that the proper compliance language is in the proper document. - section 5. I think I would change: 5. Definitions MAP-E-MIB DEFINITIONS ::= BEGIN IMPORTS MODULE-IDENTITY, OBJECT-TYPE, mib-2, Integer32, Unsigned32, Counter64 FROM SNMPv2-SMI ifIndex FROM IF-MIB InetAddressType, InetAddress, InetAddressPrefixLength FROM INET-ADDRESS-MIB OBJECT-GROUP, MODULE-COMPLIANCE FROM SNMPv2-CONF; Into 5. Definitions The following MIB module imports definitions from [RFC2578], [RFC2580], [RFC2863], and [RFC4001]. MAP-E-MIB DEFINITIONS ::= BEGIN IMPORTS MODULE-IDENTITY, OBJECT-TYPE, mib-2, Integer32, Unsigned32, Counter64 FROM SNMPv2-SMI -- RFC2578 ifIndex FROM IF-MIB -- RFC2863 InetAddressType, InetAddress, InetAddressPrefixLength FROM INET-ADDRESS-MIB -- RFC4001 OBJECT-GROUP, MODULE-COMPLIANCE FROM SNMPv2-CONF; -- RFC2580 That way, you have a reference to each document (they are all normative) and in the MIB module itself (where you cannot put references/citations) you would still have document which RFC the imports are from. Handy when a MIB module has been extracted from an RFC. In the MIB module itself: mapRuleID OBJECT-TYPE SYNTAX Integer32 (1..2147483647) MAX-ACCESS not-accessible STATUS current DESCRIPTION "An identifier used to distinguish the multiple mapping rule which is unique with each CE in the same BR." ::= { mapRuleEntry 1 } Since this is an index object, it be better defined as unsigned 32. See RFC4181, section 4.6.1.1. Specifically page 15, which states - Unsigned32 with a range that excludes zero is RECOMMENDED for most index objects. It is acceptable to include zero in the range when it is semantically significant or when it is used as the index value for a unique row with special properties. Such usage SHOULD be clearly documented in the DESCRIPTION clause. mapRuleIPv6PrefixType OBJECT-TYPE SYNTAX InetAddressType MAX-ACCESS read-only STATUS current DESCRIPTION "This object MUST be set to the value of ipv6(2) to present the IPv6 address.It describes the address type of the mapRuleIPv6Prefix and mapRuleBRIPv6Address." Such MUST language is not recommended. The way to specify such mandatory value you best use the MODULE COMPLIANCE statement. Such is RECOMMENDED as per RFC4181, page 26 mapRuleIPv6Prefix OBJECT-TYPE SYNTAX InetAddress(SIZE (0..16)) MAX-ACCESS read-only STATUS current DESCRIPTION "The IPv6 prefix defined in mapping rule which will be assigned to CE. The address type is given by mapRuleIPv6PrefixType." ::= { mapRuleEntry 3 } mmmmm, when the InetAddressType is ipv6(2), then my understanding of RFC4001 is that the SIZE for the InetAddress MUST be 16. Maybe Juergen can chime in here? And if it is ALWAYS an IPv6 address, then maybe it is even better to use InetAddressIPv6 as the OBJECT-TYPE. I see the same set of 2 objects for IPv4. In my view, the idea of InetAddressType and InetAddress were/are to allow yopu to specify one or a single pair that can hold each of the 2 (or even more if needed) AddressTypes. So why are there separate objects for IPv4 and IPv6 ??? mapRulePSID OBJECT-TYPE SYNTAX Integer32 MAX-ACCESS read-only STATUS current DESCRIPTION "The PSID value algorithmically identifies a set of ports assigned to a CE." REFERENCE "PSID: section 3 of RFC 7597." ::= { mapRuleEntry 9 } Mmmm... section 3 of RFC7597 only defines the term. The algorithm is in section 5.1 Maybe that is a better place to point to. Reading that section 5.1 in RFC7597, I wonder if "Integer32" is the best representation. In section 5.1, I see a PSID that is 6 bits (figure 2 on page 10). But there is also text about a PSID of 0x00 and 0xFF, which does not fit in 6 bits. I am not an expert (basically have no knowledge about) on RFC7597. But sofar I cannot determine what the value range might be and how Integer32 is a good representation for the PSID. Please explain (not just to me, but adding text to the internet drafts and the DESCRIPTION clause of this object) mapRulePSIDLen OBJECT-TYPE SYNTAX Integer32 My understanding sofar is that this object can never take a negative value, so Unsigned32 would be better I think. mapRuleType OBJECT-TYPE SYNTAX Integer32 MAX-ACCESS read-only STATUS current DESCRIPTION "The type of the mapping rule. A value of 0 means it is a BMR; a non-zero value means it is a FMR." REFERENCE "BMR, FMR: section 5 of RFC 7597." ::= { mapRuleEntry 13 } That probably works. But it is probably much better to do a ENUMERATION, Which has a SYNTAX of INTEGER and then lists enumberated values. That makes it much more extensible in the future (if needed). mapMIBSecurityGroup OBJECT-GROUP OBJECTS { mapSecurityCheckInvalidv4, mapSecurityCheckInvalidv6 } STATUS current DESCRIPTION " The collection of this objects are used to give the information on MAP security checks." ::= { mapMIBGroups 2 } These objects are defined with ACCESS accessible-for-notify. I think that means you have to define this group as a NOTIFICATION-GROUP instead of an OBJECT-GROUP. - Section 7. Security considerations: These are the objects and their sensitivity/vulnerability: mapRuleIPv6PrefixType mapRuleIPv6Prefix ... etc (quite a list of objects). But nowhere do I see text about "their sensitivity/vulnerability". ?? Still to be added? Bert
- [MIB-DOCTORS] early MIB Doctor review for draft-i… Bert Wijnen (IETF)
- Re: [MIB-DOCTORS] early MIB Doctor review for dra… Yu Fu
- Re: [MIB-DOCTORS] early MIB Doctor review for dra… Bert Wijnen (IETF)
- Re: [MIB-DOCTORS] early MIB Doctor review for dra… Yu Fu
- Re: [MIB-DOCTORS] early MIB Doctor review for dra… Bert Wijnen (IETF)
- Re: [MIB-DOCTORS] early MIB Doctor review for dra… Yu Fu
- Re: [MIB-DOCTORS] early MIB Doctor review for dra… Bert Wijnen (IETF)
- Re: [MIB-DOCTORS] early MIB Doctor review for dra… Terry Manderson
- Re: [MIB-DOCTORS] early MIB Doctor review for dra… Yu Fu