* [PATCH] tg3: fix ring init when there are more TX than RX channels @ 2014-11-25 16:21 Thadeu Lima de Souza Cascardo 2014-11-25 19:33 ` David Miller 0 siblings, 1 reply; 4+ messages in thread From: Thadeu Lima de Souza Cascardo @ 2014-11-25 16:21 UTC (permalink / raw) To: netdev; +Cc: prashant, mchan, Thadeu Lima de Souza Cascardo If TX channels are set to 4 and RX channels are set to less than 4, using ethtool -L, the driver will try to initialize more RX channels than it has allocated, causing an oops. This fix only initializes the RX ring if it has been allocated. Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com> --- drivers/net/ethernet/broadcom/tg3.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c index dbb41c19..77f8f83 100644 --- a/drivers/net/ethernet/broadcom/tg3.c +++ b/drivers/net/ethernet/broadcom/tg3.c @@ -8563,7 +8563,8 @@ static int tg3_init_rings(struct tg3 *tp) if (tnapi->rx_rcb) memset(tnapi->rx_rcb, 0, TG3_RX_RCB_RING_BYTES(tp)); - if (tg3_rx_prodring_alloc(tp, &tnapi->prodring)) { + if (tnapi->prodring.rx_std && + tg3_rx_prodring_alloc(tp, &tnapi->prodring)) { tg3_free_rings(tp); return -ENOMEM; } -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] tg3: fix ring init when there are more TX than RX channels 2014-11-25 16:21 [PATCH] tg3: fix ring init when there are more TX than RX channels Thadeu Lima de Souza Cascardo @ 2014-11-25 19:33 ` David Miller 2014-11-25 19:57 ` cascardo 0 siblings, 1 reply; 4+ messages in thread From: David Miller @ 2014-11-25 19:33 UTC (permalink / raw) To: cascardo; +Cc: netdev, prashant, mchan From: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com> Date: Tue, 25 Nov 2014 14:21:11 -0200 > @@ -8563,7 +8563,8 @@ static int tg3_init_rings(struct tg3 *tp) > if (tnapi->rx_rcb) > memset(tnapi->rx_rcb, 0, TG3_RX_RCB_RING_BYTES(tp)); > > - if (tg3_rx_prodring_alloc(tp, &tnapi->prodring)) { > + if (tnapi->prodring.rx_std && > + tg3_rx_prodring_alloc(tp, &tnapi->prodring)) { > tg3_free_rings(tp); > return -ENOMEM; Maybe a better test is "i < tp->rxq_cnt"? This is what is used in tg3_mem_rx_acquire() to determine if tg3_rx_prodring_init() happens. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] tg3: fix ring init when there are more TX than RX channels 2014-11-25 19:33 ` David Miller @ 2014-11-25 19:57 ` cascardo 2014-11-25 20:19 ` David Miller 0 siblings, 1 reply; 4+ messages in thread From: cascardo @ 2014-11-25 19:57 UTC (permalink / raw) To: David Miller; +Cc: netdev, prashant, mchan On Tue, Nov 25, 2014 at 02:33:52PM -0500, David Miller wrote: > From: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com> > Date: Tue, 25 Nov 2014 14:21:11 -0200 > > > @@ -8563,7 +8563,8 @@ static int tg3_init_rings(struct tg3 *tp) > > if (tnapi->rx_rcb) > > memset(tnapi->rx_rcb, 0, TG3_RX_RCB_RING_BYTES(tp)); > > > > - if (tg3_rx_prodring_alloc(tp, &tnapi->prodring)) { > > + if (tnapi->prodring.rx_std && > > + tg3_rx_prodring_alloc(tp, &tnapi->prodring)) { > > tg3_free_rings(tp); > > return -ENOMEM; > > Maybe a better test is "i < tp->rxq_cnt"? This is what is used in > tg3_mem_rx_acquire() to determine if tg3_rx_prodring_init() happens. > Well, what we have in net-next/master does: for (i = 0; i < limit; i++) { struct tg3_napi *tnapi = &tp->napi[i]; if (tg3_rx_prodring_init(tp, &tnapi->prodring)) goto err_out; where limit is: limit = tp->rxq_cnt; if (tg3_flag(tp, ENABLE_RSS)) limit++; So, I thought that, instead of rewriting that same logic in tg3_init_rings, that we should better just check that the rx_std has been allocated, which is going to happen in tg3_rx_prodring_init. So, the alternative would be: diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c index dbb41c19..51d45aa 100644 --- a/drivers/net/ethernet/broadcom/tg3.c +++ b/drivers/net/ethernet/broadcom/tg3.c @@ -8540,7 +8540,11 @@ static void tg3_free_rings(struct tg3 *tp) */ static int tg3_init_rings(struct tg3 *tp) { - int i; + int i, limit; + + limit = tp->rxq_cnt; + if (tg3_flag(tp, ENABLE_RSS)) + limit++; /* Free up all the SKBs. */ tg3_free_rings(tp); @@ -8563,7 +8567,8 @@ static int tg3_init_rings(struct tg3 *tp) if (tnapi->rx_rcb) memset(tnapi->rx_rcb, 0, TG3_RX_RCB_RING_BYTES(tp)); - if (tg3_rx_prodring_alloc(tp, &tnapi->prodring)) { + if (i < limit && + tg3_rx_prodring_alloc(tp, &tnapi->prodring)) { tg3_free_rings(tp); return -ENOMEM; } ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] tg3: fix ring init when there are more TX than RX channels 2014-11-25 19:57 ` cascardo @ 2014-11-25 20:19 ` David Miller 0 siblings, 0 replies; 4+ messages in thread From: David Miller @ 2014-11-25 20:19 UTC (permalink / raw) To: cascardo; +Cc: netdev, prashant, mchan From: cascardo@linux.vnet.ibm.com Date: Tue, 25 Nov 2014 17:57:21 -0200 > On Tue, Nov 25, 2014 at 02:33:52PM -0500, David Miller wrote: >> From: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com> >> Date: Tue, 25 Nov 2014 14:21:11 -0200 >> >> > @@ -8563,7 +8563,8 @@ static int tg3_init_rings(struct tg3 *tp) >> > if (tnapi->rx_rcb) >> > memset(tnapi->rx_rcb, 0, TG3_RX_RCB_RING_BYTES(tp)); >> > >> > - if (tg3_rx_prodring_alloc(tp, &tnapi->prodring)) { >> > + if (tnapi->prodring.rx_std && >> > + tg3_rx_prodring_alloc(tp, &tnapi->prodring)) { >> > tg3_free_rings(tp); >> > return -ENOMEM; >> >> Maybe a better test is "i < tp->rxq_cnt"? This is what is used in >> tg3_mem_rx_acquire() to determine if tg3_rx_prodring_init() happens. >> > > Well, what we have in net-next/master does: > > for (i = 0; i < limit; i++) { > struct tg3_napi *tnapi = &tp->napi[i]; > > if (tg3_rx_prodring_init(tp, &tnapi->prodring)) > goto err_out; > > where limit is: > > limit = tp->rxq_cnt; > > if (tg3_flag(tp, ENABLE_RSS)) > limit++; > > So, I thought that, instead of rewriting that same logic in > tg3_init_rings, that we should better just check that the rx_std has > been allocated, which is going to happen in tg3_rx_prodring_init. > > So, the alternative would be: Ok, come to think of it, your original test is fine and I've applied that patch and queued it up for -stable. Thanks. ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-11-25 20:19 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-11-25 16:21 [PATCH] tg3: fix ring init when there are more TX than RX channels Thadeu Lima de Souza Cascardo 2014-11-25 19:33 ` David Miller 2014-11-25 19:57 ` cascardo 2014-11-25 20:19 ` David Miller
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).