netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net v3] udp: ipv4: manipulate network header of NATed UDP GRO fraglist
       [not found] <CGME20210121133649epcas2p493d5d59df1b48ee8e3282ab766f37a70@epcas2p4.samsung.com>
@ 2021-01-21 13:24 ` Dongseok Yi
  2021-01-25 12:45   ` Steffen Klassert
  0 siblings, 1 reply; 4+ messages in thread
From: Dongseok Yi @ 2021-01-21 13:24 UTC (permalink / raw)
  To: David S. Miller, Steffen Klassert, Alexander Lobakin
  Cc: namkyu78.kim, Dongseok Yi, Jakub Kicinski, Hideaki YOSHIFUJI,
	Willem de Bruijn, netdev, linux-kernel

UDP/IP header of UDP GROed frag_skbs are not updated even after NAT
forwarding. Only the header of head_skb from ip_finish_output_gso ->
skb_gso_segment is updated but following frag_skbs are not updated.

A call path skb_mac_gso_segment -> inet_gso_segment ->
udp4_ufo_fragment -> __udp_gso_segment -> __udp_gso_segment_list
does not try to update UDP/IP header of the segment list but copy
only the MAC header.

Update port, addr and check of each skb of the segment list in
__udp_gso_segment_list. It covers both SNAT and DNAT.

Fixes: 9fd1ff5d2ac7 (udp: Support UDP fraglist GRO/GSO.)
Signed-off-by: Dongseok Yi <dseok.yi@samsung.com>
---
v1:
Steffen Klassert said, there could be 2 options.
https://lore.kernel.org/patchwork/patch/1362257/
I was trying to write a quick fix, but it was not easy to forward
segmented list. Currently, assuming DNAT only.

v2:
Per Steffen Klassert request, moved the procedure from
udp4_ufo_fragment to __udp_gso_segment_list and support SNAT.

v3:
Per Steffen Klassert request, applied fast return by comparing seg
and seg->next at the beginning of __udpv4_gso_segment_list_csum.

Fixed uh->dest = *newport and iph->daddr = *newip to
*oldport = *newport and *oldip = *newip.

 include/net/udp.h      |  2 +-
 net/ipv4/udp_offload.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++----
 net/ipv6/udp_offload.c |  2 +-
 3 files changed, 69 insertions(+), 7 deletions(-)

diff --git a/include/net/udp.h b/include/net/udp.h
index 877832b..01351ba 100644
--- a/include/net/udp.h
+++ b/include/net/udp.h
@@ -178,7 +178,7 @@ struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb,
 int udp_gro_complete(struct sk_buff *skb, int nhoff, udp_lookup_t lookup);
 
 struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,
-				  netdev_features_t features);
+				  netdev_features_t features, bool is_ipv6);
 
 static inline struct udphdr *udp_gro_udphdr(struct sk_buff *skb)
 {
diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
index ff39e94..43660cf 100644
--- a/net/ipv4/udp_offload.c
+++ b/net/ipv4/udp_offload.c
@@ -187,8 +187,67 @@ struct sk_buff *skb_udp_tunnel_segment(struct sk_buff *skb,
 }
 EXPORT_SYMBOL(skb_udp_tunnel_segment);
 
+static void __udpv4_gso_segment_csum(struct sk_buff *seg,
+				     __be32 *oldip, __be32 *newip,
+				     __be16 *oldport, __be16 *newport)
+{
+	struct udphdr *uh;
+	struct iphdr *iph;
+
+	if (*oldip == *newip && *oldport == *newport)
+		return;
+
+	uh = udp_hdr(seg);
+	iph = ip_hdr(seg);
+
+	if (uh->check) {
+		inet_proto_csum_replace4(&uh->check, seg, *oldip, *newip,
+					 true);
+		inet_proto_csum_replace2(&uh->check, seg, *oldport, *newport,
+					 false);
+		if (!uh->check)
+			uh->check = CSUM_MANGLED_0;
+	}
+	*oldport = *newport;
+
+	csum_replace4(&iph->check, *oldip, *newip);
+	*oldip = *newip;
+}
+
+static struct sk_buff *__udpv4_gso_segment_list_csum(struct sk_buff *segs)
+{
+	struct sk_buff *seg;
+	struct udphdr *uh, *uh2;
+	struct iphdr *iph, *iph2;
+
+	seg = segs;
+	uh = udp_hdr(seg);
+	iph = ip_hdr(seg);
+
+	if ((udp_hdr(seg)->dest == udp_hdr(seg->next)->dest) &&
+	    (udp_hdr(seg)->source == udp_hdr(seg->next)->source) &&
+	    (ip_hdr(seg)->daddr == ip_hdr(seg->next)->daddr) &&
+	    (ip_hdr(seg)->saddr == ip_hdr(seg->next)->saddr))
+		return segs;
+
+	while ((seg = seg->next)) {
+		uh2 = udp_hdr(seg);
+		iph2 = ip_hdr(seg);
+
+		__udpv4_gso_segment_csum(seg,
+					 &iph2->saddr, &iph->saddr,
+					 &uh2->source, &uh->source);
+		__udpv4_gso_segment_csum(seg,
+					 &iph2->daddr, &iph->daddr,
+					 &uh2->dest, &uh->dest);
+	}
+
+	return segs;
+}
+
 static struct sk_buff *__udp_gso_segment_list(struct sk_buff *skb,
-					      netdev_features_t features)
+					      netdev_features_t features,
+					      bool is_ipv6)
 {
 	unsigned int mss = skb_shinfo(skb)->gso_size;
 
@@ -198,11 +257,14 @@ static struct sk_buff *__udp_gso_segment_list(struct sk_buff *skb,
 
 	udp_hdr(skb)->len = htons(sizeof(struct udphdr) + mss);
 
-	return skb;
+	if (is_ipv6)
+		return skb;
+	else
+		return __udpv4_gso_segment_list_csum(skb);
 }
 
 struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,
-				  netdev_features_t features)
+				  netdev_features_t features, bool is_ipv6)
 {
 	struct sock *sk = gso_skb->sk;
 	unsigned int sum_truesize = 0;
@@ -214,7 +276,7 @@ struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,
 	__be16 newlen;
 
 	if (skb_shinfo(gso_skb)->gso_type & SKB_GSO_FRAGLIST)
-		return __udp_gso_segment_list(gso_skb, features);
+		return __udp_gso_segment_list(gso_skb, features, is_ipv6);
 
 	mss = skb_shinfo(gso_skb)->gso_size;
 	if (gso_skb->len <= sizeof(*uh) + mss)
@@ -328,7 +390,7 @@ static struct sk_buff *udp4_ufo_fragment(struct sk_buff *skb,
 		goto out;
 
 	if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP_L4)
-		return __udp_gso_segment(skb, features);
+		return __udp_gso_segment(skb, features, false);
 
 	mss = skb_shinfo(skb)->gso_size;
 	if (unlikely(skb->len <= mss))
diff --git a/net/ipv6/udp_offload.c b/net/ipv6/udp_offload.c
index c7bd7b1..faa823c 100644
--- a/net/ipv6/udp_offload.c
+++ b/net/ipv6/udp_offload.c
@@ -42,7 +42,7 @@ static struct sk_buff *udp6_ufo_fragment(struct sk_buff *skb,
 			goto out;
 
 		if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP_L4)
-			return __udp_gso_segment(skb, features);
+			return __udp_gso_segment(skb, features, true);
 
 		mss = skb_shinfo(skb)->gso_size;
 		if (unlikely(skb->len <= mss))
-- 
2.7.4


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

* Re: [PATCH net v3] udp: ipv4: manipulate network header of NATed UDP GRO fraglist
  2021-01-21 13:24 ` [PATCH net v3] udp: ipv4: manipulate network header of NATed UDP GRO fraglist Dongseok Yi
@ 2021-01-25 12:45   ` Steffen Klassert
  2021-01-26  0:31     ` Dongseok Yi
  0 siblings, 1 reply; 4+ messages in thread
From: Steffen Klassert @ 2021-01-25 12:45 UTC (permalink / raw)
  To: Dongseok Yi
  Cc: David S. Miller, Alexander Lobakin, namkyu78.kim, Jakub Kicinski,
	Hideaki YOSHIFUJI, Willem de Bruijn, netdev, linux-kernel

On Thu, Jan 21, 2021 at 10:24:39PM +0900, Dongseok Yi wrote:
> UDP/IP header of UDP GROed frag_skbs are not updated even after NAT
> forwarding. Only the header of head_skb from ip_finish_output_gso ->
> skb_gso_segment is updated but following frag_skbs are not updated.
> 
> A call path skb_mac_gso_segment -> inet_gso_segment ->
> udp4_ufo_fragment -> __udp_gso_segment -> __udp_gso_segment_list
> does not try to update UDP/IP header of the segment list but copy
> only the MAC header.
> 
> Update port, addr and check of each skb of the segment list in
> __udp_gso_segment_list. It covers both SNAT and DNAT.
> 
> Fixes: 9fd1ff5d2ac7 (udp: Support UDP fraglist GRO/GSO.)
> Signed-off-by: Dongseok Yi <dseok.yi@samsung.com>
> ---
> v1:
> Steffen Klassert said, there could be 2 options.
> https://lore.kernel.org/patchwork/patch/1362257/
> I was trying to write a quick fix, but it was not easy to forward
> segmented list. Currently, assuming DNAT only.
> 
> v2:
> Per Steffen Klassert request, moved the procedure from
> udp4_ufo_fragment to __udp_gso_segment_list and support SNAT.
> 
> v3:
> Per Steffen Klassert request, applied fast return by comparing seg
> and seg->next at the beginning of __udpv4_gso_segment_list_csum.
> 
> Fixed uh->dest = *newport and iph->daddr = *newip to
> *oldport = *newport and *oldip = *newip.
> 
>  include/net/udp.h      |  2 +-
>  net/ipv4/udp_offload.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++----
>  net/ipv6/udp_offload.c |  2 +-
>  3 files changed, 69 insertions(+), 7 deletions(-)
> 
> diff --git a/include/net/udp.h b/include/net/udp.h
> index 877832b..01351ba 100644
> --- a/include/net/udp.h
> +++ b/include/net/udp.h
> @@ -178,7 +178,7 @@ struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb,
>  int udp_gro_complete(struct sk_buff *skb, int nhoff, udp_lookup_t lookup);
>  
>  struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,
> -				  netdev_features_t features);
> +				  netdev_features_t features, bool is_ipv6);
>  
>  static inline struct udphdr *udp_gro_udphdr(struct sk_buff *skb)
>  {
> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> index ff39e94..43660cf 100644
> --- a/net/ipv4/udp_offload.c
> +++ b/net/ipv4/udp_offload.c
> @@ -187,8 +187,67 @@ struct sk_buff *skb_udp_tunnel_segment(struct sk_buff *skb,
>  }
>  EXPORT_SYMBOL(skb_udp_tunnel_segment);
>  
> +static void __udpv4_gso_segment_csum(struct sk_buff *seg,
> +				     __be32 *oldip, __be32 *newip,
> +				     __be16 *oldport, __be16 *newport)
> +{
> +	struct udphdr *uh;
> +	struct iphdr *iph;
> +
> +	if (*oldip == *newip && *oldport == *newport)
> +		return;

This check is redundant as you check this already in
__udpv4_gso_segment_list_csum.

Looks ok otherwise.

> +
> +	uh = udp_hdr(seg);
> +	iph = ip_hdr(seg);
> +
> +	if (uh->check) {
> +		inet_proto_csum_replace4(&uh->check, seg, *oldip, *newip,
> +					 true);
> +		inet_proto_csum_replace2(&uh->check, seg, *oldport, *newport,
> +					 false);
> +		if (!uh->check)
> +			uh->check = CSUM_MANGLED_0;
> +	}
> +	*oldport = *newport;
> +
> +	csum_replace4(&iph->check, *oldip, *newip);
> +	*oldip = *newip;
> +}
> +
> +static struct sk_buff *__udpv4_gso_segment_list_csum(struct sk_buff *segs)
> +{
> +	struct sk_buff *seg;
> +	struct udphdr *uh, *uh2;
> +	struct iphdr *iph, *iph2;
> +
> +	seg = segs;
> +	uh = udp_hdr(seg);
> +	iph = ip_hdr(seg);
> +
> +	if ((udp_hdr(seg)->dest == udp_hdr(seg->next)->dest) &&
> +	    (udp_hdr(seg)->source == udp_hdr(seg->next)->source) &&
> +	    (ip_hdr(seg)->daddr == ip_hdr(seg->next)->daddr) &&
> +	    (ip_hdr(seg)->saddr == ip_hdr(seg->next)->saddr))
> +		return segs;
> +
> +	while ((seg = seg->next)) {
> +		uh2 = udp_hdr(seg);
> +		iph2 = ip_hdr(seg);
> +
> +		__udpv4_gso_segment_csum(seg,
> +					 &iph2->saddr, &iph->saddr,
> +					 &uh2->source, &uh->source);
> +		__udpv4_gso_segment_csum(seg,
> +					 &iph2->daddr, &iph->daddr,
> +					 &uh2->dest, &uh->dest);
> +	}
> +
> +	return segs;
> +}
> +
>  static struct sk_buff *__udp_gso_segment_list(struct sk_buff *skb,
> -					      netdev_features_t features)
> +					      netdev_features_t features,
> +					      bool is_ipv6)
>  {
>  	unsigned int mss = skb_shinfo(skb)->gso_size;
>  
> @@ -198,11 +257,14 @@ static struct sk_buff *__udp_gso_segment_list(struct sk_buff *skb,
>  
>  	udp_hdr(skb)->len = htons(sizeof(struct udphdr) + mss);
>  
> -	return skb;
> +	if (is_ipv6)
> +		return skb;
> +	else
> +		return __udpv4_gso_segment_list_csum(skb);
>  }
>  
>  struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,
> -				  netdev_features_t features)
> +				  netdev_features_t features, bool is_ipv6)
>  {
>  	struct sock *sk = gso_skb->sk;
>  	unsigned int sum_truesize = 0;
> @@ -214,7 +276,7 @@ struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,
>  	__be16 newlen;
>  
>  	if (skb_shinfo(gso_skb)->gso_type & SKB_GSO_FRAGLIST)
> -		return __udp_gso_segment_list(gso_skb, features);
> +		return __udp_gso_segment_list(gso_skb, features, is_ipv6);
>  
>  	mss = skb_shinfo(gso_skb)->gso_size;
>  	if (gso_skb->len <= sizeof(*uh) + mss)
> @@ -328,7 +390,7 @@ static struct sk_buff *udp4_ufo_fragment(struct sk_buff *skb,
>  		goto out;
>  
>  	if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP_L4)
> -		return __udp_gso_segment(skb, features);
> +		return __udp_gso_segment(skb, features, false);
>  
>  	mss = skb_shinfo(skb)->gso_size;
>  	if (unlikely(skb->len <= mss))
> diff --git a/net/ipv6/udp_offload.c b/net/ipv6/udp_offload.c
> index c7bd7b1..faa823c 100644
> --- a/net/ipv6/udp_offload.c
> +++ b/net/ipv6/udp_offload.c
> @@ -42,7 +42,7 @@ static struct sk_buff *udp6_ufo_fragment(struct sk_buff *skb,
>  			goto out;
>  
>  		if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP_L4)
> -			return __udp_gso_segment(skb, features);
> +			return __udp_gso_segment(skb, features, true);
>  
>  		mss = skb_shinfo(skb)->gso_size;
>  		if (unlikely(skb->len <= mss))
> -- 
> 2.7.4

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

* RE: [PATCH net v3] udp: ipv4: manipulate network header of NATed UDP GRO fraglist
  2021-01-25 12:45   ` Steffen Klassert
@ 2021-01-26  0:31     ` Dongseok Yi
  2021-01-26 12:50       ` Steffen Klassert
  0 siblings, 1 reply; 4+ messages in thread
From: Dongseok Yi @ 2021-01-26  0:31 UTC (permalink / raw)
  To: 'Steffen Klassert'
  Cc: 'David S. Miller', 'Alexander Lobakin',
	namkyu78.kim, 'Jakub Kicinski',
	'Hideaki YOSHIFUJI', 'Willem de Bruijn',
	netdev, linux-kernel

On 1/25/21 9:45 PM, Steffen Klassert wrote:
> On Thu, Jan 21, 2021 at 10:24:39PM +0900, Dongseok Yi wrote:
> > UDP/IP header of UDP GROed frag_skbs are not updated even after NAT
> > forwarding. Only the header of head_skb from ip_finish_output_gso ->
> > skb_gso_segment is updated but following frag_skbs are not updated.
> >
> > A call path skb_mac_gso_segment -> inet_gso_segment ->
> > udp4_ufo_fragment -> __udp_gso_segment -> __udp_gso_segment_list
> > does not try to update UDP/IP header of the segment list but copy
> > only the MAC header.
> >
> > Update port, addr and check of each skb of the segment list in
> > __udp_gso_segment_list. It covers both SNAT and DNAT.
> >
> > Fixes: 9fd1ff5d2ac7 (udp: Support UDP fraglist GRO/GSO.)
> > Signed-off-by: Dongseok Yi <dseok.yi@samsung.com>
> > ---
> > v1:
> > Steffen Klassert said, there could be 2 options.
> > https://lore.kernel.org/patchwork/patch/1362257/
> > I was trying to write a quick fix, but it was not easy to forward
> > segmented list. Currently, assuming DNAT only.
> >
> > v2:
> > Per Steffen Klassert request, moved the procedure from
> > udp4_ufo_fragment to __udp_gso_segment_list and support SNAT.
> >
> > v3:
> > Per Steffen Klassert request, applied fast return by comparing seg
> > and seg->next at the beginning of __udpv4_gso_segment_list_csum.
> >
> > Fixed uh->dest = *newport and iph->daddr = *newip to
> > *oldport = *newport and *oldip = *newip.
> >
> >  include/net/udp.h      |  2 +-
> >  net/ipv4/udp_offload.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++----
> >  net/ipv6/udp_offload.c |  2 +-
> >  3 files changed, 69 insertions(+), 7 deletions(-)
> >
> > diff --git a/include/net/udp.h b/include/net/udp.h
> > index 877832b..01351ba 100644
> > --- a/include/net/udp.h
> > +++ b/include/net/udp.h
> > @@ -178,7 +178,7 @@ struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb,
> >  int udp_gro_complete(struct sk_buff *skb, int nhoff, udp_lookup_t lookup);
> >
> >  struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,
> > -				  netdev_features_t features);
> > +				  netdev_features_t features, bool is_ipv6);
> >
> >  static inline struct udphdr *udp_gro_udphdr(struct sk_buff *skb)
> >  {
> > diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> > index ff39e94..43660cf 100644
> > --- a/net/ipv4/udp_offload.c
> > +++ b/net/ipv4/udp_offload.c
> > @@ -187,8 +187,67 @@ struct sk_buff *skb_udp_tunnel_segment(struct sk_buff *skb,
> >  }
> >  EXPORT_SYMBOL(skb_udp_tunnel_segment);
> >
> > +static void __udpv4_gso_segment_csum(struct sk_buff *seg,
> > +				     __be32 *oldip, __be32 *newip,
> > +				     __be16 *oldport, __be16 *newport)
> > +{
> > +	struct udphdr *uh;
> > +	struct iphdr *iph;
> > +
> > +	if (*oldip == *newip && *oldport == *newport)
> > +		return;
> 
> This check is redundant as you check this already in
> __udpv4_gso_segment_list_csum.

When comes in __udpv4_gso_segment_csum, the condition would be
SNAT or DNAT. I think we don't need to do the function if the
condition is not met. I want to skip the function for SNAT checksum
when DNAT only case. Is it better to remove the check?

> 
> Looks ok otherwise.
> 
> > +
> > +	uh = udp_hdr(seg);
> > +	iph = ip_hdr(seg);
> > +
> > +	if (uh->check) {
> > +		inet_proto_csum_replace4(&uh->check, seg, *oldip, *newip,
> > +					 true);
> > +		inet_proto_csum_replace2(&uh->check, seg, *oldport, *newport,
> > +					 false);
> > +		if (!uh->check)
> > +			uh->check = CSUM_MANGLED_0;
> > +	}
> > +	*oldport = *newport;
> > +
> > +	csum_replace4(&iph->check, *oldip, *newip);
> > +	*oldip = *newip;
> > +}
> > +
> > +static struct sk_buff *__udpv4_gso_segment_list_csum(struct sk_buff *segs)
> > +{
> > +	struct sk_buff *seg;
> > +	struct udphdr *uh, *uh2;
> > +	struct iphdr *iph, *iph2;
> > +
> > +	seg = segs;
> > +	uh = udp_hdr(seg);
> > +	iph = ip_hdr(seg);
> > +
> > +	if ((udp_hdr(seg)->dest == udp_hdr(seg->next)->dest) &&
> > +	    (udp_hdr(seg)->source == udp_hdr(seg->next)->source) &&
> > +	    (ip_hdr(seg)->daddr == ip_hdr(seg->next)->daddr) &&
> > +	    (ip_hdr(seg)->saddr == ip_hdr(seg->next)->saddr))
> > +		return segs;
> > +
> > +	while ((seg = seg->next)) {
> > +		uh2 = udp_hdr(seg);
> > +		iph2 = ip_hdr(seg);
> > +
> > +		__udpv4_gso_segment_csum(seg,
> > +					 &iph2->saddr, &iph->saddr,
> > +					 &uh2->source, &uh->source);
> > +		__udpv4_gso_segment_csum(seg,
> > +					 &iph2->daddr, &iph->daddr,
> > +					 &uh2->dest, &uh->dest);
> > +	}
> > +
> > +	return segs;
> > +}
> > +
> >  static struct sk_buff *__udp_gso_segment_list(struct sk_buff *skb,
> > -					      netdev_features_t features)
> > +					      netdev_features_t features,
> > +					      bool is_ipv6)
> >  {
> >  	unsigned int mss = skb_shinfo(skb)->gso_size;
> >
> > @@ -198,11 +257,14 @@ static struct sk_buff *__udp_gso_segment_list(struct sk_buff *skb,
> >
> >  	udp_hdr(skb)->len = htons(sizeof(struct udphdr) + mss);
> >
> > -	return skb;
> > +	if (is_ipv6)
> > +		return skb;
> > +	else
> > +		return __udpv4_gso_segment_list_csum(skb);
> >  }
> >
> >  struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,
> > -				  netdev_features_t features)
> > +				  netdev_features_t features, bool is_ipv6)
> >  {
> >  	struct sock *sk = gso_skb->sk;
> >  	unsigned int sum_truesize = 0;
> > @@ -214,7 +276,7 @@ struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,
> >  	__be16 newlen;
> >
> >  	if (skb_shinfo(gso_skb)->gso_type & SKB_GSO_FRAGLIST)
> > -		return __udp_gso_segment_list(gso_skb, features);
> > +		return __udp_gso_segment_list(gso_skb, features, is_ipv6);
> >
> >  	mss = skb_shinfo(gso_skb)->gso_size;
> >  	if (gso_skb->len <= sizeof(*uh) + mss)
> > @@ -328,7 +390,7 @@ static struct sk_buff *udp4_ufo_fragment(struct sk_buff *skb,
> >  		goto out;
> >
> >  	if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP_L4)
> > -		return __udp_gso_segment(skb, features);
> > +		return __udp_gso_segment(skb, features, false);
> >
> >  	mss = skb_shinfo(skb)->gso_size;
> >  	if (unlikely(skb->len <= mss))
> > diff --git a/net/ipv6/udp_offload.c b/net/ipv6/udp_offload.c
> > index c7bd7b1..faa823c 100644
> > --- a/net/ipv6/udp_offload.c
> > +++ b/net/ipv6/udp_offload.c
> > @@ -42,7 +42,7 @@ static struct sk_buff *udp6_ufo_fragment(struct sk_buff *skb,
> >  			goto out;
> >
> >  		if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP_L4)
> > -			return __udp_gso_segment(skb, features);
> > +			return __udp_gso_segment(skb, features, true);
> >
> >  		mss = skb_shinfo(skb)->gso_size;
> >  		if (unlikely(skb->len <= mss))
> > --
> > 2.7.4


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

* Re: [PATCH net v3] udp: ipv4: manipulate network header of NATed UDP GRO fraglist
  2021-01-26  0:31     ` Dongseok Yi
@ 2021-01-26 12:50       ` Steffen Klassert
  0 siblings, 0 replies; 4+ messages in thread
From: Steffen Klassert @ 2021-01-26 12:50 UTC (permalink / raw)
  To: Dongseok Yi
  Cc: 'David S. Miller', 'Alexander Lobakin',
	namkyu78.kim, 'Jakub Kicinski',
	'Hideaki YOSHIFUJI', 'Willem de Bruijn',
	netdev, linux-kernel

On Tue, Jan 26, 2021 at 09:31:29AM +0900, Dongseok Yi wrote:
> On 1/25/21 9:45 PM, Steffen Klassert wrote:
> > On Thu, Jan 21, 2021 at 10:24:39PM +0900, Dongseok Yi wrote:
> > >
> > > +static void __udpv4_gso_segment_csum(struct sk_buff *seg,
> > > +				     __be32 *oldip, __be32 *newip,
> > > +				     __be16 *oldport, __be16 *newport)
> > > +{
> > > +	struct udphdr *uh;
> > > +	struct iphdr *iph;
> > > +
> > > +	if (*oldip == *newip && *oldport == *newport)
> > > +		return;
> > 
> > This check is redundant as you check this already in
> > __udpv4_gso_segment_list_csum.
> 
> When comes in __udpv4_gso_segment_csum, the condition would be
> SNAT or DNAT. I think we don't need to do the function if the
> condition is not met. I want to skip the function for SNAT checksum
> when DNAT only case. Is it better to remove the check?

Ok, so it can be seen as an optimization. It is ok as it is.

Acked-by: Steffen Klassert <steffen.klassert@secunet.com>

Thanks!

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

end of thread, other threads:[~2021-01-26 20:49 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20210121133649epcas2p493d5d59df1b48ee8e3282ab766f37a70@epcas2p4.samsung.com>
2021-01-21 13:24 ` [PATCH net v3] udp: ipv4: manipulate network header of NATed UDP GRO fraglist Dongseok Yi
2021-01-25 12:45   ` Steffen Klassert
2021-01-26  0:31     ` Dongseok Yi
2021-01-26 12:50       ` Steffen Klassert

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).