| Message ID | 20230516212117.1726491-1-shenwei.wang@nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] net: fec: add wmb to ensure correct descriptor values | expand |
> -----Original Message----- > From: Shenwei Wang <shenwei.wang@nxp.com> > Sent: 2023年5月17日 5:21 > 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> > 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 net] net: fec: add wmb to ensure correct descriptor values > > Two wmb() are added in the XDP TX path to ensure proper ordering of > descriptor and buffer updates: > 1. A 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 wmb() is also added after updating the tx_skbuff and 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> > --- > drivers/net/ethernet/freescale/fec_main.c | 19 +++++++++++++------ > 1 file changed, 13 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/ethernet/freescale/fec_main.c > b/drivers/net/ethernet/freescale/fec_main.c > index 6d0b46c76924..ba4335d5ddc3 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. > + */ > + 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,16 @@ 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 and tx_skbuff are performed before > + * txq->bd.cur. > + */ I think this description should be modify appropriately, because tx_skbuff is updated at the first wmb(), not here. > + wmb(); > + > txq->bd.cur = bdp; > > + /* Trigger transmission start */ > + writel(0, txq->bd.reg_desc_active); > + > return 0; > } > > @@ -3873,12 +3886,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
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 6d0b46c76924..ba4335d5ddc3 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. + */ + 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,16 @@ 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 and tx_skbuff are performed before + * txq->bd.cur. + */ + wmb(); + txq->bd.cur = bdp; + /* Trigger transmission start */ + writel(0, txq->bd.reg_desc_active); + return 0; } @@ -3873,12 +3886,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;
Two wmb() are added in the XDP TX path to ensure proper ordering of descriptor and buffer updates: 1. A 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 wmb() is also added after updating the tx_skbuff and 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> --- drivers/net/ethernet/freescale/fec_main.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-)