ofproto_port_open_type() was surprisingly slow because it called the
function ofproto_class_find__(), which itself was surprisingly slow because
it actually creates a set of strings and enumerates all of the available
classes.
This patch improves performance by eliminating the call to
ofproto_class_find__() from ofproto_port_open_type(). In turn that
required changing a parameter type and updating all the callers.
Possibly it would be worth making ofproto_class_find__() itself faster,
but it doesn't look like any of its other callers would be used in inner
loops.
A different patch that was also meant to speed this up reported the
following performance improvements. That patch just eliminated half of the
calls to ofproto_class_find__() in inner loops, whereas this one eliminates
all of them and should therefore perform even better.
This patch arises as a result of testing done by Ali Ginwala and Han
Zhou. Their test showed that commit 2d4beba resulted in slower
performance of ovs-vswitchd than was seen in previous versions of OVS.
With this patch, Ali retested and reported that this patch had nearly
the same effect on performance as reverting 2d4beba.
The test was to create 10000 logical switch ports using 20 farm nodes,
each running 50 HV sandboxes. "Base" in the tests below is OVS master
with Han Zhou's ovn-controller incremental processing patch applied.
base: Test completed in 8 hours
base with 2d4beba reverted: Test completed in 5 hours 28 minutes
base with this patch: Test completed in 5 hours 30 minutes
Reported-by: Mark Michelson <mmich...@redhat.com>
Signed-off-by: Ben Pfaff <b...@ovn.org>
---
ofproto/in-band.c | 2 +-
ofproto/ofproto.c | 30 ++++++++++--------------------
ofproto/ofproto.h | 2 +-
vswitchd/bridge.c | 12 ++++--------
4 files changed, 16 insertions(+), 30 deletions(-)
diff --git a/ofproto/in-band.c b/ofproto/in-band.c
index 849b1cedaff0..82d8dfa14774 100644
--- a/ofproto/in-band.c
+++ b/ofproto/in-band.c
@@ -428,7 +428,7 @@ in_band_create(struct ofproto *ofproto, const char
*local_name,
struct in_band *in_band;
struct netdev *local_netdev;
int error;
- const char *type = ofproto_port_open_type(ofproto->type, "internal");
+ const char *type = ofproto_port_open_type(ofproto, "internal");
*in_bandp = NULL;
error = netdev_open(local_name, type, &local_netdev);
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index f28bb896eee9..a982de9d8db4 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -1966,27 +1966,18 @@ ofproto_port_dump_done(struct ofproto_port_dump *dump)
return dump->error == EOF ? 0 : dump->error;
}
-/* Returns the type to pass to netdev_open() when a datapath of type
- * 'datapath_type' has a port of type 'port_type', for a few special
- * cases when a netdev type differs from a port type. For example, when
- * using the userspace datapath, a port of type "internal" needs to be
- * opened as "tap".
+/* Returns the type to pass to netdev_open() when 'ofproto' has a port of type
+ * 'port_type', for a few special cases when a netdev type differs from a port
+ * type. For example, when using the userspace datapath, a port of type
+ * "internal" needs to be opened as "tap".
*
* Returns either 'type' itself or a string literal, which must not be
* freed. */
const char *
-ofproto_port_open_type(const char *datapath_type, const char *port_type)
+ofproto_port_open_type(const struct ofproto *ofproto, const char *port_type)
{
- const struct ofproto_class *class;
-
- datapath_type = ofproto_normalize_type(datapath_type);
- class = ofproto_class_find__(datapath_type);
- if (!class) {
- return port_type;
- }
-
- return (class->port_open_type
- ? class->port_open_type(datapath_type, port_type)
+ return (ofproto->ofproto_class->port_open_type
+ ? ofproto->ofproto_class->port_open_type(ofproto->type, port_type)
: port_type);
}
@@ -2728,10 +2719,9 @@ init_ports(struct ofproto *p)
static bool
ofport_is_internal_or_patch(const struct ofproto *p, const struct ofport
*port)
{
- return !strcmp(netdev_get_type(port->netdev),
- ofproto_port_open_type(p->type, "internal")) ||
- !strcmp(netdev_get_type(port->netdev),
- ofproto_port_open_type(p->type, "patch"));
+ const char *netdev_type = netdev_get_type(port->netdev);
+ return !strcmp(netdev_type, ofproto_port_open_type(p, "internal")) ||
+ !strcmp(netdev_type, ofproto_port_open_type(p, "patch"));
}
/* If 'port' is internal or patch and if the user didn't explicitly specify an
diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
index 1e48e1952aa2..8c85bbf7fb26 100644
--- a/ofproto/ofproto.h
+++ b/ofproto/ofproto.h
@@ -290,7 +290,7 @@ int ofproto_port_dump_done(struct ofproto_port_dump *);
#define OFPROTO_FLOW_LIMIT_DEFAULT 200000
#define OFPROTO_MAX_IDLE_DEFAULT 10000 /* ms */
-const char *ofproto_port_open_type(const char *datapath_type,
+const char *ofproto_port_open_type(const struct ofproto *,
const char *port_type);
int ofproto_port_add(struct ofproto *, struct netdev *, ofp_port_t
*ofp_portp);
int ofproto_port_del(struct ofproto *, ofp_port_t ofp_port);
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index cf9c79fd77cf..f03fb567f0be 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -88,7 +88,6 @@ struct iface {
/* These members are valid only within bridge_reconfigure(). */
const char *type; /* Usually same as cfg->type. */
- const char *netdev_type; /* type that should be used for netdev_open. */
const struct ovsrec_interface *cfg;
};
@@ -822,7 +821,9 @@ bridge_delete_or_reconfigure_ports(struct bridge *br)
goto delete;
}
- if (strcmp(ofproto_port.type, iface->netdev_type)
+ const char *netdev_type = ofproto_port_open_type(br->ofproto,
+ iface->type);
+ if (strcmp(ofproto_port.type, netdev_type)
|| netdev_set_config(iface->netdev, &iface->cfg->options, NULL)) {
/* The interface is the wrong type or can't be configured.
* Delete it. */
@@ -1778,7 +1779,7 @@ iface_do_create(const struct bridge *br,
goto error;
}
- type = ofproto_port_open_type(br->cfg->datapath_type,
+ type = ofproto_port_open_type(br->ofproto,
iface_get_type(iface_cfg, br->cfg));
error = netdev_open(iface_cfg->name, type, &netdev);
if (error) {
@@ -1856,8 +1857,6 @@ iface_create(struct bridge *br, const struct
ovsrec_interface *iface_cfg,
iface->ofp_port = ofp_port;
iface->netdev = netdev;
iface->type = iface_get_type(iface_cfg, br->cfg);
- iface->netdev_type = ofproto_port_open_type(br->cfg->datapath_type,
- iface->type);
iface->cfg = iface_cfg;
hmap_insert(&br->ifaces, &iface->ofp_port_node,
hash_ofp_port(ofp_port));
@@ -3445,13 +3444,10 @@ bridge_del_ports(struct bridge *br, const struct shash
*wanted_ports)
const struct ovsrec_interface *cfg = port_rec->interfaces[i];
struct iface *iface = iface_lookup(br, cfg->name);
const char *type = iface_get_type(cfg, br->cfg);
- const char *dp_type = br->cfg->datapath_type;
- const char *netdev_type = ofproto_port_open_type(dp_type, type);
if (iface) {
iface->cfg = cfg;
iface->type = type;
- iface->netdev_type = netdev_type;
} else if (!strcmp(type, "null")) {
VLOG_WARN_ONCE("%s: The null interface type is deprecated and"
" may be removed in February 2013. Please
email"
