Commit bd379220 authored by Jenny Derzhavetz's avatar Jenny Derzhavetz Committed by Nicholas Bellinger

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: (ce9a9fc2 "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: default avatarJenny Derzhavetz <jennyf@mellanox.com>
Signed-off-by: default avatarSagi Grimberg <sagig@mellanox.com>
Cc: <stable@vger.kernel.org> # v3.10+
Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
parent ed8cb0a4
...@@ -634,7 +634,7 @@ static void ...@@ -634,7 +634,7 @@ static void
isert_init_conn(struct isert_conn *isert_conn) isert_init_conn(struct isert_conn *isert_conn)
{ {
isert_conn->state = ISER_CONN_INIT; isert_conn->state = ISER_CONN_INIT;
INIT_LIST_HEAD(&isert_conn->accept_node); INIT_LIST_HEAD(&isert_conn->node);
init_completion(&isert_conn->login_comp); init_completion(&isert_conn->login_comp);
init_completion(&isert_conn->login_req_comp); init_completion(&isert_conn->login_req_comp);
init_completion(&isert_conn->wait); init_completion(&isert_conn->wait);
...@@ -762,28 +762,15 @@ isert_connect_request(struct rdma_cm_id *cma_id, struct rdma_cm_event *event) ...@@ -762,28 +762,15 @@ isert_connect_request(struct rdma_cm_id *cma_id, struct rdma_cm_event *event)
ret = isert_rdma_post_recvl(isert_conn); ret = isert_rdma_post_recvl(isert_conn);
if (ret) if (ret)
goto out_conn_dev; goto out_conn_dev;
/*
* Obtain the second reference now before isert_rdma_accept() to
* ensure that any initiator generated REJECT CM event that occurs
* asynchronously won't drop the last reference until the error path
* in iscsi_target_login_sess_out() does it's ->iscsit_free_conn() ->
* isert_free_conn() -> isert_put_conn() -> kref_put().
*/
if (!kref_get_unless_zero(&isert_conn->kref)) {
isert_warn("conn %p connect_release is running\n", isert_conn);
goto out_conn_dev;
}
ret = isert_rdma_accept(isert_conn); ret = isert_rdma_accept(isert_conn);
if (ret) if (ret)
goto out_conn_dev; goto out_conn_dev;
mutex_lock(&isert_np->mutex); mutex_lock(&isert_np->mutex);
list_add_tail(&isert_conn->accept_node, &isert_np->accept_list); list_add_tail(&isert_conn->node, &isert_np->accepted);
mutex_unlock(&isert_np->mutex); mutex_unlock(&isert_np->mutex);
isert_info("np %p: Allow accept_np to continue\n", np);
up(&isert_np->sem);
return 0; return 0;
out_conn_dev: out_conn_dev:
...@@ -831,13 +818,21 @@ static void ...@@ -831,13 +818,21 @@ static void
isert_connected_handler(struct rdma_cm_id *cma_id) isert_connected_handler(struct rdma_cm_id *cma_id)
{ {
struct isert_conn *isert_conn = cma_id->qp->qp_context; struct isert_conn *isert_conn = cma_id->qp->qp_context;
struct isert_np *isert_np = cma_id->context;
isert_info("conn %p\n", isert_conn); isert_info("conn %p\n", isert_conn);
mutex_lock(&isert_conn->mutex); mutex_lock(&isert_conn->mutex);
if (isert_conn->state != ISER_CONN_FULL_FEATURE)
isert_conn->state = ISER_CONN_UP; isert_conn->state = ISER_CONN_UP;
kref_get(&isert_conn->kref);
mutex_unlock(&isert_conn->mutex); mutex_unlock(&isert_conn->mutex);
mutex_lock(&isert_np->mutex);
list_move_tail(&isert_conn->node, &isert_np->pending);
mutex_unlock(&isert_np->mutex);
isert_info("np %p: Allow accept_np to continue\n", isert_np);
up(&isert_np->sem);
} }
static void static void
...@@ -946,8 +941,8 @@ isert_disconnected_handler(struct rdma_cm_id *cma_id, ...@@ -946,8 +941,8 @@ isert_disconnected_handler(struct rdma_cm_id *cma_id,
goto out; goto out;
mutex_lock(&isert_np->mutex); mutex_lock(&isert_np->mutex);
if (!list_empty(&isert_conn->accept_node)) { if (!list_empty(&isert_conn->node)) {
list_del_init(&isert_conn->accept_node); list_del_init(&isert_conn->node);
isert_put_conn(isert_conn); isert_put_conn(isert_conn);
queue_work(isert_release_wq, &isert_conn->release_work); queue_work(isert_release_wq, &isert_conn->release_work);
} }
...@@ -962,6 +957,7 @@ isert_connect_error(struct rdma_cm_id *cma_id) ...@@ -962,6 +957,7 @@ isert_connect_error(struct rdma_cm_id *cma_id)
{ {
struct isert_conn *isert_conn = cma_id->qp->qp_context; struct isert_conn *isert_conn = cma_id->qp->qp_context;
list_del_init(&isert_conn->node);
isert_conn->cm_id = NULL; isert_conn->cm_id = NULL;
isert_put_conn(isert_conn); isert_put_conn(isert_conn);
...@@ -3115,7 +3111,8 @@ isert_setup_np(struct iscsi_np *np, ...@@ -3115,7 +3111,8 @@ isert_setup_np(struct iscsi_np *np,
} }
sema_init(&isert_np->sem, 0); sema_init(&isert_np->sem, 0);
mutex_init(&isert_np->mutex); mutex_init(&isert_np->mutex);
INIT_LIST_HEAD(&isert_np->accept_list); INIT_LIST_HEAD(&isert_np->accepted);
INIT_LIST_HEAD(&isert_np->pending);
isert_np->np = np; isert_np->np = np;
/* /*
...@@ -3238,13 +3235,13 @@ isert_accept_np(struct iscsi_np *np, struct iscsi_conn *conn) ...@@ -3238,13 +3235,13 @@ isert_accept_np(struct iscsi_np *np, struct iscsi_conn *conn)
spin_unlock_bh(&np->np_thread_lock); spin_unlock_bh(&np->np_thread_lock);
mutex_lock(&isert_np->mutex); mutex_lock(&isert_np->mutex);
if (list_empty(&isert_np->accept_list)) { if (list_empty(&isert_np->pending)) {
mutex_unlock(&isert_np->mutex); mutex_unlock(&isert_np->mutex);
goto accept_wait; goto accept_wait;
} }
isert_conn = list_first_entry(&isert_np->accept_list, isert_conn = list_first_entry(&isert_np->pending,
struct isert_conn, accept_node); struct isert_conn, node);
list_del_init(&isert_conn->accept_node); list_del_init(&isert_conn->node);
mutex_unlock(&isert_np->mutex); mutex_unlock(&isert_np->mutex);
conn->context = isert_conn; conn->context = isert_conn;
...@@ -3271,14 +3268,25 @@ isert_free_np(struct iscsi_np *np) ...@@ -3271,14 +3268,25 @@ isert_free_np(struct iscsi_np *np)
* that at this point we don't have hanging connections that * that at this point we don't have hanging connections that
* completed RDMA establishment but didn't start iscsi login * completed RDMA establishment but didn't start iscsi login
* process. So work-around this by cleaning up what ever piled * process. So work-around this by cleaning up what ever piled
* up in accept_list. * up in accepted and pending lists.
*/ */
mutex_lock(&isert_np->mutex); mutex_lock(&isert_np->mutex);
if (!list_empty(&isert_np->accept_list)) { if (!list_empty(&isert_np->pending)) {
isert_info("Still have isert connections, cleaning up...\n"); isert_info("Still have isert pending connections\n");
list_for_each_entry_safe(isert_conn, n,
&isert_np->pending,
node) {
isert_info("cleaning isert_conn %p state (%d)\n",
isert_conn, isert_conn->state);
isert_connect_release(isert_conn);
}
}
if (!list_empty(&isert_np->accepted)) {
isert_info("Still have isert accepted connections\n");
list_for_each_entry_safe(isert_conn, n, list_for_each_entry_safe(isert_conn, n,
&isert_np->accept_list, &isert_np->accepted,
accept_node) { node) {
isert_info("cleaning isert_conn %p state (%d)\n", isert_info("cleaning isert_conn %p state (%d)\n",
isert_conn, isert_conn->state); isert_conn, isert_conn->state);
isert_connect_release(isert_conn); isert_connect_release(isert_conn);
......
...@@ -159,7 +159,7 @@ struct isert_conn { ...@@ -159,7 +159,7 @@ struct isert_conn {
struct iser_rx_desc *rx_descs; struct iser_rx_desc *rx_descs;
struct ib_recv_wr rx_wr[ISERT_MIN_POSTED_RX]; struct ib_recv_wr rx_wr[ISERT_MIN_POSTED_RX];
struct iscsi_conn *conn; struct iscsi_conn *conn;
struct list_head accept_node; struct list_head node;
struct completion login_comp; struct completion login_comp;
struct completion login_req_comp; struct completion login_req_comp;
struct iser_tx_desc login_tx_desc; struct iser_tx_desc login_tx_desc;
...@@ -221,5 +221,6 @@ struct isert_np { ...@@ -221,5 +221,6 @@ struct isert_np {
struct semaphore sem; struct semaphore sem;
struct rdma_cm_id *cm_id; struct rdma_cm_id *cm_id;
struct mutex mutex; struct mutex mutex;
struct list_head accept_list; struct list_head accepted;
struct list_head pending;
}; };
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment