On Fri, Sep 12, 2025 at 3:51 PM Lorenzo Bianconi via dev < [email protected]> wrote:
> > 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 Thank you Xaiver, Dumitru and Lorenzo, I have merged the patch into main and backported all the way down 24.03. I have missed the nit sorry about that. Regards, Ales _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
