The current northd code builds routing policy flows using the same lflow_ref as all other datapath flows.
Our goal is to allow for incremental processing of routing policy flows. This commit gets us a step closer by adding an lflow_ref to the datpath_route_policies structure, and moving code around when building logical flows. We still build the default flows for logical router policies using the logical router's lflow_ref, but the actual meat and potatoes logical router policy flows are now built using an lflow ref per datapath_route_policy. To facilitate this, the actual route policies have been restructured. Now instead of using one large hmap, we use a sparse_array, with an hmap of route_policies per logical router. This will make it easier in the upcoming commits to isolate changes to route_policies based on the datapath they are attached to. Note that no actual incremental processing has been added in this patch. We're simply paving the way for more bite-sized commits to add the actual incremental processing. Signed-off-by: Mark Michelson <[email protected]> --- northd/en-lflow.c | 2 +- northd/en-northd.c | 9 ++- northd/en-route-policies.c | 115 +++++++++++++++++++++++++++++++------ northd/en-route-policies.h | 19 +++++- northd/northd.c | 76 +++++++++++++++++------- northd/northd.h | 2 +- 6 files changed, 178 insertions(+), 45 deletions(-) diff --git a/northd/en-lflow.c b/northd/en-lflow.c index aa1cab3df..b5204a8f2 100644 --- a/northd/en-lflow.c +++ b/northd/en-lflow.c @@ -91,7 +91,7 @@ lflow_get_input_data(struct engine_node *node, lflow_input->bfd_ports = &bfd_sync_data->bfd_ports; lflow_input->route_data = group_ecmp_route_data; lflow_input->route_tables = &routes_data->route_tables; - lflow_input->route_policies = &route_policies_data->route_policies; + lflow_input->dp_route_policies = &route_policies_data->dp_route_policies; lflow_input->igmp_groups = &multicat_igmp_data->igmp_groups; lflow_input->igmp_lflow_ref = multicat_igmp_data->lflow_ref; lflow_input->ic_learned_svc_monitors_map = diff --git a/northd/en-northd.c b/northd/en-northd.c index 2ae4838d2..0ac1fb63a 100644 --- a/northd/en-northd.c +++ b/northd/en-northd.c @@ -526,9 +526,12 @@ en_bfd_sync_run(struct engine_node *node, void *data) struct uuidset bfd_active_connections = UUIDSET_INITIALIZER(&bfd_active_connections); struct uuidset_node *uuid_node; - UUIDSET_FOR_EACH (uuid_node, - &route_policies_data->bfd_active_connections) { - uuidset_insert(&bfd_active_connections, &uuid_node->uuid); + struct datapath_bfd_active_connections *dp_bfd; + SPARSE_ARRAY_FOR_EACH (&route_policies_data->dp_bfd_active_connections, + dp_bfd) { + UUIDSET_FOR_EACH (uuid_node, &dp_bfd->active_connections) { + uuidset_insert(&bfd_active_connections, &uuid_node->uuid); + } } UUIDSET_FOR_EACH (uuid_node, &routes_data->bfd_active_connections) { uuidset_insert(&bfd_active_connections, &uuid_node->uuid); diff --git a/northd/en-route-policies.c b/northd/en-route-policies.c index 86a2818f4..69187ca26 100644 --- a/northd/en-route-policies.c +++ b/northd/en-route-policies.c @@ -17,6 +17,7 @@ #include "en-datapath-logical-router.h" #include "en-route-policies.h" +#include "lflow-mgr.h" #include "northd.h" #include "ovn-nb-idl.h" @@ -163,7 +164,8 @@ static void build_route_policies(struct ovn_datapath *od, struct hmap *route_policies, struct uuidset *bfd_active_connections, - struct simap *chain_ids) + struct simap *chain_ids, + struct hmap *ecmp_group_ids) { /* Create chain numeric ids for policies with chain name set */ for (int i = 0; i < od->nbr->n_policies; i++) { @@ -175,11 +177,11 @@ build_route_policies(struct ovn_datapath *od, } } - size_t hash = uuid_hash(&od->key); - uint16_t ecmp_group_id = 1; + uint32_t last_ecmp_group_id = 0; for (int i = 0; i < od->nbr->n_policies; i++) { const struct nbrec_logical_router_policy *rule = od->nbr->policies[i]; + size_t hash = uuid_hash(&rule->header_.uuid); if (route_policies_lookup(route_policies, hash, rule)) { continue; } @@ -304,17 +306,24 @@ build_route_policies(struct ovn_datapath *od, } } - uint16_t group = 0; + uint32_t ecmp_group_id = 0; if (vector_len(&valid_nexthops) > 1) { - group = ecmp_group_id++; + ecmp_group_id = ovn_allocate_tnlid(ecmp_group_ids, "route_policy", + 1, UINT16_MAX, + &last_ecmp_group_id); + if (ecmp_group_id == 0) { + vector_destroy(&valid_nexthops); + continue; + } } + struct route_policy *new_rp = xmalloc(sizeof *new_rp); *new_rp = (struct route_policy) { .rule = rule, .valid_nexthops = vector_steal(&valid_nexthops), .chain_id = chain_id, .jump_chain_id = jump_chain_id, - .ecmp_group_id = group, + .ecmp_group_id = ecmp_group_id, }; hmap_insert(route_policies, &new_rp->key_node, hash); } @@ -323,20 +332,75 @@ build_route_policies(struct ovn_datapath *od, static void route_policies_init(struct route_policies_data *data) { - hmap_init(&data->route_policies); - uuidset_init(&data->bfd_active_connections); + sparse_array_init(&data->dp_route_policies, 0); + sparse_array_init(&data->dp_bfd_active_connections, 0); +} + +static struct datapath_route_policies * +datapath_route_policies_alloc(const struct ovn_datapath *od) +{ + struct datapath_route_policies *dp_rp = xmalloc(sizeof *dp_rp); + *dp_rp = (struct datapath_route_policies) { + .chain_ids = SIMAP_INITIALIZER(&dp_rp->chain_ids), + .route_policies = HMAP_INITIALIZER(&dp_rp->route_policies), + .ecmp_group_ids = HMAP_INITIALIZER(&dp_rp->ecmp_group_ids), + .dp_index = od->sdp->index, + .lflow_ref = lflow_ref_create(), + }; + + return dp_rp; } static void -route_policies_destroy(struct route_policies_data *data) +datapath_route_policies_destroy(struct datapath_route_policies *dp_rp) { struct route_policy *rp; - HMAP_FOR_EACH_POP (rp, key_node, &data->route_policies) { + HMAP_FOR_EACH_POP (rp, key_node, &dp_rp->route_policies) { vector_destroy(&rp->valid_nexthops); free(rp); }; - hmap_destroy(&data->route_policies); - uuidset_destroy(&data->bfd_active_connections); + hmap_destroy(&dp_rp->route_policies); + ovn_destroy_tnlids(&dp_rp->ecmp_group_ids); + simap_destroy(&dp_rp->chain_ids); + lflow_ref_destroy(dp_rp->lflow_ref); + free(dp_rp); +} + +static struct datapath_bfd_active_connections * +dp_bfd_active_connections_alloc(void) +{ + struct datapath_bfd_active_connections *dp_bfd = + xmalloc(sizeof *dp_bfd); + + *dp_bfd = (struct datapath_bfd_active_connections) { + .active_connections = UUIDSET_INITIALIZER(&dp_bfd->active_connections), + }; + + return dp_bfd; +} + +static void +dp_bfd_active_connections_destroy( + struct datapath_bfd_active_connections *dp_bfd) +{ + uuidset_destroy(&dp_bfd->active_connections); + free(dp_bfd); +} + +static void +route_policies_destroy(struct route_policies_data *data) +{ + struct datapath_route_policies *dp_rp; + SPARSE_ARRAY_FOR_EACH (&data->dp_route_policies, dp_rp) { + datapath_route_policies_destroy(dp_rp); + } + sparse_array_destroy(&data->dp_route_policies); + + struct datapath_bfd_active_connections *dp_bfd; + SPARSE_ARRAY_FOR_EACH (&data->dp_bfd_active_connections, dp_bfd) { + dp_bfd_active_connections_destroy(dp_bfd); + } + sparse_array_destroy(&data->dp_bfd_active_connections); } enum engine_node_state @@ -350,12 +414,29 @@ en_route_policies_run(struct engine_node *node, void *data) struct ovn_datapath *od; HMAP_FOR_EACH (od, key_node, &northd_data->lr_datapaths.datapaths) { - struct simap chain_ids = SIMAP_INITIALIZER(&chain_ids); + struct datapath_route_policies *dp_rp = + datapath_route_policies_alloc(od); + struct datapath_bfd_active_connections *dp_bfd = + dp_bfd_active_connections_alloc(); + build_route_policies(od, &dp_rp->route_policies, + &dp_bfd->active_connections, + &dp_rp->chain_ids, + &dp_rp->ecmp_group_ids); + + if (hmap_is_empty(&dp_rp->route_policies)) { + datapath_route_policies_destroy(dp_rp); + } else { + sparse_array_add_at(&route_policies_data->dp_route_policies, dp_rp, + od->sdp->index); + } - build_route_policies(od, &route_policies_data->route_policies, - &route_policies_data->bfd_active_connections, - &chain_ids); - simap_destroy(&chain_ids); + if (uuidset_is_empty(&dp_bfd->active_connections)) { + dp_bfd_active_connections_destroy(dp_bfd); + } else { + sparse_array_add_at( + &route_policies_data->dp_bfd_active_connections, dp_bfd, + od->sdp->index); + } } return EN_UPDATED; diff --git a/northd/en-route-policies.h b/northd/en-route-policies.h index 9daaa826f..3dd42c41d 100644 --- a/northd/en-route-policies.h +++ b/northd/en-route-policies.h @@ -24,6 +24,8 @@ #include "openvswitch/hmap.h" #include "vec.h" #include "uuidset.h" +#include "sparse-array.h" +#include "simap.h" /* Each instance of this represents a nexthop for a router * policy with "reroute" action. The fields are used for @@ -50,10 +52,23 @@ struct route_policy { uint32_t ecmp_group_id; }; +struct datapath_route_policies { + struct simap chain_ids; + struct hmap route_policies; + struct hmap ecmp_group_ids; + uint32_t dp_index; + struct lflow_ref *lflow_ref; +}; + +struct datapath_bfd_active_connections { + struct uuidset active_connections; +}; + /* Global route policy data exported by en-route-policies. */ struct route_policies_data { - struct hmap route_policies; - struct uuidset bfd_active_connections; + /* Each entry is a struct datapath_route_policies pointer */ + struct sparse_array dp_route_policies; + struct sparse_array dp_bfd_active_connections; }; void en_route_policies_cleanup(void *data); diff --git a/northd/northd.c b/northd/northd.c index 170f3266a..3fa84963d 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -15630,6 +15630,23 @@ build_mcast_lookup_flows_for_lrouter(struct ovn_datapath *od, } } +static void +build_default_ingress_policy_flows_for_lrouter(struct ovn_datapath *od, + struct lflow_table *lflows, + struct lflow_ref *lflow_ref) +{ + ovs_assert(od->nbr); + /* This is a catch-all rule. It has the lowest priority (0) + * does a match-all("1") and pass-through (next) */ + ovn_lflow_add(lflows, od, S_ROUTER_IN_POLICY, 0, "1", + REG_ECMP_GROUP_ID" = 0; next;", + lflow_ref); + ovn_lflow_add(lflows, od, S_ROUTER_IN_POLICY_ECMP, 150, + REG_ECMP_GROUP_ID" == 0", "next;", + lflow_ref); + ovn_lflow_add_default_drop(lflows, od, S_ROUTER_IN_POLICY_ECMP, + lflow_ref); +} /* Logical router ingress table POLICY: Policy. * @@ -15643,25 +15660,14 @@ build_mcast_lookup_flows_for_lrouter(struct ovn_datapath *od, static void build_ingress_policy_flows_for_lrouter( struct ovn_datapath *od, struct lflow_table *lflows, - struct hmap *route_policies, + const struct hmap *route_policies, struct lflow_ref *lflow_ref) { ovs_assert(od->nbr); - /* This is a catch-all rule. It has the lowest priority (0) - * does a match-all("1") and pass-through (next) */ - ovn_lflow_add(lflows, od, S_ROUTER_IN_POLICY, 0, "1", - REG_ECMP_GROUP_ID" = 0; next;", - lflow_ref); - ovn_lflow_add(lflows, od, S_ROUTER_IN_POLICY_ECMP, 150, - REG_ECMP_GROUP_ID" == 0", "next;", - lflow_ref); - ovn_lflow_add_default_drop(lflows, od, S_ROUTER_IN_POLICY_ECMP, - lflow_ref); /* Convert routing policies to flows. */ struct route_policy *rp; - HMAP_FOR_EACH_WITH_HASH (rp, key_node, uuid_hash(&od->key), - route_policies) { + HMAP_FOR_EACH (rp, key_node, route_policies) { const struct nbrec_logical_router_policy *rule = rp->rule; bool is_ecmp_reroute = rp->ecmp_group_id != 0; @@ -20052,7 +20058,7 @@ struct lswitch_flow_build_info { const char *svc_monitor_mac; const struct sampling_app_table *sampling_apps; const struct group_ecmp_route_data *route_data; - struct hmap *route_policies; + struct sparse_array *dp_route_policies; struct simap *route_tables; const struct sbrec_acl_id_table *sbrec_acl_id_table; }; @@ -20118,8 +20124,7 @@ build_lswitch_and_lrouter_iterate_by_lr(struct ovn_datapath *od, lsi->bfd_ports); build_mcast_lookup_flows_for_lrouter(od, lsi->lflows, &lsi->match, od->datapath_lflows); - build_ingress_policy_flows_for_lrouter(od, lsi->lflows, - lsi->route_policies, + build_default_ingress_policy_flows_for_lrouter(od, lsi->lflows, od->datapath_lflows); build_arp_resolve_flows_for_lrouter(od, lsi->lflows, od->datapath_lflows); build_check_pkt_len_flows_for_lrouter(od, lsi->lflows, lsi->lr_ports, @@ -20250,6 +20255,7 @@ build_lflows_thread(void *arg) * - lb_dps->lflow_ref * - lr_stateful_rec->lflow_ref * - ls_stateful_rec->lflow_ref + * - dp_rp->lflow_ref * are not accessed by multiple threads at the same time. */ while (!stop_parallel_processing()) { wait_for_work(control); @@ -20393,6 +20399,23 @@ build_lflows_thread(void *arg) lsi->sbrec_acl_id_table); } } + for (bnum = control->id; + bnum < sparse_array_len(lsi->dp_route_policies); + bnum += control->pool->size) { + struct datapath_route_policies *dp_rp = + sparse_array_get(lsi->dp_route_policies, bnum); + if (!dp_rp) { + /* It's perfectly reasonable for a sparse array to have a + * gap in it. Just move on if that is the case. + */ + continue; + } + od = sparse_array_get(&lsi->lr_datapaths->dps, + dp_rp->dp_index); + build_ingress_policy_flows_for_lrouter(od, lsi->lflows, + &dp_rp->route_policies, + dp_rp->lflow_ref); + } lsi->thread_lflow_counter = thread_lflow_counter; } post_completed_work(control); @@ -20450,7 +20473,7 @@ build_lswitch_and_lrouter_flows( const char *svc_monitor_mac, const struct sampling_app_table *sampling_apps, const struct group_ecmp_route_data *route_data, - struct hmap *route_policies, + struct sparse_array *dp_route_policies, struct simap *route_tables, const struct sbrec_acl_id_table *sbrec_acl_id_table) { @@ -20491,7 +20514,7 @@ build_lswitch_and_lrouter_flows( lsiv[index].sampling_apps = sampling_apps; lsiv[index].route_data = route_data; lsiv[index].route_tables = route_tables; - lsiv[index].route_policies = route_policies; + lsiv[index].dp_route_policies = dp_route_policies; lsiv[index].sbrec_acl_id_table = sbrec_acl_id_table; ds_init(&lsiv[index].match); ds_init(&lsiv[index].actions); @@ -20539,7 +20562,7 @@ build_lswitch_and_lrouter_flows( .sampling_apps = sampling_apps, .route_data = route_data, .route_tables = route_tables, - .route_policies = route_policies, + .dp_route_policies = dp_route_policies, .match = DS_EMPTY_INITIALIZER, .actions = DS_EMPTY_INITIALIZER, .sbrec_acl_id_table = sbrec_acl_id_table, @@ -20620,6 +20643,13 @@ build_lswitch_and_lrouter_flows( lsi.lflows, lsi.sbrec_acl_id_table); } + struct datapath_route_policies *dp_rp; + SPARSE_ARRAY_FOR_EACH (lsi.dp_route_policies, dp_rp) { + od = sparse_array_get(&lsi.lr_datapaths->dps, dp_rp->dp_index); + build_ingress_policy_flows_for_lrouter(od, lsi.lflows, + &dp_rp->route_policies, + dp_rp->lflow_ref); + } ds_destroy(&lsi.match); ds_destroy(&lsi.actions); @@ -20712,7 +20742,7 @@ void build_lflows(struct lflow_input *input_data, input_data->svc_monitor_mac, input_data->sampling_apps, input_data->route_data, - input_data->route_policies, + input_data->dp_route_policies, input_data->route_tables, input_data->sbrec_acl_id_table); build_igmp_lflows(input_data->igmp_groups, @@ -20773,6 +20803,11 @@ lflow_reset_northd_refs(struct lflow_input *lflow_input) HMAP_FOR_EACH (od, key_node, &lflow_input->ls_datapaths->datapaths) { lflow_ref_clear(od->datapath_lflows); } + + struct datapath_route_policies *dp_rp; + SPARSE_ARRAY_FOR_EACH (lflow_input->dp_route_policies, dp_rp) { + lflow_ref_clear(dp_rp->lflow_ref); + } } void @@ -20794,7 +20829,6 @@ lflow_handle_northd_lr_changes(struct tracked_dps *tracked_lrs, .lflows = lflows, .route_data = lflow_input->route_data, .route_tables = lflow_input->route_tables, - .route_policies = lflow_input->route_policies, .match = DS_EMPTY_INITIALIZER, .actions = DS_EMPTY_INITIALIZER, }; diff --git a/northd/northd.h b/northd/northd.h index 9c7ca1363..fb7d3e44e 100644 --- a/northd/northd.h +++ b/northd/northd.h @@ -265,7 +265,7 @@ struct lflow_input { const char *svc_monitor_mac; const struct sampling_app_table *sampling_apps; struct group_ecmp_route_data *route_data; - struct hmap *route_policies; + struct sparse_array *dp_route_policies; struct simap *route_tables; struct hmap *igmp_groups; struct lflow_ref *igmp_lflow_ref; -- 2.55.0 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
