Re: [PATCH 3/4] RDMA/siw: Set qp_state in siw_query_qp

[Guoqing Jiang <guoqing.jiang@linux.dev>]

Hi Bernard,

On 12/1/23 02:09, Bernard Metzler wrote:
>> -----Original Message-----
>> From: Guoqing Jiang <guoqing.jiang@linux.dev>
>> Sent: Wednesday, November 29, 2023 4:24 AM
>> To: Bernard Metzler <BMT@zurich.ibm.com>; jgg@ziepe.ca; leon@kernel.org
>> Cc: linux-rdma@vger.kernel.org; guoqing.jiang@linux.dev
>> Subject: [EXTERNAL] [PATCH 3/4] RDMA/siw: Set qp_state in siw_query_qp
>>
>> Run test_query_rc_qp against siw failed since siw didn't set qp_state
>> accordingly. To address it, introduce siw_qp_state_to_ib_qp_state
>> which convert SIW_QP_STATE_IDLE to IB_QPS_INIT which is similar as
>> in cxgb4.
>>
>> rdma-core# ./build/bin/run_tests.py --dev siw0
>> tests.test_qp.QPTest.test_query_rc_qp -v
>> test_query_rc_qp (tests.test_qp.QPTest)
>> Queries an RC QP after creation. Verifies that its properties are as ...
>> FAIL
>>
>> ======================================================================
>> FAIL: test_query_rc_qp (tests.test_qp.QPTest)
>> Queries an RC QP after creation. Verifies that its properties are as
>> ----------------------------------------------------------------------
>> Traceback (most recent call last):
>>    File "/home/gjiang/rdma-core/tests/test_qp.py", line 284, in
>> test_query_rc_qp
>>      self.query_qp_common_test(e.IBV_QPT_RC)
>>    File "/home/gjiang/rdma-core/tests/test_qp.py", line 265, in
>> query_qp_common_test
>>      self.verify_qp_attrs(caps, e.IBV_QPS_INIT, qp_init_attr, qp_attr)
>>    File "/home/gjiang/rdma-core/tests/test_qp.py", line 239, in
>> verify_qp_attrs
>>      self.assertEqual(state, attr.qp_state)
>> AssertionError: <ibv_qp_state.IBV_QPS_INIT: 1> != 0
>>
>> ----------------------------------------------------------------------
>> Ran 1 test in 0.057s
>>
>> FAILED (failures=1)
>>
>> Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
> Very nice finding! How could that remain undetected.
> Probably no real application checks QP state 😉

My guess is no one run the test after the commit 😉.

698f2ae80476 tests: Fix comparing qp_state for iWARP providers.

>> ---
>>   drivers/infiniband/sw/siw/siw_verbs.c | 10 ++++++++++
>>   1 file changed, 10 insertions(+)
>>
>> diff --git a/drivers/infiniband/sw/siw/siw_verbs.c
>> b/drivers/infiniband/sw/siw/siw_verbs.c
>> index dca6a155523d..233985434cfd 100644
>> --- a/drivers/infiniband/sw/siw/siw_verbs.c
>> +++ b/drivers/infiniband/sw/siw/siw_verbs.c
>> @@ -19,6 +19,15 @@
>>   #include "siw_verbs.h"
>>   #include "siw_mem.h"
>>
>> +static int siw_qp_state_to_ib_qp_state[IB_QPS_ERR] = {
> You may better use SIW_QP_STATE_COUNT for the size
> of the array

Right, fully agree, will change it in next version after you finish
review this version.

Thanks,
Guoqing