Netdev List
 help / color / mirror / Atom feed
* [PATCH] rat_cs: Remove duplicate code
From: Hariprasad Kelam @ 2019-07-20 17:46 UTC (permalink / raw)
  To: Kalle Valo, David S. Miller, linux-wireless, netdev, linux-kernel

Code is same if translate is true/false in case invalid packet is
received.So remove else part.

Issue identified with coccicheck

Signed-off-by: Hariprasad Kelam <hariprasad.kelam@gmail.com>
---
 drivers/net/wireless/ray_cs.c | 29 ++++++++---------------------
 1 file changed, 8 insertions(+), 21 deletions(-)

diff --git a/drivers/net/wireless/ray_cs.c b/drivers/net/wireless/ray_cs.c
index cf37268..a51bbe7 100644
--- a/drivers/net/wireless/ray_cs.c
+++ b/drivers/net/wireless/ray_cs.c
@@ -2108,29 +2108,16 @@ static void rx_data(struct net_device *dev, struct rcs __iomem *prcs,
 #endif
 
 	if (!sniffer) {
-		if (translate) {
 /* TBD length needs fixing for translated header */
-			if (rx_len < (ETH_HLEN + RX_MAC_HEADER_LENGTH) ||
-			    rx_len >
-			    (dev->mtu + RX_MAC_HEADER_LENGTH + ETH_HLEN +
-			     FCS_LEN)) {
-				pr_debug(
-				      "ray_cs invalid packet length %d received\n",
-				      rx_len);
-				return;
-			}
-		} else { /* encapsulated ethernet */
-
-			if (rx_len < (ETH_HLEN + RX_MAC_HEADER_LENGTH) ||
-			    rx_len >
-			    (dev->mtu + RX_MAC_HEADER_LENGTH + ETH_HLEN +
-			     FCS_LEN)) {
-				pr_debug(
-				      "ray_cs invalid packet length %d received\n",
-				      rx_len);
-				return;
+		if (rx_len < (ETH_HLEN + RX_MAC_HEADER_LENGTH) ||
+		    rx_len >
+		    (dev->mtu + RX_MAC_HEADER_LENGTH + ETH_HLEN +
+		     FCS_LEN)) {
+			pr_debug(
+			      "ray_cs invalid packet length %d received\n",
+			      rx_len);
+			return;
 			}
-		}
 	}
 	pr_debug("ray_cs rx_data packet\n");
 	/* If fragmented packet, verify sizes of fragments add up */
-- 
2.7.4


^ permalink raw reply related

* Re: linux-headers-5.2 and proper use of SIOCGSTAMP
From: Florian Weimer @ 2019-07-20 18:10 UTC (permalink / raw)
  To: Sergei Trofimovich
  Cc: netdev, linux-kernel, libc-alpha, Arnd Bergmann, David S. Miller,
	mtk.manpages, linux-man
In-Reply-To: <20190720174844.4b989d34@sf>

* Sergei Trofimovich:

> Should #include <linux/sockios.h> always be included by user app?
> Or should glibc tweak it's definition of '#include <sys/socket.h>'
> to make it available on both old and new version of linux headers?

What is the reason for dropping SIOCGSTAMP from <asm/socket.h>?

If we know that, it will be much easier to decide what to do about
<sys/socket.h>.

^ permalink raw reply

* Re: linux-headers-5.2 and proper use of SIOCGSTAMP
From: Arnd Bergmann @ 2019-07-20 18:50 UTC (permalink / raw)
  To: Florian Weimer
  Cc: Sergei Trofimovich, Networking, Linux Kernel Mailing List,
	GNU C Library, David S. Miller, Michael Kerrisk, linux-man
In-Reply-To: <87wogca86l.fsf@mid.deneb.enyo.de>

On Sat, Jul 20, 2019 at 8:10 PM Florian Weimer <fw@deneb.enyo.de> wrote:
>
> * Sergei Trofimovich:
>
> > Should #include <linux/sockios.h> always be included by user app?
> > Or should glibc tweak it's definition of '#include <sys/socket.h>'
> > to make it available on both old and new version of linux headers?
>
> What is the reason for dropping SIOCGSTAMP from <asm/socket.h>?
>
> If we know that, it will be much easier to decide what to do about
> <sys/socket.h>.

As far as I can tell, nobody thought it would be a problem to move it
from asm/sockios.h to linux/sockios.h, as the general rule is that one
should use the linux/*.h version if both exist, and that the asm/*.h
version only contains architecture specific definitions. The new
definition is the same across all architectures, so it made sense to
have it in the common file.

If the assumption was wrong, the obvious solution is to duplicate the
definitions everywhere or move the common parts into
asm-generic/sockios.h, but it would have been better to hear about
that earlier.

      Arnd

^ permalink raw reply

* Re: [PATCH net] r8169: fix RTL8168g PHY init
From: David Miller @ 2019-07-20 19:18 UTC (permalink / raw)
  To: hkallweit1; +Cc: nic_swsd, netdev, tv
In-Reply-To: <eeb20312-1418-24c3-6482-09c051075b9e@gmail.com>

From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Sat, 20 Jul 2019 19:01:22 +0200

> From: Thomas Voegtle <tv@lio96.de>
> This fixes a copy&paste error in the original patch. Setting the wrong
> register resulted in massive packet loss on some systems.
> 
> Fixes: a2928d28643e ("r8169: use paged versions of phylib MDIO access functions")
> Tested-by: Thomas Voegtle <tv@lio96.de>
> Signed-off-by: Thomas Voegtle <tv@lio96.de>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Applied.

^ permalink raw reply

* Re: linux-headers-5.2 and proper use of SIOCGSTAMP
From: Florian Weimer @ 2019-07-20 19:34 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Sergei Trofimovich, Networking, Linux Kernel Mailing List,
	GNU C Library, David S. Miller, Michael Kerrisk, linux-man
In-Reply-To: <CAK8P3a3s3OeBj1MviaJV2UR0eUhF0GKPBi1iFf_3QKQyNPkuqw@mail.gmail.com>

* Arnd Bergmann:

> On Sat, Jul 20, 2019 at 8:10 PM Florian Weimer <fw@deneb.enyo.de> wrote:
>>
>> * Sergei Trofimovich:
>>
>> > Should #include <linux/sockios.h> always be included by user app?
>> > Or should glibc tweak it's definition of '#include <sys/socket.h>'
>> > to make it available on both old and new version of linux headers?
>>
>> What is the reason for dropping SIOCGSTAMP from <asm/socket.h>?
>>
>> If we know that, it will be much easier to decide what to do about
>> <sys/socket.h>.
>
> As far as I can tell, nobody thought it would be a problem to move it
> from asm/sockios.h to linux/sockios.h, as the general rule is that one
> should use the linux/*.h version if both exist, and that the asm/*.h
> version only contains architecture specific definitions. The new
> definition is the same across all architectures, so it made sense to
> have it in the common file.

Most of the socket-related constants are not exposed in UAPI headers,
although userspace is expected to use them.  It seems to me that due
to the lack of other options among the UAPI headers, <asm/socket.h>
has been a dumping ground for various socket-related things in the
past, whether actually architecture-specific or not.

<linux/socket.h> does not include <asm/socket.h>, so that's why we
usually end up with including <asm/socket.h> (perhaps indirectly via
<sys/socket.h>), which used to include <asm/sockios.h> on most (all?)
architectures.  That in turn provided some of the SIOC* constants in
the past, so people didn't investigate other options.

I think we can change glibc to include <linux/sockios.h> in addition
to <asm/socket.h>.  <linux/sockios.h> looks reasonably clean to me,
much better than <asm/socket.h>.  I'm still working on the other
breakage, and I'm severely limited by the machine resources I have
access to.

^ permalink raw reply

* pull-request: mac80211 2019-07-20
From: Johannes Berg @ 2019-07-20 20:24 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-wireless

Hi Dave,

Sorry, this really should've gone out much earlier, in partilar
the vendor command fixes. Not much for now, more -next material
will come later.

Please pull and let me know if there's any problem.

Thanks,
johannes



The following changes since commit 1a03bb532934e90c7d662f7c59f4f66ea8451fa4:

  r8169: fix RTL8168g PHY init (2019-07-20 12:17:45 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211.git tags/mac80211-for-davem-2019-07-20

for you to fetch changes up to d2b3fe42bc629c2d4002f652b3abdfb2e72991c7:

  mac80211: don't warn about CW params when not using them (2019-07-20 21:40:32 +0200)

----------------------------------------------------------------
We have a handful of fixes:
 * ignore bad CW parameters if we aren't using them,
   instead of warning
 * fix operation (and then build) with the new netlink vendor
   command policy requirement
 * fix a memory leak in an error path when setting beacons

----------------------------------------------------------------
Brian Norris (1):
      mac80211: don't warn about CW params when not using them

Johannes Berg (2):
      wireless: fix nl80211 vendor commands
      nl80211: fix VENDOR_CMD_RAW_DATA

John Crispin (1):
      nl80211: fix NL80211_HE_MAX_CAPABILITY_LEN

Lorenzo Bianconi (1):
      mac80211: fix possible memory leak in ieee80211_assign_beacon

 drivers/net/wireless/ath/wil6210/cfg80211.c               |  4 ++++
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/vendor.c |  1 +
 drivers/net/wireless/ti/wlcore/vendor_cmd.c               |  3 +++
 include/net/cfg80211.h                                    |  2 +-
 include/uapi/linux/nl80211.h                              |  2 +-
 net/mac80211/cfg.c                                        |  8 ++++++--
 net/mac80211/driver-ops.c                                 | 13 +++++++++----
 7 files changed, 25 insertions(+), 8 deletions(-)


^ permalink raw reply

* Re: linux-headers-5.2 and proper use of SIOCGSTAMP
From: Arnd Bergmann @ 2019-07-20 20:40 UTC (permalink / raw)
  To: Florian Weimer
  Cc: Sergei Trofimovich, Networking, Linux Kernel Mailing List,
	GNU C Library, David S. Miller, Michael Kerrisk, linux-man
In-Reply-To: <87muh8a4a3.fsf@mid.deneb.enyo.de>

On Sat, Jul 20, 2019 at 9:34 PM Florian Weimer <fw@deneb.enyo.de> wrote:
> * Arnd Bergmann:
> > On Sat, Jul 20, 2019 at 8:10 PM Florian Weimer <fw@deneb.enyo.de> wrote:
> > As far as I can tell, nobody thought it would be a problem to move it
> > from asm/sockios.h to linux/sockios.h, as the general rule is that one
> > should use the linux/*.h version if both exist, and that the asm/*.h
> > version only contains architecture specific definitions. The new
> > definition is the same across all architectures, so it made sense to
> > have it in the common file.
>
> Most of the socket-related constants are not exposed in UAPI headers,
> although userspace is expected to use them.  It seems to me that due
> to the lack of other options among the UAPI headers, <asm/socket.h>
> has been a dumping ground for various socket-related things in the
> past, whether actually architecture-specific or not.
>
> <linux/socket.h> does not include <asm/socket.h>, so that's why we
> usually end up with including <asm/socket.h> (perhaps indirectly via
> <sys/socket.h>), which used to include <asm/sockios.h> on most (all?)
> architectures.  That in turn provided some of the SIOC* constants in
> the past, so people didn't investigate other options.

It seems that both the missing constants and the fact that
linux/socket.h doesn't include asm/socket.h and linux/sockios.h
goes back to a 21 year old commit:

commit 74f513101058f7585176ea8cdf6fb026faea8a7e
Author: linus1 <torvalds@linuxfoundation.org>
Date:   Wed May 20 11:00:00 1998 -0800

    [tytso] include/asm-i386/posix_types.h
    This quick fix eliminates a lot of warning messages when
    compiling e2fsprogs under glibc.  This is because the glibc header files
    defines its own version of FD_SET, FD_ZERO, etc., and so if you need to
    #include the kernel include files, you get a lot of duplicate defined
    macro warning messages.  This patch simply #ifdef's out the kernel
    versions of these function if the kernel is not being compiled and the
    glibc header files are in use.

diff --git a/include/linux/socket.h b/include/linux/socket.h
index 08f0d281401c..35a7629b6b70 100644
--- a/include/linux/socket.h
+++ b/include/linux/socket.h
@@ -1,6 +1,8 @@
 #ifndef _LINUX_SOCKET_H
 #define _LINUX_SOCKET_H

+#if defined(__KERNEL__) || !defined(__GLIBC__) || (__GLIBC__ < 2)
+
 #include <asm/socket.h>                        /* arch-dependent
defines       */
 #include <linux/sockios.h>             /* the SIOCxxx I/O controls     */
 #include <linux/uio.h>                 /* iovec support                */
@@ -256,4 +258,5 @@ extern int move_addr_to_user(void *kaddr, int
klen, void *uaddr, int *ulen);
 extern int move_addr_to_kernel(void *uaddr, int ulen, void *kaddr);
 extern int put_cmsg(struct msghdr*, int level, int type, int len, void *data);
 #endif
+#endif /* not kernel and not glibc */
 #endif /* _LINUX_SOCKET_H */

(the same commit did similar changes in linux/stat.h and asm/posix_types.h)

Over time, the check for glibc was removed (to allow including linux/socket.h
before sys/socket.h), and all the #ifdef __KERNEL__ bits were removed
from the installed header as part of the uapi header split.

> I think we can change glibc to include <linux/sockios.h> in addition
> to <asm/socket.h>.  <linux/sockios.h> looks reasonably clean to me,
> much better than <asm/socket.h>.

That seems reasonable to me, but overall my fear is that these headers
are already so broken that any change will risk breaking something
in more or less unexpected ways.

        Arnd

^ permalink raw reply related

* Re: WARNING in xt_compat_add_offset
From: syzbot @ 2019-07-20 23:55 UTC (permalink / raw)
  To: bridge, coreteam, davem, fw, kadlec, kadlec, linux-kernel, netdev,
	netfilter-devel, nikolay, pablo, roopa, syzkaller-bugs
In-Reply-To: <00000000000081994205827ea9a0@google.com>

syzbot has found a reproducer for the following crash on:

HEAD commit:    abdfd52a Merge tag 'armsoc-defconfig' of git://git.kernel...
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=146c4968600000
kernel config:  https://syzkaller.appspot.com/x/.config?x=b8e53b1e149c0183
dashboard link: https://syzkaller.appspot.com/bug?extid=276ddebab3382bbf72db
compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
userspace arch: i386
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=159be500600000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=139364f0600000

The bug was bisected to:

commit 2035f3ff8eaa29cfb5c8e2160b0f6e85eeb21a95
Author: Florian Westphal <fw@strlen.de>
Date:   Mon Jan 21 20:54:36 2019 +0000

     netfilter: ebtables: compat: un-break 32bit setsockopt when no rules  
are present

bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=1462834d200000
final crash:    https://syzkaller.appspot.com/x/report.txt?x=1662834d200000
console output: https://syzkaller.appspot.com/x/log.txt?x=1262834d200000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+276ddebab3382bbf72db@syzkaller.appspotmail.com
Fixes: 2035f3ff8eaa ("netfilter: ebtables: compat: un-break 32bit  
setsockopt when no rules are present")

------------[ cut here ]------------
WARNING: CPU: 1 PID: 9012 at net/netfilter/x_tables.c:649  
xt_compat_add_offset.cold+0x11/0x36 /net/netfilter/x_tables.c:649
Kernel panic - not syncing: panic_on_warn set ...
CPU: 1 PID: 9012 Comm: syz-executor131 Not tainted 5.2.0+ #64
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Call Trace:
  __dump_stack /lib/dump_stack.c:77 [inline]
  dump_stack+0x172/0x1f0 /lib/dump_stack.c:113
  panic+0x2dc/0x755 /kernel/panic.c:219
  __warn.cold+0x20/0x4c /kernel/panic.c:576
  report_bug+0x263/0x2b0 /lib/bug.c:186
  fixup_bug /arch/x86/kernel/traps.c:179 [inline]
  fixup_bug /arch/x86/kernel/traps.c:174 [inline]
  do_error_trap+0x11b/0x200 /arch/x86/kernel/traps.c:272
  do_invalid_op+0x37/0x50 /arch/x86/kernel/traps.c:291
  invalid_op+0x14/0x20 /arch/x86/entry/entry_64.S:1008
RIP: 0010:xt_compat_add_offset.cold+0x11/0x36 /net/netfilter/x_tables.c:649
Code: 89 ee 48 c7 c7 c0 29 2d 88 e8 0c 76 7b fb 41 bc ea ff ff ff e9 87 88  
ff ff e8 08 d3 91 fb 48 c7 c7 00 2a 2d 88 e8 f0 75 7b fb <0f> 0b 41 bc f4  
ff ff ff e9 01 8b ff ff e8 ea d2 91 fb 48 c7 c7 00
RSP: 0018:ffff8880a382f8d8 EFLAGS: 00010286
RAX: 0000000000000024 RBX: ffff888216b74ad0 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffffffff815c3a26 RDI: ffffed1014705f0d
RBP: ffff8880a382f908 R08: 0000000000000024 R09: ffffed1015d260b1
R10: ffffed1015d260b0 R11: ffff8880ae930587 R12: 0000000000000014
R13: 0000000000000060 R14: ffff88808b7b6180 R15: 0000000000000000
  size_entry_mwt /net/bridge/netfilter/ebtables.c:2122 [inline]
  compat_copy_entries+0x10e9/0x1340 /net/bridge/netfilter/ebtables.c:2147
  compat_do_replace+0x3b3/0x680 /net/bridge/netfilter/ebtables.c:2243
  compat_do_ebt_set_ctl+0x22f/0x27e /net/bridge/netfilter/ebtables.c:2325
  compat_nf_sockopt /net/netfilter/nf_sockopt.c:144 [inline]
  compat_nf_setsockopt+0x98/0x140 /net/netfilter/nf_sockopt.c:156
  compat_ip_setsockopt /net/ipv4/ip_sockglue.c:1286 [inline]
  compat_ip_setsockopt+0x106/0x140 /net/ipv4/ip_sockglue.c:1267
  compat_raw_setsockopt+0xe0/0x100 /net/ipv4/raw.c:865
  compat_sock_common_setsockopt+0xb2/0x140 /net/core/sock.c:3141
  __compat_sys_setsockopt+0x185/0x380 /net/compat.c:384
  __do_compat_sys_setsockopt /net/compat.c:397 [inline]
  __se_compat_sys_setsockopt /net/compat.c:394 [inline]
  __ia32_compat_sys_setsockopt+0xbd/0x150 /net/compat.c:394
  do_syscall_32_irqs_on /arch/x86/entry/common.c:332 [inline]
  do_fast_syscall_32+0x27b/0xdb3 /arch/x86/entry/common.c:403
  entry_SYSENTER_compat+0x70/0x7f /arch/x86/entry/entry_64_compat.S:139
RIP: 0023:0xf7f489c9
Code: d3 83 c4 10 5b 5e 5d c3 ba 80 96 98 00 eb a9 8b 04 24 c3 8b 34 24 c3  
8b 3c 24 c3 90 90 90 90 90 90 51 52 55 89 e5 0f 34 cd 80 <5d> 5a 59 c3 90  
90 90 90 eb 0d 90 90 90 90 90 90 90 90 90 90 90 90
RSP: 002b:00000000ffaa6d8c EFLAGS: 00000292 ORIG_RAX: 000000000000016e
RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 0000000000000000
RDX: 0000000000000080 RSI: 0000000020000000 RDI: 00000000000001fc
RBP: 0000000000000012 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
Kernel Offset: disabled
Rebooting in 86400 seconds..


^ permalink raw reply

* Re: [PATCH] netfilter: ebtables: compat: fix a memory leak bug
From: Florian Westphal @ 2019-07-21  0:26 UTC (permalink / raw)
  To: Wenwen Wang
  Cc: Wenwen Wang, Pablo Neira Ayuso, Jozsef Kadlecsik,
	Florian Westphal, Roopa Prabhu, Nikolay Aleksandrov,
	David S. Miller, open list:NETFILTER, open list:NETFILTER,
	moderated list:ETHERNET BRIDGE, open list:ETHERNET BRIDGE,
	open list
In-Reply-To: <1563625366-3602-1-git-send-email-wang6495@umn.edu>

Wenwen Wang <wang6495@umn.edu> wrote:
> From: Wenwen Wang <wenwen@cs.uga.edu>
> 
> In compat_do_replace(), a temporary buffer is allocated through vmalloc()
> to hold entries copied from the user space. The buffer address is firstly
> saved to 'newinfo->entries', and later on assigned to 'entries_tmp'. Then
> the entries in this temporary buffer is copied to the internal kernel
> structure through compat_copy_entries(). If this copy process fails,
> compat_do_replace() should be terminated. However, the allocated temporary
> buffer is not freed on this path, leading to a memory leak.
> 
> To fix the bug, free the buffer before returning from compat_do_replace().
> 
> Signed-off-by: Wenwen Wang <wenwen@cs.uga.edu>

Reviewed-by: Florian Westphal <fw@strlen.de>

^ permalink raw reply

* [PATCH] allocate_flower_entry: should check for null deref
From: Navid Emamdoost @ 2019-07-21  6:37 UTC (permalink / raw)
  Cc: kjlu, smccaman, secalert, emamd001, Navid Emamdoost,
	Vishal Kulkarni, David S. Miller, netdev, linux-kernel

allocate_flower_entry does not check for allocation success, but tries
to deref the result. I only moved the spin_lock under null check, because
 the caller is checking allocation's status at line 652.

Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
---
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c
index 312599c6b35a..e447976bdd3e 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c
@@ -67,7 +67,8 @@ static struct ch_tc_pedit_fields pedits[] = {
 static struct ch_tc_flower_entry *allocate_flower_entry(void)
 {
 	struct ch_tc_flower_entry *new = kzalloc(sizeof(*new), GFP_KERNEL);
-	spin_lock_init(&new->lock);
+	if (new)
+		spin_lock_init(&new->lock);
 	return new;
 }
 
-- 
2.17.1


^ permalink raw reply related

* Re: [PATCH] rat_cs: Remove duplicate code
From: Stefano Brivio @ 2019-07-21  9:12 UTC (permalink / raw)
  To: Hariprasad Kelam
  Cc: Kalle Valo, David S. Miller, linux-wireless, netdev, linux-kernel
In-Reply-To: <20190720174613.GA31062@hari-Inspiron-1545>

On Sat, 20 Jul 2019 23:16:47 +0530
Hariprasad Kelam <hariprasad.kelam@gmail.com> wrote:

> Code is same if translate is true/false in case invalid packet is
> received.So remove else part.
> 
> Issue identified with coccicheck
> 
> Signed-off-by: Hariprasad Kelam <hariprasad.kelam@gmail.com>
> ---
>  drivers/net/wireless/ray_cs.c | 29 ++++++++---------------------
>  1 file changed, 8 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/net/wireless/ray_cs.c b/drivers/net/wireless/ray_cs.c
> index cf37268..a51bbe7 100644
> --- a/drivers/net/wireless/ray_cs.c
> +++ b/drivers/net/wireless/ray_cs.c
> @@ -2108,29 +2108,16 @@ static void rx_data(struct net_device *dev, struct rcs __iomem *prcs,
>  #endif
>  
>  	if (!sniffer) {
> -		if (translate) {
>  /* TBD length needs fixing for translated header */
> -			if (rx_len < (ETH_HLEN + RX_MAC_HEADER_LENGTH) ||
> -			    rx_len >
> -			    (dev->mtu + RX_MAC_HEADER_LENGTH + ETH_HLEN +
> -			     FCS_LEN)) {
> -				pr_debug(
> -				      "ray_cs invalid packet length %d received\n",
> -				      rx_len);
> -				return;
> -			}
> -		} else { /* encapsulated ethernet */
> -
> -			if (rx_len < (ETH_HLEN + RX_MAC_HEADER_LENGTH) ||
> -			    rx_len >
> -			    (dev->mtu + RX_MAC_HEADER_LENGTH + ETH_HLEN +
> -			     FCS_LEN)) {
> -				pr_debug(
> -				      "ray_cs invalid packet length %d received\n",
> -				      rx_len);
> -				return;
> +		if (rx_len < (ETH_HLEN + RX_MAC_HEADER_LENGTH) ||
> +		    rx_len >
> +		    (dev->mtu + RX_MAC_HEADER_LENGTH + ETH_HLEN +
> +		     FCS_LEN)) {
> +			pr_debug(
> +			      "ray_cs invalid packet length %d received\n",
> +			      rx_len);
> +			return;
>  			}
> -		}

NACK. The TBD comment makes no sense anymore if you remove one of the
branches. Believe me or not, I have one of those cards, a (yes, 22
years old) Buslink Raytheon model 24020. That check needed (for sure)
and needs (maybe) to be fixed.

Besides, patch subject and resulting coding style are also wrong.

-- 
Stefano

^ permalink raw reply

* [PATCH] tipc: Fix a typo
From: Christophe JAILLET @ 2019-07-21 10:38 UTC (permalink / raw)
  To: jon.maloy, ying.xue, davem
  Cc: netdev, tipc-discussion, linux-kernel, kernel-janitors,
	Christophe JAILLET

s/tipc_toprsv_listener_data_ready/tipc_topsrv_listener_data_ready/
(r and s switched in topsrv)

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
The function name could also be removed from the comment. It does not
bring any useful information IMHO.
---
 net/tipc/topsrv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/tipc/topsrv.c b/net/tipc/topsrv.c
index f345662890a6..ca8ac96d22a9 100644
--- a/net/tipc/topsrv.c
+++ b/net/tipc/topsrv.c
@@ -476,7 +476,7 @@ static void tipc_topsrv_accept(struct work_struct *work)
 	}
 }
 
-/* tipc_toprsv_listener_data_ready - interrupt callback with connection request
+/* tipc_topsrv_listener_data_ready - interrupt callback with connection request
  * The queued job is launched into tipc_topsrv_accept()
  */
 static void tipc_topsrv_listener_data_ready(struct sock *sk)
-- 
2.20.1


^ permalink raw reply related

* Re: ENOBUILD in nf_tables
From: Jeremy Sowden @ 2019-07-21 11:15 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: Jakub Kicinski, netdev@vger.kernel.org
In-Reply-To: <20190720074443.nteynyxuptizbkdx@salvia>

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

On 2019-07-20, at 09:44:43 +0200, Pablo Neira Ayuso wrote:
> On Fri, Jul 19, 2019 at 10:07:43AM -0700, Jakub Kicinski wrote:
> > I hit the following build breakage on net with the config attached.
> >
> > GCC 9, doesn't seem like your just-posted series fixes this?
>
> $ gcc -v
> ...
> gcc version 9.1.0
>
> $ make
>   ...
>   CC      include/net/netfilter/nf_tables_offload.h.s
>   ...
> $
>
> Works for me here.

I think the compiler version is a red herring.  I reproduced the failure
with gcc-8 (Debian 8.3.0-6) and gcc-9 (Debian 9.1.0-4).  The problem is
that CONFIG_HEADER_TEST is defined and nf_tables_offload.h is not on the
header-test blacklist, but includes nf_tables.h, which is.

J.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* [PATCH net] kbuild: add net/netfilter/nf_tables_offload.h to header-test blacklist.
From: Jeremy Sowden @ 2019-07-21 11:31 UTC (permalink / raw)
  To: Netdev; +Cc: Pablo Neira Ayuso, Jakub Kicinski
In-Reply-To: <20190719100743.2ea14575@cakuba.netronome.com>

net/netfilter/nf_tables_offload.h includes net/netfilter/nf_tables.h
which is itself on the blacklist.

Reported-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 include/Kbuild | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/Kbuild b/include/Kbuild
index 7e9f1acb9dd5..8de846a83d8f 100644
--- a/include/Kbuild
+++ b/include/Kbuild
@@ -909,6 +909,7 @@ header-test-			+= net/netfilter/nf_tables.h
 header-test-			+= net/netfilter/nf_tables_core.h
 header-test-			+= net/netfilter/nf_tables_ipv4.h
 header-test-			+= net/netfilter/nf_tables_ipv6.h
+header-test-			+= net/netfilter/nf_tables_offload.h
 header-test-			+= net/netfilter/nft_fib.h
 header-test-			+= net/netfilter/nft_meta.h
 header-test-			+= net/netfilter/nft_reject.h
-- 
2.20.1


^ permalink raw reply related

* [PATCHv2 0/2] forcedeth: recv cache to make NIC work steadily
From: Zhu Yanjun @ 2019-07-21 12:53 UTC (permalink / raw)
  To: yanjun.zhu, davem, netdev

These patches are to this scenario:

"
When the host run for long time, there are a lot of memory fragments in
the hosts. And it is possible that kernel will compact memory fragments.
But normally it is difficult for NIC driver to allocate a memory from
kernel. From this variable stat_rx_dropped, we can confirm that NIC driver
can not allocate skb very frequently.
"

Since NIC driver can not allocate skb in time, this makes some important
tasks not be completed in time.
To avoid it, a recv cache is created to pre-allocate skb for NIC driver.
This can make the important tasks be completed in time.
From Nan's tests in LAB, these patches can make NIC driver work steadily.
Now in production hosts, these patches are applied.

With these patches, one NIC port needs 125MiB reserved. This 125MiB memory
can not be used by others. To a host on which the communications are not
mandatory, it is not necessary to reserve so much memory. So this recv cache
is disabled by default.

V1->V2:
1. ndelay is replaced with GFP_KERNEL function __netdev_alloc_skb.
2. skb_queue_purge is used when recv cache is destroyed.
3. RECV_LIST_ALLOCATE bit is removed.
4. schedule_delayed_work is moved out of while loop.

Zhu Yanjun (2):
  forcedeth: add recv cache make nic work steadily
  forcedeth: disable recv cache by default

 drivers/net/ethernet/nvidia/Kconfig     |  11 +++
 drivers/net/ethernet/nvidia/Makefile    |   1 +
 drivers/net/ethernet/nvidia/forcedeth.c | 129 +++++++++++++++++++++++++++++++-
 3 files changed, 139 insertions(+), 2 deletions(-)

-- 
2.7.4


^ permalink raw reply

* [PATCHv2 1/2] forcedeth: add recv cache to make nic work steadily
From: Zhu Yanjun @ 2019-07-21 12:53 UTC (permalink / raw)
  To: yanjun.zhu, davem, netdev
In-Reply-To: <1563713633-25528-1-git-send-email-yanjun.zhu@oracle.com>

A recv cache is added. The size of recv cache is 1000Mbit / skb_length.
When the system memory is not enough, this recv cache can make nic work
steadily.
When nic is up, this recv cache and work queue are created. When nic
is down, this recv cache will be destroyed and delayed workqueue is
canceled.
When nic is polled or rx interrupt is triggerred, rx handler will
get a skb from recv cache. Then a work is queued to fill up recv cache.
When skb size is changed, the old recv cache is destroyed and new recv
cache is created.
When the system memory is not enough, the allocation of skb failed. Then
recv cache will continue allocate skb with GFP_KERNEL until the recv
cache is filled up. When the system memory is not enough, this can make
nic work steadily. Becase of recv cache, the performance of nic is
enhanced.

CC: Joe Jin <joe.jin@oracle.com>
CC: Junxiao Bi <junxiao.bi@oracle.com>
Tested-by: Nan san <nan.1986san@gmail.com>
Signed-off-by: Zhu Yanjun <yanjun.zhu@oracle.com>
---
 drivers/net/ethernet/nvidia/forcedeth.c | 103 +++++++++++++++++++++++++++++++-
 1 file changed, 101 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/nvidia/forcedeth.c b/drivers/net/ethernet/nvidia/forcedeth.c
index b327b29..f8e766f 100644
--- a/drivers/net/ethernet/nvidia/forcedeth.c
+++ b/drivers/net/ethernet/nvidia/forcedeth.c
@@ -674,6 +674,11 @@ struct nv_ethtool_stats {
 	u64 tx_broadcast;
 };
 
+/* 1000Mb is 125M bytes, 125 * 1024 * 1024 bytes
+ * The length of recv cache is 125M / skb_length
+ */
+#define RECV_CACHE_LIST_LENGTH		(125 * 1024 * 1024 / np->rx_buf_sz)
+
 #define NV_DEV_STATISTICS_V3_COUNT (sizeof(struct nv_ethtool_stats)/sizeof(u64))
 #define NV_DEV_STATISTICS_V2_COUNT (NV_DEV_STATISTICS_V3_COUNT - 3)
 #define NV_DEV_STATISTICS_V1_COUNT (NV_DEV_STATISTICS_V2_COUNT - 6)
@@ -844,6 +849,11 @@ struct fe_priv {
 	char name_rx[IFNAMSIZ + 3];       /* -rx    */
 	char name_tx[IFNAMSIZ + 3];       /* -tx    */
 	char name_other[IFNAMSIZ + 6];    /* -other */
+
+	/* This is to schedule work */
+	struct delayed_work     recv_cache_work;
+	/* This list is to store skb queue for recv */
+	struct sk_buff_head recv_list;
 };
 
 /*
@@ -1804,7 +1814,8 @@ static int nv_alloc_rx(struct net_device *dev)
 		less_rx = np->last_rx.orig;
 
 	while (np->put_rx.orig != less_rx) {
-		struct sk_buff *skb = netdev_alloc_skb(dev, np->rx_buf_sz + NV_RX_ALLOC_PAD);
+		struct sk_buff *skb = skb_dequeue(&np->recv_list);
+
 		if (likely(skb)) {
 			np->put_rx_ctx->skb = skb;
 			np->put_rx_ctx->dma = dma_map_single(&np->pci_dev->dev,
@@ -1829,9 +1840,15 @@ static int nv_alloc_rx(struct net_device *dev)
 			u64_stats_update_begin(&np->swstats_rx_syncp);
 			np->stat_rx_dropped++;
 			u64_stats_update_end(&np->swstats_rx_syncp);
+
+			schedule_delayed_work(&np->recv_cache_work, 0);
+
 			return 1;
 		}
 	}
+
+	schedule_delayed_work(&np->recv_cache_work, 0);
+
 	return 0;
 }
 
@@ -1845,7 +1862,8 @@ static int nv_alloc_rx_optimized(struct net_device *dev)
 		less_rx = np->last_rx.ex;
 
 	while (np->put_rx.ex != less_rx) {
-		struct sk_buff *skb = netdev_alloc_skb(dev, np->rx_buf_sz + NV_RX_ALLOC_PAD);
+		struct sk_buff *skb = skb_dequeue(&np->recv_list);
+
 		if (likely(skb)) {
 			np->put_rx_ctx->skb = skb;
 			np->put_rx_ctx->dma = dma_map_single(&np->pci_dev->dev,
@@ -1871,9 +1889,15 @@ static int nv_alloc_rx_optimized(struct net_device *dev)
 			u64_stats_update_begin(&np->swstats_rx_syncp);
 			np->stat_rx_dropped++;
 			u64_stats_update_end(&np->swstats_rx_syncp);
+
+			schedule_delayed_work(&np->recv_cache_work, 0);
+
 			return 1;
 		}
 	}
+
+	schedule_delayed_work(&np->recv_cache_work, 0);
+
 	return 0;
 }
 
@@ -1957,6 +1981,43 @@ static void nv_init_tx(struct net_device *dev)
 	}
 }
 
+static void nv_init_recv_cache(struct net_device *dev)
+{
+	struct fe_priv *np = netdev_priv(dev);
+
+	skb_queue_head_init(&np->recv_list);
+	while (skb_queue_len(&np->recv_list) < RECV_CACHE_LIST_LENGTH) {
+		struct sk_buff *skb = netdev_alloc_skb(dev,
+				 np->rx_buf_sz + NV_RX_ALLOC_PAD);
+		/* skb is null. This indicates that memory is not
+		 * enough.
+		 */
+		if (unlikely(!skb)) {
+			/* When allocating memory with GFP_ATOMIC fails,
+			 * allocating with GFP_KERNEL will get memory
+			 * finally.
+			 */
+			skb = __netdev_alloc_skb(dev,
+						 np->rx_buf_sz +
+						 NV_RX_ALLOC_PAD,
+						 GFP_KERNEL);
+		}
+
+		skb_queue_tail(&np->recv_list, skb);
+	}
+}
+
+static void nv_destroy_recv_cache(struct net_device *dev)
+{
+	struct fe_priv *np = netdev_priv(dev);
+
+	cancel_delayed_work_sync(&np->recv_cache_work);
+	WARN_ON(delayed_work_pending(&np->recv_cache_work));
+
+	skb_queue_purge(&np->recv_list);
+	WARN_ON(skb_queue_len(&np->recv_list));
+}
+
 static int nv_init_ring(struct net_device *dev)
 {
 	struct fe_priv *np = netdev_priv(dev);
@@ -3047,6 +3108,8 @@ static int nv_change_mtu(struct net_device *dev, int new_mtu)
 		nv_drain_rxtx(dev);
 		/* reinit driver view of the rx queue */
 		set_bufsize(dev);
+		nv_destroy_recv_cache(dev);
+		nv_init_recv_cache(dev);
 		if (nv_init_ring(dev)) {
 			if (!np->in_shutdown)
 				mod_timer(&np->oom_kick, jiffies + OOM_REFILL);
@@ -4074,6 +4137,32 @@ static void nv_free_irq(struct net_device *dev)
 	}
 }
 
+static void nv_recv_cache_worker(struct work_struct *work)
+{
+	struct fe_priv *np = container_of(work, struct fe_priv,
+					  recv_cache_work.work);
+
+	while (skb_queue_len(&np->recv_list) < RECV_CACHE_LIST_LENGTH) {
+		struct sk_buff *skb = netdev_alloc_skb(np->dev,
+				np->rx_buf_sz + NV_RX_ALLOC_PAD);
+		/* skb is null. This indicates that memory is not
+		 * enough.
+		 */
+		if (unlikely(!skb)) {
+			/* When allocating memory with GFP_ATOMIC fails,
+			 * allocating with GFP_KERNEL will get memory
+			 * finally.
+			 */
+			skb = __netdev_alloc_skb(np->dev,
+						 np->rx_buf_sz +
+						 NV_RX_ALLOC_PAD,
+						 GFP_KERNEL);
+		}
+
+		skb_queue_tail(&np->recv_list, skb);
+	}
+}
+
 static void nv_do_nic_poll(struct timer_list *t)
 {
 	struct fe_priv *np = from_timer(np, t, nic_poll);
@@ -4129,6 +4218,8 @@ static void nv_do_nic_poll(struct timer_list *t)
 			nv_drain_rxtx(dev);
 			/* reinit driver view of the rx queue */
 			set_bufsize(dev);
+			nv_destroy_recv_cache(dev);
+			nv_init_recv_cache(dev);
 			if (nv_init_ring(dev)) {
 				if (!np->in_shutdown)
 					mod_timer(&np->oom_kick, jiffies + OOM_REFILL);
@@ -4681,6 +4772,8 @@ static int nv_set_ringparam(struct net_device *dev, struct ethtool_ringparam* ri
 	if (netif_running(dev)) {
 		/* reinit driver view of the queues */
 		set_bufsize(dev);
+		nv_destroy_recv_cache(dev);
+		nv_init_recv_cache(dev);
 		if (nv_init_ring(dev)) {
 			if (!np->in_shutdown)
 				mod_timer(&np->oom_kick, jiffies + OOM_REFILL);
@@ -5402,6 +5495,9 @@ static int nv_open(struct net_device *dev)
 
 	/* initialize descriptor rings */
 	set_bufsize(dev);
+	nv_init_recv_cache(dev);
+
+	INIT_DELAYED_WORK(&np->recv_cache_work, nv_recv_cache_worker);
 	oom = nv_init_ring(dev);
 
 	writel(0, base + NvRegLinkSpeed);
@@ -5583,6 +5679,9 @@ static int nv_close(struct net_device *dev)
 		nv_txrx_gate(dev, true);
 	}
 
+	/* free all SKBs in recv cache */
+	nv_destroy_recv_cache(dev);
+
 	/* FIXME: power down nic */
 
 	return 0;
-- 
2.7.4


^ permalink raw reply related

* [PATCHv2 2/2] forcedeth: disable recv cache by default
From: Zhu Yanjun @ 2019-07-21 12:53 UTC (permalink / raw)
  To: yanjun.zhu, davem, netdev
In-Reply-To: <1563713633-25528-1-git-send-email-yanjun.zhu@oracle.com>

The recv cache is to allocate 125MiB memory to reserve for NIC.
In the past time, this recv cache works very well. When the memory
is not enough, this recv cache reserves memory for NIC.
And the communications through this NIC is not affected by the
memory shortage. And the performance of NIC is better because of
this recv cache.
But this recv cache reserves 125MiB memory for one NIC port. Normally
there are 2 NIC ports in one card. So in a host, there are about 250
MiB memory reserved for NIC ports. To a host on which communications
are not mandatory, it is not necessary to reserve memory.
So this recv cache is disabled by default.

CC: Joe Jin <joe.jin@oracle.com>
CC: Junxiao Bi <junxiao.bi@oracle.com>
Tested-by: Nan san <nan.1986san@gmail.com>
Signed-off-by: Zhu Yanjun <yanjun.zhu@oracle.com>
---
 drivers/net/ethernet/nvidia/Kconfig     | 11 ++++++++
 drivers/net/ethernet/nvidia/Makefile    |  1 +
 drivers/net/ethernet/nvidia/forcedeth.c | 46 ++++++++++++++++++++++++++-------
 3 files changed, 48 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/nvidia/Kconfig b/drivers/net/ethernet/nvidia/Kconfig
index faacbd1..9a9f42a 100644
--- a/drivers/net/ethernet/nvidia/Kconfig
+++ b/drivers/net/ethernet/nvidia/Kconfig
@@ -26,4 +26,15 @@ config FORCEDETH
 	  To compile this driver as a module, choose M here. The module
 	  will be called forcedeth.
 
+config	FORCEDETH_RECV_CACHE
+	bool "nForce Ethernet recv cache support"
+	depends on FORCEDETH
+	default n
+	---help---
+	  The recv cache can make nic work steadily when the system memory is
+	  not enough. And it can also enhance nic performance. But to a host
+	  on which the communications are not mandatory, it is not necessary
+	  to reserve 125MiB memory for NIC.
+	  So recv cache is disabled by default.
+
 endif # NET_VENDOR_NVIDIA
diff --git a/drivers/net/ethernet/nvidia/Makefile b/drivers/net/ethernet/nvidia/Makefile
index 8935699..40c055e 100644
--- a/drivers/net/ethernet/nvidia/Makefile
+++ b/drivers/net/ethernet/nvidia/Makefile
@@ -4,3 +4,4 @@
 #
 
 obj-$(CONFIG_FORCEDETH) += forcedeth.o
+ccflags-$(CONFIG_FORCEDETH_RECV_CACHE)	:=	-DFORCEDETH_RECV_CACHE
diff --git a/drivers/net/ethernet/nvidia/forcedeth.c b/drivers/net/ethernet/nvidia/forcedeth.c
index f8e766f..deda276 100644
--- a/drivers/net/ethernet/nvidia/forcedeth.c
+++ b/drivers/net/ethernet/nvidia/forcedeth.c
@@ -674,10 +674,12 @@ struct nv_ethtool_stats {
 	u64 tx_broadcast;
 };
 
+#ifdef FORCEDETH_RECV_CACHE
 /* 1000Mb is 125M bytes, 125 * 1024 * 1024 bytes
  * The length of recv cache is 125M / skb_length
  */
 #define RECV_CACHE_LIST_LENGTH		(125 * 1024 * 1024 / np->rx_buf_sz)
+#endif
 
 #define NV_DEV_STATISTICS_V3_COUNT (sizeof(struct nv_ethtool_stats)/sizeof(u64))
 #define NV_DEV_STATISTICS_V2_COUNT (NV_DEV_STATISTICS_V3_COUNT - 3)
@@ -850,10 +852,12 @@ struct fe_priv {
 	char name_tx[IFNAMSIZ + 3];       /* -tx    */
 	char name_other[IFNAMSIZ + 6];    /* -other */
 
+#ifdef FORCEDETH_RECV_CACHE
 	/* This is to schedule work */
 	struct delayed_work     recv_cache_work;
 	/* This list is to store skb queue for recv */
 	struct sk_buff_head recv_list;
+#endif
 };
 
 /*
@@ -1814,8 +1818,12 @@ static int nv_alloc_rx(struct net_device *dev)
 		less_rx = np->last_rx.orig;
 
 	while (np->put_rx.orig != less_rx) {
+#ifdef FORCEDETH_RECV_CACHE
 		struct sk_buff *skb = skb_dequeue(&np->recv_list);
-
+#else
+		struct sk_buff *skb = netdev_alloc_skb(np->dev,
+					 np->rx_buf_sz + NV_RX_ALLOC_PAD);
+#endif
 		if (likely(skb)) {
 			np->put_rx_ctx->skb = skb;
 			np->put_rx_ctx->dma = dma_map_single(&np->pci_dev->dev,
@@ -1840,15 +1848,15 @@ static int nv_alloc_rx(struct net_device *dev)
 			u64_stats_update_begin(&np->swstats_rx_syncp);
 			np->stat_rx_dropped++;
 			u64_stats_update_end(&np->swstats_rx_syncp);
-
+#ifdef FORCEDETH_RECV_CACHE
 			schedule_delayed_work(&np->recv_cache_work, 0);
-
+#endif
 			return 1;
 		}
 	}
-
+#ifdef FORCEDETH_RECV_CACHE
 	schedule_delayed_work(&np->recv_cache_work, 0);
-
+#endif
 	return 0;
 }
 
@@ -1862,7 +1870,12 @@ static int nv_alloc_rx_optimized(struct net_device *dev)
 		less_rx = np->last_rx.ex;
 
 	while (np->put_rx.ex != less_rx) {
+#ifdef FORCEDETH_RECV_CACHE
 		struct sk_buff *skb = skb_dequeue(&np->recv_list);
+#else
+		struct sk_buff *skb = netdev_alloc_skb(np->dev,
+					np->rx_buf_sz + NV_RX_ALLOC_PAD);
+#endif
 
 		if (likely(skb)) {
 			np->put_rx_ctx->skb = skb;
@@ -1889,15 +1902,15 @@ static int nv_alloc_rx_optimized(struct net_device *dev)
 			u64_stats_update_begin(&np->swstats_rx_syncp);
 			np->stat_rx_dropped++;
 			u64_stats_update_end(&np->swstats_rx_syncp);
-
+#ifdef FORCEDETH_RECV_CACHE
 			schedule_delayed_work(&np->recv_cache_work, 0);
-
+#endif
 			return 1;
 		}
 	}
-
+#ifdef FORCEDETH_RECV_CACHE
 	schedule_delayed_work(&np->recv_cache_work, 0);
-
+#endif
 	return 0;
 }
 
@@ -1981,6 +1994,7 @@ static void nv_init_tx(struct net_device *dev)
 	}
 }
 
+#ifdef FORCEDETH_RECV_CACHE
 static void nv_init_recv_cache(struct net_device *dev)
 {
 	struct fe_priv *np = netdev_priv(dev);
@@ -2017,6 +2031,7 @@ static void nv_destroy_recv_cache(struct net_device *dev)
 	skb_queue_purge(&np->recv_list);
 	WARN_ON(skb_queue_len(&np->recv_list));
 }
+#endif
 
 static int nv_init_ring(struct net_device *dev)
 {
@@ -3108,8 +3123,10 @@ static int nv_change_mtu(struct net_device *dev, int new_mtu)
 		nv_drain_rxtx(dev);
 		/* reinit driver view of the rx queue */
 		set_bufsize(dev);
+#ifdef FORCEDETH_RECV_CACHE
 		nv_destroy_recv_cache(dev);
 		nv_init_recv_cache(dev);
+#endif
 		if (nv_init_ring(dev)) {
 			if (!np->in_shutdown)
 				mod_timer(&np->oom_kick, jiffies + OOM_REFILL);
@@ -4137,6 +4154,7 @@ static void nv_free_irq(struct net_device *dev)
 	}
 }
 
+#ifdef FORCEDETH_RECV_CACHE
 static void nv_recv_cache_worker(struct work_struct *work)
 {
 	struct fe_priv *np = container_of(work, struct fe_priv,
@@ -4162,6 +4180,7 @@ static void nv_recv_cache_worker(struct work_struct *work)
 		skb_queue_tail(&np->recv_list, skb);
 	}
 }
+#endif
 
 static void nv_do_nic_poll(struct timer_list *t)
 {
@@ -4218,8 +4237,10 @@ static void nv_do_nic_poll(struct timer_list *t)
 			nv_drain_rxtx(dev);
 			/* reinit driver view of the rx queue */
 			set_bufsize(dev);
+#ifdef FORCEDETH_RECV_CACHE
 			nv_destroy_recv_cache(dev);
 			nv_init_recv_cache(dev);
+#endif
 			if (nv_init_ring(dev)) {
 				if (!np->in_shutdown)
 					mod_timer(&np->oom_kick, jiffies + OOM_REFILL);
@@ -4772,8 +4793,10 @@ static int nv_set_ringparam(struct net_device *dev, struct ethtool_ringparam* ri
 	if (netif_running(dev)) {
 		/* reinit driver view of the queues */
 		set_bufsize(dev);
+#ifdef FORCEDETH_RECV_CACHE
 		nv_destroy_recv_cache(dev);
 		nv_init_recv_cache(dev);
+#endif
 		if (nv_init_ring(dev)) {
 			if (!np->in_shutdown)
 				mod_timer(&np->oom_kick, jiffies + OOM_REFILL);
@@ -5495,9 +5518,11 @@ static int nv_open(struct net_device *dev)
 
 	/* initialize descriptor rings */
 	set_bufsize(dev);
+#ifdef FORCEDETH_RECV_CACHE
 	nv_init_recv_cache(dev);
 
 	INIT_DELAYED_WORK(&np->recv_cache_work, nv_recv_cache_worker);
+#endif
 	oom = nv_init_ring(dev);
 
 	writel(0, base + NvRegLinkSpeed);
@@ -5679,9 +5704,10 @@ static int nv_close(struct net_device *dev)
 		nv_txrx_gate(dev, true);
 	}
 
+#ifdef FORCEDETH_RECV_CACHE
 	/* free all SKBs in recv cache */
 	nv_destroy_recv_cache(dev);
-
+#endif
 	/* FIXME: power down nic */
 
 	return 0;
-- 
2.7.4


^ permalink raw reply related

* [PATCH] net: hns3: typo in the name of a constant
From: Christophe JAILLET @ 2019-07-21 13:08 UTC (permalink / raw)
  To: yisen.zhuang, salil.mehta, davem, tanhuazhong, lipeng321,
	shenjian15, liuzhongzhu
  Cc: netdev, linux-kernel, kernel-janitors, Christophe JAILLET

All constant in 'enum HCLGE_MBX_OPCODE' start with HCLGE, except
'HLCGE_MBX_PUSH_VLAN_INFO' (C and L switched)

s/HLC/HCL/

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
 drivers/net/ethernet/hisilicon/hns3/hclge_mbx.h          | 2 +-
 drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mbx.c   | 2 +-
 drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_mbx.c | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hclge_mbx.h b/drivers/net/ethernet/hisilicon/hns3/hclge_mbx.h
index 8ad5292eebbe..75329ab775a6 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hclge_mbx.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hclge_mbx.h
@@ -43,7 +43,7 @@ enum HCLGE_MBX_OPCODE {
 	HCLGE_MBX_GET_QID_IN_PF,	/* (VF -> PF) get queue id in pf */
 	HCLGE_MBX_LINK_STAT_MODE,	/* (PF -> VF) link mode has changed */
 	HCLGE_MBX_GET_LINK_MODE,	/* (VF -> PF) get the link mode of pf */
-	HLCGE_MBX_PUSH_VLAN_INFO,	/* (PF -> VF) push port base vlan */
+	HCLGE_MBX_PUSH_VLAN_INFO,	/* (PF -> VF) push port base vlan */
 	HCLGE_MBX_GET_MEDIA_TYPE,       /* (VF -> PF) get media type */
 
 	HCLGE_MBX_GET_VF_FLR_STATUS = 200, /* (M7 -> PF) get vf reset status */
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mbx.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mbx.c
index 64578e96b2e2..d1f5ccee79d3 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mbx.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mbx.c
@@ -300,7 +300,7 @@ int hclge_push_vf_port_base_vlan_info(struct hclge_vport *vport, u8 vfid,
 	memcpy(&msg_data[6], &vlan_tag, sizeof(u16));
 
 	return hclge_send_mbx_msg(vport, msg_data, sizeof(msg_data),
-				  HLCGE_MBX_PUSH_VLAN_INFO, vfid);
+				  HCLGE_MBX_PUSH_VLAN_INFO, vfid);
 }
 
 static int hclge_set_vf_vlan_cfg(struct hclge_vport *vport,
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_mbx.c b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_mbx.c
index 30f2e9352cf3..2bf6ace289ff 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_mbx.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_mbx.c
@@ -203,7 +203,7 @@ void hclgevf_mbx_handler(struct hclgevf_dev *hdev)
 		case HCLGE_MBX_LINK_STAT_CHANGE:
 		case HCLGE_MBX_ASSERTING_RESET:
 		case HCLGE_MBX_LINK_STAT_MODE:
-		case HLCGE_MBX_PUSH_VLAN_INFO:
+		case HCLGE_MBX_PUSH_VLAN_INFO:
 			/* set this mbx event as pending. This is required as we
 			 * might loose interrupt event when mbx task is busy
 			 * handling. This shall be cleared when mbx task just
@@ -306,7 +306,7 @@ void hclgevf_mbx_async_handler(struct hclgevf_dev *hdev)
 			hclgevf_reset_task_schedule(hdev);
 
 			break;
-		case HLCGE_MBX_PUSH_VLAN_INFO:
+		case HCLGE_MBX_PUSH_VLAN_INFO:
 			state = le16_to_cpu(msg_q[1]);
 			vlan_info = &msg_q[1];
 			hclgevf_update_port_base_vlan_info(hdev, state,
-- 
2.20.1


^ permalink raw reply related

* [PATCH] chelsio: Fix a typo in a function name
From: Christophe JAILLET @ 2019-07-21 13:16 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux-kernel, kernel-janitors, Christophe JAILLET

It is likely that 'my3216_poll()' should be 'my3126_poll()'. (1 and 2
switched in 3126.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
 drivers/net/ethernet/chelsio/cxgb/my3126.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/chelsio/cxgb/my3126.c b/drivers/net/ethernet/chelsio/cxgb/my3126.c
index 20c09cc4b323..60aa45b375b6 100644
--- a/drivers/net/ethernet/chelsio/cxgb/my3126.c
+++ b/drivers/net/ethernet/chelsio/cxgb/my3126.c
@@ -94,7 +94,7 @@ static int my3126_interrupt_handler(struct cphy *cphy)
 	return cphy_cause_link_change;
 }
 
-static void my3216_poll(struct work_struct *work)
+static void my3126_poll(struct work_struct *work)
 {
 	struct cphy *cphy = container_of(work, struct cphy, phy_update.work);
 
@@ -177,7 +177,7 @@ static struct cphy *my3126_phy_create(struct net_device *dev,
 		return NULL;
 
 	cphy_init(cphy, dev, phy_addr, &my3126_ops, mdio_ops);
-	INIT_DELAYED_WORK(&cphy->phy_update, my3216_poll);
+	INIT_DELAYED_WORK(&cphy->phy_update, my3126_poll);
 	cphy->bmsr = 0;
 
 	return cphy;
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH net v3] net: neigh: fix multiple neigh timer scheduling
From: Julian Anastasov @ 2019-07-21 13:46 UTC (permalink / raw)
  To: Lorenzo Bianconi; +Cc: davem, netdev, dsahern, marek
In-Reply-To: <552d7c8de6a07e12f7b76791da953e81478138cd.1563134704.git.lorenzo.bianconi@redhat.com>


	Hello,

On Sun, 14 Jul 2019, Lorenzo Bianconi wrote:

> Neigh timer can be scheduled multiple times from userspace adding

	If the garbage comes from ndm_state, why we should create
a patch that just covers the problem?:

State: INCOMPLETE, STALE, FAILED, 0x8400 (0x8425)

	User space is trying to create entry that is both
STALE (no timer) and INCOMPLETE (with timer). So, in the
2nd NL message __neigh_event_send() detects timer with NUD_STALE
bit. What if this 2nd message never comes? Such inconsistence
between nud_state and the timer can trigger other bugs in
other functions.

	May be we just need to restrict ndm_state and to drop
this patch, for example, by adding checks in __neigh_update():

        if (!(flags & NEIGH_UPDATE_F_ADMIN) &&
            (old & (NUD_NOARP | NUD_PERMANENT)))
                goto out;
+	/* State must be single bit or 0 */
+	if (new & (new - 1))
+		goto out;
        if (neigh->dead) {

	If needed, this check can be moved only for ndm_state
in neigh_add().

> multiple neigh entries and forcing the neigh timer scheduling passing
> NTF_USE in the netlink requests.
> This will result in a refcount leak and in the following dump stack:

	It is a single create with multiple bits in state with following
__neigh_event_send(). And who knows, this bug may exist even in Linux 2.2 
and below...

> [   32.465295] NEIGH: BUG, double timer add, state is 8
> [   32.465308] CPU: 0 PID: 416 Comm: double_timer_ad Not tainted 5.2.0+ #65
> [   32.465311] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.12.0-2.fc30 04/01/2014
> [   32.465313] Call Trace:
> [   32.465318]  dump_stack+0x7c/0xc0
> [   32.465323]  __neigh_event_send+0x20c/0x880
> [   32.465326]  ? ___neigh_create+0x846/0xfb0
> [   32.465329]  ? neigh_lookup+0x2a9/0x410
> [   32.465332]  ? neightbl_fill_info.constprop.0+0x800/0x800
> [   32.465334]  neigh_add+0x4f8/0x5e0
> [   32.465337]  ? neigh_xmit+0x620/0x620
> [   32.465341]  ? find_held_lock+0x85/0xa0
> [   32.465345]  rtnetlink_rcv_msg+0x204/0x570
> [   32.465348]  ? rtnl_dellink+0x450/0x450
> [   32.465351]  ? mark_held_locks+0x90/0x90
> [   32.465354]  ? match_held_lock+0x1b/0x230
> [   32.465357]  netlink_rcv_skb+0xc4/0x1d0
> [   32.465360]  ? rtnl_dellink+0x450/0x450
> [   32.465363]  ? netlink_ack+0x420/0x420
> [   32.465366]  ? netlink_deliver_tap+0x115/0x560
> [   32.465369]  ? __alloc_skb+0xc9/0x2f0
> [   32.465372]  netlink_unicast+0x270/0x330
> [   32.465375]  ? netlink_attachskb+0x2f0/0x2f0
> [   32.465378]  netlink_sendmsg+0x34f/0x5a0
> [   32.465381]  ? netlink_unicast+0x330/0x330
> [   32.465385]  ? move_addr_to_kernel.part.0+0x20/0x20
> [   32.465388]  ? netlink_unicast+0x330/0x330
> [   32.465391]  sock_sendmsg+0x91/0xa0
> [   32.465394]  ___sys_sendmsg+0x407/0x480
> [   32.465397]  ? copy_msghdr_from_user+0x200/0x200
> [   32.465401]  ? _raw_spin_unlock_irqrestore+0x37/0x40
> [   32.465404]  ? lockdep_hardirqs_on+0x17d/0x250
> [   32.465407]  ? __wake_up_common_lock+0xcb/0x110
> [   32.465410]  ? __wake_up_common+0x230/0x230
> [   32.465413]  ? netlink_bind+0x3e1/0x490
> [   32.465416]  ? netlink_setsockopt+0x540/0x540
> [   32.465420]  ? __fget_light+0x9c/0xf0
> [   32.465423]  ? sockfd_lookup_light+0x8c/0xb0
> [   32.465426]  __sys_sendmsg+0xa5/0x110
> [   32.465429]  ? __ia32_sys_shutdown+0x30/0x30
> [   32.465432]  ? __fd_install+0xe1/0x2c0
> [   32.465435]  ? lockdep_hardirqs_off+0xb5/0x100
> [   32.465438]  ? mark_held_locks+0x24/0x90
> [   32.465441]  ? do_syscall_64+0xf/0x270
> [   32.465444]  do_syscall_64+0x63/0x270
> [   32.465448]  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> 
> Fix the issue unscheduling neigh_timer if selected entry is in 'IN_TIMER'
> receiving a netlink request with NTF_USE flag set
> 
> Reported-by: Marek Majkowski <marek@cloudflare.com>
> Fixes: 0c5c2d308906 ("neigh: Allow for user space users of the neighbour table")
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
> ---
> Changes since v2:
> - remove check_timer flag and run neigh_del_timer directly
> Changes since v1:
> - fix compilation errors defining neigh_event_send_check_timer routine
> ---
>  net/core/neighbour.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index 742cea4ce72e..0dfc97bc8760 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -1124,6 +1124,7 @@ int __neigh_event_send(struct neighbour *neigh, struct sk_buff *skb)
>  
>  			atomic_set(&neigh->probes,
>  				   NEIGH_VAR(neigh->parms, UCAST_PROBES));
> +			neigh_del_timer(neigh);
>  			neigh->nud_state     = NUD_INCOMPLETE;
>  			neigh->updated = now;
>  			next = now + max(NEIGH_VAR(neigh->parms, RETRANS_TIME),
> @@ -1140,6 +1141,7 @@ int __neigh_event_send(struct neighbour *neigh, struct sk_buff *skb)
>  		}
>  	} else if (neigh->nud_state & NUD_STALE) {
>  		neigh_dbg(2, "neigh %p is delayed\n", neigh);
> +		neigh_del_timer(neigh);
>  		neigh->nud_state = NUD_DELAY;
>  		neigh->updated = jiffies;
>  		neigh_add_timer(neigh, jiffies +
> -- 
> 2.21.0

Regards

--
Julian Anastasov <ja@ssi.bg>

^ permalink raw reply

* [PATCH net-next] net: sched: verify that q!=NULL before setting q->flags
From: Vlad Buslov @ 2019-07-21 14:44 UTC (permalink / raw)
  To: netdev; +Cc: jhs, xiyou.wangcong, jiri, davem, Vlad Buslov

In function int tc_new_tfilter() q pointer can be NULL when adding filter
on a shared block. With recent change that resets TCQ_F_CAN_BYPASS after
filter creation, following NULL pointer dereference happens in case parent
block is shared:

[  212.925060] BUG: kernel NULL pointer dereference, address: 0000000000000010
[  212.925445] #PF: supervisor write access in kernel mode
[  212.925709] #PF: error_code(0x0002) - not-present page
[  212.925965] PGD 8000000827923067 P4D 8000000827923067 PUD 827924067 PMD 0
[  212.926302] Oops: 0002 [#1] SMP KASAN PTI
[  212.926539] CPU: 18 PID: 2617 Comm: tc Tainted: G    B             5.2.0+ #512
[  212.926938] Hardware name: Supermicro SYS-2028TP-DECR/X10DRT-P, BIOS 2.0b 03/30/2017
[  212.927364] RIP: 0010:tc_new_tfilter+0x698/0xd40
[  212.927633] Code: 74 0d 48 85 c0 74 08 48 89 ef e8 03 aa 62 00 48 8b 84 24 a0 00 00 00 48 8d 78 10 48 89 44 24 18 e8 4d 0c 6b ff 48 8b 44 24 18 <83> 60 10 f
b 48 85 ed 0f 85 3d fe ff ff e9 4f fe ff ff e8 81 26 f8
[  212.928607] RSP: 0018:ffff88884fd5f5d8 EFLAGS: 00010296
[  212.928905] RAX: 0000000000000000 RBX: 0000000000000000 RCX: dffffc0000000000
[  212.929201] RDX: 0000000000000007 RSI: 0000000000000004 RDI: 0000000000000297
[  212.929402] RBP: ffff88886bedd600 R08: ffffffffb91d4b51 R09: fffffbfff7616e4d
[  212.929609] R10: fffffbfff7616e4c R11: ffffffffbb0b7263 R12: ffff88886bc61040
[  212.929803] R13: ffff88884fd5f950 R14: ffffc900039c5000 R15: ffff88835e927680
[  212.929999] FS:  00007fe7c50b6480(0000) GS:ffff88886f980000(0000) knlGS:0000000000000000
[  212.930235] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  212.930394] CR2: 0000000000000010 CR3: 000000085bd04002 CR4: 00000000001606e0
[  212.930588] Call Trace:
[  212.930682]  ? tc_del_tfilter+0xa40/0xa40
[  212.930811]  ? __lock_acquire+0x5b5/0x2460
[  212.930948]  ? find_held_lock+0x85/0xa0
[  212.931081]  ? tc_del_tfilter+0xa40/0xa40
[  212.931201]  rtnetlink_rcv_msg+0x4ab/0x5f0
[  212.931332]  ? rtnl_dellink+0x490/0x490
[  212.931454]  ? lockdep_hardirqs_on+0x260/0x260
[  212.931589]  ? netlink_deliver_tap+0xab/0x5a0
[  212.931717]  ? match_held_lock+0x1b/0x240
[  212.931844]  netlink_rcv_skb+0xd0/0x200
[  212.931958]  ? rtnl_dellink+0x490/0x490
[  212.932079]  ? netlink_ack+0x440/0x440
[  212.932205]  ? netlink_deliver_tap+0x161/0x5a0
[  212.932335]  ? lock_downgrade+0x360/0x360
[  212.932457]  ? lock_acquire+0xe5/0x210
[  212.932579]  netlink_unicast+0x296/0x350
[  212.932705]  ? netlink_attachskb+0x390/0x390
[  212.932834]  ? _copy_from_iter_full+0xe0/0x3a0
[  212.932976]  netlink_sendmsg+0x394/0x600
[  212.937998]  ? netlink_unicast+0x350/0x350
[  212.943033]  ? move_addr_to_kernel.part.0+0x90/0x90
[  212.948115]  ? netlink_unicast+0x350/0x350
[  212.953185]  sock_sendmsg+0x96/0xa0
[  212.958099]  ___sys_sendmsg+0x482/0x520
[  212.962881]  ? match_held_lock+0x1b/0x240
[  212.967618]  ? copy_msghdr_from_user+0x250/0x250
[  212.972337]  ? lock_downgrade+0x360/0x360
[  212.976973]  ? rwlock_bug.part.0+0x60/0x60
[  212.981548]  ? __mod_node_page_state+0x1f/0xa0
[  212.986060]  ? match_held_lock+0x1b/0x240
[  212.990567]  ? find_held_lock+0x85/0xa0
[  212.994989]  ? do_user_addr_fault+0x349/0x5b0
[  212.999387]  ? lock_downgrade+0x360/0x360
[  213.003713]  ? find_held_lock+0x85/0xa0
[  213.007972]  ? __fget_light+0xa1/0xf0
[  213.012143]  ? sockfd_lookup_light+0x91/0xb0
[  213.016165]  __sys_sendmsg+0xba/0x130
[  213.020040]  ? __sys_sendmsg_sock+0xb0/0xb0
[  213.023870]  ? handle_mm_fault+0x337/0x470
[  213.027592]  ? page_fault+0x8/0x30
[  213.031316]  ? lockdep_hardirqs_off+0xbe/0x100
[  213.034999]  ? mark_held_locks+0x24/0x90
[  213.038671]  ? do_syscall_64+0x1e/0xe0
[  213.042297]  do_syscall_64+0x74/0xe0
[  213.045828]  entry_SYSCALL_64_after_hwframe+0x49/0xbe
[  213.049354] RIP: 0033:0x7fe7c527c7b8
[  213.052792] Code: 89 02 48 c7 c0 ff ff ff ff eb bb 0f 1f 80 00 00 00 00 f3 0f 1e fa 48 8d 05 65 8f 0c 00 8b 00 85 c0 75 17 b8 2e 00 00 00 0f 05 <48> 3d 00 f
0 ff ff 77 58 c3 0f 1f 80 00 00 00 00 48 83 ec 28 89 54
[  213.060269] RSP: 002b:00007ffc3f7908a8 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
[  213.064144] RAX: ffffffffffffffda RBX: 000000005d34716f RCX: 00007fe7c527c7b8
[  213.068094] RDX: 0000000000000000 RSI: 00007ffc3f790910 RDI: 0000000000000003
[  213.072109] RBP: 0000000000000000 R08: 0000000000000001 R09: 00007fe7c5340cc0
[  213.076113] R10: 0000000000404ec2 R11: 0000000000000246 R12: 0000000000000080
[  213.080146] R13: 0000000000480640 R14: 0000000000000080 R15: 0000000000000000
[  213.084147] Modules linked in: act_gact cls_flower sch_ingress nfsv3 nfs_acl nfs lockd grace fscache bridge stp llc sunrpc intel_rapl_msr intel_rapl_common
^[[<1;69;32Msb_edac rdma_ucm rdma_cm x86_pkg_temp_thermal iw_cm intel_powerclamp ib_cm coretemp kvm_intel kvm irqbypass mlx5_ib ib_uverbs ib_core crct10dif_pclmul crc32_pc
lmul crc32c_intel ghash_clmulni_intel mlx5_core intel_cstate intel_uncore iTCO_wdt igb iTCO_vendor_support mlxfw mei_me ptp ses intel_rapl_perf mei pcspkr ipmi
_ssif i2c_i801 joydev enclosure pps_core lpc_ich ioatdma wmi dca ipmi_si ipmi_devintf ipmi_msghandler acpi_power_meter acpi_pad ast i2c_algo_bit drm_vram_helpe
r ttm drm_kms_helper drm mpt3sas raid_class scsi_transport_sas
[  213.112326] CR2: 0000000000000010
[  213.117429] ---[ end trace adb58eb0a4ee6283 ]---

Verify that q pointer is not NULL before setting the 'flags' field.

Fixes: 3f05e6886a59 ("net_sched: unset TCQ_F_CAN_BYPASS when adding filters")
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
 net/sched/cls_api.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index d144233423c5..0c5660bd0331 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -2152,7 +2152,9 @@ static int tc_new_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
 		tfilter_notify(net, skb, n, tp, block, q, parent, fh,
 			       RTM_NEWTFILTER, false, rtnl_held);
 		tfilter_put(tp, fh);
-		q->flags &= ~TCQ_F_CAN_BYPASS;
+		/* q pointer is NULL for shared blocks */
+		if (q)
+			q->flags &= ~TCQ_F_CAN_BYPASS;
 	}
 
 errout:
-- 
2.21.0


^ permalink raw reply related

* Re: [PATCHv2 2/2] forcedeth: disable recv cache by default
From: Andrew Lunn @ 2019-07-21 14:48 UTC (permalink / raw)
  To: Zhu Yanjun; +Cc: davem, netdev
In-Reply-To: <1563713633-25528-3-git-send-email-yanjun.zhu@oracle.com>

On Sun, Jul 21, 2019 at 08:53:53AM -0400, Zhu Yanjun wrote:
> The recv cache is to allocate 125MiB memory to reserve for NIC.
> In the past time, this recv cache works very well. When the memory
> is not enough, this recv cache reserves memory for NIC.
> And the communications through this NIC is not affected by the
> memory shortage. And the performance of NIC is better because of
> this recv cache.
> But this recv cache reserves 125MiB memory for one NIC port. Normally
> there are 2 NIC ports in one card. So in a host, there are about 250
> MiB memory reserved for NIC ports. To a host on which communications
> are not mandatory, it is not necessary to reserve memory.
> So this recv cache is disabled by default.
> 
> CC: Joe Jin <joe.jin@oracle.com>
> CC: Junxiao Bi <junxiao.bi@oracle.com>
> Tested-by: Nan san <nan.1986san@gmail.com>
> Signed-off-by: Zhu Yanjun <yanjun.zhu@oracle.com>
> ---
>  drivers/net/ethernet/nvidia/Kconfig     | 11 ++++++++
>  drivers/net/ethernet/nvidia/Makefile    |  1 +
>  drivers/net/ethernet/nvidia/forcedeth.c | 46 ++++++++++++++++++++++++++-------
>  3 files changed, 48 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/ethernet/nvidia/Kconfig b/drivers/net/ethernet/nvidia/Kconfig
> index faacbd1..9a9f42a 100644
> --- a/drivers/net/ethernet/nvidia/Kconfig
> +++ b/drivers/net/ethernet/nvidia/Kconfig
> @@ -26,4 +26,15 @@ config FORCEDETH
>  	  To compile this driver as a module, choose M here. The module
>  	  will be called forcedeth.
>  
> +config	FORCEDETH_RECV_CACHE
> +	bool "nForce Ethernet recv cache support"
> +	depends on FORCEDETH
> +	default n
> +	---help---
> +	  The recv cache can make nic work steadily when the system memory is
> +	  not enough. And it can also enhance nic performance. But to a host
> +	  on which the communications are not mandatory, it is not necessary
> +	  to reserve 125MiB memory for NIC.
> +	  So recv cache is disabled by default.
> +
>  endif # NET_VENDOR_NVIDIA
> diff --git a/drivers/net/ethernet/nvidia/Makefile b/drivers/net/ethernet/nvidia/Makefile
> index 8935699..40c055e 100644
> --- a/drivers/net/ethernet/nvidia/Makefile
> +++ b/drivers/net/ethernet/nvidia/Makefile
> @@ -4,3 +4,4 @@
>  #
>  
>  obj-$(CONFIG_FORCEDETH) += forcedeth.o
> +ccflags-$(CONFIG_FORCEDETH_RECV_CACHE)	:=	-DFORCEDETH_RECV_CACHE
> diff --git a/drivers/net/ethernet/nvidia/forcedeth.c b/drivers/net/ethernet/nvidia/forcedeth.c
> index f8e766f..deda276 100644
> --- a/drivers/net/ethernet/nvidia/forcedeth.c
> +++ b/drivers/net/ethernet/nvidia/forcedeth.c
> @@ -674,10 +674,12 @@ struct nv_ethtool_stats {
>  	u64 tx_broadcast;
>  };
>  
> +#ifdef FORCEDETH_RECV_CACHE
>  /* 1000Mb is 125M bytes, 125 * 1024 * 1024 bytes
>   * The length of recv cache is 125M / skb_length
>   */
>  #define RECV_CACHE_LIST_LENGTH		(125 * 1024 * 1024 / np->rx_buf_sz)
> +#endif
>  
>  #define NV_DEV_STATISTICS_V3_COUNT (sizeof(struct nv_ethtool_stats)/sizeof(u64))
>  #define NV_DEV_STATISTICS_V2_COUNT (NV_DEV_STATISTICS_V3_COUNT - 3)
> @@ -850,10 +852,12 @@ struct fe_priv {
>  	char name_tx[IFNAMSIZ + 3];       /* -tx    */
>  	char name_other[IFNAMSIZ + 6];    /* -other */
>  
> +#ifdef FORCEDETH_RECV_CACHE
>  	/* This is to schedule work */
>  	struct delayed_work     recv_cache_work;
>  	/* This list is to store skb queue for recv */
>  	struct sk_buff_head recv_list;
> +#endif
>  };
>  
>  /*
> @@ -1814,8 +1818,12 @@ static int nv_alloc_rx(struct net_device *dev)
>  		less_rx = np->last_rx.orig;
>  
>  	while (np->put_rx.orig != less_rx) {
> +#ifdef FORCEDETH_RECV_CACHE
>  		struct sk_buff *skb = skb_dequeue(&np->recv_list);
> -
> +#else
> +		struct sk_buff *skb = netdev_alloc_skb(np->dev,
> +					 np->rx_buf_sz + NV_RX_ALLOC_PAD);
> +#endif
>  		if (likely(skb)) {
>  			np->put_rx_ctx->skb = skb;
>  			np->put_rx_ctx->dma = dma_map_single(&np->pci_dev->dev,
> @@ -1840,15 +1848,15 @@ static int nv_alloc_rx(struct net_device *dev)
>  			u64_stats_update_begin(&np->swstats_rx_syncp);
>  			np->stat_rx_dropped++;
>  			u64_stats_update_end(&np->swstats_rx_syncp);
> -
> +#ifdef FORCEDETH_RECV_CACHE
>  			schedule_delayed_work(&np->recv_cache_work, 0);
> -
> +#endif

All these #ifdef are pretty ugly. It also makes for easy to break code
since most of the time this option will not be enabled. Please
refactor the code so that is uses

if (IS_ENABLED(FORCEDETH_RECV_CACHE))

so that the compiler at least compiles the code every time, and then
optimizing it out.

	   Andrew

^ permalink raw reply

* Re: [PATCHv2 0/2] forcedeth: recv cache to make NIC work steadily
From: Andrew Lunn @ 2019-07-21 14:53 UTC (permalink / raw)
  To: Zhu Yanjun; +Cc: davem, netdev
In-Reply-To: <1563713633-25528-1-git-send-email-yanjun.zhu@oracle.com>

On Sun, Jul 21, 2019 at 08:53:51AM -0400, Zhu Yanjun wrote:
> These patches are to this scenario:
> 
> "
> When the host run for long time, there are a lot of memory fragments in
> the hosts. And it is possible that kernel will compact memory fragments.
> But normally it is difficult for NIC driver to allocate a memory from
> kernel. From this variable stat_rx_dropped, we can confirm that NIC driver
> can not allocate skb very frequently.
> "
> 
> Since NIC driver can not allocate skb in time, this makes some important
> tasks not be completed in time.
> To avoid it, a recv cache is created to pre-allocate skb for NIC driver.
> This can make the important tasks be completed in time.
> >From Nan's tests in LAB, these patches can make NIC driver work steadily.
> Now in production hosts, these patches are applied.
> 
> With these patches, one NIC port needs 125MiB reserved. This 125MiB memory
> can not be used by others. To a host on which the communications are not
> mandatory, it is not necessary to reserve so much memory. So this recv cache
> is disabled by default.
> 
> V1->V2:
> 1. ndelay is replaced with GFP_KERNEL function __netdev_alloc_skb.
> 2. skb_queue_purge is used when recv cache is destroyed.
> 3. RECV_LIST_ALLOCATE bit is removed.
> 4. schedule_delayed_work is moved out of while loop.

Hi Zhu

You don't appear to of address David's comment that this is probably
the wrong way to do this, it should be a generic solution.

Also, that there should be enough atomic memory in the system
anyway. Have you looked at what other drivers are using atomic memory?
It could actually be you need to debug some other driver, rather than
add hacks to forcedeth.

    Andrew

^ permalink raw reply

* Re: [PATCH net-next 00/12] mlx5 TLS TX HW offload support
From: Tariq Toukan @ 2019-07-21 15:19 UTC (permalink / raw)
  To: David Miller, jakub.kicinski@netronome.com
  Cc: Tariq Toukan, netdev@vger.kernel.org, Eran Ben Elisha,
	Saeed Mahameed, Moshe Shemesh
In-Reply-To: <20190718.120910.1323935732125670131.davem@davemloft.net>



On 7/18/2019 10:09 PM, David Miller wrote:
> From: Jakub Kicinski <jakub.kicinski@netronome.com>
> Date: Thu, 18 Jul 2019 10:08:47 -0700
> 
>> Yes, certainly. It's documentation and renaming a stat before it makes
>> it into an official release.
> 
> Agreed.
> 

Ack.
I'll prepare and send this week.

Tariq

^ permalink raw reply

* Re: [PATCH net-next] net: sched: verify that q!=NULL before setting q->flags
From: Jiri Pirko @ 2019-07-21 15:24 UTC (permalink / raw)
  To: Vlad Buslov; +Cc: netdev, jhs, xiyou.wangcong, davem
In-Reply-To: <20190721144412.2783-1-vladbu@mellanox.com>

Sun, Jul 21, 2019 at 04:44:12PM CEST, vladbu@mellanox.com wrote:
>In function int tc_new_tfilter() q pointer can be NULL when adding filter
>on a shared block. With recent change that resets TCQ_F_CAN_BYPASS after
>filter creation, following NULL pointer dereference happens in case parent
>block is shared:
>
>[  212.925060] BUG: kernel NULL pointer dereference, address: 0000000000000010
>[  212.925445] #PF: supervisor write access in kernel mode
>[  212.925709] #PF: error_code(0x0002) - not-present page
>[  212.925965] PGD 8000000827923067 P4D 8000000827923067 PUD 827924067 PMD 0
>[  212.926302] Oops: 0002 [#1] SMP KASAN PTI
>[  212.926539] CPU: 18 PID: 2617 Comm: tc Tainted: G    B             5.2.0+ #512
>[  212.926938] Hardware name: Supermicro SYS-2028TP-DECR/X10DRT-P, BIOS 2.0b 03/30/2017
>[  212.927364] RIP: 0010:tc_new_tfilter+0x698/0xd40
>[  212.927633] Code: 74 0d 48 85 c0 74 08 48 89 ef e8 03 aa 62 00 48 8b 84 24 a0 00 00 00 48 8d 78 10 48 89 44 24 18 e8 4d 0c 6b ff 48 8b 44 24 18 <83> 60 10 f
>b 48 85 ed 0f 85 3d fe ff ff e9 4f fe ff ff e8 81 26 f8
>[  212.928607] RSP: 0018:ffff88884fd5f5d8 EFLAGS: 00010296
>[  212.928905] RAX: 0000000000000000 RBX: 0000000000000000 RCX: dffffc0000000000
>[  212.929201] RDX: 0000000000000007 RSI: 0000000000000004 RDI: 0000000000000297
>[  212.929402] RBP: ffff88886bedd600 R08: ffffffffb91d4b51 R09: fffffbfff7616e4d
>[  212.929609] R10: fffffbfff7616e4c R11: ffffffffbb0b7263 R12: ffff88886bc61040
>[  212.929803] R13: ffff88884fd5f950 R14: ffffc900039c5000 R15: ffff88835e927680
>[  212.929999] FS:  00007fe7c50b6480(0000) GS:ffff88886f980000(0000) knlGS:0000000000000000
>[  212.930235] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>[  212.930394] CR2: 0000000000000010 CR3: 000000085bd04002 CR4: 00000000001606e0
>[  212.930588] Call Trace:
>[  212.930682]  ? tc_del_tfilter+0xa40/0xa40
>[  212.930811]  ? __lock_acquire+0x5b5/0x2460
>[  212.930948]  ? find_held_lock+0x85/0xa0
>[  212.931081]  ? tc_del_tfilter+0xa40/0xa40
>[  212.931201]  rtnetlink_rcv_msg+0x4ab/0x5f0
>[  212.931332]  ? rtnl_dellink+0x490/0x490
>[  212.931454]  ? lockdep_hardirqs_on+0x260/0x260
>[  212.931589]  ? netlink_deliver_tap+0xab/0x5a0
>[  212.931717]  ? match_held_lock+0x1b/0x240
>[  212.931844]  netlink_rcv_skb+0xd0/0x200
>[  212.931958]  ? rtnl_dellink+0x490/0x490
>[  212.932079]  ? netlink_ack+0x440/0x440
>[  212.932205]  ? netlink_deliver_tap+0x161/0x5a0
>[  212.932335]  ? lock_downgrade+0x360/0x360
>[  212.932457]  ? lock_acquire+0xe5/0x210
>[  212.932579]  netlink_unicast+0x296/0x350
>[  212.932705]  ? netlink_attachskb+0x390/0x390
>[  212.932834]  ? _copy_from_iter_full+0xe0/0x3a0
>[  212.932976]  netlink_sendmsg+0x394/0x600
>[  212.937998]  ? netlink_unicast+0x350/0x350
>[  212.943033]  ? move_addr_to_kernel.part.0+0x90/0x90
>[  212.948115]  ? netlink_unicast+0x350/0x350
>[  212.953185]  sock_sendmsg+0x96/0xa0
>[  212.958099]  ___sys_sendmsg+0x482/0x520
>[  212.962881]  ? match_held_lock+0x1b/0x240
>[  212.967618]  ? copy_msghdr_from_user+0x250/0x250
>[  212.972337]  ? lock_downgrade+0x360/0x360
>[  212.976973]  ? rwlock_bug.part.0+0x60/0x60
>[  212.981548]  ? __mod_node_page_state+0x1f/0xa0
>[  212.986060]  ? match_held_lock+0x1b/0x240
>[  212.990567]  ? find_held_lock+0x85/0xa0
>[  212.994989]  ? do_user_addr_fault+0x349/0x5b0
>[  212.999387]  ? lock_downgrade+0x360/0x360
>[  213.003713]  ? find_held_lock+0x85/0xa0
>[  213.007972]  ? __fget_light+0xa1/0xf0
>[  213.012143]  ? sockfd_lookup_light+0x91/0xb0
>[  213.016165]  __sys_sendmsg+0xba/0x130
>[  213.020040]  ? __sys_sendmsg_sock+0xb0/0xb0
>[  213.023870]  ? handle_mm_fault+0x337/0x470
>[  213.027592]  ? page_fault+0x8/0x30
>[  213.031316]  ? lockdep_hardirqs_off+0xbe/0x100
>[  213.034999]  ? mark_held_locks+0x24/0x90
>[  213.038671]  ? do_syscall_64+0x1e/0xe0
>[  213.042297]  do_syscall_64+0x74/0xe0
>[  213.045828]  entry_SYSCALL_64_after_hwframe+0x49/0xbe
>[  213.049354] RIP: 0033:0x7fe7c527c7b8
>[  213.052792] Code: 89 02 48 c7 c0 ff ff ff ff eb bb 0f 1f 80 00 00 00 00 f3 0f 1e fa 48 8d 05 65 8f 0c 00 8b 00 85 c0 75 17 b8 2e 00 00 00 0f 05 <48> 3d 00 f
>0 ff ff 77 58 c3 0f 1f 80 00 00 00 00 48 83 ec 28 89 54
>[  213.060269] RSP: 002b:00007ffc3f7908a8 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
>[  213.064144] RAX: ffffffffffffffda RBX: 000000005d34716f RCX: 00007fe7c527c7b8
>[  213.068094] RDX: 0000000000000000 RSI: 00007ffc3f790910 RDI: 0000000000000003
>[  213.072109] RBP: 0000000000000000 R08: 0000000000000001 R09: 00007fe7c5340cc0
>[  213.076113] R10: 0000000000404ec2 R11: 0000000000000246 R12: 0000000000000080
>[  213.080146] R13: 0000000000480640 R14: 0000000000000080 R15: 0000000000000000
>[  213.084147] Modules linked in: act_gact cls_flower sch_ingress nfsv3 nfs_acl nfs lockd grace fscache bridge stp llc sunrpc intel_rapl_msr intel_rapl_common
>^[[<1;69;32Msb_edac rdma_ucm rdma_cm x86_pkg_temp_thermal iw_cm intel_powerclamp ib_cm coretemp kvm_intel kvm irqbypass mlx5_ib ib_uverbs ib_core crct10dif_pclmul crc32_pc
>lmul crc32c_intel ghash_clmulni_intel mlx5_core intel_cstate intel_uncore iTCO_wdt igb iTCO_vendor_support mlxfw mei_me ptp ses intel_rapl_perf mei pcspkr ipmi
>_ssif i2c_i801 joydev enclosure pps_core lpc_ich ioatdma wmi dca ipmi_si ipmi_devintf ipmi_msghandler acpi_power_meter acpi_pad ast i2c_algo_bit drm_vram_helpe
>r ttm drm_kms_helper drm mpt3sas raid_class scsi_transport_sas
>[  213.112326] CR2: 0000000000000010
>[  213.117429] ---[ end trace adb58eb0a4ee6283 ]---
>
>Verify that q pointer is not NULL before setting the 'flags' field.
>
>Fixes: 3f05e6886a59 ("net_sched: unset TCQ_F_CAN_BYPASS when adding filters")
>Signed-off-by: Vlad Buslov <vladbu@mellanox.com>

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

Thanks!

^ 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