| 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: <6a7f0976-f35e-4e5b-a77d-fbf6509a4ee4@suse.cz>
Date: Thu, 19 Jun 2025 11:32:57 +0200
From: Vlastimil Babka <vbabka@...e.cz>
To: Byungchul Park <byungchul@...com>, willy@...radead.org,
netdev@...r.kernel.org
Cc: linux-kernel@...r.kernel.org, linux-mm@...ck.org,
kernel_team@...ynix.com, kuba@...nel.org, almasrymina@...gle.com,
ilias.apalodimas@...aro.org, harry.yoo@...cle.com, hawk@...nel.org,
akpm@...ux-foundation.org, davem@...emloft.net, john.fastabend@...il.com,
andrew+netdev@...n.ch, asml.silence@...il.com, toke@...hat.com,
tariqt@...dia.com, edumazet@...gle.com, pabeni@...hat.com,
saeedm@...dia.com, leon@...nel.org, ast@...nel.org, daniel@...earbox.net,
david@...hat.com, lorenzo.stoakes@...cle.com, Liam.Howlett@...cle.com,
rppt@...nel.org, surenb@...gle.com, mhocko@...e.com, horms@...nel.org,
linux-rdma@...r.kernel.org, bpf@...r.kernel.org, vishal.moola@...il.com
Subject: Re: [PATCH net-next 1/9] netmem: introduce struct netmem_desc
mirroring struct page
On 6/9/25 06:32, Byungchul Park wrote:
> To simplify struct page, the page pool members of struct page should be
> moved to other, allowing these members to be removed from struct page.
>
> Introduce a network memory descriptor to store the members, struct
> netmem_desc, and make it union'ed with the existing fields in struct
> net_iov, allowing to organize the fields of struct net_iov.
>
> Signed-off-by: Byungchul Park <byungchul@...com>
> Reviewed-by: Toke Høiland-Jørgensen <toke@...hat.com>
> Reviewed-by: Pavel Begunkov <asml.silence@...il.com>
> Reviewed-by: Mina Almasry <almasrymina@...gle.com>
I'm not willy or David, but I believe I know enough about the memdesc plans
and as I worked on the struct slab, can confirm this goes in pretty much the
same direction, so looks good to me. As for struct net_iov I trust the
networking people know what they're doing there and it seems to make sense
to me too.
Reviewed-by: Vlastimil Babka <vbabka@...e.cz>
> ---
> include/net/netmem.h | 94 ++++++++++++++++++++++++++++++++++----------
> 1 file changed, 73 insertions(+), 21 deletions(-)
>
> diff --git a/include/net/netmem.h b/include/net/netmem.h
> index 386164fb9c18..2687c8051ca5 100644
> --- a/include/net/netmem.h
> +++ b/include/net/netmem.h
> @@ -12,6 +12,50 @@
> #include <linux/mm.h>
> #include <net/net_debug.h>
>
> +/* These fields in struct page are used by the page_pool and net stack:
> + *
> + * struct {
> + * unsigned long pp_magic;
> + * struct page_pool *pp;
> + * unsigned long _pp_mapping_pad;
> + * unsigned long dma_addr;
> + * atomic_long_t pp_ref_count;
> + * };
> + *
> + * We mirror the page_pool fields here so the page_pool can access these
> + * fields without worrying whether the underlying fields belong to a
> + * page or netmem_desc.
> + *
> + * CAUTION: Do not update the fields in netmem_desc without also
> + * updating the anonymous aliasing union in struct net_iov.
> + */
> +struct netmem_desc {
> + unsigned long _flags;
> + unsigned long pp_magic;
> + struct page_pool *pp;
> + unsigned long _pp_mapping_pad;
> + unsigned long dma_addr;
> + atomic_long_t pp_ref_count;
> +};
> +
> +#define NETMEM_DESC_ASSERT_OFFSET(pg, desc) \
> + static_assert(offsetof(struct page, pg) == \
> + offsetof(struct netmem_desc, desc))
> +NETMEM_DESC_ASSERT_OFFSET(flags, _flags);
> +NETMEM_DESC_ASSERT_OFFSET(pp_magic, pp_magic);
> +NETMEM_DESC_ASSERT_OFFSET(pp, pp);
> +NETMEM_DESC_ASSERT_OFFSET(_pp_mapping_pad, _pp_mapping_pad);
> +NETMEM_DESC_ASSERT_OFFSET(dma_addr, dma_addr);
> +NETMEM_DESC_ASSERT_OFFSET(pp_ref_count, pp_ref_count);
> +#undef NETMEM_DESC_ASSERT_OFFSET
> +
> +/*
> + * Since struct netmem_desc uses the space in struct page, the size
> + * should be checked, until struct netmem_desc has its own instance from
> + * slab, to avoid conflicting with other members within struct page.
> + */
> +static_assert(sizeof(struct netmem_desc) <= offsetof(struct page, _refcount));
> +
> /* net_iov */
>
> DECLARE_STATIC_KEY_FALSE(page_pool_mem_providers);
> @@ -31,12 +75,25 @@ enum net_iov_type {
> };
>
> struct net_iov {
> - enum net_iov_type type;
> - unsigned long pp_magic;
> - struct page_pool *pp;
> + union {
> + struct netmem_desc desc;
> +
> + /* XXX: The following part should be removed once all
> + * the references to them are converted so as to be
> + * accessed via netmem_desc e.g. niov->desc.pp instead
> + * of niov->pp.
> + */
> + struct {
> + unsigned long _flags;
> + unsigned long pp_magic;
> + struct page_pool *pp;
> + unsigned long _pp_mapping_pad;
> + unsigned long dma_addr;
> + atomic_long_t pp_ref_count;
> + };
> + };
> struct net_iov_area *owner;
> - unsigned long dma_addr;
> - atomic_long_t pp_ref_count;
> + enum net_iov_type type;
> };
>
> struct net_iov_area {
> @@ -48,27 +105,22 @@ struct net_iov_area {
> unsigned long base_virtual;
> };
>
> -/* These fields in struct page are used by the page_pool and net stack:
> +/* net_iov is union'ed with struct netmem_desc mirroring struct page, so
> + * the page_pool can access these fields without worrying whether the
> + * underlying fields are accessed via netmem_desc or directly via
> + * net_iov, until all the references to them are converted so as to be
> + * accessed via netmem_desc e.g. niov->desc.pp instead of niov->pp.
> *
> - * struct {
> - * unsigned long pp_magic;
> - * struct page_pool *pp;
> - * unsigned long _pp_mapping_pad;
> - * unsigned long dma_addr;
> - * atomic_long_t pp_ref_count;
> - * };
> - *
> - * We mirror the page_pool fields here so the page_pool can access these fields
> - * without worrying whether the underlying fields belong to a page or net_iov.
> - *
> - * The non-net stack fields of struct page are private to the mm stack and must
> - * never be mirrored to net_iov.
> + * The non-net stack fields of struct page are private to the mm stack
> + * and must never be mirrored to net_iov.
> */
> -#define NET_IOV_ASSERT_OFFSET(pg, iov) \
> - static_assert(offsetof(struct page, pg) == \
> +#define NET_IOV_ASSERT_OFFSET(desc, iov) \
> + static_assert(offsetof(struct netmem_desc, desc) == \
> offsetof(struct net_iov, iov))
> +NET_IOV_ASSERT_OFFSET(_flags, _flags);
> NET_IOV_ASSERT_OFFSET(pp_magic, pp_magic);
> NET_IOV_ASSERT_OFFSET(pp, pp);
> +NET_IOV_ASSERT_OFFSET(_pp_mapping_pad, _pp_mapping_pad);
> NET_IOV_ASSERT_OFFSET(dma_addr, dma_addr);
> NET_IOV_ASSERT_OFFSET(pp_ref_count, pp_ref_count);
> #undef NET_IOV_ASSERT_OFFSET
Powered by blists - more mailing lists