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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <Ztb4sx49J46VLBFh@hog>
Date: Tue, 3 Sep 2024 13:53:23 +0200
From: Sabrina Dubroca <sd@...asysnail.net>
To: Bharat Bhushan <bharatb.linux@...il.com>
Cc: Bharat Bhushan <bbhushan2@...vell.com>, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org, sgoutham@...vell.com,
	gakula@...vell.com, sbhatta@...vell.com, hkelam@...vell.com,
	davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
	pabeni@...hat.com, jerinj@...vell.com, lcherian@...vell.com,
	richardcochran@...il.com
Subject: Re: [net-next PATCH v8 5/8] cn10k-ipsec: Add SA add/del support for
 outb ipsec crypto offload

2024-09-03, 16:03:06 +0530, Bharat Bhushan wrote:
> On Tue, Sep 3, 2024 at 3:08 PM Sabrina Dubroca <sd@...asysnail.net> 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