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@fastly.com>
Suggested-by: Jakub Kicinski <kuba@kernel.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;
+
 		break;
 	case NETDEV_QUEUE_TYPE_TX:
 		txq = netdev_get_tx_queue(netdev, q_idx);
 		if (nla_put_napi_id(rsp, txq->napi))
 			goto nla_put_failure;
+
+		if (txq->pool)
+			if (nla_put_empty_nest(rsp, NETDEV_A_QUEUE_XSK))
+				goto nla_put_failure;
+
+		break;
 	}
 
 	genlmsg_end(rsp, hdr);
diff --git a/tools/include/uapi/linux/netdev.h b/tools/include/uapi/linux/netdev.h
index 6c6ee183802d..4e82f3871473 100644
--- a/tools/include/uapi/linux/netdev.h
+++ b/tools/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)
-- 
2.43.0

Hi Joe,

kernel test robot noticed the following build errors:

[auto build test ERROR on 233a2b1480a0bdf6b40d4debf58a07084e9921ff]

url:    https://github.com/intel-lab-lkp/linux/commits/Joe-Damato/netlink-Add-nla_put_empty_nest-helper/20250208-121856
base:   233a2b1480a0bdf6b40d4debf58a07084e9921ff
patch link:    https://lore.kernel.org/r/20250208041248.111118-3-jdamato%40fastly.com
patch subject: [PATCH net-next v5 2/3] netdev-genl: Add an XSK attribute to queues
config: x86_64-kexec (https://download.01.org/0day-ci/archive/20250209/202502091844.PyraqTPE-lkp@intel.com/config)
compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project ab51eccf88f5321e7c60591c5546b254b6afab99)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250209/202502091844.PyraqTPE-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202502091844.PyraqTPE-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from net/core/netdev-genl.c:3:
   In file included from include/linux/netdevice.h:38:
   In file included from include/net/net_namespace.h:43:
   In file included from include/linux/skbuff.h:17:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:8:
   In file included from include/linux/cacheflush.h:5:
   In file included from arch/x86/include/asm/cacheflush.h:5:
   In file included from include/linux/mm.h:2224:
   include/linux/vmstat.h:504:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     504 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     505 |                            item];
         |                            ~~~~
   include/linux/vmstat.h:511:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     511 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     512 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
   include/linux/vmstat.h:524:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     524 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     525 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
>> net/core/netdev-genl.c:404:12: error: no member named 'pool' in 'struct netdev_rx_queue'
     404 |                 if (rxq->pool)
         |                     ~~~  ^
>> net/core/netdev-genl.c:414:12: error: no member named 'pool' in 'struct netdev_queue'
     414 |                 if (txq->pool)
         |                     ~~~  ^
   3 warnings and 2 errors generated.


vim +404 net/core/netdev-genl.c

   374	
   375	static int
   376	netdev_nl_queue_fill_one(struct sk_buff *rsp, struct net_device *netdev,
   377				 u32 q_idx, u32 q_type, const struct genl_info *info)
   378	{
   379		struct pp_memory_provider_params *params;
   380		struct netdev_rx_queue *rxq;
   381		struct netdev_queue *txq;
   382		void *hdr;
   383	
   384		hdr = genlmsg_iput(rsp, info);
   385		if (!hdr)
   386			return -EMSGSIZE;
   387	
   388		if (nla_put_u32(rsp, NETDEV_A_QUEUE_ID, q_idx) ||
   389		    nla_put_u32(rsp, NETDEV_A_QUEUE_TYPE, q_type) ||
   390		    nla_put_u32(rsp, NETDEV_A_QUEUE_IFINDEX, netdev->ifindex))
   391			goto nla_put_failure;
   392	
   393		switch (q_type) {
   394		case NETDEV_QUEUE_TYPE_RX:
   395			rxq = __netif_get_rx_queue(netdev, q_idx);
   396			if (nla_put_napi_id(rsp, rxq->napi))
   397				goto nla_put_failure;
   398	
   399			params = &rxq->mp_params;
   400			if (params->mp_ops &&
   401			    params->mp_ops->nl_fill(params->mp_priv, rsp, rxq))
   402				goto nla_put_failure;
   403	
 > 404			if (rxq->pool)
   405				if (nla_put_empty_nest(rsp, NETDEV_A_QUEUE_XSK))
   406					goto nla_put_failure;
   407	
   408			break;
   409		case NETDEV_QUEUE_TYPE_TX:
   410			txq = netdev_get_tx_queue(netdev, q_idx);
   411			if (nla_put_napi_id(rsp, txq->napi))
   412				goto nla_put_failure;
   413	
 > 414			if (txq->pool)
   415				if (nla_put_empty_nest(rsp, NETDEV_A_QUEUE_XSK))
   416					goto nla_put_failure;
   417	
   418			break;
   419		}
   420	
   421		genlmsg_end(rsp, hdr);
   422	
   423		return 0;
   424	
   425	nla_put_failure:
   426		genlmsg_cancel(rsp, hdr);
   427		return -EMSGSIZE;
   428	}
   429	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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@fastly.com>
> Suggested-by: Jakub Kicinski <kuba@kernel.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)
      |                        ^~


---
pw-bot: cr

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@fastly.com>
> > Suggested-by: Jakub Kicinski <kuba@kernel.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 ;)

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@fastly.com>
> > > Suggested-by: Jakub Kicinski <kuba@kernel.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.

On Mon, Feb 10, 2025 at 11:08:10AM -0800, Stanislav Fomichev wrote:
> 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@fastly.com>
> > > > Suggested-by: Jakub Kicinski <kuba@kernel.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.

Yea, I saw that. OK, I'll just drop the ifdefs in and see what
happens.

Thanks for the review; appreciate your time and energy.