Subject: Re: [ANCP] comments on -12

From: Tom Taylor <[email-masked]>
Date: Sun, 02 Jan 2011 21:17:31 -0500

Having finished the update at last, I need to re-answer the reviews with 
what is now in place. Thanks again for your comments.

On 22/09/2010 2:16 PM, DE CNODDER, STEFAAN (STEFAAN) wrote:
> Hi Tom,
>
> Below some comments on draft-ietf-ancp-protocol-12, I hope they are useful.
>
> Regards,
> Stefaan
>
>
> -       Section 4.5.2 says that the pType field should be set to zero (No Partition).  I guess that is wrong because it has to contain the partition id?

[PTT] The pType field actually says how the PartionID is negotiated, 
rather than giving its value. Here is the updated text for pType:

"The pType field MAY be set to 0 (No Partition) or another value 
depending on local configuration."

> -       Section 4.6.2 says that if the recipient of a message does not understand a particular TLV, it must silently ignore it, but on the other hand there is a code defined "84 TLV not supported".  So must an unknown TLV be ignored, or should a failure be returned with this error code?

[PTT] Deleted the error code.

> -       Tech type has been removed in the adjacency (see note in section 4.5.1, btw, better to remove that note in the RFC version), but not in the other messages.  Is that the intention?  Should it not be better to make the Techtype field "reserved"?

[PTT] I left Tech Type in the Port Up/Down message because it may be 
useful. It has been changed to Reserved in the Port Management message 
and text has been added explaining how Port Management can be used for 
other capabilities and technologies. One issue is that the access line 
identifiers specified in the base document may not apply to non-DSL 
technologies.

> -       In section 5.2.3.1 I find it a bit confusing that it says "MUST support the following ANCP protocol elements, while some TLVs are mandatory.  The wording does not look correct to me.  The same comment applies to the other use cases.  In that same section, capability type should be 0x0001 instead of 0x01

[PTT] Rewritten according to the resolution agreed on the list. Fixed 
the capability type values.

> -       Figure 8, 13 and 16 have a typo in the ANCP general message header: should refer to section 4.6 instead of 4.4

[PTT] Fixed.

> -       Section 5.2.3.3.3: In case a DSLAM is directly connected to the NAS, a single VLAN can be used, so this should also be covered, e.g, Length could be set to 4 if only 1 VLAN, or keep it to 8 and set outer VLAN to 0.

[PTT] Done. I chose this method because I didn't know if there was any 
otherwise forbidden value of VLAN identifier to usae as a null entry.

> -       Section 5.2.3.3.4: The text implies that the NAS must be able to parse the text string while the format of it is not a MUST, but is just a typical format.  I would just remove this TLV.  The VLANs are known from the previous TLV.  What if the uplink is a link aggregate?  What if the uplink changes?  These things are not covered.

[PTT] I think that has been addressed by leaving it all up to the 
control application. I gave references for formats in an informational 
section, but not the actual formats themselves.

> -       No Iana for DSL-Type TLV?  I would also rename "UNKNOWN" into "OTHER" because the DSL-type is always known by the DSLAM, and you might give it a value 0 or 0xffffffff.

[PTT] DSL-Type is there. 0x0091. UNKNOWN changed to OTHER. Gave it value 0.

> -       According to figure 13, the # of TLV field is 15 bits, and the extension block length field is 17 bits.  I believe it should be 16 bits each.

[PTT] Fixed.

> -       Section 5.3.3.3: A "MUST" in combination with "depending upon the deployment scenario" is quite vague.  I do not see immediately when which TLV is used.  Could this be made more formal and when which TLV to be used?

[PTT] Fixed by the rewrite. It's all up to the control application now, 
although I do describe informatively what that application has to consider.