From: rtgwg <rtgwg-bounces@xxxxxxxx> on behalf of Yingzhen Qu <yingzhen.ietf@xxxxxxxxx> Sent: 23 April 2021 20:22 Hi Martin, I just published version -08 and removed the sentence in the module description and made a few editorial changes. Thank you again for the review and please let us know if you have more comments. <tp> using a recent post, this is from Tom not Martin. I do not see an improvement, perhaps -08 is worse. I find the introductory text confusing. A small but for me a telling point; RIB needs an article, 'a' or 'the' which have slightly different meanings and is mostly lacking More technically, when RFC8349 was being developed. the sense was that 'path' had no meaning and so RFC8349 never uses it, only 'route'. You use 'path' so you have to define it which experience with RFC8349 suggests is not straightforward. You use 'best' in several places which is meaningless without criteria; best availability, best for delay variance, best for ...? Again RFC8349 gets it right by using 'preferred' but that could be wrong here. You seem to miss the concept that a RIB is a list of routes for one address family. I expect the Abstract to lead me into the Introduction which leads me into subsequent sections. Here I get confused as to just what this module is adding. The Abstract has 'multiple next hops', The Introduction has multiple paths, route metrics and administrative tags. s.3 then has augmentations for static routes to support multiple next-hop and more next-hop. which is then contradicted by 3.1 and 3.2 so what is the point of this I-D? I should be clear before I start to look at the details of the YANG module and I am not. So, Not Ready IMHO. Tom Petch Thanks, Yingzhen On Fri, Apr 23, 2021 at 10:10 AM Acee Lindem (acee) <acee@xxxxxxxxx<mailto:acee@xxxxxxxxx>> wrote: Hi Martin, See inline below. On 4/23/21, 3:19 AM, "Martin Björklund" <mbj+ietf@xxxxxxx<mailto:mbj%2Bietf@xxxxxxx>> wrote: Hi, Thank you for addressing my comments. Pruning to the one remaining question. Yingzhen Qu <yingzhen.ietf@xxxxxxxxx<mailto:yingzhen.ietf@xxxxxxxxx>> wrote: > Hi Martin, > > Thank you for your review, and we've published version -07 to address your > comments. > > Please see my answers below inline. > > Thanks, > Yingzhen > > On Wed, Apr 14, 2021 at 4:51 AM Martin Björklund via Datatracker < > noreply@xxxxxxxx<mailto:noreply@xxxxxxxx>> wrote: > > > Reviewer: Martin Björklund > > Review result: Ready with Nits > > > > Here is my YANG doctors review of draft-ietf-rtgwg-yang-rib-extend-06. > > This is a well-written draft, and my comments are minor. [...] > > o module description > > > > This YANG module extends the generic data model for > > RIB by augmenting the ietf-routing model. It is > > intended that the module will be extended by vendors > > to define vendor-specific RIB parameters. > > > > I don't think I understand this description. Here's my understanding, > > but I don't think it is correct: > > > > 1. This module extends the existing RIB data model by using > > augmentations. > > 2. The existing RIB data model is defined in the YANG module > > ietf-routing. > > 3. The purpose of this new module is to allow vendors to extend the > > the existing RIB data model with vendor-specific parameters. > > > > It seems 3 is at least incomplete, since this module defines some > > additional config param for static routes, and addtional state and > > statistics for ribs. > > > > It is not clear how vendors are expected to extend this model; the > > word "vendor" doesn't show up anywhere else. > > > > [Yingzhen]: This module does define additional parameters and is > augmenting the existing RIB model. The module can be further augmented. Any > suggestions for a replacement of "vendor-specific"? All models can be augmented so I don't think this needs to be spelled out. When it is spelled out like this I expect some discussion about how it differ from the "normal" augment that always can be done. Some modules define some generic common structure, but are not very useful unless they are augmented; they need to define how vendors (or sdos) should extend the module. However, I think that this module is useful on its own, and thus I would remove the sentence "It is intended ...". I'm not sure the source of this text but it has been almost boiler plate for protocol YANG models. Having said that, I don't have any problem removing it. Thanks, Acee /martin -- last-call mailing list last-call@xxxxxxxx https://www.ietf.org/mailman/listinfo/last-call
