* [PATCH net-next 0/7] drop_monitor: Convert to use devlink tracepoint
@ 2020-09-29 8:15 Ido Schimmel
2020-09-29 8:15 ` [PATCH net-next 1/7] devlink: Add a tracepoint for trap reports Ido Schimmel
` (7 more replies)
0 siblings, 8 replies; 9+ messages in thread
From: Ido Schimmel @ 2020-09-29 8:15 UTC (permalink / raw)
To: netdev
Cc: davem, kuba, nhorman, jiri, roopa, aroulin, ayal, masahiroy,
mlxsw, Ido Schimmel
From: Ido Schimmel <idosch@nvidia.com>
Drop monitor is able to monitor both software and hardware originated
drops. Software drops are monitored by having drop monitor register its
probe on the 'kfree_skb' tracepoint. Hardware originated drops are
monitored by having devlink call into drop monitor whenever it receives
a dropped packet from the underlying hardware.
This patch set converts drop monitor to monitor both software and
hardware originated drops in the same way - by registering its probe on
the relevant tracepoint.
In addition to drop monitor being more consistent, it is now also
possible to build drop monitor as module instead of as a builtin and
still monitor hardware originated drops. Initially, CONFIG_NET_DEVLINK
implied CONFIG_NET_DROP_MONITOR, but after commit def2fbffe62c
("kconfig: allow symbols implied by y to become m") we can have
CONFIG_NET_DEVLINK=y and CONFIG_NET_DROP_MONITOR=m and hardware
originated drops will not be monitored.
Patch set overview:
Patch #1 adds a tracepoint in devlink for trap reports.
Patch #2 prepares probe functions in drop monitor for the new
tracepoint.
Patch #3 converts drop monitor to use the new tracepoint.
Patches #4-#6 perform cleanups after the conversion.
Patch #7 adds a test case for drop monitor. Both software originated
drops and hardware originated drops (using netdevsim) are tested.
Tested:
| CONFIG_NET_DEVLINK | CONFIG_NET_DROP_MONITOR | Build | SW drops | HW drops |
| -------------------|-------------------------|-------|----------|----------|
| y | y | v | v | v |
| y | m | v | v | v |
| y | n | v | x | x |
| n | y | v | v | x |
| n | m | v | v | x |
| n | n | v | x | x |
Ido Schimmel (7):
devlink: Add a tracepoint for trap reports
drop_monitor: Prepare probe functions for devlink tracepoint
drop_monitor: Convert to using devlink tracepoint
drop_monitor: Remove no longer used functions
drop_monitor: Remove duplicate struct
drop_monitor: Filter control packets in drop monitor
selftests: net: Add drop monitor test
MAINTAINERS | 1 -
include/net/devlink.h | 16 ++
include/net/drop_monitor.h | 36 ---
include/trace/events/devlink.h | 37 +++
net/Kconfig | 1 -
net/core/devlink.c | 37 ++-
net/core/drop_monitor.c | 133 ++++++-----
tools/testing/selftests/net/Makefile | 1 +
tools/testing/selftests/net/config | 3 +
.../selftests/net/drop_monitor_tests.sh | 215 ++++++++++++++++++
10 files changed, 368 insertions(+), 112 deletions(-)
delete mode 100644 include/net/drop_monitor.h
create mode 100755 tools/testing/selftests/net/drop_monitor_tests.sh
--
2.26.2
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH net-next 1/7] devlink: Add a tracepoint for trap reports
2020-09-29 8:15 [PATCH net-next 0/7] drop_monitor: Convert to use devlink tracepoint Ido Schimmel
@ 2020-09-29 8:15 ` Ido Schimmel
2020-09-29 8:15 ` [PATCH net-next 2/7] drop_monitor: Prepare probe functions for devlink tracepoint Ido Schimmel
` (6 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Ido Schimmel @ 2020-09-29 8:15 UTC (permalink / raw)
To: netdev
Cc: davem, kuba, nhorman, jiri, roopa, aroulin, ayal, masahiroy,
mlxsw, Ido Schimmel
From: Ido Schimmel <idosch@mellanox.com>
Add a tracepoint for trap reports so that drop monitor could register
its probe on it. Use trace_devlink_trap_report_enabled() to avoid
wasting cycles setting the trap metadata if the tracepoint is not
enabled.
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
include/net/devlink.h | 14 +++++++++++++
include/trace/events/devlink.h | 37 ++++++++++++++++++++++++++++++++++
net/core/devlink.c | 25 +++++++++++++++++++++++
3 files changed, 76 insertions(+)
diff --git a/include/net/devlink.h b/include/net/devlink.h
index 7339bf9ba6b4..1014294ba6a0 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -624,6 +624,20 @@ struct devlink_health_reporter_ops {
struct netlink_ext_ack *extack);
};
+/**
+ * struct devlink_trap_metadata - Packet trap metadata.
+ * @trap_name: Trap name.
+ * @trap_group_name: Trap group name.
+ * @input_dev: Input netdevice.
+ * @fa_cookie: Flow action user cookie.
+ */
+struct devlink_trap_metadata {
+ const char *trap_name;
+ const char *trap_group_name;
+ struct net_device *input_dev;
+ const struct flow_action_cookie *fa_cookie;
+};
+
/**
* struct devlink_trap_policer - Immutable packet trap policer attributes.
* @id: Policer identifier.
diff --git a/include/trace/events/devlink.h b/include/trace/events/devlink.h
index 6f60a78d9a7e..44d8e2981065 100644
--- a/include/trace/events/devlink.h
+++ b/include/trace/events/devlink.h
@@ -171,6 +171,43 @@ TRACE_EVENT(devlink_health_reporter_state_update,
__entry->new_state)
);
+/*
+ * Tracepoint for devlink packet trap:
+ */
+TRACE_EVENT(devlink_trap_report,
+ TP_PROTO(const struct devlink *devlink, struct sk_buff *skb,
+ const struct devlink_trap_metadata *metadata),
+
+ TP_ARGS(devlink, skb, metadata),
+
+ TP_STRUCT__entry(
+ __string(bus_name, devlink->dev->bus->name)
+ __string(dev_name, dev_name(devlink->dev))
+ __string(driver_name, devlink->dev->driver->name)
+ __string(trap_name, metadata->trap_name)
+ __string(trap_group_name, metadata->trap_group_name)
+ __dynamic_array(char, input_dev_name, IFNAMSIZ)
+ ),
+
+ TP_fast_assign(
+ struct net_device *input_dev = metadata->input_dev;
+
+ __assign_str(bus_name, devlink->dev->bus->name);
+ __assign_str(dev_name, dev_name(devlink->dev));
+ __assign_str(driver_name, devlink->dev->driver->name);
+ __assign_str(trap_name, metadata->trap_name);
+ __assign_str(trap_group_name, metadata->trap_group_name);
+ __assign_str(input_dev_name,
+ (input_dev ? input_dev->name : "NULL"));
+ ),
+
+ TP_printk("bus_name=%s dev_name=%s driver_name=%s trap_name=%s "
+ "trap_group_name=%s input_dev_name=%s", __get_str(bus_name),
+ __get_str(dev_name), __get_str(driver_name),
+ __get_str(trap_name), __get_str(trap_group_name),
+ __get_str(input_dev_name))
+);
+
#endif /* _TRACE_DEVLINK_H */
/* This part must be outside protection */
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 7a38f9e25922..c0f300507c37 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -84,6 +84,7 @@ EXPORT_SYMBOL(devlink_dpipe_header_ipv6);
EXPORT_TRACEPOINT_SYMBOL_GPL(devlink_hwmsg);
EXPORT_TRACEPOINT_SYMBOL_GPL(devlink_hwerr);
+EXPORT_TRACEPOINT_SYMBOL_GPL(devlink_trap_report);
static const struct nla_policy devlink_function_nl_policy[DEVLINK_PORT_FUNCTION_ATTR_MAX + 1] = {
[DEVLINK_PORT_FUNCTION_ATTR_HW_ADDR] = { .type = NLA_BINARY },
@@ -9278,6 +9279,22 @@ devlink_trap_report_metadata_fill(struct net_dm_hw_metadata *hw_metadata,
spin_unlock(&in_devlink_port->type_lock);
}
+static void
+devlink_trap_report_metadata_set(struct devlink_trap_metadata *metadata,
+ const struct devlink_trap_item *trap_item,
+ struct devlink_port *in_devlink_port,
+ const struct flow_action_cookie *fa_cookie)
+{
+ metadata->trap_name = trap_item->trap->name;
+ metadata->trap_group_name = trap_item->group_item->group->name;
+ metadata->fa_cookie = fa_cookie;
+
+ spin_lock(&in_devlink_port->type_lock);
+ if (in_devlink_port->type == DEVLINK_PORT_TYPE_ETH)
+ metadata->input_dev = in_devlink_port->type_dev;
+ spin_unlock(&in_devlink_port->type_lock);
+}
+
/**
* devlink_trap_report - Report trapped packet to drop monitor.
* @devlink: devlink.
@@ -9307,6 +9324,14 @@ void devlink_trap_report(struct devlink *devlink, struct sk_buff *skb,
devlink_trap_report_metadata_fill(&hw_metadata, trap_item,
in_devlink_port, fa_cookie);
net_dm_hw_report(skb, &hw_metadata);
+
+ if (trace_devlink_trap_report_enabled()) {
+ struct devlink_trap_metadata metadata = {};
+
+ devlink_trap_report_metadata_set(&metadata, trap_item,
+ in_devlink_port, fa_cookie);
+ trace_devlink_trap_report(devlink, skb, &metadata);
+ }
}
EXPORT_SYMBOL_GPL(devlink_trap_report);
--
2.26.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH net-next 2/7] drop_monitor: Prepare probe functions for devlink tracepoint
2020-09-29 8:15 [PATCH net-next 0/7] drop_monitor: Convert to use devlink tracepoint Ido Schimmel
2020-09-29 8:15 ` [PATCH net-next 1/7] devlink: Add a tracepoint for trap reports Ido Schimmel
@ 2020-09-29 8:15 ` Ido Schimmel
2020-09-29 8:15 ` [PATCH net-next 3/7] drop_monitor: Convert to using " Ido Schimmel
` (5 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Ido Schimmel @ 2020-09-29 8:15 UTC (permalink / raw)
To: netdev
Cc: davem, kuba, nhorman, jiri, roopa, aroulin, ayal, masahiroy,
mlxsw, Ido Schimmel
From: Ido Schimmel <idosch@mellanox.com>
Drop monitor supports two alerting modes: Summary and packet. Prepare a
probe function for each, so that they could be later registered on the
devlink tracepoint by calling register_trace_devlink_trap_report(),
based on the configured alerting mode.
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
net/core/drop_monitor.c | 146 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 146 insertions(+)
diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c
index 9704522b0872..03aba582c0b9 100644
--- a/net/core/drop_monitor.c
+++ b/net/core/drop_monitor.c
@@ -30,6 +30,7 @@
#include <net/genetlink.h>
#include <net/netevent.h>
#include <net/flow_offload.h>
+#include <net/devlink.h>
#include <trace/events/skb.h>
#include <trace/events/napi.h>
@@ -116,6 +117,9 @@ struct net_dm_alert_ops {
void (*hw_work_item_func)(struct work_struct *work);
void (*hw_probe)(struct sk_buff *skb,
const struct net_dm_hw_metadata *hw_metadata);
+ void (*hw_trap_probe)(void *ignore, const struct devlink *devlink,
+ struct sk_buff *skb,
+ const struct devlink_trap_metadata *metadata);
};
struct net_dm_skb_cb {
@@ -474,12 +478,57 @@ net_dm_hw_summary_probe(struct sk_buff *skb,
spin_unlock_irqrestore(&hw_data->lock, flags);
}
+static void
+net_dm_hw_trap_summary_probe(void *ignore, const struct devlink *devlink,
+ struct sk_buff *skb,
+ const struct devlink_trap_metadata *metadata)
+{
+ struct net_dm_hw_entries *hw_entries;
+ struct net_dm_hw_entry *hw_entry;
+ struct per_cpu_dm_data *hw_data;
+ unsigned long flags;
+ int i;
+
+ hw_data = this_cpu_ptr(&dm_hw_cpu_data);
+ spin_lock_irqsave(&hw_data->lock, flags);
+ hw_entries = hw_data->hw_entries;
+
+ if (!hw_entries)
+ goto out;
+
+ for (i = 0; i < hw_entries->num_entries; i++) {
+ hw_entry = &hw_entries->entries[i];
+ if (!strncmp(hw_entry->trap_name, metadata->trap_name,
+ NET_DM_MAX_HW_TRAP_NAME_LEN - 1)) {
+ hw_entry->count++;
+ goto out;
+ }
+ }
+ if (WARN_ON_ONCE(hw_entries->num_entries == dm_hit_limit))
+ goto out;
+
+ hw_entry = &hw_entries->entries[hw_entries->num_entries];
+ strlcpy(hw_entry->trap_name, metadata->trap_name,
+ NET_DM_MAX_HW_TRAP_NAME_LEN - 1);
+ hw_entry->count = 1;
+ hw_entries->num_entries++;
+
+ if (!timer_pending(&hw_data->send_timer)) {
+ hw_data->send_timer.expires = jiffies + dm_delay * HZ;
+ add_timer(&hw_data->send_timer);
+ }
+
+out:
+ spin_unlock_irqrestore(&hw_data->lock, flags);
+}
+
static const struct net_dm_alert_ops net_dm_alert_summary_ops = {
.kfree_skb_probe = trace_kfree_skb_hit,
.napi_poll_probe = trace_napi_poll_hit,
.work_item_func = send_dm_alert,
.hw_work_item_func = net_dm_hw_summary_work,
.hw_probe = net_dm_hw_summary_probe,
+ .hw_trap_probe = net_dm_hw_trap_summary_probe,
};
static void net_dm_packet_trace_kfree_skb_hit(void *ignore,
@@ -858,6 +907,54 @@ net_dm_hw_metadata_clone(const struct net_dm_hw_metadata *hw_metadata)
return NULL;
}
+static struct net_dm_hw_metadata *
+net_dm_hw_metadata_copy(const struct devlink_trap_metadata *metadata)
+{
+ const struct flow_action_cookie *fa_cookie;
+ struct net_dm_hw_metadata *hw_metadata;
+ const char *trap_group_name;
+ const char *trap_name;
+
+ hw_metadata = kzalloc(sizeof(*hw_metadata), GFP_ATOMIC);
+ if (!hw_metadata)
+ return NULL;
+
+ trap_group_name = kstrdup(metadata->trap_group_name, GFP_ATOMIC);
+ if (!trap_group_name)
+ goto free_hw_metadata;
+ hw_metadata->trap_group_name = trap_group_name;
+
+ trap_name = kstrdup(metadata->trap_name, GFP_ATOMIC);
+ if (!trap_name)
+ goto free_trap_group;
+ hw_metadata->trap_name = trap_name;
+
+ if (metadata->fa_cookie) {
+ size_t cookie_size = sizeof(*fa_cookie) +
+ metadata->fa_cookie->cookie_len;
+
+ fa_cookie = kmemdup(metadata->fa_cookie, cookie_size,
+ GFP_ATOMIC);
+ if (!fa_cookie)
+ goto free_trap_name;
+ hw_metadata->fa_cookie = fa_cookie;
+ }
+
+ hw_metadata->input_dev = metadata->input_dev;
+ if (hw_metadata->input_dev)
+ dev_hold(hw_metadata->input_dev);
+
+ return hw_metadata;
+
+free_trap_name:
+ kfree(trap_name);
+free_trap_group:
+ kfree(trap_group_name);
+free_hw_metadata:
+ kfree(hw_metadata);
+ return NULL;
+}
+
static void
net_dm_hw_metadata_free(const struct net_dm_hw_metadata *hw_metadata)
{
@@ -970,12 +1067,61 @@ net_dm_hw_packet_probe(struct sk_buff *skb,
consume_skb(nskb);
}
+static void
+net_dm_hw_trap_packet_probe(void *ignore, const struct devlink *devlink,
+ struct sk_buff *skb,
+ const struct devlink_trap_metadata *metadata)
+{
+ struct net_dm_hw_metadata *n_hw_metadata;
+ ktime_t tstamp = ktime_get_real();
+ struct per_cpu_dm_data *hw_data;
+ struct sk_buff *nskb;
+ unsigned long flags;
+
+ if (!skb_mac_header_was_set(skb))
+ return;
+
+ nskb = skb_clone(skb, GFP_ATOMIC);
+ if (!nskb)
+ return;
+
+ n_hw_metadata = net_dm_hw_metadata_copy(metadata);
+ if (!n_hw_metadata)
+ goto free;
+
+ NET_DM_SKB_CB(nskb)->hw_metadata = n_hw_metadata;
+ nskb->tstamp = tstamp;
+
+ hw_data = this_cpu_ptr(&dm_hw_cpu_data);
+
+ spin_lock_irqsave(&hw_data->drop_queue.lock, flags);
+ if (skb_queue_len(&hw_data->drop_queue) < net_dm_queue_len)
+ __skb_queue_tail(&hw_data->drop_queue, nskb);
+ else
+ goto unlock_free;
+ spin_unlock_irqrestore(&hw_data->drop_queue.lock, flags);
+
+ schedule_work(&hw_data->dm_alert_work);
+
+ return;
+
+unlock_free:
+ spin_unlock_irqrestore(&hw_data->drop_queue.lock, flags);
+ u64_stats_update_begin(&hw_data->stats.syncp);
+ hw_data->stats.dropped++;
+ u64_stats_update_end(&hw_data->stats.syncp);
+ net_dm_hw_metadata_free(n_hw_metadata);
+free:
+ consume_skb(nskb);
+}
+
static const struct net_dm_alert_ops net_dm_alert_packet_ops = {
.kfree_skb_probe = net_dm_packet_trace_kfree_skb_hit,
.napi_poll_probe = net_dm_packet_trace_napi_poll_hit,
.work_item_func = net_dm_packet_work,
.hw_work_item_func = net_dm_hw_packet_work,
.hw_probe = net_dm_hw_packet_probe,
+ .hw_trap_probe = net_dm_hw_trap_packet_probe,
};
static const struct net_dm_alert_ops *net_dm_alert_ops_arr[] = {
--
2.26.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH net-next 3/7] drop_monitor: Convert to using devlink tracepoint
2020-09-29 8:15 [PATCH net-next 0/7] drop_monitor: Convert to use devlink tracepoint Ido Schimmel
2020-09-29 8:15 ` [PATCH net-next 1/7] devlink: Add a tracepoint for trap reports Ido Schimmel
2020-09-29 8:15 ` [PATCH net-next 2/7] drop_monitor: Prepare probe functions for devlink tracepoint Ido Schimmel
@ 2020-09-29 8:15 ` Ido Schimmel
2020-09-29 8:15 ` [PATCH net-next 4/7] drop_monitor: Remove no longer used functions Ido Schimmel
` (4 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Ido Schimmel @ 2020-09-29 8:15 UTC (permalink / raw)
To: netdev
Cc: davem, kuba, nhorman, jiri, roopa, aroulin, ayal, masahiroy,
mlxsw, Ido Schimmel
From: Ido Schimmel <idosch@mellanox.com>
Convert drop monitor to use the recently introduced
'devlink_trap_report' tracepoint instead of having devlink call into
drop monitor.
This is both consistent with software originated drops ('kfree_skb'
tracepoint) and also allows drop monitor to be built as a module and
still report hardware originated drops.
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
MAINTAINERS | 1 -
include/net/drop_monitor.h | 36 ------------------------
net/Kconfig | 1 -
net/core/devlink.c | 24 ----------------
net/core/drop_monitor.c | 56 +++++++++++++++++++++++++++-----------
5 files changed, 40 insertions(+), 78 deletions(-)
delete mode 100644 include/net/drop_monitor.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 42c69d2eeece..c1e946606dce 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12065,7 +12065,6 @@ M: Neil Horman <nhorman@tuxdriver.com>
L: netdev@vger.kernel.org
S: Maintained
W: https://fedorahosted.org/dropwatch/
-F: include/net/drop_monitor.h
F: include/uapi/linux/net_dropmon.h
F: net/core/drop_monitor.c
diff --git a/include/net/drop_monitor.h b/include/net/drop_monitor.h
deleted file mode 100644
index 3f5b6ddb3179..000000000000
--- a/include/net/drop_monitor.h
+++ /dev/null
@@ -1,36 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-
-#ifndef _NET_DROP_MONITOR_H_
-#define _NET_DROP_MONITOR_H_
-
-#include <linux/ktime.h>
-#include <linux/netdevice.h>
-#include <linux/skbuff.h>
-#include <net/flow_offload.h>
-
-/**
- * struct net_dm_hw_metadata - Hardware-supplied packet metadata.
- * @trap_group_name: Hardware trap group name.
- * @trap_name: Hardware trap name.
- * @input_dev: Input netdevice.
- * @fa_cookie: Flow action user cookie.
- */
-struct net_dm_hw_metadata {
- const char *trap_group_name;
- const char *trap_name;
- struct net_device *input_dev;
- const struct flow_action_cookie *fa_cookie;
-};
-
-#if IS_REACHABLE(CONFIG_NET_DROP_MONITOR)
-void net_dm_hw_report(struct sk_buff *skb,
- const struct net_dm_hw_metadata *hw_metadata);
-#else
-static inline void
-net_dm_hw_report(struct sk_buff *skb,
- const struct net_dm_hw_metadata *hw_metadata)
-{
-}
-#endif
-
-#endif /* _NET_DROP_MONITOR_H_ */
diff --git a/net/Kconfig b/net/Kconfig
index 3831206977a1..d6567162c1cf 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -434,7 +434,6 @@ config NET_SOCK_MSG
config NET_DEVLINK
bool
default n
- imply NET_DROP_MONITOR
config PAGE_POOL
bool
diff --git a/net/core/devlink.c b/net/core/devlink.c
index c0f300507c37..2ea9fdc0df2d 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -27,7 +27,6 @@
#include <net/net_namespace.h>
#include <net/sock.h>
#include <net/devlink.h>
-#include <net/drop_monitor.h>
#define CREATE_TRACE_POINTS
#include <trace/events/devlink.h>
@@ -9261,24 +9260,6 @@ devlink_trap_stats_update(struct devlink_stats __percpu *trap_stats,
u64_stats_update_end(&stats->syncp);
}
-static void
-devlink_trap_report_metadata_fill(struct net_dm_hw_metadata *hw_metadata,
- const struct devlink_trap_item *trap_item,
- struct devlink_port *in_devlink_port,
- const struct flow_action_cookie *fa_cookie)
-{
- struct devlink_trap_group_item *group_item = trap_item->group_item;
-
- hw_metadata->trap_group_name = group_item->group->name;
- hw_metadata->trap_name = trap_item->trap->name;
- hw_metadata->fa_cookie = fa_cookie;
-
- spin_lock(&in_devlink_port->type_lock);
- if (in_devlink_port->type == DEVLINK_PORT_TYPE_ETH)
- hw_metadata->input_dev = in_devlink_port->type_dev;
- spin_unlock(&in_devlink_port->type_lock);
-}
-
static void
devlink_trap_report_metadata_set(struct devlink_trap_metadata *metadata,
const struct devlink_trap_item *trap_item,
@@ -9309,7 +9290,6 @@ void devlink_trap_report(struct devlink *devlink, struct sk_buff *skb,
{
struct devlink_trap_item *trap_item = trap_ctx;
- struct net_dm_hw_metadata hw_metadata = {};
devlink_trap_stats_update(trap_item->stats, skb->len);
devlink_trap_stats_update(trap_item->group_item->stats, skb->len);
@@ -9321,10 +9301,6 @@ void devlink_trap_report(struct devlink *devlink, struct sk_buff *skb,
if (trap_item->trap->type == DEVLINK_TRAP_TYPE_CONTROL)
return;
- devlink_trap_report_metadata_fill(&hw_metadata, trap_item,
- in_devlink_port, fa_cookie);
- net_dm_hw_report(skb, &hw_metadata);
-
if (trace_devlink_trap_report_enabled()) {
struct devlink_trap_metadata metadata = {};
diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c
index 03aba582c0b9..c14278fd6405 100644
--- a/net/core/drop_monitor.c
+++ b/net/core/drop_monitor.c
@@ -26,7 +26,6 @@
#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>
#include <net/flow_offload.h>
@@ -34,6 +33,7 @@
#include <trace/events/skb.h>
#include <trace/events/napi.h>
+#include <trace/events/devlink.h>
#include <asm/unaligned.h>
@@ -108,6 +108,13 @@ static enum net_dm_alert_mode net_dm_alert_mode = NET_DM_ALERT_MODE_SUMMARY;
static u32 net_dm_trunc_len;
static u32 net_dm_queue_len = 1000;
+struct net_dm_hw_metadata {
+ const char *trap_group_name;
+ const char *trap_name;
+ struct net_device *input_dev;
+ const struct flow_action_cookie *fa_cookie;
+};
+
struct net_dm_alert_ops {
void (*kfree_skb_probe)(void *ignore, struct sk_buff *skb,
void *location);
@@ -1129,25 +1136,32 @@ static const struct net_dm_alert_ops *net_dm_alert_ops_arr[] = {
[NET_DM_ALERT_MODE_PACKET] = &net_dm_alert_packet_ops,
};
-void net_dm_hw_report(struct sk_buff *skb,
- const struct net_dm_hw_metadata *hw_metadata)
+#if IS_ENABLED(CONFIG_NET_DEVLINK)
+static int net_dm_hw_probe_register(const struct net_dm_alert_ops *ops)
{
- rcu_read_lock();
-
- if (!monitor_hw)
- goto out;
+ return register_trace_devlink_trap_report(ops->hw_trap_probe, NULL);
+}
- net_dm_alert_ops_arr[net_dm_alert_mode]->hw_probe(skb, hw_metadata);
+static void net_dm_hw_probe_unregister(const struct net_dm_alert_ops *ops)
+{
+ unregister_trace_devlink_trap_report(ops->hw_trap_probe, NULL);
+ tracepoint_synchronize_unregister();
+}
+#else
+static int net_dm_hw_probe_register(const struct net_dm_alert_ops *ops)
+{
+ return -EOPNOTSUPP;
+}
-out:
- rcu_read_unlock();
+static void net_dm_hw_probe_unregister(const struct net_dm_alert_ops *ops)
+{
}
-EXPORT_SYMBOL_GPL(net_dm_hw_report);
+#endif
static int net_dm_hw_monitor_start(struct netlink_ext_ack *extack)
{
const struct net_dm_alert_ops *ops;
- int cpu;
+ int cpu, rc;
if (monitor_hw) {
NL_SET_ERR_MSG_MOD(extack, "Hardware monitoring already enabled");
@@ -1171,13 +1185,24 @@ static int net_dm_hw_monitor_start(struct netlink_ext_ack *extack)
kfree(hw_entries);
}
+ rc = net_dm_hw_probe_register(ops);
+ if (rc) {
+ NL_SET_ERR_MSG_MOD(extack, "Failed to connect probe to devlink_trap_probe() tracepoint");
+ goto err_module_put;
+ }
+
monitor_hw = true;
return 0;
+
+err_module_put:
+ module_put(THIS_MODULE);
+ return rc;
}
static void net_dm_hw_monitor_stop(struct netlink_ext_ack *extack)
{
+ const struct net_dm_alert_ops *ops;
int cpu;
if (!monitor_hw) {
@@ -1185,12 +1210,11 @@ static void net_dm_hw_monitor_stop(struct netlink_ext_ack *extack)
return;
}
+ ops = net_dm_alert_ops_arr[net_dm_alert_mode];
+
monitor_hw = false;
- /* After this call returns we are guaranteed that no CPU is processing
- * any hardware drops.
- */
- synchronize_rcu();
+ net_dm_hw_probe_unregister(ops);
for_each_possible_cpu(cpu) {
struct per_cpu_dm_data *hw_data = &per_cpu(dm_hw_cpu_data, cpu);
--
2.26.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH net-next 4/7] drop_monitor: Remove no longer used functions
2020-09-29 8:15 [PATCH net-next 0/7] drop_monitor: Convert to use devlink tracepoint Ido Schimmel
` (2 preceding siblings ...)
2020-09-29 8:15 ` [PATCH net-next 3/7] drop_monitor: Convert to using " Ido Schimmel
@ 2020-09-29 8:15 ` Ido Schimmel
2020-09-29 8:15 ` [PATCH net-next 5/7] drop_monitor: Remove duplicate struct Ido Schimmel
` (3 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Ido Schimmel @ 2020-09-29 8:15 UTC (permalink / raw)
To: netdev
Cc: davem, kuba, nhorman, jiri, roopa, aroulin, ayal, masahiroy,
mlxsw, Ido Schimmel
From: Ido Schimmel <idosch@mellanox.com>
The old probe functions that were invoked by drop monitor code are no
longer called and can thus be removed. They were replaced by actual
probe functions that are registered on the recently introduced
'devlink_trap_report' tracepoint.
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
net/core/drop_monitor.c | 142 ----------------------------------------
1 file changed, 142 deletions(-)
diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c
index c14278fd6405..db6d87ddde8d 100644
--- a/net/core/drop_monitor.c
+++ b/net/core/drop_monitor.c
@@ -122,8 +122,6 @@ struct net_dm_alert_ops {
int work, int budget);
void (*work_item_func)(struct work_struct *work);
void (*hw_work_item_func)(struct work_struct *work);
- void (*hw_probe)(struct sk_buff *skb,
- const struct net_dm_hw_metadata *hw_metadata);
void (*hw_trap_probe)(void *ignore, const struct devlink *devlink,
struct sk_buff *skb,
const struct devlink_trap_metadata *metadata);
@@ -442,49 +440,6 @@ static void net_dm_hw_summary_work(struct work_struct *work)
kfree(hw_entries);
}
-static void
-net_dm_hw_summary_probe(struct sk_buff *skb,
- const struct net_dm_hw_metadata *hw_metadata)
-{
- struct net_dm_hw_entries *hw_entries;
- struct net_dm_hw_entry *hw_entry;
- struct per_cpu_dm_data *hw_data;
- unsigned long flags;
- int i;
-
- hw_data = this_cpu_ptr(&dm_hw_cpu_data);
- spin_lock_irqsave(&hw_data->lock, flags);
- hw_entries = hw_data->hw_entries;
-
- if (!hw_entries)
- goto out;
-
- for (i = 0; i < hw_entries->num_entries; i++) {
- hw_entry = &hw_entries->entries[i];
- if (!strncmp(hw_entry->trap_name, hw_metadata->trap_name,
- NET_DM_MAX_HW_TRAP_NAME_LEN - 1)) {
- hw_entry->count++;
- goto out;
- }
- }
- if (WARN_ON_ONCE(hw_entries->num_entries == dm_hit_limit))
- goto out;
-
- hw_entry = &hw_entries->entries[hw_entries->num_entries];
- strlcpy(hw_entry->trap_name, hw_metadata->trap_name,
- NET_DM_MAX_HW_TRAP_NAME_LEN - 1);
- hw_entry->count = 1;
- hw_entries->num_entries++;
-
- if (!timer_pending(&hw_data->send_timer)) {
- hw_data->send_timer.expires = jiffies + dm_delay * HZ;
- add_timer(&hw_data->send_timer);
- }
-
-out:
- spin_unlock_irqrestore(&hw_data->lock, flags);
-}
-
static void
net_dm_hw_trap_summary_probe(void *ignore, const struct devlink *devlink,
struct sk_buff *skb,
@@ -534,7 +489,6 @@ static const struct net_dm_alert_ops net_dm_alert_summary_ops = {
.napi_poll_probe = trace_napi_poll_hit,
.work_item_func = send_dm_alert,
.hw_work_item_func = net_dm_hw_summary_work,
- .hw_probe = net_dm_hw_summary_probe,
.hw_trap_probe = net_dm_hw_trap_summary_probe,
};
@@ -866,54 +820,6 @@ static int net_dm_hw_packet_report_fill(struct sk_buff *msg,
return -EMSGSIZE;
}
-static struct net_dm_hw_metadata *
-net_dm_hw_metadata_clone(const struct net_dm_hw_metadata *hw_metadata)
-{
- const struct flow_action_cookie *fa_cookie;
- struct net_dm_hw_metadata *n_hw_metadata;
- const char *trap_group_name;
- const char *trap_name;
-
- n_hw_metadata = kzalloc(sizeof(*hw_metadata), GFP_ATOMIC);
- if (!n_hw_metadata)
- return NULL;
-
- trap_group_name = kstrdup(hw_metadata->trap_group_name, GFP_ATOMIC);
- if (!trap_group_name)
- goto free_hw_metadata;
- n_hw_metadata->trap_group_name = trap_group_name;
-
- trap_name = kstrdup(hw_metadata->trap_name, GFP_ATOMIC);
- if (!trap_name)
- goto free_trap_group;
- n_hw_metadata->trap_name = trap_name;
-
- if (hw_metadata->fa_cookie) {
- size_t cookie_size = sizeof(*fa_cookie) +
- hw_metadata->fa_cookie->cookie_len;
-
- fa_cookie = kmemdup(hw_metadata->fa_cookie, cookie_size,
- GFP_ATOMIC);
- if (!fa_cookie)
- goto free_trap_name;
- n_hw_metadata->fa_cookie = fa_cookie;
- }
-
- n_hw_metadata->input_dev = hw_metadata->input_dev;
- if (n_hw_metadata->input_dev)
- dev_hold(n_hw_metadata->input_dev);
-
- return n_hw_metadata;
-
-free_trap_name:
- kfree(trap_name);
-free_trap_group:
- kfree(trap_group_name);
-free_hw_metadata:
- kfree(n_hw_metadata);
- return NULL;
-}
-
static struct net_dm_hw_metadata *
net_dm_hw_metadata_copy(const struct devlink_trap_metadata *metadata)
{
@@ -1027,53 +933,6 @@ static void net_dm_hw_packet_work(struct work_struct *work)
net_dm_hw_packet_report(skb);
}
-static void
-net_dm_hw_packet_probe(struct sk_buff *skb,
- const struct net_dm_hw_metadata *hw_metadata)
-{
- struct net_dm_hw_metadata *n_hw_metadata;
- ktime_t tstamp = ktime_get_real();
- struct per_cpu_dm_data *hw_data;
- struct sk_buff *nskb;
- unsigned long flags;
-
- if (!skb_mac_header_was_set(skb))
- return;
-
- nskb = skb_clone(skb, GFP_ATOMIC);
- if (!nskb)
- return;
-
- n_hw_metadata = net_dm_hw_metadata_clone(hw_metadata);
- if (!n_hw_metadata)
- goto free;
-
- NET_DM_SKB_CB(nskb)->hw_metadata = n_hw_metadata;
- nskb->tstamp = tstamp;
-
- hw_data = this_cpu_ptr(&dm_hw_cpu_data);
-
- spin_lock_irqsave(&hw_data->drop_queue.lock, flags);
- if (skb_queue_len(&hw_data->drop_queue) < net_dm_queue_len)
- __skb_queue_tail(&hw_data->drop_queue, nskb);
- else
- goto unlock_free;
- spin_unlock_irqrestore(&hw_data->drop_queue.lock, flags);
-
- schedule_work(&hw_data->dm_alert_work);
-
- return;
-
-unlock_free:
- spin_unlock_irqrestore(&hw_data->drop_queue.lock, flags);
- u64_stats_update_begin(&hw_data->stats.syncp);
- hw_data->stats.dropped++;
- u64_stats_update_end(&hw_data->stats.syncp);
- net_dm_hw_metadata_free(n_hw_metadata);
-free:
- consume_skb(nskb);
-}
-
static void
net_dm_hw_trap_packet_probe(void *ignore, const struct devlink *devlink,
struct sk_buff *skb,
@@ -1127,7 +986,6 @@ static const struct net_dm_alert_ops net_dm_alert_packet_ops = {
.napi_poll_probe = net_dm_packet_trace_napi_poll_hit,
.work_item_func = net_dm_packet_work,
.hw_work_item_func = net_dm_hw_packet_work,
- .hw_probe = net_dm_hw_packet_probe,
.hw_trap_probe = net_dm_hw_trap_packet_probe,
};
--
2.26.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH net-next 5/7] drop_monitor: Remove duplicate struct
2020-09-29 8:15 [PATCH net-next 0/7] drop_monitor: Convert to use devlink tracepoint Ido Schimmel
` (3 preceding siblings ...)
2020-09-29 8:15 ` [PATCH net-next 4/7] drop_monitor: Remove no longer used functions Ido Schimmel
@ 2020-09-29 8:15 ` Ido Schimmel
2020-09-29 8:15 ` [PATCH net-next 6/7] drop_monitor: Filter control packets in drop monitor Ido Schimmel
` (2 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Ido Schimmel @ 2020-09-29 8:15 UTC (permalink / raw)
To: netdev
Cc: davem, kuba, nhorman, jiri, roopa, aroulin, ayal, masahiroy,
mlxsw, Ido Schimmel
From: Ido Schimmel <idosch@mellanox.com>
'struct net_dm_hw_metadata' is a duplicate of 'struct
devlink_trap_metadata'.
Remove the former and simplify the code.
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
net/core/drop_monitor.c | 27 ++++++++++-----------------
1 file changed, 10 insertions(+), 17 deletions(-)
diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c
index db6d87ddde8d..0e4309414a30 100644
--- a/net/core/drop_monitor.c
+++ b/net/core/drop_monitor.c
@@ -108,13 +108,6 @@ static enum net_dm_alert_mode net_dm_alert_mode = NET_DM_ALERT_MODE_SUMMARY;
static u32 net_dm_trunc_len;
static u32 net_dm_queue_len = 1000;
-struct net_dm_hw_metadata {
- const char *trap_group_name;
- const char *trap_name;
- struct net_device *input_dev;
- const struct flow_action_cookie *fa_cookie;
-};
-
struct net_dm_alert_ops {
void (*kfree_skb_probe)(void *ignore, struct sk_buff *skb,
void *location);
@@ -129,7 +122,7 @@ struct net_dm_alert_ops {
struct net_dm_skb_cb {
union {
- struct net_dm_hw_metadata *hw_metadata;
+ struct devlink_trap_metadata *hw_metadata;
void *pc;
};
};
@@ -715,7 +708,7 @@ static void net_dm_packet_work(struct work_struct *work)
}
static size_t
-net_dm_flow_action_cookie_size(const struct net_dm_hw_metadata *hw_metadata)
+net_dm_flow_action_cookie_size(const struct devlink_trap_metadata *hw_metadata)
{
return hw_metadata->fa_cookie ?
nla_total_size(hw_metadata->fa_cookie->cookie_len) : 0;
@@ -723,7 +716,7 @@ net_dm_flow_action_cookie_size(const struct net_dm_hw_metadata *hw_metadata)
static size_t
net_dm_hw_packet_report_size(size_t payload_len,
- const struct net_dm_hw_metadata *hw_metadata)
+ const struct devlink_trap_metadata *hw_metadata)
{
size_t size;
@@ -753,7 +746,7 @@ net_dm_hw_packet_report_size(size_t payload_len,
static int net_dm_hw_packet_report_fill(struct sk_buff *msg,
struct sk_buff *skb, size_t payload_len)
{
- struct net_dm_hw_metadata *hw_metadata;
+ struct devlink_trap_metadata *hw_metadata;
struct nlattr *attr;
void *hdr;
@@ -820,11 +813,11 @@ static int net_dm_hw_packet_report_fill(struct sk_buff *msg,
return -EMSGSIZE;
}
-static struct net_dm_hw_metadata *
+static struct devlink_trap_metadata *
net_dm_hw_metadata_copy(const struct devlink_trap_metadata *metadata)
{
const struct flow_action_cookie *fa_cookie;
- struct net_dm_hw_metadata *hw_metadata;
+ struct devlink_trap_metadata *hw_metadata;
const char *trap_group_name;
const char *trap_name;
@@ -869,7 +862,7 @@ net_dm_hw_metadata_copy(const struct devlink_trap_metadata *metadata)
}
static void
-net_dm_hw_metadata_free(const struct net_dm_hw_metadata *hw_metadata)
+net_dm_hw_metadata_free(const struct devlink_trap_metadata *hw_metadata)
{
if (hw_metadata->input_dev)
dev_put(hw_metadata->input_dev);
@@ -881,7 +874,7 @@ net_dm_hw_metadata_free(const struct net_dm_hw_metadata *hw_metadata)
static void net_dm_hw_packet_report(struct sk_buff *skb)
{
- struct net_dm_hw_metadata *hw_metadata;
+ struct devlink_trap_metadata *hw_metadata;
struct sk_buff *msg;
size_t payload_len;
int rc;
@@ -938,7 +931,7 @@ net_dm_hw_trap_packet_probe(void *ignore, const struct devlink *devlink,
struct sk_buff *skb,
const struct devlink_trap_metadata *metadata)
{
- struct net_dm_hw_metadata *n_hw_metadata;
+ struct devlink_trap_metadata *n_hw_metadata;
ktime_t tstamp = ktime_get_real();
struct per_cpu_dm_data *hw_data;
struct sk_buff *nskb;
@@ -1081,7 +1074,7 @@ static void net_dm_hw_monitor_stop(struct netlink_ext_ack *extack)
del_timer_sync(&hw_data->send_timer);
cancel_work_sync(&hw_data->dm_alert_work);
while ((skb = __skb_dequeue(&hw_data->drop_queue))) {
- struct net_dm_hw_metadata *hw_metadata;
+ struct devlink_trap_metadata *hw_metadata;
hw_metadata = NET_DM_SKB_CB(skb)->hw_metadata;
net_dm_hw_metadata_free(hw_metadata);
--
2.26.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH net-next 6/7] drop_monitor: Filter control packets in drop monitor
2020-09-29 8:15 [PATCH net-next 0/7] drop_monitor: Convert to use devlink tracepoint Ido Schimmel
` (4 preceding siblings ...)
2020-09-29 8:15 ` [PATCH net-next 5/7] drop_monitor: Remove duplicate struct Ido Schimmel
@ 2020-09-29 8:15 ` Ido Schimmel
2020-09-29 8:15 ` [PATCH net-next 7/7] selftests: net: Add drop monitor test Ido Schimmel
2020-10-01 1:01 ` [PATCH net-next 0/7] drop_monitor: Convert to use devlink tracepoint David Miller
7 siblings, 0 replies; 9+ messages in thread
From: Ido Schimmel @ 2020-09-29 8:15 UTC (permalink / raw)
To: netdev
Cc: davem, kuba, nhorman, jiri, roopa, aroulin, ayal, masahiroy,
mlxsw, Ido Schimmel
From: Ido Schimmel <idosch@mellanox.com>
Previously, devlink called into drop monitor in order to report hardware
originated drops / exceptions. devlink intentionally filtered control
packets and did not pass them to drop monitor as they were not dropped
by the underlying hardware.
Now drop monitor registers its probe on a generic 'devlink_trap_report'
tracepoint and should therefore perform this filtering itself instead of
having devlink do that.
Add the trap type as metadata and have drop monitor ignore control
packets.
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
include/net/devlink.h | 2 ++
net/core/devlink.c | 8 +-------
net/core/drop_monitor.c | 6 ++++++
3 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/include/net/devlink.h b/include/net/devlink.h
index 1014294ba6a0..1c286e9a3590 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -630,12 +630,14 @@ struct devlink_health_reporter_ops {
* @trap_group_name: Trap group name.
* @input_dev: Input netdevice.
* @fa_cookie: Flow action user cookie.
+ * @trap_type: Trap type.
*/
struct devlink_trap_metadata {
const char *trap_name;
const char *trap_group_name;
struct net_device *input_dev;
const struct flow_action_cookie *fa_cookie;
+ enum devlink_trap_type trap_type;
};
/**
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 2ea9fdc0df2d..6f2863e717a9 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -9269,6 +9269,7 @@ devlink_trap_report_metadata_set(struct devlink_trap_metadata *metadata,
metadata->trap_name = trap_item->trap->name;
metadata->trap_group_name = trap_item->group_item->group->name;
metadata->fa_cookie = fa_cookie;
+ metadata->trap_type = trap_item->trap->type;
spin_lock(&in_devlink_port->type_lock);
if (in_devlink_port->type == DEVLINK_PORT_TYPE_ETH)
@@ -9294,13 +9295,6 @@ void devlink_trap_report(struct devlink *devlink, struct sk_buff *skb,
devlink_trap_stats_update(trap_item->stats, skb->len);
devlink_trap_stats_update(trap_item->group_item->stats, skb->len);
- /* Control packets were not dropped by the device or encountered an
- * exception during forwarding and therefore should not be reported to
- * the kernel's drop monitor.
- */
- if (trap_item->trap->type == DEVLINK_TRAP_TYPE_CONTROL)
- return;
-
if (trace_devlink_trap_report_enabled()) {
struct devlink_trap_metadata metadata = {};
diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c
index 0e4309414a30..a28b743489c5 100644
--- a/net/core/drop_monitor.c
+++ b/net/core/drop_monitor.c
@@ -444,6 +444,9 @@ net_dm_hw_trap_summary_probe(void *ignore, const struct devlink *devlink,
unsigned long flags;
int i;
+ if (metadata->trap_type == DEVLINK_TRAP_TYPE_CONTROL)
+ return;
+
hw_data = this_cpu_ptr(&dm_hw_cpu_data);
spin_lock_irqsave(&hw_data->lock, flags);
hw_entries = hw_data->hw_entries;
@@ -937,6 +940,9 @@ net_dm_hw_trap_packet_probe(void *ignore, const struct devlink *devlink,
struct sk_buff *nskb;
unsigned long flags;
+ if (metadata->trap_type == DEVLINK_TRAP_TYPE_CONTROL)
+ return;
+
if (!skb_mac_header_was_set(skb))
return;
--
2.26.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH net-next 7/7] selftests: net: Add drop monitor test
2020-09-29 8:15 [PATCH net-next 0/7] drop_monitor: Convert to use devlink tracepoint Ido Schimmel
` (5 preceding siblings ...)
2020-09-29 8:15 ` [PATCH net-next 6/7] drop_monitor: Filter control packets in drop monitor Ido Schimmel
@ 2020-09-29 8:15 ` Ido Schimmel
2020-10-01 1:01 ` [PATCH net-next 0/7] drop_monitor: Convert to use devlink tracepoint David Miller
7 siblings, 0 replies; 9+ messages in thread
From: Ido Schimmel @ 2020-09-29 8:15 UTC (permalink / raw)
To: netdev
Cc: davem, kuba, nhorman, jiri, roopa, aroulin, ayal, masahiroy,
mlxsw, Ido Schimmel
From: Ido Schimmel <idosch@mellanox.com>
Test that drop monitor correctly captures both software and hardware
originated packet drops.
# ./drop_monitor_tests.sh
Software drops test
TEST: Capturing active software drops [ OK ]
TEST: Capturing inactive software drops [ OK ]
Hardware drops test
TEST: Capturing active hardware drops [ OK ]
TEST: Capturing inactive hardware drops [ OK ]
Tests passed: 4
Tests failed: 0
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
tools/testing/selftests/net/Makefile | 1 +
tools/testing/selftests/net/config | 3 +
.../selftests/net/drop_monitor_tests.sh | 215 ++++++++++++++++++
3 files changed, 219 insertions(+)
create mode 100755 tools/testing/selftests/net/drop_monitor_tests.sh
diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index 9491bbaa0831..52923eb08934 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -19,6 +19,7 @@ TEST_PROGS += txtimestamp.sh
TEST_PROGS += vrf-xfrm-tests.sh
TEST_PROGS += rxtimestamp.sh
TEST_PROGS += devlink_port_split.py
+TEST_PROGS += drop_monitor_tests.sh
TEST_PROGS_EXTENDED := in_netns.sh
TEST_GEN_FILES = socket nettest
TEST_GEN_FILES += psock_fanout psock_tpacket msg_zerocopy reuseport_addr_any
diff --git a/tools/testing/selftests/net/config b/tools/testing/selftests/net/config
index 5a57ea02802d..43649242adc0 100644
--- a/tools/testing/selftests/net/config
+++ b/tools/testing/selftests/net/config
@@ -30,3 +30,6 @@ CONFIG_NET_SCH_ETF=m
CONFIG_NET_SCH_NETEM=y
CONFIG_TEST_BLACKHOLE_DEV=m
CONFIG_KALLSYMS=y
+CONFIG_TRACEPOINTS=y
+CONFIG_NET_DROP_MONITOR=m
+CONFIG_NETDEVSIM=m
diff --git a/tools/testing/selftests/net/drop_monitor_tests.sh b/tools/testing/selftests/net/drop_monitor_tests.sh
new file mode 100755
index 000000000000..b7650e30d18b
--- /dev/null
+++ b/tools/testing/selftests/net/drop_monitor_tests.sh
@@ -0,0 +1,215 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+# This test is for checking drop monitor functionality.
+
+ret=0
+# Kselftest framework requirement - SKIP code is 4.
+ksft_skip=4
+
+# all tests in this script. Can be overridden with -t option
+TESTS="
+ sw_drops
+ hw_drops
+"
+
+IP="ip -netns ns1"
+TC="tc -netns ns1"
+DEVLINK="devlink -N ns1"
+NS_EXEC="ip netns exec ns1"
+NETDEVSIM_PATH=/sys/bus/netdevsim/
+DEV_ADDR=1337
+DEV=netdevsim${DEV_ADDR}
+DEVLINK_DEV=netdevsim/${DEV}
+
+log_test()
+{
+ local rc=$1
+ local expected=$2
+ local msg="$3"
+
+ if [ ${rc} -eq ${expected} ]; then
+ printf " TEST: %-60s [ OK ]\n" "${msg}"
+ nsuccess=$((nsuccess+1))
+ else
+ ret=1
+ nfail=$((nfail+1))
+ printf " TEST: %-60s [FAIL]\n" "${msg}"
+ fi
+}
+
+setup()
+{
+ modprobe netdevsim &> /dev/null
+
+ set -e
+ ip netns add ns1
+ $IP link add dummy10 up type dummy
+
+ $NS_EXEC echo "$DEV_ADDR 1" > ${NETDEVSIM_PATH}/new_device
+ udevadm settle
+ local netdev=$($NS_EXEC ls ${NETDEVSIM_PATH}/devices/${DEV}/net/)
+ $IP link set dev $netdev up
+
+ set +e
+}
+
+cleanup()
+{
+ $NS_EXEC echo "$DEV_ADDR" > ${NETDEVSIM_PATH}/del_device
+ ip netns del ns1
+}
+
+sw_drops_test()
+{
+ echo
+ echo "Software drops test"
+
+ setup
+
+ local dir=$(mktemp -d)
+
+ $TC qdisc add dev dummy10 clsact
+ $TC filter add dev dummy10 egress pref 1 handle 101 proto ip \
+ flower dst_ip 192.0.2.10 action drop
+
+ $NS_EXEC mausezahn dummy10 -a 00:11:22:33:44:55 -b 00:aa:bb:cc:dd:ee \
+ -A 192.0.2.1 -B 192.0.2.10 -t udp sp=12345,dp=54321 -c 0 -q \
+ -d 100msec &
+ timeout 5 dwdump -o sw -w ${dir}/packets.pcap
+ (( $(tshark -r ${dir}/packets.pcap \
+ -Y 'ip.dst == 192.0.2.10' 2> /dev/null | wc -l) != 0))
+ log_test $? 0 "Capturing active software drops"
+
+ rm ${dir}/packets.pcap
+
+ { kill %% && wait %%; } 2>/dev/null
+ timeout 5 dwdump -o sw -w ${dir}/packets.pcap
+ (( $(tshark -r ${dir}/packets.pcap \
+ -Y 'ip.dst == 192.0.2.10' 2> /dev/null | wc -l) == 0))
+ log_test $? 0 "Capturing inactive software drops"
+
+ rm -r $dir
+
+ cleanup
+}
+
+hw_drops_test()
+{
+ echo
+ echo "Hardware drops test"
+
+ setup
+
+ local dir=$(mktemp -d)
+
+ $DEVLINK trap set $DEVLINK_DEV trap blackhole_route action trap
+ timeout 5 dwdump -o hw -w ${dir}/packets.pcap
+ (( $(tshark -r ${dir}/packets.pcap \
+ -Y 'net_dm.hw_trap_name== blackhole_route' 2> /dev/null \
+ | wc -l) != 0))
+ log_test $? 0 "Capturing active hardware drops"
+
+ rm ${dir}/packets.pcap
+
+ $DEVLINK trap set $DEVLINK_DEV trap blackhole_route action drop
+ timeout 5 dwdump -o hw -w ${dir}/packets.pcap
+ (( $(tshark -r ${dir}/packets.pcap \
+ -Y 'net_dm.hw_trap_name== blackhole_route' 2> /dev/null \
+ | wc -l) == 0))
+ log_test $? 0 "Capturing inactive hardware drops"
+
+ rm -r $dir
+
+ cleanup
+}
+
+################################################################################
+# usage
+
+usage()
+{
+ cat <<EOF
+usage: ${0##*/} OPTS
+
+ -t <test> Test(s) to run (default: all)
+ (options: $TESTS)
+EOF
+}
+
+################################################################################
+# main
+
+while getopts ":t:h" opt; do
+ case $opt in
+ t) TESTS=$OPTARG;;
+ h) usage; exit 0;;
+ *) usage; exit 1;;
+ esac
+done
+
+if [ "$(id -u)" -ne 0 ];then
+ echo "SKIP: Need root privileges"
+ exit $ksft_skip;
+fi
+
+if [ ! -x "$(command -v ip)" ]; then
+ echo "SKIP: Could not run test without ip tool"
+ exit $ksft_skip
+fi
+
+if [ ! -x "$(command -v devlink)" ]; then
+ echo "SKIP: Could not run test without devlink tool"
+ exit $ksft_skip
+fi
+
+if [ ! -x "$(command -v tshark)" ]; then
+ echo "SKIP: Could not run test without tshark tool"
+ exit $ksft_skip
+fi
+
+if [ ! -x "$(command -v dwdump)" ]; then
+ echo "SKIP: Could not run test without dwdump tool"
+ exit $ksft_skip
+fi
+
+if [ ! -x "$(command -v udevadm)" ]; then
+ echo "SKIP: Could not run test without udevadm tool"
+ exit $ksft_skip
+fi
+
+if [ ! -x "$(command -v timeout)" ]; then
+ echo "SKIP: Could not run test without timeout tool"
+ exit $ksft_skip
+fi
+
+if [ ! -x "$(command -v mausezahn)" ]; then
+ echo "SKIP: Could not run test without mausezahn tool"
+ exit $ksft_skip
+fi
+
+tshark -G fields 2> /dev/null | grep -q net_dm
+if [ $? -ne 0 ]; then
+ echo "SKIP: tshark too old, missing net_dm dissector"
+ exit $ksft_skip
+fi
+
+# start clean
+cleanup &> /dev/null
+
+for t in $TESTS
+do
+ case $t in
+ sw_drops|sw) sw_drops_test;;
+ hw_drops|hw) hw_drops_test;;
+
+ help) echo "Test names: $TESTS"; exit 0;;
+ esac
+done
+
+if [ "$TESTS" != "none" ]; then
+ printf "\nTests passed: %3d\n" ${nsuccess}
+ printf "Tests failed: %3d\n" ${nfail}
+fi
+
+exit $ret
--
2.26.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH net-next 0/7] drop_monitor: Convert to use devlink tracepoint
2020-09-29 8:15 [PATCH net-next 0/7] drop_monitor: Convert to use devlink tracepoint Ido Schimmel
` (6 preceding siblings ...)
2020-09-29 8:15 ` [PATCH net-next 7/7] selftests: net: Add drop monitor test Ido Schimmel
@ 2020-10-01 1:01 ` David Miller
7 siblings, 0 replies; 9+ messages in thread
From: David Miller @ 2020-10-01 1:01 UTC (permalink / raw)
To: idosch
Cc: netdev, kuba, nhorman, jiri, roopa, aroulin, ayal, masahiroy,
mlxsw, idosch
From: Ido Schimmel <idosch@idosch.org>
Date: Tue, 29 Sep 2020 11:15:49 +0300
> From: Ido Schimmel <idosch@nvidia.com>
>
> Drop monitor is able to monitor both software and hardware originated
> drops. Software drops are monitored by having drop monitor register its
> probe on the 'kfree_skb' tracepoint. Hardware originated drops are
> monitored by having devlink call into drop monitor whenever it receives
> a dropped packet from the underlying hardware.
>
> This patch set converts drop monitor to monitor both software and
> hardware originated drops in the same way - by registering its probe on
> the relevant tracepoint.
>
> In addition to drop monitor being more consistent, it is now also
> possible to build drop monitor as module instead of as a builtin and
> still monitor hardware originated drops. Initially, CONFIG_NET_DEVLINK
> implied CONFIG_NET_DROP_MONITOR, but after commit def2fbffe62c
> ("kconfig: allow symbols implied by y to become m") we can have
> CONFIG_NET_DEVLINK=y and CONFIG_NET_DROP_MONITOR=m and hardware
> originated drops will not be monitored.
...
Series applied, thank you.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-10-01 1:01 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-29 8:15 [PATCH net-next 0/7] drop_monitor: Convert to use devlink tracepoint Ido Schimmel
2020-09-29 8:15 ` [PATCH net-next 1/7] devlink: Add a tracepoint for trap reports Ido Schimmel
2020-09-29 8:15 ` [PATCH net-next 2/7] drop_monitor: Prepare probe functions for devlink tracepoint Ido Schimmel
2020-09-29 8:15 ` [PATCH net-next 3/7] drop_monitor: Convert to using " Ido Schimmel
2020-09-29 8:15 ` [PATCH net-next 4/7] drop_monitor: Remove no longer used functions Ido Schimmel
2020-09-29 8:15 ` [PATCH net-next 5/7] drop_monitor: Remove duplicate struct Ido Schimmel
2020-09-29 8:15 ` [PATCH net-next 6/7] drop_monitor: Filter control packets in drop monitor Ido Schimmel
2020-09-29 8:15 ` [PATCH net-next 7/7] selftests: net: Add drop monitor test Ido Schimmel
2020-10-01 1:01 ` [PATCH net-next 0/7] drop_monitor: Convert to use devlink tracepoint David Miller
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).