On 21. 07. 22, 22:44, Wei Wang wrote:
This reverts commit 4a41f453bedfd5e9cd040bad509d9da49feb3e2c.
This to-be-reverted commit was meant to apply a stricter rule for the
stack to enter pingpong mode. However, the condition used to check for
interactive session "before(tp->lsndtime, icsk->icsk_ack.lrcvtime)" is
jiffy based and might be too coarse, which delays the stack entering
pingpong mode.
We revert this patch so that we no longer use the above condition to
determine interactive session, and also reduce pingpong threshold to 1.
Fixes: 4a41f453bedf ("tcp: change pingpong threshold to 3")
Reported-by: LemmyHuang <hlm3280@xxxxxxx>
Suggested-by: Neal Cardwell <ncardwell@xxxxxxxxxx>
Signed-off-by: Wei Wang <weiwan@xxxxxxxxxx>
This breaks python-eventlet [1] (and was backported to stable trees):
________________ TestHttpd.test_018b_http_10_keepalive_framing
_________________
self = <tests.wsgi_test.TestHttpd
testMethod=test_018b_http_10_keepalive_framing>
def test_018b_http_10_keepalive_framing(self):
# verify that if an http/1.0 client sends connection: keep-alive
# that we don't mangle the request framing if the app doesn't
read the request
def app(environ, start_response):
resp_body = {
'/1': b'first response',
'/2': b'second response',
'/3': b'third response',
}.get(environ['PATH_INFO'])
if resp_body is None:
resp_body = 'Unexpected path: ' + environ['PATH_INFO']
if six.PY3:
resp_body = resp_body.encode('latin1')
# Never look at wsgi.input!
start_response('200 OK', [('Content-type', 'text/plain')])
return [resp_body]
self.site.application = app
sock = eventlet.connect(self.server_addr)
req_body = b'GET /tricksy HTTP/1.1\r\n'
body_len = str(len(req_body)).encode('ascii')
sock.sendall(b'PUT /1 HTTP/1.0\r\nHost:
localhost\r\nConnection: keep-alive\r\n'
b'Content-Length: ' + body_len + b'\r\n\r\n' +
req_body)
result1 = read_http(sock)
self.assertEqual(b'first response', result1.body)
self.assertEqual(result1.headers_original.get('Connection'),
'keep-alive')
sock.sendall(b'PUT /2 HTTP/1.0\r\nHost:
localhost\r\nConnection: keep-alive\r\n'
b'Content-Length: ' + body_len + b'\r\nExpect:
100-continue\r\n\r\n')
# Client may have a short timeout waiting on that 100 Continue
# and basically immediately send its body
sock.sendall(req_body)
result2 = read_http(sock)
self.assertEqual(b'second response', result2.body)
self.assertEqual(result2.headers_original.get('Connection'),
'close')
> sock.sendall(b'PUT /3 HTTP/1.0\r\nHost:
localhost\r\nConnection: close\r\n\r\n')
tests/wsgi_test.py:648:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
_ _ _ _
eventlet/greenio/base.py:407: in sendall
tail = self.send(data, flags)
eventlet/greenio/base.py:401: in send
return self._send_loop(self.fd.send, data, flags)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
_ _ _ _
self = <eventlet.greenio.base.GreenSocket object at 0x7f5f2f73c9a0>
send_method = <built-in method send of socket object at 0x7f5f2f73d520>
data = b'PUT /3 HTTP/1.0\r\nHost: localhost\r\nConnection: close\r\n\r\n'
args = (0,), _timeout_exc = timeout('timed out'), eno = 32
def _send_loop(self, send_method, data, *args):
if self.act_non_blocking:
return send_method(data, *args)
_timeout_exc = socket_timeout('timed out')
while True:
try:
> return send_method(data, *args)
E BrokenPipeError: [Errno 32] Broken pipe
eventlet/greenio/base.py:388: BrokenPipeError
====================
Reverting this revert on the top of 5.19 solves the issue.
Any ideas?
[1] https://github.com/eventlet/eventlet
---
v2: added Fixes tag
---
include/net/inet_connection_sock.h | 10 +---------
net/ipv4/tcp_output.c | 15 ++++++---------
2 files changed, 7 insertions(+), 18 deletions(-)
diff --git a/include/net/inet_connection_sock.h b/include/net/inet_connection_sock.h
index 85cd695e7fd1..ee88f0f1350f 100644
--- a/include/net/inet_connection_sock.h
+++ b/include/net/inet_connection_sock.h
@@ -321,7 +321,7 @@ void inet_csk_update_fastreuse(struct inet_bind_bucket *tb,
struct dst_entry *inet_csk_update_pmtu(struct sock *sk, u32 mtu);
-#define TCP_PINGPONG_THRESH 3
+#define TCP_PINGPONG_THRESH 1
static inline void inet_csk_enter_pingpong_mode(struct sock *sk)
{
@@ -338,14 +338,6 @@ static inline bool inet_csk_in_pingpong_mode(struct sock *sk)
return inet_csk(sk)->icsk_ack.pingpong >= TCP_PINGPONG_THRESH;
}
-static inline void inet_csk_inc_pingpong_cnt(struct sock *sk)
-{
- struct inet_connection_sock *icsk = inet_csk(sk);
-
- if (icsk->icsk_ack.pingpong < U8_MAX)
- icsk->icsk_ack.pingpong++;
-}
-
static inline bool inet_csk_has_ulp(struct sock *sk)
{
return inet_sk(sk)->is_icsk && !!inet_csk(sk)->icsk_ulp_ops;
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index c38e07b50639..d06e72e141ac 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -167,16 +167,13 @@ static void tcp_event_data_sent(struct tcp_sock *tp,
if (tcp_packets_in_flight(tp) == 0)
tcp_ca_event(sk, CA_EVENT_TX_START);
- /* If this is the first data packet sent in response to the
- * previous received data,
- * and it is a reply for ato after last received packet,
- * increase pingpong count.
- */
- if (before(tp->lsndtime, icsk->icsk_ack.lrcvtime) &&
- (u32)(now - icsk->icsk_ack.lrcvtime) < icsk->icsk_ack.ato)
- inet_csk_inc_pingpong_cnt(sk);
-
tp->lsndtime = now;
+
+ /* If it is a reply for ato after last received
+ * packet, enter pingpong mode.
+ */
+ if ((u32)(now - icsk->icsk_ack.lrcvtime) < icsk->icsk_ack.ato)
+ inet_csk_enter_pingpong_mode(sk);
}
/* Account for an ACK we sent. */
thanks,
--
js
suse labs
