Hi, Review part 2.
Cheers, Ian ----- 4. Attributes 4.a) 'sub options' is used. Conventionally, this is hypenated as 'sub-options'. Please change throughout. 4.b) It would be more accurate to say: Different combinations of sub-options are required for each type of S46 Container... 4.c) " The RADIUS attribute for Dual-Stack Lite [RFC6333] is defined in [RFC6519]." This is the first time in the document that DSLite is referenced. Would it be better to put a paragraph in the Introduction saying that this document is not concerned with DSLite/RADIUS as it's already covered in RFC6519. 4.c) The S46 Container Options section describes how the containers and options are constructed. It would read better to have this overview text before descrbing the structure of any of the attributes/options. 4.e) In the description text around Fig.2 a pointer to the table in 4.7 would be helpful. It may actually be better to move the table from section 4.7 into section 4.2 so they can be compared directly by the reader. 4.f) Figure 2 doesn't really make it clear that the relevant (and necessary) sub-options (numbered 1-5) is decided by which on of the three S46 container options is being used. This makes it confusing to understand. As a suggestion, would 3 diagrams (one for each type) structured like this be more clear showing what is necessary and what is optional? / (Mandatory) /1.Rule-IPv6-Prefix | | sub-option | 1.S46-Rule ------------+ 2.Rule-IPv4-Prefix | sub-option | sub-option | | 3.EA Length S46 MAP-E Container--+ \ sub-option Option | 2.S46-BR Sub Option - - - - - - - - - - - - - - - - - - - - - - - - - | (Optional) /1.PSID-offset | | sub-option | 3.S46-PORTPARAMS -------+ 2.PSID-len | sub-option | sub-option | | 3.PSID \ \ sub-option The paragraph begining 'There are three types of S46 Container Options...' would need to be moved before this for readability. 4.g) The paragraph begining 'There are three types of S46-Rule Sub Options...' seems to be in the wrong section. It belongs in 4.3.1. 4.h) s/The S46-BR Sub Option can only be encapsulated in the MAP-E Container Option or the Lightweight 4over6 Container Option./ The S46-BR Sub Option can only be encapsulated in the MAP-E or Lightweight 4over6 Container Options./ 4.i) 4.3.3. S46-DMR Sub Option s/set to all zero./set to all zeros./ 4.j) 4.3.4. S46-V4V6Bind Sub Option s/There MUST be at most one/There MUST be exactly one/ 4.k) 4.3.5. S46-PORTPARAMS Sub Option Suggest that the first sentance is extended to say: The S46-PORTPARAMS Sub Option specifies optional port set information that MAY be provisioned to CEs to configure sharing of an IPv4 address. 4.l) Throughout this section, it would be a good idea to put in a pointer to the section of RFC7598 that the option/sub-option corresponds to. 4.m) s/The Softwire46-Multicast attribute conveys the IPv6 prefixes to be used in [RFC8114] to synthesize IPv4-embedded IPv6 addresses./ The Softwire46-Multicast attribute conveys the IPv6 prefixes to be used to synthesize IPv4-embedded IPv6 addresses as per [RFC8114]./ 4.n) This attribute MAY be used in Access-Request packets as a hint to the RADIUS server. For example, if the BNG is pre-configured with Softwire46-Multicast, these prefixes MAY be inserted in the attribute. Is this saying that the attribute could be pre-configured with a value in the BNG's AAA client meaning that it wouldn't be requested from AAA, but would be supplied in the DHCP message? Please can you expand the description. I also wonder if this statement is true for any of the other attributes described in the document. 4.o) The definitions of which RADIUS messages types the multicast attribute can appear in should also exist for the unicast and priority attributes. Update - having read section 4.10, the information is there, but is duplicated for multicast. One common format for the information and pointers would be cleaner. 4.p) The description text and formatting between the 4.2-4.8 options is inconsistent and should be aligned. Personally, I think the bullet point list of what is and isn't permitted in section 4.9 is clear and could be usefully used in 4.2-4.9. 4.q) 4.9.1. ASM-Prefix64 TLV The field in the diagram is called 'Prefix-Length', but in the description, this field is 'Length'. This also needs fixing in 4.9.2 and 4.9.3 4.r) 4.9.2 & 4.9.3 s/This fiel is reserved./This field is reserved./ 4.s) Section 4.9 uses TBD1 as a placeholder for an IANA assignment. TBD1-3 are already used earlier in the document. --------- 6. IANA Considerations 6.a) "This document requires the assignment of two new RADIUS Attribute..." there is then a list of 3 new attributes. 6.b) 6.1. S46 Mechanisms and Their Identifying Option Codes It seems that there is a requirement here for the BNG AAA client/DHCPv6 server to provide a translation between the RADIUS option code in the newly requested registry and the RFC8026 option 111 values (which will be sent to the DHCPv6 client) listed in the "Option Codes Permitted in the S46 Priority Option" IANA registry. This process isn't described at the moment, and I wonder if this is really the right way to solve the problem. Is this something that you have considered? ------------ 7. Security Considerations 7.a) " A malicious user may use MAC address spoofing on the shared password that has been preconfigured on the DHCPv6 server to get unauthorized configuration information." I'm not sure I understand the nature of this attack. Is it really the DHCPv6 server that you mean here, or the BNG (specifically the AAA client function)? Is there a reference to where this attack is described in greater deatail. 7.b) RFCs 7599 and 8114 should also be listed. I-D Nits output idnits 2.15.01 tmp/draft-ietf-softwire-map-radius-15.txt: Attempted to download rfc5822 state... Failure fetching the file, proceeding without it. Checking boilerplate required by RFC 5378 and the IETF Trust (see https://trustee.ietf.org/license-info): ---------------------------------------------------------------------------- No issues found here. Checking nits according to https://www.ietf.org/id-info/1id-guidelines.txt: ---------------------------------------------------------------------------- No issues found here. Checking nits according to https://www.ietf.org/id-info/checklist : ---------------------------------------------------------------------------- No issues found here. Miscellaneous warnings: ---------------------------------------------------------------------------- -- The document date (March 21, 2018) is 16 days in the past. Is this intentional? Checking references for intended status: Proposed Standard ---------------------------------------------------------------------------- (See RFCs 3967 and 4897 for information about using normative references to lower-maturity documents in RFCs) -- Obsolete informational reference (is this intentional?): RFC 5226 (Obsoleted by RFC 8126) Summary: 0 errors (**), 0 flaws (~~), 0 warnings (==), 2 comments (--). Run idnits with the --verbose option for more detailed information about the items above. -------------------------------------------------------------------------------- > On 26. Mar 2018, at 15:43, Yong Cui <cuiy...@tsinghua.edu.cn> wrote: > > Hi folks, > > The authors of draft-ietf-softwire-map-radius-14 believe this document is > ready for advancement. > We would like to issue a working group last call starting from today to April > 9th. > > Please send your substantial comments to the list during the last call. You > are also welcome to send your editorial comments directly to the authors. > > Thanks for reviewing the draft. > > > Yong Cui and Ian Farrer > > > _______________________________________________ > Softwires mailing list > Softwires@ietf.org > https://www.ietf.org/mailman/listinfo/softwires
_______________________________________________ Softwires mailing list Softwires@ietf.org https://www.ietf.org/mailman/listinfo/softwires