* Re: [PATCH 1/2] virito: introduce methods of fixing device features
From: Rusty Russell @ 2014-11-24 1:08 UTC (permalink / raw)
To: Jason Wang, mst, virtualization, linux-kernel; +Cc: netdev
In-Reply-To: <1415857974-23326-1-git-send-email-jasowang@redhat.com>
Jason Wang <jasowang@redhat.com> writes:
> Buggy host may advertised buggy host features (a usual case is that host
> advertise a feature whose dependencies were missed). In this case, driver
> should detect and disable the buggy features by itself.
Sorry, I've been focussing elsewhere.
I would really prefer that drivers offer a "feature_depends" table,
which can indicate that feature A depends on feature B, and have the
core iterate, complain and fixup as necessary.
Is that expressive enough, or do we need more?
Thanks,
Rusty.
^ permalink raw reply
* Re: [PATCH 08/17] {macvtap,tun}_get_user(): switch to iov_iter
From: Ben Hutchings @ 2014-11-24 1:06 UTC (permalink / raw)
To: Al Viro
Cc: David Miller, torvalds, netdev, linux-kernel, target-devel, nab,
hch
In-Reply-To: <1416788862.7215.65.camel@decadent.org.uk>
[-- Attachment #1: Type: text/plain, Size: 641 bytes --]
On Mon, 2014-11-24 at 00:27 +0000, Ben Hutchings wrote:
> On Sat, 2014-11-22 at 04:33 +0000, Al Viro wrote:
[...]
> Does skb_copy_datagram_from_iter() really need a len parameter? Here it
> is equal to iov_iter_count(from).
[...]
> Again len is equal to iov_iter_count(from), so I think that parameter is
> redundant.
Having read further patches, I see that unix_stream_sendmsg() is the one
exception where the length parameter is different. But maybe the common
case (len = iter_iov_count(iov)) deserves a wrapper function?
Ben.
--
Ben Hutchings
Absolutum obsoletum. (If it works, it's out of date.) - Stafford Beer
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply
* Re: [PATCH 0/6] kernel tinification: optionally compile out splice family of syscalls (splice, vmsplice, tee and sendfile)
From: Josh Triplett @ 2014-11-24 0:32 UTC (permalink / raw)
To: Jeff Layton
Cc: Pieter Smith, David Miller,
alexander.h.duyck-ral2JQCrhuEAvxtiuMwx3w,
viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn, ast-uqk4Ao+rVK5Wk0Htik3J/w,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
beber-2YnHqweIUXrk1uMJSBkQmQ,
catalina.mocanu-Re5JQEeQqe8AvxtiuMwx3w,
dborkman-H+wXaHxf7aLQT0dZR+AlfA, edumazet-hpIqsD4AKlfQT0dZR+AlfA,
ebiederm-aS9lmoZGLiVWk0Htik3J/w, fabf-AgBVmzD5pcezQB+pC5nmwQ,
fuse-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
geert-Td1EMuHUCqxL1ZNQvxDV9g, hughd-hpIqsD4AKlfQT0dZR+AlfA,
iulia.manda21-Re5JQEeQqe8AvxtiuMwx3w, JBeulich-IBi9RG/b67k,
bfields-uC3wQj2KruNg9hUCZPvPmw, linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, mcgrof-IBi9RG/b67k,
mattst88-Re5JQEeQqe8AvxtiuMwx3w, mgorman-l3A5Bk7waGM,
mst-H+wXaHxf7aLQT0dZR+AlfA, miklos-sUDqSbJrdHQHWmgEVkV9KA,
netdev-u79uwXL29TY76Z2rM5mHXA, oleg-H+wXaHxf7aLQT0dZR+AlfA,
Paul.Durrant-Sxgqhf6Nn4DQT0dZR+AlfA,
paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
pefoley2-lY0TAiDIAFlBDgjK7y7TUQ, tgraf-G/eBtMaohhA,
therbert-hpIqsD4AKlfQT0dZR+AlfA, willemb-hpIqsD4AKlfQT0dZR+AlfA,
xiaoguangrong-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
zhenglong.cai-TJRtMXcVgQTM1kAEIRd3EQ
In-Reply-To: <20141123192810.682a223e-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org>
On Sun, Nov 23, 2014 at 07:28:10PM -0500, Jeff Layton wrote:
> On Sun, 23 Nov 2014 15:36:37 -0800
> Josh Triplett <josh-iaAMLnmF4UmaiuxdJuQwMA@public.gmane.org> wrote:
>
> > On Sun, Nov 23, 2014 at 09:30:40PM +0100, Pieter Smith wrote:
> > > On Sun, Nov 23, 2014 at 11:43:26AM -0800, Josh Triplett wrote:
> > > > On Sun, Nov 23, 2014 at 01:46:23PM -0500, David Miller wrote:
> > > > > Truly removing sendfile/sendpage means that you can't even compile NFS
> > > > > into the tree.
> > > >
> > > > If you mean the in-kernel nfsd (CONFIG_NFSD), that already has a large
> > > > stack of "select" and "depends on", both directly and indirectly; adding
> > > > a "select SPLICE_SYSCALL" to it seems fine. (That select does need
> > > > adding, though. Pieter, you need to test-compile more than just
> > > > tinyconfig and defconfig. Try an allyesconfig with *just* splice turned
> > > > off, and make sure that compiles.)
> > >
> > > Did exacly that. Took forever on my hardware, but no problems.
> >
> > Ah, I see. Looking more closely at nfsd, it looks like it already has a
> > code path for filesystems that don't do splice. I think, rather than
> > making nfsd select SPLICE_SYSCALL, that it would suffice to change the
> > "rqstp->rq_splice_ok = true;" in svc_process_common (net/sunrpc/svc.c)
> > to:
> >
> > rqstp->rq_splice_ok = IS_ENABLED(CONFIG_SPLICE_SYSCALL);
> >
> > Then nfsd should simply *always* fall back to its non-splice support.
> >
>
> I'd probably prefer the above, actually. We have to keep supporting
> non-splice enabled fs' for the forseeable future, so we may as well
> allow people to run nfsd in such configurations. It could even be
> useful for testing the non-splice-enabled codepaths.
Good point!
- Josh Triplett
^ permalink raw reply
* Re: [PATCH 07/17] new helpers: skb_copy_datagram_from_iter() and zerocopy_sg_from_iter()
From: Ben Hutchings @ 2014-11-24 0:29 UTC (permalink / raw)
To: Al Viro
Cc: David Miller, torvalds, netdev, linux-kernel, target-devel, nab,
hch
In-Reply-To: <1416787365.7215.63.camel@decadent.org.uk>
[-- Attachment #1: Type: text/plain, Size: 514 bytes --]
On Mon, 2014-11-24 at 00:02 +0000, Ben Hutchings wrote:
> On Sat, 2014-11-22 at 04:33 +0000, Al Viro wrote:
> [...]
> > --- a/net/core/datagram.c
> > +++ b/net/core/datagram.c
> > @@ -572,6 +572,77 @@ fault:
> > }
> > EXPORT_SYMBOL(skb_copy_datagram_from_iovec);
> >
>
> Missing kernel-doc.
[...]
Never mind, I can see that patches 9 and 10 recycle the _iovec
functions' kernel-doc comments.
Ben.
--
Ben Hutchings
Absolutum obsoletum. (If it works, it's out of date.) - Stafford Beer
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply
* Re: [PATCH 0/6] kernel tinification: optionally compile out splice family of syscalls (splice, vmsplice, tee and sendfile)
From: Jeff Layton @ 2014-11-24 0:28 UTC (permalink / raw)
To: Josh Triplett
Cc: Pieter Smith, David Miller, alexander.h.duyck, viro, ast, akpm,
beber, catalina.mocanu, dborkman, edumazet, ebiederm, fabf,
fuse-devel, geert, hughd, iulia.manda21, JBeulich, bfields,
linux-api, linux-fsdevel, linux-kernel, mcgrof, mattst88, mgorman,
mst, miklos, netdev, oleg, Paul.Durrant, paulmck, pefoley2, tgraf,
therbert, willemb, xiaoguangrong, zhenglong.cai
In-Reply-To: <20141123233637.GC12456@thin>
On Sun, 23 Nov 2014 15:36:37 -0800
Josh Triplett <josh@joshtriplett.org> wrote:
> On Sun, Nov 23, 2014 at 09:30:40PM +0100, Pieter Smith wrote:
> > On Sun, Nov 23, 2014 at 11:43:26AM -0800, Josh Triplett wrote:
> > > On Sun, Nov 23, 2014 at 01:46:23PM -0500, David Miller wrote:
> > > > Truly removing sendfile/sendpage means that you can't even compile NFS
> > > > into the tree.
> > >
> > > If you mean the in-kernel nfsd (CONFIG_NFSD), that already has a large
> > > stack of "select" and "depends on", both directly and indirectly; adding
> > > a "select SPLICE_SYSCALL" to it seems fine. (That select does need
> > > adding, though. Pieter, you need to test-compile more than just
> > > tinyconfig and defconfig. Try an allyesconfig with *just* splice turned
> > > off, and make sure that compiles.)
> >
> > Did exacly that. Took forever on my hardware, but no problems.
>
> Ah, I see. Looking more closely at nfsd, it looks like it already has a
> code path for filesystems that don't do splice. I think, rather than
> making nfsd select SPLICE_SYSCALL, that it would suffice to change the
> "rqstp->rq_splice_ok = true;" in svc_process_common (net/sunrpc/svc.c)
> to:
>
> rqstp->rq_splice_ok = IS_ENABLED(CONFIG_SPLICE_SYSCALL);
>
> Then nfsd should simply *always* fall back to its non-splice support.
>
I'd probably prefer the above, actually. We have to keep supporting
non-splice enabled fs' for the forseeable future, so we may as well
allow people to run nfsd in such configurations. It could even be
useful for testing the non-splice-enabled codepaths.
> That said, given that it seems exceedingly unlikely that anyone would
> use the in-kernel nfsd on a system trying to minimize kernel size, it
> still seems cleaner to just "select SPLICE_SYSCALL" from NFSD in
> Kconfig. That avoids making any changes at all to the nfsd source in
> this patch series.
>
--
Jeff Layton <jlayton@poochiereds.net>
^ permalink raw reply
* Re: [PATCH 08/17] {macvtap,tun}_get_user(): switch to iov_iter
From: Ben Hutchings @ 2014-11-24 0:27 UTC (permalink / raw)
To: Al Viro
Cc: David Miller, torvalds, netdev, linux-kernel, target-devel, nab,
hch
In-Reply-To: <20141122043352.GH30478@ZenIV.linux.org.uk>
[-- Attachment #1: Type: text/plain, Size: 6827 bytes --]
On Sat, 2014-11-22 at 04:33 +0000, Al Viro wrote:
> allows to switch macvtap and tun from ->aio_write() to ->write_iter()
>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
> drivers/net/macvtap.c | 43 ++++++++++++++++++++-----------------------
> drivers/net/tun.c | 43 +++++++++++++++++++++++--------------------
> 2 files changed, 43 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
> index cdd820f..2bf08c6 100644
> --- a/drivers/net/macvtap.c
> +++ b/drivers/net/macvtap.c
> @@ -640,12 +640,12 @@ static void macvtap_skb_to_vnet_hdr(const struct sk_buff *skb,
>
> /* Get packet from user space buffer */
> static ssize_t macvtap_get_user(struct macvtap_queue *q, struct msghdr *m,
> - const struct iovec *iv, unsigned long total_len,
> - size_t count, int noblock)
> + struct iov_iter *from, int noblock)
> {
> int good_linear = SKB_MAX_HEAD(NET_IP_ALIGN);
> struct sk_buff *skb;
> struct macvlan_dev *vlan;
> + unsigned long total_len = iov_iter_count(from);
> unsigned long len = total_len;
> int err;
> struct virtio_net_hdr vnet_hdr = { 0 };
> @@ -653,6 +653,7 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q, struct msghdr *m,
> int copylen = 0;
> bool zerocopy = false;
> size_t linear;
> + ssize_t n;
>
> if (q->flags & IFF_VNET_HDR) {
> vnet_hdr_len = q->vnet_hdr_sz;
> @@ -662,10 +663,11 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q, struct msghdr *m,
> goto err;
> len -= vnet_hdr_len;
>
> - err = memcpy_fromiovecend((void *)&vnet_hdr, iv, 0,
> - sizeof(vnet_hdr));
> - if (err < 0)
> + err = -EFAULT;
> + n = copy_from_iter(&vnet_hdr, sizeof(vnet_hdr), from);
> + if (n != sizeof(vnet_hdr))
> goto err;
> + iov_iter_advance(from, vnet_hdr_len - sizeof(vnet_hdr));
> if ((vnet_hdr.flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) &&
> vnet_hdr.csum_start + vnet_hdr.csum_offset + 2 >
> vnet_hdr.hdr_len)
> @@ -680,17 +682,15 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q, struct msghdr *m,
> if (unlikely(len < ETH_HLEN))
> goto err;
>
> - err = -EMSGSIZE;
> - if (unlikely(count > UIO_MAXIOV))
> - goto err;
> -
> if (m && m->msg_control && sock_flag(&q->sk, SOCK_ZEROCOPY)) {
> + struct iov_iter i;
Blank line needed after a declaration.
> copylen = vnet_hdr.hdr_len ? vnet_hdr.hdr_len : GOODCOPY_LEN;
> if (copylen > good_linear)
> copylen = good_linear;
> linear = copylen;
> - if (iov_pages(iv, vnet_hdr_len + copylen, count)
> - <= MAX_SKB_FRAGS)
> + i = *from;
> + iov_iter_advance(&i, copylen);
> + if (iov_iter_npages(&i, INT_MAX) <= MAX_SKB_FRAGS)
The maxpages argument should be MAX_SKB_FRAGS + 1 as we don't need the
exact number.
> zerocopy = true;
> }
>
> @@ -708,10 +708,9 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q, struct msghdr *m,
> goto err;
>
> if (zerocopy)
> - err = zerocopy_sg_from_iovec(skb, iv, vnet_hdr_len, count);
> + err = zerocopy_sg_from_iter(skb, from);
> else {
> - err = skb_copy_datagram_from_iovec(skb, 0, iv, vnet_hdr_len,
> - len);
> + err = skb_copy_datagram_from_iter(skb, 0, from, len);
Does skb_copy_datagram_from_iter() really need a len parameter? Here it
is equal to iov_iter_count(from).
[...]
> --- 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);
tun->vnet_hdr_sz - sizeof(gso)
> }
>
> if ((tun->flags & TUN_TYPE_MASK) == TUN_TAP_DEV) {
> @@ -1063,6 +1065,7 @@ 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;
Blank line needed after a declaration.
> /* 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 +1074,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)
Again, the maxpages argument should be MAX_SKB_FRAGS + 1.
> zerocopy = true;
> }
>
> @@ -1091,9 +1095,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);
[...]
Again len is equal to iov_iter_count(from), so I think that parameter is
redundant.
Ben.
--
Ben Hutchings
Absolutum obsoletum. (If it works, it's out of date.) - Stafford Beer
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply
* Re: [PATCH 07/17] new helpers: skb_copy_datagram_from_iter() and zerocopy_sg_from_iter()
From: Ben Hutchings @ 2014-11-24 0:02 UTC (permalink / raw)
To: Al Viro
Cc: David Miller, torvalds, netdev, linux-kernel, target-devel, nab,
hch
In-Reply-To: <20141122043320.GG30478@ZenIV.linux.org.uk>
[-- Attachment #1: Type: text/plain, Size: 4179 bytes --]
On Sat, 2014-11-22 at 04:33 +0000, Al Viro wrote:
[...]
> --- a/net/core/datagram.c
> +++ b/net/core/datagram.c
> @@ -572,6 +572,77 @@ fault:
> }
> EXPORT_SYMBOL(skb_copy_datagram_from_iovec);
>
Missing kernel-doc.
> +int skb_copy_datagram_from_iter(struct sk_buff *skb, int offset,
> + struct iov_iter *from,
> + int len)
> +{
> + int start = skb_headlen(skb);
> + int i, copy = start - offset;
> + struct sk_buff *frag_iter;
> +
> + /* Copy header. */
> + if (copy > 0) {
> + if (copy > len)
> + copy = len;
> + if (copy_from_iter(skb->data + offset, copy, from) != copy)
> + goto fault;
> + if ((len -= copy) == 0)
> + return 0;
> + offset += copy;
> + }
> +
> + /* Copy paged appendix. Hmm... why does this look so complicated? */
> + for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
> + int end;
> + const skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
> +
> + WARN_ON(start > offset + len);
> +
> + end = start + skb_frag_size(frag);
> + if ((copy = end - offset) > 0) {
> + size_t copied;
Blank line needed after a declaration.
> + if (copy > len)
> + copy = len;
> + copied = copy_page_from_iter(skb_frag_page(frag),
> + frag->page_offset + offset - start,
> + copy, from);
> + if (copied != copy)
> + goto fault;
> +
> + if (!(len -= copy))
> + return 0;
The other two instances of this condition are written as:
if ((len -= copy) == 0)
Similarly in skb_copy_bits().
> + offset += copy;
> + }
> + start = end;
> + }
> +
> + skb_walk_frags(skb, frag_iter) {
> + int end;
> +
> + WARN_ON(start > offset + len);
> +
> + end = start + frag_iter->len;
> + if ((copy = end - offset) > 0) {
> + if (copy > len)
> + copy = len;
> + if (skb_copy_datagram_from_iter(frag_iter,
> + offset - start,
> + from, copy))
> + goto fault;
> + if ((len -= copy) == 0)
> + return 0;
> + offset += copy;
> + }
> + start = end;
> + }
> + if (!len)
> + return 0;
> +
> +fault:
> + return -EFAULT;
> +}
> +EXPORT_SYMBOL(skb_copy_datagram_from_iter);
> +
> /**
> * zerocopy_sg_from_iovec - Build a zerocopy datagram from an iovec
> * @skb: buffer to copy
> @@ -643,6 +714,50 @@ int zerocopy_sg_from_iovec(struct sk_buff *skb, const struct iovec *from,
> }
> EXPORT_SYMBOL(zerocopy_sg_from_iovec);
>
Missing kernel-doc.
> +int zerocopy_sg_from_iter(struct sk_buff *skb, struct iov_iter *from)
> +{
> + int len = iov_iter_count(from);
> + int copy = min_t(int, skb_headlen(skb), len);
> + int i = 0;
> +
> + /* copy up to skb headlen */
> + if (skb_copy_datagram_from_iter(skb, 0, from, copy))
> + return -EFAULT;
> +
> + while (iov_iter_count(from)) {
> + struct page *pages[MAX_SKB_FRAGS];
> + size_t start;
> + ssize_t copied;
> + unsigned long truesize;
> + int n = 0;
> +
> + copied = iov_iter_get_pages(from, pages, ~0U, MAX_SKB_FRAGS, &start);
> + if (copied < 0)
> + return -EFAULT;
> +
> + truesize = DIV_ROUND_UP(copied + start, PAGE_SIZE) * PAGE_SIZE;
PAGE_ALIGN(copied + start) ?
> + skb->data_len += copied;
> + skb->len += copied;
> + skb->truesize += truesize;
> + atomic_add(truesize, &skb->sk->sk_wmem_alloc);
> + while (copied) {
> + int off = start;
This variable seems redundant. Can't we use start directly and move the
'start = 0' to the bottom of the loop?
> + int size = min_t(int, copied, PAGE_SIZE - off);
> + start = 0;
> + if (i < MAX_SKB_FRAGS)
> + skb_fill_page_desc(skb, i, pages[n], off, size);
> + else
> + put_page(pages[n]);
Why is this condition needed, given we told iov_iter_get_pages() to
limit to MAX_SKB_FRAGS pages?
> + copied -= size;
> + i++, n++;
> + }
> + if (i > MAX_SKB_FRAGS)
> + return -EMSGSIZE;
Same here.
Ben.
> + }
> + return 0;
> +}
> +EXPORT_SYMBOL(zerocopy_sg_from_iter);
> +
> static int skb_copy_and_csum_datagram(const struct sk_buff *skb, int offset,
> u8 __user *to, int len,
> __wsum *csump)
--
Ben Hutchings
Never put off till tomorrow what you can avoid all together.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply
* Re: [PATCH 0/6] kernel tinification: optionally compile out splice family of syscalls (splice, vmsplice, tee and sendfile)
From: Josh Triplett @ 2014-11-23 23:36 UTC (permalink / raw)
To: Pieter Smith
Cc: David Miller, alexander.h.duyck-ral2JQCrhuEAvxtiuMwx3w,
viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn, ast-uqk4Ao+rVK5Wk0Htik3J/w,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
beber-2YnHqweIUXrk1uMJSBkQmQ,
catalina.mocanu-Re5JQEeQqe8AvxtiuMwx3w,
dborkman-H+wXaHxf7aLQT0dZR+AlfA, edumazet-hpIqsD4AKlfQT0dZR+AlfA,
ebiederm-aS9lmoZGLiVWk0Htik3J/w, fabf-AgBVmzD5pcezQB+pC5nmwQ,
fuse-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
geert-Td1EMuHUCqxL1ZNQvxDV9g, hughd-hpIqsD4AKlfQT0dZR+AlfA,
iulia.manda21-Re5JQEeQqe8AvxtiuMwx3w, JBeulich-IBi9RG/b67k,
bfields-uC3wQj2KruNg9hUCZPvPmw, jlayton-vpEMnDpepFuMZCB2o+C8xQ,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, mcgrof-IBi9RG/b67k,
mattst88-Re5JQEeQqe8AvxtiuMwx3w, mgorman-l3A5Bk7waGM,
mst-H+wXaHxf7aLQT0dZR+AlfA, miklos-sUDqSbJrdHQHWmgEVkV9KA,
netdev-u79uwXL29TY76Z2rM5mHXA, oleg-H+wXaHxf7aLQT0dZR+AlfA,
Paul.Durrant-Sxgqhf6Nn4DQT0dZR+AlfA,
paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
pefoley2-lY0TAiDIAFlBDgjK7y7TUQ, tgraf-G/eBtMaohhA,
therbert-hpIqsD4AKlfQT0dZR+AlfA, willemb-hpIqsD4AKlfQT0dZR+AlfA,
xiaoguangrong-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
zhenglong.cai-TJRtMXcVgQTM1kAEIRd3EQ
In-Reply-To: <20141123203040.GB26749@smipidev>
On Sun, Nov 23, 2014 at 09:30:40PM +0100, Pieter Smith wrote:
> On Sun, Nov 23, 2014 at 11:43:26AM -0800, Josh Triplett wrote:
> > On Sun, Nov 23, 2014 at 01:46:23PM -0500, David Miller wrote:
> > > Truly removing sendfile/sendpage means that you can't even compile NFS
> > > into the tree.
> >
> > If you mean the in-kernel nfsd (CONFIG_NFSD), that already has a large
> > stack of "select" and "depends on", both directly and indirectly; adding
> > a "select SPLICE_SYSCALL" to it seems fine. (That select does need
> > adding, though. Pieter, you need to test-compile more than just
> > tinyconfig and defconfig. Try an allyesconfig with *just* splice turned
> > off, and make sure that compiles.)
>
> Did exacly that. Took forever on my hardware, but no problems.
Ah, I see. Looking more closely at nfsd, it looks like it already has a
code path for filesystems that don't do splice. I think, rather than
making nfsd select SPLICE_SYSCALL, that it would suffice to change the
"rqstp->rq_splice_ok = true;" in svc_process_common (net/sunrpc/svc.c)
to:
rqstp->rq_splice_ok = IS_ENABLED(CONFIG_SPLICE_SYSCALL);
Then nfsd should simply *always* fall back to its non-splice support.
That said, given that it seems exceedingly unlikely that anyone would
use the in-kernel nfsd on a system trying to minimize kernel size, it
still seems cleaner to just "select SPLICE_SYSCALL" from NFSD in
Kconfig. That avoids making any changes at all to the nfsd source in
this patch series.
- Josh Triplett
^ permalink raw reply
* Re: [PATCH 06/17] switch macvtap to ->read_iter()
From: Ben Hutchings @ 2014-11-23 23:29 UTC (permalink / raw)
To: Al Viro
Cc: David Miller, torvalds, netdev, linux-kernel, target-devel, nab,
hch
In-Reply-To: <20141122043230.GF30478@ZenIV.linux.org.uk>
[-- Attachment #1: Type: text/plain, Size: 1533 bytes --]
On Sat, 2014-11-22 at 04:32 +0000, Al Viro wrote:
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
> drivers/net/macvtap.c | 39 ++++++++++++++++-----------------------
> 1 file changed, 16 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
> index cea99d4..cdd820f 100644
> --- a/drivers/net/macvtap.c
> +++ b/drivers/net/macvtap.c
> @@ -829,16 +829,17 @@ done:
> }
>
> static ssize_t macvtap_do_read(struct macvtap_queue *q,
> - const struct iovec *iv, unsigned long segs,
> - unsigned long len,
> + struct iov_iter *to,
> int noblock)
> {
> DEFINE_WAIT(wait);
> struct sk_buff *skb;
> ssize_t ret = 0;
> - struct iov_iter iter;
>
> - while (len) {
> + if (!iov_iter_count(to))
> + return 0;
> +
> + while (1) {
> if (!noblock)
> prepare_to_wait(sk_sleep(&q->sk), &wait,
> TASK_INTERRUPTIBLE);
> @@ -856,37 +857,27 @@ static ssize_t macvtap_do_read(struct macvtap_queue *q,
> }
> /* Nothing to read, let's sleep */
> schedule();
> - continue;
> }
> - iov_iter_init(&iter, READ, iv, segs, len);
> - ret = macvtap_put_user(q, skb, &iter);
> + }
> + if (skb) {
> + ret = macvtap_put_user(q, skb, to);
> kfree_skb(skb);
> - break;
[...]
You need to leave this break at the bottom of the loop body, or change
it to:
do {
...
} while (!skb);
Ben.
--
Ben Hutchings
Never put off till tomorrow what you can avoid all together.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply
* Re: [fuse-devel] [PATCH 4/6] fs/fuse: support compiling out splice
From: Josh Triplett @ 2014-11-23 23:23 UTC (permalink / raw)
To: Richard Weinberger
Cc: Pieter Smith, Michael S. Tsirkin, Bertrand Jacquin, Oleg Nesterov,
J. Bruce Fields, Eric Dumazet, 蔡正龙,
Jeff Layton, Tom Herbert, Alexei Starovoitov, Miklos Szeredi,
Peter Foley, Hugh Dickins, Xiao Guangrong, Geert Uytterhoeven,
Mel Gorman, Matt Turner, Paul E. McKenney, Alexander Duyck,
open list:FUSE: FILESYSTEM..., Luis R. Rodriguez
In-Reply-To: <CAFLxGvxq3Mr9n8xMO_fPZDhrGmBh=WKyU0+RF13JBdjssX6qfw@mail.gmail.com>
On Sun, Nov 23, 2014 at 11:29:08PM +0100, Richard Weinberger wrote:
> On Sun, Nov 23, 2014 at 3:20 PM, Pieter Smith <pieter@boesman.nl> wrote:
> > To implement splice support, fs/fuse makes use of nosteal_pipe_buf_ops. This
> > struct is exported by fs/splice. The goal of the larger patch set is to
> > completely compile out fs/splice, so uses of the exported struct need to be
> > compiled out along with fs/splice.
> >
> > This patch therefore compiles out splice support in fs/fuse when
> > CONFIG_SYSCALL_SPLICE is undefined.
> >
> > Signed-off-by: Pieter Smith <pieter@boesman.nl>
> > ---
> > fs/fuse/dev.c | 4 ++--
> > include/linux/fs.h | 6 ++++++
> > 2 files changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> > index ca88731..f8f92a4 100644
> > --- a/fs/fuse/dev.c
> > +++ b/fs/fuse/dev.c
> > @@ -1291,7 +1291,7 @@ static ssize_t fuse_dev_read(struct kiocb *iocb, const struct iovec *iov,
> > return fuse_dev_do_read(fc, file, &cs, iov_length(iov, nr_segs));
> > }
> >
> > -static ssize_t fuse_dev_splice_read(struct file *in, loff_t *ppos,
> > +static ssize_t __maybe_unused fuse_dev_splice_read(struct file *in, loff_t *ppos,
> > struct pipe_inode_info *pipe,
> > size_t len, unsigned int flags)
> > {
> > @@ -2144,7 +2144,7 @@ const struct file_operations fuse_dev_operations = {
> > .llseek = no_llseek,
> > .read = do_sync_read,
> > .aio_read = fuse_dev_read,
> > - .splice_read = fuse_dev_splice_read,
> > + .splice_read = __splice_p(fuse_dev_splice_read),
> > .write = do_sync_write,
> > .aio_write = fuse_dev_write,
> > .splice_write = fuse_dev_splice_write,
> > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > index a957d43..04c0975 100644
> > --- a/include/linux/fs.h
> > +++ b/include/linux/fs.h
> > @@ -2443,6 +2443,12 @@ extern int blkdev_fsync(struct file *filp, loff_t start, loff_t end,
> > int datasync);
> > extern void block_sync_page(struct page *page);
> >
> > +#ifdef CONFIG_SYSCALL_SPLICE
> > +#define __splice_p(x) x
> > +#else
> > +#define __splice_p(x) NULL
> > +#endif
> > +
>
> This needs to go into a different patch.
> One logical change per patch please. :-)
Easy enough to merge this one into the patch introducing
CONFIG_SYSCALL_SPLICE, then.
- Josh Triplett
^ permalink raw reply
* Re: [fuse-devel] [PATCH 4/6] fs/fuse: support compiling out splice
From: Richard Weinberger @ 2014-11-23 22:29 UTC (permalink / raw)
To: Pieter Smith
Cc: Michael S. Tsirkin, Bertrand Jacquin, Oleg Nesterov,
J. Bruce Fields, Eric Dumazet, 蔡正龙,
Jeff Layton, Tom Herbert, Alexei Starovoitov, Miklos Szeredi,
Peter Foley, Hugh Dickins, Xiao Guangrong, Geert Uytterhoeven,
Mel Gorman, Matt Turner, Paul E. McKenney, Alexander Duyck,
open list:FUSE: FILESYSTEM..., Luis R. Rodriguez, Josh Triplett
In-Reply-To: <1416752468-1626-5-git-send-email-pieter-qeJ+1H9vRZbz+pZb47iToQ@public.gmane.org>
On Sun, Nov 23, 2014 at 3:20 PM, Pieter Smith <pieter-qeJ+1H9vRZbz+pZb47iToQ@public.gmane.org> wrote:
> To implement splice support, fs/fuse makes use of nosteal_pipe_buf_ops. This
> struct is exported by fs/splice. The goal of the larger patch set is to
> completely compile out fs/splice, so uses of the exported struct need to be
> compiled out along with fs/splice.
>
> This patch therefore compiles out splice support in fs/fuse when
> CONFIG_SYSCALL_SPLICE is undefined.
>
> Signed-off-by: Pieter Smith <pieter-qeJ+1H9vRZbz+pZb47iToQ@public.gmane.org>
> ---
> fs/fuse/dev.c | 4 ++--
> include/linux/fs.h | 6 ++++++
> 2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> index ca88731..f8f92a4 100644
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -1291,7 +1291,7 @@ static ssize_t fuse_dev_read(struct kiocb *iocb, const struct iovec *iov,
> return fuse_dev_do_read(fc, file, &cs, iov_length(iov, nr_segs));
> }
>
> -static ssize_t fuse_dev_splice_read(struct file *in, loff_t *ppos,
> +static ssize_t __maybe_unused fuse_dev_splice_read(struct file *in, loff_t *ppos,
> struct pipe_inode_info *pipe,
> size_t len, unsigned int flags)
> {
> @@ -2144,7 +2144,7 @@ const struct file_operations fuse_dev_operations = {
> .llseek = no_llseek,
> .read = do_sync_read,
> .aio_read = fuse_dev_read,
> - .splice_read = fuse_dev_splice_read,
> + .splice_read = __splice_p(fuse_dev_splice_read),
> .write = do_sync_write,
> .aio_write = fuse_dev_write,
> .splice_write = fuse_dev_splice_write,
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index a957d43..04c0975 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2443,6 +2443,12 @@ extern int blkdev_fsync(struct file *filp, loff_t start, loff_t end,
> int datasync);
> extern void block_sync_page(struct page *page);
>
> +#ifdef CONFIG_SYSCALL_SPLICE
> +#define __splice_p(x) x
> +#else
> +#define __splice_p(x) NULL
> +#endif
> +
This needs to go into a different patch.
One logical change per patch please. :-)
--
Thanks,
//richard
^ permalink raw reply
* Re: [PATCH net-next v2] ipv6: coding style improvements (remove assignment in if statements)
From: Joe Perches @ 2014-11-23 21:50 UTC (permalink / raw)
To: Ian Morris; +Cc: netdev
In-Reply-To: <1416778123-5803-1-git-send-email-ipm@chirality.org.uk>
On Sun, 2014-11-23 at 21:28 +0000, Ian Morris wrote:
> This change has no functional impact and simply addresses some coding
> style issues detected by checkpatch. Specifically this change
> adjusts "if" statements which also include the assignment of a
> variable.
Unrelated trivia:
> net/ipv6/addrconf.c | 12 ++++++++----
[]
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
[]
> @@ -2690,7 +2691,8 @@ static void init_loopback(struct net_device *dev)
>
> ASSERT_RTNL();
>
> - if ((idev = ipv6_find_idev(dev)) == NULL) {
> + idev = ipv6_find_idev(dev);
> + if (idev == NULL) {
> pr_debug("%s: add_dev failed\n", __func__);
> return;
> }
> @@ -2813,7 +2815,8 @@ static void addrconf_sit_config(struct net_device *dev)
> * our v4 addrs in the tunnel
> */
>
> - if ((idev = ipv6_find_idev(dev)) == NULL) {
> + idev = ipv6_find_idev(dev);
> + if (idev == NULL) {
> pr_debug("%s: add_dev failed\n", __func__);
> return;
> }
> @@ -2837,7 +2840,8 @@ static void addrconf_gre_config(struct net_device *dev)
>
> ASSERT_RTNL();
>
> - if ((idev = ipv6_find_idev(dev)) == NULL) {
> + idev = ipv6_find_idev(dev);
> + if (idev == NULL) {
> pr_debug("%s: add_dev failed\n", __func__);
> return;
> }
It looks like these should not be "add_dev failed"
but "ipv6_find_idev failed"
^ permalink raw reply
* Personal
From: Patrick McSweeney @ 2014-11-23 21:31 UTC (permalink / raw)
--
I have a proposal with mutual benefit for you, contact me for more
information.
Regards
Patrick
^ permalink raw reply
* [PATCH net-next v2] ipv6: coding style improvements (remove assignment in if statements)
From: Ian Morris @ 2014-11-23 21:28 UTC (permalink / raw)
To: netdev; +Cc: Ian Morris
This change has no functional impact and simply addresses some coding
style issues detected by checkpatch. Specifically this change
adjusts "if" statements which also include the assignment of a
variable.
No changes to the resultant object files result as determined by objdiff.
Signed-off-by: Ian Morris <ipm@chirality.org.uk>
---
net/ipv6/addrconf.c | 12 ++++++++----
net/ipv6/ah6.c | 7 ++++---
net/ipv6/esp6.c | 3 ++-
net/ipv6/icmp.c | 3 ++-
net/ipv6/ip6_flowlabel.c | 6 +++++-
net/ipv6/ip6_input.c | 3 ++-
net/ipv6/ip6_output.c | 12 ++++++++----
net/ipv6/ip6_tunnel.c | 15 ++++++++-------
net/ipv6/ip6_vti.c | 4 ++--
net/ipv6/ndisc.c | 7 ++++---
net/ipv6/reassembly.c | 3 ++-
net/ipv6/sit.c | 7 ++++---
net/ipv6/udp.c | 6 ++++--
13 files changed, 55 insertions(+), 33 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 251fcb4..9eac3a7 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -2543,7 +2543,8 @@ static int inet6_addr_del(struct net *net, int ifindex, u32 ifa_flags,
if (!dev)
return -ENODEV;
- if ((idev = __in6_dev_get(dev)) == NULL)
+ idev = __in6_dev_get(dev);
+ if (idev == NULL)
return -ENXIO;
read_lock_bh(&idev->lock);
@@ -2690,7 +2691,8 @@ static void init_loopback(struct net_device *dev)
ASSERT_RTNL();
- if ((idev = ipv6_find_idev(dev)) == NULL) {
+ idev = ipv6_find_idev(dev);
+ if (idev == NULL) {
pr_debug("%s: add_dev failed\n", __func__);
return;
}
@@ -2813,7 +2815,8 @@ static void addrconf_sit_config(struct net_device *dev)
* our v4 addrs in the tunnel
*/
- if ((idev = ipv6_find_idev(dev)) == NULL) {
+ idev = ipv6_find_idev(dev);
+ if (idev == NULL) {
pr_debug("%s: add_dev failed\n", __func__);
return;
}
@@ -2837,7 +2840,8 @@ static void addrconf_gre_config(struct net_device *dev)
ASSERT_RTNL();
- if ((idev = ipv6_find_idev(dev)) == NULL) {
+ idev = ipv6_find_idev(dev);
+ if (idev == NULL) {
pr_debug("%s: add_dev failed\n", __func__);
return;
}
diff --git a/net/ipv6/ah6.c b/net/ipv6/ah6.c
index 8ab1989..a6727ad 100644
--- a/net/ipv6/ah6.c
+++ b/net/ipv6/ah6.c
@@ -353,7 +353,8 @@ static int ah6_output(struct xfrm_state *x, struct sk_buff *skb)
ahp = x->data;
ahash = ahp->ahash;
- if ((err = skb_cow_data(skb, 0, &trailer)) < 0)
+ err = skb_cow_data(skb, 0, &trailer);
+ if (err < 0)
goto out;
nfrags = err;
@@ -559,8 +560,8 @@ static int ah6_input(struct xfrm_state *x, struct sk_buff *skb)
if (!pskb_may_pull(skb, ah_hlen))
goto out;
-
- if ((err = skb_cow_data(skb, 0, &trailer)) < 0)
+ err = skb_cow_data(skb, 0, &trailer);
+ if (err < 0)
goto out;
nfrags = err;
diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
index d2c2d74..e48f2c7 100644
--- a/net/ipv6/esp6.c
+++ b/net/ipv6/esp6.c
@@ -345,7 +345,8 @@ static int esp6_input(struct xfrm_state *x, struct sk_buff *skb)
goto out;
}
- if ((nfrags = skb_cow_data(skb, 0, &trailer)) < 0) {
+ nfrags = skb_cow_data(skb, 0, &trailer);
+ if (nfrags < 0) {
ret = -EINVAL;
goto out;
}
diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index 39b3ff9..d674152 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -243,7 +243,8 @@ int icmpv6_push_pending_frames(struct sock *sk, struct flowi6 *fl6,
struct icmp6hdr *icmp6h;
int err = 0;
- if ((skb = skb_peek(&sk->sk_write_queue)) == NULL)
+ skb = skb_peek(&sk->sk_write_queue);
+ if (skb == NULL)
goto out;
icmp6h = icmp6_hdr(skb);
diff --git a/net/ipv6/ip6_flowlabel.c b/net/ipv6/ip6_flowlabel.c
index 7221021..2f780cb 100644
--- a/net/ipv6/ip6_flowlabel.c
+++ b/net/ipv6/ip6_flowlabel.c
@@ -654,7 +654,11 @@ release:
goto done;
err = -ENOMEM;
- if (sfl1 == NULL || (err = mem_check(sk)) != 0)
+ if (sfl1 == NULL)
+ goto done;
+
+ err = mem_check(sk);
+ if (err != 0)
goto done;
fl1 = fl_intern(net, fl, freq.flr_label);
diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c
index a3084ab..aacdcb4 100644
--- a/net/ipv6/ip6_input.c
+++ b/net/ipv6/ip6_input.c
@@ -220,7 +220,8 @@ resubmit:
nexthdr = skb_network_header(skb)[nhoff];
raw = raw6_local_deliver(skb, nexthdr);
- if ((ipprot = rcu_dereference(inet6_protos[nexthdr])) != NULL) {
+ ipprot = rcu_dereference(inet6_protos[nexthdr]);
+ if (ipprot != NULL) {
int ret;
if (ipprot->flags & INET6_PROTO_FINAL) {
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 916d2a1..ce69a12 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -898,7 +898,8 @@ static int ip6_dst_lookup_tail(struct sock *sk,
if (*dst == NULL)
*dst = ip6_route_output(net, sk, fl6);
- if ((err = (*dst)->error))
+ err = (*dst)->error;
+ if (err)
goto out_err_release;
if (ipv6_addr_any(&fl6->saddr)) {
@@ -946,7 +947,8 @@ static int ip6_dst_lookup_tail(struct sock *sk,
memcpy(&fl_gw6, fl6, sizeof(struct flowi6));
memset(&fl_gw6.daddr, 0, sizeof(struct in6_addr));
*dst = ip6_route_output(net, sk, &fl_gw6);
- if ((err = (*dst)->error))
+ err = (*dst)->error;
+ if (err)
goto out_err_release;
}
}
@@ -1054,7 +1056,8 @@ static inline int ip6_ufo_append_data(struct sock *sk,
* device, so create one single skb packet containing complete
* udp datagram
*/
- if ((skb = skb_peek_tail(&sk->sk_write_queue)) == NULL) {
+ skb = skb_peek_tail(&sk->sk_write_queue);
+ if (skb == NULL) {
skb = sock_alloc_send_skb(sk,
hh_len + fragheaderlen + transhdrlen + 20,
(flags & MSG_DONTWAIT), &err);
@@ -1534,7 +1537,8 @@ int ip6_push_pending_frames(struct sock *sk)
unsigned char proto = fl6->flowi6_proto;
int err = 0;
- if ((skb = __skb_dequeue(&sk->sk_write_queue)) == NULL)
+ skb = __skb_dequeue(&sk->sk_write_queue);
+ if (skb == NULL)
goto out;
tail_skb = &(skb_shinfo(skb)->frag_list);
diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index e2b6cfb..92b3da5 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -501,8 +501,8 @@ ip6_tnl_err(struct sk_buff *skb, __u8 ipproto, struct inet6_skb_parm *opt,
processing of the error. */
rcu_read_lock();
- if ((t = ip6_tnl_lookup(dev_net(skb->dev), &ipv6h->daddr,
- &ipv6h->saddr)) == NULL)
+ t = ip6_tnl_lookup(dev_net(skb->dev), &ipv6h->daddr, &ipv6h->saddr);
+ if (t == NULL)
goto out;
tproto = ACCESS_ONCE(t->parms.proto);
@@ -550,7 +550,8 @@ ip6_tnl_err(struct sk_buff *skb, __u8 ipproto, struct inet6_skb_parm *opt,
mtu = IPV6_MIN_MTU;
t->dev->mtu = mtu;
- if ((len = sizeof(*ipv6h) + ntohs(ipv6h->payload_len)) > mtu) {
+ len = sizeof(*ipv6h) + ntohs(ipv6h->payload_len);
+ if (len > mtu) {
rel_type = ICMPV6_PKT_TOOBIG;
rel_code = 0;
rel_info = mtu;
@@ -811,9 +812,8 @@ static int ip6_tnl_rcv(struct sk_buff *skb, __u16 protocol,
int err;
rcu_read_lock();
-
- if ((t = ip6_tnl_lookup(dev_net(skb->dev), &ipv6h->saddr,
- &ipv6h->daddr)) != NULL) {
+ t = ip6_tnl_lookup(dev_net(skb->dev), &ipv6h->saddr, &ipv6h->daddr);
+ if (t != NULL) {
struct pcpu_sw_netstats *tstats;
tproto = ACCESS_ONCE(t->parms.proto);
@@ -1069,7 +1069,8 @@ static int ip6_tnl_xmit2(struct sk_buff *skb,
(skb_cloned(skb) && !skb_clone_writable(skb, 0))) {
struct sk_buff *new_skb;
- if (!(new_skb = skb_realloc_headroom(skb, max_headroom)))
+ new_skb = skb_realloc_headroom(skb, max_headroom);
+ if (!new_skb)
goto tx_err_dst_release;
if (skb->sk)
diff --git a/net/ipv6/ip6_vti.c b/net/ipv6/ip6_vti.c
index ec84d03..8308216 100644
--- a/net/ipv6/ip6_vti.c
+++ b/net/ipv6/ip6_vti.c
@@ -287,8 +287,8 @@ static int vti6_rcv(struct sk_buff *skb)
const struct ipv6hdr *ipv6h = ipv6_hdr(skb);
rcu_read_lock();
- if ((t = vti6_tnl_lookup(dev_net(skb->dev), &ipv6h->saddr,
- &ipv6h->daddr)) != NULL) {
+ t = vti6_tnl_lookup(dev_net(skb->dev), &ipv6h->saddr, &ipv6h->daddr);
+ if (t != NULL) {
if (t->parms.proto != IPPROTO_IPV6 && t->parms.proto != 0) {
rcu_read_unlock();
goto discard;
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index 2c9f6bf..6828667 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -162,7 +162,8 @@ static void ndisc_fill_addr_option(struct sk_buff *skb, int type, void *data)
memcpy(opt+2, data, data_len);
data_len += 2;
opt += data_len;
- if ((space -= data_len) > 0)
+ space -= data_len;
+ if (space > 0)
memset(opt, 0, space);
}
@@ -656,8 +657,8 @@ static void ndisc_solicit(struct neighbour *neigh, struct sk_buff *skb)
if (skb && ipv6_chk_addr(dev_net(dev), &ipv6_hdr(skb)->saddr, dev, 1))
saddr = &ipv6_hdr(skb)->saddr;
-
- if ((probes -= NEIGH_VAR(neigh->parms, UCAST_PROBES)) < 0) {
+ probes -= NEIGH_VAR(neigh->parms, UCAST_PROBES);
+ if (probes < 0) {
if (!(neigh->nud_state & NUD_VALID)) {
ND_PRINTK(1, dbg,
"%s: trying to ucast probe in NUD_INVALID: %pI6\n",
diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c
index 51ab096..d7d70e6 100644
--- a/net/ipv6/reassembly.c
+++ b/net/ipv6/reassembly.c
@@ -429,7 +429,8 @@ static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *prev,
struct sk_buff *clone;
int i, plen = 0;
- if ((clone = alloc_skb(0, GFP_ATOMIC)) == NULL)
+ clone = alloc_skb(0, GFP_ATOMIC);
+ if (clone == NULL)
goto out_oom;
clone->next = head->next;
head->next = clone;
diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
index 660496d..213546b 100644
--- a/net/ipv6/sit.c
+++ b/net/ipv6/sit.c
@@ -1241,7 +1241,8 @@ ipip6_tunnel_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
if (copy_from_user(&p, ifr->ifr_ifru.ifru_data, sizeof(p)))
goto done;
err = -ENOENT;
- if ((t = ipip6_tunnel_locate(net, &p, 0)) == NULL)
+ t = ipip6_tunnel_locate(net, &p, 0);
+ if (t == NULL)
goto done;
err = -EPERM;
if (t == netdev_priv(sitn->fb_tunnel_dev))
@@ -1836,8 +1837,8 @@ static int __net_init sit_init_net(struct net *net)
goto err_dev_free;
ipip6_tunnel_clone_6rd(sitn->fb_tunnel_dev, sitn);
-
- if ((err = register_netdev(sitn->fb_tunnel_dev)))
+ err = register_netdev(sitn->fb_tunnel_dev);
+ if (err)
goto err_reg_dev;
t = netdev_priv(sitn->fb_tunnel_dev);
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 0ba3de4..dbc0b04 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -357,7 +357,8 @@ static struct sock *__udp6_lib_lookup_skb(struct sk_buff *skb,
struct sock *sk;
const struct ipv6hdr *iph = ipv6_hdr(skb);
- if (unlikely(sk = skb_steal_sock(skb)))
+ sk = skb_steal_sock(skb);
+ if (unlikely(sk))
return sk;
return __udp6_lib_lookup(dev_net(skb_dst(skb)->dev), &iph->saddr, sport,
&iph->daddr, dport, inet6_iif(skb),
@@ -1026,7 +1027,8 @@ static int udp_v6_push_pending_frames(struct sock *sk)
fl6 = &inet->cork.fl.u.ip6;
/* Grab the skbuff where UDP header space exists. */
- if ((skb = skb_peek(&sk->sk_write_queue)) == NULL)
+ skb = skb_peek(&sk->sk_write_queue);
+ if (skb == NULL)
goto out;
/*
--
1.9.1
^ permalink raw reply related
* Re: [PATCH net-next] mlx4: fix mlx4_en_set_rxfh()
From: Joe Perches @ 2014-11-23 20:56 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Amir Vadai, Ben Hutchings, David S. Miller, netdev
In-Reply-To: <1416762320.17888.21.camel@edumazet-glaptop2.roam.corp.google.com>
On Sun, 2014-11-23 at 09:05 -0800, Eric Dumazet wrote:
> On Sun, 2014-11-23 at 18:53 +0200, Amir Vadai wrote:
> > > */
> > > for (i = 0; i < priv->rx_ring_num; i++) {
> > > + if (!ring_index)
> > > + continue;
> >
> > Why didn't you put the whole loop under the 'if'?
>
> To avoid adding one indentation on the block, and ease this code review.
>
> This is hardly fast path, and compiler does the optim for us anyway.
It might have been more sensible
to use break instead of continue
^ permalink raw reply
* Re: [PATCH 0/6] kernel tinification: optionally compile out splice family of syscalls (splice, vmsplice, tee and sendfile)
From: Pieter Smith @ 2014-11-23 20:30 UTC (permalink / raw)
To: Josh Triplett
Cc: David Miller, alexander.h.duyck, viro, ast, akpm, beber,
catalina.mocanu, dborkman, edumazet, ebiederm, fabf, fuse-devel,
geert, hughd, iulia.manda21, JBeulich, bfields, jlayton,
linux-api, linux-fsdevel, linux-kernel, mcgrof, mattst88, mgorman,
mst, miklos, netdev, oleg, Paul.Durrant, paulmck, pefoley2, tgraf,
therbert, willemb, xiaoguangrong, zhenglong.cai
In-Reply-To: <20141123194326.GB8517@thin>
On Sun, Nov 23, 2014 at 11:43:26AM -0800, Josh Triplett wrote:
> On Sun, Nov 23, 2014 at 01:46:23PM -0500, David Miller wrote:
> > Truly removing sendfile/sendpage means that you can't even compile NFS
> > into the tree.
>
> If you mean the in-kernel nfsd (CONFIG_NFSD), that already has a large
> stack of "select" and "depends on", both directly and indirectly; adding
> a "select SPLICE_SYSCALL" to it seems fine. (That select does need
> adding, though. Pieter, you need to test-compile more than just
> tinyconfig and defconfig. Try an allyesconfig with *just* splice turned
> off, and make sure that compiles.)
Did exacly that. Took forever on my hardware, but no problems.
> Given the requirements of running a file server in the kernel, I'd
> expect CONFIG_NFSD to end up with several more selects of optional
> functionality in the future. It seems rather likely that the average
> embedded system will be compiling out NFS. :)
>
> Also, this patch series compiles out splice and sendfile, including
> several *users* of sendpage; it doesn't compile out the sendpage
> support/infrastructure itself.
>
> - Josh Triplett
^ permalink raw reply
* Re: [PATCH net-net 0/4] Increase the limit of tuntap queues
From: Michael S. Tsirkin @ 2014-11-23 20:30 UTC (permalink / raw)
To: David Miller
Cc: pagupta, linux-kernel, netdev, jasowang, dgibson, vfalico,
edumazet, vyasevic, hkchu, wuzhy, xemul, therbert, bhutchings,
xii, stephen, jiri, sergei.shtylyov
In-Reply-To: <20141123.134323.1154795102254868842.davem@davemloft.net>
On Sun, Nov 23, 2014 at 01:43:23PM -0500, David Miller wrote:
> From: "Michael S. Tsirkin" <mst@redhat.com>
> Date: Sun, 23 Nov 2014 12:46:23 +0200
>
> > At the moment attaching/detaching queues is an unpriveledged operation.
> >
> > Shouldn't we worry that an application can cause large
> > allocations, and provide a way to limit these?
> >
> > David, could you comment on this please?
>
> I don't want arbitrary limits imposed.
>
> Where does this "application" run? If it's in the host, then who
> cares? If they suck up all of their available memory with queue
> resources, it's their problem.
qemu runs in the host, but it's unpriveledged: it gets
passed tun FDs by a priveledged daemon, and it only
has the rights to some operations,
in particular to attach and detach queues.
The assumption always was that this operation is safe
and can't make kernel run out of resources.
--
MST
^ permalink raw reply
* [PATCH net-next] enic: use netdev_rss_key_fill() helper
From: Eric Dumazet @ 2014-11-23 20:27 UTC (permalink / raw)
To: David Miller
Cc: Christian Benvenuti, Govindarajulu Varadarajan, netdev,
Sujith Sankar
From: Eric Dumazet <edumazet@google.com>
Use of well known RSS key might increase attack surface.
Switch to a random one, using generic helper so that all
ports share a common key.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Christian Benvenuti <benve@cisco.com>
Cc: Govindarajulu Varadarajan <_govind@gmx.com>
Cc: Sujith Sankar <ssujith@cisco.com>
---
drivers/net/ethernet/cisco/enic/enic_main.c | 24 +++++++++---------
| 9 +++++-
2 files changed, 19 insertions(+), 14 deletions(-)
diff --git a/drivers/net/ethernet/cisco/enic/enic_main.c b/drivers/net/ethernet/cisco/enic/enic_main.c
index 46647407d585b54dba0baa5ef357401f17b7c167..86ee350e57f0e03bd79908ade0782d9af99646b4 100644
--- a/drivers/net/ethernet/cisco/enic/enic_main.c
+++ b/drivers/net/ethernet/cisco/enic/enic_main.c
@@ -1890,23 +1890,23 @@ static int enic_dev_hang_reset(struct enic *enic)
static int enic_set_rsskey(struct enic *enic)
{
+ union vnic_rss_key *rss_key_buf_va;
dma_addr_t rss_key_buf_pa;
- union vnic_rss_key *rss_key_buf_va = NULL;
- union vnic_rss_key rss_key = {
- .key[0].b = {85, 67, 83, 97, 119, 101, 115, 111, 109, 101},
- .key[1].b = {80, 65, 76, 79, 117, 110, 105, 113, 117, 101},
- .key[2].b = {76, 73, 78, 85, 88, 114, 111, 99, 107, 115},
- .key[3].b = {69, 78, 73, 67, 105, 115, 99, 111, 111, 108},
- };
- int err;
+ u8 rss_key[ENIC_RSS_LEN];
+ int i, kidx, bidx, err;
- rss_key_buf_va = pci_alloc_consistent(enic->pdev,
- sizeof(union vnic_rss_key), &rss_key_buf_pa);
+ rss_key_buf_va = pci_zalloc_consistent(enic->pdev,
+ sizeof(union vnic_rss_key),
+ &rss_key_buf_pa);
if (!rss_key_buf_va)
return -ENOMEM;
- memcpy(rss_key_buf_va, &rss_key, sizeof(union vnic_rss_key));
-
+ netdev_rss_key_fill(rss_key, ENIC_RSS_LEN);
+ for (i = 0; i < ENIC_RSS_LEN; i++) {
+ kidx = i / ENIC_RSS_BYTES_PER_KEY;
+ bidx = i % ENIC_RSS_BYTES_PER_KEY;
+ rss_key_buf_va->key[kidx].b[bidx] = rss_key[i];
+ }
spin_lock_bh(&enic->devcmd_lock);
err = enic_set_rss_key(enic,
rss_key_buf_pa,
--git a/drivers/net/ethernet/cisco/enic/vnic_rss.h b/drivers/net/ethernet/cisco/enic/vnic_rss.h
index fa421baf45b87eadbf5857843869f1bbcabcdb08..881fa18542b382bc52e222dc4ff47114d052fb00 100644
--- a/drivers/net/ethernet/cisco/enic/vnic_rss.h
+++ b/drivers/net/ethernet/cisco/enic/vnic_rss.h
@@ -20,11 +20,16 @@
#define _VNIC_RSS_H_
/* RSS key array */
+
+#define ENIC_RSS_BYTES_PER_KEY 10
+#define ENIC_RSS_KEYS 4
+#define ENIC_RSS_LEN (ENIC_RSS_BYTES_PER_KEY * ENIC_RSS_KEYS)
+
union vnic_rss_key {
struct {
- u8 b[10];
+ u8 b[ENIC_RSS_BYTES_PER_KEY];
u8 b_pad[6];
- } key[4];
+ } key[ENIC_RSS_KEYS];
u64 raw[8];
};
^ permalink raw reply related
* Re: [PATCH 0/6] kernel tinification: optionally compile out splice family of syscalls (splice, vmsplice, tee and sendfile)
From: Josh Triplett @ 2014-11-23 19:43 UTC (permalink / raw)
To: David Miller
Cc: pieter-qeJ+1H9vRZbz+pZb47iToQ,
alexander.h.duyck-ral2JQCrhuEAvxtiuMwx3w,
viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn, ast-uqk4Ao+rVK5Wk0Htik3J/w,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
beber-2YnHqweIUXrk1uMJSBkQmQ,
catalina.mocanu-Re5JQEeQqe8AvxtiuMwx3w,
dborkman-H+wXaHxf7aLQT0dZR+AlfA, edumazet-hpIqsD4AKlfQT0dZR+AlfA,
ebiederm-aS9lmoZGLiVWk0Htik3J/w, fabf-AgBVmzD5pcezQB+pC5nmwQ,
fuse-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
geert-Td1EMuHUCqxL1ZNQvxDV9g, hughd-hpIqsD4AKlfQT0dZR+AlfA,
iulia.manda21-Re5JQEeQqe8AvxtiuMwx3w, JBeulich-IBi9RG/b67k,
bfields-uC3wQj2KruNg9hUCZPvPmw, jlayton-vpEMnDpepFuMZCB2o+C8xQ,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, mcgrof-IBi9RG/b67k,
mattst88-Re5JQEeQqe8AvxtiuMwx3w, mgorman-l3A5Bk7waGM,
mst-H+wXaHxf7aLQT0dZR+AlfA, miklos-sUDqSbJrdHQHWmgEVkV9KA,
netdev-u79uwXL29TY76Z2rM5mHXA, oleg-H+wXaHxf7aLQT0dZR+AlfA,
Paul.Durrant-Sxgqhf6Nn4DQT0dZR+AlfA,
paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
pefoley2-lY0TAiDIAFlBDgjK7y7TUQ, tgraf-G/eBtMaohhA,
therbert-hpIqsD4AKlfQT0dZR+AlfA, willemb-hpIqsD4AKlfQT0dZR+AlfA,
xiaoguangrong-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
zhenglong.cai-TJRtMXcVgQTM1kAEIRd3EQ
In-Reply-To: <20141123.134623.2061031332250984539.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
On Sun, Nov 23, 2014 at 01:46:23PM -0500, David Miller wrote:
> Truly removing sendfile/sendpage means that you can't even compile NFS
> into the tree.
If you mean the in-kernel nfsd (CONFIG_NFSD), that already has a large
stack of "select" and "depends on", both directly and indirectly; adding
a "select SPLICE_SYSCALL" to it seems fine. (That select does need
adding, though. Pieter, you need to test-compile more than just
tinyconfig and defconfig. Try an allyesconfig with *just* splice turned
off, and make sure that compiles.)
Given the requirements of running a file server in the kernel, I'd
expect CONFIG_NFSD to end up with several more selects of optional
functionality in the future. It seems rather likely that the average
embedded system will be compiling out NFS. :)
Also, this patch series compiles out splice and sendfile, including
several *users* of sendpage; it doesn't compile out the sendpage
support/infrastructure itself.
- Josh Triplett
^ permalink raw reply
* Re: [PATCH 4/6] solos-pci: fix error return code
From: David Miller @ 2014-11-23 19:30 UTC (permalink / raw)
To: Julia.Lawall
Cc: chas, kernel-janitors, linux-atm-general, netdev, linux-kernel
In-Reply-To: <1416667159-9808-5-git-send-email-Julia.Lawall@lip6.fr>
From: Julia Lawall <Julia.Lawall@lip6.fr>
Date: Sat, 22 Nov 2014 15:39:17 +0100
> From: Julia Lawall <Julia.Lawall@lip6.fr>
>
> Return a negative error code on failure.
>
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
...
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
Applied, thanks Julia.
^ permalink raw reply
* Re: [PATCH net-next] ipv6: coding style improvements (remove assignment in if statements)
From: David Miller @ 2014-11-23 19:29 UTC (permalink / raw)
To: ipm; +Cc: netdev
In-Reply-To: <1416657725-7829-1-git-send-email-ipm@chirality.org.uk>
From: Ian Morris <ipm@chirality.org.uk>
Date: Sat, 22 Nov 2014 12:02:05 +0000
> diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
> index 7226697..7d7733a 100644
> --- a/net/ipv6/ip6mr.c
> +++ b/net/ipv6/ip6mr.c
> @@ -2410,8 +2410,7 @@ static int mr6_msgsize(bool unresolved, int maxvif)
> + nla_total_size(0) /* RTA_MULTIPATH */
> + maxvif * NLA_ALIGN(sizeof(struct rtnexthop))
> /* RTA_MFC_STATS */
> - + nla_total_size(sizeof(struct rta_mfc_stats))
> - ;
> + + nla_total_size(sizeof(struct rta_mfc_stats));
>
> return len;
> }
This first of all has absolutely nothing to do with your patch, it is
not eliminating an assignment from an if statement.
Second of all, this layout is absolutely intentional.
It makes it such that when new netlink attributed are added, the
patch to add them to this calculation is minimized to one line.
Therefore, please keep that semicolon on a line all by itself.
^ permalink raw reply
* Re: [net 3/3] igb: Fixes needed for surprise removal support
From: David Miller @ 2014-11-23 19:27 UTC (permalink / raw)
To: jeffrey.t.kirsher
Cc: carolyn.wyborny, netdev, nhorman, sassmann, jogreene,
yanirx.lubetkin
In-Reply-To: <1416642774-17077-3-git-send-email-jeffrey.t.kirsher@intel.com>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Fri, 21 Nov 2014 23:52:54 -0800
> From: Carolyn Wyborny <carolyn.wyborny@intel.com>
>
> This patch adds some checks in order to prevent panic's on surprise
> removal of devices during S0, S3, S4. Without this patch, Thunderbolt
> type device removal will panic the system.
>
> Signed-off-by: Yanir Lubetkin <yanirx.lubetkin@intel.com>
> Signed-off-by: Carolyn Wyborny <carolyn.wyborny@intel.com>
> Tested-by: Aaron Brown <aaron.f.brown@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Applied.
^ permalink raw reply
* Re: [net 2/3] ixgbe: fix use after free adapter->state test in ixgbe_remove/ixgbe_probe
From: David Miller @ 2014-11-23 19:27 UTC (permalink / raw)
To: jeffrey.t.kirsher
Cc: dborkman, netdev, nhorman, sassmann, jogreene, stable,
mark.d.rustad
In-Reply-To: <1416642774-17077-2-git-send-email-jeffrey.t.kirsher@intel.com>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Fri, 21 Nov 2014 23:52:53 -0800
> From: Daniel Borkmann <dborkman@redhat.com>
>
> While working on a different issue, I noticed an annoying use
> after free bug on my machine when unloading the ixgbe driver:
...
> The issue is that test_and_set_bit() done on adapter->state is being
> performed *after* the netdevice has been freed via free_netdev().
>
> When netdev is being allocated on initialization time, it allocates
> a private area, here struct ixgbe_adapter, that resides after the
> net_device structure. In ixgbe_probe(), the device init routine,
> we set up the adapter after alloc_etherdev_mq() on the private area
> and add a reference for the pci_dev as well via pci_set_drvdata().
>
> Both in the error path of ixgbe_probe(), but also on module unload
> when ixgbe_remove() is being called, commit 41c62843eb6a ("ixgbe:
> Fix rcu warnings induced by LER") accesses adapter after free_netdev().
> The patch stores the result in a bool and thus fixes above oops on my
> side.
>
> Fixes: 41c62843eb6a ("ixgbe: Fix rcu warnings induced by LER")
> Cc: stable <stable@vger.kernel.org>
> Cc: Mark Rustad <mark.d.rustad@intel.com>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Applied.
^ permalink raw reply
* Re: [net 1/3] ixgbe: Correctly disable VLAN filter in promiscuous mode
From: David Miller @ 2014-11-23 19:26 UTC (permalink / raw)
To: jeffrey.t.kirsher
Cc: vyasevich, netdev, nhorman, sassmann, jogreene, stable,
jacob.e.keller, vyasevic
In-Reply-To: <1416642774-17077-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Fri, 21 Nov 2014 23:52:52 -0800
> From: Vlad Yasevich <vyasevich@gmail.com>
>
> IXGBE adapter seems to require that VLAN filtering be enabled if
> VMDQ or SRIOV are enabled. When those functions are disabled,
> VLAN filtering may be disabled in promiscuous mode.
>
> Prior to commit a9b8943ee129 ("ixgbe: remove vlan_filter_disable
> and enable functions")
>
> The logic was correct. However, after the commit the logic
> got reversed and VLAN filtered in now turned on when VMDQ/SRIOV
> is disabled.
>
> This patch changes the condition to enable hw vlan filtered
> when VMDQ or SRIOV is enabled.
>
> Fixes: a9b8943ee129 ("ixgbe: remove vlan_filter_disable and enable functions")
> Cc: stable <stable@vger.kernel.org>
> CC: Jacob Keller <jacob.e.keller@intel.com>
> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
> Acked-by: Emil Tantilov <emil.s.tantilov@intel.com>
> Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Applied.
^ permalink raw reply
* Re: [PATCH 1/1] fix return code from fib_rules_lookup()
From: David Miller @ 2014-11-23 19:24 UTC (permalink / raw)
To: ani; +Cc: maze, edumazet, netdev, fruggeri
In-Reply-To: <1416637521-20173-1-git-send-email-ani@arista.com>
From: Ani Sinha <ani@arista.com>
Date: Fri, 21 Nov 2014 22:25:21 -0800
> fib_lookup() api returns two different types of error
> codes. When no custom FIB rules are installed and
> lookup fails, it returns ENETUNREACH. However, when
> custom rules are installed, __fib_lookup() calls
> fib_rules_lookup() which returns ESRCH when the rule
> lookup fails. This patch makes both code paths return
> identical error codes under lookup failure.
>
> Signed-off-by: Ani Sinha <ani@arista.com>
How does this relate to the already committed:
commit 49dd18ba4615eaa72f15c9087dea1c2ab4744cf5
Author: Panu Matilainen <pmatilai@redhat.com>
Date: Fri Nov 14 13:14:32 2014 +0200
ipv4: Fix incorrect error code when adding an unreachable route
Trying to add an unreachable route incorrectly returns -ESRCH if
if custom FIB rules are present:
[root@localhost ~]# ip route add 74.125.31.199 dev eth0 via 1.2.3.4
RTNETLINK answers: Network is unreachable
[root@localhost ~]# ip rule add to 55.66.77.88 table 200
[root@localhost ~]# ip route add 74.125.31.199 dev eth0 via 1.2.3.4
RTNETLINK answers: No such process
[root@localhost ~]#
Commit 83886b6b636173b206f475929e58fac75c6f2446 ("[NET]: Change "not found"
return value for rule lookup") changed fib_rules_lookup()
to use -ESRCH as a "not found" code internally, but for user space it
should be translated into -ENETUNREACH. Handle the translation centrally in
ipv4-specific fib_lookup(), leaving the DECnet case alone.
On a related note, commit b7a71b51ee37d919e4098cd961d59a883fd272d8
("ipv4: removed redundant conditional") removed a similar translation from
ip_route_input_slow() prematurely AIUI.
Fixes: b7a71b51ee37 ("ipv4: removed redundant conditional")
Signed-off-by: Panu Matilainen <pmatilai@redhat.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
diff --git a/net/ipv4/fib_rules.c b/net/ipv4/fib_rules.c
index f2e1573..8f7bd56 100644
--- a/net/ipv4/fib_rules.c
+++ b/net/ipv4/fib_rules.c
@@ -62,6 +62,10 @@ int __fib_lookup(struct net *net, struct flowi4 *flp, struct fib_result *res)
else
res->tclassid = 0;
#endif
+
+ if (err == -ESRCH)
+ err = -ENETUNREACH;
+
return err;
}
EXPORT_SYMBOL_GPL(__fib_lookup);
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox