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
| ||
|
Message-ID: <CAHS8izOu=DxeRdD7Gdt-N2qvH_Nnwpcem1KkNgjOLeWzHZ_5JQ@mail.gmail.com> Date: Thu, 17 Aug 2023 14:56:01 -0700 From: Mina Almasry <almasrymina@...gle.com> To: Jakub Kicinski <kuba@...nel.org> Cc: netdev@...r.kernel.org, hawk@...nel.org, ilias.apalodimas@...aro.org, aleksander.lobakin@...el.com, linyunsheng@...wei.com Subject: Re: [RFC net-next 04/13] net: page_pool: id the page pools On Wed, Aug 16, 2023 at 4:43 PM Jakub Kicinski <kuba@...nel.org> wrote: > > To give ourselves the flexibility of creating netlink commands > and ability to refer to page pool instances in uAPIs create > IDs for page pools. > Sorry maybe I'm missing something, but it's a bit curious to me that this ID is needed. An rx queue can only ever have 1 page-pool associated with it at a time, right? Could you instead add a pointer to the page_pool into 'struct netdev_rx_queue', and then page-pool can be referred to by the netdev id & the rx-queue number? Wouldn't that make the implementation much simpler? I also believe the userspace refers to the rx-queue by its index number for the ethtool APIs like adding flow steering rules, so extending that to here maybe makes sense. > Signed-off-by: Jakub Kicinski <kuba@...nel.org> > --- > include/net/page_pool/types.h | 4 ++++ > net/core/Makefile | 2 +- > net/core/page_pool.c | 21 +++++++++++++++----- > net/core/page_pool_priv.h | 9 +++++++++ > net/core/page_pool_user.c | 36 +++++++++++++++++++++++++++++++++++ > 5 files changed, 66 insertions(+), 6 deletions(-) > create mode 100644 net/core/page_pool_priv.h > create mode 100644 net/core/page_pool_user.c > > diff --git a/include/net/page_pool/types.h b/include/net/page_pool/types.h > index 1ac7ce25fbd4..9fadf15dadfa 100644 > --- a/include/net/page_pool/types.h > +++ b/include/net/page_pool/types.h > @@ -189,6 +189,10 @@ struct page_pool { > > /* Slow/Control-path information follows */ > struct page_pool_params_slow slow; > + /* User-facing fields, protected by page_pools_lock */ > + struct { > + u32 id; > + } user; > }; > > struct page *page_pool_alloc_pages(struct page_pool *pool, gfp_t gfp); > diff --git a/net/core/Makefile b/net/core/Makefile > index 731db2eaa610..4ae3d83f67d5 100644 > --- a/net/core/Makefile > +++ b/net/core/Makefile > @@ -18,7 +18,7 @@ obj-y += dev.o dev_addr_lists.o dst.o netevent.o \ > obj-$(CONFIG_NETDEV_ADDR_LIST_TEST) += dev_addr_lists_test.o > > obj-y += net-sysfs.o > -obj-$(CONFIG_PAGE_POOL) += page_pool.o > +obj-$(CONFIG_PAGE_POOL) += page_pool.o page_pool_user.o > obj-$(CONFIG_PROC_FS) += net-procfs.o > obj-$(CONFIG_NET_PKTGEN) += pktgen.o > obj-$(CONFIG_NETPOLL) += netpoll.o > diff --git a/net/core/page_pool.c b/net/core/page_pool.c > index 8e71e116224d..de199c356043 100644 > --- a/net/core/page_pool.c > +++ b/net/core/page_pool.c > @@ -23,6 +23,8 @@ > > #include <trace/events/page_pool.h> > > +#include "page_pool_priv.h" > + > #define DEFER_TIME (msecs_to_jiffies(1000)) > #define DEFER_WARN_INTERVAL (60 * HZ) > > @@ -264,13 +266,21 @@ struct page_pool *page_pool_create(const struct page_pool_params *params) > return ERR_PTR(-ENOMEM); > > err = page_pool_init(pool, params); > - if (err < 0) { > - pr_warn("%s() gave up with errno %d\n", __func__, err); > - kfree(pool); > - return ERR_PTR(err); > - } > + if (err < 0) > + goto err_free; > + > + err = page_pool_list(pool); > + if (err) > + goto err_uninit; > > return pool; > + > +err_uninit: > + page_pool_uninit(pool); > +err_free: > + pr_warn("%s() gave up with errno %d\n", __func__, err); > + kfree(pool); > + return ERR_PTR(err); > } > EXPORT_SYMBOL(page_pool_create); > > @@ -818,6 +828,7 @@ static void page_pool_free(struct page_pool *pool) > if (pool->disconnect) > pool->disconnect(pool); > > + page_pool_unlist(pool); > page_pool_uninit(pool); > kfree(pool); > } > diff --git a/net/core/page_pool_priv.h b/net/core/page_pool_priv.h > new file mode 100644 > index 000000000000..6c4e4aeed02a > --- /dev/null > +++ b/net/core/page_pool_priv.h > @@ -0,0 +1,9 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +#ifndef __PAGE_POOL_PRIV_H > +#define __PAGE_POOL_PRIV_H > + > +int page_pool_list(struct page_pool *pool); > +void page_pool_unlist(struct page_pool *pool); > + > +#endif > diff --git a/net/core/page_pool_user.c b/net/core/page_pool_user.c > new file mode 100644 > index 000000000000..af4ac38a2de1 > --- /dev/null > +++ b/net/core/page_pool_user.c > @@ -0,0 +1,36 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > + > +#include <linux/mutex.h> > +#include <linux/xarray.h> > +#include <net/page_pool/types.h> > + > +#include "page_pool_priv.h" > + > +static DEFINE_XARRAY_FLAGS(page_pools, XA_FLAGS_ALLOC1); > +static DEFINE_MUTEX(page_pools_lock); > + > +int page_pool_list(struct page_pool *pool) > +{ > + static u32 id_alloc_next; > + int err; > + > + mutex_lock(&page_pools_lock); > + err = xa_alloc_cyclic(&page_pools, &pool->user.id, pool, xa_limit_32b, > + &id_alloc_next, GFP_KERNEL); > + if (err < 0) > + goto err_unlock; > + > + mutex_unlock(&page_pools_lock); > + return 0; > + > +err_unlock: > + mutex_unlock(&page_pools_lock); > + return err; > +} > + > +void page_pool_unlist(struct page_pool *pool) > +{ > + mutex_lock(&page_pools_lock); > + xa_erase(&page_pools, pool->user.id); > + mutex_unlock(&page_pools_lock); > +} > -- > 2.41.0 > -- Thanks, Mina
Powered by blists - more mailing lists