Re: [Last-Call] Yangdoctors last call review of draft-ietf-bfd-rfc9127-bis-01

["Jan Lindblad (jlindbla)" <[e-mail]>]

Mahesh,

Thanks for the updated proposal.

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.

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) <[e-mail]<mailto:[e-mail]>> wrote:

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



Thanks first of all for the review.

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

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.

How does this sound?

OLD:
      "Updates to add client configuration parameters feature.";

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.";


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
[e-mail]<mailto:[e-mail]>



Mahesh Jethanandani
[e-mail]<mailto:[e-mail]>