| 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: <Z6pOmivi7Q402eGu@mini-arch>
Date: Mon, 10 Feb 2025 11:08:10 -0800
From: Stanislav Fomichev <stfomichev@...il.com>
To: Joe Damato <jdamato@...tly.com>, netdev@...r.kernel.org,
horms@...nel.org, kuba@...nel.org,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>,
Donald Hunter <donald.hunter@...il.com>,
Andrew Lunn <andrew+netdev@...n.ch>,
Xuan Zhuo <xuanzhuo@...ux.alibaba.com>,
Stanislav Fomichev <sdf@...ichev.me>,
Mina Almasry <almasrymina@...gle.com>,
Martin Karsten <mkarsten@...terloo.ca>,
Sridhar Samudrala <sridhar.samudrala@...el.com>,
David Wei <dw@...idwei.uk>,
open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net-next v5 2/3] netdev-genl: Add an XSK attribute to
queues
On 02/10, Joe Damato wrote:
> On Sat, Feb 08, 2025 at 05:43:47PM -0800, Stanislav Fomichev wrote:
> > On 02/08, Joe Damato wrote:
> > > Expose a new per-queue nest attribute, xsk, which will be present for
> > > queues that are being used for AF_XDP. If the queue is not being used for
> > > AF_XDP, the nest will not be present.
> > >
> > > In the future, this attribute can be extended to include more data about
> > > XSK as it is needed.
> > >
> > > Signed-off-by: Joe Damato <jdamato@...tly.com>
> > > Suggested-by: Jakub Kicinski <kuba@...nel.org>
> > > ---
> > > v5:
> > > - Removed unused variable, ret, from netdev_nl_queue_fill_one.
> > >
> > > v4:
> > > - Updated netdev_nl_queue_fill_one to use the empty nest helper added
> > > in patch 1.
> > >
> > > v2:
> > > - Patch adjusted to include an attribute, xsk, which is an empty nest
> > > and exposed for queues which have a pool.
> > >
> > > Documentation/netlink/specs/netdev.yaml | 13 ++++++++++++-
> > > include/uapi/linux/netdev.h | 6 ++++++
> > > net/core/netdev-genl.c | 11 +++++++++++
> > > tools/include/uapi/linux/netdev.h | 6 ++++++
> > > 4 files changed, 35 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/Documentation/netlink/specs/netdev.yaml b/Documentation/netlink/specs/netdev.yaml
> > > index 288923e965ae..85402a2e289c 100644
> > > --- a/Documentation/netlink/specs/netdev.yaml
> > > +++ b/Documentation/netlink/specs/netdev.yaml
> > > @@ -276,6 +276,9 @@ attribute-sets:
> > > doc: The timeout, in nanoseconds, of how long to suspend irq
> > > processing, if event polling finds events
> > > type: uint
> > > + -
> > > + name: xsk-info
> > > + attributes: []
> > > -
> > > name: queue
> > > attributes:
> > > @@ -294,6 +297,9 @@ attribute-sets:
> > > -
> > > name: type
> > > doc: Queue type as rx, tx. Each queue type defines a separate ID space.
> > > + XDP TX queues allocated in the kernel are not linked to NAPIs and
> > > + thus not listed. AF_XDP queues will have more information set in
> > > + the xsk attribute.
> > > type: u32
> > > enum: queue-type
> > > -
> > > @@ -309,7 +315,11 @@ attribute-sets:
> > > doc: io_uring memory provider information.
> > > type: nest
> > > nested-attributes: io-uring-provider-info
> > > -
> > > + -
> > > + name: xsk
> > > + doc: XSK information for this queue, if any.
> > > + type: nest
> > > + nested-attributes: xsk-info
> > > -
> > > name: qstats
> > > doc: |
> > > @@ -652,6 +662,7 @@ operations:
> > > - ifindex
> > > - dmabuf
> > > - io-uring
> > > + - xsk
> > > dump:
> > > request:
> > > attributes:
> > > diff --git a/include/uapi/linux/netdev.h b/include/uapi/linux/netdev.h
> > > index 6c6ee183802d..4e82f3871473 100644
> > > --- a/include/uapi/linux/netdev.h
> > > +++ b/include/uapi/linux/netdev.h
> > > @@ -136,6 +136,11 @@ enum {
> > > NETDEV_A_NAPI_MAX = (__NETDEV_A_NAPI_MAX - 1)
> > > };
> > >
> > > +enum {
> > > + __NETDEV_A_XSK_INFO_MAX,
> > > + NETDEV_A_XSK_INFO_MAX = (__NETDEV_A_XSK_INFO_MAX - 1)
> > > +};
> > > +
> > > enum {
> > > NETDEV_A_QUEUE_ID = 1,
> > > NETDEV_A_QUEUE_IFINDEX,
> > > @@ -143,6 +148,7 @@ enum {
> > > NETDEV_A_QUEUE_NAPI_ID,
> > > NETDEV_A_QUEUE_DMABUF,
> > > NETDEV_A_QUEUE_IO_URING,
> > > + NETDEV_A_QUEUE_XSK,
> > >
> > > __NETDEV_A_QUEUE_MAX,
> > > NETDEV_A_QUEUE_MAX = (__NETDEV_A_QUEUE_MAX - 1)
> > > diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
> > > index 0dcd4faefd8d..b5a93a449af9 100644
> > > --- a/net/core/netdev-genl.c
> > > +++ b/net/core/netdev-genl.c
> > > @@ -400,11 +400,22 @@ netdev_nl_queue_fill_one(struct sk_buff *rsp, struct net_device *netdev,
> > > if (params->mp_ops &&
> > > params->mp_ops->nl_fill(params->mp_priv, rsp, rxq))
> > > goto nla_put_failure;
> > > +
> > > + if (rxq->pool)
> > > + if (nla_put_empty_nest(rsp, NETDEV_A_QUEUE_XSK))
> > > + goto nla_put_failure;
> >
> > Needs to be guarded by ifdef CONFIG_XDP_SOCKETS?
> >
> >
> > net/core/netdev-genl.c: In function `netdev_nl_queue_fill_one´:
> > net/core/netdev-genl.c:404:24: error: `struct netdev_rx_queue´ has no member named `pool´
> > 404 | if (rxq->pool)
> > | ^~
> > net/core/netdev-genl.c:414:24: error: `struct netdev_queue´ has no member named `pool´
> > 414 | if (txq->pool)
> > | ^~
>
> Ah, thanks.
>
> I'm trying to decide if it'll look better factored out into helpers
> vs just dropping the #ifdefs in netdev_nl_queue_fill_one.
>
> Open to opinions so that hopefully v6 will be the last one ;)
Might be too much boilerplate for the helpers? (assuming you
want empty helpers for #else case). The only other place that tests
rxq->pool is devmem (net/core/devmem.c) and it uses simple ifdef
in place.
Powered by blists - more mailing lists