From nobody Thu Mar 14 00:24:17 2024 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4Tw7RT04x6z5Dlvn; Thu, 14 Mar 2024 00:24:21 +0000 (UTC) (envelope-from glebius@freebsd.org) Received: from smtp.freebsd.org (smtp.freebsd.org [IPv6:2610:1c1:1:606c::24b:4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Tw7RS6b4xz4DmL; Thu, 14 Mar 2024 00:24:20 +0000 (UTC) (envelope-from glebius@freebsd.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1710375860; 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: in-reply-to:in-reply-to:references:references; bh=cHnGYxE0M/yAkN5dzrFBgbrmkhgAGkgNzs+9S4gZjcM=; b=O40dfpe1/MkHvdJ8t74w/6L5r3PsbMfoxi4jfLzmc6JeKYoAWx9Ik/pCwZcZ+cWtSqcaJo iFqWFDGYsFbalVxBjrRHmnp8xdI/kRwuEua7DGrQa5F7nek+J6eEPX5RhMR/pEwmexLMtn qC2rIGRYL+MnY6hXCQxKyLC7OM/O06/LtAbOvbauDQtljsPpYrQIt3ZDPZlb1SFn8Ib+II WpK0zSX+/A10STrGNWNxZ6maZDyJve2jKogyWfNYwofVqCK7Ba3SsbU/WhGcxgvc08ta3f 6zjZ9GbL5Q9iTw2R+1mBdTOWww7QltvTGFTYr+DTgfn95V1Uh9m5lqsxj3QZdw== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1710375860; a=rsa-sha256; cv=none; b=RoNgrAdNEwlUax55lnUmZewnGK4gszeh7GUDl39iIgWlDnrzuVXZWxy5L6v15fu7fHgO3t nF+Vjk1D5X+Df0r1n4L0l04jPB7wTr0MoNtUeKDLCQrd5dAfwDn4LinVAV3k9GMJ7uZbbq g53E4aVYN3ADKM1qKHLR0mCT2y32U4cUfvMVDjGvMHW5QO/MSwF/dH9r/+P3FMbCu/6e0N q6ywS+Ibrhoak4bl3CJc1KveLZKIneMwzbWB17/O+Hx7XED7IPWbRUYVi/qBp89MrVEza3 4avFhFCDDGOVl5mpucPTQpPtaz0mF9GlWrv3E0i+setG/VGODPaDkSi+R8WUbg== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1710375860; 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: in-reply-to:in-reply-to:references:references; bh=cHnGYxE0M/yAkN5dzrFBgbrmkhgAGkgNzs+9S4gZjcM=; b=Ne6FtcV1XCqMXtOX51I8nkLmsBQXMT7RK6ZHdqCNoi9ooYz2Xcba/LVDr1rHzkNcsAVoU3 K5dVSYUHnE3d7wX4QWz+Bw0tMYtgM31eoRxP31edOEqzUCUT1mbW0GrgoYh5gqlQf7sl9z 4pemWilkLFKjBkBHQmAYZZ1aQDXiJvr3v0YQakSo/a+mVhnim/E70ypRAPDEFUieQjDehx EZxj0kFuiOmwOy7MS+Pa3aym6cfha40+W7xgxxR4vw5vlnC4erLqY0x0M/sBQfyxuAgjEf nzjlMYtp5gx0bS3YX3INYRPE1PsQa6dmXG2MKjloSrPg5Llc1yWDojET3kOfrQ== Received: from cell.glebi.us (glebi.us [162.251.186.162]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) (Authenticated sender: glebius) by smtp.freebsd.org (Postfix) with ESMTPSA id 4Tw7RS2SPYz1Hh3; Thu, 14 Mar 2024 00:24:20 +0000 (UTC) (envelope-from glebius@freebsd.org) Date: Wed, 13 Mar 2024 17:24:17 -0700 From: Gleb Smirnoff To: Konstantin Belousov Cc: src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: 220ee18f1964 - main - netinet/tcp_var.h: always define IS_FASTOPEN() for kernel compilation env Message-ID: References: <202403132321.42DNLNIX087785@gitrepo.freebsd.org> List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <202403132321.42DNLNIX087785@gitrepo.freebsd.org> On Wed, Mar 13, 2024 at 11:21:23PM +0000, Konstantin Belousov wrote: K> diff --git a/sys/netinet/tcp_var.h b/sys/netinet/tcp_var.h K> index 6f7f7115c2f4..7b5c57d39213 100644 K> --- a/sys/netinet/tcp_var.h K> +++ b/sys/netinet/tcp_var.h K> @@ -812,11 +812,13 @@ tcp_packets_this_ack(struct tcpcb *tp, tcp_seq ack) K> #define ENTER_RECOVERY(t_flags) t_flags |= (TF_CONGRECOVERY | TF_FASTRECOVERY) K> #define EXIT_RECOVERY(t_flags) t_flags &= ~(TF_CONGRECOVERY | TF_FASTRECOVERY) K> K> -#if defined(_KERNEL) && !defined(TCP_RFC7413) K> +#if defined(_KERNEL) K> +#if !defined(TCP_RFC7413) K> #define IS_FASTOPEN(t_flags) (false) K> #else K> #define IS_FASTOPEN(t_flags) (t_flags & TF_FASTOPEN) K> #endif K> +#endif K> K> #define BYTES_THIS_ACK(tp, th) (th->th_ack - tp->snd_una) I know Konstantin in doing that to clear path for IPSEC changes, and the patch does improve code. So the message isn't addressed to him, rather it is for other src-committers. Using ifdefs that come from the kernel config inside include files that are not ephemeral opt_foo.h is laying out a minefield for the future. Best case - for yourself, worst case - for somebody else. In the best case this ends in cryptic kernel failure builds, where your custom kernel doesn't build, but GENERIC builds and it is not clear why. In the worst case this creates runtime failures even more cryptic in their nature. In this particular case TCP_RFC7413 comes via opt_inet.h. Thus, if you got a userland tool, e.g. netstat(1) using IS_FASTOPEN() it will always be false. Again, netstat would be a best case. A worst case would be netinet6 kernel compilation unit that does not include opt_inet.h, but uses IS_FASTOPEN(). Other than that, we got 32 flags for t_flags and only one is obfuscated via a macro. I'd really like to remove the macro and test the flag directly. Any objections? -- Gleb Smirnoff