[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20240910094240.ileosxxgujfolkza@DEN-DL-M70577>
Date: Tue, 10 Sep 2024 09:42:40 +0000
From: Daniel Machon <daniel.machon@...rochip.com>
To: Paolo Abeni <pabeni@...hat.com>
CC: Horatiu Vultur <horatiu.vultur@...rochip.com>,
<UNGLinuxDriver@...rochip.com>, "David S. Miller" <davem@...emloft.net>,
"Eric Dumazet" <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
"Alexei Starovoitov" <ast@...nel.org>, Daniel Borkmann
<daniel@...earbox.net>, "Jesper Dangaard Brouer" <hawk@...nel.org>, John
Fastabend <john.fastabend@...il.com>, <netdev@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <bpf@...r.kernel.org>
Subject: Re: [PATCH net-next 04/12] net: lan966x: use the FDMA library for
allocation of rx buffers
> Use the two functions: fdma_alloc_phys() and fdma_dcb_init() for rx
> > buffer allocation and use the new buffers throughout.
> >
> > In order to replace the old buffers with the new ones, we have to do the
> > following refactoring:
> >
> > - use fdma_alloc_phys() and fdma_dcb_init()
> >
> > - replace the variables: rx->dma, rx->dcbs and rx->last_entry
> > with the equivalents from the FDMA struct.
> >
> > - make use of fdma->db_size for rx buffer size.
> >
> > - add lan966x_fdma_rx_dataptr_cb callback for obtaining the dataptr.
> >
> > - Initialize FDMA struct values.
> >
> > Signed-off-by: Daniel Machon <daniel.machon@...rochip.com>
> > Reviewed-by: Horatiu Vultur <horatiu.vultur@...rochip.com>
> > ---
> > .../net/ethernet/microchip/lan966x/lan966x_fdma.c | 116 ++++++++++-----------
> > .../net/ethernet/microchip/lan966x/lan966x_main.h | 15 ---
> > 2 files changed, 55 insertions(+), 76 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c b/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c
> > index b64f04ff99a8..99d09c97737e 100644
> > --- a/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c
> > +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c
> > @@ -6,13 +6,30 @@
> >
> > #include "lan966x_main.h"
> >
> > +static int lan966x_fdma_rx_dataptr_cb(struct fdma *fdma, int dcb, int db,
> > + u64 *dataptr)
> > +{
> > + struct lan966x *lan966x = (struct lan966x *)fdma->priv;
> > + struct lan966x_rx *rx = &lan966x->rx;
> > + struct page *page;
> > +
> > + page = page_pool_dev_alloc_pages(rx->page_pool);
> > + if (unlikely(!page))
> > + return -ENOMEM;
> > +
> > + rx->page[dcb][db] = page;
> > + *dataptr = page_pool_get_dma_addr(page) + XDP_PACKET_HEADROOM;
> > +
> > + return 0;
> > +}
>
> Very nice cleanup indeed!
>
> Out of ENOMEM I can't recall if the following was already discussed, but
> looking at this cb, I'm wondering if a possible follow-up could replace
> the dataptr_cb() and nextptr_cb() with lib functions i.e. operating on
> page pool or doing netdev allocations according to some fdma lib flags.
>
> Cheers,
>
> Paolo
>
Hi Paolo,
Something like this could definitely be added down the road. I initially
left this out to reduce library complexity.
Thanks for reviewing!
/Daniel
Powered by blists - more mailing lists