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
