On Thu 04 Aug 2022 at 08:52, Simon Horman <simon.hor...@corigine.com> wrote: > On Thu, Aug 04, 2022 at 09:33:15AM +0200, Ilya Maximets wrote: >> On 8/4/22 09:18, Simon Horman wrote: >> > On Wed, Aug 03, 2022 at 12:32:46PM +0200, Vlad Buslov wrote: >> >> Referenced commit changed policer action type from TC_ACT_UNSPEC >> >> (continue) >> >> to TC_ACT_PIPE. However, since neither TC hardware offload layer nor mlx5 >> >> driver at the time validated action type and always assumed 'continue', >> >> the >> >> breakage wasn't caught until later validation code was added. The change >> >> also broke valid configuration when sending from offload-capable device to >> >> non-offload capable. For example, when sending from mlx5 VF to OvS bridge >> >> netdevice the traffic that passed matchall classifier with policer could >> >> no >> >> longer match the following flower rule in software: >> > >> > ... >> > >> >> Notes: >> >> Changes V2 -> V3: >> >> >> >> - Refactored the fix to set action TC_ACT_UNSPEC only for >> >> matchall/basic >> >> classifiers. >> >> >> >> Changes V1 -> V2: >> >> >> >> - Rebase on latest master. >> > >> > ... >> > >> > Hi Vlad, >> > >> > thanks for addressing our concerns. We have reviewed and tested this patch >> > and do not observe any problems for our use-cases: in particular mirred >> > after meter (police). >> > >> > Acked-by: Simon Horman <simon.hor...@corigine.com> >> > >> > I'd also be happy to apply this to the upstream tree >> > if there are no objections from others. >> >> No objections from my side. >> >> Regarding backports, AFAICT, we'll need that change down to 2.16, right? >> Can code from v1 be used on 2.17 and 2.16 or do we need a separate backport >> patch? > > I think that the complexity that v3 addresses arises from metering support > which allows other actions to follow a police action (meter). > > So, assuming metering is not present in 2.17 and 2.16, and given that it > turns out that UNSPEC is fine for both PPS and BPS, then I suspect we > can go for a simple backport-patch which simply changes TC_ACT_PIPE > to TC_ACT_UNSPEC in nl_msg_act_police_end_nest(). > > This would need some testing, IMHO, and I may end up eating the words > above.
Looking at the code it seem indeed to be the case. Sending the fix for 2.17 branch. _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
