Re: [Idr] AD Review of draft-ietf-idr-bgpls-segment-routing-epe-17

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

Hi Alvaro,

Please check inline below.

From: Alvaro Retana <[e-mail]>
Sent: 03 April 2019 20:52
To: Ketan Talaulikar (ketant) <[e-mail]>
Cc: Hares Susan <[e-mail]>; [e-mail]; [e-mail]; [e-mail]
Subject: RE: AD Review of draft-ietf-idr-bgpls-segment-routing-epe-17

On March 24, 2019 at 11:43:02 AM, Ketan Talaulikar (ketant) ([e-mail]<mailto:[e-mail]>) wrote:

Ketan:

Hi!

Thanks for your work on this document!

I still have a couple of comments, specially related to Manageability/Security…and a couple of new nits.  I am starting the IETF LC as I think these can be addressed with any other incoming comments.
[KT] Thanks. Please see further below.

Thanks!

Alvaro.


...
316   o  Member-ASN, which contains the ASN of the confederation member, if
317      BGP confederations are used, of the local BGP node.

[major] This description matches the definition of the TLV (§4.1), but it also matches the description of the ASN descriptor in §4.2.  What is the difference?  What if the contents are different?  [Same question applies below.]
[KT] With ref to rfc5065, in the case of confederations, the TLV 512 carries the AS Confederation Identifier while the TLV 517 carries the Member-AS Number. I’ve fixed the terminologies to more accurately reflect those from rfc5065 and also included additional reference to rfc5056 in TLV 512 use description.

Ok.

I think we may need a reference to rfc7752 when TLV 512 is mentioned because that is where it is defined.

[KT] Ack – will fix in next update.

...
860 9.  Manageability Considerations
...
868   Specifically, the malformed NLRI attribute tests for syntactic checks
869   in the Fault Management section of [RFC7752] now apply to the TLVs
870   for the BGP-LS NLRI TLVs defined in this document.  The semantic or
871   content checking for the TLVs specified in this document and their
872   association with the BGP-LS NLRI types or their associated BGP-LS
873   Attributes is left to the consumer of the BGP-LS information (e.g. an
874   application or a controller) and not the BGP protocol.

[nit] s/the malformed NLRI attribute tests/the malformed attribute tests
[KT] Fixed to read “Link-State NLRI and BGP-LS Attribute”

rfc7752 doesn’t cover the NLRI in the Fault Management section.

[KT] Actually, it does – please see the text below which is about the link-state NLRI and TLVs therein. However, we can’t do “attribute discard” handling really for malformed NLRI and I hope we can fix all this in RFC7752bis.
   o  Does the sum of all TLVs found in the BGP MP_REACH_NLRI attribute
      correspond to the BGP MP_REACH_NLRI length?

   o  Does the sum of all TLVs found in the BGP MP_UNREACH_NLRI
      attribute correspond to the BGP MP_UNREACH_NLRI length?

   o  Does the sum of all TLVs found in a Node, Link or Prefix
      Descriptor NLRI attribute correspond to the Total NLRI Length
      field of the Node, Link, or Prefix Descriptors?

   o  Does any fixed-length TLV correspond to the TLV Length field in
      this document?

...
[minor] "...retrieving this information...over a BGP-LS session, via some APIs or a data model (refer Section 1 and 2 of [RFC7752])."  I think that even mentioning that an API or data model can be used instead of BGP-LS is a stretch -- that is not how I interpret the initial sections in rfc7752 (which are just background sections), and there are no formal API/data model definition.
[KT] How else would Alto Server or a PCE component get access to the BGP-LS information? There would be some APIs or a model. It may not be formal but there needs to be one?

We talked about this (rfc7752 refers to BGP-LS) while in Prague.

[KT] Ack – will update this text to “A consumer of the BGP-LS information retrieves this information from a BGP Speaker, over a BGP-LS session”


[major] "...an error detected in the NLRI descriptor TLVs would result in that specific NLRI update being unusable and hence its update to be discarded along with an error log."  According to rfc7752, this statement is true only for syntactic errors, not semantic ones.
[KT] This is about the TLV validation by the consumer – which can also perform semantic checks. This aspect is one that needs clarification in the fault management section of RFC7752. The text on validation by consumer was added to this draft to prevent gating it on this gap in RFC7752.

I rather that is addressed in rfc7752bis.  I think that the new text at the end of this section covers enough.

[KT] Ack


Semantic errors ("left to the consumer", as mentioned above) means that the BGP-LS session can be used to transport trash (trash-in-trash-out).  This issue is bigger than this document -- but (because we don't have a current solution) I would like to see it mentioned somewhere as a potential issue (maybe in the Management or Security Considerations sections, your choice).
[major] "...an error detected in the NLRI descriptor TLVs would result in that specific NLRI update being unusable and hence its update to be discarded..."  This text says that an error in a TLV results in the whole update (not just the TLV or the BGL-LS attribute) being discarded.  This action goes beyond what is specified in rfc7752, which doesn't really specify what do to in those cases.  If that is what is expected here, then please make this behavior more prominent...use Normative language, etc..
[KT] I agree. As mentioned above, this was an interim text as mentioned above. I am open to suggestions. My concern is that multiple BGP-LS extensions don’t need to do this and I would rather have the normative language in an update to RFC7752.

Agreed — as we discussed in Prague.  Same comment as above.

[KT] Ack



...
...
895   BGP Peering Segments are associated with the normal BGP routing
896   peering sessions.  However, the BGP peering information along with
897   these Peering Segments themselves are advertised via a distinct BGP-
898   LS peering session.  It is expected that this isolation as described
899   in [RFC7752] is followed when advertising BGP peering topology
900   information via BGP-LS.

[major] rfc7752 doesn't talk about this type of isolation (it only talks about isolation through dedicated RRs).  See also comment in the Security Considerations section related to isolation.
[KT] Agree – this is the outcome of the discussions in the IDR WG and shepherd review. Ideally this should have been captured in RFC7752 and not individual extension documents.

Right.  The major point was that the reference to rfc7752 is misleading because this type of isolation is not discussed there.

[KT] Ack – I would not have any reference to RFC7752 for this text. But will keep it so that this document can progress w/o dependency on the RFC7752bis. This being BGP-EPE, it was felt that this text was required as part of the shepherd review/feedback.

...

[major] "...precaution is necessary to ensure that the BGP peering information collected via BGP-LS is limited to specific controllers or applications..."  This sounds as if you're referring to information that (once collected) can be shared between controllers -- I think that case is out of scope.
[KT] Actually the intention here is to highlight that since EPE can be used to engineer the path out of a domain at the BGP router, this information about the EPE SIDs should be maintained in a secure manner. The knowledge of EPE SIDs may be used to influence traffic flows both within and egressing outside the domain. If you still think this is out of scope and should be taken out then I will do so.

I think it is.  The SR Architecture already talks about the use inside a domain…so I think that could be opening a can of worms with this that we really don’t want to open.

[KT] Sure. The text only talks about inside the SR domain – I’ve fixed a nit which might have given the impression that it is otherwise.

...

§1: [nit] s/Link State NLRI/Link-State NLRI

§3: [nit] s/his section/This section

[nit]: The length in Figure 2 is not needed.
[KT] Fixed all of the above.

Thanks,
Ketan