ethtool: Handle more errors when getting feature names

'ethtool -k eth0' generally works for unprivileged users, but in some
kernel versions ETHTOOL_GSSET_INFO wrongly returns -EPERM.  We need
to fall back to the old offload operations in this case.  Also, if
we get some other error code, report that before exiting.

Add regression test cases covering this and some other operations
that may fail (which were already handled correctly).

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>

2 files changed, 38 insertions, 2 deletions

diff --git a/ethtool.c b/ethtool.c
index 984273b..3c34273 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -1360,6 +1360,10 @@ static struct feature_defs *get_feature_defs(struct cmd_context *ctx)
 	} else if (errno == EOPNOTSUPP || errno == EINVAL) {
 		/* Kernel doesn't support named features; not an error */
 		n_features = 0;
+	} else if (errno == EPERM) {
+		/* Kernel bug: ETHTOOL_GSSET_INFO was privileged.
+		 * Work around it. */
+		n_features = 0;
 	} else {
 		return NULL;
 	}
@@ -1873,8 +1877,10 @@ static int do_gfeatures(struct cmd_context *ctx)
 		exit_bad_args();
 
 	defs = get_feature_defs(ctx);
-	if (!defs)
+	if (!defs) {
+		perror("Cannot get device feature names");
 		return 1;
+	}
 
 	fprintf(stdout, "Features for %s:\n", ctx->devname);
 
@@ -1902,8 +1908,10 @@ static int do_sfeatures(struct cmd_context *ctx)
 	int i, j;
 
 	defs = get_feature_defs(ctx);
-	if (!defs)
+	if (!defs) {
+		perror("Cannot get device feature names");
 		return 1;
+	}
 	if (defs->n_features) {
 		efeatures = malloc(sizeof(*efeatures) +
 				   FEATURE_BITS_TO_BLOCKS(defs->n_features) *
diff --git a/test-features.c b/test-features.c
index a48c701..d7bd994 100644
--- a/test-features.c
+++ b/test-features.c
@@ -80,6 +80,32 @@ static const struct cmd_expect cmd_expect_get_features_off_old[] = {
 	{ 0, 0, 0, 0, 0 }
 };
 
+static const struct cmd_expect cmd_expect_get_features_off_old_some_unsup[] = {
+	{ &cmd_gssetinfo, sizeof(cmd_gssetinfo.cmd), -EINVAL },
+	{ &cmd_grxcsum_off, 4, 0, &cmd_grxcsum_off, sizeof(cmd_grxcsum_off) },
+	{ &cmd_gtxcsum_off, 4, 0, &cmd_gtxcsum_off, sizeof(cmd_gtxcsum_off) },
+	{ &cmd_gsg_off, 4, 0, &cmd_gsg_off, sizeof(cmd_gsg_off) },
+	{ &cmd_gtso_off, 4, 0, &cmd_gtso_off, sizeof(cmd_gtso_off) },
+	{ &cmd_gufo_off, 4, -EOPNOTSUPP },
+	{ &cmd_ggso_off, 4, 0, &cmd_ggso_off, sizeof(cmd_ggso_off) },
+	{ &cmd_ggro_off, 4, -EOPNOTSUPP },
+	{ &cmd_gflags_off, 4, 0, &cmd_gflags_off, sizeof(cmd_gflags_off) },
+	{ 0, 0, 0, 0, 0 }
+};
+
+static const struct cmd_expect cmd_expect_get_features_off_old_some_priv[] = {
+	{ &cmd_gssetinfo, sizeof(cmd_gssetinfo.cmd), -EPERM },
+	{ &cmd_grxcsum_off, 4, 0, &cmd_grxcsum_off, sizeof(cmd_grxcsum_off) },
+	{ &cmd_gtxcsum_off, 4, 0, &cmd_gtxcsum_off, sizeof(cmd_gtxcsum_off) },
+	{ &cmd_gsg_off, 4, 0, &cmd_gsg_off, sizeof(cmd_gsg_off) },
+	{ &cmd_gtso_off, 4, 0, &cmd_gtso_off, sizeof(cmd_gtso_off) },
+	{ &cmd_gufo_off, 4, 0, &cmd_gufo_off, sizeof(cmd_gufo_off) },
+	{ &cmd_ggso_off, 4, 0, &cmd_ggso_off, sizeof(cmd_ggso_off) },
+	{ &cmd_ggro_off, 4, -EPERM },
+	{ &cmd_gflags_off, 4, 0, &cmd_gflags_off, sizeof(cmd_gflags_off) },
+	{ 0, 0, 0, 0, 0 }
+};
+
 static const struct cmd_expect cmd_expect_set_features_off_old[] = {
 	{ &cmd_gssetinfo, sizeof(cmd_gssetinfo.cmd), -EINVAL },
 	{ &cmd_grxcsum_on, 4, 0, &cmd_grxcsum_on, sizeof(cmd_grxcsum_on) },
@@ -443,6 +469,8 @@ static struct test_case {
 	const struct cmd_expect *expect;
 } const test_cases[] = {
 	{ 0, "-k devname", cmd_expect_get_features_off_old },
+	{ 0, "-k dev_unsup", cmd_expect_get_features_off_old_some_unsup },
+	{ 0, "-k dev_priv", cmd_expect_get_features_off_old_some_priv },
 	{ 0, "-K devname rx off tx off sg off tso off ufo off gso off lro off rxvlan off txvlan off ntuple off rxhash off gro off",
 	  cmd_expect_set_features_off_old },
 	{ 0, "-K devname rx on tx on sg on tso on ufo on gso on lro on rxvlan on txvlan on ntuple on rxhash on gro on",