Hi, Some comments inline.
"Rob Wilton (rwilton)" <[email protected]> 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 <[email protected]> On Behalf Of Vladimir Vassilev > > Sent: 13 August 2019 17:05 > > To: Kent Watsen <[email protected]>; [email protected] > > 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? > > 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. > > 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? > > 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 /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 > > > [email protected] > > > https://www.ietf.org/mailman/listinfo/netmod > > > > _______________________________________________ > > netmod mailing list > > [email protected] > > https://www.ietf.org/mailman/listinfo/netmod > _______________________________________________ > netmod mailing list > [email protected] > https://www.ietf.org/mailman/listinfo/netmod _______________________________________________ netmod mailing list [email protected] https://www.ietf.org/mailman/listinfo/netmod
