* [patch net-next 01/10] flow_offload: pass action cookie through offload structures
2020-02-24 21:07 [patch net-next 00/10] mlxsw: Implement ACL-dropped packets identification Jiri Pirko
@ 2020-02-24 21:07 ` Jiri Pirko
2020-02-25 4:16 ` Jakub Kicinski
2020-02-24 21:07 ` [patch net-next 02/10] devlink: add trap metadata type for cookie Jiri Pirko
` (8 subsequent siblings)
9 siblings, 1 reply; 23+ messages in thread
From: Jiri Pirko @ 2020-02-24 21:07 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, nhorman, jhs, xiyou.wangcong, idosch, mlxsw
From: Jiri Pirko <jiri@mellanox.com>
Extend struct flow_action_entry in order to hold TC action cookie
specified by user inserting the action.
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
include/net/flow_offload.h | 11 +++++++++++
net/core/flow_offload.c | 21 +++++++++++++++++++++
net/sched/cls_api.c | 31 ++++++++++++++++++++++++++++++-
3 files changed, 62 insertions(+), 1 deletion(-)
diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h
index c6f7bd22db60..4d72224de438 100644
--- a/include/net/flow_offload.h
+++ b/include/net/flow_offload.h
@@ -156,6 +156,16 @@ enum flow_action_mangle_base {
typedef void (*action_destr)(void *priv);
+struct flow_action_cookie {
+ unsigned int cookie_len;
+ unsigned long cookie[0];
+};
+
+struct flow_action_cookie *flow_action_cookie_create(void *data,
+ unsigned int len,
+ gfp_t gfp);
+void flow_action_cookie_destroy(struct flow_action_cookie *cookie);
+
struct flow_action_entry {
enum flow_action_id id;
action_destr destructor;
@@ -214,6 +224,7 @@ struct flow_action_entry {
u8 ttl;
} mpls_mangle;
};
+ struct flow_action_cookie *cookie; /* user defined action cookie */
};
struct flow_action {
diff --git a/net/core/flow_offload.c b/net/core/flow_offload.c
index 45b6a59ac124..d21348202ba6 100644
--- a/net/core/flow_offload.c
+++ b/net/core/flow_offload.c
@@ -167,6 +167,27 @@ void flow_rule_match_enc_opts(const struct flow_rule *rule,
}
EXPORT_SYMBOL(flow_rule_match_enc_opts);
+struct flow_action_cookie *flow_action_cookie_create(void *data,
+ unsigned int len,
+ gfp_t gfp)
+{
+ struct flow_action_cookie *cookie;
+
+ cookie = kmalloc(sizeof(*cookie) + len, gfp);
+ if (!cookie)
+ return NULL;
+ cookie->cookie_len = len;
+ memcpy(cookie->cookie, data, len);
+ return cookie;
+}
+EXPORT_SYMBOL(flow_action_cookie_create);
+
+void flow_action_cookie_destroy(struct flow_action_cookie *cookie)
+{
+ kfree(cookie);
+}
+EXPORT_SYMBOL(flow_action_cookie_destroy);
+
struct flow_block_cb *flow_block_cb_alloc(flow_setup_cb_t *cb,
void *cb_ident, void *cb_priv,
void (*release)(void *cb_priv))
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 13c33eaf1ca1..4e766c5ab77a 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -3382,14 +3382,40 @@ int tc_setup_cb_reoffload(struct tcf_block *block, struct tcf_proto *tp,
}
EXPORT_SYMBOL(tc_setup_cb_reoffload);
+static int tcf_act_get_cookie(struct flow_action_entry *entry,
+ const struct tc_action *act)
+{
+ struct tc_cookie *cookie;
+ int err = 0;
+
+ rcu_read_lock();
+ cookie = rcu_dereference(act->act_cookie);
+ if (cookie) {
+ entry->cookie = flow_action_cookie_create(cookie->data,
+ cookie->len,
+ GFP_ATOMIC);
+ if (!entry->cookie)
+ err = -ENOMEM;
+ }
+ rcu_read_unlock();
+ return err;
+}
+
+static void tcf_act_put_cookie(struct flow_action_entry *entry)
+{
+ flow_action_cookie_destroy(entry->cookie);
+}
+
void tc_cleanup_flow_action(struct flow_action *flow_action)
{
struct flow_action_entry *entry;
int i;
- flow_action_for_each(i, entry, flow_action)
+ flow_action_for_each(i, entry, flow_action) {
+ tcf_act_put_cookie(entry);
if (entry->destructor)
entry->destructor(entry->destructor_priv);
+ }
}
EXPORT_SYMBOL(tc_cleanup_flow_action);
@@ -3447,6 +3473,9 @@ int tc_setup_flow_action(struct flow_action *flow_action,
entry = &flow_action->entries[j];
spin_lock_bh(&act->tcfa_lock);
+ err = tcf_act_get_cookie(entry, act);
+ if (err)
+ goto err_out_locked;
if (is_tcf_gact_ok(act)) {
entry->id = FLOW_ACTION_ACCEPT;
} else if (is_tcf_gact_shot(act)) {
--
2.21.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [patch net-next 02/10] devlink: add trap metadata type for cookie
2020-02-24 21:07 [patch net-next 00/10] mlxsw: Implement ACL-dropped packets identification Jiri Pirko
2020-02-24 21:07 ` [patch net-next 01/10] flow_offload: pass action cookie through offload structures Jiri Pirko
@ 2020-02-24 21:07 ` Jiri Pirko
2020-02-25 4:19 ` Jakub Kicinski
2020-02-24 21:07 ` [patch net-next 03/10] drop_monitor: extend by passing cookie from driver Jiri Pirko
` (7 subsequent siblings)
9 siblings, 1 reply; 23+ messages in thread
From: Jiri Pirko @ 2020-02-24 21:07 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, nhorman, jhs, xiyou.wangcong, idosch, mlxsw
From: Jiri Pirko <jiri@mellanox.com>
Allow driver to indicate cookie metadata for registered traps.
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
include/net/devlink.h | 1 +
include/uapi/linux/devlink.h | 2 ++
net/core/devlink.c | 3 +++
3 files changed, 6 insertions(+)
diff --git a/include/net/devlink.h b/include/net/devlink.h
index 07923e619206..014a8b3d1499 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -541,6 +541,7 @@ struct devlink_trap_group {
};
#define DEVLINK_TRAP_METADATA_TYPE_F_IN_PORT BIT(0)
+#define DEVLINK_TRAP_METADATA_TYPE_F_FA_COOKIE BIT(1)
/**
* struct devlink_trap - Immutable packet trap attributes.
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index ae37fd4d194a..be2a2948f452 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -252,6 +252,8 @@ enum devlink_trap_type {
enum {
/* Trap can report input port as metadata */
DEVLINK_ATTR_TRAP_METADATA_TYPE_IN_PORT,
+ /* Trap can report flow action cookie as metadata */
+ DEVLINK_ATTR_TRAP_METADATA_TYPE_FA_COOKIE,
};
enum devlink_attr {
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 0d7c5d3443d2..12e6ef749b8a 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -5540,6 +5540,9 @@ static int devlink_trap_metadata_put(struct sk_buff *msg,
if ((trap->metadata_cap & DEVLINK_TRAP_METADATA_TYPE_F_IN_PORT) &&
nla_put_flag(msg, DEVLINK_ATTR_TRAP_METADATA_TYPE_IN_PORT))
goto nla_put_failure;
+ if ((trap->metadata_cap & DEVLINK_TRAP_METADATA_TYPE_F_FA_COOKIE) &&
+ nla_put_flag(msg, DEVLINK_ATTR_TRAP_METADATA_TYPE_FA_COOKIE))
+ goto nla_put_failure;
nla_nest_end(msg, attr);
--
2.21.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [patch net-next 03/10] drop_monitor: extend by passing cookie from driver
2020-02-24 21:07 [patch net-next 00/10] mlxsw: Implement ACL-dropped packets identification Jiri Pirko
2020-02-24 21:07 ` [patch net-next 01/10] flow_offload: pass action cookie through offload structures Jiri Pirko
2020-02-24 21:07 ` [patch net-next 02/10] devlink: add trap metadata type for cookie Jiri Pirko
@ 2020-02-24 21:07 ` Jiri Pirko
2020-02-25 4:19 ` Jakub Kicinski
2020-02-24 21:07 ` [patch net-next 04/10] devlink: extend devlink_trap_report() to accept cookie and pass Jiri Pirko
` (6 subsequent siblings)
9 siblings, 1 reply; 23+ messages in thread
From: Jiri Pirko @ 2020-02-24 21:07 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, nhorman, jhs, xiyou.wangcong, idosch, mlxsw
From: Jiri Pirko <jiri@mellanox.com>
If driver passed along the cookie, push it through Netlink.
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
include/net/drop_monitor.h | 3 +++
include/uapi/linux/net_dropmon.h | 1 +
net/core/drop_monitor.c | 33 +++++++++++++++++++++++++++++++-
3 files changed, 36 insertions(+), 1 deletion(-)
diff --git a/include/net/drop_monitor.h b/include/net/drop_monitor.h
index 2ab668461463..ddd441a60e03 100644
--- a/include/net/drop_monitor.h
+++ b/include/net/drop_monitor.h
@@ -6,17 +6,20 @@
#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_ENABLED(CONFIG_NET_DROP_MONITOR)
diff --git a/include/uapi/linux/net_dropmon.h b/include/uapi/linux/net_dropmon.h
index 8bf79a9eb234..66048cc5d7b3 100644
--- a/include/uapi/linux/net_dropmon.h
+++ b/include/uapi/linux/net_dropmon.h
@@ -92,6 +92,7 @@ enum net_dm_attr {
NET_DM_ATTR_HW_TRAP_COUNT, /* u32 */
NET_DM_ATTR_SW_DROPS, /* flag */
NET_DM_ATTR_HW_DROPS, /* flag */
+ NET_DM_ATTR_FLOW_ACTION_COOKIE, /* binary */
__NET_DM_ATTR_MAX,
NET_DM_ATTR_MAX = __NET_DM_ATTR_MAX - 1
diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c
index 31700e0c3928..185c9f3a77cc 100644
--- a/net/core/drop_monitor.c
+++ b/net/core/drop_monitor.c
@@ -29,6 +29,7 @@
#include <net/drop_monitor.h>
#include <net/genetlink.h>
#include <net/netevent.h>
+#include <net/flow_offload.h>
#include <trace/events/skb.h>
#include <trace/events/napi.h>
@@ -700,6 +701,13 @@ static void net_dm_packet_work(struct work_struct *work)
net_dm_packet_report(skb);
}
+static size_t
+net_dm_flow_action_cookie_size(const struct net_dm_hw_metadata *hw_metadata)
+{
+ return hw_metadata->fa_cookie ?
+ nla_total_size(hw_metadata->fa_cookie->cookie_len) : 0;
+}
+
static size_t
net_dm_hw_packet_report_size(size_t payload_len,
const struct net_dm_hw_metadata *hw_metadata)
@@ -717,6 +725,8 @@ net_dm_hw_packet_report_size(size_t payload_len,
nla_total_size(strlen(hw_metadata->trap_name) + 1) +
/* NET_DM_ATTR_IN_PORT */
net_dm_in_port_size() +
+ /* NET_DM_ATTR_FLOW_ACTION_COOKIE */
+ net_dm_flow_action_cookie_size(hw_metadata) +
/* NET_DM_ATTR_TIMESTAMP */
nla_total_size(sizeof(u64)) +
/* NET_DM_ATTR_ORIG_LEN */
@@ -762,6 +772,12 @@ static int net_dm_hw_packet_report_fill(struct sk_buff *msg,
goto nla_put_failure;
}
+ if (hw_metadata->fa_cookie &&
+ nla_put(msg, NET_DM_ATTR_FLOW_ACTION_COOKIE,
+ hw_metadata->fa_cookie->cookie_len,
+ hw_metadata->fa_cookie->cookie))
+ goto nla_put_failure;
+
if (nla_put_u64_64bit(msg, NET_DM_ATTR_TIMESTAMP,
ktime_to_ns(skb->tstamp), NET_DM_ATTR_PAD))
goto nla_put_failure;
@@ -794,11 +810,12 @@ static int net_dm_hw_packet_report_fill(struct sk_buff *msg,
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 = kmalloc(sizeof(*hw_metadata), GFP_ATOMIC);
+ n_hw_metadata = kzalloc(sizeof(*hw_metadata), GFP_ATOMIC);
if (!n_hw_metadata)
return NULL;
@@ -812,12 +829,25 @@ net_dm_hw_metadata_clone(const struct net_dm_hw_metadata *hw_metadata)
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 | __GFP_ZERO);
+ 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:
@@ -830,6 +860,7 @@ net_dm_hw_metadata_free(const struct net_dm_hw_metadata *hw_metadata)
{
if (hw_metadata->input_dev)
dev_put(hw_metadata->input_dev);
+ kfree(hw_metadata->fa_cookie);
kfree(hw_metadata->trap_name);
kfree(hw_metadata->trap_group_name);
kfree(hw_metadata);
--
2.21.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [patch net-next 03/10] drop_monitor: extend by passing cookie from driver
2020-02-24 21:07 ` [patch net-next 03/10] drop_monitor: extend by passing cookie from driver Jiri Pirko
@ 2020-02-25 4:19 ` Jakub Kicinski
2020-02-25 7:04 ` Jiri Pirko
0 siblings, 1 reply; 23+ messages in thread
From: Jakub Kicinski @ 2020-02-25 4:19 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, davem, nhorman, jhs, xiyou.wangcong, idosch, mlxsw
On Mon, 24 Feb 2020 22:07:51 +0100, Jiri Pirko wrote:
> + fa_cookie = kmemdup(hw_metadata->fa_cookie, cookie_size,
> + GFP_ATOMIC | __GFP_ZERO);
nit: kmemdup with GFP_ZERO seems like a strange combination
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [patch net-next 03/10] drop_monitor: extend by passing cookie from driver
2020-02-25 4:19 ` Jakub Kicinski
@ 2020-02-25 7:04 ` Jiri Pirko
0 siblings, 0 replies; 23+ messages in thread
From: Jiri Pirko @ 2020-02-25 7:04 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev, davem, nhorman, jhs, xiyou.wangcong, idosch, mlxsw
Tue, Feb 25, 2020 at 05:19:10AM CET, kuba@kernel.org wrote:
>On Mon, 24 Feb 2020 22:07:51 +0100, Jiri Pirko wrote:
>> + fa_cookie = kmemdup(hw_metadata->fa_cookie, cookie_size,
>> + GFP_ATOMIC | __GFP_ZERO);
>
>nit: kmemdup with GFP_ZERO seems like a strange combination
Okay, removed.
^ permalink raw reply [flat|nested] 23+ messages in thread
* [patch net-next 04/10] devlink: extend devlink_trap_report() to accept cookie and pass
2020-02-24 21:07 [patch net-next 00/10] mlxsw: Implement ACL-dropped packets identification Jiri Pirko
` (2 preceding siblings ...)
2020-02-24 21:07 ` [patch net-next 03/10] drop_monitor: extend by passing cookie from driver Jiri Pirko
@ 2020-02-24 21:07 ` Jiri Pirko
2020-02-25 4:20 ` Jakub Kicinski
2020-02-24 21:07 ` [patch net-next 05/10] mlxsw: core_acl_flex_actions: Add trap with userdef action Jiri Pirko
` (5 subsequent siblings)
9 siblings, 1 reply; 23+ messages in thread
From: Jiri Pirko @ 2020-02-24 21:07 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, nhorman, jhs, xiyou.wangcong, idosch, mlxsw
From: Jiri Pirko <jiri@mellanox.com>
Add cookie argument to devlink_trap_report() allowing driver to pass in
the user cookie. Pass on the cookie down to drop monitor code.
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.c | 4 ++--
drivers/net/netdevsim/dev.c | 2 +-
include/net/devlink.h | 7 ++++---
net/core/devlink.c | 11 ++++++++---
4 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.c
index 04f2445f6d43..a55577a50e90 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.c
@@ -71,7 +71,7 @@ static void mlxsw_sp_rx_drop_listener(struct sk_buff *skb, u8 local_port,
in_devlink_port = mlxsw_core_port_devlink_port_get(mlxsw_sp->core,
local_port);
skb_push(skb, ETH_HLEN);
- devlink_trap_report(devlink, skb, trap_ctx, in_devlink_port);
+ devlink_trap_report(devlink, skb, trap_ctx, in_devlink_port, NULL);
consume_skb(skb);
}
@@ -95,7 +95,7 @@ static void mlxsw_sp_rx_exception_listener(struct sk_buff *skb, u8 local_port,
in_devlink_port = mlxsw_core_port_devlink_port_get(mlxsw_sp->core,
local_port);
skb_push(skb, ETH_HLEN);
- devlink_trap_report(devlink, skb, trap_ctx, in_devlink_port);
+ devlink_trap_report(devlink, skb, trap_ctx, in_devlink_port, NULL);
skb_pull(skb, ETH_HLEN);
skb->offload_fwd_mark = 1;
netif_receive_skb(skb);
diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index d7706a0346f2..aa17533c06e1 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -385,7 +385,7 @@ static void nsim_dev_trap_report(struct nsim_dev_port *nsim_dev_port)
*/
local_bh_disable();
devlink_trap_report(devlink, skb, nsim_trap_item->trap_ctx,
- &nsim_dev_port->devlink_port);
+ &nsim_dev_port->devlink_port, NULL);
local_bh_enable();
consume_skb(skb);
}
diff --git a/include/net/devlink.h b/include/net/devlink.h
index 014a8b3d1499..c9ca86b054bc 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -16,6 +16,7 @@
#include <linux/workqueue.h>
#include <linux/refcount.h>
#include <net/net_namespace.h>
+#include <net/flow_offload.h>
#include <uapi/linux/devlink.h>
struct devlink_ops;
@@ -1050,9 +1051,9 @@ int devlink_traps_register(struct devlink *devlink,
void devlink_traps_unregister(struct devlink *devlink,
const struct devlink_trap *traps,
size_t traps_count);
-void devlink_trap_report(struct devlink *devlink,
- struct sk_buff *skb, void *trap_ctx,
- struct devlink_port *in_devlink_port);
+void devlink_trap_report(struct devlink *devlink, struct sk_buff *skb,
+ void *trap_ctx, struct devlink_port *in_devlink_port,
+ const struct flow_action_cookie *fa_cookie);
void *devlink_trap_ctx_priv(void *trap_ctx);
#if IS_ENABLED(CONFIG_NET_DEVLINK)
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 12e6ef749b8a..49706031ab45 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -8205,12 +8205,14 @@ devlink_trap_stats_update(struct devlink_stats __percpu *trap_stats,
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)
+ 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)
@@ -8224,9 +8226,12 @@ devlink_trap_report_metadata_fill(struct net_dm_hw_metadata *hw_metadata,
* @skb: Trapped packet.
* @trap_ctx: Trap context.
* @in_devlink_port: Input devlink port.
+ * @fa_cookie: Flow action cookie. Could be NULL.
*/
void devlink_trap_report(struct devlink *devlink, struct sk_buff *skb,
- void *trap_ctx, struct devlink_port *in_devlink_port)
+ void *trap_ctx, struct devlink_port *in_devlink_port,
+ const struct flow_action_cookie *fa_cookie)
+
{
struct devlink_trap_item *trap_item = trap_ctx;
struct net_dm_hw_metadata hw_metadata = {};
@@ -8235,7 +8240,7 @@ void devlink_trap_report(struct devlink *devlink, struct sk_buff *skb,
devlink_trap_stats_update(trap_item->group_item->stats, skb->len);
devlink_trap_report_metadata_fill(&hw_metadata, trap_item,
- in_devlink_port);
+ in_devlink_port, fa_cookie);
net_dm_hw_report(skb, &hw_metadata);
}
EXPORT_SYMBOL_GPL(devlink_trap_report);
--
2.21.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [patch net-next 05/10] mlxsw: core_acl_flex_actions: Add trap with userdef action
2020-02-24 21:07 [patch net-next 00/10] mlxsw: Implement ACL-dropped packets identification Jiri Pirko
` (3 preceding siblings ...)
2020-02-24 21:07 ` [patch net-next 04/10] devlink: extend devlink_trap_report() to accept cookie and pass Jiri Pirko
@ 2020-02-24 21:07 ` Jiri Pirko
2020-02-24 21:07 ` [patch net-next 06/10] mlxsw: core_acl_flex_actions: Implement flow_offload action cookie offload Jiri Pirko
` (4 subsequent siblings)
9 siblings, 0 replies; 23+ messages in thread
From: Jiri Pirko @ 2020-02-24 21:07 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, nhorman, jhs, xiyou.wangcong, idosch, mlxsw
From: Jiri Pirko <jiri@mellanox.com>
Expose "Trap action with userdef". It is the same as already
defined "Trap action" with a difference that it would ask the policy
engine to pass arbitrary value (userdef) alongside with received packets.
This would be later on used to carry cookie index.
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
.../mellanox/mlxsw/core_acl_flex_actions.c | 30 +++++++++++++++++--
1 file changed, 28 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_acl_flex_actions.c b/drivers/net/ethernet/mellanox/mlxsw/core_acl_flex_actions.c
index 424ef26e6cca..b7a846dd8f32 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core_acl_flex_actions.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core_acl_flex_actions.c
@@ -747,18 +747,25 @@ int mlxsw_afa_block_append_vlan_modify(struct mlxsw_afa_block *block,
}
EXPORT_SYMBOL(mlxsw_afa_block_append_vlan_modify);
-/* Trap Action
- * -----------
+/* Trap Action / Trap With Userdef Action
+ * --------------------------------------
* The Trap action enables trapping / mirroring packets to the CPU
* as well as discarding packets.
* The ACL Trap / Discard separates the forward/discard control from CPU
* trap control. In addition, the Trap / Discard action enables activating
* SPAN (port mirroring).
+ *
+ * The Trap with userdef action action has the same functionality as
+ * the Trap action with addition of user defined value that can be set
+ * and used by higher layer applications.
*/
#define MLXSW_AFA_TRAP_CODE 0x03
#define MLXSW_AFA_TRAP_SIZE 1
+#define MLXSW_AFA_TRAPWU_CODE 0x04
+#define MLXSW_AFA_TRAPWU_SIZE 2
+
enum mlxsw_afa_trap_trap_action {
MLXSW_AFA_TRAP_TRAP_ACTION_NOP = 0,
MLXSW_AFA_TRAP_TRAP_ACTION_TRAP = 2,
@@ -794,6 +801,15 @@ MLXSW_ITEM32(afa, trap, mirror_agent, 0x08, 29, 3);
*/
MLXSW_ITEM32(afa, trap, mirror_enable, 0x08, 24, 1);
+/* user_def_val
+ * Value for the SW usage. Can be used to pass information of which
+ * rule has caused a trap. This may be overwritten by later traps.
+ * This field does a set on the packet's user_def_val only if this
+ * is the first trap_id or if the trap_id has replaced the previous
+ * packet's trap_id.
+ */
+MLXSW_ITEM32(afa, trap, user_def_val, 0x0C, 0, 20);
+
static inline void
mlxsw_afa_trap_pack(char *payload,
enum mlxsw_afa_trap_trap_action trap_action,
@@ -805,6 +821,16 @@ mlxsw_afa_trap_pack(char *payload,
mlxsw_afa_trap_trap_id_set(payload, trap_id);
}
+static inline void
+mlxsw_afa_trapwu_pack(char *payload,
+ enum mlxsw_afa_trap_trap_action trap_action,
+ enum mlxsw_afa_trap_forward_action forward_action,
+ u16 trap_id, u32 user_def_val)
+{
+ mlxsw_afa_trap_pack(payload, trap_action, forward_action, trap_id);
+ mlxsw_afa_trap_user_def_val_set(payload, user_def_val);
+}
+
static inline void
mlxsw_afa_trap_mirror_pack(char *payload, bool mirror_enable,
u8 mirror_agent)
--
2.21.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [patch net-next 06/10] mlxsw: core_acl_flex_actions: Implement flow_offload action cookie offload
2020-02-24 21:07 [patch net-next 00/10] mlxsw: Implement ACL-dropped packets identification Jiri Pirko
` (4 preceding siblings ...)
2020-02-24 21:07 ` [patch net-next 05/10] mlxsw: core_acl_flex_actions: Add trap with userdef action Jiri Pirko
@ 2020-02-24 21:07 ` Jiri Pirko
2020-02-24 21:07 ` [patch net-next 07/10] mlxsw: pci: Extract cookie index for ACL discard trap packets Jiri Pirko
` (3 subsequent siblings)
9 siblings, 0 replies; 23+ messages in thread
From: Jiri Pirko @ 2020-02-24 21:07 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, nhorman, jhs, xiyou.wangcong, idosch, mlxsw
From: Jiri Pirko <jiri@mellanox.com>
Track cookies coming down to driver by flow_offload.
Assign a cookie_index to each unique cookie binary. Use previously
defined "Trap with userdef" flex action to ask HW to pass cookie_index
alongside with the dropped packets.
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
.../mellanox/mlxsw/core_acl_flex_actions.c | 243 +++++++++++++++++-
.../mellanox/mlxsw/core_acl_flex_actions.h | 5 +-
.../net/ethernet/mellanox/mlxsw/spectrum.h | 5 +-
.../ethernet/mellanox/mlxsw/spectrum_acl.c | 7 +-
.../ethernet/mellanox/mlxsw/spectrum_flower.c | 3 +-
5 files changed, 257 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_acl_flex_actions.c b/drivers/net/ethernet/mellanox/mlxsw/core_acl_flex_actions.c
index b7a846dd8f32..9fad56df8303 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core_acl_flex_actions.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core_acl_flex_actions.c
@@ -7,6 +7,9 @@
#include <linux/errno.h>
#include <linux/rhashtable.h>
#include <linux/list.h>
+#include <linux/idr.h>
+#include <linux/refcount.h>
+#include <net/flow_offload.h>
#include "item.h"
#include "trap.h"
@@ -63,6 +66,8 @@ struct mlxsw_afa {
void *ops_priv;
struct rhashtable set_ht;
struct rhashtable fwd_entry_ht;
+ struct rhashtable cookie_ht;
+ struct idr cookie_idr;
};
#define MLXSW_AFA_SET_LEN 0xA8
@@ -121,6 +126,55 @@ static const struct rhashtable_params mlxsw_afa_fwd_entry_ht_params = {
.automatic_shrinking = true,
};
+struct mlxsw_afa_cookie {
+ struct rhash_head ht_node;
+ refcount_t ref_count;
+ struct rcu_head rcu;
+ u32 cookie_index;
+ struct flow_action_cookie fa_cookie;
+};
+
+static u32 mlxsw_afa_cookie_hash(const struct flow_action_cookie *fa_cookie,
+ u32 seed)
+{
+ return jhash2((u32 *) fa_cookie->cookie,
+ fa_cookie->cookie_len / sizeof(u32), seed);
+}
+
+static u32 mlxsw_afa_cookie_key_hashfn(const void *data, u32 len, u32 seed)
+{
+ const struct flow_action_cookie *fa_cookie = data;
+
+ return mlxsw_afa_cookie_hash(fa_cookie, seed);
+}
+
+static u32 mlxsw_afa_cookie_obj_hashfn(const void *data, u32 len, u32 seed)
+{
+ const struct mlxsw_afa_cookie *cookie = data;
+
+ return mlxsw_afa_cookie_hash(&cookie->fa_cookie, seed);
+}
+
+static int mlxsw_afa_cookie_obj_cmpfn(struct rhashtable_compare_arg *arg,
+ const void *obj)
+{
+ const struct flow_action_cookie *fa_cookie = arg->key;
+ const struct mlxsw_afa_cookie *cookie = obj;
+
+ if (cookie->fa_cookie.cookie_len == fa_cookie->cookie_len)
+ return memcmp(cookie->fa_cookie.cookie, fa_cookie->cookie,
+ fa_cookie->cookie_len);
+ return 1;
+}
+
+static const struct rhashtable_params mlxsw_afa_cookie_ht_params = {
+ .head_offset = offsetof(struct mlxsw_afa_cookie, ht_node),
+ .hashfn = mlxsw_afa_cookie_key_hashfn,
+ .obj_hashfn = mlxsw_afa_cookie_obj_hashfn,
+ .obj_cmpfn = mlxsw_afa_cookie_obj_cmpfn,
+ .automatic_shrinking = true,
+};
+
struct mlxsw_afa *mlxsw_afa_create(unsigned int max_acts_per_set,
const struct mlxsw_afa_ops *ops,
void *ops_priv)
@@ -138,11 +192,18 @@ struct mlxsw_afa *mlxsw_afa_create(unsigned int max_acts_per_set,
&mlxsw_afa_fwd_entry_ht_params);
if (err)
goto err_fwd_entry_rhashtable_init;
+ err = rhashtable_init(&mlxsw_afa->cookie_ht,
+ &mlxsw_afa_cookie_ht_params);
+ if (err)
+ goto err_cookie_rhashtable_init;
+ idr_init(&mlxsw_afa->cookie_idr);
mlxsw_afa->max_acts_per_set = max_acts_per_set;
mlxsw_afa->ops = ops;
mlxsw_afa->ops_priv = ops_priv;
return mlxsw_afa;
+err_cookie_rhashtable_init:
+ rhashtable_destroy(&mlxsw_afa->fwd_entry_ht);
err_fwd_entry_rhashtable_init:
rhashtable_destroy(&mlxsw_afa->set_ht);
err_set_rhashtable_init:
@@ -153,6 +214,9 @@ EXPORT_SYMBOL(mlxsw_afa_create);
void mlxsw_afa_destroy(struct mlxsw_afa *mlxsw_afa)
{
+ WARN_ON(!idr_is_empty(&mlxsw_afa->cookie_idr));
+ idr_destroy(&mlxsw_afa->cookie_idr);
+ rhashtable_destroy(&mlxsw_afa->cookie_ht);
rhashtable_destroy(&mlxsw_afa->fwd_entry_ht);
rhashtable_destroy(&mlxsw_afa->set_ht);
kfree(mlxsw_afa);
@@ -627,6 +691,135 @@ mlxsw_afa_counter_create(struct mlxsw_afa_block *block)
return ERR_PTR(err);
}
+/* 20 bits is a maximum that hardware can handle in trap with userdef action
+ * and carry along with the trapped packet.
+ */
+#define MLXSW_AFA_COOKIE_INDEX_BITS 20
+#define MLXSW_AFA_COOKIE_INDEX_MAX ((1 << MLXSW_AFA_COOKIE_INDEX_BITS) - 1)
+
+static struct mlxsw_afa_cookie *
+mlxsw_afa_cookie_create(struct mlxsw_afa *mlxsw_afa,
+ const struct flow_action_cookie *fa_cookie)
+{
+ struct mlxsw_afa_cookie *cookie;
+ u32 cookie_index;
+ int err;
+
+ cookie = kzalloc(sizeof(*cookie) + fa_cookie->cookie_len, GFP_KERNEL);
+ if (!cookie)
+ return ERR_PTR(-ENOMEM);
+ refcount_set(&cookie->ref_count, 1);
+ memcpy(&cookie->fa_cookie, fa_cookie,
+ sizeof(*fa_cookie) + fa_cookie->cookie_len);
+
+ err = rhashtable_insert_fast(&mlxsw_afa->cookie_ht, &cookie->ht_node,
+ mlxsw_afa_cookie_ht_params);
+ if (err)
+ goto err_rhashtable_insert;
+
+ /* Start cookie indexes with 1. Leave the 0 index unused. Packets
+ * that come from the HW which are not dropped by drop-with-cookie
+ * action are going to pass cookie_index 0 to lookup.
+ */
+ cookie_index = 1;
+ err = idr_alloc_u32(&mlxsw_afa->cookie_idr, cookie, &cookie_index,
+ MLXSW_AFA_COOKIE_INDEX_MAX, GFP_KERNEL);
+ if (err)
+ goto err_idr_alloc;
+ cookie->cookie_index = cookie_index;
+ return cookie;
+
+err_idr_alloc:
+ rhashtable_remove_fast(&mlxsw_afa->cookie_ht, &cookie->ht_node,
+ mlxsw_afa_cookie_ht_params);
+err_rhashtable_insert:
+ kfree(cookie);
+ return ERR_PTR(err);
+}
+
+static void mlxsw_afa_cookie_destroy(struct mlxsw_afa *mlxsw_afa,
+ struct mlxsw_afa_cookie *cookie)
+{
+ idr_remove(&mlxsw_afa->cookie_idr, cookie->cookie_index);
+ rhashtable_remove_fast(&mlxsw_afa->cookie_ht, &cookie->ht_node,
+ mlxsw_afa_cookie_ht_params);
+ kfree_rcu(cookie, rcu);
+}
+
+static struct mlxsw_afa_cookie *
+mlxsw_afa_cookie_get(struct mlxsw_afa *mlxsw_afa,
+ const struct flow_action_cookie *fa_cookie)
+{
+ struct mlxsw_afa_cookie *cookie;
+
+ cookie = rhashtable_lookup_fast(&mlxsw_afa->cookie_ht, fa_cookie,
+ mlxsw_afa_cookie_ht_params);
+ if (cookie) {
+ refcount_inc(&cookie->ref_count);
+ return cookie;
+ }
+ return mlxsw_afa_cookie_create(mlxsw_afa, fa_cookie);
+}
+
+static void mlxsw_afa_cookie_put(struct mlxsw_afa *mlxsw_afa,
+ struct mlxsw_afa_cookie *cookie)
+{
+ if (!refcount_dec_and_test(&cookie->ref_count))
+ return;
+ mlxsw_afa_cookie_destroy(mlxsw_afa, cookie);
+}
+
+struct mlxsw_afa_cookie_ref {
+ struct mlxsw_afa_resource resource;
+ struct mlxsw_afa_cookie *cookie;
+};
+
+static void
+mlxsw_afa_cookie_ref_destroy(struct mlxsw_afa_block *block,
+ struct mlxsw_afa_cookie_ref *cookie_ref)
+{
+ mlxsw_afa_resource_del(&cookie_ref->resource);
+ mlxsw_afa_cookie_put(block->afa, cookie_ref->cookie);
+ kfree(cookie_ref);
+}
+
+static void
+mlxsw_afa_cookie_ref_destructor(struct mlxsw_afa_block *block,
+ struct mlxsw_afa_resource *resource)
+{
+ struct mlxsw_afa_cookie_ref *cookie_ref;
+
+ cookie_ref = container_of(resource, struct mlxsw_afa_cookie_ref,
+ resource);
+ mlxsw_afa_cookie_ref_destroy(block, cookie_ref);
+}
+
+static struct mlxsw_afa_cookie_ref *
+mlxsw_afa_cookie_ref_create(struct mlxsw_afa_block *block,
+ const struct flow_action_cookie *fa_cookie)
+{
+ struct mlxsw_afa_cookie_ref *cookie_ref;
+ struct mlxsw_afa_cookie *cookie;
+ int err;
+
+ cookie_ref = kzalloc(sizeof(*cookie_ref), GFP_KERNEL);
+ if (!cookie_ref)
+ return ERR_PTR(-ENOMEM);
+ cookie = mlxsw_afa_cookie_get(block->afa, fa_cookie);
+ if (IS_ERR(cookie)) {
+ err = PTR_ERR(cookie);
+ goto err_cookie_get;
+ }
+ cookie_ref->cookie = cookie;
+ cookie_ref->resource.destructor = mlxsw_afa_cookie_ref_destructor;
+ mlxsw_afa_resource_add(block, &cookie_ref->resource);
+ return cookie_ref;
+
+err_cookie_get:
+ kfree(cookie_ref);
+ return ERR_PTR(err);
+}
+
#define MLXSW_AFA_ONE_ACTION_LEN 32
#define MLXSW_AFA_PAYLOAD_OFFSET 4
@@ -839,7 +1032,8 @@ mlxsw_afa_trap_mirror_pack(char *payload, bool mirror_enable,
mlxsw_afa_trap_mirror_agent_set(payload, mirror_agent);
}
-int mlxsw_afa_block_append_drop(struct mlxsw_afa_block *block, bool ingress)
+static int mlxsw_afa_block_append_drop_plain(struct mlxsw_afa_block *block,
+ bool ingress)
{
char *act = mlxsw_afa_block_append_action(block, MLXSW_AFA_TRAP_CODE,
MLXSW_AFA_TRAP_SIZE);
@@ -852,6 +1046,53 @@ int mlxsw_afa_block_append_drop(struct mlxsw_afa_block *block, bool ingress)
MLXSW_TRAP_ID_DISCARD_EGRESS_ACL);
return 0;
}
+
+static int
+mlxsw_afa_block_append_drop_with_cookie(struct mlxsw_afa_block *block,
+ bool ingress,
+ const struct flow_action_cookie *fa_cookie,
+ struct netlink_ext_ack *extack)
+{
+ struct mlxsw_afa_cookie_ref *cookie_ref;
+ u32 cookie_index;
+ char *act;
+ int err;
+
+ cookie_ref = mlxsw_afa_cookie_ref_create(block, fa_cookie);
+ if (IS_ERR(cookie_ref)) {
+ NL_SET_ERR_MSG_MOD(extack, "Cannot create cookie for drop action");
+ return PTR_ERR(cookie_ref);
+ }
+ cookie_index = cookie_ref->cookie->cookie_index;
+
+ act = mlxsw_afa_block_append_action(block, MLXSW_AFA_TRAPWU_CODE,
+ MLXSW_AFA_TRAPWU_SIZE);
+ if (IS_ERR(act)) {
+ NL_SET_ERR_MSG_MOD(extack, "Cannot append drop with cookie action");
+ err = PTR_ERR(act);
+ goto err_append_action;
+ }
+ mlxsw_afa_trapwu_pack(act, MLXSW_AFA_TRAP_TRAP_ACTION_TRAP,
+ MLXSW_AFA_TRAP_FORWARD_ACTION_DISCARD,
+ ingress ? MLXSW_TRAP_ID_DISCARD_INGRESS_ACL :
+ MLXSW_TRAP_ID_DISCARD_EGRESS_ACL,
+ cookie_index);
+ return 0;
+
+err_append_action:
+ mlxsw_afa_cookie_ref_destroy(block, cookie_ref);
+ return err;
+}
+
+int mlxsw_afa_block_append_drop(struct mlxsw_afa_block *block, bool ingress,
+ const struct flow_action_cookie *fa_cookie,
+ struct netlink_ext_ack *extack)
+{
+ return fa_cookie ?
+ mlxsw_afa_block_append_drop_with_cookie(block, ingress,
+ fa_cookie, extack) :
+ mlxsw_afa_block_append_drop_plain(block, ingress);
+}
EXPORT_SYMBOL(mlxsw_afa_block_append_drop);
int mlxsw_afa_block_append_trap(struct mlxsw_afa_block *block, u16 trap_id)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_acl_flex_actions.h b/drivers/net/ethernet/mellanox/mlxsw/core_acl_flex_actions.h
index 28b2576ea272..67473f8bd12b 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core_acl_flex_actions.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/core_acl_flex_actions.h
@@ -6,6 +6,7 @@
#include <linux/types.h>
#include <linux/netdevice.h>
+#include <net/flow_offload.h>
struct mlxsw_afa;
struct mlxsw_afa_block;
@@ -42,7 +43,9 @@ int mlxsw_afa_block_activity_get(struct mlxsw_afa_block *block, bool *activity);
int mlxsw_afa_block_continue(struct mlxsw_afa_block *block);
int mlxsw_afa_block_jump(struct mlxsw_afa_block *block, u16 group_id);
int mlxsw_afa_block_terminate(struct mlxsw_afa_block *block);
-int mlxsw_afa_block_append_drop(struct mlxsw_afa_block *block, bool ingress);
+int mlxsw_afa_block_append_drop(struct mlxsw_afa_block *block, bool ingress,
+ const struct flow_action_cookie *fa_cookie,
+ struct netlink_ext_ack *extack);
int mlxsw_afa_block_append_trap(struct mlxsw_afa_block *block, u16 trap_id);
int mlxsw_afa_block_append_trap_and_forward(struct mlxsw_afa_block *block,
u16 trap_id);
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
index cb3ff8d021a4..c88f00b293a1 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
@@ -19,6 +19,7 @@
#include <net/pkt_cls.h>
#include <net/red.h>
#include <net/vxlan.h>
+#include <net/flow_offload.h>
#include "port.h"
#include "core.h"
@@ -726,7 +727,9 @@ int mlxsw_sp_acl_rulei_act_jump(struct mlxsw_sp_acl_rule_info *rulei,
u16 group_id);
int mlxsw_sp_acl_rulei_act_terminate(struct mlxsw_sp_acl_rule_info *rulei);
int mlxsw_sp_acl_rulei_act_drop(struct mlxsw_sp_acl_rule_info *rulei,
- bool ingress);
+ bool ingress,
+ const struct flow_action_cookie *fa_cookie,
+ struct netlink_ext_ack *extack);
int mlxsw_sp_acl_rulei_act_trap(struct mlxsw_sp_acl_rule_info *rulei);
int mlxsw_sp_acl_rulei_act_mirror(struct mlxsw_sp *mlxsw_sp,
struct mlxsw_sp_acl_rule_info *rulei,
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c
index abd749adb0f5..36b264798f04 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c
@@ -536,9 +536,12 @@ int mlxsw_sp_acl_rulei_act_terminate(struct mlxsw_sp_acl_rule_info *rulei)
}
int mlxsw_sp_acl_rulei_act_drop(struct mlxsw_sp_acl_rule_info *rulei,
- bool ingress)
+ bool ingress,
+ const struct flow_action_cookie *fa_cookie,
+ struct netlink_ext_ack *extack)
{
- return mlxsw_afa_block_append_drop(rulei->act_block, ingress);
+ return mlxsw_afa_block_append_drop(rulei->act_block, ingress,
+ fa_cookie, extack);
}
int mlxsw_sp_acl_rulei_act_trap(struct mlxsw_sp_acl_rule_info *rulei)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c
index 17368ef8cee0..0011a71114e3 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c
@@ -49,7 +49,8 @@ static int mlxsw_sp_flower_parse_actions(struct mlxsw_sp *mlxsw_sp,
return -EOPNOTSUPP;
}
ingress = mlxsw_sp_acl_block_is_ingress_bound(block);
- err = mlxsw_sp_acl_rulei_act_drop(rulei, ingress);
+ err = mlxsw_sp_acl_rulei_act_drop(rulei, ingress,
+ act->cookie, extack);
if (err) {
NL_SET_ERR_MSG_MOD(extack, "Cannot append drop action");
return err;
--
2.21.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [patch net-next 07/10] mlxsw: pci: Extract cookie index for ACL discard trap packets
2020-02-24 21:07 [patch net-next 00/10] mlxsw: Implement ACL-dropped packets identification Jiri Pirko
` (5 preceding siblings ...)
2020-02-24 21:07 ` [patch net-next 06/10] mlxsw: core_acl_flex_actions: Implement flow_offload action cookie offload Jiri Pirko
@ 2020-02-24 21:07 ` Jiri Pirko
2020-02-24 21:07 ` [patch net-next 08/10] mlxsw: spectrum_trap: Lookup and pass cookie down to devlink_trap_report() Jiri Pirko
` (2 subsequent siblings)
9 siblings, 0 replies; 23+ messages in thread
From: Jiri Pirko @ 2020-02-24 21:07 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, nhorman, jhs, xiyou.wangcong, idosch, mlxsw
From: Jiri Pirko <jiri@mellanox.com>
In case the received packet comes in due to one of ACL discard traps,
take the user_def_val_orig_pkt_len field from CQE and store it
in skb->cb as ACL cookie index.
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/core.h | 5 ++++-
drivers/net/ethernet/mellanox/mlxsw/pci.c | 9 +++++++++
drivers/net/ethernet/mellanox/mlxsw/pci_hw.h | 5 +++++
3 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.h b/drivers/net/ethernet/mellanox/mlxsw/core.h
index 00e44e778aca..46226823c7a6 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.h
@@ -473,7 +473,10 @@ enum mlxsw_devlink_param_id {
};
struct mlxsw_skb_cb {
- struct mlxsw_tx_info tx_info;
+ union {
+ struct mlxsw_tx_info tx_info;
+ u32 cookie_index; /* Only used during receive */
+ };
};
static inline struct mlxsw_skb_cb *mlxsw_skb_cb(struct sk_buff *skb)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/pci.c b/drivers/net/ethernet/mellanox/mlxsw/pci.c
index 914c33e46fb4..67ee0da75af2 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/pci.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/pci.c
@@ -575,6 +575,15 @@ static void mlxsw_pci_cqe_rdq_handle(struct mlxsw_pci *mlxsw_pci,
rx_info.trap_id = mlxsw_pci_cqe_trap_id_get(cqe);
+ if (rx_info.trap_id == MLXSW_TRAP_ID_DISCARD_INGRESS_ACL ||
+ rx_info.trap_id == MLXSW_TRAP_ID_DISCARD_EGRESS_ACL) {
+ u32 cookie_index = 0;
+
+ if (mlxsw_pci->max_cqe_ver >= MLXSW_PCI_CQE_V2)
+ cookie_index = mlxsw_pci_cqe2_user_def_val_orig_pkt_len_get(cqe);
+ mlxsw_skb_cb(skb)->cookie_index = cookie_index;
+ }
+
byte_count = mlxsw_pci_cqe_byte_count_get(cqe);
if (mlxsw_pci_cqe_crc_get(cqe_v, cqe))
byte_count -= ETH_FCS_LEN;
diff --git a/drivers/net/ethernet/mellanox/mlxsw/pci_hw.h b/drivers/net/ethernet/mellanox/mlxsw/pci_hw.h
index c5ceb0bb6485..32c7cabfb261 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/pci_hw.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/pci_hw.h
@@ -208,6 +208,11 @@ MLXSW_ITEM32(pci, cqe0, dqn, 0x0C, 1, 5);
MLXSW_ITEM32(pci, cqe12, dqn, 0x0C, 1, 6);
mlxsw_pci_cqe_item_helpers(dqn, 0, 12, 12);
+/* pci_cqe_user_def_val_orig_pkt_len
+ * When trap_id is an ACL: User defined value from policy engine action.
+ */
+MLXSW_ITEM32(pci, cqe2, user_def_val_orig_pkt_len, 0x14, 0, 20);
+
/* pci_cqe_owner
* Ownership bit.
*/
--
2.21.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [patch net-next 08/10] mlxsw: spectrum_trap: Lookup and pass cookie down to devlink_trap_report()
2020-02-24 21:07 [patch net-next 00/10] mlxsw: Implement ACL-dropped packets identification Jiri Pirko
` (6 preceding siblings ...)
2020-02-24 21:07 ` [patch net-next 07/10] mlxsw: pci: Extract cookie index for ACL discard trap packets Jiri Pirko
@ 2020-02-24 21:07 ` Jiri Pirko
2020-02-24 21:07 ` [patch net-next 09/10] netdevsim: add ACL trap reporting cookie as a metadata Jiri Pirko
2020-02-24 21:07 ` [patch net-next 10/10] selftests: netdevsim: Extend devlink trap test to include flow action cookie Jiri Pirko
9 siblings, 0 replies; 23+ messages in thread
From: Jiri Pirko @ 2020-02-24 21:07 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, nhorman, jhs, xiyou.wangcong, idosch, mlxsw
From: Jiri Pirko <jiri@mellanox.com>
Use the cookie index received along with the packet to lookup original
flow_offload cookie binary and pass it down to devlink_trap_report().
Add "fa_cookie" metadata to the ACL trap.
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
.../mellanox/mlxsw/core_acl_flex_actions.c | 16 +++++++
.../mellanox/mlxsw/core_acl_flex_actions.h | 2 +
.../net/ethernet/mellanox/mlxsw/spectrum.h | 6 +++
.../ethernet/mellanox/mlxsw/spectrum_trap.c | 42 +++++++++++++++++--
4 files changed, 63 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_acl_flex_actions.c b/drivers/net/ethernet/mellanox/mlxsw/core_acl_flex_actions.c
index 9fad56df8303..1f2e6db743e1 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core_acl_flex_actions.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core_acl_flex_actions.c
@@ -769,6 +769,22 @@ static void mlxsw_afa_cookie_put(struct mlxsw_afa *mlxsw_afa,
mlxsw_afa_cookie_destroy(mlxsw_afa, cookie);
}
+/* RCU read lock must be held */
+const struct flow_action_cookie *
+mlxsw_afa_cookie_lookup(struct mlxsw_afa *mlxsw_afa, u32 cookie_index)
+{
+ struct mlxsw_afa_cookie *cookie;
+
+ /* 0 index means no cookie */
+ if (!cookie_index)
+ return NULL;
+ cookie = idr_find(&mlxsw_afa->cookie_idr, cookie_index);
+ if (!cookie)
+ return NULL;
+ return &cookie->fa_cookie;
+}
+EXPORT_SYMBOL(mlxsw_afa_cookie_lookup);
+
struct mlxsw_afa_cookie_ref {
struct mlxsw_afa_resource resource;
struct mlxsw_afa_cookie *cookie;
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_acl_flex_actions.h b/drivers/net/ethernet/mellanox/mlxsw/core_acl_flex_actions.h
index 67473f8bd12b..5f4c1e505136 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core_acl_flex_actions.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/core_acl_flex_actions.h
@@ -43,6 +43,8 @@ int mlxsw_afa_block_activity_get(struct mlxsw_afa_block *block, bool *activity);
int mlxsw_afa_block_continue(struct mlxsw_afa_block *block);
int mlxsw_afa_block_jump(struct mlxsw_afa_block *block, u16 group_id);
int mlxsw_afa_block_terminate(struct mlxsw_afa_block *block);
+const struct flow_action_cookie *
+mlxsw_afa_cookie_lookup(struct mlxsw_afa *mlxsw_afa, u32 cookie_index);
int mlxsw_afa_block_append_drop(struct mlxsw_afa_block *block, bool ingress,
const struct flow_action_cookie *fa_cookie,
struct netlink_ext_ack *extack);
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
index c88f00b293a1..3522f9674577 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
@@ -780,6 +780,12 @@ int mlxsw_sp_acl_rule_get_stats(struct mlxsw_sp *mlxsw_sp,
struct mlxsw_sp_fid *mlxsw_sp_acl_dummy_fid(struct mlxsw_sp *mlxsw_sp);
+static inline const struct flow_action_cookie *
+mlxsw_sp_acl_act_cookie_lookup(struct mlxsw_sp *mlxsw_sp, u32 cookie_index)
+{
+ return mlxsw_afa_cookie_lookup(mlxsw_sp->afa, cookie_index);
+}
+
int mlxsw_sp_acl_init(struct mlxsw_sp *mlxsw_sp);
void mlxsw_sp_acl_fini(struct mlxsw_sp *mlxsw_sp);
u32 mlxsw_sp_acl_region_rehash_intrvl_get(struct mlxsw_sp *mlxsw_sp);
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.c
index a55577a50e90..9c300d625e04 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_trap.c
@@ -75,6 +75,35 @@ static void mlxsw_sp_rx_drop_listener(struct sk_buff *skb, u8 local_port,
consume_skb(skb);
}
+static void mlxsw_sp_rx_acl_drop_listener(struct sk_buff *skb, u8 local_port,
+ void *trap_ctx)
+{
+ u32 cookie_index = mlxsw_skb_cb(skb)->cookie_index;
+ const struct flow_action_cookie *fa_cookie;
+ struct devlink_port *in_devlink_port;
+ struct mlxsw_sp_port *mlxsw_sp_port;
+ struct mlxsw_sp *mlxsw_sp;
+ struct devlink *devlink;
+ int err;
+
+ mlxsw_sp = devlink_trap_ctx_priv(trap_ctx);
+ mlxsw_sp_port = mlxsw_sp->ports[local_port];
+
+ err = mlxsw_sp_rx_listener(mlxsw_sp, skb, local_port, mlxsw_sp_port);
+ if (err)
+ return;
+
+ devlink = priv_to_devlink(mlxsw_sp->core);
+ in_devlink_port = mlxsw_core_port_devlink_port_get(mlxsw_sp->core,
+ local_port);
+ skb_push(skb, ETH_HLEN);
+ rcu_read_lock();
+ fa_cookie = mlxsw_sp_acl_act_cookie_lookup(mlxsw_sp, cookie_index);
+ devlink_trap_report(devlink, skb, trap_ctx, in_devlink_port, fa_cookie);
+ rcu_read_unlock();
+ consume_skb(skb);
+}
+
static void mlxsw_sp_rx_exception_listener(struct sk_buff *skb, u8 local_port,
void *trap_ctx)
{
@@ -106,6 +135,11 @@ static void mlxsw_sp_rx_exception_listener(struct sk_buff *skb, u8 local_port,
DEVLINK_TRAP_GROUP_GENERIC(_group_id), \
MLXSW_SP_TRAP_METADATA)
+#define MLXSW_SP_TRAP_DROP_EXT(_id, _group_id, _metadata) \
+ DEVLINK_TRAP_GENERIC(DROP, DROP, _id, \
+ DEVLINK_TRAP_GROUP_GENERIC(_group_id), \
+ MLXSW_SP_TRAP_METADATA | (_metadata))
+
#define MLXSW_SP_TRAP_DRIVER_DROP(_id, _group_id) \
DEVLINK_TRAP_DRIVER(DROP, DROP, DEVLINK_MLXSW_TRAP_ID_##_id, \
DEVLINK_MLXSW_TRAP_NAME_##_id, \
@@ -123,7 +157,7 @@ static void mlxsw_sp_rx_exception_listener(struct sk_buff *skb, u8 local_port,
SET_FW_DEFAULT, SP_##_group_id)
#define MLXSW_SP_RXL_ACL_DISCARD(_id, _en_group_id, _dis_group_id) \
- MLXSW_RXL_DIS(mlxsw_sp_rx_drop_listener, DISCARD_##_id, \
+ MLXSW_RXL_DIS(mlxsw_sp_rx_acl_drop_listener, DISCARD_##_id, \
TRAP_EXCEPTION_TO_CPU, false, SP_##_en_group_id, \
SET_FW_DEFAULT, SP_##_dis_group_id)
@@ -160,8 +194,10 @@ static const struct devlink_trap mlxsw_sp_traps_arr[] = {
MLXSW_SP_TRAP_DROP(NON_ROUTABLE, L3_DROPS),
MLXSW_SP_TRAP_EXCEPTION(DECAP_ERROR, TUNNEL_DROPS),
MLXSW_SP_TRAP_DROP(OVERLAY_SMAC_MC, TUNNEL_DROPS),
- MLXSW_SP_TRAP_DROP(INGRESS_FLOW_ACTION_DROP, ACL_DROPS),
- MLXSW_SP_TRAP_DROP(EGRESS_FLOW_ACTION_DROP, ACL_DROPS),
+ MLXSW_SP_TRAP_DROP_EXT(INGRESS_FLOW_ACTION_DROP, ACL_DROPS,
+ DEVLINK_TRAP_METADATA_TYPE_F_FA_COOKIE),
+ MLXSW_SP_TRAP_DROP_EXT(EGRESS_FLOW_ACTION_DROP, ACL_DROPS,
+ DEVLINK_TRAP_METADATA_TYPE_F_FA_COOKIE),
};
static const struct mlxsw_listener mlxsw_sp_listeners_arr[] = {
--
2.21.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [patch net-next 09/10] netdevsim: add ACL trap reporting cookie as a metadata
2020-02-24 21:07 [patch net-next 00/10] mlxsw: Implement ACL-dropped packets identification Jiri Pirko
` (7 preceding siblings ...)
2020-02-24 21:07 ` [patch net-next 08/10] mlxsw: spectrum_trap: Lookup and pass cookie down to devlink_trap_report() Jiri Pirko
@ 2020-02-24 21:07 ` Jiri Pirko
2020-02-25 4:42 ` Jakub Kicinski
2020-02-24 21:07 ` [patch net-next 10/10] selftests: netdevsim: Extend devlink trap test to include flow action cookie Jiri Pirko
9 siblings, 1 reply; 23+ messages in thread
From: Jiri Pirko @ 2020-02-24 21:07 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, nhorman, jhs, xiyou.wangcong, idosch, mlxsw
From: Jiri Pirko <jiri@mellanox.com>
Add new trap ACL which reports flow action cookie in a metadata. Allow
used to setup the cookie using debugfs file.
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
drivers/net/netdevsim/dev.c | 118 +++++++++++++++++++++++++++++-
drivers/net/netdevsim/netdevsim.h | 2 +
2 files changed, 117 insertions(+), 3 deletions(-)
diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index aa17533c06e1..71b60e06b141 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -28,6 +28,7 @@
#include <linux/workqueue.h>
#include <net/devlink.h>
#include <net/ip.h>
+#include <net/flow_offload.h>
#include <uapi/linux/devlink.h>
#include <uapi/linux/ip.h>
#include <uapi/linux/udp.h>
@@ -71,6 +72,99 @@ static const struct file_operations nsim_dev_take_snapshot_fops = {
.llseek = generic_file_llseek,
};
+static ssize_t nsim_dev_trap_fa_cookie_read(struct file *file,
+ char __user *data,
+ size_t count, loff_t *ppos)
+{
+ struct nsim_dev *nsim_dev = file->private_data;
+ struct flow_action_cookie *fa_cookie;
+ unsigned int buf_len;
+ ssize_t ret;
+ char *buf;
+
+ spin_lock(&nsim_dev->fa_cookie_lock);
+ fa_cookie = nsim_dev->fa_cookie;
+ if (!fa_cookie) {
+ ret = -EINVAL;
+ goto errout;
+ }
+ buf_len = fa_cookie->cookie_len * 2;
+ buf = kmalloc(buf_len, GFP_ATOMIC);
+ if (!buf) {
+ ret = -ENOMEM;
+ goto errout;
+ }
+ bin2hex(buf, fa_cookie->cookie, fa_cookie->cookie_len);
+ spin_unlock(&nsim_dev->fa_cookie_lock);
+
+ ret = simple_read_from_buffer(data, count, ppos, buf, buf_len);
+
+ kfree(buf);
+ return ret;
+
+errout:
+ spin_unlock(&nsim_dev->fa_cookie_lock);
+ return ret;
+}
+
+static ssize_t nsim_dev_trap_fa_cookie_write(struct file *file,
+ const char __user *data,
+ size_t count, loff_t *ppos)
+{
+ struct nsim_dev *nsim_dev = file->private_data;
+ struct flow_action_cookie *fa_cookie;
+ size_t cookie_len = count / 2;
+ ssize_t ret;
+ char *buf;
+
+ if (*ppos != 0)
+ return 0;
+ cookie_len = (count - 1) / 2;
+ if ((count - 1) % 2)
+ return -EINVAL;
+ buf = kmalloc(count, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ ret = simple_write_to_buffer(buf, count, ppos, data, count);
+ if (ret < 0)
+ goto err_write_to_buffer;
+
+ fa_cookie = kmalloc(sizeof(*fa_cookie) + cookie_len,
+ GFP_KERNEL | __GFP_NOWARN);
+ if (!fa_cookie) {
+ ret = -ENOMEM;
+ goto err_alloc_cookie;
+ }
+
+ fa_cookie->cookie_len = cookie_len;
+ ret = hex2bin((u8 *) fa_cookie->cookie, buf, cookie_len);
+ if (ret)
+ goto err_hex2bin;
+ kfree(buf);
+
+ spin_lock(&nsim_dev->fa_cookie_lock);
+ kfree(nsim_dev->fa_cookie);
+ nsim_dev->fa_cookie = fa_cookie;
+ spin_unlock(&nsim_dev->fa_cookie_lock);
+
+ return count;
+
+err_hex2bin:
+ kfree(fa_cookie);
+err_alloc_cookie:
+err_write_to_buffer:
+ kfree(buf);
+ return ret;
+}
+
+static const struct file_operations nsim_dev_trap_fa_cookie_fops = {
+ .open = simple_open,
+ .read = nsim_dev_trap_fa_cookie_read,
+ .write = nsim_dev_trap_fa_cookie_write,
+ .llseek = generic_file_llseek,
+};
+
static int nsim_dev_debugfs_init(struct nsim_dev *nsim_dev)
{
char dev_ddir_name[sizeof(DRV_NAME) + 10];
@@ -97,6 +191,8 @@ static int nsim_dev_debugfs_init(struct nsim_dev *nsim_dev)
&nsim_dev->dont_allow_reload);
debugfs_create_bool("fail_reload", 0600, nsim_dev->ddir,
&nsim_dev->fail_reload);
+ debugfs_create_file("trap_flow_action_cookie", 0600, nsim_dev->ddir,
+ nsim_dev, &nsim_dev_trap_fa_cookie_fops);
return 0;
}
@@ -288,6 +384,10 @@ enum {
DEVLINK_TRAP_GENERIC(DROP, DROP, _id, \
DEVLINK_TRAP_GROUP_GENERIC(_group_id), \
NSIM_TRAP_METADATA)
+#define NSIM_TRAP_DROP_EXT(_id, _group_id, _metadata) \
+ DEVLINK_TRAP_GENERIC(DROP, DROP, _id, \
+ DEVLINK_TRAP_GROUP_GENERIC(_group_id), \
+ NSIM_TRAP_METADATA | (_metadata))
#define NSIM_TRAP_EXCEPTION(_id, _group_id) \
DEVLINK_TRAP_GENERIC(EXCEPTION, TRAP, _id, \
DEVLINK_TRAP_GROUP_GENERIC(_group_id), \
@@ -309,6 +409,10 @@ static const struct devlink_trap nsim_traps_arr[] = {
NSIM_TRAP_DROP(BLACKHOLE_ROUTE, L3_DROPS),
NSIM_TRAP_EXCEPTION(TTL_ERROR, L3_DROPS),
NSIM_TRAP_DROP(TAIL_DROP, BUFFER_DROPS),
+ NSIM_TRAP_DROP_EXT(INGRESS_FLOW_ACTION_DROP, ACL_DROPS,
+ DEVLINK_TRAP_METADATA_TYPE_F_FA_COOKIE),
+ NSIM_TRAP_DROP_EXT(EGRESS_FLOW_ACTION_DROP, ACL_DROPS,
+ DEVLINK_TRAP_METADATA_TYPE_F_FA_COOKIE),
};
#define NSIM_TRAP_L4_DATA_LEN 100
@@ -366,8 +470,13 @@ static void nsim_dev_trap_report(struct nsim_dev_port *nsim_dev_port)
spin_lock(&nsim_trap_data->trap_lock);
for (i = 0; i < ARRAY_SIZE(nsim_traps_arr); i++) {
+ struct flow_action_cookie *fa_cookie = NULL;
struct nsim_trap_item *nsim_trap_item;
struct sk_buff *skb;
+ bool has_fa_cookie;
+
+ has_fa_cookie = nsim_traps_arr[i].metadata_cap &
+ DEVLINK_TRAP_METADATA_TYPE_F_FA_COOKIE;
nsim_trap_item = &nsim_trap_data->trap_items_arr[i];
if (nsim_trap_item->action == DEVLINK_TRAP_ACTION_DROP)
@@ -383,10 +492,12 @@ static void nsim_dev_trap_report(struct nsim_dev_port *nsim_dev_port)
* softIRQs to prevent lockdep from complaining about
* "incosistent lock state".
*/
- local_bh_disable();
+
+ spin_lock_bh(&nsim_dev->fa_cookie_lock);
+ fa_cookie = has_fa_cookie ? nsim_dev->fa_cookie : NULL;
devlink_trap_report(devlink, skb, nsim_trap_item->trap_ctx,
- &nsim_dev_port->devlink_port, NULL);
- local_bh_enable();
+ &nsim_dev_port->devlink_port, fa_cookie);
+ spin_unlock_bh(&nsim_dev->fa_cookie_lock);
consume_skb(skb);
}
spin_unlock(&nsim_trap_data->trap_lock);
@@ -780,6 +891,7 @@ int nsim_dev_probe(struct nsim_bus_dev *nsim_bus_dev)
nsim_dev->fw_update_status = true;
nsim_dev->max_macs = NSIM_DEV_MAX_MACS_DEFAULT;
nsim_dev->test1 = NSIM_DEV_TEST1_DEFAULT;
+ spin_lock_init(&nsim_dev->fa_cookie_lock);
dev_set_drvdata(&nsim_bus_dev->dev, nsim_dev);
diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
index 2eb7b0dc1594..e46fc565b981 100644
--- a/drivers/net/netdevsim/netdevsim.h
+++ b/drivers/net/netdevsim/netdevsim.h
@@ -178,6 +178,8 @@ struct nsim_dev {
bool fail_reload;
struct devlink_region *dummy_region;
struct nsim_dev_health health;
+ struct flow_action_cookie *fa_cookie;
+ spinlock_t fa_cookie_lock; /* protects fa_cookie */
};
static inline struct net *nsim_dev_net(struct nsim_dev *nsim_dev)
--
2.21.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [patch net-next 09/10] netdevsim: add ACL trap reporting cookie as a metadata
2020-02-24 21:07 ` [patch net-next 09/10] netdevsim: add ACL trap reporting cookie as a metadata Jiri Pirko
@ 2020-02-25 4:42 ` Jakub Kicinski
2020-02-25 7:40 ` Jiri Pirko
0 siblings, 1 reply; 23+ messages in thread
From: Jakub Kicinski @ 2020-02-25 4:42 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, davem, nhorman, jhs, xiyou.wangcong, idosch, mlxsw
On Mon, 24 Feb 2020 22:07:57 +0100, Jiri Pirko wrote:
> +static ssize_t nsim_dev_trap_fa_cookie_write(struct file *file,
> + const char __user *data,
> + size_t count, loff_t *ppos)
> +{
> + struct nsim_dev *nsim_dev = file->private_data;
> + struct flow_action_cookie *fa_cookie;
> + size_t cookie_len = count / 2;
> + ssize_t ret;
> + char *buf;
> +
> + if (*ppos != 0)
> + return 0;
return 0? Should this not be an error?
> + cookie_len = (count - 1) / 2;
why was cookie_len initialized when it was declared?
> + if ((count - 1) % 2)
> + return -EINVAL;
> + buf = kmalloc(count, GFP_KERNEL);
Strangely the malloc below has a NOWARN, but this one doesn't?
> + if (!buf)
> + return -ENOMEM;
> +
> + ret = simple_write_to_buffer(buf, count, ppos, data, count);
> + if (ret < 0)
> + goto err_write_to_buffer;
> +
> + fa_cookie = kmalloc(sizeof(*fa_cookie) + cookie_len,
> + GFP_KERNEL | __GFP_NOWARN);
> + if (!fa_cookie) {
> + ret = -ENOMEM;
> + goto err_alloc_cookie;
> + }
> +
> + fa_cookie->cookie_len = cookie_len;
> + ret = hex2bin((u8 *) fa_cookie->cookie, buf, cookie_len);
this u8 cast won't be necessary if type of cookie changes :)
Also I feel like we could just hold onto the ASCII hex buf,
and simplify the reading side. If the hex part is needed in
the first place, hexdump and xxd exist..
> + if (ret)
> + goto err_hex2bin;
> + kfree(buf);
> +
> + spin_lock(&nsim_dev->fa_cookie_lock);
> + kfree(nsim_dev->fa_cookie);
> + nsim_dev->fa_cookie = fa_cookie;
> + spin_unlock(&nsim_dev->fa_cookie_lock);
> +
> + return count;
> +
> +err_hex2bin:
> + kfree(fa_cookie);
> +err_alloc_cookie:
> +err_write_to_buffer:
Error labels should be named after what they undo, not after
destination. That makes both the source and target of the jump
easy to review.
> + kfree(buf);
> + return ret;
> +}
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [patch net-next 09/10] netdevsim: add ACL trap reporting cookie as a metadata
2020-02-25 4:42 ` Jakub Kicinski
@ 2020-02-25 7:40 ` Jiri Pirko
2020-02-25 17:53 ` Jakub Kicinski
0 siblings, 1 reply; 23+ messages in thread
From: Jiri Pirko @ 2020-02-25 7:40 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev, davem, nhorman, jhs, xiyou.wangcong, idosch, mlxsw
Tue, Feb 25, 2020 at 05:42:57AM CET, kuba@kernel.org wrote:
>On Mon, 24 Feb 2020 22:07:57 +0100, Jiri Pirko wrote:
>> +static ssize_t nsim_dev_trap_fa_cookie_write(struct file *file,
>> + const char __user *data,
>> + size_t count, loff_t *ppos)
>> +{
>> + struct nsim_dev *nsim_dev = file->private_data;
>> + struct flow_action_cookie *fa_cookie;
>> + size_t cookie_len = count / 2;
>> + ssize_t ret;
>> + char *buf;
>> +
>> + if (*ppos != 0)
>> + return 0;
>
>return 0? Should this not be an error?
Correct. Changed to return -EINVAL;
>
>> + cookie_len = (count - 1) / 2;
>
>why was cookie_len initialized when it was declared?
Forgotten init. Fixed.
>
>> + if ((count - 1) % 2)
>> + return -EINVAL;
>> + buf = kmalloc(count, GFP_KERNEL);
>
>Strangely the malloc below has a NOWARN, but this one doesn't?
Added nowarn flag here too.
>
>> + if (!buf)
>> + return -ENOMEM;
>> +
>> + ret = simple_write_to_buffer(buf, count, ppos, data, count);
>> + if (ret < 0)
>> + goto err_write_to_buffer;
>> +
>> + fa_cookie = kmalloc(sizeof(*fa_cookie) + cookie_len,
>> + GFP_KERNEL | __GFP_NOWARN);
>> + if (!fa_cookie) {
>> + ret = -ENOMEM;
>> + goto err_alloc_cookie;
>> + }
>> +
>> + fa_cookie->cookie_len = cookie_len;
>> + ret = hex2bin((u8 *) fa_cookie->cookie, buf, cookie_len);
>
>this u8 cast won't be necessary if type of cookie changes :)
Removed.
>
>Also I feel like we could just hold onto the ASCII hex buf,
>and simplify the reading side. If the hex part is needed in
>the first place, hexdump and xxd exist..
I don't understand. Do you suggest to keep the write hex "buf" as well
and just print it out in "read()" function? I don't like to store one
info in 2 places. We need to have the cookie in fa_cookie anyway. Easy
to bin2hex from it and send to userspace.
>
>> + if (ret)
>> + goto err_hex2bin;
>> + kfree(buf);
>> +
>> + spin_lock(&nsim_dev->fa_cookie_lock);
>> + kfree(nsim_dev->fa_cookie);
>> + nsim_dev->fa_cookie = fa_cookie;
>> + spin_unlock(&nsim_dev->fa_cookie_lock);
>> +
>> + return count;
>> +
>> +err_hex2bin:
>> + kfree(fa_cookie);
>> +err_alloc_cookie:
>> +err_write_to_buffer:
>
>Error labels should be named after what they undo, not after
>destination. That makes both the source and target of the jump
>easy to review.
Well, it's a matter of a code you look at. I actually like it better
with err_*. Anyway, netdevsim uses the convention you want, changed.
Thanks for review!
>
>> + kfree(buf);
>> + return ret;
>> +}
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [patch net-next 09/10] netdevsim: add ACL trap reporting cookie as a metadata
2020-02-25 7:40 ` Jiri Pirko
@ 2020-02-25 17:53 ` Jakub Kicinski
0 siblings, 0 replies; 23+ messages in thread
From: Jakub Kicinski @ 2020-02-25 17:53 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, davem, nhorman, jhs, xiyou.wangcong, idosch, mlxsw
On Tue, 25 Feb 2020 08:40:19 +0100 Jiri Pirko wrote:
> >Also I feel like we could just hold onto the ASCII hex buf,
> >and simplify the reading side. If the hex part is needed in
> >the first place, hexdump and xxd exist..
>
> I don't understand. Do you suggest to keep the write hex "buf" as well
> and just print it out in "read()" function? I don't like to store one
> info in 2 places. We need to have the cookie in fa_cookie anyway. Easy
> to bin2hex from it and send to userspace.
Okay, no strong feelings. I did not like the GFP_ATOMIC allocation on
read, just to convert formats. But it's not a big deal.
Thanks for the adjustments!
^ permalink raw reply [flat|nested] 23+ messages in thread
* [patch net-next 10/10] selftests: netdevsim: Extend devlink trap test to include flow action cookie
2020-02-24 21:07 [patch net-next 00/10] mlxsw: Implement ACL-dropped packets identification Jiri Pirko
` (8 preceding siblings ...)
2020-02-24 21:07 ` [patch net-next 09/10] netdevsim: add ACL trap reporting cookie as a metadata Jiri Pirko
@ 2020-02-24 21:07 ` Jiri Pirko
2020-02-25 4:43 ` Jakub Kicinski
9 siblings, 1 reply; 23+ messages in thread
From: Jiri Pirko @ 2020-02-24 21:07 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, nhorman, jhs, xiyou.wangcong, idosch, mlxsw
From: Jiri Pirko <jiri@mellanox.com>
Extend existing devlink trap test to include metadata type for flow
action cookie.
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
.../testing/selftests/drivers/net/netdevsim/devlink_trap.sh | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/tools/testing/selftests/drivers/net/netdevsim/devlink_trap.sh b/tools/testing/selftests/drivers/net/netdevsim/devlink_trap.sh
index f101ab9441e2..437d32bd4cfd 100755
--- a/tools/testing/selftests/drivers/net/netdevsim/devlink_trap.sh
+++ b/tools/testing/selftests/drivers/net/netdevsim/devlink_trap.sh
@@ -103,6 +103,11 @@ trap_metadata_test()
for trap_name in $(devlink_traps_get); do
devlink_trap_metadata_test $trap_name "input_port"
check_err $? "Input port not reported as metadata of trap $trap_name"
+ if [ $trap_name == "ingress_flow_action_drop" ] ||
+ [ $trap_name == "egress_flow_action_drop" ]; then
+ devlink_trap_metadata_test $trap_name "flow_action_cookie"
+ check_err $? "Flow action cookie not reported as metadata of trap $trap_name"
+ fi
done
log_test "Trap metadata"
--
2.21.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [patch net-next 10/10] selftests: netdevsim: Extend devlink trap test to include flow action cookie
2020-02-24 21:07 ` [patch net-next 10/10] selftests: netdevsim: Extend devlink trap test to include flow action cookie Jiri Pirko
@ 2020-02-25 4:43 ` Jakub Kicinski
2020-02-25 7:46 ` Jiri Pirko
0 siblings, 1 reply; 23+ messages in thread
From: Jakub Kicinski @ 2020-02-25 4:43 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, davem, nhorman, jhs, xiyou.wangcong, idosch, mlxsw
On Mon, 24 Feb 2020 22:07:58 +0100, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
>
> Extend existing devlink trap test to include metadata type for flow
> action cookie.
>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> ---
> .../testing/selftests/drivers/net/netdevsim/devlink_trap.sh | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/tools/testing/selftests/drivers/net/netdevsim/devlink_trap.sh b/tools/testing/selftests/drivers/net/netdevsim/devlink_trap.sh
> index f101ab9441e2..437d32bd4cfd 100755
> --- a/tools/testing/selftests/drivers/net/netdevsim/devlink_trap.sh
> +++ b/tools/testing/selftests/drivers/net/netdevsim/devlink_trap.sh
> @@ -103,6 +103,11 @@ trap_metadata_test()
> for trap_name in $(devlink_traps_get); do
> devlink_trap_metadata_test $trap_name "input_port"
> check_err $? "Input port not reported as metadata of trap $trap_name"
> + if [ $trap_name == "ingress_flow_action_drop" ] ||
> + [ $trap_name == "egress_flow_action_drop" ]; then
> + devlink_trap_metadata_test $trap_name "flow_action_cookie"
> + check_err $? "Flow action cookie not reported as metadata of trap $trap_name"
> + fi
> done
>
> log_test "Trap metadata"
Oh, this doesn't seem to check the contents of the trap at all, does it?
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [patch net-next 10/10] selftests: netdevsim: Extend devlink trap test to include flow action cookie
2020-02-25 4:43 ` Jakub Kicinski
@ 2020-02-25 7:46 ` Jiri Pirko
2020-02-25 17:57 ` Jakub Kicinski
0 siblings, 1 reply; 23+ messages in thread
From: Jiri Pirko @ 2020-02-25 7:46 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev, davem, nhorman, jhs, xiyou.wangcong, idosch, mlxsw
Tue, Feb 25, 2020 at 05:43:32AM CET, kuba@kernel.org wrote:
>On Mon, 24 Feb 2020 22:07:58 +0100, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@mellanox.com>
>>
>> Extend existing devlink trap test to include metadata type for flow
>> action cookie.
>>
>> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
>> ---
>> .../testing/selftests/drivers/net/netdevsim/devlink_trap.sh | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/tools/testing/selftests/drivers/net/netdevsim/devlink_trap.sh b/tools/testing/selftests/drivers/net/netdevsim/devlink_trap.sh
>> index f101ab9441e2..437d32bd4cfd 100755
>> --- a/tools/testing/selftests/drivers/net/netdevsim/devlink_trap.sh
>> +++ b/tools/testing/selftests/drivers/net/netdevsim/devlink_trap.sh
>> @@ -103,6 +103,11 @@ trap_metadata_test()
>> for trap_name in $(devlink_traps_get); do
>> devlink_trap_metadata_test $trap_name "input_port"
>> check_err $? "Input port not reported as metadata of trap $trap_name"
>> + if [ $trap_name == "ingress_flow_action_drop" ] ||
>> + [ $trap_name == "egress_flow_action_drop" ]; then
>> + devlink_trap_metadata_test $trap_name "flow_action_cookie"
>> + check_err $? "Flow action cookie not reported as metadata of trap $trap_name"
>> + fi
>> done
>>
>> log_test "Trap metadata"
>
>Oh, this doesn't seem to check the contents of the trap at all, does it?
No. This is not the test for the actual trapped packets. It is a test to
list devlink traps and supported metadata.
The packet trapping is done using dropmonitor which is currently
not implemented in selftests, even for the existing traps. Not even for
mlxsw. There is a plan to introduce these tests in the future, Ido is
working on a tool to catch those packets to pcap using dropmon. I think
he plans to send it to dropmon git soon. Then we can implement the
selftests using it.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [patch net-next 10/10] selftests: netdevsim: Extend devlink trap test to include flow action cookie
2020-02-25 7:46 ` Jiri Pirko
@ 2020-02-25 17:57 ` Jakub Kicinski
2020-02-26 7:34 ` Jiri Pirko
0 siblings, 1 reply; 23+ messages in thread
From: Jakub Kicinski @ 2020-02-25 17:57 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, davem, nhorman, jhs, xiyou.wangcong, idosch, mlxsw
On Tue, 25 Feb 2020 08:46:03 +0100 Jiri Pirko wrote:
> >> devlink_trap_metadata_test $trap_name "input_port"
> >> check_err $? "Input port not reported as metadata of trap $trap_name"
> >> + if [ $trap_name == "ingress_flow_action_drop" ] ||
> >> + [ $trap_name == "egress_flow_action_drop" ]; then
> >> + devlink_trap_metadata_test $trap_name "flow_action_cookie"
> >> + check_err $? "Flow action cookie not reported as metadata of trap $trap_name"
> >> + fi
> >> done
> >>
> >> log_test "Trap metadata"
> >
> >Oh, this doesn't seem to check the contents of the trap at all, does it?
>
> No. This is not the test for the actual trapped packets. It is a test to
> list devlink traps and supported metadata.
>
> The packet trapping is done using dropmonitor which is currently
> not implemented in selftests, even for the existing traps. Not even for
> mlxsw. There is a plan to introduce these tests in the future, Ido is
> working on a tool to catch those packets to pcap using dropmon. I think
> he plans to send it to dropmon git soon. Then we can implement the
> selftests using it.
The extra 100 lines of code in netdevsim which is not used by selftests
does make me a little sad.. but okay, looking forward to fuller tests.
Those tests better make use of the variable cookie size, 'cause
otherwise we could have just stored the cookie on a u64 and avoided the
custom read/write functions all together ;)
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [patch net-next 10/10] selftests: netdevsim: Extend devlink trap test to include flow action cookie
2020-02-25 17:57 ` Jakub Kicinski
@ 2020-02-26 7:34 ` Jiri Pirko
0 siblings, 0 replies; 23+ messages in thread
From: Jiri Pirko @ 2020-02-26 7:34 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev, davem, nhorman, jhs, xiyou.wangcong, idosch, mlxsw
Tue, Feb 25, 2020 at 06:57:21PM CET, kuba@kernel.org wrote:
>On Tue, 25 Feb 2020 08:46:03 +0100 Jiri Pirko wrote:
>> >> devlink_trap_metadata_test $trap_name "input_port"
>> >> check_err $? "Input port not reported as metadata of trap $trap_name"
>> >> + if [ $trap_name == "ingress_flow_action_drop" ] ||
>> >> + [ $trap_name == "egress_flow_action_drop" ]; then
>> >> + devlink_trap_metadata_test $trap_name "flow_action_cookie"
>> >> + check_err $? "Flow action cookie not reported as metadata of trap $trap_name"
>> >> + fi
>> >> done
>> >>
>> >> log_test "Trap metadata"
>> >
>> >Oh, this doesn't seem to check the contents of the trap at all, does it?
>>
>> No. This is not the test for the actual trapped packets. It is a test to
>> list devlink traps and supported metadata.
>>
>> The packet trapping is done using dropmonitor which is currently
>> not implemented in selftests, even for the existing traps. Not even for
>> mlxsw. There is a plan to introduce these tests in the future, Ido is
>> working on a tool to catch those packets to pcap using dropmon. I think
>> he plans to send it to dropmon git soon. Then we can implement the
>> selftests using it.
>
>The extra 100 lines of code in netdevsim which is not used by selftests
>does make me a little sad.. but okay, looking forward to fuller tests.
>Those tests better make use of the variable cookie size, 'cause
>otherwise we could have just stored the cookie on a u64 and avoided the
>custom read/write functions all together ;)
Will do.
^ permalink raw reply [flat|nested] 23+ messages in thread