netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] iproute2: ip addr: Accept 'optimistic' flag
@ 2020-05-24  1:51 Ian K. Coolidge
  2020-05-24  5:06 ` Erik Kline
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Ian K. Coolidge @ 2020-05-24  1:51 UTC (permalink / raw)
  To: netdev; +Cc: Ian K. Coolidge

This allows addresses added to use IPv6 optimistic DAD.
---
 ip/ipaddress.c           | 7 ++++++-
 man/man8/ip-address.8.in | 7 ++++++-
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 80d27ce2..48cf5e41 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -72,7 +72,7 @@ static void usage(void)
 		"           [-]tentative | [-]deprecated | [-]dadfailed | temporary |\n"
 		"           CONFFLAG-LIST ]\n"
 		"CONFFLAG-LIST := [ CONFFLAG-LIST ] CONFFLAG\n"
-		"CONFFLAG  := [ home | nodad | mngtmpaddr | noprefixroute | autojoin ]\n"
+		"CONFFLAG  := [ home | nodad | optimistic | mngtmpaddr | noprefixroute | autojoin ]\n"
 		"LIFETIME := [ valid_lft LFT ] [ preferred_lft LFT ]\n"
 		"LFT := forever | SECONDS\n"
 		"TYPE := { vlan | veth | vcan | vxcan | dummy | ifb | macvlan | macvtap |\n"
@@ -2335,6 +2335,11 @@ static int ipaddr_modify(int cmd, int flags, int argc, char **argv)
 				ifa_flags |= IFA_F_HOMEADDRESS;
 			else
 				fprintf(stderr, "Warning: home option can be set only for IPv6 addresses\n");
+		} else if (strcmp(*argv, "optimistic") == 0) {
+			if (req.ifa.ifa_family == AF_INET6)
+				ifa_flags |= IFA_F_OPTIMISTIC;
+			else
+				fprintf(stderr, "Warning: optimistic 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;
diff --git a/man/man8/ip-address.8.in b/man/man8/ip-address.8.in
index 2a553190..fe773c91 100644
--- a/man/man8/ip-address.8.in
+++ b/man/man8/ip-address.8.in
@@ -92,7 +92,7 @@ ip-address \- protocol address management
 
 .ti -8
 .IR CONFFLAG " := "
-.RB "[ " home " | " mngtmpaddr " | " nodad " | " noprefixroute " | " autojoin " ]"
+.RB "[ " home " | " mngtmpaddr " | " nodad " | " optimstic " | " noprefixroute " | " autojoin " ]"
 
 .ti -8
 .IR LIFETIME " := [ "
@@ -258,6 +258,11 @@ stateless auto-configuration was active.
 (IPv6 only) do not perform Duplicate Address Detection (RFC 4862) when
 adding this address.
 
+.TP
+.B optimistic
+(IPv6 only) When performing Duplicate Address Detection, use the RFC 4429
+optimistic variant.
+
 .TP
 .B noprefixroute
 Do not automatically create a route for the network prefix of the added
-- 
2.27.0.rc0.183.gde8f92d652-goog


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] iproute2: ip addr: Accept 'optimistic' flag
  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 ` [PATCH v2 1/2] iproute2: ip addr: Organize flag properties structurally Ian K. Coolidge
  2 siblings, 0 replies; 6+ messages in thread
From: Erik Kline @ 2020-05-24  5:06 UTC (permalink / raw)
  To: Ian K. Coolidge; +Cc: netdev

[-- Attachment #1: Type: text/plain, Size: 2894 bytes --]

patched and tested on a 5.6.13 system

Acked-by: Erik Kline <ek@google.com>


On Sat, 23 May 2020 at 19:01, Ian K. Coolidge <icoolidge@google.com> wrote:
>
> This allows addresses added to use IPv6 optimistic DAD.
> ---
>  ip/ipaddress.c           | 7 ++++++-
>  man/man8/ip-address.8.in | 7 ++++++-
>  2 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/ip/ipaddress.c b/ip/ipaddress.c
> index 80d27ce2..48cf5e41 100644
> --- a/ip/ipaddress.c
> +++ b/ip/ipaddress.c
> @@ -72,7 +72,7 @@ static void usage(void)
>                 "           [-]tentative | [-]deprecated | [-]dadfailed | temporary |\n"
>                 "           CONFFLAG-LIST ]\n"
>                 "CONFFLAG-LIST := [ CONFFLAG-LIST ] CONFFLAG\n"
> -               "CONFFLAG  := [ home | nodad | mngtmpaddr | noprefixroute | autojoin ]\n"
> +               "CONFFLAG  := [ home | nodad | optimistic | mngtmpaddr | noprefixroute | autojoin ]\n"
>                 "LIFETIME := [ valid_lft LFT ] [ preferred_lft LFT ]\n"
>                 "LFT := forever | SECONDS\n"
>                 "TYPE := { vlan | veth | vcan | vxcan | dummy | ifb | macvlan | macvtap |\n"
> @@ -2335,6 +2335,11 @@ static int ipaddr_modify(int cmd, int flags, int argc, char **argv)
>                                 ifa_flags |= IFA_F_HOMEADDRESS;
>                         else
>                                 fprintf(stderr, "Warning: home option can be set only for IPv6 addresses\n");
> +               } else if (strcmp(*argv, "optimistic") == 0) {
> +                       if (req.ifa.ifa_family == AF_INET6)
> +                               ifa_flags |= IFA_F_OPTIMISTIC;
> +                       else
> +                               fprintf(stderr, "Warning: optimistic 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;
> diff --git a/man/man8/ip-address.8.in b/man/man8/ip-address.8.in
> index 2a553190..fe773c91 100644
> --- a/man/man8/ip-address.8.in
> +++ b/man/man8/ip-address.8.in
> @@ -92,7 +92,7 @@ ip-address \- protocol address management
>
>  .ti -8
>  .IR CONFFLAG " := "
> -.RB "[ " home " | " mngtmpaddr " | " nodad " | " noprefixroute " | " autojoin " ]"
> +.RB "[ " home " | " mngtmpaddr " | " nodad " | " optimstic " | " noprefixroute " | " autojoin " ]"
>
>  .ti -8
>  .IR LIFETIME " := [ "
> @@ -258,6 +258,11 @@ stateless auto-configuration was active.
>  (IPv6 only) do not perform Duplicate Address Detection (RFC 4862) when
>  adding this address.
>
> +.TP
> +.B optimistic
> +(IPv6 only) When performing Duplicate Address Detection, use the RFC 4429
> +optimistic variant.
> +
>  .TP
>  .B noprefixroute
>  Do not automatically create a route for the network prefix of the added
> --
> 2.27.0.rc0.183.gde8f92d652-goog
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 3842 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] iproute2: ip addr: Accept 'optimistic' flag
  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 ` [PATCH v2 1/2] iproute2: ip addr: Organize flag properties structurally Ian K. Coolidge
  2 siblings, 0 replies; 6+ messages in thread
From: Stephen Hemminger @ 2020-05-24 19:09 UTC (permalink / raw)
  To: Ian K. Coolidge; +Cc: netdev

On Sat, 23 May 2020 18:51:44 -0700
"Ian K. Coolidge" <icoolidge@google.com> wrote:

> This allows addresses added to use IPv6 optimistic DAD.
> ---
>  ip/ipaddress.c           | 7 ++++++-
>  man/man8/ip-address.8.in | 7 ++++++-
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/ip/ipaddress.c b/ip/ipaddress.c
> index 80d27ce2..48cf5e41 100644
> --- a/ip/ipaddress.c
> +++ b/ip/ipaddress.c
> @@ -72,7 +72,7 @@ static void usage(void)
>  		"           [-]tentative | [-]deprecated | [-]dadfailed | temporary |\n"
>  		"           CONFFLAG-LIST ]\n"
>  		"CONFFLAG-LIST := [ CONFFLAG-LIST ] CONFFLAG\n"
> -		"CONFFLAG  := [ home | nodad | mngtmpaddr | noprefixroute | autojoin ]\n"
> +		"CONFFLAG  := [ home | nodad | optimistic | mngtmpaddr | noprefixroute | autojoin ]\n"
>  		"LIFETIME := [ valid_lft LFT ] [ preferred_lft LFT ]\n"
>  		"LFT := forever | SECONDS\n"
>  		"TYPE := { vlan | veth | vcan | vxcan | dummy | ifb | macvlan | macvtap |\n"
> @@ -2335,6 +2335,11 @@ static int ipaddr_modify(int cmd, int flags, int argc, char **argv)
>  				ifa_flags |= IFA_F_HOMEADDRESS;
>  			else
>  				fprintf(stderr, "Warning: home option can be set only for IPv6 addresses\n");
> +		} else if (strcmp(*argv, "optimistic") == 0) {
> +			if (req.ifa.ifa_family == AF_INET6)
> +				ifa_flags |= IFA_F_OPTIMISTIC;
> +			else
> +				fprintf(stderr, "Warning: optimistic 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;
> diff --git a/man/man8/ip-address.8.in b/man/man8/ip-address.8.in
> index 2a553190..fe773c91 100644
> --- a/man/man8/ip-address.8.in
> +++ b/man/man8/ip-address.8.in
> @@ -92,7 +92,7 @@ ip-address \- protocol address management
>  
>  .ti -8
>  .IR CONFFLAG " := "
> -.RB "[ " home " | " mngtmpaddr " | " nodad " | " noprefixroute " | " autojoin " ]"
> +.RB "[ " home " | " mngtmpaddr " | " nodad " | " optimstic " | " noprefixroute " | " autojoin " ]"
>  
>  .ti -8
>  .IR LIFETIME " := [ "
> @@ -258,6 +258,11 @@ stateless auto-configuration was active.
>  (IPv6 only) do not perform Duplicate Address Detection (RFC 4862) when
>  adding this address.
>  
> +.TP
> +.B optimistic
> +(IPv6 only) When performing Duplicate Address Detection, use the RFC 4429
> +optimistic variant.
> +
>  .TP
>  .B noprefixroute
>  Do not automatically create a route for the network prefix of the added

Since there already is a table for print() code, could that be used for parse()?
Something like the following UNTESTED

diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 80d27ce27d0c..debb83157d60 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -1236,7 +1236,7 @@ static unsigned int get_ifa_flags(struct ifaddrmsg *ifa,
 /* Mapping from argument to address flag mask */
 static const struct {
 	const char *name;
-	unsigned long value;
+	unsigned int value;
 } ifa_flag_names[] = {
 	{ "secondary",		IFA_F_SECONDARY },
 	{ "temporary",		IFA_F_SECONDARY },
@@ -1253,13 +1253,46 @@ static const struct {
 	{ "stable-privacy",	IFA_F_STABLE_PRIVACY },
 };
 
+#define IPV6ONLY_FLAGS	\
+		(IFA_F_NODAD | IFA_F_OPTIMISTIC | IFA_F_DADFAILED | \
+		 IFA_F_HOMEADDRESS | IFA_F_TENTATIVE | \
+		 IFA_F_MANAGETEMPADDR | IFA_F_STABLE_PRIVACY)
+
+#define READONLY_FLAGS \
+	( IFA_F_SECONDARY | IFA_F_DADFAILED | IFA_F_DEPRECATED )
+
+static int parse_ifa_flags(int family, const char *arg, unsigned int *flags)
+{
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(ifa_flag_names); i++) {
+		const char *name = ifa_flag_names[i].name;
+		unsigned int mask = ifa_flag_names[i].value;
+
+		if (strcasecmp(arg, name))
+			continue;
+
+		if (mask & READONLY_FLAGS)
+			fprintf(stderr,
+				"Warning: %s flag can not be set.\n", name);
+		else if ((mask & IPV6ONLY_FLAGS) && family != AF_INET6)
+			fprintf(stderr,
+				"Warning: %s flag can be set only for IPV6 addresses\n",
+				name);
+		else
+			*flags |= mask;
+		return 0;
+	}
+	return -1;
+}
+
 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;
+		unsigned int mask = ifa_flag_names[i].value;
 
 		if (mask == IFA_F_PERMANENT) {
 			if (!(flags & mask))
@@ -2330,26 +2363,7 @@ 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 {
+		} else if (parse_ifa_flags(req.ifa.ifa_family, *argv, &ifa_flags) != 0) {
 			if (strcmp(*argv, "local") == 0)
 				NEXT_ARG();
 			if (matches(*argv, "help") == 0)

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 1/2] iproute2: ip addr: Organize flag properties structurally
  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
  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
  2 siblings, 2 replies; 6+ messages in thread
From: Ian K. Coolidge @ 2020-05-27 18:03 UTC (permalink / raw)
  To: netdev; +Cc: ek, Ian K. Coolidge

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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 2/2] iproute2: ip addr: Add support for setting 'optimistic'
  2020-05-27 18:03 ` [PATCH v2 1/2] iproute2: ip addr: Organize flag properties structurally Ian K. Coolidge
@ 2020-05-27 18:03   ` Ian K. Coolidge
  2020-05-31 23:09   ` [PATCH v2 1/2] iproute2: ip addr: Organize flag properties structurally David Ahern
  1 sibling, 0 replies; 6+ messages in thread
From: Ian K. Coolidge @ 2020-05-27 18:03 UTC (permalink / raw)
  To: netdev; +Cc: ek, Ian K. Coolidge

optimistic DAD is controllable via sysctl for an interface
or all interfaces on the system. This would affect addresses
added by the kernel only.

Recent kernels, however, have enabled support for adding optimistic
address via userspace. This plumbs that support.
---
 ip/ipaddress.c           | 2 +-
 man/man8/ip-address.8.in | 7 ++++++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 403f7010..3b53933f 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -1243,7 +1243,7 @@ static const struct ifa_flag_data_t {
 	{ .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 = "optimistic",		.mask = IFA_F_OPTIMISTIC,	.readonly = false,	.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},
diff --git a/man/man8/ip-address.8.in b/man/man8/ip-address.8.in
index 2a553190..fe773c91 100644
--- a/man/man8/ip-address.8.in
+++ b/man/man8/ip-address.8.in
@@ -92,7 +92,7 @@ ip-address \- protocol address management
 
 .ti -8
 .IR CONFFLAG " := "
-.RB "[ " home " | " mngtmpaddr " | " nodad " | " noprefixroute " | " autojoin " ]"
+.RB "[ " home " | " mngtmpaddr " | " nodad " | " optimstic " | " noprefixroute " | " autojoin " ]"
 
 .ti -8
 .IR LIFETIME " := [ "
@@ -258,6 +258,11 @@ stateless auto-configuration was active.
 (IPv6 only) do not perform Duplicate Address Detection (RFC 4862) when
 adding this address.
 
+.TP
+.B optimistic
+(IPv6 only) When performing Duplicate Address Detection, use the RFC 4429
+optimistic variant.
+
 .TP
 .B noprefixroute
 Do not automatically create a route for the network prefix of the added
-- 
2.27.0.rc0.183.gde8f92d652-goog


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/2] iproute2: ip addr: Organize flag properties structurally
  2020-05-27 18:03 ` [PATCH v2 1/2] iproute2: ip addr: Organize flag properties structurally Ian K. Coolidge
  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   ` David Ahern
  1 sibling, 0 replies; 6+ messages in thread
From: David Ahern @ 2020-05-31 23:09 UTC (permalink / raw)
  To: Ian K. Coolidge, netdev; +Cc: ek

On 5/27/20 12:03 PM, Ian K. Coolidge wrote:
> 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(-)
> 

applied both to iproute2-next. Thanks,

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-05-31 23:10 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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 ` [PATCH v2 1/2] iproute2: ip addr: Organize flag properties structurally Ian K. Coolidge
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

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).