From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: passt.top; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=BFxR4wZj; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by passt.top (Postfix) with ESMTPS id 4BD295A028A for ; Thu, 12 Jun 2025 17:17:18 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1749741437; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Of2i5FQ840txAXbDIGQIVH34RlasoNEDK5z5n07r4i4=; b=BFxR4wZj8dFdtwG/a7wwbIa2QH5r2OUPAukPXsMIPpCgx27VJH6p5t5ABF3pmSYF2rmxgO e2hDbKWlkGPm+zMuDFBZ2yiMuKDtg1bWXXjC3vCpys/rFOf7JYV5Y7C6CHlVktSW9P7rbh flp39tvCbmlVC6XkMvGy4rhDcPtHgOw= Received: from mail-wm1-f71.google.com (mail-wm1-f71.google.com [209.85.128.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-507-u4ns0KPAPACgQ-PhPpqLRw-1; Thu, 12 Jun 2025 11:17:15 -0400 X-MC-Unique: u4ns0KPAPACgQ-PhPpqLRw-1 X-Mimecast-MFC-AGG-ID: u4ns0KPAPACgQ-PhPpqLRw_1749741434 Received: by mail-wm1-f71.google.com with SMTP id 5b1f17b1804b1-4530ec2c87cso5405565e9.0 for ; Thu, 12 Jun 2025 08:17:15 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1749741434; x=1750346234; h=content-transfer-encoding:mime-version:organization:references :in-reply-to:message-id:subject:cc:to:from:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=Of2i5FQ840txAXbDIGQIVH34RlasoNEDK5z5n07r4i4=; b=RsfaPZvsv7E4t3FhQbIs0p8UZBMXF66vOTQJIprzyUL2C0HJmSr/Dn7+LdYZIAwdQ2 wspuC7pKSWL27pmJDopdjWlOCIQnh/oobr1Kf3q7mnUEewzkmxoemBIIfZDWNcUapXuH MDENpgNAwiYJDs0UXhz6dfXxn/NCNs9pIUXU4CFeecWTTsZ5yCD+5f2mAqkyIEQZl6Vk h3INzxld0r4kHWLEEEqNp9BvBsf9tlpwN4T/5ijU4gbNVge0OAOWpaTbH82pd06uiqFm AjV62Idfj4Xy1NEcgwD0NuHyuFGEKCVCtKeNhbAYT5Q23Y5nccCZDKvPJr8eR/xVINvS N3jg== X-Forwarded-Encrypted: i=1; AJvYcCWEesY7jB4YyCRRvLizgGXeu5/TV7xNkPesfiHqwkGtrakyYDptPK104HH/fbUKZHDutYRvxnaL2BI=@passt.top X-Gm-Message-State: AOJu0YyNkqZvnKlntPUpsZbP9P3HkAHLWX14h173DZckADq0CBBztvwf 5hZpV2G0sepNWagjzk+JY8Zu+f9jBS+IRJ9pbAPJ+gpIFOIt1FJmMR5uJyFG1oxliNk4EWVm5tt Aekhcon5QacvFPCmFP+oCpPBxABVygr0ylBNhpBCIFh5E/XfvNotrLdgRNlRwJTITXC1QxRVPQQ 3IQZf2ocLK9Do/2sGoPKB32acs741YEldeIHmF X-Gm-Gg: ASbGncv7LVaGvrGt28VqI9MYHZl9RJ4guy1q1c/MTxm9oQ8zK27XVkKktp4nI/SVv8C vkqGDYQ8txGe+znr+AeN+JAn1CRMitU61+sv/QwIzWxkwKFvJyRgIoyIwuKE9HnzORxD7X+48Y2 Opd74hsDXockIt3z37zOeTk9VNfBw4MGyNN/ukxGUfT4lm3EsOLBP4/CxFc+eOTIltd+9uHifu4 t+1GBCWz8gr+ysXeuDb/OLwPBFWdnWHPUEgos6ZNyBIwyRVQ3b4pp612a+lHRRDB/rHG1qTybiX 9vRCcx8eilQfSoP25Bh8vt3DXkAK5ck6BQ== X-Received: by 2002:a05:600c:c4a4:b0:450:d01f:de6f with SMTP id 5b1f17b1804b1-4532b916237mr47501075e9.15.1749741433574; Thu, 12 Jun 2025 08:17:13 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEegLqAz60Ei/81Pti3YymfFyQeF0pcC1BiAdyraSSmU1PPXU/Q3PicA3+jTaemDxW7HcVG3w== X-Received: by 2002:a05:600c:c4a4:b0:450:d01f:de6f with SMTP id 5b1f17b1804b1-4532b916237mr47500375e9.15.1749741432836; Thu, 12 Jun 2025 08:17:12 -0700 (PDT) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [2a10:fc81:a806:d6a9::1]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4532e244392sm23391395e9.22.2025.06.12.08.17.09 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 12 Jun 2025 08:17:09 -0700 (PDT) Date: Thu, 12 Jun 2025 17:17:05 +0200 From: Stefano Brivio To: Jon Maloy Subject: Re: [PATCH v2 1/8] netlink: Add function to extract mac addresses from arp table Message-ID: <20250612171705.0594ffa4@elisabeth> In-Reply-To: <20250612042152.695879-2-jmaloy@redhat.com> References: <20250612042152.695879-1-jmaloy@redhat.com> <20250612042152.695879-2-jmaloy@redhat.com> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.49; x86_64-pc-linux-gnu) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: SPywRIZOQLvCQOk7Z33yZ6bLvHlZYFJP-tHWPQkmfWo_1749741434 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: D72NWB4NDO5QSHVIVH632FNLHQNAPFQE X-Message-ID-Hash: D72NWB4NDO5QSHVIVH632FNLHQNAPFQE X-MailFrom: sbrivio@redhat.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: dgibson@redhat.com, passt-dev@passt.top X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Thu, 12 Jun 2025 00:21:45 -0400 Jon Maloy wrote: > The solution to bug #120 requires the ability to translate from > an IP address to its corresponding MAC address in cases where > those are present in the ARP/NTP table. Nit: NDP. > We add this feature here. > > Signed-off-by: Jon Maloy Nit: David's address is david@gibson.dropbear.id.au (in case you need to Cc: him specifically). Nit: while bug #120 isn't fixed at once by one specific patch in this series, I would rather be liberal with Link: tags, better a few than nothing, should we ever need to look up commits in the future. That is, Link: https://bugs.passt.top/show_bug.cgi?id=120 would fit nicely here and in a few other patches, I think. > --- > netlink.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ > netlink.h | 1 + > 2 files changed, 59 insertions(+) > > diff --git a/netlink.c b/netlink.c > index ee9325a..6c55c4c 100644 > --- a/netlink.c > +++ b/netlink.c > @@ -800,6 +800,64 @@ 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 Nit: MAC address > + * @s: Netlink socket > + * @addr: IPv4 or IPv6 address > + * @mac: Array to place the returned MAC address > + * > + * Return: 0 on success, negative error code on failure Kind of. As far as I understand, this also returns 0 if the MAC address is not found. Should we perhaps set @mac to all-zeroes in that case, and say "0 if found or not in table, ..."? > + */ > +int nl_mac_get(int s, const union inany_addr *addr, unsigned char *mac) > +{ > + struct nlmsghdr *nlh; > + char buf[NLBUFSIZ]; > + const void *ip; I guess you should be able to use inany_equals() and keep addresses as inany_addr as much as possible. > + ssize_t status; > + uint32_t seq; > + int alen; > + struct { > + struct nlmsghdr nlh; > + struct ndmsg ndm; > + } req; You can actually ask the kernel to do the filtering for you (as long as NETLINK_GET_STRICT_CHK is available, >= 4.20), by passing a NDA_DST attribute in the request. See e.g. 'strace ip neigh get ... dev ...'. As far as I know, even if the device (ndm_ifindex) is mandatory in ip-neighbour(8), the kernel doesn't actually need it. By the way, I'm not sure if we want to give a preference to a particular interface, in case we have different MAC addresses for the same IP address on different interfaces. I would say we don't care and we can just pick one because we don't always have the inbound interface available anyway, but I haven't really thought it through. > + > + if (IN6_IS_ADDR_V4MAPPED(&addr->a6)) { > + ip = &addr->v4mapped.a4; > + alen = sizeof(struct in_addr); > + req.ndm.ndm_family = AF_INET; > + } else { > + ip = &addr->a6; > + alen = sizeof(struct in6_addr); > + req.ndm.ndm_family = AF_INET6; > + } > + > + seq = nl_send(s, &req, RTM_GETNEIGH, NLM_F_DUMP, sizeof(req)); > + nl_foreach_oftype(nlh, status, s, buf, seq, RTM_NEWNEIGH) { > + struct ndmsg *ndm = NLMSG_DATA(nlh); > + struct rtattr *attr = (struct rtattr *)(ndm + 1); > + int attrlen = nlh->nlmsg_len - NLMSG_LENGTH(sizeof(*ndm)); > + unsigned char *lladdr = NULL; > + void *neigh_ip = NULL; cppcheck says: netlink.c:839:18: style: Variable 'lladdr' can be declared as pointer to const [constVariablePointer] unsigned char *lladdr = NULL; ^ netlink.c:840:9: style: Variable 'neigh_ip' can be declared as pointer to const [constVariablePointer] void *neigh_ip = NULL; ^ Try 'make cppcheck' (or running all the tests, that would be even better). > + > + for (; RTA_OK(attr, attrlen); attr = RTA_NEXT(attr, attrlen)) { > + if (attr->rta_type == NDA_DST) > + neigh_ip = RTA_DATA(attr); > + else if (attr->rta_type == NDA_LLADDR) > + lladdr = RTA_DATA(attr); > + } > + > + if (!neigh_ip || !lladdr) > + continue; > + > + if (!memcmp(neigh_ip, ip, alen)) { ...the filtering is still needed for kernel versions < 4.20 (we also do it in other functions, such as nl_route_dup()), but in general it should be unnecessary. That's important to avoid huge netlink messages and substantial overhead in case we have a lot of neighbours in the table. > + memcpy(mac, lladdr, ETH_ALEN); > + return 0; > + } > + } > + > + 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..2f674d7 100644 > --- a/netlink.h > +++ b/netlink.h > @@ -17,6 +17,7 @@ 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, 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); -- Stefano