Support accepting and displaying port names in OVS tools.

Until now, most ovs-ofctl commands have not accepted names for ports, only
numbers, and have not been able to display port names either.  It's a lot
easier for users if they can use and see meaningful names instead of
arbitrary numbers.  This commit adds that support.

For backward compatibility, only interactive ovs-ofctl commands by default
display port names; to display them in scripts, use the new --names
option.

Signed-off-by: Ben Pfaff <blp@ovn.org>
Tested-by: Aaron Conole <aconole@redhat.com>

1 files changed, 223 insertions, 20 deletions

diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 8f1c40227..157b66b93 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -33,6 +33,7 @@
 #include "id-pool.h"
 #include "openflow/netronome-ext.h"
 #include "openvswitch/dynamic-string.h"
+#include "openvswitch/json.h"
 #include "openvswitch/meta-flow.h"
 #include "openvswitch/ofp-actions.h"
 #include "openvswitch/ofp-errors.h"
@@ -7270,7 +7271,8 @@ ofputil_port_to_ofp11(ofp_port_t ofp10_port)
 
 /* Stores the port number represented by 's' into '*portp'.  's' may be an
  * integer or, for reserved ports, the standard OpenFlow name for the port
- * (e.g. "LOCAL").
+ * (e.g. "LOCAL").  If 'port_map' is nonnull, also accepts names in it (quoted
+ * or unquoted).
  *
  * Returns true if successful, false if 's' is not a valid OpenFlow port number
  * or name.  The caller should issue an error message in this case, because
@@ -7282,7 +7284,9 @@ ofputil_port_to_ofp11(ofp_port_t ofp10_port)
  * of OpenFlow 1.1+ port numbers, mapping those port numbers into the 16-bit
  * range as described in include/openflow/openflow-1.1.h. */
 bool
-ofputil_port_from_string(const char *s, ofp_port_t *portp)
+ofputil_port_from_string(const char *s,
+                         const struct ofputil_port_map *port_map,
+                         ofp_port_t *portp)
 {
     unsigned int port32; /* int is at least 32 bits wide. */
 
@@ -7301,7 +7305,8 @@ ofputil_port_from_string(const char *s, ofp_port_t *portp)
         } else if (port32 <= ofp_to_u16(OFPP_LAST_RESV)) {
             char name[OFP10_MAX_PORT_NAME_LEN];
 
-            ofputil_port_to_string(u16_to_ofp(port32), name, sizeof name);
+            ofputil_port_to_string(u16_to_ofp(port32), NULL,
+                                   name, sizeof name);
             VLOG_WARN_ONCE("referring to port %s as %"PRIu32" is deprecated "
                            "for compatibility with OpenFlow 1.1 and later",
                            name, port32);
@@ -7334,20 +7339,89 @@ ofputil_port_from_string(const char *s, ofp_port_t *portp)
                 return true;
             }
         }
+
+        ofp_port_t ofp_port = OFPP_NONE;
+        if (s[0] != '"') {
+            ofp_port = ofputil_port_map_get_number(port_map, s);
+        } else {
+            size_t length = strlen(s);
+            char *name = NULL;
+            if (length > 1
+                && s[length - 1] == '"'
+                && json_string_unescape(s + 1, length - 2, &name)) {
+                ofp_port = ofputil_port_map_get_number(port_map, name);
+            }
+            free(name);
+        }
+        if (ofp_port != OFPP_NONE) {
+            *portp = ofp_port;
+            return true;
+        }
+
         return false;
     }
 }
 
+const char *
+ofputil_port_get_reserved_name(ofp_port_t port)
+{
+    switch (port) {
+#define OFPUTIL_NAMED_PORT(NAME) case OFPP_##NAME: return #NAME;
+        OFPUTIL_NAMED_PORTS
+#undef OFPUTIL_NAMED_PORT
+
+    default:
+        return NULL;
+    }
+}
+
+/* A port name doesn't need to be quoted if it is alphanumeric and starts with
+ * a letter. */
+static bool
+port_name_needs_quotes(const char *port_name)
+{
+    if (!isalpha((unsigned char) port_name[0])) {
+        return true;
+    }
+
+    for (const char *p = port_name + 1; *p; p++) {
+        if (!isalnum((unsigned char) *p)) {
+            return true;
+        }
+    }
+    return false;
+}
+
+static void
+put_port_name(const char *port_name, struct ds *s)
+{
+    if (port_name_needs_quotes(port_name)) {
+        json_string_escape(port_name, s);
+    } else {
+        ds_put_cstr(s, port_name);
+    }
+}
+
 /* Appends to 's' a string representation of the OpenFlow port number 'port'.
  * Most ports' string representation is just the port number, but for special
  * ports, e.g. OFPP_LOCAL, it is the name, e.g. "LOCAL". */
 void
-ofputil_format_port(ofp_port_t port, struct ds *s)
+ofputil_format_port(ofp_port_t port, const struct ofputil_port_map *port_map,
+                    struct ds *s)
 {
-    char name[OFP10_MAX_PORT_NAME_LEN];
+    const char *reserved_name = ofputil_port_get_reserved_name(port);
+    if (reserved_name) {
+        ds_put_cstr(s, reserved_name);
+        return;
+    }
 
-    ofputil_port_to_string(port, name, sizeof name);
-    ds_put_cstr(s, name);
+    const char *port_name = ofputil_port_map_get_name(port_map, port);
+    if (port_name) {
+        put_port_name(port_name, s);
+        return;
+    }
+
+    ds_put_format(s, "%"PRIu32, port);
 }
 
 /* Puts in the 'bufsize' byte in 'namebuf' a null-terminated string
@@ -7356,22 +7430,149 @@ ofputil_format_port(ofp_port_t port, struct ds *s)
  * by name, e.g. "LOCAL". */
 void
 ofputil_port_to_string(ofp_port_t port,
-                       char namebuf[OFP10_MAX_PORT_NAME_LEN], size_t bufsize)
+                       const struct ofputil_port_map *port_map,
+                       char *namebuf, size_t bufsize)
 {
-    switch (port) {
-#define OFPUTIL_NAMED_PORT(NAME)                        \
-        case OFPP_##NAME:                               \
-            ovs_strlcpy(namebuf, #NAME, bufsize);       \
-            break;
-        OFPUTIL_NAMED_PORTS
-#undef OFPUTIL_NAMED_PORT
+    const char *reserved_name = ofputil_port_get_reserved_name(port);
+    if (reserved_name) {
+        ovs_strlcpy(namebuf, reserved_name, bufsize);
+        return;
+    }
 
-    default:
-        snprintf(namebuf, bufsize, "%"PRIu32, port);
-        break;
+    const char *port_name = ofputil_port_map_get_name(port_map, port);
+    if (port_name) {
+        struct ds s = DS_EMPTY_INITIALIZER;
+        put_port_name(port_name, &s);
+        ovs_strlcpy(namebuf, ds_cstr(&s), bufsize);
+        ds_destroy(&s);
+        return;
     }
+
+    snprintf(namebuf, bufsize, "%"PRIu32, port);
 }
+
+/* ofputil_port_map.  */
+struct ofputil_port_map_node {
+    struct hmap_node name_node;
+    struct hmap_node number_node;
+    ofp_port_t ofp_port;        /* Port number. */
+    char *name;                 /* Port name. */
+
+    /* OpenFlow doesn't require port names to be unique, although that's the
+     * only sensible way.  However, even in Open vSwitch it's possible for two
+     * ports to appear to have the same name if their names are longer than the
+     * maximum length supported by a given version of OpenFlow.  So, we guard
+     * against duplicate names to avoid giving unexpected results in this
+     * corner case.
+     *
+     * OpenFlow does require port numbers to be unique.  We check for duplicate
+     * ports numbers just in case a switch has a bug. */
+    bool duplicate;
+};
 
+void
+ofputil_port_map_init(struct ofputil_port_map *map)
+{
+    hmap_init(&map->by_name);
+    hmap_init(&map->by_number);
+}
+
+static struct ofputil_port_map_node *
+ofputil_port_map_find_by_name(const struct ofputil_port_map *map,
+                              const char *name)
+{
+    struct ofputil_port_map_node *node;
+
+    HMAP_FOR_EACH_WITH_HASH (node, name_node, hash_string(name, 0),
+                             &map->by_name) {
+        if (!strcmp(name, node->name)) {
+            return node;
+        }
+    }
+    return NULL;
+}
+
+static struct ofputil_port_map_node *
+ofputil_port_map_find_by_number(const struct ofputil_port_map *map,
+                                ofp_port_t ofp_port)
+{
+    struct ofputil_port_map_node *node;
+
+    HMAP_FOR_EACH_IN_BUCKET (node, number_node, hash_ofp_port(ofp_port),
+                             &map->by_number) {
+        if (node->ofp_port == ofp_port) {
+            return node;
+        }
+    }
+    return NULL;
+}
+
+void
+ofputil_port_map_put(struct ofputil_port_map *map,
+                     ofp_port_t ofp_port, const char *name)
+{
+    struct ofputil_port_map_node *node;
+
+    /* Look for duplicate name. */
+    node = ofputil_port_map_find_by_name(map, name);
+    if (node) {
+        if (node->ofp_port != ofp_port) {
+            node->duplicate = true;
+        }
+        return;
+    }
+
+    /* Look for duplicate number. */
+    node = ofputil_port_map_find_by_number(map, ofp_port);
+    if (node) {
+        node->duplicate = true;
+        return;
+    }
+
+    /* Add new node. */
+    node = xmalloc(sizeof *node);
+    hmap_insert(&map->by_number, &node->number_node, hash_ofp_port(ofp_port));
+    hmap_insert(&map->by_name, &node->name_node, hash_string(name, 0));
+    node->ofp_port = ofp_port;
+    node->name = xstrdup(name);
+    node->duplicate = false;
+}
+
+const char *
+ofputil_port_map_get_name(const struct ofputil_port_map *map,
+                          ofp_port_t ofp_port)
+{
+    struct ofputil_port_map_node *node
+        = map ? ofputil_port_map_find_by_number(map, ofp_port) : NULL;
+    return node && !node->duplicate ? node->name : NULL;
+}
+
+ofp_port_t
+ofputil_port_map_get_number(const struct ofputil_port_map *map,
+                            const char *name)
+{
+    struct ofputil_port_map_node *node
+        = map ? ofputil_port_map_find_by_name(map, name) : NULL;
+    return node && !node->duplicate ? node->ofp_port : OFPP_NONE;
+}
+
+void
+ofputil_port_map_destroy(struct ofputil_port_map *map)
+{
+    if (map) {
+        struct ofputil_port_map_node *node, *next;
+
+        HMAP_FOR_EACH_SAFE (node, next, name_node, &map->by_name) {
+            hmap_remove(&map->by_name, &node->name_node);
+            hmap_remove(&map->by_number, &node->number_node);
+            free(node->name);
+            free(node);
+        }
+        hmap_destroy(&map->by_name);
+        hmap_destroy(&map->by_number);
+    }
+}
+
 /* Stores the group id represented by 's' into '*group_idp'.  's' may be an
  * integer or, for reserved group IDs, the standard OpenFlow name for the group
  * (either "ANY" or "ALL").
@@ -7547,13 +7748,15 @@ ofputil_normalize_match__(struct match *match, bool may_log)
     /* Log any changes. */
     if (!flow_wildcards_equal(&wc, &match->wc)) {
         bool log = may_log && !VLOG_DROP_INFO(&bad_ofmsg_rl);
-        char *pre = log ? match_to_string(match, OFP_DEFAULT_PRIORITY) : NULL;
+        char *pre = (log
+                     ? match_to_string(match, NULL, OFP_DEFAULT_PRIORITY)
+                     : NULL);
 
         match->wc = wc;
         match_zero_wildcarded_fields(match);
 
         if (log) {
-            char *post = match_to_string(match, OFP_DEFAULT_PRIORITY);
+            char *post = match_to_string(match, NULL, OFP_DEFAULT_PRIORITY);
             VLOG_INFO("normalization changed ofp_match, details:");
             VLOG_INFO(" pre: %s", pre);
             VLOG_INFO("post: %s", post);