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 <mjethanand...@gmail.com> Date: Saturday, 26 April 2025 at 00:19 To: draft-ietf-netmod-intf-ext-yang....@ietf.org <draft-ietf-netmod-intf-ext-yang....@ietf.org> Cc: NETMOD Group <netmod@ietf.org> 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. 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. 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 vlan encapsulation, 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. 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/> 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/> 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), 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, which has been taken out of service: * 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 mjethanand...@gmail.com
_______________________________________________ netmod mailing list -- netmod@ietf.org To unsubscribe send an email to netmod-le...@ietf.org
