Skip to main content

Early Review of draft-ietf-lpwan-schc-compound-ack-14
review-ietf-lpwan-schc-compound-ack-14-yangdoctors-early-aries-2023-03-20-00

Request Review of draft-ietf-lpwan-schc-compound-ack-12
Requested revision 12 (document currently at 17)
Type Early Review
Team YANG Doctors (yangdoctors)
Deadline 2023-03-17
Requested 2023-02-22
Requested by Alexander Pelov
Authors Juan-Carlos Zúñiga , Carles Gomez , Sergio Aguilar , Laurent Toutain , Sandra Cespedes , Diego S. Wistuba La Torre
I-D last updated 2023-03-20
Completed reviews Yangdoctors Early review of -14 by Ebben Aries (diff)
Secdir Last Call review of -13 by Brian Weis (diff)
Opsdir Last Call review of -12 by Tianran Zhou (diff)
Tsvart Last Call review of -14 by Wesley Eddy (diff)
Comments
This review is requested ahead of an IETF last call for the document.

The YANG model seems to be a straightforward one, but we want to follow the process and make sure everything is in line with the latest version of the SCHC Yang Data Model ( draft-ietf-lpwan-schc-yang-data-model-21 (RFC-to-be 9363) ).

Thank you!
Alexander
Assignment Reviewer Ebben Aries
State Completed
Request Early review on draft-ietf-lpwan-schc-compound-ack by YANG Doctors Assigned
Posted at https://mailarchive.ietf.org/arch/msg/yang-doctors/mwB3n4fX7Jkzit1YCjYHf8xda2c
Reviewed revision 14 (document currently at 17)
Result On the Right Track
Completed 2023-03-20
review-ietf-lpwan-schc-compound-ack-14-yangdoctors-early-aries-2023-03-20-00
1 module in this draft:
- ietf-lpwan-schc-compound-ack@2022-12-02.yang

YANG compiler errors or warnings (pyang 2.5.3, yanglint 2.1.55, yangson 1.4.16)
- No compiler errors or warnings for tree outputs
- Instance data however fails validation (see below)

Module ietf-lpwan-schc-compound-ack@2022-12-02.yang
- Overall, the module is small (2 leaf augments to ietf-schc) and concise.
  There are some style nits (alignment/spacing) that can be cleaned up by
  running the module through a linter and adding back to the draft code blocks
- As mentioned above, instance data will fail to validate due to the following
  when stmt

    when "derived-from(../schc:fragmentation-mode,"
    +" 'schc:fragmentation-mode-ack-on-error')";

  Should rather be:

    when "derived-from-or-self(../schc:fragmentation-mode," +
         "'schc:fragmentation-mode-ack-on-error')";
- Since you are applying the same when restriction to 2 leaf nodes here, I
  would recommend grouping the leaf nodes and gating the augment by way of
  uses + when

  e.g.

  augment "/schc:schc/schc:rule/schc:nature/schc:fragmentation" +
          "/schc:mode/schc:ack-on-error" {
    description
      "";

    uses ack-on-error {
      when "derived-from-or-self(./schc:fragmentation-mode, " +
           "'schc:fragmentation-mode-ack-on-error')";
    }
  }

General comments on the draft/modules:
- Section 5.2 - It appears that only 1 augment is conveyed here and the Figure
  10 line is out of place.  This section should conform to RFC8340 and be
  labeled "Tree Diagram" as seen in other published drafts/RFCs
- Module contact information - Feel free to include authors as stated in
  RFC8407 Section 4.8
- Module description - Put the description of the module at the top and the
  Copyright information beneth with correct line breaks and removal of the
  asterisk delimeter
- For all description statements in the module, use correct capitilization and
  be as descriptive as possible without being overly verbose.
- Section 7 - Include the full Security Considerations as is in any other
  drafts/RFCs related to YANG modules vs. point to a related RFC
- Section 8 - IANA Considerations.  You are introducing a new module that
  contains a new namespace so will require registering a new URI.  This will
  need to follow the same process and contain the same verbiage as other
  related drafts/RFCs

Example validated instance data after the when stmt fix:

<schc xmlns="urn:ietf:params:xml:ns:yang:ietf-schc"
  xmlns:schc-compound-ack="urn:ietf:params:xml:ns:yang:ietf-lpwan-schc-compound-ack">
  <rule>
    <rule-id-value>100</rule-id-value>
    <rule-id-length>1</rule-id-length>
    <rule-nature>nature-fragmentation</rule-nature>
    <fragmentation-mode>fragmentation-mode-ack-on-error</fragmentation-mode>
    <direction>di-up</direction>
    <fcn-size>2</fcn-size>
    <schc-compound-ack:bitmap-format>schc-compound-ack:bitmap-compound-ack</schc-compound-ack:bitmap-format>
    <schc-compound-ack:last-bitmap-compression>true</schc-compound-ack:last-bitmap-compression>
  </rule>
</schc>

{
    "ietf-schc:schc": {
        "rule": [
            {
                "rule-id-value": 100,
                "rule-id-length": 1,
                "rule-nature": "ietf-schc:nature-fragmentation",
                "fragmentation-mode":
                "ietf-schc:fragmentation-mode-ack-on-error", "direction":
                "ietf-schc:di-up", "fcn-size": 2,
                "ietf-lpwan-schc-compound-ack:bitmap-format":
                "ietf-lpwan-schc-compound-ack:bitmap-compound-ack",
                "ietf-lpwan-schc-compound-ack:last-bitmap-compression": true
            }
        ]
    }
}