* Re: [PATCH bpf-next] bpf/verifier: enable ctx + const + 0.
From: William Tu @ 2018-05-02 17:54 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Alexei Starovoitov, Linux Kernel Network Developers,
Yonghong Song, Yifeng Sun
In-Reply-To: <e4f93f62-d7f4-0851-d1c2-34a13c2af4c6@iogearbox.net>
On Wed, May 2, 2018 at 1:29 AM, Daniel Borkmann <daniel@iogearbox.net> wrote:
> On 05/02/2018 06:52 AM, Alexei Starovoitov wrote:
>> On Tue, May 01, 2018 at 09:35:29PM -0700, William Tu wrote:
>>>
>>>> How did you test this patch?
>>>>
>>> Without the patch, the test case will fail.
>>> With the patch, the test case passes.
>>
>> Please test it with real program and you'll see crashes and garbage returned.
>
> +1, *convert_ctx_access() use bpf_insn's off to determine what to rewrite,
> so this is definitely buggy, and wasn't properly tested as it should have
> been. The test case is also way too simple, just the LDX and then doing a
> return 0 will get you past verifier, but won't give you anything in terms
> of runtime testing that test_verifier is doing. A single test case for a
> non trivial verifier change like this is also _completely insufficient_,
> this really needs to test all sort of weird corner cases (involving out of
> bounds accesses, overflows, etc).
Thanks, now I understand.
It's much more complicated than I thought.
William
^ permalink raw reply
* INFO: rcu detected stall in __schedule
From: syzbot @ 2018-05-02 17:56 UTC (permalink / raw)
To: linux-kernel, linux-ppp, netdev, paulus, syzkaller-bugs
Hello,
syzbot found the following crash on:
HEAD commit: f2125992e7cb Merge tag 'xfs-4.17-fixes-1' of
git://git.kern...
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?id=4755940087693312
kernel config:
https://syzkaller.appspot.com/x/.config?id=6493557782959164711
dashboard link: https://syzkaller.appspot.com/bug?extid=f16b3e3512a1e3c1d1f6
compiler: gcc (GCC) 8.0.1 20180413 (experimental)
Unfortunately, I don't have any reproducer for this crash yet.
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+f16b3e3512a1e3c1d1f6@syzkaller.appspotmail.com
do_dccp_getsockopt: sockopt(PACKET_SIZE) is deprecated: fix your app
do_dccp_getsockopt: sockopt(PACKET_SIZE) is deprecated: fix your app
ntfs: (device loop6): parse_options(): Unrecognized mount option
error�n��uldip.
INFO: rcu_sched self-detected stall on CPU
0-...!: (125000 ticks this GP) idle=f3e/1/4611686018427387906
softirq=112858/112858 fqs=0
(t=125000 jiffies g=61626 c=61625 q=1534)
rcu_sched kthread starved for 125000 jiffies! g61626 c61625 f0x0
RCU_GP_WAIT_FQS(3) ->state=0x402 ->cpu=0
RCU grace-period kthread stack dump:
rcu_sched I23592 9 2 0x80000000
Call Trace:
context_switch kernel/sched/core.c:2848 [inline]
__schedule+0x801/0x1e30 kernel/sched/core.c:3490
schedule+0xef/0x430 kernel/sched/core.c:3549
schedule_timeout+0x138/0x240 kernel/time/timer.c:1801
rcu_gp_kthread+0x6b5/0x1940 kernel/rcu/tree.c:2231
kthread+0x345/0x410 kernel/kthread.c:238
ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:412
NMI backtrace for cpu 0
CPU: 0 PID: 26694 Comm: syz-executor1 Not tainted 4.17.0-rc3+ #28
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
<IRQ>
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x1b9/0x294 lib/dump_stack.c:113
nmi_cpu_backtrace.cold.4+0x19/0xce lib/nmi_backtrace.c:103
nmi_trigger_cpumask_backtrace+0x151/0x192 lib/nmi_backtrace.c:62
arch_trigger_cpumask_backtrace+0x14/0x20 arch/x86/kernel/apic/hw_nmi.c:38
trigger_single_cpu_backtrace include/linux/nmi.h:156 [inline]
rcu_dump_cpu_stacks+0x175/0x1c2 kernel/rcu/tree.c:1376
print_cpu_stall kernel/rcu/tree.c:1525 [inline]
check_cpu_stall.isra.61.cold.80+0x36c/0x59a kernel/rcu/tree.c:1593
__rcu_pending kernel/rcu/tree.c:3356 [inline]
rcu_pending kernel/rcu/tree.c:3401 [inline]
rcu_check_callbacks+0x21b/0xad0 kernel/rcu/tree.c:2763
update_process_times+0x2d/0x70 kernel/time/timer.c:1636
tick_sched_handle+0x9f/0x180 kernel/time/tick-sched.c:164
tick_sched_timer+0x45/0x130 kernel/time/tick-sched.c:1274
__run_hrtimer kernel/time/hrtimer.c:1398 [inline]
__hrtimer_run_queues+0x3e3/0x10a0 kernel/time/hrtimer.c:1460
hrtimer_interrupt+0x2f3/0x750 kernel/time/hrtimer.c:1518
local_apic_timer_interrupt arch/x86/kernel/apic/apic.c:1025 [inline]
smp_apic_timer_interrupt+0x15d/0x710 arch/x86/kernel/apic/apic.c:1050
apic_timer_interrupt+0xf/0x20 arch/x86/entry/entry_64.S:863
</IRQ>
RIP: 0010:arch_local_irq_enable arch/x86/include/asm/paravirt.h:793 [inline]
RIP: 0010:__raw_spin_unlock_irq include/linux/spinlock_api_smp.h:168
[inline]
RIP: 0010:_raw_spin_unlock_irq+0x56/0x70 kernel/locking/spinlock.c:192
RSP: 0018:ffff8801b3dbf438 EFLAGS: 00000282 ORIG_RAX: ffffffffffffff13
RAX: dffffc0000000000 RBX: ffff8801dae2c680 RCX: 1ffff100361ebd4d
RDX: 1ffffffff11a316f RSI: ffff8801b0f5ea48 RDI: ffffffff88d18b78
RBP: ffff8801b3dbf440 R08: ffff8801b0f5e9f8 R09: 0000000000000006
R10: ffff8801b0f5e1c0 R11: 0000000000000000 R12: ffff8801b0f5e1c0
R13: ffff8801b0f5e7a0 R14: dffffc0000000000 R15: ffff8801b0f5e1c0
rq_unlock_irq kernel/sched/sched.h:1824 [inline]
__schedule+0x144f/0x1e30 kernel/sched/core.c:3493
schedule+0xef/0x430 kernel/sched/core.c:3549
do_sched_yield+0x187/0x240 kernel/sched/core.c:4965
yield+0xa5/0xe0 kernel/sched/core.c:5054
tasklet_kill+0x4e/0xd0 kernel/softirq.c:559
ppp_asynctty_close+0x9e/0x150 drivers/net/ppp/ppp_async.c:239
ppp_asynctty_hangup+0x15/0x20 drivers/net/ppp/ppp_async.c:256
tty_ldisc_hangup+0x138/0x640 drivers/tty/tty_ldisc.c:730
__tty_hangup.part.21+0x2da/0x6e0 drivers/tty/tty_io.c:621
__tty_hangup drivers/tty/tty_io.c:571 [inline]
tty_vhangup+0x21/0x30 drivers/tty/tty_io.c:694
pty_close+0x3bd/0x510 drivers/tty/pty.c:78
tty_release+0x494/0x12e0 drivers/tty/tty_io.c:1656
__fput+0x34d/0x890 fs/file_table.c:209
____fput+0x15/0x20 fs/file_table.c:243
task_work_run+0x1e4/0x290 kernel/task_work.c:113
tracehook_notify_resume include/linux/tracehook.h:191 [inline]
exit_to_usermode_loop+0x2bd/0x310 arch/x86/entry/common.c:166
prepare_exit_to_usermode arch/x86/entry/common.c:196 [inline]
syscall_return_slowpath arch/x86/entry/common.c:265 [inline]
do_syscall_64+0x6ac/0x800 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x455979
RSP: 002b:00007f92c3751c68 EFLAGS: 00000246 ORIG_RAX: 0000000000000003
RAX: 0000000000000000 RBX: 00007f92c37526d4 RCX: 0000000000455979
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000013
RBP: 000000000072bf50 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00000000ffffffff
R13: 0000000000000053 R14: 00000000006f4868 R15: 0000000000000001
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this bug report.
If you forgot to add the Reported-by tag, once the fix for this bug is
merged
into any tree, please reply to this email with:
#syz fix: exact-commit-title
To mark this as a duplicate of another syzbot report, please reply with:
#syz dup: exact-subject-of-another-report
If it's a one-off invalid bug report, please reply with:
#syz invalid
Note: if the crash happens again, it will cause creation of a new bug
report.
Note: all commands must start from beginning of the line in the email body.
^ permalink raw reply
* Re: [PATCH bpf-next 1/2] bpf: enable stackmap with build_id in nmi context
From: Song Liu @ 2018-05-02 17:57 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Networking, Kernel Team, Alexei Starovoitov, Daniel Borkmann
In-Reply-To: <20180502173027.GM12180@hirez.programming.kicks-ass.net>
> On May 2, 2018, at 10:30 AM, Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Wed, May 02, 2018 at 04:48:32PM +0000, Song Liu wrote:
>>> It's broken though, I've bet you've never actually ran this with lockdep
>>> enabled for example.
>>
>> I am not following here. I just run the new selftest with CONFIG_LOCKDEP on,
>> and got no warning for this.
>
> Weird, I would be expecting complaints about releasing an unheld lock.
>
> nmi_enter(),nmi_exit() have lockdep_off(),lockdep_on() resp. Which means
> that the down_trylock() will not be recorded. The up, which is done from
> IRQ context, will not be so supressed and should hit
> print_unlock_imbalance_bug().
>
I am still not sure whether I am following. I guess your concern apply to
spinlock only? lock_acquire() has the following in the beginning:
if (unlikely(current->lockdep_recursion))
return;
So it will not run in nmi context?
On the other hand, semaphore and rw_semaphore should be ok in such cases?
Thanks,
Song
^ permalink raw reply
* [PATCH bpf 0/2] Two x86 BPF JIT fixes
From: Daniel Borkmann @ 2018-05-02 18:12 UTC (permalink / raw)
To: ast; +Cc: netdev, Daniel Borkmann
Fix two memory leaks in x86 JIT. For details, please see
individual patches in this series. Thanks!
Daniel Borkmann (2):
bpf, x64: fix memleak when not converging after image
bpf, x64: fix memleak when not converging on calls
arch/x86/net/bpf_jit_comp.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
--
2.9.5
^ permalink raw reply
* [PATCH bpf 1/2] bpf, x64: fix memleak when not converging after image
From: Daniel Borkmann @ 2018-05-02 18:12 UTC (permalink / raw)
To: ast; +Cc: netdev, Daniel Borkmann
In-Reply-To: <20180502181223.30613-1-daniel@iogearbox.net>
While reviewing x64 JIT code, I noticed that we leak the prior allocated
JIT image in the case where proglen != oldproglen during the JIT passes.
Prior to the commit e0ee9c12157d ("x86: bpf_jit: fix two bugs in eBPF JIT
compiler") we would just break out of the loop, and using the image as the
JITed prog since it could only shrink in size anyway. After e0ee9c12157d,
we would bail out to out_addrs label where we free addrs and jit_data but
not the image coming from bpf_jit_binary_alloc().
Fixes: e0ee9c12157d ("x86: bpf_jit: fix two bugs in eBPF JIT compiler")
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
arch/x86/net/bpf_jit_comp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index abce27c..9ae7b93 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -1236,6 +1236,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
for (pass = 0; pass < 20 || image; pass++) {
proglen = do_jit(prog, addrs, image, oldproglen, &ctx);
if (proglen <= 0) {
+out_image:
image = NULL;
if (header)
bpf_jit_binary_free(header);
@@ -1246,8 +1247,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
if (proglen != oldproglen) {
pr_err("bpf_jit: proglen=%d != oldproglen=%d\n",
proglen, oldproglen);
- prog = orig_prog;
- goto out_addrs;
+ goto out_image;
}
break;
}
--
2.9.5
^ permalink raw reply related
* [PATCH bpf 2/2] bpf, x64: fix memleak when not converging on calls
From: Daniel Borkmann @ 2018-05-02 18:12 UTC (permalink / raw)
To: ast; +Cc: netdev, Daniel Borkmann
In-Reply-To: <20180502181223.30613-1-daniel@iogearbox.net>
The JIT logic in jit_subprogs() is as follows: for all subprogs we
allocate a bpf_prog_alloc(), populate it (prog->is_func = 1 here),
and pass it to bpf_int_jit_compile(). If a failure occurred during
JIT and prog->jited is not set, then we bail out from attempting to
JIT the whole program, and punt to the interpreter instead. In case
JITing went successful, we fixup BPF call offsets and do another
pass to bpf_int_jit_compile() (extra_pass is true at that point) to
complete JITing calls. Given that requires to pass JIT context around
addrs and jit_data from x86 JIT are freed in the extra_pass in
bpf_int_jit_compile() when calls are involved (if not, they can
be freed immediately). However, if in the original pass, the JIT
image didn't converge then we leak addrs and jit_data since image
itself is NULL, the prog->is_func is set and extra_pass is false
in that case, meaning both will become unreachable and are never
cleaned up, therefore we need to free as well on !image. Only x64
JIT is affected.
Fixes: 1c2a088a6626 ("bpf: x64: add JIT support for multi-function programs")
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
arch/x86/net/bpf_jit_comp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 9ae7b93..263c845 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -1283,7 +1283,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
prog = orig_prog;
}
- if (!prog->is_func || extra_pass) {
+ if (!image || !prog->is_func || extra_pass) {
out_addrs:
kfree(addrs);
kfree(jit_data);
--
2.9.5
^ permalink raw reply related
* Re: [PATCH net-next v9 0/4] Prerequisites for Cavium OCTEON-III network driver.
From: Steven J. Hill @ 2018-05-02 18:19 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20180429.203347.679522174405836744.davem@davemloft.net>
On 04/29/2018 07:33 PM, David Miller wrote:
>
> I don't know if we really want all of these MIPS specific changes to
> go via the net-next tree.
>
> The right way to do this is probably getting this series into the MIPS
> architecture tree.
>
David,
Correct, and I should have been clearer about that. The MIPS-specific
changes are targeted for 4.18 and I will be working with James/Ralf
to get those in. The actual changes for adding the driver, however,
should be fine to go into net-next. The netdev list should have been
a CC: for those patches and the Linux/MIPS mailing list as the main
recipient. Mea culpa.
Steve
^ permalink raw reply
* Re: [PATCH net-next] net: phy: broadcom: add support for BCM89610 PHY
From: Bhadram Varka @ 2018-05-02 18:47 UTC (permalink / raw)
To: David Miller; +Cc: andrew, f.fainelli, netdev
In-Reply-To: <20180502.132136.1844584450359331860.davem@davemloft.net>
HI David,
On 5/2/2018 10:51 PM, David Miller wrote:
>
> Please remove the email footer from your postings here that talks about
> confidential information and whatnot.
>
> That is expressly inappropriate for this mailing list, and any such
> postings shall be ignored in their entirety.
>
I Understand this. Fixed my e-mail client to address this.
Sorry for the noise.
Thanks,
Bhadram.
^ permalink raw reply
* Re: [bpf PATCH 1/3] bpf: sockmap, fix scatterlist update on error path in send with apply
From: David Miller @ 2018-05-02 18:51 UTC (permalink / raw)
To: john.fastabend; +Cc: borkmann, ast, netdev
In-Reply-To: <20180502174727.17875.95306.stgit@john-Precision-Tower-5810>
From: John Fastabend <john.fastabend@gmail.com>
Date: Wed, 02 May 2018 10:47:27 -0700
> When the call to do_tcp_sendpage() fails to send the complete block
> requested we either retry if only a partial send was completed or
> abort if we receive a error less than or equal to zero. Before
> returning though we must update the scatterlist length/offset to
> account for any partial send completed.
>
> Before this patch we did this at the end of the retry loop, but
> this was buggy when used while applying a verdict to fewer bytes
> than in the scatterlist. When the scatterlist length was being set
> we forgot to account for the apply logic reducing the size variable.
> So the result was we chopped off some bytes in the scatterlist without
> doing proper cleanup on them. This results in a WARNING when the
> sock is tore down because the bytes have previously been charged to
> the socket but are never uncharged.
>
> The simple fix is to simply do the accounting inside the retry loop
> subtracting from the absolute scatterlist values rather than trying
> to accumulate the totals and subtract at the end.
>
> Reported-by: Alexei Starovoitov <ast@kernel.org>
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
Acked-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* Re: [bpf PATCH 2/3] bpf: sockmap, zero sg_size on error when buffer is released
From: David Miller @ 2018-05-02 18:51 UTC (permalink / raw)
To: john.fastabend; +Cc: borkmann, ast, netdev
In-Reply-To: <20180502174732.17875.75344.stgit@john-Precision-Tower-5810>
From: John Fastabend <john.fastabend@gmail.com>
Date: Wed, 02 May 2018 10:47:32 -0700
> When an error occurs during a redirect we have two cases that need
> to be handled (i) we have a cork'ed buffer (ii) we have a normal
> sendmsg buffer.
>
> In the cork'ed buffer case we don't currently support recovering from
> errors in a redirect action. So the buffer is released and the error
> should _not_ be pushed back to the caller of sendmsg/sendpage. The
> rationale here is the user will get an error that relates to old
> data that may have been sent by some arbitrary thread on that sock.
> Instead we simple consume the data and tell the user that the data
> has been consumed. We may add proper error recovery in the future.
> However, this patch fixes a bug where the bytes outstanding counter
> sg_size was not zeroed. This could result in a case where if the user
> has both a cork'ed action and apply action in progress we may
> incorrectly call into the BPF program when the user expected an
> old verdict to be applied via the apply action. I don't have a use
> case where using apply and cork at the same time is valid but we
> never explicitly reject it because it should work fine. This patch
> ensures the sg_size is zeroed so we don't have this case.
>
> In the normal sendmsg buffer case (no cork data) we also do not
> zero sg_size. Again this can confuse the apply logic when the logic
> calls into the BPF program when the BPF programmer expected the old
> verdict to remain. So ensure we set sg_size to zero here as well. And
> additionally to keep the psock state in-sync with the sk_msg_buff
> release all the memory as well. Previously we did this before
> returning to the user but this left a gap where psock and sk_msg_buff
> states were out of sync which seems fragile. No additional overhead
> is taken here except for a call to check the length and realize its
> already been freed. This is in the error path as well so in my
> opinion lets have robust code over optimized error paths.
>
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
Acked-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* Re: [bpf PATCH 3/3] bpf: sockmap, fix error handling in redirect failures
From: David Miller @ 2018-05-02 18:51 UTC (permalink / raw)
To: john.fastabend; +Cc: borkmann, ast, netdev
In-Reply-To: <20180502174737.17875.74031.stgit@john-Precision-Tower-5810>
From: John Fastabend <john.fastabend@gmail.com>
Date: Wed, 02 May 2018 10:47:37 -0700
> When a redirect failure happens we release the buffers in-flight
> without calling a sk_mem_uncharge(), the uncharge is called before
> dropping the sock lock for the redirecte, however we missed updating
> the ring start index. When no apply actions are in progress this
> is OK because we uncharge the entire buffer before the redirect.
> But, when we have apply logic running its possible that only a
> portion of the buffer is being redirected. In this case we only
> do memory accounting for the buffer slice being redirected and
> expect to be able to loop over the BPF program again and/or if
> a sock is closed uncharge the memory at sock destruct time.
>
> With an invalid start index however the program logic looks at
> the start pointer index, checks the length, and when seeing the
> length is zero (from the initial release and failure to update
> the pointer) aborts without uncharging/releasing the remaining
> memory.
>
> The fix for this is simply to update the start index. To avoid
> fixing this error in two locations we do a small refactor and
> remove one case where it is open-coded. Then fix it in the
> single function.
>
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
Acked-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* Re: [PATCH net-next 1/4] ipv6: Calculate hash thresholds for IPv6 nexthops
From: Ido Schimmel @ 2018-05-02 18:53 UTC (permalink / raw)
To: Eric Dumazet, Thomas.Winter
Cc: David Ahern, Ido Schimmel, netdev, davem, roopa, nikolay, pch,
jkbs, yoshfuji, mlxsw
In-Reply-To: <20180502175244.GA14587@splinter>
On Wed, May 02, 2018 at 08:52:44PM +0300, Ido Schimmel wrote:
> On Wed, May 02, 2018 at 08:21:06PM +0300, Ido Schimmel wrote:
> > On Wed, May 02, 2018 at 09:43:50AM -0700, Eric Dumazet wrote:
> > >
> > >
> > > On 01/09/2018 07:43 PM, David Ahern wrote:
> > > > On 1/9/18 7:40 AM, Ido Schimmel wrote:
> > > >> Before we convert IPv6 to use hash-threshold instead of modulo-N, we
> > > >> first need each nexthop to store its region boundary in the hash
> > > >> function's output space.
> > > >>
> > > >> The boundary is calculated by dividing the output space equally between
> > > >> the different active nexthops. That is, nexthops that are not dead or
> > > >> linkdown.
> > > >>
> > > >> The boundaries are rebalanced whenever a nexthop is added or removed to
> > > >> a multipath route and whenever a nexthop becomes active or inactive.
> > > >>
> > > >> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> > > >> ---
> > > >> include/net/ip6_fib.h | 1 +
> > > >> include/net/ip6_route.h | 7 ++++
> > > >> net/ipv6/ip6_fib.c | 8 ++---
> > > >> net/ipv6/route.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++++
> > > >> 4 files changed, 106 insertions(+), 6 deletions(-)
> > > >>
> > > >
> > > > LGTM.
> > > > Acked-by: David Ahern <dsahern@gmail.com>
> > > >
> > >
> > > For some reason I have a divide by zero error booting my hosts with latest net tree.
> > >
> > > What guarantee do we have that total is not zero when rt6_upper_bound_set() is called ?
> >
> > Thanks for the report, Eric. I believe I didn't cover all the cases and
> > 'rt6i_nh_weight' might be 0 is some cases. I'll try to reproduce and
> > work on a fix.
>
> Hmmm, I think it's due to commit edd7ceb78296 ("ipv6: Allow non-gateway
> ECMP for IPv6") which allows routes without a gateway (such as those
> configured using slaac) to have siblings.
>
> Can you please check if reverting the patch / applying the below fixes
> the issue?
So this fixes the issue for me. To reproduce:
# ip -6 address add 2001:db8::1/64 dev dummy0
# ip -6 address add 2001:db8::1/64 dev dummy1
This reproduces the issue because due to above commit both local routes
are considered siblings... :/
local 2001:db8::1 proto kernel metric 0
nexthop dev dummy0 weight 1
nexthop dev dummy1 weight 1 pref medium
I think it's best to revert the patch and have Thomas submit a fixed
version to net-next. I was actually surprised to see it applied to net.
>
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index f4d61736c41a..129dd4f4b264 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -3606,6 +3606,7 @@ struct rt6_info *addrconf_dst_alloc(struct inet6_dev *idev,
> rt->dst.input = ip6_input;
> rt->dst.output = ip6_output;
> rt->rt6i_idev = idev;
> + rt->rt6i_nh_weight = 1;
>
> rt->rt6i_protocol = RTPROT_KERNEL;
> rt->rt6i_flags = RTF_UP | RTF_NONEXTHOP;
^ permalink raw reply
* Re: [PATCH bpf 1/2] bpf, x64: fix memleak when not converging after image
From: David Miller @ 2018-05-02 18:53 UTC (permalink / raw)
To: daniel; +Cc: ast, netdev
In-Reply-To: <20180502181223.30613-2-daniel@iogearbox.net>
From: Daniel Borkmann <daniel@iogearbox.net>
Date: Wed, 2 May 2018 20:12:22 +0200
> While reviewing x64 JIT code, I noticed that we leak the prior allocated
> JIT image in the case where proglen != oldproglen during the JIT passes.
> Prior to the commit e0ee9c12157d ("x86: bpf_jit: fix two bugs in eBPF JIT
> compiler") we would just break out of the loop, and using the image as the
> JITed prog since it could only shrink in size anyway. After e0ee9c12157d,
> we would bail out to out_addrs label where we free addrs and jit_data but
> not the image coming from bpf_jit_binary_alloc().
>
> Fixes: e0ee9c12157d ("x86: bpf_jit: fix two bugs in eBPF JIT compiler")
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> Acked-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* Re: [PATCH bpf 2/2] bpf, x64: fix memleak when not converging on calls
From: David Miller @ 2018-05-02 18:54 UTC (permalink / raw)
To: daniel; +Cc: ast, netdev
In-Reply-To: <20180502181223.30613-3-daniel@iogearbox.net>
From: Daniel Borkmann <daniel@iogearbox.net>
Date: Wed, 2 May 2018 20:12:23 +0200
> The JIT logic in jit_subprogs() is as follows: for all subprogs we
> allocate a bpf_prog_alloc(), populate it (prog->is_func = 1 here),
> and pass it to bpf_int_jit_compile(). If a failure occurred during
> JIT and prog->jited is not set, then we bail out from attempting to
> JIT the whole program, and punt to the interpreter instead. In case
> JITing went successful, we fixup BPF call offsets and do another
> pass to bpf_int_jit_compile() (extra_pass is true at that point) to
> complete JITing calls. Given that requires to pass JIT context around
> addrs and jit_data from x86 JIT are freed in the extra_pass in
> bpf_int_jit_compile() when calls are involved (if not, they can
> be freed immediately). However, if in the original pass, the JIT
> image didn't converge then we leak addrs and jit_data since image
> itself is NULL, the prog->is_func is set and extra_pass is false
> in that case, meaning both will become unreachable and are never
> cleaned up, therefore we need to free as well on !image. Only x64
> JIT is affected.
>
> Fixes: 1c2a088a6626 ("bpf: x64: add JIT support for multi-function programs")
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> Acked-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* Re: [PATCH net-next 1/4] ipv6: Calculate hash thresholds for IPv6 nexthops
From: David Ahern @ 2018-05-02 18:58 UTC (permalink / raw)
To: Ido Schimmel, Eric Dumazet, Thomas.Winter, davem
Cc: Ido Schimmel, netdev, roopa, nikolay, pch, jkbs, yoshfuji, mlxsw
In-Reply-To: <20180502185310.GA31998@splinter>
On 5/2/18 12:53 PM, Ido Schimmel wrote:
>
> So this fixes the issue for me. To reproduce:
>
> # ip -6 address add 2001:db8::1/64 dev dummy0
> # ip -6 address add 2001:db8::1/64 dev dummy1
>
> This reproduces the issue because due to above commit both local routes
> are considered siblings... :/
>
> local 2001:db8::1 proto kernel metric 0
> nexthop dev dummy0 weight 1
> nexthop dev dummy1 weight 1 pref medium
>
> I think it's best to revert the patch and have Thomas submit a fixed
> version to net-next. I was actually surprised to see it applied to net.
ugly side effect of the way ecmp routes are managed in IPv6. I think
revert is the best option for now.
I need to look into a bug report related to v6 and route replace with
ecmp. I'll take a look at why the above is consolidated as well. Those
should not become an ecmp route.
^ permalink raw reply
* Re: [PATCH net-next 1/4] ipv6: Calculate hash thresholds for IPv6 nexthops
From: Ido Schimmel @ 2018-05-02 19:04 UTC (permalink / raw)
To: David Ahern
Cc: Eric Dumazet, Thomas.Winter, davem, Ido Schimmel, netdev, roopa,
nikolay, pch, jkbs, yoshfuji, mlxsw
In-Reply-To: <017f58ee-8655-60d1-19aa-a6276c639065@gmail.com>
On Wed, May 02, 2018 at 12:58:56PM -0600, David Ahern wrote:
> On 5/2/18 12:53 PM, Ido Schimmel wrote:
> >
> > So this fixes the issue for me. To reproduce:
> >
> > # ip -6 address add 2001:db8::1/64 dev dummy0
> > # ip -6 address add 2001:db8::1/64 dev dummy1
> >
> > This reproduces the issue because due to above commit both local routes
> > are considered siblings... :/
> >
> > local 2001:db8::1 proto kernel metric 0
> > nexthop dev dummy0 weight 1
> > nexthop dev dummy1 weight 1 pref medium
> >
> > I think it's best to revert the patch and have Thomas submit a fixed
> > version to net-next. I was actually surprised to see it applied to net.
>
> ugly side effect of the way ecmp routes are managed in IPv6. I think
> revert is the best option for now.
OK. I'll send a patch.
^ permalink raw reply
* RE: [PATCH 2/3] mlx4: Don't bother using skb_tx_hash in mlx4_en_select_queue
From: Ruhl, Michael J @ 2018-05-02 18:09 UTC (permalink / raw)
To: Duyck, Alexander H, netdev@vger.kernel.org, davem@davemloft.net
Cc: linux-rdma@vger.kernel.org, Dalessandro, Dennis,
Vishwanathapura, Niranjana, tariqt@mellanox.com
In-Reply-To: <20180427180640.4883.75746.stgit@ahduyck-green-test.jf.intel.com>
>-----Original Message-----
>From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-
>owner@vger.kernel.org] On Behalf Of Alexander Duyck
>Sent: Friday, April 27, 2018 2:07 PM
>To: netdev@vger.kernel.org; davem@davemloft.net
>Cc: linux-rdma@vger.kernel.org; Dalessandro, Dennis
><dennis.dalessandro@intel.com>; Vishwanathapura, Niranjana
><niranjana.vishwanathapura@intel.com>; tariqt@mellanox.com
>Subject: [PATCH 2/3] mlx4: Don't bother using skb_tx_hash in
>mlx4_en_select_queue
>
>The code in the fallback path has supported XDP in conjunction with the Tx
>traffic classification for TCs for over a year now. So instead of just
>calling skb_tx_hash for every packet we are better off using the fallback
>since that will record the Tx queue to the socket and then that can be used
>instead of having to recompute the hash every time.
>
>Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
>---
> drivers/net/ethernet/mellanox/mlx4/en_tx.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/drivers/net/ethernet/mellanox/mlx4/en_tx.c
>b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
>index 6b68537..0227786 100644
>--- a/drivers/net/ethernet/mellanox/mlx4/en_tx.c
>+++ b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
>@@ -694,7 +694,7 @@ u16 mlx4_en_select_queue(struct net_device *dev,
>struct sk_buff *skb,
> u16 rings_p_up = priv->num_tx_rings_p_up;
>
> if (netdev_get_num_tc(dev))
>- return skb_tx_hash(dev, skb);
>+ return fallback(dev, skb);
>
> return fallback(dev, skb) % rings_p_up;
Hi Alexander,
The final return fallback() call is doing a % rings_p_up.
Do you need to do that for the new fallback() call?
Maybe you can get rid of the netdev_get_num_tc() call altogether?
Thanks,
Mike
> }
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH bpf-next 0/3] bpf: cleanups on managing subprog information
From: Jiong Wang @ 2018-05-02 19:22 UTC (permalink / raw)
To: John Fastabend, Alexei Starovoitov; +Cc: borkmann, ecree, netdev, oss-drivers
In-Reply-To: <d91879b5-5389-c35b-93d0-59295e83520b@gmail.com>
On 02/05/2018 18:24, John Fastabend wrote:
> On 05/02/2018 09:59 AM, Jiong Wang wrote:
>> On 01/05/2018 23:22, Alexei Starovoitov wrote:
>> ...
>>> [ 27.784931] ? bpf_int_jit_compile+0x7ac/0xab0
>>> [ 27.785475] bpf_int_jit_compile+0x2b6/0xab0
>>> [ 27.786001] ? do_jit+0x6020/0x6020
>>> [ 27.786428] ? kasan_kmalloc+0xa0/0xd0
>>> [ 27.786885] bpf_check+0x2c05/0x4c40
>>> [ 27.787346] ? fixup_bpf_calls+0x1140/0x1140
>>> [ 27.787865] ? kasan_unpoison_shadow+0x30/0x40
>>> [ 27.788406] ? kasan_kmalloc+0xa0/0xd0
>>> [ 27.788865] ? memset+0x1f/0x40
>>> [ 27.789255] ? bpf_obj_name_cpy+0x2d/0x200
>>> [ 27.789750] bpf_prog_load+0xb07/0xeb0
>>>
>>> simply running test_verifier with JIT and kasan on.
>> Ah, sorry, I should add "sysctl net/core/bpf_jit_enable=1" to my test
>> script, error reproduced.
>>
>> convert_ctx_accesses and fixup_bpf_calls might insert ebpf insns that
>> prog->len would change.
>>
>> The new fake "exit" subprog whose .start offset is prog->len should be
>> updated as well.
>>
>> The "for" condition in adjust_subprog_starts:
>>
>> for (i = 0; i < env->subprog_cnt; i++) {
>>
>> need to be changed into:
>>
>> for (i = 0; i <= env->subprog_cnt; i++) {
>>
>> Will respin the patch set.
>>
>> Thanks.
>>
>> Regards,
>> Jiong
>>
> Also a bit of a nit, but if you are doing a respin. How about
> consider renaming BPF_MAX_SUBPROGS -> BPF_MAX_PROGS. It will
> make the naming more accurate and also avoid some diffs below
> where changing '>=' to '>' is required.
I have been pondering renaming BPF_MAX_SUBPROGS to other name like
what you suggested, but failed to convince myself, mostly due to there
are quite a few other variables etc that are using the "subprog" name
convention, so I am thinking use subprog is also fine as traditional
main prog/func is also a sub prog/func, it is just the entry one.
So I am thinking it might be not worth renaming everything related, and
tend to just keep it as is.
Thanks.
Regards,
Jiong
>
> @@ -191,7 +191,7 @@ struct bpf_verifier_env {
> bool seen_direct_write;
> struct bpf_insn_aux_data *insn_aux_data; /* array of per-insn state */
> struct bpf_verifier_log log;
> - u32 subprog_starts[BPF_MAX_SUBPROGS];
> + u32 subprog_starts[BPF_MAX_SUBPROGS + 1];
> /* computes the stack depth of each bpf function */
> u16 subprog_stack_depth[BPF_MAX_SUBPROGS + 1];
> u32 subprog_cnt;
^ permalink raw reply
* [PATCH net-next 00/10] r8169: series with further improvements
From: Heiner Kallweit @ 2018-05-02 19:28 UTC (permalink / raw)
To: David Miller, Realtek linux nic maintainers; +Cc: netdev@vger.kernel.org
I thought I'm more or less done with the basic refactoring. But again
I stumbled across things that can be improved / simplified.
Heiner Kallweit (10):
r8169: remove unneeded check in r8168_pll_power_down
r8169: remove 810x_phy_power_up/down
r8169: merge r810x_pll_power_down/up into r8168_pll_power_down/up
r8169: drop member pll_power_ops from struct rtl8169_private
r8169: simplify code by using ranges in switch clauses
r8169: replace longer if statements with switch statements
r8169: drop rtl_generic_op
r8169: improve PCI config space access
r8169: avoid potentially misaligned access when getting mac address
r8169: replace get_protocol with vlan_get_protocol
drivers/net/ethernet/realtek/r8169.c | 565 +++++----------------------
1 file changed, 98 insertions(+), 467 deletions(-)
--
2.17.0
^ permalink raw reply
* Re: [PATCH RFC net-next] net: ipvs: Adjust gso_size for IPPROTO_TCP
From: Julian Anastasov @ 2018-05-02 19:30 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: netdev, David Ahern, Tom Herbert, Eric Dumazet, Nikita Shirokov,
kernel-team, lvs-devel
In-Reply-To: <20180502171041.s3euld6i7hm6bw5c@kafai-mbp>
Hello,
On Wed, 2 May 2018, Martin KaFai Lau wrote:
> On Wed, May 02, 2018 at 09:38:43AM +0300, Julian Anastasov wrote:
> >
> > - initial traffic for port 21 does not use GSO. But after
> > every packet IPVS calls maybe_update_pmtu (rt->dst.ops->update_pmtu)
> > to report the reduced MTU. These updates are stored in fnhe_pmtu
> > but they do not go to any route, even if we try to get fresh
> > output route. Why? Because the local routes are not cached, so
> > they can not use the fnhe. This is what my patch for route.c
> > will fix. With this fix FTP-DATA gets route with reduced PMTU.
> For IPv6, the 'if (rt6->rt6i_flags & RTF_LOCAL)' gate in
> __ip6_rt_update_pmtu() may need to be lifted also.
Probably. I completely forgot the IPv6 part
but as I don't know the IPv6 code enough, it may take
some time to understand what can be the problem there...
I'm not sure whether everything started with commit 0a6b2a1dc2a2,
so that in some configurations before that commit things
worked and problem was not noticed.
I think, we should focus on such direction for IPv6:
- do we remember per-VIP PMTU for the local routes
- when exactly we start to use the new PMTU, eg. what happens
in case socket caches the route, whether route is killed via
dst->obsolete. Or may be while the PMTU expiration is handled
per-packet, the PMTU change is noticed only on ICMP...
- as IPVS reports the PMTU via dst.ops->update_pmtu() long
before any large packets are sent, do we propagate the
PMTU. Also, for IPv4 __ip_rt_update_pmtu() has some protection
from such per-packet updates that do not change the PMTU.
- if IPVS starts to send ICMP when gso_size exceeds PMTU,
like in my draft patch, whether the PMTU is propagated
to route and then to socket. As for the gso_size decrease,
playing in IPVS is not very safe, at least, we need help
from GSO experts to know how we should use it.
Regards
^ permalink raw reply
* Re: [bpf PATCH 3/3] bpf: sockmap, fix error handling in redirect failures
From: Alexei Starovoitov @ 2018-05-02 19:34 UTC (permalink / raw)
To: John Fastabend; +Cc: borkmann, ast, netdev
In-Reply-To: <20180502174737.17875.74031.stgit@john-Precision-Tower-5810>
On Wed, May 02, 2018 at 10:47:37AM -0700, John Fastabend wrote:
> When a redirect failure happens we release the buffers in-flight
> without calling a sk_mem_uncharge(), the uncharge is called before
> dropping the sock lock for the redirecte, however we missed updating
> the ring start index. When no apply actions are in progress this
> is OK because we uncharge the entire buffer before the redirect.
> But, when we have apply logic running its possible that only a
> portion of the buffer is being redirected. In this case we only
> do memory accounting for the buffer slice being redirected and
> expect to be able to loop over the BPF program again and/or if
> a sock is closed uncharge the memory at sock destruct time.
>
> With an invalid start index however the program logic looks at
> the start pointer index, checks the length, and when seeing the
> length is zero (from the initial release and failure to update
> the pointer) aborts without uncharging/releasing the remaining
> memory.
>
> The fix for this is simply to update the start index. To avoid
> fixing this error in two locations we do a small refactor and
> remove one case where it is open-coded. Then fix it in the
> single function.
>
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
> ---
> kernel/bpf/sockmap.c | 26 +++++++++++---------------
> 1 file changed, 11 insertions(+), 15 deletions(-)
>
> diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
> index 052c313..7e3c4cd 100644
> --- a/kernel/bpf/sockmap.c
> +++ b/kernel/bpf/sockmap.c
> @@ -393,7 +393,8 @@ static void return_mem_sg(struct sock *sk, int bytes, struct sk_msg_buff *md)
> } while (i != md->sg_end);
> }
>
> -static void free_bytes_sg(struct sock *sk, int bytes, struct sk_msg_buff *md)
> +static void free_bytes_sg(struct sock *sk, int bytes,
> + struct sk_msg_buff *md, bool charge)
> {
> struct scatterlist *sg = md->sg_data;
> int i = md->sg_start, free;
> @@ -403,11 +404,13 @@ static void free_bytes_sg(struct sock *sk, int bytes, struct sk_msg_buff *md)
> if (bytes < free) {
> sg[i].length -= bytes;
> sg[i].offset += bytes;
> - sk_mem_uncharge(sk, bytes);
> + if (charge)
> + sk_mem_uncharge(sk, bytes);
> break;
> }
>
> - sk_mem_uncharge(sk, sg[i].length);
> + if (charge)
> + sk_mem_uncharge(sk, sg[i].length);
> put_page(sg_page(&sg[i]));
> bytes -= sg[i].length;
> sg[i].length = 0;
> @@ -418,6 +421,7 @@ static void free_bytes_sg(struct sock *sk, int bytes, struct sk_msg_buff *md)
> if (i == MAX_SKB_FRAGS)
> i = 0;
> }
> + md->sg_start = i;
> }
>
> static int free_sg(struct sock *sk, int start, struct sk_msg_buff *md)
> @@ -578,7 +582,7 @@ static int bpf_tcp_sendmsg_do_redirect(struct sock *sk, int send,
> {
> struct smap_psock *psock;
> struct scatterlist *sg;
> - int i, err, free = 0;
> + int i, err = 0;
> bool ingress = !!(md->flags & BPF_F_INGRESS);
>
> sg = md->sg_data;
> @@ -607,16 +611,8 @@ static int bpf_tcp_sendmsg_do_redirect(struct sock *sk, int send,
> out_rcu:
> rcu_read_unlock();
> out:
> - i = md->sg_start;
> - while (sg[i].length) {
> - free += sg[i].length;
> - put_page(sg_page(&sg[i]));
> - sg[i].length = 0;
> - i++;
> - if (i == MAX_SKB_FRAGS)
> - i = 0;
> - }
this hunk is causing:
../kernel/bpf/sockmap.c:585:6: warning: unused variable ‘i’ [-Wunused-variable]
int i, err = 0;
please respin.
^ permalink raw reply
* Re: [PATCH bpf 0/2] Two x86 BPF JIT fixes
From: Alexei Starovoitov @ 2018-05-02 19:39 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: ast, netdev
In-Reply-To: <20180502181223.30613-1-daniel@iogearbox.net>
On Wed, May 02, 2018 at 08:12:21PM +0200, Daniel Borkmann wrote:
> Fix two memory leaks in x86 JIT. For details, please see
> individual patches in this series. Thanks!
Applied to bpf tree, Thanks Daniel.
^ permalink raw reply
* [PATCH net-next 01/10] r8169: remove unneeded check in r8168_pll_power_down
From: Heiner Kallweit @ 2018-05-02 19:39 UTC (permalink / raw)
To: David Miller, Realtek linux nic maintainers; +Cc: netdev@vger.kernel.org
In-Reply-To: <2c7cae9e-d989-8529-7209-5a271242ce39@gmail.com>
RTL_GIGA_MAC_VER_23/24 are configured by rtl_hw_start_8168cp_2()
and rtl_hw_start_8168cp_3() respectively which both apply
CPCMD_QUIRK_MASK, thus clearing bit ASF.
Bit ASF isn't set at any other place in the driver, therefore this
check can be removed.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/ethernet/realtek/r8169.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 66f10d11..ce7843aa 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -4838,12 +4838,6 @@ static void r8168_pll_power_down(struct rtl8169_private *tp)
if (r8168_check_dash(tp))
return;
- if ((tp->mac_version == RTL_GIGA_MAC_VER_23 ||
- tp->mac_version == RTL_GIGA_MAC_VER_24) &&
- (tp->cp_cmd & ASF)) {
- return;
- }
-
if (tp->mac_version == RTL_GIGA_MAC_VER_32 ||
tp->mac_version == RTL_GIGA_MAC_VER_33)
rtl_ephy_write(tp, 0x19, 0xff64);
--
2.17.0
^ permalink raw reply related
* [PATCH net-next 03/10] r8169: merge r810x_pll_power_down/up into r8168_pll_power_down/up
From: Heiner Kallweit @ 2018-05-02 19:39 UTC (permalink / raw)
To: David Miller, Realtek linux nic maintainers; +Cc: netdev@vger.kernel.org
In-Reply-To: <2c7cae9e-d989-8529-7209-5a271242ce39@gmail.com>
r810x_pll_power_down/up and r8168_pll_power_down/up have a lot in common,
so we can simplify the code by merging the former into the latter.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/ethernet/realtek/r8169.c | 61 ++++++++--------------------
1 file changed, 16 insertions(+), 45 deletions(-)
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 58559a00..0585a9c6 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -4778,49 +4778,6 @@ static void r8168_phy_power_down(struct rtl8169_private *tp)
}
}
-static void r810x_pll_power_down(struct rtl8169_private *tp)
-{
- if (rtl_wol_pll_power_down(tp))
- return;
-
- r8168_phy_power_down(tp);
-
- switch (tp->mac_version) {
- case RTL_GIGA_MAC_VER_07:
- case RTL_GIGA_MAC_VER_08:
- case RTL_GIGA_MAC_VER_09:
- case RTL_GIGA_MAC_VER_10:
- case RTL_GIGA_MAC_VER_13:
- case RTL_GIGA_MAC_VER_16:
- break;
- default:
- RTL_W8(tp, PMCH, RTL_R8(tp, PMCH) & ~0x80);
- break;
- }
-}
-
-static void r810x_pll_power_up(struct rtl8169_private *tp)
-{
- r8168_phy_power_up(tp);
-
- switch (tp->mac_version) {
- case RTL_GIGA_MAC_VER_07:
- case RTL_GIGA_MAC_VER_08:
- case RTL_GIGA_MAC_VER_09:
- case RTL_GIGA_MAC_VER_10:
- case RTL_GIGA_MAC_VER_13:
- case RTL_GIGA_MAC_VER_16:
- break;
- case RTL_GIGA_MAC_VER_47:
- case RTL_GIGA_MAC_VER_48:
- RTL_W8(tp, PMCH, RTL_R8(tp, PMCH) | 0xc0);
- break;
- default:
- RTL_W8(tp, PMCH, RTL_R8(tp, PMCH) | 0x80);
- break;
- }
-}
-
static void r8168_pll_power_down(struct rtl8169_private *tp)
{
if (r8168_check_dash(tp))
@@ -4840,12 +4797,19 @@ static void r8168_pll_power_down(struct rtl8169_private *tp)
case RTL_GIGA_MAC_VER_26:
case RTL_GIGA_MAC_VER_27:
case RTL_GIGA_MAC_VER_28:
+ case RTL_GIGA_MAC_VER_29:
+ case RTL_GIGA_MAC_VER_30:
case RTL_GIGA_MAC_VER_31:
case RTL_GIGA_MAC_VER_32:
case RTL_GIGA_MAC_VER_33:
+ case RTL_GIGA_MAC_VER_37:
+ case RTL_GIGA_MAC_VER_39:
+ case RTL_GIGA_MAC_VER_43:
case RTL_GIGA_MAC_VER_44:
case RTL_GIGA_MAC_VER_45:
case RTL_GIGA_MAC_VER_46:
+ case RTL_GIGA_MAC_VER_47:
+ case RTL_GIGA_MAC_VER_48:
case RTL_GIGA_MAC_VER_50:
case RTL_GIGA_MAC_VER_51:
RTL_W8(tp, PMCH, RTL_R8(tp, PMCH) & ~0x80);
@@ -4867,14 +4831,21 @@ static void r8168_pll_power_up(struct rtl8169_private *tp)
case RTL_GIGA_MAC_VER_26:
case RTL_GIGA_MAC_VER_27:
case RTL_GIGA_MAC_VER_28:
+ case RTL_GIGA_MAC_VER_29:
+ case RTL_GIGA_MAC_VER_30:
case RTL_GIGA_MAC_VER_31:
case RTL_GIGA_MAC_VER_32:
case RTL_GIGA_MAC_VER_33:
+ case RTL_GIGA_MAC_VER_37:
+ case RTL_GIGA_MAC_VER_39:
+ case RTL_GIGA_MAC_VER_43:
RTL_W8(tp, PMCH, RTL_R8(tp, PMCH) | 0x80);
break;
case RTL_GIGA_MAC_VER_44:
case RTL_GIGA_MAC_VER_45:
case RTL_GIGA_MAC_VER_46:
+ case RTL_GIGA_MAC_VER_47:
+ case RTL_GIGA_MAC_VER_48:
case RTL_GIGA_MAC_VER_50:
case RTL_GIGA_MAC_VER_51:
RTL_W8(tp, PMCH, RTL_R8(tp, PMCH) | 0xc0);
@@ -4925,8 +4896,8 @@ static void rtl_init_pll_power_ops(struct rtl8169_private *tp)
case RTL_GIGA_MAC_VER_43:
case RTL_GIGA_MAC_VER_47:
case RTL_GIGA_MAC_VER_48:
- ops->down = r810x_pll_power_down;
- ops->up = r810x_pll_power_up;
+ ops->down = r8168_pll_power_down;
+ ops->up = r8168_pll_power_up;
break;
case RTL_GIGA_MAC_VER_11:
--
2.17.0
^ permalink raw reply related
* [PATCH net-next 02/10] r8169: remove 810x_phy_power_up/down
From: Heiner Kallweit @ 2018-05-02 19:39 UTC (permalink / raw)
To: David Miller, Realtek linux nic maintainers; +Cc: netdev@vger.kernel.org
In-Reply-To: <2c7cae9e-d989-8529-7209-5a271242ce39@gmail.com>
The functionality of 810x_phy_power_up/down is covered by the default
clause in 8168_phy_power_up/down. Therefore we don't need these
functions.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/ethernet/realtek/r8169.c | 98 ++++++++++++----------------
1 file changed, 43 insertions(+), 55 deletions(-)
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index ce7843aa..58559a00 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -4718,61 +4718,6 @@ static bool rtl_wol_pll_power_down(struct rtl8169_private *tp)
return true;
}
-static void r810x_phy_power_down(struct rtl8169_private *tp)
-{
- rtl_writephy(tp, 0x1f, 0x0000);
- rtl_writephy(tp, MII_BMCR, BMCR_PDOWN);
-}
-
-static void r810x_phy_power_up(struct rtl8169_private *tp)
-{
- rtl_writephy(tp, 0x1f, 0x0000);
- rtl_writephy(tp, MII_BMCR, BMCR_ANENABLE);
-}
-
-static void r810x_pll_power_down(struct rtl8169_private *tp)
-{
- if (rtl_wol_pll_power_down(tp))
- return;
-
- r810x_phy_power_down(tp);
-
- switch (tp->mac_version) {
- case RTL_GIGA_MAC_VER_07:
- case RTL_GIGA_MAC_VER_08:
- case RTL_GIGA_MAC_VER_09:
- case RTL_GIGA_MAC_VER_10:
- case RTL_GIGA_MAC_VER_13:
- case RTL_GIGA_MAC_VER_16:
- break;
- default:
- RTL_W8(tp, PMCH, RTL_R8(tp, PMCH) & ~0x80);
- break;
- }
-}
-
-static void r810x_pll_power_up(struct rtl8169_private *tp)
-{
- r810x_phy_power_up(tp);
-
- switch (tp->mac_version) {
- case RTL_GIGA_MAC_VER_07:
- case RTL_GIGA_MAC_VER_08:
- case RTL_GIGA_MAC_VER_09:
- case RTL_GIGA_MAC_VER_10:
- case RTL_GIGA_MAC_VER_13:
- case RTL_GIGA_MAC_VER_16:
- break;
- case RTL_GIGA_MAC_VER_47:
- case RTL_GIGA_MAC_VER_48:
- RTL_W8(tp, PMCH, RTL_R8(tp, PMCH) | 0xc0);
- break;
- default:
- RTL_W8(tp, PMCH, RTL_R8(tp, PMCH) | 0x80);
- break;
- }
-}
-
static void r8168_phy_power_up(struct rtl8169_private *tp)
{
rtl_writephy(tp, 0x1f, 0x0000);
@@ -4833,6 +4778,49 @@ static void r8168_phy_power_down(struct rtl8169_private *tp)
}
}
+static void r810x_pll_power_down(struct rtl8169_private *tp)
+{
+ if (rtl_wol_pll_power_down(tp))
+ return;
+
+ r8168_phy_power_down(tp);
+
+ switch (tp->mac_version) {
+ case RTL_GIGA_MAC_VER_07:
+ case RTL_GIGA_MAC_VER_08:
+ case RTL_GIGA_MAC_VER_09:
+ case RTL_GIGA_MAC_VER_10:
+ case RTL_GIGA_MAC_VER_13:
+ case RTL_GIGA_MAC_VER_16:
+ break;
+ default:
+ RTL_W8(tp, PMCH, RTL_R8(tp, PMCH) & ~0x80);
+ break;
+ }
+}
+
+static void r810x_pll_power_up(struct rtl8169_private *tp)
+{
+ r8168_phy_power_up(tp);
+
+ switch (tp->mac_version) {
+ case RTL_GIGA_MAC_VER_07:
+ case RTL_GIGA_MAC_VER_08:
+ case RTL_GIGA_MAC_VER_09:
+ case RTL_GIGA_MAC_VER_10:
+ case RTL_GIGA_MAC_VER_13:
+ case RTL_GIGA_MAC_VER_16:
+ break;
+ case RTL_GIGA_MAC_VER_47:
+ case RTL_GIGA_MAC_VER_48:
+ RTL_W8(tp, PMCH, RTL_R8(tp, PMCH) | 0xc0);
+ break;
+ default:
+ RTL_W8(tp, PMCH, RTL_R8(tp, PMCH) | 0x80);
+ break;
+ }
+}
+
static void r8168_pll_power_down(struct rtl8169_private *tp)
{
if (r8168_check_dash(tp))
--
2.17.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox