Commit a115177b authored by Shuhei Matsumoto's avatar Shuhei Matsumoto Committed by Jim Harris
Browse files

iscsi: Clear only R2T tasks prior to task management request 



As described in RFC3720, task management request will act on all
the commands from the same session having a CmdSN lower than the
task management CmdSN.

Current implementation clears all R2T tasks without checking CmdSN.

This patch changes the implementation to clear only R2T tasks
whose CmdSN is smaller than the task management request.
Additionally this patch adds to UT code to verify the change works
as expected.

Change-Id: I0b368cb13741bc05259924d27793438e9250b951
Signed-off-by: default avatarShuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/434761


Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
Reviewed-by: default avatarChangpeng Liu <changpeng.liu@intel.com>
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
parent f550dbe8

lib/iscsi/conn.c

+2 −2
Original line number Diff line number Diff line
@@ -529,7 +529,7 @@ _spdk_iscsi_conn_destruct(struct spdk_iscsi_conn *conn) 
{

	int rc;



	spdk_clear_all_transfer_task(conn, NULL);

	spdk_clear_all_transfer_task(conn, NULL, NULL);

	spdk_iscsi_poll_group_remove_conn_sock(conn);

	spdk_sock_close(&conn->sock);

	spdk_poller_unregister(&conn->logout_timer);
@@ -668,7 +668,7 @@ _iscsi_conn_remove_lun(void *arg1, void *arg2) 
		return;

	}



	spdk_clear_all_transfer_task(conn, lun);

	spdk_clear_all_transfer_task(conn, lun, NULL);

	TAILQ_FOREACH_SAFE(pdu, &conn->write_pdu_list, tailq, tmp_pdu) {

		if (pdu->task && (lun == pdu->task->scsi.lun)) {

			TAILQ_REMOVE(&conn->write_pdu_list, pdu, tailq);

lib/iscsi/iscsi.c

+17 −8
Original line number Diff line number Diff line
@@ -2991,11 +2991,11 @@ spdk_abort_transfer_task_in_task_mgmt_resp(struct spdk_iscsi_conn *conn, 


	/* abort all tasks issued via this session on the LUN */

	case ISCSI_TASK_FUNC_ABORT_TASK_SET:

		spdk_clear_all_transfer_task(conn, task->scsi.lun);

		spdk_clear_all_transfer_task(conn, task->scsi.lun, pdu);

		break;



	case ISCSI_TASK_FUNC_LOGICAL_UNIT_RESET:

		spdk_clear_all_transfer_task(conn, task->scsi.lun);

		spdk_clear_all_transfer_task(conn, task->scsi.lun, pdu);

		break;

	}

}
@@ -3523,9 +3523,12 @@ void spdk_del_transfer_task(struct spdk_iscsi_conn *conn, uint32_t task_tag) 


static void

spdk_del_connection_queued_task(struct spdk_iscsi_conn *conn, void *tailq,

				struct spdk_scsi_lun *lun)

				struct spdk_scsi_lun *lun,

				struct spdk_iscsi_pdu *pdu)

{

	struct spdk_iscsi_task *task, *task_tmp;

	struct spdk_iscsi_pdu *pdu_tmp;



	/*

	 * Temporary used to index spdk_scsi_task related

	 *  queues of the connection.
@@ -3534,7 +3537,9 @@ spdk_del_connection_queued_task(struct spdk_iscsi_conn *conn, void *tailq, 
	head = (struct queued_tasks *)tailq;



	TAILQ_FOREACH_SAFE(task, head, link, task_tmp) {

		if (lun == NULL || lun == task->scsi.lun) {

		pdu_tmp = spdk_iscsi_task_get_pdu(task);

		if ((lun == NULL || lun == task->scsi.lun) &&

		    (pdu == NULL || SN32_LT(pdu_tmp->cmd_sn, pdu->cmd_sn))) {

			TAILQ_REMOVE(head, task, link);

			if (lun != NULL && spdk_scsi_lun_is_removing(lun)) {

				spdk_scsi_task_process_null_lun(&task->scsi);
@@ -3546,15 +3551,19 @@ spdk_del_connection_queued_task(struct spdk_iscsi_conn *conn, void *tailq, 
}



void spdk_clear_all_transfer_task(struct spdk_iscsi_conn *conn,

				  struct spdk_scsi_lun *lun)

				  struct spdk_scsi_lun *lun,

				  struct spdk_iscsi_pdu *pdu)

{

	int i, j, pending_r2t;

	struct spdk_iscsi_task *task;

	struct spdk_iscsi_pdu *pdu_tmp;



	pending_r2t = conn->pending_r2t;

	for (i = 0; i < pending_r2t; i++) {

		task = conn->outstanding_r2t_tasks[i];

		if (lun == NULL || lun == task->scsi.lun) {

		pdu_tmp = spdk_iscsi_task_get_pdu(task);

		if ((lun == NULL || lun == task->scsi.lun) &&

		    (pdu == NULL || SN32_LT(pdu_tmp->cmd_sn, pdu->cmd_sn))) {

			conn->outstanding_r2t_tasks[i] = NULL;

			task->outstanding_r2t = 0;

			task->next_r2t_offset = 0;
@@ -3577,8 +3586,8 @@ void spdk_clear_all_transfer_task(struct spdk_iscsi_conn *conn, 
		}

	}



	spdk_del_connection_queued_task(conn, &conn->active_r2t_tasks, lun);

	spdk_del_connection_queued_task(conn, &conn->queued_r2t_tasks, lun);

	spdk_del_connection_queued_task(conn, &conn->active_r2t_tasks, lun, pdu);

	spdk_del_connection_queued_task(conn, &conn->queued_r2t_tasks, lun, pdu);



	spdk_start_queued_transfer_tasks(conn);

}

lib/iscsi/iscsi.h

+2 −1
Original line number Diff line number Diff line
@@ -425,7 +425,8 @@ int spdk_iscsi_sess_params_init(struct iscsi_param **params); 


void spdk_free_sess(struct spdk_iscsi_sess *sess);

void spdk_clear_all_transfer_task(struct spdk_iscsi_conn *conn,

				  struct spdk_scsi_lun *lun);

				  struct spdk_scsi_lun *lun,

				  struct spdk_iscsi_pdu *pdu);

void spdk_del_transfer_task(struct spdk_iscsi_conn *conn, uint32_t CmdSN);

bool spdk_iscsi_is_deferred_free_pdu(struct spdk_iscsi_pdu *pdu);

test/unit/lib/iscsi/conn.c/conn_ut.c

+2 −1
Original line number Diff line number Diff line
@@ -216,7 +216,8 @@ spdk_iscsi_conn_params_init(struct iscsi_param **params) 
}



void spdk_clear_all_transfer_task(struct spdk_iscsi_conn *conn,

				  struct spdk_scsi_lun *lun)

				  struct spdk_scsi_lun *lun,

				  struct spdk_iscsi_pdu *pdu)

{

}

test/unit/lib/iscsi/iscsi.c/iscsi_ut.c

+64 −4
Original line number Diff line number Diff line
@@ -810,9 +810,11 @@ clear_all_transfer_tasks_test(void) 
{

	struct spdk_iscsi_sess sess;

	struct spdk_iscsi_conn conn;

	struct spdk_iscsi_task *task1, *task2, *task3, *task4, *task5;

	struct spdk_iscsi_pdu *pdu1, *pdu2, *pdu3, *pdu4, *pdu5, *pdu;

	struct spdk_iscsi_task *task1, *task2, *task3, *task4, *task5, *task6;

	struct spdk_iscsi_pdu *pdu1, *pdu2, *pdu3, *pdu4, *pdu5, *pdu6, *pdu;

	struct spdk_iscsi_pdu *mgmt_pdu1, *mgmt_pdu2;

	struct spdk_scsi_lun lun1, lun2;

	uint32_t alloc_cmd_sn;

	int rc;



	memset(&sess, 0, sizeof(sess));
@@ -827,12 +829,16 @@ clear_all_transfer_tasks_test(void) 
	TAILQ_INIT(&conn.active_r2t_tasks);

	TAILQ_INIT(&conn.queued_r2t_tasks);



	alloc_cmd_sn = 10;



	task1 = spdk_iscsi_task_get(&conn, NULL, NULL);

	SPDK_CU_ASSERT_FATAL(task1 != NULL);

	pdu1 = spdk_get_pdu();

	SPDK_CU_ASSERT_FATAL(pdu1 != NULL);



	pdu1->data_segment_len = SPDK_ISCSI_MAX_RECV_DATA_SEGMENT_LENGTH;

	pdu1->cmd_sn = alloc_cmd_sn;

	alloc_cmd_sn++;

	task1->scsi.transfer_len = SPDK_ISCSI_MAX_RECV_DATA_SEGMENT_LENGTH;

	task1->scsi.lun = &lun1;

	spdk_iscsi_task_set_pdu(task1, pdu1);
@@ -840,12 +846,20 @@ clear_all_transfer_tasks_test(void) 
	rc = spdk_add_transfer_task(&conn, task1);

	CU_ASSERT(rc == SPDK_SUCCESS);



	mgmt_pdu1 = spdk_get_pdu();

	SPDK_CU_ASSERT_FATAL(mgmt_pdu1 != NULL);



	mgmt_pdu1->cmd_sn = alloc_cmd_sn;

	alloc_cmd_sn++;



	task2 = spdk_iscsi_task_get(&conn, NULL, NULL);

	SPDK_CU_ASSERT_FATAL(task2 != NULL);

	pdu2 = spdk_get_pdu();

	SPDK_CU_ASSERT_FATAL(pdu2 != NULL);



	pdu2->data_segment_len = SPDK_ISCSI_MAX_RECV_DATA_SEGMENT_LENGTH;

	pdu2->cmd_sn = alloc_cmd_sn;

	alloc_cmd_sn++;

	task2->scsi.transfer_len = SPDK_ISCSI_MAX_RECV_DATA_SEGMENT_LENGTH;

	task2->scsi.lun = &lun1;

	spdk_iscsi_task_set_pdu(task2, pdu2);
@@ -859,6 +873,8 @@ clear_all_transfer_tasks_test(void) 
	SPDK_CU_ASSERT_FATAL(pdu3 != NULL);



	pdu3->data_segment_len = SPDK_ISCSI_MAX_RECV_DATA_SEGMENT_LENGTH;

	pdu3->cmd_sn = alloc_cmd_sn;

	alloc_cmd_sn++;

	task3->scsi.transfer_len = SPDK_ISCSI_MAX_RECV_DATA_SEGMENT_LENGTH;

	task3->scsi.lun = &lun1;

	spdk_iscsi_task_set_pdu(task3, pdu3);
@@ -872,6 +888,8 @@ clear_all_transfer_tasks_test(void) 
	SPDK_CU_ASSERT_FATAL(pdu4 != NULL);



	pdu4->data_segment_len = SPDK_ISCSI_MAX_RECV_DATA_SEGMENT_LENGTH;

	pdu4->cmd_sn = alloc_cmd_sn;

	alloc_cmd_sn++;

	task4->scsi.transfer_len = SPDK_ISCSI_MAX_RECV_DATA_SEGMENT_LENGTH;

	task4->scsi.lun = &lun2;

	spdk_iscsi_task_set_pdu(task4, pdu4);
@@ -885,6 +903,8 @@ clear_all_transfer_tasks_test(void) 
	SPDK_CU_ASSERT_FATAL(pdu5 != NULL);



	pdu5->data_segment_len = SPDK_ISCSI_MAX_RECV_DATA_SEGMENT_LENGTH;

	pdu5->cmd_sn = alloc_cmd_sn;

	alloc_cmd_sn++;

	task5->scsi.transfer_len = SPDK_ISCSI_MAX_RECV_DATA_SEGMENT_LENGTH;

	task5->scsi.lun = &lun2;

	spdk_iscsi_task_set_pdu(task5, pdu5);
@@ -892,6 +912,27 @@ clear_all_transfer_tasks_test(void) 
	rc = spdk_add_transfer_task(&conn, task5);

	CU_ASSERT(rc == SPDK_SUCCESS);



	mgmt_pdu2 = spdk_get_pdu();

	SPDK_CU_ASSERT_FATAL(mgmt_pdu2 != NULL);



	mgmt_pdu2->cmd_sn = alloc_cmd_sn;

	alloc_cmd_sn++;



	task6 = spdk_iscsi_task_get(&conn, NULL, NULL);

	SPDK_CU_ASSERT_FATAL(task6 != NULL);

	pdu6 = spdk_get_pdu();

	SPDK_CU_ASSERT_FATAL(pdu6 != NULL);



	pdu6->data_segment_len = SPDK_ISCSI_MAX_RECV_DATA_SEGMENT_LENGTH;

	pdu6->cmd_sn = alloc_cmd_sn;

	alloc_cmd_sn++;

	task5->scsi.transfer_len = SPDK_ISCSI_MAX_RECV_DATA_SEGMENT_LENGTH;

	task6->scsi.lun = &lun2;

	spdk_iscsi_task_set_pdu(task6, pdu6);



	rc = spdk_add_transfer_task(&conn, task6);

	CU_ASSERT(rc == SPDK_SUCCESS);



	CU_ASSERT(conn.ttt == 4);



	CU_ASSERT(spdk_get_transfer_task(&conn, 1) == task1);
@@ -900,7 +941,17 @@ clear_all_transfer_tasks_test(void) 
	CU_ASSERT(spdk_get_transfer_task(&conn, 4) == task4);

	CU_ASSERT(spdk_get_transfer_task(&conn, 5) == NULL);



	spdk_clear_all_transfer_task(&conn, &lun1);

	spdk_clear_all_transfer_task(&conn, &lun1, mgmt_pdu1);



	CU_ASSERT(!TAILQ_EMPTY(&conn.queued_r2t_tasks));

	CU_ASSERT(spdk_get_transfer_task(&conn, 1) == NULL);

	CU_ASSERT(spdk_get_transfer_task(&conn, 2) == task2);

	CU_ASSERT(spdk_get_transfer_task(&conn, 3) == task3);

	CU_ASSERT(spdk_get_transfer_task(&conn, 4) == task4);

	CU_ASSERT(spdk_get_transfer_task(&conn, 5) == task5);

	CU_ASSERT(spdk_get_transfer_task(&conn, 6) == NULL);



	spdk_clear_all_transfer_task(&conn, &lun1, NULL);



	CU_ASSERT(TAILQ_EMPTY(&conn.queued_r2t_tasks));

	CU_ASSERT(spdk_get_transfer_task(&conn, 1) == NULL);
@@ -908,11 +959,17 @@ clear_all_transfer_tasks_test(void) 
	CU_ASSERT(spdk_get_transfer_task(&conn, 3) == NULL);

	CU_ASSERT(spdk_get_transfer_task(&conn, 4) == task4);

	CU_ASSERT(spdk_get_transfer_task(&conn, 5) == task5);

	CU_ASSERT(spdk_get_transfer_task(&conn, 6) == task6);



	spdk_clear_all_transfer_task(&conn, NULL);

	spdk_clear_all_transfer_task(&conn, &lun2, mgmt_pdu2);



	CU_ASSERT(spdk_get_transfer_task(&conn, 4) == NULL);

	CU_ASSERT(spdk_get_transfer_task(&conn, 5) == NULL);

	CU_ASSERT(spdk_get_transfer_task(&conn, 6) == task6);



	spdk_clear_all_transfer_task(&conn, NULL, NULL);



	CU_ASSERT(spdk_get_transfer_task(&conn, 6) == NULL);



	CU_ASSERT(TAILQ_EMPTY(&conn.active_r2t_tasks));

	while (!TAILQ_EMPTY(&g_write_pdu_list)) {
@@ -921,6 +978,9 @@ clear_all_transfer_tasks_test(void) 
		spdk_put_pdu(pdu);

	}



	spdk_put_pdu(mgmt_pdu2);

	spdk_put_pdu(mgmt_pdu1);

	spdk_put_pdu(pdu6);

	spdk_put_pdu(pdu5);

	spdk_put_pdu(pdu4);

	spdk_put_pdu(pdu3);