Received-SPF: pass (google.com: domain of linux-kernel+bounces-16743-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <d3de02c2-5158-4cae-b6f7-02d0fe7dae35@arista.com>
Date: Thu, 4 Jan 2024 13:57:49 +0000
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] net/tcp: Only produce AO/MD5 logs if there are any keys
Content-Language: en-US
To: Eric Dumazet <edumazet@google.com>, "David S. Miller"
 <davem@davemloft.net>, Jakub Kicinski <kuba@kernel.org>,
 Paolo Abeni <pabeni@redhat.com>
Cc: Christian Kujau <lists@nerdbynature.de>,
 Salam Noureddine <noureddine@arista.com>,
 Dmitry Safonov <0x7f454c46@gmail.com>, netdev@vger.kernel.org,
 linux-kernel@vger.kernel.org
References: <20240104-tcp_hash_fail-logs-v1-1-ff3e1f6f9e72@arista.com>
From: Dmitry Safonov <dima@arista.com>
In-Reply-To: <20240104-tcp_hash_fail-logs-v1-1-ff3e1f6f9e72@arista.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit

On 1/4/24 13:42, Dmitry Safonov wrote:
> User won't care about inproper hash options in the TCP header if they
> don't use neither TCP-AO nor TCP-MD5. Yet, those logs can add up in
> syslog, while not being a real concern to the host admin:
>> kernel: TCP: TCP segment has incorrect auth options set for XX.20.239.12.54681->XX.XX.90.103.80 [S]
> 
> Keep silent and avoid logging when there aren't any keys in the system.
> 
> Side-note: I also defined static_branch_tcp_*() helpers to avoid more
> ifdeffery, going to remove more ifdeffery further with their help.
> 
> Reported-by: Christian Kujau <lists@nerdbynature.de>
> Closes: https://lore.kernel.org/all/f6b59324-1417-566f-a976-ff2402718a8d@nerdbynature.de/

Probably, it also can have

Fixes: 2717b5adea9e ("net/tcp: Add tcp_hash_fail() ratelimited logs")

> Signed-off-by: Dmitry Safonov <dima@arista.com>
> ---
>  include/net/tcp.h    |  2 --
>  include/net/tcp_ao.h | 26 +++++++++++++++++++++++---
>  2 files changed, 23 insertions(+), 5 deletions(-)
> 
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index 144ba48bb07b..87f0e6c2e1f2 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -1788,8 +1788,6 @@ struct tcp_md5sig_key *tcp_v4_md5_lookup(const struct sock *sk,
>  					 const struct sock *addr_sk);
>  
>  #ifdef CONFIG_TCP_MD5SIG
> -#include <linux/jump_label.h>
> -extern struct static_key_false_deferred tcp_md5_needed;
>  struct tcp_md5sig_key *__tcp_md5_do_lookup(const struct sock *sk, int l3index,
>  					   const union tcp_md5_addr *addr,
>  					   int family, bool any_l3index);
> diff --git a/include/net/tcp_ao.h b/include/net/tcp_ao.h
> index 647781080613..b04afced4cc9 100644
> --- a/include/net/tcp_ao.h
> +++ b/include/net/tcp_ao.h
> @@ -127,12 +127,35 @@ struct tcp_ao_info {
>  	struct rcu_head		rcu;
>  };
>  
> +#ifdef CONFIG_TCP_MD5SIG
> +#include <linux/jump_label.h>
> +extern struct static_key_false_deferred tcp_md5_needed;
> +#define static_branch_tcp_md5()	static_branch_unlikely(&tcp_md5_needed.key)
> +#else
> +#define static_branch_tcp_md5()	false
> +#endif
> +#ifdef CONFIG_TCP_AO
> +/* TCP-AO structures and functions */
> +#include <linux/jump_label.h>
> +extern struct static_key_false_deferred tcp_ao_needed;
> +#define static_branch_tcp_ao()	static_branch_unlikely(&tcp_ao_needed.key)
> +#else
> +#define static_branch_tcp_ao()	false
> +#endif
> +
> +static inline bool tcp_hash_should_produce_warnings(void)
> +{
> +	return static_branch_tcp_md5() || static_branch_tcp_ao();
> +}
> +
>  #define tcp_hash_fail(msg, family, skb, fmt, ...)			\
>  do {									\
>  	const struct tcphdr *th = tcp_hdr(skb);				\
>  	char hdr_flags[6];						\
>  	char *f = hdr_flags;						\
>  									\
> +	if (!tcp_hash_should_produce_warnings())			\
> +		break;							\
>  	if (th->fin)							\
>  		*f++ = 'F';						\
>  	if (th->syn)							\
> @@ -159,9 +182,6 @@ do {									\
>  
>  #ifdef CONFIG_TCP_AO
>  /* TCP-AO structures and functions */
> -#include <linux/jump_label.h>
> -extern struct static_key_false_deferred tcp_ao_needed;
> -
>  struct tcp4_ao_context {
>  	__be32		saddr;
>  	__be32		daddr;
> 
> ---
> base-commit: ac865f00af293d081356bec56eea90815094a60e
> change-id: 20240104-tcp_hash_fail-logs-daa1a4dde694
> 
> Best regards,

-- 
Dmitry