Re: [PATCH net-next v2 2/3] netlink: specs: rt-addr: document AF_PACKET multicast dumps

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 0 (1) · Low: 1

- [Low] The new getmulticast doc text in Documentation/netlink/specs/rt-
  addr.yaml enumerates ifa-family values (AF_INET, AF_INET6, AF_PACKET)…

Pre-existing issues:
- [Medium] Documentation/netlink/specs/rt-addr.yaml declares a `do:`
  operation for getmulticast (request value 58, reply value 58), and…

--

netlink: specs: rt-addr: document AF_PACKET multicast dumps

This updates the getmulticast doc string in the rt-addr YAML spec to
describe the ifa-family values it accepts, including the newly added
AF_PACKET link-layer multicast dumps and the meaning of the permanent
flag for those entries.

> diff --git a/Documentation/netlink/specs/rt-addr.yaml b/Documentation/netlink/specs/rt-addr.yaml
> index 0ecbd24c890c..2a2310cb0df0 100644
> --- a/Documentation/netlink/specs/rt-addr.yaml
> +++ b/Documentation/netlink/specs/rt-addr.yaml
> @@ -168,7 +168,12 @@ operations:
>            attributes: *ifaddr-all
>      -
>        name: getmulticast
> -      doc: Get / dump IPv4/IPv6 multicast addresses.
> +      doc: |
> +        Get / dump multicast addresses. ifa-family selects the address
> +        family: AF_INET or AF_INET6 for the IP multicast groups joined on
> +        a device, AF_PACKET for the link-layer multicast addresses in the
> +        device filter. Link-layer entries added with SIOCADDMULTI are
> +        reported with the permanent flag set.

[Severity: Low]
The new text reads as if ifa-family were a plain family selector, but
should it also mention that an AF_UNSPEC dump does not include the
AF_PACKET entries?

The PF_PACKET handler is only reachable when the request names the family
explicitly:

net/core/rtnetlink.c:
	{.protocol = PF_PACKET, .msgtype = RTM_GETMULTICAST,
	 .dumpit = rtnl_dump_mcaddr, .flags = RTNL_FLAG_DUMP_UNLOCKED},

and the "all families" walk in rtnl_dump_all() skips it:

net/core/rtnetlink.c:rtnl_dump_all() {
	...
		if (idx < s_idx || idx == PF_PACKET)
			continue;
	...
}

So a client that sends RTM_GETMULTICAST with ifa_family = AF_UNSPEC and
NLM_F_DUMP gets only the IP multicast groups, with no error and no hint
that the device multicast filter was left out.

Since documenting the new AF_PACKET behaviour is the purpose of this
change, would it be worth stating that caveat here too?

>        attribute-set: addr-attrs
>        fixed-header: ifaddrmsg
>        do:

[Severity: Medium]
This is a pre-existing issue and not introduced by this patch, but the
doc string being reworded here still says "Get / dump", and the spec still
declares a do operation for getmulticast:

Documentation/netlink/specs/rt-addr.yaml:
      do:
        request:
          value: 58
          attributes: []
        reply:
          value: 58
          attributes: &mcaddr-attrs

Is there any family that implements a non-dump RTM_GETMULTICAST?  All
three registrations are dumpit-only:

net/core/rtnetlink.c:
	{.protocol = PF_PACKET, .msgtype = RTM_GETMULTICAST,
	 .dumpit = rtnl_dump_mcaddr, .flags = RTNL_FLAG_DUMP_UNLOCKED},

net/ipv4/devinet.c:
	{.owner = THIS_MODULE, .protocol = PF_INET, .msgtype = RTM_GETMULTICAST,
	 .dumpit = inet_dump_ifmcaddr, .flags = RTNL_FLAG_DUMP_UNLOCKED},

net/ipv6/addrconf.c:
	{.owner = THIS_MODULE, .protocol = PF_INET6, .msgtype = RTM_GETMULTICAST,
	 .dumpit = inet6_dump_ifmcaddr,
	 .flags = RTNL_FLAG_DUMP_UNLOCKED},

The non-dump dispatch path needs a doit callback, otherwise err stays
-EOPNOTSUPP:

net/core/rtnetlink.c:rtnetlink_rcv_msg() {
	...
	link = rtnl_get_link(family, type);
	if (!link || !link->doit) {
		family = PF_UNSPEC;
		link = rtnl_get_link(PF_UNSPEC, type);
		if (!link || !link->doit)
			goto out_unlock;
	}
	...
}

So a ynl-generated single-object get built from this do block fails for
every ifa-family value, including the AF_PACKET case being documented
here.  Since this change is extending the "Get" wording to AF_PACKET,
would it make sense to either drop the do block or say the operation is
dump-only?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260909013401.14408-1-sigefriedhyy%40gmail.com