'Re: RFR: JDK-8057556: JDP should better handle non-active interfaces'

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

List:       openjdk-serviceability-dev
Subject:    Re: RFR: JDK-8057556: JDP should better handle non-active interfaces
From:       Dmitry Samersoff <dmitry.samersoff () oracle ! com>
Date:       2014-09-10 6:46:10
Message-ID: 540FF3B2.2030908 () oracle ! com
[Download RAW message or body]

Looks good for me.


On 2014-09-10 07:36, Yasumasa Suenaga wrote:
> Hi Dmitry,
> 
> Okay, I've uploaded new webrev for JDK-8057556:
> http://cr.openjdk.java.net/~ysuenaga/JDK-8057556/webrev.2/
> 
> This change fixes a comment from Jarosalv as below:
>>>>> L103 Please, move "else" to the previous line
> 
> Could you review it again?
> 
> 
> Thanks,
> 
> Yasumasa
> 
> 
> (2014/09/10 6:40), Dmitry Samersoff wrote:
>> Yasumasa,
>>
>> To allow multiple application to use the same *multicast* address:port
>> you probably just need to move
>>
>>   channel.setOption(StandardSocketOptions.SO_REUSEADDR, true);
>>
>> before bind.
>>
>>
>> As for bind call lets leave this question out of scope of these two
>> fixes. Different OS'es behave differently and I need to check it before
>> we can go further. I'll come back later and file a separate CR if
>> necessary.
>>
>> -Dmitry
>>
>> On 2014-09-09 07:45, Yasumasa Suenaga wrote:
>>>> Bind call is required to listen on particular address. So please, keep
>>>> it.
>>>
>>> I think JdpBroadcaster need not to call bind().
>>> bind() binds address to application, so another application on same host
>>> cannot use JDP.
>>>
>>> I've changed JdpBroadcaster as below:
>>> -----------
>>> diff -r 68a6bb51cb26
>>> src/java.management/share/classes/sun/management/jdp/JdpBroadcaster.java
>>> ---
>>> a/src/java.management/share/classes/sun/management/jdp/JdpBroadcaster.java
>>>
>>> Mon Sep 01 13:33:28 2014 +0200
>>> +++
>>> b/src/java.management/share/classes/sun/management/jdp/JdpBroadcaster.java
>>>
>>> Tue Sep 09 12:27:33 2014 +0900
>>> @@ -79,11 +79,7 @@
>>>           if (srcAddress != null) {
>>>               // User requests particular interface to bind to
>>>               NetworkInterface interf =
>>> NetworkInterface.getByInetAddress(srcAddress);
>>> -            try {
>>> -                channel.bind(new InetSocketAddress(srcAddress, 0));
>>> -            } catch (UnsupportedAddressTypeException ex) {
>>> -                throw new JdpException("Unable to bind to source
>>> address");
>>> -            }
>>> +            channel.setOption(StandardSocketOptions.SO_REUSEADDR,
>>> true);
>>>               channel.setOption(StandardSocketOptions.IP_MULTICAST_IF,
>>> interf);
>>>           }
>>>       }
>>> -----------
>>>
>>> I ran two JVMs as below:
>>> -----------
>>> java -Dcom.sun.management.jmxremote.port=<port>
>>> -Dcom.sun.management.jmxremote.authenticate=false
>>>       -Dcom.sun.management.jmxremote.ssl=false
>>> -Dcom.sun.management.jmxremote.autodiscovery=true
>>>       -Dcom.sun.management.jdp.source_addr=<IP> <Long sleep application>
>>> -----------
>>>
>>> Both JVM instances could send JDP packet.
>>> I checked it with tcpdump and my JDP receiver application.
>>>
>>>
>>> If you keep bind() call, JDP broadcaster only exists one instance
>>> on same host.
>>> I think JDP should be used by multiple JVM instance on same host.
>>>
>>>
>>> Thanks,
>>>
>>> Yasumasa
>>>
>>>
>>> (2014/09/09 0:48), Dmitry Samersoff wrote:
>>>> Jaroslav,
>>>>
>>>>> L96-100 Do we still need these lines? Isn't
>>>>> `channel.setOption(StandardSocketOptions.IP_MULTICAST_IF, interf);`
>>>>> enough to listen on the interface?
>>>>
>>>> Bind call is required to listen on particular address. So please, keep
>>>> it.
>>>>
>>>> -Dmitry
>>>>
>>>>
>>>> On 2014-09-08 14:12, Jaroslav Bachorik wrote:
>>>>> Hi Yasamusa,
>>>>>
>>>>> On 09/05/2014 12:28 PM, Yasumasa Suenaga wrote:
>>>>>> Hi Peter,
>>>>>>
>>>>>> I fixed it and created new webrev.
>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8057556/webrev.1/
>>>>>>
>>>>>> Could you review it again?
>>>>>
>>>>> Just a few nits ...
>>>>>
>>>>> L103 Please, move "else" to the previous line
>>>>> L96-100 Do we still need these lines? Isn't
>>>>> `channel.setOption(StandardSocketOptions.IP_MULTICAST_IF, interf);`
>>>>> enough to listen on the interface?
>>>>>
>>>>> -JB-
>>>>>
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Yasumasa
>>>>>>
>>>>>>
>>>>>> (2014/09/05 17:20), Peter Allwin wrote:
>>>>>>> Looks like only the first Interface will be considered if no
>>>>>>> srcAddress is provided (succeeded will be false and we will throw to
>>>>>>> exit the while loop). Is this intended?
>>>>>>>
>>>>>>> Thanks!
>>>>>>> /peter
>>>>>>>
>>>>>>>> On 4 sep 2014, at 17:59, Yasumasa Suenaga <yasuenag@gmail.com>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>> Hi all,
>>>>>>>>
>>>>>>>> Thank you so much, Dmitry!
>>>>>>>>
>>>>>>>> I've created webrev for it.
>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8057556/webrev.0/
>>>>>>>>
>>>>>>>> Please review.
>>>>>>>>
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>>
>>>>>>>> Yasumasa
>>>>>>>>
>>>>>>>>
>>>>>>>> (2014/09/04 21:26), Dmitry Samersoff wrote:
>>>>>>>>> Yasumasa,
>>>>>>>>>
>>>>>>>>> The CR number is JDK-8057556
>>>>>>>>>
>>>>>>>>> I'll care about it's integration.
>>>>>>>>>
>>>>>>>>> -Dmitry
>>>>>>>>>
>>>>>>>>>> On 2014-09-02 18:52, Yasumasa Suenaga wrote:
>>>>>>>>>> Hi all,
>>>>>>>>>>
>>>>>>>>>> I'm trying to use JDP on my Fedora20 machine.
>>>>>>>>>> My machine has two NICs and only one NIC is up.
>>>>>>>>>>
>>>>>>>>>> I passed system properties as below, however JDP broadcaster
>>>>>>>>>> thread was not started:
>>>>>>>>>>
>>>>>>>>>>      -Dcom.sun.management.jmxremote.port=7091
>>>>>>>>>>      -Dcom.sun.management.jmxremote.authenticate=false
>>>>>>>>>>      -Dcom.sun.management.jmxremote.ssl=false
>>>>>>>>>>      -Dcom.sun.management.jmxremote.autodiscovery=true
>>>>>>>>>>      -Dcom.sun.management.jdp.name=TEST
>>>>>>>>>>
>>>>>>>>>> I checked exceptions with jdb, SocketException was occurred in
>>>>>>>>>> JDPControllerRunner#run(), and it was caused by another NIC
>>>>>>>>>> is down.
>>>>>>>>>>
>>>>>>>>>> Currently, DiagramChannel which is used in JDPBroadcaster
>>>>>>>>>> tries to send JDP packet through all "UP" NICs.
>>>>>>>>>> However, NIC which is controlled by NetworkManager seems to
>>>>>>>>>> be still "UP" when ifdown command is executed.
>>>>>>>>>> (It seems to be removed IP address from NIC only.)
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> This problem may be Fedora, however I think it should be
>>>>>>>>>> improved in JDK.
>>>>>>>>>> I've created a patch as below, and it works fine in my
>>>>>>>>>> environment.
>>>>>>>>>> (jdk9/dev/jdk)
>>>>>>>>>>
>>>>>>>>>> If this patch may be accepted, I will file this to JBS.
>>>>>>>>>>
>>>>>>>>>> --------------------
>>>>>>>>>> diff -r 68a6bb51cb26
>>>>>>>>>> src/java.management/share/classes/sun/management/jdp/JdpBroadcaster.java
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> ---
>>>>>>>>>> a/src/java.management/share/classes/sun/management/jdp/JdpBroadcaster.java
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Mon Sep 01 13:33:28 2014 +0200
>>>>>>>>>> +++что случилось с go contacts
>>>>>>>>>> b/src/java.management/share/classes/sun/management/jdp/JdpBroadcaster.java
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Tue Sep 02 23:25:50 2014 +0900
>>>>>>>>>> @@ -35,6 +35,7 @@
>>>>>>>>>>     import java.nio.ByteBuffer;
>>>>>>>>>>     import java.nio.channels.DatagramChannel;
>>>>>>>>>>     import java.nio.channels.UnsupportedAddressTypeException;
>>>>>>>>>> +import java.util.Enumeration;
>>>>>>>>>>
>>>>>>>>>>     /**
>>>>>>>>>>      * JdpBroadcaster is responsible for sending pre-built JDP
>>>>>>>>>> packet
>>>>>>>>>> across a Net
>>>>>>>>>> @@ -79,6 +80,15 @@
>>>>>>>>>>             if (srcAddress != null) {
>>>>>>>>>>                 // User requests particular interface to bind to
>>>>>>>>>>                 NetworkInterface interf =
>>>>>>>>>> NetworkInterface.getByInetAddress(srcAddress);
>>>>>>>>>> +
>>>>>>>>>> +            if (interf == null) {
>>>>>>>>>> +                throw new JdpException("Unable to get network
>>>>>>>>>> interface for " + srcAddress.toString());
>>>>>>>>>> +            }
>>>>>>>>>> +
>>>>>>>>>> +            if (!interf.isUp() || !interf.supportsMulticast()) {
>>>>>>>>>> +                throw new JdpException(interf.getName() + " does
>>>>>>>>>> not support multicast.");
>>>>>>>>>> +            }
>>>>>>>>>> +
>>>>>>>>>>                 try {
>>>>>>>>>>                     channel.bind(new
>>>>>>>>>> InetSocketAddress(srcAddress, 0));
>>>>>>>>>>                 } catch (UnsupportedAddressTypeException ex) {
>>>>>>>>>> @@ -86,6 +96,23 @@
>>>>>>>>>>                 }
>>>>>>>>>>
>>>>>>>>>> channel.setOption(StandardSocketOptions.IP_MULTICAST_IF, interf);
>>>>>>>>>>             }
>>>>>>>>>> +        else {
>>>>>>>>>> +            Enumeration<NetworkInterface> nics =
>>>>>>>>>> NetworkInterface.getNetworkInterfaces();
>>>>>>>>>> +            while (nics.hasMoreElements()) {
>>>>>>>>>> +                NetworkInterface nic = nics.nextElement();
>>>>>>>>>> +
>>>>>>>>>> +                if (nic.isUp() && nic.supportsMulticast()) {
>>>>>>>>>> +                    try {
>>>>>>>>>> +
>>>>>>>>>> channel.setOption(StandardSocketOptions.IP_MULTICAST_IF, nic);
>>>>>>>>>> +                    } catch (IOException ex) {
>>>>>>>>>> +                        System.err.println("WARNING: JDP
>>>>>>>>>> broadcaster cannot use " + nic.getName() + ": " +
>>>>>>>>>> ex.getMessage());
>>>>>>>>>> +                    }
>>>>>>>>>> +                }
>>>>>>>>>> +
>>>>>>>>>> +            }
>>>>>>>>>> +
>>>>>>>>>> +        }
>>>>>>>>>> +
>>>>>>>>>>         }
>>>>>>>>>>
>>>>>>>>>>         /**
>>>>>>>>>> --------------------
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>>
>>>>>>>>>> Yasumasa
>>>>>>>>>
>>>>>>>>>
>>>>>
>>>>
>>>>
>>
>>


-- 
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* I would love to change the world, but they won't give me the sources.
[prev in list] [next in list] [prev in thread] [next in thread]
Configure | About | News | Add a list | Sponsored by KoreLogic