[ovs-dev] [PATCH v12 00/11] Add offload support for sFlow
cmi at nvidia.com
Tue Jun 8 08:08:17 UTC 2021
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
>>>> 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
>>> In general, I think, it might be good to have a separate dpif type,
>>> 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
>>> 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() ->
> 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
> What do you think?
Any comments about Eli's suggestion?
More information about the dev