[collectd] [PATCH 2/3] network plugin: Bind to device for unicast

[Max Henkel]

Hello list!

On Tue, Mar 09, 2010 at 06:17:35PM +0100, Florian Forster wrote:
[...]
> On Fri, Feb 26, 2010 at 12:49:02PM +0100, Max Henkel wrote:
[...]
> > +           if (! IN_MULTICAST (ntohl (addr->sin_addr.s_addr)))
> > +                   return (0);
> 
> Doesn't it make sense to be able to set the interface in unicast mode,
> too? For example if the host has multiple default gateways. The
> socket(7) option "SO_BINDTODEVICE" could be used for this I guess.
[...]

Thus this patch binds the socket to a specific interface for unicast
traffic, too. But, as Sebastian already mentioned, the behaviour is
Linux specific.

Best regards,

Max

---
 src/collectd.conf.pod |   10 +++--
 src/network.c         |  100 ++++++++++++++++++++++++++++++++++++++-----------
 2 files changed, 84 insertions(+), 26 deletions(-)

diff --git a/src/collectd.conf.pod b/src/collectd.conf.pod
index 4538232..7941a6c 100644
--- a/src/collectd.conf.pod
+++ b/src/collectd.conf.pod
@@ -2445,10 +2445,12 @@ operating systems.
 
 =item B<Interface> I<Interface name>
 
-Set the outgoing or incoming interface for multicast packets. This applies
-at least to IPv6 packets and if possible to IPv4. If it is not applicable or
-defined the default behaviour is to let the kernel choose the appropriate
-interface.
+Set the outgoing or incoming interface for IP packets. This applies at least
+to IPv6 packets and if possible to IPv4. If this option is not applicable,
+undefined or a non-existent interface name is specified, the default
+behaviour is to let the kernel choose the appropriate interface. Be warned
+that the manual selection of an interface for unicast traffic is only
+necessary in rare cases.
 
 =item B<MaxPacketSize> I<1024-65535>
 
diff --git a/src/network.c b/src/network.c
index b6e21b9..eb074e9 100644
--- a/src/network.c
+++ b/src/network.c
@@ -1597,50 +1597,77 @@ static int network_set_interface (const sockent_t *se, const struct addrinfo *ai
 	if (ai->ai_family == AF_INET)
 	{
 		struct sockaddr_in *addr = (struct sockaddr_in *) ai->ai_addr;
+
+		if (IN_MULTICAST (ntohl (addr->sin_addr.s_addr)))
+		{
 #if KERNEL_LINUX
-		struct ip_mreqn mreq;
+			struct ip_mreqn mreq;
 #else
-		struct ip_mreq mreq;
+			struct ip_mreq mreq;
 #endif
 
-		if (! IN_MULTICAST (ntohl (addr->sin_addr.s_addr)))
-			return (0);
-
-		mreq.imr_multiaddr.s_addr = addr->sin_addr.s_addr;
+			mreq.imr_multiaddr.s_addr = addr->sin_addr.s_addr;
 #if KERNEL_LINUX
-		mreq.imr_address.s_addr = ntohl (INADDR_ANY);
-		mreq.imr_ifindex = network_config_interface_idx;
+			mreq.imr_address.s_addr = ntohl (INADDR_ANY);
+			mreq.imr_ifindex = network_config_interface_idx;
 #else
-		mreq.imr_interface.s_addr = ntohl (INADDR_ANY);
+			mreq.imr_interface.s_addr = ntohl (INADDR_ANY);
 #endif
 
-		if (setsockopt (se->data.client.fd, IPPROTO_IP, IP_MULTICAST_IF,
-					&mreq, sizeof (mreq)) == -1)
-		{
-			char errbuf[1024];
-			ERROR ("setsockopt: %s",
-					sstrerror (errno, errbuf, sizeof (errbuf)));
-			return (-1);
+			if (setsockopt (se->data.client.fd, IPPROTO_IP, IP_MULTICAST_IF,
+						&mreq, sizeof (mreq)) == -1)
+			{
+				char errbuf[1024];
+				ERROR ("setsockopt: %s",
+						sstrerror (errno, errbuf, sizeof (errbuf)));
+				return (-1);
+			}
+
+			return (0);
 		}
 	}
 	else if (ai->ai_family == AF_INET6)
 	{
 		struct sockaddr_in6 *addr = (struct sockaddr_in6 *) ai->ai_addr;
 
-		if (! IN6_IS_ADDR_MULTICAST (&addr->sin6_addr))
+		if (IN6_IS_ADDR_MULTICAST (&addr->sin6_addr))
+		{
+			if (setsockopt (se->data.client.fd, IPPROTO_IPV6, IPV6_MULTICAST_IF,
+						&network_config_interface_idx,
+						sizeof (network_config_interface_idx)) == -1)
+			{
+				char errbuf[1024];
+				ERROR ("setsockopt: %s",
+						sstrerror (errno, errbuf,
+							sizeof (errbuf)));
+				return (-1);
+			}
+
 			return (0);
+		}
+	}
+
+#if KERNEL_LINUX
+	if (network_config_interface_idx != 0)
+	{
+		char interface_name[IFNAMSIZ];
+
+		if (if_indextoname (network_config_interface_idx, interface_name) == NULL)
+			return (-1);
 
-		if (setsockopt (se->data.client.fd, IPPROTO_IPV6, IPV6_MULTICAST_IF,
-					&network_config_interface_idx,
-					sizeof (network_config_interface_idx)) == -1)
+		DEBUG ("network plugin: Binding socket to interface %s", interface_name);
+
+		if (setsockopt (se->data.client.fd, SOL_SOCKET, SO_BINDTODEVICE,
+					interface_name,
+					sizeof(interface_name)) == -1 )
 		{
 			char errbuf[1024];
 			ERROR ("setsockopt: %s",
-					sstrerror (errno, errbuf,
-						sizeof (errbuf)));
+					sstrerror (errno, errbuf, sizeof (errbuf)));
 			return (-1);
 		}
 	}
+#endif
 
 	return (0);
 } /* }}} network_set_interface */
@@ -1709,6 +1736,8 @@ static int network_bind_socket (int fd, const struct addrinfo *ai)
 							sizeof (errbuf)));
 				return (-1);
 			}
+
+			return (0);
 		}
 	}
 	else if (ai->ai_family == AF_INET6)
@@ -1755,8 +1784,35 @@ static int network_bind_socket (int fd, const struct addrinfo *ai)
 							sizeof (errbuf)));
 				return (-1);
 			}
+
+			return (0);
+		}
+	}
+
+#if KERNEL_LINUX
+	/* if a specific interface was set, bind the socket to it. But to avoid
+ 	 * possible problems with multicast routing, only do that for non-multicast
+	 * addresses */
+	if (network_config_interface_idx != 0)
+	{
+		char interface_name[IFNAMSIZ];
+
+		if (if_indextoname (network_config_interface_idx, interface_name) == NULL)
+			return (-1);
+
+		DEBUG ("fd = %i; Binding socket to interface %s", fd, interface_name);
+
+		if (setsockopt (fd, SOL_SOCKET, SO_BINDTODEVICE,
+					interface_name,
+					sizeof(interface_name)) == -1 )
+		{
+			char errbuf[1024];
+			ERROR ("setsockopt: %s",
+					sstrerror (errno, errbuf, sizeof (errbuf)));
+			return (-1);
 		}
 	}
+#endif
 
 	return (0);
 } /* int network_bind_socket */
-- 
1.7.0