Re: [Idr] AD Review of draft-ietf-idr-rfc7752bis-10

[Ketan Talaulikar <[e-mail]>]

Hi Alvaro,

Thanks for your updated review comments and please check inline below for
response with KT2

I've just posted an update with the changes as discussed below:
https://datatracker.ietf.org/doc/html/draft-ietf-idr-rfc7752bis-12


On Mon, Oct 24, 2022 at 8:58 PM Alvaro Retana <[e-mail]>
wrote:

> On September 29, 2022 at 12:38:33 PM, Ketan Talaulikar wrote:
>
>
> Ketan:
>
> Hi!
>
> > Thanks for your detailed review and feedback/suggestions. Please check
> inline
> > below for responses.
> >
> > I have also posted an updated version with the changes discussed below:
> > https://datatracker.ietf.org/doc/html/draft-ietf-idr-rfc7752bis-11
>
> I left below instances where I still have comments/questions.  I'm
> also appending comments on -11 -- in most cases related to the updated
> text.
>
> While I still have issues that I would like to see resolved, I am
> starting the IETF LC.  Given that we are close to IETF 115, the LC
> will end afterwards.  Maybe we can find a time there to talk about any
> remaining issues.
>

KT2> I am attending IETF 115 remotely and I hope we can still find time to
meet virtually for any points that need further discussions.


>
> Thanks!
>
> Alvaro.
>
>
> ...
> > > 196 Figure 1: Collection of Link-State and TE Information
> > >
> > > [minor] This figure now includes router names, which is good for later
> > > identification and explanations. I understand the general BGP
> > > distribution through route reflectors and figured that to be the
> > > reason for naming the nodes in the middle RRx. However, there is no
> > > specific tie-in made between the speakers in the middle row and a
> > > route reflector function. OTOH, §4.9 mentions a route reflector
> > > (abbreviated as RR), but then figure 31 names that BGP-LS speaker "R0"
> > > (not RR0 to follow the naming here).
> > >
> > > All this is to say that the first inclination anyone familiar with BGP
> > > has is to associate "RR" with a route reflector. But that doesn't
> > > seem to be the use in Figure 1 -- IOW, I don't see the need to label
> > > BGP Speakers differently.
> > >
> > > In the end, I'm just looking for consistency.
> >
> > KT> Fixed the figure and description in Section 4.9 to use the RR
> terminology
> > as in Figure 1.
>
> Sorry, but I don't think that's better.  Maybe I should have been
> clearer about asking to avoid using RRx to describe a BGP-LS
> Propagator -- and to be consistent in the naming: there's no reason
> for a Propagator to not just be Rx, or for a Producer to not be RR1 --
> and Propagators can also be Producers, etc..
>
> Maybe I'm looking too much into this.  Let's see if others also find
> it inconsistent or misleading.  No change needed.
>

KT2> You are of course correct! However, the reason for showing RRs in that
picture is two-fold - (1) shows a typical deployment scenario and (2) the
roles in this design are somewhat separated and hence easy to comprehend.
The text does cover your point.


>
>
>
> ...
> > > [minor] "If R1 and R2 are in the same IGP area, then likely they are
> > > originating the same link-state information into BGP-LS. R1 may also
> > > source information from sources other than IGP, e.g. its local node
> > > information."
> > >
> > > These two sentences, while true, are not needed as they include
> > > concepts that don't seem relevant -- both at this point and in
> > > general.
> > >
> > > - "IGP area" is only mentioned *before* this point, so the duplication
> > > of information hinted here doesn't seem important
> >
> > KT> I believe this is an important concept to describe. Do you see an
> issue
> > with doing so? Further in Sec 4.9, we get into some of the challenges
> this
> > poses and so it seemed good to first introduce that two BGP producers
> > sourcing info from the same IGP area are going to be originating the same
> > information.
>
> Can it at least be called an "IGP flooding domain"?  Area is specific
> to OSPF -- IS-IS has levels.
>

KT2> Ack - changed.


>
> s/then likely they are originating the same link-state
> information/they should originate the same link-state information
>

KT2> Ack


>
>
>
> ...
> > > 451 When there are multiple BGP-LS Producers originating the same link-
> > > 452 state information, implementation variations of BGP-LS Producers
> may
> > > 453 result in the generation of different and inconsistent BGP-LS
> updates
> > > 454 for the same link-state object based on the inclusion or exclusion
> of
> > > 455 optional TLVs. An inconsistency between BGP-LS Producers with
> > > 456 regards to the inclusion of optional TLVs in the NLRI results in
> > > 457 multiple NLRIs being generated for the same link-state object. A
> > > 458 BGP-LS Consumer would need the ability to merge such duplicate
> > > 459 updates to handle such situations. An inconsistency between BGP-LS
> > > 460 Producers with regards to the inclusion of optional TLVs in the
> BGP-
> > > 461 LS Attribute results in one of them being delivered to a BGP-LS
> > > 462 Consumer as part the BGP propagation and best-path selection
> > > 463 procedures in most typical deployments. This can result in a BGP-LS
> > > 464 Consumer missing out on some of the information in a potentially
> > > 465 unpredictable manner. The use of BGP-LS Producers that have a
> > > 466 consistent support for the origination of optional TLVs between
> them
> > > 467 can help mitigate such situations for the BGP-LS Consumers.
> > >
> > > [major] "implementation variations"
> > >
> > > Isn't the job of the specifications (including this one!) to be clear
> > > enough guarantee interoperability? If the specifications are clear
> > > then we don't need to worry about implementation variations. If the
> > > specifications are not clear then we should fix them!
> > >
> > > I know this text resulted from the discussion about BGP-LS Producers
> > > inconsistently advertising information, specifically optional TLVs.
> > > However, to support a specific feature the origination of the
> > > corresponding TLVs becomes necessary. It seems to me that beyond
> > > "implementation variations" we're talking about inconsistent
> > > configuration (and maybe both).
> >
> > KT> The objective here is to describe the consequences of implementation
> > variations that we have seen in the production and interop tests. Some of
> > them may be from implementations that followed RFC7752 that was not as
> clear
> > about which TLVs are optional and which are mandatory in the NLRI. Even
> with
> > this document (and other BGP-LS specs), we can still have
> inconsistencies in
> > the BGP-LS attribute simply because two implementations don't support the
> > same attributes for an object.
>
> Can we get rid of "implementation variations"?
>
> Suggestion>
>    The origination of the same link-state information by multiple
>    BGP-LS Producers may result in differences and inconsistencies
>    due to the inclusion or exclusion of optional TLVs.  Different
>    optional TLVs in the NLRI results in multiple NLRIs being
>    generated for the same link-state object.  Different optional
>    TLVs in the BGP-LS Attribute may result in the propagation of
>    partial information.  To address these inconsistencies, the
>    BGP-LS Consumer will need to recognize and merge the duplicate
>    information, or to deal with missing information.  The deployment
>    of BGP-LS Producers that consistently originate the same set of
>    optional TLVs is recommended to mitigate such situations.
>
> KT2> Thanks for that suggestion. Included that.


>
>
> ...
> > > 854 A BGP-LS Producer MAY suppress the advertisement of a Link NLRI,
> > > 855 corresponding to a half link, from a link-state IGP unless it has
> > > 856 verified that the link is being reported in the IS-IS LSP or OSPF
> > > 857 Router LSA by both the nodes connected by that link. This 'two-way
> > > 858 connectivity check' is performed by link-state IGPs during their
> > > 859 computation and may be leveraged before passing information for any
> > > 860 half-link that is reported from these IGPs into BGP-LS. This
> ensures
> > > 861 that only those Link State IGP adjacencies which are established
> get
> > > 862 reported via Link NLRIs. Such a 'two-way connectivity check' may be
> > > 863 also required in certain cases (e.g. with OSPF) to obtain the
> proper
> > > 864 link identifiers of the remote node.
> ...
> > > [major] "A BGP-LS Producer MAY suppress the advertisement of a...half
> > > link...unless it has verified that the link is being reported [in the
> > > IGP] by both the nodes connected by that link."
> > >
> > > Another way of looking at this option is to require the converse
> > > behavior: "the BGP-LS Producer MUST NOT suppress...if the two-way
> > > connectivity has been advertised on the IGP".
> > >
> > > I both cases we would be requiring the BGP-LS Producer to perform the
> > > two-way connectivity check using the IGP information. That seems to
> > > me to be way outside the scope of BGP (or BGP-LS).
> > >
> > > As far as I can tell, the two-way connectivity check is done by the
> > > IGPs when running SPF, so that result wouldn't be available in the
> > > LSBD.
> >
> > KT> Whether the result of 2-way connectivity check is stored/available
> in the
> > LSDB is implementation specific. It is not so that information from
> LSAs/LSPs
> > can be simply plucked from a LSDB built from raw IGP PDUs and advertised
> into
> > BGP-LS. Information from multiple LSAs/LSPs can contribute to a single
> > link-state object. This requires additional processing in the form of
> > collating information into some form of a topology database (nodes,
> links,
> > prefixes). Ultimately, it is up to the implementation how it realizes the
> > LSDB.
>
> It looks like we're saying the same thing: the 2-way connectivity
> information may not be immediately available to BGP-LS.  Right?
>

KT2> It can be available - it depends on how the interface between IGP and
BGP is implemented. Note that the text does not say "BGP or BGP-LS" but the
"BGP-LS Producer" - the intention here is reference to the "router" and not
to a specific protocol component.


>
> My issue with the text above is that it is Normative ("BGP-LS Producer
> MAY suppress").  The only way to do that is to either expect that the
> LSDB will provide the information, or to have BGP-LS figure it out.  I
> think we're both saying that neither option is feasible.  So, how can
> the Normative statement be satisfied?
>

KT2> Please see my previous response and the next one.


>
> Suggestion:  s/MAY/may and add a statement about the mechanism itself
> to figure out the 2WCC being out of scope.
>

KT2> The text describes a mechanism - i.e., use/leverage the check
performed by the IGPs. However, added text in the new section 4 introduced
to clarify this a bit further.


>
>
>
> ...
> > > 1720 4.9. Handling of Unreachable IGP Nodes
> > >
> > > 1722 The origination and propagation of IGP link-state information via
> BGP
> > > 1723 needs to provide a consistent and true view of the topology of the
> > > 1724 IGP domain. BGP-LS provides an abstraction of the IGP specifics
> and
> > > 1725 BGP-LS Consumers may be varied types of applications. While the
> > > 1726 information propagated via BGP-LS from a link-state routing
> protocol
> > > 1727 is sourced from that protocol's LSDB, it does not serve as a true
> > > 1728 reflection of the originating router's LSDB since it does not
> include
> > > 1729 the LSA/LSP sequence number information. The sequence numbers are
> > > 1730 not included since a single NLRI update may be put together with
> > > 1731 information that is coming from multiple LSAs/LSPs.
> > >
> > > [] This is a nice introductory paragraph to BGP-LS in general. Too
> > > bad it is hidden here and not in the Introduction (for example).
> >
> > KT> Would it make sense to insert a new section 4 "Advertising IGP
> > information into BGP-LS" to hold this text and some other that has been
> added
> > in section 4 in this update v11? The introduction is focussed on other
> > aspects and this might get lost in there.
>
> Yes, please.
>

KT2> Inserted the new section and moved text into it from other parts of
the document. Also, some clarifications as discussed in the previous
comments. Please review and let me know if that addresses your comments.


>
>
>
> ...
> > > [major] The RFC Required policy allows any RFC to allocate a value;
> > > from rfc8126:
> > >
> > > With the RFC Required policy, the registration request, along with
> > > associated documentation, must be published in an RFC. The RFC need
> > > not be in the IETF stream, but may be in any RFC stream (currently an
> > > RFC may be in the IETF, IRTF, IAB, or Independent Submission streams
> > > [RFC5742]).
> > >
> > > Unless otherwise specified, any type of RFC is sufficient (currently
> > > Standards Track, BCP, Informational, Experimental, or Historic).
> > >
> > > IOW, no one is required to review the request. Given the size of this
> > > range, it seems like a risky strategy. Why are you deviating from the
> > > existing "Expert Review" criteria?
> >
> > KT> It should have been the IETF Review policy. Given the limited size,
> that
> > would seem to be prudent IMHO.
>
> Why not keep "Expert Review"?
>
> IETF Review allows any RFC, from any WG to allocate a value.  It is
> unlikely, but an allocation request could be missed.  Expert Review
> guarantees that someone will ask questions and that idr will be
> notified for proper review.
>

KT2> Sure, will change to Expert Review to stay consistent with other
registries.


>
>
>
> ...
> > > 2352 Note that the 'Attribute Discard' action results in the loss of
> all
> > > 2353 TLVs in the BGP-LS Attribute and not the removal of a specific
> > > 2354 malformed TLV. The removal of specific malformed TLVs may give a
> > > 2355 wrong indication to a BGP-LS Consumer of that specific information
> > > 2356 being deleted or not available.
> ...
> > > [major] This point is here for the record...
> > >
> > > rfc7606 says that 'attribute discard' "MUST NOT be used except in the
> > > case of an attribute that has no effect on route selection or
> > > installation."
> >
> > KT> This is true in the case of BGP-LS.
> >
> > >
> > > Even if the operation of the BGP-LS Consumer is out of scope, it is
> > > expected to (in many cases) calculate routes and program the network
> > > with them. As explained above (and considering also the NLRI
> > > 'Treat-as-withdraw' case), the BGP-LS Consumer will not have a
> > > complete picture which can affect routing in the network.
> >
> > KT> This is a debatable point and depends on the consumer application.
> If a
> > node or link is malformed, what would be better - to simply remove it
> from
> > the topology view for the consumer or keep it with an error flagged on
> it. In
> > either case, that node/link would be unusable, but in the latter case, at
> > least the consumer application can detect that this is not a link/node
> down
> > but some BGP-LS error and flag it appropriately. In conclusion, we have
> gone
> > with the latter approach.
>
> Again, just for the record...I don't expect changes in the document.
>
> Yes, it depends on what the consumer is doing, but by removing it
> we're not following rfc7606 ("MUST NOT be used") if we believe the
> consumer can use it for route selection.
>

KT2> When removing the BGP-LS Attribute due to attribute discard, we are
making that attribute not usable by the consumer. What goes in the NLRI or
other attributes is unaffected by the RFC7606 attribute discard for the
BGP-LS attribute. Practically, without the BGP-LS attribute there may be
very little (or nothing?) that a consumer may be able to do with the BGP-LS
advertisement - of course it depends on the application.


>
>
> > > A simple attack vector is to not order the TLVs in an otherwise valid
> > > route, or change the length of a TLV... The impact goes beyond BGP-LS
> > > and what this document covers. :-(
> >
> > KT> Sure. But does this not apply to BGP in general?
>
> Yes -- the impact is wider... <sigh>
>

KT2> Agree and we already have the following in the Security
Considerations. Are you suggesting to add more or specifics?

   An error or tampering of the link-state information that is
   originated into BGP-LS and propagated through the network for use by
   BGP-LS Consumers applications can result in the malfunction of those
   applications.  Some examples of such risks are the origination of
   incorrect information that is not present or consistent with the IGP
   LSDB at the BGP-LS Producer, incorrect ordering of TLVs in the NLRI
   or inconsistent origination from multiple BGP-LS Producers and
   updates to either the NLRI or BGP-LS Attribute during propagation
   (including discarding due to errors).



>
>
>
> ...
> > > 2358 When a BGP Speaker receives an update message with Link-State
> NLRI(s)
> > > 2359 in the MP_REACH_NLRI but without the BGP-LS Attribute, it is most
> > > 2360 likely an indication that a BGP Speaker preceding it has performed
> > > 2361 the 'Attribute Discard' fault handling. An implementation SHOULD
> > > 2362 preserve and propagate the Link-State NLRIs in such an update
> message
> > > 2363 so that the BGP-LS Consumers can detect the loss of link-state
> > > 2364 information for that object and not assume its deletion/withdraw.
> > > 2365 This also makes it possible for a network operator to trace back
> to
> > > 2366 the BGP-LS Propagator that detected the fault with the BGP-LS
> > > 2367 Attribute.
> > >
> > > [major] "SHOULD preserve and propagate"
> > >
> > > This is a very useful signal that something went wrong. Why is it
> > > recommended and not required? When is it ok to not preserve and
> > > propagate.
> >
> > KT> Good point. There is a flip side here. Assume that there were two
> BGP-LS
> > Producers for the same link-state object. For some reason, one of them
> > generated a malformed TLV in the BGP-LS attribute while the other did
> not. If
> > we do "treat as withdraw" then that malformed update is gone and the
> other
> > will go through. However, if we were to retain with "attribute discard"
> then
> > there is no guarantee which of the two updates would go through.
>
> I still don't understand.  When is it ok to not "preserve and
> propagate the Link-State NLRIs" (after another node used "attribute
> discard")?  I'm not sure if you're arguing for "MAY preserve and
> propagate".
>

KT2> The discard (or absence) of BGP-LS Attribute does not affect its
eligibility for best path selection. Hence, it would get propagated further
w/o the BGP-LS attribute. Please also see my next response - this was just
for discussion and no change to text is proposed.


>
>
> > I was tempted to introduce a tweak/change in the BGP Decision process for
> > BGP-LS to consider updates w/o the BGP-LS Attribute as less preferred.
> > However, it would have been a significant change. Perhaps we can still
> > check/socialize with the WG?
>
> I would hate to see AF-specific Decision Processes. :-(
>
> In the general case (not just talking about BGP-LS), the BGP
> propagation model and the Decision Process don't seem to fit a lot of
> the stuff that we make it carry.  But if we changed the propagation
> model and/or the Decision Process for every AF then the complexity
> would sky rocket! :-(   And we would end up having a different
> protocol (with common packet formats and basic operation) for every
> AF.
>
> In any case, it's up to you if you want to bring this to the WG.
>

KT2> What you mentioned above is precisely why such change is not a good
approach. I am not advocating for that change and this discussion was just
for completeness in case others in the WG have different views.


>
>
>
> ...
> > > 2397 7.2.3. Configuration Management
> > > ...
> > > 2421 An implementation SHOULD allow the operator to configure the
> maximum
> > > 2422 size of the BGP-LS Attribute that may be used on a BGP-LS
> Producer.
> > >
> > > [?] How does limiting the size of the BGP-LS Attribute help anything?
> > > Is this related to the maximum message size or something else?
> >
> > KT> Yes, it is related to the maximum message size that an operator
> knows can
> > be used safely across their network.
>
> How can an operator even start to think about what to configure here?
>
> While operators are very smart, the maximum size of the BGP-LS
> Attribute depends on the max message size, other attributes, and the
> size of the NLRI (and the descriptors in it), etc.  That doesn't seem
> like a straight-forward calculation what anyone can make.
>

KT2> I see your point now. I would expect that normally this choice is
boolean - 4K or 64K - depending on whether all routers support extended
messages or not. Clarified the text.


>
> Isn't a too big BGP-LS Attribute already considered in §4.3?
>

KT2> Ack


>
>
>
> ...
> > > 2444 7.2.6. Security Management
> > >
> > > 2446 An operator SHOULD define an import policy to limit inbound
> updates
> > > 2447 as follows:
> > >
> > > 2449 o Drop all updates from peers that are only serving BGP-LS
> > > 2450 Consumers.
> > >
> > > [major] The "SHOULD" above conflicts with "MUST NOT accept updates" in
> > > §9. I realize that the action here is about the operator defining
> > > policy vs a BGP speaker not accepting updates...but the result of the
> > > specification would be inconsistent if the requirement in §9 depends
> > > on the recommendation here.
> > >
> > > See also related comments in §9 related to "peers that are only
> > > serving BGP-LS Consumers".
> >
> > KT> Changing to SHOULD NOT in Sec 9 to be consistent.
>
> §9 changed to "should not" (non-normative), which I think is ok.
>
> The text in this section still says: "operator SHOULD define an import
> policy to limit inbound updates".  When is it ok for the operator to
> not configure the policy (and accept updates from peers that are only
> serving BGP-LS Consumers)?  Assuming that the operator is aware of the
> "BGP speaker's nature and that of its peerings" (as explained in §9),
> when would it not configure the policy?  IOW, why is this action
> recommended and not required?
>

KT2> I see your point. Changed SHOULD to MUST.


>
>
>
> +++===+++ Review of -11 +++===+++
>
>
> ...
> 348     3.  BGP Speaker Roles for BGP-LS
> ...
> 365        *  BGP-LS Consumer: The term BGP-LS Consumer refers to a
> consumer
> 366           application/process and not a BGP Speaker.  The BGP Speakers
> RR1
> 367           and Rn are handing off the BGP-LS information that they have
> 368           collected to a consumer application.  The BGP protocol
> 369           implementation and the consumer application may be on the
> same or
> 370           different nodes.  This document only covers the BGP
> 371           implementation.  The consumer application and the design of
> the
> 372           interface between BGP and the consumer application may be
> 373           implementation specific and are outside the scope of this
> 374           document.  The communication of information is expected to be
> 375           unidirectional (i.e., from a BGP Speaker to the BGP-LS
> Consumer
> 376           application) and a BGP-LS Consumer is not able to send
> information
> 377           to a BGP Speaker for origination into BGP-LS.
>
> [] Can we make this last sentence stronger?
>
> Suggestion>
>    The communication of information MUST be unidirectional (i.e.,
>    from a BGP Speaker to the BGP-LS Consumer application) and a
>    BGP-LS Consumer MUST NOT be able to send information to a BGP
>    Speaker for origination into BGP-LS.
>

KT2> Ack


>
>
>
> 379        *  BGP-LS Propagator: The term BGP-LS Propagator refers to a BGP
> 380           Speaker that is performing BGP protocol processing on the
> link-
> 381           state information.  The BGP Speaker RRm propagates the BGP-LS
> 382           information between the BGP Speaker Rn and the BGP Speaker
> RR1.
> 383           The BGP implementation on RRm is doing the propagation of
> BGP-LS
> 384           updates and performing BGP Decision Process.  Similarly, the
> BGP
> 385           Speaker RR1 is receiving BGP-LS information from R1, R2, and
> RRm
> 386           and propagating the information to the BGP-LS Consumer after
> 387           performing BGP Decision Process.
>
> [major] "BGP-LS updates"
>
> s/updates/UPDATE messages/g
> To be in line with rfc4271.
>

KT2> Ack


>
>
>
> 389        The above roles are not mutually exclusive.  The same BGP
> Speaker may
> 390        be the BGP-LS Producer for some link-state information and
> BGP-LS
> 391        Propagator for some other link-state information while also
> providing
> 392        this information to a BGP-LS Consumer.
>
> 394        Nothing precludes a node that is a BGP Speaker from performing
> 395        additional validation and processing on behalf of a BGP-LS
> Consumer
> 396        as long as it does not impact the semantics of its role as a
> BGP-LS
> 397        Propagator as described in this document.  Therefore, if a
> BGP-LS
> 398        update is found to be invalid based on specific semantic
> validation
> 399        for a specific BGP-LS Consumer and hence it is not passed to
> the BGP-
> 400        LS Consumer, it still needs to be propagated to other BGP
> Speakers as
> 401        long as it is valid from the BGP-LS validation perspective.
>
> s/a node that is a BGP Speaker/a BGP Speaker
>

KT2> Ack


>
> I still have the same questions/doubts as before (below with >), but
> the second sentence (new) added some more.
>
> > The definition of BGP-LS Consumer clearly says that the BGP
> implementation
> > and the consumer application are independent of each other. How can a BGP
> > speaker perform "validation and processing on behalf of the BGP-LS
> Consumer"
> > if the operation of, and the interface to, the BGP-LS Consumer are out of
> > scope?
> >
> > What type of "validation and processing" beyond basic BGP processing are
> you
> > referring to? By "basic" BGP processing I mean: UPDATE error handling
> > (§6.3/rfc4271) and UPDATE message handling (§9/rfc4271) -- IOW, nothing
> > explicitly related to the content of what is specified in this document.
>
> The second sentence added "specific semantic validation for a specific
> BGP-LS Consumer".  Again, if the operation of the Consumer is out of
> scope...??
>
> §7.2.2 says:
>
>    A BGP-LS Propagator, even when it has a coexisting BGP-LS Consumer on
>    the same node, should not perform semantic validation of the Link-
>    State NLRI or the BGP-LS Attribute to determine if it is malformed or
>    invalid.  Some types of semantic validation that are not to be
>    performed by a BGP-LS Propagator are as follows (and this is not to
>    be considered as an exhaustive list):
>
>
> Also, I don't understand the cases where "is not passed to the BGP-LS
> Consumer, [but] it still needs to be propagated to other BGP Speakers
> as long as it is valid from the BGP-LS validation perspective".  My
> conclusion has to be that the "specific semantic validation for a
> specific BGP-LS Consumer" has to do with the operation of the
> Consumer, which is out of scope.
>
>
> > I don't understand what you mean by "the semantics of its role as BGP-LS
> > Propagator" and what the impact of that may be.
>
> Once the Propagator starts doing "specific semantic validation for a
> specific BGP-LS Consumer", don't the semantincs of the role change?
>
>
> > Also, not all BGP-LS Propagators have a related BGP-LS Consumer
> associated to
> > them. Are you expecting all BGP-LS Propagators to perform checks "on
> behalf
> > of the BGP-LS Consumer" even if no BGP-LS Consumer is attached?
>
>
KT2> I see your point and I've removed that block of text.


>
>
> ...
> 499     4.2.  The Link-State NLRI
> ...
> 597           0                   1                   2                   3
> 598           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
> 599          +-+-+-+-+-+-+-+-+
> 600          |  Protocol-ID  |
> 601
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 602          |                           Identifier
>    |
> 603          +                           (8 octets)
>    +
> 604          |
>   |
> 605
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 606          //            Local Node Descriptors TLV (variable)
>  //
> 607
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 608          //            Remote Node Descriptors TLV (variable)
>   //
> 609
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 610          //               Link Descriptors TLVs (variable)
>  //
> 611
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>
> [minor] It looks like all the field names should be plural: "TLVs", right?
>

KT2> No. Local/Remote Node Descriptor TLVs are envelopes (with their code
point) within which other TLVs are carried. Link/Prefix Descriptor TLVs is
a reference to a collection of TLVs.


>
>
>
> ...
> 726     4.2.1.1.  Globally Unique Node/Link/Prefix Identifiers
> ...
> 739        We define an "IGP domain" to be the set of nodes (hence, by
> extension
> 740        links and prefixes) within which each node has a unique IGP
> 741        representation by using the combination of OSPF Area-ID,
> Router-ID,
> 742        Protocol-ID, Multi-Topology ID, and BGP-LS Instance-ID.  The
> problem
> 743        is that BGP may receive node/link/prefix information from
> multiple
> 744        independent "IGP domains", and we need to distinguish between
> them.
> 745        Moreover, we can't assume there is always one and only one IGP
> domain
> 746        per AS.  During IGP transitions, it may happen that two
> redundant
> 747        IGPs are in place.
>
> [] "each node has a unique IGP representation by using the combination
> of OSPF Area-ID"
>
> In this case, it should just be Area-ID (not including OSPF) because
> this is a general IGP statement, not specific to OSPF.  [§4.3.1.2 also
> defines the IS-IS Area Identifier TLV.]
>

KT2> The area-id being part of the node identifier is specific to OSPF and
hence OSPF Area-ID. ISIS is different - its area-id is just an attribute.


>
>
>
> ...
> 794     4.2.1.4.  Node Descriptor Sub-TLVs
> ...
> 822        BGP-LS Identifier:  Opaque value (32-bit ID).  This is an
> optional
> 823           TLV.  Its original purpose was that, in conjunction with
> 824           Autonomous System Number (ASN), it would uniquely identify
> the
> 825           BGP-LS domain and that the combination of ASN and BGP-LS ID
> would
> 826           be globally unique.  However, the BGP-LS Instance-ID carried
> in
> 827           the Identifier field in the fixed part of the NLRI also
> provides a
> 828           similar functionality.  Hence the inclusion of the BGP-LS
> 829           Identifier TLV is not necessary.  If advertised, all BGP-LS
> 830           speakers within an IGP flooding-set (set of IGP nodes within
> which
> 831           an LSP/LSA is flooded) MUST use the same (ASN, BGP-LS ID)
> tuple
> 832           and if an IGP domain consists of multiple flooding-sets,
> then all
> 833           BGP-LS speakers within the IGP domain SHOULD use the same
> ASN,
> 834           BGP-LS ID tuple.
>
> [nit] s/same ASN, BGP-LS ID tuple/same (ASN, BGP-LS ID) tuple
>

KT2> Ack


>
>
>
> ...
> 1237    4.3.1.2.  IS-IS Area Identifier TLV
>
> 1239       An IS-IS node can be part of only a single IS-IS area.
> However, a
> 1240       node can have multiple synonymous area addresses.  Each of
> these area
> 1241       addresses is carried in the IS-IS Area Identifier TLV.  If
> multiple
> 1242       area addresses are present, multiple TLVs are used to encode
> them.
> 1243       The IS-IS Area Identifier TLV may be present in the BGP-LS
> Attribute
> 1244       only when advertised in the Link-State Node NLRI.
>
> [] Are we talking about Areas or about Levels?  I had assumed Levels.
>

KT2> We are talking about Areas.


>
> Because §4.3.1 is about Node Attribute TLVs and a "node can be part of
> only a single IS-IS area", then how can multiple area addresses be
> present (in a single node)?
>

KT2> Multiple area addresses are synonyms for the same area. I will try to
find the ISIS ISO spec for the same.


>
>
> ...
> 2145    6.1.1.  BGP-LS NLRI Types Registry
> ...
> 2151                Type      NLRI Type                  Reference
> 2152            -----------------------------------------------------------
> 2153                 0        Reserved                   [This document]
> 2154                 1        Node NLRI                  [This document]
> 2155                 2        Link NLRI                  [This document]
> 2156                 3        IPv4 Topology Prefix NLRI  [This document]
> 2157                 4        IPv6 Topology Prefix NLRI  [This document]
> 2158             65000-65535  Private Use                [This document]
>
> [] Add table numbers/descriptions for all the tables in §6.
>

KT2> Ack


>
>
>
> ...
> 2408    7.2.2.  Fault Management
> ...
> 2418       A BGP-LS Speaker MUST perform the following syntactic
> validation of
> 2419       the Link-State NLRI to determine if it is malformed.
>
> 2421       *  Does the sum of all TLVs lengths found in the BGP
> MP_REACH_NLRI
> 2422          attribute correspond to the BGP MP_REACH_NLRI length?
>
> 2424       *  Does the sum of all TLVs lengths found in the BGP
> MP_UNREACH_NLRI
> 2425          attribute correspond to the BGP MP_UNREACH_NLRI length?
>
> 2427       *  Does the sum of all TLVs lengths found in a Link-State NLRI
> 2428          correspond to the Total NLRI Length field of all its
> Descriptors?
>
> 2430       *  Is the length of the TLVs and, when the TLV is recognized
> then,
> 2431          the length of its sub-TLVs in the NLRI valid?
>
> 2433       *  Has the syntactic correctness of the NLRI fields been
> verified as
> 2434          per [RFC7606]?
>
> 2436       *  Has the rule regarding the ordering of TLVs been followed as
> 2437          described in Section 4.1?
>
> 2439       *  For NLRIs carrying either a Local or Remote Node Descriptor
> TLV,
> 2440          is there more than one instance of a sub-TLV present?
>
> [] Reading this again, the introduction text says that a "BGP-LS
> Speaker MUST perform the following syntactic validation", and then we
> have a series of questions.  I guess that the validation passes if the
> answer to the questions is "Yes", right?
>
> That's what I originally thought, but then you added the last
> question.  The correct answer there is "No".
>
> I think it would be better if the checks were worded as statements, for
> example:
>
>    *  The sum of all TLVs lengths found in the BGP MP_REACH_NLRI
>       attribute corresponds to the BGP MP_REACH_NLRI length.
> ...
>    *  For NLRIs carrying either a Local or Remote Node Descriptor TLV,
>       at most one instance of each sub-TLV type is present.
>

KT2> Ack - reworded based on your suggestion.


>
>
>
> ...
> 2522    7.2.3.  Configuration Management
> ...
> 2540       An implementation SHOULD allow the operator to configure a
> 64-bit
> 2541       BGP-LS Instance-ID.  Refer to Section 4.2 for guidance to the
> 2542       operator for the configuration of BGP-LS Instance-ID.
>
> [major] The recommendation here ("SHOULD allow the operator to
> configure") contradicts the requirement in §4.2 ("The network operator
> MUST assign the same BGP-LS Instance-IDs on all BGP-LS Producers
> within a given IGP domain.").  If the implementation doesn't allow the
> configuration (permitted by the SHOULD), then how can the requirement
> in §4.2 be satisfied?
>
> s/SHOULD/MUST
>

KT2> Ack


>
> Also, §4.2 requires configuration only when multiple IGP instances are
> present: "An implementation that supports multiple IGP instances MUST
> support the configuration of unique BGP-LS Instance-IDs at the routing
> protocol instance level."  Given the text in this section, the text in
> §4.2 is not needed.
>

KT2> Tweaked to avoid repetition of normative language but otherwise kept
the text in section 4.2 since it makes things clear in that section context.


>
>
> [minor] s/64-bit/8-octet
>

KT2> Ack


>
>
>
> ...
> 2678    9.  Security Considerations
> ...
> 2685       In the context of the BGP peerings associated with this
> document, the
> 2686       operator should ensure that a BGP speaker does not accept
> updates
> 2687       from a peer that is only providing information to a BGP-LS
> Consumer
> 2688       using policy configuration options discussed in Section 7.2.3
> and
> 2689       Section 7.2.6.  Generally, an operator is aware of a
> participating
> 2690       BGP speaker's nature and that of its peerings for link-state.
> With
> 2691       this information the operator should protect that BGP speaker
> from
> 2692       peers sending updates that either represent erroneous
> information
> 2693       feedback loops or are false input.  Such protection can be
> achieved
> 2694       by manual configuration of peers that are attached only to
> BGP-LS
> 2695       Consumers at the BGP speaker.
>
> [] I tried to simplify a little (below).
>
> Suggestion>
>    The operator should ensure that a BGP-LS speaker does not accept
>    UPDATE messages from a peer that only provides information to a
>    BGP-LS Consumer by using the policy configuration options discussed
>    in Section 7.2.3 and Section 7.2.6.  Generally, an operator is aware
>    of the BGP-LS speaker's role and link-state peerings. Therefore, the
>    operator can protect the BGP-LS speaker from peers sending updates
>    that may represent erroneous information, feedback loops, or false
>    input.
>

KT2> Ack. Incorporated.


>
>
> 2697       An error or tampering of the link-state information that is
> 2698       originated into BGP-LS and propagated through the network for
> use by
> 2699       BGP-LS Consumers applications can result in the malfunction of
> those
> 2700       applications.  Some examples of such risks are the origination
> of
> 2701       incorrect information that is not present or consistent with
> the IGP
> 2702       LSDB at the BGP-LS Producer, incorrect ordering of TLVs in the
> NLRI
> 2703       or inconsistent origination from multiple BGP-LS Producers and
> 2704       updates to either the NLRI or BGP-LS Attribute during
> propagation
> 2705       (including discarding due to errors).
>
> [] Add a sentence explaining that this ("error or tampering
> of...information") is not a new risk, it is already present in BGP.
>

KT2> Ack.

Thanks,
Ketan



>
>
> [EoR -11]
>