[ovs-dev] [PATCH] ovs-vswitchd: Allow more convenient 0x short form for specifying DPIDs.

[Ben Pfaff]

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 at google.com>
Signed-off-by: Ben Pfaff <blp at ovn.org>
---
 NEWS                  |  2 ++
 lib/packets.c         | 12 +++++++++++-
 tests/ovs-vswitchd.at | 38 ++++++++++++++++++++++++++++++++++++++
 vswitchd/vswitch.xml  |  5 +++--
 4 files changed, 54 insertions(+), 3 deletions(-)

diff --git a/NEWS b/NEWS
index 1325d3146b3c..0ec5999ee84b 100644
--- a/NEWS
+++ b/NEWS
@@ -5,6 +5,8 @@ Post-v2.8.0
        chassis "hostname" in addition to a chassis "name".
    - Linux kernel 4.13
      * Add support for compiling OVS with the latest Linux 4.13 kernel
+   - 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 74d87eda89e1..d71a3d211b69 100644
--- a/lib/packets.c
+++ b/lib/packets.c
@@ -49,6 +49,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.
@@ -57,7 +64,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 84545305b759..2adb81e1d31a 100644
--- 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 d7f68393b25e..ae22fbb165db 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -867,8 +867,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">
-- 
2.10.2