diff mbox series

[ovs-dev,v3] northd: Add change handler for FDB updates.

Message ID 20240729095612.107553-1-naveen.yerramneni@nutanix.com
State Accepted
Delegated to: Numan Siddique
Headers show
Series [ovs-dev,v3] northd: Add change handler for FDB updates. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/github-robot-_ovn-kubernetes success github build: passed

Commit Message

Naveen Yerramneni July 29, 2024, 9:56 a.m. UTC 
This change avoids northd full recompute for FDB updates.
Also, fixed incremental processing for port deletion to
delete all fdb entries associated to a port.

Signed-off-by: Naveen Yerramneni <naveen.yerramneni@nutanix.com>
---
v2:
  - Addressed review comments from Ales.
  - Fixed incremental processing for port deletion to
    delete all fdb entries associated to a port.
v3:
  - Addressed review comments from Ales.
---
 northd/en-northd.c       | 37 +++++++++++++++++++++++++++++++++++++
 northd/en-northd.h       |  1 +
 northd/inc-proc-northd.c |  2 +-
 northd/northd.c          | 18 ++++++++----------
 northd/northd.h          |  3 +++
 5 files changed, 50 insertions(+), 11 deletions(-)

Comments

Ales Musil July 31, 2024, 5:36 a.m. UTC | #1 
On Mon, Jul 29, 2024 at 11:56 AM Naveen Yerramneni <
naveen.yerramneni@nutanix.com> wrote:

> This change avoids northd full recompute for FDB updates.
> Also, fixed incremental processing for port deletion to
> delete all fdb entries associated to a port.
>
> Signed-off-by: Naveen Yerramneni <naveen.yerramneni@nutanix.com>
> ---
> v2:
>   - Addressed review comments from Ales.
>   - Fixed incremental processing for port deletion to
>     delete all fdb entries associated to a port.
> v3:
>   - Addressed review comments from Ales.
> ---
>  northd/en-northd.c       | 37 +++++++++++++++++++++++++++++++++++++
>  northd/en-northd.h       |  1 +
>  northd/inc-proc-northd.c |  2 +-
>  northd/northd.c          | 18 ++++++++----------
>  northd/northd.h          |  3 +++
>  5 files changed, 50 insertions(+), 11 deletions(-)
>
> diff --git a/northd/en-northd.c b/northd/en-northd.c
> index 4479b4aff..99e6f72fd 100644
> --- a/northd/en-northd.c
> +++ b/northd/en-northd.c
> @@ -259,3 +259,40 @@ en_northd_clear_tracked_data(void *data_)
>      struct northd_data *data = data_;
>      destroy_northd_data_tracked_changes(data);
>  }
> +
> +bool
> +sb_fdb_change_handler(struct engine_node *node, void *data)
> +{
> +    struct northd_data *nd = data;
> +    const struct sbrec_fdb_table *sbrec_fdb_table =
> +        EN_OVSDB_GET(engine_get_input("SB_fdb", node));
> +
> +    /* check if changed rows are stale and delete them */
> +    const struct sbrec_fdb *fdb_e, *fdb_prev_del = NULL;
> +    SBREC_FDB_TABLE_FOR_EACH_TRACKED (fdb_e, sbrec_fdb_table) {
> +        if (sbrec_fdb_is_deleted(fdb_e)) {
> +            continue;
> +        }
> +
> +        if (fdb_prev_del) {
> +            sbrec_fdb_delete(fdb_prev_del);
> +        }
> +
> +        fdb_prev_del = fdb_e;
> +        struct ovn_datapath *od
> +            = ovn_datapath_find_by_key(&nd->ls_datapaths.datapaths,
> +                                          fdb_e->dp_key);
> +        if (od) {
> +            if (ovn_tnlid_present(&od->port_tnlids, fdb_e->port_key)) {
> +                fdb_prev_del = NULL;
> +            }
> +        }
> +    }
> +
> +    if (fdb_prev_del) {
> +        sbrec_fdb_delete(fdb_prev_del);
> +    }
> +
> +    engine_set_node_state(node, EN_UPDATED);
> +    return true;
> +}
> diff --git a/northd/en-northd.h b/northd/en-northd.h
> index 9b7bda32a..9c722e401 100644
> --- a/northd/en-northd.h
> +++ b/northd/en-northd.h
> @@ -19,5 +19,6 @@ bool northd_nb_logical_switch_handler(struct engine_node
> *, void *data);
>  bool northd_nb_logical_router_handler(struct engine_node *, void *data);
>  bool northd_sb_port_binding_handler(struct engine_node *, void *data);
>  bool northd_lb_data_handler(struct engine_node *, void *data);
> +bool sb_fdb_change_handler(struct engine_node *node, void *data);
>
>  #endif /* EN_NORTHD_H */
> diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
> index d56e9783a..213e6d88a 100644
> --- a/northd/inc-proc-northd.c
> +++ b/northd/inc-proc-northd.c
> @@ -189,7 +189,7 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
>      engine_add_input(&en_northd, &en_sb_ha_chassis_group, NULL);
>      engine_add_input(&en_northd, &en_sb_ip_multicast, NULL);
>      engine_add_input(&en_northd, &en_sb_service_monitor, NULL);
> -    engine_add_input(&en_northd, &en_sb_fdb, NULL);
> +    engine_add_input(&en_northd, &en_sb_fdb, sb_fdb_change_handler);
>      engine_add_input(&en_northd, &en_sb_static_mac_binding, NULL);
>      engine_add_input(&en_northd, &en_sb_chassis_template_var, NULL);
>      engine_add_input(&en_northd, &en_global_config,
> diff --git a/northd/northd.c b/northd/northd.c
> index 6898daa00..475e79db1 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -496,7 +496,7 @@ ovn_datapath_find(const struct hmap *datapaths,
>      return ovn_datapath_find_(datapaths, uuid);
>  }
>
> -static struct ovn_datapath *
> +struct ovn_datapath *
>  ovn_datapath_find_by_key(struct hmap *datapaths, uint32_t dp_key)
>  {
>      struct ovn_datapath *od;
> @@ -3292,7 +3292,7 @@ cleanup_stale_fdb_entries(const struct
> sbrec_fdb_table *sbrec_fdb_table,
>  }
>
>  static void
> -delete_fdb_entry(struct ovsdb_idl_index *sbrec_fdb_by_dp_and_port,
> +delete_fdb_entries(struct ovsdb_idl_index *sbrec_fdb_by_dp_and_port,
>                   uint32_t dp_key, uint32_t port_key)
>  {
>      struct sbrec_fdb *target =
> @@ -3300,13 +3300,11 @@ delete_fdb_entry(struct ovsdb_idl_index
> *sbrec_fdb_by_dp_and_port,
>      sbrec_fdb_index_set_dp_key(target, dp_key);
>      sbrec_fdb_index_set_port_key(target, port_key);
>
> -    struct sbrec_fdb *fdb_e =
> sbrec_fdb_index_find(sbrec_fdb_by_dp_and_port,
> -                                                   target);
> -    sbrec_fdb_index_destroy_row(target);
> -
> -    if (fdb_e) {
> +    struct sbrec_fdb *fdb_e;
> +    SBREC_FDB_FOR_EACH_EQUAL (fdb_e, target, sbrec_fdb_by_dp_and_port) {
>          sbrec_fdb_delete(fdb_e);
>      }
> +    sbrec_fdb_index_destroy_row(target);
>  }
>
>  struct service_monitor_info {
> @@ -4587,8 +4585,8 @@ ls_handle_lsp_changes(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
>              add_op_to_northd_tracked_ports(&trk_lsps->updated, op);
>
>              if (old_tunnel_key != op->tunnel_key) {
> -                delete_fdb_entry(ni->sbrec_fdb_by_dp_and_port,
> od->tunnel_key,
> -                                 old_tunnel_key);
> +                delete_fdb_entries(ni->sbrec_fdb_by_dp_and_port,
> +                                   od->tunnel_key, old_tunnel_key);
>              }
>          }
>          op->visited = true;
> @@ -4609,7 +4607,7 @@ ls_handle_lsp_changes(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
>              hmap_remove(&nd->ls_ports, &op->key_node);
>              hmap_remove(&od->ports, &op->dp_node);
>              sbrec_port_binding_delete(op->sb);
> -            delete_fdb_entry(ni->sbrec_fdb_by_dp_and_port, od->tunnel_key,
> +            delete_fdb_entries(ni->sbrec_fdb_by_dp_and_port,
> od->tunnel_key,
>                                  op->tunnel_key);
>          }
>      }
> diff --git a/northd/northd.h b/northd/northd.h
> index d4a8d75ab..26fe24df3 100644
> --- a/northd/northd.h
> +++ b/northd/northd.h
> @@ -87,6 +87,9 @@ ods_size(const struct ovn_datapaths *datapaths)
>      return hmap_count(&datapaths->datapaths);
>  }
>
> +struct ovn_datapath *
> +ovn_datapath_find_by_key(struct hmap *datapaths, uint32_t dp_key);
> +
>  bool od_has_lb_vip(const struct ovn_datapath *od);
>
>  struct tracked_ovn_ports {
> --
> 2.36.6
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
Looks good to me, thanks!

Acked-by: Ales Musil <amusil@redhat.com>
Numan Siddique Aug. 8, 2024, 6:37 p.m. UTC | #2 
On Wed, Jul 31, 2024 at 1:37 AM Ales Musil <amusil@redhat.com> wrote:
>
> On Mon, Jul 29, 2024 at 11:56 AM Naveen Yerramneni <
> naveen.yerramneni@nutanix.com> wrote:
>
> > This change avoids northd full recompute for FDB updates.
> > Also, fixed incremental processing for port deletion to
> > delete all fdb entries associated to a port.
> >
> > Signed-off-by: Naveen Yerramneni <naveen.yerramneni@nutanix.com>

Thanks for the patch.  I applied this patch to the main with the below changes.

----------------------------------
diff --git a/northd/en-northd.c b/northd/en-northd.c
index e3d2d397e3..643827970c 100644
--- a/northd/en-northd.c
+++ b/northd/en-northd.c
@@ -261,7 +261,7 @@ en_northd_clear_tracked_data(void *data_)
 }

 bool
-sb_fdb_change_handler(struct engine_node *node, void *data)
+northd_sb_fdb_change_handler(struct engine_node *node, void *data)
 {
     struct northd_data *nd = data;
     const struct sbrec_fdb_table *sbrec_fdb_table =
@@ -293,6 +293,5 @@ sb_fdb_change_handler(struct engine_node *node, void *data)
         sbrec_fdb_delete(fdb_prev_del);
     }

-    engine_set_node_state(node, EN_UPDATED);
     return true;
 }
diff --git a/northd/en-northd.h b/northd/en-northd.h
index 9c722e4017..fb0e3e4e41 100644
--- a/northd/en-northd.h
+++ b/northd/en-northd.h
@@ -19,6 +19,6 @@ bool northd_nb_logical_switch_handler(struct
engine_node *, void *data);
 bool northd_nb_logical_router_handler(struct engine_node *, void *data);
 bool northd_sb_port_binding_handler(struct engine_node *, void *data);
 bool northd_lb_data_handler(struct engine_node *, void *data);
-bool sb_fdb_change_handler(struct engine_node *node, void *data);
+bool northd_sb_fdb_change_handler(struct engine_node *node, void *data);

 #endif /* EN_NORTHD_H */
diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
index 5dfebea019..c574d1f060 100644
--- a/northd/inc-proc-northd.c
+++ b/northd/inc-proc-northd.c
@@ -195,9 +195,9 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
     engine_add_input(&en_northd, &en_sb_ha_chassis_group, NULL);
     engine_add_input(&en_northd, &en_sb_ip_multicast, NULL);
     engine_add_input(&en_northd, &en_sb_service_monitor, NULL);
-    engine_add_input(&en_northd, &en_sb_fdb, sb_fdb_change_handler);
     engine_add_input(&en_northd, &en_sb_static_mac_binding, NULL);
     engine_add_input(&en_northd, &en_sb_chassis_template_var, NULL);
+    engine_add_input(&en_northd, &en_sb_fdb, northd_sb_fdb_change_handler);
     engine_add_input(&en_northd, &en_global_config,
                      northd_global_config_handler);
------------------------------------------------------


Please see below for some comments and why I did the above changes.

Numan

> > ---
> > v2:
> >   - Addressed review comments from Ales.
> >   - Fixed incremental processing for port deletion to
> >     delete all fdb entries associated to a port.
> > v3:
> >   - Addressed review comments from Ales.
> > ---
> >  northd/en-northd.c       | 37 +++++++++++++++++++++++++++++++++++++
> >  northd/en-northd.h       |  1 +
> >  northd/inc-proc-northd.c |  2 +-
> >  northd/northd.c          | 18 ++++++++----------
> >  northd/northd.h          |  3 +++
> >  5 files changed, 50 insertions(+), 11 deletions(-)
> >
> > diff --git a/northd/en-northd.c b/northd/en-northd.c
> > index 4479b4aff..99e6f72fd 100644
> > --- a/northd/en-northd.c
> > +++ b/northd/en-northd.c
> > @@ -259,3 +259,40 @@ en_northd_clear_tracked_data(void *data_)
> >      struct northd_data *data = data_;
> >      destroy_northd_data_tracked_changes(data);
> >  }
> > +
> > +bool
> > +sb_fdb_change_handler(struct engine_node *node, void *data)
> > +{
> > +    struct northd_data *nd = data;
> > +    const struct sbrec_fdb_table *sbrec_fdb_table =
> > +        EN_OVSDB_GET(engine_get_input("SB_fdb", node));
> > +
> > +    /* check if changed rows are stale and delete them */
> > +    const struct sbrec_fdb *fdb_e, *fdb_prev_del = NULL;
> > +    SBREC_FDB_TABLE_FOR_EACH_TRACKED (fdb_e, sbrec_fdb_table) {
> > +        if (sbrec_fdb_is_deleted(fdb_e)) {
> > +            continue;
> > +        }
> > +
> > +        if (fdb_prev_del) {
> > +            sbrec_fdb_delete(fdb_prev_del);
> > +        }
> > +
> > +        fdb_prev_del = fdb_e;
> > +        struct ovn_datapath *od
> > +            = ovn_datapath_find_by_key(&nd->ls_datapaths.datapaths,
> > +                                          fdb_e->dp_key);
> > +        if (od) {
> > +            if (ovn_tnlid_present(&od->port_tnlids, fdb_e->port_key)) {
> > +                fdb_prev_del = NULL;
> > +            }
> > +        }
> > +    }
> > +
> > +    if (fdb_prev_del) {
> > +        sbrec_fdb_delete(fdb_prev_del);
> > +    }
> > +
> > +    engine_set_node_state(node, EN_UPDATED);

There is a small problem above when you update the node.
When northd node gets updated,  its dependent nodes - ls_stateful, en_lflow etc
handlers are called and they are falling back to recompute because we are not
providing any tracking information.

Your patch avoids recompute of northd engine node but not lflow engine node.

Since the above handler is only cleaning up the stale FDB entries and
northd engine
data is not modified with it,  I removed the above line to update the
northd engine node.
With this, other engine nodes don't fall back to recompute for SB FDB changes.

I also changed the handler name from 'sb_fdb_change_handler' to
'northd_sb_fdb_change_handler'.

Thanks
Numan

> > +    return true;
> > +}
> > diff --git a/northd/en-northd.h b/northd/en-northd.h
> > index 9b7bda32a..9c722e401 100644
> > --- a/northd/en-northd.h
> > +++ b/northd/en-northd.h
> > @@ -19,5 +19,6 @@ bool northd_nb_logical_switch_handler(struct engine_node
> > *, void *data);
> >  bool northd_nb_logical_router_handler(struct engine_node *, void *data);
> >  bool northd_sb_port_binding_handler(struct engine_node *, void *data);
> >  bool northd_lb_data_handler(struct engine_node *, void *data);
> > +bool sb_fdb_change_handler(struct engine_node *node, void *data);
> >
> >  #endif /* EN_NORTHD_H */
> > diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
> > index d56e9783a..213e6d88a 100644
> > --- a/northd/inc-proc-northd.c
> > +++ b/northd/inc-proc-northd.c
> > @@ -189,7 +189,7 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
> >      engine_add_input(&en_northd, &en_sb_ha_chassis_group, NULL);
> >      engine_add_input(&en_northd, &en_sb_ip_multicast, NULL);
> >      engine_add_input(&en_northd, &en_sb_service_monitor, NULL);
> > -    engine_add_input(&en_northd, &en_sb_fdb, NULL);
> > +    engine_add_input(&en_northd, &en_sb_fdb, sb_fdb_change_handler);
> >      engine_add_input(&en_northd, &en_sb_static_mac_binding, NULL);
> >      engine_add_input(&en_northd, &en_sb_chassis_template_var, NULL);
> >      engine_add_input(&en_northd, &en_global_config,
> > diff --git a/northd/northd.c b/northd/northd.c
> > index 6898daa00..475e79db1 100644
> > --- a/northd/northd.c
> > +++ b/northd/northd.c
> > @@ -496,7 +496,7 @@ ovn_datapath_find(const struct hmap *datapaths,
> >      return ovn_datapath_find_(datapaths, uuid);
> >  }
> >
> > -static struct ovn_datapath *
> > +struct ovn_datapath *
> >  ovn_datapath_find_by_key(struct hmap *datapaths, uint32_t dp_key)
> >  {
> >      struct ovn_datapath *od;
> > @@ -3292,7 +3292,7 @@ cleanup_stale_fdb_entries(const struct
> > sbrec_fdb_table *sbrec_fdb_table,
> >  }
> >
> >  static void
> > -delete_fdb_entry(struct ovsdb_idl_index *sbrec_fdb_by_dp_and_port,
> > +delete_fdb_entries(struct ovsdb_idl_index *sbrec_fdb_by_dp_and_port,
> >                   uint32_t dp_key, uint32_t port_key)
> >  {
> >      struct sbrec_fdb *target =
> > @@ -3300,13 +3300,11 @@ delete_fdb_entry(struct ovsdb_idl_index
> > *sbrec_fdb_by_dp_and_port,
> >      sbrec_fdb_index_set_dp_key(target, dp_key);
> >      sbrec_fdb_index_set_port_key(target, port_key);
> >
> > -    struct sbrec_fdb *fdb_e =
> > sbrec_fdb_index_find(sbrec_fdb_by_dp_and_port,
> > -                                                   target);
> > -    sbrec_fdb_index_destroy_row(target);
> > -
> > -    if (fdb_e) {
> > +    struct sbrec_fdb *fdb_e;
> > +    SBREC_FDB_FOR_EACH_EQUAL (fdb_e, target, sbrec_fdb_by_dp_and_port) {
> >          sbrec_fdb_delete(fdb_e);
> >      }
> > +    sbrec_fdb_index_destroy_row(target);
> >  }
> >
> >  struct service_monitor_info {
> > @@ -4587,8 +4585,8 @@ ls_handle_lsp_changes(struct ovsdb_idl_txn
> > *ovnsb_idl_txn,
> >              add_op_to_northd_tracked_ports(&trk_lsps->updated, op);
> >
> >              if (old_tunnel_key != op->tunnel_key) {
> > -                delete_fdb_entry(ni->sbrec_fdb_by_dp_and_port,
> > od->tunnel_key,
> > -                                 old_tunnel_key);
> > +                delete_fdb_entries(ni->sbrec_fdb_by_dp_and_port,
> > +                                   od->tunnel_key, old_tunnel_key);
> >              }
> >          }
> >          op->visited = true;
> > @@ -4609,7 +4607,7 @@ ls_handle_lsp_changes(struct ovsdb_idl_txn
> > *ovnsb_idl_txn,
> >              hmap_remove(&nd->ls_ports, &op->key_node);
> >              hmap_remove(&od->ports, &op->dp_node);
> >              sbrec_port_binding_delete(op->sb);
> > -            delete_fdb_entry(ni->sbrec_fdb_by_dp_and_port, od->tunnel_key,
> > +            delete_fdb_entries(ni->sbrec_fdb_by_dp_and_port,
> > od->tunnel_key,
> >                                  op->tunnel_key);
> >          }
> >      }
> > diff --git a/northd/northd.h b/northd/northd.h
> > index d4a8d75ab..26fe24df3 100644
> > --- a/northd/northd.h
> > +++ b/northd/northd.h
> > @@ -87,6 +87,9 @@ ods_size(const struct ovn_datapaths *datapaths)
> >      return hmap_count(&datapaths->datapaths);
> >  }
> >
> > +struct ovn_datapath *
> > +ovn_datapath_find_by_key(struct hmap *datapaths, uint32_t dp_key);
> > +
> >  bool od_has_lb_vip(const struct ovn_datapath *od);
> >
> >  struct tracked_ovn_ports {
> > --
> > 2.36.6
> >
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
> >
> Looks good to me, thanks!
>
> Acked-by: Ales Musil <amusil@redhat.com>
>
> --
>
> Ales Musil
>
> Senior Software Engineer - OVN Core
>
> Red Hat EMEA <https://www.redhat.com>
>
> amusil@redhat.com
> <https://red.ht/sig>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff mbox series

Patch

diff --git a/northd/en-northd.c b/northd/en-northd.c
index 4479b4aff..99e6f72fd 100644
--- a/northd/en-northd.c
+++ b/northd/en-northd.c
@@ -259,3 +259,40 @@  en_northd_clear_tracked_data(void *data_)
     struct northd_data *data = data_;
     destroy_northd_data_tracked_changes(data);
 }
+
+bool
+sb_fdb_change_handler(struct engine_node *node, void *data)
+{
+    struct northd_data *nd = data;
+    const struct sbrec_fdb_table *sbrec_fdb_table =
+        EN_OVSDB_GET(engine_get_input("SB_fdb", node));
+
+    /* check if changed rows are stale and delete them */
+    const struct sbrec_fdb *fdb_e, *fdb_prev_del = NULL;
+    SBREC_FDB_TABLE_FOR_EACH_TRACKED (fdb_e, sbrec_fdb_table) {
+        if (sbrec_fdb_is_deleted(fdb_e)) {
+            continue;
+        }
+
+        if (fdb_prev_del) {
+            sbrec_fdb_delete(fdb_prev_del);
+        }
+
+        fdb_prev_del = fdb_e;
+        struct ovn_datapath *od
+            = ovn_datapath_find_by_key(&nd->ls_datapaths.datapaths,
+                                          fdb_e->dp_key);
+        if (od) {
+            if (ovn_tnlid_present(&od->port_tnlids, fdb_e->port_key)) {
+                fdb_prev_del = NULL;
+            }
+        }
+    }
+
+    if (fdb_prev_del) {
+        sbrec_fdb_delete(fdb_prev_del);
+    }
+
+    engine_set_node_state(node, EN_UPDATED);
+    return true;
+}
diff --git a/northd/en-northd.h b/northd/en-northd.h
index 9b7bda32a..9c722e401 100644
--- a/northd/en-northd.h
+++ b/northd/en-northd.h
@@ -19,5 +19,6 @@  bool northd_nb_logical_switch_handler(struct engine_node *, void *data);
 bool northd_nb_logical_router_handler(struct engine_node *, void *data);
 bool northd_sb_port_binding_handler(struct engine_node *, void *data);
 bool northd_lb_data_handler(struct engine_node *, void *data);
+bool sb_fdb_change_handler(struct engine_node *node, void *data);
 
 #endif /* EN_NORTHD_H */
diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
index d56e9783a..213e6d88a 100644
--- a/northd/inc-proc-northd.c
+++ b/northd/inc-proc-northd.c
@@ -189,7 +189,7 @@  void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
     engine_add_input(&en_northd, &en_sb_ha_chassis_group, NULL);
     engine_add_input(&en_northd, &en_sb_ip_multicast, NULL);
     engine_add_input(&en_northd, &en_sb_service_monitor, NULL);
-    engine_add_input(&en_northd, &en_sb_fdb, NULL);
+    engine_add_input(&en_northd, &en_sb_fdb, sb_fdb_change_handler);
     engine_add_input(&en_northd, &en_sb_static_mac_binding, NULL);
     engine_add_input(&en_northd, &en_sb_chassis_template_var, NULL);
     engine_add_input(&en_northd, &en_global_config,
diff --git a/northd/northd.c b/northd/northd.c
index 6898daa00..475e79db1 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -496,7 +496,7 @@  ovn_datapath_find(const struct hmap *datapaths,
     return ovn_datapath_find_(datapaths, uuid);
 }
 
-static struct ovn_datapath *
+struct ovn_datapath *
 ovn_datapath_find_by_key(struct hmap *datapaths, uint32_t dp_key)
 {
     struct ovn_datapath *od;
@@ -3292,7 +3292,7 @@  cleanup_stale_fdb_entries(const struct sbrec_fdb_table *sbrec_fdb_table,
 }
 
 static void
-delete_fdb_entry(struct ovsdb_idl_index *sbrec_fdb_by_dp_and_port,
+delete_fdb_entries(struct ovsdb_idl_index *sbrec_fdb_by_dp_and_port,
                  uint32_t dp_key, uint32_t port_key)
 {
     struct sbrec_fdb *target =
@@ -3300,13 +3300,11 @@  delete_fdb_entry(struct ovsdb_idl_index *sbrec_fdb_by_dp_and_port,
     sbrec_fdb_index_set_dp_key(target, dp_key);
     sbrec_fdb_index_set_port_key(target, port_key);
 
-    struct sbrec_fdb *fdb_e = sbrec_fdb_index_find(sbrec_fdb_by_dp_and_port,
-                                                   target);
-    sbrec_fdb_index_destroy_row(target);
-
-    if (fdb_e) {
+    struct sbrec_fdb *fdb_e;
+    SBREC_FDB_FOR_EACH_EQUAL (fdb_e, target, sbrec_fdb_by_dp_and_port) {
         sbrec_fdb_delete(fdb_e);
     }
+    sbrec_fdb_index_destroy_row(target);
 }
 
 struct service_monitor_info {
@@ -4587,8 +4585,8 @@  ls_handle_lsp_changes(struct ovsdb_idl_txn *ovnsb_idl_txn,
             add_op_to_northd_tracked_ports(&trk_lsps->updated, op);
 
             if (old_tunnel_key != op->tunnel_key) {
-                delete_fdb_entry(ni->sbrec_fdb_by_dp_and_port, od->tunnel_key,
-                                 old_tunnel_key);
+                delete_fdb_entries(ni->sbrec_fdb_by_dp_and_port,
+                                   od->tunnel_key, old_tunnel_key);
             }
         }
         op->visited = true;
@@ -4609,7 +4607,7 @@  ls_handle_lsp_changes(struct ovsdb_idl_txn *ovnsb_idl_txn,
             hmap_remove(&nd->ls_ports, &op->key_node);
             hmap_remove(&od->ports, &op->dp_node);
             sbrec_port_binding_delete(op->sb);
-            delete_fdb_entry(ni->sbrec_fdb_by_dp_and_port, od->tunnel_key,
+            delete_fdb_entries(ni->sbrec_fdb_by_dp_and_port, od->tunnel_key,
                                 op->tunnel_key);
         }
     }
diff --git a/northd/northd.h b/northd/northd.h
index d4a8d75ab..26fe24df3 100644
--- a/northd/northd.h
+++ b/northd/northd.h
@@ -87,6 +87,9 @@  ods_size(const struct ovn_datapaths *datapaths)
     return hmap_count(&datapaths->datapaths);
 }
 
+struct ovn_datapath *
+ovn_datapath_find_by_key(struct hmap *datapaths, uint32_t dp_key);
+
 bool od_has_lb_vip(const struct ovn_datapath *od);
 
 struct tracked_ovn_ports {