PING: RFR (S) JDK-8160174: java.net.NetworkInterface - fixes and improvements for network interface listing

[Chris Hegarty]

Hi Christoph,

On 19/07/16 14:48, Langer, Christoph wrote:
> Hi all,
>
> can I please get a review for my change: http://cr.openjdk.java.net/~clanger/webrevs/8160174.3/ ?

This really hard to review, since so many parts of the code
are moving around, but we did put this off a few times
already so probably makes sense to do it now. The code
should be more readable afterwards are as such easier to
maintain.

I ran it through our internal build and test system and there
were no surprises.  Consider it reviewed.

-Chris.

> I made some minor updates in place, mostly formatting, to the version from one week ago. The only real change I made is that I now set the scope id of IPv6 addresses to the interface index also on BSD, where it was not done before. Here I have the question if this is really the desired behavior to always set the interface as scope of any IPv6 address?
>
> Thanks in advance
> Christoph
>
>
>> -----Original Message-----
>> From: Langer, Christoph
>> Sent: Mittwoch, 13. Juli 2016 17:02
>> To: 'Chris Hegarty' <chris.hegarty at oracle.com>
>> Cc: net-dev at openjdk.java.net
>> Subject: RE: RFR (S) JDK-8160174: java.net.NetworkInterface - fixes and
>> improvements for network interface listing
>>
>> Hi Chris,
>>
>> ok, here is my new version which I think is quite a nice cleanup - though
>> probably not "S" any more:
>>
>> Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8160174.3/
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8160174
>>
>> I updated the bug report to list all the small things that I've fixed. I now took the
>> approach to depuzzle all "enum*Interfaces" functions for the platforms and
>> now the coding should really be easier to read - though, of course, some parts
>> got duplicated.
>>
>> The notable differences in the output of NetworkInterface.getAll() are that a)
>> no broadcast address is returned any more for loopback addresses on Linux and
>> b) subnet prefixes for AIX IPv6 interfaces should work now. The rest should be
>> optimizations under the cover.
>>
>> Please review.
>>
>> Thanks
>> Christoph
>>
>>> -----Original Message-----
>>> From: Chris Hegarty [mailto:chris.hegarty at oracle.com]
>>> Sent: Dienstag, 12. Juli 2016 16:10
>>> To: Langer, Christoph <christoph.langer at sap.com>
>>> Cc: net-dev at openjdk.java.net
>>> Subject: Re: RFR (S) JDK-8160174: java.net.NetworkInterface - fixes and
>>> improvements for network interface listing
>>>
>>> Christoph,
>>>
>>>> On 11 Jul 2016, at 14:36, Langer, Christoph <christoph.langer at sap.com>
>>> wrote:
>>>>
>>>> Hi Chris (or anyone who is holding a stake in the NetworkInterface native
>>> implementation),
>>>>
>>>> I’ve spent some time on cleaning up NetworkInterface.c and came up with a
>>> bigger change:http://cr.openjdk.java.net/~clanger/webrevs/8160174.2/
>>>>
>>>> With this I attempted to consolidate the interface listing functionality of the
>> 2
>>> main categories – one is ioctl (used on Linux IPv4, Solaris and AIX) and the
>>> other is getifaddrs (used on MacOS). I introduced some defines to switch
>>> between the implementations. I also consolidated the functionality for the
>> ioctl
>>> based network interface listings by using an #ifdef section to distinguish
>>> between the Linux/AIX versus Solaris field and constant names.
>>>>
>>>> I’ve spent some time testing on the platforms and in principal it works. But
>> as
>>> it is a matter of taste, I’d like to ask you if you support this type of change or
>>> have any hints or recommendations before going forward to finalize this.
>>>
>>> I’m personally don’t like this approach much. I think it adds further
>>> complexity and difficulty to this code, which already has its fair share
>>> of both.
>>>
>>>> For Linux I’m also suggesting to use the getifaddrs approach – I tested and
>>> found it working everywhere and with this we could get rid of the
>>> implementation to read /proc/net for IPv6.
>>>
>>> You should probably break this type of change out, so that it can be
>>> evaluated independently, on its own merit. If it add nothing more than
>>> clean up, may it makes sense for this to happen early in 10, where it
>>> can have a longer time to bake.
>>>
>>>> Furthermore I’m generally setting Null for the IPv6 broadcast address –
>> which
>>> I think is common sense for IPv6.
>>>
>>> Same as above.  This “smaller” changes can sometimes get lost in
>>> the noise of larger refactoring.
>>>
>>> -Chris.
>>>
>>>
>>>> Thanks in advance and best regards
>>>> Christoph
>>>>
>>>>
>>>> From: Langer, Christoph
>>>> Sent: Donnerstag, 23. Juni 2016 16:37
>>>> To: 'net-dev at openjdk.java.net' <net-dev at openjdk.java.net>
>>>> Subject: RFR (S) JDK-8160174: java.net.NetworkInterface - fixes and
>>> improvements for network interface listing
>>>>
>>>> Hi,
>>>>
>>>> can I please get a review the following change:
>>>> Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8160174.1/
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8160174
>>>>
>>>> I made further fixes and cleanups for listing Unix type network interfaces,
>>> especially on AIX and Linux. I ran builds and verified also on Solaris and Mac.
>>>>
>>>> Thanks
>>>> Christoph
>