Re: [Lsr] AD review of draft-ietf-lsr-ospf-bfd-strict-mode-05

[John Scudder <[e-mail]>]

Hi Ketan,

Thanks for the prompt update. I have one question about your edits. In Section 6, I had suggested

OLD:
   Implementations MAY
   provide a local configuration option to specifically enable BFD
   operation in OSPF BFD strict-mode only. 

NEW (my suggestion):
   Implementations MAY
   provide a local configuration option to require BFD strict-mode.

NEW (your revision):
   Implementations MAY
   provide a local configuration option to require BFD operation in OSPF
   BFD strict-mode only.  

I’m wrestling with whether this option is supposed to mean “the adjacency must not establish unless BFD comes up first” or “if the neighbor supports BFD, then BFD must come up first before the adjacency is allowed to establish”. I.e., as written I can make the argument that if my neighbor doesn’t advertise the B-bit, then it’s fine for the adjacency to come up as long as BFD isn’t run at all. The former interpretation seems more operationally useful, but the spec text is ambiguous.

I’m not sure my proposed text even resolved that problem, on reflection. In any case, I would be happier if the ambiguity were resolved somehow.

Other comments below.

> On Sep 1, 2022, at 11:36 AM, Ketan Talaulikar <[e-mail]> wrote:

[snip]

>  Status of This Memo
> @@ -92,10 +92,19 @@
>     protocols like OSPFv2 [RFC2328] and OSPFv3 [RFC5340] to detect
>     connectivity failures for established adjacencies faster than the
>     OSPF hello dead timer detection and trigger rerouting of traffic
> -   around the failure.  The use of BFD for monitoring routing protocols
> +   around the failure.  The use of BFD for monitoring routing protocol
>     adjacencies is described in [RFC5882].
>  
> -   When BFD monitoring is enabled for OSPF adjacencies, the BFD session
> +jgs: In the paragraph below, I found the flow of reading disrupted by
> +the dawning realization that it's describing the status quo prior to
> +introduction of this spec, i.e. the shortcomings that drove development
> +of the spec. I've proposed an additional first clause that might help
> +mitigate that, feel free to use it if you like.  (It's a little 
> +inelegant, you might want to do a more intrusive edit instead, up to
> +you.)
> 
> KT> Ack. I've modified the text a little differently than your suggestion. Please let me know if it works.

Yes, that’s fine, thanks.

[snip]

> +jgs: I expect that taken as a whole, this document is clear enough to
> +enable an implementor to do the right thing, and that's the main goal.
> +It does seem to me however that "updating the state machine" by means
> +of putting a few notes into one of the states isn't the most thorough
> +way of doing it -- probably if you were doing this from the get-go you'd
> +introduce a new substate called "waiting for BFD establishment" or 
> +something like that, and then transition to it from Init at the appropriate
> +time. Or something like that, I'm just making things up on the fly here. 
> +Likewise you'd introduce a new "BFD comes up" event that gets you out of
> +your new state and back to the main flow of the state machine. Instead 
> +that's encapsulated in the note you've added to Init.
> +
> +I don't want a return to the Bad Old Days where we spent months of our
> +lives wrangling over state machines in the Routing Area (if you missed
> +that time count yourself lucky). However, it would make me feel better to
> +know that the WG has at least considered the question and decided to move
> +ahead without doing a full update to the state machine. So, I'd appreciate
> +a bit of discussion on this point, thanks.
> 
> KT> I don't remember this discussion on the mailer. I do remember some discussion during an initial presentation of this draft to the WG though not sure if it was during the session or offline. The introduction of a new state would result in far more intrusive changes to implementations. We would have this state whether or not the strict-mode is in use (BFD itself is optional). It was simpler to update the INIT state as proposed by the draft instead. We've added text in the Operations section so implementations show this additional BFD wait information along with the adjacency state and transitions. That should help in troubleshooting and operations.

I’m satisfied that you’ve considered it, that the issue has been raised on the WG list (it has now) and that anyone who wants to comment has had the opportunity (they do, now).

> +Also: when you write "This document updates ... [RFC2328]" it seems like
> +there's a decent chance that someone's going to ask if the document should
> +formally Update RFC 2328.  If you're not going to use the updates header
> +(and I don't insist you do) you might at least consider the reasons such
> +an update isn't called for. It might be helpful too, to update the 
> +shepherd write-up with some text saying the WG considered the question.
> 
> KT> This is a good point and at least I was not sure if doing the formal update to RFC2328 was the right way since this is an optional feature. That said, we can do the formal updates if that is the right way to go about this. Looking for feedback if this change is required.

Your point about the feature being optional is well taken. The other perspective to consider is what might happen if someone else feels the need to also modify the Init state in their own spec. If they do, they really need to take your changes into account as well — it wouldn’t be great to have two documents, both purporting to update Init in some uncoordinated way. Adding the Updates: header makes this somewhat less likely to happen, because a person looking at 2328 will see the Updated by: header listing your RFC (to be).

So I think as a practical matter, it would be useful to add the Updates header. Regrettably, there is no formal definition of what the semantics of “Updates” are (!!) so all we have to go on is tradition and practicality.

>     Whenever the neighbor state transitions to Down state, the removal of
>     the BFD session associated with that neighbor SHOULD be requested by
> @@ -246,6 +281,14 @@
>     result in the deletion and creation of the BFD session respectively
>     when OSPF is the only client interested in the BFD session with the
>     neighbor address.
> +   
> +jgs: Please do a global search for the RFC 2119 keyword SHOULD and in
> +each instance, consider whether it can be a MUST, or the normal English
> +word "should" (in lower or mixed case).  If SHOULD is the appropriate
> +keyword, please supply some detail about when it would be appropriate
> +for an implementor to disregard the SHOULD.  (The one place where I
> +thought "should" might be the appropriate choice is in the Operations &
> +Management Considerations section.)
> 
> KT> In this specific case, we are discussing implementation specifics that are also applicable without the strict-mode, and hence introducing a MUST here was not appropriate. We could change it to "should" if that would be more appropriate. Looking for feedback if this change is required.

This actually motivates the question, why does that paragraph need to be there at all? Was the base behavior underspecified and in need of update? Is this just a nota bene for the implementor? Should you more clearly signal that’s what it is? But this is in any case tangential to the main point; we can return to it later perhaps. I did note the peculiar nature of the paragraph on my first read-through, and figured it did no harm to clarify that this is how implementations should be done regardless — but in any case I don’t see why that makes MUST inappropriate.

> In some other places, the behavior may be applicable and shared with "base" BFD and hence we felt not appropriate to make it a MUST. However, we have taken the opportunity to suggest a desirable behavior. We look for feedback if any specific instance needs change or clarification.

I have a different take on the appropriate use of SHOULD vs. MUST. Let’s look at what RFC 2119 says about SHOULD:

3. SHOULD   This word, or the adjective "RECOMMENDED", mean that there
   may exist valid reasons in particular circumstances to ignore a
   particular item, but the full implications must be understood and
   carefully weighed before choosing a different course.

I tend to think of SHOULD informally as a “MUST… UNLESS” pair.

Regarding your point that the extension is optional, it’s my position that in the case of a specification for an optional extension, MUST means “if you choose to practice this extension to the protocol, i.e. if the extension is both implemented and enabled by configuration, then you MUST”.

If you still feel uncertain about this, we can dig into it case-by-case and discuss each occurrence, but possibly the context above will help? (Note that I am not the only reviewer who’s likely to be triggered by unqualified use of SHOULD…)

>     An implementation MUST NOT wait for BFD session establishment in Init
>     state unless OSPF BFD strict-mode is enabled on the interface and the
> @@ -268,6 +311,12 @@
>     BFD sessions.  Disabling BFD would result in the BFD session being
>     brought down due to Admin reason [RFC5882] and hence would not bring
>     down the OSPF adjacency.
> +   
> +jgs: Regarding the last sentence above, suppose the router is configured 
> +as you permit in Section 6, such that strict mode is required in order
> +to establish an adjacency.  Would it then be appropriate to bring
> +down the adjacency if the B-bit is cleared?  If the B-bit is cleared and
> +the BFD session moves to admin down?
> 
> KT> No. The OSPF adjacency would not be brought down. The checking for B-bit and BFD session UP dependency check is only while in the INIT state. 

I’m going to defer this until we settle on what exactly the “local configuration option” text is trying to say (the discussion at the beginning of this note).

Thanks,

—John