Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net] xfrm: Clear sk_dst_cache when applying per-socket policy.
From: Steffen Klassert @ 2017-10-25  4:25 UTC (permalink / raw)
  To: Jonathan Basseri 😶
  Cc: netdev, Jakub Sitnicki, David Miller, Lorenzo Colitti,
	Eric Dumazet
In-Reply-To: <CABcqWgLEQ6j0FDPOQmMpQugPonBoUfKB2sfnP8+bJtYdNS8Viw@mail.gmail.com>

On Tue, Oct 24, 2017 at 09:58:48AM -0700, Jonathan Basseri 😶 wrote:
> On Tue, Oct 24, 2017 at 12:04 AM, Steffen Klassert
> <steffen.klassert@secunet.com> wrote:
> >
> > On Mon, Oct 23, 2017 at 06:18:55PM -0700, Jonathan Basseri wrote:
> > > If a socket has a valid dst cache, then xfrm_lookup_route will get
> > > skipped. However, the cache is not invalidated when applying policy to a
> > > socket (i.e. IPV6_XFRM_POLICY). The result is that new policies are
> > > sometimes ignored on those sockets. (Note: This was broken for IPv4 and
> > > IPv6 at different times.)
> > >
> > > This can be demonstrated like so,
> > > 1. Create UDP socket.
> > > 2. connect() the socket.
> > > 3. Apply an outbound XFRM policy to the socket.
> > > 4. send() data on the socket.
> > >
> > > Packets will continue to be sent in the clear instead of matching an
> > > xfrm or returning a no-match error (EAGAIN). This affects calls to
> > > send() and not sendto().
> > >
> > > Invalidating the sk_dst_cache is necessary to correctly apply xfrm
> > > policies. Since we do this in xfrm_user_policy(), the sk_lock was
> > > already acquired in either do_ip_setsockopt() or do_ipv6_setsockopt(),
> > > and we may call __sk_dst_reset().
> > >
> > > Performance impact should be negligible, since this code is only called
> > > when changing xfrm policy, and only affects the socket in question.
> > >
> > > Note: Creating normal XFRM policies should have a similar effect on
> > > sk_dst_cache entries that match the policy, but that is not fixed in
> > > this patch.
> >
> > I think we don't have this problem with 'normal' policies. When
> > inserting such a policy, we bump the IPv4/IPv6 genid. This should
> > invalidate all cached dst entries, no?
> >
> That sounds reasonable to me. I had not confirmed the behavior for
> normal policies, so I was trying to point out that this fix is only
> for socket policies. Should I modify the commit message?

Yes, please do so. This comment may lead people to the wrong direction.

Thanks!

^ permalink raw reply

* Re: [PATCH] drivers/net: wan/sbni: Convert timers to use timer_setup()
From: David Miller @ 2017-10-25  4:22 UTC (permalink / raw)
  To: keescook; +Cc: dhowells, netdev, linux-kernel
In-Reply-To: <20171024084700.GA24005@beast>

From: Kees Cook <keescook@chromium.org>
Date: Tue, 24 Oct 2017 01:47:00 -0700

> In preparation for unconditionally passing the struct timer_list pointer to
> all timer callbacks, switch to using the new timer_setup() and from_timer()
> to pass the timer pointer explicitly.
> 
> Cc: David Howells <dhowells@redhat.com>
> Cc: netdev@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>

Applied.

^ permalink raw reply

* Re: [PATCH] drivers/net: sis: Convert timers to use timer_setup()
From: David Miller @ 2017-10-25  4:22 UTC (permalink / raw)
  To: keescook; +Cc: romieu, venza, netdev, linux-kernel
In-Reply-To: <20171024084652.GA23986@beast>

From: Kees Cook <keescook@chromium.org>
Date: Tue, 24 Oct 2017 01:46:52 -0700

> In preparation for unconditionally passing the struct timer_list pointer to
> all timer callbacks, switch to using the new timer_setup() and from_timer()
> to pass the timer pointer explicitly.
> 
> Cc: Francois Romieu <romieu@fr.zoreil.com>
> Cc: Daniele Venzano <venza@brownhat.org>
> Cc: netdev@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>

Applied.

^ permalink raw reply

* Re: [PATCH] net: xfrm_user: use BUG_ON instead of if condition followed by BUG
From: David Miller @ 2017-10-25  4:22 UTC (permalink / raw)
  To: herbert; +Cc: garsilva, steffen.klassert, netdev, linux-kernel
In-Reply-To: <20171025040541.GA1999@gondor.apana.org.au>

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Wed, 25 Oct 2017 12:05:41 +0800

> On Tue, Oct 24, 2017 at 05:48:42PM +0900, David Miller wrote:
>> 
>> This discussion has happened before.
>> 
>> But I'll explain the conclusion here for your benefit.
>> 
>> BUG_ON() is a statement and everything inside of it will
>> always execute.
>> 
>> BUG_ON() is always preferred because it allows arch
>> specific code to pass the conditional result properly
>> into inline asm and builtins for optimal code generation.
> 
> This is a good point.  However, while a little bit more verbose you
> can still achieve the same assembly-level result by something like
> 
> 	int err;
> 
> 	err = <insert real code here>;
> 	BUG_ON(err);
> 
> Having real code in BUG_ON may pose problems to people reading the
> code because some of us tend to ignore code in BUG_ON and similar
> macros such as BUILD_BUG_ON.

I agree that this makes the code easier to read and audit.

^ permalink raw reply

* Re: [PATCH] net: atm/mpc: Stop using open-coded timer .data field
From: David Miller @ 2017-10-25  4:08 UTC (permalink / raw)
  To: keescook; +Cc: bhumirks, akpm, adobriyan, elena.reshetova, netdev, linux-kernel
In-Reply-To: <20171024084645.GA23963@beast>

From: Kees Cook <keescook@chromium.org>
Date: Tue, 24 Oct 2017 01:46:45 -0700

> In preparation for unconditionally passing the struct timer_list pointer to
> all timer callbacks, switch to using an explicit static variable to hold
> additional expiration details.
> 
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Bhumika Goyal <bhumirks@gmail.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Alexey Dobriyan <adobriyan@gmail.com>
> Cc: "Reshetova, Elena" <elena.reshetova@intel.com>
> Cc: netdev@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>

Applied.

^ permalink raw reply

* Re: [PATCH] ipv6: esp6: use BUG_ON instead of if condition followed by BUG
From: Herbert Xu @ 2017-10-25  4:07 UTC (permalink / raw)
  To: Gustavo A. R. Silva
  Cc: Steffen Klassert, David S. Miller, Alexey Kuznetsov,
	Hideaki YOSHIFUJI, netdev, linux-kernel
In-Reply-To: <20171024162826.GA29550@embeddedor.com>

On Tue, Oct 24, 2017 at 11:28:26AM -0500, Gustavo A. R. Silva wrote:
> Use BUG_ON instead of if condition followed by BUG in esp_remove_trailer.
> 
> This issue was detected with the help of Coccinelle.
> 
> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
> ---
>  net/ipv6/esp6.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
> index 89910e2..603ff06 100644
> --- a/net/ipv6/esp6.c
> +++ b/net/ipv6/esp6.c
> @@ -483,8 +483,7 @@ static inline int esp_remove_trailer(struct sk_buff *skb)
>  		goto out;
>  	}
>  
> -	if (skb_copy_bits(skb, skb->len - alen - 2, nexthdr, 2))
> -		BUG();
> +	BUG_ON(skb_copy_bits(skb, skb->len - alen - 2, nexthdr, 2));

How about

	ret = skb_copy_bits(skb, skb->len - alen - 2, nexthdr, 2);
	BUG_ON(ret);

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH] tap: double-free in error path in tap_open()
From: Girish Moodalbail @ 2017-10-25  4:07 UTC (permalink / raw)
  To: Jason Wang, netdev, davem
In-Reply-To: <e5816501-b58c-506c-161f-8ee7a80ea347@redhat.com>

On 10/24/17 6:55 PM, Jason Wang wrote:
> 
> 
> On 2017年10月25日 05:41, Girish Moodalbail wrote:
>> Double free of skb_array in tap module is causing kernel panic. When
>> tap_set_queue() fails we free skb_array right away by calling
>> skb_array_cleanup(). However, later on skb_array_cleanup() is called
>> again by tap_sock_destruct through sock_put(). This patch fixes that
>> issue.
>>
>> Fixes: 362899b8725b35e3 (macvtap: switch to use skb array)
>> Signed-off-by: Girish Moodalbail <girish.moodalbail@oracle.com>
>> ---
>>   drivers/net/tap.c | 12 ++++++------
>>   1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/net/tap.c b/drivers/net/tap.c
>> index 21b71ae..878520b 100644
>> --- a/drivers/net/tap.c
>> +++ b/drivers/net/tap.c
>> @@ -542,20 +542,20 @@ static int tap_open(struct inode *inode, struct file *file)
>>       err = -ENOMEM;
>>       if (skb_array_init(&q->skb_array, tap->dev->tx_queue_len, GFP_KERNEL))
>> -        goto err_array;
>> +        goto err_put;
> 
> Looks like this will cause skb_array_cleanup() to be called when skb array was 
> uninitialized?

Thanks Jason.

This is an existing issue outside of my code. I will send a v2 of the patch that 
incorporates the fix for this issue as well. I am considering moving 
skb_array_init() to the beginning of tap_open() (near to where we allocate 
memory for the tap_queue itself).

Thanks again.

regards,
~Girish


> 
> Thanks
> 
>>       err = tap_set_queue(tap, file, q);
>> -    if (err)
>> -        goto err_queue;
>> +    if (err) {
>> +        /* tap_sock_destruct() will take care of freeing skb_array */
>> +        goto err_put;
>> +    }
>>       dev_put(tap->dev);
>>       rtnl_unlock();
>>       return err;
>> -err_queue:
>> -    skb_array_cleanup(&q->skb_array);
>> -err_array:
>> +err_put:
>>       sock_put(&q->sk);
>>   err:
>>       if (tap)
> 

^ permalink raw reply

* Re: [PATCH] net: xfrm_user: use BUG_ON instead of if condition followed by BUG
From: Herbert Xu @ 2017-10-25  4:05 UTC (permalink / raw)
  To: David Miller; +Cc: garsilva, steffen.klassert, netdev, linux-kernel
In-Reply-To: <20171024.174842.272234711073180328.davem@davemloft.net>

On Tue, Oct 24, 2017 at 05:48:42PM +0900, David Miller wrote:
> 
> This discussion has happened before.
> 
> But I'll explain the conclusion here for your benefit.
> 
> BUG_ON() is a statement and everything inside of it will
> always execute.
> 
> BUG_ON() is always preferred because it allows arch
> specific code to pass the conditional result properly
> into inline asm and builtins for optimal code generation.

This is a good point.  However, while a little bit more verbose you
can still achieve the same assembly-level result by something like

	int err;

	err = <insert real code here>;
	BUG_ON(err);

Having real code in BUG_ON may pose problems to people reading the
code because some of us tend to ignore code in BUG_ON and similar
macros such as BUILD_BUG_ON.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH] net: af_packet: Convert timers to use timer_setup()
From: David Miller @ 2017-10-25  4:03 UTC (permalink / raw)
  To: keescook; +Cc: edumazet, willemb, maloney, jarno, netdev, linux-kernel
In-Reply-To: <20171024084626.GA23908@beast>

From: Kees Cook <keescook@chromium.org>
Date: Tue, 24 Oct 2017 01:46:26 -0700

> In preparation for unconditionally passing the struct timer_list pointer to
> all timer callbacks, switch to using the new timer_setup() and from_timer()
> to pass the timer pointer explicitly.
> 
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Willem de Bruijn <willemb@google.com>
> Cc: Mike Maloney <maloney@google.com>
> Cc: Jarno Rajahalme <jarno@ovn.org>
> Cc: netdev@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>

Applied.

^ permalink raw reply

* Re: [PATCH] net: hsr: Convert timers to use timer_setup()
From: David Miller @ 2017-10-25  4:03 UTC (permalink / raw)
  To: keescook; +Cc: arvid.brodin, netdev, linux-kernel
In-Reply-To: <20171024084616.GA23889@beast>

From: Kees Cook <keescook@chromium.org>
Date: Tue, 24 Oct 2017 01:46:16 -0700

> In preparation for unconditionally passing the struct timer_list pointer to
> all timer callbacks, switch to using the new timer_setup() and from_timer()
> to pass the timer pointer explicitly.
> 
> Cc: Arvid Brodin <arvid.brodin@alten.se>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: netdev@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>

Applied.

^ permalink raw reply

* Re: [PATCH] net: dccp: Convert timers to use timer_setup()
From: David Miller @ 2017-10-25  3:59 UTC (permalink / raw)
  To: keescook; +Cc: gerrit, soheil, hannes, edumazet, dccp, netdev, linux-kernel
In-Reply-To: <20171024084609.GA23862@beast>

From: Kees Cook <keescook@chromium.org>
Date: Tue, 24 Oct 2017 01:46:09 -0700

> In preparation for unconditionally passing the struct timer_list pointer to
> all timer callbacks, switch to using the new timer_setup() and from_timer()
> to pass the timer pointer explicitly. Adds a pointer back to the sock.
> 
> Cc: Gerrit Renker <gerrit@erg.abdn.ac.uk>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Soheil Hassas Yeganeh <soheil@google.com>
> Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: dccp@vger.kernel.org
> Cc: netdev@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>

Applied.

^ permalink raw reply

* Re: [PATCH] net: ethernet/sfc: Convert timers to use timer_setup()
From: David Miller @ 2017-10-25  3:57 UTC (permalink / raw)
  To: keescook
  Cc: linux-net-drivers, ecree, bkenward, edumazet, jiri, jhs, mingo,
	netdev, linux-kernel
In-Reply-To: <20171024084559.GA23827@beast>

From: Kees Cook <keescook@chromium.org>
Date: Tue, 24 Oct 2017 01:45:59 -0700

> In preparation for unconditionally passing the struct timer_list pointer to
> all timer callbacks, switch to using the new timer_setup() and from_timer()
> to pass the timer pointer explicitly.
> 
> Cc: Solarflare linux maintainers <linux-net-drivers@solarflare.com>
> Cc: Edward Cree <ecree@solarflare.com>
> Cc: Bert Kenward <bkenward@solarflare.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Jiri Pirko <jiri@mellanox.com>
> Cc: Jamal Hadi Salim <jhs@mojatatu.com>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: netdev@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>

Applied.

^ permalink raw reply

* [PATCH net-next] tools: bpftool: try to mount bpffs if required for pinning objects
From: Jakub Kicinski @ 2017-10-25  3:11 UTC (permalink / raw)
  To: netdev
  Cc: oss-drivers, alexei.starovoitov, daniel, Quentin Monnet,
	Jakub Kicinski

From: Quentin Monnet <quentin.monnet@netronome.com>

One possible cause of failure for `bpftool {prog|map} pin * file FILE`
is the FILE not being in an eBPF virtual file system (bpffs). In this
case, make bpftool attempt to mount bpffs on the parent directory of the
FILE. Then, if this operation is successful, try again to pin the
object.

The code for mnt_bpffs() is a copy of function bpf_mnt_fs() from
iproute2 package (under lib/bpf.c, taken at commit 4b73d52f8a81), with
modifications regarding handling of error messages.

Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 tools/bpf/bpftool/common.c | 71 ++++++++++++++++++++++++++++++++++++++++------
 tools/bpf/bpftool/main.h   |  2 ++
 2 files changed, 65 insertions(+), 8 deletions(-)

diff --git a/tools/bpf/bpftool/common.c b/tools/bpf/bpftool/common.c
index b2533f1cae3e..f0288269dae8 100644
--- a/tools/bpf/bpftool/common.c
+++ b/tools/bpf/bpftool/common.c
@@ -42,6 +42,7 @@
 #include <unistd.h>
 #include <linux/limits.h>
 #include <linux/magic.h>
+#include <sys/mount.h>
 #include <sys/types.h>
 #include <sys/vfs.h>
 
@@ -59,6 +60,37 @@ static bool is_bpffs(char *path)
 	return (unsigned long)st_fs.f_type == BPF_FS_MAGIC;
 }
 
+static int mnt_bpffs(const char *target, char *buff, size_t bufflen)
+{
+	bool bind_done = false;
+
+	while (mount("", target, "none", MS_PRIVATE | MS_REC, NULL)) {
+		if (errno != EINVAL || bind_done) {
+			snprintf(buff, bufflen,
+				 "mount --make-private %s failed: %s",
+				 target, strerror(errno));
+			return -1;
+		}
+
+		if (mount(target, target, "none", MS_BIND, NULL)) {
+			snprintf(buff, bufflen,
+				 "mount --bind %s %s failed: %s",
+				 target, target, strerror(errno));
+			return -1;
+		}
+
+		bind_done = true;
+	}
+
+	if (mount("bpf", target, "bpf", 0, "mode=0700")) {
+		snprintf(buff, bufflen, "mount -t bpf bpf %s failed: %s",
+			 target, strerror(errno));
+		return -1;
+	}
+
+	return 0;
+}
+
 int open_obj_pinned_any(char *path, enum bpf_obj_type exp_type)
 {
 	enum bpf_obj_type type;
@@ -89,8 +121,11 @@ int open_obj_pinned_any(char *path, enum bpf_obj_type exp_type)
 
 int do_pin_any(int argc, char **argv, int (*get_fd_by_id)(__u32))
 {
+	char err_str[ERR_MAX_LEN];
 	unsigned int id;
 	char *endptr;
+	char *file;
+	char *dir;
 	int err;
 	int fd;
 
@@ -117,16 +152,36 @@ int do_pin_any(int argc, char **argv, int (*get_fd_by_id)(__u32))
 	}
 
 	err = bpf_obj_pin(fd, *argv);
-	close(fd);
-	if (err) {
-		p_err("can't pin the object (%s): %s", *argv,
-		      errno == EACCES && !is_bpffs(dirname(*argv)) ?
-		    "directory not in bpf file system (bpffs)" :
-		    strerror(errno));
-		return -1;
+	if (!err)
+		goto out_close;
+
+	file = malloc(strlen(*argv) + 1);
+	strcpy(file, *argv);
+	dir = dirname(file);
+
+	if (errno != EPERM || is_bpffs(dir)) {
+		p_err("can't pin the object (%s): %s", *argv, strerror(errno));
+		goto out_free;
 	}
 
-	return 0;
+	/* Attempt to mount bpffs, then retry pinning. */
+	err = mnt_bpffs(dir, err_str, ERR_MAX_LEN);
+	if (!err) {
+		err = bpf_obj_pin(fd, *argv);
+		if (err)
+			p_err("can't pin the object (%s): %s", *argv,
+			      strerror(errno));
+	} else {
+		err_str[ERR_MAX_LEN - 1] = '\0';
+		p_err("can't mount BPF file system to pin the object (%s): %s",
+		      *argv, err_str);
+	}
+
+out_free:
+	free(file);
+out_close:
+	close(fd);
+	return err;
 }
 
 const char *get_fd_type_name(enum bpf_obj_type type)
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index 2f94bed03a8d..d315d01be645 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -51,6 +51,8 @@
 #define NEXT_ARGP()	({ (*argc)--; (*argv)++; if (*argc < 0) usage(); })
 #define BAD_ARG()	({ p_err("what is '%s'?\n", *argv); -1; })
 
+#define ERR_MAX_LEN	1024
+
 #define BPF_TAG_FMT	"%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx"
 
 #define HELP_SPEC_PROGRAM						\
-- 
2.14.1

^ permalink raw reply related

* Re: [PATCH] net: LLC: Convert timers to use timer_setup()
From: David Miller @ 2017-10-25  3:06 UTC (permalink / raw)
  To: keescook
  Cc: edumazet, ishkamiel, paulmck, elena.reshetova, netdev,
	linux-kernel
In-Reply-To: <20171024084548.GA23776@beast>

From: Kees Cook <keescook@chromium.org>
Date: Tue, 24 Oct 2017 01:45:48 -0700

> In preparation for unconditionally passing the struct timer_list pointer to
> all timer callbacks, switch to using the new timer_setup() and from_timer()
> to pass the timer pointer explicitly.
> 
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Hans Liljestrand <ishkamiel@gmail.com>
> Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> Cc: "Reshetova, Elena" <elena.reshetova@intel.com>
> Cc: netdev@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH] net: ax25: Convert timers to use timer_setup()
From: David Miller @ 2017-10-25  3:04 UTC (permalink / raw)
  To: keescook; +Cc: jreuter, ralf, linux-hams, netdev, linux-kernel
In-Reply-To: <20171024084539.GA23749@beast>

From: Kees Cook <keescook@chromium.org>
Date: Tue, 24 Oct 2017 01:45:39 -0700

> In preparation for unconditionally passing the struct timer_list pointer to
> all timer callbacks, switch to using the new timer_setup() and from_timer()
> to pass the timer pointer explicitly.
> 
> Cc: Joerg Reuter <jreuter@yaina.de>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: linux-hams@vger.kernel.org
> Cc: netdev@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>

Applied.

^ permalink raw reply

* Re: [PATCH] net: sctp: Convert timers to use timer_setup()
From: David Miller @ 2017-10-25  3:02 UTC (permalink / raw)
  To: keescook; +Cc: vyasevich, nhorman, linux-sctp, netdev, linux-kernel
In-Reply-To: <20171024084531.GA23722@beast>

From: Kees Cook <keescook@chromium.org>
Date: Tue, 24 Oct 2017 01:45:31 -0700

> In preparation for unconditionally passing the struct timer_list pointer to
> all timer callbacks, switch to using the new timer_setup() and from_timer()
> to pass the timer pointer explicitly.
> 
> Cc: Vlad Yasevich <vyasevich@gmail.com>
> Cc: Neil Horman <nhorman@tuxdriver.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: linux-sctp@vger.kernel.org
> Cc: netdev@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>

Applied.

^ permalink raw reply

* Re: [PATCH net-next v3 01/10] devlink: Add permanent config parameter get/set operations
From: David Miller @ 2017-10-25  2:53 UTC (permalink / raw)
  To: steven.lin1; +Cc: netdev, jiri, michael.chan, linville, gospo, yuvalm
In-Reply-To: <1508875962-39233-2-git-send-email-steven.lin1@broadcom.com>

From: Steve Lin <steven.lin1@broadcom.com>
Date: Tue, 24 Oct 2017 16:12:33 -0400

> +	switch (type) {
> +	case NLA_U8:
> +		val = *((u8 *)value);
> +		if (nla_put_u8(msg, DEVLINK_ATTR_PERM_CONFIG_VALUE, val))
> +			goto nest_err;
> +		break;
> +	case NLA_U16:
> +		val = *((u16 *)value);
> +		if (nla_put_u16(msg, DEVLINK_ATTR_PERM_CONFIG_VALUE, val))
> +			goto nest_err;
> +		break;
> +	case NLA_U32:
> +		val = *((u32 *)value);
> +		if (nla_put_u32(msg, DEVLINK_ATTR_PERM_CONFIG_VALUE, val))
> +			goto nest_err;
> +		break;
> +	}
> +	nla_nest_end(msg, param_attr);
> +
> +	kfree(value);

You have to get the endianness right on these things.  Netlink could
theoretically be done over a network, so just saying "device and
system endianness match" is not a valid argument.

Typing and endianness is so important for interfaces like this, so
please contruct the interfaces such that the compiler and 'sparse' can
help us make sure it is done properly.

Thanks.

^ permalink raw reply

* Re: [PATCH net-next v3 2/2] net: ethernet: socionext: add AVE ethernet driver
From: Masahiro Yamada @ 2017-10-25  2:52 UTC (permalink / raw)
  To: Kunihiko Hayashi
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA, Andrew Lunn, Florian Fainelli,
	Rob Herring, Mark Rutland, linux-arm-kernel,
	Linux Kernel Mailing List, devicetree-u79uwXL29TY76Z2rM5mHXA,
	Masami Hiramatsu, Jassi Brar
In-Reply-To: <1508893679-11727-3-git-send-email-hayashi.kunihiko-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>

2017-10-25 10:07 GMT+09:00 Kunihiko Hayashi <hayashi.kunihiko-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>:

> +
> +/* Descriptor Control Register Group */
> +#define AVE_DESCC      0x300   /* Descriptor Control */
> +#define AVE_TXDC       0x304   /* TX Descriptor Configuration */
> +#define AVE_RXDC0      0x308   /* RX Descriptor Ring0 Configuration */
> +#define AVE_IIRQC      0x34c   /* Interval IRQ Control */
> +
> +/* Packet Filter Register Group */
> +#define AVE_PKTF_BASE          0x800   /* PF Base Address */
> +#define AVE_PFMBYTE_BASE       0xd00   /* PF Mask Byte Base Address */
> +#define AVE_PFMBIT_BASE                0xe00   /* PF Mask Bit Base Address */
> +#define AVE_PFSEL_BASE         0xf00   /* PF Selector Base Address */
> +#define AVE_PFEN               0xffc   /* Packet Filter Enable */
> +#define AVE_PKTF(ent)          (AVE_PKTF_BASE + (ent) * 0x40)
> +#define AVE_PFMBYTE(ent)       (AVE_PFMBYTE_BASE + (ent) * 8)
> +#define AVE_PFMBIT(ent)                (AVE_PFMBIT_BASE + (ent) * 4)
> +#define AVE_PFSEL(ent)         (AVE_PFSEL_BASE + (ent) * 4)
> +
> +/* 64bit descriptor memory */
> +#define AVE_DESC_SIZE_64       12      /* Descriptor Size */
> +
> +#define AVE_TXDM_64            0x1000  /* Tx Descriptor Memory */
> +#define AVE_RXDM_64            0x1c00  /* Rx Descriptor Memory */
> +
> +#define AVE_TXDM_SIZE_64       0x0ba0  /* Tx Descriptor Memory Size 3KB */
> +#define AVE_RXDM_SIZE_64       0x6000  /* Rx Descriptor Memory Size 24KB */
> +
> +/* 32bit descriptor memory */
> +#define AVE_DESC_SIZE_32       8       /* Descriptor Size */
> +
> +#define AVE_TXDM_32            0x1000  /* Tx Descriptor Memory */
> +#define AVE_RXDM_32            0x1800  /* Rx Descriptor Memory */
> +
> +#define AVE_TXDM_SIZE_32       0x07c0  /* Tx Descriptor Memory Size 2KB */
> +#define AVE_RXDM_SIZE_32       0x4000  /* Rx Descriptor Memory Size 16KB */
> +
> +/* RMII Bridge Register Group */
> +#define AVE_RSTCTRL            0x8028  /* Reset control */
> +#define AVE_RSTCTRL_RMIIRST    BIT(16)
> +#define AVE_LINKSEL            0x8034  /* Link speed setting */
> +#define AVE_LINKSEL_100M       BIT(0)
> +
> +/* AVE_GRR */
> +#define AVE_GRR_RXFFR          BIT(5)  /* Reset RxFIFO */
> +#define AVE_GRR_PHYRST         BIT(4)  /* Reset external PHY */
> +#define AVE_GRR_GRST           BIT(0)  /* Reset all MAC */
> +
> +/* AVE_GISR (common with GIMR) */
> +#define AVE_GI_PHY             BIT(24) /* PHY interrupt */
> +#define AVE_GI_TX              BIT(16) /* Tx complete */
> +#define AVE_GI_RXERR           BIT(8)  /* Receive frame more than max size */
> +#define AVE_GI_RXOVF           BIT(7)  /* Overflow at the RxFIFO */
> +#define AVE_GI_RXDROP          BIT(6)  /* Drop packet */
> +#define AVE_GI_RXIINT          BIT(5)  /* Interval interrupt */
> +
> +/* AVE_TXCR */
> +#define AVE_TXCR_FLOCTR                BIT(18) /* Flow control */
> +#define AVE_TXCR_TXSPD_1G      BIT(17)
> +#define AVE_TXCR_TXSPD_100     BIT(16)
> +
> +/* AVE_RXCR */
> +#define AVE_RXCR_RXEN          BIT(30) /* Rx enable */
> +#define AVE_RXCR_FDUPEN                BIT(22) /* Interface mode */
> +#define AVE_RXCR_FLOCTR                BIT(21) /* Flow control */
> +#define AVE_RXCR_AFEN          BIT(19) /* MAC address filter */
> +#define AVE_RXCR_DRPEN         BIT(18) /* Drop pause frame */
> +#define AVE_RXCR_MPSIZ_MASK    GENMASK(10, 0)
> +
> +/* AVE_MDIOCTR */
> +#define AVE_MDIOCTR_RREQ       BIT(3)  /* Read request */
> +#define AVE_MDIOCTR_WREQ       BIT(2)  /* Write request */


BIT() is descending.



> +/* AVE_MDIOSR */
> +#define AVE_MDIOSR_STS         BIT(0)  /* access status */
> +
> +/* AVE_DESCC */
> +#define AVE_DESCC_TD           BIT(0)  /* Enable Tx descriptor */
> +#define AVE_DESCC_RDSTP                BIT(4)  /* Pause Rx descriptor */
> +#define AVE_DESCC_RD0          BIT(8)  /* Enable Rx descriptor Ring0 */
> +#define AVE_DESCC_STATUS_MASK  GENMASK(31, 16)


BIT() is ascending.

Please be consistent.






> +
> +static void ave_wdesc_addr(struct net_device *ndev, enum desc_id id,
> +                          int entry, dma_addr_t paddr)
> +{
> +       struct ave_private *priv = netdev_priv(ndev);
> +
> +       ave_wdesc(ndev, id, entry, AVE_DESC_OFS_ADDRL, lower_32_bits(paddr));
> +       if (IS_DESC_64BIT(priv))
> +               ave_wdesc(ndev, id,
> +                         entry, AVE_DESC_OFS_ADDRU, upper_32_bits(paddr));
> +       else if ((u64)paddr > (u64)U32_MAX)
> +               netdev_warn(ndev, "DMA address exceeds the address space\n");
> +}

Yuk!
Why don't you use dma_set_mask() instead of this?




> +
> +static int ave_mdio_busywait(struct net_device *ndev)
> +{
> +       int ret = 0, loop = 100;
> +       u32 mdiosr;
> +
> +       /* wait until completion */
> +       while (--loop) {
> +               mdiosr = ave_r32(ndev, AVE_MDIOSR);
> +               if (!(mdiosr & AVE_MDIOSR_STS))
> +                       break;
> +
> +               usleep_range(10, 20);
> +       }
> +
> +       if (!loop) {
> +               netdev_err(ndev,
> +                          "failed to read from MDIO (status:0x%08x)\n",
> +                          mdiosr);
> +               ret = -ETIMEDOUT;
> +       }
> +
> +       return ret;
> +}


The whole of this function can be replaced with
readl_poll_timeout() unless you stick to ave_r32.


I do not understand why you are such a big fan of
driver-specific accessors like ave_r32/ave_w32.



> +static int ave_mdiobus_read(struct mii_bus *bus, int phyid, int regnum)
> +{
> +       struct net_device *ndev = bus->priv;
> +       u32 mdioctl;
> +       int ret;
> +
> +       /* write address */
> +       ave_w32(ndev, AVE_MDIOAR, (phyid << 8) | regnum);
> +
> +       /* read request */
> +       mdioctl = ave_r32(ndev, AVE_MDIOCTR);
> +       ave_w32(ndev, AVE_MDIOCTR,
> +               (mdioctl | AVE_MDIOCTR_RREQ) & ~AVE_MDIOCTR_WREQ);
> +
> +       ret = ave_mdio_busywait(ndev);
> +       if (ret) {
> +               netdev_err(ndev, "phy-%d reg-%x read failed\n",
> +                          phyid, regnum);
> +               return ret;
> +       }
> +
> +       return ave_r32(ndev, AVE_MDIORDR) & GENMASK(15, 0);
> +}
> +
> +static int ave_mdiobus_write(struct mii_bus *bus,
> +                            int phyid, int regnum, u16 val)
> +{
> +       struct net_device *ndev = bus->priv;
> +       u32 mdioctl;
> +       int ret;
> +
> +       /* write address */
> +       ave_w32(ndev, AVE_MDIOAR, (phyid << 8) | regnum);
> +
> +       /* write data */
> +       ave_w32(ndev, AVE_MDIOWDR, val);
> +
> +       /* write request */
> +       mdioctl = ave_r32(ndev, AVE_MDIOCTR);
> +       ave_w32(ndev, AVE_MDIOCTR,
> +               (mdioctl | AVE_MDIOCTR_WREQ) & ~AVE_MDIOCTR_RREQ);
> +
> +       ret = ave_mdio_busywait(ndev);
> +       if (ret)
> +               netdev_err(ndev, "phy-%d reg-%x write failed\n",
> +                          phyid, regnum);
> +
> +       return ret;
> +}
> +
> +static dma_addr_t ave_dma_map(struct net_device *ndev, struct ave_desc *desc,
> +                             void *ptr, size_t len,
> +                             enum dma_data_direction dir)
> +{
> +       dma_addr_t paddr;
> +
> +       paddr = dma_map_single(ndev->dev.parent, ptr, len, dir);
> +       if (unlikely(dma_mapping_error(ndev->dev.parent, paddr))) {
> +               paddr = (dma_addr_t)-ENOMEM;

Yuk!

Re-write the code.


> +       } else {
> +               desc->skbs_dma = paddr;
> +               desc->skbs_dmalen = len;
> +       }
> +
> +       return paddr;
> +}


-- 
Best Regards
Masahiro Yamada
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH net-next v3 01/10] devlink: Add permanent config parameter get/set operations
From: Steve Lin @ 2017-10-25  2:14 UTC (permalink / raw)
  To: Yuval Mintz
  Cc: Jiri Pirko, davem@davemloft.net, michael.chan@broadcom.com,
	linville@tuxdriver.com, gospo@broadcom.com,
	netdev@vger.kernel.org
In-Reply-To: <AM0PR0502MB368378B824556ED230AF10C7BF470@AM0PR0502MB3683.eurprd05.prod.outlook.com>

On Tue, Oct 24, 2017 at 5:22 PM, Yuval Mintz <yuvalm@mellanox.com> wrote:
>> Add support for permanent config parameter get/set commands. Used
>> for persistent device configuration parameters.
>>
> ...
>> +     int (*perm_config_get)(struct devlink *devlink,
>> +                            enum devlink_perm_config_param param, u8
>> type,
>> +                            void *value);
>> +     int (*perm_config_set)(struct devlink *devlink,
>> +                            enum devlink_perm_config_param param, u8
>> type,
>> +                            void *value, u8 *restart_reqd);
>>  };
>> +static int devlink_nl_single_param_get(struct sk_buff *msg,
>> +                                    struct devlink *devlink,
>> +                                    u32 param, u8 type)
>> +{
>> +     const struct devlink_ops *ops = devlink->ops;
>> +     struct nlattr *param_attr;
>> +     void *value;
>> +     u32 val;
>> +     int err;
>> +
>> +     /* Allocate buffer for parameter value */
>> +     switch (type) {
>> +     case NLA_U8:
>> +             value = kmalloc(sizeof(u8), GFP_KERNEL);
>> +             break;
>> +     case NLA_U16:
>> +             value = kmalloc(sizeof(u16), GFP_KERNEL);
>> +             break;
>> +     case NLA_U32:
>> +             value = kmalloc(sizeof(u32), GFP_KERNEL);
>> +             break;
>> +     default:
>> +             return -EINVAL; /* Unsupported Type */
>> +     }
>> +
>> +     if (!value)
>> +             return -ENOMEM;
>> +
>> +     err = ops->perm_config_get(devlink, param, type, value);
>> +     if (err)
>> +             return err;
>
> I suspect this logic might be risky - its dependent on the driver to cast the
> 'value' into the proper type or else, E.g., the following switch might break
> for BE platforms.
> Is there any reason to have the devlink <-> driver API be based on void*
> and not on some typed data [of sufficient size]?
> ...
>> +     switch (type) {
>> +     case NLA_U8:
>> +             val = *((u8 *)value);
>> +             if (nla_put_u8(msg, DEVLINK_ATTR_PERM_CONFIG_VALUE,
>> val))
>> +                     goto nest_err;
>> +             break;
>> +     case NLA_U16:
>> +             val = *((u16 *)value);
>> +             if (nla_put_u16(msg,
>> DEVLINK_ATTR_PERM_CONFIG_VALUE, val))
>> +                     goto nest_err;
>> +             break;
>> +     case NLA_U32:
>> +             val = *((u32 *)value);
>> +             if (nla_put_u32(msg,
>> DEVLINK_ATTR_PERM_CONFIG_VALUE, val))
>> +                     goto nest_err;
>> +             break;
>> +     }

Why might this break on a BE system?  It's not as though driver will
be compiled LE and kernel BE or vice versa - as long as driver and
kernel are same endian-ness, I would think it should be okay?

In general, the issue is that the parameter could be any of the
netlink types (per Jiri's suggestion to the previous version of this
patch).  So, we allocate some space, tell the driver the type we're
expecting (the type argument to the perm_config_get() function), and
yes, we rely on the driver to write something of the type we request
to the pointer we provide.  Are you suggesting defining a union of
U8/U16/U32, and passing a pointer to that for the driver to fill in?
The issue is that whatever the types we support now, we want future
parameters to be able to be of arbitrary types.  Defining the
interface to use the void pointer means that some future parameter can
be of some other type, without having to update all the drivers using
this API...

Or did I misunderstand your suggestion?

^ permalink raw reply

* Re: [PATCH] rxrpc: recvmsg: use BUG_ON instead of if condition followed by BUG
From: Gustavo A. R. Silva @ 2017-10-25  2:02 UTC (permalink / raw)
  To: Tom Herbert; +Cc: David S. Miller, Linux Kernel Network Developers, LKML
In-Reply-To: <CALx6S34LmkS1kYmYxaNZcUHFc8=f_oxauojG0b-t0zAD1YRKkQ@mail.gmail.com>

Hi Tom,

Quoting Tom Herbert <tom@herbertland.com>:

> Please combine these related patches fixing BUG in rxrpc into a patch
> set with proper annotation,
>

I can do that.

> Also, can any of these BUG_ONs be replaced by WARN_ONs? Warnings are
> generally preferable to crashing the system.
>

What about WARN_ON_ONCE instead?

Thanks
--
Gustavo A. R. Silva

> Tom
>
>
> On Tue, Oct 24, 2017 at 9:20 AM, Gustavo A. R. Silva
> <garsilva@embeddedor.com> wrote:
>> Use BUG_ON instead of if condition followed by BUG.
>>
>> This issue was detected with the help of Coccinelle.
>>
>> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
>> ---
>>  net/rxrpc/recvmsg.c | 6 ++----
>>  1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/net/rxrpc/recvmsg.c b/net/rxrpc/recvmsg.c
>> index bdece21..9598b92 100644
>> --- a/net/rxrpc/recvmsg.c
>> +++ b/net/rxrpc/recvmsg.c
>> @@ -243,8 +243,7 @@ static int rxrpc_verify_packet(struct  
>> rxrpc_call *call, struct sk_buff *skb,
>>          */
>>         if ((annotation & RXRPC_RX_ANNO_JUMBO) > 1) {
>>                 __be16 tmp;
>> -               if (skb_copy_bits(skb, offset - 2, &tmp, 2) < 0)
>> -                       BUG();
>> +               BUG_ON(skb_copy_bits(skb, offset - 2, &tmp, 2) < 0);
>>                 cksum = ntohs(tmp);
>>                 seq += (annotation & RXRPC_RX_ANNO_JUMBO) - 1;
>>         }
>> @@ -503,8 +502,7 @@ int rxrpc_recvmsg(struct socket *sock, struct  
>> msghdr *msg, size_t len,
>>
>>         release_sock(&rx->sk);
>>
>> -       if (test_bit(RXRPC_CALL_RELEASED, &call->flags))
>> -               BUG();
>> +       BUG_ON(test_bit(RXRPC_CALL_RELEASED, &call->flags));
>>
>>         if (test_bit(RXRPC_CALL_HAS_USERID, &call->flags)) {
>>                 if (flags & MSG_CMSG_COMPAT) {
>> --
>> 2.7.4
>>

^ permalink raw reply

* Re: [PATCH net-next v3 06/10] bnxt: Add devlink support for config get/set
From: Steve Lin @ 2017-10-25  2:01 UTC (permalink / raw)
  To: Michael Chan
  Cc: Netdev, Jiri Pirko, David Miller, John W. Linville,
	Andy Gospodarek, Yuval Mintz
In-Reply-To: <CACKFLimoALDGk-ErroVWRGGQVa9QrTQQFXEuNAWUJvuaqGH41w@mail.gmail.com>

On Tue, Oct 24, 2017 at 4:52 PM, Michael Chan <michael.chan@broadcom.com> wrote:
> On Tue, Oct 24, 2017 at 1:12 PM, Steve Lin <steven.lin1@broadcom.com> wrote:
>> Implements get and set of configuration parameters using new devlink
>> config get/set API.  Parameters themselves defined in later patches.
>>
>> Signed-off-by: Steve Lin <steven.lin1@broadcom.com>
>> Acked-by: Andy Gospodarek <gospo@broadcom.com>
>> ---
>>  drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 262 +++++++++++++++++++++-
>>  drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h |  17 ++
>>  drivers/net/ethernet/broadcom/bnxt/bnxt_hsi.h     | 100 +++++++++
>>  3 files changed, 373 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
>> index f3f6aa8..81ab77e 100644
>> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
>> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
>> @@ -14,11 +14,261 @@
>>  #include "bnxt_vfr.h"
>>  #include "bnxt_devlink.h"
>>
>> -static const struct devlink_ops bnxt_dl_ops = {
>> +struct bnxt_drv_cfgparam bnxt_drv_cfgparam_list[] = {
>> +};
>> +
>> +#define BNXT_NUM_DRV_CFGPARAM ARRAY_SIZE(bnxt_drv_cfgparam_list)
>> +
>> +static int bnxt_nvm_read(struct bnxt *bp, int nvm_param, int idx,
>> +                        void *buf, int size)
>> +{
>> +       struct hwrm_nvm_get_variable_input req = {0};
>> +       dma_addr_t dest_data_dma_addr;
>> +       void *dest_data_addr = NULL;
>> +       int bytesize;
>> +       int rc;
>> +
>> +       bytesize = (size + 7) / BITS_PER_BYTE;
>> +       dest_data_addr = dma_alloc_coherent(&bp->pdev->dev, bytesize,
>> +                                           &dest_data_dma_addr, GFP_KERNEL);
>> +       if (!dest_data_addr) {
>> +               netdev_err(bp->dev, "dma_alloc_coherent failure\n");
>> +               return -ENOMEM;
>> +       }
>> +
>> +       bnxt_hwrm_cmd_hdr_init(bp, &req, HWRM_NVM_GET_VARIABLE, -1, -1);
>> +       req.dest_data_addr = cpu_to_le64(dest_data_dma_addr);
>> +       req.data_len = cpu_to_le16(size);
>> +       req.option_num = cpu_to_le16(nvm_param);
>> +       req.index_0 = cpu_to_le16(idx);
>> +       if (idx != 0)
>> +               req.dimensions = cpu_to_le16(1);
>> +
>> +       rc = _hwrm_send_message(bp, &req, sizeof(req), HWRM_CMD_TIMEOUT);
>
> This won't have the proper mutex protection.  You should call
> hwrm_send_message() instead.

Ok, thanks, I'll fix that.

^ permalink raw reply

* Re: [PATCH] tap: double-free in error path in tap_open()
From: Jason Wang @ 2017-10-25  1:55 UTC (permalink / raw)
  To: Girish Moodalbail, netdev, davem
In-Reply-To: <1508881270-4124-1-git-send-email-girish.moodalbail@oracle.com>



On 2017年10月25日 05:41, Girish Moodalbail wrote:
> Double free of skb_array in tap module is causing kernel panic. When
> tap_set_queue() fails we free skb_array right away by calling
> skb_array_cleanup(). However, later on skb_array_cleanup() is called
> again by tap_sock_destruct through sock_put(). This patch fixes that
> issue.
>
> Fixes: 362899b8725b35e3 (macvtap: switch to use skb array)
> Signed-off-by: Girish Moodalbail <girish.moodalbail@oracle.com>
> ---
>   drivers/net/tap.c | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/tap.c b/drivers/net/tap.c
> index 21b71ae..878520b 100644
> --- a/drivers/net/tap.c
> +++ b/drivers/net/tap.c
> @@ -542,20 +542,20 @@ static int tap_open(struct inode *inode, struct file *file)
>   
>   	err = -ENOMEM;
>   	if (skb_array_init(&q->skb_array, tap->dev->tx_queue_len, GFP_KERNEL))
> -		goto err_array;
> +		goto err_put;

Looks like this will cause skb_array_cleanup() to be called when skb 
array was uninitialized?

Thanks

>   
>   	err = tap_set_queue(tap, file, q);
> -	if (err)
> -		goto err_queue;
> +	if (err) {
> +		/* tap_sock_destruct() will take care of freeing skb_array */
> +		goto err_put;
> +	}
>   
>   	dev_put(tap->dev);
>   
>   	rtnl_unlock();
>   	return err;
>   
> -err_queue:
> -	skb_array_cleanup(&q->skb_array);
> -err_array:
> +err_put:
>   	sock_put(&q->sk);
>   err:
>   	if (tap)

^ permalink raw reply

* Re: [PATCHv3 net-next 0/4] net: remove rtmsg_ifinfo used in bridge and bonding
From: David Miller @ 2017-10-25  1:55 UTC (permalink / raw)
  To: lucien.xin; +Cc: netdev, dsahern, jiri
In-Reply-To: <cover.1508824093.git.lucien.xin@gmail.com>

From: Xin Long <lucien.xin@gmail.com>
Date: Tue, 24 Oct 2017 13:54:16 +0800

> It's better to send notifications to userspace by the events in
> rtnetlink_event, instead of calling rtmsg_ifinfo directly.
> 
> This patcheset is to remove rtmsg_ifinfo called in bonding and
> bridge, the notifications can be handled by NETDEV_CHANGEUPPER
> and NETDEV_CHANGELOWERSTATE events in rtnetlink_event.
> 
> It could also fix some redundant notifications from bonding and
> bridge.
> 
> v1->v2:
>   - post to net-next.git instead of net.git, for it's more like an
>     improvement for bonding
> v2->v3:
>   - add patch 1/4 to remove rtmsg_ifinfo called in add_del_if

Series applied, thanks.

^ permalink raw reply

* Re: [PATCH net] net: dsa: check master device before put
From: David Miller @ 2017-10-25  1:49 UTC (permalink / raw)
  To: vivien.didelot; +Cc: netdev, linux-kernel, kernel, f.fainelli, andrew
In-Reply-To: <20171024203719.25183-1-vivien.didelot@savoirfairelinux.com>

From: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Date: Tue, 24 Oct 2017 16:37:19 -0400

> In the case of pdata, the dsa_cpu_parse function calls dev_put() before
> making sure it isn't NULL. Fix this.
> 
> Fixes: 71e0bbde0d88 ("net: dsa: Add support for platform data")
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>

Applied and queued up for -stable.

Thank you.

^ permalink raw reply

* Re: [PATCH net-next v3 0/3] bpf: permit multiple bpf attachments for a single perf tracepoint event
From: David Miller @ 2017-10-25  1:48 UTC (permalink / raw)
  To: yhs; +Cc: peterz, rostedt, ast, daniel, kafai, netdev, kernel-team
In-Reply-To: <20171024065309.2401893-1-yhs@fb.com>

From: Yonghong Song <yhs@fb.com>
Date: Mon, 23 Oct 2017 23:53:06 -0700

> This patch set adds support to permit multiple bpf prog attachments
> for a single perf tracepoint event. Patch 1 does some cleanup such
> that perf_event_{set|free}_bpf_handler is called under the
> same condition. Patch 2 has the core implementation, and
> Patch 3 adds a test case.
> 
> Changelogs:
> v2 -> v3:
>   . fix compilation error.
> v1 -> v2:
>   . fix a potential deadlock issue discovered by Daniel.
>   . fix some coding style issues.

Series applied.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox