Re: [net-next PATCH v8 5/8] cn10k-ipsec: Add SA add/del support for outb ipsec crypto offload

From: Sabrina Dubroca
Date: Tue Sep 03 2024 - 07:53:51 EST


2024-09-03, 16:03:06 +0530, Bharat Bhushan wrote:
> On Tue, Sep 3, 2024 at 3:08 PM Sabrina Dubroca <sd@xxxxxxxxxxxxxxx> wrote:
> > 2024-09-03, 10:29:34 +0530, Bharat Bhushan wrote:
> > > +static void cn10k_ipsec_del_state(struct xfrm_state *x)
> > > +{
> > > + struct net_device *netdev = x->xso.dev;
> > > + struct cn10k_tx_sa_s *sa_entry;
> > > + struct cpt_ctx_info_s *sa_info;
> > > + struct otx2_nic *pf;
> > > + int sa_index;
> > > +
> > > + if (x->xso.dir == XFRM_DEV_OFFLOAD_IN)
> > > + return;
> > > +
> > > + pf = netdev_priv(netdev);
> > > + if (!mutex_trylock(&pf->ipsec.lock)) {
> > > + netdev_err(netdev, "IPSEC device is busy\n");
> > > + return;
> >
> > If we can't take the lock, we leave the state installed on the device
> > and leak some memory? That's not good. I assume we're going to reach
> > HW limits if this happens a bunch of times, and then we can't offload
> > ipsec at all anymore?
> >
> > I think it would be better to wait until we can take the lock.
>
> This is atomic context (in_atomic() is true). So we need to call the
> trylock variant.

Ok. Then I think this should be a spinlock instead of mutex.

--
Sabrina