[PATCH net] xfrm: propagate -EINPROGRESS from validate_xmit_xfrm()

[PATCH net] xfrm: propagate -EINPROGRESS from validate_xmit_xfrm()

[Petr Wozniak]

validate_xmit_xfrm() returns NULL both when a packet is dropped and
when it is stolen by async crypto (-EINPROGRESS from the offload xmit
op).  Callers cannot distinguish the two cases.

commit f53c723902d1 ("net: Add asynchronous callbacks for xfrm on
layer 2.") changed the semantics of validate_xmit_xfrm() returning
NULL from "packet was dropped" to "packet was stolen (may still be in
flight)", but __dev_queue_xmit() was not updated to reflect this.

On devices with a real qdisc, sch_direct_xmit() handles a NULL skb
gracefully and async completion via xfrm_dev_resume() delivers the
packet correctly.  On virtual/bridge interfaces (noqueue qdisc),
however, __dev_queue_xmit() takes the direct branch:

  rc = -ENOMEM;
  skb = validate_xmit_skb(skb, dev, &again);
  if (!skb)
      goto out;          /* returns -ENOMEM to the caller */

The packet is in fact delivered correctly by the async completion
path — the -ENOMEM is a misleading return code, not an actual drop.

Fix this by returning ERR_PTR(-EINPROGRESS) from validate_xmit_xfrm()
for the async case so callers can differentiate it from a real drop.
Update validate_xmit_skb_list() and __dev_queue_xmit() accordingly.
__dev_queue_xmit() now returns NET_XMIT_SUCCESS for the async case,
which accurately reflects that the packet has been accepted.

Fixes: f53c723902d1 ("net: Add asynchronous callbacks for xfrm on layer 2.")
Suggested-by: Sabrina Dubroca <sd@queasysnail.net>
Signed-off-by: Petr Wozniak <petr.wozniak@gmail.com>
---
 net/core/dev.c        | 10 ++++++----
 net/xfrm/xfrm_device.c |  2 +-
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index XXXXXXX..XXXXXXX 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3846,7 +3846,7 @@ struct sk_buff *validate_xmit_skb_list(struct sk_buff *skb, struct net_device *
 		skb = validate_xmit_skb(skb, dev, again);
-		if (!skb)
+		if (IS_ERR_OR_NULL(skb))
 			continue;
 
 		if (!head)
@@ -4552,8 +4552,10 @@ int __dev_queue_xmit(struct sk_buff *skb, struct net_device *sb_dev)
 
 			skb = validate_xmit_skb(skb, dev, &again);
-			if (!skb)
-				goto out;
+			if (IS_ERR_OR_NULL(skb)) {
+				if (IS_ERR(skb))
+					rc = 0;
+				goto out;
+			}
 
 			HARD_TX_LOCK(dev, txq, cpu);
diff --git a/net/xfrm/xfrm_device.c b/net/xfrm/xfrm_device.c
index XXXXXXX..XXXXXXX 100644
--- a/net/xfrm/xfrm_device.c
+++ b/net/xfrm/xfrm_device.c
@@ -191,7 +191,7 @@ struct sk_buff *validate_xmit_xfrm(struct sk_buff *skb, netdev_features_t featu
 		err = x->type_offload->xmit(x, skb, esp_features);
 		if (err) {
 			if (err == -EINPROGRESS)
-				return NULL;
+				return ERR_PTR(-EINPROGRESS);
 
 			XFRM_INC_STATS(xs_net(x), LINUX_MIB_XFRMOUTSTATEPROTOERROR);
 			kfree_skb(skb);
-- 
2.50.1

Re: [PATCH net] xfrm: propagate -EINPROGRESS from validate_xmit_xfrm()

[Sabrina Dubroca]

2026-05-28, 08:43:49 +0200, Petr Wozniak wrote:
> diff --git a/net/core/dev.c b/net/core/dev.c
> index XXXXXXX..XXXXXXX 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -3846,7 +3846,7 @@ struct sk_buff *validate_xmit_skb_list(struct sk_buff *skb, struct net_device *
>  		skb = validate_xmit_skb(skb, dev, again);
> -		if (!skb)
> +		if (IS_ERR_OR_NULL(skb))
>  			continue;
>  
>  		if (!head)
> @@ -4552,8 +4552,10 @@ int __dev_queue_xmit(struct sk_buff *skb, struct net_device *sb_dev)
>  
>  			skb = validate_xmit_skb(skb, dev, &again);
> -			if (!skb)
> -				goto out;
> +			if (IS_ERR_OR_NULL(skb)) {
> +				if (IS_ERR(skb))
> +					rc = 0;

I'd only reset rc to 0 if the error was -EINPROGRESS. Other errors
(none exist for now, but just to make this code future-proof) should
be counted as errors (maybe setting rc to that error).

And this probably deserves a comment such as "validate_xmit_skb can
return -EINPROGRESS, for example when a packet is stolen by async
crypto in xfrm".

If you resend this patch, please wait the required 24 hours:
https://docs.kernel.org/process/maintainer-netdev.html#tl-dr

> +				goto out;
> +			}
>  
>  			HARD_TX_LOCK(dev, txq, cpu);
> diff --git a/net/xfrm/xfrm_device.c b/net/xfrm/xfrm_device.c
> index XXXXXXX..XXXXXXX 100644
> --- a/net/xfrm/xfrm_device.c
> +++ b/net/xfrm/xfrm_device.c
> @@ -191,7 +191,7 @@ struct sk_buff *validate_xmit_xfrm(struct sk_buff *skb, netdev_features_t featu
>  		err = x->type_offload->xmit(x, skb, esp_features);
>  		if (err) {
>  			if (err == -EINPROGRESS)
> -				return NULL;
> +				return ERR_PTR(-EINPROGRESS);
>  
>  			XFRM_INC_STATS(xs_net(x), LINUX_MIB_XFRMOUTSTATEPROTOERROR);
>  			kfree_skb(skb);

What about the skb_list_walk_safe loop? If ->xmit() returns
-EINPROGRESS for all the skbs in the chain, we'll end up returning
NULL even though all the packets were stolen by the async path.

-- 
Sabrina