All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V2] net: gro: skb_gro_header helper function
@ 2022-08-23  7:10 Richard Gobert
  2022-08-23 16:36 ` Eric Dumazet
  2022-08-25  8:40 ` patchwork-bot+netdevbpf
  0 siblings, 2 replies; 5+ messages in thread
From: Richard Gobert @ 2022-08-23  7:10 UTC (permalink / raw)
  To: davem, edumazet, kuba, pabeni, yoshfuji, dsahern, xeb, roopa,
	eng.alaamohamedsoliman.am, bigeasy, heikki.krogerus, gregkh,
	iwienand, netdev

Introduce a simple helper function to replace a common pattern.
When accessing the GRO header, we fetch the pointer from frag0,
then test its validity and fetch it from the skb when necessary.

This leads to the pattern
skb_gro_header_fast -> skb_gro_header_hard -> skb_gro_header_slow
recurring many times throughout GRO code.

This patch replaces these patterns with a single inlined function
call, improving code readability.

Signed-off-by: Richard Gobert <richardbgobert@gmail.com>
---
 drivers/net/geneve.c           |  9 +++------
 drivers/net/vxlan/vxlan_core.c |  9 +++------
 include/net/gro.h              | 33 ++++++++++++++++++---------------
 net/8021q/vlan_core.c          |  9 +++------
 net/ethernet/eth.c             |  9 +++------
 net/ipv4/af_inet.c             |  9 +++------
 net/ipv4/fou.c                 |  9 +++------
 net/ipv4/gre_offload.c         |  9 +++------
 net/ipv4/tcp_offload.c         |  9 +++------
 net/ipv6/ip6_offload.c         |  9 +++------
 10 files changed, 45 insertions(+), 69 deletions(-)

diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index 7962c37b3f14..01aa94776ce3 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -503,12 +503,9 @@ static struct sk_buff *geneve_gro_receive(struct sock *sk,
 
 	off_gnv = skb_gro_offset(skb);
 	hlen = off_gnv + sizeof(*gh);
-	gh = skb_gro_header_fast(skb, off_gnv);
-	if (skb_gro_header_hard(skb, hlen)) {
-		gh = skb_gro_header_slow(skb, hlen, off_gnv);
-		if (unlikely(!gh))
-			goto out;
-	}
+	gh = skb_gro_header(skb, hlen, off_gnv);
+	if (unlikely(!gh))
+		goto out;
 
 	if (gh->ver != GENEVE_VER || gh->oam)
 		goto out;
diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
index c3285242f74f..1a47d04f5d1a 100644
--- a/drivers/net/vxlan/vxlan_core.c
+++ b/drivers/net/vxlan/vxlan_core.c
@@ -713,12 +713,9 @@ static struct sk_buff *vxlan_gro_receive(struct sock *sk,
 
 	off_vx = skb_gro_offset(skb);
 	hlen = off_vx + sizeof(*vh);
-	vh   = skb_gro_header_fast(skb, off_vx);
-	if (skb_gro_header_hard(skb, hlen)) {
-		vh = skb_gro_header_slow(skb, hlen, off_vx);
-		if (unlikely(!vh))
-			goto out;
-	}
+	vh = skb_gro_header(skb, hlen, off_vx);
+	if (unlikely(!vh))
+		goto out;
 
 	skb_gro_postpull_rcsum(skb, vh, sizeof(struct vxlanhdr));
 
diff --git a/include/net/gro.h b/include/net/gro.h
index 867656b0739c..5bf15c212434 100644
--- a/include/net/gro.h
+++ b/include/net/gro.h
@@ -160,6 +160,17 @@ static inline void *skb_gro_header_slow(struct sk_buff *skb, unsigned int hlen,
 	return skb->data + offset;
 }
 
+static inline void *skb_gro_header(struct sk_buff *skb,
+					unsigned int hlen, unsigned int offset)
+{
+	void *ptr;
+
+	ptr = skb_gro_header_fast(skb, offset);
+	if (skb_gro_header_hard(skb, hlen))
+		ptr = skb_gro_header_slow(skb, hlen, offset);
+	return ptr;
+}
+
 static inline void *skb_gro_network_header(struct sk_buff *skb)
 {
 	return (NAPI_GRO_CB(skb)->frag0 ?: skb->data) +
@@ -301,12 +312,9 @@ static inline void *skb_gro_remcsum_process(struct sk_buff *skb, void *ptr,
 		return ptr;
 	}
 
-	ptr = skb_gro_header_fast(skb, off);
-	if (skb_gro_header_hard(skb, off + plen)) {
-		ptr = skb_gro_header_slow(skb, off + plen, off);
-		if (!ptr)
-			return NULL;
-	}
+	ptr = skb_gro_header(skb, off + plen, off);
+	if (!ptr)
+		return NULL;
 
 	delta = remcsum_adjust(ptr + hdrlen, NAPI_GRO_CB(skb)->csum,
 			       start, offset);
@@ -329,12 +337,9 @@ static inline void skb_gro_remcsum_cleanup(struct sk_buff *skb,
 	if (!grc->delta)
 		return;
 
-	ptr = skb_gro_header_fast(skb, grc->offset);
-	if (skb_gro_header_hard(skb, grc->offset + sizeof(u16))) {
-		ptr = skb_gro_header_slow(skb, plen, grc->offset);
-		if (!ptr)
-			return;
-	}
+	ptr = skb_gro_header(skb, plen, grc->offset);
+	if (!ptr)
+		return;
 
 	remcsum_unadjust((__sum16 *)ptr, grc->delta);
 }
@@ -405,9 +410,7 @@ static inline struct udphdr *udp_gro_udphdr(struct sk_buff *skb)
 
 	off  = skb_gro_offset(skb);
 	hlen = off + sizeof(*uh);
-	uh   = skb_gro_header_fast(skb, off);
-	if (skb_gro_header_hard(skb, hlen))
-		uh = skb_gro_header_slow(skb, hlen, off);
+	uh   = skb_gro_header(skb, hlen, off);
 
 	return uh;
 }
diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
index 5aa8144101dc..0beb44f2fe1f 100644
--- a/net/8021q/vlan_core.c
+++ b/net/8021q/vlan_core.c
@@ -467,12 +467,9 @@ static struct sk_buff *vlan_gro_receive(struct list_head *head,
 
 	off_vlan = skb_gro_offset(skb);
 	hlen = off_vlan + sizeof(*vhdr);
-	vhdr = skb_gro_header_fast(skb, off_vlan);
-	if (skb_gro_header_hard(skb, hlen)) {
-		vhdr = skb_gro_header_slow(skb, hlen, off_vlan);
-		if (unlikely(!vhdr))
-			goto out;
-	}
+	vhdr = skb_gro_header(skb, hlen, off_vlan);
+	if (unlikely(!vhdr))
+		goto out;
 
 	type = vhdr->h_vlan_encapsulated_proto;
 
diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
index 62b89d6f54fd..e02daa74e833 100644
--- a/net/ethernet/eth.c
+++ b/net/ethernet/eth.c
@@ -414,12 +414,9 @@ struct sk_buff *eth_gro_receive(struct list_head *head, struct sk_buff *skb)
 
 	off_eth = skb_gro_offset(skb);
 	hlen = off_eth + sizeof(*eh);
-	eh = skb_gro_header_fast(skb, off_eth);
-	if (skb_gro_header_hard(skb, hlen)) {
-		eh = skb_gro_header_slow(skb, hlen, off_eth);
-		if (unlikely(!eh))
-			goto out;
-	}
+	eh = skb_gro_header(skb, hlen, off_eth);
+	if (unlikely(!eh))
+		goto out;
 
 	flush = 0;
 
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 3ca0cc467886..1676e5b9e000 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1448,12 +1448,9 @@ struct sk_buff *inet_gro_receive(struct list_head *head, struct sk_buff *skb)
 
 	off = skb_gro_offset(skb);
 	hlen = off + sizeof(*iph);
-	iph = skb_gro_header_fast(skb, off);
-	if (skb_gro_header_hard(skb, hlen)) {
-		iph = skb_gro_header_slow(skb, hlen, off);
-		if (unlikely(!iph))
-			goto out;
-	}
+	iph = skb_gro_header(skb, hlen, off);
+	if (unlikely(!iph))
+		goto out;
 
 	proto = iph->protocol;
 
diff --git a/net/ipv4/fou.c b/net/ipv4/fou.c
index 025a33c1b04d..cb5bfb77944c 100644
--- a/net/ipv4/fou.c
+++ b/net/ipv4/fou.c
@@ -323,12 +323,9 @@ static struct sk_buff *gue_gro_receive(struct sock *sk,
 	off = skb_gro_offset(skb);
 	len = off + sizeof(*guehdr);
 
-	guehdr = skb_gro_header_fast(skb, off);
-	if (skb_gro_header_hard(skb, len)) {
-		guehdr = skb_gro_header_slow(skb, len, off);
-		if (unlikely(!guehdr))
-			goto out;
-	}
+	guehdr = skb_gro_header(skb, len, off);
+	if (unlikely(!guehdr))
+		goto out;
 
 	switch (guehdr->version) {
 	case 0:
diff --git a/net/ipv4/gre_offload.c b/net/ipv4/gre_offload.c
index 07073fa35205..2b9cb5398335 100644
--- a/net/ipv4/gre_offload.c
+++ b/net/ipv4/gre_offload.c
@@ -137,12 +137,9 @@ static struct sk_buff *gre_gro_receive(struct list_head *head,
 
 	off = skb_gro_offset(skb);
 	hlen = off + sizeof(*greh);
-	greh = skb_gro_header_fast(skb, off);
-	if (skb_gro_header_hard(skb, hlen)) {
-		greh = skb_gro_header_slow(skb, hlen, off);
-		if (unlikely(!greh))
-			goto out;
-	}
+	greh = skb_gro_header(skb, hlen, off);
+	if (unlikely(!greh))
+		goto out;
 
 	/* Only support version 0 and K (key), C (csum) flags. Note that
 	 * although the support for the S (seq#) flag can be added easily
diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c
index 30abde86db45..a844a0d38482 100644
--- a/net/ipv4/tcp_offload.c
+++ b/net/ipv4/tcp_offload.c
@@ -195,12 +195,9 @@ struct sk_buff *tcp_gro_receive(struct list_head *head, struct sk_buff *skb)
 
 	off = skb_gro_offset(skb);
 	hlen = off + sizeof(*th);
-	th = skb_gro_header_fast(skb, off);
-	if (skb_gro_header_hard(skb, hlen)) {
-		th = skb_gro_header_slow(skb, hlen, off);
-		if (unlikely(!th))
-			goto out;
-	}
+	th = skb_gro_header(skb, hlen, off);
+	if (unlikely(!th))
+		goto out;
 
 	thlen = th->doff * 4;
 	if (thlen < sizeof(*th))
diff --git a/net/ipv6/ip6_offload.c b/net/ipv6/ip6_offload.c
index d12dba2dd535..d37a8c97e6de 100644
--- a/net/ipv6/ip6_offload.c
+++ b/net/ipv6/ip6_offload.c
@@ -219,12 +219,9 @@ INDIRECT_CALLABLE_SCOPE struct sk_buff *ipv6_gro_receive(struct list_head *head,
 
 	off = skb_gro_offset(skb);
 	hlen = off + sizeof(*iph);
-	iph = skb_gro_header_fast(skb, off);
-	if (skb_gro_header_hard(skb, hlen)) {
-		iph = skb_gro_header_slow(skb, hlen, off);
-		if (unlikely(!iph))
-			goto out;
-	}
+	iph = skb_gro_header_slow(skb, hlen, off);
+	if (unlikely(!iph))
+		goto out;
 
 	skb_set_network_header(skb, off);
 	skb_gro_pull(skb, sizeof(*iph));
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH V2] net: gro: skb_gro_header helper function
  2022-08-23  7:10 [PATCH V2] net: gro: skb_gro_header helper function Richard Gobert
@ 2022-08-23 16:36 ` Eric Dumazet
  2022-08-25  8:40 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 5+ messages in thread
From: Eric Dumazet @ 2022-08-23 16:36 UTC (permalink / raw)
  To: Richard Gobert
  Cc: David Miller, Jakub Kicinski, Paolo Abeni, Hideaki YOSHIFUJI,
	David Ahern, Dmitry Kozlov, Roopa Prabhu,
	eng.alaamohamedsoliman.am, Sebastian Andrzej Siewior,
	heikki.krogerus, Greg Kroah-Hartman, iwienand, netdev

On Tue, Aug 23, 2022 at 12:11 AM Richard Gobert
<richardbgobert@gmail.com> wrote:
>
> Introduce a simple helper function to replace a common pattern.
> When accessing the GRO header, we fetch the pointer from frag0,
> then test its validity and fetch it from the skb when necessary.
>
> This leads to the pattern
> skb_gro_header_fast -> skb_gro_header_hard -> skb_gro_header_slow
> recurring many times throughout GRO code.
>
> This patch replaces these patterns with a single inlined function
> call, improving code readability.
>
> Signed-off-by: Richard Gobert <richardbgobert@gmail.com>

Reviewed-by: Eric Dumazet <edumazet@google.com>

Please next time, add what has changed between V1 and V2, after the --- marker.

Thanks.


> ---
>  drivers/net/geneve.c           |  9 +++------
>  drivers/net/vxlan/vxlan_core.c |  9 +++------
>  include/net/gro.h              | 33 ++++++++++++++++++---------------
>  net/8021q/vlan_core.c          |  9 +++------
>  net/ethernet/eth.c             |  9 +++------
>  net/ipv4/af_inet.c             |  9 +++------
>  net/ipv4/fou.c                 |  9 +++------
>  net/ipv4/gre_offload.c         |  9 +++------
>  net/ipv4/tcp_offload.c         |  9 +++------
>  net/ipv6/ip6_offload.c         |  9 +++------
>  10 files changed, 45 insertions(+), 69 deletions(-)
>
> diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
> index 7962c37b3f14..01aa94776ce3 100644
> --- a/drivers/net/geneve.c
> +++ b/drivers/net/geneve.c
> @@ -503,12 +503,9 @@ static struct sk_buff *geneve_gro_receive(struct sock *sk,
>
>         off_gnv = skb_gro_offset(skb);
>         hlen = off_gnv + sizeof(*gh);
> -       gh = skb_gro_header_fast(skb, off_gnv);
> -       if (skb_gro_header_hard(skb, hlen)) {
> -               gh = skb_gro_header_slow(skb, hlen, off_gnv);
> -               if (unlikely(!gh))
> -                       goto out;
> -       }
> +       gh = skb_gro_header(skb, hlen, off_gnv);
> +       if (unlikely(!gh))
> +               goto out;
>
>         if (gh->ver != GENEVE_VER || gh->oam)
>                 goto out;
> diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
> index c3285242f74f..1a47d04f5d1a 100644
> --- a/drivers/net/vxlan/vxlan_core.c
> +++ b/drivers/net/vxlan/vxlan_core.c
> @@ -713,12 +713,9 @@ static struct sk_buff *vxlan_gro_receive(struct sock *sk,
>
>         off_vx = skb_gro_offset(skb);
>         hlen = off_vx + sizeof(*vh);
> -       vh   = skb_gro_header_fast(skb, off_vx);
> -       if (skb_gro_header_hard(skb, hlen)) {
> -               vh = skb_gro_header_slow(skb, hlen, off_vx);
> -               if (unlikely(!vh))
> -                       goto out;
> -       }
> +       vh = skb_gro_header(skb, hlen, off_vx);
> +       if (unlikely(!vh))
> +               goto out;
>
>         skb_gro_postpull_rcsum(skb, vh, sizeof(struct vxlanhdr));
>
> diff --git a/include/net/gro.h b/include/net/gro.h
> index 867656b0739c..5bf15c212434 100644
> --- a/include/net/gro.h
> +++ b/include/net/gro.h
> @@ -160,6 +160,17 @@ static inline void *skb_gro_header_slow(struct sk_buff *skb, unsigned int hlen,
>         return skb->data + offset;
>  }
>
> +static inline void *skb_gro_header(struct sk_buff *skb,
> +                                       unsigned int hlen, unsigned int offset)
> +{
> +       void *ptr;
> +
> +       ptr = skb_gro_header_fast(skb, offset);
> +       if (skb_gro_header_hard(skb, hlen))
> +               ptr = skb_gro_header_slow(skb, hlen, offset);
> +       return ptr;
> +}
> +
>  static inline void *skb_gro_network_header(struct sk_buff *skb)
>  {
>         return (NAPI_GRO_CB(skb)->frag0 ?: skb->data) +
> @@ -301,12 +312,9 @@ static inline void *skb_gro_remcsum_process(struct sk_buff *skb, void *ptr,
>                 return ptr;
>         }
>
> -       ptr = skb_gro_header_fast(skb, off);
> -       if (skb_gro_header_hard(skb, off + plen)) {
> -               ptr = skb_gro_header_slow(skb, off + plen, off);
> -               if (!ptr)
> -                       return NULL;
> -       }
> +       ptr = skb_gro_header(skb, off + plen, off);
> +       if (!ptr)
> +               return NULL;
>
>         delta = remcsum_adjust(ptr + hdrlen, NAPI_GRO_CB(skb)->csum,
>                                start, offset);
> @@ -329,12 +337,9 @@ static inline void skb_gro_remcsum_cleanup(struct sk_buff *skb,
>         if (!grc->delta)
>                 return;
>
> -       ptr = skb_gro_header_fast(skb, grc->offset);
> -       if (skb_gro_header_hard(skb, grc->offset + sizeof(u16))) {
> -               ptr = skb_gro_header_slow(skb, plen, grc->offset);
> -               if (!ptr)
> -                       return;
> -       }
> +       ptr = skb_gro_header(skb, plen, grc->offset);
> +       if (!ptr)
> +               return;
>
>         remcsum_unadjust((__sum16 *)ptr, grc->delta);
>  }
> @@ -405,9 +410,7 @@ static inline struct udphdr *udp_gro_udphdr(struct sk_buff *skb)
>
>         off  = skb_gro_offset(skb);
>         hlen = off + sizeof(*uh);
> -       uh   = skb_gro_header_fast(skb, off);
> -       if (skb_gro_header_hard(skb, hlen))
> -               uh = skb_gro_header_slow(skb, hlen, off);
> +       uh   = skb_gro_header(skb, hlen, off);
>
>         return uh;
>  }
> diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
> index 5aa8144101dc..0beb44f2fe1f 100644
> --- a/net/8021q/vlan_core.c
> +++ b/net/8021q/vlan_core.c
> @@ -467,12 +467,9 @@ static struct sk_buff *vlan_gro_receive(struct list_head *head,
>
>         off_vlan = skb_gro_offset(skb);
>         hlen = off_vlan + sizeof(*vhdr);
> -       vhdr = skb_gro_header_fast(skb, off_vlan);
> -       if (skb_gro_header_hard(skb, hlen)) {
> -               vhdr = skb_gro_header_slow(skb, hlen, off_vlan);
> -               if (unlikely(!vhdr))
> -                       goto out;
> -       }
> +       vhdr = skb_gro_header(skb, hlen, off_vlan);
> +       if (unlikely(!vhdr))
> +               goto out;
>
>         type = vhdr->h_vlan_encapsulated_proto;
>
> diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
> index 62b89d6f54fd..e02daa74e833 100644
> --- a/net/ethernet/eth.c
> +++ b/net/ethernet/eth.c
> @@ -414,12 +414,9 @@ struct sk_buff *eth_gro_receive(struct list_head *head, struct sk_buff *skb)
>
>         off_eth = skb_gro_offset(skb);
>         hlen = off_eth + sizeof(*eh);
> -       eh = skb_gro_header_fast(skb, off_eth);
> -       if (skb_gro_header_hard(skb, hlen)) {
> -               eh = skb_gro_header_slow(skb, hlen, off_eth);
> -               if (unlikely(!eh))
> -                       goto out;
> -       }
> +       eh = skb_gro_header(skb, hlen, off_eth);
> +       if (unlikely(!eh))
> +               goto out;
>
>         flush = 0;
>
> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> index 3ca0cc467886..1676e5b9e000 100644
> --- a/net/ipv4/af_inet.c
> +++ b/net/ipv4/af_inet.c
> @@ -1448,12 +1448,9 @@ struct sk_buff *inet_gro_receive(struct list_head *head, struct sk_buff *skb)
>
>         off = skb_gro_offset(skb);
>         hlen = off + sizeof(*iph);
> -       iph = skb_gro_header_fast(skb, off);
> -       if (skb_gro_header_hard(skb, hlen)) {
> -               iph = skb_gro_header_slow(skb, hlen, off);
> -               if (unlikely(!iph))
> -                       goto out;
> -       }
> +       iph = skb_gro_header(skb, hlen, off);
> +       if (unlikely(!iph))
> +               goto out;
>
>         proto = iph->protocol;
>
> diff --git a/net/ipv4/fou.c b/net/ipv4/fou.c
> index 025a33c1b04d..cb5bfb77944c 100644
> --- a/net/ipv4/fou.c
> +++ b/net/ipv4/fou.c
> @@ -323,12 +323,9 @@ static struct sk_buff *gue_gro_receive(struct sock *sk,
>         off = skb_gro_offset(skb);
>         len = off + sizeof(*guehdr);
>
> -       guehdr = skb_gro_header_fast(skb, off);
> -       if (skb_gro_header_hard(skb, len)) {
> -               guehdr = skb_gro_header_slow(skb, len, off);
> -               if (unlikely(!guehdr))
> -                       goto out;
> -       }
> +       guehdr = skb_gro_header(skb, len, off);
> +       if (unlikely(!guehdr))
> +               goto out;
>
>         switch (guehdr->version) {
>         case 0:
> diff --git a/net/ipv4/gre_offload.c b/net/ipv4/gre_offload.c
> index 07073fa35205..2b9cb5398335 100644
> --- a/net/ipv4/gre_offload.c
> +++ b/net/ipv4/gre_offload.c
> @@ -137,12 +137,9 @@ static struct sk_buff *gre_gro_receive(struct list_head *head,
>
>         off = skb_gro_offset(skb);
>         hlen = off + sizeof(*greh);
> -       greh = skb_gro_header_fast(skb, off);
> -       if (skb_gro_header_hard(skb, hlen)) {
> -               greh = skb_gro_header_slow(skb, hlen, off);
> -               if (unlikely(!greh))
> -                       goto out;
> -       }
> +       greh = skb_gro_header(skb, hlen, off);
> +       if (unlikely(!greh))
> +               goto out;
>
>         /* Only support version 0 and K (key), C (csum) flags. Note that
>          * although the support for the S (seq#) flag can be added easily
> diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c
> index 30abde86db45..a844a0d38482 100644
> --- a/net/ipv4/tcp_offload.c
> +++ b/net/ipv4/tcp_offload.c
> @@ -195,12 +195,9 @@ struct sk_buff *tcp_gro_receive(struct list_head *head, struct sk_buff *skb)
>
>         off = skb_gro_offset(skb);
>         hlen = off + sizeof(*th);
> -       th = skb_gro_header_fast(skb, off);
> -       if (skb_gro_header_hard(skb, hlen)) {
> -               th = skb_gro_header_slow(skb, hlen, off);
> -               if (unlikely(!th))
> -                       goto out;
> -       }
> +       th = skb_gro_header(skb, hlen, off);
> +       if (unlikely(!th))
> +               goto out;
>
>         thlen = th->doff * 4;
>         if (thlen < sizeof(*th))
> diff --git a/net/ipv6/ip6_offload.c b/net/ipv6/ip6_offload.c
> index d12dba2dd535..d37a8c97e6de 100644
> --- a/net/ipv6/ip6_offload.c
> +++ b/net/ipv6/ip6_offload.c
> @@ -219,12 +219,9 @@ INDIRECT_CALLABLE_SCOPE struct sk_buff *ipv6_gro_receive(struct list_head *head,
>
>         off = skb_gro_offset(skb);
>         hlen = off + sizeof(*iph);
> -       iph = skb_gro_header_fast(skb, off);
> -       if (skb_gro_header_hard(skb, hlen)) {
> -               iph = skb_gro_header_slow(skb, hlen, off);
> -               if (unlikely(!iph))
> -                       goto out;
> -       }
> +       iph = skb_gro_header_slow(skb, hlen, off);
> +       if (unlikely(!iph))
> +               goto out;
>
>         skb_set_network_header(skb, off);
>         skb_gro_pull(skb, sizeof(*iph));
> --
> 2.36.1
>

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH V2] net: gro: skb_gro_header helper function
  2022-08-23  7:10 [PATCH V2] net: gro: skb_gro_header helper function Richard Gobert
  2022-08-23 16:36 ` Eric Dumazet
@ 2022-08-25  8:40 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 5+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-08-25  8:40 UTC (permalink / raw)
  To: Richard Gobert
  Cc: davem, edumazet, kuba, pabeni, yoshfuji, dsahern, xeb, roopa,
	eng.alaamohamedsoliman.am, bigeasy, heikki.krogerus, gregkh,
	iwienand, netdev

Hello:

This patch was applied to netdev/net-next.git (master)
by Paolo Abeni <pabeni@redhat.com>:

On Tue, 23 Aug 2022 09:10:49 +0200 you wrote:
> Introduce a simple helper function to replace a common pattern.
> When accessing the GRO header, we fetch the pointer from frag0,
> then test its validity and fetch it from the skb when necessary.
> 
> This leads to the pattern
> skb_gro_header_fast -> skb_gro_header_hard -> skb_gro_header_slow
> recurring many times throughout GRO code.
> 
> [...]

Here is the summary with links:
  - [V2] net: gro: skb_gro_header helper function
    https://git.kernel.org/netdev/net-next/c/35ffb6654729

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH v2] net: gro: skb_gro_header helper function
  2022-08-02 14:38 [PATCH v2] " Richard Gobert
@ 2022-08-02 19:28 ` Jakub Kicinski
  0 siblings, 0 replies; 5+ messages in thread
From: Jakub Kicinski @ 2022-08-02 19:28 UTC (permalink / raw)
  To: Richard Gobert
  Cc: davem, pabeni, yoshfuji, dsahern, xeb, edumazet, roopa, bigeasy,
	iwienand, heikki.krogerus, arnd, netdev

On Tue, 2 Aug 2022 16:38:02 +0200 Richard Gobert wrote:
> Introduce a simple helper function to replace a common pattern.
> When accessing the GRO header, we fetch the pointer from frag0,
> then test its validity and fetch it from the skb when necessary.
> 
> This leads to the pattern
> skb_gro_header_fast -> skb_gro_header_hard -> skb_gro_header_slow
> recurring many times throughout GRO code.
> 
> This patch replaces these patterns with a single inlined function
> call, improving code readability.
> 
> Signed-off-by: Richard Gobert <richardbgobert@gmail.com>

# Form letter - net-next is closed

The merge window for Linux 6.0 has started and therefore 
net-next is closed for new drivers, features, code refactoring 
and optimizations. We are currently accepting bug fixes only.

Please repost when net-next reopens after 6.0-rc1 is cut.

RFC patches sent for review only are obviously welcome at any time.

^ permalink raw reply	[flat|nested] 5+ messages in thread
* [PATCH v2] net: gro: skb_gro_header helper function
@ 2022-08-02 14:38 Richard Gobert
  2022-08-02 19:28 ` Jakub Kicinski
  0 siblings, 1 reply; 5+ messages in thread
From: Richard Gobert @ 2022-08-02 14:38 UTC (permalink / raw)
  To: davem, kuba, pabeni, yoshfuji, dsahern, xeb, edumazet, roopa,
	bigeasy, iwienand, heikki.krogerus, arnd, netdev

Introduce a simple helper function to replace a common pattern.
When accessing the GRO header, we fetch the pointer from frag0,
then test its validity and fetch it from the skb when necessary.

This leads to the pattern
skb_gro_header_fast -> skb_gro_header_hard -> skb_gro_header_slow
recurring many times throughout GRO code.

This patch replaces these patterns with a single inlined function
call, improving code readability.

Signed-off-by: Richard Gobert <richardbgobert@gmail.com>
---
 drivers/net/geneve.c           |  9 +++------
 drivers/net/vxlan/vxlan_core.c |  9 +++------
 include/net/gro.h              | 33 ++++++++++++++++++---------------
 net/8021q/vlan_core.c          |  9 +++------
 net/ethernet/eth.c             |  9 +++------
 net/ipv4/af_inet.c             |  9 +++------
 net/ipv4/fou.c                 |  9 +++------
 net/ipv4/gre_offload.c         |  9 +++------
 net/ipv4/tcp_offload.c         |  9 +++------
 net/ipv6/ip6_offload.c         |  9 +++------
 10 files changed, 45 insertions(+), 69 deletions(-)

diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index 2495a5719e1c..98c18a7a27d3 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -503,12 +503,9 @@ static struct sk_buff *geneve_gro_receive(struct sock *sk,
 
 	off_gnv = skb_gro_offset(skb);
 	hlen = off_gnv + sizeof(*gh);
-	gh = skb_gro_header_fast(skb, off_gnv);
-	if (skb_gro_header_hard(skb, hlen)) {
-		gh = skb_gro_header_slow(skb, hlen, off_gnv);
-		if (unlikely(!gh))
-			goto out;
-	}
+	gh = skb_gro_header(skb, hlen, off_gnv);
+	if (unlikely(!gh))
+		goto out;
 
 	if (gh->ver != GENEVE_VER || gh->oam)
 		goto out;
diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
index 8a5e3a6d32d7..b55d7ddab12a 100644
--- a/drivers/net/vxlan/vxlan_core.c
+++ b/drivers/net/vxlan/vxlan_core.c
@@ -713,12 +713,9 @@ static struct sk_buff *vxlan_gro_receive(struct sock *sk,
 
 	off_vx = skb_gro_offset(skb);
 	hlen = off_vx + sizeof(*vh);
-	vh   = skb_gro_header_fast(skb, off_vx);
-	if (skb_gro_header_hard(skb, hlen)) {
-		vh = skb_gro_header_slow(skb, hlen, off_vx);
-		if (unlikely(!vh))
-			goto out;
-	}
+	vh = skb_gro_header(skb, hlen, off_vx);
+	if (unlikely(!vh))
+		goto out;
 
 	skb_gro_postpull_rcsum(skb, vh, sizeof(struct vxlanhdr));
 
diff --git a/include/net/gro.h b/include/net/gro.h
index 867656b0739c..5bf15c212434 100644
--- a/include/net/gro.h
+++ b/include/net/gro.h
@@ -160,6 +160,17 @@ static inline void *skb_gro_header_slow(struct sk_buff *skb, unsigned int hlen,
 	return skb->data + offset;
 }
 
+static inline void *skb_gro_header(struct sk_buff *skb,
+					unsigned int hlen, unsigned int offset)
+{
+	void *ptr;
+
+	ptr = skb_gro_header_fast(skb, offset);
+	if (skb_gro_header_hard(skb, hlen))
+		ptr = skb_gro_header_slow(skb, hlen, offset);
+	return ptr;
+}
+
 static inline void *skb_gro_network_header(struct sk_buff *skb)
 {
 	return (NAPI_GRO_CB(skb)->frag0 ?: skb->data) +
@@ -301,12 +312,9 @@ static inline void *skb_gro_remcsum_process(struct sk_buff *skb, void *ptr,
 		return ptr;
 	}
 
-	ptr = skb_gro_header_fast(skb, off);
-	if (skb_gro_header_hard(skb, off + plen)) {
-		ptr = skb_gro_header_slow(skb, off + plen, off);
-		if (!ptr)
-			return NULL;
-	}
+	ptr = skb_gro_header(skb, off + plen, off);
+	if (!ptr)
+		return NULL;
 
 	delta = remcsum_adjust(ptr + hdrlen, NAPI_GRO_CB(skb)->csum,
 			       start, offset);
@@ -329,12 +337,9 @@ static inline void skb_gro_remcsum_cleanup(struct sk_buff *skb,
 	if (!grc->delta)
 		return;
 
-	ptr = skb_gro_header_fast(skb, grc->offset);
-	if (skb_gro_header_hard(skb, grc->offset + sizeof(u16))) {
-		ptr = skb_gro_header_slow(skb, plen, grc->offset);
-		if (!ptr)
-			return;
-	}
+	ptr = skb_gro_header(skb, plen, grc->offset);
+	if (!ptr)
+		return;
 
 	remcsum_unadjust((__sum16 *)ptr, grc->delta);
 }
@@ -405,9 +410,7 @@ static inline struct udphdr *udp_gro_udphdr(struct sk_buff *skb)
 
 	off  = skb_gro_offset(skb);
 	hlen = off + sizeof(*uh);
-	uh   = skb_gro_header_fast(skb, off);
-	if (skb_gro_header_hard(skb, hlen))
-		uh = skb_gro_header_slow(skb, hlen, off);
+	uh   = skb_gro_header(skb, hlen, off);
 
 	return uh;
 }
diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
index acf8c791f320..91506dd11156 100644
--- a/net/8021q/vlan_core.c
+++ b/net/8021q/vlan_core.c
@@ -467,12 +467,9 @@ static struct sk_buff *vlan_gro_receive(struct list_head *head,
 
 	off_vlan = skb_gro_offset(skb);
 	hlen = off_vlan + sizeof(*vhdr);
-	vhdr = skb_gro_header_fast(skb, off_vlan);
-	if (skb_gro_header_hard(skb, hlen)) {
-		vhdr = skb_gro_header_slow(skb, hlen, off_vlan);
-		if (unlikely(!vhdr))
-			goto out;
-	}
+	vhdr = skb_gro_header(skb, hlen, off_vlan);
+	if (unlikely(!vhdr))
+		goto out;
 
 	type = vhdr->h_vlan_encapsulated_proto;
 
diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
index 62b89d6f54fd..e02daa74e833 100644
--- a/net/ethernet/eth.c
+++ b/net/ethernet/eth.c
@@ -414,12 +414,9 @@ struct sk_buff *eth_gro_receive(struct list_head *head, struct sk_buff *skb)
 
 	off_eth = skb_gro_offset(skb);
 	hlen = off_eth + sizeof(*eh);
-	eh = skb_gro_header_fast(skb, off_eth);
-	if (skb_gro_header_hard(skb, hlen)) {
-		eh = skb_gro_header_slow(skb, hlen, off_eth);
-		if (unlikely(!eh))
-			goto out;
-	}
+	eh = skb_gro_header(skb, hlen, off_eth);
+	if (unlikely(!eh))
+		goto out;
 
 	flush = 0;
 
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 93da9f783bec..59488f888e12 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1447,12 +1447,9 @@ struct sk_buff *inet_gro_receive(struct list_head *head, struct sk_buff *skb)
 
 	off = skb_gro_offset(skb);
 	hlen = off + sizeof(*iph);
-	iph = skb_gro_header_fast(skb, off);
-	if (skb_gro_header_hard(skb, hlen)) {
-		iph = skb_gro_header_slow(skb, hlen, off);
-		if (unlikely(!iph))
-			goto out;
-	}
+	iph = skb_gro_header(skb, hlen, off);
+	if (unlikely(!iph))
+		goto out;
 
 	proto = iph->protocol;
 
diff --git a/net/ipv4/fou.c b/net/ipv4/fou.c
index 025a33c1b04d..cb5bfb77944c 100644
--- a/net/ipv4/fou.c
+++ b/net/ipv4/fou.c
@@ -323,12 +323,9 @@ static struct sk_buff *gue_gro_receive(struct sock *sk,
 	off = skb_gro_offset(skb);
 	len = off + sizeof(*guehdr);
 
-	guehdr = skb_gro_header_fast(skb, off);
-	if (skb_gro_header_hard(skb, len)) {
-		guehdr = skb_gro_header_slow(skb, len, off);
-		if (unlikely(!guehdr))
-			goto out;
-	}
+	guehdr = skb_gro_header(skb, len, off);
+	if (unlikely(!guehdr))
+		goto out;
 
 	switch (guehdr->version) {
 	case 0:
diff --git a/net/ipv4/gre_offload.c b/net/ipv4/gre_offload.c
index 07073fa35205..2b9cb5398335 100644
--- a/net/ipv4/gre_offload.c
+++ b/net/ipv4/gre_offload.c
@@ -137,12 +137,9 @@ static struct sk_buff *gre_gro_receive(struct list_head *head,
 
 	off = skb_gro_offset(skb);
 	hlen = off + sizeof(*greh);
-	greh = skb_gro_header_fast(skb, off);
-	if (skb_gro_header_hard(skb, hlen)) {
-		greh = skb_gro_header_slow(skb, hlen, off);
-		if (unlikely(!greh))
-			goto out;
-	}
+	greh = skb_gro_header(skb, hlen, off);
+	if (unlikely(!greh))
+		goto out;
 
 	/* Only support version 0 and K (key), C (csum) flags. Note that
 	 * although the support for the S (seq#) flag can be added easily
diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c
index 30abde86db45..a844a0d38482 100644
--- a/net/ipv4/tcp_offload.c
+++ b/net/ipv4/tcp_offload.c
@@ -195,12 +195,9 @@ struct sk_buff *tcp_gro_receive(struct list_head *head, struct sk_buff *skb)
 
 	off = skb_gro_offset(skb);
 	hlen = off + sizeof(*th);
-	th = skb_gro_header_fast(skb, off);
-	if (skb_gro_header_hard(skb, hlen)) {
-		th = skb_gro_header_slow(skb, hlen, off);
-		if (unlikely(!th))
-			goto out;
-	}
+	th = skb_gro_header(skb, hlen, off);
+	if (unlikely(!th))
+		goto out;
 
 	thlen = th->doff * 4;
 	if (thlen < sizeof(*th))
diff --git a/net/ipv6/ip6_offload.c b/net/ipv6/ip6_offload.c
index c4fc03c1ac99..616d17b8ca5e 100644
--- a/net/ipv6/ip6_offload.c
+++ b/net/ipv6/ip6_offload.c
@@ -197,12 +197,9 @@ INDIRECT_CALLABLE_SCOPE struct sk_buff *ipv6_gro_receive(struct list_head *head,
 
 	off = skb_gro_offset(skb);
 	hlen = off + sizeof(*iph);
-	iph = skb_gro_header_fast(skb, off);
-	if (skb_gro_header_hard(skb, hlen)) {
-		iph = skb_gro_header_slow(skb, hlen, off);
-		if (unlikely(!iph))
-			goto out;
-	}
+	iph = skb_gro_header_slow(skb, hlen, off);
+	if (unlikely(!iph))
+		goto out;
 
 	skb_set_network_header(skb, off);
 	skb_gro_pull(skb, sizeof(*iph));
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-08-25  8:40 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-23  7:10 [PATCH V2] net: gro: skb_gro_header helper function Richard Gobert
2022-08-23 16:36 ` Eric Dumazet
2022-08-25  8:40 ` patchwork-bot+netdevbpf
  -- strict thread matches above, loose matches on Subject: below --
2022-08-02 14:38 [PATCH v2] " Richard Gobert
2022-08-02 19:28 ` Jakub Kicinski

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.