Re: [PATCH net-next 2/2] net: marvell: prestera: Add matchall support

From: Vladimir Oltean
Date: Tue Jun 15 2021 - 21:07:56 EST

Next message: liuqi (BA): "Re: [PATCH v6 2/2] drivers/perf: hisi: Add driver for HiSilicon PCIe PMU"
Previous message: libaokun (A): "Re: [PATCH -next] ide-cd: fix warning: variable 'stat' set but not used"
In reply to: Vadym Kochan: "[PATCH net-next 2/2] net: marvell: prestera: Add matchall support"
Next in thread: David Miller: "Re: [PATCH net-next 0/2] Marvell Prestera add flower and match all support"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Tue, Jun 15, 2021 at 03:54:44PM +0300, Vadym Kochan wrote:
> From: Serhiy Boiko <serhiy.boiko@xxxxxxxxxxx>
>
> - Introduce matchall filter support
> - Add SPAN API to configure port mirroring.
> - Add tc mirror action.
>
> At this moment, only mirror (egress) action is supported.
>
> Example:
> tc filter ... action mirred egress mirror dev DEV
>
> Co-developed-by: Volodymyr Mytnyk <vmytnyk@xxxxxxxxxxx>
> Signed-off-by: Volodymyr Mytnyk <vmytnyk@xxxxxxxxxxx>
> Signed-off-by: Serhiy Boiko <serhiy.boiko@xxxxxxxxxxx>
> Signed-off-by: Vadym Kochan <vkochan@xxxxxxxxxxx>
> ---
> .../net/ethernet/marvell/prestera/Makefile | 2 +-
> .../net/ethernet/marvell/prestera/prestera.h | 2 +
> .../ethernet/marvell/prestera/prestera_acl.c | 2 +
> .../ethernet/marvell/prestera/prestera_acl.h | 3 +-
> .../ethernet/marvell/prestera/prestera_flow.c | 19 ++
> .../ethernet/marvell/prestera/prestera_hw.c | 69 +++++
> .../ethernet/marvell/prestera/prestera_hw.h | 6 +
> .../ethernet/marvell/prestera/prestera_main.c | 8 +
> .../ethernet/marvell/prestera/prestera_span.c | 245 ++++++++++++++++++
> .../ethernet/marvell/prestera/prestera_span.h | 20 ++
> 10 files changed, 374 insertions(+), 2 deletions(-)
> create mode 100644 drivers/net/ethernet/marvell/prestera/prestera_span.c
> create mode 100644 drivers/net/ethernet/marvell/prestera/prestera_span.h
>
> diff --git a/drivers/net/ethernet/marvell/prestera/Makefile b/drivers/net/ethernet/marvell/prestera/Makefile
> index 42327c4afdbf..0609df8b913d 100644
> --- a/drivers/net/ethernet/marvell/prestera/Makefile
> +++ b/drivers/net/ethernet/marvell/prestera/Makefile
> @@ -3,6 +3,6 @@ obj-$(CONFIG_PRESTERA) += prestera.o
> prestera-objs := prestera_main.o prestera_hw.o prestera_dsa.o \
> prestera_rxtx.o prestera_devlink.o prestera_ethtool.o \
> prestera_switchdev.o prestera_acl.o prestera_flow.o \
> - prestera_flower.o
> + prestera_flower.o prestera_span.o
>
> obj-$(CONFIG_PRESTERA_PCI) += prestera_pci.o
> diff --git a/drivers/net/ethernet/marvell/prestera/prestera.h b/drivers/net/ethernet/marvell/prestera/prestera.h
> index bbbe780d0886..f18fe664b373 100644
> --- a/drivers/net/ethernet/marvell/prestera/prestera.h
> +++ b/drivers/net/ethernet/marvell/prestera/prestera.h
> @@ -172,6 +172,7 @@ struct prestera_event {
> };
>
> struct prestera_switchdev;
> +struct prestera_span;
> struct prestera_rxtx;
> struct prestera_trap_data;
> struct prestera_acl;
> @@ -181,6 +182,7 @@ struct prestera_switch {
> struct prestera_switchdev *swdev;
> struct prestera_rxtx *rxtx;
> struct prestera_acl *acl;
> + struct prestera_span *span;
> struct list_head event_handlers;
> struct notifier_block netdev_nb;
> struct prestera_trap_data *trap_data;
> diff --git a/drivers/net/ethernet/marvell/prestera/prestera_acl.c b/drivers/net/ethernet/marvell/prestera/prestera_acl.c
> index 817f78b1e90c..5165ac96a70e 100644
> --- a/drivers/net/ethernet/marvell/prestera/prestera_acl.c
> +++ b/drivers/net/ethernet/marvell/prestera/prestera_acl.c
> @@ -6,6 +6,7 @@
> #include "prestera.h"
> #include "prestera_hw.h"
> #include "prestera_acl.h"
> +#include "prestera_span.h"
>
> struct prestera_acl {
> struct prestera_switch *sw;
> @@ -149,6 +150,7 @@ int prestera_acl_block_bind(struct prestera_flow_block *block,
> binding = kzalloc(sizeof(*binding), GFP_KERNEL);
> if (!binding)
> return -ENOMEM;
> + binding->span_id = PRESTERA_SPAN_INVALID_ID;
> binding->port = port;
>
> err = prestera_hw_acl_port_bind(port, block->ruleset->id);
> diff --git a/drivers/net/ethernet/marvell/prestera/prestera_acl.h b/drivers/net/ethernet/marvell/prestera/prestera_acl.h
> index 935c79a26036..2f129eb55547 100644
> --- a/drivers/net/ethernet/marvell/prestera/prestera_acl.h
> +++ b/drivers/net/ethernet/marvell/prestera/prestera_acl.h
> @@ -28,14 +28,15 @@ enum prestera_acl_rule_action {
> PRESTERA_ACL_RULE_ACTION_TRAP
> };
>
> -struct prestera_switch;
> struct prestera_port;
> +struct prestera_switch;

This doesn't appear to be moved for a good reason.

> struct prestera_acl_rule;
> struct prestera_acl_ruleset;
>
> struct prestera_flow_block_binding {
> struct list_head list;
> struct prestera_port *port;
> + int span_id;
> };
>
> struct prestera_flow_block {
> diff --git a/drivers/net/ethernet/marvell/prestera/prestera_flow.c b/drivers/net/ethernet/marvell/prestera/prestera_flow.c
> index b818dd871512..b350082a1815 100644
> --- a/drivers/net/ethernet/marvell/prestera/prestera_flow.c
> +++ b/drivers/net/ethernet/marvell/prestera/prestera_flow.c
> @@ -7,10 +7,25 @@
> #include "prestera.h"
> #include "prestera_acl.h"
> #include "prestera_flow.h"
> +#include "prestera_span.h"
> #include "prestera_flower.h"
>
> static LIST_HEAD(prestera_block_cb_list);
>
> +static int prestera_flow_block_mall_cb(struct prestera_flow_block *block,
> + struct tc_cls_matchall_offload *f)
> +{
> + switch (f->command) {
> + case TC_CLSMATCHALL_REPLACE:
> + return prestera_span_replace(block, f);
> + case TC_CLSMATCHALL_DESTROY:
> + prestera_span_destroy(block);
> + return 0;
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> static int prestera_flow_block_flower_cb(struct prestera_flow_block *block,
> struct flow_cls_offload *f)
> {
> @@ -41,6 +56,8 @@ static int prestera_flow_block_cb(enum tc_setup_type type,
> switch (type) {
> case TC_SETUP_CLSFLOWER:
> return prestera_flow_block_flower_cb(block, type_data);
> + case TC_SETUP_CLSMATCHALL:
> + return prestera_flow_block_mall_cb(block, type_data);
> default:
> return -EOPNOTSUPP;
> }
> @@ -163,6 +180,8 @@ static void prestera_setup_flow_block_unbind(struct prestera_port *port,
> if (!tc_can_offload(port->dev))
> prestera_acl_block_disable_dec(block);
>
> + prestera_span_destroy(block);
> +
> err = prestera_acl_block_unbind(block, port);
> if (err)
> goto error;
> diff --git a/drivers/net/ethernet/marvell/prestera/prestera_hw.c b/drivers/net/ethernet/marvell/prestera/prestera_hw.c
> index 42b8d9f56468..c1297859e471 100644
> --- a/drivers/net/ethernet/marvell/prestera/prestera_hw.c
> +++ b/drivers/net/ethernet/marvell/prestera/prestera_hw.c
> @@ -56,6 +56,11 @@ enum prestera_cmd_type_t {
>
> PRESTERA_CMD_TYPE_STP_PORT_SET = 0x1000,
>
> + PRESTERA_CMD_TYPE_SPAN_GET = 0x1100,
> + PRESTERA_CMD_TYPE_SPAN_BIND = 0x1101,
> + PRESTERA_CMD_TYPE_SPAN_UNBIND = 0x1102,
> + PRESTERA_CMD_TYPE_SPAN_RELEASE = 0x1103,
> +
> PRESTERA_CMD_TYPE_CPU_CODE_COUNTERS_GET = 0x2000,
>
> PRESTERA_CMD_TYPE_ACK = 0x10000,
> @@ -377,6 +382,18 @@ struct prestera_msg_acl_ruleset_resp {
> u16 id;
> };
>
> +struct prestera_msg_span_req {
> + struct prestera_msg_cmd cmd;
> + u32 port;
> + u32 dev;
> + u8 id;
> +} __packed __aligned(4);
> +
> +struct prestera_msg_span_resp {
> + struct prestera_msg_ret ret;
> + u8 id;
> +} __packed __aligned(4);
> +
> struct prestera_msg_stp_req {
> struct prestera_msg_cmd cmd;
> u32 port;
> @@ -1055,6 +1072,58 @@ int prestera_hw_acl_port_unbind(const struct prestera_port *port,
> &req.cmd, sizeof(req));
> }
>
> +int prestera_hw_span_get(const struct prestera_port *port, u8 *span_id)
> +{
> + struct prestera_msg_span_resp resp;
> + struct prestera_msg_span_req req = {
> + .port = port->hw_id,
> + .dev = port->dev_id,
> + };
> + int err;
> +
> + err = prestera_cmd_ret(port->sw, PRESTERA_CMD_TYPE_SPAN_GET,
> + &req.cmd, sizeof(req), &resp.ret, sizeof(resp));
> + if (err)
> + return err;
> +
> + *span_id = resp.id;
> +
> + return 0;
> +}
> +
> +int prestera_hw_span_bind(const struct prestera_port *port, u8 span_id)
> +{
> + struct prestera_msg_span_req req = {
> + .port = port->hw_id,
> + .dev = port->dev_id,
> + .id = span_id,
> + };
> +
> + return prestera_cmd(port->sw, PRESTERA_CMD_TYPE_SPAN_BIND,
> + &req.cmd, sizeof(req));
> +}
> +
> +int prestera_hw_span_unbind(const struct prestera_port *port)
> +{
> + struct prestera_msg_span_req req = {
> + .port = port->hw_id,
> + .dev = port->dev_id,
> + };
> +
> + return prestera_cmd(port->sw, PRESTERA_CMD_TYPE_SPAN_UNBIND,
> + &req.cmd, sizeof(req));
> +}
> +
> +int prestera_hw_span_release(struct prestera_switch *sw, u8 span_id)
> +{
> + struct prestera_msg_span_req req = {
> + .id = span_id
> + };
> +
> + return prestera_cmd(sw, PRESTERA_CMD_TYPE_SPAN_RELEASE,
> + &req.cmd, sizeof(req));
> +}
> +
> int prestera_hw_port_type_get(const struct prestera_port *port, u8 *type)
> {
> struct prestera_msg_port_attr_req req = {
> diff --git a/drivers/net/ethernet/marvell/prestera/prestera_hw.h b/drivers/net/ethernet/marvell/prestera/prestera_hw.h
> index c01d376574d2..546d5fd8240d 100644
> --- a/drivers/net/ethernet/marvell/prestera/prestera_hw.h
> +++ b/drivers/net/ethernet/marvell/prestera/prestera_hw.h
> @@ -188,6 +188,12 @@ int prestera_hw_acl_port_bind(const struct prestera_port *port,
> int prestera_hw_acl_port_unbind(const struct prestera_port *port,
> u16 ruleset_id);
>
> +/* SPAN API */
> +int prestera_hw_span_get(const struct prestera_port *port, u8 *span_id);
> +int prestera_hw_span_bind(const struct prestera_port *port, u8 span_id);
> +int prestera_hw_span_unbind(const struct prestera_port *port);
> +int prestera_hw_span_release(struct prestera_switch *sw, u8 span_id);
> +
> /* Event handlers */
> int prestera_hw_event_handler_register(struct prestera_switch *sw,
> enum prestera_event_type type,
> diff --git a/drivers/net/ethernet/marvell/prestera/prestera_main.c b/drivers/net/ethernet/marvell/prestera/prestera_main.c
> index ea683b5a8a2e..d2a738da368a 100644
> --- a/drivers/net/ethernet/marvell/prestera/prestera_main.c
> +++ b/drivers/net/ethernet/marvell/prestera/prestera_main.c
> @@ -14,6 +14,7 @@
> #include "prestera_hw.h"
> #include "prestera_acl.h"
> #include "prestera_flow.h"
> +#include "prestera_span.h"
> #include "prestera_rxtx.h"
> #include "prestera_devlink.h"
> #include "prestera_ethtool.h"
> @@ -909,6 +910,10 @@ static int prestera_switch_init(struct prestera_switch *sw)
> if (err)
> goto err_acl_init;
>
> + err = prestera_span_init(sw);
> + if (err)
> + goto err_span_init;
> +
> err = prestera_devlink_register(sw);
> if (err)
> goto err_dl_register;
> @@ -928,6 +933,8 @@ static int prestera_switch_init(struct prestera_switch *sw)
> err_lag_init:
> prestera_devlink_unregister(sw);
> err_dl_register:
> + prestera_span_fini(sw);
> +err_span_init:
> prestera_acl_fini(sw);
> err_acl_init:
> prestera_event_handlers_unregister(sw);
> @@ -947,6 +954,7 @@ static void prestera_switch_fini(struct prestera_switch *sw)
> prestera_destroy_ports(sw);
> prestera_lag_fini(sw);
> prestera_devlink_unregister(sw);
> + prestera_span_fini(sw);
> prestera_acl_fini(sw);
> prestera_event_handlers_unregister(sw);
> prestera_rxtx_switch_fini(sw);
> diff --git a/drivers/net/ethernet/marvell/prestera/prestera_span.c b/drivers/net/ethernet/marvell/prestera/prestera_span.c
> new file mode 100644
> index 000000000000..d399e47fb429
> --- /dev/null
> +++ b/drivers/net/ethernet/marvell/prestera/prestera_span.c
> @@ -0,0 +1,245 @@
> +// SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0
> +/* Copyright (c) 2020 Marvell International Ltd. All rights reserved */
> +
> +#include <linux/kernel.h>
> +#include <linux/list.h>
> +
> +#include "prestera.h"
> +#include "prestera_hw.h"
> +#include "prestera_acl.h"
> +#include "prestera_span.h"
> +
> +struct prestera_span_entry {
> + struct list_head list;
> + struct prestera_port *port;
> + refcount_t ref_count;
> + u8 id;
> +};
> +
> +struct prestera_span {
> + struct prestera_switch *sw;
> + struct list_head entries;
> +};
> +
> +static struct prestera_span_entry *
> +prestera_span_entry_create(struct prestera_port *port, u8 span_id)
> +{
> + struct prestera_span_entry *entry;
> +
> + entry = kzalloc(sizeof(*entry), GFP_KERNEL);
> + if (!entry)
> + return ERR_PTR(-ENOMEM);
> +
> + refcount_set(&entry->ref_count, 1);
> + entry->port = port;
> + entry->id = span_id;
> + list_add_tail(&entry->list, &port->sw->span->entries);
> +
> + return entry;
> +}
> +
> +static void prestera_span_entry_del(struct prestera_span_entry *entry)
> +{
> + list_del(&entry->list);
> + kfree(entry);
> +}
> +
> +static struct prestera_span_entry *
> +prestera_span_entry_find_by_id(struct prestera_span *span, u8 span_id)
> +{
> + struct prestera_span_entry *entry;
> +
> + list_for_each_entry(entry, &span->entries, list) {
> + if (entry->id == span_id)
> + return entry;
> + }
> +
> + return NULL;
> +}
> +
> +static struct prestera_span_entry *
> +prestera_span_entry_find_by_port(struct prestera_span *span,
> + struct prestera_port *port)
> +{
> + struct prestera_span_entry *entry;
> +
> + list_for_each_entry(entry, &span->entries, list) {
> + if (entry->port == port)
> + return entry;
> + }
> +
> + return NULL;
> +}
> +
> +static int prestera_span_get(struct prestera_port *port, u8 *span_id)
> +{
> + u8 new_span_id;
> + struct prestera_switch *sw = port->sw;
> + struct prestera_span_entry *entry;
> + int err;
> +
> + entry = prestera_span_entry_find_by_port(sw->span, port);
> + if (entry) {
> + refcount_inc(&entry->ref_count);
> + *span_id = entry->id;
> + return 0;
> + }
> +
> + err = prestera_hw_span_get(port, &new_span_id);
> + if (err)
> + return err;
> +
> + entry = prestera_span_entry_create(port, new_span_id);
> + if (IS_ERR(entry)) {
> + prestera_hw_span_release(sw, new_span_id);
> + return PTR_ERR(entry);
> + }
> +
> + *span_id = new_span_id;
> + return 0;
> +}
> +
> +static int prestera_span_put(struct prestera_switch *sw, u8 span_id)
> +{
> + struct prestera_span_entry *entry;
> + int err;
> +
> + entry = prestera_span_entry_find_by_id(sw->span, span_id);
> + if (!entry)
> + return false;
> +
> + if (!refcount_dec_and_test(&entry->ref_count))
> + return 0;
> +
> + err = prestera_hw_span_release(sw, span_id);
> + if (err)
> + return err;
> +
> + prestera_span_entry_del(entry);
> + return 0;
> +}
> +
> +static int prestera_span_rule_add(struct prestera_flow_block_binding *binding,
> + struct prestera_port *to_port)
> +{
> + int err;
> + u8 span_id;
> + struct prestera_switch *sw = binding->port->sw;

Networking coding style is to declare variables in reverse order of line length (similar for prestera_span_get).

> +
> + if (binding->span_id != PRESTERA_SPAN_INVALID_ID)
> + /* port already in mirroring */
> + return -EEXIST;
> +
> + err = prestera_span_get(to_port, &span_id);
> + if (err)
> + return err;
> +
> + err = prestera_hw_span_bind(binding->port, span_id);
> + if (err) {
> + prestera_span_put(sw, span_id);
> + return err;
> + }
> +
> + binding->span_id = span_id;
> + return 0;
> +}
> +
> +static int prestera_span_rule_del(struct prestera_flow_block_binding *binding)
> +{
> + int err;
> +
> + err = prestera_hw_span_unbind(binding->port);
> + if (err)
> + return err;
> +
> + err = prestera_span_put(binding->port->sw, binding->span_id);
> + if (err)
> + return err;
> +
> + binding->span_id = PRESTERA_SPAN_INVALID_ID;
> + return 0;
> +}
> +
> +int prestera_span_replace(struct prestera_flow_block *block,
> + struct tc_cls_matchall_offload *f)
> +{
> + struct prestera_flow_block_binding *binding;
> + __be16 protocol = f->common.protocol;
> + struct flow_action_entry *act;
> + struct prestera_port *port;
> + int err;
> +
> + if (!flow_offload_has_one_action(&f->rule->action)) {
> + NL_SET_ERR_MSG(f->common.extack,
> + "Only singular actions are supported");
> + return -EOPNOTSUPP;
> + }
> +
> + if (f->common.chain_index) {
> + NL_SET_ERR_MSG(f->common.extack, "Only chain 0 is supported");
> + return -EOPNOTSUPP;
> + }

I wonder if you could just use a single tc_cls_can_offload_and_chain0()
in the TC_SETUP_BLOCK callback?

> +
> + act = &f->rule->action.entries[0];
> +
> + if (act->id != FLOW_ACTION_MIRRED)
> + return -EOPNOTSUPP;
> +
> + if (protocol != htons(ETH_P_ALL))
> + return -EOPNOTSUPP;
> +
> + /* TODO: add prio check */

What prio check?

> +
> + if (!prestera_netdev_check(act->dev)) {
> + NL_SET_ERR_MSG(f->common.extack,
> + "Only switchdev port is supported");

Actually only a Marvell Prestera port is supported. Switchdev is a much broader category.

> + return -EINVAL;
> + }
> +
> + port = netdev_priv(act->dev);
> + list_for_each_entry(binding, &block->binding_list, list) {
> + err = prestera_span_rule_add(binding, port);
> + if (err)
> + goto rollback;
> + }
> + return 0;
> +
> +rollback:
> + list_for_each_entry_continue_reverse(binding,
> + &block->binding_list, list)
> + prestera_span_rule_del(binding);
> + return err;
> +}
> +
> +void prestera_span_destroy(struct prestera_flow_block *block)
> +{
> + struct prestera_flow_block_binding *binding;
> +
> + list_for_each_entry(binding, &block->binding_list, list)
> + prestera_span_rule_del(binding);
> +}
> +
> +int prestera_span_init(struct prestera_switch *sw)
> +{
> + struct prestera_span *span;
> +
> + span = kzalloc(sizeof(*span), GFP_KERNEL);
> + if (!span)
> + return -ENOMEM;
> +
> + INIT_LIST_HEAD(&span->entries);
> +
> + sw->span = span;
> + span->sw = sw;
> +
> + return 0;
> +}
> +
> +void prestera_span_fini(struct prestera_switch *sw)
> +{
> + struct prestera_span *span = sw->span;
> +
> + WARN_ON(!list_empty(&span->entries));
> + kfree(span);
> +}
> +
> diff --git a/drivers/net/ethernet/marvell/prestera/prestera_span.h b/drivers/net/ethernet/marvell/prestera/prestera_span.h
> new file mode 100644
> index 000000000000..f0644521f78a
> --- /dev/null
> +++ b/drivers/net/ethernet/marvell/prestera/prestera_span.h
> @@ -0,0 +1,20 @@
> +/* SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0 */
> +/* Copyright (c) 2019-2020 Marvell International Ltd. All rights reserved. */
> +
> +#ifndef _PRESTERA_SPAN_H_
> +#define _PRESTERA_SPAN_H_
> +
> +#include <net/pkt_cls.h>
> +
> +#define PRESTERA_SPAN_INVALID_ID -1
> +
> +struct prestera_switch;
> +struct prestera_flow_block;
> +
> +int prestera_span_init(struct prestera_switch *sw);
> +void prestera_span_fini(struct prestera_switch *sw);
> +int prestera_span_replace(struct prestera_flow_block *block,
> + struct tc_cls_matchall_offload *f);
> +void prestera_span_destroy(struct prestera_flow_block *block);
> +
> +#endif /* _PRESTERA_SPAN_H_ */
> --
> 2.17.1
>

Next message: liuqi (BA): "Re: [PATCH v6 2/2] drivers/perf: hisi: Add driver for HiSilicon PCIe PMU"
Previous message: libaokun (A): "Re: [PATCH -next] ide-cd: fix warning: variable 'stat' set but not used"
In reply to: Vadym Kochan: "[PATCH net-next 2/2] net: marvell: prestera: Add matchall support"
Next in thread: David Miller: "Re: [PATCH net-next 0/2] Marvell Prestera add flower and match all support"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]