On 2/7/26 10:48 PM, Alexandra Rukomoinikova wrote: > Signed-off-by: Alexandra Rukomoinikova <[email protected]> > --- > v1 --> v2: nothind changed > ---
Hi Alexandra, Mark had a comment on this patch in the previous version: https://mail.openvswitch.org/pipermail/ovs-dev/2026-January/429632.html That hasn't been addressed as far as I can tell. Which makes me wonder, should've this series (that we're reviewing now) been v3 instead? > northd/en-sync-from-sb.c | 28 +++++++++++++++++++--------- > northd/en-sync-from-sb.h | 6 ++++++ > northd/northd.c | 29 +++++++++++++++++------------ > northd/northd.h | 7 +++---- > 4 files changed, 45 insertions(+), 25 deletions(-) > > diff --git a/northd/en-sync-from-sb.c b/northd/en-sync-from-sb.c > index dde0e9f55..6d4ff3e39 100644 > --- a/northd/en-sync-from-sb.c > +++ b/northd/en-sync-from-sb.c > @@ -40,23 +40,33 @@ void * > en_sync_from_sb_init(struct engine_node *node OVS_UNUSED, > struct engine_arg *arg OVS_UNUSED) > { > - return NULL; > + struct en_sync_from_sb_data *data = xmalloc(sizeof *data); We're leaking this. We should free it in en_sync_from_sb_cleanup(). > + return data; > +} > + > +static void > +en_sync_from_sb_get_input_data(struct engine_node *node, > + struct en_sync_from_sb_data *data) > +{ > + data->sb_pb_table = > + EN_OVSDB_GET(engine_get_input("SB_port_binding", node)); > + data->sb_ha_ch_grp_table = > + EN_OVSDB_GET(engine_get_input("SB_ha_chassis_group", node)); > } > > enum engine_node_state > -en_sync_from_sb_run(struct engine_node *node, void *data OVS_UNUSED) > +en_sync_from_sb_run(struct engine_node *node, void *data_) > { > + struct en_sync_from_sb_data *data = > + (struct en_sync_from_sb_data *) data_; > const struct engine_context *eng_ctx = engine_get_context(); > - struct northd_data *nd = engine_get_input_data("northd", node); > + struct northd_data *northd_data = engine_get_input_data("northd", node); > + > + en_sync_from_sb_get_input_data(node, data); > > - const struct sbrec_port_binding_table *sb_pb_table = > - EN_OVSDB_GET(engine_get_input("SB_port_binding", node)); > - const struct sbrec_ha_chassis_group_table *sb_ha_ch_grp_table = > - EN_OVSDB_GET(engine_get_input("SB_ha_chassis_group", node)); > stopwatch_start(OVNSB_DB_RUN_STOPWATCH_NAME, time_msec()); > ovnsb_db_run(eng_ctx->ovnsb_idl_txn, > - sb_pb_table, sb_ha_ch_grp_table, > - &nd->ls_ports, &nd->lr_ports); > + data, northd_data); > stopwatch_stop(OVNSB_DB_RUN_STOPWATCH_NAME, time_msec()); > > return EN_UNCHANGED; > diff --git a/northd/en-sync-from-sb.h b/northd/en-sync-from-sb.h > index 0ad07853a..bea248c45 100644 > --- a/northd/en-sync-from-sb.h > +++ b/northd/en-sync-from-sb.h > @@ -3,6 +3,12 @@ > > #include "lib/inc-proc-eng.h" > > +struct en_sync_from_sb_data { > + /* Southbound table references */ > + const struct sbrec_port_binding_table *sb_pb_table; > + const struct sbrec_ha_chassis_group_table *sb_ha_ch_grp_table; > +}; > + > void *en_sync_from_sb_init(struct engine_node *, struct engine_arg *); > enum engine_node_state en_sync_from_sb_run(struct engine_node *, void *data); > void en_sync_from_sb_cleanup(void *data); > diff --git a/northd/northd.c b/northd/northd.c > index 8cb307ca3..1904b162b 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -55,6 +55,7 @@ > #include "en-sampling-app.h" > #include "en-datapath-logical-switch.h" > #include "en-datapath-logical-router.h" > +#include "en-sync-from-sb.h" > #include "lib/ovn-parallel-hmap.h" > #include "ovn/actions.h" > #include "ovn/features.h" > @@ -2439,7 +2440,7 @@ op_get_name(const struct ovn_port *op) > } > > static void > -ovn_update_ipv6_prefix(struct hmap *lr_ports) > +ovn_update_ipv6_prefix(const struct hmap *lr_ports) > { > const struct ovn_port *op; > HMAP_FOR_EACH (op, key_node, lr_ports) { > @@ -21011,10 +21012,11 @@ handle_cr_port_binding_changes(const struct > sbrec_port_binding *sb, > * this column is not empty, it means we need to set the corresponding > logical > * port as 'up' in the northbound DB. */ > static void > -handle_port_binding_changes(const struct sbrec_port_binding_table > *sb_pb_table, > +handle_port_binding_changes( > + const struct sbrec_port_binding_table *sb_pb_table, > const struct sbrec_ha_chassis_group_table > *sb_ha_ch_grp_table, > - struct hmap *ls_ports, > - struct hmap *lr_ports, > + const struct hmap *ls_ports, > + const struct hmap *lr_ports, > struct shash *ha_ref_chassis_map) > { > struct hmapx lr_groups = HMAPX_INITIALIZER(&lr_groups); > @@ -21090,23 +21092,26 @@ handle_port_binding_changes(const struct > sbrec_port_binding_table *sb_pb_table, > /* Handle a fairly small set of changes in the southbound database. */ > void > ovnsb_db_run(struct ovsdb_idl_txn *ovnsb_txn, > - const struct sbrec_port_binding_table *sb_pb_table, > - const struct sbrec_ha_chassis_group_table *sb_ha_ch_grp_table, > - struct hmap *ls_ports, > - struct hmap *lr_ports) > + const struct en_sync_from_sb_data *sync_from_sb_data, > + const struct northd_data *northd_data) > { > if (!ovsdb_idl_has_ever_connected(ovsdb_idl_txn_get_idl(ovnsb_txn))) { > return; > } > > struct shash ha_ref_chassis_map = SHASH_INITIALIZER(&ha_ref_chassis_map); > - handle_port_binding_changes(sb_pb_table, sb_ha_ch_grp_table, > - ls_ports, lr_ports, &ha_ref_chassis_map); > - update_sb_ha_group_ref_chassis(sb_ha_ch_grp_table, &ha_ref_chassis_map); > + handle_port_binding_changes(sync_from_sb_data->sb_pb_table, > + sync_from_sb_data->sb_ha_ch_grp_table, > + &northd_data->ls_ports, > + &northd_data->lr_ports, > + &ha_ref_chassis_map); > + > + update_sb_ha_group_ref_chassis(sync_from_sb_data->sb_ha_ch_grp_table, > + &ha_ref_chassis_map); > > shash_destroy(&ha_ref_chassis_map); > > - ovn_update_ipv6_prefix(lr_ports); > + ovn_update_ipv6_prefix(&northd_data->lr_ports); > } > > const struct ovn_datapath * > diff --git a/northd/northd.h b/northd/northd.h > index 77e8b049b..f5c8b17dd 100644 > --- a/northd/northd.h > +++ b/northd/northd.h > @@ -22,6 +22,7 @@ > #include "lib/sset.h" > #include "lib/hmapx.h" > #include "northd/en-port-group.h" > +#include "northd/en-sync-from-sb.h" > #include "northd/ipam.h" > #include "northd/lb.h" > #include "openvswitch/hmap.h" > @@ -892,10 +893,8 @@ void ovnnb_db_run(struct northd_input *input_data, > struct northd_data *data, > struct ovsdb_idl_txn *ovnsb_txn); > void ovnsb_db_run(struct ovsdb_idl_txn *ovnsb_txn, > - const struct sbrec_port_binding_table *, > - const struct sbrec_ha_chassis_group_table *, > - struct hmap *ls_ports, > - struct hmap *lr_ports); > + const struct en_sync_from_sb_data *sync_from_sb_data, > + const struct northd_data *northd_data); > bool northd_handle_ls_changes(struct ovsdb_idl_txn *, > const struct northd_input *, > struct northd_data *); Regards, Dumitru _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
