Thanks, applied to master.
On Thu, Jun 06, 2013 at 01:33:17PM -0700, Andy Zhou wrote:
> Looks good. Thanks.
>
>
> On Wed, Jun 5, 2013 at 1:05 PM, Ben Pfaff <b...@nicira.com> wrote:
>
> > 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 <b...@nicira.com>
> > ---
> > 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 ac20ae7..1383b58 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 9ea8d24..36992a6 100644
> > --- a/lib/dpif-netdev.c
> > +++ b/lib/dpif-netdev.c
> > @@ -436,8 +436,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;
> > @@ -446,12 +449,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;
> > }
> > @@ -1074,6 +1076,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 4bb41bd..bdb0c4d 100644
> > --- a/lib/netdev-vport.c
> > +++ b/lib/netdev-vport.c
> > @@ -121,12 +121,12 @@ 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)
> > {
> > 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
> > @@ -134,10 +134,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);
> > const char *dpif_port = netdev_vport_class_get_dpif_port(class);
> > @@ -145,6 +146,15 @@ netdev_vport_get_dpif_port(const struct netdev
> > *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
> > --- 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 096f0f2..cfcd4c8 100644
> > --- a/ofproto/ofproto-dpif.c
> > +++ b/ofproto/ofproto-dpif.c
> > @@ -956,13 +956,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);
> > @@ -1806,6 +1808,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;
> >
> > @@ -1834,7 +1837,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;
> > @@ -1871,9 +1875,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
> > @@ -3317,14 +3324,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
> >
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
> >
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
