Improve dump header

include/utils.h

@@ -0,0 +1,37 @@
+/*
+   Utility helper functions.
+
+   Copyright (C) 2023 by zhenwei pi <[email protected]>
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU Lesser General Public License as published by
+   the Free Software Foundation; either version 2.1 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public License
+   along with this program; if not, see <http://www.gnu.org/licenses/>.
+*/
+#ifndef __iscsi_utils_h__
+#define __iscsi_utils_h__
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+struct iscsi_value_string {
+   int value;
+   const char *string;
+};
+
+const char *iscsi_value_string_find(struct iscsi_value_string *values, int value, const char *not_found);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* __iscsi_utils_h__ */

lib/Makefile.am

@@ -6,7 +6,7 @@ libiscsipriv_la_SOURCES = \
 	connect.c crc32c.c discovery.c init.c \
 	login.c nop.c pdu.c iscsi-command.c \
 	scsi-lowlevel.c socket.c sync.c task_mgmt.c \
-	logging.c
+	logging.c utils.c
 
 if TARGET_OS_IS_WIN32
 libiscsipriv_la_SOURCES += ../win32/win32_compat.c

lib/pdu.c

@@ -48,6 +48,7 @@
 #include "iscsi-private.h"
 #include "scsi-lowlevel.h"
 #include "slist.h"
+#include "utils.h"
 
 /* This adds 32-bit serial comparision as defined in RFC1982.
  * It returns 0 for equality, 1 if s1 is greater than s2 and
@@ -78,19 +79,106 @@ iscsi_itt_post_increment(struct iscsi_context *iscsi) {
 	return old_itt;
 }
 
+static const char *
+iscsi_opcode_str(int opcode)
+{
+	static struct iscsi_value_string opcode_strings[] = {
+		{ ISCSI_PDU_NOP_OUT,
+			"NOP_OUT" },
+		{ ISCSI_PDU_SCSI_REQUEST,
+			"SCSI_REQUEST" },
+		{ ISCSI_PDU_SCSI_TASK_MANAGEMENT_REQUEST,
+			"SCSI_TASK_MANAGEMENT_REQUEST" },
+		{ ISCSI_PDU_LOGIN_REQUEST,
+			"LOGIN_REQUEST" },
+		{ ISCSI_PDU_TEXT_REQUEST,
+			"TEXT_REQUEST" },
+		{ ISCSI_PDU_DATA_OUT,
+			"DATA_OUT" },
+		{ ISCSI_PDU_LOGOUT_REQUEST,
+			"LOGOUT_REQUEST" },
+		{ ISCSI_PDU_NOP_IN,
+			"NOP_IN" },
+		{ ISCSI_PDU_SCSI_RESPONSE,
+			"SCSI_RESPONSE" },
+		{ ISCSI_PDU_SCSI_TASK_MANAGEMENT_RESPONSE,
+			"SCSI_TASK_MANAGEMENT_RESPONSE" },
+		{ ISCSI_PDU_LOGIN_RESPONSE,
+			"LOGIN_RESPONSE" },
+		{ ISCSI_PDU_TEXT_RESPONSE,
+			"TEXT_RESPONSE" },
+		{ ISCSI_PDU_DATA_IN,
+			"DATA_IN" },
+		{ ISCSI_PDU_LOGOUT_RESPONSE,
+			"LOGOUT_RESPONSE" },
+		{ ISCSI_PDU_R2T,
+			"R2T" },
+		{ ISCSI_PDU_ASYNC_MSG,
+			"ASYNC_MSG" },
+		{ ISCSI_PDU_REJECT,
+			"REJECT" },
+		{ ISCSI_PDU_NO_PDU,
+			"NO_PDU" },
+		{0, NULL}
+	};
+
+	return iscsi_value_string_find(opcode_strings, opcode, "UNKNOWN");
+}
+
 void iscsi_dump_pdu_header(struct iscsi_context *iscsi, unsigned char *data) {
-	char dump1[33*3+1]={0};
-	char dump2[(ISCSI_RAW_HEADER_SIZE-33)*3+1]={0};
-	const char *opcode;
-	int i;
-	for (i=0;i<33;i++) {
-		snprintf(&dump1[i * 3], 4, " %02x", data[i]);
-	}
-	opcode = scsi_opcode_str(data[32]);
-	for (;i<ISCSI_RAW_HEADER_SIZE;i++) {
-		snprintf(&dump2[(i-33) * 3], 4, " %02x", data[i]);
-	}
-	ISCSI_LOG(iscsi, 2, "PDU header:%s[%s]%s", dump1, opcode, dump2);
+	char output[1024] = { 0 };
+	unsigned char iscsi_opcode;
+	const char *iscsi_opcode_string;
+	size_t output_off = 0;
+	int data_off = 0;
+
+
+	/* start to dump iSCSI opcode - data[0] */
+	iscsi_opcode = data[data_off];
+	iscsi_opcode_string = iscsi_opcode_str(iscsi_opcode);
+	output_off += snprintf(&output[output_off], sizeof(output) - output_off - 1, "%02x[%s]",
+			iscsi_opcode, iscsi_opcode_string);
+	data_off++;
+
+	if (iscsi_opcode == ISCSI_PDU_SCSI_REQUEST) {
+		unsigned char scsi_opcode;
+		const char *scsi_opcode_string;
+
+		/* the rest iSCSI PDU: data[1] - data[31] */
+		for ( ; data_off < 32; data_off++) {
+			if (sizeof(output) - 1 > output_off) {
+				output_off += snprintf(&output[output_off], sizeof(output) - output_off - 1,
+						" %02x", data[data_off]);
+			}
+		}
+
+		/* SCSI opcode: data[32] */
+		scsi_opcode = data[data_off];
+		scsi_opcode_string = scsi_opcode_str(scsi_opcode);
+		if (sizeof(output) - 1 > output_off) {
+			output_off += snprintf(&output[output_off], sizeof(output) - output_off - 1,
+					" %02x[%s]", scsi_opcode, scsi_opcode_string);
+			data_off++;
+		}
+
+		/* the rest SCSI PDU: data[33] - data[ISCSI_RAW_HEADER_SIZE - 1] */
+		for ( ; data_off < ISCSI_RAW_HEADER_SIZE; data_off++) {
+			if (sizeof(output) - 1 > output_off) {
+				output_off += snprintf(&output[output_off], sizeof(output) - output_off - 1,
+						" %02x", data[data_off]);
+			}
+		}
+	} else {
+		/* the rest iSCSI PDU: data[1] - data[ISCSI_RAW_HEADER_SIZE - 1] */
+		for ( ; data_off < ISCSI_RAW_HEADER_SIZE; data_off++) {
+			if (sizeof(output) - 1 > output_off) {
+				output_off += snprintf(&output[output_off], sizeof(output) - output_off - 1,
+						" %02x", data[data_off]);
+			}
+		}
+	}
+
+	ISCSI_LOG(iscsi, 2, "PDU header: %s", output);
 }
 
 struct iscsi_pdu*

lib/scsi-lowlevel.c

@@ -56,6 +56,7 @@
 #include <errno.h>
 #include "slist.h"
 #include "scsi-lowlevel.h"
+#include "utils.h"
 
 void scsi_task_set_iov_out(struct scsi_task *task, struct scsi_iovec *iov, int niov);
 
@@ -116,26 +117,10 @@ scsi_malloc(struct scsi_task *task, size_t size)
 	return &mem->buf[0];
 }
 
-struct value_string {
-   int value;
-   const char *string;
-};
-
-static const char *
-value_string_find(struct value_string *values, int value)
-{
-	for (; values->string; values++) {
-		if (value == values->value) {
-			return values->string;
-		}
-	}
-	return NULL;
-}
-
 const char *
 scsi_sense_key_str(int key)
 {
-	static struct value_string keys[] = {
+	static struct iscsi_value_string keys[] = {
 		{SCSI_SENSE_NO_SENSE,
 		 "NO SENSE"},
 		{SCSI_SENSE_RECOVERED_ERROR,
@@ -167,13 +152,13 @@ scsi_sense_key_str(int key)
 	       {0, NULL}
 	};
 
-	return value_string_find(keys, key);
+	return iscsi_value_string_find(keys, key, "UNKNOWN");
 }
 
 const char *
 scsi_sense_ascq_str(int ascq)
 {
-	static struct value_string ascqs[] = {
+	static struct iscsi_value_string ascqs[] = {
 		{SCSI_SENSE_ASCQ_SANITIZE_IN_PROGRESS,
 		 "SANITIZE_IN_PROGRESS"},
 		{SCSI_SENSE_ASCQ_WRITE_AFTER_SANITIZE_REQUIRED,
@@ -239,13 +224,13 @@ scsi_sense_ascq_str(int ascq)
 	       {0, NULL}
 	};
 
-	return value_string_find(ascqs, ascq);
+	return iscsi_value_string_find(ascqs, ascq, "UNKNOWN");
 }
 
 const char *
 scsi_pr_type_str(enum scsi_persistent_out_type pr_type)
 {
-	static struct value_string pr_type_strings[] = {
+	static struct iscsi_value_string pr_type_strings[] = {
 		{SCSI_PERSISTENT_RESERVE_TYPE_WRITE_EXCLUSIVE,
 		 "Write Exclusive"},
 		{SCSI_PERSISTENT_RESERVE_TYPE_EXCLUSIVE_ACCESS,
@@ -261,13 +246,13 @@ scsi_pr_type_str(enum scsi_persistent_out_type pr_type)
 		{0, NULL}
 	};
 
-	return value_string_find(pr_type_strings, pr_type);
+	return iscsi_value_string_find(pr_type_strings, pr_type, "UNKNOWN");
 }
 
 const char *
 scsi_opcode_str(int opcode)
 {
-	static struct value_string opcode_strings[] = {
+	static struct iscsi_value_string opcode_strings[] = {
 		{SCSI_OPCODE_TESTUNITREADY,
 		 "TESTUNITREADY"},
 		{SCSI_OPCODE_READ6,
@@ -361,7 +346,7 @@ scsi_opcode_str(int opcode)
 		{0, NULL}
 	};
 
-	return value_string_find(opcode_strings, opcode);
+	return iscsi_value_string_find(opcode_strings, opcode, "UNKNOWN");
 }
 
 uint64_t

lib/utils.c

@@ -0,0 +1,35 @@
+/*
+   Utility helper functions.
+
+   Copyright (C) 2023 by zhenwei pi <[email protected]>
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU Lesser General Public License as published by
+   the Free Software Foundation; either version 2.1 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public License
+   along with this program; if not, see <http://www.gnu.org/licenses/>.
+*/
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
+#include "utils.h"
+
+const char *iscsi_value_string_find(struct iscsi_value_string *values,
+                                    int value, const char *not_found)
+{
+	for ( ; values->string; values++) {
+		if (value == values->value) {
+			return values->string;
+		}
+	}
+
+	return not_found;
+}