Subject: [yang-doctors] Yangdoctors Early Review of draft-ietf-lpwan-schc-yang-data-model-04

From: "Mehmet Ersue" <[email-masked]>
Date: Sun, 18 Apr 2021 16:59:01 +0200

I'm sending this on behalf of Carl Moberg.

It seems there was an issue and the review mail has not been generated.

Text copied from 

https://datatracker.ietf.org/doc/review-ietf-lpwan-schc-yang-data-model-04-y
angdoctors-early-moberg-2021-04-13/

Mehmet

Reviewer: Carl Moberg

Review result: On the Right Track

This is my YANG Doctors Early Review of
draft-ietf-lpwan-schc-yang-data-model-04

It is obviously challenging to provide early review feedback on how useful
and efficient the YANG model representation is for a set of technologies
that I am not an expert on, but here goes.

Understanding that it is still early in the lifecycle of the module, I have
not spent time on providing feedback on e.g. description texts,
capitalization and spelling but have focused on the general structure and
design of the module. I also haven't spent any time dissecting the output of
`pyang --ietf`, but I would suggest the authors start looking into that as
the content of the module moves ahead towards publication.

I would also suggest considering running the module through `pyang -f yang`
as that provides nice and consistent formatting and quoting.

As for the content itself. I believe to the best of my understanding that
the YANG module reflects the core data structures of RFC8724 well. I have a
couple of questions that may lead to broader discussion on _how_ to capture
certain aspects of these data structures in a way that makes it realistic to
implement while still easy to use.

As is right now, the YANG module assumes that all implementations support
all FID types defined to be derived from field-id-base-type. It includes
fields related IPv6, COAP/OSCORE, and ICMPv6 all in the same module. Is
there a possibility that some implementations won't implement all three of
those protocol groups? If so, it might be worth considering making FID type
groups either optional using YANG 'feature' statements or break them out
into separate modules to be advertised separately.

There is currently no correlation between field-id type and field-length
types in the same compression rule entry. I.e. the current YANG permits a
field-identifier 'fid-ipv6-version' combined with a field-length
'fl-token-length' in a rule entry, which I understand to be nonsensical. If
I am right in that it's an example of meaningless configuration, does the
authors think it important (and possible) to work towards a more stringent
validation of "meaningful" configuration by capturing the relationships
between fields like in this example?

---

From: Laurent Toutain <[email-masked]>
Date: Thu, 22 Apr 2021 10:45:01 +0200

Dear Carl and Mehmet,

Thank you very much for your review and all the advice, this will help us
to have a better document. I'm quite new in YANG and I was not aware of the
--ietf argument. It reports a lot of errors, I will correct them. -f yang
is also an option I was looking for to have more readable documents.

Regarding the FID, you are right, it is too flat right now, we will change
it to have something more hierarchical that allows also to go inside a
field to allow subfields ids.

For the last point, how do you establish a relation between fields in a
yang model?

Thank you again for your help

Laurent

On Sun, Apr 18, 2021 at 5:16 PM Mehmet Ersue <[email-masked]> wrote:

> I’m sending this on behalf of Carl Moberg.
>
> https://datatracker.ietf.org/doc/review-ietf-lpwan-schc-yang-data-model-04-yangdoctors-early-moberg-2021-04-13/
>
>
>
> Mehmet
>
>
>
> Reviewer: Carl Moberg
>
> Understanding that it is still early in the lifecycle of the module, I
> have not spent time on providing feedback on e.g. description texts,
> design of the module. I also haven't spent any time dissecting the output
> of `pyang --ietf`, but I would suggest the authors start looking into that
> as the content of the module moves ahead towards publication.
>
>
>
> I would also suggest considering running the module through `pyang -f
> yang` as that provides nice and consistent formatting and quoting.
>
>
>
> certain aspects of these data structures in a way that makes it realistic
> to implement while still easy to use.
>
>
>

---

From: Laurent Toutain <[email-masked]>
Date: Sun, 25 Apr 2021 19:48:46 +0200

Hi Carl,

I've issued an updated version of the YANG model:

https://github.com/lp-wan/datamodel/blob/master/ietf-schc%402021-04-23.yang

The changes are the following:

- pyang --ietf return no error
- reformatted with pyang -f yang

The field-id are now hierarchical, I created a new identity for each
protocol, for instance for IPv6 fields:

identity field-id-base-type {
description "Field ID base type for all fields";
}

identity field-id-ipv6-base-type {
base field-id-base-type;
description "Field IP base type for IPv6 headers described in RFC 8200";
}

identity fid-ipv6-version {
base field-id-ipv6-base-type;
description "IPv6 version field from RFC8200";
}

I also try to keep the hierarchy for sub-fields:

identity fid-ipv6-trafficclass {
base field-id-ipv6-base-type;
description "IPv6 Traffic Class field from RFC8200";
}

identity fid-ipv6-trafficclass-ds {
base fid-ipv6-trafficclass;
description "IPv6 Traffic Class field from RFC8200,
DiffServ field from RFC3168";
}

identity fid-ipv6-trafficclass-ecn {
base fid-ipv6-trafficclass;
description "IPv6 Traffic Class field from RFC8200,
ECN field from RFC3168";
}

For the correlation between fields, it is still a mystery, I tried this
syntax, but it does not pass pyang validation:
leaf field-length {
type schc:field-length-type;
mandatory true;
when "derived-from(../field-id, 'fid-ipv6-version')" {
default 4;
}
description "Field Length in bit or through a function defined as a YANG
referenceid";
}

I tried with another field, in the fragmentation part, when no-ack is used,
window size does not have to be specified:

leaf wsize {
when "not(derived-from(../fragmentation-mode, 'fragmentation-mode-no-ack'))"
;
type uint8;
description "size in bit of the window field";
}

there is no error, but I'm not sure it is correct.

Many thanks for your help,

Laurent

On Thu, Apr 22, 2021 at 11:27 AM Carl Moberg <[email-masked]> wrote:

> Laurent,
>
>  See inline below:
>
> > On 22 Apr 2021, at 10:45, Laurent Toutain <
> [email-masked]> wrote:
> >
> > Dear Carl and Mehmet,
> >
> > Thank you very much for your review and all the advice, this will help
> us to have a better document. I'm quite new in YANG and I was not aware of
> the --ietf argument. It reports a lot of errors, I will correct them. -f
> yang is also an option I was looking for to have more readable documents.
> >
> > Regarding the FID, you are right, it is too flat right now, we will
> change it to have something more hierarchical that allows also to go inside
> a field to allow subfields ids.
> >
>  We could consider the use of “when”-statements (see:
> https://tools.ietf.org/html/rfc7950#section-7.21.5) to constrain the type
> of the length-field strictly based on the derived identity of the
> field-identifier.
>
>  Take a look and see what you can make out of it and if it applies in this
> case.
>
> > It is obviously challenging to provide early review feedback on how
> useful and efficient the YANG model representation is for a set of
> technologies that I am not an expert on, but here goes.
> >
> >
> >
> > As for the content itself. I believe to the best of my understanding
> that the YANG module reflects the core data structures of RFC8724 well. I
> have a couple of questions that may lead to broader discussion on _how_ to
> capture certain aspects of these data structures in a way that makes it
> realistic to implement while still easy to use.
> >
> >
> >
> > As is right now, the YANG module assumes that all implementations
> support all FID types defined to be derived from field-id-base-type. It
> includes fields related IPv6, COAP/OSCORE, and ICMPv6 all in the same
> module. Is there a possibility that some implementations won't implement
> all three of those protocol groups? If so, it might be worth considering
> making FID type groups either optional using YANG 'feature' statements or
> break them out into separate modules to be advertised separately.
> >
> >
> >
> > _______________________________________________
> > yang-doctors mailing list
> > [email-masked]
> > https://www.ietf.org/mailman/listinfo/yang-doctors
>
>