Skip to content

Commit

Permalink
datapath: Make adding and attaching a vport a single step.
Browse files Browse the repository at this point in the history 
For some time now, Open vSwitch datapaths have internally made a
distinction between adding a vport and attaching it to a datapath.  Adding
a vport just means to create it, as an entity detached from any datapath.
Attaching it gives it a port number and a datapath.  Similarly, a vport
could be detached and deleted separately.

After some study, I think I understand why this distinction exists.  It is
because ovs-vswitchd tries to open all the datapath ports before it tries
to create them.  However, changing it to create them before it tries to
open them is not difficult, so this commit does this.

The bulk of this commit, however, changes the datapath interface to one
that always creates a vport and attaches it to a datapath in a single step,
and similarly detaches a vport and deletes it in a single step.

Signed-off-by: Ben Pfaff <[email protected]>
Acked-by: Jesse Gross <[email protected]>
  • Loading branch information
@blp
blp committed Dec 3, 2010
1 parent 94903c9 commit c3827f6
Show file tree
Hide file tree
Showing 26 changed files with 501 additions and 773 deletions.
83 changes: 30 additions & 53 deletions datapath/datapath.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,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);
internal_dev_port.flags = ODP_PORT_INTERNAL;
strcpy(internal_dev_port.type, "internal");
err = new_dp_port(dp, &internal_dev_port, ODPP_LOCAL);
if (err) {
if (err == -EBUSY)
Expand All @@ -275,7 +275,7 @@ static int create_dp(int dp_idx, const char __user *devnamep)
return 0;

err_destroy_local_port:
dp_detach_port(dp->ports[ODPP_LOCAL], 1);
dp_detach_port(dp->ports[ODPP_LOCAL]);
err_destroy_table:
tbl_destroy(dp->table, NULL);
err_free_dp:
Expand All @@ -296,13 +296,13 @@ static void do_destroy_dp(struct datapath *dp)

list_for_each_entry_safe (p, n, &dp->port_list, node)
if (p->port_no != ODPP_LOCAL)
dp_detach_port(p, 1);
dp_detach_port(p);

dp_sysfs_del_dp(dp);

rcu_assign_pointer(dps[dp->dp_idx], NULL);

dp_detach_port(dp->ports[ODPP_LOCAL], 1);
dp_detach_port(dp->ports[ODPP_LOCAL]);

tbl_destroy(dp->table, flow_free_tbl);

Expand Down Expand Up @@ -350,25 +350,21 @@ static struct kobj_type brport_ktype = {
/* Called with RTNL lock and dp_mutex. */
static int new_dp_port(struct datapath *dp, struct odp_port *odp_port, int port_no)
{
struct vport_parms parms;
struct vport *vport;
struct dp_port *p;
int err;

vport = vport_locate(odp_port->devname);
if (!vport) {
struct vport_parms parms;
parms.name = odp_port->devname;
parms.type = odp_port->type;
parms.config = odp_port->config;

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();

vport_lock();
vport = vport_add(&parms);
vport_unlock();

if (IS_ERR(vport))
return PTR_ERR(vport);
}
if (IS_ERR(vport))
return PTR_ERR(vport);

p = kzalloc(sizeof(*p), GFP_KERNEL);
if (!p)
Expand Down Expand Up @@ -410,6 +406,7 @@ 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);
Expand Down Expand Up @@ -441,7 +438,7 @@ static int attach_port(int dp_idx, struct odp_port __user *portp)
return err;
}

int dp_detach_port(struct dp_port *p, int may_delete)
int dp_detach_port(struct dp_port *p)
{
struct vport *vport = p->vport;
int err;
Expand All @@ -464,15 +461,9 @@ int dp_detach_port(struct dp_port *p, int may_delete)
/* Then wait until no one is still using it, and destroy it. */
synchronize_rcu();

if (may_delete) {
const char *port_type = vport_get_type(vport);

if (!strcmp(port_type, "netdev") || !strcmp(port_type, "internal")) {
vport_lock();
vport_del(vport);
vport_unlock();
}
}
vport_lock();
vport_del(vport);
vport_unlock();

kobject_put(&p->kobj);

Expand Down Expand Up @@ -500,7 +491,7 @@ static int detach_port(int dp_idx, int port_no)
if (!p)
goto out_unlock_dp;

err = dp_detach_port(p, 1);
err = dp_detach_port(p);

out_unlock_dp:
mutex_unlock(&dp->mutex);
Expand Down Expand Up @@ -1437,10 +1428,10 @@ static int put_port(const struct dp_port *p, struct odp_port __user *uop)

rcu_read_lock();
strncpy(op.devname, vport_get_name(p->vport), sizeof op.devname);
strncpy(op.type, vport_get_type(p->vport), sizeof op.type);
rcu_read_unlock();

op.port = p->port_no;
op.flags = is_internal_vport(p->vport) ? ODP_PORT_INTERNAL : 0;

return copy_to_user(uop, &op, sizeof op) ? -EFAULT : 0;
}
Expand Down Expand Up @@ -1553,26 +1544,18 @@ static long openvswitch_ioctl(struct file *f, unsigned int cmd,
err = destroy_dp(dp_idx);
goto exit;

case ODP_PORT_ATTACH:
case ODP_VPORT_ATTACH:
err = attach_port(dp_idx, (struct odp_port __user *)argp);
goto exit;

case ODP_PORT_DETACH:
case ODP_VPORT_DETACH:
err = get_user(port_no, (int __user *)argp);
if (!err)
err = detach_port(dp_idx, port_no);
goto exit;

case ODP_VPORT_ADD:
err = vport_user_add((struct odp_vport_add __user *)argp);
goto exit;

case ODP_VPORT_MOD:
err = vport_user_mod((struct odp_vport_mod __user *)argp);
goto exit;

case ODP_VPORT_DEL:
err = vport_user_del((char __user *)argp);
err = vport_user_mod((struct odp_port __user *)argp);
goto exit;

case ODP_VPORT_STATS_GET:
Expand Down Expand Up @@ -1650,11 +1633,11 @@ static long openvswitch_ioctl(struct file *f, unsigned int cmd,
dp->sflow_probability = sflow_probability;
break;

case ODP_PORT_QUERY:
case ODP_VPORT_QUERY:
err = query_port(dp, (struct odp_port __user *)argp);
break;

case ODP_PORT_LIST:
case ODP_VPORT_LIST:
err = list_ports(dp, (struct odp_portvec __user *)argp);
break;

Expand Down Expand Up @@ -1910,9 +1893,9 @@ static long openvswitch_compat_ioctl(struct file *f, unsigned int cmd, unsigned
return openvswitch_ioctl(f, cmd, argp);

case ODP_DP_CREATE:
case ODP_PORT_ATTACH:
case ODP_PORT_DETACH:
case ODP_VPORT_DEL:
case ODP_VPORT_ATTACH:
case ODP_VPORT_DETACH:
case ODP_VPORT_MOD:
case ODP_VPORT_MTU_SET:
case ODP_VPORT_MTU_GET:
case ODP_VPORT_ETHER_SET:
Expand All @@ -1926,15 +1909,9 @@ static long openvswitch_compat_ioctl(struct file *f, unsigned int cmd, unsigned
case ODP_GET_LISTEN_MASK:
case ODP_SET_SFLOW_PROBABILITY:
case ODP_GET_SFLOW_PROBABILITY:
case ODP_PORT_QUERY:
case ODP_VPORT_QUERY:
/* Ioctls that just need their pointer argument extended. */
return openvswitch_ioctl(f, cmd, (unsigned long)compat_ptr(argp));

case ODP_VPORT_ADD32:
return compat_vport_user_add(compat_ptr(argp));

case ODP_VPORT_MOD32:
return compat_vport_user_mod(compat_ptr(argp));
}

dp = get_dp_locked(dp_idx);
Expand All @@ -1943,7 +1920,7 @@ static long openvswitch_compat_ioctl(struct file *f, unsigned int cmd, unsigned
goto exit;

switch (cmd) {
case ODP_PORT_LIST32:
case ODP_VPORT_LIST32:
err = compat_list_ports(dp, compat_ptr(argp));
break;

Expand Down
2 changes: 1 addition & 1 deletion datapath/datapath.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ extern struct notifier_block dp_device_notifier;
extern int (*dp_ioctl_hook)(struct net_device *dev, struct ifreq *rq, int cmd);

void dp_process_received_packet(struct dp_port *, struct sk_buff *);
int dp_detach_port(struct dp_port *, int may_delete);
int dp_detach_port(struct dp_port *);
int dp_output_control(struct datapath *, struct sk_buff *, int, u32 arg);
int dp_min_mtu(const struct datapath *dp);
void set_internal_devs_mtu(const struct datapath *dp);
Expand Down
2 changes: 1 addition & 1 deletion datapath/dp_notify.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ static int dp_device_event(struct notifier_block *unused, unsigned long event,
switch (event) {
case NETDEV_UNREGISTER:
mutex_lock(&dp->mutex);
dp_detach_port(p, 1);
dp_detach_port(p);
mutex_unlock(&dp->mutex);
break;

Expand Down
13 changes: 0 additions & 13 deletions datapath/odp-compat.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@
#define ODP_FLOW_DEL32 _IOWR('O', 17, struct compat_odp_flow)
#define ODP_EXECUTE32 _IOR('O', 18, struct compat_odp_execute)
#define ODP_FLOW_DEL32 _IOWR('O', 17, struct compat_odp_flow)
#define ODP_VPORT_ADD32 _IOR('O', 21, struct compat_odp_vport_add)
#define ODP_VPORT_MOD32 _IOR('O', 22, struct compat_odp_vport_mod)

struct compat_odp_portvec {
compat_uptr_t ports;
Expand Down Expand Up @@ -59,17 +57,6 @@ struct compat_odp_execute {
compat_uptr_t data;
u32 length;
};

struct compat_odp_vport_add {
char port_type[VPORT_TYPE_SIZE];
char devname[16]; /* IFNAMSIZ */
compat_uptr_t config;
};

struct compat_odp_vport_mod {
char devname[16]; /* IFNAMSIZ */
compat_uptr_t config;
};
#endif /* CONFIG_COMPAT */

#endif /* odp-compat.h */
9 changes: 4 additions & 5 deletions datapath/tunnel.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1300,15 +1300,14 @@ int tnl_send(struct vport *vport, struct sk_buff *skb)
return sent_len;
}

static int set_config(const void __user *uconfig, const struct tnl_ops *tnl_ops,
static int set_config(const void *config, const struct tnl_ops *tnl_ops,
const struct vport *cur_vport,
struct tnl_mutable_config *mutable)
{
const struct vport *old_vport;
const struct tnl_mutable_config *old_mutable;

if (copy_from_user(&mutable->port_config, uconfig, sizeof(struct tnl_port_config)))
return -EFAULT;
mutable->port_config = *(struct tnl_port_config *)config;

if (mutable->port_config.daddr == 0)
return -EINVAL;
Expand Down Expand Up @@ -1401,7 +1400,7 @@ struct vport *tnl_create(const struct vport_parms *parms,
return ERR_PTR(err);
}

int tnl_modify(struct vport *vport, const void __user *config)
int tnl_modify(struct vport *vport, struct odp_port *port)
{
struct tnl_vport *tnl_vport = tnl_vport_priv(vport);
struct tnl_mutable_config *mutable;
Expand All @@ -1413,7 +1412,7 @@ int tnl_modify(struct vport *vport, const void __user *config)
goto error;
}

err = set_config(config, tnl_vport->tnl_ops, vport, mutable);
err = set_config(port->config, tnl_vport->tnl_ops, vport, mutable);
if (err)
goto error_free;

Expand Down
2 changes: 1 addition & 1 deletion datapath/tunnel.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ struct tnl_vport {

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_modify(struct vport *, struct odp_port *);
int tnl_destroy(struct vport *);
int tnl_set_mtu(struct vport *vport, int mtu);
int tnl_set_addr(struct vport *vport, const unsigned char *addr);
Expand Down
13 changes: 4 additions & 9 deletions datapath/vport-patch.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,17 +83,12 @@ static void patch_exit(void)
kfree(peer_table);
}

static int set_config(struct vport *vport, const void __user *uconfig)
static int set_config(struct vport *vport, const void *config)
{
struct patch_vport *patch_vport = patch_vport_priv(vport);
char peer_name[IFNAMSIZ];
int retval;

retval = strncpy_from_user(peer_name, uconfig, IFNAMSIZ);
if (retval < 0)
return -EFAULT;
else if (retval >= IFNAMSIZ)
return -ENAMETOOLONG;
strlcpy(peer_name, config, IFNAMSIZ);

if (!strcmp(patch_vport->name, peer_name))
return -EINVAL;
Expand Down Expand Up @@ -149,9 +144,9 @@ static struct vport *patch_create(const struct vport_parms *parms)
return ERR_PTR(err);
}

static int patch_modify(struct vport *vport, const void __user *config)
static int patch_modify(struct vport *vport, struct odp_port *port)
{
return set_config(vport, config);
return set_config(vport, port->config);
}

static int patch_destroy(struct vport *vport)
Expand Down
Loading

0 comments on commit c3827f6

Please sign in to comment.