Skip to contentSkip to conversation

tcp: handle EOR and FIN conditions the same in tcp_tso_should_defer()

PATCH Applied Eric Dumazet 3 participants 3 messages Dec 10, 2018

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 tcp: handle EOR and FIN conditions the same in tcp_tso_should_defer()

In commit f9bfe4e6a9d0 ("tcp: lack of available data can also cause TSO defer") we moved the test in tcp_tso_should_defer() for packets with a FIN flag, and we mentioned that the same would be done later for EOR flag.

Both flags should be handled at the same time, after all other heuristics have been considered. They both mean that no more bytes can be added to this skb by an application.

---
 net/ipv4/tcp_output.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)
net/ipv4/tcp_output.c +2 -5
index c31badfee806a70d5af6dc033cf9155adb1f9375..730bc44dbad9363814705b28c2f91a2253d91207 100644
@@ -1947,10 +1947,6 @@ static bool tcp_tso_should_defer(struct sock *sk, struct sk_buff *skb,
 	if ((skb != tcp_write_queue_tail(sk)) && (limit >= skb->len))
 		goto send_now;
 
-	/* If this packet won't get more data, do not wait. */
-	if (TCP_SKB_CB(skb)->eor)
-		goto send_now;
-
 	win_divisor = READ_ONCE(sock_net(sk)->ipv4.sysctl_tcp_tso_win_divisor);
 	if (win_divisor) {
 		u32 chunk = min(tp->snd_wnd, tp->snd_cwnd * tp->mss_cache);
@@ -1999,7 +1995,8 @@ static bool tcp_tso_should_defer(struct sock *sk, struct sk_buff *skb,
 	}
 
 	/* If this packet won't get more data, do not wait. */
-	if (TCP_SKB_CB(skb)->tcp_flags & TCPHDR_FIN)
+	if ((TCP_SKB_CB(skb)->tcp_flags & TCPHDR_FIN) ||
+	    TCP_SKB_CB(skb)->eor)
 		goto send_now;
 
 	return true;
Signature · 1 line
2.20.0.rc2.403.gdbc3b29805-goog
Acked-by on v1 1/1
#1 lore raw
Quoted 11 lines · Eric Dumazet

In commit f9bfe4e6a9d0 ("tcp: lack of available data can also cause TSO defer") we moved the test in tcp_tso_should_defer() for packets with a FIN flag, and we mentioned that the same would be done later for EOR flag.

Both flags should be handled at the same time, after all other heuristics have been considered. They both mean that no more bytes can be added to this skb by an application.

Thank you Eric for the patch!

Quoted 32 lines
---
 net/ipv4/tcp_output.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)
net/ipv4/tcp_output.c +2 -6
index c31badfee806a70d5af6dc033cf9155adb1f9375..730bc44dbad9363814705b28c2f91a2253d91207 100644
@@ -1947,10 +1947,6 @@ static bool tcp_tso_should_defer(struct sock *sk, struct sk_buff *skb,
        if ((skb != tcp_write_queue_tail(sk)) && (limit >= skb->len))
                goto send_now;
 
-       /* If this packet won't get more data, do not wait. */
-       if (TCP_SKB_CB(skb)->eor)
-               goto send_now;
-
        win_divisor = READ_ONCE(sock_net(sk)->ipv4.sysctl_tcp_tso_win_divisor);
        if (win_divisor) {
                u32 chunk = min(tp->snd_wnd, tp->snd_cwnd * tp->mss_cache);
@@ -1999,7 +1995,8 @@ static bool tcp_tso_should_defer(struct sock *sk, struct sk_buff *skb,
        }
 
        /* If this packet won't get more data, do not wait. */
-       if (TCP_SKB_CB(skb)->tcp_flags & TCPHDR_FIN)
+       if ((TCP_SKB_CB(skb)->tcp_flags & TCPHDR_FIN) ||
+           TCP_SKB_CB(skb)->eor)
                goto send_now;
 
        return true;
--

2.20.0.rc2.403.gdbc3b29805-goog

maintainer Applied
#2 lore raw
In reply to Eric Dumazet

From: Eric Dumazet <[email protected]>
Date: Mon, 10 Dec 2018 06:10:02 -0800

Quoted 10 lines

In commit f9bfe4e6a9d0 ("tcp: lack of available data can also cause TSO defer") we moved the test in tcp_tso_should_defer() for packets with a FIN flag, and we mentioned that the same would be done later for EOR flag.

Both flags should be handled at the same time, after all other heuristics have been considered. They both mean that no more bytes can be added to this skb by an application.

Applied, thanks Eric.