On Mon, Jul 29, 2024 at 11:56 AM Naveen Yerramneni <
[email protected]> 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 <[email protected]>
> ---
> 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
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
Looks good to me, thanks!

Acked-by: Ales Musil <[email protected]>

-- 

Ales Musil

Senior Software Engineer - OVN Core

Red Hat EMEA <https://www.redhat.com>

[email protected]
<https://red.ht/sig>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to