2007-06-27 15:51:31

by Steve Wise

[permalink] [raw]
Subject: [PATCH 0/6] iw_cxgb3: Bug Fixes for 2.6.23


Hey Roland,

Here are some bug fixes to the iw_cxgb3 driver that I'd like included
for 2.6.23. NOTE: Patch 1 requires a firmware interface change, so
there is a version bump to 4.3 included in that patch that hits cxgb3.
This will likely conflict with a previous version change that is in
Jeff's upstream branch. The net is: we need the firmware version bumped
to 4.3 with these iw_cxgb3 changes.

Thanks,

Steve.

Shortlog:
iw_cxgb3: Streaming -> RDMA mode transition fixes.
iw_cxgb3: TERMINATE WRs can hang the tx ofld queue.
iw_cxgb3: Don't count neg_adv abort_req_rss messages as real aborts.
iw_cxgb3: ctrl-qp init/clear shouldn't set the gen bit.
iw_cxgb3: Don't post TID_RELEASE message.
iw_cxgb3: Don't abort after failures sending the mpa reply.


2007-06-27 15:51:52

by Steve Wise

[permalink] [raw]
Subject: [PATCH 1/6] iw_cxgb3: Streaming -> RDMA mode transition fixes.


iw_cxgb3: Streaming -> RDMA mode transition fixes.

Due to a HW issue, our current scheme to transition the connection from
streaming to rdma mode is broken on the passive side. The firmware
and driver now support a new transition scheme for the passive side:

- driver posts rdma_init_wr (now including the initial receive seqno)

- driver posts last streaming message via TX_DATA message (MPA start
response)

- uP atomically sends the last streaming message and transitions the
tcb to rdma mode.

- driver waits for wr_ack indicating the last streaming message was ACKed.

NOTE: This change also bumps the required firmware version to 4.3.

Signed-off-by: Steve Wise <[email protected]>
---

drivers/infiniband/hw/cxgb3/cxio_hal.c | 2 -
drivers/infiniband/hw/cxgb3/cxio_wr.h | 3 +
drivers/infiniband/hw/cxgb3/iwch_cm.c | 82 ++++++++++++--------------------
drivers/infiniband/hw/cxgb3/iwch_cm.h | 1
drivers/infiniband/hw/cxgb3/iwch_qp.c | 1
drivers/net/cxgb3/version.h | 2 -
6 files changed, 38 insertions(+), 53 deletions(-)

diff --git a/drivers/infiniband/hw/cxgb3/cxio_hal.c b/drivers/infiniband/hw/cxgb3/cxio_hal.c
index 76049af..215bbe5 100644
--- a/drivers/infiniband/hw/cxgb3/cxio_hal.c
+++ b/drivers/infiniband/hw/cxgb3/cxio_hal.c
@@ -833,7 +833,7 @@ int cxio_rdma_init(struct cxio_rdev *rde
wqe->ird = cpu_to_be32(attr->ird);
wqe->qp_dma_addr = cpu_to_be64(attr->qp_dma_addr);
wqe->qp_dma_size = cpu_to_be32(attr->qp_dma_size);
- wqe->rsvd = 0;
+ wqe->irs = cpu_to_be32(attr->irs);
skb->priority = 0; /* 0=>ToeQ; 1=>CtrlQ */
return (cxgb3_ofld_send(rdev_p->t3cdev_p, skb));
}
diff --git a/drivers/infiniband/hw/cxgb3/cxio_wr.h b/drivers/infiniband/hw/cxgb3/cxio_wr.h
index ff7290e..c84d4ac 100644
--- a/drivers/infiniband/hw/cxgb3/cxio_wr.h
+++ b/drivers/infiniband/hw/cxgb3/cxio_wr.h
@@ -294,6 +294,7 @@ struct t3_rdma_init_attr {
u64 qp_dma_addr;
u32 qp_dma_size;
u32 flags;
+ u32 irs;
};

struct t3_rdma_init_wr {
@@ -314,7 +315,7 @@ struct t3_rdma_init_wr {
__be32 ird;
__be64 qp_dma_addr; /* 7 */
__be32 qp_dma_size; /* 8 */
- u32 rsvd;
+ u32 irs;
};

struct t3_genbit {
diff --git a/drivers/infiniband/hw/cxgb3/iwch_cm.c b/drivers/infiniband/hw/cxgb3/iwch_cm.c
index b2faff5..7b8d5aa 100644
--- a/drivers/infiniband/hw/cxgb3/iwch_cm.c
+++ b/drivers/infiniband/hw/cxgb3/iwch_cm.c
@@ -515,7 +515,7 @@ static void send_mpa_req(struct iwch_ep
req->len = htonl(len);
req->param = htonl(V_TX_PORT(ep->l2t->smt_idx) |
V_TX_SNDBUF(snd_win>>15));
- req->flags = htonl(F_TX_IMM_ACK|F_TX_INIT);
+ req->flags = htonl(F_TX_INIT);
req->sndseq = htonl(ep->snd_seq);
BUG_ON(ep->mpa_skb);
ep->mpa_skb = skb;
@@ -566,7 +566,7 @@ static int send_mpa_reject(struct iwch_e
req->len = htonl(mpalen);
req->param = htonl(V_TX_PORT(ep->l2t->smt_idx) |
V_TX_SNDBUF(snd_win>>15));
- req->flags = htonl(F_TX_IMM_ACK|F_TX_INIT);
+ req->flags = htonl(F_TX_INIT);
req->sndseq = htonl(ep->snd_seq);
BUG_ON(ep->mpa_skb);
ep->mpa_skb = skb;
@@ -618,7 +618,7 @@ static int send_mpa_reply(struct iwch_ep
req->len = htonl(len);
req->param = htonl(V_TX_PORT(ep->l2t->smt_idx) |
V_TX_SNDBUF(snd_win>>15));
- req->flags = htonl(F_TX_MORE | F_TX_IMM_ACK | F_TX_INIT);
+ req->flags = htonl(F_TX_INIT);
req->sndseq = htonl(ep->snd_seq);
ep->mpa_skb = skb;
state_set(&ep->com, MPA_REP_SENT);
@@ -641,6 +641,7 @@ static int act_establish(struct t3cdev *
cxgb3_insert_tid(ep->com.tdev, &t3c_client, ep, tid);

ep->snd_seq = ntohl(req->snd_isn);
+ ep->rcv_seq = ntohl(req->rcv_isn);

set_emss(ep, ntohs(req->tcp_opt));

@@ -1023,6 +1024,9 @@ static int rx_data(struct t3cdev *tdev,
skb_pull(skb, sizeof(*hdr));
skb_trim(skb, dlen);

+ ep->rcv_seq += dlen;
+ BUG_ON(ep->rcv_seq != (ntohl(hdr->seq) + dlen));
+
switch (state_read(&ep->com)) {
case MPA_REQ_SENT:
process_mpa_reply(ep, skb);
@@ -1060,7 +1064,6 @@ static int tx_ack(struct t3cdev *tdev, s
struct iwch_ep *ep = ctx;
struct cpl_wr_ack *hdr = cplhdr(skb);
unsigned int credits = ntohs(hdr->credits);
- enum iwch_qp_attr_mask mask;

PDBG("%s ep %p credits %u\n", __FUNCTION__, ep, credits);

@@ -1072,30 +1075,6 @@ static int tx_ack(struct t3cdev *tdev, s
ep->mpa_skb = NULL;
dst_confirm(ep->dst);
if (state_read(&ep->com) == MPA_REP_SENT) {
- struct iwch_qp_attributes attrs;
-
- /* bind QP to EP and move to RTS */
- attrs.mpa_attr = ep->mpa_attr;
- attrs.max_ird = ep->ord;
- attrs.max_ord = ep->ord;
- attrs.llp_stream_handle = ep;
- attrs.next_state = IWCH_QP_STATE_RTS;
-
- /* bind QP and TID with INIT_WR */
- mask = IWCH_QP_ATTR_NEXT_STATE |
- IWCH_QP_ATTR_LLP_STREAM_HANDLE |
- IWCH_QP_ATTR_MPA_ATTR |
- IWCH_QP_ATTR_MAX_IRD |
- IWCH_QP_ATTR_MAX_ORD;
-
- ep->com.rpl_err = iwch_modify_qp(ep->com.qp->rhp,
- ep->com.qp, mask, &attrs, 1);
-
- if (!ep->com.rpl_err) {
- state_set(&ep->com, FPDU_MODE);
- established_upcall(ep);
- }
-
ep->com.rpl_done = 1;
PDBG("waking up ep %p\n", ep);
wake_up(&ep->com.waitq);
@@ -1378,6 +1357,7 @@ static int pass_establish(struct t3cdev

PDBG("%s ep %p\n", __FUNCTION__, ep);
ep->snd_seq = ntohl(req->snd_isn);
+ ep->rcv_seq = ntohl(req->rcv_isn);

set_emss(ep, ntohs(req->tcp_opt));

@@ -1732,10 +1712,8 @@ int iwch_accept_cr(struct iw_cm_id *cm_i
struct iwch_qp *qp = get_qhp(h, conn_param->qpn);

PDBG("%s ep %p tid %u\n", __FUNCTION__, ep, ep->hwtid);
- if (state_read(&ep->com) == DEAD) {
- put_ep(&ep->com);
+ if (state_read(&ep->com) == DEAD)
return -ECONNRESET;
- }

BUG_ON(state_read(&ep->com) != MPA_REQ_RCVD);
BUG_ON(!qp);
@@ -1755,17 +1733,8 @@ int iwch_accept_cr(struct iw_cm_id *cm_i
ep->ird = conn_param->ird;
ep->ord = conn_param->ord;
PDBG("%s %d ird %d ord %d\n", __FUNCTION__, __LINE__, ep->ird, ep->ord);
+
get_ep(&ep->com);
- err = send_mpa_reply(ep, conn_param->private_data,
- conn_param->private_data_len);
- if (err) {
- ep->com.cm_id = NULL;
- ep->com.qp = NULL;
- cm_id->rem_ref(cm_id);
- abort_connection(ep, NULL, GFP_KERNEL);
- put_ep(&ep->com);
- return err;
- }

/* bind QP to EP and move to RTS */
attrs.mpa_attr = ep->mpa_attr;
@@ -1783,16 +1752,29 @@ int iwch_accept_cr(struct iw_cm_id *cm_i

err = iwch_modify_qp(ep->com.qp->rhp,
ep->com.qp, mask, &attrs, 1);
+ if (err)
+ goto err;

- if (err) {
- ep->com.cm_id = NULL;
- ep->com.qp = NULL;
- cm_id->rem_ref(cm_id);
- abort_connection(ep, NULL, GFP_KERNEL);
- } else {
- state_set(&ep->com, FPDU_MODE);
- established_upcall(ep);
- }
+ err = send_mpa_reply(ep, conn_param->private_data,
+ conn_param->private_data_len);
+ if (err)
+ goto err;
+
+ /* wait for wr_ack */
+ wait_event(ep->com.waitq, ep->com.rpl_done);
+ err = ep->com.rpl_err;
+ if (err)
+ goto err;
+
+ state_set(&ep->com, FPDU_MODE);
+ established_upcall(ep);
+ put_ep(&ep->com);
+ return 0;
+err:
+ ep->com.cm_id = NULL;
+ ep->com.qp = NULL;
+ cm_id->rem_ref(cm_id);
+ abort_connection(ep, NULL, GFP_KERNEL);
put_ep(&ep->com);
return err;
}
diff --git a/drivers/infiniband/hw/cxgb3/iwch_cm.h b/drivers/infiniband/hw/cxgb3/iwch_cm.h
index 21a388c..6107e7c 100644
--- a/drivers/infiniband/hw/cxgb3/iwch_cm.h
+++ b/drivers/infiniband/hw/cxgb3/iwch_cm.h
@@ -175,6 +175,7 @@ struct iwch_ep {
unsigned int atid;
u32 hwtid;
u32 snd_seq;
+ u32 rcv_seq;
struct l2t_entry *l2t;
struct dst_entry *dst;
struct sk_buff *mpa_skb;
diff --git a/drivers/infiniband/hw/cxgb3/iwch_qp.c b/drivers/infiniband/hw/cxgb3/iwch_qp.c
index 714dddb..679b7c1 100644
--- a/drivers/infiniband/hw/cxgb3/iwch_qp.c
+++ b/drivers/infiniband/hw/cxgb3/iwch_qp.c
@@ -732,6 +732,7 @@ #endif
init_attr.qp_dma_addr = qhp->wq.dma_addr;
init_attr.qp_dma_size = (1UL << qhp->wq.size_log2);
init_attr.flags = rqes_posted(qhp) ? RECVS_POSTED : 0;
+ init_attr.irs = qhp->ep->rcv_seq;
PDBG("%s init_attr.rq_addr 0x%x init_attr.rq_size = %d "
"flags 0x%x qpcaps 0x%x\n", __FUNCTION__,
init_attr.rq_addr, init_attr.rq_size,
diff --git a/drivers/net/cxgb3/version.h b/drivers/net/cxgb3/version.h
index b112317..eb508bf 100644
--- a/drivers/net/cxgb3/version.h
+++ b/drivers/net/cxgb3/version.h
@@ -39,6 +39,6 @@ #define DRV_VERSION "1.0-ko"

/* Firmware version */
#define FW_VERSION_MAJOR 4
-#define FW_VERSION_MINOR 0
+#define FW_VERSION_MINOR 3
#define FW_VERSION_MICRO 0
#endif /* __CHELSIO_VERSION_H */

2007-06-27 15:52:16

by Steve Wise

[permalink] [raw]
Subject: [PATCH 2/6] iw_cxgb3: TERMINATE WRs can hang the tx ofld queue.


iw_cxgb3: TERMINATE WRs can hang the tx ofld queue.

Don't set the gen bits nor length bits in the terminate wr. This is
done by the LLD driver.

Signed-off-by: Steve Wise <[email protected]>
---

drivers/infiniband/hw/cxgb3/iwch_qp.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/hw/cxgb3/iwch_qp.c b/drivers/infiniband/hw/cxgb3/iwch_qp.c
index 679b7c1..dd89b6b 100644
--- a/drivers/infiniband/hw/cxgb3/iwch_qp.c
+++ b/drivers/infiniband/hw/cxgb3/iwch_qp.c
@@ -628,9 +628,9 @@ int iwch_post_terminate(struct iwch_qp *
/* immediate data starts here. */
term = (struct terminate_message *)wqe->send.sgl;
build_term_codes(rsp_msg, &term->layer_etype, &term->ecode);
- build_fw_riwrh((void *)wqe, T3_WR_SEND,
- T3_COMPLETION_FLAG | T3_NOTIFY_FLAG, 1,
- qhp->ep->hwtid, 5);
+ wqe->send.wrh.op_seop_flags = cpu_to_be32(V_FW_RIWR_OP(T3_WR_SEND) |
+ V_FW_RIWR_FLAGS(T3_COMPLETION_FLAG | T3_NOTIFY_FLAG));
+ wqe->send.wrh.gen_tid_len = cpu_to_be32(V_FW_RIWR_TID(qhp->ep->hwtid));
skb->priority = CPL_PRIORITY_DATA;
return cxgb3_ofld_send(qhp->rhp->rdev.t3cdev_p, skb);
}

2007-06-27 15:52:28

by Steve Wise

[permalink] [raw]
Subject: [PATCH 3/6] iw_cxgb3: Don't count neg_adv abort_req_rss messages as real aborts.


iw_cxgb3: Don't count neg_adv abort_req_rss messages as real aborts.

negative advice messages should _not_ count toward the 2 abort requests
needed to indicate an abort request.

Signed-off-by: Steve Wise <[email protected]>
---

drivers/infiniband/hw/cxgb3/iwch_cm.c | 14 +++++++-------
1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/infiniband/hw/cxgb3/iwch_cm.c b/drivers/infiniband/hw/cxgb3/iwch_cm.c
index 7b8d5aa..4d7c277 100644
--- a/drivers/infiniband/hw/cxgb3/iwch_cm.c
+++ b/drivers/infiniband/hw/cxgb3/iwch_cm.c
@@ -1465,6 +1465,13 @@ static int peer_abort(struct t3cdev *tde
int ret;
int state;

+ if (is_neg_adv_abort(req->status)) {
+ PDBG("%s neg_adv_abort ep %p tid %d\n", __FUNCTION__, ep,
+ ep->hwtid);
+ t3_l2t_send_event(ep->com.tdev, ep->l2t);
+ return CPL_RET_BUF_DONE;
+ }
+
/*
* We get 2 peer aborts from the HW. The first one must
* be ignored except for scribbling that we need one more.
@@ -1474,13 +1481,6 @@ static int peer_abort(struct t3cdev *tde
return CPL_RET_BUF_DONE;
}

- if (is_neg_adv_abort(req->status)) {
- PDBG("%s neg_adv_abort ep %p tid %d\n", __FUNCTION__, ep,
- ep->hwtid);
- t3_l2t_send_event(ep->com.tdev, ep->l2t);
- return CPL_RET_BUF_DONE;
- }
-
state = state_read(&ep->com);
PDBG("%s ep %p state %u\n", __FUNCTION__, ep, state);
switch (state) {

2007-06-27 15:52:44

by Steve Wise

[permalink] [raw]
Subject: [PATCH 4/6] iw_cxgb3: ctrl-qp init/clear shouldn't set the gen bit.


iw_cxgb3: ctrl-qp init/clear shouldn't set the gen bit.

Signed-off-by: Steve Wise <[email protected]>
---

drivers/infiniband/hw/cxgb3/cxio_hal.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/cxgb3/cxio_hal.c b/drivers/infiniband/hw/cxgb3/cxio_hal.c
index 215bbe5..1518b41 100644
--- a/drivers/infiniband/hw/cxgb3/cxio_hal.c
+++ b/drivers/infiniband/hw/cxgb3/cxio_hal.c
@@ -144,7 +144,7 @@ static int cxio_hal_clear_qp_ctx(struct
}
wqe = (struct t3_modify_qp_wr *) skb_put(skb, sizeof(*wqe));
memset(wqe, 0, sizeof(*wqe));
- build_fw_riwrh((struct fw_riwrh *) wqe, T3_WR_QP_MOD, 3, 1, qpid, 7);
+ build_fw_riwrh((struct fw_riwrh *) wqe, T3_WR_QP_MOD, 3, 0, qpid, 7);
wqe->flags = cpu_to_be32(MODQP_WRITE_EC);
sge_cmd = qpid << 8 | 3;
wqe->sge_cmd = cpu_to_be64(sge_cmd);
@@ -548,7 +548,7 @@ static int cxio_hal_init_ctrl_qp(struct
V_EC_UP_TOKEN(T3_CTL_QP_TID) | F_EC_VALID)) << 32;
wqe = (struct t3_modify_qp_wr *) skb_put(skb, sizeof(*wqe));
memset(wqe, 0, sizeof(*wqe));
- build_fw_riwrh((struct fw_riwrh *) wqe, T3_WR_QP_MOD, 0, 1,
+ build_fw_riwrh((struct fw_riwrh *) wqe, T3_WR_QP_MOD, 0, 0,
T3_CTL_QP_TID, 7);
wqe->flags = cpu_to_be32(MODQP_WRITE_EC);
sge_cmd = (3ULL << 56) | FW_RI_SGEEC_START << 8 | 3;

2007-06-27 15:52:59

by Steve Wise

[permalink] [raw]
Subject: [PATCH 5/6] iw_cxgb3: Don't post TID_RELEASE message.


iw_cxgb3: Don't post TID_RELEASE message.

The LLD does this for us in cxgb3_remove_tid().

Also fixed active open failure cases where we shouldn't
be releasing the TID as well.

Signed-off-by: Steve Wise <[email protected]>
---

drivers/infiniband/hw/cxgb3/iwch_cm.c | 13 ++++++++++---
1 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/hw/cxgb3/iwch_cm.c b/drivers/infiniband/hw/cxgb3/iwch_cm.c
index 4d7c277..228721f 100644
--- a/drivers/infiniband/hw/cxgb3/iwch_cm.c
+++ b/drivers/infiniband/hw/cxgb3/iwch_cm.c
@@ -254,8 +254,6 @@ static void release_ep_resources(struct
cxgb3_remove_tid(ep->com.tdev, (void *)ep, ep->hwtid);
dst_release(ep->dst);
l2t_release(L2DATA(ep->com.tdev), ep->l2t);
- if (ep->com.tdev->type == T3B)
- release_tid(ep->com.tdev, ep->hwtid, NULL);
put_ep(&ep->com);
}

@@ -1103,6 +1101,15 @@ static int abort_rpl(struct t3cdev *tdev
return CPL_RET_BUF_DONE;
}

+/*
+ * Return whether a failed active open has allocated a TID
+ */
+static inline int act_open_has_tid(int status)
+{
+ return status != CPL_ERR_TCAM_FULL && status != CPL_ERR_CONN_EXIST &&
+ status != CPL_ERR_ARP_MISS;
+}
+
static int act_open_rpl(struct t3cdev *tdev, struct sk_buff *skb, void *ctx)
{
struct iwch_ep *ep = ctx;
@@ -1112,7 +1119,7 @@ static int act_open_rpl(struct t3cdev *t
status2errno(rpl->status));
connect_reply_upcall(ep, status2errno(rpl->status));
state_set(&ep->com, DEAD);
- if (ep->com.tdev->type == T3B)
+ if (ep->com.tdev->type == T3B && act_open_has_tid(rpl->status))
release_tid(ep->com.tdev, GET_TID(rpl), NULL);
cxgb3_free_atid(ep->com.tdev, ep->atid);
dst_release(ep->dst);

2007-06-27 15:53:29

by Steve Wise

[permalink] [raw]
Subject: [PATCH 6/6] iw_cxgb3: Don't abort after failures sending the mpa reply.


iw_cxgb3: Don't abort after failures sending the mpa reply.

This bug results in an abort request being sent down _after_ the tid
has been released. If the tid happens to have been reused, then the
subsequent generation of the tid gets incorrectly aborted.

The thread running iwch_accecpt_cr() must not abort a connection if an
error is returned after being awakened. If any errors did occur while
iwch_accept_cr() is blocked, then the connection has already been aborted
on the thread processing the error.

Signed-off-by: Steve Wise <[email protected]>
---

drivers/infiniband/hw/cxgb3/iwch_cm.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/infiniband/hw/cxgb3/iwch_cm.c b/drivers/infiniband/hw/cxgb3/iwch_cm.c
index 228721f..3b41dc0 100644
--- a/drivers/infiniband/hw/cxgb3/iwch_cm.c
+++ b/drivers/infiniband/hw/cxgb3/iwch_cm.c
@@ -1781,7 +1781,6 @@ err:
ep->com.cm_id = NULL;
ep->com.qp = NULL;
cm_id->rem_ref(cm_id);
- abort_connection(ep, NULL, GFP_KERNEL);
put_ep(&ep->com);
return err;
}

2007-06-27 19:15:07

by Roland Dreier

[permalink] [raw]
Subject: Re: [PATCH 0/6] iw_cxgb3: Bug Fixes for 2.6.23

> Here are some bug fixes to the iw_cxgb3 driver that I'd like included
> for 2.6.23. NOTE: Patch 1 requires a firmware interface change, so
> there is a version bump to 4.3 included in that patch that hits cxgb3.
> This will likely conflict with a previous version change that is in
> Jeff's upstream branch. The net is: we need the firmware version bumped
> to 4.3 with these iw_cxgb3 changes.

OK, I'll probably pull this into my tree and hold off on asking Linus
to pull until after he pulls Jeff's net driver tree. Once that
happens I'll fix up any conflicts and ask Linus to pull.

2007-06-27 19:32:09

by Steve Wise

[permalink] [raw]
Subject: Re: [PATCH 0/6] iw_cxgb3: Bug Fixes for 2.6.23

Roland Dreier wrote:
> > Here are some bug fixes to the iw_cxgb3 driver that I'd like included
> > for 2.6.23. NOTE: Patch 1 requires a firmware interface change, so
> > there is a version bump to 4.3 included in that patch that hits cxgb3.
> > This will likely conflict with a previous version change that is in
> > Jeff's upstream branch. The net is: we need the firmware version bumped
> > to 4.3 with these iw_cxgb3 changes.
>
> OK, I'll probably pull this into my tree and hold off on asking Linus
> to pull until after he pulls Jeff's net driver tree. Once that
> happens I'll fix up any conflicts and ask Linus to pull.

Sounds good.

Thanks,

Steve.