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 +0000, 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 <[email protected]>
> 
> 
> On 7/5/17, 5:20 PM, "[email protected] on behalf of Ben Pfaff" 
> <[email protected] on behalf of [email protected]> 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 <[email protected]>
>     Signed-off-by: Ben Pfaff <[email protected]>
>     ---
>      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 
> <signal.h>]])
>      AC_CHECK_MEMBERS([struct stat.st_mtim.tv_nsec, struct stat.st_mtimensec],
>        [], [], [[#include <sys/stat.h>]])
>      AC_CHECK_MEMBERS([struct ifreq.ifr_flagshigh], [], [], [[#include 
> <net/if.h>]])
>     +AC_CHECK_MEMBERS([struct sockaddr_in6.sin6_scope_id], [], [],
>     +  [[#include <sys/socket.h>
>     +#include <netinet/in.h>]])
>      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 <sys/types.h>
>     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(&host_s, "%");
>     +
>              sin6->sin6_family = AF_INET6;
>              sin6->sin6_port = htons(port);
>     -        if (!ipv6_parse(host_s, &sin6->sin6_addr)) {
>     -            VLOG_ERR("%s: bad IPv6 address \"%s\"", s, host_s);
>     +        if (!addr || !*addr || !ipv6_parse(addr, &sin6->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(&host_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->sin_addr.s_addr)) {
>     +        if (host_s && !ip_parse(host_s, &sin->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. \fBtcp:[::1]:6653\fR.  For \fBssl\fR, the \fB\-\-private\-key\fR,
>     -\fB\-\-certificate\fR, and \fB\-\-ca\-cert\fR options are mandatory.
>     +e.g. \fBtcp:[::1]:6653\fR.  On Linux, use \fB%\fIdevice\fR to
>     +designate a scope for IPv6 link-level addresses,
>     +e.g. \fBtcp:[fe80::1234%eth0]:6653\fR.  For \fBssl\fR, the
>     +\fB\-\-private\-key\fR, \fB\-\-certificate\fR, and \fB\-\-ca\-cert\fR
>     +options are mandatory.
>      .IP
>      If \fIport\fR is not specified, it defaults to 6653.
>      .TP
>     diff --git a/lib/vconn-passive.man b/lib/vconn-passive.man
>     index 9d9050b020ff..1ffa183972b8 100644
>     --- a/lib/vconn-passive.man
>     +++ b/lib/vconn-passive.man
>     @@ -1,10 +1,12 @@
>      .IP "\fBpssl:\fR[\fIport\fR][\fB:\fIip\fR]"
>      .IQ "\fBptcp:\fR[\fIport\fR][\fB:\fIip\fR]"
>      Listens for OpenFlow connections on \fIport\fR.  The default
>     -\fIport\fR is 6653.  By default, connections
>     -are allowed from any IPv4 address.  Specify \fIip\fR as an IPv4
>     -address or a bracketed IPv6 address (e.g. \fBptcp:6653:[::1]\fR).  DNS
>     -names may not be used.  For \fBpssl\fR, the
>     +\fIport\fR is 6653.  By default, connections are allowed from any IPv4
>     +address.  Specify \fIip\fR as an IPv4 address or a bracketed IPv6
>     +address (e.g. \fBptcp:6653:[::1]\fR).  On Linux, use \fB%\fIdevice\fR
>     +to designate a scope for IPv6 link-level addresses,
>     +e.g. \fBptcp:6653:[fe80::1234%eth0]\fR.  DNS names may
>     +not be used.  For \fBpssl\fR, the
>      \fB\-\-private\-key\fR,\fB\-\-certificate\fR, and \fB\-\-ca\-cert\fR
>      options are mandatory.
>      .IP
>     diff --git a/ovsdb/remote-active.man b/ovsdb/remote-active.man
>     index 83d64652ddb9..bf5b323dd55e 100644
>     --- a/ovsdb/remote-active.man
>     +++ b/ovsdb/remote-active.man
>     @@ -1,15 +1,13 @@
>      .IP "\fBssl:\fIip\fB:\fIport\fR"
>     -The specified SSL \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.  If \fIip\fR is an IPv6 address, then wrap \fIip\fR with square
>     -brackets, e.g.: \fBssl:[::1]:6640\fR.
>     -The \fB\-\-private\-key\fR, \fB\-\-certificate\fR, and \fB\-\-ca\-cert\fR
>     -options are mandatory when this form is used.
>     -.
>     -.IP "\fBtcp:\fIip\fB:\fIport\fR"
>     -Connect to the given TCP \fIport\fR on \fIip\fR, where \fIip\fR can be 
> IPv4
>     -or IPv6 address. If \fIip\fR is an IPv6 address, then wrap \fIip\fR with
>     -square brackets, e.g.: \fBtcp:[::1]:6640\fR.
>     +.IQ "\fBtcp:\fIip\fB:\fIport\fR"
>     +The given SSL or plain TCP \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.  If \fIip\fR is an IPv6 address, then
>     +wrap \fIip\fR with square brackets, e.g.: \fBssl:[::1]:6640\fR.  On
>     +Linux, use \fB%\fIdevice\fR to designate a scope for IPv6 link-level
>     +addresses, e.g. \fBssl:[fe80::1234%eth0]:6653\fR.  For \fBssl\fR, the
>     +\fB\-\-private\-key\fR, \fB\-\-certificate\fR, and \fB\-\-ca\-cert\fR
>     +options are mandatory.
>      .
>      .IP "\fBunix:\fIfile\fR"
>      On POSIX, connect to the Unix domain server socket named \fIfile\fR.
>     diff --git a/ovsdb/remote-passive.man b/ovsdb/remote-passive.man
>     index 5da2de87bf29..f2a1868442da 100644
>     --- a/ovsdb/remote-passive.man
>     +++ b/ovsdb/remote-passive.man
>     @@ -1,22 +1,15 @@
>      .IP "\fBpssl:\fIport\fR[\fB:\fIip\fR]"
>     -Listen on the given SSL \fIport\fR for a connection.  By default,
>     -connections are not bound to a particular local IP address and
>     -it listens only on IPv4 (but not IPv6) addresses, but
>     -specifying \fIip\fR limits connections to those from the given
>     -\fIip\fR, either IPv4 or IPv6 address.  If \fIip\fR is
>     -an IPv6 address, then wrap \fIip\fR with square brackets, e.g.:
>     -\fBpssl:6640:[::1]\fR.  The \fB\-\-private\-key\fR,
>     -\fB\-\-certificate\fR, and \fB\-\-ca\-cert\fR options are mandatory
>     -when this form is used.
>     -.
>     -.IP "\fBptcp:\fIport\fR[\fB:\fIip\fR]"
>     -Listen on the given TCP \fIport\fR for a connection.  By default,
>     -connections are not bound to a particular local IP address and
>     -it listens only on IPv4 (but not IPv6) addresses, but
>     -\fIip\fR may be specified to listen only for connections to the given
>     -\fIip\fR, either IPv4 or IPv6 address.  If \fIip\fR is
>     -an IPv6 address, then wrap \fIip\fR with square brackets, e.g.:
>     -\fBptcp:6640:[::1]\fR.
>     +.IQ "\fBptcp:\fIport\fR[\fB:\fIip\fR]"
>     +Listen on the given SSL or TCP \fIport\fR for a connection.  By
>     +default, connections are not bound to a particular local IP address
>     +and it listens only on IPv4 (but not IPv6) addresses, but specifying
>     +\fIip\fR limits connections to those from the given \fIip\fR, either
>     +IPv4 or IPv6 address.  If \fIip\fR is an IPv6 address, then wrap
>     +\fIip\fR with square brackets, e.g.: \fBpssl:6640:[::1]\fR.  On Linux,
>     +use \fB%\fIdevice\fR to designate a scope for IPv6 link-level
>     +addresses, e.g. \fBpssl:6653:[fe80::1234%eth0]\fR.  For \fBpssl\fR,
>     +the \fB\-\-private\-key\fR, \fB\-\-certificate\fR, and
>     +\fB\-\-ca\-cert\fR options are mandatory.
>      .
>      .IP "\fBpunix:\fIfile\fR"
>      On POSIX, listen on the Unix domain server socket named \fIfile\fR for a
>     -- 
>     2.10.2
>     
>     _______________________________________________
>     dev mailing list
>     [email protected]
>     
> https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=akEkPPZsOEbokZhc8suC1ou0OMIPQyhld31E2MZqJU0&s=t7TENvQoXaoiRdAGBpQ6FOFdEHczVIVWFTXppzAeBPI&e=
>  
>     
> 
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to