-----"Leon Romanovsky" <leon@xxxxxxxxxx> wrote: -----
>To: "Bernard Metzler" <bmt@xxxxxxxxxxxxxx>
>From: "Leon Romanovsky" <leon@xxxxxxxxxx>
>Date: 04/26/2019 11:12PM
>Cc: linux-rdma@xxxxxxxxxxxxxxx, "Bernard Metzler"
><bmt@xxxxxxxxxxxxxxxxxxx>
>Subject: Re: [PATCH v8 07/12] SIW queue pair methods
>
>On Fri, Apr 26, 2019 at 03:18:47PM +0200, Bernard Metzler wrote:
>> From: Bernard Metzler <bmt@xxxxxxxxxxxxxxxxxxx>
>>
>> Signed-off-by: Bernard Metzler <bmt@xxxxxxxxxxxxxx>
>> ---
>> drivers/infiniband/sw/siw/siw_qp.c | 1354
>++++++++++++++++++++++++++++
>> 1 file changed, 1354 insertions(+)
>> create mode 100644 drivers/infiniband/sw/siw/siw_qp.c
>>
>> diff --git a/drivers/infiniband/sw/siw/siw_qp.c
>b/drivers/infiniband/sw/siw/siw_qp.c
>> new file mode 100644
>> index 000000000000..9c94a406288c
>> --- /dev/null
>> +++ b/drivers/infiniband/sw/siw/siw_qp.c
>> @@ -0,0 +1,1354 @@
>
>...
>
>> +int siw_qp_modify(struct siw_qp *qp, struct siw_qp_attrs *attrs,
>> + enum siw_qp_attr_mask mask)
>> +{
>> + int drop_conn = 0, rv = 0;
>> +
>> + if (!mask)
>> + return 0;
>> +
>> + siw_dbg_qp(qp, "state: %s => %s\n",
>> + siw_qp_state_to_string[qp->attrs.state],
>> + siw_qp_state_to_string[attrs->state]);
>> +
>> + if (mask != SIW_QP_ATTR_STATE)
>> + siw_qp_modify_nonstate(qp, attrs, mask);
>> +
>> + if (!(mask & SIW_QP_ATTR_STATE))
>> + return 0;
>> +
>> + switch (qp->attrs.state) {
>> + case SIW_QP_STATE_IDLE:
>> + case SIW_QP_STATE_RTR:
>> + switch (attrs->state) {
>> + case SIW_QP_STATE_RTS:
>> +
>> + if (attrs->flags & SIW_MPA_CRC) {
>> + rv = siw_qp_enable_crc(qp);
>> + if (rv)
>> + break;
>> + }
>> + if (!(mask & SIW_QP_ATTR_LLP_HANDLE)) {
>> + siw_dbg_qp(qp, "no socket\n");
>> + rv = -EINVAL;
>> + break;
>> + }
>> + if (!(mask & SIW_QP_ATTR_MPA)) {
>> + siw_dbg_qp(qp, "no MPA\n");
>> + rv = -EINVAL;
>> + break;
>> + }
>> + siw_dbg_qp(qp, "enter rts, peer 0x%08x, loc 0x%08x\n",
>> + qp->cep->llp.raddr.sin_addr.s_addr,
>> + qp->cep->llp.laddr.sin_addr.s_addr);
>> + /*
>> + * Initialize iWARP TX state
>> + */
>> + qp->tx_ctx.ddp_msn[RDMAP_UNTAGGED_QN_SEND] = 0;
>> + qp->tx_ctx.ddp_msn[RDMAP_UNTAGGED_QN_RDMA_READ] = 0;
>> + qp->tx_ctx.ddp_msn[RDMAP_UNTAGGED_QN_TERMINATE] = 0;
>> +
>> + /*
>> + * Initialize iWARP RX state
>> + */
>> + qp->rx_ctx.ddp_msn[RDMAP_UNTAGGED_QN_SEND] = 1;
>> + qp->rx_ctx.ddp_msn[RDMAP_UNTAGGED_QN_RDMA_READ] = 1;
>> + qp->rx_ctx.ddp_msn[RDMAP_UNTAGGED_QN_TERMINATE] = 1;
>> +
>> + /*
>> + * init IRD free queue, caller has already checked
>> + * limits.
>> + */
>> + rv = siw_qp_readq_init(qp, attrs->irq_size,
>> + attrs->orq_size);
>> + if (rv)
>> + break;
>> +
>> + qp->attrs.sk = attrs->sk;
>> + qp->attrs.state = SIW_QP_STATE_RTS;
>> + break;
>> +
>> + case SIW_QP_STATE_ERROR:
>> + siw_rq_flush(qp);
>> + qp->attrs.state = SIW_QP_STATE_ERROR;
>> + if (qp->cep) {
>> + siw_cep_put(qp->cep);
>> + qp->cep = NULL;
>> + }
>> + break;
>> +
>> + case SIW_QP_STATE_RTR:
>> + /* ignore */
>> + break;
>> +
>> + default:
>> + siw_dbg_qp(qp, "state transition undefined: %s => %s\n",
>> + siw_qp_state_to_string[qp->attrs.state],
>> + siw_qp_state_to_string[attrs->state]);
>> + break;
>> + }
>> + break;
>> +
>> + case SIW_QP_STATE_RTS:
>> + switch (attrs->state) {
>> + case SIW_QP_STATE_CLOSING:
>> + /*
>> + * Verbs: move to IDLE if SQ and ORQ are empty.
>> + * Move to ERROR otherwise. But first of all we must
>> + * close the connection. So we keep CLOSING or ERROR
>> + * as a transient state, schedule connection drop work
>> + * and wait for the socket state change upcall to
>> + * come back closed.
>> + */
>> + if (tx_wqe(qp)->wr_status == SIW_WR_IDLE) {
>> + qp->attrs.state = SIW_QP_STATE_CLOSING;
>> + } else {
>> + qp->attrs.state = SIW_QP_STATE_ERROR;
>> + siw_sq_flush(qp);
>> + }
>> + siw_rq_flush(qp);
>> +
>> + drop_conn = 1;
>> + break;
>> +
>> + case SIW_QP_STATE_TERMINATE:
>> + qp->attrs.state = SIW_QP_STATE_TERMINATE;
>> +
>> + siw_init_terminate(qp, TERM_ERROR_LAYER_RDMAP,
>> + RDMAP_ETYPE_CATASTROPHIC,
>> + RDMAP_ECODE_UNSPECIFIED, 1);
>> + drop_conn = 1;
>> + break;
>> +
>> + case SIW_QP_STATE_ERROR:
>> + /*
>> + * This is an emergency close.
>> + *
>> + * Any in progress transmit operation will get
>> + * cancelled.
>> + * This will likely result in a protocol failure,
>> + * if a TX operation is in transit. The caller
>> + * could unconditional wait to give the current
>> + * operation a chance to complete.
>> + * Esp., how to handle the non-empty IRQ case?
>> + * The peer was asking for data transfer at a valid
>> + * point in time.
>> + */
>> + siw_sq_flush(qp);
>> + siw_rq_flush(qp);
>> + qp->attrs.state = SIW_QP_STATE_ERROR;
>> + drop_conn = 1;
>> + break;
>> +
>> + default:
>> + siw_dbg_qp(qp, "state transition undefined: %s => %s\n",
>> + siw_qp_state_to_string[qp->attrs.state],
>> + siw_qp_state_to_string[attrs->state]);
>> + break;
>> + }
>> + break;
>> +
>> + case SIW_QP_STATE_TERMINATE:
>> + switch (attrs->state) {
>> + case SIW_QP_STATE_ERROR:
>> + siw_rq_flush(qp);
>> + qp->attrs.state = SIW_QP_STATE_ERROR;
>> +
>> + if (tx_wqe(qp)->wr_status != SIW_WR_IDLE)
>> + siw_sq_flush(qp);
>> + break;
>> +
>> + default:
>> + siw_dbg_qp(qp, "state transition undefined: %s => %s\n",
>> + siw_qp_state_to_string[qp->attrs.state],
>> + siw_qp_state_to_string[attrs->state]);
>> + }
>> + break;
>> +
>> + case SIW_QP_STATE_CLOSING:
>> + switch (attrs->state) {
>> + case SIW_QP_STATE_IDLE:
>> + WARN_ON(tx_wqe(qp)->wr_status != SIW_WR_IDLE);
>> + qp->attrs.state = SIW_QP_STATE_IDLE;
>> + break;
>> +
>> + case SIW_QP_STATE_CLOSING:
>> + /*
>> + * The LLP may already moved the QP to closing
>> + * due to graceful peer close init
>> + */
>> + break;
>> +
>> + case SIW_QP_STATE_ERROR:
>> + /*
>> + * QP was moved to CLOSING by LLP event
>> + * not yet seen by user.
>> + */
>> + qp->attrs.state = SIW_QP_STATE_ERROR;
>> +
>> + if (tx_wqe(qp)->wr_status != SIW_WR_IDLE)
>> + siw_sq_flush(qp);
>> +
>> + siw_rq_flush(qp);
>> + break;
>> +
>> + default:
>> + siw_dbg_qp(qp, "state transition undefined: %s => %s\n",
>> + siw_qp_state_to_string[qp->attrs.state],
>> + siw_qp_state_to_string[attrs->state]);
>> +
>> + return -ECONNABORTED;
>> + }
>> + break;
>> +
>> + default:
>> + siw_dbg_qp(qp, " noop: state %s\n",
>> + siw_qp_state_to_string[qp->attrs.state]);
>> + break;
>> + }
>> + if (drop_conn)
>> + siw_qp_cm_drop(qp, 0);
>
>I wonder if it is just a bad name, but why modify QP should handle
>connections?
Some QP state changes result in a forced TCP disconnect.
So, siw_qp_modify() modifies the state of the QP, and per
iWarp spec, some state transitions request a TCP close.
This is why I have it within the QP modify function itself,
at the very end.
Otherwise, the caller would have to deduct from current
QP state and the success of a requested QP state change if
it must now do the TCP close. I did'n want to replicate
that QP state change logic for all possible callers.
>
>Thanks
>
>
