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://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", .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
