All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] net: gro: respect nf_conntrack_checksum for skipping csum verification
@ 2022-06-10 10:36 Rafał Miłecki
  2022-06-10 14:08 ` Eric Dumazet
  0 siblings, 1 reply; 2+ messages in thread
From: Rafał Miłecki @ 2022-06-10 10:36 UTC (permalink / raw)
  To: openwrt-devel, Eric Dumazet, David S . Miller, Hideaki YOSHIFUJI,
	David Ahern, Jakub Kicinski, Paolo Abeni
  Cc: netdev, linux-kernel, Rafał Miłecki

From: Rafał Miłecki <rafal@milecki.pl>

Netfilter allows disabling checksum verification of incoming packets by
setting nf_conntrack_checksum variable. That feature is very useful for
home routers which:
1. Most of the time just /forward/ network traffic
2. Have slow CPU(s) and csum calculation is a challenge

Some projects like OpenWrt set nf_conntrack_checksum to 0 by default.

It would be nice to allow similar optimization in the GRO code paths.
This patch simply reuses nf_conntrack_checksum variable to skip
skb_gro_checksum_validate() calls if applicable.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
Hi guys,

I'm not very familiar with net subsystem, please let me know if there is
a better way of implementing such a feature.
---
 net/ipv4/tcp_offload.c   | 3 +++
 net/ipv6/tcpv6_offload.c | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c
index 30abde86db45..734a3c0f3d4a 100644
--- a/net/ipv4/tcp_offload.c
+++ b/net/ipv4/tcp_offload.c
@@ -311,6 +311,9 @@ struct sk_buff *tcp4_gro_receive(struct list_head *head, struct sk_buff *skb)
 {
 	/* Don't bother verifying checksum if we're going to flush anyway. */
 	if (!NAPI_GRO_CB(skb)->flush &&
+#if IS_ENABLED(CONFIG_NF_CONNTRACK)
+	    dev_net(skb->dev)->ct.sysctl_checksum &&
+#endif
 	    skb_gro_checksum_validate(skb, IPPROTO_TCP,
 				      inet_gro_compute_pseudo)) {
 		NAPI_GRO_CB(skb)->flush = 1;
diff --git a/net/ipv6/tcpv6_offload.c b/net/ipv6/tcpv6_offload.c
index 39db5a226855..2144afa56fa3 100644
--- a/net/ipv6/tcpv6_offload.c
+++ b/net/ipv6/tcpv6_offload.c
@@ -18,6 +18,9 @@ struct sk_buff *tcp6_gro_receive(struct list_head *head, struct sk_buff *skb)
 {
 	/* Don't bother verifying checksum if we're going to flush anyway. */
 	if (!NAPI_GRO_CB(skb)->flush &&
+#if IS_ENABLED(CONFIG_NF_CONNTRACK)
+	    dev_net(skb->dev)->ct.sysctl_checksum &&
+#endif
 	    skb_gro_checksum_validate(skb, IPPROTO_TCP,
 				      ip6_gro_compute_pseudo)) {
 		NAPI_GRO_CB(skb)->flush = 1;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] net: gro: respect nf_conntrack_checksum for skipping csum verification
  2022-06-10 10:36 [PATCH] net: gro: respect nf_conntrack_checksum for skipping csum verification Rafał Miłecki
@ 2022-06-10 14:08 ` Eric Dumazet
  0 siblings, 0 replies; 2+ messages in thread
From: Eric Dumazet @ 2022-06-10 14:08 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: openwrt-devel, David S . Miller, Hideaki YOSHIFUJI, David Ahern,
	Jakub Kicinski, Paolo Abeni, netdev, LKML,
	Rafał Miłecki

On Fri, Jun 10, 2022 at 3:37 AM Rafał Miłecki <zajec5@gmail.com> wrote:
>
> From: Rafał Miłecki <rafal@milecki.pl>
>
> Netfilter allows disabling checksum verification of incoming packets by
> setting nf_conntrack_checksum variable. That feature is very useful for
> home routers which:
> 1. Most of the time just /forward/ network traffic
> 2. Have slow CPU(s) and csum calculation is a challenge
>
> Some projects like OpenWrt set nf_conntrack_checksum to 0 by default.
>
> It would be nice to allow similar optimization in the GRO code paths.
> This patch simply reuses nf_conntrack_checksum variable to skip
> skb_gro_checksum_validate() calls if applicable.
>

Problem is that GRO will be followed by TSO on the egress side.

TSO will generate segments with recomputed checksums for each one of them.

GRO only keeps one copy of the headers, so does not track original
checksums for all
segments at ingress side.

So if you want to use TSO, GRO has to validate checksums.

I am afraid this nf_conntrack_checksum idea can not be transposed to GRO.

> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> Hi guys,
>
> I'm not very familiar with net subsystem, please let me know if there is
> a better way of implementing such a feature.
> ---
>  net/ipv4/tcp_offload.c   | 3 +++
>  net/ipv6/tcpv6_offload.c | 3 +++
>  2 files changed, 6 insertions(+)
>
> diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c
> index 30abde86db45..734a3c0f3d4a 100644
> --- a/net/ipv4/tcp_offload.c
> +++ b/net/ipv4/tcp_offload.c
> @@ -311,6 +311,9 @@ struct sk_buff *tcp4_gro_receive(struct list_head *head, struct sk_buff *skb)
>  {
>         /* Don't bother verifying checksum if we're going to flush anyway. */
>         if (!NAPI_GRO_CB(skb)->flush &&
> +#if IS_ENABLED(CONFIG_NF_CONNTRACK)
> +           dev_net(skb->dev)->ct.sysctl_checksum &&
> +#endif
>             skb_gro_checksum_validate(skb, IPPROTO_TCP,
>                                       inet_gro_compute_pseudo)) {
>                 NAPI_GRO_CB(skb)->flush = 1;
> diff --git a/net/ipv6/tcpv6_offload.c b/net/ipv6/tcpv6_offload.c
> index 39db5a226855..2144afa56fa3 100644
> --- a/net/ipv6/tcpv6_offload.c
> +++ b/net/ipv6/tcpv6_offload.c
> @@ -18,6 +18,9 @@ struct sk_buff *tcp6_gro_receive(struct list_head *head, struct sk_buff *skb)
>  {
>         /* Don't bother verifying checksum if we're going to flush anyway. */
>         if (!NAPI_GRO_CB(skb)->flush &&
> +#if IS_ENABLED(CONFIG_NF_CONNTRACK)
> +           dev_net(skb->dev)->ct.sysctl_checksum &&
> +#endif
>             skb_gro_checksum_validate(skb, IPPROTO_TCP,
>                                       ip6_gro_compute_pseudo)) {
>                 NAPI_GRO_CB(skb)->flush = 1;
> --
> 2.34.1
>

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2022-06-10 14:08 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-10 10:36 [PATCH] net: gro: respect nf_conntrack_checksum for skipping csum verification Rafał Miłecki
2022-06-10 14:08 ` Eric Dumazet

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.