Subject: [Lsr] Francesca Palombini's Discuss on draft-ietf-lsr-isis-srv6-extensions-17: (with DISCUSS)

From: Francesca Palombini via Datatracker <[email-masked]>
Date: Mon, 18 Oct 2021 14:57:54 -0700

Francesca Palombini has entered the following ballot position for
draft-ietf-lsr-isis-srv6-extensions-17: 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/blog/handling-iesg-ballot-positions/
for more information about how to handle DISCUSS and COMMENT positions.

The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-lsr-isis-srv6-extensions/

----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

Thanks for the work on this document.

I was not around for RFC 8986, and I am not sure I understand the use case
fully (I agree with Ben there), but I'll trust the responsible AD and the wg.
I'll also note that I was hoping to see "Implementation status report" in the
draft, as mentioned in the shepherd writeup, and was disappointed not to find
any.

However, I'd like to discuss a number of points, mostly on the IANA
considerations and on detailed fields descriptions. I also want to bring 7.
below regarding the IANA registries names to your attention, although it's not
a hill I am willing to die on (that one is a "let's talk" DISCUSS, the rest I
hope can be acted upon).

As noted in https://www.ietf.org/blog/handling-iesg-ballot-positions/, a
DISCUSS ballot is a request to have a discussion; I really think that the
document would be improved with a change here, but can be convinced otherwise.

Francesca

1. -----

Section 7.1

FP: The locator entries ASCII figure is not consistent with the descriptive
text following it: specifically, Loc Size should follow Algorithm directly;
instead, the picture seems to show there are 2 octets unused between Algorithm
and Loc Size.

2. -----

      Type: 5.

FP: For consistency (and to make sure implementers don't rely on the ASCII
figure), it would be good to indicate Type's length (1 octet I assume).

3. -----

    Length: variable.

FP: This does not help much understanding what this field is supposed to
contain.

4. -----

Section 7.2

FP: Same issue as in 1. for the ASCII figure.

5. -----

Sections 8.1 and 8.2

FP: Same comments as 1. 2. and 3.

6. -----

  If a behavior is advertised it MUST
   only be advertised in the TLV[s] as indicated by "Y" in the table
   below, and MUST NOT be advertised in the TLV[s] as indicated by "N"
   in the table below.

FP: I find the sentence after the comma confusing, and don't understand the
presence of the MUST NOT here.

7. -----

Section 11.1.1

FP: It sounds like a bad idea in general to have to rename the registry every
time a TLV needs to be added to the registry... Maybe the wg and the AD should
consider renaming the registries so not to have this sort of dependency. (I
understand that this is a low priority comment, but still, it feels wrong to
put in titles what would fit really well in a registry itself). This very much
applies to Section 11.6 as well: the registry's name with the hierarchy of TLVs
as part of the name feels like a really bad idea. That is typically data that
goes into registries.

8. -----

Section 11.3

FP: The registry needs to be defined in the document. In particular, I see that
IANA is interpreting the columns as "Value" "Description" "Reference"; is that
right or should this be "Type" "Description" "Reference" (I see a mix of the
two for different IANA registries)?

9. -----

Section 11.8

FP: Are bits 0, 2-15 reserved or unassigned? The terminology in section 2 is
ambiguous, as it talks about "reserved for future use" (but the IANA section
leaves them unassigned). Please clarify for IANA.

10. -----

Section 11.10

FP: Please define the registry (I assume it is going to be "Bit #", "Name",
"Reference").

---

From: Peter Psenak <[email-masked]>
Date: Tue, 19 Oct 2021 15:19:26 +0200

Hi Francesca,

thanks for your review, please see my responses inline (##PP):

> https://datatracker.ietf.org/doc/draft-ietf-lsr-isis-srv6-extensions/

##PP
there was one in the earlier version of the document, but it was removed 
as part of the process to prepare for publication. Please look at 
version 11:

https://datatracker.ietf.org/doc/html/draft-ietf-lsr-isis-srv6-extensions-11

> instead, the picture seems to show there are 2 octets unused between Algorithm

##PP
there isn't anything between the "Algorithm" and "Loc Size" fields. If 
there was, there would have been a "|" at the end of the two bytes extra 
section. ISIS TLVs are not 4 bytes aligned, so it's sometimes tricky to 
draw them in a 4 bytes boundary. We have been using this style in other 
documents:

Example:
https://datatracker.ietf.org/doc/html/rfc8667#section-3.1

I can offer two alternatives, if you like them better:

a) divide to multiple blocks:

        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
       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
       |                          Metric                               |
       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
       |   Flags       |  Algorithm    |
       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
       |  Loc Size     | Locator (variable)...                         |
       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
       |  Sub-TLV-len  |         Sub-TLVs (variable) . . .             |
       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

b) fully packed to 4 bytes boundary:

        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
       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
       |                          Metric                               |
       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
       |   Flags       |  Algorithm    |  Loc Size     | Locator  ...
       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
       //            Locator (continued, variable)                    //
       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
       |  Sub-TLV-len  |         Sub-TLVs (variable) . . .             |
       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

Please let me know your preference and I will update all drawings 
consistently.

> figure), it would be good to indicate Type's length (1 octet I assume).

##PP
It's specified in ISO10589 as 1 octet. I looked at several RFCs and we 
never specify the length of the Type field.

Example:

https://datatracker.ietf.org/doc/html/rfc8667#section-3.1

> FP: This does not help much understanding what this field is supposed to

##PP
It's specified in ISO10589 as "total length of the value field". If the 
value is of constant length, we indicate the constant, if the length is 
variable we say "Variable". That is what we have been consistently doing 
in many ISIS RFCs.

Example:
https://datatracker.ietf.org/doc/html/rfc8667#section-3.1

> FP: Same issue as in 1. for the ASCII figure.

##PP
will address the same way for all cases based on your preference

> FP: Same comments as 1. 2. and 3.

##PP
will address the same way for all cases based on your preference

> presence of the MUST NOT here.

##PP
We have two categories of SRv6 SID in ISIS

a) those that are bound to an adjacency
b) those that are not bound to an adjacency

Based on the above the SID must be advertised in a different set of TLVs.

The table indicates which ISIS TLV(s) to use for advertising SRv6 SIDs 
supported by ISIS. The table has "Y" and "N" variables. We need to say 
what "Y" and "N" means.

Please note that the Y/N is something that we use in many ISIS RFCs, 
even ISIS IANA uses it for several registries.

Example:

https://www.iana.org/assignments/isis-tlv-codepoints/isis-tlv-codepoints.xhtml#isis-tlv-codepoints-22-23-25-141-222-223

which says:

"n - sub-TLV MUST NOT appear in TLV 25"

If the above still does not address your comment, please let me know 
what would be your preferred way to address it.

> as part of the name feels like a really bad idea. That is typically data that

##PP
I followed the process we used in the past and consulted with the 
Designated Experts we have. I'm open to rename the registry to whatever 
we agree on. Changing the name of the old registry is going to be more 
difficult ( Section 11.1.1), but for the Section 11.6 maybe we should 
start with a shorter name and skip the TLVs:

"sub-sub-TLVs for SRv6 End SID and SRv6 End.X SID"

> two for different IANA registries)?

##PP
I see both ["Type", "Description", "Reference"] as well as
["Value", "Name", "Reference"] being used for ISIS registries.

Any preference from your side?
	
> leaves them unassigned). Please clarify for IANA.

##PP
sure, will add.

> FP: Please define the registry (I assume it is going to be "Bit #", "Name",

##PP
ok, I will specify:

Bits 0-7 - Unassigned

thanks,
Peter

---

From: Francesca Palombini <[email-masked]>
Date: Tue, 19 Oct 2021 13:54:18 +0000

Hi Peter! Thanks for the quick reaction. Inline.

Francesca

From: Peter Psenak <[email-masked]>
Date: Tuesday, 19 October 2021 at 15:19
To: Francesca Palombini <[email-masked]>, The IESG <[email-masked]>
Cc: [email-masked] <[email-masked]>, [email-masked] <[email-masked]>, [email-masked] <[email-masked]>, Christian Hopps <[email-masked]>, [email-masked] <[email-masked]>
Subject: Re: Francesca Palombini's Discuss on draft-ietf-lsr-isis-srv6-extensions-17: (with DISCUSS)
Hi Francesca,

##PP

FP: Thank you.

##PP

FP: I did not realize that they are not 4 Bytes aligned, however I do think the current figure might be misleading for implementers, hence my comment. To keep consistency with previously published RFC, maybe a. would be best? (Although I am more used to b, but it might be me) Either of these are fine to me.

Example:

        0                   1                   2                   3

        0                   1                   2                   3

##PP

FP: Then maybe a reference to the relevant section of ISO10589 would be useful. Can’t hurt to repeat, even though it is specified in other standards, and in the figure itself.

Example:

##PP

FP: Again, I think adding a reference can’t hurt and would ease my mind that this is easily understandable by implementers. I am aware ISO10589 is a normative reference, so a pointer to the right section of that document is all I am looking for.

Example:

##PP

FP: Thank you!

##PP

FP: Thanks.

##PP

FP: I understand now, thank you. My confusion was more with the way this is phrased, rather than the BCP 14 language itself – I didn’t understand “as indicated by “Y”” referred to the TLVs, and not the whole sentence (I read this as “it MUST only be advertised as indicated by Y”). Changed “as indicated by” with “marked with” or something similar would have been clearer to me, but I’ll move this to COMMENT since this is more clarifications for the reader and this is not an implementation issue. Thank you for the clarification.

Example:

which says:

##PP

FP: Thank you for considering, and yes, this is more a comment to Alvaro, the wg and the experts rather than you authors. I brought it up here because this made me notice it. My suggestion is to have the working group work on a document fixing this. Might be annoying but the right thing to do, IMO.

##PP

FP: No preference, just making sure you had considered either (I assume you and the working group knows best which one is better) and decided on a precise instruction for IANA.

##PP
sure, will add.

FP: Thanks.

##PP
ok, I will specify:

Bits 0-7 – Unassigned

FP: Not exactly what I meant: I was asking to specify the name of the registry’s columns, to give IANA clear instructions of what that would look like.

thanks,
Peter

---

From: Alvaro Retana <[email-masked]>
Date: Tue, 19 Oct 2021 07:20:43 -0700

On October 19, 2021 at 9:54:20 AM, Francesca Palombini wrote:

Francesca:

Hi!  Thanks for the review!

...
> > 7. -----
> >
> > Section 11.1.1
> >
> > FP: It sounds like a bad idea in general to have to rename the registry
> > every time a TLV needs to be added to the registry... Maybe the wg and the
> > AD should consider renaming the registries so not to have this sort of
> > dependency. (I understand that this is a low priority comment, but still,
> > it feels wrong to put in titles what would fit really well in a registry
> > itself). This very much applies to Section 11.6 as well: the registry's
> > name with the hierarchy of TLVs as part of the name feels like a really
> > bad idea.  That is typically data that goes into registries.
>
> ##PP
> FP: Thank you for considering, and yes, this is more a comment to Alvaro, the
> wg and the experts rather than you authors. I brought it up here because this
> made me notice it. My suggestion is to have the working group work on a
> document fixing this. Might be annoying but the right thing to do, IMO.

To close the loop on the list.   I made the DEs aware of your comment
and asked them to consider possible changes (which would affect the
whole TLV registry, not just the sub-registries mentioned in this
document) that the WG may consider.

Thanks!!

Alvaro.

---

From: Peter Psenak <[email-masked]>
Date: Wed, 20 Oct 2021 10:34:46 +0200

Hi Francesca,

I have uploaded the new version of the draft that addresses all your 
comments.

thanks,
Peter

On 19/10/2021 15:54, Francesca Palombini wrote:
> *From: *Peter Psenak <[email-masked]>
> *Date: *Tuesday, 19 October 2021 at 15:19
> *To: *Francesca Palombini <[email-masked]>, The IESG 
> <[email-masked]>
> *Cc: [email-masked] 
> <[email-masked]>, [email-masked] 
> <[email-masked]>, [email-masked] <[email-masked]>, Christian Hopps 
> <[email-masked]>, [email-masked] <[email-masked]>
> *Subject: *Re: Francesca Palombini's Discuss on 
> draft-ietf-lsr-isis-srv6-extensions-17: (with DISCUSS)
> 
> Hi Francesca,
> 
> FP: I did not realize that they are not 4 Bytes aligned, however I do 
> think the current figure might be misleading for implementers, hence my 
> comment. To keep consistency with previously published RFC, maybe a. 
> would be best? (Although I am more used to b, but it might be me) Either 
> of these are fine to me.
> 
> 
> Example:
> FP: Then maybe a reference to the relevant section of ISO10589 would be 
> useful. Can’t hurt to repeat, even though it is specified in other 
> standards, and in the figure itself.
> 
> 
> Example:
> 
> FP: Again, I think adding a reference can’t hurt and would ease my mind 
> that this is easily understandable by implementers. I am aware ISO10589 
> is a normative reference, so a pointer to the right section of that 
> document is all I am looking for.
> 
> 
> Example:
> FP: I understand now, thank you. My confusion was more with the way this 
> is phrased, rather than the BCP 14 language itself – I didn’t understand 
> “as indicated by “Y”” referred to the TLVs, and not the whole sentence 
> (I read this as “it MUST only be advertised as indicated by Y”). Changed 
> “as indicated by” with “marked with” or something similar would have 
> been clearer to me, but I’ll move this to COMMENT since this is more 
> clarifications for the reader and this is not an implementation issue. 
> Thank you for the clarification.
> 
> 
> 
> FP: Thank you for considering, and yes, this is more a comment to 
> Alvaro, the wg and the experts rather than you authors. I brought it up 
> here because this made me notice it. My suggestion is to have the 
> working group work on a document fixing this. Might be annoying but the 
> right thing to do, IMO.
> 
> 
>  >
>  > 8. -----
>  >
>  > Section 11.3
>  >
> FP: No preference, just making sure you had considered either (I assume 
> you and the working group knows best which one is better) and decided on 
> a precise instruction for IANA.
> 
> 
> 
>  >
>  > 9. -----
>  >
>  > Section 11.8
>  >
> FP: Not exactly what I meant: I was asking to specify the name of the 
> registry’s columns, to give IANA clear instructions of what that would 
> look like.
> 
> 
> 
> thanks,
> Peter
>  >
>  >
>  >
>  >
>  >
>  >
>  >
>