[stgt] CHAP doesn't work as expected ?! Or user error :) ?

[FUJITA Tomonori]

On Tue, 23 Feb 2010 09:06:57 -0800
Chandra Seetharaman <sekharan at us.ibm.com> wrote:

> IMHO, interface looks fine.
> 
> Also, shouldn't we add mutual authentication for discovery too ?

The patch supports it. I was too lazy to add the code to print
outgoing account though. I've attached the updated patch.

You can set up it as normal session authentication (that README.iscsi
exmplains):

tgtadm --lld iscsi --op bind --mode account --user hoge --outgoing


> I seek some clarification though.
> 
> In a Netapp box, I just specify incoming and outgoing password specified
> at each target level. No separate password for discovery.
> 
> Is that an assumed behavior ? if so, aren't we deviating from it ?

My Peer Storage box doesn't support discovery session
authentication. I guess that there is no standard way about how to
support discovery authentication. I can't recall what my EMC box does
but IIRC it doesn't use the same account for discovery and normal
sessions (like NetApp. I have NetApp too).


diff --git a/usr/iscsi/iscsid.c b/usr/iscsi/iscsid.c
index 2adc6a8..2ceb8be 100644
--- a/usr/iscsi/iscsid.c
+++ b/usr/iscsi/iscsid.c
@@ -38,6 +38,7 @@
 #include "util.h"
 #include "driver.h"
 #include "scsi.h"
+#include "tgtadm.h"
 #include "crc32c.h"
 
 #define MAX_QUEUE_CMD	128
@@ -467,6 +468,9 @@ static void login_start(struct iscsi_connection *conn)
 		}
 	}
 
+	if (conn->session_type == SESSION_DISCOVERY)
+		conn->tid = GLOBAL_TID;
+
 	if (conn->session_type == SESSION_NORMAL) {
 		if (!target_name) {
 			rsp->status_class = ISCSI_STATUS_CLS_INITIATOR_ERR;
diff --git a/usr/target.c b/usr/target.c
index c848757..852c183 100644
--- a/usr/target.c
+++ b/usr/target.c
@@ -42,6 +42,8 @@
 
 static LIST_HEAD(device_type_list);
 
+static struct target global_target;
+
 int device_type_register(struct device_type_template *t)
 {
 	list_add_tail(&t->device_type_siblings, &device_type_list);
@@ -1165,7 +1167,10 @@ int account_lookup(int tid, int type, char *user, int ulen, char *password, int
 	struct target *target;
 	struct account_entry *ac;
 
-	target = target_lookup(tid);
+	if (tid == GLOBAL_TID)
+		target = &global_target;
+	else
+		target = target_lookup(tid);
 	if (!target)
 		return -ENOENT;
 
@@ -1272,7 +1277,10 @@ int account_ctl(int tid, int type, char *user, int bind)
 	struct account_entry *ac;
 	int i, err = 0;
 
-	target = target_lookup(tid);
+	if (tid == GLOBAL_TID)
+		target = &global_target;
+	else
+		target = target_lookup(tid);
 	if (!target)
 		return TGTADM_NO_TARGET;
 
@@ -1323,6 +1331,9 @@ void account_del(char *user)
 		account_ctl(target->tid, ACCOUNT_TYPE_OUTGOING, ac->user, 0);
 	}
 
+	account_ctl(GLOBAL_TID, ACCOUNT_TYPE_INCOMING, ac->user, 0);
+	account_ctl(GLOBAL_TID, ACCOUNT_TYPE_OUTGOING, ac->user, 0);
+
 	list_del(&ac->account_siblings);
 	free(ac->user);
 	free(ac->password);
@@ -1333,7 +1344,10 @@ int account_available(int tid, int dir)
 {
 	struct target *target;
 
-	target = target_lookup(tid);
+	if (tid == GLOBAL_TID)
+		target = &global_target;
+	else
+		target = target_lookup(tid);
 	if (!target)
 		return 0;
 
@@ -1869,6 +1883,23 @@ int system_show(int mode, char *buf, int rest)
 	shprintf(total, buf, rest, _TAB1 "State: %s\n",
 		 system_state_name(sys_state));
 
+	if (global_target.account.nr_inaccount) {
+		int i, aid;
+		shprintf(total, buf, rest,
+			 "Account information:\n");
+		for (i = 0; i < global_target.account.nr_inaccount; i++) {
+			aid = global_target.account.in_aids[i];
+			shprintf(total, buf, rest, _TAB1 "%s\n",
+				 __account_lookup_id(aid)->user);
+		}
+		if (global_target.account.out_aid) {
+			aid = global_target.account.out_aid;
+			shprintf(total, buf, rest,
+				 _TAB1 "%s (outgoing)\n",
+				 __account_lookup_id(aid)->user);
+		}
+	}
+
 	return total;
 overflow:
 	return max;
@@ -1883,3 +1914,16 @@ int is_system_inactive(void)
 {
 	return list_empty(&target_list);
 }
+
+__attribute__((constructor)) static void target_constructor(void)
+{
+	static int global_target_aids[DEFAULT_NR_ACCOUNT];
+
+	memset(global_target_aids, 0, sizeof(global_target_aids));
+	global_target.account.in_aids = global_target_aids;
+	global_target.account.max_inaccount = DEFAULT_NR_ACCOUNT;
+
+	global_target.tid = GLOBAL_TID;
+
+	INIT_LIST_HEAD(&global_target.acl_list);
+}
diff --git a/usr/tgtadm.c b/usr/tgtadm.c
index dd46985..5d85c5f 100644
--- a/usr/tgtadm.c
+++ b/usr/tgtadm.c
@@ -647,10 +647,8 @@ int main(int argc, char **argv)
 				eprintf("'user' option is necessary\n");
 				exit(EINVAL);
 			}
-			if (tid <= 0) {
-				eprintf("'tid' option is necessary\n");
-				exit(EINVAL);
-			}
+			if (tid == -1)
+				tid = GLOBAL_TID;
 			break;
 		case OP_UNBIND:
 			rc = verify_mode_params(argc, argv, "Lmou");
@@ -663,10 +661,8 @@ int main(int argc, char **argv)
 				eprintf("'user' option is necessary\n");
 				exit(EINVAL);
 			}
-			if (tid <= 0) {
-				eprintf("'tid' option is necessary\n");
-				exit(EINVAL);
-			}
+			if (tid == -1)
+				tid = GLOBAL_TID;
 			break;
 		default:
 			eprintf("option %d not supported in account mode\n", op);
diff --git a/usr/tgtadm.h b/usr/tgtadm.h
index 60b984d..8e04a3c 100644
--- a/usr/tgtadm.h
+++ b/usr/tgtadm.h
@@ -4,6 +4,8 @@
 #define TGT_IPC_NAMESPACE	"/tmp/.TGT_IPC_ABSTRACT_NAMESPACE"
 #define TGT_LLD_NAME_LEN	64
 
+#define GLOBAL_TID (~0U)
+
 #include "tgtadm_error.h"
 
 enum tgtadm_op {
--
To unsubscribe from this list: send the line "unsubscribe stgt" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html