Re: [Lsr] AD review of draft-ietf-lsr-flex-algo-20

[Peter Psenak <[e-mail]>]

Hi John,

please see inline (##PP2)

On 09/09/2022 17:29, John Scudder wrote:
> Hi Peter,
> 
> Thanks for your reply and for the ping.
> 
> Where necessary I’ve replied in line below. I’ve snipped any points that 
> are agreed and don’t need further discussion. I’ve also reviewed the -21 
> diffs, basically LGTM. It looks like you missed a few of the nits, I 
> don’t know if this was by choice or oversight. I’ve attached an edited 
> version of -21 that captures the remaining ones, plus a few new ones I 
> noticed. You can diff if you want to pick those up for inclusion in -22.

##PP2
I fixed all nits, hopefully.

> 
>> On Aug 31, 2022, at 10:23 AM, Peter Psenak <ppsenak=[e-mail]> wrote:
>> 
>> [External Email. Be cautious of content]
>> 
>> 
>> Hi John,
>> 
>> thanks a lot for the thorough review.
>> 
>> I incorporated all your edits and almost all of your comments.
>> 
>> For the few that I have not, please see inline (loop for ##PP):
> 
> [snip]
> 
>>>     Any change in the Flex-Algorithm definition may result in temporary
>>>     disruption of traffic that is forwarded based on such Flex-Algorithm
>>>     paths.  The impact is similar to any other event that requires
>>>     network-wide convergence.
>>> +
>>> +jgs: IMO this would merit discussion in the Operational Considerations
>>> +section.  In particular, is there any advice regarding how to
>>> +stage/sequence FAD config changes in order to minimize disruption?
>> 
>> ##PP
>> I don't really see much to add here. FAD changes the constraints used
>> during the algo specific SPF and as such any change in it requires all
>> routers to recompute the entire topology. In terms of the order of
>> changes, I don't see why that would be significant and why someone would
>> not want to advertise all changes at once if there are any multiple
>> changes in the FAD.
> 
> I take your point, thanks. I guess about the most that you could say in 
> Operational Considerations would be something like
> 
> ---
> 15.X  FAD Changes
> 
> As [Section 5.3] notes, a change in the Flex-Algorithm definition may 
> require network-wide SPF recomputation and network reconvergence. This 
> potential for disruption should be taken into consideration when 
> planning and making changes to the FAD.

##PP2
Added above to the Operational Consideration section.

> ---
> 
> Obvs, re-word as appropriate. IMHO it's worth elevating in the O.C. 
> section even if only briefly, but I don’t insist.
> 
> [snip]
> 
>>> +jgs: Are "sender" and "receiver" sufficiently clear to OSPF practitioners
>>> +that there would be no ambiguity? I can think of two different ways
>>> +to read them -- one is that the "sender" is the router that
>>> +originates the LSA, and the "receiver" is any router that processes
>>> +the LSA. I think that's what you mean. The other, pedantic, reading,
>>> +is the "sender" is any router that puts the LSA on the wire, and the
>>> +"receiver" is any router that takes the LSA from the wire, so anyone
>>> +participating in flooding would be both a "sender" and a "receiver"
>>> +at times.
>>> +
>>> +If this is how people write OSPF specs and talk about OSPF, fine.
>>> +But if there are more precise terms than "sender" and "receiver" in
>>> +use, it would be nice to use them.
>> 
>> ##PP
>> send/receive is the standard term used, e.g
>> https://urldefense.com/v3/__https://datatracker.ietf.org/doc/html/rfc8665*section-5__;Iw!!NEt6yMaO-gk!HAdyT2s3c5I_fktGSY3YPuQdVeF95m5Yb-utZWsGn9214bNEm1SkQ1dDgtbTL2tEJz4kJBwXudGrWyHF3P9zB8IEVqDz$ 
> <https://urldefense.com/v3/__https://datatracker.ietf.org/doc/html/rfc8665*section-5__;Iw!!NEt6yMaO-gk!HAdyT2s3c5I_fktGSY3YPuQdVeF95m5Yb-utZWsGn9214bNEm1SkQ1dDgtbTL2tEJz4kJBwXudGrWyHF3P9zB8IEVqDz$>
>> 
>> I can replace sender with originator, if you prefer, but receiver would
>> remain.
> 
> As you prefer. It’s not a big deal. I agree, by the way, that receiver 
> is unambiguous.

##PP
replaced sender with originator.

> 
> [snip]
> 
>>> 
>>> @@ -1194,15 +1258,36 @@
>>>       |                                                               |
>>>       +-                            TLVs                             -+
>>>       |                             ...                               |
>>> +
>>> +jgs: Maybe add something like
>>> 
>>> +   Other than where specified below, these fields' definitions are as
>>> +   given in [RFC2328] Section A.4.1.
>> 
>> ##PP
>> RFC2328 does not use TLVs, so that would not be correct.
> 
> I looked again, and the fields that are excluded from my suggested 
> statement, since they are “where specified below” are Opaque Type, 
> Opaque ID, Length, and TLVs. That leaves LS Age, Options, LS Type, 
> Advertising Router, LS sequence number, and LS checksum. But if my 
> suggested wording is wrong, that’s fine, choose your own -- the more 
> general observation is that specs that provide a packet diagram usually 
> tell the reader what the fields mean, either by defining them, or by 
> saying what reference to look in.

##PP2
A I added reference to all fields in the Opaque LSA.

> 
> [snip]
> 
>> ##PP
>> Though I agree that the order is not important for now, one can imagine
>> that in the future there could be rules added for which the order would
>> be important. I feel numbering these rules and keep them in the strict
>> order would help in such case. And mandating the order from the
>> beginning does not make any harm. So I would prefer to keep it as it is.
> 
> I disagree, but it’s not a blocking disagreement, if you’ve considered 
> this and decided to keep it as written, so be it.

##PP
yes, I would prefer to keep it as it is.

> 
> But to give a little outline of why I still don’t agree, it goes like
> 
> - The rules as written are overspecified.
> - This means a savvy implementor will perceive they are free to ignore 
> the ordering requirement.
> - This means an implementation might indeed ignore it. It will still 
> operate per-spec.
> - If in fact a later spec introduces something where ordering is 
> relevant, in part because of the foregoing observations it will be 
> necessary for the spec to be clear about its ordering assumptions 
> anyway. So I don’t think you’ve really relieved future spec authors, or 
> implementors, of any work.
> 
> But TBH that wasn’t in my mind when I wrote the comment, it was just the 
> general “don’t overspecify” heuristic.
> 
> Anyhow, do as you prefer, I’ve said my piece.
> 
> [snip]
> 
>>>     Algorithm (with FAD selected includes the M-Flag) where the
>>>     advertising ASBR is in a remote area, the metric will be the sum of
>>>     the following:
>>> +
>>> +jgs: I don't understand what the words in parentheses are trying to
>>> say, can
>>> +you explain?
>> 
>> ##PP
>> it means that the "winning" FAD includes the M-bit.
> 
> Then how about this replacement text?
> 
> OLD:
>     In the case of OSPF, when calculating external routes in a Flex-
>     Algorithm (with FAD selected includes the M-Flag) where the
>     advertising ASBR is in a remote area, the metric will be the sum of
>     the following:
> 
> NEW:
>     In the case of OSPF, when calculating external routes in a Flex-
>     Algorithm, if the winning FAD includes the M-Flag, and where the
>     advertising ASBR is in a remote area, the metric will be the sum of
>     the following:

##PP2
updated as you proposed.

thanks,
Peter

> 
> Thanks,
> 
> —John
>