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

[Robert Wilton <[e-mail]>]

Lada,

Thanks for the comments, some further comments inline ...


On 10/03/2017 14:09, Ladislav Lhotka wrote:
> Hi Rob,
>
> please see inline.
>
> Robert Wilton <[e-mail]> writes:
>
>> Hi Lada,
>>
>> To pick up an old thread, I'm updating the interface model per your
>> comments below, and I had a few questions that you might have an opinion on.
>>
>> On 22/12/2016 14:32, Robert Wilton wrote:
>>> Hi Lada,
>>>
>>> Thanks for the review and comments.
>>>
>>>
>>> On 21/12/2016 13:08, Ladislav Lhotka wrote:
>>>> 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.
>>> OK.
>> Fixed.
>>>> *** Sec. 2
>>>>       - first line: s/of of/of/
>> Fixed.
>>
>>>> *** 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.
>>> 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.
>> I think that we need a better name than just "bandwidth" since this leaf
>> doesn't affect the actual bandwidth on the of the link, just the
>> bandwidth that is reported to routing protocols to implicitly adjust
>> their link metrics.  Hence, I was considering renaming this to
>> "reported-bandwidth"?
> This looks like something for routing people to decide.
>
>> I also think that it would be good to follow up with the routing folks
>> to see if this leaf would be better covered in a general routing model.
>> Where do you think is the best place that I raise this, the routing area
>> yang design team?
> Maybe, or RTGWG?
OK.  I'll send an email that way, after I've published this next revision.

>
>>
>>>> *** 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.
>>> 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.
>> I've updated the text to make it clear that it is only a L2 MTU
>> configuration leaf, and if not configured, the system will automatically
>> decide on the L2 MTU.
> Shouldn't it have a "when" substatement to make it available only for
> "layer-2" interfaces?
It can apply to all interfaces that use a layer 2 encapsulation, i.e. 
most interfaces.

The only interfaces that it wouldn't apply to are:
  - interfaces that are performing optical switching - but then I think 
that they are probably not really interfaces at all.
  - software interfaces that represent a tunnel endpoint that carriers 
L3 frames with no L2 encapsulation.  Although in this case you could 
just regard them as having a 0 byte layer 2 overhead.



>
>>>
>>>> *** 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"?
>> I agree that "transport-layer" is somewhat ambiguous.
>>
>> I was wondering whether "service-layer" or "service-type" would work?
> This looks more like a business term, "iso-osi-layer" or just
> "osi-layer" seems better to me, even if you include only layers
> 1-3. Actually, the OSI model calls these three "media layers", but this
> term isn't commonly used.
The problem with osi-layer is that I don't think that the name really 
indicates what it means.

>
>> Or perhaps it could be called "forwarding-mode"?
> IMO, the layers are not only about forwarding.
Agreed, but this configuration is to provide a constraint as to what 
services and forwarding paradigm can be enabled.

E.g. an "l2 transport/service" would be connected to an L2VPN instance 
but not IP.
An ACL applied to a "L2 transport/service" would filter on MAC 
addresses/VLAN Ids rather than IP header fields.

>
>>>>       - 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?
>> I'm not sure that defining the higher layers of the OSI model helps.
>> Perhaps using identities would be a better choice?
>>
>> Perhaps:
>>    "physical-layer"
>>    "data-link-layer"
>>    "network-layer"
>>
>> But I prefer "layer-2" over "data-link-layer" since I think that term
>> is much more widespread.  I'm also not quite sure whether
> OK.
>
>> "physical-layer" is actually the right term, I think that is probably
>> really "below-layer-2"
>>
> In terms of the OSI model, there is only layer 1 (physical) below layer
> 2. Or do you mean things like Ethernet-over-MPLS? I don't know how the
> layer classification works in such convoluted cases.
For layer-2, I think of carrying L2 frames over another service (e.g. 
IP, MPLS, or MAC-in-MAC).
For layer-1, I was thinking of things like optical switching or OTN 
switching.  But again it does come back to whether those are represented 
as interfaces.  Although, even in the optical switching cases, I think 
that devices sometimes snoop the layer 2 frame statistics.

>
>>>>       - 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?
>>> This needs some further thought/work.
>> I think that using sub-interfaces is the cleanest way of doing this, so
>> each sub-interface can be offering a service at a different layer.
>>
>>
>>> 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, I don't dare to give any suggestions here.
>
>>>> *** 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.
>>> Yes, I'll fix these, probably using Martin's suggested approach.
>> I've fixed the descriptions, and merged all the augments except for the
>> sub-interface one (which is an if-feature conditional augment of a
>> mandatory node).
> Good.
>
>> I also have a question about the "encapsulation" container that is
>> currently defined as:
>>
>>       /*
>>        * Various types of interfaces support a configurable layer 2
>>        * encapsulation, any that are supported by YANG should be
>>        * listed here.
>>        *
>>        * Different encapsulations can hook into the common encaps-type
>>        * choice statement.
>>        */
>>       container encapsulation {
>>         when
>>           "derived-from-or-self(if:type, 'ianaift:ethernetCsmacd') or
>>            derived-from-or-self(if:type, 'ianaift:ieee8023adLag') or
>>            derived-from-or-self(if:type, 'ianaift:pos') or
>>            derived-from-or-self(if:type, 'ianaift:atmSubInterface') or
>>            derived-from-or-self(if:type, 'ethSubInterface')" {
>>
>>           description
>>             "All interface types that can have a configurable L2
>>              encapsulation";
>>           /*
>>            * TODO - Should we introduce an abstract type to make this
>>            *        extensible to new interface types, or vendor
>>            *        specific interface types?
>>            */
>>         }
>>
>>         description
>>           "Holds the OSI layer 2 encapsulation associated with an
>>            interface";
>>         choice encaps-type {
>>           description "Extensible choice of L2 encapsulations";
>>         }
>>       }
>>
>> I was considering removing the when statement to generalize this, since
>> the case statements can be constricted to suitable interface types using
>> when statements anyway.  E.g. I'm proposing to changing it to something
>> like this:
>>
>>       /*
>>        * Various types of interfaces support a configurable
>>        * encapsulation.
>>        *
>>        * Different encapsulations (often layer 2) can hook into the
>>        * common encaps-type choice statement.
>>        */
>>       container encapsulation {
>>         description
>>           "Holds the encapsulation (often layer 2) associated with an
>>            interface";
>>         choice encaps-type {
>>           description
>>             "Extensible choice of encapsulations";
>>         }
>>       }
>>
>> Do you have a view on this?
> This demonstrates that the IANA interface types are not very useful for
> restricting the data model. As you indicate, multiple levels of the
> interface hierarchy (abstract interface types) might be better, perhaps
> you could also make use of multiple inheritance of identities, hence
> allowing for arbitrary mixes of interface properties encoded in the type.
Yes, I think that the IANA interface types are a problem.

Previously I was planning on writing up a draft with abstract types 
deriving from the IANA types, but I'm not sure whether this helps in the 
general case.

It might be that we need to define a set of abstract types (e.g. 
physical interfaces, ethernet-like, sub-interface, logical interface, 
etc) and then update the type identities in iana-if-type.yang to derive 
from those base types (which I believe would be an allowed backwards 
compatible change).

Thanks,
Rob


>
> Cheers, Lada
>
>> Thanks,
>> Rob
>>
>>
>>> Thanks again,
>>> Rob
>>>
>>>> Lada
>>>>