[netmod] Re: AD review of draft-ietf-netmod-sub-intf-vlan-model-15

[Mahesh Jethanandani <[e-mail]>]

Hi Rob/Scott,

It has been quite some time since this was last reviewed. Therefore, I took another full pass at the draft and cross-checked it against the INTDIR (Darren Dukes, -17, 2025-11-03), SECDIR (Dave Thaler, -17, 2025-10-20), GENART (Sue, -17, 2025-10-14), and OPSDIR (Linda Dunbar, -17, 2025-10-22) reviews performed after my -15 review, plus the YANGDOCTORS early review (Per Andersson, -14). Thanks to all the reviewers. Most of the substantive comments across all of these reviews have been addressed in -18 -- I note the ones I found not fully resolved below, plus a couple of issues I don’t believe any prior review caught. I don't repeat items that I've confirmed are now fixed (e.g., the frame diagram, the dropped-config oper-status nit, the LAG/PCP/DEI expansions, the duplicate "WG Web" line, the E.g./ i.e. punctuation pass, the revision description text).

I have but one MAJOR comment, followed by a few MINOR/NIT level comments.

MAJOR:

Section 10, Security Considerations:

1461	   There are a number of data nodes defined in these YANG modules that
1462	   are writable/creatable/deletable (i.e., config true, which is the
1463	   default).  All writable data nodes are likely to be reasonably
1464	   sensitive or vulnerable in some network environments.  Write
1465	   operations (e.g., edit-config) and delete operations to these data
1466	   nodes without proper protection or authentication can have a negative
1467	   effect on network operations.  The following subtrees and data nodes
1468	   have particular sensitivities/vulnerabilities: There are no
1469	   particularly sensitive writable data nodes.

I find this paragraph self-contradictory as written: it states that all writable nodes are "likely to be reasonably sensitive or vulnerable,” introduces "the following subtrees and data nodes have particular sensitivities," and then immediately asserts there are none. In -15 (and still in -17, per GENART's comment on this same paragraph), Sections
10.1/10.2 enumerated the actual sensitive writable nodes module by module -- outer-tag/vlan-id, second-tag/vlan-id, the rewrite push/pop leaves, local-traffic-default-encaps -- along with the concrete operational risk of getting them wrong (misclassified or dropped traffic, CFM session failure, MTU mismatches on L2 tunnels). That enumeration is gone in -18, and what's left doesn't match the document's own description of the model's blast radius. GENART flagged the incomplete final sentence on -17, and it does not appear to have been fixed going into -18. I'd ask the authors to restore the per-node enumeration (it doesn't need to be split by module the way -15 had it, a single consolidated list would do) rather than the blanket statement.

MINOR:

Sections 5 and 6, VLAN and Flexible Encapsulation YANG Modules:

INTDIR's review of -17 asked whether filtering or rewriting ever needs to handle VLAN ID 0 (priority-tagged) or 4095 (reserved), given that the imported dot1q-types vlanid type is restricted to 1..4094. I don't see this addressed anywhere in -18 -- there's no discussion of what happens if a frame arrives with a reserved VLAN ID, or whether that's simply out of scope because such frames are non-conformant on the wire. I’d suggest at least a sentence confirming that assumption so implementers don't have to guess.

---

Section 5, dot1q-vlan container, and Section 6, match container:

602	              Only frames matching the classification configured on a
603	              sub-interface/interface are processed on that
604	              sub-interface/interface.
605
606	              Frames that do not match any sub-interface are processed
607	              directly on the parent interface, if it is associated with
608	              a forwarding instance, otherwise they are dropped.";

(and the equivalent text at lines 1019-1021 in Section 6's match container). INTDIR's -17 review pointed out roughly ten places where behavioral requirements are stated descriptively rather than with RFC 2119 keywords, and gave the ingress-drop sentence in Section 4 as its example -- that one is now fixed ("MUST be dropped and counted against the...counter", lines 380-382). This one and its Section 6 counterpart look like the same category of issue and weren't picked up in the same pass. I'd ask the authors to do the fuller sweep INTDIR asked for rather than fixing only the one example given.

---

Section 6, <CODE BEGINS> lines:

491	   <CODE BEGINS> file "[e-mail]"
...
570	     revision 2026-07-06 {

661	   <CODE BEGINS> file "[e-mail]"
...
741	     revision 2026-07-06 {

The vlan-encapsulation module's file date matches its revision date (2026-07-06), but the flexible-encapsulation module's <CODE BEGINS> filename says 2026-04-16 while its revision statement says 2026-07-06. 

---

Section 9 / Section 10 boundary:

I think an Operational Considerations section would be a natural place to answer OPSDIR's question -- "is the intent of this model purely to provide a standardized YANG abstraction for management systems, or to influence device-level configuration semantics" -- which as far as I can tell wasn't directly answered in the document itself (Appendix A.3 was extended to address the *coexistence* half of OPSDIR's comment, which I think works, but not the intent question). It doesn't need to be a big section, but a short one covering deployment/manageability expectations would likely close out both threads at once.

---

General, YANG Doctors review:

Since the document exited the WG the module has picked up the match-precedence normative text in Section 4, lost the dot1q-tag-rewrites feature, and had its Security Considerations restructured. None of that is necessarily a problem, but it's enough churn that I'd feel better having the WG look at -18 (or whatever revision addresses the items above) before this goes to the telechat, rather than relying on a WG review of a fairly different -14.

Chairs, once the authors have addressed these comments, could you run this through the WG again?

----------------------------------------------------------------------
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. There is no
need to let me know what you did with these suggestions.

Section 6, local-traffic-default-encaps description:

1106	                The default encapsulation can only be specified for
1107	                802.1 VLAN tags that are matched by the encapsulation.
s/802.1 VLAN tags/802.1Q VLAN tags/

---

Section 6, match container description:

1025	                sub-interface with the most specific match.  E.g.,
1026	                matching two VLAN tags in the frame is more specific
1027	                than matching the outermost VLAN tag, which is more
1028	                specific than the catch-all 'default' match.";
s/E.g., matching two VLAN tags/For example, matching two VLAN tags/
(GENART's suggested "E.g." -> "For example," fix seems to have been
applied everywhere else in the module but missed this instance.)

---

Section 5, contact statement:

531	       "WG Web:   <http://datatracker.ietf.org/wg/netmod/>
(and the equivalent line 702 in Section 6)
The tools.ietf.org -> datatracker.ietf.org substitution from my -15
review got applied, but the URL is still http rather than https.

Thanks.

> On Sep 29, 2025, at 6:51 PM, Mahesh Jethanandani <[e-mail]> wrote:
> 
> Hi Rob/Scott,
> 
> Going through the comments and responses I received, there is one comment that I do not see addressed. See inline with [mj].
> 
>> On Jul 29, 2025, at 1:00 PM, Mahesh Jethanandani <[e-mail]> wrote:
>> 
>> Hi Rob, 
>> 
>> Thanks for considering some of my comments. See inline with some of my follow-up comments. 
>> 
>> On Tue, Jul 15, 2025 at 9:48 AM Rob Wilton (rwilton) <[e-mail] <mailto:[e-mail]>> wrote:
>>> Hi Mahesh,
>>> 
>>> Thanks for the review!  I’ve added some comments inline …
>>> 
>>>  
>>> 
>>> From: Mahesh Jethanandani <[e-mail] <mailto:[e-mail]>>
>>> Date: Wednesday, 2 July 2025 at 07:25
>>> To: [e-mail] <mailto:[e-mail]> <[e-mail] <mailto:[e-mail]>>
>>> Cc: NETMOD Group <[e-mail] <mailto:[e-mail]>>
>>> Subject: AD review of draft-ietf-netmod-sub-intf-vlan-model-15
>>> 
>>> 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.
>>> 
>>> Yes, okay, will change to “two YANG 1.1 [RFC 7950] modules”.
>>> 
>>> 
>>> 
>>> 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?
>>> 
>>> So, it could be used for both, but the core idea is that this module is mainly used for L3 services, whilst the other, more flexible matching, is normally used for 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?
>>> 
>>> The interface extensions draft module augments the parent (trunk) interface with a generic unknown encaps counter that should be incremented if the package cannot be demultiplexed to any of the child sub-interfaces, i.e., if the packet doesn’t match any of them. In the generic case doesn’t necessarily have to just be because of VLAN Id, if more sophisticated matches were being performed.
>>> 
>>>        +--ro in-discard-unknown-encaps?   yang:counter64
>>> 
>>> 
>>> Do you think this would be sufficient?
>>> 
>> Yes. That and having the text above would help.
> 
> [mj] I do not see this in latest version of the draft.
> 
> Thanks.
>>>  
>>> 
>>> 
>>> 
>>> 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.
>>> 
>>> The package might have more than two tags, even though this model only allows for matching on two tags, which I think matches common current hardware capabilities.  We didn’t want to use s-tag and c-tag because that has other connotations (e.g., what the ethertype of the tag would be).
>>> 
>>> We would prefer to keep this as is, if that is okay?
>>> 
>> You are right that most hardware capabilities are for two tags, outer and inner tags. But if we want to keep the option of having one, two, or more tags, it would help to have a consistent naming scheme, as in, first-tag, second-tag, third-tag etc.
>>> 
>>> 
>>> 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?
>>> 
>>> 
>>> The draft does contain an example (in 7.2) of interacting with the IETF L2VPN YANG model, but that draft hasn’t progressed for quite some time, and hence it is possible that this example will end up being out of date.  If the L2VPN model does get published then perhaps bis’ing this document to update the example might be worth thinking about.
>>> 
>>> I’m slightly worried about saying too much in this document because there is quite a lot of flexibility, e.g., some deployments may choose to strip the tags across the L2VPN service, whereas other deployments may want to keep the tags intact, or even do a combination of the two.
>>> 
>>> I propose that we add some generic text about services being bound to an interface or sub-interface as the mechanism to pass traffic to/from interface/sub-interface to service.  Okay?
>>> 
>> Sounds good.
>>  
>>> 
>>> 
>>> 
>>> 
>>> 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.
>>> 
>>> Good idea.  Yes, we can add a diagram illustrating an Ethernet Frame.
>>> 
>>> 
>>> 
>>> 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.
>>> 
>>> The idea is that may be more than two tags, and this text is trying to clarify that it is the second tag starting with the outermost, rather than counting from the innermost tag.  In the YANG itself, we just refer to it as second-tag and only use “outermost” in the description to give additional clarity.  So, I propose that we leave this text unchanged.  Okay?
>>> 
>> Ok. 
>>> 
>>> 
>>> 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?
>>> 
>>> We had included oper-status and statistics so that the examples validated with YANG lint.  So, the choice is between examples that validate or examples that are minimal.  Do you have a preference as to which way we go?
>>> 
>> If we want to keep the example, make it clear that the example is for validation purposes and that some of the fields are destined for operational (ro) datastore. 
>>> 
>>> 
>>> 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.
>>> 
>>> Yes, we will update the template in this document and the other way to match 84097bis.
>>> 
>>> 
>>> 
>>> 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.
>>> 
>>> Is this really a DOWNREF?  If so, I assume that this doesn’t matter.
>>> 
>> You can ignore this message. 
>>> 
>>> 
>>> 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"
>>> 
>>> I would like to keep “native” because it is a well-known term in 802.1Q and moving away from this would likely only cause confusion.
>>> 
>> Ok. If it comes up in AUTH48, be prepared to explain it. 
>> 
>> Thanks.
>>> 
>>> 
>>> -------------------------------------------------------------------------------
>>> 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 <http://tools/> with https://datatracker <https://datatracker/>
>>> Will fix.
>>> 
>>> 
>>> 
>>> Section 5, paragraph 9
>>> >          "VLAN encapsulations YANG model";
>>> 
>>> s/VLAN encapsulations YANG model/Initial version of the model./
>>> 
>>> Will fix.
>>> 
>>> 
>>> 
>>> Section 6, paragraph 4
>>> 
>>>        "WG Web:   <http://tools.ietf.org/wg/netmod/>
>>> Substitute http://tools <http://tools/> with https://datatracker <https://datatracker/>
>>> Will fix.
>>> 
>>> 
>>> 
>>> Section 6, paragraph 10
>>> >          "Interface flexible encapsulations YANG model";
>>> 
>>> s/Interface flexible encapsulation YANG model/Initial version of the module/
>>> 
>>> Will fix.
>>> 
>>> 
>>> 
>>> Section 6, paragraph 9
>>> >          "This feature indicates that the network element supports
>>> >            specifying flexible rewrite operations.";
>>> 
>>> One extra space in the indent.
>>> 
>>> Will fix.
>>> 
>>> 
>>> 
>>> 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".
>>> 
>>> Please see reply above.
>>> 
>>> 
>>> 
>>> These URLs point to tools.ietf.org <http://tools.ietf.org/>, which has been taken out of service:
>>> 
>>>  * http://tools.ietf.org/wg/netmod/
>>> 
>>> Yes, will fix.
>>> 
>>> 
>>> 
>>> These URLs in the document did not return content:
>>> 
>>>  * https://www.rfc-editor.org/info/rfcBBBB
>>> 
>>> Yes, will fix.
>>> 
>>> 
>>> 
>>> 
>>> 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"?
>>> 
>>> Sorry for being British 😉.  Will fix.
>>> 
>>> 
>>> 
>>> 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"?
>>> 
>>> Ditto.
>>> 
>>> 
>>> 
>>> 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.
>>> 
>>> Will fix.
>>> 
>>> 
>>> 
>>> Section 6, paragraph 14
>>> > ng a VLAN tag, or rewriting the VLAN Id carried in a VLAN tag."; choice dire
>>> >                                      ^^
>>> Possible spelling mistake found.
>>> 
>>> Will leave.
>>> 
>>> 
>>> 
>>> Section 6, paragraph 16
>>> > c with a single S-VLAN tag, with VLAN Id 11. COMMENT: { "ietf-interfaces:inte
>>> >                                       ^^
>>> Possible spelling mistake found.
>>> 
>>> Will leave.
>>> 
>>> 
>>> 
>>> 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".
>>> 
>>> Will fix.
>>> 
>>> 
>>> 
>>> 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.
>>> 
>>> I think that this is fine, will leave to RFC editor.
>>> 
>>> 
>>> 
>>> 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"?
>>> 
>>> We think “on” is better in this context.  Will leave to the RFC editor.
>>> 
>>> 
>>> 
>>> 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"?
>>> 
>>> Will fix.
>>> 
>>> 
>>> 
>>> 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.
>>> 
>>> Will fix.  I’ll defer to Scott for the right spelling to use here since my British English is no good ;-)
>>> 
>>> Many thanks for your thoughtful review.
>>> 
>>> Kind regards,
>>> Scott & Rob
>>> 
>>> 
>>> 
>>> Mahesh Jethanandani
>>> [e-mail] <mailto:[e-mail]>
>>> 
>>> 
>> 
>> 
>> 
>> -- 
>> Mahesh Jethanandani
>> [e-mail] <mailto:[e-mail]>
> 
> 
> Mahesh Jethanandani
> [e-mail] <mailto:[e-mail]>

Mahesh Jethanandani
[e-mail]