netdev - Re: [PATCH net-next] net: mana: Improve mana_set

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: <20240830051321.GA27295@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
Date: Thu, 29 Aug 2024 22:13:21 -0700
From: Shradha Gupta <shradhagupta@...ux.microsoft.com>
To: Haiyang Zhang <haiyangz@...rosoft.com>
Cc: Gerhard Engleder <gerhard@...leder-embedded.com>,
	"linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
	KY Srinivasan <kys@...rosoft.com>, Wei Liu <wei.liu@...nel.org>,
	Dexuan Cui <decui@...rosoft.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Long Li <longli@...rosoft.com>, Simon Horman <horms@...nel.org>,
	Konstantin Taranov <kotaranov@...rosoft.com>,
	Souradeep Chakrabarti <schakrabarti@...ux.microsoft.com>,
	Erick Archer <erick.archer@...look.com>,
	Pavan Chebbi <pavan.chebbi@...adcom.com>,
	Ahmed Zaki <ahmed.zaki@...el.com>,
	Colin Ian King <colin.i.king@...il.com>,
	Shradha Gupta <shradhagupta@...rosoft.com>
Subject: Re: [PATCH net-next] net: mana: Improve mana_set_channels() for low
 mem conditions

On Thu, Aug 29, 2024 at 09:00:05PM +0000, Haiyang Zhang wrote:
> 
> 
> > -----Original Message-----
> > From: Gerhard Engleder <gerhard@...leder-embedded.com>
> > Sent: Thursday, August 29, 2024 3:54 PM
> > To: Shradha Gupta <shradhagupta@...ux.microsoft.com>; linux-
> > hyperv@...r.kernel.org; netdev@...r.kernel.org; linux-
> > kernel@...r.kernel.org; linux-rdma@...r.kernel.org
> > Cc: KY Srinivasan <kys@...rosoft.com>; Haiyang Zhang
> > <haiyangz@...rosoft.com>; Wei Liu <wei.liu@...nel.org>; Dexuan Cui
> > <decui@...rosoft.com>; David S. Miller <davem@...emloft.net>; Eric
> > Dumazet <edumazet@...gle.com>; Jakub Kicinski <kuba@...nel.org>; Paolo
> > Abeni <pabeni@...hat.com>; Long Li <longli@...rosoft.com>; Simon Horman
> > <horms@...nel.org>; Konstantin Taranov <kotaranov@...rosoft.com>;
> > Souradeep Chakrabarti <schakrabarti@...ux.microsoft.com>; Erick Archer
> > <erick.archer@...look.com>; Pavan Chebbi <pavan.chebbi@...adcom.com>;
> > Ahmed Zaki <ahmed.zaki@...el.com>; Colin Ian King
> > <colin.i.king@...il.com>; Shradha Gupta <shradhagupta@...rosoft.com>
> > Subject: Re: [PATCH net-next] net: mana: Improve mana_set_channels() for
> > low mem conditions
> > 
> > [Some people who received this message don't often get email from
> > gerhard@...leder-embedded.com. Learn why this is important at
> > https://aka.ms/LearnAboutSenderIdentification ]
> > 
> > On 29.08.24 16:16, Shradha Gupta wrote:
> > > The mana_set_channels() function requires detaching the mana
> > > driver and reattaching it with changed channel values.
> > > During this operation if the system is low on memory, the reattach
> > > might fail, causing the network device being down.
> > > To avoid this we pre-allocate buffers at the beginning of set
> > operation,
> > > to prevent complete network loss
> > >
> > > Signed-off-by: Shradha Gupta <shradhagupta@...ux.microsoft.com>
> > > ---
> > >   .../ethernet/microsoft/mana/mana_ethtool.c    | 28 +++++++++++-------
> > -
> > >   1 file changed, 16 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
> > b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
> > > index d6a35fbda447..5077493fdfde 100644
> > > --- a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
> > > +++ b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
> > > @@ -345,27 +345,31 @@ static int mana_set_channels(struct net_device
> > *ndev,
> > >       struct mana_port_context *apc = netdev_priv(ndev);
> > >       unsigned int new_count = channels->combined_count;
> > >       unsigned int old_count = apc->num_queues;
> > > -     int err, err2;
> > > +     int err;
> > > +
> > > +     apc->num_queues = new_count;
> > > +     err = mana_pre_alloc_rxbufs(apc, ndev->mtu);
> > > +     apc->num_queues = old_count;
> > 
> > Are you sure that temporary changing num_queues has no side effects on
> > other num_queues users like mana_chn_setxdp()?
> > 
> 
> mana_chn_setxdp() is protected by rtnl_lock, which is OK. But I'm not sure
> if all other users are protected. mana_get_stats64() seems not.
> 
> @Shradha Gupta You can add num_queues as an argument of mana_pre_alloc_rxbufs()
> to avoid changing apc->num_queues.
> 
> Thanks,
> - Haiyang

Thanks Haiyang and Gerhard. Instead of changing the apc structure value,
I will pass it to the mana_pre_alloc_rxbufs() function in the next
version. That should make sure other calls are unaffected.

Thanks,
Shradha.