Subject: Re: [netmod] WG Last Call: draft-ietf-netmod-intf-ext-yang-07

From: "Rob Wilton (rwilton)" <[email-masked]>
Date: Tue, 5 Nov 2019 13:46:45 +0000

Hi Qin,

Thank you for the review comments.

Apologies for the late reply on action on the WG LC comments.

Please see inline ...

> -----Original Message-----
> From: netmod <[email-masked]> On Behalf Of Qin Wu
> Sent: 19 August 2019 10:23
> To: Kent Watsen <[email-masked]>; [email-masked]
> Subject: Re: [netmod] WG Last Call: draft-ietf-netmod-intf-ext-yang-07
> 
> I have reviewed this document and have a few comments as follows:
> 1. Suggest to add references for imported module
[RW] 

I presume that you mean:

  import ietf-interfaces {
    prefix if;
    reference
      "RFC 8343: A YANG Data Model For Interface Management";
  }

If so, this is now done.

 2. Suggest to add a
> paragraph in the section 5 to explain which common type or type in
> specific module is imported
[RW] 

Please can you clarify this comment, because I'm not sure what you are requesting here.  I've left an open issue to track this:  https://github.com/netmod-wg/interface-extensions-yang/issues/21

 3. s/ reference "Internet draft: draft-ietf-
> netmod-intf-ext-yang-07";/ reference "RFCxxxx: Common Interface Extension
> YANG Data Models";
[RW] 

I've changed the references to:

  revision 2019-11-04 {
    description
      "Initial revision.";

    reference
      "RFC XXXX, Common Interface Extension YANG Data Models";
  }

And

  feature carrier-delay {
    description
      "This feature indicates that configurable interface
       carrier delay is supported, which is a feature is used to
       limit the propagation of very short interface link state
       flaps.";
    reference "RFC XXXX, Section 2.1 Carrier Delay";
  }

 4. I am not sure L2 MTU is common attribute applicable
> to all packet frame based interface, in most case, we are using L3 MTU.
> >From the definition of L2 MTU
> " A layer 2 MTU configuration leaf (l2-mtu) is provided to specify the
> maximum size of a layer 2 frame that may be transmitted or received on an
> interface. "
> I am wondering this L2 MTU is related to Maximum Receive Unit defined in
> RFC4638. If the answer is YES, I would suggest to rename it, but it is
> still not clear whether it should be An common attribute part of ietf-
> interfaces-common.
> If it is No, I am wondering why L2 MTU is not augmented from IP address
> management module which define common MTU attribute, also it is not clear
> to me if ietf-interfaces-common Is positioned as technology specific
> model? When we choose to use MTU defined in RFC8344 and when we should
> choose to use L2 MTU defined in draft-ietf-netmod-intf-ext-yang-07.
> I think L3 MTU is common and widely deployed and supported by most of
> implementations. But go to L2 MTU:
> "
> The payload MTU available to higher layer protocols is either derived from
> the layer 2 MTU, taking into account the size of the layer 2 header, or is
> further restricted by explicit layer
> 3 or protocol specific MTU configuration."; "
> You add a lot of flexibility or multiple options, therefore I think it is
> hard to implement it.
[RW] 
Some platforms define MTU in terms of L3, and derive the maximum L2 frame size from that value.
Other platforms define MTU in terms of L2 frame size, and derive the maximum L3 packet size from that value.

It is also useful to be able to see (e.g. in operational state) the actual maximum L2 frame that may be sent/received on an interface.

Further, if a service is L2 based then describing the maximum L2 frame that can be forwarded is more meaningful that describing the L3 payload of the data that may be carried, particularly if the size of the L2 header may not be of fixed size (e.g. depending on how many VLAN tags are configured).

I've changed the name and definition of L2 MTU to:

    /*
     * Allows the maximum frame size to be configured or reported.
     */
    leaf max-frame-size {
      if-feature "max-frame-size";
      type uint32 {
        range "64 .. max";
      }
      description
        "The maximum size of layer 2 frames that may be transmitted
         or received on the interface (including any frame header,
         maximum frame payload size, and frame checksum sequence).

         If configured, the max-frame-size also limits the maximum
         frame size of any child sub-interfaces.  The MTU available
         to higher layer protocols is restricted to the maximum frame
         payload size, and MAY be further restricted by explicit
         layer 3 or protocol specific MTU configuration.";
      
      reference "RFC XXXX, Section 2.5 Maximum Frame Size";
    }

But I'll start a separate thread to close on the l2-mtu/max-frame-size issue (and the others).

Thanks,
Rob

> 
> -Qin
> -----Original Message-----
> From: netmod <[email-masked]> On Behalf Of Kent Watsen
> Sent: 2019. július 10., szerda 2:15
> To: [email-masked]
> Subject: [netmod] WG Last Call: draft-ietf-netmod-intf-ext-yang-07
> 
> All,
> 
> This starts a twelve-day working group last call for
> draft-ietf-netmod-intf-ext-yang-07
> 
> The working group last call ends on July 21 (the day before the NETMOD 105
> sessions).  Please send your comments to the working group mailing list.
> 
> Positive comments, e.g., "I've reviewed this document and believe it is
> ready for publication", are welcome!  This is useful and important, even
> from authors.
> 
> Thank you,
> NETMOD Chairs
> _______________________________________________
> netmod mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/netmod
> 
> _______________________________________________
> netmod mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/netmod

---

From: "Rob Wilton (rwilton)" <[email-masked]>
Date: Tue, 5 Nov 2019 15:04:38 +0000

Hi Vladamir,

> From: netmod <[email-masked]> On Behalf Of Rob Wilton (rwilton)
> Sent: 29 August 2019 17:20
> To: Vladimir Vassilev <[email-masked]>; [email-masked]
> Hi Vladimir,
> > From: Vladimir Vassilev <[email-masked]>
> > Sent: 27 August 2019 15:55
> > To: Rob Wilton (rwilton) <[email-masked]>; [email-masked]
> > On 22/08/2019 12.13, Rob Wilton (rwilton) wrote:
> >
> > > Hi Vladimir,
> > >
> > > Thanks for your detailed review.  Sorry for the slow reply, I've
> > > been
> > away.  I'm also about to be away again for a couple of days.
> > >
> > > Please see my comments inline ...
> > >
> > > I'll also track these issues to closure on
> > > https://github.com/netmod-wg/interface-extensions-yang/issues
> > >
> > >> From: netmod <[email-masked]> On Behalf Of Vladimir
> > >> Vassilev
> > >> Sent: 13 August 2019 17:05
> > >> I have reviewed the draft. I have the following (19) IMO useful
> > proposals:
> > >>
> > >> 1. Dedicated module (ietf-if-oper-status-debounce.yang) for the
> > >> oper- status debouncing/dampening functionality currently in
> > >> ietf-interfaces- common.yang.
> > > I don't think that we want a proliferation of too many separate YANG
> > modules for small features.  Each of the areas of different
> > functionality within this module are already conditional on
> > if-feature, so I don't think that there is a strong justification to
> > separating this out as a separate module.
> >
> > I still think that especially the "dampening" mechanism is not common
> > enough and is quite complex to be added to ietf-interfaces-common. If
> > a feature is not common or does not enable the use of generic modeling
> > mechanism (like sub-interfaces etc.) it should not be in
> > ietf-interfaces- common. I do not think "dampening" (maybe at some
> > point we should go back to damping instead e.g. rfc2439 ... seems
> > there is difference between dampening and damping and damping seems to
> > be the correct one) is that common to deserve a place in ietf-
> interfaces-common.
> [RW]
> 
> I've renamed the module to ietf-if-extensions.yang.
> 
> I still don't see that splitting this to a separate YANG module is
> helpful.
> 
[RW] 
I've now closed this issue (with the module renaming).

> 
> >
> > >
> > >> 2. In sec "3.1 Carrier delay" use of the under-defined "Carrier"
> > >> definition can be replaced with direct reference to the oper-status
> > >> leaf (which is what is actually targeted by the algorithm)
> > >> "Operational status transition debouncing".
> > > I think that different vendors have different names for this
> technology.
> > I've just used the one that our products use.  I think that this is
> > just a name, rather than something that has to be defined.  I could
> > add a comment that this feature is sometimes called hold time?
> >
> > I looked for precedents -  "carrier-delay" leaf Cisco, "debouncing-
> > interval" leaf Juniper, "interface-phys-holdtime-config"
> > leaf OpenConfig.
> >
> > I think "Carrier" is confusing since what is delayed actually is the
> > transition of the oper-status.
> [RW]
> 
> But it is not just the oper-status that is delayed (which would only
> affect manageability).
> 
> Instead, it is the internal notification to the higher layer protocols
> that the underlying interface link state has changed.  E.g. with carrier-
> delay down, the IP layer may still think that the interface is up when the
> Ethernet layer signalling indicates that the interface is actually down.
> 
> I'll have a think and see if I can come up with a clearer name for this.
[RW] 

Possibly, this could be renamed to something like "link-flap-suppression" or "state-flap-suppression"?

I've included this as one of the open issues, and will track on a separate thread.  

> 
> 
> >
> > >> 3. "timer-running" and "suppressed" leafs are both "config false"
> > >> and have "default" statements. Although this is valid YANG I do not
> > >> think the "default" statements are intended.
> > > I think that this is a more general question that needs a bit more
> > discussion.  Here, I am using defaults for the config false node to
> > document what the normal value is expected.
> > Well not a real issue but I thought it was an unusual use of default.
> 
[RW] 

I've removed the default values because they could potentially confusion if no value is returned (although RFC 8342 section 5.3 indicates the expected semantics in this case).

> 
> 
> > >
> > >
> > >> 4. Dedicated module (ietf-if-loopback.yang) for the loopback
> > >> functionality currently in ietf-interfaces-common.yang.
> > > Same answer as for 1. I don't think that we should have too many
> > > really
> > small modules.
> > If the loopback was modeled as a boolean leaf (as in OpenConfig) I
> > would have agreed. However even small modules that define base
> > identities benefit from dedicated namespace. For me
> > ietf-if-loopback.yang will pay off since loopback='internal' is better
> then loopback='loopback-internal'
> > and there are going to be many test cases that use that line. An last
> > but not least I never had problems with too much modularity.
> 
> OK.  So, I think that this issue is really the same issue as (5) below.
> I.e. you are asking for a separate module to keep the name of the
> identities shorter.
> 
> 
> > >
> > >> 5. Less verbose loopback identities. With dedicated module the
> > >> (loopback-* identities can be shortened skipping the prefix).
> > > I think that it is normal to bind the identity names to the common
> > > base
> > identity.  I don't see that the length of the identities should really
> > be an issue.
> > For me the length of identities does matter since I often use command
> > line tools. But it is mostly the irritation caused by the tautology
> > loopback='loopback-internal' that everyone writing network
> > interconnect testcases is going to be stuck with forever if we leave
> > the loopback control model as part of ietf-interfaces-common and not
> > separate ietf-if- loopback. What do others think?
> 
> There are some choices here:
> (1) Keep with identities with a "loopback-" prefix.  This causes
> loopback='loopback-internal'
> (2) Keep with identities, but loose the common prefix, i.e. the identities
> become "internal", "line", "connector"
> (3) Use shorter identities, but also put them in a separate module.
> (4) Use an enum rather than identities.  Although this has the potential
> issue that enums cannot be extended.
> 
> I've also asked the YANG doctors for their input on this question -
> consistent modelling behaviour across modules is also important.
> 
> So far the consensus is to keep them in the same module but drop the
> "loopback-" prefix.  Hence, this is my proposed resolution.
[RW] 
Issue closed.  I've keep the prefixes in the same module, but with shorter names by loosing the prefix.

> 
> 
> > >
> > >> 6. The draft introduces "loopback-internal", "loopback-line" and
> > >> "loopback-connector" loopback identities. What is confusing is that
> > >> "internal loopback" is historically the opposite of "external
> loopback"
> > >> which is a loopback with a connector. I think terminology already
> > >> in use like "near-end" and "far-end" is less confusing.
> > > The internal/line loopback configuration has been used in parts of
> > > the
> > industry for at least 20 years, so this terminology is already in use.
> > >
> > > I'm not sure that "near-end" and "far-end" would be less confusing.
> > Assuming that "loopback far-end" was equivalent to "loopback-line"
> > then it would be somewhat of a misnomer since it acts on the near end,
> > not the far end.
> > >
> > > I.e. both loopback internal, and loopback line act on the local
> > interface, the only difference is in which direction they reflect the
> > signals, i.e. Egress -> Ingress (internal), or Ingress -> Egress (line).
> 
> 
> > I can live with local/line/connector but I do not agree near-end/far-
> > end/connector  is more confusing.
> 
> OK.
[RW] 
Issue closed.

> 
> 
> > >
> > > Perhaps the description text could be slightly clarified here to
> > > help
> > avoid confusion?
> > >
> > > OLD:
> > >
> > >     The following loopback modes are defined:
> > >
> > >     o  Internal loopback - All egress traffic on the interface is
> > >        internally looped back within the interface to be received on
> the
> > >        ingress path.
> > >
> > >     o  Line loopback - All ingress traffic received on the interface
> is
> > >        internally looped back within the interface to the egress path.
> > >
> > >     o  Loopback Connector - The interface has a physical loopback
> > >        connector attached that loops all egress traffic back into the
> > >        interface's ingress path, with equivalent semantics to internal
> > >        loopback.
> > >
> > > NEW:
> > >
> > >     The following loopback modes are defined:
> > >
> > >     o  Internal loopback - All frames that egress out of the interface
> > >        are looped back internally within the interface hardware
> > >        to be received on the ingress path.
> > >
> > >     o  Line loopback - All ingress frames received on the interface
> from
> > >        the line are looped back within the interface hardware and
> > >        transmitted back out of the interface.
> > >
> > >     o  Loopback connector - The interface has a physical loopback
> > >        connector attached that loops all egress frames back into the
> > >        interface's ingress path, with equivalent semantics to internal
> > >        loopback.
> > Adding frames excludes continuous stream (no frame) devices like
> > legacy serial links and high speed transceivers, optical links.
> > Loopback is applicable there too. IMO the OLD text was better.
> 
> OK, I will keep with the current definitions.
> 
[RW] 
Issue closed.

> 
> 
> 
> > >> 7. I am not sure standardizing the "loopback-connector" identity is
> > >> justified. All usecases of connecting a loopback connector I can
> > >> think of require the system to not be aware there is special
> > >> external loopback connector on the interface.
> > > I think that it will depend on how smart of dumb the external
> > > loopback
> > connector is.  If it is just a dumb electrical or optical loopback
> > then the source and destination MAC addresses need to be swapped, or
> > otherwise any egress frames out of the interface will fail the
> > destination MAC address filter when they are looped around.
> > OK
> 
> I've closed this issue.
> 
> > >
> > > Some implementations also use this configuration to force self ping
> > packets out through the interface, so that the full datapath is
> > tested, rather than the packets being looped internally within the L3
> > forwarding code.
> > >
> > >
> > >> 8. Some interfaces that implement "loopback-internal" do not
> > >> implement "loopback-line" - e.g. classical ethernetCsmacd
> > >> (Carrier-sense multiple access with collision detection) has a
> > >> physical layer that by design can not implement such loopback.
> > >> Maybe introducing a dedicated feature to enable the "loopback-line"
> > >> is a good
> > idea.
> > > I'm not sure on this one, i.e. whether it really helps or just adds
> > extra clutter.
> > > Realistically, I think that ethernetCsmacd is dead.  Do you have
> > > other
> > examples of interface types that do support loopback, but not in both
> > directions?
> > All interfaces that are not point-to-point (e.g. common wifi).
> 
> Even then, I still think that loopback internal and line can both work.
> 
> Loopback internal - you loop all frames that would have been transmitted
> out of the radio interface.
> Loopback line - you loop and frames received on the radio interface.
> 
> Having a connector on a radio interface probably doesn't make as much
> sense.
> 
> But still suggest that we rely on deviations to handle this case, and
> hence close this issue.
[RW] 

Issue closed.

> 
> 
> > > This might be something better handled via a deviation, or the
> > > device
> > failing the configuration when it is verified.
> > OK
> > > As a side note, one of the limitations of features and deviations is
> > that the apply to all interfaces on the device, but the actual
> > properties of an interface might vary depending on the speed, type and
> > hardware associated with the interface.
> > >
> > >
> > >> 9. Appropriate entry in the "11. Security Considerations" noting
> > >> the possibility of DoS attacks and broadcast traffic storms
> > >> resulting from
> > >> loopbacks:
> > >>
> > >> OLD:
> > >>
> > >>      The following leaf could cause the interface to go down, and
> stop
> > >>      processing any ingress or egress traffic on the interface:
> > >>
> > >>      o  /if:interfaces/if:interface/loopback
> > >>
> > >> NEW:
> > >>
> > >>      The following leaf could cause the interface to go down, and
> stop
> > >>      processing any ingress or egress traffic on the interface. It
> > could
> > >>      cause broadcast traffic storms.
> > >>
> > >>      o  /if:interfaces/if:interface/loopback
> > >>
> > > Ack.
[RW] 

Draft updated.

> > >
> > >
> > >
> > >> 10. Introducing config true "forwarding-mode" leaf breaks clients
> > >> that support e.g. rfc8344 ietf-ip (which has its dedicated
> > >> forwarding leafs e.g.
> > >> /ietf-interfaces:interfaces/interface/ietf-ip:ipv4/forwarding ) by
> > >> introducing this new module with a new leaf they know nothing about.
> > >> I support this leaf as config false. If NETCONF was not
> > >> transactional a
> > global leaf enabling the forwarding configuration would be a feature.
> > >> But NETCONF is transactional.
> > > I don't get the relevance of transactions, but it isn't intended to
> > break existing clients/YANG modules.
> > >
> > > The idea of this leaf is that if it is configured then the system
> > > can
> > use it to check other constraints.  E.g. to validate that an L2 QoS
> > policy isn’t being configured on an L3 interface.  If the leaf isn't
> > configured then those constraints are not checked.
> > If this leaf is only enabling optional additional constrains (and this
> > is the only backward compatible option) It won't be that useful as
> > config true.
> > >
> > >
> > >> 11. The "forwarding-mode" leaf has the following set of identities
> > >> {optical-layer, l2-forwarding, network-layer}. We could make the
> > identity
> > >> names shorter and consistent. l1,l2,l3 or physical,data-link,network.
> > > I've tried to use names here that network engineers are most likely
> > > to
> > be familiar with.
> > >
> > > I think that using the OSI layer names (e.g. l1, l2, l3) would be
> > > too
> > terse.
> > >
> > > We could change "l2-forwarding" to "data-link-layer", but I would
> > > think
> > that people would be more familiar with "l2-forwarding" as a term.  E.g.
> > related to L2VPN.
[RW] 

I've made this leaf config false, and changed the names to "physical", "data-link", and "network".

> > >
> > >
> > >> 12. I do not agree we need this text. Normally NETCONF devices
> > >> should accept transactions to any valid configuration:
> > >>
> > >> OLD:
> > >>      ...
> > >>      Normally devices will not allow the parent-interface leaf to be
> > >>      changed after the interfce has been created.  If an
> implementation
> > >>      did allow the parent-interface leaf to be changed then it
> > >> could
> > cause
> > >>      all traffic on the affected interface to be dropped.  The
> affected
> > >>      leaf is:
> > >>
> > >>      o  /if:interfaces/if:interface/parent-interface
> > >>      ...
> > >>
> > >> NEW:
> > >>      ...
> > >>      Changing the parent-interface leaf could cause
> > >>      all traffic on the affected interface to be dropped.
> > >>      The affected leaf is:
> > >>
> > >>      o  /if:interfaces/if:interface/parent-interface
> > >>      ...
> > > This isn't about transactions so much as valid configuration.
> > >
> > > Normally, the name of the sub-interface is tightly bound to the
> > > parent
> > interface.  E.g. if the parent in "Ethernet0/1" then the sub-interface
> > would be "Ethernet0/1.1".  If you tried to change the parent-interface
> > leaf of "Ethernet0/1.1" to "Ethernet2/2" then I would expect the
> > system to reject that change (because the configuration is invalid not
> > because of transactions).
> > >
[RW] 

I have made this change (the text is in the security section).

> > >
> > >> 13. The in-drop-unknown-dest-mac-pkts changes the behavior of the
> > >> in- unicast-pkts,in-multicast-pkts and in-broadcast-pkts. I do not
> > >> agree
> > any
> > >> discarded packets in the forwarding process should be subtracted
> > >> from
> > the
> > >> interface counters.
> > >>
> > >> Here is the current description:
> > >>
> > >> OLD:
> > >>                   For consistency, frames counted against this drop
> > >>                   counters are also counted against the IETF
> interfaces
> > >>                   statistics.  In particular, they are included in
> > >>                   in-octets and in-discards, but are not included in
> > >>                   in-unicast-pkts, in-multicast-pkts or
> > >> in-broadcast-
> > pkts,
> > >>                   because they are not delivered to a higher layer.
> > >> NEW:
> > >>                   The implementation of this counter does not
> > >>                   change the behavior of the counters defined in
> > >>                   IETF interfaces statistics.
> > >>
> > > It is not changing the definitions of those counters at all.  It is
> > > just
> > explaining the relationship between them.
> >
> > The problem I see is that today there are existing systems that
> > implement ietf-interfaces and discard packets because they have
> > unknown MAC destination. But those systems do not subtract the number
> > of discarded packets from the "in-unicast-pkts" counter. Those systems
> > will have to change their behavior in a non-backward compatible way to
> > be able to implement the in-drop-unknown-dest-mac-pkts with this
> > description. In my view this text changes the definition of
> > in-unicast-pkts enforcing certain design that was not enforced before.
> 
> I don't think that those systems are correctly implementing the "in-
> unicast-pkts" counter.
> 
> I.e. you shouldn't be handing packets off to the IP layer to forward, if
> it fails an L2 dest MAC check.  If you don't hand the packet to IP, then
> you shouldn't be incrementing "in-unicast-pkts" as per the "delivered by
> this sub-layer to a higher (sub-layer)" comment.
> 
>            leaf in-unicast-pkts {
>              type yang:counter64;
>              description
>                "The number of packets, delivered by this sub-layer to a
>                 higher (sub-)layer, that were not addressed to a
>                 multicast or broadcast address at this sub-layer.
>                 ..."
> 
> And also the "prevent their being deliverable to a higher-layer protocol"
> comment in in-discards:
> 
>            leaf in-discards {
>              type yang:counter32;
>              description
>                "The number of inbound packets that were chosen to be
>                 discarded even though no errors had been detected to
>                 prevent their being deliverable to a higher-layer
>                 protocol.  One possible reason for discarding such a
>                 packet could be to free up buffer space.
> 
[RW] 

I've kept the original text, it is useful clarification, and does not
change the meaning of the existing counter.

> 
> 
> 
> 
> >
> > >
> > >
> > >
> > >>
> > >> 14. I propose the in-pkts and out-pkts counters standardized too.
> > >>
> > https://github.com/YangModels/yang/blob/master/vendor/cisco/xe/1641/ie
> > tf-
> > >> interfaces-ext.yang.
> > >> And yes someone forgot to update the boilerplate text.
> > > This one I think that we need to get further input on.
[RW] 

I've kept this as an open issue to get closure on.

> > >
> > >
> > https://github.com/YangModels/yang/blob/master/standard/ieee/published
> > /802
> > .3/ieee802-ethernet-interface.yang
> > >
> > > defines in-frames and out-frames, but these are only for Ethernet,
> > > but
> > you are probably looking for a counter across all interface types.
> > Yes. in-pkts and out-pkts are used by OpenFlow, OpenDaylight,
> > OpenConfig, Cisco etc. Those are interface independent. It is
> > impossible to implement ietf-interfaces for devices that provide only
> > in-pkts and out-pkts (that would be all OpenFlow switches) which is a
> > good argument to have the counters standardized.
> > >
> > >> 15. I propose that new "ietf-interfaces-common:in-discards-overflow"
> > >> counter is added. Currently the "ietf-interfaces:in-discards" can
> > contain
> > >> both discards like the ones accumulated in
> > >> in-drop-unknown-dest-mac-
> > pkts
> > >> and discards caused by overflows (performance related loss of
> > >> packets
> > like
> > >> freeing buffer space in devices that in certain cases are
> > >> forwarding slower then the line speed). Turns out knowing if device
> > >> is discarding
> > >> (loosing) packets due to performance shortage and discarding
> > (filtering)
> > >> unwanted packets are two different events that one needs to
> > differentiate
> > >> between are currently in the same in-discards counter. We can fix
> > >> that with the introduction of in-discards-overflow counter.
> > > This one I think that we need to get further input on.  I think that
> > this might be useful.  But we might need some care to ensure that it
> > fits cleanly with QoS drops.
> > >
> > > If we were to add this then the definition of "ietf-interfaces:in-
> > discards" cannot change.  i.e. In-discards-overflow would be a subset
> > of "in-discards".
> > OK
[RW] 

I've kept this as an open issue.

> > >
> > >
> > >> 16. We can replace
> > >> "ietf-interfaces-ethernet-like:in-drop-unknown-dest-mac-pkts" with
> > >> (in- discards - in-discards-overflow) for MAC Bridges or any other
> > >> Ethernet interface plus save us the introduction of technology
> > >> specific similar counters for the rest of the Bridges and non-
> Ethernet interfaces.
> > > For Ethernet, having a technology specific
> > > in-drop-unknown-dest-mac-pkts
> > is useful.
> > >
> > > In the WG discussion, there was agreement to also add a drop counter
> > > for
> > packets that are dropped because they cannot be demuxed to any sub-
> > interface.
> > >
> > > Personally, I think that it is useful to have an overall drop
> > > counter
> > that captures everything, along with more specific drop counters that
> > sometimes give more information as to what has causes specific drops.
> > Specifically, just because a more specific drop counter has been
> > defined, that doesn't mean that it shouldn't also be included in the
> > general drop counter.
> 
> > IMO the in-discards counter should be incremented in very rare
> > circumstances (e.g. ingress clock frequency above supported) and
> > ideally reserved for reporting performance issues in the MAC that
> > should normally not exist. The "unknown-dest-mac" and "packets that
> > are dropped because they cannot be demuxed to any sub-interface"
> > should be handled at another "sub-layer" and do not need to be
> > subtracted from the ietf-interfaces in-*-pkt counters because this is
> > very confusing. But probably there are systems out there that already
> > use "in-discards" for all sorts of discards.
> 
> My interpretation of RFC 8343 is that ingress packets to an interface are
> all included in the in-octets counter, and then categorized into one of 6
> packet counters:
>  - good packets (delivered to a high layer protocol) are counted against
> one of in-unicast-pkts, in-broadcast-pkts or in-multicast-pkts
>  - valid packets dropped because the higher layer protocol is unknown (or
> not configured/enabled) are counted against in-unknown-protos
>  - erroneous packets (e.g. all framing errors, too long, too short) are
> counted against in-errors,
>  - all other packets dropped on the ingress interface for any other reason
> not covered above are counted against in-discards
> 
> in-drop-unknown-dest-mac-pkts doesn't change the above definitions.  Is
> this the subset of in-discards that are dropped because the dest MAC
> address is invalid.  If frames counted against in-drop-unknown-dest-mac-
> pkts weren't also counted against in-discards then that would effectively
> changes the definition in-discards, and would break implementations that
> are not checking the in-drop-unknown-dest-mac-pkts counter.
> 
[RW] 

I have kept in-drop-unknown-dest-mac-pkts, because it is useful counter.

Thanks,
Rob

> Thanks,
> Rob
> 
> 
> > >
> > >
> > >> 17. I have separately posted my arguments against introduction of
> > >> leaf named l2-mtu and the need of a config false leaf that has
> > >> similar semantics as the ifMtu object from IF-MIB.
> > > OK, lets keep this issue on that other thread.
> > OK
> > >
> > >
> > >> 18. Some references to relevant IEEE standards and IEEE maintained
> > >> YANG modules should be added (in the scope of
> > >> ietf-interfaces-ethernet-
> > like).
> > >> Also a few lines explaining the policy change and why none of the
> > >> RFC3635 managed objects are part of the new
> > >> ietf-interfaces-ethernet-
> > like
> > >> YANG module.
> > > Yes, OK.
> > >
> > >
> > >> 19. ietf-if-common.yang and ietf-if-ethernet-like.yang instead of
> > >> ietf- interfaces-common.yang and ietf-interfaces-ethernet-like.yang.
> > >> Setting a shorter naming precedent for future modules augmenting
> > >> ietf- interfaces.
> > > I'm not opposed to shorter names, but would be interested in the
> > > views
> > of others in the WG.
> >
> > OK
> >
> >
> > /Vladimir
> >
> > >
> > > Thanks again for the review.  It is appreciated.
> > >
> > > Rob
> > >
> > >
> > >> /Vladimir