On 5/25/2021 7:19 PM, Eli Britstein wrote:
On 5/19/2021 12:47 PM, Simon Horman wrote:
External email: Use caution opening links or attachments
On Tue, May 18, 2021 at 02:22:26PM +0200, Ilya Maximets wrote:
On 4/27/21 3:23 AM, Chris Mi wrote:
...
Hi Ilya,
The code according to your suggestion is ready. But during the
internal
code review, Eli Britstein thought flow_api is netdev based, but the
psample/sFlow offload is not netdev based, but dpif based. Maybe we
should introduce dpif-offload-provider and have a dpf_offload_api, so
it won't be related to a specific netdev, but to a dpif type.
Could I know what's your opinion about it?
This might be not bad idea in general. The problem is that if we'll
introduce this kind of API, we'll need to remove the usage of
netdev-offload API from all the layers higher than dpif implementation
including dpif.c and make all offloading go through
dpif-offload-provider.
In general, I think, it might be good to have a separate dpif type,
e.g.
dpif-offload that will use netdev-offload APIs internally and provide
offloading service. In this case, dpif_operate(), depending on offload
configuration, will try to install flows to dpif-offload first and
fallback to install flows to the main dpif implementation if offload
failed or if it was partial (i.e. classification offloading). This
way
we will delete all the offloading related code from dpif-netdev and
dpif-netlink and will have a unified offloading architecture. It's not
easy to implement and there will be a lot of issues that we'll need to
figure out how to fix (e.g. dpif-netdev performs upcalls by itself,
so it
will have to install resulted flows by calling a dpif_flow_put/operate
and not install flows directly to its local flow tables). Anyway, this
looks like a long run and will require rework of the whole offloading
architecture, so might be a separate thing to work on. Thoughts?
Hi Ilya, Hi Chris,
I think that this approach makes sense in terms of an evolution of
the internal offload APIs/architecture. And might lean itself to more
advanced options in future - such as layered dpif implementations for
some purpose.
I do also there will be quite a lot of implementation challenges
to overcome.
Hi Ilya, Simon,
This approach was initially thought of following the suggested design
to have:
handler thread ->
netdev_offload_recv() ->
netdev_offload_tc_recv() ->
nl_sock_recv()
The ofproto layer does not have any 'netdev' to pass, yet calling a
new netdev based API (in flow_api) netdev_offload_recv.
The initial approach was to pass const char *dpif_type, and in
netdev-offload-tc.c just take the first netdev of this dpif type, and
work on it.
However, this didn't fit right, so we thought to have this new layer,
but not as this layered design, but only to serve this case, at least
at first.
I also agree with Simon that there will be a lot of challenges.
How about we will have phases towards such a complete design, also to
better cope with those challenges?
At the first phase we will just introduce such a layer (struct
offload_api in struct dpif for example), with a
dpif-offload-provider.h to declare it.
There will be functions like dpif_offload_recv(), to be used for this
sFlow support.
At later phase, the further architecture refactoring (layering) can be
done.
What do you think?
Hi Ilya,
Any comments about Eli's suggestion?
Thanks,
Chris
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
