| Message ID | 2245b9d01fa91d84a8af1e17e1b97f0763f1b792.1388114391.git.panweiping3@gmail.com |
|---|---|
| State | Rejected, archived |
| Delegated to: | David Miller |
| Headers | show |
On Fri, 2013-12-27 at 11:20 +0800, Weiping Pan wrote: > Commit 4e4f1fc22681(tcp: properly increase rcv_ssthresh for ofo packets) can > grow receive window for out of order packets, > but for in order packets, we only call tcp_grow_window() if skb->len >= 128, > I think we should add the same condition for out of order packets. > Why do you think so ? Do you have any experimental data to share ? I personally think the basic test in tcp_event_data_recv() is redundant with the logic in tcp_grow_window() skb->len < 128 seems a poor test, real factor is skb->len/skb->truesize ratio. Note: Please always CC a patch author when you mention a commit in a changelog. Thanks -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 12/27/2013 02:11 PM, Eric Dumazet wrote: > On Fri, 2013-12-27 at 11:20 +0800, Weiping Pan wrote: >> Commit 4e4f1fc22681(tcp: properly increase rcv_ssthresh for ofo packets) can >> grow receive window for out of order packets, >> but for in order packets, we only call tcp_grow_window() if skb->len >= 128, >> I think we should add the same condition for out of order packets. >> > Why do you think so ? Do you have any experimental data to share ? Hi, Eric, No. I am just wondering why you not treat out of order and in order packets in the same way, so I raised this question. > > I personally think the basic test in tcp_event_data_recv() is redundant > with the logic in tcp_grow_window() I think the original purpose of this test is to reduce the number of times of modifying rcv_ssthresh, then to avoid SWS. And it can help reduce CPU cycles waisted in tcp_grow_window() since it is on the critical path for in order packets. > > skb->len < 128 seems a poor test, real factor is skb->len/skb->truesize > ratio. Yes. But since we already have "if (tcp_win_from_space(skb->truesize) <= skb->len)" in tcp_grow_window(), I do not know how to choose another test, or the logic in tcp_grow_window() is enough and we can skip that test in tcp_event_data_recv(). > > Note: Please always CC a patch author when you mention a commit in > a changelog. Ok, I will remember that. thanks Weiping Pan > > Thanks > > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c index c53b7f3..2272774 100644 --- a/net/ipv4/tcp_input.c +++ b/net/ipv4/tcp_input.c @@ -343,6 +343,9 @@ static void tcp_grow_window(struct sock *sk, const struct sk_buff *skb) { struct tcp_sock *tp = tcp_sk(sk); + if (skb->len < 128) + return; + /* Check #1 */ if (tp->rcv_ssthresh < tp->window_clamp && (int)tp->rcv_ssthresh < tcp_space(sk) && @@ -654,8 +657,7 @@ static void tcp_event_data_recv(struct sock *sk, struct sk_buff *skb) TCP_ECN_check_ce(tp, skb); - if (skb->len >= 128) - tcp_grow_window(sk, skb); + tcp_grow_window(sk, skb); } /* Called to compute a smoothed rtt estimate. The data fed to this
Commit 4e4f1fc22681(tcp: properly increase rcv_ssthresh for ofo packets) can grow receive window for out of order packets, but for in order packets, we only call tcp_grow_window() if skb->len >= 128, I think we should add the same condition for out of order packets. Signed-off-by: Weiping Pan <panweiping3@gmail.com> --- net/ipv4/tcp_input.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-)