* [PATCH net-next 00/11] mlxsw: Offload tc police action
@ 2020-07-15 8:27 Ido Schimmel
2020-07-15 8:27 ` [PATCH net-next 01/11] mlxsw: reg: Add policer bandwidth limits Ido Schimmel
` (11 more replies)
0 siblings, 12 replies; 17+ messages in thread
From: Ido Schimmel @ 2020-07-15 8:27 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, jiri, petrm, mlxsw, Ido Schimmel
From: Ido Schimmel <idosch@mellanox.com>
This patch set adds support for tc police action in mlxsw.
Patches #1-#2 add defines for policer bandwidth limits and resource
identifiers (e.g., maximum number of policers).
Patch #3 adds a common policer core in mlxsw. Currently it is only used
by the policy engine, but future patch sets will use it for trap
policers and storm control policers. The common core allows us to share
common logic between all policer types and abstract certain details from
the various users in mlxsw.
Patch #4 exposes the maximum number of supported policers and their
current usage to user space via devlink-resource. This provides better
visibility and also used for selftests purposes.
Patches #5-#7 gradually add support for tc police action in the policy
engine by calling into previously mentioned policer core.
Patch #8 adds a generic selftest for tc-police that can be used with
veth pairs or physical loopbacks.
Patches #9-#11 add mlxsw-specific selftests.
Ido Schimmel (11):
mlxsw: reg: Add policer bandwidth limits
mlxsw: resources: Add resource identifier for global policers
mlxsw: spectrum_policer: Add policer core
mlxsw: spectrum_policer: Add devlink resource support
mlxsw: core_acl_flex_actions: Work around hardware limitation
mlxsw: core_acl_flex_actions: Add police action
mlxsw: spectrum_acl: Offload FLOW_ACTION_POLICE
selftests: forwarding: Add tc-police tests
selftests: mlxsw: tc_restrictions: Test tc-police restrictions
selftests: mlxsw: Add scale test for tc-police
selftests: mlxsw: Test policers' occupancy
drivers/net/ethernet/mellanox/mlxsw/Makefile | 2 +-
.../mellanox/mlxsw/core_acl_flex_actions.c | 304 +++++++++++-
.../mellanox/mlxsw/core_acl_flex_actions.h | 8 +
drivers/net/ethernet/mellanox/mlxsw/reg.h | 9 +
.../net/ethernet/mellanox/mlxsw/resources.h | 2 +
.../net/ethernet/mellanox/mlxsw/spectrum.c | 20 +
.../net/ethernet/mellanox/mlxsw/spectrum.h | 46 +-
.../ethernet/mellanox/mlxsw/spectrum_acl.c | 33 +-
.../mlxsw/spectrum_acl_flex_actions.c | 27 +
.../ethernet/mellanox/mlxsw/spectrum_flower.c | 30 +-
.../mellanox/mlxsw/spectrum_policer.c | 468 ++++++++++++++++++
.../net/mlxsw/spectrum-2/resource_scale.sh | 2 +-
.../net/mlxsw/spectrum-2/tc_police_scale.sh | 16 +
.../net/mlxsw/spectrum/resource_scale.sh | 2 +-
.../net/mlxsw/spectrum/tc_police_scale.sh | 16 +
.../drivers/net/mlxsw/tc_police_occ.sh | 108 ++++
.../drivers/net/mlxsw/tc_police_scale.sh | 92 ++++
.../drivers/net/mlxsw/tc_restrictions.sh | 76 +++
.../selftests/net/forwarding/devlink_lib.sh | 5 +
.../selftests/net/forwarding/tc_police.sh | 333 +++++++++++++
20 files changed, 1575 insertions(+), 24 deletions(-)
create mode 100644 drivers/net/ethernet/mellanox/mlxsw/spectrum_policer.c
create mode 100644 tools/testing/selftests/drivers/net/mlxsw/spectrum-2/tc_police_scale.sh
create mode 100644 tools/testing/selftests/drivers/net/mlxsw/spectrum/tc_police_scale.sh
create mode 100755 tools/testing/selftests/drivers/net/mlxsw/tc_police_occ.sh
create mode 100644 tools/testing/selftests/drivers/net/mlxsw/tc_police_scale.sh
create mode 100755 tools/testing/selftests/net/forwarding/tc_police.sh
--
2.26.2
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH net-next 01/11] mlxsw: reg: Add policer bandwidth limits
2020-07-15 8:27 [PATCH net-next 00/11] mlxsw: Offload tc police action Ido Schimmel
@ 2020-07-15 8:27 ` Ido Schimmel
2020-07-15 8:27 ` [PATCH net-next 02/11] mlxsw: resources: Add resource identifier for global policers Ido Schimmel
` (10 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Ido Schimmel @ 2020-07-15 8:27 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, jiri, petrm, mlxsw, Ido Schimmel
From: Ido Schimmel <idosch@mellanox.com>
Add policer bandwidth limits for both rate and burst size so that they
could be enforced by a later patch.
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
Reviewed-by: Petr Machata <petrm@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/reg.h | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/reg.h b/drivers/net/ethernet/mellanox/mlxsw/reg.h
index 408003520602..3c5b25495751 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/reg.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/reg.h
@@ -3405,11 +3405,20 @@ MLXSW_ITEM32(reg, qpcr, violate_action, 0x18, 0, 4);
*/
MLXSW_ITEM64(reg, qpcr, violate_count, 0x20, 0, 64);
+/* Packets */
#define MLXSW_REG_QPCR_LOWEST_CIR 1
#define MLXSW_REG_QPCR_HIGHEST_CIR (2 * 1000 * 1000 * 1000) /* 2Gpps */
#define MLXSW_REG_QPCR_LOWEST_CBS 4
#define MLXSW_REG_QPCR_HIGHEST_CBS 24
+/* Bandwidth */
+#define MLXSW_REG_QPCR_LOWEST_CIR_BITS 1024 /* bps */
+#define MLXSW_REG_QPCR_HIGHEST_CIR_BITS 2000000000000ULL /* 2Tbps */
+#define MLXSW_REG_QPCR_LOWEST_CBS_BITS_SP1 4
+#define MLXSW_REG_QPCR_LOWEST_CBS_BITS_SP2 4
+#define MLXSW_REG_QPCR_HIGHEST_CBS_BITS_SP1 25
+#define MLXSW_REG_QPCR_HIGHEST_CBS_BITS_SP2 31
+
static inline void mlxsw_reg_qpcr_pack(char *payload, u16 pid,
enum mlxsw_reg_qpcr_ir_units ir_units,
bool bytes, u32 cir, u16 cbs)
--
2.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH net-next 02/11] mlxsw: resources: Add resource identifier for global policers
2020-07-15 8:27 [PATCH net-next 00/11] mlxsw: Offload tc police action Ido Schimmel
2020-07-15 8:27 ` [PATCH net-next 01/11] mlxsw: reg: Add policer bandwidth limits Ido Schimmel
@ 2020-07-15 8:27 ` Ido Schimmel
2020-07-15 8:27 ` [PATCH net-next 03/11] mlxsw: spectrum_policer: Add policer core Ido Schimmel
` (9 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Ido Schimmel @ 2020-07-15 8:27 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, jiri, petrm, mlxsw, Ido Schimmel
From: Ido Schimmel <idosch@mellanox.com>
Add a resource identifier for maximum global policers so that it could
be later used to query the information from firmware.
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
Reviewed-by: Petr Machata <petrm@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/resources.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/resources.h b/drivers/net/ethernet/mellanox/mlxsw/resources.h
index d62496ef299c..a56c9e19a390 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/resources.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/resources.h
@@ -47,6 +47,7 @@ enum mlxsw_res_id {
MLXSW_RES_ID_ACL_ERPT_ENTRIES_8KB,
MLXSW_RES_ID_ACL_ERPT_ENTRIES_12KB,
MLXSW_RES_ID_ACL_MAX_BF_LOG,
+ MLXSW_RES_ID_MAX_GLOBAL_POLICERS,
MLXSW_RES_ID_MAX_CPU_POLICERS,
MLXSW_RES_ID_MAX_VRS,
MLXSW_RES_ID_MAX_RIFS,
@@ -105,6 +106,7 @@ static u16 mlxsw_res_ids[] = {
[MLXSW_RES_ID_ACL_ERPT_ENTRIES_8KB] = 0x2952,
[MLXSW_RES_ID_ACL_ERPT_ENTRIES_12KB] = 0x2953,
[MLXSW_RES_ID_ACL_MAX_BF_LOG] = 0x2960,
+ [MLXSW_RES_ID_MAX_GLOBAL_POLICERS] = 0x2A10,
[MLXSW_RES_ID_MAX_CPU_POLICERS] = 0x2A13,
[MLXSW_RES_ID_MAX_VRS] = 0x2C01,
[MLXSW_RES_ID_MAX_RIFS] = 0x2C02,
--
2.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH net-next 03/11] mlxsw: spectrum_policer: Add policer core
2020-07-15 8:27 [PATCH net-next 00/11] mlxsw: Offload tc police action Ido Schimmel
2020-07-15 8:27 ` [PATCH net-next 01/11] mlxsw: reg: Add policer bandwidth limits Ido Schimmel
2020-07-15 8:27 ` [PATCH net-next 02/11] mlxsw: resources: Add resource identifier for global policers Ido Schimmel
@ 2020-07-15 8:27 ` Ido Schimmel
2020-08-17 15:38 ` Bjorn Helgaas
2020-07-15 8:27 ` [PATCH net-next 04/11] mlxsw: spectrum_policer: Add devlink resource support Ido Schimmel
` (8 subsequent siblings)
11 siblings, 1 reply; 17+ messages in thread
From: Ido Schimmel @ 2020-07-15 8:27 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, jiri, petrm, mlxsw, Ido Schimmel
From: Ido Schimmel <idosch@mellanox.com>
Add common code to handle all policer-related functionality in mlxsw.
Currently, only policer for policy engines are supported, but it in the
future more policer families will be added such as CPU (trap) policers
and storm control policers.
The API allows different modules to add / delete policers and read their
drop counter.
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
Reviewed-by: Petr Machata <petrm@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/Makefile | 2 +-
.../net/ethernet/mellanox/mlxsw/spectrum.c | 12 +
.../net/ethernet/mellanox/mlxsw/spectrum.h | 32 ++
.../mellanox/mlxsw/spectrum_policer.c | 403 ++++++++++++++++++
4 files changed, 448 insertions(+), 1 deletion(-)
create mode 100644 drivers/net/ethernet/mellanox/mlxsw/spectrum_policer.c
diff --git a/drivers/net/ethernet/mellanox/mlxsw/Makefile b/drivers/net/ethernet/mellanox/mlxsw/Makefile
index 3709983fbd77..892724380ea2 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/Makefile
+++ b/drivers/net/ethernet/mellanox/mlxsw/Makefile
@@ -31,7 +31,7 @@ mlxsw_spectrum-objs := spectrum.o spectrum_buffers.o \
spectrum_qdisc.o spectrum_span.o \
spectrum_nve.o spectrum_nve_vxlan.o \
spectrum_dpipe.o spectrum_trap.o \
- spectrum_ethtool.o
+ spectrum_ethtool.o spectrum_policer.o
mlxsw_spectrum-$(CONFIG_MLXSW_SPECTRUM_DCB) += spectrum_dcb.o
mlxsw_spectrum-$(CONFIG_PTP_1588_CLOCK) += spectrum_ptp.o
obj-$(CONFIG_MLXSW_MINIMAL) += mlxsw_minimal.o
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 4ac634bd3571..c6ab61818800 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -2860,6 +2860,12 @@ static int mlxsw_sp_init(struct mlxsw_core *mlxsw_core,
goto err_fids_init;
}
+ err = mlxsw_sp_policers_init(mlxsw_sp);
+ if (err) {
+ dev_err(mlxsw_sp->bus_info->dev, "Failed to initialize policers\n");
+ goto err_policers_init;
+ }
+
err = mlxsw_sp_traps_init(mlxsw_sp);
if (err) {
dev_err(mlxsw_sp->bus_info->dev, "Failed to set traps\n");
@@ -3019,6 +3025,8 @@ static int mlxsw_sp_init(struct mlxsw_core *mlxsw_core,
err_devlink_traps_init:
mlxsw_sp_traps_fini(mlxsw_sp);
err_traps_init:
+ mlxsw_sp_policers_fini(mlxsw_sp);
+err_policers_init:
mlxsw_sp_fids_fini(mlxsw_sp);
err_fids_init:
mlxsw_sp_kvdl_fini(mlxsw_sp);
@@ -3046,6 +3054,7 @@ static int mlxsw_sp1_init(struct mlxsw_core *mlxsw_core,
mlxsw_sp->port_type_speed_ops = &mlxsw_sp1_port_type_speed_ops;
mlxsw_sp->ptp_ops = &mlxsw_sp1_ptp_ops;
mlxsw_sp->span_ops = &mlxsw_sp1_span_ops;
+ mlxsw_sp->policer_core_ops = &mlxsw_sp1_policer_core_ops;
mlxsw_sp->listeners = mlxsw_sp1_listener;
mlxsw_sp->listeners_count = ARRAY_SIZE(mlxsw_sp1_listener);
mlxsw_sp->lowest_shaper_bs = MLXSW_REG_QEEC_LOWEST_SHAPER_BS_SP1;
@@ -3074,6 +3083,7 @@ static int mlxsw_sp2_init(struct mlxsw_core *mlxsw_core,
mlxsw_sp->port_type_speed_ops = &mlxsw_sp2_port_type_speed_ops;
mlxsw_sp->ptp_ops = &mlxsw_sp2_ptp_ops;
mlxsw_sp->span_ops = &mlxsw_sp2_span_ops;
+ mlxsw_sp->policer_core_ops = &mlxsw_sp2_policer_core_ops;
mlxsw_sp->lowest_shaper_bs = MLXSW_REG_QEEC_LOWEST_SHAPER_BS_SP2;
return mlxsw_sp_init(mlxsw_core, mlxsw_bus_info, extack);
@@ -3100,6 +3110,7 @@ static int mlxsw_sp3_init(struct mlxsw_core *mlxsw_core,
mlxsw_sp->port_type_speed_ops = &mlxsw_sp2_port_type_speed_ops;
mlxsw_sp->ptp_ops = &mlxsw_sp2_ptp_ops;
mlxsw_sp->span_ops = &mlxsw_sp3_span_ops;
+ mlxsw_sp->policer_core_ops = &mlxsw_sp2_policer_core_ops;
mlxsw_sp->lowest_shaper_bs = MLXSW_REG_QEEC_LOWEST_SHAPER_BS_SP3;
return mlxsw_sp_init(mlxsw_core, mlxsw_bus_info, extack);
@@ -3129,6 +3140,7 @@ static void mlxsw_sp_fini(struct mlxsw_core *mlxsw_core)
mlxsw_sp_buffers_fini(mlxsw_sp);
mlxsw_sp_devlink_traps_fini(mlxsw_sp);
mlxsw_sp_traps_fini(mlxsw_sp);
+ mlxsw_sp_policers_fini(mlxsw_sp);
mlxsw_sp_fids_fini(mlxsw_sp);
mlxsw_sp_kvdl_fini(mlxsw_sp);
}
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
index c00811178637..82227e87ef7c 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
@@ -151,6 +151,7 @@ struct mlxsw_sp {
struct mlxsw_afa *afa;
struct mlxsw_sp_acl *acl;
struct mlxsw_sp_fid_core *fid_core;
+ struct mlxsw_sp_policer_core *policer_core;
struct mlxsw_sp_kvdl *kvdl;
struct mlxsw_sp_nve *nve;
struct notifier_block netdevice_nb;
@@ -173,6 +174,7 @@ struct mlxsw_sp {
const struct mlxsw_sp_port_type_speed_ops *port_type_speed_ops;
const struct mlxsw_sp_ptp_ops *ptp_ops;
const struct mlxsw_sp_span_ops *span_ops;
+ const struct mlxsw_sp_policer_core_ops *policer_core_ops;
const struct mlxsw_listener *listeners;
size_t listeners_count;
u32 lowest_shaper_bs;
@@ -1196,4 +1198,34 @@ extern const struct ethtool_ops mlxsw_sp_port_ethtool_ops;
extern const struct mlxsw_sp_port_type_speed_ops mlxsw_sp1_port_type_speed_ops;
extern const struct mlxsw_sp_port_type_speed_ops mlxsw_sp2_port_type_speed_ops;
+/* spectrum_policer.c */
+extern const struct mlxsw_sp_policer_core_ops mlxsw_sp1_policer_core_ops;
+extern const struct mlxsw_sp_policer_core_ops mlxsw_sp2_policer_core_ops;
+
+enum mlxsw_sp_policer_type {
+ MLXSW_SP_POLICER_TYPE_SINGLE_RATE,
+
+ __MLXSW_SP_POLICER_TYPE_MAX,
+ MLXSW_SP_POLICER_TYPE_MAX = __MLXSW_SP_POLICER_TYPE_MAX - 1,
+};
+
+struct mlxsw_sp_policer_params {
+ u64 rate;
+ u64 burst;
+ bool bytes;
+};
+
+int mlxsw_sp_policer_add(struct mlxsw_sp *mlxsw_sp,
+ enum mlxsw_sp_policer_type type,
+ const struct mlxsw_sp_policer_params *params,
+ struct netlink_ext_ack *extack, u16 *p_policer_index);
+void mlxsw_sp_policer_del(struct mlxsw_sp *mlxsw_sp,
+ enum mlxsw_sp_policer_type type,
+ u16 policer_index);
+int mlxsw_sp_policer_drops_counter_get(struct mlxsw_sp *mlxsw_sp,
+ enum mlxsw_sp_policer_type type,
+ u16 policer_index, u64 *p_drops);
+int mlxsw_sp_policers_init(struct mlxsw_sp *mlxsw_sp);
+void mlxsw_sp_policers_fini(struct mlxsw_sp *mlxsw_sp);
+
#endif
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_policer.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_policer.c
new file mode 100644
index 000000000000..74766e936e0a
--- /dev/null
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_policer.c
@@ -0,0 +1,403 @@
+// SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0
+/* Copyright (c) 2020 Mellanox Technologies. All rights reserved */
+
+#include <linux/idr.h>
+#include <linux/log2.h>
+#include <linux/mutex.h>
+#include <linux/netlink.h>
+
+#include "spectrum.h"
+
+struct mlxsw_sp_policer_family {
+ enum mlxsw_sp_policer_type type;
+ enum mlxsw_reg_qpcr_g qpcr_type;
+ struct mlxsw_sp *mlxsw_sp;
+ u16 start_index; /* Inclusive */
+ u16 end_index; /* Exclusive */
+ struct idr policer_idr;
+ struct mutex lock; /* Protects policer_idr */
+ const struct mlxsw_sp_policer_family_ops *ops;
+};
+
+struct mlxsw_sp_policer {
+ struct mlxsw_sp_policer_params params;
+ u16 index;
+};
+
+struct mlxsw_sp_policer_family_ops {
+ int (*init)(struct mlxsw_sp_policer_family *family);
+ void (*fini)(struct mlxsw_sp_policer_family *family);
+ int (*policer_index_alloc)(struct mlxsw_sp_policer_family *family,
+ struct mlxsw_sp_policer *policer);
+ struct mlxsw_sp_policer * (*policer_index_free)(struct mlxsw_sp_policer_family *family,
+ u16 policer_index);
+ int (*policer_init)(struct mlxsw_sp_policer_family *family,
+ const struct mlxsw_sp_policer *policer);
+ int (*policer_params_check)(const struct mlxsw_sp_policer_family *family,
+ const struct mlxsw_sp_policer_params *params,
+ struct netlink_ext_ack *extack);
+};
+
+struct mlxsw_sp_policer_core {
+ struct mlxsw_sp_policer_family *family_arr[MLXSW_SP_POLICER_TYPE_MAX + 1];
+ const struct mlxsw_sp_policer_core_ops *ops;
+ u8 lowest_bs_bits;
+ u8 highest_bs_bits;
+};
+
+struct mlxsw_sp_policer_core_ops {
+ int (*init)(struct mlxsw_sp_policer_core *policer_core);
+};
+
+static u64 mlxsw_sp_policer_rate_bytes_ps_kbps(u64 rate_bytes_ps)
+{
+ return div_u64(rate_bytes_ps, 1000) * BITS_PER_BYTE;
+}
+
+static u8 mlxsw_sp_policer_burst_bytes_hw_units(u64 burst_bytes)
+{
+ /* Provided burst size is in bytes. The ASIC burst size value is
+ * (2 ^ bs) * 512 bits. Convert the provided size to 512-bit units.
+ */
+ u64 bs512 = div_u64(burst_bytes, 64);
+
+ if (!bs512)
+ return 0;
+
+ return fls64(bs512) - 1;
+}
+
+static int
+mlxsw_sp_policer_single_rate_family_init(struct mlxsw_sp_policer_family *family)
+{
+ struct mlxsw_core *core = family->mlxsw_sp->core;
+
+ /* CPU policers are allocated from the first N policers in the global
+ * range, so skip them.
+ */
+ if (!MLXSW_CORE_RES_VALID(core, MAX_GLOBAL_POLICERS) ||
+ !MLXSW_CORE_RES_VALID(core, MAX_CPU_POLICERS))
+ return -EIO;
+
+ family->start_index = MLXSW_CORE_RES_GET(core, MAX_CPU_POLICERS);
+ family->end_index = MLXSW_CORE_RES_GET(core, MAX_GLOBAL_POLICERS);
+
+ return 0;
+}
+
+static void
+mlxsw_sp_policer_single_rate_family_fini(struct mlxsw_sp_policer_family *family)
+{
+}
+
+static int
+mlxsw_sp_policer_single_rate_index_alloc(struct mlxsw_sp_policer_family *family,
+ struct mlxsw_sp_policer *policer)
+{
+ int id;
+
+ mutex_lock(&family->lock);
+ id = idr_alloc(&family->policer_idr, policer, family->start_index,
+ family->end_index, GFP_KERNEL);
+ mutex_unlock(&family->lock);
+
+ if (id < 0)
+ return id;
+
+ policer->index = id;
+
+ return 0;
+}
+
+static struct mlxsw_sp_policer *
+mlxsw_sp_policer_single_rate_index_free(struct mlxsw_sp_policer_family *family,
+ u16 policer_index)
+{
+ struct mlxsw_sp_policer *policer;
+
+ mutex_lock(&family->lock);
+ policer = idr_remove(&family->policer_idr, policer_index);
+ mutex_unlock(&family->lock);
+
+ WARN_ON(!policer);
+
+ return policer;
+}
+
+static int
+mlxsw_sp_policer_single_rate_init(struct mlxsw_sp_policer_family *family,
+ const struct mlxsw_sp_policer *policer)
+{
+ u64 rate_kbps = mlxsw_sp_policer_rate_bytes_ps_kbps(policer->params.rate);
+ u8 bs = mlxsw_sp_policer_burst_bytes_hw_units(policer->params.burst);
+ struct mlxsw_sp *mlxsw_sp = family->mlxsw_sp;
+ char qpcr_pl[MLXSW_REG_QPCR_LEN];
+
+ mlxsw_reg_qpcr_pack(qpcr_pl, policer->index, MLXSW_REG_QPCR_IR_UNITS_K,
+ true, rate_kbps, bs);
+ mlxsw_reg_qpcr_clear_counter_set(qpcr_pl, true);
+
+ return mlxsw_reg_write(mlxsw_sp->core, MLXSW_REG(qpcr), qpcr_pl);
+}
+
+static int
+mlxsw_sp_policer_single_rate_params_check(const struct mlxsw_sp_policer_family *family,
+ const struct mlxsw_sp_policer_params *params,
+ struct netlink_ext_ack *extack)
+{
+ struct mlxsw_sp_policer_core *policer_core = family->mlxsw_sp->policer_core;
+ u64 rate_bps = params->rate * BITS_PER_BYTE;
+ u8 bs;
+
+ if (!params->bytes) {
+ NL_SET_ERR_MSG_MOD(extack, "Only bandwidth policing is currently supported by single rate policers");
+ return -EINVAL;
+ }
+
+ if (!is_power_of_2(params->burst)) {
+ NL_SET_ERR_MSG_MOD(extack, "Policer burst size is not power of two");
+ return -EINVAL;
+ }
+
+ bs = mlxsw_sp_policer_burst_bytes_hw_units(params->burst);
+
+ if (bs < policer_core->lowest_bs_bits) {
+ NL_SET_ERR_MSG_MOD(extack, "Policer burst size lower than limit");
+ return -EINVAL;
+ }
+
+ if (bs > policer_core->highest_bs_bits) {
+ NL_SET_ERR_MSG_MOD(extack, "Policer burst size higher than limit");
+ return -EINVAL;
+ }
+
+ if (rate_bps < MLXSW_REG_QPCR_LOWEST_CIR_BITS) {
+ NL_SET_ERR_MSG_MOD(extack, "Policer rate lower than limit");
+ return -EINVAL;
+ }
+
+ if (rate_bps > MLXSW_REG_QPCR_HIGHEST_CIR_BITS) {
+ NL_SET_ERR_MSG_MOD(extack, "Policer rate higher than limit");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static const struct mlxsw_sp_policer_family_ops mlxsw_sp_policer_single_rate_ops = {
+ .init = mlxsw_sp_policer_single_rate_family_init,
+ .fini = mlxsw_sp_policer_single_rate_family_fini,
+ .policer_index_alloc = mlxsw_sp_policer_single_rate_index_alloc,
+ .policer_index_free = mlxsw_sp_policer_single_rate_index_free,
+ .policer_init = mlxsw_sp_policer_single_rate_init,
+ .policer_params_check = mlxsw_sp_policer_single_rate_params_check,
+};
+
+static const struct mlxsw_sp_policer_family mlxsw_sp_policer_single_rate_family = {
+ .type = MLXSW_SP_POLICER_TYPE_SINGLE_RATE,
+ .qpcr_type = MLXSW_REG_QPCR_G_GLOBAL,
+ .ops = &mlxsw_sp_policer_single_rate_ops,
+};
+
+static const struct mlxsw_sp_policer_family *mlxsw_sp_policer_family_arr[] = {
+ [MLXSW_SP_POLICER_TYPE_SINGLE_RATE] = &mlxsw_sp_policer_single_rate_family,
+};
+
+int mlxsw_sp_policer_add(struct mlxsw_sp *mlxsw_sp,
+ enum mlxsw_sp_policer_type type,
+ const struct mlxsw_sp_policer_params *params,
+ struct netlink_ext_ack *extack, u16 *p_policer_index)
+{
+ struct mlxsw_sp_policer_family *family;
+ struct mlxsw_sp_policer *policer;
+ int err;
+
+ family = mlxsw_sp->policer_core->family_arr[type];
+
+ err = family->ops->policer_params_check(family, params, extack);
+ if (err)
+ return err;
+
+ policer = kmalloc(sizeof(*policer), GFP_KERNEL);
+ if (!policer)
+ return -ENOMEM;
+ policer->params = *params;
+
+ err = family->ops->policer_index_alloc(family, policer);
+ if (err) {
+ NL_SET_ERR_MSG_MOD(extack, "Failed to allocate policer index");
+ goto err_policer_index_alloc;
+ }
+
+ err = family->ops->policer_init(family, policer);
+ if (err) {
+ NL_SET_ERR_MSG_MOD(extack, "Failed to initialize policer");
+ goto err_policer_init;
+ }
+
+ *p_policer_index = policer->index;
+
+ return 0;
+
+err_policer_init:
+ family->ops->policer_index_free(family, policer->index);
+err_policer_index_alloc:
+ kfree(policer);
+ return err;
+}
+
+void mlxsw_sp_policer_del(struct mlxsw_sp *mlxsw_sp,
+ enum mlxsw_sp_policer_type type, u16 policer_index)
+{
+ struct mlxsw_sp_policer_family *family;
+ struct mlxsw_sp_policer *policer;
+
+ family = mlxsw_sp->policer_core->family_arr[type];
+ policer = family->ops->policer_index_free(family, policer_index);
+ kfree(policer);
+}
+
+int mlxsw_sp_policer_drops_counter_get(struct mlxsw_sp *mlxsw_sp,
+ enum mlxsw_sp_policer_type type,
+ u16 policer_index, u64 *p_drops)
+{
+ struct mlxsw_sp_policer_family *family;
+ char qpcr_pl[MLXSW_REG_QPCR_LEN];
+ int err;
+
+ family = mlxsw_sp->policer_core->family_arr[type];
+
+ MLXSW_REG_ZERO(qpcr, qpcr_pl);
+ mlxsw_reg_qpcr_pid_set(qpcr_pl, policer_index);
+ mlxsw_reg_qpcr_g_set(qpcr_pl, family->qpcr_type);
+ err = mlxsw_reg_query(mlxsw_sp->core, MLXSW_REG(qpcr), qpcr_pl);
+ if (err)
+ return err;
+
+ *p_drops = mlxsw_reg_qpcr_violate_count_get(qpcr_pl);
+
+ return 0;
+}
+
+static int
+mlxsw_sp_policer_family_register(struct mlxsw_sp *mlxsw_sp,
+ const struct mlxsw_sp_policer_family *tmpl)
+{
+ struct mlxsw_sp_policer_family *family;
+ int err;
+
+ family = kmemdup(tmpl, sizeof(*family), GFP_KERNEL);
+ if (!family)
+ return -ENOMEM;
+
+ family->mlxsw_sp = mlxsw_sp;
+ idr_init(&family->policer_idr);
+ mutex_init(&family->lock);
+
+ err = family->ops->init(family);
+ if (err)
+ goto err_family_init;
+
+ if (WARN_ON(family->start_index >= family->end_index)) {
+ err = -EINVAL;
+ goto err_index_check;
+ }
+
+ mlxsw_sp->policer_core->family_arr[tmpl->type] = family;
+
+ return 0;
+
+err_index_check:
+ family->ops->fini(family);
+err_family_init:
+ mutex_destroy(&family->lock);
+ idr_destroy(&family->policer_idr);
+ kfree(family);
+ return err;
+}
+
+static void
+mlxsw_sp_policer_family_unregister(struct mlxsw_sp *mlxsw_sp,
+ struct mlxsw_sp_policer_family *family)
+{
+ family->ops->fini(family);
+ mutex_destroy(&family->lock);
+ WARN_ON(!idr_is_empty(&family->policer_idr));
+ idr_destroy(&family->policer_idr);
+ kfree(family);
+}
+
+int mlxsw_sp_policers_init(struct mlxsw_sp *mlxsw_sp)
+{
+ struct mlxsw_sp_policer_core *policer_core;
+ int i, err;
+
+ policer_core = kzalloc(sizeof(*policer_core), GFP_KERNEL);
+ if (!policer_core)
+ return -ENOMEM;
+ mlxsw_sp->policer_core = policer_core;
+ policer_core->ops = mlxsw_sp->policer_core_ops;
+
+ err = policer_core->ops->init(policer_core);
+ if (err)
+ goto err_init;
+
+ for (i = 0; i < MLXSW_SP_POLICER_TYPE_MAX + 1; i++) {
+ err = mlxsw_sp_policer_family_register(mlxsw_sp, mlxsw_sp_policer_family_arr[i]);
+ if (err)
+ goto err_family_register;
+ }
+
+ return 0;
+
+err_family_register:
+ for (i--; i >= 0; i--) {
+ struct mlxsw_sp_policer_family *family;
+
+ family = mlxsw_sp->policer_core->family_arr[i];
+ mlxsw_sp_policer_family_unregister(mlxsw_sp, family);
+ }
+err_init:
+ kfree(mlxsw_sp->policer_core);
+ return err;
+}
+
+void mlxsw_sp_policers_fini(struct mlxsw_sp *mlxsw_sp)
+{
+ int i;
+
+ for (i = MLXSW_SP_POLICER_TYPE_MAX; i >= 0; i--) {
+ struct mlxsw_sp_policer_family *family;
+
+ family = mlxsw_sp->policer_core->family_arr[i];
+ mlxsw_sp_policer_family_unregister(mlxsw_sp, family);
+ }
+
+ kfree(mlxsw_sp->policer_core);
+}
+
+static int
+mlxsw_sp1_policer_core_init(struct mlxsw_sp_policer_core *policer_core)
+{
+ policer_core->lowest_bs_bits = MLXSW_REG_QPCR_LOWEST_CBS_BITS_SP1;
+ policer_core->highest_bs_bits = MLXSW_REG_QPCR_HIGHEST_CBS_BITS_SP1;
+
+ return 0;
+}
+
+const struct mlxsw_sp_policer_core_ops mlxsw_sp1_policer_core_ops = {
+ .init = mlxsw_sp1_policer_core_init,
+};
+
+static int
+mlxsw_sp2_policer_core_init(struct mlxsw_sp_policer_core *policer_core)
+{
+ policer_core->lowest_bs_bits = MLXSW_REG_QPCR_LOWEST_CBS_BITS_SP2;
+ policer_core->highest_bs_bits = MLXSW_REG_QPCR_HIGHEST_CBS_BITS_SP2;
+
+ return 0;
+}
+
+const struct mlxsw_sp_policer_core_ops mlxsw_sp2_policer_core_ops = {
+ .init = mlxsw_sp2_policer_core_init,
+};
--
2.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH net-next 04/11] mlxsw: spectrum_policer: Add devlink resource support
2020-07-15 8:27 [PATCH net-next 00/11] mlxsw: Offload tc police action Ido Schimmel
` (2 preceding siblings ...)
2020-07-15 8:27 ` [PATCH net-next 03/11] mlxsw: spectrum_policer: Add policer core Ido Schimmel
@ 2020-07-15 8:27 ` Ido Schimmel
2020-07-15 8:27 ` [PATCH net-next 05/11] mlxsw: core_acl_flex_actions: Work around hardware limitation Ido Schimmel
` (7 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Ido Schimmel @ 2020-07-15 8:27 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, jiri, petrm, mlxsw, Ido Schimmel
From: Ido Schimmel <idosch@mellanox.com>
Expose via devlink-resource the maximum number of single-rate policers
and their current occupancy. Example:
$ devlink resource show pci/0000:01:00.0
...
name global_policers size 1000 unit entry dpipe_tables none
resources:
name single_rate_policers size 968 occ 0 unit entry dpipe_tables none
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
Reviewed-by: Petr Machata <petrm@mellanox.com>
---
.../net/ethernet/mellanox/mlxsw/spectrum.c | 8 +++
.../net/ethernet/mellanox/mlxsw/spectrum.h | 3 +
.../mellanox/mlxsw/spectrum_policer.c | 65 +++++++++++++++++++
3 files changed, 76 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index c6ab61818800..519eb44e4097 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -3352,6 +3352,10 @@ static int mlxsw_sp1_resources_register(struct mlxsw_core *mlxsw_core)
if (err)
goto err_resources_counter_register;
+ err = mlxsw_sp_policer_resources_register(mlxsw_core);
+ if (err)
+ goto err_resources_counter_register;
+
return 0;
err_resources_counter_register:
@@ -3376,6 +3380,10 @@ static int mlxsw_sp2_resources_register(struct mlxsw_core *mlxsw_core)
if (err)
goto err_resources_counter_register;
+ err = mlxsw_sp_policer_resources_register(mlxsw_core);
+ if (err)
+ goto err_resources_counter_register;
+
return 0;
err_resources_counter_register:
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
index 82227e87ef7c..defe1d82d83e 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
@@ -62,6 +62,8 @@ enum mlxsw_sp_resource_id {
MLXSW_SP_RESOURCE_COUNTERS,
MLXSW_SP_RESOURCE_COUNTERS_FLOW,
MLXSW_SP_RESOURCE_COUNTERS_RIF,
+ MLXSW_SP_RESOURCE_GLOBAL_POLICERS,
+ MLXSW_SP_RESOURCE_SINGLE_RATE_POLICERS,
};
struct mlxsw_sp_port;
@@ -1227,5 +1229,6 @@ int mlxsw_sp_policer_drops_counter_get(struct mlxsw_sp *mlxsw_sp,
u16 policer_index, u64 *p_drops);
int mlxsw_sp_policers_init(struct mlxsw_sp *mlxsw_sp);
void mlxsw_sp_policers_fini(struct mlxsw_sp *mlxsw_sp);
+int mlxsw_sp_policer_resources_register(struct mlxsw_core *mlxsw_core);
#endif
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_policer.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_policer.c
index 74766e936e0a..39052e5c12fd 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_policer.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_policer.c
@@ -5,6 +5,7 @@
#include <linux/log2.h>
#include <linux/mutex.h>
#include <linux/netlink.h>
+#include <net/devlink.h>
#include "spectrum.h"
@@ -16,6 +17,7 @@ struct mlxsw_sp_policer_family {
u16 end_index; /* Exclusive */
struct idr policer_idr;
struct mutex lock; /* Protects policer_idr */
+ atomic_t policers_count;
const struct mlxsw_sp_policer_family_ops *ops;
};
@@ -67,10 +69,18 @@ static u8 mlxsw_sp_policer_burst_bytes_hw_units(u64 burst_bytes)
return fls64(bs512) - 1;
}
+static u64 mlxsw_sp_policer_single_rate_occ_get(void *priv)
+{
+ struct mlxsw_sp_policer_family *family = priv;
+
+ return atomic_read(&family->policers_count);
+}
+
static int
mlxsw_sp_policer_single_rate_family_init(struct mlxsw_sp_policer_family *family)
{
struct mlxsw_core *core = family->mlxsw_sp->core;
+ struct devlink *devlink;
/* CPU policers are allocated from the first N policers in the global
* range, so skip them.
@@ -82,12 +92,24 @@ mlxsw_sp_policer_single_rate_family_init(struct mlxsw_sp_policer_family *family)
family->start_index = MLXSW_CORE_RES_GET(core, MAX_CPU_POLICERS);
family->end_index = MLXSW_CORE_RES_GET(core, MAX_GLOBAL_POLICERS);
+ atomic_set(&family->policers_count, 0);
+ devlink = priv_to_devlink(core);
+ devlink_resource_occ_get_register(devlink,
+ MLXSW_SP_RESOURCE_SINGLE_RATE_POLICERS,
+ mlxsw_sp_policer_single_rate_occ_get,
+ family);
+
return 0;
}
static void
mlxsw_sp_policer_single_rate_family_fini(struct mlxsw_sp_policer_family *family)
{
+ struct devlink *devlink = priv_to_devlink(family->mlxsw_sp->core);
+
+ devlink_resource_occ_get_unregister(devlink,
+ MLXSW_SP_RESOURCE_SINGLE_RATE_POLICERS);
+ WARN_ON(atomic_read(&family->policers_count) != 0);
}
static int
@@ -104,6 +126,7 @@ mlxsw_sp_policer_single_rate_index_alloc(struct mlxsw_sp_policer_family *family,
if (id < 0)
return id;
+ atomic_inc(&family->policers_count);
policer->index = id;
return 0;
@@ -115,6 +138,8 @@ mlxsw_sp_policer_single_rate_index_free(struct mlxsw_sp_policer_family *family,
{
struct mlxsw_sp_policer *policer;
+ atomic_dec(&family->policers_count);
+
mutex_lock(&family->lock);
policer = idr_remove(&family->policer_idr, policer_index);
mutex_unlock(&family->lock);
@@ -376,6 +401,46 @@ void mlxsw_sp_policers_fini(struct mlxsw_sp *mlxsw_sp)
kfree(mlxsw_sp->policer_core);
}
+int mlxsw_sp_policer_resources_register(struct mlxsw_core *mlxsw_core)
+{
+ u64 global_policers, cpu_policers, single_rate_policers;
+ struct devlink *devlink = priv_to_devlink(mlxsw_core);
+ struct devlink_resource_size_params size_params;
+ int err;
+
+ if (!MLXSW_CORE_RES_VALID(mlxsw_core, MAX_GLOBAL_POLICERS) ||
+ !MLXSW_CORE_RES_VALID(mlxsw_core, MAX_CPU_POLICERS))
+ return -EIO;
+
+ global_policers = MLXSW_CORE_RES_GET(mlxsw_core, MAX_GLOBAL_POLICERS);
+ cpu_policers = MLXSW_CORE_RES_GET(mlxsw_core, MAX_CPU_POLICERS);
+ single_rate_policers = global_policers - cpu_policers;
+
+ devlink_resource_size_params_init(&size_params, global_policers,
+ global_policers, 1,
+ DEVLINK_RESOURCE_UNIT_ENTRY);
+ err = devlink_resource_register(devlink, "global_policers",
+ global_policers,
+ MLXSW_SP_RESOURCE_GLOBAL_POLICERS,
+ DEVLINK_RESOURCE_ID_PARENT_TOP,
+ &size_params);
+ if (err)
+ return err;
+
+ devlink_resource_size_params_init(&size_params, single_rate_policers,
+ single_rate_policers, 1,
+ DEVLINK_RESOURCE_UNIT_ENTRY);
+ err = devlink_resource_register(devlink, "single_rate_policers",
+ single_rate_policers,
+ MLXSW_SP_RESOURCE_SINGLE_RATE_POLICERS,
+ MLXSW_SP_RESOURCE_GLOBAL_POLICERS,
+ &size_params);
+ if (err)
+ return err;
+
+ return 0;
+}
+
static int
mlxsw_sp1_policer_core_init(struct mlxsw_sp_policer_core *policer_core)
{
--
2.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH net-next 05/11] mlxsw: core_acl_flex_actions: Work around hardware limitation
2020-07-15 8:27 [PATCH net-next 00/11] mlxsw: Offload tc police action Ido Schimmel
` (3 preceding siblings ...)
2020-07-15 8:27 ` [PATCH net-next 04/11] mlxsw: spectrum_policer: Add devlink resource support Ido Schimmel
@ 2020-07-15 8:27 ` Ido Schimmel
2020-07-15 8:27 ` [PATCH net-next 06/11] mlxsw: core_acl_flex_actions: Add police action Ido Schimmel
` (6 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Ido Schimmel @ 2020-07-15 8:27 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, jiri, petrm, mlxsw, Ido Schimmel
From: Ido Schimmel <idosch@mellanox.com>
In the policy engine, each ACL rule points to an action block where the
ACL actions are stored. Each action block consists of one or more action
sets. Each action set holds one or more individual actions, up to a
maximum queried from the device. For example:
Action set #1 Action set #2
+----------+ +--------------+ +--------------+
| ACL rule +----------> Action #1 | +-----> Action #4 |
+----------+ +--------------+ | +--------------+
| Action #2 | | | Action #5 |
+--------------+ | +--------------+
| Action #3 +------+ | |
+--------------+ +--------------+
<---------+ Action block +----------------->
The hardware has a limitation that prevents a policing action
(MLXSW_AFA_POLCNT_CODE when used with a policer, not a counter) from
being configured in the same action set with a trap action (i.e.,
MLXSW_AFA_TRAP_CODE or MLXSW_AFA_TRAPWU_CODE). Note that the latter used
to implement multiple actions: 'trap', 'mirred', 'drop'.
Work around this limitation by teaching mlxsw_afa_block_append_action()
to create a new action set not only when there is no more room left in
the current set, but also when there is a conflict between previously
mentioned actions.
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
Reviewed-by: Petr Machata <petrm@mellanox.com>
---
.../mellanox/mlxsw/core_acl_flex_actions.c | 87 +++++++++++++++----
1 file changed, 71 insertions(+), 16 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 30a7d5afdec7..06a43913b9ce 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core_acl_flex_actions.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core_acl_flex_actions.c
@@ -88,9 +88,11 @@ struct mlxsw_afa_set {
struct rhash_head ht_node;
struct mlxsw_afa_set_ht_key ht_key;
u32 kvdl_index;
- bool shared; /* Inserted in hashtable (doesn't mean that
+ u8 shared:1, /* Inserted in hashtable (doesn't mean that
* kvdl_index is valid).
*/
+ has_trap:1,
+ has_police:1;
unsigned int ref_count;
struct mlxsw_afa_set *next; /* Pointer to the next set. */
struct mlxsw_afa_set *prev; /* Pointer to the previous set,
@@ -839,16 +841,38 @@ mlxsw_afa_cookie_ref_create(struct mlxsw_afa_block *block,
#define MLXSW_AFA_ONE_ACTION_LEN 32
#define MLXSW_AFA_PAYLOAD_OFFSET 4
-static char *mlxsw_afa_block_append_action(struct mlxsw_afa_block *block,
- u8 action_code, u8 action_size)
+enum mlxsw_afa_action_type {
+ MLXSW_AFA_ACTION_TYPE_TRAP,
+ MLXSW_AFA_ACTION_TYPE_POLICE,
+ MLXSW_AFA_ACTION_TYPE_OTHER,
+};
+
+static bool
+mlxsw_afa_block_need_split(const struct mlxsw_afa_block *block,
+ enum mlxsw_afa_action_type type)
+{
+ struct mlxsw_afa_set *cur_set = block->cur_set;
+
+ /* Due to a hardware limitation, police action cannot be in the same
+ * action set with MLXSW_AFA_TRAP_CODE or MLXSW_AFA_TRAPWU_CODE
+ * actions. Work around this limitation by creating a new action set
+ * and place the new action there.
+ */
+ return (cur_set->has_trap && type == MLXSW_AFA_ACTION_TYPE_POLICE) ||
+ (cur_set->has_police && type == MLXSW_AFA_ACTION_TYPE_TRAP);
+}
+
+static char *mlxsw_afa_block_append_action_ext(struct mlxsw_afa_block *block,
+ u8 action_code, u8 action_size,
+ enum mlxsw_afa_action_type type)
{
char *oneact;
char *actions;
if (block->finished)
return ERR_PTR(-EINVAL);
- if (block->cur_act_index + action_size >
- block->afa->max_acts_per_set) {
+ if (block->cur_act_index + action_size > block->afa->max_acts_per_set ||
+ mlxsw_afa_block_need_split(block, type)) {
struct mlxsw_afa_set *set;
/* The appended action won't fit into the current action set,
@@ -863,6 +887,17 @@ static char *mlxsw_afa_block_append_action(struct mlxsw_afa_block *block,
block->cur_set = set;
}
+ switch (type) {
+ case MLXSW_AFA_ACTION_TYPE_TRAP:
+ block->cur_set->has_trap = true;
+ break;
+ case MLXSW_AFA_ACTION_TYPE_POLICE:
+ block->cur_set->has_police = true;
+ break;
+ default:
+ break;
+ }
+
actions = block->cur_set->ht_key.enc_actions;
oneact = actions + block->cur_act_index * MLXSW_AFA_ONE_ACTION_LEN;
block->cur_act_index += action_size;
@@ -870,6 +905,14 @@ static char *mlxsw_afa_block_append_action(struct mlxsw_afa_block *block,
return oneact + MLXSW_AFA_PAYLOAD_OFFSET;
}
+static char *mlxsw_afa_block_append_action(struct mlxsw_afa_block *block,
+ u8 action_code, u8 action_size)
+{
+ return mlxsw_afa_block_append_action_ext(block, action_code,
+ action_size,
+ MLXSW_AFA_ACTION_TYPE_OTHER);
+}
+
/* VLAN Action
* -----------
* VLAN action is used for manipulating VLANs. It can be used to implement QinQ,
@@ -1048,11 +1091,20 @@ mlxsw_afa_trap_mirror_pack(char *payload, bool mirror_enable,
mlxsw_afa_trap_mirror_agent_set(payload, mirror_agent);
}
+static char *mlxsw_afa_block_append_action_trap(struct mlxsw_afa_block *block,
+ u8 action_code, u8 action_size)
+{
+ return mlxsw_afa_block_append_action_ext(block, action_code,
+ action_size,
+ MLXSW_AFA_ACTION_TYPE_TRAP);
+}
+
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);
+ char *act = mlxsw_afa_block_append_action_trap(block,
+ MLXSW_AFA_TRAP_CODE,
+ MLXSW_AFA_TRAP_SIZE);
if (IS_ERR(act))
return PTR_ERR(act);
@@ -1081,8 +1133,8 @@ mlxsw_afa_block_append_drop_with_cookie(struct mlxsw_afa_block *block,
}
cookie_index = cookie_ref->cookie->cookie_index;
- act = mlxsw_afa_block_append_action(block, MLXSW_AFA_TRAPWU_CODE,
- MLXSW_AFA_TRAPWU_SIZE);
+ act = mlxsw_afa_block_append_action_trap(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);
@@ -1113,8 +1165,9 @@ EXPORT_SYMBOL(mlxsw_afa_block_append_drop);
int mlxsw_afa_block_append_trap(struct mlxsw_afa_block *block, u16 trap_id)
{
- char *act = mlxsw_afa_block_append_action(block, MLXSW_AFA_TRAP_CODE,
- MLXSW_AFA_TRAP_SIZE);
+ char *act = mlxsw_afa_block_append_action_trap(block,
+ MLXSW_AFA_TRAP_CODE,
+ MLXSW_AFA_TRAP_SIZE);
if (IS_ERR(act))
return PTR_ERR(act);
@@ -1127,8 +1180,9 @@ EXPORT_SYMBOL(mlxsw_afa_block_append_trap);
int mlxsw_afa_block_append_trap_and_forward(struct mlxsw_afa_block *block,
u16 trap_id)
{
- char *act = mlxsw_afa_block_append_action(block, MLXSW_AFA_TRAP_CODE,
- MLXSW_AFA_TRAP_SIZE);
+ char *act = mlxsw_afa_block_append_action_trap(block,
+ MLXSW_AFA_TRAP_CODE,
+ MLXSW_AFA_TRAP_SIZE);
if (IS_ERR(act))
return PTR_ERR(act);
@@ -1199,9 +1253,10 @@ static int
mlxsw_afa_block_append_allocated_mirror(struct mlxsw_afa_block *block,
u8 mirror_agent)
{
- char *act = mlxsw_afa_block_append_action(block,
- MLXSW_AFA_TRAP_CODE,
- MLXSW_AFA_TRAP_SIZE);
+ char *act = mlxsw_afa_block_append_action_trap(block,
+ MLXSW_AFA_TRAP_CODE,
+ MLXSW_AFA_TRAP_SIZE);
+
if (IS_ERR(act))
return PTR_ERR(act);
mlxsw_afa_trap_pack(act, MLXSW_AFA_TRAP_TRAP_ACTION_NOP,
--
2.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH net-next 06/11] mlxsw: core_acl_flex_actions: Add police action
2020-07-15 8:27 [PATCH net-next 00/11] mlxsw: Offload tc police action Ido Schimmel
` (4 preceding siblings ...)
2020-07-15 8:27 ` [PATCH net-next 05/11] mlxsw: core_acl_flex_actions: Work around hardware limitation Ido Schimmel
@ 2020-07-15 8:27 ` Ido Schimmel
2020-07-15 8:27 ` [PATCH net-next 07/11] mlxsw: spectrum_acl: Offload FLOW_ACTION_POLICE Ido Schimmel
` (5 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Ido Schimmel @ 2020-07-15 8:27 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, jiri, petrm, mlxsw, Ido Schimmel
From: Ido Schimmel <idosch@mellanox.com>
Add core functionality required to support police action in the policy
engine.
The utilized hardware policers are stored in a hash table keyed by the
flow action index. This allows to support policer sharing between
multiple ACL rules.
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
Reviewed-by: Petr Machata <petrm@mellanox.com>
---
.../mellanox/mlxsw/core_acl_flex_actions.c | 217 ++++++++++++++++++
.../mellanox/mlxsw/core_acl_flex_actions.h | 8 +
2 files changed, 225 insertions(+)
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 06a43913b9ce..4d699fe98cb6 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core_acl_flex_actions.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core_acl_flex_actions.c
@@ -67,7 +67,9 @@ struct mlxsw_afa {
struct rhashtable set_ht;
struct rhashtable fwd_entry_ht;
struct rhashtable cookie_ht;
+ struct rhashtable policer_ht;
struct idr cookie_idr;
+ struct list_head policer_list;
};
#define MLXSW_AFA_SET_LEN 0xA8
@@ -177,6 +179,21 @@ static const struct rhashtable_params mlxsw_afa_cookie_ht_params = {
.automatic_shrinking = true,
};
+struct mlxsw_afa_policer {
+ struct rhash_head ht_node;
+ struct list_head list; /* Member of policer_list */
+ refcount_t ref_count;
+ u32 fa_index;
+ u16 policer_index;
+};
+
+static const struct rhashtable_params mlxsw_afa_policer_ht_params = {
+ .key_len = sizeof(u32),
+ .key_offset = offsetof(struct mlxsw_afa_policer, fa_index),
+ .head_offset = offsetof(struct mlxsw_afa_policer, ht_node),
+ .automatic_shrinking = true,
+};
+
struct mlxsw_afa *mlxsw_afa_create(unsigned int max_acts_per_set,
const struct mlxsw_afa_ops *ops,
void *ops_priv)
@@ -198,12 +215,19 @@ struct mlxsw_afa *mlxsw_afa_create(unsigned int max_acts_per_set,
&mlxsw_afa_cookie_ht_params);
if (err)
goto err_cookie_rhashtable_init;
+ err = rhashtable_init(&mlxsw_afa->policer_ht,
+ &mlxsw_afa_policer_ht_params);
+ if (err)
+ goto err_policer_rhashtable_init;
idr_init(&mlxsw_afa->cookie_idr);
+ INIT_LIST_HEAD(&mlxsw_afa->policer_list);
mlxsw_afa->max_acts_per_set = max_acts_per_set;
mlxsw_afa->ops = ops;
mlxsw_afa->ops_priv = ops_priv;
return mlxsw_afa;
+err_policer_rhashtable_init:
+ rhashtable_destroy(&mlxsw_afa->cookie_ht);
err_cookie_rhashtable_init:
rhashtable_destroy(&mlxsw_afa->fwd_entry_ht);
err_fwd_entry_rhashtable_init:
@@ -216,8 +240,10 @@ EXPORT_SYMBOL(mlxsw_afa_create);
void mlxsw_afa_destroy(struct mlxsw_afa *mlxsw_afa)
{
+ WARN_ON(!list_empty(&mlxsw_afa->policer_list));
WARN_ON(!idr_is_empty(&mlxsw_afa->cookie_idr));
idr_destroy(&mlxsw_afa->cookie_idr);
+ rhashtable_destroy(&mlxsw_afa->policer_ht);
rhashtable_destroy(&mlxsw_afa->cookie_ht);
rhashtable_destroy(&mlxsw_afa->fwd_entry_ht);
rhashtable_destroy(&mlxsw_afa->set_ht);
@@ -838,6 +864,137 @@ mlxsw_afa_cookie_ref_create(struct mlxsw_afa_block *block,
return ERR_PTR(err);
}
+static struct mlxsw_afa_policer *
+mlxsw_afa_policer_create(struct mlxsw_afa *mlxsw_afa, u32 fa_index,
+ u64 rate_bytes_ps, u32 burst,
+ struct netlink_ext_ack *extack)
+{
+ struct mlxsw_afa_policer *policer;
+ int err;
+
+ policer = kzalloc(sizeof(*policer), GFP_KERNEL);
+ if (!policer)
+ return ERR_PTR(-ENOMEM);
+
+ err = mlxsw_afa->ops->policer_add(mlxsw_afa->ops_priv, rate_bytes_ps,
+ burst, &policer->policer_index,
+ extack);
+ if (err)
+ goto err_policer_add;
+
+ refcount_set(&policer->ref_count, 1);
+ policer->fa_index = fa_index;
+
+ err = rhashtable_insert_fast(&mlxsw_afa->policer_ht, &policer->ht_node,
+ mlxsw_afa_policer_ht_params);
+ if (err)
+ goto err_rhashtable_insert;
+
+ list_add_tail(&policer->list, &mlxsw_afa->policer_list);
+
+ return policer;
+
+err_rhashtable_insert:
+ mlxsw_afa->ops->policer_del(mlxsw_afa->ops_priv,
+ policer->policer_index);
+err_policer_add:
+ kfree(policer);
+ return ERR_PTR(err);
+}
+
+static void mlxsw_afa_policer_destroy(struct mlxsw_afa *mlxsw_afa,
+ struct mlxsw_afa_policer *policer)
+{
+ list_del(&policer->list);
+ rhashtable_remove_fast(&mlxsw_afa->policer_ht, &policer->ht_node,
+ mlxsw_afa_policer_ht_params);
+ mlxsw_afa->ops->policer_del(mlxsw_afa->ops_priv,
+ policer->policer_index);
+ kfree(policer);
+}
+
+static struct mlxsw_afa_policer *
+mlxsw_afa_policer_get(struct mlxsw_afa *mlxsw_afa, u32 fa_index,
+ u64 rate_bytes_ps, u32 burst,
+ struct netlink_ext_ack *extack)
+{
+ struct mlxsw_afa_policer *policer;
+
+ policer = rhashtable_lookup_fast(&mlxsw_afa->policer_ht, &fa_index,
+ mlxsw_afa_policer_ht_params);
+ if (policer) {
+ refcount_inc(&policer->ref_count);
+ return policer;
+ }
+
+ return mlxsw_afa_policer_create(mlxsw_afa, fa_index, rate_bytes_ps,
+ burst, extack);
+}
+
+static void mlxsw_afa_policer_put(struct mlxsw_afa *mlxsw_afa,
+ struct mlxsw_afa_policer *policer)
+{
+ if (!refcount_dec_and_test(&policer->ref_count))
+ return;
+ mlxsw_afa_policer_destroy(mlxsw_afa, policer);
+}
+
+struct mlxsw_afa_policer_ref {
+ struct mlxsw_afa_resource resource;
+ struct mlxsw_afa_policer *policer;
+};
+
+static void
+mlxsw_afa_policer_ref_destroy(struct mlxsw_afa_block *block,
+ struct mlxsw_afa_policer_ref *policer_ref)
+{
+ mlxsw_afa_resource_del(&policer_ref->resource);
+ mlxsw_afa_policer_put(block->afa, policer_ref->policer);
+ kfree(policer_ref);
+}
+
+static void
+mlxsw_afa_policer_ref_destructor(struct mlxsw_afa_block *block,
+ struct mlxsw_afa_resource *resource)
+{
+ struct mlxsw_afa_policer_ref *policer_ref;
+
+ policer_ref = container_of(resource, struct mlxsw_afa_policer_ref,
+ resource);
+ mlxsw_afa_policer_ref_destroy(block, policer_ref);
+}
+
+static struct mlxsw_afa_policer_ref *
+mlxsw_afa_policer_ref_create(struct mlxsw_afa_block *block, u32 fa_index,
+ u64 rate_bytes_ps, u32 burst,
+ struct netlink_ext_ack *extack)
+{
+ struct mlxsw_afa_policer_ref *policer_ref;
+ struct mlxsw_afa_policer *policer;
+ int err;
+
+ policer_ref = kzalloc(sizeof(*policer_ref), GFP_KERNEL);
+ if (!policer_ref)
+ return ERR_PTR(-ENOMEM);
+
+ policer = mlxsw_afa_policer_get(block->afa, fa_index, rate_bytes_ps,
+ burst, extack);
+ if (IS_ERR(policer)) {
+ err = PTR_ERR(policer);
+ goto err_policer_get;
+ }
+
+ policer_ref->policer = policer;
+ policer_ref->resource.destructor = mlxsw_afa_policer_ref_destructor;
+ mlxsw_afa_resource_add(block, &policer_ref->resource);
+
+ return policer_ref;
+
+err_policer_get:
+ kfree(policer_ref);
+ return ERR_PTR(err);
+}
+
#define MLXSW_AFA_ONE_ACTION_LEN 32
#define MLXSW_AFA_PAYLOAD_OFFSET 4
@@ -1551,6 +1708,19 @@ EXPORT_SYMBOL(mlxsw_afa_block_append_fwd);
#define MLXSW_AFA_POLCNT_CODE 0x08
#define MLXSW_AFA_POLCNT_SIZE 1
+enum {
+ MLXSW_AFA_POLCNT_COUNTER,
+ MLXSW_AFA_POLCNT_POLICER,
+};
+
+/* afa_polcnt_c_p
+ * Counter or policer.
+ * Indicates whether the action binds a policer or a counter to the flow.
+ * 0: Counter
+ * 1: Policer
+ */
+MLXSW_ITEM32(afa, polcnt, c_p, 0x00, 31, 1);
+
enum mlxsw_afa_polcnt_counter_set_type {
/* No count */
MLXSW_AFA_POLCNT_COUNTER_SET_TYPE_NO_COUNT = 0x00,
@@ -1570,15 +1740,28 @@ MLXSW_ITEM32(afa, polcnt, counter_set_type, 0x04, 24, 8);
*/
MLXSW_ITEM32(afa, polcnt, counter_index, 0x04, 0, 24);
+/* afa_polcnt_pid
+ * Policer ID.
+ * Reserved when c_p = 0
+ */
+MLXSW_ITEM32(afa, polcnt, pid, 0x08, 0, 14);
+
static inline void
mlxsw_afa_polcnt_pack(char *payload,
enum mlxsw_afa_polcnt_counter_set_type set_type,
u32 counter_index)
{
+ mlxsw_afa_polcnt_c_p_set(payload, MLXSW_AFA_POLCNT_COUNTER);
mlxsw_afa_polcnt_counter_set_type_set(payload, set_type);
mlxsw_afa_polcnt_counter_index_set(payload, counter_index);
}
+static void mlxsw_afa_polcnt_policer_pack(char *payload, u16 policer_index)
+{
+ mlxsw_afa_polcnt_c_p_set(payload, MLXSW_AFA_POLCNT_POLICER);
+ mlxsw_afa_polcnt_pid_set(payload, policer_index);
+}
+
int mlxsw_afa_block_append_allocated_counter(struct mlxsw_afa_block *block,
u32 counter_index)
{
@@ -1622,6 +1805,40 @@ int mlxsw_afa_block_append_counter(struct mlxsw_afa_block *block,
}
EXPORT_SYMBOL(mlxsw_afa_block_append_counter);
+int mlxsw_afa_block_append_police(struct mlxsw_afa_block *block,
+ u32 fa_index, u64 rate_bytes_ps, u32 burst,
+ u16 *p_policer_index,
+ struct netlink_ext_ack *extack)
+{
+ struct mlxsw_afa_policer_ref *policer_ref;
+ char *act;
+ int err;
+
+ policer_ref = mlxsw_afa_policer_ref_create(block, fa_index,
+ rate_bytes_ps,
+ burst, extack);
+ if (IS_ERR(policer_ref))
+ return PTR_ERR(policer_ref);
+ *p_policer_index = policer_ref->policer->policer_index;
+
+ act = mlxsw_afa_block_append_action_ext(block, MLXSW_AFA_POLCNT_CODE,
+ MLXSW_AFA_POLCNT_SIZE,
+ MLXSW_AFA_ACTION_TYPE_POLICE);
+ if (IS_ERR(act)) {
+ NL_SET_ERR_MSG_MOD(extack, "Cannot append police action");
+ err = PTR_ERR(act);
+ goto err_append_action;
+ }
+ mlxsw_afa_polcnt_policer_pack(act, *p_policer_index);
+
+ return 0;
+
+err_append_action:
+ mlxsw_afa_policer_ref_destroy(block, policer_ref);
+ return err;
+}
+EXPORT_SYMBOL(mlxsw_afa_block_append_police);
+
/* Virtual Router and Forwarding Domain Action
* -------------------------------------------
* Virtual Switch action is used for manipulate the Virtual Router (VR),
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 a72350399bcf..b652497b1002 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core_acl_flex_actions.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/core_acl_flex_actions.h
@@ -26,6 +26,10 @@ struct mlxsw_afa_ops {
bool ingress, int *p_span_id);
void (*mirror_del)(void *priv, u8 local_in_port, int span_id,
bool ingress);
+ int (*policer_add)(void *priv, u64 rate_bytes_ps, u32 burst,
+ u16 *p_policer_index,
+ struct netlink_ext_ack *extack);
+ void (*policer_del)(void *priv, u16 policer_index);
bool dummy_first_set;
};
@@ -84,5 +88,9 @@ int mlxsw_afa_block_append_mcrouter(struct mlxsw_afa_block *block,
bool rmid_valid, u32 kvdl_index);
int mlxsw_afa_block_append_l4port(struct mlxsw_afa_block *block, bool is_dport, u16 l4_port,
struct netlink_ext_ack *extack);
+int mlxsw_afa_block_append_police(struct mlxsw_afa_block *block,
+ u32 fa_index, u64 rate_bytes_ps, u32 burst,
+ u16 *p_policer_index,
+ struct netlink_ext_ack *extack);
#endif
--
2.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH net-next 07/11] mlxsw: spectrum_acl: Offload FLOW_ACTION_POLICE
2020-07-15 8:27 [PATCH net-next 00/11] mlxsw: Offload tc police action Ido Schimmel
` (5 preceding siblings ...)
2020-07-15 8:27 ` [PATCH net-next 06/11] mlxsw: core_acl_flex_actions: Add police action Ido Schimmel
@ 2020-07-15 8:27 ` Ido Schimmel
2020-07-15 8:27 ` [PATCH net-next 08/11] selftests: forwarding: Add tc-police tests Ido Schimmel
` (4 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Ido Schimmel @ 2020-07-15 8:27 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, jiri, petrm, mlxsw, Ido Schimmel
From: Ido Schimmel <idosch@mellanox.com>
Offload action police when used with a flower classifier. The number of
dropped packets is read from the policer and reported to tc.
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
Reviewed-by: Petr Machata <petrm@mellanox.com>
---
.../net/ethernet/mellanox/mlxsw/spectrum.h | 11 +++++--
.../ethernet/mellanox/mlxsw/spectrum_acl.c | 33 ++++++++++++++++++-
.../mlxsw/spectrum_acl_flex_actions.c | 27 +++++++++++++++
.../ethernet/mellanox/mlxsw/spectrum_flower.c | 30 +++++++++++++++--
4 files changed, 96 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
index defe1d82d83e..6ab1b6d725af 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
@@ -689,8 +689,10 @@ struct mlxsw_sp_acl_rule_info {
u8 action_created:1,
ingress_bind_blocker:1,
egress_bind_blocker:1,
- counter_valid:1;
+ counter_valid:1,
+ policer_index_valid:1;
unsigned int counter_index;
+ u16 policer_index;
};
/* spectrum_flow.c */
@@ -851,6 +853,10 @@ int mlxsw_sp_acl_rulei_act_mangle(struct mlxsw_sp *mlxsw_sp,
enum flow_action_mangle_base htype,
u32 offset, u32 mask, u32 val,
struct netlink_ext_ack *extack);
+int mlxsw_sp_acl_rulei_act_police(struct mlxsw_sp *mlxsw_sp,
+ struct mlxsw_sp_acl_rule_info *rulei,
+ u32 index, u64 rate_bytes_ps,
+ u32 burst, struct netlink_ext_ack *extack);
int mlxsw_sp_acl_rulei_act_count(struct mlxsw_sp *mlxsw_sp,
struct mlxsw_sp_acl_rule_info *rulei,
struct netlink_ext_ack *extack);
@@ -883,7 +889,8 @@ struct mlxsw_sp_acl_rule_info *
mlxsw_sp_acl_rule_rulei(struct mlxsw_sp_acl_rule *rule);
int mlxsw_sp_acl_rule_get_stats(struct mlxsw_sp *mlxsw_sp,
struct mlxsw_sp_acl_rule *rule,
- u64 *packets, u64 *bytes, u64 *last_use,
+ u64 *packets, u64 *bytes, u64 *drops,
+ u64 *last_use,
enum flow_action_hw_stats *used_hw_stats);
struct mlxsw_sp_fid *mlxsw_sp_acl_dummy_fid(struct mlxsw_sp *mlxsw_sp);
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c
index a671156a1428..8cfa03a75374 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c
@@ -66,6 +66,7 @@ struct mlxsw_sp_acl_rule {
u64 last_used;
u64 last_packets;
u64 last_bytes;
+ u64 last_drops;
unsigned long priv[];
/* priv has to be always the last item */
};
@@ -648,6 +649,24 @@ int mlxsw_sp_acl_rulei_act_mangle(struct mlxsw_sp *mlxsw_sp,
return -EINVAL;
}
+int mlxsw_sp_acl_rulei_act_police(struct mlxsw_sp *mlxsw_sp,
+ struct mlxsw_sp_acl_rule_info *rulei,
+ u32 index, u64 rate_bytes_ps,
+ u32 burst, struct netlink_ext_ack *extack)
+{
+ int err;
+
+ err = mlxsw_afa_block_append_police(rulei->act_block, index,
+ rate_bytes_ps, burst,
+ &rulei->policer_index, extack);
+ if (err)
+ return err;
+
+ rulei->policer_index_valid = true;
+
+ return 0;
+}
+
int mlxsw_sp_acl_rulei_act_count(struct mlxsw_sp *mlxsw_sp,
struct mlxsw_sp_acl_rule_info *rulei,
struct netlink_ext_ack *extack)
@@ -868,13 +887,16 @@ static void mlxsw_sp_acl_rule_activity_update_work(struct work_struct *work)
int mlxsw_sp_acl_rule_get_stats(struct mlxsw_sp *mlxsw_sp,
struct mlxsw_sp_acl_rule *rule,
- u64 *packets, u64 *bytes, u64 *last_use,
+ u64 *packets, u64 *bytes, u64 *drops,
+ u64 *last_use,
enum flow_action_hw_stats *used_hw_stats)
{
+ enum mlxsw_sp_policer_type type = MLXSW_SP_POLICER_TYPE_SINGLE_RATE;
struct mlxsw_sp_acl_rule_info *rulei;
u64 current_packets = 0;
u64 current_bytes = 0;
+ u64 current_drops = 0;
int err;
rulei = mlxsw_sp_acl_rule_rulei(rule);
@@ -886,12 +908,21 @@ int mlxsw_sp_acl_rule_get_stats(struct mlxsw_sp *mlxsw_sp,
return err;
*used_hw_stats = FLOW_ACTION_HW_STATS_IMMEDIATE;
}
+ if (rulei->policer_index_valid) {
+ err = mlxsw_sp_policer_drops_counter_get(mlxsw_sp, type,
+ rulei->policer_index,
+ ¤t_drops);
+ if (err)
+ return err;
+ }
*packets = current_packets - rule->last_packets;
*bytes = current_bytes - rule->last_bytes;
+ *drops = current_drops - rule->last_drops;
*last_use = rule->last_used;
rule->last_bytes = current_bytes;
rule->last_packets = current_packets;
+ rule->last_drops = current_drops;
return 0;
}
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_flex_actions.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_flex_actions.c
index 18444f675100..90372d1c28d4 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_flex_actions.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_flex_actions.c
@@ -169,6 +169,29 @@ mlxsw_sp_act_mirror_del(void *priv, u8 local_in_port, int span_id, bool ingress)
mlxsw_sp_span_agent_put(mlxsw_sp, span_id);
}
+static int mlxsw_sp_act_policer_add(void *priv, u64 rate_bytes_ps, u32 burst,
+ u16 *p_policer_index,
+ struct netlink_ext_ack *extack)
+{
+ struct mlxsw_sp_policer_params params;
+ struct mlxsw_sp *mlxsw_sp = priv;
+
+ params.rate = rate_bytes_ps;
+ params.burst = burst;
+ params.bytes = true;
+ return mlxsw_sp_policer_add(mlxsw_sp,
+ MLXSW_SP_POLICER_TYPE_SINGLE_RATE,
+ ¶ms, extack, p_policer_index);
+}
+
+static void mlxsw_sp_act_policer_del(void *priv, u16 policer_index)
+{
+ struct mlxsw_sp *mlxsw_sp = priv;
+
+ mlxsw_sp_policer_del(mlxsw_sp, MLXSW_SP_POLICER_TYPE_SINGLE_RATE,
+ policer_index);
+}
+
const struct mlxsw_afa_ops mlxsw_sp1_act_afa_ops = {
.kvdl_set_add = mlxsw_sp1_act_kvdl_set_add,
.kvdl_set_del = mlxsw_sp_act_kvdl_set_del,
@@ -179,6 +202,8 @@ const struct mlxsw_afa_ops mlxsw_sp1_act_afa_ops = {
.counter_index_put = mlxsw_sp_act_counter_index_put,
.mirror_add = mlxsw_sp_act_mirror_add,
.mirror_del = mlxsw_sp_act_mirror_del,
+ .policer_add = mlxsw_sp_act_policer_add,
+ .policer_del = mlxsw_sp_act_policer_del,
};
const struct mlxsw_afa_ops mlxsw_sp2_act_afa_ops = {
@@ -191,6 +216,8 @@ const struct mlxsw_afa_ops mlxsw_sp2_act_afa_ops = {
.counter_index_put = mlxsw_sp_act_counter_index_put,
.mirror_add = mlxsw_sp_act_mirror_add,
.mirror_del = mlxsw_sp_act_mirror_del,
+ .policer_add = mlxsw_sp_act_policer_add,
+ .policer_del = mlxsw_sp_act_policer_del,
.dummy_first_set = true,
};
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c
index 61d21043d83a..41855e58564b 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_flower.c
@@ -4,6 +4,7 @@
#include <linux/kernel.h>
#include <linux/errno.h>
#include <linux/netdevice.h>
+#include <linux/log2.h>
#include <net/net_namespace.h>
#include <net/flow_dissector.h>
#include <net/pkt_cls.h>
@@ -22,6 +23,7 @@ static int mlxsw_sp_flower_parse_actions(struct mlxsw_sp *mlxsw_sp,
{
const struct flow_action_entry *act;
int mirror_act_count = 0;
+ int police_act_count = 0;
int err, i;
if (!flow_action_has_entries(flow_action))
@@ -180,6 +182,28 @@ static int mlxsw_sp_flower_parse_actions(struct mlxsw_sp *mlxsw_sp,
return err;
break;
}
+ case FLOW_ACTION_POLICE: {
+ u32 burst;
+
+ if (police_act_count++) {
+ NL_SET_ERR_MSG_MOD(extack, "Multiple police actions per rule are not supported");
+ return -EOPNOTSUPP;
+ }
+
+ /* The kernel might adjust the requested burst size so
+ * that it is not exactly a power of two. Re-adjust it
+ * here since the hardware only supports burst sizes
+ * that are a power of two.
+ */
+ burst = roundup_pow_of_two(act->police.burst);
+ err = mlxsw_sp_acl_rulei_act_police(mlxsw_sp, rulei,
+ act->police.index,
+ act->police.rate_bytes_ps,
+ burst, extack);
+ if (err)
+ return err;
+ break;
+ }
default:
NL_SET_ERR_MSG_MOD(extack, "Unsupported action");
dev_err(mlxsw_sp->bus_info->dev, "Unsupported action\n");
@@ -616,6 +640,7 @@ int mlxsw_sp_flower_stats(struct mlxsw_sp *mlxsw_sp,
u64 packets;
u64 lastuse;
u64 bytes;
+ u64 drops;
int err;
ruleset = mlxsw_sp_acl_ruleset_get(mlxsw_sp, block,
@@ -629,11 +654,12 @@ int mlxsw_sp_flower_stats(struct mlxsw_sp *mlxsw_sp,
return -EINVAL;
err = mlxsw_sp_acl_rule_get_stats(mlxsw_sp, rule, &packets, &bytes,
- &lastuse, &used_hw_stats);
+ &drops, &lastuse, &used_hw_stats);
if (err)
goto err_rule_get_stats;
- flow_stats_update(&f->stats, bytes, packets, 0, lastuse, used_hw_stats);
+ flow_stats_update(&f->stats, bytes, packets, drops, lastuse,
+ used_hw_stats);
mlxsw_sp_acl_ruleset_put(mlxsw_sp, ruleset);
return 0;
--
2.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH net-next 08/11] selftests: forwarding: Add tc-police tests
2020-07-15 8:27 [PATCH net-next 00/11] mlxsw: Offload tc police action Ido Schimmel
` (6 preceding siblings ...)
2020-07-15 8:27 ` [PATCH net-next 07/11] mlxsw: spectrum_acl: Offload FLOW_ACTION_POLICE Ido Schimmel
@ 2020-07-15 8:27 ` Ido Schimmel
2020-07-15 8:27 ` [PATCH net-next 09/11] selftests: mlxsw: tc_restrictions: Test tc-police restrictions Ido Schimmel
` (3 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Ido Schimmel @ 2020-07-15 8:27 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, jiri, petrm, mlxsw, Ido Schimmel
From: Ido Schimmel <idosch@mellanox.com>
Test tc-police action in various scenarios such as Rx policing, Tx
policing, shared policer and police piped to mirred. The test passes
with both veth pairs and loopbacked ports.
# ./tc_police.sh
TEST: police on rx [ OK ]
TEST: police on tx [ OK ]
TEST: police with shared policer - rx [ OK ]
TEST: police with shared policer - tx [ OK ]
TEST: police rx and mirror [ OK ]
TEST: police tx and mirror [ OK ]
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reviewed-by: Petr Machata <petrm@mellanox.com>
---
.../selftests/net/forwarding/tc_police.sh | 333 ++++++++++++++++++
1 file changed, 333 insertions(+)
create mode 100755 tools/testing/selftests/net/forwarding/tc_police.sh
diff --git a/tools/testing/selftests/net/forwarding/tc_police.sh b/tools/testing/selftests/net/forwarding/tc_police.sh
new file mode 100755
index 000000000000..160f9cccdfb7
--- /dev/null
+++ b/tools/testing/selftests/net/forwarding/tc_police.sh
@@ -0,0 +1,333 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+#
+# Test tc-police action.
+#
+# +---------------------------------+
+# | H1 (vrf) |
+# | + $h1 |
+# | | 192.0.2.1/24 |
+# | | |
+# | | default via 192.0.2.2 |
+# +----|----------------------------+
+# |
+# +----|----------------------------------------------------------------------+
+# | SW | |
+# | + $rp1 |
+# | 192.0.2.2/24 |
+# | |
+# | 198.51.100.2/24 203.0.113.2/24 |
+# | + $rp2 + $rp3 |
+# | | | |
+# +----|-----------------------------------------|----------------------------+
+# | |
+# +----|----------------------------+ +----|----------------------------+
+# | | default via 198.51.100.2 | | | default via 203.0.113.2 |
+# | | | | | |
+# | | 198.51.100.1/24 | | | 203.0.113.1/24 |
+# | + $h2 | | + $h3 |
+# | H2 (vrf) | | H3 (vrf) |
+# +---------------------------------+ +---------------------------------+
+
+ALL_TESTS="
+ police_rx_test
+ police_tx_test
+ police_shared_test
+ police_rx_mirror_test
+ police_tx_mirror_test
+"
+NUM_NETIFS=6
+source tc_common.sh
+source lib.sh
+
+h1_create()
+{
+ simple_if_init $h1 192.0.2.1/24
+
+ ip -4 route add default vrf v$h1 nexthop via 192.0.2.2
+}
+
+h1_destroy()
+{
+ ip -4 route del default vrf v$h1 nexthop via 192.0.2.2
+
+ simple_if_fini $h1 192.0.2.1/24
+}
+
+h2_create()
+{
+ simple_if_init $h2 198.51.100.1/24
+
+ ip -4 route add default vrf v$h2 nexthop via 198.51.100.2
+
+ tc qdisc add dev $h2 clsact
+}
+
+h2_destroy()
+{
+ tc qdisc del dev $h2 clsact
+
+ ip -4 route del default vrf v$h2 nexthop via 198.51.100.2
+
+ simple_if_fini $h2 198.51.100.1/24
+}
+
+h3_create()
+{
+ simple_if_init $h3 203.0.113.1/24
+
+ ip -4 route add default vrf v$h3 nexthop via 203.0.113.2
+
+ tc qdisc add dev $h3 clsact
+}
+
+h3_destroy()
+{
+ tc qdisc del dev $h3 clsact
+
+ ip -4 route del default vrf v$h3 nexthop via 203.0.113.2
+
+ simple_if_fini $h3 203.0.113.1/24
+}
+
+router_create()
+{
+ ip link set dev $rp1 up
+ ip link set dev $rp2 up
+ ip link set dev $rp3 up
+
+ __addr_add_del $rp1 add 192.0.2.2/24
+ __addr_add_del $rp2 add 198.51.100.2/24
+ __addr_add_del $rp3 add 203.0.113.2/24
+
+ tc qdisc add dev $rp1 clsact
+ tc qdisc add dev $rp2 clsact
+}
+
+router_destroy()
+{
+ tc qdisc del dev $rp2 clsact
+ tc qdisc del dev $rp1 clsact
+
+ __addr_add_del $rp3 del 203.0.113.2/24
+ __addr_add_del $rp2 del 198.51.100.2/24
+ __addr_add_del $rp1 del 192.0.2.2/24
+
+ ip link set dev $rp3 down
+ ip link set dev $rp2 down
+ ip link set dev $rp1 down
+}
+
+police_common_test()
+{
+ local test_name=$1; shift
+
+ RET=0
+
+ # Rule to measure bandwidth on ingress of $h2
+ tc filter add dev $h2 ingress protocol ip pref 1 handle 101 flower \
+ dst_ip 198.51.100.1 ip_proto udp dst_port 54321 \
+ action drop
+
+ mausezahn $h1 -a own -b $(mac_get $rp1) -A 192.0.2.1 -B 198.51.100.1 \
+ -t udp sp=12345,dp=54321 -p 1000 -c 0 -q &
+
+ local t0=$(tc_rule_stats_get $h2 1 ingress .bytes)
+ sleep 10
+ local t1=$(tc_rule_stats_get $h2 1 ingress .bytes)
+
+ local er=$((80 * 1000 * 1000))
+ local nr=$(rate $t0 $t1 10)
+ local nr_pct=$((100 * (nr - er) / er))
+ ((-10 <= nr_pct && nr_pct <= 10))
+ check_err $? "Expected rate $(humanize $er), got $(humanize $nr), which is $nr_pct% off. Required accuracy is +-10%."
+
+ log_test "$test_name"
+
+ { kill %% && wait %%; } 2>/dev/null
+ tc filter del dev $h2 ingress protocol ip pref 1 handle 101 flower
+}
+
+police_rx_test()
+{
+ # Rule to police traffic destined to $h2 on ingress of $rp1
+ tc filter add dev $rp1 ingress protocol ip pref 1 handle 101 flower \
+ dst_ip 198.51.100.1 ip_proto udp dst_port 54321 \
+ action police rate 80mbit burst 16k conform-exceed drop/ok
+
+ police_common_test "police on rx"
+
+ tc filter del dev $rp1 ingress protocol ip pref 1 handle 101 flower
+}
+
+police_tx_test()
+{
+ # Rule to police traffic destined to $h2 on egress of $rp2
+ tc filter add dev $rp2 egress protocol ip pref 1 handle 101 flower \
+ dst_ip 198.51.100.1 ip_proto udp dst_port 54321 \
+ action police rate 80mbit burst 16k conform-exceed drop/ok
+
+ police_common_test "police on tx"
+
+ tc filter del dev $rp2 egress protocol ip pref 1 handle 101 flower
+}
+
+police_shared_common_test()
+{
+ local dport=$1; shift
+ local test_name=$1; shift
+
+ RET=0
+
+ mausezahn $h1 -a own -b $(mac_get $rp1) -A 192.0.2.1 -B 198.51.100.1 \
+ -t udp sp=12345,dp=$dport -p 1000 -c 0 -q &
+
+ local t0=$(tc_rule_stats_get $h2 1 ingress .bytes)
+ sleep 10
+ local t1=$(tc_rule_stats_get $h2 1 ingress .bytes)
+
+ local er=$((80 * 1000 * 1000))
+ local nr=$(rate $t0 $t1 10)
+ local nr_pct=$((100 * (nr - er) / er))
+ ((-10 <= nr_pct && nr_pct <= 10))
+ check_err $? "Expected rate $(humanize $er), got $(humanize $nr), which is $nr_pct% off. Required accuracy is +-10%."
+
+ log_test "$test_name"
+
+ { kill %% && wait %%; } 2>/dev/null
+}
+
+police_shared_test()
+{
+ # Rule to measure bandwidth on ingress of $h2
+ tc filter add dev $h2 ingress protocol ip pref 1 handle 101 flower \
+ dst_ip 198.51.100.1 ip_proto udp src_port 12345 \
+ action drop
+
+ # Rule to police traffic destined to $h2 on ingress of $rp1
+ tc filter add dev $rp1 ingress protocol ip pref 1 handle 101 flower \
+ dst_ip 198.51.100.1 ip_proto udp dst_port 54321 \
+ action police rate 80mbit burst 16k conform-exceed drop/ok \
+ index 10
+
+ # Rule to police a different flow destined to $h2 on egress of $rp2
+ # using same policer
+ tc filter add dev $rp2 egress protocol ip pref 1 handle 101 flower \
+ dst_ip 198.51.100.1 ip_proto udp dst_port 22222 \
+ action police index 10
+
+ police_shared_common_test 54321 "police with shared policer - rx"
+
+ police_shared_common_test 22222 "police with shared policer - tx"
+
+ tc filter del dev $rp2 egress protocol ip pref 1 handle 101 flower
+ tc filter del dev $rp1 ingress protocol ip pref 1 handle 101 flower
+ tc filter del dev $h2 ingress protocol ip pref 1 handle 101 flower
+}
+
+police_mirror_common_test()
+{
+ local pol_if=$1; shift
+ local dir=$1; shift
+ local test_name=$1; shift
+
+ RET=0
+
+ # Rule to measure bandwidth on ingress of $h2
+ tc filter add dev $h2 ingress protocol ip pref 1 handle 101 flower \
+ dst_ip 198.51.100.1 ip_proto udp dst_port 54321 \
+ action drop
+
+ # Rule to measure bandwidth of mirrored traffic on ingress of $h3
+ tc filter add dev $h3 ingress protocol ip pref 1 handle 101 flower \
+ dst_ip 198.51.100.1 ip_proto udp dst_port 54321 \
+ action drop
+
+ # Rule to police traffic destined to $h2 and mirror to $h3
+ tc filter add dev $pol_if $dir protocol ip pref 1 handle 101 flower \
+ dst_ip 198.51.100.1 ip_proto udp dst_port 54321 \
+ action police rate 80mbit burst 16k conform-exceed drop/pipe \
+ action mirred egress mirror dev $rp3
+
+ mausezahn $h1 -a own -b $(mac_get $rp1) -A 192.0.2.1 -B 198.51.100.1 \
+ -t udp sp=12345,dp=54321 -p 1000 -c 0 -q &
+
+ local t0=$(tc_rule_stats_get $h2 1 ingress .bytes)
+ sleep 10
+ local t1=$(tc_rule_stats_get $h2 1 ingress .bytes)
+
+ local er=$((80 * 1000 * 1000))
+ local nr=$(rate $t0 $t1 10)
+ local nr_pct=$((100 * (nr - er) / er))
+ ((-10 <= nr_pct && nr_pct <= 10))
+ check_err $? "Expected rate $(humanize $er), got $(humanize $nr), which is $nr_pct% off. Required accuracy is +-10%."
+
+ local t0=$(tc_rule_stats_get $h3 1 ingress .bytes)
+ sleep 10
+ local t1=$(tc_rule_stats_get $h3 1 ingress .bytes)
+
+ local er=$((80 * 1000 * 1000))
+ local nr=$(rate $t0 $t1 10)
+ local nr_pct=$((100 * (nr - er) / er))
+ ((-10 <= nr_pct && nr_pct <= 10))
+ check_err $? "Expected rate $(humanize $er), got $(humanize $nr), which is $nr_pct% off. Required accuracy is +-10%."
+
+ log_test "$test_name"
+
+ { kill %% && wait %%; } 2>/dev/null
+ tc filter del dev $pol_if $dir protocol ip pref 1 handle 101 flower
+ tc filter del dev $h3 ingress protocol ip pref 1 handle 101 flower
+ tc filter del dev $h2 ingress protocol ip pref 1 handle 101 flower
+}
+
+police_rx_mirror_test()
+{
+ police_mirror_common_test $rp1 ingress "police rx and mirror"
+}
+
+police_tx_mirror_test()
+{
+ police_mirror_common_test $rp2 egress "police tx and mirror"
+}
+
+setup_prepare()
+{
+ h1=${NETIFS[p1]}
+ rp1=${NETIFS[p2]}
+
+ rp2=${NETIFS[p3]}
+ h2=${NETIFS[p4]}
+
+ rp3=${NETIFS[p5]}
+ h3=${NETIFS[p6]}
+
+ vrf_prepare
+ forwarding_enable
+
+ h1_create
+ h2_create
+ h3_create
+ router_create
+}
+
+cleanup()
+{
+ pre_cleanup
+
+ router_destroy
+ h3_destroy
+ h2_destroy
+ h1_destroy
+
+ forwarding_restore
+ vrf_cleanup
+}
+
+trap cleanup EXIT
+
+setup_prepare
+setup_wait
+
+tests_run
+
+exit $EXIT_STATUS
--
2.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH net-next 09/11] selftests: mlxsw: tc_restrictions: Test tc-police restrictions
2020-07-15 8:27 [PATCH net-next 00/11] mlxsw: Offload tc police action Ido Schimmel
` (7 preceding siblings ...)
2020-07-15 8:27 ` [PATCH net-next 08/11] selftests: forwarding: Add tc-police tests Ido Schimmel
@ 2020-07-15 8:27 ` Ido Schimmel
2020-07-15 8:27 ` [PATCH net-next 10/11] selftests: mlxsw: Add scale test for tc-police Ido Schimmel
` (2 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Ido Schimmel @ 2020-07-15 8:27 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, jiri, petrm, mlxsw, Ido Schimmel
From: Ido Schimmel <idosch@mellanox.com>
Test that upper and lower limits on rate and burst size imposed by the
device are rejected by the kernel.
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reviewed-by: Petr Machata <petrm@mellanox.com>
---
.../drivers/net/mlxsw/tc_restrictions.sh | 76 +++++++++++++++++++
1 file changed, 76 insertions(+)
diff --git a/tools/testing/selftests/drivers/net/mlxsw/tc_restrictions.sh b/tools/testing/selftests/drivers/net/mlxsw/tc_restrictions.sh
index 9241250c5921..553cb9fad508 100755
--- a/tools/testing/selftests/drivers/net/mlxsw/tc_restrictions.sh
+++ b/tools/testing/selftests/drivers/net/mlxsw/tc_restrictions.sh
@@ -11,6 +11,8 @@ ALL_TESTS="
matchall_mirror_behind_flower_ingress_test
matchall_sample_behind_flower_ingress_test
matchall_mirror_behind_flower_egress_test
+ police_limits_test
+ multi_police_test
"
NUM_NETIFS=2
@@ -287,6 +289,80 @@ matchall_mirror_behind_flower_egress_test()
matchall_behind_flower_egress_test "mirror" "mirred egress mirror dev $swp2"
}
+police_limits_test()
+{
+ RET=0
+
+ tc qdisc add dev $swp1 clsact
+
+ tc filter add dev $swp1 ingress pref 1 proto ip handle 101 \
+ flower skip_sw \
+ action police rate 0.5kbit burst 1m conform-exceed drop/ok
+ check_fail $? "Incorrect success to add police action with too low rate"
+
+ tc filter add dev $swp1 ingress pref 1 proto ip handle 101 \
+ flower skip_sw \
+ action police rate 2.5tbit burst 1g conform-exceed drop/ok
+ check_fail $? "Incorrect success to add police action with too high rate"
+
+ tc filter add dev $swp1 ingress pref 1 proto ip handle 101 \
+ flower skip_sw \
+ action police rate 1.5kbit burst 1m conform-exceed drop/ok
+ check_err $? "Failed to add police action with low rate"
+
+ tc filter del dev $swp1 ingress protocol ip pref 1 handle 101 flower
+
+ tc filter add dev $swp1 ingress pref 1 proto ip handle 101 \
+ flower skip_sw \
+ action police rate 1.9tbit burst 1g conform-exceed drop/ok
+ check_err $? "Failed to add police action with high rate"
+
+ tc filter del dev $swp1 ingress protocol ip pref 1 handle 101 flower
+
+ tc filter add dev $swp1 ingress pref 1 proto ip handle 101 \
+ flower skip_sw \
+ action police rate 1.5kbit burst 512b conform-exceed drop/ok
+ check_fail $? "Incorrect success to add police action with too low burst size"
+
+ tc filter add dev $swp1 ingress pref 1 proto ip handle 101 \
+ flower skip_sw \
+ action police rate 1.5kbit burst 2k conform-exceed drop/ok
+ check_err $? "Failed to add police action with low burst size"
+
+ tc filter del dev $swp1 ingress protocol ip pref 1 handle 101 flower
+
+ tc qdisc del dev $swp1 clsact
+
+ log_test "police rate and burst limits"
+}
+
+multi_police_test()
+{
+ RET=0
+
+ # It is forbidden in mlxsw driver to have multiple police
+ # actions in a single rule.
+
+ tc qdisc add dev $swp1 clsact
+
+ tc filter add dev $swp1 ingress protocol ip pref 1 handle 101 \
+ flower skip_sw \
+ action police rate 100mbit burst 100k conform-exceed drop/ok
+ check_err $? "Failed to add rule with single police action"
+
+ tc filter del dev $swp1 ingress protocol ip pref 1 handle 101 flower
+
+ tc filter add dev $swp1 ingress protocol ip pref 1 handle 101 \
+ flower skip_sw \
+ action police rate 100mbit burst 100k conform-exceed drop/pipe \
+ action police rate 200mbit burst 200k conform-exceed drop/ok
+ check_fail $? "Incorrect success to add rule with two police actions"
+
+ tc qdisc del dev $swp1 clsact
+
+ log_test "multi police"
+}
+
setup_prepare()
{
swp1=${NETIFS[p1]}
--
2.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH net-next 10/11] selftests: mlxsw: Add scale test for tc-police
2020-07-15 8:27 [PATCH net-next 00/11] mlxsw: Offload tc police action Ido Schimmel
` (8 preceding siblings ...)
2020-07-15 8:27 ` [PATCH net-next 09/11] selftests: mlxsw: tc_restrictions: Test tc-police restrictions Ido Schimmel
@ 2020-07-15 8:27 ` Ido Schimmel
2020-07-15 8:27 ` [PATCH net-next 11/11] selftests: mlxsw: Test policers' occupancy Ido Schimmel
2020-07-16 1:13 ` [PATCH net-next 00/11] mlxsw: Offload tc police action Jakub Kicinski
11 siblings, 0 replies; 17+ messages in thread
From: Ido Schimmel @ 2020-07-15 8:27 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, jiri, petrm, mlxsw, Ido Schimmel
From: Ido Schimmel <idosch@mellanox.com>
Query the maximum number of supported policers using devlink-resource
and test that this number can be reached by configuring tc filters with
police action. Test that an error is returned in case the maximum number
is exceeded.
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reviewed-by: Petr Machata <petrm@mellanox.com>
---
.../net/mlxsw/spectrum-2/resource_scale.sh | 2 +-
.../net/mlxsw/spectrum-2/tc_police_scale.sh | 16 ++++
.../net/mlxsw/spectrum/resource_scale.sh | 2 +-
.../net/mlxsw/spectrum/tc_police_scale.sh | 16 ++++
.../drivers/net/mlxsw/tc_police_scale.sh | 92 +++++++++++++++++++
5 files changed, 126 insertions(+), 2 deletions(-)
create mode 100644 tools/testing/selftests/drivers/net/mlxsw/spectrum-2/tc_police_scale.sh
create mode 100644 tools/testing/selftests/drivers/net/mlxsw/spectrum/tc_police_scale.sh
create mode 100644 tools/testing/selftests/drivers/net/mlxsw/tc_police_scale.sh
diff --git a/tools/testing/selftests/drivers/net/mlxsw/spectrum-2/resource_scale.sh b/tools/testing/selftests/drivers/net/mlxsw/spectrum-2/resource_scale.sh
index fd583a171db7..d7cf33a3f18d 100755
--- a/tools/testing/selftests/drivers/net/mlxsw/spectrum-2/resource_scale.sh
+++ b/tools/testing/selftests/drivers/net/mlxsw/spectrum-2/resource_scale.sh
@@ -28,7 +28,7 @@ cleanup()
trap cleanup EXIT
-ALL_TESTS="router tc_flower mirror_gre"
+ALL_TESTS="router tc_flower mirror_gre tc_police"
for current_test in ${TESTS:-$ALL_TESTS}; do
source ${current_test}_scale.sh
diff --git a/tools/testing/selftests/drivers/net/mlxsw/spectrum-2/tc_police_scale.sh b/tools/testing/selftests/drivers/net/mlxsw/spectrum-2/tc_police_scale.sh
new file mode 100644
index 000000000000..e79ac0dad1f4
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/mlxsw/spectrum-2/tc_police_scale.sh
@@ -0,0 +1,16 @@
+# SPDX-License-Identifier: GPL-2.0
+source ../tc_police_scale.sh
+
+tc_police_get_target()
+{
+ local should_fail=$1; shift
+ local target
+
+ target=$(devlink_resource_size_get global_policers single_rate_policers)
+
+ if ((! should_fail)); then
+ echo $target
+ else
+ echo $((target + 1))
+ fi
+}
diff --git a/tools/testing/selftests/drivers/net/mlxsw/spectrum/resource_scale.sh b/tools/testing/selftests/drivers/net/mlxsw/spectrum/resource_scale.sh
index 43ba1b438f6d..43f662401bc3 100755
--- a/tools/testing/selftests/drivers/net/mlxsw/spectrum/resource_scale.sh
+++ b/tools/testing/selftests/drivers/net/mlxsw/spectrum/resource_scale.sh
@@ -22,7 +22,7 @@ cleanup()
devlink_sp_read_kvd_defaults
trap cleanup EXIT
-ALL_TESTS="router tc_flower mirror_gre"
+ALL_TESTS="router tc_flower mirror_gre tc_police"
for current_test in ${TESTS:-$ALL_TESTS}; do
source ${current_test}_scale.sh
diff --git a/tools/testing/selftests/drivers/net/mlxsw/spectrum/tc_police_scale.sh b/tools/testing/selftests/drivers/net/mlxsw/spectrum/tc_police_scale.sh
new file mode 100644
index 000000000000..e79ac0dad1f4
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/mlxsw/spectrum/tc_police_scale.sh
@@ -0,0 +1,16 @@
+# SPDX-License-Identifier: GPL-2.0
+source ../tc_police_scale.sh
+
+tc_police_get_target()
+{
+ local should_fail=$1; shift
+ local target
+
+ target=$(devlink_resource_size_get global_policers single_rate_policers)
+
+ if ((! should_fail)); then
+ echo $target
+ else
+ echo $((target + 1))
+ fi
+}
diff --git a/tools/testing/selftests/drivers/net/mlxsw/tc_police_scale.sh b/tools/testing/selftests/drivers/net/mlxsw/tc_police_scale.sh
new file mode 100644
index 000000000000..4b96561c462f
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/mlxsw/tc_police_scale.sh
@@ -0,0 +1,92 @@
+# SPDX-License-Identifier: GPL-2.0
+
+TC_POLICE_NUM_NETIFS=2
+
+tc_police_h1_create()
+{
+ simple_if_init $h1
+}
+
+tc_police_h1_destroy()
+{
+ simple_if_fini $h1
+}
+
+tc_police_switch_create()
+{
+ simple_if_init $swp1
+ tc qdisc add dev $swp1 clsact
+}
+
+tc_police_switch_destroy()
+{
+ tc qdisc del dev $swp1 clsact
+ simple_if_fini $swp1
+}
+
+tc_police_rules_create()
+{
+ local count=$1; shift
+ local should_fail=$1; shift
+
+ TC_POLICE_BATCH_FILE="$(mktemp)"
+
+ for ((i = 0; i < count; ++i)); do
+ cat >> $TC_POLICE_BATCH_FILE <<-EOF
+ filter add dev $swp1 ingress \
+ prot ip \
+ flower skip_sw \
+ action police rate 10mbit burst 100k \
+ conform-exceed drop/ok
+ EOF
+ done
+
+ tc -b $TC_POLICE_BATCH_FILE
+ check_err_fail $should_fail $? "Rule insertion"
+}
+
+__tc_police_test()
+{
+ local count=$1; shift
+ local should_fail=$1; shift
+
+ tc_police_rules_create $count $should_fail
+
+ offload_count=$(tc filter show dev $swp1 ingress | grep in_hw | wc -l)
+ ((offload_count == count))
+ check_err_fail $should_fail $? "tc police offload count"
+}
+
+tc_police_test()
+{
+ local count=$1; shift
+ local should_fail=$1; shift
+
+ if ! tc_offload_check $TC_POLICE_NUM_NETIFS; then
+ check_err 1 "Could not test offloaded functionality"
+ return
+ fi
+
+ __tc_police_test $count $should_fail
+}
+
+tc_police_setup_prepare()
+{
+ h1=${NETIFS[p1]}
+ swp1=${NETIFS[p2]}
+
+ vrf_prepare
+
+ tc_police_h1_create
+ tc_police_switch_create
+}
+
+tc_police_cleanup()
+{
+ pre_cleanup
+
+ tc_police_switch_destroy
+ tc_police_h1_destroy
+
+ vrf_cleanup
+}
--
2.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH net-next 11/11] selftests: mlxsw: Test policers' occupancy
2020-07-15 8:27 [PATCH net-next 00/11] mlxsw: Offload tc police action Ido Schimmel
` (9 preceding siblings ...)
2020-07-15 8:27 ` [PATCH net-next 10/11] selftests: mlxsw: Add scale test for tc-police Ido Schimmel
@ 2020-07-15 8:27 ` Ido Schimmel
2020-07-16 1:13 ` [PATCH net-next 00/11] mlxsw: Offload tc police action Jakub Kicinski
11 siblings, 0 replies; 17+ messages in thread
From: Ido Schimmel @ 2020-07-15 8:27 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, jiri, petrm, mlxsw, Ido Schimmel
From: Ido Schimmel <idosch@mellanox.com>
Test that policers shared by different tc filters are correctly
reference counted by observing policers' occupancy via devlink-resource.
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Reviewed-by: Petr Machata <petrm@mellanox.com>
---
.../drivers/net/mlxsw/tc_police_occ.sh | 108 ++++++++++++++++++
.../selftests/net/forwarding/devlink_lib.sh | 5 +
2 files changed, 113 insertions(+)
create mode 100755 tools/testing/selftests/drivers/net/mlxsw/tc_police_occ.sh
diff --git a/tools/testing/selftests/drivers/net/mlxsw/tc_police_occ.sh b/tools/testing/selftests/drivers/net/mlxsw/tc_police_occ.sh
new file mode 100755
index 000000000000..448b75c1545a
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/mlxsw/tc_police_occ.sh
@@ -0,0 +1,108 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+#
+# Test that policers shared by different tc filters are correctly reference
+# counted by observing policers' occupancy via devlink-resource.
+
+lib_dir=$(dirname $0)/../../../net/forwarding
+
+ALL_TESTS="
+ tc_police_occ_test
+"
+NUM_NETIFS=2
+source $lib_dir/lib.sh
+source $lib_dir/devlink_lib.sh
+
+h1_create()
+{
+ simple_if_init $h1
+}
+
+h1_destroy()
+{
+ simple_if_fini $h1
+}
+
+switch_create()
+{
+ simple_if_init $swp1
+ tc qdisc add dev $swp1 clsact
+}
+
+switch_destroy()
+{
+ tc qdisc del dev $swp1 clsact
+ simple_if_fini $swp1
+}
+
+setup_prepare()
+{
+ h1=${NETIFS[p1]}
+ swp1=${NETIFS[p2]}
+
+ vrf_prepare
+
+ h1_create
+ switch_create
+}
+
+cleanup()
+{
+ pre_cleanup
+
+ switch_destroy
+ h1_destroy
+
+ vrf_cleanup
+}
+
+tc_police_occ_get()
+{
+ devlink_resource_occ_get global_policers single_rate_policers
+}
+
+tc_police_occ_test()
+{
+ RET=0
+
+ local occ=$(tc_police_occ_get)
+
+ tc filter add dev $swp1 ingress pref 1 handle 101 proto ip \
+ flower skip_sw \
+ action police rate 100mbit burst 100k conform-exceed drop/ok
+ (( occ + 1 == $(tc_police_occ_get) ))
+ check_err $? "Got occupancy $(tc_police_occ_get), expected $((occ + 1))"
+
+ tc filter del dev $swp1 ingress pref 1 handle 101 flower
+ (( occ == $(tc_police_occ_get) ))
+ check_err $? "Got occupancy $(tc_police_occ_get), expected $occ"
+
+ tc filter add dev $swp1 ingress pref 1 handle 101 proto ip \
+ flower skip_sw \
+ action police rate 100mbit burst 100k conform-exceed drop/ok \
+ index 10
+ tc filter add dev $swp1 ingress pref 2 handle 102 proto ip \
+ flower skip_sw action police index 10
+
+ (( occ + 1 == $(tc_police_occ_get) ))
+ check_err $? "Got occupancy $(tc_police_occ_get), expected $((occ + 1))"
+
+ tc filter del dev $swp1 ingress pref 2 handle 102 flower
+ (( occ + 1 == $(tc_police_occ_get) ))
+ check_err $? "Got occupancy $(tc_police_occ_get), expected $((occ + 1))"
+
+ tc filter del dev $swp1 ingress pref 1 handle 101 flower
+ (( occ == $(tc_police_occ_get) ))
+ check_err $? "Got occupancy $(tc_police_occ_get), expected $occ"
+
+ log_test "tc police occupancy"
+}
+
+trap cleanup EXIT
+
+setup_prepare
+setup_wait
+
+tests_run
+
+exit $EXIT_STATUS
diff --git a/tools/testing/selftests/net/forwarding/devlink_lib.sh b/tools/testing/selftests/net/forwarding/devlink_lib.sh
index f0e6be4c09e9..75fe24bcb9cd 100644
--- a/tools/testing/selftests/net/forwarding/devlink_lib.sh
+++ b/tools/testing/selftests/net/forwarding/devlink_lib.sh
@@ -98,6 +98,11 @@ devlink_resource_size_set()
check_err $? "Failed setting path $path to size $size"
}
+devlink_resource_occ_get()
+{
+ devlink_resource_get "$@" | jq '.["occ"]'
+}
+
devlink_reload()
{
local still_pending
--
2.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 00/11] mlxsw: Offload tc police action
2020-07-15 8:27 [PATCH net-next 00/11] mlxsw: Offload tc police action Ido Schimmel
` (10 preceding siblings ...)
2020-07-15 8:27 ` [PATCH net-next 11/11] selftests: mlxsw: Test policers' occupancy Ido Schimmel
@ 2020-07-16 1:13 ` Jakub Kicinski
11 siblings, 0 replies; 17+ messages in thread
From: Jakub Kicinski @ 2020-07-16 1:13 UTC (permalink / raw)
To: Ido Schimmel; +Cc: netdev, davem, jiri, petrm, mlxsw, Ido Schimmel
On Wed, 15 Jul 2020 11:27:22 +0300 Ido Schimmel wrote:
> From: Ido Schimmel <idosch@mellanox.com>
>
> This patch set adds support for tc police action in mlxsw.
>
> Patches #1-#2 add defines for policer bandwidth limits and resource
> identifiers (e.g., maximum number of policers).
>
> Patch #3 adds a common policer core in mlxsw. Currently it is only used
> by the policy engine, but future patch sets will use it for trap
> policers and storm control policers. The common core allows us to share
> common logic between all policer types and abstract certain details from
> the various users in mlxsw.
>
> Patch #4 exposes the maximum number of supported policers and their
> current usage to user space via devlink-resource. This provides better
> visibility and also used for selftests purposes.
>
> Patches #5-#7 gradually add support for tc police action in the policy
> engine by calling into previously mentioned policer core.
>
> Patch #8 adds a generic selftest for tc-police that can be used with
> veth pairs or physical loopbacks.
>
> Patches #9-#11 add mlxsw-specific selftests.
Applied, thanks Ido!
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 03/11] mlxsw: spectrum_policer: Add policer core
2020-07-15 8:27 ` [PATCH net-next 03/11] mlxsw: spectrum_policer: Add policer core Ido Schimmel
@ 2020-08-17 15:38 ` Bjorn Helgaas
2020-08-18 6:41 ` Ido Schimmel
0 siblings, 1 reply; 17+ messages in thread
From: Bjorn Helgaas @ 2020-08-17 15:38 UTC (permalink / raw)
To: Ido Schimmel; +Cc: netdev, davem, kuba, jiri, petrm, mlxsw, Ido Schimmel
You've likely seen this already, but Coverity found this problem:
*** CID 1466147: Control flow issues (DEADCODE)
/drivers/net/ethernet/mellanox/mlxsw/spectrum_policer.c: 380 in mlxsw_sp_policers_init()
374 }
375
376 return 0;
377
378 err_family_register:
379 for (i--; i >= 0; i--) {
>>> CID 1466147: Control flow issues (DEADCODE)
>>> Execution cannot reach this statement: "struct mlxsw_sp_policer_fam...".
380 struct mlxsw_sp_policer_family *family;
381
382 family = mlxsw_sp->policer_core->family_arr[i];
383 mlxsw_sp_policer_family_unregister(mlxsw_sp, family);
384 }
385 err_init:
I think the problem is that MLXSW_SP_POLICER_TYPE_MAX is 0 because
> +enum mlxsw_sp_policer_type {
> + MLXSW_SP_POLICER_TYPE_SINGLE_RATE,
> +
> + __MLXSW_SP_POLICER_TYPE_MAX,
> + MLXSW_SP_POLICER_TYPE_MAX = __MLXSW_SP_POLICER_TYPE_MAX - 1,
> +};
so we can only execute the family_register loop once, with i == 0,
and if we get to err_family_register via the error exit:
> + for (i = 0; i < MLXSW_SP_POLICER_TYPE_MAX + 1; i++) {
> + err = mlxsw_sp_policer_family_register(mlxsw_sp, mlxsw_sp_policer_family_arr[i]);
> + if (err)
> + goto err_family_register;
i will be 0, so i-- sets i to -1, so we don't enter the
family_unregister loop body since -1 is not >= 0.
This code is now upstream as 8d3fbae70d8d ("mlxsw: spectrum_policer:
Add policer core").
Bjorn
On Wed, Jul 15, 2020 at 11:27:25AM +0300, Ido Schimmel wrote:
> From: Ido Schimmel <idosch@mellanox.com>
>
> Add common code to handle all policer-related functionality in mlxsw.
> Currently, only policer for policy engines are supported, but it in the
> future more policer families will be added such as CPU (trap) policers
> and storm control policers.
>
> The API allows different modules to add / delete policers and read their
> drop counter.
>
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> Reviewed-by: Jiri Pirko <jiri@mellanox.com>
> Reviewed-by: Petr Machata <petrm@mellanox.com>
> ---
> drivers/net/ethernet/mellanox/mlxsw/Makefile | 2 +-
> .../net/ethernet/mellanox/mlxsw/spectrum.c | 12 +
> .../net/ethernet/mellanox/mlxsw/spectrum.h | 32 ++
> .../mellanox/mlxsw/spectrum_policer.c | 403 ++++++++++++++++++
> 4 files changed, 448 insertions(+), 1 deletion(-)
> create mode 100644 drivers/net/ethernet/mellanox/mlxsw/spectrum_policer.c
>
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/Makefile b/drivers/net/ethernet/mellanox/mlxsw/Makefile
> index 3709983fbd77..892724380ea2 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/Makefile
> +++ b/drivers/net/ethernet/mellanox/mlxsw/Makefile
> @@ -31,7 +31,7 @@ mlxsw_spectrum-objs := spectrum.o spectrum_buffers.o \
> spectrum_qdisc.o spectrum_span.o \
> spectrum_nve.o spectrum_nve_vxlan.o \
> spectrum_dpipe.o spectrum_trap.o \
> - spectrum_ethtool.o
> + spectrum_ethtool.o spectrum_policer.o
> mlxsw_spectrum-$(CONFIG_MLXSW_SPECTRUM_DCB) += spectrum_dcb.o
> mlxsw_spectrum-$(CONFIG_PTP_1588_CLOCK) += spectrum_ptp.o
> obj-$(CONFIG_MLXSW_MINIMAL) += mlxsw_minimal.o
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
> index 4ac634bd3571..c6ab61818800 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
> @@ -2860,6 +2860,12 @@ static int mlxsw_sp_init(struct mlxsw_core *mlxsw_core,
> goto err_fids_init;
> }
>
> + err = mlxsw_sp_policers_init(mlxsw_sp);
> + if (err) {
> + dev_err(mlxsw_sp->bus_info->dev, "Failed to initialize policers\n");
> + goto err_policers_init;
> + }
> +
> err = mlxsw_sp_traps_init(mlxsw_sp);
> if (err) {
> dev_err(mlxsw_sp->bus_info->dev, "Failed to set traps\n");
> @@ -3019,6 +3025,8 @@ static int mlxsw_sp_init(struct mlxsw_core *mlxsw_core,
> err_devlink_traps_init:
> mlxsw_sp_traps_fini(mlxsw_sp);
> err_traps_init:
> + mlxsw_sp_policers_fini(mlxsw_sp);
> +err_policers_init:
> mlxsw_sp_fids_fini(mlxsw_sp);
> err_fids_init:
> mlxsw_sp_kvdl_fini(mlxsw_sp);
> @@ -3046,6 +3054,7 @@ static int mlxsw_sp1_init(struct mlxsw_core *mlxsw_core,
> mlxsw_sp->port_type_speed_ops = &mlxsw_sp1_port_type_speed_ops;
> mlxsw_sp->ptp_ops = &mlxsw_sp1_ptp_ops;
> mlxsw_sp->span_ops = &mlxsw_sp1_span_ops;
> + mlxsw_sp->policer_core_ops = &mlxsw_sp1_policer_core_ops;
> mlxsw_sp->listeners = mlxsw_sp1_listener;
> mlxsw_sp->listeners_count = ARRAY_SIZE(mlxsw_sp1_listener);
> mlxsw_sp->lowest_shaper_bs = MLXSW_REG_QEEC_LOWEST_SHAPER_BS_SP1;
> @@ -3074,6 +3083,7 @@ static int mlxsw_sp2_init(struct mlxsw_core *mlxsw_core,
> mlxsw_sp->port_type_speed_ops = &mlxsw_sp2_port_type_speed_ops;
> mlxsw_sp->ptp_ops = &mlxsw_sp2_ptp_ops;
> mlxsw_sp->span_ops = &mlxsw_sp2_span_ops;
> + mlxsw_sp->policer_core_ops = &mlxsw_sp2_policer_core_ops;
> mlxsw_sp->lowest_shaper_bs = MLXSW_REG_QEEC_LOWEST_SHAPER_BS_SP2;
>
> return mlxsw_sp_init(mlxsw_core, mlxsw_bus_info, extack);
> @@ -3100,6 +3110,7 @@ static int mlxsw_sp3_init(struct mlxsw_core *mlxsw_core,
> mlxsw_sp->port_type_speed_ops = &mlxsw_sp2_port_type_speed_ops;
> mlxsw_sp->ptp_ops = &mlxsw_sp2_ptp_ops;
> mlxsw_sp->span_ops = &mlxsw_sp3_span_ops;
> + mlxsw_sp->policer_core_ops = &mlxsw_sp2_policer_core_ops;
> mlxsw_sp->lowest_shaper_bs = MLXSW_REG_QEEC_LOWEST_SHAPER_BS_SP3;
>
> return mlxsw_sp_init(mlxsw_core, mlxsw_bus_info, extack);
> @@ -3129,6 +3140,7 @@ static void mlxsw_sp_fini(struct mlxsw_core *mlxsw_core)
> mlxsw_sp_buffers_fini(mlxsw_sp);
> mlxsw_sp_devlink_traps_fini(mlxsw_sp);
> mlxsw_sp_traps_fini(mlxsw_sp);
> + mlxsw_sp_policers_fini(mlxsw_sp);
> mlxsw_sp_fids_fini(mlxsw_sp);
> mlxsw_sp_kvdl_fini(mlxsw_sp);
> }
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
> index c00811178637..82227e87ef7c 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
> @@ -151,6 +151,7 @@ struct mlxsw_sp {
> struct mlxsw_afa *afa;
> struct mlxsw_sp_acl *acl;
> struct mlxsw_sp_fid_core *fid_core;
> + struct mlxsw_sp_policer_core *policer_core;
> struct mlxsw_sp_kvdl *kvdl;
> struct mlxsw_sp_nve *nve;
> struct notifier_block netdevice_nb;
> @@ -173,6 +174,7 @@ struct mlxsw_sp {
> const struct mlxsw_sp_port_type_speed_ops *port_type_speed_ops;
> const struct mlxsw_sp_ptp_ops *ptp_ops;
> const struct mlxsw_sp_span_ops *span_ops;
> + const struct mlxsw_sp_policer_core_ops *policer_core_ops;
> const struct mlxsw_listener *listeners;
> size_t listeners_count;
> u32 lowest_shaper_bs;
> @@ -1196,4 +1198,34 @@ extern const struct ethtool_ops mlxsw_sp_port_ethtool_ops;
> extern const struct mlxsw_sp_port_type_speed_ops mlxsw_sp1_port_type_speed_ops;
> extern const struct mlxsw_sp_port_type_speed_ops mlxsw_sp2_port_type_speed_ops;
>
> +/* spectrum_policer.c */
> +extern const struct mlxsw_sp_policer_core_ops mlxsw_sp1_policer_core_ops;
> +extern const struct mlxsw_sp_policer_core_ops mlxsw_sp2_policer_core_ops;
> +
> +enum mlxsw_sp_policer_type {
> + MLXSW_SP_POLICER_TYPE_SINGLE_RATE,
> +
> + __MLXSW_SP_POLICER_TYPE_MAX,
> + MLXSW_SP_POLICER_TYPE_MAX = __MLXSW_SP_POLICER_TYPE_MAX - 1,
> +};
> +
> +struct mlxsw_sp_policer_params {
> + u64 rate;
> + u64 burst;
> + bool bytes;
> +};
> +
> +int mlxsw_sp_policer_add(struct mlxsw_sp *mlxsw_sp,
> + enum mlxsw_sp_policer_type type,
> + const struct mlxsw_sp_policer_params *params,
> + struct netlink_ext_ack *extack, u16 *p_policer_index);
> +void mlxsw_sp_policer_del(struct mlxsw_sp *mlxsw_sp,
> + enum mlxsw_sp_policer_type type,
> + u16 policer_index);
> +int mlxsw_sp_policer_drops_counter_get(struct mlxsw_sp *mlxsw_sp,
> + enum mlxsw_sp_policer_type type,
> + u16 policer_index, u64 *p_drops);
> +int mlxsw_sp_policers_init(struct mlxsw_sp *mlxsw_sp);
> +void mlxsw_sp_policers_fini(struct mlxsw_sp *mlxsw_sp);
> +
> #endif
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_policer.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_policer.c
> new file mode 100644
> index 000000000000..74766e936e0a
> --- /dev/null
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_policer.c
> @@ -0,0 +1,403 @@
> +// SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0
> +/* Copyright (c) 2020 Mellanox Technologies. All rights reserved */
> +
> +#include <linux/idr.h>
> +#include <linux/log2.h>
> +#include <linux/mutex.h>
> +#include <linux/netlink.h>
> +
> +#include "spectrum.h"
> +
> +struct mlxsw_sp_policer_family {
> + enum mlxsw_sp_policer_type type;
> + enum mlxsw_reg_qpcr_g qpcr_type;
> + struct mlxsw_sp *mlxsw_sp;
> + u16 start_index; /* Inclusive */
> + u16 end_index; /* Exclusive */
> + struct idr policer_idr;
> + struct mutex lock; /* Protects policer_idr */
> + const struct mlxsw_sp_policer_family_ops *ops;
> +};
> +
> +struct mlxsw_sp_policer {
> + struct mlxsw_sp_policer_params params;
> + u16 index;
> +};
> +
> +struct mlxsw_sp_policer_family_ops {
> + int (*init)(struct mlxsw_sp_policer_family *family);
> + void (*fini)(struct mlxsw_sp_policer_family *family);
> + int (*policer_index_alloc)(struct mlxsw_sp_policer_family *family,
> + struct mlxsw_sp_policer *policer);
> + struct mlxsw_sp_policer * (*policer_index_free)(struct mlxsw_sp_policer_family *family,
> + u16 policer_index);
> + int (*policer_init)(struct mlxsw_sp_policer_family *family,
> + const struct mlxsw_sp_policer *policer);
> + int (*policer_params_check)(const struct mlxsw_sp_policer_family *family,
> + const struct mlxsw_sp_policer_params *params,
> + struct netlink_ext_ack *extack);
> +};
> +
> +struct mlxsw_sp_policer_core {
> + struct mlxsw_sp_policer_family *family_arr[MLXSW_SP_POLICER_TYPE_MAX + 1];
> + const struct mlxsw_sp_policer_core_ops *ops;
> + u8 lowest_bs_bits;
> + u8 highest_bs_bits;
> +};
> +
> +struct mlxsw_sp_policer_core_ops {
> + int (*init)(struct mlxsw_sp_policer_core *policer_core);
> +};
> +
> +static u64 mlxsw_sp_policer_rate_bytes_ps_kbps(u64 rate_bytes_ps)
> +{
> + return div_u64(rate_bytes_ps, 1000) * BITS_PER_BYTE;
> +}
> +
> +static u8 mlxsw_sp_policer_burst_bytes_hw_units(u64 burst_bytes)
> +{
> + /* Provided burst size is in bytes. The ASIC burst size value is
> + * (2 ^ bs) * 512 bits. Convert the provided size to 512-bit units.
> + */
> + u64 bs512 = div_u64(burst_bytes, 64);
> +
> + if (!bs512)
> + return 0;
> +
> + return fls64(bs512) - 1;
> +}
> +
> +static int
> +mlxsw_sp_policer_single_rate_family_init(struct mlxsw_sp_policer_family *family)
> +{
> + struct mlxsw_core *core = family->mlxsw_sp->core;
> +
> + /* CPU policers are allocated from the first N policers in the global
> + * range, so skip them.
> + */
> + if (!MLXSW_CORE_RES_VALID(core, MAX_GLOBAL_POLICERS) ||
> + !MLXSW_CORE_RES_VALID(core, MAX_CPU_POLICERS))
> + return -EIO;
> +
> + family->start_index = MLXSW_CORE_RES_GET(core, MAX_CPU_POLICERS);
> + family->end_index = MLXSW_CORE_RES_GET(core, MAX_GLOBAL_POLICERS);
> +
> + return 0;
> +}
> +
> +static void
> +mlxsw_sp_policer_single_rate_family_fini(struct mlxsw_sp_policer_family *family)
> +{
> +}
> +
> +static int
> +mlxsw_sp_policer_single_rate_index_alloc(struct mlxsw_sp_policer_family *family,
> + struct mlxsw_sp_policer *policer)
> +{
> + int id;
> +
> + mutex_lock(&family->lock);
> + id = idr_alloc(&family->policer_idr, policer, family->start_index,
> + family->end_index, GFP_KERNEL);
> + mutex_unlock(&family->lock);
> +
> + if (id < 0)
> + return id;
> +
> + policer->index = id;
> +
> + return 0;
> +}
> +
> +static struct mlxsw_sp_policer *
> +mlxsw_sp_policer_single_rate_index_free(struct mlxsw_sp_policer_family *family,
> + u16 policer_index)
> +{
> + struct mlxsw_sp_policer *policer;
> +
> + mutex_lock(&family->lock);
> + policer = idr_remove(&family->policer_idr, policer_index);
> + mutex_unlock(&family->lock);
> +
> + WARN_ON(!policer);
> +
> + return policer;
> +}
> +
> +static int
> +mlxsw_sp_policer_single_rate_init(struct mlxsw_sp_policer_family *family,
> + const struct mlxsw_sp_policer *policer)
> +{
> + u64 rate_kbps = mlxsw_sp_policer_rate_bytes_ps_kbps(policer->params.rate);
> + u8 bs = mlxsw_sp_policer_burst_bytes_hw_units(policer->params.burst);
> + struct mlxsw_sp *mlxsw_sp = family->mlxsw_sp;
> + char qpcr_pl[MLXSW_REG_QPCR_LEN];
> +
> + mlxsw_reg_qpcr_pack(qpcr_pl, policer->index, MLXSW_REG_QPCR_IR_UNITS_K,
> + true, rate_kbps, bs);
> + mlxsw_reg_qpcr_clear_counter_set(qpcr_pl, true);
> +
> + return mlxsw_reg_write(mlxsw_sp->core, MLXSW_REG(qpcr), qpcr_pl);
> +}
> +
> +static int
> +mlxsw_sp_policer_single_rate_params_check(const struct mlxsw_sp_policer_family *family,
> + const struct mlxsw_sp_policer_params *params,
> + struct netlink_ext_ack *extack)
> +{
> + struct mlxsw_sp_policer_core *policer_core = family->mlxsw_sp->policer_core;
> + u64 rate_bps = params->rate * BITS_PER_BYTE;
> + u8 bs;
> +
> + if (!params->bytes) {
> + NL_SET_ERR_MSG_MOD(extack, "Only bandwidth policing is currently supported by single rate policers");
> + return -EINVAL;
> + }
> +
> + if (!is_power_of_2(params->burst)) {
> + NL_SET_ERR_MSG_MOD(extack, "Policer burst size is not power of two");
> + return -EINVAL;
> + }
> +
> + bs = mlxsw_sp_policer_burst_bytes_hw_units(params->burst);
> +
> + if (bs < policer_core->lowest_bs_bits) {
> + NL_SET_ERR_MSG_MOD(extack, "Policer burst size lower than limit");
> + return -EINVAL;
> + }
> +
> + if (bs > policer_core->highest_bs_bits) {
> + NL_SET_ERR_MSG_MOD(extack, "Policer burst size higher than limit");
> + return -EINVAL;
> + }
> +
> + if (rate_bps < MLXSW_REG_QPCR_LOWEST_CIR_BITS) {
> + NL_SET_ERR_MSG_MOD(extack, "Policer rate lower than limit");
> + return -EINVAL;
> + }
> +
> + if (rate_bps > MLXSW_REG_QPCR_HIGHEST_CIR_BITS) {
> + NL_SET_ERR_MSG_MOD(extack, "Policer rate higher than limit");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static const struct mlxsw_sp_policer_family_ops mlxsw_sp_policer_single_rate_ops = {
> + .init = mlxsw_sp_policer_single_rate_family_init,
> + .fini = mlxsw_sp_policer_single_rate_family_fini,
> + .policer_index_alloc = mlxsw_sp_policer_single_rate_index_alloc,
> + .policer_index_free = mlxsw_sp_policer_single_rate_index_free,
> + .policer_init = mlxsw_sp_policer_single_rate_init,
> + .policer_params_check = mlxsw_sp_policer_single_rate_params_check,
> +};
> +
> +static const struct mlxsw_sp_policer_family mlxsw_sp_policer_single_rate_family = {
> + .type = MLXSW_SP_POLICER_TYPE_SINGLE_RATE,
> + .qpcr_type = MLXSW_REG_QPCR_G_GLOBAL,
> + .ops = &mlxsw_sp_policer_single_rate_ops,
> +};
> +
> +static const struct mlxsw_sp_policer_family *mlxsw_sp_policer_family_arr[] = {
> + [MLXSW_SP_POLICER_TYPE_SINGLE_RATE] = &mlxsw_sp_policer_single_rate_family,
> +};
> +
> +int mlxsw_sp_policer_add(struct mlxsw_sp *mlxsw_sp,
> + enum mlxsw_sp_policer_type type,
> + const struct mlxsw_sp_policer_params *params,
> + struct netlink_ext_ack *extack, u16 *p_policer_index)
> +{
> + struct mlxsw_sp_policer_family *family;
> + struct mlxsw_sp_policer *policer;
> + int err;
> +
> + family = mlxsw_sp->policer_core->family_arr[type];
> +
> + err = family->ops->policer_params_check(family, params, extack);
> + if (err)
> + return err;
> +
> + policer = kmalloc(sizeof(*policer), GFP_KERNEL);
> + if (!policer)
> + return -ENOMEM;
> + policer->params = *params;
> +
> + err = family->ops->policer_index_alloc(family, policer);
> + if (err) {
> + NL_SET_ERR_MSG_MOD(extack, "Failed to allocate policer index");
> + goto err_policer_index_alloc;
> + }
> +
> + err = family->ops->policer_init(family, policer);
> + if (err) {
> + NL_SET_ERR_MSG_MOD(extack, "Failed to initialize policer");
> + goto err_policer_init;
> + }
> +
> + *p_policer_index = policer->index;
> +
> + return 0;
> +
> +err_policer_init:
> + family->ops->policer_index_free(family, policer->index);
> +err_policer_index_alloc:
> + kfree(policer);
> + return err;
> +}
> +
> +void mlxsw_sp_policer_del(struct mlxsw_sp *mlxsw_sp,
> + enum mlxsw_sp_policer_type type, u16 policer_index)
> +{
> + struct mlxsw_sp_policer_family *family;
> + struct mlxsw_sp_policer *policer;
> +
> + family = mlxsw_sp->policer_core->family_arr[type];
> + policer = family->ops->policer_index_free(family, policer_index);
> + kfree(policer);
> +}
> +
> +int mlxsw_sp_policer_drops_counter_get(struct mlxsw_sp *mlxsw_sp,
> + enum mlxsw_sp_policer_type type,
> + u16 policer_index, u64 *p_drops)
> +{
> + struct mlxsw_sp_policer_family *family;
> + char qpcr_pl[MLXSW_REG_QPCR_LEN];
> + int err;
> +
> + family = mlxsw_sp->policer_core->family_arr[type];
> +
> + MLXSW_REG_ZERO(qpcr, qpcr_pl);
> + mlxsw_reg_qpcr_pid_set(qpcr_pl, policer_index);
> + mlxsw_reg_qpcr_g_set(qpcr_pl, family->qpcr_type);
> + err = mlxsw_reg_query(mlxsw_sp->core, MLXSW_REG(qpcr), qpcr_pl);
> + if (err)
> + return err;
> +
> + *p_drops = mlxsw_reg_qpcr_violate_count_get(qpcr_pl);
> +
> + return 0;
> +}
> +
> +static int
> +mlxsw_sp_policer_family_register(struct mlxsw_sp *mlxsw_sp,
> + const struct mlxsw_sp_policer_family *tmpl)
> +{
> + struct mlxsw_sp_policer_family *family;
> + int err;
> +
> + family = kmemdup(tmpl, sizeof(*family), GFP_KERNEL);
> + if (!family)
> + return -ENOMEM;
> +
> + family->mlxsw_sp = mlxsw_sp;
> + idr_init(&family->policer_idr);
> + mutex_init(&family->lock);
> +
> + err = family->ops->init(family);
> + if (err)
> + goto err_family_init;
> +
> + if (WARN_ON(family->start_index >= family->end_index)) {
> + err = -EINVAL;
> + goto err_index_check;
> + }
> +
> + mlxsw_sp->policer_core->family_arr[tmpl->type] = family;
> +
> + return 0;
> +
> +err_index_check:
> + family->ops->fini(family);
> +err_family_init:
> + mutex_destroy(&family->lock);
> + idr_destroy(&family->policer_idr);
> + kfree(family);
> + return err;
> +}
> +
> +static void
> +mlxsw_sp_policer_family_unregister(struct mlxsw_sp *mlxsw_sp,
> + struct mlxsw_sp_policer_family *family)
> +{
> + family->ops->fini(family);
> + mutex_destroy(&family->lock);
> + WARN_ON(!idr_is_empty(&family->policer_idr));
> + idr_destroy(&family->policer_idr);
> + kfree(family);
> +}
> +
> +int mlxsw_sp_policers_init(struct mlxsw_sp *mlxsw_sp)
> +{
> + struct mlxsw_sp_policer_core *policer_core;
> + int i, err;
> +
> + policer_core = kzalloc(sizeof(*policer_core), GFP_KERNEL);
> + if (!policer_core)
> + return -ENOMEM;
> + mlxsw_sp->policer_core = policer_core;
> + policer_core->ops = mlxsw_sp->policer_core_ops;
> +
> + err = policer_core->ops->init(policer_core);
> + if (err)
> + goto err_init;
> +
> + for (i = 0; i < MLXSW_SP_POLICER_TYPE_MAX + 1; i++) {
> + err = mlxsw_sp_policer_family_register(mlxsw_sp, mlxsw_sp_policer_family_arr[i]);
> + if (err)
> + goto err_family_register;
> + }
> +
> + return 0;
> +
> +err_family_register:
> + for (i--; i >= 0; i--) {
> + struct mlxsw_sp_policer_family *family;
> +
> + family = mlxsw_sp->policer_core->family_arr[i];
> + mlxsw_sp_policer_family_unregister(mlxsw_sp, family);
> + }
> +err_init:
> + kfree(mlxsw_sp->policer_core);
> + return err;
> +}
> +
> +void mlxsw_sp_policers_fini(struct mlxsw_sp *mlxsw_sp)
> +{
> + int i;
> +
> + for (i = MLXSW_SP_POLICER_TYPE_MAX; i >= 0; i--) {
> + struct mlxsw_sp_policer_family *family;
> +
> + family = mlxsw_sp->policer_core->family_arr[i];
> + mlxsw_sp_policer_family_unregister(mlxsw_sp, family);
> + }
> +
> + kfree(mlxsw_sp->policer_core);
> +}
> +
> +static int
> +mlxsw_sp1_policer_core_init(struct mlxsw_sp_policer_core *policer_core)
> +{
> + policer_core->lowest_bs_bits = MLXSW_REG_QPCR_LOWEST_CBS_BITS_SP1;
> + policer_core->highest_bs_bits = MLXSW_REG_QPCR_HIGHEST_CBS_BITS_SP1;
> +
> + return 0;
> +}
> +
> +const struct mlxsw_sp_policer_core_ops mlxsw_sp1_policer_core_ops = {
> + .init = mlxsw_sp1_policer_core_init,
> +};
> +
> +static int
> +mlxsw_sp2_policer_core_init(struct mlxsw_sp_policer_core *policer_core)
> +{
> + policer_core->lowest_bs_bits = MLXSW_REG_QPCR_LOWEST_CBS_BITS_SP2;
> + policer_core->highest_bs_bits = MLXSW_REG_QPCR_HIGHEST_CBS_BITS_SP2;
> +
> + return 0;
> +}
> +
> +const struct mlxsw_sp_policer_core_ops mlxsw_sp2_policer_core_ops = {
> + .init = mlxsw_sp2_policer_core_init,
> +};
> --
> 2.26.2
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 03/11] mlxsw: spectrum_policer: Add policer core
2020-08-17 15:38 ` Bjorn Helgaas
@ 2020-08-18 6:41 ` Ido Schimmel
2020-08-18 9:37 ` Petr Machata
0 siblings, 1 reply; 17+ messages in thread
From: Ido Schimmel @ 2020-08-18 6:41 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: netdev, davem, kuba, jiri, petrm, mlxsw, Ido Schimmel
On Mon, Aug 17, 2020 at 10:38:24AM -0500, Bjorn Helgaas wrote:
> You've likely seen this already, but Coverity found this problem:
>
> *** CID 1466147: Control flow issues (DEADCODE)
> /drivers/net/ethernet/mellanox/mlxsw/spectrum_policer.c: 380 in mlxsw_sp_policers_init()
> 374 }
> 375
> 376 return 0;
> 377
> 378 err_family_register:
> 379 for (i--; i >= 0; i--) {
> >>> CID 1466147: Control flow issues (DEADCODE)
> >>> Execution cannot reach this statement: "struct mlxsw_sp_policer_fam...".
> 380 struct mlxsw_sp_policer_family *family;
> 381
> 382 family = mlxsw_sp->policer_core->family_arr[i];
> 383 mlxsw_sp_policer_family_unregister(mlxsw_sp, family);
> 384 }
> 385 err_init:
>
> I think the problem is that MLXSW_SP_POLICER_TYPE_MAX is 0 because
>
> > +enum mlxsw_sp_policer_type {
> > + MLXSW_SP_POLICER_TYPE_SINGLE_RATE,
> > +
> > + __MLXSW_SP_POLICER_TYPE_MAX,
> > + MLXSW_SP_POLICER_TYPE_MAX = __MLXSW_SP_POLICER_TYPE_MAX - 1,
> > +};
>
> so we can only execute the family_register loop once, with i == 0,
> and if we get to err_family_register via the error exit:
>
> > + for (i = 0; i < MLXSW_SP_POLICER_TYPE_MAX + 1; i++) {
> > + err = mlxsw_sp_policer_family_register(mlxsw_sp, mlxsw_sp_policer_family_arr[i]);
> > + if (err)
> > + goto err_family_register;
>
> i will be 0, so i-- sets i to -1, so we don't enter the
> family_unregister loop body since -1 is not >= 0.
Thanks for the report, but isn't the code doing the right thing here? I
mean, it's dead code now, but as soon as we add another family it will
be executed. It seems error prone to remove it only to please Coverity
and then add it back when it's actually needed.
>
> This code is now upstream as 8d3fbae70d8d ("mlxsw: spectrum_policer:
> Add policer core").
>
> Bjorn
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 03/11] mlxsw: spectrum_policer: Add policer core
2020-08-18 6:41 ` Ido Schimmel
@ 2020-08-18 9:37 ` Petr Machata
2020-08-18 12:10 ` Bjorn Helgaas
0 siblings, 1 reply; 17+ messages in thread
From: Petr Machata @ 2020-08-18 9:37 UTC (permalink / raw)
To: Ido Schimmel
Cc: Bjorn Helgaas, netdev, davem, kuba, jiri, mlxsw, Ido Schimmel
Ido Schimmel <idosch@idosch.org> writes:
> On Mon, Aug 17, 2020 at 10:38:24AM -0500, Bjorn Helgaas wrote:
>> You've likely seen this already, but Coverity found this problem:
>>
>> *** CID 1466147: Control flow issues (DEADCODE)
>> /drivers/net/ethernet/mellanox/mlxsw/spectrum_policer.c: 380 in mlxsw_sp_policers_init()
>> 374 }
>> 375
>> 376 return 0;
>> 377
>> 378 err_family_register:
>> 379 for (i--; i >= 0; i--) {
>> >>> CID 1466147: Control flow issues (DEADCODE)
>> >>> Execution cannot reach this statement: "struct mlxsw_sp_policer_fam...".
>> 380 struct mlxsw_sp_policer_family *family;
>> 381
>> 382 family = mlxsw_sp->policer_core->family_arr[i];
>> 383 mlxsw_sp_policer_family_unregister(mlxsw_sp, family);
>> 384 }
>> 385 err_init:
>>
>> I think the problem is that MLXSW_SP_POLICER_TYPE_MAX is 0 because
>>
>> > +enum mlxsw_sp_policer_type {
>> > + MLXSW_SP_POLICER_TYPE_SINGLE_RATE,
>> > +
>> > + __MLXSW_SP_POLICER_TYPE_MAX,
>> > + MLXSW_SP_POLICER_TYPE_MAX = __MLXSW_SP_POLICER_TYPE_MAX - 1,
>> > +};
>>
>> so we can only execute the family_register loop once, with i == 0,
>> and if we get to err_family_register via the error exit:
>>
>> > + for (i = 0; i < MLXSW_SP_POLICER_TYPE_MAX + 1; i++) {
>> > + err = mlxsw_sp_policer_family_register(mlxsw_sp, mlxsw_sp_policer_family_arr[i]);
>> > + if (err)
>> > + goto err_family_register;
>>
>> i will be 0, so i-- sets i to -1, so we don't enter the
>> family_unregister loop body since -1 is not >= 0.
>
> Thanks for the report, but isn't the code doing the right thing here? I
> mean, it's dead code now, but as soon as we add another family it will
> be executed. It seems error prone to remove it only to please Coverity
> and then add it back when it's actually needed.
Agreed.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 03/11] mlxsw: spectrum_policer: Add policer core
2020-08-18 9:37 ` Petr Machata
@ 2020-08-18 12:10 ` Bjorn Helgaas
0 siblings, 0 replies; 17+ messages in thread
From: Bjorn Helgaas @ 2020-08-18 12:10 UTC (permalink / raw)
To: Petr Machata; +Cc: Ido Schimmel, netdev, davem, kuba, jiri, mlxsw, Ido Schimmel
On Tue, Aug 18, 2020 at 11:37:45AM +0200, Petr Machata wrote:
> Ido Schimmel <idosch@idosch.org> writes:
> > On Mon, Aug 17, 2020 at 10:38:24AM -0500, Bjorn Helgaas wrote:
> >> You've likely seen this already, but Coverity found this problem:
> >>
> >> *** CID 1466147: Control flow issues (DEADCODE)
> >> /drivers/net/ethernet/mellanox/mlxsw/spectrum_policer.c: 380 in mlxsw_sp_policers_init()
> >> 374 }
> >> 375
> >> 376 return 0;
> >> 377
> >> 378 err_family_register:
> >> 379 for (i--; i >= 0; i--) {
> >> >>> CID 1466147: Control flow issues (DEADCODE)
> >> >>> Execution cannot reach this statement: "struct mlxsw_sp_policer_fam...".
> >> 380 struct mlxsw_sp_policer_family *family;
> >> 381
> >> 382 family = mlxsw_sp->policer_core->family_arr[i];
> >> 383 mlxsw_sp_policer_family_unregister(mlxsw_sp, family);
> >> 384 }
> >> 385 err_init:
> >>
> >> I think the problem is that MLXSW_SP_POLICER_TYPE_MAX is 0 because
> >>
> >> > +enum mlxsw_sp_policer_type {
> >> > + MLXSW_SP_POLICER_TYPE_SINGLE_RATE,
> >> > +
> >> > + __MLXSW_SP_POLICER_TYPE_MAX,
> >> > + MLXSW_SP_POLICER_TYPE_MAX = __MLXSW_SP_POLICER_TYPE_MAX - 1,
> >> > +};
> >>
> >> so we can only execute the family_register loop once, with i == 0,
> >> and if we get to err_family_register via the error exit:
> >>
> >> > + for (i = 0; i < MLXSW_SP_POLICER_TYPE_MAX + 1; i++) {
> >> > + err = mlxsw_sp_policer_family_register(mlxsw_sp, mlxsw_sp_policer_family_arr[i]);
> >> > + if (err)
> >> > + goto err_family_register;
> >>
> >> i will be 0, so i-- sets i to -1, so we don't enter the
> >> family_unregister loop body since -1 is not >= 0.
> >
> > Thanks for the report, but isn't the code doing the right thing here? I
> > mean, it's dead code now, but as soon as we add another family it will
> > be executed. It seems error prone to remove it only to please Coverity
> > and then add it back when it's actually needed.
>
> Agreed.
You're right, I missed the forest for the trees. Sorry for the noise.
Bjorn
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2020-08-18 12:10 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-15 8:27 [PATCH net-next 00/11] mlxsw: Offload tc police action Ido Schimmel
2020-07-15 8:27 ` [PATCH net-next 01/11] mlxsw: reg: Add policer bandwidth limits Ido Schimmel
2020-07-15 8:27 ` [PATCH net-next 02/11] mlxsw: resources: Add resource identifier for global policers Ido Schimmel
2020-07-15 8:27 ` [PATCH net-next 03/11] mlxsw: spectrum_policer: Add policer core Ido Schimmel
2020-08-17 15:38 ` Bjorn Helgaas
2020-08-18 6:41 ` Ido Schimmel
2020-08-18 9:37 ` Petr Machata
2020-08-18 12:10 ` Bjorn Helgaas
2020-07-15 8:27 ` [PATCH net-next 04/11] mlxsw: spectrum_policer: Add devlink resource support Ido Schimmel
2020-07-15 8:27 ` [PATCH net-next 05/11] mlxsw: core_acl_flex_actions: Work around hardware limitation Ido Schimmel
2020-07-15 8:27 ` [PATCH net-next 06/11] mlxsw: core_acl_flex_actions: Add police action Ido Schimmel
2020-07-15 8:27 ` [PATCH net-next 07/11] mlxsw: spectrum_acl: Offload FLOW_ACTION_POLICE Ido Schimmel
2020-07-15 8:27 ` [PATCH net-next 08/11] selftests: forwarding: Add tc-police tests Ido Schimmel
2020-07-15 8:27 ` [PATCH net-next 09/11] selftests: mlxsw: tc_restrictions: Test tc-police restrictions Ido Schimmel
2020-07-15 8:27 ` [PATCH net-next 10/11] selftests: mlxsw: Add scale test for tc-police Ido Schimmel
2020-07-15 8:27 ` [PATCH net-next 11/11] selftests: mlxsw: Test policers' occupancy Ido Schimmel
2020-07-16 1:13 ` [PATCH net-next 00/11] mlxsw: Offload tc police action Jakub Kicinski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).