| Message ID | 20231013185502.1473541-1-jacob.e.keller@intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | e411a8e3bb2d12a59d5fb3590863cb6a16f27b7a |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: stub tcp_gro_complete if CONFIG_INET=n | expand |
On 10/13/2023 11:54 AM, Jacob Keller wrote: This should have [PATCH net-next], but I forgot to set that when formatting. Sorry about that. Thanks, Jake
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@intel.com> Acked-by: Randy Dunlap <rdunlap@infradead.org> Tested-by: Randy Dunlap <rdunlap@infradead.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); >
Hello: This patch was applied to netdev/net-next.git (main) by Jakub Kicinski <kuba@kernel.org>: On Fri, 13 Oct 2023 11:54:50 -0700 you 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. > > [...] Here is the summary with links: - net: stub tcp_gro_complete if CONFIG_INET=n https://git.kernel.org/netdev/net-next/c/e411a8e3bb2d You are awesome, thank you!
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);
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@intel.com> --- I've only compile tested this. include/net/tcp.h | 4 ++++ 1 file changed, 4 insertions(+)