netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Ian K. Coolidge" <icoolidge@google.com>
To: netdev@vger.kernel.org
Cc: ek@google.com, "Ian K. Coolidge" <icoolidge@google.com>
Subject: [PATCH v2 1/2] iproute2: ip addr: Organize flag properties structurally
Date: Wed, 27 May 2020 11:03:45 -0700	[thread overview]
Message-ID: <20200527180346.58573-1-icoolidge@google.com> (raw)
In-Reply-To: <20200524015144.44017-1-icoolidge@google.com>

This creates a nice systematic way to check that the various flags are
mutable from userspace and that the address family is valid.

Mutability properties are preserved to avoid introducing any behavioral
change in this CL. However, previously, immutable flags were ignored and
fell through to this confusing error:

Error: either "local" is duplicate, or "dadfailed" is a garbage.

But now, they just warn more explicitly:

Warning: dadfailed option is not mutable from userspace
---
 ip/ipaddress.c | 112 ++++++++++++++++++++++++-------------------------
 1 file changed, 55 insertions(+), 57 deletions(-)

diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 80d27ce2..403f7010 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -1233,52 +1233,63 @@ static unsigned int get_ifa_flags(struct ifaddrmsg *ifa,
 		ifa->ifa_flags;
 }
 
-/* Mapping from argument to address flag mask */
-static const struct {
+/* Mapping from argument to address flag mask and attributes */
+static const struct ifa_flag_data_t {
 	const char *name;
-	unsigned long value;
-} ifa_flag_names[] = {
-	{ "secondary",		IFA_F_SECONDARY },
-	{ "temporary",		IFA_F_SECONDARY },
-	{ "nodad",		IFA_F_NODAD },
-	{ "optimistic",		IFA_F_OPTIMISTIC },
-	{ "dadfailed",		IFA_F_DADFAILED },
-	{ "home",		IFA_F_HOMEADDRESS },
-	{ "deprecated",		IFA_F_DEPRECATED },
-	{ "tentative",		IFA_F_TENTATIVE },
-	{ "permanent",		IFA_F_PERMANENT },
-	{ "mngtmpaddr",		IFA_F_MANAGETEMPADDR },
-	{ "noprefixroute",	IFA_F_NOPREFIXROUTE },
-	{ "autojoin",		IFA_F_MCAUTOJOIN },
-	{ "stable-privacy",	IFA_F_STABLE_PRIVACY },
+	unsigned long mask;
+	bool readonly;
+	bool v6only;
+} ifa_flag_data[] = {
+	{ .name = "secondary",		.mask = IFA_F_SECONDARY,	.readonly = true,	.v6only = false},
+	{ .name = "temporary",		.mask = IFA_F_SECONDARY,	.readonly = true,	.v6only = false},
+	{ .name = "nodad",		.mask = IFA_F_NODAD,	 	.readonly = false,	.v6only = true},
+	{ .name = "optimistic",		.mask = IFA_F_OPTIMISTIC,	.readonly = true,	.v6only = true},
+	{ .name = "dadfailed",		.mask = IFA_F_DADFAILED,	.readonly = true,	.v6only = true},
+	{ .name = "home",		.mask = IFA_F_HOMEADDRESS,	.readonly = false,	.v6only = true},
+	{ .name = "deprecated",		.mask = IFA_F_DEPRECATED,	.readonly = true,	.v6only = true},
+	{ .name = "tentative",		.mask = IFA_F_TENTATIVE,	.readonly = true,	.v6only = true},
+	{ .name = "permanent",		.mask = IFA_F_PERMANENT,	.readonly = true,	.v6only = true},
+	{ .name = "mngtmpaddr",		.mask = IFA_F_MANAGETEMPADDR,	.readonly = false,	.v6only = true},
+	{ .name = "noprefixroute",	.mask = IFA_F_NOPREFIXROUTE,	.readonly = false,	.v6only = true},
+	{ .name = "autojoin",		.mask = IFA_F_MCAUTOJOIN,	.readonly = false,	.v6only = true},
+	{ .name = "stable-privacy",	.mask = IFA_F_STABLE_PRIVACY, 	.readonly = true,	.v6only = true},
 };
 
+/* Returns a pointer to the data structure for a particular interface flag, or null if no flag could be found */
+static const struct ifa_flag_data_t* lookup_flag_data_by_name(const char* flag_name) {
+	for (int i = 0; i < ARRAY_SIZE(ifa_flag_data); ++i) {
+		if (strcmp(flag_name, ifa_flag_data[i].name) == 0)
+			return &ifa_flag_data[i];
+	}
+        return NULL;
+}
+
 static void print_ifa_flags(FILE *fp, const struct ifaddrmsg *ifa,
 			    unsigned int flags)
 {
 	unsigned int i;
 
-	for (i = 0; i < ARRAY_SIZE(ifa_flag_names); i++) {
-		unsigned long mask = ifa_flag_names[i].value;
+	for (i = 0; i < ARRAY_SIZE(ifa_flag_data); i++) {
+		const struct ifa_flag_data_t* flag_data = &ifa_flag_data[i];
 
-		if (mask == IFA_F_PERMANENT) {
-			if (!(flags & mask))
+		if (flag_data->mask == IFA_F_PERMANENT) {
+			if (!(flags & flag_data->mask))
 				print_bool(PRINT_ANY,
 					   "dynamic", "dynamic ", true);
-		} else if (flags & mask) {
-			if (mask == IFA_F_SECONDARY &&
+		} else if (flags & flag_data->mask) {
+			if (flag_data->mask == IFA_F_SECONDARY &&
 			    ifa->ifa_family == AF_INET6) {
 				print_bool(PRINT_ANY,
 					   "temporary", "temporary ", true);
 			} else {
 				print_string(PRINT_FP, NULL,
-					     "%s ", ifa_flag_names[i].name);
+					     "%s ", flag_data->name);
 				print_bool(PRINT_JSON,
-					   ifa_flag_names[i].name, NULL, true);
+					   flag_data->name, NULL, true);
 			}
 		}
 
-		flags &= ~mask;
+		flags &= ~flag_data->mask;
 	}
 
 	if (flags) {
@@ -1297,7 +1308,6 @@ static void print_ifa_flags(FILE *fp, const struct ifaddrmsg *ifa,
 static int get_filter(const char *arg)
 {
 	bool inv = false;
-	unsigned int i;
 
 	if (arg[0] == '-') {
 		inv = true;
@@ -1313,18 +1323,16 @@ static int get_filter(const char *arg)
 		arg = "secondary";
 	}
 
-	for (i = 0; i < ARRAY_SIZE(ifa_flag_names); i++) {
-		if (strcmp(arg, ifa_flag_names[i].name))
-			continue;
+	const struct ifa_flag_data_t* flag_data = lookup_flag_data_by_name(arg);
+	if (flag_data == NULL)
+		return -1;
 
-		if (inv)
-			filter.flags &= ~ifa_flag_names[i].value;
-		else
-			filter.flags |= ifa_flag_names[i].value;
-		filter.flagmask |= ifa_flag_names[i].value;
-		return 0;
-	}
-	return -1;
+	if (inv)
+		filter.flags &= ~flag_data->mask;
+	else
+		filter.flags |= flag_data->mask;
+	filter.flagmask |= flag_data->mask;
+	return 0;
 }
 
 static int ifa_label_match_rta(int ifindex, const struct rtattr *rta)
@@ -2330,25 +2338,15 @@ static int ipaddr_modify(int cmd, int flags, int argc, char **argv)
 			preferred_lftp = *argv;
 			if (set_lifetime(&preferred_lft, *argv))
 				invarg("preferred_lft value", *argv);
-		} else if (strcmp(*argv, "home") == 0) {
-			if (req.ifa.ifa_family == AF_INET6)
-				ifa_flags |= IFA_F_HOMEADDRESS;
-			else
-				fprintf(stderr, "Warning: home option can be set only for IPv6 addresses\n");
-		} else if (strcmp(*argv, "nodad") == 0) {
-			if (req.ifa.ifa_family == AF_INET6)
-				ifa_flags |= IFA_F_NODAD;
-			else
-				fprintf(stderr, "Warning: nodad option can be set only for IPv6 addresses\n");
-		} else if (strcmp(*argv, "mngtmpaddr") == 0) {
-			if (req.ifa.ifa_family == AF_INET6)
-				ifa_flags |= IFA_F_MANAGETEMPADDR;
-			else
-				fprintf(stderr, "Warning: mngtmpaddr option can be set only for IPv6 addresses\n");
-		} else if (strcmp(*argv, "noprefixroute") == 0) {
-			ifa_flags |= IFA_F_NOPREFIXROUTE;
-		} else if (strcmp(*argv, "autojoin") == 0) {
-			ifa_flags |= IFA_F_MCAUTOJOIN;
+		} else if (lookup_flag_data_by_name(*argv)) {
+			const struct ifa_flag_data_t* flag_data = lookup_flag_data_by_name(*argv);
+			if (flag_data->readonly) {
+				fprintf(stderr, "Warning: %s option is not mutable from userspace\n", flag_data->name);
+			} else if (flag_data->v6only && req.ifa.ifa_family != AF_INET6) {
+				fprintf(stderr, "Warning: %s option can be set only for IPv6 addresses\n", flag_data->name);
+			} else {
+				ifa_flags |= flag_data->mask;
+			}
 		} else {
 			if (strcmp(*argv, "local") == 0)
 				NEXT_ARG();
-- 
2.27.0.rc0.183.gde8f92d652-goog


  parent reply	other threads:[~2020-05-27 18:03 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-24  1:51 [PATCH] iproute2: ip addr: Accept 'optimistic' flag Ian K. Coolidge
2020-05-24  5:06 ` Erik Kline
2020-05-24 19:09 ` Stephen Hemminger
2020-05-27 18:03 ` Ian K. Coolidge [this message]
2020-05-27 18:03   ` [PATCH v2 2/2] iproute2: ip addr: Add support for setting 'optimistic' Ian K. Coolidge
2020-05-31 23:09   ` [PATCH v2 1/2] iproute2: ip addr: Organize flag properties structurally David Ahern

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200527180346.58573-1-icoolidge@google.com \
    --to=icoolidge@google.com \
    --cc=ek@google.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).