[ovs-dev] [PATCH] netdev-vport: Use dpif_port as base for tunnel backing port.

[Jesse Gross]

In most cases, tunnel ports specify a dpif name to act as the backing
port in the datapath. However, in the case of UDP tunnels the type is
used with the port number appended. This is potentially a problem for
IPsec tunnels because they have different types but should have the
same backing port. The hasn't been a problem in practice though because
no UDP tunnels are currently used with IPsec.

This switches to use the dpif_port in all cases plus a port number if
necessary. It does this by making the names short enough to accomodate
ports, which also makes the naming more consistent.

Signed-off-by: Jesse Gross <jesse at nicira.com>
---
 lib/netdev-vport.c | 36 +++++++++++++++++++++---------------
 lib/netdev.c       |  2 +-
 tests/ovs-vsctl.at | 16 ++++++++--------
 3 files changed, 30 insertions(+), 24 deletions(-)

diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
index c214bf7..6e427a1 100644
--- a/lib/netdev-vport.c
+++ b/lib/netdev-vport.c
@@ -146,25 +146,29 @@ const char *
 netdev_vport_get_dpif_port(const struct netdev *netdev,
                            char namebuf[], size_t bufsize)
 {
+    const struct netdev_class *class = netdev_get_class(netdev);
+    const char *dpif_port = netdev_vport_class_get_dpif_port(class);
+
+    if (!dpif_port) {
+        return netdev_get_name(netdev);
+    }
+
     if (netdev_vport_needs_dst_port(netdev)) {
         const struct netdev_vport *vport = netdev_vport_cast(netdev);
         const char *type = netdev_get_type(netdev);
 
         /*
-         * Note: IFNAMSIZ is 16 bytes long. The maximum length of a VXLAN
-         * or LISP port name below is 15 or 14 bytes respectively. Still,
-         * assert here on the size of strlen(type) in case that changes
-         * in the future.
+         * Note: IFNAMSIZ is 16 bytes long. Implementations should chose
+         * a dpif port name that is short enough to fit including any
+         * port numbers but assert just in case.
          */
         BUILD_ASSERT(NETDEV_VPORT_NAME_BUFSIZE >= IFNAMSIZ);
-        ovs_assert(strlen(type) + 10 < IFNAMSIZ);
-        snprintf(namebuf, bufsize, "%s_sys_%d", type,
+        ovs_assert(strlen(dpif_port) + 6 < IFNAMSIZ);
+        snprintf(namebuf, bufsize, "%s_%d", dpif_port,
                  ntohs(vport->tnl_cfg.dst_port));
         return namebuf;
     } else {
-        const struct netdev_class *class = netdev_get_class(netdev);
-        const char *dpif_port = netdev_vport_class_get_dpif_port(class);
-        return dpif_port ? dpif_port : netdev_get_name(netdev);
+        return dpif_port;
     }
 }
 
@@ -825,13 +829,15 @@ get_stats(const struct netdev *netdev, struct netdev_stats *stats)
 void
 netdev_vport_tunnel_register(void)
 {
+    /* The name of the dpif_port should be short enough to accomodate adding
+     * port number to the end if one is necessary. */
     static const struct vport_class vport_classes[] = {
-        TUNNEL_CLASS("gre", "gre_system"),
-        TUNNEL_CLASS("ipsec_gre", "gre_system"),
-        TUNNEL_CLASS("gre64", "gre64_system"),
-        TUNNEL_CLASS("ipsec_gre64", "gre64_system"),
-        TUNNEL_CLASS("vxlan", "vxlan_system"),
-        TUNNEL_CLASS("lisp", "lisp_system")
+        TUNNEL_CLASS("gre", "gre_sys"),
+        TUNNEL_CLASS("ipsec_gre", "gre_sys"),
+        TUNNEL_CLASS("gre64", "gre64_sys"),
+        TUNNEL_CLASS("ipsec_gre64", "gre64_sys"),
+        TUNNEL_CLASS("vxlan", "vxlan_sys"),
+        TUNNEL_CLASS("lisp", "lisp_sys")
     };
     static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
 
diff --git a/lib/netdev.c b/lib/netdev.c
index dd800a4..07cda42 100644
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -298,7 +298,7 @@ netdev_is_reserved_name(const char *name)
     ovs_mutex_lock(&netdev_class_mutex);
     HMAP_FOR_EACH (rc, hmap_node, &netdev_classes) {
         const char *dpif_port = netdev_vport_class_get_dpif_port(rc->class);
-        if (dpif_port && !strcmp(dpif_port, name)) {
+        if (dpif_port && !strncmp(name, dpif_port, strlen(dpif_port))) {
             ovs_mutex_unlock(&netdev_class_mutex);
             return true;
         }
diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
index 440bf1a..8bc5f4a 100644
--- a/tests/ovs-vsctl.at
+++ b/tests/ovs-vsctl.at
@@ -1203,10 +1203,10 @@ m4_foreach(
 [reserved_name],
 [[ovs-netdev],
 [ovs-dummy],
-[gre_system],
-[gre64_system],
-[lisp_system],
-[vxlan_system]],
+[gre_sys],
+[gre64_sys],
+[lisp_sys],
+[vxlan_sys]],
 [
 # Try creating the port
 AT_CHECK([ovs-vsctl add-port br0 reserved_name], [0], [], [dnl
@@ -1238,10 +1238,10 @@ OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=gre \
 # Test creating all reserved tunnel port names
 m4_foreach(
 [reserved_name],
-[[gre_system],
-[gre64_system],
-[lisp_system],
-[vxlan_system]],
+[[gre_sys],
+[gre64_sys],
+[lisp_sys],
+[vxlan_sys]],
 [
 # Try creating the port
 AT_CHECK([ovs-vsctl add-port br0 reserved_name], [0], [], [dnl
-- 
1.9.1