revert commit 70df1aba7dd36c38905dc643ca709d8e85e541a7

changluyi · changluyi · commit fe35e45af61b · 2025-11-05T13:59:19.000+08:00
Signed-off-by: clyi &lt;clyi@alauda.io&gt;
diff --git a/dist/images/Dockerfile.base b/dist/images/Dockerfile.base
@@ -63,6 +63,7 @@ ADD patches/e4e6ea9c5f4ba080b719924e470daa8094ff38a7.patch $SRC_DIR
 ADD patches/e7d3ba53cdcbc524bb29c54ddb07b83cc4258ed7.patch $SRC_DIR
 ADD patches/9286e1fd578fdb8f565a0f4aa9066b538295e1ac.patch $SRC_DIR
 ADD patches/737d9f932edada5a91f315b5f382daada8dee952.patch $SRC_DIR
+ADD patches/revert_70df1aba7dd36c38905dc643ca709d8e85e541a7.patch $SRC_DIR
 
 RUN --mount=type=cache,target=/var/cache/apt,sharing=locked \
     --mount=type=cache,target=/var/lib/apt,sharing=locked \
@@ -125,7 +126,9 @@ RUN cd /usr/src/ && git clone -b branch-25.03 --depth=1 https://github.com/ovn-o
     # northd: skip arp/nd request for lrp addresses from localnet ports
     git apply $SRC_DIR/477695a010affe56efdd66b60510fa612f8704c1.patch && \
     # add skip conntrack ipcidrs support
-    git apply $SRC_DIR/737d9f932edada5a91f315b5f382daada8dee952.patch
+    git apply $SRC_DIR/737d9f932edada5a91f315b5f382daada8dee952.patch && \
+    # revert lport_pb_is_chassis_resident changes to fix BFD flap issues
+    git apply $SRC_DIR/revert_70df1aba7dd36c38905dc643ca709d8e85e541a7.patch
 
 RUN --mount=type=cache,target=/var/cache/apt,sharing=locked \
     --mount=type=cache,target=/var/lib/apt,sharing=locked \
diff --git a/dist/images/patches/revert_70df1aba7dd36c38905dc643ca709d8e85e541a7.patch b/dist/images/patches/revert_70df1aba7dd36c38905dc643ca709d8e85e541a7.patch
@@ -0,0 +1,327 @@
+commit 70df1aba7dd36c38905dc643ca709d8e85e541a7
+Author: Ales Musil <amusil@redhat.com>
+Date:   Wed Jun 18 07:33:10 2025 +0200
+
+    controller: Modify the behavior of lport_pb_is_chassis_resident.
+    
+    The function behavior would be unexpected in cases when there is
+    BFD flap between two gateway chassis. This would result in two
+    chassis racing to claim the same port binding. This leads to
+    multiple problems like unsent gARPs. The gARP problem on its
+    own was solved [0], but unfortunately reintroduced [1] in
+    slightly different form.
+    
+    The conclusion from the investigation is that the function in
+    the current form is easy to misuse. To prevent that use
+    only SB state to determine which chassis has actually claimed
+    the port binding. This is the desired behavior in cases where
+    this function was used previously.
+    
+    The code that determines which chassis should claim port binding
+    based on BFD status remains unchanged, thus the behavior during
+    failover should be the same with minimizing the impact for BFD
+    flapping cases.
+    
+    [0] 289ec19b01ad ("pinctrl: Fix missing garp.")
+    [1] 05527bd6ccdb ("controller: Extract garp_rarp to engine node.").
+    
+    Acked-by: Dumitru Ceara <dceara@redhat.com>
+    Signed-off-by: Ales Musil <amusil@redhat.com>
+    (cherry picked from commit aa088003406662340df41aaac6e68e5d96272fa6)
+
+diff --git b/controller/lport.c a/controller/lport.c
+index 077e66acf..92de375b5 100644
+--- b/controller/lport.c
++++ a/controller/lport.c
+@@ -65,32 +65,35 @@ lport_lookup_by_key(struct ovsdb_idl_index *sbrec_datapath_binding_by_key,
+ 
+ bool
+ lport_pb_is_chassis_resident(const struct sbrec_chassis *chassis,
++                             const struct sset *active_tunnels,
+                              const struct sbrec_port_binding *pb)
+ {
+     if (!pb || !pb->chassis) {
+         return false;
+     }
+-
+-    /* Note: we rely on SB to provide information about who owns the port
+-     * binding.  In particular, for chassisredirect ports, this avoids issues
+-     * when the underlying BFD state changes are only detected by some of the
+-     * chassis in the associated HA_Chassis_Group. */
+-    return pb->chassis == chassis;
++    if (strcmp(pb->type, "chassisredirect")) {
++        return pb->chassis == chassis;
++    } else {
++        return ha_chassis_group_is_active(pb->ha_chassis_group,
++                                          active_tunnels, chassis);
++    }
+ }
+ 
+ bool
+ lport_is_chassis_resident(struct ovsdb_idl_index *sbrec_port_binding_by_name,
+                           const struct sbrec_chassis *chassis,
++                          const struct sset *active_tunnels,
+                           const char *port_name)
+ {
+     const struct sbrec_port_binding *pb
+         = lport_lookup_by_name(sbrec_port_binding_by_name, port_name);
+-    return lport_pb_is_chassis_resident(chassis, pb);
++    return lport_pb_is_chassis_resident(chassis, active_tunnels, pb);
+ }
+ 
+ bool
+ lport_is_local(struct ovsdb_idl_index *sbrec_port_binding_by_name,
+                const struct sbrec_chassis *chassis,
++               const struct sset *active_tunnels,
+                const char *port_name)
+ {
+     const struct sbrec_port_binding *pb = lport_lookup_by_name(
+@@ -100,14 +103,14 @@ lport_is_local(struct ovsdb_idl_index *sbrec_port_binding_by_name,
+         return false;
+     }
+ 
+-    if (lport_pb_is_chassis_resident(chassis, pb)) {
++    if (lport_pb_is_chassis_resident(chassis, active_tunnels, pb)) {
+         return true;
+     }
+ 
+     const struct sbrec_port_binding *cr_pb =
+         lport_get_cr_port(sbrec_port_binding_by_name, pb, NULL);
+ 
+-    return lport_pb_is_chassis_resident(chassis, cr_pb);
++    return lport_pb_is_chassis_resident(chassis, active_tunnels, cr_pb);
+ }
+ 
+ const struct sbrec_port_binding *
+diff --git b/controller/lport.h a/controller/lport.h
+index 6d48301d2..8b1809a27 100644
+--- b/controller/lport.h
++++ a/controller/lport.h
+@@ -62,12 +62,15 @@ const struct sbrec_multicast_group *mcgroup_lookup_by_dp_name(
+ bool
+ lport_is_chassis_resident(struct ovsdb_idl_index *sbrec_port_binding_by_name,
+                           const struct sbrec_chassis *chassis,
++                          const struct sset *active_tunnels,
+                           const char *port_name);
+ bool lport_pb_is_chassis_resident(const struct sbrec_chassis *chassis,
++                                  const struct sset *active_tunnels,
+                                   const struct sbrec_port_binding *pb);
+ 
+ bool lport_is_local(struct ovsdb_idl_index *sbrec_port_binding_by_name,
+                     const struct sbrec_chassis *chassis,
++                    const struct sset *active_tunnels,
+                     const char *port_name);
+ const struct sbrec_port_binding *lport_get_peer(
+     const struct sbrec_port_binding *,
+diff --git b/controller/ovn-controller.c a/controller/ovn-controller.c
+index 8d02f4c28..4be51102b 100644
+--- b/controller/ovn-controller.c
++++ a/controller/ovn-controller.c
+@@ -5027,6 +5027,7 @@ en_route_run(struct engine_node *node, void *data)
+         .sbrec_port_binding_by_name = sbrec_port_binding_by_name,
+         .chassis = chassis,
+         .dynamic_routing_port_mapping = dynamic_routing_port_mapping,
++        .active_tunnels = &rt_data->active_tunnels,
+         .local_datapaths = &rt_data->local_datapaths,
+         .local_lports = &rt_data->local_lports,
+         .local_bindings = &rt_data->lbinding_data.bindings,
+@@ -5129,6 +5130,7 @@ route_runtime_data_handler(struct engine_node *node, void *data)
+             struct tracked_lport *lport = shash_node->data;
+ 
+             if (route_exchange_find_port(sbrec_port_binding_by_name, chassis,
++                                         &rt_data->active_tunnels,
+                                          lport->pb, NULL)) {
+                 /* XXX: Until we get I-P support for route exchange we need to
+                  * request recompute. */
+@@ -5186,6 +5188,8 @@ route_sb_port_binding_data_handler(struct engine_node *node, void *data)
+         engine_ovsdb_node_get_index(
+                 engine_get_input("SB_port_binding", node),
+                 "name");
++    struct ed_type_runtime_data *rt_data =
++        engine_get_input_data("runtime_data", node);
+ 
+ 
+     /* There are the following cases where we need to handle updates to the
+@@ -5209,7 +5213,8 @@ route_sb_port_binding_data_handler(struct engine_node *node, void *data)
+         }
+ 
+         if (route_exchange_find_port(sbrec_port_binding_by_name, chassis,
+-                                     sbrec_pb, NULL)) {
++                                     &rt_data->active_tunnels, sbrec_pb,
++                                     NULL)) {
+             /* XXX: Until we get I-P support for route exchange we need to
+              * request recompute. */
+             return false;
+@@ -6442,6 +6447,7 @@ main(int argc, char *argv[])
+                                         ovnsb_idl_loop.idl),
+                                     br_int, chassis,
+                                     &runtime_data->local_datapaths,
++                                    &runtime_data->active_tunnels,
+                                     &runtime_data->local_active_ports_ipv6_pd,
+                                     &runtime_data->local_active_ports_ras,
+                                     ovsrec_open_vswitch_table_get(
+diff --git b/controller/physical.c a/controller/physical.c
+index d068d65c1..d0d9d9006 100644
+--- b/controller/physical.c
++++ a/controller/physical.c
+@@ -871,7 +871,8 @@ put_replace_router_port_mac_flows(const struct physical_ctx *ctx,
+         struct ofpact_mac *replace_mac;
+         char *cr_peer_name = xasprintf("cr-%s", rport_binding->logical_port);
+         if (lport_is_chassis_resident(ctx->sbrec_port_binding_by_name,
+-                                      ctx->chassis, cr_peer_name)) {
++                                      ctx->chassis, ctx->active_tunnels,
++                                      cr_peer_name)) {
+             /* If a router port's chassisredirect port is
+              * resident on this chassis, then we need not do mac replace. */
+             free(cr_peer_name);
+diff --git b/controller/pinctrl.c a/controller/pinctrl.c
+index 893a2c440..748d4b442 100644
+--- b/controller/pinctrl.c
++++ a/controller/pinctrl.c
+@@ -377,7 +377,8 @@ pinctrl_handle_bfd_msg(struct rconn *swconn, const struct flow *ip_flow,
+ static void bfd_monitor_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
+                             const struct sbrec_bfd_table *bfd_table,
+                             struct ovsdb_idl_index *sbrec_port_binding_by_name,
+-                            const struct sbrec_chassis *chassis)
++                            const struct sbrec_chassis *chassis,
++                            const struct sset *active_tunnels)
+                             OVS_REQUIRES(pinctrl_mutex);
+ static void init_fdb_entries(void);
+ static void destroy_fdb_entries(void);
+@@ -1443,6 +1444,7 @@ prepare_ipv6_prefixd(struct ovsdb_idl_txn *ovnsb_idl_txn,
+                      struct ovsdb_idl_index *sbrec_port_binding_by_name,
+                      const struct shash *local_active_ports_ipv6_pd,
+                      const struct sbrec_chassis *chassis,
++                     const struct sset *active_tunnels,
+                      const struct hmap *local_datapaths)
+     OVS_REQUIRES(pinctrl_mutex)
+ {
+@@ -1471,8 +1473,9 @@ prepare_ipv6_prefixd(struct ovsdb_idl_txn *ovnsb_idl_txn,
+         }
+ 
+         char *redirect_name = xasprintf("cr-%s", pb->logical_port);
+-        bool resident = lport_is_chassis_resident(sbrec_port_binding_by_name,
+-                                                  chassis, redirect_name);
++        bool resident = lport_is_chassis_resident(
++                sbrec_port_binding_by_name, chassis, active_tunnels,
++                redirect_name);
+         free(redirect_name);
+         if ((strcmp(pb->type, "l3gateway") || pb->chassis != chassis) &&
+             !resident) {
+@@ -4086,6 +4089,7 @@ pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
+             const struct ovsrec_bridge *br_int,
+             const struct sbrec_chassis *chassis,
+             const struct hmap *local_datapaths,
++            const struct sset *active_tunnels,
+             const struct shash *local_active_ports_ipv6_pd,
+             const struct shash *local_active_ports_ras,
+             const struct ovsrec_open_vswitch_table *ovs_table,
+@@ -4106,7 +4110,7 @@ pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
+     prepare_ipv6_ras(local_active_ports_ras, sbrec_port_binding_by_name);
+     prepare_ipv6_prefixd(ovnsb_idl_txn, sbrec_port_binding_by_name,
+                          local_active_ports_ipv6_pd, chassis,
+-                         local_datapaths);
++                         active_tunnels, local_datapaths);
+     controller_event_run(ovnsb_idl_txn, ce_table, chassis);
+     ip_mcast_sync(ovnsb_idl_txn, chassis, local_datapaths,
+                   sbrec_datapath_binding_by_key,
+@@ -4121,7 +4125,7 @@ pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
+     sync_svc_monitors(ovnsb_idl_txn, svc_mon_table, sbrec_port_binding_by_name,
+                       chassis);
+     bfd_monitor_run(ovnsb_idl_txn, bfd_table, sbrec_port_binding_by_name,
+-                    chassis);
++                    chassis, active_tunnels);
+     run_put_fdbs(ovnsb_idl_txn, sbrec_port_binding_by_key,
+                  sbrec_datapath_binding_by_key, sbrec_fdb_by_dp_key_mac,
+                  cur_cfg);
+@@ -8369,7 +8373,8 @@ static void
+ bfd_monitor_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
+                 const struct sbrec_bfd_table *bfd_table,
+                 struct ovsdb_idl_index *sbrec_port_binding_by_name,
+-                const struct sbrec_chassis *chassis)
++                const struct sbrec_chassis *chassis,
++                const struct sset *active_tunnels)
+     OVS_REQUIRES(pinctrl_mutex)
+ {
+     struct bfd_entry *entry;
+@@ -8401,8 +8406,9 @@ bfd_monitor_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
+         }
+ 
+         char *redirect_name = xasprintf("cr-%s", pb->logical_port);
+-        bool resident = lport_is_chassis_resident(sbrec_port_binding_by_name,
+-                                                  chassis, redirect_name);
++        bool resident = lport_is_chassis_resident(
++                sbrec_port_binding_by_name, chassis, active_tunnels,
++                redirect_name);
+         free(redirect_name);
+         if ((strcmp(pb->type, "l3gateway") || pb->chassis != chassis) &&
+             !resident) {
+diff --git b/controller/pinctrl.h a/controller/pinctrl.h
+index 77df0abfb..f2aed9b5e 100644
+--- b/controller/pinctrl.h
++++ a/controller/pinctrl.h
+@@ -59,6 +59,7 @@ void pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
+                  const struct sbrec_ecmp_nexthop_table *,
+                  const struct ovsrec_bridge *, const struct sbrec_chassis *,
+                  const struct hmap *local_datapaths,
++                 const struct sset *active_tunnels,
+                  const struct shash *local_active_ports_ipv6_pd,
+                  const struct shash *local_active_ports_ras,
+                  const struct ovsrec_open_vswitch_table *ovs_table,
+diff --git b/controller/route.c a/controller/route.c
+index e6b34855f..9a1858a08 100644
+--- b/controller/route.c
++++ a/controller/route.c
+@@ -52,6 +52,7 @@ advertise_route_hash(const struct in6_addr *dst, unsigned int plen)
+ const struct sbrec_port_binding*
+ route_exchange_find_port(struct ovsdb_idl_index *sbrec_port_binding_by_name,
+                          const struct sbrec_chassis *chassis,
++                         const struct sset *active_tunnels,
+                          const struct sbrec_port_binding *pb,
+                          const char **dynamic_routing_port_name)
+ {
+@@ -82,7 +83,7 @@ route_exchange_find_port(struct ovsdb_idl_index *sbrec_port_binding_by_name,
+             smap_get(&cr_pb->options, "dynamic-routing-port-name");
+     }
+ 
+-    if (!lport_pb_is_chassis_resident(chassis, cr_pb)) {
++    if (!lport_pb_is_chassis_resident(chassis, active_tunnels, cr_pb)) {
+         return NULL;
+     }
+ 
+@@ -186,6 +187,7 @@ route_run(struct route_ctx_in *r_ctx_in,
+             const struct sbrec_port_binding *repb =
+                 route_exchange_find_port(r_ctx_in->sbrec_port_binding_by_name,
+                                          r_ctx_in->chassis,
++                                         r_ctx_in->active_tunnels,
+                                          local_peer, &port_name);
+             if (port_name) {
+                 lr_has_port_name_filter = true;
+@@ -289,6 +291,7 @@ route_run(struct route_ctx_in *r_ctx_in,
+                               false);
+             if (lport_is_local(r_ctx_in->sbrec_port_binding_by_name,
+                                r_ctx_in->chassis,
++                               r_ctx_in->active_tunnels,
+                                route->tracked_port->logical_port)) {
+                 priority = PRIORITY_LOCAL_BOUND;
+                 sset_add(r_ctx_out->tracked_ports_local,
+diff --git b/controller/route.h a/controller/route.h
+index e5a96bb31..7d0dd14f1 100644
+--- b/controller/route.h
++++ a/controller/route.h
+@@ -35,6 +35,7 @@ struct route_ctx_in {
+     struct ovsdb_idl_index *sbrec_port_binding_by_name;
+     const struct sbrec_chassis *chassis;
+     const char *dynamic_routing_port_mapping;
++    const struct sset *active_tunnels;
+     const struct hmap *local_datapaths;
+     const struct sset *local_lports;
+     struct shash *local_bindings;
+@@ -81,6 +82,7 @@ struct advertise_route_entry {
+ const struct sbrec_port_binding *route_exchange_find_port(
+     struct ovsdb_idl_index *sbrec_port_binding_by_name,
+     const struct sbrec_chassis *chassis,
++    const struct sset *active_tunnels,
+     const struct sbrec_port_binding *pb,
+     const char **dynamic_routing_port_name);
+ uint32_t advertise_route_hash(const struct in6_addr *dst, unsigned int plen);
