netdev-vport: Don't return static data in netdev_vport_get_dpif_port().

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@nicira.com>

diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c
index ac20ae7..1383b58 100644 (file)
--- a/lib/dpif-linux.c
+++ b/lib/dpif-linux.c
@@ -484,7 +484,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 30a1832..52aedb6 100644 (file)
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -440,8 +440,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;
@@ -450,12 +453,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;
 }
@@ -1079,6 +1081,7 @@ dpif_netdev_run(struct dpif *dpif)
             dp_netdev_port_input(dp, port, &packet, 0, 0, NULL);
         } 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 3558f43..c0d8a3c 100644 (file)
--- a/lib/netdev-vport.c
+++ b/lib/netdev-vport.c
@@ -121,14 +121,14 @@ netdev_vport_class_get_dpif_port(const struct netdev_class *class)
 }
 
 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 char *dpif_port;
 
     if (netdev_vport_needs_dst_port(netdev)) {
         const struct netdev_vport *vport = netdev_vport_cast(netdev);
         const char *type = netdev_get_type(netdev);
-        static char dpif_port_combined[IFNAMSIZ];
 
         /*
          * Note: IFNAMSIZ is 16 bytes long. The maximum length of a VXLAN
@@ -136,10 +136,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;
     } else {
         const struct netdev_class *class = netdev_get_class(netdev);
         dpif_port = netdev_vport_class_get_dpif_port(class);
@@ -148,6 +149,15 @@ netdev_vport_get_dpif_port(const struct netdev *netdev)
     return dpif_port ? dpif_port : 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 **netdevp)

diff --git a/lib/netdev-vport.h b/lib/netdev-vport.h
index 94fe937..5394966 100644 (file)
--- a/lib/netdev-vport.h
+++ b/lib/netdev-vport.h
@@ -18,6 +18,7 @@
 #define NETDEV_VPORT_H 1
 
 #include <stdbool.h>
+#include <stddef.h>
 
 struct dpif_linux_vport;
 struct dpif_flow_stats;
@@ -37,7 +38,11 @@ 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 *);
 const char *netdev_vport_class_get_dpif_port(const struct netdev_class *);
 
+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 776be96..ec2d95d 100644 (file)
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -957,13 +957,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);
@@ -1807,6 +1809,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;
 
@@ -1835,7 +1838,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;
@@ -1872,9 +1876,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
@@ -3318,14 +3325,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;