[ovs-dev] [netlink v3 5/5] datapath: Merge vport "attach" into "create" and "detach" into "destroy".
Ben Pfaff
blp at nicira.com
Wed Nov 17 01:11:26 UTC 2010
These steps are sequentially in lockstep, so we might as well combine them.
This expands the region over which the vport_lock is held. I didn't
carefully verify that this was OK.
This also eliminates the synchronize_rcu() call from destruction of tunnel
vports, since they didn't appear to me to need it.
It should be possible to eliminate the synchronize_rcu() from the netdev,
patch, and internal_dev vports, but this commit does not do that.
Signed-off-by: Ben Pfaff <blp at nicira.com>
---
datapath/datapath.c | 19 +---------------
datapath/vport-internal_dev.c | 33 ++++++++---------------------
datapath/vport-netdev.c | 43 ++++++++++++-------------------------
datapath/vport-patch.c | 40 ++++++++++++-----------------------
datapath/vport.c | 46 ++++------------------------------------
datapath/vport.h | 13 +----------
6 files changed, 45 insertions(+), 149 deletions(-)
diff --git a/datapath/datapath.c b/datapath/datapath.c
index a3e7872..6c8177c 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -339,7 +339,6 @@ static int new_vport(struct datapath *dp, struct odp_port *odp_port, int port_no
{
struct vport_parms parms;
struct vport *vport;
- int err;
parms.name = odp_port->devname;
parms.type = odp_port->type;
@@ -354,12 +353,6 @@ static int new_vport(struct datapath *dp, struct odp_port *odp_port, int port_no
if (IS_ERR(vport))
return PTR_ERR(vport);
- err = vport_attach(vport);
- if (err) {
- vport_del(vport);
- return err;
- }
-
rcu_assign_pointer(dp->ports[port_no], vport);
list_add_rcu(&vport->node, &dp->port_list);
dp->n_ports++;
@@ -414,8 +407,6 @@ out:
int dp_detach_port(struct vport *p)
{
- int err;
-
ASSERT_RTNL();
if (p->port_no != ODPP_LOCAL)
@@ -427,15 +418,9 @@ int dp_detach_port(struct vport *p)
list_del_rcu(&p->node);
rcu_assign_pointer(p->dp->ports[p->port_no], NULL);
- err = vport_detach(p);
- if (err)
- return err;
-
- /* Then wait until no one is still using it, and destroy it. */
- synchronize_rcu();
-
+ /* Then destroy it. */
vport_lock();
- vport_del(p);
+ vport_del(p); /* XXX handle error */
vport_unlock();
return 0;
diff --git a/datapath/vport-internal_dev.c b/datapath/vport-internal_dev.c
index f6348be..09bd55b 100644
--- a/datapath/vport-internal_dev.c
+++ b/datapath/vport-internal_dev.c
@@ -214,6 +214,10 @@ static struct vport *internal_dev_create(const struct vport_parms *parms)
if (err)
goto error_free_netdev;
+ rcu_assign_pointer(internal_dev->attached_vport, internal_dev->vport);
+ dev_set_promiscuity(netdev_vport->dev, 1);
+ netif_start_queue(netdev_vport->dev);
+
return vport;
error_free_netdev:
@@ -227,34 +231,17 @@ error:
static int internal_dev_destroy(struct vport *vport)
{
struct netdev_vport *netdev_vport = netdev_vport_priv(vport);
-
- unregister_netdevice(netdev_vport->dev);
- vport_free(vport);
-
- return 0;
-}
-
-static int internal_dev_attach(struct vport *vport)
-{
- struct netdev_vport *netdev_vport = netdev_vport_priv(vport);
- struct internal_dev *internal_dev = internal_dev_priv(netdev_vport->dev);
-
- rcu_assign_pointer(internal_dev->attached_vport, internal_dev->vport);
- dev_set_promiscuity(netdev_vport->dev, 1);
- netif_start_queue(netdev_vport->dev);
-
- return 0;
-}
-
-static int internal_dev_detach(struct vport *vport)
-{
- struct netdev_vport *netdev_vport = netdev_vport_priv(vport);
struct internal_dev *internal_dev = internal_dev_priv(netdev_vport->dev);
netif_stop_queue(netdev_vport->dev);
dev_set_promiscuity(netdev_vport->dev, -1);
rcu_assign_pointer(internal_dev->attached_vport, NULL);
+ synchronize_rcu();
+
+ unregister_netdevice(netdev_vport->dev);
+ vport_free(vport);
+
return 0;
}
@@ -282,8 +269,6 @@ struct vport_ops internal_vport_ops = {
.flags = VPORT_F_REQUIRED | VPORT_F_GEN_STATS | VPORT_F_FLOW,
.create = internal_dev_create,
.destroy = internal_dev_destroy,
- .attach = internal_dev_attach,
- .detach = internal_dev_detach,
.set_mtu = netdev_set_mtu,
.set_addr = netdev_set_addr,
.get_name = netdev_get_name,
diff --git a/datapath/vport-netdev.c b/datapath/vport-netdev.c
index d4536a4..a5096df 100644
--- a/datapath/vport-netdev.c
+++ b/datapath/vport-netdev.c
@@ -125,6 +125,15 @@ static struct vport *netdev_create(const struct vport_parms *parms)
vport_set_stats(vport, &stats);
}
+ err = netdev_rx_handler_register(netdev_vport->dev, netdev_frame_hook,
+ vport);
+ if (err)
+ goto error_put;
+
+ dev_set_promiscuity(netdev_vport->dev, 1);
+ dev_disable_lro(netdev_vport->dev);
+ netdev_vport->dev->priv_flags |= IFF_OVS_DATAPATH;
+
return vport;
error_put:
@@ -139,37 +148,15 @@ static int netdev_destroy(struct vport *vport)
{
struct netdev_vport *netdev_vport = netdev_vport_priv(vport);
- dev_put(netdev_vport->dev);
- vport_free(vport);
-
- return 0;
-}
-
-static int netdev_attach(struct vport *vport)
-{
- struct netdev_vport *netdev_vport = netdev_vport_priv(vport);
- int err;
-
- err = netdev_rx_handler_register(netdev_vport->dev, netdev_frame_hook,
- vport);
- if (err)
- return err;
-
- dev_set_promiscuity(netdev_vport->dev, 1);
- dev_disable_lro(netdev_vport->dev);
- netdev_vport->dev->priv_flags |= IFF_OVS_DATAPATH;
-
- return 0;
-}
-
-static int netdev_detach(struct vport *vport)
-{
- struct netdev_vport *netdev_vport = netdev_vport_priv(vport);
-
netdev_vport->dev->priv_flags &= ~IFF_OVS_DATAPATH;
netdev_rx_handler_unregister(netdev_vport->dev);
dev_set_promiscuity(netdev_vport->dev, -1);
+ synchronize_rcu();
+
+ dev_put(netdev_vport->dev);
+ vport_free(vport);
+
return 0;
}
@@ -309,8 +296,6 @@ struct vport_ops netdev_vport_ops = {
.exit = netdev_exit,
.create = netdev_create,
.destroy = netdev_destroy,
- .attach = netdev_attach,
- .detach = netdev_detach,
.set_mtu = netdev_set_mtu,
.set_addr = netdev_set_addr,
.get_name = netdev_get_name,
diff --git a/datapath/vport-patch.c b/datapath/vport-patch.c
index 52d3da1..a0dd36d 100644
--- a/datapath/vport-patch.c
+++ b/datapath/vport-patch.c
@@ -39,6 +39,8 @@ struct patch_vport {
static struct hlist_head *peer_table;
#define PEER_HASH_BUCKETS 256
+static void update_peers(const char *name, struct vport *);
+
static inline struct patch_vport *patch_vport_priv(const struct vport *vport)
{
return vport_priv(vport);
@@ -130,6 +132,11 @@ static struct vport *patch_create(const struct vport_parms *parms)
* bottleneck on systems using jumbo frames. */
patch_vport->devconf->mtu = 65535;
+ hlist_add_head(&patch_vport->hash_node, hash_bucket(patch_vport->peer_name));
+
+ rcu_assign_pointer(patch_vport->peer, vport_locate(patch_vport->peer_name));
+ update_peers(patch_vport->name, vport);
+
return vport;
error_free_vport:
@@ -155,6 +162,13 @@ static int patch_destroy(struct vport *vport)
{
struct patch_vport *patch_vport = patch_vport_priv(vport);
+ update_peers(patch_vport->name, NULL);
+ rcu_assign_pointer(patch_vport->peer, NULL);
+
+ hlist_del(&patch_vport->hash_node);
+
+ synchronize_rcu();
+
kfree(patch_vport->devconf);
vport_free(vport);
@@ -172,30 +186,6 @@ static void update_peers(const char *name, struct vport *vport)
rcu_assign_pointer(peer_vport->peer, vport);
}
-static int patch_attach(struct vport *vport)
-{
- struct patch_vport *patch_vport = patch_vport_priv(vport);
-
- hlist_add_head(&patch_vport->hash_node, hash_bucket(patch_vport->peer_name));
-
- rcu_assign_pointer(patch_vport->peer, vport_locate(patch_vport->peer_name));
- update_peers(patch_vport->name, vport);
-
- return 0;
-}
-
-static int patch_detach(struct vport *vport)
-{
- struct patch_vport *patch_vport = patch_vport_priv(vport);
-
- update_peers(patch_vport->name, NULL);
- rcu_assign_pointer(patch_vport->peer, NULL);
-
- hlist_del(&patch_vport->hash_node);
-
- return 0;
-}
-
static int patch_set_mtu(struct vport *vport, int mtu)
{
struct patch_vport *patch_vport = patch_vport_priv(vport);
@@ -270,8 +260,6 @@ struct vport_ops patch_vport_ops = {
.create = patch_create,
.modify = patch_modify,
.destroy = patch_destroy,
- .attach = patch_attach,
- .detach = patch_detach,
.set_mtu = patch_set_mtu,
.set_addr = patch_set_addr,
.get_name = patch_get_name,
diff --git a/datapath/vport.c b/datapath/vport.c
index b9334ee..cddb891 100644
--- a/datapath/vport.c
+++ b/datapath/vport.c
@@ -587,8 +587,9 @@ void vport_free(struct vport *vport)
*
* @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.
+ * Creates a new vport with the specified configuration (which is dependent on
+ * device type) and attaches it to a datapath. Both RTNL and vport locks must
+ * be held.
*/
struct vport *vport_add(const struct vport_parms *parms)
{
@@ -643,9 +644,8 @@ int vport_mod(struct vport *vport, struct odp_port *port)
*
* @vport: vport to delete.
*
- * Deletes the specified device. The device must not be currently attached to
- * a datapath. It is possible to fail for reasons such as lack of memory.
- * Both RTNL and vport locks must be held.
+ * Detaches @vport from its datapath and destroys it. It is possible to fail
+ * for reasons such as lack of memory. Both RTNL and vport locks must be held.
*/
int vport_del(struct vport *vport)
{
@@ -658,42 +658,6 @@ int vport_del(struct vport *vport)
}
/**
- * vport_attach - notify a vport that it has been attached to a datapath
- *
- * @vport: vport to attach.
- *
- * Performs vport-specific actions so that packets may be exchanged. RTNL lock
- * and the appropriate DP mutex must be held.
- */
-int vport_attach(struct vport *vport)
-{
- ASSERT_RTNL();
-
- if (vport->ops->attach)
- return vport->ops->attach(vport);
-
- return 0;
-}
-
-/**
- * vport_detach - detach a vport from a datapath
- *
- * @vport: vport to detach.
- *
- * Performs vport-specific actions before a vport is detached from a datapath.
- * May fail for a variety of reasons, including lack of memory. RTNL lock and
- * the appropriate DP mutex must be held.
- */
-int vport_detach(struct vport *vport)
-{
- ASSERT_RTNL();
-
- if (vport->ops->detach)
- return vport->ops->detach(vport);
- return 0;
-}
-
-/**
* vport_set_mtu - set device MTU (for kernel callers)
*
* @vport: vport on which to set MTU.
diff --git a/datapath/vport.h b/datapath/vport.h
index 13772c7..f53291d 100644
--- a/datapath/vport.h
+++ b/datapath/vport.h
@@ -45,9 +45,6 @@ int vport_del(struct vport *);
struct vport *vport_locate(const char *name);
-int vport_attach(struct vport *);
-int vport_detach(struct vport *);
-
int vport_set_mtu(struct vport *, int mtu);
int vport_set_addr(struct vport *, const unsigned char *);
int vport_set_stats(struct vport *, struct rtnl_link_stats64 *);
@@ -170,12 +167,7 @@ struct vport_parms {
* 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
- * @detach will be called followed by synchronize_rcu().
- * @attach: Attach a previously created vport to a datapath. After attachment
- * packets may be sent and received. Prior to attachment any packets may be
- * silently discarded. May be null if not needed.
- * @detach: Detach a vport from a datapath. May be null if not needed.
+ * @destroy: Detach and destroy a vport.
* @set_mtu: Set the device's MTU. May be null if not supported.
* @set_addr: Set the device's MAC address. May be null if not supported.
* @set_stats: Provides stats as an offset to be added to the device stats.
@@ -211,9 +203,6 @@ struct vport_ops {
int (*modify)(struct vport *, struct odp_port *);
int (*destroy)(struct vport *);
- int (*attach)(struct vport *);
- int (*detach)(struct vport *);
-
int (*set_mtu)(struct vport *, int mtu);
int (*set_addr)(struct vport *, const unsigned char *);
int (*set_stats)(const struct vport *, struct rtnl_link_stats64 *);
--
1.7.1
More information about the dev
mailing list