northd: Mark BFD sessions down when no chassis can run them.

The Southbound BFD "status" column is only ever written by the
ovn-controller that runs the session: bfd_monitor_run() picks a session
up on the chassis where the port is a bound "l3gateway" port or where
the port's "cr-" chassisredirect twin is chassis-resident.  When that
chassis goes away and no other chassis can take the gateway port over
(e.g. it was the only chassis with a bridge mapping for the provider
network), nothing is left to update the session status: the NB/SB
status stays "up" forever even though no BFD packets are exchanged any
more.  BFD monitored (ECMP) static routes then keep selecting the dead
next hop and traffic is blackholed.

Fix this in ovn-northd, which can see that no chassis owns the port:
teach bfd_table_sync() to determine the chassis that would run the
session, mirroring the ownership rule of bfd_monitor_run(), and force
both the NB and SB status to "down" when there is no such chassis,
leaving "admin_down" sessions untouched.  Route (and policy) processing
already drops BFD monitored next hops whose session is "down", so this
is enough to withdraw the routes.  Once a chassis (re)claims the
gateway port, its ovn-controller creates a fresh monitor entry and
kicks the session back into negotiation, so a northd-written "down"
does not prevent recovery.

Use the same ownership rule to maintain the SB BFD "chassis_name"
column.  It was previously derived from the port's own Port_Binding
only, but for a distributed gateway port that binding is a "patch"
port whose chassis column is never set, so chassis_name stayed empty
even for healthy sessions.  It now tracks the chassis bound to the
chassisredirect port and is cleared when the session has no owner.

Finally, make the incremental processing engine reevaluate the session
ownership when the Port_Binding of a port with a BFD session changes
its bound chassis: the "northd" engine node deliberately ignores
chassis-only binding updates, so a gateway port being released would
not have re-triggered bfd_table_sync().  Add SB Port_Binding as an
input of the "bfd_sync" engine node, with a change handler that falls
back to recompute when the "chassis" column changed on the Port_Binding
of a port (or of a chassisredirect twin of a port) that has a BFD
session, and ignores all other Port_Binding changes.

Reported-at: #320
Assisted-by: Claude Code
Signed-off-by: Premysl Kouril <premysl.kouril@gmail.com>

Author: remeq

northd/en-northd.c

@@ -566,6 +566,30 @@ bfd_sync_routes_change_handler(struct engine_node *node,
     return EN_HANDLED_UNCHANGED;
 }
 
+enum engine_input_handler_result
+bfd_sync_sb_port_binding_change_handler(struct engine_node *node, void *data)
+{
+    struct bfd_sync_data *bfd_sync_data = data;
+    struct northd_data *northd_data = engine_get_input_data("northd", node);
+    const struct sbrec_port_binding_table *sbrec_port_binding_table =
+        EN_OVSDB_GET(engine_get_input("SB_port_binding", node));
+
+    /* bfd_table_sync() derives the chassis that runs a BFD session (and
+     * with it the NB/SB BFD "status" and the SB BFD "chassis_name") from
+     * the Port_Binding of the session's logical port and of its
+     * chassisredirect twin.  If a chassis binding of such a port changed
+     * (e.g. the chassis running the session went away and the binding
+     * was released), fall back to recompute; all other Port_Binding
+     * changes are irrelevant to BFD. */
+    if (!bfd_sync_handle_sb_port_binding_changes(sbrec_port_binding_table,
+                                                 &northd_data->lr_ports,
+                                                 &bfd_sync_data->bfd_ports)) {
+        return EN_UNHANDLED;
+    }
+
+    return EN_HANDLED_UNCHANGED;
+}
+
 enum engine_node_state
 en_bfd_sync_run(struct engine_node *node, void *data)
 {

northd/en-northd.h

@@ -58,6 +58,9 @@ bfd_sync_northd_change_handler(struct engine_node *node,
 enum engine_input_handler_result
 bfd_sync_routes_change_handler(struct engine_node *node,
                                void *data OVS_UNUSED);
+enum engine_input_handler_result
+bfd_sync_sb_port_binding_change_handler(struct engine_node *node,
+                                        void *data);
 
 enum engine_node_state en_bfd_sync_run(struct engine_node *node, void *data);
 void en_bfd_sync_cleanup(void *data OVS_UNUSED);

northd/inc-proc-northd.c

@@ -345,6 +345,8 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
     engine_add_input(&en_bfd_sync, &en_routes, bfd_sync_routes_change_handler);
     engine_add_input(&en_bfd_sync, &en_route_policies, NULL);
     engine_add_input(&en_bfd_sync, &en_northd, bfd_sync_northd_change_handler);
+    engine_add_input(&en_bfd_sync, &en_sb_port_binding,
+                     bfd_sync_sb_port_binding_change_handler);
 
     engine_add_input(&en_ecmp_nexthop, &en_global_config, NULL);
     engine_add_input(&en_ecmp_nexthop, &en_routes, NULL);

northd/northd.c

@@ -11955,6 +11955,23 @@ bfd_get_connection_status(const struct nbrec_bfd *nb_bt,
     return bfd_rp ? bfd_rp->status : bfd_sr->status;
 }
 
+/* Returns the chassis that runs the BFD session for the logical router
+ * port 'op', mirroring the ownership rule implemented by bfd_monitor_run()
+ * in controller/pinctrl.c: ovn-controller runs the session if the
+ * chassisredirect twin of 'op' is bound to the chassis or if 'op' itself
+ * (an "l3gateway" port) is bound to the chassis.  Returns NULL if there is
+ * no currently bound BFD owner. */
+static const struct sbrec_chassis *
+bfd_get_session_chassis(const struct ovn_port *op)
+{
+    if (op->cr_port && op->cr_port->sb) {
+        return op->cr_port->sb->chassis;
+    }
+
+    return (op->sb && !strcmp(op->sb->type, "l3gateway"))
+           ? op->sb->chassis : NULL;
+}
+
 void
 bfd_table_sync(struct ovsdb_idl_txn *ovnsb_txn,
                const struct nbrec_bfd_table *nbrec_bfd_table,
@@ -11996,10 +12013,24 @@ bfd_table_sync(struct ovsdb_idl_txn *ovnsb_txn,
             continue;
         }
 
+        const struct sbrec_chassis *chassis = bfd_get_session_chassis(op);
+
         nbrec_bfd_set_status(nb_bt,
                              bfd_get_connection_status(nb_bt,
                                                        rp_bfd_connections,
                                                        sr_bfd_connections));
+
+        if (!chassis && strcmp(nb_bt->status, "admin_down") &&
+            strcmp(nb_bt->status, "down")) {
+            /* No chassis currently owns the BFD session for this port, e.g.
+             * the chassis that was running it disappeared and no other
+             * chassis took the port over.  ovn-controller is the only writer of
+             * the SB BFD status, so mark the session down from here;
+             * otherwise a stale "up" status would keep BFD-monitored
+             * static routes pointing at a dead next hop. */
+            nbrec_bfd_set_status(nb_bt, "down");
+        }
+
         if (!bfd_e->sb_bt) {
             int udp_src = bfd_get_unused_port(bfd_src_ports);
             if (udp_src < 0) {
@@ -12013,8 +12044,8 @@ bfd_table_sync(struct ovsdb_idl_txn *ovnsb_txn,
             sbrec_bfd_set_disc(sb_bt, 1 + random_uint32());
             sbrec_bfd_set_src_port(sb_bt, udp_src);
             sbrec_bfd_set_status(sb_bt, nb_bt->status);
-            if (op->sb->chassis) {
-                sbrec_bfd_set_chassis_name(sb_bt, op->sb->chassis->name);
+            if (chassis) {
+                sbrec_bfd_set_chassis_name(sb_bt, chassis->name);
             }
 
             int min_tx = nb_bt->n_min_tx ? nb_bt->min_tx[0] : BFD_DEF_MINTX;
@@ -12025,7 +12056,14 @@ bfd_table_sync(struct ovsdb_idl_txn *ovnsb_txn,
                                               : BFD_DEF_DETECT_MULT;
             sbrec_bfd_set_detect_mult(sb_bt, d_mult);
         } else {
-            if (strcmp(bfd_e->sb_bt->status, nb_bt->status)) {
+            if (!chassis && strcmp(nb_bt->status, "admin_down")) {
+                /* NB status has already been set to "down" above; make
+                 * the SB status follow it instead of syncing the stale
+                 * SB status back into the NB. */
+                if (strcmp(bfd_e->sb_bt->status, "down")) {
+                    sbrec_bfd_set_status(bfd_e->sb_bt, "down");
+                }
+            } else if (strcmp(bfd_e->sb_bt->status, nb_bt->status)) {
                 if (!strcmp(nb_bt->status, "admin_down") ||
                     !strcmp(bfd_e->sb_bt->status, "admin_down")) {
                     sbrec_bfd_set_status(bfd_e->sb_bt, nb_bt->status);
@@ -12035,10 +12073,10 @@ bfd_table_sync(struct ovsdb_idl_txn *ovnsb_txn,
             }
 
             build_bfd_update_sb_conf(nb_bt, bfd_e->sb_bt);
-            if (op->sb->chassis && !strcmp(op->sb->chassis->name,
-                                           bfd_e->sb_bt->chassis_name)) {
-                sbrec_bfd_set_chassis_name(bfd_e->sb_bt,
-                                           op->sb->chassis->name);
+
+            const char *chassis_name = chassis ? chassis->name : "";
+            if (strcmp(chassis_name, bfd_e->sb_bt->chassis_name)) {
+                sbrec_bfd_set_chassis_name(bfd_e->sb_bt, chassis_name);
             }
         }
 
@@ -12089,6 +12127,50 @@ build_bfd_map(const struct nbrec_bfd_table *nbrec_bfd_table,
     }
 }
 
+/* Returns false if a tracked SB Port_Binding change modified the chassis
+ * a BFD session runs on, i.e. the "chassis" column of a Port_Binding
+ * whose logical port (or, for a chassisredirect port, whose distributed
+ * port) has a BFD session changed.  bfd_table_sync() then needs to run
+ * again to reevaluate the session ownership: e.g. the chassis running
+ * the session went away and the binding was released, in which case the
+ * NB/SB BFD status must be marked "down".  Returns true if none of the
+ * tracked changes are relevant to BFD. */
+bool
+bfd_sync_handle_sb_port_binding_changes(
+    const struct sbrec_port_binding_table *sbrec_port_binding_table,
+    const struct hmap *lr_ports, const struct sset *bfd_ports)
+{
+    const struct sbrec_port_binding *pb;
+    SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (pb,
+                                               sbrec_port_binding_table) {
+        if (sbrec_port_binding_is_new(pb) ||
+            sbrec_port_binding_is_deleted(pb)) {
+            continue;
+        }
+
+        if (!sbrec_port_binding_is_updated(pb,
+                                           SBREC_PORT_BINDING_COL_CHASSIS)) {
+            continue;
+        }
+
+        const struct ovn_port *op = ovn_port_find(lr_ports,
+                                                  pb->logical_port);
+        if (!op) {
+            continue;
+        }
+
+        if (op->primary_port) {
+            op = op->primary_port;
+        }
+
+        if (bfd_is_port_running(bfd_ports, op->key)) {
+            return false;
+        }
+    }
+
+    return true;
+}
+
 void
 build_ic_learned_svc_monitors_map(
     struct hmap *ic_learned_svc_monitors_map,

northd/northd.h

@@ -1030,6 +1030,9 @@ void bfd_table_sync(struct ovsdb_idl_txn *, const struct nbrec_bfd_table *,
                     struct sset *);
 void build_bfd_map(const struct nbrec_bfd_table *,
                    const struct sbrec_bfd_table *, struct hmap *);
+bool bfd_sync_handle_sb_port_binding_changes(
+    const struct sbrec_port_binding_table *,
+    const struct hmap *lr_ports, const struct sset *bfd_ports);
 
 void build_ic_learned_svc_monitors_map(
     struct hmap *ic_learned_svc_monitors_map,

ovn-nb.xml

@@ -6704,6 +6704,13 @@ or
             </li>
           </ul>
         </p>
+        <p>
+          The status is normally reported by the <code>ovn-controller</code>
+          running the BFD session.  If no chassis currently owns the session
+          for the logical port (e.g. the gateway-port binding was released),
+          <code>ovn-northd</code> sets the status of a session that is not
+          <code>admin_down</code> to <code>down</code>.
+        </p>
       </column>
     </group>
   </table>

ovn-sb.xml

@@ -5411,7 +5411,9 @@ tcp.flags = RST;
       </column>
 
       <column name="chassis_name">
-        The name of the chassis where the logical port is bound.
+        The name of the chassis running the BFD session.  For a
+        distributed gateway port this is the chassis its chassisredirect
+        port is bound to.  Empty when there is no currently bound owner.
       </column>
 
       <column name="options">

tests/ovn-inc-proc-graph-dump.at

@@ -164,6 +164,7 @@ digraph "Incremental-Processing-Engine" {
 	routes -> bfd_sync [[label="bfd_sync_routes_change_handler"]];
 	route_policies -> bfd_sync [[label=""]];
 	northd -> bfd_sync [[label="bfd_sync_northd_change_handler"]];
+	SB_port_binding -> bfd_sync [[label="bfd_sync_sb_port_binding_change_handler"]];
 	SB_learned_route [[style=filled, shape=box, fillcolor=white, label="SB_learned_route"]];
 	learned_route_sync [[style=filled, shape=box, fillcolor=white, label="learned_route_sync"]];
 	SB_learned_route -> learned_route_sync [[label="learned_route_sync_sb_learned_route_change_handler"]];

tests/ovn-northd.at

@@ -4419,8 +4419,15 @@ wait_row_count bfd 5
 
 # Simulate BFD up in Southbound for an automatically created entry.
 # This entry is referenced so the state in the Northbound should also
-# become "up".
+# become "up".  A BFD session can only be up if some chassis runs it, so
+# turn r0-sw2 into a gateway port and bind its chassisredirect port
+# first, like ovn-controller would do.
 wait_column down nb:bfd status logical_port=r0-sw2
+check ovn-sbctl chassis-add hv1 geneve 127.0.0.1
+check ovn-nbctl --wait=sb lrp-set-gateway-chassis r0-sw2 hv1 10
+hv1_uuid=$(fetch_column chassis _uuid name=hv1)
+check ovn-sbctl set port_binding cr-r0-sw2 chassis=$hv1_uuid
+wait_column hv1 bfd chassis_name logical_port=r0-sw2
 bfd2_uuid=$(fetch_column bfd _uuid logical_port=r0-sw2)
 check ovn-sbctl set bfd $bfd2_uuid status=up
 wait_column up nb:bfd status logical_port=r0-sw2
@@ -4468,6 +4475,122 @@ OVN_CLEANUP_NORTHD
 AT_CLEANUP
 ])
 
+OVN_FOR_EACH_NORTHD_NO_HV([
+AT_SETUP([BFD sessions marked down when no chassis can run them])
+AT_KEYWORDS([northd-bfd])
+ovn_start
+
+check ovn-sbctl chassis-add hv1 geneve 127.0.0.1
+
+check ovn-nbctl lr-add r0
+check ovn-nbctl lrp-add r0 r0-public 00:00:20:20:12:13 172.16.0.1/24
+check ovn-nbctl ls-add public
+check ovn-nbctl lsp-add-router-port public public-r0 r0-public
+check ovn-nbctl --wait=sb lrp-set-gateway-chassis r0-public hv1 10
+
+bfd_uuid=$(ovn-nbctl create bfd logical_port=r0-public dst_ip=172.16.0.50)
+check test -n "$bfd_uuid"
+check ovn-nbctl --bfd=$bfd_uuid lr-route-add r0 100.0.0.0/8 172.16.0.50
+
+# The gateway port is not bound to any chassis yet, so no chassis runs
+# the BFD session: both NB and SB status must be "down".
+wait_column down nb:bfd status logical_port=r0-public
+wait_column down bfd status logical_port=r0-public
+check_column "" bfd chassis_name logical_port=r0-public
+
+# Simulate hv1 claiming the chassisredirect port and the BFD session
+# coming up, like ovn-controller would do.
+hv1_uuid=$(fetch_column chassis _uuid name=hv1)
+check ovn-sbctl set port_binding cr-r0-public chassis=$hv1_uuid
+wait_column hv1 bfd chassis_name logical_port=r0-public
+
+sb_bfd_uuid=$(fetch_column bfd _uuid logical_port=r0-public)
+check ovn-sbctl set bfd $sb_bfd_uuid status=up
+wait_column up nb:bfd status logical_port=r0-public
+
+# The BFD monitored route is programmed while the session is up.
+OVS_WAIT_UNTIL([ovn-sbctl dump-flows r0 > r0flows dnl
+&& grep -q "ip4.dst == 100.0.0.0/8" r0flows])
+
+# Release the chassisredirect binding while leaving the Chassis row intact.
+# This specifically exercises the incremental Port_Binding.chassis input.
+# The session must be marked "down" in both databases and the BFD monitored
+# route must be withdrawn.
+check ovn-sbctl clear port_binding cr-r0-public chassis
+wait_column down nb:bfd status logical_port=r0-public
+wait_column down bfd status logical_port=r0-public
+check_column "" bfd chassis_name logical_port=r0-public
+OVS_WAIT_UNTIL([ovn-sbctl dump-flows r0 > r0flows dnl
+&& ! grep -q "ip4.dst == 100.0.0.0/8" r0flows])
+
+# Give the session an owner again: northd must track the owning chassis
+# but must not flip the status to "up" on its own; that is up to
+# ovn-controller's BFD negotiation.
+check ovn-sbctl set port_binding cr-r0-public chassis=$hv1_uuid
+wait_column hv1 bfd chassis_name logical_port=r0-public
+check ovn-nbctl --wait=sb sync
+check_column down nb:bfd status logical_port=r0-public
+check_column down bfd status logical_port=r0-public
+
+# Once "ovn-controller" reports the session up again, the status must
+# propagate to the Northbound as usual.
+check ovn-sbctl set bfd $sb_bfd_uuid status=up
+wait_column up nb:bfd status logical_port=r0-public
+
+# An "admin_down" session must be left untouched when it loses its
+# owner.
+route_uuid=$(fetch_column nb:logical_router_static_route _uuid \
+             ip_prefix="100.0.0.0/8")
+check ovn-nbctl clear logical_router_static_route $route_uuid bfd
+wait_column admin_down nb:bfd status logical_port=r0-public
+wait_column admin_down bfd status logical_port=r0-public
+check ovn-sbctl clear port_binding cr-r0-public chassis
+check ovn-nbctl --wait=sb sync
+check_column admin_down nb:bfd status logical_port=r0-public
+check_column admin_down bfd status logical_port=r0-public
+check_column "" bfd chassis_name logical_port=r0-public
+
+OVN_CLEANUP_NORTHD
+AT_CLEANUP
+])
+
+OVN_FOR_EACH_NORTHD_NO_HV([
+AT_SETUP([BFD ownership on an l3gateway port])
+AT_KEYWORDS([northd-bfd])
+ovn_start
+
+check ovn-sbctl chassis-add hv1 geneve 127.0.0.1
+hv1_uuid=$(fetch_column chassis _uuid name=hv1)
+
+check ovn-nbctl lr-add r0
+check ovn-nbctl set logical_router r0 options:chassis=hv1
+check ovn-nbctl lrp-add r0 r0-public 00:00:20:20:12:13 172.16.0.1/24
+check ovn-nbctl ls-add public
+check ovn-nbctl lsp-add-router-port public public-r0 r0-public
+
+bfd_uuid=$(ovn-nbctl create bfd logical_port=r0-public dst_ip=172.16.0.50)
+check test -n "$bfd_uuid"
+check ovn-nbctl --bfd=$bfd_uuid lr-route-add r0 100.0.0.0/8 172.16.0.50
+
+wait_column l3gateway port_binding type logical_port=r0-public
+check ovn-sbctl set port_binding r0-public chassis=$hv1_uuid
+wait_column hv1 bfd chassis_name logical_port=r0-public
+
+sb_bfd_uuid=$(fetch_column bfd _uuid logical_port=r0-public)
+check ovn-sbctl set bfd $sb_bfd_uuid status=up
+wait_column up nb:bfd status logical_port=r0-public
+
+# The direct l3gateway binding is the owner.  Releasing it must invalidate
+# the stale status through the same incremental Port_Binding input.
+check ovn-sbctl clear port_binding r0-public chassis
+wait_column down nb:bfd status logical_port=r0-public
+wait_column down bfd status logical_port=r0-public
+check_column "" bfd chassis_name logical_port=r0-public
+
+OVN_CLEANUP_NORTHD
+AT_CLEANUP
+])
+
 OVN_FOR_EACH_NORTHD_NO_HV([
 AT_SETUP([ovn -- check CoPP config])
 AT_KEYWORDS([northd-CoPP])