[ovs-dev] [PATCH v12 00/11] Add offload support for sFlow

Eli Britstein elibr at nvidia.com
Tue May 25 11:19:42 UTC 2021


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?

Thanks,

Eli

>
> Kind regards,
> Simon
>
>


More information about the dev mailing list