Re: AD review of draft-ietf-bfd-unsolicited-09

[Reshad Rahman <[e-mail]>]

 Hi John,
Thanks for the review and for your patience...
I'm ok with this form of comments. I don't think it necessarily saves us time, unless I'm missing something, since we edit the xml version.
Response below <RR>, co-authors please keep me honest.
Regards,Reshad.
    On Tuesday, August 23, 2022, 12:40:46 PM EDT, John Scudder <jgs=[e-mail]> wrote:  
 
  Dear Authors,

Thanks for your patience. Here’s my review of your document. There are some questions I’ve raised that will need some discussion before I can be sure I’ve properly understood the doc.

I’ve supplied my questions and comments in the form of an edited copy of the draft. Minor editorial suggestions I’ve made in place without further comment, more substantive questions and comments are done in-line and prefixed with “jgs:”. You can use your favorite diff tool to review them; I’ve attached a PDF of the iddiff output for your convenience if you’d like to use it. I’ve also pasted a traditional diff below in case you want to use it for in-line reply. I’d appreciate feedback regarding whether you found this a useful way to receive my comments as compared to a more traditional numbered list of comments with selective quotation from the draft.

Thanks,

—John



--- draft-ietf-bfd-unsolicited-09.txt   2022-08-17 20:43:32.000000000 -0400
+++ draft-ietf-bfd-unsolicited-09-jgs-comments.txt      2022-08-23 12:27:35.000000000 -0400
@@ -19,7 +19,7 @@
 
    For operational simplification of "sessionless" applications using
    BFD, in this document we present procedures for "unsolicited BFD"
-   that allow a BFD session to be initiated by only one side, and be
+   that allow a BFD session to be initiated by only one side, and<RR> Good.
    established without explicit per-session configuration or
    registration by the other side (subject to certain per-interface or
    per-router policies).
@@ -128,13 +128,17 @@
       the Router Server), and the nexthop of each router must be present
       at the same time.  These issues are also discussed in
       [I-D.ietf-idr-rs-bfd].
+      
+jgs: I don't get what you're driving at with "and the nexthop of
+each router must be present at the same time", isn't that already 
+covered by the previous clause?<RR> This clause is about IXP where we have 3 nodes. The text "and the nexthop ... at the same time" is a tad confusing, we will remove it.
 
    Clearly it is beneficial and desirable to reduce or eliminate
    unnecessary configurations and coordination in these "sessionless"
    applications using BFD.
 
    In this document we present procedures for "unsolicited BFD" that
-   allow a BFD session to be initiated by only one side, and be
+   allow a BFD session to be initiated by only one side, and<RR> Good.
    established without explicit per-session configuration or
    registration by the other side (subject to certain per-interface or
    per-router policies).
@@ -149,11 +153,11 @@
    Thus we believe that this proposal is inherently simpler in the
    protocol itself and deployment.  As an example, it does not require
    the exchange of BFD discriminators over an out-of-band channel before
-   the BFD session bring-up.
+   BFD session bring-up.<RR> Good.
 
-   When BGP Add-Path [RFC7911] is deployed at an IXP using the Route
-   Server, multiple BGP paths (when exist) can be made available to the
-   clients of the Router Server as described in [RFC7947].  The
+   When BGP Add-Path [RFC7911] is deployed at an IXP using a Route
+   Server, multiple BGP paths (when they exist) can be made available to the
+   clients of the Route Server as described in [RFC7947].  The
    "unsolicited BFD" can be used in BGP route selection by these clients<RR> Good.
    to eliminate paths with "inaccessible nexthops".
 
@@ -177,6 +181,43 @@
    its BFD Control packets.  The "My Discriminator", however, MUST be
    chosen to allow multiple unsolicited BFD sessions.
 
+---   
+jgs: The intent of this paragraph isn't completely clear to me. Of
+lesser importance, the parenthetical is a bit awkward. Would the 
+rewrite below accurately capture your intent?  Note that I changed
+your SHOULD to a MUST -- if it does need to be SHOULD, I think we 
+ought to have a conversation about what the exception cases are. 
+
+   Passive unsolicited BFD support MUST be disabled by default, and
+   MUST require explicit configuration to enable.  
+   <RR> The authors have discussed this point and we've decided that it is preferable not to discuss per-interface v/s global configuration in this section. Thanks for the suggestion and the text above.

+I think that may be sufficient!  I'm not convinced it's necessary for 
+the spec to talk about global vs per-interface configuration, isn't this
+a commonplace in most implementations, and done in an implementation-
+dependent way?  It seems to me that if you DO feel it necessary to go 
+into this level of detail, you've left out the idea of per-interface
+configuration overriding global configuration.  So if you DO cover
+configuration here (again, I discourage this), I think you should add
+something about that point.  Here is some text that might do it?
+
+   It MAY be enabled using a global configuration option (which applies 
+   to all interfaces), or by per-interface configuration, or a choice of
+   either.  If both global and per-interface configuration are in use,
+   the more-specific (that is, per-interface) configuration SHOULD 
+   take precedence over the less-specific (that is, global).
+   
+   Similarly, the BFD parameters to be applied can be either 
+   per-interface or global.  Alternately, the parameters to be used
+   MAY be chosen to be the parameters the active side uses in its BFD
+   Control packets.  The "My Discriminator" value, however, MUST be
+   chosen to allow multiple unsolicited BFD sessions.
+   
+One more question, does the spec need to be at all prescriptive about 
+how the parameters are chosen?  Or would it be perfectly fine for an
+implementation to (for example) always use the active side's params
+and not offer any configurability? <RR> We believe that configuration is handy e.g. to prevent use of too aggressive timers.  But if there is no such constraint, or if the constraint is hard-coded, what you suggested works too.
+---
+
    The active side starts sending the BFD Control packets as specified
    in [RFC5880].  The passive side does not send BFD Control packets.
 
@@ -190,7 +231,7 @@
    BFD session.  If the BFD session fails to get established within
    certain specified time, or if an established BFD session goes down,
    the passive side would stop sending BFD Control packets and MAY
-   delete the BFD session created until the BFD Control packets is
+   delete the BFD session created until the BFD Control packets are<RR> Good.

    initiated by the active side again.
 
    When an Unsolicited BFD session goes down, an implementation MAY
@@ -215,6 +256,58 @@
    for unsolicited behaviour) MUST be set to NULL if present on the
    interface.
 
+jgs: I think we need to discuss just what exactly UnsolicitedRole is
+for and how it's expected to be used.  It's "a new state variable"
+that is "the operational mode".  That is fairly vague.  Per a side
+conversation I had with the shepherd (Jeff), I'm given to understand
+that this variable is supposed to be a read-only (from the user's 
+perspective) value, and is present as essentially a debugging aid --
+to answer the question "where did this session come from?". I guess
+the values would then map to,
+
+PASSIVE: it came up because we were configured to accept unsolicited
+sessions, and one arrived.
+
+ACTIVE: it came up because we were configured to initiate sessions, and
+we did initiate one, and it came up.
+
+       Question: Do we set this state variable even if the other end isn't
+       actually passive? Presumably so, we have no way to know if the other
+       end even has unsolicited configured.  If both ends are active, can
+       the session really be called "unsolicited" in any meaningful way? 
+       (Surely not.)  And if not, then does it really make sense to set a
+       variable called "bfdUnsolicitedRole" for it?
+
+NULL: I actually have no idea what this is supposed to indicate, since
+if I'm right that this is meant to tell me "here's why the session came
+up" it has to be one, or the other.  So either NULL is not useful, or my
+understanding is wrong.  The fact that in the YANG, unsolicited-role
+captures "active" and "passive" but has no consideration for this NULL
+thing, deepens my suspicion.
+
+First, let's get me to understand what bfd.UnsolicitedRole is for -- if
+my description above is more-or-less accurate, or not.  After that, we
+can work on any changes that might be needed.<RR> The role is from section 6.1 of RFC5880. You are correct that it can be set all the time, there is no need for NULL and it is not specific to unsolicited. IIRC the reason I called it UnsolicitedRole is because it's being introduced in the bfd-unsolicited document. We will rename it to "Role" and refer to 5880. Corresponding YANG naming changes must also be made.
+
+
+
+jgs: As far as I can tell, all the new procedures in this document 
+relate to the passive party -- the active party is configured per
+RFC 9127 (and its underlying specs).  As such, I'm having a hard time
+understanding what use the ACTIVE value has -- you don't say what to
+do if UnsolicitedRole is set to ACTIVE, and indeed I think there's 
+nothing TO do, ACTIVE is neither sufficient, nor necessary, in order
+to make a BFD speaker be the active side in an unsolicited session.
+
+If that's right, I think both the name, and values, of this state
+variable are a bit off.  It seems to me it would be better off as a
+boolean named something like bfd.UnsolicitiedEnabled or maybe 
+bfd.PermitUnsolicited, with default state FALSE, which substitutes
+for what you're calling NULL, and TRUE substitutes for what you've 
+called PASSIVE.
+
+Indeed, that seems to be what the YANG module already represents --
+we have a boolean, "enabled", which corresponds to what I've described.<RR> The "enabled" configuration leaf node is to enable/allow unsolicited BFD (i.e passive mode). The leaf node "role" in the operational model tells us the current role of the BFD session, it is there for informational purposes only.
 
 
 
@@ -565,24 +658,48 @@
 7.1.  BFD Protocol Security Considerations
 
    The same security considerations and protection measures as those
-   described in [RFC5880] and [RFC5881] normatively apply to this
-   document.  With "unsolicited BFD" there is potential risk for
+   described in [RFC5880] and [RFC5881] apply to this
+   document.  In addition, with "unsolicited BFD" there is potential risk for<RR> Good.

    excessive resource usage by BFD from "unexpected" remote systems.  To
    mitigate such risks, the following measures are mandatory:
 
-   *  Limit the feature to specific interfaces, and to a single-hop BFD
+   *  Limit the feature to specific interfaces, and to single-hop BFD<RR> Good.

       with "TTL=255" [RFC5082].  For numbered interfaces, the source
       address of an incoming BFD packet should belong to the subnet of
       the interface on which the BFD packet is received.  For unnumbered
       interfaces the above check should be aligned with routing protocol
       addresses running on such pair of interfaces.
+      
+jgs: Is the above addressed to the coder writing the implementation, or to
+the operator configuring it?  I think probably it's to the coder and is
+meant to constrain how the implementation operates.  If so I think (a) it
+would be helpful to consider whether some of the "should" ought to be 
+RFC 2119 SHOULD or MUST, and (b) it might be a good idea to move this out
+of Security Considerations and into Procedures, perhaps with some 
+elaboration.<RR> Yes this is for the implementors, we will move it out.
+
    *  Apply "policy" to allow BFD packets only from certain subnets or
       hosts.
    *  Deploy the feature only in certain "trustworthy" environment,
       e.g., at an IXP, or between a provider and its customers.
+      
+jgs: I presume the two above are deployment guidance to the operator.<RR> Yes.
+
    *  Adjust BFD parameters as needed for the particular deployment and
       scale.
+      
+jgs: Doesn't that one go without saying?  If there's something more
+actionable you have in mind, you should supply more detail.  If there
+isn't, I'm not sure the above is helpful.<RR> Nothing more comes to mind, we'll remove it.
+      
    *  Use BFD authentication.
+   
+jgs: This could also use more detail.  E.g., RFC 5880 specifies no
+fewer than three different authentication modes.  Are you recommending
+one?  Also, some of your motivating use cases (for example, IXP) have
+the underlying assumption that there is little or no coordination 
+between operators of the two ends of the BFD session.  When this is 
+lacking (again, think IXP) how would BFD authentication be used?  <RR> Yes BFD auth is difficult for IXP. BFD auth can be used for the static route use-case: although this requires extra config, it's typically per interface (not per next-hop or per route config). We will add some text here. We will recommend SHA1 meticulous if BFD auth is used.
 
 7.2.  YANG Module Security Considerations