Re: [PATCH 1/1] IB/iser: Remove hard coded values for cqe and send_wr

Sagi Grimberg
2014-10-21T14:49:24Z

On 10/20/2014 8:36 AM, Minh Duc Tran wrote:
> Hi Sagi,
>
> I've created a new patch over the 21 iser patches you have mentioned early in this thread. It is pasted at the end of this email.
> If I understand correctly, this patch will be applied along with Or's TODO list.
>
>
>>> ISER_MAX_RX_CQ_LEN 4096 /* This number should be calculated during create_session */
>
>> So in iSER CQs are shared a across the device - so this number should satisfy maximum number of connections per CQ (which is currently 8).
>
> Again, there should be no rules enforcing a CQ to support 8 connections. The underlining hw should be able to supports more or less as it is configured to do. Specific to the ocrdma hw, it has lower number of CQE per CQ but it has 32 CQ

We can take the minimum of supported, calculated.

>
>>> ISER_QP_MAX_RECV_DTOS 512 /* Why can't we use ISCSI_DEF_XMIT_CMDS_MAX here ? */
>
>> iSER creates the connection QP when before the session is created - so it doesn't know what the user will set at cmds_max (which is potentially larger than ISCSI_DEF_XMIT_CMDS_MAX). So we allow 512 at the moment and adjust the session cmds_max accordingly. I agree that this is a >work around for the moment as we don't know at QP creation time what is the user setting of cmds_max.
>
> Like any other drivers, we should limit this number to the range hw supports. If user setting is within hw range then number will be adjusted accordingly. If user setting is not within hw range, set it to default values.
> What about something like this:
> #define ISER_GET_MAX_XMIT_CMDS(send_wr) (send_wr - ISER_MAX_TX_MISC_PDUS - \
> ISER_MAX_RX_MISC_PDUS) / \
> (1 + ISER_INFLIGHT_DATAOUTS)
> cmds_max_supported = ISER_GET_MAX_XMIT_CMDS(dev_attr->max_qp_wr)
>

I agree, but we should not let the user set too much commands if we do.

>
>>> ISER_MAX_TX_CQ_LEN 36944 /* the mlx4 hw supports up to 3 CQ, but the ocrdma hw supports up to 32CQ with lower number of cqe per CQ */
>
>> What led you to conclude that: "the mlx4 hw supports up to 3 CQ"?
>> TX CQ length should be
>
> I was debugging some iser target problems sometimes back. I could be wrong with 3CQ but it's not far from the hard limit of 4CQ set by current ib/iser code

This specifically originates in a mlx4_core bug in RoCE mode (supports
only 3 EQs). CCing Matan who promised me he would fix it...

Sagi.