Re: [PATCH ipsec 2/3] Revert "xfrm: Remove unneeded device check from validate_xmit_xfrm"

[Sabrina Dubroca <sd@queasysnail.net>]

2025-07-30, 12:32:13 +0000, Cosmin Ratiu wrote:
> On Wed, 2025-07-30 at 12:26 +0200, Sabrina Dubroca wrote:
> > 2025-07-29, 15:27:39 +0000, Cosmin Ratiu wrote:
> > > On Mon, 2025-07-28 at 17:17 +0200, Sabrina Dubroca wrote:
> > > > This reverts commit d53dda291bbd993a29b84d358d282076e3d01506.
> > > > 
> > > > This change causes traffic using GSO with SW crypto running
> > > > through a
> > > > NIC capable of HW offload to no longer get segmented during
> > > > validate_xmit_xfrm.
> > > > 
> > > > Fixes: d53dda291bbd ("xfrm: Remove unneeded device check from
> > > > validate_xmit_xfrm")
> > > > 
> > > 
> > > Thanks for the fix, but I'm curious about details.
> > > 
> > > In that commit, I tried to map all of the possible code paths. Can
> > > you
> > > please explain what code paths I missed that need real_dev given
> > > that
> > > only bonding should use it now?
> > 
> > After running some more tests, it's not about real_dev, it's the
> > other
> > check ("unlikely(x->xso.dev != dev)" below) that you also removed in
> > that patch that causes the issue in my setup. I don't know how you
> > decided that it should be dropped, since it predates bonding's ipsec
> > offload.
> 
> Apologies for that, I think I assumed that if offload is off, then
> xfrm_offload(skb) is NULL and the code bails out early on "if (!xo)".
> Seems I was wrong. On the TX side, the only place that adds a secpath
> and increments sp->olen (and thus add an xfrm_offload) is in
> xfrm_output, after the xfrm_dev_offload_ok check.

Yes, the "offload" code is used for both HW offload and "SW offloads"
(aka GSO/GRO).

> > The codepath is the usual:
> > __dev_queue_xmit -> validate_xmit_skb -> validate_xmit_xfrm
> > 
> > Since the commit message made the incorrect claim "ESP offload off:
> > validate_xmit_xfrm returns early on !xo." I didn't check if a partial
> > revert was enough to fix the issue. My bad.
> > 
> No problem, good that we caught the actual issue. Will you prepare a
> follow-up patch then?

I'll send a v2 of this series with this patch updated. Thanks.

-- 
Sabrina