lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Sat, 6 Aug 2022 12:02:01 +0200 From: Jiri Slaby <jirislaby@...nel.org> To: Wei Wang <weiwan@...gle.com>, David Miller <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org Cc: Soheil Hassas Yeganeh <soheil@...gle.com>, Yuchung Cheng <ycheng@...gle.com>, LemmyHuang <hlm3280@....com>, Neal Cardwell <ncardwell@...gle.com>, stable <stable@...r.kernel.org> Subject: Re: [PATCH net v2] Revert "tcp: change pingpong threshold to 3" 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@....com> > Suggested-by: Neal Cardwell <ncardwell@...gle.com> > Signed-off-by: Wei Wang <weiwan@...gle.com> 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
Powered by blists - more mailing lists