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
>