While the process is in place to minimize the unnecessary pains, we probably also should use the judgement to balance between the monkey dance on the downstream consumer side and the monkey dance on the VPP developer side.
In this case: 1) for a long time, there is a "correct" value that should have been used, which is also the "standard" default. Therefore outside of CSIT and VAT (which is not recommended as a component anyway, AFAIK), I would expect everyone to already be using ~0. Being extra conservative - the vat code should be fixed to add the support, such that the ~0 is usable there. This is a good hygiene in any case, and should have been done in the first place instead of adding the hack of using the value of 0. 2) If we assume that there exist a downstream user using 0 in lieu of ~0 for this API call, and not reading the VPP mail list, we can tackle this by mentioning this change in the 21.10 release notes. together with (1) I would assert this will make any potential fix a trivial one-liner. Also, that fix is downwards compatible - so it will continue to work on the "old" versions of VPP - so the "upgrade to one release" freedom that we aim to provide is not constrained for the user who applies this bugfix. 3) with the above two items, I think going through the separate message and deprecation cycle creates unnecessary overhead on the VPP developer side, as compared to the relative overhead on the downstream consumer side. So, with that I would still suggest the variation of the third approach, whereby first the vat code is altered in the timeframe before 21.06 RC1, and this change can then go into the release notes of 21.06. After the RC1 branch split, we can adjust correct default. I think this would reasonably minimize the amount of work required on both sides, and still maintain the gracefulness of the change. --a On 5/4/21, Vratko Polak -X (vrpolak - PANTHEON TECH SRO at Cisco) <vrpo...@cisco.com> wrote: >> I would argue that this case can be classified as "bugfix" > > Yes, [0] was an ugly workaround, we can call it a bug. > But the important thing is there are users depending on the buggy behavior. > Also, the "buggy" behavior was explained in message documentation, > so if we "fix" the bug, we will need to make incompatible edit > in the documentation. > > Here is the longer version of the story. > Once upon a time, VPP had no strict process for handling API changes, > and sw_interface_dump message had no sw_if_index param. > Paul added [4] that param with the usual ~0 standing for "all". > But one of "downstream" users was VAT, and Paul had not added > anything related to the new param before message is sent [5]. > In those times, we had no support for non-zero default values, > so VPP saw zero as the value for sw_if_index and returned > just details for the local interface, no other. > CSIT was using the VAT call, so suddenly all tests started failing > due to missing details for other interfaces. > There was a discussion on vpp-dev, in 4 threads: [6] [7] [8] [9]. > I was not able to figure out a quick enough fix for VAT, > but my workaround for API has been merged. > Of course, after [2] nobody recalled > that the workaround is no longer needed. > >> the reason for it is minimize the amount of forced work downstream, >> caused by the API changes, and minimize the element of surprise. > > Ok, so there are actually three solutions (in my decreasing preference). > > One is to add sw_interface_dump_v2 without deprecating sw_interface_dump. > That way, old users are not surprised, and no work is required, > but the new users may be confused on why there are two very similar > messages. > (it counts as kind of "read and think to realize no real coding is needed" > work). > An example for multiple similar messages is > how we added create_loopback_instance without removing create_loopback. > > The second solution is to add _v2 but also deprecate the old message, > to keep the API simpler (the release after next) > at the cost of bothering those old users who relied on the "buggy" > behavior. > > The third solution is to keep the message name, > but change description and behavior (and bump semver), > and rely on old users (starting with VAT) to update their usage > (after recovering from the initial surprise). > >> and giving the folks a few weeks (namely, until after RC1), to adapt. > > That would be fine for people consuming master HEAD regularly. > I imagine there are people who upgrade VPP once a release (relying on API > stability) > without reading vpp-dev mailing list much. > Those are going to be surprised by the third solution. > > Vratko. > > [4] https://gerrit.fd.io/r/c/vpp/+/18693 > [5] > https://github.com/FDio/vpp/blob/6407ba56a392f37322001d0ffdca002223b095c0/src/vat/api_format.c#L5978 > [6] https://lists.fd.io/g/vpp-dev/topic/30423722#12521 > [7] https://lists.fd.io/g/vpp-dev/topic/30426855#12548 > [8] https://lists.fd.io/g/vpp-dev/topic/31234917#12817 > [9] https://lists.fd.io/g/vpp-dev/topic/31307751#12840 > > -----Original Message----- > From: Andrew 👽 Yourtchenko <ayour...@gmail.com> > Sent: Monday, 2021-May-03 11:41 > To: Vratko Polak -X (vrpolak - PANTHEON TECH SRO at Cisco) > <vrpo...@cisco.com> > Cc: jiangxiaom...@outlook.com; vpp-dev@lists.fd.io > Subject: Re: [vpp-dev] sw_interface_dump currently cann't dump interface > which sw_if_index == 0 #vapi > > I would argue that this case can be classified as "bugfix" - there was > no good reason to use the 0 as a wildcard value in the first place, > since it is a valid sw_if_index, and there is a perfectly good > "wildcard value" of ~0 that already works, right ? > > So I would say this discussion should serve as the announcement (Or > there can be another separate thread with the explicit subject of it), > and post-branch of stable/2106, we can apply the fix to consider the 0 > to be a valid interface ID. > > And of course bump the semver for those who look at it. > > It's always useful to keep in mind "why" the process is in place, when > evaluating how to apply it: > > the reason for it is minimize the amount of forced work downstream, > caused by the API changes, and minimize the element of surprise. > > In this case, having the "buggy" clients use ~0 in place of 0 (if they > ever used that) is strictly less work than having *all* clients to > switch to using a new message name even if they used the ~0 to begin > with. > > We can take care of minimizing the "element of surprise" by this > discussion, or maybe a separate mail - and giving the folks a few > weeks (namely, until after RC1), to adapt. > > This way the spirit of why the process there in the first place will > be fulfilled, without incurring the unnecessary effort for everyone. > > Does this make sense ? > > --a > > On 5/3/21, Vratko Polak -X (vrpolak - PANTHEON TECHNOLOGIES at Cisco) > via lists.fd.io <vrpolak=cisco....@lists.fd.io> wrote: >>> Is there any plan for support selecting only index==0 ? >> >> Good news first. >> I added the TODO here [0], but since then >> CSIT stopped using the VAT command in [1], >> and other uses based on PAPI should be ready since [2]. >> >> The bad news is VPP now has more strict process [3] >> regarding changes to API. >> This particular case is debatable, as the change >> will not affect the message CRC value, >> only the behavior for a particular argument value. >> But I think it would be an API change overall, >> so not directly allowed (as sw_interface_dump is a production API). >> >> The indirect fix is to add a new API message >> (sw_interface_dump_v2) and mark the old one as deprecated. >> >> Vratko. >> >> [0] https://gerrit.fd.io/r/c/vpp/+/19106 >> [1] https://gerrit.fd.io/r/c/csit/+/21149 >> [2] https://gerrit.fd.io/r/c/vpp/+/23530 >> [3] https://wiki.fd.io/view/VPP/ApiChangeProcess >> >> From: vpp-dev@lists.fd.io <vpp-dev@lists.fd.io> On Behalf Of >> jiangxiaom...@outlook.com >> Sent: Thursday, 2021-April-29 13:41 >> To: vpp-dev@lists.fd.io >> Subject: [vpp-dev] sw_interface_dump currently cann't dump interface >> which >> sw_if_index == 0 #vapi >> >> Hi Vratko Polak, >> >> currently sw_interface_dump will dump all interfaces if sw_if_index == 0. >> In >> my case, i need get interface information by id without by dumping all, >> but >> there is no way for this. >> I notice there is a TODO comment for sw_interface_dump in interface.api >> file. >> /** \brief Request all or filtered subset of sw_interface_details >> @param client_index - opaque cookie to identify the sender >> @param context - sender context, to match reply w/ request >> @param sw_if_index - index of the interface to dump info on, 0 or ~0 >> if >> on all >> TODO: Support selecting only index==0 when CSIT is ready. >> @param name_filter_valid - 1 if requesting a filtered subset of >> records >> else 0 >> if name filter is set as valid, sw_if_index value is ignored and >> all >> interfaces are examined >> @param name_filter - interface name substring filter. Eg. loop1 >> returns >> [loop1, loop10] >> */ >> define sw_interface_dump >> Is there any plan for support selecting only index==0 ? >> >
-=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#19334): https://lists.fd.io/g/vpp-dev/message/19334 Mute This Topic: https://lists.fd.io/mt/82453036/21656 Mute #vapi:https://lists.fd.io/g/vpp-dev/mutehashtag/vapi Group Owner: vpp-dev+ow...@lists.fd.io Unsubscribe: https://lists.fd.io/g/vpp-dev/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
