High-speed tun receive and xmit
Subsystems (7)
cover High-speed tun receive and xmit
kvm (and lguest!) want to get more speed out of the tun device. We already have an ABI for guest<->host comms, called virtio_ring; extending tun to understand this (with its async nature and batching) make for an efficient network.
But moreover: the same things that make virtio a good guest<->host transport make it appealing as a userspace<->kernel transport. So rather than do something just for tun, we create a /dev/vring. We could do a system call, but /dev/vring is sufficient. It is a nice, simple format for normal tun usage too.
We do need a small (backwards-compatible) extension to the ring format if we're going to do this however, since we didn't previously expose the consumer's view of the producer (and vice verse) in the ring because the other end doesn't need to see it. However, it means that a middleman (as the kvm or lguest launcher process now becomes) cannot save and restore all the ring state.
Cheers,
Rusty.
PATCH 1/5 virtio: put last_used and last_avail index into ring itself.
Generally, the other end of the virtio ring doesn't need to see where you're up to in consuming the ring. However, in order for an external entity to understand it, it must be exposed. For example, if you want to save and restore a virtio_ring, but you're not the consumer because the kernel is using it directly.
Fortunately, we have room to expand: the ring is always a whole number of pages and there's hundreds of bytes of padding after the avail ring and the used ring, whatever the number of descriptors (which must be a power of 2).
We add a feature bit so the guest can tell the host that it's writing out the current value there, if it wants to use that.
--- drivers/virtio/virtio_ring.c | 20 ++++++++++++-------- include/linux/virtio_config.h | 3 +++ include/linux/virtio_ring.h | 12 +++++++++++- 3 files changed, 26 insertions(+), 9 deletions(-)
drivers/virtio/virtio_ring.c +12 -8
@@ -18,6 +18,7 @@*/ #include <linux/virtio.h> #include <linux/virtio_ring.h> +#include <linux/virtio_config.h> #include <linux/device.h> #ifdef DEBUG
@@ -51,9 +52,6 @@ struct vring_virtqueueunsigned int free_head;/* Number we've added since last sync. */ unsigned int num_added; - -/* Last used index we've seen. */ -u16 last_used_idx;/* How to notify other side. FIXME: commonalize hcalls! */ void (*notify)(struct virtqueue *vq);
@@ -173,12 +171,13 @@ static void detach_buf(struct vring_virtstatic inline bool more_used (const struct vring_virtqueue *vq) { -return vq->last_used_idx != vq->vring.used->idx; +return vring_last_used (&vq->vring) != vq->vring.used->idx; }static void *vring_get_buf (struct virtqueue *_vq,unsigned int *len) {struct vring_virtqueue *vq =to_vvq (_vq); +struct vring_used_elem *u;void *ret;unsigned int i;
@@ -195,8 +194,11 @@ static void *vring_get_buf(struct virtqureturn NULL ; } - i = vq->vring.used->ring[vq->last_used_idx%vq->vring.num].id; - *len = vq->vring.used->ring[vq->last_used_idx%vq->vring.num].len; + u = &vq->vring.used->ring[vring_last_used (&vq->vring) % vq->vring.num]; + i = u->id; + *len = u->len; +/* Make sure we don't reload i after doing checks. */ +rmb ();if (unlikely (i >= vq->vring.num)) {BAD_RING (vq,"id %u out of range\n" , i);
@@ -210,7 +212,7 @@ static void *vring_get_buf(struct virtqu/* detach_buf clears data, so grab it now. */ ret = vq->data[i];detach_buf (vq, i); - vq->last_used_idx++; +vring_last_used (&vq->vring)++;END_USE (vq);return ret; }
@@ -302,7 +304,6 @@ struct virtqueue *vring_new_virtqueue(un vq->vq.vq_ops = &vring_vq_ops; vq->notify = notify; vq->broken = false; - vq->last_used_idx =0 ; vq->num_added =0 ;#ifdef DEBUG vq->in_use = false;
@@ -318,6 +319,9 @@ struct virtqueue *vring_new_virtqueue(unfor (i =0 ; i < num-1 ; i++) vq->vring.desc[i].next = i+1 ; +/* We now publish our last_used index in the rings. */ + vdev->config->feature (vdev,VIRTIO_RING_F_PUBLISH_INDICES ); +return &vq->vq; }EXPORT_SYMBOL_GPL (vring_new_virtqueue);
include/linux/virtio_config.h +3 -0
@@ -14,6 +14,9 @@#define VIRTIO_CONFIG_S_DRIVER_OK 4/* We've given up on this device. */ #define VIRTIO_CONFIG_S_FAILED 0x80 + +/* Transport virtio feature bits (currently bits 24 to 32 are reserved +* for this), the rest are per-device feature bits. */ #ifdef __KERNEL__struct virtio_device ;
include/linux/virtio_ring.h +11 -1
@@ -23,6 +23,9 @@* when you consume a buffer. It's unreliable, so it's simply an * optimization. */ #define VRING_AVAIL_F_NO_INTERRUPT 1 + +/* We publish our last-seen used index at the end of the avail ring. */ +#define VIRTIO_RING_F_PUBLISH_INDICES 24/* Virtio ring descriptors: 16 bytes. These can chain together via "next". */ struct vring_desc
@@ -82,6 +85,7 @@ struct vring {* __u16 avail_flags; * __u16 avail_idx; * __u16 available[num]; +* __u16 last_used_idx; * * // Padding to the next page boundary. * char pad[];
@@ -90,6 +94,7 @@ struct vring {* __u16 used_flags; * __u16 used_idx; * struct vring_used_elem used[num]; +* __u16 last_avail_idx; * }; */ static inline void vring_init(struct vring *vr, unsigned int num, void *p,
@@ -106,8 +111,13 @@ static inline unsigned vring_size(unsign {return ((sizeof (struct vring_desc ) * num +sizeof (__u16) * (2 + num) + pagesize -1 ) & ~(pagesize -1 )) - +sizeof (__u16) *2 +sizeof (struct vring_used_elem ) * num; + +sizeof (__u16) *2 +sizeof (struct vring_used_elem ) * num +2 ; } + +/* We publish the last-seen used index at the end of the available ring, and +* vice-versa. These are at the end for backwards compatibility. */ +#define vring_last_used (vr) ((vr)->avail->ring[(vr)->num]) +#define vring_last_avail (vr) (*(__u16 *)&(vr)->used->ring[(vr)->num])#ifdef __KERNEL__#include <linux/irqreturn.h>
PATCH 2/5 /dev/vring: simple userspace-kernel ringbuffer interface.
virtio introduced a ring structure ABI for guest-host communications (currently used by lguest and kvm). Using this same ABI, we can create a nice fd version.
This is useful for efficiently passing packets to and from the tun, for example.
--- drivers/char/Kconfig | 9 + drivers/char/Makefile | 2 drivers/char/vring.c | 400 ++++++++++++++++++++++++++++++++++++++++++++++++++ include/linux/vring.h | 58 +++++++ 4 files changed, 469 insertions(+)
drivers/char/Kconfig +9 -0
@@ -1049,5 +1049,14 @@ config DEVPORT source "drivers/s390/char/Kconfig" +config VRING + tristate "/dev/vring support (EXPERIMENTAL)" + depends on EXPERIMENTAL + help + vring is a ringbuffer implementation for efficient I/O. It is + currently used by virtualization hosts (lguest, kvm) for efficient + networking using the tun driver. + + If unsure, say N, but there's a part of you that wants to say M. endmenu
drivers/char/Makefile +2 -0
@@ -112,6 +112,8 @@ obj-$(CONFIG_JS_RTC) += js-rtc.o obj-$(CONFIG_JS_RTC) += js-rtc.o js-rtc-y = rtc.o +obj-$(CONFIG_VRING) += vring.o + # Files generated that shall be removed upon make clean clean-files := consolemap_deftbl.c defkeymap.c
drivers/char/vring.c new file +400 -0
400 lines not on this page · raw message
include/linux/vring.h new file +58 -0
58 lines not on this page · raw message
PATCH 2/5 /dev/vring: simple userspace-kernel ringbuffer interface.
In reply to Rusty Russellvirtio introduced a ring structure ABI for guest-host communications (currently used by lguest and kvm). Using this same ABI, we can create a nice fd version.
This is useful for efficiently passing packets to and from the tun, for example.
--- drivers/char/Kconfig | 9 + drivers/char/Makefile | 2 drivers/char/vring.c | 400 ++++++++++++++++++++++++++++++++++++++++++++++++++ include/linux/vring.h | 58 +++++++ 4 files changed, 469 insertions(+)
drivers/char/Kconfig +9 -0
@@ -1049,5 +1049,14 @@ config DEVPORT source "drivers/s390/char/Kconfig" +config VRING + tristate "/dev/vring support (EXPERIMENTAL)" + depends on EXPERIMENTAL + help + vring is a ringbuffer implementation for efficient I/O. It is + currently used by virtualization hosts (lguest, kvm) for efficient + networking using the tun driver. + + If unsure, say N, but there's a part of you that wants to say M. endmenu
drivers/char/Makefile +2 -0
@@ -112,6 +112,8 @@ obj-$(CONFIG_JS_RTC) += js-rtc.o obj-$(CONFIG_JS_RTC) += js-rtc.o js-rtc-y = rtc.o +obj-$(CONFIG_VRING) += vring.o + # Files generated that shall be removed upon make clean clean-files := consolemap_deftbl.c defkeymap.c
drivers/char/vring.c new file +400 -0
400 lines not on this page · raw message
include/linux/vring.h new file +58 -0
58 lines not on this page · raw message
PATCH 3/5 /dev/vring limit and base ioctls
It turns out the lguest (and possibly kvm) want the addresses in the ring buffer to only cover a certain part of memory, and be offset.
It makes sense that this be an ioctl.
--- Documentation/ioctl-number.txt | 3 +-- drivers/char/vring.c | 37 +++++++++++++++++++++++++++++++++++-- include/linux/vring.h | 9 +++++++++ 3 files changed, 45 insertions(+), 4 deletions(-)
Documentation/ioctl-number.txt +1 -2
@@ -181,8 +181,7 @@ 0xA3 90-9F linux/dtlk.h 0xA3 90-9F linux/dtlk.h 0xAB 00-1F linux/nbd.h 0xAC 00-1F linux/raw.h -0xAD 00 Netfilter device in development: - <mailto:[email protected]> +0xAD 00-01 linux/vring.h 0xB0 all RATIO devices in development: <mailto:[email protected]> 0xB1 00-1F PPPoX <mailto:[email protected]>
drivers/char/vring.c +35 -2
@@ -32,6 +32,8 @@ struct vring_info {struct vring ring;u16 mask;u16 last_used; + +unsigned long base, limit;const struct vring_ops *ops;void *ops_data;
@@ -163,6 +165,26 @@ static int vring_open(struct inode *in,init_waitqueue_head (&vr->poll_wait);mutex_init (&vr->lock); + vr->limit =-1UL ; + vr->base =0 ; +return 0 ; +} + +static int vring_ioctl (struct inode *in,struct file *filp, +unsigned int cmd,unsigned long arg) +{ +struct vring_info *vr = filp->private_data; + +switch (cmd) { +case VRINGSETBASE : + vr->base = arg; +break ; +case VRINGSETLIMIT : + vr->limit = arg; +break ; +default : +return -ENOTTY ; + }return 0 ; }
@@ -173,6 +195,7 @@ static const struct file_operations vrin .read = vring_read, .write = vring_write, .poll = vring_poll, + .ioctl = vring_ioctl, };/**
@@ -220,6 +243,8 @@ int vring_get_buffer(struct vring_info * i = head; do { + void __user *base; + if (unlikely(i >= vr->ring.num)) { pr_debug("vring: bad index: %u\n", i); return -EINVAL;
@@ -227,6 +252,14 @@ int vring_get_buffer(struct vring_info *if (copy_from_user (&d, &vr->ring.desc[i],sizeof (d)) !=0 )return -EFAULT ; + +if (d.addr + d.len > vr->limit || (d.addr + d.len < d.addr)) { +pr_debug ("vring: bad addr/len: %u@%p\n" , + d.len, (void *)(unsigned long )d.addr); +return -EINVAL ; + } + + base = (void __user *)(unsigned long )d.addr + vr->base;if (d.flags &VRING_DESC_F_WRITE ) {/* Check for length and iovec overflows */
@@ -239,7 +272,7 @@ int vring_get_buffer(struct vring_info *return -E2BIG ; in_iov[in].iov_len = d.len; *in_len += d.len; - in_iov[in].iov_base = (void __user *)(long )d.addr; + in_iov[in].iov_base = base; in++; }else {if (!num_out) {
@@ -251,7 +284,7 @@ int vring_get_buffer(struct vring_info *return -E2BIG ; out_iov[out].iov_len = d.len; *out_len += d.len; - out_iov[out].iov_base = (void __user *)(long )d.addr; + out_iov[out].iov_base = base; out++; }
include/linux/vring.h +9 -0
@@ -18,6 +18,13 @@*/ #ifndef _LINUX_VRING_H#define _LINUX_VRING_H +#include <linux/types.h> + +/* Ioctl defines. */ +#define VRINGSETBASE _IO(0xAD, 0) +#define VRINGSETLIMIT _IO(0xAD, 1) + +#ifdef __KERNEL__/** * vring_ops - operations for a vring fd.
@@ -55,4 +62,6 @@ void vring_used_buffer(struct vring_infovoid vring_used_buffer (struct vring_info *vr,int id,u32 len);void vring_wake (struct vring_info *vr); +#endif /* __KERNEL__ */ +#endif /* _LINUX_VRING_H */
PATCH 3/5 /dev/vring limit and base ioctls
In reply to Rusty RussellIt turns out the lguest (and possibly kvm) want the addresses in the ring buffer to only cover a certain part of memory, and be offset.
It makes sense that this be an ioctl.
--- Documentation/ioctl-number.txt | 3 +-- drivers/char/vring.c | 37 +++++++++++++++++++++++++++++++++++-- include/linux/vring.h | 9 +++++++++ 3 files changed, 45 insertions(+), 4 deletions(-)
Documentation/ioctl-number.txt +1 -2
@@ -181,8 +181,7 @@ 0xA3 90-9F linux/dtlk.h 0xA3 90-9F linux/dtlk.h 0xAB 00-1F linux/nbd.h 0xAC 00-1F linux/raw.h -0xAD 00 Netfilter device in development: - <mailto:[email protected]> +0xAD 00-01 linux/vring.h 0xB0 all RATIO devices in development: <mailto:[email protected]> 0xB1 00-1F PPPoX <mailto:[email protected]>
drivers/char/vring.c +35 -2
@@ -32,6 +32,8 @@ struct vring_info {struct vring ring;u16 mask;u16 last_used; + +unsigned long base, limit;const struct vring_ops *ops;void *ops_data;
@@ -163,6 +165,26 @@ static int vring_open(struct inode *in,init_waitqueue_head (&vr->poll_wait);mutex_init (&vr->lock); + vr->limit =-1UL ; + vr->base =0 ; +return 0 ; +} + +static int vring_ioctl (struct inode *in,struct file *filp, +unsigned int cmd,unsigned long arg) +{ +struct vring_info *vr = filp->private_data; + +switch (cmd) { +case VRINGSETBASE : + vr->base = arg; +break ; +case VRINGSETLIMIT : + vr->limit = arg; +break ; +default : +return -ENOTTY ; + }return 0 ; }
@@ -173,6 +195,7 @@ static const struct file_operations vrin .read = vring_read, .write = vring_write, .poll = vring_poll, + .ioctl = vring_ioctl, };/**
@@ -220,6 +243,8 @@ int vring_get_buffer(struct vring_info * i = head; do { + void __user *base; + if (unlikely(i >= vr->ring.num)) { pr_debug("vring: bad index: %u\n", i); return -EINVAL;
@@ -227,6 +252,14 @@ int vring_get_buffer(struct vring_info *if (copy_from_user (&d, &vr->ring.desc[i],sizeof (d)) !=0 )return -EFAULT ; + +if (d.addr + d.len > vr->limit || (d.addr + d.len < d.addr)) { +pr_debug ("vring: bad addr/len: %u@%p\n" , + d.len, (void *)(unsigned long )d.addr); +return -EINVAL ; + } + + base = (void __user *)(unsigned long )d.addr + vr->base;if (d.flags &VRING_DESC_F_WRITE ) {/* Check for length and iovec overflows */
@@ -239,7 +272,7 @@ int vring_get_buffer(struct vring_info *return -E2BIG ; in_iov[in].iov_len = d.len; *in_len += d.len; - in_iov[in].iov_base = (void __user *)(long )d.addr; + in_iov[in].iov_base = base; in++; }else {if (!num_out) {
@@ -251,7 +284,7 @@ int vring_get_buffer(struct vring_info *return -E2BIG ; out_iov[out].iov_len = d.len; *out_len += d.len; - out_iov[out].iov_base = (void __user *)(long )d.addr; + out_iov[out].iov_base = base; out++; }
include/linux/vring.h +9 -0
@@ -18,6 +18,13 @@*/ #ifndef _LINUX_VRING_H#define _LINUX_VRING_H +#include <linux/types.h> + +/* Ioctl defines. */ +#define VRINGSETBASE _IO(0xAD, 0) +#define VRINGSETLIMIT _IO(0xAD, 1) + +#ifdef __KERNEL__/** * vring_ops - operations for a vring fd.
@@ -55,4 +62,6 @@ void vring_used_buffer(struct vring_infovoid vring_used_buffer (struct vring_info *vr,int id,u32 len);void vring_wake (struct vring_info *vr); +#endif /* __KERNEL__ */ +#endif /* _LINUX_VRING_H */
PATCH 4/5 tun: vringfd receive support.
This patch modifies tun to allow a vringfd to specify the receive buffer. Because we can't copy to userspace in bh context, we queue like normal then use the "pull" hook to actually do the copy.
We use struct virtio_net_hdr prepended to packets in the ring to allow userspace to receive GSO packets in future (at the moment, the tun driver doesn't tell the stack it can handle them, so these cases are never taken). This will need to be something that userspace tells us it can handle.
--- drivers/net/Kconfig | 2 drivers/net/tun.c | 159 +++++++++++++++++++++++++++++++++++++++++++++++++ include/linux/if_tun.h | 1 3 files changed, 162 insertions(+)
drivers/net/Kconfig +2 -0
@@ -120,6 +120,8 @@ config TUN config TUN tristate "Universal TUN/TAP device driver support" select CRC32 +# If no VRING at all, that's fine, but if it's a module, we must be, too. + depends on !VRING || VRING ---help--- TUN/TAP provides packet reception and transmission for user space programs. It can be viewed as a simple Point-to-Point or Ethernet
drivers/net/tun.c +159 -0
201 lines not on this page · raw message
include/linux/if_tun.h +1 -0
@@ -42,6 +42,7 @@#define TUNSETOWNER _IOW('T', 204, int)#define TUNSETLINK _IOW('T', 205, int)#define TUNSETGROUP _IOW('T', 206, int) +#define TUNSETRECVVRING _IOW('T', 207, int)/* TUNSETIFF ifr flags */ #define IFF_TUN 0x0001
PATCH 4/5 tun: vringfd receive support.
In reply to Rusty RussellThis patch modifies tun to allow a vringfd to specify the receive buffer. Because we can't copy to userspace in bh context, we queue like normal then use the "pull" hook to actually do the copy.
We use struct virtio_net_hdr prepended to packets in the ring to allow userspace to receive GSO packets in future (at the moment, the tun driver doesn't tell the stack it can handle them, so these cases are never taken). This will need to be something that userspace tells us it can handle.
--- drivers/net/Kconfig | 2 drivers/net/tun.c | 159 +++++++++++++++++++++++++++++++++++++++++++++++++ include/linux/if_tun.h | 1 3 files changed, 162 insertions(+)
drivers/net/Kconfig +2 -0
@@ -120,6 +120,8 @@ config TUN config TUN tristate "Universal TUN/TAP device driver support" select CRC32 +# If no VRING at all, that's fine, but if it's a module, we must be, too. + depends on !VRING || VRING ---help--- TUN/TAP provides packet reception and transmission for user space programs. It can be viewed as a simple Point-to-Point or Ethernet
drivers/net/tun.c +159 -0
201 lines not on this page · raw message
include/linux/if_tun.h +1 -0
@@ -42,6 +42,7 @@#define TUNSETOWNER _IOW('T', 204, int)#define TUNSETLINK _IOW('T', 205, int)#define TUNSETGROUP _IOW('T', 206, int) +#define TUNSETRECVVRING _IOW('T', 207, int)/* TUNSETIFF ifr flags */ #define IFF_TUN 0x0001
PATCH 5/5 tun: vringfd xmit support.
This patch modifies tun to allow a vringfd to specify the send buffer. The user does a write to push out packets from the buffer.
Again we use the 'struct virtio_net_hdr' to allow userspace to send GSO packets. In this case, it can hint how much to copy, and the other pages will be made into skb fragments.
--- drivers/net/tun.c | 410 +++++++++++++++++++++++++++++++++++++++++-------- include/linux/if_tun.h | 1 2 files changed, 351 insertions(+), 60 deletions(-)
drivers/net/tun.c +350 -60
@@ -65,6 +65,8 @@#include <linux/vring.h> #include <linux/virtio_net.h> #include <linux/file.h> +#include <linux/spinlock.h> +#include <linux/kthread.h> #include <net/net_namespace.h> #include <asm/system.h>
@@ -102,8 +104,8 @@ struct tun_struct {u32 chr_filter[2 ];u32 net_filter[2 ]; -struct vring_info *inring; -struct file *infile; +struct vring_info *inring, *outring; +struct file *infile, *outfile;#ifdef TUN_DEBUG int debug;
@@ -258,6 +261,169 @@ static void tun_net_init(struct net_devi dev->tx_queue_len =TUN_READQ_SIZE ;/* We prefer our own queue length */ break ; } +} + +/* We don't consolidate consecutive iovecs, so huge iovecs can break here. +* Users will learn not to do that. */ +static int get_user_skb_frags (const struct iovec *iv,size_t len, +struct skb_frag_struct *f) +{ +unsigned int i, j, num_pg =0 ; +int err; +struct page *pages[MAX_SKB_FRAGS ]; + +down_read (¤t->mm->mmap_sem); +while (len) { +int n, npages; +unsigned long base, len; + base = (unsigned long )iv->iov_base; + len = (unsigned long )iv->iov_len; + +if (len ==0 ) { + iv++; +continue ; + } + +/* How many pages will this take? */ + npages =1 + (base + len -1 )/PAGE_SIZE - base/PAGE_SIZE ; +if (unlikely (num_pg + npages >MAX_SKB_FRAGS )) { + err = -ENOSPC ; + goto fail; + } + n =get_user_pages (current, current->mm, base, npages, +0 ,0 , pages,NULL ); +if (unlikely (n <0 )) { + err = n; + goto fail; + } + +/* Transfer pages to the frag array */ +for (j =0 ; j < n; j++) { + f[num_pg].page = pages[j]; +if (j ==0 ) { + f[num_pg].page_offset =offset_in_page (base); + f[num_pg].size =min (len,PAGE_SIZE - + f[num_pg].page_offset); + }else { + f[num_pg].page_offset =0 ; + f[num_pg].size =min (len,PAGE_SIZE ); + } + len -= f[num_pg].size; + base += f[num_pg].size; + num_pg++; + } + +if (unlikely (n != npages)) { + err = -EFAULT ; + goto fail; + } + } +up_read (¤t->mm->mmap_sem); +return num_pg; + +fail: +for (i =0 ; i < num_pg; i++) +put_page (f[i].page); +up_read (¤t->mm->mmap_sem); +return err; +} + +/* We actually store this at the head of the skb. */ +struct skb_tun_hdr { +struct list_head list; +struct tun_struct *tun; +unsigned int id; +unsigned int len; +}; + +/* Get packet from user space buffer. copylen is a hint as to how +* much to copy (rest is pinned). */ +static struct sk_buff *get_user_skb (struct tun_struct *tun,struct iovec *iv, +size_t copylen,size_t len) +{ +struct tun_pi pi = {0 ,__constant_htons (ETH_P_IP ) }; +struct sk_buff *skb; +size_t align =0 , extra =0 ; +int err; + +if (!(tun->flags &TUN_NO_PI )) { +if (len <sizeof (pi)) { + err = -EINVAL ; + goto fail; + } + len -=sizeof (pi); + +if (memcpy_fromiovec ((void *)&pi, iv,sizeof (pi))) { + err = -EFAULT ; + goto fail; + } +if (copylen > len) + copylen = len; + } + +if ((tun->flags &TUN_TYPE_MASK ) ==TUN_TAP_DEV ) { + align =NET_IP_ALIGN ; +if (unlikely (copylen <ETH_HLEN )) { +if (len <ETH_HLEN ) { + err = -EINVAL ; + goto fail; + } + copylen =ETH_HLEN ; + } + } + +/* Allocate extra header if we need */ +if (copylen != len) + extra =sizeof (struct skb_tun_hdr ); + + skb =alloc_skb (extra + copylen + align,GFP_KERNEL ); +if (!skb) { + err = -ENOMEM ; + goto fail; + } + +if (extra + align) +skb_reserve (skb, extra + align); + +if (memcpy_fromiovec (skb_put (skb, copylen), iv, copylen)) { + err = -EFAULT ; + goto free_skb; + } + +switch (tun->flags &TUN_TYPE_MASK ) { +case TUN_TUN_DEV : +skb_reset_mac_header (skb); + skb->protocol = pi.proto; + skb->dev = tun->dev; +break ; +case TUN_TAP_DEV : + skb->protocol =eth_type_trans (skb, tun->dev); +break ; + }; + +if (tun->flags &TUN_NOCHECKSUM ) + skb->ip_summed =CHECKSUM_UNNECESSARY ; + +/* Anything left gets put into frags. */ +if (extra) { +struct skb_shared_info *sinfo =skb_shinfo (skb); +int err =get_user_skb_frags (iv, len - copylen, sinfo->frags); +if (err <0 ) + goto free_skb; + sinfo->nr_frags = err; + } + tun->dev->last_rx = jiffies; + + tun->dev->stats.rx_packets++; + tun->dev->stats.rx_bytes += len; + +return skb; + +free_skb: +kfree_skb (skb); +fail: + tun->dev->stats.rx_dropped++; +return ERR_PTR (err); }#if defined(CONFIG_VRING ) || defined(CONFIG_VRING_MODULE )
@@ -355,6 +521,132 @@ static struct vring_ops recvops = { .pull = pull_recv_skbs, }; +static DEFINE_SPINLOCK (finished_lock); +static LIST_HEAD (shinfo_finished_list); +static struct task_struct *shinfo_finisher; + +static void used_buffer (struct skb_tun_hdr *tunh) +{ +/* Woot, something happened. */ +vring_wake (tunh->tun->outring); + +/* Release device. Keeping this reference blocks file close. */ +dev_put (tunh->tun->dev); + +/* tunh == skb->head. */ +kfree (tunh); +} + +static int do_shinfo_finisher (void *unused) +{ +LIST_HEAD (list); +struct skb_tun_hdr *i; + +while (!kthread_should_stop ()) { +set_current_state (TASK_INTERRUPTIBLE ); + +spin_lock_irq (&finished_lock); +list_splice_init (&list, &shinfo_finished_list); +spin_unlock_irq (&finished_lock); + +if (list_empty (&list)) { +schedule (); +continue ; + } + +list_for_each_entry (i, &list, list) { +vring_used_buffer (i->tun->outring, i->id, i->len); +used_buffer (i); + } + } +return 0 ; +} + +/* We are done with this skb data: put it in the used pile. */ +static void shinfo_finished (struct skb_shared_info *sinfo) +{ +struct skb_tun_hdr *tunh = (void *)skb_shinfo_to_head (sinfo); +unsigned long flags; + +spin_lock_irqsave (&finished_lock, flags); +list_add (&tunh->list, &shinfo_finished_list); +spin_unlock_irqrestore (&finished_lock, flags); + +wake_up_process (shinfo_finisher); +} + +static int xmit_packets (void *_tun) +{ +struct tun_struct *tun = _tun; +struct iovec iov[1 +MAX_SKB_FRAGS ]; +unsigned int iovnum =ARRAY_SIZE (iov); +int id, err, wake =0 ; +unsigned long len; + +while ((id =vring_get_buffer (tun->outring,NULL ,NULL ,NULL , + iov, &iovnum, &len)) >0 ) { +struct virtio_net_hdr h; +struct sk_buff *skb; +struct skb_shared_info *shinfo; + +if (unlikely (len <sizeof (h))) +return -EINVAL ; + + err =memcpy_fromiovec ((void *)&h, iov,sizeof (h)); +if (unlikely (err)) +return -EFAULT ; + + len -=sizeof (h); +if (h.hdr_len > len) +return -EINVAL ; + +/* Without GSO, we copy entire packet. */ +if (h.gso_type ==VIRTIO_NET_HDR_GSO_NONE ) + h.hdr_len = len; + + skb =get_user_skb (tun, iov, h.hdr_len, len); +if (IS_ERR (skb)) +return PTR_ERR (skb); + +if ((h.flags &VIRTIO_NET_HDR_F_NEEDS_CSUM ) && + !skb_partial_csum_set (skb, h.csum_start, h.csum_offset)) { +kfree_skb (skb); +return -EINVAL ; + } + +/* If it has fragments, set up destructor for later. */ + shinfo =skb_shinfo (skb); +if (skb_shinfo (skb)->nr_frags) { +struct skb_tun_hdr *tunh = (void *)skb->head; + shinfo->destructor = shinfo_finished; + tunh->id = id; + tunh->len =sizeof (h) + skb->len; + }else { +vring_used_buffer (tun->outring, id,sizeof (h)+skb->len); + wake =1 ; + } +netif_rx_ni (skb); + } + +if (wake) +vring_wake (tun->outring); + +/* 0 or error. */ +return id; +} + +static struct vring_ops xmitops = { + .push = xmit_packets, +}; + +static int init_vring (void ) +{ + shinfo_finisher =kthread_run (do_shinfo_finisher,NULL ,"tun" ); +if (IS_ERR (shinfo_finisher)) +return PTR_ERR (shinfo_finisher); +return 0 ; +} +static int set_recv_vring (struct tun_struct *tun,int fd) {int err;
@@ -391,9 +685,47 @@ static void unset_vrings(struct tun_struvring_unset_ops (tun->inring); fput(tun->infile); } +if (tun->outring) { +vring_unset_ops (tun->outring); +fput (tun->outfile); + } +} + +static int set_xmit_vring (struct tun_struct *tun,int fd) +{ +int err; + +if (tun->outring) +return -EBUSY ; + + tun->outfile =fget (fd); +if (!tun->outfile) +return -EBADF ; + + tun->outring =vring_get (tun->outfile); +if (!tun->outring) { + err = -EBADF ; + goto put; + } + + err =vring_set_ops (tun->outring, &xmitops, tun); +if (err) { + tun->outring =NULL ; + goto put; + } +return 0 ; + +put: +fput (tun->outfile); + tun->outfile =NULL ; +return err; }#else /* ... !CONFIG_VRING */ static int set_recv_vring (struct tun_struct *tun,int fd) +{ +return -ENOTTY ; +} +static int set_xmit_vring (struct tun_struct *tun,int fd) {return -ENOTTY ; }
@@ -424,74 +756,26 @@ static unsigned int tun_chr_poll(structreturn mask; } -/* Get packet from user space buffer */ -static __inline__ssize_t tun_get_user (struct tun_struct *tun,struct iovec *iv,size_t count) -{ -struct tun_pi pi = {0 ,__constant_htons (ETH_P_IP ) }; -struct sk_buff *skb; -size_t len = count, align =0 ; - -if (!(tun->flags &TUN_NO_PI )) { -if ((len -=sizeof (pi)) > count) -return -EINVAL ; - -if (memcpy_fromiovec ((void *)&pi, iv,sizeof (pi))) -return -EFAULT ; - } - -if ((tun->flags &TUN_TYPE_MASK ) ==TUN_TAP_DEV ) { - align =NET_IP_ALIGN ; -if (unlikely (len <ETH_HLEN )) -return -EINVAL ; - } - -if (!(skb =alloc_skb (len + align,GFP_KERNEL ))) { - tun->dev->stats.rx_dropped++; -return -ENOMEM ; - } - -if (align) -skb_reserve (skb, align); -if (memcpy_fromiovec (skb_put (skb, len), iv, len)) { - tun->dev->stats.rx_dropped++; -kfree_skb (skb); -return -EFAULT ; - } - -switch (tun->flags &TUN_TYPE_MASK ) { -case TUN_TUN_DEV : -skb_reset_mac_header (skb); - skb->protocol = pi.proto; - skb->dev = tun->dev; -break ; -case TUN_TAP_DEV : - skb->protocol =eth_type_trans (skb, tun->dev); -break ; - }; - -if (tun->flags &TUN_NOCHECKSUM ) - skb->ip_summed =CHECKSUM_UNNECESSARY ; - -netif_rx_ni (skb); - tun->dev->last_rx = jiffies; - - tun->dev->stats.rx_packets++; - tun->dev->stats.rx_bytes += len; - -return count; -} -static ssize_t tun_chr_aio_write (struct kiocb *iocb,const struct iovec *iv,unsigned long count,loff_t pos) {struct tun_struct *tun = iocb->ki_filp->private_data; +size_t len; +struct sk_buff *skb;if (!tun)return -EBADFD ;DBG (KERN_INFO "%s: tun_chr_write %ld\n" , tun->dev->name, count); -return tun_get_user (tun, (struct iovec *) iv,iov_length (iv, count)); + len =iov_length (iv, count); + + skb =get_user_skb (tun, (struct iovec *)iv, len, len); +if (IS_ERR (skb)) +return PTR_ERR (skb); + +netif_rx_ni (skb); +return len; }/* Put packet to the user space buffer */
@@ -831,6 +1115,9 @@ static int tun_chr_ioctl(struct inode *icase TUNSETRECVVRING :return set_recv_vring (tun, arg); +case TUNSETXMITVRING : +return set_xmit_vring (tun, arg); +case SIOCGIFFLAGS : ifr.ifr_flags = tun->if_flags;if (copy_to_user ( argp, &ifr,sizeof ifr))
@@ -1078,6 +1365,12 @@ static int __init tun_init(void) ret =misc_register (&tun_miscdev);if (ret)printk (KERN_ERR "tun: Can't register misc device %d\n" ,TUN_MINOR ); +else { + ret =init_vring (); +if (ret) +misc_deregister (&tun_miscdev); + } +return ret; }
include/linux/if_tun.h +1 -0
@@ -43,6 +43,7 @@#define TUNSETLINK _IOW('T', 205, int)#define TUNSETGROUP _IOW('T', 206, int)#define TUNSETRECVVRING _IOW('T', 207, int) +#define TUNSETXMITVRING _IOW('T', 208, int)/* TUNSETIFF ifr flags */ #define IFF_TUN 0x0001
PATCH 5/5 tun: vringfd xmit support.
In reply to Rusty RussellThis patch modifies tun to allow a vringfd to specify the send buffer. The user does a write to push out packets from the buffer.
Again we use the 'struct virtio_net_hdr' to allow userspace to send GSO packets. In this case, it can hint how much to copy, and the other pages will be made into skb fragments.
--- drivers/net/tun.c | 410 +++++++++++++++++++++++++++++++++++++++++-------- include/linux/if_tun.h | 1 2 files changed, 351 insertions(+), 60 deletions(-)
drivers/net/tun.c +350 -60
@@ -65,6 +65,8 @@#include <linux/vring.h> #include <linux/virtio_net.h> #include <linux/file.h> +#include <linux/spinlock.h> +#include <linux/kthread.h> #include <net/net_namespace.h> #include <asm/system.h>
@@ -102,8 +104,8 @@ struct tun_struct {u32 chr_filter[2 ];u32 net_filter[2 ]; -struct vring_info *inring; -struct file *infile; +struct vring_info *inring, *outring; +struct file *infile, *outfile;#ifdef TUN_DEBUG int debug;
@@ -258,6 +261,169 @@ static void tun_net_init(struct net_devi dev->tx_queue_len =TUN_READQ_SIZE ;/* We prefer our own queue length */ break ; } +} + +/* We don't consolidate consecutive iovecs, so huge iovecs can break here. +* Users will learn not to do that. */ +static int get_user_skb_frags (const struct iovec *iv,size_t len, +struct skb_frag_struct *f) +{ +unsigned int i, j, num_pg =0 ; +int err; +struct page *pages[MAX_SKB_FRAGS ]; + +down_read (¤t->mm->mmap_sem); +while (len) { +int n, npages; +unsigned long base, len; + base = (unsigned long )iv->iov_base; + len = (unsigned long )iv->iov_len; + +if (len ==0 ) { + iv++; +continue ; + } + +/* How many pages will this take? */ + npages =1 + (base + len -1 )/PAGE_SIZE - base/PAGE_SIZE ; +if (unlikely (num_pg + npages >MAX_SKB_FRAGS )) { + err = -ENOSPC ; + goto fail; + } + n =get_user_pages (current, current->mm, base, npages, +0 ,0 , pages,NULL ); +if (unlikely (n <0 )) { + err = n; + goto fail; + } + +/* Transfer pages to the frag array */ +for (j =0 ; j < n; j++) { + f[num_pg].page = pages[j]; +if (j ==0 ) { + f[num_pg].page_offset =offset_in_page (base); + f[num_pg].size =min (len,PAGE_SIZE - + f[num_pg].page_offset); + }else { + f[num_pg].page_offset =0 ; + f[num_pg].size =min (len,PAGE_SIZE ); + } + len -= f[num_pg].size; + base += f[num_pg].size; + num_pg++; + } + +if (unlikely (n != npages)) { + err = -EFAULT ; + goto fail; + } + } +up_read (¤t->mm->mmap_sem); +return num_pg; + +fail: +for (i =0 ; i < num_pg; i++) +put_page (f[i].page); +up_read (¤t->mm->mmap_sem); +return err; +} + +/* We actually store this at the head of the skb. */ +struct skb_tun_hdr { +struct list_head list; +struct tun_struct *tun; +unsigned int id; +unsigned int len; +}; + +/* Get packet from user space buffer. copylen is a hint as to how +* much to copy (rest is pinned). */ +static struct sk_buff *get_user_skb (struct tun_struct *tun,struct iovec *iv, +size_t copylen,size_t len) +{ +struct tun_pi pi = {0 ,__constant_htons (ETH_P_IP ) }; +struct sk_buff *skb; +size_t align =0 , extra =0 ; +int err; + +if (!(tun->flags &TUN_NO_PI )) { +if (len <sizeof (pi)) { + err = -EINVAL ; + goto fail; + } + len -=sizeof (pi); + +if (memcpy_fromiovec ((void *)&pi, iv,sizeof (pi))) { + err = -EFAULT ; + goto fail; + } +if (copylen > len) + copylen = len; + } + +if ((tun->flags &TUN_TYPE_MASK ) ==TUN_TAP_DEV ) { + align =NET_IP_ALIGN ; +if (unlikely (copylen <ETH_HLEN )) { +if (len <ETH_HLEN ) { + err = -EINVAL ; + goto fail; + } + copylen =ETH_HLEN ; + } + } + +/* Allocate extra header if we need */ +if (copylen != len) + extra =sizeof (struct skb_tun_hdr ); + + skb =alloc_skb (extra + copylen + align,GFP_KERNEL ); +if (!skb) { + err = -ENOMEM ; + goto fail; + } + +if (extra + align) +skb_reserve (skb, extra + align); + +if (memcpy_fromiovec (skb_put (skb, copylen), iv, copylen)) { + err = -EFAULT ; + goto free_skb; + } + +switch (tun->flags &TUN_TYPE_MASK ) { +case TUN_TUN_DEV : +skb_reset_mac_header (skb); + skb->protocol = pi.proto; + skb->dev = tun->dev; +break ; +case TUN_TAP_DEV : + skb->protocol =eth_type_trans (skb, tun->dev); +break ; + }; + +if (tun->flags &TUN_NOCHECKSUM ) + skb->ip_summed =CHECKSUM_UNNECESSARY ; + +/* Anything left gets put into frags. */ +if (extra) { +struct skb_shared_info *sinfo =skb_shinfo (skb); +int err =get_user_skb_frags (iv, len - copylen, sinfo->frags); +if (err <0 ) + goto free_skb; + sinfo->nr_frags = err; + } + tun->dev->last_rx = jiffies; + + tun->dev->stats.rx_packets++; + tun->dev->stats.rx_bytes += len; + +return skb; + +free_skb: +kfree_skb (skb); +fail: + tun->dev->stats.rx_dropped++; +return ERR_PTR (err); }#if defined(CONFIG_VRING ) || defined(CONFIG_VRING_MODULE )
@@ -355,6 +521,132 @@ static struct vring_ops recvops = { .pull = pull_recv_skbs, }; +static DEFINE_SPINLOCK (finished_lock); +static LIST_HEAD (shinfo_finished_list); +static struct task_struct *shinfo_finisher; + +static void used_buffer (struct skb_tun_hdr *tunh) +{ +/* Woot, something happened. */ +vring_wake (tunh->tun->outring); + +/* Release device. Keeping this reference blocks file close. */ +dev_put (tunh->tun->dev); + +/* tunh == skb->head. */ +kfree (tunh); +} + +static int do_shinfo_finisher (void *unused) +{ +LIST_HEAD (list); +struct skb_tun_hdr *i; + +while (!kthread_should_stop ()) { +set_current_state (TASK_INTERRUPTIBLE ); + +spin_lock_irq (&finished_lock); +list_splice_init (&list, &shinfo_finished_list); +spin_unlock_irq (&finished_lock); + +if (list_empty (&list)) { +schedule (); +continue ; + } + +list_for_each_entry (i, &list, list) { +vring_used_buffer (i->tun->outring, i->id, i->len); +used_buffer (i); + } + } +return 0 ; +} + +/* We are done with this skb data: put it in the used pile. */ +static void shinfo_finished (struct skb_shared_info *sinfo) +{ +struct skb_tun_hdr *tunh = (void *)skb_shinfo_to_head (sinfo); +unsigned long flags; + +spin_lock_irqsave (&finished_lock, flags); +list_add (&tunh->list, &shinfo_finished_list); +spin_unlock_irqrestore (&finished_lock, flags); + +wake_up_process (shinfo_finisher); +} + +static int xmit_packets (void *_tun) +{ +struct tun_struct *tun = _tun; +struct iovec iov[1 +MAX_SKB_FRAGS ]; +unsigned int iovnum =ARRAY_SIZE (iov); +int id, err, wake =0 ; +unsigned long len; + +while ((id =vring_get_buffer (tun->outring,NULL ,NULL ,NULL , + iov, &iovnum, &len)) >0 ) { +struct virtio_net_hdr h; +struct sk_buff *skb; +struct skb_shared_info *shinfo; + +if (unlikely (len <sizeof (h))) +return -EINVAL ; + + err =memcpy_fromiovec ((void *)&h, iov,sizeof (h)); +if (unlikely (err)) +return -EFAULT ; + + len -=sizeof (h); +if (h.hdr_len > len) +return -EINVAL ; + +/* Without GSO, we copy entire packet. */ +if (h.gso_type ==VIRTIO_NET_HDR_GSO_NONE ) + h.hdr_len = len; + + skb =get_user_skb (tun, iov, h.hdr_len, len); +if (IS_ERR (skb)) +return PTR_ERR (skb); + +if ((h.flags &VIRTIO_NET_HDR_F_NEEDS_CSUM ) && + !skb_partial_csum_set (skb, h.csum_start, h.csum_offset)) { +kfree_skb (skb); +return -EINVAL ; + } + +/* If it has fragments, set up destructor for later. */ + shinfo =skb_shinfo (skb); +if (skb_shinfo (skb)->nr_frags) { +struct skb_tun_hdr *tunh = (void *)skb->head; + shinfo->destructor = shinfo_finished; + tunh->id = id; + tunh->len =sizeof (h) + skb->len; + }else { +vring_used_buffer (tun->outring, id,sizeof (h)+skb->len); + wake =1 ; + } +netif_rx_ni (skb); + } + +if (wake) +vring_wake (tun->outring); + +/* 0 or error. */ +return id; +} + +static struct vring_ops xmitops = { + .push = xmit_packets, +}; + +static int init_vring (void ) +{ + shinfo_finisher =kthread_run (do_shinfo_finisher,NULL ,"tun" ); +if (IS_ERR (shinfo_finisher)) +return PTR_ERR (shinfo_finisher); +return 0 ; +} +static int set_recv_vring (struct tun_struct *tun,int fd) {int err;
@@ -391,9 +685,47 @@ static void unset_vrings(struct tun_struvring_unset_ops (tun->inring); fput(tun->infile); } +if (tun->outring) { +vring_unset_ops (tun->outring); +fput (tun->outfile); + } +} + +static int set_xmit_vring (struct tun_struct *tun,int fd) +{ +int err; + +if (tun->outring) +return -EBUSY ; + + tun->outfile =fget (fd); +if (!tun->outfile) +return -EBADF ; + + tun->outring =vring_get (tun->outfile); +if (!tun->outring) { + err = -EBADF ; + goto put; + } + + err =vring_set_ops (tun->outring, &xmitops, tun); +if (err) { + tun->outring =NULL ; + goto put; + } +return 0 ; + +put: +fput (tun->outfile); + tun->outfile =NULL ; +return err; }#else /* ... !CONFIG_VRING */ static int set_recv_vring (struct tun_struct *tun,int fd) +{ +return -ENOTTY ; +} +static int set_xmit_vring (struct tun_struct *tun,int fd) {return -ENOTTY ; }
@@ -424,74 +756,26 @@ static unsigned int tun_chr_poll(structreturn mask; } -/* Get packet from user space buffer */ -static __inline__ssize_t tun_get_user (struct tun_struct *tun,struct iovec *iv,size_t count) -{ -struct tun_pi pi = {0 ,__constant_htons (ETH_P_IP ) }; -struct sk_buff *skb; -size_t len = count, align =0 ; - -if (!(tun->flags &TUN_NO_PI )) { -if ((len -=sizeof (pi)) > count) -return -EINVAL ; - -if (memcpy_fromiovec ((void *)&pi, iv,sizeof (pi))) -return -EFAULT ; - } - -if ((tun->flags &TUN_TYPE_MASK ) ==TUN_TAP_DEV ) { - align =NET_IP_ALIGN ; -if (unlikely (len <ETH_HLEN )) -return -EINVAL ; - } - -if (!(skb =alloc_skb (len + align,GFP_KERNEL ))) { - tun->dev->stats.rx_dropped++; -return -ENOMEM ; - } - -if (align) -skb_reserve (skb, align); -if (memcpy_fromiovec (skb_put (skb, len), iv, len)) { - tun->dev->stats.rx_dropped++; -kfree_skb (skb); -return -EFAULT ; - } - -switch (tun->flags &TUN_TYPE_MASK ) { -case TUN_TUN_DEV : -skb_reset_mac_header (skb); - skb->protocol = pi.proto; - skb->dev = tun->dev; -break ; -case TUN_TAP_DEV : - skb->protocol =eth_type_trans (skb, tun->dev); -break ; - }; - -if (tun->flags &TUN_NOCHECKSUM ) - skb->ip_summed =CHECKSUM_UNNECESSARY ; - -netif_rx_ni (skb); - tun->dev->last_rx = jiffies; - - tun->dev->stats.rx_packets++; - tun->dev->stats.rx_bytes += len; - -return count; -} -static ssize_t tun_chr_aio_write (struct kiocb *iocb,const struct iovec *iv,unsigned long count,loff_t pos) {struct tun_struct *tun = iocb->ki_filp->private_data; +size_t len; +struct sk_buff *skb;if (!tun)return -EBADFD ;DBG (KERN_INFO "%s: tun_chr_write %ld\n" , tun->dev->name, count); -return tun_get_user (tun, (struct iovec *) iv,iov_length (iv, count)); + len =iov_length (iv, count); + + skb =get_user_skb (tun, (struct iovec *)iv, len, len); +if (IS_ERR (skb)) +return PTR_ERR (skb); + +netif_rx_ni (skb); +return len; }/* Put packet to the user space buffer */
@@ -831,6 +1115,9 @@ static int tun_chr_ioctl(struct inode *icase TUNSETRECVVRING :return set_recv_vring (tun, arg); +case TUNSETXMITVRING : +return set_xmit_vring (tun, arg); +case SIOCGIFFLAGS : ifr.ifr_flags = tun->if_flags;if (copy_to_user ( argp, &ifr,sizeof ifr))
@@ -1078,6 +1365,12 @@ static int __init tun_init(void) ret =misc_register (&tun_miscdev);if (ret)printk (KERN_ERR "tun: Can't register misc device %d\n" ,TUN_MINOR ); +else { + ret =init_vring (); +if (ret) +misc_deregister (&tun_miscdev); + } +return ret; }
include/linux/if_tun.h +1 -0
@@ -43,6 +43,7 @@#define TUNSETLINK _IOW('T', 205, int)#define TUNSETGROUP _IOW('T', 206, int)#define TUNSETRECVVRING _IOW('T', 207, int) +#define TUNSETXMITVRING _IOW('T', 208, int)/* TUNSETIFF ifr flags */ #define IFF_TUN 0x0001
PATCH 5/5 tun: vringfd xmit support.
In reply to Rusty RussellThis patch modifies tun to allow a vringfd to specify the send buffer. The user does a write to push out packets from the buffer.
Again we use the 'struct virtio_net_hdr' to allow userspace to send GSO packets. In this case, it can hint how much to copy, and the other pages will be made into skb fragments.
--- drivers/net/tun.c | 410 +++++++++++++++++++++++++++++++++++++++++-------- include/linux/if_tun.h | 1 2 files changed, 351 insertions(+), 60 deletions(-)
drivers/net/tun.c +350 -60
@@ -65,6 +65,8 @@#include <linux/vring.h> #include <linux/virtio_net.h> #include <linux/file.h> +#include <linux/spinlock.h> +#include <linux/kthread.h> #include <net/net_namespace.h> #include <asm/system.h>
@@ -102,8 +104,8 @@ struct tun_struct {u32 chr_filter[2 ];u32 net_filter[2 ]; -struct vring_info *inring; -struct file *infile; +struct vring_info *inring, *outring; +struct file *infile, *outfile;#ifdef TUN_DEBUG int debug;
@@ -258,6 +261,169 @@ static void tun_net_init(struct net_devi dev->tx_queue_len =TUN_READQ_SIZE ;/* We prefer our own queue length */ break ; } +} + +/* We don't consolidate consecutive iovecs, so huge iovecs can break here. +* Users will learn not to do that. */ +static int get_user_skb_frags (const struct iovec *iv,size_t len, +struct skb_frag_struct *f) +{ +unsigned int i, j, num_pg =0 ; +int err; +struct page *pages[MAX_SKB_FRAGS ]; + +down_read (¤t->mm->mmap_sem); +while (len) { +int n, npages; +unsigned long base, len; + base = (unsigned long )iv->iov_base; + len = (unsigned long )iv->iov_len; + +if (len ==0 ) { + iv++; +continue ; + } + +/* How many pages will this take? */ + npages =1 + (base + len -1 )/PAGE_SIZE - base/PAGE_SIZE ; +if (unlikely (num_pg + npages >MAX_SKB_FRAGS )) { + err = -ENOSPC ; + goto fail; + } + n =get_user_pages (current, current->mm, base, npages, +0 ,0 , pages,NULL ); +if (unlikely (n <0 )) { + err = n; + goto fail; + } + +/* Transfer pages to the frag array */ +for (j =0 ; j < n; j++) { + f[num_pg].page = pages[j]; +if (j ==0 ) { + f[num_pg].page_offset =offset_in_page (base); + f[num_pg].size =min (len,PAGE_SIZE - + f[num_pg].page_offset); + }else { + f[num_pg].page_offset =0 ; + f[num_pg].size =min (len,PAGE_SIZE ); + } + len -= f[num_pg].size; + base += f[num_pg].size; + num_pg++; + } + +if (unlikely (n != npages)) { + err = -EFAULT ; + goto fail; + } + } +up_read (¤t->mm->mmap_sem); +return num_pg; + +fail: +for (i =0 ; i < num_pg; i++) +put_page (f[i].page); +up_read (¤t->mm->mmap_sem); +return err; +} + +/* We actually store this at the head of the skb. */ +struct skb_tun_hdr { +struct list_head list; +struct tun_struct *tun; +unsigned int id; +unsigned int len; +}; + +/* Get packet from user space buffer. copylen is a hint as to how +* much to copy (rest is pinned). */ +static struct sk_buff *get_user_skb (struct tun_struct *tun,struct iovec *iv, +size_t copylen,size_t len) +{ +struct tun_pi pi = {0 ,__constant_htons (ETH_P_IP ) }; +struct sk_buff *skb; +size_t align =0 , extra =0 ; +int err; + +if (!(tun->flags &TUN_NO_PI )) { +if (len <sizeof (pi)) { + err = -EINVAL ; + goto fail; + } + len -=sizeof (pi); + +if (memcpy_fromiovec ((void *)&pi, iv,sizeof (pi))) { + err = -EFAULT ; + goto fail; + } +if (copylen > len) + copylen = len; + } + +if ((tun->flags &TUN_TYPE_MASK ) ==TUN_TAP_DEV ) { + align =NET_IP_ALIGN ; +if (unlikely (copylen <ETH_HLEN )) { +if (len <ETH_HLEN ) { + err = -EINVAL ; + goto fail; + } + copylen =ETH_HLEN ; + } + } + +/* Allocate extra header if we need */ +if (copylen != len) + extra =sizeof (struct skb_tun_hdr ); + + skb =alloc_skb (extra + copylen + align,GFP_KERNEL ); +if (!skb) { + err = -ENOMEM ; + goto fail; + } + +if (extra + align) +skb_reserve (skb, extra + align); + +if (memcpy_fromiovec (skb_put (skb, copylen), iv, copylen)) { + err = -EFAULT ; + goto free_skb; + } + +switch (tun->flags &TUN_TYPE_MASK ) { +case TUN_TUN_DEV : +skb_reset_mac_header (skb); + skb->protocol = pi.proto; + skb->dev = tun->dev; +break ; +case TUN_TAP_DEV : + skb->protocol =eth_type_trans (skb, tun->dev); +break ; + }; + +if (tun->flags &TUN_NOCHECKSUM ) + skb->ip_summed =CHECKSUM_UNNECESSARY ; + +/* Anything left gets put into frags. */ +if (extra) { +struct skb_shared_info *sinfo =skb_shinfo (skb); +int err =get_user_skb_frags (iv, len - copylen, sinfo->frags); +if (err <0 ) + goto free_skb; + sinfo->nr_frags = err; + } + tun->dev->last_rx = jiffies; + + tun->dev->stats.rx_packets++; + tun->dev->stats.rx_bytes += len; + +return skb; + +free_skb: +kfree_skb (skb); +fail: + tun->dev->stats.rx_dropped++; +return ERR_PTR (err); }#if defined(CONFIG_VRING ) || defined(CONFIG_VRING_MODULE )
@@ -355,6 +521,132 @@ static struct vring_ops recvops = { .pull = pull_recv_skbs, }; +static DEFINE_SPINLOCK (finished_lock); +static LIST_HEAD (shinfo_finished_list); +static struct task_struct *shinfo_finisher; + +static void used_buffer (struct skb_tun_hdr *tunh) +{ +/* Woot, something happened. */ +vring_wake (tunh->tun->outring); + +/* Release device. Keeping this reference blocks file close. */ +dev_put (tunh->tun->dev); + +/* tunh == skb->head. */ +kfree (tunh); +} + +static int do_shinfo_finisher (void *unused) +{ +LIST_HEAD (list); +struct skb_tun_hdr *i; + +while (!kthread_should_stop ()) { +set_current_state (TASK_INTERRUPTIBLE ); + +spin_lock_irq (&finished_lock); +list_splice_init (&list, &shinfo_finished_list); +spin_unlock_irq (&finished_lock); + +if (list_empty (&list)) { +schedule (); +continue ; + } + +list_for_each_entry (i, &list, list) { +vring_used_buffer (i->tun->outring, i->id, i->len); +used_buffer (i); + } + } +return 0 ; +} + +/* We are done with this skb data: put it in the used pile. */ +static void shinfo_finished (struct skb_shared_info *sinfo) +{ +struct skb_tun_hdr *tunh = (void *)skb_shinfo_to_head (sinfo); +unsigned long flags; + +spin_lock_irqsave (&finished_lock, flags); +list_add (&tunh->list, &shinfo_finished_list); +spin_unlock_irqrestore (&finished_lock, flags); + +wake_up_process (shinfo_finisher); +} + +static int xmit_packets (void *_tun) +{ +struct tun_struct *tun = _tun; +struct iovec iov[1 +MAX_SKB_FRAGS ]; +unsigned int iovnum =ARRAY_SIZE (iov); +int id, err, wake =0 ; +unsigned long len; + +while ((id =vring_get_buffer (tun->outring,NULL ,NULL ,NULL , + iov, &iovnum, &len)) >0 ) { +struct virtio_net_hdr h; +struct sk_buff *skb; +struct skb_shared_info *shinfo; + +if (unlikely (len <sizeof (h))) +return -EINVAL ; + + err =memcpy_fromiovec ((void *)&h, iov,sizeof (h)); +if (unlikely (err)) +return -EFAULT ; + + len -=sizeof (h); +if (h.hdr_len > len) +return -EINVAL ; + +/* Without GSO, we copy entire packet. */ +if (h.gso_type ==VIRTIO_NET_HDR_GSO_NONE ) + h.hdr_len = len; + + skb =get_user_skb (tun, iov, h.hdr_len, len); +if (IS_ERR (skb)) +return PTR_ERR (skb); + +if ((h.flags &VIRTIO_NET_HDR_F_NEEDS_CSUM ) && + !skb_partial_csum_set (skb, h.csum_start, h.csum_offset)) { +kfree_skb (skb); +return -EINVAL ; + } + +/* If it has fragments, set up destructor for later. */ + shinfo =skb_shinfo (skb); +if (skb_shinfo (skb)->nr_frags) { +struct skb_tun_hdr *tunh = (void *)skb->head; + shinfo->destructor = shinfo_finished; + tunh->id = id; + tunh->len =sizeof (h) + skb->len; + }else { +vring_used_buffer (tun->outring, id,sizeof (h)+skb->len); + wake =1 ; + } +netif_rx_ni (skb); + } + +if (wake) +vring_wake (tun->outring); + +/* 0 or error. */ +return id; +} + +static struct vring_ops xmitops = { + .push = xmit_packets, +}; + +static int init_vring (void ) +{ + shinfo_finisher =kthread_run (do_shinfo_finisher,NULL ,"tun" ); +if (IS_ERR (shinfo_finisher)) +return PTR_ERR (shinfo_finisher); +return 0 ; +} +static int set_recv_vring (struct tun_struct *tun,int fd) {int err;
@@ -391,9 +685,47 @@ static void unset_vrings(struct tun_struvring_unset_ops (tun->inring); fput(tun->infile); } +if (tun->outring) { +vring_unset_ops (tun->outring); +fput (tun->outfile); + } +} + +static int set_xmit_vring (struct tun_struct *tun,int fd) +{ +int err; + +if (tun->outring) +return -EBUSY ; + + tun->outfile =fget (fd); +if (!tun->outfile) +return -EBADF ; + + tun->outring =vring_get (tun->outfile); +if (!tun->outring) { + err = -EBADF ; + goto put; + } + + err =vring_set_ops (tun->outring, &xmitops, tun); +if (err) { + tun->outring =NULL ; + goto put; + } +return 0 ; + +put: +fput (tun->outfile); + tun->outfile =NULL ; +return err; }#else /* ... !CONFIG_VRING */ static int set_recv_vring (struct tun_struct *tun,int fd) +{ +return -ENOTTY ; +} +static int set_xmit_vring (struct tun_struct *tun,int fd) {return -ENOTTY ; }
@@ -424,74 +756,26 @@ static unsigned int tun_chr_poll(structreturn mask; } -/* Get packet from user space buffer */ -static __inline__ssize_t tun_get_user (struct tun_struct *tun,struct iovec *iv,size_t count) -{ -struct tun_pi pi = {0 ,__constant_htons (ETH_P_IP ) }; -struct sk_buff *skb; -size_t len = count, align =0 ; - -if (!(tun->flags &TUN_NO_PI )) { -if ((len -=sizeof (pi)) > count) -return -EINVAL ; - -if (memcpy_fromiovec ((void *)&pi, iv,sizeof (pi))) -return -EFAULT ; - } - -if ((tun->flags &TUN_TYPE_MASK ) ==TUN_TAP_DEV ) { - align =NET_IP_ALIGN ; -if (unlikely (len <ETH_HLEN )) -return -EINVAL ; - } - -if (!(skb =alloc_skb (len + align,GFP_KERNEL ))) { - tun->dev->stats.rx_dropped++; -return -ENOMEM ; - } - -if (align) -skb_reserve (skb, align); -if (memcpy_fromiovec (skb_put (skb, len), iv, len)) { - tun->dev->stats.rx_dropped++; -kfree_skb (skb); -return -EFAULT ; - } - -switch (tun->flags &TUN_TYPE_MASK ) { -case TUN_TUN_DEV : -skb_reset_mac_header (skb); - skb->protocol = pi.proto; - skb->dev = tun->dev; -break ; -case TUN_TAP_DEV : - skb->protocol =eth_type_trans (skb, tun->dev); -break ; - }; - -if (tun->flags &TUN_NOCHECKSUM ) - skb->ip_summed =CHECKSUM_UNNECESSARY ; - -netif_rx_ni (skb); - tun->dev->last_rx = jiffies; - - tun->dev->stats.rx_packets++; - tun->dev->stats.rx_bytes += len; - -return count; -} -static ssize_t tun_chr_aio_write (struct kiocb *iocb,const struct iovec *iv,unsigned long count,loff_t pos) {struct tun_struct *tun = iocb->ki_filp->private_data; +size_t len; +struct sk_buff *skb;if (!tun)return -EBADFD ;DBG (KERN_INFO "%s: tun_chr_write %ld\n" , tun->dev->name, count); -return tun_get_user (tun, (struct iovec *) iv,iov_length (iv, count)); + len =iov_length (iv, count); + + skb =get_user_skb (tun, (struct iovec *)iv, len, len); +if (IS_ERR (skb)) +return PTR_ERR (skb); + +netif_rx_ni (skb); +return len; }/* Put packet to the user space buffer */
@@ -831,6 +1115,9 @@ static int tun_chr_ioctl(struct inode *icase TUNSETRECVVRING :return set_recv_vring (tun, arg); +case TUNSETXMITVRING : +return set_xmit_vring (tun, arg); +case SIOCGIFFLAGS : ifr.ifr_flags = tun->if_flags;if (copy_to_user ( argp, &ifr,sizeof ifr))
@@ -1078,6 +1365,12 @@ static int __init tun_init(void) ret =misc_register (&tun_miscdev);if (ret)printk (KERN_ERR "tun: Can't register misc device %d\n" ,TUN_MINOR ); +else { + ret =init_vring (); +if (ret) +misc_deregister (&tun_miscdev); + } +return ret; }
include/linux/if_tun.h +1 -0
@@ -43,6 +43,7 @@#define TUNSETLINK _IOW('T', 205, int)#define TUNSETGROUP _IOW('T', 206, int)#define TUNSETRECVVRING _IOW('T', 207, int) +#define TUNSETXMITVRING _IOW('T', 208, int)/* TUNSETIFF ifr flags */ #define IFF_TUN 0x0001
Andrew Morton
maintainerRe: [PATCH 5/5] tun: vringfd xmit support.
Quoted 23 lines · On Fri, 18 Apr 2008 14:43:24 +1000 Rusty Russell <[email protected]> wrote:
This patch modifies tun to allow a vringfd to specify the send buffer. The user does a write to push out packets from the buffer.
Again we use the 'struct
virtio_net_hdr' to allow userspace to send GSO packets. In this case, it can hint how much to copy, and the other pages will be made into skb fragments....
drivers/net/tun.c +13
-0+/* We don't consolidate consecutive iovecs, so huge iovecs can break here. +* Users will learn not to do that. */ +static int get_user_skb_frags (const struct iovec *iv,size_t len, +struct skb_frag_struct *f) +{ +unsigned int i, j, num_pg =0 ; +int err; +struct page *pages[MAX_SKB_FRAGS ]; + +down_read (¤t->mm->mmap_sem); +while (len) { +int n, npages; +unsigned long base, len;
drivers/net/tun.c static void tun_net_init(struct net_devi · line 288
Quoted 10 lines
drivers/net/tun.c +10
-0+ base = (unsigned long )iv->iov_base; + len = (unsigned long )iv->iov_len; + +if (len ==0 ) { + iv++; +continue ; + } + +/* How many pages will this take? */ + npages =1 + (base + len -1 )/PAGE_SIZE - base/PAGE_SIZE ;
Brain hurts. I hope you got that right.
drivers/net/tun.c static void tun_net_init(struct net_devi · line 294
Quoted 6 lines
drivers/net/tun.c +6
-0+if (unlikely (num_pg + npages >MAX_SKB_FRAGS )) { + err = -ENOSPC ; + goto fail; + } + n =get_user_pages (current, current->mm, base, npages, +0 ,0 , pages,NULL );
What is the maximum numbet of pages which an unpriviliged user can concurrently pin with this code?
Quoted 10 lines
drivers/net/tun.c +10
-0+if (unlikely (n <0 )) { + err = n; + goto fail; + } + +/* Transfer pages to the frag array */ +for (j =0 ; j < n; j++) { + f[num_pg].page = pages[j]; +if (j ==0 ) { + f[num_pg].page_offset =offset_in_page (base);
drivers/net/tun.c static void tun_net_init(struct net_devi · line 314
Quoted 10 lines
drivers/net/tun.c +10
-0+ f[num_pg].size =min (len,PAGE_SIZE - + f[num_pg].page_offset); + }else { + f[num_pg].page_offset =0 ; + f[num_pg].size =min (len,PAGE_SIZE ); + } + len -= f[num_pg].size; + base += f[num_pg].size; + num_pg++; + }
This loop is a fancy way of doing
num_pg = n;
drivers/net/tun.c static void tun_net_init(struct net_devi · line 319
Quoted 4 lines
drivers/net/tun.c +4
-0+if (unlikely (n != npages)) { + err = -EFAULT ; + goto fail; + }
why not do this immediately after running get_user_pages()?
drivers/net/tun.c static void tun_net_init(struct net_devi · line 326
Quoted 7 lines
drivers/net/tun.c +7
-0+ } +up_read (¤t->mm->mmap_sem); +return num_pg; + +fail: +for (i =0 ; i < num_pg; i++) +put_page (f[i].page);
release_pages() could be a tad more efficient, but it's only error-path.
Quoted 5 lines
drivers/net/tun.c +4
-0+up_read (¤t->mm->mmap_sem); +return err; +} +
Re: [PATCH 5/5] tun: vringfd xmit support.
Quoted 5 lines · On Friday 18 April 2008 21:31:20 Andrew Morton wrote:
Review
Quoted 2 lines · On Fri, 18 Apr 2008 14:43:24 +1000 Rusty Russell <[email protected]> wrote:
(quoted diff) +2
-0+ /* How many pages will this take? */ + npages = 1 + (base + len - 1)/PAGE_SIZE - base/PAGE_SIZE;Brain hurts. I hope you got that right.
I tested it when I wrote it, but just wrote a tester again:
base len npages
0 1 1 0xfff 1 1 0x1000 1 1 0 4096 1 0x1 4096 2 0xfff 4096 2 0x1000 4096 1 0xfffff000 4096 1 0xfffff000 4097 4293918722
Quoted 9 lines
Review
Quoted 6 lines
(quoted diff) +6
-0+ if (unlikely(num_pg + npages > MAX_SKB_FRAGS)) { + err = -ENOSPC; + goto fail; + } + n = get_user_pages(current, current->mm, base, npages, + 0, 0, pages, NULL);What is the maximum numbet of pages which an unpriviliged user can concurrently pin with this code?
Since only root can open the tun device, it's currently OK. The old code kmalloced and copied: is there some mm-fu reason why pinning userspace memory is worse?
But I actually think it's OK even for non-root, since these become skbs, which means they either go into an outgoing device queue or a socket queue which is accounted for exactly for this reason.
Quoted 23 lines
Quoted 10 lines
(quoted diff) +10
-0+ if (unlikely(n < 0)) { + err = n; + goto fail; + } + + /* Transfer pages to the frag array */ + for (j = 0; j < n; j++) { + f[num_pg].page = pages[j]; + if (j == 0) { + f[num_pg].page_offset = offset_in_page(base);Review
Quoted 10 lines
(quoted diff) +10
-0+ f[num_pg].size = min(len, PAGE_SIZE - + f[num_pg].page_offset); + } else { + f[num_pg].page_offset = 0; + f[num_pg].size = min(len, PAGE_SIZE); + } + len -= f[num_pg].size; + base += f[num_pg].size; + num_pg++; + }This loop is a fancy way of doing
Quoted 1 line
num_pg = n;
Damn, you had me reworking this until I realized why. It's not: we're inside a loop, doing one iovec array element at a time.
Quoted 6 lines
Review
Quoted 4 lines
(quoted diff) +4
-0+ if (unlikely(n != npages)) { + err = -EFAULT; + goto fail; + }why not do this immediately after running
get_user_pages()?
To simplify the failure path. Hmm, I would use release_pages here...
Quoted 5 lines
Review
Quoted 3 lines
(quoted diff) +3
-0+fail: + for (i = 0; i < num_pg; i++) + put_page(f[i].page);
release_pages()could be a tad more efficient, but it's only error-path.
... but I didn't know that existed. Had to include pagemap.h, and it's not exported. It seems to be a useful interface; see patch.
Cheers,
Rusty.
Subject: Export release_pages; nice undo for get_user_pages.
Andrew Morton suggests tun/tap use release_pages, but it's not exported. It's not clear to me why this is in swap.c, but it exists even without CONFIG_SWAP, so that's OK.
mm/swap.c +1 -0
@@ -346,6 +346,7 @@ void release_pages(struct page **pages,pagevec_free (&pages_to_free); } +EXPORT_SYMBOL (release_pages);/* * The pages which we're about to release may be in the deferred lru-addition
Re: [PATCH 5/5] tun: vringfd xmit support.
Quoted 10 lines · Rusty Russell
Quoted 6 lines · On Friday 18 April 2008 21:31:20 Andrew Morton wrote:
Review
Quoted 2 lines · On Fri, 18 Apr 2008 14:43:24 +1000 Rusty Russell <[email protected]> wrote:
(quoted diff) +2
-0+ /* How many pages will this take? */ + npages = 1 + (base + len - 1)/PAGE_SIZE - base/PAGE_SIZE;Brain hurts. I hope you got that right.
I tested it when I wrote it, but just wrote a tester again:
Quoted 10 lines
base len npages 0 1 1 0xfff 1 1 0x1000 1 1 0 4096 1 0x1 4096 2 0xfff 4096 2 0x1000 4096 1 0xfffff000 4096 1 0xfffff000 4097 4293918722
Picky, picky :-).
If base were always page aligned, we'd want
npages = (len + PAGE_SIZE - 1) / PAGE_SIZE;
which is simply rounding len up to the next largest page size. So when base is not page aligned, increase len by the left-over space at the beginning, and then do the same calculation as above. (ie, pretend base is page aligned, and instead count the excess at the beginning as part of len.)
npages = ( (base & PAGE_MASK) + len + PAGE_SIZE - 1) / PAGE_SIZE;
As long as len < PAGE_MASK - PAGE_SIZE, we're safe from overflows.
(The above gives a different answer when len=0, but you guard for that.)
Re: [PATCH 5/5] tun: vringfd xmit support.
In reply to Rusty RussellQuoted 7 lines · On Sat, 19 Apr 2008 01:15:15 +1000 Rusty Russell <[email protected]> wrote:
Quoted 3 lines
What is the maximum numbet of pages which an unpriviliged user can concurrently pin with this code?
Since only root can open the tun device, it's currently OK. The old code kmalloced and copied: is there some mm-fu reason why pinning userspace memory is worse?
We generally try to avoid it - it allows users to dos the box. Although I suspect that direct-io presently permits users to transiently pin an amount of memory which is proportional to the number of disks upon which they can open files.
Quoted 12 lines
Subject: Export
release_pages; nice undo forget_user_pages.Andrew Morton suggests tun/tap use
release_pages, but it's not exported. It's not clear to me why this is inswap.c, but it exists even withoutCONFIG_SWAP, so that's OK.Signed-off-by Rusty Russell <[email protected]>diff -r
abd2ad431e5cmm/swap.c
mm/swap.c void release_pages(struct page **pages, · line 352
Quoted 7 lines
mm/swap.c +1
-0@@ -346,6 +346,7 @@ void release_pages(struct page **pages,pagevec_free (&pages_to_free); } +EXPORT_SYMBOL (release_pages);/* * The pages which we're about to release may be in the deferred lru-addition
Re: [PATCH 5/5] tun: vringfd xmit support.
Quoted 1 line · On Saturday 19 April 2008 05:06:34 Andrew Morton wrote:
On Sat, 19 Apr 2008 01:15:15 +1000 Rusty Russell <[email protected]>
Quoted 8 lines · wrote:
Quoted 6 lines
Quoted 2 lines
What is the maximum numbet of pages which an unpriviliged user can concurrently pin with this code?
Since only root can open the tun device, it's currently OK. The old code kmalloced and copied: is there some mm-fu reason why pinning userspace memory is worse?
We generally try to avoid it - it allows users to dos the box.
My question is: is pinning a page worse than allocating a (kernel) page in some way?
Cheers,
Rusty.
Re: [PATCH 5/5] tun: vringfd xmit support.
In reply to Andrew MortonQuoted 1 line · On Saturday 19 April 2008 05:06:34 Andrew Morton wrote:
On Sat, 19 Apr 2008 01:15:15 +1000 Rusty Russell <[email protected]>
Quoted 8 lines · wrote:
Quoted 6 lines
Quoted 2 lines
What is the maximum numbet of pages which an unpriviliged user can concurrently pin with this code?
Since only root can open the tun device, it's currently OK. The old code kmalloced and copied: is there some mm-fu reason why pinning userspace memory is worse?
We generally try to avoid it - it allows users to dos the box.
My question is: is pinning a page worse than allocating a (kernel) page in some way?
Cheers,
Rusty.
Andrew Morton
maintainerRe: [PATCH 5/5] tun: vringfd xmit support.
Quoted 7 lines · On Sun, 20 Apr 2008 00:41:43 +1000 Rusty Russell <[email protected]> wrote:
Quoted 1 line · On Saturday 19 April 2008 05:06:34 Andrew Morton wrote:
On Sat, 19 Apr 2008 01:15:15 +1000 Rusty Russell <[email protected]>
Quoted 3 lines · wrote:
Quoted 3 lines
Quoted 2 lines
What is the maximum numbet of pages which an unpriviliged user can concurrently pin with this code?
Quoted 9 lines
Quoted 5 lines
Quoted 3 lines
Since only root can open the tun device, it's currently OK. The old code kmalloced and copied: is there some mm-fu reason why pinning userspace memory is worse?
We generally try to avoid it - it allows users to dos the box.
My question is: is pinning a page worse than allocating a (kernel) page in some way?
I guess pinning is not as bad as straight-out allocating.
Pinning is limited to the size of the program's VM. Pinning it at least pining something which is accounted and is exposed to admin tools.
But they're both pretty similar in effect and risk.
Andrew Morton
maintainerRe: [PATCH 5/5] tun: vringfd xmit support.
In reply to Rusty RussellQuoted 7 lines · On Sun, 20 Apr 2008 00:41:43 +1000 Rusty Russell <[email protected]> wrote:
Quoted 1 line · On Saturday 19 April 2008 05:06:34 Andrew Morton wrote:
On Sat, 19 Apr 2008 01:15:15 +1000 Rusty Russell <[email protected]>
Quoted 3 lines · wrote:
Quoted 3 lines
Quoted 2 lines
What is the maximum numbet of pages which an unpriviliged user can concurrently pin with this code?
Quoted 9 lines
Quoted 5 lines
Quoted 3 lines
Since only root can open the tun device, it's currently OK. The old code kmalloced and copied: is there some mm-fu reason why pinning userspace memory is worse?
We generally try to avoid it - it allows users to dos the box.
My question is: is pinning a page worse than allocating a (kernel) page in some way?
I guess pinning is not as bad as straight-out allocating.
Pinning is limited to the size of the program's VM. Pinning it at least pining something which is accounted and is exposed to admin tools.
But they're both pretty similar in effect and risk.
Andrew Morton
maintainerRe: [PATCH 5/5] tun: vringfd xmit support.
In reply to Rusty RussellQuoted 2 lines · On Sat, 19 Apr 2008 01:15:15 +1000 Rusty Russell <[email protected]> wrote:
It's not clear to me why this is in
swap.c, but it exists even withoutCONFIG_SWAP, so that's OK.
We should have done mv mm/swap.c mm/mmlib.c years ago.
Re: [PATCH 5/5] tun: vringfd xmit support.
In reply to Rusty RussellQuoted 57 lines · Rusty Russell
This patch modifies tun to allow a vringfd to specify the send
buffer. The user does a write to push out packets from the buffer.Again we use the 'struct
virtio_net_hdr' to allow userspace to send GSO packets. In this case, it can hint how much to copy, and the other pages will be made into skb fragments.Signed-off-by Rusty Russell <[email protected]>---
drivers/net/tun.c | 410 +++++++++++++++++++++++++++++++++++++++++-------- include/linux/if_tun.h | 1 2 files changed, 351 insertions(+), 60 deletions(-)diff -r
f797ec115d1bdrivers/net/tun.c
---a/drivers/net/tun.cFri Apr 18 05:58:40 2008 +1000 +++b/drivers/net/tun.cFri Apr 18 06:07:21 2008 +1000 @@ -65,6 +65,8 @@
#include <linux/vring.h>
#include <linux/virtio_net.h>
#include <linux/file.h>
+#include <linux/spinlock.h>
+#include <linux/kthread.h>
#include <net/net_namespace.h>#include <
asm/system.h>@@ -102,8 +104,8 @@ struct tun_struct { u32 chr_filter[2]; u32 net_filter[2];- struct vring_info *inring; - struct file *infile; + struct vring_info *inring, *outring; + struct file *infile, *outfile;#ifdef TUN_DEBUG int debug; @@ -258,6 +261,169 @@ static void tun_net_init(struct net_devi dev->tx_queue_len = TUN_READQ_SIZE; /* We prefer our own queue length */ break; } +} + +/* We don't consolidate consecutive iovecs, so huge iovecs can break here. + * Users will learn not to do that. */ +static int get_user_skb_frags(const struct iovec *iv, size_t len, + struct skb_frag_struct *f) +{ + unsigned int i, j, num_pg = 0; + int err; + struct page *pages[MAX_SKB_FRAGS]; + + down_read(¤t->mm->mmap_sem); + while (len) { + int n, npages; + unsigned long base, len; + base = (unsigned long)iv->iov_base; + len = (unsigned long)iv->iov_len;
Quoted 8 lines
+
+ if (len == 0) {
+ iv++;
+ continue;
+ }
+
+ /* How many pages will this take? */
+ npages = 1 + (base + len - 1)/PAGE_SIZE - base/PAGE_SIZE;
Hi Rusty,
A trivial suggestion, how about
npages = 1+(len -1)/PAGE_SIZE ?
Thanks,
--Pradeep
Quoted 446 lines
+ if (
unlikely(num_pg + npages > MAX_SKB_FRAGS)) {
+ err =-ENOSPC;
+ goto fail;
+ }
+ n = get_user_pages(current,current->mm, base, npages,
+ 0, 0, pages, NULL);
+ if (unlikely(n < 0)) {
+ err = n;
+ goto fail;
+ }
+
+ /* Transfer pages to the frag array */
+ for (j = 0; j < n; j++) {
+ f[num_pg].page = pages[j];
+ if (j == 0) {
+ f[num_pg].page_offset =offset_in_page(base);
+ f[num_pg].size = min(len,PAGE_SIZE-
+ f[num_pg].page_offset);
+ } else {
+ f[num_pg].page_offset = 0;
+ f[num_pg].size =min(len, PAGE_SIZE);+ } + len -= f[num_pg].size; + base += f[num_pg].size; + num_pg++; + } + + if (unlikely(n != npages)) { + err = -EFAULT; + goto fail; + } + } + up_read(¤t->mm->mmap_sem); + return num_pg; + +fail: + for (i = 0; i < num_pg; i++) + put_page(f[i].page); + up_read(¤t->mm->mmap_sem); + return err; +} + +/* We actually store this at the head of the skb. */ +struct skb_tun_hdr { + struct list_head list; + struct tun_struct *tun; + unsigned int id; + unsigned int len; +}; + +/* Get packet from user space buffer. copylen is a hint as to how + * much to copy (rest is pinned). */ +static struct sk_buff *get_user_skb(struct tun_struct *tun, struct iovec *iv, + size_t copylen, size_t len) +{ + struct tun_pi pi = { 0, __constant_htons(ETH_P_IP) }; + struct sk_buff *skb; + size_t align = 0, extra = 0; + int err; + + if (!(tun->flags & TUN_NO_PI)) { + if (len < sizeof(pi)) { + err = -EINVAL; + goto fail; + } + len -= sizeof(pi); + + if (memcpy_fromiovec((void *)&pi, iv, sizeof(pi))) { + err = -EFAULT; + goto fail; + } + if (copylen > len) + copylen = len; + } + + if ((tun->flags & TUN_TYPE_MASK) == TUN_TAP_DEV) { + align = NET_IP_ALIGN; + if (unlikely(copylen < ETH_HLEN)) { + if (len < ETH_HLEN) { + err = -EINVAL; + goto fail; + } + copylen = ETH_HLEN; + } + } + + /* Allocate extra header if we need */ + if (copylen != len) + extra = sizeof(struct skb_tun_hdr); + + skb = alloc_skb(extra + copylen + align, GFP_KERNEL); + if (!skb) { + err = -ENOMEM; + goto fail; + } + + if (extra + align) + skb_reserve(skb, extra + align); + + if (memcpy_fromiovec(skb_put(skb, copylen), iv, copylen)) { + err = -EFAULT; + goto free_skb; + } + + switch (tun->flags & TUN_TYPE_MASK) { + case TUN_TUN_DEV: + skb_reset_mac_header(skb); + skb->protocol = pi.proto; + skb->dev = tun->dev; + break; + case TUN_TAP_DEV: + skb->protocol = eth_type_trans(skb, tun->dev); + break; + }; + + if (tun->flags & TUN_NOCHECKSUM) + skb->ip_summed = CHECKSUM_UNNECESSARY; + + /* Anything left gets put into frags. */ + if (extra) { + struct skb_shared_info *sinfo = skb_shinfo(skb); + int err = get_user_skb_frags(iv, len - copylen, sinfo->frags); + if (err < 0)+ goto
free_skb;
+sinfo->nr_frags= err;
+ }
+tun->dev->last_rx= jiffies;
+
+ tun->dev->stats.rx_packets++;
+tun->dev->stats.rx_bytes+= len;
+
+ return skb;
+
+free_skb:
+kfree_skb(skb);
+fail:
+ tun->dev->stats.rx_dropped++;
+ returnERR_PTR(err);
}#if
defined(CONFIG_VRING)||defined(CONFIG_VRING_MODULE)
@@ -355,6 +521,132 @@ static structvring_opsrecvops = {
.pull =pull_recv_skbs,
};+static
DEFINE_SPINLOCK(finished_lock); +staticLIST_HEAD(shinfo_finished_list); +static structtask_struct*shinfo_finisher; +
+static voidused_buffer(struct skb_tun_hdr *tunh)+{
+ /* Woot, something happened. */
+vring_wake(tunh->tun->outring);
+
+ /* Release device. Keeping this reference blocks file close. */
+dev_put(tunh->tun->dev);
+
+ /* tunh ==skb->head. */
+ kfree(tunh);
+}
+
+static intdo_shinfo_finisher(void *unused)+{
+LIST_HEAD(list);
+ structskb_tun_hdr*i;
+
+ while (!kthread_should_stop()) {
+set_current_state(TASK_INTERRUPTIBLE);
++ spin_lock_irq(&finished_lock); + list_splice_init(&list, &shinfo_finished_list); + spin_unlock_irq(&finished_lock); + + if (list_empty(&list)) { + schedule(); + continue; + } + + list_for_each_entry(i, &list, list) { + vring_used_buffer(i->tun->outring, i->id, i->len); + used_buffer(i); + } + } + return 0; +} + +/* We are done with this skb data: put it in the used pile. */ +static void shinfo_finished(struct skb_shared_info *sinfo) +{ + struct skb_tun_hdr *tunh = (void *)skb_shinfo_to_head(sinfo); + unsigned long flags; + + spin_lock_irqsave(&finished_lock, flags); + list_add(&tunh->list, &shinfo_finished_list); + spin_unlock_irqrestore(&finished_lock, flags); + + wake_up_process(shinfo_finisher); +} + +static int xmit_packets(void *_tun) +{ + struct tun_struct *tun = _tun; + struct iovec iov[1+MAX_SKB_FRAGS]; + unsigned int iovnum = ARRAY_SIZE(iov); + int id, err, wake = 0; + unsigned long len; + + while ((id = vring_get_buffer(tun->outring, NULL, NULL, NULL, + iov, &iovnum, &len)) > 0) { + struct virtio_net_hdr h; + struct sk_buff *skb; + struct skb_shared_info *shinfo; + + if (unlikely(len < sizeof(h))) + return -EINVAL; + + err = memcpy_fromiovec((void *)&h, iov, sizeof(h)); + if (unlikely(err)) + return -EFAULT; + + len -= sizeof(h); + if (h.hdr_len > len) + return -EINVAL; + + /* Without GSO, we copy entire packet. */ + if (h.gso_type == VIRTIO_NET_HDR_GSO_NONE) + h.hdr_len = len; + + skb = get_user_skb(tun, iov, h.hdr_len, len); + if (IS_ERR(skb)) + return PTR_ERR(skb); + + if ((h.flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) && + !skb_partial_csum_set(skb, h.csum_start, h.csum_offset)) { + kfree_skb(skb); + return -EINVAL; + } + + /* If it has fragments, set up destructor for later. */ + shinfo = skb_shinfo(skb); + if (skb_shinfo(skb)->nr_frags) { + struct skb_tun_hdr *tunh = (void *)skb->head; + shinfo->destructor = shinfo_finished; + tunh->id = id; + tunh->len = sizeof(h) + skb->len; + } else { + vring_used_buffer(tun->outring, id, sizeof(h)+skb->len); + wake = 1; + } + netif_rx_ni(skb); + } + + if (wake) + vring_wake(tun->outring); + + /* 0 or error. */ + return id; +} + +static struct vring_ops xmitops = { + .push = xmit_packets, +}; + +static int init_vring(void) +{ + shinfo_finisher = kthread_run(do_shinfo_finisher, NULL, "tun"); + if (IS_ERR(shinfo_finisher)) + return PTR_ERR(shinfo_finisher); + return 0; +} + static int set_recv_vring(struct tun_struct *tun, int fd) { int err; @@ -391,9 +685,47 @@ static void unset_vrings(struct tun_stru vring_unset_ops(tun->inring); fput(tun->infile); } + if (tun->outring) { + vring_unset_ops(tun->outring); + fput(tun->outfile); + } +} + +static int set_xmit_vring(struct tun_struct *tun, int fd) +{ + int err; + + if (tun->outring) + return -EBUSY; + + tun->outfile = fget(fd); + if (!tun->outfile) + return -EBADF; + + tun->outring = vring_get(tun->outfile); + if (!tun->outring) { + err = -EBADF; + goto put; + } + + err = vring_set_ops(tun->outring, &xmitops, tun); + if (err) { + tun->outring = NULL; + goto put; + } + return 0; + +put: + fput(tun->outfile); + tun->outfile = NULL; + return err; } #else /* ... !CONFIG_VRING */ static int set_recv_vring(struct tun_struct *tun, int fd) +{ + return -ENOTTY; +} +static int set_xmit_vring(struct tun_struct *tun, int fd) { return -ENOTTY; } @@ -424,74 +756,26 @@ static unsigned int tun_chr_poll(struct return mask;}
-/* Get packet from user space buffer */
-static__inline__ssize_ttun_get_user(struct tun_struct *tun, struct iovec *iv, size_t count)-{- struct tun_pi pi = { 0, __constant_htons(ETH_P_IP) }; - struct sk_buff *skb; - size_t len = count, align = 0; - - if (!(tun->flags & TUN_NO_PI)) { - if ((len -= sizeof(pi)) > count) - return -EINVAL; - - if(memcpy_fromiovec((void *)&pi, iv, sizeof(pi))) - return -EFAULT; - } - - if ((tun->flags & TUN_TYPE_MASK) == TUN_TAP_DEV) { - align = NET_IP_ALIGN; - if (unlikely(len < ETH_HLEN)) - return -EINVAL; - } - - if (!(skb = alloc_skb(len + align, GFP_KERNEL))) { - tun->dev->stats.rx_dropped++; - return -ENOMEM; - } - - if (align) - skb_reserve(skb, align); - if (memcpy_fromiovec(skb_put(skb, len), iv, len)) { - tun->dev->stats.rx_dropped++; - kfree_skb(skb); - return -EFAULT; - } - - switch (tun->flags & TUN_TYPE_MASK) { - case TUN_TUN_DEV: - skb_reset_mac_header(skb); - skb->protocol = pi.proto; - skb->dev = tun->dev; - break; - case TUN_TAP_DEV: - skb->protocol = eth_type_trans(skb, tun->dev); - break; - }; - - if (tun->flags & TUN_NOCHECKSUM) - skb->ip_summed = CHECKSUM_UNNECESSARY; - - netif_rx_ni(skb); - tun->dev->last_rx = jiffies; - - tun->dev->stats.rx_packets++; - tun->dev->stats.rx_bytes += len; - - return count; -} - static ssize_t tun_chr_aio_write(struct kiocb *iocb, const struct iovec *iv, unsigned long count, loff_t pos) { struct tun_struct *tun = iocb->ki_filp->private_data;+
size_tlen;
+ structsk_buff*skb;if (!tun) return -EBADFD; DBG(KERN_INFO "%s: tun_chr_write %ld\n", tun->dev->name, count);- return
tun_get_user(tun, (struct iovec *) iv, iov_length(iv, count));
+ len =iov_length(iv, count);
+
+ skb =get_user_skb(tun, (struct iovec *)iv, len, len);
+ if (IS_ERR(skb))
+ returnPTR_ERR(skb);
+
+netif_rx_ni(skb);
+ return len;
}/* Put packet to the user space buffer */
@@ -831,6 +1115,9 @@ static int tun_chr_ioctl(struct inode *icase TUNSETRECVVRING: return set_recv_vring(tun, arg);+ case TUNSETXMITVRING:
+ returnset_xmit_vring(tun, arg);
+
case SIOCGIFFLAGS:
ifr.ifr_flags =tun->if_flags;
if (copy_to_user( argp, &ifr, sizeof ifr))
@@ -1078,6 +1365,12 @@ static int__inittun_init(void)ret = misc_register(&tun_miscdev); if (ret) printk(KERN_ERR "tun: Can't register misc device %d\n", TUN_MINOR); + else { + ret = init_vring(); + if (ret) + misc_deregister(&tun_miscdev); + } + return ret;}
diff -r
f797ec115d1binclude/linux/if_tun.h
---a/include/linux/if_tun.hFri Apr 18 05:58:40 2008 +1000
+++b/include/linux/if_tun.hFri Apr 18 06:07:21 2008 +1000
@@ -43,6 +43,7 @@
#define TUNSETLINK_IOW('T', 205, int)
#define TUNSETGROUP_IOW('T', 206, int)
#define TUNSETRECVVRING_IOW('T', 207, int)
+#define TUNSETXMITVRING_IOW('T', 208, int)/* TUNSETIFF ifr flags */
#defineIFF_TUN0x0001
_______________________________________________
Virtualization mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/virtualization
Signature · 3 lines
Pradeep Singh Rautela http://eagain.wordpress.com http://emptydomain.googlepages.com
Re: [PATCH 5/5] tun: vringfd xmit support.
Quoted 8 lines · pradeep singh rautela
Quoted 2 lines · Rusty Russell
+ /* How many pages will this take? */
+ npages = 1 + (base + len - 1)/PAGE_SIZE - base/PAGE_SIZE;Hi Rusty,
A trivial suggestion, how aboutnpages = 1+(len -1)/PAGE_SIZE ?
That's not the same. In particular, his version accounts for the fractional page at the beginning, while yours doesn't.
While it's tempting to use algebra to simplify things, it's not safe to do so when the expression involves division over the integers. The only care-free integer math in a computer is subtraction and ++.
Re: [PATCH 5/5] tun: vringfd xmit support.
Quoted 8 lines · Ray Lee
Quoted 6 lines · pradeep singh rautela
Quoted 2 lines · Rusty Russell
+ /* How many pages will this take? */
+ npages = 1 + (base + len - 1)/PAGE_SIZE - base/PAGE_SIZE;
Quoted 6 lines
Quoted 3 lines
Hi Rusty,
A trivial suggestion, how aboutnpages = 1+(len -1)/PAGE_SIZE ?That's not the same. In particular, his version accounts for the fractional page at the beginning, while yours doesn't.
Oh thanks for correcting me Ray. :)
Quoted 4 lines
While it's tempting to use algebra to simplify things, it's not safe to do so when the expression involves division over the integers. The only care-free integer math in a computer is subtraction and ++.
I stand corrected.
Sorry for noise.
Thanks,
Signature · 3 lines
Pradeep Singh Rautela http://eagain.wordpress.com http://emptydomain.googlepages.com
Andrew Morton
maintainerRe: [PATCH 2/5] /dev/vring: simple userspace-kernel ringbuffer interface.
In reply to Rusty RussellQuoted 10 lines · On Fri, 18 Apr 2008 14:39:48 +1000 Rusty Russell <[email protected]> wrote:
virtio introduced a ring structure ABI for guest-host communications (currently used by lguest and kvm). Using this same ABI, we can create a nice fd version.
This is useful for efficiently passing packets to and from the tun, for example.
...
drivers/char/vring.c new file +1
-0+static int vring_mmap (struct file *filp,struct vm_area_struct *vma)
drivers/char/vring.c line 131
Quoted 10 lines
drivers/char/vring.c new file +10
-0+{ +unsigned long size, num_descs; +struct vring_info *vr = filp->private_data; +int err; + +/* We overload mmap's offset to hold the ring number. */ + num_descs = vma->vm_pgoff; + +/* Must be a power of two, and limit indices to a u16. */ +if (!num_descs || (num_descs & (num_descs-1 )) || num_descs >65536 )
We have an is_power_of_2().
Quoted 28 lines
drivers/char/vring.c new file +25
-0+return -EINVAL ; + +/* mmap size must be what we expect for such a ring. */ + size = vma->vm_end - vma->vm_start; +if (size !=ALIGN (vring_size (num_descs,PAGE_SIZE ),PAGE_SIZE )) +return -EINVAL ; + +/* We only let them map this in one place. */ +mutex_lock (&vr->lock); +if (vr->ring.num !=0 ) { + err = -EBUSY ; + goto unlock; + } + +vring_init (&vr->ring, num_descs, (void *)vma->vm_start,PAGE_SIZE ); + + vr->mask = num_descs -1 ; + err =0 ; + +unlock: +mutex_unlock (&vr->lock); +return err; +} +/** +* vring_get - check out a vring file descriptor
drivers/char/vring.c line 192
Quoted 10 lines
drivers/char/vring.c new file +10
-0+* @filp: the file structure to attach to (eg. from fget()). +* +* Userspace opens /dev/vring and mmaps it, then hands that fd to the +* kernel subsystem it wants to communicate with. That subsystem uses +* this routine and vring_set_ops() to attach to it. +* +* This simply checks that it really is a vring fd (otherwise it +* returns NULL), the other routine checks that it's not already +* attached. +*/
hm, I don't understand the big picture here yet.
Isn't this kinda-sorta like what a relayfs file does? The oprofile buffers? etc? Nothing in common at all, no hope?
Quoted 34 lines
drivers/char/vring.c new file +34
-0+struct vring_info *vring_get (struct file *filp) +{ +/* Must be one of ours. */ +if (filp->f_op != &vring_fops) +return NULL ; + +return filp->private_data; +} +EXPORT_SYMBOL_GPL (vring_get); + +/** +* vring_set_ops - attach operations to a vring file descriptor. +* @vr: the vring_info returned from vring_get. +* @ops: the operations to attach. +* @ops_data: the argument to the ops callbacks. +* +* This is called after vring_get(): the reason for the two-part +* process is that the ops can be called before vring_set_ops returns +* (we don't do locking), so you really need to set things up before +* this call. +* +* This simply checks that the ring is not already attached to something, +* then sets the ops. +*/ +int vring_set_ops (struct vring_info *vr, +const struct vring_ops *ops,void *ops_data) +{ +int err; + +mutex_lock (&vr->lock); +if (vr->ops) { + err = -EBUSY ; + goto unlock; + }
drivers/char/vring.c line 365
Quoted 10 lines
drivers/char/vring.c new file +10
-0+ +/* We don't lock, so make sure we get this in the right order. */ + vr->ops_data = ops_data; +wmb (); + vr->ops = ops; + + err =0 ; +unlock: +mutex_unlock (&vr->lock); +local_irq_enable ();
what's this doing here?
Quoted 6 lines
drivers/char/vring.c new file +6
-0+return err; +} +EXPORT_SYMBOL_GPL (vring_set_ops); + +/** +* vring_unset_ops - remove operations to a vring file descriptor.
drivers/char/vring.c line 381
Quoted 10 lines
drivers/char/vring.c new file +10
-0+* @vr: the vring_info previously successfully vring_set_ops'd +*/ +void vring_unset_ops (struct vring_info *vr) +{ +BUG_ON (!vr->ops); +mutex_lock (&vr->lock); + vr->ops =NULL ; +mutex_unlock (&vr->lock); +} +EXPORT_SYMBOL_GPL (vring_unset_ops);
Isn't this just vring_set_ops(vr, NULL, NULL)?
Quoted 30 lines
drivers/char/vring.c new file +18
-0+static struct miscdevice vring_dev = { + .minor =MISC_DYNAMIC_MINOR , + .name =KBUILD_MODNAME , + .fops = &vring_fops, +}; + +static int __init init (void ) +{ +return misc_register (&vring_dev); +} + +static void __exit fini(void) +{ +misc_deregister (&vring_dev); +} + +module_init (init); +module_exit (fini);diff -r
b2d9869d338finclude/linux/vring.hinclude/linux/vring.h new file +8
-0@@ -0,0 +1,58 @@ +/* Ring-buffer file descriptor implementation. +* +* Copyright 2008 Rusty Russell IBM Corporation +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation; either version 2 of the License, or +* (at your option) any later version.
include/linux/vring.h line 18
Quoted 10 lines
include/linux/vring.h new file +10
-0+* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, write to the Free Software +* Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA +*/
ponders #include <copyright.h>
Quoted 16 lines
include/linux/vring.h new file +16
-0+#ifndef _LINUX_VRING_H +#define _LINUX_VRING_H + +/** + * vring_ops - operations for a vring fd. + * @needs_pull: more data is pending, need to call pull. + * @pull: callback when read() is called to report used buffers. + * @push: callback when write() is called to notify of added buffers. + * + * Any of these callbacks can be NULL, if you don't need them. + */ +struct vring_ops { + bool (*needs_pull)(void *ops_data); + + /* Returns 0 or negative errno. */ + int (*pull)(void *ops_data);
include/linux/vring.h line 44
Quoted 10 lines
include/linux/vring.h new file +10
-0+ +/* Returns 0 or negative errno. */ +int (*push)(void *ops_data); +}; + +struct file ; + +struct vring_info *vring_get (struct file *filp); +int vring_set_ops (struct vring_info *, +const struct vring_ops *ops,void *ops_data);
the first arg to vring_set_ops() lost its name.
Quoted 14 lines
include/linux/vring.h new file +14
-0+void vring_unset_ops (struct vring_info *vr); +struct iovec ; + +/* Returns an error, or 0 (no buffers), or an id for vring_used_buffer() */ +int vring_get_buffer (struct vring_info *vr, +struct iovec *in_iov, +unsigned int *num_in,unsigned long *in_len, +struct iovec *out_iov, +unsigned int *num_out,unsigned long *out_len); + +void vring_used_buffer (struct vring_info *vr,int id,u32 len); + +void vring_wake (struct vring_info *vr); +#endif /* _LINUX_VRING_H */
Andrew Morton
maintainerRe: [PATCH 2/5] /dev/vring: simple userspace-kernel ringbuffer interface.
In reply to Rusty RussellQuoted 10 lines · On Fri, 18 Apr 2008 14:39:48 +1000 Rusty Russell <[email protected]> wrote:
virtio introduced a ring structure ABI for guest-host communications (currently used by lguest and kvm). Using this same ABI, we can create a nice fd version.
This is useful for efficiently passing packets to and from the tun, for example.
...
drivers/char/vring.c new file +1
-0+static int vring_mmap (struct file *filp,struct vm_area_struct *vma)
drivers/char/vring.c line 131
Quoted 10 lines
drivers/char/vring.c new file +10
-0+{ +unsigned long size, num_descs; +struct vring_info *vr = filp->private_data; +int err; + +/* We overload mmap's offset to hold the ring number. */ + num_descs = vma->vm_pgoff; + +/* Must be a power of two, and limit indices to a u16. */ +if (!num_descs || (num_descs & (num_descs-1 )) || num_descs >65536 )
We have an is_power_of_2().
Quoted 28 lines
drivers/char/vring.c new file +25
-0+return -EINVAL ; + +/* mmap size must be what we expect for such a ring. */ + size = vma->vm_end - vma->vm_start; +if (size !=ALIGN (vring_size (num_descs,PAGE_SIZE ),PAGE_SIZE )) +return -EINVAL ; + +/* We only let them map this in one place. */ +mutex_lock (&vr->lock); +if (vr->ring.num !=0 ) { + err = -EBUSY ; + goto unlock; + } + +vring_init (&vr->ring, num_descs, (void *)vma->vm_start,PAGE_SIZE ); + + vr->mask = num_descs -1 ; + err =0 ; + +unlock: +mutex_unlock (&vr->lock); +return err; +} +/** +* vring_get - check out a vring file descriptor
drivers/char/vring.c line 192
Quoted 10 lines
drivers/char/vring.c new file +10
-0+* @filp: the file structure to attach to (eg. from fget()). +* +* Userspace opens /dev/vring and mmaps it, then hands that fd to the +* kernel subsystem it wants to communicate with. That subsystem uses +* this routine and vring_set_ops() to attach to it. +* +* This simply checks that it really is a vring fd (otherwise it +* returns NULL), the other routine checks that it's not already +* attached. +*/
hm, I don't understand the big picture here yet.
Isn't this kinda-sorta like what a relayfs file does? The oprofile buffers? etc? Nothing in common at all, no hope?
Quoted 34 lines
drivers/char/vring.c new file +34
-0+struct vring_info *vring_get (struct file *filp) +{ +/* Must be one of ours. */ +if (filp->f_op != &vring_fops) +return NULL ; + +return filp->private_data; +} +EXPORT_SYMBOL_GPL (vring_get); + +/** +* vring_set_ops - attach operations to a vring file descriptor. +* @vr: the vring_info returned from vring_get. +* @ops: the operations to attach. +* @ops_data: the argument to the ops callbacks. +* +* This is called after vring_get(): the reason for the two-part +* process is that the ops can be called before vring_set_ops returns +* (we don't do locking), so you really need to set things up before +* this call. +* +* This simply checks that the ring is not already attached to something, +* then sets the ops. +*/ +int vring_set_ops (struct vring_info *vr, +const struct vring_ops *ops,void *ops_data) +{ +int err; + +mutex_lock (&vr->lock); +if (vr->ops) { + err = -EBUSY ; + goto unlock; + }
drivers/char/vring.c line 365
Quoted 10 lines
drivers/char/vring.c new file +10
-0+ +/* We don't lock, so make sure we get this in the right order. */ + vr->ops_data = ops_data; +wmb (); + vr->ops = ops; + + err =0 ; +unlock: +mutex_unlock (&vr->lock); +local_irq_enable ();
what's this doing here?
Quoted 6 lines
drivers/char/vring.c new file +6
-0+return err; +} +EXPORT_SYMBOL_GPL (vring_set_ops); + +/** +* vring_unset_ops - remove operations to a vring file descriptor.
drivers/char/vring.c line 381
Quoted 10 lines
drivers/char/vring.c new file +10
-0+* @vr: the vring_info previously successfully vring_set_ops'd +*/ +void vring_unset_ops (struct vring_info *vr) +{ +BUG_ON (!vr->ops); +mutex_lock (&vr->lock); + vr->ops =NULL ; +mutex_unlock (&vr->lock); +} +EXPORT_SYMBOL_GPL (vring_unset_ops);
Isn't this just vring_set_ops(vr, NULL, NULL)?
Quoted 30 lines
drivers/char/vring.c new file +18
-0+static struct miscdevice vring_dev = { + .minor =MISC_DYNAMIC_MINOR , + .name =KBUILD_MODNAME , + .fops = &vring_fops, +}; + +static int __init init (void ) +{ +return misc_register (&vring_dev); +} + +static void __exit fini(void) +{ +misc_deregister (&vring_dev); +} + +module_init (init); +module_exit (fini);diff -r
b2d9869d338finclude/linux/vring.hinclude/linux/vring.h new file +8
-0@@ -0,0 +1,58 @@ +/* Ring-buffer file descriptor implementation. +* +* Copyright 2008 Rusty Russell IBM Corporation +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation; either version 2 of the License, or +* (at your option) any later version.
include/linux/vring.h line 18
Quoted 10 lines
include/linux/vring.h new file +10
-0+* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, write to the Free Software +* Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA +*/
ponders #include <copyright.h>
Quoted 16 lines
include/linux/vring.h new file +16
-0+#ifndef _LINUX_VRING_H +#define _LINUX_VRING_H + +/** + * vring_ops - operations for a vring fd. + * @needs_pull: more data is pending, need to call pull. + * @pull: callback when read() is called to report used buffers. + * @push: callback when write() is called to notify of added buffers. + * + * Any of these callbacks can be NULL, if you don't need them. + */ +struct vring_ops { + bool (*needs_pull)(void *ops_data); + + /* Returns 0 or negative errno. */ + int (*pull)(void *ops_data);
include/linux/vring.h line 44
Quoted 10 lines
include/linux/vring.h new file +10
-0+ +/* Returns 0 or negative errno. */ +int (*push)(void *ops_data); +}; + +struct file ; + +struct vring_info *vring_get (struct file *filp); +int vring_set_ops (struct vring_info *, +const struct vring_ops *ops,void *ops_data);
the first arg to vring_set_ops() lost its name.
Quoted 14 lines
include/linux/vring.h new file +14
-0+void vring_unset_ops (struct vring_info *vr); +struct iovec ; + +/* Returns an error, or 0 (no buffers), or an id for vring_used_buffer() */ +int vring_get_buffer (struct vring_info *vr, +struct iovec *in_iov, +unsigned int *num_in,unsigned long *in_len, +struct iovec *out_iov, +unsigned int *num_out,unsigned long *out_len); + +void vring_used_buffer (struct vring_info *vr,int id,u32 len); + +void vring_wake (struct vring_info *vr); +#endif /* _LINUX_VRING_H */
Re: [PATCH 2/5] /dev/vring: simple userspace-kernel ringbuffer interface.
Quoted 4 lines · On Friday 18 April 2008 21:18:46 Andrew Morton wrote:
drivers/char/vring.c line 131
Quoted 2 lines
drivers/char/vring.c new file +2
-0+/* Must be a power of two, and limit indices to a u16. */ +if (!num_descs || (num_descs & (num_descs-1 )) || num_descs >65536 )We have an
is_power_of_2().
Thanks, fixed.
Quoted 12 lines
Quoted 11 lines
+ *
vring_get- check out a vring file descriptor
+ * @filp: the file structure to attach to (eg. fromfget()).
+ *
+ * Userspace opens/dev/vringand mmaps it, then hands that fd to the
+ * kernel subsystem it wants to communicate with. That subsystem uses
+ * this routine andvring_set_ops()to attach to it.
+ *
+ * This simply checks that it really is a vring fd (otherwise it
+ * returns NULL), the other routine checks that it's not already
+ * attached.
+ */
Quoted 4 lines
hm, I don't understand the big picture here yet.
Isn't this kinda-sorta like what a relayfs file does? The oprofile buffers? etc? Nothing in common at all, no hope?
An excellent question, but I thought the modern kernel etiquette was to only comment on whitespace and formatting, and call it "review"? :)
Yes, kinda-sorta in that it's a ring buffer. No, in that it's bidir and consumption can be out-of-order (kind of important for I/O buffers).
But the reason I'm not proposing it as a syscall is that I'm not convinced it's the One True Solution which everyone should be using. Time will tell:
it's clearly not tied to tun and it's been generically useful for virtual I/O, but history has not been kind to new userspace interfaces.
Quoted 4 lines
drivers/char/vring.c line 365
Quoted 2 lines
drivers/char/vring.c new file +2
-0+mutex_unlock (&vr->lock); +local_irq_enable ();what's this doing here?
Snot from previous version. Removed.
Quoted 10 lines
drivers/char/vring.c line 381
Quoted 8 lines
drivers/char/vring.c new file +8
-0+void vring_unset_ops (struct vring_info *vr) +{ +BUG_ON (!vr->ops); +mutex_lock (&vr->lock); + vr->ops =NULL ; +mutex_unlock (&vr->lock); +} +EXPORT_SYMBOL_GPL (vring_unset_ops);Isn't this just
vring_set_ops(vr, NULL, NULL)?
Yes, except I like the clarity and the BUG_ON.
Quoted 1 line
ponders #include <
copyright.h>
"#include <copyright-gpl2-plus>" for me, just to add more inclement weather to that teacup...
Thanks,
Rusty.
Andrew Morton
maintainerRe: [PATCH 2/5] /dev/vring: simple userspace-kernel ringbuffer interface.
Quoted 6 lines · On Sat, 19 Apr 2008 00:32:39 +1000 Rusty Russell <[email protected]> wrote:
Quoted 2 lines
Isn't this kinda-sorta like what a relayfs file does? The oprofile buffers? etc? Nothing in common at all, no hope?
An excellent question, but I thought the modern kernel etiquette was to only comment on whitespace and formatting, and call it "review"? :)
Quoted 7 lines
Yes, kinda-sorta in that it's a ring buffer. No, in that it's bidir and consumption can be out-of-order (kind of important for I/O buffers).
But the reason I'm not proposing it as a syscall is that I'm not convinced it's the One True Solution which everyone should be using. Time will tell:
it's clearly not tied to tun and it's been generically useful for virtual I/O, but history has not been kind to new userspace interfaces.
This is may be our third high-bandwidth user/kernel interface to transport bulk data ("hbukittbd") which was implemented because its predecessors weren't quite right. In a year or two's time someone else will need a hbukittbd and will find that the existing three aren't quite right and will give us another one. One day we need to stop doing this ;)
It could be that this person will look at Rusty's hbukittbd and find that it _could_ be tweaked to do what he wants, but it's already shipping and it's part of the kernel API and hence can't be made to do what he wants.
So I think it would be good to plonk the proposed interface on the table and have a poke at it. Is it compat-safe? Is it extensible in a backward-compatible fashion? Are there future-safe changes we should make to it? Can Michael Kerrisk understand, review and document it? etc.
You know what I'm saying ;) What is the proposed interface?
Re: [PATCH 2/5] /dev/vring: simple userspace-kernel ringbuffer interface.
Quoted 23 lines · Andrew Morton
Quoted 6 lines · On Sat, 19 Apr 2008 00:32:39 +1000 Rusty Russell <[email protected]> wrote:
Quoted 2 lines
Isn't this kinda-sorta like what a relayfs file does? The oprofile buffers? etc? Nothing in common at all, no hope?
An excellent question, but I thought the modern kernel etiquette was to only comment on whitespace and formatting, and call it "review"? :)
Quoted 7 lines
Yes, kinda-sorta in that it's a ring buffer. No, in that it's bidir and consumption can be out-of-order (kind of important for I/O buffers).
But the reason I'm not proposing it as a syscall is that I'm not convinced it's the One True Solution which everyone should be using. Time will tell:
it's clearly not tied to tun and it's been generically useful for virtual I/O, but history has not been kind to new userspace interfaces.This is may be our third high-bandwidth user/kernel interface to transport
bulk data ("hbukittbd") which was implemented because its predecessors weren't quite right. In a year or two's time someone else will need a hbukittbd and will find that the existing three aren't quite right and will give us another one. One day we need to stop doing this ;)
Quoted 8 lines
It could be that this person will look at Rusty's hbukittbd and find that it _could_ be tweaked to do what he wants, but it's already shipping and it's part of the kernel API and hence can't be made to do what he wants.
So I think it would be good to plonk the proposed interface on the table and have a poke at it. Is it compat-safe? Is it extensible in a backward-compatible fashion? Are there future-safe changes we should make to it? Can Michael Kerrisk understand, review and document it? etc.
Well, it helps if he's CCed....
I'm happy to work *with someone* on the documentation (pointless to do it on my own -- how do I know what Rusty's *intended* behavior for the interface is), and review, and testing.
Re: [PATCH 2/5] /dev/vring: simple userspace-kernel ringbuffer interface.
Quoted 7 lines · On Saturday 19 April 2008 05:38:50 Michael Kerrisk wrote:
Quoted 6 lines · Andrew Morton
This is may be our third high-bandwidth user/kernel interface to transport bulk data ("hbukittbd") which was implemented because its predecessors weren't quite right. In a year or two's time someone else will need a hbukittbd and will find that the existing three aren't quite right and will give us another one. One day we need to stop doing this ;)
If only there were some kind of, I don't know... summit... for kernel people...
Quoted 4 lines
Quoted 4 lines
It could be that this person will look at Rusty's hbukittbd and find
that it _could_ be tweaked to do what he wants, but it's already shipping and it's part of the kernel API and hence can't be made to do what he wants.
Indeed. I marked it experimental because of these questions (ie. it's not yet kernel ABI). Getting everyone's attention is hard tho, so I figured we put it in as a device and moving to a syscall if and when we feel it's ready.
Quoted 7 lines
Quoted 5 lines
So I think it would be good to plonk the proposed interface on the table and have a poke at it. Is it compat-safe? Is it extensible in a backward-compatible fashion? Are there future-safe changes we should
make to it? Can Michael Kerrisk understand, review and document it? etc.Well, it helps if he's CCed....
It is compat safe, and we've already extended it once, so I'm reasonably happy so far. If it were a syscall I'd add a flags arg, for the device it'd be an ioctl. Starting with the virtio ABI seemed a reasonable first step, because *we* can use this today even if noone else does.
Quoted 3 lines
I'm happy to work *with someone* on the documentation (pointless to do it on my own -- how do I know what Rusty's *intended* behavior for the interface is), and review, and testing.
Document coming up...
Rusty.
David Miller
maintainerRe: [PATCH 2/5] /dev/vring: simple userspace-kernel ringbuffer interface.
From: Rusty Russell <[email protected]>
Date: Sun, 20 Apr 2008 02:41:14 +1000
Quoted 2 lines
If only there were some kind of, I don't know... summit... for kernel people...
I'm starting to disbelieve the myth that because we can discuss technical issues on mailing lists, we should talk primarily about process issues during the kernel summit.
There is a distinct advantage to discussing and hashing things out in person. You can't say "screw you, your idea sucks" when you're face to face with the other person, whereas online it's way too easy.
David Miller
maintainerRe: [PATCH 2/5] /dev/vring: simple userspace-kernel ringbuffer interface.
In reply to Rusty RussellFrom: Rusty Russell <[email protected]>
Date: Sun, 20 Apr 2008 02:41:14 +1000
Quoted 2 lines
If only there were some kind of, I don't know... summit... for kernel people...
I'm starting to disbelieve the myth that because we can discuss technical issues on mailing lists, we should talk primarily about process issues during the kernel summit.
There is a distinct advantage to discussing and hashing things out in person. You can't say "screw you, your idea sucks" when you're face to face with the other person, whereas online it's way too easy.
Re: [PATCH 2/5] /dev/vring: simple userspace-kernel ringbuffer interface.
In reply to Michael KerriskQuoted 7 lines · On Saturday 19 April 2008 05:38:50 Michael Kerrisk wrote:
Quoted 6 lines · Andrew Morton
This is may be our third high-bandwidth user/kernel interface to transport bulk data ("hbukittbd") which was implemented because its predecessors weren't quite right. In a year or two's time someone else will need a hbukittbd and will find that the existing three aren't quite right and will give us another one. One day we need to stop doing this ;)
If only there were some kind of, I don't know... summit... for kernel people...
Quoted 4 lines
Quoted 4 lines
It could be that this person will look at Rusty's hbukittbd and find
that it _could_ be tweaked to do what he wants, but it's already shipping and it's part of the kernel API and hence can't be made to do what he wants.
Indeed. I marked it experimental because of these questions (ie. it's not yet kernel ABI). Getting everyone's attention is hard tho, so I figured we put it in as a device and moving to a syscall if and when we feel it's ready.
Quoted 7 lines
Quoted 5 lines
So I think it would be good to plonk the proposed interface on the table and have a poke at it. Is it compat-safe? Is it extensible in a backward-compatible fashion? Are there future-safe changes we should
make to it? Can Michael Kerrisk understand, review and document it? etc.Well, it helps if he's CCed....
It is compat safe, and we've already extended it once, so I'm reasonably happy so far. If it were a syscall I'd add a flags arg, for the device it'd be an ioctl. Starting with the virtio ABI seemed a reasonable first step, because *we* can use this today even if noone else does.
Quoted 3 lines
I'm happy to work *with someone* on the documentation (pointless to do it on my own -- how do I know what Rusty's *intended* behavior for the interface is), and review, and testing.
Document coming up...
Rusty.
Jonathan Corbet
maintainerRe: [PATCH 2/5] /dev/vring: simple userspace-kernel ringbuffer interface.
In reply to Andrew MortonQuoted 6 lines
So I think it would be good to plonk the proposed interface on the table and have a poke at it. Is it compat-safe? Is it extensible in a backward-compatible fashion? Are there future-safe changes we should make to it? Can Michael Kerrisk understand, review and document it? etc.
You know what I'm saying ;) What is the proposed interface?
So, I'm not Michael, but I *did* make an attempt to document this interface - user and kernel sides - so that it could be more easily understood:
http://lwn.net/Articles/276856/
That was the previous posting, but a quick look suggests it hasn't changed *that* much in this round.
jon
Jonathan Corbet
maintainerRe: [PATCH 2/5] /dev/vring: simple userspace-kernel ringbuffer interface.
In reply to Andrew MortonQuoted 6 lines
So I think it would be good to plonk the proposed interface on the table and have a poke at it. Is it compat-safe? Is it extensible in a backward-compatible fashion? Are there future-safe changes we should make to it? Can Michael Kerrisk understand, review and document it? etc.
You know what I'm saying ;) What is the proposed interface?
So, I'm not Michael, but I *did* make an attempt to document this interface - user and kernel sides - so that it could be more easily understood:
http://lwn.net/Articles/276856/
That was the previous posting, but a quick look suggests it hasn't changed *that* much in this round.
jon
Re: [PATCH 2/5] /dev/vring: simple userspace-kernel ringbuffer interface.
In reply to Rusty RussellHi.
drivers/char/vring.c line 202
Quoted 10 lines · Rusty Russell
drivers/char/vring.c new file +10
-0+int vring_get_buffer (struct vring_info *vr, +struct iovec *in_iov, +unsigned int *num_in,unsigned long *in_len, +struct iovec *out_iov, +unsigned int *num_out,unsigned long *out_len) +{ +unsigned int i, in =0 , out =0 ; +unsigned long dummy; +u16 avail, last_avail, head; +struct vring_desc d;
Should this whole function and vring_used_buffer() be protected with vr->lock mutex?
Quoted 99 lines
drivers/char/vring.c new file +99
-0+if (unlikely (get_user (avail, &vr->ring.avail->idx))) +return -EFAULT ; +if (unlikely (get_user (last_avail, &vring_last_avail (&vr->ring)))) +return -EFAULT ; + +if (last_avail == avail) +return 0 ; + +if (!in_len) + in_len = &dummy; +if (!out_len) + out_len = &dummy; + + *in_len = *out_len =0 ; + +if (unlikely (get_user (head, &vr->ring.avail->ring[last_avail + & vr->mask]))) +return -EFAULT ; + + i = head; +do { +if (unlikely (i >= vr->ring.num)) { +pr_debug ("vring: bad index: %u\n" , i); +return -EINVAL ; + } + +if (copy_from_user (&d, &vr->ring.desc[i],sizeof (d)) !=0 ) +return -EFAULT ; + +if (d.flags &VRING_DESC_F_WRITE ) { +/* Check for length and iovec overflows */ +if (!num_in) { +pr_debug ("vring: writable desc %u in ring %p\n" , + i, vr->ring.desc); +return -EINVAL ; + } +if (in == *num_in || *in_len + d.len < *in_len) +return -E2BIG ; + in_iov[in].iov_len = d.len; + *in_len += d.len; + in_iov[in].iov_base = (void __user *)(long )d.addr; + in++; + }else { +if (!num_out) { +pr_debug ("vring: readable desc %u in ring %p\n" , + i, vr->ring.desc); +return -EINVAL ; + } +if (out == *num_out || *out_len + d.len < *out_len) +return -E2BIG ; + out_iov[out].iov_len = d.len; + *out_len += d.len; + out_iov[out].iov_base = (void __user *)(long )d.addr; + out++; + } + + i = d.next; + }while (d.flags &VRING_DESC_F_NEXT ); + +if (num_in) + *num_in = in; +if (num_out) + *num_out = out; + + last_avail++; +put_user (last_avail, &vring_last_avail (&vr->ring)); + +/* 0 is a valid head, so add one. */ +return head +1 ; +} +EXPORT_SYMBOL_GPL (vring_get_buffer); + +/** +* vring_used_buffer - return a used buffer to the vring +* @vr: the vring +* @id: the id returned from vring_get_buffer +* @len: the total bytes *written* to the buffer +*/ +void vring_used_buffer (struct vring_info *vr,int id,u32 len) +{ +struct vring_used_elem used; +u16 used_idx; + +BUG_ON (id <=0 || id > vr->ring.num); + + used.id = id -1 ; + used.len = len; +if (get_user (used_idx, &vr->ring.used->idx) !=0 ) +return ; + +if (copy_to_user (&vr->ring.used->ring[used_idx & vr->mask], &used, +sizeof (used))) +return ; + +wmb (); + used_idx++; +put_user (used_idx, &vr->ring.used->idx); +} +EXPORT_SYMBOL_GPL (vring_used_buffer);
Signature · 1 line
Evgeniy Polyakov
Re: [PATCH 2/5] /dev/vring: simple userspace-kernel ringbuffer interface.
In reply to Rusty RussellHi.
drivers/char/vring.c line 202
Quoted 10 lines · Rusty Russell
drivers/char/vring.c new file +10
-0+int vring_get_buffer (struct vring_info *vr, +struct iovec *in_iov, +unsigned int *num_in,unsigned long *in_len, +struct iovec *out_iov, +unsigned int *num_out,unsigned long *out_len) +{ +unsigned int i, in =0 , out =0 ; +unsigned long dummy; +u16 avail, last_avail, head; +struct vring_desc d;
Should this whole function and vring_used_buffer() be protected with vr->lock mutex?
Quoted 99 lines
drivers/char/vring.c new file +99
-0+if (unlikely (get_user (avail, &vr->ring.avail->idx))) +return -EFAULT ; +if (unlikely (get_user (last_avail, &vring_last_avail (&vr->ring)))) +return -EFAULT ; + +if (last_avail == avail) +return 0 ; + +if (!in_len) + in_len = &dummy; +if (!out_len) + out_len = &dummy; + + *in_len = *out_len =0 ; + +if (unlikely (get_user (head, &vr->ring.avail->ring[last_avail + & vr->mask]))) +return -EFAULT ; + + i = head; +do { +if (unlikely (i >= vr->ring.num)) { +pr_debug ("vring: bad index: %u\n" , i); +return -EINVAL ; + } + +if (copy_from_user (&d, &vr->ring.desc[i],sizeof (d)) !=0 ) +return -EFAULT ; + +if (d.flags &VRING_DESC_F_WRITE ) { +/* Check for length and iovec overflows */ +if (!num_in) { +pr_debug ("vring: writable desc %u in ring %p\n" , + i, vr->ring.desc); +return -EINVAL ; + } +if (in == *num_in || *in_len + d.len < *in_len) +return -E2BIG ; + in_iov[in].iov_len = d.len; + *in_len += d.len; + in_iov[in].iov_base = (void __user *)(long )d.addr; + in++; + }else { +if (!num_out) { +pr_debug ("vring: readable desc %u in ring %p\n" , + i, vr->ring.desc); +return -EINVAL ; + } +if (out == *num_out || *out_len + d.len < *out_len) +return -E2BIG ; + out_iov[out].iov_len = d.len; + *out_len += d.len; + out_iov[out].iov_base = (void __user *)(long )d.addr; + out++; + } + + i = d.next; + }while (d.flags &VRING_DESC_F_NEXT ); + +if (num_in) + *num_in = in; +if (num_out) + *num_out = out; + + last_avail++; +put_user (last_avail, &vring_last_avail (&vr->ring)); + +/* 0 is a valid head, so add one. */ +return head +1 ; +} +EXPORT_SYMBOL_GPL (vring_get_buffer); + +/** +* vring_used_buffer - return a used buffer to the vring +* @vr: the vring +* @id: the id returned from vring_get_buffer +* @len: the total bytes *written* to the buffer +*/ +void vring_used_buffer (struct vring_info *vr,int id,u32 len) +{ +struct vring_used_elem used; +u16 used_idx; + +BUG_ON (id <=0 || id > vr->ring.num); + + used.id = id -1 ; + used.len = len; +if (get_user (used_idx, &vr->ring.used->idx) !=0 ) +return ; + +if (copy_to_user (&vr->ring.used->ring[used_idx & vr->mask], &used, +sizeof (used))) +return ; + +wmb (); + used_idx++; +put_user (used_idx, &vr->ring.used->idx); +} +EXPORT_SYMBOL_GPL (vring_used_buffer);
Signature · 1 line
Evgeniy Polyakov
Re: [PATCH 2/5] /dev/vring: simple userspace-kernel ringbuffer interface.
Quoted 3 lines · On Saturday 19 April 2008 20:22:15 Evgeniy Polyakov wrote:
Hi.
On Fri, Apr 18, 2008 at 02:39:48PM +1000, Rusty Russell
Quoted 5 lines · ([email protected]) wrote:
Quoted 5 lines
drivers/char/vring.c new file +5
-0+int vring_get_buffer (struct vring_info *vr, +struct iovec *in_iov, +unsigned int *num_in,unsigned long *in_len, +struct iovec *out_iov, +unsigned int *num_out,unsigned long *out_len)
Quoted 8 lines
drivers/char/vring.c line 202
Quoted 5 lines
drivers/char/vring.c new file +5
-0+{ +unsigned int i, in =0 , out =0 ; +unsigned long dummy; +u16 avail, last_avail, head; +struct vring_desc d;Should this whole function and
vring_used_buffer()be protected withvr->lockmutex?
No; it's up to the caller to make sure that they are serialized. In the case of tun that happens naturally.
There are two reasons not to grab the lock. It turns out that if we tried to lock here, we'd deadlock, since the callbacks are called under the lock. Secondly, it's possible to implement an atomic vring_used_buffer variant, which could fail: this would avoid using the thread most of the time.
Hope that helps,
Rusty.
Re: [PATCH 2/5] /dev/vring: simple userspace-kernel ringbuffer interface.
Quoted 10 lines · Rusty Russell
Quoted 2 lines
Should this whole function and
vring_used_buffer()be protected withvr->lockmutex?No; it's up to the caller to make sure that they are serialized. In the case of tun that happens naturally.
There are two reasons not to grab the lock. It turns out that if we tried to lock here, we'd deadlock, since the callbacks are called under the lock. Secondly, it's possible to implement an atomic
vring_used_buffervariant, which could fail: this would avoid using the thread most of the time.
Yep, I decided that too. But it limits its usage to tun only or any other system where only single thread picks up results, so no generic userspace ring buffers?
Signature · 1 line
Evgeniy Polyakov
Re: [PATCH 2/5] /dev/vring: simple userspace-kernel ringbuffer interface.
In reply to Rusty RussellQuoted 10 lines · Rusty Russell
Quoted 2 lines
Should this whole function and
vring_used_buffer()be protected withvr->lockmutex?No; it's up to the caller to make sure that they are serialized. In the case of tun that happens naturally.
There are two reasons not to grab the lock. It turns out that if we tried to lock here, we'd deadlock, since the callbacks are called under the lock. Secondly, it's possible to implement an atomic
vring_used_buffervariant, which could fail: this would avoid using the thread most of the time.
Yep, I decided that too. But it limits its usage to tun only or any other system where only single thread picks up results, so no generic userspace ring buffers?
Signature · 1 line
Evgeniy Polyakov
Re: [PATCH 2/5] /dev/vring: simple userspace-kernel ringbuffer interface.
Quoted 1 line · On Sunday 20 April 2008 02:33:22 Evgeniy Polyakov wrote:
On Sun, Apr 20, 2008 at 02:05:31AM +1000, Rusty Russell
Quoted 9 lines · ([email protected]) wrote:
Quoted 5 lines
There are two reasons not to grab the lock. It turns out that if we tried to lock here, we'd deadlock, since the callbacks are called under the lock. Secondly, it's possible to implement an atomic
vring_used_buffervariant, which could fail: this would avoid using the thread most of the time.Yep, I decided that too. But it limits its usage to tun only or any other system where only single thread picks up results, so no generic userspace ring buffers?
I don't think so, it just externalizes the locking. The mutex protects the attaching and detaching of the ops structure, some other lock or code protects simultenous kernel ring accesses.
Cheers,
Rusty.
PATCH 1/5 virtio: put last_used and last_avail index into ring itself.
In reply to Rusty RussellGenerally, the other end of the virtio ring doesn't need to see where you're up to in consuming the ring. However, in order for an external entity to understand it, it must be exposed. For example, if you want to save and restore a virtio_ring, but you're not the consumer because the kernel is using it directly.
Fortunately, we have room to expand: the ring is always a whole number of pages and there's hundreds of bytes of padding after the avail ring and the used ring, whatever the number of descriptors (which must be a power of 2).
We add a feature bit so the guest can tell the host that it's writing out the current value there, if it wants to use that.
--- drivers/virtio/virtio_ring.c | 20 ++++++++++++-------- include/linux/virtio_config.h | 3 +++ include/linux/virtio_ring.h | 12 +++++++++++- 3 files changed, 26 insertions(+), 9 deletions(-)
drivers/virtio/virtio_ring.c +12 -8
@@ -18,6 +18,7 @@*/ #include <linux/virtio.h> #include <linux/virtio_ring.h> +#include <linux/virtio_config.h> #include <linux/device.h> #ifdef DEBUG
@@ -51,9 +52,6 @@ struct vring_virtqueueunsigned int free_head;/* Number we've added since last sync. */ unsigned int num_added; - -/* Last used index we've seen. */ -u16 last_used_idx;/* How to notify other side. FIXME: commonalize hcalls! */ void (*notify)(struct virtqueue *vq);
@@ -173,12 +171,13 @@ static void detach_buf(struct vring_virtstatic inline bool more_used (const struct vring_virtqueue *vq) { -return vq->last_used_idx != vq->vring.used->idx; +return vring_last_used (&vq->vring) != vq->vring.used->idx; }static void *vring_get_buf (struct virtqueue *_vq,unsigned int *len) {struct vring_virtqueue *vq =to_vvq (_vq); +struct vring_used_elem *u;void *ret;unsigned int i;
@@ -195,8 +194,11 @@ static void *vring_get_buf(struct virtqureturn NULL ; } - i = vq->vring.used->ring[vq->last_used_idx%vq->vring.num].id; - *len = vq->vring.used->ring[vq->last_used_idx%vq->vring.num].len; + u = &vq->vring.used->ring[vring_last_used (&vq->vring) % vq->vring.num]; + i = u->id; + *len = u->len; +/* Make sure we don't reload i after doing checks. */ +rmb ();if (unlikely (i >= vq->vring.num)) {BAD_RING (vq,"id %u out of range\n" , i);
@@ -210,7 +212,7 @@ static void *vring_get_buf(struct virtqu/* detach_buf clears data, so grab it now. */ ret = vq->data[i];detach_buf (vq, i); - vq->last_used_idx++; +vring_last_used (&vq->vring)++;END_USE (vq);return ret; }
@@ -302,7 +304,6 @@ struct virtqueue *vring_new_virtqueue(un vq->vq.vq_ops = &vring_vq_ops; vq->notify = notify; vq->broken = false; - vq->last_used_idx =0 ; vq->num_added =0 ;#ifdef DEBUG vq->in_use = false;
@@ -318,6 +319,9 @@ struct virtqueue *vring_new_virtqueue(unfor (i =0 ; i < num-1 ; i++) vq->vring.desc[i].next = i+1 ; +/* We now publish our last_used index in the rings. */ + vdev->config->feature (vdev,VIRTIO_RING_F_PUBLISH_INDICES ); +return &vq->vq; }EXPORT_SYMBOL_GPL (vring_new_virtqueue);
include/linux/virtio_config.h +3 -0
@@ -14,6 +14,9 @@#define VIRTIO_CONFIG_S_DRIVER_OK 4/* We've given up on this device. */ #define VIRTIO_CONFIG_S_FAILED 0x80 + +/* Transport virtio feature bits (currently bits 24 to 32 are reserved +* for this), the rest are per-device feature bits. */ #ifdef __KERNEL__struct virtio_device ;
include/linux/virtio_ring.h +11 -1
@@ -23,6 +23,9 @@* when you consume a buffer. It's unreliable, so it's simply an * optimization. */ #define VRING_AVAIL_F_NO_INTERRUPT 1 + +/* We publish our last-seen used index at the end of the avail ring. */ +#define VIRTIO_RING_F_PUBLISH_INDICES 24/* Virtio ring descriptors: 16 bytes. These can chain together via "next". */ struct vring_desc
@@ -82,6 +85,7 @@ struct vring {* __u16 avail_flags; * __u16 avail_idx; * __u16 available[num]; +* __u16 last_used_idx; * * // Padding to the next page boundary. * char pad[];
@@ -90,6 +94,7 @@ struct vring {* __u16 used_flags; * __u16 used_idx; * struct vring_used_elem used[num]; +* __u16 last_avail_idx; * }; */ static inline void vring_init(struct vring *vr, unsigned int num, void *p,
@@ -106,8 +111,13 @@ static inline unsigned vring_size(unsign {return ((sizeof (struct vring_desc ) * num +sizeof (__u16) * (2 + num) + pagesize -1 ) & ~(pagesize -1 )) - +sizeof (__u16) *2 +sizeof (struct vring_used_elem ) * num; + +sizeof (__u16) *2 +sizeof (struct vring_used_elem ) * num +2 ; } + +/* We publish the last-seen used index at the end of the available ring, and +* vice-versa. These are at the end for backwards compatibility. */ +#define vring_last_used (vr) ((vr)->avail->ring[(vr)->num]) +#define vring_last_avail (vr) (*(__u16 *)&(vr)->used->ring[(vr)->num])#ifdef __KERNEL__#include <linux/irqreturn.h>