On Fri, Feb 9, 2024 at 5:51 PM Mark Michelson <[email protected]> wrote:

> Hi Ales,
>
> I have one comment below, but it's small enough it can be fixed during
> merge.
>
> Acked-by: Mark Michelson <[email protected]>
>
> On 2/8/24 13:17, Ales Musil wrote:
> > There were some comments left with hardcoded numbers. Even if it
> > wouldn't break any test table shift/change it would just left the
> > comment outdated.
> >
> > Signed-off-by: Ales Musil <[email protected]>
> > ---
> >   tests/ovn-northd.at | 2 +-
> >   tests/ovn.at        | 8 ++++----
> >   2 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> > index 591ad5aad..bb5e1f958 100644
> > --- a/tests/ovn-northd.at
> > +++ b/tests/ovn-northd.at
> > @@ -2150,7 +2150,7 @@ AT_CLEANUP
> >
> >   # This test case tests that when a logical switch has load balancers
> associated
> >   # (with VIPs configured), the below logical flow is added by
> ovn-northd.
> > -# table=1 (ls_out_pre_lb      ), priority=100  , match=(ip),
> action=(reg0[[0]] = 1; next;)
> > +# (ls_out_pre_lb      ), priority=100  , match=(ip), action=(reg0[[0]]
> = 1; next;)
>
> The other changes in this patch are formatted differently than this one.
> If this were modeled after the other changes, it would be
>
> # table=ls_out_pr_lb, priority=100...
>
> Should this comment be formatted the same as the others in this change?
>

We can follow the same "convention" as for the others.


> >   # This test case is added for the BZ -
> >   # https://bugzilla.redhat.com/show_bug.cgi?id=1849162
> >   #
> > diff --git a/tests/ovn.at b/tests/ovn.at
> > index 2cf335cf4..0af60f893 100644
> > --- a/tests/ovn.at
> > +++ b/tests/ovn.at
> > @@ -18372,8 +18372,8 @@ AT_CHECK([cat 2.packets], [0], [expout])
> >
> >   # There should be total of 9 flows present with conjunction action and
> 2 flows
> >   # with conj match. Eg.
> > -# table=44, priority=2001,conj_id=2,metadata=0x1 actions=resubmit(,45)
> > -# table=44, priority=2001,conj_id=3,metadata=0x1 actions=drop
> > +# table=ls_out_acl_eval, priority=2001,conj_id=2,metadata=0x1
> actions=resubmit(,ls_out_acl_action)
> > +# table=ls_out_acl_eval, priority=2001,conj_id=3,metadata=0x1
> actions=drop
> >   # priority=2001,ip,metadata=0x1,nw_dst=10.0.0.6
> actions=conjunction(2,2/2)
> >   # priority=2001,ip,metadata=0x1,nw_dst=10.0.0.4
> actions=conjunction(2,2/2)
> >   # priority=2001,ip,metadata=0x1,nw_dst=10.0.0.5
> actions=conjunction(2,2/2)
> > @@ -18413,7 +18413,7 @@ AT_CHECK([cat 2.packets], [0], [])
> >   # properly.
> >   # There should be total of 6 flows present with conjunction action and
> 1 flow
> >   # with conj match. Eg.
> > -# table=44, priority=2001,conj_id=3,metadata=0x1 actions=drop
> > +# table=ls_out_acl_eval, priority=2001,conj_id=3,metadata=0x1
> actions=drop
> >   # priority=2001,ip,metadata=0x1,nw_dst=10.0.0.7
> actions=conjunction(4,2/2)
> >   # priority=2001,ip,metadata=0x1,nw_dst=10.0.0.9
> actions=conjunction(4,2/2)
> >   # priority=2001,ip,metadata=0x1,nw_dst=10.0.0.8
> actions=conjunction(4,2/2)
> > @@ -34296,7 +34296,7 @@ in_port_sec=OFTABLE_CHK_IN_PORT_SEC
> >   in_port_sec_nd=OFTABLE_CHK_IN_PORT_SEC_ND
> >   out_port_sec=OFTABLE_CHK_OUT_PORT_SEC
> >
> > -# There should be no flows in table OFTABLE_CHK_IN_PORT_SEC, 74 and 75
> in hv1 and hv2
> > +# There should be no flows in table OFTABLE_CHK_IN_PORT_SEC,
> OFTABLE_CHK_IN_PORT_SEC_ND and OFTABLE_CHK_OUT_PORT_SEC in hv1 and hv2
> >   > hv1_t${in_port_sec}_flows.expected
> >   > hv1_t${in_port_sec_nd}_flows.expected
> >   > hv1_t${out_port_sec}_flows.expected
>
>
Thanks,
Ales

-- 

Ales Musil

Senior Software Engineer - OVN Core

Red Hat EMEA <https://www.redhat.com>

[email protected]
<https://red.ht/sig>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to