| Message ID | 7366c766111954b8f9251ebcb97a9837ed916874.1625680335.git.lorenzo.bianconi@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | incrementally process ras-ipv6 pd router ports | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
Like with patch 1, I think the hmaps here can be replaced with shashes instead. See below for another finding On 7/7/21 1:54 PM, Lorenzo Bianconi wrote: > Incrementally manage local_active_ports_ras map for interfaces > where periodic router advertisement has been enabled. This patch > allows to avoid looping over all local interfaces to check if > periodic RA is running on the current port binding. > > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> > --- > controller/binding.c | 7 +++ > controller/binding.h | 1 + > controller/ovn-controller.c | 10 +++- > controller/pinctrl.c | 96 ++++++++++++++++++++----------------- > controller/pinctrl.h | 3 +- > 5 files changed, 72 insertions(+), 45 deletions(-) > > diff --git a/controller/binding.c b/controller/binding.c > index 2cec7ea84..8a66a867c 100644 > --- a/controller/binding.c > +++ b/controller/binding.c > @@ -1675,6 +1675,9 @@ binding_run(struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out) > update_active_pb_ras_pd(pb, b_ctx_out->local_datapaths, > 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, > + "ipv6_ra_send_periodic"); > > enum en_lport_type lport_type = get_lport_type(pb); > > @@ -2517,6 +2520,10 @@ delete_done: > 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, > + "ipv6_ra_send_periodic"); > + > enum en_lport_type lport_type = get_lport_type(pb); > > struct binding_lport *b_lport = > diff --git a/controller/binding.h b/controller/binding.h > index 5a6f46a14..a0ac47dd4 100644 > --- a/controller/binding.h > +++ b/controller/binding.h > @@ -73,6 +73,7 @@ void related_lports_destroy(struct related_lports *); > struct binding_ctx_out { > struct hmap *local_datapaths; > struct hmap *local_active_ports_ipv6_pd; > + struct hmap *local_active_ports_ras; > struct local_binding_data *lbinding_data; > > /* sset of (potential) local lports. */ > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > index a42b96ddb..b779f0c58 100644 > --- a/controller/ovn-controller.c > +++ b/controller/ovn-controller.c > @@ -1044,6 +1044,7 @@ struct ed_type_runtime_data { > struct hmap tracked_dp_bindings; > > struct hmap local_active_ports_ipv6_pd; > + struct hmap local_active_ports_ras; > }; > > /* struct ed_type_runtime_data has the below members for tracking the > @@ -1132,6 +1133,7 @@ en_runtime_data_init(struct engine_node *node OVS_UNUSED, > smap_init(&data->local_iface_ids); > local_binding_data_init(&data->lbinding_data); > hmap_init(&data->local_active_ports_ipv6_pd); > + hmap_init(&data->local_active_ports_ras); > > /* Init the tracked data. */ > hmap_init(&data->tracked_dp_bindings); > @@ -1158,6 +1160,7 @@ en_runtime_data_cleanup(void *data) > } > hmap_destroy(&rt_data->local_datapaths); > hmap_destroy(&rt_data->local_active_ports_ipv6_pd); > + hmap_destroy(&rt_data->local_active_ports_ras); > local_binding_data_destroy(&rt_data->lbinding_data); > } > > @@ -1238,6 +1241,8 @@ init_binding_ctx(struct engine_node *node, > b_ctx_out->local_datapaths = &rt_data->local_datapaths; > b_ctx_out->local_active_ports_ipv6_pd = > &rt_data->local_active_ports_ipv6_pd; > + b_ctx_out->local_active_ports_ras = > + &rt_data->local_active_ports_ras; > b_ctx_out->local_lports = &rt_data->local_lports; > b_ctx_out->local_lports_changed = false; > b_ctx_out->related_lports = &rt_data->related_lports; > @@ -1256,6 +1261,7 @@ en_runtime_data_run(struct engine_node *node, void *data) > struct ed_type_runtime_data *rt_data = data; > struct hmap *local_datapaths = &rt_data->local_datapaths; > struct hmap *local_active_ipv6_pd = &rt_data->local_active_ports_ipv6_pd; > + struct hmap *local_active_ras = &rt_data->local_active_ports_ras; > struct sset *local_lports = &rt_data->local_lports; > struct sset *active_tunnels = &rt_data->active_tunnels; > > @@ -1272,6 +1278,7 @@ en_runtime_data_run(struct engine_node *node, void *data) > } > hmap_clear(local_datapaths); > hmap_clear(local_active_ipv6_pd); > + hmap_clear(local_active_ras); > local_binding_data_destroy(&rt_data->lbinding_data); > sset_destroy(local_lports); > related_lports_destroy(&rt_data->related_lports); > @@ -3286,7 +3293,8 @@ main(int argc, char *argv[]) > br_int, chassis, > &runtime_data->local_datapaths, > &runtime_data->active_tunnels, > - &runtime_data->local_active_ports_ipv6_pd); > + &runtime_data->local_active_ports_ipv6_pd, > + &runtime_data->local_active_ports_ras); > /* Updating monitor conditions if runtime data or > * logical datapath goups changed. */ > if (engine_node_changed(&en_runtime_data) > diff --git a/controller/pinctrl.c b/controller/pinctrl.c > index 1a74af872..199a32685 100644 > --- a/controller/pinctrl.c > +++ b/controller/pinctrl.c > @@ -243,7 +243,9 @@ static void wait_controller_event(struct ovsdb_idl_txn *ovnsb_idl_txn); > static void init_ipv6_ras(void); > static void destroy_ipv6_ras(void); > static void ipv6_ra_wait(long long int send_ipv6_ra_time); > -static void prepare_ipv6_ras(const struct hmap *local_datapaths) > +static void prepare_ipv6_ras( > + const struct hmap *local_active_ports_ras, > + struct ovsdb_idl_index *sbrec_port_binding_by_name) > OVS_REQUIRES(pinctrl_mutex); > static void send_ipv6_ras(struct rconn *swconn, > long long int *send_ipv6_ra_time) > @@ -3409,7 +3411,8 @@ pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn, > const struct sbrec_chassis *chassis, > const struct hmap *local_datapaths, > const struct sset *active_tunnels, > - const struct hmap *local_active_ports_ipv6_pd) > + const struct hmap *local_active_ports_ipv6_pd, > + const struct hmap *local_active_ports_ras) > { > ovs_mutex_lock(&pinctrl_mutex); > pinctrl_set_br_int_name_(br_int->name); > @@ -3422,7 +3425,7 @@ pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn, > sbrec_port_binding_by_name, > sbrec_mac_binding_by_lport_ip, br_int, chassis, > local_datapaths, active_tunnels); > - prepare_ipv6_ras(local_datapaths); > + 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); > @@ -3879,7 +3882,8 @@ send_ipv6_ras(struct rconn *swconn, long long int *send_ipv6_ra_time) > /* Called by pinctrl_run(). Runs with in the main ovn-controller > * thread context. */ > static void > -prepare_ipv6_ras(const struct hmap *local_datapaths) > +prepare_ipv6_ras(const struct hmap *local_active_ports_ras, > + struct ovsdb_idl_index *sbrec_port_binding_by_name) > OVS_REQUIRES(pinctrl_mutex) > { > struct shash_node *iter, *iter_next; > @@ -3890,52 +3894,58 @@ prepare_ipv6_ras(const struct hmap *local_datapaths) > } > > bool changed = false; > - const struct local_datapath *ld; > - HMAP_FOR_EACH (ld, hmap_node, local_datapaths) { > + const struct pb_active_ra_pd *ras; > + HMAP_FOR_EACH (ras, hmap_node, local_active_ports_ras) { > + const struct sbrec_port_binding *pb = ras->pb; > + if (!smap_get_bool(&pb->options, "ipv6_ra_send_periodic", false)) { > + continue; > + } This call to smap_get_bool() is not necessary. In order to be in the ras map, "ipv6_ra_send_periodic" must be true. > > - for (size_t i = 0; i < ld->n_peer_ports; i++) { > - const struct sbrec_port_binding *peer = ld->peer_ports[i].remote; > - const struct sbrec_port_binding *pb = ld->peer_ports[i].local; > + const char *peer_s = smap_get(&pb->options, "peer"); > + if (!peer_s) { > + continue; > + } > > - if (!smap_get_bool(&pb->options, "ipv6_ra_send_periodic", false)) { > - continue; > - } > + const struct sbrec_port_binding *peer > + = lport_lookup_by_name(sbrec_port_binding_by_name, peer_s); > + if (!peer) { > + continue; > + } > > - struct ipv6_ra_config *config = ipv6_ra_update_config(pb); > - if (!config) { > - continue; > - } > + struct ipv6_ra_config *config = ipv6_ra_update_config(pb); > + if (!config) { > + continue; > + } > > - struct ipv6_ra_state *ra > - = shash_find_data(&ipv6_ras, pb->logical_port); > - if (!ra) { > - ra = xzalloc(sizeof *ra); > - ra->config = config; > + struct ipv6_ra_state *ra > + = shash_find_data(&ipv6_ras, pb->logical_port); > + if (!ra) { > + ra = xzalloc(sizeof *ra); > + ra->config = config; > + ra->next_announce = ipv6_ra_calc_next_announce( > + ra->config->min_interval, > + ra->config->max_interval); > + shash_add(&ipv6_ras, pb->logical_port, ra); > + changed = true; > + } else { > + if (config->min_interval != ra->config->min_interval || > + config->max_interval != ra->config->max_interval) > ra->next_announce = ipv6_ra_calc_next_announce( > - ra->config->min_interval, > - ra->config->max_interval); > - shash_add(&ipv6_ras, pb->logical_port, ra); > - changed = true; > - } else { > - if (config->min_interval != ra->config->min_interval || > - config->max_interval != ra->config->max_interval) > - ra->next_announce = ipv6_ra_calc_next_announce( > - config->min_interval, > - config->max_interval); > - ipv6_ra_config_delete(ra->config); > - ra->config = config; > - } > + config->min_interval, > + config->max_interval); > + ipv6_ra_config_delete(ra->config); > + ra->config = config; > + } > > - /* Peer is the logical switch port that the logical > - * router port is connected to. The RA is injected > - * into that logical switch port. > - */ > - ra->port_key = peer->tunnel_key; > - ra->metadata = peer->datapath->tunnel_key; > - ra->delete_me = false; > + /* Peer is the logical switch port that the logical > + * router port is connected to. The RA is injected > + * into that logical switch port. > + */ > + ra->port_key = peer->tunnel_key; > + ra->metadata = peer->datapath->tunnel_key; > + ra->delete_me = false; > > - /* pinctrl_handler thread will send the IPv6 RAs. */ > - } > + /* pinctrl_handler thread will send the IPv6 RAs. */ > } > > /* Remove those that are no longer in the SB database */ > diff --git a/controller/pinctrl.h b/controller/pinctrl.h > index 9d98ee72e..5bb74d5f7 100644 > --- a/controller/pinctrl.h > +++ b/controller/pinctrl.h > @@ -50,7 +50,8 @@ void pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn, > const struct ovsrec_bridge *, const struct sbrec_chassis *, > const struct hmap *local_datapaths, > const struct sset *active_tunnels, > - const struct hmap *local_active_ports_ipv6_pd); > + const struct hmap *local_active_ports_ipv6_pd, > + const struct hmap *local_active_ports_ras); > void pinctrl_wait(struct ovsdb_idl_txn *ovnsb_idl_txn); > void pinctrl_destroy(void); > void pinctrl_set_br_int_name(char *br_int_name); >
diff --git a/controller/binding.c b/controller/binding.c index 2cec7ea84..8a66a867c 100644 --- a/controller/binding.c +++ b/controller/binding.c @@ -1675,6 +1675,9 @@ binding_run(struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out) update_active_pb_ras_pd(pb, b_ctx_out->local_datapaths, 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, + "ipv6_ra_send_periodic"); enum en_lport_type lport_type = get_lport_type(pb); @@ -2517,6 +2520,10 @@ delete_done: 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, + "ipv6_ra_send_periodic"); + enum en_lport_type lport_type = get_lport_type(pb); struct binding_lport *b_lport = diff --git a/controller/binding.h b/controller/binding.h index 5a6f46a14..a0ac47dd4 100644 --- a/controller/binding.h +++ b/controller/binding.h @@ -73,6 +73,7 @@ void related_lports_destroy(struct related_lports *); struct binding_ctx_out { struct hmap *local_datapaths; struct hmap *local_active_ports_ipv6_pd; + struct hmap *local_active_ports_ras; struct local_binding_data *lbinding_data; /* sset of (potential) local lports. */ diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index a42b96ddb..b779f0c58 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -1044,6 +1044,7 @@ struct ed_type_runtime_data { struct hmap tracked_dp_bindings; struct hmap local_active_ports_ipv6_pd; + struct hmap local_active_ports_ras; }; /* struct ed_type_runtime_data has the below members for tracking the @@ -1132,6 +1133,7 @@ en_runtime_data_init(struct engine_node *node OVS_UNUSED, smap_init(&data->local_iface_ids); local_binding_data_init(&data->lbinding_data); hmap_init(&data->local_active_ports_ipv6_pd); + hmap_init(&data->local_active_ports_ras); /* Init the tracked data. */ hmap_init(&data->tracked_dp_bindings); @@ -1158,6 +1160,7 @@ en_runtime_data_cleanup(void *data) } hmap_destroy(&rt_data->local_datapaths); hmap_destroy(&rt_data->local_active_ports_ipv6_pd); + hmap_destroy(&rt_data->local_active_ports_ras); local_binding_data_destroy(&rt_data->lbinding_data); } @@ -1238,6 +1241,8 @@ init_binding_ctx(struct engine_node *node, b_ctx_out->local_datapaths = &rt_data->local_datapaths; b_ctx_out->local_active_ports_ipv6_pd = &rt_data->local_active_ports_ipv6_pd; + b_ctx_out->local_active_ports_ras = + &rt_data->local_active_ports_ras; b_ctx_out->local_lports = &rt_data->local_lports; b_ctx_out->local_lports_changed = false; b_ctx_out->related_lports = &rt_data->related_lports; @@ -1256,6 +1261,7 @@ en_runtime_data_run(struct engine_node *node, void *data) struct ed_type_runtime_data *rt_data = data; struct hmap *local_datapaths = &rt_data->local_datapaths; struct hmap *local_active_ipv6_pd = &rt_data->local_active_ports_ipv6_pd; + struct hmap *local_active_ras = &rt_data->local_active_ports_ras; struct sset *local_lports = &rt_data->local_lports; struct sset *active_tunnels = &rt_data->active_tunnels; @@ -1272,6 +1278,7 @@ en_runtime_data_run(struct engine_node *node, void *data) } hmap_clear(local_datapaths); hmap_clear(local_active_ipv6_pd); + hmap_clear(local_active_ras); local_binding_data_destroy(&rt_data->lbinding_data); sset_destroy(local_lports); related_lports_destroy(&rt_data->related_lports); @@ -3286,7 +3293,8 @@ main(int argc, char *argv[]) br_int, chassis, &runtime_data->local_datapaths, &runtime_data->active_tunnels, - &runtime_data->local_active_ports_ipv6_pd); + &runtime_data->local_active_ports_ipv6_pd, + &runtime_data->local_active_ports_ras); /* Updating monitor conditions if runtime data or * logical datapath goups changed. */ if (engine_node_changed(&en_runtime_data) diff --git a/controller/pinctrl.c b/controller/pinctrl.c index 1a74af872..199a32685 100644 --- a/controller/pinctrl.c +++ b/controller/pinctrl.c @@ -243,7 +243,9 @@ static void wait_controller_event(struct ovsdb_idl_txn *ovnsb_idl_txn); static void init_ipv6_ras(void); static void destroy_ipv6_ras(void); static void ipv6_ra_wait(long long int send_ipv6_ra_time); -static void prepare_ipv6_ras(const struct hmap *local_datapaths) +static void prepare_ipv6_ras( + const struct hmap *local_active_ports_ras, + struct ovsdb_idl_index *sbrec_port_binding_by_name) OVS_REQUIRES(pinctrl_mutex); static void send_ipv6_ras(struct rconn *swconn, long long int *send_ipv6_ra_time) @@ -3409,7 +3411,8 @@ pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn, const struct sbrec_chassis *chassis, const struct hmap *local_datapaths, const struct sset *active_tunnels, - const struct hmap *local_active_ports_ipv6_pd) + const struct hmap *local_active_ports_ipv6_pd, + const struct hmap *local_active_ports_ras) { ovs_mutex_lock(&pinctrl_mutex); pinctrl_set_br_int_name_(br_int->name); @@ -3422,7 +3425,7 @@ pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn, sbrec_port_binding_by_name, sbrec_mac_binding_by_lport_ip, br_int, chassis, local_datapaths, active_tunnels); - prepare_ipv6_ras(local_datapaths); + 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); @@ -3879,7 +3882,8 @@ send_ipv6_ras(struct rconn *swconn, long long int *send_ipv6_ra_time) /* Called by pinctrl_run(). Runs with in the main ovn-controller * thread context. */ static void -prepare_ipv6_ras(const struct hmap *local_datapaths) +prepare_ipv6_ras(const struct hmap *local_active_ports_ras, + struct ovsdb_idl_index *sbrec_port_binding_by_name) OVS_REQUIRES(pinctrl_mutex) { struct shash_node *iter, *iter_next; @@ -3890,52 +3894,58 @@ prepare_ipv6_ras(const struct hmap *local_datapaths) } bool changed = false; - const struct local_datapath *ld; - HMAP_FOR_EACH (ld, hmap_node, local_datapaths) { + const struct pb_active_ra_pd *ras; + HMAP_FOR_EACH (ras, hmap_node, local_active_ports_ras) { + const struct sbrec_port_binding *pb = ras->pb; + if (!smap_get_bool(&pb->options, "ipv6_ra_send_periodic", false)) { + continue; + } - for (size_t i = 0; i < ld->n_peer_ports; i++) { - const struct sbrec_port_binding *peer = ld->peer_ports[i].remote; - const struct sbrec_port_binding *pb = ld->peer_ports[i].local; + const char *peer_s = smap_get(&pb->options, "peer"); + if (!peer_s) { + continue; + } - if (!smap_get_bool(&pb->options, "ipv6_ra_send_periodic", false)) { - continue; - } + const struct sbrec_port_binding *peer + = lport_lookup_by_name(sbrec_port_binding_by_name, peer_s); + if (!peer) { + continue; + } - struct ipv6_ra_config *config = ipv6_ra_update_config(pb); - if (!config) { - continue; - } + struct ipv6_ra_config *config = ipv6_ra_update_config(pb); + if (!config) { + continue; + } - struct ipv6_ra_state *ra - = shash_find_data(&ipv6_ras, pb->logical_port); - if (!ra) { - ra = xzalloc(sizeof *ra); - ra->config = config; + struct ipv6_ra_state *ra + = shash_find_data(&ipv6_ras, pb->logical_port); + if (!ra) { + ra = xzalloc(sizeof *ra); + ra->config = config; + ra->next_announce = ipv6_ra_calc_next_announce( + ra->config->min_interval, + ra->config->max_interval); + shash_add(&ipv6_ras, pb->logical_port, ra); + changed = true; + } else { + if (config->min_interval != ra->config->min_interval || + config->max_interval != ra->config->max_interval) ra->next_announce = ipv6_ra_calc_next_announce( - ra->config->min_interval, - ra->config->max_interval); - shash_add(&ipv6_ras, pb->logical_port, ra); - changed = true; - } else { - if (config->min_interval != ra->config->min_interval || - config->max_interval != ra->config->max_interval) - ra->next_announce = ipv6_ra_calc_next_announce( - config->min_interval, - config->max_interval); - ipv6_ra_config_delete(ra->config); - ra->config = config; - } + config->min_interval, + config->max_interval); + ipv6_ra_config_delete(ra->config); + ra->config = config; + } - /* Peer is the logical switch port that the logical - * router port is connected to. The RA is injected - * into that logical switch port. - */ - ra->port_key = peer->tunnel_key; - ra->metadata = peer->datapath->tunnel_key; - ra->delete_me = false; + /* Peer is the logical switch port that the logical + * router port is connected to. The RA is injected + * into that logical switch port. + */ + ra->port_key = peer->tunnel_key; + ra->metadata = peer->datapath->tunnel_key; + ra->delete_me = false; - /* pinctrl_handler thread will send the IPv6 RAs. */ - } + /* pinctrl_handler thread will send the IPv6 RAs. */ } /* Remove those that are no longer in the SB database */ diff --git a/controller/pinctrl.h b/controller/pinctrl.h index 9d98ee72e..5bb74d5f7 100644 --- a/controller/pinctrl.h +++ b/controller/pinctrl.h @@ -50,7 +50,8 @@ void pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn, const struct ovsrec_bridge *, const struct sbrec_chassis *, const struct hmap *local_datapaths, const struct sset *active_tunnels, - const struct hmap *local_active_ports_ipv6_pd); + const struct hmap *local_active_ports_ipv6_pd, + const struct hmap *local_active_ports_ras); void pinctrl_wait(struct ovsdb_idl_txn *ovnsb_idl_txn); void pinctrl_destroy(void); void pinctrl_set_br_int_name(char *br_int_name);
Incrementally manage local_active_ports_ras map for interfaces where periodic router advertisement has been enabled. This patch allows to avoid looping over all local interfaces to check if periodic RA is running on the current port binding. Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> --- controller/binding.c | 7 +++ controller/binding.h | 1 + controller/ovn-controller.c | 10 +++- controller/pinctrl.c | 96 ++++++++++++++++++++----------------- controller/pinctrl.h | 3 +- 5 files changed, 72 insertions(+), 45 deletions(-)