The local_datapaths are populated by iterating through
all available port bindings or new ones added during I-C.
The port bindings are not ordered, it might happen that
the order between engine runs are different. This can cause
the prefix delegation shash to be with NULL local_datapath.
Even in cases that the port binding is actually part of
the local_datapath. Store only port binding in the shash
and during pinctrl_run check if the stored port binding
is part of local_datapaths. Because pinctrl runs only
after the engine computation is complete we should have
complete hmap of all local_datapaths.
Fixes: 647920b6 ("controller: incrementally create ipv6 prefix delegation
port_binding list")
Reported-at: https://bugzilla.redhat.com/2108726
Signed-off-by: Ales Musil <[email protected]>
---
controller/binding.c | 35 ++++++++---------------------------
controller/ovn-controller.c | 8 ++++----
controller/ovn-controller.h | 6 ------
controller/pinctrl.c | 18 ++++++++++--------
4 files changed, 22 insertions(+), 45 deletions(-)
diff --git a/controller/binding.c b/controller/binding.c
index ba803ae3e..19b28369f 100644
--- a/controller/binding.c
+++ b/controller/binding.c
@@ -509,35 +509,20 @@ static void
delete_active_pb_ras_pd(const struct sbrec_port_binding *pb,
struct shash *ras_pd_map)
{
- struct pb_ld_binding *ras_pd =
- shash_find_and_delete(ras_pd_map, pb->logical_port);
-
- free(ras_pd);
+ shash_find_and_delete(ras_pd_map, pb->logical_port);
}
static void
update_active_pb_ras_pd(const struct sbrec_port_binding *pb,
- struct hmap *local_datapaths,
struct shash *map, const char *conf)
{
bool ras_pd_conf = smap_get_bool(&pb->options, conf, false);
struct shash_node *iter = shash_find(map, pb->logical_port);
- struct pb_ld_binding *ras_pd = iter ? iter->data : NULL;
- if (iter && !ras_pd_conf) {
+ if (!ras_pd_conf && iter) {
shash_delete(map, iter);
- free(ras_pd);
- return;
- }
- if (ras_pd_conf) {
- if (!ras_pd) {
- ras_pd = xzalloc(sizeof *ras_pd);
- ras_pd->pb = pb;
- shash_add(map, pb->logical_port, ras_pd);
- }
- ovs_assert(ras_pd);
- ras_pd->ld = get_local_datapath(local_datapaths,
- pb->datapath->tunnel_key);
+ } else if (ras_pd_conf && !iter) {
+ shash_add(map, pb->logical_port, pb);
}
}
@@ -1735,11 +1720,9 @@ binding_run(struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out)
const struct sbrec_port_binding *pb;
SBREC_PORT_BINDING_TABLE_FOR_EACH (pb,
b_ctx_in->port_binding_table) {
- update_active_pb_ras_pd(pb, b_ctx_out->local_datapaths,
- b_ctx_out->local_active_ports_ipv6_pd,
+ update_active_pb_ras_pd(pb, b_ctx_out->local_active_ports_ipv6_pd,
"ipv6_prefix_delegation");
- update_active_pb_ras_pd(pb, b_ctx_out->local_datapaths,
- b_ctx_out->local_active_ports_ras,
+ update_active_pb_ras_pd(pb, b_ctx_out->local_active_ports_ras,
"ipv6_ra_send_periodic");
enum en_lport_type lport_type = get_lport_type(pb);
@@ -2629,12 +2612,10 @@ delete_done:
continue;
}
- update_active_pb_ras_pd(pb, b_ctx_out->local_datapaths,
- b_ctx_out->local_active_ports_ipv6_pd,
+ update_active_pb_ras_pd(pb, b_ctx_out->local_active_ports_ipv6_pd,
"ipv6_prefix_delegation");
- update_active_pb_ras_pd(pb, b_ctx_out->local_datapaths,
- b_ctx_out->local_active_ports_ras,
+ update_active_pb_ras_pd(pb, b_ctx_out->local_active_ports_ras,
"ipv6_ra_send_periodic");
enum en_lport_type lport_type = get_lport_type(pb);
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 2e9138036..5449743e8 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -1312,8 +1312,8 @@ en_runtime_data_cleanup(void *data)
sset_destroy(&rt_data->egress_ifaces);
smap_destroy(&rt_data->local_iface_ids);
local_datapaths_destroy(&rt_data->local_datapaths);
- shash_destroy_free_data(&rt_data->local_active_ports_ipv6_pd);
- shash_destroy_free_data(&rt_data->local_active_ports_ras);
+ shash_destroy(&rt_data->local_active_ports_ipv6_pd);
+ shash_destroy(&rt_data->local_active_ports_ras);
local_binding_data_destroy(&rt_data->lbinding_data);
}
@@ -1425,8 +1425,8 @@ en_runtime_data_run(struct engine_node *node, void *data)
first_run = false;
} else {
local_datapaths_destroy(local_datapaths);
- shash_clear_free_data(local_active_ipv6_pd);
- shash_clear_free_data(local_active_ras);
+ shash_clear(local_active_ipv6_pd);
+ shash_clear(local_active_ras);
local_binding_data_destroy(&rt_data->lbinding_data);
sset_destroy(local_lports);
related_lports_destroy(&rt_data->related_lports);
diff --git a/controller/ovn-controller.h b/controller/ovn-controller.h
index df28c62cb..3a0e95377 100644
--- a/controller/ovn-controller.h
+++ b/controller/ovn-controller.h
@@ -45,10 +45,4 @@ const struct ovsrec_bridge *get_bridge(const struct
ovsrec_bridge_table *,
uint32_t get_tunnel_type(const char *name);
-struct pb_ld_binding {
- const struct sbrec_port_binding *pb;
- const struct local_datapath *ld;
- struct hmap_node hmap_node;
-};
-
#endif /* controller/ovn-controller.h */
diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index 38e8590af..5ea54e7d5 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -1292,18 +1292,20 @@ prepare_ipv6_prefixd(struct ovsdb_idl_txn
*ovnsb_idl_txn,
struct ovsdb_idl_index *sbrec_port_binding_by_name,
const struct shash *local_active_ports_ipv6_pd,
const struct sbrec_chassis *chassis,
- const struct sset *active_tunnels)
+ const struct sset *active_tunnels,
+ const struct hmap *local_datapaths)
OVS_REQUIRES(pinctrl_mutex)
{
bool changed = false;
struct shash_node *iter;
SHASH_FOR_EACH (iter, local_active_ports_ipv6_pd) {
- const struct pb_ld_binding *pb_ipv6 = iter->data;
- const struct sbrec_port_binding *pb = pb_ipv6->pb;
+ const struct sbrec_port_binding *pb = iter->data;
+ const struct local_datapath *ld =
+ get_local_datapath(local_datapaths, pb->datapath->tunnel_key);
int j;
- if (!pb_ipv6->ld) {
+ if (!ld) {
continue;
}
@@ -1354,7 +1356,7 @@ prepare_ipv6_prefixd(struct ovsdb_idl_txn *ovnsb_idl_txn,
in6_generate_lla(ea, &ip6_addr);
}
- changed |= fill_ipv6_prefix_state(ovnsb_idl_txn, pb_ipv6->ld,
+ changed |= fill_ipv6_prefix_state(ovnsb_idl_txn, ld,
ea, ip6_addr,
peer->tunnel_key,
peer->datapath->tunnel_key);
@@ -3538,7 +3540,7 @@ pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
prepare_ipv6_ras(local_active_ports_ras, sbrec_port_binding_by_name);
prepare_ipv6_prefixd(ovnsb_idl_txn, sbrec_port_binding_by_name,
local_active_ports_ipv6_pd, chassis,
- active_tunnels);
+ active_tunnels, local_datapaths);
sync_dns_cache(dns_table);
controller_event_run(ovnsb_idl_txn, ce_table, chassis);
ip_mcast_sync(ovnsb_idl_txn, chassis, local_datapaths,
@@ -3982,8 +3984,8 @@ prepare_ipv6_ras(const struct shash
*local_active_ports_ras,
bool changed = false;
SHASH_FOR_EACH (iter, local_active_ports_ras) {
- const struct pb_ld_binding *ras = iter->data;
- const struct sbrec_port_binding *pb = ras->pb;
+ const struct sbrec_port_binding *pb = iter->data;
+ VLOG_INFO("Pop %p", pb);
