On 10/07/2026 18:29, Ilya Maximets wrote:
External email: Use caution opening links or attachments
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.
The experimental and the performance considerations are not the same.
It happens to be that the only usage
OVS currently have (/had) of experimental API involves this performance
issue, but it could be (as in netdev-doca series)
that experimental API doesn't involve any performance issue.
It's fine to decide as a policy that we don't want experimental API, but
it doesn't necessarily mean a performance degradation.
I think the "The reason" sentence should be rephrased a bit.
Other than that, LGTM.
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.
I doubt this API will ever become stable.
The first approach we sent at the time for decap path didn't require it,
but it was rejected by the community, thus introducing this API.
AFAIK the only user for this API is OVS. With this gone, there wouldn't
be a reason to keep it in dpdk. I guess it will stay as a stale code
until someone will do a cleanup, like recently done in OVS.
With this code removed, there are no more offload providers that
implements post processing and can return EOPNOTSUPP on a per-device
basis. However, calling into a separate translation unit, getting
RCU-protected offload provider and checking if the method exists is
still expensive if done per packet. Simple vhost-to-vhost forwarding
test with offload enabled (but not supported, obviously) shows 1-1.5%
performance degradation. So, the early check of the
'post_process_api_supported' before calling into dpif_offload module
is still needed.
Doca offloads will need it.
Signed-off-by: Ilya Maximets<[email protected]>
---
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
