[PATCH 1/1] tap: RCU usage and comment fixes

[Wang Jian]

The tap_queue and the 'tap_dev' are loosely coupled, not 'macvlan_dev'.

Taking rcu_read_lock a little later seems can slightly reduce rcu read critical 
section.

Signed-off-by: Wang Jian 
---
 drivers/net/tap.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/tap.c b/drivers/net/tap.c
index f0f7cd977667..e5e5a8e4a60d 100644
--- a/drivers/net/tap.c
+++ b/drivers/net/tap.c
@@ -125,7 +125,7 @@ static struct tap_dev *tap_dev_get_rcu(const struct 
net_device *dev)
 
 /*
  * RCU usage:
- * The tap_queue and the macvlan_dev are loosely coupled, the
+ * The tap_queue and the tap_dev are loosely coupled, the
  * pointers from one to the other can only be read while rcu_read_lock
  * or rtnl is held.
  *
@@ -720,8 +720,6 @@ static ssize_t tap_get_user(struct tap_queue *q, struct 
msghdr *m,
__vlan_get_protocol(skb, skb->protocol, ) != 0)
skb_set_network_header(skb, depth);
 
-   rcu_read_lock();
-   tap = rcu_dereference(q->tap);
/* copy skb_ubuf_info for callback when skb has no error */
if (zerocopy) {
skb_shinfo(skb)->destructor_arg = m->msg_control;
@@ -732,6 +730,8 @@ static ssize_t tap_get_user(struct tap_queue *q, struct 
msghdr *m,
uarg->callback(uarg, false);
}
 
+   rcu_read_lock();
+   tap = rcu_dereference(q->tap);
if (tap) {
skb->dev = tap->dev;
dev_queue_xmit(skb);
-- 
2.17.1

[PATCH 1/1] tcp: tcp_mtup_probe -> tcp_mtu_probe

[Wang Jian]

Comment correction.

Signed-off-by: Jian Wang 
---
 net/ipv4/tcp_input.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 355d3df..fd7b766 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -2862,7 +2862,7 @@ static void tcp_fastretrans_alert(struct sock
*sk, const u32 prior_snd_una,
icsk->icsk_mtup.probe_size &&
tp->snd_una == tp->mtu_probe.probe_seq_start) {
tcp_mtup_probe_failed(sk);
-   /* Restores the reduction we did in tcp_mtup_probe() */
+   /* Restores the reduction we did in tcp_mtu_probe() */
tp->snd_cwnd++;
tcp_simple_retransmit(sk);
return;


-- 
Regards,
Wang Jian

[PATCH 1/1] selftest: check tunnel type more accurately

[Wang Jian]

Grep tunnel type directly to make sure 'ip' command supports it.

Signed-off-by: Jian Wang 
---
 tools/testing/selftests/bpf/test_tunnel.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/test_tunnel.sh
b/tools/testing/selftests/bpf/test_tunnel.sh
index aeb2901..c4b5fbb 100755
--- a/tools/testing/selftests/bpf/test_tunnel.sh
+++ b/tools/testing/selftests/bpf/test_tunnel.sh
@@ -668,7 +668,7 @@ cleanup_exit()

 check()
 {
-   ip link help $1 2>&1 | grep -q "^Usage:"
+   ip link help 2>&1 | grep -q "\s$1\s"
if [ $? -ne 0 ];then
echo "SKIP $1: iproute2 not support"
cleanup

Re: One question about __tcp_select_window()

[Wang Jian]

Thanks for your reply, Eric.

Actually, this is a query about the code while I am reading code.
>From my instinct and the comment, I think we should choose the bigger
one but maybe I miss something(like your said, autotuning)
Anyway, I will read more codes and do more tests.

Thanks.

On Tue, Apr 17, 2018 at 10:43 PM, Eric Dumazet <eric.duma...@gmail.com> wrote:
>
>
> On 04/17/2018 06:53 AM, Wang Jian wrote:
>> I test the fix with 4.17.0-rc1+ and it seems work.
>>
>> 1. iperf -c IP -i 20 -t 60 -w 1K
>>  with-fix vs without-fix : 1.15Gbits/sec vs 1.05Gbits/sec
>> I also try other windows and have similar results.
>>
>> 2. Use tcp probe trace snd_wind.
>> with-fix vs without-fix: 1245568 vs 1042816
>>
>> 3. I don't see extra retransmit/drops.
>>
>
> Unfortunately I have no idea what exact problem you had to solve.
>
> Setting small windows is not exactly the path we are taking.
>
> And I do not know how many side effects your change will have for 'standard' 
> flows
> using autotuning or sane windows.
>

Re: One question about __tcp_select_window()

[Wang Jian]

I test the fix with 4.17.0-rc1+ and it seems work.

1. iperf -c IP -i 20 -t 60 -w 1K
 with-fix vs without-fix : 1.15Gbits/sec vs 1.05Gbits/sec
I also try other windows and have similar results.

2. Use tcp probe trace snd_wind.
with-fix vs without-fix: 1245568 vs 1042816

3. I don't see extra retransmit/drops.

On Sun, Apr 15, 2018 at 8:50 PM, Wang Jian <jianjian.wa...@gmail.com> wrote:
> Hi all,
>
> While I read __tcp_select_window() code, I find that it maybe return a
> smaller window.
> Below is one scenario I thought, may be not right:
> In function __tcp_select_window(), assume:
> full_space is 6mss, free_space is 2mss, tp->rcv_wnd is 3MSS.
> And assume disable window scaling, then
> window = tp->rcv_wnd > free_space && window > free_space
> then it will round down free_space and return it.
>
> Is this expected behavior? The comment is also saying
> "Get the largest window that is a nice multiple of mss."
>
> Should we do something like below ? Or I miss something?
> I don't know how to verify it now.
>
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -2680,9 +2680,9 @@ u32 __tcp_select_window(struct sock *sk)
>  * We also don't do any window rounding when the free space
>  * is too small.
>  */
> -   if (window <= free_space - mss || window > free_space)
> +   if (window <= free_space - mss)
> window = rounddown(free_space, mss);
> -   else if (mss == full_space &&
> +   else if (window <= free_space && mss == full_space &&
>  free_space > window + (full_space >> 1))
> window = free_space;
> }
>
> Thanks.

One question about __tcp_select_window()

[Wang Jian]

Hi all,

While I read __tcp_select_window() code, I find that it maybe return a
smaller window.
Below is one scenario I thought, may be not right:
In function __tcp_select_window(), assume:
full_space is 6mss, free_space is 2mss, tp->rcv_wnd is 3MSS.
And assume disable window scaling, then
window = tp->rcv_wnd > free_space && window > free_space
then it will round down free_space and return it.

Is this expected behavior? The comment is also saying
"Get the largest window that is a nice multiple of mss."

Should we do something like below ? Or I miss something?
I don't know how to verify it now.

--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -2680,9 +2680,9 @@ u32 __tcp_select_window(struct sock *sk)
 * We also don't do any window rounding when the free space
 * is too small.
 */
-   if (window <= free_space - mss || window > free_space)
+   if (window <= free_space - mss)
window = rounddown(free_space, mss);
-   else if (mss == full_space &&
+   else if (window <= free_space && mss == full_space &&
 free_space > window + (full_space >> 1))
window = free_space;
}

Thanks.

Re: [PATCH] pktgen: use dynamic allocation for debug print buffer

[Wang Jian]

>> +  kfree(buf);
free tb? buf is an array.

On Wed, Mar 14, 2018 at 8:25 AM, David Miller  wrote:
> From: Arnd Bergmann 
> Date: Tue, 13 Mar 2018 21:58:39 +0100
>
>> After the removal of the VLA, we get a harmless warning about a large
>> stack frame:
>>
>> net/core/pktgen.c: In function 'pktgen_if_write':
>> net/core/pktgen.c:1710:1: error: the frame size of 1076 bytes is larger than 
>> 1024 bytes [-Werror=frame-larger-than=]
>>
>> The function was previously shown to be safe despite hitting
>> the 1024 bye warning level. To get rid of the annoyging warning,
>> while keeping it readable, this changes it to use strndup_user().
>>
>> Obviously this is not a fast path, so the kmalloc() overhead
>> can be disregarded.
>>
>> Fixes: 35951393bbff ("pktgen: Remove VLA usage")
>> Signed-off-by: Arnd Bergmann 
>
> Applied, thanks.

where can I find netdev2.2 slides?

[Wang Jian]

Hi all,
Sorry for broadcast.
Just a query, where can I find netdev2.2 slides?
Seems they are not uploaded to netdev websites.