Skip to content

Commit

Permalink
ovn-controller: Track individual IP information of address set during…
Browse files Browse the repository at this point in the history 
… lflow parsing.

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 <hzhou@ovn.org>
Acked-by: Mark Michelson <mmichels@redhat.com>
Acked-by: Numan Siddique <numans@ovn.org>
  • Loading branch information
@hzhou8
hzhou8 committed Feb 24, 2022
1 parent 72ad727 commit eeb51fa
Show file tree
Hide file tree
Showing 6 changed files with 233 additions and 37 deletions.
19 changes: 15 additions & 4 deletions controller/lflow.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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);
}
}
Expand Down Expand Up @@ -1529,7 +1540,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);
Expand Down Expand Up @@ -1703,7 +1714,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);

}
Expand Down
140 changes: 128 additions & 12 deletions controller/ofctrl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 sb_list; /* Node in desired_flow.references. */
struct ovs_list flow_list; /* Node in sb_to_flow.flows. */
struct ovs_list as_ip_flow_list; /* Node in as_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 as_ip_to_flow_map; /* map from IPs in the address set to flows.
Each node is as_ip_to_flow_node. */
};

struct as_ip_to_flow_node {
struct hmap_node hmap_node; /* Node in sb_addrset_ref.as_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
Expand Down Expand Up @@ -1030,9 +1050,27 @@ sb_to_flow_find(struct hmap *uuid_flow_table, const struct uuid *sb_uuid)
return NULL;
}

static struct as_ip_to_flow_node *
as_ip_to_flow_find(struct hmap *as_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 as_ip_to_flow_node *itfn;
HMAP_FOR_EACH_WITH_HASH (itfn, hmap_node, hash, as_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);
Expand All @@ -1046,10 +1084,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->as_ip_to_flow_map);
ovs_list_insert(&stf->addrsets, &sar->list_node);
}

struct as_ip_to_flow_node * itfn =
as_ip_to_flow_find(&sar->as_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->as_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. */
Expand All @@ -1068,13 +1144,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) {
Expand All @@ -1091,7 +1171,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");
}
Expand All @@ -1103,19 +1183,20 @@ 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
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
Expand All @@ -1127,7 +1208,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;
Expand Down Expand Up @@ -1156,11 +1238,26 @@ ofctrl_add_or_append_flow(struct ovn_desired_flow_table *desired_flows,
ofpbuf_uninit(&compound);
desired_flow_destroy(f);
f = existing;

/* Since the flow now shared by more than one SB lflows, don't track
* it with address set ips. So remove any existed as_info tracking, and
* then add the new sb link without as_info.
*
* XXX: this may still be tracked if the flow is shared by different
* lflows, but we need to remove the related conjunction from the
* actions properly when handle addrset ip deletion, instead of simply
* delete the 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_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) {
Expand Down Expand Up @@ -1269,6 +1366,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);
Expand All @@ -1286,6 +1384,23 @@ 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 as_ip_to_flow_node *itfn, *itfn_next;
HMAP_FOR_EACH_SAFE (itfn, itfn_next, hmap_node,
&sar->as_ip_to_flow_map) {
hmap_remove(&sar->as_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->as_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);
Expand All @@ -1300,6 +1415,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) {
Expand Down
19 changes: 15 additions & 4 deletions controller/ofctrl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down Expand Up @@ -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);


Expand Down
2 changes: 1 addition & 1 deletion controller/physical.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down
9 changes: 9 additions & 0 deletions include/ovn/expr.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 *,
Expand Down Expand Up @@ -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 *);
Expand Down

0 comments on commit eeb51fa

Please sign in to comment.