Re: [PATCH v2] qga: implement 'guest-network-get-route' for FreeBSD

["Daniel P. Berrangé" <berrange@redhat.com>]

On Sat, Oct 17, 2026 at 03:35:32PM +0200, Roman Bogorodskiy wrote:
> Implement 'guest-network-get-route' for FreeBSD. This
> implementation uses the sysctl(3) interface to get
> the routing table.
> 
> Limitations:
> 
>  - This implementation does not populate irtt, refcnt, use,
>    window, source, and srcprefixlen fields.
>  - The implementation uses the default FIB (routing table) 0.
>    Selecting QGA's active FIB is left for the follow-up patch.
> 
> While here, spell Windows correctly in @GuestNetworkRoute documentation
> in qapi-schema.json.
> 
> Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
> Acked-by: Markus Armbruster <armbru@redhat.com>
> ---
> Changes since v1:
> 
>  - qga/qapi-schema.json spelling: windows -> Windows, freebsd -> FreeBSD
>    in the "@GuestNetworkRoute" documentation.
>  - meson.build: check that the 'struct rt_metrics' has 'rmx_metric'
>    member which is available only on FreeBSD 16.
> 
>  meson.build          |   6 ++
>  qga/commands-bsd.c   | 180 +++++++++++++++++++++++++++++++++++++++++++
>  qga/qapi-schema.json |  31 ++++----
>  3 files changed, 202 insertions(+), 15 deletions(-)
> 
> diff --git a/meson.build b/meson.build
> index 255c887e8e..cb69d5a3c7 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -2759,6 +2759,12 @@ config_host_data.set('HAVE_STRUCT_STAT_ST_ATIM',
>  config_host_data.set('HAVE_BLK_ZONE_REP_CAPACITY',
>                       cc.has_member('struct blk_zone', 'capacity',
>                                     prefix: '#include <linux/blkzoned.h>'))
> +config_host_data.set('HAVE_STRUCT_RT_METRICS_RMX_METRIC',
> +                     cc.has_member('struct rt_metrics', 'rmx_metric',
> +                                   prefix: '''
> +                                       #include <sys/types.h>
> +                                       #include <sys/socket.h>
> +                                       #include <net/route.h>'''))
>  
>  # has_type
>  config_host_data.set('CONFIG_IOVEC',
> diff --git a/qga/commands-bsd.c b/qga/commands-bsd.c
> index 1c51d099eb..63254e0812 100644
> --- a/qga/commands-bsd.c
> +++ b/qga/commands-bsd.c
> @@ -21,6 +21,11 @@
>  #include <sys/mount.h>
>  #include <net/if_dl.h>
>  #include <sys/statvfs.h>
> +#if defined(__FreeBSD__)
> +#include <sys/sysctl.h>
> +#include <net/route.h>
> +#include <net/if.h>
> +#endif
>  #if defined(__NetBSD__) || defined(__OpenBSD__)
>  #include <net/if_arp.h>
>  #include <netinet/if_ether.h>
> @@ -228,4 +233,179 @@ GuestFilesystemInfoList *qmp_guest_get_fsinfo(Error **errp)
>      free_fs_mount_list(&mounts);
>      return ret;
>  }
> +
> +GuestNetworkRouteList *qmp_guest_network_get_route(Error **errp)
> +{
> +    GuestNetworkRouteList *head = NULL, **tail = &head;
> +    int mib[] = { CTL_NET, PF_ROUTE, 0, AF_UNSPEC, NET_RT_DUMP, 0, 0 };
> +    g_autofree char *buf = NULL;
> +    size_t len;
> +
> +    if (sysctl(mib, nitems(mib), NULL, &len, NULL, 0) == -1) {
> +        error_setg_errno(errp, errno, "sysctl size");
> +        return NULL;
> +    }
> +
> +    buf = g_malloc0(len);
> +
> +    if (sysctl(mib, nitems(mib), buf, &len, NULL, 0) == -1) {
> +        error_setg_errno(errp, errno, "sysctl routes");
> +        return NULL;
> +    }
> +
> +    for (char *p = buf, *end = buf + len; p < end; ) {
> +        struct rt_msghdr *rtm = (struct rt_msghdr *)p;
> +        struct sockaddr *addresses[RTAX_MAX] = { NULL };
> +        struct sockaddr *sa;
> +        char destination[NI_MAXHOST];
> +        char gateway[NI_MAXHOST];
> +        char ifname[IFNAMSIZ] = "?";

IIUC, by s3etting this to "?" you're expecting it to be
left alone if ....

> +        const struct sockaddr *dst = NULL;
> +        const struct sockaddr *gw = NULL;
> +        const struct sockaddr *netmask = NULL;
> +        struct sockaddr_in normalized_mask = { 0 };
> +        char netmask_buf[INET_ADDRSTRLEN];
> +
> +        g_autoptr(GuestNetworkRoute) route = g_new0(GuestNetworkRoute, 1);
> +
> +        if ((size_t)(end - p) < sizeof(*rtm) ||
> +            rtm->rtm_msglen < sizeof(*rtm) ||
> +            rtm->rtm_msglen > (size_t)(end - p)) {
> +            break;
> +        }
> +
> +        p += rtm->rtm_msglen;
> +
> +        if (rtm->rtm_version != RTM_VERSION) {
> +            continue;
> +        }
> +
> +        sa = (struct sockaddr *)(rtm + 1);
> +        for (int i = 0; i < RTAX_MAX; i++) {
> +            if (rtm->rtm_addrs & (1U << i)) {
> +                addresses[i] = sa;
> +                sa = (struct sockaddr *)((char *)sa + SA_SIZE(sa));
> +            }
> +        }
> +
> +        if_indextoname(rtm->rtm_index, ifname);

....this fails to resolve the interface name. 

> +
> +        route->iface = g_strdup(ifname);

This is a non-optional field so we must return something on
failure, and I see that the Windows impl returns a string
based on the if index. eg matching logic would be more
like:

   if (if_indextoname(rtm->rtm_index, ifname) > 0) {
      route->iface = g_strdup(ifname)
   } else {
      route->iface = g_strdup_printf("if%u", rtm->rtm_index);
   }


> +        if (dst->sa_family == AF_INET) {
> +            if (inet_ntop(AF_INET,
> +                          &((const struct sockaddr_in *)dst)->sin_addr,
> +                          destination, NI_MAXHOST) == NULL) {
> +                continue;
> +            }
> +            route->version = 4;
> +        } else if (dst->sa_family == AF_INET6) {
> +            if (getnameinfo(dst, dst->sa_len,
> +                            destination, NI_MAXHOST,
> +                            NULL, 0, NI_NUMERICHOST) != 0)
> +                continue;
> +            route->version = 6;

Do you need to have different code paths for AF_INET
vs AF_INET6 ?   IIUC,    getnameinfo() should handle
both  AF_INET  and AF_INET6 the same way, so we ought
to not need inet_ntop() path at all.

> +        } else {
> +            continue;
> +        }
> +        route->destination = g_strdup(destination);
> +
> +        gw = addresses[RTAX_GATEWAY];
> +        if (gw) {
> +            if (gw->sa_family == AF_INET) {
> +                if (inet_ntop(AF_INET,
> +                              &((const struct sockaddr_in *)gw)->sin_addr,
> +                              gateway, NI_MAXHOST) == NULL) {
> +                    continue;
> +                }
> +                route->gateway = g_strdup(gateway);
> +            } else if (gw->sa_family == AF_INET6) {
> +                if (getnameinfo(gw, gw->sa_len,
> +                                gateway, NI_MAXHOST,
> +                                NULL, 0, NI_NUMERICHOST) != 0) {
> +                    continue;
> +                }
> +                route->nexthop = g_strdup(gateway);
> +            }

Same Q about using getnameinfo() exclusively

> +        }
> +
> +        netmask = addresses[RTAX_NETMASK];
> +        if (dst->sa_family == AF_INET) {
> +            if (netmask && netmask->sa_len != 0) {
> +                memcpy(&normalized_mask, netmask,
> +                       MIN(netmask->sa_len, sizeof(normalized_mask)));
> +            } else if (rtm->rtm_flags & RTF_HOST) {
> +                normalized_mask.sin_addr.s_addr = 0xFFFFFFFF;
> +            }
> +
> +            if (inet_ntop(AF_INET, &normalized_mask.sin_addr,
> +                          netmask_buf, sizeof(netmask_buf)) != NULL) {
> +                route->mask = g_strdup(netmask_buf);
> +            } else {
> +                route->mask = NULL;
> +            }
> +        } else if (dst->sa_family == AF_INET6) {
> +            struct sockaddr_in6 normalized_mask6;
> +            memset(&normalized_mask6, 0, sizeof(normalized_mask6));
> +
> +            if (netmask && netmask->sa_len != 0) {
> +                int prefix_len = 0;
> +                bool valid_mask = true;
> +                bool zero_found = false;
> +
> +                memcpy(&normalized_mask6, netmask,
> +                       MIN(netmask->sa_len, sizeof(normalized_mask6)));
> +
> +                for (int i = 0; i < sizeof(normalized_mask6.sin6_addr); i++) {
> +                    uint8_t byte = normalized_mask6.sin6_addr.s6_addr[i];
> +                    for (int bit = 7; bit >= 0; bit--) {
> +                        if ((byte >> bit) & 1) {
> +                            if (zero_found) {
> +                                valid_mask = false;
> +                                break;
> +                            }
> +                            prefix_len++;
> +                        } else {
> +                            zero_found = true;
> +                        }
> +                    }
> +                    if (!valid_mask) {
> +                        break;
> +                    }
> +                }
> +
> +                if (valid_mask) {
> +                    route->desprefixlen = g_strdup_printf("%d", prefix_len);
> +                } else {
> +                    route->desprefixlen = NULL;
> +                }
> +            } else if (rtm->rtm_flags & RTF_HOST) {
> +                route->desprefixlen = g_strdup("128");
> +            } else {
> +                route->desprefixlen = g_strdup("0");
> +            }
> +        }
> +
> +        if (rtm->rtm_rmx.rmx_mtu) {
> +            route->mtu = rtm->rtm_rmx.rmx_mtu;
> +            route->has_mtu = true;
> +        }
> +
> +        route->flags = (uint32_t)rtm->rtm_flags;
> +        route->has_flags = true;
> +
> +        QAPI_LIST_APPEND(tail, route);
> +        route = NULL;
> +    }
> +
> +    return head;
> +}
>  #endif /* __FreeBSD__ */


With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|