On Tue, Jan 13, 2026 at 12:20 PM Mark Michelson <[email protected]> wrote: > > Hi Han, > > I have one super tiny nit below, but other than that, > > Acked-by: Mark Michelson <[email protected]>
Thanks Mark! I addressed your minor comment and merged to main and backported to 25.09 and 25.03. Best, Han > > On Mon, Jan 12, 2026 at 11:39 AM Han Zhou <[email protected]> wrote: > > > > When a remote chassis has multiple encap IPs, the original code only > > created ARP reply and ND NA ingress flows for the first tunnel found. > > This caused responses arriving on other tunnels to miss the ingress > > pipeline, failing to learn MAC bindings. > > > > Fix by iterating over all tunnels in the ingress loop and creating flows > > for each tunnel that belongs to a remote chassis. The egress flood flow > > still uses only one tunnel per chassis to avoid duplicate traffic. > > > > Fixes: 7c3f7f415f1d ("northd, controller: Flood ARP and NA packet on transit router.") > > Assisted-by: Cursor, with model: claude-4.5-opus-high > > Signed-off-by: Han Zhou <[email protected]> > > --- > > controller/physical.c | 56 +++++++++++++++++++++++++++-------- > > tests/ovn-controller.at | 65 +++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 109 insertions(+), 12 deletions(-) > > > > diff --git a/controller/physical.c b/controller/physical.c > > index b9c60c8ab13c..103fd4d9c4ff 100644 > > --- a/controller/physical.c > > +++ b/controller/physical.c > > @@ -3098,6 +3098,9 @@ physical_eval_remote_chassis_flows(const struct physical_ctx *ctx, > > > > ofpbuf_clear(egress_ofpacts); > > > > + /* For egress flooding, we only need one tunnel per remote chassis. > > + * Using chassis_tunnel_find() which returns the first tunnel is sufficient > > + * since we just need to reach the remote chassis once per flood. */ > > const struct sbrec_chassis *chassis; > > SBREC_CHASSIS_TABLE_FOR_EACH (chassis, ctx->chassis_table) { > > if (!smap_get_bool(&chassis->other_config, "is-remote", false)) { > > @@ -3125,7 +3128,46 @@ physical_eval_remote_chassis_flows(const struct physical_ctx *ctx, > > > > ofpact_put_OUTPUT(egress_ofpacts)->port = tun->ofport; > > prev = tun; > > + } > > + > > + if (egress_ofpacts->size > 0) { > > + match_init_catchall(&match); > > + /* Match if the packet wasn't already received from tunnel. > > + * This prevents from looping it back to the tunnel again. */ > > + match_set_reg_masked(&match, MFF_LOG_FLAGS - MFF_REG0, 0, > > + MLF_RX_FROM_TUNNEL); > > + ofctrl_add_flow(flow_table, OFTABLE_FLOOD_REMOTE_CHASSIS, 100, 0, > > + &match, egress_ofpacts, hc_uuid); > > + } > > > > + /* For ingress, we need to handle ALL tunnels to remote chassis because > > + * ARP replies and ND NA responses could arrive on any tunnel. A remote > > + * chassis may have multiple tunnels (e.g., multiple encap IPs). */ > > + struct chassis_tunnel *tun; > > + HMAP_FOR_EACH (tun, hmap_node, ctx->chassis_tunnels) { > > + char *chassis_name = NULL; > > + if (!encaps_tunnel_id_parse(tun->chassis_id, &chassis_name, > > + NULL, NULL)) { > > + continue; > > + } > > + > > + /* Look up the chassis record and check if it's a remote chassis. */ > > + const struct sbrec_chassis *remote_chassis = > > + chassis_lookup_by_name(ctx->sbrec_chassis_by_name, chassis_name); > > + free(chassis_name); > > + > > + if (!remote_chassis || > > + !smap_get_bool(&remote_chassis->other_config, > > + "is-remote", false)) { > > + continue; > > + } > > + > > + /* Do not create flows for Geneve if the TLV negotiation is not > > + * finished. > > + */ > > + if (tun->type == GENEVE && !ctx->mff_ovn_geneve) { > > + continue; > > + } > > Nit: This if check is much cheaper compared to allocating a > chassis_name, looking up a chassis, and looking up chassis options. > It's probably worthwhile to put this at the top of the loop instead. > > > > > ofpbuf_clear(&ingress_ofpacts); > > put_load(1, MFF_LOG_FLAGS, MLF_RX_FROM_TUNNEL_BIT, 1, > > @@ -3147,7 +3189,7 @@ physical_eval_remote_chassis_flows(const struct physical_ctx *ctx, > > ctx->mff_ovn_geneve); > > > > ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 120, > > - chassis->header_.uuid.parts[0], > > + remote_chassis->header_.uuid.parts[0], > > &match, &ingress_ofpacts, hc_uuid); > > > > /* Add match on ND NA coming from remote chassis. */ > > @@ -3161,20 +3203,10 @@ physical_eval_remote_chassis_flows(const struct physical_ctx *ctx, > > ctx->mff_ovn_geneve); > > > > ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 120, > > - chassis->header_.uuid.parts[0], > > + remote_chassis->header_.uuid.parts[0], > > &match, &ingress_ofpacts, hc_uuid); > > } > > > > - if (egress_ofpacts->size > 0) { > > - match_init_catchall(&match); > > - /* Match if the packet wasn't already received from tunnel. > > - * This prevents from looping it back to the tunnel again. */ > > - match_set_reg_masked(&match, MFF_LOG_FLAGS - MFF_REG0, 0, > > - MLF_RX_FROM_TUNNEL); > > - ofctrl_add_flow(flow_table, OFTABLE_FLOOD_REMOTE_CHASSIS, 100, 0, > > - &match, egress_ofpacts, hc_uuid); > > - } > > - > > ofpbuf_uninit(&ingress_ofpacts); > > } > > > > diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at > > index d7cc06d006a9..33bae69e17bf 100644 > > --- a/tests/ovn-controller.at > > +++ b/tests/ovn-controller.at > > @@ -3672,6 +3672,71 @@ AT_CHECK_UNQUOTED([grep "cookie=$hv3_cookie," phy_to_log_flows], [0], [dnl > > OVN_CLEANUP([hv1]) > > AT_CLEANUP > > > > +AT_SETUP([Remote chassis flood flows - multiple tunnels per chassis]) > > +ovn_start > > + > > +net_add n1 > > +sim_add hv1 > > +as hv1 > > +check ovs-vsctl add-br br-phys > > +ovn_attach n1 br-phys 192.168.0.11 24 geneve > > + > > +check ovs-vsctl set open . external_ids:ovn-is-interconn=true > > + > > +# Add a remote chassis with TWO encap IPs (two tunnels). > > +# This tests that ingress flows are created for ALL tunnels to a remote > > +# chassis, not just the first one found. > > +check ovn-sbctl chassis-add hv2 geneve 192.168.0.12 > > +check_uuid ovn-sbctl --id=@encap create Encap type=geneve ip=192.168.0.13 \ > > + -- add Chassis hv2 encaps @encap > > +check ovn-sbctl set chassis hv2 other_config:is-remote=true > > + > > +check ovn-nbctl --wait=hv sync > > + > > +# Wait for both tunnels to be created by checking ovn-chassis-id external_ids. > > +# Each tunnel port should have ovn-chassis-id=hv2@ <remote_ip>%<local_ip>. > > +ovs-vsctl list port > > +ovs-vsctl list interface > > +OVS_WAIT_UNTIL([ > > + test "$(ovs-vsctl --bare --columns external_ids list port | \ > > + grep -c 'ovn-chassis-id=hv2@')" -eq 2 > > +]) > > + > > +chassis_cookie() { > > + ovn-debug uuid-to-cookie $(fetch_column chassis _uuid name=$1) > > +} > > + > > +ovs-ofctl dump-flows --names --no-stats br-int table=OFTABLE_PHY_TO_LOG > phy_to_log_flows > > +ovs-ofctl dump-flows --names --no-stats br-int table=OFTABLE_FLOOD_REMOTE_CHASSIS > flood_flows > > + > > +# Check that egress flood flow outputs to only ONE tunnel (first found). > > +# We should have exactly one output action to hv2. > > +AT_CHECK([grep -c 'output:"ovn-hv2' flood_flows], [0], [dnl > > +1 > > +]) > > + > > +# Check that ingress flows exist for BOTH tunnels to hv2. > > +# Each tunnel should have ARP and ND NA ingress flows. > > +hv2_cookie="$(chassis_cookie hv2)" > > + > > +# Count total ingress flows with hv2's cookie - should be 4 (2 tunnels x 2 flows each) > > +AT_CHECK([grep -c "cookie=$hv2_cookie," phy_to_log_flows], [0], [dnl > > +4 > > +]) > > + > > +# Verify we have ARP flows for both tunnel ports > > +AT_CHECK([grep "cookie=$hv2_cookie," phy_to_log_flows | grep -c "arp,.*in_port=\"ovn-hv2"], [0], [dnl > > +2 > > +]) > > + > > +# Verify we have ND NA flows for both tunnel ports > > +AT_CHECK([grep "cookie=$hv2_cookie," phy_to_log_flows | grep -c "icmp6,.*in_port=\"ovn-hv2"], [0], [dnl > > +2 > > +]) > > + > > +OVN_CLEANUP([hv1]) > > +AT_CLEANUP > > + > > AT_SETUP([ovn-cntroller exit - Cleanup]) > > ovn_start > > > > -- > > 2.38.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
