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

["Holland, Jake" <[e-mail]>]

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"?

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.

> - The data in ietf-dorms is said to be read-only but doesn't have "config
> false"

I will add a note to experiment with this and see if I run into any
trouble.  It might make good sense to make the whole tree config false.

I was thinking of the intended meaning as "read-only access is provided
to most", rather than "all values are inherently read-only".  However, I
think there's probably no operational difference, so as long as I can
get it working I think I'd be fine moving it all to config false.

> - I'd add a "dorms" container at the top with "metadata" under "dorms".
> If other DORMS data needs to be added in the future, it would get added under
> "dorms". This would minimize top-level nodes as per RFC8407 section 4.10.

This seems reasonable and prudent, thanks.  I'll go ahead and do that.

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

> - If a group requires a minimum number of udp-stream entries (e.g. 1), add a
> "min-elements" statement under "group". If not, leave as-is.

It doesn't, no.  It's possible in theory to have non-UDP traffic, though
some of the extensions will be specific to UDP traffic.

> - 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?

> 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).

> - 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

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