14436 "dladm show-overlay <overlay>" exits zero when varpd doesn't know about the overlay

Reviewed by: Robert Mustacchi <rm+illumos@fingolfin.org>
Reviewed by: Andy Fiddaman <andy@omnios.org>
Approved by: Gordon Ross <gordon.w.ross@gmail.com>

6 files changed, 167 insertions, 25 deletions

diff --git a/usr/src/cmd/dladm/dladm.c b/usr/src/cmd/dladm/dladm.c
index 020d0d7266..456811663e 100644
--- a/usr/src/cmd/dladm/dladm.c
+++ b/usr/src/cmd/dladm/dladm.c
@@ -191,6 +191,11 @@ typedef struct show_usage_state_s {
 	ofmt_handle_t	us_ofmt;
 } show_usage_state_t;
 
+typedef struct show_overlay_request_s {
+	boolean_t	sor_failed;
+	ofmt_handle_t	sor_ofmt;
+} show_overlay_request_t;
+
 /*
  * callback functions for printing output and error diagnostics.
  */
@@ -10160,21 +10165,35 @@ static int
 show_one_overlay(dladm_handle_t hdl, datalink_id_t linkid, void *arg)
 {
 	char			buf[MAXLINKNAMELEN];
+	dladm_status_t		info_status;
 	showoverlay_state_t	state;
 	datalink_class_t	class;
+	show_overlay_request_t	*req = arg;
 
-	if (dladm_datalink_id2info(hdl, linkid, NULL, &class, NULL, buf,
-	    MAXLINKNAMELEN) != DLADM_STATUS_OK ||
-	    class != DATALINK_CLASS_OVERLAY)
+	if ((info_status = dladm_datalink_id2info(hdl, linkid, NULL, &class,
+	    NULL, buf, MAXLINKNAMELEN)) != DLADM_STATUS_OK) {
+		warn_dlerr(info_status, "failed to get info for "
+		    "datalink id %u", linkid);
+		req->sor_failed = B_TRUE;
 		return (DLADM_WALK_CONTINUE);
+	}
+
+	if (class != DATALINK_CLASS_OVERLAY) {
+		warn("%s is not an overlay", buf);
+		req->sor_failed = B_TRUE;
+		return (DLADM_WALK_CONTINUE);
+	}
 
 	state.sho_linkname = buf;
-	state.sho_ofmt = arg;
+	state.sho_ofmt = req->sor_ofmt;
 
 	dladm_errlist_reset(&errlist);
 	(void) dladm_overlay_walk_prop(handle, linkid, dladm_overlay_show_one,
 	    &state, &errlist);
 	warn_dlerrlist(&errlist);
+	if (errlist.el_count) {
+		req->sor_failed = B_TRUE;
+	}
 
 	return (DLADM_WALK_CONTINUE);
 }
@@ -10275,15 +10294,26 @@ static int
 show_one_overlay_table(dladm_handle_t handle, datalink_id_t linkid, void *arg)
 {
 	char				linkbuf[MAXLINKNAMELEN];
+	dladm_status_t			info_status;
 	showoverlay_targ_state_t	shot;
 	datalink_class_t		class;
+	show_overlay_request_t		*req = arg;
 
-	if (dladm_datalink_id2info(handle, linkid, NULL, &class, NULL, linkbuf,
-	    MAXLINKNAMELEN) != DLADM_STATUS_OK ||
-	    class != DATALINK_CLASS_OVERLAY)
+	if ((info_status = dladm_datalink_id2info(handle, linkid, NULL, &class,
+	    NULL, linkbuf, MAXLINKNAMELEN)) != DLADM_STATUS_OK) {
+		warn_dlerr(info_status, "failed to get info for "
+		    "datalink id %u", linkid);
+		req->sor_failed = B_TRUE;
 		return (DLADM_WALK_CONTINUE);
+	}
+
+	if (class != DATALINK_CLASS_OVERLAY) {
+		warn("%s is not an overlay", linkbuf);
+		req->sor_failed = B_TRUE;
+		return (DLADM_WALK_CONTINUE);
+	}
 
-	shot.shot_ofmt = arg;
+	shot.shot_ofmt = req->sor_ofmt;
 	shot.shot_linkname = linkbuf;
 
 	(void) dladm_overlay_walk_cache(handle, linkid,
@@ -10334,6 +10364,7 @@ show_one_overlay_fma(dladm_handle_t handle, datalink_id_t linkid, void *arg)
 	char			linkbuf[MAXLINKNAMELEN];
 	datalink_class_t	class;
 	showoverlay_fma_state_t	shof;
+	show_overlay_request_t	*req = arg;
 
 	if (dladm_datalink_id2info(handle, linkid, NULL, &class, NULL, linkbuf,
 	    MAXLINKNAMELEN) != DLADM_STATUS_OK ||
@@ -10341,7 +10372,7 @@ show_one_overlay_fma(dladm_handle_t handle, datalink_id_t linkid, void *arg)
 		die("datalink %s is not an overlay device\n", linkbuf);
 	}
 
-	shof.shof_ofmt = arg;
+	shof.shof_ofmt = req->sor_ofmt;
 	shof.shof_linkname = linkbuf;
 
 	status = dladm_overlay_status(handle, linkid,
@@ -10364,14 +10395,14 @@ do_show_overlay(int argc, char *argv[], const char *use)
 	const ofmt_field_t	*fieldsp;
 	ofmt_status_t		oferr;
 	boolean_t		parse;
-	ofmt_handle_t		ofmt;
+	show_overlay_request_t	req;
 	uint_t			ofmtflags;
 	int			err;
 
-
 	funcp = show_one_overlay;
 	fieldsp = overlay_fields;
 	parse = B_FALSE;
+	req.sor_failed = B_FALSE;
 	ofmtflags = OFMT_WRAP;
 	while ((opt = getopt_long(argc, argv, ":o:pft", overlay_show_lopts,
 	    NULL)) != -1) {
@@ -10399,8 +10430,8 @@ do_show_overlay(int argc, char *argv[], const char *use)
 	if (fields_str != NULL && strcasecmp(fields_str, "all") == 0)
 		fields_str = NULL;
 
-	oferr = ofmt_open(fields_str, fieldsp, ofmtflags, 0, &ofmt);
-	ofmt_check(oferr, parse, ofmt, die, warn);
+	oferr = ofmt_open(fields_str, fieldsp, ofmtflags, 0, &req.sor_ofmt);
+	ofmt_check(oferr, parse, req.sor_ofmt, die, warn);
 
 	err = 0;
 	if (argc > optind) {
@@ -10413,14 +10444,17 @@ do_show_overlay(int argc, char *argv[], const char *use)
 				err = 1;
 				continue;
 			}
-			(void) funcp(handle, linkid, ofmt);
+			(void) funcp(handle, linkid, &req);
 		}
 	} else {
-		(void) dladm_walk_datalink_id(funcp, handle, ofmt,
+		(void) dladm_walk_datalink_id(funcp, handle, &req,
 		    DATALINK_CLASS_OVERLAY, DATALINK_ANY_MEDIATYPE,
 		    DLADM_OPT_ACTIVE);
 	}
-	ofmt_close(ofmt);
+	if (req.sor_failed) {
+		err = 1;
+	}
+	ofmt_close(req.sor_ofmt);
 
 	exit(err);
 }
diff --git a/usr/src/lib/libdladm/common/libdloverlay.c b/usr/src/lib/libdladm/common/libdloverlay.c
index a25be3d201..ecb604fde9 100644
--- a/usr/src/lib/libdladm/common/libdloverlay.c
+++ b/usr/src/lib/libdladm/common/libdloverlay.c
@@ -389,24 +389,37 @@ dladm_overlay_walk_prop(dladm_handle_t handle, datalink_id_t linkid,
     dladm_overlay_prop_f func, void *arg, dladm_errlist_t *errs)
 {
 	int i, ret;
+	char buf[MAXLINKNAMELEN];
+	char errmsg[DLADM_STRSIZE];
 	datalink_class_t class;
+	dladm_status_t info_status;
 	overlay_ioc_nprops_t oin;
 	overlay_ioc_propinfo_t oipi;
 	dladm_overlay_propinfo_t dop;
 	uint64_t varpdid = UINT64_MAX;
 
-	if (dladm_datalink_id2info(handle, linkid, NULL, &class, NULL,
-	    NULL, 0) != DLADM_STATUS_OK)
+	if ((info_status = dladm_datalink_id2info(handle, linkid, NULL, &class,
+	    NULL, buf, MAXLINKNAMELEN)) != DLADM_STATUS_OK) {
+		(void) dladm_errlist_append(errs, "failed to get info for "
+		    "datalink id %u: %s",
+		    linkid, dladm_status2str(info_status, errmsg));
 		return (DLADM_STATUS_BADARG);
+	}
 
-	if (class != DATALINK_CLASS_OVERLAY)
+	if (class != DATALINK_CLASS_OVERLAY) {
+		(void) dladm_errlist_append(errs, "%s is not an overlay", buf);
 		return (DLADM_STATUS_BADARG);
+	}
 
 	bzero(&oin, sizeof (overlay_ioc_nprops_t));
 	oin.oipn_linkid = linkid;
 	ret = ioctl(dladm_dld_fd(handle), OVERLAY_IOC_NPROPS, &oin);
-	if (ret != 0)
+	if (ret != 0) {
+		(void) dladm_errlist_append(errs, "failed to get "
+		    "overlay properties for overlay %s: %s",
+		    buf, strerror(errno));
 		return (dladm_errno2status(errno));
+	}
 
 	for (i = 0; i < oin.oipn_nprops; i++) {
 		bzero(&dop, sizeof (dladm_overlay_propinfo_t));
@@ -416,7 +429,8 @@ dladm_overlay_walk_prop(dladm_handle_t handle, datalink_id_t linkid,
 		ret = ioctl(dladm_dld_fd(handle), OVERLAY_IOC_PROPINFO, &oipi);
 		if (ret != 0) {
 			(void) dladm_errlist_append(errs, "failed to get "
-			    "propinfo for property %d: %s", i, strerror(errno));
+			    "propinfo for overlay %s, property %d: %s",
+			    buf, i, strerror(errno));
 			return (dladm_errno2status(errno));
 		}
 
@@ -446,8 +460,15 @@ dladm_overlay_walk_prop(dladm_handle_t handle, datalink_id_t linkid,
 	if (varpdid == UINT64_MAX)
 		return (DLADM_STATUS_OK);
 
-	return (dladm_overlay_walk_varpd_prop(handle, linkid, varpdid, func,
-	    arg));
+	ret = dladm_overlay_walk_varpd_prop(handle, linkid, varpdid, func,
+	    arg);
+	if (ret != DLADM_STATUS_OK) {
+		(void) dladm_errlist_append(errs,
+		    "failed to get varpd props for "
+		    "overlay %s, varpd id %llu: %s",
+		    buf, varpdid, dladm_status2str(info_status, errmsg));
+	}
+	return (ret);
 }
 
 dladm_status_t
diff --git a/usr/src/pkg/manifests/system-test-utiltest.p5m b/usr/src/pkg/manifests/system-test-utiltest.p5m
index 5b82b5dbf2..9c779d6e3d 100644
--- a/usr/src/pkg/manifests/system-test-utiltest.p5m
+++ b/usr/src/pkg/manifests/system-test-utiltest.p5m
@@ -15,7 +15,7 @@
 # Copyright 2014 Nexenta Systems, Inc. All rights reserved.
 # Copyright 2020 Joyent, Inc.
 # Copyright 2017 Jason King.
-# Copyright 2020 OmniOS Community Edition (OmniOSce) Association.
+# Copyright 2022 OmniOS Community Edition (OmniOSce) Association.
 # Copyright 2021 Oxide Computer Company
 #
 
@@ -1840,6 +1840,7 @@ file path=opt/util-tests/tests/sed/regress.multitest.out/9.8 mode=0444
 file path=opt/util-tests/tests/sed/regress.multitest.out/9.9 mode=0444
 file path=opt/util-tests/tests/sed/sed_addr mode=0555
 file path=opt/util-tests/tests/set-linkprop mode=0555
+file path=opt/util-tests/tests/show-overlay-exit mode=0555
 dir  path=opt/util-tests/tests/sleep
 file path=opt/util-tests/tests/sleep/sleep.awk mode=0444
 file path=opt/util-tests/tests/sleep/sleep.d mode=0444
diff --git a/usr/src/test/util-tests/runfiles/default.run b/usr/src/test/util-tests/runfiles/default.run
index c4b0cf1412..4d9acd9d47 100644
--- a/usr/src/test/util-tests/runfiles/default.run
+++ b/usr/src/test/util-tests/runfiles/default.run
@@ -32,6 +32,9 @@ outputdir = /var/tmp/test_results
 [/opt/util-tests/tests/libjedec_test]
 [/opt/util-tests/tests/smbios]
 
+[/opt/util-tests/tests/show-overlay-exit]
+user = root
+
 #
 # The sleep tests require destructive DTrace. Therefore they must be run
 # as root.
diff --git a/usr/src/test/util-tests/tests/dladm/Makefile b/usr/src/test/util-tests/tests/dladm/Makefile
index 53fc2ce092..50ef1b876a 100644
--- a/usr/src/test/util-tests/tests/dladm/Makefile
+++ b/usr/src/test/util-tests/tests/dladm/Makefile
@@ -17,7 +17,7 @@ include $(SRC)/cmd/Makefile.cmd
 include $(SRC)/test/Makefile.com
 
 ROOTOPTPKG = $(ROOT)/opt/util-tests/tests
-PROG = allowed-ips set-linkprop
+PROG = allowed-ips set-linkprop show-overlay-exit
 
 ROOTPROG = $(PROG:%=$(ROOTOPTPKG)/%)
 
diff --git a/usr/src/test/util-tests/tests/dladm/show-overlay-exit.ksh b/usr/src/test/util-tests/tests/dladm/show-overlay-exit.ksh
new file mode 100644
index 0000000000..b6e9f0cbf4
--- /dev/null
+++ b/usr/src/test/util-tests/tests/dladm/show-overlay-exit.ksh
@@ -0,0 +1,83 @@
+#!/bin/ksh
+#
+# This file and its contents are supplied under the terms of the
+# Common Development and Distribution License ("CDDL"), version 1.0.
+# You may only use this file in accordance with the terms of version
+# 1.0 of the CDDL.
+#
+# A full copy of the text of the CDDL should have accompanied this
+# source.  A copy of the CDDL is also available via the Internet at
+# http://www.illumos.org/license/CDDL.
+#
+
+#
+# Copyright (c) 2017, Joyent, Inc.
+# Copyright 2022 OmniOS Community Edition (OmniOSce) Association.
+#
+
+soe_arg0="$(basename $0)"
+
+soe_overlay="soe_overlay$$"
+soe_dummy_ip="169.254.0.0"
+
+soe_port="2000"
+soe_vnetid=20
+soe_encap="vxlan"
+soe_search="direct"
+
+soe_etherstub="soe_teststub$$"
+
+function fatal
+{
+	typeset msg="$*"
+	[[ -z "$msg" ]] && msg="failed"
+	echo "TEST_FAIL: $vt_arg0: $msg" >&2
+	dladm delete-overlay $soe_overlay
+	dladm delete-etherstub $soe_etherstub
+	exit 1
+}
+
+function setup
+{
+	dladm create-overlay -t -v $soe_vnetid -e $soe_encap -s $soe_search \
+	    -p vxlan/listen_ip=$soe_dummy_ip -p direct/dest_ip=$soe_dummy_ip \
+	    -p direct/dest_port=$soe_port $soe_overlay || \
+	    fatal "failed to create overlay"
+
+	dladm create-etherstub $soe_etherstub || \
+	    fatal "failed to create etherstub"
+}
+
+function cleanup
+{
+	dladm delete-overlay $soe_overlay || \
+	    fatal "failed to remove overlay"
+	dladm delete-etherstub $soe_etherstub || \
+	    fatal "failed to remove etherstub"
+}
+
+function runtest
+{
+	dladm show-overlay $* > /dev/null 2>&1
+}
+
+function epass
+{
+	runtest $* || fatal "show-overlay=$* failed, expected success\n"
+}
+
+function efail
+{
+	runtest $* && fatal "show-overlay=$* succeeded, expected failure\n"
+}
+
+setup
+
+epass $soe_overlay
+efail $soe_etherstub
+efail $soe_etherstub $soe_overlay
+efail $soe_overlay $soe_etherstub
+
+cleanup
+
+print "TEST PASS: $soe_arg0"