Netdev List
 help / color / mirror / Atom feed
* Re: net build error
From: Dmitry Vyukov @ 2018-09-23  8:14 UTC (permalink / raw)
  To: syzbot, netdev; +Cc: LKML, syzkaller-bugs
In-Reply-To: <0000000000002cdabf057685027b@google.com>

On Sun, Sep 23, 2018 at 9:42 AM, syzbot
<syzbot+a72ba31e3224309179d4@syzkaller.appspotmail.com> wrote:
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit:    474ff2600889 net-ethtool: ETHTOOL_GUFO did not and should ..
> git tree:       net
> console output: https://syzkaller.appspot.com/x/log.txt?x=164c4059400000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=5fa12be50bca08d8
> dashboard link: https://syzkaller.appspot.com/bug?extid=a72ba31e3224309179d4
> compiler:       gcc (GCC) 8.0.1 20180413 (experimental)
>
> Unfortunately, I don't have any reproducer for this crash yet.
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+a72ba31e3224309179d4@syzkaller.appspotmail.com

+netdev

The root cause:


net/rds/ib.o:(.data..percpu..shared_aligned+0x0): multiple definition
of `rds_ib_stats'
net/rds/rdma_transport.o:(.data..percpu..shared_aligned+0x0): first defined here
net/rds/ib_cm.o:(.data..percpu..shared_aligned+0x0): multiple
definition of `rds_ib_stats'
net/rds/rdma_transport.o:/syzkaller/managers/upstream-net-this-kasan-gce/kernel/net/rds/rdma_transport.c:284:
first defined here
net/rds/ib_recv.o:(.data..percpu..shared_aligned+0x0): multiple
definition of `rds_ib_stats'
net/rds/rdma_transport.o:/syzkaller/managers/upstream-net-this-kasan-gce/kernel/net/rds/rdma_transport.c:284:
first defined here
net/rds/ib_ring.o:(.data..percpu..shared_aligned+0x0): multiple
definition of `rds_ib_stats'
net/rds/rdma_transport.o:/syzkaller/managers/upstream-net-this-kasan-gce/kernel/net/rds/rdma_transport.c:284:
first defined here
net/rds/ib_send.o:(.data..percpu..shared_aligned+0x0): multiple
definition of `rds_ib_stats'
net/rds/rdma_transport.o:/syzkaller/managers/upstream-net-this-kasan-gce/kernel/net/rds/rdma_transport.c:284:
first defined here
net/rds/ib_stats.o:(.data..percpu..shared_aligned+0x0): multiple
definition of `rds_ib_stats'
net/rds/rdma_transport.o:/syzkaller/managers/upstream-net-this-kasan-gce/kernel/net/rds/rdma_transport.c:284:
first defined here
net/rds/ib_sysctl.o:(.data..percpu..shared_aligned+0x0): multiple
definition of `rds_ib_stats'
net/rds/rdma_transport.o:/syzkaller/managers/upstream-net-this-kasan-gce/kernel/net/rds/rdma_transport.c:284:
first defined here
net/rds/ib_rdma.o:(.data..percpu..shared_aligned+0x0): multiple
definition of `rds_ib_stats'
net/rds/rdma_transport.o:/syzkaller/managers/upstream-net-this-kasan-gce/kernel/net/rds/rdma_transport.c:284:
first defined here
net/rds/ib_fmr.o:(.data..percpu..shared_aligned+0x0): multiple
definition of `rds_ib_stats'
net/rds/rdma_transport.o:/syzkaller/managers/upstream-net-this-kasan-gce/kernel/net/rds/rdma_transport.c:284:
first defined here
net/rds/ib_frmr.o:(.data..percpu..shared_aligned+0x0): multiple
definition of `rds_ib_stats'
net/rds/rdma_transport.o:/syzkaller/managers/upstream-net-this-kasan-gce/kernel/net/rds/rdma_transport.c:284:
first defined here
Makefile:1030: recipe for target 'vmlinux' failed
make: *** [vmlinux] Error 1


> ---
> This bug is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syzkaller@googlegroups.com.
>
> syzbot will keep track of this bug report. See:
> https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
> syzbot.

^ permalink raw reply

* Re: [PATCH net-next 1/5] net: allow binding socket in a VRF when there's an unbound socket
From: kbuild test robot @ 2018-09-23  8:47 UTC (permalink / raw)
  To: Mike Manning; +Cc: kbuild-all, netdev, Robert Shearman
In-Reply-To: <20180920085848.17721-2-mmanning@vyatta.att-mail.com>

[-- Attachment #1: Type: text/plain, Size: 1476 bytes --]

Hi Robert,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Mike-Manning/vrf-allow-simultaneous-service-instances-in-default-and-other-VRFs/20180923-162308
config: x86_64-randconfig-x004-201838 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   In file included from include/net/tcp.h:36:0,
                    from net/core/sock.c:143:
   include/net/inet_hashtables.h: In function 'inet_sk_bound_dev_eq':
>> include/net/inet_hashtables.h:196:29: error: 'struct netns_ipv4' has no member named 'sysctl_tcp_l3mdev_accept'; did you mean 'sysctl_tcp_fwmark_accept'?
      return !sdif || net->ipv4.sysctl_tcp_l3mdev_accept;
                                ^~~~~~~~~~~~~~~~~~~~~~~~
                                sysctl_tcp_fwmark_accept

vim +196 include/net/inet_hashtables.h

   191	
   192	static inline bool inet_sk_bound_dev_eq(struct net *net, int bound_dev_if,
   193						int dif, int sdif)
   194	{
   195		if (!bound_dev_if)
 > 196			return !sdif || net->ipv4.sysctl_tcp_l3mdev_accept;
   197		return bound_dev_if == dif || bound_dev_if == sdif;
   198	}
   199	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 32260 bytes --]

^ permalink raw reply

* [PATCH 2/2] netfilter: nf_tables: add requirements for connsecmark support
From: Christian Göttsche @ 2018-09-23  9:16 UTC (permalink / raw)
  To: pablo, kadlec, fw, davem, netfilter-devel, coreteam, netdev,
	linux-kernel, paul, sds, eparis, jmorris, serge, selinux,
	linux-security-module
In-Reply-To: <20180923091611.19815-1-cgzones@googlemail.com>

Add ability to set the connection tracking secmark value.

Add ability to set the meta secmark value.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---

Based on nf-next
Tested with v4.18.8

 net/netfilter/nft_ct.c   | 15 +++++++++++++++
 net/netfilter/nft_meta.c |  8 ++++++++
 2 files changed, 23 insertions(+)

diff --git a/net/netfilter/nft_ct.c b/net/netfilter/nft_ct.c
index d74afa707..dcc451c20 100644
--- a/net/netfilter/nft_ct.c
+++ b/net/netfilter/nft_ct.c
@@ -298,6 +298,14 @@ static void nft_ct_set_eval(const struct nft_expr *expr,
 		}
 		break;
 #endif
+#ifdef CONFIG_NF_CONNTRACK_SECMARK
+	case NFT_CT_SECMARK:
+		if (ct->secmark != value) {
+			ct->secmark = value;
+			nf_conntrack_event_cache(IPCT_SECMARK, ct);
+		}
+		break;
+#endif
 #ifdef CONFIG_NF_CONNTRACK_LABELS
 	case NFT_CT_LABELS:
 		nf_connlabels_replace(ct,
@@ -564,6 +572,13 @@ static int nft_ct_set_init(const struct nft_ctx *ctx,
 			return -EINVAL;
 		len = sizeof(u32);
 		break;
+#endif
+#ifdef CONFIG_NF_CONNTRACK_SECMARK
+	case NFT_CT_SECMARK:
+		if (tb[NFTA_CT_DIRECTION])
+			return -EINVAL;
+		len = sizeof(u32);
+		break;
 #endif
 	default:
 		return -EOPNOTSUPP;
diff --git a/net/netfilter/nft_meta.c b/net/netfilter/nft_meta.c
index ac5df9508..555fcd66b 100644
--- a/net/netfilter/nft_meta.c
+++ b/net/netfilter/nft_meta.c
@@ -284,6 +284,11 @@ static void nft_meta_set_eval(const struct nft_expr *expr,
 
 		skb->nf_trace = !!value8;
 		break;
+#ifdef CONFIG_NETWORK_SECMARK
+	case NFT_META_SECMARK:
+		skb->secmark = value;
+		break;
+#endif
 	default:
 		WARN_ON(1);
 	}
@@ -436,6 +441,9 @@ static int nft_meta_set_init(const struct nft_ctx *ctx,
 	switch (priv->key) {
 	case NFT_META_MARK:
 	case NFT_META_PRIORITY:
+#ifdef CONFIG_NETWORK_SECMARK
+	case NFT_META_SECMARK:
+#endif
 		len = sizeof(u32);
 		break;
 	case NFT_META_NFTRACE:
-- 
2.19.0

^ permalink raw reply related

* Re: [PATCH v2 1/2] netfilter: nf_tables: add SECMARK support
From: Christian Göttsche @ 2018-09-23 15:31 UTC (permalink / raw)
  To: fw
  Cc: pablo, kadlec, davem, netfilter-devel, coreteam, netdev,
	linux-kernel, Paul Moore, Stephen Smalley, Eric Paris, jmorris,
	serge, selinux, linux-security-module
In-Reply-To: <20180923135555.7kwa3kyachwcfy24@breakpoint.cc>

> > +struct nft_secmark {
> > +     char ctx[NFT_SECMARK_CTX_MAXLEN];
> > +     int len;
> > +     u32 secid;
> > +};
>
> Can you change this to:
>
> struct nft_secmark {
>         u32 secid;
>         char *ctx;
> };

Does the nla_policy struct needs an update too? (regarding then .len member)

+static const struct nla_policy nft_secmark_policy[NFTA_SECMARK_MAX + 1] = {
+        [NFTA_SECMARK_CTX] = { .type = NLA_STRING, .len =
NFT_SECMARK_CTX_MAXLEN },
+}

NFT_SECMARK_CTX_MAXLEN might be dropped then..

^ permalink raw reply

* Re: [PATCH v2 1/2] netfilter: nf_tables: add SECMARK support
From: Florian Westphal @ 2018-09-23 15:41 UTC (permalink / raw)
  To: Christian Göttsche
  Cc: fw, pablo, kadlec, davem, netfilter-devel, coreteam, netdev,
	linux-kernel, Paul Moore, Stephen Smalley, Eric Paris, jmorris,
	serge, selinux, linux-security-module
In-Reply-To: <CAJ2a_DfCmbFmpTngFnXzYy07gvEzN5UhAhrE8Cd8PowevrLUNw@mail.gmail.com>

Christian Göttsche <cgzones@googlemail.com> wrote:
> > Can you change this to:
> >
> > struct nft_secmark {
> >         u32 secid;
> >         char *ctx;
> > };
> 
> Does the nla_policy struct needs an update too? (regarding then .len member)
> 
> +static const struct nla_policy nft_secmark_policy[NFTA_SECMARK_MAX + 1] = {
> +        [NFTA_SECMARK_CTX] = { .type = NLA_STRING, .len =
> NFT_SECMARK_CTX_MAXLEN },
> +}
> 
> NFT_SECMARK_CTX_MAXLEN might be dropped then..

Better keep it, we can always increase this later it if needed.
Given the length matches what xtables uses it should be fine.

^ permalink raw reply

* Re: [PATCH net-next 1/5] net: allow binding socket in a VRF when there's an unbound socket
From: kbuild test robot @ 2018-09-23  9:58 UTC (permalink / raw)
  To: Mike Manning; +Cc: kbuild-all, netdev, Robert Shearman
In-Reply-To: <20180920085848.17721-2-mmanning@vyatta.att-mail.com>

[-- Attachment #1: Type: text/plain, Size: 1361 bytes --]

Hi Robert,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Mike-Manning/vrf-allow-simultaneous-service-instances-in-default-and-other-VRFs/20180923-162308
config: i386-randconfig-x0-09231642 (attached as .config)
compiler: gcc-5 (Debian 5.5.0-3) 5.4.1 20171010
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   In file included from include/net/tcp.h:36:0,
                    from net/ipv6/af_inet6.c:51:
   include/net/inet_hashtables.h: In function 'inet_sk_bound_dev_eq':
>> include/net/inet_hashtables.h:196:28: error: 'struct netns_ipv4' has no member named 'sysctl_tcp_l3mdev_accept'
      return !sdif || net->ipv4.sysctl_tcp_l3mdev_accept;
                               ^

vim +196 include/net/inet_hashtables.h

   191	
   192	static inline bool inet_sk_bound_dev_eq(struct net *net, int bound_dev_if,
   193						int dif, int sdif)
   194	{
   195		if (!bound_dev_if)
 > 196			return !sdif || net->ipv4.sysctl_tcp_l3mdev_accept;
   197		return bound_dev_if == dif || bound_dev_if == sdif;
   198	}
   199	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30848 bytes --]

^ permalink raw reply

* Re: [PATCH 2/2] netfilter: nf_tables: add requirements for connsecmark support
From: kbuild test robot @ 2018-09-23 17:13 UTC (permalink / raw)
  To: Christian Göttsche
  Cc: kbuild-all, pablo, kadlec, fw, davem, netfilter-devel, coreteam,
	netdev, linux-kernel, paul, sds, eparis, jmorris, serge, selinux,
	linux-security-module
In-Reply-To: <20180923091611.19815-2-cgzones@googlemail.com>

[-- Attachment #1: Type: text/plain, Size: 3060 bytes --]

Hi Christian,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on nf-next/master]
[also build test ERROR on v4.19-rc4 next-20180921]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Christian-G-ttsche/netfilter-nf_tables-add-SECMARK-support/20180923-213820
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next.git master
config: x86_64-randconfig-s2-09240020 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   net/netfilter/nft_ct.c: In function 'nft_ct_set_eval':
>> net/netfilter/nft_ct.c:303:22: error: 'value' undeclared (first use in this function)
      if (ct->secmark != value) {
                         ^~~~~
   net/netfilter/nft_ct.c:303:22: note: each undeclared identifier is reported only once for each function it appears in

vim +/value +303 net/netfilter/nft_ct.c

   275	
   276	static void nft_ct_set_eval(const struct nft_expr *expr,
   277				    struct nft_regs *regs,
   278				    const struct nft_pktinfo *pkt)
   279	{
   280		const struct nft_ct *priv = nft_expr_priv(expr);
   281		struct sk_buff *skb = pkt->skb;
   282	#ifdef CONFIG_NF_CONNTRACK_MARK
   283		u32 value = regs->data[priv->sreg];
   284	#endif
   285		enum ip_conntrack_info ctinfo;
   286		struct nf_conn *ct;
   287	
   288		ct = nf_ct_get(skb, &ctinfo);
   289		if (ct == NULL || nf_ct_is_template(ct))
   290			return;
   291	
   292		switch (priv->key) {
   293	#ifdef CONFIG_NF_CONNTRACK_MARK
   294		case NFT_CT_MARK:
   295			if (ct->mark != value) {
   296				ct->mark = value;
   297				nf_conntrack_event_cache(IPCT_MARK, ct);
   298			}
   299			break;
   300	#endif
   301	#ifdef CONFIG_NF_CONNTRACK_SECMARK
   302		case NFT_CT_SECMARK:
 > 303			if (ct->secmark != value) {
   304				ct->secmark = value;
   305				nf_conntrack_event_cache(IPCT_SECMARK, ct);
   306			}
   307			break;
   308	#endif
   309	#ifdef CONFIG_NF_CONNTRACK_LABELS
   310		case NFT_CT_LABELS:
   311			nf_connlabels_replace(ct,
   312					      &regs->data[priv->sreg],
   313					      &regs->data[priv->sreg],
   314					      NF_CT_LABELS_MAX_SIZE / sizeof(u32));
   315			break;
   316	#endif
   317	#ifdef CONFIG_NF_CONNTRACK_EVENTS
   318		case NFT_CT_EVENTMASK: {
   319			struct nf_conntrack_ecache *e = nf_ct_ecache_find(ct);
   320			u32 ctmask = regs->data[priv->sreg];
   321	
   322			if (e) {
   323				if (e->ctmask != ctmask)
   324					e->ctmask = ctmask;
   325				break;
   326			}
   327	
   328			if (ctmask && !nf_ct_is_confirmed(ct))
   329				nf_ct_ecache_ext_add(ct, ctmask, 0, GFP_ATOMIC);
   330			break;
   331		}
   332	#endif
   333		default:
   334			break;
   335		}
   336	}
   337	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 33232 bytes --]

^ permalink raw reply

* [PATCH v3 2/2] netfilter: nf_tables: add requirements for connsecmark support
From: Christian Göttsche @ 2018-09-23 18:26 UTC (permalink / raw)
  To: pablo, kadlec, fw, davem, netfilter-devel, coreteam, netdev,
	linux-kernel, paul, sds, eparis, jmorris, serge, selinux,
	linux-security-module
In-Reply-To: <20180923182616.11398-1-cgzones@googlemail.com>

Add ability to set the connection tracking secmark value.

Add ability to set the meta secmark value.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---

v3: fix compile error when CONFIG_NF_CONNTRACK_MARK not defined

Based on nf-next
Tested with v4.18.8

 net/netfilter/nft_ct.c   | 17 ++++++++++++++++-
 net/netfilter/nft_meta.c |  8 ++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/nft_ct.c b/net/netfilter/nft_ct.c
index d74afa707..586627c36 100644
--- a/net/netfilter/nft_ct.c
+++ b/net/netfilter/nft_ct.c
@@ -279,7 +279,7 @@ static void nft_ct_set_eval(const struct nft_expr *expr,
 {
 	const struct nft_ct *priv = nft_expr_priv(expr);
 	struct sk_buff *skb = pkt->skb;
-#ifdef CONFIG_NF_CONNTRACK_MARK
+#if defined(CONFIG_NF_CONNTRACK_MARK) || defined(CONFIG_NF_CONNTRACK_SECMARK)
 	u32 value = regs->data[priv->sreg];
 #endif
 	enum ip_conntrack_info ctinfo;
@@ -298,6 +298,14 @@ static void nft_ct_set_eval(const struct nft_expr *expr,
 		}
 		break;
 #endif
+#ifdef CONFIG_NF_CONNTRACK_SECMARK
+	case NFT_CT_SECMARK:
+		if (ct->secmark != value) {
+			ct->secmark = value;
+			nf_conntrack_event_cache(IPCT_SECMARK, ct);
+		}
+		break;
+#endif
 #ifdef CONFIG_NF_CONNTRACK_LABELS
 	case NFT_CT_LABELS:
 		nf_connlabels_replace(ct,
@@ -564,6 +572,13 @@ static int nft_ct_set_init(const struct nft_ctx *ctx,
 			return -EINVAL;
 		len = sizeof(u32);
 		break;
+#endif
+#ifdef CONFIG_NF_CONNTRACK_SECMARK
+	case NFT_CT_SECMARK:
+		if (tb[NFTA_CT_DIRECTION])
+			return -EINVAL;
+		len = sizeof(u32);
+		break;
 #endif
 	default:
 		return -EOPNOTSUPP;
diff --git a/net/netfilter/nft_meta.c b/net/netfilter/nft_meta.c
index c8ac0ef4b..a6715c816 100644
--- a/net/netfilter/nft_meta.c
+++ b/net/netfilter/nft_meta.c
@@ -284,6 +284,11 @@ static void nft_meta_set_eval(const struct nft_expr *expr,
 
 		skb->nf_trace = !!value8;
 		break;
+#ifdef CONFIG_NETWORK_SECMARK
+	case NFT_META_SECMARK:
+		skb->secmark = value;
+		break;
+#endif
 	default:
 		WARN_ON(1);
 	}
@@ -436,6 +441,9 @@ static int nft_meta_set_init(const struct nft_ctx *ctx,
 	switch (priv->key) {
 	case NFT_META_MARK:
 	case NFT_META_PRIORITY:
+#ifdef CONFIG_NETWORK_SECMARK
+	case NFT_META_SECMARK:
+#endif
 		len = sizeof(u32);
 		break;
 	case NFT_META_NFTRACE:
-- 
2.19.0

^ permalink raw reply related

* Re: [PATCH] RDS: IB: Use DEFINE_PER_CPU_SHARED_ALIGNED for rds_ib_stats
From: David Miller @ 2018-09-23 19:19 UTC (permalink / raw)
  To: natechancellor
  Cc: santosh.shilimkar, netdev, linux-rdma, linux-kernel, ndesaulniers
In-Reply-To: <20180923064445.GC12338@flashbox>

From: Nathan Chancellor <natechancellor@gmail.com>
Date: Sat, 22 Sep 2018 23:44:45 -0700

> I have sent a v2 because this should be DECLARE_PER_CPU_SHARED_ALIGNED,
> not DEFINE (thanks to 0day for catching it).

Your first version was already applied to the networking GIT tree, so
you will need to submit a relative fixup with a proper "Fixes: " tag.

^ permalink raw reply

* Re: [PATCH] RDS: IB: Use DEFINE_PER_CPU_SHARED_ALIGNED for rds_ib_stats
From: David Miller @ 2018-09-23 19:26 UTC (permalink / raw)
  To: natechancellor
  Cc: santosh.shilimkar, netdev, linux-rdma, linux-kernel, ndesaulniers
In-Reply-To: <20180923.121927.436285462706036460.davem@davemloft.net>

From: David Miller <davem@davemloft.net>
Date: Sun, 23 Sep 2018 12:19:27 -0700 (PDT)

> From: Nathan Chancellor <natechancellor@gmail.com>
> Date: Sat, 22 Sep 2018 23:44:45 -0700
> 
>> I have sent a v2 because this should be DECLARE_PER_CPU_SHARED_ALIGNED,
>> not DEFINE (thanks to 0day for catching it).
> 
> Your first version was already applied to the networking GIT tree, so
> you will need to submit a relative fixup with a proper "Fixes: " tag.

Nevermind, I took care of it myself.

====================
>From 16fdf8ba98391650ce4bc4f3f71629d8a413bc21 Mon Sep 17 00:00:00 2001
From: "David S. Miller" <davem@davemloft.net>
Date: Sun, 23 Sep 2018 12:25:15 -0700
Subject: [PATCH] rds: Fix build regression.

Use DECLARE_* not DEFINE_*

Fixes: 8360ed6745df ("RDS: IB: Use DEFINE_PER_CPU_SHARED_ALIGNED for rds_ib_stats")
Signed-off-by: David S. Miller <davem@davemloft.net>
---
 net/rds/ib.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/rds/ib.h b/net/rds/ib.h
index fd483760c910..71ff356ee702 100644
--- a/net/rds/ib.h
+++ b/net/rds/ib.h
@@ -443,7 +443,7 @@ int rds_ib_send_grab_credits(struct rds_ib_connection *ic, u32 wanted,
 int rds_ib_xmit_atomic(struct rds_connection *conn, struct rm_atomic_op *op);
 
 /* ib_stats.c */
-DEFINE_PER_CPU_SHARED_ALIGNED(struct rds_ib_statistics, rds_ib_stats);
+DECLARE_PER_CPU_SHARED_ALIGNED(struct rds_ib_statistics, rds_ib_stats);
 #define rds_ib_stats_inc(member) rds_stats_inc_which(rds_ib_stats, member)
 #define rds_ib_stats_add(member, count) \
 		rds_stats_add_which(rds_ib_stats, member, count)
-- 
2.13.6

^ permalink raw reply related

* [PATCH net v2] net: phy: fix WoL handling when suspending the PHY
From: Heiner Kallweit @ 2018-09-23 13:33 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, David Miller,
	Realtek linux nic maintainers
  Cc: netdev@vger.kernel.org
In-Reply-To: <4dbcdd2c-96c2-2921-5016-affc8fce1d19@gmail.com>

Actually there's nothing wrong with the two changes marked as "Fixes",
they just revealed a problem which has been existing before.
After having switched r8169 to phylib it was reported that WoL from
shutdown doesn't work any longer (WoL from suspend isn't affected).
Reason is that during shutdown phy_disconnect()->phy_detach()->
phy_suspend() is called.
A similar issue occurs when the phylib state machine calls
phy_suspend() when handling state PHY_HALTED.

Core of the problem is that phy_suspend() suspends the PHY when it
should not due to WoL. phy_suspend() checks for WoL already, but this
works only if the PHY driver handles WoL (what is rarely the case).
Typically WoL is handled by the MAC driver.

phylib knows about this and handles it in mdio_bus_phy_may_suspend(),
but that's used only when suspending the system, not in other cases
like shutdown.

Therefore factor out the relevant check from
mdio_bus_phy_may_suspend() to a new function phy_may_suspend() and
use it in phy_suspend().

Last but not least change phy_detach() to call phy_suspend() before
attached_dev is set to NULL. phy_suspend() accesses attached_dev
when checking whether the MAC driver activated WoL.

Fixes: f1e911d5d0df ("r8169: add basic phylib support")
Fixes: e8cfd9d6c772 ("net: phy: call state machine synchronously in phy_stop")
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
v2:
- improved commit message
- reduced scope of patch, don't touch functionality of
  mdio_bus_phy_suspend and mdio_bus_phy_resume
---
 drivers/net/phy/phy_device.c | 42 ++++++++++++++++++++++--------------
 1 file changed, 26 insertions(+), 16 deletions(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index af64a9320..4cab94bae 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -75,6 +75,26 @@ extern struct phy_driver genphy_10g_driver;
 static LIST_HEAD(phy_fixup_list);
 static DEFINE_MUTEX(phy_fixup_lock);
 
+static bool phy_may_suspend(struct phy_device *phydev)
+{
+	struct net_device *netdev = phydev->attached_dev;
+
+	if (!netdev)
+		return true;
+
+	/* Don't suspend PHY if the attached netdev parent may wakeup.
+	 * The parent may point to a PCI device, as in tg3 driver.
+	 */
+	if (netdev->dev.parent && device_may_wakeup(netdev->dev.parent))
+		return false;
+
+	/* Also don't suspend PHY if the netdev itself may wakeup. This
+	 * is the case for devices w/o underlaying pwr. mgmt. aware bus,
+	 * e.g. SoC devices.
+	 */
+	return !device_may_wakeup(&netdev->dev);
+}
+
 #ifdef CONFIG_PM
 static bool mdio_bus_phy_may_suspend(struct phy_device *phydev)
 {
@@ -93,20 +113,7 @@ static bool mdio_bus_phy_may_suspend(struct phy_device *phydev)
 	if (!netdev)
 		return !phydev->suspended;
 
-	/* Don't suspend PHY if the attached netdev parent may wakeup.
-	 * The parent may point to a PCI device, as in tg3 driver.
-	 */
-	if (netdev->dev.parent && device_may_wakeup(netdev->dev.parent))
-		return false;
-
-	/* Also don't suspend PHY if the netdev itself may wakeup. This
-	 * is the case for devices w/o underlaying pwr. mgmt. aware bus,
-	 * e.g. SoC devices.
-	 */
-	if (device_may_wakeup(&netdev->dev))
-		return false;
-
-	return true;
+	return phy_may_suspend(phydev);
 }
 
 static int mdio_bus_phy_suspend(struct device *dev)
@@ -1132,9 +1139,9 @@ void phy_detach(struct phy_device *phydev)
 		sysfs_remove_link(&dev->dev.kobj, "phydev");
 		sysfs_remove_link(&phydev->mdio.dev.kobj, "attached_dev");
 	}
+	phy_suspend(phydev);
 	phydev->attached_dev->phydev = NULL;
 	phydev->attached_dev = NULL;
-	phy_suspend(phydev);
 	phydev->phylink = NULL;
 
 	phy_led_triggers_unregister(phydev);
@@ -1171,9 +1178,12 @@ int phy_suspend(struct phy_device *phydev)
 	struct ethtool_wolinfo wol = { .cmd = ETHTOOL_GWOL };
 	int ret = 0;
 
+	if (phydev->suspended)
+		return 0;
+
 	/* If the device has WOL enabled, we cannot suspend the PHY */
 	phy_ethtool_get_wol(phydev, &wol);
-	if (wol.wolopts)
+	if (wol.wolopts || !phy_may_suspend(phydev))
 		return -EBUSY;
 
 	if (phydev->drv && phydrv->suspend)
-- 
2.19.0

^ permalink raw reply related

* [PATCH net v2] net: phy: fix WoL handling when suspending the PHY
From: Heiner Kallweit @ 2018-09-23 13:38 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, David Miller,
	Realtek linux nic maintainers
  Cc: netdev@vger.kernel.org

Actually there's nothing wrong with the two changes marked as "Fixes",
they just revealed a problem which has been existing before.
After having switched r8169 to phylib it was reported that WoL from
shutdown doesn't work any longer (WoL from suspend isn't affected).
Reason is that during shutdown phy_disconnect()->phy_detach()->
phy_suspend() is called.
A similar issue occurs when the phylib state machine calls
phy_suspend() when handling state PHY_HALTED.

Core of the problem is that phy_suspend() suspends the PHY when it
should not due to WoL. phy_suspend() checks for WoL already, but this
works only if the PHY driver handles WoL (what is rarely the case).
Typically WoL is handled by the MAC driver.

phylib knows about this and handles it in mdio_bus_phy_may_suspend(),
but that's used only when suspending the system, not in other cases
like shutdown.

Therefore factor out the relevant check from
mdio_bus_phy_may_suspend() to a new function phy_may_suspend() and
use it in phy_suspend().

Last but not least change phy_detach() to call phy_suspend() before
attached_dev is set to NULL. phy_suspend() accesses attached_dev
when checking whether the MAC driver activated WoL.

Fixes: f1e911d5d0df ("r8169: add basic phylib support")
Fixes: e8cfd9d6c772 ("net: phy: call state machine synchronously in phy_stop")
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
v2:
- improved commit message
- reduced scope of patch, don't touch functionality of
  mdio_bus_phy_suspend and mdio_bus_phy_resume
---
 drivers/net/phy/phy_device.c | 42 ++++++++++++++++++++++--------------
 1 file changed, 26 insertions(+), 16 deletions(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index af64a9320..4cab94bae 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -75,6 +75,26 @@ extern struct phy_driver genphy_10g_driver;
 static LIST_HEAD(phy_fixup_list);
 static DEFINE_MUTEX(phy_fixup_lock);
 
+static bool phy_may_suspend(struct phy_device *phydev)
+{
+	struct net_device *netdev = phydev->attached_dev;
+
+	if (!netdev)
+		return true;
+
+	/* Don't suspend PHY if the attached netdev parent may wakeup.
+	 * The parent may point to a PCI device, as in tg3 driver.
+	 */
+	if (netdev->dev.parent && device_may_wakeup(netdev->dev.parent))
+		return false;
+
+	/* Also don't suspend PHY if the netdev itself may wakeup. This
+	 * is the case for devices w/o underlaying pwr. mgmt. aware bus,
+	 * e.g. SoC devices.
+	 */
+	return !device_may_wakeup(&netdev->dev);
+}
+
 #ifdef CONFIG_PM
 static bool mdio_bus_phy_may_suspend(struct phy_device *phydev)
 {
@@ -93,20 +113,7 @@ static bool mdio_bus_phy_may_suspend(struct phy_device *phydev)
 	if (!netdev)
 		return !phydev->suspended;
 
-	/* Don't suspend PHY if the attached netdev parent may wakeup.
-	 * The parent may point to a PCI device, as in tg3 driver.
-	 */
-	if (netdev->dev.parent && device_may_wakeup(netdev->dev.parent))
-		return false;
-
-	/* Also don't suspend PHY if the netdev itself may wakeup. This
-	 * is the case for devices w/o underlaying pwr. mgmt. aware bus,
-	 * e.g. SoC devices.
-	 */
-	if (device_may_wakeup(&netdev->dev))
-		return false;
-
-	return true;
+	return phy_may_suspend(phydev);
 }
 
 static int mdio_bus_phy_suspend(struct device *dev)
@@ -1132,9 +1139,9 @@ void phy_detach(struct phy_device *phydev)
 		sysfs_remove_link(&dev->dev.kobj, "phydev");
 		sysfs_remove_link(&phydev->mdio.dev.kobj, "attached_dev");
 	}
+	phy_suspend(phydev);
 	phydev->attached_dev->phydev = NULL;
 	phydev->attached_dev = NULL;
-	phy_suspend(phydev);
 	phydev->phylink = NULL;
 
 	phy_led_triggers_unregister(phydev);
@@ -1171,9 +1178,12 @@ int phy_suspend(struct phy_device *phydev)
 	struct ethtool_wolinfo wol = { .cmd = ETHTOOL_GWOL };
 	int ret = 0;
 
+	if (phydev->suspended)
+		return 0;
+
 	/* If the device has WOL enabled, we cannot suspend the PHY */
 	phy_ethtool_get_wol(phydev, &wol);
-	if (wol.wolopts)
+	if (wol.wolopts || !phy_may_suspend(phydev))
 		return -EBUSY;
 
 	if (phydev->drv && phydrv->suspend)
-- 
2.19.0

^ permalink raw reply related

* Re: [PATCH net v2] net: phy: fix WoL handling when suspending the PHY
From: Heiner Kallweit @ 2018-09-23 13:39 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, David Miller,
	Realtek linux nic maintainers
  Cc: netdev@vger.kernel.org
In-Reply-To: <f8737cef-88d2-0f60-bc96-88251e77c1bd@gmail.com>

Sorry, this patch was accidentally sent as reply.

On 23.09.2018 15:33, Heiner Kallweit wrote:
> Actually there's nothing wrong with the two changes marked as "Fixes",
> they just revealed a problem which has been existing before.
> After having switched r8169 to phylib it was reported that WoL from
> shutdown doesn't work any longer (WoL from suspend isn't affected).
> Reason is that during shutdown phy_disconnect()->phy_detach()->
> phy_suspend() is called.
> A similar issue occurs when the phylib state machine calls
> phy_suspend() when handling state PHY_HALTED.
> 
> Core of the problem is that phy_suspend() suspends the PHY when it
> should not due to WoL. phy_suspend() checks for WoL already, but this
> works only if the PHY driver handles WoL (what is rarely the case).
> Typically WoL is handled by the MAC driver.
> 
> phylib knows about this and handles it in mdio_bus_phy_may_suspend(),
> but that's used only when suspending the system, not in other cases
> like shutdown.
> 
> Therefore factor out the relevant check from
> mdio_bus_phy_may_suspend() to a new function phy_may_suspend() and
> use it in phy_suspend().
> 
> Last but not least change phy_detach() to call phy_suspend() before
> attached_dev is set to NULL. phy_suspend() accesses attached_dev
> when checking whether the MAC driver activated WoL.
> 
> Fixes: f1e911d5d0df ("r8169: add basic phylib support")
> Fixes: e8cfd9d6c772 ("net: phy: call state machine synchronously in phy_stop")
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
> v2:
> - improved commit message
> - reduced scope of patch, don't touch functionality of
>   mdio_bus_phy_suspend and mdio_bus_phy_resume
> ---
>  drivers/net/phy/phy_device.c | 42 ++++++++++++++++++++++--------------
>  1 file changed, 26 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index af64a9320..4cab94bae 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -75,6 +75,26 @@ extern struct phy_driver genphy_10g_driver;
>  static LIST_HEAD(phy_fixup_list);
>  static DEFINE_MUTEX(phy_fixup_lock);
>  
> +static bool phy_may_suspend(struct phy_device *phydev)
> +{
> +	struct net_device *netdev = phydev->attached_dev;
> +
> +	if (!netdev)
> +		return true;
> +
> +	/* Don't suspend PHY if the attached netdev parent may wakeup.
> +	 * The parent may point to a PCI device, as in tg3 driver.
> +	 */
> +	if (netdev->dev.parent && device_may_wakeup(netdev->dev.parent))
> +		return false;
> +
> +	/* Also don't suspend PHY if the netdev itself may wakeup. This
> +	 * is the case for devices w/o underlaying pwr. mgmt. aware bus,
> +	 * e.g. SoC devices.
> +	 */
> +	return !device_may_wakeup(&netdev->dev);
> +}
> +
>  #ifdef CONFIG_PM
>  static bool mdio_bus_phy_may_suspend(struct phy_device *phydev)
>  {
> @@ -93,20 +113,7 @@ static bool mdio_bus_phy_may_suspend(struct phy_device *phydev)
>  	if (!netdev)
>  		return !phydev->suspended;
>  
> -	/* Don't suspend PHY if the attached netdev parent may wakeup.
> -	 * The parent may point to a PCI device, as in tg3 driver.
> -	 */
> -	if (netdev->dev.parent && device_may_wakeup(netdev->dev.parent))
> -		return false;
> -
> -	/* Also don't suspend PHY if the netdev itself may wakeup. This
> -	 * is the case for devices w/o underlaying pwr. mgmt. aware bus,
> -	 * e.g. SoC devices.
> -	 */
> -	if (device_may_wakeup(&netdev->dev))
> -		return false;
> -
> -	return true;
> +	return phy_may_suspend(phydev);
>  }
>  
>  static int mdio_bus_phy_suspend(struct device *dev)
> @@ -1132,9 +1139,9 @@ void phy_detach(struct phy_device *phydev)
>  		sysfs_remove_link(&dev->dev.kobj, "phydev");
>  		sysfs_remove_link(&phydev->mdio.dev.kobj, "attached_dev");
>  	}
> +	phy_suspend(phydev);
>  	phydev->attached_dev->phydev = NULL;
>  	phydev->attached_dev = NULL;
> -	phy_suspend(phydev);
>  	phydev->phylink = NULL;
>  
>  	phy_led_triggers_unregister(phydev);
> @@ -1171,9 +1178,12 @@ int phy_suspend(struct phy_device *phydev)
>  	struct ethtool_wolinfo wol = { .cmd = ETHTOOL_GWOL };
>  	int ret = 0;
>  
> +	if (phydev->suspended)
> +		return 0;
> +
>  	/* If the device has WOL enabled, we cannot suspend the PHY */
>  	phy_ethtool_get_wol(phydev, &wol);
> -	if (wol.wolopts)
> +	if (wol.wolopts || !phy_may_suspend(phydev))
>  		return -EBUSY;
>  
>  	if (phydev->drv && phydrv->suspend)
> 

^ permalink raw reply

* Re: [PATCH] RDS: IB: Use DEFINE_PER_CPU_SHARED_ALIGNED for rds_ib_stats
From: Nathan Chancellor @ 2018-09-23 19:38 UTC (permalink / raw)
  To: David Miller
  Cc: santosh.shilimkar, netdev, linux-rdma, linux-kernel, ndesaulniers
In-Reply-To: <20180923.121927.436285462706036460.davem@davemloft.net>

On Sun, Sep 23, 2018 at 12:19:27PM -0700, David Miller wrote:
> From: Nathan Chancellor <natechancellor@gmail.com>
> Date: Sat, 22 Sep 2018 23:44:45 -0700
> 
> > I have sent a v2 because this should be DECLARE_PER_CPU_SHARED_ALIGNED,
> > not DEFINE (thanks to 0day for catching it).
> 
> Your first version was already applied to the networking GIT tree, so
> you will need to submit a relative fixup with a proper "Fixes: " tag.

Hi David,

My apologies, I didn't receive an email saying my patch was applied so I
didn't think to check your tree. I'll be better about this in the
future.

Thank you for fixing this up yourself and all your patience, sorry for
all the noise!

Nathan

^ permalink raw reply

* Re: [PATCH net-next v9 0/6] net: vhost: improve performance when enable busyloop
From: Tonghao Zhang @ 2018-09-23 13:48 UTC (permalink / raw)
  To: jasowang, mst, makita.toshiaki
  Cc: virtualization, Linux Kernel Network Developers
In-Reply-To: <1536493887-2637-1-git-send-email-xiangxia.m.yue@gmail.com>

On Sun, Sep 9, 2018 at 7:52 PM <xiangxia.m.yue@gmail.com> wrote:
>
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>
> This patches improve the guest receive performance.
> On the handle_tx side, we poll the sock receive queue
> at the same time. handle_rx do that in the same way.
>
> For more performance report, see patch 4, 5, 6
>
> Tonghao Zhang (6):
>   net: vhost: lock the vqs one by one
>   net: vhost: replace magic number of lock annotation
>   net: vhost: factor out busy polling logic to vhost_net_busy_poll()
>   net: vhost: add rx busy polling in tx path
>   net: vhost: disable rx wakeup during tx busypoll
>   net: vhost: make busyloop_intr more accurate
>
>  drivers/vhost/net.c   | 163 +++++++++++++++++++++++++++++++-------------------
>  drivers/vhost/vhost.c |  24 +++-----
>  2 files changed, 108 insertions(+), 79 deletions(-)
>
> --
> 1.8.3.1
>
ping ?

^ permalink raw reply

* [PATCH net-next] tcp: expose sk_state in tcp_retransmit_skb tracepoint
From: Yafang Shao @ 2018-09-23 19:49 UTC (permalink / raw)
  To: edumazet, davem; +Cc: netdev, linux-kernel, Yafang Shao

With sk_state, we can know whether this connection is in SYN_SENT state
or ESTBLISHED state.
The reason to distinguish between these two scenario is that the
retransmission in ESTABLISHED state always mean network congestion while
in SYN_SENT state it always mean server issue, i.e. the syn packet is
dropped due to syn backlog queue full.

PS: SYNACK retransmission is traced in tcp_retransmit_synack tracepoint.

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 include/trace/events/tcp.h | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/include/trace/events/tcp.h b/include/trace/events/tcp.h
index ac55b32..2bc9960 100644
--- a/include/trace/events/tcp.h
+++ b/include/trace/events/tcp.h
@@ -56,6 +56,7 @@
 	TP_STRUCT__entry(
 		__field(const void *, skbaddr)
 		__field(const void *, skaddr)
+		__field(int, state)
 		__field(__u16, sport)
 		__field(__u16, dport)
 		__array(__u8, saddr, 4)
@@ -70,6 +71,7 @@
 
 		__entry->skbaddr = skb;
 		__entry->skaddr = sk;
+		__entry->state = sk->sk_state;
 
 		__entry->sport = ntohs(inet->inet_sport);
 		__entry->dport = ntohs(inet->inet_dport);
@@ -84,9 +86,10 @@
 			      sk->sk_v6_rcv_saddr, sk->sk_v6_daddr);
 	),
 
-	TP_printk("sport=%hu dport=%hu saddr=%pI4 daddr=%pI4 saddrv6=%pI6c daddrv6=%pI6c",
+	TP_printk("sport=%hu dport=%hu saddr=%pI4 daddr=%pI4 saddrv6=%pI6c daddrv6=%pI6c state=%s\n",
 		  __entry->sport, __entry->dport, __entry->saddr, __entry->daddr,
-		  __entry->saddr_v6, __entry->daddr_v6)
+		  __entry->saddr_v6, __entry->daddr_v6,
+		  show_tcp_state_name(__entry->state))
 );
 
 DEFINE_EVENT(tcp_event_sk_skb, tcp_retransmit_skb,
-- 
1.8.3.1

^ permalink raw reply related

* Re: [PATCH RFT net-next 0/2] net: phy: Eliminate unnecessary soft
From: Clemens Gruber @ 2018-09-23 14:25 UTC (permalink / raw)
  To: netdev; +Cc: Florian Fainelli, David S. Miller, Andrew Lunn
In-Reply-To: <20180919013505.11347-1-f.fainelli@gmail.com>

Hi,

On Tue, Sep 18, 2018 at 06:35:03PM -0700, Florian Fainelli wrote:
> Hi all,
> 
> This patch series eliminates unnecessary software resets of the PHY.
> This should hopefully not break anybody's hardware; but I would
> appreciate testing to make sure this is is the case.
> 
> Sorry for this long email list, I wanted to make sure I reached out to
> all people who made changes to the Marvell PHY driver.
> 
> Thank you!
> 
> Florian Fainelli (2):
>   net: phy: Stop with excessive soft reset
>   net: phy: marvell: Avoid unnecessary soft reset
> 
>  drivers/net/phy/marvell.c    | 63 ++++++++++++------------------------
>  drivers/net/phy/phy_device.c |  2 --
>  2 files changed, 21 insertions(+), 44 deletions(-)
> 
> -- 
> 2.17.1
>

I tested your patches on our board with a Marvell 88E1510. Looks good!

Tested-by: Clemens Gruber <clemens.gruber@pqgruber.com>

Clemens

^ permalink raw reply

* [PATCH net-next] mlxsw: Make MLXSW_SP1_FWREV_MINOR a hard requirement
From: Ido Schimmel @ 2018-09-23 14:48 UTC (permalink / raw)
  To: netdev; +Cc: davem, jiri, petrm, mlxsw, Ido Schimmel

From: Petr Machata <petrm@mellanox.com>

Up until now, mlxsw tolerated firmware versions that weren't exactly
matching the required version, if the branch number matched. That
allowed the users to test various firmware versions as long as they were
on the right branch.

On the other hand, it made it impossible for mlxsw to put a hard lower
bound on a version that fixes all problems known to date. If a user had
a somewhat older FW version installed, mlxsw would start up just fine,
possibly performing non-optimally as it would use features that trigger
problematic behavior.

Therefore tweak the check to accept any FW version that is:

- on the same branch as the preferred version, and
- the same as or newer than the preferred version.

Signed-off-by: Petr Machata <petrm@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 1fc20263b15b..33a024274a1f 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -331,7 +331,10 @@ static int mlxsw_sp_fw_rev_validate(struct mlxsw_sp *mlxsw_sp)
 		return -EINVAL;
 	}
 	if (MLXSW_SP_FWREV_MINOR_TO_BRANCH(rev->minor) ==
-	    MLXSW_SP_FWREV_MINOR_TO_BRANCH(req_rev->minor))
+	    MLXSW_SP_FWREV_MINOR_TO_BRANCH(req_rev->minor) &&
+	    (rev->minor > req_rev->minor ||
+	     (rev->minor == req_rev->minor &&
+	      rev->subminor >= req_rev->subminor)))
 		return 0;
 
 	dev_info(mlxsw_sp->bus_info->dev, "The firmware version %d.%d.%d is incompatible with the driver\n",
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCH net-next] tcp: expose sk_state in tcp_retransmit_skb tracepoint
From: Eric Dumazet @ 2018-09-23 21:42 UTC (permalink / raw)
  To: Yafang Shao, edumazet, davem; +Cc: netdev, linux-kernel
In-Reply-To: <1537732141-22468-1-git-send-email-laoar.shao@gmail.com>



On 09/23/2018 12:49 PM, Yafang Shao wrote:
> With sk_state, we can know whether this connection is in SYN_SENT state
> or ESTBLISHED state.
> The reason to distinguish between these two scenario is that the
> retransmission in ESTABLISHED state always mean network congestion while
> in SYN_SENT state it always mean server issue, i.e. the syn packet is
> dropped due to syn backlog queue full.

You mean, a packet drop on the remote peer ?

It could also be a packet drop in the network.

Your patch is good, but changelog is quite misleading.

^ permalink raw reply

* Re: [RFC 2/3 net] lorawan: Add macro and definition for LoRaWAN class modlue
From: Andreas Färber @ 2018-09-23 16:06 UTC (permalink / raw)
  To: Jian-Hong Pan
  Cc: netdev, linux-arm-kernel, linux-kernel, Jiri Pirko,
	Marcel Holtmann, David S. Miller, Matthias Brugger, Janus Piwek,
	Michael Röder, Dollar Chen, Ken Yu, Konstantin Böhm,
	Jan Jongboom, Jon Ortego, contact@snootlab.com, Ben Whitten,
	Brian Ray, lora, Alexander Graf, Michal Kubeček
In-Reply-To: <f7ea222671a2bc6fe0a060c4bdb1ebae162d0d91.1535039998.git.starnight@g.ncu.edu.tw>

Hi Jian-Hong,

Many thanks and sorry for the delay. This patch mostly looks good and
should go in before its first uses, so I would like to queue it soon for
my hardware-MAC module drivers - but some questions below. Also a typo
in the subject, and we should probably prepend "net: " and I would
personally not mention the module here as it's a userspace API.

Am 23.08.18 um 19:15 schrieb Jian-Hong Pan:
> This patch add the macro and definition for the implementation of
> LoRaWAN protocol.

I would fix up the grammar nits in my tree then.

> 
> Signed-off-by: Jian-Hong Pan <starnight@g.ncu.edu.tw>
> ---
>  include/linux/socket.h              | 5 ++++-
>  include/uapi/linux/if_arp.h         | 1 +
>  include/uapi/linux/if_ether.h       | 1 +
>  net/core/dev.c                      | 4 ++--
>  security/selinux/hooks.c            | 4 +++-
>  security/selinux/include/classmap.h | 4 +++-
>  6 files changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/socket.h b/include/linux/socket.h
> index aa1e288b1659..e5c8381fd1aa 100644
> --- a/include/linux/socket.h
> +++ b/include/linux/socket.h
> @@ -209,8 +209,9 @@ struct ucred {
>  				 */
>  #define AF_XDP		44	/* XDP sockets			*/
>  #define AF_LORA		45	/* LoRa sockets			*/
> +#define AF_LORAWAN	46	/* LoRaWAN sockets			*/
>  
> -#define AF_MAX		46	/* For now.. */
> +#define AF_MAX		47	/* For now.. */
>  
>  /* Protocol families, same as address families. */
>  #define PF_UNSPEC	AF_UNSPEC
> @@ -261,6 +262,7 @@ struct ucred {
>  #define PF_SMC		AF_SMC
>  #define PF_XDP		AF_XDP
>  #define PF_LORA		AF_LORA
> +#define PF_LORAWAN	AF_LORAWAN
>  #define PF_MAX		AF_MAX
>  
>  /* Maximum queue length specifiable by listen.  */
> @@ -343,6 +345,7 @@ struct ucred {
>  #define SOL_KCM		281
>  #define SOL_TLS		282
>  #define SOL_XDP		283
> +#define SOL_LORAWAN	284
>  
>  /* IPX options */
>  #define IPX_TYPE	1
> diff --git a/include/uapi/linux/if_arp.h b/include/uapi/linux/if_arp.h
> index 1ed7cb3f2129..2376f7839355 100644
> --- a/include/uapi/linux/if_arp.h
> +++ b/include/uapi/linux/if_arp.h
> @@ -99,6 +99,7 @@
>  #define ARPHRD_6LOWPAN	825		/* IPv6 over LoWPAN             */
>  #define ARPHRD_VSOCKMON	826		/* Vsock monitor header		*/
>  #define ARPHRD_LORA	827		/* LoRa				*/
> +#define ARPHRD_LORAWAN	828		/* LoRaWAN			*/
>  
>  #define ARPHRD_VOID	  0xFFFF	/* Void type, nothing is known */
>  #define ARPHRD_NONE	  0xFFFE	/* zero header length */
> diff --git a/include/uapi/linux/if_ether.h b/include/uapi/linux/if_ether.h
> index 45644dcf5b39..b1ac70d4a377 100644
> --- a/include/uapi/linux/if_ether.h
> +++ b/include/uapi/linux/if_ether.h
> @@ -148,6 +148,7 @@
>  					 * aggregation protocol
>  					 */
>  #define ETH_P_LORA	0x00FA		/* LoRa				*/
> +#define ETH_P_LORAWAN	0x00FB		/* LoRaWAN			*/
>  
>  /*
>   *	This is an Ethernet frame header.
> diff --git a/net/core/dev.c b/net/core/dev.c
> index f68122f0ab02..b95ce79ec5a8 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -297,7 +297,7 @@ static const unsigned short netdev_lock_type[] = {
>  	 ARPHRD_IRDA, ARPHRD_FCPP, ARPHRD_FCAL, ARPHRD_FCPL,
>  	 ARPHRD_FCFABRIC, ARPHRD_IEEE80211, ARPHRD_IEEE80211_PRISM,
>  	 ARPHRD_IEEE80211_RADIOTAP, ARPHRD_PHONET, ARPHRD_PHONET_PIPE,
> -	 ARPHRD_IEEE802154, ARPHRD_VOID, ARPHRD_NONE};
> +	 ARPHRD_IEEE802154, ARPHRD_LORAWAN, ARPHRD_VOID, ARPHRD_NONE};
>  
>  static const char *const netdev_lock_name[] = {
>  	"_xmit_NETROM", "_xmit_ETHER", "_xmit_EETHER", "_xmit_AX25",
> @@ -314,7 +314,7 @@ static const char *const netdev_lock_name[] = {
>  	"_xmit_IRDA", "_xmit_FCPP", "_xmit_FCAL", "_xmit_FCPL",
>  	"_xmit_FCFABRIC", "_xmit_IEEE80211", "_xmit_IEEE80211_PRISM",
>  	"_xmit_IEEE80211_RADIOTAP", "_xmit_PHONET", "_xmit_PHONET_PIPE",
> -	"_xmit_IEEE802154", "_xmit_VOID", "_xmit_NONE"};
> +	"_xmit_IEEE802154", "_xmit_LORAWAN", "_xmit_VOID", "_xmit_NONE"};
>  
>  static struct lock_class_key netdev_xmit_lock_key[ARRAY_SIZE(netdev_lock_type)];
>  static struct lock_class_key netdev_addr_lock_key[ARRAY_SIZE(netdev_lock_type)];

All your new constants except SOL_LORAWAN are always next to a LoRa one,
but not in these two arrays. I don't have such changes in my original
LoRa patch - am I missing such an array extension for ARPHRD_LORA then,
or what is the criteria for when to add this?

Other changes look fine to me.

Regards,
Andreas

> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index aaf520a689d8..0da3a1d69cb8 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -1477,7 +1477,9 @@ static inline u16 socket_type_to_security_class(int family, int type, int protoc
>  			return SECCLASS_XDP_SOCKET;
>  		case PF_LORA:
>  			return SECCLASS_LORA_SOCKET;
> -#if PF_MAX > 46
> +		case PF_LORAWAN:
> +			return SECCLASS_LORAWAN_SOCKET;
> +#if PF_MAX > 47
>  #error New address family defined, please update this function.
>  #endif
>  		}
> diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
> index 060d4bf8385e..fa0151fe6f32 100644
> --- a/security/selinux/include/classmap.h
> +++ b/security/selinux/include/classmap.h
> @@ -244,9 +244,11 @@ struct security_class_mapping secclass_map[] = {
>  	  { COMMON_SOCK_PERMS, NULL } },
>  	{ "lora_socket",
>  	  { COMMON_SOCK_PERMS, NULL } },
> +	{ "lorawan_socket",
> +	  { COMMON_SOCK_PERMS, NULL } },
>  	{ NULL }
>    };
>  
> -#if PF_MAX > 46
> +#if PF_MAX > 47
>  #error New address family defined, please update secclass_map.
>  #endif
> 


-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

^ permalink raw reply

* Re: KMSAN: uninit-value in memcmp (2)
From: Dmitry Vyukov @ 2018-09-23 16:22 UTC (permalink / raw)
  To: Vladis Dronov
  Cc: syzbot, syzkaller-bugs, David Miller, Eric Dumazet, LKML, netdev,
	sunlianwen
In-Reply-To: <125732064.15444205.1537718529926.JavaMail.zimbra@redhat.com>

On Sun, Sep 23, 2018 at 6:02 PM, Vladis Dronov <vdronov@redhat.com> wrote:
> #syz fix: net: fix uninit-value in __hw_addr_add_ex()

Hi Vladis,

This can be fixed with "net: fix uninit-value in __hw_addr_add_ex()".
That commit landed in April, syzbot waited till the commit reached all
tested trees, and then closed the bug.
But the similar bug continued to happen, so syzbot created second
version of this bug (2). You can see on dashboard that the last crash
for the second version (2) happened just few days ago. So this is a
different bug.

^ permalink raw reply

* Re: [RFC 1/3 net] lorawan: Add LoRaWAN class module
From: Andreas Färber @ 2018-09-23 16:40 UTC (permalink / raw)
  To: Jian-Hong Pan
  Cc: netdev, linux-arm-kernel, linux-kernel, Jiri Pirko,
	Marcel Holtmann, David S. Miller, Matthias Brugger, Janus Piwek,
	Michael Röder, Dollar Chen, Ken Yu, Konstantin Böhm,
	Jan Jongboom, Jon Ortego, contact@snootlab.com, Ben Whitten,
	Brian Ray, lora, Alexander Graf, Michal Kubeček
In-Reply-To: <fc737f3940bbe91341fb15d85ac11931eb56d1fc.1535039998.git.starnight@g.ncu.edu.tw>

Hi Jian-Hong,

[+ Afonso]

Am 23.08.18 um 19:15 schrieb Jian-Hong Pan:
> LoRaWAN defined by LoRa Alliance(TM) is the MAC layer over LoRa devices.
> 
> This patch implements part of Class A end-devices features defined in
> LoRaWAN(TM) Specification Ver. 1.0.2:
> 1. End-device receive slot timing
> 2. Only single channel and single data rate for now
> 3. Unconfirmed data up/down message types
> 4. Encryption/decryption for up/down link data messages
> 
> It also implements the the functions and maps to Datagram socket for
> LoRaWAN unconfirmed data messages.
> 
> On the other side, it defines the basic interface and operation
> functions for compatible LoRa device drivers.
> 
> Signed-off-by: Jian-Hong Pan <starnight@g.ncu.edu.tw>
> ---
>  include/linux/maclorawan/lora.h | 239 +++++++++++
>  net/maclorawan/Kconfig          |  14 +
>  net/maclorawan/Makefile         |   2 +
>  net/maclorawan/lorawan.h        | 219 ++++++++++
>  net/maclorawan/lrwsec.c         | 237 +++++++++++
>  net/maclorawan/lrwsec.h         |  57 +++
>  net/maclorawan/mac.c            | 552 +++++++++++++++++++++++++
>  net/maclorawan/main.c           | 665 ++++++++++++++++++++++++++++++
>  net/maclorawan/socket.c         | 700 ++++++++++++++++++++++++++++++++
>  9 files changed, 2685 insertions(+)
>  create mode 100644 include/linux/maclorawan/lora.h

Can we use include/linux/lora/lorawan.h for simplicity?

>  create mode 100644 net/maclorawan/Kconfig
>  create mode 100644 net/maclorawan/Makefile
>  create mode 100644 net/maclorawan/lorawan.h
>  create mode 100644 net/maclorawan/lrwsec.c
>  create mode 100644 net/maclorawan/lrwsec.h
>  create mode 100644 net/maclorawan/mac.c
>  create mode 100644 net/maclorawan/main.c
>  create mode 100644 net/maclorawan/socket.c

This patch is much too large for me to review...

It also doesn't seem to follow the structure I suggested: 802.15.4 has
two separate directories, net/ieee802154/ and net/mac802154/. Therefore
I had suggested net/maclorawan/ only for code that translates from
ETH_P_LORAWAN to ETH_P_LORA socket buffers, i.e. your soft MAC. Generic
socket code that applies also to hard MAC drivers I expected to go
either to net/lora/lorawan/ or better net/lorawan/ or some other clearly
separate location, with its own Kconfig symbol - any reason not to?
Consider which parts can be enabled/used independently of each other,
then you can put them into two (or more?) different patches.

Also please use SPDX license identifiers in your headers.
And please don't put the whole world in To, use CC.

Thanks,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

^ permalink raw reply

* Re: [PATCH net v2] net: phy: fix WoL handling when suspending the PHY
From: Florian Fainelli @ 2018-09-23 16:49 UTC (permalink / raw)
  To: Heiner Kallweit, Andrew Lunn, David Miller,
	Realtek linux nic maintainers
  Cc: netdev@vger.kernel.org
In-Reply-To: <faf1163d-7ba8-8cd9-f84c-3b924f3a6a16@gmail.com>



On September 23, 2018 6:38:21 AM PDT, Heiner Kallweit <hkallweit1@gmail.com> wrote:
>Actually there's nothing wrong with the two changes marked as "Fixes",
>they just revealed a problem which has been existing before.
>After having switched r8169 to phylib it was reported that WoL from
>shutdown doesn't work any longer (WoL from suspend isn't affected).
>Reason is that during shutdown phy_disconnect()->phy_detach()->
>phy_suspend() is called.
>A similar issue occurs when the phylib state machine calls
>phy_suspend() when handling state PHY_HALTED.
>
>Core of the problem is that phy_suspend() suspends the PHY when it
>should not due to WoL. phy_suspend() checks for WoL already, but this
>works only if the PHY driver handles WoL (what is rarely the case).
>Typically WoL is handled by the MAC driver.
>
>phylib knows about this and handles it in mdio_bus_phy_may_suspend(),
>but that's used only when suspending the system, not in other cases
>like shutdown.
>
>Therefore factor out the relevant check from
>mdio_bus_phy_may_suspend() to a new function phy_may_suspend() and
>use it in phy_suspend().
>
>Last but not least change phy_detach() to call phy_suspend() before
>attached_dev is set to NULL. phy_suspend() accesses attached_dev
>when checking whether the MAC driver activated WoL.

The rationale for the change makes sense but I am worried about a couple of things:

- we have seen drivers fail to call SET_NETDEV_DEV() correctly, or sometimes it is made possible because they are Device Tree only objects without a backing parent struct device (CONFIG_OF makes it possible), so we could be missing some cases here but this is not a big deal

- most Ethernet controller implementations typically set the device as a wake-up enabled device when an user issues the appropriate ethtool -s iface wol <parameters> and not at driver probe or ndo_open() time. There are exceptions like ASIX USB Ethernet adapters that AFAIR are wake-up enabled all the time. The main concern here is that at the time of suspend device_may_wakeup() evaluates to true and we keep the PHY on even though the driver/user was not requesting for WoL per-se

What you encode here is definitely the behavior that Ethernet controllers should have, but we should also audit drivers that use PHYLIB and implement WoL that this is not going to regresses their power management or ability to wake up from PHY, and finally, that there is not redundant code already in place.

>
>Fixes: f1e911d5d0df ("r8169: add basic phylib support")
>Fixes: e8cfd9d6c772 ("net: phy: call state machine synchronously in
>phy_stop")
>Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>---
>v2:
>- improved commit message
>- reduced scope of patch, don't touch functionality of
>  mdio_bus_phy_suspend and mdio_bus_phy_resume
>---
> drivers/net/phy/phy_device.c | 42 ++++++++++++++++++++++--------------
> 1 file changed, 26 insertions(+), 16 deletions(-)
>
>diff --git a/drivers/net/phy/phy_device.c
>b/drivers/net/phy/phy_device.c
>index af64a9320..4cab94bae 100644
>--- a/drivers/net/phy/phy_device.c
>+++ b/drivers/net/phy/phy_device.c
>@@ -75,6 +75,26 @@ extern struct phy_driver genphy_10g_driver;
> static LIST_HEAD(phy_fixup_list);
> static DEFINE_MUTEX(phy_fixup_lock);
> 
>+static bool phy_may_suspend(struct phy_device *phydev)
>+{
>+	struct net_device *netdev = phydev->attached_dev;
>+
>+	if (!netdev)
>+		return true;
>+
>+	/* Don't suspend PHY if the attached netdev parent may wakeup.
>+	 * The parent may point to a PCI device, as in tg3 driver.
>+	 */
>+	if (netdev->dev.parent && device_may_wakeup(netdev->dev.parent))
>+		return false;
>+
>+	/* Also don't suspend PHY if the netdev itself may wakeup. This
>+	 * is the case for devices w/o underlaying pwr. mgmt. aware bus,
>+	 * e.g. SoC devices.
>+	 */
>+	return !device_may_wakeup(&netdev->dev);
>+}
>+
> #ifdef CONFIG_PM
> static bool mdio_bus_phy_may_suspend(struct phy_device *phydev)
> {
>@@ -93,20 +113,7 @@ static bool mdio_bus_phy_may_suspend(struct
>phy_device *phydev)
> 	if (!netdev)
> 		return !phydev->suspended;
> 
>-	/* Don't suspend PHY if the attached netdev parent may wakeup.
>-	 * The parent may point to a PCI device, as in tg3 driver.
>-	 */
>-	if (netdev->dev.parent && device_may_wakeup(netdev->dev.parent))
>-		return false;
>-
>-	/* Also don't suspend PHY if the netdev itself may wakeup. This
>-	 * is the case for devices w/o underlaying pwr. mgmt. aware bus,
>-	 * e.g. SoC devices.
>-	 */
>-	if (device_may_wakeup(&netdev->dev))
>-		return false;
>-
>-	return true;
>+	return phy_may_suspend(phydev);
> }
> 
> static int mdio_bus_phy_suspend(struct device *dev)
>@@ -1132,9 +1139,9 @@ void phy_detach(struct phy_device *phydev)
> 		sysfs_remove_link(&dev->dev.kobj, "phydev");
> 		sysfs_remove_link(&phydev->mdio.dev.kobj, "attached_dev");
> 	}
>+	phy_suspend(phydev);
> 	phydev->attached_dev->phydev = NULL;
> 	phydev->attached_dev = NULL;
>-	phy_suspend(phydev);
> 	phydev->phylink = NULL;
> 
> 	phy_led_triggers_unregister(phydev);
>@@ -1171,9 +1178,12 @@ int phy_suspend(struct phy_device *phydev)
> 	struct ethtool_wolinfo wol = { .cmd = ETHTOOL_GWOL };
> 	int ret = 0;
> 
>+	if (phydev->suspended)
>+		return 0;
>+
> 	/* If the device has WOL enabled, we cannot suspend the PHY */
> 	phy_ethtool_get_wol(phydev, &wol);
>-	if (wol.wolopts)
>+	if (wol.wolopts || !phy_may_suspend(phydev))
> 		return -EBUSY;
> 
> 	if (phydev->drv && phydrv->suspend)

-- 
Florian

^ permalink raw reply

* Re: [PATCH net] af_key: free SKBs under RCU protection
From: Eric Dumazet @ 2018-09-23 17:15 UTC (permalink / raw)
  To: stranche, Eric Dumazet; +Cc: netdev, steffen.klassert
In-Reply-To: <357e28c3fa0c7bacaffde4e960f58a87@codeaurora.org>

[-- Attachment #1: Type: text/plain, Size: 818 bytes --]



On 09/20/2018 12:25 PM, stranche@codeaurora.org wrote:

> Perhaps a cleaner solution here is to always clone the SKB in
> pfkey_broadcast_one(). That will ensure that the two kfree_skb() calls
> in pfkey_broadcast() will never be passed an SKB with sock_rfree() as
> its destructor, and we can avoid this race condition.

Yes, this whole idea of avoiding the cloning is brain dead.

Better play safe and having a straightforward implementation.

I suggest something like this (I could not reproduce the bug with the syzkaller repro)

Note that I removed the sock_hold(sk)/sock_put(sk) pair as this is useless.
The only time GFP_KERNEL might be used is when the sk is already owned by the caller.


 net/key/af_key.c |   40 +++++++++++++++-------------------------
 1 file changed, 15 insertions(+), 25 deletions(-)


[-- Attachment #2: patch2712.txt --]
[-- Type: text/plain, Size: 2418 bytes --]

diff --git a/net/key/af_key.c b/net/key/af_key.c
index 9d61266526e767770d9a1ce184ac8cdd59de309a..7da629d5971712d5219528c55bad869bb084a343 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -196,30 +196,22 @@ static int pfkey_release(struct socket *sock)
 	return 0;
 }
 
-static int pfkey_broadcast_one(struct sk_buff *skb, struct sk_buff **skb2,
-			       gfp_t allocation, struct sock *sk)
+static int pfkey_broadcast_one(struct sk_buff *skb, gfp_t allocation,
+			       struct sock *sk)
 {
 	int err = -ENOBUFS;
 
-	sock_hold(sk);
-	if (*skb2 == NULL) {
-		if (refcount_read(&skb->users) != 1) {
-			*skb2 = skb_clone(skb, allocation);
-		} else {
-			*skb2 = skb;
-			refcount_inc(&skb->users);
-		}
-	}
-	if (*skb2 != NULL) {
-		if (atomic_read(&sk->sk_rmem_alloc) <= sk->sk_rcvbuf) {
-			skb_set_owner_r(*skb2, sk);
-			skb_queue_tail(&sk->sk_receive_queue, *skb2);
-			sk->sk_data_ready(sk);
-			*skb2 = NULL;
-			err = 0;
-		}
+	if (atomic_read(&sk->sk_rmem_alloc) > sk->sk_rcvbuf)
+		return err;
+
+	skb = skb_clone(skb, allocation);
+
+	if (skb) {
+		skb_set_owner_r(skb, sk);
+		skb_queue_tail(&sk->sk_receive_queue, skb);
+		sk->sk_data_ready(sk);
+		err = 0;
 	}
-	sock_put(sk);
 	return err;
 }
 
@@ -234,7 +226,6 @@ static int pfkey_broadcast(struct sk_buff *skb, gfp_t allocation,
 {
 	struct netns_pfkey *net_pfkey = net_generic(net, pfkey_net_id);
 	struct sock *sk;
-	struct sk_buff *skb2 = NULL;
 	int err = -ESRCH;
 
 	/* XXX Do we need something like netlink_overrun?  I think
@@ -253,7 +244,7 @@ static int pfkey_broadcast(struct sk_buff *skb, gfp_t allocation,
 		 * socket.
 		 */
 		if (pfk->promisc)
-			pfkey_broadcast_one(skb, &skb2, GFP_ATOMIC, sk);
+			pfkey_broadcast_one(skb, GFP_ATOMIC, sk);
 
 		/* the exact target will be processed later */
 		if (sk == one_sk)
@@ -268,7 +259,7 @@ static int pfkey_broadcast(struct sk_buff *skb, gfp_t allocation,
 				continue;
 		}
 
-		err2 = pfkey_broadcast_one(skb, &skb2, GFP_ATOMIC, sk);
+		err2 = pfkey_broadcast_one(skb, GFP_ATOMIC, sk);
 
 		/* Error is cleared after successful sending to at least one
 		 * registered KM */
@@ -278,9 +269,8 @@ static int pfkey_broadcast(struct sk_buff *skb, gfp_t allocation,
 	rcu_read_unlock();
 
 	if (one_sk != NULL)
-		err = pfkey_broadcast_one(skb, &skb2, allocation, one_sk);
+		err = pfkey_broadcast_one(skb, allocation, one_sk);
 
-	kfree_skb(skb2);
 	kfree_skb(skb);
 	return err;
 }

^ permalink raw reply related

* Re: [PATCH net] devlink: double free in devlink_resource_fill()
From: Jiri Pirko @ 2018-09-23 17:20 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: Jiri Pirko, David S. Miller, netdev, kernel-janitors
In-Reply-To: <20180921080755.GA16307@mwanda>

Fri, Sep 21, 2018 at 10:07:55AM CEST, dan.carpenter@oracle.com wrote:
>Smatch reports that devlink_dpipe_send_and_alloc_skb() frees the skb
>on error so this is a double free.  We fixed a bunch of these bugs in
>commit 7fe4d6dcbcb4 ("devlink: Remove redundant free on error path") but
>we accidentally overlooked this one.
>
>Fixes: d9f9b9a4d05f ("devlink: Add support for resource abstraction")
>Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Acked-by: Jiri Pirko <jiri@mellanox.com>

^ 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