> 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 (#19325): https://lists.fd.io/g/vpp-dev/message/19325 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] -=-=-=-=-=-=-=-=-=-=-=-
