[PATCH net-next 0/3] net: remove some skb_mac_header assumptions

[PATCH net-next 0/3] net: remove some skb_mac_header assumptions

[Eric Dumazet]

Historically, we tried o maintain skb_mac_header available in most of
networking paths.

When reaching ndo_start_xmit() handlers, skb_mac_header() should always
be skb->data.

With recent additions of skb_mac_header_was_set() and 
DEBUG_NET_WARN_ON_ONCE() in skb_mac_header(), we can attempt
to remove our reliance on skb_mac_header in TX paths.

When this effort completes we will remove skb_reset_mac_header()
from __dev_queue_xmit() and replace it by
skb_unset_mac_header() on DEBUG_NET builds.

Eric Dumazet (3):
  net: do not use skb_mac_header() in qdisc_pkt_len_init()
  sch_cake: do not use skb_mac_header() in cake_overhead()
  net/sched: remove two skb_mac_header() uses

 net/core/dev.c         | 8 ++++----
 net/sched/act_mirred.c | 2 +-
 net/sched/act_mpls.c   | 2 +-
 net/sched/sch_cake.c   | 6 +++---
 4 files changed, 9 insertions(+), 9 deletions(-)

-- 
2.40.0.rc2.332.ga46443480c-goog

[PATCH net-next 1/3] net: do not use skb_mac_header() in qdisc_pkt_len_init()

[Eric Dumazet]

We want to remove our use of skb_mac_header() in tx paths,
eg remove skb_reset_mac_header() from __dev_queue_xmit().

Idea is that ndo_start_xmit() can get the mac header
simply looking at skb->data.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/core/dev.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index c7853192563d2ee6cd43293c84b9ae5073346580..c610efc5fcd60247799c6ee7fd1b3fb429cabfb5 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3733,25 +3733,25 @@ static void qdisc_pkt_len_init(struct sk_buff *skb)
 	 * we add to pkt_len the headers size of all segments
 	 */
 	if (shinfo->gso_size && skb_transport_header_was_set(skb)) {
-		unsigned int hdr_len;
 		u16 gso_segs = shinfo->gso_segs;
+		unsigned int hdr_len;
 
 		/* mac layer + network layer */
-		hdr_len = skb_transport_header(skb) - skb_mac_header(skb);
+		hdr_len = skb_transport_offset(skb);
 
 		/* + transport layer */
 		if (likely(shinfo->gso_type & (SKB_GSO_TCPV4 | SKB_GSO_TCPV6))) {
 			const struct tcphdr *th;
 			struct tcphdr _tcphdr;
 
-			th = skb_header_pointer(skb, skb_transport_offset(skb),
+			th = skb_header_pointer(skb, hdr_len,
 						sizeof(_tcphdr), &_tcphdr);
 			if (likely(th))
 				hdr_len += __tcp_hdrlen(th);
 		} else {
 			struct udphdr _udphdr;
 
-			if (skb_header_pointer(skb, skb_transport_offset(skb),
+			if (skb_header_pointer(skb, hdr_len,
 					       sizeof(_udphdr), &_udphdr))
 				hdr_len += sizeof(struct udphdr);
 		}
-- 
2.40.0.rc2.332.ga46443480c-goog

[PATCH net-next 2/3] sch_cake: do not use skb_mac_header() in cake_overhead()

[Eric Dumazet]

We want to remove our use of skb_mac_header() in tx paths,
eg remove skb_reset_mac_header() from __dev_queue_xmit().

Idea is that ndo_start_xmit() can get the mac header
simply looking at skb->data.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/sched/sch_cake.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/sched/sch_cake.c b/net/sched/sch_cake.c
index 7970217b565a4ebbee3797ec0e5fb5667c5be795..891e007d5c0bf972a686b892c13d647e46b3274b 100644
--- a/net/sched/sch_cake.c
+++ b/net/sched/sch_cake.c
@@ -1360,7 +1360,7 @@ static u32 cake_overhead(struct cake_sched_data *q, const struct sk_buff *skb)
 		return cake_calc_overhead(q, len, off);
 
 	/* borrowed from qdisc_pkt_len_init() */
-	hdr_len = skb_transport_header(skb) - skb_mac_header(skb);
+	hdr_len = skb_transport_offset(skb);
 
 	/* + transport layer */
 	if (likely(shinfo->gso_type & (SKB_GSO_TCPV4 |
@@ -1368,14 +1368,14 @@ static u32 cake_overhead(struct cake_sched_data *q, const struct sk_buff *skb)
 		const struct tcphdr *th;
 		struct tcphdr _tcphdr;
 
-		th = skb_header_pointer(skb, skb_transport_offset(skb),
+		th = skb_header_pointer(skb, hdr_len,
 					sizeof(_tcphdr), &_tcphdr);
 		if (likely(th))
 			hdr_len += __tcp_hdrlen(th);
 	} else {
 		struct udphdr _udphdr;
 
-		if (skb_header_pointer(skb, skb_transport_offset(skb),
+		if (skb_header_pointer(skb, hdr_len,
 				       sizeof(_udphdr), &_udphdr))
 			hdr_len += sizeof(struct udphdr);
 	}
-- 
2.40.0.rc2.332.ga46443480c-goog

[PATCH net-next 3/3] net/sched: remove two skb_mac_header() uses

[Eric Dumazet]

tcf_mirred_act() and tcf_mpls_act() can use skb_network_offset()
instead of relying on skb_mac_header().

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/sched/act_mirred.c | 2 +-
 net/sched/act_mpls.c   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 8037ec9b1d311486d803159b3f2f77eedf6b31d3..ec43764e92e7ba66586310628d78a6b184957fd2 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -295,7 +295,7 @@ TC_INDIRECT_SCOPE int tcf_mirred_act(struct sk_buff *skb,
 	at_nh = skb->data == skb_network_header(skb);
 	if (at_nh != expects_nh) {
 		mac_len = skb_at_tc_ingress(skb) ? skb->mac_len :
-			  skb_network_header(skb) - skb_mac_header(skb);
+			  skb_network_offset(skb);
 		if (expects_nh) {
 			/* target device/action expect data at nh */
 			skb_pull_rcsum(skb2, mac_len);
diff --git a/net/sched/act_mpls.c b/net/sched/act_mpls.c
index 809f7928a1be622671e47b8bdf384a8a10349926..1010dc632874ec5f4f0517f0512ab3ed30f14e24 100644
--- a/net/sched/act_mpls.c
+++ b/net/sched/act_mpls.c
@@ -69,7 +69,7 @@ TC_INDIRECT_SCOPE int tcf_mpls_act(struct sk_buff *skb,
 		skb_push_rcsum(skb, skb->mac_len);
 		mac_len = skb->mac_len;
 	} else {
-		mac_len = skb_network_header(skb) - skb_mac_header(skb);
+		mac_len = skb_network_offset(skb);
 	}
 
 	ret = READ_ONCE(m->tcf_action);
-- 
2.40.0.rc2.332.ga46443480c-goog

Re: [PATCH net-next 0/3] net: remove some skb_mac_header assumptions

[Vladimir Oltean]

Hi Eric,

On Tue, Mar 21, 2023 at 04:45:16PM +0000, Eric Dumazet wrote:
> Historically, we tried o maintain skb_mac_header available in most of
> networking paths.
> 
> When reaching ndo_start_xmit() handlers, skb_mac_header() should always
> be skb->data.
> 
> With recent additions of skb_mac_header_was_set() and 
> DEBUG_NET_WARN_ON_ONCE() in skb_mac_header(), we can attempt
> to remove our reliance on skb_mac_header in TX paths.
> 
> When this effort completes we will remove skb_reset_mac_header()
> from __dev_queue_xmit() and replace it by
> skb_unset_mac_header() on DEBUG_NET builds.
> 
> Eric Dumazet (3):
>   net: do not use skb_mac_header() in qdisc_pkt_len_init()
>   sch_cake: do not use skb_mac_header() in cake_overhead()
>   net/sched: remove two skb_mac_header() uses
> 
>  net/core/dev.c         | 8 ++++----
>  net/sched/act_mirred.c | 2 +-
>  net/sched/act_mpls.c   | 2 +-
>  net/sched/sch_cake.c   | 6 +++---
>  4 files changed, 9 insertions(+), 9 deletions(-)
> 
> -- 
> 2.40.0.rc2.332.ga46443480c-goog
> 

skb_mac_header() shows quite a few hits in net/dsa/ as well, in functions
that contain "xmit" in the name. Should those be changed as well?

Re: [PATCH net-next 0/3] net: remove some skb_mac_header assumptions

[Eric Dumazet]

On Tue, Mar 21, 2023 at 10:13 AM Vladimir Oltean <olteanv@gmail.com> wrote:
>
> Hi Eric,
>

> skb_mac_header() shows quite a few hits in net/dsa/ as well, in functions
> that contain "xmit" in the name. Should those be changed as well?

Hi Vladimir.

Yes, any help that you can provide (especially if you can test
patches) will be much welcomed.

Thanks !

Re: [PATCH net-next 1/3] net: do not use skb_mac_header() in qdisc_pkt_len_init()

[Simon Horman]

On Tue, Mar 21, 2023 at 04:45:17PM +0000, Eric Dumazet wrote:
> We want to remove our use of skb_mac_header() in tx paths,
> eg remove skb_reset_mac_header() from __dev_queue_xmit().
> 
> Idea is that ndo_start_xmit() can get the mac header
> simply looking at skb->data.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Reviewed-by: Simon Horman <simon.horman@corigine.com>

Re: [PATCH net-next 2/3] sch_cake: do not use skb_mac_header() in cake_overhead()

[Simon Horman]

On Tue, Mar 21, 2023 at 04:45:18PM +0000, Eric Dumazet wrote:
> We want to remove our use of skb_mac_header() in tx paths,
> eg remove skb_reset_mac_header() from __dev_queue_xmit().
> 
> Idea is that ndo_start_xmit() can get the mac header
> simply looking at skb->data.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Reviewed-by: Simon Horman <simon.horman@corigine.com>

Re: [PATCH net-next 3/3] net/sched: remove two skb_mac_header() uses

[Simon Horman]

On Tue, Mar 21, 2023 at 04:45:19PM +0000, Eric Dumazet wrote:
> tcf_mirred_act() and tcf_mpls_act() can use skb_network_offset()
> instead of relying on skb_mac_header().
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Reviewed-by: Simon Horman <simon.horman@corigine.com>

Re: [PATCH net-next 0/3] net: remove some skb_mac_header assumptions

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Tue, 21 Mar 2023 16:45:16 +0000 you wrote:
> Historically, we tried o maintain skb_mac_header available in most of
> networking paths.
> 
> When reaching ndo_start_xmit() handlers, skb_mac_header() should always
> be skb->data.
> 
> With recent additions of skb_mac_header_was_set() and
> DEBUG_NET_WARN_ON_ONCE() in skb_mac_header(), we can attempt
> to remove our reliance on skb_mac_header in TX paths.
> 
> [...]

Here is the summary with links:
  - [net-next,1/3] net: do not use skb_mac_header() in qdisc_pkt_len_init()
    https://git.kernel.org/netdev/net-next/c/f5fca219ad45
  - [net-next,2/3] sch_cake: do not use skb_mac_header() in cake_overhead()
    https://git.kernel.org/netdev/net-next/c/e495a9673caf
  - [net-next,3/3] net/sched: remove two skb_mac_header() uses
    https://git.kernel.org/netdev/net-next/c/b3be94885af4

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