Hi Joel, 

> On Dec 13, 2024, at 20:10, Joel Halpern <jmh.dir...@joelhalpern.com> wrote:
> 
> Thank you for the clarifications.
> 
> With regard to section 6.1, my concern is not that the system will not 
> converge.  It will.  The oddity is that the wording in the second paragraph 
> makes it sound like the neighbor accepting self-originated NLRI is 
> sufficient.  If you modified the sentence:
> 
> "This is due to the fact that BGP SPF speakers adjacent to the router always 
> accept self-originated NLRI from the associated speaker as more recent (rule 
> # 1). "
> 
> to read )I think this is what you said):
> 
> "While the rules in the next section will ensure convergence, this is aided 
> and hastened by the fact that BGP SPF speakers adjacent to the router always 
> accept self-originated NLRI from the associated speaker as more recent (rule 
> # 1). "
> 
Upon a reflection, I think this rule may not have the impact that I was hoping 
for in the case of complete state loss. It is strange that no one has brought 
this up until now but I think I was envisioning the advanced NLRI sequence 
number being regenerated by the BGP SPF neighbors adjacent to the restarting 
router (which is not the case).

Thanks,
Acee



> Yours,
> 
> Joel
> 
> On 12/13/2024 4:54 PM, Acee Lindem wrote:
>> Hi Joel,
>> 
>> Thanks for the review. 
>> 
>>> On Dec 5, 2024, at 14:09, Joel Halpern via Datatracker <nore...@ietf.org> 
>>> <mailto:nore...@ietf.org> wrote:
>>> 
>>> Reviewer: Joel Halpern
>>> Review result: Ready with Issues
>>> 
>>> I am the assigned Gen-ART reviewer for this draft. The General Area
>>> Review Team (Gen-ART) reviews all IETF documents being processed
>>> by the IESG for the IETF Chair.  Please treat these comments just
>>> like any other last call comments.
>>> 
>>> For more information, please see the FAQ at
>>> 
>>> <https://wiki.ietf.org/en/group/gen/GenArtFAQ> 
>>> <https://wiki.ietf.org/en/group/gen/GenArtFAQ>.
>>> 
>>> Document: draft-ietf-lsvr-bgp-spf-39
>>> Reviewer: Joel Halpern
>>> Review Date: 2024-12-05
>>> IETF LC End Date: 2024-12-10
>>> IESG Telechat date: Not scheduled for a telechat
>>> 
>>> Summary:
>>> 
>>> Major issues:
>>> 
>>> Minor issues:
>>>   In section 5.2.1.1 on the SPF Status TLV there is an odd phrasing that I
>>>   think I understand, and if so should be clarified.  In discussing unknown
>>>   status value handling the text reads "However, a BGP SPF speaker MUST NOT
>>>   use the Status TLV in its SPF computation."  I presume that the NLRI and 
>>> all
>>>   its TLVs should not be used, as the Status is unkown, rather than just
>>>   ignoring the Status?  If the intention is that the NLRI is used, and the
>>>   status is assumed to make this node available for SPF, please reword to 
>>> make
>>>   clear that the Status TLV is ignore and the node information IS USED in 
>>> the
>>>   SPF.  (I tend to assume one errs on the side of avoiding nodes that may 
>>> not
>>>   be usable.  Reading section 6.3 I think your intention is the obverse. 
>>>   Which is your call.  In any case, we want everyone to understand this the
>>>   same way so clarification is a good idea.)  This applies to the other 
>>> Status
>>>   TLV descriptions as well.
>> I’ve updated these. 
>> 
>>       with a non-reserved value (2-254) that is unknown SHOULD be advertised 
>> to other
>>       BGP SPF speakers and MUST ignore the Status TLV with an unknown value 
>> in
>>       the SPF computation.
>> 
>> 
>> 
>>>    
>>>    In section 5.2.2.1 on BGP-LS-LINK NLRI Address Family Descriptor Link 
>>> TLV,
>>>    it is unclear if the intention is that unnumbered links must support only
>>>    one of IPv4 or IPv6, or if they are permitted to support both.  I would
>>>    guess that the intention is to permit both, and that the advertiser 
>>> simply
>>>    includes two such TLVs. It would help to be clear about the case.
>> I added text for this. I have a second comment on this as well.
>> 
>>     Note that an unnumbered link can be used for both the IPv4 and IPv6
>>     SPF computation by advertising separate Address Family Link Descriptor
>>     TLVs for IPv4 and IPv6.
>> 
>> 
>> 
>>>    In section 6.1 on BGP SPF NLRI Selection, I missed something in the
>>>    recovvery from complete state loss.  I understand how the preference for
>>>    self-originated information from direct neighbors helps direct neighbors,
>>>    and why that is specified.  The text seems to say that takes care of all
>>>    problems.  I don't see how it fixes remote stale information.  Suppose we
>>>    have routers A and B adjacent, and Router C further away.  Router A goes
>>>    down so hard it loses the upper part of its sequence number, and 
>>> therefore
>>>    comes up with lower sequence numbers than it was using before.  Router B
>>>    will as we want prefer these new advertisements anyway.  But now consider
>>>    router C.  It has in its store old advertisements from router A.  The new
>>>    advertisements will have lower sequence numbers, so, if I read this
>>>    correctly, router C will prefer the old (stale) information.  What drives
>>>    convergence so C eventually prefers new information from A? (As far as I
>>>    can tell, because B will prefer the new information, it may well not 
>>> send A
>>>    the old information, thus preventing A from updating its sequence 
>>> numbers.)
>> You’d have to rely on 6.1.1 for the convergence on non-directly connected 
>> routers.
>> 
>> 
>> 
>>> Nits/editorial comments:
>>>   I found the first paragraph of the introduction jarring.  It seemed to mix
>>>   propaganda for this work with a description of something (unclear what)
>>>   about current practices in MSDCs.  I think it would benefit from 
>>> rephrasing.
>>>    I suggest a single sentence.  "This document describes a technique to use
>>>   BGP, BGP-LS, and the SPF technology from IGPs to provide routing for
>>>   Massively Scaled Data Centers (MSDCs)."
>> This is what the WG set out to do. 
>> 
>> 
>>>    Is it my imagination or do section 5.1.1 and the first part of section
>>>    5.1.2 say exactly the same thing?
>> I’ve removed the second one since it is tangential. 
>> 
>> 
>>>    I would suggest slightly adjusting the initial text in section 5.2.4 on 
>>> the
>>>    BGP_LS Attribute Sequence NUmber TLV.  The text currently reads "A BGP-LS
>>>    Attribute TLV of the BGP-LS-SPF NLRI" which caused me to go check what 
>>> 1181
>>>    was defined to be.  Could it instead read "A BGP-LS Attribute Sequence
>>>    Number TLV of the BGP-LS-SPF NLRI" (e.g. add "Sequence Number".)
>> I changed three instances where BGP-LS Attribute TLV is used generically to 
>> reference the specific TLV. 
>> 
>> 
>>>   Several short paragraphs in the middle of section 7.1 on processing of
>>>   BGP-LS-SPF TLVs begin with the TLV name and then say " TLVs and their 
>>> error
>>>   handling rules are either defined in section 5.2.1 of [RFC9552]. ".  What 
>>> is
>>>   the "either"?  There appears to only be the reference to section 5.2.1 of
>>>   RFC 9552?
>> I’ve fixed the instances where there isn’t an alternative. 
>> 
>> Thanks,
>> Acee
>> 
>> 
>> 
>>> 

_______________________________________________
Gen-art mailing list -- gen-art@ietf.org
To unsubscribe send an email to gen-art-le...@ietf.org

Reply via email to