Netdev Archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 net] net: fec: add dma_wmb to ensure correct descriptor values
@ 2023-05-18 15:02 Shenwei Wang
  2023-05-19  1:33 ` Wei Fang
  2023-05-20  4:51 ` Jakub Kicinski
  0 siblings, 2 replies; 3+ messages in thread
From: Shenwei Wang @ 2023-05-18 15:02 UTC (permalink / raw
  To: Wei Fang, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, Horatiu Vultur, Heiner Kallweit
  Cc: Shenwei Wang, Clark Wang, NXP Linux Team, netdev, imx

Two dma_wmb() are added in the XDP TX path to ensure proper ordering of
descriptor and buffer updates:
1. A dma_wmb() is added after updating the last BD to make sure
   the updates to rest of the descriptor are visible before
   transferring ownership to FEC.
2. A dma_wmb() is also added after updating the bdp to ensure these
   updates are visible before updating txq->bd.cur.
3. Start the xmit of the frame immediately right after configuring the
   tx descriptor.

Fixes: 6d6b39f180b8 ("net: fec: add initial XDP support")
Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
---
 v3:
  - use the lightweight memory barrier dma_wmb.

 v2:
  - update the inline comments for 2nd wmb per Wei Fang's review.

 drivers/net/ethernet/freescale/fec_main.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 6d0b46c76924..a5096c3cac01 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -3834,6 +3834,11 @@ static int fec_enet_txq_xmit_frame(struct fec_enet_private *fep,
 	index = fec_enet_get_bd_index(last_bdp, &txq->bd);
 	txq->tx_skbuff[index] = NULL;

+	/* Make sure the updates to rest of the descriptor are performed before
+	 * transferring ownership.
+	 */
+	dma_wmb();
+
 	/* Send it on its way.  Tell FEC it's ready, interrupt when done,
 	 * it's the last BD of the frame, and to put the CRC on the end.
 	 */
@@ -3843,8 +3848,14 @@ static int fec_enet_txq_xmit_frame(struct fec_enet_private *fep,
 	/* If this was the last BD in the ring, start at the beginning again. */
 	bdp = fec_enet_get_nextdesc(last_bdp, &txq->bd);

+	/* Make sure the update to bdp are performed before txq->bd.cur. */
+	dma_wmb();
+
 	txq->bd.cur = bdp;

+	/* Trigger transmission start */
+	writel(0, txq->bd.reg_desc_active);
+
 	return 0;
 }

@@ -3873,12 +3884,6 @@ static int fec_enet_xdp_xmit(struct net_device *dev,
 		sent_frames++;
 	}

-	/* Make sure the update to bdp and tx_skbuff are performed. */
-	wmb();
-
-	/* Trigger transmission start */
-	writel(0, txq->bd.reg_desc_active);
-
 	__netif_tx_unlock(nq);

 	return sent_frames;
--
2.34.1


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

* RE: [PATCH v3 net] net: fec: add dma_wmb to ensure correct descriptor values
  2023-05-18 15:02 [PATCH v3 net] net: fec: add dma_wmb to ensure correct descriptor values Shenwei Wang
@ 2023-05-19  1:33 ` Wei Fang
  2023-05-20  4:51 ` Jakub Kicinski
  1 sibling, 0 replies; 3+ messages in thread
From: Wei Fang @ 2023-05-19  1:33 UTC (permalink / raw
  To: Shenwei Wang, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, Horatiu Vultur, Heiner Kallweit
  Cc: Clark Wang, dl-linux-imx, netdev@vger.kernel.org,
	imx@lists.linux.dev

> -----Original Message-----
> From: Shenwei Wang <shenwei.wang@nxp.com>
> Sent: 2023年5月18日 23:02
> To: Wei Fang <wei.fang@nxp.com>; David S. Miller <davem@davemloft.net>;
> Eric Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>;
> Paolo Abeni <pabeni@redhat.com>; Simon Horman <horms@kernel.org>;
> Horatiu Vultur <horatiu.vultur@microchip.com>; Heiner Kallweit
> <hkallweit1@gmail.com>
> Cc: Shenwei Wang <shenwei.wang@nxp.com>; Clark Wang
> <xiaoning.wang@nxp.com>; dl-linux-imx <linux-imx@nxp.com>;
> netdev@vger.kernel.org; imx@lists.linux.dev
> Subject: [PATCH v3 net] net: fec: add dma_wmb to ensure correct descriptor
> values
> 
> Two dma_wmb() are added in the XDP TX path to ensure proper ordering of
> descriptor and buffer updates:
> 1. A dma_wmb() is added after updating the last BD to make sure
>    the updates to rest of the descriptor are visible before
>    transferring ownership to FEC.
> 2. A dma_wmb() is also added after updating the bdp to ensure these
>    updates are visible before updating txq->bd.cur.
> 3. Start the xmit of the frame immediately right after configuring the
>    tx descriptor.
> 
> Fixes: 6d6b39f180b8 ("net: fec: add initial XDP support")
> Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
> ---
>  v3:
>   - use the lightweight memory barrier dma_wmb.
> 
>  v2:
>   - update the inline comments for 2nd wmb per Wei Fang's review.
> 
>  drivers/net/ethernet/freescale/fec_main.c | 17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/fec_main.c
> b/drivers/net/ethernet/freescale/fec_main.c
> index 6d0b46c76924..a5096c3cac01 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -3834,6 +3834,11 @@ static int fec_enet_txq_xmit_frame(struct
> fec_enet_private *fep,
>  	index = fec_enet_get_bd_index(last_bdp, &txq->bd);
>  	txq->tx_skbuff[index] = NULL;
> 
> +	/* Make sure the updates to rest of the descriptor are performed before
> +	 * transferring ownership.
> +	 */
> +	dma_wmb();
> +
>  	/* Send it on its way.  Tell FEC it's ready, interrupt when done,
>  	 * it's the last BD of the frame, and to put the CRC on the end.
>  	 */
> @@ -3843,8 +3848,14 @@ static int fec_enet_txq_xmit_frame(struct
> fec_enet_private *fep,
>  	/* If this was the last BD in the ring, start at the beginning again. */
>  	bdp = fec_enet_get_nextdesc(last_bdp, &txq->bd);
> 
> +	/* Make sure the update to bdp are performed before txq->bd.cur. */
> +	dma_wmb();
> +
>  	txq->bd.cur = bdp;
> 
> +	/* Trigger transmission start */
> +	writel(0, txq->bd.reg_desc_active);
> +
>  	return 0;
>  }
> 
> @@ -3873,12 +3884,6 @@ static int fec_enet_xdp_xmit(struct net_device
> *dev,
>  		sent_frames++;
>  	}
> 
> -	/* Make sure the update to bdp and tx_skbuff are performed. */
> -	wmb();
> -
> -	/* Trigger transmission start */
> -	writel(0, txq->bd.reg_desc_active);
> -
>  	__netif_tx_unlock(nq);
> 
>  	return sent_frames;
> --
> 2.34.1

Reviewed-by: Wei Fang <wei.fang@nxp.com>


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

* Re: [PATCH v3 net] net: fec: add dma_wmb to ensure correct descriptor values
  2023-05-18 15:02 [PATCH v3 net] net: fec: add dma_wmb to ensure correct descriptor values Shenwei Wang
  2023-05-19  1:33 ` Wei Fang
@ 2023-05-20  4:51 ` Jakub Kicinski
  1 sibling, 0 replies; 3+ messages in thread
From: Jakub Kicinski @ 2023-05-20  4:51 UTC (permalink / raw
  To: Shenwei Wang
  Cc: Wei Fang, David S. Miller, Eric Dumazet, Paolo Abeni,
	Simon Horman, Horatiu Vultur, Heiner Kallweit, Clark Wang,
	NXP Linux Team, netdev, imx

On Thu, 18 May 2023 10:02:02 -0500 Shenwei Wang wrote:
> Two dma_wmb() are added in the XDP TX path to ensure proper ordering of
> descriptor and buffer updates:
> 1. A dma_wmb() is added after updating the last BD to make sure
>    the updates to rest of the descriptor are visible before
>    transferring ownership to FEC.
> 2. A dma_wmb() is also added after updating the bdp to ensure these
>    updates are visible before updating txq->bd.cur.
> 3. Start the xmit of the frame immediately right after configuring the
>    tx descriptor.
> 
> Fixes: 6d6b39f180b8 ("net: fec: add initial XDP support")
> Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>

Applied, thanks (commit 9025944fddfed).

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

end of thread, other threads:[~2023-05-20  4:51 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-18 15:02 [PATCH v3 net] net: fec: add dma_wmb to ensure correct descriptor values Shenwei Wang
2023-05-19  1:33 ` Wei Fang
2023-05-20  4:51 ` Jakub Kicinski

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 read-only IMAP folder(s) and NNTP newsgroup(s).