Hi Robert, Thank you for your review of the draft and thoughtful comments, sincerely appreciated. Please find my answers and notes in-line tagged [DXJ].
Regards, Xiaojian -----Original Message----- From: Robert Wilton [mailto:[email protected]] Sent: Wednesday, January 17, 2018 12:54 AM To: dingxiaojian (A) <[email protected]> Cc: [email protected] Subject: Re: FW: New Version Notification for draft-ding-rtgwg-arp-yang-model-00.txt Hi Xiaojian, Just some quick (mostly structural) comments from a cursory review of the tree structure: 1) I don't think that you need the arp-static-tables container at all, since "if:interfaces/if:interface/ipv4/neighbor" can be used to configure static ARP entries. [DXJ]: Yes, the static arp configuration defined in "if:interfaces/if:interface/ipv4/neighbor" can be used to configure the static ARP entries under an interface. However, there may also have another scenario, which the static ARP entries are configured global and is not independent on the interface. In this case, we describe the static arp global configuration within the arp-static-tables container. 2) I would rename the "arp-statistics" container to "statistics", since it is already under the "arp" top level container. [DXJ]: Good point. We will change it in the next version. 3) I would delete the "arp/arp-statistics/local-statistics" container. These statistics can just be specified directly under the interface. [DXJ]: Sorry, it was a mistake. local-statistics was defined duplicated. We will delete the redundant part in the next version. 4) It looks like "augment /if:interfaces/if:interface/ip:ipv4/ip:neighbor:" is empty and can be deleted? [DXJ]: This question is relate to the first comment. "augment/if:interfaces/if:interface/ip:ipv4/ip:neighbor:" was refer to the local static configuration defined in RFC 7277. I think you are right, the empty augment will be deleted in the next version. 5) Not sure about the "vrf-name" augmentation to "/if:interfaces-state/if:interface/ip:ipv4/ip:neighbor", shouldn't the VRF binding be at the interface level rather than ARP entry level? [DXJ]: Good point. "vrf-name" entry will be deleted in the augmentation in the next version. ?6) Put the interface augmentations under an "arp" sub-container, then you can lose the "arp" prefix on the individual leaves. [DXJ]: Good point. The problem you said in indeed exist. In order to solve this problem, we refer to the method used in draft-ietf-netmod-rfc8022bis-09. In this draft, the "ietf-ipv6-unicast-routing" module is first defined, and its submodule "ietf-ipv6-router-advertisements" is also defined to augment the "ietf-interfaces" [I-D.ietf-netmod-rfc7223bis] and "ietf-ip" [I-D.ietf-netmod-rfc7277bis] modules. Likewise, in this case, the interface augmentations can be defined as a submodule of ARP YANG module. 7) Perhaps make "arp-gratuitous" a container with presence with arp-gratuitous-interval underneath, probably renamed just to "interval". [DXJ]: Good point. This makes the text more readable. 8) All the "config false" state variables should follow the NMDA architecture. I.e. for this draft, I would suggest just augmenting them under the config tree, and depend on RFC 7223bis. So all the ARP interface counters should probably go under /if:interfaces/if:interface:/arp:arp/arp:statistics [DXJ]: Good point. The next version will follow the NMDA architecture. 9) This is just a stylistic preference, but I would reduce (or perhaps avoid) groupings, I think that the YANG file is easier to read without them. [DXJ]: Good point. This comment is similar to comment 7. We will revise it together. 10) In terms of naming the statistics, I would suggest aligning them to the statistics style in RFC 7223 if possible. [DXJ]: Good point. We will align statistics names to the statistics style in RFC 7223 in the next version. Anyway, hopefully these comments are useful. Thanks, Rob On 15/01/2018 03:04, dingxiaojian (A) wrote: > Hi rtgwg, > > After IETF 100, we have received some valuable suggestions for improving the > ARP YANG module. According to the suggestion of WG chair, we put the work on > RTGWG and submit a new draft for ARP YANG module. > In the new version, we have also clarified some functions (e.g., > arp-static-table configuration) are different from the previously defined > ones (e.g., arp static tables defined in RFC 7277). > Please have a look and provide comments on the list. > Best Regards, > Xiaojian > > -----Original Message----- > From: [email protected] [mailto:[email protected]] > Sent: Friday, January 12, 2018 11:20 PM > To: Zhengfeng (Habby) <[email protected]>; dingxiaojian (A) > <[email protected]>; dingxiaojian (A) > <[email protected]> > Subject: New Version Notification for > draft-ding-rtgwg-arp-yang-model-00.txt > > > A new version of I-D, draft-ding-rtgwg-arp-yang-model-00.txt > has been successfully submitted by Xiaojian Ding and posted to the IETF > repository. > > Name: draft-ding-rtgwg-arp-yang-model > Revision: 00 > Title: YANG Data Model for ARP > Document date: 2018-01-11 > Group: Individual Submission > Pages: 16 > URL: > https://www.ietf.org/internet-drafts/draft-ding-rtgwg-arp-yang-model-00.txt > Status: > https://datatracker.ietf.org/doc/draft-ding-rtgwg-arp-yang-model/ > Htmlized: https://tools.ietf.org/html/draft-ding-rtgwg-arp-yang-model-00 > Htmlized: > https://datatracker.ietf.org/doc/html/draft-ding-rtgwg-arp-yang-model-00 > > > Abstract: > This document defines a YANG data model to describe Address > Resolution Protocol (ARP) configurations. It is intended this model > be used by service providers who manipulate devices from different > vendors in a standard way. > > > > > > Please note that it may take a couple of minutes from the time of submission > until the htmlized version and diff are available at tools.ietf.org. > > The IETF Secretariat > > _______________________________________________ > rtgwg mailing list > [email protected] > https://www.ietf.org/mailman/listinfo/rtgwg > . > _______________________________________________ rtgwg mailing list [email protected] https://www.ietf.org/mailman/listinfo/rtgwg
