On 10.06.2019 10:02, Roni Bar Yanai wrote:
> Hi Ilya,
> 
> See inline
> 
>> -----Original Message-----
>> From: Ilya Maximets <[email protected]>
>> Sent: Thursday, June 6, 2019 2:19 PM
>> To: Roni Bar Yanai <[email protected]>; Ophir Munk
>> <[email protected]>; Roi Dayan <[email protected]>; ovs-
>> [email protected]
>> Cc: Ian Stokes <[email protected]>; Flavio Leitner <[email protected]>; 
>> Kevin
>> Traynor <[email protected]>; Finn Christensen <[email protected]>; Ben
>> Pfaff <[email protected]>; Simon Horman <[email protected]>; Olga
>> Shern <[email protected]>; Asaf Penso <[email protected]>; Majd Dibbiny
>> <[email protected]>; Oz Shlomo <[email protected]>
>> Subject: Re: [PATCH v4 1/4] netdev: Dynamic per-port Flow API.
>>
>> 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.
> 
> Can you explain what exactly are the use cases?
> DPDK supports full offload when using port representors, it is just a matter 
> of integrating it into OVS.

Yeah. I know. I just meant that netdev-rte-offload doesn't support full
offload yet, while linux_tc supports it already. The use-case is the same:
running userspace datapath with port representors, but opening them via
netdev-linux (i.e. usual linux port representors). This should be possible
to do that. Right now there could be no much profit from this setup, but,
after netdev-afxdp integration, it should be possible to use xdp for port
representors. This should give full HW offloading (via linux_tc)  + good
performance for unmatched traffic (via AF_XDP) for OVS. This might be
interesting out-of-the-box solution for users who don't want to use DPDK
for some reasons.

Another use-case for linux_tc offload from the userspace datapath is the
optimization of packet flows between linux interfaces. Some setups like
OpenStack installations has various linux interfaces (tap, veth, eth) for
the management purposes. They handle dhcp, some kind of routing, monitoring
and administration. Using linux_tc offloading we could keep most of this
traffic inside the kernel without need to handle it by the OVS main thread
in userspace.

> 
>>
>> 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 <[email protected]>
>>>> Sent: Monday, June 3, 2019 6:30 PM
>>>> To: Ophir Munk <[email protected]>; Roi Dayan <[email protected]>;
>>>> [email protected]
>>>> Cc: Ian Stokes <[email protected]>; Flavio Leitner <[email protected]>;
>> Kevin
>>>> Traynor <[email protected]>; Roni Bar Yanai <[email protected]>;
>> Finn
>>>> Christensen <[email protected]>; Ben Pfaff <[email protected]>; Simon Horman
>>>> <[email protected]>; Olga Shern <[email protected]>; Asaf
>>>> Penso <[email protected]>; Majd Dibbiny <[email protected]>; Oz
>> Shlomo
>>>> <[email protected]>
>>>> 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 <[email protected]>
>>>>>>>> Sent: Wednesday, May 22, 2019 2:50 PM
>>>>>>>> To: Ophir Munk <[email protected]>; Roi Dayan
>>>>>>>> <[email protected]>; [email protected]
>>>>>>>> Cc: Ian Stokes <[email protected]>; Flavio Leitner 
>>>>>>>> <[email protected]>;
>>>>>>>> Kevin Traynor <[email protected]>; Roni Bar Yanai
>>>>>>>> <[email protected]>; Finn Christensen <[email protected]>; Ben
>> Pfaff
>>>>>>>> <[email protected]>; Simon Horman <[email protected]>; Olga
>>>>>>>> Shern <[email protected]>; Asaf Penso <[email protected]>; Majd
>>>>>>>> Dibbiny <[email protected]>
>>>>>>>> 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 <[email protected]>; ovs-
>>>>>>>> [email protected]
>>>>>>>>>>> Cc: Ian Stokes <[email protected]>; Flavio Leitner
>>>>>>>>>>> <[email protected]>; Ophir Munk <[email protected]>; Kevin
>>>>>>>> Traynor
>>>>>>>>>>> <[email protected]>; Roni Bar Yanai <[email protected]>;
>> Finn
>>>>>>>>>>> Christensen <[email protected]>; Ben Pfaff <[email protected]>; Simon
>>>>>>>> Horman
>>>>>>>>>>> <[email protected]>
>>>>>>>>>>> Subject: Re: [PATCH v4 1/4] netdev: Dynamic per-port Flow API.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Acked-by: Roi Dayan <[email protected]>
>>>>>>>>>>
>>>>>>>>>> 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.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to