Re: [PATCH] iproute2: ip addr: Accept 'optimistic' flag

[Stephen Hemminger <stephen@networkplumber.org>]

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)