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

["Rob Wilton (rwilton)" <[e-mail]>]

Hi Martin,

Please see comments inline ...

> -----Original Message-----
> From: Martin Bjorklund <[e-mail]>
> Sent: 22 August 2019 12:35
> To: Rob Wilton (rwilton) <[e-mail]>
> Cc: [e-mail]; [e-mail]
> Subject: Re: [netmod] WG Last Call: draft-ietf-netmod-intf-ext-yang-07
> 
> Hi,
> 
> Some comments inline.
> 
> 
> "Rob Wilton (rwilton)" <[e-mail]> 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
> >
> > > -----Original Message-----
> > > From: netmod <[e-mail]> On Behalf Of Vladimir
> > > Vassilev
> > > Sent: 13 August 2019 17:05
> > > To: Kent Watsen <[e-mail]>; [e-mail]
> > > Subject: Re: [netmod] WG Last Call:
> > > draft-ietf-netmod-intf-ext-yang-07
> > >
> > > 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 agree.
> 
> > > 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.
> 
> I agree.
> 
> 
> > > 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.
> 
> 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:
> > >     ...
> > >     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).
> 
> 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.


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


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

I prefer the shorter names personally.

Thanks,
Rob


> 
> /martin
> 
> 
> >
> > Thanks again for the review.  It is appreciated.
> >
> > Rob
> >
> >
> > >
> > > /Vladimir
> > >
> > > On 10/07/2019 02.15, Kent Watsen 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
> > > > [e-mail]
> > > > https://www.ietf.org/mailman/listinfo/netmod
> > >
> > > _______________________________________________
> > > netmod mailing list
> > > [e-mail]
> > > https://www.ietf.org/mailman/listinfo/netmod
> > _______________________________________________
> > netmod mailing list
> > [e-mail]
> > https://www.ietf.org/mailman/listinfo/netmod