* Re: [PATCH] net: tcp_input: Neaten DBGUNDO
From: David Miller @ 2017-09-18 18:29 UTC (permalink / raw)
To: joe; +Cc: kuznet, yoshfuji, netdev, linux-kernel
In-Reply-To: <fcac7d078947b59e47ad4474d2c60d4fd3395bcd.1505095255.git.joe@perches.com>
From: Joe Perches <joe@perches.com>
Date: Sun, 10 Sep 2017 19:02:25 -0700
> Move the #ifdef into the static void function so that the use
> of DBGUNDO is validated when FASTRETRANS_DEBUG <= 1.
>
> Remove the now unnecessary #else and #define DBGUNDO.
>
> Signed-off-by: Joe Perches <joe@perches.com>
Applied.
^ permalink raw reply
* Re: [PATCH 3/3] selftests: silence test output by default
From: Josef Bacik @ 2017-09-18 18:24 UTC (permalink / raw)
To: Shuah Khan
Cc: Josef Bacik, Shuah Khan, Josef Bacik, David S. Miller,
linux-kernel, linux-kselftest, netdev
In-Reply-To: <ed8261a2-557a-cf1c-7298-d530bb7861d0@osg.samsung.com>
On Mon, Sep 18, 2017 at 12:13:40PM -0600, Shuah Khan wrote:
> On 09/18/2017 11:52 AM, Josef Bacik wrote:
> > On Mon, Sep 18, 2017 at 11:46:18AM -0600, Shuah Khan wrote:
> >> On 09/18/2017 11:37 AM, josef@toxicpanda.com wrote:
> >>> From: Josef Bacik <jbacik@fb.com>
> >>>
> >>> Some of the networking tests are very noisy and make it impossible to
> >>> see if we actually passed the tests as they run. Default to suppressing
> >>> the output from any tests run in order to make it easier to track what
> >>> failed.
> >>>
> >>> Signed-off-by: Josef Bacik <jbacik@fb.com>
> >>> --
> >>
> >> This change suppresses pass/fail wrapper output for all tests, not just the
> >> networking tests.
> >>
> >> Could you please send me before and after results for what you are trying
> >> to fix.
> >>
> >
> > Yeah I wanted to suppress extraneous output from everybody, I just happened to
> > notice it because I was testing net. The default thing already spits out what
> > it's running and pass/fail, there's no need to include all of the random output
> > unless the user wants to go and run the test manually. As it is now it's
> > _impossible_ to tell what ran and what passed/failed because of all the random
> > output.
>
> Unfortunately kselftests have lots of users that want different things. A recent
> request is to use TAP13 format for output for external parsers to be able to parse.
> That is what this change to add TAP13 header does.
>
> The output you are seeing is the TAP 13 format to indicate the test has passed.
>
> The right fix would be to suppress the Pass/Fail from the individual shell script
> and have the shell script exit with error code. kselftest lib.mk will handle the
> error code and print out pass/fail like it is doing now.
>
> Using the common logic will help avoid duplicate code in tests/test scripts and
> also makes the pass/fail messages consistent.
>
> In the following output the individual test output can be eliminated since lib.mk
> run_tests does that for you. In addition, you will also get a count of tests at
> the end of the run of all tests in a test directory.
>
> TAP version 13
> selftests: run_netsocktests
> ========================================
> --------------------
> running socket test
> --------------------
> [PASS]
> ok 1..1 selftests: run_netsocktests [PASS]
> selftests: run_afpackettests
> ========================================
> must be run as root
> ok 1..2 selftests: run_afpackettests [PASS]
> selftests: test_bpf.sh
> ========================================
> test_bpf: [FAIL]
> not ok 1..3 selftests: test_bpf.sh [FAIL]
> selftests: netdevice.sh
> ========================================
> SKIP: Need root privileges
> ok 1..4 selftests: netdevice.sh [PASS]
>
> If you eliminate that you will just see the common lib.mk results.
>
> TAP version 13
> selftests: run_netsocktests
> ========================================
> ok 1..1 selftests: run_netsocktests [PASS]
> selftests: run_afpackettests
> ========================================
> must be run as root
> ok 1..2 selftests: run_afpackettests [PASS]
> ========================================
> selftests: test_bpf.sh
> ========================================
> not ok 1..3 selftests: test_bpf.sh [FAIL]
> selftests: netdevice.sh
> ========================================
> SKIP: Need root privileges
> ok 1..4 selftests: netdevice.sh [PASS]
>
>
> If you would like to fix the duplicate output, please send me patches
> to remove pass/fail output strings from tests instead. It is on my
> todo to do that this release.
>
I'm confused, this is exactly what my patch does, it strips all of the
extraneous output and leaves only the TAP13 output. Here is the output without
my suppression patch
https://da.gd/pup0
and here is the output with my suppression patch
https://da.gd/3olKj
Unless I'm missing something subtle it appears to be exactly the output you
want, without the random crap from the other tests. The only thing I'm
redirecting is the output of the _test_ itself, $$BASENAME_TEST from what I can
tell is the actual test we're running, not the wrapper, so everything is as it
should be. Thanks,
Josef
^ permalink raw reply
* [PATCH] vsock: vmci: Remove unneeded linux/miscdevice.h include
From: Corentin Labbe @ 2017-09-18 18:18 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-kernel, Corentin Labbe
net/vmw_vsock/vmci_transport.c does not use any miscdevice so this patch
remove this unnecessary inclusion.
Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
net/vmw_vsock/vmci_transport.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
index 10ae7823a19d..0206155bff53 100644
--- a/net/vmw_vsock/vmci_transport.c
+++ b/net/vmw_vsock/vmci_transport.c
@@ -21,7 +21,6 @@
#include <linux/kernel.h>
#include <linux/kmod.h>
#include <linux/list.h>
-#include <linux/miscdevice.h>
#include <linux/module.h>
#include <linux/mutex.h>
#include <linux/net.h>
--
2.13.5
^ permalink raw reply related
* Re: [PATCH 3/3] selftests: silence test output by default
From: Shuah Khan @ 2017-09-18 18:13 UTC (permalink / raw)
To: Josef Bacik, Shuah Khan
Cc: Josef Bacik, David S. Miller, linux-kernel, linux-kselftest,
netdev, Shuah Khan
In-Reply-To: <20170918175254.lsdzqvjm4uvix4rj@destiny>
On 09/18/2017 11:52 AM, Josef Bacik wrote:
> On Mon, Sep 18, 2017 at 11:46:18AM -0600, Shuah Khan wrote:
>> On 09/18/2017 11:37 AM, josef@toxicpanda.com wrote:
>>> From: Josef Bacik <jbacik@fb.com>
>>>
>>> Some of the networking tests are very noisy and make it impossible to
>>> see if we actually passed the tests as they run. Default to suppressing
>>> the output from any tests run in order to make it easier to track what
>>> failed.
>>>
>>> Signed-off-by: Josef Bacik <jbacik@fb.com>
>>> --
>>
>> This change suppresses pass/fail wrapper output for all tests, not just the
>> networking tests.
>>
>> Could you please send me before and after results for what you are trying
>> to fix.
>>
>
> Yeah I wanted to suppress extraneous output from everybody, I just happened to
> notice it because I was testing net. The default thing already spits out what
> it's running and pass/fail, there's no need to include all of the random output
> unless the user wants to go and run the test manually. As it is now it's
> _impossible_ to tell what ran and what passed/failed because of all the random
> output.
Unfortunately kselftests have lots of users that want different things. A recent
request is to use TAP13 format for output for external parsers to be able to parse.
That is what this change to add TAP13 header does.
The output you are seeing is the TAP 13 format to indicate the test has passed.
The right fix would be to suppress the Pass/Fail from the individual shell script
and have the shell script exit with error code. kselftest lib.mk will handle the
error code and print out pass/fail like it is doing now.
Using the common logic will help avoid duplicate code in tests/test scripts and
also makes the pass/fail messages consistent.
In the following output the individual test output can be eliminated since lib.mk
run_tests does that for you. In addition, you will also get a count of tests at
the end of the run of all tests in a test directory.
TAP version 13
selftests: run_netsocktests
========================================
--------------------
running socket test
--------------------
[PASS]
ok 1..1 selftests: run_netsocktests [PASS]
selftests: run_afpackettests
========================================
must be run as root
ok 1..2 selftests: run_afpackettests [PASS]
selftests: test_bpf.sh
========================================
test_bpf: [FAIL]
not ok 1..3 selftests: test_bpf.sh [FAIL]
selftests: netdevice.sh
========================================
SKIP: Need root privileges
ok 1..4 selftests: netdevice.sh [PASS]
If you eliminate that you will just see the common lib.mk results.
TAP version 13
selftests: run_netsocktests
========================================
ok 1..1 selftests: run_netsocktests [PASS]
selftests: run_afpackettests
========================================
must be run as root
ok 1..2 selftests: run_afpackettests [PASS]
========================================
selftests: test_bpf.sh
========================================
not ok 1..3 selftests: test_bpf.sh [FAIL]
selftests: netdevice.sh
========================================
SKIP: Need root privileges
ok 1..4 selftests: netdevice.sh [PASS]
If you would like to fix the duplicate output, please send me patches
to remove pass/fail output strings from tests instead. It is on my
todo to do that this release.
thanks,
-- Shuah
^ permalink raw reply
* Re: Regression in throughput between kvm guests over virtual bridge
From: Matthew Rosato @ 2017-09-18 18:11 UTC (permalink / raw)
To: Jason Wang, netdev; +Cc: davem, mst
In-Reply-To: <c9d43af9-f095-043a-7d46-acd84a18f75a@redhat.com>
On 09/18/2017 03:36 AM, Jason Wang wrote:
>
>
> On 2017年09月18日 11:13, Jason Wang wrote:
>>
>>
>> On 2017年09月16日 03:19, Matthew Rosato wrote:
>>>> It looks like vhost is slowed down for some reason which leads to more
>>>> idle time on 4.13+VHOST_RX_BATCH=1. Appreciated if you can collect the
>>>> perf.diff on host, one for rx and one for tx.
>>>>
>>> perf data below for the associated vhost threads, baseline=4.12,
>>> delta1=4.13, delta2=4.13+VHOST_RX_BATCH=1
>>>
>>> Client vhost:
>>>
>>> 60.12% -11.11% -12.34% [kernel.vmlinux] [k] raw_copy_from_user
>>> 13.76% -1.28% -0.74% [kernel.vmlinux] [k] get_page_from_freelist
>>> 2.00% +3.69% +3.54% [kernel.vmlinux] [k] __wake_up_sync_key
>>> 1.19% +0.60% +0.66% [kernel.vmlinux] [k] __alloc_pages_nodemask
>>> 1.12% +0.76% +0.86% [kernel.vmlinux] [k] copy_page_from_iter
>>> 1.09% +0.28% +0.35% [vhost] [k] vhost_get_vq_desc
>>> 1.07% +0.31% +0.26% [kernel.vmlinux] [k] alloc_skb_with_frags
>>> 0.94% +0.42% +0.65% [kernel.vmlinux] [k] alloc_pages_current
>>> 0.91% -0.19% -0.18% [kernel.vmlinux] [k] memcpy
>>> 0.88% +0.26% +0.30% [kernel.vmlinux] [k] __next_zones_zonelist
>>> 0.85% +0.05% +0.12% [kernel.vmlinux] [k] iov_iter_advance
>>> 0.79% +0.09% +0.19% [vhost] [k] __vhost_add_used_n
>>> 0.74% [kernel.vmlinux] [k] get_task_policy.part.7
>>> 0.74% -0.01% -0.05% [kernel.vmlinux] [k] tun_net_xmit
>>> 0.60% +0.17% +0.33% [kernel.vmlinux] [k] policy_nodemask
>>> 0.58% -0.15% -0.12% [ebtables] [k] ebt_do_table
>>> 0.52% -0.25% -0.22% [kernel.vmlinux] [k] __alloc_skb
>>> ...
>>> 0.42% +0.58% +0.59% [kernel.vmlinux] [k] eventfd_signal
>>> ...
>>> 0.32% +0.96% +0.93% [kernel.vmlinux] [k] finish_task_switch
>>> ...
>>> +1.50% +1.16% [kernel.vmlinux] [k] get_task_policy.part.9
>>> +0.40% +0.42% [kernel.vmlinux] [k] __skb_get_hash_symmetr
>>> +0.39% +0.40% [kernel.vmlinux] [k] _copy_from_iter_full
>>> +0.24% +0.23% [vhost_net] [k] vhost_net_buf_peek
>>>
>>> Server vhost:
>>>
>>> 61.93% -10.72% -10.91% [kernel.vmlinux] [k] raw_copy_to_user
>>> 9.25% +0.47% +0.86% [kernel.vmlinux] [k] free_hot_cold_page
>>> 5.16% +1.41% +1.57% [vhost] [k] vhost_get_vq_desc
>>> 5.12% -3.81% -3.78% [kernel.vmlinux] [k] skb_release_data
>>> 3.30% +0.42% +0.55% [kernel.vmlinux] [k] raw_copy_from_user
>>> 1.29% +2.20% +2.28% [kernel.vmlinux] [k] copy_page_to_iter
>>> 1.24% +1.65% +0.45% [vhost_net] [k] handle_rx
>>> 1.08% +3.03% +2.85% [kernel.vmlinux] [k] __wake_up_sync_key
>>> 0.96% +0.70% +1.10% [vhost] [k] translate_desc
>>> 0.69% -0.20% -0.22% [kernel.vmlinux] [k] tun_do_read.part.10
>>> 0.69% [kernel.vmlinux] [k] tun_peek_len
>>> 0.67% +0.75% +0.78% [kernel.vmlinux] [k] eventfd_signal
>>> 0.52% +0.96% +0.98% [kernel.vmlinux] [k] finish_task_switch
>>> 0.50% +0.05% +0.09% [vhost] [k] vhost_add_used_n
>>> ...
>>> +0.63% +0.58% [vhost_net] [k] vhost_net_buf_peek
>>> +0.32% +0.32% [kernel.vmlinux] [k] _copy_to_iter
>>> +0.19% +0.19% [kernel.vmlinux] [k] __skb_get_hash_symmetr
>>> +0.11% +0.21% [vhost] [k] vhost_umem_interval_tr
>>>
>>
>> Looks like for some unknown reason which leads more wakeups.
>>
>> Could you please try to attached patch to see if it solves or mitigate
>> the issue?
>>
>> Thanks
>
> My bad, please try this.
>
> Thanks
Thanks Jason. Built 4.13 + supplied patch, I see some decrease in
wakeups, but there's still quite a bit more compared to 4.12
(baseline=4.12, delta1=4.13, delta2=4.13+patch):
client:
2.00% +3.69% +2.55% [kernel.vmlinux] [k] __wake_up_sync_key
server:
1.08% +3.03% +1.85% [kernel.vmlinux] [k] __wake_up_sync_key
Throughput was roughly equivalent to base 4.13 (so, still seeing the
regression w/ this patch applied).
^ permalink raw reply
* [PATCH net] tcp: remove two unused functions
From: Yuchung Cheng @ 2017-09-18 18:05 UTC (permalink / raw)
To: davem; +Cc: netdev, Yuchung Cheng, Neal Cardwell, Eric Dumazet
remove tcp_may_send_now and tcp_snd_test that are no longer used
Fixes: 840a3cbe8969 ("tcp: remove forward retransmit feature")
Signed-off-by: Yuchung Cheng <ycheng@google.com>
Signed-off-by: Neal Cardwell <ncardwell@google.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/net/tcp.h | 1 -
net/ipv4/tcp_output.c | 34 ----------------------------------
2 files changed, 35 deletions(-)
diff --git a/include/net/tcp.h b/include/net/tcp.h
index ada65e767b28..a25c97f13b62 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -543,7 +543,6 @@ u32 tcp_tso_autosize(const struct sock *sk, unsigned int mss_now,
int min_tso_segs);
void __tcp_push_pending_frames(struct sock *sk, unsigned int cur_mss,
int nonagle);
-bool tcp_may_send_now(struct sock *sk);
int __tcp_retransmit_skb(struct sock *sk, struct sk_buff *skb, int segs);
int tcp_retransmit_skb(struct sock *sk, struct sk_buff *skb, int segs);
void tcp_retransmit_timer(struct sock *sk);
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index b7661a68d498..b2d5d1c7ec6d 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1803,40 +1803,6 @@ static bool tcp_snd_wnd_test(const struct tcp_sock *tp,
return !after(end_seq, tcp_wnd_end(tp));
}
-/* This checks if the data bearing packet SKB (usually tcp_send_head(sk))
- * should be put on the wire right now. If so, it returns the number of
- * packets allowed by the congestion window.
- */
-static unsigned int tcp_snd_test(const struct sock *sk, struct sk_buff *skb,
- unsigned int cur_mss, int nonagle)
-{
- const struct tcp_sock *tp = tcp_sk(sk);
- unsigned int cwnd_quota;
-
- tcp_init_tso_segs(skb, cur_mss);
-
- if (!tcp_nagle_test(tp, skb, cur_mss, nonagle))
- return 0;
-
- cwnd_quota = tcp_cwnd_test(tp, skb);
- if (cwnd_quota && !tcp_snd_wnd_test(tp, skb, cur_mss))
- cwnd_quota = 0;
-
- return cwnd_quota;
-}
-
-/* Test if sending is allowed right now. */
-bool tcp_may_send_now(struct sock *sk)
-{
- const struct tcp_sock *tp = tcp_sk(sk);
- struct sk_buff *skb = tcp_send_head(sk);
-
- return skb &&
- tcp_snd_test(sk, skb, tcp_current_mss(sk),
- (tcp_skb_is_last(sk, skb) ?
- tp->nonagle : TCP_NAGLE_PUSH));
-}
-
/* Trim TSO SKB to LEN bytes, put the remaining data into a new packet
* which is put after SKB on the list. It is very much like
* tcp_fragment() except that it may make several kinds of assumptions
--
2.14.1.690.gbb1197296e-goog
^ permalink raw reply related
* Re: [REGRESSION] Warning in tcp_fastretrans_alert() of net/ipv4/tcp_input.c
From: Oleksandr Natalenko @ 2017-09-18 18:04 UTC (permalink / raw)
To: Yuchung Cheng
Cc: Neal Cardwell, David S. Miller, Alexey Kuznetsov,
Hideaki YOSHIFUJI, Netdev
In-Reply-To: <CAK6E8=djbT-35JuHq+hhOKOG+BEO3RCwgsVPoKcDhNBph5OezA@mail.gmail.com>
On pondělí 18. září 2017 20:01:42 CEST Yuchung Cheng wrote:
> Yes since it is disabled in the upstream by default. Although you can
> experiment FACK enabled additionally.
OK.
> Do we know the crash you first experienced is tied to this issue?
No, unfortunately. I wasn't able to re-create it again, so lets focus on
tcp_fastretrans_alert warning only.
^ permalink raw reply
* Re: [REGRESSION] Warning in tcp_fastretrans_alert() of net/ipv4/tcp_input.c
From: Yuchung Cheng @ 2017-09-18 18:01 UTC (permalink / raw)
To: Oleksandr Natalenko
Cc: Neal Cardwell, David S. Miller, Alexey Kuznetsov,
Hideaki YOSHIFUJI, Netdev
In-Reply-To: <40697505.YK5nrFG7Le@natalenko.name>
On Mon, Sep 18, 2017 at 10:59 AM, Oleksandr Natalenko
<oleksandr@natalenko.name> wrote:
> OK. Should I keep FACK disabled?
Yes since it is disabled in the upstream by default. Although you can
experiment FACK enabled additionally.
Do we know the crash you first experienced is tied to this issue?
>
> On pondělí 18. září 2017 19:51:21 CEST Yuchung Cheng wrote:
>> Can you try this patch to verify my theory with tcp_recovery=0 and 1? thanks
>>
>> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
>> index 5af2f04f8859..9253d9ee7d0e 100644
>> --- a/net/ipv4/tcp_input.c
>> +++ b/net/ipv4/tcp_input.c
>> @@ -2381,6 +2381,7 @@ static void tcp_undo_cwnd_reduction(struct sock
>> *sk, bool unmark_loss)
>> }
>> tp->snd_cwnd_stamp = tcp_time_stamp;
>> tp->undo_marker = 0;
>> + WARN_ON(tp->retrans_out);
>> }
>
^ permalink raw reply
* Re: [REGRESSION] Warning in tcp_fastretrans_alert() of net/ipv4/tcp_input.c
From: Oleksandr Natalenko @ 2017-09-18 17:59 UTC (permalink / raw)
To: Yuchung Cheng
Cc: Neal Cardwell, David S. Miller, Alexey Kuznetsov,
Hideaki YOSHIFUJI, Netdev
In-Reply-To: <CAK6E8=ekw7ybf6nwNy7S49ASZBsCS4c_7KR2Bcxq6WiXUKY56w@mail.gmail.com>
OK. Should I keep FACK disabled?
On pondělí 18. září 2017 19:51:21 CEST Yuchung Cheng wrote:
> Can you try this patch to verify my theory with tcp_recovery=0 and 1? thanks
>
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index 5af2f04f8859..9253d9ee7d0e 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -2381,6 +2381,7 @@ static void tcp_undo_cwnd_reduction(struct sock
> *sk, bool unmark_loss)
> }
> tp->snd_cwnd_stamp = tcp_time_stamp;
> tp->undo_marker = 0;
> + WARN_ON(tp->retrans_out);
> }
^ permalink raw reply
* Re: [PATCH 3/3] selftests: silence test output by default
From: Josef Bacik @ 2017-09-18 17:52 UTC (permalink / raw)
To: Shuah Khan
Cc: josef, Josef Bacik, David S. Miller, linux-kernel,
linux-kselftest, netdev, shuah Khan
In-Reply-To: <e8b067d2-9cef-2f3c-78a5-d6630aa765c1@kernel.org>
On Mon, Sep 18, 2017 at 11:46:18AM -0600, Shuah Khan wrote:
> On 09/18/2017 11:37 AM, josef@toxicpanda.com wrote:
> > From: Josef Bacik <jbacik@fb.com>
> >
> > Some of the networking tests are very noisy and make it impossible to
> > see if we actually passed the tests as they run. Default to suppressing
> > the output from any tests run in order to make it easier to track what
> > failed.
> >
> > Signed-off-by: Josef Bacik <jbacik@fb.com>
> > --
>
> This change suppresses pass/fail wrapper output for all tests, not just the
> networking tests.
>
> Could you please send me before and after results for what you are trying
> to fix.
>
Yeah I wanted to suppress extraneous output from everybody, I just happened to
notice it because I was testing net. The default thing already spits out what
it's running and pass/fail, there's no need to include all of the random output
unless the user wants to go and run the test manually. As it is now it's
_impossible_ to tell what ran and what passed/failed because of all the random
output.
Ideally kselftests would work like xfstests does and simply capture the output
to a log so you could go check afterwards, but that's a lot more work. Making
it easier to tell which tests passed/failed is a good enough first step.
Thanks,
Josef
^ permalink raw reply
* Re: [REGRESSION] Warning in tcp_fastretrans_alert() of net/ipv4/tcp_input.c
From: Yuchung Cheng @ 2017-09-18 17:51 UTC (permalink / raw)
To: Oleksandr Natalenko
Cc: Neal Cardwell, David S. Miller, Alexey Kuznetsov,
Hideaki YOSHIFUJI, Netdev
In-Reply-To: <CAK6E8=fF0hMmQ+m4JZPWFFJixYa3Vp5WrQapYu4zq9Zc5eMe_w@mail.gmail.com>
On Mon, Sep 18, 2017 at 10:18 AM, Yuchung Cheng <ycheng@google.com> wrote:
> On Sun, Sep 17, 2017 at 11:43 AM, Oleksandr Natalenko
> <oleksandr@natalenko.name> wrote:
>> Hi.
>>
>> Just to note that it looks like disabling RACK and re-enabling FACK prevents
>> warning from happening:
>>
>> net.ipv4.tcp_fack = 1
>> net.ipv4.tcp_recovery = 0
>>
>> Hope I get semantics of these tunables right.
> Thanks.
>
> One difference between RACK and FACK is that RACK can detect lost
> retransmission in CA_Recovery (fast recovery) and CA_Loss (post RTO)
> mode, while the current FACK can not. A previous FACK version can also
> detect lost retransmission in CA_recovery with limited-transmit. I
> suspect it is RACK's special ability that triggers this warning.
>
> IMO, however, this warning itself is questionably valid: with undo
> (TCP Eifel), the sender can detect and revert a false CA_Recovery /
> CA_Loss to CA_Open, with spurious retransmission in-flight
> (tp->retrans_out > 0). Then another SACK after undo triggers this
> warning. Neal and I are not sure if this is causing the panics you're
> seeing, but personally I'd argue this warning is false, or at least
> should be revised to skip undo case.
Can you try this patch to verify my theory with tcp_recovery=0 and 1? thanks
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 5af2f04f8859..9253d9ee7d0e 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -2381,6 +2381,7 @@ static void tcp_undo_cwnd_reduction(struct sock
*sk, bool unmark_loss)
}
tp->snd_cwnd_stamp = tcp_time_stamp;
tp->undo_marker = 0;
+ WARN_ON(tp->retrans_out);
}
>
>
>>
>> On pátek 15. září 2017 21:04:36 CEST Oleksandr Natalenko wrote:
>>> Hello.
>>>
>>> With net.ipv4.tcp_fack set to 0 the warning still appears:
>>>
>>> ===
>>> » sysctl net.ipv4.tcp_fack
>>> net.ipv4.tcp_fack = 0
>>>
>>> » LC_TIME=C dmesg -T | grep WARNING
>>> [Fri Sep 15 20:40:30 2017] WARNING: CPU: 1 PID: 711 at net/ipv4/tcp_input.c:
>>> 2826 tcp_fastretrans_alert+0x7c8/0x990
>>> [Fri Sep 15 20:40:30 2017] WARNING: CPU: 0 PID: 711 at net/ipv4/tcp_input.c:
>>> 2826 tcp_fastretrans_alert+0x7c8/0x990
>>> [Fri Sep 15 20:48:37 2017] WARNING: CPU: 1 PID: 711 at net/ipv4/tcp_input.c:
>>> 2826 tcp_fastretrans_alert+0x7c8/0x990
>>> [Fri Sep 15 20:48:55 2017] WARNING: CPU: 0 PID: 711 at net/ipv4/tcp_input.c:
>>> 2826 tcp_fastretrans_alert+0x7c8/0x990
>>>
>>> » ps -up 711
>>> USER PID %CPU %MEM VSZ RSS TTY STAT START TIME COMMAND
>>> root 711 4.3 0.0 0 0 ? S 18:12 7:23 [irq/123-
>>> enp3s0]
>>> ===
>>>
>>> Any suggestions?
>>>
>>> On pátek 15. září 2017 16:03:00 CEST Neal Cardwell wrote:
>>> > Thanks for testing that. That is a very useful data point.
>>> >
>>> > I was able to cook up a packetdrill test that could put the connection
>>> > in CA_Disorder with retransmitted packets out, but not in CA_Open. So
>>> > we do not yet have a test case to reproduce this.
>>> >
>>> > We do not see this warning on our fleet at Google. One significant
>>> > difference I see between our environment and yours is that it seems
>>> >
>>> > you run with FACK enabled:
>>> > net.ipv4.tcp_fack = 1
>>> >
>>> > Note that FACK was disabled by default (since it was replaced by RACK)
>>> > between kernel v4.10 and v4.11. And this is exactly the time when this
>>> > bug started manifesting itself for you and some others, but not our
>>> > fleet. So my new working hypothesis would be that this warning is due
>>> > to a behavior that only shows up in kernels >=4.11 when FACK is
>>> > enabled.
>>> >
>>> > Would you be able to disable FACK ("sysctl net.ipv4.tcp_fack=0" at
>>> > boot, or net.ipv4.tcp_fack=0 in /etc/sysctl.conf, or equivalent),
>>> > reboot, and test the kernel for a few days to see if the warning still
>>> > pops up?
>>> >
>>> > thanks,
>>> > neal
>>> >
>>> > [ps: apologies for the previous, mis-formatted post...]
>>
>>
^ permalink raw reply related
* [RFC] endianness issues in drivers/net/ethernet/qlogic/qed
From: Al Viro @ 2017-09-18 17:49 UTC (permalink / raw)
To: netdev; +Cc: Yuval Mintz, David Miller
"qed: Utilize FW 8.10.3.0" has attempted some endianness
annotations in that driver; unfortunately, either annotations are
BS or the driver is genuinely broken on big-endian hosts.
For example, struct init_qm_vport_params is claimed to have
->vport_wfq little-endian 16bit. However, *all* uses are host-endian -
the things like
vport_params[i].vport_wfq = (wfq_speed * QED_WFQ_UNIT) /
min_pf_rate;
and it's not even "... and at some point we convert it to little-endian
in place, or when copying to another instance". It is consistently
host-endian. If that's how it should be, that __le16 is BS; otherwise,
the driver's broken on big-endian.
Another example:
struct qm_rf_pq_map {
__le32 reg;
#define QM_RF_PQ_MAP_PQ_VALID_MASK 0x1
#define QM_RF_PQ_MAP_PQ_VALID_SHIFT 0
#define QM_RF_PQ_MAP_RL_ID_MASK 0xFF
#define QM_RF_PQ_MAP_RL_ID_SHIFT 1
#define QM_RF_PQ_MAP_VP_PQ_ID_MASK 0x1FF
#define QM_RF_PQ_MAP_VP_PQ_ID_SHIFT 9
#define QM_RF_PQ_MAP_VOQ_MASK 0x1F
#define QM_RF_PQ_MAP_VOQ_SHIFT 18
#define QM_RF_PQ_MAP_WRR_WEIGHT_GROUP_MASK 0x3
#define QM_RF_PQ_MAP_WRR_WEIGHT_GROUP_SHIFT 23
#define QM_RF_PQ_MAP_RL_VALID_MASK 0x1
#define QM_RF_PQ_MAP_RL_VALID_SHIFT 25
#define QM_RF_PQ_MAP_RESERVED_MASK 0x3F
#define QM_RF_PQ_MAP_RESERVED_SHIFT 26
};
with instances of that manipulated with
memset(&tx_pq_map, 0, sizeof(tx_pq_map));
SET_FIELD(tx_pq_map.reg, QM_RF_PQ_MAP_PQ_VALID, 1);
SET_FIELD(tx_pq_map.reg,
QM_RF_PQ_MAP_RL_VALID, rl_valid ? 1 : 0);
SET_FIELD(tx_pq_map.reg, QM_RF_PQ_MAP_VP_PQ_ID, first_tx_pq_id);
SET_FIELD(tx_pq_map.reg, QM_RF_PQ_MAP_RL_ID,
rl_valid ?
p_params->pq_params[i].vport_id : 0);
SET_FIELD(tx_pq_map.reg, QM_RF_PQ_MAP_VOQ, voq);
SET_FIELD(tx_pq_map.reg, QM_RF_PQ_MAP_WRR_WEIGHT_GROUP,
p_params->pq_params[i].wrr_group);
/* Write PQ map entry to CAM */
STORE_RT_REG(p_hwfn, QM_REG_TXPQMAP_RT_OFFSET + pq_id,
*((u32 *)&tx_pq_map));
SET_FIELD manipulates the damn thing as *host-endian* - e.g.
SET_FIELD(v, QM_RF_PQ_MAP_RL_ID, n)
would expand to v = (v & ~0x1fe) | ((n & 0xff) << 1). Then we proceed to
pass tx_pq_map.reg (fetched in a bloody convoluted way) to
qed_init_store_rt_reg(), which stores it into p_hwfn->rt_data.init_val[...],
which is apparently host-endian.
So what is this __le32 about?
The really worrying case is this:
/* Binary buffer header */
struct bin_buffer_hdr {
__le32 offset;
__le32 length;
};
int qed_init_fw_data(struct qed_dev *cdev, const u8 *data)
{
struct qed_fw_data *fw = cdev->fw_data;
struct bin_buffer_hdr *buf_hdr;
u32 offset, len;
if (!data) {
DP_NOTICE(cdev, "Invalid fw data\n");
return -EINVAL;
}
/* First Dword contains metadata and should be skipped */
buf_hdr = (struct bin_buffer_hdr *)data;
offset = buf_hdr[BIN_BUF_INIT_FW_VER_INFO].offset;
fw->fw_ver_info = (struct fw_ver_info *)(data + offset);
offset = buf_hdr[BIN_BUF_INIT_CMD].offset;
fw->init_ops = (union init_op *)(data + offset);
We are clearly using those as host-endian here. Yet in _that_
case fixed-endian would appear to make sense, unless we want
separate firmware images for e.g. amd64 and sparc64 hosts...
Another fun one is struct regpair:
p_ramrod->qp_handle_for_cqe.hi = cpu_to_le32(qp->qp_handle.hi);
p_ramrod->qp_handle_for_cqe.lo = cpu_to_le32(qp->qp_handle.lo);
Both sides are struct regpair and by the look of helper macros it *is*
meant to be little-endian. Here, however, one of those (and not
necessary p_ramrod->qp_handle_for_cqe) is host-endian.
Is that driver intended to be used on big-endian hosts at all?
^ permalink raw reply
* Re: [PATCH 3/3] selftests: silence test output by default
From: Shuah Khan @ 2017-09-18 17:46 UTC (permalink / raw)
To: josef
Cc: Josef Bacik, David S. Miller, linux-kernel, linux-kselftest,
netdev, shuah Khan
In-Reply-To: <1505756224-8187-1-git-send-email-jbacik@fb.com>
On 09/18/2017 11:37 AM, josef@toxicpanda.com wrote:
> From: Josef Bacik <jbacik@fb.com>
>
> Some of the networking tests are very noisy and make it impossible to
> see if we actually passed the tests as they run. Default to suppressing
> the output from any tests run in order to make it easier to track what
> failed.
>
> Signed-off-by: Josef Bacik <jbacik@fb.com>
> --
This change suppresses pass/fail wrapper output for all tests, not just the
networking tests.
Could you please send me before and after results for what you are trying
to fix.
> tools/testing/selftests/lib.mk | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
> index 693616651da5..223234cd98e9 100644
> --- a/tools/testing/selftests/lib.mk
> +++ b/tools/testing/selftests/lib.mk
> @@ -24,7 +24,7 @@ define RUN_TESTS
> echo "selftests: Warning: file $$BASENAME_TEST is not executable, correct this.";\
> echo "not ok 1..$$test_num selftests: $$BASENAME_TEST [FAIL]"; \
> else \
> - cd `dirname $$TEST` > /dev/null; (./$$BASENAME_TEST && echo "ok 1..$$test_num selftests: $$BASENAME_TEST [PASS]") || echo "not ok 1..$$test_num selftests: $$BASENAME_TEST [FAIL]"; cd - > /dev/null;\
> + cd `dirname $$TEST` > /dev/null; (./$$BASENAME_TEST > /dev/null 2>&1 && echo "ok 1..$$test_num selftests: $$BASENAME_TEST [PASS]") || echo "not ok 1..$$test_num selftests: $$BASENAME_TEST [FAIL]"; cd - > /dev/null;\
> fi; \
> done;
> endef
> @@ -55,7 +55,7 @@ endif
> define EMIT_TESTS
> @for TEST in $(TEST_GEN_PROGS) $(TEST_PROGS); do \
> BASENAME_TEST=`basename $$TEST`; \
> - echo "(./$$BASENAME_TEST && echo \"selftests: $$BASENAME_TEST [PASS]\") || echo \"selftests: $$BASENAME_TEST [FAIL]\""; \
> + echo "(./$$BASENAME_TEST > /dev/null 2>&1 && echo \"selftests: $$BASENAME_TEST [PASS]\") || echo \"selftests: $$BASENAME_TEST [FAIL]\""; \
> done;
> endef
>
>
thanks,
-- Shuah
^ permalink raw reply
* Re: [PATCH 0/3] fix reuseaddr regression
From: Cole Robinson @ 2017-09-18 17:44 UTC (permalink / raw)
To: josef, davem, netdev, linux-kernel, labbott, kernel-team
In-Reply-To: <1505752137-15522-1-git-send-email-jbacik@fb.com>
On 09/18/2017 12:28 PM, josef@toxicpanda.com wrote:
> I introduced a regression when reworking the fastreuse port stuff that allows
> bind conflicts to occur once a reuseaddr socket successfully opens on an
> existing tb. The root cause is I reversed an if statement which caused us to
> set the tb as if there were no owners on the socket if there were, which
> obviously is not correct.
>
> Dave I have follow up patches that will add a selftest for this case and I ran
> the other reuseport related tests as well. These need to go in pretty quickly
> as it breaks kvm, I've marked them for stable. Sorry for the regression,
>
To clarify, it doesn't really break KVM specifically, but it breaks a
port collision detection idiom that libvirt depends on to successfully
launch qemu/xen/... VMs in certain cases.
Thanks,
Cole
^ permalink raw reply
* [PATCH 3/3] selftests: silence test output by default
From: josef @ 2017-09-18 17:37 UTC (permalink / raw)
Cc: Josef Bacik, Shuah Khan, David S. Miller, linux-kernel,
linux-kselftest, netdev
In-Reply-To: <1505755982-7855-1-git-send-email-jbacik@fb.com>
From: Josef Bacik <jbacik@fb.com>
Some of the networking tests are very noisy and make it impossible to
see if we actually passed the tests as they run. Default to suppressing
the output from any tests run in order to make it easier to track what
failed.
Signed-off-by: Josef Bacik <jbacik@fb.com>
---
tools/testing/selftests/lib.mk | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
index 693616651da5..223234cd98e9 100644
--- a/tools/testing/selftests/lib.mk
+++ b/tools/testing/selftests/lib.mk
@@ -24,7 +24,7 @@ define RUN_TESTS
echo "selftests: Warning: file $$BASENAME_TEST is not executable, correct this.";\
echo "not ok 1..$$test_num selftests: $$BASENAME_TEST [FAIL]"; \
else \
- cd `dirname $$TEST` > /dev/null; (./$$BASENAME_TEST && echo "ok 1..$$test_num selftests: $$BASENAME_TEST [PASS]") || echo "not ok 1..$$test_num selftests: $$BASENAME_TEST [FAIL]"; cd - > /dev/null;\
+ cd `dirname $$TEST` > /dev/null; (./$$BASENAME_TEST > /dev/null 2>&1 && echo "ok 1..$$test_num selftests: $$BASENAME_TEST [PASS]") || echo "not ok 1..$$test_num selftests: $$BASENAME_TEST [FAIL]"; cd - > /dev/null;\
fi; \
done;
endef
@@ -55,7 +55,7 @@ endif
define EMIT_TESTS
@for TEST in $(TEST_GEN_PROGS) $(TEST_PROGS); do \
BASENAME_TEST=`basename $$TEST`; \
- echo "(./$$BASENAME_TEST && echo \"selftests: $$BASENAME_TEST [PASS]\") || echo \"selftests: $$BASENAME_TEST [FAIL]\""; \
+ echo "(./$$BASENAME_TEST > /dev/null 2>&1 && echo \"selftests: $$BASENAME_TEST [PASS]\") || echo \"selftests: $$BASENAME_TEST [FAIL]\""; \
done;
endef
--
2.7.4
^ permalink raw reply related
* [PATCH 2/3] selftests: actually run the various net selftests
From: josef @ 2017-09-18 17:32 UTC (permalink / raw)
Cc: Josef Bacik, Shuah Khan, David S. Miller, linux-kernel,
linux-kselftest, netdev
In-Reply-To: <1505755982-7855-1-git-send-email-jbacik@fb.com>
From: Josef Bacik <jbacik@fb.com>
These self tests are just self contained binaries, they are not run by
any of the scripts in the directory. This means they need to be marked
with TEST_GEN_PROGS to actually be run, not TEST_GEN_FILES.
Signed-off-by: Josef Bacik <jbacik@fb.com>
---
tools/testing/selftests/net/Makefile | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index 3df542c84610..45a4e77a47c4 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -6,8 +6,8 @@ CFLAGS += -I../../../../usr/include/
TEST_PROGS := run_netsocktests run_afpackettests test_bpf.sh netdevice.sh rtnetlink.sh
TEST_GEN_FILES = socket
TEST_GEN_FILES += psock_fanout psock_tpacket
-TEST_GEN_FILES += reuseport_bpf reuseport_bpf_cpu reuseport_bpf_numa
-TEST_GEN_FILES += reuseport_dualstack msg_zerocopy reuseaddr_conflict
+TEST_GEN_PROGS += reuseport_bpf reuseport_bpf_cpu reuseport_bpf_numa
+TEST_GEN_PROGS += reuseport_dualstack msg_zerocopy reuseaddr_conflict
include ../lib.mk
--
2.7.4
^ permalink raw reply related
* [PATCH 1/3] selftest: add a reuseaddr test
From: josef @ 2017-09-18 17:32 UTC (permalink / raw)
Cc: Josef Bacik, Shuah Khan, David S. Miller, linux-kernel,
linux-kselftest, netdev
From: Josef Bacik <jbacik@fb.com>
This is to test for a regression introduced by
b9470c27607b ("inet: kill smallest_size and smallest_port")
which introduced a problem with reuseaddr and bind conflicts.
Signed-off-by: Josef Bacik <jbacik@fb.com>
---
tools/testing/selftests/net/.gitignore | 1 +
tools/testing/selftests/net/Makefile | 2 +-
tools/testing/selftests/net/reuseaddr_conflict.c | 114 +++++++++++++++++++++++
3 files changed, 116 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/net/reuseaddr_conflict.c
diff --git a/tools/testing/selftests/net/.gitignore b/tools/testing/selftests/net/.gitignore
index 9801253e4802..c612d6e38c62 100644
--- a/tools/testing/selftests/net/.gitignore
+++ b/tools/testing/selftests/net/.gitignore
@@ -6,3 +6,4 @@ reuseport_bpf
reuseport_bpf_cpu
reuseport_bpf_numa
reuseport_dualstack
+reuseaddr_conflict
diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index de1f5772b878..3df542c84610 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -7,7 +7,7 @@ TEST_PROGS := run_netsocktests run_afpackettests test_bpf.sh netdevice.sh rtnetl
TEST_GEN_FILES = socket
TEST_GEN_FILES += psock_fanout psock_tpacket
TEST_GEN_FILES += reuseport_bpf reuseport_bpf_cpu reuseport_bpf_numa
-TEST_GEN_FILES += reuseport_dualstack msg_zerocopy
+TEST_GEN_FILES += reuseport_dualstack msg_zerocopy reuseaddr_conflict
include ../lib.mk
diff --git a/tools/testing/selftests/net/reuseaddr_conflict.c b/tools/testing/selftests/net/reuseaddr_conflict.c
new file mode 100644
index 000000000000..7c5b12664b03
--- /dev/null
+++ b/tools/testing/selftests/net/reuseaddr_conflict.c
@@ -0,0 +1,114 @@
+/*
+ * Test for the regression introduced by
+ *
+ * b9470c27607b ("inet: kill smallest_size and smallest_port")
+ *
+ * If we open an ipv4 socket on a port with reuseaddr we shouldn't reset the tb
+ * when we open the ipv6 conterpart, which is what was happening previously.
+ */
+#include <errno.h>
+#include <error.h>
+#include <arpa/inet.h>
+#include <netinet/in.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <sys/socket.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+#define PORT 9999
+
+int open_port(int ipv6, int any)
+{
+ int fd = -1;
+ int reuseaddr = 1;
+ int v6only = 1;
+ int addrlen;
+ int ret = -1;
+ struct sockaddr *addr;
+ int family = ipv6 ? AF_INET6 : AF_INET;
+
+ struct sockaddr_in6 addr6 = {
+ .sin6_family = AF_INET6,
+ .sin6_port = htons(PORT),
+ .sin6_addr = in6addr_any
+ };
+ struct sockaddr_in addr4 = {
+ .sin_family = AF_INET,
+ .sin_port = htons(PORT),
+ .sin_addr.s_addr = any ? htonl(INADDR_ANY) : inet_addr("127.0.0.1"),
+ };
+
+
+ if (ipv6) {
+ addr = (struct sockaddr*)&addr6;
+ addrlen = sizeof(addr6);
+ } else {
+ addr = (struct sockaddr*)&addr4;
+ addrlen = sizeof(addr4);
+ }
+
+ if ((fd = socket(family, SOCK_STREAM, IPPROTO_TCP)) < 0) {
+ perror("socket");
+ goto out;
+ }
+
+ if (ipv6 && setsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY, (void*)&v6only,
+ sizeof(v6only)) < 0) {
+ perror("setsockopt IPV6_V6ONLY");
+ goto out;
+ }
+
+ if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &reuseaddr,
+ sizeof(reuseaddr)) < 0) {
+ perror("setsockopt SO_REUSEADDR");
+ goto out;
+ }
+
+ if (bind(fd, addr, addrlen) < 0) {
+ perror("bind");
+ goto out;
+ }
+
+ if (any)
+ return fd;
+
+ if (listen(fd, 1) < 0) {
+ perror("listen");
+ goto out;
+ }
+ return fd;
+out:
+ close(fd);
+ return ret;
+}
+
+int main(void)
+{
+ int listenfd;
+ int fd1, fd2;
+
+ fprintf(stderr, "Opening 127.0.0.1:%d\n", PORT);
+ listenfd = open_port(0, 0);
+ if (listenfd < 0)
+ error(1, errno, "Couldn't open listen socket");
+ fprintf(stderr, "Opening INADDR_ANY:%d\n", PORT);
+ fd1 = open_port(0, 1);
+ if (fd1 >= 0)
+ error(1, 0, "Was allowed to create an ipv4 reuseport on a already bound non-reuseport socket");
+ fprintf(stderr, "Opening in6addr_any:%d\n", PORT);
+ fd1 = open_port(1, 1);
+ if (fd1 < 0)
+ error(1, errno, "Couldn't open ipv6 reuseport");
+ fprintf(stderr, "Opening INADDR_ANY:%d\n", PORT);
+ fd2 = open_port(0, 1);
+ if (fd2 >= 0)
+ error(1, 0, "Was allowed to create an ipv4 reuseport on a already bound non-reuseport socket");
+ close(fd1);
+ fprintf(stderr, "Opening INADDR_ANY:%d after closing ipv6 socket\n", PORT);
+ fd1 = open_port(0, 1);
+ if (fd1 >= 0)
+ error(1, 0, "Was allowed to create an ipv4 reuseport on an already bound non-reuseport socket with no ipv6");
+ fprintf(stderr, "Success");
+ return 0;
+}
--
2.7.4
^ permalink raw reply related
* RE: [Intel-wired-lan] [PATCH] fm10k: Fix misuse of net_ratelimit()
From: Singh, Krishneil K @ 2017-09-18 17:22 UTC (permalink / raw)
To: Joe Perches, Kirsher, Jeffrey T
Cc: netdev@vger.kernel.org, intel-wired-lan@lists.osuosl.org,
linux-kernel@vger.kernel.org
In-Reply-To: <83d6fcab6a180d7006c8eb396c9aa3169dc6a84d.1502468221.git.joe@perches.com>
> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On Behalf
> Of Joe Perches
> Sent: Friday, August 11, 2017 9:17 AM
> To: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>
> Cc: netdev@vger.kernel.org; intel-wired-lan@lists.osuosl.org; linux-
> kernel@vger.kernel.org
> Subject: [Intel-wired-lan] [PATCH] fm10k: Fix misuse of net_ratelimit()
>
> Correct the backward logic using !net_ratelimit()
>
> Miscellanea:
>
> o Add a blank line before the error return label
>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
Tested-by: Krishneil Singh <krishneil.k.singh@intel.com>
^ permalink raw reply
* Re: [REGRESSION] Warning in tcp_fastretrans_alert() of net/ipv4/tcp_input.c
From: Yuchung Cheng @ 2017-09-18 17:18 UTC (permalink / raw)
To: Oleksandr Natalenko
Cc: Neal Cardwell, David S. Miller, Alexey Kuznetsov,
Hideaki YOSHIFUJI, Netdev
In-Reply-To: <22474097.Jky8MxLkJU@natalenko.name>
On Sun, Sep 17, 2017 at 11:43 AM, Oleksandr Natalenko
<oleksandr@natalenko.name> wrote:
> Hi.
>
> Just to note that it looks like disabling RACK and re-enabling FACK prevents
> warning from happening:
>
> net.ipv4.tcp_fack = 1
> net.ipv4.tcp_recovery = 0
>
> Hope I get semantics of these tunables right.
Thanks.
One difference between RACK and FACK is that RACK can detect lost
retransmission in CA_Recovery (fast recovery) and CA_Loss (post RTO)
mode, while the current FACK can not. A previous FACK version can also
detect lost retransmission in CA_recovery with limited-transmit. I
suspect it is RACK's special ability that triggers this warning.
IMO, however, this warning itself is questionably valid: with undo
(TCP Eifel), the sender can detect and revert a false CA_Recovery /
CA_Loss to CA_Open, with spurious retransmission in-flight
(tp->retrans_out > 0). Then another SACK after undo triggers this
warning. Neal and I are not sure if this is causing the panics you're
seeing, but personally I'd argue this warning is false, or at least
should be revised to skip undo case.
>
> On pátek 15. září 2017 21:04:36 CEST Oleksandr Natalenko wrote:
>> Hello.
>>
>> With net.ipv4.tcp_fack set to 0 the warning still appears:
>>
>> ===
>> » sysctl net.ipv4.tcp_fack
>> net.ipv4.tcp_fack = 0
>>
>> » LC_TIME=C dmesg -T | grep WARNING
>> [Fri Sep 15 20:40:30 2017] WARNING: CPU: 1 PID: 711 at net/ipv4/tcp_input.c:
>> 2826 tcp_fastretrans_alert+0x7c8/0x990
>> [Fri Sep 15 20:40:30 2017] WARNING: CPU: 0 PID: 711 at net/ipv4/tcp_input.c:
>> 2826 tcp_fastretrans_alert+0x7c8/0x990
>> [Fri Sep 15 20:48:37 2017] WARNING: CPU: 1 PID: 711 at net/ipv4/tcp_input.c:
>> 2826 tcp_fastretrans_alert+0x7c8/0x990
>> [Fri Sep 15 20:48:55 2017] WARNING: CPU: 0 PID: 711 at net/ipv4/tcp_input.c:
>> 2826 tcp_fastretrans_alert+0x7c8/0x990
>>
>> » ps -up 711
>> USER PID %CPU %MEM VSZ RSS TTY STAT START TIME COMMAND
>> root 711 4.3 0.0 0 0 ? S 18:12 7:23 [irq/123-
>> enp3s0]
>> ===
>>
>> Any suggestions?
>>
>> On pátek 15. září 2017 16:03:00 CEST Neal Cardwell wrote:
>> > Thanks for testing that. That is a very useful data point.
>> >
>> > I was able to cook up a packetdrill test that could put the connection
>> > in CA_Disorder with retransmitted packets out, but not in CA_Open. So
>> > we do not yet have a test case to reproduce this.
>> >
>> > We do not see this warning on our fleet at Google. One significant
>> > difference I see between our environment and yours is that it seems
>> >
>> > you run with FACK enabled:
>> > net.ipv4.tcp_fack = 1
>> >
>> > Note that FACK was disabled by default (since it was replaced by RACK)
>> > between kernel v4.10 and v4.11. And this is exactly the time when this
>> > bug started manifesting itself for you and some others, but not our
>> > fleet. So my new working hypothesis would be that this warning is due
>> > to a behavior that only shows up in kernels >=4.11 when FACK is
>> > enabled.
>> >
>> > Would you be able to disable FACK ("sysctl net.ipv4.tcp_fack=0" at
>> > boot, or net.ipv4.tcp_fack=0 in /etc/sysctl.conf, or equivalent),
>> > reboot, and test the kernel for a few days to see if the warning still
>> > pops up?
>> >
>> > thanks,
>> > neal
>> >
>> > [ps: apologies for the previous, mis-formatted post...]
>
>
^ permalink raw reply
* RE: [Intel-wired-lan] [PATCH] fm10k: Use seq_putc() in fm10k_dbg_desc_break()
From: Singh, Krishneil K @ 2017-09-18 16:59 UTC (permalink / raw)
To: SF Markus Elfring, intel-wired-lan@lists.osuosl.org,
netdev@vger.kernel.org, Kirsher, Jeffrey T
Cc: kernel-janitors@vger.kernel.org, LKML
In-Reply-To: <4f8686c6-76cb-3d12-934e-6af67b6f084b@users.sourceforge.net>
> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On Behalf
> Of SF Markus Elfring
> Sent: Monday, May 8, 2017 9:18 AM
> To: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; Kirsher, Jeffrey T
> <jeffrey.t.kirsher@intel.com>
> Cc: kernel-janitors@vger.kernel.org; LKML <linux-kernel@vger.kernel.org>
> Subject: [Intel-wired-lan] [PATCH] fm10k: Use seq_putc() in
> fm10k_dbg_desc_break()
>
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Mon, 8 May 2017 18:10:39 +0200
>
> Two single characters should be put into a sequence.
> Thus use the corresponding function "seq_putc".
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
Tested-by: Krishneil Singh <krishneil.k.singh@intel.com>
^ permalink raw reply
* [PATCH] vhost: remove unneeded linux/miscdevice.h include
From: Corentin Labbe @ 2017-09-18 16:58 UTC (permalink / raw)
To: mst, jasowang; +Cc: kvm, virtualization, netdev, linux-kernel, Corentin Labbe
drivers/vhost/vhost.c does not use any miscdevice, so this patch
remove this unnecessary inclusion.
Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
drivers/vhost/vhost.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 33ac2b186b85..33ab839696f9 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -16,7 +16,6 @@
#include <linux/uio.h>
#include <linux/mm.h>
#include <linux/mmu_context.h>
-#include <linux/miscdevice.h>
#include <linux/mutex.h>
#include <linux/poll.h>
#include <linux/file.h>
--
2.13.5
^ permalink raw reply related
* [PATCH v4 net 3/3] lan78xx: Use default values loaded from EEPROM/OTP after reset
From: Nisar Sayed @ 2017-09-18 22:02 UTC (permalink / raw)
To: davem; +Cc: UNGLinuxDriver, netdev
In-Reply-To: <20170918220225.3767-1-Nisar.Sayed@microchip.com>
Use default value of auto duplex and auto speed values loaded
from EEPROM/OTP after reset. The LAN78xx allows platform
configurations to be loaded from EEPROM/OTP.
Ex: When external phy is connected, the MAC can be configured to
have correct auto speed, auto duplex, auto polarity configured
from the EEPROM/OTP.
Fixes: 55d7de9de6c3 ("Microchip's LAN7800 family USB 2/3 to 10/100/1000 Ethernet device driver")
Signed-off-by: Nisar Sayed <Nisar.Sayed@microchip.com>
---
drivers/net/usb/lan78xx.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index 3292f56ffe02..0fc3c19d5aef 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -2449,7 +2449,6 @@ static int lan78xx_reset(struct lan78xx_net *dev)
/* LAN7801 only has RGMII mode */
if (dev->chipid == ID_REV_CHIP_ID_7801_)
buf &= ~MAC_CR_GMII_EN_;
- buf |= MAC_CR_AUTO_DUPLEX_ | MAC_CR_AUTO_SPEED_;
ret = lan78xx_write_reg(dev, MAC_CR, buf);
ret = lan78xx_read_reg(dev, MAC_TX, &buf);
--
2.14.1
^ permalink raw reply related
* [PATCH v4 net 2/3] lan78xx: Allow EEPROM write for less than MAX_EEPROM_SIZE
From: Nisar Sayed @ 2017-09-18 22:02 UTC (permalink / raw)
To: davem; +Cc: UNGLinuxDriver, netdev
In-Reply-To: <20170918220225.3767-1-Nisar.Sayed@microchip.com>
Allow EEPROM write for less than MAX_EEPROM_SIZE
Fixes: 55d7de9de6c3 ("Microchip's LAN7800 family USB 2/3 to 10/100/1000 Ethernet device driver")
Signed-off-by: Nisar Sayed <Nisar.Sayed@microchip.com>
---
drivers/net/usb/lan78xx.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index fcf85ae37435..3292f56ffe02 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -1290,11 +1290,10 @@ static int lan78xx_ethtool_set_eeprom(struct net_device *netdev,
if (ret)
return ret;
- /* Allow entire eeprom update only */
- if ((ee->magic == LAN78XX_EEPROM_MAGIC) &&
- (ee->offset == 0) &&
- (ee->len == 512) &&
- (data[0] == EEPROM_INDICATOR))
+ /* Invalid EEPROM_INDICATOR at offset zero will result in fail to
+ * load data from EEPROM
+ */
+ if (ee->magic == LAN78XX_EEPROM_MAGIC)
ret = lan78xx_write_raw_eeprom(dev, ee->offset, ee->len, data);
else if ((ee->magic == LAN78XX_OTP_MAGIC) &&
(ee->offset == 0) &&
--
2.14.1
^ permalink raw reply related
* [PATCH v4 net 1/3] lan78xx: Fix for eeprom read/write when device auto suspend
From: Nisar Sayed @ 2017-09-18 22:02 UTC (permalink / raw)
To: davem; +Cc: UNGLinuxDriver, netdev
In-Reply-To: <20170918220225.3767-1-Nisar.Sayed@microchip.com>
Fix for eeprom read/write when device auto suspend
Fixes: 55d7de9de6c3 ("Microchip's LAN7800 family USB 2/3 to 10/100/1000 Ethernet device driver")
Signed-off-by: Nisar Sayed <Nisar.Sayed@microchip.com>
---
drivers/net/usb/lan78xx.c | 24 ++++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index b99a7fb09f8e..fcf85ae37435 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -1265,30 +1265,46 @@ static int lan78xx_ethtool_get_eeprom(struct net_device *netdev,
struct ethtool_eeprom *ee, u8 *data)
{
struct lan78xx_net *dev = netdev_priv(netdev);
+ int ret;
+
+ ret = usb_autopm_get_interface(dev->intf);
+ if (ret)
+ return ret;
ee->magic = LAN78XX_EEPROM_MAGIC;
- return lan78xx_read_raw_eeprom(dev, ee->offset, ee->len, data);
+ ret = lan78xx_read_raw_eeprom(dev, ee->offset, ee->len, data);
+
+ usb_autopm_put_interface(dev->intf);
+
+ return ret;
}
static int lan78xx_ethtool_set_eeprom(struct net_device *netdev,
struct ethtool_eeprom *ee, u8 *data)
{
struct lan78xx_net *dev = netdev_priv(netdev);
+ int ret;
+
+ ret = usb_autopm_get_interface(dev->intf);
+ if (ret)
+ return ret;
/* Allow entire eeprom update only */
if ((ee->magic == LAN78XX_EEPROM_MAGIC) &&
(ee->offset == 0) &&
(ee->len == 512) &&
(data[0] == EEPROM_INDICATOR))
- return lan78xx_write_raw_eeprom(dev, ee->offset, ee->len, data);
+ ret = lan78xx_write_raw_eeprom(dev, ee->offset, ee->len, data);
else if ((ee->magic == LAN78XX_OTP_MAGIC) &&
(ee->offset == 0) &&
(ee->len == 512) &&
(data[0] == OTP_INDICATOR_1))
- return lan78xx_write_raw_otp(dev, ee->offset, ee->len, data);
+ ret = lan78xx_write_raw_otp(dev, ee->offset, ee->len, data);
- return -EINVAL;
+ usb_autopm_put_interface(dev->intf);
+
+ return ret;
}
static void lan78xx_get_strings(struct net_device *netdev, u32 stringset,
--
2.14.1
^ permalink raw reply related
* [PATCH v4 net 0/3] lan78xx: This series of patches are for lan78xx driver.
From: Nisar Sayed @ 2017-09-18 22:02 UTC (permalink / raw)
To: davem; +Cc: UNGLinuxDriver, netdev
This series of patches are for lan78xx driver.
These patches fixes potential issues associated with lan78xx driver.
v4
- Updated changes to handle return values as per comments
- Updated EEPROM write handling as per comments
v3
- Updated chagnes as per comments
v2
- Added patch version information
- Added fixes tag
- Updated patch description
- Updated chagnes as per comments
v1
- Splitted patches as per comments
- Dropped "fixed_phy device support" and "Fix for system suspend" changes
Nisar Sayed (3):
Fix for eeprom read/write when device auto suspend
Allow EEPROM write for less than MAX_EEPROM_SIZE
Use default values loaded from EEPROM/OTP after reset
drivers/net/usb/lan78xx.c | 34 ++++++++++++++++++++++++----------
1 file changed, 24 insertions(+), 10 deletions(-)
--
2.14.1
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox