iscsi: change the return value of ACL func to bool

The following functions returns 1 and 0 for succcess and error,
respectively:
- spdk_iscsi_tgt_node_allow_ipv6()
- spdk_iscsi_tgt_node_allow_ipv4()
- spdk_iscsi_tgt_node_allow_netmask()
- spdk_iscsi_tgt_node_access()

Using bool for this purpose will avoid our misunderstanding.

Change-Id: I927876e0503c0eee5364e829a4713f9a345996f6
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-on: https://review.gerrithub.io/383664
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>

lib/iscsi/iscsi.c

@@ -1391,7 +1391,7 @@ spdk_iscsi_op_login_check_target(struct spdk_iscsi_conn *conn,
 				 struct spdk_iscsi_tgt_node **target)
 
 {
-	int rc;
+	bool result;
 	struct iscsi_bhs_login_rsp *rsph;
 
 	rsph = (struct iscsi_bhs_login_rsp *)&rsp_pdu->bhs;
@@ -1403,17 +1403,10 @@ spdk_iscsi_op_login_check_target(struct spdk_iscsi_conn *conn,
 		rsph->status_detail = ISCSI_LOGIN_TARGET_NOT_FOUND;
 		return SPDK_ISCSI_LOGIN_ERROR_RESPONSE;
 	}
-	rc = spdk_iscsi_tgt_node_access(conn, *target,
+	result = spdk_iscsi_tgt_node_access(conn, *target,
-					conn->initiator_name,
+					    conn->initiator_name,
-					conn->initiator_addr);
+					    conn->initiator_addr);
-	if (rc < 0) {
+	if (!result) {
-		SPDK_WARNLOG("lu_access() failed\n");
-		/* Not found */
-		rsph->status_class = ISCSI_CLASS_INITIATOR_ERROR;
-		rsph->status_detail = ISCSI_LOGIN_TARGET_NOT_FOUND;
-		return SPDK_ISCSI_LOGIN_ERROR_RESPONSE;
-	}
-	if (rc == 0) {
 		SPDK_ERRLOG("access denied\n");
 		/* Not found */
 		rsph->status_class = ISCSI_CLASS_INITIATOR_ERROR;

lib/iscsi/tgt_node.c

@@ -52,7 +52,7 @@
 #define MAX_TMPBUF 1024
 #define MAX_MASKBUF 128
 
-static int
+static bool
 spdk_iscsi_tgt_node_allow_ipv6(const char *netmask, const char *addr)
 {
 	struct in6_addr in6_mask;
@@ -64,13 +64,13 @@ spdk_iscsi_tgt_node_allow_ipv6(const char *netmask, const char *addr)
 	int i;
 
 	if (netmask[0] != '[')
-		return 0;
+		return false;
 	p = strchr(netmask, ']');
 	if (p == NULL)
-		return 0;
+		return false;
 	n = p - (netmask + 1);
 	if (n + 1 > sizeof mask)
-		return 0;
+		return false;
 
 	memcpy(mask, netmask + 1, n);
 	mask[n] = '\0';
@@ -79,7 +79,7 @@ spdk_iscsi_tgt_node_allow_ipv6(const char *netmask, const char *addr)
 	if (p[0] == '/') {
 		bits = (int) strtol(p + 1, NULL, 10);
 		if (bits < 0 || bits > 128)
-			return 0;
+			return false;
 	} else {
 		bits = 128;
 	}
@@ -92,25 +92,25 @@ spdk_iscsi_tgt_node_allow_ipv6(const char *netmask, const char *addr)
 	/* presentation to network order binary */
 	if (inet_pton(AF_INET6, mask, &in6_mask) <= 0
 	    || inet_pton(AF_INET6, addr, &in6_addr) <= 0) {
-		return 0;
+		return false;
 	}
 
 	/* check 128bits */
 	for (i = 0; i < (bits / 8); i++) {
 		if (in6_mask.s6_addr[i] != in6_addr.s6_addr[i])
-			return 0;
+			return false;
 	}
 	if (bits % 8) {
 		bmask = (0xffU << (8 - (bits % 8))) & 0xffU;
 		if ((in6_mask.s6_addr[i] & bmask) != (in6_addr.s6_addr[i] & bmask))
-			return 0;
+			return false;
 	}
 
 	/* match */
-	return 1;
+	return true;
 }
 
-static int
+static bool
 spdk_iscsi_tgt_node_allow_ipv4(const char *netmask, const char *addr)
 {
 	struct in_addr in4_mask;
@@ -127,7 +127,7 @@ spdk_iscsi_tgt_node_allow_ipv4(const char *netmask, const char *addr)
 	}
 	n = p - netmask;
 	if (n + 1 > sizeof mask)
-		return 0;
+		return false;
 
 	memcpy(mask, netmask, n);
 	mask[n] = '\0';
@@ -135,7 +135,7 @@ spdk_iscsi_tgt_node_allow_ipv4(const char *netmask, const char *addr)
 	if (p[0] == '/') {
 		bits = (int) strtol(p + 1, NULL, 10);
 		if (bits < 0 || bits > 32)
-			return 0;
+			return false;
 	} else {
 		bits = 32;
 	}
@@ -143,48 +143,47 @@ spdk_iscsi_tgt_node_allow_ipv4(const char *netmask, const char *addr)
 	/* presentation to network order binary */
 	if (inet_pton(AF_INET, mask, &in4_mask) <= 0
 	    || inet_pton(AF_INET, addr, &in4_addr) <= 0) {
-		return 0;
+		return false;
 	}
 
 	/* check 32bits */
 	bmask = (0xffffffffULL << (32 - bits)) & 0xffffffffU;
 	if ((ntohl(in4_mask.s_addr) & bmask) != (ntohl(in4_addr.s_addr) & bmask))
-		return 0;
+		return false;
 
 	/* match */
-	return 1;
+	return true;
 }
 
-static int
+static bool
 spdk_iscsi_tgt_node_allow_netmask(const char *netmask, const char *addr)
 {
 	if (netmask == NULL || addr == NULL)
-		return 0;
+		return false;
 	if (strcasecmp(netmask, "ALL") == 0)
-		return 1;
+		return true;
 	if (netmask[0] == '[') {
 		/* IPv6 */
 		if (spdk_iscsi_tgt_node_allow_ipv6(netmask, addr))
-			return 1;
+			return true;
 	} else {
 		/* IPv4 */
 		if (spdk_iscsi_tgt_node_allow_ipv4(netmask, addr))
-			return 1;
+			return true;
 	}
-	return 0;
+	return false;
 }
 
-int
+bool
 spdk_iscsi_tgt_node_access(struct spdk_iscsi_conn *conn,
 			   struct spdk_iscsi_tgt_node *target, const char *iqn, const char *addr)
 {
 	struct spdk_iscsi_portal_grp *pg;
 	struct spdk_iscsi_init_grp *igp;
-	int rc;
 	int i, j, k;
 
 	if (conn == NULL || target == NULL || iqn == NULL || addr == NULL)
-		return 0;
+		return false;
 	pg = conn->portal->group;
 
 	SPDK_DEBUGLOG(SPDK_TRACE_ISCSI, "pg=%d, iqn=%s, addr=%s\n",
@@ -205,7 +204,7 @@ spdk_iscsi_tgt_node_access(struct spdk_iscsi_conn *conn,
 					      "access denied from %s (%s) to %s (%s:%s,%d)\n",
 					      iqn, addr, target->name, conn->portal->host,
 					      conn->portal->port, conn->portal->group->tag);
-				return 0;
+				return false;
 			}
 			/* allow initiators */
 			if (strcasecmp(igp->initiators[j], "ALL") == 0
@@ -215,10 +214,9 @@ spdk_iscsi_tgt_node_access(struct spdk_iscsi_conn *conn,
 					SPDK_DEBUGLOG(SPDK_TRACE_ISCSI,
 						      "netmask=%s, addr=%s\n",
 						      igp->netmasks[k], addr);
-					rc = spdk_iscsi_tgt_node_allow_netmask(igp->netmasks[k], addr);
+					if (spdk_iscsi_tgt_node_allow_netmask(igp->netmasks[k], addr)) {
-					if (rc > 0) {
 						/* OK netmask */
-						return 1;
+						return true;
 					}
 				}
 				/* NG netmask in this group */
@@ -230,17 +228,17 @@ spdk_iscsi_tgt_node_access(struct spdk_iscsi_conn *conn,
 	SPDK_DEBUGLOG(SPDK_TRACE_ISCSI, "access denied from %s (%s) to %s (%s:%s,%d)\n",
 		      iqn, addr, target->name, conn->portal->host,
 		      conn->portal->port, conn->portal->group->tag);
-	return 0;
+	return false;
 }
 
-static int
+static bool
 spdk_iscsi_tgt_node_visible(struct spdk_iscsi_tgt_node *target, const char *iqn)
 {
 	struct spdk_iscsi_init_grp *igp;
 	int i, j;
 
 	if (target == NULL || iqn == NULL)
-		return 0;
+		return false;
 
 	for (i = 0; i < target->maxmap; i++) {
 		/* iqn is initiator group? */
@@ -250,18 +248,18 @@ spdk_iscsi_tgt_node_visible(struct spdk_iscsi_tgt_node *target, const char *iqn)
 			    && (strcasecmp(&igp->initiators[j][1], "ALL") == 0
 				|| strcasecmp(&igp->initiators[j][1], iqn) == 0)) {
 				/* NG */
-				return 0;
+				return false;
 			}
 			if (strcasecmp(igp->initiators[j], "ALL") == 0
 			    || strcasecmp(igp->initiators[j], iqn) == 0) {
 				/* OK iqn, no check addr */
-				return 1;
+				return true;
 			}
 		}
 	}
 
 	/* NG */
-	return 0;
+	return false;
 }
 
 int

lib/iscsi/tgt_node.h

@@ -99,9 +99,9 @@ spdk_iscsi_tgt_node_construct(int target_index,
 			      int no_auth_chap, int auth_chap, int auth_chap_mutual, int auth_group,
 			      int header_digest, int data_digest);
 
-int spdk_iscsi_tgt_node_access(struct spdk_iscsi_conn *conn,
+bool spdk_iscsi_tgt_node_access(struct spdk_iscsi_conn *conn,
-			       struct spdk_iscsi_tgt_node *target, const char *iqn,
+				struct spdk_iscsi_tgt_node *target, const char *iqn,
-			       const char *addr);
+				const char *addr);
 struct spdk_iscsi_tgt_node *spdk_iscsi_find_tgt_node(const char *target_name);
 int spdk_iscsi_tgt_node_reset(struct spdk_iscsi_tgt_node *target,
 			      uint64_t lun);

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

@@ -45,18 +45,31 @@
 #include "iscsi/portal_grp.h"
 #include "scsi/scsi_internal.h"
 
+#define UT_TARGET_NAME1		"iqn.2017-11.spdk.io:t0001"
+#define UT_TARGET_NAME2		"iqn.2017-11.spdk.io:t0002"
+#define UT_INITIATOR_NAME1	"iqn.2017-11.spdk.io:i0001"
+#define UT_INITIATOR_NAME2	"iqn.2017-11.spdk.io:i0002"
+
 struct spdk_iscsi_tgt_node *
 spdk_iscsi_find_tgt_node(const char *target_name)
 {
-	return NULL;
+	if (strcasecmp(target_name, UT_TARGET_NAME1) == 0) {
+		return (struct spdk_iscsi_tgt_node *)1;
+	} else {
+		return NULL;
+	}
 }
 
-int
+bool
 spdk_iscsi_tgt_node_access(struct spdk_iscsi_conn *conn,
 			   struct spdk_iscsi_tgt_node *target,
 			   const char *iqn, const char *addr)
 {
-	return 0;
+	if (strcasecmp(conn->initiator_name, UT_INITIATOR_NAME1) == 0) {
+		return true;
+	} else {
+		return false;
+	}
 }
 
 int
@@ -98,6 +111,39 @@ spdk_scsi_dev_get_lun(struct spdk_scsi_dev *dev, int lun_id)
 	return dev->lun[lun_id];
 }
 
+static void
+op_login_check_target_test(void)
+{
+	struct spdk_iscsi_conn conn;
+	struct spdk_iscsi_pdu rsp_pdu;
+	struct spdk_iscsi_tgt_node *target;
+	int rc;
+
+	/* expect sucess */
+	snprintf(conn.initiator_name, sizeof(conn.initiator_name),
+		 "%s", UT_INITIATOR_NAME1);
+
+	rc = spdk_iscsi_op_login_check_target(&conn, &rsp_pdu,
+					      UT_TARGET_NAME1, &target);
+	CU_ASSERT(rc == 0);
+
+	/* expect failure */
+	snprintf(conn.initiator_name, sizeof(conn.initiator_name),
+		 "%s", UT_INITIATOR_NAME1);
+
+	rc = spdk_iscsi_op_login_check_target(&conn, &rsp_pdu,
+					      UT_TARGET_NAME2, &target);
+	CU_ASSERT(rc != 0);
+
+	/* expect failure */
+	snprintf(conn.initiator_name, sizeof(conn.initiator_name),
+		 "%s", UT_INITIATOR_NAME2);
+
+	rc = spdk_iscsi_op_login_check_target(&conn, &rsp_pdu,
+					      UT_TARGET_NAME1, &target);
+	CU_ASSERT(rc != 0);
+}
+
 static void
 maxburstlength_test(void)
 {
@@ -206,7 +252,8 @@ main(int argc, char **argv)
 	}
 
 	if (
-		CU_add_test(suite, "maxburstlength test", maxburstlength_test) == NULL
+		CU_add_test(suite, "login check target test", op_login_check_target_test) == NULL
+		|| CU_add_test(suite, "maxburstlength test", maxburstlength_test) == NULL
 	) {
 		CU_cleanup_registry();
 		return CU_get_error();

test/unit/lib/iscsi/param.c/param_ut.c

@@ -48,12 +48,12 @@ spdk_iscsi_find_tgt_node(const char *target_name)
 	return NULL;
 }
 
-int
+bool
 spdk_iscsi_tgt_node_access(struct spdk_iscsi_conn *conn,
 			   struct spdk_iscsi_tgt_node *target,
 			   const char *iqn, const char *addr)
 {
-	return 0;
+	return false;
 }
 
 int

test/unit/lib/iscsi/tgt_node.c/tgt_node_ut.c

@@ -111,69 +111,69 @@ config_file_fail_cases(void)
 static void
 allow_ipv6_allowed(void)
 {
-	int rc;
+	bool result;
 	char *netmask;
 	char *addr;
 
 	netmask = "[2001:ad6:1234::]/48";
 	addr = "2001:ad6:1234:5678:9abc::";
 
-	rc = spdk_iscsi_tgt_node_allow_ipv6(netmask, addr);
+	result = spdk_iscsi_tgt_node_allow_ipv6(netmask, addr);
-	CU_ASSERT(rc != 0);
+	CU_ASSERT(result == true);
 
-	rc = spdk_iscsi_tgt_node_allow_netmask(netmask, addr);
+	result = spdk_iscsi_tgt_node_allow_netmask(netmask, addr);
-	CU_ASSERT(rc != 0);
+	CU_ASSERT(result == true);
 }
 
 static void
 allow_ipv6_denied(void)
 {
-	int rc;
+	bool result;
 	char *netmask;
 	char *addr;
 
 	netmask = "[2001:ad6:1234::]/56";
 	addr = "2001:ad6:1234:5678:9abc::";
 
-	rc = spdk_iscsi_tgt_node_allow_ipv6(netmask, addr);
+	result = spdk_iscsi_tgt_node_allow_ipv6(netmask, addr);
-	CU_ASSERT(rc == 0);
+	CU_ASSERT(result == false);
 
-	rc = spdk_iscsi_tgt_node_allow_netmask(netmask, addr);
+	result = spdk_iscsi_tgt_node_allow_netmask(netmask, addr);
-	CU_ASSERT(rc == 0);
+	CU_ASSERT(result == false);
 }
 
 static void
 allow_ipv4_allowed(void)
 {
-	int rc;
+	bool result;
 	char *netmask;
 	char *addr;
 
 	netmask = "192.168.2.0/24";
 	addr = "192.168.2.1";
 
-	rc = spdk_iscsi_tgt_node_allow_ipv4(netmask, addr);
+	result = spdk_iscsi_tgt_node_allow_ipv4(netmask, addr);
-	CU_ASSERT(rc != 0);
+	CU_ASSERT(result == true);
 
-	rc = spdk_iscsi_tgt_node_allow_netmask(netmask, addr);
+	result = spdk_iscsi_tgt_node_allow_netmask(netmask, addr);
-	CU_ASSERT(rc != 0);
+	CU_ASSERT(result == true);
 }
 
 static void
 allow_ipv4_denied(void)
 {
-	int rc;
+	bool result;
 	char *netmask;
 	char *addr;
 
 	netmask = "192.168.2.0";
 	addr  = "192.168.2.1";
 
-	rc = spdk_iscsi_tgt_node_allow_ipv4(netmask, addr);
+	result = spdk_iscsi_tgt_node_allow_ipv4(netmask, addr);
-	CU_ASSERT(rc == 0);
+	CU_ASSERT(result == false);
 
-	rc = spdk_iscsi_tgt_node_allow_netmask(netmask, addr);
+	result = spdk_iscsi_tgt_node_allow_netmask(netmask, addr);
-	CU_ASSERT(rc == 0);
+	CU_ASSERT(result == false);
 }
 
 static void
@@ -187,7 +187,7 @@ node_access_allowed(void)
 	char *initiators[] = {"iqn.2017-10.spdk.io:0001"};
 	char *netmasks[] = {"192.168.2.0/24"};
 	char *iqn, *addr;
-	int rc;
+	bool result;
 
 	/* portal group initialization */
 	memset(&pg, 0, sizeof(struct spdk_iscsi_portal_grp));
@@ -223,9 +223,8 @@ node_access_allowed(void)
 	iqn = "iqn.2017-10.spdk.io:0001";
 	addr = "192.168.2.1";
 
-	rc = spdk_iscsi_tgt_node_access(&conn, &tgtnode, iqn, addr);
+	result = spdk_iscsi_tgt_node_access(&conn, &tgtnode, iqn, addr);
-	CU_ASSERT(rc == 1);
+	CU_ASSERT(result == true);
-
 }
 
 static void
@@ -238,7 +237,7 @@ node_access_denied_by_empty_netmask(void)
 	struct spdk_iscsi_portal portal;
 	char *initiators[] = {"iqn.2017-10.spdk.io:0001"};
 	char *iqn, *addr;
-	int rc;
+	bool result;
 
 	/* portal group initialization */
 	memset(&pg, 0, sizeof(struct spdk_iscsi_portal_grp));
@@ -274,8 +273,8 @@ node_access_denied_by_empty_netmask(void)
 	iqn = "iqn.2017-10.spdk.io:0001";
 	addr = "192.168.3.1";
 
-	rc = spdk_iscsi_tgt_node_access(&conn, &tgtnode, iqn, addr);
+	result = spdk_iscsi_tgt_node_access(&conn, &tgtnode, iqn, addr);
-	CU_ASSERT(rc == 0);
+	CU_ASSERT(result == false);
 
 }