netdev - Re: [PATCH net] bnxt_en: do not map packet buffers twice

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CACKFLinEXJmvZv=qAHd4UkwMTEcPYc+9td+MePcU+VgYLLq6jA@mail.gmail.com>
Date: Thu, 14 Dec 2023 16:27:25 -0800
From: Michael Chan <michael.chan@...adcom.com>
To: David Wei <dw@...idwei.uk>
Cc: davem@...emloft.net, netdev@...r.kernel.org, edumazet@...gle.com, 
	kuba@...nel.org, pabeni@...hat.com, bpf@...r.kernel.org, hawk@...nel.org, 
	ast@...nel.org, daniel@...earbox.net, john.fastabend@...il.com, 
	Andy Gospodarek <andrew.gospodarek@...adcom.com>, Somnath Kotur <somnath.kotur@...adcom.com>
Subject: Re: [PATCH net] bnxt_en: do not map packet buffers twice

On Thu, Dec 14, 2023 at 3:18 PM David Wei <dw@...idwei.uk> wrote:
>
> On 2023-12-14 13:31, Michael Chan wrote:
> > From: Andy Gospodarek <andrew.gospodarek@...adcom.com>
> >
> > Remove double-mapping of DMA buffers as it can prevent page pool entries
> > from being freed.  Mapping is managed by page pool infrastructure and
> > was previously managed by the driver in __bnxt_alloc_rx_page before
> > allowing the page pool infrastructure to manage it.
> >
> > Fixes: 578fcfd26e2a ("bnxt_en: Let the page pool manage the DMA mapping")
> > Reviewed-by: Somnath Kotur <somnath.kotur@...adcom.com>
> > Signed-off-by: Andy Gospodarek <andrew.gospodarek@...adcom.com>
> > Signed-off-by: Michael Chan <michael.chan@...adcom.com>
> > ---
> >  drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c | 11 ++---------
> >  1 file changed, 2 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c
> > index 96f5ca778c67..8cb9a99154aa 100644
> > --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c
> > +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c
> > @@ -59,7 +59,6 @@ struct bnxt_sw_tx_bd *bnxt_xmit_bd(struct bnxt *bp,
> >       for (i = 0; i < num_frags ; i++) {
> >               skb_frag_t *frag = &sinfo->frags[i];
> >               struct bnxt_sw_tx_bd *frag_tx_buf;
> > -             struct pci_dev *pdev = bp->pdev;
> >               dma_addr_t frag_mapping;
> >               int frag_len;
> >
> > @@ -73,16 +72,10 @@ struct bnxt_sw_tx_bd *bnxt_xmit_bd(struct bnxt *bp,
> >               txbd = &txr->tx_desc_ring[TX_RING(prod)][TX_IDX(prod)];
> >
> >               frag_len = skb_frag_size(frag);
> > -             frag_mapping = skb_frag_dma_map(&pdev->dev, frag, 0,
> > -                                             frag_len, DMA_TO_DEVICE);
> > -
> > -             if (unlikely(dma_mapping_error(&pdev->dev, frag_mapping)))
> > -                     return NULL;
> > -
> > -             dma_unmap_addr_set(frag_tx_buf, mapping, frag_mapping);
>
> If this is no longer set, what would happen to dma_unmap_single() in
> bnxt_tx_int_xdp() that is then reading `mapping` via dma_unmap_addr()?

The dma_unmap_addr() call in bnxt_tx_int_xdp() is for XDP_REDIRECT
packets only.  Our current XDP_REDIRECT implementation supports only a
single buffer per packet.  Here, this code path is for XDP_TX
multi-buffers.  The DMA mapping for the frag pages is always handled
by the page pool.  Thanks.

Download attachment "smime.p7s" of type "application/pkcs7-signature" (4209 bytes)