Subject: [ANCP] comments on -12

From: "DE CNODDER, STEFAAN (STEFAAN)" <[email-masked]>
Date: Wed, 22 Sep 2010 20:16:50 +0200

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?
-       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?
-       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"?
-       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
-       Figure 8, 13 and 16 have a typo in the ANCP general message header: should refer to section 4.6 instead of 4.4
-       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.
-       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.
-       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.
-       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.
-       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?

---

From: Tom Taylor <[email-masked]>
Date: Wed, 22 Sep 2010 22:54:18 -0400

Thanks a lot for your careful reading. I've responded to each point. In 
some cases I need further input from the Working Group. If it would be 
helpful, I can break these issues out into separate notes.

On 22/09/2010 2:16 PM, DE CNODDER, STEFAAN (STEFAAN) wrote:
> 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] OK

> -       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] Yes, I was half-aware of this contradiction. Maybe I meant to fix 
it but forgot. I'll take out the error code unless I remember a good 
reason to keep it, in which case we can discuss it.

> -       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] In the other messages it identifies the specific technology for 
which the message applies. Remember that we decided to make capabilities 
either technology-specific or technology-independent. I don't know what 
the messages would look like for other access technologies. (Time to 
read the PON document, I guess.)

> -       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] The intention is to say to implementors: if you claim support for 
this capability you must support the following sections of this 
specification. Maybe that's exactly how the text should read.

[PTT] Actually, I myself am confused over the distinction between 
mandatory and optional. I interpreted "mandatory" to mean "must always 
appear in the message" -- in other words, mandatory to use. I think 
optional means mandatory to implement, optional to use. The Working 
Group may have a different opinion, in which case, please speak up.

> -       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] OK. That's a manual thing that has to be fixed every time the 
document is reorganized.

> -       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] OK, which alternative does the WG prefer?

> -       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] What does the WG say?

> -       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] Thanks for catching the oversight.

> -       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] A classic bug. Will fix.

> -       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] I agree it is vague. I look to the WG for guidance on what should 
be added.

---

From: "DE CNODDER, STEFAAN (STEFAAN)" <[email-masked]>
Date: Thu, 23 Sep 2010 10:07:10 +0200

Hi Tom,

[Stefaan] My understanding is that an optional TLV does not have to be in the message.  For instance, in case of topology discovery, the DSLAM has to send all mandatory TLVs, but not the optional TLVs.  What the NAS does with these TLVs, is up to the NAS, and might even not actually use some of the mandatory TLVs.

---

From: Alan Kavanagh <[email-masked]>
Date: Thu, 23 Sep 2010 20:45:53 -0400

Hi Tom

I agree with Stefaan, I don't see the need for the DSLAM/AN sending the optional TLVs.

Alan 

-----Original Message-----
From: [email-masked] [mailto:[email-masked]] On Behalf Of DE CNODDER, STEFAAN (STEFAAN)
Sent: September-23-10 4:07 AM
To: Tom Taylor
Cc: [email-masked]
Subject: Re: [ANCP] comments on -12

Hi Tom,

> capability type should be 0x0001 instead of 0x01

[PTT] The intention is to say to implementors: if you claim support for this capability you must support the following sections of this specification. Maybe that's exactly how the text should read.

[PTT] Actually, I myself am confused over the distinction between mandatory and optional. I interpreted "mandatory" to mean "must always appear in the message" -- in other words, mandatory to use. I think optional means mandatory to implement, optional to use. The Working Group may have a different opinion, in which case, please speak up.

_______________________________________________
ANCP mailing list
[email-masked]
https://www.ietf.org/mailman/listinfo/ancp

---

From: Tom Taylor <[email-masked]>
Date: Thu, 23 Sep 2010 20:58:11 -0400

I think that's still coming down to "mandatory to implement, optional to 
use". If that is not the intention of the WG, I can certainly rephrase 
the text.

---

From: <[email-masked]>
Date: Fri, 24 Sep 2010 03:24:43 +0200

All,

I don't think that we should differentiate between optional and mandatory because the definitons of the TLVs are needed anyway and available in access nodes.

Particular I refer to chapter 5.2.3.3.6.1 - 5.2.3.3.6.15 of current protocol draft version 12.

Considering BBF TR-147 Table 1, page 45 there is no difference between mandatory and optional and all related parameters are mandatory.
Only TR-101 allows to differenciate between optional and mandatory in conjunction to DHCP relay requirements.
Implementations follow the whole parameterset.

In order to keep consistency between both workflows and avoiding interoperability issues I really propose to set all these parameters mentioned above to "mandatory". It really does not make sense if different access node implementations may exist using different TLVs terminated possibly at same BRAS.

Regards

Thomas

-----Ursprüngliche Nachricht-----
Von: [email-masked] [mailto:[email-masked]] Im Auftrag von Alan Kavanagh
Gesendet: Freitag, 24. September 2010 02:46
An: DE CNODDER, STEFAAN (STEFAAN); Tom Taylor
Cc: [email-masked]
Betreff: Re: [ANCP] comments on -12

Hi Tom

Alan 

-----Original Message-----
From: [email-masked] [mailto:[email-masked]] On Behalf Of DE CNODDER, STEFAAN (STEFAAN)
Sent: September-23-10 4:07 AM
To: Tom Taylor
Cc: [email-masked]
Subject: Re: [ANCP] comments on -12

Hi Tom,

---

From: Tom Taylor <[email-masked]>
Date: Thu, 23 Sep 2010 22:45:29 -0400

After some discussion with interested parties here at the BBF, here is 
my proposed resolution:

-- leave the text that says "implementations that support capability X 
must support the following protocol elements:"

-- remove all mention of "optional" and "mandatory" from the 
documentation of individual TLVs

-- perhaps add text in the procedural section that says that the usage 
of individual TLVs is as indicated by applicable specifications, in 
particular, TR-101.

> In order to keep consistency between both workflows and avoiding interoperability issues I really propose to set all these parameters mentioned above to "mandatory". It really does not make sense i…
...

---

From: <[email-masked]>
Date: Fri, 24 Sep 2010 04:58:26 +0200

Tom,

Thanx. So far I agree. But please refer to TR-147 instead of TR-101. 
TR-147 is the related BBF work für layer 2 control.

Regards
Thomas

-----Ursprüngliche Nachricht-----
Von: Tom Taylor [mailto:[email-masked]] 
Gesendet: Freitag, 24. September 2010 04:45
An: Haag, Thomas
Cc: [email-masked]; [email-masked]; [email-masked]
Betreff: Re: AW: [ANCP] comments on -12

...