[ovs-dev] [netlink v3 1/5] datapath: Encapsulate parameters for new vports in new struct vport_parms.

Ben Pfaff blp at nicira.com
Wed Nov 17 01:11:22 UTC 2010


Upcoming commits will keep needing to pass more information to the vport
'create' member function.  It's annoying to have to modify a dozen pieces
of code every time just to do this, so this commit encapsulates all of the
parameters in a new struct and passes that instead.
---
 datapath/datapath.c           |   11 ++++++-----
 datapath/tunnel.c             |    6 +++---
 datapath/tunnel.h             |    3 +--
 datapath/vport-capwap.c       |    4 ++--
 datapath/vport-gre.c          |    4 ++--
 datapath/vport-internal_dev.c |    5 ++---
 datapath/vport-netdev.c       |    4 ++--
 datapath/vport-patch.c        |    6 +++---
 datapath/vport.c              |   19 ++++++++++---------
 datapath/vport.h              |   24 +++++++++++++++++++-----
 10 files changed, 50 insertions(+), 36 deletions(-)

diff --git a/datapath/datapath.c b/datapath/datapath.c
index 522d4ec..6fcedd3 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -357,13 +357,14 @@ static int new_dp_port(struct datapath *dp, struct odp_port *odp_port, int port_
 
 	vport = vport_locate(odp_port->devname);
 	if (!vport) {
-		vport_lock();
+		struct vport_parms parms;
 
-		if (odp_port->flags & ODP_PORT_INTERNAL)
-			vport = vport_add(odp_port->devname, "internal", NULL);
-		else
-			vport = vport_add(odp_port->devname, "netdev", NULL);
+		parms.name = odp_port->devname;
+		parms.type = odp_port->flags & ODP_PORT_INTERNAL ? "internal" : "netdev";
+		parms.config = NULL;
 
+		vport_lock();
+		vport = vport_add(&parms);
 		vport_unlock();
 
 		if (IS_ERR(vport))
diff --git a/datapath/tunnel.c b/datapath/tunnel.c
index cf74212..36477d1 100644
--- a/datapath/tunnel.c
+++ b/datapath/tunnel.c
@@ -1351,7 +1351,7 @@ static int set_config(const void __user *uconfig, const struct tnl_ops *tnl_ops,
 	return 0;
 }
 
-struct vport *tnl_create(const char *name, const void __user *config,
+struct vport *tnl_create(const struct vport_parms *parms,
 			 const struct vport_ops *vport_ops,
 			 const struct tnl_ops *tnl_ops)
 {
@@ -1368,7 +1368,7 @@ struct vport *tnl_create(const char *name, const void __user *config,
 
 	tnl_vport = tnl_vport_priv(vport);
 
-	strcpy(tnl_vport->name, name);
+	strcpy(tnl_vport->name, parms->name);
 	tnl_vport->tnl_ops = tnl_ops;
 
 	tnl_vport->mutable = kzalloc(sizeof(struct tnl_mutable_config), GFP_KERNEL);
@@ -1383,7 +1383,7 @@ struct vport *tnl_create(const char *name, const void __user *config,
 	get_random_bytes(&initial_frag_id, sizeof(int));
 	atomic_set(&tnl_vport->frag_id, initial_frag_id);
 
-	err = set_config(config, tnl_ops, NULL, tnl_vport->mutable);
+	err = set_config(parms->config, tnl_ops, NULL, tnl_vport->mutable);
 	if (err)
 		goto error_free_mutable;
 
diff --git a/datapath/tunnel.h b/datapath/tunnel.h
index 8ffb7bf..63f7dd4 100644
--- a/datapath/tunnel.h
+++ b/datapath/tunnel.h
@@ -184,8 +184,7 @@ struct tnl_vport {
 #endif
 };
 
-struct vport *tnl_create(const char *name, const void __user *config,
-			 const struct vport_ops *,
+struct vport *tnl_create(const struct vport_parms *, const struct vport_ops *,
 			 const struct tnl_ops *);
 int tnl_modify(struct vport *, const void __user *config);
 int tnl_destroy(struct vport *);
diff --git a/datapath/vport-capwap.c b/datapath/vport-capwap.c
index bf1465f..2e32094 100644
--- a/datapath/vport-capwap.c
+++ b/datapath/vport-capwap.c
@@ -220,9 +220,9 @@ struct tnl_ops capwap_tnl_ops = {
 	.update_header	= capwap_update_header,
 };
 
-static struct vport *capwap_create(const char *name, const void __user *config)
+static struct vport *capwap_create(const struct vport_parms *parms)
 {
-	return tnl_create(name, config, &capwap_vport_ops, &capwap_tnl_ops);
+	return tnl_create(parms, &capwap_vport_ops, &capwap_tnl_ops);
 }
 
 /* Random value.  Irrelevant as long as it's not 0 since we set the handler. */
diff --git a/datapath/vport-gre.c b/datapath/vport-gre.c
index be8fb53..3c84cdf 100644
--- a/datapath/vport-gre.c
+++ b/datapath/vport-gre.c
@@ -340,9 +340,9 @@ struct tnl_ops gre_tnl_ops = {
 	.update_header	= gre_update_header,
 };
 
-static struct vport *gre_create(const char *name, const void __user *config)
+static struct vport *gre_create(const struct vport_parms *parms)
 {
-	return tnl_create(name, config, &gre_vport_ops, &gre_tnl_ops);
+	return tnl_create(parms, &gre_vport_ops, &gre_tnl_ops);
 }
 
 static struct net_protocol gre_protocol_handlers = {
diff --git a/datapath/vport-internal_dev.c b/datapath/vport-internal_dev.c
index 52bd6ce..c74bce8 100644
--- a/datapath/vport-internal_dev.c
+++ b/datapath/vport-internal_dev.c
@@ -195,8 +195,7 @@ static void do_setup(struct net_device *netdev)
 	vport_gen_rand_ether_addr(netdev->dev_addr);
 }
 
-static struct vport *internal_dev_create(const char *name,
-					 const void __user *config)
+static struct vport *internal_dev_create(const struct vport_parms *parms)
 {
 	struct vport *vport;
 	struct netdev_vport *netdev_vport;
@@ -211,7 +210,7 @@ static struct vport *internal_dev_create(const char *name,
 
 	netdev_vport = netdev_vport_priv(vport);
 
-	netdev_vport->dev = alloc_netdev(sizeof(struct internal_dev), name, do_setup);
+	netdev_vport->dev = alloc_netdev(sizeof(struct internal_dev), parms->name, do_setup);
 	if (!netdev_vport->dev) {
 		err = -ENOMEM;
 		goto error_free_vport;
diff --git a/datapath/vport-netdev.c b/datapath/vport-netdev.c
index c023c85..f32ab9f 100644
--- a/datapath/vport-netdev.c
+++ b/datapath/vport-netdev.c
@@ -88,7 +88,7 @@ static void netdev_exit(void)
 }
 #endif
 
-static struct vport *netdev_create(const char *name, const void __user *config)
+static struct vport *netdev_create(const struct vport_parms *parms)
 {
 	struct vport *vport;
 	struct netdev_vport *netdev_vport;
@@ -102,7 +102,7 @@ static struct vport *netdev_create(const char *name, const void __user *config)
 
 	netdev_vport = netdev_vport_priv(vport);
 
-	netdev_vport->dev = dev_get_by_name(&init_net, name);
+	netdev_vport->dev = dev_get_by_name(&init_net, parms->name);
 	if (!netdev_vport->dev) {
 		err = -ENODEV;
 		goto error_free_vport;
diff --git a/datapath/vport-patch.c b/datapath/vport-patch.c
index 62fd71f..f931359 100644
--- a/datapath/vport-patch.c
+++ b/datapath/vport-patch.c
@@ -109,7 +109,7 @@ static int set_config(struct vport *vport, const void __user *uconfig)
 	return 0;
 }
 
-static struct vport *patch_create(const char *name, const void __user *config)
+static struct vport *patch_create(const struct vport_parms *parms)
 {
 	struct vport *vport;
 	struct patch_vport *patch_vport;
@@ -123,9 +123,9 @@ static struct vport *patch_create(const char *name, const void __user *config)
 
 	patch_vport = patch_vport_priv(vport);
 
-	strcpy(patch_vport->name, name);
+	strcpy(patch_vport->name, parms->name);
 
-	err = set_config(vport, config);
+	err = set_config(vport, parms->config);
 	if (err)
 		goto error_free_vport;
 
diff --git a/datapath/vport.c b/datapath/vport.c
index 37d6d8a..f1c105e 100644
--- a/datapath/vport.c
+++ b/datapath/vport.c
@@ -179,6 +179,7 @@ void vport_exit(void)
 
 static int do_vport_add(struct odp_vport_add *vport_config)
 {
+	struct vport_parms parms;
 	struct vport *vport;
 	int err = 0;
 
@@ -193,9 +194,12 @@ static int do_vport_add(struct odp_vport_add *vport_config)
 		goto out;
 	}
 
+	parms.name = vport_config->devname;
+	parms.type = vport_config->port_type;
+	parms.config = vport_config->config;
+
 	vport_lock();
-	vport = vport_add(vport_config->devname, vport_config->port_type,
-			  vport_config->config);
+	vport = vport_add(&parms);
 	vport_unlock();
 
 	if (IS_ERR(vport))
@@ -703,15 +707,12 @@ void vport_free(struct vport *vport)
 /**
  *	vport_add - add vport device (for kernel callers)
  *
- * @name: Name of new device.
- * @type: Type of new device (to be matched against types in registered vport
- * ops).
- * @config: Device type specific configuration.  Userspace pointer.
+ * @parms: Information about new vport.
  *
  * Creates a new vport with the specified configuration (which is dependent
  * on device type).  Both RTNL and vport locks must be held.
  */
-struct vport *vport_add(const char *name, const char *type, const void __user *config)
+struct vport *vport_add(const struct vport_parms *parms)
 {
 	struct vport *vport;
 	int err = 0;
@@ -721,8 +722,8 @@ struct vport *vport_add(const char *name, const char *type, const void __user *c
 	ASSERT_VPORT();
 
 	for (i = 0; i < n_vport_types; i++) {
-		if (!strcmp(vport_ops_list[i]->type, type)) {
-			vport = vport_ops_list[i]->create(name, config);
+		if (!strcmp(vport_ops_list[i]->type, parms->type)) {
+			vport = vport_ops_list[i]->create(parms);
 			if (IS_ERR(vport)) {
 				err = PTR_ERR(vport);
 				goto out;
diff --git a/datapath/vport.h b/datapath/vport.h
index 186d6bf..83ad3e8 100644
--- a/datapath/vport.h
+++ b/datapath/vport.h
@@ -19,6 +19,7 @@
 #include "odp-compat.h"
 
 struct vport;
+struct vport_parms;
 struct dp_port;
 
 /* The following definitions are for users of the vport subsytem: */
@@ -45,7 +46,7 @@ void vport_unlock(void);
 int vport_init(void);
 void vport_exit(void);
 
-struct vport *vport_add(const char *name, const char *type, const void __user *config);
+struct vport *vport_add(const struct vport_parms *);
 int vport_mod(struct vport *, const void __user *config);
 int vport_del(struct vport *);
 
@@ -116,6 +117,20 @@ struct vport {
 #define VPORT_F_TUN_ID		(1 << 3) /* Sets OVS_CB(skb)->tun_id. */
 
 /**
+ * struct vport_parms - parameters for creating a new vport
+ *
+ * @name: New vport's name.
+ * @type: New vport's type.
+ * @config: New vport's configuration, as %NULL or a userspace pointer to an
+ * arbitrary type-specific structure.
+ */
+struct vport_parms {
+	const char *name;
+	const char *type;
+	const void __user *config;
+};
+
+/**
  * struct vport_ops - definition of a type of virtual port
  *
  * @type: Name of port type, such as "netdev" or "internal" to be matched
@@ -126,9 +141,8 @@ struct vport {
  * failure of this function will cause the module to not load.  If the flag is
  * not set and initialzation fails then no vports of this type can be created.
  * @exit: Called at module unload.
- * @create: Create a new vport called 'name' with vport type specific
- * configuration 'config' (which must be copied from userspace before use).  On
- * success must allocate a new vport using vport_alloc().
+ * @create: Create a new vport configured as specified.  On success returns
+ * a new vport allocated with vport_alloc(), otherwise an ERR_PTR() value.
  * @modify: Modify the configuration of an existing vport.  May be null if
  * modification is not supported.
  * @destroy: Destroy and free a vport using vport_free().  Prior to destruction
@@ -168,7 +182,7 @@ struct vport_ops {
 	void (*exit)(void);
 
 	/* Called with RTNL lock. */
-	struct vport *(*create)(const char *name, const void __user *config);
+	struct vport *(*create)(const struct vport_parms *);
 	int (*modify)(struct vport *, const void __user *config);
 	int (*destroy)(struct vport *);
 
-- 
1.7.1





More information about the dev mailing list