Subject: Re: [Gen-art] Gen-ART Last Call review of draft-ietf-ccamp-rsvp-te-bandwidth-availability-13

From: "Yemin (Amy)" <[email-masked]>
Date: Fri, 15 Feb 2019 09:57:39 +0000

Hi Paul, 

Sorry for the late reply, please see my reply inline below.  

BR,
Amy
-----Original Message-----
From: Paul Kyzivat [mailto:[email-masked]] 
Sent: Friday, January 25, 2019 4:38 AM
To: Yemin (Amy) <[email-masked]>; [email-masked]
Cc: General Area Review Team <[email-masked]>
Subject: Re: Gen-ART Last Call review of draft-ietf-ccamp-rsvp-te-bandwidth-availability-13

Amy,

On 1/24/19 11:00 AM, Yemin (Amy) wrote:
> Hi Paul,
> 
> Thanks for the comments.
> Regarding nits: we followed the usual way, just show the value field. But I don't mind to make a complete figure.

In most places that define TLVs I see the type and length included in the figure. If the documents related to yours don't do that, and you want to follow that style, then please at least indicate in the text that you are only showing the value part, and explicitly call out the type and length.
[Amy] I will add the type and length filed, to save the clarification text.

> Minor issue: you're quiet right. It should be a MUST here.

You didn't mention my question:

>    When a node does not support the Availability TLV, it SHOULD
>    generate PathErr message with the error code "Extended Class-Type
>    Error" and the error value "Class-Type mismatch" (see [RFC2205]).
> 
> I have a question about this: Is this the default behavior that is 
> prescribed for any unknown TLV type found in an Ethernet SENDER_TSPEC object, or is this behavior specific to the "availability" TLV?

You can't mandate new behavior for things that don't implement this specification. If this is already the mandated behavior for an unknown TLV type then you technically don't need to say anything. If you want to, then best to reference the place that mandated this behavior. (I didn't find it in RFC2205.) And you could quote what that behavior is, or paraphrase it in a non-normative way.

[Amy] Thanks for this discussion. In RFC6003 where Ethernet SENDER_TSPEC object is defined, MUST is used for the error message. However, RFC5711 provides additional information on PathErr message, there's no "one" behavior mandated.
Per section 2.1 of RFC5711, "In the case of fatal errors ("Hard Preemption"; see Section 4.7.3 of [RFC3209] ), the detecting node MUST send a PathErr message reporting the error condition, ....  In the case of non-fatal errors, the detecting node should send a PathErr message, and must not clear control plane or data plane state. "
Considering the case in draft, such error can be considered as non-fatal error(the path can still be set up disregarding the Availability TLV), thus SHOULD is more appropriate here.  

	Thanks,
	Paul

> We will address these comments in next version. Thanks again.
> 
> BR,
> Amy
> ________________________________________
> 发件人: Paul Kyzivat [[email-masked]]
> 发送时间: 2019年1月24日 2:28
> 收件人: [email-masked]
> 抄送: General Area Review Team
> 主题: Gen-ART Last Call review of 
> draft-ietf-ccamp-rsvp-te-bandwidth-availability-13
> 
> I am the assigned Gen-ART reviewer for this draft. The General Area 
> Review Team (Gen-ART) reviews all IETF documents being processed by 
> the IESG for the IETF Chair. Please treat these comments just like any 
> other last call comments. For more information, please see the FAQ at 
> <​http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.
> 
> Document: draft-ietf-ccamp-rsvp-te-bandwidth-availability-13
> Reviewer: Paul Kyzivat
> Review Date: 2019-01-23
> IETF LC End Date: 2019-01-31
> IESG Telechat date: ?
> 
> Summary:
> 
> This draft is on the right track but has open issues, described in the 
> review.
> 
> Issues:
> 
> Major: 0
> Minor: 1
> Nits:  1
> 
> 1) NIT:
> 
> Section 3.1 includes the following:
> 
>      ... The Ethernet SENDER_TSPEC object
>      MAY include more than one Availability TLV. The Availability TLV has
>      the following format:
> 
>          0                   1                   2                   3
>          0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
>         +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>         |    Index      |                 Reserved                      |
>         +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>         |                          Availability                         |
>         
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 
> I find it confusing that this is described as a TLV yet it shows 
> neither a type nor a length. Apparently it is only showing the value 
> portion of the TLV. The type portion is only mentioned in the IANA 
> considerations section, and isn't explicitly linked to this element. I 
> would expect this to be shown something like:
> 
>          0                   1                   2                   3
>          0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
>         +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>         |    Type=4                     | Length=96                     |
>         +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>         |    Index      |                 Reserved                      |
>         +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>         |                          Availability                         |
>         
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 
> (But the type value needs to be keyed to the IANA assigned value, 
> since there is no guarantee that IANA will assign 4 for this.)
> 
> 2) MINOR:
> 
> Section 3.2 includes the following:
> 
>      When a node does not support the Availability TLV, it SHOULD
>      generate PathErr message with the error code "Extended Class-Type
>      Error" and the error value "Class-Type mismatch" (see [RFC2205]).
> 
> I have a question about this: Is this the default behavior that is 
> prescribed for any unknown TLV type found in an Ethernet SENDER_TSPEC 
> object, or is this behavior specific to the "availability" TLV? It is 
> a problem if this is not the default behavior.
> 
> And is there a reason for SHOULD (rather than MUST) here? If so, 
> please describe the situations where this need not be done.
>