Subject: [yang-doctors] Yangdoctors early review of draft-ietf-mboned-cbacc-02

From: Reshad Rahman via Datatracker <[email-masked]>
Date: Sun, 11 Apr 2021 14:15:35 -0700

Reviewer: Reshad Rahman
Review result: Almost Ready

YANG Doctor review of rev-02 by Reshad Rahman.

Comments/questions:

The document and YANG module-name have CBACC, yet the module prefix is "ambi",
is this on purpose? Intuitively, I was expecting the prefix to be "cbacc".

For presence statement, use CBACC-enabled instead of cbacc-enabled?

Is max-mss for a TCP Max Segment Size, or is this really max packet size? And
no need for jumbograms since this is for UDP?

Consider renaming max-bits-per-second to something along the lines of
max-speed. Description says kilobits (not bits).

Add "unit" statement e.g. to data-rate-window and max-bits-per-second

OOC, why so many priorities? I'm used to seeing 3 or 8 bits for priority.

Security considerations should mention the YANG data nodes.

Regards,
Reshad.

---

From: "Holland, Jake" <[email-masked]>
Date: Sat, 17 Apr 2021 21:38:04 +0000

Hi Reshad,

Thanks for your review!  Sorry for the slow response, it's been a hectic
time for me.

> is this on purpose? Intuitively, I was expecting the prefix to be "cbacc".

Whoops, thanks for catching this, I have it fixed now in my local copy.  This
was not on purpose.

> no need for jumbograms since this is for UDP?

This is not a TCP MSS option, but rather intended to capture a similar
concept for the multicast IP packets.  This is meant to be an
advertisement of the max IP payload size that the sender will send for
this data stream.  I guess I would call it a CBACC MSS option, instead
of a TCP MSS option, but do you think that will cause confusion?  Maybe
I should rename it to something like max-pdu or max-packet-size?

(The value is often useful to check against network mtu or packet buffer
size for a receiving app.)

> max-speed. Description says kilobits (not bits).

Thanks, I agree and will go ahead with this, and thanks for catching it.

> Add "unit" statement e.g. to data-rate-window and max-bits-per-second

Thanks, I hadn't noticed this YANG feature.  It seems to be named "units"
though, I think that's what you mean? (From section 7.3.3 of RFC 7950:
https://tools.ietf.org/html/rfc7950#section-7.3.3 )

I don't see guidance on standardized fields, but I assume that the suggestion
is satisfied by adding this to max-speed:
            units "kilobits/second";
and this to data-rate-window:
            units "milliseconds";

Is that correct?

> OOC, why so many priorities? I'm used to seeing 3 or 8 bits for priority.

I guess I hadn't given the size much thought, but more seemed better than
less here, since it's not taking packet space in the data stream, and
seems a pretty trivial difference in resources at the per-stream control
layer.  I was aiming to let the sender specify relative priority between
all the different streams they might be serving, which could in theory go
to a much higher count, but in practice I expected this to be plenty.

But based on the early experiments with deploying it, I'm finding I might
need to refactor the priority in this spec, so maybe this will become moot.

That analysis and experimenting is still in progress, and I don't yet have
a concrete replacement after thinking through the use cases more carefully.
But I'll consider whether there's value in reducing this size if it keeps
the same form, and it's a good question.

> Security considerations should mention the YANG data nodes.

Thanks, I've added a note to my local copy.  I'm tentatively interpreting
this comment as "follow the directions from Section 3.7 of RFC 8407", is
that a correct summary of this advice?
https://tools.ietf.org/html/rfc8407#section-3.7

Very helpful review, much appreciated!

Best regards,
Jake