BFD Fans,

I wanted to emphasize Jeff’s request for the WG to take a minute to consider 
the update. As far as I can tell, once Reshad fixes the final nit Robert 
raised, the document is done. Unless there’s any objection raised [*] by the 
end of Tuesday, May 2, I’ll plan to send the document to the RFC Editor. Thanks 
to Rob, Jeff, and the authors for working through these changes, I think they 
improved the product.

—John

[*] Including “I need more time please” as a potential objection.

> On Apr 27, 2023, at 11:41 AM, Jeffrey Haas <[email protected]> wrote:
> 
> 
> Presuming Rob acks the changes as having addressed his concerns, I believe 
> this resolves all lingering IESG comments.
> 
> The Working Group should spend a moment to review the changes since it's 
> technically a functional change in the YANG model. I believe it's the right 
> thing to do.
> 
> After letting the draft rest a few days to permit WG review, hopefully the 
> IESG will proceed with publication.
> 
> Thanks everyone for the work!
> 
> -- Jeff
> 
> 
>> On Apr 27, 2023, at 11:36 AM, Reshad Rahman 
>> <[email protected]> wrote:
>> 
>> Hi Rob,
>> 
>> Changes made in rev-15 to simplify the model:
>> - Removed the feature for global config, also removed the global enabled leaf
>> - At the per-interface level, the enabled leaf doesn't depend on the 
>> params-per-interface feature anymore
>> - Removed the must statements (since global parms config is not feature 
>> dependent anymore)
>> 
>> Other changes:
>> - Changed role from enum to identity (Jeff's suggestion)
>> - Updated acknowledgement section
>> 
>> Regards,
>> Reshad.
>> 
>> 
>> 
>> Name:        draft-ietf-bfd-unsolicited
>> Revision:    15
>> Title:        Unsolicited BFD for Sessionless Applications
>> Document date:    2023-04-27
>> Group:        bfd
>> Pages:        18
>> URL:            
>> https://www.ietf.org/archive/id/draft-ietf-bfd-unsolicited-15.txt
>> Status:         https://datatracker.ietf.org/doc/draft-ietf-bfd-unsolicited/
>> Htmlized:       
>> https://datatracker.ietf.org/doc/html/draft-ietf-bfd-unsolicited
>> Diff:           
>> https://author-tools.ietf.org/iddiff?url2=draft-ietf-bfd-unsolicited-15
>> 
>> 
>> On Wednesday, April 19, 2023, 09:31:32 AM EDT, Rob Wilton (rwilton) 
>> <[email protected]> wrote:
>> 
>> 
>> Hi Reshad,
>> 
>>  
>> Please see inline …
>> 
>>  
>> From: Reshad Rahman <[email protected]> 
>> Sent: 19 April 2023 03:38
>> To: The IESG <[email protected]>; Rob Wilton (rwilton) <[email protected]>
>> Cc: [email protected]; [email protected]; 
>> [email protected]; Jeffrey Haas <[email protected]>
>> Subject: Re: Robert Wilton's Discuss on draft-ietf-bfd-unsolicited-11: (with 
>> DISCUSS and COMMENT)
>> 
>>  
>> Hi Rob,
>> 
>>  
>> Thanks for the feedback. And no need to apologize since it took us much 
>> longer to respond to your initial comments...
>> 
>>  
>> Please see inline.
>> 
>>  
>> On Tuesday, April 18, 2023, 11:55:03 AM EDT, Rob Wilton (rwilton) 
>> <[email protected]> wrote:
>> 
>>  
>>  
>> Hi Reshad,
>> 
>>  
>> Apologies for the delay.
>> 
>>  
>> The changes that you have made are sufficient to clear my discuss.
>> 
>>  
>> As a non-discuss (and non-blocking) comment, I do still find this 
>> hierarchical configuration to be somewhat complex on two points:
>> 
>>  
>> 1.     The configuration is under optional feature statements both at the 
>> global level and the per-interface level, and I think that the model would 
>> allow neither feature to be specified, in which case the defaults would be 
>> ambiguous.  I’m sure that the WG has a good reason for why it is designed 
>> the way it is, but I can’t help wondering whether it would make the model 
>> cleaner/simpler to require support for the global level configuration, and 
>> only have per-interface level configuration under an optional feature.  
>> I.e., if this was done, then logically, there are always well-defined 
>> default values without requiring evaluation of the must-statement.
>> 
>> <RR> Initially when I did the 2 features I was looking for a way to enforce 
>> that at least 1 of the 2 features is supported but afaik there's no way in 
>> YANG 1.1 to do that. When I addressed your comments about config 
>> hierarchy/inheritance, I added the must statements to address that. I did 
>> consider removing one of the features but it wasn't clear to me which one 
>> should be removed, in that some implementations might prefer having just 
>> global config and others would prefer per-interface configuration. But I'm 
>> ok with making the global config mandatory (i.e. remove the feature) if 
>> there's consensus on that, need to discuss with the co-authors too.
>> 
>> [Rob Wilton (rwilton)]
>> 
>> Ack.  The only reason that I went with making supporting global 
>> configuration mandatory was that it felt like it should be simpler to 
>> implement, and that it makes the inheritance more robust/simpler.
>> 
>>  
>> 
>> I think that it maybe possible to achieve what you desire in YANG 1.1 (i.e., 
>> require at least 1 of the 2 features to always be enabled), but I don’t 
>> think that it is a good idea, and hence I wouldn’t recommend that you do it. 
>>  Unless I’m mistaken, what you desire may be achieved in YANG by defining a 
>> third feature (let’s call it HACK) that has its own if-feature sub-statement 
>> of “(GLOBAL or PER-INTERFACE)”, and then make everything at the top level of 
>> the module conditional on the “HACK” feature.  However, I think that this 
>> would probably just make the model ugly and confusing to users and probably 
>> isn’t worth the additional complexity/confusion that it would likely cause.
>> 
>>  
>> 
>> Regards,
>> Rob
>> 
>>  
>> 1.     I don’t think that the descriptions are necessarily clear about if, 
>> and how, single-interval on the interface is allowed to override 
>> desired-min-tx-interval and required-min-rx-interval configured globally, or 
>> vice-versa.  Please consider whether it would be helpful to update the 
>> descriptions of these fields under the interface configuration to clarify 
>> these semantics.
>> 
>>  <RR> Ack, I will update the description.
>> 
>>  
>> Regards,
>> 
>> Reshad.
>> 
>>  
>> Regards,
>> 
>> Rob
>> 
>>  
>>  
>> From: Reshad Rahman <[email protected]> 
>> Sent: 21 March 2023 01:32
>> To: The IESG <[email protected]>; Rob Wilton (rwilton) <[email protected]>
>> Cc: [email protected]; [email protected]; 
>> [email protected]; Jeffrey Haas <[email protected]>
>> Subject: Re: Robert Wilton's Discuss on draft-ietf-bfd-unsolicited-11: (with 
>> DISCUSS and COMMENT)
>> 
>>  
>> Hi Rob,
>> 
>>  
>> I believe rev-12 addresses the 3 DISCUSS points below.
>> 
>>  
>> We still have to do further updates to the document.
>> 
>>  
>> Regards,
>> 
>> Reshad.
>> 
>>  
>> On Monday, December 12, 2022, 12:03:19 PM EST, Robert Wilton via Datatracker 
>> <[email protected]> wrote:
>> 
>>  
>>  
>> Robert Wilton has entered the following ballot position for
>> 
>> draft-ietf-bfd-unsolicited-11: Discuss
>> 
>>  
>> When responding, please keep the subject line intact and reply to all
>> 
>> email addresses included in the To and CC lines. (Feel free to cut this
>> 
>> introductory paragraph, however.)
>> 
>>  
>>  
>> Please refer to 
>> https://www.ietf.org/about/groups/iesg/statements/handling-ballot-positions/
>> 
>> for more information about how to handle DISCUSS and COMMENT positions.
>> 
>>  
>>  
>> The document, along with other ballot positions, can be found here:
>> 
>> https://datatracker.ietf.org/doc/draft-ietf-bfd-unsolicited/
>> 
>>  
>>  
>>  
>> ----------------------------------------------------------------------
>> 
>> DISCUSS:
>> 
>> ----------------------------------------------------------------------
>> 
>>  
>> Hi,
>> 
>>  
>> Thanks for this document.
>> 
>>  
>> Please see my comments below for more details, but I'm balloting discuss on 3
>> 
>> points: (1) The document is somewhat unclear as to whether the configuration 
>> is
>> 
>> applied hierarchically (I presume that it is, if not then my second discuss
>> 
>> point is not valid and can be ignored). (2) As specified, I don't think that
>> 
>> the hierarchical configuration will work, because the interface level leaf
>> 
>> "defaults" will override an explicit value configured globally.  I.e.,
>> 
>> logically, the interface level leaf, if in scope, will always have a value. 
>> (3)
>> 
>> The document should provide an instance-data example in the appendix to
>> 
>> illustrate the use of this configuration.
>> 
>>  
>> Regards,
>> 
>> Rob
>> 
>>  
>>  
>> ----------------------------------------------------------------------
>> 
>> COMMENT:
>> 
>> ----------------------------------------------------------------------
>> 
>>  
>> Moderate level comments:
>> 
>>  
>> (1) p 3, sec 2.  Procedures for Unsolicited BFD
>> 
>>  
>>   When the passive side receives a BFD Control packet from the active
>> 
>>   side with 0 as "Your Discriminator" and does not find an existing BFD
>> 
>>   session, the passive side MAY create a matching BFD session toward
>> 
>>   the active side, if permitted by local configuration and policy.
>> 
>>  
>> I'm surprised that this is only a MAY and not a SHOULD or MUST.  I.e., if the
>> 
>> local configuration & policy allows passive BFD sessions why would they not 
>> be
>> 
>> created?
>> 
>>  
>> (2) p 4, sec 4.1.  Unsolicited BFD Hierarchy
>> 
>>  
>>   *  Globally, i.e. for all interfaces.  This requires support for the
>> 
>>       "unsolicited-params-global" feature.
>> 
>>   *  For specific interfaces.  This requires support for the
>> 
>>       "unsolicited-params-per-interface" feature.
>> 
>>  
>> >From this description, it is unclear to me whether the features enabling 
>> >global
>> 
>> or per-interface configuration are meant to be an either/or (in which case, 
>> the
>> 
>> constraint could probably be expressed in the features), or whether a server 
>> is
>> 
>> allowed to support configuration both globally and override the global
>> 
>> configuration with interface specific configuration.  My subsequent discuss
>> 
>> comments assume the latter.  Either way, it would be helpful for this text in
>> 
>> this section (and probably the YANG module) to be a bit more explicit on this
>> 
>> regard.
>> 
>>  
>> (3) p 8, sec 4.2.  Unsolicited BFD Module
>> 
>>  
>>       augment "/rt:routing/rt:control-plane-protocols/"
>> 
>>             + "rt:control-plane-protocol/bfd:bfd/bfd-ip-sh:ip-sh/"
>> 
>>             + "bfd-ip-sh:interfaces" {
>> 
>>         if-feature bfd-unsol:unsolicited-params-per-interface;
>> 
>>         description
>> 
>>           "Augmentation for BFD unsolicited on IP single-hop interface";
>> 
>>         container unsolicited {
>> 
>>           description
>> 
>>             "BFD IP single-hop interface unsolicited top level
>> 
>>             container";
>> 
>>           leaf enabled {
>> 
>>             type boolean;
>> 
>>             default false;
>> 
>>  
>> I'm not sure that you want a default value specified in the YANG here since
>> 
>> this would have precedence over any explicitly configured global default 
>> value.
>> 
>>  
>> (4) p 8, sec 4.2.  Unsolicited BFD Module
>> 
>>  
>>             description
>> 
>>               "BFD unsolicited enabled on this interface.";
>> 
>>           }
>> 
>>           uses bfd-types:base-cfg-parms;
>> 
>>  
>> You have the same issue here as above, in that the default values directly
>> 
>> associated with the leaves in this grouping will always take precedence over
>> 
>> any configured global value.  I.e., the configuration, if properly 
>> implemented
>> 
>> cannot be hierarchical.  The pragmatic solution is to copy the used grouping
>> 
>> inline here and delete the default statements.  This has the advantage that 
>> the
>> 
>> descriptions can also make the hierarchical behavior of the configuration
>> 
>> explicit.
>> 
>>  
>> (5) p 9, sec 4.2.  Unsolicited BFD Module
>> 
>>  
>>     augment "/rt:routing/rt:control-plane-protocols/"
>> 
>>           + "rt:control-plane-protocol/bfd:bfd/bfd-ip-sh:ip-sh/"
>> 
>>           + "bfd-ip-sh:sessions/bfd-ip-sh:session" {
>> 
>>       description
>> 
>>         "Augmentation for BFD unsolicited on IP single-hop session";
>> 
>>         leaf role {
>> 
>>           type bfd-unsol:role;
>> 
>>           config false;
>> 
>>           description
>> 
>>             "Role of local system during BFD session initialization.";
>> 
>>         }
>> 
>>     }
>> 
>>   }
>> 
>>   <CODE ENDS>
>> 
>>  
>> Please add an instance data example to an appendix to illustrate the use of
>> 
>> this YANG model.  This helps readers and can further emphasize the 
>> hierarchical
>> 
>> nature of the configuration.
>> 
>>  
>> Minor level comments:
>> 
>>  
>> (6) p 3, sec 2.  Procedures for Unsolicited BFD
>> 
>>  
>>   Passive unsolicited BFD support MUST be disabled by default, and MUST
>> 
>>   require explicit configuration to be enabled.  On the passive side,
>> 
>>   the desired BFD parameters SHOULD be configurable.  The passive side
>> 
>>   MAY also choose to use the parameters that the active side uses in
>> 
>>   its BFD Control packets.  The "My Discriminator", however, MUST be
>> 
>>   chosen to allow multiple unsolicited BFD sessions.
>> 
>>  
>> Rather then configured values on the passive side, did the authors consider
>> 
>> setting minimum configuration limits?  E.g., rather than define desired
>> 
>> send/receive limits, instead, configure lower bounds on what the minimum tx
>> 
>> interval may be (to prevent DDOS attacks).
>> 
>>  
>> Nit level comments:
>> 
>>  
>> (7) p 3, sec 2.  Procedures for Unsolicited BFD
>> 
>>  
>>   The passive side MUST then start sending BFD Control packets and
>> 
>>   perform the necessary procedure for bringing up, maintaining and
>> 
>>   tearing down the 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 SHOULD stop sending BFD Control
>> 
>>   packets and MAY delete the BFD session created until BFD Control
>> 
>>   packets are initiated by the active side again.
>> 
>>  
>> Nit, within certain specified => within a specified
>> 
>>  
>> (8) p 6, sec 4.2.  Unsolicited BFD Module
>> 
>>  
>>         Copyright (c) 2021 IETF Trust and the persons
>> 
>>         identified as authors of the code.  All rights reserved.
>> 
>>  
>> This copyright statement will need to be fixed.
>> 
>>  
>>  
>>  
> 

Reply via email to