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 <[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&dat >> a=02%7C01%7Croniba%40mellanox.com%7C90cd4c9ba95b4629884f08d6e8384c41 >> %7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636951725824527442&am >> p;sdata=n%2FVMz34ICfk0dvGOP77Ip4L008RRKdraRXMG%2FtFRM64%3D&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&dat >> a=02%7C01%7Croniba%40mellanox.com%7C90cd4c9ba95b4629884f08d6e8384c41 >> %7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636951725824527442&am >> p;sdata=pfTmeLEuMW1DyJck3T%2BypfDI7ZlZ7K2%2FO6810tis4PQ%3D&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
