net: gro: skb_gro_header_try_fast helper function

PATCH Superseded Richard Gobert 3 participants 3 messages Jul 28, 2022

Subsystems (3)

Networking Network drivers TCP

lore

Share & tools

lore

Richard Gobert

author

2022-07-28 11:41 UTC #0 lore raw

PATCH 1/1 net: gro: skb_gro_header_try_fast helper function

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 the GRO code.

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

Signed-off-by Richard Gobert <[email protected]>

---
 include/net/gro.h      | 33 ++++++++++++++++++---------------
 net/ethernet/eth.c     |  9 +++------
 net/ipv4/af_inet.c     |  9 +++------
 net/ipv4/gre_offload.c |  9 +++------
 net/ipv4/tcp_offload.c |  9 +++------
 5 files changed, 30 insertions(+), 39 deletions(-)

include/net/gro.h +18 ~~-15~~

index 867656b0739c..c37c5d6f8c02 100644
@@ -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_try_fast(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_try_fast(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_try_fast(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_try_fast(skb, hlen, off);
 
 	return uh;
 }

net/ethernet/eth.c +3 -6

index 62b89d6f54fd..3e74a3ce4984 100644
@@ -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_try_fast(skb, hlen, off_eth);
+	if (unlikely(!eh))
+		goto out;
 
 	flush = 0;

net/ipv4/af_inet.c +3 -6

index 93da9f783bec..4591c1e1a8e1 100644
@@ -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_try_fast(skb, hlen, off);
+	if (unlikely(!iph))
+		goto out;
 
 	proto = iph->protocol;

net/ipv4/gre_offload.c +3 -6

index 07073fa35205..a97ed708b47a 100644
@@ -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_try_fast(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

net/ipv4/tcp_offload.c +3 -6

index 30abde86db45..a1229253d558 100644
@@ -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_try_fast(skb, hlen, off);
+	if (unlikely(!th))
+		goto out;
 
 	thlen = th->doff * 4;
 	if (thlen < sizeof(*th))

Signature · 1 line

2.36.1

Eric Dumazet

2022-07-28 16:01 UTC #1 lore raw

Quoted 12 lines · Richard Gobert

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 the GRO code.
This patch replaces these patterns with a single inlined function call, improving code readability.

Quoted 7 lines

Signed-off-by Richard Gobert <[email protected]>

---
 include/net/gro.h      | 33 ++++++++++++++++++---------------
 net/ethernet/eth.c     |  9 +++------
 net/ipv4/af_inet.c     |  9 +++------
 net/ipv4/gre_offload.c |  9 +++------
 net/ipv4/tcp_offload.c |  9 +++------

It seems there are other places this helper could be used ?

drivers/net/geneve.c
drivers/net/vxlan/vxlan_core.c
net/8021q/vlan_core.c
net/ipv4/fou.c

David Ahern

2022-07-28 16:07 UTC #2 lore raw

In reply to Richard Gobert

Quoted 5 lines · Richard Gobert

include/net/gro.h +1 -0

@@ -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_try_fast(struct sk_buff *skb,

include/net/gro.h static inline void *skb_gro_header_slow(struct sk_buff *skb, unsigned int hlen, · line 173

Quoted 10 lines

include/net/gro.h +10 -0

+					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;
+}
+

skb_get_gro_header() or just skb_gro_header() since it does fast then slow variants?