netdev - Re: [PATCH rdma-next 1/2] RDMA/mlx5: Get upper device only if device is lagged

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 for Android: free password hash cracker in your pocket

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

Date: Fri, 18 Aug 2023 13:42:30 -0300
From: Jason Gunthorpe <jgg@...dia.com>
To: Leon Romanovsky <leon@...nel.org>
Cc: Mark Bloch <mbloch@...dia.com>, "David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, linux-rdma@...r.kernel.org,
	Mark Zhang <markzhang@...dia.com>, netdev@...r.kernel.org,
	Paolo Abeni <pabeni@...hat.com>, Saeed Mahameed <saeedm@...dia.com>
Subject: Re: [PATCH rdma-next 1/2] RDMA/mlx5: Get upper device only if device
 is lagged

On Fri, Aug 18, 2023 at 01:33:35PM -0300, Jason Gunthorpe wrote:
> On Wed, Aug 16, 2023 at 09:52:23AM +0300, Leon Romanovsky wrote:
> > From: Mark Bloch <mbloch@...dia.com>
> > 
> > If the RDMA device isn't in LAG mode there is no need
> > to try to get the upper device.
> > 
> > Signed-off-by: Mark Bloch <mbloch@...dia.com>
> > Signed-off-by: Leon Romanovsky <leonro@...dia.com>
> > ---
> >  drivers/infiniband/hw/mlx5/main.c | 22 +++++++++++++++-------
> >  1 file changed, 15 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
> > index f0b394ed7452..215d7b0add8f 100644
> > --- a/drivers/infiniband/hw/mlx5/main.c
> > +++ b/drivers/infiniband/hw/mlx5/main.c
> > @@ -195,12 +195,18 @@ static int mlx5_netdev_event(struct notifier_block *this,
> >  	case NETDEV_CHANGE:
> >  	case NETDEV_UP:
> >  	case NETDEV_DOWN: {
> > -		struct net_device *lag_ndev = mlx5_lag_get_roce_netdev(mdev);
> >  		struct net_device *upper = NULL;
> >  
> > -		if (lag_ndev) {
> > -			upper = netdev_master_upper_dev_get(lag_ndev);
> > -			dev_put(lag_ndev);
> > +		if (ibdev->lag_active) {
> 
> Needs locking to read lag_active

Specifically the use of the bitfield looks messed up.. If lag_active
and some others were set only during probe it could be OK.

But mixing other stuff that is being written concurrently is not OK to
do like this. (eg ib_active via a mlx5 notifier)

Jason