On Thu, Jan 24, 2019 at 03:47:35PM +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
>
> Change-Id: I6e428d1d6b2d345ccfbd74d482c69b56f8fe98da
> Signed-off-by: Håkon Bugge <haakon.bugge@xxxxxxxxxx>
> ---
[snip]
> diff --git a/ibacm/src/acm_util.c b/ibacm/src/acm_util.c
> index f4654f12..fc22432c 100644
> --- a/ibacm/src/acm_util.c
> +++ b/ibacm/src/acm_util.c
> @@ -27,48 +27,28 @@
> * SOFTWARE.
> */
>
> +#include <arpa/inet.h>
> +#include <asm/types.h>
> +#include <errno.h>
> +#include <linux/netlink.h>
> +#include <net/if.h>
> +#include <net/if_arp.h>
> +#include <netlink/route/addr.h>
> +#include <netlink/route/link.h>
> +#include <netlink/socket.h>
> +#include <stdbool.h>
> #include <stdio.h>
> #include <stdlib.h>
> -#include <net/if_arp.h>
> #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 <unistd.h>
NIT:
Lots of noise here in moving the includes around. It would be better if you
did not move the include lines around so we can see that you are removing and
adding the correct includes.
But I'm going to assume this compiles fine...
[snip]
> +
> +/* pre-decl to avoid compiler warning */
> +void acm_if_iter(struct nl_object *obj, void *_ctx_and_cb);
> +void acm_if_iter(struct nl_object *obj, void *_ctx_and_cb)
What Jason said...
> +{
> + 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;
> + unsigned long *guid_ptr;
> + 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));
> + guid_ptr = (unsigned long *)&sgid.raw;
This does not look right to support both IPv4 and v6.
> +
> + 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(*(guid_ptr + 1)));
Use sgid.global.interface_id to represent the guid. That is what ibv_gid union
is for...
Ira
> +
> + 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 */
> --
> 2.20.1
>
