LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] net: drop_monitor: make drop_monitor built-in
@ 2020-03-11  5:49 Masahiro Yamada
  2020-03-11  6:30 ` Masahiro Yamada
  0 siblings, 1 reply; 2+ messages in thread
From: Masahiro Yamada @ 2020-03-11  5:49 UTC (permalink / raw)
  To: Neil Horman, netdev, David S . Miller
  Cc: Ido Schimmel, Jiri Pirko, Stephen Rothwell, Nicolas Pitre,
	linux-kbuild, Masahiro Yamada, Jakub Kicinski, linux-kernel

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@canb.auug.org.au>
Signed-off-by: Masahiro Yamada <masahiroy@kernel.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@tuxdriver.com>");
-MODULE_ALIAS_GENL_FAMILY("NET_DM");
+device_initcall(init_net_drop_monitor);
-- 
2.17.1


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] net: drop_monitor: make drop_monitor built-in
  2020-03-11  5:49 [PATCH] net: drop_monitor: make drop_monitor built-in Masahiro Yamada
@ 2020-03-11  6:30 ` Masahiro Yamada
  0 siblings, 0 replies; 2+ messages in thread
From: Masahiro Yamada @ 2020-03-11  6:30 UTC (permalink / raw)
  To: Neil Horman, Networking, David S . Miller
  Cc: Ido Schimmel, Jiri Pirko, Stephen Rothwell, Nicolas Pitre,
	Linux Kbuild mailing list, Jakub Kicinski,
	Linux Kernel Mailing List

On Wed, Mar 11, 2020 at 2:50 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> 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@canb.auug.org.au>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>


Sorry, I take this back.
This is probably not the right fix.



I just sent an alternative patch.
( Replace IS_ENABLE with IS_REACHABLE )


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2020-03-11  6:31 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-11  5:49 [PATCH] net: drop_monitor: make drop_monitor built-in Masahiro Yamada
2020-03-11  6:30 ` Masahiro Yamada

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).