[PATCH net 0/2] net/sched: sch_teql: fix shared headroom and header strip

[PATCH net 0/2] net/sched: sch_teql: fix shared headroom and header strip

[Jamal Hadi Salim]

Two pre-existing defects on the teql transmit path, both around
teql_master_xmit() -> teql_resolve().  The first makes the slave's
headroom write corrupt a shared skb and strips a userspace-supplied link
header on retry; the second leaves skb->dev pointing at the teql master
while the skb is parked on a slave's neighbour arp_queue.

Patch 1 fixes the shared-headroom write and the header-strip on retry,
tracking the header length the slave actually added.
Patch 2 moves skb->dev = slave ahead of teql_resolve() and restores
skb->dev at the arp-queue reinjection point, so a parked skb names the
device whose neighbour took it and is delivered with a link header.

Both are follow-ups to commit dc4b95b8fee9 ("net/sched: sch_teql: restore
skb->dev on the slave failure path").

Jamal Hadi Salim (2):
  net/sched: sch_teql: fix shared headroom and header strip on slave
    retry
  net/sched: sch_teql: keep skb->dev consistent on the arp-queue path

 net/core/neighbour.c |  1 +
 net/sched/sch_teql.c | 34 ++++++++++++++++++++++++++++------
 2 files changed, 29 insertions(+), 6 deletions(-)

-- 
2.43.0

[PATCH net 0/2] net/sched: sch_teql: fix shared headroom and header strip

[Jamal Hadi Salim]

Two pre-existing defects on the teql transmit path, both around
teql_master_xmit() -> teql_resolve().  The first makes the slave's
headroom write corrupt a shared skb and strips a userspace-supplied link
header on retry; the second leaves skb->dev pointing at the teql master
while the skb is parked on a slave's neighbour arp_queue.

Patch 1 fixes the shared-headroom write and the header-strip on retry,
tracking the header length the slave actually added.
Patch 2 moves skb->dev = slave ahead of teql_resolve() and restores
skb->dev at the arp-queue reinjection point, so a parked skb names the
device whose neighbour took it and is delivered with a link header.

Both are follow-ups to commit dc4b95b8fee9 ("net/sched: sch_teql: restore
skb->dev on the slave failure path").

Jamal Hadi Salim (2):
  net/sched: sch_teql: fix shared headroom and header strip on slave
    retry
  net/sched: sch_teql: keep skb->dev consistent on the arp-queue path

 net/core/neighbour.c |  1 +
 net/sched/sch_teql.c | 34 ++++++++++++++++++++++++++++------
 2 files changed, 29 insertions(+), 6 deletions(-)

-- 
2.43.0

[PATCH net 1/2] net/sched: sch_teql: fix shared headroom and header strip on slave retry

[Jamal Hadi Salim]

teql_master_xmit() writes the link header of the current slave directly
into the skb via teql_resolve(), and on retry undoes it with
__skb_pull(skb, skb_network_offset(skb)). Two problems exist on retry:

1. When the skb is shared (for example a packet tap installed on the
   master makes xmit_one() clone it before teql_master_xmit()),
   dev_hard_header() writes into memory shared with the other clones and
   corrupts them.  The headroom must be made private before the write.

2. The pull blindly removes skb_network_offset() bytes.  For a frame
   that carries its own link header in the payload (AF_PACKET/SOCK_RAW
   with no dst entry), teql_resolve() returns 0 without adding a header,
   so the retry strips the userspace-supplied header and pushes a frame
   whose first bytes are payload.

Fix this by calling skb_cow_head() before dev_hard_header(), and by
pulling back only the header length that this slave actually added,
tracked per iteration.  An AF_PACKET/SOCK_RAW frame is then retried on
the next slave with its header intact, and a shared skb is unshared
before it is modified.

This is a follow-up to commit dc4b95b8fee9 ("net/sched: sch_teql: restore
skb->dev on the slave failure path"), which restored skb->dev to the
master on the slave failure path but left these pre-existing defects on
the same teql_resolve()/retry path.

Conditions to recreate the bug: a teql master with at least two slaves,
the first of which does not consume the skb, then send an
AF_PACKET/SOCK_RAW frame (no dst entry) through the master: the frame
arriving on the second slave's peer has its MAC header stripped.  The
shared-headroom write needs a packet tap on the master so the skb is
cloned before teql_master_xmit().

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: Sashiko (gemini) <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260824115928.4099988-1-victor@mojatatu.com
Link: https://lore.kernel.org/netdev/20260824115928.4099988-1-victor@mojatatu.com/
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 net/sched/sch_teql.c | 32 +++++++++++++++++++++++++++-----
 1 file changed, 27 insertions(+), 5 deletions(-)

diff --git a/net/sched/sch_teql.c b/net/sched/sch_teql.c
index 409ce50cc0db..acd03f9afc6b 100644
--- a/net/sched/sch_teql.c
+++ b/net/sched/sch_teql.c
@@ -245,7 +245,7 @@ static int teql_qdisc_init(struct Qdisc *sch, struct nlattr *opt,
 static int
 __teql_resolve(struct sk_buff *skb, struct sk_buff *skb_res,
 	       struct net_device *dev, struct netdev_queue *txq,
-	       struct dst_entry *dst)
+	       struct dst_entry *dst, int *hlen)
 {
 	struct neighbour *n;
 	int err = 0;
@@ -265,15 +265,28 @@ __teql_resolve(struct sk_buff *skb, struct sk_buff *skb_res,
 	}
 
 	if (neigh_event_send(n, skb_res) == 0) {
+		int off = skb_network_offset(skb);
 		char haddr[MAX_ADDR_LEN];
 
 		neigh_ha_snapshot(haddr, n, dev);
+		/* The skb may be shared (e.g. a packet tap clone); make the
+		 * headroom private before dev_hard_header() writes into it.
+		 */
+		if (skb_cow_head(skb, LL_RESERVED_SPACE(dev)) < 0) {
+			err = -ENOMEM;
+			goto out;
+		}
 		if (dev_hard_header(skb, dev, ntohs(skb_protocol(skb, false)),
 				    haddr, NULL, skb->len) < 0)
 			err = -EINVAL;
+		/* The header, if any, is prepended above skb->data, so the
+		 * network offset grew by exactly the bytes to undo later.
+		 */
+		*hlen = skb_network_offset(skb) - off;
 	} else {
 		err = (skb_res == NULL) ? -EAGAIN : 1;
 	}
+out:
 	neigh_release(n);
 	return err;
 }
@@ -281,11 +294,13 @@ __teql_resolve(struct sk_buff *skb, struct sk_buff *skb_res,
 static inline int teql_resolve(struct sk_buff *skb,
 			       struct sk_buff *skb_res,
 			       struct net_device *dev,
-			       struct netdev_queue *txq)
+			       struct netdev_queue *txq,
+			       int *hlen)
 {
 	struct dst_entry *dst = skb_dst(skb);
 	int res;
 
+	*hlen = 0;
 	if (rcu_access_pointer(txq->qdisc) == &noop_qdisc)
 		return -ENODEV;
 
@@ -293,7 +308,7 @@ static inline int teql_resolve(struct sk_buff *skb,
 		return 0;
 
 	rcu_read_lock();
-	res = __teql_resolve(skb, skb_res, dev, txq, dst);
+	res = __teql_resolve(skb, skb_res, dev, txq, dst, hlen);
 	rcu_read_unlock();
 
 	return res;
@@ -305,6 +320,7 @@ static netdev_tx_t teql_master_xmit(struct sk_buff *skb, struct net_device *dev)
 	struct Qdisc *start, *q;
 	int busy;
 	int nores;
+	int hlen;
 	int subq = skb_get_queue_mapping(skb);
 	struct sk_buff *skb_res = NULL;
 
@@ -332,7 +348,7 @@ static netdev_tx_t teql_master_xmit(struct sk_buff *skb, struct net_device *dev)
 			continue;
 		}
 
-		switch (teql_resolve(skb, skb_res, slave, slave_txq)) {
+		switch (teql_resolve(skb, skb_res, slave, slave_txq, &hlen)) {
 		case 0:
 			if (__netif_tx_trylock(slave_txq)) {
 				unsigned int length = qdisc_pkt_len(skb);
@@ -374,8 +390,14 @@ static netdev_tx_t teql_master_xmit(struct sk_buff *skb, struct net_device *dev)
 			nores = 1;
 			break;
 		}
+		/* Undo only the header teql_resolve() pushed for this slave.
+		 * Pulling skb_network_offset() instead would strip a
+		 * userspace-supplied header when the slave added none, e.g.
+		 * an AF_PACKET/SOCK_RAW frame with no dst entry.
+		 */
+		if (hlen > 0)
+			__skb_pull(skb, hlen);
 		skb->dev = dev;
-		__skb_pull(skb, skb_network_offset(skb));
 	} while ((q = rcu_dereference(NEXT_SLAVE(q))) != start);
 
 	if (nores && skb_res == NULL) {
-- 
2.43.0

[PATCH net 2/2] net/sched: sch_teql: keep skb->dev consistent on the arp-queue path

[Jamal Hadi Salim]

This is a follow-up to commit dc4b95b8fee9 ("net/sched: sch_teql:
restore skb->dev on the slave failure path"), which restored skb->dev
to the master after each slave iteration but left the master-vs-slave
mismatch on the neighbour/ARP-queue path, and left the queued skb
naming the master rather than the device whose neighbour took it.

teql_master_xmit() sets skb->dev = slave only right before calling the
slave's ndo_start_xmit(). When the slave has no resolved neighbour,
teql_resolve() hands the skb to neigh_event_send(), which queues it on
the slave's neighbour arp_queue while skb->dev still points at the
teql* master. skb->dev takes no netdev reference, and the master is
freed unconditionally on teql_exit() while the slave is still alive, so
a queued skb that runs the ARP retransmit path can dereference a freed
device.

Move the assignment to before teql_resolve(), so a skb parked on the
slave's arp_queue carries the slave's device and is flushed with it.

There is a second issue: when the neighbour resolves,
neigh_update_process_arp_queue() re-looks-up the top-level neighbour so
shaper, eql and teql can be re-entered, but it calls n1->output()
without restoring skb->dev. An skb that was parked with skb->dev set to
the slave then leaves through the slave directly, skipping
teql_master_xmit() and the dev_hard_header() call that builds the
slave's link-layer header, and is transmitted headerless. Restore
skb->dev = n1->dev at the reinjection point; this is the identity for
every other parker (neigh_resolve_output() parks with skb->dev ==
neigh->dev, ndisc probes carry no skb) and the needed repair for teql.

Conditions to recreate the bug: a teql master with a slave whose
neighbour is unresolved; send one packet through the master so the skb
parks on the slave's arp_queue. On the unfixed tree the parked skb
names the master (kprobe on __neigh_event_send shows skb->dev=teql0);
with only the first hunk applied, the parked packet is delivered to the
slave's peer without a link-layer header when ARP completes.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: Sashiko (nipa) <sashiko-bot@kernel.org>
Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260824115928.4099988-1-victor@mojatatu.com
Link: https://lore.kernel.org/netdev/20260824115928.4099988-1-victor@mojatatu.com/
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 net/core/neighbour.c | 1 +
 net/sched/sch_teql.c | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 7448320f7ad5..5823f5007812 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -1336,6 +1336,7 @@ static void neigh_update_process_arp_queue(struct neighbour *neigh)
 			if (n2)
 				n1 = n2;
 		}
+		skb->dev = n1->dev;
 		READ_ONCE(n1->output)(n1, skb);
 		if (n2)
 			neigh_release(n2);
diff --git a/net/sched/sch_teql.c b/net/sched/sch_teql.c
index acd03f9afc6b..3fbd5af10028 100644
--- a/net/sched/sch_teql.c
+++ b/net/sched/sch_teql.c
@@ -348,12 +348,12 @@ static netdev_tx_t teql_master_xmit(struct sk_buff *skb, struct net_device *dev)
 			continue;
 		}
 
+		skb->dev = slave;
 		switch (teql_resolve(skb, skb_res, slave, slave_txq, &hlen)) {
 		case 0:
 			if (__netif_tx_trylock(slave_txq)) {
 				unsigned int length = qdisc_pkt_len(skb);
 
-				skb->dev = slave;
 				if (!netif_xmit_frozen_or_stopped(slave_txq) &&
 				    netdev_start_xmit(skb, slave, slave_txq, false) ==
 				    NETDEV_TX_OK) {
-- 
2.43.0

Re: [PATCH net 1/2] net/sched: sch_teql: fix shared headroom and header strip on slave retry

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 (1) · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The commit message and the new code comment give the wrong
  reason for problem 1 and the wrong way to reproduce it.

Pre-existing issues:
- [Critical] teql_master_xmit() in net/sched/sch_teql.c treats a slave
  transmit as successful only when `netdev_start_xmit(skb, slave,…

--

net/sched: sch_teql: fix shared headroom and header strip on slave retry

The patch adds a skb_cow_head() call before dev_hard_header() in
__teql_resolve(). It also changes teql_master_xmit() so that, when a
slave attempt fails, it pulls back only the header length that
teql_resolve() pushed for that slave. It no longer pulls
skb_network_offset() bytes.

Link: https://lore.kernel.org/netdev/20260824115928.4099988-1-victor@mojatatu.com/
Closes: https://sashiko.dev/#/patchset/20260824115928.4099988-1-victor@mojatatu.com

> diff --git a/net/sched/sch_teql.c b/net/sched/sch_teql.c
> index 409ce50cc0dbc..acd03f9afc6b8 100644
> --- a/net/sched/sch_teql.c
> +++ b/net/sched/sch_teql.c

[ ... ]

> @@ -265,15 +265,28 @@ __teql_resolve(struct sk_buff *skb, struct sk_buff *skb_res,
>  	}
>  
>  	if (neigh_event_send(n, skb_res) == 0) {
> +		int off = skb_network_offset(skb);
>  		char haddr[MAX_ADDR_LEN];
>  
>  		neigh_ha_snapshot(haddr, n, dev);
> +		/* The skb may be shared (e.g. a packet tap clone); make the
> +		 * headroom private before dev_hard_header() writes into it.
> +		 */
> +		if (skb_cow_head(skb, LL_RESERVED_SPACE(dev)) < 0) {

[Severity: Medium]

The skb_cow_head() call looks right. Are this comment and the commit
message accurate about why it is needed?

The commit message says:

    1. When the skb is shared (for example a packet tap installed on the
       master makes xmit_one() clone it before teql_master_xmit()),

and later:

    and a shared skb is unshared before it is modified.

skb_cow_head() only handles skb_header_cloned(). It does nothing when
skb_shared() is true (users > 1). If it has to expand the head,
pskb_expand_head() has:

    BUG_ON(skb_shared(skb));

Would "header-cloned" be a more accurate term than "shared" in both
places?

Can the packet tap reproducer actually show corruption?
teql_master_setup() never sets header_ops and uses ARPHRD_VOID, so
dev_has_header() is false for the master. packet_rcv() and tpacket_rcv()
only push back to the mac header when it is true:

    if (dev_has_header(dev)) {
        if (sk->sk_type != SOCK_DGRAM)
            skb_push(skb, skb->data - skb_mac_header(skb));

So the tap only sees bytes from skb->data onward. __teql_resolve()
writes the slave header into [data - hlen, data), which the tap never
reads.

A trigger that does seem to cause visible corruption is a tc mirred
mirror on a physical device's clsact egress, with the teql master as
the target.

sch_handle_egress() runs before skb_dst_drop(), so the clone keeps its
dst. ARPHRD_VOID is not a mac-header-xmit type, so tcf_mirred_to_dev()
pulls mac_len off the clone:

    expects_nh = want_ingress || !m_mac_header_xmit;
    at_nh = skb->data == skb_network_header(skb);
    if (at_nh != expects_nh) {
        ...
            skb_pull_rcsum(skb_to_send, mac_len);

The original skb's Ethernet header then sits in the clone's shared
headroom, and dev_hard_header() in teql overwrites it.

Should the reproduction steps in the commit message describe this case
instead?

The subject and body also say both problems happen "on slave retry".
The header write, and now the COW, happens on every slave attempt,
including a first attempt that succeeds. Could the wording reflect
that?

[ ... ]

> @@ -374,8 +390,14 @@ static netdev_tx_t teql_master_xmit(struct sk_buff *skb, struct net_device *dev)
>  			nores = 1;
>  			break;
>  		}
> +		/* Undo only the header teql_resolve() pushed for this slave.
> +		 * Pulling skb_network_offset() instead would strip a
> +		 * userspace-supplied header when the slave added none, e.g.
> +		 * an AF_PACKET/SOCK_RAW frame with no dst entry.
> +		 */
> +		if (hlen > 0)
> +			__skb_pull(skb, hlen);
>  		skb->dev = dev;

[Severity: Critical]

This isn't a bug introduced by this patch, but can this __skb_pull()
and the skb->dev write touch an skb that the slave has already
consumed?

teql_master_xmit() counts a slave transmit as successful only when it
returns NETDEV_TX_OK:

    if (!netif_xmit_frozen_or_stopped(slave_txq) &&
        netdev_start_xmit(skb, slave, slave_txq, false) ==
        NETDEV_TX_OK) {

Stacked slaves such as 8021q, and macvlan through macvlan_start_xmit(),
return the lower device's dev_queue_xmit() result unchanged:

net/8021q/vlan_dev.c:vlan_dev_hard_start_xmit() {
    ...
    ret = dev_queue_xmit(skb);
    ...
    return ret;
}

That result can be NET_XMIT_DROP, NET_XMIT_CN or a negative errno.
dev_xmit_complete() treats all of these as consumed:

    if (likely(rc < NET_XMIT_MASK))
        return true;

For NET_XMIT_DROP the skb is already freed, for example in
__dev_xmit_skb():

    kfree_skb_reason(skb, SKB_DROP_REASON_QDISC_BURST_DROP);
    return NET_XMIT_DROP;

For NET_XMIT_CN it may still be queued on the lower qdisc.

In both cases teql falls through to this block. For a VLAN slave with a
resolved neighbour, hlen is ETH_HLEN + VLAN_HLEN, so __skb_pull() and
skb->dev = dev write to the consumed skb.

The loop then calls teql_resolve(), skb_cow_head(), dev_hard_header()
and netdev_start_xmit() again on it, or it reaches the drop label:

    master->tx_dropped++;
    rcu_read_unlock();
    dev_kfree_skb(skb);

Would that be a double free? The function can also return
NETDEV_TX_BUSY, and the core would then requeue a freed skb.

The setup needs CAP_NET_ADMIN to attach teql as root on a VLAN or
macvlan device. After that, ordinary congestion on the lower device is
enough to hit this path. The rest of the series does not seem to fix it
either; the final state still compares against NETDEV_TX_OK.

Since this patch already reworks the post-failure skb handling on this
path, would it make sense to check dev_xmit_complete() on the return
value instead of comparing it with NETDEV_TX_OK?

>  	} while ((q = rcu_dereference(NEXT_SLAVE(q))) != start);
>  
>  	if (nores && skb_res == NULL) {

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/QDISC-6HS9.v1.20260928064754%40mojatatu.com

Re: [PATCH net 2/2] net/sched: sch_teql: keep skb->dev consistent on the arp-queue path

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The new line `skb->dev = n1->dev;` in
  neigh_update_process_arp_queue() (net/core/neighbour.c:1339) runs for…

--

net/sched: sch_teql: keep skb->dev consistent on the arp-queue path

This moves the skb->dev = slave assignment in teql_master_xmit() to before
teql_resolve(), so an skb parked on the slave's arp_queue names the slave.
It also makes neigh_update_process_arp_queue() set skb->dev = n1->dev
before reinjecting each queued skb through n1->output().

Link: https://lore.kernel.org/netdev/20260824115928.4099988-1-victor@mojatatu.com/

> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index 7448320f7ad52..5823f5007812b 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -1336,6 +1336,7 @@ static void neigh_update_process_arp_queue(struct neighbour *neigh)
>  			if (n2)
>  				n1 = n2;
>  		}
> +		skb->dev = n1->dev;
>  		READ_ONCE(n1->output)(n1, skb);

[Severity: Medium]
Is the "identity for every other parker" claim in the commit message
accurate?

The commit message says:

  Restore skb->dev = n1->dev at the reinjection point; this is the
  identity for every other parker (neigh_resolve_output() parks with
  skb->dev == neigh->dev, ndisc probes carry no skb)

When a dst is attached, n1 is replaced by n2 = dst_neigh_lookup_skb(dst,
skb). For IPv4 that is ipv4_neigh_lookup(), which picks the neighbour on
the dst's device and not on the device the skb was parked on:

net/ipv4/route.c:ipv4_neigh_lookup() {
    ...
	dev = dst_dev_rcu(dst);
	if (likely(rt->rt_gw_family == AF_INET)) {
		n = ip_neigh_gw4(dev, rt->rt_gw4);
    ...
}

So this new line runs for every neighbour table, and it can change the
egress device for parkers other than teql.

One case is bpf_redirect_neigh() with BPF_F_NEXTHOP from tc egress. The
tc egress hook runs in __dev_queue_xmit() before the IFF_XMIT_DST_RELEASE
drop. __bpf_redirect_neigh_v4() replaces the dst only in the !nh branch:

net/core/filter.c:__bpf_redirect_neigh_v4() {
    ...
		skb_dst_drop(skb);
		skb_dst_set(skb, &rt->dst);
	}
    ...
}

With an explicit nexthop, the original route dst (say on eth0) stays
attached. bpf_out_neigh_v4() then sets skb->dev to the BPF target (say
eth1) and calls neigh_output() on ip_neigh_gw4(eth1, nh). If that
neighbour is unresolved, the skb is parked on eth1's neighbour.

When it resolves, n2 is the neighbour on eth0, and this line sets
skb->dev = eth0. Do the queued packets now leave through the route device
instead of the device the BPF program chose?

Before this patch they went out eth1 with eth0's L2 header, which was
also wrong, but the behaviour was different.

A second case is bridged DNAT in br_netfilter with pass_vlan_indev set.
br_nf_pre_routing_finish() only calls br_nf_pre_routing_finish_bridge()
when skb_dst(skb)->dev is the VLAN upper device (br0.VID), and that
function does:

net/bridge/br_netfilter_hooks.c:br_nf_pre_routing_finish_bridge() {
    ...
	skb->dev = bridge_parent(skb->dev);
    ...
	neigh = dst_neigh_lookup_skb(dst, skb);
    ...
			ret = READ_ONCE(neigh->output)(neigh, skb);
    ...
}

Here skb->dev is br0 while the neighbour is on br0.VID, so skb->dev ==
neigh->dev does not hold. Before this patch, parked skbs left through br0,
the same as skbs that were not parked.

With this change, won't parked skbs go through dev_queue_xmit(br0.VID)
and vlan_dev_hard_start_xmit()? That path can rewrite the hwaccel VLAN
TCI/PCP from the egress QoS map and bumps the VLAN device stats. Parked
and non-parked packets would then take different paths.

Could the fix be kept inside teql instead? Another option is to limit it
to the teql master/slave case, where n2 differs from the parking
neighbour, so other neighbour users keep their current skb->dev.

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/QDISC-6HS9.v1.20260928064754%40mojatatu.com

Re: [PATCH net 1/2] net/sched: sch_teql: fix shared headroom and header strip on slave retry

[Jamal Hadi Salim]

On Wed, Sep 30, 2026 at 9:07 AM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
>
> Critical: 0 (1) · High: 0 · Medium: 1 · Low: 0
>
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
>
> - [Medium] The commit message and the new code comment give the wrong
>   reason for problem 1 and the wrong way to reproduce it.
>
> Pre-existing issues:
> - [Critical] teql_master_xmit() in net/sched/sch_teql.c treats a slave
>   transmit as successful only when `netdev_start_xmit(skb, slave,…
>

We have captured this as a follouwup.

> --
>
> net/sched: sch_teql: fix shared headroom and header strip on slave retry
>
> The patch adds a skb_cow_head() call before dev_hard_header() in
> __teql_resolve(). It also changes teql_master_xmit() so that, when a
> slave attempt fails, it pulls back only the header length that
> teql_resolve() pushed for that slave. It no longer pulls
> skb_network_offset() bytes.
>

Given this is a doc only issue, I will not be resending this patch for
this specific reason. But if i find you caught technical issues in
patch2 I'll say "header-cloned", use the tc mirred mirror on a
physical device's clsact egress -> teql master as the reproducer, and
drop the "on slave retry" wording (the header write and the COW happen
on every slave attempt, including a first attempt that succeeds)

cheers,
jamal
> Link: https://lore.kernel.org/netdev/20260824115928.4099988-1-victor@mojatatu.com/
> Closes: https://sashiko.dev/#/patchset/20260824115928.4099988-1-victor@mojatatu.com
>
> > diff --git a/net/sched/sch_teql.c b/net/sched/sch_teql.c
> > index 409ce50cc0dbc..acd03f9afc6b8 100644
> > --- a/net/sched/sch_teql.c
> > +++ b/net/sched/sch_teql.c
>
> [ ... ]
>
> > @@ -265,15 +265,28 @@ __teql_resolve(struct sk_buff *skb, struct sk_buff *skb_res,
> >       }
> >
> >       if (neigh_event_send(n, skb_res) == 0) {
> > +             int off = skb_network_offset(skb);
> >               char haddr[MAX_ADDR_LEN];
> >
> >               neigh_ha_snapshot(haddr, n, dev);
> > +             /* The skb may be shared (e.g. a packet tap clone); make the
> > +              * headroom private before dev_hard_header() writes into it.
> > +              */
> > +             if (skb_cow_head(skb, LL_RESERVED_SPACE(dev)) < 0) {
>
> [Severity: Medium]
>
> The skb_cow_head() call looks right. Are this comment and the commit
> message accurate about why it is needed?
>
> The commit message says:
>
>     1. When the skb is shared (for example a packet tap installed on the
>        master makes xmit_one() clone it before teql_master_xmit()),
>
> and later:
>
>     and a shared skb is unshared before it is modified.
>
> skb_cow_head() only handles skb_header_cloned(). It does nothing when
> skb_shared() is true (users > 1). If it has to expand the head,
> pskb_expand_head() has:
>
>     BUG_ON(skb_shared(skb));
>
> Would "header-cloned" be a more accurate term than "shared" in both
> places?
>
> Can the packet tap reproducer actually show corruption?
> teql_master_setup() never sets header_ops and uses ARPHRD_VOID, so
> dev_has_header() is false for the master. packet_rcv() and tpacket_rcv()
> only push back to the mac header when it is true:
>
>     if (dev_has_header(dev)) {
>         if (sk->sk_type != SOCK_DGRAM)
>             skb_push(skb, skb->data - skb_mac_header(skb));
>
> So the tap only sees bytes from skb->data onward. __teql_resolve()
> writes the slave header into [data - hlen, data), which the tap never
> reads.
>
> A trigger that does seem to cause visible corruption is a tc mirred
> mirror on a physical device's clsact egress, with the teql master as
> the target.
>
> sch_handle_egress() runs before skb_dst_drop(), so the clone keeps its
> dst. ARPHRD_VOID is not a mac-header-xmit type, so tcf_mirred_to_dev()
> pulls mac_len off the clone:
>
>     expects_nh = want_ingress || !m_mac_header_xmit;
>     at_nh = skb->data == skb_network_header(skb);
>     if (at_nh != expects_nh) {
>         ...
>             skb_pull_rcsum(skb_to_send, mac_len);
>
> The original skb's Ethernet header then sits in the clone's shared
> headroom, and dev_hard_header() in teql overwrites it.
>
> Should the reproduction steps in the commit message describe this case
> instead?
>
> The subject and body also say both problems happen "on slave retry".
> The header write, and now the COW, happens on every slave attempt,
> including a first attempt that succeeds. Could the wording reflect
> that?
>
> [ ... ]
>
> > @@ -374,8 +390,14 @@ static netdev_tx_t teql_master_xmit(struct sk_buff *skb, struct net_device *dev)
> >                       nores = 1;
> >                       break;
> >               }
> > +             /* Undo only the header teql_resolve() pushed for this slave.
> > +              * Pulling skb_network_offset() instead would strip a
> > +              * userspace-supplied header when the slave added none, e.g.
> > +              * an AF_PACKET/SOCK_RAW frame with no dst entry.
> > +              */
> > +             if (hlen > 0)
> > +                     __skb_pull(skb, hlen);
> >               skb->dev = dev;
>
> [Severity: Critical]
>
> This isn't a bug introduced by this patch, but can this __skb_pull()
> and the skb->dev write touch an skb that the slave has already
> consumed?
>
> teql_master_xmit() counts a slave transmit as successful only when it
> returns NETDEV_TX_OK:
>
>     if (!netif_xmit_frozen_or_stopped(slave_txq) &&
>         netdev_start_xmit(skb, slave, slave_txq, false) ==
>         NETDEV_TX_OK) {
>
> Stacked slaves such as 8021q, and macvlan through macvlan_start_xmit(),
> return the lower device's dev_queue_xmit() result unchanged:
>
> net/8021q/vlan_dev.c:vlan_dev_hard_start_xmit() {
>     ...
>     ret = dev_queue_xmit(skb);
>     ...
>     return ret;
> }
>
> That result can be NET_XMIT_DROP, NET_XMIT_CN or a negative errno.
> dev_xmit_complete() treats all of these as consumed:
>
>     if (likely(rc < NET_XMIT_MASK))
>         return true;
>
> For NET_XMIT_DROP the skb is already freed, for example in
> __dev_xmit_skb():
>
>     kfree_skb_reason(skb, SKB_DROP_REASON_QDISC_BURST_DROP);
>     return NET_XMIT_DROP;
>
> For NET_XMIT_CN it may still be queued on the lower qdisc.
>
> In both cases teql falls through to this block. For a VLAN slave with a
> resolved neighbour, hlen is ETH_HLEN + VLAN_HLEN, so __skb_pull() and
> skb->dev = dev write to the consumed skb.
>
> The loop then calls teql_resolve(), skb_cow_head(), dev_hard_header()
> and netdev_start_xmit() again on it, or it reaches the drop label:
>
>     master->tx_dropped++;
>     rcu_read_unlock();
>     dev_kfree_skb(skb);
>
> Would that be a double free? The function can also return
> NETDEV_TX_BUSY, and the core would then requeue a freed skb.
>
> The setup needs CAP_NET_ADMIN to attach teql as root on a VLAN or
> macvlan device. After that, ordinary congestion on the lower device is
> enough to hit this path. The rest of the series does not seem to fix it
> either; the final state still compares against NETDEV_TX_OK.
>
> Since this patch already reworks the post-failure skb handling on this
> path, would it make sense to check dev_xmit_complete() on the return
> value instead of comparing it with NETDEV_TX_OK?
>
> >       } while ((q = rcu_dereference(NEXT_SLAVE(q))) != start);
> >
> >       if (nores && skb_res == NULL) {
>
> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/QDISC-6HS9.v1.20260928064754%40mojatatu.com

Re: [PATCH net 1/2] net/sched: sch_teql: fix shared headroom and header strip on slave retry

[Jamal Hadi Salim]

On Wed, Sep 30, 2026 at 9:07 AM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
>
> Critical: 0 (1) · High: 0 · Medium: 1 · Low: 0
>
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
>
> - [Medium] The commit message and the new code comment give the wrong
>   reason for problem 1 and the wrong way to reproduce it.
>
> Pre-existing issues:
> - [Critical] teql_master_xmit() in net/sched/sch_teql.c treats a slave
>   transmit as successful only when `netdev_start_xmit(skb, slave,…
>

Sorry, I missed responding to this:
This is  a different root cause from this patch series - we are going
to send a followup fix if no one beats us to it.

cheers,
jamal

> --
>
> net/sched: sch_teql: fix shared headroom and header strip on slave retry
>
> The patch adds a skb_cow_head() call before dev_hard_header() in
> __teql_resolve(). It also changes teql_master_xmit() so that, when a
> slave attempt fails, it pulls back only the header length that
> teql_resolve() pushed for that slave. It no longer pulls
> skb_network_offset() bytes.
>
> Link: https://lore.kernel.org/netdev/20260824115928.4099988-1-victor@mojatatu.com/
> Closes: https://sashiko.dev/#/patchset/20260824115928.4099988-1-victor@mojatatu.com
>
> > diff --git a/net/sched/sch_teql.c b/net/sched/sch_teql.c
> > index 409ce50cc0dbc..acd03f9afc6b8 100644
> > --- a/net/sched/sch_teql.c
> > +++ b/net/sched/sch_teql.c
>
> [ ... ]
>
> > @@ -265,15 +265,28 @@ __teql_resolve(struct sk_buff *skb, struct sk_buff *skb_res,
> >       }
> >
> >       if (neigh_event_send(n, skb_res) == 0) {
> > +             int off = skb_network_offset(skb);
> >               char haddr[MAX_ADDR_LEN];
> >
> >               neigh_ha_snapshot(haddr, n, dev);
> > +             /* The skb may be shared (e.g. a packet tap clone); make the
> > +              * headroom private before dev_hard_header() writes into it.
> > +              */
> > +             if (skb_cow_head(skb, LL_RESERVED_SPACE(dev)) < 0) {
>
> [Severity: Medium]
>
> The skb_cow_head() call looks right. Are this comment and the commit
> message accurate about why it is needed?
>
> The commit message says:
>
>     1. When the skb is shared (for example a packet tap installed on the
>        master makes xmit_one() clone it before teql_master_xmit()),
>
> and later:
>
>     and a shared skb is unshared before it is modified.
>
> skb_cow_head() only handles skb_header_cloned(). It does nothing when
> skb_shared() is true (users > 1). If it has to expand the head,
> pskb_expand_head() has:
>
>     BUG_ON(skb_shared(skb));
>
> Would "header-cloned" be a more accurate term than "shared" in both
> places?
>
> Can the packet tap reproducer actually show corruption?
> teql_master_setup() never sets header_ops and uses ARPHRD_VOID, so
> dev_has_header() is false for the master. packet_rcv() and tpacket_rcv()
> only push back to the mac header when it is true:
>
>     if (dev_has_header(dev)) {
>         if (sk->sk_type != SOCK_DGRAM)
>             skb_push(skb, skb->data - skb_mac_header(skb));
>
> So the tap only sees bytes from skb->data onward. __teql_resolve()
> writes the slave header into [data - hlen, data), which the tap never
> reads.
>
> A trigger that does seem to cause visible corruption is a tc mirred
> mirror on a physical device's clsact egress, with the teql master as
> the target.
>
> sch_handle_egress() runs before skb_dst_drop(), so the clone keeps its
> dst. ARPHRD_VOID is not a mac-header-xmit type, so tcf_mirred_to_dev()
> pulls mac_len off the clone:
>
>     expects_nh = want_ingress || !m_mac_header_xmit;
>     at_nh = skb->data == skb_network_header(skb);
>     if (at_nh != expects_nh) {
>         ...
>             skb_pull_rcsum(skb_to_send, mac_len);
>
> The original skb's Ethernet header then sits in the clone's shared
> headroom, and dev_hard_header() in teql overwrites it.
>
> Should the reproduction steps in the commit message describe this case
> instead?
>
> The subject and body also say both problems happen "on slave retry".
> The header write, and now the COW, happens on every slave attempt,
> including a first attempt that succeeds. Could the wording reflect
> that?
>
> [ ... ]
>
> > @@ -374,8 +390,14 @@ static netdev_tx_t teql_master_xmit(struct sk_buff *skb, struct net_device *dev)
> >                       nores = 1;
> >                       break;
> >               }
> > +             /* Undo only the header teql_resolve() pushed for this slave.
> > +              * Pulling skb_network_offset() instead would strip a
> > +              * userspace-supplied header when the slave added none, e.g.
> > +              * an AF_PACKET/SOCK_RAW frame with no dst entry.
> > +              */
> > +             if (hlen > 0)
> > +                     __skb_pull(skb, hlen);
> >               skb->dev = dev;
>
> [Severity: Critical]
>
> This isn't a bug introduced by this patch, but can this __skb_pull()
> and the skb->dev write touch an skb that the slave has already
> consumed?
>
> teql_master_xmit() counts a slave transmit as successful only when it
> returns NETDEV_TX_OK:
>
>     if (!netif_xmit_frozen_or_stopped(slave_txq) &&
>         netdev_start_xmit(skb, slave, slave_txq, false) ==
>         NETDEV_TX_OK) {
>
> Stacked slaves such as 8021q, and macvlan through macvlan_start_xmit(),
> return the lower device's dev_queue_xmit() result unchanged:
>
> net/8021q/vlan_dev.c:vlan_dev_hard_start_xmit() {
>     ...
>     ret = dev_queue_xmit(skb);
>     ...
>     return ret;
> }
>
> That result can be NET_XMIT_DROP, NET_XMIT_CN or a negative errno.
> dev_xmit_complete() treats all of these as consumed:
>
>     if (likely(rc < NET_XMIT_MASK))
>         return true;
>
> For NET_XMIT_DROP the skb is already freed, for example in
> __dev_xmit_skb():
>
>     kfree_skb_reason(skb, SKB_DROP_REASON_QDISC_BURST_DROP);
>     return NET_XMIT_DROP;
>
> For NET_XMIT_CN it may still be queued on the lower qdisc.
>
> In both cases teql falls through to this block. For a VLAN slave with a
> resolved neighbour, hlen is ETH_HLEN + VLAN_HLEN, so __skb_pull() and
> skb->dev = dev write to the consumed skb.
>
> The loop then calls teql_resolve(), skb_cow_head(), dev_hard_header()
> and netdev_start_xmit() again on it, or it reaches the drop label:
>
>     master->tx_dropped++;
>     rcu_read_unlock();
>     dev_kfree_skb(skb);
>
> Would that be a double free? The function can also return
> NETDEV_TX_BUSY, and the core would then requeue a freed skb.
>
> The setup needs CAP_NET_ADMIN to attach teql as root on a VLAN or
> macvlan device. After that, ordinary congestion on the lower device is
> enough to hit this path. The rest of the series does not seem to fix it
> either; the final state still compares against NETDEV_TX_OK.
>
> Since this patch already reworks the post-failure skb handling on this
> path, would it make sense to check dev_xmit_complete() on the return
> value instead of comparing it with NETDEV_TX_OK?
>
> >       } while ((q = rcu_dereference(NEXT_SLAVE(q))) != start);
> >
> >       if (nores && skb_res == NULL) {
>
> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/QDISC-6HS9.v1.20260928064754%40mojatatu.com

Re: [PATCH net 1/2] net/sched: sch_teql: fix shared headroom and header strip on slave retry

[Jamal Hadi Salim]

On Sat, Oct 3, 2026 at 6:57 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
> On Wed, Sep 30, 2026 at 9:07 AM <netdev-bot+sashiko@kernel.org> wrote:
> >
> > Thank you for your contribution! Sashiko AI review found 1 potential
> > issue(s) to consider.
> >
> > Critical: 0 (1) · High: 0 · Medium: 1 · Low: 0
> >
> > If you believe none of the issues are legitimate, please reply to each
> > issue of Medium severity or higher and explain why the AI is wrong.
> > If any of them are legitimate and you plan to respin, please let patchwork
> > know by including "pw-bot: cr" as a separate line at the end of your reply
> > (one such reply per series is enough).
> >
> > - [Medium] The commit message and the new code comment give the wrong
> >   reason for problem 1 and the wrong way to reproduce it.
> >
> > Pre-existing issues:
> > - [Critical] teql_master_xmit() in net/sched/sch_teql.c treats a slave
> >   transmit as successful only when `netdev_start_xmit(skb, slave,…
> >
>
> Sorry, I missed responding to this:
> This is  a different root cause from this patch series - we are going
> to send a followup fix if no one beats us to it.
>

For completion:  nothing on patch 2. So i dont see any reason to
resend this series.

cheers,
jamal

> cheers,
> jamal
>
> > --
> >
> > net/sched: sch_teql: fix shared headroom and header strip on slave retry
> >
> > The patch adds a skb_cow_head() call before dev_hard_header() in
> > __teql_resolve(). It also changes teql_master_xmit() so that, when a
> > slave attempt fails, it pulls back only the header length that
> > teql_resolve() pushed for that slave. It no longer pulls
> > skb_network_offset() bytes.
> >
> > Link: https://lore.kernel.org/netdev/20260824115928.4099988-1-victor@mojatatu.com/
> > Closes: https://sashiko.dev/#/patchset/20260824115928.4099988-1-victor@mojatatu.com
> >
> > > diff --git a/net/sched/sch_teql.c b/net/sched/sch_teql.c
> > > index 409ce50cc0dbc..acd03f9afc6b8 100644
> > > --- a/net/sched/sch_teql.c
> > > +++ b/net/sched/sch_teql.c
> >
> > [ ... ]
> >
> > > @@ -265,15 +265,28 @@ __teql_resolve(struct sk_buff *skb, struct sk_buff *skb_res,
> > >       }
> > >
> > >       if (neigh_event_send(n, skb_res) == 0) {
> > > +             int off = skb_network_offset(skb);
> > >               char haddr[MAX_ADDR_LEN];
> > >
> > >               neigh_ha_snapshot(haddr, n, dev);
> > > +             /* The skb may be shared (e.g. a packet tap clone); make the
> > > +              * headroom private before dev_hard_header() writes into it.
> > > +              */
> > > +             if (skb_cow_head(skb, LL_RESERVED_SPACE(dev)) < 0) {
> >
> > [Severity: Medium]
> >
> > The skb_cow_head() call looks right. Are this comment and the commit
> > message accurate about why it is needed?
> >
> > The commit message says:
> >
> >     1. When the skb is shared (for example a packet tap installed on the
> >        master makes xmit_one() clone it before teql_master_xmit()),
> >
> > and later:
> >
> >     and a shared skb is unshared before it is modified.
> >
> > skb_cow_head() only handles skb_header_cloned(). It does nothing when
> > skb_shared() is true (users > 1). If it has to expand the head,
> > pskb_expand_head() has:
> >
> >     BUG_ON(skb_shared(skb));
> >
> > Would "header-cloned" be a more accurate term than "shared" in both
> > places?
> >
> > Can the packet tap reproducer actually show corruption?
> > teql_master_setup() never sets header_ops and uses ARPHRD_VOID, so
> > dev_has_header() is false for the master. packet_rcv() and tpacket_rcv()
> > only push back to the mac header when it is true:
> >
> >     if (dev_has_header(dev)) {
> >         if (sk->sk_type != SOCK_DGRAM)
> >             skb_push(skb, skb->data - skb_mac_header(skb));
> >
> > So the tap only sees bytes from skb->data onward. __teql_resolve()
> > writes the slave header into [data - hlen, data), which the tap never
> > reads.
> >
> > A trigger that does seem to cause visible corruption is a tc mirred
> > mirror on a physical device's clsact egress, with the teql master as
> > the target.
> >
> > sch_handle_egress() runs before skb_dst_drop(), so the clone keeps its
> > dst. ARPHRD_VOID is not a mac-header-xmit type, so tcf_mirred_to_dev()
> > pulls mac_len off the clone:
> >
> >     expects_nh = want_ingress || !m_mac_header_xmit;
> >     at_nh = skb->data == skb_network_header(skb);
> >     if (at_nh != expects_nh) {
> >         ...
> >             skb_pull_rcsum(skb_to_send, mac_len);
> >
> > The original skb's Ethernet header then sits in the clone's shared
> > headroom, and dev_hard_header() in teql overwrites it.
> >
> > Should the reproduction steps in the commit message describe this case
> > instead?
> >
> > The subject and body also say both problems happen "on slave retry".
> > The header write, and now the COW, happens on every slave attempt,
> > including a first attempt that succeeds. Could the wording reflect
> > that?
> >
> > [ ... ]
> >
> > > @@ -374,8 +390,14 @@ static netdev_tx_t teql_master_xmit(struct sk_buff *skb, struct net_device *dev)
> > >                       nores = 1;
> > >                       break;
> > >               }
> > > +             /* Undo only the header teql_resolve() pushed for this slave.
> > > +              * Pulling skb_network_offset() instead would strip a
> > > +              * userspace-supplied header when the slave added none, e.g.
> > > +              * an AF_PACKET/SOCK_RAW frame with no dst entry.
> > > +              */
> > > +             if (hlen > 0)
> > > +                     __skb_pull(skb, hlen);
> > >               skb->dev = dev;
> >
> > [Severity: Critical]
> >
> > This isn't a bug introduced by this patch, but can this __skb_pull()
> > and the skb->dev write touch an skb that the slave has already
> > consumed?
> >
> > teql_master_xmit() counts a slave transmit as successful only when it
> > returns NETDEV_TX_OK:
> >
> >     if (!netif_xmit_frozen_or_stopped(slave_txq) &&
> >         netdev_start_xmit(skb, slave, slave_txq, false) ==
> >         NETDEV_TX_OK) {
> >
> > Stacked slaves such as 8021q, and macvlan through macvlan_start_xmit(),
> > return the lower device's dev_queue_xmit() result unchanged:
> >
> > net/8021q/vlan_dev.c:vlan_dev_hard_start_xmit() {
> >     ...
> >     ret = dev_queue_xmit(skb);
> >     ...
> >     return ret;
> > }
> >
> > That result can be NET_XMIT_DROP, NET_XMIT_CN or a negative errno.
> > dev_xmit_complete() treats all of these as consumed:
> >
> >     if (likely(rc < NET_XMIT_MASK))
> >         return true;
> >
> > For NET_XMIT_DROP the skb is already freed, for example in
> > __dev_xmit_skb():
> >
> >     kfree_skb_reason(skb, SKB_DROP_REASON_QDISC_BURST_DROP);
> >     return NET_XMIT_DROP;
> >
> > For NET_XMIT_CN it may still be queued on the lower qdisc.
> >
> > In both cases teql falls through to this block. For a VLAN slave with a
> > resolved neighbour, hlen is ETH_HLEN + VLAN_HLEN, so __skb_pull() and
> > skb->dev = dev write to the consumed skb.
> >
> > The loop then calls teql_resolve(), skb_cow_head(), dev_hard_header()
> > and netdev_start_xmit() again on it, or it reaches the drop label:
> >
> >     master->tx_dropped++;
> >     rcu_read_unlock();
> >     dev_kfree_skb(skb);
> >
> > Would that be a double free? The function can also return
> > NETDEV_TX_BUSY, and the core would then requeue a freed skb.
> >
> > The setup needs CAP_NET_ADMIN to attach teql as root on a VLAN or
> > macvlan device. After that, ordinary congestion on the lower device is
> > enough to hit this path. The rest of the series does not seem to fix it
> > either; the final state still compares against NETDEV_TX_OK.
> >
> > Since this patch already reworks the post-failure skb handling on this
> > path, would it make sense to check dev_xmit_complete() on the return
> > value instead of comparing it with NETDEV_TX_OK?
> >
> > >       } while ((q = rcu_dereference(NEXT_SLAVE(q))) != start);
> > >
> > >       if (nores && skb_res == NULL) {
> >
> > --
> > Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/QDISC-6HS9.v1.20260928064754%40mojatatu.com