Michal Prívozník wrote:
> On 4/14/26 19:55, Roman Bogorodskiy wrote:
> > Add a FreeBSD implementation of the virArpTableGet() function.
> >
> > Update the bhyve driver's bhyveDomainInterfaceAddresses()
> > to use it for the VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_ARP
> > source type.
> >
> > Signed-off-by: Roman Bogorodskiy <[email protected]>
> > ---
> > src/bhyve/bhyve_driver.c | 5 +-
> > src/util/virarptable.c | 105 ++++++++++++++++++++++++++++++++++++++-
> > 2 files changed, 107 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
> > index 93c07c6945..c8dd1a728a 100644
> > --- a/src/bhyve/bhyve_driver.c
> > +++ b/src/bhyve/bhyve_driver.c
> > @@ -1868,12 +1868,15 @@ bhyveDomainInterfaceAddresses(virDomainPtr domain,
> > goto cleanup;
> >
> > switch (source) {
> > + case VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_ARP:
> > + ret = virDomainNetARPInterfaces(vm->def, ifaces);
> > + break;
> > +
> > case VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_LEASE:
> > ret = virDomainNetDHCPInterfaces(vm->def, ifaces);
> > break;
> >
> > case VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_AGENT:
> > - case VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_ARP:
> > virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
> > _("Unsupported IP address data source %1$d"),
> > source);
> > diff --git a/src/util/virarptable.c b/src/util/virarptable.c
> > index 20d11f97b0..2c5a58b695 100644
> > --- a/src/util/virarptable.c
> > +++ b/src/util/virarptable.c
> > @@ -1,7 +1,8 @@
> > /*
> > - * virarptable.c Linux ARP table handling
> > + * virarptable.c ARP table handling
> > *
> > * Copyright (C) 2018 Chen Hanxiao
> > + * Copyright (C) 2026 The FreeBSD Foundation
> > *
> > * This library is free software; you can redistribute it and/or
> > * modify it under the terms of the GNU Lesser General Public
> > @@ -22,6 +23,16 @@
> >
> > #ifdef __linux__
> > # include <linux/rtnetlink.h>
> > +#elif defined(__FreeBSD__)
> > +# include <sys/types.h>
> > +# include <sys/socket.h>
> > +# include <sys/sysctl.h>
> > +# include <net/if.h>
> > +# include <net/if_dl.h>
> > +# include <net/if_types.h>
> > +# include <netinet/in.h>
> > +# include <net/route.h>
> > +# include <arpa/inet.h>
> > #endif
> >
> > #include "viralloc.h"
> > @@ -145,6 +156,96 @@ virArpTableGet(void)
> > return NULL;
> > }
> >
> > +#elif defined(__FreeBSD__)
> > +
> > +# define ROUNDUP(a) \
> > + ((a) > 0 ? (1 + (((a) - 1) | (sizeof(long) - 1))) : sizeof(long))
> > +# define ADVANCE(x, n) (x += ROUNDUP((n)->rtm_msglen))
> > +
> > +virArpTable *
> > +virArpTableGet(void)
> > +{
> > + int num = 0;
> > + int mib[6];
> > + size_t needed;
> > + g_autofree char *buf = NULL;
> > + char *next, *lim;
> > + struct rt_msghdr *rtm;
> > + struct sockaddr_in *sin;
> > + struct sockaddr_dl *sdl;
> > + virArpTable *table = NULL;
> > +
> > + mib[0] = CTL_NET;
> > + mib[1] = PF_ROUTE;
> > + mib[2] = 0;
> > + mib[3] = AF_INET;
> > + mib[4] = NET_RT_FLAGS;
> > + mib[5] = RTF_LLINFO;
> > +
> > + if (sysctl(mib, 6, NULL, &needed, NULL, 0) < 0) {
> > + virReportSystemError(errno, "%s", _("failed to get ARP table via
> > sysctl"));
> > + return NULL;
> > + }
> > +
> > + if (needed == 0)
> > + return NULL;
> > +
> > + buf = g_new0(char, needed);
>
> I wonder if we'd use posix_memalign() instead of g_new0() whether that
> ROUNDUP() macro is needed. I mean, looking at
> findDefaultRouteInterface() gives me some hope.
I think posix_memalign() would not replace the macro because the macro
is needed to properly iterate through variable length aligned routing messages.
With network_pf.c it's a bit different as we don't need extra
alignment logic to go through the data in the buffer. Probably it'll
even work fine with `g_new0()` instead of `posix_memalign()`.
> > +
> > + if (sysctl(mib, 6, buf, &needed, NULL, 0) < 0) {
> > + virReportSystemError(errno, "%s", _("failed to get ARP table via
> > sysctl"));
> > + return NULL;
> > + }
> > +
> > + table = g_new0(virArpTable, 1);
> > +
> > + lim = buf + needed;
> > + VIR_WARNINGS_NO_CAST_ALIGN
> > + for (next = buf; next < lim;) {
> > + rtm = (struct rt_msghdr *)next;
> > +
> > + if (rtm->rtm_msglen == 0)
> > + break;
> > +
> > + sin = (struct sockaddr_in *)(rtm + 1);
> > + sdl = (struct sockaddr_dl *)((char *)sin + ROUNDUP(sin->sin_len));
> > +
> > + if (sdl->sdl_alen &&
> > + (sdl->sdl_type == IFT_ETHER || sdl->sdl_type == IFT_BRIDGE)) {
> > + g_autofree char *ipstr = NULL;
> > + virSocketAddr virAddr = { 0 };
> > + virMacAddr macaddr;
> > + char ifmac[VIR_MAC_STRING_BUFLEN];
> > +
> > + VIR_REALLOC_N(table->t, num + 1);
> > + table->n = num + 1;
> > +
> > + virAddr.len = sizeof(virAddr.data.inet4);
> > + virAddr.data.inet4.sin_family = AF_INET;
> > + virAddr.data.inet4.sin_addr = sin->sin_addr;
> > + ipstr = virSocketAddrFormat(&virAddr);
> > +
> > + table->t[num].ipaddr = g_strdup(ipstr);
>
> I know you took inspiration from the linux version of this, but there is
> also this needless string copy. VirSocketAddrFormat() already returns an
> allocated string, so this could be g_steal_pointer(&ipstr) instead of
> g_strdup().
>
> > +
> > + memcpy(macaddr.addr, LLADDR(sdl), VIR_MAC_BUFLEN);
> > +
> > + virMacAddrFormat(&macaddr, ifmac);
> > +
> > + table->t[num].mac = g_strdup(ifmac);
> > +
> > + num++;
> > + }
> > +
> > + ADVANCE(next, rtm);
> > + }
> > + VIR_WARNINGS_RESET
> > +
> > + return table;
> > +}
> > +
> > +# undef ROUNDUP
> > +# undef ADVANCE
> > +
> > #else
> >
> > virArpTable *
> > @@ -155,7 +256,7 @@ virArpTableGet(void)
> > return NULL;
> > }
> >
> > -#endif /* __linux__ */
> > +#endif
> >
> > void
> > virArpTableFree(virArpTable *table)
>
> Reviewed-by: Michal Privoznik <[email protected]>
>
> Michal
>
