> When using Distributed Gateway Port and HA, IPv6 ND_NA packet
> received on the router port causes all HA chassis to try updating the
> Mac_Binding table, often causing transaction errors.
> This patch prevents this by avoiding Mac_Binding updates due to ND_NS or NA
> packets received on non residend DGP.
> 
> Modified test "4 HV, 3 LS, 2 LR, packet test with HA distributed router 
> gateway port"
> to often create such behavior.
> 
> While at modifying the test, also fix some potential flakes.
> 

Hi Xavier,

The patch looks good to me, just a nit inline. Anyway:
Acked-by: Lorenzo Bianconi <[email protected]>

> Signed-off-by: Xavier Simonart <[email protected]>
> Reported-at: https://issues.redhat.com/browse/FDP-1567
> 
> ---
> v2: - Resent v1 by error.
> v3: - Rebased on origin/main.
>     - Fix few flakes:
>     - 1) 102 mac_binding seen instead of 101, due to an additional mac 
> binding for the router port.
>          It is an independent minor OVN issue. Avoid the issue in the test 
> for now.
>     - 2) Missing garp, on slow systems. Flake introduced by v1.
>     - 3) If ovs is slower than test script, reset_pcap might happen before 
> 1st packet is received, and packet is seen twice.
>          This flake existed before the patch.
> ---
>  northd/northd.c     | 13 ++++++++++++
>  tests/ovn-northd.at |  3 +++
>  tests/ovn.at        | 52 +++++++++++++++++++++++++++++++++++++++------
>  3 files changed, 61 insertions(+), 7 deletions(-)
> 
> diff --git a/northd/northd.c b/northd/northd.c
> index 8b5413ef3..eb5551d6a 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -13742,6 +13742,19 @@ build_neigh_learning_flows_for_lrouter_port(
>                                  &op->nbrp->header_,
>                                  lflow_ref);
>      }
> +
> +    if (lrp_is_l3dgw(op)) {
> +        ds_clear(match);
> +        ds_put_format(match, "inport == %s && (nd_na || nd_ns) && "
> +                             "!is_chassis_resident(%s)", op->json_key,
> +                             op->cr_port->json_key);
> +        ovn_lflow_add_with_hint(lflows, op->od,
> +                                S_ROUTER_IN_LOOKUP_NEIGHBOR, 120,
> +                                ds_cstr(match),
> +                                REGBIT_LOOKUP_NEIGHBOR_RESULT" = 1; next;",
> +                                &op->nbrp->header_,
> +                                lflow_ref);
> +    }
>  }
>  
>  /* Logical router ingress table ND_RA_OPTIONS & ND_RA_RESPONSE: IPv6 Router

[...]

>  test_ip_packet()
>  {
>      local active_gw=$1
>      local backup_gw=$2
>  
> +    # Send many GARP/ND_NA from outside1 to the router.

Maybe you can add an explanation about why we are sending these packets.

> +    for i in $(seq 11 $((n_external_ip + 10))); do
> +        send_garp ext1 ext1-vif1 1 "00:00:00:00:20:$i" "ff:ff:ff:ff:ff:ff" 
> 172.16.1.$i 172.16.1.$i
> +        send_na ext1 ext1-vif1 "00:00:00:00:30:$i" "33:33:00:00:00:01" 
> "fd10::$i" "ff02::1"
> +    done
> +
>      # Send ip packet between foo1 and outside1
>      src_mac="f00000010203" # foo1 mac
>      dst_mac="000001010203" # foo-R0 mac (internal router leg)
> @@ -12633,6 +12654,10 @@ grep actions=mod_dl_dst:f0:00:00:01:02:04 | wc -l` 
> -eq 1
>      dst_ip=`ip_to_hex 172 16 1 3`
>      
> expected=${dst_mac}${src_mac}08004500001c000000003e110200${src_ip}${dst_ip}0035111100080000
>      echo $expected > ext1-vif1.expected
> +
> +    # Wait for first packet to be received before resetting pcap on ext1
> +    OVN_CHECK_PACKETS_CONTAIN([ext1/vif1-tx.pcap], [ext1-vif1.expected])
> +
>      
> exp_gw_ip_garp=ffffffffffff00000201020308060001080006040001000002010203ac100101000000000000ac100101
>      echo $exp_gw_ip_garp >> ext1-vif1.expected
>  
> @@ -12665,16 +12690,29 @@ AT_CHECK([ovn-nbctl --wait=hv \
>      --id=@gc1 create Gateway_Chassis name=alice_gw2 \
>                                       chassis_name=gw2 \
>                                       priority=20 -- \
> -    set Logical_Router_Port alice 'gateway_chassis=[@gc0,@gc1]' | uuidfilt], 
> [0], [<0>
> +    --id=@gc2 create Gateway_Chassis name=alice_gw3 \
> +                                     chassis_name=gw3 \
> +                                     priority=5 -- \
> +    set Logical_Router_Port alice 'gateway_chassis=[@gc0,@gc1,@gc2]' | 
> uuidfilt], [0], [<0>
>  <1>
> +<2>
>  ])
>  
>  OVS_WAIT_UNTIL([
> -    test 1 = $(as hv1 ovs-ofctl dump-flows br-int 
> table=OFTABLE_REMOTE_OUTPUT | grep -c 
> "active_backup,ofport,members:$hv1_gw2_ofport,$hv1_gw1_ofport")
> +    test 1 = $(as hv1 ovs-ofctl dump-flows br-int 
> table=OFTABLE_REMOTE_OUTPUT | grep -c 
> "active_backup,ofport,members:$hv1_gw2_ofport,$hv1_gw1_ofport,$hv1_gw3_ofport")
>  ])
>  
>  test_ip_packet gw2 gw1
>  
> +# n_external_ip * 2 + **1** as 172.16.1.3.
> +# 172.16.1.1 (i.e. the router port) might sometimes be seen as a 
> mac_binding. Ignore it.
> +wait_row_count MAC_Binding $((n_external_ip * 2 + 1)) ip!=172.16.1.1
> +
> +for i in gw1 gw2 gw3; do
> +    AT_CHECK([grep -c "Transaction causes multiple rows" 
> $i/ovn-controller.log], [1], [0
> +])
> +done
> +
>  OVN_CLEANUP([hv1],[gw1],[gw2],[ext1])
>  AT_CLEANUP
>  ])
> -- 
> 2.47.1
> 
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> 
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to