Re: [yang-doctors] [MBONED] Yangdoctors early review of draft-ietf-mboned-dorms-01

[Reshad Rahman <[e-mail]>]

Hi Jake,

Trimmed response below.

On 2021-04-17, 7:22 PM, "Holland, Jake" <[e-mail]> wrote:

    Hi Reshad,

    Thanks again for your review(s), much appreciated!

    On 04-11, 1:58 PM, "Reshad Rahman via Datatracker" <[e-mail]> wrote:
    > Caveat: I won't pretend to fully understand the motivation behind DORMS. I've
    > reviewed this mostly from a YANG perspective.

    Thanks for the note, and I think that was the main intent of the
    review request and a very reasonable choice in making your review.
    But this makes me want to ask a clarifying question:

    Is this more of a "I didn't try to grasp the motivation", or a "I
    tried and failed to grasp the motivation"?
<RR> Neither __ . What I meant is that I just looked at this document and didn't go digging in mail archives or WG presentations, and I am also not a multicast expert (but familiar with SSM). So I had no idea whether there was more to this.
Section 1.3 is clear and yes the IETF106 slides do help but I think 1.3 is good. 

    I'm trying to understand whether I should read this comment as
    evidence of a substantial weakness in section 1.3, or as a note
    that you didn't try hard to follow that section and aren't commenting
    one way or the other?
    https://datatracker.ietf.org/doc/html/draft-ietf-mboned-dorms-01#section-1.3

    I'm assuming the latter, but if it's the former I'd be grateful for
    elaboration on the trouble you had (perhaps without the YANG doctor
    hat on, if that helps any).  So just in case it's the former and
    you need a better context before you know how to answer, here's the
    link to the presentation ahead of adoption that tried to go over the
    purpose:
    https://www.youtube.com/watch?v=ttGJyd5is2w&t=3525s
    https://datatracker.ietf.org/meeting/106/materials/slides-106-mboned-multicast-to-the-browser

    If the motivation was unclear in the doc but clear(er) in the
    presentation, I'd be grateful for any explanation of what's missing,
    if you saw something you can explain.

    > Comments/questions:
    > - OOC, why is DORMS limited to RESTCONF? I do understand why RESTCONF is
    > appealing, but potential deployments might be using NETCONF, CORECONF or gNMI?

    The intent is to provide a sort of minimal protocol suite that
    remote clients can rely on being present when they implement
    discovery.  However, there's no reason that other protocols
    couldn't be used in some deployments, I guess.  I could add a
    note to that effect.  Any objections to this text?

      This document does not prohibit the use of the "ietf-dorms"
      model with other protocols such as NETCONF, CORECONF, or gNMI,
      but the semantics of using the model with those protocols is out
      of scope for this document.  This document only defines the
      discovery and use of the "ietf-dorms" YANG model under RESTCONF.
<RR> While I am ok with the text above and focussing on RESTCONF in these documents, it is unfortunate that the R in DORMS is for RESTCONF. Because afaik nothing in these 2 documents prevent another protocol from being used? E.g. the DNS query could return info on a server (NETCONF or gNMI), not just a RESTCONF server?
However, since the WG adopted this document, the WG is fine with this being RESTCONF specific. So maybe my thoughts on this are biased and they have nothing to do with the YANG model itself.

    > - "mandatory" is not needed for list keys, it's actually ignored as mentioned in
    > RFC7950 section section 7.8.2

    Does this mean I should remove it?  This wasn't in the list of recommended
    omitted default values from section 4.4 of RFC 8407, so I wasn't sure.

    Even if it wasn't a key (e.g. if the key somehow became a numeric id),
    having this value would be mandatory, so I thought it was better to make
    it explicit, but I don't have any other objections and will accept your
    expert opinion if you confirm that you think it's better off removed.
<RR> You can keep it, as long as the tools don't complain.

    > - 7.1 Security considerations, looks like the read-only data is not deemed
    > sensitive, please add a statement to that effect.

    I thought section 7.2 tries to cover this, are you saying 7.1 needs to
    address it also?  Or maybe saying that something needs to explicitly
    discuss the fields as described by section 3.7 of RFC 8407?
<RR> Yes 7.2 covers this but I think explicitly listing the impact of the fields as per RF8407 would be good.

    > Regarding NACM, consider a SHOULD instead of MAY?

    I don't think a SHOULD is appropriate here for a broadcast use case, so
    I don't think this should have a blanket SHOULD (though for other use
    cases it might make more sense, which is why I wanted to make sure
    clients would understand it's possible, which is why I put in the MAY).
<RR> Add the text above to explain this. Anyway, I'll defer to security review on this.

    > - Even though the YANG model is not complex, adding an example always helps.

    I agree, but I think there's an example in section 2.3.4:
    https://datatracker.ietf.org/doc/html/draft-ietf-mboned-dorms-01#section-2.3.4
<RR> You are correct, this is enough in this document. Please make sure the examples are verified/validated (e.g. with yanglint).
Also, I'd usually suggest using documentation addresses but since this is multicast...

Regards,
Reshad.

    So I'm not sure I understand what other examples should be added.
    Are you saying I should have an example that includes an augmentation?
    (And if so, is the link to another spec that does it sufficient, such
    as the CBACC reference in section 4?) Or is there another kind of
    example that seems to be missing, or that you think would be
    especially helpful?

    But I'll make a note to look for some places that could use more
    examples, thanks for raising the point.


    Thanks for another very helpful review!

    Best regards,
    Jake