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

From: "Acee Lindem (acee)" <[email-masked]>
Date: Mon, 5 Aug 2019 17:51:59 +0000

Hi Rob, 
It seems these counters have been considered at great length. I agree we should move forward with the model as it is today.
Thanks,
Acee

On 7/17/19, 11:36 AM, "Rob Wilton (rwilton)" <[email-masked]> wrote:

    Hi Acee,
    
    Thanks for the review, and apologies for the delayed reply.
    
    Regarding your stats question, there was some effort to handle this as part of defining the Ethernet interface YANG (IEEE 802.3.2-2019) (https://github.com/YangModels/yang/tree/master/standard/ieee/published/802.3) that I was involved in the earlier parts of.  Please see the attached XLS that was my earlier effort to rationalize the different ethernet interfaces counters between RFC 7223, Ethernet YANG, Etherlike MIB, RMON MIBs, and the counters exposed in the 802.3 clause 30 management API.
    
    For physical Ethernet interfaces (and anything that looks very similar to a physical Ethernet interface) then I think that we should be well covered by the combination of what is in ietf-interfaces, and IEEE 802.3.2.
    
    There are also some counters that apply to all Ethernet-like interfaces (really anything using Ethernet framing, but not an Ethernet physical layer).  The only counter currently defined in this category is in-drop-unknown-dest-mac-pkts in ietf-interfaces-ethernet-like.  Arguably we could also add a drop counter for frames that could not be demuxed to a sub-interface because it didn't match any of the sub-interface match expressions.
    
    There was one set of counters that 802.3.2 didn't want to include in their YANG module which related to the histogram frame statistics.  E.g. counters like the following (taken from IOS XR):
    
        Input pkts 65-127 bytes     = 0
        Input pkts 128-255 bytes    = 0
        Input pkts 256-511 bytes    = 0
        Input pkts 512-1023 bytes   = 0
        Input pkts 1024-1518 bytes  = 0
        Input pkts 1519-Max bytes   = 0
    
        Output pkts 65-127 bytes    = 0
        Output pkts 128-255 bytes   = 0
        Output pkts 256-511 bytes   = 0
        Output pkts 512-1023 bytes  = 0
        Output pkts 1024-1518 bytes = 0
        Output pkts 1519-Max bytes  = 0
    
    The 802.3 YANG WG had two issues with including counters like these:
    (1) They didn't really want to define histogram counter values for MTUs that are above the officially sanctioned MTU of 1514/1518 in the Ethernet specification, even though a lot of hardware supports up to 9K+.
    (2) The bucket ranges, at least once you get past the "512-1023" bucket, seem to somewhat vary by ASIC vendor.
    (3) IEEE 802.3 has a well defined internal management API (802.3 clause 30), and these histogram counters are not currently defined as part of that internal management API.  Extending the internal 802.3 management API seems tricky due to point (1) and (2) above.
    
    There was a suggestion in the 802.3 discussions that these counters could be defined in an IETF YANG module (skirting the IEEE concerns about maximum MTUs).  The proposal was to allow the operational data to return a list of bucket entries, where each entry defines the inclusive range of the bucket, and a count of the pkts that matched the bucket range (in either the ingress or egress direction).  This list would sit alongside a RECOMMENDATION of what bucket sizes to use, basically doubling each time up to the MTU, with some consideration around the 1514/1518/1522 boundary, but allowing freedom for a device to accurately return the histogram ranges actually supported by the hardware.
    
    However, I'm not sure it is worth delaying these drafts to add these counters in now, particularly because there are dependencies on them.  Possibly best done as future work?  Do you, or anyone else in the WG have an opinion on this?
    
    Thanks,
    Rob

    -----Original Message-----
    From: netmod <[email-masked]> On Behalf Of Acee Lindem (acee)
    Sent: 10 July 2019 14:09
    To: Kent Watsen <[email-masked]>; [email-masked]
    Subject: Re: [netmod] WG Last Call: draft-ietf-netmod-intf-ext-yang-07
    
    I have reviewed the subject document and support publication. I have the following comment:
    
      Perhaps ietf-interface-ethernet-like module ethlike:ethernet-like/ethlike:statistics could include a subset of the counters from RFC 3635. I say a subset since some of these counters are a bit archaic given the state of the technology and judgement should be applied on which to include.
    
      Thanks,
    Acee 
    
    On 7/9/19, 8:16 PM, "netmod on behalf of Kent Watsen" <[email-masked] on behalf of [email-masked]> wrote:
    
        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: Wed, 7 Aug 2019 14:14:43 +0000

Hi Acee,

Thanks.  This was also discussed in the NETMOD WG meeting (I know that you had a conflict).  

My reading of the consensus in the room was that the histogram statistics should be deferred at this time.  In particular, it seems like it would take some time/effort to agree on exactly how these counters should be modelled.  I also said that I would contact the IEEE 802.3 WG chair to see if we could progress a histogram model within the IETF.  I have sent an email out, but not heard anything back yet.

There was consensus in the room to add a sub-interface demux drop counter into the current module.

Lou also proposed that I rename "l2-mtu" to something like "max-frame-size" for consistency (I need to check the recording).

It also looks like I should generate and add -state trees to the appendix.

Thanks,
Rob

-----Original Message-----
From: Acee Lindem (acee) 
Sent: 05 August 2019 18:52
To: Rob Wilton (rwilton) <[email-masked]>; Kent Watsen <[email-masked]>; [email-masked]
Subject: Re: [netmod] WG Last Call: draft-ietf-netmod-intf-ext-yang-07

Hi Rob, 

    Hi Acee,

    

    

    

    Thanks,
    Rob

    -----Original Message-----
    From: netmod <[email-masked]> On Behalf Of Acee Lindem (acee)
    Sent: 10 July 2019 14:09
    To: Kent Watsen <[email-masked]>; [email-masked]
    Subject: Re: [netmod] WG Last Call: draft-ietf-netmod-intf-ext-yang-07

    
      Thanks,
    Acee 

        All,

        
        
        Thank you,
        NETMOD Chairs

---

From: Vladimir Vassilev <[email-masked]>
Date: Sat, 10 Aug 2019 16:23:53 +0200

> Lou also proposed that I rename "l2-mtu" to something like "max-frame-size" for consistency (I need to check the recording).
I think avoiding the MTU confusion was the correct decision. The MTU 
definition from RFC791 is consistently used in all RFCs known to me.

In addition to renaming the leaf there is contradiction between the 
description and range statements in draft-ietf-netmod-intf-ext-yang-07:

<CODE BEGINS>

...

      leaf l2-mtu {
        if-feature "configurable-l2-mtu";
        type uint16 {
          range "64 .. 65535";
        }
        description
          "The maximum size of layer 2 frames that may be transmitted
           or received on the interface (excluding any FCS overhead).
           In the case of Ethernet interfaces it also excludes the
           4-8 byte overhead of any known (i.e. explicitly matched by
           a child sub-interface) 802.1Q VLAN tags.";
        reference "RFC XXX, Section 3.5 Layer 2 MTU";
      }

...

<CODE ENDS>

Obviously minimum Ethernet frame is 64 bytes when FCS bytes are 
included. I also do not think there is consensus that 4-8 bytes should 
be subtracted if there are sub-interfaces with VLAN encapsulation 
configured since this complicates the logic.

IMO There have been too few reviews of this work. I will go through the 
draft and the relevant mailing list threads during the weekend and post 
my review.

/Vladimir

---

From: "Rob Wilton (rwilton)" <[email-masked]>
Date: Sun, 11 Aug 2019 11:50:38 +0000

Hi Vladimir,

Thanks for the comments. 

Regarding increasing the L2 MTU if 802.1Q tags are present, this is because of how IEEE 802.3 defines the MTU of an Ethernet interface (at least for a single 802.1Q tag).

If you have a mix of single and double tagged sub-intefaces, it also means that the IP MTU for all of those sub-interfaces can be the same.

E.g. if you set the L2 MTU of a physical interface to 1514 (excluding FCS bytes) then the IP MTU for each of the sub-interfaces would be 1500 bytes regardless of whether they are configured to match single or double VLAN tags.

Conversely, if you have a strict L2 MTU that doesn't have this flexibility then a single tagged sub-interface would end up with an IP MTU of 1496, and a double tagged sub-interface would end up with an IP MTU of 1492, complicating L3 configuration.

BTW, I'll be on PTO for a week, so please expect a delay in response.

Thanks,
Rob

-----Original Message-----
From: Vladimir Vassilev <[email-masked]> 
Sent: 10 August 2019 15:24
To: Rob Wilton (rwilton) <[email-masked]>; Acee Lindem (acee) <[email-masked]>; Kent Watsen <[email-masked]>; [email-masked]; Lou Berger <[email-masked]>
Subject: Re: [netmod] WG Last Call: draft-ietf-netmod-intf-ext-yang-07

> Lou also proposed that I rename "l2-mtu" to something like "max-frame-size" for consistency (I need to check the recording).
I think avoiding the MTU confusion was the correct decision. The MTU definition from RFC791 is consistently used in all RFCs known to me.

In addition to renaming the leaf there is contradiction between the description and range statements in draft-ietf-netmod-intf-ext-yang-07:

<CODE BEGINS>

...

      leaf l2-mtu {

...

<CODE ENDS>

Obviously minimum Ethernet frame is 64 bytes when FCS bytes are included. I also do not think there is consensus that 4-8 bytes should be subtracted if there are sub-interfaces with VLAN encapsulation configured since this complicates the logic.

IMO There have been too few reviews of this work. I will go through the draft and the relevant mailing list threads during the weekend and post my review.

/Vladimir

---

From: Vladimir Vassilev <[email-masked]>
Date: Sun, 11 Aug 2019 19:55:57 +0200

> Regarding increasing the L2 MTU if 802.1Q tags are present, this is because of how IEEE 802.3 defines the MTU of an Ethernet interface (at least for a single 802.1Q tag).
Where does IEEE 802.3 define "MTU of an Ethernet interface" (me being 
positive IEEE 802.3 or 802.1Q do not use MTU definition at all)?
> Conversely, if you have a strict L2 MTU that doesn't have this flexibility then a single tagged sub-interface would end up with an IP MTU of 1496, and a double tagged sub-interface would end up wit…

The problem can become very complicated  if we introduce new definition 
of MTU (I still do not agree L2 MTU is a thing).

Why the ifMtu object from IF-MIB is not mapped to config false 
/ietf-interfaces:interfaces/interface/ietf-interfaces-common:mtu in 
ietf-intefaces-common.yang and instead we need something else?

 From RFC 2863:

  ifMtu OBJECT-TYPE
     SYNTAX      Integer32
     MAX-ACCESS  read-only
     STATUS      current
     DESCRIPTION
             "The size of the largest packet which can be sent/received
             on the interface, specified in octets.  For interfaces that
             are used for transmitting network datagrams, this is the
             size of the largest network datagram that can be sent on the
             interface."
     ::= { ifEntry 4 }
IMO this sounds as useful today as it did before.

 From RFC 8343 "4.  Relationship to the IF-MIB":

    The ifMtu object from the IF-MIB is not mapped to the
    "ietf-interfaces" module.  It is expected that interface-type-
    specific YANG modules provide interface-type-specific MTU leafs by
    augmenting the "ietf-interfaces" model.

I am aware of that text too but I do not agree mapping ifMtu which is 
interface type independent to /interfaces/interface/mtu is not necessay 
and can be replaced by introducing "interface-type-specific MTU leafs".

Vladimir

---

From: Balázs Lengyel <[email-masked]>
Date: Mon, 12 Aug 2019 12:05:24 +0000

I have reviewed the subject document and support publication. 
Regards Balazs

-----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

---

From: Vladimir Vassilev <[email-masked]>
Date: Tue, 13 Aug 2019 18:05:22 +0200

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.

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".

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.

4. Dedicated module (ietf-if-loopback.yang) for the loopback 
functionality currently in ietf-interfaces-common.yang.

5. Less verbose loopback identities. With dedicated module the 
(loopback-* identities can be shortened skipping 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.

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.

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.

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

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.

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.

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
    ...

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.

14. I propose the in-pkts and out-pkts counters standardized too. 
https://github.com/YangModels/yang/blob/master/vendor/cisco/xe/1641/ietf-interfaces-ext.yang. 
And yes someone forgot to update the boilerplate text.

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.

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.

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.

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.

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.

/Vladimir

---

From: Martin Bjorklund <[email-masked]>
Date: Wed, 21 Aug 2019 15:59:50 +0200 (CEST)

Hi,

Here is my (late) review of draft-ietf-netmod-intf-ext-yang-07.  It is
a well-written document and my comments are mostly minor.

o  Abstract

  OLD:

   The YANG data model in this document conforms to the Network
   Management Datastore Architecture (NMDA) defined in RFC 8342.

  NEW:

   The YANG modules in this document conform to the Network
   Management Datastore Architecture (NMDA) defined in RFC 8342.

o  Section 1

     One of the aims of this draft is to provide a standard namespace and
     path for these configuration items regardless of the underlying
     interface type.  For example a standard namespace and path for

  "standard namespace and path" sounds a bit clumsy.  In section 2 you
  use "standard definition", perhaps that can be use here.

o  General

  s/this internet draft/this document/g
  s/this draft/this document/g

o  Section 2

  It seems this short section mostly says what is already said in
  section 1.  Remove?

o  3

  The text says:

    o  A parent interface leaf useable for all types of sub-interface
       that are children of parent interfaces.

  I suggest you add before that bullet:

    o  A generic "sub-interface" identity that an interface identity
       defintion can derive from if it defines a sub-interface.

o  3.1

  The text says:

    E.g. in the
    case that the link state transition is suppressed then there is no
    change of the /if:interfaces-state/if:interface/oper-status or
    /if:interfaces-state/if:interfaces/last-change leaves for the
    interface that the feature is operating on.

  This should be:

    no change of the /if:interfaces/if:interface/oper-status or
    /if:interfaces/if:interfaces/last-change leaves for the
    interface that the feature is operating on.

o  3.2

  It took me some time to understand the dampening algorithm.  Why is
  it important to talk about nominal values and that a device doesn't
  have to use 1000 as the penalty, as long as they scale the given
  values?  Wouldn't it be easier to describe the algorithm w/o any
  nominal values, and then explain that an implementation is free to
  implement this algorithm in any way it wants (which of course is
  true for everything we do...)

  Otherwise, the text currently says:

   Implementations are not required to use a penalty of 1000 units in
   their dampening algorithm, but should ensure that the Suppress
   Threshold and Reuse Threshold values are scaled relative to the
   nominal 1000 unit penalty to ensure that the same configuration
   values provide consistent behaviour.

  Should "should" in this text be "SHOULD"?  Or perhaps "MUST"?

o  3.2.1

  The text says:

   When the accumulated penalty reaches the default or
   configured suppress threshold, the interface is placed in a dampened
   state.

  The term "dampended state" occurs twice, in 3.2.1 and 3.2.3.  It is
  not used in the YANG model.  I suspect the leaf "suppressed"
  reflects this.  Perhaps align naming.

o  4

  It would be useful with a sentence that describes the relationship
  to /if:interfaces/if:interface/if:phys-address.

  It seems that the mac-address leaf is useful when the mac address
  can be configured; otherwise if:phys-address should be sufficient,
  right?  Should the mac-address leaf have a feature, or can we expect
  all implementations to support configurable mac addresses?

o  4

  You add a container 'statistics' under 'ethernet-like', so we have:

  +--rw interfaces
     +--rw interface* [name]
        ...
        +--ro statistics
           ...
        +--rw ethlike:ethernet-like
           +--ro ethlike:statistics
              ...

  Did you consider augmenting the container if:statistics instead?  I
  think it can be useful to have all statistics in the same container
  in this case.

o  7.2

  Perhaps show the (related) 'if:oper-status' leaf as well.

o  7.3

  Perhaps show the (related) if:phys-address' leaf as well in the
  first and third examples.

  Before the second example, perhaps change:

   The following example shows an explicit MAC address being configured
   on interface eth0.

  to:

   The following example shows the intended configuration for
   interface eth0 with an explicit MAC address being configured.

o  YANG nits

  Both YANG modules list the WG chairs; we don't do that anymore.

  Both YANG modules have the IETF Trust Copyright statement, but not
  exactly as it should be (try: pyang --ietf and/or pyang --ietf-help)

  Many descriptions are full sentences w/o the ending ".".

  The reference in the revision statement should be changed to "RFC
  XXXX: <title>"

/martin

---

From: "Rob Wilton (rwilton)" <[email-masked]>
Date: Thu, 22 Aug 2019 10:13:38 +0000

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
> 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.

> (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?

> 
> 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.

> 
> 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.

> (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.

> 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).

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.

> justified. All usecases of connecting a loopback connector I can think of
> require the system to not be aware there is special external loopback

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.

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.

> 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?  This might be something better handled via a deviation, or the device failing the configuration when it is verified.

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.

>     o  /if:interfaces/if:interface/loopback

Ack.

> 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.

> {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.

>     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).

> 
> 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.
> 

It is not changing the definitions of those counters at all.  It is just explaining the relationship between them.

> And yes someone forgot to update the boilerplate text.

This one I think that we need to get further input 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.

> 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".

> 
> 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.

> semantics as the ifMtu object from IF-MIB.

OK, lets keep this issue on that other thread.

> 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.

Thanks again for the review.  It is appreciated.

Rob

> > 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.
> >

---

From: Martin Bjorklund <[email-masked]>
Date: Thu, 22 Aug 2019 13:34:49 +0200 (CEST)

Hi,

Some comments inline.

"Rob Wilton (rwilton)" <[email-masked]> 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.
> 
> I'll also track these issues to closure on
> https://github.com/netmod-wg/interface-extensions-yang/issues
> 
> 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 agree.

> > functionality

I agree.

> 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.

Hmm.  Are you saying that this leaf doesn't have any direct effect in
the server?

> 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).

Well, this is already described in section 3.6.  The quoted text is
from Security Considerations.  I agree with Vladimir; I think his
suggested text is better.

> defines in-frames and out-frames, but these are only for Ethernet, but
> you are probably looking for a counter across all interface types.

in-pkts is presumably in-unicast-pkts + in-broadcast-pkts +
in-multicast-pkts.  So is this really needed?

> > ietf-

I had a similar concern for the modules in the sub-intf-vlan draft (I
will post my review of that doc later).

Currently we have:

  ietf-interfaces-common
  ietf-interfaces-ethernet-like
  ietf-if-l3-vlan
  ietf-flexible-encapsulation

I think we should have consistency, either:

  ietf-interfaces-common
  ietf-interfaces-ethernet-like
  ietf-interfaces-l3-vlan
  ietf-interfaces-flexible-encapsulation

or

  ietf-if-common
  ietf-if-ethernet-like
  ietf-if-l3-vlan
  ietf-if-flexible-encapsulation

/martin

---

From: Martin Bjorklund <[email-masked]>
Date: Thu, 22 Aug 2019 13:59:58 +0200 (CEST)

> > > From: netmod <[email-masked]> On Behalf Of Vladimir Vassilev

[...]

>   ietf-if-flexible-encapsulation

One comment re naming here.

The name "ietf-interfaces-common" seems a bit odd; isn't
"ietf-interfaces" for "common" definitions?

I was going to suggest "ietf-interfaces-extensions", but then I
re-read the description in the module:

      This module contains common definitions for extending the IETF
      interface YANG model (RFC 8343) with common configurable layer 2
      properties.

So perhaps "ietf-interfaces-l2-extensions" would be better?

... but then "forwarding-mode" isn't a l2 property.

/martin

---

From: "Rob Wilton (rwilton)" <[email-masked]>
Date: Thu, 22 Aug 2019 15:09:34 +0000

Hi Martin,

Please see comments inline ...

> From: Martin Bjorklund <[email-masked]>
> Sent: 22 August 2019 12:35
> To: Rob Wilton (rwilton) <[email-masked]>
> Cc: [email-masked]; [email-masked]
> > > oper- status debouncing/dampening functionality currently in
> > > ietf-interfaces-
> > > common.yang.
> >
> > > 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.
> Hmm.  Are you saying that this leaf doesn't have any direct effect in the
> server?

It depends on the device.  Some devices require a leaf like this (or similar) to correctly provision the services.  Other devices don't need it.

> 
> > > 12. I do not agree we need this text. Normally NETCONF devices
> > > should accept transactions to any valid configuration:
> > >
> > > OLD:
> > >     ...
> Well, this is already described in section 3.6.  The quoted text is from
> Security Considerations.  I agree with Vladimir; I think his suggested
> text is better.

Ah, OK.  If it is in the security section, then I agree that it isn't required.

> > > https://github.com/YangModels/yang/blob/master/vendor/cisco/xe/1641/
> > > ietf-
> > > interfaces-ext.yang.
> in-pkts is presumably in-unicast-pkts + in-broadcast-pkts + in-multicast-
> pkts.  So is this really needed?

It seems that this might be an issue related to the route policy language that wants the policy to use the total number of good pkts, but is unable of adding the 3 individual values together.

It seems slightly strange to me that the total input/output pkt counters don't exist, particularly if you had a point-to-point L2 protocol that doesn't differentiate between ucast, mcast and bcast.

> > > ietf- interfaces.
> I had a similar concern for the modules in the sub-intf-vlan draft (I will
> post my review of that doc later).
> 
> Currently we have:
> 
>   ietf-interfaces-common

I prefer the shorter names personally.

Thanks,
Rob

> > > 105 sessions).  Please send your comments to the working group
> > > mailing list.
> > > >
> > > ready for publication", are welcome!  This is useful and important,
> > > even from authors.
> > > >
> > > > Thank you,
> > > > NETMOD Chairs

---

From: Martin Bjorklund <[email-masked]>
Date: Mon, 26 Aug 2019 09:04:08 +0200 (CEST)

"Rob Wilton (rwilton)" <[email-masked]> wrote:
> > > I don't get the relevance of transactions

Neither do I.

> > > , but it isn't intended to
> > > break existing clients/YANG modules.
> > >
> > the
> It depends on the device.  Some devices require a leaf like this (or
> similar) to correctly provision the services.  Other devices don't
> need it.

Hmm, is this really a property of the device implementation?  Isn't it
a property of the data models that describe these services?

It seems to me that the semantics of this leaf is a bit unclear,
esp. if we look at the sub-intf-vlan model which uses this leaf:

    container dot1q-vlan {
      must
        'count(../../if-cmn:forwarding-mode) = 0 or ' +
        'derived-from-or-self(../../if-cmn:forwarding-mode,' +
                              '"if-cmn:layer-3-forwarding")' 

This means that it is perfectly ok for a client to configure
"dot1q-vlan" without also setting forwarding-mode.

> I prefer the shorter names personally.

Ok.

/martin

---

From: Vladimir Vassilev <[email-masked]>
Date: Tue, 27 Aug 2019 16:54:44 +0200

> 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-featu…

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.

> 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 defin…

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.

> 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.
> 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.
> 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?
> 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 -> Egres…
I can live with local/line/connector but I do not agree 
near-end/far-end/connector  is more confusing.
>        interface's ingress path, with equivalent semantics to internal
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.
> 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 sw…
OK
> 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).
> This might be something better handled via a deviation, or the device failing the configuration when it is verified.
OK
> 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 l…
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.
> 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.

> 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.
> 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
> 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 spe…
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.
> OK, lets keep this issue on that other thread.
OK
> I'm not opposed to shorter names, but would be interested in the views of others in the WG.

OK

/Vladimir

---

From: "Rob Wilton (rwilton)" <[email-masked]>
Date: Thu, 29 Aug 2019 09:29:43 +0000

Hi Martin,

Thanks for the review.  Mostly just updated, a couple of comments/questions inline below.

Latest source XML and txt are at: https://github.com/netmod-wg/interface-extensions-yang/tree/wglc

> From: netmod <[email-masked]> On Behalf Of Martin Bjorklund
> Sent: 21 August 2019 15:00
> To: [email-masked]
> Here is my (late) review of draft-ietf-netmod-intf-ext-yang-07.  It is a
> well-written document and my comments are mostly minor.
> 
> 
> o  Abstract
> 
>   OLD:
> 

OK.

>   use "standard definition", perhaps that can be use here.

OK. 

>   s/this draft/this document/g

OK.

>   It seems this short section mostly says what is already said in

At this stage yes, I think that this can be removed.

>        defintion can derive from if it defines a sub-interface.

OK.

>     interface that the feature is operating on.

OK.

>   true for everything we do...)

That makes sense.  I have tweaked the description.

>   Should "should" in this text be "SHOULD"?  Or perhaps "MUST"?

I've just removed this text.  As you say above, this is just a definition of an API, and vendors are free to implement however they wish as long as they are consistent with the API.

>   reflects this.  Perhaps align naming.

I've changed this to suppressed.

>   right?

Yes, if not configured, it returns the same value as if:phys-address (but constrained to being exactly 6 bytes long).

Perhaps a better long term solution here would be to allow if:phys-address to be configurable, and add a hw-phys-address config false leaf to indicate the default hardware value?

  Should the mac-address leaf have a feature, or can we expect
>   all implementations to support configurable mac addresses?

No, I don't think that all Ethernet interfaces will necessarily support configurable MAC addresses, e.g. I suspect that a lightbulb or simple home router probably does not allow it to be configured.  But I would expect Linux and more sophisticated network devices to allow MAC addresses to be configured.  Hence making this conditional on a feature makes sense.

         leaf mac-address {
           type yang:mac-address;
           description
             "The MAC address of the interface.";
         }

In terms of the relationship to phy-address, I would suggest the following update:

      leaf mac-address {
        if-feature "configurable-mac-address";
        type yang:mac-address;
        description
          "The MAC address of the interface.  The operational value
           matches the if:phys-address leaf defined in
           ietf-interface.yang";
      }

>   think it can be useful to have all statistics in the same container

I think that given that we are only adding one counters and they are probably widely useful for Ethernet interfaces then I think that augmenting the existing statistics container makes sense.  I will change this.

If we end up defining Ethernet histogram counters (which we are not doing now, and would be a separate draft), then we might want to think carefully whether or not to put those counters under the main interface statistics container, because I suspect that most of the time clients probably won't be interested in those values.

>   Perhaps show the (related) 'if:oper-status' leaf as well.

Yes.

>   first and third examples.

OK.

>    interface eth0 with an explicit MAC address being configured.

Fixed.

>   Both YANG modules list the WG chairs; we don't do that anymore.

Fixed.

>   Many descriptions are full sentences w/o the ending ".".

Fixed, I think.

>   The reference in the revision statement should be changed to "RFC

Fixed.

Thanks,
Rob

---

From: "Rob Wilton (rwilton)" <[email-masked]>
Date: Thu, 29 Aug 2019 16:20:02 +0000

Hi Vladimir,

Please see inline ...

> Sent: 27 August 2019 15:55
> To: Rob Wilton (rwilton) <[email-masked]>; [email-masked]
> >> status debouncing/dampening functionality currently in
> >> ietf-interfaces- common.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.
> 
> 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.

> >> 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.
> 
[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.

> > 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.

> 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.

> > 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
> 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.

> >> 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.

> applicable there too. IMO the OLD text was better.

OK, I will keep with the current definitions.

> >> 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.
> > Realistically, I think that ethernetCsmacd is dead.  Do you have other
> examples of interface types that do support loopback, but not in both
> directions?

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.

> > 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.
> >
> >
> >> that support e.g. rfc8344 ietf-ip (which has its dedicated forwarding
> >> leafs e.g.
> >> I support this leaf as config false. If NETCONF was not transactional a
> 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.
> >
> >
> identity
> > 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.
> >
> >
> 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).
> >
> >
> > It is not changing the definitions of those counters at all.  It is just
> explaining the relationship between them.
> 

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.

> contain
> > 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
> >
> >
> >> 16. We can replace
> > 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.

> probably there are systems out there that already use "in-discards" for

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.

Thanks,
Rob