diff mbox series

[net-next] netlink: support dumping IPv4 multicast addresses

Message ID 20241217063124.3766605-1-yuyanghuang@google.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series [net-next] netlink: support dumping IPv4 multicast addresses | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/build_tools success Errors and warnings before: 0 (+23) this patch: 0 (+23)
netdev/cc_maintainers warning 1 maintainers not CCed: pruddy@vyatta.att-mail.com
netdev/build_clang success Errors and warnings before: 131 this patch: 131
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 28 this patch: 28
netdev/checkpatch fail CHECK: Blank lines aren't necessary after an open brace '{' CHECK: Comparison to NULL could be written "pmc" CHECK: Macro argument 'in_dev' may be better as '(in_dev)' to avoid precedence issues ERROR: space required before the open parenthesis '('
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 2 this patch: 2
netdev/source_inline success Was 0 now: 0
netdev/contest fail net-next-2024-12-17--21-00 (tests: 883)

Commit Message

Yuyang Huang Dec. 17, 2024, 6:31 a.m. UTC
Extended RTM_GETMULTICAST to support dumping joined IPv4 multicast
addresses, in addition to the existing IPv6 functionality. This allows
userspace applications to retrieve both IPv4 and IPv6 multicast
addresses through similar netlink command and then monitor future
changes by registering to RTNLGRP_IPV4_MCADDR and RTNLGRP_IPV6_MCADDR.

Cc: Maciej Żenczykowski <maze@google.com>
Cc: Lorenzo Colitti <lorenzo@google.com>
Signed-off-by: Yuyang Huang <yuyanghuang@google.com>
---
 include/linux/igmp.h |  7 +++++
 net/ipv4/devinet.c   | 63 +++++++++++++++++++++++++++++++++++++-------
 net/ipv4/igmp.c      | 14 ++++------
 3 files changed, 65 insertions(+), 19 deletions(-)

Comments

Eric Dumazet Dec. 17, 2024, 7:07 a.m. UTC | #1
On Tue, Dec 17, 2024 at 7:31 AM Yuyang Huang <yuyanghuang@google.com> wrote:
>
> Extended RTM_GETMULTICAST to support dumping joined IPv4 multicast
> addresses, in addition to the existing IPv6 functionality. This allows
> userspace applications to retrieve both IPv4 and IPv6 multicast
> addresses through similar netlink command and then monitor future
> changes by registering to RTNLGRP_IPV4_MCADDR and RTNLGRP_IPV6_MCADDR.

Hi Yuyang

Have you tested your patch with LOCKDEP enabled ?

CONFIG_PROVE_LOCKING=y

I think this should splat, considering your use of for_each_pmc_rtnl()
in a section where rtnl is not held.

Please make sure to use RCU variant only in the dump operation.
Yuyang Huang Dec. 17, 2024, 10:03 a.m. UTC | #2
Hi Eric

Thanks for the suggestion.

>Have you tested your patch with LOCKDEP enabled ?

>CONFIG_PROVE_LOCKING=y

>I think this should splat, considering your use of for_each_pmc_rtnl()
>in a section where rtnl is not held.

>Please make sure to use RCU variant only in the dump operation.

After turn on CONFIG_PROVE_LOCKING=y, I can see the error like follows:

net/ipv4/devinet.c:1876 suspicious rcu_dereference_protected() usage!

I will fix it properly in the next patch version.

Thanks,
Yuyang



On Tue, Dec 17, 2024 at 4:07 PM Eric Dumazet <edumazet@google.com> wrote:
>
> On Tue, Dec 17, 2024 at 7:31 AM Yuyang Huang <yuyanghuang@google.com> wrote:
> >
> > Extended RTM_GETMULTICAST to support dumping joined IPv4 multicast
> > addresses, in addition to the existing IPv6 functionality. This allows
> > userspace applications to retrieve both IPv4 and IPv6 multicast
> > addresses through similar netlink command and then monitor future
> > changes by registering to RTNLGRP_IPV4_MCADDR and RTNLGRP_IPV6_MCADDR.
>
> Hi Yuyang
>
> Have you tested your patch with LOCKDEP enabled ?
>
> CONFIG_PROVE_LOCKING=y
>
> I think this should splat, considering your use of for_each_pmc_rtnl()
> in a section where rtnl is not held.
>
> Please make sure to use RCU variant only in the dump operation.
diff mbox series

Patch

diff --git a/include/linux/igmp.h b/include/linux/igmp.h
index 073b30a9b850..1e4164f931d6 100644
--- a/include/linux/igmp.h
+++ b/include/linux/igmp.h
@@ -142,4 +142,11 @@  extern void __ip_mc_inc_group(struct in_device *in_dev, __be32 addr,
 extern void ip_mc_inc_group(struct in_device *in_dev, __be32 addr);
 int ip_mc_check_igmp(struct sk_buff *skb);
 
+#define for_each_pmc_rtnl(in_dev, pmc)				\
+	for (pmc = rtnl_dereference(in_dev->mc_list);		\
+	     pmc != NULL;					\
+	     pmc = rtnl_dereference(pmc->next_rcu))
+
+int inet_fill_ifmcaddr(struct sk_buff *skb, struct net_device *dev,
+		       const struct ip_mc_list *im, int event, int flags);
 #endif
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index c8b3cf5fba4c..d3ddb0586074 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -114,6 +114,7 @@  struct inet_fill_args {
 	unsigned int flags;
 	int netnsid;
 	int ifindex;
+	enum addr_type_t type;
 };
 
 #define IN4_ADDR_HSIZE_SHIFT	8
@@ -1850,21 +1851,44 @@  static int in_dev_dump_addr(struct in_device *in_dev, struct sk_buff *skb,
 			    struct netlink_callback *cb, int *s_ip_idx,
 			    struct inet_fill_args *fillargs)
 {
+	struct ip_mc_list *im;
 	struct in_ifaddr *ifa;
 	int ip_idx = 0;
 	int err;
 
-	in_dev_for_each_ifa_rcu(ifa, in_dev) {
-		if (ip_idx < *s_ip_idx) {
+	switch(fillargs->type) {
+	case UNICAST_ADDR:
+		fillargs->event = RTM_NEWADDR;
+		in_dev_for_each_ifa_rcu(ifa, in_dev) {
+			if (ip_idx < *s_ip_idx) {
+				ip_idx++;
+				continue;
+			}
+			err = inet_fill_ifaddr(skb, ifa, fillargs);
+			if (err < 0)
+				goto done;
+
+			nl_dump_check_consistent(cb, nlmsg_hdr(skb));
 			ip_idx++;
-			continue;
 		}
-		err = inet_fill_ifaddr(skb, ifa, fillargs);
-		if (err < 0)
-			goto done;
+		break;
+	case MULTICAST_ADDR:
+		for_each_pmc_rtnl(in_dev, im) {
+			if (ip_idx < *s_ip_idx) {
+				ip_idx++;
+				continue;
+			}
+			err = inet_fill_ifmcaddr(skb, in_dev->dev, im,
+						 RTM_GETMULTICAST, NLM_F_MULTI);
+			if (err < 0)
+				goto done;
 
-		nl_dump_check_consistent(cb, nlmsg_hdr(skb));
-		ip_idx++;
+			nl_dump_check_consistent(cb, nlmsg_hdr(skb));
+			ip_idx++;
+		}
+		break;
+	default:
+		break;
 	}
 	err = 0;
 	ip_idx = 0;
@@ -1889,15 +1913,16 @@  static u32 inet_base_seq(const struct net *net)
 	return res;
 }
 
-static int inet_dump_ifaddr(struct sk_buff *skb, struct netlink_callback *cb)
+static int inet_dump_addr(struct sk_buff *skb, struct netlink_callback *cb,
+			  enum addr_type_t type)
 {
 	const struct nlmsghdr *nlh = cb->nlh;
 	struct inet_fill_args fillargs = {
 		.portid = NETLINK_CB(cb->skb).portid,
 		.seq = nlh->nlmsg_seq,
-		.event = RTM_NEWADDR,
 		.flags = NLM_F_MULTI,
 		.netnsid = -1,
+		.type = type,
 	};
 	struct net *net = sock_net(skb->sk);
 	struct net *tgt_net = net;
@@ -1949,6 +1974,22 @@  static int inet_dump_ifaddr(struct sk_buff *skb, struct netlink_callback *cb)
 	return err;
 }
 
+static int inet_dump_ifaddr(struct sk_buff *skb, struct netlink_callback *cb)
+{
+
+	enum addr_type_t type = UNICAST_ADDR;
+
+	return inet_dump_addr(skb, cb, type);
+}
+
+static int inet_dump_ifmcaddr(struct sk_buff *skb, struct netlink_callback *cb)
+{
+
+	enum addr_type_t type = MULTICAST_ADDR;
+
+	return inet_dump_addr(skb, cb, type);
+}
+
 static void rtmsg_ifa(int event, struct in_ifaddr *ifa, struct nlmsghdr *nlh,
 		      u32 portid)
 {
@@ -2845,6 +2886,8 @@  static const struct rtnl_msg_handler devinet_rtnl_msg_handlers[] __initconst = {
 	{.protocol = PF_INET, .msgtype = RTM_GETNETCONF,
 	 .doit = inet_netconf_get_devconf, .dumpit = inet_netconf_dump_devconf,
 	 .flags = RTNL_FLAG_DOIT_UNLOCKED | RTNL_FLAG_DUMP_UNLOCKED},
+	{.owner = THIS_MODULE, .protocol = PF_INET, .msgtype = RTM_GETMULTICAST,
+	 .dumpit = inet_dump_ifmcaddr, .flags = RTNL_FLAG_DUMP_UNLOCKED},
 };
 
 void __init devinet_init(void)
diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index 8a370ef37d3f..7e085bdc1c40 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -179,11 +179,6 @@  static void ip_ma_put(struct ip_mc_list *im)
 	     pmc != NULL;					\
 	     pmc = rcu_dereference(pmc->next_rcu))
 
-#define for_each_pmc_rtnl(in_dev, pmc)				\
-	for (pmc = rtnl_dereference(in_dev->mc_list);		\
-	     pmc != NULL;					\
-	     pmc = rtnl_dereference(pmc->next_rcu))
-
 static void ip_sf_list_clear_all(struct ip_sf_list *psf)
 {
 	struct ip_sf_list *next;
@@ -1432,14 +1427,14 @@  static void ip_mc_hash_remove(struct in_device *in_dev,
 	*mc_hash = im->next_hash;
 }
 
-static int inet_fill_ifmcaddr(struct sk_buff *skb, struct net_device *dev,
-			      const struct ip_mc_list *im, int event)
+int inet_fill_ifmcaddr(struct sk_buff *skb, struct net_device *dev,
+		       const struct ip_mc_list *im, int event, int flags)
 {
 	struct ifa_cacheinfo ci;
 	struct ifaddrmsg *ifm;
 	struct nlmsghdr *nlh;
 
-	nlh = nlmsg_put(skb, 0, 0, event, sizeof(struct ifaddrmsg), 0);
+	nlh = nlmsg_put(skb, 0, 0, event, sizeof(struct ifaddrmsg), flags);
 	if (!nlh)
 		return -EMSGSIZE;
 
@@ -1464,6 +1459,7 @@  static int inet_fill_ifmcaddr(struct sk_buff *skb, struct net_device *dev,
 	nlmsg_end(skb, nlh);
 	return 0;
 }
+EXPORT_SYMBOL(inet_fill_ifmcaddr);
 
 static void inet_ifmcaddr_notify(struct net_device *dev,
 				 const struct ip_mc_list *im, int event)
@@ -1477,7 +1473,7 @@  static void inet_ifmcaddr_notify(struct net_device *dev,
 	if (!skb)
 		goto error;
 
-	err = inet_fill_ifmcaddr(skb, dev, im, event);
+	err = inet_fill_ifmcaddr(skb, dev, im, event, 0);
 	if (err < 0) {
 		WARN_ON_ONCE(err == -EMSGSIZE);
 		nlmsg_free(skb);