Skip to contentSkip to conversation

sfc: bare bones TC offload

PATCH Applied [email protected] 5 participants 19 messages Sep 26, 2022 – Oct 3, 2022

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

v2 cover sfc: bare bones TC offload

From: Edward Cree <[email protected]>

This series begins the work of supporting TC flower offload on EF100 NICs. This is the absolute minimum viable TC implementation to get traffic to
VFs and allow them to be tested; it supports no match fields besides ingress port, no actions besides mirred and drop, and no stats.
More matches, actions, and counters will be added in subsequent patches.

Changed in v2:
- Add missing 'static' on declarations (kernel test robot, sparse)

Edward Cree (6):
sfc: bind blocks for TC offload on EF100
sfc: bind indirect blocks for TC offload on EF100
sfc: optional logging of TC offload errors
sfc: add a hashtable for offloaded TC rules
sfc: interrogate MAE capabilities at probe time
sfc: bare bones TC offload on EF100

 drivers/net/ethernet/sfc/Makefile         |   2 +-
 drivers/net/ethernet/sfc/ef100_ethtool.c  |   2 +
 drivers/net/ethernet/sfc/ef100_netdev.c   |   4 +
 drivers/net/ethernet/sfc/ef100_nic.c      |   3 +
 drivers/net/ethernet/sfc/ef100_rep.c      |  18 +-
 drivers/net/ethernet/sfc/ef100_rep.h      |   1 +
 drivers/net/ethernet/sfc/ethtool_common.c |  37 ++
 drivers/net/ethernet/sfc/ethtool_common.h |   2 +
 drivers/net/ethernet/sfc/mae.c            | 165 +++++++++
 drivers/net/ethernet/sfc/mae.h            |  14 +
 drivers/net/ethernet/sfc/mcdi.h           |  10 +
 drivers/net/ethernet/sfc/net_driver.h     |   2 +
 drivers/net/ethernet/sfc/tc.c             | 430 +++++++++++++++++++++-
 drivers/net/ethernet/sfc/tc.h             |  36 ++
 drivers/net/ethernet/sfc/tc_bindings.c    | 228 ++++++++++++
 drivers/net/ethernet/sfc/tc_bindings.h    |  29 ++
 16 files changed, 980 insertions(+), 3 deletions(-)
 create mode 100644 drivers/net/ethernet/sfc/tc_bindings.c
 create mode 100644 drivers/net/ethernet/sfc/tc_bindings.h
#1 lore raw

PATCH v2 1/6 sfc: bind blocks for TC offload on EF100

From: Edward Cree <[email protected]>

Bind direct blocks for the MAE-admin PF and each VF representor. Currently these connect to a stub efx_tc_flower() that only returns
-EOPNOTSUPP; subsequent patches will implement flower offloads to the Match-Action Engine.

---
 drivers/net/ethernet/sfc/Makefile       |   2 +-
 drivers/net/ethernet/sfc/ef100_netdev.c |   4 +
 drivers/net/ethernet/sfc/ef100_nic.c    |   3 +
 drivers/net/ethernet/sfc/ef100_rep.c    |  16 +++
 drivers/net/ethernet/sfc/tc.c           |  14 ++-
 drivers/net/ethernet/sfc/tc.h           |   7 ++
 drivers/net/ethernet/sfc/tc_bindings.c  | 157 ++++++++++++++++++++++++
 drivers/net/ethernet/sfc/tc_bindings.h  |  23 ++++
 8 files changed, 224 insertions(+), 2 deletions(-)
 create mode 100644 drivers/net/ethernet/sfc/tc_bindings.c
 create mode 100644 drivers/net/ethernet/sfc/tc_bindings.h
drivers/net/ethernet/sfc/Makefile +1 -1
index bb06fa228367..b5e45fc6337e 100644
@@ -9,7 +9,7 @@ sfc-y			+= efx.o efx_common.o efx_channels.o nic.o \
 			   ef100_ethtool.o ef100_rx.o ef100_tx.o
 sfc-$(CONFIG_SFC_MTD)	+= mtd.o
 sfc-$(CONFIG_SFC_SRIOV)	+= sriov.o ef10_sriov.o ef100_sriov.o ef100_rep.o \
-                           mae.o tc.o
+                           mae.o tc.o tc_bindings.o
 
 obj-$(CONFIG_SFC)	+= sfc.o
 
drivers/net/ethernet/sfc/ef100_netdev.c +4 -0
index 17b9d37218cb..88fa29572e23 100644
@@ -23,6 +23,7 @@
 #include "mcdi_filters.h"
 #include "rx_common.h"
 #include "ef100_sriov.h"
+#include "tc_bindings.h"
 
 static void ef100_update_name(struct efx_nic *efx)
 {
@@ -246,6 +247,9 @@ static const struct net_device_ops ef100_netdev_ops = {
 #ifdef CONFIG_RFS_ACCEL
 	.ndo_rx_flow_steer      = efx_filter_rfs,
 #endif
+#ifdef CONFIG_SFC_SRIOV
+	.ndo_setup_tc		= efx_tc_setup,
+#endif
 };
 
 /*	Netdev registration
drivers/net/ethernet/sfc/ef100_nic.c +3 -0
index 8061efdaf82c..ad686c671ab8 100644
@@ -1137,6 +1137,9 @@ int ef100_probe_netdev_pf(struct efx_nic *efx)
 		 */
 		netif_warn(efx, probe, net_dev, "Failed to probe MAE rc %d\n",
 			   rc);
+	} else {
+		net_dev->features |= NETIF_F_HW_TC;
+		efx->fixed_features |= NETIF_F_HW_TC;
 	}
 #endif
 	return 0;
drivers/net/ethernet/sfc/ef100_rep.c +16 -0
index 73ae4656a6e7..0a631e0c9914 100644
@@ -14,6 +14,7 @@
 #include "ef100_nic.h"
 #include "mae.h"
 #include "rx_common.h"
+#include "tc_bindings.h"
 
 #define EFX_EF100_REP_DRIVER	"efx_ef100_rep"
 
@@ -107,6 +108,20 @@ static int efx_ef100_rep_get_phys_port_name(struct net_device *dev,
 	return 0;
 }
 
+static int efx_ef100_rep_setup_tc(struct net_device *net_dev,
+				  enum tc_setup_type type, void *type_data)
+{
+	struct efx_rep *efv = netdev_priv(net_dev);
+	struct efx_nic *efx = efv->parent;
+
+	if (type == TC_SETUP_CLSFLOWER)
+		return efx_tc_flower(efx, net_dev, type_data, efv);
+	if (type == TC_SETUP_BLOCK)
+		return efx_tc_setup_block(net_dev, efx, type_data, efv);
+
+	return -EOPNOTSUPP;
+}
+
 static void efx_ef100_rep_get_stats64(struct net_device *dev,
 				      struct rtnl_link_stats64 *stats)
 {
@@ -127,6 +142,7 @@ static const struct net_device_ops efx_ef100_rep_netdev_ops = {
 	.ndo_get_port_parent_id	= efx_ef100_rep_get_port_parent_id,
 	.ndo_get_phys_port_name	= efx_ef100_rep_get_phys_port_name,
 	.ndo_get_stats64	= efx_ef100_rep_get_stats64,
+	.ndo_setup_tc		= efx_ef100_rep_setup_tc,
 };
 
 static void efx_ef100_rep_get_drvinfo(struct net_device *dev,
drivers/net/ethernet/sfc/tc.c +13 -1
index 0c0aeb91f500..23c4325e739a 100644
@@ -58,6 +58,12 @@ static void efx_tc_delete_rule(struct efx_nic *efx, struct efx_tc_flow_rule *rul
 	rule->fw_id = MC_CMD_MAE_ACTION_RULE_INSERT_OUT_ACTION_RULE_ID_NULL;
 }
 
+int efx_tc_flower(struct efx_nic *efx, struct net_device *net_dev,
+		  struct flow_cls_offload *tc, struct efx_rep *efv)
+{
+	return -EOPNOTSUPP;
+}
+
 static int efx_tc_configure_default_rule(struct efx_nic *efx, u32 ing_port,
 					 u32 eg_port, struct efx_tc_flow_rule *rule)
 {
@@ -207,7 +213,11 @@ int efx_init_tc(struct efx_nic *efx)
 	rc = efx_tc_configure_default_rule_wire(efx);
 	if (rc)
 		return rc;
-	return efx_tc_configure_rep_mport(efx);
+	rc = efx_tc_configure_rep_mport(efx);
+	if (rc)
+		return rc;
+	efx->tc->up = true;
+	return 0;
 }
 
 void efx_fini_tc(struct efx_nic *efx)
@@ -218,6 +228,7 @@ void efx_fini_tc(struct efx_nic *efx)
 	efx_tc_deconfigure_rep_mport(efx);
 	efx_tc_deconfigure_default_rule(efx, &efx->tc->dflt.pf);
 	efx_tc_deconfigure_default_rule(efx, &efx->tc->dflt.wire);
+	efx->tc->up = false;
 }
 
 int efx_init_struct_tc(struct efx_nic *efx)
@@ -228,6 +239,7 @@ int efx_init_struct_tc(struct efx_nic *efx)
 	efx->tc = kzalloc(sizeof(*efx->tc), GFP_KERNEL);
 	if (!efx->tc)
 		return -ENOMEM;
+	INIT_LIST_HEAD(&efx->tc->block_list);
 
 	efx->tc->reps_filter_uc = -1;
 	efx->tc->reps_filter_mc = -1;
drivers/net/ethernet/sfc/tc.h +7 -0
index 309123c6b386..7b1a6fa0097d 100644
@@ -11,6 +11,7 @@
 
 #ifndef EFX_TC_H
 #define EFX_TC_H
+#include <net/flow_offload.h>
 #include "net_driver.h"
 
 struct efx_tc_action_set {
@@ -49,6 +50,7 @@ enum efx_tc_rule_prios {
 /**
  * struct efx_tc_state - control plane data for TC offload
  *
+ * @block_list: List of &struct efx_tc_block_binding
  * @reps_mport_id: MAE port allocated for representor RX
  * @reps_filter_uc: VNIC filter for representor unicast RX (promisc)
  * @reps_filter_mc: VNIC filter for representor multicast RX (allmulti)
@@ -57,14 +59,17 @@ enum efx_tc_rule_prios {
  *	%EFX_TC_PRIO_DFLT.  Named by *ingress* port
  * @dflt.pf: rule for traffic ingressing from PF (egresses to wire)
  * @dflt.wire: rule for traffic ingressing from wire (egresses to PF)
+ * @up: have TC datastructures been set up?
  */
 struct efx_tc_state {
+	struct list_head block_list;
 	u32 reps_mport_id, reps_mport_vport_id;
 	s32 reps_filter_uc, reps_filter_mc;
 	struct {
 		struct efx_tc_flow_rule pf;
 		struct efx_tc_flow_rule wire;
 	} dflt;
+	bool up;
 };
 
 struct efx_rep;
@@ -72,6 +77,8 @@ struct efx_rep;
 int efx_tc_configure_default_rule_rep(struct efx_rep *efv);
 void efx_tc_deconfigure_default_rule(struct efx_nic *efx,
 				     struct efx_tc_flow_rule *rule);
+int efx_tc_flower(struct efx_nic *efx, struct net_device *net_dev,
+		  struct flow_cls_offload *tc, struct efx_rep *efv);
 
 int efx_tc_insert_rep_filters(struct efx_nic *efx);
 void efx_tc_remove_rep_filters(struct efx_nic *efx);
drivers/net/ethernet/sfc/tc_bindings.c new file +157 -0

157 lines not on this page · raw message

drivers/net/ethernet/sfc/tc_bindings.h new file +23 -0
new file mode 100644
index 000000000000..bcd63c270585
@@ -0,0 +1,23 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/****************************************************************************
+ * Driver for Solarflare network controllers and boards
+ * Copyright 2022 Xilinx Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published
+ * by the Free Software Foundation, incorporated herein by reference.
+ */
+
+#ifndef EFX_TC_BINDINGS_H
+#define EFX_TC_BINDINGS_H
+#include "net_driver.h"
+
+#include <net/sch_generic.h>
+
+struct efx_rep;
+
+int efx_tc_setup_block(struct net_device *net_dev, struct efx_nic *efx,
+		       struct flow_block_offload *tcb, struct efx_rep *efv);
+int efx_tc_setup(struct net_device *net_dev, enum tc_setup_type type,
+		 void *type_data);
+#endif /* EFX_TC_BINDINGS_H */
#2 lore raw

Re: [PATCH v2 net-next 1/6] sfc: bind blocks for TC offload on EF100

Quoted 21 lines · On Mon, Sep 26, 2022 at 07:57:31PM +0100, [email protected] wrote:

From: Edward Cree <[email protected]>

Bind direct blocks for the MAE-admin PF and each VF representor. Currently these connect to a stub efx_tc_flower() that only returns
-EOPNOTSUPP; subsequent patches will implement flower offloads to the Match-Action Engine.

---
 drivers/net/ethernet/sfc/Makefile       |   2 +-
 drivers/net/ethernet/sfc/ef100_netdev.c |   4 +
 drivers/net/ethernet/sfc/ef100_nic.c    |   3 +
 drivers/net/ethernet/sfc/ef100_rep.c    |  16 +++
 drivers/net/ethernet/sfc/tc.c           |  14 ++-
 drivers/net/ethernet/sfc/tc.h           |   7 ++
 drivers/net/ethernet/sfc/tc_bindings.c  | 157 ++++++++++++++++++++++++
 drivers/net/ethernet/sfc/tc_bindings.h  |  23 ++++
 8 files changed, 224 insertions(+), 2 deletions(-)
 create mode 100644 drivers/net/ethernet/sfc/tc_bindings.c
 create mode 100644 drivers/net/ethernet/sfc/tc_bindings.h

<snip>

drivers/net/ethernet/sfc/tc_bindings.c line 4

Quoted 10 lines
drivers/net/ethernet/sfc/tc_bindings.c new file +4 -0
new file mode 100644
index 000000000000..d9401ee7b8e1
@@ -0,0 +1,157 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/****************************************************************************
+ * Driver for Solarflare network controllers and boards
+ * Copyright 2022 Xilinx Inc.

New files should have "Copyright 2022 Advanced Micro Devices, Inc." Same for the .h file below.

Signature · 1 line
Martin Habets <[email protected]>
Quoted 182 lines
drivers/net/ethernet/sfc/tc_bindings.c new file +153 -0
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published
+ * by the Free Software Foundation, incorporated herein by reference.
+ */
+
+#include "tc_bindings.h"
+#include "tc.h"
+
+struct efx_tc_block_binding {
+	struct list_head list;
+	struct efx_nic *efx;
+	struct efx_rep *efv;
+	struct net_device *otherdev; /* may actually be us */
+	struct flow_block *block;
+};
+
+static struct efx_tc_block_binding *efx_tc_find_binding(struct efx_nic *efx,
+							struct net_device *otherdev)
+{
+	struct efx_tc_block_binding *binding;
+
+	ASSERT_RTNL();
+	list_for_each_entry(binding, &efx->tc->block_list, list)
+		if (binding->otherdev == otherdev)
+			return binding;
+	return NULL;
+}
+
+static int efx_tc_block_cb(enum tc_setup_type type, void *type_data,
+			   void *cb_priv)
+{
+	struct efx_tc_block_binding *binding = cb_priv;
+	struct flow_cls_offload *tcf = type_data;
+
+	switch (type) {
+	case TC_SETUP_CLSFLOWER:
+		return efx_tc_flower(binding->efx, binding->otherdev,
+				     tcf, binding->efv);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static void efx_tc_block_unbind(void *cb_priv)
+{
+	struct efx_tc_block_binding *binding = cb_priv;
+
+	list_del(&binding->list);
+	kfree(binding);
+}
+
+static struct efx_tc_block_binding *efx_tc_create_binding(
+			struct efx_nic *efx, struct efx_rep *efv,
+			struct net_device *otherdev, struct flow_block *block)
+{
+	struct efx_tc_block_binding *binding = kmalloc(sizeof(*binding), GFP_KERNEL);
+
+	if (!binding)
+		return ERR_PTR(-ENOMEM);
+	binding->efx = efx;
+	binding->efv = efv;
+	binding->otherdev = otherdev;
+	binding->block = block;
+	list_add(&binding->list, &efx->tc->block_list);
+	return binding;
+}
+
+int efx_tc_setup_block(struct net_device *net_dev, struct efx_nic *efx,
+		       struct flow_block_offload *tcb, struct efx_rep *efv)
+{
+	struct efx_tc_block_binding *binding;
+	struct flow_block_cb *block_cb;
+	int rc;
+
+	if (tcb->binder_type != FLOW_BLOCK_BINDER_TYPE_CLSACT_INGRESS)
+		return -EOPNOTSUPP;
+
+	if (WARN_ON(!efx->tc))
+		return -ENETDOWN;
+
+	switch (tcb->command) {
+	case FLOW_BLOCK_BIND:
+		binding = efx_tc_create_binding(efx, efv, net_dev, tcb->block);
+		if (IS_ERR(binding))
+			return PTR_ERR(binding);
+		block_cb = flow_block_cb_alloc(efx_tc_block_cb, binding,
+					       binding, efx_tc_block_unbind);
+		rc = PTR_ERR_OR_ZERO(block_cb);
+		netif_dbg(efx, drv, efx->net_dev,
+			  "bind %sdirect block for device %s, rc %d\n",
+			  net_dev == efx->net_dev ? "" :
+			  efv ? "semi" : "in",
+			  net_dev ? net_dev->name : NULL, rc);
+		if (rc) {
+			list_del(&binding->list);
+			kfree(binding);
+		} else {
+			flow_block_cb_add(block_cb, tcb);
+		}
+		return rc;
+	case FLOW_BLOCK_UNBIND:
+		binding = efx_tc_find_binding(efx, net_dev);
+		if (binding) {
+			block_cb = flow_block_cb_lookup(tcb->block,
+							efx_tc_block_cb,
+							binding);
+			if (block_cb) {
+				flow_block_cb_remove(block_cb, tcb);
+				netif_dbg(efx, drv, efx->net_dev,
+					  "unbound %sdirect block for device %s\n",
+					  net_dev == efx->net_dev ? "" :
+					  binding->efv ? "semi" : "in",
+					  net_dev ? net_dev->name : NULL);
+				return 0;
+			}
+		}
+		/* If we're in driver teardown, then we expect to have
+		 * already unbound all our blocks (we did it early while
+		 * we still had MCDI to remove the filters), so getting
+		 * unbind callbacks now isn't a problem.
+		 */
+		netif_cond_dbg(efx, drv, efx->net_dev,
+			       !efx->tc->up, warn,
+			       "%sdirect block unbind for device %s, was never bound\n",
+			       net_dev == efx->net_dev ? "" : "in",
+			       net_dev ? net_dev->name : NULL);
+		return -ENOENT;
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+/* .ndo_setup_tc implementation
+ * Entry point for flower block and filter management.
+ */
+int efx_tc_setup(struct net_device *net_dev, enum tc_setup_type type,
+		 void *type_data)
+{
+	struct efx_nic *efx = efx_netdev_priv(net_dev);
+
+	if (efx->type->is_vf)
+		return -EOPNOTSUPP;
+	if (!efx->tc)
+		return -EOPNOTSUPP;
+
+	if (type == TC_SETUP_CLSFLOWER)
+		return efx_tc_flower(efx, net_dev, type_data, NULL);
+	if (type == TC_SETUP_BLOCK)
+		return efx_tc_setup_block(net_dev, efx, type_data, NULL);
+
+	return -EOPNOTSUPP;
+}
drivers/net/ethernet/sfc/tc_bindings.h new file +23 -0
new file mode 100644
index 000000000000..bcd63c270585
@@ -0,0 +1,23 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/****************************************************************************
+ * Driver for Solarflare network controllers and boards
+ * Copyright 2022 Xilinx Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published
+ * by the Free Software Foundation, incorporated herein by reference.
+ */
+
+#ifndef EFX_TC_BINDINGS_H
+#define EFX_TC_BINDINGS_H
+#include "net_driver.h"
+
+#include <net/sch_generic.h>
+
+struct efx_rep;
+
+int efx_tc_setup_block(struct net_device *net_dev, struct efx_nic *efx,
+		       struct flow_block_offload *tcb, struct efx_rep *efv);
+int efx_tc_setup(struct net_device *net_dev, enum tc_setup_type type,
+		 void *type_data);
+#endif /* EFX_TC_BINDINGS_H */
#3 lore raw

PATCH v2 2/6 sfc: bind indirect blocks for TC offload on EF100

In reply to [email protected]

From: Edward Cree <[email protected]>

Bind indirect blocks for recognised tunnel netdevices. Currently these connect to a stub efx_tc_flower() that only returns
-EOPNOTSUPP; subsequent patches will implement flower offloads to the Match-Action Engine.

---
 drivers/net/ethernet/sfc/tc.c          |  6 +++
 drivers/net/ethernet/sfc/tc_bindings.c | 73 +++++++++++++++++++++++++-
 drivers/net/ethernet/sfc/tc_bindings.h |  6 +++
 3 files changed, 84 insertions(+), 1 deletion(-)
drivers/net/ethernet/sfc/tc.c +6 -0
index 23c4325e739a..cb7f76c74e66 100644
@@ -10,6 +10,7 @@
  */
 
 #include "tc.h"
+#include "tc_bindings.h"
 #include "mae.h"
 #include "ef100_rep.h"
 #include "efx.h"
@@ -217,6 +218,9 @@ int efx_init_tc(struct efx_nic *efx)
 	if (rc)
 		return rc;
 	efx->tc->up = true;
+	rc = flow_indr_dev_register(efx_tc_indr_setup_cb, efx);
+	if (rc)
+		return rc;
 	return 0;
 }
 
@@ -225,6 +229,8 @@ void efx_fini_tc(struct efx_nic *efx)
 	/* We can get called even if efx_init_struct_tc() failed */
 	if (!efx->tc)
 		return;
+	if (efx->tc->up)
+		flow_indr_dev_unregister(efx_tc_indr_setup_cb, efx, efx_tc_block_unbind);
 	efx_tc_deconfigure_rep_mport(efx);
 	efx_tc_deconfigure_default_rule(efx, &efx->tc->dflt.pf);
 	efx_tc_deconfigure_default_rule(efx, &efx->tc->dflt.wire);
drivers/net/ethernet/sfc/tc_bindings.c +72 -1

85 lines not on this page · raw message

drivers/net/ethernet/sfc/tc_bindings.h +6 -0
index bcd63c270585..c210bb09150e 100644
@@ -16,8 +16,14 @@
 
 struct efx_rep;
 
+void efx_tc_block_unbind(void *cb_priv);
 int efx_tc_setup_block(struct net_device *net_dev, struct efx_nic *efx,
 		       struct flow_block_offload *tcb, struct efx_rep *efv);
 int efx_tc_setup(struct net_device *net_dev, enum tc_setup_type type,
 		 void *type_data);
+
+int efx_tc_indr_setup_cb(struct net_device *net_dev, struct Qdisc *sch,
+			 void *cb_priv, enum tc_setup_type type,
+			 void *type_data, void *data,
+			 void (*cleanup)(struct flow_block_cb *block_cb));
 #endif /* EFX_TC_BINDINGS_H */
#4 lore raw

PATCH v2 3/6 sfc: optional logging of TC offload errors

In reply to [email protected]

From: Edward Cree <[email protected]>

TC offload support will involve complex limitations on what matches and
actions a rule can do, in some cases potentially depending on rules already offloaded. So add an ethtool private flag "log-tc-errors" which controls reporting the reasons for un-offloadable TC rules at NETIF_INFO.

---
 drivers/net/ethernet/sfc/ef100_ethtool.c  |  2 ++
 drivers/net/ethernet/sfc/ethtool_common.c | 37 +++++++++++++++++++++++
 drivers/net/ethernet/sfc/ethtool_common.h |  2 ++
 drivers/net/ethernet/sfc/net_driver.h     |  2 ++
 drivers/net/ethernet/sfc/tc.h             | 18 +++++++++++
 5 files changed, 61 insertions(+)
drivers/net/ethernet/sfc/ef100_ethtool.c +2 -0
index 702abbe59b76..135ece2f1375 100644
@@ -43,6 +43,8 @@ const struct ethtool_ops ef100_ethtool_ops = {
 	.get_pauseparam         = efx_ethtool_get_pauseparam,
 	.set_pauseparam         = efx_ethtool_set_pauseparam,
 	.get_sset_count		= efx_ethtool_get_sset_count,
+	.get_priv_flags		= efx_ethtool_get_priv_flags,
+	.set_priv_flags		= efx_ethtool_set_priv_flags,
 	.self_test		= efx_ethtool_self_test,
 	.get_strings		= efx_ethtool_get_strings,
 	.get_link_ksettings	= efx_ethtool_get_link_ksettings,
drivers/net/ethernet/sfc/ethtool_common.c +37 -0
index a8cbceeb301b..6649a2327d03 100644
@@ -101,6 +101,14 @@ static const struct efx_sw_stat_desc efx_sw_stat_desc[] = {
 
 #define EFX_ETHTOOL_SW_STAT_COUNT ARRAY_SIZE(efx_sw_stat_desc)
 
+static const char efx_ethtool_priv_flags_strings[][ETH_GSTRING_LEN] = {
+	"log-tc-errors",
+};
+
+#define EFX_ETHTOOL_PRIV_FLAGS_LOG_TC_ERRS		BIT(0)
+
+#define EFX_ETHTOOL_PRIV_FLAGS_COUNT ARRAY_SIZE(efx_ethtool_priv_flags_strings)
+
 void efx_ethtool_get_drvinfo(struct net_device *net_dev,
 			     struct ethtool_drvinfo *info)
 {
@@ -452,6 +460,8 @@ int efx_ethtool_get_sset_count(struct net_device *net_dev, int string_set)
 		       efx_ptp_describe_stats(efx, NULL);
 	case ETH_SS_TEST:
 		return efx_ethtool_fill_self_tests(efx, NULL, NULL, NULL);
+	case ETH_SS_PRIV_FLAGS:
+		return EFX_ETHTOOL_PRIV_FLAGS_COUNT;
 	default:
 		return -EINVAL;
 	}
@@ -478,12 +488,39 @@ void efx_ethtool_get_strings(struct net_device *net_dev,
 	case ETH_SS_TEST:
 		efx_ethtool_fill_self_tests(efx, NULL, strings, NULL);
 		break;
+	case ETH_SS_PRIV_FLAGS:
+		for (i = 0; i < EFX_ETHTOOL_PRIV_FLAGS_COUNT; i++)
+			strscpy(strings + i * ETH_GSTRING_LEN,
+				efx_ethtool_priv_flags_strings[i],
+				ETH_GSTRING_LEN);
+		break;
 	default:
 		/* No other string sets */
 		break;
 	}
 }
 
+u32 efx_ethtool_get_priv_flags(struct net_device *net_dev)
+{
+	struct efx_nic *efx = efx_netdev_priv(net_dev);
+	u32 ret_flags = 0;
+
+	if (efx->log_tc_errs)
+		ret_flags |= EFX_ETHTOOL_PRIV_FLAGS_LOG_TC_ERRS;
+
+	return ret_flags;
+}
+
+int efx_ethtool_set_priv_flags(struct net_device *net_dev, u32 flags)
+{
+	struct efx_nic *efx = efx_netdev_priv(net_dev);
+
+	efx->log_tc_errs =
+		!!(flags & EFX_ETHTOOL_PRIV_FLAGS_LOG_TC_ERRS);
+
+	return 0;
+}
+
 void efx_ethtool_get_stats(struct net_device *net_dev,
 			   struct ethtool_stats *stats,
 			   u64 *data)
drivers/net/ethernet/sfc/ethtool_common.h +2 -0
index 659491932101..0afc74021a5e 100644
@@ -27,6 +27,8 @@ int efx_ethtool_fill_self_tests(struct efx_nic *efx,
 int efx_ethtool_get_sset_count(struct net_device *net_dev, int string_set);
 void efx_ethtool_get_strings(struct net_device *net_dev, u32 string_set,
 			     u8 *strings);
+u32 efx_ethtool_get_priv_flags(struct net_device *net_dev);
+int efx_ethtool_set_priv_flags(struct net_device *net_dev, u32 flags);
 void efx_ethtool_get_stats(struct net_device *net_dev,
 			   struct ethtool_stats *stats __attribute__ ((unused)),
 			   u64 *data);
drivers/net/ethernet/sfc/net_driver.h +2 -0
index 7ef823d7a89a..2e9ba0cfe848 100644
@@ -855,6 +855,7 @@ enum efx_xdp_tx_queues_mode {
  * @timer_max_ns: Interrupt timer maximum value, in nanoseconds
  * @irq_rx_adaptive: Adaptive IRQ moderation enabled for RX event queues
  * @irqs_hooked: Channel interrupts are hooked
+ * @log_tc_errs: Error logging for TC filter insertion is enabled
  * @irq_rx_mod_step_us: Step size for IRQ moderation for RX event queues
  * @irq_rx_moderation_us: IRQ moderation time for RX event queues
  * @msg_enable: Log message enable flags
@@ -1017,6 +1018,7 @@ struct efx_nic {
 	unsigned int timer_max_ns;
 	bool irq_rx_adaptive;
 	bool irqs_hooked;
+	bool log_tc_errs;
 	unsigned int irq_mod_step_us;
 	unsigned int irq_rx_moderation_us;
 	u32 msg_enable;
drivers/net/ethernet/sfc/tc.h +18 -0
index 7b1a6fa0097d..3e2299c5a885 100644
@@ -14,6 +14,24 @@
 #include <net/flow_offload.h>
 #include "net_driver.h"
 
+/* Error reporting: convenience macros.  For indicating why a given filter
+ * insertion is not supported; errors in internal operation or in the
+ * hardware should be netif_err()s instead.
+ */
+/* Used when error message is constant. */
+#define EFX_TC_ERR_MSG(efx, extack, message)	do {			\
+	NL_SET_ERR_MSG_MOD(extack, message);				\
+	if (efx->log_tc_errs)						\
+		netif_info(efx, drv, efx->net_dev, "%s\n", message);	\
+} while (0)
+/* Used when error message is not constant; caller should also supply a
+ * constant extack message with NL_SET_ERR_MSG_MOD().
+ */
+#define efx_tc_err(efx, fmt, args...)	do {		\
+if (efx->log_tc_errs)					\
+	netif_info(efx, drv, efx->net_dev, fmt, ##args);\
+} while (0)
+
 struct efx_tc_action_set {
 	u16 deliver:1;
 	u32 dest_mport;
maintainer
#5 lore raw

Re: [PATCH v2 net-next 3/6] sfc: optional logging of TC offload errors

Quoted 4 lines · On Mon, 26 Sep 2022 19:57:33 +0100 [email protected] wrote:

TC offload support will involve complex limitations on what matches and
actions a rule can do, in some cases potentially depending on rules already offloaded. So add an ethtool private flag "log-tc-errors" which controls reporting the reasons for un-offloadable TC rules at NETIF_INFO.

Because extack does not work somehow?

Somehow you limitations are harder to debug that everyone else's so you need a private flag? :/

maintainer
#6 lore raw

Re: [PATCH v2 net-next 3/6] sfc: optional logging of TC offload errors

Quoted 7 lines · Jakub Kicinski
Quoted 4 lines · On Mon, 26 Sep 2022 19:57:33 +0100 [email protected] wrote:

TC offload support will involve complex limitations on what matches and
actions a rule can do, in some cases potentially depending on rules already offloaded. So add an ethtool private flag "log-tc-errors" which controls reporting the reasons for un-offloadable TC rules at NETIF_INFO.

Because extack does not work somehow?

Last I checked, flow rules coming from an indirect binding to a tunnel
netdev did not report the hw driver's extack (or even rc) back to the user.
Also, extack can only contain fixed strings (netlink.h: "/* Currently
string formatting is not supported (due to the lack of an output buffer.) */") which was a real problem for us.

Quoted 2 lines

Somehow you limitations are harder to debug that everyone else's so you need a private flag? :/

It's not about debugging the driver, it's about communicating the
limitations to the end user. Having TC rules mysteriously fail to be offloaded with no indication of why is not a great UX :(
I couldn't see a way to handle this without vendor-specific ugliness,
but if you have a proposal I don't mind putting in some work to implement it.

-ed

maintainer
#7 lore raw

Re: [PATCH v2 net-next 3/6] sfc: optional logging of TC offload errors

Quoted 9 lines · On Wed, 28 Sep 2022 19:17:51 +0100 Edward Cree wrote:
Quoted 1 line

Because extack does not work somehow?

Last I checked, flow rules coming from an indirect binding to a tunnel
netdev did not report the hw driver's extack (or even rc) back to the user.
Also, extack can only contain fixed strings (netlink.h: "/* Currently
string formatting is not supported (due to the lack of an output buffer.) */") which was a real problem for us.

Quoted 6 lines
Quoted 2 lines

Somehow you limitations are harder to debug that everyone else's so you need a private flag? :/

It's not about debugging the driver, it's about communicating the
limitations to the end user. Having TC rules mysteriously fail to be offloaded with no indication of why is not a great UX :(

Yes, but everyone has the same problem.

Quoted 3 lines

I couldn't see a way to handle this without vendor-specific ugliness,
but if you have a proposal I don't mind putting in some work to implement it.

I won't help with the indirect stuff, I fixed it once a while back already and it keeps getting broken. It must be a case of the extack not being plumbed thru, or people being conservative because the errors are not fatal, right? Solvable.

The printf'ing? I recon something simple like adding a destructor for the message to the exack struct so you can allocate the message, or adding a small buffer in place (the messages aren't very long, usually) come to mind.

maintainer
#8 lore raw

Re: [PATCH v2 net-next 3/6] sfc: optional logging of TC offload errors

Quoted 4 lines · Jakub Kicinski

I won't help with the indirect stuff, I fixed it once a while back already and it keeps getting broken. It must be a case of the extack not being plumbed thru, or people being conservative because the errors are not fatal, right? Solvable.

The conceptual problem, as I see it, is that multiple hw drivers /
driver instances might be trying to offload the same tunnel rule, because the ingress device isn't actually specified anywhere in the weird inside-out way TC tunnel rules work.
So how do you deal with the case where one driver succeeds and
another fails to offload, or two fail with different rc and extack messages?

But I really need to go and check what it does right now, because
my information might be out of date — some of this driver code was first written two years ago so maybe it's since been solved.

Quoted 2 lines

The printf'ing? I recon something simple like adding a destructor for the message to the exack struct so you can allocate the message,

What about just a flag to say "please kfree() the msg on destruct"? I have a hard time imagining a destructor that would need to do
anything different.

Quoted 2 lines

or adding a small buffer in place (the messages aren't very long, usually) come to mind.

Also an option, yeah. Downside is that it consumes that memory
(I guess 80B or so?) for every netlink response whether it's using formatted extack or not; idk if people with lots of netlink traffic might start to care about that.
Should I rustle up an RFC patch for one of these, or post an RFD to
the list to canvass other vendors' opinions?

-ed

maintainer
#9 lore raw

Re: [PATCH v2 net-next 3/6] sfc: optional logging of TC offload errors

Quoted 6 lines · On Wed, 28 Sep 2022 19:58:23 +0100 Edward Cree wrote:
Quoted 4 lines · Jakub Kicinski

I won't help with the indirect stuff, I fixed it once a while back already and it keeps getting broken. It must be a case of the extack not being plumbed thru, or people being conservative because the errors are not fatal, right? Solvable.

Quoted 7 lines

The conceptual problem, as I see it, is that multiple hw drivers /
driver instances might be trying to offload the same tunnel rule, because the ingress device isn't actually specified anywhere in the weird inside-out way TC tunnel rules work.
So how do you deal with the case where one driver succeeds and
another fails to offload, or two fail with different rc and extack messages?

Let's solve practical problems first :) The cases with multiple devices offloading are rare AFAIK.

Quoted 10 lines

But I really need to go and check what it does right now, because
my information might be out of date — some of this driver code was first written two years ago so maybe it's since been solved.

Quoted 2 lines

The printf'ing? I recon something simple like adding a destructor for the message to the exack struct so you can allocate the message,

What about just a flag to say "please kfree() the msg on destruct"? I have a hard time imagining a destructor that would need to do
anything different.

Yes, seems like that could be good enough. I was wondering if perhaps someone would like to have a "static" buffer and manage ownership (given most of the config happens under rtnl_lock) but perhaps that's unnecessary complexity.

BTW we will prolly need two bits, one to indicate the creator will actually call free and the other to mark that it's needed. Otherwise we'd need to sift thru the stack and find all extack instances. You can if you want to but...

Quoted 7 lines
Quoted 2 lines

or adding a small buffer in place (the messages aren't very long, usually) come to mind.

Also an option, yeah. Downside is that it consumes that memory
(I guess 80B or so?) for every netlink response whether it's using formatted extack or not; idk if people with lots of netlink traffic might start to care about that.

It's just a buffer on the stack, in the struct, the extack is transformed into netlink attrs in the same way regardless. Stack use is the only concern, no other impact on those not using it.

Quoted 2 lines

Should I rustle up an RFC patch for one of these, or post an RFD to
the list to canvass other vendors' opinions?

Would be great! Maybe also grep the archive, cause this came up before. Someone was against this in the past, perhaps, perhaps even me :) But if it wasn't me we should CC them.

maintainer
#10 lore raw

Re: [PATCH v2 net-next 3/6] sfc: optional logging of TC offload errors

Quoted 2 lines · Jakub Kicinski

Let's solve practical problems first :) The cases with multiple devices offloading are rare AFAIK.

I know of someone who is working on such a use-case for the Alveo U25N
and running into Interesting difficulties with the same rule getting offloaded twice; they probably would care about getting both devices' error messages. I know the plural of anecdote is not data; but I think it's not so rare that we can completely ignore it.

Quoted 3 lines

It's just a buffer on the stack, in the struct, the extack is transformed into netlink attrs in the same way regardless. Stack use is the only concern, no other impact on those not using it.

Okay, I'd probably go with this approach then.

Quoted 6 lines
Quoted 2 lines

Should I rustle up an RFC patch for one of these, or post an RFD to
the list to canvass other vendors' opinions?

Would be great! Maybe also grep the archive, cause this came up before. Someone was against this in the past, perhaps, perhaps even me :) But if it wasn't me we should CC them.

Possibly I'm searching for the wrong magic words, all I can find is
https://lkml.org/lkml/2021/8/9/747 which is vaguely related at best.

-ed

maintainer
#11 lore raw

Re: [PATCH v2 net-next 3/6] sfc: optional logging of TC offload errors

Quoted 9 lines · On Wed, 28 Sep 2022 22:14:37 +0100 Edward Cree wrote:
Quoted 2 lines · Jakub Kicinski

Let's solve practical problems first :) The cases with multiple devices offloading are rare AFAIK.

I know of someone who is working on such a use-case for the Alveo U25N
and running into Interesting difficulties with the same rule getting offloaded twice; they probably would care about getting both devices' error messages. I know the plural of anecdote is not data; but I think it's not so rare that we can completely ignore it.

Hm. I wonder if throwing a tracepoint into the extack setting machinery would be a reasonable stop gap for debugging.

maintainer
#12 lore raw

Re: [PATCH v2 net-next 3/6] sfc: optional logging of TC offload errors

Quoted 2 lines · Jakub Kicinski

Hm. I wonder if throwing a tracepoint into the extack setting machinery would be a reasonable stop gap for debugging.

It has one (do_trace_netlink_extack()), but sadly that won't play
so well with formatted extacks since AIUI trace needs a constant string (I'm just giving it the format string in my prototype).
But yeah it's better than nothing.

maintainer
#13 lore raw

Re: [PATCH v2 net-next 3/6] sfc: optional logging of TC offload errors

Quoted 8 lines · On Fri, 30 Sep 2022 10:03:01 +0100 Edward Cree wrote:
Quoted 2 lines · Jakub Kicinski

Hm. I wonder if throwing a tracepoint into the extack setting machinery would be a reasonable stop gap for debugging.

It has one (do_trace_netlink_extack()), but sadly that won't play
so well with formatted extacks since AIUI trace needs a constant string (I'm just giving it the format string in my prototype).
But yeah it's better than nothing.

We can add a new one which copies the data. Presumably we'd have a "set an extack msg which needs to be freed" helper were we could place it? It'd mean we cut off at a static length but good enough, I say.

maintainer
#14 lore raw

Re: [PATCH v2 net-next 3/6] sfc: optional logging of TC offload errors

Quoted 9 lines · Jakub Kicinski
Quoted 4 lines · On Fri, 30 Sep 2022 10:03:01 +0100 Edward Cree wrote:

It has one (do_trace_netlink_extack()), but sadly that won't play
so well with formatted extacks since AIUI trace needs a constant string (I'm just giving it the format string in my prototype).
But yeah it's better than nothing.

We can add a new one which copies the data. Presumably we'd have a "set an extack msg which needs to be freed" helper were we could place it? It'd mean we cut off at a static length but good enough, I say.

Hmm, seems I was wrong about the tracepoint, the machinery already
handles a dynamic string just fine.
I should have an RFC patch ready in a few days.

-ed

#15 lore raw

PATCH v2 4/6 sfc: add a hashtable for offloaded TC rules

In reply to [email protected]

From: Edward Cree <[email protected]>

Nothing inserts into this table yet, but we have code to remove rules
on FLOW_CLS_DESTROY or at driver teardown time, in both cases also attempting to remove the corresponding hardware rules.

---
 drivers/net/ethernet/sfc/ef100_rep.c |   2 +-
 drivers/net/ethernet/sfc/ef100_rep.h |   1 +
 drivers/net/ethernet/sfc/tc.c        | 115 ++++++++++++++++++++++++++-
 drivers/net/ethernet/sfc/tc.h        |   7 ++
 4 files changed, 123 insertions(+), 2 deletions(-)
drivers/net/ethernet/sfc/ef100_rep.c +1 -1
index 0a631e0c9914..869f806a6b67 100644
@@ -135,7 +135,7 @@ static void efx_ef100_rep_get_stats64(struct net_device *dev,
 	stats->tx_errors = atomic64_read(&efv->stats.tx_errors);
 }
 
-static const struct net_device_ops efx_ef100_rep_netdev_ops = {
+const struct net_device_ops efx_ef100_rep_netdev_ops = {
 	.ndo_open		= efx_ef100_rep_open,
 	.ndo_stop		= efx_ef100_rep_close,
 	.ndo_start_xmit		= efx_ef100_rep_xmit,
drivers/net/ethernet/sfc/ef100_rep.h +1 -0
index 070f700893c1..c21bc716f847 100644
@@ -66,4 +66,5 @@ void efx_ef100_rep_rx_packet(struct efx_rep *efv, struct efx_rx_buffer *rx_buf);
  * Caller must hold rcu_read_lock().
  */
 struct efx_rep *efx_ef100_find_rep_by_mport(struct efx_nic *efx, u16 mport);
+extern const struct net_device_ops efx_ef100_rep_netdev_ops;
 #endif /* EF100_REP_H */
drivers/net/ethernet/sfc/tc.c +114 -1

158 lines not on this page · raw message

drivers/net/ethernet/sfc/tc.h +7 -0
index 3e2299c5a885..94a04374e505 100644
@@ -12,6 +12,7 @@
 #ifndef EFX_TC_H
 #define EFX_TC_H
 #include <net/flow_offload.h>
+#include <linux/rhashtable.h>
 #include "net_driver.h"
 
 /* Error reporting: convenience macros.  For indicating why a given filter
@@ -55,6 +56,8 @@ struct efx_tc_action_set_list {
 };
 
 struct efx_tc_flow_rule {
+	unsigned long cookie;
+	struct rhash_head linkage;
 	struct efx_tc_match match;
 	struct efx_tc_action_set_list acts;
 	u32 fw_id;
@@ -69,6 +72,8 @@ enum efx_tc_rule_prios {
  * struct efx_tc_state - control plane data for TC offload
  *
  * @block_list: List of &struct efx_tc_block_binding
+ * @mutex: Used to serialise operations on TC hashtables
+ * @match_action_ht: Hashtable of TC match-action rules
  * @reps_mport_id: MAE port allocated for representor RX
  * @reps_filter_uc: VNIC filter for representor unicast RX (promisc)
  * @reps_filter_mc: VNIC filter for representor multicast RX (allmulti)
@@ -81,6 +86,8 @@ enum efx_tc_rule_prios {
  */
 struct efx_tc_state {
 	struct list_head block_list;
+	struct mutex mutex;
+	struct rhashtable match_action_ht;
 	u32 reps_mport_id, reps_mport_vport_id;
 	s32 reps_filter_uc, reps_filter_mc;
 	struct {
#16 lore raw

PATCH v2 5/6 sfc: interrogate MAE capabilities at probe time

In reply to [email protected]

From: Edward Cree <[email protected]>

Different versions of EF100 firmware and FPGA bitstreams support different
matching capabilities in the Match-Action Engine. Probe for these at start of day; subsequent patches will validate TC offload requests against the reported capabilities.

---
 drivers/net/ethernet/sfc/mae.c | 56 ++++++++++++++++++++++++++++++++++
 drivers/net/ethernet/sfc/mae.h | 10 ++++++
 drivers/net/ethernet/sfc/tc.c  | 25 +++++++++++++++
 drivers/net/ethernet/sfc/tc.h  |  2 ++
 4 files changed, 93 insertions(+)
drivers/net/ethernet/sfc/mae.c +56 -0
index 97627f5e3674..19138b2d2f5c 100644
@@ -112,6 +112,62 @@ int efx_mae_lookup_mport(struct efx_nic *efx, u32 selector, u32 *id)
 	return 0;
 }
 
+static int efx_mae_get_basic_caps(struct efx_nic *efx, struct mae_caps *caps)
+{
+	MCDI_DECLARE_BUF(outbuf, MC_CMD_MAE_GET_CAPS_OUT_LEN);
+	size_t outlen;
+	int rc;
+
+	BUILD_BUG_ON(MC_CMD_MAE_GET_CAPS_IN_LEN);
+
+	rc = efx_mcdi_rpc(efx, MC_CMD_MAE_GET_CAPS, NULL, 0, outbuf,
+			  sizeof(outbuf), &outlen);
+	if (rc)
+		return rc;
+	if (outlen < sizeof(outbuf))
+		return -EIO;
+	caps->match_field_count = MCDI_DWORD(outbuf, MAE_GET_CAPS_OUT_MATCH_FIELD_COUNT);
+	caps->action_prios = MCDI_DWORD(outbuf, MAE_GET_CAPS_OUT_ACTION_PRIOS);
+	return 0;
+}
+
+static int efx_mae_get_rule_fields(struct efx_nic *efx, u32 cmd,
+				   u8 *field_support)
+{
+	MCDI_DECLARE_BUF(outbuf, MC_CMD_MAE_GET_AR_CAPS_OUT_LEN(MAE_NUM_FIELDS));
+	MCDI_DECLARE_STRUCT_PTR(caps);
+	unsigned int count;
+	size_t outlen;
+	int rc, i;
+
+	BUILD_BUG_ON(MC_CMD_MAE_GET_AR_CAPS_IN_LEN);
+
+	rc = efx_mcdi_rpc(efx, cmd, NULL, 0, outbuf, sizeof(outbuf), &outlen);
+	if (rc)
+		return rc;
+	count = MCDI_DWORD(outbuf, MAE_GET_AR_CAPS_OUT_COUNT);
+	memset(field_support, MAE_FIELD_UNSUPPORTED, MAE_NUM_FIELDS);
+	caps = _MCDI_DWORD(outbuf, MAE_GET_AR_CAPS_OUT_FIELD_FLAGS);
+	/* We're only interested in the support status enum, not any other
+	 * flags, so just extract that from each entry.
+	 */
+	for (i = 0; i < count; i++)
+		if (i * sizeof(*outbuf) + MC_CMD_MAE_GET_AR_CAPS_OUT_FIELD_FLAGS_OFST < outlen)
+			field_support[i] = EFX_DWORD_FIELD(caps[i], MAE_FIELD_FLAGS_SUPPORT_STATUS);
+	return 0;
+}
+
+int efx_mae_get_caps(struct efx_nic *efx, struct mae_caps *caps)
+{
+	int rc;
+
+	rc = efx_mae_get_basic_caps(efx, caps);
+	if (rc)
+		return rc;
+	return efx_mae_get_rule_fields(efx, MC_CMD_MAE_GET_AR_CAPS,
+				       caps->action_rule_fields);
+}
+
 static bool efx_mae_asl_id(u32 id)
 {
 	return !!(id & BIT(31));
drivers/net/ethernet/sfc/mae.h +10 -0
index 0369be4d8983..2b49a88b303c 100644
@@ -27,6 +27,16 @@ void efx_mae_mport_mport(struct efx_nic *efx, u32 mport_id, u32 *out);
 
 int efx_mae_lookup_mport(struct efx_nic *efx, u32 selector, u32 *id);
 
+#define MAE_NUM_FIELDS	(MAE_FIELD_ENC_VNET_ID + 1)
+
+struct mae_caps {
+	u32 match_field_count;
+	u32 action_prios;
+	u8 action_rule_fields[MAE_NUM_FIELDS];
+};
+
+int efx_mae_get_caps(struct efx_nic *efx, struct mae_caps *caps);
+
 int efx_mae_alloc_action_set(struct efx_nic *efx, struct efx_tc_action_set *act);
 int efx_mae_free_action_set(struct efx_nic *efx, u32 fw_id);
 
drivers/net/ethernet/sfc/tc.c +25 -0
index 08e2af665380..2b2d45b97305 100644
@@ -305,6 +305,23 @@ int efx_init_tc(struct efx_nic *efx)
 {
 	int rc;
 
+	rc = efx_mae_get_caps(efx, efx->tc->caps);
+	if (rc)
+		return rc;
+	if (efx->tc->caps->match_field_count > MAE_NUM_FIELDS)
+		/* Firmware supports some match fields the driver doesn't know
+		 * about.  Not fatal, unless any of those fields are required
+		 * (MAE_FIELD_SUPPORTED_MATCH_ALWAYS) but if so we don't know.
+		 */
+		netif_warn(efx, probe, efx->net_dev,
+			   "FW reports additional match fields %u\n",
+			   efx->tc->caps->match_field_count);
+	if (efx->tc->caps->action_prios < EFX_TC_PRIO__NUM) {
+		netif_err(efx, probe, efx->net_dev,
+			  "Too few action prios supported (have %u, need %u)\n",
+			  efx->tc->caps->action_prios, EFX_TC_PRIO__NUM);
+		return -EIO;
+	}
 	rc = efx_tc_configure_default_rule_pf(efx);
 	if (rc)
 		return rc;
@@ -344,6 +361,11 @@ int efx_init_struct_tc(struct efx_nic *efx)
 	efx->tc = kzalloc(sizeof(*efx->tc), GFP_KERNEL);
 	if (!efx->tc)
 		return -ENOMEM;
+	efx->tc->caps = kzalloc(sizeof(struct mae_caps), GFP_KERNEL);
+	if (!efx->tc->caps) {
+		rc = -ENOMEM;
+		goto fail_alloc_caps;
+	}
 	INIT_LIST_HEAD(&efx->tc->block_list);
 
 	mutex_init(&efx->tc->mutex);
@@ -359,6 +381,8 @@ int efx_init_struct_tc(struct efx_nic *efx)
 	return 0;
 fail_match_action_ht:
 	mutex_destroy(&efx->tc->mutex);
+	kfree(efx->tc->caps);
+fail_alloc_caps:
 	kfree(efx->tc);
 	efx->tc = NULL;
 	return rc;
@@ -378,6 +402,7 @@ void efx_fini_struct_tc(struct efx_nic *efx)
 				    efx);
 	mutex_unlock(&efx->tc->mutex);
 	mutex_destroy(&efx->tc->mutex);
+	kfree(efx->tc->caps);
 	kfree(efx->tc);
 	efx->tc = NULL;
 }
drivers/net/ethernet/sfc/tc.h +2 -0
index 94a04374e505..baf1e67b58a5 100644
@@ -71,6 +71,7 @@ enum efx_tc_rule_prios {
 /**
  * struct efx_tc_state - control plane data for TC offload
  *
+ * @caps: MAE capabilities reported by MCDI
  * @block_list: List of &struct efx_tc_block_binding
  * @mutex: Used to serialise operations on TC hashtables
  * @match_action_ht: Hashtable of TC match-action rules
@@ -85,6 +86,7 @@ enum efx_tc_rule_prios {
  * @up: have TC datastructures been set up?
  */
 struct efx_tc_state {
+	struct mae_caps *caps;
 	struct list_head block_list;
 	struct mutex mutex;
 	struct rhashtable match_action_ht;
#17 lore raw

PATCH v2 6/6 sfc: bare bones TC offload on EF100

In reply to [email protected]

From: Edward Cree <[email protected]>

This is the absolute minimum viable TC implementation to get traffic to
VFs and allow them to be tested; it supports no match fields besides ingress port, no actions besides mirred and drop, and no stats.
Example usage:
tc filter add dev $PF parent ffff: flower skip_sw \
action mirred egress mirror dev $VFREP
tc filter add dev $VFREP parent ffff: flower skip_sw \
action mirred egress redirect dev $PF
gives a VF unfiltered access to the network out the physical port ($PF acts here as a physical port representor).
More matches, actions, and counters will be added in subsequent patches.

---
 drivers/net/ethernet/sfc/mae.c  | 109 +++++++++++++
 drivers/net/ethernet/sfc/mae.h  |   4 +
 drivers/net/ethernet/sfc/mcdi.h |  10 ++
 drivers/net/ethernet/sfc/tc.c   | 272 ++++++++++++++++++++++++++++++++
 drivers/net/ethernet/sfc/tc.h   |   2 +
 5 files changed, 397 insertions(+)
drivers/net/ethernet/sfc/mae.c +109 -0
index 19138b2d2f5c..874c765b2465 100644
@@ -168,6 +168,111 @@ int efx_mae_get_caps(struct efx_nic *efx, struct mae_caps *caps)
 				       caps->action_rule_fields);
 }
 
+/* Bit twiddling:
+ * Prefix: 1...110...0
+ *      ~: 0...001...1
+ *    + 1: 0...010...0 is power of two
+ * so (~x) & ((~x) + 1) == 0.  Converse holds also.
+ */
+#define is_prefix_byte(_x)	!(((_x) ^ 0xff) & (((_x) ^ 0xff) + 1))
+
+enum mask_type { MASK_ONES, MASK_ZEROES, MASK_PREFIX, MASK_OTHER };
+
+static const char *mask_type_name(enum mask_type typ)
+{
+	switch (typ) {
+	case MASK_ONES:
+		return "all-1s";
+	case MASK_ZEROES:
+		return "all-0s";
+	case MASK_PREFIX:
+		return "prefix";
+	case MASK_OTHER:
+		return "arbitrary";
+	default: /* can't happen */
+		return "unknown";
+	}
+}
+
+/* Checks a (big-endian) bytestring is a bit prefix */
+static enum mask_type classify_mask(const u8 *mask, size_t len)
+{
+	bool zeroes = true; /* All bits seen so far are zeroes */
+	bool ones = true; /* All bits seen so far are ones */
+	bool prefix = true; /* Valid prefix so far */
+	size_t i;
+
+	for (i = 0; i < len; i++) {
+		if (ones) {
+			if (!is_prefix_byte(mask[i]))
+				prefix = false;
+		} else if (mask[i]) {
+			prefix = false;
+		}
+		if (mask[i] != 0xff)
+			ones = false;
+		if (mask[i])
+			zeroes = false;
+	}
+	if (ones)
+		return MASK_ONES;
+	if (zeroes)
+		return MASK_ZEROES;
+	if (prefix)
+		return MASK_PREFIX;
+	return MASK_OTHER;
+}
+
+static int efx_mae_match_check_cap_typ(u8 support, enum mask_type typ)
+{
+	switch (support) {
+	case MAE_FIELD_UNSUPPORTED:
+	case MAE_FIELD_SUPPORTED_MATCH_NEVER:
+		if (typ == MASK_ZEROES)
+			return 0;
+		return -EOPNOTSUPP;
+	case MAE_FIELD_SUPPORTED_MATCH_OPTIONAL:
+		if (typ == MASK_ZEROES)
+			return 0;
+		fallthrough;
+	case MAE_FIELD_SUPPORTED_MATCH_ALWAYS:
+		if (typ == MASK_ONES)
+			return 0;
+		return -EINVAL;
+	case MAE_FIELD_SUPPORTED_MATCH_PREFIX:
+		if (typ == MASK_OTHER)
+			return -EOPNOTSUPP;
+		return 0;
+	case MAE_FIELD_SUPPORTED_MATCH_MASK:
+		return 0;
+	default:
+		return -EIO;
+	}
+}
+
+int efx_mae_match_check_caps(struct efx_nic *efx,
+			     const struct efx_tc_match_fields *mask,
+			     struct netlink_ext_ack *extack)
+{
+	const u8 *supported_fields = efx->tc->caps->action_rule_fields;
+	__be32 ingress_port = cpu_to_be32(mask->ingress_port);
+	enum mask_type ingress_port_mask_type;
+	int rc;
+
+	/* Check for _PREFIX assumes big-endian, so we need to convert */
+	ingress_port_mask_type = classify_mask((const u8 *)&ingress_port,
+					       sizeof(ingress_port));
+	rc = efx_mae_match_check_cap_typ(supported_fields[MAE_FIELD_INGRESS_PORT],
+					 ingress_port_mask_type);
+	if (rc) {
+		efx_tc_err(efx, "No support for %s mask in field ingress_port\n",
+			   mask_type_name(ingress_port_mask_type));
+		NL_SET_ERR_MSG_MOD(extack, "Unsupported mask type for ingress_port");
+		return rc;
+	}
+	return 0;
+}
+
 static bool efx_mae_asl_id(u32 id)
 {
 	return !!(id & BIT(31));
@@ -335,6 +440,10 @@ static int efx_mae_populate_match_criteria(MCDI_DECLARE_STRUCT_PTR(match_crit),
 	}
 	MCDI_STRUCT_SET_DWORD(match_crit, MAE_FIELD_MASK_VALUE_PAIRS_V2_INGRESS_MPORT_SELECTOR_MASK,
 			      match->mask.ingress_port);
+	MCDI_STRUCT_SET_BYTE(match_crit, MAE_FIELD_MASK_VALUE_PAIRS_V2_RECIRC_ID,
+			     match->value.recirc_id);
+	MCDI_STRUCT_SET_BYTE(match_crit, MAE_FIELD_MASK_VALUE_PAIRS_V2_RECIRC_ID_MASK,
+			     match->mask.recirc_id);
 	return 0;
 }
 
drivers/net/ethernet/sfc/mae.h +4 -0
index 2b49a88b303c..3e0cd238d523 100644
@@ -37,6 +37,10 @@ struct mae_caps {
 
 int efx_mae_get_caps(struct efx_nic *efx, struct mae_caps *caps);
 
+int efx_mae_match_check_caps(struct efx_nic *efx,
+			     const struct efx_tc_match_fields *mask,
+			     struct netlink_ext_ack *extack);
+
 int efx_mae_alloc_action_set(struct efx_nic *efx, struct efx_tc_action_set *act);
 int efx_mae_free_action_set(struct efx_nic *efx, u32 fw_id);
 
drivers/net/ethernet/sfc/mcdi.h +10 -0
index 26bc69f76801..1f18e9dc62e8 100644
@@ -201,6 +201,12 @@ void efx_mcdi_sensor_event(struct efx_nic *efx, efx_qword_t *ev);
 	((u8 *)(_buf) + (_offset))
 #define MCDI_PTR(_buf, _field)						\
 	_MCDI_PTR(_buf, MC_CMD_ ## _field ## _OFST)
+/* Use MCDI_STRUCT_ functions to access members of MCDI structuredefs.
+ * _buf should point to the start of the structure, typically obtained with
+ * MCDI_DECLARE_STRUCT_PTR(structure) = _MCDI_DWORD(mcdi_buf, FIELD_WHICH_IS_STRUCT);
+ */
+#define MCDI_STRUCT_PTR(_buf, _field)					\
+	_MCDI_PTR(_buf, _field ## _OFST)
 #define _MCDI_CHECK_ALIGN(_ofst, _align)				\
 	((_ofst) + BUILD_BUG_ON_ZERO((_ofst) & (_align - 1)))
 #define _MCDI_DWORD(_buf, _field)					\
@@ -208,6 +214,10 @@ void efx_mcdi_sensor_event(struct efx_nic *efx, efx_qword_t *ev);
 #define _MCDI_STRUCT_DWORD(_buf, _field)				\
 	((_buf) + (_MCDI_CHECK_ALIGN(_field ## _OFST, 4) >> 2))
 
+#define MCDI_STRUCT_SET_BYTE(_buf, _field, _value) do {			\
+	BUILD_BUG_ON(_field ## _LEN != 1);				\
+	*(u8 *)MCDI_STRUCT_PTR(_buf, _field) = _value;			\
+	} while (0)
 #define MCDI_BYTE(_buf, _field)						\
 	((void)BUILD_BUG_ON_ZERO(MC_CMD_ ## _field ## _LEN != 1),	\
 	 *MCDI_PTR(_buf, _field))
drivers/net/ethernet/sfc/tc.c +272 -0

296 lines not on this page · raw message

drivers/net/ethernet/sfc/tc.h +2 -0
index baf1e67b58a5..196fd74ed973 100644
@@ -43,6 +43,7 @@ struct efx_tc_action_set {
 struct efx_tc_match_fields {
 	/* L1 */
 	u32 ingress_port;
+	u8 recirc_id;
 };
 
 struct efx_tc_match {
@@ -64,6 +65,7 @@ struct efx_tc_flow_rule {
 };
 
 enum efx_tc_rule_prios {
+	EFX_TC_PRIO_TC, /* Rule inserted by TC */
 	EFX_TC_PRIO_DFLT, /* Default switch rule; one of efx_tc_default_rules */
 	EFX_TC_PRIO__NUM
 };
#18 lore raw
In reply to [email protected]
This series was applied to netdev/net-next.git (master) by David S. Miller <[email protected]>:

Hello:

This series was applied to netdev/net-next.git (master) by David S. Miller <[email protected]>:

Quoted 9 lines · On Mon, 26 Sep 2022 19:57:30 +0100 you wrote:

From: Edward Cree <[email protected]>

This series begins the work of supporting TC flower offload on EF100 NICs. This is the absolute minimum viable TC implementation to get traffic to
VFs and allow them to be tested; it supports no match fields besides ingress port, no actions besides mirred and drop, and no stats.
More matches, actions, and counters will be added in subsequent patches.

[...]

Here is the summary with links:
- [v2,net-next,1/6] sfc: bind blocks for TC offload on EF100

    https://git.kernel.org/netdev/net-next/c/9dc0cad203ab
  - [v2,net-next,2/6] sfc: bind indirect blocks for TC offload on EF100
    https://git.kernel.org/netdev/net-next/c/5b2e12d51bd8
  - [v2,net-next,3/6] sfc: optional logging of TC offload errors
    https://git.kernel.org/netdev/net-next/c/7c9d266d8faf
  - [v2,net-next,4/6] sfc: add a hashtable for offloaded TC rules
    https://git.kernel.org/netdev/net-next/c/f54a28a21166
  - [v2,net-next,5/6] sfc: interrogate MAE capabilities at probe time
    https://git.kernel.org/netdev/net-next/c/7ce3e235f212
  - [v2,net-next,6/6] sfc: bare bones TC offload on EF100
    https://git.kernel.org/netdev/net-next/c/d902e1a737d4

You are awesome, thank you!

Signature · 2 lines
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html