> On 24 Jul 2024, at 1:54 PM, Ales Musil <[email protected]> wrote:
>
> CAUTION: External Email
>
>
> 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.
Sure, I will update the handler to walk through only changed entries in the FDB
table.
>
> 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] [mail.openvswitch.org [mail.openvswitch.org]]
> >
> >
> > Thanks,
> > Ales
> > --
> > Ales Musil
> > Senior Software Engineer - OVN Core
> > Red Hat EMEA [redhat.com [redhat.com]][email protected] [red.ht [red.ht]]
>
>
>
> Thanks,
> Ales
>
> --
> Ales Musil
> Senior Software Engineer - OVN Core
> Red Hat EMEA [redhat.com][email protected] [red.ht]
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
