multipath-tcp / mptcp

⚠️⚠️⚠️ Deprecated 🚫 Out-of-tree Linux Kernel implementation of MultiPath TCP. 👉 Use https://github.com/multipath-tcp/mptcp_net-next repo instead ⚠️⚠️⚠️
https://github.com/multipath-tcp/mptcp_net-next
Other
890 stars 335 forks source link

tcp advertised window issue #231

Closed madhanraj closed 6 years ago

madhanraj commented 6 years ago

Facing the issue MPTCP v0.93 while using WiFi 802.11ac MPTCP uses more skb memory than TCP. The truesize is more than rmem/2, hence the advertised window at max reaches rmem/4

In detail : Truesize : 2944 skb->len: 1472

    if (tcp_win_from_space(skb->truesize) <= skb->len)
        incr = 2 * meta_tp->advmss;
    else
        incr = __tcp_grow_window(meta_sk, skb);

so 1472 < 1448 is false Hence it always enters __tcp_grow_window but the incr value would be 0

We tried three workaround 1) changing the tcp_adv_win_scale value . Which would inturn increase tcp_win_from_space. This worked well . However, not a proper solution

2) changing the cb in the struct sk_buff { From 80 to 72 . which worked . However could be problem with other WiFi drivers (may be 802.11n)

3) in struct tcp_skb_cb{

we tried moving the mptcp options inside the union union { struct { / There is space for up to 24 bytes / __u32 in_flight:30,/ Bytes in flight at transmit / is_app_limited:1, / cwnd not fully used? / unused:1; / pkts S/ACKed so far upon tx of skb, incl retrans: / __u32 delivered; / start of send pipeline phase / struct skb_mstamp first_tx_mstamp; / when we reached the "delivered" count / struct skb_mstamp delivered_mstamp; } tx; / only used for outgoing skbs / union { struct inet_skb_parm h4;

if IS_ENABLED(CONFIG_IPV6)

struct inet6_skb_parm h6;

endif

} header; / For incoming skbs /

ifdef CONFIG_MPTCP

union { / For MPTCP outgoing frames / __u32 path_mask; / paths that tried to send this skb / __u32 dss[6]; / DSS options / };

endif

}

This inturn can reduce the cb further down (may be 56) which would be proper . Testing under progress

Any pointers on this issue would be helpful .

PS: This is done for patched android device 4.9 kernel.

cpaasch commented 6 years ago

Hi,

one question is: why is skb->truesize 2944 ?

What driver is this? 2944 looks a bit too high to me. skb_shared_info is 320 bytes, and sk_buff is around 250, and the data fits in a 2048 byte allocated memory region.

Then, why does __tcp_grow_window returns 0?

Then, in your example, you say truesize is 2944 and skb->len is 1448. In that case, reducing skb->cb from 80 to 72 would give you a truesize of 2936 (thus tcp_win_from_space will be 1468). Then, the condition tcp_win_from_space(skb->truesize) <= skb->len will still be false - so the problem is not solved then.

As for reducing to 72 - that depends on the kernel config. I guess you CONFIG_IPV6_MIP6 disabled ?

In general - yes we need to reduce the size of the struct tcp_skb_cb down to its original 48 bytes. However, the way you do it with the union won't work, because you are overwriting fields like in_flight.

madhanraj commented 6 years ago

one question is: why is skb->truesize 2944 ?

This normal WiFi BCM packet flow in the android 4.9 (connected over 802.11ac )

Then, why does __tcp_grow_window returns 0?

while (tp->rcv_ssthresh <= window) { // <- this condition is false hence returns 0 after rmem[2]/4 if (truesize <= skb->len) return 2 * inet_csk(sk)->icsk_ack.rcv_mss; truesize >>= 1; window >>= 1; } return 0;

Then, in your example, you say truesize is 2944 and skb->len is 1448. In that case, reducing skb->cb from 80

cb[80] to cb[72] which reduced truesize from 1472 to 1408 (1472-8*8),

As for reducing to 72 - that depends on the kernel config. I guess you CONFIG_IPV6_MIP6 disabled ?

CONFIG_IPV6_MIP6 is enabled in my setup .

cpaasch commented 6 years ago

one question is: why is skb->truesize 2944 ?

This normal WiFi BCM packet flow in the android 4.9 (connected over 802.11ac )

What is the driver?

Then, why does __tcp_grow_window returns 0?

while (tp->rcv_ssthresh <= window) { // <- this condition is false hence returns 0 after rmem[2]/4 if (truesize <= skb->len) return 2 * inet_csk(sk)->icsk_ack.rcv_mss; truesize >>= 1; window >>= 1; } return 0;

What is the value of tp->rcv_ssthresh in your experiments? Can you do some test-runs and print the values of rcv_ssthresh, window, truesize and skb->len ?

Then, in your example, you say truesize is 2944 and skb->len is 1448. In that case, reducing skb->cb from 80

cb[80] to cb[72] which reduced truesize from 1472 to 1408 (1472-8*8),

Hmm... Why do you multiply by 8 here?

As for reducing to 72 - that depends on the kernel config. I guess you CONFIG_IPV6_MIP6 disabled ? CONFIG_IPV6_MIP6 is enabled in my setup .

I see - If you can send a patch or pull-request to bring it back down to 72, that would be great.

cpaasch commented 6 years ago

Btw., I just tried out 72 bytes for skb->cb. Indeed, it works on v0.93, but not on mptcp_trunk :-(

madhanraj commented 6 years ago

Will send the patch and the trace soon .. missed to see this

cpaasch commented 6 years ago

72 is too small for mptcp_trunk though

madhanraj commented 6 years ago

Added additional logs . This is the place where the window growth is struck.

<7>[ 763.544762] [6: Thread-3:28505] 1.0 __tcp_grow_window Entered <7>[ 763.544765] [6: Thread-3:28505] 1.1 __tcp_grow_window truesize:736 ,window:1048576 ,rc_ssthresh:1051248 ,skb_len 1448,skb->truesize 2944 ,tcp_rmem 4194304 <7>[ 763.544768] [6: Thread-3:28505] 1.4 __tcp_grow_window return 0 <7>[ 763.544770] [6: Thread-3:28505] 2.2 tcp_grow_window incr:0 <7>[ 763.544773] [6: Thread-3:28505] 2.3 tcp_grow_window port:51974 rcv_ssthresh:1051248 window_clamp:1272792 <7>[ 763.544775] [6: Thread-3:28505] 2.4 tcp_grow_window Exited
madhanraj commented 6 years ago

72 is too small for mptcp_trunk though

Yup rite ktime_t has been introduced, which makes the structure compact. i.e previously there is some padding. However, in the trunk, there is no extra padding as it is compactly packed structure..! How do we handle this..!

cpaasch commented 6 years ago

@madhanraj can you still reply to my other question:

cb[80] to cb[72] which reduced truesize from 1472 to 1408 (1472-8*8),

Hmm... Why do you multiply by 8 here?

Yup rite ktime_t has been introduced, which makes the structure compact. i.e previously there is some padding. However, in the trunk, there is no extra padding as it is compactly packed structure..! How do we handle this..!

We need to reduce the overhead. At the moment, there is no easy solution to reduce tcp_skb_cb. Another path you can try is to remove kernel-configs that are bloating the skb (e.g., CONFIG_XFRM, CONFIG_NETWORK_SECMARK,...)

madhanraj commented 6 years ago

I misunderstood, it to be 8*8 (due to aligned(8) padding).

However, this is logs after making the CB to 72

<4>[ 1253.876945] I[4: swapper/4: 0] m+++ sysctl_tcp_adv_win_scale = 1 space = 4194304 <7>[ 1253.876952] I[4: swapper/4: 0] port:44248 recv_wnd:2067584[tcp_select_window] <7>[ 1161.191442] [0:AgifCallService:14640] 1.1 __tcp_grow_window window:1048576 ,rc_ssthresh:87600 ,skb->truesize 2816 ,tcp_rmem 4194304 Truesize prev : 2944 , current true size 2816 hence the overhead i.e [space - (space>>sysctl_tcp_adv_win_scale)] is changed from 1472 to 1408.. will dig in more .. As of now my problem is solved, by changing to 72.
cpaasch commented 6 years ago

Ok. We know that we need to find a solution for the bloating of the sk_buff. We will have to address this soon.