Re: [ovs-dev] [PATCH] Support IPv6 link-local address scopes on Linux.

[Ben Pfaff]

On Wed, Jul 12, 2017 at 07:11:31AM +, Darrell Ball wrote:
> 
> 
> On 7/11/17, 9:23 PM, "Ben Pfaff"  wrote:
> 
> Thanks for the review.
> 
> I'm very new to the concept of IPv6 link-local address scoping.  What
> does this subset out of the larger feature?
> 
> There is lots of flexibility here, mostly not relevant to ovs; for example 
> one basic
> zone mapping option is the ifindex directly.
> It can be used for direct mapping to zone id.
> If this option of encoding were to be additionally supported, it would mean
> something like 
> 
> sudo ovs-vsctl set-controller s1 'tcp:[fe80::41f3:ab56:bbab:a528%1]'
> where ‘1’ is the ifindex
> is another option.
> 
> In this case, it means the scope string converted to int would be the
> ifindex directly, something like
> 
> > +char *scope = strsep(_s, "%");
> > +if (scope && *scope) {
> > +sin6->sin6_scope_id = str_to_int(scope);
> 
> I did not elaborate earlier since ifindex is mostly secondary in ovs

OK, I see.

I realized that this patch was incomplete: it accepted scope
identifiers, but it dropped them if they needed to be formatted later.
But for formatting, it's unsafe to just randomly put in any netdev name
(it might contain confusing punctuation), so in a case like that one
needs to use the ifindex.  And once that's in there, for consistency one
should accept ifindex values also.

Anyway, to fix all of that, I respun this as a 3-patch series, starting here:
https://mail.openvswitch.org/pipermail/ovs-dev/2017-July/335663.html
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] Support IPv6 link-local address scopes on Linux.

[Darrell Ball]

On 7/11/17, 9:23 PM, "Ben Pfaff"  wrote:

Thanks for the review.

I'm very new to the concept of IPv6 link-local address scoping.  What
does this subset out of the larger feature?

There is lots of flexibility here, mostly not relevant to ovs; for example one 
basic
zone mapping option is the ifindex directly.
It can be used for direct mapping to zone id.
If this option of encoding were to be additionally supported, it would mean
something like 

sudo ovs-vsctl set-controller s1 'tcp:[fe80::41f3:ab56:bbab:a528%1]'
where ‘1’ is the ifindex
is another option.

In this case, it means the scope string converted to int would be the
ifindex directly, something like

> +char *scope = strsep(_s, "%");
> +if (scope && *scope) {
> +sin6->sin6_scope_id = str_to_int(scope);

I did not elaborate earlier since ifindex is mostly secondary in ovs


On Tue, Jul 11, 2017 at 11:30:29PM +, Darrell Ball wrote:
> This looks correct to me and a reasonable subset of support of scoping, 
although I did not test it.
> I noticed there was some unrelated cleanup w.r.t. 
parse_sockaddr_components(), but those
> changes are minor.
> 
> Acked-by: Darrell Ball 
> 
> 
> On 7/5/17, 5:20 PM, "ovs-dev-boun...@openvswitch.org on behalf of Ben 
Pfaff"  wrote:
> 
> I hadn't even heard of this feature before, but it seems to be at 
least
> semi-standard to support Linux link-local address scopes via a % 
suffix,
> e.g. fe80::1234%eth0 for a link-local address scoped to eth0.  This 
commit
> adds support.
> 
> I'd appreciate feedback from folks who understand this feature better 
than
> me.
> 
> Reported-by: Ali Volkan Atli 
> Signed-off-by: Ben Pfaff 
> ---
>  NEWS |  2 ++
>  configure.ac |  3 +++
>  lib/socket-util.c| 31 ++-
>  lib/vconn-active.man |  7 +--
>  lib/vconn-passive.man| 10 ++
>  ovsdb/remote-active.man  | 20 +---
>  ovsdb/remote-passive.man | 29 +++--
>  7 files changed, 58 insertions(+), 44 deletions(-)
> 
> diff --git a/NEWS b/NEWS
> index 0f2604ff5c51..c8c14481a1db 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -66,6 +66,8 @@ Post-v2.7.0
> - Add experimental support for hardware offloading
>   * HW offloading is disabled by default.
>   * HW offloading is done through the TC interface.
> +   - IPv6 link local addresses are now supported on Linux.  Use % to 
designate
> + the scope device.
>  
>  v2.7.0 - 21 Feb 2017
>  -
> diff --git a/configure.ac b/configure.ac
> index 6404b5fc1222..9f78184f52a7 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -106,6 +106,9 @@ AC_CHECK_DECLS([sys_siglist], [], [], [[#include 
]])
>  AC_CHECK_MEMBERS([struct stat.st_mtim.tv_nsec, struct 
stat.st_mtimensec],
>[], [], [[#include ]])
>  AC_CHECK_MEMBERS([struct ifreq.ifr_flagshigh], [], [], [[#include 
]])
> +AC_CHECK_MEMBERS([struct sockaddr_in6.sin6_scope_id], [], [],
> +  [[#include 
> +#include ]])
>  AC_CHECK_FUNCS([mlockall strnlen getloadavg statvfs getmntent_r])
>  AC_CHECK_HEADERS([mntent.h sys/statvfs.h linux/types.h 
linux/if_ether.h stdatomic.h])
>  AC_CHECK_HEADERS([net/if_mib.h], [], [], [[#include 
> diff --git a/lib/socket-util.c b/lib/socket-util.c
> index 2c0f1e62bd99..fbed172bdefb 100644
> --- a/lib/socket-util.c
> +++ b/lib/socket-util.c
> @@ -365,7 +365,7 @@ parse_bracketed_token(char **pp)
>  
>  static bool
>  parse_sockaddr_components(struct sockaddr_storage *ss,
> -  const char *host_s,
> +  char *host_s,
>const char *port_s, uint16_t default_port,
>const char *s)
>  {
> @@ -382,20 +382,34 @@ parse_sockaddr_components(struct 
sockaddr_storage *ss,
>  }
>  
>  memset(ss, 0, sizeof *ss);
> -if (strchr(host_s, ':')) {
> +if (host_s && strchr(host_s, ':')) {
>  struct sockaddr_in6 *sin6
>  = ALIGNED_CAST(struct sockaddr_in6 *, ss);
>  
> +char *addr = strsep(_s, "%");
> +
>  sin6->sin6_family = AF_INET6;
>  sin6->sin6_port = htons(port);
> -if (!ipv6_parse(host_s, 

Re: [ovs-dev] [PATCH] Support IPv6 link-local address scopes on Linux.

[Ben Pfaff]

Thanks for the review.

I'm very new to the concept of IPv6 link-local address scoping.  What
does this subset out of the larger feature?

On Tue, Jul 11, 2017 at 11:30:29PM +, Darrell Ball wrote:
> This looks correct to me and a reasonable subset of support of scoping, 
> although I did not test it.
> I noticed there was some unrelated cleanup w.r.t. 
> parse_sockaddr_components(), but those
> changes are minor.
> 
> Acked-by: Darrell Ball 
> 
> 
> On 7/5/17, 5:20 PM, "ovs-dev-boun...@openvswitch.org on behalf of Ben Pfaff" 
>  wrote:
> 
> I hadn't even heard of this feature before, but it seems to be at least
> semi-standard to support Linux link-local address scopes via a % suffix,
> e.g. fe80::1234%eth0 for a link-local address scoped to eth0.  This commit
> adds support.
> 
> I'd appreciate feedback from folks who understand this feature better than
> me.
> 
> Reported-by: Ali Volkan Atli 
> Signed-off-by: Ben Pfaff 
> ---
>  NEWS |  2 ++
>  configure.ac |  3 +++
>  lib/socket-util.c| 31 ++-
>  lib/vconn-active.man |  7 +--
>  lib/vconn-passive.man| 10 ++
>  ovsdb/remote-active.man  | 20 +---
>  ovsdb/remote-passive.man | 29 +++--
>  7 files changed, 58 insertions(+), 44 deletions(-)
> 
> diff --git a/NEWS b/NEWS
> index 0f2604ff5c51..c8c14481a1db 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -66,6 +66,8 @@ Post-v2.7.0
> - Add experimental support for hardware offloading
>   * HW offloading is disabled by default.
>   * HW offloading is done through the TC interface.
> +   - IPv6 link local addresses are now supported on Linux.  Use % to 
> designate
> + the scope device.
>  
>  v2.7.0 - 21 Feb 2017
>  -
> diff --git a/configure.ac b/configure.ac
> index 6404b5fc1222..9f78184f52a7 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -106,6 +106,9 @@ AC_CHECK_DECLS([sys_siglist], [], [], [[#include 
> ]])
>  AC_CHECK_MEMBERS([struct stat.st_mtim.tv_nsec, struct stat.st_mtimensec],
>[], [], [[#include ]])
>  AC_CHECK_MEMBERS([struct ifreq.ifr_flagshigh], [], [], [[#include 
> ]])
> +AC_CHECK_MEMBERS([struct sockaddr_in6.sin6_scope_id], [], [],
> +  [[#include 
> +#include ]])
>  AC_CHECK_FUNCS([mlockall strnlen getloadavg statvfs getmntent_r])
>  AC_CHECK_HEADERS([mntent.h sys/statvfs.h linux/types.h linux/if_ether.h 
> stdatomic.h])
>  AC_CHECK_HEADERS([net/if_mib.h], [], [], [[#include 
> diff --git a/lib/socket-util.c b/lib/socket-util.c
> index 2c0f1e62bd99..fbed172bdefb 100644
> --- a/lib/socket-util.c
> +++ b/lib/socket-util.c
> @@ -365,7 +365,7 @@ parse_bracketed_token(char **pp)
>  
>  static bool
>  parse_sockaddr_components(struct sockaddr_storage *ss,
> -  const char *host_s,
> +  char *host_s,
>const char *port_s, uint16_t default_port,
>const char *s)
>  {
> @@ -382,20 +382,34 @@ parse_sockaddr_components(struct sockaddr_storage 
> *ss,
>  }
>  
>  memset(ss, 0, sizeof *ss);
> -if (strchr(host_s, ':')) {
> +if (host_s && strchr(host_s, ':')) {
>  struct sockaddr_in6 *sin6
>  = ALIGNED_CAST(struct sockaddr_in6 *, ss);
>  
> +char *addr = strsep(_s, "%");
> +
>  sin6->sin6_family = AF_INET6;
>  sin6->sin6_port = htons(port);
> -if (!ipv6_parse(host_s, >sin6_addr)) {
> -VLOG_ERR("%s: bad IPv6 address \"%s\"", s, host_s);
> +if (!addr || !*addr || !ipv6_parse(addr, >sin6_addr)) {
> +VLOG_ERR("%s: bad IPv6 address \"%s\"", s, addr ? addr : "");
>  goto exit;
>  }
> +
> +#ifdef HAVE_STRUCT_SOCKADDR_IN6_SIN6_SCOPE_ID
> +char *scope = strsep(_s, "%");
> +if (scope && *scope) {
> +sin6->sin6_scope_id = if_nametoindex(scope);
> +if (!sin6->sin6_scope_id) {
> +VLOG_ERR("%s: bad IPv6 scope \"%s\" (%s)",
> + s, scope, ovs_strerror(errno));
> +goto exit;
> +}
> +}
> +#endif
>  } else {
>  sin->sin_family = AF_INET;
>  sin->sin_port = htons(port);
> -if (!ip_parse(host_s, >sin_addr.s_addr)) {
> +if (host_s && !ip_parse(host_s, >sin_addr.s_addr)) {
>  VLOG_ERR("%s: bad IPv4 address \"%s\"", s, host_s);
>  goto exit;
>  }
> @@ -421,7 

Re: [ovs-dev] [PATCH] Support IPv6 link-local address scopes on Linux.

[Darrell Ball]

This looks correct to me and a reasonable subset of support of scoping, 
although I did not test it.
I noticed there was some unrelated cleanup w.r.t. parse_sockaddr_components(), 
but those
changes are minor.

Acked-by: Darrell Ball 


On 7/5/17, 5:20 PM, "ovs-dev-boun...@openvswitch.org on behalf of Ben Pfaff" 
 wrote:

I hadn't even heard of this feature before, but it seems to be at least
semi-standard to support Linux link-local address scopes via a % suffix,
e.g. fe80::1234%eth0 for a link-local address scoped to eth0.  This commit
adds support.

I'd appreciate feedback from folks who understand this feature better than
me.

Reported-by: Ali Volkan Atli 
Signed-off-by: Ben Pfaff 
---
 NEWS |  2 ++
 configure.ac |  3 +++
 lib/socket-util.c| 31 ++-
 lib/vconn-active.man |  7 +--
 lib/vconn-passive.man| 10 ++
 ovsdb/remote-active.man  | 20 +---
 ovsdb/remote-passive.man | 29 +++--
 7 files changed, 58 insertions(+), 44 deletions(-)

diff --git a/NEWS b/NEWS
index 0f2604ff5c51..c8c14481a1db 100644
--- a/NEWS
+++ b/NEWS
@@ -66,6 +66,8 @@ Post-v2.7.0
- Add experimental support for hardware offloading
  * HW offloading is disabled by default.
  * HW offloading is done through the TC interface.
+   - IPv6 link local addresses are now supported on Linux.  Use % to 
designate
+ the scope device.
 
 v2.7.0 - 21 Feb 2017
 -
diff --git a/configure.ac b/configure.ac
index 6404b5fc1222..9f78184f52a7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -106,6 +106,9 @@ AC_CHECK_DECLS([sys_siglist], [], [], [[#include 
]])
 AC_CHECK_MEMBERS([struct stat.st_mtim.tv_nsec, struct stat.st_mtimensec],
   [], [], [[#include ]])
 AC_CHECK_MEMBERS([struct ifreq.ifr_flagshigh], [], [], [[#include 
]])
+AC_CHECK_MEMBERS([struct sockaddr_in6.sin6_scope_id], [], [],
+  [[#include 
+#include ]])
 AC_CHECK_FUNCS([mlockall strnlen getloadavg statvfs getmntent_r])
 AC_CHECK_HEADERS([mntent.h sys/statvfs.h linux/types.h linux/if_ether.h 
stdatomic.h])
 AC_CHECK_HEADERS([net/if_mib.h], [], [], [[#include 
diff --git a/lib/socket-util.c b/lib/socket-util.c
index 2c0f1e62bd99..fbed172bdefb 100644
--- a/lib/socket-util.c
+++ b/lib/socket-util.c
@@ -365,7 +365,7 @@ parse_bracketed_token(char **pp)
 
 static bool
 parse_sockaddr_components(struct sockaddr_storage *ss,
-  const char *host_s,
+  char *host_s,
   const char *port_s, uint16_t default_port,
   const char *s)
 {
@@ -382,20 +382,34 @@ parse_sockaddr_components(struct sockaddr_storage *ss,
 }
 
 memset(ss, 0, sizeof *ss);
-if (strchr(host_s, ':')) {
+if (host_s && strchr(host_s, ':')) {
 struct sockaddr_in6 *sin6
 = ALIGNED_CAST(struct sockaddr_in6 *, ss);
 
+char *addr = strsep(_s, "%");
+
 sin6->sin6_family = AF_INET6;
 sin6->sin6_port = htons(port);
-if (!ipv6_parse(host_s, >sin6_addr)) {
-VLOG_ERR("%s: bad IPv6 address \"%s\"", s, host_s);
+if (!addr || !*addr || !ipv6_parse(addr, >sin6_addr)) {
+VLOG_ERR("%s: bad IPv6 address \"%s\"", s, addr ? addr : "");
 goto exit;
 }
+
+#ifdef HAVE_STRUCT_SOCKADDR_IN6_SIN6_SCOPE_ID
+char *scope = strsep(_s, "%");
+if (scope && *scope) {
+sin6->sin6_scope_id = if_nametoindex(scope);
+if (!sin6->sin6_scope_id) {
+VLOG_ERR("%s: bad IPv6 scope \"%s\" (%s)",
+ s, scope, ovs_strerror(errno));
+goto exit;
+}
+}
+#endif
 } else {
 sin->sin_family = AF_INET;
 sin->sin_port = htons(port);
-if (!ip_parse(host_s, >sin_addr.s_addr)) {
+if (host_s && !ip_parse(host_s, >sin_addr.s_addr)) {
 VLOG_ERR("%s: bad IPv4 address \"%s\"", s, host_s);
 goto exit;
 }
@@ -421,7 +435,7 @@ inet_parse_active(const char *target_, uint16_t 
default_port,
 {
 char *target = xstrdup(target_);
 const char *port;
-const char *host;
+char *host;
 char *p;
 bool ok;
 
@@ -548,7 +562,7 @@ inet_parse_passive(const char *target_, int 
default_port,
 {
 char *target = xstrdup(target_);
 const char *port;
-const char *host;
+char 

[ovs-dev] [PATCH] Support IPv6 link-local address scopes on Linux.

[Ben Pfaff]

I hadn't even heard of this feature before, but it seems to be at least
semi-standard to support Linux link-local address scopes via a % suffix,
e.g. fe80::1234%eth0 for a link-local address scoped to eth0.  This commit
adds support.

I'd appreciate feedback from folks who understand this feature better than
me.

Reported-by: Ali Volkan Atli 
Signed-off-by: Ben Pfaff 
---
 NEWS |  2 ++
 configure.ac |  3 +++
 lib/socket-util.c| 31 ++-
 lib/vconn-active.man |  7 +--
 lib/vconn-passive.man| 10 ++
 ovsdb/remote-active.man  | 20 +---
 ovsdb/remote-passive.man | 29 +++--
 7 files changed, 58 insertions(+), 44 deletions(-)

diff --git a/NEWS b/NEWS
index 0f2604ff5c51..c8c14481a1db 100644
--- a/NEWS
+++ b/NEWS
@@ -66,6 +66,8 @@ Post-v2.7.0
- Add experimental support for hardware offloading
  * HW offloading is disabled by default.
  * HW offloading is done through the TC interface.
+   - IPv6 link local addresses are now supported on Linux.  Use % to designate
+ the scope device.
 
 v2.7.0 - 21 Feb 2017
 -
diff --git a/configure.ac b/configure.ac
index 6404b5fc1222..9f78184f52a7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -106,6 +106,9 @@ AC_CHECK_DECLS([sys_siglist], [], [], [[#include 
]])
 AC_CHECK_MEMBERS([struct stat.st_mtim.tv_nsec, struct stat.st_mtimensec],
   [], [], [[#include ]])
 AC_CHECK_MEMBERS([struct ifreq.ifr_flagshigh], [], [], [[#include ]])
+AC_CHECK_MEMBERS([struct sockaddr_in6.sin6_scope_id], [], [],
+  [[#include 
+#include ]])
 AC_CHECK_FUNCS([mlockall strnlen getloadavg statvfs getmntent_r])
 AC_CHECK_HEADERS([mntent.h sys/statvfs.h linux/types.h linux/if_ether.h 
stdatomic.h])
 AC_CHECK_HEADERS([net/if_mib.h], [], [], [[#include 
diff --git a/lib/socket-util.c b/lib/socket-util.c
index 2c0f1e62bd99..fbed172bdefb 100644
--- a/lib/socket-util.c
+++ b/lib/socket-util.c
@@ -365,7 +365,7 @@ parse_bracketed_token(char **pp)
 
 static bool
 parse_sockaddr_components(struct sockaddr_storage *ss,
-  const char *host_s,
+  char *host_s,
   const char *port_s, uint16_t default_port,
   const char *s)
 {
@@ -382,20 +382,34 @@ parse_sockaddr_components(struct sockaddr_storage *ss,
 }
 
 memset(ss, 0, sizeof *ss);
-if (strchr(host_s, ':')) {
+if (host_s && strchr(host_s, ':')) {
 struct sockaddr_in6 *sin6
 = ALIGNED_CAST(struct sockaddr_in6 *, ss);
 
+char *addr = strsep(_s, "%");
+
 sin6->sin6_family = AF_INET6;
 sin6->sin6_port = htons(port);
-if (!ipv6_parse(host_s, >sin6_addr)) {
-VLOG_ERR("%s: bad IPv6 address \"%s\"", s, host_s);
+if (!addr || !*addr || !ipv6_parse(addr, >sin6_addr)) {
+VLOG_ERR("%s: bad IPv6 address \"%s\"", s, addr ? addr : "");
 goto exit;
 }
+
+#ifdef HAVE_STRUCT_SOCKADDR_IN6_SIN6_SCOPE_ID
+char *scope = strsep(_s, "%");
+if (scope && *scope) {
+sin6->sin6_scope_id = if_nametoindex(scope);
+if (!sin6->sin6_scope_id) {
+VLOG_ERR("%s: bad IPv6 scope \"%s\" (%s)",
+ s, scope, ovs_strerror(errno));
+goto exit;
+}
+}
+#endif
 } else {
 sin->sin_family = AF_INET;
 sin->sin_port = htons(port);
-if (!ip_parse(host_s, >sin_addr.s_addr)) {
+if (host_s && !ip_parse(host_s, >sin_addr.s_addr)) {
 VLOG_ERR("%s: bad IPv4 address \"%s\"", s, host_s);
 goto exit;
 }
@@ -421,7 +435,7 @@ inet_parse_active(const char *target_, uint16_t 
default_port,
 {
 char *target = xstrdup(target_);
 const char *port;
-const char *host;
+char *host;
 char *p;
 bool ok;
 
@@ -548,7 +562,7 @@ inet_parse_passive(const char *target_, int default_port,
 {
 char *target = xstrdup(target_);
 const char *port;
-const char *host;
+char *host;
 char *p;
 bool ok;
 
@@ -559,8 +573,7 @@ inet_parse_passive(const char *target_, int default_port,
 VLOG_ERR("%s: port must be specified", target_);
 ok = false;
 } else {
-ok = parse_sockaddr_components(ss, host ? host : "0.0.0.0",
-   port, default_port, target_);
+ok = parse_sockaddr_components(ss, host, port, default_port, target_);
 }
 if (!ok) {
 memset(ss, 0, sizeof *ss);
diff --git a/lib/vconn-active.man b/lib/vconn-active.man
index 3e789cc88e16..395879c8a58b 100644
--- a/lib/vconn-active.man
+++ b/lib/vconn-active.man
@@ -3,8 +3,11 @@
 The specified \fIport\fR on the host at the given \fIip\fR, which must
 be expressed as an IP address (not a DNS name) in IPv4 or IPv6 address
 format.  Wrap IPv6 addresses in square brackets,
-e.g.