Hi Rob, Inline with [mj].
> On Jul 30, 2025, at 7:59 AM, Rob Wilton (rwilton) <[email protected]> wrote: > > Hi Mahesh, > > Please see inline … > > From: Mahesh Jethanandani <[email protected] > <mailto:[email protected]>> > Date: Wednesday, 30 July 2025 at 01:05 > To: Rob Wilton (rwilton) <[email protected] <mailto:[email protected]>> > Cc: NetMod WG <[email protected] <mailto:[email protected]>>, Scott Mansfield > <[email protected] <mailto:[email protected]>> > Subject: Re: AD review of draft-ietf-netmod-intf-ext-yang-16 > > Hi Rob, > > Following up on your responses. > > On Tue, Jul 15, 2025 at 9:55 AM Rob Wilton (rwilton) <[email protected] > <mailto:[email protected]>> wrote: > HI Mahesh, > > Thanks for the review. > > We have only given quick answers for the moment – Scott and I will need in > Madrid (Tuesday morning) to meet/propose text and then give you more concrete > answers and text proposals, so please expect a more detailed follow up. > > From: Mahesh Jethanandani <[email protected] > <mailto:[email protected]>> > Date: Saturday, 26 April 2025 at 00:19 > 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-intf-ext-yang-16 > > Hi Authors, > > Thanks first of all for working on this document. It has been a long time > coming. > > Please see my comments on the document, which I hope will go towards > improving the document. > > ------------------------------------------------------------------------------- > COMMENT > ------------------------------------------------------------------------------- > > Section 2.5, paragraph 0 > > A maximum frame size configuration leaf (max-frame-size) is provided > > to specify the maximum size of a layer 2 frame that may be > > transmitted or received on an interface. The value includes the > > overhead of any layer 2 header, the maximum length of the payload, > > and any frame check sequence (FCS) bytes. If configured, the max- > > frame-size leaf on an interface also restricts the max-frame-size of > > any child sub-interfaces, and the available MTU for protocols. > > If we are providing the ability to configure maximum frame size, should we > providing a counter for packets that are discarded because the max size was > exceeded? > > This one probably needs a bit more thought/discussion. > > For Ethernet interfaces, there is already the in-error-oversize-frames > counter defined in 802.3.2. For other interfaces, I suspect that this would > mostly be caught by an MRU/MTU check. > > Hence, we don’t think that adding another counter is the right approach > because it likely wouldn’t be enforced. Normally, this would be handled at > other forwarding layers, if required. > > Ok. On second thought, I do agree that this can only be counted at the > physical interface level, and if it is being done as part of the 802.3 YANG > model, we should be good. > > > Section 2.6, paragraph 0 > > The sub-interface feature specifies the minimal leaves required to > > define a child interface that is parented to another interface. > > I find the definition of sub-interface to be under specified. For example, if > an implementation wants to enable configuration of a sub-interface that takes > a ip-address, how would they go about it? And how would that relationship be > tied to the parent-interface definition here? An example that shows how this > can be enabled can go a long way in making this draft very useful for > addressing a very common implementation. > > Ah, yes. You are right, this document doesn’t contain an example, but > draft-ietf-netmod-sub-intf-vlan-model does. Possibly referencing that > document as an example would be helpful. Or the example could be reproduced > in this document as well. > > We’ll also check whether any more descriptive prose or additional text in the > model is needed. > > I took a look at the example(s), but I it does not quite cover what I had in > mind. What you have as an example is subinterfaces with vlan tag, with the IP > address is configured on the interface, not on the subinterface. One of the > more common implementation that I have seen is where the IP address is > configured on the subinterface as follows. I do not see how this model covers > that use case. > > The example in 7.1 of draft-ietf-netmod-sub-intf-vlan-model covers > configuring an IPv6 address on the sub-interface. > > Specifically, in the JSON below, the name of the sub-interface is “eth0.1” > which is a sub-interface of “eth0”, and as you can see the IETF IPv6 > configuration model just works with sub-interfaces without requiring any > changes. This will be the same for any IETF YANG configuration model that > augments "/if:interfaces/if:interface". This is because in this YANG model a > sub-interface *is* also an interface, like Ethernet, LAG or tunnel > interfaces, …. Hence interface configuration works the same way for both > interfaces and sub-interfaces. Neat, no? ;-) [mj] Ahh! I missed that. I now understand the statement that subinterfaces (and other interfaces such as bond) are nothing but another type of interface. It is neat indeed. > > > { > "name": "eth0.1", // This is a sub-interface in the > /if:interfaces/if:interface list > "type": "iana-if-type:l2vlan", > "oper-status": "up", > "statistics": { > "discontinuity-time": "2023-12-15T09:04:00-05:00" > }, > "ietf-if-extensions:encapsulation": { > "ietf-if-vlan-encapsulation:dot1q-vlan": { > "outer-tag": { > "tag-type": "ieee802-dot1q-types:s-vlan", > "vlan-id": 10 > }, > "second-tag": { > "tag-type": "ieee802-dot1q-types:c-vlan", > "vlan-id": 20 > } > } > }, > "ietf-if-extensions:parent-interface": "eth0", // This is > sub-interface’s parent interface (also in the interfaces list) > "ietf-ip:ipv6": { > "enabled": true, > "forwarding": true, > "address": [ > { > "ip": "2001:db8:11::1", // Configuring an IPv6 address on the > sub-interface, exactly as you would on any other type of interface. > "prefix-length": 48 > } > ] > } > }, > > > > > interfaces > interface X > type ethernetCsmacd > subinterface Y > vlan vlan-id Y > ipv4 address Z > prefix-length A > ipv6 address B > prefix-length C > subinterface D > vlan vlan-id E > ipv4 address F > prefix-length G > ipv6 address H > prefix-length I > > I do realize that the issue is with ietf-ip and how it augments, i.e., an > augmentation of the interfaces model, which essentially rules out IP > addresses being configured under the subinterface. How would you propose we > solve for that use case using this model? > > Already solved and illustrated in the example. > > > > > The document introduces the concept of a sub-interface as a logical interface > that handles a subset of the traffic received by the parent interface. > However, it stops short of defining a container for it that could contain > attributes specific to the sub-interface. > > As I see it, the sub-interface will need some kind of index or id that > uniquely identifies it. That index or id can come in the form of VLAN. To > configure the VLAN, the ietf-if-vlan-encapsulation model could be used except > that it dot1q-vlan is at an interface level, not at a sub-interface level. > Similarly, if an ip-address needs to configured, ietf-ip module could have > been used, but like the vlanencapsulation, the IP address can only be > specified at the interface level. > This is common implementation, but this model and other related models fall > short of supporting such an implementation. > > The key part of the design of this model is that a sub-interface is just a > type of interface, which appears in the standard list of interfaces. > > E.g., you might have an entry called “Ethernet0/1” in the interfaces list, > and an entry called “Ethernet0/1.1” in the interface list that represents a > sub-interface of “Ethernet0/1”. It would have type l2vlan, a > parent-interface of “Ethernet0/1”. > > Hence, the VLAN encapsulation would be configured under the “Ethernet0/1.1” > entry in the interface list, as would the IP address, using the ietf-ip YANG > model with no additional changes required. > > See my example above. I see ietf-ip augmenting the interfaces modules as > follows: > > augment "/if:interfaces/if:interface" { > description > "IP parameters on interfaces. > > How would the subinterface be configured for IP address? > > See above. This just works for sub-interfaces as well. > > Your confusion may arise from the OpenConfig YANG models that do this > differently, where sub-interfaces are not interfaces, and hence the models > end up being more complex to handle that design decision. > > Please let me know if that clarifies or if a short call to explain would be > helpful. [mj] I am good with the explanation. Thanks for providing it. At this point I will await an updated I-D with any changes we agreed upon for me to progress the draft. Cheers. > > Kind regards, > Rob > > > > Cheers. > > Note, the approach in this draft differs from OpenConfig where a > sub-interface is not an interface object, but a different object in the data > model that exists in a different list under the interface. > > The benefit of the approach in this draft, is that all other models that add > functionality or reference interfaces just work with sub-interfaces without > needing any changes. > > > > If this document considers all the work of defining a sub-interface to be > out-of-scope, it should remove this section and let another document define > it. > > No, this using this draft and the dot1q draft (for the case of VLAN > sub-interfaces) should be sufficient for configuring L2 and L3 VLAN > sub-interfaces. > > They should also be sufficient for generically modelling other types of > sub-interfaces, although other sub-interface specific fields may be required > or useful (e.g., as per the draft-ietf-netmod-sub-intf-vlan-model that adds > in the extra properties for VLAN sub-interfaces). > > > > Section 2.7, paragraph 1 > > The forwarding mode leaf provides additional information as to what > > mode or layer an interface is logically operating and forwarding > > traffic at. The implication of this leaf is that for traffic > > forwarded at a given layer that any headers for lower layers are > > stripped off before the packet is forwarded at the given layer. > > Conversely, on egress any lower layer headers must be added to the > > packet before it is transmitted out of the interface. > > I am assuming that 'forwarding-mode' applies to both the physical as well as > the logical (sub-interface) level. Can that be made clear? Also, if we are > identifying the forwarding mode, can it support counters related to them, > e.g., VLAN discards or IP discards etc.? > > For the first part of your question, we could do, but that might be best done > in the example, rather than in the normative text. Explicitly calling out > that this configuration also applies to sub-interface may make it appear to > be special in some way, as opposed to all regular interface augmentations > also applying to sub-interfaces because they are just another type of > interface. > > For your second question, I think that it would be best covered by forwarding > specific counters. E.g., IETF interfaces already defined an > in-unknown-protos. An IP forwarding model should have an invalid destination > IP address, the L2VPN model could report drops due to unknown MAC address (or > perhaps VLAN) depending on whether the MAC address are dynamically learned or > not. > > > > Section 4, paragraph 9 > > "WG Web: <http://tools.ietf.org/wg/netmod/ > > <http://tools.ietf.org/wg/netmod/>> > > > Please update the link to the datatracker link. > > Will do, thanks. > > > > Section 4, paragraph 26 > > presence > > "Enable interface link flap dampening with default settings > > (that are vendor/device specific)."; > > Not clear on how "default settings" can be vendor/device specific? Are > default values not specified in the model with the vendor/device overriding > the default values if necessary? > > We think that it may be hard to get exact agreement on what these default > settings should be because they are not defined by any IETF protocol and are > likely to differ by vendor. In these sorts of cases the OpenConfig group, > who have more deployment experience, seem to have reached the conclusion that > it is better to leave such defaults out of the models. > > As you say, this still leaves the choice to the vendors to augment in their > default values if they wish. > > > > Section 4, paragraph 26 > > leaf half-life { > > type uint32; > > units seconds; > > description > > "The time (in seconds) after which a penalty would be half > > its original value. Once the interface has been assigned > > a penalty, the penalty is decreased at a decay rate > > equivalent to the half-life. For some devices, the > > allowed values may be restricted to particular multiples > > of seconds. The default value is vendor/device > > specific."; > > reference "RFC XXXX, Section 2.3.2 Half-Life Period"; > > } > > Same comment as above. > > Thanks. Answer also as above. > > > > Section 5, paragraph 8 > > "WG Web: <http://tools.ietf.org/wg/netmod/ > > <http://tools.ietf.org/wg/netmod/>> > > Please update the link to the datatracker link. > > We will do. > > > > Section 5, paragraph 18 > > leaf in-pkts-64-octets { > > type yang:counter64; > > units frames; > > description > > "The total number of packets (including bad packets) > > received that were 64 octets in length (excluding framing > > bits but including FCS octets). > > Is the unit "frames" or "packets"? The description uses "packets". Can we be > consistent? > > > > Good catch. Probably using packets is fine, and this would be consistent > with the equivalent counters in RFC 2819. > > > > Section 6.1, paragraph 2 > > { > > "ietf-interfaces:interfaces": { > > "interface": [ > > { > > "name": "eth0", > > "type": "iana-if-type:ethernetCsmacd", > > "oper-status": "up", > > "statistics": { > > "discontinuity-time": "2023-12-15T09:04:00-05:00" > > }, > > "ietf-if-extensions:link-flap-suppression": { > > "down": 0, > > "up": 50 > > } > > } > > ] > > } > > } > > Thanks for extensive and useful complete examples. It would be helpful if > these were tagged with <CODE START> and <CODE ENDS> tag to enable extraction > of them from the document and validate them against the model. > > > > We will add these. > > > > Section 6.3, paragraph 6 > > { > > "ietf-interfaces:interfaces": { > > "interface": [ > > { > > "name": "eth0", > > "type": "iana-if-type:ethernetCsmacd", > > "phys-address": "00:00:5E:00:53:30", > > "oper-status": "up", > > "admin-status": "up", > > "if-index": 1, > > "ietf-if-ethernet-like:ethernet-like": { > > "mac-address": "00:00:5E:00:53:30", > > "bia-mac-address": "00:00:5E:00:53:30" > > }, > > "statistics": { > > "discontinuity-time": "2023-12-15T09:04:00-05:00" > > } > > } > > ] > > } > > } > > > > The following example shows the intended configuration for interface > > eth0 with an explicit MAC address configured. > > > > { > > "ietf-interfaces:interfaces": { > > "interface": [ > > { > > "name": "eth0", > > "type": "iana-if-type:ethernetCsmacd", > > "ietf-if-ethernet-like:ethernet-like": { > > "mac-address": "00:00:5E:00:53:30" > > } > > } > > ] > > } > > } > > > > After the MAC address configuration has been successfully applied, > > the operational state datastore reporting the interface MAC address > > properties would contain the following, with the mac-address leaf > > updated to match the configured value, but the bia-mac-address leaf > > retaining the same value - which should never change. > > > > { > > "ietf-interfaces:interfaces": { > > "interface": [ > > { > > "name": "eth0", > > "type": "iana-if-type:ethernetCsmacd", > > "phys-address": "00:00:5E:00:53:35", > > "oper-status": "up", > > "admin-status": "up", > > "if-index": 1, > > "ietf-if-ethernet-like:ethernet-like": { > > "mac-address": "00:00:5E:00:53:35", > > "bia-mac-address": "00:00:5E:00:53:30" > > }, > > "statistics": { > > "discontinuity-time": "2023-12-15T09:04:00-05:00" > > } > > } > > ] > > } > > } > > > These examples are confusing, if they are related, as it appears from the > description. Maybe part of the confusion is stemming from the fact that same > MAC addresses are reused in the example. > > The first example is that of the operational datastore before any config is > applied and shows both "mac-address" and "bia-mac-address" as > "00:00:5E:00:53:30”. > > The second example is of for intended configuration datastore and is trying > to set the "mac-address" to "00:00:5E:00:53:30". Could a different MAC > address be used here to demonstrate the change, e.g., "00:00:5E:00:53:35" or > something other than what is already showed in the first example. > > > > The third example appears to be the state of the operational datastore once > the above configuration is applied. According to the example, the > "mac-address" does not seem to have changed from before because the address > being configured is "00:00:5E:00:53:30", while the operational datastore is > showing the mac-address as "00:00:5E:00:53:35". On the other hand, the > bia-mac-address seems to have changed to "00:00:5E:00:53:30" because that is > the address that is configured in the config example, when BIA MAC addresses > are never supposed to change. > > > > Good catch. I think that there is a mistake in the examples, the intention > was that the :35 was used for the configured MAC address. With that change, > I think that the examples all become consistent with each other. > > > > Section 9, paragraph 0 > > The YANG module defined in this memo is designed to be accessed via > > the NETCONF protocol RFC 6241 [RFC6241]. The lowest NETCONF layer is > > the secure transport layer and the mandatory to implement secure > > transport is SSH RFC 6242 [RFC6242]. The NETCONF access control > > model RFC 8341 [RFC8341] provides the means to restrict access for > > particular NETCONF users to a pre-configured subset of all available > > NETCONF protocol operations and content. > > Please update this to reflect the latest changes in the template as > documented in rfc8407bis. > > > > We will do. > > > > ------------------------------------------------------------------------------- > 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) > <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 1, paragraph 3 > > Several of the augmentations defined here are not backed by any > > formal standard specification. Instead, they are for features that > > are commonly implemented in equivalent ways by multiple independent > > network equipment vendors. The aim of this document is to define > > common paths and leaves for the configuration of these equivalent > > features in a uniform way, making it easier for users of the YANG > > model to access these features in a vendor independent way. Where > > necessary, a description of the expected behavior is also provided > > with the aim of ensuring vendors implementations are consistent with > > the specified behavior. > > RFC 7950 does not use leaves. When referring to the plural of leaf, it uses > leafs. I would suggest: > > s/leaves/leafs/ > > in this paragraph and the rest of the document. > > We will change. > > > > Section 4, paragraph 11 > > "This module contains common definitions for extending the IETF > > interface YANG model (RFC 8343) with common configurable layer 2 > > properties. > > Maybe one too many "common" occurances in the above statement? > > > > We will elide the first “common”. > > > > Section 4, paragraph 48 > > Discontinuities in the values of this counter can occur at > > re-initialization of the management system, and at other > > times as indicated by the value of the 'discontinuity-time' > > leaf defined in the ietf-interfaces YANG module > > (RFC 8343). > > "; > > Does the closing quote and semicolon need to be on a newline? > > > > We will fix. > > > > These URLs point to tools.ietf.org <http://tools.ietf.org/>, which has been > taken out of service: > > * http://tools.ietf.org/wg/netmod/ <http://tools.ietf.org/wg/netmod/> > > > We will fix to point to datatracker. > > > > Section 2, paragraph 12 > > tate transition that occurs and reverts back within the specified time inter > > ^^^^^^^^^^^^ > Consider using just "reverts". > > Section 4, paragraph 28 > > specific. The penalty value for a link up->down state change is 1000 units." > > ^^^^^^^ > When "link-up" is used as a noun or modifier, it needs to be hyphenated. > > Section 4, paragraph 28 > > specific. The penalty value for a link up->down state change is 1000 units." > > ^^^^^^^ > When "link-up" is used as a noun or modifier, it needs to be hyphenated. > > Section 4, paragraph 39 > > f the number of frames that were well formed, but otherwise discarded > > because > > ^^^^^^^^^^^ > This word is normally spelled with a hyphen. > > > > This I think is for well-formed. We will fix. > > > > Section 5, paragraph 31 > > iption "The 'burnt-in' MAC address. I.e the default MAC address assigned to > > ^^^ > The abbreviation "i.e." (= that is) requires two periods. > > Section 5, paragraph 33 > > f the number of frames that were well formed, but otherwise discarded > > because > > ^^^^^^^^^^^ > This word is normally spelled with a hyphen. > > > > We will fix these. Thanks for the careful review. > > Kind regards, > Scott & Rob > Mahesh Jethanandani [email protected]
_______________________________________________ netmod mailing list -- [email protected] To unsubscribe send an email to [email protected]
