[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: jmx-dev [ding] Re: [pong] Re: [ping] Re: RFR 8146015: JMXInterfaceBindingTest is failing intermi
From:       Severin Gehwolf <sgehwolf () redhat ! com>
Date:       2016-01-15 9:30:50
Message-ID: 1452850250.3564.13.camel () redhat ! com
[Download RAW message or body]

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
[prev in list] [next in list] [prev in thread] [next in thread]