Skip to content

Commit

Permalink
controller: Fix IPv6 prefix delegation
Browse files Browse the repository at this point in the history 
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: 647920b ("controller: incrementally create ipv6 prefix delegation port_binding list")
Reported-at: https://bugzilla.redhat.com/2108726
Signed-off-by: Ales Musil <amusil@redhat.com>
Acked-by: Mark Michelson <mmichels@redhat.com>
Signed-off-by: Numan Siddique <numans@ovn.org>
(cherry picked from commit bd7ce24)
  • Loading branch information
@almusil @numansiddique
almusil authored and numansiddique committed Aug 2, 2022
1 parent dd0bdf5 commit ea17a67
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 45 deletions.
35 changes: 8 additions & 27 deletions controller/binding.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -485,35 +485,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);
}
}

Expand Down Expand Up @@ -1565,11 +1550,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);
Expand Down Expand Up @@ -2438,12 +2421,10 @@ binding_handle_port_binding_changes(struct binding_ctx_in *b_ctx_in,
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);
Expand Down
8 changes: 4 additions & 4 deletions controller/ovn-controller.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1265,8 +1265,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);
}

Expand Down Expand Up @@ -1378,8 +1378,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);
Expand Down
6 changes: 0 additions & 6 deletions controller/ovn-controller.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
17 changes: 9 additions & 8 deletions controller/pinctrl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1278,18 +1278,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;
}

Expand Down Expand Up @@ -1340,7 +1342,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);
Expand Down Expand Up @@ -3493,7 +3495,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,
Expand Down Expand Up @@ -3935,8 +3937,7 @@ 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;

const char *peer_s = smap_get(&pb->options, "peer");
if (!peer_s) {
Expand Down

0 comments on commit ea17a67

Please sign in to comment.