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

From: Sabrina Dubroca <[email-masked]>
Date: Tue, 3 Sep 2024 13:53:23 +0200

2024-09-03, 16:03:06 +0530, Bharat Bhushan wrote:
> On Tue, Sep 3, 2024 at 3:08 PM Sabrina Dubroca <[email-masked]> 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