Skip to content

Commit

Permalink
ovn-controller: Only process lflows for local datapaths.
Browse files Browse the repository at this point in the history
Previously, ovn-controller translated logical flows into OpenFlow flows
for *every* logical datapath.  This patch makes it so we skip doing so
for the egress pipeline if the datapath is a logical switch with no
logical ports bound locally.  In that case, the flows have no effect.

This was the code path taking the most time in a large scale OVN
environment and was an easy optimization to make based on the existing
local_datapaths info.

In this environment, while idling, ovn-controller was taking up about
20% CPU with this patch, while other nodes were in the 40-70% range.

Reported-at: https://bugs.launchpad.net/networking-ovn/+bug/1536003
Signed-off-by: Russell Bryant <[email protected]>
Tested-by: Matt Mulsow <[email protected]>
Acked-by: Ben Pfaff <[email protected]>
Acked-By: Kyle Mestery <[email protected]>
  • Loading branch information
russellb committed Feb 3, 2016
1 parent 993225b commit b1e0451
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 4 deletions.
34 changes: 32 additions & 2 deletions ovn/controller/lflow.c
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,11 @@ symtab_init(void)

/* Logical datapaths and logical port numbers. */

enum ldp_type {
LDP_TYPE_ROUTER,
LDP_TYPE_SWITCH,
};

/* A logical datapath.
*
* 'ports' maps 'logical_port' names to 'tunnel_key' values in the OVN_SB
Expand All @@ -166,6 +171,7 @@ struct logical_datapath {
struct uuid uuid; /* UUID from Datapath_Binding row. */
uint32_t tunnel_key; /* 'tunnel_key' from Datapath_Binding row. */
struct simap ports; /* Logical port name to port number. */
enum ldp_type type; /* Type of logical datapath */
};

/* Contains "struct logical_datapath"s. */
Expand Down Expand Up @@ -197,6 +203,8 @@ ldp_create(const struct sbrec_datapath_binding *binding)
uuid_hash(&binding->header_.uuid));
ldp->uuid = binding->header_.uuid;
ldp->tunnel_key = binding->tunnel_key;
const char *ls = smap_get(&binding->external_ids, "logical-switch");
ldp->type = ls ? LDP_TYPE_SWITCH : LDP_TYPE_ROUTER;
simap_init(&ldp->ports);
return ldp;
}
Expand Down Expand Up @@ -267,7 +275,8 @@ lflow_init(void)
* into OpenFlow flows. See ovn-architecture(7) for more information. */
void
lflow_run(struct controller_ctx *ctx, struct hmap *flow_table,
const struct simap *ct_zones)
const struct simap *ct_zones,
struct hmap *local_datapaths)
{
struct hmap flows = HMAP_INITIALIZER(&flows);
uint32_t conj_id_ofs = 1;
Expand All @@ -286,8 +295,29 @@ lflow_run(struct controller_ctx *ctx, struct hmap *flow_table,
continue;
}

/* Determine translation of logical table IDs to physical table IDs. */
bool ingress = !strcmp(lflow->pipeline, "ingress");

if (ldp->type == LDP_TYPE_SWITCH && !ingress) {
/* For a logical switch datapath, local_datapaths tells us if there
* are any local ports for this datapath. If not, processing
* logical flows for the egress pipeline of this datapath is
* unnecessary.
*
* We still need the ingress pipeline because even if there are no
* local ports, we still may need to execute the ingress pipeline
* after a packet leaves a logical router. Further optimization
* is possible, but not based on what we know with local_datapaths
* right now.
*/

struct hmap_node *ld;
ld = hmap_first_with_hash(local_datapaths, ldp->tunnel_key);
if (!ld) {
continue;
}
}

/* Determine translation of logical table IDs to physical table IDs. */
uint8_t first_ptable = (ingress
? OFTABLE_LOG_INGRESS_PIPELINE
: OFTABLE_LOG_EGRESS_PIPELINE);
Expand Down
3 changes: 2 additions & 1 deletion ovn/controller/lflow.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@ struct uuid;

void lflow_init(void);
void lflow_run(struct controller_ctx *, struct hmap *flow_table,
const struct simap *ct_zones);
const struct simap *ct_zones,
struct hmap *local_datapaths);
void lflow_destroy(void);

#endif /* ovn/lflow.h */
2 changes: 1 addition & 1 deletion ovn/controller/ovn-controller.c
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ main(int argc, char *argv[])
pinctrl_run(&ctx, br_int);

struct hmap flow_table = HMAP_INITIALIZER(&flow_table);
lflow_run(&ctx, &flow_table, &ct_zones);
lflow_run(&ctx, &flow_table, &ct_zones, &local_datapaths);
if (chassis_id) {
physical_run(&ctx, mff_ovn_geneve,
br_int, chassis_id, &ct_zones, &flow_table);
Expand Down

0 comments on commit b1e0451

Please sign in to comment.