Skip to content

Commit bd37922

Browse files
jennyj-mellanoxNicholas Bellinger
authored andcommitted
iser-target: Fix pending connections handling in target stack shutdown sequnce
Instead of handing a connection to the iscsi stack for processing right after accepting (rdma_accept) we only hand the connection to the iscsi core after we reached to a connected state (ESTABLISHED CM event). This will prevent two error scenrios: 1. race between rdma connection teardown and iscsi login sequence reported by Nic in: (ce9a9fc "iser-target: Fix REJECT CM event use-after-free OOPs") 2. target stack shutdown sequence race with constant login attempts by multiple initiators. We address this by maintaining two queues at the isert_np level: - accepted: connections that were accepted but have not reached connected state (might get rejected, unreachable or error). - pending: connections in connected state, but have yet to handed to the iscsi core for login processing. iser connections are promoted to the pending queue only from the accepted queue. This way the iscsi core now will only handle functional iser connections and once we shutdown the target stack, we look for any stales that got left behind so we can safely release them. Signed-off-by: Jenny Derzhavetz <[email protected]> Signed-off-by: Sagi Grimberg <[email protected]> Cc: <[email protected]> # v3.10+ Signed-off-by: Nicholas Bellinger <[email protected]>
1 parent ed8cb0a commit bd37922

File tree

2 files changed

+40
-31
lines changed

2 files changed

+40
-31
lines changed

drivers/infiniband/ulp/isert/ib_isert.c

Lines changed: 37 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -634,7 +634,7 @@ static void
634634
isert_init_conn(struct isert_conn *isert_conn)
635635
{
636636
isert_conn->state = ISER_CONN_INIT;
637-
INIT_LIST_HEAD(&isert_conn->accept_node);
637+
INIT_LIST_HEAD(&isert_conn->node);
638638
init_completion(&isert_conn->login_comp);
639639
init_completion(&isert_conn->login_req_comp);
640640
init_completion(&isert_conn->wait);
@@ -762,28 +762,15 @@ isert_connect_request(struct rdma_cm_id *cma_id, struct rdma_cm_event *event)
762762
ret = isert_rdma_post_recvl(isert_conn);
763763
if (ret)
764764
goto out_conn_dev;
765-
/*
766-
* Obtain the second reference now before isert_rdma_accept() to
767-
* ensure that any initiator generated REJECT CM event that occurs
768-
* asynchronously won't drop the last reference until the error path
769-
* in iscsi_target_login_sess_out() does it's ->iscsit_free_conn() ->
770-
* isert_free_conn() -> isert_put_conn() -> kref_put().
771-
*/
772-
if (!kref_get_unless_zero(&isert_conn->kref)) {
773-
isert_warn("conn %p connect_release is running\n", isert_conn);
774-
goto out_conn_dev;
775-
}
776765

777766
ret = isert_rdma_accept(isert_conn);
778767
if (ret)
779768
goto out_conn_dev;
780769

781770
mutex_lock(&isert_np->mutex);
782-
list_add_tail(&isert_conn->accept_node, &isert_np->accept_list);
771+
list_add_tail(&isert_conn->node, &isert_np->accepted);
783772
mutex_unlock(&isert_np->mutex);
784773

785-
isert_info("np %p: Allow accept_np to continue\n", np);
786-
up(&isert_np->sem);
787774
return 0;
788775

789776
out_conn_dev:
@@ -831,13 +818,21 @@ static void
831818
isert_connected_handler(struct rdma_cm_id *cma_id)
832819
{
833820
struct isert_conn *isert_conn = cma_id->qp->qp_context;
821+
struct isert_np *isert_np = cma_id->context;
834822

835823
isert_info("conn %p\n", isert_conn);
836824

837825
mutex_lock(&isert_conn->mutex);
838-
if (isert_conn->state != ISER_CONN_FULL_FEATURE)
839-
isert_conn->state = ISER_CONN_UP;
826+
isert_conn->state = ISER_CONN_UP;
827+
kref_get(&isert_conn->kref);
840828
mutex_unlock(&isert_conn->mutex);
829+
830+
mutex_lock(&isert_np->mutex);
831+
list_move_tail(&isert_conn->node, &isert_np->pending);
832+
mutex_unlock(&isert_np->mutex);
833+
834+
isert_info("np %p: Allow accept_np to continue\n", isert_np);
835+
up(&isert_np->sem);
841836
}
842837

843838
static void
@@ -946,8 +941,8 @@ isert_disconnected_handler(struct rdma_cm_id *cma_id,
946941
goto out;
947942

948943
mutex_lock(&isert_np->mutex);
949-
if (!list_empty(&isert_conn->accept_node)) {
950-
list_del_init(&isert_conn->accept_node);
944+
if (!list_empty(&isert_conn->node)) {
945+
list_del_init(&isert_conn->node);
951946
isert_put_conn(isert_conn);
952947
queue_work(isert_release_wq, &isert_conn->release_work);
953948
}
@@ -962,6 +957,7 @@ isert_connect_error(struct rdma_cm_id *cma_id)
962957
{
963958
struct isert_conn *isert_conn = cma_id->qp->qp_context;
964959

960+
list_del_init(&isert_conn->node);
965961
isert_conn->cm_id = NULL;
966962
isert_put_conn(isert_conn);
967963

@@ -3115,7 +3111,8 @@ isert_setup_np(struct iscsi_np *np,
31153111
}
31163112
sema_init(&isert_np->sem, 0);
31173113
mutex_init(&isert_np->mutex);
3118-
INIT_LIST_HEAD(&isert_np->accept_list);
3114+
INIT_LIST_HEAD(&isert_np->accepted);
3115+
INIT_LIST_HEAD(&isert_np->pending);
31193116
isert_np->np = np;
31203117

31213118
/*
@@ -3238,13 +3235,13 @@ isert_accept_np(struct iscsi_np *np, struct iscsi_conn *conn)
32383235
spin_unlock_bh(&np->np_thread_lock);
32393236

32403237
mutex_lock(&isert_np->mutex);
3241-
if (list_empty(&isert_np->accept_list)) {
3238+
if (list_empty(&isert_np->pending)) {
32423239
mutex_unlock(&isert_np->mutex);
32433240
goto accept_wait;
32443241
}
3245-
isert_conn = list_first_entry(&isert_np->accept_list,
3246-
struct isert_conn, accept_node);
3247-
list_del_init(&isert_conn->accept_node);
3242+
isert_conn = list_first_entry(&isert_np->pending,
3243+
struct isert_conn, node);
3244+
list_del_init(&isert_conn->node);
32483245
mutex_unlock(&isert_np->mutex);
32493246

32503247
conn->context = isert_conn;
@@ -3271,14 +3268,25 @@ isert_free_np(struct iscsi_np *np)
32713268
* that at this point we don't have hanging connections that
32723269
* completed RDMA establishment but didn't start iscsi login
32733270
* process. So work-around this by cleaning up what ever piled
3274-
* up in accept_list.
3271+
* up in accepted and pending lists.
32753272
*/
32763273
mutex_lock(&isert_np->mutex);
3277-
if (!list_empty(&isert_np->accept_list)) {
3278-
isert_info("Still have isert connections, cleaning up...\n");
3274+
if (!list_empty(&isert_np->pending)) {
3275+
isert_info("Still have isert pending connections\n");
3276+
list_for_each_entry_safe(isert_conn, n,
3277+
&isert_np->pending,
3278+
node) {
3279+
isert_info("cleaning isert_conn %p state (%d)\n",
3280+
isert_conn, isert_conn->state);
3281+
isert_connect_release(isert_conn);
3282+
}
3283+
}
3284+
3285+
if (!list_empty(&isert_np->accepted)) {
3286+
isert_info("Still have isert accepted connections\n");
32793287
list_for_each_entry_safe(isert_conn, n,
3280-
&isert_np->accept_list,
3281-
accept_node) {
3288+
&isert_np->accepted,
3289+
node) {
32823290
isert_info("cleaning isert_conn %p state (%d)\n",
32833291
isert_conn, isert_conn->state);
32843292
isert_connect_release(isert_conn);

drivers/infiniband/ulp/isert/ib_isert.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ struct isert_conn {
159159
struct iser_rx_desc *rx_descs;
160160
struct ib_recv_wr rx_wr[ISERT_MIN_POSTED_RX];
161161
struct iscsi_conn *conn;
162-
struct list_head accept_node;
162+
struct list_head node;
163163
struct completion login_comp;
164164
struct completion login_req_comp;
165165
struct iser_tx_desc login_tx_desc;
@@ -221,5 +221,6 @@ struct isert_np {
221221
struct semaphore sem;
222222
struct rdma_cm_id *cm_id;
223223
struct mutex mutex;
224-
struct list_head accept_list;
224+
struct list_head accepted;
225+
struct list_head pending;
225226
};

0 commit comments

Comments
 (0)