Re: [PATCH ipsec-next v1 1/5] xfrm: delay initialization of offload path till its actually requested

[Leon Romanovsky <leon@kernel.org>]

On Fri, Jun 06, 2025 at 05:12:52PM +0200, Sabrina Dubroca wrote:
> 2025-06-05, 17:16:24 +0300, Leon Romanovsky wrote:
> > On Thu, Jun 05, 2025 at 03:09:19PM +0200, Sabrina Dubroca wrote:
> > > Hello,
> > > 
> > > I think we need to revert this patch. It causes a severe performance
> > > regression for SW IPsec (around 40-50%).
> > > 
> > > 2025-02-19, 15:50:57 +0200, Leon Romanovsky wrote:
> > > > From: Leon Romanovsky <leonro@nvidia.com>
> > > > 
> > > > XFRM offload path is probed even if offload isn't needed at all. Let's
> > > > make sure that x->type_offload pointer stays NULL for such path to
> > > > reduce ambiguity.
> > > 
> > > x->type_offload is used for GRO with SW IPsec, not just for HW offload.
> > 
> > Thanks for the report, can you please try the following fix?
> 
> Seems to work in my setup. That's basically a revert of every
> functional change in 585b64f5a620 ("xfrm: delay initialization of
> offload path till its actually requested"), except that now we set
> ->type_offload during xfrm_state_construct instead of
> __xfrm_init_state, so other callers of __xfrm_init_state
> (xfrm_state_migrate and pfkey - we can ignore ipcomp since it doesn't
> have ->type_offload) won't get ->type_offload set correctly. I'm not
> sure we want that.
> 
> Do you need to also revert 49431af6c4ef ("xfrm: rely on XFRM offload")
> from this series? The assumption that x->type_offload is set only for
> HW offload isn't correct.

I don't think so, we are not setting x->type_offload in crypto and packet
offload modes and it is enough for us to rely on offload type.

  230 int xfrm_dev_state_add(struct net *net, struct xfrm_state *x,
  231                        struct xfrm_user_offload *xuo,
  232                        struct netlink_ext_ack *extack)
  233 {
  ...
  308         if (!x->type_offload) {
  309                 NL_SET_ERR_MSG(extack, "Type doesn't support offload");
  310                 dev_put(dev);
  311                 return -EINVAL;
  312         }

Thanks

> 
> -- 
> Sabrina