<div dir="ltr">Looks good. Thanks. <br></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Jun 5, 2013 at 1:05 PM, Ben Pfaff <span dir="ltr"><<a href="mailto:blp@nicira.com" target="_blank">blp@nicira.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Returning a static data buffer makes code more brittle and definitely<br>
not thread-safe, so this commit switches to using a caller-provided<br>
buffer instead.<br>
<br>
Signed-off-by: Ben Pfaff <<a href="mailto:blp@nicira.com">blp@nicira.com</a>><br>
---<br>
lib/dpif-linux.c | 4 +++-<br>
lib/dpif-netdev.c | 9 ++++++---<br>
lib/netdev-vport.c | 18 ++++++++++++++----<br>
lib/netdev-vport.h | 7 ++++++-<br>
ofproto/ofproto-dpif.c | 17 +++++++++++++----<br>
5 files changed, 42 insertions(+), 13 deletions(-)<br>
<br>
diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c<br>
index ac20ae7..1383b58 100644<br>
--- a/lib/dpif-linux.c<br>
+++ b/lib/dpif-linux.c<br>
@@ -484,7 +484,9 @@ dpif_linux_port_add(struct dpif *dpif_, struct netdev *netdev,<br>
{<br>
struct dpif_linux *dpif = dpif_linux_cast(dpif_);<br>
const struct netdev_tunnel_config *tnl_cfg;<br>
- const char *name = netdev_vport_get_dpif_port(netdev);<br>
+ char namebuf[NETDEV_VPORT_NAME_BUFSIZE];<br>
+ const char *name = netdev_vport_get_dpif_port(netdev,<br>
+ namebuf, sizeof namebuf);<br>
const char *type = netdev_get_type(netdev);<br>
struct dpif_linux_vport request, reply;<br>
struct nl_sock *sock = NULL;<br>
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c<br>
index 9ea8d24..36992a6 100644<br>
--- a/lib/dpif-netdev.c<br>
+++ b/lib/dpif-netdev.c<br>
@@ -436,8 +436,11 @@ dpif_netdev_port_add(struct dpif *dpif, struct netdev *netdev,<br>
uint32_t *port_nop)<br>
{<br>
struct dp_netdev *dp = get_dp_netdev(dpif);<br>
+ char namebuf[NETDEV_VPORT_NAME_BUFSIZE];<br>
+ const char *dpif_port;<br>
int port_no;<br>
<br>
+ dpif_port = netdev_vport_get_dpif_port(netdev, namebuf, sizeof namebuf);<br>
if (*port_nop != UINT32_MAX) {<br>
if (*port_nop >= MAX_PORTS) {<br>
return EFBIG;<br>
@@ -446,12 +449,11 @@ dpif_netdev_port_add(struct dpif *dpif, struct netdev *netdev,<br>
}<br>
port_no = *port_nop;<br>
} else {<br>
- port_no = choose_port(dp, netdev_vport_get_dpif_port(netdev));<br>
+ port_no = choose_port(dp, dpif_port);<br>
}<br>
if (port_no >= 0) {<br>
*port_nop = port_no;<br>
- return do_add_port(dp, netdev_vport_get_dpif_port(netdev),<br>
- netdev_get_type(netdev), port_no);<br>
+ return do_add_port(dp, dpif_port, netdev_get_type(netdev), port_no);<br>
}<br>
return EFBIG;<br>
}<br>
@@ -1074,6 +1076,7 @@ dpif_netdev_run(struct dpif *dpif)<br>
dp_netdev_port_input(dp, port, &packet);<br>
} else if (error != EAGAIN && error != EOPNOTSUPP) {<br>
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);<br>
+<br>
VLOG_ERR_RL(&rl, "error receiving data from %s: %s",<br>
netdev_get_name(port->netdev), strerror(error));<br>
}<br>
diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c<br>
index 4bb41bd..bdb0c4d 100644<br>
--- a/lib/netdev-vport.c<br>
+++ b/lib/netdev-vport.c<br>
@@ -121,12 +121,12 @@ netdev_vport_class_get_dpif_port(const struct netdev_class *class)<br>
}<br>
<br>
const char *<br>
-netdev_vport_get_dpif_port(const struct netdev *netdev)<br>
+netdev_vport_get_dpif_port(const struct netdev *netdev,<br>
+ char namebuf[], size_t bufsize)<br>
{<br>
if (netdev_vport_needs_dst_port(netdev)) {<br>
const struct netdev_vport *vport = netdev_vport_cast(netdev);<br>
const char *type = netdev_get_type(netdev);<br>
- static char dpif_port_combined[IFNAMSIZ];<br>
<br>
/*<br>
* Note: IFNAMSIZ is 16 bytes long. The maximum length of a VXLAN<br>
@@ -134,10 +134,11 @@ netdev_vport_get_dpif_port(const struct netdev *netdev)<br>
* assert here on the size of strlen(type) in case that changes<br>
* in the future.<br>
*/<br>
+ BUILD_ASSERT(NETDEV_VPORT_NAME_BUFSIZE >= IFNAMSIZ);<br>
ovs_assert(strlen(type) + 10 < IFNAMSIZ);<br>
- snprintf(dpif_port_combined, IFNAMSIZ, "%s_sys_%d", type,<br>
+ snprintf(namebuf, bufsize, "%s_sys_%d", type,<br>
ntohs(vport->tnl_cfg.dst_port));<br>
- return dpif_port_combined;<br>
+ return namebuf;<br>
} else {<br>
const struct netdev_class *class = netdev_get_class(netdev);<br>
const char *dpif_port = netdev_vport_class_get_dpif_port(class);<br>
@@ -145,6 +146,15 @@ netdev_vport_get_dpif_port(const struct netdev *netdev)<br>
}<br>
}<br>
<br>
+char *<br>
+netdev_vport_get_dpif_port_strdup(const struct netdev *netdev)<br>
+{<br>
+ char namebuf[NETDEV_VPORT_NAME_BUFSIZE];<br>
+<br>
+ return xstrdup(netdev_vport_get_dpif_port(netdev, namebuf,<br>
+ sizeof namebuf));<br>
+}<br>
+<br>
static int<br>
netdev_vport_create(const struct netdev_class *netdev_class, const char *name,<br>
struct netdev **netdevp)<br>
diff --git a/lib/netdev-vport.h b/lib/netdev-vport.h<br>
index 94fe937..5394966 100644<br>
--- a/lib/netdev-vport.h<br>
+++ b/lib/netdev-vport.h<br>
@@ -18,6 +18,7 @@<br>
#define NETDEV_VPORT_H 1<br>
<br>
#include <stdbool.h><br>
+#include <stddef.h><br>
<br>
struct dpif_linux_vport;<br>
struct dpif_flow_stats;<br>
@@ -37,7 +38,11 @@ void netdev_vport_inc_rx(const struct netdev *,<br>
void netdev_vport_inc_tx(const struct netdev *,<br>
const struct dpif_flow_stats *);<br>
<br>
-const char *netdev_vport_get_dpif_port(const struct netdev *);<br>
const char *netdev_vport_class_get_dpif_port(const struct netdev_class *);<br>
<br>
+enum { NETDEV_VPORT_NAME_BUFSIZE = 16 };<br>
+const char *netdev_vport_get_dpif_port(const struct netdev *,<br>
+ char namebuf[], size_t bufsize);<br>
+char *netdev_vport_get_dpif_port_strdup(const struct netdev *);<br>
+<br>
#endif /* netdev-vport.h */<br>
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c<br>
index 096f0f2..cfcd4c8 100644<br>
--- a/ofproto/ofproto-dpif.c<br>
+++ b/ofproto/ofproto-dpif.c<br>
@@ -956,13 +956,15 @@ type_run(const char *type)<br>
}<br>
<br>
HMAP_FOR_EACH (iter, up.hmap_node, &ofproto->up.ports) {<br>
+ char namebuf[NETDEV_VPORT_NAME_BUFSIZE];<br>
const char *dp_port;<br>
<br>
if (!iter->tnl_port) {<br>
continue;<br>
}<br>
<br>
- dp_port = netdev_vport_get_dpif_port(iter->up.netdev);<br>
+ dp_port = netdev_vport_get_dpif_port(iter->up.netdev,<br>
+ namebuf, sizeof namebuf);<br>
node = simap_find(&tmp_backers, dp_port);<br>
if (node) {<br>
simap_put(&backer->tnl_backers, dp_port, node->data);<br>
@@ -1806,6 +1808,7 @@ port_construct(struct ofport *port_)<br>
struct ofport_dpif *port = ofport_dpif_cast(port_);<br>
struct ofproto_dpif *ofproto = ofproto_dpif_cast(port->up.ofproto);<br>
const struct netdev *netdev = port->up.netdev;<br>
+ char namebuf[NETDEV_VPORT_NAME_BUFSIZE];<br>
struct dpif_port dpif_port;<br>
int error;<br>
<br>
@@ -1834,7 +1837,8 @@ port_construct(struct ofport *port_)<br>
}<br>
<br>
error = dpif_port_query_by_name(ofproto->backer->dpif,<br>
- netdev_vport_get_dpif_port(netdev),<br>
+ netdev_vport_get_dpif_port(netdev, namebuf,<br>
+ sizeof namebuf),<br>
&dpif_port);<br>
if (error) {<br>
return error;<br>
@@ -1871,9 +1875,12 @@ port_destruct(struct ofport *port_)<br>
{<br>
struct ofport_dpif *port = ofport_dpif_cast(port_);<br>
struct ofproto_dpif *ofproto = ofproto_dpif_cast(port->up.ofproto);<br>
- const char *dp_port_name = netdev_vport_get_dpif_port(port->up.netdev);<br>
const char *devname = netdev_get_name(port->up.netdev);<br>
+ char namebuf[NETDEV_VPORT_NAME_BUFSIZE];<br>
+ const char *dp_port_name;<br>
<br>
+ dp_port_name = netdev_vport_get_dpif_port(port->up.netdev, namebuf,<br>
+ sizeof namebuf);<br>
if (dpif_port_exists(ofproto->backer->dpif, dp_port_name)) {<br>
/* The underlying device is still there, so delete it. This<br>
* happens when the ofproto is being destroyed, since the caller<br>
@@ -3317,14 +3324,16 @@ static int<br>
port_add(struct ofproto *ofproto_, struct netdev *netdev)<br>
{<br>
struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);<br>
- const char *dp_port_name = netdev_vport_get_dpif_port(netdev);<br>
const char *devname = netdev_get_name(netdev);<br>
+ char namebuf[NETDEV_VPORT_NAME_BUFSIZE];<br>
+ const char *dp_port_name;<br>
<br>
if (netdev_vport_is_patch(netdev)) {<br>
sset_add(&ofproto->ghost_ports, netdev_get_name(netdev));<br>
return 0;<br>
}<br>
<br>
+ dp_port_name = netdev_vport_get_dpif_port(netdev, namebuf, sizeof namebuf);<br>
if (!dpif_port_exists(ofproto->backer->dpif, dp_port_name)) {<br>
uint32_t port_no = UINT32_MAX;<br>
int error;<br>
<span class="HOEnZb"><font color="#888888">--<br>
1.7.2.5<br>
<br>
_______________________________________________<br>
dev mailing list<br>
<a href="mailto:dev@openvswitch.org">dev@openvswitch.org</a><br>
<a href="http://openvswitch.org/mailman/listinfo/dev" target="_blank">http://openvswitch.org/mailman/listinfo/dev</a><br>
</font></span></blockquote></div><br></div>