[PATCH net-next 00/14] nfp: abm: RED/MQ qdisc offload

[PATCH net-next 00/14] nfp: abm: RED/MQ qdisc offload

[Jakub Kicinski]

Hi!

This is second batch of advanced buffer management nfp driver
changes.  This series adds the qdisc offload.  Support for
a very simple subset of RED qdisc offload is added as needed
for DCTCP ECN marking (min and max thresholds set to the same
value).

The first two patches fix glitches introduced by the previous
series.  We have to be careful about phys_port_name handling,
because VFs share the same code path, and some user space may
get confused by the names we chose.

Since unlike previous offloads we can report the queue backlog
both in bytes and packets we need to adjust how statistics are
added up in the core (patch 6).

There are some extra statistics we want to expose which don't
fit into TC stats, namely counts of packets which have been fast-
-forwarded without getting enqueued because there was no
contention and number of packets that were ever queued (sum of
all momentary backlogs).  We expose those through ethtool stats
(patches 8 and 9).

Remaining 5 patches add MQ offload - to be able to set different
configurations on different queues.  Representors are made multi-
-queue and we add offload support to MQ.  MQ stats are added up
before calling ->dump qdiscs on the children, and therefore don't
include updated offload values.  To avoid clearly incorrect stats
MQ is made to also request stats update from offloads.  This way
we can correct the diff at the driver level.


Jakub Kicinski (14):
  nfp: return -EOPNOTSUPP from .ndo_get_phys_port_name for VFs
  nfp: prefix vNIC phys_port_name with 'n'
  nfp: abm: enable advanced queuing on demand
  nfp: abm: add helpers for configuring queue marking levels
  nfp: abm: add simple RED offload
  net: sched: add qstats.qlen to qlen
  nfp: abm: report statistics from RED offload
  nfp: allow apps to add extra stats to ports
  nfp: abm: expose the internal stats in ethtool
  nfp: abm: expose all PF queues
  net: sched: mq: add simple offload notification
  nfp: abm: multi-queue RED offload
  net: sched: mq: request stats from offloads
  nfp: abm: report correct MQ stats

 drivers/net/ethernet/netronome/nfp/abm/ctrl.c | 275 +++++++++++++
 drivers/net/ethernet/netronome/nfp/abm/main.c | 374 +++++++++++++++++-
 drivers/net/ethernet/netronome/nfp/abm/main.h |  67 ++++
 drivers/net/ethernet/netronome/nfp/nfp_abi.h  |  14 +
 drivers/net/ethernet/netronome/nfp/nfp_app.c  |  22 ++
 drivers/net/ethernet/netronome/nfp/nfp_app.h  |  13 +
 .../ethernet/netronome/nfp/nfp_net_common.c   |  11 +-
 .../ethernet/netronome/nfp/nfp_net_ethtool.c  |  10 +-
 .../net/ethernet/netronome/nfp/nfp_net_repr.c |   5 +-
 .../net/ethernet/netronome/nfp/nfp_net_repr.h |   7 +-
 drivers/net/ethernet/netronome/nfp/nfp_port.h |   2 +
 .../ethernet/netronome/nfp/nfpcore/nfp_cpp.h  |   5 +
 include/linux/netdevice.h                     |   1 +
 include/net/pkt_cls.h                         |  12 +
 include/net/sch_generic.h                     |   4 +-
 net/sched/sch_mq.c                            |  37 ++
 16 files changed, 843 insertions(+), 16 deletions(-)

-- 
2.17.0

[PATCH net-next 01/14] nfp: return -EOPNOTSUPP from .ndo_get_phys_port_name for VFs

[Jakub Kicinski]

After recent change we started returning 0 from
ndo_get_phys_port_name for VFs.  The name parameter for
ndo_get_phys_port_name is not initialized by the stack so
this can lead to a crash.  We should have kept returning
-EOPNOTSUPP in the first place.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/nfp_net_common.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
index eea11e881bf5..1f572896d1ee 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
@@ -3286,11 +3286,12 @@ nfp_net_get_phys_port_name(struct net_device *netdev, char *name, size_t len)
 	if (nn->port)
 		return nfp_port_get_phys_port_name(netdev, name, len);
 
-	if (!nn->dp.is_vf) {
-		n = snprintf(name, len, "%d", nn->id);
-		if (n >= len)
-			return -EINVAL;
-	}
+	if (nn->dp.is_vf)
+		return -EOPNOTSUPP;
+
+	n = snprintf(name, len, "%d", nn->id);
+	if (n >= len)
+		return -EINVAL;
 
 	return 0;
 }
-- 
2.17.0

[PATCH net-next 02/14] nfp: prefix vNIC phys_port_name with 'n'

[Jakub Kicinski]

Some drivers are using a bare number inside phys_port_name
as VF id and OpenStack's regexps will pick it up.  We can't
use a bare number for your vNICs, prefix the names with 'n'.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/nfp_net_common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
index 1f572896d1ee..75110c8d6a90 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
@@ -3289,7 +3289,7 @@ nfp_net_get_phys_port_name(struct net_device *netdev, char *name, size_t len)
 	if (nn->dp.is_vf)
 		return -EOPNOTSUPP;
 
-	n = snprintf(name, len, "%d", nn->id);
+	n = snprintf(name, len, "n%d", nn->id);
 	if (n >= len)
 		return -EINVAL;
 
-- 
2.17.0

[PATCH net-next 03/14] nfp: abm: enable advanced queuing on demand

[Jakub Kicinski]

ABM NIC FW has a cut-through mode where the PCIe queuing
is bypassed, thus working like our standard NIC FWs.  Use this
mode by default and only enable queuing in switchdev mode where
users can configure it.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Dirk van der Merwe <dirk.vandermerwe@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/abm/ctrl.c | 13 +++++++++++++
 drivers/net/ethernet/netronome/nfp/abm/main.c | 11 +++++++++++
 drivers/net/ethernet/netronome/nfp/abm/main.h |  2 ++
 drivers/net/ethernet/netronome/nfp/nfp_abi.h  | 14 ++++++++++++++
 4 files changed, 40 insertions(+)

diff --git a/drivers/net/ethernet/netronome/nfp/abm/ctrl.c b/drivers/net/ethernet/netronome/nfp/abm/ctrl.c
index e40f6f06417b..676d3afc9bdd 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/ctrl.c
+++ b/drivers/net/ethernet/netronome/nfp/abm/ctrl.c
@@ -36,10 +36,23 @@
 
 #include "../nfpcore/nfp_cpp.h"
 #include "../nfp_app.h"
+#include "../nfp_abi.h"
 #include "../nfp_main.h"
 #include "../nfp_net.h"
 #include "main.h"
 
+int nfp_abm_ctrl_qm_enable(struct nfp_abm *abm)
+{
+	return nfp_mbox_cmd(abm->app->pf, NFP_MBOX_PCIE_ABM_ENABLE,
+			    NULL, 0, NULL, 0);
+}
+
+int nfp_abm_ctrl_qm_disable(struct nfp_abm *abm)
+{
+	return nfp_mbox_cmd(abm->app->pf, NFP_MBOX_PCIE_ABM_DISABLE,
+			    NULL, 0, NULL, 0);
+}
+
 void nfp_abm_ctrl_read_params(struct nfp_abm_link *alink)
 {
 	alink->queue_base = nn_readl(alink->vnic, NFP_NET_CFG_START_RXQ);
diff --git a/drivers/net/ethernet/netronome/nfp/abm/main.c b/drivers/net/ethernet/netronome/nfp/abm/main.c
index 5a12bb20bced..28a18ac62040 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/main.c
+++ b/drivers/net/ethernet/netronome/nfp/abm/main.c
@@ -182,6 +182,7 @@ static enum devlink_eswitch_mode nfp_abm_eswitch_mode_get(struct nfp_app *app)
 static int nfp_abm_eswitch_set_legacy(struct nfp_abm *abm)
 {
 	nfp_abm_kill_reprs_all(abm);
+	nfp_abm_ctrl_qm_disable(abm);
 
 	abm->eswitch_mode = DEVLINK_ESWITCH_MODE_LEGACY;
 	return 0;
@@ -200,6 +201,10 @@ static int nfp_abm_eswitch_set_switchdev(struct nfp_abm *abm)
 	struct nfp_net *nn;
 	int err;
 
+	err = nfp_abm_ctrl_qm_enable(abm);
+	if (err)
+		return err;
+
 	list_for_each_entry(nn, &pf->vnics, vnic_list) {
 		struct nfp_abm_link *alink = nn->app_priv;
 
@@ -217,6 +222,7 @@ static int nfp_abm_eswitch_set_switchdev(struct nfp_abm *abm)
 
 err_kill_all_reprs:
 	nfp_abm_kill_reprs_all(abm);
+	nfp_abm_ctrl_qm_disable(abm);
 	return err;
 }
 
@@ -350,6 +356,11 @@ static int nfp_abm_init(struct nfp_app *app)
 	if (err)
 		goto err_free_abm;
 
+	/* We start in legacy mode, make sure advanced queuing is disabled */
+	err = nfp_abm_ctrl_qm_disable(abm);
+	if (err)
+		goto err_free_abm;
+
 	err = -ENOMEM;
 	reprs = nfp_reprs_alloc(pf->max_data_vnics);
 	if (!reprs)
diff --git a/drivers/net/ethernet/netronome/nfp/abm/main.h b/drivers/net/ethernet/netronome/nfp/abm/main.h
index 5938b69b8a84..7d129b205535 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/main.h
+++ b/drivers/net/ethernet/netronome/nfp/abm/main.h
@@ -72,4 +72,6 @@ struct nfp_abm_link {
 
 void nfp_abm_ctrl_read_params(struct nfp_abm_link *alink);
 int nfp_abm_ctrl_find_addrs(struct nfp_abm *abm);
+int nfp_abm_ctrl_qm_enable(struct nfp_abm *abm);
+int nfp_abm_ctrl_qm_disable(struct nfp_abm *abm);
 #endif
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_abi.h b/drivers/net/ethernet/netronome/nfp/nfp_abi.h
index 7ffa6e6a9d1c..8b56c27931bf 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_abi.h
+++ b/drivers/net/ethernet/netronome/nfp/nfp_abi.h
@@ -59,12 +59,26 @@
  * @NFP_MBOX_POOL_SET:	set shared buffer pool info/config
  * Input  - struct nfp_shared_buf_pool_info_set
  * Output - None
+ *
+ * @NFP_MBOX_PCIE_ABM_ENABLE:	enable PCIe-side advanced buffer management
+ * Enable advanced buffer management of the PCIe block.  If ABM is disabled
+ * PCIe block maintains a very short queue of buffers and does tail drop.
+ * ABM allows more advanced buffering and priority control.
+ * Input  - None
+ * Output - None
+ *
+ * @NFP_MBOX_PCIE_ABM_DISABLE:	disable PCIe-side advanced buffer management
+ * Input  - None
+ * Output - None
  */
 enum nfp_mbox_cmd {
 	NFP_MBOX_NO_CMD			= 0x00,
 
 	NFP_MBOX_POOL_GET		= 0x01,
 	NFP_MBOX_POOL_SET		= 0x02,
+
+	NFP_MBOX_PCIE_ABM_ENABLE	= 0x03,
+	NFP_MBOX_PCIE_ABM_DISABLE	= 0x04,
 };
 
 #define NFP_SHARED_BUF_COUNT_SYM_NAME	"_abi_nfd_pf%u_sb_cnt"
-- 
2.17.0

[PATCH net-next 04/14] nfp: abm: add helpers for configuring queue marking levels

[Jakub Kicinski]

Queue levels for simple ECN marking are stored in _abi_nfd_out_q_lvls_X
symbol, where X is the PCIe PF id.  Find out the location of that symbol
and add helpers for modifying it.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/abm/ctrl.c | 80 +++++++++++++++++++
 drivers/net/ethernet/netronome/nfp/abm/main.h |  3 +
 .../ethernet/netronome/nfp/nfpcore/nfp_cpp.h  |  5 ++
 3 files changed, 88 insertions(+)

diff --git a/drivers/net/ethernet/netronome/nfp/abm/ctrl.c b/drivers/net/ethernet/netronome/nfp/abm/ctrl.c
index 676d3afc9bdd..978884a0be19 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/ctrl.c
+++ b/drivers/net/ethernet/netronome/nfp/abm/ctrl.c
@@ -35,12 +35,57 @@
 #include <linux/kernel.h>
 
 #include "../nfpcore/nfp_cpp.h"
+#include "../nfpcore/nfp_nffw.h"
 #include "../nfp_app.h"
 #include "../nfp_abi.h"
 #include "../nfp_main.h"
 #include "../nfp_net.h"
 #include "main.h"
 
+#define NFP_QLVL_SYM_NAME	"_abi_nfd_out_q_lvls_%u"
+#define NFP_QLVL_STRIDE		16
+#define NFP_QLVL_THRS		8
+
+static unsigned long long
+nfp_abm_q_lvl_thrs(struct nfp_abm_link *alink, unsigned int queue)
+{
+	return alink->abm->q_lvls->addr +
+		(alink->queue_base + queue) * NFP_QLVL_STRIDE + NFP_QLVL_THRS;
+}
+
+static int
+nfp_abm_ctrl_set_q_lvl(struct nfp_abm_link *alink, unsigned int i, u32 val)
+{
+	struct nfp_cpp *cpp = alink->abm->app->cpp;
+	u32 muw;
+	int err;
+
+	muw = NFP_CPP_ATOMIC_WR(alink->abm->q_lvls->target,
+				alink->abm->q_lvls->domain);
+
+	err = nfp_cpp_writel(cpp, muw, nfp_abm_q_lvl_thrs(alink, i), val);
+	if (err) {
+		nfp_err(cpp, "RED offload setting level failed on vNIC %d queue %d\n",
+			alink->id, i);
+		return err;
+	}
+
+	return 0;
+}
+
+int nfp_abm_ctrl_set_all_q_lvls(struct nfp_abm_link *alink, u32 val)
+{
+	int i, err;
+
+	for (i = 0; i < alink->vnic->max_rx_rings; i++) {
+		err = nfp_abm_ctrl_set_q_lvl(alink, i, val);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
 int nfp_abm_ctrl_qm_enable(struct nfp_abm *abm)
 {
 	return nfp_mbox_cmd(abm->app->pf, NFP_MBOX_PCIE_ABM_ENABLE,
@@ -59,13 +104,48 @@ void nfp_abm_ctrl_read_params(struct nfp_abm_link *alink)
 	alink->queue_base /= alink->vnic->stride_rx;
 }
 
+static const struct nfp_rtsym *
+nfp_abm_ctrl_find_rtsym(struct nfp_pf *pf, const char *name, unsigned int size)
+{
+	const struct nfp_rtsym *sym;
+
+	sym = nfp_rtsym_lookup(pf->rtbl, name);
+	if (!sym) {
+		nfp_err(pf->cpp, "Symbol '%s' not found\n", name);
+		return ERR_PTR(-ENOENT);
+	}
+	if (sym->size != size) {
+		nfp_err(pf->cpp,
+			"Symbol '%s' wrong size: expected %u got %llu\n",
+			name, size, sym->size);
+		return ERR_PTR(-EINVAL);
+	}
+
+	return sym;
+}
+
+static const struct nfp_rtsym *
+nfp_abm_ctrl_find_q_rtsym(struct nfp_pf *pf, const char *name,
+			  unsigned int size)
+{
+	return nfp_abm_ctrl_find_rtsym(pf, name, size * NFP_NET_MAX_RX_RINGS);
+}
+
 int nfp_abm_ctrl_find_addrs(struct nfp_abm *abm)
 {
 	struct nfp_pf *pf = abm->app->pf;
+	const struct nfp_rtsym *sym;
 	unsigned int pf_id;
+	char pf_symbol[64];
 
 	pf_id =	nfp_cppcore_pcie_unit(pf->cpp);
 	abm->pf_id = pf_id;
 
+	snprintf(pf_symbol, sizeof(pf_symbol), NFP_QLVL_SYM_NAME, pf_id);
+	sym = nfp_abm_ctrl_find_q_rtsym(pf, pf_symbol, NFP_QLVL_STRIDE);
+	if (IS_ERR(sym))
+		return PTR_ERR(sym);
+	abm->q_lvls = sym;
+
 	return 0;
 }
diff --git a/drivers/net/ethernet/netronome/nfp/abm/main.h b/drivers/net/ethernet/netronome/nfp/abm/main.h
index 7d129b205535..1ac651cdc140 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/main.h
+++ b/drivers/net/ethernet/netronome/nfp/abm/main.h
@@ -49,11 +49,13 @@ struct nfp_net;
  * @pf_id:	ID of our PF link
  * @eswitch_mode:	devlink eswitch mode, advanced functions only visible
  *			in switchdev mode
+ * @q_lvls:	queue level control area
  */
 struct nfp_abm {
 	struct nfp_app *app;
 	unsigned int pf_id;
 	enum devlink_eswitch_mode eswitch_mode;
+	const struct nfp_rtsym *q_lvls;
 };
 
 /**
@@ -72,6 +74,7 @@ struct nfp_abm_link {
 
 void nfp_abm_ctrl_read_params(struct nfp_abm_link *alink);
 int nfp_abm_ctrl_find_addrs(struct nfp_abm *abm);
+int nfp_abm_ctrl_set_all_q_lvls(struct nfp_abm_link *alink, u32 val);
 int nfp_abm_ctrl_qm_enable(struct nfp_abm *abm);
 int nfp_abm_ctrl_qm_disable(struct nfp_abm *abm);
 #endif
diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_cpp.h b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_cpp.h
index 4e19add1c539..b0da3d436850 100644
--- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_cpp.h
+++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_cpp.h
@@ -87,6 +87,11 @@ struct resource;
 
 #define NFP_CPP_TARGET_ID_MASK          0x1f
 
+#define NFP_CPP_ATOMIC_RD(target, island) \
+	NFP_CPP_ISLAND_ID((target), 3, 0, (island))
+#define NFP_CPP_ATOMIC_WR(target, island) \
+	NFP_CPP_ISLAND_ID((target), 4, 0, (island))
+
 /**
  * NFP_CPP_ID() - pack target, token, and action into a CPP ID.
  * @target:     NFP CPP target id
-- 
2.17.0

[PATCH net-next 05/14] nfp: abm: add simple RED offload

[Jakub Kicinski]

Offload simple RED configurations.  For now support only DCTCP
like scenarios where min and max are the same.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/abm/main.c | 82 +++++++++++++++++++
 drivers/net/ethernet/netronome/nfp/abm/main.h | 10 +++
 2 files changed, 92 insertions(+)

diff --git a/drivers/net/ethernet/netronome/nfp/abm/main.c b/drivers/net/ethernet/netronome/nfp/abm/main.c
index 28a18ac62040..22251d88c958 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/main.c
+++ b/drivers/net/ethernet/netronome/nfp/abm/main.c
@@ -38,6 +38,8 @@
 #include <linux/netdevice.h>
 #include <linux/rcupdate.h>
 #include <linux/slab.h>
+#include <net/pkt_cls.h>
+#include <net/pkt_sched.h>
 
 #include "../nfpcore/nfp.h"
 #include "../nfpcore/nfp_cpp.h"
@@ -55,6 +57,84 @@ static u32 nfp_abm_portid(enum nfp_repr_type rtype, unsigned int id)
 	       FIELD_PREP(NFP_ABM_PORTID_ID, id);
 }
 
+static void
+nfp_abm_red_destroy(struct net_device *netdev, struct nfp_abm_link *alink,
+		    u32 handle)
+{
+	struct nfp_port *port = nfp_port_from_netdev(netdev);
+
+	if (handle != alink->qdiscs[0].handle)
+		return;
+
+	alink->qdiscs[0].handle = TC_H_UNSPEC;
+	port->tc_offload_cnt = 0;
+	nfp_abm_ctrl_set_all_q_lvls(alink, ~0);
+}
+
+static int
+nfp_abm_red_replace(struct net_device *netdev, struct nfp_abm_link *alink,
+		    struct tc_red_qopt_offload *opt)
+{
+	struct nfp_port *port = nfp_port_from_netdev(netdev);
+	int err;
+
+	if (opt->set.min != opt->set.max || !opt->set.is_ecn) {
+		nfp_warn(alink->abm->app->cpp,
+			 "RED offload failed - unsupported parameters\n");
+		err = -EINVAL;
+		goto err_destroy;
+	}
+	err = nfp_abm_ctrl_set_all_q_lvls(alink, opt->set.min);
+	if (err)
+		goto err_destroy;
+
+	alink->qdiscs[0].handle = opt->handle;
+	port->tc_offload_cnt = 1;
+
+	return 0;
+err_destroy:
+	if (alink->qdiscs[0].handle != TC_H_UNSPEC)
+		nfp_abm_red_destroy(netdev, alink, alink->qdiscs[0].handle);
+	return err;
+}
+
+static int
+nfp_abm_setup_tc_red(struct net_device *netdev, struct nfp_abm_link *alink,
+		     struct tc_red_qopt_offload *opt)
+{
+	if (opt->parent != TC_H_ROOT)
+		return -EOPNOTSUPP;
+
+	switch (opt->command) {
+	case TC_RED_REPLACE:
+		return nfp_abm_red_replace(netdev, alink, opt);
+	case TC_RED_DESTROY:
+		nfp_abm_red_destroy(netdev, alink, opt->handle);
+		return 0;
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static int
+nfp_abm_setup_tc(struct nfp_app *app, struct net_device *netdev,
+		 enum tc_setup_type type, void *type_data)
+{
+	struct nfp_repr *repr = netdev_priv(netdev);
+	struct nfp_port *port;
+
+	port = nfp_port_from_netdev(netdev);
+	if (!port || port->type != NFP_PORT_PF_PORT)
+		return -EOPNOTSUPP;
+
+	switch (type) {
+	case TC_SETUP_QDISC_RED:
+		return nfp_abm_setup_tc_red(netdev, repr->app_priv, type_data);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
 static struct net_device *nfp_abm_repr_get(struct nfp_app *app, u32 port_id)
 {
 	enum nfp_repr_type rtype;
@@ -403,6 +483,8 @@ const struct nfp_app_type app_abm = {
 	.vnic_alloc	= nfp_abm_vnic_alloc,
 	.vnic_free	= nfp_abm_vnic_free,
 
+	.setup_tc	= nfp_abm_setup_tc,
+
 	.eswitch_mode_get	= nfp_abm_eswitch_mode_get,
 	.eswitch_mode_set	= nfp_abm_eswitch_mode_set,
 
diff --git a/drivers/net/ethernet/netronome/nfp/abm/main.h b/drivers/net/ethernet/netronome/nfp/abm/main.h
index 1ac651cdc140..979f98fb808b 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/main.h
+++ b/drivers/net/ethernet/netronome/nfp/abm/main.h
@@ -58,18 +58,28 @@ struct nfp_abm {
 	const struct nfp_rtsym *q_lvls;
 };
 
+/**
+ * struct nfp_red_qdisc - representation of single RED Qdisc
+ * @handle:	handle of currently offloaded RED Qdisc
+ */
+struct nfp_red_qdisc {
+	u32 handle;
+};
+
 /**
  * struct nfp_abm_link - port tuple of a ABM NIC
  * @abm:	back pointer to nfp_abm
  * @vnic:	data vNIC
  * @id:		id of the data vNIC
  * @queue_base:	id of base to host queue within PCIe (not QC idx)
+ * @qdiscs:	array of qdiscs
  */
 struct nfp_abm_link {
 	struct nfp_abm *abm;
 	struct nfp_net *vnic;
 	unsigned int id;
 	unsigned int queue_base;
+	struct nfp_red_qdisc qdiscs[1];
 };
 
 void nfp_abm_ctrl_read_params(struct nfp_abm_link *alink);
-- 
2.17.0

[PATCH net-next 06/14] net: sched: add qstats.qlen to qlen

[Jakub Kicinski]

AFAICT struct gnet_stats_queue.qlen is not used in Qdiscs.
It may, however, be useful for offloads to report HW queue
length there.  Add that value to the result of qdisc_qlen_sum().

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 include/net/sch_generic.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 98c10a28cd01..0b786c8204b9 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -350,14 +350,14 @@ static inline int qdisc_qlen(const struct Qdisc *q)
 
 static inline int qdisc_qlen_sum(const struct Qdisc *q)
 {
-	__u32 qlen = 0;
+	__u32 qlen = q->qstats.qlen;
 	int i;
 
 	if (q->flags & TCQ_F_NOLOCK) {
 		for_each_possible_cpu(i)
 			qlen += per_cpu_ptr(q->cpu_qstats, i)->qlen;
 	} else {
-		qlen = q->q.qlen;
+		qlen += q->q.qlen;
 	}
 
 	return qlen;
-- 
2.17.0

[PATCH net-next 07/14] nfp: abm: report statistics from RED offload

[Jakub Kicinski]

Report basic and extended RED statistics back to TC.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Dirk van der Merwe <dirk.vandermerwe@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/abm/ctrl.c | 114 ++++++++++++++++++
 drivers/net/ethernet/netronome/nfp/abm/main.c |  92 ++++++++++++++
 drivers/net/ethernet/netronome/nfp/abm/main.h |  38 ++++++
 3 files changed, 244 insertions(+)

diff --git a/drivers/net/ethernet/netronome/nfp/abm/ctrl.c b/drivers/net/ethernet/netronome/nfp/abm/ctrl.c
index 978884a0be19..d2d9ca7a727c 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/ctrl.c
+++ b/drivers/net/ethernet/netronome/nfp/abm/ctrl.c
@@ -44,8 +44,15 @@
 
 #define NFP_QLVL_SYM_NAME	"_abi_nfd_out_q_lvls_%u"
 #define NFP_QLVL_STRIDE		16
+#define NFP_QLVL_BLOG_BYTES	0
+#define NFP_QLVL_BLOG_PKTS	4
 #define NFP_QLVL_THRS		8
 
+#define NFP_QMSTAT_SYM_NAME	"_abi_nfdqm%u_stats"
+#define NFP_QMSTAT_STRIDE	32
+#define NFP_QMSTAT_DROP		16
+#define NFP_QMSTAT_ECN		24
+
 static unsigned long long
 nfp_abm_q_lvl_thrs(struct nfp_abm_link *alink, unsigned int queue)
 {
@@ -53,6 +60,55 @@ nfp_abm_q_lvl_thrs(struct nfp_abm_link *alink, unsigned int queue)
 		(alink->queue_base + queue) * NFP_QLVL_STRIDE + NFP_QLVL_THRS;
 }
 
+static int
+nfp_abm_ctrl_stat(struct nfp_abm_link *alink, const struct nfp_rtsym *sym,
+		  unsigned int stride, unsigned int offset, unsigned int i,
+		  bool is_u64, u64 *res)
+{
+	struct nfp_cpp *cpp = alink->abm->app->cpp;
+	u32 val32, mur;
+	u64 val, addr;
+	int err;
+
+	mur = NFP_CPP_ATOMIC_RD(sym->target, sym->domain);
+
+	addr = sym->addr + (alink->queue_base + i) * stride + offset;
+	if (is_u64)
+		err = nfp_cpp_readq(cpp, mur, addr, &val);
+	else
+		err = nfp_cpp_readl(cpp, mur, addr, &val32);
+	if (err) {
+		nfp_err(cpp,
+			"RED offload reading stat failed on vNIC %d queue %d\n",
+			alink->id, i);
+		return err;
+	}
+
+	*res = is_u64 ? val : val32;
+	return 0;
+}
+
+static int
+nfp_abm_ctrl_stat_all(struct nfp_abm_link *alink, const struct nfp_rtsym *sym,
+		      unsigned int stride, unsigned int offset, bool is_u64,
+		      u64 *res)
+{
+	u64 val, sum = 0;
+	unsigned int i;
+	int err;
+
+	for (i = 0; i < alink->vnic->max_rx_rings; i++) {
+		err = nfp_abm_ctrl_stat(alink, sym, stride, offset, i,
+					is_u64, &val);
+		if (err)
+			return err;
+		sum += val;
+	}
+
+	*res = sum;
+	return 0;
+}
+
 static int
 nfp_abm_ctrl_set_q_lvl(struct nfp_abm_link *alink, unsigned int i, u32 val)
 {
@@ -86,6 +142,58 @@ int nfp_abm_ctrl_set_all_q_lvls(struct nfp_abm_link *alink, u32 val)
 	return 0;
 }
 
+int nfp_abm_ctrl_read_stats(struct nfp_abm_link *alink,
+			    struct nfp_alink_stats *stats)
+{
+	u64 pkts = 0, bytes = 0;
+	int i, err;
+
+	for (i = 0; i < alink->vnic->max_rx_rings; i++) {
+		pkts += nn_readq(alink->vnic, NFP_NET_CFG_RXR_STATS(i));
+		bytes += nn_readq(alink->vnic, NFP_NET_CFG_RXR_STATS(i) + 8);
+	}
+	stats->tx_pkts = pkts;
+	stats->tx_bytes = bytes;
+
+	err = nfp_abm_ctrl_stat_all(alink, alink->abm->q_lvls,
+				    NFP_QLVL_STRIDE, NFP_QLVL_BLOG_BYTES,
+				    false, &stats->backlog_bytes);
+	if (err)
+		return err;
+
+	err = nfp_abm_ctrl_stat_all(alink, alink->abm->q_lvls,
+				    NFP_QLVL_STRIDE, NFP_QLVL_BLOG_PKTS,
+				    false, &stats->backlog_pkts);
+	if (err)
+		return err;
+
+	err = nfp_abm_ctrl_stat_all(alink, alink->abm->qm_stats,
+				    NFP_QMSTAT_STRIDE, NFP_QMSTAT_DROP,
+				    true, &stats->drops);
+	if (err)
+		return err;
+
+	return nfp_abm_ctrl_stat_all(alink, alink->abm->qm_stats,
+				     NFP_QMSTAT_STRIDE, NFP_QMSTAT_ECN,
+				     true, &stats->overlimits);
+}
+
+int nfp_abm_ctrl_read_xstats(struct nfp_abm_link *alink,
+			     struct nfp_alink_xstats *xstats)
+{
+	int err;
+
+	err = nfp_abm_ctrl_stat_all(alink, alink->abm->qm_stats,
+				    NFP_QMSTAT_STRIDE, NFP_QMSTAT_DROP,
+				    true, &xstats->pdrop);
+	if (err)
+		return err;
+
+	return nfp_abm_ctrl_stat_all(alink, alink->abm->qm_stats,
+				     NFP_QMSTAT_STRIDE, NFP_QMSTAT_ECN,
+				     true, &xstats->ecn_marked);
+}
+
 int nfp_abm_ctrl_qm_enable(struct nfp_abm *abm)
 {
 	return nfp_mbox_cmd(abm->app->pf, NFP_MBOX_PCIE_ABM_ENABLE,
@@ -147,5 +255,11 @@ int nfp_abm_ctrl_find_addrs(struct nfp_abm *abm)
 		return PTR_ERR(sym);
 	abm->q_lvls = sym;
 
+	snprintf(pf_symbol, sizeof(pf_symbol), NFP_QMSTAT_SYM_NAME, pf_id);
+	sym = nfp_abm_ctrl_find_q_rtsym(pf, pf_symbol, NFP_QMSTAT_STRIDE);
+	if (IS_ERR(sym))
+		return PTR_ERR(sym);
+	abm->qm_stats = sym;
+
 	return 0;
 }
diff --git a/drivers/net/ethernet/netronome/nfp/abm/main.c b/drivers/net/ethernet/netronome/nfp/abm/main.c
index 22251d88c958..d0c21899a8b7 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/main.c
+++ b/drivers/net/ethernet/netronome/nfp/abm/main.c
@@ -40,6 +40,7 @@
 #include <linux/slab.h>
 #include <net/pkt_cls.h>
 #include <net/pkt_sched.h>
+#include <net/red.h>
 
 #include "../nfpcore/nfp.h"
 #include "../nfpcore/nfp_cpp.h"
@@ -57,6 +58,23 @@ static u32 nfp_abm_portid(enum nfp_repr_type rtype, unsigned int id)
 	       FIELD_PREP(NFP_ABM_PORTID_ID, id);
 }
 
+static int nfp_abm_reset_stats(struct nfp_abm_link *alink)
+{
+	int err;
+
+	err = nfp_abm_ctrl_read_stats(alink, &alink->qdiscs[0].stats);
+	if (err)
+		return err;
+	alink->qdiscs[0].stats.backlog_pkts = 0;
+	alink->qdiscs[0].stats.backlog_bytes = 0;
+
+	err = nfp_abm_ctrl_read_xstats(alink, &alink->qdiscs[0].xstats);
+	if (err)
+		return err;
+
+	return 0;
+}
+
 static void
 nfp_abm_red_destroy(struct net_device *netdev, struct nfp_abm_link *alink,
 		    u32 handle)
@@ -88,16 +106,86 @@ nfp_abm_red_replace(struct net_device *netdev, struct nfp_abm_link *alink,
 	if (err)
 		goto err_destroy;
 
+	/* Reset stats only on new qdisc */
+	if (alink->qdiscs[0].handle != opt->handle) {
+		err = nfp_abm_reset_stats(alink);
+		if (err)
+			goto err_destroy;
+	}
+
 	alink->qdiscs[0].handle = opt->handle;
 	port->tc_offload_cnt = 1;
 
 	return 0;
 err_destroy:
+	/* If the qdisc keeps on living, but we can't offload undo changes */
+	if (alink->qdiscs[0].handle == opt->handle) {
+		opt->set.qstats->qlen -= alink->qdiscs[0].stats.backlog_pkts;
+		opt->set.qstats->backlog -=
+			alink->qdiscs[0].stats.backlog_bytes;
+	}
 	if (alink->qdiscs[0].handle != TC_H_UNSPEC)
 		nfp_abm_red_destroy(netdev, alink, alink->qdiscs[0].handle);
 	return err;
 }
 
+static void
+nfp_abm_update_stats(struct nfp_alink_stats *new, struct nfp_alink_stats *old,
+		     struct tc_qopt_offload_stats *stats)
+{
+	_bstats_update(stats->bstats, new->tx_bytes - old->tx_bytes,
+		       new->tx_pkts - old->tx_pkts);
+	stats->qstats->qlen += new->backlog_pkts - old->backlog_pkts;
+	stats->qstats->backlog += new->backlog_bytes - old->backlog_bytes;
+	stats->qstats->overlimits += new->overlimits - old->overlimits;
+	stats->qstats->drops += new->drops - old->drops;
+}
+
+static int
+nfp_abm_red_stats(struct nfp_abm_link *alink, struct tc_red_qopt_offload *opt)
+{
+	struct nfp_alink_stats *prev_stats;
+	struct nfp_alink_stats stats;
+	int err;
+
+	if (alink->qdiscs[0].handle != opt->handle)
+		return -EOPNOTSUPP;
+	prev_stats = &alink->qdiscs[0].stats;
+
+	err = nfp_abm_ctrl_read_stats(alink, &stats);
+	if (err)
+		return err;
+
+	nfp_abm_update_stats(&stats, prev_stats, &opt->stats);
+
+	*prev_stats = stats;
+
+	return 0;
+}
+
+static int
+nfp_abm_red_xstats(struct nfp_abm_link *alink, struct tc_red_qopt_offload *opt)
+{
+	struct nfp_alink_xstats *prev_xstats;
+	struct nfp_alink_xstats xstats;
+	int err;
+
+	if (alink->qdiscs[0].handle != opt->handle)
+		return -EOPNOTSUPP;
+	prev_xstats = &alink->qdiscs[0].xstats;
+
+	err = nfp_abm_ctrl_read_xstats(alink, &xstats);
+	if (err)
+		return err;
+
+	opt->xstats->forced_mark += xstats.ecn_marked - prev_xstats->ecn_marked;
+	opt->xstats->pdrop += xstats.pdrop - prev_xstats->pdrop;
+
+	*prev_xstats = xstats;
+
+	return 0;
+}
+
 static int
 nfp_abm_setup_tc_red(struct net_device *netdev, struct nfp_abm_link *alink,
 		     struct tc_red_qopt_offload *opt)
@@ -111,6 +199,10 @@ nfp_abm_setup_tc_red(struct net_device *netdev, struct nfp_abm_link *alink,
 	case TC_RED_DESTROY:
 		nfp_abm_red_destroy(netdev, alink, opt->handle);
 		return 0;
+	case TC_RED_STATS:
+		return nfp_abm_red_stats(alink, opt);
+	case TC_RED_XSTATS:
+		return nfp_abm_red_xstats(alink, opt);
 	default:
 		return -EOPNOTSUPP;
 	}
diff --git a/drivers/net/ethernet/netronome/nfp/abm/main.h b/drivers/net/ethernet/netronome/nfp/abm/main.h
index 979f98fb808b..93a3b79cf468 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/main.h
+++ b/drivers/net/ethernet/netronome/nfp/abm/main.h
@@ -50,20 +50,54 @@ struct nfp_net;
  * @eswitch_mode:	devlink eswitch mode, advanced functions only visible
  *			in switchdev mode
  * @q_lvls:	queue level control area
+ * @qm_stats:	queue statistics symbol
  */
 struct nfp_abm {
 	struct nfp_app *app;
 	unsigned int pf_id;
 	enum devlink_eswitch_mode eswitch_mode;
 	const struct nfp_rtsym *q_lvls;
+	const struct nfp_rtsym *qm_stats;
+};
+
+/**
+ * struct nfp_alink_stats - ABM NIC statistics
+ * @tx_pkts:		number of TXed packets
+ * @tx_bytes:		number of TXed bytes
+ * @backlog_pkts:	momentary backlog length (packets)
+ * @backlog_bytes:	momentary backlog length (bytes)
+ * @overlimits:		number of ECN marked TXed packets (accumulative)
+ * @drops:		number of tail-dropped packets (accumulative)
+ */
+struct nfp_alink_stats {
+	u64 tx_pkts;
+	u64 tx_bytes;
+	u64 backlog_pkts;
+	u64 backlog_bytes;
+	u64 overlimits;
+	u64 drops;
+};
+
+/**
+ * struct nfp_alink_xstats - extended ABM NIC statistics
+ * @ecn_marked:		number of ECN marked TXed packets
+ * @pdrop:		number of hard drops due to queue limit
+ */
+struct nfp_alink_xstats {
+	u64 ecn_marked;
+	u64 pdrop;
 };
 
 /**
  * struct nfp_red_qdisc - representation of single RED Qdisc
  * @handle:	handle of currently offloaded RED Qdisc
+ * @stats:	statistics from last refresh
+ * @xstats:	base of extended statistics
  */
 struct nfp_red_qdisc {
 	u32 handle;
+	struct nfp_alink_stats stats;
+	struct nfp_alink_xstats xstats;
 };
 
 /**
@@ -85,6 +119,10 @@ struct nfp_abm_link {
 void nfp_abm_ctrl_read_params(struct nfp_abm_link *alink);
 int nfp_abm_ctrl_find_addrs(struct nfp_abm *abm);
 int nfp_abm_ctrl_set_all_q_lvls(struct nfp_abm_link *alink, u32 val);
+int nfp_abm_ctrl_read_stats(struct nfp_abm_link *alink,
+			    struct nfp_alink_stats *stats);
+int nfp_abm_ctrl_read_xstats(struct nfp_abm_link *alink,
+			     struct nfp_alink_xstats *xstats);
 int nfp_abm_ctrl_qm_enable(struct nfp_abm *abm);
 int nfp_abm_ctrl_qm_disable(struct nfp_abm *abm);
 #endif
-- 
2.17.0

[PATCH net-next 08/14] nfp: allow apps to add extra stats to ports

[Jakub Kicinski]

Allow nfp apps to add extra ethtool stats.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/nfp_app.c  | 22 +++++++++++++++++++
 drivers/net/ethernet/netronome/nfp/nfp_app.h  | 13 +++++++++++
 .../ethernet/netronome/nfp/nfp_net_ethtool.c  | 10 +++++++--
 drivers/net/ethernet/netronome/nfp/nfp_port.h |  2 ++
 4 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_app.c b/drivers/net/ethernet/netronome/nfp/nfp_app.c
index c9d8a7ab311e..f28b244f4ee7 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_app.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_app.c
@@ -43,6 +43,7 @@
 #include "nfp_main.h"
 #include "nfp_net.h"
 #include "nfp_net_repr.h"
+#include "nfp_port.h"
 
 static const struct nfp_app_type *apps[] = {
 	[NFP_APP_CORE_NIC]	= &app_nic,
@@ -85,6 +86,27 @@ const char *nfp_app_mip_name(struct nfp_app *app)
 	return nfp_mip_name(app->pf->mip);
 }
 
+u64 *nfp_app_port_get_stats(struct nfp_port *port, u64 *data)
+{
+	if (!port || !port->app || !port->app->type->port_get_stats)
+		return data;
+	return port->app->type->port_get_stats(port->app, port, data);
+}
+
+int nfp_app_port_get_stats_count(struct nfp_port *port)
+{
+	if (!port || !port->app || !port->app->type->port_get_stats_count)
+		return 0;
+	return port->app->type->port_get_stats_count(port->app, port);
+}
+
+u8 *nfp_app_port_get_stats_strings(struct nfp_port *port, u8 *data)
+{
+	if (!port || !port->app || !port->app->type->port_get_stats_strings)
+		return data;
+	return port->app->type->port_get_stats_strings(port->app, port, data);
+}
+
 struct sk_buff *
 nfp_app_ctrl_msg_alloc(struct nfp_app *app, unsigned int size, gfp_t priority)
 {
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_app.h b/drivers/net/ethernet/netronome/nfp/nfp_app.h
index 23b99a4e05c2..ee74caacb015 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_app.h
+++ b/drivers/net/ethernet/netronome/nfp/nfp_app.h
@@ -90,6 +90,9 @@ extern const struct nfp_app_type app_abm;
  * @repr_stop:	representor netdev stop callback
  * @check_mtu:	MTU change request on a netdev (verify it is valid)
  * @repr_change_mtu:	MTU change request on repr (make and verify change)
+ * @port_get_stats:		get extra ethtool statistics for a port
+ * @port_get_stats_count:	get count of extra statistics for a port
+ * @port_get_stats_strings:	get strings for extra statistics
  * @start:	start application logic
  * @stop:	stop application logic
  * @ctrl_msg_rx:    control message handler
@@ -132,6 +135,12 @@ struct nfp_app_type {
 	int (*repr_change_mtu)(struct nfp_app *app, struct net_device *netdev,
 			       int new_mtu);
 
+	u64 *(*port_get_stats)(struct nfp_app *app,
+			       struct nfp_port *port, u64 *data);
+	int (*port_get_stats_count)(struct nfp_app *app, struct nfp_port *port);
+	u8 *(*port_get_stats_strings)(struct nfp_app *app,
+				      struct nfp_port *port, u8 *data);
+
 	int (*start)(struct nfp_app *app);
 	void (*stop)(struct nfp_app *app);
 
@@ -404,6 +413,10 @@ static inline struct net_device *nfp_app_repr_get(struct nfp_app *app, u32 id)
 
 struct nfp_app *nfp_app_from_netdev(struct net_device *netdev);
 
+u64 *nfp_app_port_get_stats(struct nfp_port *port, u64 *data);
+int nfp_app_port_get_stats_count(struct nfp_port *port);
+u8 *nfp_app_port_get_stats_strings(struct nfp_port *port, u8 *data);
+
 struct nfp_reprs *
 nfp_reprs_get_locked(struct nfp_app *app, enum nfp_repr_type type);
 struct nfp_reprs *
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
index c9016419bfa0..26d1cc4e2906 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
@@ -437,7 +437,7 @@ static int nfp_net_set_ringparam(struct net_device *netdev,
 	return nfp_net_set_ring_size(nn, rxd_cnt, txd_cnt);
 }
 
-static __printf(2, 3) u8 *nfp_pr_et(u8 *data, const char *fmt, ...)
+__printf(2, 3) u8 *nfp_pr_et(u8 *data, const char *fmt, ...)
 {
 	va_list args;
 
@@ -637,6 +637,7 @@ static void nfp_net_get_strings(struct net_device *netdev,
 						     nn->dp.num_tx_rings,
 						     false);
 		data = nfp_mac_get_stats_strings(netdev, data);
+		data = nfp_app_port_get_stats_strings(nn->port, data);
 		break;
 	}
 }
@@ -651,6 +652,7 @@ nfp_net_get_stats(struct net_device *netdev, struct ethtool_stats *stats,
 	data = nfp_vnic_get_hw_stats(data, nn->dp.ctrl_bar,
 				     nn->dp.num_rx_rings, nn->dp.num_tx_rings);
 	data = nfp_mac_get_stats(netdev, data);
+	data = nfp_app_port_get_stats(nn->port, data);
 }
 
 static int nfp_net_get_sset_count(struct net_device *netdev, int sset)
@@ -662,7 +664,8 @@ static int nfp_net_get_sset_count(struct net_device *netdev, int sset)
 		return nfp_vnic_get_sw_stats_count(netdev) +
 		       nfp_vnic_get_hw_stats_count(nn->dp.num_rx_rings,
 						   nn->dp.num_tx_rings) +
-		       nfp_mac_get_stats_count(netdev);
+		       nfp_mac_get_stats_count(netdev) +
+		       nfp_app_port_get_stats_count(nn->port);
 	default:
 		return -EOPNOTSUPP;
 	}
@@ -679,6 +682,7 @@ static void nfp_port_get_strings(struct net_device *netdev,
 			data = nfp_vnic_get_hw_stats_strings(data, 0, 0, true);
 		else
 			data = nfp_mac_get_stats_strings(netdev, data);
+		data = nfp_app_port_get_stats_strings(port, data);
 		break;
 	}
 }
@@ -693,6 +697,7 @@ nfp_port_get_stats(struct net_device *netdev, struct ethtool_stats *stats,
 		data = nfp_vnic_get_hw_stats(data, port->vnic, 0, 0);
 	else
 		data = nfp_mac_get_stats(netdev, data);
+	data = nfp_app_port_get_stats(port, data);
 }
 
 static int nfp_port_get_sset_count(struct net_device *netdev, int sset)
@@ -706,6 +711,7 @@ static int nfp_port_get_sset_count(struct net_device *netdev, int sset)
 			count = nfp_vnic_get_hw_stats_count(0, 0);
 		else
 			count = nfp_mac_get_stats_count(netdev);
+		count += nfp_app_port_get_stats_count(port);
 		return count;
 	default:
 		return -EOPNOTSUPP;
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_port.h b/drivers/net/ethernet/netronome/nfp/nfp_port.h
index 18666750456e..51f10ae2d53e 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_port.h
+++ b/drivers/net/ethernet/netronome/nfp/nfp_port.h
@@ -122,6 +122,8 @@ struct nfp_port {
 extern const struct ethtool_ops nfp_port_ethtool_ops;
 extern const struct switchdev_ops nfp_port_switchdev_ops;
 
+__printf(2, 3) u8 *nfp_pr_et(u8 *data, const char *fmt, ...);
+
 int nfp_port_setup_tc(struct net_device *netdev, enum tc_setup_type type,
 		      void *type_data);
 
-- 
2.17.0

[PATCH net-next 09/14] nfp: abm: expose the internal stats in ethtool

[Jakub Kicinski]

There is a handful of statistics exposing some internal details
of the implementation.  Expose those via ethtool.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/abm/ctrl.c | 22 ++++++++
 drivers/net/ethernet/netronome/nfp/abm/main.c | 51 +++++++++++++++++++
 drivers/net/ethernet/netronome/nfp/abm/main.h |  2 +
 3 files changed, 75 insertions(+)

diff --git a/drivers/net/ethernet/netronome/nfp/abm/ctrl.c b/drivers/net/ethernet/netronome/nfp/abm/ctrl.c
index d2d9ca7a727c..79fc9147c012 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/ctrl.c
+++ b/drivers/net/ethernet/netronome/nfp/abm/ctrl.c
@@ -50,6 +50,8 @@
 
 #define NFP_QMSTAT_SYM_NAME	"_abi_nfdqm%u_stats"
 #define NFP_QMSTAT_STRIDE	32
+#define NFP_QMSTAT_NON_STO	0
+#define NFP_QMSTAT_STO		8
 #define NFP_QMSTAT_DROP		16
 #define NFP_QMSTAT_ECN		24
 
@@ -142,6 +144,26 @@ int nfp_abm_ctrl_set_all_q_lvls(struct nfp_abm_link *alink, u32 val)
 	return 0;
 }
 
+u64 nfp_abm_ctrl_stat_non_sto(struct nfp_abm_link *alink, unsigned int i)
+{
+	u64 val;
+
+	if (nfp_abm_ctrl_stat(alink, alink->abm->qm_stats, NFP_QMSTAT_STRIDE,
+			      NFP_QMSTAT_NON_STO, i, true, &val))
+		return 0;
+	return val;
+}
+
+u64 nfp_abm_ctrl_stat_sto(struct nfp_abm_link *alink, unsigned int i)
+{
+	u64 val;
+
+	if (nfp_abm_ctrl_stat(alink, alink->abm->qm_stats, NFP_QMSTAT_STRIDE,
+			      NFP_QMSTAT_STO, i, true, &val))
+		return 0;
+	return val;
+}
+
 int nfp_abm_ctrl_read_stats(struct nfp_abm_link *alink,
 			    struct nfp_alink_stats *stats)
 {
diff --git a/drivers/net/ethernet/netronome/nfp/abm/main.c b/drivers/net/ethernet/netronome/nfp/abm/main.c
index d0c21899a8b7..4e89159f13d3 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/main.c
+++ b/drivers/net/ethernet/netronome/nfp/abm/main.c
@@ -497,6 +497,53 @@ static void nfp_abm_vnic_free(struct nfp_app *app, struct nfp_net *nn)
 	kfree(alink);
 }
 
+static u64 *
+nfp_abm_port_get_stats(struct nfp_app *app, struct nfp_port *port, u64 *data)
+{
+	struct nfp_repr *repr = netdev_priv(port->netdev);
+	struct nfp_abm_link *alink;
+	unsigned int i;
+
+	if (port->type != NFP_PORT_PF_PORT)
+		return data;
+	alink = repr->app_priv;
+	for (i = 0; i < alink->vnic->dp.num_r_vecs; i++) {
+		*data++ = nfp_abm_ctrl_stat_non_sto(alink, i);
+		*data++ = nfp_abm_ctrl_stat_sto(alink, i);
+	}
+	return data;
+}
+
+static int
+nfp_abm_port_get_stats_count(struct nfp_app *app, struct nfp_port *port)
+{
+	struct nfp_repr *repr = netdev_priv(port->netdev);
+	struct nfp_abm_link *alink;
+
+	if (port->type != NFP_PORT_PF_PORT)
+		return 0;
+	alink = repr->app_priv;
+	return alink->vnic->dp.num_r_vecs * 2;
+}
+
+static u8 *
+nfp_abm_port_get_stats_strings(struct nfp_app *app, struct nfp_port *port,
+			       u8 *data)
+{
+	struct nfp_repr *repr = netdev_priv(port->netdev);
+	struct nfp_abm_link *alink;
+	unsigned int i;
+
+	if (port->type != NFP_PORT_PF_PORT)
+		return data;
+	alink = repr->app_priv;
+	for (i = 0; i < alink->vnic->dp.num_r_vecs; i++) {
+		data = nfp_pr_et(data, "q%u_no_wait", i);
+		data = nfp_pr_et(data, "q%u_delayed", i);
+	}
+	return data;
+}
+
 static int nfp_abm_init(struct nfp_app *app)
 {
 	struct nfp_pf *pf = app->pf;
@@ -575,6 +622,10 @@ const struct nfp_app_type app_abm = {
 	.vnic_alloc	= nfp_abm_vnic_alloc,
 	.vnic_free	= nfp_abm_vnic_free,
 
+	.port_get_stats		= nfp_abm_port_get_stats,
+	.port_get_stats_count	= nfp_abm_port_get_stats_count,
+	.port_get_stats_strings	= nfp_abm_port_get_stats_strings,
+
 	.setup_tc	= nfp_abm_setup_tc,
 
 	.eswitch_mode_get	= nfp_abm_eswitch_mode_get,
diff --git a/drivers/net/ethernet/netronome/nfp/abm/main.h b/drivers/net/ethernet/netronome/nfp/abm/main.h
index 93a3b79cf468..09fd15847961 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/main.h
+++ b/drivers/net/ethernet/netronome/nfp/abm/main.h
@@ -123,6 +123,8 @@ int nfp_abm_ctrl_read_stats(struct nfp_abm_link *alink,
 			    struct nfp_alink_stats *stats);
 int nfp_abm_ctrl_read_xstats(struct nfp_abm_link *alink,
 			     struct nfp_alink_xstats *xstats);
+u64 nfp_abm_ctrl_stat_non_sto(struct nfp_abm_link *alink, unsigned int i);
+u64 nfp_abm_ctrl_stat_sto(struct nfp_abm_link *alink, unsigned int i);
 int nfp_abm_ctrl_qm_enable(struct nfp_abm *abm);
 int nfp_abm_ctrl_qm_disable(struct nfp_abm *abm);
 #endif
-- 
2.17.0

[PATCH net-next 10/14] nfp: abm: expose all PF queues

[Jakub Kicinski]

Allocate the PF representor as multi-queue to allow setting
the configuration per-queue.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/abm/main.c     | 10 +++++++---
 drivers/net/ethernet/netronome/nfp/nfp_net_repr.c |  5 +++--
 drivers/net/ethernet/netronome/nfp/nfp_net_repr.h |  7 ++++++-
 3 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/abm/main.c b/drivers/net/ethernet/netronome/nfp/abm/main.c
index 4e89159f13d3..ef77d7b0d99d 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/main.c
+++ b/drivers/net/ethernet/netronome/nfp/abm/main.c
@@ -255,14 +255,18 @@ nfp_abm_spawn_repr(struct nfp_app *app, struct nfp_abm_link *alink,
 	struct nfp_reprs *reprs;
 	struct nfp_repr *repr;
 	struct nfp_port *port;
+	unsigned int txqs;
 	int err;
 
-	if (ptype == NFP_PORT_PHYS_PORT)
+	if (ptype == NFP_PORT_PHYS_PORT) {
 		rtype = NFP_REPR_TYPE_PHYS_PORT;
-	else
+		txqs = 1;
+	} else {
 		rtype = NFP_REPR_TYPE_PF;
+		txqs = alink->vnic->max_rx_rings;
+	}
 
-	netdev = nfp_repr_alloc(app);
+	netdev = nfp_repr_alloc_mqs(app, txqs, 1);
 	if (!netdev)
 		return -ENOMEM;
 	repr = netdev_priv(netdev);
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_repr.c b/drivers/net/ethernet/netronome/nfp/nfp_net_repr.c
index 117eca6819de..d7b712f6362f 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_repr.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_repr.c
@@ -360,12 +360,13 @@ void nfp_repr_free(struct net_device *netdev)
 	__nfp_repr_free(netdev_priv(netdev));
 }
 
-struct net_device *nfp_repr_alloc(struct nfp_app *app)
+struct net_device *
+nfp_repr_alloc_mqs(struct nfp_app *app, unsigned int txqs, unsigned int rxqs)
 {
 	struct net_device *netdev;
 	struct nfp_repr *repr;
 
-	netdev = alloc_etherdev(sizeof(*repr));
+	netdev = alloc_etherdev_mqs(sizeof(*repr), txqs, rxqs);
 	if (!netdev)
 		return NULL;
 
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_repr.h b/drivers/net/ethernet/netronome/nfp/nfp_net_repr.h
index 8366e4f3c623..1bf2b18109ab 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_repr.h
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_repr.h
@@ -126,7 +126,8 @@ int nfp_repr_init(struct nfp_app *app, struct net_device *netdev,
 		  u32 cmsg_port_id, struct nfp_port *port,
 		  struct net_device *pf_netdev);
 void nfp_repr_free(struct net_device *netdev);
-struct net_device *nfp_repr_alloc(struct nfp_app *app);
+struct net_device *
+nfp_repr_alloc_mqs(struct nfp_app *app, unsigned int txqs, unsigned int rxqs);
 void nfp_repr_clean_and_free(struct nfp_repr *repr);
 void nfp_reprs_clean_and_free(struct nfp_app *app, struct nfp_reprs *reprs);
 void nfp_reprs_clean_and_free_by_type(struct nfp_app *app,
@@ -134,4 +135,8 @@ void nfp_reprs_clean_and_free_by_type(struct nfp_app *app,
 struct nfp_reprs *nfp_reprs_alloc(unsigned int num_reprs);
 int nfp_reprs_resync_phys_ports(struct nfp_app *app);
 
+static inline struct net_device *nfp_repr_alloc(struct nfp_app *app)
+{
+	return nfp_repr_alloc_mqs(app, 1, 1);
+}
 #endif /* NFP_NET_REPR_H */
-- 
2.17.0

[PATCH net-next 11/14] net: sched: mq: add simple offload notification

[Jakub Kicinski]

mq offload is trivial, we just need to let the device know
that the root qdisc is mq.  Alternative approach would be
to export qdisc_lookup() and make drivers check the root
type themselves, but notification via ndo_setup_tc is more
in line with other qdiscs.

Note that mq doesn't hold any stats on it's own, it just
adds up stats of its children.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 include/linux/netdevice.h |  1 +
 include/net/pkt_cls.h     | 10 ++++++++++
 net/sched/sch_mq.c        | 19 +++++++++++++++++++
 3 files changed, 30 insertions(+)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 8452f72087ef..29ef76360cc8 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -791,6 +791,7 @@ enum tc_setup_type {
 	TC_SETUP_QDISC_CBS,
 	TC_SETUP_QDISC_RED,
 	TC_SETUP_QDISC_PRIO,
+	TC_SETUP_QDISC_MQ,
 };
 
 /* These structures hold the attributes of bpf state that are being passed
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index f3ec43725724..942f839dbca4 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -778,6 +778,16 @@ struct tc_qopt_offload_stats {
 	struct gnet_stats_queue *qstats;
 };
 
+enum tc_mq_command {
+	TC_MQ_CREATE,
+	TC_MQ_DESTROY,
+};
+
+struct tc_mq_qopt_offload {
+	enum tc_mq_command command;
+	u32 handle;
+};
+
 enum tc_red_command {
 	TC_RED_REPLACE,
 	TC_RED_DESTROY,
diff --git a/net/sched/sch_mq.c b/net/sched/sch_mq.c
index f062a18e9162..6ccf6daa2503 100644
--- a/net/sched/sch_mq.c
+++ b/net/sched/sch_mq.c
@@ -16,6 +16,7 @@
 #include <linux/errno.h>
 #include <linux/skbuff.h>
 #include <net/netlink.h>
+#include <net/pkt_cls.h>
 #include <net/pkt_sched.h>
 #include <net/sch_generic.h>
 
@@ -23,12 +24,28 @@ struct mq_sched {
 	struct Qdisc		**qdiscs;
 };
 
+static int mq_offload(struct Qdisc *sch, enum tc_mq_command cmd)
+{
+	struct net_device *dev = qdisc_dev(sch);
+	struct tc_mq_qopt_offload opt = {
+		.command = cmd,
+		.handle = sch->handle,
+	};
+
+	if (!tc_can_offload(dev) || !dev->netdev_ops->ndo_setup_tc)
+		return -EOPNOTSUPP;
+
+	return dev->netdev_ops->ndo_setup_tc(dev, TC_SETUP_QDISC_MQ, &opt);
+}
+
 static void mq_destroy(struct Qdisc *sch)
 {
 	struct net_device *dev = qdisc_dev(sch);
 	struct mq_sched *priv = qdisc_priv(sch);
 	unsigned int ntx;
 
+	mq_offload(sch, TC_MQ_DESTROY);
+
 	if (!priv->qdiscs)
 		return;
 	for (ntx = 0; ntx < dev->num_tx_queues && priv->qdiscs[ntx]; ntx++)
@@ -70,6 +87,8 @@ static int mq_init(struct Qdisc *sch, struct nlattr *opt,
 	}
 
 	sch->flags |= TCQ_F_MQROOT;
+
+	mq_offload(sch, TC_MQ_CREATE);
 	return 0;
 }
 
-- 
2.17.0

[PATCH net-next 12/14] nfp: abm: multi-queue RED offload

[Jakub Kicinski]

Add support for MQ offload and setting RED parameters
on queue-by-queue basis.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Dirk van der Merwe <dirk.vandermerwe@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/abm/ctrl.c |  50 ++++-
 drivers/net/ethernet/netronome/nfp/abm/main.c | 192 ++++++++++++++----
 drivers/net/ethernet/netronome/nfp/abm/main.h |  14 +-
 3 files changed, 208 insertions(+), 48 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/abm/ctrl.c b/drivers/net/ethernet/netronome/nfp/abm/ctrl.c
index 79fc9147c012..b157ccd8c80f 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/ctrl.c
+++ b/drivers/net/ethernet/netronome/nfp/abm/ctrl.c
@@ -111,8 +111,7 @@ nfp_abm_ctrl_stat_all(struct nfp_abm_link *alink, const struct nfp_rtsym *sym,
 	return 0;
 }
 
-static int
-nfp_abm_ctrl_set_q_lvl(struct nfp_abm_link *alink, unsigned int i, u32 val)
+int nfp_abm_ctrl_set_q_lvl(struct nfp_abm_link *alink, unsigned int i, u32 val)
 {
 	struct nfp_cpp *cpp = alink->abm->app->cpp;
 	u32 muw;
@@ -164,6 +163,37 @@ u64 nfp_abm_ctrl_stat_sto(struct nfp_abm_link *alink, unsigned int i)
 	return val;
 }
 
+int nfp_abm_ctrl_read_q_stats(struct nfp_abm_link *alink, unsigned int i,
+			      struct nfp_alink_stats *stats)
+{
+	int err;
+
+	stats->tx_pkts = nn_readq(alink->vnic, NFP_NET_CFG_RXR_STATS(i));
+	stats->tx_bytes = nn_readq(alink->vnic, NFP_NET_CFG_RXR_STATS(i) + 8);
+
+	err = nfp_abm_ctrl_stat(alink, alink->abm->q_lvls,
+				NFP_QLVL_STRIDE, NFP_QLVL_BLOG_BYTES,
+				i, false, &stats->backlog_bytes);
+	if (err)
+		return err;
+
+	err = nfp_abm_ctrl_stat(alink, alink->abm->q_lvls,
+				NFP_QLVL_STRIDE, NFP_QLVL_BLOG_PKTS,
+				i, false, &stats->backlog_pkts);
+	if (err)
+		return err;
+
+	err = nfp_abm_ctrl_stat(alink, alink->abm->qm_stats,
+				NFP_QMSTAT_STRIDE, NFP_QMSTAT_DROP,
+				i, true, &stats->drops);
+	if (err)
+		return err;
+
+	return nfp_abm_ctrl_stat(alink, alink->abm->qm_stats,
+				 NFP_QMSTAT_STRIDE, NFP_QMSTAT_ECN,
+				 i, true, &stats->overlimits);
+}
+
 int nfp_abm_ctrl_read_stats(struct nfp_abm_link *alink,
 			    struct nfp_alink_stats *stats)
 {
@@ -200,6 +230,22 @@ int nfp_abm_ctrl_read_stats(struct nfp_abm_link *alink,
 				     true, &stats->overlimits);
 }
 
+int nfp_abm_ctrl_read_q_xstats(struct nfp_abm_link *alink, unsigned int i,
+			       struct nfp_alink_xstats *xstats)
+{
+	int err;
+
+	err = nfp_abm_ctrl_stat(alink, alink->abm->qm_stats,
+				NFP_QMSTAT_STRIDE, NFP_QMSTAT_DROP,
+				i, true, &xstats->pdrop);
+	if (err)
+		return err;
+
+	return nfp_abm_ctrl_stat(alink, alink->abm->qm_stats,
+				 NFP_QMSTAT_STRIDE, NFP_QMSTAT_ECN,
+				 i, true, &xstats->ecn_marked);
+}
+
 int nfp_abm_ctrl_read_xstats(struct nfp_abm_link *alink,
 			     struct nfp_alink_xstats *xstats)
 {
diff --git a/drivers/net/ethernet/netronome/nfp/abm/main.c b/drivers/net/ethernet/netronome/nfp/abm/main.c
index ef77d7b0d99d..21d5af1fb061 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/main.c
+++ b/drivers/net/ethernet/netronome/nfp/abm/main.c
@@ -58,43 +58,77 @@ static u32 nfp_abm_portid(enum nfp_repr_type rtype, unsigned int id)
 	       FIELD_PREP(NFP_ABM_PORTID_ID, id);
 }
 
-static int nfp_abm_reset_stats(struct nfp_abm_link *alink)
+static int
+__nfp_abm_reset_root(struct net_device *netdev, struct nfp_abm_link *alink,
+		     u32 handle, unsigned int qs, u32 init_val)
 {
-	int err;
+	struct nfp_port *port = nfp_port_from_netdev(netdev);
+	int ret;
 
-	err = nfp_abm_ctrl_read_stats(alink, &alink->qdiscs[0].stats);
-	if (err)
-		return err;
-	alink->qdiscs[0].stats.backlog_pkts = 0;
-	alink->qdiscs[0].stats.backlog_bytes = 0;
+	ret = nfp_abm_ctrl_set_all_q_lvls(alink, init_val);
+	memset(alink->qdiscs, 0, sizeof(*alink->qdiscs) * alink->num_qdiscs);
 
-	err = nfp_abm_ctrl_read_xstats(alink, &alink->qdiscs[0].xstats);
-	if (err)
-		return err;
+	alink->parent = handle;
+	alink->num_qdiscs = qs;
+	port->tc_offload_cnt = qs;
 
-	return 0;
+	return ret;
+}
+
+static void
+nfp_abm_reset_root(struct net_device *netdev, struct nfp_abm_link *alink,
+		   u32 handle, unsigned int qs)
+{
+	__nfp_abm_reset_root(netdev, alink, handle, qs, ~0);
+}
+
+static int
+nfp_abm_red_find(struct nfp_abm_link *alink, struct tc_red_qopt_offload *opt)
+{
+	unsigned int i = TC_H_MIN(opt->parent) - 1;
+
+	if (opt->parent == TC_H_ROOT)
+		i = 0;
+	else if (TC_H_MAJ(alink->parent) == TC_H_MAJ(opt->parent))
+		i = TC_H_MIN(opt->parent) - 1;
+	else
+		return -EOPNOTSUPP;
+
+	if (i >= alink->num_qdiscs || opt->handle != alink->qdiscs[i].handle)
+		return -EOPNOTSUPP;
+
+	return i;
 }
 
 static void
 nfp_abm_red_destroy(struct net_device *netdev, struct nfp_abm_link *alink,
 		    u32 handle)
 {
-	struct nfp_port *port = nfp_port_from_netdev(netdev);
+	unsigned int i;
 
-	if (handle != alink->qdiscs[0].handle)
+	for (i = 0; i < alink->num_qdiscs; i++)
+		if (handle == alink->qdiscs[i].handle)
+			break;
+	if (i == alink->num_qdiscs)
 		return;
 
-	alink->qdiscs[0].handle = TC_H_UNSPEC;
-	port->tc_offload_cnt = 0;
-	nfp_abm_ctrl_set_all_q_lvls(alink, ~0);
+	if (alink->parent == TC_H_ROOT) {
+		nfp_abm_reset_root(netdev, alink, TC_H_ROOT, 0);
+	} else {
+		nfp_abm_ctrl_set_q_lvl(alink, i, ~0);
+		memset(&alink->qdiscs[i], 0, sizeof(*alink->qdiscs));
+	}
 }
 
 static int
 nfp_abm_red_replace(struct net_device *netdev, struct nfp_abm_link *alink,
 		    struct tc_red_qopt_offload *opt)
 {
-	struct nfp_port *port = nfp_port_from_netdev(netdev);
-	int err;
+	bool existing;
+	int i, err;
+
+	i = nfp_abm_red_find(alink, opt);
+	existing = i >= 0;
 
 	if (opt->set.min != opt->set.max || !opt->set.is_ecn) {
 		nfp_warn(alink->abm->app->cpp,
@@ -102,30 +136,62 @@ nfp_abm_red_replace(struct net_device *netdev, struct nfp_abm_link *alink,
 		err = -EINVAL;
 		goto err_destroy;
 	}
-	err = nfp_abm_ctrl_set_all_q_lvls(alink, opt->set.min);
-	if (err)
-		goto err_destroy;
 
-	/* Reset stats only on new qdisc */
-	if (alink->qdiscs[0].handle != opt->handle) {
-		err = nfp_abm_reset_stats(alink);
+	if (existing) {
+		if (alink->parent == TC_H_ROOT)
+			err = nfp_abm_ctrl_set_all_q_lvls(alink, opt->set.min);
+		else
+			err = nfp_abm_ctrl_set_q_lvl(alink, i, opt->set.min);
 		if (err)
 			goto err_destroy;
+		return 0;
 	}
 
-	alink->qdiscs[0].handle = opt->handle;
-	port->tc_offload_cnt = 1;
+	if (opt->parent == TC_H_ROOT) {
+		i = 0;
+		err = __nfp_abm_reset_root(netdev, alink, TC_H_ROOT, 1,
+					   opt->set.min);
+	} else if (TC_H_MAJ(alink->parent) == TC_H_MAJ(opt->parent)) {
+		i = TC_H_MIN(opt->parent) - 1;
+		err = nfp_abm_ctrl_set_q_lvl(alink, i, opt->set.min);
+	} else {
+		return -EINVAL;
+	}
+	/* Set the handle to try full clean up, in case IO failed */
+	alink->qdiscs[i].handle = opt->handle;
+	if (err)
+		goto err_destroy;
+
+	if (opt->parent == TC_H_ROOT)
+		err = nfp_abm_ctrl_read_stats(alink, &alink->qdiscs[i].stats);
+	else
+		err = nfp_abm_ctrl_read_q_stats(alink, i,
+						&alink->qdiscs[i].stats);
+	if (err)
+		goto err_destroy;
+
+	if (opt->parent == TC_H_ROOT)
+		err = nfp_abm_ctrl_read_xstats(alink,
+					       &alink->qdiscs[i].xstats);
+	else
+		err = nfp_abm_ctrl_read_q_xstats(alink, i,
+						 &alink->qdiscs[i].xstats);
+	if (err)
+		goto err_destroy;
+
+	alink->qdiscs[i].stats.backlog_pkts = 0;
+	alink->qdiscs[i].stats.backlog_bytes = 0;
 
 	return 0;
 err_destroy:
 	/* If the qdisc keeps on living, but we can't offload undo changes */
-	if (alink->qdiscs[0].handle == opt->handle) {
-		opt->set.qstats->qlen -= alink->qdiscs[0].stats.backlog_pkts;
+	if (existing) {
+		opt->set.qstats->qlen -= alink->qdiscs[i].stats.backlog_pkts;
 		opt->set.qstats->backlog -=
-			alink->qdiscs[0].stats.backlog_bytes;
+			alink->qdiscs[i].stats.backlog_bytes;
 	}
-	if (alink->qdiscs[0].handle != TC_H_UNSPEC)
-		nfp_abm_red_destroy(netdev, alink, alink->qdiscs[0].handle);
+	nfp_abm_red_destroy(netdev, alink, opt->handle);
+
 	return err;
 }
 
@@ -146,13 +212,17 @@ nfp_abm_red_stats(struct nfp_abm_link *alink, struct tc_red_qopt_offload *opt)
 {
 	struct nfp_alink_stats *prev_stats;
 	struct nfp_alink_stats stats;
-	int err;
+	int i, err;
 
-	if (alink->qdiscs[0].handle != opt->handle)
-		return -EOPNOTSUPP;
-	prev_stats = &alink->qdiscs[0].stats;
+	i = nfp_abm_red_find(alink, opt);
+	if (i < 0)
+		return i;
+	prev_stats = &alink->qdiscs[i].stats;
 
-	err = nfp_abm_ctrl_read_stats(alink, &stats);
+	if (alink->parent == TC_H_ROOT)
+		err = nfp_abm_ctrl_read_stats(alink, &stats);
+	else
+		err = nfp_abm_ctrl_read_q_stats(alink, i, &stats);
 	if (err)
 		return err;
 
@@ -168,13 +238,17 @@ nfp_abm_red_xstats(struct nfp_abm_link *alink, struct tc_red_qopt_offload *opt)
 {
 	struct nfp_alink_xstats *prev_xstats;
 	struct nfp_alink_xstats xstats;
-	int err;
+	int i, err;
 
-	if (alink->qdiscs[0].handle != opt->handle)
-		return -EOPNOTSUPP;
-	prev_xstats = &alink->qdiscs[0].xstats;
+	i = nfp_abm_red_find(alink, opt);
+	if (i < 0)
+		return i;
+	prev_xstats = &alink->qdiscs[i].xstats;
 
-	err = nfp_abm_ctrl_read_xstats(alink, &xstats);
+	if (alink->parent == TC_H_ROOT)
+		err = nfp_abm_ctrl_read_xstats(alink, &xstats);
+	else
+		err = nfp_abm_ctrl_read_q_xstats(alink, i, &xstats);
 	if (err)
 		return err;
 
@@ -190,9 +264,6 @@ static int
 nfp_abm_setup_tc_red(struct net_device *netdev, struct nfp_abm_link *alink,
 		     struct tc_red_qopt_offload *opt)
 {
-	if (opt->parent != TC_H_ROOT)
-		return -EOPNOTSUPP;
-
 	switch (opt->command) {
 	case TC_RED_REPLACE:
 		return nfp_abm_red_replace(netdev, alink, opt);
@@ -208,6 +279,24 @@ nfp_abm_setup_tc_red(struct net_device *netdev, struct nfp_abm_link *alink,
 	}
 }
 
+static int
+nfp_abm_setup_tc_mq(struct net_device *netdev, struct nfp_abm_link *alink,
+		    struct tc_mq_qopt_offload *opt)
+{
+	switch (opt->command) {
+	case TC_MQ_CREATE:
+		nfp_abm_reset_root(netdev, alink, opt->handle,
+				   alink->total_queues);
+		return 0;
+	case TC_MQ_DESTROY:
+		if (opt->handle == alink->parent)
+			nfp_abm_reset_root(netdev, alink, TC_H_ROOT, 0);
+		return 0;
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
 static int
 nfp_abm_setup_tc(struct nfp_app *app, struct net_device *netdev,
 		 enum tc_setup_type type, void *type_data)
@@ -220,6 +309,8 @@ nfp_abm_setup_tc(struct nfp_app *app, struct net_device *netdev,
 		return -EOPNOTSUPP;
 
 	switch (type) {
+	case TC_SETUP_QDISC_MQ:
+		return nfp_abm_setup_tc_mq(netdev, repr->app_priv, type_data);
 	case TC_SETUP_QDISC_RED:
 		return nfp_abm_setup_tc_red(netdev, repr->app_priv, type_data);
 	default:
@@ -473,13 +564,21 @@ nfp_abm_vnic_alloc(struct nfp_app *app, struct nfp_net *nn, unsigned int id)
 	alink->abm = abm;
 	alink->vnic = nn;
 	alink->id = id;
+	alink->parent = TC_H_ROOT;
+	alink->total_queues = alink->vnic->max_rx_rings;
+	alink->qdiscs = kvzalloc(sizeof(*alink->qdiscs) * alink->total_queues,
+				 GFP_KERNEL);
+	if (!alink->qdiscs) {
+		err = -ENOMEM;
+		goto err_free_alink;
+	}
 
 	/* This is a multi-host app, make sure MAC/PHY is up, but don't
 	 * make the MAC/PHY state follow the state of any of the ports.
 	 */
 	err = nfp_eth_set_configured(app->cpp, eth_port->index, true);
 	if (err < 0)
-		goto err_free_alink;
+		goto err_free_qdiscs;
 
 	netif_keep_dst(nn->dp.netdev);
 
@@ -488,6 +587,8 @@ nfp_abm_vnic_alloc(struct nfp_app *app, struct nfp_net *nn, unsigned int id)
 
 	return 0;
 
+err_free_qdiscs:
+	kvfree(alink->qdiscs);
 err_free_alink:
 	kfree(alink);
 	return err;
@@ -498,6 +599,7 @@ static void nfp_abm_vnic_free(struct nfp_app *app, struct nfp_net *nn)
 	struct nfp_abm_link *alink = nn->app_priv;
 
 	nfp_abm_kill_reprs(alink->abm, alink);
+	kvfree(alink->qdiscs);
 	kfree(alink);
 }
 
diff --git a/drivers/net/ethernet/netronome/nfp/abm/main.h b/drivers/net/ethernet/netronome/nfp/abm/main.h
index 09fd15847961..934a70835473 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/main.h
+++ b/drivers/net/ethernet/netronome/nfp/abm/main.h
@@ -106,6 +106,9 @@ struct nfp_red_qdisc {
  * @vnic:	data vNIC
  * @id:		id of the data vNIC
  * @queue_base:	id of base to host queue within PCIe (not QC idx)
+ * @total_queues:	number of PF queues
+ * @parent:	handle of expected parent, i.e. handle of MQ, or TC_H_ROOT
+ * @num_qdiscs:	number of currently used qdiscs
  * @qdiscs:	array of qdiscs
  */
 struct nfp_abm_link {
@@ -113,16 +116,25 @@ struct nfp_abm_link {
 	struct nfp_net *vnic;
 	unsigned int id;
 	unsigned int queue_base;
-	struct nfp_red_qdisc qdiscs[1];
+	unsigned int total_queues;
+	u32 parent;
+	unsigned int num_qdiscs;
+	struct nfp_red_qdisc *qdiscs;
 };
 
 void nfp_abm_ctrl_read_params(struct nfp_abm_link *alink);
 int nfp_abm_ctrl_find_addrs(struct nfp_abm *abm);
 int nfp_abm_ctrl_set_all_q_lvls(struct nfp_abm_link *alink, u32 val);
+int nfp_abm_ctrl_set_q_lvl(struct nfp_abm_link *alink, unsigned int i,
+			   u32 val);
 int nfp_abm_ctrl_read_stats(struct nfp_abm_link *alink,
 			    struct nfp_alink_stats *stats);
+int nfp_abm_ctrl_read_q_stats(struct nfp_abm_link *alink, unsigned int i,
+			      struct nfp_alink_stats *stats);
 int nfp_abm_ctrl_read_xstats(struct nfp_abm_link *alink,
 			     struct nfp_alink_xstats *xstats);
+int nfp_abm_ctrl_read_q_xstats(struct nfp_abm_link *alink, unsigned int i,
+			       struct nfp_alink_xstats *xstats);
 u64 nfp_abm_ctrl_stat_non_sto(struct nfp_abm_link *alink, unsigned int i);
 u64 nfp_abm_ctrl_stat_sto(struct nfp_abm_link *alink, unsigned int i);
 int nfp_abm_ctrl_qm_enable(struct nfp_abm *abm);
-- 
2.17.0

[PATCH net-next 13/14] net: sched: mq: request stats from offloads

[Jakub Kicinski]

MQ doesn't hold any statistics on its own, however, statistic
from offloads are requested starting from the root, hence MQ
will read the old values for its sums.  Call into the drivers,
because of the additive nature of the stats drivers are aware
of how much "pending updates" they have to children of the MQ.
Since MQ reset its stats on every dump we can simply offset
the stats, predicting how stats of offloaded children will
change.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 include/net/pkt_cls.h |  2 ++
 net/sched/sch_mq.c    | 18 ++++++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index 942f839dbca4..a3c1a2c47cd4 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -781,11 +781,13 @@ struct tc_qopt_offload_stats {
 enum tc_mq_command {
 	TC_MQ_CREATE,
 	TC_MQ_DESTROY,
+	TC_MQ_STATS,
 };
 
 struct tc_mq_qopt_offload {
 	enum tc_mq_command command;
 	u32 handle;
+	struct tc_qopt_offload_stats stats;
 };
 
 enum tc_red_command {
diff --git a/net/sched/sch_mq.c b/net/sched/sch_mq.c
index 6ccf6daa2503..d6b8ae4ed7a3 100644
--- a/net/sched/sch_mq.c
+++ b/net/sched/sch_mq.c
@@ -38,6 +38,22 @@ static int mq_offload(struct Qdisc *sch, enum tc_mq_command cmd)
 	return dev->netdev_ops->ndo_setup_tc(dev, TC_SETUP_QDISC_MQ, &opt);
 }
 
+static void mq_offload_stats(struct Qdisc *sch)
+{
+	struct net_device *dev = qdisc_dev(sch);
+	struct tc_mq_qopt_offload opt = {
+		.command = TC_MQ_STATS,
+		.handle = sch->handle,
+		.stats = {
+			.bstats = &sch->bstats,
+			.qstats = &sch->qstats,
+		},
+	};
+
+	if (tc_can_offload(dev) && dev->netdev_ops->ndo_setup_tc)
+		dev->netdev_ops->ndo_setup_tc(dev, TC_SETUP_QDISC_MQ, &opt);
+}
+
 static void mq_destroy(struct Qdisc *sch)
 {
 	struct net_device *dev = qdisc_dev(sch);
@@ -146,6 +162,7 @@ static int mq_dump(struct Qdisc *sch, struct sk_buff *skb)
 			sch->q.qlen		+= qdisc->q.qlen;
 			sch->bstats.bytes	+= qdisc->bstats.bytes;
 			sch->bstats.packets	+= qdisc->bstats.packets;
+			sch->qstats.qlen	+= qdisc->qstats.qlen;
 			sch->qstats.backlog	+= qdisc->qstats.backlog;
 			sch->qstats.drops	+= qdisc->qstats.drops;
 			sch->qstats.requeues	+= qdisc->qstats.requeues;
@@ -154,6 +171,7 @@ static int mq_dump(struct Qdisc *sch, struct sk_buff *skb)
 
 		spin_unlock_bh(qdisc_lock(qdisc));
 	}
+	mq_offload_stats(sch);
 
 	return 0;
 }
-- 
2.17.0

[PATCH net-next 14/14] nfp: abm: report correct MQ stats

[Jakub Kicinski]

Report the stat diff to make sure MQ stats add up to child stats.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Dirk van der Merwe <dirk.vandermerwe@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/abm/main.c | 24 +++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/net/ethernet/netronome/nfp/abm/main.c b/drivers/net/ethernet/netronome/nfp/abm/main.c
index 21d5af1fb061..1561c2724c26 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/main.c
+++ b/drivers/net/ethernet/netronome/nfp/abm/main.c
@@ -279,6 +279,28 @@ nfp_abm_setup_tc_red(struct net_device *netdev, struct nfp_abm_link *alink,
 	}
 }
 
+static int
+nfp_abm_mq_stats(struct nfp_abm_link *alink, struct tc_mq_qopt_offload *opt)
+{
+	struct nfp_alink_stats stats;
+	unsigned int i;
+	int err;
+
+	for (i = 0; i < alink->num_qdiscs; i++) {
+		if (alink->qdiscs[i].handle == TC_H_UNSPEC)
+			continue;
+
+		err = nfp_abm_ctrl_read_q_stats(alink, i, &stats);
+		if (err)
+			return err;
+
+		nfp_abm_update_stats(&stats, &alink->qdiscs[i].stats,
+				     &opt->stats);
+	}
+
+	return 0;
+}
+
 static int
 nfp_abm_setup_tc_mq(struct net_device *netdev, struct nfp_abm_link *alink,
 		    struct tc_mq_qopt_offload *opt)
@@ -292,6 +314,8 @@ nfp_abm_setup_tc_mq(struct net_device *netdev, struct nfp_abm_link *alink,
 		if (opt->handle == alink->parent)
 			nfp_abm_reset_root(netdev, alink, TC_H_ROOT, 0);
 		return 0;
+	case TC_MQ_STATS:
+		return nfp_abm_mq_stats(alink, opt);
 	default:
 		return -EOPNOTSUPP;
 	}
-- 
2.17.0

Re: [PATCH net-next 05/14] nfp: abm: add simple RED offload

[Nogah Frankel]

On 26-May-18 7:53 AM, Jakub Kicinski wrote:
> Offload simple RED configurations.  For now support only DCTCP
> like scenarios where min and max are the same.
> 
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> ---
>   drivers/net/ethernet/netronome/nfp/abm/main.c | 82 +++++++++++++++++++
>   drivers/net/ethernet/netronome/nfp/abm/main.h | 10 +++
>   2 files changed, 92 insertions(+)
> 
> diff --git a/drivers/net/ethernet/netronome/nfp/abm/main.c b/drivers/net/ethernet/netronome/nfp/abm/main.c
> index 28a18ac62040..22251d88c958 100644
> --- a/drivers/net/ethernet/netronome/nfp/abm/main.c
> +++ b/drivers/net/ethernet/netronome/nfp/abm/main.c
> @@ -38,6 +38,8 @@
>   #include <linux/netdevice.h>
>   #include <linux/rcupdate.h>
>   #include <linux/slab.h>
> +#include <net/pkt_cls.h>
> +#include <net/pkt_sched.h>
>   
>   #include "../nfpcore/nfp.h"
>   #include "../nfpcore/nfp_cpp.h"
> @@ -55,6 +57,84 @@ static u32 nfp_abm_portid(enum nfp_repr_type rtype, unsigned int id)
>   	       FIELD_PREP(NFP_ABM_PORTID_ID, id);
>   }
>   
> +static void
> +nfp_abm_red_destroy(struct net_device *netdev, struct nfp_abm_link *alink,
> +		    u32 handle)
> +{
> +	struct nfp_port *port = nfp_port_from_netdev(netdev);
> +
> +	if (handle != alink->qdiscs[0].handle)
> +		return;
> +
> +	alink->qdiscs[0].handle = TC_H_UNSPEC;
> +	port->tc_offload_cnt = 0;
> +	nfp_abm_ctrl_set_all_q_lvls(alink, ~0);
> +}
> +
> +static int
> +nfp_abm_red_replace(struct net_device *netdev, struct nfp_abm_link *alink,
> +		    struct tc_red_qopt_offload *opt)
> +{
> +	struct nfp_port *port = nfp_port_from_netdev(netdev);
> +	int err;
> +
> +	if (opt->set.min != opt->set.max || !opt->set.is_ecn) {

I am a bit worried about the min == max.
sch_red doesn't really support it. It will calculate incorrect delta 
value. (And that only if tc_red_eval_P in iproute2 won't reject it).
You might maybe use max = min+1,  because in real life it will probably 
act the same but without this problem.

Nogah Frankel
(from a new mail address)

> +		nfp_warn(alink->abm->app->cpp,
> +			 "RED offload failed - unsupported parameters\n");
> +		err = -EINVAL;
> +		goto err_destroy;
> +	}
> +	err = nfp_abm_ctrl_set_all_q_lvls(alink, opt->set.min);
> +	if (err)
> +		goto err_destroy;
> +
> +	alink->qdiscs[0].handle = opt->handle;
> +	port->tc_offload_cnt = 1;
> +
> +	return 0;
> +err_destroy:
> +	if (alink->qdiscs[0].handle != TC_H_UNSPEC)
> +		nfp_abm_red_destroy(netdev, alink, alink->qdiscs[0].handle);
> +	return err;
> +}
> +
> +static int
> +nfp_abm_setup_tc_red(struct net_device *netdev, struct nfp_abm_link *alink,
> +		     struct tc_red_qopt_offload *opt)
> +{
> +	if (opt->parent != TC_H_ROOT)
> +		return -EOPNOTSUPP;
> +
> +	switch (opt->command) {
> +	case TC_RED_REPLACE:
> +		return nfp_abm_red_replace(netdev, alink, opt);
> +	case TC_RED_DESTROY:
> +		nfp_abm_red_destroy(netdev, alink, opt->handle);
> +		return 0;
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +}
> +
> +static int
> +nfp_abm_setup_tc(struct nfp_app *app, struct net_device *netdev,
> +		 enum tc_setup_type type, void *type_data)
> +{
> +	struct nfp_repr *repr = netdev_priv(netdev);
> +	struct nfp_port *port;
> +
> +	port = nfp_port_from_netdev(netdev);
> +	if (!port || port->type != NFP_PORT_PF_PORT)
> +		return -EOPNOTSUPP;
> +
> +	switch (type) {
> +	case TC_SETUP_QDISC_RED:
> +		return nfp_abm_setup_tc_red(netdev, repr->app_priv, type_data);
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +}
> +
>   static struct net_device *nfp_abm_repr_get(struct nfp_app *app, u32 port_id)
>   {
>   	enum nfp_repr_type rtype;
> @@ -403,6 +483,8 @@ const struct nfp_app_type app_abm = {
>   	.vnic_alloc	= nfp_abm_vnic_alloc,
>   	.vnic_free	= nfp_abm_vnic_free,
>   
> +	.setup_tc	= nfp_abm_setup_tc,
> +
>   	.eswitch_mode_get	= nfp_abm_eswitch_mode_get,
>   	.eswitch_mode_set	= nfp_abm_eswitch_mode_set,
>   
> diff --git a/drivers/net/ethernet/netronome/nfp/abm/main.h b/drivers/net/ethernet/netronome/nfp/abm/main.h
> index 1ac651cdc140..979f98fb808b 100644
> --- a/drivers/net/ethernet/netronome/nfp/abm/main.h
> +++ b/drivers/net/ethernet/netronome/nfp/abm/main.h
> @@ -58,18 +58,28 @@ struct nfp_abm {
>   	const struct nfp_rtsym *q_lvls;
>   };
>   
> +/**
> + * struct nfp_red_qdisc - representation of single RED Qdisc
> + * @handle:	handle of currently offloaded RED Qdisc
> + */
> +struct nfp_red_qdisc {
> +	u32 handle;
> +};
> +
>   /**
>    * struct nfp_abm_link - port tuple of a ABM NIC
>    * @abm:	back pointer to nfp_abm
>    * @vnic:	data vNIC
>    * @id:		id of the data vNIC
>    * @queue_base:	id of base to host queue within PCIe (not QC idx)
> + * @qdiscs:	array of qdiscs
>    */
>   struct nfp_abm_link {
>   	struct nfp_abm *abm;
>   	struct nfp_net *vnic;
>   	unsigned int id;
>   	unsigned int queue_base;
> +	struct nfp_red_qdisc qdiscs[1];
>   };
>   
>   void nfp_abm_ctrl_read_params(struct nfp_abm_link *alink);
> 

Re: [PATCH net-next 05/14] nfp: abm: add simple RED offload

[Jakub Kicinski]

Hi Nogah!

On Mon, 28 May 2018 18:49:51 +0300, Nogah Frankel wrote:
> > +static int
> > +nfp_abm_red_replace(struct net_device *netdev, struct nfp_abm_link *alink,
> > +		    struct tc_red_qopt_offload *opt)
> > +{
> > +	struct nfp_port *port = nfp_port_from_netdev(netdev);
> > +	int err;
> > +
> > +	if (opt->set.min != opt->set.max || !opt->set.is_ecn) {  
> 
> I am a bit worried about the min == max.
> sch_red doesn't really support it. It will calculate incorrect delta 
> value. (And that only if tc_red_eval_P in iproute2 won't reject it).
> You might maybe use max = min+1,  because in real life it will probably 
> act the same but without this problem.

I remember having a long think about this when I wrote the code.  
My conclusion was that the two would operate almost the same, and
setting min == max may be most obvious to the user.

If min + 1 == max sch_red would act probabilistically for qavg == min,
which is not what the card would do.

Userspace now does this:

tc_red_eval_P() {
	int i = qmax - qmin;
 
	if (!i)
		return 0;
	if (i < 0)
		return -1;
	...
}

And you've fixed delta to be treated as 1 to avoid division by 0 in
commit 5c472203421a ("net_sched: red: Avoid devision by zero"):

red_set_parms() {
	int delta = qth_max - qth_min;
	u32 max_p_delta;

	p->qth_min	= qth_min << Wlog;
	p->qth_max	= qth_max << Wlog;
	p->Wlog		= Wlog;
	p->Plog		= Plog;
	if (delta <= 0)
		delta = 1;
	p->qth_delta	= delta;
	...
}

So we should be safe.  Targets will match.  Probability adjustment for
adaptive should work correctly.  Which doesn't matter anyway, since we
will never use the probabilistic action...

> Nogah Frankel
> (from a new mail address)

Noted :)

Re: [PATCH net-next 05/14] nfp: abm: add simple RED offload

[Nogah Frankel]

On 29-May-18 2:05 AM, Jakub Kicinski wrote:

Hi Jakub,

> Hi Nogah!
> 
> On Mon, 28 May 2018 18:49:51 +0300, Nogah Frankel wrote:
>>> +static int
>>> +nfp_abm_red_replace(struct net_device *netdev, struct nfp_abm_link *alink,
>>> +		    struct tc_red_qopt_offload *opt)
>>> +{
>>> +	struct nfp_port *port = nfp_port_from_netdev(netdev);
>>> +	int err;
>>> +
>>> +	if (opt->set.min != opt->set.max || !opt->set.is_ecn) {
>>
>> I am a bit worried about the min == max.
>> sch_red doesn't really support it. It will calculate incorrect delta
>> value. (And that only if tc_red_eval_P in iproute2 won't reject it).
>> You might maybe use max = min+1,  because in real life it will probably
>> act the same but without this problem.
> 
> I remember having a long think about this when I wrote the code.
> My conclusion was that the two would operate almost the same, and
> setting min == max may be most obvious to the user.

I agree.

> 
> If min + 1 == max sch_red would act probabilistically for qavg == min,
> which is not what the card would do.
> 
> Userspace now does this:
> 
> tc_red_eval_P() {
> 	int i = qmax - qmin;
>   
> 	if (!i)
> 		return 0;
> 	if (i < 0)
> 		return -1;
> 	...
> }
> 
> And you've fixed delta to be treated as 1 to avoid division by 0 in
> commit 5c472203421a ("net_sched: red: Avoid devision by zero"):
> 
> red_set_parms() {
> 	int delta = qth_max - qth_min;
> 	u32 max_p_delta;
> 
> 	p->qth_min	= qth_min << Wlog;
> 	p->qth_max	= qth_max << Wlog;
> 	p->Wlog		= Wlog;
> 	p->Plog		= Plog;
> 	if (delta <= 0)
> 		delta = 1;
> 	p->qth_delta	= delta;
> 	...
> }

I changes it to avoid division by 0, but I wasn't sure that the delta 
value of 1 will be good, just that it is better then 0.

> 
> So we should be safe.  Targets will match.  Probability adjustment for
> adaptive should work correctly.  Which doesn't matter anyway, since we
> will never use the probabilistic action...

That makes sense, and it is a better way to set this setup (DCTCP, I 
guess?) than before.

Thanks

Nogah


> 
>> Nogah Frankel
>> (from a new mail address)
> 
> Noted :)
> 

Re: [PATCH net-next 00/14] nfp: abm: RED/MQ qdisc offload

[David Miller]

From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Fri, 25 May 2018 21:53:24 -0700

> This is second batch of advanced buffer management nfp driver
> changes.  This series adds the qdisc offload.  Support for
> a very simple subset of RED qdisc offload is added as needed
> for DCTCP ECN marking (min and max thresholds set to the same
> value).
> 
> The first two patches fix glitches introduced by the previous
> series.  We have to be careful about phys_port_name handling,
> because VFs share the same code path, and some user space may
> get confused by the names we chose.
> 
> Since unlike previous offloads we can report the queue backlog
> both in bytes and packets we need to adjust how statistics are
> added up in the core (patch 6).
> 
> There are some extra statistics we want to expose which don't
> fit into TC stats, namely counts of packets which have been fast-
> -forwarded without getting enqueued because there was no
> contention and number of packets that were ever queued (sum of
> all momentary backlogs).  We expose those through ethtool stats
> (patches 8 and 9).
> 
> Remaining 5 patches add MQ offload - to be able to set different
> configurations on different queues.  Representors are made multi-
> -queue and we add offload support to MQ.  MQ stats are added up
> before calling ->dump qdiscs on the children, and therefore don't
> include updated offload values.  To avoid clearly incorrect stats
> MQ is made to also request stats update from offloads.  This way
> we can correct the diff at the driver level.

Series applied, thanks Jakub.