[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7056440.9J7NaK4W3v@benoit.monin>
Date: Mon, 07 Oct 2024 16:54:12 +0200
From: Benoît Monin <benoit.monin@....fr>
To: "David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>, Jiri Pirko <jiri@...nulli.us>,
Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
Lorenzo Bianconi <lorenzo@...nel.org>,
Willem de Bruijn <willemdebruijn.kernel@...il.com>
Cc: netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject:
Re: [PATCH net-next] net: skip offload for NETIF_F_IPV6_CSUM if ipv6 header
contains extension
07/10/2024 Willem de Bruijn wrote :
> Benoît Monin wrote:
> > Devices with NETIF_F_IP_CSUM capability can checksum TCP and UDP over
> > IPv4 with an IP header that may contains options; whereas devices with
> > NETIF_F_IPV6_CSUM capability can only checksum TCP and UDP over IPv6 if
> > the IP header does not contains extension.
>
> Are both these statements universally true across devices?
>
> I can believe for NETIF_F_IP_CSUM that this is the definition, and
> that devices that cannot handle options must fix it up indivually in
> ndo_features_check.
>
> And same for NETIF_F_IPV6_CSUM with extension headers.
>
> But it would be good to see where this is asserted in the code, or
> examples of drivers that have to perform such actions.
>
I was referring to the documentation in skbuff.h that describes
NETIF_F_IP_CSUM and NETIF_F_IPV6_CSUM.
For NETIF_F_IPV6_CSUM, at least fsl_dpa and r8169 expect
ipv6_hdr(skb)->nexthdr to be IPPROTO_{TCP,UDP} to compute the correct
checksum for IPv6.
I posted more details about the problem I am trying to fix with this
patch in the following thread:
https://lore.kernel.org/netdev/26548921.1r3eYUQgxm@benoit.monin/T/#u
> > Enforce that in skb_csum_hwoffload_help by checking the network header
> > length in the case where the IP header version is 6. We cannot simply
> > rely on the network header length since the IPv4 header can from 20 to
> > 60 bytes whereas the IPv6 header must be 40 bytes. So we check the
> > version field which is common to IPv4 and IPv6 headers.
> >
> > This fixes checksumming errors seen with ip6_tunnel and fou6
> > encapsulation, for example with GRE-in-UDP over IPv6:
> > * fou6 adds a UDP header with a partial checksum if the inner packet
> > does not contains a valid checksum.
>
> Where in the code is this conditional on the inner packet csum?
>
This is done by udp6_set_csum, which called by fou6_build_udp.
> > * ip6_tunnel adds an IPv6 header with a destination option extension
> > header if encap_limit is non-zero (the default value is 4).
>
>
> If this is a fix, we'll need to target net and best effort find a
> suitable fixes tag.
>
I guess the particular problem I have found is present since the merge
of fou6 in 4.7, but it might not be the only code path to create an
IPv6 packet with an extension header and a partial checksum.
> > Signed-off-by: Benoît Monin <benoit.monin@....fr>
> > ---
> > net/core/dev.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index ea5fbcd133ae..199831d86ec1 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -3639,6 +3639,9 @@ int skb_csum_hwoffload_help(struct sk_buff *skb,
> > return 0;
> >
> > if (features & (NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM)) {
> > + if (ip_hdr(skb)->version == 6 &&
> > + skb_network_header_len(skb) != sizeof(struct ipv6hdr))
> > + goto sw_checksum;
> > switch (skb->csum_offset) {
> > case offsetof(struct tcphdr, check):
> > case offsetof(struct udphdr, check):
> > @@ -3646,6 +3649,7 @@ int skb_csum_hwoffload_help(struct sk_buff *skb,
> > }
> > }
> >
> > +sw_checksum:
> > return skb_checksum_help(skb);
> > }
> > EXPORT_SYMBOL(skb_csum_hwoffload_help);
>
--
Benoît
Powered by blists - more mailing lists