Hello Dumitru and Mark! In v1, we discussed whether this patch series had a chance to make it into the release. I spent a long time editing the comments and only just submitted v2. I made patches 1-5 with simple functionality additions and didn't make any unnecessary code changes. I also made patches 6-8 separately with some attempts to refactor the code. I don't think 6-8 should be eligible for this release, and I completely understand if they don't receive any review soon. If they aren't merged now, I might submit these three patches later, after the release. Thanks)
On 08.02.2026 00:48, Alexandra Rukomoinikova wrote: > Replace individual global service monitor address parameters with unified > svc_monitor_addresses structure to simplify function signatures. > > Signed-off-by: Alexandra Rukomoinikova <[email protected]> > --- > northd/en-global-config.c | 47 +++++++++++--------- > northd/en-global-config.h | 29 ++++++------ > northd/en-lflow.c | 3 +- > northd/en-northd.c | 8 +--- > northd/en-sync-sb.c | 4 +- > northd/northd.c | 94 +++++++++++++++------------------------ > northd/northd.h | 8 +--- > 7 files changed, 84 insertions(+), 109 deletions(-) > > diff --git a/northd/en-global-config.c b/northd/en-global-config.c > index 2556b2888..bad1bcf68 100644 > --- a/northd/en-global-config.c > +++ b/northd/en-global-config.c > @@ -124,14 +124,17 @@ en_global_config_run(struct engine_node *node , void > *data) > const char *mac_addr_prefix = set_mac_prefix(smap_get(&nb->options, > "mac_prefix")); > > + struct svc_monitor_addresses *svc_addresses = > + &config_data->svc_global_addresses; > + > const char *monitor_mac = smap_get(&nb->options, "svc_monitor_mac"); > if (monitor_mac) { > if (eth_addr_from_string(monitor_mac, > - &config_data->svc_monitor_mac_ea)) { > - snprintf(config_data->svc_monitor_mac, > - sizeof config_data->svc_monitor_mac, > + &svc_addresses->mac_ea_src)) { > + snprintf(svc_addresses->mac_src, > + sizeof svc_addresses->mac_src, > ETH_ADDR_FMT, > - ETH_ADDR_ARGS(config_data->svc_monitor_mac_ea)); > + ETH_ADDR_ARGS(svc_addresses->mac_ea_src)); > } else { > monitor_mac = NULL; > } > @@ -141,22 +144,22 @@ en_global_config_run(struct engine_node *node , void > *data) > "svc_monitor_mac_dst"); > if (dst_monitor_mac) { > if (eth_addr_from_string(dst_monitor_mac, > - &config_data->svc_monitor_mac_ea_dst)) { > - snprintf(config_data->svc_monitor_mac_dst, > - sizeof config_data->svc_monitor_mac_dst, > + &svc_addresses->mac_ea_dst)) { > + snprintf(svc_addresses->mac_dst, > + sizeof svc_addresses->mac_dst, > ETH_ADDR_FMT, > - ETH_ADDR_ARGS(config_data->svc_monitor_mac_ea_dst)); > + ETH_ADDR_ARGS(svc_addresses->mac_ea_dst)); > } else { > dst_monitor_mac = NULL; > } > } > > const char *monitor_ip = smap_get(&nb->options, "svc_monitor_ip"); > - update_svc_monitor_addr(monitor_ip, &config_data->svc_monitor_ip); > + update_svc_monitor_addr(monitor_ip, &svc_addresses->ip_src); > > const char *monitor_ip_dst = smap_get(&nb->options, > "svc_monitor_ip_dst"); > update_svc_monitor_addr(monitor_ip_dst, > - &config_data->svc_monitor_ip_dst); > + &svc_addresses->ip_dst); > > struct smap *options = &config_data->nb_options; > smap_destroy(options); > @@ -165,21 +168,21 @@ en_global_config_run(struct engine_node *node , void > *data) > smap_replace(options, "mac_prefix", mac_addr_prefix); > > if (!monitor_mac) { > - eth_addr_random(&config_data->svc_monitor_mac_ea); > - snprintf(config_data->svc_monitor_mac, > - sizeof config_data->svc_monitor_mac, ETH_ADDR_FMT, > - ETH_ADDR_ARGS(config_data->svc_monitor_mac_ea)); > + eth_addr_random(&svc_addresses->mac_ea_src); > + snprintf(svc_addresses->mac_src, > + sizeof svc_addresses->mac_src, ETH_ADDR_FMT, > + ETH_ADDR_ARGS(svc_addresses->mac_ea_src)); > smap_replace(options, "svc_monitor_mac", > - config_data->svc_monitor_mac); > + svc_addresses->mac_src); > } > > if (!dst_monitor_mac) { > - eth_addr_random(&config_data->svc_monitor_mac_ea_dst); > - snprintf(config_data->svc_monitor_mac_dst, > - sizeof config_data->svc_monitor_mac_dst, ETH_ADDR_FMT, > - ETH_ADDR_ARGS(config_data->svc_monitor_mac_ea_dst)); > + eth_addr_random(&svc_addresses->mac_ea_dst); > + snprintf(svc_addresses->mac_dst, > + sizeof svc_addresses->mac_dst, ETH_ADDR_FMT, > + ETH_ADDR_ARGS(svc_addresses->mac_ea_dst)); > smap_replace(options, "svc_monitor_mac_dst", > - config_data->svc_monitor_mac_dst); > + svc_addresses->mac_dst); > } > > bool ic_vxlan_mode = false; > @@ -244,8 +247,8 @@ void en_global_config_cleanup(void *data OVS_UNUSED) > struct ed_type_global_config *config_data = data; > smap_destroy(&config_data->nb_options); > smap_destroy(&config_data->sb_options); > - free(config_data->svc_monitor_ip); > - free(config_data->svc_monitor_ip_dst); > + free(config_data->svc_global_addresses.ip_src); > + free(config_data->svc_global_addresses.ip_dst); > destroy_debug_config(); > } > > diff --git a/northd/en-global-config.h b/northd/en-global-config.h > index 413cd3849..e84b0e9e5 100644 > --- a/northd/en-global-config.h > +++ b/northd/en-global-config.h > @@ -30,27 +30,30 @@ struct global_config_tracked_data { > bool chassis_features_changed; > }; > > -/* struct which maintains the data of the engine node global_config. */ > -struct ed_type_global_config { > - struct smap nb_options; > - struct smap sb_options; > - const struct nbrec_nb_global *nb_global; > - const struct sbrec_sb_global *sb_global; > - > +struct svc_monitor_addresses { > /* MAC allocated for service monitor usage. Just one pair is allocated > * for this purpose and ovn-controller's on each chassis will make use > * of this pair when sending out the packets to monitor the services > * defined in Service_Monitor Southbound table. Since these packets > * are locally handled, having just one pair is good enough. */ > - char svc_monitor_mac[ETH_ADDR_STRLEN + 1]; > - struct eth_addr svc_monitor_mac_ea; > - char svc_monitor_mac_dst[ETH_ADDR_STRLEN + 1]; > - struct eth_addr svc_monitor_mac_ea_dst; > + char mac_src[ETH_ADDR_STRLEN + 1]; > + struct eth_addr mac_ea_src; > + char mac_dst[ETH_ADDR_STRLEN + 1]; > + struct eth_addr mac_ea_dst; > > /* IP addresses configured for NF service monitor usage. */ > - char *svc_monitor_ip; > - char *svc_monitor_ip_dst; > + char *ip_src; > + char *ip_dst; > +}; > + > +/* struct which maintains the data of the engine node global_config. */ > +struct ed_type_global_config { > + struct smap nb_options; > + struct smap sb_options; > + const struct nbrec_nb_global *nb_global; > + const struct sbrec_sb_global *sb_global; > > + struct svc_monitor_addresses svc_global_addresses; > struct chassis_features features; > > bool ovn_internal_version_changed; > diff --git a/northd/en-lflow.c b/northd/en-lflow.c > index 41a5a97ff..0f64aef49 100644 > --- a/northd/en-lflow.c > +++ b/northd/en-lflow.c > @@ -113,7 +113,8 @@ lflow_get_input_data(struct engine_node *node, > lflow_input->features = &global_config->features; > lflow_input->ovn_internal_version_changed = > global_config->ovn_internal_version_changed; > - lflow_input->svc_monitor_mac = global_config->svc_monitor_mac; > + lflow_input->svc_monitor_mac = > + global_config->svc_global_addresses.mac_src; > > struct ed_type_sampling_app_data *sampling_app_data = > engine_get_input_data("sampling_app", node); > diff --git a/northd/en-northd.c b/northd/en-northd.c > index 9b37f3eee..e81e4bbb2 100644 > --- a/northd/en-northd.c > +++ b/northd/en-northd.c > @@ -117,12 +117,8 @@ northd_get_input_data(struct engine_node *node, > engine_get_input_data("global_config", node); > input_data->nb_options = &global_config->nb_options; > input_data->sb_options = &global_config->sb_options; > - input_data->svc_monitor_mac = global_config->svc_monitor_mac; > - input_data->svc_monitor_mac_ea = global_config->svc_monitor_mac_ea; > - input_data->svc_monitor_mac_dst = global_config->svc_monitor_mac_dst; > - input_data->svc_monitor_mac_ea_dst = > global_config->svc_monitor_mac_ea_dst; > - input_data->svc_monitor_ip = global_config->svc_monitor_ip; > - input_data->svc_monitor_ip_dst = global_config->svc_monitor_ip_dst; > + > + input_data->svc_global_addresses = &global_config->svc_global_addresses; > input_data->features = &global_config->features; > input_data->vxlan_mode = global_config->vxlan_mode; > > diff --git a/northd/en-sync-sb.c b/northd/en-sync-sb.c > index fbe26eb8c..0b2a85b3b 100644 > --- a/northd/en-sync-sb.c > +++ b/northd/en-sync-sb.c > @@ -106,8 +106,8 @@ en_sync_to_sb_addr_set_run(struct engine_node *node, void > *data OVS_UNUSED) > nb_port_group_table, sb_address_set_table, > &lr_stateful_data->table, > &northd_data->lr_datapaths, > - global_config->svc_monitor_mac, > - global_config->svc_monitor_mac_dst); > + global_config->svc_global_addresses.mac_src, > + global_config->svc_global_addresses.mac_dst); > > return EN_UPDATED; > } > diff --git a/northd/northd.c b/northd/northd.c > index 7a2c472c0..629651efb 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -3165,28 +3165,28 @@ svc_mon_update_status_if_online(const struct > sbrec_service_monitor *sbrec_mon, > > static void > svc_mon_update_mac_ip_addresses(const struct sbrec_service_monitor > *sbrec_mon, > - const struct eth_addr *svc_monitor_mac_ea, > - const char *source_mac, > - const char *target_mac, > - const char *source_ip, > - const char *target_ip) > + const struct eth_addr *mac_ea_src, > + const char *mac_src, > + const char *mac_dst, > + const char *ip_src, > + const char *ip_dst) > { > struct eth_addr ea; > if (sbrec_mon->src_mac || > !eth_addr_from_string(sbrec_mon->src_mac, &ea) || > - !eth_addr_equals(ea, *svc_monitor_mac_ea)) { > - sbrec_service_monitor_set_src_mac(sbrec_mon, source_mac); > + !eth_addr_equals(ea, *mac_ea_src)) { > + sbrec_service_monitor_set_src_mac(sbrec_mon, mac_src); > } > > - if (target_mac) { > - svc_mon_update_field_if_changed(sbrec_mon, sbrec_mon->mac, > target_mac, > + if (mac_dst) { > + svc_mon_update_field_if_changed(sbrec_mon, sbrec_mon->mac, mac_dst, > sbrec_service_monitor_set_mac); > } > > - svc_mon_update_field_if_changed(sbrec_mon, sbrec_mon->src_ip, source_ip, > + svc_mon_update_field_if_changed(sbrec_mon, sbrec_mon->src_ip, ip_src, > sbrec_service_monitor_set_src_ip); > - if (target_ip) { > - svc_mon_update_field_if_changed(sbrec_mon, sbrec_mon->ip, target_ip, > + if (ip_dst) { > + svc_mon_update_field_if_changed(sbrec_mon, sbrec_mon->ip, ip_dst, > sbrec_service_monitor_set_ip); > } > } > @@ -3194,15 +3194,11 @@ svc_mon_update_mac_ip_addresses(const struct > sbrec_service_monitor *sbrec_mon, > static void > ovn_nf_svc_create(struct ovsdb_idl_txn *ovnsb_txn, > const struct nbrec_network_function *nbrec_nf, > + const struct svc_monitor_addresses *svc_global_addresses, > struct hmap *local_svc_monitors_map, > struct hmap *ic_learned_svc_monitors_map, > struct sset *svc_monitor_lsps, > - struct hmap *ls_ports, > - const char *global_svc_monitor_mac_src, > - const struct eth_addr *global_svc_monitor_mac_ea_src, > - const char *global_svc_monitor_mac_dst, > - const char *global_svc_monitor_ip_src, > - const char *global_svc_monitor_ip_dst) > + struct hmap *ls_ports) > { > if (!nbrec_nf->health_check) { > return; > @@ -3241,22 +3237,18 @@ ovn_nf_svc_create(struct ovsdb_idl_txn *ovnsb_txn, > local_svc_monitors_map, > ic_learned_svc_monitors_map, > "network-function", > - global_svc_monitor_ip_dst, > + svc_global_addresses->ip_dst, > nbrec_nf->outport->name, > nbrec_nf->inport->name, > - 0, > - "icmp", > - chassis_name, > - false); > + 0, "icmp", chassis_name, false); > set_service_mon_options(mon_info->sbrec_mon, > - &nbrec_nf->health_check->options, > - NULL); > + &nbrec_nf->health_check->options, NULL); > svc_mon_update_mac_ip_addresses(mon_info->sbrec_mon, > - global_svc_monitor_mac_ea_src, > - global_svc_monitor_mac_src, > - global_svc_monitor_mac_dst, > - global_svc_monitor_ip_src, > - global_svc_monitor_ip_dst); > + &svc_global_addresses->mac_ea_src, > + svc_global_addresses->mac_src, > + svc_global_addresses->mac_dst, > + svc_global_addresses->ip_src, > + svc_global_addresses->ip_dst); > svc_mon_update_status_if_online(mon_info->sbrec_mon, > !port_up_condition); > } > @@ -3264,8 +3256,7 @@ ovn_nf_svc_create(struct ovsdb_idl_txn *ovnsb_txn, > static void > ovn_lb_svc_create(struct ovsdb_idl_txn *ovnsb_txn, > const struct ovn_northd_lb *lb, > - const char *global_svc_monitor_mac_src, > - const struct eth_addr *global_svc_monitor_mac_ea_src, > + const struct svc_monitor_addresses *svc_global_addresses, > struct hmap *ls_ports, > struct sset *svc_monitor_lsps, > struct hmap *local_svc_monitors_map, > @@ -3311,8 +3302,8 @@ ovn_lb_svc_create(struct ovsdb_idl_txn *ovnsb_txn, > source_mac_ea = &backend_nb->svc_mon_lrp->lrp_networks.ea; > source_mac = backend_nb->svc_mon_lrp->lrp_networks.ea_s; > } else { > - source_mac_ea = global_svc_monitor_mac_ea_src; > - source_mac = global_svc_monitor_mac_src; > + source_mac_ea = &svc_global_addresses->mac_ea_src; > + source_mac = svc_global_addresses->mac_src; > } > > const char *protocol = lb->nlb->protocol; > @@ -3554,8 +3545,7 @@ build_lb_datapaths(const struct hmap *lbs, const struct > hmap *lb_groups, > static void > ovn_lsp_svc_monitors_process_port( > struct ovsdb_idl_txn *ovnsb_txn, const struct ovn_port *op, > - const char *global_svc_monitor_mac_src, > - const struct eth_addr *global_svc_monitor_mac_ea_src, > + const struct svc_monitor_addresses *svc_global_addresses, > struct hmap *local_svc_monitors_map, > struct sset *svc_monitor_lsps) > { > @@ -3597,8 +3587,8 @@ ovn_lsp_svc_monitors_process_port( > set_service_mon_options(mon_info->sbrec_mon, > &lsp_hc->options, NULL); > svc_mon_update_mac_ip_addresses(mon_info->sbrec_mon, > - global_svc_monitor_mac_ea_src, > - global_svc_monitor_mac_src, > + &svc_global_addresses->mac_ea_src, > + svc_global_addresses->mac_src, > NULL, lsp_hc->src_ip, NULL); > svc_mon_update_status_if_online(mon_info->sbrec_mon, > port_up_condition); > @@ -3610,11 +3600,7 @@ build_svc_monitors_data( > struct ovsdb_idl_txn *ovnsb_txn, > struct ovsdb_idl_index *sbrec_service_monitor_by_learned_type, > const struct nbrec_network_function_table *nbrec_network_function_table, > - const char *global_svc_monitor_mac_src, > - const struct eth_addr *global_svc_monitor_mac_ea_src, > - const char *global_svc_monitor_mac_dst, > - const char *global_svc_monitor_ip_src, > - const char *global_svc_monitor_ip_dst, > + const struct svc_monitor_addresses *svc_global_addresses, > struct hmap *ls_ports, struct hmap *lb_dps_map, > struct sset *svc_monitor_lsps, > struct hmap *local_svc_monitors_map, > @@ -3645,8 +3631,7 @@ build_svc_monitors_data( > struct ovn_lb_datapaths *lb_dps; > HMAP_FOR_EACH (lb_dps, hmap_node, lb_dps_map) { > ovn_lb_svc_create(ovnsb_txn, lb_dps->lb, > - global_svc_monitor_mac_src, > - global_svc_monitor_mac_ea_src, > + svc_global_addresses, > ls_ports, > svc_monitor_lsps, > local_svc_monitors_map, > @@ -3658,15 +3643,11 @@ build_svc_monitors_data( > nbrec_network_function_table) { > ovn_nf_svc_create(ovnsb_txn, > nbrec_nf, > + svc_global_addresses, > local_svc_monitors_map, > ic_learned_svc_monitors_map, > svc_monitor_lsps, > - ls_ports, > - global_svc_monitor_mac_src, > - global_svc_monitor_mac_ea_src, > - global_svc_monitor_mac_dst, > - global_svc_monitor_ip_src, > - global_svc_monitor_ip_dst); > + ls_ports); > } > > struct hmapx_node *hmapx_node; > @@ -3675,8 +3656,7 @@ build_svc_monitors_data( > op = hmapx_node->data; > ovn_lsp_svc_monitors_process_port(ovnsb_txn, > op, > - global_svc_monitor_mac_src, > - global_svc_monitor_mac_ea_src, > + svc_global_addresses, > local_svc_monitors_map, > svc_monitor_lsps); > } > @@ -20771,11 +20751,7 @@ ovnnb_db_run(struct northd_input *input_data, > build_svc_monitors_data(ovnsb_txn, > input_data->sbrec_service_monitor_by_learned_type, > input_data->nbrec_network_function_table, > - input_data->svc_monitor_mac, > - &input_data->svc_monitor_mac_ea, > - input_data->svc_monitor_mac_dst, > - input_data->svc_monitor_ip, > - input_data->svc_monitor_ip_dst, > + input_data->svc_global_addresses, > &data->ls_ports, &data->lb_datapaths_map, > &data->svc_monitor_lsps, &data->local_svc_monitors_map, > input_data->ic_learned_svc_monitors_map, > @@ -20785,7 +20761,7 @@ ovnnb_db_run(struct northd_input *input_data, > input_data->nbrec_network_function_group_table, > &data->local_svc_monitors_map, > input_data->ic_learned_svc_monitors_map, > - input_data->svc_monitor_ip_dst); > + input_data->svc_global_addresses->ip_dst); > build_ipam(&data->ls_datapaths.datapaths); > build_lrouter_groups(&data->lr_ports, &data->lr_datapaths); > build_ip_mcast(ovnsb_txn, input_data->sbrec_ip_multicast_table, > diff --git a/northd/northd.h b/northd/northd.h > index f5c8b17dd..b7cea07c1 100644 > --- a/northd/northd.h > +++ b/northd/northd.h > @@ -23,6 +23,7 @@ > #include "lib/hmapx.h" > #include "northd/en-port-group.h" > #include "northd/en-sync-from-sb.h" > +#include "northd/en-global-config.h" > #include "northd/ipam.h" > #include "northd/lb.h" > #include "openvswitch/hmap.h" > @@ -70,12 +71,7 @@ struct northd_input { > /* Global config data node inputs. */ > const struct smap *nb_options; > const struct smap *sb_options; > - const char *svc_monitor_mac; > - struct eth_addr svc_monitor_mac_ea; > - const char *svc_monitor_mac_dst; > - struct eth_addr svc_monitor_mac_ea_dst; > - char *svc_monitor_ip; > - char *svc_monitor_ip_dst; > + const struct svc_monitor_addresses *svc_global_addresses; > const struct chassis_features *features; > bool vxlan_mode; > -- regards, Alexandra. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
