On Tue, Jun 4, 2024 at 2:19 PM Xavier Simonart <[email protected]> wrote:
> Hi Ales > > Thanks for the patch. > Hi Xavier, thank you for the review. > On Fri, May 31, 2024 at 2:52 PM Ales Musil <[email protected]> wrote: > >> Add missing sync calls to make sure that the flows are present and >> strip the statistics from the flows. >> >> Fixes: 3faadc76ad71 ("northd: Fix pmtud for non routed traffic.") >> Signed-off-by: Ales Musil <[email protected]> >> --- >> tests/ovn-controller.at | 42 +++++++++++++++++++++++------------------ >> 1 file changed, 24 insertions(+), 18 deletions(-) >> >> diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at >> index fc50b6ff8..3d3825cb8 100644 >> --- a/tests/ovn-controller.at >> +++ b/tests/ovn-controller.at >> @@ -3036,13 +3036,16 @@ check ovs-vsctl \ >> -- add-port br-int vif2 \ >> -- set Interface vif2 external_ids:iface-id=lsp2 >> >> +wait_for_ports_up >> +check ovn-nbctl --wait=hv sync >> + >> AT_CHECK([as hv1 ovs-ofctl dump-flows br-int >> table=OFTABLE_CT_ZONE_LOOKUP | \ >> - sed -e 's/cookie=0x.*, duration=.*, table/cookie=??, >> duration=??, table/' | \ >> sed -e >> 's/actions=load:0x.*->NXM_NX_REG13/actions=load:0x?->NXM_NX_REG13/' | \ >> - grep -v NXST_FLOW |sort], [0], [dnl >> - cookie=??, duration=??, table=OFTABLE_CT_ZONE_LOOKUP, n_packets=0, >> n_bytes=0, idle_age=0, priority=0 >> actions=resubmit(,OFTABLE_LOG_INGRESS_PIPELINE) >> - cookie=??, duration=??, table=OFTABLE_CT_ZONE_LOOKUP, n_packets=0, >> n_bytes=0, idle_age=0, priority=100,reg14=0x1,metadata=0x1 >> actions=load:0x?->NXM_NX_REG13[[0..15]],load:0x2->NXM_NX_REG11[[]],load:0x1->NXM_NX_REG12[[]],resubmit(,OFTABLE_LOG_INGRESS_PIPELINE) >> - cookie=??, duration=??, table=OFTABLE_CT_ZONE_LOOKUP, n_packets=0, >> n_bytes=0, idle_age=0, priority=100,reg14=0x2,metadata=0x1 >> actions=load:0x?->NXM_NX_REG13[[0..15]],load:0x2->NXM_NX_REG11[[]],load:0x1->NXM_NX_REG12[[]],resubmit(,OFTABLE_LOG_INGRESS_PIPELINE) >> + grep -v NXST_FLOW | \ >> + awk '{print $7, $8}' | sort], [0], [dnl >> +priority=0 actions=resubmit(,OFTABLE_LOG_INGRESS_PIPELINE) >> +priority=100,reg14=0x1,metadata=0x1 >> actions=load:0x?->NXM_NX_REG13[[0..15]],load:0x2->NXM_NX_REG11[[]],load:0x1->NXM_NX_REG12[[]],resubmit(,OFTABLE_LOG_INGRESS_PIPELINE) >> +priority=100,reg14=0x2,metadata=0x1 >> actions=load:0x?->NXM_NX_REG13[[0..15]],load:0x2->NXM_NX_REG11[[]],load:0x1->NXM_NX_REG12[[]],resubmit(,OFTABLE_LOG_INGRESS_PIPELINE) >> ]) >> >> In addition to your change, I have seen a different kind of failure for > this test: on some systems, the test always fails because ct_zones id are > different, and hence reg11 and reg12 are filled with different values. > I think that this happens depending on whether the system uses murmur or > crc for hashing. > Hence, I think we should also sed/replace the values loaded in reg11 and > reg12 as we do for reg13. > Same comment applies to the similar checks below. > I have such a system where tests are failing. So, if it's easier for you, > let me know if you'd like me to apply a further patch, or test your > changes, or provide access to the system. > It makes sense, I agree that we should replace all loads and the reg values, which makes me wonder if it isn't actually better to just count the flows instead WDYT? > > check ovn-nbctl lsp-add ls1 lsp3 \ >> @@ -3051,24 +3054,27 @@ check ovs-vsctl \ >> -- add-port br-int vif3 \ >> -- set Interface vif3 external_ids:iface-id=lsp3 >> >> +wait_for_ports_up >> +check ovn-nbctl --wait=hv sync >> + >> AT_CHECK([as hv1 ovs-ofctl dump-flows br-int >> table=OFTABLE_CT_ZONE_LOOKUP | \ >> - sed -e 's/cookie=0x.*, duration=.*, table/cookie=??, >> duration=??, table/' | \ >> sed -e >> 's/actions=load:0x.*->NXM_NX_REG13/actions=load:0x?->NXM_NX_REG13/' | \ >> - grep -v NXST_FLOW |sort], [0], [dnl >> - cookie=??, duration=??, table=OFTABLE_CT_ZONE_LOOKUP, n_packets=0, >> n_bytes=0, idle_age=0, priority=0 >> actions=resubmit(,OFTABLE_LOG_INGRESS_PIPELINE) >> - cookie=??, duration=??, table=OFTABLE_CT_ZONE_LOOKUP, n_packets=0, >> n_bytes=0, idle_age=0, priority=100,reg14=0x1,metadata=0x1 >> actions=load:0x?->NXM_NX_REG13[[0..15]],load:0x2->NXM_NX_REG11[[]],load:0x1->NXM_NX_REG12[[]],resubmit(,OFTABLE_LOG_INGRESS_PIPELINE) >> - cookie=??, duration=??, table=OFTABLE_CT_ZONE_LOOKUP, n_packets=0, >> n_bytes=0, idle_age=0, priority=100,reg14=0x2,metadata=0x1 >> actions=load:0x?->NXM_NX_REG13[[0..15]],load:0x2->NXM_NX_REG11[[]],load:0x1->NXM_NX_REG12[[]],resubmit(,OFTABLE_LOG_INGRESS_PIPELINE) >> - cookie=??, duration=??, table=OFTABLE_CT_ZONE_LOOKUP, n_packets=0, >> n_bytes=0, idle_age=0, priority=100,reg14=0x3,metadata=0x1 >> actions=load:0x?->NXM_NX_REG13[[0..15]],load:0x2->NXM_NX_REG11[[]],load:0x1->NXM_NX_REG12[[]],resubmit(,OFTABLE_LOG_INGRESS_PIPELINE) >> + grep -v NXST_FLOW | \ >> + awk '{print $7, $8}' | sort], [0], [dnl >> +priority=0 actions=resubmit(,OFTABLE_LOG_INGRESS_PIPELINE) >> +priority=100,reg14=0x1,metadata=0x1 >> actions=load:0x?->NXM_NX_REG13[[0..15]],load:0x2->NXM_NX_REG11[[]],load:0x1->NXM_NX_REG12[[]],resubmit(,OFTABLE_LOG_INGRESS_PIPELINE) >> +priority=100,reg14=0x2,metadata=0x1 >> actions=load:0x?->NXM_NX_REG13[[0..15]],load:0x2->NXM_NX_REG11[[]],load:0x1->NXM_NX_REG12[[]],resubmit(,OFTABLE_LOG_INGRESS_PIPELINE) >> +priority=100,reg14=0x3,metadata=0x1 >> actions=load:0x?->NXM_NX_REG13[[0..15]],load:0x2->NXM_NX_REG11[[]],load:0x1->NXM_NX_REG12[[]],resubmit(,OFTABLE_LOG_INGRESS_PIPELINE) >> ]) >> >> -check ovn-nbctl lsp-del lsp3 >> +check ovn-nbctl --wait=hv lsp-del lsp3 >> AT_CHECK([as hv1 ovs-ofctl dump-flows br-int >> table=OFTABLE_CT_ZONE_LOOKUP | \ >> - sed -e 's/cookie=0x.*, duration=.*, table/cookie=??, >> duration=??, table/' | \ >> - sed -e >> 's/actions=load:0x.*->NXM_NX_REG13/actions=load:0x?->NXM_NX_REG13/' | >> - grep -v NXST_FLOW |sort], [0], [dnl >> - cookie=??, duration=??, table=OFTABLE_CT_ZONE_LOOKUP, n_packets=0, >> n_bytes=0, idle_age=0, priority=0 >> actions=resubmit(,OFTABLE_LOG_INGRESS_PIPELINE) >> - cookie=??, duration=??, table=OFTABLE_CT_ZONE_LOOKUP, n_packets=0, >> n_bytes=0, idle_age=0, priority=100,reg14=0x1,metadata=0x1 >> actions=load:0x?->NXM_NX_REG13[[0..15]],load:0x2->NXM_NX_REG11[[]],load:0x1->NXM_NX_REG12[[]],resubmit(,OFTABLE_LOG_INGRESS_PIPELINE) >> - cookie=??, duration=??, table=OFTABLE_CT_ZONE_LOOKUP, n_packets=0, >> n_bytes=0, idle_age=0, priority=100,reg14=0x2,metadata=0x1 >> actions=load:0x?->NXM_NX_REG13[[0..15]],load:0x2->NXM_NX_REG11[[]],load:0x1->NXM_NX_REG12[[]],resubmit(,OFTABLE_LOG_INGRESS_PIPELINE) >> + sed -e >> 's/actions=load:0x.*->NXM_NX_REG13/actions=load:0x?->NXM_NX_REG13/' | \ >> + grep -v NXST_FLOW | \ >> + awk '{print $7, $8}' | sort], [0], [dnl >> +priority=0 actions=resubmit(,OFTABLE_LOG_INGRESS_PIPELINE) >> +priority=100,reg14=0x1,metadata=0x1 >> actions=load:0x?->NXM_NX_REG13[[0..15]],load:0x2->NXM_NX_REG11[[]],load:0x1->NXM_NX_REG12[[]],resubmit(,OFTABLE_LOG_INGRESS_PIPELINE) >> +priority=100,reg14=0x2,metadata=0x1 >> actions=load:0x?->NXM_NX_REG13[[0..15]],load:0x2->NXM_NX_REG11[[]],load:0x1->NXM_NX_REG12[[]],resubmit(,OFTABLE_LOG_INGRESS_PIPELINE) >> ]) >> >> OVN_CLEANUP([hv1]) >> -- >> 2.45.0 >> >> Besides the comment above, it looks good to me. > Thanks > Xavier > >> _______________________________________________ >> dev mailing list >> [email protected] >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> >> 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
