From 8f9b34dd74687b32b7d5db082587b69deff3697f Mon Sep 17 00:00:00 2001 From: Shuhei Matsumoto Date: Thu, 17 Oct 2019 08:00:56 +0900 Subject: [PATCH] lib/iscsi: Separate PDU header and payload handling Extract PDU header handling from PDU payload handling for all PDU types, and then group them into a new function iscsi_pdu_hdr_handle(). Then the original iscsi_execute() is renamed to iscsi_pdu_payload_handle(). Signed-off-by: Shuhei Matsumoto Change-Id: I1fb1937cfaf502797f2c4edb3aeeb97d4697c7d4 Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/471015 Tested-by: SPDK CI Jenkins Reviewed-by: Ben Walker Reviewed-by: Changpeng Liu Reviewed-by: Jim Harris --- lib/iscsi/iscsi.c | 97 +++++++++++++++++--------- test/unit/lib/iscsi/iscsi.c/iscsi_ut.c | 10 ++- 2 files changed, 72 insertions(+), 35 deletions(-) diff --git a/lib/iscsi/iscsi.c b/lib/iscsi/iscsi.c index f1715b23e..568b48594 100644 --- a/lib/iscsi/iscsi.c +++ b/lib/iscsi/iscsi.c @@ -2201,9 +2201,8 @@ iscsi_op_login(struct spdk_iscsi_conn *conn, struct spdk_iscsi_pdu *pdu) struct iscsi_param *params = NULL; int cid; - rc = iscsi_pdu_hdr_op_login(conn, pdu); - if (rc != 0 || pdu->is_rejected || conn->login_rsp_pdu == NULL) { - return rc; + if (conn->login_rsp_pdu == NULL) { + return 0; } rsp_pdu = conn->login_rsp_pdu; @@ -2322,11 +2321,6 @@ iscsi_op_text(struct spdk_iscsi_conn *conn, struct spdk_iscsi_pdu *pdu) struct iscsi_bhs_text_req *reqh; struct iscsi_bhs_text_resp *rsph; - rc = iscsi_pdu_hdr_op_text(conn, pdu); - if (rc != 0 || pdu->is_rejected) { - return rc; - } - data_len = 0; alloc_len = conn->MaxRecvDataSegmentLength; @@ -3491,11 +3485,9 @@ static int iscsi_op_scsi(struct spdk_iscsi_conn *conn, struct spdk_iscsi_pdu *pdu) { struct spdk_iscsi_task *task; - int rc; - rc = iscsi_pdu_hdr_op_scsi(conn, pdu); - if (rc != 0 || pdu->is_rejected || pdu->task == NULL) { - return rc; + if (pdu->task == NULL) { + return 0; } task = pdu->task; @@ -3983,12 +3975,6 @@ iscsi_op_nopout(struct spdk_iscsi_conn *conn, struct spdk_iscsi_pdu *pdu) uint32_t task_tag; int I_bit; int data_len; - int rc; - - rc = iscsi_pdu_hdr_op_nopout(conn, pdu); - if (rc != 0 || pdu->is_rejected) { - return rc; - } reqh = (struct iscsi_bhs_nop_out *)&pdu->bhs; I_bit = reqh->immediate; @@ -4502,9 +4488,8 @@ iscsi_op_data(struct spdk_iscsi_conn *conn, struct spdk_iscsi_pdu *pdu) int F_bit; int rc; - rc = iscsi_pdu_hdr_op_data(conn, pdu); - if (rc != 0 || pdu->is_rejected || pdu->task == NULL) { - return rc; + if (pdu->task == NULL) { + return 0; } subtask = pdu->task; @@ -4667,7 +4652,7 @@ iscsi_update_cmdsn(struct spdk_iscsi_conn *conn, struct spdk_iscsi_pdu *pdu) } static int -iscsi_execute(struct spdk_iscsi_conn *conn, struct spdk_iscsi_pdu *pdu) +iscsi_pdu_hdr_handle(struct spdk_iscsi_conn *conn, struct spdk_iscsi_pdu *pdu) { int opcode; int rc; @@ -4682,11 +4667,7 @@ iscsi_execute(struct spdk_iscsi_conn *conn, struct spdk_iscsi_pdu *pdu) SPDK_DEBUGLOG(SPDK_LOG_ISCSI, "opcode %x\n", opcode); if (opcode == ISCSI_OP_LOGIN) { - rc = iscsi_op_login(conn, pdu); - if (rc < 0) { - SPDK_ERRLOG("iscsi_op_login() failed\n"); - } - return rc; + return iscsi_pdu_hdr_op_login(conn, pdu); } /* connection in login phase but receive non-login opcode @@ -4714,18 +4695,18 @@ iscsi_execute(struct spdk_iscsi_conn *conn, struct spdk_iscsi_pdu *pdu) switch (opcode) { case ISCSI_OP_NOPOUT: - rc = iscsi_op_nopout(conn, pdu); + rc = iscsi_pdu_hdr_op_nopout(conn, pdu); break; case ISCSI_OP_SCSI: - rc = iscsi_op_scsi(conn, pdu); + rc = iscsi_pdu_hdr_op_scsi(conn, pdu); break; case ISCSI_OP_TASK: rc = iscsi_pdu_hdr_op_task(conn, pdu); break; case ISCSI_OP_TEXT: - rc = iscsi_op_text(conn, pdu); + rc = iscsi_pdu_hdr_op_text(conn, pdu); break; case ISCSI_OP_LOGOUT: @@ -4733,7 +4714,7 @@ iscsi_execute(struct spdk_iscsi_conn *conn, struct spdk_iscsi_pdu *pdu) break; case ISCSI_OP_SCSI_DATAOUT: - rc = iscsi_op_data(conn, pdu); + rc = iscsi_pdu_hdr_op_data(conn, pdu); break; case ISCSI_OP_SNACK: @@ -4746,7 +4727,54 @@ iscsi_execute(struct spdk_iscsi_conn *conn, struct spdk_iscsi_pdu *pdu) } if (rc < 0) { - SPDK_ERRLOG("processing PDU (opcode=%x) failed on %s(%s)\n", + SPDK_ERRLOG("processing PDU header (opcode=%x) failed on %s(%s)\n", + opcode, + conn->target_port != NULL ? spdk_scsi_port_get_name(conn->target_port) : "NULL", + conn->initiator_port != NULL ? spdk_scsi_port_get_name(conn->initiator_port) : "NULL"); + } + + return rc; +} + +static int +iscsi_pdu_payload_handle(struct spdk_iscsi_conn *conn, struct spdk_iscsi_pdu *pdu) +{ + int opcode; + int rc = 0; + + opcode = pdu->bhs.opcode; + + SPDK_DEBUGLOG(SPDK_LOG_ISCSI, "opcode %x\n", opcode); + + switch (opcode) { + case ISCSI_OP_LOGIN: + rc = iscsi_op_login(conn, pdu); + break; + case ISCSI_OP_NOPOUT: + rc = iscsi_op_nopout(conn, pdu); + break; + case ISCSI_OP_SCSI: + rc = iscsi_op_scsi(conn, pdu); + break; + case ISCSI_OP_TASK: + break; + case ISCSI_OP_TEXT: + rc = iscsi_op_text(conn, pdu); + break; + case ISCSI_OP_LOGOUT: + break; + case ISCSI_OP_SCSI_DATAOUT: + rc = iscsi_op_data(conn, pdu); + break; + case ISCSI_OP_SNACK: + break; + default: + SPDK_ERRLOG("unsupported opcode %x\n", opcode); + return iscsi_reject(conn, pdu, ISCSI_REASON_PROTOCOL_ERROR); + } + + if (rc < 0) { + SPDK_ERRLOG("processing PDU payload (opcode=%x) failed on %s(%s)\n", opcode, conn->target_port != NULL ? spdk_scsi_port_get_name(conn->target_port) : "NULL", conn->initiator_port != NULL ? spdk_scsi_port_get_name(conn->initiator_port) : "NULL"); @@ -4925,7 +4953,10 @@ iscsi_read_pdu(struct spdk_iscsi_conn *conn) break; } - rc = iscsi_execute(conn, pdu); + rc = iscsi_pdu_hdr_handle(conn, pdu); + if (rc == 0 && !pdu->is_rejected) { + rc = iscsi_pdu_payload_handle(conn, pdu); + } if (rc == 0) { spdk_trace_record(TRACE_ISCSI_TASK_EXECUTED, 0, 0, (uintptr_t)pdu, 0); spdk_put_pdu(pdu); diff --git a/test/unit/lib/iscsi/iscsi.c/iscsi_ut.c b/test/unit/lib/iscsi/iscsi.c/iscsi_ut.c index bb7ec6107..e90a12a6f 100644 --- a/test/unit/lib/iscsi/iscsi.c/iscsi_ut.c +++ b/test/unit/lib/iscsi/iscsi.c/iscsi_ut.c @@ -295,7 +295,10 @@ maxburstlength_test(void) req->write_bit = 1; req->final_bit = 1; - rc = iscsi_execute(&conn, req_pdu); + rc = iscsi_pdu_hdr_handle(&conn, req_pdu); + if (rc == 0 && !req_pdu->is_rejected) { + rc = iscsi_pdu_payload_handle(&conn, req_pdu); + } CU_ASSERT(rc == 0); response_pdu = TAILQ_FIRST(&g_write_pdu_list); @@ -320,7 +323,10 @@ maxburstlength_test(void) data_out->ttt = r2t->ttt; DSET24(data_out->data_segment_len, 1028); - rc = iscsi_execute(&conn, data_out_pdu); + rc = iscsi_pdu_hdr_handle(&conn, data_out_pdu); + if (rc == 0 && !data_out_pdu->is_rejected) { + rc = iscsi_pdu_payload_handle(&conn, data_out_pdu); + } CU_ASSERT(rc == SPDK_ISCSI_CONNECTION_FATAL); SPDK_CU_ASSERT_FATAL(response_pdu->task != NULL);