Re: [mpls] Adam Roach's Discuss on draft-ietf-mpls-spring-lsp-ping-11: (with DISCUSS and COMMENT)

[Adam Roach <[e-mail]>]

Thanks! These proposed changes all look correct, assuming the "Local 
Interface ID" changes are mirrored in "Remote Interface ID" as well.

/a

On 10/11/17 7:42 PM, Carlos Pignataro (cpignata) wrote:
> Hi, Adam,
>
> You found a nice bug. Thank you! Please see inline.
>
>> On Oct 11, 2017, at 6:44 PM, Adam Roach <[e-mail]> wrote:
>>
>> Adam Roach has entered the following ballot position for
>> draft-ietf-mpls-spring-lsp-ping-11: Discuss
>>
>> When responding, please keep the subject line intact and reply to all
>> email addresses included in the To and CC lines. (Feel free to cut this
>> introductory paragraph, however.)
>>
>>
>> Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
>> for more information about IESG DISCUSS and COMMENT positions.
>>
>>
>> The document, along with other ballot positions, can be found here:
>> https://datatracker.ietf.org/doc/draft-ietf-mpls-spring-lsp-ping/
>>
>>
>>
>> ----------------------------------------------------------------------
>> DISCUSS:
>> ----------------------------------------------------------------------
>>
>> Section 5.3 indicates that "Advertising Node Identifier" and "Receiving Node
>> Identifier" are "4 or 6 octets." There are two issues that arise with the way
>> this is currently specified, both of which can lead to a lack of
>> interoperability:
> Indeed. This text, which is included both in the Advertising and Receiving Node ID, needs fixing. In summary, Protocol 0 needs specification of the length and value, and Protocol 1 needs to disambiguate the length.
>
>> 1. While implementors might infer that "Protocol=1" results in a 4-byte value,
>> and that "Protocol=2" results in a 6-byte value, it's a bit unclear what length
>> is to be used here for "Protocol=0.”
> Indeed. For Protocol=0, there’s little we can glean. See text below.
>
>> 2. The descriptions for both of these fields include: "When Protocol is set to
>> 1, then the 32 rightmost bits represent OSPF Router ID."  This implies that the
>> field is *wider* than 32 bits when Protocol=1, which leaves deep ambiguity
>> about the circumstances under which the field is allowed to be 4 octets.
> Correct. Meaning, you are correct and the text is incorrect. Protocol=1, 4-octet field, 32-bit OSPF Router-ID. Will fix. See below.
>
>> I would strongly recommend that this section add clear language that
>> unambiguously spells out how implementations are expected to select the field
>> width for the four variable-width fields in this Sub-TLV (the two I cite above
>> as well as the interface ID fields).
>>
> Yes. New text:
>
>     Local Interface ID
>
>        An identifier that is assigned by the local LSR for a link to
>        which Adjacency Segment ID is bound.  This field is set to a local
>        link address (IPv4 or IPv6).  For IPv4, this field is 4 octets;
>        for IPv6, this field is 16 octets.  In case of unnumbered, this
>        field is 4 octets and includes a 32 bit link identifier as defined
>        in [RFC4203], [RFC5307].  If the Adjacency Segment ID represents
>        parallel adjacencies ([I-D.ietf-spring-segment-routing]), this
>        field is 4 octets and MUST be set to 4 octets of zeroes.
> …
>     Advertising Node Identifier
>
>        It specifies the advertising node identifier.  When Protocol is
>        set to 1, then this field is 4 octets and carries the 32-bit OSPF
>        Router ID; if Protocol is set to 2, then this field is 6 octets
>        and carries the 48-bit ISIS System ID; if Protocol is set to 0,
>        then this field is 4 octets, and MUST be set to zero.
>
>     Receiving Node Identifier
>
>        It specifies the downstream node identifier.  When Protocol is set
>        to 1, then this field is 4 octets and carries the 32-bit OSPF
>        Router ID; if Protocol is set to 2, then this field is 6 octets
>        and carries the 48-bit ISIS System ID; if Protocol is set to 0,
>        then this field is 4 octets, and MUST be set to zero.
>
>
> Thoughts? Other comments?
>
>> ----------------------------------------------------------------------
>> COMMENT:
>> ----------------------------------------------------------------------
>>
>> Sections 5.1, 5.2, and 5.3 define "Reserved" fields without indication of how
>> these fields should be treated. Recommend each of these be defined to "MUST be
>> set to 0 on send, MUST be ignored on receipt" -- this is the scheme that
>> maximizes the ability to use them in the future.
>>
> Correct. New text:
>
>     Reserved
>
>        MUST be set to 0 on send, and MUST be ignored on receipt.
>
> In the three sections.
>
>> Section 5.3 sefines three values for "Adj Type": 0, 4, and 6. Please either
>> state that all other values are and will always be an error, or create an IANA
>> registry for this field.
> Done as per Alvaro’s COMMENT.
>
>> Section 5.3 sefines three values for "Protocol": 0, 1, and 2. Please either
>> state that all other values are and will always be an error, or create an IANA
>> registry for this field.
>>
>>
> Done as per Alvaro’s COMMENT.
>
> Thanks,
>
> — Carlos.
>