I want to thank the authors for plugging in a key piece of the (sub)interface 
model. The document is very well written. I would also like to thank Per 
Andersson for providing an early YANG doctor's review.

Please find below a few comments that I hope will improve the document further.


Section 1, paragraph 0
>    This document defines two YANG [RFC7950] modules that augment the
>    encapsulation choice YANG element defined in Interface Extensions
>    YANG [I-D.ietf-netmod-intf-ext-yang] and the generic interfaces data
>    model defined in [RFC8343].  The two modules provide configuration
>    nodes to support classification of Ethernet/VLAN traffic to sub-
>    interfaces, that can have interface based feature and service
>    configuration applied to them.

Perhaps further qualify that the document defines a YANG 1.1 model.

Section 1, paragraph 3
>       ietf-if-vlan-encapsulation.yang - Defines the model for basic
>       classification of VLAN tagged traffic, normally to L3 packet
>       forwarding services

Is it for L3 or L3 and L2 services?

Section 3, paragraph 4

The document talks about dropping packets if, for example, on ingress if the 
VLAN tag does not match. Where are these statistics being maintained? Are there 
other statistics that would be helpful to debug issues?

Section 3, paragraph 4
>      augment /if:interfaces/if:interface/if-ext:encapsulation
>                /if-ext:encaps-type:
>        +--:(dot1q-vlan)
>           +--rw dot1q-vlan
>              +--rw outer-tag
>              |  +--rw tag-type    dot1q-tag-type
>              |  +--rw vlan-id     vlanid
>              +--rw second-tag!
>                 +--rw tag-type    dot1q-tag-type
>                 +--rw vlan-id     vlanid

Interesting choice of name for the second tag as 'second-tag'. I would have 
thought that since the outer tag is called 'outer-tag', the inner tag would 
have been called 'inner-tag'. Or even the use of  's-tag' and 'c-tag’ would be 
familiar. 

Section 4, paragraph 0
>    The Interface Flexible Encapsulation model is designed to allow for
>    the flexible provisioning of layer 2 services.  It provides the
>    capability to classify and demultiplex Ethernet/VLAN frames received
>    on an Ethernet trunk interface to sub-interfaces based on the fields
>    available in the layer 2 headers.  Once classified to sub-interfaces,
>    it provides the capability to selectively modify fields within the
>    layer 2 frame header before the frame is handed off to the
>    appropriate forwarding code for further handling.  The forwarding
>    instance, e.g., L2VPN, VPLS, etc., is configured using a separate
>    YANG configuration model defined elsewhere.

While it is not the responsibility of this draft to define how the other models 
define the appropriate forwarding behaviour, it is not clear how this model is 
supposed to interoperate with these other models. Take the L2VPN YANG module 
draft as an example. How are the tags defined here mapped to a particular 
instance of L2VPN? Is there any guidance in this document to help those other 
models?

Section 4, paragraph 0
>    The model supports a common core set of layer 2 header matches based
>    on the 802.1Q tag type and VLAN Ids contained within the header up to
>    a tag stack depth of two tags.

Can a diagram (or reference to a diagram) that shows an Ethernet frame with one 
and two VLAN tags be added here? I think it would help to understand some of 
the terms like outer tag, EtherType, and others.

Section 5, paragraph 11
>                "Specifies the VLAN tag values to match against the
>                 second outermost 802.1Q VLAN tag in the frame.";

What is "second outermost 802.1Q VLAN tag"? Till now the reference has been to 
"second tag". Calling it "inner tag" or "C-VLAN tag" may make more sense.

Section 7.1, paragraph 4
>    {
>      "ietf-interfaces:interfaces": {
>        "interface": [
>          {
>            "name": "eth0",
>            "type": "iana-if-type:ethernetCsmacd",
>            "oper-status": "up",
>            "statistics": {
>              "discontinuity-time": "2023-12-15T09:04:00-05:00"
>            }


If this is a configuration example, why is 'oper-status' being configured?

Section 10, paragraph 0
>    The "ietf-if-flexible-encapsulation" and "ietf-if-vlan-encapsulation"
>    YANG modules define data models that are designed to be accessed via
>    YANG-based management protocols, such as NETCONF [RFC6241] and
>    RESTCONF [RFC8040].  These protocols have to use a secure transport
>    layer (e.g., SSH [RFC6242], TLS [RFC8446], and QUIC [RFC9000]) and
>    have to use mutual authentication.

Please make sure this template matches the template in 8407bis.

No reference entries found for these items, which were mentioned in the text:
[draft-ietf-netmod-intf-ext-yang]. I think you meant 
[I-D.ietf-netmod-intf-ext-yang].

Possible DOWNREF from this Standards Track doc to [IEEE_802.1Q_2022]. If so,
the IESG needs to approve it.

Found terminology that should be reviewed for inclusivity; see
https://www.rfc-editor.org/part2/#inclusive_language for background and more
guidance:

 * Term "native"; alternatives might be "built-in", "fundamental", "ingrained",
   "intrinsic", "original"

-------------------------------------------------------------------------------
NIT
-------------------------------------------------------------------------------

All comments below are about very minor potential issues that you may choose to
address in some way - or ignore - as you see fit. Some were flagged by
automated tools (via https://github.com/larseggert/ietf-reviewtool), so there
will likely be some false positives. There is no need to let me know what you
did with these suggestions.

Section 5, paragraph 4
>        "WG Web:   <http://tools.ietf.org/wg/netmod/>

Substitute http://tools with https://datatracker

Section 5, paragraph 9
>          "VLAN encapsulations YANG model";

s/VLAN encapsulations YANG model/Initial version of the model./

Section 6, paragraph 4

       "WG Web:   <http://tools.ietf.org/wg/netmod/>
Substitute http://tools with https://datatracker

Section 6, paragraph 10
>          "Interface flexible encapsulations YANG model";

s/Interface flexible encapsulation YANG model/Initial version of the module/

Section 6, paragraph 9
>          "This feature indicates that the network element supports
>            specifying flexible rewrite operations.";

One extra space in the indent.

Section 6, paragraph 12
>                    "For IEEE 802.1Q interoperability, when matching two
>                     tags, it is required that the outermost (first) tag
>                     exists and is an S-VLAN, and the second outermost
>                     tag is a C-VLAN.";


Please clarify what is "second outermost tag".

These URLs point to tools.ietf.org, which has been taken out of service:

 * http://tools.ietf.org/wg/netmod/

These URLs in the document did not return content:

 * https://www.rfc-editor.org/info/rfcBBBB


Section 1.3, paragraph 2
>  that they can be cleanly extended in future. 2.1. Interoperability with IEE
>                                    ^^^^^^^^^
The phrase "in future" is British English. Did you mean: "in the future"?

Section 6, paragraph 14
>  of other fields in the L2 header in future."; container dot1q-tag-rewrite { 
>                                   ^^^^^^^^^
The phrase "in future" is British English. Did you mean: "in the future"?

Section 6, paragraph 14
> , which is more specific than the catch all 'default' match."; uses flexible-
>                                   ^^^^^^^^^
It seems that a hyphen in the noun or adjective "catch-all" is missing.

Section 6, paragraph 14
> ng a VLAN tag, or rewriting the VLAN Id carried in a VLAN tag."; choice dire
>                                      ^^
Possible spelling mistake found.

Section 6, paragraph 16
> c with a single S-VLAN tag, with VLAN Id 11. COMMENT: { "ietf-interfaces:inte
>                                       ^^
Possible spelling mistake found.

Section 6, paragraph 20
> 0.3' configured to match traffic with a S-VLAN tag of 10, and C-VLAN tag of 2
>                                       ^
Use "an" instead of "a" if the following word starts with a vowel sound, e.g.
"an article", "an hour".

Section 6, paragraph 25
> and Dan Romascanu for their help progressing this draft. The authors would a
>                                  ^^^^^^^^^^^
The verb "help" is used with an infinitive.

Section 9.1, paragraph 17
> l, the classification of traffic arriving on an interface to a given sub-inte
>                                  ^^^^^^^^^^^
The usual preposition after "arriving" is "at", not "on". Did you mean
"arriving at"?

Section 10.2, paragraph 5
>  the 802.1Q bridge model can also be use to implement L2 services in some sce
>                                      ^^^
Did you mean "used"?

Section 10.2, paragraph 9
> ment scenarios for which they are optimized. Devices may choose which of the
>                                   ^^^^^^^^^
Do not mix variants of the same word ("optimize" and "optimise") within a
single text.

Mahesh Jethanandani
mjethanand...@gmail.com



_______________________________________________
netmod mailing list -- netmod@ietf.org
To unsubscribe send an email to netmod-le...@ietf.org

Reply via email to