Re: git: 2f8f892ca344 - main - rtnetlink: Add FreeBSD-specific IFLAF_GROUP support

From: Kevin Bowling <kevin.bowling_at_kev009.com>
Date: Mon, 31 Aug 2026 15:23:32 UTC
On Thu, Aug 27, 2026 at 6:28 AM Pouria Mousavizadeh Tehrani
<pouria@freebsd.org> wrote:
>
> The branch main has been updated by pouria:
>
> URL: https://cgit.FreeBSD.org/src/commit/?id=2f8f892ca344d884abbed0305886bb532f16368f
>
> commit 2f8f892ca344d884abbed0305886bb532f16368f
> Author:     Pouria Mousavizadeh Tehrani <pouria@FreeBSD.org>
> AuthorDate: 2026-08-27 12:43:28 +0000
> Commit:     Pouria Mousavizadeh Tehrani <pouria@FreeBSD.org>
> CommitDate: 2026-08-27 13:04:53 +0000
>
>     rtnetlink: Add FreeBSD-specific IFLAF_GROUP support
>
>     Netlink IFLA_GROUP works with a single group id, in our
>     implementation an interface can be joined to multiple groups
>     and it works with group name.
>     Store interface groups in IFLAF_GROUP attribute.
>
>     Reviewed by:    glebius, melifaro
>     Discussed with: markj
>     Differential Revision: https://reviews.freebsd.org/D58643
> ---
>  sys/net/if.c                            | 24 ++++++++++++++++++++++
>  sys/net/if_var.h                        |  2 ++
>  sys/netlink/netlink_snl_route_parsers.h |  8 ++++++++
>  sys/netlink/route/iface.c               | 35 ++++++++++++++++++++++++++++++++-
>  sys/netlink/route/interface.h           |  1 +
>  5 files changed, 69 insertions(+), 1 deletion(-)
>
> diff --git a/sys/net/if.c b/sys/net/if.c
> index 1c57eb1492c2..644a039c8d25 100644
> --- a/sys/net/if.c
> +++ b/sys/net/if.c
> @@ -1468,6 +1468,30 @@ if_delgroups(struct ifnet *ifp)
>         IFNET_WUNLOCK();
>  }
>
> +/*
> + * XXX: This KPI should not expose ifg_group. therefore the current
> + * implementation is questionable and may change in the future.
> + */
> +int
> +if_foreach_group(struct ifnet *ifp, if_foreach_group_cb_t cb, void *cb_arg)
> +{
> +       struct ifg_list *ifgl;
> +       int error;
> +
> +       MPASS(cb);
> +
> +       error = 0;
> +       IFNET_RLOCK();

I'm seeing lock recursion panics under WITNESS if I create and destroy
interfaces with this change.  Please see
https://reviews.freebsd.org/D59289 for a proposed fix.

> +       CK_STAILQ_FOREACH(ifgl, &ifp->if_groups, ifgl_next) {
> +               error = cb(ifgl->ifgl_group, cb_arg);
> +               if (error != 0)
> +                       break;
> +       }
> +       IFNET_RUNLOCK();
> +
> +       return (error);
> +}
> +
>  /*
>   * Stores all groups from an interface in memory pointed to by ifgr.
>   */
> diff --git a/sys/net/if_var.h b/sys/net/if_var.h
> index 9a425fd81054..d3d4b1e2a36c 100644
> --- a/sys/net/if_var.h
> +++ b/sys/net/if_var.h
> @@ -525,6 +525,8 @@ VNET_DECLARE(if_t, loif);   /* first loopback interface */
>  #define MCDPRINTF(...)
>  #endif
>
> +typedef int (*if_foreach_group_cb_t)(struct ifg_group *, void *);
> +int    if_foreach_group(if_t, if_foreach_group_cb_t, void *);
>  int    if_addgroup(if_t, const char *);
>  int    if_delgroup(if_t, const char *);
>  int    if_addmulti(if_t, struct sockaddr *, struct ifmultiaddr **);
> diff --git a/sys/netlink/netlink_snl_route_parsers.h b/sys/netlink/netlink_snl_route_parsers.h
> index 10698ca987f3..495dee5ec862 100644
> --- a/sys/netlink/netlink_snl_route_parsers.h
> +++ b/sys/netlink/netlink_snl_route_parsers.h
> @@ -174,6 +174,11 @@ SNL_DECLARE_PARSER_EXT(snl_rtm_route_parser, sizeof(struct rtmsg),
>                 sizeof(struct snl_parsed_route), _fp_p_route, _nla_p_route,
>                 _cb_p_route);
>
> +static const struct snl_attr_parser _nla_p_ifgroups[] = {
> +       { .type = IFLAF_GROUP, .cb = snl_attr_copy_string, .arg_u32 = sizeof(char[IFNAMSIZ]) },
> +};
> +SNL_DECLARE_ATTR_PARSER_EXT(_ifgroups_parser, sizeof(char[IFNAMSIZ]), _nla_p_ifgroups, NULL);
> +
>  /* RTM_<NEW|DEL|GET>LINK message parser */
>  struct snl_parsed_link {
>         uint32_t                        ifi_index;
> @@ -191,6 +196,7 @@ struct snl_parsed_link {
>         struct rtnl_link_stats64        *ifla_stats64;
>         struct nlattr                   *iflaf_orig_hwaddr;
>         struct snl_attr_bitset          iflaf_caps;
> +       struct snl_parray               iflaf_groups;
>  };
>
>  #define        _IN(_field)     offsetof(struct ifinfomsg, _field)
> @@ -198,6 +204,8 @@ struct snl_parsed_link {
>  static const struct snl_attr_parser _nla_p_link_fbsd[] = {
>         { .type = IFLAF_ORIG_HWADDR, .off = _OUT(iflaf_orig_hwaddr), .cb = snl_attr_dup_nla },
>         { .type = IFLAF_CAPS, .off = _OUT(iflaf_caps), .cb = snl_attr_get_bitset_c },
> +       { .type = IFLAF_GROUP, .off = _OUT(iflaf_groups), .cb = snl_attr_get_parray,
> +               .arg = &_ifgroups_parser },
>  };
>  SNL_DECLARE_ATTR_PARSER(_link_fbsd_parser, _nla_p_link_fbsd);
>
> diff --git a/sys/netlink/route/iface.c b/sys/netlink/route/iface.c
> index 5b6e58a598aa..50dd35b74479 100644
> --- a/sys/netlink/route/iface.c
> +++ b/sys/netlink/route/iface.c
> @@ -204,6 +204,38 @@ get_hwaddr(struct nl_writer *nw, if_t ifp)
>         }
>  }
>
> +static int
> +dump_group_cb(struct ifg_group *ifg, void *_arg)
> +{
> +       struct nl_writer *nw = (struct nl_writer *)_arg;
> +       int off, ret = 0;
> +
> +       off = nlattr_add_nested(nw, IFLAF_GROUP);
> +       if (off == 0)
> +               return (ENOMEM);
> +
> +       if (!nlattr_add_string(nw, IFLAF_GROUP, ifg->ifg_group))
> +               ret = ENOMEM;
> +
> +       nlattr_set_len(nw, off);
> +       return (ret);
> +}
> +
> +static int
> +dump_group(struct nl_writer *nw, if_t ifp)
> +{
> +       int off, ret;
> +
> +       off = nlattr_add_nested(nw, IFLAF_GROUP);
> +       if (off == 0)
> +               return (ENOMEM);
> +
> +       ret = if_foreach_group(ifp, dump_group_cb, nw);
> +       nlattr_set_len(nw, off);
> +
> +       return (ret);
> +}
> +
>  static unsigned
>  ifp_flags_to_netlink(const if_t ifp)
>  {
> @@ -344,7 +376,6 @@ dump_iface(struct nl_writer *nw, if_t ifp, const struct nlmsghdr *hdr,
>  /*
>          nlattr_add_u32(nw, IFLA_MIN_MTU, 60);
>          nlattr_add_u32(nw, IFLA_MAX_MTU, 9000);
> -        nlattr_add_u32(nw, IFLA_GROUP, 0);
>  */
>
>         if (if_getdescr(ifp) != NULL)
> @@ -355,6 +386,8 @@ dump_iface(struct nl_writer *nw, if_t ifp, const struct nlmsghdr *hdr,
>         if (off != 0) {
>                 get_hwaddr(nw, ifp);
>                 dump_iface_caps(nw, ifp);
> +               if (dump_group(nw, ifp) != 0)
> +                       goto enomem;
>
>                 nlattr_set_len(nw, off);
>         }
> diff --git a/sys/netlink/route/interface.h b/sys/netlink/route/interface.h
> index c018a0adb20d..4394dcfef21d 100644
> --- a/sys/netlink/route/interface.h
> +++ b/sys/netlink/route/interface.h
> @@ -152,6 +152,7 @@ enum {
>         IFLAF_ORIG_IFNAME       = 1,    /* string, original interface name at creation */
>         IFLAF_ORIG_HWADDR       = 2,    /* binary, original hardware address */
>         IFLAF_CAPS              = 3,    /* bitset, interface capabilities */
> +       IFLAF_GROUP             = 4,    /* string, interface group name (multi-attr) */
>         __IFLAF_MAX
>  };
>  #define IFLAF_MAX (__IFLAF_MAX - 1)
>