On Wed, Feb 9, 2022 at 12:55 PM Han Zhou <[email protected]> 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 <[email protected]>
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
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
