Netdev Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH net] net: gre: recompute gre csum for sctp over gre tunnels
@ 2020-07-31 18:12 Lorenzo Bianconi
  2020-07-31 21:29 ` Lorenzo Bianconi
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Lorenzo Bianconi @ 2020-07-31 18:12 UTC (permalink / raw)
  To: netdev; +Cc: davem, lorenzo.bianconi, tom

The GRE tunnel can be used to transport traffic that does not rely on a
Internet checksum (e.g. SCTP). The issue can be triggered creating a GRE
or GRETAP tunnel and transmitting SCTP traffic ontop of it where CRC
offload has been disabled. In order to fix the issue we need to
recompute the GRE csum in gre_gso_segment() not relying on the inner
checksum.
The issue is still present when we have the CRC offload enabled.
In this case we need to disable the CRC offload if we require GRE
checksum since otherwise skb_checksum() will report a wrong value.

Fixes: 4749c09c37030 ("gre: Call gso_make_checksum")
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 net/ipv4/gre_offload.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/gre_offload.c b/net/ipv4/gre_offload.c
index 2e6d1b7a7bc9..e0a246575887 100644
--- a/net/ipv4/gre_offload.c
+++ b/net/ipv4/gre_offload.c
@@ -15,12 +15,12 @@ static struct sk_buff *gre_gso_segment(struct sk_buff *skb,
 				       netdev_features_t features)
 {
 	int tnl_hlen = skb_inner_mac_header(skb) - skb_transport_header(skb);
+	bool need_csum, need_recompute_csum, gso_partial;
 	struct sk_buff *segs = ERR_PTR(-EINVAL);
 	u16 mac_offset = skb->mac_header;
 	__be16 protocol = skb->protocol;
 	u16 mac_len = skb->mac_len;
 	int gre_offset, outer_hlen;
-	bool need_csum, gso_partial;
 
 	if (!skb->encapsulation)
 		goto out;
@@ -41,6 +41,7 @@ static struct sk_buff *gre_gso_segment(struct sk_buff *skb,
 	skb->protocol = skb->inner_protocol;
 
 	need_csum = !!(skb_shinfo(skb)->gso_type & SKB_GSO_GRE_CSUM);
+	need_recompute_csum = skb->csum_not_inet;
 	skb->encap_hdr_csum = need_csum;
 
 	features &= skb->dev->hw_enc_features;
@@ -98,7 +99,15 @@ static struct sk_buff *gre_gso_segment(struct sk_buff *skb,
 		}
 
 		*(pcsum + 1) = 0;
-		*pcsum = gso_make_checksum(skb, 0);
+		if (need_recompute_csum && !skb_is_gso(skb)) {
+			__wsum csum;
+
+			csum = skb_checksum(skb, gre_offset,
+					    skb->len - gre_offset, 0);
+			*pcsum = csum_fold(csum);
+		} else {
+			*pcsum = gso_make_checksum(skb, 0);
+		}
 	} while ((skb = skb->next));
 out:
 	return segs;
-- 
2.26.2


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

* Re: [PATCH net] net: gre: recompute gre csum for sctp over gre tunnels
  2020-07-31 18:12 [PATCH net] net: gre: recompute gre csum for sctp over gre tunnels Lorenzo Bianconi
@ 2020-07-31 21:29 ` Lorenzo Bianconi
  2020-07-31 22:04 ` Marcelo Ricardo Leitner
  2020-08-03 22:30 ` David Miller
  2 siblings, 0 replies; 4+ messages in thread
From: Lorenzo Bianconi @ 2020-07-31 21:29 UTC (permalink / raw)
  To: Lorenzo Bianconi; +Cc: netdev, davem, tom, marcelo.leitner


[-- Attachment #1: Type: text/plain, Size: 2513 bytes --]

> The GRE tunnel can be used to transport traffic that does not rely on a
> Internet checksum (e.g. SCTP). The issue can be triggered creating a GRE
> or GRETAP tunnel and transmitting SCTP traffic ontop of it where CRC
> offload has been disabled. In order to fix the issue we need to
> recompute the GRE csum in gre_gso_segment() not relying on the inner
> checksum.
> The issue is still present when we have the CRC offload enabled.
> In this case we need to disable the CRC offload if we require GRE
> checksum since otherwise skb_checksum() will report a wrong value.
> 
> Fixes: 4749c09c37030 ("gre: Call gso_make_checksum")

I put the wrong Fixes tag, reviewing it I guess the right one is:

Fixes: 90017accff61 ("sctp: Add GSO support")

sorry for the noise.

Regards,
Lorenzo

> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
>  net/ipv4/gre_offload.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/net/ipv4/gre_offload.c b/net/ipv4/gre_offload.c
> index 2e6d1b7a7bc9..e0a246575887 100644
> --- a/net/ipv4/gre_offload.c
> +++ b/net/ipv4/gre_offload.c
> @@ -15,12 +15,12 @@ static struct sk_buff *gre_gso_segment(struct sk_buff *skb,
>  				       netdev_features_t features)
>  {
>  	int tnl_hlen = skb_inner_mac_header(skb) - skb_transport_header(skb);
> +	bool need_csum, need_recompute_csum, gso_partial;
>  	struct sk_buff *segs = ERR_PTR(-EINVAL);
>  	u16 mac_offset = skb->mac_header;
>  	__be16 protocol = skb->protocol;
>  	u16 mac_len = skb->mac_len;
>  	int gre_offset, outer_hlen;
> -	bool need_csum, gso_partial;
>  
>  	if (!skb->encapsulation)
>  		goto out;
> @@ -41,6 +41,7 @@ static struct sk_buff *gre_gso_segment(struct sk_buff *skb,
>  	skb->protocol = skb->inner_protocol;
>  
>  	need_csum = !!(skb_shinfo(skb)->gso_type & SKB_GSO_GRE_CSUM);
> +	need_recompute_csum = skb->csum_not_inet;
>  	skb->encap_hdr_csum = need_csum;
>  
>  	features &= skb->dev->hw_enc_features;
> @@ -98,7 +99,15 @@ static struct sk_buff *gre_gso_segment(struct sk_buff *skb,
>  		}
>  
>  		*(pcsum + 1) = 0;
> -		*pcsum = gso_make_checksum(skb, 0);
> +		if (need_recompute_csum && !skb_is_gso(skb)) {
> +			__wsum csum;
> +
> +			csum = skb_checksum(skb, gre_offset,
> +					    skb->len - gre_offset, 0);
> +			*pcsum = csum_fold(csum);
> +		} else {
> +			*pcsum = gso_make_checksum(skb, 0);
> +		}
>  	} while ((skb = skb->next));
>  out:
>  	return segs;
> -- 
> 2.26.2
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

* Re: [PATCH net] net: gre: recompute gre csum for sctp over gre tunnels
  2020-07-31 18:12 [PATCH net] net: gre: recompute gre csum for sctp over gre tunnels Lorenzo Bianconi
  2020-07-31 21:29 ` Lorenzo Bianconi
@ 2020-07-31 22:04 ` Marcelo Ricardo Leitner
  2020-08-03 22:30 ` David Miller
  2 siblings, 0 replies; 4+ messages in thread
From: Marcelo Ricardo Leitner @ 2020-07-31 22:04 UTC (permalink / raw)
  To: Lorenzo Bianconi; +Cc: netdev, davem, lorenzo.bianconi, tom

On Fri, Jul 31, 2020 at 08:12:05PM +0200, Lorenzo Bianconi wrote:
> The GRE tunnel can be used to transport traffic that does not rely on a
> Internet checksum (e.g. SCTP). The issue can be triggered creating a GRE
> or GRETAP tunnel and transmitting SCTP traffic ontop of it where CRC
> offload has been disabled. In order to fix the issue we need to
> recompute the GRE csum in gre_gso_segment() not relying on the inner
> checksum.
> The issue is still present when we have the CRC offload enabled.
> In this case we need to disable the CRC offload if we require GRE
> checksum since otherwise skb_checksum() will report a wrong value.
> 
> Fixes: 4749c09c37030 ("gre: Call gso_make_checksum")
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>

Reviewed-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>

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

* Re: [PATCH net] net: gre: recompute gre csum for sctp over gre tunnels
  2020-07-31 18:12 [PATCH net] net: gre: recompute gre csum for sctp over gre tunnels Lorenzo Bianconi
  2020-07-31 21:29 ` Lorenzo Bianconi
  2020-07-31 22:04 ` Marcelo Ricardo Leitner
@ 2020-08-03 22:30 ` David Miller
  2 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2020-08-03 22:30 UTC (permalink / raw)
  To: lorenzo; +Cc: netdev, lorenzo.bianconi, tom

From: Lorenzo Bianconi <lorenzo@kernel.org>
Date: Fri, 31 Jul 2020 20:12:05 +0200

> The GRE tunnel can be used to transport traffic that does not rely on a
> Internet checksum (e.g. SCTP). The issue can be triggered creating a GRE
> or GRETAP tunnel and transmitting SCTP traffic ontop of it where CRC
> offload has been disabled. In order to fix the issue we need to
> recompute the GRE csum in gre_gso_segment() not relying on the inner
> checksum.
> The issue is still present when we have the CRC offload enabled.
> In this case we need to disable the CRC offload if we require GRE
> checksum since otherwise skb_checksum() will report a wrong value.
> 
> Fixes: 4749c09c37030 ("gre: Call gso_make_checksum")
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>

Applied with Fixes: tag corrected and queued up for -stable.

Thanks.

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

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-31 18:12 [PATCH net] net: gre: recompute gre csum for sctp over gre tunnels Lorenzo Bianconi
2020-07-31 21:29 ` Lorenzo Bianconi
2020-07-31 22:04 ` Marcelo Ricardo Leitner
2020-08-03 22:30 ` David Miller

Netdev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/netdev/0 netdev/git/0.git
	git clone --mirror https://lore.kernel.org/netdev/1 netdev/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 netdev netdev/ https://lore.kernel.org/netdev \
		netdev@vger.kernel.org
	public-inbox-index netdev

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.netdev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git