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"?
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?
*** 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.
*** 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?
Or perhaps it could be called "forwarding-mode"?
- 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 "physical-layer"
is actually the right term, I think that is probably really "below-layer-2"
- 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.
*** 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).
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?
Thanks,
Rob
Thanks again,
Rob
Lada
_______________________________________________
netmod mailing list
netmod@ietf.org
https://www.ietf.org/mailman/listinfo/netmod
