jmx-dev RFR 8146015: JMXInterfaceBindingTest is failing intermittently for IPv6 addresses

classic Classic list List threaded Threaded
17 messages Options
Reply | Threaded
Open this post in threaded view
|

jmx-dev RFR 8146015: JMXInterfaceBindingTest is failing intermittently for IPv6 addresses

Jaroslav Bachorik
Please, review the following test change

Issue : https://bugs.openjdk.java.net/browse/JDK-8146015
Webrev: http://cr.openjdk.java.net/~jbachorik/8146015/webrev.00

The test fails for IPv6 addresses since the RMI expects an IPv6 address
to be properly wrapped in '[]'. In addition to that the logic for
selecting IP addresses to bind is flawed - it does not check for IP
addresses of multiple adapters but for multiple IP addresses assigned to
'localhost'. In combination with IPv4 & IPv6 this will cause the test to
attempt binding to IPv4 and IPv6 address of the same adapter
simultaneously and the test will fail.

The fix adds the requested wrapping for IPv6 addresses and adjusts the
IP selection logic to iterate over distinct adapters first and prevent
IPv4 and IPv6 address of the same adapter being treated as two addresses
(for the purposes of the test).

Thanks,

-JB-
Reply | Threaded
Open this post in threaded view
|

jmx-dev [ping] Re: RFR 8146015: JMXInterfaceBindingTest is failing intermittently for IPv6 addresses

Jaroslav Bachorik
Gentle reminder ...

On 23.12.2015 11:26, Jaroslav Bachorik wrote:

> Please, review the following test change
>
> Issue : https://bugs.openjdk.java.net/browse/JDK-8146015
> Webrev: http://cr.openjdk.java.net/~jbachorik/8146015/webrev.00
>
> The test fails for IPv6 addresses since the RMI expects an IPv6 address
> to be properly wrapped in '[]'. In addition to that the logic for
> selecting IP addresses to bind is flawed - it does not check for IP
> addresses of multiple adapters but for multiple IP addresses assigned to
> 'localhost'. In combination with IPv4 & IPv6 this will cause the test to
> attempt binding to IPv4 and IPv6 address of the same adapter
> simultaneously and the test will fail.
>
> The fix adds the requested wrapping for IPv6 addresses and adjusts the
> IP selection logic to iterate over distinct adapters first and prevent
> IPv4 and IPv6 address of the same adapter being treated as two addresses
> (for the purposes of the test).
>
> Thanks,
>
> -JB-

Reply | Threaded
Open this post in threaded view
|

jmx-dev [pong] Re: [ping] Re: RFR 8146015: JMXInterfaceBindingTest is failing intermittently for IPv6 addresses

Jaroslav Bachorik
On 4.1.2016 10:05, Jaroslav Bachorik wrote:

> Gentle reminder ...
>
> On 23.12.2015 11:26, Jaroslav Bachorik wrote:
>> Please, review the following test change
>>
>> Issue : https://bugs.openjdk.java.net/browse/JDK-8146015
>> Webrev: http://cr.openjdk.java.net/~jbachorik/8146015/webrev.00
>>
>> The test fails for IPv6 addresses since the RMI expects an IPv6 address
>> to be properly wrapped in '[]'. In addition to that the logic for
>> selecting IP addresses to bind is flawed - it does not check for IP
>> addresses of multiple adapters but for multiple IP addresses assigned to
>> 'localhost'. In combination with IPv4 & IPv6 this will cause the test to
>> attempt binding to IPv4 and IPv6 address of the same adapter
>> simultaneously and the test will fail.
>>
>> The fix adds the requested wrapping for IPv6 addresses and adjusts the
>> IP selection logic to iterate over distinct adapters first and prevent
>> IPv4 and IPv6 address of the same adapter being treated as two addresses
>> (for the purposes of the test).
>>
>> Thanks,
>>
>> -JB-
>

Reply | Threaded
Open this post in threaded view
|

jmx-dev [ding] Re: [pong] Re: [ping] Re: RFR 8146015: JMXInterfaceBindingTest is failing intermittently for IPv6 addresses

Jaroslav Bachorik
On 5.1.2016 15:30, Jaroslav Bachorik wrote:

> On 4.1.2016 10:05, Jaroslav Bachorik wrote:
>> Gentle reminder ...
>>
>> On 23.12.2015 11:26, Jaroslav Bachorik wrote:
>>> Please, review the following test change
>>>
>>> Issue : https://bugs.openjdk.java.net/browse/JDK-8146015
>>> Webrev: http://cr.openjdk.java.net/~jbachorik/8146015/webrev.00
>>>
>>> The test fails for IPv6 addresses since the RMI expects an IPv6 address
>>> to be properly wrapped in '[]'. In addition to that the logic for
>>> selecting IP addresses to bind is flawed - it does not check for IP
>>> addresses of multiple adapters but for multiple IP addresses assigned to
>>> 'localhost'. In combination with IPv4 & IPv6 this will cause the test to
>>> attempt binding to IPv4 and IPv6 address of the same adapter
>>> simultaneously and the test will fail.
>>>
>>> The fix adds the requested wrapping for IPv6 addresses and adjusts the
>>> IP selection logic to iterate over distinct adapters first and prevent
>>> IPv4 and IPv6 address of the same adapter being treated as two addresses
>>> (for the purposes of the test).
>>>
>>> Thanks,
>>>
>>> -JB-
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: jmx-dev [ding] Re: [pong] Re: [ping] Re: RFR 8146015: JMXInterfaceBindingTest is failing intermittently for IPv6 addresses

Daniel Fuchs
Hi,

This looks OK to me.
I'm not sure I understand the full impact of the changes
in getAddressesForLocalHost() though - so hopefully someone
else will jump in to review that part...

best regards,

-- daniel

On 07/01/16 16:02, Jaroslav Bachorik wrote:

> On 5.1.2016 15:30, Jaroslav Bachorik wrote:
>> On 4.1.2016 10:05, Jaroslav Bachorik wrote:
>>> Gentle reminder ...
>>>
>>> On 23.12.2015 11:26, Jaroslav Bachorik wrote:
>>>> Please, review the following test change
>>>>
>>>> Issue : https://bugs.openjdk.java.net/browse/JDK-8146015
>>>> Webrev: http://cr.openjdk.java.net/~jbachorik/8146015/webrev.00
>>>>
>>>> The test fails for IPv6 addresses since the RMI expects an IPv6 address
>>>> to be properly wrapped in '[]'. In addition to that the logic for
>>>> selecting IP addresses to bind is flawed - it does not check for IP
>>>> addresses of multiple adapters but for multiple IP addresses
>>>> assigned to
>>>> 'localhost'. In combination with IPv4 & IPv6 this will cause the
>>>> test to
>>>> attempt binding to IPv4 and IPv6 address of the same adapter
>>>> simultaneously and the test will fail.
>>>>
>>>> The fix adds the requested wrapping for IPv6 addresses and adjusts the
>>>> IP selection logic to iterate over distinct adapters first and prevent
>>>> IPv4 and IPv6 address of the same adapter being treated as two
>>>> addresses
>>>> (for the purposes of the test).
>>>>
>>>> Thanks,
>>>>
>>>> -JB-
>>>
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: jmx-dev [ding] Re: [pong] Re: [ping] Re: RFR 8146015: JMXInterfaceBindingTest is failing intermittently for IPv6 addresses

serguei.spitsyn@oracle.com
On 1/7/16 08:40, Daniel Fuchs wrote:
Hi,

This looks OK to me.
I'm not sure I understand the full impact of the changes
in getAddressesForLocalHost() though - so hopefully someone
else will jump in to review that part...

Hi Jaroslav,

I do not understand the full impact of the getAddressesForLocalHost() change either
so that, please, do not count me as a reviewer.

However, I have a question on the fragment:
+    private static boolean isLocalhost(InetAddress i) {
+        if (!i.isLoopbackAddress()) {
+            return i.getHostName().toLowerCase().equals("localhost");
+        }
+        return false;
     }

Should true be returned instead of false if the i.isLoopbackAddress() returns true?
Do we normally treat the loopbackAddress case as a localhost variant?

Thanks,
Serguei



best regards,

-- daniel

On 07/01/16 16:02, Jaroslav Bachorik wrote:
On 5.1.2016 15:30, Jaroslav Bachorik wrote:
On 4.1.2016 10:05, Jaroslav Bachorik wrote:
Gentle reminder ...

On 23.12.2015 11:26, Jaroslav Bachorik wrote:
Please, review the following test change

Issue : https://bugs.openjdk.java.net/browse/JDK-8146015
Webrev: http://cr.openjdk.java.net/~jbachorik/8146015/webrev.00

The test fails for IPv6 addresses since the RMI expects an IPv6 address
to be properly wrapped in '[]'. In addition to that the logic for
selecting IP addresses to bind is flawed - it does not check for IP
addresses of multiple adapters but for multiple IP addresses
assigned to
'localhost'. In combination with IPv4 & IPv6 this will cause the
test to
attempt binding to IPv4 and IPv6 address of the same adapter
simultaneously and the test will fail.

The fix adds the requested wrapping for IPv6 addresses and adjusts the
IP selection logic to iterate over distinct adapters first and prevent
IPv4 and IPv6 address of the same adapter being treated as two
addresses
(for the purposes of the test).

Thanks,

-JB-





Reply | Threaded
Open this post in threaded view
|

Re: jmx-dev [ding] Re: [pong] Re: [ping] Re: RFR 8146015: JMXInterfaceBindingTest is failing intermittently for IPv6 addresses

Jaroslav Bachorik
On 12.1.2016 11:47, [hidden email] wrote:

> On 1/7/16 08:40, Daniel Fuchs wrote:
>> Hi,
>>
>> This looks OK to me.
>> I'm not sure I understand the full impact of the changes
>> in getAddressesForLocalHost() though - so hopefully someone
>> else will jump in to review that part...
>
> Hi Jaroslav,
>
> I do not understand the full impact of the getAddressesForLocalHost()
> change either
> so that, please, do not count me as a reviewer.

Looks like

>
> However, I have a question on the fragment:
>
> + private static boolean isLocalhost(InetAddress i) {
> + if (!i.isLoopbackAddress()) {
> + return i.getHostName().toLowerCase().equals("localhost");
> + }
> + return false;
>       }
>
>
> Should true be returned instead of false if the i.isLoopbackAddress()
> returns true?
> Do we normally treat the loopbackAddress case as a localhost variant?

Ok, maybe the method name is missing a bit - the idea is to get all
'true' localhosts - a localhost defined on a real non-loopback adapter
(as it doesn't make sense to bind JMX remote connector to a loopback
address).

I just couldn't come up with more describing name not being excessively
long :( I'm open to any suggestions.

-JB-

>
> Thanks,
> Serguei
>
>
>>
>> best regards,
>>
>> -- daniel
>>
>> On 07/01/16 16:02, Jaroslav Bachorik wrote:
>>> On 5.1.2016 15:30, Jaroslav Bachorik wrote:
>>>> On 4.1.2016 10:05, Jaroslav Bachorik wrote:
>>>>> Gentle reminder ...
>>>>>
>>>>> On 23.12.2015 11:26, Jaroslav Bachorik wrote:
>>>>>> Please, review the following test change
>>>>>>
>>>>>> Issue : https://bugs.openjdk.java.net/browse/JDK-8146015
>>>>>> Webrev: http://cr.openjdk.java.net/~jbachorik/8146015/webrev.00
>>>>>>
>>>>>> The test fails for IPv6 addresses since the RMI expects an IPv6
>>>>>> address
>>>>>> to be properly wrapped in '[]'. In addition to that the logic for
>>>>>> selecting IP addresses to bind is flawed - it does not check for IP
>>>>>> addresses of multiple adapters but for multiple IP addresses
>>>>>> assigned to
>>>>>> 'localhost'. In combination with IPv4 & IPv6 this will cause the
>>>>>> test to
>>>>>> attempt binding to IPv4 and IPv6 address of the same adapter
>>>>>> simultaneously and the test will fail.
>>>>>>
>>>>>> The fix adds the requested wrapping for IPv6 addresses and adjusts
>>>>>> the
>>>>>> IP selection logic to iterate over distinct adapters first and
>>>>>> prevent
>>>>>> IPv4 and IPv6 address of the same adapter being treated as two
>>>>>> addresses
>>>>>> (for the purposes of the test).
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> -JB-
>>>>>
>>>>
>>>
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: jmx-dev [ding] Re: [pong] Re: [ping] Re: RFR 8146015: JMXInterfaceBindingTest is failing intermittently for IPv6 addresses

serguei.spitsyn@oracle.com
On 1/12/16 03:49, Jaroslav Bachorik wrote:

> On 12.1.2016 11:47, [hidden email] wrote:
>> On 1/7/16 08:40, Daniel Fuchs wrote:
>>> Hi,
>>>
>>> This looks OK to me.
>>> I'm not sure I understand the full impact of the changes
>>> in getAddressesForLocalHost() though - so hopefully someone
>>> else will jump in to review that part...
>>
>> Hi Jaroslav,
>>
>> I do not understand the full impact of the getAddressesForLocalHost()
>> change either
>> so that, please, do not count me as a reviewer.
>
> Looks like
>
>>
>> However, I have a question on the fragment:
>>
>> + private static boolean isLocalhost(InetAddress i) {
>> + if (!i.isLoopbackAddress()) {
>> + return i.getHostName().toLowerCase().equals("localhost");
>> + }
>> + return false;
>>       }
>>
>>
>> Should true be returned instead of false if the i.isLoopbackAddress()
>> returns true?
>> Do we normally treat the loopbackAddress case as a localhost variant?
>
> Ok, maybe the method name is missing a bit - the idea is to get all
> 'true' localhosts - a localhost defined on a real non-loopback adapter
> (as it doesn't make sense to bind JMX remote connector to a loopback
> address).

Got it.
Thank you for the explanation.


>
> I just couldn't come up with more describing name not being
> excessively long :( I'm open to any suggestions.

The name looks Ok to me.
I'd suggest to add a short comment with your explanation above.


Thanks,
Serguei


>
> -JB-
>
>>
>> Thanks,
>> Serguei
>>
>>
>>>
>>> best regards,
>>>
>>> -- daniel
>>>
>>> On 07/01/16 16:02, Jaroslav Bachorik wrote:
>>>> On 5.1.2016 15:30, Jaroslav Bachorik wrote:
>>>>> On 4.1.2016 10:05, Jaroslav Bachorik wrote:
>>>>>> Gentle reminder ...
>>>>>>
>>>>>> On 23.12.2015 11:26, Jaroslav Bachorik wrote:
>>>>>>> Please, review the following test change
>>>>>>>
>>>>>>> Issue : https://bugs.openjdk.java.net/browse/JDK-8146015
>>>>>>> Webrev: http://cr.openjdk.java.net/~jbachorik/8146015/webrev.00
>>>>>>>
>>>>>>> The test fails for IPv6 addresses since the RMI expects an IPv6
>>>>>>> address
>>>>>>> to be properly wrapped in '[]'. In addition to that the logic for
>>>>>>> selecting IP addresses to bind is flawed - it does not check for IP
>>>>>>> addresses of multiple adapters but for multiple IP addresses
>>>>>>> assigned to
>>>>>>> 'localhost'. In combination with IPv4 & IPv6 this will cause the
>>>>>>> test to
>>>>>>> attempt binding to IPv4 and IPv6 address of the same adapter
>>>>>>> simultaneously and the test will fail.
>>>>>>>
>>>>>>> The fix adds the requested wrapping for IPv6 addresses and adjusts
>>>>>>> the
>>>>>>> IP selection logic to iterate over distinct adapters first and
>>>>>>> prevent
>>>>>>> IPv4 and IPv6 address of the same adapter being treated as two
>>>>>>> addresses
>>>>>>> (for the purposes of the test).
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> -JB-
>>>>>>
>>>>>
>>>>
>>>
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: jmx-dev [ding] Re: [pong] Re: [ping] Re: RFR 8146015: JMXInterfaceBindingTest is failing intermittently for IPv6 addresses

Jaroslav Bachorik
On 12.1.2016 12:55, [hidden email] wrote:

> On 1/12/16 03:49, Jaroslav Bachorik wrote:
>> On 12.1.2016 11:47, [hidden email] wrote:
>>> On 1/7/16 08:40, Daniel Fuchs wrote:
>>>> Hi,
>>>>
>>>> This looks OK to me.
>>>> I'm not sure I understand the full impact of the changes
>>>> in getAddressesForLocalHost() though - so hopefully someone
>>>> else will jump in to review that part...
>>>
>>> Hi Jaroslav,
>>>
>>> I do not understand the full impact of the getAddressesForLocalHost()
>>> change either
>>> so that, please, do not count me as a reviewer.
>>
>> Looks like
>>
>>>
>>> However, I have a question on the fragment:
>>>
>>> + private static boolean isLocalhost(InetAddress i) {
>>> + if (!i.isLoopbackAddress()) {
>>> + return i.getHostName().toLowerCase().equals("localhost");
>>> + }
>>> + return false;
>>>       }
>>>
>>>
>>> Should true be returned instead of false if the i.isLoopbackAddress()
>>> returns true?
>>> Do we normally treat the loopbackAddress case as a localhost variant?
>>
>> Ok, maybe the method name is missing a bit - the idea is to get all
>> 'true' localhosts - a localhost defined on a real non-loopback adapter
>> (as it doesn't make sense to bind JMX remote connector to a loopback
>> address).
>
> Got it.
> Thank you for the explanation.
>
>
>>
>> I just couldn't come up with more describing name not being
>> excessively long :( I'm open to any suggestions.
>
> The name looks Ok to me.
> I'd suggest to add a short comment with your explanation above.

Thanks. I've added the explanation and also disambiguated the method
name a bit.

http://cr.openjdk.java.net/~jbachorik/8146015/webrev.01

It looks like no one else is going to jump in and verify the
getAddressesForLocalHost() change (basically, excluding the loopback
localhost addresses as they are not suitable for binding the remote JMX
connector to) :/ Maybe Severing could comment, as the orignal author?

I would like to integrate this change ASAP since the test is failing
100% in nightly runs.

-JB-

>
>
> Thanks,
> Serguei
>
>
>>
>> -JB-
>>
>>>
>>> Thanks,
>>> Serguei
>>>
>>>
>>>>
>>>> best regards,
>>>>
>>>> -- daniel
>>>>
>>>> On 07/01/16 16:02, Jaroslav Bachorik wrote:
>>>>> On 5.1.2016 15:30, Jaroslav Bachorik wrote:
>>>>>> On 4.1.2016 10:05, Jaroslav Bachorik wrote:
>>>>>>> Gentle reminder ...
>>>>>>>
>>>>>>> On 23.12.2015 11:26, Jaroslav Bachorik wrote:
>>>>>>>> Please, review the following test change
>>>>>>>>
>>>>>>>> Issue : https://bugs.openjdk.java.net/browse/JDK-8146015
>>>>>>>> Webrev: http://cr.openjdk.java.net/~jbachorik/8146015/webrev.00
>>>>>>>>
>>>>>>>> The test fails for IPv6 addresses since the RMI expects an IPv6
>>>>>>>> address
>>>>>>>> to be properly wrapped in '[]'. In addition to that the logic for
>>>>>>>> selecting IP addresses to bind is flawed - it does not check for IP
>>>>>>>> addresses of multiple adapters but for multiple IP addresses
>>>>>>>> assigned to
>>>>>>>> 'localhost'. In combination with IPv4 & IPv6 this will cause the
>>>>>>>> test to
>>>>>>>> attempt binding to IPv4 and IPv6 address of the same adapter
>>>>>>>> simultaneously and the test will fail.
>>>>>>>>
>>>>>>>> The fix adds the requested wrapping for IPv6 addresses and adjusts
>>>>>>>> the
>>>>>>>> IP selection logic to iterate over distinct adapters first and
>>>>>>>> prevent
>>>>>>>> IPv4 and IPv6 address of the same adapter being treated as two
>>>>>>>> addresses
>>>>>>>> (for the purposes of the test).
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>>
>>>>>>>> -JB-
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: jmx-dev [ding] Re: [pong] Re: [ping] Re: RFR 8146015: JMXInterfaceBindingTest is failing intermittently for IPv6 addresses

Daniel Fuchs
On 13/01/16 12:16, Jaroslav Bachorik wrote:

> Thanks. I've added the explanation and also disambiguated the method
> name a bit.
>
> http://cr.openjdk.java.net/~jbachorik/8146015/webrev.01
>
> It looks like no one else is going to jump in and verify the
> getAddressesForLocalHost() change (basically, excluding the loopback
> localhost addresses as they are not suitable for binding the remote JMX
> connector to) :/ Maybe Severing could comment, as the orignal author?
>
> I would like to integrate this change ASAP since the test is failing
> 100% in nightly runs.
>

Looks to me Jaroslav :-)

-- daniel
Reply | Threaded
Open this post in threaded view
|

Re: jmx-dev [ding] Re: [pong] Re: [ping] Re: RFR 8146015: JMXInterfaceBindingTest is failing intermittently for IPv6 addresses

Jaroslav Bachorik
On 13.1.2016 13:00, Daniel Fuchs wrote:

> On 13/01/16 12:16, Jaroslav Bachorik wrote:
>> Thanks. I've added the explanation and also disambiguated the method
>> name a bit.
>>
>> http://cr.openjdk.java.net/~jbachorik/8146015/webrev.01
>>
>> It looks like no one else is going to jump in and verify the
>> getAddressesForLocalHost() change (basically, excluding the loopback
>> localhost addresses as they are not suitable for binding the remote JMX
>> connector to) :/ Maybe Severing could comment, as the orignal author?
>>
>> I would like to integrate this change ASAP since the test is failing
>> 100% in nightly runs.
>>
>
> Looks to me Jaroslav :-)

Thanks :) It's nice that this kind of change is named after me ;)

-JB-

>
> -- daniel

Reply | Threaded
Open this post in threaded view
|

Re: jmx-dev [ding] Re: [pong] Re: [ping] Re: RFR 8146015: JMXInterfaceBindingTest is failing intermittently for IPv6 addresses

serguei.spitsyn@oracle.com
In reply to this post by Jaroslav Bachorik
On 1/13/16 03:16, Jaroslav Bachorik wrote:

> On 12.1.2016 12:55, [hidden email] wrote:
>> On 1/12/16 03:49, Jaroslav Bachorik wrote:
>>> On 12.1.2016 11:47, [hidden email] wrote:
>>>> On 1/7/16 08:40, Daniel Fuchs wrote:
>>>>> Hi,
>>>>>
>>>>> This looks OK to me.
>>>>> I'm not sure I understand the full impact of the changes
>>>>> in getAddressesForLocalHost() though - so hopefully someone
>>>>> else will jump in to review that part...
>>>>
>>>> Hi Jaroslav,
>>>>
>>>> I do not understand the full impact of the getAddressesForLocalHost()
>>>> change either
>>>> so that, please, do not count me as a reviewer.
>>>
>>> Looks like
>>>
>>>>
>>>> However, I have a question on the fragment:
>>>>
>>>> + private static boolean isLocalhost(InetAddress i) {
>>>> + if (!i.isLoopbackAddress()) {
>>>> + return i.getHostName().toLowerCase().equals("localhost");
>>>> + }
>>>> + return false;
>>>>       }
>>>>
>>>>
>>>> Should true be returned instead of false if the i.isLoopbackAddress()
>>>> returns true?
>>>> Do we normally treat the loopbackAddress case as a localhost variant?
>>>
>>> Ok, maybe the method name is missing a bit - the idea is to get all
>>> 'true' localhosts - a localhost defined on a real non-loopback adapter
>>> (as it doesn't make sense to bind JMX remote connector to a loopback
>>> address).
>>
>> Got it.
>> Thank you for the explanation.
>>
>>
>>>
>>> I just couldn't come up with more describing name not being
>>> excessively long :( I'm open to any suggestions.
>>
>> The name looks Ok to me.
>> I'd suggest to add a short comment with your explanation above.
>
> Thanks. I've added the explanation and also disambiguated the method
> name a bit.
>
> http://cr.openjdk.java.net/~jbachorik/8146015/webrev.01
>
> It looks like no one else is going to jump in and verify the
> getAddressesForLocalHost() change (basically, excluding the loopback
> localhost addresses as they are not suitable for binding the remote
> JMX connector to) :/ Maybe Severing could comment, as the orignal author?
>
> I would like to integrate this change ASAP since the test is failing
> 100% in nightly runs.

Ok. I looked  to the getAddressesForLocalHost() part and think this fix
is fine.
Thumbs up from me.

Thanks,
Serguei

>
> -JB-
>
>>
>>
>> Thanks,
>> Serguei
>>
>>
>>>
>>> -JB-
>>>
>>>>
>>>> Thanks,
>>>> Serguei
>>>>
>>>>
>>>>>
>>>>> best regards,
>>>>>
>>>>> -- daniel
>>>>>
>>>>> On 07/01/16 16:02, Jaroslav Bachorik wrote:
>>>>>> On 5.1.2016 15:30, Jaroslav Bachorik wrote:
>>>>>>> On 4.1.2016 10:05, Jaroslav Bachorik wrote:
>>>>>>>> Gentle reminder ...
>>>>>>>>
>>>>>>>> On 23.12.2015 11:26, Jaroslav Bachorik wrote:
>>>>>>>>> Please, review the following test change
>>>>>>>>>
>>>>>>>>> Issue : https://bugs.openjdk.java.net/browse/JDK-8146015
>>>>>>>>> Webrev: http://cr.openjdk.java.net/~jbachorik/8146015/webrev.00
>>>>>>>>>
>>>>>>>>> The test fails for IPv6 addresses since the RMI expects an IPv6
>>>>>>>>> address
>>>>>>>>> to be properly wrapped in '[]'. In addition to that the logic for
>>>>>>>>> selecting IP addresses to bind is flawed - it does not check
>>>>>>>>> for IP
>>>>>>>>> addresses of multiple adapters but for multiple IP addresses
>>>>>>>>> assigned to
>>>>>>>>> 'localhost'. In combination with IPv4 & IPv6 this will cause the
>>>>>>>>> test to
>>>>>>>>> attempt binding to IPv4 and IPv6 address of the same adapter
>>>>>>>>> simultaneously and the test will fail.
>>>>>>>>>
>>>>>>>>> The fix adds the requested wrapping for IPv6 addresses and
>>>>>>>>> adjusts
>>>>>>>>> the
>>>>>>>>> IP selection logic to iterate over distinct adapters first and
>>>>>>>>> prevent
>>>>>>>>> IPv4 and IPv6 address of the same adapter being treated as two
>>>>>>>>> addresses
>>>>>>>>> (for the purposes of the test).
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>>
>>>>>>>>> -JB-
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: jmx-dev [ding] Re: [pong] Re: [ping] Re: RFR 8146015: JMXInterfaceBindingTest is failing intermittently for IPv6 addresses

Severin Gehwolf
Hi,

On Wed, 2016-01-13 at 10:51 -0800, [hidden email] wrote:

> On 1/13/16 03:16, Jaroslav Bachorik wrote:
> > On 12.1.2016 12:55, [hidden email] wrote:
> > > On 1/12/16 03:49, Jaroslav Bachorik wrote:
> > > > On 12.1.2016 11:47, [hidden email] wrote:
> > > > > On 1/7/16 08:40, Daniel Fuchs wrote:
> > > > > > Hi,
> > > > > >
> > > > > > This looks OK to me.
> > > > > > I'm not sure I understand the full impact of the changes
> > > > > > in getAddressesForLocalHost() though - so hopefully someone
> > > > > > else will jump in to review that part...
> > > > >
> > > > > Hi Jaroslav,
> > > > >
> > > > > I do not understand the full impact of the getAddressesForLocalHost()
> > > > > change either
> > > > > so that, please, do not count me as a reviewer.
> > > >
> > > > Looks like
> > > >
> > > > >
> > > > > However, I have a question on the fragment:
> > > > >
> > > > > + private static boolean isLocalhost(InetAddress i) {
> > > > > + if (!i.isLoopbackAddress()) {
> > > > > + return i.getHostName().toLowerCase().equals("localhost");
> > > > > + }
> > > > > + return false;
> > > > >       }
> > > > >
> > > > >
> > > > > Should true be returned instead of false if the i.isLoopbackAddress()
> > > > > returns true?
> > > > > Do we normally treat the loopbackAddress case as a localhost variant?
> > > >
> > > > Ok, maybe the method name is missing a bit - the idea is to get all
> > > > 'true' localhosts - a localhost defined on a real non-loopback adapter
> > > > (as it doesn't make sense to bind JMX remote connector to a loopback
> > > > address).
> > >
> > > Got it.
> > > Thank you for the explanation.
> > >
> > >
> > > >
> > > > I just couldn't come up with more describing name not being
> > > > excessively long :( I'm open to any suggestions.
> > >
> > > The name looks Ok to me.
> > > I'd suggest to add a short comment with your explanation above.
> >
> > Thanks. I've added the explanation and also disambiguated the method 
> > name a bit.
> >
> > http://cr.openjdk.java.net/~jbachorik/8146015/webrev.01
> >
> > It looks like no one else is going to jump in and verify the 
> > getAddressesForLocalHost() change (basically, excluding the loopback 
> > localhost addresses as they are not suitable for binding the remote 
> > JMX connector to) :/ Maybe Severing could comment, as the orignal author?

Sorry for being late in the game with this. 

+    private static List<InetAddress> getAddressesForLocalHost() {
+
         try {
-            addrs = InetAddress.getAllByName("localhost");
-        } catch (UnknownHostException e) {
+            return NetworkInterface.networkInterfaces()
+                .flatMap(NetworkInterface::inetAddresses)
+                .filter(JMXInterfaceBindingTest::isNonloopbackLocalhost)
+                .collect(Collectors.toList());

I wonder if this does what you claim it does. It looks like it's
getting all the addresses per adapter (IPv4 or IPv6) and then filters
addresses out which have "localhost" as name and aren't loopback. It's
not really what you said it does:

""""
The fix adds the requested wrapping for IPv6 addresses and adjusts the 
IP selection logic to iterate over distinct adapters first and prevent 
IPv4 and IPv6 address of the same adapter being treated as two
addresses
""""

How is it preventing IPv4 and IPv6 addresses *both* being used for one
adapter? Could it be that this fix works because loopback was the only
one with IPv4 and IPv6 address config on the test server?

Say one has the following /etc/hosts config:
192.168.1.14              localhost
fe80::56ee:75ff:fe35:d1d4 localhost

with the following adapter info (from ifconfig):

enp0s25: flags=4163<UP,BROADCAST,RUNNING,MULTICAST>  mtu 1500
        inet 192.168.1.14  netmask 255.255.255.0  broadcast 192.168.1.255
        inet6 fe80::56ee:75ff:fe35:d1d4

I.e. for adapter enp0s25 I specify the IP addresses 192.168.1.14 and
fe80::56ee:75ff:fe35:d1d4 to be "localhost". I'd expect the test to
still fail after this patch.

+    // we need 'real' localhost addresses only (eg. not loopback ones)
+    // so we can bind the remote JMX connector to them
+    private static boolean isNonloopbackLocalhost(InetAddress i) {
+        if (!i.isLoopbackAddress()) {
+            return i.getHostName().toLowerCase().equals("localhost");
+        }
+        return false;

TBH, I don't understand why this comment is there. Particularly, the
loopback exclusion. Yet, this may be platform specific. It worked for
me under linux. It was fine with binding to IPv4 loopback (127.0.0.1)
and some interface address (e.g. 192.168.1.x). Looking at the bug it
seems the main issue was a conflict of binding to IPv4 loopback + IPv6
loopback.

So the gist would be to not use both IPv4 *and* IPv6 addresses of the
same adapter for the binding. How about using something like this for a
filter:
private static boolean isIpV4Address(InetAddress i) {
   return i instanceof Inet4Address;
}

Thoughts?

Anyway, looks like it's too late now :-/

Cheers,
Severin

> > I would like to integrate this change ASAP since the test is failing 
> > 100% in nightly runs.
>
> Ok. I looked  to the getAddressesForLocalHost() part and think this fix 
> is fine.
> Thumbs up from me.
>
> Thanks,
> Serguei
>
> >
> > -JB-
> >
> > >
> > >
> > > Thanks,
> > > Serguei
> > >
> > >
> > > >
> > > > -JB-
> > > >
> > > > >
> > > > > Thanks,
> > > > > Serguei
> > > > >
> > > > >
> > > > > >
> > > > > > best regards,
> > > > > >
> > > > > > -- daniel
> > > > > >
> > > > > > On 07/01/16 16:02, Jaroslav Bachorik wrote:
> > > > > > > On 5.1.2016 15:30, Jaroslav Bachorik wrote:
> > > > > > > > On 4.1.2016 10:05, Jaroslav Bachorik wrote:
> > > > > > > > > Gentle reminder ...
> > > > > > > > >
> > > > > > > > > On 23.12.2015 11:26, Jaroslav Bachorik wrote:
> > > > > > > > > > Please, review the following test change
> > > > > > > > > >
> > > > > > > > > > Issue : https://bugs.openjdk.java.net/browse/JDK-8146015
> > > > > > > > > > Webrev: http://cr.openjdk.java.net/~jbachorik/8146015/webrev.00
> > > > > > > > > >
> > > > > > > > > > The test fails for IPv6 addresses since the RMI expects an IPv6
> > > > > > > > > > address
> > > > > > > > > > to be properly wrapped in '[]'. In addition to that the logic for
> > > > > > > > > > selecting IP addresses to bind is flawed - it does not check 
> > > > > > > > > > for IP
> > > > > > > > > > addresses of multiple adapters but for multiple IP addresses
> > > > > > > > > > assigned to
> > > > > > > > > > 'localhost'. In combination with IPv4 & IPv6 this will cause the
> > > > > > > > > > test to
> > > > > > > > > > attempt binding to IPv4 and IPv6 address of the same adapter
> > > > > > > > > > simultaneously and the test will fail.
> > > > > > > > > >
> > > > > > > > > > The fix adds the requested wrapping for IPv6 addresses and 
> > > > > > > > > > adjusts
> > > > > > > > > > the
> > > > > > > > > > IP selection logic to iterate over distinct adapters first and
> > > > > > > > > > prevent
> > > > > > > > > > IPv4 and IPv6 address of the same adapter being treated as two
> > > > > > > > > > addresses
> > > > > > > > > > (for the purposes of the test).
> > > > > > > > > >
> > > > > > > > > > Thanks,
> > > > > > > > > >
> > > > > > > > > > -JB-
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>

Reply | Threaded
Open this post in threaded view
|

Re: jmx-dev [ding] Re: [pong] Re: [ping] Re: RFR 8146015: JMXInterfaceBindingTest is failing intermittently for IPv6 addresses

Jaroslav Bachorik
On 14.1.2016 17:23, Severin Gehwolf wrote:

> Hi,
>
> On Wed, 2016-01-13 at 10:51 -0800, [hidden email] wrote:
> > On 1/13/16 03:16, Jaroslav Bachorik wrote:
> >> On 12.1.2016 12:55, [hidden email] wrote:
> >>> On 1/12/16 03:49, Jaroslav Bachorik wrote:
> >>>> On 12.1.2016 11:47, [hidden email] wrote:
> >>>>> On 1/7/16 08:40, Daniel Fuchs wrote:
> >>>>>> Hi,
> >>>>>>
> >>>>>> This looks OK to me.
> >>>>>> I'm not sure I understand the full impact of the changes
> >>>>>> in getAddressesForLocalHost() though - so hopefully someone
> >>>>>> else will jump in to review that part...
> >>>>>
> >>>>> Hi Jaroslav,
> >>>>>
> >>>>> I do not understand the full impact of the getAddressesForLocalHost()
> >>>>> change either
> >>>>> so that, please, do not count me as a reviewer.
> >>>>
> >>>> Looks like
> >>>>
> >>>>>
> >>>>> However, I have a question on the fragment:
> >>>>>
> >>>>> + private static boolean isLocalhost(InetAddress i) {
> >>>>> + if (!i.isLoopbackAddress()) {
> >>>>> + return i.getHostName().toLowerCase().equals("localhost");
> >>>>> + }
> >>>>> + return false;
> >>>>>        }
> >>>>>
> >>>>>
> >>>>> Should true be returned instead of false if the i.isLoopbackAddress()
> >>>>> returns true?
> >>>>> Do we normally treat the loopbackAddress case as a localhost variant?
> >>>>
> >>>> Ok, maybe the method name is missing a bit - the idea is to get all
> >>>> 'true' localhosts - a localhost defined on a real non-loopback adapter
> >>>> (as it doesn't make sense to bind JMX remote connector to a loopback
> >>>> address).
> >>>
> >>> Got it.
> >>> Thank you for the explanation.
> >>>
> >>>
> >>>>
> >>>> I just couldn't come up with more describing name not being
> >>>> excessively long :( I'm open to any suggestions.
> >>>
> >>> The name looks Ok to me.
> >>> I'd suggest to add a short comment with your explanation above.
> >>
> >> Thanks. I've added the explanation and also disambiguated the method
> >> name a bit.
> >>
> >> http://cr.openjdk.java.net/~jbachorik/8146015/webrev.01
> >>
> >> It looks like no one else is going to jump in and verify the
> >> getAddressesForLocalHost() change (basically, excluding the loopback
> >> localhost addresses as they are not suitable for binding the remote
> >> JMX connector to) :/ Maybe Severing could comment, as the orignal author?
>
> Sorry for being late in the game with this.
>
> +    private static List<InetAddress> getAddressesForLocalHost() {
> +
>           try {
> -            addrs = InetAddress.getAllByName("localhost");
> -        } catch (UnknownHostException e) {
> +            return NetworkInterface.networkInterfaces()
> +                .flatMap(NetworkInterface::inetAddresses)
> +                .filter(JMXInterfaceBindingTest::isNonloopbackLocalhost)
> +                .collect(Collectors.toList());
>
> I wonder if this does what you claim it does. It looks like it's
> getting all the addresses per adapter (IPv4 or IPv6) and then filters
> addresses out which have "localhost" as name and aren't loopback. It's
> not really what you said it does:
>
> """"
> The fix adds the requested wrapping for IPv6 addresses and adjusts the
> IP selection logic to iterate over distinct adapters first and prevent
> IPv4 and IPv6 address of the same adapter being treated as two
> addresses
> """"
>
> How is it preventing IPv4 and IPv6 addresses *both* being used for one
> adapter? Could it be that this fix works because loopback was the only
> one with IPv4 and IPv6 address config on the test server?
>
> Say one has the following /etc/hosts config:
> 192.168.1.14              localhost
> fe80::56ee:75ff:fe35:d1d4 localhost
>
> with the following adapter info (from ifconfig):
>
> enp0s25: flags=4163<UP,BROADCAST,RUNNING,MULTICAST>  mtu 1500
>          inet 192.168.1.14  netmask 255.255.255.0  broadcast 192.168.1.255
>          inet6 fe80::56ee:75ff:fe35:d1d4
>
> I.e. for adapter enp0s25 I specify the IP addresses 192.168.1.14 and
> fe80::56ee:75ff:fe35:d1d4 to be "localhost". I'd expect the test to
> still fail after this patch.
>
> +    // we need 'real' localhost addresses only (eg. not loopback ones)
> +    // so we can bind the remote JMX connector to them
> +    private static boolean isNonloopbackLocalhost(InetAddress i) {
> +        if (!i.isLoopbackAddress()) {
> +            return i.getHostName().toLowerCase().equals("localhost");
> +        }
> +        return false;
>
> TBH, I don't understand why this comment is there. Particularly, the
> loopback exclusion. Yet, this may be platform specific. It worked for
> me under linux. It was fine with binding to IPv4 loopback (127.0.0.1)
> and some interface address (e.g. 192.168.1.x). Looking at the bug it
> seems the main issue was a conflict of binding to IPv4 loopback + IPv6
> loopback.
You can have problems also with a configuration with several loopbacks. The test considers such a setup as a machine with multiple interfaces while it is, in fact, only one interface. Also, it does not really make any sense to test binding to a loopback address - we are starting a remote JMX connector and loopback address is not accessible from outside, anyway.

>
> So the gist would be to not use both IPv4 *and* IPv6 addresses of the
> same adapter for the binding. How about using something like this for a
> filter:
> private static boolean isIpV4Address(InetAddress i) {
>     return i instanceof Inet4Address;
> }
>
> Thoughts?
>
> Anyway, looks like it's too late now :-/

Yep. But feel free to open a new issue, with the reproducer description, and, hopefully a proposed patch :) I will sponsor the push happily.

-JB-

>
> Cheers,
> Severin
>
> >> I would like to integrate this change ASAP since the test is failing
> >> 100% in nightly runs.
> >
> > Ok. I looked  to the getAddressesForLocalHost() part and think this fix
> > is fine.
> > Thumbs up from me.
> >
> > Thanks,
> > Serguei
> >
> >>
> >> -JB-
> >>
> >>>
> >>>
> >>> Thanks,
> >>> Serguei
> >>>
> >>>
> >>>>
> >>>> -JB-
> >>>>
> >>>>>
> >>>>> Thanks,
> >>>>> Serguei
> >>>>>
> >>>>>
> >>>>>>
> >>>>>> best regards,
> >>>>>>
> >>>>>> -- daniel
> >>>>>>
> >>>>>> On 07/01/16 16:02, Jaroslav Bachorik wrote:
> >>>>>>> On 5.1.2016 15:30, Jaroslav Bachorik wrote:
> >>>>>>>> On 4.1.2016 10:05, Jaroslav Bachorik wrote:
> >>>>>>>>> Gentle reminder ...
> >>>>>>>>>
> >>>>>>>>> On 23.12.2015 11:26, Jaroslav Bachorik wrote:
> >>>>>>>>>> Please, review the following test change
> >>>>>>>>>>
> >>>>>>>>>> Issue : https://bugs.openjdk.java.net/browse/JDK-8146015
> >>>>>>>>>> Webrev: http://cr.openjdk.java.net/~jbachorik/8146015/webrev.00
> >>>>>>>>>>
> >>>>>>>>>> The test fails for IPv6 addresses since the RMI expects an IPv6
> >>>>>>>>>> address
> >>>>>>>>>> to be properly wrapped in '[]'. In addition to that the logic for
> >>>>>>>>>> selecting IP addresses to bind is flawed - it does not check
> >>>>>>>>>> for IP
> >>>>>>>>>> addresses of multiple adapters but for multiple IP addresses
> >>>>>>>>>> assigned to
> >>>>>>>>>> 'localhost'. In combination with IPv4 & IPv6 this will cause the
> >>>>>>>>>> test to
> >>>>>>>>>> attempt binding to IPv4 and IPv6 address of the same adapter
> >>>>>>>>>> simultaneously and the test will fail.
> >>>>>>>>>>
> >>>>>>>>>> The fix adds the requested wrapping for IPv6 addresses and
> >>>>>>>>>> adjusts
> >>>>>>>>>> the
> >>>>>>>>>> IP selection logic to iterate over distinct adapters first and
> >>>>>>>>>> prevent
> >>>>>>>>>> IPv4 and IPv6 address of the same adapter being treated as two
> >>>>>>>>>> addresses
> >>>>>>>>>> (for the purposes of the test).
> >>>>>>>>>>
> >>>>>>>>>> Thanks,
> >>>>>>>>>>
> >>>>>>>>>> -JB-
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>>
> >>>>>
> >>>>
> >>>
> >>
> >
>

Reply | Threaded
Open this post in threaded view
|

Re: jmx-dev [ding] Re: [pong] Re: [ping] Re: RFR 8146015: JMXInterfaceBindingTest is failing intermittently for IPv6 addresses

Severin Gehwolf
Hi,

On Fri, 2016-01-15 at 09:50 +0100, Jaroslav Bachorik wrote:

> > Sorry for being late in the game with this.
> >
> > +    private static List getAddressesForLocalHost() {
> > +
> >           try {
> > -            addrs = InetAddress.getAllByName("localhost");
> > -        } catch (UnknownHostException e) {
> > +            return NetworkInterface.networkInterfaces()
> > +                .flatMap(NetworkInterface::inetAddresses)
> > +                .filter(JMXInterfaceBindingTest::isNonloopbackLocalhost)
> > +                .collect(Collectors.toList());
> >
> > I wonder if this does what you claim it does. It looks like it's
> > getting all the addresses per adapter (IPv4 or IPv6) and then filters
> > addresses out which have "localhost" as name and aren't loopback. It's
> > not really what you said it does:
> >
> > """"
> > The fix adds the requested wrapping for IPv6 addresses and adjusts the
> > IP selection logic to iterate over distinct adapters first and prevent
> > IPv4 and IPv6 address of the same adapter being treated as two
> > addresses
> > """"
> >
> > How is it preventing IPv4 and IPv6 addresses *both* being used for one
> > adapter? Could it be that this fix works because loopback was the only
> > one with IPv4 and IPv6 address config on the test server?
> >
> > Say one has the following /etc/hosts config:
> > 192.168.1.14              localhost
> > fe80::56ee:75ff:fe35:d1d4 localhost
> >
> > with the following adapter info (from ifconfig):
> >
> > enp0s25: flags=4163  mtu 1500
> >          inet 192.168.1.14  netmask 255.255.255.0  broadcast 192.168.1.255
> >          inet6 fe80::56ee:75ff:fe35:d1d4
> >
> > I.e. for adapter enp0s25 I specify the IP addresses 192.168.1.14 and
> > fe80::56ee:75ff:fe35:d1d4 to be "localhost". I'd expect the test to
> > still fail after this patch.
> >
> > +    // we need 'real' localhost addresses only (eg. not loopback ones)
> > +    // so we can bind the remote JMX connector to them
> > +    private static boolean isNonloopbackLocalhost(InetAddress i) {
> > +        if (!i.isLoopbackAddress()) {
> > +            return i.getHostName().toLowerCase().equals("localhost");
> > +        }
> > +        return false;
> >
> > TBH, I don't understand why this comment is there. Particularly, the
> > loopback exclusion. Yet, this may be platform specific. It worked for
> > me under linux. It was fine with binding to IPv4 loopback (127.0.0.1)
> > and some interface address (e.g. 192.168.1.x). Looking at the bug it
> > seems the main issue was a conflict of binding to IPv4 loopback + IPv6
> > loopback.

> You can have problems also with a configuration with several
> loopbacks. The test considers such a setup as a machine with multiple
> interfaces while it is, in fact, only one interface.

Right. For the purpose of the test it would still be acceptable to bind
to loopback, no? One of the reasons why I'd think it would be useful is
that it wouldn't require the test to be run on a machine with > 1
adapters/virtual adapters.

> Also, it does not really make any sense to test binding to a loopback
> address - we are starting a remote JMX connector and loopback address
> is not accessible from outside, anyway.

Conceptually yes, but due to lack of configure options (AFAIK) for the
local JMX agent it might still be useful.

> >
> > So the gist would be to not use both IPv4 *and* IPv6 addresses of the
> > same adapter for the binding. How about using something like this for a
> > filter:
> > private static boolean isIpV4Address(InetAddress i) {
> >     return i instanceof Inet4Address;
> > }
> >
> > Thoughts?
> >
> > Anyway, looks like it's too late now :-/
>
> Yep. But feel free to open a new issue, with the reproducer
> description, and, hopefully a proposed patch :)

Sure, not a problem. I'll see what I can do.

> I will sponsor the push happily.

Thank you!

Cheers,
Severin
Reply | Threaded
Open this post in threaded view
|

Re: jmx-dev [ding] Re: [pong] Re: [ping] Re: RFR 8146015: JMXInterfaceBindingTest is failing intermittently for IPv6 addresses

Jaroslav Bachorik
On 15.1.2016 10:24, Severin Gehwolf wrote:

> Hi,
>
> On Fri, 2016-01-15 at 09:50 +0100, Jaroslav Bachorik wrote:
>>> Sorry for being late in the game with this.
>>>
>>> +    private static List getAddressesForLocalHost() {
>>> +
>>>            try {
>>> -            addrs = InetAddress.getAllByName("localhost");
>>> -        } catch (UnknownHostException e) {
>>> +            return NetworkInterface.networkInterfaces()
>>> +                .flatMap(NetworkInterface::inetAddresses)
>>> +                .filter(JMXInterfaceBindingTest::isNonloopbackLocalhost)
>>> +                .collect(Collectors.toList());
>>>
>>> I wonder if this does what you claim it does. It looks like it's
>>> getting all the addresses per adapter (IPv4 or IPv6) and then filters
>>> addresses out which have "localhost" as name and aren't loopback. It's
>>> not really what you said it does:
>>>
>>> """"
>>> The fix adds the requested wrapping for IPv6 addresses and adjusts the
>>> IP selection logic to iterate over distinct adapters first and prevent
>>> IPv4 and IPv6 address of the same adapter being treated as two
>>> addresses
>>> """"
>>>
>>> How is it preventing IPv4 and IPv6 addresses *both* being used for one
>>> adapter? Could it be that this fix works because loopback was the only
>>> one with IPv4 and IPv6 address config on the test server?
>>>
>>> Say one has the following /etc/hosts config:
>>> 192.168.1.14              localhost
>>> fe80::56ee:75ff:fe35:d1d4 localhost
>>>
>>> with the following adapter info (from ifconfig):
>>>
>>> enp0s25: flags=4163  mtu 1500
>>>           inet 192.168.1.14  netmask 255.255.255.0  broadcast 192.168.1.255
>>>           inet6 fe80::56ee:75ff:fe35:d1d4
>>>
>>> I.e. for adapter enp0s25 I specify the IP addresses 192.168.1.14 and
>>> fe80::56ee:75ff:fe35:d1d4 to be "localhost". I'd expect the test to
>>> still fail after this patch.
>>>
>>> +    // we need 'real' localhost addresses only (eg. not loopback ones)
>>> +    // so we can bind the remote JMX connector to them
>>> +    private static boolean isNonloopbackLocalhost(InetAddress i) {
>>> +        if (!i.isLoopbackAddress()) {
>>> +            return i.getHostName().toLowerCase().equals("localhost");
>>> +        }
>>> +        return false;
>>>
>>> TBH, I don't understand why this comment is there. Particularly, the
>>> loopback exclusion. Yet, this may be platform specific. It worked for
>>> me under linux. It was fine with binding to IPv4 loopback (127.0.0.1)
>>> and some interface address (e.g. 192.168.1.x). Looking at the bug it
>>> seems the main issue was a conflict of binding to IPv4 loopback + IPv6
>>> loopback.
>
>> You can have problems also with a configuration with several
>> loopbacks. The test considers such a setup as a machine with multiple
>> interfaces while it is, in fact, only one interface.
>
> Right. For the purpose of the test it would still be acceptable to bind
> to loopback, no? One of the reasons why I'd think it would be useful is
> that it wouldn't require the test to be run on a machine with > 1
> adapters/virtual adapters.

You should test this config first. If I remember correctly I had some
problems using multiple loopback addresses.

-JB-

>
>> Also, it does not really make any sense to test binding to a loopback
>> address - we are starting a remote JMX connector and loopback address
>> is not accessible from outside, anyway.
>
> Conceptually yes, but due to lack of configure options (AFAIK) for the
> local JMX agent it might still be useful.
>
>>>
>>> So the gist would be to not use both IPv4 *and* IPv6 addresses of the
>>> same adapter for the binding. How about using something like this for a
>>> filter:
>>> private static boolean isIpV4Address(InetAddress i) {
>>>      return i instanceof Inet4Address;
>>> }
>>>
>>> Thoughts?
>>>
>>> Anyway, looks like it's too late now :-/
>>
>> Yep. But feel free to open a new issue, with the reproducer
>> description, and, hopefully a proposed patch :)
>
> Sure, not a problem. I'll see what I can do.
>
>> I will sponsor the push happily.
>
> Thank you!
>
> Cheers,
> Severin
>

Reply | Threaded
Open this post in threaded view
|

Re: jmx-dev [ding] Re: [pong] Re: [ping] Re: RFR 8146015: JMXInterfaceBindingTest is failing intermittently for IPv6 addresses

Severin Gehwolf
On Fri, 2016-01-15 at 10:28 +0100, Jaroslav Bachorik wrote:

> On 15.1.2016 10:24, Severin Gehwolf wrote:
> > Hi,
> >
> > On Fri, 2016-01-15 at 09:50 +0100, Jaroslav Bachorik wrote:
> > > > Sorry for being late in the game with this.
> > > >
> > > > +    private static List getAddressesForLocalHost() {
> > > > +
> > > >            try {
> > > > -            addrs = InetAddress.getAllByName("localhost");
> > > > -        } catch (UnknownHostException e) {
> > > > +            return NetworkInterface.networkInterfaces()
> > > > +                .flatMap(NetworkInterface::inetAddresses)
> > > > +                .filter(JMXInterfaceBindingTest::isNonloopbackLocalhost)
> > > > +                .collect(Collectors.toList());
> > > >
> > > > I wonder if this does what you claim it does. It looks like it's
> > > > getting all the addresses per adapter (IPv4 or IPv6) and then filters
> > > > addresses out which have "localhost" as name and aren't loopback. It's
> > > > not really what you said it does:
> > > >
> > > > """"
> > > > The fix adds the requested wrapping for IPv6 addresses and adjusts the
> > > > IP selection logic to iterate over distinct adapters first and prevent
> > > > IPv4 and IPv6 address of the same adapter being treated as two
> > > > addresses
> > > > """"
> > > >
> > > > How is it preventing IPv4 and IPv6 addresses *both* being used for one
> > > > adapter? Could it be that this fix works because loopback was the only
> > > > one with IPv4 and IPv6 address config on the test server?
> > > >
> > > > Say one has the following /etc/hosts config:
> > > > 192.168.1.14              localhost
> > > > fe80::56ee:75ff:fe35:d1d4 localhost
> > > >
> > > > with the following adapter info (from ifconfig):
> > > >
> > > > enp0s25: flags=4163  mtu 1500
> > > >           inet 192.168.1.14  netmask 255.255.255.0  broadcast 192.168.1.255
> > > >           inet6 fe80::56ee:75ff:fe35:d1d4
> > > >
> > > > I.e. for adapter enp0s25 I specify the IP addresses 192.168.1.14 and
> > > > fe80::56ee:75ff:fe35:d1d4 to be "localhost". I'd expect the test to
> > > > still fail after this patch.
> > > >
> > > > +    // we need 'real' localhost addresses only (eg. not loopback ones)
> > > > +    // so we can bind the remote JMX connector to them
> > > > +    private static boolean isNonloopbackLocalhost(InetAddress i) {
> > > > +        if (!i.isLoopbackAddress()) {
> > > > +            return i.getHostName().toLowerCase().equals("localhost");
> > > > +        }
> > > > +        return false;
> > > >
> > > > TBH, I don't understand why this comment is there. Particularly, the
> > > > loopback exclusion. Yet, this may be platform specific. It worked for
> > > > me under linux. It was fine with binding to IPv4 loopback (127.0.0.1)
> > > > and some interface address (e.g. 192.168.1.x). Looking at the bug it
> > > > seems the main issue was a conflict of binding to IPv4 loopback + IPv6
> > > > loopback.
> >
> > > You can have problems also with a configuration with several
> > > loopbacks. The test considers such a setup as a machine with multiple
> > > interfaces while it is, in fact, only one interface.
> >
> > Right. For the purpose of the test it would still be acceptable to bind
> > to loopback, no? One of the reasons why I'd think it would be useful is
> > that it wouldn't require the test to be run on a machine with > 1
> > adapters/virtual adapters.
>
> You should test this config first. If I remember correctly I had some 
> problems using multiple loopback addresses.

OK. Thanks for the heads-up.

Cheers,
Severin