- The v6 BSD socket method was updated to use a single UDP BSD socket

no matter how many interfaces are involved, differentiating the
  interfaces the packets were received on by the interface index supplied
  by the OS.
- The relay agent no longer listens to the All DHCP Servers Multicast
  address.
- A bug was fixed in data_string_sprintfa() where va_start was only called
  once for two invocations of vsprintf() variants.
  [ISC-Bugs #17610b]

RELNOTES

@@ -136,6 +136,17 @@ work on other platforms. Please report any problems and suggested fixes to
 - Compliation warnings on GCC 4.3 relating to bootp source address
   selection were repaired.
 
+- The v6 BSD socket method was updated to use a single UDP BSD socket
+  no matter how many interfaces are involved, differentiating the
+  interfaces the packets were received on by the interface index supplied
+  by the OS.
+
+- The relay agent no longer listens to the All DHCP Servers Multicast
+  address.
+
+- A bug was fixed in data_string_sprintfa() where va_start was only called
+  once for two invocations of vsprintf() variants.
+
 			Changes since 4.0.0 (new features)
 
 - Added DHCPv6 rapid commit support.

common/discover.c

@@ -1265,6 +1265,17 @@ discover_interfaces(int state) {
 		if (status != ISC_R_SUCCESS)
 			log_fatal ("Can't register I/O handle for %s: %s",
 				   tmp -> name, isc_result_totext (status));
+
+#if defined(DHCPv6)
+		/* Only register the first interface for V6, since they all
+		 * use the same socket.  XXX: This has some messy side
+		 * effects if we start dynamically adding and removing
+		 * interfaces, but we're well beyond that point in terms of
+		 * mess.
+		 */
+		if (local_family == AF_INET6)
+			break;
+#endif
 	}
 
 	if (state == DISCOVER_SERVER && wifcount == 0) {
@@ -1395,6 +1406,7 @@ got_one_v6(omapi_object_t *h) {
 	char buf[65536];	/* maximum size for a UDP packet is 65536 */
 	struct interface_info *ip;
 	int is_unicast;
+	unsigned int if_idx = 0;
 
 	if (h->type != dhcp_type_interface) {
 		return ISC_R_INVALIDARG;
@@ -1402,12 +1414,16 @@ got_one_v6(omapi_object_t *h) {
 	ip = (struct interface_info *)h;
 
 	result = receive_packet6(ip, (unsigned char *)buf, sizeof(buf),
-				 &from, &to);
+				 &from, &to, &if_idx);
 	if (result < 0) {
 		log_error("receive_packet6() failed on %s: %m", ip->name);
 		return ISC_R_UNEXPECTED;
 	}
 
+	/* 0 is 'any' interface. */
+	if (if_idx == 0)
+		return ISC_R_NOTFOUND;
+
 	if (dhcpv6_packet_handler != NULL) {
 		/*
 		 * If a packet is not multicast, we assume it is unicast.
@@ -1421,6 +1437,13 @@ got_one_v6(omapi_object_t *h) {
 		ifrom.len = 16;
 		memcpy(ifrom.iabuf, &from.sin6_addr, ifrom.len);
 
+		/* Seek forward to find the matching source interface. */
+		while ((ip != NULL) && (if_nametoindex(ip->name) != if_idx))
+			ip = ip->next;
+
+		if (ip == NULL)
+			return ISC_R_NOTFOUND;
+
 		(*dhcpv6_packet_handler)(ip, buf, 
 					 result, from.sin6_port, 
 					 &ifrom, is_unicast);

common/socket.c

@@ -54,6 +54,17 @@
 # endif
 #endif
 
+#if defined(DHCPv6)
+/*
+ * XXX: this is gross.  we need to go back and overhaul the API for socket
+ * handling.
+ */
+static unsigned int global_v6_socket_references = 0;
+static int global_v6_socket = -1;
+
+static void if_register_multicast(struct interface_info *info);
+#endif
+
 /*
  * If we can't bind() to a specific interface, then we can only have
  * a single socket. This variable insures that we don't try to listen
@@ -95,43 +106,11 @@ void if_reinitialize_receive (info)
 #if defined (USE_SOCKET_SEND) || \
 	defined (USE_SOCKET_RECEIVE) || \
 		defined (USE_SOCKET_FALLBACK)
-#ifdef DHCPv6
-/* Get the best (i.e., global or at least site-local) address
-   of the interface. */
-static isc_result_t
-get_ifaddr6(struct interface_info *info, struct in6_addr *ifaddr6) {
-	int i;
-	struct in6_addr *a, *ba = NULL;
-
-	for (i = 0; i < info->v6address_count; i++) {
-		a = &info->v6addresses[i];
-		if (IN6_IS_ADDR_UNSPECIFIED(a) ||
-		    IN6_IS_ADDR_LOOPBACK(a) ||
-		    IN6_IS_ADDR_MULTICAST(a) ||
-		    IN6_IS_ADDR_LINKLOCAL(a) ||
-		    IN6_IS_ADDR_V4MAPPED(a))
-			continue;
-
-		if (ba == NULL)
-			ba = a;
-
-		if (!IN6_IS_ADDR_SITELOCAL(a)) {
-                        ba = a;
-			break;
-                }
-	}
-
-	if (ba == NULL)
-		return ISC_R_NOTFOUND;
-
-	*ifaddr6 = *ba;
-	return ISC_R_SUCCESS;
-}
-#endif /* DHCPv6 */
-
 /* Generic interface registration routine... */
 int
-if_register_socket(struct interface_info *info, int family, int do_multicast) {
+if_register_socket(struct interface_info *info, int family,
+		   int *do_multicast)
+{
 	struct sockaddr_storage name;
 	int name_len;
 	int sock;
@@ -159,6 +138,7 @@ if_register_socket(struct interface_info *info, int family, int do_multicast) {
 		struct sockaddr_in6 *addr = (struct sockaddr_in6 *)&name; 
 		addr->sin6_family = AF_INET6;
 		addr->sin6_port = local_port;
+		/* XXX: What will happen to multicasts if this is nonzero? */
 		memcpy(&addr->sin6_addr,
 		       &local_address6, 
 		       sizeof(addr->sin6_addr));
@@ -168,14 +148,7 @@ if_register_socket(struct interface_info *info, int family, int do_multicast) {
 		name_len = sizeof(*addr);
 		domain = PF_INET6;
 		if ((info->flags & INTERFACE_STREAMS) == INTERFACE_UPSTREAM) {
-			struct in6_addr ifaddr6;
+			*do_multicast = 0;
-
-			do_multicast = 0;
-			if (get_ifaddr6(info, &ifaddr6) == ISC_R_SUCCESS) {
-				memcpy(&addr->sin6_addr,
-				       &ifaddr6,
-				       sizeof(addr->sin6_addr));
-			}
 		}
 	} else { 
 #else 
@@ -217,6 +190,24 @@ if_register_socket(struct interface_info *info, int family, int do_multicast) {
 		log_fatal("Can't set SO_BROADCAST option on dhcp socket: %m");
 	}
 
+#if defined(DHCPv6) && defined(SO_REUSEPORT)
+	/*
+	 * We only set SO_REUSEPORT on AF_INET6 sockets, so that multiple
+	 * daemons can bind to their own sockets and get data for their
+	 * respective interfaces.  This does not (and should not) affect
+	 * DHCPv4 sockets; we can't yet support BSD sockets well, much
+	 * less multiple sockets.
+	 */
+	if (local_family == AF_INET6) {
+		flag = 1;
+		if (setsockopt(sock, SOL_SOCKET, SO_REUSEPORT,
+			       (char *)&flag, sizeof(flag)) < 0) {
+			log_fatal("Can't set SO_REUSEPORT option on dhcp "
+				  "socket: %m");
+		}
+	}
+#endif
+
 	/* Bind the socket to this interface's IP address. */
 	if (bind(sock, (struct sockaddr *)&name, name_len) < 0) {
 		log_error("Can't bind to dhcp address: %m");
@@ -229,7 +220,7 @@ if_register_socket(struct interface_info *info, int family, int do_multicast) {
 
 #if defined(SO_BINDTODEVICE)
 	/* Bind this socket to this interface. */
-	if (info->ifp &&
+	if ((local_family != AF_INET6) && (info->ifp != NULL) &&
 	    setsockopt(sock, SOL_SOCKET, SO_BINDTODEVICE,
 			(char *)(info -> ifp), sizeof(*(info -> ifp))) < 0) {
 		log_fatal("setsockopt: SO_BINDTODEVICE: %m");
@@ -273,36 +264,6 @@ if_register_socket(struct interface_info *info, int family, int do_multicast) {
 #endif
 	}
 
-	if ((family == AF_INET6) && do_multicast) {
-		struct ipv6_mreq mreq;
-
-		/*
-		 * Join the DHCPv6 multicast groups so we will receive
-		 * multicast messages.
-		 */
-		if (inet_pton(AF_INET6, All_DHCP_Relay_Agents_and_Servers,
-			      &mreq.ipv6mr_multiaddr) <= 0) {
-			log_fatal("inet_pton: unable to convert '%s'", 
-				  All_DHCP_Relay_Agents_and_Servers);
-		}
-		mreq.ipv6mr_interface = if_nametoindex(info->name);
-		if (setsockopt(sock, IPPROTO_IPV6, IPV6_JOIN_GROUP, 
-			       &mreq, sizeof(mreq)) < 0) {
-			log_fatal("setsockopt: IPV6_JOIN_GROUP: %m");
-		}
-		if (inet_pton(AF_INET6, All_DHCP_Servers,
-			      &mreq.ipv6mr_multiaddr) <= 0) {
-			log_fatal("inet_pton: unable to convert '%s'", 
-				  All_DHCP_Servers);
-		}
-		mreq.ipv6mr_interface = if_nametoindex(info->name);
-		if (((info->flags & INTERFACE_DOWNSTREAM) == 0) &&
-		    (setsockopt(sock, IPPROTO_IPV6, IPV6_JOIN_GROUP,
-			       &mreq, sizeof(mreq)) < 0)) {
-			log_fatal("setsockopt: IPV6_JOIN_GROUP: %m");
-		}
-	}
-
 	if ((family == AF_INET6) &&
 	    ((info->flags & INTERFACE_UPSTREAM) != 0)) {
 		int hop_limit = 32;
@@ -313,7 +274,8 @@ if_register_socket(struct interface_info *info, int family, int do_multicast) {
 	}
 #endif /* DHCPv6 */
 
-	if (strcmp(info->name, "fallback") != 0)
+	/* If this is a normal IPv4 address, get the hardware address. */
+	if ((local_family == AF_INET) && (strcmp(info->name, "fallback") != 0))
 		get_hw_addr(info->name, &info->hw_address);
 
 	return sock;
@@ -393,15 +355,82 @@ void if_deregister_receive (info)
 
 
 #ifdef DHCPv6 
+/*
+ * This function joins the interface to DHCPv6 multicast groups so we will
+ * receive multicast messages.
+ */
+static void
+if_register_multicast(struct interface_info *info) {
+	int sock = info->rfdesc;
+	struct ipv6_mreq mreq;
+
+	if (inet_pton(AF_INET6, All_DHCP_Relay_Agents_and_Servers,
+		      &mreq.ipv6mr_multiaddr) <= 0) {
+		log_fatal("inet_pton: unable to convert '%s'", 
+			  All_DHCP_Relay_Agents_and_Servers);
+	}
+	mreq.ipv6mr_interface = if_nametoindex(info->name);
+	if (setsockopt(sock, IPPROTO_IPV6, IPV6_JOIN_GROUP, 
+		       &mreq, sizeof(mreq)) < 0) {
+		log_fatal("setsockopt: IPV6_JOIN_GROUP: %m");
+	}
+
+	/*
+	 * The relay agent code sets the streams so you know which way
+	 * is up and down.  But a relay agent shouldn't join to the
+	 * Server address, or else you get fun loops.  So up or down
+	 * doesn't matter, we're just using that config to sense this is
+	 * a relay agent.
+	 */
+	if ((info->flags & INTERFACE_STREAMS) == 0) {
+		if (inet_pton(AF_INET6, All_DHCP_Servers,
+			      &mreq.ipv6mr_multiaddr) <= 0) {
+			log_fatal("inet_pton: unable to convert '%s'", 
+				  All_DHCP_Servers);
+		}
+		mreq.ipv6mr_interface = if_nametoindex(info->name);
+		if (setsockopt(sock, IPPROTO_IPV6, IPV6_JOIN_GROUP, 
+			       &mreq, sizeof(mreq)) < 0) {
+			log_fatal("setsockopt: IPV6_JOIN_GROUP: %m");
+		}
+	}
+}
+
 void
 if_register6(struct interface_info *info, int do_multicast) {
-	info->rfdesc = if_register_socket(info, AF_INET6, do_multicast);
+	/* Bounce do_multicast to a stack variable because we may change it. */
-	info->wfdesc = info->rfdesc;
+	int req_multi = do_multicast;
+
+	if (global_v6_socket_references == 0) {
+		global_v6_socket = if_register_socket(info, AF_INET6,
+						      &req_multi);
+		if (global_v6_socket < 0) {
+			/*
+			 * if_register_socket() fatally logs if it fails to
+			 * create a socket, this is just a sanity check.
+			 */
+			log_fatal("Impossible condition at %s:%d", MDL);
+		} else {
+			log_info("Bound to *:%d", ntohs(local_port));
+		}
+	}
+		
+	info->rfdesc = global_v6_socket;
+	info->wfdesc = global_v6_socket;
+	global_v6_socket_references++;
+
+	if (req_multi)
+		if_register_multicast(info);
+
+	get_hw_addr(info->name, &info->hw_address);
+
 	if (!quiet_interface_discovery) {
 		if (info->shared_network != NULL) {
-			log_info("Listening on Socket/%s/%s", info->name, 
+			log_info("Listening on Socket/%d/%s/%s",
+				 global_v6_socket, info->name, 
 				 info->shared_network->name);
-			log_info("Sending on   Socket/%s/%s", info->name,
+			log_info("Sending on   Socket/%d/%s/%s",
+				 global_v6_socket, info->name,
 				 info->shared_network->name);
 		} else {
 			log_info("Listening on Socket/%s", info->name);
@@ -412,14 +441,16 @@ if_register6(struct interface_info *info, int do_multicast) {
 
 void 
 if_deregister6(struct interface_info *info) {
-	/* 
+	/* Dereference the global v6 socket. */
-	 * XXX: it would be nice to check for >= 0, but we need to change 
+	if ((info->rfdesc == global_v6_socket) &&
-	 *      interface_allocate() to set the file descriptors for that.
+	    (info->wfdesc == global_v6_socket) &&
-	 */
+	    (global_v6_socket_references > 0)) {
-	close(info->rfdesc);	
+		global_v6_socket_references--;
 		info->rfdesc = -1;
-	close(info->wfdesc);	
 		info->wfdesc = -1;
+	} else {
+		log_fatal("Impossible condition at %s:%d", MDL);
+	}
 
 	if (!quiet_interface_discovery) {
 		if (info->shared_network != NULL) {
@@ -432,6 +463,13 @@ if_deregister6(struct interface_info *info) {
 			log_info("Disabling output on Socket/%s", info->name);
 		}
 	}
+
+	if (global_v6_socket_references == 0) {
+		close(global_v6_socket);
+		global_v6_socket = -1;
+
+		log_info("Unbound from *:%d", ntohs(local_port));
+	}
 }
 #endif /* DHCPv6 */
 
@@ -640,13 +678,15 @@ ssize_t receive_packet (interface, buf, len, from, hfrom)
 ssize_t 
 receive_packet6(struct interface_info *interface, 
 		unsigned char *buf, size_t len, 
-		struct sockaddr_in6 *from, struct in6_addr *to_addr) {
+		struct sockaddr_in6 *from, struct in6_addr *to_addr,
+		unsigned int *if_idx)
+{
 	struct msghdr m;
 	struct iovec v;
 	int result;
 	struct cmsghdr *cmsg;
 	struct in6_pktinfo *pktinfo;
-	int found_to_addr;
+	int found_pktinfo;
 	union {
 	        struct cmsghdr cmsg_sizer;
 	        u_int8_t pktinfo_sizer[CMSG_SPACE(sizeof(struct in6_pktinfo))];
@@ -695,18 +735,19 @@ receive_packet6(struct interface_info *interface,
 		 * We also keep a flag to see if we found it. If we 
 		 * didn't, then we consider this to be an error.
 		 */
-		found_to_addr = 0;
+		found_pktinfo = 0;
 		cmsg = CMSG_FIRSTHDR(&m);
 		while (cmsg != NULL) {
 			if ((cmsg->cmsg_level == IPPROTO_IPV6) && 
 			    (cmsg->cmsg_type == IPV6_PKTINFO)) {
 				pktinfo = (struct in6_pktinfo *)CMSG_DATA(cmsg);
 				*to_addr = pktinfo->ipi6_addr;
-				found_to_addr = 1;
+				*if_idx = pktinfo->ipi6_ifindex;
+				found_pktinfo = 1;
 			}
 			cmsg = CMSG_NXTHDR(&m, cmsg);
 		}
-		if (!found_to_addr) {
+		if (!found_pktinfo) {
 			result = -1;
 			errno = EIO;
 		}

common/tree.c

@@ -99,6 +99,7 @@ data_string_sprintfa(struct data_string *ds, const char *fmt, ...) {
 	 */
 	va_start(args, fmt);
 	vsnprintf_ret = vsnprintf((char *)ds->data+cur_strlen, max, fmt, args);
+	va_end(args);
 	/* INSIST(vsnprintf_ret >= 0); */
 
 	/*
@@ -127,7 +128,11 @@ data_string_sprintfa(struct data_string *ds, const char *fmt, ...) {
 			return 0;
 		}
 		memcpy(tmp_buffer->data, ds->data, cur_strlen);
+
+		/* Rerun the vsprintf. */
+		va_start(args, fmt);
 		vsprintf((char *)tmp_buffer->data + cur_strlen, fmt, args);
+		va_end(args);
 
 		/*
 		 * Replace our old buffer with the new buffer.
@@ -138,7 +143,6 @@ data_string_sprintfa(struct data_string *ds, const char *fmt, ...) {
 		ds->data = ds->buffer->data;
 		ds->len = new_len;
 	}
-	va_end(args);
 	return 1;
 }
 

includes/dhcpd.h

@@ -1128,7 +1128,7 @@ struct interface_info {
 	unsigned remote_id_len;		/* Length of Remote ID. */
 
 	char name [IFNAMSIZ];		/* Its name... */
-	int index;			/* Its index. */
+	int index;			/* Its if_nametoindex(). */
 	int rfdesc;			/* Its read file descriptor. */
 	int wfdesc;			/* Its write file descriptor, if
 					   different. */
@@ -2140,7 +2140,7 @@ void get_hw_addr(const char *name, struct hardware *hw);
 /* socket.c */
 #if defined (USE_SOCKET_SEND) || defined (USE_SOCKET_RECEIVE) \
 	|| defined (USE_SOCKET_FALLBACK)
-int if_register_socket(struct interface_info *, int, int);
+int if_register_socket(struct interface_info *, int, int *);
 #endif
 
 #if defined (USE_SOCKET_FALLBACK) && !defined (USE_SOCKET_SEND)
@@ -2189,7 +2189,8 @@ void maybe_setup_fallback PROTO ((void));
 void if_register6(struct interface_info *info, int do_multicast);
 ssize_t receive_packet6(struct interface_info *interface,
 			unsigned char *buf, size_t len,
-			struct sockaddr_in6 *from, struct in6_addr *to_addr);
+			struct sockaddr_in6 *from, struct in6_addr *to_addr,
+			unsigned int *if_index);
 void if_deregister6(struct interface_info *info);