Messages in this thread Patch in this message | | | Date | Mon, 17 Dec 2012 01:46:45 -0500 (EST) | From | Jason Wang <> | Subject | Re: [PATCH] tuntap: fix ambigious multiqueue API |
| |
----- Original Message ----- > The current multiqueue API is ambigious which may confuse both user > and LSM to > do things correctly: > > - Both TUNSETIFF and TUNSETQUEUE could be used to create the queues > of a tuntap > device. > - TUNSETQUEUE were used to disable and enable a specific queue of the > device. But since the state of tuntap were completely removed from > the queue, > it could be used to attach to another device (there's no such kind > of > requirement currently, and it needs new kind of LSM policy. > - TUNSETQUEUE could be used to attach to a persistent device without > any > queues. This kind of attching bypass the necessary checking during > TUNSETIFF > and may lead unexpected result. > > So this patch tries to make a cleaner and simpler API by: > > - Only allow TUNSETIFF to create queues. > - TUNSETQUEUE could be only used to disable and enabled the queues of > a device, > and the state of the tuntap device were not detachd from the queues > when it > was disabled, so TUNSETQUEUE could be only used after TUNSETIFF and > with the > same device. > > This is done by introducing a list which keeps track of all queues > which were > disabled. The queue would be moved between this list and tfiles[] > array when it > was enabled/disabled. A pointer of the tun_struct were also introdued > to track > the device it belongs to when it was disabled. > > After the change, the isolation between management and application > could be done > through: TUNSETIFF were only called by management software and > TUNSETQUEUE were > only called by application.For LSM/SELinux, the things left is to do > proper > check during tun_set_queue() if needed. > > Signed-off-by: Jason Wang <jasowang@redhat.com> > --- > drivers/net/tun.c | 86 > ++++++++++++++++++++++++++++++++++++++-------------- > 1 files changed, 63 insertions(+), 23 deletions(-) > > diff --git a/drivers/net/tun.c b/drivers/net/tun.c > index 2ac2164..6f2053d 100644 > --- a/drivers/net/tun.c > +++ b/drivers/net/tun.c > @@ -138,6 +138,8 @@ struct tun_file { > /* only used for fasnyc */ > unsigned int flags; > u16 queue_index; > + struct list_head next; > + struct tun_struct *detached; > }; > > struct tun_flow_entry { > @@ -182,6 +184,8 @@ struct tun_struct { > struct hlist_head flows[TUN_NUM_FLOW_ENTRIES]; > struct timer_list flow_gc_timer; > unsigned long ageing_time; > + unsigned int numdisabled; > + struct list_head disabled; > }; > > static inline u32 tun_hashfn(u32 rxhash) > @@ -386,6 +390,23 @@ static void tun_set_real_num_queues(struct > tun_struct *tun) > netif_set_real_num_rx_queues(tun->dev, tun->numqueues); > } > > +static void tun_disable_queue(struct tun_struct *tun, struct > tun_file *tfile) > +{ > + tfile->detached = tun; > + list_add_tail(&tfile->next, &tun->disabled); > + ++tun->numdisabled; > +} > + > +struct tun_struct *tun_enable_queue(struct tun_file *tfile) > +{ > + struct tun_struct *tun = tfile->detached; > + > + tfile->detached = NULL; > + list_del_init(&tfile->next); > + --tun->numdisabled; > + return tun; > +} > + > static void __tun_detach(struct tun_file *tfile, bool clean) > { > struct tun_file *ntfile; > @@ -407,20 +428,25 @@ static void __tun_detach(struct tun_file > *tfile, bool clean) > ntfile->queue_index = index; > > --tun->numqueues; > - sock_put(&tfile->sk); > + if (clean) > + sock_put(&tfile->sk); > + else > + tun_disable_queue(tun, tfile); > > synchronize_net(); > tun_flow_delete_by_queue(tun, tun->numqueues + 1); > /* Drop read queue */ > skb_queue_purge(&tfile->sk.sk_receive_queue); > tun_set_real_num_queues(tun); > - > - if (tun->numqueues == 0 && !(tun->flags & TUN_PERSIST)) > - if (dev->reg_state == NETREG_REGISTERED) > - unregister_netdevice(dev); > - } > + } else if (tfile->detached && clean) > + tun = tun_enable_queue(tfile); > > if (clean) { > + if (tun && tun->numqueues == 0 && tun->numdisabled == 0 && > + !(tun->flags & TUN_PERSIST)) > + if (tun->dev->reg_state == NETREG_REGISTERED) > + unregister_netdevice(tun->dev); > + > BUG_ON(!test_bit(SOCK_EXTERNALLY_ALLOCATED, > &tfile->socket.flags)); > sk_release_kernel(&tfile->sk); > @@ -437,7 +463,7 @@ static void tun_detach(struct tun_file *tfile, > bool clean) > static void tun_detach_all(struct net_device *dev) > { > struct tun_struct *tun = netdev_priv(dev); > - struct tun_file *tfile; > + struct tun_file *tfile, *tmp; > int i, n = tun->numqueues; > > for (i = 0; i < n; i++) { > @@ -458,6 +484,12 @@ static void tun_detach_all(struct net_device > *dev) > skb_queue_purge(&tfile->sk.sk_receive_queue); > sock_put(&tfile->sk); > } > + list_for_each_entry_safe(tfile, tmp, &tun->disabled, next) { > + tun_enable_queue(tfile); > + skb_queue_purge(&tfile->sk.sk_receive_queue); > + sock_put(&tfile->sk); > + } > + BUG_ON(tun->numdisabled != 0); > } > > static int tun_attach(struct tun_struct *tun, struct file *file) > @@ -474,7 +506,8 @@ static int tun_attach(struct tun_struct *tun, > struct file *file) > goto out; > > err = -E2BIG; > - if (tun->numqueues == MAX_TAP_QUEUES) > + if (!tfile->detached && > + tun->numqueues + tun->numdisabled == MAX_TAP_QUEUES) > goto out; > > err = 0; > @@ -488,9 +521,13 @@ static int tun_attach(struct tun_struct *tun, > struct file *file) > tfile->queue_index = tun->numqueues; > rcu_assign_pointer(tfile->tun, tun); > rcu_assign_pointer(tun->tfiles[tun->numqueues], tfile); > - sock_hold(&tfile->sk); > tun->numqueues++; > > + if (tfile->detached) > + tun_enable_queue(tfile); > + else > + sock_hold(&tfile->sk); > + > tun_set_real_num_queues(tun); > > /* device is allowed to go away first, so no need to hold extra > @@ -1348,6 +1385,7 @@ static void tun_free_netdev(struct net_device > *dev) > { > struct tun_struct *tun = netdev_priv(dev); > > + BUG_ON(!(list_empty(&tun->disabled))); > tun_flow_uninit(tun); > free_netdev(dev); > } > @@ -1542,6 +1580,10 @@ static int tun_set_iff(struct net *net, struct > file *file, struct ifreq *ifr) > err = tun_attach(tun, file); > if (err < 0) > return err; > + > + if (tun->flags & TUN_TAP_MQ && > + (tun->numqueues + tun->numdisabled > 1)) > + return err; > } > else { > char *name; > @@ -1600,6 +1642,7 @@ static int tun_set_iff(struct net *net, struct > file *file, struct ifreq *ifr) > TUN_USER_FEATURES; > dev->features = dev->hw_features; > > + INIT_LIST_HEAD(&tun->disabled); > err = tun_attach(tun, file); > if (err < 0) > goto err_free_dev; > @@ -1754,32 +1797,28 @@ static int tun_set_queue(struct file *file, > struct ifreq *ifr) > { > struct tun_file *tfile = file->private_data; > struct tun_struct *tun; > - struct net_device *dev; > int ret = 0; > > rtnl_lock(); > > if (ifr->ifr_flags & IFF_ATTACH_QUEUE) { > - dev = __dev_get_by_name(tfile->net, ifr->ifr_name); > - if (!dev) { > - ret = -EINVAL; > - goto unlock; > - } > - > - tun = netdev_priv(dev); > - if (dev->netdev_ops != &tap_netdev_ops && > - dev->netdev_ops != &tun_netdev_ops) > + tun = tfile->detached; > + if (!tun) > ret = -EINVAL; > else if (tun_not_capable(tun)) > ret = -EPERM; > else > ret = tun_attach(tun, file); > - } else if (ifr->ifr_flags & IFF_DETACH_QUEUE) > - __tun_detach(tfile, false); > - else > + } else if (ifr->ifr_flags & IFF_DETACH_QUEUE) { > + tun = rcu_dereference_protected(tfile->tun, > + lockdep_rtnl_is_held()); > + if (!tun || !(tun->flags & TUN_TAP_MQ)) > + ret = -EINVAL; > + else > + __tun_detach(tfile, false); > + } else > ret = -EINVAL; > > -unlock: > rtnl_unlock(); > return ret; > } > @@ -2091,6 +2130,7 @@ static int tun_chr_open(struct inode *inode, > struct file * file) > > file->private_data = tfile; > set_bit(SOCK_EXTERNALLY_ALLOCATED, &tfile->socket.flags); > + INIT_LIST_HEAD(&tfile->next); > > return 0; > } > -- > 1.7.1 > > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
| |