Netdev List
 help / color / mirror / Atom feed
* Re: WARNING in refcount_dec
From: Cong Wang @ 2018-03-28 23:16 UTC (permalink / raw)
  To: Byoungyoung Lee
  Cc: LKML, DaeLyong Jeong, Kyungtae Kim,
	Linux Kernel Network Developers, Willem de Bruijn
In-Reply-To: <CAG5TwLORDZPmfF7P0S2y9_7PscjvcuQd4WYnZTsBwCi+y47dOQ@mail.gmail.com>

(Cc'ing netdev and Willem)

On Wed, Mar 28, 2018 at 12:03 PM, Byoungyoung Lee
<byoungyoung@purdue.edu> wrote:
> Another crash patterns observed: race between (setsockopt$packet_int)
> and (bind$packet).
>
> ------------------------------
> [  357.731597] kernel BUG at
> /home/blee/project/race-fuzzer/kernels/kernel_v4.16-rc3/net/packet/af_packet.c:3107!
> [  357.733382] invalid opcode: 0000 [#1] SMP KASAN
> [  357.734017] Modules linked in:
> [  357.734662] CPU: 1 PID: 3871 Comm: repro.exe Not tainted 4.16.0-rc3 #1
> [  357.735791] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
> BIOS rel-1.8.2-0-g33fbe13 by qemu-project.org 04/01/2014
> [  357.737434] RIP: 0010:packet_do_bind+0x88d/0x950
> [  357.738121] RSP: 0018:ffff8800b2787b08 EFLAGS: 00010293
> [  357.738906] RAX: ffff8800b2fdc780 RBX: ffff880234358cc0 RCX: ffffffff838b244c
> [  357.739905] RDX: 0000000000000000 RSI: ffffffff838b257d RDI: 0000000000000001
> [  357.741315] RBP: ffff8800b2787c10 R08: ffff8800b2fdc780 R09: 0000000000000000
> [  357.743055] R10: 0000000000000001 R11: 0000000000000000 R12: ffff88023352ecc0
> [  357.744744] R13: 0000000000000000 R14: 0000000000000001 R15: 0000000000001d00
> [  357.746377] FS:  00007f4b43733700(0000) GS:ffff8800b8b00000(0000)
> knlGS:0000000000000000
> [  357.749599] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [  357.752096] CR2: 0000000020058000 CR3: 00000002334b8000 CR4: 00000000000006e0
> [  357.755045] Call Trace:
> [  357.755822]  ? compat_packet_setsockopt+0x100/0x100
> [  357.757324]  ? __sanitizer_cov_trace_const_cmp8+0x18/0x20
> [  357.758810]  packet_bind+0xa2/0xe0
> [  357.759640]  SYSC_bind+0x279/0x2f0
> [  357.760364]  ? move_addr_to_kernel.part.19+0xc0/0xc0
> [  357.761491]  ? __handle_mm_fault+0x25d0/0x25d0
> [  357.762449]  ? __sanitizer_cov_trace_const_cmp4+0x16/0x20
> [  357.763663]  ? __do_page_fault+0x417/0xba0
> [  357.764569]  ? vmalloc_fault+0x910/0x910
> [  357.765405]  ? __sanitizer_cov_trace_const_cmp4+0x16/0x20
> [  357.766525]  ? mark_held_locks+0x25/0xb0
> [  357.767336]  ? SyS_socketpair+0x4a0/0x4a0
> [  357.768182]  SyS_bind+0x24/0x30
> [  357.768851]  do_syscall_64+0x209/0x5d0
> [  357.769650]  ? syscall_return_slowpath+0x3e0/0x3e0
> [  357.770665]  ? __sanitizer_cov_trace_const_cmp4+0x16/0x20
> [  357.771779]  ? syscall_return_slowpath+0x260/0x3e0
> [  357.772748]  ? mark_held_locks+0x25/0xb0
> [  357.773581]  ? __sanitizer_cov_trace_const_cmp4+0x16/0x20
> [  357.774720]  ? retint_user+0x18/0x18
> [  357.775493]  ? trace_hardirqs_off_caller+0xb5/0x120
> [  357.776567]  ? trace_hardirqs_off_thunk+0x1a/0x1c
> [  357.777512]  entry_SYSCALL_64_after_hwframe+0x42/0xb7
> [  357.778508] RIP: 0033:0x4503a9
> [  357.779156] RSP: 002b:00007f4b43732ce8 EFLAGS: 00000246 ORIG_RAX:
> 0000000000000031
> [  357.780737] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00000000004503a9
> [  357.782169] RDX: 0000000000000014 RSI: 0000000020058000 RDI: 0000000000000003
> [  357.783710] RBP: 00007f4b43732d10 R08: 0000000000000000 R09: 0000000000000000
> [  357.785202] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
> [  357.786664] R13: 0000000000000000 R14: 00007f4b437339c0 R15: 00007f4b43733700
> [  357.788210] Code: c0 fd 48 c7 c2 00 c8 d9 84 be ab 02 00 00 48 c7
> c7 60 c8 d9 84 c6 05 e7 a2 48 02 01 e8 3f 17 af fd e9 60 fb ff ff e8
> 43 b3 c0 fd <0f> 0b e8 3c b3 c0 fd 48 8b bd 20 ff ff ff e8 60 1e e7 fd
> 4c 89
> [  357.792260] RIP: packet_do_bind+0x88d/0x950 RSP: ffff8800b2787b08
> [  357.793698] ---[ end trace 0c5a2539f0247369 ]---
> [  357.794696] Kernel panic - not syncing: Fatal exception
> [  357.795918] Kernel Offset: disabled
> [  357.796614] Rebooting in 86400 seconds..
>
> On Wed, Mar 28, 2018 at 1:19 AM, Byoungyoung Lee <byoungyoung@purdue.edu> wrote:
>> We report the crash: WARNING in refcount_dec
>>
>> This crash has been found in v4.16-rc3 using RaceFuzzer (a modified
>> version of Syzkaller), which we describe more at the end of this
>> report. Our analysis shows that the race occurs when invoking two
>> syscalls concurrently, (setsockopt$packet_int) and
>> (setsockopt$packet_rx_ring).
>>
>> C repro code : https://kiwi.cs.purdue.edu/static/race-fuzzer/repro-refcount_dec.c
>> kernel config: https://kiwi.cs.purdue.edu/static/race-fuzzer/kernel-config-v4.16-rc3


I tried your reproducer, no luck here.



>>
>> ---------------------------------------
>> [  305.838560] refcount_t: decrement hit 0; leaking memory.
>> [  305.839669] WARNING: CPU: 0 PID: 3867 at
>> /home/blee/project/race-fuzzer/kernels/kernel_v4.16-rc3/lib/refcount.c:228
>> refcount_dec+0x62/0x70
>> [  305.841441] Modules linked in:
>> [  305.841883] CPU: 0 PID: 3867 Comm: repro.exe Not tainted 4.16.0-rc3 #1
>> [  305.842803] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
>> BIOS rel-1.8.2-0-g33fbe13 by qemu-project.org 04/01/2014
>> [  305.844345] RIP: 0010:refcount_dec+0x62/0x70
>> [  305.845005] RSP: 0018:ffff880224d374f8 EFLAGS: 00010286
>> [  305.845802] RAX: 000000000000002c RBX: 0000000000000000 RCX: ffffffff81538991
>> [  305.846768] RDX: 0000000000000000 RSI: ffffffff813cd761 RDI: 0000000000000005
>> [  305.847748] RBP: ffff880224d37500 R08: ffff88023169a440 R09: 0000000000000000
>> [  305.848748] R10: 0000000000000000 R11: 0000000000000000 R12: ffff88023473ad40
>> [  305.849738] R13: ffff88023473b368 R14: 0000000000000001 R15: 0000000000000000
>> [  305.850733] FS:  0000000000c6e940(0000) GS:ffff8800b8a00000(0000)
>> knlGS:0000000000000000
>> [  305.851837] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> [  305.852652] CR2: 00007fb120571db8 CR3: 0000000005422000 CR4: 00000000000006f0
>> [  305.853619] Call Trace:
>> [  305.854086]  __unregister_prot_hook+0x15f/0x1d0
>> [  305.854722]  packet_release+0x77a/0x7a0
>> [  305.855335]  ? mark_held_locks+0x25/0xb0
>> [  305.855883]  ? packet_lookup_frame+0x110/0x110
>> [  305.856576]  ? __lock_is_held+0x39/0xc0
>> [  305.857109]  ? note_gp_changes+0x300/0x300
>> [  305.857745]  ? __sanitizer_cov_trace_const_cmp8+0x18/0x20
>> [  305.858548]  ? locks_remove_file+0x31b/0x420
>> [  305.859138]  ? fcntl_setlk+0xad0/0xad0
>> [  305.859743]  ? trace_event_raw_event_sched_switch+0x320/0x320
>> [  305.860534]  ? fsnotify_first_mark+0x2c0/0x2c0
>> [  305.861234]  sock_release+0x53/0xe0
>> [  305.861711]  ? sock_alloc_file+0x2c0/0x2c0
>> [  305.862334]  sock_close+0x16/0x20
>> [  305.862801]  __fput+0x246/0x4e0
>> [  305.863310]  ? fput+0x130/0x130
>> [  305.863743]  ? trace_event_raw_event_sched_switch+0x320/0x320
>> [  305.864604]  ____fput+0x15/0x20
>> [  305.865046]  task_work_run+0x1a5/0x200
>> [  305.865636]  ? kmem_cache_free+0x25c/0x2d0
>> [  305.866194]  ? task_work_cancel+0x1a0/0x1a0
>> [  305.866829]  ? switch_task_namespaces+0x9e/0xb0
>> [  305.867458]  do_exit+0xacf/0x10d0
>> [  305.868023]  ? mm_update_next_owner+0x650/0x650
>> [  305.868642]  ? __pv_queued_spin_lock_slowpath+0xbf0/0xbf0
>> [  305.869427]  ? trace_hardirqs_on_caller+0x136/0x2a0
>> [  305.870102]  ? trace_hardirqs_on+0xd/0x10
>> [  305.870701]  ? wake_up_new_task+0x41c/0x650
>> [  305.871324]  ? to_ratio+0x20/0x20
>> [  305.871816]  ? lock_release+0x530/0x530
>> [  305.872341]  ? trace_event_raw_event_sched_switch+0x320/0x320
>> [  305.873161]  ? match_held_lock+0x7e/0x360
>> [  305.873777]  ? trace_hardirqs_off+0x10/0x10
>> [  305.874359]  ? put_pid+0x111/0x140
>> [  305.874897]  ? task_active_pid_ns+0x70/0x70
>> [  305.875500]  ? find_held_lock+0xca/0xf0
>> [  305.876118]  ? do_group_exit+0x1f9/0x260
>> [  305.876650]  ? lock_downgrade+0x380/0x380
>> [  305.877297]  ? task_clear_jobctl_pending+0xb5/0xd0
>> [  305.877951]  ? __sanitizer_cov_trace_const_cmp4+0x16/0x20
>> [  305.878725]  ? do_raw_spin_unlock+0x112/0x1a0
>> [  305.879309]  ? do_raw_spin_trylock+0x100/0x100
>> [  305.879969]  ? mark_held_locks+0x25/0xb0
>> [  305.880505]  ? force_sig+0x30/0x30
>> [  305.881054]  ? _raw_spin_unlock_irq+0x27/0x50
>> [  305.881671]  ? trace_hardirqs_on_caller+0x136/0x2a0
>> [  305.882412]  do_group_exit+0xfb/0x260
>> [  305.882945]  ? SyS_exit+0x30/0x30
>> [  305.883442]  ? find_mergeable_anon_vma+0x90/0x90
>> [  305.884103]  ? SyS_read+0x1c0/0x1c0
>> [  305.884785]  ? mark_held_locks+0x25/0xb0
>> [  305.885503]  ? do_syscall_64+0xb2/0x5d0
>> [  305.886093]  ? do_group_exit+0x260/0x260
>> [  305.886741]  SyS_exit_group+0x1d/0x20
>> [  305.887255]  do_syscall_64+0x209/0x5d0
>> [  305.887888]  ? syscall_return_slowpath+0x3e0/0x3e0
>> [  305.888611]  ? __sanitizer_cov_trace_const_cmp4+0x16/0x20
>> [  305.889420]  ? syscall_return_slowpath+0x260/0x3e0
>> [  305.890188]  ? mark_held_locks+0x25/0xb0
>> [  305.890724]  ? entry_SYSCALL_64_after_hwframe+0x52/0xb7
>> [  305.891556]  ? trace_hardirqs_off_caller+0xb5/0x120
>> [  305.892265]  ? trace_hardirqs_off_thunk+0x1a/0x1c
>> [  305.892939]  entry_SYSCALL_64_after_hwframe+0x42/0xb7
>> [  305.893676] RIP: 0033:0x44d989
>> [  305.894100] RSP: 002b:00000000007fff38 EFLAGS: 00000202 ORIG_RAX:
>> 00000000000000e7
>> [  305.895158] RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 000000000044d989
>> [  305.896174] RDX: 00007fb120d739c0 RSI: 00007fb120572700 RDI: 0000000000000001
>> [  305.897161] RBP: 00000000007fff60 R08: 00007fb120572700 R09: 0000000000000000
>> [  305.898128] R10: 0000000000000000 R11: 0000000000000202 R12: 0000000000000000
>> [  305.899464] R13: 000000000040d270 R14: 000000000040d300 R15: 0000000000000000
>> [  305.900823] Code: b6 1d 81 9a ef 03 31 ff 89 de e8 ca a3 67 ff 84
>> db 75 df e8 f1 a2 67 ff 48 c7 c7 60 8f 83 84 c6 05 61 9a ef 03 01 e8
>> ee 5f 49 ff <0f> 0b eb c3 66 2e 0f 1f 84 00 00 00 00 00 55 48 89 e5 41
>> 57 49
>> [  305.904324] ---[ end trace 360c084b02d93021 ]---
>> [  305.919117] ------------[ cut here ]------------
>> [  305.920120] refcount_t: underflow; use-after-free.
>> [  305.921335] WARNING: CPU: 0 PID: 3867 at
>> /home/blee/project/race-fuzzer/kernels/kernel_v4.16-rc3/lib/refcount.c:187
>> refcount_sub_and_test+0x1ec/0x200
>> [  305.923927] Modules linked in:
>> [  305.924611] CPU: 0 PID: 3867 Comm: repro.exe Tainted: G        W
>>     4.16.0-rc3 #1
>> [  305.925987] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
>> BIOS rel-1.8.2-0-g33fbe13 by qemu-project.org 04/01/2014
>> [  305.928119] RIP: 0010:refcount_sub_and_test+0x1ec/0x200
>> [  305.929124] RSP: 0018:ffff880224d374a0 EFLAGS: 00010282
>> [  305.930161] RAX: 0000000000000026 RBX: 0000000000000000 RCX: ffffffff813c9644
>> [  305.931504] RDX: 0000000000000000 RSI: ffffffff813cd761 RDI: ffff880224d37018
>> [  305.932942] RBP: ffff880224d37538 R08: ffff88023169a440 R09: 0000000000000000
>> [  305.934365] R10: 0000000000000000 R11: 0000000000000000 R12: 00000000ffffffff
>> [  305.935734] R13: ffff88023473adc0 R14: 0000000000000001 R15: 1ffff100449a6e96
>> [  305.937114] FS:  0000000000c6e940(0000) GS:ffff8800b8a00000(0000)
>> knlGS:0000000000000000
>> [  305.938668] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> [  305.939768] CR2: 00007fb120571db8 CR3: 0000000005422000 CR4: 00000000000006f0
>> [  305.941212] Call Trace:
>> [  305.941689]  ? refcount_inc+0x70/0x70
>> [  305.942216]  ? skb_dequeue+0xa5/0xc0
>> [  305.942713]  refcount_dec_and_test+0x1a/0x20
>> [  305.943295]  packet_release+0x702/0x7a0
>> [  305.943816]  ? mark_held_locks+0x25/0xb0
>> [  305.944378]  ? packet_lookup_frame+0x110/0x110
>> [  305.945021]  ? __lock_is_held+0x39/0xc0
>> [  305.945561]  ? note_gp_changes+0x300/0x300
>> [  305.946132]  ? __sanitizer_cov_trace_const_cmp8+0x18/0x20
>> [  305.946866]  ? locks_remove_file+0x31b/0x420
>> [  305.947464]  ? fcntl_setlk+0xad0/0xad0
>> [  305.948000]  ? trace_event_raw_event_sched_switch+0x320/0x320
>> [  305.948781]  ? fsnotify_first_mark+0x2c0/0x2c0
>> [  305.949386]  sock_release+0x53/0xe0
>> [  305.949866]  ? sock_alloc_file+0x2c0/0x2c0
>> [  305.950437]  sock_close+0x16/0x20
>> [  305.950906]  __fput+0x246/0x4e0
>> [  305.951360]  ? fput+0x130/0x130
>> [  305.951807]  ? trace_event_raw_event_sched_switch+0x320/0x320
>> [  305.952620]  ____fput+0x15/0x20
>> [  305.953071]  task_work_run+0x1a5/0x200
>> [  305.953585]  ? kmem_cache_free+0x25c/0x2d0
>> [  305.954143]  ? task_work_cancel+0x1a0/0x1a0
>> [  305.954714]  ? switch_task_namespaces+0x9e/0xb0
>> [  305.955334]  do_exit+0xacf/0x10d0
>> [  305.955801]  ? mm_update_next_owner+0x650/0x650
>> [  305.956431]  ? __pv_queued_spin_lock_slowpath+0xbf0/0xbf0
>> [  305.957157]  ? trace_hardirqs_on_caller+0x136/0x2a0
>> [  305.957811]  ? trace_hardirqs_on+0xd/0x10
>> [  305.958360]  ? wake_up_new_task+0x41c/0x650
>> [  305.958937]  ? to_ratio+0x20/0x20
>> [  305.959391]  ? lock_release+0x530/0x530
>> [  305.959924]  ? trace_event_raw_event_sched_switch+0x320/0x320
>> [  305.960693]  ? match_held_lock+0x7e/0x360
>> [  305.961244]  ? trace_hardirqs_off+0x10/0x10
>> [  305.961810]  ? put_pid+0x111/0x140
>> [  305.962277]  ? task_active_pid_ns+0x70/0x70
>> [  305.962862]  ? find_held_lock+0xca/0xf0
>> [  305.963396]  ? do_group_exit+0x1f9/0x260
>> [  305.963933]  ? lock_downgrade+0x380/0x380
>> [  305.964508]  ? task_clear_jobctl_pending+0xb5/0xd0
>> [  305.965147]  ? __sanitizer_cov_trace_const_cmp4+0x16/0x20
>> [  305.965871]  ? do_raw_spin_unlock+0x112/0x1a0
>> [  305.966459]  ? do_raw_spin_trylock+0x100/0x100
>> [  305.967060]  ? mark_held_locks+0x25/0xb0
>> [  305.967592]  ? force_sig+0x30/0x30
>> [  305.968135]  ? _raw_spin_unlock_irq+0x27/0x50
>> [  305.968741]  ? trace_hardirqs_on_caller+0x136/0x2a0
>> [  305.969470]  do_group_exit+0xfb/0x260
>> [  305.969987]  ? SyS_exit+0x30/0x30
>> [  305.970505]  ? find_mergeable_anon_vma+0x90/0x90
>> [  305.971126]  ? SyS_read+0x1c0/0x1c0
>> [  305.971718]  ? mark_held_locks+0x25/0xb0
>> [  305.972259]  ? do_syscall_64+0xb2/0x5d0
>> [  305.972843]  ? do_group_exit+0x260/0x260
>> [  305.973374]  SyS_exit_group+0x1d/0x20
>> [  305.973932]  do_syscall_64+0x209/0x5d0
>> [  305.974452]  ? syscall_return_slowpath+0x3e0/0x3e0
>> [  305.975149]  ? __sanitizer_cov_trace_const_cmp4+0x16/0x20
>> [  305.975941]  ? syscall_return_slowpath+0x260/0x3e0
>> [  305.976669]  ? mark_held_locks+0x25/0xb0
>> [  305.977206]  ? entry_SYSCALL_64_after_hwframe+0x52/0xb7
>> [  305.977978]  ? trace_hardirqs_off_caller+0xb5/0x120
>> [  305.978690]  ? trace_hardirqs_off_thunk+0x1a/0x1c
>> [  305.979381]  entry_SYSCALL_64_after_hwframe+0x42/0xb7
>> [  305.980114] RIP: 0033:0x44d989
>> [  305.980531] RSP: 002b:00000000007fff38 EFLAGS: 00000202 ORIG_RAX:
>> 00000000000000e7
>> [  305.981664] RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 000000000044d989
>> [  305.982655] RDX: 00007fb120d739c0 RSI: 00007fb120572700 RDI: 0000000000000001
>> [  305.983654] RBP: 00000000007fff60 R08: 00007fb120572700 R09: 0000000000000000
>> [  305.984656] R10: 0000000000000000 R11: 0000000000000202 R12: 0000000000000000
>> [  305.985707] R13: 000000000040d270 R14: 000000000040d300 R15: 0000000000000000
>> [  305.986724] Code: b6 1d 18 9b ef 03 31 ff 89 de e8 60 a4 67 ff 84
>> db 75 1a e8 87 a3 67 ff 48 c7 c7 00 8f 83 84 c6 05 f8 9a ef 03 01 e8
>> 84 60 49 ff <0f> 0b 31 db e9 2b ff ff ff 66 66 2e 0f 1f 84 00 00 00 00
>> 00 55
>> [  305.990106] ---[ end trace 360c084b02d93022 ]---
>> [  305.998636] IPVS: ftp: loaded support on port[0] = 21
>> ---------------------------------------
>>
>> = About RaceFuzzer
>>
>> RaceFuzzer is a customized version of Syzkaller, specifically tailored
>> to find race condition bugs in the Linux kernel. While we leverage
>> many different technique, the notable feature of RaceFuzzer is in
>> leveraging a custom hypervisor (QEMU/KVM) to interleave the
>> scheduling. In particular, we modified the hypervisor to intentionally
>> stall a per-core execution, which is similar to supporting per-core
>> breakpoint functionality. This allows RaceFuzzer to force the kernel
>> to deterministically trigger racy condition (which may rarely happen
>> in practice due to randomness in scheduling).
>>
>> RaceFuzzer's C repro always pinpoints two racy syscalls. Since C
>> repro's scheduling synchronization should be performed at the user
>> space, its reproducibility is limited (reproduction may take from 1
>> second to 10 minutes (or even more), depending on a bug). This is
>> because, while RaceFuzzer precisely interleaves the scheduling at the
>> kernel's instruction level when finding this bug, C repro cannot fully
>> utilize such a feature. Please disregard all code related to
>> "should_hypercall" in the C repro, as this is only for our debugging
>> purposes using our own hypervisor.

^ permalink raw reply

* Re: [PATCH] gfs2: Stop using rhashtable_walk_peek
From: Andreas Gruenbacher @ 2018-03-28 22:58 UTC (permalink / raw)
  To: NeilBrown
  Cc: cluster-devel, netdev, LKML, Thomas Graf, Herbert Xu, Tom Herbert
In-Reply-To: <87woxv3m0g.fsf@notabene.neil.brown.name>

On 28 March 2018 at 23:53, NeilBrown <neilb@suse.com> wrote:
> Thank for this patch!
> The above looks a bit fragile to me.
> gfs2_glock_iter_next() (And hence gfs2_glock_seq_start()) will sometimes
> exit with gl_held true, and sometimes with it false.
> gfs2_glock_seq_stop() assumes that it is false.
> Normally gfs2_glock_seq_next() will normally be called between these
> two and will clear gl_held, but I don't think there is a hard guarantee
> of that.
> Maybe we should always 'put' gi->gl in iter_next if gl_held??

Another problem is that gfs2_glock_seq_stop only does a lockref_get
which is not allowed because the lockref may have been marked dead in
the meantime. We'll have to take a reference on each lock in
gfs2_glock_iter_next instead. Revised patch to follow tomorrow.

Thanks,
Andreas

^ permalink raw reply

* [PATCH net-next v2 2/2] sfp/phylink: move module EEPROM ethtool access into netdev core ethtool
From: Florian Fainelli @ 2018-03-28 22:44 UTC (permalink / raw)
  To: netdev
  Cc: Russell King, Florian Fainelli, Thomas Petazzoni, Andrew Lunn,
	David S. Miller, open list, Antoine Tenart, Yan Markman,
	Stefan Chulski, Maxime Chevallier, Miquel Raynal, Marcin Wojtas
In-Reply-To: <20180328224416.6981-1-f.fainelli@gmail.com>

From: Russell King <rmk+kernel@armlinux.org.uk>

Provide a pointer to the SFP bus in struct net_device, so that the
ethtool module EEPROM methods can access the SFP directly, rather
than needing every user to provide a hook for it.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/marvell/mvneta.c | 18 ------------------
 drivers/net/phy/phylink.c             | 28 ----------------------------
 drivers/net/phy/sfp-bus.c             |  6 ++----
 include/linux/netdevice.h             |  3 +++
 include/linux/phylink.h               |  3 ---
 net/core/ethtool.c                    |  7 +++++++
 6 files changed, 12 insertions(+), 53 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index cd09bde55596..25ced96750bf 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -4075,22 +4075,6 @@ static int mvneta_ethtool_set_wol(struct net_device *dev,
 	return ret;
 }
 
-static int mvneta_ethtool_get_module_info(struct net_device *dev,
-					  struct ethtool_modinfo *modinfo)
-{
-	struct mvneta_port *pp = netdev_priv(dev);
-
-	return phylink_ethtool_get_module_info(pp->phylink, modinfo);
-}
-
-static int mvneta_ethtool_get_module_eeprom(struct net_device *dev,
-					    struct ethtool_eeprom *ee, u8 *buf)
-{
-	struct mvneta_port *pp = netdev_priv(dev);
-
-	return phylink_ethtool_get_module_eeprom(pp->phylink, ee, buf);
-}
-
 static int mvneta_ethtool_get_eee(struct net_device *dev,
 				  struct ethtool_eee *eee)
 {
@@ -4165,8 +4149,6 @@ static const struct ethtool_ops mvneta_eth_tool_ops = {
 	.set_link_ksettings = mvneta_ethtool_set_link_ksettings,
 	.get_wol        = mvneta_ethtool_get_wol,
 	.set_wol        = mvneta_ethtool_set_wol,
-	.get_module_info = mvneta_ethtool_get_module_info,
-	.get_module_eeprom = mvneta_ethtool_get_module_eeprom,
 	.get_eee	= mvneta_ethtool_get_eee,
 	.set_eee	= mvneta_ethtool_set_eee,
 };
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 9b1e4721ea3a..c582b2d7546c 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -1250,34 +1250,6 @@ int phylink_ethtool_set_pauseparam(struct phylink *pl,
 }
 EXPORT_SYMBOL_GPL(phylink_ethtool_set_pauseparam);
 
-int phylink_ethtool_get_module_info(struct phylink *pl,
-				    struct ethtool_modinfo *modinfo)
-{
-	int ret = -EOPNOTSUPP;
-
-	WARN_ON(!lockdep_rtnl_is_held());
-
-	if (pl->sfp_bus)
-		ret = sfp_get_module_info(pl->sfp_bus, modinfo);
-
-	return ret;
-}
-EXPORT_SYMBOL_GPL(phylink_ethtool_get_module_info);
-
-int phylink_ethtool_get_module_eeprom(struct phylink *pl,
-				      struct ethtool_eeprom *ee, u8 *buf)
-{
-	int ret = -EOPNOTSUPP;
-
-	WARN_ON(!lockdep_rtnl_is_held());
-
-	if (pl->sfp_bus)
-		ret = sfp_get_module_eeprom(pl->sfp_bus, ee, buf);
-
-	return ret;
-}
-EXPORT_SYMBOL_GPL(phylink_ethtool_get_module_eeprom);
-
 /**
  * phylink_ethtool_get_eee_err() - read the energy efficient ethernet error
  *   counter
diff --git a/drivers/net/phy/sfp-bus.c b/drivers/net/phy/sfp-bus.c
index 3d4ff5d0d2a6..0381da78d228 100644
--- a/drivers/net/phy/sfp-bus.c
+++ b/drivers/net/phy/sfp-bus.c
@@ -342,6 +342,7 @@ static int sfp_register_bus(struct sfp_bus *bus)
 	}
 	if (bus->started)
 		bus->socket_ops->start(bus->sfp);
+	bus->netdev->sfp_bus = bus;
 	bus->registered = true;
 	return 0;
 }
@@ -356,6 +357,7 @@ static void sfp_unregister_bus(struct sfp_bus *bus)
 		if (bus->phydev && ops && ops->disconnect_phy)
 			ops->disconnect_phy(bus->upstream);
 	}
+	bus->netdev->sfp_bus = NULL;
 	bus->registered = false;
 }
 
@@ -371,8 +373,6 @@ static void sfp_unregister_bus(struct sfp_bus *bus)
  */
 int sfp_get_module_info(struct sfp_bus *bus, struct ethtool_modinfo *modinfo)
 {
-	if (!bus->registered)
-		return -ENOIOCTLCMD;
 	return bus->socket_ops->module_info(bus->sfp, modinfo);
 }
 EXPORT_SYMBOL_GPL(sfp_get_module_info);
@@ -391,8 +391,6 @@ EXPORT_SYMBOL_GPL(sfp_get_module_info);
 int sfp_get_module_eeprom(struct sfp_bus *bus, struct ethtool_eeprom *ee,
 			  u8 *data)
 {
-	if (!bus->registered)
-		return -ENOIOCTLCMD;
 	return bus->socket_ops->module_eeprom(bus->sfp, ee, data);
 }
 EXPORT_SYMBOL_GPL(sfp_get_module_eeprom);
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 2a2d9cf50aa2..53f0cd64676b 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -58,6 +58,7 @@ struct device;
 struct phy_device;
 struct dsa_port;
 
+struct sfp_bus;
 /* 802.11 specific */
 struct wireless_dev;
 /* 802.15.4 specific */
@@ -1662,6 +1663,7 @@ enum netdev_priv_flags {
  *	@priomap:	XXX: need comments on this one
  *	@phydev:	Physical device may attach itself
  *			for hardware timestamping
+ *	@sfp_bus:	attached &struct sfp_bus structure.
  *
  *	@qdisc_tx_busylock: lockdep class annotating Qdisc->busylock spinlock
  *	@qdisc_running_key: lockdep class annotating Qdisc->running seqcount
@@ -1945,6 +1947,7 @@ struct net_device {
 	struct netprio_map __rcu *priomap;
 #endif
 	struct phy_device	*phydev;
+	struct sfp_bus		*sfp_bus;
 	struct lock_class_key	*qdisc_tx_busylock;
 	struct lock_class_key	*qdisc_running_key;
 	bool			proto_down;
diff --git a/include/linux/phylink.h b/include/linux/phylink.h
index e95cc12030fa..50eeae025f1e 100644
--- a/include/linux/phylink.h
+++ b/include/linux/phylink.h
@@ -219,9 +219,6 @@ void phylink_ethtool_get_pauseparam(struct phylink *,
 				    struct ethtool_pauseparam *);
 int phylink_ethtool_set_pauseparam(struct phylink *,
 				   struct ethtool_pauseparam *);
-int phylink_ethtool_get_module_info(struct phylink *, struct ethtool_modinfo *);
-int phylink_ethtool_get_module_eeprom(struct phylink *,
-				      struct ethtool_eeprom *, u8 *);
 int phylink_get_eee_err(struct phylink *);
 int phylink_ethtool_get_eee(struct phylink *, struct ethtool_eee *);
 int phylink_ethtool_set_eee(struct phylink *, struct ethtool_eee *);
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index bb6e498c6e3d..eb55252ca1fb 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -22,6 +22,7 @@
 #include <linux/bitops.h>
 #include <linux/uaccess.h>
 #include <linux/vmalloc.h>
+#include <linux/sfp.h>
 #include <linux/slab.h>
 #include <linux/rtnetlink.h>
 #include <linux/sched/signal.h>
@@ -2245,6 +2246,9 @@ static int __ethtool_get_module_info(struct net_device *dev,
 	const struct ethtool_ops *ops = dev->ethtool_ops;
 	struct phy_device *phydev = dev->phydev;
 
+	if (dev->sfp_bus)
+		return sfp_get_module_info(dev->sfp_bus, modinfo);
+
 	if (phydev && phydev->drv && phydev->drv->module_info)
 		return phydev->drv->module_info(phydev, modinfo);
 
@@ -2279,6 +2283,9 @@ static int __ethtool_get_module_eeprom(struct net_device *dev,
 	const struct ethtool_ops *ops = dev->ethtool_ops;
 	struct phy_device *phydev = dev->phydev;
 
+	if (dev->sfp_bus)
+		return sfp_get_module_eeprom(dev->sfp_bus, ee, data);
+
 	if (phydev && phydev->drv && phydev->drv->module_eeprom)
 		return phydev->drv->module_eeprom(phydev, ee, data);
 
-- 
2.14.1

^ permalink raw reply related

* [PATCH net-next v2 1/2] net: phy: phylink: Provide PHY interface to mac_link_{up,down}
From: Florian Fainelli @ 2018-03-28 22:44 UTC (permalink / raw)
  To: netdev
  Cc: Florian Fainelli, Thomas Petazzoni, Andrew Lunn, David S. Miller,
	Russell King, open list, Antoine Tenart, Yan Markman,
	Stefan Chulski, Maxime Chevallier, Miquel Raynal, Marcin Wojtas
In-Reply-To: <20180328224416.6981-1-f.fainelli@gmail.com>

In preparation for having DSA transition entirely to PHYLINK, we need to pass a
PHY interface type to the mac_link_{up,down} callbacks because we may have to
make decisions on that (e.g: turn on/off RGMII interfaces etc.). We do not pass
an entire phylink_link_state because not all parameters (pause, duplex etc.) are
defined when the link is down, only link and interface are.

Update mvneta accordingly since it currently implements phylink_mac_ops.

Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/marvell/mvneta.c |  4 +++-
 drivers/net/phy/phylink.c             |  4 +++-
 include/linux/phylink.h               | 14 +++++++++++---
 3 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index eaa4bb80f1c9..cd09bde55596 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -3396,7 +3396,8 @@ static void mvneta_set_eee(struct mvneta_port *pp, bool enable)
 	mvreg_write(pp, MVNETA_LPI_CTRL_1, lpi_ctl1);
 }
 
-static void mvneta_mac_link_down(struct net_device *ndev, unsigned int mode)
+static void mvneta_mac_link_down(struct net_device *ndev, unsigned int mode,
+				 phy_interface_t interface)
 {
 	struct mvneta_port *pp = netdev_priv(ndev);
 	u32 val;
@@ -3415,6 +3416,7 @@ static void mvneta_mac_link_down(struct net_device *ndev, unsigned int mode)
 }
 
 static void mvneta_mac_link_up(struct net_device *ndev, unsigned int mode,
+			       phy_interface_t interface,
 			       struct phy_device *phy)
 {
 	struct mvneta_port *pp = netdev_priv(ndev);
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 51a011a349fe..9b1e4721ea3a 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -470,10 +470,12 @@ static void phylink_resolve(struct work_struct *w)
 	if (link_state.link != netif_carrier_ok(ndev)) {
 		if (!link_state.link) {
 			netif_carrier_off(ndev);
-			pl->ops->mac_link_down(ndev, pl->link_an_mode);
+			pl->ops->mac_link_down(ndev, pl->link_an_mode,
+					       pl->phy_state.interface);
 			netdev_info(ndev, "Link is Down\n");
 		} else {
 			pl->ops->mac_link_up(ndev, pl->link_an_mode,
+					     pl->phy_state.interface,
 					     pl->phydev);
 
 			netif_carrier_on(ndev);
diff --git a/include/linux/phylink.h b/include/linux/phylink.h
index bd137c273d38..e95cc12030fa 100644
--- a/include/linux/phylink.h
+++ b/include/linux/phylink.h
@@ -73,8 +73,10 @@ struct phylink_mac_ops {
 	void (*mac_config)(struct net_device *ndev, unsigned int mode,
 			   const struct phylink_link_state *state);
 	void (*mac_an_restart)(struct net_device *ndev);
-	void (*mac_link_down)(struct net_device *ndev, unsigned int mode);
+	void (*mac_link_down)(struct net_device *ndev, unsigned int mode,
+			      phy_interface_t interface);
 	void (*mac_link_up)(struct net_device *ndev, unsigned int mode,
+			    phy_interface_t interface,
 			    struct phy_device *phy);
 };
 
@@ -161,25 +163,31 @@ void mac_an_restart(struct net_device *ndev);
  * mac_link_down() - take the link down
  * @ndev: a pointer to a &struct net_device for the MAC.
  * @mode: link autonegotiation mode
+ * @interface: link &typedef phy_interface_t mode
  *
  * If @mode is not an in-band negotiation mode (as defined by
  * phylink_autoneg_inband()), force the link down and disable any
- * Energy Efficient Ethernet MAC configuration.
+ * Energy Efficient Ethernet MAC configuration. Interface type
+ * selection must be done in mac_config().
  */
-void mac_link_down(struct net_device *ndev, unsigned int mode);
+void mac_link_down(struct net_device *ndev, unsigned int mode,
+		   phy_interface_t interface);
 
 /**
  * mac_link_up() - allow the link to come up
  * @ndev: a pointer to a &struct net_device for the MAC.
  * @mode: link autonegotiation mode
+ * @interface: link &typedef phy_interface_t mode
  * @phy: any attached phy
  *
  * If @mode is not an in-band negotiation mode (as defined by
  * phylink_autoneg_inband()), allow the link to come up. If @phy
  * is non-%NULL, configure Energy Efficient Ethernet by calling
  * phy_init_eee() and perform appropriate MAC configuration for EEE.
+ * Interface type selection must be done in mac_config().
  */
 void mac_link_up(struct net_device *ndev, unsigned int mode,
+		 phy_interface_t interface,
 		 struct phy_device *phy);
 #endif
 
-- 
2.14.1

^ permalink raw reply related

* [PATCH net-next v2 0/2] phylink: API changes
From: Florian Fainelli @ 2018-03-28 22:44 UTC (permalink / raw)
  To: netdev
  Cc: Florian Fainelli, Thomas Petazzoni, Andrew Lunn, David S. Miller,
	Russell King, open list, Antoine Tenart, Yan Markman,
	Stefan Chulski, Maxime Chevallier, Miquel Raynal, Marcin Wojtas

Hi all,

This patch series contains two API changes to PHYLINK which will later be used
by DSA to migrate to PHYLINK. Because these are API changes that impact other
outstanding work (e.g: MVPP2) I would rather get them included sooner to minimize
conflicts.

Thank you!

Changes in v2:

- added missing documentation to mac_link_{up,down} that the interface
  must be configured in mac_config()

- added Russell's, Andrew's and my tags

Florian Fainelli (1):
  net: phy: phylink: Provide PHY interface to mac_link_{up,down}

Russell King (1):
  sfp/phylink: move module EEPROM ethtool access into netdev core
    ethtool

 drivers/net/ethernet/marvell/mvneta.c | 22 +++-------------------
 drivers/net/phy/phylink.c             | 32 +++-----------------------------
 drivers/net/phy/sfp-bus.c             |  6 ++----
 include/linux/netdevice.h             |  3 +++
 include/linux/phylink.h               | 17 +++++++++++------
 net/core/ethtool.c                    |  7 +++++++
 6 files changed, 29 insertions(+), 58 deletions(-)

-- 
2.14.1

^ permalink raw reply

* RE: [Resend Patch 1/3] Vmbus: Add function to report available ring buffer to write in total ring size percentage
From: Long Li @ 2018-03-28 22:42 UTC (permalink / raw)
  To: Martin K. Petersen, Long Li
  Cc: Stephen Hemminger, linux-scsi@vger.kernel.org,
	netdev@vger.kernel.org, Haiyang Zhang, James E . J . Bottomley,
	linux-kernel@vger.kernel.org, devel@linuxdriverproject.org
In-Reply-To: <yq1y3ibhndg.fsf@oracle.com>

> Subject: Re: [Resend Patch 1/3] Vmbus: Add function to report available ring
> buffer to write in total ring size percentage
> 
> 
> Long,
> 
> > Netvsc has a function to calculate how much ring buffer in percentage
> > is available to write. This function is also useful for storvsc and
> > other vmbus devices.
> 
> What is the submission strategy for this series? Do you expect it to go
> through net or scsi? If the latter, I'll need an ack from davem.

Martin,

I hope this patch set goes through SCSI, because it's purpose is to improve storvsc.

If this strategy is not possible, I can resubmit the 1st two patches to net, and the 3rd patch to scsi after the 1st two are merged.

Long

> 
> --
> Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply

* [PATCH net-next] MAINTAINERS: update vmxnet3 driver maintainer
From: Ronak Doshi @ 2018-03-28 22:38 UTC (permalink / raw)
  To: netdev
  Cc: Ronak Doshi, Shrikrishna Khare, David S. Miller,
	Mauro Carvalho Chehab, Greg Kroah-Hartman, Linus Walleij,
	Andrew Morton, Randy Dunlap, open list

Shrikrishna Khare would no longer maintain the vmxnet3 driver. Taking
over the role of vmxnet3 maintainer.

Signed-off-by: Ronak Doshi <doshir@vmware.com>
Signed-off-by: Shrikrishna Khare <skhare@vmware.com>
---
 MAINTAINERS | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 9107d9241564..aace4ae4ad30 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14940,7 +14940,7 @@ F:	drivers/input/mouse/vmmouse.c
 F:	drivers/input/mouse/vmmouse.h
 
 VMWARE VMXNET3 ETHERNET DRIVER
-M:	Shrikrishna Khare <skhare@vmware.com>
+M:	Ronak Doshi <doshir@vmware.com>
 M:	"VMware, Inc." <pv-drivers@vmware.com>
 L:	netdev@vger.kernel.org
 S:	Maintained
-- 
2.11.0

^ permalink raw reply related

* Re: [PATCH v3 net 4/5] tcp: prevent bogus undos when SACK is not enabled
From: Yuchung Cheng @ 2018-03-28 22:36 UTC (permalink / raw)
  To: Ilpo Järvinen; +Cc: netdev, Neal Cardwell, Eric Dumazet, Sergei Shtylyov
In-Reply-To: <1520936711-16784-5-git-send-email-ilpo.jarvinen@helsinki.fi>

On Tue, Mar 13, 2018 at 3:25 AM, Ilpo Järvinen
<ilpo.jarvinen@helsinki.fi> wrote:
> When a cumulative ACK lands to high_seq at the end of loss
> recovery and SACK is not enabled, the sender needs to avoid
> false fast retransmits (RFC6582). The avoidance mechanisms is
> implemented by remaining in the loss recovery CA state until
> one additional cumulative ACK arrives. During the operation of
> this avoidance mechanism, there is internal transient in the
> use of state variables which will always trigger a bogus undo.
Do we have to make undo in non-sack perfect? can we consider a much
simpler but imperfect fix of

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 8d480542aa07..95225d9de0af 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -2356,6 +2356,7 @@ static bool tcp_try_undo_recovery(struct sock *sk)
                 * fast retransmits (RFC2582). SACK TCP is safe. */
                if (!tcp_any_retrans_done(sk))
                        tp->retrans_stamp = 0;
+               tp->undo_marker = 0;
                return true;
        }



>
> When we enter to this transient state in tcp_try_undo_recovery,
> tcp_any_retrans_done is often (always?) false resulting in
> clearing retrans_stamp. On the next cumulative ACK,
> tcp_try_undo_recovery again executes because CA state still
> remains in the same recovery state and tcp_may_undo will always
> return true because tcp_packet_delayed has this condition:
>     return !tp->retrans_stamp || ...
>
> Check if the false fast retransmit transient avoidance is in
> progress in tcp_packet_delayed to avoid bogus undos. Since snd_una
> has advanced already on this ACK but CA state still remains
> unchanged (CA state is updated slightly later than undo is
> checked), prior_snd_una needs to be passed to tcp_packet_delayed
> (instead of tp->snd_una). Passing prior_snd_una around to
> the tcp_packet_delayed makes this change look more involved than
> it really is.
>
> The additional checks done in this change only affect non-SACK
> case, the SACK case remains the same.
>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
> ---
>  net/ipv4/tcp_input.c | 42 ++++++++++++++++++++++++++----------------
>  1 file changed, 26 insertions(+), 16 deletions(-)
>
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index 72ecfbb..270aa48 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -2241,10 +2241,17 @@ static bool tcp_skb_spurious_retrans(const struct tcp_sock *tp,
>  /* Nothing was retransmitted or returned timestamp is less
>   * than timestamp of the first retransmission.
>   */
> -static inline bool tcp_packet_delayed(const struct tcp_sock *tp)
> +static inline bool tcp_packet_delayed(const struct tcp_sock *tp,
> +                                     const u32 prior_snd_una)
>  {
> -       return !tp->retrans_stamp ||
> -              tcp_tsopt_ecr_before(tp, tp->retrans_stamp);
> +       if (!tp->retrans_stamp) {
> +               /* Sender will be in a transient state with cleared
> +                * retrans_stamp during false fast retransmit prevention
> +                * mechanism
> +                */
> +               return !tcp_false_fast_retrans_possible(tp, prior_snd_una);
> +       }
> +       return tcp_tsopt_ecr_before(tp, tp->retrans_stamp);
>  }
>
>  /* Undo procedures. */
> @@ -2334,17 +2341,19 @@ static void tcp_undo_cwnd_reduction(struct sock *sk, bool unmark_loss)
>         tp->rack.advanced = 1; /* Force RACK to re-exam losses */
>  }
>
> -static inline bool tcp_may_undo(const struct tcp_sock *tp)
> +static inline bool tcp_may_undo(const struct tcp_sock *tp,
> +                               const u32 prior_snd_una)
>  {
> -       return tp->undo_marker && (!tp->undo_retrans || tcp_packet_delayed(tp));
> +       return tp->undo_marker &&
> +              (!tp->undo_retrans || tcp_packet_delayed(tp, prior_snd_una));
>  }
>
>  /* People celebrate: "We love our President!" */
> -static bool tcp_try_undo_recovery(struct sock *sk)
> +static bool tcp_try_undo_recovery(struct sock *sk, const u32 prior_snd_una)
>  {
>         struct tcp_sock *tp = tcp_sk(sk);
>
> -       if (tcp_may_undo(tp)) {
> +       if (tcp_may_undo(tp, prior_snd_una)) {
>                 int mib_idx;
>
>                 /* Happy end! We did not retransmit anything
> @@ -2391,11 +2400,12 @@ static bool tcp_try_undo_dsack(struct sock *sk)
>  }
>
>  /* Undo during loss recovery after partial ACK or using F-RTO. */
> -static bool tcp_try_undo_loss(struct sock *sk, bool frto_undo)
> +static bool tcp_try_undo_loss(struct sock *sk, const u32 prior_snd_una,
> +                             bool frto_undo)
>  {
>         struct tcp_sock *tp = tcp_sk(sk);
>
> -       if (frto_undo || tcp_may_undo(tp)) {
> +       if (frto_undo || tcp_may_undo(tp, prior_snd_una)) {
>                 tcp_undo_cwnd_reduction(sk, true);
>
>                 DBGUNDO(sk, "partial loss");
> @@ -2628,13 +2638,13 @@ void tcp_enter_recovery(struct sock *sk, bool ece_ack)
>   * recovered or spurious. Otherwise retransmits more on partial ACKs.
>   */
>  static void tcp_process_loss(struct sock *sk, int flag, bool is_dupack,
> -                            int *rexmit)
> +                            int *rexmit, const u32 prior_snd_una)
>  {
>         struct tcp_sock *tp = tcp_sk(sk);
>         bool recovered = !before(tp->snd_una, tp->high_seq);
>
>         if ((flag & FLAG_SND_UNA_ADVANCED) &&
> -           tcp_try_undo_loss(sk, false))
> +           tcp_try_undo_loss(sk, prior_snd_una, false))
>                 return;
>
>         if (tp->frto) { /* F-RTO RFC5682 sec 3.1 (sack enhanced version). */
> @@ -2642,7 +2652,7 @@ static void tcp_process_loss(struct sock *sk, int flag, bool is_dupack,
>                  * lost, i.e., never-retransmitted data are (s)acked.
>                  */
>                 if ((flag & FLAG_ORIG_SACK_ACKED) &&
> -                   tcp_try_undo_loss(sk, true))
> +                   tcp_try_undo_loss(sk, prior_snd_una, true))
>                         return;
>
>                 if (after(tp->snd_nxt, tp->high_seq)) {
> @@ -2665,7 +2675,7 @@ static void tcp_process_loss(struct sock *sk, int flag, bool is_dupack,
>
>         if (recovered) {
>                 /* F-RTO RFC5682 sec 3.1 step 2.a and 1st part of step 3.a */
> -               tcp_try_undo_recovery(sk);
> +               tcp_try_undo_recovery(sk, prior_snd_una);
>                 return;
>         }
>         if (tcp_is_reno(tp)) {
> @@ -2685,7 +2695,7 @@ static bool tcp_try_undo_partial(struct sock *sk, u32 prior_snd_una)
>  {
>         struct tcp_sock *tp = tcp_sk(sk);
>
> -       if (tp->undo_marker && tcp_packet_delayed(tp)) {
> +       if (tp->undo_marker && tcp_packet_delayed(tp, prior_snd_una)) {
>                 /* Plain luck! Hole if filled with delayed
>                  * packet, rather than with a retransmit. Check reordering.
>                  */
> @@ -2788,7 +2798,7 @@ static void tcp_fastretrans_alert(struct sock *sk, const u32 prior_snd_una,
>                 case TCP_CA_Recovery:
>                         if (tcp_is_reno(tp))
>                                 tcp_reset_reno_sack(tp);
> -                       if (tcp_try_undo_recovery(sk))
> +                       if (tcp_try_undo_recovery(sk, prior_snd_una))
>                                 return;
>                         tcp_end_cwnd_reduction(sk);
>                         break;
> @@ -2815,7 +2825,7 @@ static void tcp_fastretrans_alert(struct sock *sk, const u32 prior_snd_una,
>                 tcp_rack_identify_loss(sk, ack_flag);
>                 break;
>         case TCP_CA_Loss:
> -               tcp_process_loss(sk, flag, is_dupack, rexmit);
> +               tcp_process_loss(sk, flag, is_dupack, rexmit, prior_snd_una);
>                 tcp_rack_identify_loss(sk, ack_flag);
>                 if (!(icsk->icsk_ca_state == TCP_CA_Open ||
>                       (*ack_flag & FLAG_LOST_RETRANS)))
> --
> 2.7.4
>

^ permalink raw reply related

* Stable backport needed for "net: don't call update_pmtu unconditionally"
From: Benjamin Gilbert @ 2018-03-28 22:37 UTC (permalink / raw)
  To: Linux Kernel Network Developers

Hi,

f15ca723c1 ("net: don't call update_pmtu unconditionally") fixes NULL derefs
caused by a93bf0ff4490 ("vxlan: update skb dst pmtu on tx path"), which was
backported to 4.14.24.  We've received reports of panics with Open vSwitch
and Weave through 4.14.30 (https://github.com/coreos/bugs/issues/2382) and
at least the latter is resolved by backporting the patch.

Thanks,
--Benjamin Gilbert

^ permalink raw reply

* Re: [PATCH v3 net 2/5] tcp: prevent bogus FRTO undos with non-SACK flows
From: Yuchung Cheng @ 2018-03-28 22:26 UTC (permalink / raw)
  To: Ilpo Järvinen; +Cc: netdev, Neal Cardwell, Eric Dumazet, Sergei Shtylyov
In-Reply-To: <1520936711-16784-3-git-send-email-ilpo.jarvinen@helsinki.fi>

On Tue, Mar 13, 2018 at 3:25 AM, Ilpo Järvinen
<ilpo.jarvinen@helsinki.fi> wrote:
>
> If SACK is not enabled and the first cumulative ACK after the RTO
> retransmission covers more than the retransmitted skb, a spurious
> FRTO undo will trigger (assuming FRTO is enabled for that RTO).
> The reason is that any non-retransmitted segment acknowledged will
> set FLAG_ORIG_SACK_ACKED in tcp_clean_rtx_queue even if there is
> no indication that it would have been delivered for real (the
> scoreboard is not kept with TCPCB_SACKED_ACKED bits in the non-SACK
> case so the check for that bit won't help like it does with SACK).
> Having FLAG_ORIG_SACK_ACKED set results in the spurious FRTO undo
> in tcp_process_loss.
>
> We need to use more strict condition for non-SACK case and check
> that none of the cumulatively ACKed segments were retransmitted
> to prove that progress is due to original transmissions. Only then
> keep FLAG_ORIG_SACK_ACKED set, allowing FRTO undo to proceed in
> non-SACK case.
>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
> ---
>  net/ipv4/tcp_input.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index 4a26c09..c60745c 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -3166,6 +3166,15 @@ static int tcp_clean_rtx_queue(struct sock *sk, u32 prior_fack,
>                                 pkts_acked = rexmit_acked + newdata_acked;
>
>                         tcp_remove_reno_sacks(sk, pkts_acked);
> +
> +                       /* If any of the cumulatively ACKed segments was
> +                        * retransmitted, non-SACK case cannot confirm that
> +                        * progress was due to original transmission due to
> +                        * lack of TCPCB_SACKED_ACKED bits even if some of
> +                        * the packets may have been never retransmitted.
> +                        */
> +                       if (flag & FLAG_RETRANS_DATA_ACKED)
> +                               flag &= ~FLAG_ORIG_SACK_ACKED;

How about keeping your excellent comment but move the fix to F-RTO
code directly so it's more clear? this way the flag remains clear that
indicates some never-retransmitted data are acked/sacked.

// pseudo code for illustration

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 8d480542aa07..f7f3357de618 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -2629,8 +2629,15 @@ static void tcp_process_loss(struct sock *sk,
int flag, bool is_dupack,
        if (tp->frto) { /* F-RTO RFC5682 sec 3.1 (sack enhanced version). */
                /* Step 3.b. A timeout is spurious if not all data are
                 * lost, i.e., never-retransmitted data are (s)acked.
+                *
+                * If any of the cumulatively ACKed segments was
+                * retransmitted, non-SACK case cannot confirm that
+                * progress was due to original transmission due to
+                * lack of TCPCB_SACKED_ACKED bits even if some of
+                * the packets may have been never retransmitted.
                 */
                if ((flag & FLAG_ORIG_SACK_ACKED) &&
+                   (tcp_is_sack(tp) || !FLAG_RETRANS_DATA_ACKED) &&
                    tcp_try_undo_loss(sk, true))
                        return;





>                 } else {
>                         int delta;
>
> --
> 2.7.4
>

^ permalink raw reply related

* [PATCH net-next v2 3/3] net: bcmgenet: Fix coalescing settings handling
From: Florian Fainelli @ 2018-03-28 22:15 UTC (permalink / raw)
  To: netdev
  Cc: Florian Fainelli, davem, jaedon.shin, pgynther, opendmb,
	Michael Chan, gospo, talgi, saeedm
In-Reply-To: <20180328221538.29290-1-f.fainelli@gmail.com>

There were a number of issues with setting the RX coalescing parameters:

- we would not be preserving values that would have been configured
  across close/open calls, instead we would always reset to no timeout
  and 1 interrupt per packet, this would also prevent DIM from setting its
  default usec/pkts values

- when adaptive RX would be turned on, we woud not be fetching the
  default parameters, we would stay with no timeout/1 packet per interrupt
  until the estimator kicks in and changes that

- finally disabling adaptive RX coalescing while providing parameters
  would not be honored, and we would stay with whatever DIM had previously
  determined instead of the user requested parameters

Fixes: 9f4ca05827a2 ("net: bcmgenet: Add support for adaptive RX coalescing")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 92 +++++++++++++++++---------
 drivers/net/ethernet/broadcom/genet/bcmgenet.h |  4 +-
 2 files changed, 61 insertions(+), 35 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 7db8edc643ec..f8af472f324f 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -625,26 +625,46 @@ static int bcmgenet_get_coalesce(struct net_device *dev,
 	return 0;
 }
 
-static void bcmgenet_set_rx_coalesce(struct bcmgenet_rx_ring *ring)
+static void bcmgenet_set_rx_coalesce(struct bcmgenet_rx_ring *ring,
+				     u32 usecs, u32 pkts)
 {
 	struct bcmgenet_priv *priv = ring->priv;
 	unsigned int i = ring->index;
 	u32 reg;
 
-	bcmgenet_rdma_ring_writel(priv, i, ring->dim.coal_pkts,
-				  DMA_MBUF_DONE_THRESH);
+	bcmgenet_rdma_ring_writel(priv, i, pkts, DMA_MBUF_DONE_THRESH);
 
 	reg = bcmgenet_rdma_readl(priv, DMA_RING0_TIMEOUT + i);
 	reg &= ~DMA_TIMEOUT_MASK;
-	reg |= DIV_ROUND_UP(ring->dim.coal_usecs * 1000, 8192);
+	reg |= DIV_ROUND_UP(usecs * 1000, 8192);
 	bcmgenet_rdma_writel(priv, reg, DMA_RING0_TIMEOUT + i);
 }
 
+static void bcmgenet_set_ring_rx_coalesce(struct bcmgenet_rx_ring *ring,
+					  struct ethtool_coalesce *ec)
+{
+	struct net_dim_cq_moder moder;
+	u32 usecs, pkts;
+
+	ring->rx_coalesce_usecs = ec->rx_coalesce_usecs;
+	ring->rx_max_coalesced_frames = ec->rx_max_coalesced_frames;
+	usecs = ring->rx_coalesce_usecs;
+	pkts = ring->rx_max_coalesced_frames;
+
+	if (ec->use_adaptive_rx_coalesce && !ring->dim.use_dim) {
+		moder = net_dim_get_def_profile(ring->dim.dim.mode);
+		usecs = moder.usec;
+		pkts = moder.pkts;
+	}
+
+	ring->dim.use_dim = ec->use_adaptive_rx_coalesce;
+	bcmgenet_set_rx_coalesce(ring, usecs, pkts);
+}
+
 static int bcmgenet_set_coalesce(struct net_device *dev,
 				 struct ethtool_coalesce *ec)
 {
 	struct bcmgenet_priv *priv = netdev_priv(dev);
-	struct bcmgenet_rx_ring *ring;
 	unsigned int i;
 
 	/* Base system clock is 125Mhz, DMA timeout is this reference clock
@@ -680,27 +700,9 @@ static int bcmgenet_set_coalesce(struct net_device *dev,
 				  ec->tx_max_coalesced_frames,
 				  DMA_MBUF_DONE_THRESH);
 
-	for (i = 0; i < priv->hw_params->rx_queues; i++) {
-		ring = &priv->rx_rings[i];
-		ring->dim.coal_usecs = ec->rx_coalesce_usecs;
-		ring->dim.coal_pkts = ec->rx_max_coalesced_frames;
-		if (!ec->use_adaptive_rx_coalesce && ring->dim.use_dim) {
-			ring->dim.coal_pkts = 1;
-			ring->dim.coal_usecs = 0;
-		}
-		ring->dim.use_dim = ec->use_adaptive_rx_coalesce;
-		bcmgenet_set_rx_coalesce(ring);
-	}
-
-	ring = &priv->rx_rings[DESC_INDEX];
-	ring->dim.coal_usecs = ec->rx_coalesce_usecs;
-	ring->dim.coal_pkts = ec->rx_max_coalesced_frames;
-	if (!ec->use_adaptive_rx_coalesce && ring->dim.use_dim) {
-		ring->dim.coal_pkts = 1;
-		ring->dim.coal_usecs = 0;
-	}
-	ring->dim.use_dim = ec->use_adaptive_rx_coalesce;
-	bcmgenet_set_rx_coalesce(ring);
+	for (i = 0; i < priv->hw_params->rx_queues; i++)
+		bcmgenet_set_ring_rx_coalesce(&priv->rx_rings[i], ec);
+	bcmgenet_set_ring_rx_coalesce(&priv->rx_rings[DESC_INDEX], ec);
 
 	return 0;
 }
@@ -1924,10 +1926,7 @@ static void bcmgenet_dim_work(struct work_struct *work)
 	struct net_dim_cq_moder cur_profile =
 			net_dim_get_profile(dim->mode, dim->profile_ix);
 
-	ring->dim.coal_usecs = cur_profile.usec;
-	ring->dim.coal_pkts = cur_profile.pkts;
-
-	bcmgenet_set_rx_coalesce(ring);
+	bcmgenet_set_rx_coalesce(ring, cur_profile.usec, cur_profile.pkts);
 	dim->state = NET_DIM_START_MEASURE;
 }
 
@@ -2079,9 +2078,11 @@ static void init_umac(struct bcmgenet_priv *priv)
 	dev_dbg(kdev, "done init umac\n");
 }
 
-static void bcmgenet_init_dim(struct bcmgenet_net_dim *dim,
+static void bcmgenet_init_dim(struct bcmgenet_rx_ring *ring,
 			      void (*cb)(struct work_struct *work))
 {
+	struct bcmgenet_net_dim *dim = &ring->dim;
+
 	INIT_WORK(&dim->dim.work, cb);
 	dim->dim.mode = NET_DIM_CQ_PERIOD_MODE_START_FROM_EQE;
 	dim->event_ctr = 0;
@@ -2089,6 +2090,25 @@ static void bcmgenet_init_dim(struct bcmgenet_net_dim *dim,
 	dim->bytes = 0;
 }
 
+static void bcmgenet_init_rx_coalesce(struct bcmgenet_rx_ring *ring)
+{
+	struct bcmgenet_net_dim *dim = &ring->dim;
+	struct net_dim_cq_moder moder;
+	u32 usecs, pkts;
+
+	usecs = ring->rx_coalesce_usecs;
+	pkts = ring->rx_max_coalesced_frames;
+
+	/* If DIM was enabled, re-apply default parameters */
+	if (dim->use_dim) {
+		moder = net_dim_get_def_profile(dim->dim.mode);
+		usecs = moder.usec;
+		pkts = moder.pkts;
+	}
+
+	bcmgenet_set_rx_coalesce(ring, usecs, pkts);
+}
+
 /* Initialize a Tx ring along with corresponding hardware registers */
 static void bcmgenet_init_tx_ring(struct bcmgenet_priv *priv,
 				  unsigned int index, unsigned int size,
@@ -2178,7 +2198,8 @@ static int bcmgenet_init_rx_ring(struct bcmgenet_priv *priv,
 	if (ret)
 		return ret;
 
-	bcmgenet_init_dim(&ring->dim, bcmgenet_dim_work);
+	bcmgenet_init_dim(ring, bcmgenet_dim_work);
+	bcmgenet_init_rx_coalesce(ring);
 
 	/* Initialize Rx NAPI */
 	netif_napi_add(priv->dev, &ring->napi, bcmgenet_rx_poll,
@@ -2186,7 +2207,6 @@ static int bcmgenet_init_rx_ring(struct bcmgenet_priv *priv,
 
 	bcmgenet_rdma_ring_writel(priv, index, 0, RDMA_PROD_INDEX);
 	bcmgenet_rdma_ring_writel(priv, index, 0, RDMA_CONS_INDEX);
-	bcmgenet_rdma_ring_writel(priv, index, 1, DMA_MBUF_DONE_THRESH);
 	bcmgenet_rdma_ring_writel(priv, index,
 				  ((size << DMA_RING_SIZE_SHIFT) |
 				   RX_BUF_LENGTH), DMA_RING_BUF_SIZE);
@@ -3424,6 +3444,7 @@ static int bcmgenet_probe(struct platform_device *pdev)
 	struct net_device *dev;
 	const void *macaddr;
 	struct resource *r;
+	unsigned int i;
 	int err = -EIO;
 	const char *phy_mode_str;
 
@@ -3552,6 +3573,11 @@ static int bcmgenet_probe(struct platform_device *pdev)
 	netif_set_real_num_tx_queues(priv->dev, priv->hw_params->tx_queues + 1);
 	netif_set_real_num_rx_queues(priv->dev, priv->hw_params->rx_queues + 1);
 
+	/* Set default coalescing parameters */
+	for (i = 0; i < priv->hw_params->rx_queues; i++)
+		priv->rx_rings[i].rx_max_coalesced_frames = 1;
+	priv->rx_rings[DESC_INDEX].rx_max_coalesced_frames = 1;
+
 	/* libphy will determine the link state */
 	netif_carrier_off(dev);
 
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.h b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
index 22c41e0430fb..b773bc07edf7 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.h
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
@@ -578,8 +578,6 @@ struct bcmgenet_net_dim {
 	u16		event_ctr;
 	unsigned long	packets;
 	unsigned long	bytes;
-	u32		coal_usecs;
-	u32		coal_pkts;
 	struct net_dim	dim;
 };
 
@@ -598,6 +596,8 @@ struct bcmgenet_rx_ring {
 	unsigned int	end_ptr;	/* Rx ring end CB ptr */
 	unsigned int	old_discards;
 	struct bcmgenet_net_dim dim;
+	u32		rx_max_coalesced_frames;
+	u32		rx_coalesce_usecs;
 	void (*int_enable)(struct bcmgenet_rx_ring *);
 	void (*int_disable)(struct bcmgenet_rx_ring *);
 	struct bcmgenet_priv *priv;
-- 
2.14.1

^ permalink raw reply related

* [PATCH net-next v2 2/3] net: systemport: Fix coalescing settings handling
From: Florian Fainelli @ 2018-03-28 22:15 UTC (permalink / raw)
  To: netdev
  Cc: Florian Fainelli, davem, jaedon.shin, pgynther, opendmb,
	Michael Chan, gospo, talgi, saeedm
In-Reply-To: <20180328221538.29290-1-f.fainelli@gmail.com>

There were a number of issues with setting the RX coalescing parameters:

- we would not be preserving values that would have been configured
  across close/open calls, instead we would always reset to no timeout
  and 1 interrupt per packet, this would also prevent DIM from setting its
  default usec/pkts values

- when adaptive RX would be turned on, we woud not be fetching the
  default parameters, we would stay with no timeout/1 packet per
  interrupt until the estimator kicks in and changes that

- finally disabling adaptive RX coalescing while providing parameters
  would not be honored, and we would stay with whatever DIM had
  previously determined instead of the user requested parameters

Fixes: b6e0e875421e ("net: systemport: Implement adaptive interrupt coalescing")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/broadcom/bcmsysport.c | 62 +++++++++++++++++++++---------
 drivers/net/ethernet/broadcom/bcmsysport.h |  4 +-
 2 files changed, 46 insertions(+), 20 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
index 1e52bb7d822e..4a75b1de22e0 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.c
+++ b/drivers/net/ethernet/broadcom/bcmsysport.c
@@ -574,16 +574,16 @@ static int bcm_sysport_set_wol(struct net_device *dev,
 	return 0;
 }
 
-static void bcm_sysport_set_rx_coalesce(struct bcm_sysport_priv *priv)
+static void bcm_sysport_set_rx_coalesce(struct bcm_sysport_priv *priv,
+					u32 usecs, u32 pkts)
 {
 	u32 reg;
 
 	reg = rdma_readl(priv, RDMA_MBDONE_INTR);
 	reg &= ~(RDMA_INTR_THRESH_MASK |
 		 RDMA_TIMEOUT_MASK << RDMA_TIMEOUT_SHIFT);
-	reg |= priv->dim.coal_pkts;
-	reg |= DIV_ROUND_UP(priv->dim.coal_usecs * 1000, 8192) <<
-			    RDMA_TIMEOUT_SHIFT;
+	reg |= pkts;
+	reg |= DIV_ROUND_UP(usecs * 1000, 8192) << RDMA_TIMEOUT_SHIFT;
 	rdma_writel(priv, reg, RDMA_MBDONE_INTR);
 }
 
@@ -626,6 +626,8 @@ static int bcm_sysport_set_coalesce(struct net_device *dev,
 				    struct ethtool_coalesce *ec)
 {
 	struct bcm_sysport_priv *priv = netdev_priv(dev);
+	struct net_dim_cq_moder moder;
+	u32 usecs, pkts;
 	unsigned int i;
 
 	/* Base system clock is 125Mhz, DMA timeout is this reference clock
@@ -646,15 +648,21 @@ static int bcm_sysport_set_coalesce(struct net_device *dev,
 	for (i = 0; i < dev->num_tx_queues; i++)
 		bcm_sysport_set_tx_coalesce(&priv->tx_rings[i], ec);
 
-	priv->dim.coal_usecs = ec->rx_coalesce_usecs;
-	priv->dim.coal_pkts = ec->rx_max_coalesced_frames;
+	priv->rx_coalesce_usecs = ec->rx_coalesce_usecs;
+	priv->rx_max_coalesced_frames = ec->rx_max_coalesced_frames;
+	usecs = priv->rx_coalesce_usecs;
+	pkts = priv->rx_max_coalesced_frames;
 
-	if (!ec->use_adaptive_rx_coalesce && priv->dim.use_dim) {
-		priv->dim.coal_pkts = 1;
-		priv->dim.coal_usecs = 0;
+	if (ec->use_adaptive_rx_coalesce && !priv->dim.use_dim) {
+		moder = net_dim_get_def_profile(priv->dim.dim.mode);
+		usecs = moder.usec;
+		pkts = moder.pkts;
 	}
+
 	priv->dim.use_dim = ec->use_adaptive_rx_coalesce;
-	bcm_sysport_set_rx_coalesce(priv);
+
+	/* Apply desired coalescing parameters */
+	bcm_sysport_set_rx_coalesce(priv, usecs, pkts);
 
 	return 0;
 }
@@ -1058,10 +1066,7 @@ static void bcm_sysport_dim_work(struct work_struct *work)
 	struct net_dim_cq_moder cur_profile =
 				net_dim_get_profile(dim->mode, dim->profile_ix);
 
-	priv->dim.coal_usecs = cur_profile.usec;
-	priv->dim.coal_pkts = cur_profile.pkts;
-
-	bcm_sysport_set_rx_coalesce(priv);
+	bcm_sysport_set_rx_coalesce(priv, cur_profile.usec, cur_profile.pkts);
 	dim->state = NET_DIM_START_MEASURE;
 }
 
@@ -1408,9 +1413,11 @@ static void bcm_sysport_adj_link(struct net_device *dev)
 		phy_print_status(phydev);
 }
 
-static void bcm_sysport_init_dim(struct bcm_sysport_net_dim *dim,
+static void bcm_sysport_init_dim(struct bcm_sysport_priv *priv,
 				 void (*cb)(struct work_struct *work))
 {
+	struct bcm_sysport_net_dim *dim = &priv->dim;
+
 	INIT_WORK(&dim->dim.work, cb);
 	dim->dim.mode = NET_DIM_CQ_PERIOD_MODE_START_FROM_EQE;
 	dim->event_ctr = 0;
@@ -1418,6 +1425,25 @@ static void bcm_sysport_init_dim(struct bcm_sysport_net_dim *dim,
 	dim->bytes = 0;
 }
 
+static void bcm_sysport_init_rx_coalesce(struct bcm_sysport_priv *priv)
+{
+	struct bcm_sysport_net_dim *dim = &priv->dim;
+	struct net_dim_cq_moder moder;
+	u32 usecs, pkts;
+
+	usecs = priv->rx_coalesce_usecs;
+	pkts = priv->rx_max_coalesced_frames;
+
+	/* If DIM was enabled, re-apply default parameters */
+	if (dim->use_dim) {
+		moder = net_dim_get_def_profile(dim->dim.mode);
+		usecs = moder.usec;
+		pkts = moder.pkts;
+	}
+
+	bcm_sysport_set_rx_coalesce(priv, usecs, pkts);
+}
+
 static int bcm_sysport_init_tx_ring(struct bcm_sysport_priv *priv,
 				    unsigned int index)
 {
@@ -1658,8 +1684,6 @@ static int bcm_sysport_init_rx_ring(struct bcm_sysport_priv *priv)
 	rdma_writel(priv, 0, RDMA_END_ADDR_HI);
 	rdma_writel(priv, priv->num_rx_desc_words - 1, RDMA_END_ADDR_LO);
 
-	rdma_writel(priv, 1, RDMA_MBDONE_INTR);
-
 	netif_dbg(priv, hw, priv->netdev,
 		  "RDMA cfg, num_rx_bds=%d, rx_bds=%p\n",
 		  priv->num_rx_bds, priv->rx_bds);
@@ -1827,7 +1851,8 @@ static void bcm_sysport_netif_start(struct net_device *dev)
 	struct bcm_sysport_priv *priv = netdev_priv(dev);
 
 	/* Enable NAPI */
-	bcm_sysport_init_dim(&priv->dim, bcm_sysport_dim_work);
+	bcm_sysport_init_dim(priv, bcm_sysport_dim_work);
+	bcm_sysport_init_rx_coalesce(priv);
 	napi_enable(&priv->napi);
 
 	/* Enable RX interrupt and TX ring full interrupt */
@@ -2333,6 +2358,7 @@ static int bcm_sysport_probe(struct platform_device *pdev)
 	/* libphy will adjust the link state accordingly */
 	netif_carrier_off(dev);
 
+	priv->rx_max_coalesced_frames = 1;
 	u64_stats_init(&priv->syncp);
 
 	priv->dsa_notifier.notifier_call = bcm_sysport_dsa_notifier;
diff --git a/drivers/net/ethernet/broadcom/bcmsysport.h b/drivers/net/ethernet/broadcom/bcmsysport.h
index 57e18ef8f206..d6e5d0cbf3a3 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.h
+++ b/drivers/net/ethernet/broadcom/bcmsysport.h
@@ -701,8 +701,6 @@ struct bcm_sysport_net_dim {
 	u16			event_ctr;
 	unsigned long		packets;
 	unsigned long		bytes;
-	u32			coal_usecs;
-	u32			coal_pkts;
 	struct net_dim		dim;
 };
 
@@ -755,6 +753,8 @@ struct bcm_sysport_priv {
 	unsigned int		rx_c_index;
 
 	struct bcm_sysport_net_dim	dim;
+	u32			rx_max_coalesced_frames;
+	u32			rx_coalesce_usecs;
 
 	/* PHY device */
 	struct device_node	*phy_dn;
-- 
2.14.1

^ permalink raw reply related

* [PATCH net-next v2 1/3] net: systemport: Remove adaptive TX coalescing
From: Florian Fainelli @ 2018-03-28 22:15 UTC (permalink / raw)
  To: netdev
  Cc: Florian Fainelli, davem, jaedon.shin, pgynther, opendmb,
	Michael Chan, gospo, talgi, saeedm
In-Reply-To: <20180328221538.29290-1-f.fainelli@gmail.com>

Adaptive TX coalescing is not currently giving us any advantages and
ends up making the CPU spin more frequently until TX completion. Deny
and disable adaptive TX coalescing for now and rely on static
configuration, we can always add it back later.

Reviewed-by: Tal Gilboa <talgi@mellanox.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/broadcom/bcmsysport.c | 61 ++++--------------------------
 drivers/net/ethernet/broadcom/bcmsysport.h |  1 -
 2 files changed, 8 insertions(+), 54 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
index 4e26f606a7f2..1e52bb7d822e 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.c
+++ b/drivers/net/ethernet/broadcom/bcmsysport.c
@@ -15,7 +15,6 @@
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/netdevice.h>
-#include <linux/net_dim.h>
 #include <linux/etherdevice.h>
 #include <linux/platform_device.h>
 #include <linux/of.h>
@@ -588,7 +587,8 @@ static void bcm_sysport_set_rx_coalesce(struct bcm_sysport_priv *priv)
 	rdma_writel(priv, reg, RDMA_MBDONE_INTR);
 }
 
-static void bcm_sysport_set_tx_coalesce(struct bcm_sysport_tx_ring *ring)
+static void bcm_sysport_set_tx_coalesce(struct bcm_sysport_tx_ring *ring,
+					struct ethtool_coalesce *ec)
 {
 	struct bcm_sysport_priv *priv = ring->priv;
 	u32 reg;
@@ -596,8 +596,8 @@ static void bcm_sysport_set_tx_coalesce(struct bcm_sysport_tx_ring *ring)
 	reg = tdma_readl(priv, TDMA_DESC_RING_INTR_CONTROL(ring->index));
 	reg &= ~(RING_INTR_THRESH_MASK |
 		 RING_TIMEOUT_MASK << RING_TIMEOUT_SHIFT);
-	reg |= ring->dim.coal_pkts;
-	reg |= DIV_ROUND_UP(ring->dim.coal_usecs * 1000, 8192) <<
+	reg |= ec->tx_max_coalesced_frames;
+	reg |= DIV_ROUND_UP(ec->tx_coalesce_usecs * 1000, 8192) <<
 			    RING_TIMEOUT_SHIFT;
 	tdma_writel(priv, reg, TDMA_DESC_RING_INTR_CONTROL(ring->index));
 }
@@ -606,18 +606,12 @@ static int bcm_sysport_get_coalesce(struct net_device *dev,
 				    struct ethtool_coalesce *ec)
 {
 	struct bcm_sysport_priv *priv = netdev_priv(dev);
-	struct bcm_sysport_tx_ring *ring;
-	unsigned int i;
 	u32 reg;
 
 	reg = tdma_readl(priv, TDMA_DESC_RING_INTR_CONTROL(0));
 
 	ec->tx_coalesce_usecs = (reg >> RING_TIMEOUT_SHIFT) * 8192 / 1000;
 	ec->tx_max_coalesced_frames = reg & RING_INTR_THRESH_MASK;
-	for (i = 0; i < dev->num_tx_queues; i++) {
-		ring = &priv->tx_rings[i];
-		ec->use_adaptive_tx_coalesce |= ring->dim.use_dim;
-	}
 
 	reg = rdma_readl(priv, RDMA_MBDONE_INTR);
 
@@ -632,7 +626,6 @@ static int bcm_sysport_set_coalesce(struct net_device *dev,
 				    struct ethtool_coalesce *ec)
 {
 	struct bcm_sysport_priv *priv = netdev_priv(dev);
-	struct bcm_sysport_tx_ring *ring;
 	unsigned int i;
 
 	/* Base system clock is 125Mhz, DMA timeout is this reference clock
@@ -646,20 +639,12 @@ static int bcm_sysport_set_coalesce(struct net_device *dev,
 		return -EINVAL;
 
 	if ((ec->tx_coalesce_usecs == 0 && ec->tx_max_coalesced_frames == 0) ||
-	    (ec->rx_coalesce_usecs == 0 && ec->rx_max_coalesced_frames == 0))
+	    (ec->rx_coalesce_usecs == 0 && ec->rx_max_coalesced_frames == 0) ||
+	    ec->use_adaptive_tx_coalesce)
 		return -EINVAL;
 
-	for (i = 0; i < dev->num_tx_queues; i++) {
-		ring = &priv->tx_rings[i];
-		ring->dim.coal_pkts = ec->tx_max_coalesced_frames;
-		ring->dim.coal_usecs = ec->tx_coalesce_usecs;
-		if (!ec->use_adaptive_tx_coalesce && ring->dim.use_dim) {
-			ring->dim.coal_pkts = 1;
-			ring->dim.coal_usecs = 0;
-		}
-		ring->dim.use_dim = ec->use_adaptive_tx_coalesce;
-		bcm_sysport_set_tx_coalesce(ring);
-	}
+	for (i = 0; i < dev->num_tx_queues; i++)
+		bcm_sysport_set_tx_coalesce(&priv->tx_rings[i], ec);
 
 	priv->dim.coal_usecs = ec->rx_coalesce_usecs;
 	priv->dim.coal_pkts = ec->rx_max_coalesced_frames;
@@ -940,8 +925,6 @@ static unsigned int __bcm_sysport_tx_reclaim(struct bcm_sysport_priv *priv,
 	ring->packets += pkts_compl;
 	ring->bytes += bytes_compl;
 	u64_stats_update_end(&priv->syncp);
-	ring->dim.packets = pkts_compl;
-	ring->dim.bytes = bytes_compl;
 
 	ring->c_index = c_index;
 
@@ -987,7 +970,6 @@ static int bcm_sysport_tx_poll(struct napi_struct *napi, int budget)
 {
 	struct bcm_sysport_tx_ring *ring =
 		container_of(napi, struct bcm_sysport_tx_ring, napi);
-	struct net_dim_sample dim_sample;
 	unsigned int work_done = 0;
 
 	work_done = bcm_sysport_tx_reclaim(ring->priv, ring);
@@ -1004,12 +986,6 @@ static int bcm_sysport_tx_poll(struct napi_struct *napi, int budget)
 		return 0;
 	}
 
-	if (ring->dim.use_dim) {
-		net_dim_sample(ring->dim.event_ctr, ring->dim.packets,
-			       ring->dim.bytes, &dim_sample);
-		net_dim(&ring->dim.dim, dim_sample);
-	}
-
 	return budget;
 }
 
@@ -1089,23 +1065,6 @@ static void bcm_sysport_dim_work(struct work_struct *work)
 	dim->state = NET_DIM_START_MEASURE;
 }
 
-static void bcm_sysport_dim_tx_work(struct work_struct *work)
-{
-	struct net_dim *dim = container_of(work, struct net_dim, work);
-	struct bcm_sysport_net_dim *ndim =
-			container_of(dim, struct bcm_sysport_net_dim, dim);
-	struct bcm_sysport_tx_ring *ring =
-			container_of(ndim, struct bcm_sysport_tx_ring, dim);
-	struct net_dim_cq_moder cur_profile =
-				net_dim_get_profile(dim->mode, dim->profile_ix);
-
-	ring->dim.coal_usecs = cur_profile.usec;
-	ring->dim.coal_pkts = cur_profile.pkts;
-
-	bcm_sysport_set_tx_coalesce(ring);
-	dim->state = NET_DIM_START_MEASURE;
-}
-
 /* RX and misc interrupt routine */
 static irqreturn_t bcm_sysport_rx_isr(int irq, void *dev_id)
 {
@@ -1152,7 +1111,6 @@ static irqreturn_t bcm_sysport_rx_isr(int irq, void *dev_id)
 			continue;
 
 		txr = &priv->tx_rings[ring];
-		txr->dim.event_ctr++;
 
 		if (likely(napi_schedule_prep(&txr->napi))) {
 			intrl2_0_mask_set(priv, ring_bit);
@@ -1185,7 +1143,6 @@ static irqreturn_t bcm_sysport_tx_isr(int irq, void *dev_id)
 			continue;
 
 		txr = &priv->tx_rings[ring];
-		txr->dim.event_ctr++;
 
 		if (likely(napi_schedule_prep(&txr->napi))) {
 			intrl2_1_mask_set(priv, BIT(ring));
@@ -1551,7 +1508,6 @@ static int bcm_sysport_init_tx_ring(struct bcm_sysport_priv *priv,
 	reg |= (1 << index);
 	tdma_writel(priv, reg, TDMA_TIER1_ARB_0_QUEUE_EN);
 
-	bcm_sysport_init_dim(&ring->dim, bcm_sysport_dim_tx_work);
 	napi_enable(&ring->napi);
 
 	netif_dbg(priv, hw, priv->netdev,
@@ -1582,7 +1538,6 @@ static void bcm_sysport_fini_tx_ring(struct bcm_sysport_priv *priv,
 		return;
 
 	napi_disable(&ring->napi);
-	cancel_work_sync(&ring->dim.dim.work);
 	netif_napi_del(&ring->napi);
 
 	bcm_sysport_tx_clean(priv, ring);
diff --git a/drivers/net/ethernet/broadcom/bcmsysport.h b/drivers/net/ethernet/broadcom/bcmsysport.h
index e1c97d4a82b4..57e18ef8f206 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.h
+++ b/drivers/net/ethernet/broadcom/bcmsysport.h
@@ -723,7 +723,6 @@ struct bcm_sysport_tx_ring {
 	struct bcm_sysport_priv *priv;	/* private context backpointer */
 	unsigned long	packets;	/* packets statistics */
 	unsigned long	bytes;		/* bytes statistics */
-	struct bcm_sysport_net_dim dim;	/* Net DIM context */
 	unsigned int	switch_queue;	/* switch port queue number */
 	unsigned int	switch_port;	/* switch port queue number */
 	bool		inspect;	/* inspect switch port and queue */
-- 
2.14.1

^ permalink raw reply related

* [PATCH net-next v2 0/3] net: Broadcom drivers coalescing fixes
From: Florian Fainelli @ 2018-03-28 22:15 UTC (permalink / raw)
  To: netdev
  Cc: Florian Fainelli, davem, jaedon.shin, pgynther, opendmb,
	Michael Chan, gospo, talgi, saeedm

Hi all,

Following Tal's review of the adaptive RX/TX coalescing feature added to the
SYSTEMPORT and GENET driver a number of things showed up:

- adaptive TX coalescing is not actually a good idea with the current way
  the estimator will program the ring, this results in a higher CPU load, NAPI
  on TX already does a reasonably good job at maintaining the interrupt count low

- both SYSTEMPORT and GENET would suffer from the same issues while configuring
  coalescing parameters where the values would just not be applied correctly
  based on user settings, so we fix that too

Tal, thanks again for your feedback, I would appreciate if you could review that
the new behavior appears to be implemented correctly.

Thanks!

Changes in v2:

- added Tal's reviewed-by to the first patch
- split DIM initialization from coalescing parameters initialization
- avoid duplicating the same code in bcmgenet_set_coalesce() when configuring RX rings
- fixed the condition where default DIM parameters would be applied when
  adaptive RX coalescing would be enabled, do this only if it was disabled before

Florian Fainelli (3):
  net: systemport: Remove adaptive TX coalescing
  net: systemport: Fix coalescing settings handling
  net: bcmgenet: Fix coalescing settings handling

 drivers/net/ethernet/broadcom/bcmsysport.c     | 123 +++++++++++--------------
 drivers/net/ethernet/broadcom/bcmsysport.h     |   5 +-
 drivers/net/ethernet/broadcom/genet/bcmgenet.c |  92 +++++++++++-------
 drivers/net/ethernet/broadcom/genet/bcmgenet.h |   4 +-
 4 files changed, 115 insertions(+), 109 deletions(-)

-- 
2.14.1

^ permalink raw reply

* Re: RFC on writel and writel_relaxed
From: Nicholas Piggin @ 2018-03-28 22:09 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: David Miller, paulmck, arnd, linux-rdma, linuxppc-dev, linus971,
	will.deacon, alexander.duyck, okaya, jgg, David.Laight, oohall,
	netdev, alexander.h.duyck, torvalds
In-Reply-To: <1522272692.21446.42.camel@kernel.crashing.org>

On Thu, 29 Mar 2018 08:31:32 +1100
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> On Thu, 2018-03-29 at 02:23 +1000, Nicholas Piggin wrote:
> > On Wed, 28 Mar 2018 11:55:09 -0400 (EDT)
> > David Miller <davem@davemloft.net> wrote:
> >   
> > > From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > > Date: Thu, 29 Mar 2018 02:13:16 +1100
> > >   
> > > > Let's fix all archs, it's way easier than fixing all drivers. Half of
> > > > the archs are unused or dead anyway.    
> > > 
> > > Agreed.  
> > 
> > While we're making decrees here, can we do something about mmiowb?
> > The semantics are basically indecipherable.  
> 
> I was going to tackle that next :-)
> 
> >   This is a variation on the mandatory write barrier that causes writes to weakly
> >   ordered I/O regions to be partially ordered.  Its effects may go beyond the
> >   CPU->Hardware interface and actually affect the hardware at some level.
> > 
> > How can a driver writer possibly get that right?
> > 
> > IIRC it was added for some big ia64 system that was really expensive
> > to implement the proper wmb() semantics on. So wmb() semantics were
> > quietly downgraded, then the subsequently broken drivers they cared
> > about were fixed by adding the stronger mmiowb().
> > 
> > What should have happened was wmb and writel remained correct, sane, and
> > expensive, and they add an mmio_wmb() to order MMIO stores made by the
> > writel_relaxed accessors, then use that to speed up the few drivers they
> > care about.
> > 
> > Now that ia64 doesn't matter too much, can we deprecate mmiowb and just
> > make wmb ordering talk about stores to the device, not to some
> > intermediate stage of the interconnect where it can be subsequently
> > reordered wrt the device? Drivers can be converted back to using wmb
> > or writel gradually.  
> 
> I was under the impression that mmiowb was specifically about ordering
> writel's with a subsequent spin_unlock, without it, MMIOs from
> different CPUs (within the same lock) would still arrive OO.

Yes more or less, and I think that until mmiowb was introduced, wmb
or writel was sufficient for this.

Thanks,
Nick

^ permalink raw reply

* Re: [PATCH] net: phy: marvell10g: add thermal hwmon device
From: Andrew Lunn @ 2018-03-28 22:09 UTC (permalink / raw)
  To: Russell King, Guenter Roeck; +Cc: Florian Fainelli, netdev
In-Reply-To: <E1f18Bu-0005CL-O4@rmk-PC.armlinux.org.uk>

Hi Russell

Guenter Roeck, the HWMON maintainer should also be involved in
reviewing this patch.

	  Andrew


On Wed, Mar 28, 2018 at 11:20:26AM +0100, Russell King wrote:
> Add a thermal monitoring device for the Marvell 88x3310, which updates
> once a second.  We also need to hook into the suspend/resume mechanism
> to ensure that the thermal monitoring is reconfigured when we resume.
> 
> Suggested-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>  drivers/net/phy/marvell10g.c | 184 ++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 182 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
> index 8a0bd98fdec7..db9d66781da6 100644
> --- a/drivers/net/phy/marvell10g.c
> +++ b/drivers/net/phy/marvell10g.c
> @@ -21,8 +21,10 @@
>   * If both the fiber and copper ports are connected, the first to gain
>   * link takes priority and the other port is completely locked out.
>   */
> -#include <linux/phy.h>
> +#include <linux/ctype.h>
> +#include <linux/hwmon.h>
>  #include <linux/marvell_phy.h>
> +#include <linux/phy.h>
>  
>  enum {
>  	MV_PCS_BASE_T		= 0x0000,
> @@ -40,6 +42,19 @@ enum {
>  	 */
>  	MV_AN_CTRL1000		= 0x8000, /* 1000base-T control register */
>  	MV_AN_STAT1000		= 0x8001, /* 1000base-T status register */
> +
> +	/* Vendor2 MMD registers */
> +	MV_V2_TEMP_CTRL		= 0xf08a,
> +	MV_V2_TEMP_CTRL_MASK	= 0xc000,
> +	MV_V2_TEMP_CTRL_SAMPLE	= 0x0000,
> +	MV_V2_TEMP_CTRL_DISABLE	= 0xc000,
> +	MV_V2_TEMP		= 0xf08c,
> +	MV_V2_TEMP_UNKNOWN	= 0x9600, /* unknown function */
> +};
> +
> +struct mv3310_priv {
> +	struct device *hwmon_dev;
> +	char *hwmon_name;
>  };
>  
>  static int mv3310_modify(struct phy_device *phydev, int devad, u16 reg,
> @@ -60,17 +75,180 @@ static int mv3310_modify(struct phy_device *phydev, int devad, u16 reg,
>  	return ret < 0 ? ret : 1;
>  }
>  
> +#ifdef CONFIG_HWMON
> +static umode_t mv3310_hwmon_is_visible(const void *data,
> +				       enum hwmon_sensor_types type,
> +				       u32 attr, int channel)
> +{
> +	if (type == hwmon_chip && attr == hwmon_chip_update_interval)
> +		return 0444;
> +	if (type == hwmon_temp && attr == hwmon_temp_input)
> +		return 0444;
> +	return 0;
> +}
> +
> +static int mv3310_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
> +			     u32 attr, int channel, long *value)
> +{
> +	struct phy_device *phydev = dev_get_drvdata(dev);
> +	int temp;
> +
> +	if (type == hwmon_chip && attr == hwmon_chip_update_interval) {
> +		*value = MSEC_PER_SEC;
> +		return 0;
> +	}
> +
> +	if (type == hwmon_temp && attr == hwmon_temp_input) {
> +		temp = phy_read_mmd(phydev, MDIO_MMD_VEND2, MV_V2_TEMP);
> +		if (temp < 0)
> +			return temp;
> +
> +		*value = ((temp & 0xff) - 75) * 1000;
> +
> +		return 0;
> +	}
> +
> +	return -EOPNOTSUPP;
> +}
> +
> +static const struct hwmon_ops mv3310_hwmon_ops = {
> +	.is_visible = mv3310_hwmon_is_visible,
> +	.read = mv3310_hwmon_read,
> +};
> +
> +static u32 mv3310_hwmon_chip_config[] = {
> +	HWMON_C_REGISTER_TZ | HWMON_C_UPDATE_INTERVAL,
> +	0,
> +};
> +
> +static const struct hwmon_channel_info mv3310_hwmon_chip = {
> +	.type = hwmon_chip,
> +	.config = mv3310_hwmon_chip_config,
> +};
> +
> +static u32 mv3310_hwmon_temp_config[] = {
> +	HWMON_T_INPUT,
> +	0,
> +};
> +
> +static const struct hwmon_channel_info mv3310_hwmon_temp = {
> +	.type = hwmon_temp,
> +	.config = mv3310_hwmon_temp_config,
> +};
> +
> +static const struct hwmon_channel_info *mv3310_hwmon_info[] = {
> +	&mv3310_hwmon_chip,
> +	&mv3310_hwmon_temp,
> +	NULL,
> +};
> +
> +static const struct hwmon_chip_info mv3310_hwmon_chip_info = {
> +	.ops = &mv3310_hwmon_ops,
> +	.info = mv3310_hwmon_info,
> +};
> +
> +static int mv3310_hwmon_config(struct phy_device *phydev, bool enable)
> +{
> +	u16 val;
> +	int ret;
> +
> +	ret = phy_write_mmd(phydev, MDIO_MMD_VEND2, MV_V2_TEMP,
> +			    MV_V2_TEMP_UNKNOWN);
> +	if (ret < 0)
> +		return ret;
> +
> +	val = enable ? MV_V2_TEMP_CTRL_SAMPLE : MV_V2_TEMP_CTRL_DISABLE;
> +	ret = mv3310_modify(phydev, MDIO_MMD_VEND2, MV_V2_TEMP_CTRL,
> +			    MV_V2_TEMP_CTRL_MASK, val);
> +
> +	return ret < 0 ? ret : 0;
> +}
> +
> +static void mv3310_hwmon_disable(void *data)
> +{
> +	struct phy_device *phydev = data;
> +
> +	mv3310_hwmon_config(phydev, false);
> +}
> +
> +static int mv3310_hwmon_probe(struct phy_device *phydev)
> +{
> +	struct device *dev = &phydev->mdio.dev;
> +	struct mv3310_priv *priv = dev_get_drvdata(&phydev->mdio.dev);
> +	int i, j, ret;
> +
> +	priv->hwmon_name = devm_kstrdup(dev, dev_name(dev), GFP_KERNEL);
> +	if (!priv->hwmon_name)
> +		return -ENODEV;
> +
> +	for (i = j = 0; priv->hwmon_name[i]; i++) {
> +		if (isalnum(priv->hwmon_name[i])) {
> +			if (i != j)
> +				priv->hwmon_name[j] = priv->hwmon_name[i];
> +			j++;
> +		}
> +	}
> +	priv->hwmon_name[j] = '\0';
> +
> +	ret = mv3310_hwmon_config(phydev, true);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_add_action_or_reset(dev, mv3310_hwmon_disable, phydev);
> +	if (ret)
> +		return ret;
> +
> +	priv->hwmon_dev = devm_hwmon_device_register_with_info(dev,
> +				priv->hwmon_name, phydev,
> +				&mv3310_hwmon_chip_info, NULL);
> +
> +	return PTR_ERR_OR_ZERO(priv->hwmon_dev);
> +}
> +#else
> +static inline int mv3310_hwmon_config(struct phy_device *phydev, bool enable)
> +{
> +	return 0;
> +}
> +
> +static int mv3310_hwmon_probe(struct phy_device *phydev)
> +{
> +	return 0;
> +}
> +#endif
> +
>  static int mv3310_probe(struct phy_device *phydev)
>  {
> +	struct mv3310_priv *priv;
>  	u32 mmd_mask = MDIO_DEVS_PMAPMD | MDIO_DEVS_AN;
> +	int ret;
>  
>  	if (!phydev->is_c45 ||
>  	    (phydev->c45_ids.devices_in_package & mmd_mask) != mmd_mask)
>  		return -ENODEV;
>  
> +	priv = devm_kzalloc(&phydev->mdio.dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	dev_set_drvdata(&phydev->mdio.dev, priv);
> +
> +	ret = mv3310_hwmon_probe(phydev);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static int mv3310_suspend(struct phy_device *phydev)
> +{
>  	return 0;
>  }
>  
> +static int mv3310_resume(struct phy_device *phydev)
> +{
> +	return mv3310_hwmon_config(phydev, true);
> +}
> +
>  /*
>   * Resetting the MV88X3310 causes it to become non-responsive.  Avoid
>   * setting the reset bit(s).
> @@ -376,9 +554,11 @@ static struct phy_driver mv3310_drivers[] = {
>  				  SUPPORTED_FIBRE |
>  				  SUPPORTED_10000baseT_Full |
>  				  SUPPORTED_Backplane,
> -		.probe		= mv3310_probe,
>  		.soft_reset	= mv3310_soft_reset,
>  		.config_init	= mv3310_config_init,
> +		.probe		= mv3310_probe,
> +		.suspend	= mv3310_suspend,
> +		.resume		= mv3310_resume,
>  		.config_aneg	= mv3310_config_aneg,
>  		.aneg_done	= mv3310_aneg_done,
>  		.read_status	= mv3310_read_status,
> -- 
> 2.7.4
> 

^ permalink raw reply

* [PATCH] ath10k: avoid possible string overflow
From: Arnd Bergmann @ 2018-03-28 22:06 UTC (permalink / raw)
  To: Kalle Valo
  Cc: Arnd Bergmann, Manikanta Pubbisetty, Anilkumar Kolli, Carl Huang,
	Gustavo A. R. Silva, Johannes Berg, Maharaja Kennadyrajan, ath10k,
	linux-wireless, netdev, linux-kernel

The way that 'strncat' is used here raised a warning in gcc-8:

drivers/net/wireless/ath/ath10k/wmi.c: In function 'ath10k_wmi_tpc_stats_final_disp_tables':
drivers/net/wireless/ath/ath10k/wmi.c:4649:4: error: 'strncat' output truncated before terminating nul copying as many bytes from a string as its length [-Werror=stringop-truncation]

Effectively, this is simply a strcat() but the use of strncat() suggests
some form of overflow check. Regardless of whether this might actually
overflow, using strlcat() instead of strncat() avoids the warning and
makes the code more robust.

Fixes: bc64d05220f3 ("ath10k: debugfs support to get final TPC stats for 10.4 variants")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/wireless/ath/ath10k/wmi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index 9649bb752bbd..42522ed115f3 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -4309,7 +4309,7 @@ static void ath10k_tpc_config_disp_tables(struct ath10k *ar,
 							    rate_code[i],
 							    type);
 			snprintf(buff, sizeof(buff), "%8d ", tpc[j]);
-			strncat(tpc_value, buff, strlen(buff));
+			strlcat(tpc_value, buff, sizeof(tpc_value));
 		}
 		tpc_stats->tpc_table[type].pream_idx[i] = pream_idx;
 		tpc_stats->tpc_table[type].rate_code[i] = rate_code[i];
@@ -4646,7 +4646,7 @@ ath10k_wmi_tpc_stats_final_disp_tables(struct ath10k *ar,
 							       rate_code[i],
 							       type, pream_idx);
 			snprintf(buff, sizeof(buff), "%8d ", tpc[j]);
-			strncat(tpc_value, buff, strlen(buff));
+			strlcat(tpc_value, buff, sizeof(tpc_value));
 		}
 		tpc_stats->tpc_table_final[type].pream_idx[i] = pream_idx;
 		tpc_stats->tpc_table_final[type].rate_code[i] = rate_code[i];
-- 
2.9.0

^ permalink raw reply related

* Re: [PATCH net-next 2/2] sfp/phylink: move module EEPROM ethtool access into netdev core ethtool
From: Andrew Lunn @ 2018-03-28 22:02 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: netdev, Russell King, Thomas Petazzoni, David S. Miller,
	open list, Antoine Tenart, Yan Markman, Stefan Chulski,
	Maxime Chevallier, Miquel Raynal, Marcin Wojtas
In-Reply-To: <20180328190339.31433-3-f.fainelli@gmail.com>

On Wed, Mar 28, 2018 at 12:03:39PM -0700, Florian Fainelli wrote:
> From: Russell King <rmk+kernel@armlinux.org.uk>
> 
> Provide a pointer to the SFP bus in struct net_device, so that the
> ethtool module EEPROM methods can access the SFP directly, rather
> than needing every user to provide a hook for it.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>  drivers/net/ethernet/marvell/mvneta.c | 18 ------------------
>  drivers/net/phy/phylink.c             | 28 ----------------------------
>  drivers/net/phy/sfp-bus.c             |  6 ++----
>  include/linux/netdevice.h             |  3 +++
>  include/linux/phylink.h               |  3 ---
>  net/core/ethtool.c                    |  7 +++++++
>  6 files changed, 12 insertions(+), 53 deletions(-)

The diffstats look good, DSA and all other uses of phylink will make a
similar saving in code. The new member in the netdev structure is well
away from members which are commented as being cache line sensitive.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply

* Re: [Resend Patch 1/3] Vmbus: Add function to report available ring buffer to write in total ring size percentage
From: Martin K. Petersen @ 2018-03-28 22:00 UTC (permalink / raw)
  To: Long Li
  Cc: Stephen Hemminger, linux-scsi, Martin K . Petersen, netdev,
	Haiyang Zhang, James E . J . Bottomley, linux-kernel, devel
In-Reply-To: <20180328004840.22787-1-longli@linuxonhyperv.com>


Long,

> Netvsc has a function to calculate how much ring buffer in percentage is
> available to write. This function is also useful for storvsc and other
> vmbus devices.

What is the submission strategy for this series? Do you expect it to go
through net or scsi? If the latter, I'll need an ack from davem.

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply

* Re: [RFC PATCH v2 00/14] Introducing AF_XDP support
From: Eric Leblond @ 2018-03-28 21:18 UTC (permalink / raw)
  To: Björn Töpel, magnus.karlsson, alexander.h.duyck,
	alexander.duyck, john.fastabend, ast, brouer,
	willemdebruijn.kernel, daniel, netdev
  Cc: Björn Töpel, michael.lundkvist, jesse.brandeburg,
	anjali.singhai, qi.z.zhang, ravineet.singh
In-Reply-To: <20180327165919.17933-1-bjorn.topel@gmail.com>

Hello,

On Tue, 2018-03-27 at 18:59 +0200, Björn Töpel wrote:
> From: Björn Töpel <bjorn.topel@intel.com>
> 
> 
optimized for high performance packet processing and, in upcoming
> patch sets, zero-copy semantics. In this v2 version, we have removed
> all zero-copy related code in order to make it smaller, simpler and
> hopefully more review friendly. This RFC only supports copy-mode for
> the generic XDP path (XDP_SKB) for both RX and TX and copy-mode for
> RX
> 

...
> 
> How is then packets distributed between these two XSK? We have
> introduced a new BPF map called XSKMAP (or BPF_MAP_TYPE_XSKMAP in
> full). The user-space application can place an XSK at an arbitrary
> place in this map. The XDP program can then redirect a packet to a
> specific index in this map and at this point XDP validates that the
> XSK in that map was indeed bound to that device and queue number. If
> not, the packet is dropped. If the map is empty at that index, the
> packet is also dropped. This also means that it is currently
> mandatory
> to have an XDP program loaded (and one XSK in the XSKMAP) to be able
> to get any traffic to user space through the XSK.

If I get it correctly, this feature will have to be used to bound
multiple sockets to a single queue and the eBPF filter will be
responsible of the load balancing. Am I correct ?

> AF_XDP can operate in two different modes: XDP_SKB and XDP_DRV. If
> the
> driver does not have support for XDP, or XDP_SKB is explicitly chosen
...

Thanks a lot for this work, I'm gonna try to implement this in
Suricata.

Best regards,

^ permalink raw reply

* Re: [PATCH net-next 2/3] net: systemport: Fix coalescing settings handling
From: Florian Fainelli @ 2018-03-28 21:53 UTC (permalink / raw)
  To: Tal Gilboa, netdev
  Cc: davem, jaedon.shin, pgynther, opendmb, Michael Chan, gospo,
	saeedm
In-Reply-To: <61c5d8c2-5bc8-9f61-97aa-fb42eceb753f@mellanox.com>

On 03/28/2018 02:23 PM, Tal Gilboa wrote:
> On 3/27/2018 10:47 PM, Florian Fainelli wrote:
>> There were a number of issues with setting the RX coalescing parameters:
>>
>> - we would not be preserving values that would have been configured
>>    across close/open calls, instead we would always reset to no timeout
>>    and 1 interrupt per packet, this would also prevent DIM from
>> setting its
>>    default usec/pkts values
>>
>> - when adaptive RX would be turned on, we woud not be fetching the
>>    default parameters, we would stay with no timeout/1 packet per
>>    interrupt until the estimator kicks in and changes that
>>
>> - finally disabling adaptive RX coalescing while providing parameters
>>    would not be honored, and we would stay with whatever DIM had
>>    previously determined instead of the user requested parameters
>>
>> Fixes: b6e0e875421e ("net: systemport: Implement adaptive interrupt
>> coalescing")
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>>   drivers/net/ethernet/broadcom/bcmsysport.c | 57
>> ++++++++++++++++++++----------
>>   drivers/net/ethernet/broadcom/bcmsysport.h |  4 +--
>>   2 files changed, 41 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c
>> b/drivers/net/ethernet/broadcom/bcmsysport.c
>> index 1e52bb7d822e..43ad6300c351 100644
>> --- a/drivers/net/ethernet/broadcom/bcmsysport.c
>> +++ b/drivers/net/ethernet/broadcom/bcmsysport.c
>> @@ -574,16 +574,16 @@ static int bcm_sysport_set_wol(struct net_device
>> *dev,
>>       return 0;
>>   }
>>   -static void bcm_sysport_set_rx_coalesce(struct bcm_sysport_priv *priv)
>> +static void bcm_sysport_set_rx_coalesce(struct bcm_sysport_priv *priv,
>> +                    u32 usecs, u32 pkts)
>>   {
>>       u32 reg;
>>         reg = rdma_readl(priv, RDMA_MBDONE_INTR);
>>       reg &= ~(RDMA_INTR_THRESH_MASK |
>>            RDMA_TIMEOUT_MASK << RDMA_TIMEOUT_SHIFT);
>> -    reg |= priv->dim.coal_pkts;
>> -    reg |= DIV_ROUND_UP(priv->dim.coal_usecs * 1000, 8192) <<
>> -                RDMA_TIMEOUT_SHIFT;
>> +    reg |= pkts;
>> +    reg |= DIV_ROUND_UP(usecs * 1000, 8192) << RDMA_TIMEOUT_SHIFT;
>>       rdma_writel(priv, reg, RDMA_MBDONE_INTR);
>>   }
>>   @@ -626,6 +626,8 @@ static int bcm_sysport_set_coalesce(struct
>> net_device *dev,
>>                       struct ethtool_coalesce *ec)
>>   {
>>       struct bcm_sysport_priv *priv = netdev_priv(dev);
>> +    struct net_dim_cq_moder moder;
>> +    u32 usecs, pkts;
>>       unsigned int i;
>>         /* Base system clock is 125Mhz, DMA timeout is this reference
>> clock
>> @@ -646,15 +648,22 @@ static int bcm_sysport_set_coalesce(struct
>> net_device *dev,
>>       for (i = 0; i < dev->num_tx_queues; i++)
>>           bcm_sysport_set_tx_coalesce(&priv->tx_rings[i], ec);
>>   -    priv->dim.coal_usecs = ec->rx_coalesce_usecs;
>> -    priv->dim.coal_pkts = ec->rx_max_coalesced_frames;
>> +    priv->rx_coalesce_usecs = ec->rx_coalesce_usecs;
>> +    priv->rx_max_coalesced_frames = ec->rx_max_coalesced_frames;
>> +    usecs = priv->rx_coalesce_usecs;
>> +    pkts = priv->rx_max_coalesced_frames;
>>   -    if (!ec->use_adaptive_rx_coalesce && priv->dim.use_dim) {
>> -        priv->dim.coal_pkts = 1;
>> -        priv->dim.coal_usecs = 0;
>> +    /* If DIM is enabled, immediately obtain default parameters */
>> +    if (ec->use_adaptive_rx_coalesce) {
>> +        moder = net_dim_get_def_profile(priv->dim.dim.mode);
>> +        usecs = moder.usec;
>> +        pkts = moder.pkts;
>>       }
> 
> What if DIM is already in use? We don't want to set default values if
> DIM is already working.

Indeed, good catch, thank you!

> 
>> +
>>       priv->dim.use_dim = ec->use_adaptive_rx_coalesce;
>> -    bcm_sysport_set_rx_coalesce(priv);
>> +
>> +    /* Apply desired coalescing parameters */
>> +    bcm_sysport_set_rx_coalesce(priv, usecs, pkts);
>>         return 0;
>>   }
>> @@ -1058,10 +1067,7 @@ static void bcm_sysport_dim_work(struct
>> work_struct *work)
>>       struct net_dim_cq_moder cur_profile =
>>                   net_dim_get_profile(dim->mode, dim->profile_ix);
>>   -    priv->dim.coal_usecs = cur_profile.usec;
>> -    priv->dim.coal_pkts = cur_profile.pkts;
>> -
>> -    bcm_sysport_set_rx_coalesce(priv);
>> +    bcm_sysport_set_rx_coalesce(priv, cur_profile.usec,
>> cur_profile.pkts);
>>       dim->state = NET_DIM_START_MEASURE;
>>   }
>>   @@ -1408,14 +1414,30 @@ static void bcm_sysport_adj_link(struct
>> net_device *dev)
>>           phy_print_status(phydev);
>>   }
>>   -static void bcm_sysport_init_dim(struct bcm_sysport_net_dim *dim,
>> +static void bcm_sysport_init_dim(struct bcm_sysport_priv *priv,
>>                    void (*cb)(struct work_struct *work))
>>   {
>> +    struct bcm_sysport_net_dim *dim = &priv->dim;
>> +    struct net_dim_cq_moder moder;
>> +    u32 usecs, pkts;
>> +
>>       INIT_WORK(&dim->dim.work, cb);
>>       dim->dim.mode = NET_DIM_CQ_PERIOD_MODE_START_FROM_EQE;
>>       dim->event_ctr = 0;
>>       dim->packets = 0;
>>       dim->bytes = 0;
> 
> I would expect only dim related code in a function called init_dim(). I
> think the code below should be elsewhere. Or maybe change the function
> name to init_rx_coalesce().

Sure, I can definitively do split the initialization, that makes sense.
-- 
Florian

^ permalink raw reply

* Re: [PATCH] gfs2: Stop using rhashtable_walk_peek
From: NeilBrown @ 2018-03-28 21:53 UTC (permalink / raw)
  To: Andreas Gruenbacher, cluster-devel
  Cc: netdev, linux-kernel, Thomas Graf, Herbert Xu, Tom Herbert
In-Reply-To: <20180328160009.29633-1-agruenba@redhat.com>

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

On Wed, Mar 28 2018, Andreas Gruenbacher wrote:

> Function rhashtable_walk_peek is problematic because there is no
> guarantee that the glock previously returned still exists; when that key
> is deleted, rhashtable_walk_peek can end up returning a different key,
> which would cause an inconsistent glock dump.  So instead of using
> rhashtable_walk_peek, keep track of the current glock in the seq file
> iterator functions.
>
> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
> ---
>  fs/gfs2/glock.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
> index 82fb5583445c..f1fc353875d3 100644
> --- a/fs/gfs2/glock.c
> +++ b/fs/gfs2/glock.c
> @@ -55,6 +55,7 @@ struct gfs2_glock_iter {
>  	struct gfs2_sbd *sdp;		/* incore superblock           */
>  	struct rhashtable_iter hti;	/* rhashtable iterator         */
>  	struct gfs2_glock *gl;		/* current glock struct        */
> +	bool gl_held;
>  	loff_t last_pos;		/* last position               */
>  };
>  
> @@ -1923,9 +1924,11 @@ void gfs2_glock_exit(void)
>  
>  static void gfs2_glock_iter_next(struct gfs2_glock_iter *gi, loff_t n)
>  {
> -	if (n == 0)
> -		gi->gl = rhashtable_walk_peek(&gi->hti);
> -	else {
> +	if (n != 0 || !gi->gl) {
> +		if (gi->gl_held) {
> +			gfs2_glock_queue_put(gi->gl);
> +			gi->gl_held = false;
> +		}
>  		gi->gl = rhashtable_walk_next(&gi->hti);
>  		n--;
>  	}

Thank for this patch!
The above looks a bit fragile to me.
gfs2_glock_iter_next() (And hence gfs2_glock_seq_start()) will sometimes
exit with gl_held true, and sometimes with it false.
gfs2_glock_seq_stop() assumes that it is false.
Normally gfs2_glock_seq_next() will normally be called between these
two and will clear gl_held, but I don't think there is a hard guarantee
of that.
Maybe we should always 'put' gi->gl in iter_next if gl_held??

Thanks,
NeilBrown



> @@ -1988,7 +1991,10 @@ static void gfs2_glock_seq_stop(struct seq_file *seq, void *iter_ptr)
>  {
>  	struct gfs2_glock_iter *gi = seq->private;
>  
> -	gi->gl = NULL;
> +	if (gi->gl) {
> +		lockref_get(&gi->gl->gl_lockref);
> +		gi->gl_held = true;
> +	}
>  	rhashtable_walk_stop(&gi->hti);
>  }
>  
> @@ -2061,6 +2067,7 @@ static int __gfs2_glocks_open(struct inode *inode, struct file *file,
>  		 */
>  		gi->last_pos = -1;
>  		gi->gl = NULL;
> +		gi->gl_held = false;
>  		rhashtable_walk_enter(&gl_hash_table, &gi->hti);
>  	}
>  	return ret;
> @@ -2076,7 +2083,8 @@ static int gfs2_glocks_release(struct inode *inode, struct file *file)
>  	struct seq_file *seq = file->private_data;
>  	struct gfs2_glock_iter *gi = seq->private;
>  
> -	gi->gl = NULL;
> +	if (gi->gl_held)
> +		gfs2_glock_put(gi->gl);
>  	rhashtable_walk_exit(&gi->hti);
>  	return seq_release_private(inode, file);
>  }
> -- 
> 2.14.3

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

^ permalink raw reply

* [PATCH v2 net-next 0/2] Add ATU/VTU statistics
From: Andrew Lunn @ 2018-03-28 21:50 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Florian Fainelli, Andrew Lunn

Previous patches have added basic support for Address Translation Unit
and VLAN translation Unit violation interrupts. Add statistics
counters for when these occur, which can be accessed using
ethtool. Downgrade one of the particularly spammy warnings from VTU
violations to debug only, now that we have a counter for it.

Andrew Lunn (2):
  net: dsa: mv88e6xxx: Keep ATU/VTU violation statistics
  net: dsa: mv88e6xxx: Make VTU miss violations less spammy

 drivers/net/dsa/mv88e6xxx/chip.c        | 50 ++++++++++++++++++++++++++++-----
 drivers/net/dsa/mv88e6xxx/chip.h        | 13 ++++++---
 drivers/net/dsa/mv88e6xxx/global1_atu.c | 12 +++++---
 drivers/net/dsa/mv88e6xxx/global1_vtu.c |  8 +++---
 drivers/net/dsa/mv88e6xxx/serdes.c      | 15 ++++++----
 drivers/net/dsa/mv88e6xxx/serdes.h      |  8 +++---
 6 files changed, 77 insertions(+), 29 deletions(-)

-- 
2.16.2

^ permalink raw reply

* [PATCH v2 net-next 2/2] net: dsa: mv88e6xxx: Make VTU miss violations less spammy
From: Andrew Lunn @ 2018-03-28 21:50 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Florian Fainelli, Andrew Lunn
In-Reply-To: <1522273829-23542-1-git-send-email-andrew@lunn.ch>

VTU miss violations can happen under normal conditions. Don't spam the
kernel log, downgrade the output to debug level only. The statistics
counter will indicate it is happening, if anybody not debugging is
interested.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
v2: Rather than delete, use dev_dbg_ratelimited()
---
 drivers/net/dsa/mv88e6xxx/global1_vtu.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/mv88e6xxx/global1_vtu.c b/drivers/net/dsa/mv88e6xxx/global1_vtu.c
index 2cbaf946e7ed..058326924f3e 100644
--- a/drivers/net/dsa/mv88e6xxx/global1_vtu.c
+++ b/drivers/net/dsa/mv88e6xxx/global1_vtu.c
@@ -548,10 +548,11 @@ static irqreturn_t mv88e6xxx_g1_vtu_prob_irq_thread_fn(int irq, void *dev_id)
 	}
 
 	if (val & MV88E6XXX_G1_VTU_OP_MISS_VIOLATION) {
-		dev_err_ratelimited(chip->dev, "VTU miss violation for vid %d, source port %d\n",
+		dev_dbg_ratelimited(chip->dev, "VTU miss violation for vid %d, source port %d\n",
 				    entry.vid, spid);
 		chip->ports[spid].vtu_miss_violation++;
 	}
+
 	mutex_unlock(&chip->reg_lock);
 
 	return IRQ_HANDLED;
-- 
2.16.2

^ permalink raw reply related

* [PATCH v2 net-next 1/2] net: dsa: mv88e6xxx: Keep ATU/VTU violation statistics
From: Andrew Lunn @ 2018-03-28 21:50 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Florian Fainelli, Andrew Lunn
In-Reply-To: <1522273829-23542-1-git-send-email-andrew@lunn.ch>

Count the numbers of various ATU and VTU violation statistics and
return them as part of the ethtool -S statistics.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
v2: int -> unsigned int
---
 drivers/net/dsa/mv88e6xxx/chip.c        | 50 ++++++++++++++++++++++++++++-----
 drivers/net/dsa/mv88e6xxx/chip.h        | 13 ++++++---
 drivers/net/dsa/mv88e6xxx/global1_atu.c | 12 +++++---
 drivers/net/dsa/mv88e6xxx/global1_vtu.c |  8 ++++--
 drivers/net/dsa/mv88e6xxx/serdes.c      | 15 ++++++----
 drivers/net/dsa/mv88e6xxx/serdes.h      |  8 +++---
 6 files changed, 78 insertions(+), 28 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 9a5d786b4885..3d2091099f7f 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -723,6 +723,24 @@ static int mv88e6320_stats_get_strings(struct mv88e6xxx_chip *chip,
 					   STATS_TYPE_BANK0 | STATS_TYPE_BANK1);
 }
 
+static const uint8_t *mv88e6xxx_atu_vtu_stats_strings[] = {
+	"atu_member_violation",
+	"atu_miss_violation",
+	"atu_full_violation",
+	"vtu_member_violation",
+	"vtu_miss_violation",
+};
+
+static void mv88e6xxx_atu_vtu_get_strings(uint8_t *data)
+{
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(mv88e6xxx_atu_vtu_stats_strings); i++)
+		strlcpy(data + i * ETH_GSTRING_LEN,
+			mv88e6xxx_atu_vtu_stats_strings[i],
+			ETH_GSTRING_LEN);
+}
+
 static void mv88e6xxx_get_strings(struct dsa_switch *ds, int port,
 				  uint8_t *data)
 {
@@ -736,9 +754,12 @@ static void mv88e6xxx_get_strings(struct dsa_switch *ds, int port,
 
 	if (chip->info->ops->serdes_get_strings) {
 		data += count * ETH_GSTRING_LEN;
-		chip->info->ops->serdes_get_strings(chip, port, data);
+		count = chip->info->ops->serdes_get_strings(chip, port, data);
 	}
 
+	data += count * ETH_GSTRING_LEN;
+	mv88e6xxx_atu_vtu_get_strings(data);
+
 	mutex_unlock(&chip->reg_lock);
 }
 
@@ -783,10 +804,13 @@ static int mv88e6xxx_get_sset_count(struct dsa_switch *ds, int port)
 	if (chip->info->ops->serdes_get_sset_count)
 		serdes_count = chip->info->ops->serdes_get_sset_count(chip,
 								      port);
-	if (serdes_count < 0)
+	if (serdes_count < 0) {
 		count = serdes_count;
-	else
-		count += serdes_count;
+		goto out;
+	}
+	count += serdes_count;
+	count += ARRAY_SIZE(mv88e6xxx_atu_vtu_stats_strings);
+
 out:
 	mutex_unlock(&chip->reg_lock);
 
@@ -841,6 +865,16 @@ static int mv88e6390_stats_get_stats(struct mv88e6xxx_chip *chip, int port,
 					 0);
 }
 
+static void mv88e6xxx_atu_vtu_get_stats(struct mv88e6xxx_chip *chip, int port,
+					uint64_t *data)
+{
+	*data++ = chip->ports[port].atu_member_violation;
+	*data++ = chip->ports[port].atu_miss_violation;
+	*data++ = chip->ports[port].atu_full_violation;
+	*data++ = chip->ports[port].vtu_member_violation;
+	*data++ = chip->ports[port].vtu_miss_violation;
+}
+
 static void mv88e6xxx_get_stats(struct mv88e6xxx_chip *chip, int port,
 				uint64_t *data)
 {
@@ -849,12 +883,14 @@ static void mv88e6xxx_get_stats(struct mv88e6xxx_chip *chip, int port,
 	if (chip->info->ops->stats_get_stats)
 		count = chip->info->ops->stats_get_stats(chip, port, data);
 
+	mutex_lock(&chip->reg_lock);
 	if (chip->info->ops->serdes_get_stats) {
 		data += count;
-		mutex_lock(&chip->reg_lock);
-		chip->info->ops->serdes_get_stats(chip, port, data);
-		mutex_unlock(&chip->reg_lock);
+		count = chip->info->ops->serdes_get_stats(chip, port, data);
 	}
+	data += count;
+	mv88e6xxx_atu_vtu_get_stats(chip, port, data);
+	mutex_unlock(&chip->reg_lock);
 }
 
 static void mv88e6xxx_get_ethtool_stats(struct dsa_switch *ds, int port,
diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
index bad211014e91..80490f66bc06 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.h
+++ b/drivers/net/dsa/mv88e6xxx/chip.h
@@ -194,6 +194,11 @@ struct mv88e6xxx_port_hwtstamp {
 
 struct mv88e6xxx_port {
 	u64 serdes_stats[2];
+	u64 atu_member_violation;
+	u64 atu_miss_violation;
+	u64 atu_full_violation;
+	u64 vtu_member_violation;
+	u64 vtu_miss_violation;
 };
 
 struct mv88e6xxx_chip {
@@ -409,10 +414,10 @@ struct mv88e6xxx_ops {
 
 	/* Statistics from the SERDES interface */
 	int (*serdes_get_sset_count)(struct mv88e6xxx_chip *chip, int port);
-	void (*serdes_get_strings)(struct mv88e6xxx_chip *chip,  int port,
-				   uint8_t *data);
-	void (*serdes_get_stats)(struct mv88e6xxx_chip *chip,  int port,
-				 uint64_t *data);
+	int (*serdes_get_strings)(struct mv88e6xxx_chip *chip,  int port,
+				  uint8_t *data);
+	int (*serdes_get_stats)(struct mv88e6xxx_chip *chip,  int port,
+				uint64_t *data);
 
 	/* VLAN Translation Unit operations */
 	int (*vtu_getnext)(struct mv88e6xxx_chip *chip,
diff --git a/drivers/net/dsa/mv88e6xxx/global1_atu.c b/drivers/net/dsa/mv88e6xxx/global1_atu.c
index 20d941f4273b..307410898fc9 100644
--- a/drivers/net/dsa/mv88e6xxx/global1_atu.c
+++ b/drivers/net/dsa/mv88e6xxx/global1_atu.c
@@ -336,8 +336,6 @@ static irqreturn_t mv88e6xxx_g1_atu_prob_irq_thread_fn(int irq, void *dev_id)
 	if (err)
 		goto out;
 
-	mutex_unlock(&chip->reg_lock);
-
 	if (val & MV88E6XXX_G1_ATU_OP_AGE_OUT_VIOLATION) {
 		dev_err_ratelimited(chip->dev,
 				    "ATU age out violation for %pM\n",
@@ -348,17 +346,23 @@ static irqreturn_t mv88e6xxx_g1_atu_prob_irq_thread_fn(int irq, void *dev_id)
 		dev_err_ratelimited(chip->dev,
 				    "ATU member violation for %pM portvec %x\n",
 				    entry.mac, entry.portvec);
+		chip->ports[entry.portvec].atu_member_violation++;
 	}
 
-	if (val & MV88E6XXX_G1_ATU_OP_MEMBER_VIOLATION)
+	if (val & MV88E6XXX_G1_ATU_OP_MEMBER_VIOLATION) {
 		dev_err_ratelimited(chip->dev,
 				    "ATU miss violation for %pM portvec %x\n",
 				    entry.mac, entry.portvec);
+		chip->ports[entry.portvec].atu_miss_violation++;
+	}
 
-	if (val & MV88E6XXX_G1_ATU_OP_FULL_VIOLATION)
+	if (val & MV88E6XXX_G1_ATU_OP_FULL_VIOLATION) {
 		dev_err_ratelimited(chip->dev,
 				    "ATU full violation for %pM portvec %x\n",
 				    entry.mac, entry.portvec);
+		chip->ports[entry.portvec].atu_full_violation++;
+	}
+	mutex_unlock(&chip->reg_lock);
 
 	return IRQ_HANDLED;
 
diff --git a/drivers/net/dsa/mv88e6xxx/global1_vtu.c b/drivers/net/dsa/mv88e6xxx/global1_vtu.c
index 7997961647de..2cbaf946e7ed 100644
--- a/drivers/net/dsa/mv88e6xxx/global1_vtu.c
+++ b/drivers/net/dsa/mv88e6xxx/global1_vtu.c
@@ -539,18 +539,20 @@ static irqreturn_t mv88e6xxx_g1_vtu_prob_irq_thread_fn(int irq, void *dev_id)
 	if (err)
 		goto out;
 
-	mutex_unlock(&chip->reg_lock);
-
 	spid = val & MV88E6XXX_G1_VTU_OP_SPID_MASK;
 
 	if (val & MV88E6XXX_G1_VTU_OP_MEMBER_VIOLATION) {
 		dev_err_ratelimited(chip->dev, "VTU member violation for vid %d, source port %d\n",
 				    entry.vid, spid);
+		chip->ports[spid].vtu_member_violation++;
 	}
 
-	if (val & MV88E6XXX_G1_VTU_OP_MISS_VIOLATION)
+	if (val & MV88E6XXX_G1_VTU_OP_MISS_VIOLATION) {
 		dev_err_ratelimited(chip->dev, "VTU miss violation for vid %d, source port %d\n",
 				    entry.vid, spid);
+		chip->ports[spid].vtu_miss_violation++;
+	}
+	mutex_unlock(&chip->reg_lock);
 
 	return IRQ_HANDLED;
 
diff --git a/drivers/net/dsa/mv88e6xxx/serdes.c b/drivers/net/dsa/mv88e6xxx/serdes.c
index b6166424216a..fb058fd35c0d 100644
--- a/drivers/net/dsa/mv88e6xxx/serdes.c
+++ b/drivers/net/dsa/mv88e6xxx/serdes.c
@@ -106,20 +106,21 @@ int mv88e6352_serdes_get_sset_count(struct mv88e6xxx_chip *chip, int port)
 	return 0;
 }
 
-void mv88e6352_serdes_get_strings(struct mv88e6xxx_chip *chip,
-				  int port, uint8_t *data)
+int mv88e6352_serdes_get_strings(struct mv88e6xxx_chip *chip,
+				 int port, uint8_t *data)
 {
 	struct mv88e6352_serdes_hw_stat *stat;
 	int i;
 
 	if (!mv88e6352_port_has_serdes(chip, port))
-		return;
+		return 0;
 
 	for (i = 0; i < ARRAY_SIZE(mv88e6352_serdes_hw_stats); i++) {
 		stat = &mv88e6352_serdes_hw_stats[i];
 		memcpy(data + i * ETH_GSTRING_LEN, stat->string,
 		       ETH_GSTRING_LEN);
 	}
+	return ARRAY_SIZE(mv88e6352_serdes_hw_stats);
 }
 
 static uint64_t mv88e6352_serdes_get_stat(struct mv88e6xxx_chip *chip,
@@ -149,8 +150,8 @@ static uint64_t mv88e6352_serdes_get_stat(struct mv88e6xxx_chip *chip,
 	return val;
 }
 
-void mv88e6352_serdes_get_stats(struct mv88e6xxx_chip *chip, int port,
-				uint64_t *data)
+int mv88e6352_serdes_get_stats(struct mv88e6xxx_chip *chip, int port,
+			       uint64_t *data)
 {
 	struct mv88e6xxx_port *mv88e6xxx_port = &chip->ports[port];
 	struct mv88e6352_serdes_hw_stat *stat;
@@ -158,7 +159,7 @@ void mv88e6352_serdes_get_stats(struct mv88e6xxx_chip *chip, int port,
 	int i;
 
 	if (!mv88e6352_port_has_serdes(chip, port))
-		return;
+		return 0;
 
 	BUILD_BUG_ON(ARRAY_SIZE(mv88e6352_serdes_hw_stats) >
 		     ARRAY_SIZE(mv88e6xxx_port->serdes_stats));
@@ -169,6 +170,8 @@ void mv88e6352_serdes_get_stats(struct mv88e6xxx_chip *chip, int port,
 		mv88e6xxx_port->serdes_stats[i] += value;
 		data[i] = mv88e6xxx_port->serdes_stats[i];
 	}
+
+	return ARRAY_SIZE(mv88e6352_serdes_hw_stats);
 }
 
 /* Set the power on/off for 10GBASE-R and 10GBASE-X4/X2 */
diff --git a/drivers/net/dsa/mv88e6xxx/serdes.h b/drivers/net/dsa/mv88e6xxx/serdes.h
index 641baa75f910..1897c01c6e19 100644
--- a/drivers/net/dsa/mv88e6xxx/serdes.h
+++ b/drivers/net/dsa/mv88e6xxx/serdes.h
@@ -45,8 +45,8 @@
 int mv88e6352_serdes_power(struct mv88e6xxx_chip *chip, int port, bool on);
 int mv88e6390_serdes_power(struct mv88e6xxx_chip *chip, int port, bool on);
 int mv88e6352_serdes_get_sset_count(struct mv88e6xxx_chip *chip, int port);
-void mv88e6352_serdes_get_strings(struct mv88e6xxx_chip *chip,
-				  int port, uint8_t *data);
-void mv88e6352_serdes_get_stats(struct mv88e6xxx_chip *chip, int port,
-				uint64_t *data);
+int mv88e6352_serdes_get_strings(struct mv88e6xxx_chip *chip,
+				 int port, uint8_t *data);
+int mv88e6352_serdes_get_stats(struct mv88e6xxx_chip *chip, int port,
+			       uint64_t *data);
 #endif
-- 
2.16.2

^ permalink raw reply related


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