netdev - Re: [PATCH] net: stub tcp_gro_complete if CONFIG

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <113bb0a0-b3f4-4220-a33d-d32091740634@infradead.org>
Date: Fri, 13 Oct 2023 15:04:35 -0700
From: Randy Dunlap <rdunlap@...radead.org>
To: Jacob Keller <jacob.e.keller@...el.com>, Jakub Kicinski
 <kuba@...nel.org>, David Miller <davem@...emloft.net>,
 Arnd Bergmann <arnd@...nel.org>,
 Alexander Lobakin <aleksander.lobakin@...el.com>, netdev@...r.kernel.org,
 Paolo Abeni <pabeni@...hat.com>, Eric Dumazet <edumazet@...gle.com>
Subject: Re: [PATCH] net: stub tcp_gro_complete if CONFIG_INET=n



On 10/13/23 11:54, Jacob Keller wrote:
> A few networking drivers including bnx2x, bnxt, qede, and idpf call
> tcp_gro_complete as part of offloading TCP GRO. The function is only
> defined if CONFIG_INET is true, since its TCP specific and is meaningless
> if the kernel lacks IP networking support.
> 
> The combination of trying to use the complex network drivers with
> CONFIG_NET but not CONFIG_INET is rather unlikely in practice: most use
> cases are going to need IP networking.
> 
> The tcp_gro_complete function just sets some data in the socket buffer for
> use in processing the TCP packet in the event that the GRO was offloaded to
> the device. If the kernel lacks TCP support, such setup will simply go
> unused.
> 
> The bnx2x, bnxt, and qede drivers wrap their TCP offload support in
> CONFIG_INET checks and skip handling on such kernels.
> 
> The idpf driver did not check CONFIG_INET and thus fails to link if the
> kernel is configured  with CONFIG_NET=y, CONFIG_IDPF=(m|y), and
> CONFIG_INET=n.
> 
> While checking CONFIG_INET does allow the driver to bypass significantly
> more instructions in the event that we know TCP networking isn't supported,
> the configuration is unlikely to be used widely.
> 
> Rather than require driver authors to care about this, stub the
> tcp_gro_complete function when CONFIG_INET=n. This allows drivers to be
> left as-is. It does mean the idpf driver will perform slightly more work
> than strictly necessary when CONFIG_INET=n, since it will still execute
> some of the skb setup in idpf_rx_rsc. However, that work would be performed
> in the case where CONFIG_INET=y anyways.
> 
> I did not change the existing drivers, since they appear to wrap a
> significant portion of code when CONFIG_INET=n. There is little benefit in
> trashing these drivers just to unwrap and remove the CONFIG_INET check.
> 
> Using a stub for tcp_gro_complete is still beneficial, as it means future
> drivers no longer need to worry about this case of CONFIG_NET=y and
> CONFIG_INET=n, which should reduce noise from buildbots that check such a
> configuration.
> 
> Signed-off-by: Jacob Keller <jacob.e.keller@...el.com>

Acked-by: Randy Dunlap <rdunlap@...radead.org>
Tested-by: Randy Dunlap <rdunlap@...radead.org> # build-tested

Thanks.

> ---
> I've only compile tested this.
> 
>  include/net/tcp.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index 7fdedf5c71f0..32146088a095 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -2081,7 +2081,11 @@ INDIRECT_CALLABLE_DECLARE(int tcp4_gro_complete(struct sk_buff *skb, int thoff))
>  INDIRECT_CALLABLE_DECLARE(struct sk_buff *tcp4_gro_receive(struct list_head *head, struct sk_buff *skb));
>  INDIRECT_CALLABLE_DECLARE(int tcp6_gro_complete(struct sk_buff *skb, int thoff));
>  INDIRECT_CALLABLE_DECLARE(struct sk_buff *tcp6_gro_receive(struct list_head *head, struct sk_buff *skb));
> +#ifdef CONFIG_INET
>  void tcp_gro_complete(struct sk_buff *skb);
> +#else
> +static inline void tcp_gro_complete(struct sk_buff *skb) { }
> +#endif
>  
>  void __tcp_v4_send_check(struct sk_buff *skb, __be32 saddr, __be32 daddr);
>  

-- 
~Randy