* if_nameindex/getifaddrs and dhcpcd issue @ 2014-04-08 9:11 Natanael Copa 2014-04-08 10:07 ` Justin Cormack 0 siblings, 1 reply; 26+ messages in thread From: Natanael Copa @ 2014-04-08 9:11 UTC (permalink / raw) To: musl 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: if_nameindex/getifaddrs and dhcpcd issue 2014-04-08 9:11 if_nameindex/getifaddrs and dhcpcd issue Natanael Copa @ 2014-04-08 10:07 ` Justin Cormack 2014-04-08 12:25 ` Timo Teras ` (3 more replies) 0 siblings, 4 replies; 26+ messages in thread From: Justin Cormack @ 2014-04-08 10:07 UTC (permalink / raw) To: musl 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: if_nameindex/getifaddrs and dhcpcd issue 2014-04-08 10:07 ` Justin Cormack @ 2014-04-08 12:25 ` Timo Teras 2014-04-08 14:23 ` Natanael Copa 2014-04-08 15:45 ` Rich Felker 2014-04-08 12:54 ` Szabolcs Nagy ` (2 subsequent siblings) 3 siblings, 2 replies; 26+ messages in thread From: Timo Teras @ 2014-04-08 12:25 UTC (permalink / raw) To: musl; +Cc: justin 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: if_nameindex/getifaddrs and dhcpcd issue 2014-04-08 12:25 ` Timo Teras @ 2014-04-08 14:23 ` Natanael Copa 2014-04-08 15:45 ` Rich Felker 1 sibling, 0 replies; 26+ messages in thread From: Natanael Copa @ 2014-04-08 14:23 UTC (permalink / raw) To: musl; +Cc: timo.teras, justin 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: if_nameindex/getifaddrs and dhcpcd issue 2014-04-08 12:25 ` Timo Teras 2014-04-08 14:23 ` Natanael Copa @ 2014-04-08 15:45 ` Rich Felker 2014-04-08 16:08 ` Timo Teras 2014-04-09 7:55 ` Natanael Copa 1 sibling, 2 replies; 26+ messages in thread From: Rich Felker @ 2014-04-08 15:45 UTC (permalink / raw) To: musl; +Cc: justin 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: if_nameindex/getifaddrs and dhcpcd issue 2014-04-08 15:45 ` Rich Felker @ 2014-04-08 16:08 ` Timo Teras 2014-04-08 16:19 ` Justin Cormack 2014-04-09 14:02 ` Timo Teras 2014-04-09 7:55 ` Natanael Copa 1 sibling, 2 replies; 26+ messages in thread From: Timo Teras @ 2014-04-08 16:08 UTC (permalink / raw) To: musl; +Cc: dalias, justin 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. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: if_nameindex/getifaddrs and dhcpcd issue 2014-04-08 16:08 ` Timo Teras @ 2014-04-08 16:19 ` Justin Cormack 2014-04-08 22:41 ` Rich Felker 2014-04-09 14:02 ` Timo Teras 1 sibling, 1 reply; 26+ messages in thread From: Justin Cormack @ 2014-04-08 16:19 UTC (permalink / raw) To: Timo Teras; +Cc: musl 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: if_nameindex/getifaddrs and dhcpcd issue 2014-04-08 16:19 ` Justin Cormack @ 2014-04-08 22:41 ` Rich Felker 2014-04-09 7:17 ` u-igbb 0 siblings, 1 reply; 26+ messages in thread From: Rich Felker @ 2014-04-08 22:41 UTC (permalink / raw) To: musl; +Cc: Timo Teras 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: if_nameindex/getifaddrs and dhcpcd issue 2014-04-08 22:41 ` Rich Felker @ 2014-04-09 7:17 ` u-igbb 2014-04-09 22:20 ` Rich Felker 0 siblings, 1 reply; 26+ messages in thread From: u-igbb @ 2014-04-09 7:17 UTC (permalink / raw) To: musl 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: if_nameindex/getifaddrs and dhcpcd issue 2014-04-09 7:17 ` u-igbb @ 2014-04-09 22:20 ` Rich Felker 2014-04-09 22:32 ` Justin Cormack 2014-04-10 7:40 ` u-igbb 0 siblings, 2 replies; 26+ messages in thread From: Rich Felker @ 2014-04-09 22:20 UTC (permalink / raw) To: musl 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: if_nameindex/getifaddrs and dhcpcd issue 2014-04-09 22:20 ` Rich Felker @ 2014-04-09 22:32 ` Justin Cormack 2014-04-10 7:40 ` u-igbb 1 sibling, 0 replies; 26+ messages in thread From: Justin Cormack @ 2014-04-09 22:32 UTC (permalink / raw) To: musl [-- 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 --] ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: if_nameindex/getifaddrs and dhcpcd issue 2014-04-09 22:20 ` Rich Felker 2014-04-09 22:32 ` Justin Cormack @ 2014-04-10 7:40 ` u-igbb 2014-04-10 7:52 ` Rich Felker 1 sibling, 1 reply; 26+ messages in thread From: u-igbb @ 2014-04-10 7:40 UTC (permalink / raw) To: musl 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: if_nameindex/getifaddrs and dhcpcd issue 2014-04-10 7:40 ` u-igbb @ 2014-04-10 7:52 ` Rich Felker 0 siblings, 0 replies; 26+ messages in thread From: Rich Felker @ 2014-04-10 7:52 UTC (permalink / raw) To: musl 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: if_nameindex/getifaddrs and dhcpcd issue 2014-04-08 16:08 ` Timo Teras 2014-04-08 16:19 ` Justin Cormack @ 2014-04-09 14:02 ` Timo Teras 2014-04-10 0:55 ` Rich Felker 1 sibling, 1 reply; 26+ messages in thread From: Timo Teras @ 2014-04-09 14:02 UTC (permalink / raw) To: Timo Teras; +Cc: musl, dalias, justin 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: if_nameindex/getifaddrs and dhcpcd issue 2014-04-09 14:02 ` Timo Teras @ 2014-04-10 0:55 ` Rich Felker 0 siblings, 0 replies; 26+ messages in thread From: Rich Felker @ 2014-04-10 0:55 UTC (permalink / raw) To: musl; +Cc: Timo Teras, justin 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: if_nameindex/getifaddrs and dhcpcd issue 2014-04-08 15:45 ` Rich Felker 2014-04-08 16:08 ` Timo Teras @ 2014-04-09 7:55 ` Natanael Copa 1 sibling, 0 replies; 26+ messages in thread From: Natanael Copa @ 2014-04-09 7:55 UTC (permalink / raw) To: musl; +Cc: dalias, justin 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: if_nameindex/getifaddrs and dhcpcd issue 2014-04-08 10:07 ` Justin Cormack 2014-04-08 12:25 ` Timo Teras @ 2014-04-08 12:54 ` Szabolcs Nagy 2014-04-08 13:42 ` Rich Felker 2014-04-08 14:27 ` Natanael Copa 3 siblings, 0 replies; 26+ messages in thread From: Szabolcs Nagy @ 2014-04-08 12:54 UTC (permalink / raw) To: musl * 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.. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: if_nameindex/getifaddrs and dhcpcd issue 2014-04-08 10:07 ` Justin Cormack 2014-04-08 12:25 ` Timo Teras 2014-04-08 12:54 ` Szabolcs Nagy @ 2014-04-08 13:42 ` Rich Felker 2014-04-08 14:16 ` Justin Cormack 2014-04-08 21:16 ` Natanael Copa 2014-04-08 14:27 ` Natanael Copa 3 siblings, 2 replies; 26+ messages in thread From: Rich Felker @ 2014-04-08 13:42 UTC (permalink / raw) To: musl 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: if_nameindex/getifaddrs and dhcpcd issue 2014-04-08 13:42 ` Rich Felker @ 2014-04-08 14:16 ` Justin Cormack 2014-04-08 15:38 ` Rich Felker 2014-04-08 21:16 ` Natanael Copa 1 sibling, 1 reply; 26+ messages in thread From: Justin Cormack @ 2014-04-08 14:16 UTC (permalink / raw) To: musl 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: if_nameindex/getifaddrs and dhcpcd issue 2014-04-08 14:16 ` Justin Cormack @ 2014-04-08 15:38 ` Rich Felker 2014-04-09 7:13 ` Natanael Copa 0 siblings, 1 reply; 26+ messages in thread From: Rich Felker @ 2014-04-08 15:38 UTC (permalink / raw) To: musl 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: if_nameindex/getifaddrs and dhcpcd issue 2014-04-08 15:38 ` Rich Felker @ 2014-04-09 7:13 ` Natanael Copa 2014-04-09 22:18 ` Rich Felker 0 siblings, 1 reply; 26+ messages in thread From: Natanael Copa @ 2014-04-09 7:13 UTC (permalink / raw) To: musl; +Cc: dalias 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: if_nameindex/getifaddrs and dhcpcd issue 2014-04-09 7:13 ` Natanael Copa @ 2014-04-09 22:18 ` Rich Felker 0 siblings, 0 replies; 26+ messages in thread From: Rich Felker @ 2014-04-09 22:18 UTC (permalink / raw) To: musl 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: if_nameindex/getifaddrs and dhcpcd issue 2014-04-08 13:42 ` Rich Felker 2014-04-08 14:16 ` Justin Cormack @ 2014-04-08 21:16 ` Natanael Copa 2014-04-08 21:30 ` Justin Cormack 1 sibling, 1 reply; 26+ messages in thread From: Natanael Copa @ 2014-04-08 21:16 UTC (permalink / raw) To: musl; +Cc: dalias 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: if_nameindex/getifaddrs and dhcpcd issue 2014-04-08 21:16 ` Natanael Copa @ 2014-04-08 21:30 ` Justin Cormack 2014-04-08 22:59 ` Rich Felker 0 siblings, 1 reply; 26+ messages in thread From: Justin Cormack @ 2014-04-08 21:30 UTC (permalink / raw) To: musl; +Cc: Rich Felker 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: if_nameindex/getifaddrs and dhcpcd issue 2014-04-08 21:30 ` Justin Cormack @ 2014-04-08 22:59 ` Rich Felker 0 siblings, 0 replies; 26+ messages in thread From: Rich Felker @ 2014-04-08 22:59 UTC (permalink / raw) To: musl 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: if_nameindex/getifaddrs and dhcpcd issue 2014-04-08 10:07 ` Justin Cormack ` (2 preceding siblings ...) 2014-04-08 13:42 ` Rich Felker @ 2014-04-08 14:27 ` Natanael Copa 3 siblings, 0 replies; 26+ messages in thread From: Natanael Copa @ 2014-04-08 14:27 UTC (permalink / raw) To: musl; +Cc: justin 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2014-04-10 7:52 UTC | newest] Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-04-08 9:11 if_nameindex/getifaddrs and dhcpcd issue Natanael Copa 2014-04-08 10:07 ` Justin Cormack 2014-04-08 12:25 ` Timo Teras 2014-04-08 14:23 ` Natanael Copa 2014-04-08 15:45 ` Rich Felker 2014-04-08 16:08 ` Timo Teras 2014-04-08 16:19 ` Justin Cormack 2014-04-08 22:41 ` Rich Felker 2014-04-09 7:17 ` u-igbb 2014-04-09 22:20 ` Rich Felker 2014-04-09 22:32 ` Justin Cormack 2014-04-10 7:40 ` u-igbb 2014-04-10 7:52 ` Rich Felker 2014-04-09 14:02 ` Timo Teras 2014-04-10 0:55 ` Rich Felker 2014-04-09 7:55 ` Natanael Copa 2014-04-08 12:54 ` Szabolcs Nagy 2014-04-08 13:42 ` Rich Felker 2014-04-08 14:16 ` Justin Cormack 2014-04-08 15:38 ` Rich Felker 2014-04-09 7:13 ` Natanael Copa 2014-04-09 22:18 ` Rich Felker 2014-04-08 21:16 ` Natanael Copa 2014-04-08 21:30 ` Justin Cormack 2014-04-08 22:59 ` Rich Felker 2014-04-08 14:27 ` Natanael Copa
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/musl/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).