From: Sabrina Dubroca <sd@queasysnail.net>
To: Petr Wozniak <petr.wozniak@gmail.com>
Cc: netdev@vger.kernel.org, steffen.klassert@secunet.com,
herbert@gondor.apana.org.au, davem@davemloft.net
Subject: Re: [PATCH net-next] xfrm: fix xfrm_dev_offload_ok() returning true for software SAs
Date: Wed, 27 May 2026 20:41:58 +0200 [thread overview]
Message-ID: <ahc69uCgXxxzuoJh@krikkit> (raw)
In-Reply-To: <20260527164717.23624-1-petr.wozniak@gmail.com>
2026-05-27, 18:47:17 +0200, Petr Wozniak wrote:
> 2026-05-27, Sabrina Dubroca wrote:
> > Incorrectly? IPsec in SW with GSO is a valid setup. I think you're
> > breaking that with your patch.
>
> Fair point — SW IPsec with GSO is intentional and the patch is too broad.
(this whole email looks like direct c/p from an LLM...)
> The actual observable bug on this platform (MT7988A, EIP-197 async crypto):
>
> xfrm_dev_offload_ok() → true (SW SA, dev == NULL)
> → esp4_gso_encap() marks the skb
> → validate_xmit_xfrm() → esp_xmit() → async crypto → -EINPROGRESS
> → validate_xmit_xfrm() returns NULL
>
> On bridge interfaces (noqueue qdisc), __dev_queue_xmit() takes the
> direct branch, initialises rc = -ENOMEM and never overwrites it
> when skb is NULL → ENOMEM on every packet.
>
> On real netdevs with a qdisc, sch_direct_xmit() handles NULL
> gracefully and async completion via xfrm_dev_resume() delivers
> the packet correctly.
But the packet is delivered correctly also in the bridge case, right?
Once we enter async crypto, the original datapath becomes irrelevant.
> Where would you suggest the actual fix should go — in the
> bridge/noqueue path, or in validate_xmit_xfrm() / sch_direct_xmit()?
It looks like commit f53c723902d1 ("net: Add asynchronous callbacks
for xfrm on layer 2.") updated the meaning of validate_xmit_xfrm()
returning NULL from "packet was dropped" to "packet was stolen (maybe
dropped, maybe still going)" and this isn't handled well.
I guess validate_xmit_xfrm() could propagate -EINPROGRESS via
ERR_PTR(), and validate_xmit_skb() could return that to its callers.
validate_xmit_skb_list() would need to check IS_ERR_OR_NULL(skb)
instead of !skb, and __dev_queue_xmit() could now differentiate the
EINPROGRESS case from a drop.
--
Sabrina
next prev parent reply other threads:[~2026-05-27 18:42 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20260527140948.78243-1-petr.wozniak@gmail.com>
2026-05-27 16:47 ` [PATCH net-next] xfrm: fix xfrm_dev_offload_ok() returning true for software SAs Petr Wozniak
2026-05-27 18:41 ` Sabrina Dubroca [this message]
2026-05-28 2:46 Petr Wozniak
-- strict thread matches above, loose matches on Subject: below --
2026-05-27 14:09 Petr Wozniak
2026-05-27 16:01 ` Sabrina Dubroca
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ahc69uCgXxxzuoJh@krikkit \
--to=sd@queasysnail.net \
--cc=davem@davemloft.net \
--cc=herbert@gondor.apana.org.au \
--cc=netdev@vger.kernel.org \
--cc=petr.wozniak@gmail.com \
--cc=steffen.klassert@secunet.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.