Thanks Alexandra, see in-line for comments.
On 7/31/25 8:01 AM, Alexandra Rukomoinikova wrote:
Add new I-P node that will store all the data for intreconnect
learned service monitors. In the future, support for synchronization
of service monitors between ovn deployments will be added,
this node will help us correctly process service monitors that were
learned from the interconnect southbound database.
This node is needed in many ways to create logical flows for service monitors
(arp/nd replays), when ovn-ic creates a flow we need to trigger logical flow
creations for the learned records.
As design choice there is only single lflow_ref for all ic_learned
related logical_flows that makes them not being thread safe and only
main thread can generate them during full recompute of lflow node.
Based on the contents of this series, you don't need an lflow_ref at
all. lflow_refs are used to recompute a subset of lflows in an
lflow_table in order to avoid having en_lflow recompute all lflows. See
further in-line for more details.
Signed-off-by: Alexandra Rukomoinikova <arukomoinikova@k2.cloud>
---
minor corrections in the code (in northd, controller: Add support for remote LB
health checks patch),
in the previous version there was an error in the tests during rebase and the
test did not pass, I fixed it
---
northd/en-northd.c | 47 +++++++++++++++++++++++++++++++++---
northd/en-northd.h | 6 ++++-
northd/inc-proc-northd.c | 12 ++++++++++
northd/northd.c | 52 ++++++++++++++++++++++++++++++++++++++++
northd/northd.h | 18 +++++++++++++-
ovn-sb.ovsschema | 5 ++--
ovn-sb.xml | 5 ++++
7 files changed, 138 insertions(+), 7 deletions(-)
diff --git a/northd/en-northd.c b/northd/en-northd.c
index 7be01c777..cd62bf3dd 100644
--- a/northd/en-northd.c
+++ b/northd/en-northd.c
@@ -41,6 +41,10 @@ static void
northd_get_input_data(struct engine_node *node,
struct northd_input *input_data)
{
+ input_data->nbrec_mirror_by_type_and_sink =
+ engine_ovsdb_node_get_index(
+ engine_get_input("NB_mirror", node),
+ "nbrec_mirror_by_type_and_sink");
input_data->sbrec_chassis_by_name =
engine_ovsdb_node_get_index(
engine_get_input("SB_chassis", node),
@@ -61,10 +65,10 @@ northd_get_input_data(struct engine_node *node,
engine_ovsdb_node_get_index(
engine_get_input("SB_fdb", node),
"sbrec_fdb_by_dp_and_port");
- input_data->nbrec_mirror_by_type_and_sink =
+ input_data->sbrec_service_monitor_by_learned_type =
engine_ovsdb_node_get_index(
- engine_get_input("NB_mirror", node),
- "nbrec_mirror_by_type_and_sink");
+ engine_get_input("SB_service_monitor", node),
+ "sbrec_service_monitor_by_learned_type");
input_data->nbrec_logical_switch_table =
EN_OVSDB_GET(engine_get_input("NB_logical_switch", node));
@@ -119,6 +123,11 @@ northd_get_input_data(struct engine_node *node,
input_data->svc_monitor_mac_ea = global_config->svc_monitor_mac_ea;
input_data->features = &global_config->features;
input_data->vxlan_mode = global_config->vxlan_mode;
+
+ /* Add Service Monitor data for interconnect learned records. */
+ struct ic_learned_svcs_data *ic_learned_svcs_data =
+ engine_get_input_data("ic_learned_svcs", node);
+ input_data->ic_learned_svs = &ic_learned_svcs_data->ic_learned_svs;
}
enum engine_node_state
@@ -437,6 +446,23 @@ en_bfd_sync_run(struct engine_node *node, void *data)
return new_state;
}
+enum engine_node_state
+en_ic_learned_svcs_run(struct engine_node *node, void *data_)
+{
+ struct ic_learned_svcs_data *data = data_;
+ struct ovsdb_idl_index *sbrec_service_monitor_by_learned_type =
+ engine_ovsdb_node_get_index(
+ engine_get_input("SB_service_monitor", node),
+ "sbrec_service_monitor_by_learned_type");
+
+ ic_learned_svcs_cleanup(data);
+ ic_learned_svcs_init(data);
+ build_ic_learned_svcs_map(&data->ic_learned_svs,
+ sbrec_service_monitor_by_learned_type);
+
+ return EN_UPDATED;
+}
+
void
*en_northd_init(struct engine_node *node OVS_UNUSED,
struct engine_arg *arg OVS_UNUSED)
@@ -487,6 +513,15 @@ void
return data;
}
+void
+*en_ic_learned_svcs_init(struct engine_node *node OVS_UNUSED,
+ struct engine_arg *arg OVS_UNUSED)
+{
+ struct ic_learned_svcs_data *data = xzalloc(sizeof *data);
+ ic_learned_svcs_init(data);
+ return data;
+}
+
void
en_northd_cleanup(void *data)
{
@@ -559,3 +594,9 @@ en_bfd_sync_cleanup(void *data)
{
bfd_sync_destroy(data);
}
+
+void
+en_ic_learned_svcs_cleanup(void *data)
+{
+ ic_learned_svcs_cleanup(data);
+}
diff --git a/northd/en-northd.h b/northd/en-northd.h
index 4f744a6c5..6c1e2ed23 100644
--- a/northd/en-northd.h
+++ b/northd/en-northd.h
@@ -54,5 +54,9 @@ bfd_sync_northd_change_handler(struct engine_node *node,
void *data OVS_UNUSED);
enum engine_node_state en_bfd_sync_run(struct engine_node *node, void *data);
void en_bfd_sync_cleanup(void *data OVS_UNUSED);
-
+void *en_ic_learned_svcs_init(struct engine_node *node OVS_UNUSED,
+ struct engine_arg *arg OVS_UNUSED);
+void en_ic_learned_svcs_cleanup(void *data);
+enum engine_node_state
+en_ic_learned_svcs_run(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 d43bfc16c..f5e9c8a81 100644
--- a/northd/inc-proc-northd.c
+++ b/northd/inc-proc-northd.c
@@ -179,6 +179,7 @@ static ENGINE_NODE(advertised_route_sync);
static ENGINE_NODE(learned_route_sync, CLEAR_TRACKED_DATA);
static ENGINE_NODE(dynamic_routes);
static ENGINE_NODE(group_ecmp_route, CLEAR_TRACKED_DATA);
+static ENGINE_NODE(ic_learned_svcs);
void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
struct ovsdb_idl_loop *sb)
@@ -209,6 +210,8 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
engine_add_input(&en_acl_id, &en_nb_acl, NULL);
engine_add_input(&en_acl_id, &en_sb_acl_id, NULL);
+ engine_add_input(&en_ic_learned_svcs, &en_sb_service_monitor, NULL);
+
engine_add_input(&en_northd, &en_nb_mirror, NULL);
engine_add_input(&en_northd, &en_nb_mirror_rule, NULL);
engine_add_input(&en_northd, &en_nb_static_mac_binding, NULL);
@@ -224,6 +227,7 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
engine_add_input(&en_northd, &en_sb_service_monitor, NULL);
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_ic_learned_svcs, NULL);
engine_add_input(&en_northd, &en_sb_fdb, northd_sb_fdb_change_handler);
engine_add_input(&en_northd, &en_global_config,
northd_global_config_handler);
@@ -354,6 +358,7 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
engine_add_input(&en_lflow, &en_multicast_igmp,
lflow_multicast_igmp_handler);
engine_add_input(&en_lflow, &en_sb_acl_id, NULL);
+ engine_add_input(&en_lflow, &en_ic_learned_svcs, NULL);
The above line sets en_lflow to have no input handler for
en_ic_learned_svcs. If you had an input handler, this is where you could
use the lflow_ref to only recompute the lflows associated with the
en_ic_learned_svcs. Since the input handler is NULL, there is no reason
to have an lflow_ref at all. You can simply pass NULL as the lflow_ref
argument.
The alternative would be to define an input handler for
en_ic_learned_svcs that makes use of the lflow_ref. Since you are using
a single lflow_ref for all en_ic_learned_svcs, it is possible you can
install the same lflow on multiple datapaths. If you want to see an
example of an input handler that also does this, see
lflow_multicast_igmp_handler() in northd/en-lflow.c. This would fit well
as part of patch 4, where you add the code that adds logical flows for
IC-learned service monitors.
engine_add_input(&en_sync_to_sb_addr_set, &en_northd, NULL);
engine_add_input(&en_sync_to_sb_addr_set, &en_lr_stateful, NULL);
@@ -507,6 +512,13 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
"sbrec_ecmp_nexthop_by_ip_and_port",
sbrec_ecmp_nexthop_by_ip_and_port);
+ struct ovsdb_idl_index *sbrec_service_monitor_by_learned_type
+ = ovsdb_idl_index_create1(sb->idl,
+ &sbrec_service_monitor_col_ic_learned);
+ engine_ovsdb_node_add_index(&en_sb_service_monitor,
+ "sbrec_service_monitor_by_learned_type",
+ sbrec_service_monitor_by_learned_type);
+
struct ed_type_global_config *global_config =
engine_get_internal_data(&en_global_config);
unixctl_command_register("debug/chassis-features-list", "", 0, 0,
diff --git a/northd/northd.c b/northd/northd.c
index 764575f21..3c29a4da9 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -11070,6 +11070,34 @@ build_bfd_map(const struct nbrec_bfd_table
*nbrec_bfd_table,
}
}
+
+void
+build_ic_learned_svcs_map(
+ struct hmap *ic_learned_svcs_map,
+ struct ovsdb_idl_index *sbrec_service_monitor_by_learned_type)
+{
+ struct sbrec_service_monitor *key;
+
+ key = sbrec_service_monitor_index_init_row(
+ sbrec_service_monitor_by_learned_type);
+
+ sbrec_service_monitor_set_ic_learned(key, true);
+
+ const struct sbrec_service_monitor *sbrec_mon;
+ SBREC_SERVICE_MONITOR_FOR_EACH_EQUAL (sbrec_mon, key,
+ sbrec_service_monitor_by_learned_type) {
+ uint32_t hash = sbrec_mon->port;
+ hash = hash_string(sbrec_mon->ip, hash);
+ hash = hash_string(sbrec_mon->logical_port, hash);
+ struct service_monitor_info *mon_info = xzalloc(sizeof *mon_info);
+ mon_info->sbrec_mon = sbrec_mon;
+ mon_info->required = true;
+ hmap_insert(ic_learned_svcs_map, &mon_info->hmap_node, hash);
+ }
+
+ sbrec_service_monitor_index_destroy_row(key);
+}
+
/* Returns a string of the IP address of the router port 'op' that
* overlaps with 'ip_s". If one is not found, returns NULL.
*
@@ -19433,6 +19461,13 @@ bfd_sync_destroy(struct bfd_sync_data *data)
sset_destroy(&data->bfd_ports);
}
+void
+ic_learned_svcs_init(struct ic_learned_svcs_data *data)
+{
+ hmap_init(&data->ic_learned_svs);
+ data->lflow_ref = lflow_ref_create();
+}
+
void
northd_destroy(struct northd_data *data)
{
@@ -19488,6 +19523,23 @@ bfd_destroy(struct bfd_data *data)
__bfd_destroy(&data->bfd_connections);
}
+static void
+__ic_learned_svcs_cleanup(struct hmap *ic_learned_svs_map)
+{
+ struct service_monitor_info *mon_info;
+ HMAP_FOR_EACH_POP (mon_info, hmap_node, ic_learned_svs_map) {
+ free(mon_info);
+ }
+ hmap_destroy(ic_learned_svs_map);
+}
+
+void
+ic_learned_svcs_cleanup(struct ic_learned_svcs_data *data)
+{
+ __ic_learned_svcs_cleanup(&data->ic_learned_svs);
+ lflow_ref_destroy(data->lflow_ref);
+}
+
void
route_policies_destroy(struct route_policies_data *data)
{
diff --git a/northd/northd.h b/northd/northd.h
index ac00c4ec4..5d4a596f2 100644
--- a/northd/northd.h
+++ b/northd/northd.h
@@ -71,13 +71,17 @@ struct northd_input {
/* ACL ID inputs. */
const struct acl_id_data *acl_id_data;
+ /* Service Monitor data for interconnect learned records.*/
+ struct hmap *ic_learned_svs;
+
/* Indexes */
+ struct ovsdb_idl_index *nbrec_mirror_by_type_and_sink;
struct ovsdb_idl_index *sbrec_chassis_by_name;
struct ovsdb_idl_index *sbrec_chassis_by_hostname;
struct ovsdb_idl_index *sbrec_ha_chassis_grp_by_name;
struct ovsdb_idl_index *sbrec_ip_mcast_by_dp;
struct ovsdb_idl_index *sbrec_fdb_by_dp_and_port;
- struct ovsdb_idl_index *nbrec_mirror_by_type_and_sink;
+ struct ovsdb_idl_index *sbrec_service_monitor_by_learned_type;
};
/* A collection of datapaths. E.g. all logical switch datapaths, or all
@@ -214,6 +218,10 @@ struct bfd_sync_data {
struct sset bfd_ports;
};
+struct ic_learned_svcs_data {
+ struct hmap ic_learned_svs;
+ struct lflow_ref *lflow_ref;
+};
struct lflow_ref;
struct lr_nat_table;
@@ -246,6 +254,8 @@ struct lflow_input {
struct simap *route_tables;
struct hmap *igmp_groups;
struct lflow_ref *igmp_lflow_ref;
+ struct hmap *ic_learned_svcs;
+ struct lflow_ref *ic_leared_svcs_lflow_ref;
nit: If you keep the lflow_ref, then
s/leared/learned/
};
extern int parallelization_state;
@@ -860,6 +870,9 @@ void bfd_sync_init(struct bfd_sync_data *);
void bfd_sync_swap(struct bfd_sync_data *, struct sset *bfd_ports);
void bfd_sync_destroy(struct bfd_sync_data *);
+void ic_learned_svcs_init(struct ic_learned_svcs_data *data);
+void ic_learned_svcs_cleanup(struct ic_learned_svcs_data *data);
+
struct lflow_table;
struct lr_stateful_tracked_data;
struct ls_stateful_tracked_data;
@@ -912,6 +925,9 @@ void bfd_table_sync(struct ovsdb_idl_txn *, const struct
nbrec_bfd_table *,
struct sset *);
void build_bfd_map(const struct nbrec_bfd_table *,
const struct sbrec_bfd_table *, struct hmap *);
+void build_ic_learned_svcs_map(struct hmap *ic_learned_svcs_map,
+ struct ovsdb_idl_index *sbrec_service_monitor_by_learned_type);
+
void run_update_worker_pool(int n_threads);
const struct ovn_datapath *northd_get_datapath_for_port(
diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema
index 4c24f5b51..da1fd38e8 100644
--- a/ovn-sb.ovsschema
+++ b/ovn-sb.ovsschema
@@ -1,7 +1,7 @@
{
"name": "OVN_Southbound",
- "version": "21.2.0",
- "cksum": "29145795 34859",
+ "version": "21.3.0",
+ "cksum": "3512532069 34910",
"tables": {
"SB_Global": {
"columns": {
@@ -517,6 +517,7 @@
"src_mac": {"type": "string"},
"src_ip": {"type": "string"},
"chassis_name": {"type": "string"},
+ "ic_learned": {"type": "boolean"},
"status": {
"type": {"key": {"type": "string",
"enum": ["set", ["online", "offline", "error"]]},
diff --git a/ovn-sb.xml b/ovn-sb.xml
index db5faac66..f7eafd379 100644
--- a/ovn-sb.xml
+++ b/ovn-sb.xml
@@ -5010,6 +5010,11 @@ tcp.flags = RST;
The name of the chassis where the logical port is bound.
</column>
+ <column name="ic_learned">
+ Set to true if the service monitor was propagated from another
+ OVN deployment via ovn-ic management.
+ </column>
+
<column name="options" key="interval" type='{"type": "integer"}'>
The interval, in seconds, between service monitor checks.
</column>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
