[PATCHES] multi-channel: implement network interface info ioctl

Michael Adam obnox at samba.org
Mon Jan 25 08:30:44 UTC 2016


Hi Metze,

Thanks for the comments!

On 2016-01-25 at 09:06 +0100, Stefan Metzmacher wrote:
> 
> the NDR_PRINT_DEBUG(fsctl_net_iface_info, first); needs to be on
> a higher debug level.

True, changing this..

> +	ret = ioctl(fd, SIOCETHTOOL, &ifr);
> +	if (ret == -1) {
> +		do {
> +			ret = close(fd);
> +		} while (ret == -1 && errno == EINTR);
> +
> +		return;
> +	}
> +	*speed = (ethtool_cmd_speed(&cmd)) * 1000 * 1000;
> +}
> 
> We should already close the fd, not only if ioctl fails!

Oops, true thanks! :-)

> Do we have a sys_close() that already does
>
> +		do {
> +			ret = close(fd);
> +		} while (ret == -1 && errno == EINTR);

Er no, sys_close() does not exist.

> This hight be an generic problem (or is not needed at at all)
> @Volker,Jemery: Do we need EINTR on close?

Hmm, just read up on https://lwn.net/Articles/576478/.
I think I'll go with just close then, for now..

Updated patchset attached, which addresses
all of the above.

> The correct handling for if_index will come later?

Well, I thouhgt about it for a while.
But I am not sure what the 'correct' handling is.
What relevance and value would it be for a client
to know that number instead of a made-up one.
We do have a patch that gets the if index from the
kernel. But what about the case of an interface
that is only specified via the interfaces line?
In that case we would need to make an index up
for this interface and make sure not to collide
with any of those from the kernel.

Cheers - Michael
-------------- next part --------------
From 8a4f056e39b068f3e30e6ac6072a6f6c9cd959b8 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Mon, 25 Jan 2016 03:37:38 +0100
Subject: [PATCH 1/9] librpc:idl: define FSCTL_NET_IFACE_NONE_CAPABLE in
 ioctl.idl

Signed-off-by: Michael Adam <obnox at samba.org>
---
 librpc/idl/ioctl.idl | 1 +
 1 file changed, 1 insertion(+)

diff --git a/librpc/idl/ioctl.idl b/librpc/idl/ioctl.idl
index 5c3ee6d..dbeef14 100644
--- a/librpc/idl/ioctl.idl
+++ b/librpc/idl/ioctl.idl
@@ -111,6 +111,7 @@ interface compression
 interface netinterface
 {
 	typedef [bitmap32bit] bitmap {
+		FSCTL_NET_IFACE_NONE_CAPABLE = 0x00000000,
 		FSCTL_NET_IFACE_RSS_CAPABLE = 0x00000001,
 		FSCTL_NET_IFACE_RDMA_CAPABLE = 0x00000002
 	} fsctl_net_iface_capability;
-- 
2.5.0


From 04cee76cda0f4abfaaaee7824e2e5f80c23eb01f Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Mon, 25 Jan 2016 03:38:05 +0100
Subject: [PATCH 2/9] lib:socket: add linkspeed and capability to iface_struct

Signed-off-by: Michael Adam <obnox at samba.org>
---
 lib/socket/interfaces.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/socket/interfaces.h b/lib/socket/interfaces.h
index b4e113d..5860b4a 100644
--- a/lib/socket/interfaces.h
+++ b/lib/socket/interfaces.h
@@ -27,6 +27,8 @@ struct iface_struct {
 	struct sockaddr_storage ip;
 	struct sockaddr_storage netmask;
 	struct sockaddr_storage bcast;
+	uint64_t linkspeed;
+	uint32_t capability;
 };
 
 struct interface;
-- 
2.5.0


From 7a03a061b22869bfee5f8ebd1b4acd1490e46c1c Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Mon, 25 Jan 2016 03:38:16 +0100
Subject: [PATCH 3/9] smb.h: add linkspeed and capability to interface struct

Signed-off-by: Michael Adam <obnox at samba.org>
---
 source3/include/smb.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/source3/include/smb.h b/source3/include/smb.h
index 94a0e8a..623fdd6 100644
--- a/source3/include/smb.h
+++ b/source3/include/smb.h
@@ -181,6 +181,8 @@ struct interface {
 	struct sockaddr_storage ip;
 	struct sockaddr_storage netmask;
 	struct sockaddr_storage bcast;
+	uint64_t linkspeed;
+	uint32_t capability;
 };
 
 #define SHARE_MODE_FLAG_POSIX_OPEN	0x1
-- 
2.5.0


From 42cd7c02fce7cebb905bbf44576301874c42a87a Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Mon, 25 Jan 2016 03:38:31 +0100
Subject: [PATCH 4/9] socket:interface: set defaults for linkspeed and
 capability in get_interfaces()

Signed-off-by: Michael Adam <obnox at samba.org>
---
 lib/socket/interfaces.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/lib/socket/interfaces.c b/lib/socket/interfaces.c
index e62da3c..d72e819 100644
--- a/lib/socket/interfaces.c
+++ b/lib/socket/interfaces.c
@@ -24,6 +24,7 @@
 #include "system/network.h"
 #include "interfaces.h"
 #include "lib/util/tsort.h"
+#include "librpc/gen_ndr/ioctl.h"
 
 /****************************************************************************
  Create a struct sockaddr_storage with the netmask bits set to 1.
@@ -137,6 +138,7 @@ static int _get_interfaces(TALLOC_CTX *mem_ctx, struct iface_struct **pifaces)
 	int count;
 	int total = 0;
 	size_t copy_size;
+	uint64_t if_speed = 1000 * 1000 * 1000; /* 1GBit */
 
 	if (getifaddrs(&iflist) < 0) {
 		return -1;
@@ -214,6 +216,9 @@ static int _get_interfaces(TALLOC_CTX *mem_ctx, struct iface_struct **pifaces)
 			continue;
 		}
 
+		ifaces[total].linkspeed = if_speed;
+		ifaces[total].capability = FSCTL_NET_IFACE_NONE_CAPABLE;
+
 		if (strlcpy(ifaces[total].name, ifptr->ifa_name,
 			sizeof(ifaces[total].name)) >=
 				sizeof(ifaces[total].name)) {
-- 
2.5.0


From 0558e1654b98b4cca179e5fd5334210ebb0298ac Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Sun, 24 Jan 2016 13:26:35 +0100
Subject: [PATCH 5/9] s3:lib:interface: remove an unmotivated comment

This seems to be a left-over from historic code.

Signed-off-by: Michael Adam <obnox at samba.org>
---
 source3/lib/interface.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/source3/lib/interface.c b/source3/lib/interface.c
index adf8b42..1749341 100644
--- a/source3/lib/interface.c
+++ b/source3/lib/interface.c
@@ -382,7 +382,6 @@ static void interpret_interface(char *token)
 		return;
 	}
 
-	/* maybe it is a DNS name */
 	p = strchr_m(token,'/');
 	if (p == NULL) {
 		if (!interpret_string_addr(&ss, token, 0)) {
-- 
2.5.0


From f73cbe4da3d0a0441f57973e967ceae58d27d84b Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Mon, 25 Jan 2016 03:38:54 +0100
Subject: [PATCH 6/9] s3:lib:interface: extend load_interfaces to optionally
 read speed and caps from config

New syntax for interfaces parameter:

  interfaces = address[;speed=value[,capability=value]]

capability can be RSS and RDMA

Signed-off-by: Michael Adam <obnox at samba.org>
---
 source3/lib/interface.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 85 insertions(+)

diff --git a/source3/lib/interface.c b/source3/lib/interface.c
index 1749341..f185166 100644
--- a/source3/lib/interface.c
+++ b/source3/lib/interface.c
@@ -20,6 +20,7 @@
 
 #include "includes.h"
 #include "lib/socket/interfaces.h"
+#include "librpc/gen_ndr/ioctl.h"
 
 static struct iface_struct *probed_ifaces;
 static int total_probed;
@@ -333,6 +334,8 @@ static void add_interface(const struct iface_struct *ifs)
 	iface->ip = ifs->ip;
 	iface->netmask = ifs->netmask;
 	iface->bcast = ifs->bcast;
+	iface->linkspeed = ifs->linkspeed;
+	iface->capability = ifs->capability;
 
 	DLIST_ADD(local_interfaces, iface);
 
@@ -347,6 +350,42 @@ static void add_interface(const struct iface_struct *ifs)
 			&iface->netmask) ));
 }
 
+
+static void parse_extra_info(char *key, uint64_t *speed, uint32_t *cap)
+{
+	while (key != NULL && *key != '\0') {
+		char *next_key;
+		char *val;
+
+		next_key = strchr_m(key, ',');
+		if (next_key != NULL) {
+			*next_key++ = 0;
+		}
+
+		val = strchr_m(key, '=');
+		if (val != NULL) {
+			*val++ = 0;
+
+			if (strequal_m(key, "speed")) {
+				*speed = (uint64_t)strtoull(val, NULL, 0);
+			} else if (strequal_m(key, "capability")) {
+				if (strequal_m(val, "RSS")) {
+					*cap |= FSCTL_NET_IFACE_RSS_CAPABLE;
+				} else if (strequal(val, "RDMA")) {
+					*cap |= FSCTL_NET_IFACE_RDMA_CAPABLE;
+				} else {
+					DBG_WARNING("Capability unknown: "
+						    "'%s'\n", val);
+				}
+			} else {
+				DBG_DEBUG("Key unknown: '%s'\n", key);
+			}
+		}
+
+		key = next_key;
+	}
+}
+
 /****************************************************************************
  Interpret a single element from a interfaces= config line.
 
@@ -357,6 +396,15 @@ static void add_interface(const struct iface_struct *ifs)
  3) IP/masklen
  4) ip/mask
  5) bcast/mask
+
+ Additional information for an interface can be specified with
+ this extended syntax:
+
+    interface[;key1=value1[,key2=value2[...]]]
+
+ where
+ - key known: speed, capability
+ - capabilites known: RSS, RDMA
 ****************************************************************************/
 
 static void interpret_interface(char *token)
@@ -370,6 +418,10 @@ static void interpret_interface(char *token)
 	int i;
 	bool added=false;
 	bool goodaddr = false;
+	uint64_t speed = 0;
+	uint32_t cap = FSCTL_NET_IFACE_NONE_CAPABLE;
+	bool speed_set = false;
+	bool cap_set = false;
 
 	/* first check if it is an interface name */
 	for (i=0;i<total_probed;i++) {
@@ -382,6 +434,21 @@ static void interpret_interface(char *token)
 		return;
 	}
 
+	/*
+	 * extract speed / capability information if present
+	 */
+	p = strchr_m(token, ';');
+	if (p != NULL) {
+		*p++ = 0;
+		parse_extra_info(p, &speed, &cap);
+		if (speed != 0) {
+			speed_set = true;
+		}
+		if (cap != FSCTL_NET_IFACE_NONE_CAPABLE) {
+			cap_set = true;
+		}
+	}
+
 	p = strchr_m(token,'/');
 	if (p == NULL) {
 		if (!interpret_string_addr(&ss, token, 0)) {
@@ -394,6 +461,12 @@ static void interpret_interface(char *token)
 			if (sockaddr_equal((struct sockaddr *)&ss,
 				(struct sockaddr *)&probed_ifaces[i].ip))
 			{
+				if (speed_set) {
+					probed_ifaces[i].linkspeed = speed;
+				}
+				if (cap_set) {
+					probed_ifaces[i].capability = cap;
+				}
 				add_interface(&probed_ifaces[i]);
 				return;
 			}
@@ -463,6 +536,12 @@ static void interpret_interface(char *token)
 					"config file on interface %s\n",
 					p,
 					probed_ifaces[i].name));
+				if (speed_set) {
+					probed_ifaces[i].linkspeed = speed;
+				}
+				if (cap_set) {
+					probed_ifaces[i].capability = cap;
+				}
 				add_interface(&probed_ifaces[i]);
 				probed_ifaces[i].netmask = saved_mask;
 				return;
@@ -485,6 +564,12 @@ static void interpret_interface(char *token)
 	ifs.ip = ss;
 	ifs.netmask = ss_mask;
 	ifs.bcast = ss_bcast;
+	if (speed_set) {
+		ifs.linkspeed = speed;
+	} else {
+		ifs.linkspeed = 1000 * 1000 * 1000;
+	}
+	ifs.capability = cap;
 	add_interface(&ifs);
 }
 
-- 
2.5.0


From 2eb4ac587507f251a424c822a50e645f03149021 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Mon, 25 Jan 2016 03:30:39 +0100
Subject: [PATCH 7/9] build: detect support for ethtool

Pair-Programmed-With: Anoop C S <anoopcs at redhat.com>a

Signed-off-by: Michael Adam <obnox at samba.org>
---
 lib/socket/wscript | 7 +++++++
 wscript            | 1 +
 2 files changed, 8 insertions(+)
 create mode 100644 lib/socket/wscript

diff --git a/lib/socket/wscript b/lib/socket/wscript
new file mode 100644
index 0000000..d8c269a
--- /dev/null
+++ b/lib/socket/wscript
@@ -0,0 +1,7 @@
+#!/usr/bin/env python
+
+def configure(conf):
+    conf.CHECK_HEADERS('linux/sockios.h linux/ethtool.h')
+    if (conf.CONFIG_SET('HAVE_LINUX_SOCKIOS_H') and \
+                    conf.CONFIG_SET('HAVE_LINUX_ETHTOOL_H')):
+        conf.DEFINE('HAVE_ETHTOOL', 1)
diff --git a/wscript b/wscript
index c3c3cfd..41ed5da 100644
--- a/wscript
+++ b/wscript
@@ -180,6 +180,7 @@ def configure(conf):
     conf.RECURSE('lib/texpect')
     if conf.env.with_ctdb:
         conf.RECURSE('ctdb')
+    conf.RECURSE('lib/socket')
 
     conf.SAMBA_CHECK_UNDEFINED_SYMBOL_FLAGS()
 
-- 
2.5.0


From 3f2ea03a7e0d99d7d238e2eb6a7e1d7b92e977de Mon Sep 17 00:00:00 2001
From: Anoop C S <anoopcs at redhat.com>
Date: Fri, 22 Jan 2016 20:51:55 +0530
Subject: [PATCH 8/9] lib:socket: detect link speed with ethtool ioctl in
 get_interfaces (on linux)

Pair-Programmed-With: Michael Adam <obnox at samba.org>

Signed-off-by: Anoop C S <anoopcs at redhat.com>
---
 lib/socket/interfaces.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/lib/socket/interfaces.c b/lib/socket/interfaces.c
index d72e819..0591fa9 100644
--- a/lib/socket/interfaces.c
+++ b/lib/socket/interfaces.c
@@ -26,6 +26,11 @@
 #include "lib/util/tsort.h"
 #include "librpc/gen_ndr/ioctl.h"
 
+#ifdef HAVE_ETHTOOL
+#include "linux/sockios.h"
+#include "linux/ethtool.h"
+#endif
+
 /****************************************************************************
  Create a struct sockaddr_storage with the netmask bits set to 1.
 ****************************************************************************/
@@ -120,6 +125,34 @@ void make_net(struct sockaddr_storage *pss_out,
 	make_bcast_or_net(pss_out, pss_in, nmask, false);
 }
 
+#ifdef HAVE_ETHTOOL
+static void query_iface_speed_from_name(const char *name, uint64_t *speed)
+{
+	int ret = 0;
+	struct ethtool_cmd cmd;
+	struct ifreq ifr;
+	int fd;
+
+	/* TODO: Do we need to open socket fd according to address family? */
+	fd = socket (AF_INET, SOCK_DGRAM, IPPROTO_IP);
+	if (fd == -1) {
+		DBG_ERR("Failed to open socket.");
+		return;
+	}
+
+	strncpy(ifr.ifr_name, name, IF_NAMESIZE);
+
+	ifr.ifr_data = (void *)&cmd;
+	cmd.cmd = ETHTOOL_GSET;
+
+	ret = ioctl(fd, SIOCETHTOOL, &ifr);
+	(void)close(fd);
+	if (ret == -1) {
+		return;
+	}
+	*speed = (ethtool_cmd_speed(&cmd)) * 1000 * 1000;
+}
+#endif
 
 /****************************************************************************
  Try the "standard" getifaddrs/freeifaddrs interfaces.
@@ -216,6 +249,9 @@ static int _get_interfaces(TALLOC_CTX *mem_ctx, struct iface_struct **pifaces)
 			continue;
 		}
 
+#ifdef HAVE_ETHTOOL
+		query_iface_speed_from_name(ifptr->ifa_name, &if_speed);
+#endif
 		ifaces[total].linkspeed = if_speed;
 		ifaces[total].capability = FSCTL_NET_IFACE_NONE_CAPABLE;
 
-- 
2.5.0


From a265f249e4ab06df053d469b58f4d054c0b8063f Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 13 Jun 2014 17:42:00 +0200
Subject: [PATCH 9/9] s3:smbd: implement fsctl_network_iface_info

The ioctl used for detecting interfaces for multi-channel.

Pair-Programmed-With: Michael Adam <obnox at samba.org>

Signed-off-by: Michael Adam <obnox at samba.org>
---
 source3/smbd/smb2_ioctl_network_fs.c | 119 +++++++++++++++++++++++++++++++++++
 1 file changed, 119 insertions(+)

diff --git a/source3/smbd/smb2_ioctl_network_fs.c b/source3/smbd/smb2_ioctl_network_fs.c
index 01798ac..3d34ca3 100644
--- a/source3/smbd/smb2_ioctl_network_fs.c
+++ b/source3/smbd/smb2_ioctl_network_fs.c
@@ -29,6 +29,7 @@
 #include "../librpc/ndr/libndr.h"
 #include "librpc/gen_ndr/ndr_ioctl.h"
 #include "smb2_ioctl_private.h"
+#include "../lib/tsocket/tsocket.h"
 
 #define COPYCHUNK_MAX_CHUNKS	256		/* 2k8r2 & win8 = 256 */
 #define COPYCHUNK_MAX_CHUNK_LEN	1048576		/* 2k8r2 & win8 = 1048576 */
@@ -379,6 +380,113 @@ static NTSTATUS fsctl_srv_copychunk_recv(struct tevent_req *req,
 	return status;
 }
 
+static NTSTATUS fsctl_network_iface_info(TALLOC_CTX *mem_ctx,
+					 struct tevent_context *ev,
+					 struct smbXsrv_connection *xconn,
+					 DATA_BLOB *in_input,
+					 uint32_t in_max_output,
+					 DATA_BLOB *out_output)
+{
+	struct fsctl_net_iface_info *array = NULL;
+	struct fsctl_net_iface_info *first = NULL;
+	struct fsctl_net_iface_info *last = NULL;
+	size_t i;
+	size_t num_ifaces = iface_count();
+	enum ndr_err_code ndr_err;
+
+	if (in_input->length != 0) {
+		return NT_STATUS_INVALID_PARAMETER;
+	}
+
+	*out_output = data_blob_null;
+
+	array = talloc_zero_array(mem_ctx,
+				  struct fsctl_net_iface_info,
+				  num_ifaces);
+	if (array == NULL) {
+		return NT_STATUS_NO_MEMORY;
+	}
+
+	for (i=0; i < num_ifaces; i++) {
+		struct fsctl_net_iface_info *cur = &array[i];
+		const struct interface *iface = get_interface(i);
+		const struct sockaddr_storage *ifss =
+				iface_n_sockaddr_storage(i); /* &(iface->ip); */
+		const void *ifptr = ifss;
+		const struct sockaddr *ifsa = (const struct sockaddr *)ifptr;
+		struct tsocket_address *a = NULL;
+		char *addr;
+		bool ok;
+		int ret;
+
+		ret = tsocket_address_bsd_from_sockaddr(array,
+					ifsa, sizeof(struct sockaddr_storage),
+					&a);
+		if (ret != 0) {
+			return map_nt_error_from_unix_common(errno);
+		}
+
+		ok = tsocket_address_is_inet(a, "ip");
+		if (!ok) {
+			continue;
+		}
+
+		addr = tsocket_address_inet_addr_string(a, array);
+		if (addr == NULL) {
+			TALLOC_FREE(array);
+			return NT_STATUS_NO_MEMORY;
+		}
+
+		/*
+		 * Note: the ifindex must be at least 1 for windows or
+		 * windows will ignore the interface.
+		 *
+		 * Q: should we fill ifindex from the system (if_index)?
+		 *    This is potentially problematic with interfaces lines..
+		 */
+		cur->ifindex = i+1;
+		cur->capability = iface->capability;
+		cur->linkspeed = iface->linkspeed;
+
+		ok = tsocket_address_is_inet(a, "ipv4");
+		if (ok) {
+			cur->sockaddr.family = FSCTL_NET_IFACE_AF_INET;
+			cur->sockaddr.saddr.saddr_in.ipv4 = addr;
+		}
+		ok = tsocket_address_is_inet(a, "ipv6");
+		if (ok) {
+			cur->sockaddr.family = FSCTL_NET_IFACE_AF_INET6;
+			cur->sockaddr.saddr.saddr_in6.ipv6 = addr;
+		}
+
+		if (first == NULL) {
+			first = cur;
+		}
+		if (last != NULL) {
+			last->next = cur;
+		}
+		last = cur;
+	}
+
+	if (first == NULL) {
+		TALLOC_FREE(array);
+		return NT_STATUS_OK;
+	}
+
+	if (DEBUGLEVEL >= 10) {
+		NDR_PRINT_DEBUG(fsctl_net_iface_info, first);
+	}
+
+	ndr_err = ndr_push_struct_blob(out_output, mem_ctx, first,
+			(ndr_push_flags_fn_t)ndr_push_fsctl_net_iface_info);
+	TALLOC_FREE(array);
+	if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
+		return ndr_map_error2ntstatus(ndr_err);
+	}
+
+	return NT_STATUS_OK;
+}
+
 static NTSTATUS fsctl_validate_neg_info(TALLOC_CTX *mem_ctx,
 				        struct tevent_context *ev,
 				        struct smbXsrv_connection *conn,
@@ -541,6 +649,17 @@ struct tevent_req *smb2_ioctl_network_fs(uint32_t ctl_code,
 					req);
 		return req;
 		break;
+	case FSCTL_QUERY_NETWORK_INTERFACE_INFO:
+		status = fsctl_network_iface_info(state, ev,
+						  state->smbreq->xconn,
+						  &state->in_input,
+						  state->in_max_output,
+						  &state->out_output);
+		if (!tevent_req_nterror(req, status)) {
+			tevent_req_done(req);
+		}
+		return tevent_req_post(req, ev);
+		break;
 	case FSCTL_VALIDATE_NEGOTIATE_INFO:
 		status = fsctl_validate_neg_info(state, ev,
 						 state->smbreq->xconn,
-- 
2.5.0

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20160125/223401e9/signature.sig>


More information about the samba-technical mailing list