getaddrinfo(3) / AI_ADDRCONFIG

Submitted by Rich Felker on July 11, 2018, 4:44 p.m.

Details

Message ID 20180711164417.GX1392@brightrain.aerifal.cx
State New
Series "getaddrinfo(3) / AI_ADDRCONFIG"
Headers show

Commit Message

Rich Felker July 11, 2018, 4:44 p.m.
On Wed, Jul 11, 2018 at 06:12:31AM -0400, Christopher Friedt wrote:
> On Tue, Jul 10, 2018 at 9:26 PM Rich Felker <dalias@libc.org> wrote:
> > Pulling in large amounts of additional code and O(n) runtime cost
> 
> Latest patch [1] addresses
> 
> 1) not ignoring loopback
> 2) using routability of udp packets vs O(n) lookup on network interfaces
> 
> Any other concerns, Rich?

You seem to have deleted the original patch and replaced it with a new
version. My first concern is *please* send all patches as attachments
to the list, not transient links like github or pastebins. Even if it
were still there now, it would likely not be there 5 years later when
someone is reading list archives. I'm attaching your v2 patch here now
for reference.

With that said, it still makes sprawling changes and intraduces a
gratuitous new file with external interface for something that
fundamentally takes only a few lines in one place and no external
interface at all. Formatting is also inconsistent with musl (spaces
after opening and before closing paren, etc.). And addition of the
nonstandard EAI_NODATA is an independent change that, if it makes
sense at all, needs to be discussed separately, and would need
corresponding changes elsewhere (e.g. gai_strerror).

Rich

Patch hide | download patch | download mbox

diff --git a/include/netdb.h b/include/netdb.h
index adde2c5e..73d850f6 100644
--- a/include/netdb.h
+++ b/include/netdb.h
@@ -44,6 +44,7 @@  struct addrinfo {
 #define EAI_NONAME     -2
 #define EAI_AGAIN      -3
 #define EAI_FAIL       -4
+#define EAI_NODATA     -5
 #define EAI_FAMILY     -6
 #define EAI_SOCKTYPE   -7
 #define EAI_SERVICE    -8
diff --git a/src/network/getaddrinfo.c b/src/network/getaddrinfo.c
index b9439f77..2bfba18f 100644
--- a/src/network/getaddrinfo.c
+++ b/src/network/getaddrinfo.c
@@ -19,6 +19,7 @@  int getaddrinfo(const char *restrict host, const char *restrict serv, const stru
 			struct sockaddr_in6 sin6;
 		} sa;
 	} *out;
+	struct addrconfig addrconfig;
 
 	if (!host && !serv) return EAI_NONAME;
 
@@ -33,6 +34,10 @@  int getaddrinfo(const char *restrict host, const char *restrict serv, const stru
 		if ((flags & mask) != flags)
 			return EAI_BADFLAGS;
 
+		if (flags & AI_ADDRCONFIG) {
+			__lookup_addrconfig(&addrconfig);
+		}
+
 		switch (family) {
 		case AF_INET:
 		case AF_INET6:
@@ -61,30 +66,43 @@  int getaddrinfo(const char *restrict host, const char *restrict serv, const stru
 		outcanon = 0;
 	}
 
-	for (k=i=0; i<naddrs; i++) for (j=0; j<nservs; j++, k++) {
-		out[k].ai = (struct addrinfo){
-			.ai_family = addrs[i].family,
-			.ai_socktype = ports[j].socktype,
-			.ai_protocol = ports[j].proto,
-			.ai_addrlen = addrs[i].family == AF_INET
-				? sizeof(struct sockaddr_in)
-				: sizeof(struct sockaddr_in6),
-			.ai_addr = (void *)&out[k].sa,
-			.ai_canonname = outcanon,
-			.ai_next = &out[k+1].ai };
+	for (k=i=0; i<naddrs; i++) for (j=0; j<nservs; j++) {
 		switch (addrs[i].family) {
 		case AF_INET:
+			if ((flags & AI_ADDRCONFIG) && !addrconfig.af_inet) {
+				nais--;
+				continue;
+			}
 			out[k].sa.sin.sin_family = AF_INET;
 			out[k].sa.sin.sin_port = htons(ports[j].port);
 			memcpy(&out[k].sa.sin.sin_addr, &addrs[i].addr, 4);
 			break;
 		case AF_INET6:
+			if ((flags & AI_ADDRCONFIG ) && !addrconfig.af_inet6) {
+				nais--;
+				continue;
+			}
 			out[k].sa.sin6.sin6_family = AF_INET6;
 			out[k].sa.sin6.sin6_port = htons(ports[j].port);
 			out[k].sa.sin6.sin6_scope_id = addrs[i].scopeid;
 			memcpy(&out[k].sa.sin6.sin6_addr, &addrs[i].addr, 16);
-			break;			
+			break;
 		}
+		out[k].ai = (struct addrinfo){
+			.ai_family = addrs[i].family,
+			.ai_socktype = ports[j].socktype,
+			.ai_protocol = ports[j].proto,
+			.ai_addrlen = addrs[i].family == AF_INET
+				? sizeof(struct sockaddr_in)
+				: sizeof(struct sockaddr_in6),
+			.ai_addr = (void *)&out[k].sa,
+			.ai_canonname = outcanon,
+			.ai_next = &out[k+1].ai };
+		k++;
+	}
+	if ( nais < 1 ) {
+		free( out );
+		return EAI_NODATA;
 	}
 	out[nais-1].ai.ai_next = 0;
 	*res = &out->ai;
diff --git a/src/network/lookup.h b/src/network/lookup.h
index 0468edbc..32afd545 100644
--- a/src/network/lookup.h
+++ b/src/network/lookup.h
@@ -1,6 +1,7 @@ 
 #ifndef LOOKUP_H
 #define LOOKUP_H
 
+#include <stdbool.h>
 #include <stdint.h>
 #include <stddef.h>
 
@@ -30,9 +31,15 @@  struct resolvconf {
 #define MAXADDRS 48
 #define MAXSERVS 2
 
+struct addrconfig {
+	bool af_inet  : 1;
+	bool af_inet6 : 1;
+};
+
 int __lookup_serv(struct service buf[static MAXSERVS], const char *name, int proto, int socktype, int flags);
 int __lookup_name(struct address buf[static MAXADDRS], char canon[static 256], const char *name, int family, int flags);
 int __lookup_ipliteral(struct address buf[static 1], const char *name, int family);
+void __lookup_addrconfig( struct addrconfig *cfg );
 
 int __get_resolv_conf(struct resolvconf *, char *, size_t);
 
diff --git a/src/network/lookup_addrconfig.c b/src/network/lookup_addrconfig.c
new file mode 100644
index 00000000..427d9299
--- /dev/null
+++ b/src/network/lookup_addrconfig.c
@@ -0,0 +1,40 @@ 
+#include "lookup.h"
+
+#include <arpa/inet.h>
+#include <netinet/in.h>
+#include <sys/types.h>
+#include <sys/socket.h>
+#include <unistd.h>
+
+void __lookup_addrconfig( struct addrconfig *cfg ) {
+	int r;
+	int fd;
+
+	struct sockaddr_in sai = {
+		.sin_family = AF_INET,
+		.sin_port = htons( 42 ),
+		.sin_addr.s_addr = INADDR_LOOPBACK,
+	};
+	cfg->af_inet = false;
+	r = socket( AF_INET, SOCK_DGRAM, 0 );
+	if ( -1 != r ) {
+		fd = r;
+		r = connect( fd, (struct sockaddr *) & sai, sizeof( sai ) );
+		cfg->af_inet = 0 == r;
+		close( fd );
+	}
+
+	struct sockaddr_in6 sai6 = {
+		.sin6_family = AF_INET6,
+		.sin6_port = htons( 42 ),
+		.sin6_addr = IN6ADDR_LOOPBACK_INIT,
+	};
+	cfg->af_inet6 = false;
+	r = socket( AF_INET6, SOCK_DGRAM, 0 );
+	if ( -1 != r ) {
+		fd = r;
+		r = connect( fd, (struct sockaddr *) & sai6, sizeof( sai6 ) );
+		cfg->af_inet6 = 0 == r;
+		close( fd );
+	}
+}

Comments

Christopher Friedt July 11, 2018, 4:50 p.m.
On Wed, Jul 11, 2018, 12:44 PM Rich Felker, <dalias@libc.org> wrote:

> On Wed, Jul 11, 2018 at 06:12:31AM -0400, Christopher Friedt wrote:
> > On Tue, Jul 10, 2018 at 9:26 PM Rich Felker <dalias@libc.org> wrote:
> > > Pulling in large amounts of additional code and O(n) runtime cost
> >
> > Latest patch [1] addresses
> >
> > 1) not ignoring loopback
> > 2) using routability of udp packets vs O(n) lookup on network interfaces
> >
> > Any other concerns, Rich?
>
> You seem to have deleted the original patch and replaced it with a new
>


Some lists prefer patches to be inline. I wasn't sure if you wanted the
attachment. That clarifies that.

With that said, it still makes sprawling changes and intraduces a
> gratuitous new file with external interface for something that
> fundamentally takes only a few lines in one place and no external
> interface at all. Formatting is also


I'll put it directly inside of getaddrinfo.

inconsistent with musl (spaces
> after opening and before closing paren, etc.).


Ok, will reformat.

And addition of the
> nonstandard EAI_NODATA is an independent change that, if it makes
> sense at all, needs to be discussed


Right. What error would you prefer?

C
Rich Felker July 11, 2018, 5 p.m.
On Wed, Jul 11, 2018 at 12:50:53PM -0400, Christopher Friedt wrote:
> On Wed, Jul 11, 2018, 12:44 PM Rich Felker, <dalias@libc.org> wrote:
> 
> > On Wed, Jul 11, 2018 at 06:12:31AM -0400, Christopher Friedt wrote:
> > > On Tue, Jul 10, 2018 at 9:26 PM Rich Felker <dalias@libc.org> wrote:
> > > > Pulling in large amounts of additional code and O(n) runtime cost
> > >
> > > Latest patch [1] addresses
> > >
> > > 1) not ignoring loopback
> > > 2) using routability of udp packets vs O(n) lookup on network interfaces
> > >
> > > Any other concerns, Rich?
> >
> > You seem to have deleted the original patch and replaced it with a new
> >
> 
> 
> Some lists prefer patches to be inline. I wasn't sure if you wanted the
> attachment. That clarifies that.
> 
> With that said, it still makes sprawling changes and intraduces a
> > gratuitous new file with external interface for something that
> > fundamentally takes only a few lines in one place and no external
> > interface at all. Formatting is also
> 
> 
> I'll put it directly inside of getaddrinfo.

It could probably go inside __lookup_name, but maybe in getaddrinfo is
better since that would avoid linking it for gethostbyname, etc.
(which don't need it).

> inconsistent with musl (spaces
> > after opening and before closing paren, etc.).
> 
> 
> Ok, will reformat.
> 
> And addition of the
> > nonstandard EAI_NODATA is an independent change that, if it makes
> > sense at all, needs to be discussed
> 
> 
> Right. What error would you prefer?

I think the one mandated by POSIX is EAI_NONAME ("The name does not
resolve for the supplied parameters").

Rich
Christopher Friedt July 12, 2018, 1:20 a.m.
On Wed, Jul 11, 2018 at 1:00 PM Rich Felker <dalias@libc.org> wrote:
> It could probably go inside __lookup_name, but maybe in getaddrinfo is
> better since that would avoid linking it for gethostbyname, etc.
> (which don't need it).

Done

> > inconsistent with musl (spaces
> > > after opening and before closing paren, etc.).

Done

> I think the one mandated by POSIX is EAI_NONAME ("The name does not
> resolve for the supplied parameters").

Done