On Fri, Jun 28, 2019 at 11:09 PM Jonathan Lemon <jonathan.le...@gmail.com> wrote: > > > > On 28 Jun 2019, at 13:41, Jakub Kicinski wrote: > > > On Thu, 27 Jun 2019 19:31:26 -0700, Jonathan Lemon wrote: > >> On 27 Jun 2019, at 15:38, Jakub Kicinski wrote: > >> > >>> On Thu, 27 Jun 2019 15:08:32 -0700, Jonathan Lemon wrote: > >>>> The reuseq is actually a recycle stack, only accessed from the kernel > >>>> side. > >>>> Also, the implementation details of the stack should belong to the > >>>> umem > >>>> object, and not exposed to the caller. > >>>> > >>>> Clean up and rename for consistency in preparation for the next > >>>> patch. > >>>> > >>>> Signed-off-by: Jonathan Lemon <jonathan.le...@gmail.com> > >>> > >>> Prepare/swap is to cater to how drivers should be written - being able > >>> to allocate resources independently of those currently used. Allowing > >>> for changing ring sizes and counts on the fly. This patch makes it > >>> harder to write drivers in the way we are encouraging people to. > >>> > >>> IOW no, please don't do this. > >> > >> The main reason I rewrote this was to provide the same type > >> of functionality as realloc() - no need to allocate/initialize a new > >> array if the old one would still end up being used. This would seem > >> to be a win for the typical case of having the interface go up/down. > >> > >> Perhaps I should have named the function differently? > > > > Perhaps add a helper which calls both parts to help poorly architected > > drivers? > > Still ends up taking more memory. > > There are only 3 drivers in the tree which do AF_XDP: i40e, ixgbe, and mlx5. > > All of these do the same thing: > reuseq = xsk_reuseq_prepare(n) > if (!reuseq) > error > xsk_reuseq_free(xsk_reuseq_swap(umem, reuseq)); > > I figured simplifying was a good thing. > > But I do take your point that some future driver might want to allocate > everything up front before performing a commit of the resources.
Jonathan, can you come up with a solution that satisfies both these goals: providing a lower level API that Jakub can and would like to use for his driver and a higher level helper that can be used by today's driver to make the AF_XDP part smaller and easier to implement? I like the fact that you are simplifying the AF_XDP enabled drivers that are out there today, but at the same time I do not want to hinder Jakub from, hopefully, in the future upstreaming his support. Thanks: Magnus > -- > Jonathan
