if_nameindex/getifaddrs and dhcpcd issue

if_nameindex/getifaddrs and dhcpcd issue

[Natanael Copa]

Hi,

When testing migrating from uclibc to musl libc in a virtual machine I
ended up with no working network. I found out that dhcpcd[1] simply
exited with error and added this to the syslog:

 daemon.err dhcpcd[2000]: eth0: interface not found or invalid

This doesn't happen with uclibc (or glibc I assume) so I tried to dig
up what happens.

In this case, busybox ifup calls: 'dhcpcd eth0'.

You can also run dhcpcd in a catch-all mode where it will autoconfig
all network interfaces, includng new, hotplugged ones, but in this case
it was called with a specified interface.

So I grepped dhcpcd source for the "interface not found or invalid" and
found it in dhcpcd.c[2]


        /* Start any dev listening plugin which may want to
         * change the interface name provided by the kernel */
        if ((ctx.options & (DHCPCD_MASTER | DHCPCD_DEV)) ==
            (DHCPCD_MASTER | DHCPCD_DEV))
                dev_start(&ctx);

        ctx.ifaces = discover_interfaces(&ctx, ctx.ifc, ctx.ifv);
        for (i = 0; i < ctx.ifc; i++) {
                if (find_interface(&ctx, ctx.ifv[i]) == NULL)
                        syslog(LOG_ERR, "%s: interface not found or invalid",
                            ctx.ifv[i]);
        }

After some debugging it turns out that eth0 is not detected by
discover_interfaces, and in fact, dhcpcd will not discover any
interfaces at all. 

So I took a look at discover_interfaces, found in net.c[3].

struct if_head *
discover_interfaces(struct dhcpcd_ctx *ctx, int argc, char * const *argv)
{
	struct ifaddrs *ifaddrs, *ifa;
[SNIP]...
#elif AF_PACKET
	const struct sockaddr_ll *sll;
#endif

	if (getifaddrs(&ifaddrs) == -1)
		return NULL;
	ifs = malloc(sizeof(*ifs));
	if (ifs == NULL)
		return NULL;
	TAILQ_INIT(ifs);

kk

		/* Ensure that the interface name has settled */
		if (!dev_initialized(ctx, ifa->ifa_name))
			continue;


So, dhcpcd uses getifaddrs to detect available interfaces, if the
ifa_addr is set (non-null) it will filter out those who has not
AF_PACKET family. (why? i don't know).

At this point i got curious about the getifaddrs implementation and
wrote a small testcase[4]:
/*-----8<-------------------------------------------------*/
#include <stdio.h>
#include <ifaddrs.h>

#include <sys/types.h>

int main(void)
{
        struct ifaddrs *ifap, *ifa;
        if (getifaddrs(&ifap) == -1) {
                perror("getifaddrs");
                return 1;
        }
        for (ifa = ifap; ifa != NULL; ifa = ifa->ifa_next) {
                const char *afstr = "none";
                if (ifa->ifa_addr != NULL) {
                        switch (ifa->ifa_addr->sa_family) {
                        case AF_PACKET:
                                afstr = "AF_PACKET";
                                break;
                        case AF_INET:
                                afstr = "AF_INET";
                                break;
                        case AF_INET6:
                                afstr = "AF_INET6";
                                break;
                        default:
                                afstr = "unknown";
                        }
                }
                printf("%s: %s\n", ifa->ifa_name, afstr);
        }
        return 0;
}
/*-----8<-------------------------------------------------*/

I noticed that it will only print interfaces that has an AF_INET
address configured:
lo: AF_INET
eth0: AF_INET

Try 'modprobe dummy' to create a dummy0. It will not get listed. This
is kinda okish, since getifaddrs is about getting the configured
addresses. I checked the manpage[5]:
CONFORMING TO         top

       Not in POSIX.1-2001.  This function first appeared in BSDi and is
       present on the BSD systems, but with slightly different semantics
       documented—returning one entry per interface, not per address.  This
       means ifa_addr and other fields can actually be NULL if the interface
       has no address, and no link-level address is returned if the
       interface has an IP address assigned.  Also, the way of choosing
       either ifa_broadaddr or ifa_dstaddr differs on various systems.

NOTES         top

       The addresses returned on Linux will usually be the IPv4 and IPv6
       addresses assigned to the interface, but also one AF_PACKET address
       per interface containing lower-level details about the interface and
       its physical layer.  In this case, the ifa_data field may contain a
       pointer to a struct rtnl_link_stats, defined in <linux/if_link.h> (in
       Linux 2.4 and earlier, struct net_device_stats, defined in
       <linux/netdevice.h>), which contains various interface attributes and
       statistics.

So this is what application programmers will expect, the existance of
AF_PACKET on linux and this is what Roy Marples (dhcpcd author)
expected.

So I got curious why musl doesn't show AF_PACKET.

http://git.musl-libc.org/cgit/musl/tree/src/network/getifaddrs.c#n116

First thing it does is call if_nameindex, and now I discovered that
if_nameindex does not do what would you would think it does. I would
have expected that if_nameindex returns *all* interfaces as posix says
it does, but it actually only returns interfaces with a configured
AF_INET address.

Testcase copied from if_nameindex manpage[6]:
/*-----8<-------------------------------------------------*/
#include <net/if.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>

int
main(int argc, char *argv[])
{
    struct if_nameindex *if_ni, *i;

   if_ni = if_nameindex();
    if (if_ni == NULL) {
        perror("if_nameindex");
        exit(EXIT_FAILURE);
    }

   for (i = if_ni; ! (i->if_index == 0 && i->if_name == NULL); i++)
        printf("%u: %s\n", i->if_index, i->if_name);

   if_freenameindex(if_ni);

   exit(EXIT_SUCCESS);
}
/*-----8<-------------------------------------------------*/

You will notice that it does not print dummy0 interface (modprobe dummy
first) unless you assign it an ipv4 address. It will not list it with
only ipv6 addr assigned.

At this point I thought, that we do have a real bug in musl and
wondered what would happen with dhcpcd if we fixed the if_nameindex bug.

I think what will happen is that getifaddrs will list the interface
without any address, but with ifa_addr set to null. The dhcpcd code
looked like:

	for (ifa = ifaddrs; ifa; ifa = ifa->ifa_next) {
		if (ifa->ifa_addr != NULL) {
#ifdef AF_LINK
			if (ifa->ifa_addr->sa_family != AF_LINK)
				continue;
#elif AF_PACKET
			if (ifa->ifa_addr->sa_family != AF_PACKET)
				continue;
#endif
		}


Since ifa_addr is NULL for unconfigured interfaces, it would not go to
the AF_PACKET check and the interface will not be skipped - unless it
already has a configured addr, either ipv4 or ipv6.

So I implemented an if_nameindex which parses the /proc/net/dev and the
getifaddr testcase[4] started to show "none" unless it has an ipv4 addr.

Since I am fairly eager to switch Alpine Linux development branch to
musl, I thought this would be an acceptable (temp?) solution to move
forward: Fix what I believe is a bug in if_nameindex and dhcpcd users
will be able to reboot their machines without completely losing network.

I don't like the idea of parsing /proc/net/dev for various reasons. It
will not work without a mounted /proc, it will pull in stdio (I believe
malloc is already needed) and it is ugly. But apparently, this is the
only way to get a list of interfaces in Linux without using netlink.

In my humble opinion, netlink would be a saner way to solve this. (yeah
netlink has its own set of problems...)

I also believe that the only sane way to grab network address
information on Linux (getifaddrs) nowdays is via netlink. I find the
current way to deal with ipv6 via /proc ugly too.

I can post a patch for an if_nameindex implementation that
parses /proc/net/dev if you want, but I consider it more of a temp hack
than a real solution.

I wonder how many other applications that will break due to unexpected
getifaddrs behavior...

-nc

PS. I had similar issues with uclibc some time ago and for uclibc the
solution was getifaddrs with netlink.
https://www.mail-archive.com/uclibc@uclibc.org/msg00933.html

--
[1] http://roy.marples.name/projects/dhcpcd/index
[2] http://roy.marples.name/projects/dhcpcd/artifact/9e50f49288d544dd
[3] http://roy.marples.name/projects/dhcpcd/artifact/bab4c52c9c23d06c
[4] http://dev.alpinelinux.org/~ncopa/musl/testcase/ifaddr.c
[5] http://man7.org/linux/man-pages/man3/getifaddrs.3.html#CONFORMING_TO
[6] http://man7.org/linux/man-pages/man3/if_nameindex.3.html#EXAMPLE

Re: if_nameindex/getifaddrs and dhcpcd issue

[Justin Cormack]

On Tue, Apr 8, 2014 at 10:11 AM, Natanael Copa <ncopa@alpinelinux.org> wrote:
> (snip)

I am not sure that it is appropriate that a netlink implementation,
which is the only way to do the enumeration correctly in the potential
absense of /proc, should go into Musl. I would be more inclined to
implement a new library to do netlink stuff that provides compatible
interfaces (you could use libnetlink too). The glibc implementation is
723 lines of code, and it is probably hard to make the implementation
a lot smaller, but you could make a full netlink library in not much
more as it is complicated but uniform (I wrote a partly complete one
in 1000 lines of Lua).

However I can see no reason why dhcp on a specified interface needs to
enumerate interfaces at all, and it only needs to read ipv4 addresses,
unless it is implementing dhcp6 too, maybe it does now. Again dhcp6
needs netlink, the Musl ipv6 parts for getifaddrs already use /proc
which is definitely unreliable for early boot config in a distro in my
view.

Justin

Re: if_nameindex/getifaddrs and dhcpcd issue

[Timo Teras]

On Tue, 8 Apr 2014 11:07:47 +0100
Justin Cormack <justin@specialbusservice.com> wrote:

> On Tue, Apr 8, 2014 at 10:11 AM, Natanael Copa
> <ncopa@alpinelinux.org> wrote:
> > (snip)
> 
> I am not sure that it is appropriate that a netlink implementation,
> which is the only way to do the enumeration correctly in the potential
> absense of /proc, should go into Musl. I would be more inclined to

We are not implementing full netlink. Just the bits to do enumeration.

> implement a new library to do netlink stuff that provides compatible
> interfaces (you could use libnetlink too). The glibc implementation is
> 723 lines of code, and it is probably hard to make the implementation

The basic netlink code for enumeration can be done in ~50 lines of code
(depending on how you count). The rest of code is parsing the messages
needed.

More lines come of course from the headers that are needed to be
imported.

> a lot smaller, but you could make a full netlink library in not much
> more as it is complicated but uniform (I wrote a partly complete one
> in 1000 lines of Lua).
> 
> However I can see no reason why dhcp on a specified interface needs to
> enumerate interfaces at all, and it only needs to read ipv4 addresses,
> unless it is implementing dhcp6 too, maybe it does now. Again dhcp6
> needs netlink, the Musl ipv6 parts for getifaddrs already use /proc
> which is definitely unreliable for early boot config in a distro in my
> view.

We should not be looking at dhcpd. It's the APIs musl implements:
getifaddrs() and if_nameindex() - they are not currently exposing all
the information they should.

I'd prefer using netlink, instead of trying to parse and connect data
from various /proc files.

IMHO, if someone wants to be linux compatible today, it's easier to
implement the netlink stuff; than the /proc stuff. /proc has equally
linux specific things in it and is mainly intended to be human
readable with few exceptions. /sys would be better option as it's
inteded to be machine readable, but it's still text too. 

netlink is here to stay, there's no alternate way to do certain things.
So I'd rather use it. It's the interface kernel people intended to be
used for the thing in question.

I'm willing to write an alternative getifaddrs() and if_nameindex()
implementations using netlink. Perhaps let's see how they end up?

- Timo

Re: if_nameindex/getifaddrs and dhcpcd issue

[Szabolcs Nagy]

* Justin Cormack <justin@specialbusservice.com> [2014-04-08 11:07:47 +0100]:
> I am not sure that it is appropriate that a netlink implementation,
> which is the only way to do the enumeration correctly in the potential
> absense of /proc, should go into Musl. I would be more inclined to

one could just try all the numbers.. this "works" even at boot time :)
(and there is /sys/class/net/*/ifindex but that does not help libc)


#include <net/if.h>
#include <stdlib.h>
#include <errno.h>
#define N 256
struct if_nameindex *if_nameindex()
{
	struct if_nameindex *p = malloc(N*sizeof*p + N*IF_NAMESIZE);
	if (!p) errno = ENOBUFS;
	else {
		char (*name)[IF_NAMESIZE] = (void*)(p+N);
		int i,j;
		for (i=1,j=0; i<N; i++)
			if (if_indextoname(i, name[j])) {
				p[j].if_name = name[j];
				p[j].if_index = i;
				j++;
			}
		p[j].if_name = 0;
		p[j].if_index = 0;
	}
	return p;
}

> However I can see no reason why dhcp on a specified interface needs to
> enumerate interfaces at all, and it only needs to read ipv4 addresses,
> unless it is implementing dhcp6 too, maybe it does now. Again dhcp6
> needs netlink, the Musl ipv6 parts for getifaddrs already use /proc
> which is definitely unreliable for early boot config in a distro in my
> view.

if ipv6 things require netlink anyway then probably musl does not have
much choice..

Re: if_nameindex/getifaddrs and dhcpcd issue

[Rich Felker]

On Tue, Apr 08, 2014 at 11:07:47AM +0100, Justin Cormack wrote:
> However I can see no reason why dhcp on a specified interface needs to
> enumerate interfaces at all,

Indeed. Regardless of how we address this topic in musl, I think we
should push for dhcpcd to be fixed. udhcpcd works perfectly fine with
no scanning of interfaces; you simply tell it the interface to use,
and it uses that. My understanding is that dhcpcd can do the same, but
it still insists on scanning all interfaces and refuses to run if the
interface you requested to use is not in the list. This is bad
behavior.

The other case, where no interface is specified on the command line
and dhcpcd tries all interfaces, is buggy usage by the caller. There
are all sorts of interfaces that might exist, unconfigured, and which
might not be appropriate to send dhcpc requests on. I assume dhcpcd
has some heuristics to avoid selecting things like unconfigured
tunnel, slip, etc. interfaces but if so that's just an ugly hack. The
operation of "try all instances of a given type of resource" is just
wrong by design.

> and it only needs to read ipv4 addresses,
> unless it is implementing dhcp6 too, maybe it does now. Again dhcp6
> needs netlink, the Musl ipv6 parts for getifaddrs already use /proc
> which is definitely unreliable for early boot config in a distro in my
> view.

In what way does dhcp6 need netlink? What's made this discussion
difficult so far on IRC is assertions of that form (although not the
same one) without an explanation of why it's believed to be true, so
I'd like to keep rational discussion possible by making sure that such
claims are backed up by explanation rather than just stated as fact.

Rich

Re: if_nameindex/getifaddrs and dhcpcd issue

[Justin Cormack]

On Tue, Apr 8, 2014 at 2:42 PM, Rich Felker <dalias@aerifal.cx> wrote:
>> and it only needs to read ipv4 addresses,
>> unless it is implementing dhcp6 too, maybe it does now. Again dhcp6
>> needs netlink, the Musl ipv6 parts for getifaddrs already use /proc
>> which is definitely unreliable for early boot config in a distro in my
>> view.
>
> In what way does dhcp6 need netlink? What's made this discussion
> difficult so far on IRC is assertions of that form (although not the
> same one) without an explanation of why it's believed to be true, so
> I'd like to keep rational discussion possible by making sure that such
> claims are backed up by explanation rather than just stated as fact.

I was under the impression that the ioctl-based interface for ipv6 is
incomplete under Linux. That does not mean anything needs to be in
libc though. ISC dhcp for v6 just calls out to ip in scripts, rather
than ifconfig that it uses for v4, so it is indirectly uses netlink,
but does not require any libc support, indeed all the C code is
portable.

Justin

Re: if_nameindex/getifaddrs and dhcpcd issue

[Natanael Copa]

On Tue, 8 Apr 2014 15:25:59 +0300
Timo Teras <timo.teras@iki.fi> wrote:

> IMHO, if someone wants to be linux compatible today, it's easier to
> implement the netlink stuff; than the /proc stuff. /proc has equally
> linux specific things in it and is mainly intended to be human
> readable with few exceptions. /sys would be better option as it's
> inteded to be machine readable, but it's still text too. 

But /sys is not claimed to be stable so location can move around.
Not sure we can use it for network interfaces.

Basically we want wants in /sys/class/net but sysfs-rules says thats
kernel implementation details and might change.

https://www.kernel.org/doc/Documentation/sysfs-rules.txt

We could probably scan /sys/device for dirs named 'net' but that's not
prettier than /proc.

-nc

Re: if_nameindex/getifaddrs and dhcpcd issue

[Natanael Copa]

On Tue, 8 Apr 2014 11:07:47 +0100
Justin Cormack <justin@specialbusservice.com> wrote:

> On Tue, Apr 8, 2014 at 10:11 AM, Natanael Copa <ncopa@alpinelinux.org> wrote:
> > (snip)
> 
> I am not sure that it is appropriate that a netlink implementation,
> which is the only way to do the enumeration correctly in the potential
> absense of /proc, should go into Musl. I would be more inclined to
> implement a new library to do netlink stuff that provides compatible
> interfaces (you could use libnetlink too). The glibc implementation is
> 723 lines of code, and it is probably hard to make the implementation
> a lot smaller, but you could make a full netlink library in not much
> more as it is complicated but uniform (I wrote a partly complete one
> in 1000 lines of Lua).

I believe it can be better done than what glibc does.

> However I can see no reason why dhcp on a specified interface needs to
> enumerate interfaces at all, and it only needs to read ipv4 addresses,
> unless it is implementing dhcp6 too, maybe it does now. Again dhcp6
> needs netlink, the Musl ipv6 parts for getifaddrs already use /proc
> which is definitely unreliable for early boot config in a distro in my
> view.

dhcpcd != dhcp.

In any case, i think if_nameindex should return a list of all
interfaces, not only those who have a configured ipv4 address.

I also think that the way getifaddrs uses /proc for ipv6 is ugly so I'd
say that both getifaddrs and if_nameindex would be better off with
netlink.

-nc

Re: if_nameindex/getifaddrs and dhcpcd issue

[Rich Felker]

On Tue, Apr 08, 2014 at 03:16:10PM +0100, Justin Cormack wrote:
> On Tue, Apr 8, 2014 at 2:42 PM, Rich Felker <dalias@aerifal.cx> wrote:
> >> and it only needs to read ipv4 addresses,
> >> unless it is implementing dhcp6 too, maybe it does now. Again dhcp6
> >> needs netlink, the Musl ipv6 parts for getifaddrs already use /proc
> >> which is definitely unreliable for early boot config in a distro in my
> >> view.
> >
> > In what way does dhcp6 need netlink? What's made this discussion
> > difficult so far on IRC is assertions of that form (although not the
> > same one) without an explanation of why it's believed to be true, so
> > I'd like to keep rational discussion possible by making sure that such
> > claims are backed up by explanation rather than just stated as fact.
> 
> I was under the impression that the ioctl-based interface for ipv6 is
> incomplete under Linux.

Probably "incomplete" in a sense that it can't do some special-purpose
stuff that most users don't need. Busybox entirely avoids netlink, as
far as I can tell, and it's perfectly acceptable for setting up ipv6,
at least in simple setups. You don't even need busybox's iproute2
workalikes; ifconfig and route work fine.

> That does not mean anything needs to be in
> libc though. ISC dhcp for v6 just calls out to ip in scripts, rather
> than ifconfig that it uses for v4, so it is indirectly uses netlink,
> but does not require any libc support, indeed all the C code is
> portable.

udhcpcd works the same; it doesn't make any changes to the interfaces;
it just speaks the dhcp protocol. This is really the correct
factorization.

Rich

Re: if_nameindex/getifaddrs and dhcpcd issue

[Rich Felker]

On Tue, Apr 08, 2014 at 03:25:59PM +0300, Timo Teras wrote:
> > a lot smaller, but you could make a full netlink library in not much
> > more as it is complicated but uniform (I wrote a partly complete one
> > in 1000 lines of Lua).
> > 
> > However I can see no reason why dhcp on a specified interface needs to
> > enumerate interfaces at all, and it only needs to read ipv4 addresses,
> > unless it is implementing dhcp6 too, maybe it does now. Again dhcp6
> > needs netlink, the Musl ipv6 parts for getifaddrs already use /proc
> > which is definitely unreliable for early boot config in a distro in my
> > view.
> 
> We should not be looking at dhcpd. It's the APIs musl implements:
> getifaddrs() and if_nameindex() - they are not currently exposing all
> the information they should.

One can argue about what if_nameindex should expose; for instance,
should it expose all possible interface names the kernel could
support, even with modules that haven't been loaded yet? The only
thing a strictly conforming application can _DO_ with interface
names/numbers is use them for link-local ipv6 scope ids (this is
presumably why these functions were added to POSIX in the first
place). So, from a conformance standpoint, only exposing ids that
could actually appear on a link-local address (i.e. configured
interfaces that have ipv6 link-local addresses) would be sufficient.

None of this is an argument that it wouldn't be nice to list more
interfaces, but it's not a requirement. It's more a matter of
providing an additional feature that might be useful to applications.

> I'd prefer using netlink, instead of trying to parse and connect data
> from various /proc files.
> 
> IMHO, if someone wants to be linux compatible today, it's easier to
> implement the netlink stuff; than the /proc stuff. /proc has equally
> linux specific things in it and is mainly intended to be human
> readable with few exceptions. /sys would be better option as it's
> inteded to be machine readable, but it's still text too. 

/sys is explicitly not usable by libc/apps since policy is that it's
not a stable interface.

> netlink is here to stay, there's no alternate way to do certain things.
> So I'd rather use it. It's the interface kernel people intended to be
> used for the thing in question.

As I mentioned on IRC, the current netlink protocol is deficient; my
understanding is that it's incapable of representing hardware
addresses longer than a certain fixed length, leading to truncation of
infiniband addresses. I'm not an expert on this but I seem to remember
someone (IIRC Rob Landley) knows the details. I'm not sure how fixable
this is, but it's not a problem at all with a clean text-based
interface like /proc provides.

> I'm willing to write an alternative getifaddrs() and if_nameindex()
> implementations using netlink. Perhaps let's see how they end up?

It wouldn't hurt; if they're not upstreamed they could still be used
as a separate library for the one application which needs this
functionality (dhcpcd).

Rich

Re: if_nameindex/getifaddrs and dhcpcd issue

[Timo Teras]

On Tue, 8 Apr 2014 11:45:37 -0400
Rich Felker <dalias@aerifal.cx> wrote:

> > netlink is here to stay, there's no alternate way to do certain
> > things. So I'd rather use it. It's the interface kernel people
> > intended to be used for the thing in question.  
> 
> As I mentioned on IRC, the current netlink protocol is deficient; my
> understanding is that it's incapable of representing hardware
> addresses longer than a certain fixed length, leading to truncation of
> infiniband addresses. I'm not an expert on this but I seem to remember
> someone (IIRC Rob Landley) knows the details. I'm not sure how fixable
> this is, but it's not a problem at all with a clean text-based
> interface like /proc provides.

Netlink is very extensible. If this is the case they have messed up
some struct that exposes.

glibc has some struct sockaddr_ll_max hack. It is not due to netlink.
It's due to getifaddrs() interface exposing struct sockaddr* and it's
using AF_PACKET that maps to struct sockaddr_ll. struct sockaddr_ll
cannot hold infiniband address, but glibc is 'overriding' the struct to
hold it anyway.

Basically it's getifaddrs() API that is botched,

But as to netlink, all the addresses I've looked at are not wrapped in
struct sockaddr. They are netlink 'attributes' that are variable
length.

The only ABI mess in netlink has been with 32-bit apps running on
64-bit kernels. And it has been kernel header issue.

Please let me know details on the claimed deficiency of netlink. It *a
lot* better (but also a bit complicated - and even more often
misunderstood [because the #define's exposed by kernel to use it is
garbage]) than the the traditional APIs IMHO. The 'on wire' protocol of
netlink is usable though.

> > I'm willing to write an alternative getifaddrs() and if_nameindex()
> > implementations using netlink. Perhaps let's see how they end up?  
> 
> It wouldn't hurt; if they're not upstreamed they could still be used
> as a separate library for the one application which needs this
> functionality (dhcpcd).

Yeah. I'll play with this and see what I come up with. I'll also delete
the bad kernel #define's and try to do them a bit better - not sure how
well I succeed that at, though.

Re: if_nameindex/getifaddrs and dhcpcd issue

[Justin Cormack]

On Tue, Apr 8, 2014 at 5:08 PM, Timo Teras <timo.teras@iki.fi> wrote:
> But as to netlink, all the addresses I've looked at are not wrapped in
> struct sockaddr. They are netlink 'attributes' that are variable
> length.
>
> The only ABI mess in netlink has been with 32-bit apps running on
> 64-bit kernels. And it has been kernel header issue.
>
> Please let me know details on the claimed deficiency of netlink. It *a
> lot* better (but also a bit complicated - and even more often
> misunderstood [because the #define's exposed by kernel to use it is
> garbage]) than the the traditional APIs IMHO. The 'on wire' protocol of
> netlink is usable though.

Yes I quite like netlink. If only it was documented it would be a lot
nicer. It is more pleasant to implement it in a dynamicly typed
language than in C with #defines. But I don't think it should be in
libc if possible...

Justin

Re: if_nameindex/getifaddrs and dhcpcd issue

[Natanael Copa]

On Tue, 8 Apr 2014 09:42:55 -0400
Rich Felker <dalias@aerifal.cx> wrote:

> On Tue, Apr 08, 2014 at 11:07:47AM +0100, Justin Cormack wrote:
> > However I can see no reason why dhcp on a specified interface needs to
> > enumerate interfaces at all,
> 
> Indeed. Regardless of how we address this topic in musl, I think we
> should push for dhcpcd to be fixed. udhcpcd works perfectly fine with
> no scanning of interfaces; you simply tell it the interface to use,
> and it uses that. My understanding is that dhcpcd can do the same, but
> it still insists on scanning all interfaces and refuses to run if the
> interface you requested to use is not in the list. This is bad
> behavior.

It looks like the reason for this is that you can provide a wilcard, eg
'eth*' and it will monitor all interfaces that fnmatches that.

> The other case, where no interface is specified on the command line
> and dhcpcd tries all interfaces, is buggy usage by the caller. There
> are all sorts of interfaces that might exist, unconfigured, and which
> might not be appropriate to send dhcpc requests on. I assume dhcpcd
> has some heuristics to avoid selecting things like unconfigured
> tunnel, slip, etc. interfaces but if so that's just an ugly hack. The
> operation of "try all instances of a given type of resource" is just
> wrong by design.

Yes you can configure a list of exact interfaces that you want dhcpcd
to monitor or you can have a black list - with wilcards.

See allowinterfaces and denyinterfaces (which also can be specified on
command line) http://roy.marples.name/man/html5/dhcpcd.conf.html

dhcpcd is more an ifplugd + dhcp client wit optional support for dbus.
It can also get notifications from wpa_supplicant. You can even have
dhcpcd to set static ip addresses. So its more an alternative to
NetworkManager than to udhcpc.

Other thing it can handle is currently unknown interfaces. For example
the laptop I use right now does not have any ethernet port, only wifi.
I do have an ethernet USB adapter. So eth0 is not there at bootup. I
currently get error messages on screen due to udhcpc does not find the
configured eth0 interface.

To solve this, without needing install the full NetworkManager suite or
systemds networkd, I can simply let dhcpcd run in manager mode, and it
will run the show. There is even a dhcpcd-ui applet that gives me link
status etc on my desktop.

Note that I'm not really making any judgement if this is correct or
wrong design, but I think it might explain why it works like it works.

I can of course suggest for Roy Marples to redesign it but I suspect
that the answer will be somethign like: if you want udhcpc, use udhcpc. 

> > and it only needs to read ipv4 addresses,
> > unless it is implementing dhcp6 too, maybe it does now. Again dhcp6
> > needs netlink, the Musl ipv6 parts for getifaddrs already use /proc
> > which is definitely unreliable for early boot config in a distro in my
> > view.
> 
> In what way does dhcp6 need netlink? What's made this discussion
> difficult so far on IRC is assertions of that form (although not the
> same one) without an explanation of why it's believed to be true, so
> I'd like to keep rational discussion possible by making sure that such
> claims are backed up by explanation rather than just stated as fact.

I don't really have experience with ipv6 so I can not comment on that.
But I know for sure that the current ioctl implementation cannot
display multiple addresses on same interface. IIRC that was one of the
reasons netlink exists in first place.

You can actually easily see that with the current musl getifaddrs
implementation. It will only show the primary address and nothing else.
So you can currently not get all addresses with musl.

 
> Rich

Re: if_nameindex/getifaddrs and dhcpcd issue

[Justin Cormack]

On Tue, Apr 8, 2014 at 10:16 PM, Natanael Copa <ncopa@alpinelinux.org> wrote:
> It looks like the reason for this is that you can provide a wilcard, eg
> 'eth*' and it will monitor all interfaces that fnmatches that.
>
>> The other case, where no interface is specified on the command line
>> and dhcpcd tries all interfaces, is buggy usage by the caller. There
>> are all sorts of interfaces that might exist, unconfigured, and which
>> might not be appropriate to send dhcpc requests on. I assume dhcpcd
>> has some heuristics to avoid selecting things like unconfigured
>> tunnel, slip, etc. interfaces but if so that's just an ugly hack. The
>> operation of "try all instances of a given type of resource" is just
>> wrong by design.
>
> Yes you can configure a list of exact interfaces that you want dhcpcd
> to monitor or you can have a black list - with wilcards.
>
> See allowinterfaces and denyinterfaces (which also can be specified on
> command line) http://roy.marples.name/man/html5/dhcpcd.conf.html
>
> dhcpcd is more an ifplugd + dhcp client wit optional support for dbus.
> It can also get notifications from wpa_supplicant. You can even have
> dhcpcd to set static ip addresses. So its more an alternative to
> NetworkManager than to udhcpc.
>
> Other thing it can handle is currently unknown interfaces. For example
> the laptop I use right now does not have any ethernet port, only wifi.
> I do have an ethernet USB adapter. So eth0 is not there at bootup. I
> currently get error messages on screen due to udhcpc does not find the
> configured eth0 interface.
>
> To solve this, without needing install the full NetworkManager suite or
> systemds networkd, I can simply let dhcpcd run in manager mode, and it
> will run the show. There is even a dhcpcd-ui applet that gives me link
> status etc on my desktop.

Sounds like the sort of design that should have netlink built into it
then. You can do all sorts of useful things then, like listen on a
netlink socket for new interfaces being created.

Justin

Re: if_nameindex/getifaddrs and dhcpcd issue

[Rich Felker]

On Tue, Apr 08, 2014 at 05:19:12PM +0100, Justin Cormack wrote:
> On Tue, Apr 8, 2014 at 5:08 PM, Timo Teras <timo.teras@iki.fi> wrote:
> > But as to netlink, all the addresses I've looked at are not wrapped in
> > struct sockaddr. They are netlink 'attributes' that are variable
> > length.
> >
> > The only ABI mess in netlink has been with 32-bit apps running on
> > 64-bit kernels. And it has been kernel header issue.
> >
> > Please let me know details on the claimed deficiency of netlink. It *a
> > lot* better (but also a bit complicated - and even more often
> > misunderstood [because the #define's exposed by kernel to use it is
> > garbage]) than the the traditional APIs IMHO. The 'on wire' protocol of
> > netlink is usable though.
> 
> Yes I quite like netlink. If only it was documented it would be a lot
> nicer. It is more pleasant to implement it in a dynamicly typed
> language than in C with #defines. But I don't think it should be in
> libc if possible...

As an aside, whether netlink is essential for a full-function modern
linux system is orthogonal to whether libc should be using it. A
hypothetical non-linux kernel providing linux syscall abi could
provide completely different net config utils.

Rich

Re: if_nameindex/getifaddrs and dhcpcd issue

[Rich Felker]

On Tue, Apr 08, 2014 at 10:30:34PM +0100, Justin Cormack wrote:
> On Tue, Apr 8, 2014 at 10:16 PM, Natanael Copa <ncopa@alpinelinux.org> wrote:
> > It looks like the reason for this is that you can provide a wilcard, eg
> > 'eth*' and it will monitor all interfaces that fnmatches that.
> >
> >> The other case, where no interface is specified on the command line
> >> and dhcpcd tries all interfaces, is buggy usage by the caller. There
> >> are all sorts of interfaces that might exist, unconfigured, and which
> >> might not be appropriate to send dhcpc requests on. I assume dhcpcd
> >> has some heuristics to avoid selecting things like unconfigured
> >> tunnel, slip, etc. interfaces but if so that's just an ugly hack. The
> >> operation of "try all instances of a given type of resource" is just
> >> wrong by design.
> >
> > Yes you can configure a list of exact interfaces that you want dhcpcd
> > to monitor or you can have a black list - with wilcards.
> >
> > See allowinterfaces and denyinterfaces (which also can be specified on
> > command line) http://roy.marples.name/man/html5/dhcpcd.conf.html
> >
> > dhcpcd is more an ifplugd + dhcp client wit optional support for dbus.
> > It can also get notifications from wpa_supplicant. You can even have
> > dhcpcd to set static ip addresses. So its more an alternative to
> > NetworkManager than to udhcpc.
> >
> > Other thing it can handle is currently unknown interfaces. For example
> > the laptop I use right now does not have any ethernet port, only wifi.
> > I do have an ethernet USB adapter. So eth0 is not there at bootup. I
> > currently get error messages on screen due to udhcpc does not find the
> > configured eth0 interface.
> >
> > To solve this, without needing install the full NetworkManager suite or
> > systemds networkd, I can simply let dhcpcd run in manager mode, and it
> > will run the show. There is even a dhcpcd-ui applet that gives me link
> > status etc on my desktop.
> 
> Sounds like the sort of design that should have netlink built into it
> then. You can do all sorts of useful things then, like listen on a
> netlink socket for new interfaces being created.

Yes. The above explanation of dhcpcd makes it clear what it's
enumerating interfaces for, but also suggests it should be using
netlink entirely rather than libc to avoid TOCTOU races on device
insertion/removal.

Rich

Re: if_nameindex/getifaddrs and dhcpcd issue

[Natanael Copa]

On Tue, 8 Apr 2014 11:38:41 -0400
Rich Felker <dalias@aerifal.cx> wrote:

> On Tue, Apr 08, 2014 at 03:16:10PM +0100, Justin Cormack wrote:
> > I was under the impression that the ioctl-based interface for ipv6 is
> > incomplete under Linux.
> 
> Probably "incomplete" in a sense that it can't do some special-purpose
> stuff that most users don't need.

You cannot get the configured ipv6 addresses via SIOCGIFCONF. That is
why musl needs to parse /proc for that.

ioctl-based interface for ipv4 is also "incomplete". You cannot get
anything else than the primary address so current musl getifaddrs will
not give you all configured ipv4 addresses.

> Busybox entirely avoids netlink, as
> far as I can tell,

Not entirely. the iproute implementation in bb uses netlink, but you
can of course disable that during configuration. A quick grep also
indicate that busybox ifplugd uses netlink.

> and it's perfectly acceptable for setting up ipv6,
> at least in simple setups. You don't even need busybox's iproute2
> workalikes; ifconfig and route work fine.

And if you do that, then you cannot assign more than one ip addr on
each interface. (you'll have to use alias, eth0:1 for that), but yes,
thats no longer a simple setup.

-nc

Re: if_nameindex/getifaddrs and dhcpcd issue

[u-igbb]

On Tue, Apr 08, 2014 at 06:41:12PM -0400, Rich Felker wrote:
> As an aside, whether netlink is essential for a full-function modern
> linux system is orthogonal to whether libc should be using it. A
> hypothetical non-linux kernel providing linux syscall abi could
> provide completely different net config utils.

(I can not help wondering why you call this hypothetical,
 routinely using my "linux" binaries on *BSD.
Oh wait, was it because mentioning demons' names is inappropriate? :)

Sure, it is valuable that the "general" and "kernel internals specific"
functionality is separated, please keep it this way where possible.

Rune

Re: if_nameindex/getifaddrs and dhcpcd issue

[Natanael Copa]

On Tue, 8 Apr 2014 11:45:37 -0400
Rich Felker <dalias@aerifal.cx> wrote:

> > We should not be looking at dhcpd. It's the APIs musl implements:
> > getifaddrs() and if_nameindex() - they are not currently exposing all
> > the information they should.
> 
> One can argue about what if_nameindex should expose;

Yes. This is the real issue at hand. What dhcpcd does is not that
relevant here.

> for instance,
> should it expose all possible interface names the kernel could
> support, even with modules that haven't been loaded yet?

IMHO, no. I think it should only expose what you normally find
as /sys/class/net/* (you find same list in /proc/net/dev)

A use case if_nameindex() that I think would be valid is a network config ui.

Which interface do you want to configure? 
Presents a list of currently available network interfaces. (from if_nameindex)
It has a button 'Add virtual interface', which lets user configure
bonding/bridges/vlans etc.

(yes, no application should assume that the listed interface actually
works. I could select my USB ethernet dongle and before press 'ok' I
could pull out the USB ethernet...)

> The only
> thing a strictly conforming application can _DO_ with interface
> names/numbers is use them for link-local ipv6 scope ids (this is
> presumably why these functions were added to POSIX in the first
> place). So, from a conformance standpoint, only exposing ids that
> could actually appear on a link-local address (i.e. configured
> interfaces that have ipv6 link-local addresses) would be sufficient.

Then, from a conformance standpoint, the current musl if_nameindex
implementation is broken. It only lists interfaces with configured
ipv4 addresses.

To test yourself:

modprobe dummy
ip link set up dev dummy0
ip addr   #verify that dummy has ipv6 link-local

Then run the following testcase:

#include <net/if.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>

int
main(int argc, char *argv[])
{
    struct if_nameindex *if_ni, *i;

   if_ni = if_nameindex();
    if (if_ni == NULL) {
        perror("if_nameindex");
        exit(EXIT_FAILURE);
    }

   for (i = if_ni; ! (i->if_index == 0 && i->if_name == NULL); i++)
        printf("%u: %s\n", i->if_index, i->if_name);

   if_freenameindex(if_ni);

   exit(EXIT_SUCCESS);
}

The ipv6 link-local configured dummy0 interface will not be listed.



-nc

Re: if_nameindex/getifaddrs and dhcpcd issue

[Timo Teras]

On Tue, 8 Apr 2014 19:08:07 +0300
Timo Teras <timo.teras@iki.fi> wrote:

> > > I'm willing to write an alternative getifaddrs() and
> > > if_nameindex() implementations using netlink. Perhaps let's see
> > > how they end up?  
> > 
> > It wouldn't hurt; if they're not upstreamed they could still be used
> > as a separate library for the one application which needs this
> > functionality (dhcpcd).
> 
> Yeah. I'll play with this and see what I come up with. I'll also
> delete the bad kernel #define's and try to do them a bit better - not
> sure how well I succeed that at, though.

Posting here the current version of my patch as requested.

While it has some corner cases still that need cleaning, it's a good
start. It implements the APIs, and is usable in read world.

From 4a82a84fb4fbfdae48f14bf86df0fd92086b7556 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timo=20Ter=C3=A4s?= <timo.teras@iki.fi>
Date: Tue, 8 Apr 2014 14:03:16 +0000
Subject: [PATCH] reimplement if_nameindex and getifaddrs using netlink

---
 src/network/__netlink.c    |  68 ++++++++++
 src/network/__netlink.h    | 143 ++++++++++++++++++++
 src/network/getifaddrs.c   | 322 ++++++++++++++++++++++++---------------------
 src/network/if_nameindex.c | 105 +++++++++------
 4 files changed, 451 insertions(+), 187 deletions(-)
 create mode 100644 src/network/__netlink.c
 create mode 100644 src/network/__netlink.h

diff --git a/src/network/__netlink.c b/src/network/__netlink.c
new file mode 100644
index 0000000..d0c9fab
--- /dev/null
+++ b/src/network/__netlink.c
@@ -0,0 +1,68 @@
+#define _GNU_SOURCE
+#include <errno.h>
+#include <string.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <sys/socket.h>
+#include <sys/param.h>
+#include "__netlink.h"
+
+struct __netlink_handle {
+	int fd;
+	unsigned int seq;
+	size_t bufsize;
+};
+
+struct __netlink_handle *__netlink_open(int type)
+{
+	struct __netlink_handle *nh;
+	int bufsize = getpagesize();
+	/* required buffer size is MIN(8192,pagesize)-sizeof(struct skb_shared_info)
+	 * the estimate for skb_shared_info size is conservative, but gives enough
+	 * space to fit struct __netlink_handle including malloc overhead in one page . */
+	if (bufsize > 8192) bufsize = 8192;
+	bufsize -= 128;
+	nh = malloc(sizeof(struct __netlink_handle) + bufsize);
+	if (!nh) return 0;
+	nh->fd = socket(PF_NETLINK, SOCK_RAW|SOCK_CLOEXEC, type);
+	if (nh->fd < 0) { free(nh); return 0; }
+	nh->seq = 1;
+	nh->bufsize = bufsize;
+	return nh;
+}
+
+void __netlink_close(struct __netlink_handle *nh)
+{
+	close(nh->fd);
+	free(nh);
+}
+
+int __netlink_enumerate(struct __netlink_handle *nh, int type, int (*cb)(void *ctx, struct nlmsghdr *h), void *ctx)
+{
+	struct nlmsghdr *h;
+	void *buf = (void*)(nh+1);
+	struct {
+		struct nlmsghdr nlh;
+		struct rtgenmsg g;
+	} *req = buf;
+	int r, ret = 0;
+
+	memset(req, 0, NETLINK_ALIGN(sizeof(*req)));
+	req->nlh.nlmsg_len = sizeof(*req);
+	req->nlh.nlmsg_type = type;
+	req->nlh.nlmsg_flags = NLM_F_ROOT | NLM_F_MATCH | NLM_F_REQUEST;
+	req->nlh.nlmsg_seq = nh->seq++;
+	req->g.rtgen_family = AF_UNSPEC;
+	r = send(nh->fd, req, sizeof(*req), 0);
+	if (r < 0) return r;
+
+	while (1) {
+		r = recv(nh->fd, buf, nh->bufsize, MSG_DONTWAIT);
+		if (r <= 0) return -1;
+		for (h = (struct nlmsghdr*) buf; NLMSG_OK(h, (void*)((uint8_t*)buf+r)); h = NLMSG_NEXT(h)) {
+			if (h->nlmsg_type == NLMSG_DONE) return ret;
+			if (h->nlmsg_type == NLMSG_ERROR) return -1;
+			if (!ret) ret = cb(ctx, h);
+		}
+	}
+}
diff --git a/src/network/__netlink.h b/src/network/__netlink.h
new file mode 100644
index 0000000..94728f3
--- /dev/null
+++ b/src/network/__netlink.h
@@ -0,0 +1,143 @@
+#include <stdint.h>
+
+/* linux/netlink.h */
+
+#define NETLINK_ROUTE 0
+
+struct nlmsghdr {
+	uint32_t	nlmsg_len;
+	uint16_t	nlmsg_type;
+	uint16_t	nlmsg_flags;
+	uint32_t	nlmsg_seq;
+	uint32_t	nlmsg_pid;
+};
+
+#define NLM_F_REQUEST	1
+#define NLM_F_MULTI	2
+#define NLM_F_ACK	4
+#define NLM_F_ECHO	8
+#define NLM_F_DUMP_INTR	16
+
+#define NLM_F_ROOT	0x100
+#define NLM_F_MATCH	0x200
+#define NLM_F_ATOMIC	0x400
+#define NLM_F_DUMP	(NLM_F_ROOT|NLM_F_MATCH)
+
+#define NLMSG_NOOP	0x1
+#define NLMSG_ERROR	0x2
+#define NLMSG_DONE	0x3
+#define NLMSG_OVERRUN	0x4
+
+/* linux/rtnetlink.h */
+
+#define RTM_GETLINK	18
+#define RTM_GETADDR	22
+
+struct rtattr {
+	unsigned short	rta_len;
+	unsigned short	rta_type;
+};
+
+struct rtgenmsg {
+	unsigned char	rtgen_family;
+};
+
+struct ifinfomsg {
+	unsigned char	ifi_family;
+	unsigned char	__ifi_pad;
+	unsigned short	ifi_type;
+	int		ifi_index;
+	unsigned	ifi_flags;
+	unsigned	ifi_change;
+};
+
+/* linux/if_link.h */
+
+enum {
+	IFLA_UNSPEC,
+	IFLA_ADDRESS,
+	IFLA_BROADCAST,
+	IFLA_IFNAME,
+	IFLA_MTU,
+	IFLA_LINK,
+	IFLA_QDISC,
+	IFLA_STATS,
+	IFLA_COST,
+	IFLA_PRIORITY,
+	IFLA_MASTER,
+	IFLA_WIRELESS,
+	IFLA_PROTINFO,
+	IFLA_TXQLEN,
+	IFLA_MAP,
+	IFLA_WEIGHT,
+	IFLA_OPERSTATE,
+	IFLA_LINKMODE,
+	IFLA_LINKINFO,
+	IFLA_NET_NS_PID,
+	IFLA_IFALIAS,
+	IFLA_NUM_VF,
+	IFLA_VFINFO_LIST,
+	IFLA_STATS64,
+	IFLA_VF_PORTS,
+	IFLA_PORT_SELF,
+	IFLA_AF_SPEC,
+	IFLA_GROUP,
+	IFLA_NET_NS_FD,
+	IFLA_EXT_MASK,
+	IFLA_PROMISCUITY,
+	IFLA_NUM_TX_QUEUES,
+	IFLA_NUM_RX_QUEUES,
+	IFLA_CARRIER,
+	IFLA_PHYS_PORT_ID,
+	__IFLA_MAX
+};
+
+/* linux/if_addr.h */
+
+struct ifaddrmsg {
+	uint8_t		ifa_family;
+	uint8_t		ifa_prefixlen;
+	uint8_t		ifa_flags;
+	uint8_t		ifa_scope;
+	uint32_t	ifa_index;
+};
+
+enum {
+	IFA_UNSPEC,
+	IFA_ADDRESS,
+	IFA_LOCAL,
+	IFA_LABEL,
+	IFA_BROADCAST,
+	IFA_ANYCAST,
+	IFA_CACHEINFO,
+	IFA_MULTICAST,
+	__IFA_MAX
+};
+
+/* musl */
+
+#define NETLINK_ALIGN(len)	(((len)+3) & ~3)
+#define NLMSG_DATA(nlh)		((void*)((char*)(nlh)+NETLINK_ALIGN(sizeof(struct nlmsghdr))))
+#define NLMSG_DATALEN(nlh)	((nlh)->nlmsg_len-NETLINK_ALIGN(sizeof(struct nlmsghdr)))
+#define NLMSG_DATAEND(nlh)	((char*)(nlh)+(nlh)->nlmsg_len)
+#define NLMSG_NEXT(nlh)		(struct nlmsghdr*)((char*)(nlh)+NETLINK_ALIGN((nlh)->nlmsg_len))
+#define NLMSG_OK(nlh,end)	(NLMSG_DATA(nlh) <= (end) && \
+				 (nlh)->nlmsg_len >= sizeof(struct nlmsghdr) && \
+				 (void*)NLMSG_NEXT(nlh) <= (end))
+
+#define RTA_DATA(rta)		((void*)((char*)(rta)+NETLINK_ALIGN(sizeof(struct rtattr))))
+#define RTA_DATALEN(rta)	((rta)->rta_len-NETLINK_ALIGN(sizeof(struct rtattr)))
+#define RTA_DATAEND(rta)	((char*)(rta)+(rta)->rta_len)
+#define RTA_NEXT(rta)		(struct rtattr*)((char*)(rta)+NETLINK_ALIGN((rta)->rta_len))
+#define RTA_OK(rta,end)		(RTA_DATA(rta) <= (void*)(end) && \
+				 (rta)->rta_len >= sizeof(struct rtattr) && \
+				 (void*)RTA_NEXT(rta) <= (void*)(end))
+
+#define NLMSG_RTA(nlh,len)	((void*)((char*)(nlh)+NETLINK_ALIGN(sizeof(struct nlmsghdr))+NETLINK_ALIGN(len)))
+#define NLMSG_RTAOK(rta,nlh)	RTA_OK(rta,NLMSG_DATAEND(nlh))
+
+struct __netlink_handle;
+
+struct __netlink_handle *__netlink_open(int type);
+void __netlink_close(struct __netlink_handle *h);
+int __netlink_enumerate(struct __netlink_handle *h, int type, int (*cb)(void *ctx, struct nlmsghdr *h), void *ctx);
diff --git a/src/network/getifaddrs.c b/src/network/getifaddrs.c
index 5a94cc7..5b1ebe7 100644
--- a/src/network/getifaddrs.c
+++ b/src/network/getifaddrs.c
@@ -1,181 +1,209 @@
-/* (C) 2013 John Spencer. released under musl's standard MIT license. */
-#undef _GNU_SOURCE
 #define _GNU_SOURCE
-#include <ifaddrs.h>
-#include <stdlib.h>
-#include <net/if.h> /* IFNAMSIZ, ifreq, ifconf */
-#include <stdio.h>
-#include <ctype.h>
-#include <string.h>
 #include <errno.h>
-#include <arpa/inet.h> /* inet_pton */
+#include <string.h>
+#include <stdlib.h>
 #include <unistd.h>
-#include <sys/ioctl.h>
-#include <sys/socket.h>
+#include <ifaddrs.h>
+#include <net/if.h>
+#include "__netlink.h"
 
-typedef union {
-	struct sockaddr_in6 v6;
+/* getifaddrs() uses PF_PACKET to relay hardware addresses.
+ * But Infiniband socket address length is longer, so use this hack
+ * (like glibc) to return it anyway. */
+struct sockaddr_ll_hack {
+	unsigned short sll_family, sll_protocol;
+	int sll_ifindex;
+	unsigned short sll_hatype;
+	unsigned char sll_pkttype, sll_halen;
+	unsigned char sll_addr[24];
+};
+
+union sockany {
+	struct sockaddr sa;
+	struct sockaddr_ll_hack ll;
 	struct sockaddr_in v4;
-} soa;
+	struct sockaddr_in6 v6;
+};
 
-typedef struct ifaddrs_storage {
+struct ifaddrs_storage {
 	struct ifaddrs ifa;
-	soa addr;
-	soa netmask;
-	soa dst;
+	struct ifaddrs_storage *hash_next;
+	union sockany addr, netmask, ifu;
+	unsigned int index;
 	char name[IFNAMSIZ+1];
-} stor;
-#define next ifa.ifa_next
+};
 
-static stor* list_add(stor** list, stor** head, char* ifname)
+#define IFADDRS_HASH_SIZE 64
+struct ifaddrs_ctx {
+	struct ifaddrs_storage *first;
+	struct ifaddrs_storage *last;
+	struct ifaddrs_storage *hash[IFADDRS_HASH_SIZE];
+};
+
+void freeifaddrs(struct ifaddrs *ifp)
 {
-	stor* curr = calloc(1, sizeof(stor));
-	if(curr) {
-		strcpy(curr->name, ifname);
-		curr->ifa.ifa_name = curr->name;
-		if(*head) (*head)->next = (struct ifaddrs*) curr;
-		*head = curr;
-		if(!*list) *list = curr;
+	struct ifaddrs *n;
+	while (ifp) {
+		n = ifp->ifa_next;
+		free(ifp);
+		ifp = n;
 	}
-	return curr;
 }
 
-void freeifaddrs(struct ifaddrs *ifp)
+static void addifaddrs(struct ifaddrs_ctx *ctx, struct ifaddrs_storage *add)
 {
-	stor *head = (stor *) ifp;
-	while(head) {
-		void *p = head;
-		head = (stor *) head->next;
-		free(p);
+	if (!add->ifa.ifa_name) {
+		free(add);
+		return;
 	}
+	if (!ctx->first) ctx->first = add;
+	if (ctx->last) ctx->last->ifa.ifa_next = &add->ifa;
+	ctx->last = add;
 }
 
-static void ipv6netmask(unsigned prefix_length, struct sockaddr_in6 *sa)
+static struct sockaddr* copy_lladdr(union sockany *sa, struct rtattr *rta, struct ifinfomsg *ifi)
 {
-	unsigned char* hb = sa->sin6_addr.s6_addr;
-	unsigned onebytes = prefix_length / 8;
-	unsigned bits = prefix_length % 8;
-	unsigned nullbytes = 16 - onebytes;
-	memset(hb, -1, onebytes);
-	memset(hb+onebytes, 0, nullbytes);
-	if(bits) {
-		unsigned char x = -1;
-		x <<= 8 - bits;
-		hb[onebytes] = x;
+	if (RTA_DATALEN(rta) > sizeof(sa->ll.sll_addr)) return 0;
+	sa->ll.sll_family = AF_PACKET;
+	sa->ll.sll_ifindex = ifi->ifi_index;
+	sa->ll.sll_hatype = ifi->ifi_type;
+	sa->ll.sll_halen = RTA_DATALEN(rta);
+	memcpy(sa->ll.sll_addr, RTA_DATA(rta), RTA_DATALEN(rta));
+	return &sa->sa;
+}
+
+static uint8_t *sockany_addr(int af, union sockany *sa, int *len)
+{
+	switch (af) {
+	case AF_INET: *len = 4; return (uint8_t*) &sa->v4.sin_addr;
+	case AF_INET6: *len = 16; return (uint8_t*) &sa->v6.sin6_addr;
 	}
+	return 0;
 }
 
-static void dealwithipv6(stor **list, stor** head)
+static struct sockaddr* copy_addr(int af, union sockany *sa, struct rtattr *rta)
 {
-	FILE* f = fopen("/proc/net/if_inet6", "rbe");
-	/* 00000000000000000000000000000001 01 80 10 80 lo
-	   A                                B  C  D  E  F
-	   all numbers in hex
-	   A = addr B=netlink device#, C=prefix length,
-	   D = scope value (ipv6.h) E = interface flags (rnetlink.h, addrconf.c)
-	   F = if name */
-	char v6conv[32 + 7 + 1], *v6;
-	char *line, linebuf[512];
-	if(!f) return;
-	while((line = fgets(linebuf, sizeof linebuf, f))) {
-		v6 = v6conv;
-		size_t i = 0;
-		for(; i < 8; i++) {
-			memcpy(v6, line, 4);
-			v6+=4;
-			*v6++=':';
-			line+=4;
-		}
-		--v6; *v6 = 0;
-		line++;
-		unsigned b, c, d, e;
-		char name[IFNAMSIZ+1];
-		if(5 == sscanf(line, "%x %x %x %x %s", &b, &c, &d, &e, name)) {
-			struct sockaddr_in6 sa = {0};
-			if(1 == inet_pton(AF_INET6, v6conv, &sa.sin6_addr)) {
-				sa.sin6_family = AF_INET6;
-				stor* curr = list_add(list, head, name);
-				if(!curr) goto out;
-				curr->addr.v6 = sa;
-				curr->ifa.ifa_addr = (struct sockaddr*) &curr->addr;
-				ipv6netmask(c, &sa);
-				curr->netmask.v6 = sa;
-				curr->ifa.ifa_netmask = (struct sockaddr*) &curr->netmask;
-				/* find ipv4 struct with the same interface name to copy flags */
-				stor* scan = *list;
-				for(;scan && strcmp(name, scan->name);scan=(stor*)scan->next);
-				if(scan) curr->ifa.ifa_flags = scan->ifa.ifa_flags;
-				else curr->ifa.ifa_flags = 0;
-			} else errno = 0;
-		}
+	int len;
+	uint8_t *dst = sockany_addr(af, sa, &len);
+	if (!dst || RTA_DATALEN(rta) != len) return 0;
+	sa->sa.sa_family = af;
+	memcpy(dst, RTA_DATA(rta), len);
+	return &sa->sa;
+}
+
+static struct sockaddr *gen_netmask(int af, union sockany *sa, int prefixlen)
+{
+	int maxlen, i;
+	uint8_t *dst = sockany_addr(af, sa, &maxlen);
+	if (!dst) return 0;
+	sa->sa.sa_family = af;
+	if (prefixlen > 8*maxlen) prefixlen = 8*maxlen;
+	i = prefixlen / 8;
+	memset(dst, 0xff, i);
+	if (i<maxlen) {
+		dst[i++] = 0xff << (8 - (prefixlen % 8));
+		if (i<maxlen) memset(&dst[i+1], 0x00, maxlen-i);
 	}
-	out:
-	fclose(f);
+	return &sa->sa;
 }
 
-int getifaddrs(struct ifaddrs **ifap)
+static int __handle_link(void *pctx, struct nlmsghdr *h)
 {
-	stor *list = 0, *head = 0;
-	struct if_nameindex* ii = if_nameindex();
-	if(!ii) return -1;
-	size_t i;
-	for(i = 0; ii[i].if_index || ii[i].if_name; i++) {
-		stor* curr = list_add(&list, &head, ii[i].if_name);
-		if(!curr) {
-			if_freenameindex(ii);
-			goto err2;
-		}
+	struct ifaddrs_ctx *ctx = pctx;
+	struct ifaddrs_storage *ifs;
+	struct ifinfomsg *ifi = NLMSG_DATA(h);
+	struct rtattr *rta;
+	int stats_len = 0;
+
+	for (rta = NLMSG_RTA(h, sizeof(*ifi)); NLMSG_RTAOK(rta, h); rta = RTA_NEXT(rta)) {
+		if (rta->rta_type != IFLA_STATS) continue;
+		stats_len = RTA_DATALEN(rta);
+		break;
 	}
-	if_freenameindex(ii);
-
-	int sock = socket(PF_INET, SOCK_DGRAM|SOCK_CLOEXEC, IPPROTO_IP);
-	if(sock == -1) goto err2;
-	struct ifreq reqs[32]; /* arbitrary chosen boundary */
-	struct ifconf conf = {.ifc_len = sizeof reqs, .ifc_req = reqs};
-	if(-1 == ioctl(sock, SIOCGIFCONF, &conf)) goto err;
-	size_t reqitems = conf.ifc_len / sizeof(struct ifreq);
-	for(head = list; head; head = (stor*)head->next) {
-		for(i = 0; i < reqitems; i++) {
-			// get SIOCGIFADDR of active interfaces.
-			if(!strcmp(reqs[i].ifr_name, head->name)) {
-				head->addr.v4 = *(struct sockaddr_in*)&reqs[i].ifr_addr;
-				head->ifa.ifa_addr = (struct sockaddr*) &head->addr;
-				break;
+
+	ifs = calloc(1, sizeof(struct ifaddrs_storage) + stats_len);
+	if (ifs == 0) return -1;
+
+	ifs->index = ifi->ifi_index;
+	ifs->ifa.ifa_flags = ifi->ifi_flags;
+
+	for (rta = NLMSG_RTA(h, sizeof(*ifi)); NLMSG_RTAOK(rta, h); rta = RTA_NEXT(rta)) {
+		switch (rta->rta_type) {
+		case IFLA_IFNAME:
+			if (RTA_DATALEN(rta) <= IFNAMSIZ) {
+				strncpy(ifs->name, RTA_DATA(rta), RTA_DATALEN(rta));
+				ifs->ifa.ifa_name = ifs->name;
 			}
+			break;
+		case IFLA_ADDRESS:
+			ifs->ifa.ifa_addr = copy_lladdr(&ifs->addr, rta, ifi);
+			break;
+		case IFLA_BROADCAST:
+			ifs->ifa.ifa_broadaddr = copy_lladdr(&ifs->ifu, rta, ifi);
+			break;
+		case IFLA_STATS:
+			ifs->ifa.ifa_data = (void*)(ifs+1);
+			memcpy(ifs->ifa.ifa_data, RTA_DATA(rta), RTA_DATALEN(rta));
+			break;
 		}
-		struct ifreq req;
-		snprintf(req.ifr_name, sizeof req.ifr_name, "%s", head->name);
-		if(-1 == ioctl(sock, SIOCGIFFLAGS, &req)) goto err;
-
-		head->ifa.ifa_flags = req.ifr_flags;
-		if(head->ifa.ifa_addr) {
-			/* or'ing flags with IFF_LOWER_UP on active interfaces to mimic glibc */
-			head->ifa.ifa_flags |= IFF_LOWER_UP; 
-			if(-1 == ioctl(sock, SIOCGIFNETMASK, &req)) goto err;
-			head->netmask.v4 = *(struct sockaddr_in*)&req.ifr_netmask;
-			head->ifa.ifa_netmask = (struct sockaddr*) &head->netmask;
-	
-			if(head->ifa.ifa_flags & IFF_POINTOPOINT) {
-				if(-1 == ioctl(sock, SIOCGIFDSTADDR, &req)) goto err;
-				head->dst.v4 = *(struct sockaddr_in*)&req.ifr_dstaddr;
-			} else {
-				if(-1 == ioctl(sock, SIOCGIFBRDADDR, &req)) goto err;
-				head->dst.v4 = *(struct sockaddr_in*)&req.ifr_broadaddr;
-			}
-			head->ifa.ifa_ifu.ifu_dstaddr = (struct sockaddr*) &head->dst;
+	}
+	if (ifs->ifa.ifa_name) {
+		ifs->hash_next = ctx->hash[ifs->index%IFADDRS_HASH_SIZE];
+		ctx->hash[ifs->index%IFADDRS_HASH_SIZE] = ifs;
+	}
+	addifaddrs(ctx, ifs);
+	return 0;
+}
+
+static int __handle_addr(void *pctx, struct nlmsghdr *h)
+{
+	struct ifaddrs_ctx *ctx = pctx;
+	struct ifaddrs_storage *ifs, *ifs0;
+	struct ifaddrmsg *ifa = NLMSG_DATA(h);
+	struct rtattr *rta;
+
+	ifs = calloc(1, sizeof(struct ifaddrs_storage));
+	if (ifs == 0) return -1;
+
+	for (ifs0 = ctx->hash[ifa->ifa_index%IFADDRS_HASH_SIZE]; ifs0; ifs0 = ifs0->hash_next)
+		if (ifs0->index == ifa->ifa_index)
+			break;
+	if (!ifs0) return 0;
+
+	ifs->ifa.ifa_name = ifs0->ifa.ifa_name;
+	ifs->ifa.ifa_flags = ifs0->ifa.ifa_flags;
+	for (rta = NLMSG_RTA(h, sizeof(*ifa)); NLMSG_RTAOK(rta, h); rta = RTA_NEXT(rta)) {
+		switch (rta->rta_type) {
+		case IFA_ADDRESS:
+			ifs->ifa.ifa_addr = copy_addr(ifa->ifa_family, &ifs->addr, rta);
+			if (ifs->ifa.ifa_addr)
+				ifs->ifa.ifa_netmask = gen_netmask(ifa->ifa_family, &ifs->netmask, ifa->ifa_prefixlen);
+			break;
+		case IFA_BROADCAST:
+			/* For point-to-point links this is peer, but ifa_broadaddr
+			 * and ifa_dstaddr are union, so this works for both.  */
+			ifs->ifa.ifa_broadaddr = copy_addr(ifa->ifa_family, &ifs->ifu, rta);
+			break;
 		}
 	}
-	close(sock);
-	void* last = 0;
-	for(head = list; head; head=(stor*)head->next) last=head;
-	head = last;
-	dealwithipv6(&list, &head);
-	*ifap = (struct ifaddrs*) list;
+
+	addifaddrs(ctx, ifs);
 	return 0;
-	err:
-	close(sock);
-	err2:
-	freeifaddrs((struct ifaddrs*) list);
-	return -1;
 }
 
+int getifaddrs(struct ifaddrs **ifap)
+{
+	struct ifaddrs_ctx _ctx, *ctx = &_ctx;
+	struct __netlink_handle *nh;
+	int r = 0;
+
+	nh = __netlink_open(NETLINK_ROUTE);
+	if (!nh) return -1;
+	memset(ctx, 0, sizeof(*ctx));
+	if (__netlink_enumerate(nh, RTM_GETLINK, __handle_link, ctx)) r = -1;
+	if (__netlink_enumerate(nh, RTM_GETADDR, __handle_addr, ctx)) r = -1;
+	__netlink_close(nh);
+	if (r == 0) *ifap = &ctx->first->ifa;
+	else freeifaddrs(&ctx->first->ifa);
+	return r;
+}
diff --git a/src/network/if_nameindex.c b/src/network/if_nameindex.c
index 53b80b2..d4e8b2d 100644
--- a/src/network/if_nameindex.c
+++ b/src/network/if_nameindex.c
@@ -1,55 +1,80 @@
 #define _GNU_SOURCE
 #include <net/if.h>
-#include <stdlib.h>
 #include <sys/socket.h>
-#include <sys/ioctl.h>
 #include <errno.h>
-#include "syscall.h"
+#include <unistd.h>
+#include <stdlib.h>
+#include <string.h>
+#include "__netlink.h"
+
+struct ifnamemap {
+	unsigned int index;
+	unsigned char namelen;
+	char name[IFNAMSIZ];
+};
 
-static void *do_nameindex(int s, size_t n)
+struct ifnameindexctx {
+	unsigned int num;
+	unsigned int str_bytes;
+	struct ifnamemap *list;
+};
+
+static int __handle_link(void *pctx, struct nlmsghdr *h)
 {
-	size_t i, len, k;
-	struct ifconf conf;
-	struct if_nameindex *idx;
-
-	idx = malloc(n * (sizeof(struct if_nameindex)+sizeof(struct ifreq)));
-	if (!idx) return 0;
-
-	conf.ifc_buf = (void *)&idx[n];
-	conf.ifc_len = len = n * sizeof(struct ifreq);
-	if (ioctl(s, SIOCGIFCONF, &conf) < 0) {
-		free(idx);
-		return 0;
-	}
-	if (conf.ifc_len == len) {
-		free(idx);
-		return (void *)-1;
-	}
+	struct ifnameindexctx *ctx = pctx;
+	struct ifinfomsg *ifim = NLMSG_DATA(h);
+	struct rtattr *rta;
+	struct ifnamemap *e;
 
-	n = conf.ifc_len / sizeof(struct ifreq);
-	for (i=k=0; i<n; i++) {
-		if (ioctl(s, SIOCGIFINDEX, &conf.ifc_req[i]) < 0) {
-			k++;
-			continue;
-		}
-		idx[i-k].if_index = conf.ifc_req[i].ifr_ifindex;
-		idx[i-k].if_name = conf.ifc_req[i].ifr_name;
+	for (rta = NLMSG_RTA(h, sizeof(*ifim)); NLMSG_RTAOK(rta, h); rta = RTA_NEXT(rta)) {
+		if (rta->rta_type != IFLA_IFNAME) continue;
+		if (RTA_DATALEN(rta) > IFNAMSIZ) return -ENOBUFS;
+
+		ctx->num++;
+		ctx->str_bytes += RTA_DATALEN(rta) + 1;
+		e = realloc(ctx->list, sizeof(struct ifnamemap[ctx->num]));
+		if (e == 0) return -ENOMEM;
+		ctx->list = e;
+
+		e = &ctx->list[ctx->num-1];
+		e->index = ifim->ifi_index;
+		e->namelen = RTA_DATALEN(rta);
+		memcpy(e->name,  RTA_DATA(rta), IFNAMSIZ);
 	}
-	idx[i-k].if_name = 0;
-	idx[i-k].if_index = 0;
 
-	return idx;
+	return 0;
 }
 
 struct if_nameindex *if_nameindex()
 {
-	size_t n;
-	void *p = 0;
-	int s = socket(AF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC, 0);
-	if (s>=0) {
-		for (n=0; (p=do_nameindex(s, n)) == (void *)-1; n++);
-		__syscall(SYS_close, s);
+	struct ifnameindexctx _ctx, *ctx = &_ctx;
+	struct if_nameindex *ifs = NULL;
+	struct __netlink_handle *nh;
+	int r, i;
+	char *p;
+
+	nh = __netlink_open(NETLINK_ROUTE);
+	if (!nh) goto err;
+	memset(ctx, 0, sizeof(*ctx));
+	r = __netlink_enumerate(nh, RTM_GETLINK, __handle_link, ctx);
+	__netlink_close(nh);
+	if (r < 0) goto err;
+
+	ifs = malloc(sizeof(struct if_nameindex[ctx->num+1]) + ctx->str_bytes);
+	if (ifs == 0) goto err;
+
+	p = (char*)ifs + sizeof(struct if_nameindex[ctx->num+1]);
+	for (i = 0; i < ctx->num; i++) {
+		ifs[i].if_index = ctx->list[i].index;
+		ifs[i].if_name = p;
+		memcpy(p, ctx->list[i].name, ctx->list[i].namelen);
+		p += ctx->list[i].namelen;
+		*p++ = 0;
 	}
-	errno = ENOBUFS;
-	return p;
+	ifs[i].if_index = 0;
+	ifs[i].if_name = 0;
+err:
+	free(ctx->list);
+	if (ifs == NULL) errno = ENOBUFS;
+	return ifs;
 }
-- 
1.9.1

Re: if_nameindex/getifaddrs and dhcpcd issue

[Rich Felker]

On Wed, Apr 09, 2014 at 09:13:48AM +0200, Natanael Copa wrote:
> On Tue, 8 Apr 2014 11:38:41 -0400
> Rich Felker <dalias@aerifal.cx> wrote:
> 
> > On Tue, Apr 08, 2014 at 03:16:10PM +0100, Justin Cormack wrote:
> > > I was under the impression that the ioctl-based interface for ipv6 is
> > > incomplete under Linux.
> > 
> > Probably "incomplete" in a sense that it can't do some special-purpose
> > stuff that most users don't need.
> 
> You cannot get the configured ipv6 addresses via SIOCGIFCONF. That is
> why musl needs to parse /proc for that.
> 
> ioctl-based interface for ipv4 is also "incomplete". You cannot get
> anything else than the primary address so current musl getifaddrs will
> not give you all configured ipv4 addresses.
> 
> > Busybox entirely avoids netlink, as
> > far as I can tell,
> 
> Not entirely. the iproute implementation in bb uses netlink, but you
> can of course disable that during configuration. A quick grep also
> indicate that busybox ifplugd uses netlink.
> 
> > and it's perfectly acceptable for setting up ipv6,
> > at least in simple setups. You don't even need busybox's iproute2
> > workalikes; ifconfig and route work fine.
> 
> And if you do that, then you cannot assign more than one ip addr on
> each interface. (you'll have to use alias, eth0:1 for that), but yes,
> thats no longer a simple setup.

OK, based on what you're saying, the ioctl interface is unable to
report the existence of interfaces that have only an ipv6 address
configured but no ipv4 addresses. If that's true, then yes, it's
unusable and we need a different approach, either /proc or netlink.
I'm happy to look at both approaches and see which is more efficient.

One advantage of netlink is that it can work before /proc is mounted.
However, it's documented that a fair number of interfaces in libc
depend on /proc, so unless you're going to audit programs for which
libc functions they use, it really makes sense to just make sure /proc
is mounted first. There's really no good reason for mounting /proc not
to be the first line of the init scripts. It also might be able to
make the netlink-based code smaller (in terms of what gets static
linked) than a proc-based version, since stdio/scanf is costly.

One advantage of /proc is that, on a non-linux system, you can just
drop static file in place with contents matching your machine's
configuration.

Rich

Re: if_nameindex/getifaddrs and dhcpcd issue

[Rich Felker]

On Wed, Apr 09, 2014 at 09:17:14AM +0200, u-igbb@aetey.se wrote:
> On Tue, Apr 08, 2014 at 06:41:12PM -0400, Rich Felker wrote:
> > As an aside, whether netlink is essential for a full-function modern
> > linux system is orthogonal to whether libc should be using it. A
> > hypothetical non-linux kernel providing linux syscall abi could
> > provide completely different net config utils.
> 
> (I can not help wondering why you call this hypothetical,
>  routinely using my "linux" binaries on *BSD.
> Oh wait, was it because mentioning demons' names is inappropriate? :)

Does if_nameindex "work" (modulo the issues with it) on BSD linux
emulation now, and would it fail if we switched to netlink or /proc
based? This is not necessarily a show-stopper but it would be nice if
we had a workaround, and it's something where we should have input
from users running musl binaries on BSD.

Rich

Re: if_nameindex/getifaddrs and dhcpcd issue

[Justin Cormack]

[-- Attachment #1: Type: text/plain, Size: 1172 bytes --]

On Apr 9, 2014 11:20 PM, "Rich Felker" <dalias@aerifal.cx> wrote:
>
> On Wed, Apr 09, 2014 at 09:17:14AM +0200, u-igbb@aetey.se wrote:
> > On Tue, Apr 08, 2014 at 06:41:12PM -0400, Rich Felker wrote:
> > > As an aside, whether netlink is essential for a full-function modern
> > > linux system is orthogonal to whether libc should be using it. A
> > > hypothetical non-linux kernel providing linux syscall abi could
> > > provide completely different net config utils.
> >
> > (I can not help wondering why you call this hypothetical,
> >  routinely using my "linux" binaries on *BSD.
> > Oh wait, was it because mentioning demons' names is inappropriate? :)
>
> Does if_nameindex "work" (modulo the issues with it) on BSD linux
> emulation now, and would it fail if we switched to netlink or /proc
> based? This is not necessarily a show-stopper but it would be nice if
> we had a workaround, and it's something where we should have input
> from users running musl binaries on BSD.

NetBSD has (optional) Linux /proc emulation. I will check the state of the
current emulation for the nameindex. There is no netlink emulation. As I
write the tests I get some influence...

[-- Attachment #2: Type: text/html, Size: 1516 bytes --]

Re: if_nameindex/getifaddrs and dhcpcd issue

[Rich Felker]

On Wed, Apr 09, 2014 at 05:02:22PM +0300, Timo Teras wrote:
> On Tue, 8 Apr 2014 19:08:07 +0300
> Timo Teras <timo.teras@iki.fi> wrote:
> 
> > > > I'm willing to write an alternative getifaddrs() and
> > > > if_nameindex() implementations using netlink. Perhaps let's see
> > > > how they end up?  
> > > 
> > > It wouldn't hurt; if they're not upstreamed they could still be used
> > > as a separate library for the one application which needs this
> > > functionality (dhcpcd).
> > 
> > Yeah. I'll play with this and see what I come up with. I'll also
> > delete the bad kernel #define's and try to do them a bit better - not
> > sure how well I succeed that at, though.
> 
> Posting here the current version of my patch as requested.
> 
> While it has some corner cases still that need cleaning, it's a good
> start. It implements the APIs, and is usable in read world.
> 
> From 4a82a84fb4fbfdae48f14bf86df0fd92086b7556 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Timo=20Ter=C3=A4s?= <timo.teras@iki.fi>
> Date: Tue, 8 Apr 2014 14:03:16 +0000
> Subject: [PATCH] reimplement if_nameindex and getifaddrs using netlink
> 
> ---
>  src/network/__netlink.c    |  68 ++++++++++
>  src/network/__netlink.h    | 143 ++++++++++++++++++++
>  src/network/getifaddrs.c   | 322 ++++++++++++++++++++++++---------------------
>  src/network/if_nameindex.c | 105 +++++++++------
>  4 files changed, 451 insertions(+), 187 deletions(-)
>  create mode 100644 src/network/__netlink.c
>  create mode 100644 src/network/__netlink.h
> 
> diff --git a/src/network/__netlink.c b/src/network/__netlink.c
> new file mode 100644
> index 0000000..d0c9fab
> --- /dev/null
> +++ b/src/network/__netlink.c
> @@ -0,0 +1,68 @@
> +#define _GNU_SOURCE
> +#include <errno.h>
> +#include <string.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <sys/socket.h>
> +#include <sys/param.h>
> +#include "__netlink.h"
> +
> +struct __netlink_handle {
> +	int fd;
> +	unsigned int seq;
> +	size_t bufsize;
> +};
> +
> +struct __netlink_handle *__netlink_open(int type)
> +{
> +	struct __netlink_handle *nh;
> +	int bufsize = getpagesize();
                      ^^^^^^^^^^^

getpagesize is a nonstandard function, not in the namespace that can
be used. sysconf could be used, but in musl it's preferable to use
PAGE_SIZE. On archs that don't define it, libc.h (which you can
include) defines a fallback to get the runtime value efficiently.

> +	/* required buffer size is MIN(8192,pagesize)-sizeof(struct skb_shared_info)
> +	 * the estimate for skb_shared_info size is conservative, but gives enough
> +	 * space to fit struct __netlink_handle including malloc overhead in one page . */
> +	if (bufsize > 8192) bufsize = 8192;

But in this case I would juse use 8192 as the fixed size.

> +	bufsize -= 128;
> +	nh = malloc(sizeof(struct __netlink_handle) + bufsize);

Then there's no need to malloc this, just use a normal automatic buffer.

> +	if (!nh) return 0;

And no need to check for failure.

> +	nh->fd = socket(PF_NETLINK, SOCK_RAW|SOCK_CLOEXEC, type);

socket is a cancellation point, but if_nameindex is not, so you need
to block cancellation. Note: this bug exists in the current code too.

> +	if (nh->fd < 0) { free(nh); return 0; }
> +	nh->seq = 1;
> +	nh->bufsize = bufsize;
> +	return nh;
> +}
> +
> +void __netlink_close(struct __netlink_handle *nh)
> +{
> +	close(nh->fd);
> +	free(nh);
> +}

Same for close, but here it's easy to just do
__syscall(SYS_close,nh->fd).

However, I think the whole idea of having a netlink handle that's
allocated and freed is ugly and inefficient. Just return a struct with
two members, seq and fd, by value.

> +int __netlink_enumerate(struct __netlink_handle *nh, int type, int (*cb)(void *ctx, struct nlmsghdr *h), void *ctx)
> +{
> +	struct nlmsghdr *h;
> +	void *buf = (void*)(nh+1);
> +	struct {
> +		struct nlmsghdr nlh;
> +		struct rtgenmsg g;
> +	} *req = buf;
> +	int r, ret = 0;
> +
> +	memset(req, 0, NETLINK_ALIGN(sizeof(*req)));
> +	req->nlh.nlmsg_len = sizeof(*req);
> +	req->nlh.nlmsg_type = type;
> +	req->nlh.nlmsg_flags = NLM_F_ROOT | NLM_F_MATCH | NLM_F_REQUEST;
> +	req->nlh.nlmsg_seq = nh->seq++;
> +	req->g.rtgen_family = AF_UNSPEC;
> +	r = send(nh->fd, req, sizeof(*req), 0);

send is also a cancellation point. There may be others I'm missing, so
if in doubt, all of these functions should just temporarily block and
restore cancellation state.

> +	if (r < 0) return r;
> +
> +	while (1) {
> +		r = recv(nh->fd, buf, nh->bufsize, MSG_DONTWAIT);
> +		if (r <= 0) return -1;
> +		for (h = (struct nlmsghdr*) buf; NLMSG_OK(h, (void*)((uint8_t*)buf+r)); h = NLMSG_NEXT(h)) {
> +			if (h->nlmsg_type == NLMSG_DONE) return ret;
> +			if (h->nlmsg_type == NLMSG_ERROR) return -1;
> +			if (!ret) ret = cb(ctx, h);
> +		}
> +	}
> +}
> diff --git a/src/network/__netlink.h b/src/network/__netlink.h
> new file mode 100644
> index 0000000..94728f3
> --- /dev/null
> +++ b/src/network/__netlink.h
> @@ -0,0 +1,143 @@
> +#include <stdint.h>
> +
> +/* linux/netlink.h */
> +
> +#define NETLINK_ROUTE 0
> +
> +struct nlmsghdr {
> +	uint32_t	nlmsg_len;
> +	uint16_t	nlmsg_type;
> +	uint16_t	nlmsg_flags;
> +	uint32_t	nlmsg_seq;
> +	uint32_t	nlmsg_pid;
> +};
> +
> +#define NLM_F_REQUEST	1
> +#define NLM_F_MULTI	2
> +#define NLM_F_ACK	4
> +#define NLM_F_ECHO	8
> +#define NLM_F_DUMP_INTR	16
> +
> +#define NLM_F_ROOT	0x100
> +#define NLM_F_MATCH	0x200
> +#define NLM_F_ATOMIC	0x400
> +#define NLM_F_DUMP	(NLM_F_ROOT|NLM_F_MATCH)
> +
> +#define NLMSG_NOOP	0x1
> +#define NLMSG_ERROR	0x2
> +#define NLMSG_DONE	0x3
> +#define NLMSG_OVERRUN	0x4
> +
> +/* linux/rtnetlink.h */
> +
> +#define RTM_GETLINK	18
> +#define RTM_GETADDR	22
> +
> +struct rtattr {
> +	unsigned short	rta_len;
> +	unsigned short	rta_type;
> +};
> +
> +struct rtgenmsg {
> +	unsigned char	rtgen_family;
> +};
> +
> +struct ifinfomsg {
> +	unsigned char	ifi_family;
> +	unsigned char	__ifi_pad;
> +	unsigned short	ifi_type;
> +	int		ifi_index;
> +	unsigned	ifi_flags;
> +	unsigned	ifi_change;
> +};
> +
> +/* linux/if_link.h */
> +
> +enum {
> +	IFLA_UNSPEC,
> +	IFLA_ADDRESS,
> +	IFLA_BROADCAST,
> +	IFLA_IFNAME,
> +	IFLA_MTU,
> +	IFLA_LINK,
> +	IFLA_QDISC,
> +	IFLA_STATS,
> +	IFLA_COST,
> +	IFLA_PRIORITY,
> +	IFLA_MASTER,
> +	IFLA_WIRELESS,
> +	IFLA_PROTINFO,
> +	IFLA_TXQLEN,
> +	IFLA_MAP,
> +	IFLA_WEIGHT,
> +	IFLA_OPERSTATE,
> +	IFLA_LINKMODE,
> +	IFLA_LINKINFO,
> +	IFLA_NET_NS_PID,
> +	IFLA_IFALIAS,
> +	IFLA_NUM_VF,
> +	IFLA_VFINFO_LIST,
> +	IFLA_STATS64,
> +	IFLA_VF_PORTS,
> +	IFLA_PORT_SELF,
> +	IFLA_AF_SPEC,
> +	IFLA_GROUP,
> +	IFLA_NET_NS_FD,
> +	IFLA_EXT_MASK,
> +	IFLA_PROMISCUITY,
> +	IFLA_NUM_TX_QUEUES,
> +	IFLA_NUM_RX_QUEUES,
> +	IFLA_CARRIER,
> +	IFLA_PHYS_PORT_ID,
> +	__IFLA_MAX
> +};
> +
> +/* linux/if_addr.h */
> +
> +struct ifaddrmsg {
> +	uint8_t		ifa_family;
> +	uint8_t		ifa_prefixlen;
> +	uint8_t		ifa_flags;
> +	uint8_t		ifa_scope;
> +	uint32_t	ifa_index;
> +};
> +
> +enum {
> +	IFA_UNSPEC,
> +	IFA_ADDRESS,
> +	IFA_LOCAL,
> +	IFA_LABEL,
> +	IFA_BROADCAST,
> +	IFA_ANYCAST,
> +	IFA_CACHEINFO,
> +	IFA_MULTICAST,
> +	__IFA_MAX
> +};
> +
> +/* musl */
> +
> +#define NETLINK_ALIGN(len)	(((len)+3) & ~3)
> +#define NLMSG_DATA(nlh)		((void*)((char*)(nlh)+NETLINK_ALIGN(sizeof(struct nlmsghdr))))
> +#define NLMSG_DATALEN(nlh)	((nlh)->nlmsg_len-NETLINK_ALIGN(sizeof(struct nlmsghdr)))
> +#define NLMSG_DATAEND(nlh)	((char*)(nlh)+(nlh)->nlmsg_len)
> +#define NLMSG_NEXT(nlh)		(struct nlmsghdr*)((char*)(nlh)+NETLINK_ALIGN((nlh)->nlmsg_len))
> +#define NLMSG_OK(nlh,end)	(NLMSG_DATA(nlh) <= (end) && \
> +				 (nlh)->nlmsg_len >= sizeof(struct nlmsghdr) && \
> +				 (void*)NLMSG_NEXT(nlh) <= (end))
> +
> +#define RTA_DATA(rta)		((void*)((char*)(rta)+NETLINK_ALIGN(sizeof(struct rtattr))))
> +#define RTA_DATALEN(rta)	((rta)->rta_len-NETLINK_ALIGN(sizeof(struct rtattr)))
> +#define RTA_DATAEND(rta)	((char*)(rta)+(rta)->rta_len)
> +#define RTA_NEXT(rta)		(struct rtattr*)((char*)(rta)+NETLINK_ALIGN((rta)->rta_len))
> +#define RTA_OK(rta,end)		(RTA_DATA(rta) <= (void*)(end) && \
> +				 (rta)->rta_len >= sizeof(struct rtattr) && \
> +				 (void*)RTA_NEXT(rta) <= (void*)(end))
> +
> +#define NLMSG_RTA(nlh,len)	((void*)((char*)(nlh)+NETLINK_ALIGN(sizeof(struct nlmsghdr))+NETLINK_ALIGN(len)))
> +#define NLMSG_RTAOK(rta,nlh)	RTA_OK(rta,NLMSG_DATAEND(nlh))
> +
> +struct __netlink_handle;
> +
> +struct __netlink_handle *__netlink_open(int type);
> +void __netlink_close(struct __netlink_handle *h);
> +int __netlink_enumerate(struct __netlink_handle *h, int type, int (*cb)(void *ctx, struct nlmsghdr *h), void *ctx);
> diff --git a/src/network/getifaddrs.c b/src/network/getifaddrs.c
> index 5a94cc7..5b1ebe7 100644
> --- a/src/network/getifaddrs.c
> +++ b/src/network/getifaddrs.c
> @@ -1,181 +1,209 @@
> -/* (C) 2013 John Spencer. released under musl's standard MIT license. */
> -#undef _GNU_SOURCE
>  #define _GNU_SOURCE
> -#include <ifaddrs.h>
> -#include <stdlib.h>
> -#include <net/if.h> /* IFNAMSIZ, ifreq, ifconf */
> -#include <stdio.h>
> -#include <ctype.h>
> -#include <string.h>
>  #include <errno.h>
> -#include <arpa/inet.h> /* inet_pton */
> +#include <string.h>
> +#include <stdlib.h>
>  #include <unistd.h>
> -#include <sys/ioctl.h>
> -#include <sys/socket.h>
> +#include <ifaddrs.h>
> +#include <net/if.h>
> +#include "__netlink.h"
>  
> -typedef union {
> -	struct sockaddr_in6 v6;
> +/* getifaddrs() uses PF_PACKET to relay hardware addresses.
> + * But Infiniband socket address length is longer, so use this hack
> + * (like glibc) to return it anyway. */

This comment suggests a hack is going on...

> +struct sockaddr_ll_hack {
> +	unsigned short sll_family, sll_protocol;
> +	int sll_ifindex;
> +	unsigned short sll_hatype;
> +	unsigned char sll_pkttype, sll_halen;
> +	unsigned char sll_addr[24];
> +};

...but it's not at all obvious if/where there's any hack in this
structure.

I haven't reviewed the rest yet because it looked like I'd need to
actually understand what the code is doing rather than just evaluate
general things like the above. :)

Rich

Re: if_nameindex/getifaddrs and dhcpcd issue

[u-igbb]

On Wed, Apr 09, 2014 at 06:20:41PM -0400, Rich Felker wrote:
> Does if_nameindex "work" (modulo the issues with it) on BSD linux
> emulation now, and would it fail if we switched to netlink or /proc
> based? This is not necessarily a show-stopper but it would be nice if
> we had a workaround, and it's something where we should have input
> from users running musl binaries on BSD.

I'm not aware of which of the binaries may be calling if_nameindex. The
ones which are being used continuously are Samba and Coda servers running
on FreeBSD (for the sake of ZFS), their current production versions are
still linked to uclibc.

Occationally I run "everything" i.e. a whole user environment in the
form of Linux binaries on NetBSD but it will take some time before this
happens and I test if_nameindex there.

Rune

Re: if_nameindex/getifaddrs and dhcpcd issue

[Rich Felker]

On Thu, Apr 10, 2014 at 09:40:22AM +0200, u-igbb@aetey.se wrote:
> On Wed, Apr 09, 2014 at 06:20:41PM -0400, Rich Felker wrote:
> > Does if_nameindex "work" (modulo the issues with it) on BSD linux
> > emulation now, and would it fail if we switched to netlink or /proc
> > based? This is not necessarily a show-stopper but it would be nice if
> > we had a workaround, and it's something where we should have input
> > from users running musl binaries on BSD.
> 
> I'm not aware of which of the binaries may be calling if_nameindex. The
> ones which are being used continuously are Samba and Coda servers running
> on FreeBSD (for the sake of ZFS), their current production versions are
> still linked to uclibc.
> 
> Occationally I run "everything" i.e. a whole user environment in the
> form of Linux binaries on NetBSD but it will take some time before this
> happens and I test if_nameindex there.

I think these functions are mainly useful for network-configuration
utilities, not general user-facing apps. But I do want to make things
work for users who want to run a complete userspace of musl-linked
linux binaries on a BSD kernel.

At the moment I'm leaning towards a netlink-based approach despite my
moderate disgust with the netlink API simply because the alternative
(/proc) does not seem as possible as I thought it was; I can't see any
way to get a good parsable list of devices, including unconfigured
devices and aliases/multi-addresses, from /proc. If somebody knows how
and has a proposal for how it could be done, please speak up.

Rich