Subject: Yangdoctors last call review of draft-ietf-bfd-rfc9127-bis-01

From: Jan Lindblad via Datatracker <[email-masked]>
Date: Thu, 03 Feb 2022 03:22:48 -0800

Reviewer: Jan Lindblad
Review result: Ready with Issues

This is the last call YANG Doctor review of draft-ietf-bfd-rfc9127-bis.
Browsing the mail archives, this has been a long story. Realizing that the
context of the bis is to fix a particular issue, I have focused only on the
diffs from RFC 9127. I feel any additional nitpicks I might find in a complete
review would not be welcome at this stage.

I have reviewed the diffs, and find them fulfill the desired technical goals.
Since this update breaks backwards compatibility as defined in RFC 6020 sec 10
and RFC 7950 sec 11, the process for approving this change has been discussed
at length. One argument that has been put forward for going ahead is that the
previous version of this module was released only a short time ago, so there is
no proliferation of impacted systems in the field.

Another argument has been that the YANG Versioning Design Team is working on
updated backwards compatibility rules. The Ver-DT proposed updates to the
compatibility rules would indeed allow a change of this kind under certain
conditions. A key condition for allowing such a break with the backwards
compatibility is that the module revision history announces this break clearly
to all readers. This is not the case in the -01 version of the modules.

   revision 2022-01-04 {
       description
         "Updates to add client configuration parameters feature.";

In my YANG Doctor opinion, updating the revision statement to clearly state
that this version is not backwards compatible with the previous version is an
absolute requirement. I think it would also be fair to module readers to add a
few sentences explaining what's going on here.

---

From: Mahesh Jethanandani <[email-masked]>
Date: Thu, 3 Feb 2022 09:28:59 -0800

Hi Jan,

Thanks first of all for the review.

> On Feb 3, 2022, at 3:22 AM, Jan Lindblad via Datatracker <[email-masked]> wrote:
> 
> Reviewer: Jan Lindblad

How does this sound?

OLD:

NEW:
      "Updates to add client configuration parameters feature.
       This update breaks backward compatability with earlier
       version of the model. The new feature prevents up to 
       three client configuration parameters from being
       included, where they were not needed.";

Thanks.

Mahesh Jethanandani
[email-masked]

---

From: tom petch <[email-masked]>
Date: Fri, 4 Feb 2022 09:33:14 +0000

>         included, where they were not needed.";

I think that you need more than that (for the reader who does not follow 
the BFD WG).  I think that there needs to be a reference to where the 
compatability rules are - RFC7950 s.11 - and the nature of the breakage 
in the language of NETMOD so that readers can judge the impact thereof.

Tom Petch

---

From: "Jan Lindblad (jlindbla)" <[email-masked]>
Date: Mon, 7 Feb 2022 15:49:24 +0000

Mahesh, all,

I tend to agree with Tom that the revision description you proposed was a bit terse. What I had in mind was perhaps more along these lines:

    description
      "This revision is non-backwards-compatible with the previous revision.

       This revision adds an if-feature statement for the client configuration parameters.
       If a client using the previous YANG revision of this module connects to a
       server that implements the current YANG revision of this module, but does
       not implement the feature, the client may not function properly.

       This change was introduced despite this incompatibility because ...
       ...
      ";

Don't take the text above literally, I only wrote that to give an idea what I had in mind. There isn't much precedence for how IETF documents NBC breakage in YANG modules, so we have to decide upon the right verbiage level here. In the drafts produced by the versioning design team, there will be an annotation to include in cases like this,   rev:non-backwards-compatible;   which will make this rather clear. While waiting for that to be implemented, I'd say we should err on the side of making it a little overly clear, rather than hiding the fact.

Best Regards,
/jan

On Feb 3, 2022, at 3:22 AM, Jan Lindblad via Datatracker <[email-masked]<mailto:[email-masked]>> wrote:

Reviewer: Jan Lindblad

  revision 2022-01-04 {
      description

How does this sound?

OLD:

NEW:

I think that you need more than that (for the reader who does not follow the BFD WG).  I think that there needs to be a reference to where the compatability rules are - RFC7950 s.11 - and the nature of the breakage in the language of NETMOD so that readers can judge the impact thereof.

Tom Petch

Thanks.

Mahesh Jethanandani
[email-masked]<mailto:[email-masked]>

---

From: t petch <[email-masked]>
Date: Mon, 7 Feb 2022 17:34:23 +0000

> Don't take the text above literally, I only wrote that to give an idea what I had in mind. There isn't much precedence for how IETF documents NBC breakage in YANG modules, so we have to decide upon…

That is more what I would hope to see.  The key part for me is that fact 
that an if-feature has been added.  I would have added something like
'Consequently, a client that does not support this feature may be unable 
to retrieve the objects it would expect to be able to.'
although that may be stating the obvious!

Tom Petch

---

From: Jeffrey Haas <[email-masked]>
Date: Mon, 7 Feb 2022 12:42:52 -0500

If an implementation, for whatever reason, couldn't support the items that were covered under the new if-feature, they would have already deviated them away.

An implementation now has the option to not advertise the feature for that case.  It may also perversely choose to continue to apply a deviation.

No new nodes have been added.  Implementations (however unlikely they were to have been done) for RFC 9127 for these nodes would have already had to have dealt with them.

-- Jeff

> On Feb 7, 2022, at 12:34 PM, t petch <[email-masked]> wrote:
> 
> That is more what I would hope to see.  The key part for me is that fact that an if-feature has been added.  I would have added something like
> 'Consequently, a client that does not support this feature may be unable to retrieve the objects it would expect to be able to.'
> -- 
> last-call mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/last-call

---

From: t petch <[email-masked]>
Date: Wed, 9 Feb 2022 17:07:12 +0000

> If an implementation, for whatever reason, couldn't support the items that were covered under the new if-feature, they would have already deviated them away.

I do not understand.  The case that concerns me is an implementation 
written to RFC9127 that knows the rules for backwards compatibility and 
so knows what it can rely on when a revised module of the same name is 
found.  Except, AFAICT, it cannot because we are breaking the rules and 
giving the user little or no help in this I-D to work out why their 
software no longer works.

As Jan said, we have not done this before and are in uncharted 
territory.  I think we should be more helpful.  And the NETMOD WG or the 
YANG doctors, or both, need to produce guidelines ready for the next time.

Tom Petch

> .

---

From: "Jan Lindblad (jlindbla)" <[email-masked]>
Date: Wed, 9 Feb 2022 17:36:55 +0000

Tom, Jeff,

> I do not understand.  The case that concerns me is an implementation written to RFC9127 that knows the rules for backwards compatibility and so knows what it can rely on when a revised module of the same name is found.  Except, AFAICT, it cannot because we are breaking the rules and giving the user little or no help in this I-D to work out why their software no longer works.

I think what Jeff and the BFD team are saying is that there are no such implementations in the real world. As far as I can tell, that may well be true.

> As Jan said, we have not done this before and are in uncharted territory.  I think we should be more helpful.  And the NETMOD WG or the YANG doctors, or both, need to produce guidelines ready for the next time.

This is exactly my point. We're setting a precedence here. Let's do it right. There may be nobody that cares that it's not compatible, but I still prefer that we establish a good template for how to declare breakages of this kind, and then stick to it. At least until the YANG Rev DT publishes revised rules and official YANG statements.

Best Regards,
/jan

---

From: Mahesh Jethanandani <[email-masked]>
Date: Wed, 9 Feb 2022 09:53:09 -0800

I am ok with the draft announcing backward incompatibility and explaining the backward incompatibility with a combination of description statement and “What has changed since RFC 9127” section in the draft.

But I would defer to the netmod WG and/or YANG doctors to produce the guidelines. I do not think this draft is the right place to write the guidelines.

Thanks.

> On Feb 9, 2022, at 9:36 AM, Jan Lindblad (jlindbla) <[email-masked]> wrote:
> 
> Tom, Jeff,
> 

Mahesh Jethanandani
[email-masked]

---

From: "Jan Lindblad (jlindbla)" <[email-masked]>
Date: Wed, 9 Feb 2022 20:35:03 +0000

Mahesh,

Good.

I don't think anybody is suggesting to add any guideline text for nbc changes into 9127". I just want 9127" to set a good example for similar cases in the future, until we have the Ver DT document suite in place. You already said you're fine with that above, so we're all in agreement.

Best Regards,
/jan

On Feb 9, 2022, at 9:36 AM, Jan Lindblad (jlindbla) <[email-masked]<mailto:[email-masked]>> wrote:

Tom, Jeff,

Best Regards,
/jan

-- Jeff

On Feb 7, 2022, at 12:34 PM, t petch <[email-masked]<mailto:[email-masked]>> wrote:

   description

Tom Petch

Best Regards,
/jan

On Feb 3, 2022, at 3:22 AM, Jan Lindblad via Datatracker <[email-masked]<mailto:[email-masked]><mailto:[email-masked]>> wrote:

Reviewer: Jan Lindblad

 revision 2022-01-04 {
     description

How does this sound?

OLD:

NEW:

Tom Petch

Thanks.

Mahesh Jethanandani
[email-masked]<mailto:[email-masked]><mailto:[email-masked]>

--
last-call mailing list
[email-masked]<mailto:[email-masked]>
https://www.ietf.org/mailman/listinfo/last-call

.

Mahesh Jethanandani

---

From: Jeffrey Haas <[email-masked]>
Date: Wed, 9 Feb 2022 16:28:03 -0500

Tom,

> On Feb 9, 2022, at 12:07 PM, t petch <[email-masked]> wrote:
> 

Actual implementations need to ship modules that document their augmentations and deviations.  This is the current state of things.

If a mythical implementation of a module was unable to support a given leaf in this module, a deviation would have been created for it.

In the specific example we're discussing, the only thing changing is how optionality based on a given implementation's support or lack of support of those leafs can be done:
- If that implementation previously supported all of the newly conditional if-feature nodes in its uses of that module, it will need to announce the feature.
- If that implementation previously didn't support the newly conditional leafs, it would have already had a deviation module for them.  Now it does not need the deviations, but needs to NOT announce the feature.

It is right to call out that the announcement of the feature could have impact since it's a change of behavior.  But so is the simple fact that an implementation would have had to adjust what it ships for its deviations as well.

No ecosystem of yang modules for a given implementation is going to ship without the understanding that it's the entire bundle of things - including features announced in the protocol - that make up how the schema are used.

Original bundle: You deviated, if you needed to.
New bundle: You advertise the feature, or not.

A given implementation may change its mechanism from one bundle to the next, but in this specific example the outcome is the same.

Had this been to change actual schema nodes from one bundle to the next, the case would have been interesting.

> As Jan said, we have not done this before and are in uncharted territory.  I think we should be more helpful.  And the NETMOD WG or the YANG doctors, or both, need to produce guidelines ready for t…

The guidance suggested in prior working group meetings years ago largely still apply: No module lives in isolation.  The interesting dialog will continue to be about how you handle versioning of the sets  of interdependent modules along with mechanisms like features which are dynamic.

-- Jeff

---

From: tom petch <[email-masked]>
Date: Thu, 10 Feb 2022 10:58:54 +0000

> This is exactly my point. We're setting a precedence here. Let's do it right. There may be nobody that cares that it's not compatible, but I still prefer that we establish a good template for how t…

Two thoughts.  I can see very little text in the body of this I-D 
explaining what is going on and why.  That is obvious to those that 
track the BFD WG list but we are creating documents for posterity that 
need to make sense in five years time when memories of this have faded 
and there may be no BFD WG.

Second, you may be right that no implementations exist but what if, in a 
few years time, an implementor looks at the two versions, sees one that 
is horribly complicated with a Cartesian explosion of YANG feature and 
sees no reason not to implement the much simpler, if earlier, one. 
Obscure may be, but who knows who will be doing what in the lifetime of 
these two documents (which we should assume is several years IMO).

Tom Petch

---

From: t petch <[email-masked]>
Date: Thu, 10 Feb 2022 11:07:02 +0000

> But I would defer to the netmod WG and/or YANG doctors to produce the guidelines. I do not think this draft is the right place to write the guidelines.

I agree on both points

Tom Petch

---

From: Jeffrey Haas <[email-masked]>
Date: Thu, 10 Feb 2022 07:03:39 -0500

> On Feb 10, 2022, at 5:58 AM, tom petch <[email-masked]> wrote:
> 
> Second, you may be right that no implementations exist but what if, in a few years time, an implementor looks at the two versions, sees one that is horribly complicated with a Cartesian explosion of YANG feature and sees no reason not to implement the much simpler, if earlier, one. Obscure may be, but who knows who will be doing what in the lifetime of these two documents (which we should assume is several years IMO).

If you're a vendor that supports the nodes that are being made if-feature conditional in 9127-bis and you want to ship 9127 as your supported module?

Go for it.

This entire discussion is weird.  For all of your hand wringing, absolutely none of this discussion can force a future implementor to choose to support one version of a module over another.  

The trend will ideally be to support newer versions of models because they include features they want. But perhaps they want older base-level stuff.  If that implementation is somewhere between?  Deviation modules will be crafted for that implementation.

If you think this minor bit of work creates compatibility issues even though it doesn't move or rename a single leaf?  Then you'd be for a rather rude awakening at how other organizations do this stuff.  Those other organizations don't have to give a damn about supporting the entire set of possible features or arbitrary vendor quirks.  They get to pick and choose.  The trend there isn't to use features, it's to force the deviations.  We have it easy.

-- Jeff

---

From: Mahesh Jethanandani <[email-masked]>
Date: Thu, 10 Feb 2022 15:17:52 -0800

Hi Jan,

How about this?

OLD:
      This revision is non-backwards-compatible with the previous revision.

NEW:
       This revision is non-backwards compatible with the
       previous version of this model.

       This revision adds an 'if-feature' statement called 
       'client-base-cfg-parms' for client configuration parameters.
       Clients expecting to use those parameters now need to
       declare the feature to include them.

       The change was introduced for clients that do not need
       them, and have to deviate to prevent them from being
       included.

> On Feb 7, 2022, at 7:49 AM, Jan Lindblad (jlindbla) <[email-masked]> wrote:
> 
> Mahesh, all,
> 
>>>> On Feb 3, 2022, at 3:22 AM, Jan Lindblad via Datatracker <[email-masked] <mailto:[email-masked]>> wrote:
>>>> 
>>>> Reviewer: Jan Lindblad
>>> [email-masked] <mailto:[email-masked]>

Mahesh Jethanandani
[email-masked]

---

From: "Jan Lindblad (jlindbla)" <[email-masked]>
Date: Fri, 11 Feb 2022 14:08:46 +0000

Mahesh,

Thanks for the updated proposal.

How about this?

OLD:

NEW:

Imo, this is a reasonable level of verbosity.

Since it is servers (not clients) that declare support for features, I guess the
sentence should be something like this:

       Clients expecting to use those parameters now need to
       verify that the server declares support for the feature
       before depending on their presence.

Best Regards,
/jan

On Feb 7, 2022, at 7:49 AM, Jan Lindblad (jlindbla) <[email-masked]<mailto:[email-masked]>> wrote:

Mahesh, all,

    description

Best Regards,
/jan

Reviewer: Jan Lindblad

  revision 2022-01-04 {
      description

How does this sound?

OLD:

NEW:

Tom Petch

Thanks.

Mahesh Jethanandani

Mahesh Jethanandani

---

From: Jeffrey Haas <[email-masked]>
Date: Fri, 11 Feb 2022 09:18:04 -0500

Jan,

I'm clearly not speaking for Mahesh here.

> On Feb 11, 2022, at 9:08 AM, Jan Lindblad (jlindbla) <[email-masked]> wrote:
> OLD:

This seems reasonable to me.

>       before depending on their presence.

My general advice as a RFC author and a working group chair is to try to avoid restating things that summarize to "it's in the RFC".

Implementations of yang modules are expected to handle capabilities.

-- Jeff

---

From: "Jan Lindblad (jlindbla)" <[email-masked]>
Date: Fri, 11 Feb 2022 14:36:08 +0000

Jeff,

> Implementations of yang modules are expected to handle capabilities.

I was merely trying to correct a minor technical error in the proposed sentence.

/jan

---

From: tom petch <[email-masked]>
Date: Fri, 11 Feb 2022 16:08:29 +0000

>         before depending on their presence.

Looks good to me with that replacement sentence.  As you say, it is the 
server that announces the feature (which an old client will not then 
understand the implications thereof).  I think it strikes the right 
level of detail for the YANG module.

Tom Petch