| 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: <20250530011002.GA3093@system.software.com>
Date: Fri, 30 May 2025 10:10:02 +0900
From: Byungchul Park <byungchul@...com>
To: Mina Almasry <almasrymina@...gle.com>
Cc: willy@...radead.org, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-mm@...ck.org,
kernel_team@...ynix.com, kuba@...nel.org,
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, vbabka@...e.cz, 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: [RFC v3 01/18] netmem: introduce struct netmem_desc mirroring
struct page
On Thu, May 29, 2025 at 09:31:40AM -0700, Mina Almasry wrote:
> On Wed, May 28, 2025 at 8:11 PM Byungchul Park <byungchul@...com> wrote:
> > struct net_iov {
> > - enum net_iov_type type;
> > - unsigned long pp_magic;
> > - struct page_pool *pp;
> > - struct net_iov_area *owner;
> > - unsigned long dma_addr;
> > - atomic_long_t pp_ref_count;
> > + 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.
> > + *
> > + * Plus, once struct netmem_desc has it own instance
> > + * from slab, network's fields of the following can be
> > + * moved out of struct netmem_desc like:
> > + *
> > + * struct net_iov {
> > + * struct netmem_desc desc;
> > + * struct net_iov_area *owner;
> > + * ...
> > + * };
> > + */
>
> We do not need to wait until netmem_desc has its own instance from
> slab to move the net_iov-specific fields out of netmem_desc. We can do
> that now, because there are no size restrictions on net_iov.
Got it. Thanks for explanation.
> So, I recommend change this to:
>
> struct net_iov {
> /* Union for anonymous aliasing: */
> union {
> struct netmem_desc desc;
> 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;
> enum net_iov_type type;
> };
Do you mean?
struct net_iov {
/* Union for anonymous aliasing: */
union {
struct netmem_desc desc;
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;
enum net_iov_type type;
};
Right? If so, I will.
> > struct net_iov_area {
> > @@ -48,27 +110,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, type);
>
> Remove this assertion.
I will.
>
> > NET_IOV_ASSERT_OFFSET(pp_magic, pp_magic);
> > NET_IOV_ASSERT_OFFSET(pp, pp);
> > +NET_IOV_ASSERT_OFFSET(_pp_mapping_pad, owner);
>
> And this one.
I will.
> (_flags, type) and (_pp_mapping_pad, owner) have very different
> semantics and usage, we should not assert they are not the same
> offset. However (pp, pp) and (pp_magic,pp_magic) have the same
> semantics and usage, so we do assert they are at the same offset.
>
> Code is allowed to access __netmem_clear_lsb(netmem)->pp or
> __netmem_clear_lsb(netmem)->pp_magic without caring what's the
> underlying memory type because both fields have the same semantics and
> usage.
>
> Code should *not* assume it can access
> __netmem_clear_lsb(netmem)->owner or __netmem_clear_lsb(netmem)->type
> without doing a check whether the underlying memory is
> page/netmem_desc or net_iov. These fields are only usable for net_iov,
Sounds good. Thanks.
Byungchul
> so let's explicitly move them to a different place.
>
> --
> Thanks,
> Mina
Powered by blists - more mailing lists