vswitch: Use consistent representation of DSCP bits.

There are two sensible ways to represent the 6 DSCP bits of an IP
packet.  One could represent them as an integer in the range 0 to
63.  Or one could represent them as they would appear in the tos
field (0 to 63) << 2.  Before this patch, OVS had used the former
method for the DSCP bits in the Queue Table, and the latter for the
DSCP in the Controller and Manager tables.  Since the ability to
set DSCP bits in the Controller and Manager tables is so new that
it hasn't been released yet, this patch changes it to use the
existing style employed in the Queue table.  Hopefully this should
make the code and configuration less confusing.

Signed-off-by: Ethan Jackson <ethan@nicira.com>

diff --git a/lib/socket-util.c b/lib/socket-util.c
index 3b21256f7ea4ecf6221161389b4178e2818a572d..a9b3b1adcaec1a1cc83a7ad09acb8ade31898a90 100644 (file)
--- a/lib/socket-util.c
+++ b/lib/socket-util.c
@@ -17,6 +17,7 @@
 #include <config.h>
 #include "socket-util.h"
 #include <arpa/inet.h>
+#include <assert.h>
 #include <errno.h>
 #include <fcntl.h>
 #include <net/if.h>
@@ -81,6 +82,21 @@ set_nonblocking(int fd)
     }
 }
 
+static int
+set_dscp(int fd, uint8_t dscp)
+{
+    if (dscp > 63) {
+        return EINVAL;
+    }
+
+    dscp = dscp << 2;
+    if (setsockopt(fd, IPPROTO_IP, IP_TOS, &dscp, sizeof dscp)) {
+        return errno;
+    }
+
+    return 0;
+}
+
 static bool
 rlim_is_finite(rlim_t limit)
 {
@@ -546,7 +562,9 @@ exit:
  * If 'sinp' is non-null, then on success the target address is stored into
  * '*sinp'.
  *
- * 'dscp' becomes the DSCP bits in the IP headers for the new connection. */
+ * 'dscp' becomes the DSCP bits in the IP headers for the new connection.  It
+ * should be in the range [0, 63] and will automatically be shifted to the
+ * appropriately place in the IP tos field. */
 int
 inet_open_active(int style, const char *target, uint16_t default_port,
                  struct sockaddr_in *sinp, int *fdp, uint8_t dscp)
@@ -573,12 +591,12 @@ inet_open_active(int style, const char *target, uint16_t default_port,
         goto exit;
     }
 
-    /* The socket options set here ensure that the TOS bits are set during
-     * the connection establishment.  If set after connect(), the handshake
-     * SYN frames will be sent with a TOS of 0. */
-    if (setsockopt(fd, IPPROTO_IP, IP_TOS, &dscp, sizeof dscp)) {
-        VLOG_ERR("%s: socket: %s", target, strerror(errno));
-        error = errno;
+    /* 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);
+    if (error) {
+        VLOG_ERR("%s: socket: %s", target, strerror(error));
         goto exit;
     }
 
@@ -669,7 +687,9 @@ exit:
  * If 'sinp' is non-null, then on success the bound address is stored into
  * '*sinp'.
  *
- * 'dscp' becomes the DSCP bits in the IP headers for the new connection. */
+ * 'dscp' becomes the DSCP bits in the IP headers for the new connection.  It
+ * should be in the range [0, 63] and will automatically be shifted to the
+ * appropriately place in the IP tos field. */
 int
 inet_open_passive(int style, const char *target, int default_port,
                   struct sockaddr_in *sinp, uint8_t dscp)
@@ -707,12 +727,12 @@ inet_open_passive(int style, const char *target, int default_port,
         goto error;
     }
 
-    /* The socket options set here ensure that the TOS bits are set during
-     * the connection establishment.  If set after connect(), the handshake
-     * SYN frames will be sent with a TOS of 0. */
-    if (setsockopt(fd, IPPROTO_IP, IP_TOS, &dscp, sizeof dscp)) {
-        VLOG_ERR("%s: socket: %s", target, strerror(errno));
-        error = errno;
+    /* 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);
+    if (error) {
+        VLOG_ERR("%s: socket: %s", target, strerror(error));
         goto error;
     }
 

diff --git a/lib/socket-util.h b/lib/socket-util.h
index 07c9327c4d7113d06eb0bc44ee894cf5da128c21..8ea40d5f96195f7d4bd2ca2dfeebf35a5f2bd25d 100644 (file)
--- a/lib/socket-util.h
+++ b/lib/socket-util.h
@@ -66,7 +66,7 @@ char *describe_fd(int fd);
 
 /* Default value of dscp bits for connection between controller and manager.
  * Value of IPTOS_PREC_INTERNETCONTROL = 0xc0 which is defined
- * in <netinet/ip.h> is used.  */
-#define DSCP_DEFAULT IPTOS_PREC_INTERNETCONTROL
+ * in <netinet/ip.h> is used. */
+#define DSCP_DEFAULT (IPTOS_PREC_INTERNETCONTROL >> 2)
 
 #endif /* socket-util.h */

diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
index 699401813eec3f47136fa94ab57d0330143af0f7..d8363a2cbd2c5b2d80f3d71728e15344b033ebe8 100644 (file)
--- a/ovsdb/ovsdb-server.c
+++ b/ovsdb/ovsdb-server.c
@@ -319,12 +319,11 @@ get_datum(struct ovsdb_row *row, const char *column_name,
     return &row->fields[column->index];
 }
 
-/* This function is used to read the string-string key-values from a map.
- * Returns the true if the 'key' is found and returns the "value"  associated
- * with the 'key' in 'stringp', else returns false. */
-static bool
+/* Read string-string key-values from a map.  Returns the value associated with
+ * 'key', if found, or NULL */
+static const char *
 read_map_string_column(const struct ovsdb_row *row, const char *column_name,
-                       const char **stringp, const char *key)
+                       const char *key)
 {
     const struct ovsdb_datum *datum;
     union ovsdb_atom *atom_key = NULL, *atom_value = NULL;
@@ -334,8 +333,7 @@ read_map_string_column(const struct ovsdb_row *row, const char *column_name,
                       OVSDB_TYPE_STRING, UINT_MAX);
 
     if (!datum) {
-        *stringp = NULL;
-        return false;
+        return NULL;
     }
 
     for (i = 0; i < datum->n; i++) {
@@ -346,8 +344,7 @@ read_map_string_column(const struct ovsdb_row *row, const char *column_name,
         }
     }
 
-    *stringp = atom_value ? atom_value->string : NULL;
-    return atom_value != NULL;
+    return atom_value ? atom_value->string : NULL;
 }
 
 static const union ovsdb_atom *
@@ -427,21 +424,6 @@ write_string_string_column(struct ovsdb_row *row, const char *column_name,
     ovsdb_datum_sort_assert(datum, column->type.key.type);
 }
 
-/* Get the other config for the manager from the database. */
-static void
-manager_get_other_config(const struct ovsdb_row *row,
-                         struct ovsdb_jsonrpc_options *options)
-{
-    const char *temp_string;
-
-    /* Retrieve the configs and store in the options. */
-    if (read_map_string_column(row, "other_config", &temp_string, "dscp")) {
-        options->dscp = atoi(temp_string);
-    } else {
-        options->dscp = DSCP_DEFAULT;
-    }
-}
-
 /* Adds a remote and options to 'remotes', based on the Manager table row in
  * 'row'. */
 static void
@@ -450,7 +432,7 @@ add_manager_options(struct shash *remotes, const struct ovsdb_row *row)
     static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
     struct ovsdb_jsonrpc_options *options;
     long long int max_backoff, probe_interval;
-    const char *target;
+    const char *target, *dscp_string;
 
     if (!read_string_column(row, "target", &target) || !target) {
         VLOG_INFO_RL(&rl, "Table `%s' has missing or invalid `target' column",
@@ -466,7 +448,14 @@ add_manager_options(struct shash *remotes, const struct ovsdb_row *row)
         options->probe_interval = probe_interval;
     }
 
-    manager_get_other_config(row, options);
+    options->dscp = DSCP_DEFAULT;
+    dscp_string = read_map_string_column(row, "other_config", "dscp");
+    if (dscp_string) {
+        int dscp = atoi(dscp_string);
+        if (dscp >= 0 && dscp <= 63) {
+            options->dscp = dscp;
+        }
+    }
 }
 
 static void

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 22a37069c793062d42041f8bf7b4ea3c7a1b1097..74cba8742e61d1128e31dbda06dc1f824b9f1a1f 100644 (file)
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -2436,10 +2436,14 @@ bridge_ofproto_controller_from_ovsrec(const struct ovsrec_controller *c,
     oc->enable_async_msgs = (!c->enable_async_messages
                              || *c->enable_async_messages);
     config_str = ovsrec_controller_get_other_config_value(c, "dscp", NULL);
+
+    oc->dscp = DSCP_DEFAULT;
     if (config_str) {
-        oc->dscp = atoi(config_str);
-    } else {
-        oc->dscp = DSCP_DEFAULT;
+        int dscp = atoi(config_str);
+
+        if (dscp >= 0 && dscp <= 63) {
+            oc->dscp = dscp;
+        }
     }
 }
 

diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema
index c7e1ac9ef4e619f65b682db5c16434be079b0e32..7a51a0d62e50d1fcf7cf3752bdb96fe8e7b34b00 100644 (file)
--- a/vswitchd/vswitch.ovsschema
+++ b/vswitchd/vswitch.ovsschema
@@ -1,6 +1,6 @@
 {"name": "Open_vSwitch",
- "version": "6.9.0",
- "cksum": "617116616 16682",
+ "version": "6.9.1",
+ "cksum": "3226481229 16682",
  "tables": {
    "Open_vSwitch": {
      "columns": {

diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index 1128db945f7e75c0d49f085d7d02bb1a103c17d9..ef019746cee4489b7392fb30e0b0dc6aabe1607c 100644 (file)
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -2698,16 +2698,16 @@
 
       <column name="other_config" key="dscp"
                 type='{"type": "integer"}'>
-        The Differentiated Service Code Point (DSCP) is specified in the IP
-        header. They are specified using 6 bits in the Type of Service (TOS)
-        field in the IP header. DSCP provides a mechanism to classify the
-        network traffic and provide the Quality of Service (QoS) on IP
-        networks.
-        The DSCP value passed is used when establishing the connection between
-        the controller and the Open vSwitch.  The connection must be reset
-        for the new DSCP values to take effect.  If no value is
-        specified, a default value of 192 is chosen for connection
-        establishment.  Valid DSCP values must have their lower 2 bits set to 0.
+        The Differentiated Service Code Point (DSCP) is specified using 6 bits
+        in the Type of Service (TOS) field in the IP header. DSCP provides a
+        mechanism to classify the network traffic and provide Quality of
+        Service (QoS) on IP networks.
+
+        The DSCP value specified here is used when establishing the connection
+        between the controller and the Open vSwitch.  The connection must be
+        reset for the new DSCP values to take effect.  If no value is
+        specified, a default value of 48 is chosen.  Valid DSCP values must be
+        in the range 0 to 63.
       </column>
     </group>
 
@@ -2945,16 +2945,16 @@
 
       <column name="other_config" key="dscp"
                 type='{"type": "integer"}'>
-        The Differentiated Service Code Point (DSCP) is specified in the IP
-        header. They are specified using 6 bits in the Type of Service (TOS)
-        field in the IP header. DSCP provides a mechanism to classify the
-        network traffic and provide the Quality of Service (QoS) on IP
-        networks.
-        The DSCP value passed when establishing the connection between
-        the manager and the Open vSwitch Database.  The connection must be
+        The Differentiated Service Code Point (DSCP) is specified using 6 bits
+        in the Type of Service (TOS) field in the IP header. DSCP provides a
+        mechanism to classify the network traffic and provide Quality of
+        Service (QoS) on IP networks.
+
+        The DSCP value specified here is used when establishing the connection
+        between the manager and the Open vSwitch.  The connection must be
         reset for the new DSCP values to take effect.  If no value is
-        specified, a default value of 192 is chosen for connection
-        establishment.  Valid DSCP values must have their lower 2 bits set to 0.
+        specified, a default value of 48 is chosen.  Valid DSCP values must be
+        in the range 0 to 63.
       </column>
     </group>