net_get_interfaces()

  101149
November 23, 2017 22:46 pollita@php.net (Sara Golemon)
Planning to add net_get_interfaces()
https://github.com/php/php-src/pull/2935/files for enumerating the
adapters on a system and their configured addresses.
Based on a combination of my own implementation and finding out
krakjoe and ab@ had a stalled version already.

If anyone wants an RFC I'll write one up, but this is pretty small so
mostly just offering for a look before I push.

-Sara

Ref: https://bugs.php.net/bug.php?id=17400
Ref: https://github.com/php/php-src/compare/master...weltling:17400
  101151
November 24, 2017 10:15 ab@php.net (Anatol Belski)
> -----Original Message----- > From: php@golemon.com [mailto:php@golemon.com] On Behalf Of Sara > Golemon > Sent: Thursday, November 23, 2017 11:46 PM > To: PHP internals <internals@lists.php.net> > Subject: [PHP-DEV] net_get_interfaces() > > Planning to add net_get_interfaces() > https://github.com/php/php-src/pull/2935/files for enumerating the adapters > on a system and their configured addresses. > Based on a combination of my own implementation and finding out krakjoe and > ab@ had a stalled version already. > > If anyone wants an RFC I'll write one up, but this is pretty small so mostly just > offering for a look before I push. > There's no such functionality in the core till now, whereby it was asked some years ago. IMHO it would make a little sense with the RFC, until perhaps some would doubt such a functionality makes sense.
Thanks for moving forward on this, Sara 😊 Regards anatol
  101153
November 26, 2017 15:53 ml@anderiasch.de (Florian Anderiasch)
On 23.11.2017 23:46, Sara Golemon wrote:
> Planning to add net_get_interfaces() > https://github.com/php/php-src/pull/2935/files for enumerating the > adapters on a system and their configured addresses. > Based on a combination of my own implementation and finding out > krakjoe and ab@ had a stalled version already. > > If anyone wants an RFC I'll write one up, but this is pretty small so > mostly just offering for a look before I push. > > -Sara > > Ref: https://bugs.php.net/bug.php?id=17400 > Ref: https://github.com/php/php-src/compare/master...weltling:17400 >
Looks good in theory, but without a lot of thought, how likely is this to break/work on "supported" operating systems? (Which ones are that, actually? http://php.net/manual/en/install.unix.php lists the BSDs and Solaris and HP/UX) I know, it explicitly mentions Windows and Linux - also probably someone tried it on OSX, and I don't think (Free|Open|Net)BSD will be a big problem here, but as I'm no expert on that - does that matter? Will it need "just a little work" or could the more exotic ones be more problematic? Greetings, Florian
  101154
November 26, 2017 16:10 pollita@php.net (Sara Golemon)
On Sun, Nov 26, 2017 at 10:53 AM, Florian Anderiasch <ml@anderiasch.de> wrote:
> Looks good in theory, but without a lot of thought, how likely is this > to break/work on "supported" operating systems? (Which ones are that, > actually? http://php.net/manual/en/install.unix.php lists the BSDs and > Solaris and HP/UX) > > I know, it explicitly mentions Windows and Linux - also probably someone > tried it on OSX, and I don't think (Free|Open|Net)BSD will be a big > problem here, but as I'm no expert on that - does that matter? Will it > need "just a little work" or could the more exotic ones be more problematic? > Without a comprehensive CI matrix to run diffs like this against,
there's no way to be absolutely certain it'll work everywhere. That said, the config.m4 changes specifically test for the new APIs being used. If they fail to compile in a standalone environment, the new function isn't enabled for compilation in the main build. So, at worst, we may find some OSs (AIX, Solaris, etc...) simply don't gain the new functionality, but it shouldn't {knock on wood} cause any builds to break. If it does, we have an entire year for interested parties to catch it. -Sara
  101155
November 26, 2017 16:28 ml@anderiasch.de (Florian Anderiasch)
On 26.11.2017 17:10, Sara Golemon wrote:
> On Sun, Nov 26, 2017 at 10:53 AM, Florian Anderiasch <ml@anderiasch.de> wrote: >> Looks good in theory, but without a lot of thought, how likely is this >> to break/work on "supported" operating systems? (Which ones are that, >> actually? http://php.net/manual/en/install.unix.php lists the BSDs and >> Solaris and HP/UX) >> >> I know, it explicitly mentions Windows and Linux - also probably someone >> tried it on OSX, and I don't think (Free|Open|Net)BSD will be a big >> problem here, but as I'm no expert on that - does that matter? Will it >> need "just a little work" or could the more exotic ones be more problematic? >> > Without a comprehensive CI matrix to run diffs like this against, > there's no way to be absolutely certain it'll work everywhere. > > That said, the config.m4 changes specifically test for the new APIs > being used. If they fail to compile in a standalone environment, the > new function isn't enabled for compilation in the main build. So, at > worst, we may find some OSs (AIX, Solaris, etc...) simply don't gain > the new functionality, but it shouldn't {knock on wood} cause any > builds to break. If it does, we have an entire year for interested > parties to catch it.
Yeah, I know, and I'd agree on the "absolutely certain" part - but as this is not an extension, I have somewhat of an uneasy feeling to even merge this without some at least rudimentary testing on "all" OSes. I am confident it would probably work on most and can be fixed on the others - but I see that as a TODO during the PR phase. Speaking from a user PoV - if this simply doesn't work on some OSes for no good reason.. that's a pretty leaky abstraction. If it's in the PHP core, I'd assume it will work. Also I wouldn't count on it getting done, just because there's one year of time and "we have no CI for that" is not a good excuse in my book. Don't get me wrong, this sounds like a useful feature - but (and my wild guessing could be inaccurate) I think it's low-level enough that the differences might matter. Greetings, Florian
  101159
November 27, 2017 11:15 danack@basereality.com (Dan Ackroyd)
On 26 November 2017 at 16:28, Florian Anderiasch <ml@anderiasch.de> wrote:

> if this simply doesn't work on some OSes for > no good reason.. that's a pretty leaky abstraction.
Cool. Does that mean that we can drop support for 32bit builds if leaky abstractions are an important thing?
> Looks good in theory, but without a lot of thought, how likely is this > to break/work on "supported" operating systems?
It's likely to work on platforms that have the C functions implemented. Which is the level of guarantee that PHP aims for; "If the platform implements a feature, PHP will expose it, but won't always go out of its way to work around missing features".
> just because there's one year of time and "we have no CI for that" is > not a good excuse in my book.
If this is important to you, please step up and do something about it. If it is only of just enough importance to you that you send an email to this list, asking other people to do a shedload of work to satisfy your vague feelings, then as Sara said, committing it now gives people a whole year to find issues with it, and then we can take action against known issues, rather than worrying about unknown unknowns. cheers Dan Ack
  101161
November 27, 2017 15:34 ml@anderiasch.de (Florian Anderiasch)
On 27.11.2017 12:15, Dan Ackroyd wrote:
> On 26 November 2017 at 16:28, Florian Anderiasch <ml@anderiasch.de> wrote: > >> if this simply doesn't work on some OSes for >> no good reason.. that's a pretty leaky abstraction. > > Cool. Does that mean that we can drop support for 32bit builds if > leaky abstractions are an important thing?
If someone commits something that only works on 64bit and wasn't tested a single time on a 32 bit machine - yes.
>> Looks good in theory, but without a lot of thought, how likely is this >> to break/work on "supported" operating systems? > > It's likely to work on platforms that have the C functions > implemented. Which is the level of guarantee that PHP aims for; "If > the platform implements a feature, PHP will expose it, but won't > always go out of its way to work around missing features".
Yes, I wrote that it's likely. Maybe I just didn't make myself clear enough that I see a *slight* difference between something that's relatively nicely encapsulated in the language itself versus something that not only depends on some C headers, but maybe the whole network stack of the OS.
>> just because there's one year of time and "we have no CI for that" is >> not a good excuse in my book. > > If this is important to you, please step up and do something about it. > > If it is only of just enough importance to you that you send an email > to this list, asking other people to do a shedload of work to satisfy > your vague feelings, then as Sara said, committing it now gives people > a whole year to find issues with it, and then we can take action > against known issues, rather than worrying about unknown unknowns.
I didn't ask anybody to to any work. I saw this as a call for comments, and Sara kind of addressed my *concerns* where I said I *think* this *might* break. Also I'm really sorry that I disappointed you by not managing to finish testing on OpenBSD (which I actually started just before seeing this mail) and I already sent some results for FreeBSD yesterday, which showed a tiny issue. Insert friendly suggestion to not throw stones when lingering near breakable structures. Greetings, Florian
  101162
November 27, 2017 15:38 pollita@php.net (Sara Golemon)
On Mon, Nov 27, 2017 at 10:34 AM, Florian Anderiasch <ml@anderiasch.de> wrote:
> Also I'm really sorry that I disappointed you by not managing to finish > testing on OpenBSD (which I actually started just before seeing this > mail) and I already sent some results for FreeBSD yesterday, which > showed a tiny issue. > Yes! I saw that! Thank you. It wasn't so much a BSD issue as the fact
you did a bare build, while my default build happens to include the sockets extension (which provides the AF_INET constant I was using in the unit test). Thanks to your testing, I was able to refactor the test slightly to not require ext/sockets be enabled in order to work. :D -Sara
  101163
November 27, 2017 16:38 ml@anderiasch.de (Florian Anderiasch)
On 27.11.2017 16:38, Sara Golemon wrote:
> On Mon, Nov 27, 2017 at 10:34 AM, Florian Anderiasch <ml@anderiasch.de> wrote: >> Also I'm really sorry that I disappointed you by not managing to finish >> testing on OpenBSD (which I actually started just before seeing this >> mail) and I already sent some results for FreeBSD yesterday, which >> showed a tiny issue. >> > Yes! I saw that! Thank you. It wasn't so much a BSD issue as the fact > you did a bare build, while my default build happens to include the > sockets extension (which provides the AF_INET constant I was using in > the unit test). Thanks to your testing, I was able to refactor the > test slightly to not require ext/sockets be enabled in order to work. > :D > > -Sara >
No worries, nothing in that was directed at you, Sara. (But not sure if having to enable sockets proves my point or not, I guess it did provide at least some value). Good news: master after the merge works on OpenBSD, once the build is fixed. Somehow it doesn't match the +#if HAVE_NET_IF_H in net.c I had to unconditionally include it (for testing), otherwise IFF_BROADCAST and IFF_POINTOPOINT are undefined. (they are in sys/net/if.h on OpenBSD, but I only tested 6.2). Can do a proper bug report if needed, but maybe I get around to test NetBSD as well. Greetings, Florian
  101164
November 27, 2017 16:56 pollita@php.net (Sara Golemon)
On Mon, Nov 27, 2017 at 11:38 AM, Florian Anderiasch <ml@anderiasch.de> wrote:
> Somehow it doesn't match the > +#if HAVE_NET_IF_H > in net.c > > I had to unconditionally include it (for testing), otherwise > IFF_BROADCAST and IFF_POINTOPOINT are undefined. (they are in > sys/net/if.h on OpenBSD, but I only tested 6.2). > Can you send or gist your config.log? (Optionally redacted to the
net/if.h check if you want to do that yourself) Are you saying you simply removed the `#if HAVE_NET_IF_H` check? Or that you did that *AND* changed the include directive to `#include ` ?
> Can do a proper bug report if needed, but maybe I get around to test > NetBSD as well. > Thanks!
  101165
November 27, 2017 17:23 php@beccati.com (Matteo Beccati)
Hi everyone,

On 27/11/2017 17:56, Sara Golemon wrote:
>> Can do a proper bug report if needed, but maybe I get around to test >> NetBSD as well. >> > Thanks!
I've just tested a NetBSD 7.0 i386 machine and I'm getting a few errors: ext/standard/net.c: In function ‘php_inet_ntop’: ext/standard/net.c:82:41: error: ‘NI_MAXHOST’ undeclared (first use in this function) zend_string *ret = zend_string_alloc(NI_MAXHOST, 0); ^ ext/standard/net.c:82:41: note: each undeclared identifier is reported only once for each function it appears in ext/standard/net.c:83:4: warning: implicit declaration of function ‘getnameinfo’ [-Wimplicit-function-declaration] if (getnameinfo(addr, addrlen, ZSTR_VAL(ret), NI_MAXHOST, NULL, 0, NI_NUMERICHOST) == SUCCESS) { ^ ext/standard/net.c:83:71: error: ‘NI_NUMERICHOST’ undeclared (first use in this function) if (getnameinfo(addr, addrlen, ZSTR_VAL(ret), NI_MAXHOST, NULL, 0, NI_NUMERICHOST) == SUCCESS) { ^ ext/standard/net.c: At top level: ext/standard/net.c:98:13: warning: ‘iface_append_unicast’ defined but not used [-Wunused-function] static void iface_append_unicast(zval *unicast, zend_long flags, ^ *** Error code 1 I'll see if I can get them fixed in the next few days. Cheers -- Matteo Beccati Development & Consulting - http://www.beccati.com/
  101168
November 27, 2017 17:51 pollita@php.net (Sara Golemon)
On Mon, Nov 27, 2017 at 12:23 PM, Matteo Beccati <php@beccati.com> wrote:
> I've just tested a NetBSD 7.0 i386 machine and I'm getting a few errors: > > ext/standard/net.c: At top level: > ext/standard/net.c:98:13: warning: ‘iface_append_unicast’ defined but > not used [-Wunused-function] > static void iface_append_unicast(zval *unicast, zend_long flags, > ^ > *** Error code 1 > Interesting, that tells me that getifaddrs() isn't available on your
system. I've pushed a fix for the include and the unused method, but I'll have to spin up a NetBSD VM to figure out a way to support the functionality on that platform. Thanks for testing! -Sara
  101167
November 27, 2017 17:30 ml@anderiasch.de (Florian Anderiasch)
On 27.11.2017 17:56, Sara Golemon wrote:
> On Mon, Nov 27, 2017 at 11:38 AM, Florian Anderiasch <ml@anderiasch.de> wrote: >> Somehow it doesn't match the >> +#if HAVE_NET_IF_H >> in net.c >> >> I had to unconditionally include it (for testing), otherwise >> IFF_BROADCAST and IFF_POINTOPOINT are undefined. (they are in >> sys/net/if.h on OpenBSD, but I only tested 6.2). >> > Can you send or gist your config.log? (Optionally redacted to the > net/if.h check if you want to do that yourself) > > Are you saying you simply removed the `#if HAVE_NET_IF_H` check? Or > that you did that *AND* changed the include directive to `#include > ` ? > >> Can do a proper bug report if needed, but maybe I get around to test >> NetBSD as well. >> > Thanks! >
Filed as https://bugs.php.net/bug.php?id=75582 - let's move the discussion over there, sorry for the noise. Greetings, Florian
  101166
November 27, 2017 17:28 me@kelunik.com (Niklas Keller)
Hey Sara,

I'm not sure about the naming, but I think such a function is a good idea.
Why a `net_*` prefix when it's in ext/standard? I'd suggest
`get_network_interfaces()` instead.

As previously mentioned on Twitter [1], it could be really useful for us in
amphp/socket [2].

[1] https://twitter.com/kelunik/status/934075249452306433
[2] https://github.com/amphp/socket/issues/35

Regards, Niklas

2017-11-23 23:46 GMT+01:00 Sara Golemon <pollita@php.net>:

> Planning to add net_get_interfaces() > https://github.com/php/php-src/pull/2935/files for enumerating the > adapters on a system and their configured addresses. > Based on a combination of my own implementation and finding out > krakjoe and ab@ had a stalled version already. > > If anyone wants an RFC I'll write one up, but this is pretty small so > mostly just offering for a look before I push. > > -Sara > > Ref: https://bugs.php.net/bug.php?id=17400 > Ref: https://github.com/php/php-src/compare/master...weltling:17400 > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php > >