On 6/7/26 11:46 AM, Eli Britstein via dev wrote: > > On 03/06/2026 22:21, Ilya Maximets wrote: >> As a general rule, code that requires ALLOW_EXPERIMENTAL_API is not >> allowed on the main branch. >> >> The exemption was granted to the tunnel offload implementation because >> it was a large piece of code that is hard to maintain separately and >> there was a hope that it will become stable in a relatively short >> amount of time. That was 5 years ago, so clearly that didn't work out. >> >> In the current code, even the parts that could be generic are gated >> by the ALLOW_EXPERIMENTAL_API macro. The reason is the noticeable >> performance impact packet metadata restoration API has on every packet >> even when the traffic is not offloaded. There was an attempt to make >> it faster by providing a dynamic mbuf flag, but checking dynamic flags >> still causes performance impact due to a series of indirect calls into >> DPDK per packet. >> >> We don't have test coverage for this code on main, we don't even build >> it in CI on main (and we shouldn't), and there is no much hope for it >> to become performant enough to become stable within a reasonable time >> frame. >> >> So, it's time to remove this code from main. It can live in the >> dpdk-latest branch where we have at least some compilation tests for >> the experimental API. And if someday it becomes stable, we can add >> it back to main. Though the code would likely need to be re-reviewed >> as it at least has some style issues. >> >> Keeping the generic post-processing API that is now supported by the >> dummy implementation, so the generic infrastructure will not get >> broken over time and it will be simple enough to bring support back or >> add different offload providers supporting post-processing for the >> metadata recovery. Having the dummy implementation should not be a >> big problem for the maintenance, as it is simple enough and it is >> tested in our CI. But if we won't get any non-dummy implementation of >> this API by the next LTS (Feb 2028), we'll remove it as well. The >> date is kind of arbitrary, but seems reasonable. > > Doca offloads will require this API. Hopefully it will be merged by then > to avoid this API removal. > >> >> Signed-off-by: Ilya Maximets <[email protected]> >> --- >> Documentation/howto/dpdk.rst | 5 - >> NEWS | 2 + >> lib/dpif-offload-dpdk-netdev.c | 751 +------------------------------- >> lib/dpif-offload-dpdk-private.h | 3 - >> lib/dpif-offload-dpdk.c | 17 +- >> lib/netdev-dpdk.c | 126 +----- >> lib/netdev-dpdk.h | 85 ---- >> 7 files changed, 17 insertions(+), 972 deletions(-) >> >> diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst >> index c4ab0091b..e7f20f8c9 100644 >> --- a/Documentation/howto/dpdk.rst >> +++ b/Documentation/howto/dpdk.rst >> @@ -401,11 +401,6 @@ Supported actions for hardware offload are: >> - VLAN Push/Pop (push_vlan/pop_vlan). >> - Modification of IPv6 (set_field:<ADDR>->ipv6_src/ipv6_dst/mod_nw_ttl). >> - Clone/output (tnl_push/push_vlan/output) for encapsulating over a tunnel. >> -- Tunnel pop, for packets received on physical ports. >> - >> -.. note:: >> - Tunnel offloads are experimental APIs in DPDK. In order to enable it, >> - compile with -DALLOW_EXPERIMENTAL_API. >> >> Multiprocess >> ------------ >> diff --git a/NEWS b/NEWS >> index 288ab8cc4..a8bd540bb 100644 >> --- a/NEWS >> +++ b/NEWS >> @@ -8,6 +8,8 @@ Post-v3.7.0 >> wasting resources on unused devices. This breaks DPDK netdev ports >> using the "class=eth,mac=" syntax (though it can be restored via the >> 'dpdk-probe-at-init' config option, see ovs-vswitchd.conf.db(5)). >> + * Removed support for the experimental offload of the tnl_pop() action >> + that relied on DPDK's experimental API. >> - The following deprecated AVX512-specific features of the userspace >> datapath are now removed: >> * AVX512-optimized DPCLS lookups. >> diff --git a/lib/dpif-offload-dpdk-netdev.c b/lib/dpif-offload-dpdk-netdev.c >> index efe99065e..02acb53a0 100644 >> --- a/lib/dpif-offload-dpdk-netdev.c >> +++ b/lib/dpif-offload-dpdk-netdev.c >> @@ -28,7 +28,6 @@ >> #include "dpif-offload.h" >> #include "dpif-offload-dpdk-private.h" >> #include "netdev-provider.h" >> -#include "netdev-vport.h" >> #include "odp-util.h" >> #include "openvswitch/match.h" >> #include "openvswitch/vlog.h" >> @@ -72,7 +71,6 @@ struct pmd_data { >> */ >> struct ufid_to_rte_flow_data { >> struct cmap_node node; >> - struct cmap_node mark_node; > > Removal of this and the mark_to_rte_flow map related should not be > removed in the scope of this commit. It is not related neither to dpdk > experimental API nor to tnl_pop. > > This removal means partial offloads won't work anymore. If we really > want to remove that, we should remove it in another dedicated commit, as > well as the support in the offload layer, NEWS, documentation etc. > > IMO, also looking forward towards doca offloads that will require post > process API, we should keep that API for dpdk offloads.
Yeah, you're right. I got confused a little bit with the code. It seems the dpif-offload refactor conflated the two offloads and might have also broke the partial offload, as it seems like we're setting false into the post_process_api_supported on EOPNOTSUPP from the rte_flow_get_restore_info, and this disables all the partial offload as well. That's why I ended up removing the whole thing while following the dependencies. I'll see how we can fix that. Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
