On 10/3/22 22:30, Mark Michelson wrote: > Hi Dumitru, > Hi Mark,
> This seems like a good idea for a change. I've made some minor comments > down below. Thanks! > > On 9/30/22 10:01, Dumitru Ceara wrote: >> This makes it easier to have an overview of what the code does and at the >> same time it allows multiple users to define and manage >> "resource <-> object" dependencies. >> >> Signed-off-by: Dumitru Ceara <[email protected]> >> --- >> controller/lflow.c | 336 >> ++++++++++--------------------------------- >> controller/lflow.h | 67 +-------- >> controller/ovn-controller.c | 55 +++++-- >> lib/automake.mk | 2 >> lib/objdep.c | 261 +++++++++++++++++++++++++++++++++ >> lib/objdep.h | 121 +++++++++++++++ >> 6 files changed, 510 insertions(+), 332 deletions(-) >> create mode 100644 lib/objdep.c >> create mode 100644 lib/objdep.h >> >> diff --git a/controller/lflow.c b/controller/lflow.c >> index cc0f31db0..f9f2e6286 100644 >> --- a/controller/lflow.c >> +++ b/controller/lflow.c >> @@ -61,7 +61,7 @@ struct lookup_port_aux { >> struct ovsdb_idl_index *sbrec_port_binding_by_name; >> const struct sbrec_datapath_binding *dp; >> const struct sbrec_logical_flow *lflow; >> - struct lflow_resource_ref *lfrr; >> + struct objdep_mgr *deps_mgr; >> const struct hmap *chassis_tunnels; >> }; >> @@ -72,8 +72,8 @@ struct condition_aux { >> const struct sset *active_tunnels; >> const struct sbrec_logical_flow *lflow; >> /* Resource reference to store the port name referenced >> - * in is_chassis_resident() to the logical flow. */ >> - struct lflow_resource_ref *lfrr; >> + * in is_chassis_resident() to the object (logical flow). */ >> + struct objdep_mgr *deps_mgr; >> }; >> static struct expr * >> @@ -81,7 +81,7 @@ convert_match_to_expr(const struct >> sbrec_logical_flow *, >> const struct local_datapath *ldp, >> struct expr **prereqs, const struct shash >> *addr_sets, >> const struct shash *port_groups, >> - struct lflow_resource_ref *, bool >> *pg_addr_set_ref); >> + struct objdep_mgr *, bool *pg_addr_set_ref); >> static void >> add_matches_to_flow_table(const struct sbrec_logical_flow *, >> const struct local_datapath *, >> @@ -94,17 +94,6 @@ consider_logical_flow(const struct >> sbrec_logical_flow *lflow, >> bool is_recompute, >> struct lflow_ctx_in *l_ctx_in, >> struct lflow_ctx_out *l_ctx_out); >> -static void lflow_resource_add(struct lflow_resource_ref *, enum >> ref_type, >> - const char *ref_name, const struct >> uuid *, >> - size_t ref_count); >> -static struct ref_lflow_node *ref_lflow_lookup(struct hmap >> *ref_lflow_table, >> - enum ref_type, >> - const char *ref_name); >> -static struct lflow_ref_node *lflow_ref_lookup(struct hmap >> *lflow_ref_table, >> - const struct uuid >> *lflow_uuid); >> -static void ref_lflow_node_destroy(struct ref_lflow_node *); >> -static void lflow_resource_destroy_lflow(struct lflow_resource_ref *, >> - const struct uuid *lflow_uuid); >> static void add_port_sec_flows(const struct shash *binding_lports, >> const struct sbrec_chassis *, >> @@ -125,8 +114,8 @@ lookup_port_cb(const void *aux_, const char >> *port_name, unsigned int *portp) >> /* Store the name that used to lookup the lport to lflow >> reference, so that >> * in the future when the lport's port binding changes, the >> logical flow >> * that references this lport can be reprocessed. */ >> - lflow_resource_add(aux->lfrr, REF_TYPE_PORTBINDING, port_name, >> - &aux->lflow->header_.uuid, 0); >> + objdep_mgr_add(aux->deps_mgr, OBJDEP_TYPE_PORTBINDING, port_name, >> + &aux->lflow->header_.uuid); >> const struct sbrec_port_binding *pb >> = lport_lookup_by_name(aux->sbrec_port_binding_by_name, >> port_name); >> @@ -141,8 +130,8 @@ lookup_port_cb(const void *aux_, const char >> *port_name, unsigned int *portp) >> * this multicast group can be reprocessed. */ >> struct ds mg_key = DS_EMPTY_INITIALIZER; >> get_mc_group_key(port_name, aux->dp->tunnel_key, &mg_key); >> - lflow_resource_add(aux->lfrr, REF_TYPE_MC_GROUP, ds_cstr(&mg_key), >> - &aux->lflow->header_.uuid, 0); >> + objdep_mgr_add(aux->deps_mgr, OBJDEP_TYPE_MC_GROUP, >> ds_cstr(&mg_key), >> + &aux->lflow->header_.uuid); >> ds_destroy(&mg_key); >> const struct sbrec_multicast_group *mg = >> mcgroup_lookup_by_dp_name( >> @@ -180,11 +169,11 @@ is_chassis_resident_cb(const void *c_aux_, const >> char *port_name) >> { >> const struct condition_aux *c_aux = c_aux_; >> - /* Store the port name that used to lookup the lport to lflow >> reference, so >> - * that in the future when the lport's port-binding changes the >> logical >> + /* Store the port name that used to lookup the lport to object >> reference, >> + * so that in the future when the lport's port-binding changes >> the logical >> * flow that references this lport can be reprocessed. */ >> - lflow_resource_add(c_aux->lfrr, REF_TYPE_PORTBINDING, port_name, >> - &c_aux->lflow->header_.uuid, 0); >> + objdep_mgr_add(c_aux->deps_mgr, OBJDEP_TYPE_PORTBINDING, port_name, >> + &c_aux->lflow->header_.uuid); >> const struct sbrec_port_binding *pb >> = lport_lookup_by_name(c_aux->sbrec_port_binding_by_name, >> port_name); >> @@ -207,155 +196,6 @@ is_chassis_resident_cb(const void *c_aux_, const >> char *port_name) >> } >> } >> -void >> -lflow_resource_init(struct lflow_resource_ref *lfrr) >> -{ >> - hmap_init(&lfrr->ref_lflow_table); >> - hmap_init(&lfrr->lflow_ref_table); >> -} >> - >> -void >> -lflow_resource_destroy(struct lflow_resource_ref *lfrr) >> -{ >> - struct ref_lflow_node *rlfn; >> - HMAP_FOR_EACH_SAFE (rlfn, node, &lfrr->ref_lflow_table) { >> - struct lflow_ref_list_node *lrln; >> - HMAP_FOR_EACH_SAFE (lrln, hmap_node, &rlfn->lflow_uuids) { >> - ovs_list_remove(&lrln->list_node); >> - hmap_remove(&rlfn->lflow_uuids, &lrln->hmap_node); >> - free(lrln); >> - } >> - hmap_remove(&lfrr->ref_lflow_table, &rlfn->node); >> - ref_lflow_node_destroy(rlfn); >> - } >> - hmap_destroy(&lfrr->ref_lflow_table); >> - >> - struct lflow_ref_node *lfrn; >> - HMAP_FOR_EACH_SAFE (lfrn, node, &lfrr->lflow_ref_table) { >> - hmap_remove(&lfrr->lflow_ref_table, &lfrn->node); >> - free(lfrn); >> - } >> - hmap_destroy(&lfrr->lflow_ref_table); >> -} >> - >> -void >> -lflow_resource_clear(struct lflow_resource_ref *lfrr) >> -{ >> - lflow_resource_destroy(lfrr); >> - lflow_resource_init(lfrr); >> -} >> - >> -static struct ref_lflow_node* >> -ref_lflow_lookup(struct hmap *ref_lflow_table, >> - enum ref_type type, const char *ref_name) >> -{ >> - struct ref_lflow_node *rlfn; >> - >> - HMAP_FOR_EACH_WITH_HASH (rlfn, node, hash_string(ref_name, type), >> - ref_lflow_table) { >> - if (rlfn->type == type && !strcmp(rlfn->ref_name, ref_name)) { >> - return rlfn; >> - } >> - } >> - return NULL; >> -} >> - >> -static struct lflow_ref_node* >> -lflow_ref_lookup(struct hmap *lflow_ref_table, >> - const struct uuid *lflow_uuid) >> -{ >> - struct lflow_ref_node *lfrn; >> - >> - HMAP_FOR_EACH_WITH_HASH (lfrn, node, uuid_hash(lflow_uuid), >> - lflow_ref_table) { >> - if (uuid_equals(&lfrn->lflow_uuid, lflow_uuid)) { >> - return lfrn; >> - } >> - } >> - return NULL; >> -} >> - >> -static void >> -lflow_resource_add(struct lflow_resource_ref *lfrr, enum ref_type type, >> - const char *ref_name, const struct uuid *lflow_uuid, >> - size_t ref_count) >> -{ >> - struct ref_lflow_node *rlfn = >> ref_lflow_lookup(&lfrr->ref_lflow_table, >> - type, ref_name); >> - struct lflow_ref_node *lfrn = >> lflow_ref_lookup(&lfrr->lflow_ref_table, >> - lflow_uuid); >> - if (rlfn && lfrn) { >> - /* Check if the mapping already existed before adding a new >> one. */ >> - struct lflow_ref_list_node *n; >> - HMAP_FOR_EACH_WITH_HASH (n, hmap_node, uuid_hash(lflow_uuid), >> - &rlfn->lflow_uuids) { >> - if (uuid_equals(&n->lflow_uuid, lflow_uuid)) { >> - return; >> - } >> - } >> - } >> - >> - if (!rlfn) { >> - rlfn = xzalloc(sizeof *rlfn); >> - rlfn->node.hash = hash_string(ref_name, type); >> - rlfn->type = type; >> - rlfn->ref_name = xstrdup(ref_name); >> - hmap_init(&rlfn->lflow_uuids); >> - hmap_insert(&lfrr->ref_lflow_table, &rlfn->node, >> rlfn->node.hash); >> - } >> - >> - if (!lfrn) { >> - lfrn = xzalloc(sizeof *lfrn); >> - lfrn->node.hash = uuid_hash(lflow_uuid); >> - lfrn->lflow_uuid = *lflow_uuid; >> - ovs_list_init(&lfrn->lflow_ref_head); >> - hmap_insert(&lfrr->lflow_ref_table, &lfrn->node, >> lfrn->node.hash); >> - } >> - >> - struct lflow_ref_list_node *lrln = xzalloc(sizeof *lrln); >> - lrln->lflow_uuid = *lflow_uuid; >> - lrln->ref_count = ref_count; >> - lrln->rlfn = rlfn; >> - hmap_insert(&rlfn->lflow_uuids, &lrln->hmap_node, >> uuid_hash(lflow_uuid)); >> - ovs_list_push_back(&lfrn->lflow_ref_head, &lrln->list_node); >> -} >> - >> -static void >> -ref_lflow_node_destroy(struct ref_lflow_node *rlfn) >> -{ >> - free(rlfn->ref_name); >> - hmap_destroy(&rlfn->lflow_uuids); >> - free(rlfn); >> -} >> - >> -static void >> -lflow_resource_destroy_lflow(struct lflow_resource_ref *lfrr, >> - const struct uuid *lflow_uuid) >> -{ >> - struct lflow_ref_node *lfrn = >> lflow_ref_lookup(&lfrr->lflow_ref_table, >> - lflow_uuid); >> - if (!lfrn) { >> - return; >> - } >> - >> - hmap_remove(&lfrr->lflow_ref_table, &lfrn->node); >> - struct lflow_ref_list_node *lrln; >> - LIST_FOR_EACH_SAFE (lrln, list_node, &lfrn->lflow_ref_head) { >> - ovs_list_remove(&lrln->list_node); >> - hmap_remove(&lrln->rlfn->lflow_uuids, &lrln->hmap_node); >> - >> - /* Clean up the node in ref_lflow_table if the resource is not >> - * referred by any logical flows. */ >> - if (hmap_is_empty(&lrln->rlfn->lflow_uuids)) { >> - hmap_remove(&lfrr->ref_lflow_table, &lrln->rlfn->node); >> - ref_lflow_node_destroy(lrln->rlfn); >> - } >> - >> - free(lrln); >> - } >> - free(lfrn); >> -} >> - >> /* Adds the logical flows from the Logical_Flow table to flow >> tables. */ >> static void >> add_logical_flows(struct lflow_ctx_in *l_ctx_in, >> @@ -371,8 +211,8 @@ bool >> lflow_handle_changed_flows(struct lflow_ctx_in *l_ctx_in, >> struct lflow_ctx_out *l_ctx_out) >> { >> - bool ret = true; >> const struct sbrec_logical_flow *lflow; >> + bool ret = true; > > Why is this changed? > A reminiscence of a larger change I was trying out and then ended up reverting. I'll change this back too. >> /* Flood remove the flows for all the tracked lflows. Its >> possible that >> * lflow_add_flows_for_datapath() may have been called before >> calling >> @@ -400,15 +240,14 @@ lflow_handle_changed_flows(struct lflow_ctx_in >> *l_ctx_in, >> struct uuidset_node *ofrn; >> UUIDSET_FOR_EACH (ofrn, &flood_remove_nodes) { >> /* Delete entries from lflow resource reference. */ >> - lflow_resource_destroy_lflow(l_ctx_out->lfrr, &ofrn->uuid); >> + objdep_mgr_remove_obj(l_ctx_out->lflow_deps_mgr, &ofrn->uuid); >> /* Delete conj_ids owned by the lflow. */ >> lflow_conj_ids_free(l_ctx_out->conj_ids, &ofrn->uuid); >> /* Reprocessing the lflow if the sb record is not deleted. */ >> lflow = sbrec_logical_flow_table_get_for_uuid( >> l_ctx_in->logical_flow_table, &ofrn->uuid); >> if (lflow) { >> - VLOG_DBG("re-add lflow "UUID_FMT, >> - UUID_ARGS(&lflow->header_.uuid)); >> + VLOG_DBG("re-add lflow "UUID_FMT, >> UUID_ARGS(&lflow->header_.uuid)); >> /* For the extra lflows that need to be reprocessed >> because of the >> * flood remove, remove it from lflows_processed. */ >> @@ -537,7 +376,7 @@ consider_lflow_for_added_as_ips__( >> .sbrec_port_binding_by_name = >> l_ctx_in->sbrec_port_binding_by_name, >> .dp = dp, >> .lflow = lflow, >> - .lfrr = l_ctx_out->lfrr, >> + .deps_mgr = l_ctx_out->lflow_deps_mgr, >> }; >> struct condition_aux cond_aux = { >> .sbrec_port_binding_by_name = >> l_ctx_in->sbrec_port_binding_by_name, >> @@ -545,7 +384,7 @@ consider_lflow_for_added_as_ips__( >> .chassis = l_ctx_in->chassis, >> .active_tunnels = l_ctx_in->active_tunnels, >> .lflow = lflow, >> - .lfrr = l_ctx_out->lfrr, >> + .deps_mgr = l_ctx_out->lflow_deps_mgr, >> }; >> struct hmap matches = HMAP_INITIALIZER(&matches); >> @@ -591,7 +430,7 @@ consider_lflow_for_added_as_ips__( >> struct expr *expr = convert_match_to_expr(lflow, ldp, &prereqs, >> l_ctx_in->addr_sets, >> l_ctx_in->port_groups, >> - l_ctx_out->lfrr, NULL); >> + >> l_ctx_out->lflow_deps_mgr, NULL); >> shash_replace((struct shash *)l_ctx_in->addr_sets, as_name, >> real_as); >> if (new_fake_as) { >> expr_constant_set_destroy(new_fake_as); >> @@ -787,10 +626,10 @@ lflow_handle_addr_set_update(const char *as_name, >> return false; >> } >> - struct ref_lflow_node *rlfn = >> - ref_lflow_lookup(&l_ctx_out->lfrr->ref_lflow_table, >> REF_TYPE_ADDRSET, >> - as_name); >> - if (!rlfn) { >> + struct resource_to_objects_node *resource_node = >> + objdep_mgr_find_objs(l_ctx_out->lflow_deps_mgr, >> OBJDEP_TYPE_ADDRSET, >> + as_name); >> + if (!resource_node) { >> *changed = false; >> return true; >> } >> @@ -798,22 +637,23 @@ lflow_handle_addr_set_update(const char *as_name, >> *changed = false; >> bool ret = true; >> - struct lflow_ref_list_node *lrln; >> - HMAP_FOR_EACH (lrln, hmap_node, &rlfn->lflow_uuids) { >> - if (uuidset_find(l_ctx_out->lflows_processed, >> &lrln->lflow_uuid)) { >> + struct object_to_resources_list_node *resource_list_node; >> + RESOURCE_FOR_EACH_OBJ (resource_list_node, resource_node) { >> + const struct uuid *obj_uuid = &resource_list_node->obj_uuid; >> + if (uuidset_find(l_ctx_out->lflows_processed, obj_uuid)) { >> VLOG_DBG("lflow "UUID_FMT"has been processed, skip.", >> - UUID_ARGS(&lrln->lflow_uuid)); >> + UUID_ARGS(obj_uuid)); >> continue; >> } >> const struct sbrec_logical_flow *lflow = >> >> sbrec_logical_flow_table_get_for_uuid(l_ctx_in->logical_flow_table, >> - &lrln->lflow_uuid); >> + obj_uuid); >> if (!lflow) { >> /* lflow deletion should be handled in the corresponding >> input >> * handler, so we can skip here. */ >> VLOG_DBG("lflow "UUID_FMT" not found while handling >> updates of " >> "address set %s, skip.", >> - UUID_ARGS(&lrln->lflow_uuid), as_name); >> + UUID_ARGS(obj_uuid), as_name); >> continue; >> } >> *changed = true; >> @@ -825,9 +665,9 @@ lflow_handle_addr_set_update(const char *as_name, >> if (!as_info_from_expr_const(as_name, c, &as_info)) { >> continue; >> } >> - if >> (!ofctrl_remove_flows_for_as_ip(l_ctx_out->flow_table, >> - &lrln->lflow_uuid, >> &as_info, >> - lrln->ref_count)) { >> + if (!ofctrl_remove_flows_for_as_ip( >> + l_ctx_out->flow_table, obj_uuid, &as_info, >> + resource_list_node->ref_count)) { >> ret = false; >> goto done; >> } >> @@ -836,7 +676,7 @@ lflow_handle_addr_set_update(const char *as_name, >> if (as_diff->added) { >> if (!consider_lflow_for_added_as_ips(lflow, as_name, >> - lrln->ref_count, >> + >> resource_list_node->ref_count, >> as_diff->added, >> l_ctx_in, >> l_ctx_out)) { >> ret = false; >> @@ -849,60 +689,33 @@ done: >> return ret; >> } >> -bool >> -lflow_handle_changed_ref(enum ref_type ref_type, const char *ref_name, >> - struct lflow_ctx_in *l_ctx_in, >> - struct lflow_ctx_out *l_ctx_out, >> - bool *changed) >> +void >> +lflow_handle_changed_ref(enum objdep_type type, const char *res_name, >> + struct ovs_list *objs_todo, >> + const void *in_arg, void *out_arg) >> { >> - struct ref_lflow_node *rlfn = >> - ref_lflow_lookup(&l_ctx_out->lfrr->ref_lflow_table, ref_type, >> - ref_name); >> - if (!rlfn) { >> - *changed = false; >> - return true; >> - } >> - VLOG_DBG("Handle changed lflow reference for resource type: %d," >> - " name: %s.", ref_type, ref_name); >> - *changed = false; >> - bool ret = true; >> - >> - struct ovs_list lflows_todo = OVS_LIST_INITIALIZER(&lflows_todo); >> - >> - struct lflow_ref_list_node *lrln, *lrln_uuid; >> - HMAP_FOR_EACH (lrln, hmap_node, &rlfn->lflow_uuids) { >> - if (uuidset_find(l_ctx_out->lflows_processed, >> &lrln->lflow_uuid)) { >> - continue; >> - } >> - /* Use lflow_ref_list_node as list node to store the uuid. >> - * Other fields are not used here. */ >> - lrln_uuid = xmalloc(sizeof *lrln_uuid); >> - lrln_uuid->lflow_uuid = lrln->lflow_uuid; >> - ovs_list_push_back(&lflows_todo, &lrln_uuid->list_node); >> - } >> - if (ovs_list_is_empty(&lflows_todo)) { >> - return true; >> - } >> - *changed = true; >> + struct lflow_ctx_in *l_ctx_in = CONST_CAST(struct lflow_ctx_in *, >> in_arg); >> + struct lflow_ctx_out *l_ctx_out = out_arg; >> /* Re-parse the related lflows. */ >> /* Firstly, flood remove the flows from desired flow table. */ >> + struct object_to_resources_list_node *resource_list_node_uuid; >> struct uuidset flood_remove_nodes = >> UUIDSET_INITIALIZER(&flood_remove_nodes); >> - LIST_FOR_EACH_SAFE (lrln_uuid, list_node, &lflows_todo) { >> - VLOG_DBG("Reprocess lflow "UUID_FMT" for resource type: %d," >> + LIST_FOR_EACH_SAFE (resource_list_node_uuid, list_node, objs_todo) { >> + const struct uuid *obj_uuid = >> &resource_list_node_uuid->obj_uuid; >> + VLOG_DBG("Reprocess lflow "UUID_FMT" for resource type: %s," >> " name: %s.", >> - UUID_ARGS(&lrln_uuid->lflow_uuid), >> - ref_type, ref_name); >> - uuidset_insert(&flood_remove_nodes, &lrln_uuid->lflow_uuid); >> - free(lrln_uuid); >> + UUID_ARGS(obj_uuid), objdep_type_name(type), res_name); >> + uuidset_insert(&flood_remove_nodes, obj_uuid); >> + free(resource_list_node_uuid); >> } >> ofctrl_flood_remove_flows(l_ctx_out->flow_table, >> &flood_remove_nodes); >> /* Secondly, for each lflow that is actually removed, >> reprocessing it. */ >> struct uuidset_node *ofrn; >> UUIDSET_FOR_EACH (ofrn, &flood_remove_nodes) { >> - lflow_resource_destroy_lflow(l_ctx_out->lfrr, &ofrn->uuid); >> + objdep_mgr_remove_obj(l_ctx_out->lflow_deps_mgr, &ofrn->uuid); >> lflow_conj_ids_free(l_ctx_out->conj_ids, &ofrn->uuid); >> const struct sbrec_logical_flow *lflow = >> @@ -910,9 +723,9 @@ lflow_handle_changed_ref(enum ref_type ref_type, >> const char *ref_name, >> &ofrn->uuid); >> if (!lflow) { >> VLOG_DBG("lflow "UUID_FMT" not found while reprocessing >> for" >> - " resource type: %d, name: %s.", >> + " resource type: %s, name: %s.", >> UUID_ARGS(&ofrn->uuid), >> - ref_type, ref_name); >> + objdep_type_name(type), res_name); >> continue; >> } >> @@ -929,8 +742,6 @@ lflow_handle_changed_ref(enum ref_type ref_type, >> const char *ref_name, >> consider_logical_flow(lflow, false, l_ctx_in, l_ctx_out); >> } >> uuidset_destroy(&flood_remove_nodes); >> - >> - return ret; >> } >> static void >> @@ -985,7 +796,7 @@ add_matches_to_flow_table(const struct >> sbrec_logical_flow *lflow, >> .sbrec_port_binding_by_name = >> l_ctx_in->sbrec_port_binding_by_name, >> .dp = ldp->datapath, >> .lflow = lflow, >> - .lfrr = l_ctx_out->lfrr, >> + .deps_mgr = l_ctx_out->lflow_deps_mgr, >> .chassis_tunnels = l_ctx_in->chassis_tunnels, >> }; >> @@ -1100,7 +911,7 @@ convert_match_to_expr(const struct >> sbrec_logical_flow *lflow, >> struct expr **prereqs, >> const struct shash *addr_sets, >> const struct shash *port_groups, >> - struct lflow_resource_ref *lfrr, >> + struct objdep_mgr *mgr, >> bool *pg_addr_set_ref) >> { >> struct shash addr_sets_ref = SHASH_INITIALIZER(&addr_sets_ref); >> @@ -1114,14 +925,15 @@ convert_match_to_expr(const struct >> sbrec_logical_flow *lflow, >> &error); >> struct shash_node *addr_sets_ref_node; >> SHASH_FOR_EACH (addr_sets_ref_node, &addr_sets_ref) { >> - lflow_resource_add(lfrr, REF_TYPE_ADDRSET, >> addr_sets_ref_node->name, >> - &lflow->header_.uuid, >> - *(size_t *)addr_sets_ref_node->data); >> + objdep_mgr_add_with_refcount(mgr, OBJDEP_TYPE_ADDRSET, >> + addr_sets_ref_node->name, >> + &lflow->header_.uuid, >> + *(size_t >> *)addr_sets_ref_node->data); >> } >> const char *port_group_name; >> SSET_FOR_EACH (port_group_name, &port_groups_ref) { >> - lflow_resource_add(lfrr, REF_TYPE_PORTGROUP, port_group_name, >> - &lflow->header_.uuid, 0); >> + objdep_mgr_add(mgr, OBJDEP_TYPE_PORTGROUP, port_group_name, >> + &lflow->header_.uuid); >> } >> if (pg_addr_set_ref) { >> @@ -1165,8 +977,8 @@ consider_logical_flow__(const struct >> sbrec_logical_flow *lflow, >> const char *io_port = smap_get(&lflow->tags, "in_out_port"); >> if (io_port) { >> - lflow_resource_add(l_ctx_out->lfrr, REF_TYPE_PORTBINDING, >> io_port, >> - &lflow->header_.uuid, 0); >> + objdep_mgr_add(l_ctx_out->lflow_deps_mgr, >> OBJDEP_TYPE_PORTBINDING, >> + io_port, &lflow->header_.uuid); >> const struct sbrec_port_binding *pb >> = >> lport_lookup_by_name(l_ctx_in->sbrec_port_binding_by_name, >> io_port); >> @@ -1232,7 +1044,7 @@ consider_logical_flow__(const struct >> sbrec_logical_flow *lflow, >> .sbrec_port_binding_by_name = >> l_ctx_in->sbrec_port_binding_by_name, >> .dp = dp, >> .lflow = lflow, >> - .lfrr = l_ctx_out->lfrr, >> + .deps_mgr = l_ctx_out->lflow_deps_mgr, >> }; >> struct condition_aux cond_aux = { >> .sbrec_port_binding_by_name = >> l_ctx_in->sbrec_port_binding_by_name, >> @@ -1240,7 +1052,7 @@ consider_logical_flow__(const struct >> sbrec_logical_flow *lflow, >> .chassis = l_ctx_in->chassis, >> .active_tunnels = l_ctx_in->active_tunnels, >> .lflow = lflow, >> - .lfrr = l_ctx_out->lfrr, >> + .deps_mgr = l_ctx_out->lflow_deps_mgr, >> }; >> struct lflow_cache_value *lcv = >> @@ -1272,7 +1084,8 @@ consider_logical_flow__(const struct >> sbrec_logical_flow *lflow, >> switch (lcv_type) { >> case LCACHE_T_NONE: >> expr = convert_match_to_expr(lflow, ldp, &prereqs, >> l_ctx_in->addr_sets, >> - l_ctx_in->port_groups, >> l_ctx_out->lfrr, >> + l_ctx_in->port_groups, >> + l_ctx_out->lflow_deps_mgr, >> &pg_addr_set_ref); >> if (!expr) { >> goto done; >> @@ -1345,8 +1158,8 @@ consider_logical_flow__(const struct >> sbrec_logical_flow *lflow, >> /* Cache new entry if caching is enabled. */ >> if (lflow_cache_is_enabled(l_ctx_out->lflow_cache)) { >> if (cached_expr >> - && !lflow_ref_lookup(&l_ctx_out->lfrr->lflow_ref_table, >> - &lflow->header_.uuid)) { >> + && !objdep_mgr_contains_obj(l_ctx_out->lflow_deps_mgr, >> + &lflow->header_.uuid)) { >> lflow_cache_add_matches(l_ctx_out->lflow_cache, >> &lflow->header_.uuid, >> start_conj_id, >> n_conjs, matches, >> matches_size); >> @@ -2448,7 +2261,7 @@ lflow_add_flows_for_datapath(const struct >> sbrec_datapath_binding *dp, >> &lflow->header_.uuid)) { >> continue; >> } >> - /* Don't call lflows_processed_add() because here we >> process the >> + /* Don't call uuidset_insert() because here we process the >> * lflow only for one of the DPs in the DP group, which >> may be >> * incomplete. */ >> consider_logical_flow__(lflow, dp, l_ctx_in, l_ctx_out); >> @@ -2514,7 +2327,11 @@ lflow_handle_flows_for_lport(const struct >> sbrec_port_binding *pb, >> { >> bool changed; >> - if (!lflow_handle_changed_ref(REF_TYPE_PORTBINDING, >> pb->logical_port, >> + if (!objdep_mgr_handle_change(l_ctx_out->lflow_deps_mgr, >> + OBJDEP_TYPE_PORTBINDING, >> + pb->logical_port, >> + lflow_handle_changed_ref, >> + l_ctx_out->lflows_processed, >> l_ctx_in, l_ctx_out, &changed)) { >> return false; >> } >> @@ -2549,7 +2366,11 @@ lflow_handle_changed_port_bindings(struct >> lflow_ctx_in *l_ctx_in, >> && !sbrec_port_binding_is_deleted(pb)) { >> continue; >> } >> - if (!lflow_handle_changed_ref(REF_TYPE_PORTBINDING, >> pb->logical_port, >> + if (!objdep_mgr_handle_change(l_ctx_out->lflow_deps_mgr, >> + OBJDEP_TYPE_PORTBINDING, >> + pb->logical_port, >> + lflow_handle_changed_ref, >> + l_ctx_out->lflows_processed, >> l_ctx_in, l_ctx_out, &changed)) { >> ret = false; >> break; >> @@ -2573,7 +2394,10 @@ lflow_handle_changed_mc_groups(struct >> lflow_ctx_in *l_ctx_in, >> && !sbrec_multicast_group_is_deleted(mg)) { >> continue; >> } >> - if (!lflow_handle_changed_ref(REF_TYPE_MC_GROUP, >> ds_cstr(&mg_key), >> + if (!objdep_mgr_handle_change(l_ctx_out->lflow_deps_mgr, >> + OBJDEP_TYPE_MC_GROUP, >> ds_cstr(&mg_key), >> + lflow_handle_changed_ref, >> + l_ctx_out->lflows_processed, >> l_ctx_in, l_ctx_out, &changed)) { >> ret = false; >> break; >> diff --git a/controller/lflow.h b/controller/lflow.h >> index 8cbe312ca..a9a0f7506 100644 >> --- a/controller/lflow.h >> +++ b/controller/lflow.h >> @@ -16,6 +16,9 @@ >> #ifndef OVN_LFLOW_H >> #define OVN_LFLOW_H 1 >> +#include "lib/ovn-util.h" >> +#include "lib/objdep.h" >> +#include "lib/uuidset.h" >> #include "ovn/logical-fields.h" >> /* Logical_Flow table translation to OpenFlow >> @@ -80,59 +83,6 @@ struct uuid; >> #define OFTABLE_ECMP_NH_MAC 76 >> #define OFTABLE_ECMP_NH 77 >> -enum ref_type { >> - REF_TYPE_ADDRSET, >> - REF_TYPE_PORTGROUP, >> - REF_TYPE_PORTBINDING, >> - REF_TYPE_MC_GROUP >> -}; >> - >> -struct ref_lflow_node { >> - struct hmap_node node; /* node in >> lflow_resource_ref.ref_lflow_table. */ >> - enum ref_type type; /* key */ >> - char *ref_name; /* key */ >> - struct hmap lflow_uuids; /* Contains lflow_ref_list_node. Use >> hmap instead >> - of list so that lflow_resource_add() >> can check >> - and avoid adding redundant entires in >> O(1). */ >> -}; >> - >> -struct lflow_ref_node { >> - struct hmap_node node; /* node in >> lflow_resource_ref.lflow_ref_table. */ >> - struct uuid lflow_uuid; /* key */ >> - struct ovs_list lflow_ref_head; /* Contains lflow_ref_list_node. */ >> -}; >> - >> -/* Maintains the relationship for a pair of named resource and >> - * a lflow, indexed by both ref_lflow_table and lflow_ref_table. */ >> -struct lflow_ref_list_node { >> - struct ovs_list list_node; /* node in >> lflow_ref_node.lflow_ref_head. */ >> - struct hmap_node hmap_node; /* node in >> ref_lflow_node.lflow_uuids. */ >> - struct uuid lflow_uuid; >> - size_t ref_count; /* Reference count of the resource by this lflow. >> - Currently used for the resource type >> REF_TYPE_ADDRSET >> - only, and for other types it is always 0. */ >> - struct ref_lflow_node *rlfn; >> -}; >> - >> -struct lflow_resource_ref { >> - /* A map from a referenced resource type & name (e.g. address_set >> AS1) >> - * to a list of lflows that are referencing the named resource. Data >> - * type of each node in this hmap is struct ref_lflow_node. The >> - * ref_lflow_head in each node points to a list of >> - * lflow_ref_list_node.ref_list. */ >> - struct hmap ref_lflow_table; >> - >> - /* A map from a lflow uuid to a list of named resources that are >> - * referenced by the lflow. Data type of each node in this hmap is >> - * struct lflow_ref_node. The lflow_ref_head in each node points to >> - * a list of lflow_ref_list_node.lflow_list. */ >> - struct hmap lflow_ref_table; >> -}; >> - >> -void lflow_resource_init(struct lflow_resource_ref *); >> -void lflow_resource_destroy(struct lflow_resource_ref *); >> -void lflow_resource_clear(struct lflow_resource_ref *); >> - >> struct lflow_ctx_in { >> struct ovsdb_idl_index *sbrec_multicast_group_by_name_datapath; >> struct ovsdb_idl_index *sbrec_logical_flow_by_logical_datapath; >> @@ -169,7 +119,7 @@ struct lflow_ctx_out { >> struct ovn_desired_flow_table *flow_table; >> struct ovn_extend_table *group_table; >> struct ovn_extend_table *meter_table; >> - struct lflow_resource_ref *lfrr; >> + struct objdep_mgr *lflow_deps_mgr; >> struct lflow_cache *lflow_cache; >> struct conj_ids *conj_ids; >> struct uuidset *lflows_processed; >> @@ -181,10 +131,8 @@ void lflow_init(void); >> void lflow_run(struct lflow_ctx_in *, struct lflow_ctx_out *); >> void lflow_handle_cached_flows(struct lflow_cache *, >> const struct sbrec_logical_flow_table >> *); >> -bool lflow_handle_changed_flows(struct lflow_ctx_in *, struct >> lflow_ctx_out *); >> -bool lflow_handle_changed_ref(enum ref_type, const char *ref_name, >> - struct lflow_ctx_in *, struct >> lflow_ctx_out *, >> - bool *changed); >> +bool lflow_handle_changed_flows(struct lflow_ctx_in *, >> + struct lflow_ctx_out *); >> struct addr_set_diff { >> struct expr_constant_set *added; >> @@ -194,6 +142,9 @@ bool lflow_handle_addr_set_update(const char >> *as_name, struct addr_set_diff *, >> struct lflow_ctx_in *, >> struct lflow_ctx_out *, >> bool *changed); >> +void lflow_handle_changed_ref(enum objdep_type type, const char >> *res_name, >> + struct ovs_list *objs_todo, >> + const void *in_arg, void *out_arg); >> void lflow_handle_changed_mac_bindings( >> struct ovsdb_idl_index *sbrec_port_binding_by_name, >> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c >> index 9969d317f..32a24a076 100644 >> --- a/controller/ovn-controller.c >> +++ b/controller/ovn-controller.c >> @@ -2582,8 +2582,8 @@ struct ed_type_lflow_output { >> struct ovn_extend_table group_table; >> /* meter ids for QoS */ >> struct ovn_extend_table meter_table; >> - /* lflow resource cross reference */ >> - struct lflow_resource_ref lflow_resource_ref; >> + /* lflow <-> resource cross reference */ >> + struct objdep_mgr lflow_deps_mgr;; >> /* conjunciton ID usage information of lflows */ >> struct conj_ids conj_ids; >> @@ -2742,7 +2742,7 @@ init_lflow_ctx(struct engine_node *node, >> l_ctx_out->flow_table = &fo->flow_table; >> l_ctx_out->group_table = &fo->group_table; >> l_ctx_out->meter_table = &fo->meter_table; >> - l_ctx_out->lfrr = &fo->lflow_resource_ref; >> + l_ctx_out->lflow_deps_mgr = &fo->lflow_deps_mgr; >> l_ctx_out->conj_ids = &fo->conj_ids; >> l_ctx_out->lflows_processed = &fo->lflows_processed; >> l_ctx_out->lflow_cache = fo->pd.lflow_cache; >> @@ -2758,7 +2758,7 @@ en_lflow_output_init(struct engine_node *node >> OVS_UNUSED, >> ovn_desired_flow_table_init(&data->flow_table); >> ovn_extend_table_init(&data->group_table); >> ovn_extend_table_init(&data->meter_table); >> - lflow_resource_init(&data->lflow_resource_ref); >> + objdep_mgr_init(&data->lflow_deps_mgr); >> lflow_conj_ids_init(&data->conj_ids); >> uuidset_init(&data->lflows_processed); >> simap_init(&data->hd.ids); >> @@ -2782,7 +2782,7 @@ en_lflow_output_cleanup(void *data) >> ovn_desired_flow_table_destroy(&flow_output_data->flow_table); >> ovn_extend_table_destroy(&flow_output_data->group_table); >> ovn_extend_table_destroy(&flow_output_data->meter_table); >> - lflow_resource_destroy(&flow_output_data->lflow_resource_ref); >> + objdep_mgr_destroy(&flow_output_data->lflow_deps_mgr); >> lflow_conj_ids_destroy(&flow_output_data->conj_ids); >> uuidset_destroy(&flow_output_data->lflows_processed); >> lflow_cache_destroy(flow_output_data->pd.lflow_cache); >> @@ -2818,7 +2818,7 @@ en_lflow_output_run(struct engine_node *node, >> void *data) >> struct ovn_desired_flow_table *lflow_table = &fo->flow_table; >> struct ovn_extend_table *group_table = &fo->group_table; >> struct ovn_extend_table *meter_table = &fo->meter_table; >> - struct lflow_resource_ref *lfrr = &fo->lflow_resource_ref; >> + struct objdep_mgr *lflow_deps_mgr = &fo->lflow_deps_mgr; >> static bool first_run = true; >> if (first_run) { >> @@ -2827,7 +2827,7 @@ en_lflow_output_run(struct engine_node *node, >> void *data) >> ovn_desired_flow_table_clear(lflow_table); >> ovn_extend_table_clear(group_table, false /* desired */); >> ovn_extend_table_clear(meter_table, false /* desired */); >> - lflow_resource_clear(lfrr); >> + objdep_mgr_clear(lflow_deps_mgr); >> lflow_conj_ids_clear(&fo->conj_ids); >> } >> @@ -2958,8 +2958,11 @@ lflow_output_addr_sets_handler(struct >> engine_node *node, void *data) >> } >> SSET_FOR_EACH (ref_name, &as_data->deleted) { >> - if (!lflow_handle_changed_ref(REF_TYPE_ADDRSET, ref_name, >> &l_ctx_in, >> - &l_ctx_out, &changed)) { >> + if (!objdep_mgr_handle_change(l_ctx_out.lflow_deps_mgr, >> + OBJDEP_TYPE_ADDRSET, ref_name, >> + lflow_handle_changed_ref, >> + l_ctx_out.lflows_processed, >> + &l_ctx_in, &l_ctx_out, >> &changed)) { >> return false; >> } >> if (changed) { >> @@ -2973,7 +2976,11 @@ lflow_output_addr_sets_handler(struct >> engine_node *node, void *data) >> &l_ctx_out, &changed)) { >> VLOG_DBG("Can't incrementally handle the change of >> address set %s." >> " Reprocess related lflows.", shash_node->name); >> - if (!lflow_handle_changed_ref(REF_TYPE_ADDRSET, >> shash_node->name, >> + if (!objdep_mgr_handle_change(l_ctx_out.lflow_deps_mgr, >> + OBJDEP_TYPE_ADDRSET, >> + shash_node->name, >> + lflow_handle_changed_ref, >> + l_ctx_out.lflows_processed, >> &l_ctx_in, &l_ctx_out, >> &changed)) { >> return false; >> } >> @@ -2983,8 +2990,11 @@ lflow_output_addr_sets_handler(struct >> engine_node *node, void *data) >> } >> } >> SSET_FOR_EACH (ref_name, &as_data->new) { >> - if (!lflow_handle_changed_ref(REF_TYPE_ADDRSET, ref_name, >> &l_ctx_in, >> - &l_ctx_out, &changed)) { >> + if (!objdep_mgr_handle_change(l_ctx_out.lflow_deps_mgr, >> + OBJDEP_TYPE_ADDRSET, ref_name, >> + lflow_handle_changed_ref, >> + l_ctx_out.lflows_processed, >> + &l_ctx_in, &l_ctx_out, >> &changed)) { >> return false; >> } >> if (changed) { >> @@ -3015,8 +3025,11 @@ lflow_output_port_groups_handler(struct >> engine_node *node, void *data) >> } >> SSET_FOR_EACH (ref_name, &pg_data->deleted) { >> - if (!lflow_handle_changed_ref(REF_TYPE_PORTGROUP, ref_name, >> &l_ctx_in, >> - &l_ctx_out, &changed)) { >> + if (!objdep_mgr_handle_change(l_ctx_out.lflow_deps_mgr, >> + OBJDEP_TYPE_PORTGROUP, ref_name, >> + lflow_handle_changed_ref, >> + l_ctx_out.lflows_processed, >> + &l_ctx_in, &l_ctx_out, >> &changed)) { >> return false; >> } >> if (changed) { >> @@ -3024,8 +3037,11 @@ lflow_output_port_groups_handler(struct >> engine_node *node, void *data) >> } >> } >> SSET_FOR_EACH (ref_name, &pg_data->updated) { >> - if (!lflow_handle_changed_ref(REF_TYPE_PORTGROUP, ref_name, >> &l_ctx_in, >> - &l_ctx_out, &changed)) { >> + if (!objdep_mgr_handle_change(l_ctx_out.lflow_deps_mgr, >> + OBJDEP_TYPE_PORTGROUP, ref_name, >> + lflow_handle_changed_ref, >> + l_ctx_out.lflows_processed, >> + &l_ctx_in, &l_ctx_out, >> &changed)) { >> return false; >> } >> if (changed) { >> @@ -3033,8 +3049,11 @@ lflow_output_port_groups_handler(struct >> engine_node *node, void *data) >> } >> } >> SSET_FOR_EACH (ref_name, &pg_data->new) { >> - if (!lflow_handle_changed_ref(REF_TYPE_PORTGROUP, ref_name, >> &l_ctx_in, >> - &l_ctx_out, &changed)) { >> + if (!objdep_mgr_handle_change(l_ctx_out.lflow_deps_mgr, >> + OBJDEP_TYPE_PORTGROUP, ref_name, >> + lflow_handle_changed_ref, >> + l_ctx_out.lflows_processed, >> + &l_ctx_in, &l_ctx_out, >> &changed)) { >> return false; >> } >> if (changed) { >> diff --git a/lib/automake.mk b/lib/automake.mk >> index 60bead6a6..15d4f84bb 100644 >> --- a/lib/automake.mk >> +++ b/lib/automake.mk >> @@ -31,6 +31,8 @@ lib_libovn_la_SOURCES = \ >> lib/mcast-group-index.c \ >> lib/mcast-group-index.h \ >> lib/lex.c \ >> + lib/objdep.c \ >> + lib/objdep.h \ >> lib/ovn-l7.h \ >> lib/ovn-l7.c \ >> lib/ovn-util.c \ >> diff --git a/lib/objdep.c b/lib/objdep.c >> new file mode 100644 >> index 000000000..0224a5d06 >> --- /dev/null >> +++ b/lib/objdep.c >> @@ -0,0 +1,261 @@ >> +/* Copyright (c) 2015, 2016, 2017 Nicira, Inc. >> + * Copyright (c) 2022, Red Hat, Inc. >> + * >> + * Licensed under the Apache License, Version 2.0 (the "License"); >> + * you may not use this file except in compliance with the License. >> + * You may obtain a copy of the License at: >> + * >> + * http://www.apache.org/licenses/LICENSE-2.0 >> + * >> + * Unless required by applicable law or agreed to in writing, software >> + * distributed under the License is distributed on an "AS IS" BASIS, >> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or >> implied. >> + * See the License for the specific language governing permissions and >> + * limitations under the License. >> + */ >> + >> +#include <config.h> >> + >> +#include "lib/objdep.h" >> +#include "lib/hash.h" >> +#include "lib/util.h" >> +#include "openvswitch/vlog.h" >> + >> +VLOG_DEFINE_THIS_MODULE(resource_dep); >> + >> +static void resource_node_destroy(struct resource_to_objects_node *); >> + >> +void >> +objdep_mgr_init(struct objdep_mgr *mgr) >> +{ >> + hmap_init(&mgr->resource_to_objects_table); >> + hmap_init(&mgr->object_to_resources_table); >> +} >> + >> +void >> +objdep_mgr_destroy(struct objdep_mgr *mgr) >> +{ >> + objdep_mgr_clear(mgr); >> + >> + hmap_destroy(&mgr->resource_to_objects_table); >> + hmap_destroy(&mgr->object_to_resources_table); >> +} >> + >> +void >> +objdep_mgr_clear(struct objdep_mgr *mgr) >> +{ >> + struct resource_to_objects_node *resource_node; >> + HMAP_FOR_EACH_SAFE (resource_node, node, >> &mgr->resource_to_objects_table) { >> + struct object_to_resources_list_node *object_list_node; >> + HMAP_FOR_EACH_SAFE (object_list_node, hmap_node, >> + &resource_node->objs) { >> + ovs_list_remove(&object_list_node->list_node); >> + hmap_remove(&resource_node->objs, >> &object_list_node->hmap_node); >> + free(object_list_node); >> + } >> + hmap_remove(&mgr->resource_to_objects_table, >> &resource_node->node); >> + resource_node_destroy(resource_node); >> + } >> + >> + struct object_to_resources_node *object_node; >> + HMAP_FOR_EACH_SAFE (object_node, node, >> &mgr->object_to_resources_table) { >> + hmap_remove(&mgr->object_to_resources_table, >> &object_node->node); >> + free(object_node); >> + } >> +} >> + >> +void >> +objdep_mgr_add(struct objdep_mgr *mgr, enum objdep_type type, >> + const char *res_name, const struct uuid *obj_uuid) >> +{ >> + objdep_mgr_add_with_refcount(mgr, type, res_name, obj_uuid, 0); >> +} >> + >> +void >> +objdep_mgr_add_with_refcount(struct objdep_mgr *mgr, enum objdep_type >> type, >> + const char *res_name, const struct uuid >> *obj_uuid, >> + size_t ref_count) >> +{ >> + struct resource_to_objects_node *resource_node = >> + objdep_mgr_find_objs(mgr, type, res_name); >> + struct object_to_resources_node *object_node = >> + objdep_mgr_find_resources(mgr, obj_uuid); >> + if (resource_node && object_node) { >> + /* Check if the mapping already existed before adding a new >> one. */ >> + struct object_to_resources_list_node *n; >> + HMAP_FOR_EACH_WITH_HASH (n, hmap_node, uuid_hash(obj_uuid), >> + &resource_node->objs) { >> + if (uuid_equals(&n->obj_uuid, obj_uuid)) { >> + return; >> + } >> + } > + } >> + >> + /* Create the resource node if we didn't have one already (for a >> + * different object). */ >> + if (!resource_node) { >> + resource_node = xzalloc(sizeof *resource_node); >> + resource_node->node.hash = hash_string(res_name, type); >> + resource_node->type = type; >> + resource_node->res_name = xstrdup(res_name); >> + hmap_init(&resource_node->objs); >> + hmap_insert(&mgr->resource_to_objects_table, >> + &resource_node->node, >> + resource_node->node.hash); >> + } >> + >> + /* Create the object node if we didn't have one already (for a >> + * different resource). */ >> + if (!object_node) { >> + object_node = xzalloc(sizeof *object_node); >> + object_node->node.hash = uuid_hash(obj_uuid); >> + object_node->obj_uuid = *obj_uuid; >> + ovs_list_init(&object_node->resources_head); >> + hmap_insert(&mgr->object_to_resources_table, >> + &object_node->node, >> + object_node->node.hash); >> + } >> + >> + struct object_to_resources_list_node *resource_list_node = >> + xzalloc(sizeof *resource_list_node); >> + resource_list_node->obj_uuid = *obj_uuid; >> + resource_list_node->ref_count = ref_count; >> + resource_list_node->resource_node = resource_node; >> + hmap_insert(&resource_node->objs, &resource_list_node->hmap_node, >> + uuid_hash(obj_uuid)); >> + ovs_list_push_back(&object_node->resources_head, >> + &resource_list_node->list_node); >> +} >> + >> +void >> +objdep_mgr_remove_obj(struct objdep_mgr *mgr, const struct uuid >> *obj_uuid) >> +{ >> + struct object_to_resources_node *object_node = >> + objdep_mgr_find_resources(mgr, obj_uuid); >> + if (!object_node) { >> + return; >> + } >> + >> + hmap_remove(&mgr->object_to_resources_table, &object_node->node); >> + >> + struct object_to_resources_list_node *resource_list_node; >> + LIST_FOR_EACH_SAFE (resource_list_node, list_node, >> + &object_node->resources_head) { >> + struct resource_to_objects_node *resource_node = >> + resource_list_node->resource_node; >> + ovs_list_remove(&resource_list_node->list_node); >> + hmap_remove(&resource_node->objs, >> &resource_list_node->hmap_node); >> + >> + /* Clean up the node in ref_obj_table if the resource is not >> + * referred by any logical flows. */ >> + if (hmap_is_empty(&resource_node->objs)) { >> + hmap_remove(&mgr->resource_to_objects_table, >> &resource_node->node); >> + resource_node_destroy(resource_list_node->resource_node); >> + } >> + >> + free(resource_list_node); >> + } >> + free(object_node); >> +} >> + >> +struct resource_to_objects_node * >> +objdep_mgr_find_objs(struct objdep_mgr *mgr, enum objdep_type type, >> + const char *res_name) >> +{ >> + struct resource_to_objects_node *resource_node; >> + >> + HMAP_FOR_EACH_WITH_HASH (resource_node, node, >> hash_string(res_name, type), >> + &mgr->resource_to_objects_table) { >> + if (resource_node->type == type && >> + !strcmp(resource_node->res_name, res_name)) { >> + return resource_node; >> + } >> + } >> + return NULL; >> +} >> + >> +struct object_to_resources_node * >> +objdep_mgr_find_resources(struct objdep_mgr *mgr, >> + const struct uuid *obj_uuid) >> +{ >> + struct object_to_resources_node *object_node; >> + >> + HMAP_FOR_EACH_WITH_HASH (object_node, node, uuid_hash(obj_uuid), >> + &mgr->object_to_resources_table) { >> + if (uuid_equals(&object_node->obj_uuid, obj_uuid)) { >> + return object_node; >> + } >> + } >> + return NULL; >> +} >> + >> +bool >> +objdep_mgr_contains_obj(struct objdep_mgr *mgr, const struct uuid >> *obj_uuid) >> +{ >> + return !!objdep_mgr_find_resources(mgr, obj_uuid); >> +} >> + >> +bool >> +objdep_mgr_handle_change(struct objdep_mgr *mgr, >> + enum objdep_type type, >> + const char *res_name, >> + objdep_change_handler handler, >> + struct uuidset *objs_processed, >> + const void *in_arg, void *out_arg, >> + bool *changed) >> +{ >> + struct resource_to_objects_node *resource_node = >> + objdep_mgr_find_objs(mgr, type, res_name); >> + if (!resource_node) { >> + *changed = false; >> + return true; >> + } >> + VLOG_DBG("Handle changed object reference for resource type: %s," >> + " name: %s.", objdep_type_name(type), res_name); >> + *changed = false; >> + >> + struct ovs_list objs_todo = OVS_LIST_INITIALIZER(&objs_todo); >> + >> + struct object_to_resources_list_node *resource_list_node; >> + HMAP_FOR_EACH (resource_list_node, hmap_node, >> &resource_node->objs) { >> + if (uuidset_find(objs_processed, >> &resource_list_node->obj_uuid)) { >> + continue; >> + } >> + /* Use object_to_resources_list_node as list node to store >> the uuid. >> + * Other fields are not used here. */ >> + struct object_to_resources_list_node *resource_list_node_uuid = >> + xmalloc(sizeof *resource_list_node_uuid); >> + resource_list_node_uuid->obj_uuid = >> resource_list_node->obj_uuid; >> + ovs_list_push_back(&objs_todo, >> &resource_list_node_uuid->list_node); >> + } >> + if (ovs_list_is_empty(&objs_todo)) { >> + return true; >> + } >> + *changed = true; >> + >> + /* This takes ownership of objs_todo. */ >> + handler(type, res_name, &objs_todo, in_arg, out_arg); >> + return true; >> +} >> + >> +const char * >> +objdep_type_name(enum objdep_type type) >> +{ >> + static const char *type_names[OBJDEP_TYPE_MAX] = { >> + [OBJDEP_TYPE_ADDRSET] = "Address_Set", >> + [OBJDEP_TYPE_PORTGROUP] = "Port_Group", >> + [OBJDEP_TYPE_PORTBINDING] = "Port_Binding", >> + [OBJDEP_TYPE_MC_GROUP] = "Multicast_Group", >> + }; >> + >> + ovs_assert(type < OBJDEP_TYPE_MAX); >> + return type_names[type]; >> +} >> + >> +static void >> +resource_node_destroy(struct resource_to_objects_node *resource_node) >> +{ >> + free(resource_node->res_name); >> + hmap_destroy(&resource_node->objs); >> + free(resource_node); >> +} >> diff --git a/lib/objdep.h b/lib/objdep.h >> new file mode 100644 >> index 000000000..059a11a62 >> --- /dev/null >> +++ b/lib/objdep.h >> @@ -0,0 +1,121 @@ >> +/* Copyright (c) 2015, 2016, 2017 Nicira, Inc. >> + * Copyright (c) 2022, Red Hat, Inc. >> + * >> + * Licensed under the Apache License, Version 2.0 (the "License"); >> + * you may not use this file except in compliance with the License. >> + * You may obtain a copy of the License at: >> + * >> + * http://www.apache.org/licenses/LICENSE-2.0 >> + * >> + * Unless required by applicable law or agreed to in writing, software >> + * distributed under the License is distributed on an "AS IS" BASIS, >> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or >> implied. >> + * See the License for the specific language governing permissions and >> + * limitations under the License. >> + */ >> + >> +#ifndef OVN_OBJDEP_H >> +#define OVN_OBJDEP_H 1 >> + >> +#include "lib/uuidset.h" >> +#include "openvswitch/hmap.h" >> +#include "openvswitch/list.h" >> + >> +enum objdep_type { >> + OBJDEP_TYPE_ADDRSET, >> + OBJDEP_TYPE_PORTGROUP, >> + OBJDEP_TYPE_PORTBINDING, >> + OBJDEP_TYPE_MC_GROUP, >> + OBJDEP_TYPE_MAX, >> +}; >> + >> +/* Callbacks provided by users to process changes to resources >> referred by >> + * various objects. */ >> +typedef void (*objdep_change_handler)(enum objdep_type type, >> + const char *res_name, >> + struct ovs_list *ref_nodes, >> + const void *in_arg, void >> *out_arg); > > Suggestion: I noticed that objdep_mgr_handle_change() always returns > true. Would it make sense to make objdep_change_handler return a boolean > instead of void so that it can influence the return value of > objdep_mgr_handle_change()? For lflows, this will continue to always > return true, but if this is meant to be able to handle other data types, > then it makes sense to allow for those types' handlers to return true or > false, right? > Makes sense, I'll do that in v2. >> + >> +/* A node pointing to all objects that refer to a given resource. */ >> +struct resource_to_objects_node { >> + struct hmap_node node; /* node in >> objdep.resource_to_object_table. */ > > s/objdep/objdep_mgr/ > I know it's a bit nitpicky of me, but I was quite confused until I > realized that's what was meant in this comment. > It's ok, I changed my mind at least 4 times before ending up with "objdep_mgr" so that's why there's some inconsistencies in naming here in there. I'll fix them. >> + enum objdep_type type; /* key */ >> + char *res_name; /* key */ >> + struct hmap objs; /* Contains object_to_resources_list_node. >> + * Use hmap instead of list so >> + * that obj_resource_add() can check and >> avoid >> + * and redundant entires in O(1). */ > > s/and redundant entires/any redundant entries/ > Ack. >> +}; >> + >> +#define RESOURCE_FOR_EACH_OBJ(NODE, MAP) \ >> + HMAP_FOR_EACH (NODE, hmap_node, &(MAP)->objs) >> + >> +/* A node pointing to all resources used by a given object (specified by >> + * uuid). >> + */ >> +struct object_to_resources_node { >> + struct hmap_node node; /* node in >> objdep.object_to_resources_table. */ > > s/objdep/objdep_mgr/ > Ack. >> + struct uuid obj_uuid; /* key */ >> + struct ovs_list resources_head; /* Contains elements of type >> + * object_to_resources_list_node. */ >> +}; >> + >> +/* Maintains the relationship for a pair of named resource and >> + * an object, indexed by both resource_to_object_table and >> + * object_to_resources_table. */ >> +struct object_to_resources_list_node { >> + /* node in object_to_resources_node.resources_head. */ >> + struct ovs_list list_node; >> + struct hmap_node hmap_node; /* node in >> resource_to_objects_node.objs. */ >> + struct uuid obj_uuid; >> + size_t ref_count; /* Reference count of the resource by this object. >> + * Currently only used for the resource type >> + * OBJDEP_TYPE_ADDRSET and for other types always >> + * set to 0. */ >> + struct resource_to_objects_node *resource_node; >> +}; >> + >> +struct objdep_mgr { >> + /* A map from a referenced resource type & name (e.g. address_set >> AS1) >> + * to a list of object UUIDs (e.g., lflow) that are referencing >> the named >> + * resource. Data type of each node in this hmap is struct >> + * resource_to_objects_node. The objs in each node point >> + * to a map of object_to_resources_list_node.ref_list. */ >> + struct hmap resource_to_objects_table; >> + >> + /* A map from a obj uuid to a list of named resources that are >> + * referenced by the object. Data type of each node in this hmap is >> + * struct object_to_resources_node. The resources_head in each node >> + * points to a list of object_to_resources_list_node.obj_list. */ >> + struct hmap object_to_resources_table; >> +}; >> + >> +void objdep_mgr_init(struct objdep_mgr *); >> +void objdep_mgr_destroy(struct objdep_mgr *); >> +void objdep_mgr_clear(struct objdep_mgr *); >> + >> +void objdep_mgr_add(struct objdep_mgr *, enum objdep_type type, >> + const char *res_name, const struct uuid *); >> +void objdep_mgr_add_with_refcount(struct objdep_mgr *, >> + enum objdep_type type, >> + const char *res_name, >> + const struct uuid *, >> + size_t ref_count); >> +void objdep_mgr_remove_obj(struct objdep_mgr *, const struct uuid *); >> + >> +struct resource_to_objects_node *objdep_mgr_find_objs( >> + struct objdep_mgr *, enum objdep_type type, const char *res_name); >> +struct object_to_resources_node *objdep_mgr_find_resources( >> + struct objdep_mgr *, const struct uuid *); >> +bool objdep_mgr_contains_obj(struct objdep_mgr *, const struct uuid *); >> + >> +bool objdep_mgr_handle_change(struct objdep_mgr *, enum objdep_type >> type, >> + const char *res_name, >> + objdep_change_handler handler, >> + struct uuidset *objs_processed, >> + const void *in_arg, void *out_arg, >> + bool *changed); >> + >> +const char *objdep_type_name(enum objdep_type type); >> + >> +#endif /* lib/objdep.h */ >> > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
