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: <CAHS8izPB=x1ZYhan-sjuA8ofbHmxbHJrSbtvN3z3zfziBMmMdA@mail.gmail.com> Date: Thu, 17 Aug 2023 14:21:26 -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, Willem de Bruijn <willemb@...gle.com>, Praveen Kaligineedi <pkaligineedi@...gle.com> Subject: Re: [RFC net-next 00/13] net: page_pool: add netlink-based introspection On Wed, Aug 16, 2023 at 4:43 PM Jakub Kicinski <kuba@...nel.org> wrote: > > As the page pool functionality grows we will be increasingly > constrained by the lack of any uAPI. This series is a first > step towards such a uAPI, it creates a way for users to query > information about page pools and associated statistics. > > I'm purposefully exposing only the information which I'd > find useful when monitoring production workloads. > > For the SET part (which to be clear isn't addressed by this > series at all) I think we'll need to turn to something more > along the lines of "netdev-level policy". Instead of configuring > page pools, which are somewhat ephemeral, and hard to change > at runtime, we should set the "expected page pool parameters" > at the netdev level, and have the driver consult those when > instantiating pools. My ramblings from yesterday about Queue API > may make this more or less clear... > https://lore.kernel.org/all/20230815171638.4c057dcd@kernel.org/ > The patches themselves look good to me, and I'll provide Reviewed-by for the ones I feel I understand well enough to review, but I'm a bit unsure about exposing specifically page_pool stats to the user. Isn't it better to expose rx-queue stats (slightly more generic) to the user? In my mind, the advantages: - we could implement better SET apis that allocate, delete, or reconfigure rx-queues. APIs that allocate or delete page-pool make less sense semantically maybe? The page-pool doesn't decide to instantiate itself. - The api can be extended for non-page-pool stats (per rx-queue dropped packets maybe, or something like that). - The api maybe can be extended to non-page-pool drivers. The driver may be able to implement their own function to provide equivalent stats (although this one may not be that important). - rx-queue GET API fits in nicely with what you described yesterday [1]. At the moment I'm a bit unsure because the SET api you described yesterday sounded per-rx-queue to me. But the GET api here is per-page-pool based. Maybe the distinction doesn't matter? Maybe you're thinking they're unrelated APIs? [1] https://lore.kernel.org/all/20230815171638.4c057dcd@kernel.org/ > Jakub Kicinski (13): > net: page_pool: split the page_pool_params into fast and slow > net: page_pool: avoid touching slow on the fastpath > net: page_pool: factor out uninit > net: page_pool: id the page pools > net: page_pool: record pools per netdev > net: page_pool: stash the NAPI ID for easier access > eth: link netdev to pp > net: page_pool: add nlspec for basic access to page pools > net: page_pool: implement GET in the netlink API > net: page_pool: add netlink notifications for state changes > net: page_pool: report when page pool was destroyed > net: page_pool: expose page pool stats via netlink > tools: netdev: regen after page pool changes > > Documentation/netlink/specs/netdev.yaml | 158 +++++++ > Documentation/networking/page_pool.rst | 5 +- > drivers/net/ethernet/broadcom/bnxt/bnxt.c | 1 + > .../net/ethernet/mellanox/mlx5/core/en_main.c | 1 + > drivers/net/ethernet/microsoft/mana/mana_en.c | 1 + > include/linux/list.h | 20 + > include/linux/netdevice.h | 4 + > include/net/page_pool/helpers.h | 8 +- > include/net/page_pool/types.h | 43 +- > include/uapi/linux/netdev.h | 37 ++ > net/core/Makefile | 2 +- > net/core/netdev-genl-gen.c | 41 ++ > net/core/netdev-genl-gen.h | 11 + > net/core/page_pool.c | 56 ++- > net/core/page_pool_priv.h | 12 + > net/core/page_pool_user.c | 409 +++++++++++++++++ > tools/include/uapi/linux/netdev.h | 37 ++ > tools/net/ynl/generated/netdev-user.c | 415 ++++++++++++++++++ > tools/net/ynl/generated/netdev-user.h | 169 +++++++ > 19 files changed, 1391 insertions(+), 39 deletions(-) > create mode 100644 net/core/page_pool_priv.h > create mode 100644 net/core/page_pool_user.c > > -- > 2.41.0 > -- Thanks, Mina
Powered by blists - more mailing lists