[ovs-dev] [PATCH 3/3] netdev-vport: Don't return static data in netdev_vport_get_dpif_port().
Ben Pfaff
blp at nicira.com
Wed May 1 18:08:06 UTC 2013
Returning a static data buffer makes code more brittle and definitely
not thread-safe, so this commit switches to using a caller-provided
buffer instead.
Signed-off-by: Ben Pfaff <blp at nicira.com>
---
lib/dpif-linux.c | 4 +++-
lib/dpif-netdev.c | 9 ++++++---
lib/netdev-vport.c | 18 ++++++++++++++----
lib/netdev-vport.h | 8 ++++++--
ofproto/ofproto-dpif.c | 17 +++++++++++++----
5 files changed, 42 insertions(+), 14 deletions(-)
diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c
index b863a2e..563d77f 100644
--- a/lib/dpif-linux.c
+++ b/lib/dpif-linux.c
@@ -483,7 +483,9 @@ dpif_linux_port_add(struct dpif *dpif_, struct netdev *netdev,
{
struct dpif_linux *dpif = dpif_linux_cast(dpif_);
const struct netdev_tunnel_config *tnl_cfg;
- const char *name = netdev_vport_get_dpif_port(netdev);
+ char namebuf[NETDEV_VPORT_NAME_BUFSIZE];
+ const char *name = netdev_vport_get_dpif_port(netdev,
+ namebuf, sizeof namebuf);
const char *type = netdev_get_type(netdev);
struct dpif_linux_vport request, reply;
struct nl_sock *sock = NULL;
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 5f57c45..1154518 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -428,8 +428,11 @@ dpif_netdev_port_add(struct dpif *dpif, struct netdev *netdev,
uint32_t *port_nop)
{
struct dp_netdev *dp = get_dp_netdev(dpif);
+ char namebuf[NETDEV_VPORT_NAME_BUFSIZE];
+ const char *dpif_port;
int port_no;
+ dpif_port = netdev_vport_get_dpif_port(netdev, namebuf, sizeof namebuf);
if (*port_nop != UINT32_MAX) {
if (*port_nop >= MAX_PORTS) {
return EFBIG;
@@ -438,12 +441,11 @@ dpif_netdev_port_add(struct dpif *dpif, struct netdev *netdev,
}
port_no = *port_nop;
} else {
- port_no = choose_port(dp, netdev_vport_get_dpif_port(netdev));
+ port_no = choose_port(dp, dpif_port);
}
if (port_no >= 0) {
*port_nop = port_no;
- return do_add_port(dp, netdev_vport_get_dpif_port(netdev),
- netdev_get_type(netdev), port_no);
+ return do_add_port(dp, dpif_port, netdev_get_type(netdev), port_no);
}
return EFBIG;
}
@@ -1064,6 +1066,7 @@ dpif_netdev_run(struct dpif *dpif)
dp_netdev_port_input(dp, port, &packet);
} else if (error != EAGAIN && error != EOPNOTSUPP) {
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+
VLOG_ERR_RL(&rl, "error receiving data from %s: %s",
netdev_get_name(port->netdev), strerror(error));
}
diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
index a9911d3..41f36c5 100644
--- a/lib/netdev-vport.c
+++ b/lib/netdev-vport.c
@@ -122,7 +122,8 @@ netdev_vport_needs_dst_port(const struct netdev_dev *dev)
}
const char *
-netdev_vport_get_dpif_port(const struct netdev *netdev)
+netdev_vport_get_dpif_port(const struct netdev *netdev,
+ char namebuf[], size_t bufsize)
{
const struct netdev_dev *dev = netdev_get_dev(netdev);
const struct netdev_class *class = netdev_dev_get_class(dev);
@@ -130,7 +131,6 @@ netdev_vport_get_dpif_port(const struct netdev *netdev)
if (netdev_vport_needs_dst_port(dev)) {
const struct netdev_dev_vport *vport = netdev_vport_get_dev(netdev);
const char *type = netdev_dev_get_type(dev);
- static char dpif_port_combined[IFNAMSIZ];
/*
* Note: IFNAMSIZ is 16 bytes long. The maximum length of a VXLAN
@@ -138,10 +138,11 @@ netdev_vport_get_dpif_port(const struct netdev *netdev)
* assert here on the size of strlen(type) in case that changes
* in the future.
*/
+ BUILD_ASSERT(NETDEV_VPORT_NAME_BUFSIZE >= IFNAMSIZ);
ovs_assert(strlen(type) + 10 < IFNAMSIZ);
- snprintf(dpif_port_combined, IFNAMSIZ, "%s_sys_%d", type,
+ snprintf(namebuf, bufsize, "%s_sys_%d", type,
ntohs(vport->tnl_cfg.dst_port));
- return dpif_port_combined;
+ return namebuf;
}
if (is_vport_class(class)) {
@@ -154,6 +155,15 @@ netdev_vport_get_dpif_port(const struct netdev *netdev)
return netdev_get_name(netdev);
}
+char *
+netdev_vport_get_dpif_port_strdup(const struct netdev *netdev)
+{
+ char namebuf[NETDEV_VPORT_NAME_BUFSIZE];
+
+ return xstrdup(netdev_vport_get_dpif_port(netdev, namebuf,
+ sizeof namebuf));
+}
+
static int
netdev_vport_create(const struct netdev_class *netdev_class, const char *name,
struct netdev_dev **netdev_devp)
diff --git a/lib/netdev-vport.h b/lib/netdev-vport.h
index c907b0c..5165efd 100644
--- a/lib/netdev-vport.h
+++ b/lib/netdev-vport.h
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2010, 2011 Nicira, Inc.
+ * Copyright (c) 2010, 2011, 2013 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -18,6 +18,7 @@
#define NETDEV_VPORT_H 1
#include <stdbool.h>
+#include <stddef.h>
struct dpif_linux_vport;
struct dpif_flow_stats;
@@ -36,6 +37,9 @@ void netdev_vport_inc_rx(const struct netdev *,
void netdev_vport_inc_tx(const struct netdev *,
const struct dpif_flow_stats *);
-const char *netdev_vport_get_dpif_port(const struct netdev *);
+enum { NETDEV_VPORT_NAME_BUFSIZE = 16 };
+const char *netdev_vport_get_dpif_port(const struct netdev *,
+ char namebuf[], size_t bufsize);
+char *netdev_vport_get_dpif_port_strdup(const struct netdev *);
#endif /* netdev-vport.h */
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 6ec1c23..16f4b1f 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -959,13 +959,15 @@ type_run(const char *type)
}
HMAP_FOR_EACH (iter, up.hmap_node, &ofproto->up.ports) {
+ char namebuf[NETDEV_VPORT_NAME_BUFSIZE];
const char *dp_port;
if (!iter->tnl_port) {
continue;
}
- dp_port = netdev_vport_get_dpif_port(iter->up.netdev);
+ dp_port = netdev_vport_get_dpif_port(iter->up.netdev,
+ namebuf, sizeof namebuf);
node = simap_find(&tmp_backers, dp_port);
if (node) {
simap_put(&backer->tnl_backers, dp_port, node->data);
@@ -1772,6 +1774,7 @@ port_construct(struct ofport *port_)
struct ofport_dpif *port = ofport_dpif_cast(port_);
struct ofproto_dpif *ofproto = ofproto_dpif_cast(port->up.ofproto);
const struct netdev *netdev = port->up.netdev;
+ char namebuf[NETDEV_VPORT_NAME_BUFSIZE];
struct dpif_port dpif_port;
int error;
@@ -1795,7 +1798,8 @@ port_construct(struct ofport *port_)
}
error = dpif_port_query_by_name(ofproto->backer->dpif,
- netdev_vport_get_dpif_port(netdev),
+ netdev_vport_get_dpif_port(netdev, namebuf,
+ sizeof namebuf),
&dpif_port);
if (error) {
return error;
@@ -1832,9 +1836,12 @@ port_destruct(struct ofport *port_)
{
struct ofport_dpif *port = ofport_dpif_cast(port_);
struct ofproto_dpif *ofproto = ofproto_dpif_cast(port->up.ofproto);
- const char *dp_port_name = netdev_vport_get_dpif_port(port->up.netdev);
const char *devname = netdev_get_name(port->up.netdev);
+ char namebuf[NETDEV_VPORT_NAME_BUFSIZE];
+ const char *dp_port_name;
+ dp_port_name = netdev_vport_get_dpif_port(port->up.netdev, namebuf,
+ sizeof namebuf);
if (dpif_port_exists(ofproto->backer->dpif, dp_port_name)) {
/* The underlying device is still there, so delete it. This
* happens when the ofproto is being destroyed, since the caller
@@ -3226,14 +3233,16 @@ static int
port_add(struct ofproto *ofproto_, struct netdev *netdev)
{
struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
- const char *dp_port_name = netdev_vport_get_dpif_port(netdev);
const char *devname = netdev_get_name(netdev);
+ char namebuf[NETDEV_VPORT_NAME_BUFSIZE];
+ const char *dp_port_name;
if (netdev_vport_is_patch(netdev)) {
sset_add(&ofproto->ghost_ports, netdev_get_name(netdev));
return 0;
}
+ dp_port_name = netdev_vport_get_dpif_port(netdev, namebuf, sizeof namebuf);
if (!dpif_port_exists(ofproto->backer->dpif, dp_port_name)) {
uint32_t port_no = UINT32_MAX;
int error;
--
1.7.2.5
More information about the dev
mailing list