RE: Robert Wilton's Discuss on draft-ietf-bfd-unsolicited-11: (with DISCUSS and COMMENT)

["Rob Wilton (rwilton)" <[e-mail]>]

Hi Reshad,

Please see inline …

From: Reshad Rahman <[e-mail]<mailto:[e-mail]>>
Sent: 19 April 2023 03:38
To: The IESG <[e-mail]<mailto:[e-mail]>>; Rob Wilton (rwilton) <[e-mail]<mailto:[e-mail]>>
Cc: [e-mail]<mailto:[e-mail]>; [e-mail]<mailto:[e-mail]>; [e-mail]<mailto:[e-mail]>; Jeffrey Haas <[e-mail]<mailto:[e-mail]>>
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) <[e-mail]<mailto:[e-mail]>> 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 <[e-mail]<mailto:[e-mail]>>
Sent: 21 March 2023 01:32
To: The IESG <[e-mail]<mailto:[e-mail]>>; Rob Wilton (rwilton) <[e-mail]<mailto:[e-mail]>>
Cc: [e-mail]<mailto:[e-mail]>; [e-mail]<mailto:[e-mail]>; [e-mail]<mailto:[e-mail]>; Jeffrey Haas <[e-mail]<mailto:[e-mail]>>
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 <[e-mail]<mailto:[e-mail]>> 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.