[ovs-dev,ovn] ovn.at: Fix ARP test that fails due to timing.

The test for "ARP/ND request broadcast limiting" checks that injected
ARP packets are not flooded using the MC_FLOOD multicast group. However,
this introduces a race condition in the test because GARPs generated by
OVN would also hit the same openflow rules.

Remove the checks that use the MC_FLOOD group. They are also redundant
as the rest of the test checks that packets are forwarded according to
the newly added, higher priority rules.

Fixes: 32f5ebb06226 ("ovn-northd: Limit ARP/ND broadcast domain whenever possible.")
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
---
 tests/ovn.at | 33 ---------------------------------
 1 file changed, 33 deletions(-)

On Tue, Mar 24, 2020 at 7:06 PM Dumitru Ceara <dceara@redhat.com> wrote:
>
> The test for "ARP/ND request broadcast limiting" checks that injected
> ARP packets are not flooded using the MC_FLOOD multicast group. However,
> this introduces a race condition in the test because GARPs generated by
> OVN would also hit the same openflow rules.
>
> Remove the checks that use the MC_FLOOD group. They are also redundant
> as the rest of the test checks that packets are forwarded according to
> the newly added, higher priority rules.
>
> Fixes: 32f5ebb06226 ("ovn-northd: Limit ARP/ND broadcast domain whenever possible.")
> Signed-off-by: Dumitru Ceara <dceara@redhat.com>

Thanks Dumitru. I applied this patch to master and branch-20.03

Numan

> ---
>  tests/ovn.at | 33 ---------------------------------
>  1 file changed, 33 deletions(-)
>
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 4baf2e9..9a44f0a 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -18015,9 +18015,6 @@ r1_dp_key=$(ovn-sbctl --bare --columns tunnel_key list datapath_binding rtr1)
>  r1_tnl_key=$(ovn-sbctl --bare --columns tunnel_key list port_binding sw-rtr1)
>  r2_tnl_key=$(ovn-sbctl --bare --columns tunnel_key list port_binding sw-rtr2)
>
> -mc_key=$(ovn-sbctl --bare --columns tunnel_key find multicast_group datapath=${sw_dp_uuid} name="_MC_flood")
> -mc_key=$(printf "%04x" $mc_key)
> -
>  match_sw_metadata="metadata=0x${sw_dp_key}"
>  match_r1_metadata="metadata=0x${r1_dp_key}"
>
> @@ -18042,11 +18039,6 @@ OVS_WAIT_UNTIL([
>      grep n_packets=1 -c)
>      test "0" = "${pkts_to_rtr2}"
>  ])
> -OVS_WAIT_UNTIL([
> -    pkts_flooded=$(ovs-ofctl dump-flows br-int | \
> -    grep -E "${match_sw_metadata}" | grep ${mc_key} | grep -v n_packets=0 -c)
> -    test "0" = "${pkts_flooded}"
> -])
>
>  # Inject ND_NS for ofirst router owned IP address.
>  src_ipv6=00100000000000000000000000000254
> @@ -18069,11 +18061,6 @@ OVS_WAIT_UNTIL([
>      grep n_packets=1 -c)
>      test "0" = "${pkts_to_rtr2}"
>  ])
> -OVS_WAIT_UNTIL([
> -    pkts_flooded=$(ovs-ofctl dump-flows br-int | \
> -    grep -E "${match_sw_metadata}" | grep ${mc_key} | grep -v n_packets=0 -c)
> -    test "0" = "${pkts_flooded}"
> -])
>
>  # Configure load balancing on both routers.
>  ovn-nbctl lb-add lb1-v4 10.0.0.11 42.42.42.1
> @@ -18108,11 +18095,6 @@ OVS_WAIT_UNTIL([
>      grep n_packets=1 -c)
>      test "0" = "${pkts_to_rtr2}"
>  ])
> -OVS_WAIT_UNTIL([
> -    pkts_flooded=$(ovs-ofctl dump-flows br-int | \
> -    grep -E "${match_sw_metadata}" | grep ${mc_key} | grep -v n_packets=0 -c)
> -    test "0" = "${pkts_flooded}"
> -])
>
>  # Inject ND_NS for first router owned VIP address.
>  src_ipv6=00100000000000000000000000000254
> @@ -18135,11 +18117,6 @@ OVS_WAIT_UNTIL([
>      grep n_packets=1 -c)
>      test "0" = "${pkts_to_rtr2}"
>  ])
> -OVS_WAIT_UNTIL([
> -    pkts_flooded=$(ovs-ofctl dump-flows br-int | \
> -    grep -E "${match_sw_metadata}" | grep ${mc_key} | grep -v n_packets=0 -c)
> -    test "0" = "${pkts_flooded}"
> -])
>
>  # Configure NAT on both routers.
>  ovn-nbctl lr-nat-add rtr1 dnat_and_snat 10.0.0.111 42.42.42.1
> @@ -18175,11 +18152,6 @@ OVS_WAIT_UNTIL([
>      grep n_packets=1 -c)
>      test "0" = "${pkts_to_rtr2}"
>  ])
> -OVS_WAIT_UNTIL([
> -    pkts_flooded=$(ovs-ofctl dump-flows br-int | \
> -    grep -E "${match_sw_metadata}" | grep ${mc_key} | grep -v n_packets=0 -c)
> -    test "0" = "${pkts_flooded}"
> -])
>
>  # Inject ARP request for FIP1.
>  send_arp_request 1 0 ${src_mac} $(ip_to_hex 10 0 0 254) $(ip_to_hex 10 0 0 121)
> @@ -18242,11 +18214,6 @@ OVS_WAIT_UNTIL([
>      grep n_packets=1 -c)
>      test "0" = "${pkts_to_rtr2}"
>  ])
> -OVS_WAIT_UNTIL([
> -    pkts_flooded=$(ovs-ofctl dump-flows br-int | \
> -    grep -E "${match_sw_metadata}" | grep ${mc_key} | grep -v n_packets=0 -c)
> -    test "0" = "${pkts_flooded}"
> -])
>
>  # Inject ND_NS for FIP1.
>  src_ipv6=00100000000000000000000000000254
> --
> 1.8.3.1
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>

On 3/24/20 3:45 PM, Numan Siddique wrote:
> On Tue, Mar 24, 2020 at 7:06 PM Dumitru Ceara <dceara@redhat.com> wrote:
>>
>> The test for "ARP/ND request broadcast limiting" checks that injected
>> ARP packets are not flooded using the MC_FLOOD multicast group. However,
>> this introduces a race condition in the test because GARPs generated by
>> OVN would also hit the same openflow rules.
>>
>> Remove the checks that use the MC_FLOOD group. They are also redundant
>> as the rest of the test checks that packets are forwarded according to
>> the newly added, higher priority rules.
>>
>> Fixes: 32f5ebb06226 ("ovn-northd: Limit ARP/ND broadcast domain whenever possible.")
>> Signed-off-by: Dumitru Ceara <dceara@redhat.com>
> 
> Thanks Dumitru. I applied this patch to master and branch-20.03
> 
> Numan
> 

Thanks!