[ovs-dev] [PATCH v4 1/4] netdev: Dynamic per-port Flow API.

Thu May 23 13:54:28 UTC 2019


On 23.05.2019 16:53, Ilya Maximets wrote:
> On 23.05.2019 16:18, Ophir Munk wrote:
>>
>>
>>> -----Original Message-----
>>> From: Ilya Maximets <i.maximets at samsung.com>
>>> Sent: Wednesday, May 22, 2019 2:50 PM
>>> To: Ophir Munk <ophirmu at mellanox.com>; Roi Dayan
>>> <roid at mellanox.com>; ovs-dev at openvswitch.org
>>> Cc: Ian Stokes <ian.stokes at intel.com>; Flavio Leitner <fbl at sysclose.org>;
>>> Kevin Traynor <ktraynor at redhat.com>; Roni Bar Yanai
>>> <roniba at mellanox.com>; Finn Christensen <fc at napatech.com>; Ben Pfaff
>>> <blp at ovn.org>; Simon Horman <simon.horman at netronome.com>; Olga
>>> Shern <olgas at mellanox.com>; Asaf Penso <asafp at mellanox.com>; Majd
>>> Dibbiny <majd at mellanox.com>
>>> Subject: Re: [PATCH v4 1/4] netdev: Dynamic per-port Flow API.
>>>
>>>
>>>
>>> On 22.05.2019 13:15, Ilya Maximets wrote:
>>>> On 22.05.2019 1:12, Ophir Munk wrote:
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Roi Dayan
>>>>>> Sent: Tuesday, May 21, 2019 7:48 PM
>>>>>> To: Ilya Maximets <i.maximets at samsung.com>; ovs-
>>> dev at openvswitch.org
>>>>>> Cc: Ian Stokes <ian.stokes at intel.com>; Flavio Leitner
>>>>>> <fbl at sysclose.org>; Ophir Munk <ophirmu at mellanox.com>; Kevin
>>> Traynor
>>>>>> <ktraynor at redhat.com>; Roni Bar Yanai <roniba at mellanox.com>; Finn
>>>>>> Christensen <fc at napatech.com>; Ben Pfaff <blp at ovn.org>; Simon
>>> Horman
>>>>>> <simon.horman at netronome.com>
>>>>>> Subject: Re: [PATCH v4 1/4] netdev: Dynamic per-port Flow API.
>>>>>>
>>>>>>
>>>>>> Acked-by: Roi Dayan <roid at mellanox.com>
>>>>>
>>>>> Hi Ilya,
>>>>> Can you please send a patch for the detection of netdev vport on top of
>>> this series (as you have already started suggesting in ML discussions)?
>>>>> I will then test it and will make sure it's applicable with this series. I think it
>>> is better to do that before series acceptance.
>>>>> What do you think?
>>>>
>>>> Hi.
>>>> Actually patches are already on a list. You only need to add few lines
>>>> to make them allow vxlan for netdev-offload-dpdk.
>>>>
>>>> Apply following patch sets on top of this one:
>>>>
>>>>  https://patchwork.ozlabs.org/project/openvswitch/list/?series=107534
>>
>> FYI - applying this patch succeeded, however applying the next patch failed unless I applied 
>> patches 2/4, 3/4/ 4/4 first and only then I applied the next patch.
> 
> Yes. That is expected.
> 
>>
>>>>  https://patchwork.ozlabs.org/project/openvswitch/list/?series=107545
>>>>
>>>>
>>>> Change below should than allow you to use dpdk offloading for vxlan ports:
>>
>> Why do you want to use dpdk offloading for vxlan ports?
> 
> Sorry for misunderstanding, but I thought that you're implementing
> vxlan offloading as part of dpdk offloading. If it'll be a separate
> module, it's even better.
> 
>> We need to use vport-netdev offloading for vxlan-netdev ports.
>> We need to use dpdk offloading for dpdk ports.
>> Vxlan-netdev offloading and dpdk offloading have a different implementation
>> (unlike the system case where vxlan-system offloading and system offloading are identical).
>>
>> I see four required offloading APIs:
>> 1. system
>> 2. dpdk
>> 3. vport under system (currently it is identical to system API)
>> 4. New vport under netdev.
>>
>> The first three APIs exist. The last (vxlan-netdev) will be sent soon. 
>>
>> I see two options for adding vxlan-netdev API.
>> 1. Create a new dedicated vport-netdev offload class. 
>>
>> 2. Having vport-netdev API to be identical to dpdk API but since the implementations are different we will have to know the type ("dpdk" versus "vxlan"). 
>> In pseudo code:
>> If (type=="dpdk")
>> {
>>    // handle dpdk offloading
>> }
>> If (type=="vxlan")
>> {
>>    // handle vxlan offloading
>> }
>>
>> I prefer the first option.
> 
> Yes. Sure. I alse prefer the separate class if it has separate implementation
> anyway.
> 
> 
> So, with all above patches applied you just need to make a new file:
> netdev-offload-vport-dpdk.c:
> 
> <...>
> static int netdev_offload_vport_dpdk_flow_put(...)
> {
>     ...
> }
> 
> static int netdev_offload_vport_dpdk_flow_del(...)
> {
>     ...
> }
> 
> static int
> netdev_offload_vport_dpdk_init_flow_api(struct netdev *netdev)
> {
>     if (netdev_vport_is_vport_class(netdev->netdev_class)
>         && netdev_vport_has_system_port(netdev)) {
>         VLOG_DBG("%s: vport has backing system interface. Skipping.",
>                  netdev_get_name(netdev));
>         return EOPNOTSUPP;
>     }
>     return strcmp(netdev_get_type(netdev), "vxlan");
> }
> 
> const struct netdev_flow_api netdev_offload_vport_dpdk = {
>     .type = "dpdk_flow_api",

s/type = "dpdk_flow_api"/type = "vport_dpdk_flow_api"/

>     .flow_put = netdev_offload_vport_dpdk_flow_put,
>     .flow_del = netdev_offload_vport_dpdk_flow_del,
>     .init_flow_api = netdev_offload_vport_dpdk_init_flow_api,
> };
> 
> 
> And add following line to lib/dpdk.c:
> 
> netdev_register_flow_api_provider(&netdev_offload_vport_dpdk);
> 
> 
> And following to lib/netdev-offload-provider.h:
> 
> extern const struct netdev_flow_api netdev_offload_vport_dpdk;
> 
>>
>>>> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
>>>> index b7b0616ec..32f23c401 100644
>>>> --- a/lib/netdev-offload-dpdk.c
>>>> +++ b/lib/netdev-offload-dpdk.c
>>>> @@ -760,6 +760,10 @@ netdev_offload_dpdk_init_flow_api(struct netdev
>>> *netdev)
>>>>          return EOPNOTSUPP;
>>>>      }
>>>>
>>>> +    if (!strcmp(netdev_get_name(netdev), "vxlan")) {
>>>
>>> Sorry,
>>> s/netdev_get_name/netdev_get_type/
>>>
>>>> +        return 0;
>>
>> Having said all the above - we still need a way to correctly select between vport-netdev API versus vport-system API.
>> Reading your suggestion I am still not sure we have a solution here. Say we have a system bridge and a netdev bridge both with a vxlan port.
>> When the vxlan-netdev is checked first by the system-init API it will pass the checking and it will be added as a vxlan-system. Right?
> 
> No. See the 'netdev_vport_has_system_port' function from patch
> "[RFC 2/2] netdev-offload: Disallow offloading to unrelated vports.".
> 
>> Can you please advise?
> 
> See the code snippet above.
> 
> Best regards, Ilya Maximets.
> 
>