| Message ID | 1337285040-20848-1-git-send-email-nhorman@tuxdriver.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
On Thu, 2012-05-17 at 16:04 -0400, Neil Horman wrote: > When I first wrote drop monitor I wrote it to just build monolithically. There > is no reason it can't be built modularly as well, so lets give it that > flexibiity. > > I've tested this by building it as both a module and monolithically, and it > seems to work quite well > > Change notes: > > v2) > * fixed for_each_present_cpu loops to be more correct as per Eric D. > * Converted exit path failures to BUG_ON as per Ben H. > > v3) > * Converted del_timer to del_timer_sync to close race noted by Ben H. > > Signed-off-by: Neil Horman <nhorman@tuxdriver.com> > CC: "David S. Miller" <davem@davemloft.net> > CC: Eric Dumazet <eric.dumazet@gmail.com> > CC: Ben Hutchings <bhutchings@solarflare.com> [...] Reviewed-by: Ben Hutchings <bhutchings@solarflare.com> Thanks, Ben.
From: Neil Horman <nhorman@tuxdriver.com> Date: Thu, 17 May 2012 16:04:00 -0400 > When I first wrote drop monitor I wrote it to just build monolithically. There > is no reason it can't be built modularly as well, so lets give it that > flexibiity. > > I've tested this by building it as both a module and monolithically, and it > seems to work quite well > > Change notes: > > v2) > * fixed for_each_present_cpu loops to be more correct as per Eric D. > * Converted exit path failures to BUG_ON as per Ben H. > > v3) > * Converted del_timer to del_timer_sync to close race noted by Ben H. > > Signed-off-by: Neil Horman <nhorman@tuxdriver.com> Applied, althrough it didn't apply cleanly to net-next. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, May 17, 2012 at 04:09:37PM -0400, David Miller wrote: > From: Neil Horman <nhorman@tuxdriver.com> > Date: Thu, 17 May 2012 16:04:00 -0400 > > > When I first wrote drop monitor I wrote it to just build monolithically. There > > is no reason it can't be built modularly as well, so lets give it that > > flexibiity. > > > > I've tested this by building it as both a module and monolithically, and it > > seems to work quite well > > > > Change notes: > > > > v2) > > * fixed for_each_present_cpu loops to be more correct as per Eric D. > > * Converted exit path failures to BUG_ON as per Ben H. > > > > v3) > > * Converted del_timer to del_timer_sync to close race noted by Ben H. > > > > Signed-off-by: Neil Horman <nhorman@tuxdriver.com> > > Applied, althrough it didn't apply cleanly to net-next. > Apologies Dave, should have told you that I was carrying Joe P.'s cleanup patch in my net-next tree as well: http://marc.info/?l=linux-netdev&m=133727344816140&w=2 Since you noted that you had applied it, I applied it myself here. Neil -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 2012-05-17 at 16:21 -0400, Neil Horman wrote: > On Thu, May 17, 2012 at 04:09:37PM -0400, David Miller wrote: > > > > Applied, althrough it didn't apply cleanly to net-next. > > > > Apologies Dave, should have told you that I was carrying Joe P.'s cleanup patch > in my net-next tree as well: > http://marc.info/?l=linux-netdev&m=133727344816140&w=2 > > Since you noted that you had applied it, I applied it myself here. > Neil > Any plan to autoload drop_monitor module from dropwatch, or issuing some advice ? # dropwatch -l kas Unable to find NET_DM family, dropwatch can't work Cleanuing up on socket creation error Thanks -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, May 22, 2012 at 03:05:19PM +0200, Eric Dumazet wrote: > On Thu, 2012-05-17 at 16:21 -0400, Neil Horman wrote: > > On Thu, May 17, 2012 at 04:09:37PM -0400, David Miller wrote: > > > > > > > Applied, althrough it didn't apply cleanly to net-next. > > > > > > > Apologies Dave, should have told you that I was carrying Joe P.'s cleanup patch > > in my net-next tree as well: > > http://marc.info/?l=linux-netdev&m=133727344816140&w=2 > > > > Since you noted that you had applied it, I applied it myself here. > > Neil > > > > Any plan to autoload drop_monitor module from dropwatch, > or issuing some advice ? > > # dropwatch -l kas > Unable to find NET_DM family, dropwatch can't work > Cleanuing up on socket creation error > > Thanks > I'm looking into that currently, although I was starting to wonder if its possible to do with a generic netlink socket. I can't seem to find any examples, and I can't use the net-pf-* module alias mechanism that formal protocols implement, since I don't have a defined address family. I suppose I could augment that format to support a net-pf-16-<name> alias, where name is the name of the genl family that gets registered by the module you're looking for. Does that seem like a reasonable idea? Neil > > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, May 22, 2012 at 03:05:19PM +0200, Eric Dumazet wrote: > On Thu, 2012-05-17 at 16:21 -0400, Neil Horman wrote: > > On Thu, May 17, 2012 at 04:09:37PM -0400, David Miller wrote: > > > > > > > Applied, althrough it didn't apply cleanly to net-next. > > > > > > > Apologies Dave, should have told you that I was carrying Joe P.'s cleanup patch > > in my net-next tree as well: > > http://marc.info/?l=linux-netdev&m=133727344816140&w=2 > > > > Since you noted that you had applied it, I applied it myself here. > > Neil > > > > Any plan to autoload drop_monitor module from dropwatch, > or issuing some advice ? > > # dropwatch -l kas > Unable to find NET_DM family, dropwatch can't work > Cleanuing up on socket creation error > > Thanks > > > Eric, Just FYI, I sent a series upstream to implement autoloading of generic netlink families. Please be awarem, that I've tested these with a hacked version of dropwatch, and it works great, but with the normal version of dropwatch, the drop_monitor module still doesn't autoload. This is due to libnl not explicitly requesting a family when genl_ctrl_family_resolve is called. Instead of trying to load the module, it dumps the existing registered families via a NLM_F_DUMP message. I'm working on updating libnl to correct this currently and will cc you on the patch. Neil -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2012-05-29 at 15:33 -0400, Neil Horman wrote: > Eric, > Just FYI, I sent a series upstream to implement autoloading of generic > netlink families. Please be awarem, that I've tested these with a hacked > version of dropwatch, and it works great, but with the normal version of > dropwatch, the drop_monitor module still doesn't autoload. This is due to libnl > not explicitly requesting a family when genl_ctrl_family_resolve is called. > Instead of trying to load the module, it dumps the existing registered families > via a NLM_F_DUMP message. I'm working on updating libnl to correct this > currently and will cc you on the patch. Excellent, thanks Neil -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/Kconfig b/net/Kconfig index e07272d..76ad6fa 100644 --- a/net/Kconfig +++ b/net/Kconfig @@ -295,7 +295,7 @@ config NET_TCPPROBE module will be called tcp_probe. config NET_DROP_MONITOR - boolean "Network packet drop alerting service" + tristate "Network packet drop alerting service" depends on INET && EXPERIMENTAL && 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 cfeeef8..f93f985 100644 --- a/net/core/drop_monitor.c +++ b/net/core/drop_monitor.c @@ -24,6 +24,7 @@ #include <linux/timer.h> #include <linux/bitops.h> #include <linux/slab.h> +#include <linux/module.h> #include <net/genetlink.h> #include <net/netevent.h> @@ -225,9 +226,15 @@ static int set_all_monitor_traces(int state) switch (state) { case TRACE_ON: + if (!try_module_get(THIS_MODULE)) { + rc = -ENODEV; + break; + } + rc |= register_trace_kfree_skb(trace_kfree_skb_hit, NULL); rc |= register_trace_napi_poll(trace_napi_poll_hit, NULL); break; + case TRACE_OFF: rc |= unregister_trace_kfree_skb(trace_kfree_skb_hit, NULL); rc |= unregister_trace_napi_poll(trace_napi_poll_hit, NULL); @@ -243,6 +250,9 @@ static int set_all_monitor_traces(int state) kfree_rcu(new_stat, rcu); } } + + module_put(THIS_MODULE); + break; default: rc = 1; @@ -368,7 +378,7 @@ static int __init init_net_drop_monitor(void) rc = 0; - for_each_present_cpu(cpu) { + for_each_possible_cpu(cpu) { data = &per_cpu(dm_cpu_data, cpu); reset_per_cpu_data(data); INIT_WORK(&data->dm_alert_work, send_dm_alert); @@ -385,4 +395,36 @@ out: return rc; } -late_initcall(init_net_drop_monitor); +static void exit_net_drop_monitor(void) +{ + struct per_cpu_dm_data *data; + 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 + * all we need to do is make sure that we don't have any running timers + * or pending schedule calls + */ + + for_each_possible_cpu(cpu) { + data = &per_cpu(dm_cpu_data, cpu); + del_timer_sync(&data->send_timer); + cancel_work_sync(&data->dm_alert_work); + /* + * At this point, we should have exclusive access + * to this struct and can free the skb inside it + */ + kfree_skb(data->skb); + } + + 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>");
When I first wrote drop monitor I wrote it to just build monolithically. There is no reason it can't be built modularly as well, so lets give it that flexibiity. I've tested this by building it as both a module and monolithically, and it seems to work quite well Change notes: v2) * fixed for_each_present_cpu loops to be more correct as per Eric D. * Converted exit path failures to BUG_ON as per Ben H. v3) * Converted del_timer to del_timer_sync to close race noted by Ben H. Signed-off-by: Neil Horman <nhorman@tuxdriver.com> CC: "David S. Miller" <davem@davemloft.net> CC: Eric Dumazet <eric.dumazet@gmail.com> CC: Ben Hutchings <bhutchings@solarflare.com> --- net/Kconfig | 2 +- net/core/drop_monitor.c | 46 ++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 45 insertions(+), 3 deletions(-)