On Wed, Jul 24, 2024 at 10:18 AM Naveen Yerramneni < [email protected]> wrote:
> > > > On 24 Jul 2024, at 12:02 PM, Ales Musil <[email protected]> wrote: > > > > CAUTION: External Email > > > > > > On Wed, Jul 17, 2024 at 12:46 PM Naveen Yerramneni < > [email protected]> wrote: > > This change avoids northd full recompute for FDB updates. > > > > Signed-off-by: Naveen Yerramneni <[email protected]> > > > > Hi Naveen, > > > > thank you for the patch, I have one comment down below. > > --- > > northd/en-northd.c | 11 +++++++++++ > > northd/en-northd.h | 1 + > > northd/inc-proc-northd.c | 2 +- > > northd/northd.c | 2 +- > > northd/northd.h | 2 ++ > > 5 files changed, 16 insertions(+), 2 deletions(-) > > > > diff --git a/northd/en-northd.c b/northd/en-northd.c > > index 4479b4aff..4f84a8717 100644 > > --- a/northd/en-northd.c > > +++ b/northd/en-northd.c > > @@ -259,3 +259,14 @@ 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)); > > + cleanup_stale_fdb_entries(sbrec_fdb_table, > &nd->ls_datapaths.datapaths); > > > > The cleanup walks through all FDB entries that are in the DB, how about > > using the "SBREC_FDB_TABLE_FOR_EACH_TRACKED()" and check > > only the changed rows if they are actually stale or not? > > Hi Ales, > > Thanks for the review! > cleanup_stale_fdb_entries() is called from ovnnb_db_run() as well. > That's different because ovnnb_db_run runs when the whole northd I-P node recomputes. Shall I add a new function that iterates only the changed rows and call it > from sb_fdb_change_hanlder()? > I don't think there is a need for a new function as the handler can do that inside, so far this is the only place that would handle this. > > Thanks, > Naveen > > > + 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..447d4fd19 100644 > > --- a/northd/northd.c > > +++ b/northd/northd.c > > @@ -3270,7 +3270,7 @@ cleanup_sb_ha_chassis_groups( > > } > > } > > > > -static void > > +void > > cleanup_stale_fdb_entries(const struct sbrec_fdb_table *sbrec_fdb_table, > > struct hmap *ls_datapaths) > > { > > diff --git a/northd/northd.h b/northd/northd.h > > index d4a8d75ab..e224a073e 100644 > > --- a/northd/northd.h > > +++ b/northd/northd.h > > @@ -731,6 +731,8 @@ bool sync_pbs_for_northd_changed_ovn_ports( > > struct tracked_ovn_ports *, > > const struct lr_stateful_table *); > > > > +void cleanup_stale_fdb_entries(const struct sbrec_fdb_table > *sbrec_fdb_table, > > + struct hmap *ls_datapaths); > > static inline bool > > northd_has_tracked_data(struct northd_tracked_data *trk_nd_changes) { > > return trk_nd_changes->type != NORTHD_TRACKED_NONE; > > -- > > 2.36.6 > > > > _______________________________________________ > > dev mailing list > > [email protected] > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev [ > mail.openvswitch.org] > > > > > > Thanks, > > Ales > > -- > > Ales Musil > > Senior Software Engineer - OVN Core > > Red Hat EMEA [redhat.com][email protected] [red.ht] > > > Thanks, Ales -- 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
