* net/udp: bug in skb_pull_rcsum
From: Andrey Konovalov @ 2016-11-22 11:58 UTC (permalink / raw)
To: samanthakumar, David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev, LKML
Cc: Dmitry Vyukov, Kostya Serebryany, Eric Dumazet, syzkaller
[-- Attachment #1: Type: text/plain, Size: 2944 bytes --]
Hi,
I've got the following error report while fuzzing the kernel with syzkaller.
A reproducer is attached.
On commit 9c763584b7c8911106bb77af7e648bef09af9d80 (4.9-rc6, Nov 20).
------------[ cut here ]------------
kernel BUG at net/core/skbuff.c:3029!
invalid opcode: 0000 [#1] SMP KASAN
Modules linked in:
CPU: 1 PID: 3854 Comm: a.out Not tainted 4.9.0-rc6+ #431
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
task: ffff880068472c00 task.stack: ffff880063ec8000
RIP: 0010:[<ffffffff82b8fd85>] [<ffffffff82b8fd85>]
skb_pull_rcsum+0x255/0x350 net/core/skbuff.c:3029
RSP: 0018:ffff880063ecf660 EFLAGS: 00010297
RAX: ffff880068472c00 RBX: ffff880065a2da00 RCX: 0000000000000000
RDX: 0000000000000000 RSI: 000000000000000d RDI: ffffed000c7d9ec0
RBP: ffff880063ecf690 R08: 1ffff1000d08e67e R09: 1ffff1000cb45b50
R10: dffffc0000000000 R11: 0000000000000000 R12: ffff880065a2da80
R13: 0000000000000008 R14: ffff880065a2dad8 R15: 0000000000000001
FS: 00007fbb006497c0(0000) GS:ffff88006cd00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000020032fe0 CR3: 00000000636d9000 CR4: 00000000000006e0
Stack:
ffff88006bfbb948 ffff880065a2da00 ffff880064160000 1ffff1000cb45b52
0000000000000000 1ffff1000d4d3933 ffff880063ecf6f8 ffffffff83354ced
00000000fffffe00 ffff880065a2da90 ffff880063ecf6c0 ffffffff00000001
Call Trace:
[< inline >] udp_csum_pull_header ./include/net/udp.h:166
[<ffffffff83354ced>] udpv6_queue_rcv_skb+0x37d/0x17b0 net/ipv6/udp.c:625
[< inline >] sk_backlog_rcv ./include/net/sock.h:874
[<ffffffff82b7eec6>] __release_sock+0x126/0x3a0 net/core/sock.c:2046
[<ffffffff82b7f199>] release_sock+0x59/0x1c0 net/core/sock.c:2504
[<ffffffff8334fc50>] udpv6_sendmsg+0x1310/0x24a0 net/ipv6/udp.c:1273
[<ffffffff83174fa7>] inet_sendmsg+0x317/0x4e0 net/ipv4/af_inet.c:734
[< inline >] sock_sendmsg_nosec net/socket.c:621
[<ffffffff82b7176c>] sock_sendmsg+0xcc/0x110 net/socket.c:631
[<ffffffff82b719d1>] sock_write_iter+0x221/0x3b0 net/socket.c:829
[<ffffffff8151e69b>] do_iter_readv_writev+0x2bb/0x3f0 fs/read_write.c:695
[<ffffffff81520501>] do_readv_writev+0x431/0x730 fs/read_write.c:872
[<ffffffff81520d2f>] vfs_writev+0x8f/0xc0 fs/read_write.c:911
[<ffffffff81520e41>] do_writev+0xe1/0x240 fs/read_write.c:944
[< inline >] SYSC_writev fs/read_write.c:1017
[<ffffffff81523ca7>] SyS_writev+0x27/0x30 fs/read_write.c:1014
[<ffffffff83fc4381>] entry_SYSCALL_64_fastpath+0x1f/0xc2
arch/x86/entry/entry_64.S:209
Code: 89 f8 49 c1 e8 03 47 0f b6 14 08 45 84 d2 74 0a 41 80 fa 03 0f
8e cf 00 00 00 80 a3 91 00 00 00 f9 e9 43 ff ff ff e8 3b 79 79 fe <0f>
0b e8 34 79 79 fe 0f 0b e8 2d 79 79 fe 48 8b 7d d0 31 d2 44
RIP [<ffffffff82b8fd85>] skb_pull_rcsum+0x255/0x350 net/core/skbuff.c:3029
RSP <ffff880063ecf660>
---[ end trace a5d5d2cef6a25ecb ]---
==================================================================
[-- Attachment #2: skb-pull-bug-poc.c --]
[-- Type: text/x-csrc, Size: 7858 bytes --]
// autogenerated by syzkaller (http://github.com/google/syzkaller)
#ifndef __NR_mmap
#define __NR_mmap 9
#endif
#ifndef __NR_bind
#define __NR_bind 49
#endif
#ifndef __NR_sendmsg
#define __NR_sendmsg 46
#endif
#ifndef __NR_writev
#define __NR_writev 20
#endif
#ifndef __NR_socket
#define __NR_socket 41
#endif
#ifndef __NR_syz_fuse_mount
#define __NR_syz_fuse_mount 1000004
#endif
#ifndef __NR_syz_fuseblk_mount
#define __NR_syz_fuseblk_mount 1000005
#endif
#ifndef __NR_syz_open_dev
#define __NR_syz_open_dev 1000002
#endif
#ifndef __NR_syz_open_pts
#define __NR_syz_open_pts 1000003
#endif
#ifndef __NR_syz_test
#define __NR_syz_test 1000001
#endif
#include <sys/ioctl.h>
#include <sys/socket.h>
#include <sys/stat.h>
#include <sys/syscall.h>
#include <sys/types.h>
#include <net/if_arp.h>
#include <errno.h>
#include <error.h>
#include <fcntl.h>
#include <pthread.h>
#include <setjmp.h>
#include <signal.h>
#include <stddef.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
__thread int skip_segv;
__thread jmp_buf segv_env;
static void segv_handler(int sig, siginfo_t* info, void* uctx)
{
if (__atomic_load_n(&skip_segv, __ATOMIC_RELAXED))
_longjmp(segv_env, 1);
exit(sig);
}
static void install_segv_handler()
{
struct sigaction sa;
memset(&sa, 0, sizeof(sa));
sa.sa_sigaction = segv_handler;
sa.sa_flags = SA_NODEFER | SA_SIGINFO;
sigaction(SIGSEGV, &sa, NULL);
sigaction(SIGBUS, &sa, NULL);
}
#define NONFAILING(...) \
{ \
__atomic_fetch_add(&skip_segv, 1, __ATOMIC_SEQ_CST); \
if (_setjmp(segv_env) == 0) { \
__VA_ARGS__; \
} \
__atomic_fetch_sub(&skip_segv, 1, __ATOMIC_SEQ_CST); \
}
static uintptr_t syz_open_dev(uintptr_t a0, uintptr_t a1, uintptr_t a2)
{
if (a0 == 0xc || a0 == 0xb) {
char buf[128];
sprintf(buf, "/dev/%s/%d:%d", a0 == 0xc ? "char" : "block",
(uint8_t)a1, (uint8_t)a2);
return open(buf, O_RDWR, 0);
} else {
char buf[1024];
char* hash;
strncpy(buf, (char*)a0, sizeof(buf));
buf[sizeof(buf) - 1] = 0;
while ((hash = strchr(buf, '#'))) {
*hash = '0' + (char)(a1 % 10);
a1 /= 10;
}
return open(buf, a2, 0);
}
}
static uintptr_t syz_open_pts(uintptr_t a0, uintptr_t a1)
{
int ptyno = 0;
if (ioctl(a0, TIOCGPTN, &ptyno))
return -1;
char buf[128];
sprintf(buf, "/dev/pts/%d", ptyno);
return open(buf, a1, 0);
}
static uintptr_t syz_fuse_mount(uintptr_t a0, uintptr_t a1,
uintptr_t a2, uintptr_t a3,
uintptr_t a4, uintptr_t a5)
{
uint64_t target = a0;
uint64_t mode = a1;
uint64_t uid = a2;
uint64_t gid = a3;
uint64_t maxread = a4;
uint64_t flags = a5;
int fd = open("/dev/fuse", O_RDWR);
if (fd == -1)
return fd;
char buf[1024];
sprintf(buf, "fd=%d,user_id=%ld,group_id=%ld,rootmode=0%o", fd,
(long)uid, (long)gid, (unsigned)mode & ~3u);
if (maxread != 0)
sprintf(buf + strlen(buf), ",max_read=%ld", (long)maxread);
if (mode & 1)
strcat(buf, ",default_permissions");
if (mode & 2)
strcat(buf, ",allow_other");
syscall(SYS_mount, "", target, "fuse", flags, buf);
return fd;
}
static uintptr_t syz_fuseblk_mount(uintptr_t a0, uintptr_t a1,
uintptr_t a2, uintptr_t a3,
uintptr_t a4, uintptr_t a5,
uintptr_t a6, uintptr_t a7)
{
uint64_t target = a0;
uint64_t blkdev = a1;
uint64_t mode = a2;
uint64_t uid = a3;
uint64_t gid = a4;
uint64_t maxread = a5;
uint64_t blksize = a6;
uint64_t flags = a7;
int fd = open("/dev/fuse", O_RDWR);
if (fd == -1)
return fd;
if (syscall(SYS_mknodat, AT_FDCWD, blkdev, S_IFBLK, makedev(7, 199)))
return fd;
char buf[256];
sprintf(buf, "fd=%d,user_id=%ld,group_id=%ld,rootmode=0%o", fd,
(long)uid, (long)gid, (unsigned)mode & ~3u);
if (maxread != 0)
sprintf(buf + strlen(buf), ",max_read=%ld", (long)maxread);
if (blksize != 0)
sprintf(buf + strlen(buf), ",blksize=%ld", (long)blksize);
if (mode & 1)
strcat(buf, ",default_permissions");
if (mode & 2)
strcat(buf, ",allow_other");
syscall(SYS_mount, blkdev, target, "fuseblk", flags, buf);
return fd;
}
static uintptr_t execute_syscall(int nr, uintptr_t a0, uintptr_t a1,
uintptr_t a2, uintptr_t a3,
uintptr_t a4, uintptr_t a5,
uintptr_t a6, uintptr_t a7,
uintptr_t a8)
{
switch (nr) {
default:
return syscall(nr, a0, a1, a2, a3, a4, a5);
case __NR_syz_test:
return 0;
case __NR_syz_open_dev:
return syz_open_dev(a0, a1, a2);
case __NR_syz_open_pts:
return syz_open_pts(a0, a1);
case __NR_syz_fuse_mount:
return syz_fuse_mount(a0, a1, a2, a3, a4, a5);
case __NR_syz_fuseblk_mount:
return syz_fuseblk_mount(a0, a1, a2, a3, a4, a5, a6, a7);
}
}
long r[17];
int main()
{
install_segv_handler();
memset(r, -1, sizeof(r));
r[0] = execute_syscall(__NR_mmap, 0x20000000ul, 0x35000ul, 0x3ul,
0x32ul, 0xfffffffffffffffful, 0x0ul, 0, 0, 0);
r[1] = execute_syscall(__NR_socket, 0xaul, 0x2ul, 0x88ul, 0, 0, 0, 0,
0, 0);
NONFAILING(memcpy(
(void*)0x20034000,
"\x0a\x00\x42\x42\x7a\x85\x86\xb4\x00\x00\x00\x00\x00\x00\x00\x00"
"\x00\x00\x00\x00\x00\x00\x00\x01\xb4\x73\x17\x84\x00\x00\x00\x00"
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
"\x00",
128));
r[3] = execute_syscall(__NR_bind, r[1], 0x20034000ul, 0x80ul, 0, 0, 0,
0, 0, 0);
NONFAILING(*(uint64_t*)0x2002b000 = (uint64_t)0x20021f80);
NONFAILING(*(uint32_t*)0x2002b008 = (uint32_t)0x80);
NONFAILING(*(uint64_t*)0x2002b010 = (uint64_t)0x2001f000);
NONFAILING(*(uint64_t*)0x2002b018 = (uint64_t)0x0);
NONFAILING(*(uint64_t*)0x2002b020 = (uint64_t)0x20027000);
NONFAILING(*(uint64_t*)0x2002b028 = (uint64_t)0x0);
NONFAILING(*(uint32_t*)0x2002b030 = (uint32_t)0x0);
NONFAILING(memcpy(
(void*)0x20021f80,
"\x0a\x00\x42\x42\xbe\xf9\xa8\xa3\x00\x00\x00\x00\x00\x00\x00\x00"
"\x00\x00\x00\x00\x00\x00\x00\x01\x7f\xdb\x0d\xf1\x00\x00\x00\x00"
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
"\x00",
128));
r[12] = execute_syscall(__NR_sendmsg, r[1], 0x2002b000ul, 0x8000ul, 0,
0, 0, 0, 0, 0);
NONFAILING(*(uint64_t*)0x20032fe0 = (uint64_t)0x20032000);
NONFAILING(*(uint64_t*)0x20032fe8 = (uint64_t)0x1);
NONFAILING(memcpy((void*)0x20032000, "\x0d", 1));
r[16] = execute_syscall(__NR_writev, r[1], 0x20032fe0ul, 0x1ul, 0, 0,
0, 0, 0, 0);
return 0;
}
^ permalink raw reply
* Re: mlx5 "syndrome" errors in kernel log
From: Saeed Mahameed @ 2016-11-22 12:04 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Saeed Mahameed, Tariq Toukan, netdev@vger.kernel.org
In-Reply-To: <20161122105944.0c64e77b@redhat.com>
On Tue, Nov 22, 2016 at 11:59 AM, Jesper Dangaard Brouer
<jbrouer@redhat.com> wrote:
>
> Hi Saeed,
>
> I'm seeing below dmesg errors, after pulling net-next at commit
> e796f49d826aad, before I was not seeing these errors, where my tree was
> based on top of commit 319b0534b95.
>
> mlx5_core 0000:02:00.1: mlx5_cmd_check:698:(pid 8788): ACCESS_REG(0x805) op_mod(0x1) failed, status bad parameter(0x3), syndrome (0x6c4d48)
> mlx5_core 0000:02:00.1: mlx5_cmd_check:698:(pid 8788): ACCESS_REG(0x805) op_mod(0x1) failed, status bad parameter(0x3), syndrome (0x6c4d48)
> mlx5_core 0000:02:00.0: mlx5_cmd_check:698:(pid 8788): ACCESS_REG(0x805) op_mod(0x1) failed, status bad parameter(0x3), syndrome (0x6c4d48)
> mlx5_core 0000:02:00.0: mlx5_cmd_check:698:(pid 8788): ACCESS_REG(0x805) op_mod(0x1) failed, status bad parameter(0x3), syndrome (0x6c4d48)
> mlx5_core 0000:02:00.1: mlx5_cmd_check:698:(pid 8788): ACCESS_REG(0x805) op_mod(0x1) failed, status bad parameter(0x3), syndrome (0x6c4d48)
> mlx5_core 0000:02:00.1: mlx5_cmd_check:698:(pid 8788): ACCESS_REG(0x805) op_mod(0x1) failed, status bad parameter(0x3), syndrome (0x6c4d48)
> mlx5_core 0000:02:00.0: mlx5_cmd_check:698:(pid 8788): ACCESS_REG(0x805) op_mod(0x1) failed, status bad parameter(0x3), syndrome (0x6c4d48)
> mlx5_core 0000:02:00.0: mlx5_cmd_check:698:(pid 8788): ACCESS_REG(0x805) op_mod(0x1) failed, status bad parameter(0x3), syndrome (0x6c4d48)
>
>
> Listing my firmware version:
>
> $ ethtool -i mlx5p2
> driver: mlx5_core
> version: 3.0-1 (January 2015)
> firmware-version: 12.12.1240
Hi Jesper,
Seems like this FW version doesn't support a new FW command introduced
by "net/mlx5e: Expose PCIe statistics to ethtool"
I suggest to upgrade FW, but if you don't know how to do it or in a
hurry, please go ahead and revert "
net/mlx5e: Expose PCIe statistics to ethtool"
I will need to introduce a new capability bit as a permanent solution
and a fix for the above patch.
Thanks for the report,
We will handle this.
^ permalink raw reply
* Re: [PATCH] net: ipv6: avoid errors due to per-cpu atomic alloc
From: Hannes Frederic Sowa @ 2016-11-22 12:18 UTC (permalink / raw)
To: Mike Manning, netdev
In-Reply-To: <1479810840-19122-1-git-send-email-mmanning@brocade.com>
On 22.11.2016 11:34, Mike Manning wrote:
> Bursts of failures may occur when adding IPv6 routes via Netlink to the
> kernel when testing under scale (e.g. 500 routes lost out of 1M). The
> reason is that percpu.c:pcpu_balance_workfn() is not guaranteed to have
> extended the area map in time for the atomic allocation using percpu.c:
> pcpu_alloc() to succeed. This results in route additions failing with
> an -ENOMEM error.
>
> While the sender of the Netlink msg to add this route could check for
> an ACK and retransmit in the case of an -ENOMEM error, the latter
> should not occur in the first place if there is plenty of memory. The
> solution is to use non-atomic alloc for rt6_info instead. While the
> client may now be blocked for longer depending on the state of the
> chunk being added to, this work has to be incurred at some point.
>
> The alternative solution would be to provide configurable parameters
> e.g. via sysctl in percpu.c for default map size, low/high empty pages
> and map margins. For this solution, the map margin sizes need to be
> stored per chunk, as large margins cannot be used if the dynamic early
> slots map size is in use. This is not a preferred solution though, as
> it requires tuning of these parameters to provide sufficient margins to
> avoid -ENOMEM errors depending on system requirements.
>
> Signed-off-by: Mike Manning <mmanning@brocade.com>
> ---
> net/ipv6/route.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 1b57e11..0e9bb76 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -347,7 +347,7 @@ struct rt6_info *ip6_dst_alloc(struct net *net,
> struct rt6_info *rt = __ip6_dst_alloc(net, dev, flags);
>
> if (rt) {
> - rt->rt6i_pcpu = alloc_percpu_gfp(struct rt6_info *, GFP_ATOMIC);
> + rt->rt6i_pcpu = alloc_percpu_gfp(struct rt6_info *, GFP_KERNEL);
> if (rt->rt6i_pcpu) {
> int cpu;
Nak, this doesn't work, as ip6_dst_alloc must be callable from
non-blocking code paths unfortunately.
^ permalink raw reply
* Re: [PATCH] ipv6:ipv6_pinfo dereferenced after NULL check
From: Hannes Frederic Sowa @ 2016-11-22 12:26 UTC (permalink / raw)
To: Manjeet Pawar, davem, kuznet, jmorris, yoshfuji, kaber, netdev,
linux-kernel
Cc: pankaj.m, ajeet.y, Rohit Thapliyal
In-Reply-To: <1479796024-39418-1-git-send-email-manjeet.p@samsung.com>
On 22.11.2016 07:27, Manjeet Pawar wrote:
> From: Rohit Thapliyal <r.thapliyal@samsung.com>
>
> np checked for NULL and then dereferenced. It should be modified
> for NULL case.
>
> Signed-off-by: Rohit Thapliyal <r.thapliyal@samsung.com>
> Signed-off-by: Manjeet Pawar <manjeet.p@samsung.com>
> ---
> net/ipv6/ip6_output.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> index 1dfc402..c2afa14 100644
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c
> @@ -205,14 +205,15 @@ int ip6_xmit(const struct sock *sk, struct sk_buff *skb, struct flowi6 *fl6,
> /*
> * Fill in the IPv6 header
> */
> - if (np)
> + if (np) {
> hlimit = np->hop_limit;
> + ip6_flow_hdr(
> + hdr, tclass, ip6_make_flowlabel(
> + net, skb, fl6->flowlabel,
> + np->autoflowlabel, fl6));
> + }
> if (hlimit < 0)
> hlimit = ip6_dst_hoplimit(dst);
>
> - ip6_flow_hdr(hdr, tclass, ip6_make_flowlabel(net, skb, fl6->flowlabel,
> - np->autoflowlabel, fl6));
> -
> hdr->payload_len = htons(seg_len);
> hdr->nexthdr = proto;
> hdr->hop_limit = hlimit;
>
We always should initialize hdr and not skip the ip6_flow_hdr call.
Do you saw a bug or did you find this by code review? I wonder if np can
actually be NULL at this point. Maybe we can just eliminate the NULL check.
Thanks,
Hannes
^ permalink raw reply
* Re: [RFC net-next 0/3] net: bridge: Allow CPU port configuration
From: Jiri Pirko @ 2016-11-22 12:49 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev, davem, bridge, stephen, vivien.didelot, andrew, jiri,
idosch
In-Reply-To: <20161121190925.14530-1-f.fainelli@gmail.com>
Mon, Nov 21, 2016 at 08:09:22PM CET, f.fainelli@gmail.com wrote:
>Hi all,
>
>This patch series allows using the bridge master interface to configure
>an Ethernet switch port's CPU/management port with different VLAN attributes than
>those of the bridge downstream ports/members.
>
>Jiri, Ido, Andrew, Vivien, please review the impact on mlxsw and mv88e6xxx, I
>tested this with b53 and a mockup DSA driver.
Patchset looks fine to me.
>
>Open questions:
>
>- if we have more than one bridge on top of a physical switch, the driver
> should keep track of that and verify that we are not going to change
> the CPU port VLAN attributes in a way that results in incompatible settings
> to be applied
Ack. In mlxsw this is tracked
>
>- if the default behavior is to have all VLANs associated with the CPU port
> be ingressing/egressing tagged to the CPU, is this really useful?
>
>Florian Fainelli (3):
> net: bridge: Allow bridge master device to configure switch CPU port
> net: dsa: Propagate VLAN add/del to CPU port(s)
> net: dsa: b53: Remove CPU port specific VLAN programming
>
> drivers/net/dsa/b53/b53_common.c | 22 ++++++--------------
> net/bridge/br_vlan.c | 28 ++++++++++++++++++++++---
> net/dsa/slave.c | 45 +++++++++++++++++++++++++++++-----------
> 3 files changed, 64 insertions(+), 31 deletions(-)
>
>--
>2.9.3
>
^ permalink raw reply
* Re: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable
From: Mark Lord @ 2016-11-22 13:12 UTC (permalink / raw)
To: Hayes Wang, netdev@vger.kernel.org
Cc: nic_swsd, linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org
In-Reply-To: <b9809516-d036-bfc3-b7a3-6563033ec957@pobox.com>
On 16-11-18 07:03 AM, Mark Lord wrote:
> On 16-11-18 02:57 AM, Hayes Wang wrote:
> ..
>> Besides, the maximum data length which the RTL8152 would send to
>> the host is 16KB. That is, if the agg_buf_sz is 16KB, the host
>> wouldn't split it. However, you still see problems for it.
>
> How does the RTL8152 know that the limit is 16KB,
> rather than some other number? Is this a hardwired number
> in the hardware, or is it a parameter that the software
> sends to the chip during initialization?
..
> The first issue is that a packet sometimes begins in one URB,
> and completes in the next URB, without an rx_desc at the start
> of the second URB. This I have already reported earlier.
Long run tests over the weekend, with the invalidate_dcache_range() call
before the inner loop of r8152_rx_bottom(), turned up a few instances
where packets were truncated inside a 16384 byte URB buffer, without filling the URB.
[10.293228] r8152_rx_bottom: 4278 corrupted urb: head=9d210000 urb_offset=2856/3376 pkt_len(1518) exceeds remainder(496)
[10.304523] r8152_dump_rx_desc: 044805ee 40080000 006005dc 06020000 00000000 00000000 rx_len=1518
..
[ 16.660431] r8152_rx_bottom: 7802 corrupted urb: head=9d1f8000 urb_offset=1544/2064 pkt_len(1518) exceeds remainder(496)
[ 16.671719] r8152_dump_rx_desc: 044805ee 40480000 004005dc 46020006 00000000 00000000 rx_len=1518
The r8152.c driver attempted to build skb's for the entire packet size,
even though the 1518-byte packets had only 496-bytes of data in the URB.
It is not clear what the chip did with the rest of the packets in question,
but the next URBs in each case began with a new/real rx_desc and new packet.
There were also unconnected events during the test runs where the
test code noticed totally invalid rx_desc structs in the middles of URBs.
The stock driver would again have attempted to treat those as "valid" (ugh).
..
[ 10.273906] r8152_check_rx_desc: rx_desc looks bad.
[ 10.279012] r8152_rx_bottom: 4338 corrupted urb. head=9d210000 urb_offset=2856/3376 len_used=2880
[ 10.288196] r8152_dump_rx_desc: 312e3239 382e3836 0a20382e 3d435253 3034336d 202f3a30 rx_len=12857
..
[ 7.184565] r8152_check_rx_desc: rx_desc looks bad.
[ 7.189657] r8152_rx_bottom: 1678 corrupted urb. head=9d210000 urb_offset=2856/3376 len_used=2880
[ 7.198852] r8152_dump_rx_desc: a1388402 803c9001 84380810 a67c5c4c a77c782b c64c782b rx_len=1026
..
[ 10.351251] r8152_check_rx_desc: rx_desc looks bad.
[ 10.356356] r8152_rx_bottom: 4397 corrupted urb. head=9d20c000 urb_offset=4400/7984 len_used=4424
[ 10.365543] r8152_dump_rx_desc: 312e3239 382e3836 0a20382e 3d435253 3034336d 202f3a30 rx_len=12857
..
[ 10.518119] r8152_check_rx_desc: rx_desc looks bad.
[ 10.523204] r8152_rx_bottom: 4458 corrupted urb. head=9d210000 urb_offset=4400/7984 len_used=4424
[ 10.532416] r8152_dump_rx_desc: 54544120 6e3d5352 636f6c6f 65762c6b 343d7372 6464612c rx_len=16672
..
> But the driver, as written, sometimes accesses bytes outside
> of the 16KB URB buffer, because it trusts the non-existent
> rx_desc in these cases, and also because it accesses bytes
> from the rx_desc without first checking whether there is
> sufficient remaining space in the URB to hold an rx_desc.
>
> These incorrect accesses sometimes touch memory outside
> of the URB buffer. Since the driver allocates all of its
> rx URB buffers at once, they are highly likely to be
> physically (and therefore virtually) adjacent in memory.
>
> So mistakenly accessing beyond the end of one buffer will
> often result in a read from memory of the next URB buffer.
> Which causes a portion of it to be loaded in the the D-cache.
>
> When that URB is subsequently filled by DMA, there then exists
> a data-consistency issue: the D-cache contains stale information
> from before the latest DMA cycle.
>
> So this explains the strange memory behaviour observed earlier on.
> When I add a call to invalidate_dcache_range() to the driver
> just before it begins examining a new rx URB, the problems go away.
> So this confirms the observations.
>
> Using non-cacheable RAM also makes the problem go away.
> But neither is a fix for the real buffer overrun accesses in the driver.
>
> Fix the "packet spans URBs" bug, and fix the driver to ALWAYS
> test lengths/ranges before accessing the actual buffer,
> and everything should begin working reliably.
^ permalink raw reply
* Re: [PATCH] net: ipv6: avoid errors due to per-cpu atomic alloc
From: Mike Manning @ 2016-11-22 13:17 UTC (permalink / raw)
To: Hannes Frederic Sowa, netdev
In-Reply-To: <718b6520-686e-cab3-1c8d-9e1de4cb0dbd@stressinduktion.org>
On 11/22/2016 12:18 PM, Hannes Frederic Sowa wrote:
> On 22.11.2016 11:34, Mike Manning wrote:
>> Bursts of failures may occur when adding IPv6 routes via Netlink to the
>> kernel when testing under scale (e.g. 500 routes lost out of 1M). The
>> reason is that percpu.c:pcpu_balance_workfn() is not guaranteed to have
>> extended the area map in time for the atomic allocation using percpu.c:
>> pcpu_alloc() to succeed. This results in route additions failing with
>> an -ENOMEM error.
>>
>> While the sender of the Netlink msg to add this route could check for
>> an ACK and retransmit in the case of an -ENOMEM error, the latter
>> should not occur in the first place if there is plenty of memory. The
>> solution is to use non-atomic alloc for rt6_info instead. While the
>> client may now be blocked for longer depending on the state of the
>> chunk being added to, this work has to be incurred at some point.
>>
>> The alternative solution would be to provide configurable parameters
>> e.g. via sysctl in percpu.c for default map size, low/high empty pages
>> and map margins. For this solution, the map margin sizes need to be
>> stored per chunk, as large margins cannot be used if the dynamic early
>> slots map size is in use. This is not a preferred solution though, as
>> it requires tuning of these parameters to provide sufficient margins to
>> avoid -ENOMEM errors depending on system requirements.
>>
>> Signed-off-by: Mike Manning <mmanning@brocade.com>
>> ---
>> net/ipv6/route.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
>> index 1b57e11..0e9bb76 100644
>> --- a/net/ipv6/route.c
>> +++ b/net/ipv6/route.c
>> @@ -347,7 +347,7 @@ struct rt6_info *ip6_dst_alloc(struct net *net,
>> struct rt6_info *rt = __ip6_dst_alloc(net, dev, flags);
>>
>> if (rt) {
>> - rt->rt6i_pcpu = alloc_percpu_gfp(struct rt6_info *, GFP_ATOMIC);
>> + rt->rt6i_pcpu = alloc_percpu_gfp(struct rt6_info *, GFP_KERNEL);
>> if (rt->rt6i_pcpu) {
>> int cpu;
>
> Nak, this doesn't work, as ip6_dst_alloc must be callable from
> non-blocking code paths unfortunately.
>
>
Thanks for the prompt reply.
Do you consider the alternative of providing configurable parameters for per-cpu
alloc as viable, or is there a better way of dealing with this?
While I have tested such param changes under scale as avoiding the -ENOMEM errors, it
would be good to get confirmation that this approach is acceptable prior to coding the
sysctl handling for these.
^ permalink raw reply
* [PATCH] iproute2: Nr. of packets and octets for macsec tx stats were swapped.
From: Daniel.Hopf @ 2016-11-22 13:24 UTC (permalink / raw)
To: netdev
Signed-off-by: Daniel Hopf <daniel.hopf@continental-corporation.com>
---
ip/ipmacsec.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/ip/ipmacsec.c b/ip/ipmacsec.c
index c9252bb..aa89a00 100644
--- a/ip/ipmacsec.c
+++ b/ip/ipmacsec.c
@@ -634,10 +634,10 @@ static void print_one_stat(const char **names,
struct rtattr **attr, int idx,
}
static const char *txsc_stats_names[NUM_MACSEC_TXSC_STATS_ATTR] = {
- [MACSEC_TXSC_STATS_ATTR_OUT_PKTS_PROTECTED] =
"OutOctetsProtected",
- [MACSEC_TXSC_STATS_ATTR_OUT_PKTS_ENCRYPTED] =
"OutOctetsEncrypted",
- [MACSEC_TXSC_STATS_ATTR_OUT_OCTETS_PROTECTED] =
"OutPktsProtected",
- [MACSEC_TXSC_STATS_ATTR_OUT_OCTETS_ENCRYPTED] =
"OutPktsEncrypted",
+ [MACSEC_TXSC_STATS_ATTR_OUT_PKTS_PROTECTED] = "OutPktsProtected",
+ [MACSEC_TXSC_STATS_ATTR_OUT_PKTS_ENCRYPTED] = "OutPktsEncrypted",
+ [MACSEC_TXSC_STATS_ATTR_OUT_OCTETS_PROTECTED] =
"OutOctetsProtected",
+ [MACSEC_TXSC_STATS_ATTR_OUT_OCTETS_ENCRYPTED] =
"OutOctetsEncrypted",
};
static void print_txsc_stats(const char *prefix, struct rtattr *attr)
^ permalink raw reply related
* Re: [PATCH] iproute2: Nr. of packets and octets for macsec tx stats were swapped.
From: Sabrina Dubroca @ 2016-11-22 13:56 UTC (permalink / raw)
To: Daniel.Hopf; +Cc: netdev
In-Reply-To: <OFDC737720.EA272892-ONC1258073.00458CA2-C1258073.0049AC0D@continental-corporation.com>
Hi Daniel,
Thanks for fixing this. I noticed it some time ago but it seems
I forgot to send the patch :(
Acked-by: Sabrina Dubroca <sd@queasysnail.net>
Your subject line should be:
Subject: [PATCH iproute2] macsec: Nr.of packets and octets for macsec tx stats were swapped.
with "iproute2" between the brackets.
2016-11-22, 14:24:40 +0100, Daniel.Hopf@continental-corporation.com wrote:
> Signed-off-by: Daniel Hopf <daniel.hopf@continental-corporation.com>
> ---
> ip/ipmacsec.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/ip/ipmacsec.c b/ip/ipmacsec.c
> index c9252bb..aa89a00 100644
> --- a/ip/ipmacsec.c
> +++ b/ip/ipmacsec.c
> @@ -634,10 +634,10 @@ static void print_one_stat(const char **names,
> struct rtattr **attr, int idx,
> }
>
> static const char *txsc_stats_names[NUM_MACSEC_TXSC_STATS_ATTR] = {
> - [MACSEC_TXSC_STATS_ATTR_OUT_PKTS_PROTECTED] =
> "OutOctetsProtected",
> - [MACSEC_TXSC_STATS_ATTR_OUT_PKTS_ENCRYPTED] =
> "OutOctetsEncrypted",
> - [MACSEC_TXSC_STATS_ATTR_OUT_OCTETS_PROTECTED] =
> "OutPktsProtected",
> - [MACSEC_TXSC_STATS_ATTR_OUT_OCTETS_ENCRYPTED] =
> "OutPktsEncrypted",
> + [MACSEC_TXSC_STATS_ATTR_OUT_PKTS_PROTECTED] = "OutPktsProtected",
> + [MACSEC_TXSC_STATS_ATTR_OUT_PKTS_ENCRYPTED] = "OutPktsEncrypted",
> + [MACSEC_TXSC_STATS_ATTR_OUT_OCTETS_PROTECTED] =
> "OutOctetsProtected",
> + [MACSEC_TXSC_STATS_ATTR_OUT_OCTETS_ENCRYPTED] =
> "OutOctetsEncrypted",
> };
Your patch was corrupted, probably by your email client, you have
extra newlines everywhere.
Can you send a v2 of this patch? Thanks!
--
Sabrina
^ permalink raw reply
* Re: Synopsys Ethernet QoS Driver
From: Joao Pinto @ 2016-11-22 14:16 UTC (permalink / raw)
To: Lars Persson, Giuseppe CAVALLARO
Cc: Joao Pinto, Rayagond Kokatanur, Rabin Vincent, mued dib,
David Miller, Jeff Kirsher, jiri@mellanox.com,
saeedm@mellanox.com, idosch@mellanox.com, netdev,
linux-kernel@vger.kernel.org, CARLOS.PALMINHA@synopsys.com,
Andreas Irestål, alexandre.torgue@st.com,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <7c7798b5-8cd4-ba99-f526-22d3e06e05db@synopsys.com>
Hi Lars and Peppe,
On 21-11-2016 16:11, Joao Pinto wrote:
> On 21-11-2016 15:43, Lars Persson wrote:
>>
>>
>>> 21 nov. 2016 kl. 16:06 skrev Joao Pinto <Joao.Pinto@synopsys.com>:
>>>
>>>> On 21-11-2016 14:25, Giuseppe CAVALLARO wrote:
>>>>> On 11/21/2016 2:28 PM, Lars Persson wrote:
>>>>>
>>>>>
>>>>>> 21 nov. 2016 kl. 13:53 skrev Giuseppe CAVALLARO <peppe.cavallaro@st.com>:
>>>>>>
>>>>>> Hello Joao
>>>>>>
>>>>>>> On 11/21/2016 1:32 PM, Joao Pinto wrote:
>>>>>>> Hello,
>>>>>>>
>>>>>>>>> On 21-11-2016 05:29, Rayagond Kokatanur wrote:
>>>>>>>>>> On Sat, Nov 19, 2016 at 7:26 PM, Rabin Vincent <rabin@rab.in> wrote:
>>>>>>>>>> On Fri, Nov 18, 2016 at 02:20:27PM +0000, Joao Pinto wrote:
>>>>>>>>>> For now we are interesting in improving the synopsys QoS driver under
>>>>>>>>>> /nect/ethernet/synopsys. For now the driver structure consists of a
>>>>>>>>>> single file
>>>>>>>>>> called dwc_eth_qos.c, containing synopsys ethernet qos common ops and
>
snip (...)
>>>>>>
>>>>>> Peppe
>>>>>>
>>>>>
>>>>> Hello Joao and others,
>>>>>
>>>
>>> Hi Lars,
>>>
>>>>> As the maintainer of dwc_eth_qos.c I prefer also that we put efforts on the
>>>>> most mature driver, the stmmac.
>>>>>
>>>>> I hope that the code can migrate into an ethernet/synopsys folder to keep the
>>>>> convention of naming the folder after the vendor. This makes it easy for
>>>>> others to find the driver.
>>>>>
>>>>> The dwc_eth_qos.c will eventually be removed and its DT binding interface can
>>>>> then be implemented in the stmmac driver.
>>>
>>> So your ideia is to pick the ethernet/stmmac and rename it to ethernet/synopsys
>>> and try to improve the structure and add the missing QoS features to it?
>>
>> Indeed this is what I prefer.
>
> Ok, it makes sense.
> Just for curiosity the target setup is the following:
> https://www.youtube.com/watch?v=8V-LB5y2Cos
> but instead of using internal drivers, we desire to use mainline drivers only.
>
> Thanks!
Regarding this subject, I am thinking of making the following adaption:
a) delete ethernet/synopsys
b) rename ethernet/stmicro/stmmac to ethernet/synopsys
and send you a patch for you to evaluate. Both agree with the approach?
To have a new work base would be important, because I will add to the "new"
structure some missing QoS features like Multichannel support, CBS and later TSN.
Thanks.
>
>>
>>>
>>>>
>>>> Thanks Lars, I will be happy to support all you on this transition
>>>> and I agree on renaming all.
>>>>
>>>> peppe
>>>>
>>>>
>>>>> - Lars
>>>>>
>>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>> (See http://lists.openwall.net/netdev/2016/02/29/127)
>>>>>>>>>
>>>>>>>>> The former only supports 4.x of the hardware.
>>>>>>>>>
>>>>>>>>> The later supports 4.x and 3.x and already has a platform glue driver
>>>>>>>>> with support for several platforms, a PCI glue driver, and a core driver
>>>>>>>>> with several features not present in the former (for example: TX/RX
>>>>>>>>> interrupt coalescing, EEE, PTP).
>>>>>>>>>
>>>>>>>>> Have you evaluated both drivers? Why have you decided to work on the
>>>>>>>>> former rather than the latter?
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>> Thanks.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>
^ permalink raw reply
* [PATCH net] net/mlx4_en: Free netdev resources under state lock
From: Tariq Toukan @ 2016-11-22 14:20 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Eran Ben Elisha, Saeed Mahameed, Sagi Grimberg,
Tariq Toukan, Brenden Blanco
Make sure mlx4_en_free_resources is called under the netdev state lock.
This is needed since RCU dereference of XDP prog should be protected.
Fixes: 326fe02d1ed6 ("net/mlx4_en: protect ring->xdp_prog with rcu_read_lock")
Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
Reported-by: Sagi Grimberg <sagi@grimberg.me>
CC: Brenden Blanco <bblanco@plumgrid.com>
---
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index 3a47e83d3e07..a60f635da78b 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -129,6 +129,9 @@ static enum mlx4_net_trans_rule_id mlx4_ip_proto_to_trans_rule_id(u8 ip_proto)
}
};
+/* Must not acquire state_lock, as its corresponding work_sync
+ * is done under it.
+ */
static void mlx4_en_filter_work(struct work_struct *work)
{
struct mlx4_en_filter *filter = container_of(work,
@@ -2189,13 +2192,13 @@ void mlx4_en_destroy_netdev(struct net_device *dev)
mutex_lock(&mdev->state_lock);
mdev->pndev[priv->port] = NULL;
mdev->upper[priv->port] = NULL;
- mutex_unlock(&mdev->state_lock);
#ifdef CONFIG_RFS_ACCEL
mlx4_en_cleanup_filters(priv);
#endif
mlx4_en_free_resources(priv);
+ mutex_unlock(&mdev->state_lock);
kfree(priv->tx_ring);
kfree(priv->tx_cq);
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next] marvell: mark mvneta and mvpp2 32-bit only
From: Arnd Bergmann @ 2016-11-22 14:21 UTC (permalink / raw)
To: David S. Miller
Cc: Arnd Bergmann, Gregory CLEMENT, Florian Fainelli, Marcin Wojtas,
netdev, linux-kernel
Both of these drivers won't work on 64-bit architectures unless they
are redesigned, since they store a virtual address pointer in a 32-bit
field of the descriptors:
drivers/net/ethernet/marvell/mvneta_bm.c: In function 'mvneta_bm_construct':
drivers/net/ethernet/marvell/mvneta_bm.c:103:16: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
drivers/net/ethernet/marvell/mvpp2.c: In function 'mvpp2_prs_vlan_init':
drivers/net/ethernet/marvell/mvpp2.c:2563:32: error: large integer implicitly truncated to unsigned type [-Werror=overflow]
This limits the COMPILE_TEST option for the two drivers again to
only build them on 32-bit. This seems nicer than shutting up the
warnings, in case we ever actually want to use them on 64-bit,
as the warnings indicate which parts of the driver are currently
broken there.
Fixes: a0627f776a45 ("net: marvell: Allow drivers to be built with COMPILE_TEST")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/net/ethernet/marvell/Kconfig | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/marvell/Kconfig b/drivers/net/ethernet/marvell/Kconfig
index d74d4e6f0b34..66fd9dbb2ca7 100644
--- a/drivers/net/ethernet/marvell/Kconfig
+++ b/drivers/net/ethernet/marvell/Kconfig
@@ -58,6 +58,7 @@ config MVNETA
tristate "Marvell Armada 370/38x/XP network interface support"
depends on PLAT_ORION || COMPILE_TEST
depends on HAS_DMA
+ depends on !64BIT
select MVMDIO
select FIXED_PHY
---help---
@@ -81,6 +82,7 @@ config MVPP2
tristate "Marvell Armada 375 network interface support"
depends on MACH_ARMADA_375 || COMPILE_TEST
depends on HAS_DMA
+ depends on !64BIT
select MVMDIO
---help---
This driver supports the network interface units in the
--
2.9.0
^ permalink raw reply related
* [PATCH net-next] net/sched: cls_flower: verify root pointer before dereferncing it
From: Roi Dayan @ 2016-11-22 14:25 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Jiri Pirko, Cong Wang, Or Gerlitz, Roi Dayan, Cong Wang
tp->root is being allocated in init() time and kfreed in destroy()
however it is being dereferenced in classify() path.
We could be in classify() path after destroy() was called and thus
tp->root is null. Verifying if tp->root is null in classify() path
is enough because it's being freed with kfree_rcu() and classify()
path is under rcu_read_lock().
Fixes: 1e052be69d04 ("net_sched: destroy proto tp when all filters are gone")
Signed-off-by: Roi Dayan <roid@mellanox.com>
Cc: Cong Wang <cwang@twopensource.com>
---
Hi Cong, all
As stated above, the issue was introduced with commit 1e052be69d04 ("net_sched: destroy
proto tp when all filters are gone"). This patch provides a fix only for cls_flower where
I succeeded in reproducing the issue. Cong, if you can/want to come up with a fix that
will be applicable for all the others classifiners, I am fine with that.
Thanks,
Roi
net/sched/cls_flower.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index e8dd09a..88a26c4 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -135,7 +135,7 @@ static int fl_classify(struct sk_buff *skb, const struct tcf_proto *tp,
struct fl_flow_key skb_mkey;
struct ip_tunnel_info *info;
- if (!atomic_read(&head->ht.nelems))
+ if (!head || !atomic_read(&head->ht.nelems))
return -1;
fl_clear_masked_range(&skb_key, &head->mask);
--
2.7.4
^ permalink raw reply related
* Re: [v5,1/5] soc: qcom: smem_state: Fix include for ERR_PTR()
From: Valo, Kalle @ 2016-11-22 14:55 UTC (permalink / raw)
To: Bjorn Andersson
Cc: k.eugene.e@gmail.com, Andy Gross, wcn36xx@lists.infradead.org,
linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org
In-Reply-To: <20161118183541.GI28340@tuxbot>
Bjorn Andersson <bjorn.andersson@linaro.org> writes:
> On Wed 16 Nov 10:49 PST 2016, Kalle Valo wrote:
>
>> Bjorn Andersson <bjorn.andersson@linaro.org> wrote:
>> > The correct include file for getting errno constants and ERR_PTR() is
>> > linux/err.h, rather than linux/errno.h, so fix the include.
>> >
>> > Fixes: e8b123e60084 ("soc: qcom: smem_state: Add stubs for disabled smem_state")
>> > Acked-by: Andy Gross <andy.gross@linaro.org>
>> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
>>
>> For some reason this fails to compile now. Can you take a look, please?
>>
>> ERROR: "qcom_wcnss_open_channel" [drivers/net/wireless/ath/wcn36xx/wcn36xx.ko] undefined!
>> make[1]: *** [__modpost] Error 1
>> make: *** [modules] Error 2
>>
>> 5 patches set to Changes Requested.
>>
>> 9429045 [v5,1/5] soc: qcom: smem_state: Fix include for ERR_PTR()
>> 9429047 [v5,2/5] wcn36xx: Transition driver to SMD client
>
> This patch was updated with the necessary depends in Kconfig to catch
> this exact issue and when I pull in your .config (which has QCOM_SMD=n,
> QCOM_WCNSS_CTRL=n and WCN36XX=y) I can build this just fine.
>
> I've tested the various combinations and it seems to work fine. Do you
> have any other patches in your tree?
This was with the pending branch of my ath.git tree. There are other
wireless patches (ath10k etc) but I would guess they don't affect here.
> Any stale objects?
Not sure what you mean with this question, but I didn't run 'make clean'
if that's what you are asking.
> Would you mind retesting this, before I invest more time in trying to
> reproduce the issue you're seeing?
Sure, I'll take a look but that might take few days.
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH v2] net/phy: add trace events for mdio accesses
From: Steven Rostedt @ 2016-11-22 14:55 UTC (permalink / raw)
To: Uwe Kleine-König; +Cc: Florian Fainelli, Ingo Molnar, netdev
In-Reply-To: <20161122100127.5940-1-uwe@kleine-koenig.org>
On Tue, 22 Nov 2016 11:01:27 +0100
Uwe Kleine-König <uwe@kleine-koenig.org> wrote:
> diff --git a/include/trace/events/mdio.h b/include/trace/events/mdio.h
> new file mode 100644
> index 000000000000..468e2d095d19
> --- /dev/null
> +++ b/include/trace/events/mdio.h
> @@ -0,0 +1,42 @@
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM mdio
> +
> +#if !defined(_TRACE_MDIO_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_MDIO_H
> +
> +#include <linux/tracepoint.h>
> +
> +TRACE_EVENT_CONDITION(mdio_access,
> +
> + TP_PROTO(struct mii_bus *bus, int read,
> + unsigned addr, unsigned regnum, u16 val, int err),
> +
> + TP_ARGS(bus, read, addr, regnum, val, err),
> +
> + TP_CONDITION(err >= 0),
> +
> + TP_STRUCT__entry(
> + __array(char, busid, MII_BUS_ID_SIZE)
> + __field(int, read)
read is just a 0 or 1. What about making it a char? That way we can
pack this better. If I'm not mistaken, MII_BUS_ID_SIZE is (20 - 3) or
17. If read is just one byte, then it can fit in one of those three
bytes, and you save 4 extra bytes (assuming addr will be 4 byte
aligned).
-- Steve
> + __field(unsigned, addr)
> + __field(unsigned, regnum)
> + __field(u16, val)
> + ),
> +
> + TP_fast_assign(
> + strncpy(__entry->busid, bus->id, MII_BUS_ID_SIZE);
> + __entry->read = read;
> + __entry->addr = addr;
> + __entry->regnum = regnum;
> + __entry->val = val;
> + ),
> +
> + TP_printk("%s %-5s phy:0x%02x reg:0x%02x val:0x%04hx",
> + __entry->busid, __entry->read ? "read" : "write",
> + __entry->addr, __entry->regnum, __entry->val)
> +);
> +
> +#endif /* if !defined(_TRACE_MDIO_H) || defined(TRACE_HEADER_MULTI_READ) */
> +
> +/* This part must be outside protection */
> +#include <trace/define_trace.h>
^ permalink raw reply
* Re: [PATCHv2 net-next 00/11] Start adding support for mv88e6390
From: David Miller @ 2016-11-22 14:56 UTC (permalink / raw)
To: andrew; +Cc: vivien.didelot, netdev
In-Reply-To: <1479767225-13789-1-git-send-email-andrew@lunn.ch>
From: Andrew Lunn <andrew@lunn.ch>
Date: Mon, 21 Nov 2016 23:26:54 +0100
> This is the first patchset implementing support for the mv88e6390
> family. This is a new generation of switch devices and has numerous
> incompatible changes to the registers. These patches allow the switch
> to the detected during probe, and makes the statistics unit work.
>
> These patches are insufficient to make the mv88e6390 functional. More
> patches will follow.
>
> v2:
> Move stats code into global1
> Change DT compatible string to mv88e6190
> Fixed mv88e6351 stats which v1 had broken
Series applied, thanks Andrew.
^ permalink raw reply
* Re: [net-next PATCH v2 3/5] virtio_net: Add XDP support
From: Michael S. Tsirkin @ 2016-11-22 14:58 UTC (permalink / raw)
To: John Fastabend
Cc: daniel, eric.dumazet, kubakici, shm, davem, alexei.starovoitov,
netdev, bblanco, john.r.fastabend, brouer, tgraf
In-Reply-To: <58340157.8060103@gmail.com>
On Tue, Nov 22, 2016 at 12:27:03AM -0800, John Fastabend wrote:
> On 16-11-21 03:20 PM, Michael S. Tsirkin wrote:
> > On Sat, Nov 19, 2016 at 06:50:33PM -0800, John Fastabend wrote:
> >> From: Shrijeet Mukherjee <shrijeet@gmail.com>
> >>
> >> This adds XDP support to virtio_net. Some requirements must be
> >> met for XDP to be enabled depending on the mode. First it will
> >> only be supported with LRO disabled so that data is not pushed
> >> across multiple buffers. The MTU must be less than a page size
> >> to avoid having to handle XDP across multiple pages.
> >>
> >> If mergeable receive is enabled this first series only supports
> >> the case where header and data are in the same buf which we can
> >> check when a packet is received by looking at num_buf. If the
> >> num_buf is greater than 1 and a XDP program is loaded the packet
> >> is dropped and a warning is thrown. When any_header_sg is set this
> >> does not happen and both header and data is put in a single buffer
> >> as expected so we check this when XDP programs are loaded. Note I
> >> have only tested this with Linux vhost backend.
> >>
> >> If big packets mode is enabled and MTU/LRO conditions above are
> >> met then XDP is allowed.
> >>
> >> A follow on patch can be generated to solve the mergeable receive
> >> case with num_bufs equal to 2. Buffers greater than two may not
> >> be handled has easily.
> >
> >
> > I would very much prefer support for other layouts without drops
> > before merging this.
> > header by itself can certainly be handled by skipping it.
> > People wanted to use that e.g. for zero copy.
>
> OK fair enough I'll do this now rather than push it out.
>
> >
> > Anything else can be handled by copying the packet.
>
> This though I'm not so sure about. The copy is going to be slow and
> I wonder if someone could craft a packet to cause this if it could
> be used to slow down a system.
Device can always linearize if it wants to. If device is malicious
it's hard for OS to defend itself.
> Also I can't see what would cause this to happen. With mergeable
> buffers and LRO off the num_bufs is either 1 or 2 depending on where
> the header is. Otherwise with LRO off it should be in a single page.
> At least this is the Linux vhost implementation, I guess other
> implementation might meet spec but use num_buf > 2 or multiple pages
> even in the non LRO case.
Me neither but then not a long time ago we always placed
header in a separate entry until we saw the extra s/g has
measureable overhead.
network broken is kind of a heavy handed thing, making debugging
impossible for many people.
> I tend to think dropping the packet out right is better than copying
> it around. At very least if we do this we need to put in warnings so
> users can see something is mis-configured.
>
> .John
Yes, I think that's a good idea.
--
MST
^ permalink raw reply
* Re: [net-next PATCH v2 4/5] virtio_net: add dedicated XDP transmit queues
From: Michael S. Tsirkin @ 2016-11-22 14:59 UTC (permalink / raw)
To: John Fastabend
Cc: daniel, eric.dumazet, kubakici, shm, davem, alexei.starovoitov,
netdev, bblanco, john.r.fastabend, brouer, tgraf
In-Reply-To: <5833FF24.2010800@gmail.com>
On Tue, Nov 22, 2016 at 12:17:40AM -0800, John Fastabend wrote:
> On 16-11-21 03:13 PM, Michael S. Tsirkin wrote:
> > On Sat, Nov 19, 2016 at 06:51:04PM -0800, John Fastabend wrote:
> >> XDP requires using isolated transmit queues to avoid interference
> >> with normal networking stack (BQL, NETDEV_TX_BUSY, etc). This patch
> >> adds a XDP queue per cpu when a XDP program is loaded and does not
> >> expose the queues to the OS via the normal API call to
> >> netif_set_real_num_tx_queues(). This way the stack will never push
> >> an skb to these queues.
> >>
> >> However virtio/vhost/qemu implementation only allows for creating
> >> TX/RX queue pairs at this time so creating only TX queues was not
> >> possible. And because the associated RX queues are being created I
> >> went ahead and exposed these to the stack and let the backend use
> >> them. This creates more RX queues visible to the network stack than
> >> TX queues which is worth mentioning but does not cause any issues as
> >> far as I can tell.
> >>
> >> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
> >
> > FYI what's supposed to happen is packets from the same
> > flow going in the reverse direction will go on the
> > same queue.
> >
> > This might come in handy when implementing RX XDP.
> >
>
> Yeah but if its the first packet not part of a flow then presumably it
> can pick any queue but its worth keeping in mind certainly.
>
> .John
Oh I agree, absolutely. This was just a FYI in case it comes useful
as an optimization down the road.
--
MST
^ permalink raw reply
* Re: [PATCH] net: dsa: mv88e6xxx: egress all frames
From: Andrew Lunn @ 2016-11-22 15:03 UTC (permalink / raw)
To: Stefan Eichenberger
Cc: vivien.didelot, f.fainelli, netdev, Stefan Eichenberger
In-Reply-To: <20161122103944.31381-1-stefan.eichenberger@netmodule.com>
On Tue, Nov 22, 2016 at 11:39:44AM +0100, Stefan Eichenberger wrote:
> Egress multicast and egress unicast is only enabled for CPU/DSA ports
> but for switching operation it seems it should be enabled for all ports.
> Do I miss something here?
>
> I did the following test:
> brctl addbr br0
> brctl addif br0 lan0
> brctl addif br0 lan1
>
> In this scenario the unicast and multicast packets were not forwarded,
> therefore ARP requests were not resolved, and no connection could be
> established.
Hi Stefan
This is probably specific to the 6097 family. It works fine without
this on other devices. Creating a bridge like above and pinging across
it is one of my standard tests. But i only test modern devices like
the 6165, 6352, 6351, 6390 families.
In fact, you might need to review all the code and look where
mv88e6xxx_6095_family(chip) is used and consider if you need to add
mv88e6xxx_6097_family(chip). e.g.
if (mv88e6xxx_6095_family(chip) || mv88e6xxx_6185_family(chip)) {
/* Set the upstream port this port should use */
reg |= dsa_upstream_port(ds);
/* enable forwarding of unknown multicast addresses to
* the upstream port
*/
if (port == dsa_upstream_port(ds))
reg |= PORT_CONTROL_2_FORWARD_UNKNOWN;
}
Maybe this is your problem?
Andrew
^ permalink raw reply
* Re: [patch net-next 0/2] mlxsw: core: Implement thermal zone
From: David Miller @ 2016-11-22 15:04 UTC (permalink / raw)
To: jiri; +Cc: netdev, cera, idosch, eladr, yotamg, nogahf, arkadis, ogerlitz
In-Reply-To: <1479810253-9114-1-git-send-email-jiri@resnulli.us>
From: Jiri Pirko <jiri@resnulli.us>
Date: Tue, 22 Nov 2016 11:24:11 +0100
> Implement thermal zone for mlxsw based HW.
> The first patch is just a register dependency for the second patch.
Looks good, series applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next] net/sched: cls_flower: verify root pointer before dereferncing it
From: Jiri Pirko @ 2016-11-22 14:48 UTC (permalink / raw)
To: Roi Dayan
Cc: David S. Miller, netdev, Jiri Pirko, Cong Wang, Or Gerlitz,
Cong Wang
In-Reply-To: <1479824726-62607-1-git-send-email-roid@mellanox.com>
Tue, Nov 22, 2016 at 03:25:26PM CET, roid@mellanox.com wrote:
>tp->root is being allocated in init() time and kfreed in destroy()
>however it is being dereferenced in classify() path.
>
>We could be in classify() path after destroy() was called and thus
>tp->root is null. Verifying if tp->root is null in classify() path
>is enough because it's being freed with kfree_rcu() and classify()
>path is under rcu_read_lock().
>
>Fixes: 1e052be69d04 ("net_sched: destroy proto tp when all filters are gone")
>Signed-off-by: Roi Dayan <roid@mellanox.com>
>Cc: Cong Wang <cwang@twopensource.com>
This is correct
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
The other way to fix this would be to move tp->ops->destroy call to
call_rcu phase. That would require bigger changes though. net-next
perhaps?
>---
>
>Hi Cong, all
>
>As stated above, the issue was introduced with commit 1e052be69d04 ("net_sched: destroy
>proto tp when all filters are gone"). This patch provides a fix only for cls_flower where
>I succeeded in reproducing the issue. Cong, if you can/want to come up with a fix that
>will be applicable for all the others classifiners, I am fine with that.
>
>Thanks,
>Roi
>
>
> net/sched/cls_flower.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
>index e8dd09a..88a26c4 100644
>--- a/net/sched/cls_flower.c
>+++ b/net/sched/cls_flower.c
>@@ -135,7 +135,7 @@ static int fl_classify(struct sk_buff *skb, const struct tcf_proto *tp,
> struct fl_flow_key skb_mkey;
> struct ip_tunnel_info *info;
>
>- if (!atomic_read(&head->ht.nelems))
>+ if (!head || !atomic_read(&head->ht.nelems))
> return -1;
>
> fl_clear_masked_range(&skb_key, &head->mask);
>--
>2.7.4
>
^ permalink raw reply
* Re: [PATCH] ipv6:ipv6_pinfo dereferenced after NULL check
From: David Miller @ 2016-11-22 15:06 UTC (permalink / raw)
To: hannes
Cc: manjeet.p, kuznet, jmorris, yoshfuji, kaber, netdev, linux-kernel,
pankaj.m, ajeet.y, r.thapliyal
In-Reply-To: <611c167e-cef4-691b-f154-1b6b6aa86e53@stressinduktion.org>
From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Tue, 22 Nov 2016 13:26:45 +0100
> On 22.11.2016 07:27, Manjeet Pawar wrote:
>> From: Rohit Thapliyal <r.thapliyal@samsung.com>
>>
>> np checked for NULL and then dereferenced. It should be modified
>> for NULL case.
>>
>> Signed-off-by: Rohit Thapliyal <r.thapliyal@samsung.com>
>> Signed-off-by: Manjeet Pawar <manjeet.p@samsung.com>
>> ---
>> net/ipv6/ip6_output.c | 9 +++++----
>> 1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
>> index 1dfc402..c2afa14 100644
>> --- a/net/ipv6/ip6_output.c
>> +++ b/net/ipv6/ip6_output.c
>> @@ -205,14 +205,15 @@ int ip6_xmit(const struct sock *sk, struct sk_buff *skb, struct flowi6 *fl6,
>> /*
>> * Fill in the IPv6 header
>> */
>> - if (np)
>> + if (np) {
>> hlimit = np->hop_limit;
>> + ip6_flow_hdr(
>> + hdr, tclass, ip6_make_flowlabel(
>> + net, skb, fl6->flowlabel,
>> + np->autoflowlabel, fl6));
>> + }
>> if (hlimit < 0)
>> hlimit = ip6_dst_hoplimit(dst);
>>
>> - ip6_flow_hdr(hdr, tclass, ip6_make_flowlabel(net, skb, fl6->flowlabel,
>> - np->autoflowlabel, fl6));
>> -
>> hdr->payload_len = htons(seg_len);
>> hdr->nexthdr = proto;
>> hdr->hop_limit = hlimit;
>>
>
>
> We always should initialize hdr and not skip the ip6_flow_hdr call.
>
> Do you saw a bug or did you find this by code review? I wonder if np can
> actually be NULL at this point. Maybe we can just eliminate the NULL check.
Also the indentation is really off.
^ permalink raw reply
* Re: [PATCH] fec: Always write MAC address to controller register
From: David Miller @ 2016-11-22 15:07 UTC (permalink / raw)
To: daniel.krueger; +Cc: fugang.duan, netdev, alexander.stein
In-Reply-To: <038ccd3c-6d77-d9e1-92e1-c41572684812@systec-electronic.com>
This change is already in the tree via commit
b82d44d78480faff7456e9e0999acb9d38666057 made nearly
two months ago:
commit b82d44d78480faff7456e9e0999acb9d38666057
Author: Gavin Schenk <g.schenk@eckelmann.de>
Date: Fri Sep 30 11:46:10 2016 +0200
net: fec: set mac address unconditionally
If the mac address origin is not dt, you can only safely assign a mac
address after "link up" of the device. If the link is off the clocks are
disabled and because of issues assigning registers when clocks are off the
new mac address cannot be written in .ndo_set_mac_address() on some soc's.
This fix sets the mac address unconditionally in fec_restart(...) and
ensures consistency between fec registers and the network layer.
Signed-off-by: Gavin Schenk <g.schenk@eckelmann.de>
Acked-by: Fugang Duan <fugang.duan@nxp.com>
Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Fixes: 9638d19e4816 ("net: fec: add netif status check before set mac address")
Signed-off-by: David S. Miller <davem@davemloft.net>
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 1fa2d87..48a033e 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -913,13 +913,11 @@ fec_restart(struct net_device *ndev)
* enet-mac reset will reset mac address registers too,
* so need to reconfigure it.
*/
- if (fep->quirks & FEC_QUIRK_ENET_MAC) {
- memcpy(&temp_mac, ndev->dev_addr, ETH_ALEN);
- writel((__force u32)cpu_to_be32(temp_mac[0]),
- fep->hwp + FEC_ADDR_LOW);
- writel((__force u32)cpu_to_be32(temp_mac[1]),
- fep->hwp + FEC_ADDR_HIGH);
- }
+ memcpy(&temp_mac, ndev->dev_addr, ETH_ALEN);
+ writel((__force u32)cpu_to_be32(temp_mac[0]),
+ fep->hwp + FEC_ADDR_LOW);
+ writel((__force u32)cpu_to_be32(temp_mac[1]),
+ fep->hwp + FEC_ADDR_HIGH);
/* Clear any outstanding interrupt. */
writel(0xffffffff, fep->hwp + FEC_IEVENT);
^ permalink raw reply related
* Re: [PATCH] net: dsa: mv88e6xxx: add MV88E6097 switch
From: Andrew Lunn @ 2016-11-22 15:07 UTC (permalink / raw)
To: Stefan Eichenberger
Cc: vivien.didelot, f.fainelli, netdev, Stefan Eichenberger
In-Reply-To: <20161122102836.23980-1-stefan.eichenberger@netmodule.com>
On Tue, Nov 22, 2016 at 11:28:36AM +0100, Stefan Eichenberger wrote:
> Add support for the MV88E6097 switch. The change was tested on an Armada
> based platform with a MV88E6097 switch.
Hi Stefan
Please can you based your patches on net-next. You will then find the
ops structure has gained a few more entries.
Andrew
^ permalink raw reply
* Re: [PATCH net-next] marvell: mark mvneta and mvpp2 32-bit only
From: Gregory CLEMENT @ 2016-11-22 15:13 UTC (permalink / raw)
To: Arnd Bergmann
Cc: David S. Miller, Florian Fainelli, Marcin Wojtas, netdev,
linux-kernel
In-Reply-To: <20161122142136.1690096-1-arnd@arndb.de>
Hi Arnd,
On mar., nov. 22 2016, Arnd Bergmann <arnd@arndb.de> wrote:
> Both of these drivers won't work on 64-bit architectures unless they
> are redesigned, since they store a virtual address pointer in a 32-bit
> field of the descriptors:
>
> drivers/net/ethernet/marvell/mvneta_bm.c: In function 'mvneta_bm_construct':
> drivers/net/ethernet/marvell/mvneta_bm.c:103:16: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
> drivers/net/ethernet/marvell/mvpp2.c: In function 'mvpp2_prs_vlan_init':
> drivers/net/ethernet/marvell/mvpp2.c:2563:32: error: large integer implicitly truncated to unsigned type [-Werror=overflow]
>
> This limits the COMPILE_TEST option for the two drivers again to
> only build them on 32-bit. This seems nicer than shutting up the
> warnings, in case we ever actually want to use them on 64-bit,
> as the warnings indicate which parts of the driver are currently
Actually we are using these drivers on 64-bits so obviously there are
not 32 bits only!
For mvneta currently we do not use BM on the 64-bits version. I agree
that there is a problem with mvneta_bm.c when building in 64-bits but it
should not prevent us to use the mvneta driver.
Gregory
> broken there.
>
> Fixes: a0627f776a45 ("net: marvell: Allow drivers to be built with COMPILE_TEST")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/net/ethernet/marvell/Kconfig | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/ethernet/marvell/Kconfig b/drivers/net/ethernet/marvell/Kconfig
> index d74d4e6f0b34..66fd9dbb2ca7 100644
> --- a/drivers/net/ethernet/marvell/Kconfig
> +++ b/drivers/net/ethernet/marvell/Kconfig
> @@ -58,6 +58,7 @@ config MVNETA
> tristate "Marvell Armada 370/38x/XP network interface support"
> depends on PLAT_ORION || COMPILE_TEST
> depends on HAS_DMA
> + depends on !64BIT
> select MVMDIO
> select FIXED_PHY
> ---help---
> @@ -81,6 +82,7 @@ config MVPP2
> tristate "Marvell Armada 375 network interface support"
> depends on MACH_ARMADA_375 || COMPILE_TEST
> depends on HAS_DMA
> + depends on !64BIT
> select MVMDIO
> ---help---
> This driver supports the network interface units in the
> --
> 2.9.0
>
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox