From mboxrd@z Thu Jan  1 00:00:00 1970
Authentication-Results: passt.top; dmarc=pass (p=none dis=none) header.from=redhat.com
Authentication-Results: passt.top;
	dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=Llhsi+yP;
	dkim-atps=neutral
Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124])
	by passt.top (Postfix) with ESMTPS id 840DD5A0274
	for <passt-dev@passt.top>; Mon, 20 Jan 2025 18:28:53 +0100 (CET)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;
	s=mimecast20190719; t=1737394132;
	h=from:from:reply-to:subject:subject:date:date:message-id:message-id:
	 to:to:cc:cc:mime-version:mime-version:content-type:content-type:
	 content-transfer-encoding:content-transfer-encoding:
	 in-reply-to:in-reply-to:references:references;
	bh=/K1ADfFFqnVqvlozMJRBlLGD/aPF7oeTDNog1894XsI=;
	b=Llhsi+yP11u4n1oQF/NHl8cvrdwiii78gz+D+O0Rovgqyzl697RSbBP2P7pSr5DVEqxyQY
	Lmh7BkmOlWquf+JC/H69rF3X74ovNoW6bMr26NDD+93qdWHQqIMFKj3Fn82nw74p7T/yj2
	U6gkWdpzpAvxFkPTxeylSONBupsvFG8=
Received: from mail-wm1-f72.google.com (mail-wm1-f72.google.com
 [209.85.128.72]) by relay.mimecast.com with ESMTP with STARTTLS
 (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id
 us-mta-178-cUiwmIK3NruMvt_NShCkpA-1; Mon, 20 Jan 2025 12:28:50 -0500
X-MC-Unique: cUiwmIK3NruMvt_NShCkpA-1
X-Mimecast-MFC-AGG-ID: cUiwmIK3NruMvt_NShCkpA
Received: by mail-wm1-f72.google.com with SMTP id 5b1f17b1804b1-436379713baso23080545e9.2
        for <passt-dev@passt.top>; Mon, 20 Jan 2025 09:28:50 -0800 (PST)
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
        d=1e100.net; s=20230601; t=1737394128; x=1737998928;
        h=content-transfer-encoding:mime-version:organization:references
         :in-reply-to:message-id:subject:cc:to:from:date:x-gm-message-state
         :from:to:cc:subject:date:message-id:reply-to;
        bh=/K1ADfFFqnVqvlozMJRBlLGD/aPF7oeTDNog1894XsI=;
        b=hCy8AsTGR28lp2MIj+pp6VGyf90LKowIUZzZUdccCqBRHC7s/IJroXytT3af/6xGdy
         I73LiHVC16YQeGamvTXaHEYQakrIKy+6L6d69Sg3k+/IDpU3OkOjKZsqaxMK2foRbgv2
         4RxNroKyz3WLgMNbrs8yhx1ZZAh8fnNL7E+6FlAaR5ajN2S2CiIVSDxH+tkVpSlONLCd
         quO8Nq4zymG1B/0qypG2Lh1UaeEyQBvAR8+Abq+/PHyBjg6CXLA4thlAMLX6W5vfhALM
         0+wHUVC/RCKi6CUw1v3uvsCnqSCxL1sqm4HQPbFVAHSONxeyj1ky5vdWf+ewQkYmD1sd
         Farg==
X-Gm-Message-State: AOJu0Yw0eFE21n4Pm0KNVXhGgmdFs+KCnKDdSzfeM61Vc0aoxMQmejfB
	sGaAQdV1xh1EEah8gxhgmxANUtRqXYaXB5L0fKNzy3CY1FJgbaYYgeG9UOhl6MYLh1P0CsF2Kja
	y4PfaQnrKISQ7JAgzxNdU5rEneq1RkHOS5iY5tYuPjI9f2KtuoZSBXuNyIg==
X-Gm-Gg: ASbGncu85lR5z/qPW5YO5jmkQFjUQLNLW5XU3/NZ93xHX9fK0bfSUmpizFd9XLhTsn9
	Q2IsBzBG/7Slm7t9kr3gfiMzPEpW83+YwSlTzidP4WhiUgA4pU4i2Kc7LKR/brMc542Z5D1VOjF
	9uxGHF0xhgK5Bl3274IcbOcm/1uhgyCjDX6WcySky78Fv/Sg7y17M+zfMVHjUfPwTuaIrdjkXC+
	A4YTwAFmAqJtAHTK1bOO6JO/n7QjP5iXDMGFcNpd2Ebs+lA7uZVl1+uRgU/LuETMjL6JML3gL1k
	yCbwIA==
X-Received: by 2002:a05:600c:4f43:b0:434:a1d3:a326 with SMTP id 5b1f17b1804b1-438913bf07dmr126469145e9.6.1737394128281;
        Mon, 20 Jan 2025 09:28:48 -0800 (PST)
X-Google-Smtp-Source: AGHT+IFytCNfl5nmQlu1XXmKrvYwh91fyRXLF7s62JET122niGGQcJVQaBEfke+URMrQ+tYuOv65eg==
X-Received: by 2002:a05:600c:4f43:b0:434:a1d3:a326 with SMTP id 5b1f17b1804b1-438913bf07dmr126468925e9.6.1737394127862;
        Mon, 20 Jan 2025 09:28:47 -0800 (PST)
Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [2a10:fc81:a806:d6a9::1])
        by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-437c16c4c12sm107350165e9.3.2025.01.20.09.28.47
        (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);
        Mon, 20 Jan 2025 09:28:47 -0800 (PST)
Date: Mon, 20 Jan 2025 18:28:45 +0100
From: Stefano Brivio <sbrivio@redhat.com>
To: David Gibson <david@gibson.dropbear.id.au>
Subject: Re: [PATCH] tcp: Disable Nagle's algorithm (set TCP_NODELAY) on all
 sockets
Message-ID: <20250120182845.734918ae@elisabeth>
In-Reply-To: <Z44SpZhgYEVH9qTw@zatzit>
References: <20250117093405.1253554-1-sbrivio@redhat.com>
	<Z44SpZhgYEVH9qTw@zatzit>
Organization: Red Hat
X-Mailer: Claws Mail 4.2.0 (GTK 3.24.41; x86_64-pc-linux-gnu)
MIME-Version: 1.0
X-Mimecast-Spam-Score: 0
X-Mimecast-MFC-PROC-ID: pztlftC93HoOgrI-U_KvM7_IUQvSV_kqTSqwuFywvKg_1737394129
X-Mimecast-Originator: redhat.com
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Message-ID-Hash: E25F2DEVEBETIAD5JP43Q3BULR7LF3CG
X-Message-ID-Hash: E25F2DEVEBETIAD5JP43Q3BULR7LF3CG
X-MailFrom: sbrivio@redhat.com
X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header
CC: passt-dev@passt.top
X-Mailman-Version: 3.3.8
Precedence: list
List-Id: Development discussion and patches for passt <passt-dev.passt.top>
Archived-At: <https://archives.passt.top/passt-dev/20250120182845.734918ae@elisabeth/>
Archived-At: <https://passt.top/hyperkitty/list/passt-dev@passt.top/message/E25F2DEVEBETIAD5JP43Q3BULR7LF3CG/>
List-Archive: <https://archives.passt.top/passt-dev/>
List-Archive: <https://passt.top/hyperkitty/list/passt-dev@passt.top/>
List-Help: <mailto:passt-dev-request@passt.top?subject=help>
List-Owner: <mailto:passt-dev-owner@passt.top>
List-Post: <mailto:passt-dev@passt.top>
List-Subscribe: <mailto:passt-dev-join@passt.top>
List-Unsubscribe: <mailto:passt-dev-leave@passt.top>

On Mon, 20 Jan 2025 19:38:53 +1030
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Fri, Jan 17, 2025 at 10:34:05AM +0100, Stefano Brivio wrote:
> > Following up on 725acd111ba3 ("tcp_splice: Set (again) TCP_NODELAY on
> > both sides"), David argues that, in general, we don't know what kind
> > of TCP traffic we're dealing with, on any side or path.
> > 
> > TCP segments might have been delivered to our socket with a PSH flag,
> > but we don't have a way to know about it.
> > 
> > Similarly, the guest might send us segments with PSH or URG set, but
> > we don't know if we should generally TCP_CORK sockets and uncork on
> > those flags, because that would assume they're running a Linux kernel
> > (and a particular version of it) matching the kernel that delivers
> > outbound packets for us.
> > 
> > Given that we can't make any assumption and everything might very well
> > be interactive traffic, disable Nagle's algorithm on all non-spliced
> > sockets as well.
> > 
> > After all, John Nagle himself is nowadays recommending that delayed
> > ACKs should never be enabled together with his algorithm, but we
> > don't have a practical way to ensure that our environment is free from
> > delayed ACKs (TCP_QUICKACK is not really usable for this purpose):
> > 
> >   https://news.ycombinator.com/item?id=34180239
> > 
> > Suggested-by: David Gibson <david@gibson.dropbear.id.au>
> > Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
> > ---
> >  tcp.c | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> > 
> > diff --git a/tcp.c b/tcp.c
> > index 3b3193a..c570f42 100644
> > --- a/tcp.c
> > +++ b/tcp.c
> > @@ -756,6 +756,19 @@ static void tcp_sock_set_bufsize(const struct ctx *c, int s)
> >  		trace("TCP: failed to set SO_SNDBUF to %i", v);
> >  }
> >  
> > +/**
> > + * tcp_sock_set_nodelay() - Set TCP_NODELAY option (disable Nagle's algorithm)
> > + * @s:		Socket, can be -1 to avoid check in the caller
> > + */
> > +static void tcp_sock_set_nodelay(int s)
> > +{
> > +	if (s == -1)
> > +		return;
> > +
> > +	if (setsockopt(s, SOL_TCP, TCP_NODELAY, &((int){ 1 }), sizeof(int)))
> > +		trace("TCP: failed to set TCP_NODELAY on socket %i", s);  
> 
> I think this deserves something a little stronger than trace().  It
> might have a significant impact on latency, and if we get a failure
> enabling a feature that's been in TCP longer than Linux has existed,
> something pretty weird is going on.

Hm, right, changed to debug(). For more than that, we'd need to have a
print-once option.

> > +}
> > +
> >  /**
> >   * tcp_update_csum() - Calculate TCP checksum
> >   * @psum:	Unfolded partial checksum of the IPv4 or IPv6 pseudo-header
> > @@ -1285,6 +1298,7 @@ static int tcp_conn_new_sock(const struct ctx *c, sa_family_t af)
> >  		return -errno;
> >  
> >  	tcp_sock_set_bufsize(c, s);
> > +	tcp_sock_set_nodelay(s);
> >  
> >  	return s;
> >  }
> > @@ -2261,6 +2275,8 @@ static int tcp_sock_init_one(const struct ctx *c, const union inany_addr *addr,
> >  		return s;
> >  
> >  	tcp_sock_set_bufsize(c, s);
> > +	tcp_sock_set_nodelay(s);
> > +  
> 
> This is a listening socket, not an accepted or connecting socket.
> Does NODELAY even mean anything there?
> 
> >  	return s;
> >  }
> >  
> > @@ -2322,6 +2338,8 @@ static void tcp_ns_sock_init4(const struct ctx *c, in_port_t port)
> >  	else
> >  		s = -1;
> >  
> > +	tcp_sock_set_nodelay(s);  
> 
> Same here.
> 
> >  	if (c->tcp.fwd_out.mode == FWD_AUTO)
> >  		tcp_sock_ns[port][V4] = s;
> >  }
> > @@ -2348,6 +2366,8 @@ static void tcp_ns_sock_init6(const struct ctx *c, in_port_t port)
> >  	else
> >  		s = -1;
> >  
> > +	tcp_sock_set_nodelay(s);  
> 
> And here.
> 
> >  	if (c->tcp.fwd_out.mode == FWD_AUTO)
> >  		tcp_sock_ns[port][V6] = s;
> >  }  
> 
> Do accept()ed sockets inherit NODELAY from the listening socket?  Or
> should we instead be setting NODELAY after the accept4() in
> tcp_listen_handler()?  In fact.. even if they do inhereit, would it be
> simpler to just set it at accept() time?

Oops. I just followed the example of tcp_sock_set_bufsize(), but it
turns out that neither buffer sizes nor TCP_NODELAY is inherited from
listening sockets!

Sending a patch for that, and a v2 of this one.

-- 
Stefano