[PATCH bpf] ice: xsk: do not use xdp_return_frame() on tx_buf->raw_buf

[PATCH bpf] ice: xsk: do not use xdp_return_frame() on tx_buf->raw_buf

[Maciej Fijalkowski]

Previously ice XDP xmit routine was changed in a way that it avoids
xdp_buff->xdp_frame conversion as it is simply not needed for handling
XDP_TX action and what is more it saves us CPU cycles. This routine is
re-used on ZC driver to handle XDP_TX action.

Although for XDP_TX on Rx ZC xdp_buff that comes from xsk_buff_pool is
converted to xdp_frame, xdp_frame itself is not stored inside
ice_tx_buf, we only store raw data pointer. Casting this pointer to
xdp_frame and calling against it xdp_return_frame in
ice_clean_xdp_tx_buf() results in undefined behavior.

To fix this, simply call page_frag_free() on tx_buf->raw_buf.
Later intention is to remove the buff->frame conversion in order to
simplify the codebase and improve XDP_TX performance on ZC.

Fixes: 126cdfe1007a ("ice: xsk: Improve AF_XDP ZC Tx and use batching API")
Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_xsk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_xsk.c b/drivers/net/ethernet/intel/ice/ice_xsk.c
index 056c904b83cc..79fa65d1cf20 100644
--- a/drivers/net/ethernet/intel/ice/ice_xsk.c
+++ b/drivers/net/ethernet/intel/ice/ice_xsk.c
@@ -772,7 +772,7 @@ int ice_clean_rx_irq_zc(struct ice_rx_ring *rx_ring, int budget)
 static void
 ice_clean_xdp_tx_buf(struct ice_tx_ring *xdp_ring, struct ice_tx_buf *tx_buf)
 {
-	xdp_return_frame((struct xdp_frame *)tx_buf->raw_buf);
+	page_frag_free(tx_buf->raw_buf);
 	xdp_ring->xdp_tx_active--;
 	dma_unmap_single(xdp_ring->dev, dma_unmap_addr(tx_buf, dma),
 			 dma_unmap_len(tx_buf, len), DMA_TO_DEVICE);
-- 
2.34.1

Re: [PATCH bpf] ice: xsk: do not use xdp_return_frame() on tx_buf->raw_buf

[Maciej Fijalkowski]

On Tue, Nov 29, 2022 at 03:48:44PM +0000, Robin Cowley wrote:
> Hi Maciej,
> 
> I've just tested the patch you supplied and that seems to have worked perfectly when using multi FCQs.
> Thank you getting back so quickly with the patch.

Awesome. I will add your tag to commit message and send it to upstream.
Thanks for giving it a spin!

> 
> Thanks,
> Robin
> ________________________________
> From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> Sent: 29 November 2022 14:25
> To: Robin Cowley <Robin.Cowley@thehutgroup.com>; magnus.karlsson@intel.com <magnus.karlsson@intel.com>
> Cc: xdp-newbies@vger.kernel.org <xdp-newbies@vger.kernel.org>; Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> Subject: [PATCH bpf] ice: xsk: do not use xdp_return_frame() on tx_buf->raw_buf
> 
> CAUTION: This email originates from outside THG
> 
> Previously ice XDP xmit routine was changed in a way that it avoids
> xdp_buff->xdp_frame conversion as it is simply not needed for handling
> XDP_TX action and what is more it saves us CPU cycles. This routine is
> re-used on ZC driver to handle XDP_TX action.
> 
> Although for XDP_TX on Rx ZC xdp_buff that comes from xsk_buff_pool is
> converted to xdp_frame, xdp_frame itself is not stored inside
> ice_tx_buf, we only store raw data pointer. Casting this pointer to
> xdp_frame and calling against it xdp_return_frame in
> ice_clean_xdp_tx_buf() results in undefined behavior.
> 
> To fix this, simply call page_frag_free() on tx_buf->raw_buf.
> Later intention is to remove the buff->frame conversion in order to
> simplify the codebase and improve XDP_TX performance on ZC.
> 
> Fixes: 126cdfe1007a ("ice: xsk: Improve AF_XDP ZC Tx and use batching API")
> Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_xsk.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_xsk.c b/drivers/net/ethernet/intel/ice/ice_xsk.c
> index 056c904b83cc..79fa65d1cf20 100644
> --- a/drivers/net/ethernet/intel/ice/ice_xsk.c
> +++ b/drivers/net/ethernet/intel/ice/ice_xsk.c
> @@ -772,7 +772,7 @@ int ice_clean_rx_irq_zc(struct ice_rx_ring *rx_ring, int budget)
>  static void
>  ice_clean_xdp_tx_buf(struct ice_tx_ring *xdp_ring, struct ice_tx_buf *tx_buf)
>  {
> -       xdp_return_frame((struct xdp_frame *)tx_buf->raw_buf);
> +       page_frag_free(tx_buf->raw_buf);
>          xdp_ring->xdp_tx_active--;
>          dma_unmap_single(xdp_ring->dev, dma_unmap_addr(tx_buf, dma),
>                           dma_unmap_len(tx_buf, len), DMA_TO_DEVICE);
> --
> 2.34.1
> 
> 
> Robin Cowley
> Software Engineer
> The Hut Group<http://www.thehutgroup.com/>
> 
> Tel:
> Email: Robin.Cowley@thehutgroup.com<mailto:Robin.Cowley@thehutgroup.com>
> 
> For the purposes of this email, the "company" means The Hut Group Limited, a company registered in England and Wales (company number 6539496) whose registered office is at Fifth Floor, Voyager House, Chicago Avenue, Manchester Airport, M90 3DQ and/or any of its respective subsidiaries.
> 
> Confidentiality Notice
> This e-mail is confidential and intended for the use of the named recipient only. If you are not the intended recipient please notify us by telephone immediately on +44(0)1606 811888 or return it to us by e-mail. Please then delete it from your system and note that any use, dissemination, forwarding, printing or copying is strictly prohibited. Any views or opinions are solely those of the author and do not necessarily represent those of the company.
> 
> Encryptions and Viruses
> Please note that this e-mail and any attachments have not been encrypted. They may therefore be liable to be compromised. Please also note that it is your responsibility to scan this e-mail and any attachments for viruses. We do not, to the extent permitted by law, accept any liability (whether in contract, negligence or otherwise) for any virus infection and/or external compromise of security and/or confidentiality in relation to transmissions sent by e-mail.
> 
> Monitoring
> Activity and use of the company's systems is monitored to secure its effective use and operation and for other lawful business purposes. Communications using these systems will also be monitored and may be recorded to secure effective use and operation and for other lawful business purposes.
> 
> hgvyjuv