| 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 linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <ca408271-8730-eb2b-f12e-3f66df2e643a@kernel.org>
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