On Wed, Feb 9, 2022 at 12:55 PM Han Zhou <hz...@ovn.org> wrote: > > This patch tracks individual address information when parsing address > sets from logical flows, and link to the corresponding desired flow > resulted from the IP. > > Signed-off-by: Han Zhou <hz...@ovn.org>
Hi Han, Please see below for a couple of small comments. Numan > --- > controller/lflow.c | 19 ++++-- > controller/ofctrl.c | 130 ++++++++++++++++++++++++++++++++++++++---- > controller/ofctrl.h | 19 ++++-- > controller/physical.c | 2 +- > include/ovn/expr.h | 9 +++ > lib/expr.c | 81 ++++++++++++++++++++------ > 6 files changed, 224 insertions(+), 36 deletions(-) > > diff --git a/controller/lflow.c b/controller/lflow.c > index 8b32c7469..79652735c 100644 > --- a/controller/lflow.c > +++ b/controller/lflow.c > @@ -692,13 +692,23 @@ add_matches_to_flow_table(const struct > sbrec_logical_flow *lflow, > } > } > } > + > + struct addrset_info as_info = { > + .name = m->as_name, > + .ip = m->as_ip, > + .mask = m->as_mask > + }; > if (!m->n) { > ofctrl_add_flow_metered(l_ctx_out->flow_table, ptable, > lflow->priority, > lflow->header_.uuid.parts[0], &m->match, > &ofpacts, &lflow->header_.uuid, > - ctrl_meter_id); > + ctrl_meter_id, > + as_info.name ? &as_info : NULL); > } else { > + if (m->n > 1) { > + ovs_assert(!as_info.name); > + } > uint64_t conj_stubs[64 / 8]; > struct ofpbuf conj; > > @@ -716,7 +726,8 @@ add_matches_to_flow_table(const struct sbrec_logical_flow > *lflow, > ofctrl_add_or_append_flow(l_ctx_out->flow_table, ptable, > lflow->priority, 0, > &m->match, &conj, &lflow->header_.uuid, > - ctrl_meter_id); > + ctrl_meter_id, > + as_info.name ? &as_info : NULL); > ofpbuf_uninit(&conj); > } > } > @@ -1524,7 +1535,7 @@ add_lb_ct_snat_hairpin_dp_flows(struct > ovn_controller_lb *lb, > ofctrl_add_or_append_flow(flow_table, OFTABLE_CT_SNAT_HAIRPIN, 200, > lb->slb->header_.uuid.parts[0], > &dp_match, &dp_acts, > &lb->slb->header_.uuid, > - NX_CTLR_NO_METER); > + NX_CTLR_NO_METER, NULL); > } > > ofpbuf_uninit(&dp_acts); > @@ -1698,7 +1709,7 @@ add_lb_ct_snat_hairpin_vip_flow(struct > ovn_controller_lb *lb, > ofctrl_add_or_append_flow(flow_table, OFTABLE_CT_SNAT_HAIRPIN, priority, > lb->slb->header_.uuid.parts[0], > &match, &ofpacts, &lb->slb->header_.uuid, > - NX_CTLR_NO_METER); > + NX_CTLR_NO_METER, NULL); > ofpbuf_uninit(&ofpacts); > > } > diff --git a/controller/ofctrl.c b/controller/ofctrl.c > index bcd6cea79..7671a3b7a 100644 > --- a/controller/ofctrl.c > +++ b/controller/ofctrl.c > @@ -165,15 +165,35 @@ struct sb_to_flow { > struct uuid sb_uuid; > struct ovs_list flows; /* A list of struct sb_flow_ref nodes that > are referenced by the sb_uuid. */ > + struct ovs_list addrsets; /* A list of struct sb_addrset_ref. */ > }; > > struct sb_flow_ref { > struct ovs_list sb_list; /* List node in desired_flow.references. */ > - struct ovs_list flow_list; /* List node in sb_to_flow.desired_flows. */ > + struct ovs_list flow_list; /* List node in sb_to_flow.flows. */ > + struct ovs_list as_ip_flow_list; /* List node in ip_to_flow_node.flows. > */ > struct desired_flow *flow; > struct uuid sb_uuid; > }; > > +struct sb_addrset_ref { > + struct ovs_list list_node; /* List node in sb_to_flow.addrsets. */ > + char *name; /* Name of the address set. */ > + struct hmap ip_to_flow_map; /* map from IPs in the address set to flows. > + Each node is struct ip_to_flow_node. */ > +}; > + > +struct ip_to_flow_node { I'd suggest to change the name of this struct to "as_ip_to_flow_node" to make it explicitly clear that the "ip" here refers to the address set ip. > + struct hmap_node hmap_node; /* Node in sb_addrset_ref.ip_to_flow_map. */ > + struct in6_addr as_ip; > + struct in6_addr as_mask; > + > + /* A list of struct sb_flow_ref. A single IP in an address set can be > + * used by multiple flows. e.g., in match: > + * ip.src == $as1 && ip.dst == $as1. */ > + struct ovs_list flows; > +}; > + > /* An installed flow, in static variable installed_lflows/installed_pflows. > * > * Installed flows are updated in ofctrl_put for maintaining the flow > @@ -1030,9 +1050,26 @@ sb_to_flow_find(struct hmap *uuid_flow_table, const > struct uuid *sb_uuid) > return NULL; > } > > +static struct ip_to_flow_node * > +ip_to_flow_find(struct hmap *ip_to_flow_map, const struct in6_addr *as_ip, > + const struct in6_addr *as_mask) > +{ > + uint32_t hash = hash_bytes(as_ip, sizeof *as_ip, 0); > + > + struct ip_to_flow_node *itfn; > + HMAP_FOR_EACH_WITH_HASH (itfn, hmap_node, hash, ip_to_flow_map) { > + if (ipv6_addr_equals(&itfn->as_ip, as_ip) > + && ipv6_addr_equals(&itfn->as_mask, as_mask)) { > + return itfn; > + } > + } > + return NULL; > +} > + > static void > link_flow_to_sb(struct ovn_desired_flow_table *flow_table, > - struct desired_flow *f, const struct uuid *sb_uuid) > + struct desired_flow *f, const struct uuid *sb_uuid, > + const struct addrset_info *as_info) > { > struct sb_flow_ref *sfr = xmalloc(sizeof *sfr); > mem_stats.sb_flow_ref_usage += sb_flow_ref_size(sfr); > @@ -1046,10 +1083,48 @@ link_flow_to_sb(struct ovn_desired_flow_table > *flow_table, > mem_stats.sb_flow_ref_usage += sb_to_flow_size(stf); > stf->sb_uuid = *sb_uuid; > ovs_list_init(&stf->flows); > + ovs_list_init(&stf->addrsets); > hmap_insert(&flow_table->uuid_flow_table, &stf->hmap_node, > uuid_hash(sb_uuid)); > } > ovs_list_insert(&stf->flows, &sfr->flow_list); > + > + if (!as_info) { > + ovs_list_init(&sfr->as_ip_flow_list); > + return; > + } > + > + /* link flow to address_set + ip */ > + struct sb_addrset_ref *sar; > + bool found = false; > + LIST_FOR_EACH (sar, list_node, &stf->addrsets) { > + if (!strcmp(sar->name, as_info->name)) { > + found = true; > + break; > + } > + } > + if (!found) { > + sar = xmalloc(sizeof *sar); > + mem_stats.sb_flow_ref_usage += sizeof *sar; > + sar->name = xstrdup(as_info->name); > + hmap_init(&sar->ip_to_flow_map); > + ovs_list_insert(&stf->addrsets, &sar->list_node); > + } > + > + struct ip_to_flow_node * itfn = ip_to_flow_find(&sar->ip_to_flow_map, > + &as_info->ip, > + &as_info->mask); > + if (!itfn) { > + itfn = xmalloc(sizeof *itfn); > + mem_stats.sb_flow_ref_usage += sizeof *itfn; > + itfn->as_ip = as_info->ip; > + itfn->as_mask = as_info->mask; > + ovs_list_init(&itfn->flows); > + uint32_t hash = hash_bytes(&as_info->ip, sizeof as_info->ip, 0); > + hmap_insert(&sar->ip_to_flow_map, &itfn->hmap_node, hash); > + } > + > + ovs_list_insert(&itfn->flows, &sfr->as_ip_flow_list); > } > > /* Flow table interfaces to the rest of ovn-controller. */ > @@ -1068,13 +1143,17 @@ link_flow_to_sb(struct ovn_desired_flow_table > *flow_table, > void > ofctrl_check_and_add_flow_metered(struct ovn_desired_flow_table *flow_table, > uint8_t table_id, uint16_t priority, > - uint64_t cookie, const struct match *match, > + uint64_t cookie, > + const struct match *match, > const struct ofpbuf *actions, > const struct uuid *sb_uuid, > - uint32_t meter_id, bool log_duplicate_flow) > + uint32_t meter_id, > + const struct addrset_info *as_info, > + bool log_duplicate_flow) > { > struct desired_flow *f = desired_flow_alloc(table_id, priority, cookie, > - match, actions, meter_id); > + match, actions, > + meter_id); > > if (desired_flow_lookup_check_uuid(flow_table, &f->flow, sb_uuid)) { > if (log_duplicate_flow) { > @@ -1091,7 +1170,7 @@ ofctrl_check_and_add_flow_metered(struct > ovn_desired_flow_table *flow_table, > > hmap_insert(&flow_table->match_flow_table, &f->match_hmap_node, > f->flow.hash); > - link_flow_to_sb(flow_table, f, sb_uuid); > + link_flow_to_sb(flow_table, f, sb_uuid, as_info); > track_flow_add_or_modify(flow_table, f); > ovn_flow_log(&f->flow, "ofctrl_add_flow"); > } > @@ -1103,7 +1182,7 @@ ofctrl_add_flow(struct ovn_desired_flow_table > *desired_flows, > const struct uuid *sb_uuid) > { > ofctrl_add_flow_metered(desired_flows, table_id, priority, cookie, > - match, actions, sb_uuid, NX_CTLR_NO_METER); > + match, actions, sb_uuid, NX_CTLR_NO_METER, NULL); > } > > void > @@ -1111,11 +1190,12 @@ ofctrl_add_flow_metered(struct ovn_desired_flow_table > *desired_flows, > uint8_t table_id, uint16_t priority, uint64_t cookie, > const struct match *match, > const struct ofpbuf *actions, > - const struct uuid *sb_uuid, uint32_t meter_id) > + const struct uuid *sb_uuid, uint32_t meter_id, > + const struct addrset_info *as_info) > { > ofctrl_check_and_add_flow_metered(desired_flows, table_id, priority, > cookie, match, actions, sb_uuid, > - meter_id, true); > + meter_id, as_info, true); > } > > /* Either add a new flow, or append actions on an existing flow. If the > @@ -1127,7 +1207,8 @@ ofctrl_add_or_append_flow(struct ovn_desired_flow_table > *desired_flows, > const struct match *match, > const struct ofpbuf *actions, > const struct uuid *sb_uuid, > - uint32_t meter_id) > + uint32_t meter_id, > + const struct addrset_info *as_info) > { > struct desired_flow *existing; > struct desired_flow *f; > @@ -1156,11 +1237,20 @@ ofctrl_add_or_append_flow(struct > ovn_desired_flow_table *desired_flows, > ofpbuf_uninit(&compound); > desired_flow_destroy(f); > f = existing; > + > + /* Remove as_info tracking for the existing flow. */ > + struct sb_flow_ref *sfr; > + LIST_FOR_EACH (sfr, sb_list, &f->references) { > + ovs_list_remove(&sfr->as_ip_flow_list); > + ovs_list_init(&sfr->as_ip_flow_list); > + } > + /* Link to sb but don't track the as_info. */ Can you please add the comment to why the as_info is not tracked here ? >From what I understand we don't track the as_info for flows with conjunction actions. Numan > + link_flow_to_sb(desired_flows, f, sb_uuid, NULL); > } else { > hmap_insert(&desired_flows->match_flow_table, &f->match_hmap_node, > f->flow.hash); > + link_flow_to_sb(desired_flows, f, sb_uuid, as_info); > } > - link_flow_to_sb(desired_flows, f, sb_uuid); > track_flow_add_or_modify(desired_flows, f); > > if (existing) { > @@ -1269,6 +1359,7 @@ remove_flows_from_sb_to_flow(struct > ovn_desired_flow_table *flow_table, > LIST_FOR_EACH_SAFE (sfr, next, flow_list, &stf->flows) { > ovs_list_remove(&sfr->sb_list); > ovs_list_remove(&sfr->flow_list); > + ovs_list_remove(&sfr->as_ip_flow_list); > struct desired_flow *f = sfr->flow; > mem_stats.sb_flow_ref_usage -= sb_flow_ref_size(sfr); > free(sfr); > @@ -1286,6 +1377,22 @@ remove_flows_from_sb_to_flow(struct > ovn_desired_flow_table *flow_table, > } > } > > + struct sb_addrset_ref *sar, *next_sar; > + LIST_FOR_EACH_SAFE (sar, next_sar, list_node, &stf->addrsets) { > + ovs_list_remove(&sar->list_node); > + struct ip_to_flow_node *itfn, *itfn_next; > + HMAP_FOR_EACH_SAFE (itfn, itfn_next, hmap_node, > &sar->ip_to_flow_map) { > + hmap_remove(&sar->ip_to_flow_map, &itfn->hmap_node); > + ovs_assert(ovs_list_is_empty(&itfn->flows)); > + mem_stats.sb_flow_ref_usage -= sizeof *itfn; > + free(itfn); > + } > + hmap_destroy(&sar->ip_to_flow_map); > + mem_stats.sb_flow_ref_usage -= (sizeof *sar + strlen(sar->name) + 1); > + free(sar->name); > + free(sar); > + } > + > hmap_remove(&flow_table->uuid_flow_table, &stf->hmap_node); > mem_stats.sb_flow_ref_usage -= sb_to_flow_size(stf); > free(stf); > @@ -1300,6 +1407,7 @@ remove_flows_from_sb_to_flow(struct > ovn_desired_flow_table *flow_table, > ovs_assert(!ovs_list_is_empty(&f->references)); > LIST_FOR_EACH (sfr, sb_list, &f->references) { > ovs_list_remove(&sfr->flow_list); > + ovs_list_remove(&sfr->as_ip_flow_list); > } > } > LIST_FOR_EACH_SAFE (f, f_next, list_node, &to_be_removed) { > diff --git a/controller/ofctrl.h b/controller/ofctrl.h > index 014de210d..4ec328c24 100644 > --- a/controller/ofctrl.h > +++ b/controller/ofctrl.h > @@ -71,24 +71,34 @@ char *ofctrl_inject_pkt(const struct ovsrec_bridge > *br_int, > const struct shash *port_groups); > > /* Flow table interfaces to the rest of ovn-controller. */ > + > +/* Information of IP of an address set used to track a flow that is generated > + * from a logical flow referencing address set(s). */ > +struct addrset_info { > + const char *name; /* The address set's name. */ > + struct in6_addr ip; /* An IP in the address set. */ > + struct in6_addr mask; /* The mask of the IP. */ > +}; > void ofctrl_add_flow(struct ovn_desired_flow_table *, uint8_t table_id, > uint16_t priority, uint64_t cookie, > const struct match *, const struct ofpbuf *ofpacts, > const struct uuid *); > > -void ofctrl_add_or_append_flow(struct ovn_desired_flow_table *desired_flows, > +void ofctrl_add_or_append_flow(struct ovn_desired_flow_table *, > uint8_t table_id, uint16_t priority, > - uint64_t cookie, const struct match *match, > + uint64_t cookie, const struct match *, > const struct ofpbuf *actions, > const struct uuid *sb_uuid, > - uint32_t meter_id); > + uint32_t meter_id, > + const struct addrset_info *); > > void ofctrl_add_flow_metered(struct ovn_desired_flow_table *desired_flows, > uint8_t table_id, uint16_t priority, > uint64_t cookie, const struct match *match, > const struct ofpbuf *actions, > const struct uuid *sb_uuid, > - uint32_t meter_id); > + uint32_t meter_id, > + const struct addrset_info *); > > /* Removes a bundles of flows from the flow table for a specific sb_uuid. The > * flows are removed only if they are not referenced by any other sb_uuid(s). > @@ -123,6 +133,7 @@ void ofctrl_check_and_add_flow_metered(struct > ovn_desired_flow_table *, > uint64_t cookie, const struct match *, > const struct ofpbuf *ofpacts, > const struct uuid *, uint32_t > meter_id, > + const struct addrset_info *, > bool log_duplicate_flow); > > > diff --git a/controller/physical.c b/controller/physical.c > index 6bfa2304d..033828d57 100644 > --- a/controller/physical.c > +++ b/controller/physical.c > @@ -835,7 +835,7 @@ put_local_common_flows(uint32_t dp_key, > put_stack(MFF_IN_PORT, ofpact_put_STACK_POP(ofpacts_p)); > ofctrl_check_and_add_flow_metered(flow_table, OFTABLE_SAVE_INPORT, > 100, > 0, &match, ofpacts_p, hc_uuid, > - NX_CTLR_NO_METER, false); > + NX_CTLR_NO_METER, NULL, false); > } > } > > diff --git a/include/ovn/expr.h b/include/ovn/expr.h > index 3b5653f7b..5572a1071 100644 > --- a/include/ovn/expr.h > +++ b/include/ovn/expr.h > @@ -367,6 +367,8 @@ bool expr_relop_from_token(enum lex_type type, enum > expr_relop *relop); > struct expr { > struct ovs_list node; /* In parent EXPR_T_AND or EXPR_T_OR if any. > */ > enum expr_type type; /* Expression type. */ > + char *as_name; /* Address set name. Null if it is not an > + address set. */ > > union { > /* EXPR_T_CMP. > @@ -469,6 +471,11 @@ struct expr_match { > struct match match; > struct cls_conjunction *conjunctions; > size_t n, allocated; > + > + /* Tracked address set information. */ > + char *as_name; > + struct in6_addr as_ip; > + struct in6_addr as_mask; > }; > > uint32_t expr_to_matches(const struct expr *, > @@ -526,6 +533,8 @@ struct expr_constant_set { > size_t n_values; /* Number of constants. */ > enum expr_constant_type type; /* Type of the constants. */ > bool in_curlies; /* Whether the constants were in {}. */ > + char *as_name; /* Name of an address set. It is NULL if > not > + an address set. */ > }; > > bool expr_constant_set_parse(struct lexer *, struct expr_constant_set *); > diff --git a/lib/expr.c b/lib/expr.c > index 5fc6c1ce9..af083190f 100644 > --- a/lib/expr.c > +++ b/lib/expr.c > @@ -160,7 +160,7 @@ expr_relop_test(enum expr_relop relop, int cmp) > struct expr * > expr_create_andor(enum expr_type type) > { > - struct expr *e = xmalloc(sizeof *e); > + struct expr *e = xzalloc(sizeof *e); > e->type = type; > ovs_list_init(&e->andor); > return e; > @@ -190,9 +190,17 @@ expr_combine(enum expr_type type, struct expr *a, struct > expr *b) > } else { > ovs_list_push_back(&a->andor, &b->node); > } > + if (a->as_name) { > + free(a->as_name); > + a->as_name = NULL; > + } > return a; > } else if (b->type == type) { > ovs_list_push_front(&b->andor, &a->node); > + if (b->as_name) { > + free(b->as_name); > + b->as_name = NULL; > + } > return b; > } else { > struct expr *e = expr_create_andor(type); > @@ -220,7 +228,7 @@ expr_insert_andor(struct expr *andor, struct expr > *before, struct expr *new) > struct expr * > expr_create_boolean(bool b) > { > - struct expr *e = xmalloc(sizeof *e); > + struct expr *e = xzalloc(sizeof *e); > e->type = EXPR_T_BOOLEAN; > e->boolean = b; > return e; > @@ -680,6 +688,10 @@ make_cmp(struct expr_context *ctx, > e = expr_combine(r == EXPR_R_EQ ? EXPR_T_OR : EXPR_T_AND, > e, make_cmp__(f, r, &cs->values[i])); > } > + /* Track address set */ > + if (r == EXPR_R_EQ && e->type == EXPR_T_OR && cs->as_name) { > + e->as_name = xstrdup(cs->as_name); > + } > exit: > expr_constant_set_destroy(cs); > return e; > @@ -802,6 +814,10 @@ parse_addr_sets(struct expr_context *ctx, struct > expr_constant_set *cs, > return false; > } > > + if (!cs->n_values) { > + cs->as_name = xstrdup(ctx->lexer->token.s); > + } > + > size_t n_values = cs->n_values + addr_sets->n_values; > if (n_values >= *allocated_values) { > cs->values = xrealloc(cs->values, n_values * sizeof *cs->values); > @@ -875,6 +891,13 @@ parse_constant(struct expr_context *ctx, struct > expr_constant_set *cs, > sizeof *cs->values); > } > > + if (cs->as_name) { > + /* Combining other values to the constant set that is tracking an > + * address set, so untrack it. */ > + free(cs->as_name); > + cs->as_name = NULL; > + } > + > if (ctx->lexer->token.type == LEX_T_STRING) { > if (!assign_constant_set_type(ctx, cs, EXPR_C_STRING)) { > return false; > @@ -1057,6 +1080,7 @@ expr_constant_set_destroy(struct expr_constant_set *cs) > } > } > free(cs->values); > + free(cs->as_name); > } > } > > @@ -1244,6 +1268,7 @@ expr_parse_primary(struct expr_context *ctx, bool > *atomic) > c.values = cst; > c.n_values = 1; > c.in_curlies = false; > + c.as_name = NULL; > return make_cmp(ctx, &f, EXPR_R_NE, &c); > } else if (parse_relop(ctx, &r) && parse_constant_set(ctx, &c)) { > return make_cmp(ctx, &f, r, &c); > @@ -1709,6 +1734,7 @@ expr_symtab_destroy(struct shash *symtab) > static struct expr * > expr_clone_cmp(struct expr *expr) > { > + ovs_assert(!expr->as_name); > struct expr *new = xmemdup(expr, sizeof *expr); > if (!new->cmp.symbol->width) { > new->cmp.string = xstrdup(new->cmp.string); > @@ -1732,6 +1758,7 @@ expr_clone_andor(struct expr *expr) > static struct expr * > expr_clone_condition(struct expr *expr) > { > + ovs_assert(!expr->as_name); > struct expr *new = xmemdup(expr, sizeof *expr); > new->cond.string = xstrdup(new->cond.string); > return new; > @@ -1767,6 +1794,8 @@ expr_destroy(struct expr *expr) > return; > } > > + free(expr->as_name); > + > struct expr *sub, *next; > > switch (expr->type) { > @@ -2373,7 +2402,7 @@ crush_and_string(struct expr *expr, const struct > expr_symbol *symbol) > > const char *string; > SSET_FOR_EACH (string, &result) { > - sub = xmalloc(sizeof *sub); > + sub = xzalloc(sizeof *sub); > sub->type = EXPR_T_CMP; > sub->cmp.relop = EXPR_R_EQ; > sub->cmp.symbol = symbol; > @@ -2432,7 +2461,7 @@ crush_and_numeric(struct expr *expr, const struct > expr_symbol *symbol) > return expr_create_boolean(true); > } else { > struct expr *cmp; > - cmp = xmalloc(sizeof *cmp); > + cmp = xzalloc(sizeof *cmp); > cmp->type = EXPR_T_CMP; > cmp->cmp.symbol = symbol; > cmp->cmp.relop = EXPR_R_EQ; > @@ -2447,7 +2476,7 @@ crush_and_numeric(struct expr *expr, const struct > expr_symbol *symbol) > struct expr *disjuncts = > expr_from_node(ovs_list_pop_front(&expr->andor)); > struct expr *or; > > - or = xmalloc(sizeof *or); > + or = xzalloc(sizeof *or); > or->type = EXPR_T_OR; > ovs_list_init(&or->andor); > > @@ -2483,7 +2512,7 @@ crush_and_numeric(struct expr *expr, const struct > expr_symbol *symbol) > struct expr *new = NULL; > struct expr *or; > > - or = xmalloc(sizeof *or); > + or = xzalloc(sizeof *or); > or->type = EXPR_T_OR; > ovs_list_init(&or->andor); > > @@ -2502,7 +2531,7 @@ crush_and_numeric(struct expr *expr, const struct > expr_symbol *symbol) > LIST_FOR_EACH (b, node, &bs->andor) { > ovs_assert(b->type == EXPR_T_CMP); > if (!new) { > - new = xmalloc(sizeof *new); > + new = xzalloc(sizeof *new); > new->type = EXPR_T_CMP; > new->cmp.symbol = symbol; > new->cmp.relop = EXPR_R_EQ; > @@ -2608,6 +2637,9 @@ crush_or(struct expr *expr, const struct expr_symbol > *symbol) > ovs_list_push_back(&expr->andor, &b->node); > } else { > expr_destroy(b); > + /* Member modified, so untrack address set. */ > + free(expr->as_name); > + expr->as_name = NULL; > } > } > free(subs); > @@ -2659,7 +2691,7 @@ expr_sort(struct expr *expr) > ovs_assert(expr->type == EXPR_T_AND); > > size_t n = ovs_list_size(&expr->andor); > - struct expr_sort *subs = xmalloc(n * sizeof *subs); > + struct expr_sort *subs = xzalloc(n * sizeof *subs); > struct expr *sub; > size_t i; > > @@ -2884,7 +2916,7 @@ static struct expr_match * > expr_match_new(const struct match *m, uint8_t clause, uint8_t n_clauses, > uint32_t conj_id) > { > - struct expr_match *match = xmalloc(sizeof *match); > + struct expr_match *match = xzalloc(sizeof *match); > if (m) { > match->match = *m; > } else { > @@ -2905,6 +2937,14 @@ expr_match_new(const struct match *m, uint8_t clause, > uint8_t n_clauses, > return match; > } > > +static void > +expr_match_destroy(struct expr_match *match) > +{ > + free(match->as_name); > + free(match->conjunctions); > + free(match); > +} > + > /* Adds 'match' to hash table 'matches', which becomes the new owner of > * 'match'. > * > @@ -2932,8 +2972,12 @@ expr_match_add(struct hmap *matches, struct expr_match > *match) > } > m->conjunctions[m->n++] = match->conjunctions[0]; > } > - free(match->conjunctions); > - free(match); > + if (m->as_name) { > + /* m is combined with match. so untracked the address set. */ > + free(m->as_name); > + m->as_name = NULL; > + } > + expr_match_destroy(match); > return; > } > } > @@ -2994,12 +3038,18 @@ add_disjunction(const struct expr *or, > LIST_FOR_EACH (sub, node, &or->andor) { > struct expr_match *match = expr_match_new(m, clause, n_clauses, > conj_id); > + if (or->as_name) { > + ovs_assert(sub->type == EXPR_T_CMP); > + ovs_assert(sub->cmp.symbol->width); > + match->as_name = xstrdup(or->as_name); > + match->as_ip = sub->cmp.value.ipv6; > + match->as_mask = sub->cmp.mask.ipv6; > + } > if (constrain_match(sub, lookup_port, aux, &match->match)) { > expr_match_add(matches, match); > n++; > } else { > - free(match->conjunctions); > - free(match); > + expr_match_destroy(match); > } > } > > @@ -3082,7 +3132,7 @@ add_cmp_flow(const struct expr *cmp, > if (constrain_match(cmp, lookup_port, aux, &m->match)) { > expr_match_add(matches, m); > } else { > - free(m); > + expr_match_destroy(m); > } > } > > @@ -3214,8 +3264,7 @@ expr_matches_destroy(struct hmap *matches) > } > > HMAP_FOR_EACH_POP (m, hmap_node, matches) { > - free(m->conjunctions); > - free(m); > + expr_match_destroy(m); > } > hmap_destroy(matches); > } > -- > 2.30.2 > > _______________________________________________ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev