supports IPv6 only remote target

Message ID CALZjo7tGT3QRGVhnO9H8PZmxoew-_k64fc_spiDsdV5orxqEkQ@mail.gmail.com
State New, archived
Headers

Commit Message

Tsutomu Seki Feb. 10, 2016, 11:43 a.m. UTC
  Hi Eli,
Thank you to review.

> Please use @code with the address string fe80::1.

Changed address to be "@code{fe80::1%eth1}", to include scope id
as written later.

> This example seems to imply that more than just taking brackets
> is required.

Your are right. This implies address/port separation rule and
address/scope separation rule. The former should be documented,
because address/port separation is done by the application before
passing them to getaddrinfo.

> Should we tell more about that?

On the other hand, the latter is difficult to document (for me) in
generic manner, because %-style scope-id notation depends on
implementation of getaddrinfo.

As far as I googled;

- Ubuntu: noted;
  http://manpages.ubuntu.com/manpages/precise/en/man3/getaddrinfo.3.html
> getaddrinfo() supports the address%scope-id notation for specifying the
> IPv6 scope-ID.

- FreeBSD: documented;
  https://www.freebsd.org/cgi/man.cgi?query=getaddrinfo&sektion=3
> This implementation of getaddrinfo() allows numeric IPv6 address
> notation with scope identifier, as documented in chapter 11 of
> RFC 4007.

- Windows: it seems to be supported, but not documented;
  https://msdn.microsoft.com/en-us/library/windows/desktop/ms738520(v=vs.85).aspx
  https://msdn.microsoft.com/en-us/library/windows/desktop/aa385325(v=vs.85).aspx

To avoid this ambiguity whether OS supports this manner of separation,
I want to suggest to use whole "fe80::1%eth1" including scope id as
an example of "numeric IPv6 address".

> The service name defaults to @code{"gdbremote"}

I think quotation is not needed if @code is used. Please review
another example I added to show how to use service name explicitly.

> I guess you mean "on the port assigned to that service", yes?

Yes.

> I think this will break the build with mingw.org's MinGW.  It doesn't
> have wspiapi.h, AFAIK.  What exactly is needed from that header?

It is needed to support getaddrinfo() on Windows 2000 and older versions.

What can I do for it?  The easiest solution is to replace with <ws2tcpip.h>,
but Windows 2000 support (as a host) would be dropped.

Regards,
Seki, Tsutomu

---
gdb/ChangeLog:

        * ser-tcp.c (net_open): Use getaddrinfo instead of gethostbyname
        (net_open): Add IPv6 numerical address support
        (net_open): Use "gdbremote" as default service name

gdb/doc/ChangeLog:

        * gdb.texinfo (Remote Connection Commands): Update to mention
        IPv6 numerical address and default service name.

---
 gdb/doc/gdb.texinfo | 37 +++++++++++++++++++---
 gdb/ser-tcp.c       | 91 ++++++++++++++++++++++++++++++++++-------------------
 2 files changed, 92 insertions(+), 36 deletions(-)

---
   ioarg = 1;
@@ -221,7 +243,7 @@ net_open (struct serial *scb, const char *name)

   /* Use Non-blocking connect.  connect() will return 0 if connected
      already.  */
-  n = connect (scb->fd, (struct sockaddr *) &sockaddr, sizeof (sockaddr));
+  n = connect (scb->fd, ai->ai_addr, ai->ai_addrlen);

   if (n < 0)
     {
@@ -257,7 +279,7 @@ net_open (struct serial *scb, const char *name)
  {
   errno = err;
   net_close (scb);
-  return -1;
+  goto bailout;
  }

       /* Looks like we need to wait for the connect.  */
@@ -269,7 +291,7 @@ net_open (struct serial *scb, const char *name)
       if (n < 0)
  {
   net_close (scb);
-  return -1;
+  goto bailout;
  }
     }

@@ -301,7 +323,7 @@ net_open (struct serial *scb, const char *name)
  if (err)
   errno = err;
  net_close (scb);
- return -1;
+ goto bailout;
       }
   }

@@ -323,7 +345,12 @@ net_open (struct serial *scb, const char *name)
   signal (SIGPIPE, SIG_IGN);
 #endif

+  freeaddrinfo (ai);
   return 0;
+
+ bailout:
+  freeaddrinfo (ai);
+  return -1;
 }

 void
  

Comments

Eli Zaretskii Feb. 10, 2016, 6:13 p.m. UTC | #1
> Date: Wed, 10 Feb 2016 20:43:40 +0900
> From: Tsutomu Seki <sekiriki@gmail.com>
> Cc: gdb-patches@sourceware.org
> 
> Changed address to be "@code{fe80::1%eth1}", to include scope id
> as written later.
> 
> > This example seems to imply that more than just taking brackets
> > is required.
> 
> Your are right. This implies address/port separation rule and
> address/scope separation rule. The former should be documented,
> because address/port separation is done by the application before
> passing them to getaddrinfo.
> 
> > Should we tell more about that?
> 
> On the other hand, the latter is difficult to document (for me) in
> generic manner, because %-style scope-id notation depends on
> implementation of getaddrinfo.

I think what you did is good enough, thanks.

> > I think this will break the build with mingw.org's MinGW.  It doesn't
> > have wspiapi.h, AFAIK.  What exactly is needed from that header?
> 
> It is needed to support getaddrinfo() on Windows 2000 and older versions.
> 
> What can I do for it?  The easiest solution is to replace with <ws2tcpip.h>,
> but Windows 2000 support (as a host) would be dropped.

Is it feasible to copy the necessary bits from that header, i.e. have
the prototype in ser-tcp.c (or in some suitable GDB header file)?

Thanks.
  
Tsutomu Seki Feb. 10, 2016, 6:55 p.m. UTC | #2
Hi Eli,

> Is it feasible to copy the necessary bits from that header

I do not think so, because wspapi.h itself exists for backward compatibility.
It should be easier adding wspapi.h to MSYS than copying from it.

MSYS ws2tcpip.h says;

#if (_WIN32_WINNT >= _WIN32_WINNT_WINXP)
/**
* For WIN2K the user includes wspiapi.h for these functions.
*/
void WSAAPI freeaddrinfo (struct addrinfo*);
int WSAAPI getaddrinfo (const char*,const char*,const struct addrinfo*,
        struct addrinfo**);
int WSAAPI getnameinfo(const struct sockaddr*,socklen_t,char*,DWORD,
       char*,DWORD,int);
#endif /* (_WIN32_WINNT >= _WIN32_WINNT_WINXP) */

Is it better to try to update configure.ac to decide to use IPv6(getaddrinfo)
or stay on IPv4(gethostbyname)? I'm not familiar with autoconf, though.

# I'm resending this message because I'd accidentally sent HTML mail
# from smartphone and it was rejected. I'm sorry if you have received
# duplicated message.

Regards,
Seki, Tsutomu

2016-02-11 3:13 GMT+09:00 Eli Zaretskii <eliz@gnu.org>:
>> Date: Wed, 10 Feb 2016 20:43:40 +0900
>> From: Tsutomu Seki <sekiriki@gmail.com>
>> Cc: gdb-patches@sourceware.org
>>
>> Changed address to be "@code{fe80::1%eth1}", to include scope id
>> as written later.
>>
>> > This example seems to imply that more than just taking brackets
>> > is required.
>>
>> Your are right. This implies address/port separation rule and
>> address/scope separation rule. The former should be documented,
>> because address/port separation is done by the application before
>> passing them to getaddrinfo.
>>
>> > Should we tell more about that?
>>
>> On the other hand, the latter is difficult to document (for me) in
>> generic manner, because %-style scope-id notation depends on
>> implementation of getaddrinfo.
>
> I think what you did is good enough, thanks.
>
>> > I think this will break the build with mingw.org's MinGW.  It doesn't
>> > have wspiapi.h, AFAIK.  What exactly is needed from that header?
>>
>> It is needed to support getaddrinfo() on Windows 2000 and older versions.
>>
>> What can I do for it?  The easiest solution is to replace with <ws2tcpip.h>,
>> but Windows 2000 support (as a host) would be dropped.
>
> Is it feasible to copy the necessary bits from that header, i.e. have
> the prototype in ser-tcp.c (or in some suitable GDB header file)?
>
> Thanks.
  
Eli Zaretskii Feb. 10, 2016, 7:21 p.m. UTC | #3
> Date: Thu, 11 Feb 2016 03:55:29 +0900
> From: Tsutomu Seki <sekiriki@gmail.com>
> Cc: gdb-patches@sourceware.org
> 
> MSYS ws2tcpip.h says;
> 
> #if (_WIN32_WINNT >= _WIN32_WINNT_WINXP)
> /**
> * For WIN2K the user includes wspiapi.h for these functions.
> */
> void WSAAPI freeaddrinfo (struct addrinfo*);
> int WSAAPI getaddrinfo (const char*,const char*,const struct addrinfo*,
>         struct addrinfo**);
> int WSAAPI getnameinfo(const struct sockaddr*,socklen_t,char*,DWORD,
>        char*,DWORD,int);
> #endif /* (_WIN32_WINNT >= _WIN32_WINNT_WINXP) */

Yes, because AFAIK you can have getaddrinfo on W2K only if you install
an optional upgrade kit.

> Is it better to try to update configure.ac to decide to use IPv6(getaddrinfo)
> or stay on IPv4(gethostbyname)?

No, not on Windows, where a binary compiled on one system can then be
run on another.

Do we care about W2K support at this point?  AFAIK, quite a few
MinGW64 headers require and set _WIN32_WINNT to a value that will
exclude W2K anyway.

I suggest to wait for Pedro or Joel to chime in on this issue.

Thanks.
  
Pedro Alves March 10, 2016, 4:44 p.m. UTC | #4
On 02/10/2016 07:21 PM, Eli Zaretskii wrote:

> Do we care about W2K support at this point?  AFAIK, quite a few
> MinGW64 headers require and set _WIN32_WINNT to a value that will
> exclude W2K anyway.
> 
> I suggest to wait for Pedro or Joel to chime in on this issue.

I wouldn't care about W2K at this point.

gnulib has a getaddrinfo module, but I think that pulls in gnulib's 
select replacement, which is a big problem for gdb.  So if we cared
about old hosts that don't support getaddrinfo, it'd have to be based
on #ifdef.  But, I believe that the only remaining such systems
would be old Windows hosts, since we dropped support for several old
Unix hosts in 7.10/7.11.  As such, I think we can just use getaddrinfo
unconditionally.

I think this is at least the 3rd IPv6 patch posted over the years.
It's about time we merge some in.  I've added a couple folks to
CC who proposed this before.

It'd be useful to review past discussions, and see if
there's something there we should consider.  See e.g.:

  https://sourceware.org/ml/gdb-patches/2014-02/msg00254.html

AFAICS, Tsutomu's patch doesn't cycle through available
addresses.  Should it?

Tsutomu, do you have an FSF copyright assignment?

Paul, do you have an FSF copyright assignment?

Thanks,
Pedro Alves
  
Paul Fertser March 10, 2016, 9:20 p.m. UTC | #5
On Thu, Mar 10, 2016 at 04:44:23PM +0000, Pedro Alves wrote:
> Paul, do you have an FSF copyright assignment?

No, I do not. If I remember correctly, I wasn't fully satisfied with
the patch I posted, but the IPv4-only code that was there back then
seemed to be buggy/quirky as well, so probably someone needs to decide
on the desired behaviour first, then rewrite the loop based solely on
that decision.
  
Tsutomu Seki March 11, 2016, 1:42 a.m. UTC | #6
Hi Pedro,

> Tsutomu, do you have an FSF copyright assignment?

No, I don't.

Regards

2016-03-11 6:20 GMT+09:00 Paul Fertser <fercerpav@gmail.com>:
> On Thu, Mar 10, 2016 at 04:44:23PM +0000, Pedro Alves wrote:
>> Paul, do you have an FSF copyright assignment?
>
> No, I do not. If I remember correctly, I wasn't fully satisfied with
> the patch I posted, but the IPv4-only code that was there back then
> seemed to be buggy/quirky as well, so probably someone needs to decide
> on the desired behaviour first, then rewrite the loop based solely on
> that decision.
>
> --
> Be free, use free (http://www.gnu.org/philosophy/free-sw.html) software!
> mailto:fercerpav@gmail.com
  
Pedro Alves March 11, 2016, 10:25 a.m. UTC | #7
On 03/11/2016 01:42 AM, Tsutomu Seki wrote:
> Hi Pedro,
>
>> Tsutomu, do you have an FSF copyright assignment?
>
> No, I don't.

The patch is larger than we could consider "not legally significant",
so we will need one in order to accept it.

I'll follow up on that off list.

Thanks,
Pedro Alves
  
Eli Zaretskii March 11, 2016, 3:51 p.m. UTC | #8
> From: Pedro Alves <palves@redhat.com>
> Date: Thu, 10 Mar 2016 16:44:23 +0000
> 
> On 02/10/2016 07:21 PM, Eli Zaretskii wrote:
> 
> > Do we care about W2K support at this point?  AFAIK, quite a few
> > MinGW64 headers require and set _WIN32_WINNT to a value that will
> > exclude W2K anyway.
> > 
> > I suggest to wait for Pedro or Joel to chime in on this issue.
> 
> I wouldn't care about W2K at this point.

Fair enough, but then we should set _WIN32_WINNT to a suitable value,
or else compilation using mingw.org's w32api headers will not see the
prototype of getaddrinfo, and maybe also some related stuff.
  
Sergio Durigan Junior April 10, 2018, 7:21 p.m. UTC | #9
On Friday, March 11 2016, Pedro Alves wrote:

> On 03/11/2016 01:42 AM, Tsutomu Seki wrote:
>> Hi Pedro,
>>
>>> Tsutomu, do you have an FSF copyright assignment?
>>
>> No, I don't.
>
> The patch is larger than we could consider "not legally significant",
> so we will need one in order to accept it.
>
> I'll follow up on that off list.

Hi,

Resurrecting the thread after more than 2 years.  Tsutomu, were you able
to apply for an FSF copyright assignment?  Are you still interested in
pushing this patch?  I know there's also a thread by Paul Fertser:

  https://sourceware.org/ml/gdb-patches/2014-02/msg00248.html

And apparently he didn't have a copyright assignment as well, so I'm
wondering if he's also managed to move forward with this.

Thanks,
  
Paul Fertser April 10, 2018, 8:04 p.m. UTC | #10
Hello,

On Tue, Apr 10, 2018 at 03:21:29PM -0400, Sergio Durigan Junior wrote:
> And apparently he didn't have a copyright assignment as well, so I'm
> wondering if he's also managed to move forward with this.

No, I haven't moved forward, sorry.
  
Eli Zaretskii April 11, 2018, 2:26 a.m. UTC | #11
> From: Sergio Durigan Junior <sergiodj@redhat.com>
> Cc: "Tsutomu Seki" <sekiriki@gmail.com>,  "Paul Fertser" <fercerpav@gmail.com>,  "Eli Zaretskii" <eliz@gnu.org>,  gdb-patches@sourceware.org,  "Jan Kratochvil" <jan.kratochvil@redhat.com>
> Date: Tue, 10 Apr 2018 15:21:29 -0400
> 
> On Friday, March 11 2016, Pedro Alves wrote:
> 
> > On 03/11/2016 01:42 AM, Tsutomu Seki wrote:
> >> Hi Pedro,
> >>
> >>> Tsutomu, do you have an FSF copyright assignment?
> >>
> >> No, I don't.
> >
> > The patch is larger than we could consider "not legally significant",
> > so we will need one in order to accept it.
> >
> > I'll follow up on that off list.
> 
> Hi,
> 
> Resurrecting the thread after more than 2 years.  Tsutomu, were you able
> to apply for an FSF copyright assignment?

Tsutomu does have his copyright assignment on file now.
  
Tsutomu Seki April 11, 2018, 12:19 p.m. UTC | #12
I'm so sorry to have kept you waiting. Yes, I already have assigned
the copyright to FSF.

Thanks

2018-04-11 11:26 GMT+09:00 Eli Zaretskii <eliz@gnu.org>:
>> From: Sergio Durigan Junior <sergiodj@redhat.com>
>> Cc: "Tsutomu Seki" <sekiriki@gmail.com>,  "Paul Fertser" <fercerpav@gmail.com>,  "Eli Zaretskii" <eliz@gnu.org>,  gdb-patches@sourceware.org,  "Jan Kratochvil" <jan.kratochvil@redhat.com>
>> Date: Tue, 10 Apr 2018 15:21:29 -0400
>>
>> On Friday, March 11 2016, Pedro Alves wrote:
>>
>> > On 03/11/2016 01:42 AM, Tsutomu Seki wrote:
>> >> Hi Pedro,
>> >>
>> >>> Tsutomu, do you have an FSF copyright assignment?
>> >>
>> >> No, I don't.
>> >
>> > The patch is larger than we could consider "not legally significant",
>> > so we will need one in order to accept it.
>> >
>> > I'll follow up on that off list.
>>
>> Hi,
>>
>> Resurrecting the thread after more than 2 years.  Tsutomu, were you able
>> to apply for an FSF copyright assignment?
>
> Tsutomu does have his copyright assignment on file now.
  
Sergio Durigan Junior April 11, 2018, 4:18 p.m. UTC | #13
On Wednesday, April 11 2018, Tsutomu Seki wrote:

> I'm so sorry to have kept you waiting. Yes, I already have assigned
> the copyright to FSF.

No problem, Tsutomu.  Glad to know the paperwork is in place now.  Would
you like to submit a new version of your patch so that we can discuss it
again?

Thanks,
  

Patch

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 2d09d13..ebc0263 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -19571,10 +19571,11 @@  If you're using a serial line, you may want
to give @value{GDBN} the
 @cindex @acronym{TCP} port, @code{target remote}
 Debug using a @acronym{TCP} connection to @var{port} on @var{host}.
 The @var{host} may be either a host name or a numeric @acronym{IP}
-address; @var{port} must be a decimal number.  The @var{host} could be
-the target machine itself, if it is directly connected to the net, or
-it might be a terminal server which in turn has a serial line to the
-target.
+address or a numeric @acronym{IPv6} address enclosed in square
+brackets; @var{port} may be either a service name or a decimal number.
+The @var{host} could be the target machine itself, if it is directly
+connected to the net, or it might be a terminal server which in turn
+has a serial line to the target.

 For example, to connect to port 2828 on a terminal server named
 @code{manyfarms}:
@@ -19583,6 +19584,14 @@  For example, to connect to port 2828 on a
terminal server named
 target remote manyfarms:2828
 @end smallexample

+Numeric @acronym{IPv6} address must be enclosed in square brackets.
+For example, to connect to port 2159 of a target which has
+@acronym{IPv6} address @code{fe80::1%eth1}:
+
+@smallexample
+target remote [fe80::1%eth1]:2159
+@end smallexample
+
 If your remote target is actually running on the same machine as your
 debugger session (e.g.@: a simulator for your target running on the
 same host), you can omit the hostname.  For example, to connect to
@@ -19591,6 +19600,26 @@  port 1234 on your local machine:
 @smallexample
 target remote :1234
 @end smallexample
+
+If the service name is present in service list (e.g.@: @file{/etc/services}
+on @sc{gnu}/Linux systems) on your local machine, and target is
+listening on the port assigned to that service, you can use
+human-friendly name instead of the port number.  For example,
+to connect to service named @code{icepick} on a target machine
+named @code{stormstown}:
+
+@smallexample
+target remote stormstown:icepick
+@end smallexample
+
+The service name defaults to @code{gdbremote}.  If your target is
+listening on the port assigned to @code{gdbremote}, you can omit the
+service name.  For example, to connect to service @code{gdbremote}
+on your local machine:
+
+@smallexample
+target remote :
+@end smallexample
 @noindent

 Note that the colon is still required here.
diff --git a/gdb/ser-tcp.c b/gdb/ser-tcp.c
index df3af4c..0123480 100644
--- a/gdb/ser-tcp.c
+++ b/gdb/ser-tcp.c
@@ -39,6 +39,7 @@ 

 #ifdef USE_WIN32API
 #include <winsock2.h>
+#include <ws2tcpip.h>
 #ifndef ETIMEDOUT
 #define ETIMEDOUT WSAETIMEDOUT
 #endif
@@ -156,11 +157,10 @@  int
 net_open (struct serial *scb, const char *name)
 {
   char hostname[100];
-  const char *port_str;
-  int n, port, tmp;
+  const char *port_str, *host_str;
+  int n, tmp, err;
   int use_udp;
-  struct hostent *hostent;
-  struct sockaddr_in sockaddr;
+  struct addrinfo *ai, hint;
 #ifdef USE_WIN32API
   u_long ioarg;
 #else
@@ -177,43 +177,65 @@  net_open (struct serial *scb, const char *name)
   else if (startswith (name, "tcp:"))
     name = name + 4;

-  port_str = strchr (name, ':');
+  if (name[0] == '[')
+    {
+      const char *end = strchr(name + 1, ']');
+      if (!end)
+ error (_("net_open: No close bracket in host name!"));
+
+      tmp = min (end - name - 1, (int) sizeof hostname - 1);
+      strncpy (hostname, name + 1, tmp); /* Don't want brackets. */
+      hostname[tmp] = '\0'; /* Tie off host name.  */
+      host_str = hostname;
+      port_str = end + 1;
+    }
+  else if ((port_str = strrchr(name, ':')) != NULL)
+    {
+      tmp = min (port_str - name, (int) sizeof hostname - 1);
+      strncpy (hostname, name, tmp); /* Don't want colon.  */
+      hostname[tmp] = '\0'; /* Tie off host name.  */
+      host_str = hostname;
+    }
+  else
+    {
+      host_str = name;
+      port_str = NULL;
+    }

-  if (!port_str)
-    error (_("net_open: No colon in host name!"));  /* Shouldn't ever
-       happen.  */
+  if (port_str != NULL && port_str[0] == ':')
+    port_str++;

-  tmp = min (port_str - name, (int) sizeof hostname - 1);
-  strncpy (hostname, name, tmp); /* Don't want colon.  */
-  hostname[tmp] = '\000'; /* Tie off host name.  */
-  port = atoi (port_str + 1);
+  /* Default service name is gdbremote.  */
+  if (port_str == NULL || port_str[0] == '\0')
+    port_str = "gdbremote";

   /* Default hostname is localhost.  */
-  if (!hostname[0])
-    strcpy (hostname, "localhost");
-
-  hostent = gethostbyname (hostname);
-  if (!hostent)
+  if (host_str[0] == '\0')
+    host_str = "localhost";
+
+  memset(&hint, 0, sizeof hint);
+  hint.ai_family = AF_UNSPEC;
+  hint.ai_socktype = use_udp ? SOCK_DGRAM : SOCK_STREAM;
+  hint.ai_flags = 0;
+  hint.ai_protocol = use_udp ? IPPROTO_UDP : IPPROTO_TCP;
+  hint.ai_canonname = NULL;
+  hint.ai_addr = NULL;
+  hint.ai_next = NULL;
+  ai = NULL;
+
+  if ((err = getaddrinfo (host_str, port_str, &hint, &ai)) != 0)
     {
-      fprintf_unfiltered (gdb_stderr, "%s: unknown host\n", hostname);
+      fprintf_unfiltered (gdb_stderr, "%s: %s\n", name, gai_strerror(err));
       errno = ENOENT;
       return -1;
     }

-  sockaddr.sin_family = PF_INET;
-  sockaddr.sin_port = htons (port);
-  memcpy (&sockaddr.sin_addr.s_addr, hostent->h_addr,
-  sizeof (struct in_addr));
-
  retry:

-  if (use_udp)
-    scb->fd = gdb_socket_cloexec (PF_INET, SOCK_DGRAM, 0);
-  else
-    scb->fd = gdb_socket_cloexec (PF_INET, SOCK_STREAM, 0);
+  scb->fd = gdb_socket_cloexec (ai->ai_family, ai->ai_socktype,
ai->ai_protocol);

   if (scb->fd == -1)
-    return -1;
+    goto bailout;

   /* Set socket nonblocking.  */