netdev - Re: [PATCH net-next 4/4] RDMA/rxe: Set queue pair cur_qp

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]

Message-ID: <72029ea9-f550-470e-9e5d-42e95ca4592e@linux.dev>
Date: Fri, 9 Aug 2024 19:06:34 +0800
From: Zhu Yanjun <yanjun.zhu@...ux.dev>
To: Liu Jian <liujian56@...wei.com>, linux-rdma@...r.kernel.org,
 linux-s390@...r.kernel.org, netdev@...r.kernel.org
Cc: jgg@...pe.ca, leon@...nel.org, zyjzyj2000@...il.com,
 wenjia@...ux.ibm.com, jaka@...ux.ibm.com, alibuda@...ux.alibaba.com,
 tonylu@...ux.alibaba.com, guwen@...ux.alibaba.com, davem@...emloft.net,
 edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com
Subject: Re: [PATCH net-next 4/4] RDMA/rxe: Set queue pair cur_qp_state when
 being queried

在 2024/8/9 16:31, Liu Jian 写道:
> Same with commit e375b9c92985 ("RDMA/cxgb4: Set queue pair state when
>   being queried"). The API for ib_query_qp requires the driver to set
> cur_qp_state on return, add the missing set.
> 

Add the following?
Cc: stable@...r.kernel.org

> Fixes: 8700e3e7c485 ("Soft RoCE driver")
> Signed-off-by: Liu Jian <liujian56@...wei.com>
> ---
>   drivers/infiniband/sw/rxe/rxe_verbs.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c b/drivers/infiniband/sw/rxe/rxe_verbs.c
> index 5c18f7e342f2..699b4b315336 100644
> --- a/drivers/infiniband/sw/rxe/rxe_verbs.c
> +++ b/drivers/infiniband/sw/rxe/rxe_verbs.c
> @@ -634,6 +634,8 @@ static int rxe_query_qp(struct ib_qp *ibqp, struct ib_qp_attr *attr,
>   	rxe_qp_to_init(qp, init);
>   	rxe_qp_to_attr(qp, attr, mask);
>   
> +	attr->cur_qp_state = qp->attr.qp_state;

I am fine with this commit.

But I think this "attr->cur_qp_state = qp->attr.qp_state;" should be put 
into this function rxe_qp_to_attr.

And the access to qp->attr.qp_state should be protected by spin lock 
qp->state_lock.

So the following is better.
Any way, thanks.

Reviewed-by: Zhu Yanjun <yanjun.zhu@...ux.dev>

diff --git a/drivers/infiniband/sw/rxe/rxe_qp.c 
b/drivers/infiniband/sw/rxe/rxe_qp.c
index d2f7b5195c19..da723b9690e5 100644
--- a/drivers/infiniband/sw/rxe/rxe_qp.c
+++ b/drivers/infiniband/sw/rxe/rxe_qp.c
@@ -782,6 +782,10 @@ int rxe_qp_to_attr(struct rxe_qp *qp, struct 
ib_qp_attr *attr, int mask)
                 spin_unlock_irqrestore(&qp->state_lock, flags);
         }

+       spin_lock_irqsave(&qp->state_lock, flags);
+       attr->cur_qp_state = qp_state(qp);
+       spin_unlock_irqrestore(&qp->state_lock, flags);
+
         return 0;
  }

Best Regards,
Zhu Yanjun

> +
>   	return 0;
>   }
>