Subject: [netmod] LL review of draft-ietf-netmod-intf-ext-yang-03

From: Ladislav Lhotka <[email-masked]>
Date: Wed, 21 Dec 2016 14:08:09 +0100

Hi,

I think this is a very useful addition to ietf-interfaces. In general,
the document is clearly written and YANG modules nicely designed. Here
are my comments:

*** Sec. 1
    - "This document defines two YANG RFC 7950 [RFC7950] modules …"
      looks weird. I'd suggest "… YANG 1.1 [RFC7950] …" instead.
*** Sec. 2
    - first line: s/of of/of/
*** Sec. 3.1
    - If the "bandwidth" parameter is only used for tuning routing
      algorithms and has nothing to do with real bandwidth on the
      interface, I would suggest to use a different name
      (metric?). Perhaps it may indeed be better to leave this to
      routing protocol modules because they can also specify how
      exactly such a parameter is used.
*** Sec. 3.6
    - The "l3-mtu" parameter is probably the same as "mtu" defined in
      ietf-ip. Again, I would suggest to leave the definition of MTU
      to each L3 protocol because there may be specific aspects and
      constraints.
*** Sec. 3.8
    - I think that "transport-layer" is not a good name because in the
      ISO OSI model it denotes a particular layer (L4). How about
      "iso-osi-layer"?
    - The type of "transport-layer" has three enums, namely
      "layer-[123]". I would suggest to use descriptive names
      ("physical-layer" etc.), include the remaining layers of the ISO
      OSI model, and maybe also define it as a typedef - or does this
      belong to ietf-inet-types?
    - Is a leaf sufficient? I think some interfaces can be in multiple
      layers at the same time (e.g. an ATM interface is L3 but can
      also be L2 for Classical IP over ATM). Or is the idea that such
      an interface will be split into two entries of the "interface"
      list?
*** YANG modules
    - Descriptions are not consistently formatted. I think that the
      current BCP is to start description text on the same line as the
      "description" keyword only if it all fits on one line.
    - I don't have a strong opinion on this but it might increase
      readability if the number of augments is reduced. Perhaps at
      least augments that contain only one node could be made into one
      (and the "feature" and "when" statements moved to the nodes'
      definitions.

Lada

-- 
Ladislav Lhotka, CZ.NIC Labs
PGP Key ID: 0xB8F92B08A9F76C67

---

From: Martin Bjorklund <[email-masked]>
Date: Wed, 21 Dec 2016 16:13:04 +0100 (CET)

>       "description" keyword only if it all fits on one line.

Just a FYI. Try using:

pyang --keep-comments -f yang ietf-interfaces-common\@2016-10-27.yang

It will format descriptions etc consistently.

/martin

---

From: Robert Wilton <[email-masked]>
Date: Thu, 22 Dec 2016 14:32:18 +0000

Hi Lada,

Thanks for the review and comments.

>        looks weird. I'd suggest "… YANG 1.1 [RFC7950] …" instead.
OK.
>        exactly such a parameter is used.
Yes, possibility it would be better to define this as part of the 
routing models.  The idea here is that the bandwidth would span across 
the routing protocols rather than be tied to a specific protocol.

>        to each L3 protocol because there may be specific aspects and
The MTU being defined here is an "l2-mtu" parameter.  I.e. the maximum 
size of the L2 frames that can be sent/received.  For some devices this 
value is independent of a protocol specific L3 MTU that only affects 
that particular protocol instance.

>        an interface will be split into two entries of the "interface"
This needs some further thought/work.

The main idea here was to be able to label sub-interfaces as supporting 
only L2 services or L3 services, since on some platforms different types 
of interfaces get instantiated internally.

>        (and the "feature" and "when" statements moved to the nodes'
Yes, I'll fix these, probably using Martin's suggested approach.

Thanks again,
Rob

---

From: Ladislav Lhotka <[email-masked]>
Date: Thu, 22 Dec 2016 15:43:17 +0100

> On 22 Dec 2016, at 15:32, Robert Wilton <[email-masked]> wrote:
> 
> Hi Lada,
> 
> Yes, possibility it would be better to define this as part of the routing models.  The idea here is that the bandwidth would span across the routing protocols rather than be tied to a specific protocol.

We had a similar discussion regarding a route metric in ietf-routing: whilst most protocols use some metric, its semantics, range etc. differ. I wonder if it is the same here.

> The MTU being defined here is an "l2-mtu" parameter.  I.e. the maximum size of the L2 frames that can be sent/received.  For some devices this value is independent of a protocol specific L3 MTU that only affects that particular protocol instance.

Aha, you write about "l3-mtu" leaf in the text (also in Security Considerations) but it's not present in the module.

> The main idea here was to be able to label sub-interfaces as supporting only L2 services or L3 services, since on some platforms different types of interfaces get instantiated internally.

OK.

Thanks, Lada