Netdev List
 help / color / mirror / Atom feed
* KASAN: use-after-free Read in rtnetlink_put_metrics
From: syzbot @ 2018-07-31 12:31 UTC (permalink / raw)
  To: christian.brauner, davem, dsahern, fw, jbenc, ktkhai,
	linux-kernel, lucien.xin, netdev, syzkaller-bugs

Hello,

syzbot found the following crash on:

HEAD commit:    61f4b23769f0 netlink: Don't shift with UB on nlk->ngroups
git tree:       net
console output: https://syzkaller.appspot.com/x/log.txt?x=14a9de58400000
kernel config:  https://syzkaller.appspot.com/x/.config?x=ffb4428fdc82f93b
dashboard link: https://syzkaller.appspot.com/bug?extid=41f9c04b50ef70c66947
compiler:       gcc (GCC) 8.0.1 20180413 (experimental)

Unfortunately, I don't have any reproducer for this crash yet.

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+41f9c04b50ef70c66947@syzkaller.appspotmail.com

TCP: request_sock_TCPv6: Possible SYN flooding on port 20002. Sending  
cookies.  Check SNMP counters.
==================================================================
BUG: KASAN: use-after-free in rtnetlink_put_metrics+0x621/0x690  
net/core/rtnetlink.c:754
Read of size 4 at addr ffff8801b3cd8b00 by task udevd/2613

CPU: 0 PID: 2613 Comm: udevd Not tainted 4.18.0-rc6+ #34
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Call Trace:
  <IRQ>
  __dump_stack lib/dump_stack.c:77 [inline]
  dump_stack+0x1c9/0x2b4 lib/dump_stack.c:113
  print_address_description+0x6c/0x20b mm/kasan/report.c:256
  kasan_report_error mm/kasan/report.c:354 [inline]
  kasan_report.cold.7+0x242/0x2fe mm/kasan/report.c:412
  __asan_report_load4_noabort+0x14/0x20 mm/kasan/report.c:432
  rtnetlink_put_metrics+0x621/0x690 net/core/rtnetlink.c:754
  rt6_fill_node+0x7d9/0x1540 net/ipv6/route.c:4753
  inet6_rt_notify+0x161/0x2c0 net/ipv6/route.c:4985
  fib6_del_route net/ipv6/ip6_fib.c:1788 [inline]
  fib6_del+0xf4d/0x1310 net/ipv6/ip6_fib.c:1815
  fib6_clean_node+0x3ee/0x5e0 net/ipv6/ip6_fib.c:1976
  fib6_walk_continue+0x4b1/0x8e0 net/ipv6/ip6_fib.c:1899
  fib6_walk+0x95/0xf0 net/ipv6/ip6_fib.c:1947
  fib6_clean_tree+0x1ea/0x360 net/ipv6/ip6_fib.c:2024
  __fib6_clean_all+0x21c/0x420 net/ipv6/ip6_fib.c:2040
  fib6_clean_all net/ipv6/ip6_fib.c:2051 [inline]
  fib6_run_gc+0x182/0x3d0 net/ipv6/ip6_fib.c:2107
  fib6_gc_timer_cb+0x20/0x30 net/ipv6/ip6_fib.c:2124
  call_timer_fn+0x242/0x970 kernel/time/timer.c:1326
  expire_timers kernel/time/timer.c:1363 [inline]
  __run_timers+0x7a6/0xc70 kernel/time/timer.c:1666
  run_timer_softirq+0x4c/0x70 kernel/time/timer.c:1692
  __do_softirq+0x2e8/0xb17 kernel/softirq.c:292
  invoke_softirq kernel/softirq.c:372 [inline]
  irq_exit+0x1d4/0x210 kernel/softirq.c:412
  exiting_irq arch/x86/include/asm/apic.h:527 [inline]
  smp_apic_timer_interrupt+0x186/0x730 arch/x86/kernel/apic/apic.c:1052
  apic_timer_interrupt+0xf/0x20 arch/x86/entry/entry_64.S:863
  </IRQ>
RIP: 0010:arch_local_irq_restore arch/x86/include/asm/paravirt.h:783  
[inline]
RIP: 0010:qlink_free mm/kasan/quarantine.c:150 [inline]
RIP: 0010:qlist_free_all+0xf8/0x160 mm/kasan/quarantine.c:166
Code: c7 40 10 00 00 00 00 48 83 c4 10 5b 41 5c 41 5d 41 5e 41 5f 5d c3 e8  
57 b8 a4 ff 48 83 3d 67 b4 37 06 00 74 56 48 89 df 57 9d <0f> 1f 44 00 00  
eb af ba 00 00 00 80 48 01 c2 72 43 48 b9 00 00 00
RSP: 0018:ffff8801b54a7b08 EFLAGS: 00000286 ORIG_RAX: ffffffffffffff13
RAX: 0000000000000007 RBX: 0000000000000286 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffff8801b5498a38 RDI: 0000000000000286
RBP: ffff8801b54a7b40 R08: ffff8801b5498a38 R09: 0000000000000006
R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
R13: ffff8801dad85dc0 R14: ffff8801b37ace00 R15: ffffffff87f1b0a0
  quarantine_reduce+0x163/0x1a0 mm/kasan/quarantine.c:259
  kasan_kmalloc+0x99/0xe0 mm/kasan/kasan.c:538
  kasan_slab_alloc+0x12/0x20 mm/kasan/kasan.c:490
  slab_post_alloc_hook mm/slab.h:444 [inline]
  slab_alloc mm/slab.c:3392 [inline]
  kmem_cache_alloc+0x11b/0x760 mm/slab.c:3552
  getname_flags+0xd0/0x5a0 fs/namei.c:140
  user_path_at_empty+0x2d/0x50 fs/namei.c:2584
  do_readlinkat+0x14b/0x400 fs/stat.c:394
  __do_sys_readlink fs/stat.c:427 [inline]
  __se_sys_readlink fs/stat.c:424 [inline]
  __x64_sys_readlink+0x78/0xb0 fs/stat.c:424
  do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
  entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x7efd8271b577
Code: f0 ff ff 77 02 f3 c3 48 8b 15 bd 38 2b 00 f7 d8 64 89 02 83 c8 ff c3  
90 90 90 90 90 90 90 90 90 90 90 90 b8 59 00 00 00 0f 05 <48> 3d 01 f0 ff  
ff 73 01 c3 48 8b 0d 91 38 2b 00 31 d2 48 29 c2 64
RSP: 002b:00007ffe27d23688 EFLAGS: 00000246 ORIG_RAX: 0000000000000059
RAX: ffffffffffffffda RBX: 00000000021de250 RCX: 00007efd8271b577
RDX: 0000000000000400 RSI: 00007ffe27d23690 RDI: 00007ffe27d23b70
RBP: 00007ffe27d243b0 R08: 00007ffe27d243b0 R09: 00007efd8276fdc0
R10: 7665642f7379732f R11: 0000000000000246 R12: 00007ffe27d23b70
R13: 0000000000000400 R14: 00000000021de250 R15: 00000000021e3e10

Allocated by task 8953:
  save_stack+0x43/0xd0 mm/kasan/kasan.c:448
  set_track mm/kasan/kasan.c:460 [inline]
  kasan_kmalloc+0xc4/0xe0 mm/kasan/kasan.c:553
  kmem_cache_alloc_trace+0x152/0x780 mm/slab.c:3620
  kmalloc include/linux/slab.h:513 [inline]
  kzalloc include/linux/slab.h:707 [inline]
  fib6_metric_set+0x163/0x2c0 net/ipv6/ip6_fib.c:645
  fib6_add_rt2node+0xe36/0x27f0 net/ipv6/ip6_fib.c:1000
  fib6_add+0xaae/0x14d0 net/ipv6/ip6_fib.c:1308
  __ip6_ins_rt+0x54/0x80 net/ipv6/route.c:1163
  ip6_route_add+0x6d/0xc0 net/ipv6/route.c:3171
  addrconf_prefix_route.isra.48+0x51d/0x720 net/ipv6/addrconf.c:2347
  inet6_addr_modify net/ipv6/addrconf.c:4627 [inline]
  inet6_rtm_newaddr+0x112e/0x1b50 net/ipv6/addrconf.c:4743
  rtnetlink_rcv_msg+0x46e/0xc30 net/core/rtnetlink.c:4665
  netlink_rcv_skb+0x172/0x440 net/netlink/af_netlink.c:2453
  rtnetlink_rcv+0x1c/0x20 net/core/rtnetlink.c:4683
  netlink_unicast_kernel net/netlink/af_netlink.c:1315 [inline]
  netlink_unicast+0x5a0/0x760 net/netlink/af_netlink.c:1341
  netlink_sendmsg+0xa18/0xfd0 net/netlink/af_netlink.c:1906
  sock_sendmsg_nosec net/socket.c:642 [inline]
  sock_sendmsg+0xd5/0x120 net/socket.c:652
  ___sys_sendmsg+0x7fd/0x930 net/socket.c:2126
  __sys_sendmsg+0x11d/0x290 net/socket.c:2164
  __do_sys_sendmsg net/socket.c:2173 [inline]
  __se_sys_sendmsg net/socket.c:2171 [inline]
  __x64_sys_sendmsg+0x78/0xb0 net/socket.c:2171
  do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
  entry_SYSCALL_64_after_hwframe+0x49/0xbe

Freed by task 2613:
  save_stack+0x43/0xd0 mm/kasan/kasan.c:448
  set_track mm/kasan/kasan.c:460 [inline]
  __kasan_slab_free+0x11a/0x170 mm/kasan/kasan.c:521
  kasan_slab_free+0xe/0x10 mm/kasan/kasan.c:528
  __cache_free mm/slab.c:3498 [inline]
  kfree+0xd9/0x260 mm/slab.c:3813
  fib6_metrics_release+0x77/0x90 net/ipv6/ip6_fib.c:179
  fib6_drop_pcpu_from net/ipv6/ip6_fib.c:899 [inline]
  fib6_purge_rt+0x5ec/0x7f0 net/ipv6/ip6_fib.c:934
  fib6_del_route net/ipv6/ip6_fib.c:1784 [inline]
  fib6_del+0xc11/0x1310 net/ipv6/ip6_fib.c:1815
  fib6_clean_node+0x3ee/0x5e0 net/ipv6/ip6_fib.c:1976
  fib6_walk_continue+0x4b1/0x8e0 net/ipv6/ip6_fib.c:1899
  fib6_walk+0x95/0xf0 net/ipv6/ip6_fib.c:1947
  fib6_clean_tree+0x1ea/0x360 net/ipv6/ip6_fib.c:2024
  __fib6_clean_all+0x21c/0x420 net/ipv6/ip6_fib.c:2040
  fib6_clean_all net/ipv6/ip6_fib.c:2051 [inline]
  fib6_run_gc+0x182/0x3d0 net/ipv6/ip6_fib.c:2107
  fib6_gc_timer_cb+0x20/0x30 net/ipv6/ip6_fib.c:2124
  call_timer_fn+0x242/0x970 kernel/time/timer.c:1326
  expire_timers kernel/time/timer.c:1363 [inline]
  __run_timers+0x7a6/0xc70 kernel/time/timer.c:1666
  run_timer_softirq+0x4c/0x70 kernel/time/timer.c:1692
  __do_softirq+0x2e8/0xb17 kernel/softirq.c:292

The buggy address belongs to the object at ffff8801b3cd8b00
  which belongs to the cache kmalloc-96 of size 96
The buggy address is located 0 bytes inside of
  96-byte region [ffff8801b3cd8b00, ffff8801b3cd8b60)
The buggy address belongs to the page:
page:ffffea0006cf3600 count:1 mapcount:0 mapping:ffff8801dac004c0 index:0x0
flags: 0x2fffc0000000100(slab)
raw: 02fffc0000000100 ffffea0006a6f3c8 ffffea0007018608 ffff8801dac004c0
raw: 0000000000000000 ffff8801b3cd8000 0000000100000020 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
  ffff8801b3cd8a00: fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
  ffff8801b3cd8a80: fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
> ffff8801b3cd8b00: fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
                    ^
  ffff8801b3cd8b80: fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
  ffff8801b3cd8c00: fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
==================================================================


---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with  
syzbot.

^ permalink raw reply

* WARNING: ODEBUG bug in enqueue_hrtimer
From: syzbot @ 2018-07-31 12:31 UTC (permalink / raw)
  To: davem, linux-can, linux-kernel, mkl, netdev, socketcan,
	syzkaller-bugs

Hello,

syzbot found the following crash on:

HEAD commit:    527838d470e3 Merge branch 'x86-urgent-for-linus' of git://..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=15e90362400000
kernel config:  https://syzkaller.appspot.com/x/.config?x=2dc0cd7c2eefb46f
dashboard link: https://syzkaller.appspot.com/bug?extid=3c31c6798616dc2d9ee6
compiler:       gcc (GCC) 8.0.1 20180413 (experimental)

Unfortunately, I don't have any reproducer for this crash yet.

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+3c31c6798616dc2d9ee6@syzkaller.appspotmail.com

netlink: 20 bytes leftover after parsing attributes in process  
`syz-executor3'.
------------[ cut here ]------------
ODEBUG: activate not available (active state 0) object type: hrtimer hint:  
bcm_tx_timeout_handler+0x0/0x60 net/can/bcm.c:241
WARNING: CPU: 1 PID: 18 at lib/debugobjects.c:329  
debug_print_object+0x16a/0x210 lib/debugobjects.c:326
Kernel panic - not syncing: panic_on_warn set ...

CPU: 1 PID: 18 Comm: ksoftirqd/1 Not tainted 4.18.0-rc7+ #170
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+0x1c9/0x2b4 lib/dump_stack.c:113
  panic+0x238/0x4e7 kernel/panic.c:184
  __warn.cold.8+0x163/0x1ba kernel/panic.c:536
  report_bug+0x252/0x2d0 lib/bug.c:186
  fixup_bug arch/x86/kernel/traps.c:178 [inline]
  do_error_trap+0x1fc/0x4d0 arch/x86/kernel/traps.c:296
  do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:316
  invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:992
RIP: 0010:debug_print_object+0x16a/0x210 lib/debugobjects.c:326
Code: 3a 87 48 89 fa 48 c1 ea 03 80 3c 02 00 0f 85 92 00 00 00 48 8b 14 dd  
20 74 3a 87 4c 89 f6 48 c7 c7 c0 69 3a 87 e8 c6 b0 e6 fd <0f> 0b 83 05 a9  
e6 29 05 01 48 83 c4 18 5b 41 5c 41 5d 41 5e 41 5f
RSP: 0018:ffff8801d9f27680 EFLAGS: 00010086
RAX: 0000000000000000 RBX: 0000000000000005 RCX: 0000000000000000
RDX: 0000000000000100 RSI: ffffffff81632481 RDI: 0000000000000001
RBP: ffff8801d9f276c0 R08: ffff8801d9f0c4c0 R09: ffffed003b623ec2
R10: ffffed003b623ec2 R11: ffff8801db11f617 R12: 0000000000000001
R13: ffffffff87fa0660 R14: ffffffff873a7040 R15: ffffffff816a48f0
  debug_object_activate+0x359/0x690 lib/debugobjects.c:513
  debug_hrtimer_activate kernel/time/hrtimer.c:416 [inline]
  debug_activate kernel/time/hrtimer.c:465 [inline]
  enqueue_hrtimer+0x9e/0x540 kernel/time/hrtimer.c:954
  __hrtimer_start_range_ns kernel/time/hrtimer.c:1089 [inline]
  hrtimer_start_range_ns+0x616/0xd20 kernel/time/hrtimer.c:1115
  hrtimer_start include/linux/hrtimer.h:398 [inline]
  bcm_tx_start_timer+0x11d/0x1b0 net/can/bcm.c:361
  bcm_tx_timeout_tsklet+0x1ab/0x4b0 net/can/bcm.c:392
  tasklet_action_common.isra.19+0x26f/0x720 kernel/softirq.c:522
  tasklet_action+0x1d/0x20 kernel/softirq.c:540
  __do_softirq+0x2e8/0xb17 kernel/softirq.c:292
  run_ksoftirqd+0x86/0x100 kernel/softirq.c:653
  smpboot_thread_fn+0x417/0x870 kernel/smpboot.c:164
  kthread+0x345/0x410 kernel/kthread.c:246
  ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:412

======================================================
WARNING: possible circular locking dependency detected
4.18.0-rc7+ #170 Not tainted
------------------------------------------------------
ksoftirqd/1/18 is trying to acquire lock:
000000006298a4a7 ((console_sem).lock){-.-.}, at: down_trylock+0x13/0x70  
kernel/locking/semaphore.c:136

but task is already holding lock:
0000000018a38b75 (hrtimer_bases.lock){-.-.}, at:  
lock_hrtimer_base.isra.18+0x75/0x130 kernel/time/hrtimer.c:174

which lock already depends on the new lock.


the existing dependency chain (in reverse order) is:

-> #4 (hrtimer_bases.lock){-.-.}:
        __raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline]
        _raw_spin_lock_irqsave+0x96/0xc0 kernel/locking/spinlock.c:152
        lock_hrtimer_base.isra.18+0x75/0x130 kernel/time/hrtimer.c:174
        hrtimer_start_range_ns+0x128/0xd20 kernel/time/hrtimer.c:1113
        hrtimer_start_expires include/linux/hrtimer.h:412 [inline]
        start_rt_bandwidth kernel/sched/rt.c:68 [inline]
        inc_rt_group kernel/sched/rt.c:1147 [inline]
        inc_rt_tasks kernel/sched/rt.c:1191 [inline]
        __enqueue_rt_entity kernel/sched/rt.c:1261 [inline]
        enqueue_rt_entity kernel/sched/rt.c:1305 [inline]
        enqueue_task_rt+0x96a/0xfd0 kernel/sched/rt.c:1335
        enqueue_task+0xa2/0x1d0 kernel/sched/core.c:750
        __sched_setscheduler+0xe80/0x20b0 kernel/sched/core.c:4365
        _sched_setscheduler+0x20c/0x370 kernel/sched/core.c:4402
        sched_setscheduler+0xe/0x10 kernel/sched/core.c:4417
        watchdog_set_prio kernel/watchdog.c:455 [inline]
        watchdog_enable+0x12d/0x1a0 kernel/watchdog.c:477
        smpboot_thread_fn+0x4c0/0x870 kernel/smpboot.c:145
        kthread+0x345/0x410 kernel/kthread.c:246
        ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:412

-> #3 (&rt_b->rt_runtime_lock){-.-.}:
        __raw_spin_lock include/linux/spinlock_api_smp.h:142 [inline]
        _raw_spin_lock+0x2a/0x40 kernel/locking/spinlock.c:144
        start_rt_bandwidth kernel/sched/rt.c:56 [inline]
        inc_rt_group kernel/sched/rt.c:1147 [inline]
        inc_rt_tasks kernel/sched/rt.c:1191 [inline]
        __enqueue_rt_entity kernel/sched/rt.c:1261 [inline]
        enqueue_rt_entity kernel/sched/rt.c:1305 [inline]
        enqueue_task_rt+0x618/0xfd0 kernel/sched/rt.c:1335
        enqueue_task+0xa2/0x1d0 kernel/sched/core.c:750
        __sched_setscheduler+0xe80/0x20b0 kernel/sched/core.c:4365
        _sched_setscheduler+0x20c/0x370 kernel/sched/core.c:4402
        sched_setscheduler+0xe/0x10 kernel/sched/core.c:4417
        watchdog_set_prio kernel/watchdog.c:455 [inline]
        watchdog_enable+0x12d/0x1a0 kernel/watchdog.c:477
        smpboot_thread_fn+0x4c0/0x870 kernel/smpboot.c:145
        kthread+0x345/0x410 kernel/kthread.c:246
        ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:412

-> #2 (&rq->lock){-.-.}:
        __raw_spin_lock include/linux/spinlock_api_smp.h:142 [inline]
        _raw_spin_lock+0x2a/0x40 kernel/locking/spinlock.c:144
        rq_lock kernel/sched/sched.h:1812 [inline]
        task_fork_fair+0x93/0x680 kernel/sched/fair.c:9952
        sched_fork+0x446/0xb40 kernel/sched/core.c:2381
        copy_process.part.39+0x1bf5/0x70b0 kernel/fork.c:1796
        copy_process kernel/fork.c:1639 [inline]
        _do_fork+0x291/0x12a0 kernel/fork.c:2122
        kernel_thread+0x34/0x40 kernel/fork.c:2181
        rest_init+0x22/0xe4 init/main.c:408
        start_kernel+0x90e/0x949 init/main.c:738
        x86_64_start_reservations+0x29/0x2b arch/x86/kernel/head64.c:452
        x86_64_start_kernel+0x76/0x79 arch/x86/kernel/head64.c:433
        secondary_startup_64+0xa5/0xb0 arch/x86/kernel/head_64.S:242

-> #1 (&p->pi_lock){-.-.}:
        __raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline]
        _raw_spin_lock_irqsave+0x96/0xc0 kernel/locking/spinlock.c:152
        try_to_wake_up+0xd2/0x12a0 kernel/sched/core.c:1985
        wake_up_process+0x10/0x20 kernel/sched/core.c:2148
        __up.isra.1+0x1c0/0x2a0 kernel/locking/semaphore.c:262
        up+0x13c/0x1c0 kernel/locking/semaphore.c:187
        __up_console_sem+0xbe/0x1b0 kernel/printk/printk.c:242
        console_unlock+0x7a2/0x10b0 kernel/printk/printk.c:2411
        vprintk_emit+0x6c6/0xdf0 kernel/printk/printk.c:1907
        vprintk_default+0x28/0x30 kernel/printk/printk.c:1948
        vprintk_func+0x7a/0xe7 kernel/printk/printk_safe.c:382
        printk+0xa7/0xcf kernel/printk/printk.c:1981
        load_umh+0x51/0xbd net/bpfilter/bpfilter_kern.c:98
        do_one_initcall+0x127/0x913 init/main.c:884
        do_initcall_level init/main.c:952 [inline]
        do_initcalls init/main.c:960 [inline]
        do_basic_setup init/main.c:978 [inline]
        kernel_init_freeable+0x49b/0x58e init/main.c:1135
        kernel_init+0x11/0x1b3 init/main.c:1061
        ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:412

-> #0 ((console_sem).lock){-.-.}:
        lock_acquire+0x1e4/0x540 kernel/locking/lockdep.c:3924
        __raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline]
        _raw_spin_lock_irqsave+0x96/0xc0 kernel/locking/spinlock.c:152
        down_trylock+0x13/0x70 kernel/locking/semaphore.c:136
        __down_trylock_console_sem+0xae/0x200 kernel/printk/printk.c:225
        console_trylock+0x15/0xa0 kernel/printk/printk.c:2230
        console_trylock_spinning kernel/printk/printk.c:1643 [inline]
        vprintk_emit+0x6ad/0xdf0 kernel/printk/printk.c:1906
        vprintk_default+0x28/0x30 kernel/printk/printk.c:1948
        vprintk_func+0x7a/0xe7 kernel/printk/printk_safe.c:382
        printk+0xa7/0xcf kernel/printk/printk.c:1981
        __warn_printk+0x8c/0xe0 kernel/panic.c:590
        debug_print_object+0x16a/0x210 lib/debugobjects.c:326
        debug_object_activate+0x359/0x690 lib/debugobjects.c:513
        debug_hrtimer_activate kernel/time/hrtimer.c:416 [inline]
        debug_activate kernel/time/hrtimer.c:465 [inline]
        enqueue_hrtimer+0x9e/0x540 kernel/time/hrtimer.c:954
        __hrtimer_start_range_ns kernel/time/hrtimer.c:1089 [inline]
        hrtimer_start_range_ns+0x616/0xd20 kernel/time/hrtimer.c:1115
        hrtimer_start include/linux/hrtimer.h:398 [inline]
        bcm_tx_start_timer+0x11d/0x1b0 net/can/bcm.c:361
        bcm_tx_timeout_tsklet+0x1ab/0x4b0 net/can/bcm.c:392
        tasklet_action_common.isra.19+0x26f/0x720 kernel/softirq.c:522
        tasklet_action+0x1d/0x20 kernel/softirq.c:540
        __do_softirq+0x2e8/0xb17 kernel/softirq.c:292
        run_ksoftirqd+0x86/0x100 kernel/softirq.c:653
        smpboot_thread_fn+0x417/0x870 kernel/smpboot.c:164
        kthread+0x345/0x410 kernel/kthread.c:246
        ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:412

other info that might help us debug this:

Chain exists of:
   (console_sem).lock --> &rt_b->rt_runtime_lock --> hrtimer_bases.lock

  Possible unsafe locking scenario:

        CPU0                    CPU1
        ----                    ----
   lock(hrtimer_bases.lock);
                                lock(&rt_b->rt_runtime_lock);
                                lock(hrtimer_bases.lock);
   lock((console_sem).lock);

  *** DEADLOCK ***

1 lock held by ksoftirqd/1/18:
  #0: 0000000018a38b75 (hrtimer_bases.lock){-.-.}, at:  
lock_hrtimer_base.isra.18+0x75/0x130 kernel/time/hrtimer.c:174

stack backtrace:
CPU: 1 PID: 18 Comm: ksoftirqd/1 Not tainted 4.18.0-rc7+ #170
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+0x1c9/0x2b4 lib/dump_stack.c:113
  print_circular_bug.isra.36.cold.57+0x1bd/0x27d  
kernel/locking/lockdep.c:1227
  check_prev_add kernel/locking/lockdep.c:1867 [inline]
  check_prevs_add kernel/locking/lockdep.c:1980 [inline]
  validate_chain kernel/locking/lockdep.c:2421 [inline]
  __lock_acquire+0x3449/0x5020 kernel/locking/lockdep.c:3435
  lock_acquire+0x1e4/0x540 kernel/locking/lockdep.c:3924
  __raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline]
  _raw_spin_lock_irqsave+0x96/0xc0 kernel/locking/spinlock.c:152
  down_trylock+0x13/0x70 kernel/locking/semaphore.c:136
  __down_trylock_console_sem+0xae/0x200 kernel/printk/printk.c:225
  console_trylock+0x15/0xa0 kernel/printk/printk.c:2230
  console_trylock_spinning kernel/printk/printk.c:1643 [inline]
  vprintk_emit+0x6ad/0xdf0 kernel/printk/printk.c:1906
  vprintk_default+0x28/0x30 kernel/printk/printk.c:1948
  vprintk_func+0x7a/0xe7 kernel/printk/printk_safe.c:382
  printk+0xa7/0xcf kernel/printk/printk.c:1981
  __warn_printk+0x8c/0xe0 kernel/panic.c:590
  debug_print_object+0x16a/0x210 lib/debugobjects.c:326
  debug_object_activate+0x359/0x690 lib/debugobjects.c:513
  debug_hrtimer_activate kernel/time/hrtimer.c:416 [inline]
  debug_activate kernel/time/hrtimer.c:465 [inline]
  enqueue_hrtimer+0x9e/0x540 kernel/time/hrtimer.c:954
  __hrtimer_start_range_ns kernel/time/hrtimer.c:1089 [inline]
  hrtimer_start_range_ns+0x616/0xd20 kernel/time/hrtimer.c:1115
  hrtimer_start include/linux/hrtimer.h:398 [inline]
  bcm_tx_start_timer+0x11d/0x1b0 net/can/bcm.c:361
  bcm_tx_timeout_tsklet+0x1ab/0x4b0 net/can/bcm.c:392
  tasklet_action_common.isra.19+0x26f/0x720 kernel/softirq.c:522
  tasklet_action+0x1d/0x20 kernel/softirq.c:540
  __do_softirq+0x2e8/0xb17 kernel/softirq.c:292
  run_ksoftirqd+0x86/0x100 kernel/softirq.c:653
  smpboot_thread_fn+0x417/0x870 kernel/smpboot.c:164
  kthread+0x345/0x410 kernel/kthread.c:246
  ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:412
Shutting down cpus with NMI
Dumping ftrace buffer:
    (ftrace buffer empty)
Kernel Offset: disabled
Rebooting in 86400 seconds..


---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with  
syzbot.

^ permalink raw reply

* RE: Security enhancement proposal for kernel TLS
From: Vakul Garg @ 2018-07-31 10:45 UTC (permalink / raw)
  To: Dave Watson; +Cc: netdev@vger.kernel.org, Peter Doliwa, Boris Pismenny
In-Reply-To: <20180730211611.GA48199@glawler-mbp.dhcp.thefacebook.com>



> -----Original Message-----
> From: Dave Watson [mailto:davejwatson@fb.com]
> Sent: Tuesday, July 31, 2018 2:46 AM
> To: Vakul Garg <vakul.garg@nxp.com>
> Cc: netdev@vger.kernel.org; Peter Doliwa <peter.doliwa@nxp.com>; Boris
> Pismenny <borisp@mellanox.com>
> Subject: Re: Security enhancement proposal for kernel TLS
> 
> On 07/30/18 06:31 AM, Vakul Garg wrote:
> > > It's not entirely clear how your TLS handshake daemon works -   Why is
> > > it necessary to set the keys in the kernel tls socket before the
> > > handshake is completed?
> >
> > IIUC, with the upstream implementation of tls record layer in kernel,
> > the decryption of tls FINISHED message happens in kernel. Therefore
> > the keys are already being sent to kernel tls socket before handshake is
> completed.
> 
> This is incorrect.  

Let us first reach a common ground on this.

 The kernel TLS implementation can decrypt only after setting the keys on the socket.
The TLS message 'finished' (which is encrypted) is received after receiving 'CCS'
message. After the user space  TLS library receives CCS message, it sets the keys
on kernel TLS socket. Therefore, the next message in the  socket receive queue
which is TLS finished gets decrypted in kernel only.

Please refer to following Boris's patch on openssl. The  commit log says:
" We choose to set this option at the earliest - just after CCS is complete".

------------------------------------------------------
commit a01dd062a32c687630b2a860b4bb053008f09ff5
Author: Boris Pismenny <borisp@mellanox.com>
Date:   Sun Mar 11 16:18:27 2018 +0200

    ssl: Linux TLS Rx Offload
    
    This patch adds support for the Linux TLS Rx socket option.
    It completes the previous patch for TLS Tx offload.
    If the socket option is successful, then the receive data-path of the TCP
    socket is implemented by the kernel.
    We choose to set this option at the earliest - just after CCS is complete.
------------------------------------------------------

The  fact that keys are handed over to kernel TLS socket can also be verified
by putting a log in tls_sw_recvmsg().

I would stop here for you to confirm my observation first. 
Regards. Vakul


 > Currently the kernel TLS implementation decrypts
> everything after you set the keys on the socket.  I'm suggesting that you
> don't set the keys on the socket until after the FINISHED message.
> 
> > > Or, why do you need to hand off the fd to the client program before
> > > the handshake is completed?
> >
> > The fd is always owned by the client program..
> >
> > In my proposal, the applications poll their own tcp socket using
> read/recvmsg etc.
> > If they get handshake record, they forward it to the entity running
> handshake agent.
> > The handshake agent could be a linux daemon or could run on a separate
> > security processor like 'Secure element' or say arm trustzone etc. The
> > applications forward any handshake message it gets backs from
> > handshake agent to the connected tcp socket. Therefore, the
> > applications act as a forwarder of the handshake messages between the
> peer tls endpoint and handshake agent.
> > The received data messages are absorbed by the applications themselves
> > (bypassing ssl stack completely). Similarly, the applications tx data directly
> by writing on their socket.
> >
> > > Waiting until after handshake solves both of these issues.
> >
> > The security sensitive check which is 'Wait for handshake to finish
> > completely before accepting data' should not be the onus of the
> > application. We have enough examples in past where application
> > programmers made mistakes in setting up tls correctly. The idea is to
> isolate tls session setting up from the applications.
> 
> It's not clear to me what you gain by putting this 'handshake finished'
> notification in the kernel instead of in the client's tls library - you're already
> forwarding the handshake start notification to the daemon, why can't the
> daemon notify them back in userspace that
> the handshake is finished?
> 
> If you did want to put the notification in the kernel, how would you handle
> poll on the socket, since probably both the handshake daemon and client
> might be polling the socket, but one for control messages and one for data?
> 
> The original kernel TLS RFC did split these to two separate sockets, but we
> decided it was too complicated, and that's not how userspace TLS clients
> function today.
> 
> Do you have an implementation of this?  There are a bunch of tricky corner
> cases here, it might make more sense to have something concrete to discuss.
> 
> > Further, as per tls RFC it is ok to piggyback the data records after
> > the finished handshake message. This is called early data. But then it
> > is the responsibility of applications to first complete finished message
> processing before accepting the data records.
> >
> > The proposal is to disallow application world seeing data records
> > before handshake finishes.
> 
> You're talking about the TLS 1.3 0-RTT feature, which is indeed an interesting
> case.  For in-process TLS libraries, it's fairly easy to punt, and don't set the
> kernel TLS keys until after the 0-RTT data + handshake message.  For an OOB
> handshake daemon it might indeed make more sense to leave the data in
> kernelspace ... somehow.
> 
> > > > 	- The handshake state should fallback to 'unverified' in case a
> > > > control
> > > record is seen again by kernel TLS (e.g. in case of renegotiation,
> > > post handshake client auth etc).
> > >
> > > Currently kernel tls sockets return an error unless you explicitly
> > > handle the control record for exactly this reason.
> >
> > IIRC, any kind handshake message post handshake-completion is a problem
> for kernel tls.
> > This includes renegotiation, post handshake client-auth etc.
> >
> > Please correct me if I am wrong.
> 
> You are correct, but currently kernel TLS sockets return an error unless you
> explicitly handle the control message.  This should be enough already to
> implement your proposal.

^ permalink raw reply

* [PULL] vhost: last-minute fixes
From: Michael S. Tsirkin @ 2018-07-31 12:21 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: kvm, mst, netdev, linux-kernel, stable, virtualization,
	huang.chong, jiang.biao2

The following changes since commit d72e90f33aa4709ebecc5005562f52335e106a60:

  Linux 4.18-rc6 (2018-07-22 14:12:20 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git tags/for_linus

for you to fetch changes up to 89da619bc18d79bca5304724c11d4ba3b67ce2c6:

  virtio_balloon: fix another race between migration and ballooning (2018-07-30 16:45:33 +0300)

----------------------------------------------------------------
virtio: last-minute fixes

Some bugfixes that seem important and safe enough to merge at the last
minute.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

----------------------------------------------------------------
Jiang Biao (1):
      virtio_balloon: fix another race between migration and ballooning

Michael S. Tsirkin (2):
      tools/virtio: add dma barrier stubs
      tools/virtio: add kmalloc_array stub

 drivers/virtio/virtio_balloon.c | 2 ++
 tools/virtio/asm/barrier.h      | 4 ++--
 tools/virtio/linux/kernel.h     | 5 +++++
 3 files changed, 9 insertions(+), 2 deletions(-)

^ permalink raw reply

* Re: [PATCH v6 bpf-next 4/9] veth: Handle xdp_frames in xdp napi ring
From: Toshiaki Makita @ 2018-07-31 10:40 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: Alexei Starovoitov, Daniel Borkmann, netdev, Jakub Kicinski,
	John Fastabend, Karlsson, Magnus, Björn Töpel
In-Reply-To: <20180731122603.27355719@redhat.com>

On 2018/07/31 19:26, Jesper Dangaard Brouer wrote:
> 
> Context needed from: [PATCH v6 bpf-next 2/9] veth: Add driver XDP
> 
> On Mon, 30 Jul 2018 19:43:44 +0900
> Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> wrote:
> 
>> +static struct sk_buff *veth_build_skb(void *head, int headroom, int len,
>> +				      int buflen)
>> +{
>> +	struct sk_buff *skb;
>> +
>> +	if (!buflen) {
>> +		buflen = SKB_DATA_ALIGN(headroom + len) +
>> +			 SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
>> +	}
>> +	skb = build_skb(head, buflen);
>> +	if (!skb)
>> +		return NULL;
>> +
>> +	skb_reserve(skb, headroom);
>> +	skb_put(skb, len);
>> +
>> +	return skb;
>> +}
> 
> 
> On Mon, 30 Jul 2018 19:43:46 +0900
> Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> wrote:
> 
>> +static struct sk_buff *veth_xdp_rcv_one(struct veth_priv *priv,
>> +					struct xdp_frame *frame)
>> +{
>> +	int len = frame->len, delta = 0;
>> +	struct bpf_prog *xdp_prog;
>> +	unsigned int headroom;
>> +	struct sk_buff *skb;
>> +
>> +	rcu_read_lock();
>> +	xdp_prog = rcu_dereference(priv->xdp_prog);
>> +	if (likely(xdp_prog)) {
>> +		struct xdp_buff xdp;
>> +		u32 act;
>> +
>> +		xdp.data_hard_start = frame->data - frame->headroom;
>> +		xdp.data = frame->data;
>> +		xdp.data_end = frame->data + frame->len;
>> +		xdp.data_meta = frame->data - frame->metasize;
>> +		xdp.rxq = &priv->xdp_rxq;
>> +
>> +		act = bpf_prog_run_xdp(xdp_prog, &xdp);
>> +
>> +		switch (act) {
>> +		case XDP_PASS:
>> +			delta = frame->data - xdp.data;
>> +			len = xdp.data_end - xdp.data;
>> +			break;
>> +		default:
>> +			bpf_warn_invalid_xdp_action(act);
>> +		case XDP_ABORTED:
>> +			trace_xdp_exception(priv->dev, xdp_prog, act);
>> +		case XDP_DROP:
>> +			goto err_xdp;
>> +		}
>> +	}
>> +	rcu_read_unlock();
>> +
>> +	headroom = frame->data - delta - (void *)frame;
>> +	skb = veth_build_skb(frame, headroom, len, 0);
> 
> Here you are adding an assumption that struct xdp_frame is always
> located in-the-top of the packet-data area.  I tried hard not to add
> such a dependency!  You can calculate the beginning of the frame from
> the xdp_frame->data pointer.
> 
> Why not add such a dependency?  Because for AF_XDP zero-copy, we cannot
> make such an assumption.  
> 
> Currently, when an RX-queue is in AF-XDP-ZC mode (MEM_TYPE_ZERO_COPY)
> the packet will get dropped when calling convert_to_xdp_frame(), but as
> the TODO comment indicated in convert_to_xdp_frame() this is not the
> end-goal. 
> 
> The comment in convert_to_xdp_frame(), indicate we need a full
> alloc+copy, but that is actually not necessary, if we can just use
> another memory area for struct xdp_frame, and a pointer to data.  Thus,
> allowing devmap-redir to work-ZC and allow cpumap-redir to do the copy
> on the remote CPU.

Thanks for pointing this out.
Seems you are saying xdp_frame area is not reusable. That means we
reduce usable headroom on every REDIRECT. I wanted to avoid this but
actually it is impossible, right?

>> +	if (!skb) {
>> +		xdp_return_frame(frame);
>> +		goto err;
>> +	}
>> +
>> +	memset(frame, 0, sizeof(*frame));
>> +	skb->protocol = eth_type_trans(skb, priv->dev);
>> +err:
>> +	return skb;
>> +err_xdp:
>> +	rcu_read_unlock();
>> +	xdp_return_frame(frame);
>> +
>> +	return NULL;
>> +}
> 
> 

-- 
Toshiaki Makita

^ permalink raw reply

* Re: [PATCH v4 bpf-next 08/14] bpf: introduce the bpf_get_local_storage() helper function
From: Daniel Borkmann @ 2018-07-31 10:34 UTC (permalink / raw)
  To: Roman Gushchin, netdev; +Cc: linux-kernel, kernel-team, Alexei Starovoitov
In-Reply-To: <20180727215243.3850-9-guro@fb.com>

Hi Roman,

On 07/27/2018 11:52 PM, Roman Gushchin wrote:
> The bpf_get_local_storage() helper function is used
> to get a pointer to the bpf local storage from a bpf program.
> 
> It takes a pointer to a storage map and flags as arguments.
> Right now it accepts only cgroup storage maps, and flags
> argument has to be 0. Further it can be extended to support
> other types of local storage: e.g. thread local storage etc.
> 
> Signed-off-by: Roman Gushchin <guro@fb.com>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Acked-by: Martin KaFai Lau <kafai@fb.com>
> ---
>  include/linux/bpf.h      |  2 ++
>  include/uapi/linux/bpf.h | 13 ++++++++++++-
>  kernel/bpf/cgroup.c      |  2 ++
>  kernel/bpf/core.c        |  1 +
>  kernel/bpf/helpers.c     | 20 ++++++++++++++++++++
>  kernel/bpf/verifier.c    | 18 ++++++++++++++++++
>  net/core/filter.c        | 23 ++++++++++++++++++++++-
>  7 files changed, 77 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index ca4ac2a39def..cd8790d2c6ed 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -788,6 +788,8 @@ extern const struct bpf_func_proto bpf_sock_map_update_proto;
>  extern const struct bpf_func_proto bpf_sock_hash_update_proto;
>  extern const struct bpf_func_proto bpf_get_current_cgroup_id_proto;
>  
> +extern const struct bpf_func_proto bpf_get_local_storage_proto;
> +
>  /* Shared helpers among cBPF and eBPF. */
>  void bpf_user_rnd_init_once(void);
>  u64 bpf_user_rnd_u32(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5);
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index a0aa53148763..495180f229ee 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -2081,6 +2081,16 @@ union bpf_attr {
>   * 	Return
>   * 		A 64-bit integer containing the current cgroup id based
>   * 		on the cgroup within which the current task is running.
> + *
> + * void* get_local_storage(void *map, u64 flags)
> + *	Description
> + *		Get the pointer to the local storage area.
> + *		The type and the size of the local storage is defined
> + *		by the *map* argument.
> + *		The *flags* meaning is specific for each map type,
> + *		and has to be 0 for cgroup local storage.
> + *	Return
> + *		Pointer to the local storage area.
>   */

I think it would be crucial to clarify what underlying assumption the
program writer can make with regards to concurrent access to this storage.

E.g. in this context, can _only_ BPF_XADD be used for counters as otherwise
any other type of access may race in parallel, or are we protected by the
socket lock and can safely override all data in this buffer via normal stores
(e.g. for socket related progs)? What about other types?

Right now nothing is mentioned here, but I think it must be clarified to
avoid any surprises. E.g. in normal htab case program can at least use the
map update there for atomic value updates, but those are disallowed from
the cgroup local storage, hence my question. Btw, no need to resend, I can
also update the paragraph there.

Thanks,
Daniel

^ permalink raw reply

* Re: [PATCH v6 bpf-next 4/9] veth: Handle xdp_frames in xdp napi ring
From: Jesper Dangaard Brouer @ 2018-07-31 10:26 UTC (permalink / raw)
  To: Toshiaki Makita
  Cc: Alexei Starovoitov, Daniel Borkmann, netdev, Jakub Kicinski,
	John Fastabend, brouer, Karlsson, Magnus, Björn Töpel
In-Reply-To: <1532947431-2737-5-git-send-email-makita.toshiaki@lab.ntt.co.jp>


Context needed from: [PATCH v6 bpf-next 2/9] veth: Add driver XDP

On Mon, 30 Jul 2018 19:43:44 +0900
Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> wrote:

> +static struct sk_buff *veth_build_skb(void *head, int headroom, int len,
> +				      int buflen)
> +{
> +	struct sk_buff *skb;
> +
> +	if (!buflen) {
> +		buflen = SKB_DATA_ALIGN(headroom + len) +
> +			 SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
> +	}
> +	skb = build_skb(head, buflen);
> +	if (!skb)
> +		return NULL;
> +
> +	skb_reserve(skb, headroom);
> +	skb_put(skb, len);
> +
> +	return skb;
> +}


On Mon, 30 Jul 2018 19:43:46 +0900
Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> wrote:

> +static struct sk_buff *veth_xdp_rcv_one(struct veth_priv *priv,
> +					struct xdp_frame *frame)
> +{
> +	int len = frame->len, delta = 0;
> +	struct bpf_prog *xdp_prog;
> +	unsigned int headroom;
> +	struct sk_buff *skb;
> +
> +	rcu_read_lock();
> +	xdp_prog = rcu_dereference(priv->xdp_prog);
> +	if (likely(xdp_prog)) {
> +		struct xdp_buff xdp;
> +		u32 act;
> +
> +		xdp.data_hard_start = frame->data - frame->headroom;
> +		xdp.data = frame->data;
> +		xdp.data_end = frame->data + frame->len;
> +		xdp.data_meta = frame->data - frame->metasize;
> +		xdp.rxq = &priv->xdp_rxq;
> +
> +		act = bpf_prog_run_xdp(xdp_prog, &xdp);
> +
> +		switch (act) {
> +		case XDP_PASS:
> +			delta = frame->data - xdp.data;
> +			len = xdp.data_end - xdp.data;
> +			break;
> +		default:
> +			bpf_warn_invalid_xdp_action(act);
> +		case XDP_ABORTED:
> +			trace_xdp_exception(priv->dev, xdp_prog, act);
> +		case XDP_DROP:
> +			goto err_xdp;
> +		}
> +	}
> +	rcu_read_unlock();
> +
> +	headroom = frame->data - delta - (void *)frame;
> +	skb = veth_build_skb(frame, headroom, len, 0);

Here you are adding an assumption that struct xdp_frame is always
located in-the-top of the packet-data area.  I tried hard not to add
such a dependency!  You can calculate the beginning of the frame from
the xdp_frame->data pointer.

Why not add such a dependency?  Because for AF_XDP zero-copy, we cannot
make such an assumption.  

Currently, when an RX-queue is in AF-XDP-ZC mode (MEM_TYPE_ZERO_COPY)
the packet will get dropped when calling convert_to_xdp_frame(), but as
the TODO comment indicated in convert_to_xdp_frame() this is not the
end-goal. 

The comment in convert_to_xdp_frame(), indicate we need a full
alloc+copy, but that is actually not necessary, if we can just use
another memory area for struct xdp_frame, and a pointer to data.  Thus,
allowing devmap-redir to work-ZC and allow cpumap-redir to do the copy
on the remote CPU.


> +	if (!skb) {
> +		xdp_return_frame(frame);
> +		goto err;
> +	}
> +
> +	memset(frame, 0, sizeof(*frame));
> +	skb->protocol = eth_type_trans(skb, priv->dev);
> +err:
> +	return skb;
> +err_xdp:
> +	rcu_read_unlock();
> +	xdp_return_frame(frame);
> +
> +	return NULL;
> +}


-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

^ permalink raw reply

* Re: KASAN: use-after-free Read in refcount_sub_and_test
From: Eric Dumazet @ 2018-07-31 11:50 UTC (permalink / raw)
  To: syzbot, davem, kuznet, linux-kernel, netdev, syzkaller-bugs,
	Sabrina Dubroca
In-Reply-To: <000000000000c5132b0572474587@google.com>



On 07/31/2018 01:22 AM, syzbot wrote:
> Hello,
> 
> syzbot found the following crash on:
> 
> HEAD commit:    61f4b23769f0 netlink: Don't shift with UB on nlk->ngroups
> git tree:       net
> console output: https://syzkaller.appspot.com/x/log.txt?x=16ed5cf0400000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=ffb4428fdc82f93b
> dashboard link: https://syzkaller.appspot.com/bug?extid=8c17db54fd0c0a2ee849
> compiler:       gcc (GCC) 8.0.1 20180413 (experimental)
> 
> Unfortunately, I don't have any reproducer for this crash yet.
> 
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+8c17db54fd0c0a2ee849@syzkaller.appspotmail.com
> 
> device lo left promiscuous mode
> ==================================================================
> BUG: KASAN: use-after-free in atomic_read include/asm-generic/atomic-instrumented.h:21 [inline]
> BUG: KASAN: use-after-free in refcount_sub_and_test+0x9a/0x350 lib/refcount.c:179
> Read of size 4 at addr ffff8801d96d2844 by task syz-executor1/4479
> 
> CPU: 0 PID: 4479 Comm: syz-executor1 Not tainted 4.18.0-rc6+ #34
> 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+0x1c9/0x2b4 lib/dump_stack.c:113
>  print_address_description+0x6c/0x20b mm/kasan/report.c:256
>  kasan_report_error mm/kasan/report.c:354 [inline]
>  kasan_report.cold.7+0x242/0x2fe mm/kasan/report.c:412
>  check_memory_region_inline mm/kasan/kasan.c:260 [inline]
>  check_memory_region+0x13e/0x1b0 mm/kasan/kasan.c:267
>  kasan_check_read+0x11/0x20 mm/kasan/kasan.c:272
>  atomic_read include/asm-generic/atomic-instrumented.h:21 [inline]
>  refcount_sub_and_test+0x9a/0x350 lib/refcount.c:179
>  refcount_dec_and_test+0x1a/0x20 lib/refcount.c:212
>  fib6_metrics_release+0x4f/0x90 net/ipv6/ip6_fib.c:178
>  fib6_drop_pcpu_from net/ipv6/ip6_fib.c:899 [inline]
>  fib6_purge_rt+0x5ec/0x7f0 net/ipv6/ip6_fib.c:934
>  fib6_del_route net/ipv6/ip6_fib.c:1784 [inline]
>  fib6_del+0xc11/0x1310 net/ipv6/ip6_fib.c:1815
>  fib6_clean_node+0x3ee/0x5e0 net/ipv6/ip6_fib.c:1976
>  fib6_walk_continue+0x4b1/0x8e0 net/ipv6/ip6_fib.c:1899
>  fib6_walk+0x95/0xf0 net/ipv6/ip6_fib.c:1947
>  fib6_clean_tree+0x1ea/0x360 net/ipv6/ip6_fib.c:2024
>  __fib6_clean_all+0x21c/0x420 net/ipv6/ip6_fib.c:2040
>  fib6_clean_all+0x27/0x30 net/ipv6/ip6_fib.c:2051
>  rt6_sync_down_dev net/ipv6/route.c:4083 [inline]
>  rt6_disable_ip+0x111/0x7e0 net/ipv6/route.c:4088
>  addrconf_ifdown+0x16f/0x1670 net/ipv6/addrconf.c:3650
>  addrconf_notify+0x6e9/0x27f0 net/ipv6/addrconf.c:3575
>  notifier_call_chain+0x180/0x390 kernel/notifier.c:93
>  __raw_notifier_call_chain kernel/notifier.c:394 [inline]
>  raw_notifier_call_chain+0x2d/0x40 kernel/notifier.c:401
>  call_netdevice_notifiers_info+0x3f/0x90 net/core/dev.c:1735
>  call_netdevice_notifiers net/core/dev.c:1753 [inline]
>  dev_close_many+0x447/0x8d0 net/core/dev.c:1505
>  rollback_registered_many+0x52b/0xef0 net/core/dev.c:7452
>  rollback_registered+0x1e9/0x420 net/core/dev.c:7517
>  unregister_netdevice_queue+0x32f/0x660 net/core/dev.c:8561
>  unregister_netdevice include/linux/netdevice.h:2548 [inline]
>  __tun_detach+0x11d1/0x15e0 drivers/net/tun.c:727
>  tun_detach drivers/net/tun.c:744 [inline]
>  tun_chr_close+0xe3/0x180 drivers/net/tun.c:3271
>  __fput+0x355/0x8b0 fs/file_table.c:209
>  ____fput+0x15/0x20 fs/file_table.c:243
>  task_work_run+0x1ec/0x2a0 kernel/task_work.c:113
>  exit_task_work include/linux/task_work.h:22 [inline]
>  do_exit+0x1b08/0x2750 kernel/exit.c:865
>  do_group_exit+0x177/0x440 kernel/exit.c:968
>  __do_sys_exit_group kernel/exit.c:979 [inline]
>  __se_sys_exit_group kernel/exit.c:977 [inline]
>  __x64_sys_exit_group+0x3e/0x50 kernel/exit.c:977
>  do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
>  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x456a09
> Code: Bad RIP value.
> RSP: 002b:00007fffa8ff4908 EFLAGS: 00000246 ORIG_RAX: 00000000000000e7
> RAX: ffffffffffffffda RBX: 0000000000000001 RCX: 0000000000456a09
> RDX: 00000000004104e0 RSI: 0000000000a44bd0 RDI: 0000000000000045
> RBP: 00000000004c1904 R08: 000000000000000b R09: 0000000000000000
> R10: 0000000001445940 R11: 0000000000000246 R12: 0000000001446940
> R13: 0000000000000000 R14: 000000000000057e R15: badc0ffeebadface
> 
> Allocated by task 28651:
>  save_stack+0x43/0xd0 mm/kasan/kasan.c:448
>  set_track mm/kasan/kasan.c:460 [inline]
>  kasan_kmalloc+0xc4/0xe0 mm/kasan/kasan.c:553
>  kmem_cache_alloc_trace+0x152/0x780 mm/slab.c:3620
>  kmalloc include/linux/slab.h:513 [inline]
>  kzalloc include/linux/slab.h:707 [inline]
>  fib6_metric_set+0x163/0x2c0 net/ipv6/ip6_fib.c:645
>  fib6_add_rt2node+0xe36/0x27f0 net/ipv6/ip6_fib.c:1000
>  fib6_add+0xaae/0x14d0 net/ipv6/ip6_fib.c:1308
>  __ip6_ins_rt+0x54/0x80 net/ipv6/route.c:1163
>  ip6_route_add+0x6d/0xc0 net/ipv6/route.c:3171
>  addrconf_prefix_route.isra.48+0x51d/0x720 net/ipv6/addrconf.c:2347
>  inet6_addr_modify net/ipv6/addrconf.c:4627 [inline]
>  inet6_rtm_newaddr+0x112e/0x1b50 net/ipv6/addrconf.c:4743
>  rtnetlink_rcv_msg+0x46e/0xc30 net/core/rtnetlink.c:4665
>  netlink_rcv_skb+0x172/0x440 net/netlink/af_netlink.c:2453
>  rtnetlink_rcv+0x1c/0x20 net/core/rtnetlink.c:4683
>  netlink_unicast_kernel net/netlink/af_netlink.c:1315 [inline]
>  netlink_unicast+0x5a0/0x760 net/netlink/af_netlink.c:1341
>  netlink_sendmsg+0xa18/0xfd0 net/netlink/af_netlink.c:1906
>  sock_sendmsg_nosec net/socket.c:642 [inline]
>  sock_sendmsg+0xd5/0x120 net/socket.c:652
>  ___sys_sendmsg+0x7fd/0x930 net/socket.c:2126
>  __sys_sendmsg+0x11d/0x290 net/socket.c:2164
>  __do_sys_sendmsg net/socket.c:2173 [inline]
>  __se_sys_sendmsg net/socket.c:2171 [inline]
>  __x64_sys_sendmsg+0x78/0xb0 net/socket.c:2171
>  do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
>  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> 
> Freed by task 4479:
>  save_stack+0x43/0xd0 mm/kasan/kasan.c:448
>  set_track mm/kasan/kasan.c:460 [inline]
>  __kasan_slab_free+0x11a/0x170 mm/kasan/kasan.c:521
>  kasan_slab_free+0xe/0x10 mm/kasan/kasan.c:528
>  __cache_free mm/slab.c:3498 [inline]
>  kfree+0xd9/0x260 mm/slab.c:3813
>  fib6_metrics_release+0x77/0x90 net/ipv6/ip6_fib.c:179
>  fib6_drop_pcpu_from net/ipv6/ip6_fib.c:899 [inline]
>  fib6_purge_rt+0x5ec/0x7f0 net/ipv6/ip6_fib.c:934
>  fib6_del_route net/ipv6/ip6_fib.c:1784 [inline]
>  fib6_del+0xc11/0x1310 net/ipv6/ip6_fib.c:1815
>  fib6_clean_node+0x3ee/0x5e0 net/ipv6/ip6_fib.c:1976
>  fib6_walk_continue+0x4b1/0x8e0 net/ipv6/ip6_fib.c:1899
>  fib6_walk+0x95/0xf0 net/ipv6/ip6_fib.c:1947
>  fib6_clean_tree+0x1ea/0x360 net/ipv6/ip6_fib.c:2024
>  __fib6_clean_all+0x21c/0x420 net/ipv6/ip6_fib.c:2040
>  fib6_clean_all+0x27/0x30 net/ipv6/ip6_fib.c:2051
>  rt6_sync_down_dev net/ipv6/route.c:4083 [inline]
>  rt6_disable_ip+0x111/0x7e0 net/ipv6/route.c:4088
>  addrconf_ifdown+0x16f/0x1670 net/ipv6/addrconf.c:3650
>  addrconf_notify+0x6e9/0x27f0 net/ipv6/addrconf.c:3575
>  notifier_call_chain+0x180/0x390 kernel/notifier.c:93
>  __raw_notifier_call_chain kernel/notifier.c:394 [inline]
>  raw_notifier_call_chain+0x2d/0x40 kernel/notifier.c:401
>  call_netdevice_notifiers_info+0x3f/0x90 net/core/dev.c:1735
>  call_netdevice_notifiers net/core/dev.c:1753 [inline]
>  dev_close_many+0x447/0x8d0 net/core/dev.c:1505
>  rollback_registered_many+0x52b/0xef0 net/core/dev.c:7452
>  rollback_registered+0x1e9/0x420 net/core/dev.c:7517
>  unregister_netdevice_queue+0x32f/0x660 net/core/dev.c:8561
>  unregister_netdevice include/linux/netdevice.h:2548 [inline]
>  __tun_detach+0x11d1/0x15e0 drivers/net/tun.c:727
>  tun_detach drivers/net/tun.c:744 [inline]
>  tun_chr_close+0xe3/0x180 drivers/net/tun.c:3271
>  __fput+0x355/0x8b0 fs/file_table.c:209
>  ____fput+0x15/0x20 fs/file_table.c:243
>  task_work_run+0x1ec/0x2a0 kernel/task_work.c:113
>  exit_task_work include/linux/task_work.h:22 [inline]
>  do_exit+0x1b08/0x2750 kernel/exit.c:865
>  do_group_exit+0x177/0x440 kernel/exit.c:968
>  __do_sys_exit_group kernel/exit.c:979 [inline]
>  __se_sys_exit_group kernel/exit.c:977 [inline]
>  __x64_sys_exit_group+0x3e/0x50 kernel/exit.c:977
>  do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
>  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> 
> The buggy address belongs to the object at ffff8801d96d2800
>  which belongs to the cache kmalloc-96 of size 96
> The buggy address is located 68 bytes inside of
>  96-byte region [ffff8801d96d2800, ffff8801d96d2860)
> The buggy address belongs to the page:
> page:ffffea000765b480 count:1 mapcount:0 mapping:ffff8801dac004c0 index:0xffff8801d96d2700
> flags: 0x2fffc0000000100(slab)
> raw: 02fffc0000000100 ffffea0006c46348 ffffea0006d0d288 ffff8801dac004c0
> raw: ffff8801d96d2700 ffff8801d96d2000 0000000100000003 0000000000000000
> page dumped because: kasan: bad access detected
> 
> Memory state around the buggy address:
>  ffff8801d96d2700: fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
>  ffff8801d96d2780: fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
>> ffff8801d96d2800: fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
>                                            ^
>  ffff8801d96d2880: fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
>  ffff8801d96d2900: fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
> ==================================================================
> 
> 
> ---
> This bug is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syzkaller@googlegroups.com.
> 
> syzbot will keep track of this bug report. See:
> https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with syzbot.

This might be caused by 

commit df18b50448fab1dff093731dfd0e25e77e1afcd1
Author: Sabrina Dubroca <sd@queasysnail.net>
Date:   Mon Jul 30 16:23:10 2018 +0200

    net/ipv6: fix metrics leak

^ permalink raw reply

* Re: [PATCH mlx5-next] RDMA/mlx5: Don't use cached IRQ affinity mask
From: Max Gurtovoy @ 2018-07-31 10:00 UTC (permalink / raw)
  To: Steve Wise, 'Sagi Grimberg'
  Cc: Jason Gunthorpe, 'Leon Romanovsky',
	'Doug Ledford', 'RDMA mailing list',
	'Saeed Mahameed', 'linux-netdev'
In-Reply-To: <ec4ba167-d5a3-adac-3df9-c8b84b7fb821@opengridcomputing.com>



On 7/30/2018 6:47 PM, Steve Wise wrote:
> 
> 
> On 7/23/2018 11:53 AM, Max Gurtovoy wrote:
>>
>>
>> On 7/23/2018 7:49 PM, Jason Gunthorpe wrote:
>>> On Fri, Jul 20, 2018 at 04:25:32AM +0300, Max Gurtovoy wrote:
>>>>
>>>>>>> [ 2032.194376] nvme nvme0: failed to connect queue: 9 ret=-18
>>>>>>
>>>>>> queue 9 is not mapped (overlap).
>>>>>> please try the bellow:
>>>>>>
>>>>>
>>>>> This seems to work.  Here are three mapping cases:  each vector on its
>>>>> own cpu, each vector on 1 cpu within the local numa node, and each
>>>>> vector having all cpus in its numa node.  The 2nd mapping looks kinda
>>>>> funny, but I think it achieved what you wanted?  And all the cases
>>>>> resulted in successful connections.
>>>>>
>>>>
>>>> Thanks for testing this.
>>>> I slightly improved the setting of the left CPUs and actually used
>>>> Sagi's
>>>> initial proposal.
>>>>
>>>> Sagi,
>>>> please review the attached patch and let me know if I should add your
>>>> signature on it.
>>>> I'll run some perf test early next week on it (meanwhile I run
>>>> login/logout
>>>> with different num_queues successfully and irq settings).
>>>>
>>>> Steve,
>>>> It will be great if you can apply the attached in your system and
>>>> send your
>>>> findings.
>>>>
>>>> Regards,
>>>> Max,
>>>
>>> So the conlusion to this thread is that Leon's mlx5 patch needs to wait
>>> until this block-mq patch is accepted?
>>
>> Yes, since nvmf is the only user of this function.
>> Still waiting for comments on the suggested patch :)
> 
> Hey Sagi, what do you think of Max's patch?
> 
> Max, should you resend this in a form suitable for merging?

Yes, we have already a small improvment of the naive step.
but first I want to see some feedback from other maintainers as well.

> 
> Thanks,
> 
> Steve.
> 

^ permalink raw reply

* [PATCH net-next 3/7] mlxsw: spectrum: Extract work-scheduling into a new function
From: Petr Machata @ 2018-07-31  9:56 UTC (permalink / raw)
  To: netdev, linux-doc, linux-kselftest
  Cc: davem, corbet, jiri, idosch, kuznet, yoshfuji, shuah, nikolay,
	dsahern
In-Reply-To: <cover.1533030830.git.petrm@mellanox.com>

The boilerplate to schedule NETEVENT_IPV4_MPATH_HASH_UPDATE and
NETEVENT_IPV6_MPATH_HASH_UPDATE handling is almost equivalent to that of
NETEVENT_IPV4_FWD_UPDATE_PRIORITY_UPDATE that's coming in the next
patch. The only difference is which actual worker function should be
called. Extract this boilerplate into a named function in order to allow
reuse.

Signed-off-by: Petr Machata <petrm@mellanox.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
---
 .../net/ethernet/mellanox/mlxsw/spectrum_router.c  | 38 +++++++++++++---------
 1 file changed, 23 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 8d67f0123699..5ee927626567 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -2436,17 +2436,36 @@ static void mlxsw_sp_router_mp_hash_event_work(struct work_struct *work)
 	kfree(net_work);
 }
 
+static int mlxsw_sp_router_schedule_work(struct net *net,
+					 struct notifier_block *nb,
+					 void (*cb)(struct work_struct *))
+{
+	struct mlxsw_sp_netevent_work *net_work;
+	struct mlxsw_sp_router *router;
+
+	if (!net_eq(net, &init_net))
+		return NOTIFY_DONE;
+
+	net_work = kzalloc(sizeof(*net_work), GFP_ATOMIC);
+	if (!net_work)
+		return NOTIFY_BAD;
+
+	router = container_of(nb, struct mlxsw_sp_router, netevent_nb);
+	INIT_WORK(&net_work->work, cb);
+	net_work->mlxsw_sp = router->mlxsw_sp;
+	mlxsw_core_schedule_work(&net_work->work);
+	return NOTIFY_DONE;
+}
+
 static int mlxsw_sp_router_netevent_event(struct notifier_block *nb,
 					  unsigned long event, void *ptr)
 {
 	struct mlxsw_sp_netevent_work *net_work;
 	struct mlxsw_sp_port *mlxsw_sp_port;
-	struct mlxsw_sp_router *router;
 	struct mlxsw_sp *mlxsw_sp;
 	unsigned long interval;
 	struct neigh_parms *p;
 	struct neighbour *n;
-	struct net *net;
 
 	switch (event) {
 	case NETEVENT_DELAY_PROBE_TIME_UPDATE:
@@ -2500,20 +2519,9 @@ static int mlxsw_sp_router_netevent_event(struct notifier_block *nb,
 		break;
 	case NETEVENT_IPV4_MPATH_HASH_UPDATE:
 	case NETEVENT_IPV6_MPATH_HASH_UPDATE:
-		net = ptr;
+		return mlxsw_sp_router_schedule_work(ptr, nb,
+				mlxsw_sp_router_mp_hash_event_work);
 
-		if (!net_eq(net, &init_net))
-			return NOTIFY_DONE;
-
-		net_work = kzalloc(sizeof(*net_work), GFP_ATOMIC);
-		if (!net_work)
-			return NOTIFY_BAD;
-
-		router = container_of(nb, struct mlxsw_sp_router, netevent_nb);
-		INIT_WORK(&net_work->work, mlxsw_sp_router_mp_hash_event_work);
-		net_work->mlxsw_sp = router->mlxsw_sp;
-		mlxsw_core_schedule_work(&net_work->work);
-		break;
 	}
 
 	return NOTIFY_DONE;
-- 
2.4.11

^ permalink raw reply related

* [PATCH net-next 7/7] selftests: mlxsw: Add test for ip_forward_update_priority
From: Petr Machata @ 2018-07-31  9:56 UTC (permalink / raw)
  To: netdev, linux-doc, linux-kselftest
  Cc: davem, corbet, jiri, idosch, kuznet, yoshfuji, shuah, nikolay,
	dsahern
In-Reply-To: <cover.1533030830.git.petrm@mellanox.com>

Verify that with that sysctl turned off, DSCP prioritization and rewrite
works the same way as in qos_dscp_bridge test. However when the sysctl
is charged, there should be a reprioritization after routing stage,
which will be observed by a different DSCP rewrite on egress.

Signed-off-by: Petr Machata <petrm@mellanox.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
---
 .../selftests/drivers/net/mlxsw/qos_dscp_router.sh | 233 +++++++++++++++++++++
 1 file changed, 233 insertions(+)
 create mode 100755 tools/testing/selftests/drivers/net/mlxsw/qos_dscp_router.sh

diff --git a/tools/testing/selftests/drivers/net/mlxsw/qos_dscp_router.sh b/tools/testing/selftests/drivers/net/mlxsw/qos_dscp_router.sh
new file mode 100755
index 000000000000..281d90766e12
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/mlxsw/qos_dscp_router.sh
@@ -0,0 +1,233 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+# Test for DSCP prioritization in the router.
+#
+# With ip_forward_update_priority disabled, the packets are expected to keep
+# their DSCP (which in this test uses only values 0..7) intact as they are
+# forwarded by the switch. That is verified at $h2. ICMP responses are formed
+# with the same DSCP as the requests, and likewise pass through the switch
+# intact, which is verified at $h1.
+#
+# With ip_forward_update_priority enabled, router reprioritizes the packets
+# according to the table in reprioritize(). Thus, say, DSCP 7 maps to priority
+# 4, which on egress maps back to DSCP 4. The response packet then gets
+# reprioritized to 6, getting DSCP 6 on egress.
+#
+# +----------------------+                             +----------------------+
+# | H1                   |                             |                   H2 |
+# |    + $h1             |                             |            $h2 +     |
+# |    | 192.0.2.1/28    |                             |  192.0.2.18/28 |     |
+# +----|-----------------+                             +----------------|-----+
+#      |                                                                |
+# +----|----------------------------------------------------------------|-----+
+# | SW |                                                                |     |
+# |    + $swp1                                                    $swp2 +     |
+# |      192.0.2.2/28                                     192.0.2.17/28       |
+# |      APP=0,5,0 .. 7,5,7                          APP=0,5,0 .. 7,5,7       |
+# +---------------------------------------------------------------------------+
+
+ALL_TESTS="
+	ping_ipv4
+	test_update
+	test_no_update
+"
+
+lib_dir=$(dirname $0)/../../../net/forwarding
+
+NUM_NETIFS=4
+source $lib_dir/lib.sh
+
+reprioritize()
+{
+	local in=$1; shift
+
+	# This is based on rt_tos2priority in include/net/route.h. Assuming 1:1
+	# mapping between priorities and TOS, it yields a new priority for a
+	# packet with ingress priority of $in.
+	local -a reprio=(0 0 2 2 6 6 4 4)
+
+	echo ${reprio[$in]}
+}
+
+h1_create()
+{
+	local dscp;
+
+	simple_if_init $h1 192.0.2.1/28
+	tc qdisc add dev $h1 clsact
+	dscp_capture_install $h1 0
+	ip route add vrf v$h1 192.0.2.16/28 via 192.0.2.2
+}
+
+h1_destroy()
+{
+	ip route del vrf v$h1 192.0.2.16/28 via 192.0.2.2
+	dscp_capture_uninstall $h1 0
+	tc qdisc del dev $h1 clsact
+	simple_if_fini $h1 192.0.2.1/28
+}
+
+h2_create()
+{
+	simple_if_init $h2 192.0.2.18/28
+	tc qdisc add dev $h2 clsact
+	dscp_capture_install $h2 0
+	ip route add vrf v$h2 192.0.2.0/28 via 192.0.2.17
+}
+
+h2_destroy()
+{
+	ip route del vrf v$h2 192.0.2.0/28 via 192.0.2.17
+	dscp_capture_uninstall $h2 0
+	tc qdisc del dev $h2 clsact
+	simple_if_fini $h2 192.0.2.18/28
+}
+
+dscp_map()
+{
+	local base=$1; shift
+
+	for prio in {0..7}; do
+		echo app=$prio,5,$((base + prio))
+	done
+}
+
+switch_create()
+{
+	simple_if_init $swp1 192.0.2.2/28
+	__simple_if_init $swp2 v$swp1 192.0.2.17/28
+
+	lldptool -T -i $swp1 -V APP $(dscp_map 0) >/dev/null
+	lldptool -T -i $swp2 -V APP $(dscp_map 0) >/dev/null
+	lldpad_app_wait_set $swp1
+	lldpad_app_wait_set $swp2
+}
+
+switch_destroy()
+{
+	lldptool -T -i $swp2 -V APP -d $(dscp_map 0) >/dev/null
+	lldptool -T -i $swp1 -V APP -d $(dscp_map 0) >/dev/null
+	lldpad_app_wait_del
+
+	__simple_if_fini $swp2 192.0.2.17/28
+	simple_if_fini $swp1 192.0.2.2/28
+}
+
+setup_prepare()
+{
+	h1=${NETIFS[p1]}
+	swp1=${NETIFS[p2]}
+
+	swp2=${NETIFS[p3]}
+	h2=${NETIFS[p4]}
+
+	vrf_prepare
+
+	sysctl_set net.ipv4.ip_forward_update_priority 1
+	h1_create
+	h2_create
+	switch_create
+}
+
+cleanup()
+{
+	pre_cleanup
+
+	switch_destroy
+	h2_destroy
+	h1_destroy
+	sysctl_restore net.ipv4.ip_forward_update_priority
+
+	vrf_cleanup
+}
+
+ping_ipv4()
+{
+	ping_test $h1 192.0.2.18
+}
+
+dscp_ping_test()
+{
+	local vrf_name=$1; shift
+	local sip=$1; shift
+	local dip=$1; shift
+	local prio=$1; shift
+	local reprio=$1; shift
+	local dev1=$1; shift
+	local dev2=$1; shift
+
+	local prio2=$($reprio $prio)   # ICMP Request egress prio
+	local prio3=$($reprio $prio2)  # ICMP Response egress prio
+
+	local dscp=$((prio << 2))     # ICMP Request ingress DSCP
+	local dscp2=$((prio2 << 2))   # ICMP Request egress DSCP
+	local dscp3=$((prio3 << 2))   # ICMP Response egress DSCP
+
+	RET=0
+
+	eval "local -A dev1_t0s=($(dscp_fetch_stats $dev1 0))"
+	eval "local -A dev2_t0s=($(dscp_fetch_stats $dev2 0))"
+
+	ip vrf exec $vrf_name \
+	   ${PING} -Q $dscp ${sip:+-I $sip} $dip \
+		   -c 10 -i 0.1 -w 2 &> /dev/null
+
+	eval "local -A dev1_t1s=($(dscp_fetch_stats $dev1 0))"
+	eval "local -A dev2_t1s=($(dscp_fetch_stats $dev2 0))"
+
+	for i in {0..7}; do
+		local dscpi=$((i << 2))
+		local expect2=0
+		local expect3=0
+
+		if ((i == prio2)); then
+			expect2=10
+		fi
+		if ((i == prio3)); then
+			expect3=10
+		fi
+
+		local delta=$((dev2_t1s[$i] - dev2_t0s[$i]))
+		((expect2 == delta))
+		check_err $? "DSCP $dscpi@$dev2: Expected to capture $expect2 packets, got $delta."
+
+		delta=$((dev1_t1s[$i] - dev1_t0s[$i]))
+		((expect3 == delta))
+		check_err $? "DSCP $dscpi@$dev1: Expected to capture $expect3 packets, got $delta."
+	done
+
+	log_test "DSCP rewrite: $dscp-(prio $prio2)-$dscp2-(prio $prio3)-$dscp3"
+}
+
+__test_update()
+{
+	local update=$1; shift
+	local reprio=$1; shift
+
+	sysctl_restore net.ipv4.ip_forward_update_priority
+	sysctl_set net.ipv4.ip_forward_update_priority $update
+
+	for prio in {0..7}; do
+		dscp_ping_test v$h1 192.0.2.1 192.0.2.18 $prio $reprio $h1 $h2
+	done
+}
+
+test_update()
+{
+	__test_update 1 reprioritize
+}
+
+test_no_update()
+{
+	__test_update 0 echo
+}
+
+trap cleanup EXIT
+
+setup_prepare
+setup_wait
+
+tests_run
+
+exit $EXIT_STATUS
-- 
2.4.11

^ permalink raw reply related

* [PATCH net-next 6/7] selftests: forwarding: Move DSCP capture to lib.sh
From: Petr Machata @ 2018-07-31  9:56 UTC (permalink / raw)
  To: netdev, linux-doc, linux-kselftest
  Cc: davem, corbet, jiri, idosch, kuznet, yoshfuji, shuah, nikolay,
	dsahern
In-Reply-To: <cover.1533030830.git.petrm@mellanox.com>

dscp_capture_install() and dscp_capture_uninstall() are going to be
useful for a test added by a following patch, move them therefore to
lib.sh together with related helpers.

While doing so, change the rule preference from mere DSCP value to
DSCP+100 in order to support adding captures of packets with DSCP of 0.

Signed-off-by: Petr Machata <petrm@mellanox.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
---
 .../selftests/drivers/net/mlxsw/qos_dscp_bridge.sh | 42 ----------------------
 tools/testing/selftests/net/forwarding/lib.sh      | 42 ++++++++++++++++++++++
 2 files changed, 42 insertions(+), 42 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/mlxsw/qos_dscp_bridge.sh b/tools/testing/selftests/drivers/net/mlxsw/qos_dscp_bridge.sh
index 9e875ee8dc1c..1ca631d5aaba 100755
--- a/tools/testing/selftests/drivers/net/mlxsw/qos_dscp_bridge.sh
+++ b/tools/testing/selftests/drivers/net/mlxsw/qos_dscp_bridge.sh
@@ -34,36 +34,6 @@ lib_dir=$(dirname $0)/../../../net/forwarding
 NUM_NETIFS=4
 source $lib_dir/lib.sh
 
-__dscp_capture_add_del()
-{
-	local add_del=$1; shift
-	local dev=$1; shift
-	local base=$1; shift
-	local dscp;
-
-	for prio in {0..7}; do
-		dscp=$((base + prio))
-		__icmp_capture_add_del $add_del $dscp "" $dev \
-				       "ip_tos $((dscp << 2))"
-	done
-}
-
-dscp_capture_install()
-{
-	local dev=$1; shift
-	local base=$1; shift
-
-	__dscp_capture_add_del add $dev $base
-}
-
-dscp_capture_uninstall()
-{
-	local dev=$1; shift
-	local base=$1; shift
-
-	__dscp_capture_add_del del $dev $base
-}
-
 h1_create()
 {
 	local dscp;
@@ -155,18 +125,6 @@ cleanup()
 	vrf_cleanup
 }
 
-dscp_fetch_stats()
-{
-	local dev=$1; shift
-	local base=$1; shift
-
-	for prio in {0..7}; do
-		local dscp=$((base + prio))
-		local t=$(tc_rule_stats_get $dev $dscp)
-		echo "[$dscp]=$t "
-	done
-}
-
 ping_ipv4()
 {
 	ping_test $h1 192.0.2.2
diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
index 90af5cd23417..ca53b539aa2d 100644
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -653,6 +653,48 @@ vlan_capture_uninstall()
 	__vlan_capture_add_del del 100 "$@"
 }
 
+__dscp_capture_add_del()
+{
+	local add_del=$1; shift
+	local dev=$1; shift
+	local base=$1; shift
+	local dscp;
+
+	for prio in {0..7}; do
+		dscp=$((base + prio))
+		__icmp_capture_add_del $add_del $((dscp + 100)) "" $dev \
+				       "skip_hw ip_tos $((dscp << 2))"
+	done
+}
+
+dscp_capture_install()
+{
+	local dev=$1; shift
+	local base=$1; shift
+
+	__dscp_capture_add_del add $dev $base
+}
+
+dscp_capture_uninstall()
+{
+	local dev=$1; shift
+	local base=$1; shift
+
+	__dscp_capture_add_del del $dev $base
+}
+
+dscp_fetch_stats()
+{
+	local dev=$1; shift
+	local base=$1; shift
+
+	for prio in {0..7}; do
+		local dscp=$((base + prio))
+		local t=$(tc_rule_stats_get $dev $((dscp + 100)))
+		echo "[$dscp]=$t "
+	done
+}
+
 matchall_sink_create()
 {
 	local dev=$1; shift
-- 
2.4.11

^ permalink raw reply related

* [PATCH net-next 5/7] selftests: forwarding: Move lldpad waiting to lib.sh
From: Petr Machata @ 2018-07-31  9:56 UTC (permalink / raw)
  To: netdev, linux-doc, linux-kselftest
  Cc: davem, corbet, jiri, idosch, kuznet, yoshfuji, shuah, nikolay,
	dsahern
In-Reply-To: <cover.1533030830.git.petrm@mellanox.com>

The function lldpad_wait() will be useful for a test added by a
following patch. Likewise would the "sleep 5" with its extensive
comment.

Therefore move lldpad_wait() to lib.sh in order to allow reuse. Rename
it to lldpad_app_wait_set() to recognize that what this is intended to
wait on are the pending APP sets.

For the sleeping, add a function lldpad_app_wait_del(). That will serve
to hold the related explanatory comment (which edit for clarity), and as
a token in the caller to identify the sites where this sort of waiting
takes place. That will serve when/if a better way to handle this
business is found.

Signed-off-by: Petr Machata <petrm@mellanox.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
---
 .../selftests/drivers/net/mlxsw/qos_dscp_bridge.sh | 23 +++-------------------
 tools/testing/selftests/net/forwarding/lib.sh      | 21 ++++++++++++++++++++
 2 files changed, 24 insertions(+), 20 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/mlxsw/qos_dscp_bridge.sh b/tools/testing/selftests/drivers/net/mlxsw/qos_dscp_bridge.sh
index cc527660a022..9e875ee8dc1c 100755
--- a/tools/testing/selftests/drivers/net/mlxsw/qos_dscp_bridge.sh
+++ b/tools/testing/selftests/drivers/net/mlxsw/qos_dscp_bridge.sh
@@ -103,16 +103,6 @@ dscp_map()
 	done
 }
 
-lldpad_wait()
-{
-	local dev=$1; shift
-
-	while lldptool -t -i $dev -V APP -c app | grep -q pending; do
-	    echo "$dev: waiting for lldpad to push pending APP updates"
-	    sleep 5
-	done
-}
-
 switch_create()
 {
 	ip link add name br1 type bridge vlan_filtering 1
@@ -124,22 +114,15 @@ switch_create()
 
 	lldptool -T -i $swp1 -V APP $(dscp_map 10) >/dev/null
 	lldptool -T -i $swp2 -V APP $(dscp_map 20) >/dev/null
-	lldpad_wait $swp1
-	lldpad_wait $swp2
+	lldpad_app_wait_set $swp1
+	lldpad_app_wait_set $swp2
 }
 
 switch_destroy()
 {
 	lldptool -T -i $swp2 -V APP -d $(dscp_map 20) >/dev/null
 	lldptool -T -i $swp1 -V APP -d $(dscp_map 10) >/dev/null
-
-	# Give lldpad a chance to push down the changes. If the device is downed
-	# too soon, the updates will be left pending, but will have been struck
-	# off the lldpad's DB already, and we won't be able to tell. Then on
-	# next test iteration this would cause weirdness as newly-added APP
-	# rules conflict with the old ones, sometimes getting stuck in an
-	# "unknown" state.
-	sleep 5
+	lldpad_app_wait_del
 
 	ip link set dev $swp2 nomaster
 	ip link set dev $swp1 nomaster
diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
index 843a6715924f..90af5cd23417 100644
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -247,6 +247,27 @@ setup_wait()
 	sleep $WAIT_TIME
 }
 
+lldpad_app_wait_set()
+{
+	local dev=$1; shift
+
+	while lldptool -t -i $dev -V APP -c app | grep -q pending; do
+		echo "$dev: waiting for lldpad to push pending APP updates"
+		sleep 5
+	done
+}
+
+lldpad_app_wait_del()
+{
+	# Give lldpad a chance to push down the changes. If the device is downed
+	# too soon, the updates will be left pending. However, they will have
+	# been struck off the lldpad's DB already, so we won't be able to tell
+	# they are pending. Then on next test iteration this would cause
+	# weirdness as newly-added APP rules conflict with the old ones,
+	# sometimes getting stuck in an "unknown" state.
+	sleep 5
+}
+
 pre_cleanup()
 {
 	if [ "${PAUSE_ON_CLEANUP}" = "yes" ]; then
-- 
2.4.11

^ permalink raw reply related

* [PATCH net-next 4/7] mlxsw: spectrum_router: Handle sysctl_ip_fwd_update_priority
From: Petr Machata @ 2018-07-31  9:56 UTC (permalink / raw)
  To: netdev, linux-doc, linux-kselftest
  Cc: davem, corbet, jiri, idosch, kuznet, yoshfuji, shuah, nikolay,
	dsahern
In-Reply-To: <cover.1533030830.git.petrm@mellanox.com>

This sysctl setting controls whether packet priority should be updated
after forwarding. Configure RGCR.usp accordingly so that the device is
in sync with the kernel handling.

Note that RGCR doesn't allow changing arbitrary parameters
mid-operation, however "usp" is exempt and can be reconfigured.

Also react to NETEVENT_IPV4_FWD_UPDATE_PRIORITY_UPDATE notifications
that signify change in this configuration.

Signed-off-by: Petr Machata <petrm@mellanox.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 5ee927626567..eec7166fad62 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -2436,6 +2436,18 @@ static void mlxsw_sp_router_mp_hash_event_work(struct work_struct *work)
 	kfree(net_work);
 }
 
+static int __mlxsw_sp_router_init(struct mlxsw_sp *mlxsw_sp);
+
+static void mlxsw_sp_router_update_priority_work(struct work_struct *work)
+{
+	struct mlxsw_sp_netevent_work *net_work =
+		container_of(work, struct mlxsw_sp_netevent_work, work);
+	struct mlxsw_sp *mlxsw_sp = net_work->mlxsw_sp;
+
+	__mlxsw_sp_router_init(mlxsw_sp);
+	kfree(net_work);
+}
+
 static int mlxsw_sp_router_schedule_work(struct net *net,
 					 struct notifier_block *nb,
 					 void (*cb)(struct work_struct *))
@@ -2522,6 +2534,9 @@ static int mlxsw_sp_router_netevent_event(struct notifier_block *nb,
 		return mlxsw_sp_router_schedule_work(ptr, nb,
 				mlxsw_sp_router_mp_hash_event_work);
 
+	case NETEVENT_IPV4_FWD_UPDATE_PRIORITY_UPDATE:
+		return mlxsw_sp_router_schedule_work(ptr, nb,
+				mlxsw_sp_router_update_priority_work);
 	}
 
 	return NOTIFY_DONE;
@@ -7390,6 +7405,7 @@ static int mlxsw_sp_dscp_init(struct mlxsw_sp *mlxsw_sp)
 
 static int __mlxsw_sp_router_init(struct mlxsw_sp *mlxsw_sp)
 {
+	bool usp = init_net.ipv4.sysctl_ip_fwd_update_priority;
 	char rgcr_pl[MLXSW_REG_RGCR_LEN];
 	u64 max_rifs;
 	int err;
@@ -7400,7 +7416,7 @@ static int __mlxsw_sp_router_init(struct mlxsw_sp *mlxsw_sp)
 
 	mlxsw_reg_rgcr_pack(rgcr_pl, true, true);
 	mlxsw_reg_rgcr_max_router_interfaces_set(rgcr_pl, max_rifs);
-	mlxsw_reg_rgcr_usp_set(rgcr_pl, true);
+	mlxsw_reg_rgcr_usp_set(rgcr_pl, usp);
 	err = mlxsw_reg_write(mlxsw_sp->core, MLXSW_REG(rgcr), rgcr_pl);
 	if (err)
 		return err;
-- 
2.4.11

^ permalink raw reply related

* [PATCH net-next 2/7] net: ipv4: Notify about changes to ip_forward_update_priority
From: Petr Machata @ 2018-07-31  9:56 UTC (permalink / raw)
  To: netdev, linux-doc, linux-kselftest
  Cc: davem, corbet, jiri, idosch, kuznet, yoshfuji, shuah, nikolay,
	dsahern
In-Reply-To: <cover.1533030830.git.petrm@mellanox.com>

Drivers may make offloading decision based on whether
ip_forward_update_priority is enabled or not. Therefore distribute
netevent notifications to give them a chance to react to a change.

Signed-off-by: Petr Machata <petrm@mellanox.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
---
 include/net/netevent.h     |  1 +
 net/ipv4/sysctl_net_ipv4.c | 19 ++++++++++++++++++-
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/include/net/netevent.h b/include/net/netevent.h
index d9918261701c..4107016c3bb4 100644
--- a/include/net/netevent.h
+++ b/include/net/netevent.h
@@ -28,6 +28,7 @@ enum netevent_notif_type {
 	NETEVENT_DELAY_PROBE_TIME_UPDATE, /* arg is struct neigh_parms ptr */
 	NETEVENT_IPV4_MPATH_HASH_UPDATE, /* arg is struct net ptr */
 	NETEVENT_IPV6_MPATH_HASH_UPDATE, /* arg is struct net ptr */
+	NETEVENT_IPV4_FWD_UPDATE_PRIORITY_UPDATE, /* arg is struct net ptr */
 };
 
 int register_netevent_notifier(struct notifier_block *nb);
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index e21dda015513..b92f422f2fa8 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -201,6 +201,23 @@ static int ipv4_ping_group_range(struct ctl_table *table, int write,
 	return ret;
 }
 
+static int ipv4_fwd_update_priority(struct ctl_table *table, int write,
+				    void __user *buffer,
+				    size_t *lenp, loff_t *ppos)
+{
+	struct net *net;
+	int ret;
+
+	net = container_of(table->data, struct net,
+			   ipv4.sysctl_ip_fwd_update_priority);
+	ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos);
+	if (write && ret == 0)
+		call_netevent_notifiers(NETEVENT_IPV4_FWD_UPDATE_PRIORITY_UPDATE,
+					net);
+
+	return ret;
+}
+
 static int proc_tcp_congestion_control(struct ctl_table *ctl, int write,
 				       void __user *buffer, size_t *lenp, loff_t *ppos)
 {
@@ -668,7 +685,7 @@ static struct ctl_table ipv4_net_table[] = {
 		.data		= &init_net.ipv4.sysctl_ip_fwd_update_priority,
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
-		.proc_handler   = proc_dointvec_minmax,
+		.proc_handler   = ipv4_fwd_update_priority,
 		.extra1		= &zero,
 		.extra2		= &one,
 	},
-- 
2.4.11

^ permalink raw reply related

* [PATCH net-next 1/7] net: ipv4: Control SKB reprioritization after forwarding
From: Petr Machata @ 2018-07-31  9:56 UTC (permalink / raw)
  To: netdev, linux-doc, linux-kselftest
  Cc: davem, corbet, jiri, idosch, kuznet, yoshfuji, shuah, nikolay,
	dsahern
In-Reply-To: <cover.1533030830.git.petrm@mellanox.com>

After IPv4 packets are forwarded, the priority of the corresponding SKB
is updated according to the TOS field of IPv4 header. This overrides any
prioritization done earlier by e.g. an skbedit action or ingress-qos-map
defined at a vlan device.

Such overriding may not always be desirable. Even if the packet ends up
being routed, which implies this is an L3 network node, an administrator
may wish to preserve whatever prioritization was done earlier on in the
pipeline.

Therefore introduce a sysctl that controls this behavior. Keep the
default value at 1 to maintain backward-compatible behavior.

Signed-off-by: Petr Machata <petrm@mellanox.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
---
 Documentation/networking/ip-sysctl.txt | 9 +++++++++
 include/net/netns/ipv4.h               | 1 +
 net/ipv4/af_inet.c                     | 1 +
 net/ipv4/ip_forward.c                  | 3 ++-
 net/ipv4/sysctl_net_ipv4.c             | 9 +++++++++
 5 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index 77c37fb0b6a6..e74515ecaa9c 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -81,6 +81,15 @@ fib_multipath_hash_policy - INTEGER
 	0 - Layer 3
 	1 - Layer 4
 
+ip_forward_update_priority - INTEGER
+	Whether to update SKB priority from "TOS" field in IPv4 header after it
+	is forwarded. The new SKB priority is mapped from TOS field value
+	according to an rt_tos2priority table (see e.g. man tc-prio).
+	Default: 1 (Update priority.)
+	Possible values:
+	0 - Do not update priority.
+	1 - Update priority.
+
 route/max_size - INTEGER
 	Maximum number of routes allowed in the kernel.  Increase
 	this when using large numbers of interfaces and/or routes.
diff --git a/include/net/netns/ipv4.h b/include/net/netns/ipv4.h
index 661348f23ea5..e47503b4e4d1 100644
--- a/include/net/netns/ipv4.h
+++ b/include/net/netns/ipv4.h
@@ -98,6 +98,7 @@ struct netns_ipv4 {
 	int sysctl_ip_default_ttl;
 	int sysctl_ip_no_pmtu_disc;
 	int sysctl_ip_fwd_use_pmtu;
+	int sysctl_ip_fwd_update_priority;
 	int sysctl_ip_nonlocal_bind;
 	/* Shall we try to damage output packets if routing dev changes? */
 	int sysctl_ip_dynaddr;
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index f2a0a3bab6b5..d3cfbd89ca3a 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1802,6 +1802,7 @@ static __net_init int inet_init_net(struct net *net)
 	 * We set them here, in case sysctl is not compiled.
 	 */
 	net->ipv4.sysctl_ip_default_ttl = IPDEFTTL;
+	net->ipv4.sysctl_ip_fwd_update_priority = true;
 	net->ipv4.sysctl_ip_dynaddr = 0;
 	net->ipv4.sysctl_ip_early_demux = 1;
 	net->ipv4.sysctl_udp_early_demux = 1;
diff --git a/net/ipv4/ip_forward.c b/net/ipv4/ip_forward.c
index b54b948b0596..32662e9e5d21 100644
--- a/net/ipv4/ip_forward.c
+++ b/net/ipv4/ip_forward.c
@@ -143,7 +143,8 @@ int ip_forward(struct sk_buff *skb)
 	    !skb_sec_path(skb))
 		ip_rt_send_redirect(skb);
 
-	skb->priority = rt_tos2priority(iph->tos);
+	if (net->ipv4.sysctl_ip_fwd_update_priority)
+		skb->priority = rt_tos2priority(iph->tos);
 
 	return NF_HOOK(NFPROTO_IPV4, NF_INET_FORWARD,
 		       net, NULL, skb, skb->dev, rt->dst.dev,
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index 5fa335fd3852..e21dda015513 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -664,6 +664,15 @@ static struct ctl_table ipv4_net_table[] = {
 		.proc_handler	= proc_dointvec,
 	},
 	{
+		.procname	= "ip_forward_update_priority",
+		.data		= &init_net.ipv4.sysctl_ip_fwd_update_priority,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler   = proc_dointvec_minmax,
+		.extra1		= &zero,
+		.extra2		= &one,
+	},
+	{
 		.procname	= "ip_nonlocal_bind",
 		.data		= &init_net.ipv4.sysctl_ip_nonlocal_bind,
 		.maxlen		= sizeof(int),
-- 
2.4.11

^ permalink raw reply related

* [PATCH net-next 0/7] ipv4: Control SKB reprioritization after forwarding
From: Petr Machata @ 2018-07-31  9:55 UTC (permalink / raw)
  To: netdev, linux-doc, linux-kselftest
  Cc: davem, corbet, jiri, idosch, kuznet, yoshfuji, shuah, nikolay,
	dsahern

After IPv4 packets are forwarded, the priority of the corresponding SKB
is updated according to the TOS field of IPv4 header. This overrides any
prioritization done earlier by e.g. an skbedit action or ingress-qos-map
defined at a vlan device.

Such overriding may not always be desirable. Even if the packet ends up
being routed, which implies this is an L3 network node, an administrator
may wish to preserve whatever prioritization was done earlier on in the
pipeline.

Therefore this patch set introduces a sysctl that controls this
behavior, net.ipv4.ip_forward_update_priority. It's value is 1 by
default to preserve the current behavior.

All of the above is implemented in patch #1.

Value changes prompt a new NETEVENT_IPV4_FWD_UPDATE_PRIORITY_UPDATE
notification, so that the drivers can hook up whatever logic may depend
on this value. That is implemented in patch #2.

In patches #3 and #4, mlxsw is adapted to recognize the sysctl. On
initialization, the RGCR register that handles router configuration is
set in accordance with the sysctl. The new notification is listened to
and RGCR is reconfigured as necessary.

In patches #5 to #7, a selftest is added to verify that mlxsw reflects
the sysctl value as necessary. The test is expressed in terms of the
recently-introduced ieee_setapp support, and works by observing how DSCP
value gets rewritten depending on packet priority. For this reason, the
test is added to the subdirectory drivers/net/mlxsw. Even though it's
not particularly specific to mlxsw, it's not suitable for running on
soft devices (which don't support the ieee_setapp et.al.).

Changes from RFC to v1:

- Fix wrong sysctl name in ip-sysctl.txt
- Add notifications
- Add mlxsw support
- Add self test

Petr Machata (7):
  net: ipv4: Control SKB reprioritization after forwarding
  net: ipv4: Notify about changes to ip_forward_update_priority
  mlxsw: spectrum: Extract work-scheduling into a new function
  mlxsw: spectrum_router: Handle sysctl_ip_fwd_update_priority
  selftests: forwarding: Move lldpad waiting to lib.sh
  selftests: forwarding: Move DSCP capture to lib.sh
  selftests: mlxsw: Add test for ip_forward_update_priority

 Documentation/networking/ip-sysctl.txt             |   9 +
 .../net/ethernet/mellanox/mlxsw/spectrum_router.c  |  56 +++--
 include/net/netevent.h                             |   1 +
 include/net/netns/ipv4.h                           |   1 +
 net/ipv4/af_inet.c                                 |   1 +
 net/ipv4/ip_forward.c                              |   3 +-
 net/ipv4/sysctl_net_ipv4.c                         |  26 +++
 .../selftests/drivers/net/mlxsw/qos_dscp_bridge.sh |  65 +-----
 .../selftests/drivers/net/mlxsw/qos_dscp_router.sh | 233 +++++++++++++++++++++
 tools/testing/selftests/net/forwarding/lib.sh      |  63 ++++++
 10 files changed, 379 insertions(+), 79 deletions(-)
 create mode 100755 tools/testing/selftests/drivers/net/mlxsw/qos_dscp_router.sh

-- 
2.4.11

^ permalink raw reply

* Re: unregister_netdevice: waiting for DEV to become free
From: Steffen Klassert @ 2018-07-31 11:31 UTC (permalink / raw)
  To: Tetsuo Handa
  Cc: Herbert Xu, David S. Miller, syzbot, ddstreet, dvyukov,
	linux-kernel, netdev, syzkaller-bugs
In-Reply-To: <11ddcad9-f3fa-dfbc-18a0-e5facc37a4a3@I-love.SAKURA.ne.jp>

On Tue, Jul 31, 2018 at 08:16:22PM +0900, Tetsuo Handa wrote:
> Steffen and Herbert,
> 
> Do you have any question? I think I provided enough information for debugging.

It seems that I was not on Cc at the beginning of the threat,
so I had to search the web for some context.

I'm currently trying your reproducer (still with my .config) but I don't
hit the problem.

> 
> This problem occurs because two dev_put() calls are missing (compared with not
> calling setsockopt(SOL_IPV6, IPV6_XFRM_POLICY)) because dst_release() is not
> called via fib6_info_destroy_rcu() when we called xfrm_compile_policy() from
> xfrm_user_policy() from setsockopt(SOL_IPV6, IPV6_XFRM_POLICY).

Thanks for the information! As said, I'm already working on it.

^ permalink raw reply

* re: [PATCH] cfg80211: read wmm rules from regulatory database
From: Colin Ian King @ 2018-07-31 11:27 UTC (permalink / raw)
  To: Haim Dreyfuss, David S. Miller, Johannes Berg, netdev,
	linux-wireless@vger.kernel.org
  Cc: linux-kernel@vger.kernel.org

Hi Haim,

I think there may be an issue with the commit:

>From 230ebaa189af44d50dccb4a1846e39ca594e347b Mon Sep 17 00:00:00 2001
From: Haim Dreyfuss <haim.dreyfuss@intel.com>
Date: Wed, 28 Mar 2018 13:24:09 +0300
Subject: [PATCH] cfg80211: read wmm rules from regulatory database

specifically in function: reg_copy_regd()

+       for (i = 0; i < src_regd->n_reg_rules; i++) {
                memcpy(&regd->reg_rules[i], &src_regd->reg_rules[i],
                       sizeof(struct ieee80211_reg_rule));
+               if (!src_regd->reg_rules[i].wmm_rule)
+                       continue;

+               regd->reg_rules[i].wmm_rule = d_wmm +
+                       (src_regd->reg_rules[i].wmm_rule - s_wmm) /
+                       sizeof(struct ieee80211_wmm_rule);
+       }

The pointer arithmetic (src_regd->reg_rules[i].wmm_rule - s_wmm) is
performed in terms of the size of struct ieee80211_wmm_rule and not in
bytes and I believe that the division by sizeof(struct
ieee80211_wmm_rule) is not required.

This issue was detected by static analysis with Coverity Scan,
CID#1467451 ("Extra sizeof expression"), 'suspicious_division'

I'm not 100% sure that is this a false positive or not, but I think it
looks incorrect to me.

Colin

^ permalink raw reply

* Re: [PATCH net-next 2/2] virtio-net: get rid of unnecessary container of rq stats
From: Michael S. Tsirkin @ 2018-07-31 11:22 UTC (permalink / raw)
  To: Jason Wang; +Cc: virtualization, netdev, linux-kernel, Toshiaki Makita
In-Reply-To: <1533030219-9904-2-git-send-email-jasowang@redhat.com>

On Tue, Jul 31, 2018 at 05:43:39PM +0800, Jason Wang wrote:
> We don't maintain tx counters in rx stats any more. There's no need
> for an extra container of rq stats.
> 
> Cc: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
> Signed-off-by: Jason Wang <jasowang@redhat.com>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  drivers/net/virtio_net.c | 80 ++++++++++++++++++++++--------------------------
>  1 file changed, 36 insertions(+), 44 deletions(-)
> 
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 72d3f68..14f661c 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -87,7 +87,8 @@ struct virtnet_sq_stats {
>  	u64 kicks;
>  };
>  
> -struct virtnet_rq_stat_items {
> +struct virtnet_rq_stats {
> +	struct u64_stats_sync syncp;
>  	u64 packets;
>  	u64 bytes;
>  	u64 drops;
> @@ -98,17 +99,8 @@ struct virtnet_rq_stat_items {
>  	u64 kicks;
>  };
>  
> -struct virtnet_rq_stats {
> -	struct u64_stats_sync syncp;
> -	struct virtnet_rq_stat_items items;
> -};
> -
> -struct virtnet_rx_stats {
> -	struct virtnet_rq_stat_items rx;
> -};
> -
>  #define VIRTNET_SQ_STAT(m)	offsetof(struct virtnet_sq_stats, m)
> -#define VIRTNET_RQ_STAT(m)	offsetof(struct virtnet_rq_stat_items, m)
> +#define VIRTNET_RQ_STAT(m)	offsetof(struct virtnet_rq_stats, m)
>  
>  static const struct virtnet_stat_desc virtnet_sq_stats_desc[] = {
>  	{ "packets",		VIRTNET_SQ_STAT(packets) },
> @@ -617,7 +609,7 @@ static struct sk_buff *receive_small(struct net_device *dev,
>  				     void *buf, void *ctx,
>  				     unsigned int len,
>  				     unsigned int *xdp_xmit,
> -				     struct virtnet_rx_stats *stats)
> +				     struct virtnet_rq_stats *stats)
>  {
>  	struct sk_buff *skb;
>  	struct bpf_prog *xdp_prog;
> @@ -632,7 +624,7 @@ static struct sk_buff *receive_small(struct net_device *dev,
>  	int err;
>  
>  	len -= vi->hdr_len;
> -	stats->rx.bytes += len;
> +	stats->bytes += len;
>  
>  	rcu_read_lock();
>  	xdp_prog = rcu_dereference(rq->xdp_prog);
> @@ -674,7 +666,7 @@ static struct sk_buff *receive_small(struct net_device *dev,
>  		xdp.rxq = &rq->xdp_rxq;
>  		orig_data = xdp.data;
>  		act = bpf_prog_run_xdp(xdp_prog, &xdp);
> -		stats->rx.xdp_packets++;
> +		stats->xdp_packets++;
>  
>  		switch (act) {
>  		case XDP_PASS:
> @@ -683,7 +675,7 @@ static struct sk_buff *receive_small(struct net_device *dev,
>  			len = xdp.data_end - xdp.data;
>  			break;
>  		case XDP_TX:
> -			stats->rx.xdp_tx++;
> +			stats->xdp_tx++;
>  			xdpf = convert_to_xdp_frame(&xdp);
>  			if (unlikely(!xdpf))
>  				goto err_xdp;
> @@ -696,7 +688,7 @@ static struct sk_buff *receive_small(struct net_device *dev,
>  			rcu_read_unlock();
>  			goto xdp_xmit;
>  		case XDP_REDIRECT:
> -			stats->rx.xdp_redirects++;
> +			stats->xdp_redirects++;
>  			err = xdp_do_redirect(dev, &xdp, xdp_prog);
>  			if (err)
>  				goto err_xdp;
> @@ -730,8 +722,8 @@ static struct sk_buff *receive_small(struct net_device *dev,
>  
>  err_xdp:
>  	rcu_read_unlock();
> -	stats->rx.xdp_drops++;
> -	stats->rx.drops++;
> +	stats->xdp_drops++;
> +	stats->drops++;
>  	put_page(page);
>  xdp_xmit:
>  	return NULL;
> @@ -742,19 +734,19 @@ static struct sk_buff *receive_big(struct net_device *dev,
>  				   struct receive_queue *rq,
>  				   void *buf,
>  				   unsigned int len,
> -				   struct virtnet_rx_stats *stats)
> +				   struct virtnet_rq_stats *stats)
>  {
>  	struct page *page = buf;
>  	struct sk_buff *skb = page_to_skb(vi, rq, page, 0, len, PAGE_SIZE);
>  
> -	stats->rx.bytes += len - vi->hdr_len;
> +	stats->bytes += len - vi->hdr_len;
>  	if (unlikely(!skb))
>  		goto err;
>  
>  	return skb;
>  
>  err:
> -	stats->rx.drops++;
> +	stats->drops++;
>  	give_pages(rq, page);
>  	return NULL;
>  }
> @@ -766,7 +758,7 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
>  					 void *ctx,
>  					 unsigned int len,
>  					 unsigned int *xdp_xmit,
> -					 struct virtnet_rx_stats *stats)
> +					 struct virtnet_rq_stats *stats)
>  {
>  	struct virtio_net_hdr_mrg_rxbuf *hdr = buf;
>  	u16 num_buf = virtio16_to_cpu(vi->vdev, hdr->num_buffers);
> @@ -779,7 +771,7 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
>  	int err;
>  
>  	head_skb = NULL;
> -	stats->rx.bytes += len - vi->hdr_len;
> +	stats->bytes += len - vi->hdr_len;
>  
>  	rcu_read_lock();
>  	xdp_prog = rcu_dereference(rq->xdp_prog);
> @@ -828,7 +820,7 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
>  		xdp.rxq = &rq->xdp_rxq;
>  
>  		act = bpf_prog_run_xdp(xdp_prog, &xdp);
> -		stats->rx.xdp_packets++;
> +		stats->xdp_packets++;
>  
>  		switch (act) {
>  		case XDP_PASS:
> @@ -853,7 +845,7 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
>  			}
>  			break;
>  		case XDP_TX:
> -			stats->rx.xdp_tx++;
> +			stats->xdp_tx++;
>  			xdpf = convert_to_xdp_frame(&xdp);
>  			if (unlikely(!xdpf))
>  				goto err_xdp;
> @@ -870,7 +862,7 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
>  			rcu_read_unlock();
>  			goto xdp_xmit;
>  		case XDP_REDIRECT:
> -			stats->rx.xdp_redirects++;
> +			stats->xdp_redirects++;
>  			err = xdp_do_redirect(dev, &xdp, xdp_prog);
>  			if (err) {
>  				if (unlikely(xdp_page != page))
> @@ -920,7 +912,7 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
>  			goto err_buf;
>  		}
>  
> -		stats->rx.bytes += len;
> +		stats->bytes += len;
>  		page = virt_to_head_page(buf);
>  
>  		truesize = mergeable_ctx_to_truesize(ctx);
> @@ -966,7 +958,7 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
>  
>  err_xdp:
>  	rcu_read_unlock();
> -	stats->rx.xdp_drops++;
> +	stats->xdp_drops++;
>  err_skb:
>  	put_page(page);
>  	while (num_buf-- > 1) {
> @@ -977,12 +969,12 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
>  			dev->stats.rx_length_errors++;
>  			break;
>  		}
> -		stats->rx.bytes += len;
> +		stats->bytes += len;
>  		page = virt_to_head_page(buf);
>  		put_page(page);
>  	}
>  err_buf:
> -	stats->rx.drops++;
> +	stats->drops++;
>  	dev_kfree_skb(head_skb);
>  xdp_xmit:
>  	return NULL;
> @@ -991,7 +983,7 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
>  static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
>  			void *buf, unsigned int len, void **ctx,
>  			unsigned int *xdp_xmit,
> -			struct virtnet_rx_stats *stats)
> +			struct virtnet_rq_stats *stats)
>  {
>  	struct net_device *dev = vi->dev;
>  	struct sk_buff *skb;
> @@ -1212,7 +1204,7 @@ static bool try_fill_recv(struct virtnet_info *vi, struct receive_queue *rq,
>  	} while (rq->vq->num_free);
>  	if (virtqueue_kick_prepare(rq->vq) && virtqueue_notify(rq->vq)) {
>  		u64_stats_update_begin(&rq->stats.syncp);
> -		rq->stats.items.kicks++;
> +		rq->stats.kicks++;
>  		u64_stats_update_end(&rq->stats.syncp);
>  	}
>  
> @@ -1290,7 +1282,7 @@ static int virtnet_receive(struct receive_queue *rq, int budget,
>  			   unsigned int *xdp_xmit)
>  {
>  	struct virtnet_info *vi = rq->vq->vdev->priv;
> -	struct virtnet_rx_stats stats = {};
> +	struct virtnet_rq_stats stats = {};
>  	unsigned int len;
>  	void *buf;
>  	int i;
> @@ -1298,16 +1290,16 @@ static int virtnet_receive(struct receive_queue *rq, int budget,
>  	if (!vi->big_packets || vi->mergeable_rx_bufs) {
>  		void *ctx;
>  
> -		while (stats.rx.packets < budget &&
> +		while (stats.packets < budget &&
>  		       (buf = virtqueue_get_buf_ctx(rq->vq, &len, &ctx))) {
>  			receive_buf(vi, rq, buf, len, ctx, xdp_xmit, &stats);
> -			stats.rx.packets++;
> +			stats.packets++;
>  		}
>  	} else {
> -		while (stats.rx.packets < budget &&
> +		while (stats.packets < budget &&
>  		       (buf = virtqueue_get_buf(rq->vq, &len)) != NULL) {
>  			receive_buf(vi, rq, buf, len, NULL, xdp_xmit, &stats);
> -			stats.rx.packets++;
> +			stats.packets++;
>  		}
>  	}
>  
> @@ -1321,12 +1313,12 @@ static int virtnet_receive(struct receive_queue *rq, int budget,
>  		size_t offset = virtnet_rq_stats_desc[i].offset;
>  		u64 *item;
>  
> -		item = (u64 *)((u8 *)&rq->stats.items + offset);
> -		*item += *(u64 *)((u8 *)&stats.rx + offset);
> +		item = (u64 *)((u8 *)&rq->stats + offset);
> +		*item += *(u64 *)((u8 *)&stats + offset);
>  	}
>  	u64_stats_update_end(&rq->stats.syncp);
>  
> -	return stats.rx.packets;
> +	return stats.packets;
>  }
>  
>  static void free_old_xmit_skbs(struct send_queue *sq)
> @@ -1686,9 +1678,9 @@ static void virtnet_stats(struct net_device *dev,
>  
>  		do {
>  			start = u64_stats_fetch_begin_irq(&rq->stats.syncp);
> -			rpackets = rq->stats.items.packets;
> -			rbytes   = rq->stats.items.bytes;
> -			rdrops   = rq->stats.items.drops;
> +			rpackets = rq->stats.packets;
> +			rbytes   = rq->stats.bytes;
> +			rdrops   = rq->stats.drops;
>  		} while (u64_stats_fetch_retry_irq(&rq->stats.syncp, start));
>  
>  		tot->rx_packets += rpackets;
> @@ -2078,7 +2070,7 @@ static void virtnet_get_ethtool_stats(struct net_device *dev,
>  	for (i = 0; i < vi->curr_queue_pairs; i++) {
>  		struct receive_queue *rq = &vi->rq[i];
>  
> -		stats_base = (u8 *)&rq->stats.items;
> +		stats_base = (u8 *)&rq->stats;
>  		do {
>  			start = u64_stats_fetch_begin_irq(&rq->stats.syncp);
>  			for (j = 0; j < VIRTNET_RQ_STATS_LEN; j++) {
> -- 
> 2.7.4

^ permalink raw reply

* Re: [PATCH net-next 1/2] virtio-net: correctly update XDP_TX counters
From: Michael S. Tsirkin @ 2018-07-31 11:22 UTC (permalink / raw)
  To: Jason Wang; +Cc: netdev, linux-kernel, virtualization
In-Reply-To: <1533030219-9904-1-git-send-email-jasowang@redhat.com>

On Tue, Jul 31, 2018 at 05:43:38PM +0800, Jason Wang wrote:
> Commit 5b8f3c8d30a6 ("virtio_net: Add XDP related stats") tries to
> count TX XDP stats in virtnet_receive(). This will cause several
> issues:
> 
> - virtnet_xdp_sq() was called without checking whether or not XDP is
>   set. This may cause out of bound access when there's no enough txq
>   for XDP.
> - Stats were updated even if there's no XDP/XDP_TX.
> 
> Fixing this by reusing virtnet_xdp_xmit() for XDP_TX which can counts
> TX XDP counter itself and remove the unnecessary tx stats embedded in
> rx stats.
> 
> Reported-by: syzbot+604f8271211546f5b3c7@syzkaller.appspotmail.com
> Fixes: 5b8f3c8d30a6 ("virtio_net: Add XDP related stats")
> Cc: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
> Signed-off-by: Jason Wang <jasowang@redhat.com>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  drivers/net/virtio_net.c | 39 ++++-----------------------------------
>  1 file changed, 4 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 1880c86..72d3f68 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -105,10 +105,6 @@ struct virtnet_rq_stats {
>  
>  struct virtnet_rx_stats {
>  	struct virtnet_rq_stat_items rx;
> -	struct {
> -		unsigned int xdp_tx;
> -		unsigned int xdp_tx_drops;
> -	} tx;
>  };
>  
>  #define VIRTNET_SQ_STAT(m)	offsetof(struct virtnet_sq_stats, m)
> @@ -485,22 +481,6 @@ static struct send_queue *virtnet_xdp_sq(struct virtnet_info *vi)
>  	return &vi->sq[qp];
>  }
>  
> -static int __virtnet_xdp_tx_xmit(struct virtnet_info *vi,
> -				   struct xdp_frame *xdpf)
> -{
> -	struct xdp_frame *xdpf_sent;
> -	struct send_queue *sq;
> -	unsigned int len;
> -
> -	sq = virtnet_xdp_sq(vi);
> -
> -	/* Free up any pending old buffers before queueing new ones. */
> -	while ((xdpf_sent = virtqueue_get_buf(sq->vq, &len)) != NULL)
> -		xdp_return_frame(xdpf_sent);
> -
> -	return __virtnet_xdp_xmit_one(vi, sq, xdpf);
> -}
> -
>  static int virtnet_xdp_xmit(struct net_device *dev,
>  			    int n, struct xdp_frame **frames, u32 flags)
>  {
> @@ -707,10 +687,8 @@ static struct sk_buff *receive_small(struct net_device *dev,
>  			xdpf = convert_to_xdp_frame(&xdp);
>  			if (unlikely(!xdpf))
>  				goto err_xdp;
> -			stats->tx.xdp_tx++;
> -			err = __virtnet_xdp_tx_xmit(vi, xdpf);
> -			if (unlikely(err)) {
> -				stats->tx.xdp_tx_drops++;
> +			err = virtnet_xdp_xmit(dev, 1, &xdpf, 0);
> +			if (unlikely(err < 0)) {
>  				trace_xdp_exception(vi->dev, xdp_prog, act);
>  				goto err_xdp;
>  			}
> @@ -879,10 +857,8 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
>  			xdpf = convert_to_xdp_frame(&xdp);
>  			if (unlikely(!xdpf))
>  				goto err_xdp;
> -			stats->tx.xdp_tx++;
> -			err = __virtnet_xdp_tx_xmit(vi, xdpf);
> -			if (unlikely(err)) {
> -				stats->tx.xdp_tx_drops++;
> +			err = virtnet_xdp_xmit(dev, 1, &xdpf, 0);
> +			if (unlikely(err < 0)) {
>  				trace_xdp_exception(vi->dev, xdp_prog, act);
>  				if (unlikely(xdp_page != page))
>  					put_page(xdp_page);
> @@ -1315,7 +1291,6 @@ static int virtnet_receive(struct receive_queue *rq, int budget,
>  {
>  	struct virtnet_info *vi = rq->vq->vdev->priv;
>  	struct virtnet_rx_stats stats = {};
> -	struct send_queue *sq;
>  	unsigned int len;
>  	void *buf;
>  	int i;
> @@ -1351,12 +1326,6 @@ static int virtnet_receive(struct receive_queue *rq, int budget,
>  	}
>  	u64_stats_update_end(&rq->stats.syncp);
>  
> -	sq = virtnet_xdp_sq(vi);
> -	u64_stats_update_begin(&sq->stats.syncp);
> -	sq->stats.xdp_tx += stats.tx.xdp_tx;
> -	sq->stats.xdp_tx_drops += stats.tx.xdp_tx_drops;
> -	u64_stats_update_end(&sq->stats.syncp);
> -
>  	return stats.rx.packets;
>  }
>  
> -- 
> 2.7.4

^ permalink raw reply

* Re: [PATCH net-next v5 1/4] net/sched: user-space can't set unknown tcfa_action values
From: Paolo Abeni @ 2018-07-31  9:41 UTC (permalink / raw)
  To: Jamal Hadi Salim, netdev
  Cc: Cong Wang, Jiri Pirko, Daniel Borkmann, Marcelo Ricardo Leitner,
	Eyal Birger, David S. Miller
In-Reply-To: <ab896aa9-9fa5-693c-2c56-144f439e242e@mojatatu.com>

On Mon, 2018-07-30 at 15:31 -0400, Jamal Hadi Salim wrote:
> On 30/07/18 12:41 PM, Paolo Abeni wrote:
> > On Mon, 2018-07-30 at 10:03 -0400, Jamal Hadi Salim wrote:
> > > On 30/07/18 08:30 AM, Paolo Abeni wrote:
> > > >    	}
> > > >    
> > > > +	if (!tcf_action_valid(a->tcfa_action)) {
> > > > +		NL_SET_ERR_MSG(extack, "invalid action value, using TC_ACT_UNSPEC instead");
> > > > +		a->tcfa_action = TC_ACT_UNSPEC;
> > > > +	}
> > > > +
> > > >    	return a;
> > > >    
> > > 
> > > 
> > > I think it would make a lot more sense to just reject the entry than
> > > changing it underneath the user to a default value. Least element of
> > > suprise.
> > 
> > I fear that would break existing (bad) users ?!? This way, such users
> > are notified they are doing something uncorrect, but still continue to
> > work.
> 
> 
> By "bad users" I think you mean someone setting a policy expecting
> one behavior but getting a different one? 

Generally speaking I thought about user-space pushing to the kernel
some uninitialized/random value for 'tcfa_action'.

> If yes, that policy was
> already wrong/buggy. As an example, if i configured:
> 
> match xxx action foo action goo action bar action gah
> 
> where action goo has a bad opcode
> If  you "fix it"  with TC_ACT_UNSPEC then basically the above
> policy is now equivalent to:
> 
> match xxx action foo action goo
> 
> Infact if there was a lower prio rule in the chain
> then lookup will continue there and produce even stranger
> results.

I see. 

Before this patch, the kernel exposed the same behaviour for negative
value of 'bar', while, for positive 'bar' values, the overall behaviour
was more complex (some classifier always stops with unknown positive
action value, others go to lower prio).

Overall, the kernel behavior should be more well-defined now, but yes,
there is a change of behavior under some circumstances.

What about instead mapping undefined/unknown actions value to TC_ACT_OK
(still at initialization time)? this is what is already done by
tcf_action_exec() for faulty opcodes/graphs and by tcf_ipt() and would
handle the above example more conistently.

Cheers,

Paolo

^ permalink raw reply

* pull-request: wireless-drivers 2018-07-31
From: Kalle Valo @ 2018-07-31 10:48 UTC (permalink / raw)
  To: David Miller; +Cc: linux-wireless, netdev, linux-kernel

Hi Dave,

two small patches I still would like to get to the net tree for v4.18.
Please let me know if you have any problems.

Kalle


The following changes since commit 144fe2bfd236dc814eae587aea7e2af03dbdd755:

  sock: fix sg page frag coalescing in sk_alloc_sg (2018-07-23 21:28:45 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers.git tags/wireless-drivers-for-davem-2018-07-31

for you to fetch changes up to 299b6365a3b7cf7f5ea1c945a420e9ee4841d6f7:

  brcmfmac: fix regression in parsing NVRAM for multiple devices (2018-07-25 10:30:36 +0300)

----------------------------------------------------------------
wireless-drivers fixes for 4.18

Last set of fixes before 4.18 is released

iwlwifi

* add new IDs for cards already available on the market

brcmfmac

* fix a regression introduced in v4.17

----------------------------------------------------------------
Emmanuel Grumbach (1):
      iwlwifi: add more card IDs for 9000 series

Rafał Miłecki (1):
      brcmfmac: fix regression in parsing NVRAM for multiple devices

 .../wireless/broadcom/brcm80211/brcmfmac/pcie.c    |  3 +-
 drivers/net/wireless/intel/iwlwifi/cfg/9000.c      | 69 ++++++++++++++++++++++
 drivers/net/wireless/intel/iwlwifi/iwl-config.h    |  5 ++
 drivers/net/wireless/intel/iwlwifi/pcie/drv.c      | 22 +++++++
 4 files changed, 98 insertions(+), 1 deletion(-)

-- 
Kalle Valo

^ permalink raw reply

* RE: [Intel-wired-lan] [PATCH] net: intel: ixgbe: Replace GFP_ATOMIC with GFP_KERNEL
From: Basierski, SebastianX @ 2018-07-31 10:31 UTC (permalink / raw)
  To: Jia-Ju Bai, Kirsher, Jeffrey T
  Cc: netdev@vger.kernel.org, intel-wired-lan@lists.osuosl.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <20180727082231.3798-1-baijiaju1990@gmail.com>

Acked-by: Sebastian Basierski <sebastianx.basierski@intel.com>

Pozdrawiam
Sebastian Basierski
SII Engineer
Delivering outsourced services to Intel
e-mail: sebastianx.basierski@intel.com




-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On Behalf Of Jia-Ju Bai
Sent: Friday, July 27, 2018 10:23 AM
To: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>
Cc: netdev@vger.kernel.org; Jia-Ju Bai <baijiaju1990@gmail.com>; intel-wired-lan@lists.osuosl.org; linux-kernel@vger.kernel.org
Subject: [Intel-wired-lan] [PATCH] net: intel: ixgbe: Replace GFP_ATOMIC with GFP_KERNEL

ixgbe_fcoe_ddp_setup(), ixgbe_setup_fcoe_ddp_resources() and
ixgbe_sw_init() are never called in atomic context.
They call kmalloc(), dma_pool_alloc() and kzalloc() with GFP_ATOMIC, which is not necessary.
GFP_ATOMIC can be replaced with GFP_KERNEL.

This is found by a static analysis tool named DCNS written by myself.

Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c | 4 ++--  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
index 7a09a40e4472..ff7ed77ce224 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
@@ -217,7 +217,7 @@ static int ixgbe_fcoe_ddp_setup(struct net_device *netdev, u16 xid,
 	}
 
 	/* alloc the udl from per cpu ddp pool */
-	ddp->udl = dma_pool_alloc(ddp_pool->pool, GFP_ATOMIC, &ddp->udp);
+	ddp->udl = dma_pool_alloc(ddp_pool->pool, GFP_KERNEL, &ddp->udp);
 	if (!ddp->udl) {
 		e_err(drv, "failed allocated ddp context\n");
 		goto out_noddp_unmap;
@@ -785,7 +785,7 @@ int ixgbe_setup_fcoe_ddp_resources(struct ixgbe_adapter *adapter)
 		return 0;
 
 	/* Extra buffer to be shared by all DDPs for HW work around */
-	buffer = kmalloc(IXGBE_FCBUFF_MIN, GFP_ATOMIC);
+	buffer = kmalloc(IXGBE_FCBUFF_MIN, GFP_KERNEL);
 	if (!buffer)
 		return -ENOMEM;
 
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index afadba99f7b8..fe4a6125576d 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -6138,7 +6138,7 @@ static int ixgbe_sw_init(struct ixgbe_adapter *adapter,
 
 	adapter->mac_table = kzalloc(sizeof(struct ixgbe_mac_addr) *
 				     hw->mac.num_rar_entries,
-				     GFP_ATOMIC);
+				     GFP_KERNEL);
 	if (!adapter->mac_table)
 		return -ENOMEM;
 
--
2.17.0

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

^ permalink raw reply related

* Re: [patch net-next] net: sched: don't dump chains only held by actions
From: Jiri Pirko @ 2018-07-31  8:48 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Cong Wang, Linux Kernel Network Developers, David Miller,
	Jamal Hadi Salim, mlxsw
In-Reply-To: <20180731010146.0706283a@cakuba.netronome.com>

Tue, Jul 31, 2018 at 10:01:46AM CEST, jakub.kicinski@netronome.com wrote:
>On Tue, 31 Jul 2018 08:32:58 +0200, Jiri Pirko wrote:
>> Mon, Jul 30, 2018 at 08:19:56PM CEST, xiyou.wangcong@gmail.com wrote:
>> >On Sun, Jul 29, 2018 at 12:54 AM Jiri Pirko <jiri@resnulli.us> wrote:  
>> >>
>> >> Sat, Jul 28, 2018 at 07:39:36PM CEST, xiyou.wangcong@gmail.com wrote:  
>> >> >On Sat, Jul 28, 2018 at 10:20 AM Cong Wang <xiyou.wangcong@gmail.com> wrote:  
>> >> >>
>> >> >> On Fri, Jul 27, 2018 at 12:47 AM Jiri Pirko <jiri@resnulli.us> wrote:  
>> >> >> >
>> >> >> > From: Jiri Pirko <jiri@mellanox.com>
>> >> >> >
>> >> >> > In case a chain is empty and not explicitly created by a user,
>> >> >> > such chain should not exist. The only exception is if there is
>> >> >> > an action "goto chain" pointing to it. In that case, don't show the
>> >> >> > chain in the dump. Track the chain references held by actions and
>> >> >> > use them to find out if a chain should or should not be shown
>> >> >> > in chain dump.
>> >> >> >
>> >> >> > Signed-off-by: Jiri Pirko <jiri@mellanox.com>  
>> >> >>
>> >> >> Looks reasonable to me.
>> >> >>
>> >> >> Acked-by: Cong Wang <xiyou.wangcong@gmail.com>  
>> >> >
>> >> >Hold on...
>> >> >
>> >> >If you increase the refcnt for a zombie chain on NEWCHAIN path,
>> >> >then it would become a non-zombie, this makes sense. However,
>> >> >if the action_refcnt gets increased again when another action uses it,
>> >> >it become a zombie again because refcnt==action_refcnt??  
>> >>
>> >> No. action always increases both refcnt and action_refcnt  
>> >
>> >Hmm, then the name zombie is confusing, with your definition all
>> >chains implicitly created by actions are zombies, unless touched
>> >by user explicitly. Please find a better name.  
>> 
>> Okay. Perhaps chain_inactive?
>
>FWIW to me active brings to mind that it's handling traffic.  Brining in
>my suggestions from an off-list discussion:
>
>tcf_chain_act_refs_only() or tcf_chain_pure_act_target()

:/

>
>or maybe tcf_chain_has_no_filters() ?

That is not accurate, as explicitly created chain does not have any
filters too.

I think this is good:
tcf_chain_held_by_acts_only()

^ 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