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=202408 header.b=JofVqXD/; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 978E25A026E for ; Wed, 18 Sep 2024 03:52:43 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202408; t=1726624353; bh=t/bizXEAt0/5n38vvaT7dZKE/nUQ+gmUckv+7NWAgqk=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=JofVqXD/1MjZ1brR158fr+APzIYZ0bSrJg6bThcWjmM5MbpQhpZYG8ocOiTcL7XG/ vvakN7jg1xuD5o9FtAvzoyEXun6AHoo6oHsbFS4wFeeoxPUs393VsYnPVyl0ndxJBW wk+KneR5eHVdPa5Fw0102+9E6I7E8VZR0opU8xgZ8E3Eg+T0mk46MDzvH2OYPibOAG pSpuRW/+6flzcWMkyuBOlkOqD44k0aGzbj5BqeiQ3ohL06iOctVpOhlMsM3SHkS9AB qUx+BDXBa36/Jexl5YaQ3Eh8cD+msFi8lDa7NoSzkFQwrbJUDtfs5jUhZ11SHk4j9p GfZ2+32cEyv8Q== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4X7hVT0GTrz4xYP; Wed, 18 Sep 2024 11:52:33 +1000 (AEST) Date: Wed, 18 Sep 2024 11:31:24 +1000 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH v2 03/10] tcp: Simplify ifdef logic in tcp_update_seqack_wnd() Message-ID: References: <20240913043214.1753014-1-david@gibson.dropbear.id.au> <20240913043214.1753014-4-david@gibson.dropbear.id.au> <20240917235434.22e09fef@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="ZzZZ5md7xXmkslDa" Content-Disposition: inline In-Reply-To: <20240917235434.22e09fef@elisabeth> Message-ID-Hash: T5NRNKWLDNRULMUCBYK7DOE44DTIPJST X-Message-ID-Hash: T5NRNKWLDNRULMUCBYK7DOE44DTIPJST 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: --ZzZZ5md7xXmkslDa Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Sep 17, 2024 at 11:54:34PM +0200, Stefano Brivio wrote: > On Fri, 13 Sep 2024 14:32:07 +1000 > David Gibson wrote: >=20 > > This function has a block conditional on !snd_wnd_cap shortly before an > > #ifdef HAS_SND_WND. But snd_wnd_cap implies HAS_SND_WND (if !HAS_SND_W= ND, > > snd_wnd_cap is statically false). > >=20 > > Therefore, simplify this down to a single conditional with an else bran= ch. > > While we're there, fix some improperly indented closing braces. > >=20 > > Signed-off-by: David Gibson > > --- > > tcp.c | 36 +++++++++++++++++------------------- > > 1 file changed, 17 insertions(+), 19 deletions(-) > >=20 > > diff --git a/tcp.c b/tcp.c > > index cba3f3bd..6733e7e3 100644 > > --- a/tcp.c > > +++ b/tcp.c > > @@ -1061,27 +1061,25 @@ int tcp_update_seqack_wnd(const struct ctx *c, = struct tcp_tap_conn *conn, > > conn->wnd_to_tap =3D MIN(new_wnd_to_tap >> conn->ws_to_tap, > > USHRT_MAX); > > goto out; > > - } > > - > > - if (!tinfo) { > > - if (prev_wnd_to_tap > WINDOW_DEFAULT) { > > - goto out; > > -} > > - tinfo =3D &tinfo_new; > > - if (getsockopt(s, SOL_TCP, TCP_INFO, tinfo, &sl)) { > > - goto out; > > -} > > - } > > - > > -#ifdef HAS_SND_WND > > - if ((conn->flags & LOCAL) || tcp_rtt_dst_low(conn)) { > > - new_wnd_to_tap =3D tinfo->tcpi_snd_wnd; > > } else { >=20 > I thought cppcheck would report else-after-goto, but it doesn't, just > else-after-return. In any case, we could simplify further by avoid that > else clause (and one level of indentation) in the whole block below. Good idea, done. > It would also look more natural to me: we deal with if (!snd_wnd_cap) > as a special case and go to 'out' in that special case, then we resume > with the regular path. >=20 > I guess this is better than the original anyway and it's not a strong > preference, so I can also apply this up to 4/10 as it is (or fix up on > merge). The rest of the patches up to 4/10 look good to me. Well, I've made the change now, so I might as well repost :). >=20 > > - tcp_get_sndbuf(conn); > > - new_wnd_to_tap =3D MIN((int)tinfo->tcpi_snd_wnd, > > - SNDBUF_GET(conn)); > > + if (!tinfo) { > > + if (prev_wnd_to_tap > WINDOW_DEFAULT) { > > + goto out; > > + } > > + tinfo =3D &tinfo_new; > > + if (getsockopt(s, SOL_TCP, TCP_INFO, tinfo, &sl)) { > > + goto out; > > + } > > + } > > + > > + if ((conn->flags & LOCAL) || tcp_rtt_dst_low(conn)) { > > + new_wnd_to_tap =3D tinfo->tcpi_snd_wnd; > > + } else { > > + tcp_get_sndbuf(conn); > > + new_wnd_to_tap =3D MIN((int)tinfo->tcpi_snd_wnd, > > + SNDBUF_GET(conn)); > > + } > > } > > -#endif > > =20 > > new_wnd_to_tap =3D MIN(new_wnd_to_tap, MAX_WINDOW); > > if (!(conn->events & ESTABLISHED)) >=20 --=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 --ZzZZ5md7xXmkslDa Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmbqLWwACgkQzQJF27ox 2GchDA/+O4gFN2KfFWCrCzcBXsjUH73fxjzHDjXlIiPLTOu0VKFYfR886Zofu14q 54N1bUtjevbJpP59DTc9YLlXRBlMb0/O0dJZSG8tyYK4ptGX7B/VI3czNw8fqZiU vhXFqr6dXIQFbhMyp95erYAmucYBiFEFTgSk271vIRjLsqMXHZuZDQv6X+VTYi/h G7efHmmRYK8H++n553oP75toeOtzdd6kBtQA+KDmYL7iPM7VyGQzkpMmES+9P5Pi S/RZ6qgN/IdZUwTYOFz1Eid+yaC++HYoSSs7KGGj/jx2Qnzjq8HnB9/VXIYKCc1w BxGPjXKqFt5jC97Kfvh5HmYHlKV1WG6o0OFy2mcLRrvwAEmOqzmOsVaNS+4SwoS7 L8oTPty+SyJRaXaCSeH3UmXzxwHPPkySmbfDfvXdmg6FqdaM4gA+ZQYR42ZtVqbu VTg8Jfa4ydp0GjV1TE63jPDODZdbMTE9MfsEiqtrRnTJ+d9z7ZYriwEx56/Auwd5 lJU1lqYRKyJ+Qca6tcDJ6JPXtQo0AVDC0PXDtVkMbHdto2IGMB6qqhMe3g4xZD1p FD/lxA/r9ix4Y7UMM8PLZLf/LezIv6FGYzr+X+OdIJafWOtmTu/K0ghgshDbFpej G3bdm67Lc8kcAUHz/RgIZabiZ4HbCWWzuj6n47NZHNmPJpv3rIo= =E8BI -----END PGP SIGNATURE----- --ZzZZ5md7xXmkslDa--