| Message ID | 20230911095039.3611113-1-mika.westerberg@linux.intel.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: thunderbolt: Fix TCP/UDPv6 GSO checksum calculation | expand |
On Mon, Sep 11, 2023 at 11:50 AM Mika Westerberg <mika.westerberg@linux.intel.com> wrote: > > Alex reported that running ssh over IPv6 does not work with > Thunderbolt/USB4 networking driver. The reason for that is that driver > should call skb_is_gso() before calling skb_is_gso_v6(), and it should > not return false after calculates the checksum successfully. This probably > was a copy paste error from the original driver where it was done > properly. > > While there add checksum calculation for UDPv6 GSO packets as well. This part does not belong to this patch, and should be submitted for net-next. Note that this driver is not supposed to receive UDP GSO packets. > > Cc: stable@vger.kernel.org What would be the Fixes: tag for this patch ? > Reported-by: Alex Balcanquall <alex@alexbal.com> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > --- > drivers/net/thunderbolt/main.c | 21 +++++++++++++++------ > 1 file changed, 15 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/thunderbolt/main.c b/drivers/net/thunderbolt/main.c > index 0c1e8970ee58..ba50a554478f 100644 > --- a/drivers/net/thunderbolt/main.c > +++ b/drivers/net/thunderbolt/main.c > @@ -1049,12 +1049,21 @@ static bool tbnet_xmit_csum_and_map(struct tbnet *net, struct sk_buff *skb, > *tucso = ~csum_tcpudp_magic(ip_hdr(skb)->saddr, > ip_hdr(skb)->daddr, 0, > ip_hdr(skb)->protocol, 0); > - } else if (skb_is_gso_v6(skb)) { > - tucso = dest + ((void *)&(tcp_hdr(skb)->check) - data); > - *tucso = ~csum_ipv6_magic(&ipv6_hdr(skb)->saddr, > - &ipv6_hdr(skb)->daddr, 0, > - IPPROTO_TCP, 0); > - return false; > + } else if (skb_is_gso(skb)) { > + if (skb_is_gso_v6(skb)) { > + tucso = dest + ((void *)&(tcp_hdr(skb)->check) - data); > + *tucso = ~csum_ipv6_magic(&ipv6_hdr(skb)->saddr, > + &ipv6_hdr(skb)->daddr, 0, > + IPPROTO_TCP, 0); > + } else if (protocol == htons(ETH_P_IPV6) && > + (skb_shinfo(skb)->gso_type & SKB_GSO_UDP)) { > + tucso = dest + ((void *)&(udp_hdr(skb)->check) - data); > + *tucso = ~csum_ipv6_magic(&ipv6_hdr(skb)->saddr, > + &ipv6_hdr(skb)->daddr, 0, > + IPPROTO_UDP, 0); This is dead code in the current state of this driver. > + } else { > + return false; > + } > } else if (protocol == htons(ETH_P_IPV6)) { > tucso = dest + skb_checksum_start_offset(skb) + skb->csum_offset; > *tucso = ~csum_ipv6_magic(&ipv6_hdr(skb)->saddr, > -- > 2.40.1 >
Hi, On Mon, Sep 11, 2023 at 12:03:06PM +0200, Eric Dumazet wrote: > On Mon, Sep 11, 2023 at 11:50 AM Mika Westerberg > <mika.westerberg@linux.intel.com> wrote: > > > > Alex reported that running ssh over IPv6 does not work with > > Thunderbolt/USB4 networking driver. The reason for that is that driver > > should call skb_is_gso() before calling skb_is_gso_v6(), and it should > > not return false after calculates the checksum successfully. This probably > > was a copy paste error from the original driver where it was done > > properly. > > > > While there add checksum calculation for UDPv6 GSO packets as well. > > This part does not belong to this patch, and should be submitted for net-next. > > Note that this driver is not supposed to receive UDP GSO packets. Ah, indeed. I took this part from the original driver that was submitted around 2016-2017 timeframe but was never merged. This current driver is reworked version but I missed the skb_is_gso() and the rest. > > > > Cc: stable@vger.kernel.org > > What would be the Fixes: tag for this patch ? It would be the initial commit: Fixes: e69b6c02b4c3 ("net: Add support for networking over Thunderbolt cable") But I was not sure if it is really practical here. I can add it to v2. > > Reported-by: Alex Balcanquall <alex@alexbal.com> > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > > --- > > drivers/net/thunderbolt/main.c | 21 +++++++++++++++------ > > 1 file changed, 15 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/net/thunderbolt/main.c b/drivers/net/thunderbolt/main.c > > index 0c1e8970ee58..ba50a554478f 100644 > > --- a/drivers/net/thunderbolt/main.c > > +++ b/drivers/net/thunderbolt/main.c > > @@ -1049,12 +1049,21 @@ static bool tbnet_xmit_csum_and_map(struct tbnet *net, struct sk_buff *skb, > > *tucso = ~csum_tcpudp_magic(ip_hdr(skb)->saddr, > > ip_hdr(skb)->daddr, 0, > > ip_hdr(skb)->protocol, 0); > > - } else if (skb_is_gso_v6(skb)) { > > - tucso = dest + ((void *)&(tcp_hdr(skb)->check) - data); > > - *tucso = ~csum_ipv6_magic(&ipv6_hdr(skb)->saddr, > > - &ipv6_hdr(skb)->daddr, 0, > > - IPPROTO_TCP, 0); > > - return false; > > + } else if (skb_is_gso(skb)) { > > + if (skb_is_gso_v6(skb)) { > > + tucso = dest + ((void *)&(tcp_hdr(skb)->check) - data); > > + *tucso = ~csum_ipv6_magic(&ipv6_hdr(skb)->saddr, > > + &ipv6_hdr(skb)->daddr, 0, > > + IPPROTO_TCP, 0); > > + } else if (protocol == htons(ETH_P_IPV6) && > > + (skb_shinfo(skb)->gso_type & SKB_GSO_UDP)) { > > + tucso = dest + ((void *)&(udp_hdr(skb)->check) - data); > > + *tucso = ~csum_ipv6_magic(&ipv6_hdr(skb)->saddr, > > + &ipv6_hdr(skb)->daddr, 0, > > + IPPROTO_UDP, 0); > > This is dead code in the current state of this driver. Thanks! I will submit v2 with that dropped. > > + } else { > > + return false; > > + } > > } else if (protocol == htons(ETH_P_IPV6)) { > > tucso = dest + skb_checksum_start_offset(skb) + skb->csum_offset; > > *tucso = ~csum_ipv6_magic(&ipv6_hdr(skb)->saddr, > > -- > > 2.40.1 > >
diff --git a/drivers/net/thunderbolt/main.c b/drivers/net/thunderbolt/main.c index 0c1e8970ee58..ba50a554478f 100644 --- a/drivers/net/thunderbolt/main.c +++ b/drivers/net/thunderbolt/main.c @@ -1049,12 +1049,21 @@ static bool tbnet_xmit_csum_and_map(struct tbnet *net, struct sk_buff *skb, *tucso = ~csum_tcpudp_magic(ip_hdr(skb)->saddr, ip_hdr(skb)->daddr, 0, ip_hdr(skb)->protocol, 0); - } else if (skb_is_gso_v6(skb)) { - tucso = dest + ((void *)&(tcp_hdr(skb)->check) - data); - *tucso = ~csum_ipv6_magic(&ipv6_hdr(skb)->saddr, - &ipv6_hdr(skb)->daddr, 0, - IPPROTO_TCP, 0); - return false; + } else if (skb_is_gso(skb)) { + if (skb_is_gso_v6(skb)) { + tucso = dest + ((void *)&(tcp_hdr(skb)->check) - data); + *tucso = ~csum_ipv6_magic(&ipv6_hdr(skb)->saddr, + &ipv6_hdr(skb)->daddr, 0, + IPPROTO_TCP, 0); + } else if (protocol == htons(ETH_P_IPV6) && + (skb_shinfo(skb)->gso_type & SKB_GSO_UDP)) { + tucso = dest + ((void *)&(udp_hdr(skb)->check) - data); + *tucso = ~csum_ipv6_magic(&ipv6_hdr(skb)->saddr, + &ipv6_hdr(skb)->daddr, 0, + IPPROTO_UDP, 0); + } else { + return false; + } } else if (protocol == htons(ETH_P_IPV6)) { tucso = dest + skb_checksum_start_offset(skb) + skb->csum_offset; *tucso = ~csum_ipv6_magic(&ipv6_hdr(skb)->saddr,
Alex reported that running ssh over IPv6 does not work with Thunderbolt/USB4 networking driver. The reason for that is that driver should call skb_is_gso() before calling skb_is_gso_v6(), and it should not return false after calculates the checksum successfully. This probably was a copy paste error from the original driver where it was done properly. While there add checksum calculation for UDPv6 GSO packets as well. Cc: stable@vger.kernel.org Reported-by: Alex Balcanquall <alex@alexbal.com> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> --- drivers/net/thunderbolt/main.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-)