Bug in pyverbs test_resize_cq

Bug in pyverbs test_resize_cq

[Bob Pearson]

Edward,

In the test_resize_cq test the following is written

        # Fill the CQ entries except one for avoid cq_overrun warnings.

        send_wr, _ = u.get_send_elements(self.client, False)

        ah_client = u.get_global_ah(self.client, self.gid_index, self.ib_port)

        for i in range(self.client.cq.cqe - 1):

            u.send(self.client, send_wr, ah=ah_client)



        # Decrease the CQ size to less than the CQ unpolled entries.

        new_cq_size = 1

        with self.assertRaises(PyverbsRDMAError) as ex:

            self.client.cq.resize(new_cq_size)

        self.assertEqual(ex.exception.error_code, errno.EINVAL)


No where does it make any attempt to see if the sends are completed before testing to
resize the cq. Software drivers might not get finished in time and fail the test
because there are no entries in the cq. Technically this is wrong code. You should test
for the completion before attempting to destroy them. Or insert a small delay.

Bob

Re: Bug in pyverbs test_resize_cq

[Edward Srouji]

On 6/19/2022 6:31 AM, Bob Pearson wrote:
> External email: Use caution opening links or attachments
>
>
> Edward,
>
> In the test_resize_cq test the following is written
>
>          # Fill the CQ entries except one for avoid cq_overrun warnings.
>
>          send_wr, _ = u.get_send_elements(self.client, False)
>
>          ah_client = u.get_global_ah(self.client, self.gid_index, self.ib_port)
>
>          for i in range(self.client.cq.cqe - 1):
>
>              u.send(self.client, send_wr, ah=ah_client)
>
>
>
>          # Decrease the CQ size to less than the CQ unpolled entries.
>
>          new_cq_size = 1
>
>          with self.assertRaises(PyverbsRDMAError) as ex:
>
>              self.client.cq.resize(new_cq_size)
>
>          self.assertEqual(ex.exception.error_code, errno.EINVAL)
>
>
> No where does it make any attempt to see if the sends are completed before testing to
> resize the cq. Software drivers might not get finished in time and fail the test
> because there are no entries in the cq. Technically this is wrong code. You should test
> for the completion before attempting to destroy them. Or insert a small delay.

Have you actually encountered this issue?

I understand what you're saying, but we only send 6 WQEs , and we 
intentionally don't poll them (to not "drain" the CQ).
It's enough for the device to get 2 WQEs in order to pass the test, I 
don't expect anyone to fail on that, unless there's a real performance 
issue. In case you encountered with this issue I'll re-look into it.

> Bob
Thanks.