From e0b46d0ee9c240c7430a47e9b0365674d4a04522 Mon Sep 17 00:00:00 2001 From: Herbert Xu Date: Fri, 7 Nov 2014 21:22:23 +0800 Subject: tun: Use iovec iterators This patch removes the use of skb_copy_datagram_const_iovec in favour of the iovec iterator-based skb_copy_datagram_iter. Signed-off-by: Herbert Xu Signed-off-by: David S. Miller --- drivers/net/tun.c | 65 +++++++++++++++++++++++++------------------------------ 1 file changed, 30 insertions(+), 35 deletions(-) (limited to 'drivers/net/tun.c') diff --git a/drivers/net/tun.c b/drivers/net/tun.c index 9dd3746994a4..2ff769bf3f35 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -71,6 +71,7 @@ #include #include #include +#include #include @@ -1230,11 +1231,11 @@ static ssize_t tun_chr_aio_write(struct kiocb *iocb, const struct iovec *iv, static ssize_t tun_put_user(struct tun_struct *tun, struct tun_file *tfile, struct sk_buff *skb, - const struct iovec *iv, int len) + struct iov_iter *iter) { struct tun_pi pi = { 0, skb->protocol }; - ssize_t total = 0; - int vlan_offset = 0, copied; + ssize_t total; + int vlan_offset; int vlan_hlen = 0; int vnet_hdr_sz = 0; @@ -1244,23 +1245,25 @@ static ssize_t tun_put_user(struct tun_struct *tun, if (tun->flags & TUN_VNET_HDR) vnet_hdr_sz = tun->vnet_hdr_sz; + total = skb->len + vlan_hlen + vnet_hdr_sz; + if (!(tun->flags & TUN_NO_PI)) { - if ((len -= sizeof(pi)) < 0) + if (iov_iter_count(iter) < sizeof(pi)) return -EINVAL; - if (len < skb->len + vlan_hlen + vnet_hdr_sz) { + total += sizeof(pi); + if (iov_iter_count(iter) < total) { /* Packet will be striped */ pi.flags |= TUN_PKT_STRIP; } - if (memcpy_toiovecend(iv, (void *) &pi, 0, sizeof(pi))) + if (copy_to_iter(&pi, sizeof(pi), iter) != sizeof(pi)) return -EFAULT; - total += sizeof(pi); } if (vnet_hdr_sz) { struct virtio_net_hdr gso = { 0 }; /* no info leak */ - if ((len -= vnet_hdr_sz) < 0) + if (iov_iter_count(iter) < vnet_hdr_sz) return -EINVAL; if (skb_is_gso(skb)) { @@ -1299,17 +1302,12 @@ static ssize_t tun_put_user(struct tun_struct *tun, gso.flags = VIRTIO_NET_HDR_F_DATA_VALID; } /* else everything is zero */ - if (unlikely(memcpy_toiovecend(iv, (void *)&gso, total, - sizeof(gso)))) + if (copy_to_iter(&gso, sizeof(gso), iter) != sizeof(gso)) return -EFAULT; - total += vnet_hdr_sz; } - copied = total; - len = min_t(int, skb->len + vlan_hlen, len); - total += skb->len + vlan_hlen; if (vlan_hlen) { - int copy, ret; + int ret; struct { __be16 h_vlan_proto; __be16 h_vlan_TCI; @@ -1320,36 +1318,32 @@ static ssize_t tun_put_user(struct tun_struct *tun, vlan_offset = offsetof(struct vlan_ethhdr, h_vlan_proto); - copy = min_t(int, vlan_offset, len); - ret = skb_copy_datagram_const_iovec(skb, 0, iv, copied, copy); - len -= copy; - copied += copy; - if (ret || !len) + ret = skb_copy_datagram_iter(skb, 0, iter, vlan_offset); + if (ret || !iov_iter_count(iter)) goto done; - copy = min_t(int, sizeof(veth), len); - ret = memcpy_toiovecend(iv, (void *)&veth, copied, copy); - len -= copy; - copied += copy; - if (ret || !len) + ret = copy_to_iter(&veth, sizeof(veth), iter); + if (ret != sizeof(veth) || !iov_iter_count(iter)) goto done; } - skb_copy_datagram_const_iovec(skb, vlan_offset, iv, copied, len); + skb_copy_datagram_iter(skb, vlan_offset, iter, skb->len - vlan_offset); done: tun->dev->stats.tx_packets++; - tun->dev->stats.tx_bytes += len; + tun->dev->stats.tx_bytes += skb->len + vlan_hlen; return total; } static ssize_t tun_do_read(struct tun_struct *tun, struct tun_file *tfile, - const struct iovec *iv, ssize_t len, int noblock) + const struct iovec *iv, unsigned long segs, + ssize_t len, int noblock) { struct sk_buff *skb; ssize_t ret = 0; int peeked, err, off = 0; + struct iov_iter iter; tun_debug(KERN_INFO, tun, "tun_do_read\n"); @@ -1362,11 +1356,12 @@ static ssize_t tun_do_read(struct tun_struct *tun, struct tun_file *tfile, /* Read frames from queue */ skb = __skb_recv_datagram(tfile->socket.sk, noblock ? MSG_DONTWAIT : 0, &peeked, &off, &err); - if (skb) { - ret = tun_put_user(tun, tfile, skb, iv, len); - kfree_skb(skb); - } else - ret = err; + if (!skb) + return ret; + + iov_iter_init(&iter, READ, iv, segs, len); + ret = tun_put_user(tun, tfile, skb, &iter); + kfree_skb(skb); return ret; } @@ -1387,7 +1382,7 @@ static ssize_t tun_chr_aio_read(struct kiocb *iocb, const struct iovec *iv, goto out; } - ret = tun_do_read(tun, tfile, iv, len, + ret = tun_do_read(tun, tfile, iv, count, len, file->f_flags & O_NONBLOCK); ret = min_t(ssize_t, ret, len); if (ret > 0) @@ -1488,7 +1483,7 @@ static int tun_recvmsg(struct kiocb *iocb, struct socket *sock, SOL_PACKET, TUN_TX_TIMESTAMP); goto out; } - ret = tun_do_read(tun, tfile, m->msg_iov, total_len, + ret = tun_do_read(tun, tfile, m->msg_iov, m->msg_iovlen, total_len, flags & MSG_DONTWAIT); if (ret > total_len) { m->msg_flags |= MSG_TRUNC; -- cgit v1.2.3 From 8c847d254146d32c86574a1b16923ff91bb784dd Mon Sep 17 00:00:00 2001 From: Jason Wang Date: Thu, 13 Nov 2014 16:54:14 +0800 Subject: tun: fix issues of iovec iterators using in tun_put_user() This patch fixes two issues after using iovec iterators: - vlan_offset should be initialized to zero, otherwise unexpected offset will be used in skb_copy_datagram_iter() - advance iovec iterator when vnet_hdr_sz is greater than sizeof(gso), this is the case when mergeable rx buffer were enabled for a virt guest. Fixes e0b46d0ee9c240c7430a47e9b0365674d4a04522 ("tun: Use iovec iterators") Cc: Herbert Xu Signed-off-by: Jason Wang Acked-by: Herbert Xu Signed-off-by: David S. Miller --- drivers/net/tun.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'drivers/net/tun.c') diff --git a/drivers/net/tun.c b/drivers/net/tun.c index 2ff769bf3f35..e3fa65a4d96c 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -1235,7 +1235,7 @@ static ssize_t tun_put_user(struct tun_struct *tun, { struct tun_pi pi = { 0, skb->protocol }; ssize_t total; - int vlan_offset; + int vlan_offset = 0; int vlan_hlen = 0; int vnet_hdr_sz = 0; @@ -1304,6 +1304,8 @@ static ssize_t tun_put_user(struct tun_struct *tun, if (copy_to_iter(&gso, sizeof(gso), iter) != sizeof(gso)) return -EFAULT; + + iov_iter_advance(iter, vnet_hdr_sz - sizeof(gso)); } if (vlan_hlen) { -- cgit v1.2.3 From baeababb5b85d5c4e6c917efe2a1504179438d3b Mon Sep 17 00:00:00 2001 From: Jason Wang Date: Tue, 18 Nov 2014 13:20:41 +0800 Subject: tun: return NET_XMIT_DROP for dropped packets After commit 5d097109257c03a71845729f8db6b5770c4bbedc ("tun: only queue packets on device"), NETDEV_TX_OK was returned for dropped packets. This will confuse pktgen since dropped packets were counted as sent ones. Fixing this by returning NET_XMIT_DROP to let pktgen count it as error packet. Cc: Michael S. Tsirkin Signed-off-by: Jason Wang Signed-off-by: David S. Miller --- drivers/net/tun.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/net/tun.c') diff --git a/drivers/net/tun.c b/drivers/net/tun.c index e3fa65a4d96c..ac53a7316ecd 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -819,7 +819,7 @@ drop: skb_tx_error(skb); kfree_skb(skb); rcu_read_unlock(); - return NETDEV_TX_OK; + return NET_XMIT_DROP; } static void tun_net_mclist(struct net_device *dev) -- cgit v1.2.3 From 9b067034d0cd4ee7bbbc317567f4e38ff7ca65c4 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Fri, 7 Nov 2014 13:52:07 -0500 Subject: switch drivers/net/tun.c to ->read_iter() Signed-off-by: Al Viro --- drivers/net/tun.c | 40 +++++++++++++++------------------------- 1 file changed, 15 insertions(+), 25 deletions(-) (limited to 'drivers/net/tun.c') diff --git a/drivers/net/tun.c b/drivers/net/tun.c index ac53a7316ecd..405dfdf4f8c8 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -1339,18 +1339,17 @@ done: } static ssize_t tun_do_read(struct tun_struct *tun, struct tun_file *tfile, - const struct iovec *iv, unsigned long segs, - ssize_t len, int noblock) + struct iov_iter *to, + int noblock) { struct sk_buff *skb; - ssize_t ret = 0; + ssize_t ret; int peeked, err, off = 0; - struct iov_iter iter; tun_debug(KERN_INFO, tun, "tun_do_read\n"); - if (!len) - return ret; + if (!iov_iter_count(to)) + return 0; if (tun->dev->reg_state != NETREG_REGISTERED) return -EIO; @@ -1359,37 +1358,27 @@ static ssize_t tun_do_read(struct tun_struct *tun, struct tun_file *tfile, skb = __skb_recv_datagram(tfile->socket.sk, noblock ? MSG_DONTWAIT : 0, &peeked, &off, &err); if (!skb) - return ret; + return 0; - iov_iter_init(&iter, READ, iv, segs, len); - ret = tun_put_user(tun, tfile, skb, &iter); + ret = tun_put_user(tun, tfile, skb, to); kfree_skb(skb); return ret; } -static ssize_t tun_chr_aio_read(struct kiocb *iocb, const struct iovec *iv, - unsigned long count, loff_t pos) +static ssize_t tun_chr_read_iter(struct kiocb *iocb, struct iov_iter *to) { struct file *file = iocb->ki_filp; struct tun_file *tfile = file->private_data; struct tun_struct *tun = __tun_get(tfile); - ssize_t len, ret; + ssize_t len = iov_iter_count(to), ret; if (!tun) return -EBADFD; - len = iov_length(iv, count); - if (len < 0) { - ret = -EINVAL; - goto out; - } - - ret = tun_do_read(tun, tfile, iv, count, len, - file->f_flags & O_NONBLOCK); + ret = tun_do_read(tun, tfile, to, file->f_flags & O_NONBLOCK); ret = min_t(ssize_t, ret, len); if (ret > 0) iocb->ki_pos = ret; -out: tun_put(tun); return ret; } @@ -1471,6 +1460,7 @@ static int tun_recvmsg(struct kiocb *iocb, struct socket *sock, { struct tun_file *tfile = container_of(sock, struct tun_file, socket); struct tun_struct *tun = __tun_get(tfile); + struct iov_iter to; int ret; if (!tun) @@ -1485,8 +1475,8 @@ static int tun_recvmsg(struct kiocb *iocb, struct socket *sock, SOL_PACKET, TUN_TX_TIMESTAMP); goto out; } - ret = tun_do_read(tun, tfile, m->msg_iov, m->msg_iovlen, total_len, - flags & MSG_DONTWAIT); + iov_iter_init(&to, READ, m->msg_iov, m->msg_iovlen, total_len); + ret = tun_do_read(tun, tfile, &to, flags & MSG_DONTWAIT); if (ret > total_len) { m->msg_flags |= MSG_TRUNC; ret = flags & MSG_TRUNC ? ret : total_len; @@ -2242,8 +2232,8 @@ static int tun_chr_show_fdinfo(struct seq_file *m, struct file *f) static const struct file_operations tun_fops = { .owner = THIS_MODULE, .llseek = no_llseek, - .read = do_sync_read, - .aio_read = tun_chr_aio_read, + .read = new_sync_read, + .read_iter = tun_chr_read_iter, .write = do_sync_write, .aio_write = tun_chr_aio_write, .poll = tun_chr_poll, -- cgit v1.2.3 From f5ff53b4d97c62e63ee2868fd4b8d7896643ee03 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 19 Jun 2014 15:36:49 -0400 Subject: {macvtap,tun}_get_user(): switch to iov_iter allows to switch macvtap and tun from ->aio_write() to ->write_iter() Signed-off-by: Al Viro --- drivers/net/tun.c | 44 ++++++++++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 20 deletions(-) (limited to 'drivers/net/tun.c') diff --git a/drivers/net/tun.c b/drivers/net/tun.c index 405dfdf4f8c8..4b743c612624 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -1012,28 +1012,29 @@ static struct sk_buff *tun_alloc_skb(struct tun_file *tfile, /* Get packet from user space buffer */ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile, - void *msg_control, const struct iovec *iv, - size_t total_len, size_t count, int noblock) + void *msg_control, struct iov_iter *from, + int noblock) { struct tun_pi pi = { 0, cpu_to_be16(ETH_P_IP) }; struct sk_buff *skb; + size_t total_len = iov_iter_count(from); size_t len = total_len, align = NET_SKB_PAD, linear; struct virtio_net_hdr gso = { 0 }; int good_linear; - int offset = 0; int copylen; bool zerocopy = false; int err; u32 rxhash; + ssize_t n; if (!(tun->flags & TUN_NO_PI)) { if (len < sizeof(pi)) return -EINVAL; len -= sizeof(pi); - if (memcpy_fromiovecend((void *)&pi, iv, 0, sizeof(pi))) + n = copy_from_iter(&pi, sizeof(pi), from); + if (n != sizeof(pi)) return -EFAULT; - offset += sizeof(pi); } if (tun->flags & TUN_VNET_HDR) { @@ -1041,7 +1042,8 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile, return -EINVAL; len -= tun->vnet_hdr_sz; - if (memcpy_fromiovecend((void *)&gso, iv, offset, sizeof(gso))) + n = copy_from_iter(&gso, sizeof(gso), from); + if (n != sizeof(gso)) return -EFAULT; if ((gso.flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) && @@ -1050,7 +1052,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile, if (gso.hdr_len > len) return -EINVAL; - offset += tun->vnet_hdr_sz; + iov_iter_advance(from, tun->vnet_hdr_sz); } if ((tun->flags & TUN_TYPE_MASK) == TUN_TAP_DEV) { @@ -1063,6 +1065,8 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile, good_linear = SKB_MAX_HEAD(align); if (msg_control) { + struct iov_iter i = *from; + /* There are 256 bytes to be copied in skb, so there is * enough room for skb expand head in case it is used. * The rest of the buffer is mapped from userspace. @@ -1071,7 +1075,8 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile, if (copylen > good_linear) copylen = good_linear; linear = copylen; - if (iov_pages(iv, offset + copylen, count) <= MAX_SKB_FRAGS) + iov_iter_advance(&i, copylen); + if (iov_iter_npages(&i, INT_MAX) <= MAX_SKB_FRAGS) zerocopy = true; } @@ -1091,9 +1096,9 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile, } if (zerocopy) - err = zerocopy_sg_from_iovec(skb, iv, offset, count); + err = zerocopy_sg_from_iter(skb, from); else { - err = skb_copy_datagram_from_iovec(skb, 0, iv, offset, len); + err = skb_copy_datagram_from_iter(skb, 0, from, len); if (!err && msg_control) { struct ubuf_info *uarg = msg_control; uarg->callback(uarg, false); @@ -1207,8 +1212,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile, return total_len; } -static ssize_t tun_chr_aio_write(struct kiocb *iocb, const struct iovec *iv, - unsigned long count, loff_t pos) +static ssize_t tun_chr_write_iter(struct kiocb *iocb, struct iov_iter *from) { struct file *file = iocb->ki_filp; struct tun_struct *tun = tun_get(file); @@ -1218,10 +1222,7 @@ static ssize_t tun_chr_aio_write(struct kiocb *iocb, const struct iovec *iv, if (!tun) return -EBADFD; - tun_debug(KERN_INFO, tun, "tun_chr_write %ld\n", count); - - result = tun_get_user(tun, tfile, NULL, iv, iov_length(iv, count), - count, file->f_flags & O_NONBLOCK); + result = tun_get_user(tun, tfile, NULL, from, file->f_flags & O_NONBLOCK); tun_put(tun); return result; @@ -1445,11 +1446,14 @@ static int tun_sendmsg(struct kiocb *iocb, struct socket *sock, int ret; struct tun_file *tfile = container_of(sock, struct tun_file, socket); struct tun_struct *tun = __tun_get(tfile); + struct iov_iter from; if (!tun) return -EBADFD; - ret = tun_get_user(tun, tfile, m->msg_control, m->msg_iov, total_len, - m->msg_iovlen, m->msg_flags & MSG_DONTWAIT); + + iov_iter_init(&from, WRITE, m->msg_iov, m->msg_iovlen, total_len); + ret = tun_get_user(tun, tfile, m->msg_control, &from, + m->msg_flags & MSG_DONTWAIT); tun_put(tun); return ret; } @@ -2233,9 +2237,9 @@ static const struct file_operations tun_fops = { .owner = THIS_MODULE, .llseek = no_llseek, .read = new_sync_read, + .write = new_sync_write, .read_iter = tun_chr_read_iter, - .write = do_sync_write, - .aio_write = tun_chr_aio_write, + .write_iter = tun_chr_write_iter, .poll = tun_chr_poll, .unlocked_ioctl = tun_chr_ioctl, #ifdef CONFIG_COMPAT -- cgit v1.2.3 From d8febb77b52ebddb9bd03ccaa5b61005e3a45a85 Mon Sep 17 00:00:00 2001 From: Herbert Xu Date: Sun, 30 Nov 2014 18:03:31 +0800 Subject: tun: Fix GSO meta-data handling in tun_get_user When we write the GSO meta-data in tun_get_user we end up advancing the IO vector twice, thus exhausting the user buffer before we can finish writing the packet. Fixes: f5ff53b4d97c ("{macvtap,tun}_get_user(): switch to iov_iter") Reported-by: Marcelo Ricardo Leitner Signed-off-by: Herbert Xu Acked-by: Jason Wang Signed-off-by: David S. Miller --- drivers/net/tun.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/net/tun.c') diff --git a/drivers/net/tun.c b/drivers/net/tun.c index 4b743c612624..6d44da184594 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -1052,7 +1052,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile, if (gso.hdr_len > len) return -EINVAL; - iov_iter_advance(from, tun->vnet_hdr_sz); + iov_iter_advance(from, tun->vnet_hdr_sz - sizeof(gso)); } if ((tun->flags & TUN_TYPE_MASK) == TUN_TAP_DEV) { -- cgit v1.2.3 From f51a5e82ea9aaf05106c00d976e772ca384a9199 Mon Sep 17 00:00:00 2001 From: Jason Wang Date: Mon, 1 Dec 2014 16:53:15 +0800 Subject: tun/macvtap: use consume_skb() instead of kfree_skb() when needed To be more friendly with drop monitor, we should only call kfree_skb() when the packets were dropped and use consume_skb() in other cases. Cc: Eric Dumazet Signed-off-by: Jason Wang Acked-by: Michael S. Tsirkin Signed-off-by: David S. Miller --- drivers/net/tun.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'drivers/net/tun.c') diff --git a/drivers/net/tun.c b/drivers/net/tun.c index 6d44da184594..9c58286b8a42 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -1362,7 +1362,10 @@ static ssize_t tun_do_read(struct tun_struct *tun, struct tun_file *tfile, return 0; ret = tun_put_user(tun, tfile, skb, to); - kfree_skb(skb); + if (unlikely(ret < 0)) + kfree_skb(skb); + else + consume_skb(skb); return ret; } -- cgit v1.2.3 From c0371da6047abd261bc483c744dbc7d81a116172 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Mon, 24 Nov 2014 10:42:55 -0500 Subject: put iov_iter into msghdr Note that the code _using_ ->msg_iter at that point will be very unhappy with anything other than unshifted iovec-backed iov_iter. We still need to convert users to proper primitives. Signed-off-by: Al Viro --- drivers/net/tun.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) (limited to 'drivers/net/tun.c') diff --git a/drivers/net/tun.c b/drivers/net/tun.c index 9c58286b8a42..f3e992ed87ac 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -1449,13 +1449,11 @@ static int tun_sendmsg(struct kiocb *iocb, struct socket *sock, int ret; struct tun_file *tfile = container_of(sock, struct tun_file, socket); struct tun_struct *tun = __tun_get(tfile); - struct iov_iter from; if (!tun) return -EBADFD; - iov_iter_init(&from, WRITE, m->msg_iov, m->msg_iovlen, total_len); - ret = tun_get_user(tun, tfile, m->msg_control, &from, + ret = tun_get_user(tun, tfile, m->msg_control, &m->msg_iter, m->msg_flags & MSG_DONTWAIT); tun_put(tun); return ret; @@ -1467,7 +1465,6 @@ static int tun_recvmsg(struct kiocb *iocb, struct socket *sock, { struct tun_file *tfile = container_of(sock, struct tun_file, socket); struct tun_struct *tun = __tun_get(tfile); - struct iov_iter to; int ret; if (!tun) @@ -1482,8 +1479,7 @@ static int tun_recvmsg(struct kiocb *iocb, struct socket *sock, SOL_PACKET, TUN_TX_TIMESTAMP); goto out; } - iov_iter_init(&to, READ, m->msg_iov, m->msg_iovlen, total_len); - ret = tun_do_read(tun, tfile, &to, flags & MSG_DONTWAIT); + ret = tun_do_read(tun, tfile, &m->msg_iter, flags & MSG_DONTWAIT); if (ret > total_len) { m->msg_flags |= MSG_TRUNC; ret = flags & MSG_TRUNC ? ret : total_len; -- cgit v1.2.3