Hi Ilya, See inline
>-----Original Message----- >From: Ilya Maximets <i.maxim...@samsung.com> >Sent: Thursday, June 6, 2019 2:19 PM >To: Roni Bar Yanai <ron...@mellanox.com>; Ophir Munk ><ophi...@mellanox.com>; Roi Dayan <r...@mellanox.com>; ovs- >d...@openvswitch.org >Cc: Ian Stokes <ian.sto...@intel.com>; Flavio Leitner <f...@sysclose.org>; >Kevin >Traynor <ktray...@redhat.com>; Finn Christensen <f...@napatech.com>; Ben >Pfaff <b...@ovn.org>; Simon Horman <simon.hor...@netronome.com>; Olga >Shern <ol...@mellanox.com>; Asaf Penso <as...@mellanox.com>; Majd Dibbiny ><m...@mellanox.com>; Oz Shlomo <o...@mellanox.com> >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. > >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.maxim...@samsung.com> >>> Sent: Monday, June 3, 2019 6:30 PM >>> To: Ophir Munk <ophi...@mellanox.com>; Roi Dayan <r...@mellanox.com>; >>> ovs-dev@openvswitch.org >>> Cc: Ian Stokes <ian.sto...@intel.com>; Flavio Leitner <f...@sysclose.org>; >Kevin >>> Traynor <ktray...@redhat.com>; Roni Bar Yanai <ron...@mellanox.com>; >Finn >>> Christensen <f...@napatech.com>; Ben Pfaff <b...@ovn.org>; Simon Horman >>> <simon.hor...@netronome.com>; Olga Shern <ol...@mellanox.com>; Asaf >>> Penso <as...@mellanox.com>; Majd Dibbiny <m...@mellanox.com>; Oz >Shlomo >>> <o...@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.maxim...@samsung.com> >>>>>>> Sent: Wednesday, May 22, 2019 2:50 PM >>>>>>> To: Ophir Munk <ophi...@mellanox.com>; Roi Dayan >>>>>>> <r...@mellanox.com>; ovs-dev@openvswitch.org >>>>>>> Cc: Ian Stokes <ian.sto...@intel.com>; Flavio Leitner >>>>>>> <f...@sysclose.org>; >>>>>>> Kevin Traynor <ktray...@redhat.com>; Roni Bar Yanai >>>>>>> <ron...@mellanox.com>; Finn Christensen <f...@napatech.com>; Ben >Pfaff >>>>>>> <b...@ovn.org>; Simon Horman <simon.hor...@netronome.com>; Olga >>>>>>> Shern <ol...@mellanox.com>; Asaf Penso <as...@mellanox.com>; Majd >>>>>>> Dibbiny <m...@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.maxim...@samsung.com>; ovs- >>>>>>> d...@openvswitch.org >>>>>>>>>> Cc: Ian Stokes <ian.sto...@intel.com>; Flavio Leitner >>>>>>>>>> <f...@sysclose.org>; Ophir Munk <ophi...@mellanox.com>; Kevin >>>>>>> Traynor >>>>>>>>>> <ktray...@redhat.com>; Roni Bar Yanai <ron...@mellanox.com>; >Finn >>>>>>>>>> Christensen <f...@napatech.com>; Ben Pfaff <b...@ovn.org>; Simon >>>>>>> Horman >>>>>>>>>> <simon.hor...@netronome.com> >>>>>>>>>> Subject: Re: [PATCH v4 1/4] netdev: Dynamic per-port Flow API. >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> Acked-by: Roi Dayan <r...@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&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 d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev