Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1472768 > unrolled thread

[PATCH 0/9] staging: octeon: multi rx group (queue) support

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/9] staging: octeon: multi rx group (queue) support Aaro Koskinen <aaro.koskinen@iki.fi> - 2016-08-30 20:50 +0200
    [PATCH 5/9] staging: octeon: create a struct for rx group specific data Aaro Koskinen <aaro.koskinen@iki.fi> - 2016-08-30 20:50 +0200
    [PATCH 2/9] staging: octeon: use passed interrupt number in the handler Aaro Koskinen <aaro.koskinen@iki.fi> - 2016-08-30 20:50 +0200
    [PATCH 1/9] staging: octeon: disable rx interrupts in oct_rx_shutdown Aaro Koskinen <aaro.koskinen@iki.fi> - 2016-08-30 20:50 +0200
    [PATCH 7/9] staging: octeon: move group number into rx group data Aaro Koskinen <aaro.koskinen@iki.fi> - 2016-08-30 21:00 +0200
    Re: [PATCH 0/9] staging: octeon: multi rx group (queue) support Ed Swierk <eswierk@skyportsystems.com> - 2016-08-31 03:20 +0200
      Re: [PATCH 0/9] staging: octeon: multi rx group (queue) support Aaro Koskinen <aaro.koskinen@iki.fi> - 2016-08-31 08:40 +0200
        Re: [PATCH 0/9] staging: octeon: multi rx group (queue) support Ed Swierk <eswierk@skyportsystems.com> - 2016-08-31 18:20 +0200
          Re: [PATCH 0/9] staging: octeon: multi rx group (queue) support Aaro Koskinen <aaro.koskinen@iki.fi> - 2016-08-31 23:30 +0200
            Re: [PATCH 0/9] staging: octeon: multi rx group (queue) support Ed Swierk <eswierk@skyportsystems.com> - 2016-09-01 04:00 +0200
      Re: [PATCH 0/9] staging: octeon: multi rx group (queue) support Aaro Koskinen <aaro.koskinen@iki.fi> - 2016-08-31 17:10 +0200

#1472768 — [PATCH 0/9] staging: octeon: multi rx group (queue) support

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

[toc] | [next] | [standalone]

#1472770 — [PATCH 5/9] staging: octeon: create a struct for rx group specific data

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

[toc] | [prev] | [next] | [standalone]

#1472772 — [PATCH 2/9] staging: octeon: use passed interrupt number in the handler

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

[toc] | [prev] | [next] | [standalone]

#1472773 — [PATCH 1/9] staging: octeon: disable rx interrupts in oct_rx_shutdown

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

[toc] | [prev] | [next] | [standalone]

#1472774 — [PATCH 7/9] staging: octeon: move group number into rx group data

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

[toc] | [prev] | [next] | [standalone]

#1472967

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

[toc] | [prev] | [next] | [standalone]

#1473062

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.

[toc] | [prev] | [next] | [standalone]

#1473653

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

[toc] | [prev] | [next] | [standalone]

#1473893

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.

[toc] | [prev] | [next] | [standalone]

#1474018

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

[toc] | [prev] | [next] | [standalone]

#1473597

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.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web