Skip to content

Commit

Permalink
controller: Use precomputed is_switch instead of querying external IDs.
Browse files Browse the repository at this point in the history 
We already store whether a datapath is a switch or not.  No need to do
an smap lookup through its external IDs every time we need this
information.

Also, move the functions that inspect Datapath_Binding.external_ids
out of ovn-util.c.  It makes more sense to just define them where
they're used.

Signed-off-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Numan Siddique <numans@ovn.org>
  • Loading branch information
@dceara @numansiddique
dceara authored and numansiddique committed Feb 23, 2022
1 parent d7514ab commit 979fecc
Show file tree
Hide file tree
Showing 6 changed files with 40 additions and 37 deletions.
29 changes: 16 additions & 13 deletions controller/lflow.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -622,7 +622,7 @@ lflow_parse_ctrl_meter(const struct sbrec_logical_flow *lflow,

static void
add_matches_to_flow_table(const struct sbrec_logical_flow *lflow,
const struct sbrec_datapath_binding *dp,
const struct local_datapath *ldp,
struct hmap *matches, uint8_t ptable,
uint8_t output_ptable, struct ofpbuf *ovnacts,
bool ingress, struct lflow_ctx_in *l_ctx_in,
Expand All @@ -632,7 +632,7 @@ add_matches_to_flow_table(const struct sbrec_logical_flow *lflow,
.sbrec_multicast_group_by_name_datapath
= l_ctx_in->sbrec_multicast_group_by_name_datapath,
.sbrec_port_binding_by_name = l_ctx_in->sbrec_port_binding_by_name,
.dp = dp,
.dp = ldp->datapath,
.lflow = lflow,
.lfrr = l_ctx_out->lfrr,
.chassis_tunnels = l_ctx_in->chassis_tunnels,
Expand All @@ -652,7 +652,7 @@ add_matches_to_flow_table(const struct sbrec_logical_flow *lflow,
.lookup_port = lookup_port_cb,
.tunnel_ofport = tunnel_ofport_cb,
.aux = &aux,
.is_switch = datapath_is_switch(dp),
.is_switch = ldp->is_switch,
.group_table = l_ctx_out->group_table,
.meter_table = l_ctx_out->meter_table,
.lflow_uuid = lflow->header_.uuid,
Expand All @@ -674,13 +674,13 @@ add_matches_to_flow_table(const struct sbrec_logical_flow *lflow,

struct expr_match *m;
HMAP_FOR_EACH (m, hmap_node, matches) {
match_set_metadata(&m->match, htonll(dp->tunnel_key));
if (datapath_is_switch(dp)) {
match_set_metadata(&m->match, htonll(ldp->datapath->tunnel_key));
if (ldp->is_switch) {
unsigned int reg_index
= (ingress ? MFF_LOG_INPORT : MFF_LOG_OUTPORT) - MFF_REG0;
int64_t port_id = m->match.flow.regs[reg_index];
if (port_id) {
int64_t dp_id = dp->tunnel_key;
int64_t dp_id = ldp->datapath->tunnel_key;
char buf[16];
get_unique_lport_key(dp_id, port_id, buf, sizeof(buf));
if (!sset_contains(l_ctx_in->related_lport_ids, buf)) {
Expand Down Expand Up @@ -731,7 +731,7 @@ add_matches_to_flow_table(const struct sbrec_logical_flow *lflow,
*/
static struct expr *
convert_match_to_expr(const struct sbrec_logical_flow *lflow,
const struct sbrec_datapath_binding *dp,
const struct local_datapath *ldp,
struct expr **prereqs,
const struct shash *addr_sets,
const struct shash *port_groups,
Expand All @@ -744,7 +744,8 @@ convert_match_to_expr(const struct sbrec_logical_flow *lflow,

struct expr *e = expr_parse_string(lflow->match, &symtab, addr_sets,
port_groups, &addr_sets_ref,
&port_groups_ref, dp->tunnel_key,
&port_groups_ref,
ldp->datapath->tunnel_key,
&error);
const char *addr_set_name;
SSET_FOR_EACH (addr_set_name, &addr_sets_ref) {
Expand Down Expand Up @@ -791,9 +792,11 @@ consider_logical_flow__(const struct sbrec_logical_flow *lflow,
struct lflow_ctx_in *l_ctx_in,
struct lflow_ctx_out *l_ctx_out)
{
if (!get_local_datapath(l_ctx_in->local_datapaths, dp->tunnel_key)) {
VLOG_DBG("Skip lflow "UUID_FMT" for non-local datapath %"PRId64,
UUID_ARGS(&lflow->header_.uuid), dp->tunnel_key);
struct local_datapath *ldp = get_local_datapath(l_ctx_in->local_datapaths,
dp->tunnel_key);
if (!ldp) {
VLOG_DBG("lflow "UUID_FMT" is not for local datapath, skip",
UUID_ARGS(&lflow->header_.uuid));
return;
}

Expand Down Expand Up @@ -905,7 +908,7 @@ consider_logical_flow__(const struct sbrec_logical_flow *lflow,
/* Get match expr, either from cache or from lflow match. */
switch (lcv_type) {
case LCACHE_T_NONE:
expr = convert_match_to_expr(lflow, dp, &prereqs, l_ctx_in->addr_sets,
expr = convert_match_to_expr(lflow, ldp, &prereqs, l_ctx_in->addr_sets,
l_ctx_in->port_groups, l_ctx_out->lfrr,
&pg_addr_set_ref);
if (!expr) {
Expand Down Expand Up @@ -970,7 +973,7 @@ consider_logical_flow__(const struct sbrec_logical_flow *lflow,
break;
}

add_matches_to_flow_table(lflow, dp, matches, ptable, output_ptable,
add_matches_to_flow_table(lflow, ldp, matches, ptable, output_ptable,
&ovnacts, ingress, l_ctx_in, l_ctx_out);

/* Update cache if needed. */
Expand Down
15 changes: 15 additions & 0 deletions controller/local_data.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ static struct tracked_datapath *tracked_datapath_create(
enum en_tracked_resource_type tracked_type,
struct hmap *tracked_datapaths);

static bool datapath_is_switch(const struct sbrec_datapath_binding *);
static bool datapath_is_transit_switch(const struct sbrec_datapath_binding *);

static uint64_t local_datapath_usage;

struct local_datapath *
Expand Down Expand Up @@ -605,3 +608,15 @@ local_datapath_peer_port_add(struct local_datapath *ld,
ld->peer_ports[ld->n_peer_ports - 1].local = local;
ld->peer_ports[ld->n_peer_ports - 1].remote = remote;
}

static bool
datapath_is_switch(const struct sbrec_datapath_binding *ldp)
{
return smap_get(&ldp->external_ids, "logical-switch") != NULL;
}

static bool
datapath_is_transit_switch(const struct sbrec_datapath_binding *ldp)
{
return smap_get(&ldp->external_ids, "interconn-ts") != NULL;
}
10 changes: 8 additions & 2 deletions controller/ovn-controller.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -634,6 +634,12 @@ alloc_id_to_ct_zone(const char *zone_name, struct simap *ct_zones,
return true;
}

static int
get_snat_ct_zone(const struct sbrec_datapath_binding *dp)
{
return smap_get_int(&dp->external_ids, "snat-ct-zone", -1);
}

static void
update_ct_zones(const struct shash *binding_lports,
const struct hmap *local_datapaths,
Expand Down Expand Up @@ -665,7 +671,7 @@ update_ct_zones(const struct shash *binding_lports,
shash_add(&all_lds, dnat, ld);
shash_add(&all_lds, snat, ld);

int req_snat_zone = datapath_snat_ct_zone(ld->datapath);
int req_snat_zone = get_snat_ct_zone(ld->datapath);
if (req_snat_zone >= 0) {
simap_put(&req_snat_zones, snat, req_snat_zone);
}
Expand Down Expand Up @@ -1851,7 +1857,7 @@ ct_zones_datapath_binding_handler(struct engine_node *node, void *data)
return false;
}

int req_snat_zone = datapath_snat_ct_zone(dp);
int req_snat_zone = get_snat_ct_zone(dp);
if (req_snat_zone == -1) {
/* datapath snat ct zone is not set. This condition will also hit
* when CMS clears the snat-ct-zone for the logical router.
Expand Down
2 changes: 1 addition & 1 deletion controller/pinctrl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4268,7 +4268,7 @@ run_buffered_binding(struct ovsdb_idl_index *sbrec_mac_binding_by_lport_ip,
* a router datapath. Hence we can skip logical switch
* datapaths.
* */
if (datapath_is_switch(ld->datapath)) {
if (ld->is_switch) {
continue;
}

Expand Down
18 changes: 0 additions & 18 deletions lib/ovn-util.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -559,24 +559,6 @@ ovn_logical_flow_hash_datapath(const struct uuid *logical_datapath,
return hash_add(hash, uuid_hash(logical_datapath));
}

bool
datapath_is_switch(const struct sbrec_datapath_binding *ldp)
{
return smap_get(&ldp->external_ids, "logical-switch") != NULL;
}

bool
datapath_is_transit_switch(const struct sbrec_datapath_binding *ldp)
{
return smap_get(&ldp->external_ids, "interconn-ts") != NULL;
}

int
datapath_snat_ct_zone(const struct sbrec_datapath_binding *dp)
{
return smap_get_int(&dp->external_ids, "snat-ct-zone", -1);
}


struct tnlid_node {
struct hmap_node hmap_node;
Expand Down
3 changes: 0 additions & 3 deletions lib/ovn-util.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,6 @@ uint32_t ovn_logical_flow_hash(uint8_t table_id, enum ovn_pipeline pipeline,
const char *match, const char *actions);
uint32_t ovn_logical_flow_hash_datapath(const struct uuid *logical_datapath,
uint32_t hash);
bool datapath_is_switch(const struct sbrec_datapath_binding *);
bool datapath_is_transit_switch(const struct sbrec_datapath_binding *ldp);
int datapath_snat_ct_zone(const struct sbrec_datapath_binding *ldp);
void ovn_conn_show(struct unixctl_conn *conn, int argc OVS_UNUSED,
const char *argv[] OVS_UNUSED, void *idl_);

Expand Down

0 comments on commit 979fecc

Please sign in to comment.