You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by Andrew Stitcher <as...@apache.org> on 2011/08/10 00:25:04 UTC

Review Request: IPv6 support for C++ (Windows and Linux) client/broker and python

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1440/
-----------------------------------------------------------

Review request for qpid, Alan Conway, Gordon Sim, Kenneth Giusti, Ted Ross, Steve Huston, and Cliff Jansen.


Summary
-------

This is my proposed code change for C++ and Python to support IPv6:

It includes a few different items:

IPv6 support at the socket level
- The C++ broker now listens on both TCPv4 and TCPv6 sockets for connections
  and accepts from both in the regular tcp transport.
- The C++ client will connect to both TCPv4 and TCPv6 connections. If a name
  resolves to both v4 and v6 addresses they will be tried in turn as is
  required for correct functioning in a mixed network.
- The necessary Windows code is also present but more lightly tested.
- The Python client will now similarly connect to v6 as well as v4 addresses

Connection URL syntax and parsing updated to include literal IPv6 addresses
- For example "[::1]" is now allowed
- C++ and Python URL parsers updated (very annoying that the parse different syntaxes)

Simple system test to the C++ build to smoke test IPv6 support.
- This only uses the IPv6 loopback address though


This addresses bugs QPID-3404, QPID-3405, QPID-3406, QPID-3407, and QPID-3409.
    https://issues.apache.org/jira/browse/QPID-3404
    https://issues.apache.org/jira/browse/QPID-3405
    https://issues.apache.org/jira/browse/QPID-3406
    https://issues.apache.org/jira/browse/QPID-3407
    https://issues.apache.org/jira/browse/QPID-3409


Diffs
-----

  /trunk/qpid/cpp/src/qpid/Address.cpp 1155572 
  /trunk/qpid/cpp/src/qpid/Url.cpp 1155572 
  /trunk/qpid/cpp/src/qpid/broker/windows/SslProtocolFactory.cpp 1155572 
  /trunk/qpid/cpp/src/qpid/sys/Socket.h 1155572 
  /trunk/qpid/cpp/src/qpid/sys/SocketAddress.h 1155572 
  /trunk/qpid/cpp/src/qpid/sys/TCPIOPlugin.cpp 1155572 
  /trunk/qpid/cpp/src/qpid/sys/epoll/EpollPoller.cpp 1155572 
  /trunk/qpid/cpp/src/qpid/sys/posix/AsynchIO.cpp 1155572 
  /trunk/qpid/cpp/src/qpid/sys/posix/Socket.cpp 1155572 
  /trunk/qpid/cpp/src/qpid/sys/posix/SocketAddress.cpp 1155572 
  /trunk/qpid/cpp/src/qpid/sys/windows/AsynchIO.cpp 1155572 
  /trunk/qpid/cpp/src/qpid/sys/windows/AsynchIoResult.h 1155572 
  /trunk/qpid/cpp/src/qpid/sys/windows/Socket.cpp 1155572 
  /trunk/qpid/cpp/src/qpid/sys/windows/SocketAddress.cpp 1155572 
  /trunk/qpid/cpp/src/qpid/sys/windows/SslAsynchIO.h 1155572 
  /trunk/qpid/cpp/src/tests/Makefile.am 1155572 
  /trunk/qpid/cpp/src/tests/Url.cpp 1155572 
  /trunk/qpid/cpp/src/tests/cluster_test_logs.py 1155572 
  /trunk/qpid/cpp/src/tests/ipv6_test PRE-CREATION 
  /trunk/qpid/extras/qmf/src/py/qmf/console.py 1155572 
  /trunk/qpid/python/qpid/util.py 1155572 
  /trunk/qpid/tools/src/py/qpid-route 1155572 

Diff: https://reviews.apache.org/r/1440/diff


Testing
-------

Tested (amongst other things) with included system test
Tested lightly by hand on Windows


Thanks,

Andrew


Re: Review Request: IPv6 support for C++ (Windows and Linux) client/broker and python

Posted by "Weston M. Price" <wp...@redhat.com>.
On Aug 10, 2011, at 1:29 PM, rajith attapattu wrote:

> 
> 
>> On 2011-08-10 14:15:46, Gordon Sim wrote:
>>> As you have noted the python client doesn't support the AMQP 0-10 defined url scheme. The c++ messaging API consequently supports the form of url used by python in addition. That code is in src/qpid/cpp/amqp_0_10/SimpleUrlParser.cpp and would (I believe) break for IPv6 addresses.
>> 
>> Andrew Stitcher wrote:
>>    I didn't spot this code, what is it used for? It doesn't seem to have been in the path for anything I tried. In other words nothing failed for not changing this parser.
>> 
>>    Incidentally why (oh why, oh why) do we have *two* parsers in C++ for the amqp_0_10 url scheme? I thought the one I fixed was that parser.
>> 
>> Gordon Sim wrote:
>>    SimpleUrlParser does not parse the 0-10 URL scheme (i.e. that defined by the spec). It was introduced to provide an alternative that was compatible with the form used by the python client and is only used by the messaging API for URLs that don't begin with the 'amqp' scheme identifier (the ipv6 test includes that identifier which is why it would not use this codepath). However, looking at the latest URL code it appears that the qpid::Url class would now be sufficient as it no longer restricts itself to the official scheme...
>> 
>> Gordon Sim wrote:
>>    Fyi: I have now removed SimpleUrlParser (r1156262) so this point can be ignored.
>> 
>> Andrew Stitcher wrote:
>>    Ah I understand, I think it would be nice to have a single compatible superset syntax across all the implementations. I have an idea for something I think would work.
> 
> +1 on the single syntax across all implementations. The java client could certainly make use of a better connection string syntax.

+1

> Something along the lines of "protocol://host:port; {k1:v1,.....kn:vn} would be nice, where the optional set of properties allowing implementation specific properties in addition to a bunch of common props across all clients.
> A property value can be another map ex k:{...}, a list k:[] or just a string.
> 
> 
> - rajith
> 
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1440/#review1376
> -----------------------------------------------------------
> 
> 
> On 2011-08-09 22:26:16, Andrew Stitcher wrote:
>> 
>> -----------------------------------------------------------
>> This is an automatically generated e-mail. To reply, visit:
>> https://reviews.apache.org/r/1440/
>> -----------------------------------------------------------
>> 
>> (Updated 2011-08-09 22:26:16)
>> 
>> 
>> Review request for qpid, Alan Conway, Gordon Sim, Kenneth Giusti, Ted Ross, Steve Huston, and Cliff Jansen.
>> 
>> 
>> Summary
>> -------
>> 
>> This is my proposed code change for C++ and Python to support IPv6:
>> 
>> It includes a few different items:
>> 
>> IPv6 support at the socket level
>> - The C++ broker now listens on both TCPv4 and TCPv6 sockets for connections
>>  and accepts from both in the regular tcp transport.
>> - The C++ client will connect to both TCPv4 and TCPv6 connections. If a name
>>  resolves to both v4 and v6 addresses they will be tried in turn as is
>>  required for correct functioning in a mixed network.
>> - The necessary Windows code is also present but more lightly tested.
>> - The Python client will now similarly connect to v6 as well as v4 addresses
>> 
>> Connection URL syntax and parsing updated to include literal IPv6 addresses
>> - For example "[::1]" is now allowed
>> - C++ and Python URL parsers updated (very annoying that the parse different syntaxes)
>> 
>> Simple system test to the C++ build to smoke test IPv6 support.
>> - This only uses the IPv6 loopback address though
>> 
>> I have this change broken down into smaller patches if anyone would rather see it that way. It's just easier
>> to put it up for review in one big glob.
>> 
>> 
>> This addresses bugs QPID-3404, QPID-3405, QPID-3406, QPID-3407, and QPID-3409.
>>    https://issues.apache.org/jira/browse/QPID-3404
>>    https://issues.apache.org/jira/browse/QPID-3405
>>    https://issues.apache.org/jira/browse/QPID-3406
>>    https://issues.apache.org/jira/browse/QPID-3407
>>    https://issues.apache.org/jira/browse/QPID-3409
>> 
>> 
>> Diffs
>> -----
>> 
>>  /trunk/qpid/cpp/src/qpid/Address.cpp 1155572 
>>  /trunk/qpid/cpp/src/qpid/Url.cpp 1155572 
>>  /trunk/qpid/cpp/src/qpid/broker/windows/SslProtocolFactory.cpp 1155572 
>>  /trunk/qpid/cpp/src/qpid/sys/Socket.h 1155572 
>>  /trunk/qpid/cpp/src/qpid/sys/SocketAddress.h 1155572 
>>  /trunk/qpid/cpp/src/qpid/sys/TCPIOPlugin.cpp 1155572 
>>  /trunk/qpid/cpp/src/qpid/sys/epoll/EpollPoller.cpp 1155572 
>>  /trunk/qpid/cpp/src/qpid/sys/posix/AsynchIO.cpp 1155572 
>>  /trunk/qpid/cpp/src/qpid/sys/posix/Socket.cpp 1155572 
>>  /trunk/qpid/cpp/src/qpid/sys/posix/SocketAddress.cpp 1155572 
>>  /trunk/qpid/cpp/src/qpid/sys/windows/AsynchIO.cpp 1155572 
>>  /trunk/qpid/cpp/src/qpid/sys/windows/AsynchIoResult.h 1155572 
>>  /trunk/qpid/cpp/src/qpid/sys/windows/Socket.cpp 1155572 
>>  /trunk/qpid/cpp/src/qpid/sys/windows/SocketAddress.cpp 1155572 
>>  /trunk/qpid/cpp/src/qpid/sys/windows/SslAsynchIO.h 1155572 
>>  /trunk/qpid/cpp/src/tests/Makefile.am 1155572 
>>  /trunk/qpid/cpp/src/tests/Url.cpp 1155572 
>>  /trunk/qpid/cpp/src/tests/cluster_test_logs.py 1155572 
>>  /trunk/qpid/cpp/src/tests/ipv6_test PRE-CREATION 
>>  /trunk/qpid/extras/qmf/src/py/qmf/console.py 1155572 
>>  /trunk/qpid/python/qpid/util.py 1155572 
>>  /trunk/qpid/tools/src/py/qpid-route 1155572 
>> 
>> Diff: https://reviews.apache.org/r/1440/diff
>> 
>> 
>> Testing
>> -------
>> 
>> Tested (amongst other things) with included system test
>> Tested lightly by hand on Windows
>> 
>> 
>> Thanks,
>> 
>> Andrew
>> 
>> 
> 


---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org


Re: Review Request: IPv6 support for C++ (Windows and Linux) client/broker and python

Posted by rajith attapattu <ra...@gmail.com>.

> On 2011-08-10 14:15:46, Gordon Sim wrote:
> > As you have noted the python client doesn't support the AMQP 0-10 defined url scheme. The c++ messaging API consequently supports the form of url used by python in addition. That code is in src/qpid/cpp/amqp_0_10/SimpleUrlParser.cpp and would (I believe) break for IPv6 addresses.
> 
> Andrew Stitcher wrote:
>     I didn't spot this code, what is it used for? It doesn't seem to have been in the path for anything I tried. In other words nothing failed for not changing this parser.
>     
>     Incidentally why (oh why, oh why) do we have *two* parsers in C++ for the amqp_0_10 url scheme? I thought the one I fixed was that parser.
> 
> Gordon Sim wrote:
>     SimpleUrlParser does not parse the 0-10 URL scheme (i.e. that defined by the spec). It was introduced to provide an alternative that was compatible with the form used by the python client and is only used by the messaging API for URLs that don't begin with the 'amqp' scheme identifier (the ipv6 test includes that identifier which is why it would not use this codepath). However, looking at the latest URL code it appears that the qpid::Url class would now be sufficient as it no longer restricts itself to the official scheme...
> 
> Gordon Sim wrote:
>     Fyi: I have now removed SimpleUrlParser (r1156262) so this point can be ignored.
> 
> Andrew Stitcher wrote:
>     Ah I understand, I think it would be nice to have a single compatible superset syntax across all the implementations. I have an idea for something I think would work.

+1 on the single syntax across all implementations. The java client could certainly make use of a better connection string syntax.
Something along the lines of "protocol://host:port; {k1:v1,.....kn:vn} would be nice, where the optional set of properties allowing implementation specific properties in addition to a bunch of common props across all clients.
A property value can be another map ex k:{...}, a list k:[] or just a string.


- rajith


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1440/#review1376
-----------------------------------------------------------


On 2011-08-09 22:26:16, Andrew Stitcher wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1440/
> -----------------------------------------------------------
> 
> (Updated 2011-08-09 22:26:16)
> 
> 
> Review request for qpid, Alan Conway, Gordon Sim, Kenneth Giusti, Ted Ross, Steve Huston, and Cliff Jansen.
> 
> 
> Summary
> -------
> 
> This is my proposed code change for C++ and Python to support IPv6:
> 
> It includes a few different items:
> 
> IPv6 support at the socket level
> - The C++ broker now listens on both TCPv4 and TCPv6 sockets for connections
>   and accepts from both in the regular tcp transport.
> - The C++ client will connect to both TCPv4 and TCPv6 connections. If a name
>   resolves to both v4 and v6 addresses they will be tried in turn as is
>   required for correct functioning in a mixed network.
> - The necessary Windows code is also present but more lightly tested.
> - The Python client will now similarly connect to v6 as well as v4 addresses
> 
> Connection URL syntax and parsing updated to include literal IPv6 addresses
> - For example "[::1]" is now allowed
> - C++ and Python URL parsers updated (very annoying that the parse different syntaxes)
> 
> Simple system test to the C++ build to smoke test IPv6 support.
> - This only uses the IPv6 loopback address though
> 
> I have this change broken down into smaller patches if anyone would rather see it that way. It's just easier
> to put it up for review in one big glob.
> 
> 
> This addresses bugs QPID-3404, QPID-3405, QPID-3406, QPID-3407, and QPID-3409.
>     https://issues.apache.org/jira/browse/QPID-3404
>     https://issues.apache.org/jira/browse/QPID-3405
>     https://issues.apache.org/jira/browse/QPID-3406
>     https://issues.apache.org/jira/browse/QPID-3407
>     https://issues.apache.org/jira/browse/QPID-3409
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/cpp/src/qpid/Address.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/Url.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/broker/windows/SslProtocolFactory.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/Socket.h 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/SocketAddress.h 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/TCPIOPlugin.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/epoll/EpollPoller.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/posix/AsynchIO.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/posix/Socket.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/posix/SocketAddress.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/AsynchIO.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/AsynchIoResult.h 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/Socket.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/SocketAddress.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/SslAsynchIO.h 1155572 
>   /trunk/qpid/cpp/src/tests/Makefile.am 1155572 
>   /trunk/qpid/cpp/src/tests/Url.cpp 1155572 
>   /trunk/qpid/cpp/src/tests/cluster_test_logs.py 1155572 
>   /trunk/qpid/cpp/src/tests/ipv6_test PRE-CREATION 
>   /trunk/qpid/extras/qmf/src/py/qmf/console.py 1155572 
>   /trunk/qpid/python/qpid/util.py 1155572 
>   /trunk/qpid/tools/src/py/qpid-route 1155572 
> 
> Diff: https://reviews.apache.org/r/1440/diff
> 
> 
> Testing
> -------
> 
> Tested (amongst other things) with included system test
> Tested lightly by hand on Windows
> 
> 
> Thanks,
> 
> Andrew
> 
>


Re: Review Request: IPv6 support for C++ (Windows and Linux) client/broker and python

Posted by Gordon Sim <gs...@redhat.com>.

> On 2011-08-10 14:15:46, Gordon Sim wrote:
> > As you have noted the python client doesn't support the AMQP 0-10 defined url scheme. The c++ messaging API consequently supports the form of url used by python in addition. That code is in src/qpid/cpp/amqp_0_10/SimpleUrlParser.cpp and would (I believe) break for IPv6 addresses.
> 
> Andrew Stitcher wrote:
>     I didn't spot this code, what is it used for? It doesn't seem to have been in the path for anything I tried. In other words nothing failed for not changing this parser.
>     
>     Incidentally why (oh why, oh why) do we have *two* parsers in C++ for the amqp_0_10 url scheme? I thought the one I fixed was that parser.
> 
> Gordon Sim wrote:
>     SimpleUrlParser does not parse the 0-10 URL scheme (i.e. that defined by the spec). It was introduced to provide an alternative that was compatible with the form used by the python client and is only used by the messaging API for URLs that don't begin with the 'amqp' scheme identifier (the ipv6 test includes that identifier which is why it would not use this codepath). However, looking at the latest URL code it appears that the qpid::Url class would now be sufficient as it no longer restricts itself to the official scheme...

Fyi: I have now removed SimpleUrlParser (r1156262) so this point can be ignored.


- Gordon


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1440/#review1376
-----------------------------------------------------------


On 2011-08-09 22:26:16, Andrew Stitcher wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1440/
> -----------------------------------------------------------
> 
> (Updated 2011-08-09 22:26:16)
> 
> 
> Review request for qpid, Alan Conway, Gordon Sim, Kenneth Giusti, Ted Ross, Steve Huston, and Cliff Jansen.
> 
> 
> Summary
> -------
> 
> This is my proposed code change for C++ and Python to support IPv6:
> 
> It includes a few different items:
> 
> IPv6 support at the socket level
> - The C++ broker now listens on both TCPv4 and TCPv6 sockets for connections
>   and accepts from both in the regular tcp transport.
> - The C++ client will connect to both TCPv4 and TCPv6 connections. If a name
>   resolves to both v4 and v6 addresses they will be tried in turn as is
>   required for correct functioning in a mixed network.
> - The necessary Windows code is also present but more lightly tested.
> - The Python client will now similarly connect to v6 as well as v4 addresses
> 
> Connection URL syntax and parsing updated to include literal IPv6 addresses
> - For example "[::1]" is now allowed
> - C++ and Python URL parsers updated (very annoying that the parse different syntaxes)
> 
> Simple system test to the C++ build to smoke test IPv6 support.
> - This only uses the IPv6 loopback address though
> 
> I have this change broken down into smaller patches if anyone would rather see it that way. It's just easier
> to put it up for review in one big glob.
> 
> 
> This addresses bugs QPID-3404, QPID-3405, QPID-3406, QPID-3407, and QPID-3409.
>     https://issues.apache.org/jira/browse/QPID-3404
>     https://issues.apache.org/jira/browse/QPID-3405
>     https://issues.apache.org/jira/browse/QPID-3406
>     https://issues.apache.org/jira/browse/QPID-3407
>     https://issues.apache.org/jira/browse/QPID-3409
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/cpp/src/qpid/Address.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/Url.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/broker/windows/SslProtocolFactory.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/Socket.h 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/SocketAddress.h 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/TCPIOPlugin.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/epoll/EpollPoller.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/posix/AsynchIO.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/posix/Socket.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/posix/SocketAddress.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/AsynchIO.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/AsynchIoResult.h 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/Socket.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/SocketAddress.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/SslAsynchIO.h 1155572 
>   /trunk/qpid/cpp/src/tests/Makefile.am 1155572 
>   /trunk/qpid/cpp/src/tests/Url.cpp 1155572 
>   /trunk/qpid/cpp/src/tests/cluster_test_logs.py 1155572 
>   /trunk/qpid/cpp/src/tests/ipv6_test PRE-CREATION 
>   /trunk/qpid/extras/qmf/src/py/qmf/console.py 1155572 
>   /trunk/qpid/python/qpid/util.py 1155572 
>   /trunk/qpid/tools/src/py/qpid-route 1155572 
> 
> Diff: https://reviews.apache.org/r/1440/diff
> 
> 
> Testing
> -------
> 
> Tested (amongst other things) with included system test
> Tested lightly by hand on Windows
> 
> 
> Thanks,
> 
> Andrew
> 
>


Re: Review Request: IPv6 support for C++ (Windows and Linux) client/broker and python

Posted by Andrew Stitcher <as...@apache.org>.

> On 2011-08-10 14:15:46, Gordon Sim wrote:
> > As you have noted the python client doesn't support the AMQP 0-10 defined url scheme. The c++ messaging API consequently supports the form of url used by python in addition. That code is in src/qpid/cpp/amqp_0_10/SimpleUrlParser.cpp and would (I believe) break for IPv6 addresses.
> 
> Andrew Stitcher wrote:
>     I didn't spot this code, what is it used for? It doesn't seem to have been in the path for anything I tried. In other words nothing failed for not changing this parser.
>     
>     Incidentally why (oh why, oh why) do we have *two* parsers in C++ for the amqp_0_10 url scheme? I thought the one I fixed was that parser.
> 
> Gordon Sim wrote:
>     SimpleUrlParser does not parse the 0-10 URL scheme (i.e. that defined by the spec). It was introduced to provide an alternative that was compatible with the form used by the python client and is only used by the messaging API for URLs that don't begin with the 'amqp' scheme identifier (the ipv6 test includes that identifier which is why it would not use this codepath). However, looking at the latest URL code it appears that the qpid::Url class would now be sufficient as it no longer restricts itself to the official scheme...
> 
> Gordon Sim wrote:
>     Fyi: I have now removed SimpleUrlParser (r1156262) so this point can be ignored.

Ah I understand, I think it would be nice to have a single compatible superset syntax across all the implementations. I have an idea for something I think would work.


- Andrew


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1440/#review1376
-----------------------------------------------------------


On 2011-08-09 22:26:16, Andrew Stitcher wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1440/
> -----------------------------------------------------------
> 
> (Updated 2011-08-09 22:26:16)
> 
> 
> Review request for qpid, Alan Conway, Gordon Sim, Kenneth Giusti, Ted Ross, Steve Huston, and Cliff Jansen.
> 
> 
> Summary
> -------
> 
> This is my proposed code change for C++ and Python to support IPv6:
> 
> It includes a few different items:
> 
> IPv6 support at the socket level
> - The C++ broker now listens on both TCPv4 and TCPv6 sockets for connections
>   and accepts from both in the regular tcp transport.
> - The C++ client will connect to both TCPv4 and TCPv6 connections. If a name
>   resolves to both v4 and v6 addresses they will be tried in turn as is
>   required for correct functioning in a mixed network.
> - The necessary Windows code is also present but more lightly tested.
> - The Python client will now similarly connect to v6 as well as v4 addresses
> 
> Connection URL syntax and parsing updated to include literal IPv6 addresses
> - For example "[::1]" is now allowed
> - C++ and Python URL parsers updated (very annoying that the parse different syntaxes)
> 
> Simple system test to the C++ build to smoke test IPv6 support.
> - This only uses the IPv6 loopback address though
> 
> I have this change broken down into smaller patches if anyone would rather see it that way. It's just easier
> to put it up for review in one big glob.
> 
> 
> This addresses bugs QPID-3404, QPID-3405, QPID-3406, QPID-3407, and QPID-3409.
>     https://issues.apache.org/jira/browse/QPID-3404
>     https://issues.apache.org/jira/browse/QPID-3405
>     https://issues.apache.org/jira/browse/QPID-3406
>     https://issues.apache.org/jira/browse/QPID-3407
>     https://issues.apache.org/jira/browse/QPID-3409
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/cpp/src/qpid/Address.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/Url.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/broker/windows/SslProtocolFactory.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/Socket.h 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/SocketAddress.h 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/TCPIOPlugin.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/epoll/EpollPoller.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/posix/AsynchIO.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/posix/Socket.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/posix/SocketAddress.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/AsynchIO.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/AsynchIoResult.h 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/Socket.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/SocketAddress.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/SslAsynchIO.h 1155572 
>   /trunk/qpid/cpp/src/tests/Makefile.am 1155572 
>   /trunk/qpid/cpp/src/tests/Url.cpp 1155572 
>   /trunk/qpid/cpp/src/tests/cluster_test_logs.py 1155572 
>   /trunk/qpid/cpp/src/tests/ipv6_test PRE-CREATION 
>   /trunk/qpid/extras/qmf/src/py/qmf/console.py 1155572 
>   /trunk/qpid/python/qpid/util.py 1155572 
>   /trunk/qpid/tools/src/py/qpid-route 1155572 
> 
> Diff: https://reviews.apache.org/r/1440/diff
> 
> 
> Testing
> -------
> 
> Tested (amongst other things) with included system test
> Tested lightly by hand on Windows
> 
> 
> Thanks,
> 
> Andrew
> 
>


Re: Review Request: IPv6 support for C++ (Windows and Linux) client/broker and python

Posted by Gordon Sim <gs...@redhat.com>.

> On 2011-08-10 14:15:46, Gordon Sim wrote:
> > As you have noted the python client doesn't support the AMQP 0-10 defined url scheme. The c++ messaging API consequently supports the form of url used by python in addition. That code is in src/qpid/cpp/amqp_0_10/SimpleUrlParser.cpp and would (I believe) break for IPv6 addresses.
> 
> Andrew Stitcher wrote:
>     I didn't spot this code, what is it used for? It doesn't seem to have been in the path for anything I tried. In other words nothing failed for not changing this parser.
>     
>     Incidentally why (oh why, oh why) do we have *two* parsers in C++ for the amqp_0_10 url scheme? I thought the one I fixed was that parser.

SimpleUrlParser does not parse the 0-10 URL scheme (i.e. that defined by the spec). It was introduced to provide an alternative that was compatible with the form used by the python client and is only used by the messaging API for URLs that don't begin with the 'amqp' scheme identifier (the ipv6 test includes that identifier which is why it would not use this codepath). However, looking at the latest URL code it appears that the qpid::Url class would now be sufficient as it no longer restricts itself to the official scheme...


- Gordon


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1440/#review1376
-----------------------------------------------------------


On 2011-08-09 22:26:16, Andrew Stitcher wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1440/
> -----------------------------------------------------------
> 
> (Updated 2011-08-09 22:26:16)
> 
> 
> Review request for qpid, Alan Conway, Gordon Sim, Kenneth Giusti, Ted Ross, Steve Huston, and Cliff Jansen.
> 
> 
> Summary
> -------
> 
> This is my proposed code change for C++ and Python to support IPv6:
> 
> It includes a few different items:
> 
> IPv6 support at the socket level
> - The C++ broker now listens on both TCPv4 and TCPv6 sockets for connections
>   and accepts from both in the regular tcp transport.
> - The C++ client will connect to both TCPv4 and TCPv6 connections. If a name
>   resolves to both v4 and v6 addresses they will be tried in turn as is
>   required for correct functioning in a mixed network.
> - The necessary Windows code is also present but more lightly tested.
> - The Python client will now similarly connect to v6 as well as v4 addresses
> 
> Connection URL syntax and parsing updated to include literal IPv6 addresses
> - For example "[::1]" is now allowed
> - C++ and Python URL parsers updated (very annoying that the parse different syntaxes)
> 
> Simple system test to the C++ build to smoke test IPv6 support.
> - This only uses the IPv6 loopback address though
> 
> I have this change broken down into smaller patches if anyone would rather see it that way. It's just easier
> to put it up for review in one big glob.
> 
> 
> This addresses bugs QPID-3404, QPID-3405, QPID-3406, QPID-3407, and QPID-3409.
>     https://issues.apache.org/jira/browse/QPID-3404
>     https://issues.apache.org/jira/browse/QPID-3405
>     https://issues.apache.org/jira/browse/QPID-3406
>     https://issues.apache.org/jira/browse/QPID-3407
>     https://issues.apache.org/jira/browse/QPID-3409
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/cpp/src/qpid/Address.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/Url.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/broker/windows/SslProtocolFactory.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/Socket.h 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/SocketAddress.h 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/TCPIOPlugin.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/epoll/EpollPoller.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/posix/AsynchIO.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/posix/Socket.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/posix/SocketAddress.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/AsynchIO.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/AsynchIoResult.h 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/Socket.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/SocketAddress.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/SslAsynchIO.h 1155572 
>   /trunk/qpid/cpp/src/tests/Makefile.am 1155572 
>   /trunk/qpid/cpp/src/tests/Url.cpp 1155572 
>   /trunk/qpid/cpp/src/tests/cluster_test_logs.py 1155572 
>   /trunk/qpid/cpp/src/tests/ipv6_test PRE-CREATION 
>   /trunk/qpid/extras/qmf/src/py/qmf/console.py 1155572 
>   /trunk/qpid/python/qpid/util.py 1155572 
>   /trunk/qpid/tools/src/py/qpid-route 1155572 
> 
> Diff: https://reviews.apache.org/r/1440/diff
> 
> 
> Testing
> -------
> 
> Tested (amongst other things) with included system test
> Tested lightly by hand on Windows
> 
> 
> Thanks,
> 
> Andrew
> 
>


Re: Review Request: IPv6 support for C++ (Windows and Linux) client/broker and python

Posted by Andrew Stitcher <as...@apache.org>.

> On 2011-08-10 14:15:46, Gordon Sim wrote:
> > As you have noted the python client doesn't support the AMQP 0-10 defined url scheme. The c++ messaging API consequently supports the form of url used by python in addition. That code is in src/qpid/cpp/amqp_0_10/SimpleUrlParser.cpp and would (I believe) break for IPv6 addresses.

I didn't spot this code, what is it used for? It doesn't seem to have been in the path for anything I tried. In other words nothing failed for not changing this parser.

Incidentally why (oh why, oh why) do we have *two* parsers in C++ for the amqp_0_10 url scheme? I thought the one I fixed was that parser.


> On 2011-08-10 14:15:46, Gordon Sim wrote:
> > /trunk/qpid/cpp/src/tests/Makefile.am, line 303
> > <https://reviews.apache.org/r/1440/diff/1/?file=31833#file31833line303>
> >
> >     Just for completeness, should this also be  added to the CMake build?

Sigh, yes it should, however these tests are a mess overall especially as any shell scripted tests require translation to a windows scripting language - currently powershell, but neither federation or clustering work on Windows.

I'll work on getting it into the Unix cmake build, I'm not sure about the Windows build though.


- Andrew


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1440/#review1376
-----------------------------------------------------------


On 2011-08-09 22:26:16, Andrew Stitcher wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1440/
> -----------------------------------------------------------
> 
> (Updated 2011-08-09 22:26:16)
> 
> 
> Review request for qpid, Alan Conway, Gordon Sim, Kenneth Giusti, Ted Ross, Steve Huston, and Cliff Jansen.
> 
> 
> Summary
> -------
> 
> This is my proposed code change for C++ and Python to support IPv6:
> 
> It includes a few different items:
> 
> IPv6 support at the socket level
> - The C++ broker now listens on both TCPv4 and TCPv6 sockets for connections
>   and accepts from both in the regular tcp transport.
> - The C++ client will connect to both TCPv4 and TCPv6 connections. If a name
>   resolves to both v4 and v6 addresses they will be tried in turn as is
>   required for correct functioning in a mixed network.
> - The necessary Windows code is also present but more lightly tested.
> - The Python client will now similarly connect to v6 as well as v4 addresses
> 
> Connection URL syntax and parsing updated to include literal IPv6 addresses
> - For example "[::1]" is now allowed
> - C++ and Python URL parsers updated (very annoying that the parse different syntaxes)
> 
> Simple system test to the C++ build to smoke test IPv6 support.
> - This only uses the IPv6 loopback address though
> 
> I have this change broken down into smaller patches if anyone would rather see it that way. It's just easier
> to put it up for review in one big glob.
> 
> 
> This addresses bugs QPID-3404, QPID-3405, QPID-3406, QPID-3407, and QPID-3409.
>     https://issues.apache.org/jira/browse/QPID-3404
>     https://issues.apache.org/jira/browse/QPID-3405
>     https://issues.apache.org/jira/browse/QPID-3406
>     https://issues.apache.org/jira/browse/QPID-3407
>     https://issues.apache.org/jira/browse/QPID-3409
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/cpp/src/qpid/Address.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/Url.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/broker/windows/SslProtocolFactory.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/Socket.h 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/SocketAddress.h 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/TCPIOPlugin.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/epoll/EpollPoller.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/posix/AsynchIO.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/posix/Socket.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/posix/SocketAddress.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/AsynchIO.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/AsynchIoResult.h 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/Socket.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/SocketAddress.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/SslAsynchIO.h 1155572 
>   /trunk/qpid/cpp/src/tests/Makefile.am 1155572 
>   /trunk/qpid/cpp/src/tests/Url.cpp 1155572 
>   /trunk/qpid/cpp/src/tests/cluster_test_logs.py 1155572 
>   /trunk/qpid/cpp/src/tests/ipv6_test PRE-CREATION 
>   /trunk/qpid/extras/qmf/src/py/qmf/console.py 1155572 
>   /trunk/qpid/python/qpid/util.py 1155572 
>   /trunk/qpid/tools/src/py/qpid-route 1155572 
> 
> Diff: https://reviews.apache.org/r/1440/diff
> 
> 
> Testing
> -------
> 
> Tested (amongst other things) with included system test
> Tested lightly by hand on Windows
> 
> 
> Thanks,
> 
> Andrew
> 
>


Re: Review Request: IPv6 support for C++ (Windows and Linux) client/broker and python

Posted by Gordon Sim <gs...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1440/#review1376
-----------------------------------------------------------


As you have noted the python client doesn't support the AMQP 0-10 defined url scheme. The c++ messaging API consequently supports the form of url used by python in addition. That code is in src/qpid/cpp/amqp_0_10/SimpleUrlParser.cpp and would (I believe) break for IPv6 addresses.


/trunk/qpid/cpp/src/tests/Makefile.am
<https://reviews.apache.org/r/1440/#comment3158>

    Just for completeness, should this also be  added to the CMake build?


- Gordon


On 2011-08-09 22:26:16, Andrew Stitcher wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1440/
> -----------------------------------------------------------
> 
> (Updated 2011-08-09 22:26:16)
> 
> 
> Review request for qpid, Alan Conway, Gordon Sim, Kenneth Giusti, Ted Ross, Steve Huston, and Cliff Jansen.
> 
> 
> Summary
> -------
> 
> This is my proposed code change for C++ and Python to support IPv6:
> 
> It includes a few different items:
> 
> IPv6 support at the socket level
> - The C++ broker now listens on both TCPv4 and TCPv6 sockets for connections
>   and accepts from both in the regular tcp transport.
> - The C++ client will connect to both TCPv4 and TCPv6 connections. If a name
>   resolves to both v4 and v6 addresses they will be tried in turn as is
>   required for correct functioning in a mixed network.
> - The necessary Windows code is also present but more lightly tested.
> - The Python client will now similarly connect to v6 as well as v4 addresses
> 
> Connection URL syntax and parsing updated to include literal IPv6 addresses
> - For example "[::1]" is now allowed
> - C++ and Python URL parsers updated (very annoying that the parse different syntaxes)
> 
> Simple system test to the C++ build to smoke test IPv6 support.
> - This only uses the IPv6 loopback address though
> 
> I have this change broken down into smaller patches if anyone would rather see it that way. It's just easier
> to put it up for review in one big glob.
> 
> 
> This addresses bugs QPID-3404, QPID-3405, QPID-3406, QPID-3407, and QPID-3409.
>     https://issues.apache.org/jira/browse/QPID-3404
>     https://issues.apache.org/jira/browse/QPID-3405
>     https://issues.apache.org/jira/browse/QPID-3406
>     https://issues.apache.org/jira/browse/QPID-3407
>     https://issues.apache.org/jira/browse/QPID-3409
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/cpp/src/qpid/Address.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/Url.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/broker/windows/SslProtocolFactory.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/Socket.h 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/SocketAddress.h 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/TCPIOPlugin.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/epoll/EpollPoller.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/posix/AsynchIO.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/posix/Socket.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/posix/SocketAddress.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/AsynchIO.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/AsynchIoResult.h 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/Socket.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/SocketAddress.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/SslAsynchIO.h 1155572 
>   /trunk/qpid/cpp/src/tests/Makefile.am 1155572 
>   /trunk/qpid/cpp/src/tests/Url.cpp 1155572 
>   /trunk/qpid/cpp/src/tests/cluster_test_logs.py 1155572 
>   /trunk/qpid/cpp/src/tests/ipv6_test PRE-CREATION 
>   /trunk/qpid/extras/qmf/src/py/qmf/console.py 1155572 
>   /trunk/qpid/python/qpid/util.py 1155572 
>   /trunk/qpid/tools/src/py/qpid-route 1155572 
> 
> Diff: https://reviews.apache.org/r/1440/diff
> 
> 
> Testing
> -------
> 
> Tested (amongst other things) with included system test
> Tested lightly by hand on Windows
> 
> 
> Thanks,
> 
> Andrew
> 
>


Re: Review Request: IPv6 support for C++ (Windows and Linux) client/broker and python

Posted by Gordon Sim <gs...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1440/#review1407
-----------------------------------------------------------

Ship it!


- Gordon


On 2011-08-11 05:36:00, Andrew Stitcher wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1440/
> -----------------------------------------------------------
> 
> (Updated 2011-08-11 05:36:00)
> 
> 
> Review request for qpid, Alan Conway, Gordon Sim, Kenneth Giusti, Ted Ross, Steve Huston, and Cliff Jansen.
> 
> 
> Summary
> -------
> 
> This is my proposed code change for C++ and Python to support IPv6:
> 
> It includes a few different items:
> 
> IPv6 support at the socket level
> - The C++ broker now listens on both TCPv4 and TCPv6 sockets for connections
>   and accepts from both in the regular tcp transport.
> - The C++ client will connect to both TCPv4 and TCPv6 connections. If a name
>   resolves to both v4 and v6 addresses they will be tried in turn as is
>   required for correct functioning in a mixed network.
> - The necessary Windows code is also present but more lightly tested.
> - The Python client will now similarly connect to v6 as well as v4 addresses
> 
> Connection URL syntax and parsing updated to include literal IPv6 addresses
> - For example "[::1]" is now allowed
> - C++ and Python URL parsers updated (very annoying that the parse different syntaxes)
> 
> Simple system test to the C++ build to smoke test IPv6 support.
> - This only uses the IPv6 loopback address though
> 
> I have this change broken down into smaller patches if anyone would rather see it that way. It's just easier
> to put it up for review in one big glob.
> 
> 
> This addresses bugs QPID-3404, QPID-3405, QPID-3406, QPID-3407, and QPID-3409.
>     https://issues.apache.org/jira/browse/QPID-3404
>     https://issues.apache.org/jira/browse/QPID-3405
>     https://issues.apache.org/jira/browse/QPID-3406
>     https://issues.apache.org/jira/browse/QPID-3407
>     https://issues.apache.org/jira/browse/QPID-3409
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/cpp/src/qpid/Address.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/Url.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/broker/windows/SslProtocolFactory.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/Socket.h 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/SocketAddress.h 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/TCPIOPlugin.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/epoll/EpollPoller.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/posix/AsynchIO.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/posix/Socket.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/posix/SocketAddress.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/AsynchIO.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/AsynchIoResult.h 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/Socket.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/SocketAddress.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/SslAsynchIO.h 1155572 
>   /trunk/qpid/cpp/src/tests/CMakeLists.txt 1155572 
>   /trunk/qpid/cpp/src/tests/Makefile.am 1155572 
>   /trunk/qpid/cpp/src/tests/Url.cpp 1155572 
>   /trunk/qpid/cpp/src/tests/cluster_test_logs.py 1155572 
>   /trunk/qpid/cpp/src/tests/ipv6_test PRE-CREATION 
>   /trunk/qpid/extras/qmf/src/py/qmf/console.py 1155572 
>   /trunk/qpid/python/qpid/util.py 1155572 
>   /trunk/qpid/tools/src/py/qpid-route 1155572 
> 
> Diff: https://reviews.apache.org/r/1440/diff
> 
> 
> Testing
> -------
> 
> Tested (amongst other things) with included system test
> Tested lightly by hand on Windows
> 
> 
> Thanks,
> 
> Andrew
> 
>


Re: Review Request: IPv6 support for C++ (Windows and Linux) client/broker and python

Posted by Steve Huston <sh...@riverace.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1440/#review1458
-----------------------------------------------------------

Ship it!


- Steve


On 2011-08-11 05:36:00, Andrew Stitcher wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1440/
> -----------------------------------------------------------
> 
> (Updated 2011-08-11 05:36:00)
> 
> 
> Review request for qpid, Alan Conway, Gordon Sim, Kenneth Giusti, Ted Ross, Steve Huston, and Cliff Jansen.
> 
> 
> Summary
> -------
> 
> This is my proposed code change for C++ and Python to support IPv6:
> 
> It includes a few different items:
> 
> IPv6 support at the socket level
> - The C++ broker now listens on both TCPv4 and TCPv6 sockets for connections
>   and accepts from both in the regular tcp transport.
> - The C++ client will connect to both TCPv4 and TCPv6 connections. If a name
>   resolves to both v4 and v6 addresses they will be tried in turn as is
>   required for correct functioning in a mixed network.
> - The necessary Windows code is also present but more lightly tested.
> - The Python client will now similarly connect to v6 as well as v4 addresses
> 
> Connection URL syntax and parsing updated to include literal IPv6 addresses
> - For example "[::1]" is now allowed
> - C++ and Python URL parsers updated (very annoying that the parse different syntaxes)
> 
> Simple system test to the C++ build to smoke test IPv6 support.
> - This only uses the IPv6 loopback address though
> 
> I have this change broken down into smaller patches if anyone would rather see it that way. It's just easier
> to put it up for review in one big glob.
> 
> 
> This addresses bugs QPID-3404, QPID-3405, QPID-3406, QPID-3407, and QPID-3409.
>     https://issues.apache.org/jira/browse/QPID-3404
>     https://issues.apache.org/jira/browse/QPID-3405
>     https://issues.apache.org/jira/browse/QPID-3406
>     https://issues.apache.org/jira/browse/QPID-3407
>     https://issues.apache.org/jira/browse/QPID-3409
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/cpp/src/qpid/Address.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/Url.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/broker/windows/SslProtocolFactory.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/Socket.h 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/SocketAddress.h 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/TCPIOPlugin.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/epoll/EpollPoller.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/posix/AsynchIO.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/posix/Socket.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/posix/SocketAddress.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/AsynchIO.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/AsynchIoResult.h 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/Socket.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/SocketAddress.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/SslAsynchIO.h 1155572 
>   /trunk/qpid/cpp/src/tests/CMakeLists.txt 1155572 
>   /trunk/qpid/cpp/src/tests/Makefile.am 1155572 
>   /trunk/qpid/cpp/src/tests/Url.cpp 1155572 
>   /trunk/qpid/cpp/src/tests/cluster_test_logs.py 1155572 
>   /trunk/qpid/cpp/src/tests/ipv6_test PRE-CREATION 
>   /trunk/qpid/extras/qmf/src/py/qmf/console.py 1155572 
>   /trunk/qpid/python/qpid/util.py 1155572 
>   /trunk/qpid/tools/src/py/qpid-route 1155572 
> 
> Diff: https://reviews.apache.org/r/1440/diff
> 
> 
> Testing
> -------
> 
> Tested (amongst other things) with included system test
> Tested lightly by hand on Windows
> 
> 
> Thanks,
> 
> Andrew
> 
>


Re: Review Request: IPv6 support for C++ (Windows and Linux) client/broker and python

Posted by Andrew Stitcher <as...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1440/
-----------------------------------------------------------

(Updated 2011-08-11 05:36:00.614686)


Review request for qpid, Alan Conway, Gordon Sim, Kenneth Giusti, Ted Ross, Steve Huston, and Cliff Jansen.


Changes
-------

Updated as per Gordon's review to run test under CMake too (on Linux)


Summary
-------

This is my proposed code change for C++ and Python to support IPv6:

It includes a few different items:

IPv6 support at the socket level
- The C++ broker now listens on both TCPv4 and TCPv6 sockets for connections
  and accepts from both in the regular tcp transport.
- The C++ client will connect to both TCPv4 and TCPv6 connections. If a name
  resolves to both v4 and v6 addresses they will be tried in turn as is
  required for correct functioning in a mixed network.
- The necessary Windows code is also present but more lightly tested.
- The Python client will now similarly connect to v6 as well as v4 addresses

Connection URL syntax and parsing updated to include literal IPv6 addresses
- For example "[::1]" is now allowed
- C++ and Python URL parsers updated (very annoying that the parse different syntaxes)

Simple system test to the C++ build to smoke test IPv6 support.
- This only uses the IPv6 loopback address though

I have this change broken down into smaller patches if anyone would rather see it that way. It's just easier
to put it up for review in one big glob.


This addresses bugs QPID-3404, QPID-3405, QPID-3406, QPID-3407, and QPID-3409.
    https://issues.apache.org/jira/browse/QPID-3404
    https://issues.apache.org/jira/browse/QPID-3405
    https://issues.apache.org/jira/browse/QPID-3406
    https://issues.apache.org/jira/browse/QPID-3407
    https://issues.apache.org/jira/browse/QPID-3409


Diffs (updated)
-----

  /trunk/qpid/cpp/src/qpid/Address.cpp 1155572 
  /trunk/qpid/cpp/src/qpid/Url.cpp 1155572 
  /trunk/qpid/cpp/src/qpid/broker/windows/SslProtocolFactory.cpp 1155572 
  /trunk/qpid/cpp/src/qpid/sys/Socket.h 1155572 
  /trunk/qpid/cpp/src/qpid/sys/SocketAddress.h 1155572 
  /trunk/qpid/cpp/src/qpid/sys/TCPIOPlugin.cpp 1155572 
  /trunk/qpid/cpp/src/qpid/sys/epoll/EpollPoller.cpp 1155572 
  /trunk/qpid/cpp/src/qpid/sys/posix/AsynchIO.cpp 1155572 
  /trunk/qpid/cpp/src/qpid/sys/posix/Socket.cpp 1155572 
  /trunk/qpid/cpp/src/qpid/sys/posix/SocketAddress.cpp 1155572 
  /trunk/qpid/cpp/src/qpid/sys/windows/AsynchIO.cpp 1155572 
  /trunk/qpid/cpp/src/qpid/sys/windows/AsynchIoResult.h 1155572 
  /trunk/qpid/cpp/src/qpid/sys/windows/Socket.cpp 1155572 
  /trunk/qpid/cpp/src/qpid/sys/windows/SocketAddress.cpp 1155572 
  /trunk/qpid/cpp/src/qpid/sys/windows/SslAsynchIO.h 1155572 
  /trunk/qpid/cpp/src/tests/CMakeLists.txt 1155572 
  /trunk/qpid/cpp/src/tests/Makefile.am 1155572 
  /trunk/qpid/cpp/src/tests/Url.cpp 1155572 
  /trunk/qpid/cpp/src/tests/cluster_test_logs.py 1155572 
  /trunk/qpid/cpp/src/tests/ipv6_test PRE-CREATION 
  /trunk/qpid/extras/qmf/src/py/qmf/console.py 1155572 
  /trunk/qpid/python/qpid/util.py 1155572 
  /trunk/qpid/tools/src/py/qpid-route 1155572 

Diff: https://reviews.apache.org/r/1440/diff


Testing
-------

Tested (amongst other things) with included system test
Tested lightly by hand on Windows


Thanks,

Andrew


Re: Review Request: IPv6 support for C++ (Windows and Linux) client/broker and python

Posted by Andrew Stitcher <as...@apache.org>.

> On 2011-08-10 14:06:02, Alan Conway wrote:
> > /trunk/qpid/cpp/src/qpid/sys/epoll/EpollPoller.cpp, line 388
> > <https://reviews.apache.org/r/1440/diff/1/?file=31824#file31824line388>
> >
> >     Curious, how would that happen?

This can now happen in retrying connections: When we try to connect we call ::connect() in non blocking mode and then use the poller to detect success or failure. If we get failure and need to retry then we close the socket which then causes this condition. This fix is really an unfortunate hack, but it seemed the simplest solution to this problem.


- Andrew


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1440/#review1374
-----------------------------------------------------------


On 2011-08-09 22:26:16, Andrew Stitcher wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1440/
> -----------------------------------------------------------
> 
> (Updated 2011-08-09 22:26:16)
> 
> 
> Review request for qpid, Alan Conway, Gordon Sim, Kenneth Giusti, Ted Ross, Steve Huston, and Cliff Jansen.
> 
> 
> Summary
> -------
> 
> This is my proposed code change for C++ and Python to support IPv6:
> 
> It includes a few different items:
> 
> IPv6 support at the socket level
> - The C++ broker now listens on both TCPv4 and TCPv6 sockets for connections
>   and accepts from both in the regular tcp transport.
> - The C++ client will connect to both TCPv4 and TCPv6 connections. If a name
>   resolves to both v4 and v6 addresses they will be tried in turn as is
>   required for correct functioning in a mixed network.
> - The necessary Windows code is also present but more lightly tested.
> - The Python client will now similarly connect to v6 as well as v4 addresses
> 
> Connection URL syntax and parsing updated to include literal IPv6 addresses
> - For example "[::1]" is now allowed
> - C++ and Python URL parsers updated (very annoying that the parse different syntaxes)
> 
> Simple system test to the C++ build to smoke test IPv6 support.
> - This only uses the IPv6 loopback address though
> 
> I have this change broken down into smaller patches if anyone would rather see it that way. It's just easier
> to put it up for review in one big glob.
> 
> 
> This addresses bugs QPID-3404, QPID-3405, QPID-3406, QPID-3407, and QPID-3409.
>     https://issues.apache.org/jira/browse/QPID-3404
>     https://issues.apache.org/jira/browse/QPID-3405
>     https://issues.apache.org/jira/browse/QPID-3406
>     https://issues.apache.org/jira/browse/QPID-3407
>     https://issues.apache.org/jira/browse/QPID-3409
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/cpp/src/qpid/Address.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/Url.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/broker/windows/SslProtocolFactory.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/Socket.h 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/SocketAddress.h 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/TCPIOPlugin.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/epoll/EpollPoller.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/posix/AsynchIO.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/posix/Socket.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/posix/SocketAddress.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/AsynchIO.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/AsynchIoResult.h 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/Socket.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/SocketAddress.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/SslAsynchIO.h 1155572 
>   /trunk/qpid/cpp/src/tests/Makefile.am 1155572 
>   /trunk/qpid/cpp/src/tests/Url.cpp 1155572 
>   /trunk/qpid/cpp/src/tests/cluster_test_logs.py 1155572 
>   /trunk/qpid/cpp/src/tests/ipv6_test PRE-CREATION 
>   /trunk/qpid/extras/qmf/src/py/qmf/console.py 1155572 
>   /trunk/qpid/python/qpid/util.py 1155572 
>   /trunk/qpid/tools/src/py/qpid-route 1155572 
> 
> Diff: https://reviews.apache.org/r/1440/diff
> 
> 
> Testing
> -------
> 
> Tested (amongst other things) with included system test
> Tested lightly by hand on Windows
> 
> 
> Thanks,
> 
> Andrew
> 
>


Re: Review Request: IPv6 support for C++ (Windows and Linux) client/broker and python

Posted by Alan Conway <ac...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1440/#review1374
-----------------------------------------------------------

Ship it!



/trunk/qpid/cpp/src/qpid/sys/epoll/EpollPoller.cpp
<https://reviews.apache.org/r/1440/#comment3157>

    Curious, how would that happen?


- Alan


On 2011-08-09 22:26:16, Andrew Stitcher wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1440/
> -----------------------------------------------------------
> 
> (Updated 2011-08-09 22:26:16)
> 
> 
> Review request for qpid, Alan Conway, Gordon Sim, Kenneth Giusti, Ted Ross, Steve Huston, and Cliff Jansen.
> 
> 
> Summary
> -------
> 
> This is my proposed code change for C++ and Python to support IPv6:
> 
> It includes a few different items:
> 
> IPv6 support at the socket level
> - The C++ broker now listens on both TCPv4 and TCPv6 sockets for connections
>   and accepts from both in the regular tcp transport.
> - The C++ client will connect to both TCPv4 and TCPv6 connections. If a name
>   resolves to both v4 and v6 addresses they will be tried in turn as is
>   required for correct functioning in a mixed network.
> - The necessary Windows code is also present but more lightly tested.
> - The Python client will now similarly connect to v6 as well as v4 addresses
> 
> Connection URL syntax and parsing updated to include literal IPv6 addresses
> - For example "[::1]" is now allowed
> - C++ and Python URL parsers updated (very annoying that the parse different syntaxes)
> 
> Simple system test to the C++ build to smoke test IPv6 support.
> - This only uses the IPv6 loopback address though
> 
> I have this change broken down into smaller patches if anyone would rather see it that way. It's just easier
> to put it up for review in one big glob.
> 
> 
> This addresses bugs QPID-3404, QPID-3405, QPID-3406, QPID-3407, and QPID-3409.
>     https://issues.apache.org/jira/browse/QPID-3404
>     https://issues.apache.org/jira/browse/QPID-3405
>     https://issues.apache.org/jira/browse/QPID-3406
>     https://issues.apache.org/jira/browse/QPID-3407
>     https://issues.apache.org/jira/browse/QPID-3409
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/cpp/src/qpid/Address.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/Url.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/broker/windows/SslProtocolFactory.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/Socket.h 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/SocketAddress.h 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/TCPIOPlugin.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/epoll/EpollPoller.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/posix/AsynchIO.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/posix/Socket.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/posix/SocketAddress.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/AsynchIO.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/AsynchIoResult.h 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/Socket.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/SocketAddress.cpp 1155572 
>   /trunk/qpid/cpp/src/qpid/sys/windows/SslAsynchIO.h 1155572 
>   /trunk/qpid/cpp/src/tests/Makefile.am 1155572 
>   /trunk/qpid/cpp/src/tests/Url.cpp 1155572 
>   /trunk/qpid/cpp/src/tests/cluster_test_logs.py 1155572 
>   /trunk/qpid/cpp/src/tests/ipv6_test PRE-CREATION 
>   /trunk/qpid/extras/qmf/src/py/qmf/console.py 1155572 
>   /trunk/qpid/python/qpid/util.py 1155572 
>   /trunk/qpid/tools/src/py/qpid-route 1155572 
> 
> Diff: https://reviews.apache.org/r/1440/diff
> 
> 
> Testing
> -------
> 
> Tested (amongst other things) with included system test
> Tested lightly by hand on Windows
> 
> 
> Thanks,
> 
> Andrew
> 
>


Re: Review Request: IPv6 support for C++ (Windows and Linux) client/broker and python

Posted by Andrew Stitcher <as...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1440/
-----------------------------------------------------------

(Updated 2011-08-09 22:26:16.375262)


Review request for qpid, Alan Conway, Gordon Sim, Kenneth Giusti, Ted Ross, Steve Huston, and Cliff Jansen.


Summary (updated)
-------

This is my proposed code change for C++ and Python to support IPv6:

It includes a few different items:

IPv6 support at the socket level
- The C++ broker now listens on both TCPv4 and TCPv6 sockets for connections
  and accepts from both in the regular tcp transport.
- The C++ client will connect to both TCPv4 and TCPv6 connections. If a name
  resolves to both v4 and v6 addresses they will be tried in turn as is
  required for correct functioning in a mixed network.
- The necessary Windows code is also present but more lightly tested.
- The Python client will now similarly connect to v6 as well as v4 addresses

Connection URL syntax and parsing updated to include literal IPv6 addresses
- For example "[::1]" is now allowed
- C++ and Python URL parsers updated (very annoying that the parse different syntaxes)

Simple system test to the C++ build to smoke test IPv6 support.
- This only uses the IPv6 loopback address though

I have this change broken down into smaller patches if anyone would rather see it that way. It's just easier
to put it up for review in one big glob.


This addresses bugs QPID-3404, QPID-3405, QPID-3406, QPID-3407, and QPID-3409.
    https://issues.apache.org/jira/browse/QPID-3404
    https://issues.apache.org/jira/browse/QPID-3405
    https://issues.apache.org/jira/browse/QPID-3406
    https://issues.apache.org/jira/browse/QPID-3407
    https://issues.apache.org/jira/browse/QPID-3409


Diffs
-----

  /trunk/qpid/cpp/src/qpid/Address.cpp 1155572 
  /trunk/qpid/cpp/src/qpid/Url.cpp 1155572 
  /trunk/qpid/cpp/src/qpid/broker/windows/SslProtocolFactory.cpp 1155572 
  /trunk/qpid/cpp/src/qpid/sys/Socket.h 1155572 
  /trunk/qpid/cpp/src/qpid/sys/SocketAddress.h 1155572 
  /trunk/qpid/cpp/src/qpid/sys/TCPIOPlugin.cpp 1155572 
  /trunk/qpid/cpp/src/qpid/sys/epoll/EpollPoller.cpp 1155572 
  /trunk/qpid/cpp/src/qpid/sys/posix/AsynchIO.cpp 1155572 
  /trunk/qpid/cpp/src/qpid/sys/posix/Socket.cpp 1155572 
  /trunk/qpid/cpp/src/qpid/sys/posix/SocketAddress.cpp 1155572 
  /trunk/qpid/cpp/src/qpid/sys/windows/AsynchIO.cpp 1155572 
  /trunk/qpid/cpp/src/qpid/sys/windows/AsynchIoResult.h 1155572 
  /trunk/qpid/cpp/src/qpid/sys/windows/Socket.cpp 1155572 
  /trunk/qpid/cpp/src/qpid/sys/windows/SocketAddress.cpp 1155572 
  /trunk/qpid/cpp/src/qpid/sys/windows/SslAsynchIO.h 1155572 
  /trunk/qpid/cpp/src/tests/Makefile.am 1155572 
  /trunk/qpid/cpp/src/tests/Url.cpp 1155572 
  /trunk/qpid/cpp/src/tests/cluster_test_logs.py 1155572 
  /trunk/qpid/cpp/src/tests/ipv6_test PRE-CREATION 
  /trunk/qpid/extras/qmf/src/py/qmf/console.py 1155572 
  /trunk/qpid/python/qpid/util.py 1155572 
  /trunk/qpid/tools/src/py/qpid-route 1155572 

Diff: https://reviews.apache.org/r/1440/diff


Testing
-------

Tested (amongst other things) with included system test
Tested lightly by hand on Windows


Thanks,

Andrew