Patchwork [v3,9/9] net: support to include ipv6 address by brackets

login
register
mail settings
Submitter Amos Kong
Date March 6, 2012, 10:48 p.m.
Message ID <20120306224857.24264.88329.stgit@dhcp-8-167.nay.redhat.com>
Download mbox | patch
Permalink /patch/145051/
State New
Headers show

Comments

Amos Kong - March 6, 2012, 10:48 p.m.
That method of representing an IPv6 address with a port is
discouraged because of its ambiguity. Referencing to RFC5952,
the recommended format is:

     [2312::8274]:5200

For IPv6 brackets must be mandatory if you require a port.

test status: Successed
listen side: qemu-kvm .... -incoming tcp:[2312::8274]:5200
client side: qemu-kvm ...
             (qemu) migrate -d tcp:[2312::8274]:5200

Signed-off-by: Amos Kong <akong@redhat.com>
---
 net.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)
Michael Roth - March 13, 2012, 7:47 p.m.
On Wed, Mar 07, 2012 at 06:48:57AM +0800, Amos Kong wrote:
> That method of representing an IPv6 address with a port is

I'm not sure what "that" is referencing. I assumed the previous patch
but the representation seems to be the same?

> discouraged because of its ambiguity. Referencing to RFC5952,
> the recommended format is:
> 
>      [2312::8274]:5200
> 
> For IPv6 brackets must be mandatory if you require a port.
> 
> test status: Successed
> listen side: qemu-kvm .... -incoming tcp:[2312::8274]:5200
> client side: qemu-kvm ...
>              (qemu) migrate -d tcp:[2312::8274]:5200
> 
> Signed-off-by: Amos Kong <akong@redhat.com>
> ---
>  net.c |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/net.c b/net.c
> index d6ce1fa..499ed1d 100644
> --- a/net.c
> +++ b/net.c
> @@ -88,6 +88,12 @@ static int get_str_sep(char *buf, int buf_size, const char **pp, int sep)
>      if (!p1)
>          return -1;
>      len = p1 - p;
> +    /* remove brackets which includes hostname */
> +    if (*p == '[' && *(p1-1) == ']') {
> +        p += 1;
> +        len -= 2;
> +    }

Sorry, looking again I guess net/slirp.c actually has it's own copy of
get_str_sep(), so modifying this doesn't look like it would break
anything currently. It might cause some confusion though :). And I think
the special handling for brackets should be done in
parse_host_port_info() since get_str_sep() is pretty generically named.

> +
>      p1++;
>      if (buf_size > 0) {
>          if (len > buf_size - 1)
> 
>
Amos Kong - March 14, 2012, 9:58 a.m.
On 14/03/12 03:47, Michael Roth wrote:
> On Wed, Mar 07, 2012 at 06:48:57AM +0800, Amos Kong wrote:
>> That method of representing an IPv6 address with a port is
>
> I'm not sure what "that" is referencing.

2312::8274:5200  (5200 is a port)

> I assumed the previous patch
> but the representation seems to be the same?

2312::8274:5200

[2312::8274]:5200

The second one is better.

>> discouraged because of its ambiguity. Referencing to RFC5952,
>> the recommended format is:
>>
>>       [2312::8274]:5200
>>
>> For IPv6 brackets must be mandatory if you require a port.
>>
>> test status: Successed
>> listen side: qemu-kvm .... -incoming tcp:[2312::8274]:5200
>> client side: qemu-kvm ...
>>               (qemu) migrate -d tcp:[2312::8274]:5200
>>
>> Signed-off-by: Amos Kong<akong@redhat.com>
>> ---
>>   net.c |    6 ++++++
>>   1 files changed, 6 insertions(+), 0 deletions(-)
>>
>> diff --git a/net.c b/net.c
>> index d6ce1fa..499ed1d 100644
>> --- a/net.c
>> +++ b/net.c
>> @@ -88,6 +88,12 @@ static int get_str_sep(char *buf, int buf_size, const char **pp, int sep)
>>       if (!p1)
>>           return -1;
>>       len = p1 - p;
>> +    /* remove brackets which includes hostname */
>> +    if (*p == '['&&  *(p1-1) == ']') {
>> +        p += 1;
>> +        len -= 2;
>> +    }
>
> Sorry, looking again I guess net/slirp.c actually has it's own copy of
> get_str_sep(), so modifying this doesn't look like it would break
> anything currently.

> It might cause some confusion though :). And I think
> the special handling for brackets should be done in
> parse_host_port_info() since get_str_sep() is pretty generically named.

agree.

>> +
>>       p1++;
>>       if (buf_size>  0) {
>>           if (len>  buf_size - 1)
>>
>>
Michael Roth - March 14, 2012, 3:38 p.m.
On Wed, Mar 14, 2012 at 05:58:20PM +0800, Amos Kong wrote:
> On 14/03/12 03:47, Michael Roth wrote:
> >On Wed, Mar 07, 2012 at 06:48:57AM +0800, Amos Kong wrote:
> >>That method of representing an IPv6 address with a port is
> >
> >I'm not sure what "that" is referencing.
> 
> 2312::8274:5200  (5200 is a port)
> 
> >I assumed the previous patch
> >but the representation seems to be the same?
> 
> 2312::8274:5200
> 
> [2312::8274]:5200
> 
> The second one is better.

The commit message for the previous patch also uses "[2312::8274]:5200"

If that format wasn't supported till this patch you should fix up the
example in patch 8 to be "2312::8274:5200". Small nit, but it gives the
impression [host]:port was already supported in some form, which is all
the more confusing because [host]:port *is* already supported in some
form, depending on where you are in QEMU :)

> 
> >>discouraged because of its ambiguity. Referencing to RFC5952,
> >>the recommended format is:
> >>
> >>      [2312::8274]:5200
> >>
> >>For IPv6 brackets must be mandatory if you require a port.
> >>
> >>test status: Successed
> >>listen side: qemu-kvm .... -incoming tcp:[2312::8274]:5200
> >>client side: qemu-kvm ...
> >>              (qemu) migrate -d tcp:[2312::8274]:5200
> >>
> >>Signed-off-by: Amos Kong<akong@redhat.com>
> >>---
> >>  net.c |    6 ++++++
> >>  1 files changed, 6 insertions(+), 0 deletions(-)
> >>
> >>diff --git a/net.c b/net.c
> >>index d6ce1fa..499ed1d 100644
> >>--- a/net.c
> >>+++ b/net.c
> >>@@ -88,6 +88,12 @@ static int get_str_sep(char *buf, int buf_size, const char **pp, int sep)
> >>      if (!p1)
> >>          return -1;
> >>      len = p1 - p;
> >>+    /* remove brackets which includes hostname */
> >>+    if (*p == '['&&  *(p1-1) == ']') {
> >>+        p += 1;
> >>+        len -= 2;
> >>+    }
> >
> >Sorry, looking again I guess net/slirp.c actually has it's own copy of
> >get_str_sep(), so modifying this doesn't look like it would break
> >anything currently.
> 
> >It might cause some confusion though :). And I think
> >the special handling for brackets should be done in
> >parse_host_port_info() since get_str_sep() is pretty generically named.
> 
> agree.
> 
> >>+
> >>      p1++;
> >>      if (buf_size>  0) {
> >>          if (len>  buf_size - 1)
> >>
> >>
> 
> -- 
> 			Amos.
>

Patch

diff --git a/net.c b/net.c
index d6ce1fa..499ed1d 100644
--- a/net.c
+++ b/net.c
@@ -88,6 +88,12 @@  static int get_str_sep(char *buf, int buf_size, const char **pp, int sep)
     if (!p1)
         return -1;
     len = p1 - p;
+    /* remove brackets which includes hostname */
+    if (*p == '[' && *(p1-1) == ']') {
+        p += 1;
+        len -= 2;
+    }
+
     p1++;
     if (buf_size > 0) {
         if (len > buf_size - 1)