On Wed, Feb 26, 2020 at 01:33:08AM +0000, Alexander Clemm wrote: > Hi Juergen, > > thank you for your comments! Please find replies inline, <ALEX> > > --- Alex > > > -----Original Message----- > > From: netmod <netmod-boun...@ietf.org> On Behalf Of Schönwälder, Jürgen > > Sent: Wednesday, February 19, 2020 8:57 AM > > To: Joel Jaeggli <joe...@bogus.com> > > Cc: netmod-cha...@ietf.org; netmod@ietf.org > > Subject: Re: [netmod] WGLC - Comparison of NMDA datastores - draft-ietf- > > netmod-nmda-diff-03 > > > > Hi, > > > > this is a good document that deserves to go forward. Some comments... > > > > - In the introduction, you may want to mention that applied config > > often differs from config because applied config includes stuff that > > was learned or generated by the system (e.g., IP addresses obtained > > via DHCP or generated by the kernel itself). This applies to systems > > that otherwise implement NC or RC in a synchronous update manner, > > i.e., a difference between <intended> and <applied> is for many > > operational systems likely the normal situation and not an > > exceptional one. > > <ALEX> > Agreed. Adding the following text in the Introduction to bring this out > clearer: " Likewise, during the course of operation, some data values may be > learned or updated by the system itself over time, causing datastores to > diverge. " > </ALEX>
OK > > - I do not understand this: > > > > [...] (The filter dow not contain expressions that > > would match values data nodes, as this is not required by most use > > cases and would complicate the scheme, from implementation to > > dealing with race conditions.) > > > > Despite the wording nits, I fail to understand the race condition > > argument. It seems the filters are the same as we have them in other > > places and this is good and a strong argument by itself. Reusing > > concepts is a good thing. Just state that and remove potentially > > hand-waving arguments about race conditions created by filtering on > > values. (And subtree filters can filter out certain interfaces by > > matching the <name/> element.) > > <ALEX> What this meant to explain is that the filter simply defines which > nodes are in and out of scope, independent of their actual value. In other > words, you would not be able to specify something like "have this be in > scope, but only if its value is between 5 and 10". This is to keep things > simple; the race condition concerns if there are different speeds at which > values propagate. > In order to simplify this, I suggest we simply remove the text in parantheses > (which was meant to provide clarification, but seems to add confusion, hence > best to simply strike.) > </ALEX> If the filters work exactly the same way as they do elsewhere, then I am happy and yes removing the text in parenthesis is good. > > - I think you should import the term 'schema node' (and if necessary > > also other terms) from RFC 7950. Perhaps merge section 2+3 into a > > section Terminology that has the RFC2119 blurb and states that this > > specification uses the terminology defined in RFC 7950 and RFC 8342. > > > > <ALEX> > OK, we are merging 2+3 (both are very short anyway). Putting the following > sentence in lieu of the acronyms: This specification uses the terminology > defined in [RFC7950] and [RFC8342]. For definitions of terms such as > "datastore", "schema node", "<intended>", or "origin", please refer to those > RFCs. > </ALEX> This works for me. > > - Given that the applied configuration includes learned and system > > provided data, it may make a lot of sense to filter on origin so > > that learned or system generated config is not part of the > > comparison. I think this is really missing. Of course, one can > > filter the result to get rid of all 'learned' items but the whole > > point of the compare RPC is to avoid long responses that are not > > needed. The get-data operation defined in RFC 8526 has an origin > > filter that may be reused. (Perhaps it makes sense to align the > > parameters with RFC 8526 get-data even further.) > > <ALEX>Hmm, what are you exactly suggesting here? We could add a parameter to > include (or exclude) data of a certain origin. However, that would raise the > possibility that some differences are not found whose cause is precisely that > they have a different origin than anticipated (e.g., for which it was not > clear that they could be learned, or that they would be preconfigured). > Hence not making changes just yet; could you explain your > requirement/suggestion a little more? > </ALEX> I believe that there will be quite large differences between say <running> and <operational> due to learned applied config (think of address mapping tables or forwarding table entries originating from control plane protocols) and if I am not interested in this learned state, I like to have a filter that allows me to express that I like to have some of the learned data ignored. Similarly, I having a filter to express that I like to have all config false leafs ignored seems useful if I compare a configuration datastore with <operational>. There are several useful filters in the definition of get-data [RFC 8526] and I feel we are missing several of them here. Yes, there may be cases where I do not want to use some of the filters but that does not justify that other use cases like comparing <running> against <operational> are not well supported. > > - Why do we need the 'no-matches' leaf? Why not simply return an empty > > 'differences' container? > > > <ALEX> > I guess this is a design choice. I guess we could return an empty container > as well; the "no-matches" makes it more "explicit", and in terms of response > size it does not make a difference. Unless there is a strong preference, we > would like to prefer to keep it. (Or perhaps Andy wants to jump in?) > </ALEX> $ diff rfc7950.txt rfc7950.txt $ echo $? 0 Good old tools tend to return an empty diff and not a special value if there is no difference hence I found the design a bit surprising. Perhaps there is a reason behind the design but then it is not obvious to me. > > - Nit > > > > OLD > > > > RPC request to compare <operational< (source of the comparison) with > > <intended>(target of the comparison): > > > > NEW > > > > RPC request to compare <operational> (source of the comparison) with > > <intended> (target of the comparison): > > <ALEX> fixed the spacing > </ALEX> OK > > - I have not validated the examples. > > > > - Section 7 talks about rejecting frequent requests. It may be useful > > to specify which error response is returned in this case so that > > coders implement the same behavior. > > > <ALEX> We could add that. Which one would be appropriate to return / what > should be returned? > Should we simply return error-type "rpc", with error-tag "resource-denied"? > If we do that, should we also include error-info, specifying a time interval > after which it would be "safe" to issue the next diff request? > </ALEX> I don't have the answer but obviously it makes sense to clearly spell out the error behavior. ;-) > > - Perhaps the document should spell out how compare interacts with > > NACM. I kind of assume that NACM rules are applied before the > > content is compared, i.e., data that is not accessible won't get > > compared. Well, whatever the correct behavior is, I think this > > deserves to be spelled out. > > <ALEX> Per RFC 8341, the same access control rules apply to all datastores > subjected to NACM - presumably also to all datastores that can be subjected > to nmda-diff. From that perspective we didn't think it necessary to spell it > out, but perhaps it should be. > How about adding the following at the end of the first paragraph of Section 4: > "Any access control rules as configured by NACM [RFC 8341] are applied by the > server before the comparison is performed, that is, only datastore schema > nodes for which the client has read access are included in the comparison." > </ALEX> OK > > - I would probably have picked in ietf-interfaces example to avoid a > > reference to a work in progress but this does not really matter > > much. > > <ALEX> That would have been possible as well. We prefer sticking with this > example (it's just an example, so non-normative) if acceptable since it > generates less work, however, if there is insistence that we update, we will > be happy to do so. > As I said, this does not really matter much to me. (I would have picked ietf-interfaces because it is stable and most likely widely understood by pretty much everybody and it nicely can be used to explain why <running> and <operational> differ in many different ways, i.e., loopback addresses configured by the system, IP privacy addresses created by the system, IP addresses obtained by DHCP, IP addresses not in <operational> due to hardware not present.) /js -- Juergen Schoenwaelder Jacobs University Bremen gGmbH Phone: +49 421 200 3587 Campus Ring 1 | 28759 Bremen | Germany Fax: +49 421 200 3103 <https://www.jacobs-university.de/> _______________________________________________ netmod mailing list netmod@ietf.org https://www.ietf.org/mailman/listinfo/netmod
