On Mon, Feb 27, 2023 at 04:44:02PM +0100, Ilya Maximets wrote:
> On 2/24/23 14:01, Simon Horman wrote:
> > Move check for tc ingress pps support to from aclocal to test script
> > 
> > This has several problems:
> > 
> > 1. Stderror from failing commands is output when executing
> >    various make targets.
> > 
> > 2. There are various failure conditions that lead
> >    to veth0 and veth1 being created by not cleaned up.
> > 
> > 3. The check seems to execute for many make targets.
> >    And it attempts to temporarily modify system state.
> >    This seems inappropriate.
> > 
> > 4. veth0 and veth1 seem far too generic and could easily
> >    conflict with other parts of the system.
> > 
> > All these problems are addressed by this patch.
> > 
> > Signed-off-by: Simon Horman <[email protected]>
> > Reviewed-by: Louis Peens <[email protected]>

...

> > diff --git a/tests/system-offloads-traffic.at 
> > b/tests/system-offloads-traffic.at
> > index f2bf9c0639aa..1d21da5f196d 100644
> > --- a/tests/system-offloads-traffic.at
> > +++ b/tests/system-offloads-traffic.at
> > @@ -18,6 +18,16 @@ m4_define([OVS_CHECK_ACTIONS], [
> >             [0], [$1])
> >  ])
> >  
> > +m4_define([CHECK_TC_INGRESS_PPS],
> > +[
> > +    AT_SKIP_IF([test $HAVE_TC = "no"])
> > +    AT_CHECK([ip link add ovs_tc_pps0 type veth peer name ovs_tc_pps1 || 
> > dnl
> > +                  exit 77])
> 
> I'd break the line before '||', but that's not very important.
> 
> And a capital letter and a period in the subject line might be
> good to add just for consistency.
> 
> In any case, the change seems correct to me:
> 
> Acked-by: Ilya Maximets <[email protected]>

Thanks. I fixed up the line-break and patch subject when applying.

* 5f0fdf5e2c2e ("test: Move check for tc ingress pps support to test script.")
  - https://github.com/openvswitch/ovs/commit/5f0fdf5e2c2e

I did not backport this as it doesn't seem to be a fix to me.
But I'm happy to do so if you feel otherwise.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to