From 3aa30359b1cce9b597f7c4284a80ed05619e57c1 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Wed, 1 May 2013 11:05:28 -0700 Subject: [PATCH] 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 --- lib/dpif-linux.c | 4 +++- lib/dpif-netdev.c | 9 ++++++--- lib/netdev-vport.c | 18 ++++++++++++++---- lib/netdev-vport.h | 7 ++++++- ofproto/ofproto-dpif.c | 17 +++++++++++++---- 5 files changed, 42 insertions(+), 13 deletions(-) diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c index ac20ae754..1383b5834 100644 --- 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 30a1832ee..52aedb695 100644 --- 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 3558f43a9..c0d8a3c6f 100644 --- 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 94fe93710..53949666e 100644 --- a/lib/netdev-vport.h +++ b/lib/netdev-vport.h @@ -18,6 +18,7 @@ #define NETDEV_VPORT_H 1 #include +#include 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 776be9644..ec2d95d19 100644 --- 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; -- 2.43.0