[ovs-dev] [PATCH v5 0/7] netdev datapath: Partial action offload
Ilya Maximets
i.maximets at ovn.org
Fri Jul 10 12:11:47 UTC 2020
On 7/9/20 8:47 AM, Sriharsha Basavapatna via dev wrote:
> Hi,
>
> This patchset extends the "Partial HW acceleration" mode to offload a
> part of the action processing to HW, instead of offloading just lookup
> (MARK/RSS), for "vhost-user" ports. This is referred to as "Partial Action
> Offload". This mode does not require SRIOV/switchdev configuration. In this
> mode, forwarding (output) action is still performed by OVS-DPDK SW datapath.
Hi. I like the idea of egress offloading. It's interesting.
IIUC, HW will perform matching on egress packets and perform actions before
actually sending them. Is that right?
For the implementation I have a few concerns:
1. Why only vhost-user ports? I mean, egress offloading could be done
for any ingress port in case ingress port doesn't support full offloading.
Moreover, you could have classification offloading on ingress and actions
offloading on egress at the same time. This might be useful, for example,
if we have two diferent NICs that both supports offloading, but we have to
send packets between them. But, yes, that might be a way for further
improvement.
Regarding vhost-user, you're exposing too much of netdev internals to
datapath layer by checking for a specific netdev type. This is not
a good thing to do. Especially because egress offloading doesn't depend
on a type of ingress interface.
2. I'm worried about other offload providers like tinux-tc that doesn't know
anything that happens in dpif-netdev and will not work correctly if
dpif-netdev will try to use egress offloading on it.
I see that you're using netdev_dpdk_flow_api_supported() inside the
dpif-netdev, but that is the violation of netdev-offload abstraction layer.
I think, some more generic extension of netdev-offload interface required,
so all offload providers will be able to use this API. I mean, egress
offloading should be possible with TC. You don't need to add support for
this, but we should have a generic interface to utilize this support in
the future.
At least there should be something more generic like info.offload_direction
of some enumeration type like
enum netdev_offload_direction {
NETDEV_OFFLOAD_INGRESS,
NETDEV_OFFLOAD_EGRESS,
};
And each offload provider should decide by itself if it supports some type
of offloading or not.
netdev specific or specific to particular offload provider functions should
not be used in general outside of their own modules.
3. Some new identifiers for flow dumps to distinguish classification and actions
partial offloading needed.
4. Ingress partial actions offloading sounds interesting, but current
implementation forces datapath assistance for such actions. This will
significantly impact kernel datapath as all the vlan operations will be
sent back and forth between kernel and userspace. And I'm not sure if this
will even work correctly with current patches.
I didn't make a full review. These are just quick comments from the architectural
point of view.
Best regards, Ilya Maximets.
More information about the dev
mailing list