Prestera Switchdev driver implements a set of devlink-based features,
that include both debug functionality (traps with trap statistics), as well
as functional rate limiter that is based on the devlink kernel API (interfaces).
The core prestera-devlink functionality is implemented in the prestera_devlink.c.
The patch series also extends the existing devlink kernel API with a list of core
features:
- devlink: add API for both publish/unpublish port parameters.
- devlink: add port parameters-specific ops, as current design makes it impossible
to register one parameter for multiple ports, and effectively distinguish for
what port parameter_op is called.
- devlink: add trap_drop_counter_get callback for driver to register - make it possible
to keep track of how many packets have been dropped (hard) by the switch device, before
the packets even made it to the devlink subsystem (e.g. dropped due to RXDMA buffer
overflow).
The core features that extend current functionality of prestera Switchdev driver:
- add storm control (BUM control) functionality, that is driven through the
devlink (per) port parameters.
- add logic for driver traps and drops registration (also traps with DROP action).
- add documentation for prestera driver traps and drops group.
Oleksandr Mazur (10):
net: core: devlink: add dropped stats traps field
net: core: devlink: add port_params_ops for devlink port parameters
altering
testing: selftests: net: forwarding: add devlink-required
functionality to test (hard) dropped stats field
drivers: net: netdevsim: add devlink trap_drop_counter_get
implementation
testing: selftests: drivers: net: netdevsim: devlink: add test case
for hard drop statistics
drivers: net: netdevsim: add devlink port params usage
net: marvell: prestera: devlink: add traps/groups implementation
net: marvell: prestera: devlink: add traps with DROP action
net: marvell: prestera: add storm control (rate limiter)
implementation
documentation: networking: devlink: add prestera switched driver
Documentation
Sudarsana Reddy Kalluru (1):
net: core: devlink: add apis to publish/unpublish port params
Documentation/networking/devlink/prestera.rst | 167 +++++
.../net/ethernet/marvell/prestera/prestera.h | 9 +
.../marvell/prestera/prestera_devlink.c | 664 +++++++++++++++++-
.../marvell/prestera/prestera_devlink.h | 3 +
.../ethernet/marvell/prestera/prestera_dsa.c | 3 +
.../ethernet/marvell/prestera/prestera_dsa.h | 1 +
.../ethernet/marvell/prestera/prestera_hw.c | 60 ++
.../ethernet/marvell/prestera/prestera_hw.h | 20 +
.../ethernet/marvell/prestera/prestera_rxtx.c | 7 +-
drivers/net/netdevsim/dev.c | 108 ++-
drivers/net/netdevsim/netdevsim.h | 3 +
include/net/devlink.h | 35 +
net/core/devlink.c | 139 +++-
.../drivers/net/netdevsim/devlink_trap.sh | 10 +
.../selftests/net/forwarding/devlink_lib.sh | 26 +
15 files changed, 1238 insertions(+), 17 deletions(-)
create mode 100644 Documentation/networking/devlink/prestera.rst
--
2.17.1
Whenever query statistics is issued for trap, devlink subsystem
would also fill-in statistics 'dropped' field. This field indicates
the number of packets HW dropped and failed to report to the device driver,
and thus - to the devlink subsystem itself.
In case if device driver didn't register callback for hard drop
statistics querying, 'dropped' field will be omitted and not filled.
Signed-off-by: Oleksandr Mazur <[email protected]>
---
include/net/devlink.h | 10 ++++++++
net/core/devlink.c | 53 +++++++++++++++++++++++++++++++++++++++----
2 files changed, 59 insertions(+), 4 deletions(-)
diff --git a/include/net/devlink.h b/include/net/devlink.h
index 8def0f7365da..b8c6bac067a6 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -1326,6 +1326,16 @@ struct devlink_ops {
const struct devlink_trap_group *group,
enum devlink_trap_action action,
struct netlink_ext_ack *extack);
+ /**
+ * @trap_drop_counter_get: Trap drop counter get function.
+ *
+ * Should be used by device drivers to report number of packets
+ * that have been dropped, and cannot be passed to the devlink
+ * subsystem by the underlying device.
+ */
+ int (*trap_drop_counter_get)(struct devlink *devlink,
+ const struct devlink_trap *trap,
+ u64 *p_drops);
/**
* @trap_policer_init: Trap policer initialization function.
*
diff --git a/net/core/devlink.c b/net/core/devlink.c
index e43ffc1891a4..2baf8720bb48 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -6995,8 +6995,9 @@ static void devlink_trap_stats_read(struct devlink_stats __percpu *trap_stats,
}
}
-static int devlink_trap_stats_put(struct sk_buff *msg,
- struct devlink_stats __percpu *trap_stats)
+static int
+devlink_trap_group_stats_put(struct sk_buff *msg,
+ struct devlink_stats __percpu *trap_stats)
{
struct devlink_stats stats;
struct nlattr *attr;
@@ -7024,6 +7025,50 @@ static int devlink_trap_stats_put(struct sk_buff *msg,
return -EMSGSIZE;
}
+static int devlink_trap_stats_put(struct sk_buff *msg, struct devlink *devlink,
+ const struct devlink_trap_item *trap_item)
+{
+ struct devlink_stats stats;
+ struct nlattr *attr;
+ u64 drops = 0;
+ int err;
+
+ if (devlink->ops->trap_drop_counter_get) {
+ err = devlink->ops->trap_drop_counter_get(devlink,
+ trap_item->trap,
+ &drops);
+ if (err)
+ return err;
+ }
+
+ devlink_trap_stats_read(trap_item->stats, &stats);
+
+ attr = nla_nest_start(msg, DEVLINK_ATTR_STATS);
+ if (!attr)
+ return -EMSGSIZE;
+
+ if (devlink->ops->trap_drop_counter_get &&
+ nla_put_u64_64bit(msg, DEVLINK_ATTR_STATS_RX_DROPPED, drops,
+ DEVLINK_ATTR_PAD))
+ goto nla_put_failure;
+
+ if (nla_put_u64_64bit(msg, DEVLINK_ATTR_STATS_RX_PACKETS,
+ stats.rx_packets, DEVLINK_ATTR_PAD))
+ goto nla_put_failure;
+
+ if (nla_put_u64_64bit(msg, DEVLINK_ATTR_STATS_RX_BYTES,
+ stats.rx_bytes, DEVLINK_ATTR_PAD))
+ goto nla_put_failure;
+
+ nla_nest_end(msg, attr);
+
+ return 0;
+
+nla_put_failure:
+ nla_nest_cancel(msg, attr);
+ return -EMSGSIZE;
+}
+
static int devlink_nl_trap_fill(struct sk_buff *msg, struct devlink *devlink,
const struct devlink_trap_item *trap_item,
enum devlink_command cmd, u32 portid, u32 seq,
@@ -7061,7 +7106,7 @@ static int devlink_nl_trap_fill(struct sk_buff *msg, struct devlink *devlink,
if (err)
goto nla_put_failure;
- err = devlink_trap_stats_put(msg, trap_item->stats);
+ err = devlink_trap_stats_put(msg, devlink, trap_item);
if (err)
goto nla_put_failure;
@@ -7278,7 +7323,7 @@ devlink_nl_trap_group_fill(struct sk_buff *msg, struct devlink *devlink,
group_item->policer_item->policer->id))
goto nla_put_failure;
- err = devlink_trap_stats_put(msg, group_item->stats);
+ err = devlink_trap_group_stats_put(msg, group_item->stats);
if (err)
goto nla_put_failure;
--
2.17.1
Add devlink_trap_drop_packets_get function, as well as test that are
used to verify devlink (hard) dropped stats functionality works.
Signed-off-by: Oleksandr Mazur <[email protected]>
---
.../selftests/net/forwarding/devlink_lib.sh | 26 +++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/tools/testing/selftests/net/forwarding/devlink_lib.sh b/tools/testing/selftests/net/forwarding/devlink_lib.sh
index c19e001f138b..22931dcfa182 100644
--- a/tools/testing/selftests/net/forwarding/devlink_lib.sh
+++ b/tools/testing/selftests/net/forwarding/devlink_lib.sh
@@ -324,6 +324,14 @@ devlink_trap_rx_bytes_get()
| jq '.[][][]["stats"]["rx"]["bytes"]'
}
+devlink_trap_drop_packets_get()
+{
+ local trap_name=$1; shift
+
+ devlink -js trap show $DEVLINK_DEV trap $trap_name \
+ | jq '.[][][]["stats"]["rx"]["dropped"]'
+}
+
devlink_trap_stats_idle_test()
{
local trap_name=$1; shift
@@ -345,6 +353,24 @@ devlink_trap_stats_idle_test()
fi
}
+devlink_trap_drop_stats_idle_test()
+{
+ local trap_name=$1; shift
+ local t0_packets t0_bytes
+
+ t0_packets=$(devlink_trap_drop_packets_get $trap_name)
+
+ sleep 1
+
+ t1_packets=$(devlink_trap_drop_packets_get $trap_name)
+
+ if [[ $t0_packets -eq $t1_packets ]]; then
+ return 0
+ else
+ return 1
+ fi
+}
+
devlink_traps_enable_all()
{
local trap_name
--
2.17.1
From: Sudarsana Reddy Kalluru <[email protected]>
Kernel has no interface to publish the devlink port
parameters. This is required for exporting the port params to the user space,
so that user can read or update the port params. This patch adds devlink
interfaces (for drivers) to publish/unpublish the devlink port parameters.
Co-developed-by: Ariel Elior <[email protected]>
Signed-off-by: Ariel Elior <[email protected]>
Signed-off-by: Sudarsana Reddy Kalluru <[email protected]>
Signed-off-by: Oleksandr Mazur <[email protected]>
---
include/net/devlink.h | 2 ++
net/core/devlink.c | 42 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 44 insertions(+)
diff --git a/include/net/devlink.h b/include/net/devlink.h
index 7c984cadfec4..8def0f7365da 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -1575,6 +1575,8 @@ int devlink_port_params_register(struct devlink_port *devlink_port,
void devlink_port_params_unregister(struct devlink_port *devlink_port,
const struct devlink_param *params,
size_t params_count);
+void devlink_port_params_publish(struct devlink_port *devlink_port);
+void devlink_port_params_unpublish(struct devlink_port *ddevlink_port);
int devlink_param_driverinit_value_get(struct devlink *devlink, u32 param_id,
union devlink_param_value *init_val);
int devlink_param_driverinit_value_set(struct devlink *devlink, u32 param_id,
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 69681f19388e..e43ffc1891a4 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -9265,6 +9265,48 @@ void devlink_port_params_unregister(struct devlink_port *devlink_port,
}
EXPORT_SYMBOL_GPL(devlink_port_params_unregister);
+/**
+ * devlink_port_params_publish - publish port configuration parameters
+ *
+ * @devlink_port: devlink port
+ *
+ * Publish previously registered port configuration parameters.
+ */
+void devlink_port_params_publish(struct devlink_port *devlink_port)
+{
+ struct devlink_param_item *param_item;
+
+ list_for_each_entry(param_item, &devlink_port->param_list, list) {
+ if (param_item->published)
+ continue;
+ param_item->published = true;
+ devlink_param_notify(devlink_port->devlink, devlink_port->index,
+ param_item, DEVLINK_CMD_PORT_PARAM_NEW);
+ }
+}
+EXPORT_SYMBOL_GPL(devlink_port_params_publish);
+
+/**
+ * devlink_port_params_unpublish - unpublish port configuration parameters
+ *
+ * @devlink_port: devlink port
+ *
+ * Unpublish previously registered port configuration parameters.
+ */
+void devlink_port_params_unpublish(struct devlink_port *devlink_port)
+{
+ struct devlink_param_item *param_item;
+
+ list_for_each_entry(param_item, &devlink_port->param_list, list) {
+ if (!param_item->published)
+ continue;
+ param_item->published = false;
+ devlink_param_notify(devlink_port->devlink, devlink_port->index,
+ param_item, DEVLINK_CMD_PORT_PARAM_DEL);
+ }
+}
+EXPORT_SYMBOL_GPL(devlink_port_params_unpublish);
+
static int
__devlink_param_driverinit_value_get(struct list_head *param_list, u32 param_id,
union devlink_param_value *init_val)
--
2.17.1
Current design makes it impossible to distinguish which port's parameter
should get altered (set) or retrieved (get) whenever there's a single
parameter registered within a few ports.
Change this by adding new devlink port parameter ops structure:
- introduce structure port_params_ops that has callbacks for
get/set/validate;
- if devlink has registered port_params_ops, then upon every devlink
port parameter get/set call invoke port parameters callback
Suggested-by: Vadym Kochan <[email protected]>
Signed-off-by: Oleksandr Mazur <[email protected]>
---
include/net/devlink.h | 23 ++++++++++++++++++++++
net/core/devlink.c | 44 +++++++++++++++++++++++++++++++++++--------
2 files changed, 59 insertions(+), 8 deletions(-)
diff --git a/include/net/devlink.h b/include/net/devlink.h
index b8c6bac067a6..1d687651260c 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -1191,6 +1191,28 @@ enum devlink_trap_group_generic_id {
.min_burst = _min_burst, \
}
+/**
+ * struct devlink_port_param_ops - devlink port parameters-specific ops
+ * @get: get port parameter value, used for runtime and permanent
+ * configuration modes
+ * @set: set port parameter value, used for runtime and permanent
+ * configuration modes
+ * @validate: validate input value is applicable (within value range, etc.)
+ *
+ * This struct should be used by the driver register port parameters ops,
+ * that would be used by the devlink subsystem to fetch (get) or alter (set)
+ * devlink port parameter.
+ */
+struct devlink_port_param_ops {
+ int (*get)(struct devlink_port *port, u32 id,
+ struct devlink_param_gset_ctx *ctx);
+ int (*set)(struct devlink_port *port, u32 id,
+ struct devlink_param_gset_ctx *ctx);
+ int (*validate)(struct devlink_port *port, u32 id,
+ union devlink_param_value val,
+ struct netlink_ext_ack *extack);
+};
+
struct devlink_ops {
/**
* @supported_flash_update_params:
@@ -1463,6 +1485,7 @@ struct devlink_ops {
struct devlink_port *port,
enum devlink_port_fn_state state,
struct netlink_ext_ack *extack);
+ const struct devlink_port_param_ops *port_param_ops;
};
static inline void *devlink_priv(struct devlink *devlink)
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 2baf8720bb48..79566a04083b 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -3923,6 +3923,7 @@ static int devlink_nl_param_fill(struct sk_buff *msg, struct devlink *devlink,
const struct devlink_param *param = param_item->param;
struct devlink_param_gset_ctx ctx;
struct nlattr *param_values_list;
+ struct devlink_port *dl_port;
struct nlattr *param_attr;
int nla_type;
void *hdr;
@@ -3941,7 +3942,20 @@ static int devlink_nl_param_fill(struct sk_buff *msg, struct devlink *devlink,
if (!param_item->published)
continue;
ctx.cmode = i;
- err = devlink_param_get(devlink, param, &ctx);
+ if ((cmd == DEVLINK_CMD_PORT_PARAM_GET ||
+ cmd == DEVLINK_CMD_PORT_PARAM_NEW ||
+ cmd == DEVLINK_CMD_PORT_PARAM_DEL) &&
+ devlink->ops->port_param_ops &&
+ devlink->ops->port_param_ops->get) {
+ dl_port = devlink_port_get_by_index(devlink,
+ port_index);
+ err = devlink->ops->port_param_ops->get(dl_port,
+ param->id,
+ &ctx);
+ } else {
+ err = devlink_param_get(devlink, param, &ctx);
+ }
+
if (err)
return err;
param_value[i] = ctx.val;
@@ -4201,6 +4215,7 @@ static int __devlink_nl_cmd_param_set_doit(struct devlink *devlink,
struct devlink_param_item *param_item;
const struct devlink_param *param;
union devlink_param_value value;
+ struct devlink_port *dl_port;
int err = 0;
param_item = devlink_param_get_from_info(param_list, info);
@@ -4234,13 +4249,28 @@ static int __devlink_nl_cmd_param_set_doit(struct devlink *devlink,
param_item->driverinit_value = value;
param_item->driverinit_value_valid = true;
} else {
- if (!param->set)
- return -EOPNOTSUPP;
ctx.val = value;
ctx.cmode = cmode;
- err = devlink_param_set(devlink, param, &ctx);
- if (err)
- return err;
+
+ if ((cmd == DEVLINK_CMD_PORT_PARAM_SET ||
+ cmd == DEVLINK_CMD_PORT_PARAM_NEW) &&
+ devlink->ops->port_param_ops &&
+ devlink->ops->port_param_ops->set) {
+ dl_port = devlink_port_get_by_index(devlink,
+ port_index);
+ err = devlink->ops->port_param_ops->set(dl_port,
+ param->id,
+ &ctx);
+ if (err)
+ return err;
+ } else {
+ if (!param->set)
+ return -EOPNOTSUPP;
+
+ err = devlink_param_set(devlink, param, &ctx);
+ if (err)
+ return err;
+ }
}
devlink_param_notify(devlink, port_index, param_item, cmd);
@@ -4269,8 +4299,6 @@ static int devlink_param_register_one(struct devlink *devlink,
if (param->supported_cmodes == BIT(DEVLINK_PARAM_CMODE_DRIVERINIT))
WARN_ON(param->get || param->set);
- else
- WARN_ON(!param->get || !param->set);
param_item = kzalloc(sizeof(*param_item), GFP_KERNEL);
if (!param_item)
--
2.17.1
Whenever query statistics is issued for trap with DROP action,
devlink subsystem would also fill-in statistics 'dropped' field.
In case if device driver did't register callback for hard drop
statistics querying, 'dropped' field will be omitted and not filled.
Add trap_drop_counter_get callback implementation to the netdevsim.
Add new test cases for netdevsim, to test both the callback
functionality, as well as drop statistics alteration check.
Signed-off-by: Oleksandr Mazur <[email protected]>
---
drivers/net/netdevsim/dev.c | 22 ++++++++++++++++++++++
drivers/net/netdevsim/netdevsim.h | 1 +
2 files changed, 23 insertions(+)
diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index 6189a4c0d39e..87e039433312 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -231,6 +231,9 @@ static int nsim_dev_debugfs_init(struct nsim_dev *nsim_dev)
debugfs_create_bool("fail_trap_policer_counter_get", 0600,
nsim_dev->ddir,
&nsim_dev->fail_trap_policer_counter_get);
+ debugfs_create_bool("fail_trap_counter_get", 0600,
+ nsim_dev->ddir,
+ &nsim_dev->fail_trap_counter_get);
nsim_udp_tunnels_debugfs_create(nsim_dev);
return 0;
}
@@ -416,6 +419,7 @@ struct nsim_trap_data {
struct delayed_work trap_report_dw;
struct nsim_trap_item *trap_items_arr;
u64 *trap_policers_cnt_arr;
+ u64 trap_pkt_cnt;
struct nsim_dev *nsim_dev;
spinlock_t trap_lock; /* Protects trap_items_arr */
};
@@ -892,6 +896,23 @@ nsim_dev_devlink_trap_policer_counter_get(struct devlink *devlink,
return 0;
}
+static int
+nsim_dev_devlink_trap_hw_counter_get(struct devlink *devlink,
+ const struct devlink_trap *trap,
+ u64 *p_drops)
+{
+ struct nsim_dev *nsim_dev = devlink_priv(devlink);
+ u64 *cnt;
+
+ if (nsim_dev->fail_trap_counter_get)
+ return -EINVAL;
+
+ cnt = &nsim_dev->trap_data->trap_pkt_cnt;
+ *p_drops = (*cnt)++;
+
+ return 0;
+}
+
static const struct devlink_ops nsim_dev_devlink_ops = {
.supported_flash_update_params = DEVLINK_SUPPORT_FLASH_UPDATE_COMPONENT |
DEVLINK_SUPPORT_FLASH_UPDATE_OVERWRITE_MASK,
@@ -905,6 +926,7 @@ static const struct devlink_ops nsim_dev_devlink_ops = {
.trap_group_set = nsim_dev_devlink_trap_group_set,
.trap_policer_set = nsim_dev_devlink_trap_policer_set,
.trap_policer_counter_get = nsim_dev_devlink_trap_policer_counter_get,
+ .trap_drop_counter_get = nsim_dev_devlink_trap_hw_counter_get,
};
#define NSIM_DEV_MAX_MACS_DEFAULT 32
diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
index 7ff24e03577b..db66a0e58792 100644
--- a/drivers/net/netdevsim/netdevsim.h
+++ b/drivers/net/netdevsim/netdevsim.h
@@ -236,6 +236,7 @@ struct nsim_dev {
bool fail_trap_group_set;
bool fail_trap_policer_set;
bool fail_trap_policer_counter_get;
+ bool fail_trap_counter_get;
struct {
struct udp_tunnel_nic_shared utn_shared;
u32 __ports[2][NSIM_UDP_TUNNEL_N_PORTS];
--
2.17.1
Add documentation for the devlink feature prestera switchdev driver supports:
add description for the support for the per-port devlink Parameters
(used form storm control);
add description for the support of the driver-specific devlink traps
(include both traps with action TRAP and action DROP);
Signed-off-by: Oleksandr Mazur <[email protected]>
---
Documentation/networking/devlink/prestera.rst | 167 ++++++++++++++++++
1 file changed, 167 insertions(+)
create mode 100644 Documentation/networking/devlink/prestera.rst
diff --git a/Documentation/networking/devlink/prestera.rst b/Documentation/networking/devlink/prestera.rst
new file mode 100644
index 000000000000..b73d70319344
--- /dev/null
+++ b/Documentation/networking/devlink/prestera.rst
@@ -0,0 +1,167 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=====================
+prestera devlink support
+=====================
+
+This document describes the devlink features implemented by the ``prestera``
+device driver.
+
+Parameters (per-port)
+=====================
+
+.. list-table:: Driver-specific parameters (per-port) implemented
+ :widths: 5 5 5 85
+
+ * - Name
+ - Type
+ - Mode
+ - Description
+ * - ``bc_kbyte_per_sec_rate``
+ - u32
+ - runtime
+ - Sets the maximum ingress traffic rate, at which port can
+ receive broadcast traffic.
+ * - ``unk_uc_kbyte_per_sec_rate``
+ - u32
+ - runtime
+ - Sets the maximum ingress traffic rate, at which port can
+ receive unknown unicast traffic.
+ * - ``unreg_mc_kbyte_per_sec_rate``
+ - u32
+ - runtime
+ - Sets the maximum ingress traffic rate, at which port can
+ receive unregistered multicast traffic.
+
+Driver-specific Traps
+=====================
+
+.. list-table:: List of Driver-specific Traps Registered by ``prestera``
+ :widths: 5 5 90
+
+ * - Name
+ - Type
+ - Description
+.. list-table:: List of Driver-specific Traps Registered by ``prestera``
+ :widths: 5 5 90
+
+ * - Name
+ - Type
+ - Description
+ * - ``arp_bc``
+ - ``trap``
+ - Traps ARP broadcast packets (both requests/responses)
+ * - ``is_is``
+ - ``trap``
+ - Traps IS-IS packets
+ * - ``ospf``
+ - ``trap``
+ - Traps OSPF packets
+ * - ``ip_bc_mac``
+ - ``trap``
+ - Traps IPv4 packets with broadcast DA Mac address
+ * - ``stp``
+ - ``trap``
+ - Traps STP BPDU
+ * - ``lacp``
+ - ``trap``
+ - Traps LACP packets
+ * - ``lldp``
+ - ``trap``
+ - Traps LLDP packets
+ * - ``router_mc``
+ - ``trap``
+ - Traps multicast packets
+ * - ``vrrp``
+ - ``trap``
+ - Traps VRRP packets
+ * - ``dhcp``
+ - ``trap``
+ - Traps DHCP packets
+ * - ``mtu_error``
+ - ``trap``
+ - Traps (exception) packets that exceeded port's MTU
+ * - ``mac_to_me``
+ - ``trap``
+ - Traps packets with switch-port's DA Mac address
+ * - ``ttl_error``
+ - ``trap``
+ - Traps (exception) IPv4 packets whose TTL exceeded
+ * - ``ipv4_options``
+ - ``trap``
+ - Traps (exception) packets due to the malformed IPV4 header options
+ * - ``ip_default_route``
+ - ``trap``
+ - Traps packets that have no specific IP interface (IP to me) and no forwarding prefix
+ * - ``local_route``
+ - ``trap``
+ - Traps packets that have been send to one of switch IP interfaces addresses
+ * - ``ipv4_icmp_redirect``
+ - ``trap``
+ - Traps (exception) IPV4 ICMP redirect packets
+ * - ``arp_response``
+ - ``trap``
+ - Traps ARP replies packets that have switch-port's DA Mac address
+ * - ``acl_code_0``
+ - ``trap``
+ - Traps packets that have ACL priority set to 0 (tc pref 0)
+ * - ``acl_code_1``
+ - ``trap``
+ - Traps packets that have ACL priority set to 1 (tc pref 1)
+ * - ``acl_code_2``
+ - ``trap``
+ - Traps packets that have ACL priority set to 2 (tc pref 2)
+ * - ``acl_code_3``
+ - ``trap``
+ - Traps packets that have ACL priority set to 3 (tc pref 3)
+ * - ``acl_code_4``
+ - ``trap``
+ - Traps packets that have ACL priority set to 4 (tc pref 4)
+ * - ``acl_code_5``
+ - ``trap``
+ - Traps packets that have ACL priority set to 5 (tc pref 5)
+ * - ``acl_code_6``
+ - ``trap``
+ - Traps packets that have ACL priority set to 6 (tc pref 6)
+ * - ``acl_code_7``
+ - ``trap``
+ - Traps packets that have ACL priority set to 7 (tc pref 7)
+ * - ``ipv4_bgp``
+ - ``trap``
+ - Traps IPv4 BGP packets
+ * - ``ssh``
+ - ``trap``
+ - Traps SSH packets
+ * - ``telnet``
+ - ``trap``
+ - Traps Telnet packets
+ * - ``icmp``
+ - ``trap``
+ - Traps ICMP packets
+ * - ``rxdma_drop``
+ - ``drop``
+ - Drops packets (RxDMA) due to the lack of ingress buffers etc.
+ * - ``port_no_vlan``
+ - ``drop``
+ - Drops packets due to faulty-configured network or due to internal bug (config issue).
+ * - ``local_port``
+ - ``drop``
+ - Drops packets whose decision (FDB entry) is to bridge packet back to the incoming port/trunk.
+ * - ``invalid_sa``
+ - ``drop``
+ - Drops packets with multicast source MAC address.
+ * - ``illegal_ip_addr``
+ - ``drop``
+ - Drops packets with illegal SIP/DIP multicast/unicast addresses.
+ * - ``illegal_ipv4_hdr``
+ - ``drop``
+ - Drops packets with illegal IPV4 header.
+ * - ``ip_uc_dip_da_mismatch``
+ - ``drop``
+ - Drops packets with destination MAC being unicast, but destination IP address being multicast.
+ * - ``ip_sip_is_zero``
+ - ``drop``
+ - Drops packets with zero (0) IPV4 source address.
+ * - ``met_red``
+ - ``drop``
+ - Drops non-conforming packets (dropped by Ingress policer, metering drop), e.g. packet rate exceeded configured bandwith.
--
2.17.1
Add traps that have init_action being set to DROP.
Add 'trap_drop_counter_get' (devlink API) callback implementation,
that is used to get number of packets that have been dropped by the HW
(traps with action 'DROP').
Add new FW command CPU_CODE_COUNTERS_GET.
Signed-off-by: Oleksandr Mazur <[email protected]>
---
.../marvell/prestera/prestera_devlink.c | 91 +++++++++++++++++++
.../ethernet/marvell/prestera/prestera_hw.c | 35 +++++++
.../ethernet/marvell/prestera/prestera_hw.h | 11 +++
3 files changed, 137 insertions(+)
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_devlink.c b/drivers/net/ethernet/marvell/prestera/prestera_devlink.c
index f59727f050ba..d12e21db9fd6 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_devlink.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_devlink.c
@@ -4,6 +4,7 @@
#include <net/devlink.h>
#include "prestera_devlink.h"
+#include "prestera_hw.h"
/* All driver-specific traps must be documented in
* Documentation/networking/devlink/prestera.rst
@@ -34,6 +35,15 @@ enum {
DEVLINK_PRESTERA_TRAP_ID_SSH,
DEVLINK_PRESTERA_TRAP_ID_TELNET,
DEVLINK_PRESTERA_TRAP_ID_ICMP,
+ DEVLINK_PRESTERA_TRAP_ID_MET_RED,
+ DEVLINK_PRESTERA_TRAP_ID_IP_SIP_IS_ZERO,
+ DEVLINK_PRESTERA_TRAP_ID_IP_UC_DIP_DA_MISMATCH,
+ DEVLINK_PRESTERA_TRAP_ID_ILLEGAL_IPV4_HDR,
+ DEVLINK_PRESTERA_TRAP_ID_ILLEGAL_IP_ADDR,
+ DEVLINK_PRESTERA_TRAP_ID_INVALID_SA,
+ DEVLINK_PRESTERA_TRAP_ID_LOCAL_PORT,
+ DEVLINK_PRESTERA_TRAP_ID_PORT_NO_VLAN,
+ DEVLINK_PRESTERA_TRAP_ID_RXDMA_DROP,
};
#define DEVLINK_PRESTERA_TRAP_NAME_ARP_BC \
@@ -84,6 +94,24 @@ enum {
"telnet"
#define DEVLINK_PRESTERA_TRAP_NAME_ICMP \
"icmp"
+#define DEVLINK_PRESTERA_TRAP_NAME_RXDMA_DROP \
+ "rxdma_drop"
+#define DEVLINK_PRESTERA_TRAP_NAME_PORT_NO_VLAN \
+ "port_no_vlan"
+#define DEVLINK_PRESTERA_TRAP_NAME_LOCAL_PORT \
+ "local_port"
+#define DEVLINK_PRESTERA_TRAP_NAME_INVALID_SA \
+ "invalid_sa"
+#define DEVLINK_PRESTERA_TRAP_NAME_ILLEGAL_IP_ADDR \
+ "illegal_ip_addr"
+#define DEVLINK_PRESTERA_TRAP_NAME_ILLEGAL_IPV4_HDR \
+ "illegal_ipv4_hdr"
+#define DEVLINK_PRESTERA_TRAP_NAME_IP_UC_DIP_DA_MISMATCH \
+ "ip_uc_dip_da_mismatch"
+#define DEVLINK_PRESTERA_TRAP_NAME_IP_SIP_IS_ZERO \
+ "ip_sip_is_zero"
+#define DEVLINK_PRESTERA_TRAP_NAME_MET_RED \
+ "met_red"
struct prestera_trap {
struct devlink_trap trap;
@@ -125,6 +153,12 @@ struct prestera_trap_data {
DEVLINK_TRAP_GROUP_GENERIC_ID_##_group_id, \
PRESTERA_TRAP_METADATA)
+#define PRESTERA_TRAP_DRIVER_DROP(_id, _group_id) \
+ DEVLINK_TRAP_DRIVER(DROP, DROP, DEVLINK_PRESTERA_TRAP_ID_##_id, \
+ DEVLINK_PRESTERA_TRAP_NAME_##_id, \
+ DEVLINK_TRAP_GROUP_GENERIC_ID_##_group_id, \
+ PRESTERA_TRAP_METADATA)
+
static const struct devlink_trap_group prestera_trap_groups_arr[] = {
/* No policer is associated with following groups (policerid == 0)*/
DEVLINK_TRAP_GROUP_GENERIC(L2_DROPS, 0),
@@ -142,6 +176,7 @@ static const struct devlink_trap_group prestera_trap_groups_arr[] = {
DEVLINK_TRAP_GROUP_GENERIC(DHCP, 0),
DEVLINK_TRAP_GROUP_GENERIC(BGP, 0),
DEVLINK_TRAP_GROUP_GENERIC(LOCAL_DELIVERY, 0),
+ DEVLINK_TRAP_GROUP_GENERIC(BUFFER_DROPS, 0),
};
/* Initialize trap list, as well as associate CPU code with them. */
@@ -271,10 +306,51 @@ static struct prestera_trap prestera_trap_items_arr[] = {
.trap = PRESTERA_TRAP_DRIVER_CONTROL(ICMP, LOCAL_DELIVERY),
.cpu_code = 209,
},
+ {
+ .trap = PRESTERA_TRAP_DRIVER_DROP(RXDMA_DROP, BUFFER_DROPS),
+ .cpu_code = 37,
+ },
+ {
+ .trap = PRESTERA_TRAP_DRIVER_DROP(PORT_NO_VLAN, L2_DROPS),
+ .cpu_code = 39,
+ },
+ {
+ .trap = PRESTERA_TRAP_DRIVER_DROP(LOCAL_PORT, L2_DROPS),
+ .cpu_code = 56,
+ },
+ {
+ .trap = PRESTERA_TRAP_DRIVER_DROP(INVALID_SA, L2_DROPS),
+ .cpu_code = 60,
+ },
+ {
+ .trap = PRESTERA_TRAP_DRIVER_DROP(ILLEGAL_IP_ADDR, L3_DROPS),
+ .cpu_code = 136,
+ },
+ {
+ .trap = PRESTERA_TRAP_DRIVER_DROP(ILLEGAL_IPV4_HDR, L3_DROPS),
+ .cpu_code = 137,
+ },
+ {
+ .trap = PRESTERA_TRAP_DRIVER_DROP(IP_UC_DIP_DA_MISMATCH,
+ L3_DROPS),
+ .cpu_code = 138,
+ },
+ {
+ .trap = PRESTERA_TRAP_DRIVER_DROP(IP_SIP_IS_ZERO, L3_DROPS),
+ .cpu_code = 145,
+ },
+ {
+ .trap = PRESTERA_TRAP_DRIVER_DROP(MET_RED, BUFFER_DROPS),
+ .cpu_code = 185,
+ },
};
static void prestera_devlink_traps_fini(struct prestera_switch *sw);
+static int prestera_drop_counter_get(struct devlink *devlink,
+ const struct devlink_trap *trap,
+ u64 *p_drops);
+
static int prestera_dl_info_get(struct devlink *dl,
struct devlink_info_req *req,
struct netlink_ext_ack *extack)
@@ -311,6 +387,7 @@ static const struct devlink_ops prestera_dl_ops = {
.info_get = prestera_dl_info_get,
.trap_init = prestera_trap_init,
.trap_action_set = prestera_trap_action_set,
+ .trap_drop_counter_get = prestera_drop_counter_get,
};
struct prestera_switch *prestera_devlink_alloc(void)
@@ -531,6 +608,20 @@ static int prestera_trap_action_set(struct devlink *devlink,
return -EOPNOTSUPP;
}
+static int prestera_drop_counter_get(struct devlink *devlink,
+ const struct devlink_trap *trap,
+ u64 *p_drops)
+{
+ struct prestera_switch *sw = devlink_priv(devlink);
+ enum prestera_hw_cpu_code_cnt_t cpu_code_type =
+ PRESTERA_HW_CPU_CODE_CNT_TYPE_DROP;
+ struct prestera_trap *prestera_trap =
+ container_of(trap, struct prestera_trap, trap);
+
+ return prestera_hw_cpu_code_counters_get(sw, prestera_trap->cpu_code,
+ cpu_code_type, p_drops);
+}
+
static void prestera_devlink_traps_fini(struct prestera_switch *sw)
{
struct devlink *dl = priv_to_devlink(sw);
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_hw.c b/drivers/net/ethernet/marvell/prestera/prestera_hw.c
index 96ce73b50fec..0e5b3f8e7dc7 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_hw.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_hw.c
@@ -42,6 +42,8 @@ enum prestera_cmd_type_t {
PRESTERA_CMD_TYPE_STP_PORT_SET = 0x1000,
+ PRESTERA_CMD_TYPE_CPU_CODE_COUNTERS_GET = 0x2000,
+
PRESTERA_CMD_TYPE_ACK = 0x10000,
PRESTERA_CMD_TYPE_MAX
};
@@ -305,6 +307,17 @@ struct prestera_msg_rxtx_port_req {
u32 dev;
};
+struct prestera_msg_cpu_code_counter_req {
+ struct prestera_msg_cmd cmd;
+ u8 counter_type;
+ u8 code;
+};
+
+struct mvsw_msg_cpu_code_counter_ret {
+ struct prestera_msg_ret ret;
+ u64 packet_count;
+};
+
struct prestera_msg_event {
u16 type;
u16 id;
@@ -1295,6 +1308,28 @@ int prestera_hw_rxtx_port_init(struct prestera_port *port)
&req.cmd, sizeof(req));
}
+int
+prestera_hw_cpu_code_counters_get(struct prestera_switch *sw, u8 code,
+ enum prestera_hw_cpu_code_cnt_t counter_type,
+ u64 *packet_count)
+{
+ struct prestera_msg_cpu_code_counter_req req = {
+ .counter_type = counter_type,
+ .code = code,
+ };
+ struct mvsw_msg_cpu_code_counter_ret resp;
+ int err;
+
+ err = prestera_cmd_ret(sw, PRESTERA_CMD_TYPE_CPU_CODE_COUNTERS_GET,
+ &req.cmd, sizeof(req), &resp.ret, sizeof(resp));
+ if (err)
+ return err;
+
+ *packet_count = resp.packet_count;
+
+ return 0;
+}
+
int prestera_hw_event_handler_register(struct prestera_switch *sw,
enum prestera_event_type type,
prestera_event_cb_t fn,
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_hw.h b/drivers/net/ethernet/marvell/prestera/prestera_hw.h
index e8dd0e2b81d2..aafecf0ecd16 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_hw.h
+++ b/drivers/net/ethernet/marvell/prestera/prestera_hw.h
@@ -89,6 +89,11 @@ enum {
PRESTERA_STP_FORWARD,
};
+enum prestera_hw_cpu_code_cnt_t {
+ PRESTERA_HW_CPU_CODE_CNT_TYPE_DROP = 0,
+ PRESTERA_HW_CPU_CODE_CNT_TYPE_TRAP = 1,
+};
+
struct prestera_switch;
struct prestera_port;
struct prestera_port_stats;
@@ -180,4 +185,10 @@ int prestera_hw_rxtx_init(struct prestera_switch *sw,
struct prestera_rxtx_params *params);
int prestera_hw_rxtx_port_init(struct prestera_port *port);
+/* HW trap/drop counters API */
+int
+prestera_hw_cpu_code_counters_get(struct prestera_switch *sw, u8 code,
+ enum prestera_hw_cpu_code_cnt_t counter_type,
+ u64 *packet_count);
+
#endif /* _PRESTERA_HW_H_ */
--
2.17.1
Storm control (BUM) provides a mechanism to limit rate of ingress
port traffic (matched by type). Devlink port parameter API is used:
driver registers a set of per-port parameters that can be accessed to both
get/set per-port per-type rate limit.
Add new FW command - RATE_LIMIT_MODE_SET.
Signed-off-by: Oleksandr Mazur <[email protected]>
---
.../net/ethernet/marvell/prestera/prestera.h | 7 +
.../marvell/prestera/prestera_devlink.c | 134 +++++++++++++++++-
.../ethernet/marvell/prestera/prestera_hw.c | 25 ++++
.../ethernet/marvell/prestera/prestera_hw.h | 9 ++
4 files changed, 174 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/marvell/prestera/prestera.h b/drivers/net/ethernet/marvell/prestera/prestera.h
index 2c94bdec84b1..4b99a7421452 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera.h
+++ b/drivers/net/ethernet/marvell/prestera/prestera.h
@@ -60,6 +60,12 @@ struct prestera_port_caps {
u8 transceiver;
};
+struct prestera_strom_control_cfg {
+ u32 bc_kbyte_per_sec_rate;
+ u32 unk_uc_kbyte_per_sec_rate;
+ u32 unreg_mc_kbyte_per_sec_rate;
+};
+
struct prestera_port {
struct net_device *dev;
struct prestera_switch *sw;
@@ -79,6 +85,7 @@ struct prestera_port {
struct prestera_port_stats stats;
struct delayed_work caching_dw;
} cached_hw_stats;
+ struct prestera_strom_control_cfg storm_control;
};
struct prestera_device {
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_devlink.c b/drivers/net/ethernet/marvell/prestera/prestera_devlink.c
index d12e21db9fd6..0786fbb09f71 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_devlink.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_devlink.c
@@ -2,6 +2,8 @@
/* Copyright (c) 2019-2020 Marvell International Ltd. All rights reserved */
#include <net/devlink.h>
+#include <linux/bitops.h>
+#include <linux/bitfield.h>
#include "prestera_devlink.h"
#include "prestera_hw.h"
@@ -159,6 +161,34 @@ struct prestera_trap_data {
DEVLINK_TRAP_GROUP_GENERIC_ID_##_group_id, \
PRESTERA_TRAP_METADATA)
+#define PRESTERA_PORT_PARAM_DRIVER_RUNTIME(_id, _name, _type) \
+ DEVLINK_PARAM_DRIVER(PRESTERA_DEVLINK_PORT_PARAM_ID_##_id, _name, \
+ _type, BIT(DEVLINK_PARAM_CMODE_RUNTIME), NULL, \
+ NULL, NULL)
+
+struct prestera_storm_control {
+ struct prestera_switch *sw;
+ struct prestera_strom_control_cfg *cfg;
+};
+
+enum prestera_devlink_port_param_id {
+ PRESTERA_DEVLINK_PORT_PARAM_ID_BASE = DEVLINK_PARAM_GENERIC_ID_MAX + 1,
+ PRESTERA_DEVLINK_PORT_PARAM_ID_BC_RATE,
+ PRESTERA_DEVLINK_PORT_PARAM_ID_UC_UNK_RATE,
+ PRESTERA_DEVLINK_PORT_PARAM_ID_MC_RATE,
+};
+
+struct devlink_param prestera_devlink_port_params[] = {
+ PRESTERA_PORT_PARAM_DRIVER_RUNTIME(BC_RATE, "bc_kbyte_per_sec_rate",
+ DEVLINK_PARAM_TYPE_U32),
+ PRESTERA_PORT_PARAM_DRIVER_RUNTIME(UC_UNK_RATE,
+ "unk_uc_kbyte_per_sec_rate",
+ DEVLINK_PARAM_TYPE_U32),
+ PRESTERA_PORT_PARAM_DRIVER_RUNTIME(MC_RATE,
+ "unreg_mc_kbyte_per_sec_rate",
+ DEVLINK_PARAM_TYPE_U32),
+};
+
static const struct devlink_trap_group prestera_trap_groups_arr[] = {
/* No policer is associated with following groups (policerid == 0)*/
DEVLINK_TRAP_GROUP_GENERIC(L2_DROPS, 0),
@@ -350,6 +380,10 @@ static void prestera_devlink_traps_fini(struct prestera_switch *sw);
static int prestera_drop_counter_get(struct devlink *devlink,
const struct devlink_trap *trap,
u64 *p_drops);
+static int prestera_devlink_port_param_set(struct devlink_port *dl_port, u32 id,
+ struct devlink_param_gset_ctx *ctx);
+static int prestera_devlink_port_param_get(struct devlink_port *dl_port, u32 id,
+ struct devlink_param_gset_ctx *ctx);
static int prestera_dl_info_get(struct devlink *dl,
struct devlink_info_req *req,
@@ -383,11 +417,17 @@ static int prestera_trap_action_set(struct devlink *devlink,
static int prestera_devlink_traps_register(struct prestera_switch *sw);
+static const struct devlink_port_param_ops prestera_devlink_port_param_ops = {
+ .get = prestera_devlink_port_param_get,
+ .set = prestera_devlink_port_param_set,
+};
+
static const struct devlink_ops prestera_dl_ops = {
.info_get = prestera_dl_info_get,
.trap_init = prestera_trap_init,
.trap_action_set = prestera_trap_action_set,
.trap_drop_counter_get = prestera_drop_counter_get,
+ .port_param_ops = &prestera_devlink_port_param_ops,
};
struct prestera_switch *prestera_devlink_alloc(void)
@@ -443,10 +483,12 @@ void prestera_devlink_unregister(struct prestera_switch *sw)
int prestera_devlink_port_register(struct prestera_port *port)
{
struct prestera_switch *sw = port->sw;
- struct devlink *dl = priv_to_devlink(sw);
struct devlink_port_attrs attrs = {};
+ struct devlink *dl;
int err;
+ dl = priv_to_devlink(sw);
+
attrs.flavour = DEVLINK_PORT_FLAVOUR_PHYSICAL;
attrs.phys.port_number = port->fp_id;
attrs.switch_id.id_len = sizeof(sw->id);
@@ -460,12 +502,32 @@ int prestera_devlink_port_register(struct prestera_port *port)
return err;
}
+ err = devlink_port_params_register(
+ &port->dl_port,
+ prestera_devlink_port_params,
+ ARRAY_SIZE(prestera_devlink_port_params));
+ if (err) {
+ devlink_port_unregister(&port->dl_port);
+ dev_err(sw->dev->dev, "devlink_port_params_register failed\n");
+ return err;
+ }
+
+ devlink_port_params_publish(&port->dl_port);
+
return 0;
}
void prestera_devlink_port_unregister(struct prestera_port *port)
{
+ devlink_port_params_unpublish(&port->dl_port);
+
+ devlink_port_params_unregister(
+ &port->dl_port,
+ prestera_devlink_port_params,
+ ARRAY_SIZE(prestera_devlink_port_params));
+
devlink_port_unregister(&port->dl_port);
+
}
void prestera_devlink_port_set(struct prestera_port *port)
@@ -622,6 +684,76 @@ static int prestera_drop_counter_get(struct devlink *devlink,
cpu_code_type, p_drops);
}
+static int prestera_devlink_port_param_set(struct devlink_port *dl_port, u32 id,
+ struct devlink_param_gset_ctx *ctx)
+{
+ struct prestera_strom_control_cfg *cfg;
+ u32 kbyte_per_sec_rate = ctx->val.vu32;
+ struct prestera_port *port;
+ struct prestera_switch *sw;
+ u32 *param_to_set;
+ u32 storm_type;
+ int ret;
+
+ port = container_of(dl_port, struct prestera_port, dl_port);
+ sw = devlink_priv(dl_port->devlink);
+ cfg = &port->storm_control;
+
+ switch (id) {
+ case PRESTERA_DEVLINK_PORT_PARAM_ID_BC_RATE:
+ param_to_set = &cfg->bc_kbyte_per_sec_rate;
+ storm_type = PRESTERA_PORT_STORM_CTL_TYPE_BC;
+ break;
+ case PRESTERA_DEVLINK_PORT_PARAM_ID_UC_UNK_RATE:
+ param_to_set = &cfg->unk_uc_kbyte_per_sec_rate;
+ storm_type = PRESTERA_PORT_STORM_CTL_TYPE_UC_UNK;
+ break;
+ case PRESTERA_DEVLINK_PORT_PARAM_ID_MC_RATE:
+ param_to_set = &cfg->unreg_mc_kbyte_per_sec_rate;
+ storm_type = PRESTERA_PORT_STORM_CTL_TYPE_MC;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ if (kbyte_per_sec_rate != *param_to_set) {
+ ret = prestera_hw_port_storm_control_cfg_set(port, storm_type,
+ kbyte_per_sec_rate);
+ if (ret)
+ return ret;
+
+ *param_to_set = kbyte_per_sec_rate;
+ }
+
+ return 0;
+}
+
+static int prestera_devlink_port_param_get(struct devlink_port *dl_port, u32 id,
+ struct devlink_param_gset_ctx *ctx)
+{
+ struct prestera_strom_control_cfg *cfg;
+ struct prestera_port *port;
+ struct prestera_switch *sw;
+
+ port = container_of(dl_port, struct prestera_port, dl_port);
+ sw = devlink_priv(dl_port->devlink);
+ cfg = &port->storm_control;
+
+ switch (id) {
+ case PRESTERA_DEVLINK_PORT_PARAM_ID_BC_RATE:
+ ctx->val.vu32 = cfg->bc_kbyte_per_sec_rate;
+ return 0;
+ case PRESTERA_DEVLINK_PORT_PARAM_ID_UC_UNK_RATE:
+ ctx->val.vu32 = cfg->unk_uc_kbyte_per_sec_rate;
+ return 0;
+ case PRESTERA_DEVLINK_PORT_PARAM_ID_MC_RATE:
+ ctx->val.vu32 = cfg->unreg_mc_kbyte_per_sec_rate;
+ return 0;
+ default:
+ return -EINVAL;
+ }
+}
+
static void prestera_devlink_traps_fini(struct prestera_switch *sw)
{
struct devlink *dl = priv_to_devlink(sw);
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_hw.c b/drivers/net/ethernet/marvell/prestera/prestera_hw.c
index 0e5b3f8e7dc7..85a1a15717df 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_hw.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_hw.c
@@ -20,6 +20,7 @@ enum prestera_cmd_type_t {
PRESTERA_CMD_TYPE_PORT_ATTR_SET = 0x100,
PRESTERA_CMD_TYPE_PORT_ATTR_GET = 0x101,
PRESTERA_CMD_TYPE_PORT_INFO_GET = 0x110,
+ PRESTERA_CMD_TYPE_PORT_RATE_LIMIT_MODE_SET = 0x111,
PRESTERA_CMD_TYPE_VLAN_CREATE = 0x200,
PRESTERA_CMD_TYPE_VLAN_DELETE = 0x201,
@@ -251,6 +252,14 @@ struct prestera_msg_port_info_resp {
u16 fp_id;
};
+struct prestera_msg_port_storm_control_cfg_set_req {
+ struct prestera_msg_cmd cmd;
+ u32 port;
+ u32 dev;
+ u32 storm_type;
+ u32 kbyte_per_sec_rate;
+};
+
struct prestera_msg_vlan_req {
struct prestera_msg_cmd cmd;
u32 port;
@@ -639,6 +648,22 @@ int prestera_hw_port_accept_frm_type(struct prestera_port *port,
&req.cmd, sizeof(req));
}
+int prestera_hw_port_storm_control_cfg_set(const struct prestera_port *port,
+ u32 storm_type,
+ u32 kbyte_per_sec_rate)
+{
+ struct prestera_msg_port_storm_control_cfg_set_req req = {
+ .port = port->hw_id,
+ .dev = port->dev_id,
+ .storm_type = storm_type,
+ .kbyte_per_sec_rate = kbyte_per_sec_rate
+ };
+
+ return prestera_cmd(port->sw,
+ PRESTERA_CMD_TYPE_PORT_RATE_LIMIT_MODE_SET,
+ &req.cmd, sizeof(req));
+}
+
int prestera_hw_port_cap_get(const struct prestera_port *port,
struct prestera_port_caps *caps)
{
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_hw.h b/drivers/net/ethernet/marvell/prestera/prestera_hw.h
index aafecf0ecd16..85373f1d3971 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_hw.h
+++ b/drivers/net/ethernet/marvell/prestera/prestera_hw.h
@@ -89,6 +89,12 @@ enum {
PRESTERA_STP_FORWARD,
};
+enum {
+ PRESTERA_PORT_STORM_CTL_TYPE_BC = 0,
+ PRESTERA_PORT_STORM_CTL_TYPE_UC_UNK = 1,
+ PRESTERA_PORT_STORM_CTL_TYPE_MC = 2
+};
+
enum prestera_hw_cpu_code_cnt_t {
PRESTERA_HW_CPU_CODE_CNT_TYPE_DROP = 0,
PRESTERA_HW_CPU_CODE_CNT_TYPE_TRAP = 1,
@@ -123,6 +129,9 @@ int prestera_hw_port_mac_set(const struct prestera_port *port, const char *mac);
int prestera_hw_port_mac_get(const struct prestera_port *port, char *mac);
int prestera_hw_port_cap_get(const struct prestera_port *port,
struct prestera_port_caps *caps);
+int prestera_hw_port_storm_control_cfg_set(const struct prestera_port *port,
+ u32 storm_type,
+ u32 kbyte_per_sec_rate);
int prestera_hw_port_remote_cap_get(const struct prestera_port *port,
u64 *link_mode_bitmap);
int prestera_hw_port_remote_fc_get(const struct prestera_port *port,
--
2.17.1
On Wed, Jun 09, 2021 at 06:16:00PM +0300, Oleksandr Mazur wrote:
> Storm control (BUM) provides a mechanism to limit rate of ingress
> port traffic (matched by type). Devlink port parameter API is used:
> driver registers a set of per-port parameters that can be accessed to both
> get/set per-port per-type rate limit.
> Add new FW command - RATE_LIMIT_MODE_SET.
This should be properly modeled in the bridge driver and offloaded to
capable drivers via switchdev. Modeling it as a driver-specific devlink
parameter is wrong.
On 09/06/2021 20:59, Ido Schimmel wrote:
> On Wed, Jun 09, 2021 at 06:16:00PM +0300, Oleksandr Mazur wrote:
>> Storm control (BUM) provides a mechanism to limit rate of ingress
>> port traffic (matched by type). Devlink port parameter API is used:
>> driver registers a set of per-port parameters that can be accessed to both
>> get/set per-port per-type rate limit.
>> Add new FW command - RATE_LIMIT_MODE_SET.
>
> This should be properly modeled in the bridge driver and offloaded to
> capable drivers via switchdev. Modeling it as a driver-specific devlink
> parameter is wrong.
>
Absolutely agree with Ido, there are many different ways to achieve it through
the bridge (e.g. generic bridge helpers to be used by bpf, directly by tc or new br tc hooks
to name a few). I'd personally be excited to see any of these implemented as they
could open the door for a lot other interesting use cases. Unfortunately I'm currently swamped
with per-vlan multicast support, after that depending on time availability I could look
into this unless someone beats me to it. :)
Cheers,
Nik
On Wed, Jun 09, 2021 at 06:16:00PM +0300, Oleksandr Mazur wrote:
> Storm control (BUM) provides a mechanism to limit rate of ingress
> port traffic (matched by type). Devlink port parameter API is used:
> driver registers a set of per-port parameters that can be accessed to both
> get/set per-port per-type rate limit.
> Add new FW command - RATE_LIMIT_MODE_SET.
Hi Oleksandr
Just expanding on the two comments you already received about this.
We often see people miss that switchdev is about. It is not about
writing switch drivers. It is about writing network stack
accelerators. You take a feature of the Linux network stack and you
accelerate it by offloading it to the hardware. So look around the
network stack and see how you configure it to perform rate limiting of
broadcast traffic ingress. Once you have found a suitable mechanism,
accelerate it via offloading.
If you find Linux has no way to perform a feature the hardware could
accelerate, you first need to add a pure software version of that
feature to the network stack, and then add acceleration support for
it.
Andrew
>> On Wed, Jun 09, 2021 at 06:16:00PM +0300, Oleksandr Mazur wrote:
> Storm control (BUM) provides a mechanism to limit rate of ingress
> > port traffic (matched by type). Devlink port parameter API is used:
> > driver registers a set of per-port parameters that can be accessed to both
> > get/set per-port per-type rate limit.
> > Add new FW command - RATE_LIMIT_MODE_SET.
> Hi Oleksandr
> Just expanding on the two comments you already received about this.
> We often see people miss that switchdev is about. It is not about
> writing switch drivers. It is about writing network stack
> accelerators. You take a feature of the Linux network stack and you
> accelerate it by offloading it to the hardware. So look around the
> network stack and see how you configure it to perform rate limiting of
> broadcast traffic ingress. Once you have found a suitable mechanism,
> accelerate it via offloading.
> If you find Linux has no way to perform a feature the hardware could
> accelerate, you first need to add a pure software version of that
> feature to the network stack, and then add acceleration support for
> it.
Hello Andrew, Ido, Nikolay,
I appreciate your time and comments provided over this patchset, though i have a few questions to ask, if you don't mind:
1. Does it mean that in order to support storm control in switchdev driver i need to implement software storm control in bridge driver,
and then using the switchdev attributes (notifiers) mechanism offload the configuration itself to the HW?
2. Is there any chance of keeping devlink solution untill the discussed (storm control implemented in the bridge driver) mechanism will be ready/implemented?
Anyway, it relies on the port param API from devlink which is already present in the kernel API.
On Fri, Jun 11, 2021 at 01:19:13PM +0000, Oleksandr Mazur wrote:
> >> On Wed, Jun 09, 2021 at 06:16:00PM +0300, Oleksandr Mazur wrote:
> > Storm control (BUM) provides a mechanism to limit rate of ingress
> > > port traffic (matched by type). Devlink port parameter API is used:
> > > driver registers a set of per-port parameters that can be accessed to both
> > > get/set per-port per-type rate limit.
> > > Add new FW command - RATE_LIMIT_MODE_SET.
>
> > Hi Oleksandr
>
> > Just expanding on the two comments you already received about this.
>
> > We often see people miss that switchdev is about. It is not about
> > writing switch drivers. It is about writing network stack
> > accelerators. You take a feature of the Linux network stack and you
> > accelerate it by offloading it to the hardware. So look around the
> > network stack and see how you configure it to perform rate limiting of
> > broadcast traffic ingress. Once you have found a suitable mechanism,
> > accelerate it via offloading.
>
> > If you find Linux has no way to perform a feature the hardware could
> > accelerate, you first need to add a pure software version of that
> > feature to the network stack, and then add acceleration support for
> > it.
>
>
> Hello Andrew, Ido, Nikolay,
> I appreciate your time and comments provided over this patchset, though i have a few questions to ask, if you don't mind:
>
> 1. Does it mean that in order to support storm control in switchdev
> driver i need to implement software storm control in bridge driver,
> and then using the switchdev attributes (notifiers) mechanism
> offload the configuration itself to the HW?
Hi Oleksandr
Not necessarily. Is storm control anything more than ingress packet
matching and rate limiting?
I'm not TC expert, but look for example at
https://man7.org/linux/man-pages/man8/tc-police.8.html
and the example:
# tc qdisc add dev eth0 handle ffff: ingress
# tc filter add dev eth0 parent ffff: u32 \
match u32 0 0 \
police rate 1mbit burst 100k
Replace the "match u32 0 0" with something which matches on broadcast
frames. Maybe "flower dst_mac ff:ff:ff:ff:ff:ff"
So there is a software solution. Now accelerate it.
> 2. Is there any chance of keeping devlink solution untill the
> discussed (storm control implemented in the bridge driver) mechanism
> will be ready/implemented?
No. Please do it correctly from the beginning. No hacks.
Andrew
On Fri, Jun 11, 2021 at 07:08:00PM +0200, Andrew Lunn wrote:
> On Fri, Jun 11, 2021 at 01:19:13PM +0000, Oleksandr Mazur wrote:
> > >> On Wed, Jun 09, 2021 at 06:16:00PM +0300, Oleksandr Mazur wrote:
> > > Storm control (BUM) provides a mechanism to limit rate of ingress
> > > > port traffic (matched by type). Devlink port parameter API is used:
> > > > driver registers a set of per-port parameters that can be accessed to both
> > > > get/set per-port per-type rate limit.
> > > > Add new FW command - RATE_LIMIT_MODE_SET.
> >
> > > Hi Oleksandr
> >
> > > Just expanding on the two comments you already received about this.
> >
> > > We often see people miss that switchdev is about. It is not about
> > > writing switch drivers. It is about writing network stack
> > > accelerators. You take a feature of the Linux network stack and you
> > > accelerate it by offloading it to the hardware. So look around the
> > > network stack and see how you configure it to perform rate limiting of
> > > broadcast traffic ingress. Once you have found a suitable mechanism,
> > > accelerate it via offloading.
> >
> > > If you find Linux has no way to perform a feature the hardware could
> > > accelerate, you first need to add a pure software version of that
> > > feature to the network stack, and then add acceleration support for
> > > it.
> >
> >
> > Hello Andrew, Ido, Nikolay,
> > I appreciate your time and comments provided over this patchset, though i have a few questions to ask, if you don't mind:
> >
>
> > 1. Does it mean that in order to support storm control in switchdev
> > driver i need to implement software storm control in bridge driver,
> > and then using the switchdev attributes (notifiers) mechanism
> > offload the configuration itself to the HW?
>
> Hi Oleksandr
>
> Not necessarily. Is storm control anything more than ingress packet
> matching and rate limiting?
>
> I'm not TC expert, but look for example at
> https://man7.org/linux/man-pages/man8/tc-police.8.html
>
> and the example:
>
> # tc qdisc add dev eth0 handle ffff: ingress
> # tc filter add dev eth0 parent ffff: u32 \
> match u32 0 0 \
> police rate 1mbit burst 100k
>
> Replace the "match u32 0 0" with something which matches on broadcast
> frames. Maybe "flower dst_mac ff:ff:ff:ff:ff:ff"
>
> So there is a software solution. Now accelerate it.
Storm control also needs the ability to limit other types of flooded
traffic such unknown unicast and unregistered multicast packets. The
entity which classifies packets as such is the bridge, which happens
after the ingress hook.
I see two options to support storm control in Linux:
1. By adding support in the bridge itself as a new bridge slave option.
Something like:
# ip link set dev swp1 type bridge_slave \
storm_control type { uuc | umc | bc} rate RATE mode { packet | byte }
I suspect this similar to more traditional implementations that users
might be used to and also maps nicely to hardware implementations
2. Teaching tc to call into the bridge to classify a packet. Not sure a
whole new classifier is needed for this. Maybe just extend flower with a
new key: dst_mac_type { uuc | umc }. I personally find this a bit weird,
but it is more flexible and allows to reuse existing actions
>
> > 2. Is there any chance of keeping devlink solution untill the
> > discussed (storm control implemented in the bridge driver) mechanism
> > will be ready/implemented?
>
> No. Please do it correctly from the beginning. No hacks.
+1
Hi Oleksandr,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on kselftest/next]
[also build test WARNING on net/master linus/master v5.13-rc6]
[cannot apply to net-next/master sparc-next/master next-20210615]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Oleksandr-Mazur/Marvell-Prestera-driver-implementation-of-devlink-functionality/20210616-112917
base: https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git next
config: arc-allyesconfig (attached as .config)
compiler: arceb-elf-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/d1e2db61850ee143f7aa180d34efb413f0956abd
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Oleksandr-Mazur/Marvell-Prestera-driver-implementation-of-devlink-functionality/20210616-112917
git checkout d1e2db61850ee143f7aa180d34efb413f0956abd
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arc
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>):
drivers/net/ethernet/marvell/prestera/prestera_devlink.c: In function 'prestera_devlink_port_param_set':
>> drivers/net/ethernet/marvell/prestera/prestera_devlink.c:693:26: warning: variable 'sw' set but not used [-Wunused-but-set-variable]
693 | struct prestera_switch *sw;
| ^~
drivers/net/ethernet/marvell/prestera/prestera_devlink.c: In function 'prestera_devlink_port_param_get':
drivers/net/ethernet/marvell/prestera/prestera_devlink.c:736:26: warning: variable 'sw' set but not used [-Wunused-but-set-variable]
736 | struct prestera_switch *sw;
| ^~
vim +/sw +693 drivers/net/ethernet/marvell/prestera/prestera_devlink.c
686
687 static int prestera_devlink_port_param_set(struct devlink_port *dl_port, u32 id,
688 struct devlink_param_gset_ctx *ctx)
689 {
690 struct prestera_strom_control_cfg *cfg;
691 u32 kbyte_per_sec_rate = ctx->val.vu32;
692 struct prestera_port *port;
> 693 struct prestera_switch *sw;
694 u32 *param_to_set;
695 u32 storm_type;
696 int ret;
697
698 port = container_of(dl_port, struct prestera_port, dl_port);
699 sw = devlink_priv(dl_port->devlink);
700 cfg = &port->storm_control;
701
702 switch (id) {
703 case PRESTERA_DEVLINK_PORT_PARAM_ID_BC_RATE:
704 param_to_set = &cfg->bc_kbyte_per_sec_rate;
705 storm_type = PRESTERA_PORT_STORM_CTL_TYPE_BC;
706 break;
707 case PRESTERA_DEVLINK_PORT_PARAM_ID_UC_UNK_RATE:
708 param_to_set = &cfg->unk_uc_kbyte_per_sec_rate;
709 storm_type = PRESTERA_PORT_STORM_CTL_TYPE_UC_UNK;
710 break;
711 case PRESTERA_DEVLINK_PORT_PARAM_ID_MC_RATE:
712 param_to_set = &cfg->unreg_mc_kbyte_per_sec_rate;
713 storm_type = PRESTERA_PORT_STORM_CTL_TYPE_MC;
714 break;
715 default:
716 return -EINVAL;
717 }
718
719 if (kbyte_per_sec_rate != *param_to_set) {
720 ret = prestera_hw_port_storm_control_cfg_set(port, storm_type,
721 kbyte_per_sec_rate);
722 if (ret)
723 return ret;
724
725 *param_to_set = kbyte_per_sec_rate;
726 }
727
728 return 0;
729 }
730
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
On Thu, Jun 17, 2021 at 05:30:07PM +0000, Oleksandr Mazur wrote:
> > Prestera Switchdev driver implements a set of devlink-based features,
> > that include both debug functionality (traps with trap statistics), as well
> > as functional rate limiter that is based on the devlink kernel API (interfaces).
>
> > The core prestera-devlink functionality is implemented in the prestera_devlink.c.
>
> > The patch series also extends the existing devlink kernel API with a list of core
> > features:
> > ?- devlink: add API for both publish/unpublish port parameters.
> > ?- devlink: add port parameters-specific ops, as current design makes it impossible
> > ? to register one parameter for multiple ports, and effectively distinguish for
> > ? what port parameter_op is called.
>
> As we discussed the storm control (BUM) done via devlink port params topic, and agreed that it shouldn't be done using the devlink API itself, there's an open question i'd like to address: the patch series included, for what i think, list of needed and benefitial changes, and those are the following patches:
Please wrap your emails at around 70 characters.
>
> > Oleksandr Mazur (2):
> ...
> > net: core: devlink: add port_params_ops for devlink port parameters altering
> > drivers: net: netdevsim: add devlink port params usage
>
> > Sudarsana Reddy Kalluru (1):
> > net: core: devlink: add apis to publish/unpublish port params
>
> So, should i create a new patch series that would include all of them?
>
> Because in that case the series itself would lack an actual HW usage of it. The only usage would be limited to the netdevsim driver.
We generally don't add APIs without a user. And in this case, i'm not
sure netdevsim is a valid user. Can you refactor some other driver to
make use of the new code? If not, i would suggest they are not merged
at the moment. When you do have a valid use case, you can post them
again.
Andrew
> Prestera Switchdev driver implements a set of devlink-based features,
> that include both debug functionality (traps with trap statistics), as well
> as functional rate limiter that is based on the devlink kernel API (interfaces).
> The core prestera-devlink functionality is implemented in the prestera_devlink.c.
> The patch series also extends the existing devlink kernel API with a list of core
> features:
> ?- devlink: add API for both publish/unpublish port parameters.
> ?- devlink: add port parameters-specific ops, as current design makes it impossible
> ? to register one parameter for multiple ports, and effectively distinguish for
> ? what port parameter_op is called.
As we discussed the storm control (BUM) done via devlink port params topic, and agreed that it shouldn't be done using the devlink API itself, there's an open question i'd like to address: the patch series included, for what i think, list of needed and benefitial changes, and those are the following patches:
> Oleksandr Mazur (2):
...
> net: core: devlink: add port_params_ops for devlink port parameters altering
> drivers: net: netdevsim: add devlink port params usage
> Sudarsana Reddy Kalluru (1):
> net: core: devlink: add apis to publish/unpublish port params
So, should i create a new patch series that would include all of them?
Because in that case the series itself would lack an actual HW usage of it. The only usage would be limited to the netdevsim driver.