| Message ID | 1599033403-1659-1-git-send-email-dceara@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] system-ovn.at: Make ECMP test case more resilient. | expand |
Thanks for the fix, Dumitru. Acked-by: Mark Michelson <mmichels@redhat.com> On 9/2/20 3:56 AM, Dumitru Ceara wrote: > It's not easy to predict how OVS datapath flows exactly look so it's > better to avoid matching on exact ct_label masks. > > E.g., on some runs of the "ECMP symmetric reply" test the datapath > flows are installed as: > ct_state(-new+est+rpl+trk).*ct_label(0x.*00000401020400000000/0xffffffffffffffff00000000) > > On other runs the flows are installed as: > ct_state(-new+est+rpl+trk).*ct_label(0x.*00000401020400000000/0xffffffffffffffff00000001) > > Both versions are equivalent but the test only accepts the first one. > > Fixes: 4fdca656857d ("Add ECMP symmetric replies.") > Signed-off-by: Dumitru Ceara <dceara@redhat.com> > --- > tests/system-ovn.at | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/tests/system-ovn.at b/tests/system-ovn.at > index 40ba6e4..81f9332 100644 > --- a/tests/system-ovn.at > +++ b/tests/system-ovn.at > @@ -5235,10 +5235,10 @@ icmp,orig=(src=172.16.0.1,dst=10.0.0.2,id=<cleared>,type=8,code=0),reply=(src=10 > # Ensure datapaths show conntrack states as expected > # Like with conntrack entries, we shouldn't try to predict > # port binding tunnel keys. So omit them from expected labels. > -AT_CHECK([ovs-appctl dpctl/dump-flows | grep 'ct_state(+new-est-rpl+trk).*ct(.*label=0x.*00000401020400000000/0xffffffffffffffff00000000)' -c], [0], [dnl > +AT_CHECK([ovs-appctl dpctl/dump-flows | grep 'ct_state(+new-est-rpl+trk).*ct(.*label=0x.*00000401020400000000/.*)' -c], [0], [dnl > 1 > ]) > -AT_CHECK([ovs-appctl dpctl/dump-flows | grep 'ct_state(-new+est+rpl+trk).*ct_label(0x.*00000401020400000000/0xffffffffffffffff00000000)' -c], [0], [dnl > +AT_CHECK([ovs-appctl dpctl/dump-flows | grep 'ct_state(-new+est+rpl+trk).*ct_label(0x.*00000401020400000000/.*)' -c], [0], [dnl > 1 > ]) > >
On Wed, Sep 2, 2020 at 5:38 PM Mark Michelson <mmichels@redhat.com> wrote: > Thanks for the fix, Dumitru. > > Acked-by: Mark Michelson <mmichels@redhat.com> > > Thanks Dumitru and Mark. I applied this patch to master. Numan > On 9/2/20 3:56 AM, Dumitru Ceara wrote: > > It's not easy to predict how OVS datapath flows exactly look so it's > > better to avoid matching on exact ct_label masks. > > > > E.g., on some runs of the "ECMP symmetric reply" test the datapath > > flows are installed as: > > > ct_state(-new+est+rpl+trk).*ct_label(0x.*00000401020400000000/0xffffffffffffffff00000000) > > > > On other runs the flows are installed as: > > > ct_state(-new+est+rpl+trk).*ct_label(0x.*00000401020400000000/0xffffffffffffffff00000001) > > > > Both versions are equivalent but the test only accepts the first one. > > > > Fixes: 4fdca656857d ("Add ECMP symmetric replies.") > > Signed-off-by: Dumitru Ceara <dceara@redhat.com> > > --- > > tests/system-ovn.at | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/tests/system-ovn.at b/tests/system-ovn.at > > index 40ba6e4..81f9332 100644 > > --- a/tests/system-ovn.at > > +++ b/tests/system-ovn.at > > @@ -5235,10 +5235,10 @@ > icmp,orig=(src=172.16.0.1,dst=10.0.0.2,id=<cleared>,type=8,code=0),reply=(src=10 > > # Ensure datapaths show conntrack states as expected > > # Like with conntrack entries, we shouldn't try to predict > > # port binding tunnel keys. So omit them from expected labels. > > -AT_CHECK([ovs-appctl dpctl/dump-flows | grep > 'ct_state(+new-est-rpl+trk).*ct(.*label=0x.*00000401020400000000/0xffffffffffffffff00000000)' > -c], [0], [dnl > > +AT_CHECK([ovs-appctl dpctl/dump-flows | grep > 'ct_state(+new-est-rpl+trk).*ct(.*label=0x.*00000401020400000000/.*)' -c], > [0], [dnl > > 1 > > ]) > > -AT_CHECK([ovs-appctl dpctl/dump-flows | grep > 'ct_state(-new+est+rpl+trk).*ct_label(0x.*00000401020400000000/0xffffffffffffffff00000000)' > -c], [0], [dnl > > +AT_CHECK([ovs-appctl dpctl/dump-flows | grep > 'ct_state(-new+est+rpl+trk).*ct_label(0x.*00000401020400000000/.*)' -c], > [0], [dnl > > 1 > > ]) > > > > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >
On 9/3/20 10:46 AM, Numan Siddique wrote: > > > On Wed, Sep 2, 2020 at 5:38 PM Mark Michelson <mmichels@redhat.com > <mailto:mmichels@redhat.com>> wrote: > > Thanks for the fix, Dumitru. > > Acked-by: Mark Michelson <mmichels@redhat.com > <mailto:mmichels@redhat.com>> > > > Thanks Dumitru and Mark. I applied this patch to master. > > Numan > Thanks!
diff --git a/tests/system-ovn.at b/tests/system-ovn.at index 40ba6e4..81f9332 100644 --- a/tests/system-ovn.at +++ b/tests/system-ovn.at @@ -5235,10 +5235,10 @@ icmp,orig=(src=172.16.0.1,dst=10.0.0.2,id=<cleared>,type=8,code=0),reply=(src=10 # Ensure datapaths show conntrack states as expected # Like with conntrack entries, we shouldn't try to predict # port binding tunnel keys. So omit them from expected labels. -AT_CHECK([ovs-appctl dpctl/dump-flows | grep 'ct_state(+new-est-rpl+trk).*ct(.*label=0x.*00000401020400000000/0xffffffffffffffff00000000)' -c], [0], [dnl +AT_CHECK([ovs-appctl dpctl/dump-flows | grep 'ct_state(+new-est-rpl+trk).*ct(.*label=0x.*00000401020400000000/.*)' -c], [0], [dnl 1 ]) -AT_CHECK([ovs-appctl dpctl/dump-flows | grep 'ct_state(-new+est+rpl+trk).*ct_label(0x.*00000401020400000000/0xffffffffffffffff00000000)' -c], [0], [dnl +AT_CHECK([ovs-appctl dpctl/dump-flows | grep 'ct_state(-new+est+rpl+trk).*ct_label(0x.*00000401020400000000/.*)' -c], [0], [dnl 1 ])
It's not easy to predict how OVS datapath flows exactly look so it's better to avoid matching on exact ct_label masks. E.g., on some runs of the "ECMP symmetric reply" test the datapath flows are installed as: ct_state(-new+est+rpl+trk).*ct_label(0x.*00000401020400000000/0xffffffffffffffff00000000) On other runs the flows are installed as: ct_state(-new+est+rpl+trk).*ct_label(0x.*00000401020400000000/0xffffffffffffffff00000001) Both versions are equivalent but the test only accepts the first one. Fixes: 4fdca656857d ("Add ECMP symmetric replies.") Signed-off-by: Dumitru Ceara <dceara@redhat.com> --- tests/system-ovn.at | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)