> On 22 Dec 2016, at 15:32, Robert Wilton <rwil...@cisco.com> 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.
>> *** 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.
> 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.
>
>> *** 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.
Aha, you write about "l3-mtu" leaf in the text (also in Security
Considerations) but it's not present in the module.
>
>> *** 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?
> 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.
OK.
Thanks, Lada
>
>> *** 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.
>
> Thanks again,
> Rob
>
>>
>> Lada
>>
>
--
Ladislav Lhotka, CZ.NIC Labs
PGP Key ID: 0xB8F92B08A9F76C67
_______________________________________________
netmod mailing list
netmod@ietf.org
https://www.ietf.org/mailman/listinfo/netmod
