linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/9] staging: octeon: multi rx group (queue) support
@ 2016-08-30 18:47 Aaro Koskinen
  2016-08-30 18:47 ` [PATCH 1/9] staging: octeon: disable rx interrupts in oct_rx_shutdown Aaro Koskinen
                   ` (9 more replies)
  0 siblings, 10 replies; 17+ messages in thread
From: Aaro Koskinen @ 2016-08-30 18:47 UTC (permalink / raw)
  To: Greg Kroah-Hartman, David Daney, Ed Swierk, devel
  Cc: linux-kernel, Aaro Koskinen

Hi,

This series implements multiple RX group support that should improve
the networking performance on multi-core OCTEONs. Basically we register
IRQ and NAPI for each group, and ask the HW to select the group for
the incoming packets based on hash.

Tested on EdgeRouter Lite with a simple forwarding test using two flows
and 16 RX groups distributed between two cores - the routing throughput
is roughly doubled.

A.

Aaro Koskinen (9):
  staging: octeon: disable rx interrupts in oct_rx_shutdown
  staging: octeon: use passed interrupt number in the handler
  staging: octeon: pass the NAPI instance reference to irq handler
  staging: octeon: move common poll code into a separate function
  staging: octeon: create a struct for rx group specific data
  staging: octeon: move irq into rx group specific data
  staging: octeon: move group number into rx group data
  staging: octeon: support enabling multiple rx groups
  staging: octeon: enable taking multiple rx groups into use

 drivers/staging/octeon/ethernet-rx.c     | 178 ++++++++++++++++++++-----------
 drivers/staging/octeon/ethernet.c        |  55 ++++++++--
 drivers/staging/octeon/octeon-ethernet.h |   2 +-
 3 files changed, 159 insertions(+), 76 deletions(-)

-- 
2.9.2

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 1/9] staging: octeon: disable rx interrupts in oct_rx_shutdown
  2016-08-30 18:47 [PATCH 0/9] staging: octeon: multi rx group (queue) support Aaro Koskinen
@ 2016-08-30 18:47 ` Aaro Koskinen
  2016-08-30 18:47 ` [PATCH 2/9] staging: octeon: use passed interrupt number in the handler Aaro Koskinen
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 17+ messages in thread
From: Aaro Koskinen @ 2016-08-30 18:47 UTC (permalink / raw)
  To: Greg Kroah-Hartman, David Daney, Ed Swierk, devel
  Cc: linux-kernel, Aaro Koskinen

Disable RX interrupts in oct_rx_shutdown(). This way we don't need to
expose the RX IRQ numbers outside the RX module.

Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi>
---
 drivers/staging/octeon/ethernet-rx.c | 9 +++++++++
 drivers/staging/octeon/ethernet.c    | 9 ---------
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/octeon/ethernet-rx.c b/drivers/staging/octeon/ethernet-rx.c
index a10fe3a..5b26f2a 100644
--- a/drivers/staging/octeon/ethernet-rx.c
+++ b/drivers/staging/octeon/ethernet-rx.c
@@ -495,5 +495,14 @@ void cvm_oct_rx_initialize(void)
 
 void cvm_oct_rx_shutdown(void)
 {
+	/* Disable POW interrupt */
+	if (OCTEON_IS_MODEL(OCTEON_CN68XX))
+		cvmx_write_csr(CVMX_SSO_WQ_INT_THRX(pow_receive_group), 0);
+	else
+		cvmx_write_csr(CVMX_POW_WQ_INT_THRX(pow_receive_group), 0);
+
+	/* Free the interrupt handler */
+	free_irq(OCTEON_IRQ_WORKQ0 + pow_receive_group, cvm_oct_device);
+
 	netif_napi_del(&cvm_oct_napi);
 }
diff --git a/drivers/staging/octeon/ethernet.c b/drivers/staging/octeon/ethernet.c
index 073a1e3..1e2e1ef 100644
--- a/drivers/staging/octeon/ethernet.c
+++ b/drivers/staging/octeon/ethernet.c
@@ -853,17 +853,8 @@ static int cvm_oct_remove(struct platform_device *pdev)
 {
 	int port;
 
-	/* Disable POW interrupt */
-	if (OCTEON_IS_MODEL(OCTEON_CN68XX))
-		cvmx_write_csr(CVMX_SSO_WQ_INT_THRX(pow_receive_group), 0);
-	else
-		cvmx_write_csr(CVMX_POW_WQ_INT_THRX(pow_receive_group), 0);
-
 	cvmx_ipd_disable();
 
-	/* Free the interrupt handler */
-	free_irq(OCTEON_IRQ_WORKQ0 + pow_receive_group, cvm_oct_device);
-
 	atomic_inc_return(&cvm_oct_poll_queue_stopping);
 	cancel_delayed_work_sync(&cvm_oct_rx_refill_work);
 
-- 
2.9.2

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 2/9] staging: octeon: use passed interrupt number in the handler
  2016-08-30 18:47 [PATCH 0/9] staging: octeon: multi rx group (queue) support Aaro Koskinen
  2016-08-30 18:47 ` [PATCH 1/9] staging: octeon: disable rx interrupts in oct_rx_shutdown Aaro Koskinen
@ 2016-08-30 18:47 ` Aaro Koskinen
  2016-08-30 18:47 ` [PATCH 3/9] staging: octeon: pass the NAPI instance reference to irq handler Aaro Koskinen
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 17+ messages in thread
From: Aaro Koskinen @ 2016-08-30 18:47 UTC (permalink / raw)
  To: Greg Kroah-Hartman, David Daney, Ed Swierk, devel
  Cc: linux-kernel, Aaro Koskinen

Use passed interrupt number in the handler, so we can avoid using
the global variable.

Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi>
---
 drivers/staging/octeon/ethernet-rx.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/octeon/ethernet-rx.c b/drivers/staging/octeon/ethernet-rx.c
index 5b26f2a..808c415 100644
--- a/drivers/staging/octeon/ethernet-rx.c
+++ b/drivers/staging/octeon/ethernet-rx.c
@@ -47,16 +47,16 @@ static struct napi_struct cvm_oct_napi;
 
 /**
  * cvm_oct_do_interrupt - interrupt handler.
- * @cpl: Interrupt number. Unused
+ * @irq: Interrupt number.
  * @dev_id: Cookie to identify the device. Unused
  *
  * The interrupt occurs whenever the POW has packets in our group.
  *
  */
-static irqreturn_t cvm_oct_do_interrupt(int cpl, void *dev_id)
+static irqreturn_t cvm_oct_do_interrupt(int irq, void *dev_id)
 {
 	/* Disable the IRQ and start napi_poll. */
-	disable_irq_nosync(OCTEON_IRQ_WORKQ0 + pow_receive_group);
+	disable_irq_nosync(irq);
 	napi_schedule(&cvm_oct_napi);
 
 	return IRQ_HANDLED;
-- 
2.9.2

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 3/9] staging: octeon: pass the NAPI instance reference to irq handler
  2016-08-30 18:47 [PATCH 0/9] staging: octeon: multi rx group (queue) support Aaro Koskinen
  2016-08-30 18:47 ` [PATCH 1/9] staging: octeon: disable rx interrupts in oct_rx_shutdown Aaro Koskinen
  2016-08-30 18:47 ` [PATCH 2/9] staging: octeon: use passed interrupt number in the handler Aaro Koskinen
@ 2016-08-30 18:47 ` Aaro Koskinen
  2016-08-30 18:47 ` [PATCH 4/9] staging: octeon: move common poll code into a separate function Aaro Koskinen
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 17+ messages in thread
From: Aaro Koskinen @ 2016-08-30 18:47 UTC (permalink / raw)
  To: Greg Kroah-Hartman, David Daney, Ed Swierk, devel
  Cc: linux-kernel, Aaro Koskinen

Pass the NAPI instance reference to the interrupt handler.
This is preparation for having multiple NAPI instances.

Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi>
---
 drivers/staging/octeon/ethernet-rx.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/octeon/ethernet-rx.c b/drivers/staging/octeon/ethernet-rx.c
index 808c415..27e3459 100644
--- a/drivers/staging/octeon/ethernet-rx.c
+++ b/drivers/staging/octeon/ethernet-rx.c
@@ -48,16 +48,16 @@ static struct napi_struct cvm_oct_napi;
 /**
  * cvm_oct_do_interrupt - interrupt handler.
  * @irq: Interrupt number.
- * @dev_id: Cookie to identify the device. Unused
+ * @napi_id: Cookie to identify the NAPI instance.
  *
  * The interrupt occurs whenever the POW has packets in our group.
  *
  */
-static irqreturn_t cvm_oct_do_interrupt(int irq, void *dev_id)
+static irqreturn_t cvm_oct_do_interrupt(int irq, void *napi_id)
 {
 	/* Disable the IRQ and start napi_poll. */
 	disable_irq_nosync(irq);
-	napi_schedule(&cvm_oct_napi);
+	napi_schedule(napi_id);
 
 	return IRQ_HANDLED;
 }
@@ -452,7 +452,7 @@ void cvm_oct_rx_initialize(void)
 
 	/* Register an IRQ handler to receive POW interrupts */
 	i = request_irq(OCTEON_IRQ_WORKQ0 + pow_receive_group,
-			cvm_oct_do_interrupt, 0, "Ethernet", cvm_oct_device);
+			cvm_oct_do_interrupt, 0, "Ethernet", &cvm_oct_napi);
 
 	if (i)
 		panic("Could not acquire Ethernet IRQ %d\n",
-- 
2.9.2

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 4/9] staging: octeon: move common poll code into a separate function
  2016-08-30 18:47 [PATCH 0/9] staging: octeon: multi rx group (queue) support Aaro Koskinen
                   ` (2 preceding siblings ...)
  2016-08-30 18:47 ` [PATCH 3/9] staging: octeon: pass the NAPI instance reference to irq handler Aaro Koskinen
@ 2016-08-30 18:47 ` Aaro Koskinen
  2016-08-30 18:47 ` [PATCH 5/9] staging: octeon: create a struct for rx group specific data Aaro Koskinen
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 17+ messages in thread
From: Aaro Koskinen @ 2016-08-30 18:47 UTC (permalink / raw)
  To: Greg Kroah-Hartman, David Daney, Ed Swierk, devel
  Cc: linux-kernel, Aaro Koskinen

Move common poll code into a separate function.

Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi>
---
 drivers/staging/octeon/ethernet-rx.c | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/octeon/ethernet-rx.c b/drivers/staging/octeon/ethernet-rx.c
index 27e3459..140e8af 100644
--- a/drivers/staging/octeon/ethernet-rx.c
+++ b/drivers/staging/octeon/ethernet-rx.c
@@ -143,14 +143,7 @@ static inline int cvm_oct_check_rcv_error(cvmx_wqe_t *work)
 	return 0;
 }
 
-/**
- * cvm_oct_napi_poll - the NAPI poll function.
- * @napi: The NAPI instance, or null if called from cvm_oct_poll_controller
- * @budget: Maximum number of packets to receive.
- *
- * Returns the number of packets processed.
- */
-static int cvm_oct_napi_poll(struct napi_struct *napi, int budget)
+static int cvm_oct_poll(int budget)
 {
 	const int	coreid = cvmx_get_core_num();
 	u64	old_group_mask;
@@ -410,7 +403,23 @@ static int cvm_oct_napi_poll(struct napi_struct *napi, int budget)
 	}
 	cvm_oct_rx_refill_pool(0);
 
-	if (rx_count < budget && napi) {
+	return rx_count;
+}
+
+/**
+ * cvm_oct_napi_poll - the NAPI poll function.
+ * @napi: The NAPI instance.
+ * @budget: Maximum number of packets to receive.
+ *
+ * Returns the number of packets processed.
+ */
+static int cvm_oct_napi_poll(struct napi_struct *napi, int budget)
+{
+	int rx_count;
+
+	rx_count = cvm_oct_poll(budget);
+
+	if (rx_count < budget) {
 		/* No more work */
 		napi_complete(napi);
 		enable_irq(OCTEON_IRQ_WORKQ0 + pow_receive_group);
@@ -427,7 +436,7 @@ static int cvm_oct_napi_poll(struct napi_struct *napi, int budget)
  */
 void cvm_oct_poll_controller(struct net_device *dev)
 {
-	cvm_oct_napi_poll(NULL, 16);
+	cvm_oct_poll(16);
 }
 #endif
 
-- 
2.9.2

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 5/9] staging: octeon: create a struct for rx group specific data
  2016-08-30 18:47 [PATCH 0/9] staging: octeon: multi rx group (queue) support Aaro Koskinen
                   ` (3 preceding siblings ...)
  2016-08-30 18:47 ` [PATCH 4/9] staging: octeon: move common poll code into a separate function Aaro Koskinen
@ 2016-08-30 18:47 ` Aaro Koskinen
  2016-08-30 18:47 ` [PATCH 6/9] staging: octeon: move irq into " Aaro Koskinen
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 17+ messages in thread
From: Aaro Koskinen @ 2016-08-30 18:47 UTC (permalink / raw)
  To: Greg Kroah-Hartman, David Daney, Ed Swierk, devel
  Cc: linux-kernel, Aaro Koskinen

Create a struct for RX group specific data.

Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi>
---
 drivers/staging/octeon/ethernet-rx.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/octeon/ethernet-rx.c b/drivers/staging/octeon/ethernet-rx.c
index 140e8af..65f6013 100644
--- a/drivers/staging/octeon/ethernet-rx.c
+++ b/drivers/staging/octeon/ethernet-rx.c
@@ -43,7 +43,9 @@
 
 #include <asm/octeon/cvmx-gmxx-defs.h>
 
-static struct napi_struct cvm_oct_napi;
+static struct oct_rx_group {
+	struct napi_struct napi;
+} oct_rx_group;
 
 /**
  * cvm_oct_do_interrupt - interrupt handler.
@@ -455,13 +457,14 @@ void cvm_oct_rx_initialize(void)
 	if (!dev_for_napi)
 		panic("No net_devices were allocated.");
 
-	netif_napi_add(dev_for_napi, &cvm_oct_napi, cvm_oct_napi_poll,
+	netif_napi_add(dev_for_napi, &oct_rx_group.napi, cvm_oct_napi_poll,
 		       rx_napi_weight);
-	napi_enable(&cvm_oct_napi);
+	napi_enable(&oct_rx_group.napi);
 
 	/* Register an IRQ handler to receive POW interrupts */
 	i = request_irq(OCTEON_IRQ_WORKQ0 + pow_receive_group,
-			cvm_oct_do_interrupt, 0, "Ethernet", &cvm_oct_napi);
+			cvm_oct_do_interrupt, 0, "Ethernet",
+			&oct_rx_group.napi);
 
 	if (i)
 		panic("Could not acquire Ethernet IRQ %d\n",
@@ -499,7 +502,7 @@ void cvm_oct_rx_initialize(void)
 	}
 
 	/* Schedule NAPI now. This will indirectly enable the interrupt. */
-	napi_schedule(&cvm_oct_napi);
+	napi_schedule(&oct_rx_group.napi);
 }
 
 void cvm_oct_rx_shutdown(void)
@@ -513,5 +516,5 @@ void cvm_oct_rx_shutdown(void)
 	/* Free the interrupt handler */
 	free_irq(OCTEON_IRQ_WORKQ0 + pow_receive_group, cvm_oct_device);
 
-	netif_napi_del(&cvm_oct_napi);
+	netif_napi_del(&oct_rx_group.napi);
 }
-- 
2.9.2

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 6/9] staging: octeon: move irq into rx group specific data
  2016-08-30 18:47 [PATCH 0/9] staging: octeon: multi rx group (queue) support Aaro Koskinen
                   ` (4 preceding siblings ...)
  2016-08-30 18:47 ` [PATCH 5/9] staging: octeon: create a struct for rx group specific data Aaro Koskinen
@ 2016-08-30 18:47 ` Aaro Koskinen
  2016-08-30 18:47 ` [PATCH 7/9] staging: octeon: move group number into rx group data Aaro Koskinen
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 17+ messages in thread
From: Aaro Koskinen @ 2016-08-30 18:47 UTC (permalink / raw)
  To: Greg Kroah-Hartman, David Daney, Ed Swierk, devel
  Cc: linux-kernel, Aaro Koskinen

Move IRQ number into RX group specific data.

Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi>
---
 drivers/staging/octeon/ethernet-rx.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/octeon/ethernet-rx.c b/drivers/staging/octeon/ethernet-rx.c
index 65f6013..776003c 100644
--- a/drivers/staging/octeon/ethernet-rx.c
+++ b/drivers/staging/octeon/ethernet-rx.c
@@ -44,6 +44,7 @@
 #include <asm/octeon/cvmx-gmxx-defs.h>
 
 static struct oct_rx_group {
+	int irq;
 	struct napi_struct napi;
 } oct_rx_group;
 
@@ -417,6 +418,8 @@ static int cvm_oct_poll(int budget)
  */
 static int cvm_oct_napi_poll(struct napi_struct *napi, int budget)
 {
+	struct oct_rx_group *rx_group = container_of(napi, struct oct_rx_group,
+						     napi);
 	int rx_count;
 
 	rx_count = cvm_oct_poll(budget);
@@ -424,7 +427,7 @@ static int cvm_oct_napi_poll(struct napi_struct *napi, int budget)
 	if (rx_count < budget) {
 		/* No more work */
 		napi_complete(napi);
-		enable_irq(OCTEON_IRQ_WORKQ0 + pow_receive_group);
+		enable_irq(rx_group->irq);
 	}
 	return rx_count;
 }
@@ -461,16 +464,16 @@ void cvm_oct_rx_initialize(void)
 		       rx_napi_weight);
 	napi_enable(&oct_rx_group.napi);
 
+	oct_rx_group.irq = OCTEON_IRQ_WORKQ0 + pow_receive_group;
+
 	/* Register an IRQ handler to receive POW interrupts */
-	i = request_irq(OCTEON_IRQ_WORKQ0 + pow_receive_group,
-			cvm_oct_do_interrupt, 0, "Ethernet",
+	i = request_irq(oct_rx_group.irq, cvm_oct_do_interrupt, 0, "Ethernet",
 			&oct_rx_group.napi);
 
 	if (i)
-		panic("Could not acquire Ethernet IRQ %d\n",
-		      OCTEON_IRQ_WORKQ0 + pow_receive_group);
+		panic("Could not acquire Ethernet IRQ %d\n", oct_rx_group.irq);
 
-	disable_irq_nosync(OCTEON_IRQ_WORKQ0 + pow_receive_group);
+	disable_irq_nosync(oct_rx_group.irq);
 
 	/* Enable POW interrupt when our port has at least one packet */
 	if (OCTEON_IS_MODEL(OCTEON_CN68XX)) {
@@ -514,7 +517,7 @@ void cvm_oct_rx_shutdown(void)
 		cvmx_write_csr(CVMX_POW_WQ_INT_THRX(pow_receive_group), 0);
 
 	/* Free the interrupt handler */
-	free_irq(OCTEON_IRQ_WORKQ0 + pow_receive_group, cvm_oct_device);
+	free_irq(oct_rx_group.irq, cvm_oct_device);
 
 	netif_napi_del(&oct_rx_group.napi);
 }
-- 
2.9.2

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 7/9] staging: octeon: move group number into rx group data
  2016-08-30 18:47 [PATCH 0/9] staging: octeon: multi rx group (queue) support Aaro Koskinen
                   ` (5 preceding siblings ...)
  2016-08-30 18:47 ` [PATCH 6/9] staging: octeon: move irq into " Aaro Koskinen
@ 2016-08-30 18:47 ` Aaro Koskinen
  2016-08-30 18:47 ` [PATCH 8/9] staging: octeon: support enabling multiple rx groups Aaro Koskinen
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 17+ messages in thread
From: Aaro Koskinen @ 2016-08-30 18:47 UTC (permalink / raw)
  To: Greg Kroah-Hartman, David Daney, Ed Swierk, devel
  Cc: linux-kernel, Aaro Koskinen

Move group number into RX group data.

Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi>
---
 drivers/staging/octeon/ethernet-rx.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/octeon/ethernet-rx.c b/drivers/staging/octeon/ethernet-rx.c
index 776003c..668aee6 100644
--- a/drivers/staging/octeon/ethernet-rx.c
+++ b/drivers/staging/octeon/ethernet-rx.c
@@ -45,6 +45,7 @@
 
 static struct oct_rx_group {
 	int irq;
+	int group;
 	struct napi_struct napi;
 } oct_rx_group;
 
@@ -146,7 +147,7 @@ static inline int cvm_oct_check_rcv_error(cvmx_wqe_t *work)
 	return 0;
 }
 
-static int cvm_oct_poll(int budget)
+static int cvm_oct_poll(struct oct_rx_group *rx_group, int budget)
 {
 	const int	coreid = cvmx_get_core_num();
 	u64	old_group_mask;
@@ -168,13 +169,13 @@ static int cvm_oct_poll(int budget)
 	if (OCTEON_IS_MODEL(OCTEON_CN68XX)) {
 		old_group_mask = cvmx_read_csr(CVMX_SSO_PPX_GRP_MSK(coreid));
 		cvmx_write_csr(CVMX_SSO_PPX_GRP_MSK(coreid),
-			       1ull << pow_receive_group);
+			       BIT(rx_group->group));
 		cvmx_read_csr(CVMX_SSO_PPX_GRP_MSK(coreid)); /* Flush */
 	} else {
 		old_group_mask = cvmx_read_csr(CVMX_POW_PP_GRP_MSKX(coreid));
 		cvmx_write_csr(CVMX_POW_PP_GRP_MSKX(coreid),
 			       (old_group_mask & ~0xFFFFull) |
-			       1 << pow_receive_group);
+			       BIT(rx_group->group));
 	}
 
 	if (USE_ASYNC_IOBDMA) {
@@ -199,15 +200,15 @@ static int cvm_oct_poll(int budget)
 		if (!work) {
 			if (OCTEON_IS_MODEL(OCTEON_CN68XX)) {
 				cvmx_write_csr(CVMX_SSO_WQ_IQ_DIS,
-					       1ull << pow_receive_group);
+					       BIT(rx_group->group));
 				cvmx_write_csr(CVMX_SSO_WQ_INT,
-					       1ull << pow_receive_group);
+					       BIT(rx_group->group));
 			} else {
 				union cvmx_pow_wq_int wq_int;
 
 				wq_int.u64 = 0;
-				wq_int.s.iq_dis = 1 << pow_receive_group;
-				wq_int.s.wq_int = 1 << pow_receive_group;
+				wq_int.s.iq_dis = BIT(rx_group->group);
+				wq_int.s.wq_int = BIT(rx_group->group);
 				cvmx_write_csr(CVMX_POW_WQ_INT, wq_int.u64);
 			}
 			break;
@@ -422,7 +423,7 @@ static int cvm_oct_napi_poll(struct napi_struct *napi, int budget)
 						     napi);
 	int rx_count;
 
-	rx_count = cvm_oct_poll(budget);
+	rx_count = cvm_oct_poll(rx_group, budget);
 
 	if (rx_count < budget) {
 		/* No more work */
@@ -441,7 +442,7 @@ static int cvm_oct_napi_poll(struct napi_struct *napi, int budget)
  */
 void cvm_oct_poll_controller(struct net_device *dev)
 {
-	cvm_oct_poll(16);
+	cvm_oct_poll(oct_rx_group, 16);
 }
 #endif
 
@@ -465,6 +466,7 @@ void cvm_oct_rx_initialize(void)
 	napi_enable(&oct_rx_group.napi);
 
 	oct_rx_group.irq = OCTEON_IRQ_WORKQ0 + pow_receive_group;
+	oct_rx_group.group = pow_receive_group;
 
 	/* Register an IRQ handler to receive POW interrupts */
 	i = request_irq(oct_rx_group.irq, cvm_oct_do_interrupt, 0, "Ethernet",
-- 
2.9.2

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 8/9] staging: octeon: support enabling multiple rx groups
  2016-08-30 18:47 [PATCH 0/9] staging: octeon: multi rx group (queue) support Aaro Koskinen
                   ` (6 preceding siblings ...)
  2016-08-30 18:47 ` [PATCH 7/9] staging: octeon: move group number into rx group data Aaro Koskinen
@ 2016-08-30 18:47 ` Aaro Koskinen
  2016-08-30 18:47 ` [PATCH 9/9] staging: octeon: enable taking multiple rx groups into use Aaro Koskinen
  2016-08-31  1:12 ` [PATCH 0/9] staging: octeon: multi rx group (queue) support Ed Swierk
  9 siblings, 0 replies; 17+ messages in thread
From: Aaro Koskinen @ 2016-08-30 18:47 UTC (permalink / raw)
  To: Greg Kroah-Hartman, David Daney, Ed Swierk, devel
  Cc: linux-kernel, Aaro Koskinen

Support enabling multiple RX groups.

Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi>
---
 drivers/staging/octeon/ethernet-rx.c     | 126 ++++++++++++++++++-------------
 drivers/staging/octeon/ethernet.c        |   6 +-
 drivers/staging/octeon/octeon-ethernet.h |   2 +-
 3 files changed, 81 insertions(+), 53 deletions(-)

diff --git a/drivers/staging/octeon/ethernet-rx.c b/drivers/staging/octeon/ethernet-rx.c
index 668aee6..407020f 100644
--- a/drivers/staging/octeon/ethernet-rx.c
+++ b/drivers/staging/octeon/ethernet-rx.c
@@ -47,7 +47,7 @@ static struct oct_rx_group {
 	int irq;
 	int group;
 	struct napi_struct napi;
-} oct_rx_group;
+} oct_rx_group[16];
 
 /**
  * cvm_oct_do_interrupt - interrupt handler.
@@ -442,7 +442,16 @@ static int cvm_oct_napi_poll(struct napi_struct *napi, int budget)
  */
 void cvm_oct_poll_controller(struct net_device *dev)
 {
-	cvm_oct_poll(oct_rx_group, 16);
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(oct_rx_group); i++) {
+
+		if (!(pow_receive_groups & BIT(i)))
+			continue;
+
+		cvm_oct_poll(oct_rx_group[i], 16);
+
+	}
 }
 #endif
 
@@ -461,65 +470,80 @@ void cvm_oct_rx_initialize(void)
 	if (!dev_for_napi)
 		panic("No net_devices were allocated.");
 
-	netif_napi_add(dev_for_napi, &oct_rx_group.napi, cvm_oct_napi_poll,
-		       rx_napi_weight);
-	napi_enable(&oct_rx_group.napi);
+	for (i = 0; i < ARRAY_SIZE(oct_rx_group); i++) {
+		int ret;
 
-	oct_rx_group.irq = OCTEON_IRQ_WORKQ0 + pow_receive_group;
-	oct_rx_group.group = pow_receive_group;
+		if (!(pow_receive_groups & BIT(i)))
+			continue;
 
-	/* Register an IRQ handler to receive POW interrupts */
-	i = request_irq(oct_rx_group.irq, cvm_oct_do_interrupt, 0, "Ethernet",
-			&oct_rx_group.napi);
+		netif_napi_add(dev_for_napi, &oct_rx_group[i].napi,
+			       cvm_oct_napi_poll, rx_napi_weight);
+		napi_enable(&oct_rx_group[i].napi);
 
-	if (i)
-		panic("Could not acquire Ethernet IRQ %d\n", oct_rx_group.irq);
+		oct_rx_group[i].irq = OCTEON_IRQ_WORKQ0 + i;
+		oct_rx_group[i].group = i;
 
-	disable_irq_nosync(oct_rx_group.irq);
+		/* Register an IRQ handler to receive POW interrupts */
+		ret = request_irq(oct_rx_group[i].irq, cvm_oct_do_interrupt, 0,
+				  "Ethernet", &oct_rx_group[i].napi);
+		if (ret)
+			panic("Could not acquire Ethernet IRQ %d\n",
+			      oct_rx_group[i].irq);
 
-	/* Enable POW interrupt when our port has at least one packet */
-	if (OCTEON_IS_MODEL(OCTEON_CN68XX)) {
-		union cvmx_sso_wq_int_thrx int_thr;
-		union cvmx_pow_wq_int_pc int_pc;
-
-		int_thr.u64 = 0;
-		int_thr.s.tc_en = 1;
-		int_thr.s.tc_thr = 1;
-		cvmx_write_csr(CVMX_SSO_WQ_INT_THRX(pow_receive_group),
-			       int_thr.u64);
-
-		int_pc.u64 = 0;
-		int_pc.s.pc_thr = 5;
-		cvmx_write_csr(CVMX_SSO_WQ_INT_PC, int_pc.u64);
-	} else {
-		union cvmx_pow_wq_int_thrx int_thr;
-		union cvmx_pow_wq_int_pc int_pc;
-
-		int_thr.u64 = 0;
-		int_thr.s.tc_en = 1;
-		int_thr.s.tc_thr = 1;
-		cvmx_write_csr(CVMX_POW_WQ_INT_THRX(pow_receive_group),
-			       int_thr.u64);
-
-		int_pc.u64 = 0;
-		int_pc.s.pc_thr = 5;
-		cvmx_write_csr(CVMX_POW_WQ_INT_PC, int_pc.u64);
-	}
+		disable_irq_nosync(oct_rx_group[i].irq);
+
+		/* Enable POW interrupt when our port has at least one packet */
+		if (OCTEON_IS_MODEL(OCTEON_CN68XX)) {
+			union cvmx_sso_wq_int_thrx int_thr;
+			union cvmx_pow_wq_int_pc int_pc;
+
+			int_thr.u64 = 0;
+			int_thr.s.tc_en = 1;
+			int_thr.s.tc_thr = 1;
+			cvmx_write_csr(CVMX_SSO_WQ_INT_THRX(i), int_thr.u64);
+
+			int_pc.u64 = 0;
+			int_pc.s.pc_thr = 5;
+			cvmx_write_csr(CVMX_SSO_WQ_INT_PC, int_pc.u64);
+		} else {
+			union cvmx_pow_wq_int_thrx int_thr;
+			union cvmx_pow_wq_int_pc int_pc;
 
-	/* Schedule NAPI now. This will indirectly enable the interrupt. */
-	napi_schedule(&oct_rx_group.napi);
+			int_thr.u64 = 0;
+			int_thr.s.tc_en = 1;
+			int_thr.s.tc_thr = 1;
+			cvmx_write_csr(CVMX_POW_WQ_INT_THRX(i), int_thr.u64);
+
+			int_pc.u64 = 0;
+			int_pc.s.pc_thr = 5;
+			cvmx_write_csr(CVMX_POW_WQ_INT_PC, int_pc.u64);
+		}
+
+		/* Schedule NAPI now. This will indirectly enable the
+		 * interrupt.
+		 */
+		napi_schedule(&oct_rx_group[i].napi);
+	}
 }
 
 void cvm_oct_rx_shutdown(void)
 {
-	/* Disable POW interrupt */
-	if (OCTEON_IS_MODEL(OCTEON_CN68XX))
-		cvmx_write_csr(CVMX_SSO_WQ_INT_THRX(pow_receive_group), 0);
-	else
-		cvmx_write_csr(CVMX_POW_WQ_INT_THRX(pow_receive_group), 0);
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(oct_rx_group); i++) {
+
+		if (!(pow_receive_groups & BIT(i)))
+			continue;
 
-	/* Free the interrupt handler */
-	free_irq(oct_rx_group.irq, cvm_oct_device);
+		/* Disable POW interrupt */
+		if (OCTEON_IS_MODEL(OCTEON_CN68XX))
+			cvmx_write_csr(CVMX_SSO_WQ_INT_THRX(i), 0);
+		else
+			cvmx_write_csr(CVMX_POW_WQ_INT_THRX(i), 0);
+
+		/* Free the interrupt handler */
+		free_irq(oct_rx_group[i].irq, cvm_oct_device);
 
-	netif_napi_del(&oct_rx_group.napi);
+		netif_napi_del(&oct_rx_group[i].napi);
+	}
 }
diff --git a/drivers/staging/octeon/ethernet.c b/drivers/staging/octeon/ethernet.c
index 1e2e1ef..7d48745 100644
--- a/drivers/staging/octeon/ethernet.c
+++ b/drivers/staging/octeon/ethernet.c
@@ -45,7 +45,7 @@ MODULE_PARM_DESC(num_packet_buffers, "\n"
 	"\tNumber of packet buffers to allocate and store in the\n"
 	"\tFPA. By default, 1024 packet buffers are used.\n");
 
-int pow_receive_group = 15;
+static int pow_receive_group = 15;
 module_param(pow_receive_group, int, 0444);
 MODULE_PARM_DESC(pow_receive_group, "\n"
 	"\tPOW group to receive packets from. All ethernet hardware\n"
@@ -86,6 +86,8 @@ int rx_napi_weight = 32;
 module_param(rx_napi_weight, int, 0444);
 MODULE_PARM_DESC(rx_napi_weight, "The NAPI WEIGHT parameter.");
 
+/* Mask indicating which receive groups are in use. */
+int pow_receive_groups;
 
 /*
  * cvm_oct_poll_queue_stopping - flag to indicate polling should stop.
@@ -678,6 +680,8 @@ static int cvm_oct_probe(struct platform_device *pdev)
 
 	cvmx_helper_initialize_packet_io_global();
 
+	pow_receive_groups = BIT(pow_receive_group);
+
 	/* Change the input group for all ports before input is enabled */
 	num_interfaces = cvmx_helper_get_number_of_interfaces();
 	for (interface = 0; interface < num_interfaces; interface++) {
diff --git a/drivers/staging/octeon/octeon-ethernet.h b/drivers/staging/octeon/octeon-ethernet.h
index d533aef..9c6852d 100644
--- a/drivers/staging/octeon/octeon-ethernet.h
+++ b/drivers/staging/octeon/octeon-ethernet.h
@@ -72,7 +72,7 @@ void cvm_oct_link_poll(struct net_device *dev);
 
 extern int always_use_pow;
 extern int pow_send_group;
-extern int pow_receive_group;
+extern int pow_receive_groups;
 extern char pow_send_list[];
 extern struct net_device *cvm_oct_device[];
 extern atomic_t cvm_oct_poll_queue_stopping;
-- 
2.9.2

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 9/9] staging: octeon: enable taking multiple rx groups into use
  2016-08-30 18:47 [PATCH 0/9] staging: octeon: multi rx group (queue) support Aaro Koskinen
                   ` (7 preceding siblings ...)
  2016-08-30 18:47 ` [PATCH 8/9] staging: octeon: support enabling multiple rx groups Aaro Koskinen
@ 2016-08-30 18:47 ` Aaro Koskinen
  2016-08-31  1:12 ` [PATCH 0/9] staging: octeon: multi rx group (queue) support Ed Swierk
  9 siblings, 0 replies; 17+ messages in thread
From: Aaro Koskinen @ 2016-08-30 18:47 UTC (permalink / raw)
  To: Greg Kroah-Hartman, David Daney, Ed Swierk, devel
  Cc: linux-kernel, Aaro Koskinen

Enable taking multiple RX groups into use.

Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi>
---
 drivers/staging/octeon/ethernet.c | 42 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 40 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/octeon/ethernet.c b/drivers/staging/octeon/ethernet.c
index 7d48745..c1d0cfd 100644
--- a/drivers/staging/octeon/ethernet.c
+++ b/drivers/staging/octeon/ethernet.c
@@ -53,6 +53,15 @@ MODULE_PARM_DESC(pow_receive_group, "\n"
 	"\tgroup. Also any other software can submit packets to this\n"
 	"\tgroup for the kernel to process.");
 
+static int receive_group_order;
+module_param(receive_group_order, int, 0444);
+MODULE_PARM_DESC(receive_group_order, "\n"
+	"\tOrder (0..4) of receive groups to take into use. Ethernet hardware\n"
+	"\twill be configured to send incoming packets to multiple POW\n"
+	"\tgroups. pow_receive_group parameter is ignored when multiple\n"
+	"\tgroups are taken into use and groups are allocated starting\n"
+	"\tfrom 0. By default, a single group is used.\n");
+
 int pow_send_group = -1;
 module_param(pow_send_group, int, 0644);
 MODULE_PARM_DESC(pow_send_group, "\n"
@@ -680,7 +689,13 @@ static int cvm_oct_probe(struct platform_device *pdev)
 
 	cvmx_helper_initialize_packet_io_global();
 
-	pow_receive_groups = BIT(pow_receive_group);
+	if (receive_group_order) {
+		if (receive_group_order > 4)
+			receive_group_order = 4;
+		pow_receive_groups = (1 << (1 << receive_group_order)) - 1;
+	} else {
+		pow_receive_groups = BIT(pow_receive_group);
+	}
 
 	/* Change the input group for all ports before input is enabled */
 	num_interfaces = cvmx_helper_get_number_of_interfaces();
@@ -695,7 +710,30 @@ static int cvm_oct_probe(struct platform_device *pdev)
 
 			pip_prt_tagx.u64 =
 			    cvmx_read_csr(CVMX_PIP_PRT_TAGX(port));
-			pip_prt_tagx.s.grp = pow_receive_group;
+
+			if (receive_group_order) {
+				int tag_mask;
+
+				tag_mask = ~((1 << receive_group_order) - 1);
+				pip_prt_tagx.s.grptagbase	= 0;
+				pip_prt_tagx.s.grptagmask	= tag_mask;
+				pip_prt_tagx.s.grptag		= 1;
+				pip_prt_tagx.s.tag_mode		= 0;
+				pip_prt_tagx.s.inc_prt_flag	= 1;
+				pip_prt_tagx.s.ip6_dprt_flag	= 1;
+				pip_prt_tagx.s.ip4_dprt_flag	= 1;
+				pip_prt_tagx.s.ip6_sprt_flag	= 1;
+				pip_prt_tagx.s.ip4_sprt_flag	= 1;
+				pip_prt_tagx.s.ip6_dst_flag	= 1;
+				pip_prt_tagx.s.ip4_dst_flag	= 1;
+				pip_prt_tagx.s.ip6_src_flag	= 1;
+				pip_prt_tagx.s.ip4_src_flag	= 1;
+				pip_prt_tagx.s.grp		= 0;
+			} else {
+				pip_prt_tagx.s.grptag	= 0;
+				pip_prt_tagx.s.grp	= pow_receive_group;
+			}
+
 			cvmx_write_csr(CVMX_PIP_PRT_TAGX(port),
 				       pip_prt_tagx.u64);
 		}
-- 
2.9.2

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 0/9] staging: octeon: multi rx group (queue) support
  2016-08-30 18:47 [PATCH 0/9] staging: octeon: multi rx group (queue) support Aaro Koskinen
                   ` (8 preceding siblings ...)
  2016-08-30 18:47 ` [PATCH 9/9] staging: octeon: enable taking multiple rx groups into use Aaro Koskinen
@ 2016-08-31  1:12 ` Ed Swierk
  2016-08-31  6:29   ` Aaro Koskinen
  2016-08-31 15:06   ` Aaro Koskinen
  9 siblings, 2 replies; 17+ messages in thread
From: Ed Swierk @ 2016-08-31  1:12 UTC (permalink / raw)
  To: Aaro Koskinen; +Cc: Greg Kroah-Hartman, David Daney, driverdev-devel, lkml

Hi Aaro,

On Tue, Aug 30, 2016 at 11:47 AM, Aaro Koskinen <aaro.koskinen@iki.fi> wrote:
> This series implements multiple RX group support that should improve
> the networking performance on multi-core OCTEONs. Basically we register
> IRQ and NAPI for each group, and ask the HW to select the group for
> the incoming packets based on hash.
>
> Tested on EdgeRouter Lite with a simple forwarding test using two flows
> and 16 RX groups distributed between two cores - the routing throughput
> is roughly doubled.

I applied the series to my 4.4.19 tree, which involved backporting a
bunch of other patches from master, most of them trivial.

When I test it on a Cavium Octeon 2 (CN6880) board, I get an immediate
crash (bus error) in the netif_receive_skb() call from cvm_oct_poll().
Replacing the rx_group argument to cvm_oct_poll() with int group, and
dereferencing rx_group->group in the caller (cvm_oct_napi_poll())
instead makes the crash disappear. Apparently there's some race in
dereferencing rx_group from within cvm_oct_poll().

With this workaround in place, I can send and receive on XAUI
interfaces, but don't see any performance improvement. I'm guessing I
need to set receive_group_order > 0. But any value between 1 and 4
seems to break rx altogether. When I ping another host I see both
request and response on the wire, and the interface counters increase,
but the response doesn't make it back to ping.

Is some other configuration needed to make use of multiple rx groups?

--Ed

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 0/9] staging: octeon: multi rx group (queue) support
  2016-08-31  1:12 ` [PATCH 0/9] staging: octeon: multi rx group (queue) support Ed Swierk
@ 2016-08-31  6:29   ` Aaro Koskinen
  2016-08-31 16:20     ` Ed Swierk
  2016-08-31 15:06   ` Aaro Koskinen
  1 sibling, 1 reply; 17+ messages in thread
From: Aaro Koskinen @ 2016-08-31  6:29 UTC (permalink / raw)
  To: Ed Swierk; +Cc: Greg Kroah-Hartman, David Daney, driverdev-devel, lkml

Hi,

On Tue, Aug 30, 2016 at 06:12:17PM -0700, Ed Swierk wrote:
> On Tue, Aug 30, 2016 at 11:47 AM, Aaro Koskinen <aaro.koskinen@iki.fi> wrote:
> > This series implements multiple RX group support that should improve
> > the networking performance on multi-core OCTEONs. Basically we register
> > IRQ and NAPI for each group, and ask the HW to select the group for
> > the incoming packets based on hash.
> >
> > Tested on EdgeRouter Lite with a simple forwarding test using two flows
> > and 16 RX groups distributed between two cores - the routing throughput
> > is roughly doubled.
> 
> I applied the series to my 4.4.19 tree, which involved backporting a
> bunch of other patches from master, most of them trivial.
> 
> When I test it on a Cavium Octeon 2 (CN6880) board, I get an immediate
> crash (bus error) in the netif_receive_skb() call from cvm_oct_poll().
> Replacing the rx_group argument to cvm_oct_poll() with int group, and
> dereferencing rx_group->group in the caller (cvm_oct_napi_poll())
> instead makes the crash disappear. Apparently there's some race in
> dereferencing rx_group from within cvm_oct_poll().

Oops, looks like I tested without CONFIG_NET_POLL_CONTROLLER enabled
and that seems to be broken. Sorry.

> With this workaround in place, I can send and receive on XAUI
> interfaces, but don't see any performance improvement. I'm guessing I
> need to set receive_group_order > 0. But any value between 1 and 4
> seems to break rx altogether. When I ping another host I see both
> request and response on the wire, and the interface counters increase,
> but the response doesn't make it back to ping.

Can you see multiple ethernet IRQs in /proc/interrupts and their
counters increasing?

With receive_group_order=4 you should see 16 IRQs.

> Is some other configuration needed to make use of multiple rx groups?

Once RX interrupts are working you need to divide them to multiple cores
using /proc/irq/<number>/smp_affinity, or use irqbalance or such.

A.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 0/9] staging: octeon: multi rx group (queue) support
  2016-08-31  1:12 ` [PATCH 0/9] staging: octeon: multi rx group (queue) support Ed Swierk
  2016-08-31  6:29   ` Aaro Koskinen
@ 2016-08-31 15:06   ` Aaro Koskinen
  2016-08-31 16:10     ` David Daney
  1 sibling, 1 reply; 17+ messages in thread
From: Aaro Koskinen @ 2016-08-31 15:06 UTC (permalink / raw)
  To: Ed Swierk; +Cc: driverdev-devel, Greg Kroah-Hartman, David Daney, lkml

Hi,

On Tue, Aug 30, 2016 at 06:12:17PM -0700, Ed Swierk wrote:
> On Tue, Aug 30, 2016 at 11:47 AM, Aaro Koskinen <aaro.koskinen@iki.fi> wrote:
> > This series implements multiple RX group support that should improve
> > the networking performance on multi-core OCTEONs. Basically we register
> > IRQ and NAPI for each group, and ask the HW to select the group for
> > the incoming packets based on hash.
> >
> > Tested on EdgeRouter Lite with a simple forwarding test using two flows
> > and 16 RX groups distributed between two cores - the routing throughput
> > is roughly doubled.
> 
> I applied the series to my 4.4.19 tree, which involved backporting a
> bunch of other patches from master, most of them trivial.
> 
> When I test it on a Cavium Octeon 2 (CN6880) board, I get an immediate
> crash (bus error) in the netif_receive_skb() call from cvm_oct_poll().
> Replacing the rx_group argument to cvm_oct_poll() with int group, and
> dereferencing rx_group->group in the caller (cvm_oct_napi_poll())
> instead makes the crash disappear. Apparently there's some race in
> dereferencing rx_group from within cvm_oct_poll().
> 
> With this workaround in place, I can send and receive on XAUI
> interfaces, but don't see any performance improvement. I'm guessing I
> need to set receive_group_order > 0. But any value between 1 and 4
> seems to break rx altogether. When I ping another host I see both
> request and response on the wire, and the interface counters increase,
> but the response doesn't make it back to ping.

This happens only on CN68XX, and I found the root cause.

I will send a new series later today.

A.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 0/9] staging: octeon: multi rx group (queue) support
  2016-08-31 15:06   ` Aaro Koskinen
@ 2016-08-31 16:10     ` David Daney
  0 siblings, 0 replies; 17+ messages in thread
From: David Daney @ 2016-08-31 16:10 UTC (permalink / raw)
  To: Aaro Koskinen; +Cc: Ed Swierk, driverdev-devel, Greg Kroah-Hartman, lkml

On 08/31/2016 08:06 AM, Aaro Koskinen wrote:
> Hi,
>
> On Tue, Aug 30, 2016 at 06:12:17PM -0700, Ed Swierk wrote:
>> On Tue, Aug 30, 2016 at 11:47 AM, Aaro Koskinen <aaro.koskinen@iki.fi> wrote:
>>> This series implements multiple RX group support that should improve
>>> the networking performance on multi-core OCTEONs. Basically we register
>>> IRQ and NAPI for each group, and ask the HW to select the group for
>>> the incoming packets based on hash.
>>>
>>> Tested on EdgeRouter Lite with a simple forwarding test using two flows
>>> and 16 RX groups distributed between two cores - the routing throughput
>>> is roughly doubled.
>>
>> I applied the series to my 4.4.19 tree, which involved backporting a
>> bunch of other patches from master, most of them trivial.
>>
>> When I test it on a Cavium Octeon 2 (CN6880) board, I get an immediate
>> crash (bus error) in the netif_receive_skb() call from cvm_oct_poll().
>> Replacing the rx_group argument to cvm_oct_poll() with int group, and
>> dereferencing rx_group->group in the caller (cvm_oct_napi_poll())
>> instead makes the crash disappear. Apparently there's some race in
>> dereferencing rx_group from within cvm_oct_poll().
>>
>> With this workaround in place, I can send and receive on XAUI
>> interfaces, but don't see any performance improvement. I'm guessing I
>> need to set receive_group_order > 0. But any value between 1 and 4
>> seems to break rx altogether. When I ping another host I see both
>> request and response on the wire, and the interface counters increase,
>> but the response doesn't make it back to ping.
>
> This happens only on CN68XX, and I found the root cause.
>
> I will send a new series later today.

Thanks,

cn68xx does have a slightly different SSO and interrupt implementation, 
so it will probably need some special case code to handle both styles of 
hardware.

David.

>
> A.
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 0/9] staging: octeon: multi rx group (queue) support
  2016-08-31  6:29   ` Aaro Koskinen
@ 2016-08-31 16:20     ` Ed Swierk
  2016-08-31 21:20       ` Aaro Koskinen
  0 siblings, 1 reply; 17+ messages in thread
From: Ed Swierk @ 2016-08-31 16:20 UTC (permalink / raw)
  To: Aaro Koskinen; +Cc: Greg Kroah-Hartman, David Daney, driverdev-devel, lkml

Aaro Koskinen wrote:
> Oops, looks like I tested without CONFIG_NET_POLL_CONTROLLER enabled
> and that seems to be broken. Sorry.

I'm not using CONFIG_NET_POLL_CONTROLLER either; the problem is in the
normal cvm_oct_napi_poll() path.

Here's my workaround:

--- a/drivers/staging/octeon/ethernet-rx.c
+++ b/drivers/staging/octeon/ethernet-rx.c
@@ -159,7 +159,7 @@ static inline int cvm_oct_check_rcv_error(cvmx_wqe_t *work)
 	return 0;
 }

-static int cvm_oct_poll(struct oct_rx_group *rx_group, int budget)
+static int cvm_oct_poll(int group, int budget)
 {
 	const int	coreid = cvmx_get_core_num();
 	u64	old_group_mask;
@@ -181,13 +181,13 @@ static int cvm_oct_poll(struct oct_rx_group *rx_group, int budget)
 	if (OCTEON_IS_MODEL(OCTEON_CN68XX)) {
 		old_group_mask = cvmx_read_csr(CVMX_SSO_PPX_GRP_MSK(coreid));
 		cvmx_write_csr(CVMX_SSO_PPX_GRP_MSK(coreid),
-			       BIT(rx_group->group));
+			       BIT(group));
 		cvmx_read_csr(CVMX_SSO_PPX_GRP_MSK(coreid)); /* Flush */
 	} else {
 		old_group_mask = cvmx_read_csr(CVMX_POW_PP_GRP_MSKX(coreid));
 		cvmx_write_csr(CVMX_POW_PP_GRP_MSKX(coreid),
 			       (old_group_mask & ~0xFFFFull) |
-			       BIT(rx_group->group));
+			       BIT(group));
 	}

 	if (USE_ASYNC_IOBDMA) {
@@ -212,15 +212,15 @@ static int cvm_oct_poll(struct oct_rx_group *rx_group, int budget)
 		if (!work) {
 			if (OCTEON_IS_MODEL(OCTEON_CN68XX)) {
 				cvmx_write_csr(CVMX_SSO_WQ_IQ_DIS,
-					       BIT(rx_group->group));
+					       BIT(group));
 				cvmx_write_csr(CVMX_SSO_WQ_INT,
-					       BIT(rx_group->group));
+					       BIT(group));
 			} else {
 				union cvmx_pow_wq_int wq_int;

 				wq_int.u64 = 0;
-				wq_int.s.iq_dis = BIT(rx_group->group);
-				wq_int.s.wq_int = BIT(rx_group->group);
+				wq_int.s.iq_dis = BIT(group);
+				wq_int.s.wq_int = BIT(group);
 				cvmx_write_csr(CVMX_POW_WQ_INT, wq_int.u64);
 			}
 			break;
@@ -447,7 +447,7 @@ static int cvm_oct_napi_poll(struct napi_struct *napi, int budget)
 						     napi);
 	int rx_count;

-	rx_count = cvm_oct_poll(rx_group, budget);
+	rx_count = cvm_oct_poll(rx_group->group, budget);

 	if (rx_count < budget) {
 		/* No more work */
@@ -466,7 +466,7 @@ static int cvm_oct_napi_poll(struct napi_struct *napi, int budget)
  */
 void cvm_oct_poll_controller(struct net_device *dev)
 {
-	cvm_oct_poll(oct_rx_group, 16);
+	cvm_oct_poll(oct_rx_group->group, 16);
 }
 #endif

> Can you see multiple ethernet IRQs in /proc/interrupts and their
> counters increasing?
> 
> With receive_group_order=4 you should see 16 IRQs.

I see the 16 IRQs, and the first one does increase. But packets don't make
it to the application.

--Ed

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 0/9] staging: octeon: multi rx group (queue) support
  2016-08-31 16:20     ` Ed Swierk
@ 2016-08-31 21:20       ` Aaro Koskinen
  2016-09-01  1:52         ` Ed Swierk
  0 siblings, 1 reply; 17+ messages in thread
From: Aaro Koskinen @ 2016-08-31 21:20 UTC (permalink / raw)
  To: Ed Swierk, David Daney; +Cc: driverdev-devel, Greg Kroah-Hartman, lkml

Hi,

On Wed, Aug 31, 2016 at 09:20:07AM -0700, Ed Swierk wrote:
> I'm not using CONFIG_NET_POLL_CONTROLLER either; the problem is in the
> normal cvm_oct_napi_poll() path.
> 
> Here's my workaround:

[...]

> -static int cvm_oct_poll(struct oct_rx_group *rx_group, int budget)
> +static int cvm_oct_poll(int group, int budget)
>  {
>  	const int	coreid = cvmx_get_core_num();
>  	u64	old_group_mask;
> @@ -181,13 +181,13 @@ static int cvm_oct_poll(struct oct_rx_group *rx_group, int budget)
>  	if (OCTEON_IS_MODEL(OCTEON_CN68XX)) {
>  		old_group_mask = cvmx_read_csr(CVMX_SSO_PPX_GRP_MSK(coreid));
>  		cvmx_write_csr(CVMX_SSO_PPX_GRP_MSK(coreid),
> -			       BIT(rx_group->group));
> +			       BIT(group));
> @@ -447,7 +447,7 @@ static int cvm_oct_napi_poll(struct napi_struct *napi, int budget)
>  						     napi);
>  	int rx_count;
> 
> -	rx_count = cvm_oct_poll(rx_group, budget);
> +	rx_count = cvm_oct_poll(rx_group->group, budget);

I'm confused - there should be no difference?!

> > Can you see multiple ethernet IRQs in /proc/interrupts and their
> > counters increasing?
> > 
> > With receive_group_order=4 you should see 16 IRQs.
> 
> I see the 16 IRQs, and the first one does increase. But packets don't make
> it to the application.

Yeah, turns out that CN68XX supports up to 64 receive groups, and the
reset value is such that up to 64 groups get enabled by default in the
tag mask unless we know how to disabled them. So probably your packets
end up in those 48 other groups that do not have handler. This should
be fixed in v2 (by limiting to 16).

A.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 0/9] staging: octeon: multi rx group (queue) support
  2016-08-31 21:20       ` Aaro Koskinen
@ 2016-09-01  1:52         ` Ed Swierk
  0 siblings, 0 replies; 17+ messages in thread
From: Ed Swierk @ 2016-09-01  1:52 UTC (permalink / raw)
  To: Aaro Koskinen, David Daney; +Cc: driverdev-devel, Greg Kroah-Hartman, lkml

On 8/31/16 14:20, Aaro Koskinen wrote:
> On Wed, Aug 31, 2016 at 09:20:07AM -0700, Ed Swierk wrote:
>> Here's my workaround:
> 
> [...]
> 
>> -static int cvm_oct_poll(struct oct_rx_group *rx_group, int budget)
>> +static int cvm_oct_poll(int group, int budget)
>>  {
>>  	const int	coreid = cvmx_get_core_num();
>>  	u64	old_group_mask;
>> @@ -181,13 +181,13 @@ static int cvm_oct_poll(struct oct_rx_group *rx_group, int budget)
>>  	if (OCTEON_IS_MODEL(OCTEON_CN68XX)) {
>>  		old_group_mask = cvmx_read_csr(CVMX_SSO_PPX_GRP_MSK(coreid));
>>  		cvmx_write_csr(CVMX_SSO_PPX_GRP_MSK(coreid),
>> -			       BIT(rx_group->group));
>> +			       BIT(group));
>> @@ -447,7 +447,7 @@ static int cvm_oct_napi_poll(struct napi_struct *napi, int budget)
>>  						     napi);
>>  	int rx_count;
>>
>> -	rx_count = cvm_oct_poll(rx_group, budget);
>> +	rx_count = cvm_oct_poll(rx_group->group, budget);
> 
> I'm confused - there should be no difference?!

I can't figure out the difference either. I get a crash within the first
couple packets, while with the workaround I can't get it to crash at all.
It always bombs in netif_receive_skb(), which isn't very close to any
rx_group pointer dereference.

# ping 172.16.100.253
PING 172.16.100.253 (172.16.100.253): 56 data bytes
Data bus error, epc == ffffffff803fd4ac, ra == ffffffff801943d8
Oops[#1]:
CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.4.19+ #94
task: ffffffff80863e80 ti: ffffffff80840000 task.ti: ffffffff80840000
$ 0   : 0000000000000000 ffffffff80126078 ef7bdef7bdef7bdf ffffffff815d3860
$ 4   : ffffffff80e045c8 ffffffff81aae950 ffffffff81aae950 0000000000000000
$ 8   : ffffffff81aae950 0000000000000038 0000000000000070 0000000003bf0000
$12   : 0000000054000000 0000000003bd0000 0000000000000000 0000000000000000
$16   : ffffffff81aae950 ffffffff81aae950 ffffffff80e045c8 0000000000000000
$20   : 00000000000000fa 0000000000000001 000000005c02e0fa 0000000000000000
$24   : 0000000000000062 ffffffff80548468                                  
$28   : ffffffff80840000 ffffffff808436d0 ffffffff80feba38 ffffffff801943d8
Hi    : 0000000000000000
Lo    : 05198e3760c00000
epc   : ffffffff803fd4ac __list_add_rcu+0x7c/0xa0
ra    : ffffffff801943d8 __lock_acquire+0xd94/0x1bf0
Status: 10008ce2	KX SX UX KERNEL EXL 
Cause : 40808c1c (ExcCode 07)
PrId  : 000d910a (Cavium Octeon II)
Modules linked in:
Process swapper/0 (pid: 0, threadinfo=ffffffff80840000, task=ffffffff80863e80, tls=0000000000000000)
Stack : ffffffff80863e80 ffffffff808646c8 ffffffff81aae950 ffffffff801943d8
	  00000000000000fa ffffffff808646c0 0000000000000000 0000000000000002
	  0000000000000000 ffffffff8057ab90 ffffffff80864690 ffffffff80870990
	  0000000000000001 0000000000000000 0000000000000000 0000000000000017
	  0000000000000000 ffffffff80193e08 0000000000000017 ffffffff80864688
	  0000000000000001 ffffffff8057ab90 ffffffff808a7d28 800000007f4b7500
	  800000007a0b52e8 0000000000000001 ffffffff807f0000 800000007f768068
	  ffffffff8085fac8 ffffffff8019568c 0000000000000000 0000000000000000
	  ffffffff808a7d10 ffffffff80645e60 800000007f4a8600 0000000000000254
	  ffffffff808a7d58 ffffffff8057ab90 0000000000000008 800000007f7680a0
	  ...
Call Trace:
[<__list_add_rcu at list_debug.c:97 (discriminator 2)>] __list_add_rcu+0x7c/0xa0
[<inc_chains at lockdep.c:1683
 (inlined by) lookup_chain_cache at lockdep.c:2096
 (inlined by) validate_chain at lockdep.c:2115
 (inlined by) __lock_acquire at lockdep.c:3206>] __lock_acquire+0xd94/0x1bf0
[<lock_acquire at lockdep.c:3587>] lock_acquire+0x50/0x78
[<__raw_read_lock at rwlock_api_smp.h:150
 (inlined by) _raw_read_lock at spinlock.c:223>] _raw_read_lock+0x4c/0x90
[<hlist_empty at list.h:611
 (inlined by) raw_v4_input at raw.c:177
 (inlined by) raw_local_deliver at raw.c:216>] raw_local_deliver+0x58/0x1e8
[<ip_local_deliver_finish at ip_input.c:205>] ip_local_deliver_finish+0x118/0x4a8
[<NF_HOOK_THRESH at netfilter.h:226
 (inlined by) NF_HOOK at netfilter.h:249
 (inlined by) ip_local_deliver at ip_input.c:257>] ip_local_deliver+0x68/0xe0
[<NF_HOOK_THRESH at ip_input.c:467
 (inlined by) NF_HOOK at netfilter.h:249
 (inlined by) ip_rcv at ip_input.c:455>] ip_rcv+0x398/0x478
[<__netif_receive_skb_core at dev.c:3948>] __netif_receive_skb_core+0x764/0x818
[<rcu_read_unlock at rcupdate.h:913
 (inlined by) netif_receive_skb_internal at dev.c:4012>] netif_receive_skb_internal+0x148/0x214
[<cvm_oct_poll at ethernet-rx.c:379
 (inlined by) cvm_oct_napi_poll at ethernet-rx.c:452>] cvm_oct_napi_poll+0x790/0xa2c
[<napi_poll at dev.c:4804
 (inlined by) net_rx_action at dev.c:4869>] net_rx_action+0x130/0x2e0
[<preempt_count at preempt.h:10
 (inlined by) __do_softirq at softirq.c:275>] __do_softirq+0x1f0/0x318
[<do_softirq_own_stack at interrupt.h:449
 (inlined by) invoke_softirq at softirq.c:357
 (inlined by) irq_exit at softirq.c:391>] irq_exit+0x64/0xcc
[<octeon_irq_ciu2 at octeon-irq.c:1951>] octeon_irq_ciu2+0x154/0x1c4
[<plat_irq_dispatch at octeon-irq.c:2319>] plat_irq_dispatch+0x70/0x108
[<?? at entry.S:35>] ret_from_irq+0x0/0x4
[<?? at genex.S:132>] __r4k_wait+0x20/0x40
[<arch_local_save_flags at irqflags.h:149
 (inlined by) cpuidle_idle_call at idle.c:196
 (inlined by) cpu_idle_loop at idle.c:251
 (inlined by) cpu_startup_entry at idle.c:299>] cpu_startup_entry+0x154/0x1d0
[<start_kernel at main.c:684>] start_kernel+0x538/0x554

Presumably there's some sort of race condition that my change doesn't
really fix but happens to avoid by dereferencing rx_group just once early
on?

--Ed

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2016-09-01  1:52 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-30 18:47 [PATCH 0/9] staging: octeon: multi rx group (queue) support Aaro Koskinen
2016-08-30 18:47 ` [PATCH 1/9] staging: octeon: disable rx interrupts in oct_rx_shutdown Aaro Koskinen
2016-08-30 18:47 ` [PATCH 2/9] staging: octeon: use passed interrupt number in the handler Aaro Koskinen
2016-08-30 18:47 ` [PATCH 3/9] staging: octeon: pass the NAPI instance reference to irq handler Aaro Koskinen
2016-08-30 18:47 ` [PATCH 4/9] staging: octeon: move common poll code into a separate function Aaro Koskinen
2016-08-30 18:47 ` [PATCH 5/9] staging: octeon: create a struct for rx group specific data Aaro Koskinen
2016-08-30 18:47 ` [PATCH 6/9] staging: octeon: move irq into " Aaro Koskinen
2016-08-30 18:47 ` [PATCH 7/9] staging: octeon: move group number into rx group data Aaro Koskinen
2016-08-30 18:47 ` [PATCH 8/9] staging: octeon: support enabling multiple rx groups Aaro Koskinen
2016-08-30 18:47 ` [PATCH 9/9] staging: octeon: enable taking multiple rx groups into use Aaro Koskinen
2016-08-31  1:12 ` [PATCH 0/9] staging: octeon: multi rx group (queue) support Ed Swierk
2016-08-31  6:29   ` Aaro Koskinen
2016-08-31 16:20     ` Ed Swierk
2016-08-31 21:20       ` Aaro Koskinen
2016-09-01  1:52         ` Ed Swierk
2016-08-31 15:06   ` Aaro Koskinen
2016-08-31 16:10     ` David Daney

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).