Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH net 0/2]cxgb4: Fix ethtool selftest flits calculation
@ 2020-08-18 15:40 Ganji Aravind
  2020-08-18 15:40 ` [PATCH net 1/2] cxgb4: Fix work request size calculation for loopback test Ganji Aravind
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Ganji Aravind @ 2020-08-18 15:40 UTC (permalink / raw)
  To: netdev; +Cc: davem, vishal, rahul.lakkireddy

Patch 1 will fix work request size calculation for loopback selftest.

Patch 2 will fix race between loopback selftest and normal Tx handler.

Thanks,
Ganji Aravind.

Ganji Aravind (2):
  cxgb4: Fix work request size calculation for loopback test
  cxgb4: Fix race between loopback and normal Tx path

 drivers/net/ethernet/chelsio/cxgb4/sge.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

-- 
2.26.2


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

* [PATCH net 1/2] cxgb4: Fix work request size calculation for loopback test
  2020-08-18 15:40 [PATCH net 0/2]cxgb4: Fix ethtool selftest flits calculation Ganji Aravind
@ 2020-08-18 15:40 ` Ganji Aravind
  2020-08-18 19:33   ` Jesse Brandeburg
  2020-08-18 15:40 ` [PATCH net 2/2] cxgb4: Fix race between loopback and normal Tx path Ganji Aravind
  2020-08-18 20:03 ` [PATCH net 0/2]cxgb4: Fix ethtool selftest flits calculation David Miller
  2 siblings, 1 reply; 6+ messages in thread
From: Ganji Aravind @ 2020-08-18 15:40 UTC (permalink / raw)
  To: netdev; +Cc: davem, vishal, rahul.lakkireddy

Work request used for sending loopback packet needs to add
the firmware work request only once. So, fix by using
correct structure size.

Fixes: 7235ffae3d2c ("cxgb4: add loopback ethtool self-test")
Signed-off-by: Ganji Aravind <ganji.aravind@chelsio.com>
---
 drivers/net/ethernet/chelsio/cxgb4/sge.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/chelsio/cxgb4/sge.c b/drivers/net/ethernet/chelsio/cxgb4/sge.c
index d2b587d1670a..7c9fe4bc235b 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/sge.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/sge.c
@@ -2553,8 +2553,8 @@ int cxgb4_selftest_lb_pkt(struct net_device *netdev)
 
 	pkt_len = ETH_HLEN + sizeof(CXGB4_SELFTEST_LB_STR);
 
-	flits = DIV_ROUND_UP(pkt_len + sizeof(struct cpl_tx_pkt) +
-			     sizeof(*wr), sizeof(__be64));
+	flits = DIV_ROUND_UP(pkt_len + sizeof(*cpl) + sizeof(*wr),
+			     sizeof(__be64));
 	ndesc = flits_to_desc(flits);
 
 	lb = &pi->ethtool_lb;
-- 
2.26.2


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

* [PATCH net 2/2] cxgb4: Fix race between loopback and normal Tx path
  2020-08-18 15:40 [PATCH net 0/2]cxgb4: Fix ethtool selftest flits calculation Ganji Aravind
  2020-08-18 15:40 ` [PATCH net 1/2] cxgb4: Fix work request size calculation for loopback test Ganji Aravind
@ 2020-08-18 15:40 ` Ganji Aravind
  2020-08-18 19:35   ` Jesse Brandeburg
  2020-08-18 20:03 ` [PATCH net 0/2]cxgb4: Fix ethtool selftest flits calculation David Miller
  2 siblings, 1 reply; 6+ messages in thread
From: Ganji Aravind @ 2020-08-18 15:40 UTC (permalink / raw)
  To: netdev; +Cc: davem, vishal, rahul.lakkireddy

Even after Tx queues are marked stopped, there exists a
small window where the current packet in the normal Tx
path is still being sent out and loopback selftest ends
up corrupting the same Tx ring. So, ensure selftest takes
the Tx lock to synchronize access the Tx ring.

Fixes: 7235ffae3d2c ("cxgb4: add loopback ethtool self-test")
Signed-off-by: Ganji Aravind <ganji.aravind@chelsio.com>
---
 drivers/net/ethernet/chelsio/cxgb4/sge.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/chelsio/cxgb4/sge.c b/drivers/net/ethernet/chelsio/cxgb4/sge.c
index 7c9fe4bc235b..869431a1eedd 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/sge.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/sge.c
@@ -2561,11 +2561,14 @@ int cxgb4_selftest_lb_pkt(struct net_device *netdev)
 	lb->loopback = 1;
 
 	q = &adap->sge.ethtxq[pi->first_qset];
+	__netif_tx_lock(q->txq, smp_processor_id());
 
 	reclaim_completed_tx(adap, &q->q, -1, true);
 	credits = txq_avail(&q->q) - ndesc;
-	if (unlikely(credits < 0))
+	if (unlikely(credits < 0)) {
+		__netif_tx_unlock(q->txq);
 		return -ENOMEM;
+	}
 
 	wr = (void *)&q->q.desc[q->q.pidx];
 	memset(wr, 0, sizeof(struct tx_desc));
@@ -2598,6 +2601,7 @@ int cxgb4_selftest_lb_pkt(struct net_device *netdev)
 	init_completion(&lb->completion);
 	txq_advance(&q->q, ndesc);
 	cxgb4_ring_tx_db(adap, &q->q, ndesc);
+	__netif_tx_unlock(q->txq);
 
 	/* wait for the pkt to return */
 	ret = wait_for_completion_timeout(&lb->completion, 10 * HZ);
-- 
2.26.2


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

* Re: [PATCH net 1/2] cxgb4: Fix work request size calculation for loopback test
  2020-08-18 15:40 ` [PATCH net 1/2] cxgb4: Fix work request size calculation for loopback test Ganji Aravind
@ 2020-08-18 19:33   ` Jesse Brandeburg
  0 siblings, 0 replies; 6+ messages in thread
From: Jesse Brandeburg @ 2020-08-18 19:33 UTC (permalink / raw)
  To: Ganji Aravind; +Cc: netdev, davem, vishal, rahul.lakkireddy

Ganji Aravind wrote:

> Work request used for sending loopback packet needs to add
> the firmware work request only once. So, fix by using
> correct structure size.
> 
> Fixes: 7235ffae3d2c ("cxgb4: add loopback ethtool self-test")
> Signed-off-by: Ganji Aravind <ganji.aravind@chelsio.com>

changes look ok, but to understand why this change fixed the bug, you
could have just mentioned that the cpl_tx_pkt struct has a _core member
inside of it, and then I wouldn't have had to waste review time digging
through the code in the kernel.

Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com>

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

* Re: [PATCH net 2/2] cxgb4: Fix race between loopback and normal Tx path
  2020-08-18 15:40 ` [PATCH net 2/2] cxgb4: Fix race between loopback and normal Tx path Ganji Aravind
@ 2020-08-18 19:35   ` Jesse Brandeburg
  0 siblings, 0 replies; 6+ messages in thread
From: Jesse Brandeburg @ 2020-08-18 19:35 UTC (permalink / raw)
  To: Ganji Aravind; +Cc: netdev, davem, vishal, rahul.lakkireddy

Ganji Aravind wrote:

> Even after Tx queues are marked stopped, there exists a
> small window where the current packet in the normal Tx
> path is still being sent out and loopback selftest ends
> up corrupting the same Tx ring. So, ensure selftest takes
> the Tx lock to synchronize access the Tx ring.
> 
> Fixes: 7235ffae3d2c ("cxgb4: add loopback ethtool self-test")
> Signed-off-by: Ganji Aravind <ganji.aravind@chelsio.com>

Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com>


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

* Re: [PATCH net 0/2]cxgb4: Fix ethtool selftest flits calculation
  2020-08-18 15:40 [PATCH net 0/2]cxgb4: Fix ethtool selftest flits calculation Ganji Aravind
  2020-08-18 15:40 ` [PATCH net 1/2] cxgb4: Fix work request size calculation for loopback test Ganji Aravind
  2020-08-18 15:40 ` [PATCH net 2/2] cxgb4: Fix race between loopback and normal Tx path Ganji Aravind
@ 2020-08-18 20:03 ` David Miller
  2 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2020-08-18 20:03 UTC (permalink / raw)
  To: ganji.aravind; +Cc: netdev, vishal, rahul.lakkireddy

From: Ganji Aravind <ganji.aravind@chelsio.com>
Date: Tue, 18 Aug 2020 21:10:56 +0530

> Patch 1 will fix work request size calculation for loopback selftest.
> 
> Patch 2 will fix race between loopback selftest and normal Tx handler.

Series applied.

Thanks for the review Jesse.

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

end of thread, other threads:[~2020-08-18 20:03 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-18 15:40 [PATCH net 0/2]cxgb4: Fix ethtool selftest flits calculation Ganji Aravind
2020-08-18 15:40 ` [PATCH net 1/2] cxgb4: Fix work request size calculation for loopback test Ganji Aravind
2020-08-18 19:33   ` Jesse Brandeburg
2020-08-18 15:40 ` [PATCH net 2/2] cxgb4: Fix race between loopback and normal Tx path Ganji Aravind
2020-08-18 19:35   ` Jesse Brandeburg
2020-08-18 20:03 ` [PATCH net 0/2]cxgb4: Fix ethtool selftest flits calculation 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).