On 5/23/22 20:43, Mark Michelson wrote:
> The "vtep: 3 HVs, 1 VIFs/HV, 1 GW, 1 LS" test fails consistently on PPC.
> The test expects a flow with the following to be present:
>
> is_chassis_resident("cr-lrp1") || is_chassis_resident("cr-lrp2")
>
> However, on PPC, the test fails because the flow has the following
> instead:
>
> is_chassis_resident("cr-lrp2") || is_chassis_resident("cr-lrp1")
>
> In other words, the fields are reversed. This is because the ordering of
> the ports in ovn-northd can differ depending on architecture. This is a
> case where the northd code is correct, but the test is overly
> restrictive.
>
> To fix this, the test has been broken into two parts. First, we check
> the overall flow structure to ensure that there are two
> "is_chassis_resident" fields present. Then we can break that flow down
> further and sort the fields so that they will be in a predictable order.
>
> Signed-off-by: Mark Michelson <[email protected]>
> ---
Hi Mark,
> tests/ovn.at | 17 +++++++++++++++--
> 1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 4461b840e..80ede049d 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -4269,12 +4269,25 @@
> response=${sha}${lrpmac}08060001080006040002${lrpmac}${tpa}${sha}${spa}
> # we expect arp reply packet on hv3
> echo $response >> 3.expected
>
> -AT_CHECK([ovn-sbctl lflow-list lsw0 | grep 'reg0[\[14\]]' | sort | sed
> 's/table=../table=??/g'], [0], [dnl
> +# First ensure basic flow contents are as we expect.
> +AT_CHECK([ovn-sbctl lflow-list lsw0 | grep 'reg0[\[14\]]' | sort | sed
> 's/table=../table=??/g' | sed
> 's/is_chassis_resident([[^)]]*)/is_chassis_resident("??")/g'], [0], [dnl
> table=??(ls_in_check_port_sec), priority=70 , match=(inport ==
> "lp-vtep"), action=(reg0[[14]] = 1; next(pipeline=ingress, table=??);)
> table=??(ls_in_hairpin ), priority=1000 , match=(reg0[[14]] == 1),
> action=(next(pipeline=ingress, table=??);)
> - table=??(ls_in_hairpin ), priority=2000 , match=(reg0[[14]] == 1 &&
> (is_chassis_resident("cr-lrp1") || is_chassis_resident("cr-lrp2"))),
> action=(next;)
> + table=??(ls_in_hairpin ), priority=2000 , match=(reg0[[14]] == 1 &&
> (is_chassis_resident("??") || is_chassis_resident("??"))), action=(next;)
> ])
>
> +# We've ensured that the expected hairpin flows are present
> +# and that the expected number of "is_chassis_resident" fields are in
> +# the flow. Now we need to ensure the contents are correct.
> +# Unfortunately, the order of the "is_chassis_resident" fields is
> +# unpredictable. Therefore we sort them so the order is predictable.
> +actual_chassis=$(ovn-sbctl lflow-list lsw0 | grep 'reg0[\[14\]]' | grep
> 'priority=2000' | grep -o 'is_chassis_resident([[^)]]*)' | sort)
This looks a bit weird, shouldn't the first grep be 'reg0\[[14\]]'?
Surprisingly, your version seems to work too.
To avoid this kind of confusion can we change it to the following?
actual_chassis=$(ovn-sbctl lflow-list lsw0 | grep ls_in_hairpin | grep -o
'is_chassis_resident([[^)]]*)' | sort)
In any case, I don't have a strong preference, any of the two above
will do, and if you change this at apply time please also add:
Acked-by: Dumitru Ceara <[email protected]>
> +
> +expected_chassis='is_chassis_resident("cr-lrp1")
> +is_chassis_resident("cr-lrp2")'
> +
> +check test "$expected_chassis" = "$actual_chassis"
> +
> # dump information with counters
> echo "------ OVN dump ------"
> ovn-nbctl show
Thanks,
Dumitru
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
