Welcome! Log In Create A New Profile

Advanced

[PHP-DEV] net_get_interfaces()

Posted by Sara Golemon 
Sara Golemon
[PHP-DEV] net_get_interfaces()
November 23, 2017 11:50PM
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 [email protected] 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
Anatol Belski
RE: [PHP-DEV] net_get_interfaces()
November 24, 2017 11:20AM
> -----Original Message-----
> From: php@golemon.com [mailto:[email protected]] On Behalf Of Sara
> Golemon
> Sent: Thursday, November 23, 2017 11:46 PM
> To: PHP internals <[email protected]>
> 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
> [email protected] 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
Florian Anderiasch
Re: [PHP-DEV] net_get_interfaces()
November 26, 2017 05:00PM
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 [email protected] 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

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Sara Golemon
Re: [PHP-DEV] net_get_interfaces()
November 26, 2017 05:20PM
On Sun, Nov 26, 2017 at 10:53 AM, Florian Anderiasch <[email protected]> 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

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Florian Anderiasch
Re: [PHP-DEV] net_get_interfaces()
November 26, 2017 05:30PM
On 26.11.2017 17:10, Sara Golemon wrote:
> On Sun, Nov 26, 2017 at 10:53 AM, Florian Anderiasch <[email protected]> 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

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Dan Ackroyd
Re: [PHP-DEV] net_get_interfaces()
November 27, 2017 12:20PM
On 26 November 2017 at 16:28, Florian Anderiasch <[email protected]> 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

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Florian Anderiasch
Re: [PHP-DEV] net_get_interfaces()
November 27, 2017 04:40PM
On 27.11.2017 12:15, Dan Ackroyd wrote:
> On 26 November 2017 at 16:28, Florian Anderiasch <[email protected]> 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

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Sara Golemon
Re: [PHP-DEV] net_get_interfaces()
November 27, 2017 04:50PM
On Mon, Nov 27, 2017 at 10:34 AM, Florian Anderiasch <[email protected]> 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

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Florian Anderiasch
Re: [PHP-DEV] net_get_interfaces()
November 27, 2017 05:50PM
On 27.11.2017 16:38, Sara Golemon wrote:
> On Mon, Nov 27, 2017 at 10:34 AM, Florian Anderiasch <[email protected]> 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

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Sara Golemon
Re: [PHP-DEV] net_get_interfaces()
November 27, 2017 06:00PM
On Mon, Nov 27, 2017 at 11:38 AM, Florian Anderiasch <[email protected]> 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
<sys/net/if.h>` ?

> Can do a proper bug report if needed, but maybe I get around to test
> NetBSD as well.
>
Thanks!

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Matteo Beccati
Re: [PHP-DEV] net_get_interfaces()
November 27, 2017 06:30PM
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/

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Niklas Keller
Re: [PHP-DEV] net_get_interfaces()
November 27, 2017 06:40PM
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 <[email protected]>:

> 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 [email protected] 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
>
>
Florian Anderiasch
Re: [PHP-DEV] net_get_interfaces()
November 27, 2017 06:40PM
On 27.11.2017 17:56, Sara Golemon wrote:
> On Mon, Nov 27, 2017 at 11:38 AM, Florian Anderiasch <[email protected]> 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
> <sys/net/if.h>` ?
>
>> 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

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Sara Golemon
Re: [PHP-DEV] net_get_interfaces()
November 27, 2017 07:00PM
On Mon, Nov 27, 2017 at 12:23 PM, Matteo Beccati <[email protected]> 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

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Sorry, only registered users may post in this forum.

Click here to login