From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202412 header.b=ovvaXHHj; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 235E95A0274 for ; Mon, 20 Jan 2025 10:09:20 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202412; t=1737364135; bh=cMSDyIR1HsDKuE+ZiptYSirwvC19VETsUTtjlsmJotI=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=ovvaXHHjZHjiclEiPkXappNiNSJBj264f+nmDNMZGO2n83jocar+7mZ17lw9VVT1O CbARDL+Lc2WtbuOO9fVcsB/odH52q8tr18pz2/KOiCJKru5srz0ZyH4R038UK7vD92 djjJPC6KwJt6DL8qCrykbAjJknaEZ+5q79yV3HnyyRuviX7SZdLJvHWDn+QcY4SM9y +cyivSqdxcVjg6HQGxSNBL7SulBCmOWgBqMOC5wzn2RcDF0F9CpCF969wtNrJyHLNN Tcn2YjV9HNHsPsyWHbIAiInV0PPASbWXg55/rFoXNs2VKEsaYmipTDCOMiC4+B9JwW ii20SK9f+ZIgw== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4Yc4Jl5M5Zz4x2c; Mon, 20 Jan 2025 20:08:55 +1100 (AEDT) Date: Mon, 20 Jan 2025 19:38:53 +1030 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH] tcp: Disable Nagle's algorithm (set TCP_NODELAY) on all sockets Message-ID: References: <20250117093405.1253554-1-sbrivio@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="2zv1+rRzkqrs9Rdg" Content-Disposition: inline In-Reply-To: <20250117093405.1253554-1-sbrivio@redhat.com> Message-ID-Hash: VXJ2WPDXLOQ5BL5KC2RSKO7FBZGPMM4Y X-Message-ID-Hash: VXJ2WPDXLOQ5BL5KC2RSKO7FBZGPMM4Y X-MailFrom: dgibson@gandalf.ozlabs.org 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 Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --2zv1+rRzkqrs9Rdg Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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. >=20 > TCP segments might have been delivered to our socket with a PSH flag, > but we don't have a way to know about it. >=20 > 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. >=20 > 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. >=20 > 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): >=20 > https://news.ycombinator.com/item?id=3D34180239 >=20 > Suggested-by: David Gibson > Signed-off-by: Stefano Brivio > --- > tcp.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) >=20 > 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); > } > =20 > +/** > + * tcp_sock_set_nodelay() - Set TCP_NODELAY option (disable Nagle's algo= rithm) > + * @s: Socket, can be -1 to avoid check in the caller > + */ > +static void tcp_sock_set_nodelay(int s) > +{ > + if (s =3D=3D -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. > +} > + > /** > * 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, s= a_family_t af) > return -errno; > =20 > tcp_sock_set_bufsize(c, s); > + tcp_sock_set_nodelay(s); > =20 > return s; > } > @@ -2261,6 +2275,8 @@ static int tcp_sock_init_one(const struct ctx *c, c= onst union inany_addr *addr, > return s; > =20 > 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; > } > =20 > @@ -2322,6 +2338,8 @@ static void tcp_ns_sock_init4(const struct ctx *c, = in_port_t port) > else > s =3D -1; > =20 > + tcp_sock_set_nodelay(s); Same here. > if (c->tcp.fwd_out.mode =3D=3D FWD_AUTO) > tcp_sock_ns[port][V4] =3D s; > } > @@ -2348,6 +2366,8 @@ static void tcp_ns_sock_init6(const struct ctx *c, = in_port_t port) > else > s =3D -1; > =20 > + tcp_sock_set_nodelay(s); And here. > if (c->tcp.fwd_out.mode =3D=3D FWD_AUTO) > tcp_sock_ns[port][V6] =3D 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? --=20 David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson --2zv1+rRzkqrs9Rdg Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmeOEo0ACgkQzQJF27ox 2GffYw/+Nra7VctkdWzT+MatAOA1i5O3DDky0JwWkGRH4DzezKo1fbpMoZy6A5Ie S9iwlO0sX5Yb1Fgm0BeAnjr74LK8m6oy7lfbywi1vCqBkkXeUtf05yCdcaHQIfVf 9y9gLJuyn9D+Nqh0ccQ/iJpum0Emxmogm/cbMzU7kPjbbDZo5QnbsVziXkBZ+WYK Udyh6Mbdc6lObdDCqhjLBLbx/m394EvdLm3V5cEmb9mEoEBHxIQA/2o6M+Q59yzS G8vWP/yiQdiFQHRYWG2NJzwrp8f6r8KVI5BKjx0/S8qHbsmvwwAI7O5gjeKnIu96 etyKaUxIq6Ng+ZR96mtaoHuuF/B6FSfUBGm5XM5ANiheFpUOyd6NBV0j8I8ih6+h 05cP4T0nvs7/Aqmf5Vdgps0ASAwtlQu1bq55cScDtMrSVKTPJPinM7x3Mz9O6z0S 1MSZDs5yG9vEqHXN2W/kO1OPgiOVVFr/1Ced7cM2LEmDgJjTt3eyIzFo6E+aHuxb 9iMfhdXO5f613D2M32O3jpKBNg7m5MzoQIjXBDjnm8ANSbIJGrvC+7hlg+FstgQA VnjGexeUGNCHtP3SMS4pPli9LraI9xBErEK/JQVHHLUWpx9dcebElC6gqkThx7pJ mWRTtAktt9W9l4kvmf5fLZv0706BUfQN0a3iiNWZ04Jrmc8iJK0= =hGUf -----END PGP SIGNATURE----- --2zv1+rRzkqrs9Rdg--