傅瑜
2018-04-18 07:41:11 UTC
Hi Ian,
Please see my reply inline:
Thanks for your comments
Yu
From: softwires-***@ietf.org [mailto:softwires-***@ietf.org] On Behalf Of ***@gmx.com
Sent: Friday, April 06, 2018 10:59 PM
To: Yong Cui; Softwires WG
Subject: Re: [Softwires] WGLC on draft-ietf-softwire-map-radius-14 - 2 of 2
Hi,
Review part 2.
Cheers,
Ian
-----
4. Attributes
4.a) 'sub options' is used. Conventionally, this is
hypenated as 'sub-options'. Please change throughout.
[fuyu] Ok, I will change it throughout.
4.b) It would be more accurate to say:
Different combinations of sub-options are
required for each type of S46 Container...
[fuyu] Ok, I will update this sentence.
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.
[fuyu] I will add a paragraph in the Introduction.
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.
[fuyu] OK
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.
[fuyu] Ok, I will move the table from section 4.7 to section 4.2.
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.
[fuyu] You have missed S46-DMR Sub Option. I think that one diagram will make the overall structure of the S46 Container Option more clearly than divided it into 3 diagrams. Do you think if I add the mandatory and optional for each sub-option in the figure 2 will be better?
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.
[fuyu] Yes, sorry, it is a fault.
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./
[fuyu] I will update this sentence.
4.i)
4.3.3. S46-DMR Sub Option
s/set to all zero./set to all zeros./
[fuyu] Done
4.j)
4.3.4. S46-V4V6Bind Sub Option
s/There MUST be at most one/There MUST be exactly one/
[fuyu] Done
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.
[fuyu] Done
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.
[fuyu] OK, I will add a pointer to RFC7598.
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]./
[fuyu] Done.
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.
[fuyu] Some attribute like Sofwire46-Multicast and other attributes described in the document
may be pre-configured in the BNG based on the implementation by the different device company.
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.
[fuyu] I will make a common format for the unicast and multicast.
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.
[fuyu] Sorry, because the section 4.9 is merged into the document after 12 version.
So the description between the 4.2-4.8 and 4.9 is not the same. I will try to update
the 4.2-4.8 to be the same as 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
[fuyu] Ok, I will update it.
4.r)
4.9.2 & 4.9.3
s/This fiel is reserved./This field is reserved./
[fuyu] Done.
4.s)
Section 4.9 uses TBD1 as a placeholder for an IANA assignment. TBD1-3
are already used earlier in the document.
[fuyu] I will update it.
---------
6. IANA Considerations
6.a)
"This document requires the assignment of two new RADIUS Attribute..."
there is then a list of 3 new attributes.
[fuyu] Sorry, it is a fault, I will update it.
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?
[fuyu] I think the BNG will provide a translation between the âSoftwire46-Priority Attribute TBD2â and the RFC8026 option 111 values.
------------
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.
[fuyu] Sorry, this description may make some confusing. I think delete this paragraph will be better.
7.b)
RFCs 7599 and 8114 should also be listed.
[fuyu] I will add these two RFCs
I-D Nits output
[fuyu] I will try to figure out the I-D nits in the next version.
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.
--------------------------------------------------------------------------------
Please see my reply inline:
Thanks for your comments
Yu
From: softwires-***@ietf.org [mailto:softwires-***@ietf.org] On Behalf Of ***@gmx.com
Sent: Friday, April 06, 2018 10:59 PM
To: Yong Cui; Softwires WG
Subject: Re: [Softwires] WGLC on draft-ietf-softwire-map-radius-14 - 2 of 2
Hi,
Review part 2.
Cheers,
Ian
-----
4. Attributes
4.a) 'sub options' is used. Conventionally, this is
hypenated as 'sub-options'. Please change throughout.
[fuyu] Ok, I will change it throughout.
4.b) It would be more accurate to say:
Different combinations of sub-options are
required for each type of S46 Container...
[fuyu] Ok, I will update this sentence.
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.
[fuyu] I will add a paragraph in the Introduction.
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.
[fuyu] OK
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.
[fuyu] Ok, I will move the table from section 4.7 to section 4.2.
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.
[fuyu] You have missed S46-DMR Sub Option. I think that one diagram will make the overall structure of the S46 Container Option more clearly than divided it into 3 diagrams. Do you think if I add the mandatory and optional for each sub-option in the figure 2 will be better?
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.
[fuyu] Yes, sorry, it is a fault.
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./
[fuyu] I will update this sentence.
4.i)
4.3.3. S46-DMR Sub Option
s/set to all zero./set to all zeros./
[fuyu] Done
4.j)
4.3.4. S46-V4V6Bind Sub Option
s/There MUST be at most one/There MUST be exactly one/
[fuyu] Done
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.
[fuyu] Done
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.
[fuyu] OK, I will add a pointer to RFC7598.
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]./
[fuyu] Done.
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.
[fuyu] Some attribute like Sofwire46-Multicast and other attributes described in the document
may be pre-configured in the BNG based on the implementation by the different device company.
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.
[fuyu] I will make a common format for the unicast and multicast.
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.
[fuyu] Sorry, because the section 4.9 is merged into the document after 12 version.
So the description between the 4.2-4.8 and 4.9 is not the same. I will try to update
the 4.2-4.8 to be the same as 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
[fuyu] Ok, I will update it.
4.r)
4.9.2 & 4.9.3
s/This fiel is reserved./This field is reserved./
[fuyu] Done.
4.s)
Section 4.9 uses TBD1 as a placeholder for an IANA assignment. TBD1-3
are already used earlier in the document.
[fuyu] I will update it.
---------
6. IANA Considerations
6.a)
"This document requires the assignment of two new RADIUS Attribute..."
there is then a list of 3 new attributes.
[fuyu] Sorry, it is a fault, I will update it.
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?
[fuyu] I think the BNG will provide a translation between the âSoftwire46-Priority Attribute TBD2â and the RFC8026 option 111 values.
------------
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.
[fuyu] Sorry, this description may make some confusing. I think delete this paragraph will be better.
7.b)
RFCs 7599 and 8114 should also be listed.
[fuyu] I will add these two RFCs
I-D Nits output
[fuyu] I will try to figure out the I-D nits in the next version.
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.
--------------------------------------------------------------------------------