On 4/15/21 7:04 PM, Mike Christie wrote:
> During ep_disconnect we have been doing iscsi_suspend_tx/queue to block
> new IO but every driver except cxgbi and iscsi_tcp can still get IO from
> __iscsi_conn_send_pdu if we haven't called iscsi_conn_failure before
> ep_disconnect. This could happen if we were terminating the session, and
> the logout timedout before it was even sent to libiscsi.
>
> This patch fixes the issue by adding a helper which reverses the bind_conn
> call that allows new IO to be queued. Drivers implementing ep_disconnect
> can use this to make sure new IO is not queued to them when handling the
> disconnect.
>
> Signed-off-by: Mike Christie <michael.christie@xxxxxxxxxx>
> ---
> drivers/infiniband/ulp/iser/iscsi_iser.c | 1 +
> drivers/scsi/be2iscsi/be_main.c | 1 +
> drivers/scsi/bnx2i/bnx2i_iscsi.c | 1 +
> drivers/scsi/cxgbi/cxgb3i/cxgb3i.c | 1 +
> drivers/scsi/cxgbi/cxgb4i/cxgb4i.c | 1 +
> drivers/scsi/libiscsi.c | 61 +++++++++++++++++++++---
> drivers/scsi/qedi/qedi_iscsi.c | 1 +
> drivers/scsi/qla4xxx/ql4_os.c | 1 +
> drivers/scsi/scsi_transport_iscsi.c | 3 ++
> include/scsi/libiscsi.h | 1 +
> include/scsi/scsi_transport_iscsi.h | 1 +
> 11 files changed, 67 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c b/drivers/infiniband/ulp/iser/iscsi_iser.c
> index 8fcaa1136f2c..6baebcb6d14d 100644
> --- a/drivers/infiniband/ulp/iser/iscsi_iser.c
> +++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
> @@ -1002,6 +1002,7 @@ static struct iscsi_transport iscsi_iser_transport = {
> /* connection management */
> .create_conn = iscsi_iser_conn_create,
> .bind_conn = iscsi_iser_conn_bind,
> + .unbind_conn = iscsi_conn_unbind,
> .destroy_conn = iscsi_conn_teardown,
> .attr_is_visible = iser_attr_is_visible,
> .set_param = iscsi_iser_set_param,
> diff --git a/drivers/scsi/be2iscsi/be_main.c b/drivers/scsi/be2iscsi/be_main.c
> index 90fcddb76f46..e9658a67d9da 100644
> --- a/drivers/scsi/be2iscsi/be_main.c
> +++ b/drivers/scsi/be2iscsi/be_main.c
> @@ -5809,6 +5809,7 @@ struct iscsi_transport beiscsi_iscsi_transport = {
> .destroy_session = beiscsi_session_destroy,
> .create_conn = beiscsi_conn_create,
> .bind_conn = beiscsi_conn_bind,
> + .unbind_conn = iscsi_conn_unbind,
> .destroy_conn = iscsi_conn_teardown,
> .attr_is_visible = beiscsi_attr_is_visible,
> .set_iface_param = beiscsi_iface_set_param,
> diff --git a/drivers/scsi/bnx2i/bnx2i_iscsi.c b/drivers/scsi/bnx2i/bnx2i_iscsi.c
> index 1e6d8f62ea3c..b6c1da46d582 100644
> --- a/drivers/scsi/bnx2i/bnx2i_iscsi.c
> +++ b/drivers/scsi/bnx2i/bnx2i_iscsi.c
> @@ -2276,6 +2276,7 @@ struct iscsi_transport bnx2i_iscsi_transport = {
> .destroy_session = bnx2i_session_destroy,
> .create_conn = bnx2i_conn_create,
> .bind_conn = bnx2i_conn_bind,
> + .unbind_conn = iscsi_conn_unbind,
> .destroy_conn = bnx2i_conn_destroy,
> .attr_is_visible = bnx2i_attr_is_visible,
> .set_param = iscsi_set_param,
> diff --git a/drivers/scsi/cxgbi/cxgb3i/cxgb3i.c b/drivers/scsi/cxgbi/cxgb3i/cxgb3i.c
> index 37d99357120f..edcd3fab6973 100644
> --- a/drivers/scsi/cxgbi/cxgb3i/cxgb3i.c
> +++ b/drivers/scsi/cxgbi/cxgb3i/cxgb3i.c
> @@ -117,6 +117,7 @@ static struct iscsi_transport cxgb3i_iscsi_transport = {
> /* connection management */
> .create_conn = cxgbi_create_conn,
> .bind_conn = cxgbi_bind_conn,
> + .unbind_conn = iscsi_conn_unbind,
> .destroy_conn = iscsi_tcp_conn_teardown,
> .start_conn = iscsi_conn_start,
> .stop_conn = iscsi_conn_stop,
> diff --git a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
> index 2c3491528d42..efb3e2b3398e 100644
> --- a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
> +++ b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
> @@ -134,6 +134,7 @@ static struct iscsi_transport cxgb4i_iscsi_transport = {
> /* connection management */
> .create_conn = cxgbi_create_conn,
> .bind_conn = cxgbi_bind_conn,
> + .unbind_conn = iscsi_conn_unbind,
> .destroy_conn = iscsi_tcp_conn_teardown,
> .start_conn = iscsi_conn_start,
> .stop_conn = iscsi_conn_stop,
> diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
> index aa5ceaffc697..ce3898fdb10f 100644
> --- a/drivers/scsi/libiscsi.c
> +++ b/drivers/scsi/libiscsi.c
> @@ -1387,22 +1387,28 @@ void iscsi_session_failure(struct iscsi_session *session,
> }
> EXPORT_SYMBOL_GPL(iscsi_session_failure);
>
> -void iscsi_conn_failure(struct iscsi_conn *conn, enum iscsi_err err)
> +static void iscsi_set_conn_failed(struct iscsi_conn *conn)
> {
> struct iscsi_session *session = conn->session;
>
> - spin_lock_bh(&session->frwd_lock);
> - if (session->state == ISCSI_STATE_FAILED) {
> - spin_unlock_bh(&session->frwd_lock);
> + if (session->state == ISCSI_STATE_FAILED)
> return;
> - }
>
> if (conn->stop_stage == 0)
> session->state = ISCSI_STATE_FAILED;
> - spin_unlock_bh(&session->frwd_lock);
>
> set_bit(ISCSI_SUSPEND_BIT, &conn->suspend_tx);
> set_bit(ISCSI_SUSPEND_BIT, &conn->suspend_rx);
> +}
> +
> +void iscsi_conn_failure(struct iscsi_conn *conn, enum iscsi_err err)
> +{
> + struct iscsi_session *session = conn->session;
> +
> + spin_lock_bh(&session->frwd_lock);
> + iscsi_set_conn_failed(conn);
> + spin_unlock_bh(&session->frwd_lock);
> +
> iscsi_conn_error_event(conn->cls_conn, err);
> }
> EXPORT_SYMBOL_GPL(iscsi_conn_failure);
> @@ -2220,6 +2226,49 @@ static void iscsi_check_transport_timeouts(struct timer_list *t)
> spin_unlock(&session->frwd_lock);
> }
>
> +/*
> + * iscsi_conn_unbind - prevent queueing to conn.
> + * @conn: iscsi conn ep is bound to.
> + *
> + * This must be called by drivers implementing the ep_disconnect callout.
> + * It disables queueing to the connection from libiscsi in preparation for
> + * an ep_disconnect call.
> + */
> +void iscsi_conn_unbind(struct iscsi_cls_conn *cls_conn)
> +{
> + struct iscsi_session *session;
> + struct iscsi_conn *conn;
> +
> + if (!cls_conn)
> + return;
> +
> + conn = cls_conn->dd_data;
> + session = conn->session;
> + /*
> + * Wait for iscsi_eh calls to exit. We don't wait for the tmf to
> + * complete or timeout. The caller just wants to know what's running
> + * is everything that needs to be cleaned up, and no cmds will be
> + * queued.
> + */
> + mutex_lock(&session->eh_mutex);
> +
> + iscsi_suspend_queue(conn);
> + iscsi_suspend_tx(conn);
> +
> + spin_lock_bh(&session->frwd_lock);
> + /*
> + * if logout timed out before userspace could even send a PDU the
> + * state might still be in ISCSI_STATE_LOGGED_IN and allowing new cmds
> + * and TMFs.
> + */
> + if (session->state == ISCSI_STATE_LOGGED_IN)
> + iscsi_set_conn_failed(conn);
> +
> + spin_unlock_bh(&session->frwd_lock);
> + mutex_unlock(&session->eh_mutex);
> +}
> +EXPORT_SYMBOL_GPL(iscsi_conn_unbind);
> +
> static void iscsi_prep_abort_task_pdu(struct iscsi_task *task,
> struct iscsi_tm *hdr)
> {
> diff --git a/drivers/scsi/qedi/qedi_iscsi.c b/drivers/scsi/qedi/qedi_iscsi.c
> index 08c05403cd72..ef16537c523c 100644
> --- a/drivers/scsi/qedi/qedi_iscsi.c
> +++ b/drivers/scsi/qedi/qedi_iscsi.c
> @@ -1401,6 +1401,7 @@ struct iscsi_transport qedi_iscsi_transport = {
> .destroy_session = qedi_session_destroy,
> .create_conn = qedi_conn_create,
> .bind_conn = qedi_conn_bind,
> + .unbind_conn = iscsi_conn_unbind,
> .start_conn = qedi_conn_start,
> .stop_conn = iscsi_conn_stop,
> .destroy_conn = qedi_conn_destroy,
> diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
> index 7bd9a4a04ad5..ff663cb330c2 100644
> --- a/drivers/scsi/qla4xxx/ql4_os.c
> +++ b/drivers/scsi/qla4xxx/ql4_os.c
> @@ -259,6 +259,7 @@ static struct iscsi_transport qla4xxx_iscsi_transport = {
> .start_conn = qla4xxx_conn_start,
> .create_conn = qla4xxx_conn_create,
> .bind_conn = qla4xxx_conn_bind,
> + .unbind_conn = iscsi_conn_unbind,
> .stop_conn = iscsi_conn_stop,
> .destroy_conn = qla4xxx_conn_destroy,
> .set_param = iscsi_set_param,
> diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c
> index 441f0152193f..833114c8e197 100644
> --- a/drivers/scsi/scsi_transport_iscsi.c
> +++ b/drivers/scsi/scsi_transport_iscsi.c
> @@ -2981,6 +2981,8 @@ static int iscsi_if_ep_disconnect(struct iscsi_transport *transport,
> conn->ep = NULL;
> mutex_unlock(&conn->ep_mutex);
> conn->state = ISCSI_CONN_FAILED;
> +
> + transport->unbind_conn(conn);
> }
>
> transport->ep_disconnect(ep);
> @@ -4656,6 +4658,7 @@ iscsi_register_transport(struct iscsi_transport *tt)
> int err;
>
> BUG_ON(!tt);
> + WARN_ON(tt->ep_disconnect && !tt->unbind_conn);
>
> priv = iscsi_if_transport_lookup(tt);
> if (priv)
> diff --git a/include/scsi/libiscsi.h b/include/scsi/libiscsi.h
> index 8c6d358a8abc..ec6d508e7a4a 100644
> --- a/include/scsi/libiscsi.h
> +++ b/include/scsi/libiscsi.h
> @@ -431,6 +431,7 @@ extern int iscsi_conn_start(struct iscsi_cls_conn *);
> extern void iscsi_conn_stop(struct iscsi_cls_conn *, int);
> extern int iscsi_conn_bind(struct iscsi_cls_session *, struct iscsi_cls_conn *,
> int);
> +extern void iscsi_conn_unbind(struct iscsi_cls_conn *cls_conn);
> extern void iscsi_conn_failure(struct iscsi_conn *conn, enum iscsi_err err);
> extern void iscsi_session_failure(struct iscsi_session *session,
> enum iscsi_err err);
> diff --git a/include/scsi/scsi_transport_iscsi.h b/include/scsi/scsi_transport_iscsi.h
> index fc5a39839b4b..afc61a23628d 100644
> --- a/include/scsi/scsi_transport_iscsi.h
> +++ b/include/scsi/scsi_transport_iscsi.h
> @@ -82,6 +82,7 @@ struct iscsi_transport {
> void (*destroy_session) (struct iscsi_cls_session *session);
> struct iscsi_cls_conn *(*create_conn) (struct iscsi_cls_session *sess,
> uint32_t cid);
> + void (*unbind_conn) (struct iscsi_cls_conn *conn);
> int (*bind_conn) (struct iscsi_cls_session *session,
> struct iscsi_cls_conn *cls_conn,
> uint64_t transport_eph, int is_leading);
>
Reviewed-by: Lee Duncan <lduncan@xxxxxxxx>
