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

[Sabrina Dubroca <sd@queasysnail.net>]

2026-05-30, 08:16:19 +0200, Petr Wozniak wrote:
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 804e8ad25..618e6299f 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -4079,6 +4079,7 @@ static struct sk_buff *validate_xmit_skb(struct sk_buff *skb, struct net_device
>  struct sk_buff *validate_xmit_skb_list(struct sk_buff *skb, struct net_device *dev, bool *again)
>  {
>  	struct sk_buff *next, *head = NULL, *tail;
> +	bool stolen = false;
>  
>  	for (; skb != NULL; skb = next) {
>  		next = skb->next;
> @@ -4088,8 +4089,11 @@ struct sk_buff *validate_xmit_skb_list(struct sk_buff *skb, struct net_device *d
>  		skb->prev = skb;
>  
>  		skb = validate_xmit_skb(skb, dev, again);
> -		if (!skb)
> +		if (IS_ERR_OR_NULL(skb)) {
> +			if (IS_ERR(skb))
> +				stolen = true;
>  			continue;
> +		}
>  
>  		if (!head)
>  			head = skb;
> @@ -4100,6 +4104,8 @@ struct sk_buff *validate_xmit_skb_list(struct sk_buff *skb, struct net_device *d
>  		 */
>  		tail = skb->prev;
>  	}
> +	if (!head && stolen)
> +		return ERR_PTR(-EINPROGRESS);

Why? You said previously that this path isn't affected (sure, since
validate_xmit_skb_list ignores intermediate returns).

Also, sashiko complains that if you're going to do this, the callers
should be updated to handle EINPROGRESS. And from my side, if you want
to keep this "bool stolen" handling here, stolen should only be true
only for EINPROGRESS, not all errors that validate_xmit_skb could
return.

>  	return head;
>  }
>  EXPORT_SYMBOL_GPL(validate_xmit_skb_list);
> @@ -4859,8 +4865,13 @@ int __dev_queue_xmit(struct sk_buff *skb, struct net_device *sb_dev)
>  			goto recursion_alert;
>  
>  		skb = validate_xmit_skb(skb, dev, &again);
> -		if (!skb)
> +		if (IS_ERR_OR_NULL(skb)) {
> +			/* -EINPROGRESS: packet stolen by async xfrm crypto,
> +			 * delivered via xfrm_dev_resume(). */
> +			if (PTR_ERR(skb) == -EINPROGRESS)
> +				rc = NET_XMIT_SUCCESS;
>  			goto out;
> +		}
>  
>  		HARD_TX_LOCK(dev, txq, cpu);
>  
> diff --git a/net/xfrm/xfrm_device.c b/net/xfrm/xfrm_device.c
> index 5454be0b2..7702cca2b 100644
> --- a/net/xfrm/xfrm_device.c
> +++ b/net/xfrm/xfrm_device.c
> @@ -182,7 +182,7 @@ struct sk_buff *validate_xmit_xfrm(struct sk_buff *skb, netdev_features_t featur
>  		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);

c/p'ing my comment from v1 since you didn't address it (either with a
change or a reply to explain why it's not a valid concern):

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