Hi Ales On Tue, Jun 4, 2024 at 2:36 PM Ales Musil <[email protected]> wrote:
> > > 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? > In fact, I was wondering about almost the opposite (:-)) - we could query the ct-zone-list to find out the zone_id and check whether the flows are exactly what we expect. But then I found that to be really overkill. So, I think that checking the flows (w/ zone-id hidden) seemed a correct trade-off. Also, it makes the test easier to understand (we know what we expect), and make debugging easier if the test fails. 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 > Thanks Xavier > -- > > 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
