ctladm: better documentation for adding and removing cfiscsi ports

Sponsored by:	Axcient
Reviewed by:    mav
Pull Request:   https://github.com/freebsd/freebsd-src/pull/1279

(cherry picked from commit afecc74cd7158da8e89c26c5267bc715c2417fe7)

3 files changed, 191 insertions, 14 deletions

diff --git a/sys/cam/ctl/ctl_frontend_iscsi.c b/sys/cam/ctl/ctl_frontend_iscsi.c
index 253ffbe7dbdc..03d5c91f7bf5 100644
--- a/sys/cam/ctl/ctl_frontend_iscsi.c
+++ b/sys/cam/ctl/ctl_frontend_iscsi.c
@@ -2150,17 +2150,24 @@ cfiscsi_ioctl_port_create(struct ctl_req *req)
 	uint16_t tag;
 
 	target = dnvlist_get_string(req->args_nvl, "cfiscsi_target", NULL);
-	alias = dnvlist_get_string(req->args_nvl, "cfiscsi_target_alias", NULL);
+	if (target == NULL) {
+		req->status = CTL_LUN_ERROR;
+		snprintf(req->error_str, sizeof(req->error_str),
+		    "Missing required argument: cfiscsi_target");
+		return;
+	}
+
 	val = dnvlist_get_string(req->args_nvl, "cfiscsi_portal_group_tag",
 	    NULL);
-
-	if (target == NULL || val == NULL) {
+	if (val == NULL) {
 		req->status = CTL_LUN_ERROR;
 		snprintf(req->error_str, sizeof(req->error_str),
-		    "Missing required argument");
+		    "Missing required argument: cfiscsi_portal_group_tag");
 		return;
 	}
 
+	alias = dnvlist_get_string(req->args_nvl, "cfiscsi_target_alias", NULL);
+
 	tag = strtoul(val, NULL, 0);
 	ct = cfiscsi_target_find_or_create(&cfiscsi_softc, target, alias, tag);
 	if (ct == NULL) {
@@ -2251,13 +2258,19 @@ cfiscsi_ioctl_port_remove(struct ctl_req *req)
 	uint16_t tag;
 
 	target = dnvlist_get_string(req->args_nvl, "cfiscsi_target", NULL);
+	if (target == NULL) {
+		req->status = CTL_LUN_ERROR;
+		snprintf(req->error_str, sizeof(req->error_str),
+		    "Missing required argument: cfiscsi_target");
+		return;
+	}
+
 	val = dnvlist_get_string(req->args_nvl, "cfiscsi_portal_group_tag",
 	    NULL);
-
-	if (target == NULL || val == NULL) {
+	if (val == NULL) {
 		req->status = CTL_LUN_ERROR;
 		snprintf(req->error_str, sizeof(req->error_str),
-		    "Missing required argument");
+		    "Missing required argument: cfiscsi_portal_group_tag");
 		return;
 	}
 
diff --git a/usr.sbin/ctladm/ctladm.8 b/usr.sbin/ctladm/ctladm.8
index 4e7288dac6b6..a7eb1d58cbbf 100644
--- a/usr.sbin/ctladm/ctladm.8
+++ b/usr.sbin/ctladm/ctladm.8
@@ -35,7 +35,7 @@
 .\"
 .\" $Id: //depot/users/kenm/FreeBSD-test2/usr.sbin/ctladm/ctladm.8#3 $
 .\"
-.Dd June 5, 2024
+.Dd June 6, 2024
 .Dt CTLADM 8
 .Os
 .Sh NAME
@@ -166,7 +166,7 @@
 .Op Fl o Ar on|off
 .Op Fl w Ar wwpn
 .Op Fl W Ar wwnn
-.Op Fl O Ar pp|vp
+.Op Fl O Ar name=value
 .Op Fl p Ar targ_port
 .Op Fl r
 .Op Fl t Ar fe_type
@@ -618,7 +618,7 @@ The WWNN and WWPN may both be specified at the same time, but cannot be
 combined with enabling/disabling or listing ports.
 .Bl -tag -width 12n
 .It Fl c
-Create new frontend port using free pp and vp=0.
+Create new frontend port.
 .It Fl d Ar driver
 Specify the name of the frontend driver used by the
 .Pq Fl c
@@ -637,7 +637,31 @@ If no port number or port type is specified, all ports are turned on or
 off.
 .It Fl O Ar pp|vp
 Specify generic options on the ioctl frontend port.
-At present, only pp and vp port numbers can be set.
+The list of recognized options is driver-dependent.
+The
+.Dq ioctl
+driver recognizes
+.Dq pp
+and
+.Dq vp .
+The
+.Dq iscsi
+driver recongizes
+.Dq cfiscsi_portal_group_tag ,
+.Dq cfiscsi_target ,
+and
+.Dq cfiscsi_target_alias .
+The
+.Dq nvmf
+driver recognizes
+.Dq subnqn ,
+.Dq portid ,
+.Dq max_io_qsize ,
+.Dq enable_timeout ,
+.Dq ioccsz ,
+.Dq nn ,
+and
+.Dq serial .
 .It Fl p Ar targ_port
 Specify the frontend port number.
 The port numbers can be found in the frontend port list.
diff --git a/usr.sbin/ctladm/tests/port.sh b/usr.sbin/ctladm/tests/port.sh
index 782a8ee5b6c1..161759ec14d0 100644
--- a/usr.sbin/ctladm/tests/port.sh
+++ b/usr.sbin/ctladm/tests/port.sh
@@ -30,11 +30,20 @@
 # * Creating umass ports
 
 # TODO
-# * Creating iscsi ports
 # * Creating nvmf ports
 # * Creating ha ports
 # * Creating fc ports
 
+# The PGTAG can be any 16-bit number.  The only constraint is that each
+# PGTAG,TARGET pair must be globally unique.
+PGTAG=30257
+
+load_cfiscsi() {
+	if ! kldstat -q -m cfiscsi; then
+		kldload cfiscsi || atf_skip "could not load cfscsi kernel mod"
+	fi
+}
+
 skip_if_ctld() {
 	if service ctld onestatus > /dev/null; then
 		# If ctld is running on this server, let's not interfere.
@@ -46,8 +55,18 @@ cleanup() {
 	driver=$1
 
 	if [ -e port-create.txt ]; then
-		portnum=`awk '/port:/ {print $2}' port-create.txt`
-		ctladm port -r -d $driver -p $portnum
+		case "$driver" in
+		"ioctl")
+			PORTNUM=`awk '/port:/ {print $2}' port-create.txt`
+			ctladm port -r -d $driver -p $PORTNUM
+			;;
+		"iscsi")
+			TARGET=`awk '/target:/ {print $2}' port-create.txt`
+			# PORTNUM is ignored, but must be set
+			PORTNUM=9999
+			ctladm port -r -d $driver -p "$PORTNUM" -O cfiscsi_portal_group_tag=$PGTAG -O cfiscsi_target=$TARGET
+			;;
+		esac
 	fi
 }
 
@@ -74,6 +93,75 @@ create_ioctl_cleanup()
 	cleanup ioctl
 }
 
+atf_test_case create_iscsi cleanup
+create_iscsi_head()
+{
+	atf_set "descr" "ctladm can create a new iscsi port"
+	atf_set "require.user" "root"
+}
+create_iscsi_body()
+{
+	skip_if_ctld
+	load_cfiscsi
+
+	TARGET=iqn.2018-10.myhost.create_iscsi
+	atf_check -o save:port-create.txt ctladm port -c -d "iscsi" -O cfiscsi_portal_group_tag=$PGTAG -O cfiscsi_target="$TARGET"
+	echo "target: $TARGET" >> port-create.txt
+	atf_check egrep -q "Port created successfully" port-create.txt
+	atf_check egrep -q "frontend: *iscsi" port-create.txt
+	atf_check egrep -q "port: *[0-9]+" port-create.txt
+	atf_check -o save:portlist.txt ctladm portlist -qf iscsi
+	# Unlike the ioctl driver, the iscsi driver creates ports in a disabled
+	# state, so the port's lunmap may be set before enabling it.
+	atf_check egrep -q "$portnum *NO *iscsi *iscsi.*$TARGET" portlist.txt
+}
+create_iscsi_cleanup()
+{
+	cleanup iscsi
+}
+
+atf_test_case create_iscsi_alias cleanup
+create_iscsi_alias_head()
+{
+	atf_set "descr" "ctladm can create a new iscsi port with a target alias"
+	atf_set "require.user" "root"
+}
+create_iscsi_alias_body()
+{
+	skip_if_ctld
+	load_cfiscsi
+
+	TARGET=iqn.2018-10.myhost.create_iscsi_alias
+	ALIAS="foobar"
+	atf_check -o save:port-create.txt ctladm port -c -d "iscsi" -O cfiscsi_portal_group_tag=$PGTAG -O cfiscsi_target="$TARGET" -O cfiscsi_target_alias="$ALIAS"
+	echo "target: $TARGET" >> port-create.txt
+	atf_check egrep -q "Port created successfully" port-create.txt
+	atf_check egrep -q "frontend: *iscsi" port-create.txt
+	atf_check egrep -q "port: *[0-9]+" port-create.txt
+	atf_check -o save:portlist.txt ctladm portlist -qvf iscsi
+	atf_check egrep -q "cfiscsi_target_alias=$ALIAS" portlist.txt
+}
+create_iscsi_alias_cleanup()
+{
+	cleanup iscsi
+}
+
+atf_test_case create_iscsi_without_required_args
+create_iscsi_without_required_args_head()
+{
+	atf_set "descr" "ctladm will gracefully fail to create an iSCSI target if required arguments are missing"
+	atf_set "require.user" "root"
+}
+create_iscsi_without_required_args_body()
+{
+	skip_if_ctld
+	load_cfiscsi
+
+	TARGET=iqn.2018-10.myhost.create_iscsi
+	atf_check -s exit:1 -e match:"Missing required argument: cfiscsi_target" ctladm port -c -d "iscsi" -O cfiscsi_portal_group_tag=$PGTAG
+	atf_check -s exit:1 -e match:"Missing required argument: cfiscsi_portal_group_tag" ctladm port -c -d "iscsi" -O cfiscsi_target=$TARGET
+}
+
 atf_test_case create_ioctl_options cleanup
 create_ioctl_options_head()
 {
@@ -168,11 +256,63 @@ remove_ioctl_body()
 	fi
 }
 
+atf_test_case remove_iscsi
+remove_iscsi_head()
+{
+	atf_set "descr" "ctladm can remove an iscsi port"
+	atf_set "require.user" "root"
+}
+remove_iscsi_body()
+{
+	skip_if_ctld
+	load_cfiscsi
+
+	TARGET=iqn.2018-10.myhost.remove_iscsi
+	atf_check -o save:port-create.txt ctladm port -c -d "iscsi" -O cfiscsi_portal_group_tag=$PGTAG -O cfiscsi_target="$TARGET"
+	portnum=`awk '/port:/ {print $2}' port-create.txt`
+	atf_check -o save:portlist.txt ctladm portlist -qf iscsi
+	atf_check -o inline:"Port destroyed successfully\n" ctladm port -r -d iscsi -p 9999 -O cfiscsi_portal_group_tag=$PGTAG -O cfiscsi_target="$TARGET"
+	# Check that the port was removed.  A new port may have been added with
+	# the same ID, so match against the target and tag, too.
+	PGTAGHEX=0x7631	# PGTAG in hex
+	if ctladm portlist -qf iscsi | egrep -q "^${portnum} .*$PGTAG +[0-9]+ +$TARGET,t,$PGTAGHEX"; then
+		ctladm portlist -qf iscsi
+		atf_fail "port was not removed"
+	fi
+}
+
+atf_test_case remove_iscsi_without_required_args cleanup
+remove_iscsi_without_required_args_head()
+{
+	atf_set "descr" "ctladm will gracefully fail to remove an iSCSI target if required arguments are missing"
+	atf_set "require.user" "root"
+}
+remove_iscsi_without_required_args_body()
+{
+	skip_if_ctld
+	load_cfiscsi
+
+	TARGET=iqn.2018-10.myhost.remove_iscsi_without_required_args
+	atf_check -o save:port-create.txt ctladm port -c -d "iscsi" -O cfiscsi_portal_group_tag=$PGTAG -O cfiscsi_target="$TARGET"
+	echo "target: $TARGET" >> port-create.txt
+	atf_check -s exit:1 -e match:"Missing required argument: cfiscsi_portal_group_tag" ctladm port -r -d iscsi -p 9999 -O cfiscsi_target="$TARGET"
+	atf_check -s exit:1 -e match:"Missing required argument: cfiscsi_target" ctladm port -r -d iscsi -p 9999 -O cfiscsi_portal_group_tag=$PGTAG
+}
+remove_iscsi_without_required_args_cleanup()
+{
+	cleanup iscsi
+}
+
 atf_init_test_cases()
 {
 	atf_add_test_case create_ioctl
+	atf_add_test_case create_iscsi
+	atf_add_test_case create_iscsi_without_required_args
+	atf_add_test_case create_iscsi_alias
 	atf_add_test_case create_ioctl_options
 	atf_add_test_case disable_ioctl
 	atf_add_test_case enable_ioctl
 	atf_add_test_case remove_ioctl
+	atf_add_test_case remove_iscsi
+	atf_add_test_case remove_iscsi_without_required_args
 }