Hi Rob,
You haven't replied to my WGLC email below, and it seems the issues I
brought up are not fixed in -06. Did you miss this email?
/martin
--- Begin Message ---
Hi,
Here is my (late) review of draft-ietf-netmod-sub-intf-vlan-model-05.
o 1
The YANG module names are not correct; they are listed as:
if-l3-vlan.yang - Defines the model for basic classification of
VLAN tagged traffic to L3 transport services
flexible-encapsulation.yang - Defines the model for flexible
classification of Ethernet/VLAN traffic to L2 transport services
Should be "ietf-if-l3-vlan" and "ietf-flexible-encapsulation".
Or "ietf-if-l3-vlan" and "ietf-if-flexible-encapsulation".
But I also wonder if these names should somehow be changed. What
is a "l3-vlan"? And "flexible-encapsulation" sound a bit too
generic.
o 1.1
The text says:
Sub-interface: A sub-interface is a small augmentation of a regular
interface in the standard YANG module for Interface Management that
represents a subset of the traffic handled by its parent interface.
I think the augmentation is the YANG-realization of a sub-interface,
but it is not what a sub-interface is. Also, this definition is
mis-leading; it doesn't mention that a sub-interface has its own
interface type and is represented as one separate entry in the
interface list. I think it is better to import this term from
draft-ietf-netmod-intf-ext-yang (section 3.6)
o 3
The text says:
The L3 Interface VLAN model provides appropriate leaves for
termination of an 802.1Q VLAN tagged segment to a sub-interface based
L3 service.
There is a comment in the YANG model that says the same thing.
But the YANG model itself augments not only to sub-interface-based
interface, but also to ethernet-like interfaces.
o YANG modules
Both modules lack the IETF Trust Copyright statement.
We don't list WG Chairs anymore.
The revision statements should be on the form: "RFC XXXX: <title>"
Many descriptions are full sentences w/o the ending ".".
The modules should be indented properly; a starting point can be
pyang -f yang --yang-line-length 69
o ietf-if-l3-vlan
There is a comment:
/*
* Matches a single VLAN Id, or a pair of VLAN Ids to classify
* traffic into an L3 service.
*/
This should be moved into a description clause.
o ietf-if-l3-vlan / container dot1q-vlan
The must statement has:
count(../../if-cmn:forwarding-mode) = 0
This can be changed to not(../../if-cmn:forwarding-mode) which is
more direct imo.
The must statement's description statement seems to be a
copy-and-paste error.
o ietf-if-l3-vlan / container dot1q-vlan
The descriptions talk about "matching frames" and "classifying
traffic", but it is not described anywhere how the matching and
classifying is used.
(also applies to ietf-flexible-encapsulation)
o ietf-if-l3-vlan / outer-tag / second-tag
These names are a bit inconsistent. The description describes them
as "outermost tag" and "second outermost tag". Perhaps use these
names instead?
(same names are found in ietf-flexible-encapsulation)
o ietf-flexible-encapsulation / all features
The features are described as:
"This feature indicates whether the network element supports
specifying flexible rewrite operations";
Should this be s/whether/that ?
o ietf-flexible-encapsulation
There is some descriptive text in comments that should be moved to
description statements.
o ietf-flexible-encapsulation
The descriptions for pop/push are a bit terse. It seems to assume
that readers already know what this (from somewhere) is so it
doesn't need to be described. If this is intended, perhaps add a
reference to where this is described.
/martin
_______________________________________________
netmod mailing list
netmod@ietf.org
https://www.ietf.org/mailman/listinfo/netmod
--- End Message ---
_______________________________________________
netmod mailing list
netmod@ietf.org
https://www.ietf.org/mailman/listinfo/netmod