Re: [Softwires] I-D Action: draft-ietf-softwire-map-radius-16.txt

[ianfarrer]

Hi Yu,

Please see inline below.

Thanks,
Ian

> On 17. Aug 2018, at 11:48, Yu Fu  wrote:
> 
> Hi Ian
> 
>  
> 
> Thanks for your thorough  comments. Please see my reply below:
> 
>  
> 
> BR
> 
> Yu 
> 
>  
> 
> Hi Yu,
> 
> Thanks for the update. I think the changes that you have made have improved 
> the document significantly, but there’s still a few things that need to be 
> addressed. Please see below.
> 
> Also, Jordi asked a question about this draft, which I don’t think has been 
> replied to:
> https://mailarchive.ietf.org/arch/msg/softwires/_7ocUxBwy9i2UqNj2tzqri9p0Gw 
> 
> [fuyu]: From my side. I am happy to support 464XLAT, but I have made  a 
> tremendous effort to consistent with the different attribute format and 
> writing style after adding the multicast use case. 
> I am not sure that I can do the same work after adding this new function.  
> Could the Jordi help me to do that work? 

[if -  I’ve emailed the authors separately on this]

>  
> 
> 
> Thanks,
> Ian
> 
> New comments on -16:
> 
> Throughout - the use of 'Sub TLV' and 'Sub-TLV' is not consistent.
> Sub-TLV seems to be the convention in other RFCs (e.g. RFC6929).
> 
> [fuyu] I will check out all through the text for consistent.
> 
> 
> 
> Introduction.
> 
> s/At the Section 4.9/In Section 4.9/
> 
> As the softwire prioritisation funciton of RFC8026 is also
> included, there should be a short paragraph (a cut down version of
> sec 4, para 3) stating that this function is included.
> 
> [fuyu]: Do you think add a sentence as “A new Softwire46-Priority Attribute 
> contains RADIUS information corresponding to OPTION_S46_PRIORITY defined in 
> [RFC8026] is defined in Section 4.8.” will be Ok?
> 
[if - OK]

> 
> 
> 1.i)
> Last paragraph: It would also be worth saying how the structure of the
> DHCP options and field namings are preserved so they can
> easily be mapped between DHCP and RADIUS.
> 
> [if - I can't find any changes in the text for this, or any repsonse
> to the comment.]
> 
> [fuyu]: Do you think a table as following will be help?
> 
> 
>   
> +---+-+
> 
>   |  DHCPv6 Option   | RADIUS Attribute/Sub TLV |
> 
>   
> +---+---+
> 
>   | OPTION_S46_RULE| S46-Rule Sub TLV   |  
> 
>   
> +-+-+
> 
>   |   OPTION_S46_BR   |S46-BR Sub TLV   |  
> 
>   
> +---++ 
> 
>   |  ……|……
> |  
> 
>   +--+--+ 
> 
>
> 
[if - Yes, that’s what I had in mind]
> 
> 
> 
> 
> 3.c)
> The figure is easier to follow if it is space out a bit more and
> clafifies the first step:
> 
>  CE BNG AAA Server
>  |   |   |
>  |---1.DHCPv6 Solicit--->|   |
>  |(ORO with unicast and/or m'cast|   |
>  |container option code(s))  |   |
>  |   |   |
>  |   |---2.Access-Request--->|
>  |   | (S46-Configuration attribute  |
>  |   |and/or S46-Multicast attribute)|
>  |   |   |
>  |   |<--3.Access-Accept-|
>  |   | (S46-Configuration attribute  |
>  |   |and/or S46-Multicast attribute)|
>  |   |   |
>  |<4.DHCPv6 Advertisement|   |
>  | (container option(s)) |   |
>  |   |   |
>  |---5.DHCPv6  Request-->|   |
>  | (container Option(s)) |   |
>  |   |   |
>  |<6.DHCPv6 Reply|   |
>  | (container option(s)) |   |
>  |   |   |
>   DHCPv6 RADIUS
> 
> [if - Old diagram is still present, please use the above figure.]
> [fuyu]: Ok, I have updated it.
> 
> 3.f)
> Replace:
> For the multicast case, OPTION_V6_PREFIX64 should be included for the 
> delivery of multicast
>   services 

Re: [Softwires] I-D Action: draft-ietf-softwire-map-radius-16.txt

[Yu Fu]

Hi Ian

 

Thanks for your thorough  comments. Please see my reply below:

 

BR

Yu 

 


Hi Yu,

Thanks for the update. I think the changes that you have made have improved the 
document significantly, but there’s still a few things that need to be 
addressed. Please see below.

Also, Jordi asked a question about this draft, which I don’t think has been 
replied to:
https://mailarchive.ietf.org/arch/msg/softwires/_7ocUxBwy9i2UqNj2tzqri9p0Gw

[fuyu]: From my side. I am happy to support 464XLAT, but I have made  a 
tremendous effort to consistent with the different attribute format and writing 
style after adding the multicast use case. 
I am not sure that I can do the same work after adding this new function.  
Could the Jordi help me to do that work? 
 



Thanks,
Ian

New comments on -16:

Throughout - the use of 'Sub TLV' and 'Sub-TLV' is not consistent.
Sub-TLV seems to be the convention in other RFCs (e.g. RFC6929).

[fuyu] I will check out all through the text for consistent.



Introduction.

s/At the Section 4.9/In Section 4.9/

As the softwire prioritisation funciton of RFC8026 is also
included, there should be a short paragraph (a cut down version of
sec 4, para 3) stating that this function is included.

[fuyu]: Do you think add a sentence as “A new Softwire46-Priority Attribute 
contains RADIUS information corresponding to OPTION_S46_PRIORITY defined in 
[RFC8026] is defined in Section 4.8.” will be Ok?

Section 3, point 6.

In the diagram, you show a simple reply message being sent
to the client, but the accompanying text describes a number
of other communication flows and updates that can potentially
need to happen.

Really, the whole of this section is not really well constructed
in it's current form. The purpose of the numbered points is to
describe what is in the flows in the diagram, but point 6. goes
on to included a further page and a half of options and
considerations. Can point 6 not be ended after the sentance ending
enumerated in the ORO.? and the remaining text in section 3
be put into relevantly named sub-sections.



[fuyu]: Yes, I will update it as the point 6 will be ended by “enumerated in 
the ORO” and a sub-sections will be added


4.4.2 S46-BR Sub TLV
S46-Lightweight--4over6 TLV - please remove duplicate '-'.
[fuyu]: Done


4.5 Sub-TLVs for S46-Rule Sub TLV
Given the RFC2119 language used in the requirements for the
other options, suggest the following:
s/It should appear for once and only once./It MUST appear
exactly once./
[fuyu]:Done
4.6.2 The Bind-IPv6-Prefix Sub-TLV

s/The bind-ipv6-prefix field specified in this field/The bind-ipv6-prefix 
specified in this field/
[fuyu]: Done
-

Updated comments from my previous review:


1.f)
After the sentence "A DHCPv6 server function is
  assumed to be embedded in the BNG that allows it to locally handle
  any DHCPv6 requests initiated by hosts." it would be
  worth adding that the term BNG is used througout the document
  to describe a device which functions as both the AAA client
  and DHCPv6 server.

[if - New version is better, but a suggested rewording for these two sentances:
A DHCPv6 server function is assumed to be embedded in the BNG
that allows it to locally handle DHCPv6 requests initiated by
hosts.  The abbrieviation BNG is used in this document to describe a device
which functions as both the AAA client and DHCPv6 sever.]
[fuyu]: The new sentences is updated 

1.i)
Last paragraph: It would also be worth saying how the structure of the
DHCP options and field namings are preserved so they can
easily be mapped between DHCP and RADIUS.

[if - I can't find any changes in the text for this, or any repsonse
to the comment.]

[fuyu]: Do you think a table as following will be help?


  
+---+-+

  |  DHCPv6 Option   | RADIUS Attribute/Sub TLV |

  
+---+---+

  | OPTION_S46_RULE| S46-Rule Sub TLV   |  

  
+-+-+

  |   OPTION_S46_BR   |S46-BR Sub TLV   |  

  
+---++ 

  |  ……|……  
  |  

  +--+--+ 

   





3.c)
The figure is easier to follow if it is space out a bit more and
clafifies the first step:

 CE BNG AAA Server
 |   |   |
 |---1.DHCPv6 Solicit--->|   |
 |(ORO with unicast and/or m'cast|   |
 |container option code(s))  |   |
 |   |