socket-util: Use correct address family in set_dscp(), instead of guessing.

The set_dscp() function, until now, tried to set the DSCP as IPv4 and as
IPv6. This worked OK on Linux, where an ENOPROTOOPT error made it really
clear which one was wrong, but FreeBSD uses EINVAL instead, which has
multiple meanings and which it therefore seems somewhat risky to ignore.
Instead, this commit just tries to set the correct address family's DSCP
option.

Tested by Alex Wang on FreeBSD 9.3.

Reported-by: Atanu Ghosh <atanu@acm.org>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Co-authored-by: Alex Wang <alexw@nicira.com>
Signed-off-by: Alex Wang <alexw@nicira.com>
Tested-by: Alex Wang <alexw@nicira.com>

diff --git a/lib/socket-util.c b/lib/socket-util.c
index aa0c719..e32aa2b 100644 (file)
--- a/lib/socket-util.c
+++ b/lib/socket-util.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -106,46 +106,34 @@ xset_nonblocking(int fd)
     }
 }
 
+/* Sets the DSCP value of socket 'fd' to 'dscp', which must be 63 or less.
+ * 'family' must indicate the socket's address family (AF_INET or AF_INET6, to
+ * do anything useful). */
 int
-set_dscp(int fd, uint8_t dscp)
+set_dscp(int fd, int family, uint8_t dscp)
 {
+    int retval;
     int val;
-    bool success;
 
     if (dscp > 63) {
         return EINVAL;
     }
-
-    /* Note: this function is used for both of IPv4 and IPv6 sockets */
-    success = false;
     val = dscp << 2;
-    if (setsockopt(fd, IPPROTO_IP, IP_TOS, &val, sizeof val)) {
-#ifndef _WIN32
-        if (sock_errno() != ENOPROTOOPT) {
-#else
-        if (sock_errno() != WSAENOPROTOOPT) {
-#endif
-            return sock_errno();
-        }
-    } else {
-        success = true;
-    }
-    if (setsockopt(fd, IPPROTO_IPV6, IPV6_TCLASS, &val, sizeof val)) {
-#ifndef _WIN32
-        if (sock_errno() != ENOPROTOOPT) {
-#else
-        if (sock_errno() != WSAENOPROTOOPT) {
-#endif
-            return sock_errno();
-        }
-    } else {
-        success = true;
-    }
-    if (!success) {
+
+    switch (family) {
+    case AF_INET:
+        retval = setsockopt(fd, IPPROTO_IP, IP_TOS, &val, sizeof val);
+        break;
+
+    case AF_INET6:
+        retval = setsockopt(fd, IPPROTO_IPV6, IPV6_TCLASS, &val, sizeof val);
+        break;
+
+    default:
         return ENOPROTOOPT;
     }
 
-    return 0;
+    return retval ? sock_errno() : 0;
 }
 
 /* Translates 'host_name', which must be a string representation of an IP
@@ -778,7 +766,7 @@ inet_open_active(int style, const char *target, uint16_t default_port,
     /* The dscp bits must be configured before connect() to ensure that the
      * TOS field is set during the connection establishment.  If set after
      * connect(), the handshake SYN frames will be sent with a TOS of 0. */
-    error = set_dscp(fd, dscp);
+    error = set_dscp(fd, ss.ss_family, dscp);
     if (error) {
         VLOG_ERR("%s: set_dscp: %s", target, sock_strerror(error));
         goto exit;
@@ -915,7 +903,7 @@ inet_open_passive(int style, const char *target, int default_port,
     /* The dscp bits must be configured before connect() to ensure that the TOS
      * field is set during the connection establishment.  If set after
      * connect(), the handshake SYN frames will be sent with a TOS of 0. */
-    error = set_dscp(fd, dscp);
+    error = set_dscp(fd, ss.ss_family, dscp);
     if (error) {
         VLOG_ERR("%s: set_dscp: %s", target, sock_strerror(error));
         goto error;

diff --git a/lib/socket-util.h b/lib/socket-util.h
index 2acc974..63f1d5f 100644 (file)
--- a/lib/socket-util.h
+++ b/lib/socket-util.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -29,7 +29,7 @@
 
 int set_nonblocking(int fd);
 void xset_nonblocking(int fd);
-int set_dscp(int fd, uint8_t dscp);
+int set_dscp(int fd, int family, uint8_t dscp);
 
 int lookup_ip(const char *host_name, struct in_addr *address);
 int lookup_ipv6(const char *host_name, struct in6_addr *address);

diff --git a/python/ovs/socket_util.py b/python/ovs/socket_util.py
index 1af6474..f657d11 100644 (file)
--- a/python/ovs/socket_util.py
+++ b/python/ovs/socket_util.py
@@ -1,4 +1,4 @@
-# Copyright (c) 2010, 2012, 2014 Nicira, Inc.
+# Copyright (c) 2010, 2012, 2014, 2015 Nicira, Inc.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -213,14 +213,16 @@ def inet_open_active(style, target, default_port, dscp):
         is_addr_inet = is_valid_ipv4_address(address[0])
         if is_addr_inet:
             sock = socket.socket(socket.AF_INET, style, 0)
+            family = socket.AF_INET
         else:
             sock = socket.socket(socket.AF_INET6, style, 0)
+            family = socket.AF_INET6
     except socket.error, e:
         return get_exception_errno(e), None
 
     try:
         set_nonblocking(sock)
-        set_dscp(sock, dscp)
+        set_dscp(sock, family, dscp)
         try:
             sock.connect(address)
         except socket.error, e:
@@ -292,21 +294,20 @@ def set_nonblocking(sock):
                  % os.strerror(get_exception_errno(e)))
 
 
-def set_dscp(sock, dscp):
+def set_dscp(sock, family, dscp):
     if dscp > 63:
         raise ValueError("Invalid dscp %d" % dscp)
 
-    # Note: this function is used for both of IPv4 and IPv6 sockets
-    success = False
     val = dscp << 2
-    try:
-        sock.setsockopt(socket.IPPROTO_IP, socket.IP_TOS, val)
-    except socket.error, e:
-        if get_exception_errno(e) != errno.ENOPROTOOPT:
+    if family == socket.AF_INET:
+        try:
+            sock.setsockopt(socket.IPPROTO_IP, socket.IP_TOS, val)
+        except socket.error, e:
             raise
-    success = True
-    try:
-        sock.setsockopt(socket.IPPROTO_IPV6, socket.IPV6_TCLASS, val)
-    except socket.error, e:
-        if get_exception_errno(e) != errno.ENOPROTOOPT or not success:
+    elif family == socket.AF_INET6:
+        try:
+            sock.setsockopt(socket.IPPROTO_IPV6, socket.IPV6_TCLASS, val)
+        except socket.error, e:
             raise
+    else:
+        raise