ovs-vswitchd: Allow more convenient 0x short form for specifying DPIDs.

Until now, ovs-vswitchd has insisted that other-config:datapath-id be
exactly 16 hex digits.  This commit allows shorter forms prefixed by 0x.
This was prompted by Faucet controller examples such as this one:
https://github.com/faucetsdn/faucet/blob/master/docs/README_config.rst
which tend to suggest datapath IDs like 0x1.

CC: Josh Bailey <joshb@google.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>

diff --git a/NEWS b/NEWS
index 89bb7a7dee2aabfeda8454f509f2c23eb0fa30fb..86fa7ea0dde81ebec6ae0af325e253f692394e34 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -25,6 +25,8 @@ Post-v2.8.0
    - DPDK:
      * Add support for DPDK v17.11
      * Add support for vHost IOMMU
+   - vswitchd:
+     * Datapath IDs may now be specified as 0x1 (etc.) instead of 16 digits.
 
 v2.8.0 - 31 Aug 2017
 --------------------

diff --git a/lib/packets.c b/lib/packets.c
index f58937085a8f92cfc4a2de2a3e3a0179b5be9429..7341f5f1f46171fdc888b04993a21261a005c57b 100644 (file)
--- a/lib/packets.c
+++ b/lib/packets.c
@@ -51,6 +51,13 @@ flow_tnl_src(const struct flow_tnl *tnl)
     return tnl->ip_src ? in6_addr_mapped_ipv4(tnl->ip_src) : tnl->ipv6_src;
 }
 
+/* Returns true if 's' consists entirely of hex digits, false otherwise. */
+static bool
+is_all_hex(const char *s)
+{
+    return s[strspn(s, "0123456789abcdefABCDEF")] == '\0';
+}
+
 /* Parses 's' as a 16-digit hexadecimal number representing a datapath ID.  On
  * success stores the dpid into '*dpidp' and returns true, on failure stores 0
  * into '*dpidp' and returns false.
@@ -59,7 +66,10 @@ flow_tnl_src(const struct flow_tnl *tnl)
 bool
 dpid_from_string(const char *s, uint64_t *dpidp)
 {
-    *dpidp = (strlen(s) == 16 && strspn(s, "0123456789abcdefABCDEF") == 16
+    size_t len = strlen(s);
+    *dpidp = ((len == 16 && is_all_hex(s))
+              || (len <= 18 && s[0] == '0' && (s[1] == 'x' || s[1] == 'X')
+                  && is_all_hex(s + 2))
               ? strtoull(s, NULL, 16)
               : 0);
     return *dpidp != 0;

diff --git a/tests/ovs-vswitchd.at b/tests/ovs-vswitchd.at
index 84545305b759b74831557b632c619d3db4b560b5..2adb81e1d31ae53ad07525837171103456e17b2d 100644 (file)
--- a/tests/ovs-vswitchd.at
+++ b/tests/ovs-vswitchd.at
@@ -225,3 +225,41 @@ AT_CHECK([test ! -e b/c.mgmt])
 
 OVS_VSWITCHD_STOP(['/ignoring bridge with invalid name/d'])
 AT_CLEANUP
+
+dnl ----------------------------------------------------------------------
+AT_SETUP([ovs-vswitchd - set datapath IDs])
+OVS_VSWITCHD_START([remove bridge br0 other-config datapath-id])
+
+# Get the default dpid and verify that it is of the expected form.
+AT_CHECK([ovs-vsctl --timeout=10 wait-until bridge br0 datapath-id!='[[]]'])
+AT_CHECK([ovs-vsctl get bridge br0 datapath-id], [0], [stdout])
+orig_dpid=$(tr -d \" < stdout)
+AT_CHECK([sed 's/[[0-9a-f]]/x/g' stdout], [0], ["xxxxxxxxxxxxxxxx"
+])
+AT_CHECK_UNQUOTED([ovs-ofctl show br0 | strip_xids | head -1], [0], [dnl
+OFPT_FEATURES_REPLY: dpid:$orig_dpid
+])
+
+# Set a dpid with 16 hex digits.
+AT_CHECK([ovs-vsctl set bridge br0 other-config:datapath-id=0123456789abcdef])
+AT_CHECK([ovs-vsctl --timeout=10 wait-until bridge br0 datapath-id=0123456789abcdef])
+AT_CHECK([ovs-ofctl show br0 | strip_xids | head -1], [0], [dnl
+OFPT_FEATURES_REPLY: dpid:0123456789abcdef
+])
+
+# Set a dpif with 0x prefix.
+AT_CHECK([ovs-vsctl set bridge br0 other-config:datapath-id=0x5ad515c0])
+AT_CHECK([ovs-vsctl --timeout=10 wait-until bridge br0 datapath-id=000000005ad515c0])
+AT_CHECK([ovs-ofctl show br0 | strip_xids | head -1], [0], [dnl
+OFPT_FEATURES_REPLY: dpid:000000005ad515c0
+])
+
+# Set invalid all-zeros dpid and make sure that the default reappears.
+AT_CHECK([ovs-vsctl set bridge br0 other-config:datapath-id=0x00])
+AT_CHECK([ovs-vsctl --timeout=10 wait-until bridge br0 datapath-id=$orig_dpid])
+AT_CHECK_UNQUOTED([ovs-ofctl show br0 | strip_xids | head -1], [0], [dnl
+OFPT_FEATURES_REPLY: dpid:$orig_dpid
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP

diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index 018d64435387125e627b5d06a21729c8712c4311..37d04b7cf9a4fe4dca4d723c2acd5c5728d014ce 100644 (file)
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -882,8 +882,9 @@
       </column>
 
       <column name="other_config" key="datapath-id">
-        Exactly 16 hex digits to set the OpenFlow datapath ID to a specific
-        value.  May not be all-zero.
+        Overrides the default OpenFlow datapath ID, setting it to the specified
+        value specified in hex.  The value must either have a <code>0x</code>
+        prefix or be exactly 16 hex digits long.  May not be all-zero.
       </column>
 
       <column name="other_config" key="dp-desc">