Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next 00/14]  nfp: abm: RED/MQ qdisc offload
From: Jakub Kicinski @ 2018-05-26  4:53 UTC (permalink / raw)
  To: davem
  Cc: jiri, xiyou.wangcong, john.fastabend, netdev, oss-drivers,
	alexei.starovoitov, nogahf, yuvalm, gerlitz.or, 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

^ permalink raw reply

* [PATCH net-next 01/14] nfp: return -EOPNOTSUPP from .ndo_get_phys_port_name for VFs
From: Jakub Kicinski @ 2018-05-26  4:53 UTC (permalink / raw)
  To: davem
  Cc: jiri, xiyou.wangcong, john.fastabend, netdev, oss-drivers,
	alexei.starovoitov, nogahf, yuvalm, gerlitz.or, Jakub Kicinski
In-Reply-To: <20180526045338.10993-1-jakub.kicinski@netronome.com>

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

^ permalink raw reply related

* [PATCH net-next 02/14] nfp: prefix vNIC phys_port_name with 'n'
From: Jakub Kicinski @ 2018-05-26  4:53 UTC (permalink / raw)
  To: davem
  Cc: jiri, xiyou.wangcong, john.fastabend, netdev, oss-drivers,
	alexei.starovoitov, nogahf, yuvalm, gerlitz.or, Jakub Kicinski
In-Reply-To: <20180526045338.10993-1-jakub.kicinski@netronome.com>

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

^ permalink raw reply related

* [PATCH net-next 03/14] nfp: abm: enable advanced queuing on demand
From: Jakub Kicinski @ 2018-05-26  4:53 UTC (permalink / raw)
  To: davem
  Cc: jiri, xiyou.wangcong, john.fastabend, netdev, oss-drivers,
	alexei.starovoitov, nogahf, yuvalm, gerlitz.or, Jakub Kicinski
In-Reply-To: <20180526045338.10993-1-jakub.kicinski@netronome.com>

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

^ permalink raw reply related

* [PATCH net-next 04/14] nfp: abm: add helpers for configuring queue marking levels
From: Jakub Kicinski @ 2018-05-26  4:53 UTC (permalink / raw)
  To: davem
  Cc: jiri, xiyou.wangcong, john.fastabend, netdev, oss-drivers,
	alexei.starovoitov, nogahf, yuvalm, gerlitz.or, Jakub Kicinski
In-Reply-To: <20180526045338.10993-1-jakub.kicinski@netronome.com>

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

^ permalink raw reply related

* [PATCH net-next 05/14] nfp: abm: add simple RED offload
From: Jakub Kicinski @ 2018-05-26  4:53 UTC (permalink / raw)
  To: davem
  Cc: jiri, xiyou.wangcong, john.fastabend, netdev, oss-drivers,
	alexei.starovoitov, nogahf, yuvalm, gerlitz.or, Jakub Kicinski
In-Reply-To: <20180526045338.10993-1-jakub.kicinski@netronome.com>

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

^ permalink raw reply related

* [PATCH net-next 06/14] net: sched: add qstats.qlen to qlen
From: Jakub Kicinski @ 2018-05-26  4:53 UTC (permalink / raw)
  To: davem
  Cc: jiri, xiyou.wangcong, john.fastabend, netdev, oss-drivers,
	alexei.starovoitov, nogahf, yuvalm, gerlitz.or, Jakub Kicinski
In-Reply-To: <20180526045338.10993-1-jakub.kicinski@netronome.com>

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

^ permalink raw reply related

* [PATCH net-next 07/14] nfp: abm: report statistics from RED offload
From: Jakub Kicinski @ 2018-05-26  4:53 UTC (permalink / raw)
  To: davem
  Cc: jiri, xiyou.wangcong, john.fastabend, netdev, oss-drivers,
	alexei.starovoitov, nogahf, yuvalm, gerlitz.or, Jakub Kicinski
In-Reply-To: <20180526045338.10993-1-jakub.kicinski@netronome.com>

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

^ permalink raw reply related

* [PATCH net-next 08/14] nfp: allow apps to add extra stats to ports
From: Jakub Kicinski @ 2018-05-26  4:53 UTC (permalink / raw)
  To: davem
  Cc: jiri, xiyou.wangcong, john.fastabend, netdev, oss-drivers,
	alexei.starovoitov, nogahf, yuvalm, gerlitz.or, Jakub Kicinski
In-Reply-To: <20180526045338.10993-1-jakub.kicinski@netronome.com>

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

^ permalink raw reply related

* [PATCH net-next 09/14] nfp: abm: expose the internal stats in ethtool
From: Jakub Kicinski @ 2018-05-26  4:53 UTC (permalink / raw)
  To: davem
  Cc: jiri, xiyou.wangcong, john.fastabend, netdev, oss-drivers,
	alexei.starovoitov, nogahf, yuvalm, gerlitz.or, Jakub Kicinski
In-Reply-To: <20180526045338.10993-1-jakub.kicinski@netronome.com>

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

^ permalink raw reply related

* [PATCH net-next 10/14] nfp: abm: expose all PF queues
From: Jakub Kicinski @ 2018-05-26  4:53 UTC (permalink / raw)
  To: davem
  Cc: jiri, xiyou.wangcong, john.fastabend, netdev, oss-drivers,
	alexei.starovoitov, nogahf, yuvalm, gerlitz.or, Jakub Kicinski
In-Reply-To: <20180526045338.10993-1-jakub.kicinski@netronome.com>

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

^ permalink raw reply related

* [PATCH net-next 11/14] net: sched: mq: add simple offload notification
From: Jakub Kicinski @ 2018-05-26  4:53 UTC (permalink / raw)
  To: davem
  Cc: jiri, xiyou.wangcong, john.fastabend, netdev, oss-drivers,
	alexei.starovoitov, nogahf, yuvalm, gerlitz.or, Jakub Kicinski
In-Reply-To: <20180526045338.10993-1-jakub.kicinski@netronome.com>

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

^ permalink raw reply related

* [PATCH net-next 12/14] nfp: abm: multi-queue RED offload
From: Jakub Kicinski @ 2018-05-26  4:53 UTC (permalink / raw)
  To: davem
  Cc: jiri, xiyou.wangcong, john.fastabend, netdev, oss-drivers,
	alexei.starovoitov, nogahf, yuvalm, gerlitz.or, Jakub Kicinski
In-Reply-To: <20180526045338.10993-1-jakub.kicinski@netronome.com>

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

^ permalink raw reply related

* [PATCH net-next 13/14] net: sched: mq: request stats from offloads
From: Jakub Kicinski @ 2018-05-26  4:53 UTC (permalink / raw)
  To: davem
  Cc: jiri, xiyou.wangcong, john.fastabend, netdev, oss-drivers,
	alexei.starovoitov, nogahf, yuvalm, gerlitz.or, Jakub Kicinski
In-Reply-To: <20180526045338.10993-1-jakub.kicinski@netronome.com>

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

^ permalink raw reply related

* [PATCH net-next 14/14] nfp: abm: report correct MQ stats
From: Jakub Kicinski @ 2018-05-26  4:53 UTC (permalink / raw)
  To: davem
  Cc: jiri, xiyou.wangcong, john.fastabend, netdev, oss-drivers,
	alexei.starovoitov, nogahf, yuvalm, gerlitz.or, Jakub Kicinski
In-Reply-To: <20180526045338.10993-1-jakub.kicinski@netronome.com>

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

^ permalink raw reply related

* Re: [bpf-next PATCH] bpf: sockhash fix race with bpf_tcp_close and map delete
From: Song Liu @ 2018-05-26  6:07 UTC (permalink / raw)
  To: John Fastabend; +Cc: ast, daniel, netdev
In-Reply-To: <20180525173712.4004.70590.stgit@john-Precision-Tower-5810>

On Fri, May 25, 2018 at 10:37 AM, John Fastabend
<john.fastabend@gmail.com> wrote:
> syzbot reported two related splats, a use after free and null
> pointer dereference, when a TCP socket is closed while the map is
> also being removed.
>
> The psock keeps a reference to all map slots that have a reference
> to the sock so that when the sock is closed we can clean up any
> outstanding sock{map|hash} entries. This avoids pinning a sock
> forever if the map owner fails to do proper cleanup. However, the
> result is we have two paths that can free an entry in the map. Even
> the comment in the sock{map|hash} tear down function, sock_hash_free()
> notes this:
>
>  At this point no update, lookup or delete operations can happen.
>  However, be aware we can still get a socket state event updates,
>  and data ready callbacks that reference the psock from sk_user_data.
>
> Both removal paths omitted taking the hash bucket lock resulting
> in the case where we have two references that are in the process
> of being free'd.
>
> Reported-by: syzbot+a761b81c211794fa1072@syzkaller.appspotmail.com
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>

Acked-by: Song Liu <songliubraving@fb.com>

> ---
>  kernel/bpf/sockmap.c |   33 +++++++++++++++++++++------------
>  1 file changed, 21 insertions(+), 12 deletions(-)
>
> diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
> index 52a91d8..b508141f 100644
> --- a/kernel/bpf/sockmap.c
> +++ b/kernel/bpf/sockmap.c
> @@ -225,6 +225,16 @@ static void free_htab_elem(struct bpf_htab *htab, struct htab_elem *l)
>         kfree_rcu(l, rcu);
>  }
>
> +static inline struct bucket *__select_bucket(struct bpf_htab *htab, u32 hash)
> +{
> +       return &htab->buckets[hash & (htab->n_buckets - 1)];
> +}
> +
> +static inline struct hlist_head *select_bucket(struct bpf_htab *htab, u32 hash)
> +{
> +       return &__select_bucket(htab, hash)->head;
> +}
> +
>  static void bpf_tcp_close(struct sock *sk, long timeout)
>  {
>         void (*close_fun)(struct sock *sk, long timeout);
> @@ -268,9 +278,15 @@ static void bpf_tcp_close(struct sock *sk, long timeout)
>                                 smap_release_sock(psock, sk);
>                         }
>                 } else {
> +                       u32 hash = e->hash_link->hash;
> +                       struct bucket *b;
> +
> +                       b = __select_bucket(e->htab, hash);
> +                       raw_spin_lock_bh(&b->lock);
>                         hlist_del_rcu(&e->hash_link->hash_node);
>                         smap_release_sock(psock, e->hash_link->sk);
>                         free_htab_elem(e->htab, e->hash_link);
> +                       raw_spin_unlock_bh(&b->lock);
>                 }
>         }
>         write_unlock_bh(&sk->sk_callback_lock);
> @@ -2043,16 +2059,6 @@ static struct bpf_map *sock_hash_alloc(union bpf_attr *attr)
>         return ERR_PTR(err);
>  }
>
> -static inline struct bucket *__select_bucket(struct bpf_htab *htab, u32 hash)
> -{
> -       return &htab->buckets[hash & (htab->n_buckets - 1)];
> -}
> -
> -static inline struct hlist_head *select_bucket(struct bpf_htab *htab, u32 hash)
> -{
> -       return &__select_bucket(htab, hash)->head;
> -}
> -
>  static void sock_hash_free(struct bpf_map *map)
>  {
>         struct bpf_htab *htab = container_of(map, struct bpf_htab, map);
> @@ -2069,10 +2075,12 @@ static void sock_hash_free(struct bpf_map *map)
>          */
>         rcu_read_lock();
>         for (i = 0; i < htab->n_buckets; i++) {
> -               struct hlist_head *head = select_bucket(htab, i);
> +               struct bucket *b = __select_bucket(htab, i);
> +               struct hlist_head *head = &b->head;
>                 struct hlist_node *n;
>                 struct htab_elem *l;
>
> +               raw_spin_lock_bh(&b->lock);
>                 hlist_for_each_entry_safe(l, n, head, hash_node) {
>                         struct sock *sock = l->sk;
>                         struct smap_psock *psock;
> @@ -2090,8 +2098,9 @@ static void sock_hash_free(struct bpf_map *map)
>                                 smap_release_sock(psock, sock);
>                         }
>                         write_unlock_bh(&sock->sk_callback_lock);
> -                       kfree(l);
> +                       free_htab_elem(htab, l);
>                 }
> +               raw_spin_unlock_bh(&b->lock);
>         }
>         rcu_read_unlock();
>         bpf_map_area_free(htab->buckets);
>

^ permalink raw reply

* Re: [PATCH] net: netsec: reduce DMA mask to 40 bits
From: Ard Biesheuvel @ 2018-05-26  6:16 UTC (permalink / raw)
  To: Jassi Brar
  Cc: Robin Murphy, <netdev@vger.kernel.org>, David S. Miller,
	Masahisa Kojima, Ilias Apalodimas, nd
In-Reply-To: <CAJe_ZhfJFdS-CWtGGuaejvVm=pb4i5YpYE9XUQPViO9ucbgkZA@mail.gmail.com>

On 26 May 2018 at 05:44, Jassi Brar <jaswinder.singh@linaro.org> wrote:
> On 26 May 2018 at 08:56, Jassi Brar <jaswinder.singh@linaro.org> wrote:
>> On 26 May 2018 at 01:07, Robin Murphy <robin.murphy@arm.com> wrote:
>>> On Sat, 26 May 2018 00:33:05 +0530
>>> Jassi Brar <jaswinder.singh@linaro.org> wrote:
>>>
>>>> On 25 May 2018 at 18:20, Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>> wrote:
>>>> > The netsec network controller IP can drive 64 address bits for DMA,
>>>> > and the DMA mask is set accordingly in the driver. However, the
>>>> > SynQuacer SoC, which is the only silicon incorporating this IP at
>>>> > the moment, integrates this IP in a manner that leaves address bits
>>>> > [63:40] unconnected.
>>>> >
>>>> > Up until now, this has not resulted in any problems, given that the
>>>> > DDR controller doesn't decode those bits to begin with. However,
>>>> > recent firmware updates for platforms incorporating this SoC allow
>>>> > the IOMMU to be enabled, which does decode address bits [47:40],
>>>> > and allocates top down from the IOVA space, producing DMA addresses
>>>> > that have bits set that have been left unconnected.
>>>> >
>>>> > Both the DT and ACPI (IORT) descriptions of the platform take this
>>>> > into account, and only describe a DMA address space of 40 bits
>>>> > (using either dma-ranges DT properties, or DMA address limits in
>>>> > IORT named component nodes). However, even though our IOMMU and bus
>>>> > layers may take such limitations into account by setting a narrower
>>>> > DMA mask when creating the platform device, the netsec probe()
>>>> > entrypoint follows the common practice of setting the DMA mask
>>>> > uncondionally, according to the capabilities of the IP block itself
>>>> > rather than to its integration into the chip.
>>>> >
>>>> > It is currently unclear what the correct fix is here. We could hack
>>>> > around it by only setting the DMA mask if it deviates from its
>>>> > default value of DMA_BIT_MASK(32). However, this makes it
>>>> > impossible for the bus layer to use DMA_BIT_MASK(32) as the bus
>>>> > limit, and so it appears that a more comprehensive approach is
>>>> > required to take DMA limits imposed by the SoC as a whole into
>>>> > account.
>>>> >
>>>> > In the mean time, let's limit the DMA mask to 40 bits. Given that
>>>> > there is currently only one SoC that incorporates this IP, this is
>>>> > a reasonable approach that can be backported to -stable and buys us
>>>> > some time to come up with a proper fix going forward.
>>>> >
>>>> I am sure you already thought about it, but why not let the platform
>>>> specify the bit mask for the driver (via some "bus-width" property),
>>>> to override the default 64 bit mask?
>>>
>>> Because lack of a property to describe the integration is not the
>>> problem. There are already at least two ways: the general DT/IORT
>>> properties for describing DMA addressing - which it would be a bit
>>> ungainly for a driver to parse for this reason, but not impossible -
>> ....
>>
>>
>>> and inferring it from a SoC-specific compatible - which is more
>>> appropriate, and what we happen to be able to do here.
>>>
>> Sorry, I am not sure I follow. This patch changes from 64-bits default
>> to 40-bits capability without checking for the parent SoC. If the next
>> generation implements the full 64-bit or just 32-bit bus, we'll be
>> back in the pit again. No?
>>
> Probably you meant we'll change the ethernet compatible string for
> differently capable SoC. OK, but here it is more of integration issue
> than controller version.
>
> Which makes me realise the extant compatible property for netsec is
> not so correct (it embeds the platform name). So I am ok either way.
>

The platform in question has a dma-ranges DT property at the root
level that only describes 40 bits' worth of DMA. Also, the ACPI
description in the IORT table of the IOMMU integration of the netsec
controller limits DMA to 40 bits. In the latter case, we actually
enter netsec_probe() with the correct value already assigned to the
DMA mask fields. (In the former case, the DMA limit is ignored
entirely)

In other words, we can already describe these SoC limitations and
distinguish them from device limitations. The problem is that drivers
ignore the existing values of DMA mask.

Robin has volunteered to look into fixing this, but this cannot be
done in a way that is suitable for -stable. In the mean time, we have
a single platform using this network IP in the field that cannot
upgrade its firmware to a version that describes the IOMMU, because
the existing DMA layer code will start driving address bits that are
correctly described as unconnected by the DT/ACPI tables.

So as a a workaround, until Robin fixes things properly, let's reduce
the DMA mask to 40 bits.

^ permalink raw reply

* Re: aio poll and a new in-kernel poll API V13
From: Al Viro @ 2018-05-26  7:09 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Avi Kivity, linux-aio, linux-fsdevel, netdev, linux-api,
	linux-kernel
In-Reply-To: <20180526001111.GL30522@ZenIV.linux.org.uk>

On Sat, May 26, 2018 at 01:11:11AM +0100, Al Viro wrote:
> On Wed, May 23, 2018 at 09:19:49PM +0200, Christoph Hellwig wrote:
> > Hi all,
> > 
> > this series adds support for the IOCB_CMD_POLL operation to poll for the
> > readyness of file descriptors using the aio subsystem.  The API is based
> > on patches that existed in RHAS2.1 and RHEL3, which means it already is
> > supported by libaio.  To implement the poll support efficiently new
> > methods to poll are introduced in struct file_operations:  get_poll_head
> > and poll_mask.  The first one returns a wait_queue_head to wait on
> > (lifetime is bound by the file), and the second does a non-blocking
> > check for the POLL* events.  This allows aio poll to work without
> > any additional context switches, unlike epoll.
> > 
> > This series sits on top of the aio-fsync series that also includes
> > support for io_pgetevents.
> 
> OK, I can live with that, except for one problem - the first patch shouldn't
> be sitting on top of arseloads of next window fodder.
> 
> Please, rebase the rest of the series on top of merge of vfs.git#fixes
> (4faa99965e02) with your aio-fsync.4 and tell me what to pull.

UGH

You've based it on vfs.git#hch.aio (== your aio-fsync.4) + baf10564fbb6
(== vfs.git#fixes^), *and* started with cherry-pick of vfs.git#fixes
on top of that, followed by your series.

That makes no sense whatsoever.  Please, take your aio-fsync.4, merge
vfs.git#fixes (== 4faa99965e02, "fix io_destroy()/aio_complete() race",
same change as your 4e79230e5254) into it and rebase the rest of your
branch on top of that (from "uapi: turn __poll_t sparse checkin
on by default" to "random: convert to ->poll_mask").  BTW, you probably
want s/checkin/checks/ in the first one of those...

--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org.  For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>

^ permalink raw reply

* Re: [PATCH net-next v12 2/5] netvsc: refactor notifier/event handling code to use the failover framework
From: Samudrala, Sridhar @ 2018-05-26  7:22 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: mst, davem, netdev, virtualization, virtio-dev, jesse.brandeburg,
	alexander.h.duyck, kubakici, jasowang, loseweigh, jiri,
	aaron.f.brown, anjali.singhai
In-Reply-To: <20180525162839.3c7028e1@xeon-e3>

On 5/25/2018 4:28 PM, Stephen Hemminger wrote:
> On Fri, 25 May 2018 16:11:47 -0700
> "Samudrala, Sridhar" <sridhar.samudrala@intel.com> wrote:
>
>> On 5/25/2018 3:34 PM, Stephen Hemminger wrote:
>>> On Thu, 24 May 2018 09:55:14 -0700
>>> Sridhar Samudrala <sridhar.samudrala@intel.com> wrote:
>>>   
>>>> --- a/drivers/net/hyperv/Kconfig
>>>> +++ b/drivers/net/hyperv/Kconfig
>>>> @@ -2,5 +2,6 @@ config HYPERV_NET
>>>>    	tristate "Microsoft Hyper-V virtual network driver"
>>>>    	depends on HYPERV
>>>>    	select UCS2_STRING
>>>> +	select FAILOVER
>>> When I take a working kernel config, add the patches then do
>>> make oldconfig
>>>
>>> It is not autoselecting FAILOVER, it prompts me for it. This means
>>> if user says no then a non-working netvsc device is made.
>> I see
>>      Generic failover module (FAILOVER) [M/y/?] (NEW)
>>
>> So the user is given an option to either build as a Module or part of the
>> kernel. 'n' is not an option.
> With most libraries there is no prompt at all.

Not sure what you meant by this.
Without any patches applied, i had a .config file with HYPERV_NET configured
as a module.
Then after applying the first 2 patches in this series, i did a
   make oldconfig
and i see the above prompt.

Are you saying that on some distros, 'make oldconfig creates a .config
file without any prompt and FAILOVER is not getting selected even when HYPERV_NET
is enabled?

^ permalink raw reply

* Re: aio poll and a new in-kernel poll API V13
From: Christoph Hellwig @ 2018-05-26  7:23 UTC (permalink / raw)
  To: Al Viro
  Cc: Christoph Hellwig, Avi Kivity, linux-aio, linux-fsdevel, netdev,
	linux-api, linux-kernel
In-Reply-To: <20180526070937.GA4188@ZenIV.linux.org.uk>

I'm still waking up..

--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org.  For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>

^ permalink raw reply

* Re: [PATCH net-next] bpfilter: fix a build err
From: Yafang Shao @ 2018-05-26  7:41 UTC (permalink / raw)
  To: YueHaibing; +Cc: Alexei Starovoitov, David Miller, ast, netdev, LKML
In-Reply-To: <176ee190-ecda-9b3e-5066-ecf3fbaf32bb@huawei.com>

On Sat, May 26, 2018 at 10:25 AM, YueHaibing <yuehaibing@huawei.com> wrote:
> On 2018/5/26 0:19, Alexei Starovoitov wrote:
>> On Fri, May 25, 2018 at 06:17:57PM +0800, YueHaibing wrote:
>>> gcc-7.3.0 report following err:
>>>
>>>   HOSTCC  net/bpfilter/main.o
>>> In file included from net/bpfilter/main.c:9:0:
>>> ./include/uapi/linux/bpf.h:12:10: fatal error: linux/bpf_common.h: No such file or directory
>>>  #include <linux/bpf_common.h>
>>>
>>> remove it by adding a include path.
>>> Fixes: d2ba09c17a06 ("net: add skeleton of bpfilter kernel module")
>>>
>>> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
>>> ---
>>>  net/bpfilter/Makefile | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/net/bpfilter/Makefile b/net/bpfilter/Makefile
>>> index 2af752c..3f3cb87 100644
>>> --- a/net/bpfilter/Makefile
>>> +++ b/net/bpfilter/Makefile
>>> @@ -5,7 +5,7 @@
>>>
>>>  hostprogs-y := bpfilter_umh
>>>  bpfilter_umh-objs := main.o
>>> -HOSTCFLAGS += -I. -Itools/include/
>>> +HOSTCFLAGS += -I. -Itools/include/ -Itools/include/uapi
>>
>> Strangely I don't see this error with gcc 7.3
>> I've tried this patch and it doesn't hurt,
>> but before it gets applied could you please try
>> the top two patches from this tree:
>> https://git.kernel.org/pub/scm/linux/kernel/git/ast/bpf.git/?h=ipt_bpf
>> in your environment?
>> These two patches add the actual meat of bpfilter and I'd like
>> to make sure the build setup is good for everyone before
>> we proceed too far.
>
> after applied these two patches on net-next, the err still here:
>  bpfilter: rough bpfilter codegen example hack
>  bpfilter: add iptable get/set parsing
>
>   HOSTCC  net/bpfilter/main.o
> In file included from net/bpfilter/main.c:13:0:
> ./include/uapi/linux/bpf.h:12:10: fatal error: linux/bpf_common.h: No such file or directory
>  #include <linux/bpf_common.h>
>           ^~~~~~~~~~~~~~~~~~~~
> compilation terminated.
> make[2]: *** [net/bpfilter/main.o] Error 1
> make[1]: *** [net/bpfilter] Error 2
> make: *** [net] Error 2
>
> Also I compile your tree, error is same
>
> my gcc version info as follow:
> [root@localhost net-next]# gcc -v
> Using built-in specs.
> COLLECT_GCC=gcc
> COLLECT_LTO_WRAPPER=/home/yuehb/gcc-7.3.0-tools/libexec/gcc/x86_64-pc-linux-gnu/7.3.0/lto-wrapper
> Target: x86_64-pc-linux-gnu
> Configured with: ../gcc-7.3.0/configure --enable-checking=release --enable-languages=c,c++
> --disable-multilib --prefix=/home/yuehb/gcc-7.3.0-tools
> Thread model: posix
> gcc version 7.3.0 (GCC)
>
>>


This error also occurs on gcc-4.8.5.
After applied Haibin's patch, this build error disapears.

Thanks
Yafang

^ permalink raw reply

* Re: [PATCH net-next v12 1/5] net: Introduce generic failover module
From: Jiri Pirko @ 2018-05-26  7:43 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Sridhar Samudrala, mst, davem, netdev, virtualization, virtio-dev,
	jesse.brandeburg, alexander.h.duyck, kubakici, jasowang,
	loseweigh, aaron.f.brown, anjali.singhai
In-Reply-To: <20180525153744.2b53c449@xeon-e3>

Sat, May 26, 2018 at 12:37:44AM CEST, stephen@networkplumber.org wrote:
>On Thu, 24 May 2018 09:55:13 -0700
>Sridhar Samudrala <sridhar.samudrala@intel.com> wrote:
>
>
>> +	spin_lock(&failover_lock);
>
>Since register is not in fast path, this should be a mutex?

I don't get it. Why would you prefer mutex over spinlock here?

^ permalink raw reply

* Re: [PATCH net-next v12 2/5] netvsc: refactor notifier/event handling code to use the failover framework
From: Jiri Pirko @ 2018-05-26  7:51 UTC (permalink / raw)
  To: Samudrala, Sridhar
  Cc: alexander.h.duyck, virtio-dev, mst, kubakici, netdev,
	virtualization, loseweigh, anjali.singhai, aaron.f.brown, davem
In-Reply-To: <cffe3c66-ae31-ae13-27ce-55f9be2d53d4@intel.com>

Sat, May 26, 2018 at 09:22:18AM CEST, sridhar.samudrala@intel.com wrote:
>On 5/25/2018 4:28 PM, Stephen Hemminger wrote:
>> On Fri, 25 May 2018 16:11:47 -0700
>> "Samudrala, Sridhar" <sridhar.samudrala@intel.com> wrote:
>> 
>> > On 5/25/2018 3:34 PM, Stephen Hemminger wrote:
>> > > On Thu, 24 May 2018 09:55:14 -0700
>> > > Sridhar Samudrala <sridhar.samudrala@intel.com> wrote:
>> > > > --- a/drivers/net/hyperv/Kconfig
>> > > > +++ b/drivers/net/hyperv/Kconfig
>> > > > @@ -2,5 +2,6 @@ config HYPERV_NET
>> > > >    	tristate "Microsoft Hyper-V virtual network driver"
>> > > >    	depends on HYPERV
>> > > >    	select UCS2_STRING
>> > > > +	select FAILOVER
>> > > When I take a working kernel config, add the patches then do
>> > > make oldconfig
>> > > 
>> > > It is not autoselecting FAILOVER, it prompts me for it. This means
>> > > if user says no then a non-working netvsc device is made.
>> > I see
>> >      Generic failover module (FAILOVER) [M/y/?] (NEW)
>> > 
>> > So the user is given an option to either build as a Module or part of the
>> > kernel. 'n' is not an option.
>> With most libraries there is no prompt at all.
>
>Not sure what you meant by this.
>Without any patches applied, i had a .config file with HYPERV_NET configured
>as a module.
>Then after applying the first 2 patches in this series, i did a
>  make oldconfig
>and i see the above prompt.
>
>Are you saying that on some distros, 'make oldconfig creates a .config
>file without any prompt and FAILOVER is not getting selected even when HYPERV_NET
>is enabled?
>
>

Well the thing is that for a user, it makes no sense to select
"FAILOVER" by hand. It is a lib, so it should be only select it by a
user. It has no sense to have it turned on by hand - no lib user.
You can achieve that by simply removing "help" for the Kconfig
item. Same thing for "NET_FAILOVER".

^ permalink raw reply

* Re: WARNING: ODEBUG bug in __sk_destruct
From: syzbot @ 2018-05-26  7:52 UTC (permalink / raw)
  To: davem, linux-kernel, linux-s390, netdev, syzkaller-bugs, ubraun
In-Reply-To: <000000000000451f9d056aff4397@google.com>

syzbot has found a reproducer for the following crash on:

HEAD commit:    e52cde717093 net: dsa: dsa_loop: Make dynamic debugging he..
git tree:       net-next
console output: https://syzkaller.appspot.com/x/log.txt?x=1424a4b7800000
kernel config:  https://syzkaller.appspot.com/x/.config?x=e4078980b886800c
dashboard link: https://syzkaller.appspot.com/bug?extid=92209502e7aab127c75f
compiler:       gcc (GCC) 8.0.1 20180413 (experimental)
syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=1071bc2f800000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=16b51cb7800000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+92209502e7aab127c75f@syzkaller.appspotmail.com

------------[ cut here ]------------
ODEBUG: free active (active state 0) object type: work_struct hint:  
smc_tx_work+0x0/0x350 include/linux/compiler.h:188
WARNING: CPU: 0 PID: 5254 at lib/debugobjects.c:329  
debug_print_object+0x16a/0x210 lib/debugobjects.c:326
Kernel panic - not syncing: panic_on_warn set ...

CPU: 0 PID: 5254 Comm: syz-executor351 Not tainted 4.17.0-rc6+ #64
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Call Trace:
  __dump_stack lib/dump_stack.c:77 [inline]
  dump_stack+0x1b9/0x294 lib/dump_stack.c:113
  panic+0x22f/0x4de kernel/panic.c:184
  __warn.cold.8+0x163/0x1b3 kernel/panic.c:536
  report_bug+0x252/0x2d0 lib/bug.c:186
  fixup_bug arch/x86/kernel/traps.c:178 [inline]
  do_error_trap+0x1de/0x490 arch/x86/kernel/traps.c:296
  do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:315
  invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:992
RIP: 0010:debug_print_object+0x16a/0x210 lib/debugobjects.c:326
RSP: 0018:ffff8801c6f67158 EFLAGS: 00010082
RAX: 0000000000000059 RBX: 0000000000000003 RCX: ffffffff818435f8
RDX: 0000000000000000 RSI: ffffffff8160f2c1 RDI: 0000000000000001
RBP: ffff8801c6f67198 R08: ffff8801cb640580 R09: ffffed003b5c3eb2
R10: ffffed003b5c3eb2 R11: ffff8801dae1f597 R12: 0000000000000001
R13: ffffffff88d5f040 R14: ffffffff87fa2a00 R15: ffffffff814ccb10
  __debug_check_no_obj_freed lib/debugobjects.c:783 [inline]
  debug_check_no_obj_freed+0x3a6/0x584 lib/debugobjects.c:815
  kmem_cache_free+0x216/0x2d0 mm/slab.c:3755
  sk_prot_free net/core/sock.c:1516 [inline]
  __sk_destruct+0x6fe/0xa40 net/core/sock.c:1600
  sk_destruct+0x78/0x90 net/core/sock.c:1608
  __sk_free+0xcf/0x300 net/core/sock.c:1619
  sk_free+0x42/0x50 net/core/sock.c:1630
  sock_put include/net/sock.h:1669 [inline]
  smc_release+0x459/0x610 net/smc/af_smc.c:156
  sock_release+0x96/0x1b0 net/socket.c:594
  sock_close+0x16/0x20 net/socket.c:1149
  __fput+0x34d/0x890 fs/file_table.c:209
  ____fput+0x15/0x20 fs/file_table.c:243
  task_work_run+0x1e4/0x290 kernel/task_work.c:113
  exit_task_work include/linux/task_work.h:22 [inline]
  do_exit+0x1aee/0x2730 kernel/exit.c:865
  do_group_exit+0x16f/0x430 kernel/exit.c:968
  __do_sys_exit_group kernel/exit.c:979 [inline]
  __se_sys_exit_group kernel/exit.c:977 [inline]
  __x64_sys_exit_group+0x3e/0x50 kernel/exit.c:977
  do_syscall_64+0x1b1/0x800 arch/x86/entry/common.c:287
  entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x4424f9
RSP: 002b:00007ffcbea55c78 EFLAGS: 00000202 ORIG_RAX: 00000000000000e7
RAX: ffffffffffffffda RBX: 00000000000002c0 RCX: 00000000004424f9
RDX: 00000000004424f9 RSI: 0000000000000001 RDI: 0000000000000000
RBP: 00007ffcbea55db0 R08: 0000000300000000 R09: 00007ffcbea55cc0
R10: 0000000000000004 R11: 0000000000000202 R12: 0000000000000000
R13: 0000000000000000 R14: 0000000000001380 R15: 00007ffcbea55dd8

======================================================
WARNING: possible circular locking dependency detected
4.17.0-rc6+ #64 Not tainted
------------------------------------------------------
syz-executor351/5254 is trying to acquire lock:
         (ptrval) ((console_sem).lock){-...}, at: down_trylock+0x13/0x70  
kernel/locking/semaphore.c:136

but task is already holding lock:
         (ptrval) (&obj_hash[i].lock){-.-.}, at: __debug_check_no_obj_freed  
lib/debugobjects.c:774 [inline]
         (ptrval) (&obj_hash[i].lock){-.-.}, at:  
debug_check_no_obj_freed+0x159/0x584 lib/debugobjects.c:815

which lock already depends on the new lock.


the existing dependency chain (in reverse order) is:

-> #3 (&obj_hash[i].lock){-.-.}:
        __raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline]
        _raw_spin_lock_irqsave+0x96/0xc0 kernel/locking/spinlock.c:152
        __debug_object_init+0x11f/0x12c0 lib/debugobjects.c:381
        debug_object_init+0x16/0x20 lib/debugobjects.c:429
        debug_hrtimer_init kernel/time/hrtimer.c:410 [inline]
        debug_init kernel/time/hrtimer.c:458 [inline]
        hrtimer_init+0x8f/0x460 kernel/time/hrtimer.c:1308
        init_dl_task_timer+0x1b/0x50 kernel/sched/deadline.c:1056
        __sched_fork+0x2ae/0xc20 kernel/sched/core.c:2166
        init_idle+0x75/0x7a0 kernel/sched/core.c:5402
        sched_init+0xbeb/0xd10 kernel/sched/core.c:6100
        start_kernel+0x475/0x92d init/main.c:601
        x86_64_start_reservations+0x29/0x2b arch/x86/kernel/head64.c:453
        x86_64_start_kernel+0x76/0x79 arch/x86/kernel/head64.c:434
        secondary_startup_64+0xa5/0xb0 arch/x86/kernel/head_64.S:242

-> #2 (&rq->lock){-.-.}:
        __raw_spin_lock include/linux/spinlock_api_smp.h:142 [inline]
        _raw_spin_lock+0x2a/0x40 kernel/locking/spinlock.c:144
        rq_lock kernel/sched/sched.h:1799 [inline]
        task_fork_fair+0x8a/0x660 kernel/sched/fair.c:9912
        sched_fork+0x43e/0xb30 kernel/sched/core.c:2382
        copy_process.part.38+0x1c18/0x6e90 kernel/fork.c:1764
        copy_process kernel/fork.c:1607 [inline]
        _do_fork+0x291/0x12a0 kernel/fork.c:2088
        kernel_thread+0x34/0x40 kernel/fork.c:2147
        rest_init+0x22/0xe4 init/main.c:407
        start_kernel+0x906/0x92d init/main.c:737
        x86_64_start_reservations+0x29/0x2b arch/x86/kernel/head64.c:453
        x86_64_start_kernel+0x76/0x79 arch/x86/kernel/head64.c:434
        secondary_startup_64+0xa5/0xb0 arch/x86/kernel/head_64.S:242

-> #1 (&p->pi_lock){-.-.}:
        __raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline]
        _raw_spin_lock_irqsave+0x96/0xc0 kernel/locking/spinlock.c:152
        try_to_wake_up+0xca/0x1190 kernel/sched/core.c:1966
        wake_up_process+0x10/0x20 kernel/sched/core.c:2129
        __up.isra.1+0x1b8/0x290 kernel/locking/semaphore.c:262
        up+0x12f/0x1b0 kernel/locking/semaphore.c:187
        __up_console_sem+0xbe/0x1b0 kernel/printk/printk.c:242
        console_unlock+0x7d6/0x1100 kernel/printk/printk.c:2417
        do_con_write+0x12b2/0x2280 drivers/tty/vt/vt.c:2437
        con_write+0x25/0xc0 drivers/tty/vt/vt.c:2786
        process_output_block drivers/tty/n_tty.c:579 [inline]
        n_tty_write+0x6b9/0x1180 drivers/tty/n_tty.c:2308
        do_tty_write drivers/tty/tty_io.c:958 [inline]
        tty_write+0x3f1/0x880 drivers/tty/tty_io.c:1042
        __vfs_write+0x10b/0x960 fs/read_write.c:485
        vfs_write+0x1f8/0x560 fs/read_write.c:549
        ksys_write+0xf9/0x250 fs/read_write.c:598
        __do_sys_write fs/read_write.c:610 [inline]
        __se_sys_write fs/read_write.c:607 [inline]
        __x64_sys_write+0x73/0xb0 fs/read_write.c:607
        do_syscall_64+0x1b1/0x800 arch/x86/entry/common.c:287
        entry_SYSCALL_64_after_hwframe+0x49/0xbe

-> #0 ((console_sem).lock){-...}:
        lock_acquire+0x1dc/0x520 kernel/locking/lockdep.c:3920
        __raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline]
        _raw_spin_lock_irqsave+0x96/0xc0 kernel/locking/spinlock.c:152
        down_trylock+0x13/0x70 kernel/locking/semaphore.c:136
        __down_trylock_console_sem+0xae/0x200 kernel/printk/printk.c:225
        console_trylock+0x15/0xa0 kernel/printk/printk.c:2229
        console_trylock_spinning kernel/printk/printk.c:1643 [inline]
        vprintk_emit+0x694/0xdd0 kernel/printk/printk.c:1906
        vprintk_default+0x28/0x30 kernel/printk/printk.c:1947
        vprintk_func+0x7a/0xe7 kernel/printk/printk_safe.c:379
        printk+0x9e/0xba kernel/printk/printk.c:1980
        __warn_printk+0x83/0xd0 kernel/panic.c:590
        debug_print_object+0x16a/0x210 lib/debugobjects.c:326
        __debug_check_no_obj_freed lib/debugobjects.c:783 [inline]
        debug_check_no_obj_freed+0x3a6/0x584 lib/debugobjects.c:815
        kmem_cache_free+0x216/0x2d0 mm/slab.c:3755
        sk_prot_free net/core/sock.c:1516 [inline]
        __sk_destruct+0x6fe/0xa40 net/core/sock.c:1600
        sk_destruct+0x78/0x90 net/core/sock.c:1608
        __sk_free+0xcf/0x300 net/core/sock.c:1619
        sk_free+0x42/0x50 net/core/sock.c:1630
        sock_put include/net/sock.h:1669 [inline]
        smc_release+0x459/0x610 net/smc/af_smc.c:156
        sock_release+0x96/0x1b0 net/socket.c:594
        sock_close+0x16/0x20 net/socket.c:1149
        __fput+0x34d/0x890 fs/file_table.c:209
        ____fput+0x15/0x20 fs/file_table.c:243
        task_work_run+0x1e4/0x290 kernel/task_work.c:113
        exit_task_work include/linux/task_work.h:22 [inline]
        do_exit+0x1aee/0x2730 kernel/exit.c:865
        do_group_exit+0x16f/0x430 kernel/exit.c:968
        __do_sys_exit_group kernel/exit.c:979 [inline]
        __se_sys_exit_group kernel/exit.c:977 [inline]
        __x64_sys_exit_group+0x3e/0x50 kernel/exit.c:977
        do_syscall_64+0x1b1/0x800 arch/x86/entry/common.c:287
        entry_SYSCALL_64_after_hwframe+0x49/0xbe

other info that might help us debug this:

Chain exists of:
   (console_sem).lock --> &rq->lock --> &obj_hash[i].lock

  Possible unsafe locking scenario:

        CPU0                    CPU1
        ----                    ----
   lock(&obj_hash[i].lock);
                                lock(&rq->lock);
                                lock(&obj_hash[i].lock);
   lock((console_sem).lock);

  *** DEADLOCK ***

1 lock held by syz-executor351/5254:
  #0:         (ptrval) (&obj_hash[i].lock){-.-.}, at:  
__debug_check_no_obj_freed lib/debugobjects.c:774 [inline]
  #0:         (ptrval) (&obj_hash[i].lock){-.-.}, at:  
debug_check_no_obj_freed+0x159/0x584 lib/debugobjects.c:815

stack backtrace:
CPU: 0 PID: 5254 Comm: syz-executor351 Not tainted 4.17.0-rc6+ #64
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Call Trace:
  __dump_stack lib/dump_stack.c:77 [inline]
  dump_stack+0x1b9/0x294 lib/dump_stack.c:113
  print_circular_bug.isra.36.cold.54+0x1bd/0x27d  
kernel/locking/lockdep.c:1223
  check_prev_add kernel/locking/lockdep.c:1863 [inline]
  check_prevs_add kernel/locking/lockdep.c:1976 [inline]
  validate_chain kernel/locking/lockdep.c:2417 [inline]
  __lock_acquire+0x343e/0x5140 kernel/locking/lockdep.c:3431
  lock_acquire+0x1dc/0x520 kernel/locking/lockdep.c:3920
  __raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline]
  _raw_spin_lock_irqsave+0x96/0xc0 kernel/locking/spinlock.c:152
  down_trylock+0x13/0x70 kernel/locking/semaphore.c:136
  __down_trylock_console_sem+0xae/0x200 kernel/printk/printk.c:225
  console_trylock+0x15/0xa0 kernel/printk/printk.c:2229
  console_trylock_spinning kernel/printk/printk.c:1643 [inline]
  vprintk_emit+0x694/0xdd0 kernel/printk/printk.c:1906
  vprintk_default+0x28/0x30 kernel/printk/printk.c:1947
  vprintk_func+0x7a/0xe7 kernel/printk/printk_safe.c:379
  printk+0x9e/0xba kernel/printk/printk.c:1980
  __warn_printk+0x83/0xd0 kernel/panic.c:590
  debug_print_object+0x16a/0x210 lib/debugobjects.c:326
  __debug_check_no_obj_freed lib/debugobjects.c:783 [inline]
  debug_check_no_obj_freed+0x3a6/0x584 lib/debugobjects.c:815
  kmem_cache_free+0x216/0x2d0 mm/slab.c:3755
  sk_prot_free net/core/sock.c:1516 [inline]
  __sk_destruct+0x6fe/0xa40 net/core/sock.c:1600
  sk_destruct+0x78/0x90 net/core/sock.c:1608
  __sk_free+0xcf/0x300 net/core/sock.c:1619
  sk_free+0x42/0x50 net/core/sock.c:1630
  sock_put include/net/sock.h:1669 [inline]
  smc_release+0x459/0x610 net/smc/af_smc.c:156
  sock_release+0x96/0x1b0 net/socket.c:594
  sock_close+0x16/0x20 net/socket.c:1149
  __fput+0x34d/0x890 fs/file_table.c:209
  ____fput+0x15/0x20 fs/file_table.c:243
  task_work_run+0x1e4/0x290 kernel/task_work.c:113
  exit_task_work include/linux/task_work.h:22 [inline]
  do_exit+0x1aee/0x2730 kernel/exit.c:865
  do_group_exit+0x16f/0x430 kernel/exit.c:968
  __do_sys_exit_group kernel/exit.c:979 [inline]
  __se_sys_exit_group kernel/exit.c:977 [inline]
  __x64_sys_exit_group+0x3e/0x50 kernel/exit.c:977
  do_syscall_64+0x1b1/0x800 arch/x86/entry/common.c:287
  entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x4424f9
RSP: 002b:00007ffcbea55c78 EFLAGS: 00000202 ORIG_RAX: 00000000000000e7
RAX: ffffffffffffffda RBX: 00000000000002c0 RCX: 00000000004424f9
RDX: 00000000004424f9 RSI: 0000000000000001 RDI: 0000000000000000
RBP: 00007ffcbea55db0 R08: 000000030
Lost 2 message(s)!
Dumping ftrace buffer:
    (ftrace buffer empty)
Kernel Offset: disabled
Rebooting in 86400 seconds..

^ permalink raw reply

* Re: [bpf-next PATCH] bpf: sockhash fix race with bpf_tcp_close and map delete
From: Daniel Borkmann @ 2018-05-26  8:30 UTC (permalink / raw)
  To: John Fastabend, ast; +Cc: netdev
In-Reply-To: <20180525173712.4004.70590.stgit@john-Precision-Tower-5810>

Hi John,

On 05/25/2018 07:37 PM, John Fastabend wrote:
> syzbot reported two related splats, a use after free and null
> pointer dereference, when a TCP socket is closed while the map is
> also being removed.
> 
> The psock keeps a reference to all map slots that have a reference
> to the sock so that when the sock is closed we can clean up any
> outstanding sock{map|hash} entries. This avoids pinning a sock
> forever if the map owner fails to do proper cleanup. However, the
> result is we have two paths that can free an entry in the map. Even
> the comment in the sock{map|hash} tear down function, sock_hash_free()
> notes this:
> 
>  At this point no update, lookup or delete operations can happen.
>  However, be aware we can still get a socket state event updates,
>  and data ready callbacks that reference the psock from sk_user_data.
> 
> Both removal paths omitted taking the hash bucket lock resulting
> in the case where we have two references that are in the process
> of being free'd.
> 
> Reported-by: syzbot+a761b81c211794fa1072@syzkaller.appspotmail.com
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>

Could you also shortly reply with a Fixes: tag so we can track all
fixes for the original commit.

Thanks,
Daniel

P.s.: still waiting on net-next to get fast-forwarded, then I'll
fast-forward bpf-next and process the queue.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox