Hi Ole, Here's a patch which I think should help correct the issue - https://gerrit.fd.io/r/c/vpp/+/45490. Would you mind reviewing it?
Thanks, -Matt On Wed, Apr 8, 2026 at 3:37 PM Ole Troan via lists.fd.io <otroan= [email protected]> wrote: > Hi Matt, > > Thanks for fixing this one. > Yes this seems like a sound plan! > > Best regards, > Ole > > On 8 Apr 2026, at 21:24, Matthew Smith via lists.fd.io <mgsmith= > [email protected]> wrote: > > > Hi, > > A patch went in a while back that broke my client code which sends the > gre_tunnel_dump API and processes the replies. Here's the patch - > https://github.com/FDio/vpp/commit/74cf96576768f6e9200848c613428990049b3563 > . > > This was discovered a couple of months ago when we updated our build from > stable/2506 to stable/2510. At the time, we just applied a patch locally to > make things work because we did not have time to properly fix it. I'm > trying to update again from stable/2510 to stable/2602 so I thought I > should probably fix it upstream for real this time. > > Here are some details on the issues: > > The patch adds a global 'option status = "in_progress";'. I have seen that > option on individual API messages but I have no idea whether this has any > meaning when declared globally. Is this a valid usage or is it just being > silently ignored by vppapigen? > > It also adds this service definition: > > service { > rpc gre_tunnel_dump returns gre_tunnel_dump_reply events > gre_tunnel_details; > rpc gre_tunnel_dump_v2 returns gre_tunnel_dump_v2_reply events > gre_tunnel_details_v2; > }; > > I don't think "events" is correct there. I think if the API handlers for > gre_tunnel_dump and gre_tunnel_dump_v2 used a cursor and the macros to send > details messages in batches, it might be appropriate to use "stream" there > instead of event. As it is, those API handlers don't do that cursor magic, > they just iterate all GRE tunnels and send details messages without sending > the reply message (i.e. typical dump/details behavior), so really this > service definition and the dump_reply/dump_reply_v2 message types might be > entirely useless. > > I don't think vppapigen does the right thing with the names > gre_tunnel_dump_v2 and gre_tunnel_details_v2. The script expects > _dump/_details at the end of a dump/details API name. The new APIs should > be called gre_tunnel_v2_dump/gre_tunnel_v2_details. Random non sequitur - > there is one other dump/details message that likely has this problem > - policer_dump_v2. > > So... I'm trying to figure out how to make things work correctly again. > Ideally we could revert the patch and tell the author "try again". But it > was merged > 6 months ago and it adds new functionality (support for GRE > key), which I assume someone somewhere might care about and would not like > to disappear. So just reverting it is probably not the nice thing to do at > this point. So the second best option here IMO would be: > > - Remove the global option status in_progress. Even if it's a valid > configuration, I don't think the GRE APIs, which were formerly considered > production, should be considered "in progress". > - Put gre_tunnel_dump back the way it was before (remove it from that > service rpc definition, remove or deprecate the unused > gre_tunnel_dump_reply) > - Either rename the new APIs or add new ones with the correct names > and mark the bad ones deprecated so they can be removed later. > > Does this seem like a sound plan? Is any of this going to trip up the crc > checker script? Is there some reason why the stuff I wrote above is wrong > and this patch is good and I should leave it alone? > > Thanks, > -Matt > > > > > > > >
-=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#26955): https://lists.fd.io/g/vpp-dev/message/26955 Mute This Topic: https://lists.fd.io/mt/118730935/21656 Group Owner: [email protected] Unsubscribe: https://lists.fd.io/g/vpp-dev/leave/14379924/21656/631435203/xyzzy [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
