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

Ilya Maximets i.maximets at samsung.com
Thu Jun 6 11:19:05 UTC 2019


On 06.06.2019 13:38, Roni Bar Yanai wrote:
> Hi Ilya, 
> was curious myself. Mark & RSS is working  (didn't test with representors yet).

Hi. Thanks for testing!

> I've tested offload is supported on vport using !has_system_port, do you think failing in this test is enough to say that this is netdev bridge port?

I think it's OK for now. '!has_system_port' allows to say that it's not a
system vport and, since 'dummy' flow API doesn't support vports offloading,
we could be sure that only dpdk flow API could be used (if allowed).

> When I returned supported, "dpdk_put" was called as expected (after removing the disallow).
> 
> One thing I've encounters is while addin a tap to the dpdk bridge. I got this:
> 
> 2019-06-06T10:18:21.865Z|00055|netdev_offload_tc|INFO|probe tc: block offload is supported.
> 2019-06-06T10:18:21.866Z|00056|netdev_offload_tc|INFO|added ingress qdisc to veth0
> 2019-06-06T10:18:21.866Z|00057|netdev_offload|INFO|veth0: Assigned flow API 'linux_tc'.
> 2019-06-06T10:18:21.866Z|00058|bridge|INFO|bridge br-int: added interface veth0 on port 1
> 2019-06-06T10:18:21.867Z|00059|bridge|INFO|bridge br-phy: added interface br-phy on port 65534
> 
> Seems that we should block this as well.

I don't think that we should block this, because we should allow
'linux_tc' offloading for linux interfaces. For example, someone
could open two linux ports from the userspace datapath and expect
that flows could be offloaded to HW/tc layer. I agree that it will
not fully work in case of mixing port types within a single OVS
bridge, however this should be a valid case. Packets from linux to
DPDK ports and backwards will go through OVS, because such flows
could not be offloaded.

Curious fact is that working this way (opening physical ports via
netdev-linux) could be even faster than using DPDK ports in some
cases, because 'linux_tc' supports full HW offloading unlike DPDK.

We'll also have AF_XDP support some time soon which will give even
more benefits to linux interfaces (XDP bypasses tc layer, however
it might be possible to install flows to HW and handle unmatched
packets in userspace).

Best regards, Ilya Maximets.

> 
> BR,
> Roni
> 
>> -----Original Message-----
>> From: Ilya Maximets <i.maximets at samsung.com>
>> Sent: Monday, June 3, 2019 6:30 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>; Oz Shlomo
>> <ozsh at mellanox.com>
>> Subject: Re: [PATCH v4 1/4] netdev: Dynamic per-port Flow API.
>>
>> Hi Ophir.
>> I'm curious, what is the current status of your testing?
>>
>> I'd like to apply this series to not block further development.
>>
>> Best regards, Ilya Maximets.
>>
>> On 23.05.2019 16:54, Ilya Maximets wrote:
>>> 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://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork
>> .ozlabs.org%2Fproject%2Fopenvswitch%2Flist%2F%3Fseries%3D107534&amp;dat
>> a=02%7C01%7Croniba%40mellanox.com%7C90cd4c9ba95b4629884f08d6e8384c41
>> %7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636951725824527442&am
>> p;sdata=n%2FVMz34ICfk0dvGOP77Ip4L008RRKdraRXMG%2FtFRM64%3D&amp;re
>> served=0
>>>>>
>>>>> 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://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork
>> .ozlabs.org%2Fproject%2Fopenvswitch%2Flist%2F%3Fseries%3D107545&amp;dat
>> a=02%7C01%7Croniba%40mellanox.com%7C90cd4c9ba95b4629884f08d6e8384c41
>> %7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636951725824527442&am
>> p;sdata=pfTmeLEuMW1DyJck3T%2BypfDI7ZlZ7K2%2FO6810tis4PQ%3D&amp;reser
>> ved=0
>>>>>>>
>>>>>>>
>>>>>>> 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.


More information about the dev mailing list