[ovs-dev] [netlink v3 16/16] datapath: Change vport type from string to integer enumeration.

Ben Pfaff blp at nicira.com
Thu Dec 30 00:56:37 UTC 2010


I plan to make the vport type part of the standard header stuck on each
Netlink message related to a vport.  As such, it is more convenient to use
an integer than a string.  In addition, by being fundamentally different
from strings, using an integer may reduce the confusion we've had in the
past over the differences in userspace and kernel names for network device
and vport types.

Signed-off-by: Ben Pfaff <blp at nicira.com>
---
 datapath/datapath.c                     |    5 +-
 datapath/vport-capwap.c                 |    2 +-
 datapath/vport-gre.c                    |    2 +-
 datapath/vport-internal_dev.c           |    2 +-
 datapath/vport-netdev.c                 |    2 +-
 datapath/vport-patch.c                  |    2 +-
 datapath/vport.c                        |    4 +-
 datapath/vport.h                        |    9 ++--
 include/openvswitch/datapath-protocol.h |   15 +++++-
 lib/dpif-linux.c                        |   74 +++++++++++++++++++-----------
 lib/netdev-vport.c                      |   11 +++--
 11 files changed, 79 insertions(+), 49 deletions(-)

diff --git a/datapath/datapath.c b/datapath/datapath.c
index d5791d9..3eed867 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -268,7 +268,7 @@ static int create_dp(int dp_idx, const char __user *devnamep)
 	/* Set up our datapath device. */
 	BUILD_BUG_ON(sizeof(internal_dev_port.devname) != sizeof(devname));
 	strcpy(internal_dev_port.devname, devname);
-	strcpy(internal_dev_port.type, "internal");
+	internal_dev_port.type = ODPVT_INTERNAL;
 	err = new_vport(dp, &internal_dev_port, ODPP_LOCAL);
 	if (err) {
 		if (err == -EBUSY)
@@ -390,7 +390,6 @@ static int attach_port(int dp_idx, struct odp_port __user *portp)
 	if (copy_from_user(&port, portp, sizeof port))
 		goto out;
 	port.devname[IFNAMSIZ - 1] = '\0';
-	port.type[VPORT_TYPE_SIZE - 1] = '\0';
 
 	rtnl_lock();
 	dp = get_dp_locked(dp_idx);
@@ -1345,7 +1344,7 @@ static void compose_odp_port(const struct vport *vport, struct odp_port *odp_por
 {
 	rcu_read_lock();
 	strncpy(odp_port->devname, vport_get_name(vport), sizeof(odp_port->devname));
-	strncpy(odp_port->type, vport_get_type(vport), sizeof(odp_port->type));
+	odp_port->type = vport_get_type(vport);
 	vport_get_config(vport, odp_port->config);
 	odp_port->port = vport->port_no;
 	odp_port->dp_idx = vport->dp->dp_idx;
diff --git a/datapath/vport-capwap.c b/datapath/vport-capwap.c
index fc2ea8e..4ef5386 100644
--- a/datapath/vport-capwap.c
+++ b/datapath/vport-capwap.c
@@ -645,7 +645,7 @@ static void capwap_frag_expire(unsigned long ifq)
 }
 
 const struct vport_ops capwap_vport_ops = {
-	.type		= "capwap",
+	.type		= ODPVT_CAPWAP,
 	.flags		= VPORT_F_GEN_STATS,
 	.init		= capwap_init,
 	.exit		= capwap_exit,
diff --git a/datapath/vport-gre.c b/datapath/vport-gre.c
index bf8179b..88e1abe 100644
--- a/datapath/vport-gre.c
+++ b/datapath/vport-gre.c
@@ -387,7 +387,7 @@ static void gre_exit(void)
 }
 
 const struct vport_ops gre_vport_ops = {
-	.type		= "gre",
+	.type		= ODPVT_GRE,
 	.flags		= VPORT_F_GEN_STATS | VPORT_F_TUN_ID,
 	.init		= gre_init,
 	.exit		= gre_exit,
diff --git a/datapath/vport-internal_dev.c b/datapath/vport-internal_dev.c
index b9e54ab..4ec767f 100644
--- a/datapath/vport-internal_dev.c
+++ b/datapath/vport-internal_dev.c
@@ -245,7 +245,7 @@ static int internal_dev_recv(struct vport *vport, struct sk_buff *skb)
 }
 
 const struct vport_ops internal_vport_ops = {
-	.type		= "internal",
+	.type		= ODPVT_INTERNAL,
 	.flags		= VPORT_F_REQUIRED | VPORT_F_GEN_STATS | VPORT_F_FLOW,
 	.create		= internal_dev_create,
 	.destroy	= internal_dev_destroy,
diff --git a/datapath/vport-netdev.c b/datapath/vport-netdev.c
index d492d19..0444848 100644
--- a/datapath/vport-netdev.c
+++ b/datapath/vport-netdev.c
@@ -288,7 +288,7 @@ struct vport *netdev_get_vport(struct net_device *dev)
 }
 
 const struct vport_ops netdev_vport_ops = {
-	.type		= "netdev",
+	.type		= ODPVT_NETDEV,
 	.flags          = (VPORT_F_REQUIRED |
 			  (USE_VPORT_STATS ? VPORT_F_GEN_STATS : 0)),
 	.init		= netdev_init,
diff --git a/datapath/vport-patch.c b/datapath/vport-patch.c
index 8a3b465..4aa7b6b 100644
--- a/datapath/vport-patch.c
+++ b/datapath/vport-patch.c
@@ -297,7 +297,7 @@ static int patch_send(struct vport *vport, struct sk_buff *skb)
 }
 
 const struct vport_ops patch_vport_ops = {
-	.type		= "patch",
+	.type		= ODPVT_PATCH,
 	.flags		= VPORT_F_GEN_STATS,
 	.init		= patch_init,
 	.exit		= patch_exit,
diff --git a/datapath/vport.c b/datapath/vport.c
index 6c053ac..862d170 100644
--- a/datapath/vport.c
+++ b/datapath/vport.c
@@ -591,7 +591,7 @@ struct vport *vport_add(const struct vport_parms *parms)
 	ASSERT_VPORT();
 
 	for (i = 0; i < n_vport_types; i++) {
-		if (!strcmp(vport_ops_list[i]->type, parms->type)) {
+		if (vport_ops_list[i]->type == parms->type) {
 			vport = vport_ops_list[i]->create(parms);
 			if (IS_ERR(vport)) {
 				err = PTR_ERR(vport);
@@ -748,7 +748,7 @@ const char *vport_get_name(const struct vport *vport)
  * Retrieves the type of the given device.  Either RTNL lock or rcu_read_lock
  * must be held for the entire duration that the type is in use.
  */
-const char *vport_get_type(const struct vport *vport)
+enum odp_vport_type vport_get_type(const struct vport *vport)
 {
 	return vport->ops->type;
 }
diff --git a/datapath/vport.h b/datapath/vport.h
index 6ba7f2f..f112745 100644
--- a/datapath/vport.h
+++ b/datapath/vport.h
@@ -49,7 +49,7 @@ int vport_set_addr(struct vport *, const unsigned char *);
 int vport_set_stats(struct vport *, struct rtnl_link_stats64 *);
 
 const char *vport_get_name(const struct vport *);
-const char *vport_get_type(const struct vport *);
+enum odp_vport_type vport_get_type(const struct vport *);
 const unsigned char *vport_get_addr(const struct vport *);
 
 struct kobject *vport_get_kobj(const struct vport *);
@@ -140,7 +140,7 @@ struct vport {
  */
 struct vport_parms {
 	const char *name;
-	const char *type;
+	enum odp_vport_type type;
 	const void *config;
 
 	/* For vport_alloc(). */
@@ -151,8 +151,7 @@ struct vport_parms {
 /**
  * struct vport_ops - definition of a type of virtual port
  *
- * @type: Name of port type, such as "netdev" or "internal" to be matched
- * against the device type when a new port needs to be created.
+ * @type: %ODPVT_* value for this type of virtual port.
  * @flags: Flags of type VPORT_F_* that influence how the generic vport layer
  * handles this vport.
  * @init: Called at module initialization.  If VPORT_F_REQUIRED is set then the
@@ -188,7 +187,7 @@ struct vport_parms {
  * @send: Send a packet on the device.  Returns the length of the packet sent.
  */
 struct vport_ops {
-	const char *type;
+	enum odp_vport_type type;
 	u32 flags;
 
 	/* Called at module init and exit respectively. */
diff --git a/include/openvswitch/datapath-protocol.h b/include/openvswitch/datapath-protocol.h
index e4c850c..0c92900 100644
--- a/include/openvswitch/datapath-protocol.h
+++ b/include/openvswitch/datapath-protocol.h
@@ -173,17 +173,28 @@ struct odp_upcall {
 	uint32_t type;
 };
 
-#define VPORT_TYPE_SIZE     16
 #define VPORT_CONFIG_SIZE     32
 struct odp_port {
     char devname[16];           /* IFNAMSIZ */
-    char type[VPORT_TYPE_SIZE];
+    uint32_t type;		/* One of ODPVT_*. */
     uint16_t port;
     uint16_t dp_idx;
     uint32_t reserved2;
     __aligned_u64 config[VPORT_CONFIG_SIZE / 8]; /* type-specific */
 };
 
+enum odp_vport_type {
+	ODPVT_UNSPEC,
+	ODPVT_NETDEV,		/* network device */
+	ODPVT_INTERNAL,		/* network device implemented by datapath */
+	ODPVT_PATCH,		/* virtual tunnel connecting two vports */
+	ODPVT_GRE,		/* GRE tunnel */
+	ODPVT_CAPWAP,		/* CAPWAP tunnel */
+	__ODPVT_MAX
+};
+
+#define ODPVT_MAX (__ODPVT_MAX - 1)
+
 /**
  * struct odp_vport_dump - ODP_VPORT_DUMP argument.
  * @port: Points to port structure to fill in.
diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c
index 34ef33d..ce55312 100644
--- a/lib/dpif-linux.c
+++ b/lib/dpif-linux.c
@@ -231,33 +231,49 @@ dpif_linux_set_drop_frags(struct dpif *dpif_, bool drop_frags)
     return do_ioctl(dpif_, ODP_SET_DROP_FRAGS, &drop_frags_int);
 }
 
-static void
-translate_vport_type_to_netdev_type(struct odp_port *port)
+static const char *
+vport_type_to_netdev_type(const struct odp_port *odp_port)
 {
-    char *type = port->type;
+    struct tnl_port_config tnl_config;
 
-    if (!strcmp(type, "netdev")) {
-        ovs_strlcpy(type, "system", sizeof port->type);
-    } else if (!strcmp(type, "gre")) {
-        const struct tnl_port_config *config;
+    switch (odp_port->type) {
+    case ODPVT_UNSPEC:
+        break;
 
-        config = (struct tnl_port_config *)port->config;
-        if (config->flags & TNL_F_IPSEC) {
-            ovs_strlcpy(type, "ipsec_gre", sizeof port->type);
-        }
+    case ODPVT_NETDEV:
+        return "system";
+
+    case ODPVT_INTERNAL:
+        return "internal";
+
+    case ODPVT_PATCH:
+        return "patch";
+
+    case ODPVT_GRE:
+        memcpy(&tnl_config, odp_port->config, sizeof tnl_config);
+        return tnl_config.flags & TNL_F_IPSEC ? "ipsec_gre" : "gre";
+
+    case ODPVT_CAPWAP:
+        return "capwap";
+
+    case __ODPVT_MAX:
+        break;
     }
+
+    VLOG_WARN_RL(&error_rl, "dp%d: port `%s' has unsupported type %"PRIu32,
+                 odp_port->dp_idx, odp_port->devname, odp_port->type);
+    return "unknown";
 }
 
-static void
-translate_netdev_type_to_vport_type(struct odp_port *port)
+static enum odp_vport_type
+netdev_type_to_vport_type(const char *type)
 {
-    char *type = port->type;
-
-    if (!strcmp(type, "system")) {
-        ovs_strlcpy(type, "netdev", sizeof port->type);
-    } else if (!strcmp(type, "ipsec_gre")) {
-        ovs_strlcpy(type, "gre", sizeof port->type);
-    }
+    return (!strcmp(type, "system") ? ODPVT_NETDEV
+            : !strcmp(type, "internal") ? ODPVT_INTERNAL
+            : !strcmp(type, "patch") ? ODPVT_PATCH
+            : !strcmp(type, "gre") || !strcmp(type, "ipsec_gre") ? ODPVT_GRE
+            : !strcmp(type, "capwap") ? ODPVT_CAPWAP
+            : ODPVT_UNSPEC);
 }
 
 static int
@@ -271,9 +287,15 @@ dpif_linux_port_add(struct dpif *dpif, struct netdev *netdev,
 
     memset(&port, 0, sizeof port);
     strncpy(port.devname, name, sizeof port.devname);
-    strncpy(port.type, type, sizeof port.type);
     netdev_vport_get_config(netdev, port.config);
-    translate_netdev_type_to_vport_type(&port);
+
+    port.type = netdev_type_to_vport_type(type);
+    if (port.type == ODPVT_UNSPEC) {
+        VLOG_WARN_RL(&error_rl, "%s: cannot create port `%s' because it has "
+                     "unsupported type `%s'",
+                     dpif_name(dpif), name, type);
+        return EINVAL;
+    }
 
     error = do_ioctl(dpif, ODP_VPORT_ATTACH, &port);
     if (!error) {
@@ -308,9 +330,8 @@ dpif_linux_port_query__(const struct dpif *dpif, uint32_t port_no,
         /* A vport named 'port_name' exists but in some other datapath.  */
         return ENOENT;
     } else {
-        translate_vport_type_to_netdev_type(&odp_port);
         dpif_port->name = xstrdup(odp_port.devname);
-        dpif_port->type = xstrdup(odp_port.type);
+        dpif_port->type = xstrdup(vport_type_to_netdev_type(&odp_port));
         dpif_port->port_no = odp_port.port;
         return 0;
     }
@@ -359,9 +380,8 @@ dpif_linux_port_dump_next(const struct dpif *dpif, void *state,
     } else if (odp_port->devname[0] == '\0') {
         return EOF;
     } else {
-        translate_vport_type_to_netdev_type(odp_port);
         dpif_port->name = odp_port->devname;
-        dpif_port->type = odp_port->type;
+        dpif_port->type = (char *) vport_type_to_netdev_type(odp_port);
         dpif_port->port_no = odp_port->port;
         odp_port->port++;
         return 0;
@@ -678,7 +698,7 @@ lookup_internal_device(const char *name, int *dp_idx, int *port_no)
                          name, strerror(errno));
         }
         return errno;
-    } else if (!strcmp(odp_port.type, "internal")) {
+    } else if (odp_port.type == ODPVT_INTERNAL) {
         *dp_idx = odp_port.dp_idx;
         *port_no = odp_port.port;
         return 0;
diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
index 6561052..e43d23a 100644
--- a/lib/netdev-vport.c
+++ b/lib/netdev-vport.c
@@ -51,6 +51,7 @@ struct netdev_vport {
 };
 
 struct vport_class {
+    enum odp_vport_type type;
     struct netdev_class netdev_class;
     int (*parse_config)(const char *name, const char *type,
                         const struct shash *args, void *config);
@@ -209,7 +210,7 @@ netdev_vport_set_config(struct netdev_dev *dev_, const struct shash *args)
 
     memset(&port, 0, sizeof port);
     strncpy(port.devname, netdev_dev_get_name(dev_), sizeof port.devname);
-    strncpy(port.type, netdev_dev_get_type(dev_), sizeof port.type);
+    port.type = vport_class->type;
     error = vport_class->parse_config(netdev_dev_get_name(dev_),
                                       netdev_dev_get_type(dev_),
                                       args, port.config);
@@ -772,13 +773,13 @@ void
 netdev_vport_register(void)
 {
     static const struct vport_class vport_classes[] = {
-        { { "gre", VPORT_FUNCTIONS },
+        { ODPVT_GRE, { "gre", VPORT_FUNCTIONS },
           parse_tunnel_config, unparse_tunnel_config },
-        { { "ipsec_gre", VPORT_FUNCTIONS },
+        { ODPVT_GRE, { "ipsec_gre", VPORT_FUNCTIONS },
           parse_tunnel_config, unparse_tunnel_config },
-        { { "capwap", VPORT_FUNCTIONS },
+        { ODPVT_CAPWAP, { "capwap", VPORT_FUNCTIONS },
           parse_tunnel_config, unparse_tunnel_config },
-        { { "patch", VPORT_FUNCTIONS },
+        { ODPVT_PATCH, { "patch", VPORT_FUNCTIONS },
           parse_patch_config, unparse_patch_config }
     };
 
-- 
1.7.1





More information about the dev mailing list