Subject: [lp-wan] review of draft-ietf-lpwan-schc-yang-data-model-03

From: Ivaylo Petrov <[email-masked]>
Date: Thu, 30 Jul 2020 09:40:14 +0200

Dear all,

This is my review of draft-ietf-lpwan-schc-yang-data-model-03. While there
is more work to be done on that document (missing introduction, etc), I
believe it is going in the right direction.

== Comments

I like the use of identity refs as it leaves us the possibility to extend
the model in the future fairly easily. Here are my comments.

* How important is the dependency on I-D.barthel-lpwan-oam-schc? I am not
sure what the timeline for that document is, but my impression is that this
is an unnecessary normative reference that could delay the current one.
Also, as the intended status of I-D.barthel-lpwan-oam-schc appears to be an
informational document and the data-model is on the Standards Track, it
will be a downref, which will need to be justified.
* It appears that at places you have tried to save bytes by not
including position when it has value 0 in a list (for example definition in
section 2.6 and usage in 3.1.1). If I read sec 7.8.2. from RFC 7950
correctly, that is not allowed. You might need something slightly more
complex if you want to achieve this I am afraid.
* Is there a particular reason not to use YANG 1.1? Or at least use a
reference statement for pointing to RFCs (check out sec 7.21.4. of RFC 7950
for more information).
* Do we expect timers to be very small? I am wondering if seconds is the
best unit, do we need more flexibility, etc... Maybe a multiple of 10s will
be precise enough for our needs and will be smaller in a number of cases.
Just a thought.
* I was slightly confused about compression-rule-entry, compression-content
and the fact that the rule id is only found in `schc/rule`. One way for me
to improve the readability could be to have the rule-id-type (which is
surprisingly a grouping - maybe it should be renamed) in a grouping with
`compression-content` (which could be renamed to `compression-rule-fields`
probably) called `compression-rule-entry` and similarly for the
fragmentation. It will have a bit of repetition, but will be more readable
IMO. And `compression-rule-entry` could be renamed to
`compression-rule-field-macher`, for example.
* I believe it is a good practice to register YANG modules in "YANG Module
Names" registry and the YANG module namespace in the "IETF XML Registry"
* Module revisions make sense in my opinion only for modules that have been
published and used. I would expect the RFC editor to remove
the different revisions that you are providing in any case, but maybe you
can leave a note about that.
* YANG modules usually have a description that begins with a copyright
statement. Please let me know if you need help for that I will find the
correct pointers.YANG Module Names
* Security considerations need to specify in my opinion at least that the
transport protocol should provide sufficient security as otherwise there
could be misconfigurations or serious security issues (it is obvious, I
know, but I still think it could be included).

== Nits and editorial changes

I did not focus too much on those, but here are a few suggestions.

* Sec 2
  - s/at the boundary this/at the boundary of this/
  - s/Draft [RFC8724]/[RFC8724/
  - s/an update the other end to set/an update mechanism for the other end
to set/
* Sec 2.2
  - first and second paragraph could be better linked
* Sec 2.4
  - I expect 0 not to be a positive integer :) Maybe you can replace
positive with non-negative
* Sec 3
  - s/context can be identify/context can be identified/
* Sec 3.1.1
  - s/The compression rule described Figure 1/The compression rule
described in Figure 1/

Cheers,
Ivaylo