Re: [Idr] Rtgdir early review of draft-ietf-idr-bgpls-srv6-ext-07

["Ketan Talaulikar (ketant)" <[e-mail]>]

Hi Adrian,

Thanks for your quick response. Please check inline below for clarifications with KT2. 

I will hold the changes indicated below in my edit buffer and push them out with the next update.

-----Original Message-----
From: Adrian Farrel <[e-mail]> 
Sent: 09 June 2021 14:26
To: Ketan Talaulikar (ketant) <[e-mail]>; [e-mail]
Cc: [e-mail]; [e-mail]
Subject: RE: Rtgdir early review of draft-ietf-idr-bgpls-srv6-ext-07

Hi Ketan,

Thanks for the detailed response, it really helps cross-check the review.

There are a few residual comments/suggestions below with everything else snipped out.

None of these remaining points merits a response: either touch the document or ignore the email 😊

Best,
Adrian

== Minor Issues ==

You will need to reduce the number of front-page authors to five or fewer, or you will need to provide the document shepherd with a credible reason to diverge from this rule.

[AF] I assume, from no change to the document, that you will be giving the shepherd some reasoning for them to use in the write-up.
[KT2] Yes.

---

Section 1

   On the similar lines,
   introducing the SRv6 related information in BGP-LS allows consumer
   applications that require topological visibility to also receive the
   SRv6 SIDs from nodes across a domain or even across Autonomous
   Systems (AS), as required.

I caution you to be *very* careful with the word "domain". The IESG has recently been concerned by the definition contained in 8402. In that document, the "SR domain" (also just "domain") is the collection of all interconnected SR-capable nodes. Here (and in other parts of the
document) I think you are using "domain" in a more restricted sense. I don't know how you fix that, but I believe you should do something.
Perhaps you can call out the terminology issues here by noting the 8402 definition and specifically defining what you mean by your local definition of "domain".
[KT] The "domain" is "IGP domain" in this context and this is fixed.

[AF] I still see Section 11 "(e.g., between multiple AS/domains within a single provider network)". I think that use of "domain" needs qualification as well.
[KT2] I missed that. Will correct that as "multiple AS or IGP domains" in the next update.

---

Section 2

   The SRv6 SIDs associated with the node are advertised using the BGP-
   LS SRv6 SID NLRI introduced in this document.  This enables the BGP-
   LS encoding to scale to cover a potentially large set of SRv6 SIDs
   instantiated on a node with the granularity of individual SIDs and
   without affecting the size and scalability of the BGP-LS updates.

The claims of scalability are not expanded here or anywhere else in the document. Scalability of BGP-LS is important, so I'd prefer some explanation. But if that isn't available, it might be better to leave out these mentions.
[KT] Advertisement of large number of SRv6 SIDs as part of the BGP-LS Link Attribute associated with the existing Node NLRI would make that BGP-LS update rather large. Using the new separate SRv6 SID NLRI alleviates the problem. This is the scalability consideration for the protocol encoding design. I am not sure what further details are required.

[AF] Well, just a few words to help clarify using some of your words here.
Advertisement of large number of SRv6 SIDs as part of the BGP-LS Link Attribute associated with the existing Node NLRI could make that BGP-LS update rather large, causing scaling concerns as the number of SRv6 SIDs instantiated on a node. The SRv6 SID NLRI introduced in this document alleviates this problem by <doing what?>"
[KT2] I will add this text in that paragraph to clarify further. " Had the SRv6 SIDs been advertised within the BGP-LS Link Attribute associated with the existing Node NLRI, the BGP-LS update would have grown rather large with the increase in SRv6 SIDs on the node and would have also required a large update message to be generated for any change to even a single SRv6 SID." 

---

[snip]

---

7.1

   The SRv6 Endpoint Behavior TLV is a mandatory TLV that MUST be
   included in the BGP-LS Attribute associated with the BGP-LS SRv6 SID
   NLRI.  

What if it is missing?
[KT] The consumer of the BGP-LS information would not be able to use this information. The BGP-LS speakers themselves do not perform semantic validation for the TLVs per RFC7752 and this is clarified further in the draft-ietf-idr-rfc7752bis.

[AF] A fine answer, and the text can be enhanced with a quick pointer to 7752 (or the bis, but you probably want a normative reference and 752 will do the job) saying "Processing and validation rules for BGP-LS TLVs are described in [RFC7752]." Do this in the right place and it covers all issues.
[KT2] I believe this is already discussed in Sec 10 of the document. 

---

[snip]

== Nits ==

[snip]

4.1, 4.2, 5.1, and 6.1

Can you say what the Length field is. "The length of the value part of the TLV in bytes including any sub-TLVs that may be present." ??
[KT] This is base BGP-LS encoding. I don't think we need to repeat it for each TLV that is defined.

[AF] Can you then add (probably in Section 2) to say this? Something like...
OLD
   BGP-LS Attribute TLVs for the SRv6 SID NLRI are
   introduced in this document as follows:
NEW
   BGP-LS Attribute TLVs for the SRv6 SID NLRI are
   introduced in this document as follows.  These new TLVs follow the
   formats and common field definitions provided in [RFC7752].
END
[KT2] Ack.  I will add that sentence at the end of Sec 2.

---

[snip]

---

6.

The figure has
    |                        Identifier                             |
    |                        (64 bits)                              |
and the text has
   o  Identifier: 8 octet value as defined in [RFC7752].

Be consistent?
[KT] Not sure that I follow - 8 octets = 64 bits. We've tried to keep the figure aligned with RFC7752. While this document describes fields in terms of octets - so it is consistent.

[AF] Consistent would be to always say "8 octets" or always say "64 bits"
[KT2] OK. I will change the 64-bits to 8 octets. 

Thanks,
Ketan 

---

[snip]