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 "[email protected]"
...
570 revision 2026-07-06 {
661 <CODE BEGINS> file "[email protected]"
...
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 <[email protected]>
> 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 <[email protected]>
>> 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) <[email protected]
>> <mailto:[email protected]>> wrote:
>>> Hi Mahesh,
>>>
>>> Thanks for the review! I’ve added some comments inline …
>>>
>>>
>>>
>>> From: Mahesh Jethanandani <[email protected]
>>> <mailto:[email protected]>>
>>> Date: Wednesday, 2 July 2025 at 07:25
>>> To: [email protected]
>>> <mailto:[email protected]>
>>> <[email protected]
>>> <mailto:[email protected]>>
>>> Cc: NETMOD Group <[email protected] <mailto:[email protected]>>
>>> 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
>>> [email protected] <mailto:[email protected]>
>>>
>>>
>>
>>
>>
>> --
>> Mahesh Jethanandani
>> [email protected] <mailto:[email protected]>
>
>
> Mahesh Jethanandani
> [email protected] <mailto:[email protected]>
Mahesh Jethanandani
[email protected]
_______________________________________________
netmod mailing list -- [email protected]
To unsubscribe send an email to [email protected]
