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