> Thanks for this and the other review.  Did you try building it?  I have

no.

> not build-tested any of the changes in this series outside of a
> GNU/Linux environment.

i can try a build on netbsd more easily if you can provide
a git repo url i can pull from.

YAMAMOTO Takashi

> 
> On Fri, Aug 02, 2013 at 02:35:42AM +0000, YAMAMOTO Takashi wrote:
>> looks ok to me.
>> 
>> YAMAMOTO Takashi
>> 
>> > Signed-off-by: Ben Pfaff <b...@nicira.com>
>> > CC: Ed Maste <ema...@freebsd.org>
>> > CC: YAMAMOTO Takashi <y...@mwd.biglobe.ne.jp>
>> > ---
>> >  lib/netdev-bsd.c |   91 
>> > +++++++++++++++++++++++++-----------------------------
>> >  1 file changed, 42 insertions(+), 49 deletions(-)
>> > 
>> > diff --git a/lib/netdev-bsd.c b/lib/netdev-bsd.c
>> > index 503207f..d365ebf 100644
>> > --- a/lib/netdev-bsd.c
>> > +++ b/lib/netdev-bsd.c
>> > @@ -50,6 +50,7 @@
>> >  #include "fatal-signal.h"
>> >  #include "ofpbuf.h"
>> >  #include "openflow/openflow.h"
>> > +#include "ovs-thread.h"
>> >  #include "packets.h"
>> >  #include "poll-loop.h"
>> >  #include "socket-util.h"
>> > @@ -107,11 +108,6 @@ enum {
>> >      VALID_CARRIER = 1 << 5
>> >  };
>> >  
>> > -#if defined(__NetBSD__)
>> > -/* AF_LINK socket used for netdev_bsd_get_stats and set_etheraddr */
>> > -static int af_link_sock = -1;
>> > -#endif /* defined(__NetBSD__) */
>> > -
>> >  #define PCAP_SNAPLEN 2048
>> >  
>> >  
>> > @@ -144,6 +140,10 @@ static int get_ifindex(const struct netdev *, int 
>> > *ifindexp);
>> >  static int ifr_get_flags(const struct ifreq *);
>> >  static void ifr_set_flags(struct ifreq *, int flags);
>> >  
>> > +#ifdef __NetBSD__
>> > +static int af_link_ioctl(int command, const void *arg);
>> > +#endif
>> > +
>> >  static int netdev_bsd_init(void);
>> >  
>> >  static bool
>> > @@ -172,29 +172,6 @@ netdev_get_kernel_name(const struct netdev *netdev)
>> >      return netdev_bsd_cast(netdev)->kernel_name;
>> >  }
>> >  
>> > -/* Initialize the AF_INET socket used for ioctl operations */
>> > -static int
>> > -netdev_bsd_init(void)
>> > -{
>> > -#if defined(__NetBSD__)
>> > -    static int status = -1;
>> > -
>> > -    if (status >= 0) {  /* already initialized */
>> > -        return status;
>> > -    }
>> > -
>> > -    af_link_sock = socket(AF_LINK, SOCK_DGRAM, 0);
>> > -    status = af_link_sock >= 0 ? 0 : errno;
>> > -    if (status) {
>> > -        VLOG_ERR("failed to create link socket: %s", 
>> > ovs_strerror(status));
>> > -    }
>> > -
>> > -    return status;
>> > -#else
>> > -    return 0;
>> > -#endif /* defined(__NetBSD__) */
>> > -}
>> > -
>> >  /*
>> >   * Perform periodic work needed by netdev. In BSD netdevs it checks for 
>> > any
>> >   * interface status changes, and eventually calls all the user callbacks.
>> > @@ -931,19 +908,16 @@ netdev_bsd_get_stats(const struct netdev *netdev_, 
>> > struct netdev_stats *stats)
>> >      return 0;
>> >  #elif defined(__NetBSD__)
>> >      struct ifdatareq ifdr;
>> > -    int saved_errno;
>> > -    int ret;
>> > +    int error;
>> >  
>> >      memset(&ifdr, 0, sizeof(ifdr));
>> >      strncpy(ifdr.ifdr_name, netdev_get_kernel_name(netdev_),
>> >              sizeof(ifdr.ifdr_name));
>> > -    ret = ioctl(af_link_sock, SIOCGIFDATA, &ifdr);
>> > -    saved_errno = errno;
>> > -    if (ret == -1) {
>> > -        return saved_errno;
>> > +    error = af_link_ioctl(SIOCGIFDATA, &ifdr);
>> > +    if (!error) {
>> > +        convert_stats(stats, &ifdr.ifdr_data);
>> >      }
>> > -    convert_stats(stats, &ifdr.ifdr_data);
>> > -    return 0;
>> > +    return error;
>> >  #else
>> >  #error not implemented
>> >  #endif
>> > @@ -1402,7 +1376,7 @@ netdev_bsd_change_seq(const struct netdev *netdev)
>> >  const struct netdev_class netdev_bsd_class = {
>> >      "system",
>> >  
>> > -    netdev_bsd_init,
>> > +    NULL, /* init */
>> >      netdev_bsd_run,
>> >      netdev_bsd_wait,
>> >      netdev_bsd_create_system,
>> > @@ -1625,7 +1599,7 @@ set_etheraddr(const char *netdev_name OVS_UNUSED, 
>> > int hwaddr_family OVS_UNUSED,
>> >      struct if_laddrreq req;
>> >      struct sockaddr_dl *sdl;
>> >      struct sockaddr_storage oldaddr;
>> > -    int ret;
>> > +    int error;
>> >  
>> >      /*
>> >       * get the old address, add new one, and then remove old one.
>> > @@ -1641,9 +1615,10 @@ set_etheraddr(const char *netdev_name OVS_UNUSED, 
>> > int hwaddr_family OVS_UNUSED,
>> >      req.addr.ss_family = hwaddr_family;
>> >      sdl = (struct sockaddr_dl *)&req.addr;
>> >      sdl->sdl_alen = hwaddr_len;
>> > -    ret = ioctl(af_link_sock, SIOCGLIFADDR, &req);
>> > -    if (ret == -1) {
>> > -        return errno;
>> > +
>> > +    error = af_link_ioctl(SIOCGLIFADDR, &req);
>> > +    if (error) {
>> > +        return error;
>> >      }
>> >      if (!memcmp(&sdl->sdl_data[sdl->sdl_nlen], mac, hwaddr_len)) {
>> >          return 0;
>> > @@ -1658,19 +1633,15 @@ set_etheraddr(const char *netdev_name OVS_UNUSED, 
>> > int hwaddr_family OVS_UNUSED,
>> >      sdl->sdl_alen = hwaddr_len;
>> >      sdl->sdl_family = hwaddr_family;
>> >      memcpy(sdl->sdl_data, mac, hwaddr_len);
>> > -    ret = ioctl(af_link_sock, SIOCALIFADDR, &req);
>> > -    if (ret == -1) {
>> > -        return errno;
>> > +    error = af_link_ioctl(SIOCALIFADDR, &req);
>> > +    if (error) {
>> > +        return error;
>> >      }
>> >  
>> >      memset(&req, 0, sizeof(req));
>> >      strncpy(req.iflr_name, netdev_name, sizeof(req.iflr_name));
>> >      req.addr = oldaddr;
>> > -    ret = ioctl(af_link_sock, SIOCDLIFADDR, &req);
>> > -    if (ret == -1) {
>> > -        return errno;
>> > -    }
>> > -    return 0;
>> > +    return af_link_ioctl(SIOCDLIFADDR, &req);
>> >  #else
>> >  #error not implemented
>> >  #endif
>> > @@ -1694,3 +1665,25 @@ ifr_set_flags(struct ifreq *ifr, int flags)
>> >      ifr->ifr_flagshigh = flags >> 16;
>> >  #endif
>> >  }
>> > +
>> > +/* Calls ioctl() on an AF_LINK sock, passing the specified 'command' and
>> > + * 'arg'.  Returns 0 if successful, otherwise a positive errno value. */
>> > +int
>> > +af_link_ioctl(int command, const void *arg)
>> > +{
>> > +    static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
>> > +    static int sock;
>> > +
>> > +    if (ovsthread_once_start(&once)) {
>> > +        sock = socket(AF_LINK, SOCK_DGRAM, 0);
>> > +        if (sock < 0) {
>> > +            sock = -errno;
>> > +            VLOG_ERR("failed to create link socket: %s", 
>> > ovs_strerror(errno));
>> > +        }
>> > +        ovsthread_once_done(&once);
>> > +    }
>> > +
>> > +    return (sock < 0 ? -sock
>> > +            : ioctl(sock, command, arg) == -1 ? errno
>> > +            : 0);
>> > +}
>> > -- 
>> > 1.7.10.4
>> > 
>> > 
>> > 
> 
> 
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to