Date:   Wed, 11 Mar 2020 14:49:53 +0900
From:   Masahiro Yamada <masahiroy@...nel.org>
To:     Neil Horman <nhorman@...driver.com>, netdev@...r.kernel.org,
        "David S . Miller" <davem@...emloft.net>
Cc:     Ido Schimmel <idosch@...lanox.com>, Jiri Pirko <jiri@...lanox.com>,
        Stephen Rothwell <sfr@...b.auug.org.au>,
        Nicolas Pitre <nico@...xnic.net>, linux-kbuild@...r.kernel.org,
        Masahiro Yamada <masahiroy@...nel.org>,
        Jakub Kicinski <kuba@...nel.org>, linux-kernel@...r.kernel.org
Subject: [PATCH] net: drop_monitor: make drop_monitor built-in

In net/Kconfig, NET_DEVLINK implies NET_DROP_MONITOR.

The original behavior of the 'imply' keyword prevents NET_DROP_MONITOR
from being 'm' when NET_DEVLINK=y.

With the planned Kconfig change that relaxes the 'imply', the
combination of NET_DEVLINK=y and NET_DROP_MONITOR=m would be allowed,
causing a link error of vmlinux.

As far as I see the mainline code, NET_DROP_MONITOR=m does not provide
any useful case.

The call-site of net_dm_hw_report() only exists in net/core/devlink.c,
which is always built-in since NET_DEVLINK is a bool type option.

So, NET_DROP_MONITOR=m causes a build error, or creates an unused
module at best.

Make NET_DROP_MONITOR a bool option, and remove the module exit code.
I also unexported net_dm_hw_report because I see no other call-site
in upstream.

Reported-by: Stephen Rothwell <sfr@...b.auug.org.au>
Signed-off-by: Masahiro Yamada <masahiroy@...nel.org>
---

This build error was reported in linux-next.
https://lkml.org/lkml/2020/3/10/1936

A less invasive change would be to change
IS_ENABLED(CONFIG_NET_DROP_MONITOR) in include/net/drop_monitor.h
to IS_REACHABLE(CONFIG_NET_DROP_MONITOR).

If you want to keep this modular, it is fine too.

If this patch is acceptable,
I'd like to get Ack from the maintainers,
and insert this patch before my Kconfig change.


 include/net/drop_monitor.h |  2 +-
 net/Kconfig                |  2 +-
 net/core/drop_monitor.c    | 56 ++------------------------------------
 3 files changed, 4 insertions(+), 56 deletions(-)

diff --git a/include/net/drop_monitor.h b/include/net/drop_monitor.h
index 2ab668461463..aa775f243b61 100644
--- a/include/net/drop_monitor.h
+++ b/include/net/drop_monitor.h
@@ -19,7 +19,7 @@ struct net_dm_hw_metadata {
 	struct net_device *input_dev;
 };
 
-#if IS_ENABLED(CONFIG_NET_DROP_MONITOR)
+#ifdef CONFIG_NET_DROP_MONITOR
 void net_dm_hw_report(struct sk_buff *skb,
 		      const struct net_dm_hw_metadata *hw_metadata);
 #else
diff --git a/net/Kconfig b/net/Kconfig
index 2eeb0e55f7c9..6ad5d3e95be6 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -347,7 +347,7 @@ config NET_PKTGEN
 	  module will be called pktgen.
 
 config NET_DROP_MONITOR
-	tristate "Network packet drop alerting service"
+	bool "Network packet drop alerting service"
 	depends on INET && TRACEPOINTS
 	---help---
 	  This feature provides an alerting service to userspace in the
diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c
index 31700e0c3928..25466b7a0176 100644
--- a/net/core/drop_monitor.c
+++ b/net/core/drop_monitor.c
@@ -13,6 +13,7 @@
 #include <linux/if_arp.h>
 #include <linux/inetdevice.h>
 #include <linux/inet.h>
+#include <linux/init.h>
 #include <linux/interrupt.h>
 #include <linux/netpoll.h>
 #include <linux/sched.h>
@@ -25,7 +26,6 @@
 #include <linux/timer.h>
 #include <linux/bitops.h>
 #include <linux/slab.h>
-#include <linux/module.h>
 #include <net/drop_monitor.h>
 #include <net/genetlink.h>
 #include <net/netevent.h>
@@ -962,7 +962,6 @@ void net_dm_hw_report(struct sk_buff *skb,
 out:
 	rcu_read_unlock();
 }
-EXPORT_SYMBOL_GPL(net_dm_hw_report);
 
 static int net_dm_hw_monitor_start(struct netlink_ext_ack *extack)
 {
@@ -1581,11 +1580,6 @@ static void __net_dm_cpu_data_init(struct per_cpu_dm_data *data)
 	u64_stats_init(&data->stats.syncp);
 }
 
-static void __net_dm_cpu_data_fini(struct per_cpu_dm_data *data)
-{
-	WARN_ON(!skb_queue_empty(&data->drop_queue));
-}
-
 static void net_dm_cpu_data_init(int cpu)
 {
 	struct per_cpu_dm_data *data;
@@ -1594,18 +1588,6 @@ static void net_dm_cpu_data_init(int cpu)
 	__net_dm_cpu_data_init(data);
 }
 
-static void net_dm_cpu_data_fini(int cpu)
-{
-	struct per_cpu_dm_data *data;
-
-	data = &per_cpu(dm_cpu_data, cpu);
-	/* At this point, we should have exclusive access
-	 * to this struct and can free the skb inside it.
-	 */
-	consume_skb(data->skb);
-	__net_dm_cpu_data_fini(data);
-}
-
 static void net_dm_hw_cpu_data_init(int cpu)
 {
 	struct per_cpu_dm_data *hw_data;
@@ -1614,15 +1596,6 @@ static void net_dm_hw_cpu_data_init(int cpu)
 	__net_dm_cpu_data_init(hw_data);
 }
 
-static void net_dm_hw_cpu_data_fini(int cpu)
-{
-	struct per_cpu_dm_data *hw_data;
-
-	hw_data = &per_cpu(dm_hw_cpu_data, cpu);
-	kfree(hw_data->hw_entries);
-	__net_dm_cpu_data_fini(hw_data);
-}
-
 static int __init init_net_drop_monitor(void)
 {
 	int cpu, rc;
@@ -1661,29 +1634,4 @@ static int __init init_net_drop_monitor(void)
 out:
 	return rc;
 }
-
-static void exit_net_drop_monitor(void)
-{
-	int cpu;
-
-	BUG_ON(unregister_netdevice_notifier(&dropmon_net_notifier));
-
-	/*
-	 * Because of the module_get/put we do in the trace state change path
-	 * we are guarnateed not to have any current users when we get here
-	 */
-
-	for_each_possible_cpu(cpu) {
-		net_dm_hw_cpu_data_fini(cpu);
-		net_dm_cpu_data_fini(cpu);
-	}
-
-	BUG_ON(genl_unregister_family(&net_drop_monitor_family));
-}
-
-module_init(init_net_drop_monitor);
-module_exit(exit_net_drop_monitor);
-
-MODULE_LICENSE("GPL v2");
-MODULE_AUTHOR("Neil Horman <nhorman@...driver.com>");
-MODULE_ALIAS_GENL_FAMILY("NET_DM");
+device_initcall(init_net_drop_monitor);
-- 
2.17.1

Powered by blists - more mailing lists