diff mbox series

[v3,1/4] ibacm: Replace ioctl with netlink

Message ID 20190201164557.234039-2-haakon.bugge@oracle.com (mailing list archive)
State Superseded
Headers show
Series ibacm: Replace ioctl with netlink and fix inablity to resurrect an interface | expand

Commit Message

Haakon Bugge Feb. 1, 2019, 4:45 p.m. UTC
ibacm uses ioctl with SIOCGIFCONF to enumerate the interfaces. This
particular ioctl will only return interfaces that are "running".

This is a problem if, for example, ibacm is started before the
interfaces get plumbed, or when an interface gets resurrected.

By using netlink (nl), we avoid using the sysfs interface to read the
gid, as it is supplied directly by nl.

This commit does not change the existing functionality, but is made
to prepare for a fix where ibacm is unable to resurrect an interface.

Detailed logging added:

acm_if_iter: name: stib0 label:     stib0 index:  9
flags: broadcast,multicast,up,running,lowerup
addr: 192.168.200.200/24 pkey: 0x88b4 guid: 0x21280001a17c97

Signed-off-by: Håkon Bugge <haakon.bugge@oracle.com>
Reviewed-by: Ira Weiny <ira.weiny@intel.com>

---

v1 -> v2:
   * Use static for CB function (Jason, Ira)
   * Use original order of includes (Ira)
   * Use ib_gid's global.interface_id instead of casting (Ira)
   * Require Netlink in CMakeLists.txt (Jason)
   * Removed ioctl.h include (Håkon)
   * Removed Gerrit's Change-Id tag (Håkon)
---
 ibacm/CMakeLists.txt |   4 +
 ibacm/src/acm.c      |   7 ++
 ibacm/src/acm_util.c | 251 ++++++++++++++++++++++++-------------------
 ibacm/src/acm_util.h |   5 +-
 4 files changed, 155 insertions(+), 112 deletions(-)

Comments

Jason Gunthorpe Feb. 1, 2019, 5:10 p.m. UTC | #1
On Fri, Feb 01, 2019 at 05:45:54PM +0100, Håkon Bugge wrote:
> ibacm uses ioctl with SIOCGIFCONF to enumerate the interfaces. This
> particular ioctl will only return interfaces that are "running".
> 
> This is a problem if, for example, ibacm is started before the
> interfaces get plumbed, or when an interface gets resurrected.
> 
> By using netlink (nl), we avoid using the sysfs interface to read the
> gid, as it is supplied directly by nl.
> 
> This commit does not change the existing functionality, but is made
> to prepare for a fix where ibacm is unable to resurrect an interface.
> 
> Detailed logging added:
> 
> acm_if_iter: name: stib0 label:     stib0 index:  9
> flags: broadcast,multicast,up,running,lowerup
> addr: 192.168.200.200/24 pkey: 0x88b4 guid: 0x21280001a17c97
> 
> Signed-off-by: Håkon Bugge <haakon.bugge@oracle.com>
> Reviewed-by: Ira Weiny <ira.weiny@intel.com>
> 
> 
> v1 -> v2:
>    * Use static for CB function (Jason, Ira)
>    * Use original order of includes (Ira)
>    * Use ib_gid's global.interface_id instead of casting (Ira)
>    * Require Netlink in CMakeLists.txt (Jason)
>    * Removed ioctl.h include (Håkon)
>    * Removed Gerrit's Change-Id tag (Håkon)
>  ibacm/CMakeLists.txt |   4 +
>  ibacm/src/acm.c      |   7 ++
>  ibacm/src/acm_util.c | 251 ++++++++++++++++++++++++-------------------
>  ibacm/src/acm_util.h |   5 +-
>  4 files changed, 155 insertions(+), 112 deletions(-)
> 
> diff --git a/ibacm/CMakeLists.txt b/ibacm/CMakeLists.txt
> index 3a4e2632..7020def6 100644
> +++ b/ibacm/CMakeLists.txt
> @@ -1,3 +1,5 @@
> +pkg_check_modules(NL libnl-3.0 libnl-route-3.0 REQUIRED)

This should follow the pattern of iwpmd:

if (NOT NL_KIND EQUAL 0)
  add_subdirectory(iwpmd)
endif()

Jason
Jason Gunthorpe Feb. 1, 2019, 6:32 p.m. UTC | #2
On Fri, Feb 01, 2019 at 07:25:47PM +0100, Håkon Bugge wrote:
>    I see. In addition to what I have in ibacm/CMakeLists.txt or replacing
>    it?

Replacing

Jason
diff mbox series

Patch

diff --git a/ibacm/CMakeLists.txt b/ibacm/CMakeLists.txt
index 3a4e2632..7020def6 100644
--- a/ibacm/CMakeLists.txt
+++ b/ibacm/CMakeLists.txt
@@ -1,3 +1,5 @@ 
+pkg_check_modules(NL libnl-3.0 libnl-route-3.0 REQUIRED)
+
 publish_headers(infiniband
   include/infiniband/acm.h
   include/infiniband/acm_prov.h
@@ -7,6 +9,7 @@  publish_headers(infiniband
 include_directories("include")
 include_directories("src")
 include_directories("linux")
+include_directories(${NL_INCLUDE_DIRS})
 
 # NOTE: ibacm exports symbols from its own binary for use by ibacm
 rdma_sbin_executable(ibacm
@@ -16,6 +19,7 @@  rdma_sbin_executable(ibacm
 target_link_libraries(ibacm LINK_PRIVATE
   ibverbs
   ibumad
+  ${NL_LIBRARIES}
   ${SYSTEMD_LIBRARIES}
   ${CMAKE_THREAD_LIBS_INIT}
   ${CMAKE_DL_LIBS}
diff --git a/ibacm/src/acm.c b/ibacm/src/acm.c
index 6453c5f0..e8d50a4a 100644
--- a/ibacm/src/acm.c
+++ b/ibacm/src/acm.c
@@ -3250,6 +3250,12 @@  int main(int argc, char **argv)
 		acm_log(0, "Error: failed to create sa resp rcving thread");
 		return -1;
 	}
+
+	if (acm_init_if_iter_sys()) {
+		acm_log(0, "Error: unable to initialize acm_if_iter_sys");
+		return -1;
+	}
+
 	acm_activate_devices();
 	acm_log(1, "starting server\n");
 	acm_server(systemd);
@@ -3260,6 +3266,7 @@  int main(int argc, char **argv)
 	acm_close_providers();
 	acm_stop_sa_handler();
 	umad_done();
+	acm_fini_if_iter_sys();
 	fclose(flog);
 	return 0;
 }
diff --git a/ibacm/src/acm_util.c b/ibacm/src/acm_util.c
index f4654f12..fecb6c89 100644
--- a/ibacm/src/acm_util.c
+++ b/ibacm/src/acm_util.c
@@ -33,42 +33,18 @@ 
 #include <string.h>
 #include <unistd.h>
 #include <arpa/inet.h>
-#include <sys/ioctl.h>
 #include <net/if.h>
 #include <sys/socket.h>
 #include <sys/types.h>
 #include <errno.h>
+#include <netlink/route/addr.h>
+#include <netlink/route/link.h>
+#include <netlink/socket.h>
 
 #include <infiniband/acm.h>
 #include "acm_mad.h"
 #include "acm_util.h"
 
-int acm_if_is_ib(char *ifname)
-{
-	unsigned type;
-	char buf[128];
-	FILE *f;
-	int ret;
-
-	snprintf(buf, sizeof buf, "//sys//class//net//%s//type", ifname);
-	f = fopen(buf, "r");
-	if (!f) {
-		acm_log(0, "failed to open %s\n", buf);
-		return 0;
-	}
-
-	if (fgets(buf, sizeof buf, f)) {
-		type = strtol(buf, NULL, 0);
-		ret = (type == ARPHRD_INFINIBAND);
-	} else {
-		acm_log(0, "failed to read interface type\n");
-		ret = 0;
-	}
-
-	fclose(f);
-	return ret;
-}
-
 int acm_if_get_pkey(char *ifname, uint16_t *pkey)
 {
 	char buf[128], *end;
@@ -122,103 +98,156 @@  int acm_if_get_sgid(char *ifname, union ibv_gid *sgid)
 	return ret;
 }
 
-int acm_if_iter_sys(acm_if_iter_cb cb, void *ctx)
+static struct nl_sock *sk = NULL;
+static struct nl_cache *link_cache;
+static struct nl_cache *addr_cache;
+
+int acm_init_if_iter_sys(void)
 {
-	struct ifconf *ifc;
-	struct ifreq *ifr;
-	char ip_str[INET6_ADDRSTRLEN];
-	int family = AF_INET6;
-	int s, ret, i, len;
-	uint16_t pkey;
-	union ibv_gid sgid;
-	uint8_t addr_type;
-	uint8_t addr[ACM_MAX_ADDRESS];
-	char *alias_sep;
-
-	s = socket(family, SOCK_DGRAM, 0);
-	if (!s) {
-		family = AF_INET;
-		s = socket(family, SOCK_DGRAM, 0);
-		if (!s)
-			return -1;
+	int sts;
+
+	sk = nl_socket_alloc();
+	if (!sk) {
+		acm_log(0, "nl_socket_alloc");
+		return -1;
 	}
 
-	len = sizeof(*ifc) + sizeof(*ifr) * 64;
-	ifc = malloc(len);
-	if (!ifc) {
-		ret = -1;
-		goto out1;
+	sts = nl_connect(sk, NETLINK_ROUTE);
+	if (sts) {
+		acm_log(0, "nl_connect failed");
+		goto out_connect;
 	}
 
-	memset(ifc, 0, len);
-	ifc->ifc_len = len - sizeof(*ifc);
-	ifc->ifc_req = (struct ifreq *) (ifc + 1);
-
-retry_ioctl:
-	ret = ioctl(s, SIOCGIFCONF, ifc);
-	if (ret < 0) {
-		acm_log(0, "ioctl IPv%s ifconf error: %s\n",
-			(family == AF_INET6) ? "6" : "4", strerror(errno));
-		if (family == AF_INET6) {
-			close(s);
-			family = AF_INET;
-			s = socket(family, SOCK_DGRAM, 0);
-			if (!s) {
-				free(ifc);
-				return ret;
-			}
-			goto retry_ioctl;
-		}
-		goto out2;
+	sts = rtnl_link_alloc_cache(sk, AF_UNSPEC, &link_cache);
+	if (sts) {
+		acm_log(0, "rtnl_link_alloc_cache failed");
+		goto out_connect;
 	}
 
-	ifr = ifc->ifc_req;
-	for (i = 0; i < ifc->ifc_len / sizeof(struct ifreq); i++) {
-		switch (ifr[i].ifr_addr.sa_family) {
-		case AF_INET:
-			addr_type = ACM_ADDRESS_IP;
-			memcpy(&addr, &((struct sockaddr_in *) &ifr[i].ifr_addr)->sin_addr,
-				sizeof addr);
-			inet_ntop(ifr[i].ifr_addr.sa_family,
-				&((struct sockaddr_in *) &ifr[i].ifr_addr)->sin_addr,
-				ip_str, sizeof ip_str);
-			break;
-		case AF_INET6:
-			addr_type = ACM_ADDRESS_IP6;
-			memcpy(&addr, &((struct sockaddr_in6 *) &ifr[i].ifr_addr)->sin6_addr,
-				sizeof addr);
-			inet_ntop(ifr[i].ifr_addr.sa_family,
-				&((struct sockaddr_in6 *) &ifr[i].ifr_addr)->sin6_addr,
-				ip_str, sizeof ip_str);
-			break;
-		default:
-			continue;
-		}
+	sts = rtnl_addr_alloc_cache(sk, &addr_cache);
+	if (sts) {
+		acm_log(0, "rtnl_addr_alloc_cache");
+		goto out_addr;
+	}
 
-		acm_log(2, "%s\n", ifr[i].ifr_name);
+	return 0;
 
-		alias_sep = strchr(ifr[i].ifr_name, ':');
-		if (alias_sep)
-			*alias_sep = '\0';
+out_addr:
+	nl_cache_free(link_cache);
 
-		if (!acm_if_is_ib(ifr[i].ifr_name))
-			continue;
+out_connect:
+	nl_close(sk);
+	return sts;
+}
 
-		ret = acm_if_get_sgid(ifr[i].ifr_name, &sgid);
-		if (ret)
-			continue;
+void acm_fini_if_iter_sys(void)
+{
+	nl_cache_free(link_cache);
+	nl_cache_free(addr_cache);
+	nl_close(sk);
+}
 
-		ret = acm_if_get_pkey(ifr[i].ifr_name, &pkey);
-		if (ret)
-			continue;
+static inline int af2acm_addr_type(int af)
+{
+	switch (af) {
+	case AF_INET:
+		return ACM_ADDRESS_IP;
 
-		cb(ifr[i].ifr_name, &sgid, pkey, addr_type, addr, ip_str, ctx);
+	case AF_INET6:
+		return ACM_ADDRESS_IP6;
 	}
-	ret = 0;
 
-out2:
-	free(ifc);
-out1:
-	close(s);
-	return ret;
+	acm_log(0, "Unnkown address family\n");
+	return ACM_ADDRESS_INVALID;
+}
+
+struct ctx_and_cb {
+	void *ctx;
+	acm_if_iter_cb cb;
+};
+
+static void acm_if_iter(struct nl_object *obj, void *_ctx_and_cb)
+{
+	struct ctx_and_cb *ctx_cb = (struct ctx_and_cb *)_ctx_and_cb;
+	struct rtnl_addr *addr = (struct rtnl_addr *)obj;
+	uint8_t bin_addr[ACM_MAX_ADDRESS] = {};
+	char ip_str[INET6_ADDRSTRLEN];
+	struct nl_addr *link_addr;
+	struct rtnl_link *link;
+	char flags_str[128];
+	union ibv_gid sgid;
+	struct nl_addr *a;
+	uint16_t pkey;
+	int addr_len;
+	char *label;
+	int flags;
+	int ret;
+	int af;
+
+	link = rtnl_link_get(link_cache, rtnl_addr_get_ifindex(addr));
+	flags = rtnl_link_get_flags(link);
+
+	if (rtnl_link_get_arptype(link) != ARPHRD_INFINIBAND)
+		return;
+
+	if (!(flags & IFF_RUNNING))
+		return;
+
+	if (!(a = rtnl_addr_get_local(addr)))
+		return;
+
+	if ((addr_len = nl_addr_get_len(a)) > ACM_MAX_ADDRESS) {
+		acm_log(0, "address too long (%d)\n", addr_len);
+		return;
+	}
+
+	af = nl_addr_get_family(a);
+	if (af != AF_INET && af != AF_INET6)
+		return;
+
+	label = rtnl_addr_get_label(addr);
+	if (!label)
+		return;
+
+	link_addr = rtnl_link_get_addr(link);
+	memcpy(sgid.raw, nl_addr_get_binary_addr(link_addr) + 4, sizeof(sgid));
+
+	ret = acm_if_get_pkey(rtnl_link_get_name(link), &pkey);
+	if (ret)
+		return;
+
+	acm_log(2, "name: %5s label: %9s index: %2d flags: %s addr: %s pkey: 0x%04x guid: 0x%lx\n",
+		rtnl_link_get_name(link), label,
+		rtnl_addr_get_ifindex(addr),
+		rtnl_link_flags2str(flags, flags_str, sizeof(flags_str)),
+		nl_addr2str(a, ip_str, sizeof(ip_str)),	pkey,
+		be64toh(sgid.global.interface_id));
+
+	memcpy(&bin_addr, nl_addr_get_binary_addr(a), addr_len);
+	ctx_cb->cb(rtnl_link_get_name(link), &sgid, pkey, af2acm_addr_type(af), bin_addr, ip_str, ctx_cb->ctx);
+}
+
+
+int acm_if_iter_sys(acm_if_iter_cb cb, void *ctx)
+{
+	struct ctx_and_cb ctx_cb;
+	int sts;
+
+	sts = nl_cache_refill(sk, link_cache);
+	if (sts) {
+		acm_log(0, "nl_cache_refill link_cache");
+		return sts;
+	}
+
+	sts = nl_cache_refill(sk, addr_cache);
+	if (sts) {
+		acm_log(0, "nl_cache_refill addr_cache");
+		return sts;
+	}
+
+	ctx_cb.ctx = ctx;
+	ctx_cb.cb = cb;
+	nl_cache_foreach(addr_cache, acm_if_iter, (void *)&ctx_cb);
+
+	return 0;
 }
diff --git a/ibacm/src/acm_util.h b/ibacm/src/acm_util.h
index cfa6f1a2..83b49edd 100644
--- a/ibacm/src/acm_util.h
+++ b/ibacm/src/acm_util.h
@@ -33,6 +33,7 @@ 
 #include <infiniband/verbs.h>
 #include <infiniband/acm_prov.h>
 
+
 #ifdef ACME_PRINTS
 
 #undef acm_log
@@ -47,12 +48,14 @@ 
 int acm_if_is_ib(char *ifname);
 int acm_if_get_pkey(char *ifname, uint16_t *pkey);
 int acm_if_get_sgid(char *ifname, union ibv_gid *sgid);
-
+int acm_init_if_iter_sys(void);
+void acm_fini_if_iter_sys(void);
 typedef void (*acm_if_iter_cb)(char *ifname, union ibv_gid *gid, uint16_t pkey,
 				uint8_t addr_type, uint8_t *addr,
 				char *ip_str, void *ctx);
 int acm_if_iter_sys(acm_if_iter_cb cb, void *ctx);
 
+
 char **parse(const char *args, int *count);
 
 #endif /* ACM_IF_H */