Dear Benoit and Dave, Thanks for your extensive review. Please see my reply inline.
On 2/12/2015 5:54 am, "Softwires on behalf of Dave Thaler" <[email protected] on behalf of [email protected]> wrote: >I was assigned to do the MIB doctor review of this document, since I >previously >did an early review of -03. My full comments are in the marked up copy at >http://research.microsoft.com/~dthaler/draft-ietf-softwire-dslite-mib-12.p >df >Below is a summary of the issues called out therein. > >Substantive issues: >1) >RFC 4001 requires each InetAddress object to explicitly state >which other InetAddressType object indicates the type. None of the >objects >in this document do so. RFC 7659 (the NATv2 MIB) does, and can be used as >an example. [Yu]: We will add the statement in the updated version according to the RFC 7659 as an example. >2) >dsliteNATBindEntry >includes >dsliteTunnelStartAddPreLen in the INDEX. >To confirm this was intended: >Can you really have 2 entries that have all >other INDEX values the same and differ only in the prefix length? [Yu]: Only the dsliteTunnelStartAddress would be enough to identify the IP address of the tunnel startpoint, so we will delete the dsliteTunnelStartAddPreLen in the INDEX of dsliteNATBindEntry. >3) >dsliteNATBindTable states that it extends natv2PortMapTable in RFC 7659, >but rather than reusing the not-accessible objects from that table in its >own >INDEX clause, it defines its own. That¹s fine, but it is then not clear >whether >each such object in the >dsliteNATBindTable INDEX needs to match >a corresponding value in the natv2PortMapTable INDEX, >or whether there can be additional entries that do not >appear in the natv2PortMapTable. This should be clarified. [Yu]: Each entry in the dsliteNATBindTable not only need to match a corresponding entry in the natv2PortMapTable but also a corresponding entry in the dsliteTunnelTable, so each such object in the dsliteNATBindTable INDEX needs to match a corresponding value in the natv2PortMapTable INDEX and a correspongding value in the dsliteTunnelTable INDEX. We will add a clarification in the DESCRIPTION of the dsliteNATBindEntry. >4) >Many objects in that table, such as dsliteNATBindMappingIntRealm, >have very terse DESCRIPTIONs, whereas the DESCRIPTION of >the corresponding object in the natv2PortMapTable is quite >detailed. Hence this draft is far less clear than RFC 7659, >since this draft has no such language. [Yu]: We will add more detailed description in the defined object in the dsliteNATBindTable. >5) >Objects of type InetAddress incorrectly have a REFERENCE >clause pointing to the definition of the InetAddress TC. >REFERENCE clauses should be used to point to the spec defining >the semantics, rather than the syntax. For example, >dsliteNATBindMappingIntAddress is incorrect, whereas the >corresponding object in RFC 7659 (natv2AddressMapInternalAddressType) >is correct and points into the DS-Lite RFC (RFC 6333). [Yu] We will correct it in the next version. >6) >I didn¹t understand DsliteNATBindEntry at all. Its >dsliteNATBindMappingMapBehavior object has a value >addressAndPortDependent(2) which ³maps to a separate external >address and port combination for each different >destination address and port combination reached >through the same external realm². However, the external >port >is in the INDEX clause and the destination address does >not appear to be >in the table at all. Since the 0 value for the external port already has >a different >special meaning, it can¹t be 0 either. So I don¹t understand how this >table >can work. [Yu]: It need more clarification sentence here. Because the AFTR is also a CGN device (Carrier Grade NAT) which translates private IPv4 addresses to and from global IPv4 addresses according to a classical NAPT scheme configured by the Service Provider, the dsliteNATBindMappingMapBehavior object only has a value addressAndPortDependent(2) here in DS-Lite scenario. And the dsliteNATBindMappingExtPort must be a non-zero value. It also need some clarification in the DESCRIPTION of the natv2PortMapExternalPort. Let me take a simple example for the extended IPv4 NAT binding table according to the definition of dsliteNATBindTable: +------------------------------------------------------------------------------------------+---------------------------------------------------------------+ | TunnelStartAddress/IPv4InternalAddress/Protocol/InternalPort | IPv4ExternalAddress/Protocol/ExternalPort | +-------------------------------------------------------------------------------------------+------------------------------------------------------------ -+ | 2001:db8:0:1::1 / 10.0.0.1 / TCP / 10000 | 192.0.2.1 / TCP / 5000 | +-------------------------------------------------------------------------------------------+------------------------------------------------------ -------+ the translation tables in the AFTR are configured to forward between IP/TCP (10.0.0.1/10000) and IP/TCP (192.0.2.1/5000). That is, a datagram received by the Dual-Stack Lite home router from the host at address 10.0.0.1, using TCP port 10000, will be translated to a datagram with IPv4 SRC address 192.0.2.1 and TCP SRC port 5000 in the Internet. This Entry is created when the AFTR receives datagram on the outbound direction. When the datagram is returned from the inbound direction, it will do a reverse lookup in the extended IPv4 NAT binding table to know how to reconstruct the IPv6 encapsulation back to the home router. >7) >dsliteAFTRAlarmProtocolType is underspecified. It¹s a string, >but the description is very confusing as to what the legal string >values are, making it sound more like an INTEGER was intended. >(³This object indicate the protocol type of alarm, > 0:tcp,1:udp,2:icmp,3:total²) >E.g., does that mean the string is ³0:tcp² or ³0² or ³tcp² or what? [Yu]: We will add a clarification to describe the string as 0:tcp. >8) >The >dsliteStatisticTransmitted object seems to combine sent + received >packets into a single counter with a name that implies only one direction. >This is confusing, especially since most other MIB modules separate sent >vs received into different counters. [Yu]: We will separate dsliteStatisticTransmitted object into two objects for sent and received packets. >9) >dsliteAFTRUserSessionNumAlarm and dsliteAFTRPortUsageOfSpecificIpAlarm >both refer to ³the threshold² without stating what threshold that is. >There doesn¹t seem to be any such threshold object in this MIB module or >elsewhere that I could find. [Yu]: We will define these threshold objects in the next version. >10) >dsliteAFTRAlarmScalarGroup >is mandatory and requires read-write access. >But a lesson learned from the NAT MIB (and many other MIBs) is that many >people don¹t want write support in their MIB modules. Does the WG really >feel that write support is required in all implementations? I¹d recommend >also having a read-only compliance statement, as is done in many other >MIB modules. [Yu]:The read-write object defined in this MIB is intended to set a threshold value used to trigger a trap. So I think the write support is required in DSLite-MIB as the object of natv2InstanceThresholdPortMapEntriesHigh defined in RFC 7659 >11) >The security considerations section uses the correct boilerplate for >sensitive read-only objects, which includes ³These are the tables and >objects and their sensitivity/vulnerability². However it then only >lists the tables/objects and contains no discussion of their >sensitivity/vulnerability. >This is required in order to comply with MIB review guidelines in RFC >4181. [Yu]: We will add the discussion in the security considerations section. >12) >Per discussion on MIB Doctors, the root OID should probably not be >{ transmission xxx }, since that space usually implies that xxx is an >ifType >(not tunnel type) value. [Yu]: We will move the DSLite-MIB under mib-2 >13) >A number of undefined terms are used that I could not find in the DS-Lite >RFC (6333) either, e.g., connection, session, etc. As such, I couldn¹t >tell what >was meant, and whether there were issues with the meaning. At minimum, >REFERENCE clauses should be added to point to a specific section of a >document >that defines the terms. [Yu]: The session mentioned here is referred to the RFC 4008. We will add the REFERENCE clauses and some description for connection in the object definition. > >Editorial issues: >14) >The title, abstract, MIB module name, etc. all say the MIB module is for > >managing DS-Lite, but DS-Lite has two roles: AFTR and B4, and this >document >only instruments the AFTR. Hence I would really recommend putting AFTR >in the title, abstract, and name. [Yu]: We will putting the AFTR in the title, abstract, the name. >15) >There are a bunch of editorial issues (grammar, typos) that should be >cleaned up before publication. [Yu]: We will revise the editorial issues accordingly. >16) >In a number of places, it seems to refer to some objects in some MIB >module, >whether this document or RFC 7659, without stating the name of the object >so it is not clear which object was meant. [Yu]: We will add the name of the object. >17) >The document still refers to >draft-perrault-behave-natv2-mib >and should be RFC 7659. [Yu]: We will update it for RFC7659 > >Dave > Thanks very much for you review. An updated version of the draft will be submitted soon. BR Yu _______________________________________________ Softwires mailing list [email protected] https://www.ietf.org/mailman/listinfo/softwires
