Re: [PATCH v3 1/8] netlink: add function to extract MAC addresses from NDP/ARP table

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 4738 bytes --]

On Sun, Jun 29, 2025 at 01:13:40PM -0400, Jon Maloy wrote:
> The solution to bug https://bugs.passt.top/show_bug.cgi?id=120
> requires the ability to translate from an IP address to its
> corresponding MAC address in cases where those are present in
> the ARP/NDP table.
> 
> We add this feature here.
> 
> Signed-off-by: Jon Maloy <jmaloy@redhat.com>
> 
> ---
> v3: - Added an attribute contianing NDA_DST to sent message, so
>       that we let the kernel do the filtering of the IP address
>       and return only one entry.
>     - Added interface index to the call signature. Since the only
>       interface we know is the template interface, this limits
>       the number of hosts that will be seen as 'network segment
>       local' from a PASST viewpoint.
> ---
>  netlink.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  netlink.h |  2 ++
>  2 files changed, 64 insertions(+)
> 
> diff --git a/netlink.c b/netlink.c
> index ee9325a..16bb995 100644
> --- a/netlink.c
> +++ b/netlink.c
> @@ -800,6 +800,68 @@ int nl_addr_get(int s, unsigned int ifi, sa_family_t af,
>  	return status;
>  }
>  
> +/**
> + * nl_mac_get() - Get MAC address corresponding to given IP address
> + * @s:		Netlink socket
> + * @addr:	IPv4 or IPv6 address
> + * @ifi:	Interface index
> + * @mac:	Array to place the returned MAC address
> + *
> + * Return: 0 if found or not in table, negative error code on failure.
> + *         Leaves MAC array unchanged if no match found
> + */
> +int nl_mac_get(int s, const union inany_addr *addr, int ifi, unsigned char *mac)

Looking at the name alone, I'd think this got the local MAC, rather
than a neighbour MAC.  Maybe rename to nl_neigh_mac_get() or even just
nl_neigh_get()?

> +{
> +	struct req_t {
> +		struct nlmsghdr nlh;
> +		struct ndmsg ndm;
> +		struct rtattr rta;
> +		char ip[RTA_ALIGN(sizeof(struct in6_addr))];
> +	} req = {
> +		.ndm.ndm_ifindex = ifi,
> +		.rta.rta_type = NDA_DST
> +	};
> +	struct nlmsghdr *nh;
> +	char buf[NLBUFSIZ];
> +	const void *ip;
> +	ssize_t status;
> +	uint32_t seq;
> +	int msglen;
> +	int iplen;
> +
> +	if (inany_v4(addr)) {
> +		ip = &addr->v4mapped.a4;

I generally try to avoid reaching into inany internal fields,
particularly the v4 ones.  You could instead use:

	if ((ip = inany_v4(addr))) {
		...

> +		iplen = sizeof(struct in_addr);
> +		req.ndm.ndm_family = AF_INET;
> +	} else {
> +		ip = &addr->a6;

and here just ip = &addr.

> +		iplen = sizeof(struct in6_addr);
> +		req.ndm.ndm_family = AF_INET6;
> +	}
> +	req.rta.rta_len = RTA_LENGTH(iplen);
> +	memcpy(RTA_DATA(&req.rta), ip, iplen);
> +	msglen = NLMSG_ALIGN(sizeof(req.nlh) + sizeof(req.ndm) + RTA_LENGTH(iplen));
> +
> +	seq = nl_send(s, &req, RTM_GETNEIGH, 0, msglen);
> +	nl_foreach_oftype(nh, status, s, buf, seq, RTM_NEWNEIGH) {
> +		struct ndmsg *ndm = NLMSG_DATA(nh);
> +		struct rtattr *rta = (struct rtattr *)(ndm + 1);
> +		size_t na = RTM_PAYLOAD(nh);
> +		bool found = false;
> +
> +		for (; RTA_OK(rta, na); rta = RTA_NEXT(rta, na)) {
> +			if (rta->rta_type == NDA_DST) {
> +				if (memcmp(RTA_DATA(rta), ip, iplen) == 0)
> +					found = true;

Do you need this logic now that you're using the kernel filtering?  If
you do, is it guaranteed that the NDA_DST attribute will come before
the NDA_LLADDR attribute?  This code won't work if they're reversed.

> +			} else if (rta->rta_type == NDA_LLADDR && found) {
> +				memcpy(mac, RTA_DATA(rta), ETH_ALEN);

It's unusual, but the host interface could be something other than
ethernet, so you need to check the type/length of the host link level
address, in case it's not an ethernet style MAC address.

> +			}
> +		}
> +	}
> +
> +	return status;
> +}
> +
>  /**
>   * nl_addr_get_ll() - Get first IPv6 link-local address for a given interface
>   * @s:		Netlink socket
> diff --git a/netlink.h b/netlink.h
> index b51e99c..51ba49f 100644
> --- a/netlink.h
> +++ b/netlink.h
> @@ -17,6 +17,8 @@ int nl_route_dup(int s_src, unsigned int ifi_src,
>  		 int s_dst, unsigned int ifi_dst, sa_family_t af);
>  int nl_addr_get(int s, unsigned int ifi, sa_family_t af,
>  		void *addr, int *prefix_len, void *addr_l);
> +int nl_mac_get(int s, const union inany_addr *addr, int ifi,
> +	       unsigned char *mac);
>  int nl_addr_set(int s, unsigned int ifi, sa_family_t af,
>  		const void *addr, int prefix_len);
>  int nl_addr_get_ll(int s, unsigned int ifi, struct in6_addr *addr);

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]