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

Chris Mi 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 
>>>> 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


More information about the dev mailing list