> -----Original Message----- > From: Ilya Maximets <i.maxim...@ovn.org> > Sent: Tuesday, February 25, 2020 6:31 PM > To: Yanqin Wei <yanqin....@arm.com>; Ilya Maximets > <i.maxim...@ovn.org>; d...@openvswitch.org > Cc: nd <n...@arm.com>; Lijian Zhang <lijian.zh...@arm.com>; Gavin Hu > <gavin...@arm.com> > Subject: Re: [ovs-dev] [PATCH v1] netdev: fix partial offloading test cases > failure > > On 2/25/20 10:59 AM, Yanqin Wei wrote: > > Hi Ilya, > > > > > >> -----Original Message----- > >> From: Ilya Maximets <i.maxim...@ovn.org> > >> Sent: Tuesday, February 25, 2020 5:25 PM > >> To: Yanqin Wei <yanqin....@arm.com>; d...@openvswitch.org > >> Cc: nd <n...@arm.com>; Lijian Zhang <lijian.zh...@arm.com>; Gavin Hu > >> <gavin...@arm.com>; i.maxim...@ovn.org > >> Subject: Re: [ovs-dev] [PATCH v1] netdev: fix partial offloading test > >> cases failure > >> > >> On 2/25/20 2:46 AM, Yanqin Wei wrote: > >>> Some partial offloading test cases are failing inconsistently. The > >>> root cause is that dummy netdev is assigned with incorrect > >>> offloading flow > >> API. > >>> dpif-netdev - partial hw offload - dummy dpif-netdev - partial hw > >>> offload - dummy-pmd dpif-netdev - partial hw offload with packet > >>> modifications - dummy dpif-netdev - partial hw offload with packet > >>> modifications - dummy-pmd > >>> > >>> This patch fixes this issue by adding a specified flow api type in netdev. > >>> Dummy netdev class can specify flow type in construct function. All > >>> of the above cases can pass consistently. > >> > >> Could you, please, clarify which offload provider is assigned to > >> dummy ports in your case and why this happens? > >> > >> In general, we need to fix offload providers to only accept ports > >> that are usable for them instead of hardcoding the type. > >> > > [Yanqin] Sometimes " linux_tc" is assigned to dummy netdev by mistake. > Currently, dpif traverses all offloading provider and select the first > provider > with successful initialization. It makes the result uncertain and random. > > I think the problem is who should take responsible for offloading > > providers assignment. Both dpif and netdev class impact the provider > selection Firstly, netdev may specify offloading provider if the option is > unique. > Secondly, dpif should determine if netdev has more than one option (instead > of traverse all providers). > > This patch implement the first one. The second one could be implemented in > netdev_init_flow_api by means of dpif class and netdev class. > > What do you think of this? > > I think that current implementation of dynamic flow API assignment is > somewhat OK and there is no significant issues that we should address. > > For this particular issue, I think we just need to be a little bit more > careful with > tests. I believe that removing of 'options:ifindex=1' from the test along > with > applying of the following patch: > https://patchwork.ozlabs.org/patch/1226013/ > should fix the occasional assignment of tinux_tc flow API for dummy port. > > For now, as a workaround, to fix this particular case we could change > 'options:ifindex=1' to some fairly big value instead of using '1', which is > likely > used by some of the existing system interfaces. > [Yanqin] Using ifindex to determine offloading provider is a little tricky. If more netdev need support offloading (which have valid ifindex but not support tc offloading), the logic needs to change. But as you said, there is no significant issue so far. We could use the workaround you suggest and refine the provider assignment when required. I will update v2 based on this workaround.
> Best regards, Ilya Maximets. _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
