netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [net-next PATCH v8 00/10] Introduce queue and NAPI support in netdev-genl (Was: Introduce NAPI queues support)
@ 2023-11-17  1:16 Amritha Nambiar
  2023-11-17  1:16 ` [net-next PATCH v8 01/10] netdev-genl: spec: Extend netdev netlink spec in YAML for queue Amritha Nambiar
                   ` (10 more replies)
  0 siblings, 11 replies; 21+ messages in thread
From: Amritha Nambiar @ 2023-11-17  1:16 UTC (permalink / raw)
  To: netdev, kuba, pabeni; +Cc: sridhar.samudrala, amritha.nambiar

Add the capability to export the following via netdev-genl interface:
- queue information supported by the device
- NAPI information supported by the device

Introduce support for associating queue and NAPI instance.
Extend the netdev_genl generic netlink family for netdev
with queue and NAPI data. 

The queue parameters exposed are:
- queue index
- queue type
- ifindex
- NAPI id associated with the queue

Additional rx and tx queue parameters can be exposed in follow up
patches by stashing them in netdev queue structures. XDP queue type
can also be supported in future.

The NAPI fields exposed are:
- NAPI id
- NAPI device ifindex
- Interrupt number associated with the NAPI instance
- PID for the NAPI thread

This series only supports 'get' ability for retrieving
certain queue and NAPI attributes. The 'set' ability for
configuring queue and associated NAPI instance via netdev-genl
will be submitted as a separate patch series.

Previous discussion at:
https://lore.kernel.org/netdev/c8476530638a5f4381d64db0e024ed49c2db3b02.camel@gmail.com/T/#m00999652a8b4731fbdb7bf698d2e3666c65a60e7

$ ./cli.py --spec netdev.yaml --do queue-get  --json='{"ifindex": 12, "id": 0, "type": 0}'
{'id': 0, 'ifindex': 12, 'napi-id': 593, 'type': 'rx'}

$ ./cli.py --spec netdev.yaml  --do queue-get --json='{"ifindex": 12, "id": 0, "type": 1}'
{'id': 0, 'ifindex': 12, 'napi-id': 593, 'type': 'tx'}

$ ./cli.py --spec netdev.yaml  --dump queue-get --json='{"ifindex": 12}'
[{'id': 0, 'ifindex': 12, 'napi-id': 593, 'type': 'rx'},
 {'id': 1, 'ifindex': 12, 'napi-id': 594, 'type': 'rx'},
 {'id': 2, 'ifindex': 12, 'napi-id': 595, 'type': 'rx'},
 {'id': 3, 'ifindex': 12, 'napi-id': 596, 'type': 'rx'},
 {'id': 0, 'ifindex': 12, 'napi-id': 593, 'type': 'tx'},
 {'id': 1, 'ifindex': 12, 'napi-id': 594, 'type': 'tx'},
 {'id': 2, 'ifindex': 12, 'napi-id': 595, 'type': 'tx'},
 {'id': 3, 'ifindex': 12, 'napi-id': 596, 'type': 'tx'}]

$ ./cli.py --spec netdev.yaml --do napi-get --json='{"id": 593}'
{'id': 593, 'ifindex': 12, 'irq': 291, 'pid': 3727}

$ ./cli.py --spec netdev.yaml --dump napi-get --json='{"ifindex": 12}'
[{'id': 596, 'ifindex': 12, 'irq': 294, 'pid': 3724},
 {'id': 595, 'ifindex': 12, 'irq': 293, 'pid': 3725},
 {'id': 594, 'ifindex': 12, 'irq': 292, 'pid': 3726},
 {'id': 593, 'ifindex': 12, 'irq': 291, 'pid': 3727}]

v7 -> v8
* Removed $obj prefix from attribute names in yaml spec

v6 -> v7
* Added more documentation in spec file
* Addressed other review comments related to lock

v5 -> v6
* Fixed build warning in prototype for ice_queue_set_napi()

v4 -> v5
* Removed tx_maxrate in queue atrributes
* Added lock protection for queue->napi
* Addressed other review comments

v3 -> v4
Minor nits, changed function name, used list_for_each_entry in place
of _rcu

v2 -> v3
* Implemented queue as separate netlink object
with support for exposing per-queue paramters
* Removed queue-list associations with NAPI
* Addressed other review feedback WRT tracking list
iterations

v1 -> v2
* Removed multi-attr nest for NAPI object
* Added support for flat/individual NAPI objects
* Changed 'do' command to take napi-id as argument
* Supported filtered 'dump' (dump with ifindex for a netdev and dump for
  all netdevs)

RFC -> v1
* Changed to separate 'napi_get' command
* Added support to expose interrupt and PID for the NAPI
* Used list of netdev queue structs
* Split patches further and fixed code style and errors

---

Amritha Nambiar (10):
      netdev-genl: spec: Extend netdev netlink spec in YAML for queue
      net: Add queue and napi association
      ice: Add support in the driver for associating queue with napi
      netdev-genl: Add netlink framework functions for queue
      netdev-genl: spec: Extend netdev netlink spec in YAML for NAPI
      netdev-genl: Add netlink framework functions for napi
      netdev-genl: spec: Add irq in netdev netlink YAML spec
      net: Add NAPI IRQ support
      netdev-genl: spec: Add PID in netdev netlink YAML spec
      netdev-genl: Add PID for the NAPI thread


 Documentation/netlink/specs/netdev.yaml   |   94 +++++++++
 drivers/net/ethernet/intel/ice/ice_lib.c  |   64 ++++++
 drivers/net/ethernet/intel/ice/ice_lib.h  |    4 
 drivers/net/ethernet/intel/ice/ice_main.c |    4 
 include/linux/netdevice.h                 |   17 ++
 include/net/netdev_rx_queue.h             |    4 
 include/uapi/linux/netdev.h               |   27 ++
 net/core/dev.c                            |   49 ++++
 net/core/dev.h                            |    2 
 net/core/netdev-genl-gen.c                |   50 +++++
 net/core/netdev-genl-gen.h                |    5 
 net/core/netdev-genl.c                    |  320 +++++++++++++++++++++++++++++
 tools/include/uapi/linux/netdev.h         |   27 ++
 tools/net/ynl/generated/netdev-user.c     |  289 ++++++++++++++++++++++++++
 tools/net/ynl/generated/netdev-user.h     |  178 ++++++++++++++++
 15 files changed, 1130 insertions(+), 4 deletions(-)

--

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [net-next PATCH v8 01/10] netdev-genl: spec: Extend netdev netlink spec in YAML for queue
  2023-11-17  1:16 [net-next PATCH v8 00/10] Introduce queue and NAPI support in netdev-genl (Was: Introduce NAPI queues support) Amritha Nambiar
@ 2023-11-17  1:16 ` Amritha Nambiar
  2023-11-17  1:16 ` [net-next PATCH v8 02/10] net: Add queue and napi association Amritha Nambiar
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 21+ messages in thread
From: Amritha Nambiar @ 2023-11-17  1:16 UTC (permalink / raw)
  To: netdev, kuba, pabeni; +Cc: sridhar.samudrala, amritha.nambiar

Add support in netlink spec(netdev.yaml) for queue information.
Add code generated from the spec.

Note: The "queue-type" attribute takes values 0 and 1 for rx
and tx queue type respectively.

Signed-off-by: Amritha Nambiar <amritha.nambiar@intel.com>
Reviewed-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
---
 Documentation/netlink/specs/netdev.yaml |   52 +++++++++++
 include/uapi/linux/netdev.h             |   16 +++
 net/core/netdev-genl-gen.c              |   26 +++++
 net/core/netdev-genl-gen.h              |    3 +
 net/core/netdev-genl.c                  |   10 ++
 tools/include/uapi/linux/netdev.h       |   16 +++
 tools/net/ynl/generated/netdev-user.c   |  153 +++++++++++++++++++++++++++++++
 tools/net/ynl/generated/netdev-user.h   |   99 ++++++++++++++++++++
 8 files changed, 375 insertions(+)

diff --git a/Documentation/netlink/specs/netdev.yaml b/Documentation/netlink/specs/netdev.yaml
index 14511b13f305..f9382b7705ab 100644
--- a/Documentation/netlink/specs/netdev.yaml
+++ b/Documentation/netlink/specs/netdev.yaml
@@ -55,6 +55,10 @@ definitions:
         name: hash
         doc:
           Device is capable of exposing receive packet hash via bpf_xdp_metadata_rx_hash().
+  -
+    name: queue-type
+    type: enum
+    entries: [ rx, tx ]
 
 attribute-sets:
   -
@@ -87,6 +91,31 @@ attribute-sets:
         type: u64
         enum: xdp-rx-metadata
 
+  -
+    name: queue
+    attributes:
+      -
+        name: id
+        doc: Queue index; most queue types are indexed like a C array, with
+             indexes starting at 0 and ending at queue count - 1. Queue indexes
+             are scoped to an interface and queue type.
+        type: u32
+      -
+        name: ifindex
+        doc: ifindex of the netdevice to which the queue belongs.
+        type: u32
+        checks:
+          min: 1
+      -
+        name: type
+        doc: Queue type as rx, tx. Each queue type defines a separate ID space.
+        type: u32
+        enum: queue-type
+      -
+        name: napi-id
+        doc: ID of the NAPI instance which services this queue.
+        type: u32
+
 operations:
   list:
     -
@@ -120,6 +149,29 @@ operations:
       doc: Notification about device configuration being changed.
       notify: dev-get
       mcgrp: mgmt
+    -
+      name: queue-get
+      doc: Get queue information from the kernel.
+           Only configured queues will be reported (as opposed to all available
+           hardware queues).
+      attribute-set: queue
+      do:
+        request:
+          attributes:
+            - ifindex
+            - type
+            - id
+        reply: &queue-get-op
+          attributes:
+            - id
+            - type
+            - napi-id
+            - ifindex
+      dump:
+        request:
+          attributes:
+            - ifindex
+        reply: *queue-get-op
 
 mcast-groups:
   list:
diff --git a/include/uapi/linux/netdev.h b/include/uapi/linux/netdev.h
index 2943a151d4f1..c440017241f0 100644
--- a/include/uapi/linux/netdev.h
+++ b/include/uapi/linux/netdev.h
@@ -53,6 +53,11 @@ enum netdev_xdp_rx_metadata {
 	NETDEV_XDP_RX_METADATA_MASK = 3,
 };
 
+enum netdev_queue_type {
+	NETDEV_QUEUE_TYPE_RX,
+	NETDEV_QUEUE_TYPE_TX,
+};
+
 enum {
 	NETDEV_A_DEV_IFINDEX = 1,
 	NETDEV_A_DEV_PAD,
@@ -64,11 +69,22 @@ enum {
 	NETDEV_A_DEV_MAX = (__NETDEV_A_DEV_MAX - 1)
 };
 
+enum {
+	NETDEV_A_QUEUE_ID = 1,
+	NETDEV_A_QUEUE_IFINDEX,
+	NETDEV_A_QUEUE_TYPE,
+	NETDEV_A_QUEUE_NAPI_ID,
+
+	__NETDEV_A_QUEUE_MAX,
+	NETDEV_A_QUEUE_MAX = (__NETDEV_A_QUEUE_MAX - 1)
+};
+
 enum {
 	NETDEV_CMD_DEV_GET = 1,
 	NETDEV_CMD_DEV_ADD_NTF,
 	NETDEV_CMD_DEV_DEL_NTF,
 	NETDEV_CMD_DEV_CHANGE_NTF,
+	NETDEV_CMD_QUEUE_GET,
 
 	__NETDEV_CMD_MAX,
 	NETDEV_CMD_MAX = (__NETDEV_CMD_MAX - 1)
diff --git a/net/core/netdev-genl-gen.c b/net/core/netdev-genl-gen.c
index ea9231378aa6..97ee7ebbad57 100644
--- a/net/core/netdev-genl-gen.c
+++ b/net/core/netdev-genl-gen.c
@@ -15,6 +15,18 @@ static const struct nla_policy netdev_dev_get_nl_policy[NETDEV_A_DEV_IFINDEX + 1
 	[NETDEV_A_DEV_IFINDEX] = NLA_POLICY_MIN(NLA_U32, 1),
 };
 
+/* NETDEV_CMD_QUEUE_GET - do */
+static const struct nla_policy netdev_queue_get_do_nl_policy[NETDEV_A_QUEUE_TYPE + 1] = {
+	[NETDEV_A_QUEUE_IFINDEX] = NLA_POLICY_MIN(NLA_U32, 1),
+	[NETDEV_A_QUEUE_TYPE] = NLA_POLICY_MAX(NLA_U32, 1),
+	[NETDEV_A_QUEUE_ID] = { .type = NLA_U32, },
+};
+
+/* NETDEV_CMD_QUEUE_GET - dump */
+static const struct nla_policy netdev_queue_get_dump_nl_policy[NETDEV_A_QUEUE_IFINDEX + 1] = {
+	[NETDEV_A_QUEUE_IFINDEX] = NLA_POLICY_MIN(NLA_U32, 1),
+};
+
 /* Ops table for netdev */
 static const struct genl_split_ops netdev_nl_ops[] = {
 	{
@@ -29,6 +41,20 @@ static const struct genl_split_ops netdev_nl_ops[] = {
 		.dumpit	= netdev_nl_dev_get_dumpit,
 		.flags	= GENL_CMD_CAP_DUMP,
 	},
+	{
+		.cmd		= NETDEV_CMD_QUEUE_GET,
+		.doit		= netdev_nl_queue_get_doit,
+		.policy		= netdev_queue_get_do_nl_policy,
+		.maxattr	= NETDEV_A_QUEUE_TYPE,
+		.flags		= GENL_CMD_CAP_DO,
+	},
+	{
+		.cmd		= NETDEV_CMD_QUEUE_GET,
+		.dumpit		= netdev_nl_queue_get_dumpit,
+		.policy		= netdev_queue_get_dump_nl_policy,
+		.maxattr	= NETDEV_A_QUEUE_IFINDEX,
+		.flags		= GENL_CMD_CAP_DUMP,
+	},
 };
 
 static const struct genl_multicast_group netdev_nl_mcgrps[] = {
diff --git a/net/core/netdev-genl-gen.h b/net/core/netdev-genl-gen.h
index 7b370c073e7d..263c94f77bad 100644
--- a/net/core/netdev-genl-gen.h
+++ b/net/core/netdev-genl-gen.h
@@ -13,6 +13,9 @@
 
 int netdev_nl_dev_get_doit(struct sk_buff *skb, struct genl_info *info);
 int netdev_nl_dev_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb);
+int netdev_nl_queue_get_doit(struct sk_buff *skb, struct genl_info *info);
+int netdev_nl_queue_get_dumpit(struct sk_buff *skb,
+			       struct netlink_callback *cb);
 
 enum {
 	NETDEV_NLGRP_MGMT,
diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
index fe61f85bcf33..336c608e6a6b 100644
--- a/net/core/netdev-genl.c
+++ b/net/core/netdev-genl.c
@@ -129,6 +129,16 @@ int netdev_nl_dev_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
 	return skb->len;
 }
 
+int netdev_nl_queue_get_doit(struct sk_buff *skb, struct genl_info *info)
+{
+	return -EOPNOTSUPP;
+}
+
+int netdev_nl_queue_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
+{
+	return -EOPNOTSUPP;
+}
+
 static int netdev_genl_netdevice_event(struct notifier_block *nb,
 				       unsigned long event, void *ptr)
 {
diff --git a/tools/include/uapi/linux/netdev.h b/tools/include/uapi/linux/netdev.h
index 2943a151d4f1..c440017241f0 100644
--- a/tools/include/uapi/linux/netdev.h
+++ b/tools/include/uapi/linux/netdev.h
@@ -53,6 +53,11 @@ enum netdev_xdp_rx_metadata {
 	NETDEV_XDP_RX_METADATA_MASK = 3,
 };
 
+enum netdev_queue_type {
+	NETDEV_QUEUE_TYPE_RX,
+	NETDEV_QUEUE_TYPE_TX,
+};
+
 enum {
 	NETDEV_A_DEV_IFINDEX = 1,
 	NETDEV_A_DEV_PAD,
@@ -64,11 +69,22 @@ enum {
 	NETDEV_A_DEV_MAX = (__NETDEV_A_DEV_MAX - 1)
 };
 
+enum {
+	NETDEV_A_QUEUE_ID = 1,
+	NETDEV_A_QUEUE_IFINDEX,
+	NETDEV_A_QUEUE_TYPE,
+	NETDEV_A_QUEUE_NAPI_ID,
+
+	__NETDEV_A_QUEUE_MAX,
+	NETDEV_A_QUEUE_MAX = (__NETDEV_A_QUEUE_MAX - 1)
+};
+
 enum {
 	NETDEV_CMD_DEV_GET = 1,
 	NETDEV_CMD_DEV_ADD_NTF,
 	NETDEV_CMD_DEV_DEL_NTF,
 	NETDEV_CMD_DEV_CHANGE_NTF,
+	NETDEV_CMD_QUEUE_GET,
 
 	__NETDEV_CMD_MAX,
 	NETDEV_CMD_MAX = (__NETDEV_CMD_MAX - 1)
diff --git a/tools/net/ynl/generated/netdev-user.c b/tools/net/ynl/generated/netdev-user.c
index b5ffe8cd1144..87845c707b95 100644
--- a/tools/net/ynl/generated/netdev-user.c
+++ b/tools/net/ynl/generated/netdev-user.c
@@ -18,6 +18,7 @@ static const char * const netdev_op_strmap[] = {
 	[NETDEV_CMD_DEV_ADD_NTF] = "dev-add-ntf",
 	[NETDEV_CMD_DEV_DEL_NTF] = "dev-del-ntf",
 	[NETDEV_CMD_DEV_CHANGE_NTF] = "dev-change-ntf",
+	[NETDEV_CMD_QUEUE_GET] = "queue-get",
 };
 
 const char *netdev_op_str(int op)
@@ -58,6 +59,18 @@ const char *netdev_xdp_rx_metadata_str(enum netdev_xdp_rx_metadata value)
 	return netdev_xdp_rx_metadata_strmap[value];
 }
 
+static const char * const netdev_queue_type_strmap[] = {
+	[0] = "rx",
+	[1] = "tx",
+};
+
+const char *netdev_queue_type_str(enum netdev_queue_type value)
+{
+	if (value < 0 || value >= (int)MNL_ARRAY_SIZE(netdev_queue_type_strmap))
+		return NULL;
+	return netdev_queue_type_strmap[value];
+}
+
 /* Policies */
 struct ynl_policy_attr netdev_dev_policy[NETDEV_A_DEV_MAX + 1] = {
 	[NETDEV_A_DEV_IFINDEX] = { .name = "ifindex", .type = YNL_PT_U32, },
@@ -72,6 +85,18 @@ struct ynl_policy_nest netdev_dev_nest = {
 	.table = netdev_dev_policy,
 };
 
+struct ynl_policy_attr netdev_queue_policy[NETDEV_A_QUEUE_MAX + 1] = {
+	[NETDEV_A_QUEUE_ID] = { .name = "id", .type = YNL_PT_U32, },
+	[NETDEV_A_QUEUE_IFINDEX] = { .name = "ifindex", .type = YNL_PT_U32, },
+	[NETDEV_A_QUEUE_TYPE] = { .name = "type", .type = YNL_PT_U32, },
+	[NETDEV_A_QUEUE_NAPI_ID] = { .name = "napi-id", .type = YNL_PT_U32, },
+};
+
+struct ynl_policy_nest netdev_queue_nest = {
+	.max_attr = NETDEV_A_QUEUE_MAX,
+	.table = netdev_queue_policy,
+};
+
 /* Common nested types */
 /* ============== NETDEV_CMD_DEV_GET ============== */
 /* NETDEV_CMD_DEV_GET - do */
@@ -197,6 +222,134 @@ void netdev_dev_get_ntf_free(struct netdev_dev_get_ntf *rsp)
 	free(rsp);
 }
 
+/* ============== NETDEV_CMD_QUEUE_GET ============== */
+/* NETDEV_CMD_QUEUE_GET - do */
+void netdev_queue_get_req_free(struct netdev_queue_get_req *req)
+{
+	free(req);
+}
+
+void netdev_queue_get_rsp_free(struct netdev_queue_get_rsp *rsp)
+{
+	free(rsp);
+}
+
+int netdev_queue_get_rsp_parse(const struct nlmsghdr *nlh, void *data)
+{
+	struct ynl_parse_arg *yarg = data;
+	struct netdev_queue_get_rsp *dst;
+	const struct nlattr *attr;
+
+	dst = yarg->data;
+
+	mnl_attr_for_each(attr, nlh, sizeof(struct genlmsghdr)) {
+		unsigned int type = mnl_attr_get_type(attr);
+
+		if (type == NETDEV_A_QUEUE_ID) {
+			if (ynl_attr_validate(yarg, attr))
+				return MNL_CB_ERROR;
+			dst->_present.id = 1;
+			dst->id = mnl_attr_get_u32(attr);
+		} else if (type == NETDEV_A_QUEUE_TYPE) {
+			if (ynl_attr_validate(yarg, attr))
+				return MNL_CB_ERROR;
+			dst->_present.type = 1;
+			dst->type = mnl_attr_get_u32(attr);
+		} else if (type == NETDEV_A_QUEUE_NAPI_ID) {
+			if (ynl_attr_validate(yarg, attr))
+				return MNL_CB_ERROR;
+			dst->_present.napi_id = 1;
+			dst->napi_id = mnl_attr_get_u32(attr);
+		} else if (type == NETDEV_A_QUEUE_IFINDEX) {
+			if (ynl_attr_validate(yarg, attr))
+				return MNL_CB_ERROR;
+			dst->_present.ifindex = 1;
+			dst->ifindex = mnl_attr_get_u32(attr);
+		}
+	}
+
+	return MNL_CB_OK;
+}
+
+struct netdev_queue_get_rsp *
+netdev_queue_get(struct ynl_sock *ys, struct netdev_queue_get_req *req)
+{
+	struct ynl_req_state yrs = { .yarg = { .ys = ys, }, };
+	struct netdev_queue_get_rsp *rsp;
+	struct nlmsghdr *nlh;
+	int err;
+
+	nlh = ynl_gemsg_start_req(ys, ys->family_id, NETDEV_CMD_QUEUE_GET, 1);
+	ys->req_policy = &netdev_queue_nest;
+	yrs.yarg.rsp_policy = &netdev_queue_nest;
+
+	if (req->_present.ifindex)
+		mnl_attr_put_u32(nlh, NETDEV_A_QUEUE_IFINDEX, req->ifindex);
+	if (req->_present.type)
+		mnl_attr_put_u32(nlh, NETDEV_A_QUEUE_TYPE, req->type);
+	if (req->_present.id)
+		mnl_attr_put_u32(nlh, NETDEV_A_QUEUE_ID, req->id);
+
+	rsp = calloc(1, sizeof(*rsp));
+	yrs.yarg.data = rsp;
+	yrs.cb = netdev_queue_get_rsp_parse;
+	yrs.rsp_cmd = NETDEV_CMD_QUEUE_GET;
+
+	err = ynl_exec(ys, nlh, &yrs);
+	if (err < 0)
+		goto err_free;
+
+	return rsp;
+
+err_free:
+	netdev_queue_get_rsp_free(rsp);
+	return NULL;
+}
+
+/* NETDEV_CMD_QUEUE_GET - dump */
+void netdev_queue_get_list_free(struct netdev_queue_get_list *rsp)
+{
+	struct netdev_queue_get_list *next = rsp;
+
+	while ((void *)next != YNL_LIST_END) {
+		rsp = next;
+		next = rsp->next;
+
+		free(rsp);
+	}
+}
+
+struct netdev_queue_get_list *
+netdev_queue_get_dump(struct ynl_sock *ys,
+		      struct netdev_queue_get_req_dump *req)
+{
+	struct ynl_dump_state yds = {};
+	struct nlmsghdr *nlh;
+	int err;
+
+	yds.ys = ys;
+	yds.alloc_sz = sizeof(struct netdev_queue_get_list);
+	yds.cb = netdev_queue_get_rsp_parse;
+	yds.rsp_cmd = NETDEV_CMD_QUEUE_GET;
+	yds.rsp_policy = &netdev_queue_nest;
+
+	nlh = ynl_gemsg_start_dump(ys, ys->family_id, NETDEV_CMD_QUEUE_GET, 1);
+	ys->req_policy = &netdev_queue_nest;
+
+	if (req->_present.ifindex)
+		mnl_attr_put_u32(nlh, NETDEV_A_QUEUE_IFINDEX, req->ifindex);
+
+	err = ynl_exec_dump(ys, nlh, &yds);
+	if (err < 0)
+		goto free_list;
+
+	return yds.first;
+
+free_list:
+	netdev_queue_get_list_free(yds.first);
+	return NULL;
+}
+
 static const struct ynl_ntf_info netdev_ntf_info[] =  {
 	[NETDEV_CMD_DEV_ADD_NTF] =  {
 		.alloc_sz	= sizeof(struct netdev_dev_get_ntf),
diff --git a/tools/net/ynl/generated/netdev-user.h b/tools/net/ynl/generated/netdev-user.h
index 4fafac879df3..50d214e8819d 100644
--- a/tools/net/ynl/generated/netdev-user.h
+++ b/tools/net/ynl/generated/netdev-user.h
@@ -19,6 +19,7 @@ extern const struct ynl_family ynl_netdev_family;
 const char *netdev_op_str(int op);
 const char *netdev_xdp_act_str(enum netdev_xdp_act value);
 const char *netdev_xdp_rx_metadata_str(enum netdev_xdp_rx_metadata value);
+const char *netdev_queue_type_str(enum netdev_queue_type value);
 
 /* Common nested types */
 /* ============== NETDEV_CMD_DEV_GET ============== */
@@ -87,4 +88,102 @@ struct netdev_dev_get_ntf {
 
 void netdev_dev_get_ntf_free(struct netdev_dev_get_ntf *rsp);
 
+/* ============== NETDEV_CMD_QUEUE_GET ============== */
+/* NETDEV_CMD_QUEUE_GET - do */
+struct netdev_queue_get_req {
+	struct {
+		__u32 ifindex:1;
+		__u32 type:1;
+		__u32 id:1;
+	} _present;
+
+	__u32 ifindex;
+	enum netdev_queue_type type;
+	__u32 id;
+};
+
+static inline struct netdev_queue_get_req *netdev_queue_get_req_alloc(void)
+{
+	return calloc(1, sizeof(struct netdev_queue_get_req));
+}
+void netdev_queue_get_req_free(struct netdev_queue_get_req *req);
+
+static inline void
+netdev_queue_get_req_set_ifindex(struct netdev_queue_get_req *req,
+				 __u32 ifindex)
+{
+	req->_present.ifindex = 1;
+	req->ifindex = ifindex;
+}
+static inline void
+netdev_queue_get_req_set_type(struct netdev_queue_get_req *req,
+			      enum netdev_queue_type type)
+{
+	req->_present.type = 1;
+	req->type = type;
+}
+static inline void
+netdev_queue_get_req_set_id(struct netdev_queue_get_req *req, __u32 id)
+{
+	req->_present.id = 1;
+	req->id = id;
+}
+
+struct netdev_queue_get_rsp {
+	struct {
+		__u32 id:1;
+		__u32 type:1;
+		__u32 napi_id:1;
+		__u32 ifindex:1;
+	} _present;
+
+	__u32 id;
+	enum netdev_queue_type type;
+	__u32 napi_id;
+	__u32 ifindex;
+};
+
+void netdev_queue_get_rsp_free(struct netdev_queue_get_rsp *rsp);
+
+/*
+ * Get queue information from the kernel. Only configured queues will be reported (as opposed to all available hardware queues).
+ */
+struct netdev_queue_get_rsp *
+netdev_queue_get(struct ynl_sock *ys, struct netdev_queue_get_req *req);
+
+/* NETDEV_CMD_QUEUE_GET - dump */
+struct netdev_queue_get_req_dump {
+	struct {
+		__u32 ifindex:1;
+	} _present;
+
+	__u32 ifindex;
+};
+
+static inline struct netdev_queue_get_req_dump *
+netdev_queue_get_req_dump_alloc(void)
+{
+	return calloc(1, sizeof(struct netdev_queue_get_req_dump));
+}
+void netdev_queue_get_req_dump_free(struct netdev_queue_get_req_dump *req);
+
+static inline void
+netdev_queue_get_req_dump_set_ifindex(struct netdev_queue_get_req_dump *req,
+				      __u32 ifindex)
+{
+	req->_present.ifindex = 1;
+	req->ifindex = ifindex;
+}
+
+struct netdev_queue_get_list {
+	struct netdev_queue_get_list *next;
+	struct netdev_queue_get_rsp obj __attribute__((aligned(8)));
+};
+
+void netdev_queue_get_list_free(struct netdev_queue_get_list *rsp);
+
+struct netdev_queue_get_list *
+netdev_queue_get_dump(struct ynl_sock *ys,
+		      struct netdev_queue_get_req_dump *req);
+
 #endif /* _LINUX_NETDEV_GEN_H */


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [net-next PATCH v8 02/10] net: Add queue and napi association
  2023-11-17  1:16 [net-next PATCH v8 00/10] Introduce queue and NAPI support in netdev-genl (Was: Introduce NAPI queues support) Amritha Nambiar
  2023-11-17  1:16 ` [net-next PATCH v8 01/10] netdev-genl: spec: Extend netdev netlink spec in YAML for queue Amritha Nambiar
@ 2023-11-17  1:16 ` Amritha Nambiar
  2023-11-20 23:54   ` Jakub Kicinski
  2023-11-17  1:16 ` [net-next PATCH v8 03/10] ice: Add support in the driver for associating queue with napi Amritha Nambiar
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 21+ messages in thread
From: Amritha Nambiar @ 2023-11-17  1:16 UTC (permalink / raw)
  To: netdev, kuba, pabeni; +Cc: sridhar.samudrala, amritha.nambiar

Add the napi pointer in netdev queue for tracking the napi
instance for each queue. This achieves the queue<->napi mapping.

Signed-off-by: Amritha Nambiar <amritha.nambiar@intel.com>
Reviewed-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
---
 include/linux/netdevice.h     |   11 ++++++++++
 include/net/netdev_rx_queue.h |    4 ++++
 net/core/dev.c                |   45 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 60 insertions(+)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index a16c9cc063fe..8c8010e78240 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -665,6 +665,10 @@ struct netdev_queue {
 #ifdef CONFIG_XDP_SOCKETS
 	struct xsk_buff_pool    *pool;
 #endif
+	/* NAPI instance for the queue
+	 * Readers and writers must hold RTNL
+	 */
+	struct napi_struct      *napi;
 /*
  * write-mostly part
  */
@@ -2639,6 +2643,13 @@ static inline void *netdev_priv(const struct net_device *dev)
  */
 #define SET_NETDEV_DEVTYPE(net, devtype)	((net)->dev.type = (devtype))
 
+void netif_queue_set_napi(unsigned int queue_index, enum netdev_queue_type type,
+			  struct napi_struct *napi);
+
+void __netif_queue_set_napi(unsigned int queue_index,
+			    enum netdev_queue_type type,
+			    struct napi_struct *napi);
+
 /* Default NAPI poll() weight
  * Device drivers are strongly advised to not use bigger value
  */
diff --git a/include/net/netdev_rx_queue.h b/include/net/netdev_rx_queue.h
index cdcafb30d437..aa1716fb0e53 100644
--- a/include/net/netdev_rx_queue.h
+++ b/include/net/netdev_rx_queue.h
@@ -21,6 +21,10 @@ struct netdev_rx_queue {
 #ifdef CONFIG_XDP_SOCKETS
 	struct xsk_buff_pool            *pool;
 #endif
+	/* NAPI instance for the queue
+	 * Readers and writers must hold RTNL
+	 */
+	struct napi_struct		*napi;
 } ____cacheline_aligned_in_smp;
 
 /*
diff --git a/net/core/dev.c b/net/core/dev.c
index af53f6d838ce..cdbc916d0208 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6400,6 +6400,51 @@ int dev_set_threaded(struct net_device *dev, bool threaded)
 }
 EXPORT_SYMBOL(dev_set_threaded);
 
+/**
+ * __netif_queue_set_napi - Associate queue with the napi
+ * @queue_index: Index of queue
+ * @type: queue type as RX or TX
+ * @napi: NAPI context
+ *
+ * Set queue with its corresponding napi context. This should be done after
+ * registering the NAPI handler for the queue-vector and the queues have been
+ * mapped to the corresponding interrupt vector.
+ */
+void __netif_queue_set_napi(unsigned int queue_index,
+			    enum netdev_queue_type type,
+			    struct napi_struct *napi)
+{
+	struct net_device *dev = napi->dev;
+	struct netdev_rx_queue *rxq;
+	struct netdev_queue *txq;
+
+	if (WARN_ON_ONCE(!dev))
+		return;
+
+	switch (type) {
+	case NETDEV_QUEUE_TYPE_RX:
+		rxq = __netif_get_rx_queue(dev, queue_index);
+		rxq->napi = napi;
+		return;
+	case NETDEV_QUEUE_TYPE_TX:
+		txq = netdev_get_tx_queue(dev, queue_index);
+		txq->napi = napi;
+		return;
+	default:
+		return;
+	}
+}
+EXPORT_SYMBOL(__netif_queue_set_napi);
+
+void netif_queue_set_napi(unsigned int queue_index, enum netdev_queue_type type,
+			  struct napi_struct *napi)
+{
+	rtnl_lock();
+	__netif_queue_set_napi(queue_index, type, napi);
+	rtnl_unlock();
+}
+EXPORT_SYMBOL(netif_queue_set_napi);
+
 void netif_napi_add_weight(struct net_device *dev, struct napi_struct *napi,
 			   int (*poll)(struct napi_struct *, int), int weight)
 {


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [net-next PATCH v8 03/10] ice: Add support in the driver for associating queue with napi
  2023-11-17  1:16 [net-next PATCH v8 00/10] Introduce queue and NAPI support in netdev-genl (Was: Introduce NAPI queues support) Amritha Nambiar
  2023-11-17  1:16 ` [net-next PATCH v8 01/10] netdev-genl: spec: Extend netdev netlink spec in YAML for queue Amritha Nambiar
  2023-11-17  1:16 ` [net-next PATCH v8 02/10] net: Add queue and napi association Amritha Nambiar
@ 2023-11-17  1:16 ` Amritha Nambiar
  2023-11-17  1:16 ` [net-next PATCH v8 04/10] netdev-genl: Add netlink framework functions for queue Amritha Nambiar
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 21+ messages in thread
From: Amritha Nambiar @ 2023-11-17  1:16 UTC (permalink / raw)
  To: netdev, kuba, pabeni; +Cc: sridhar.samudrala, amritha.nambiar

After the napi context is initialized, map the napi instance
with the queue/queue-set on the corresponding irq line.

Signed-off-by: Amritha Nambiar <amritha.nambiar@intel.com>
Reviewed-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_lib.c  |   62 +++++++++++++++++++++++++++++
 drivers/net/ethernet/intel/ice/ice_lib.h  |    4 ++
 drivers/net/ethernet/intel/ice/ice_main.c |    4 +-
 3 files changed, 69 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
index 4b1e56396293..e2c24853376f 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_lib.c
@@ -2447,6 +2447,10 @@ ice_vsi_cfg_def(struct ice_vsi *vsi, struct ice_vsi_cfg_params *params)
 			goto unroll_vector_base;
 
 		ice_vsi_map_rings_to_vectors(vsi);
+
+		/* Associate q_vector rings to napi */
+		ice_vsi_set_napi_queues(vsi, true);
+
 		vsi->stat_offsets_loaded = false;
 
 		if (ice_is_xdp_ena_vsi(vsi)) {
@@ -2926,6 +2930,64 @@ void ice_vsi_dis_irq(struct ice_vsi *vsi)
 		synchronize_irq(vsi->q_vectors[i]->irq.virq);
 }
 
+/**
+ * ice_queue_set_napi - Set the napi instance for the queue
+ * @queue_index: Index of queue
+ * @type: queue type as RX or TX
+ * @napi: NAPI context
+ * @locked: is the rtnl_lock already held
+ *
+ * Set the napi instance for the queue
+ */
+static void
+ice_queue_set_napi(unsigned int queue_index, enum netdev_queue_type type,
+		   struct napi_struct *napi, bool locked)
+{
+	if (locked)
+		__netif_queue_set_napi(queue_index, type, napi);
+	else
+		netif_queue_set_napi(queue_index, type, napi);
+}
+
+/**
+ * ice_q_vector_set_napi_queues - Map queue[s] associated with the napi
+ * @q_vector: q_vector pointer
+ * @locked: is the rtnl_lock already held
+ *
+ * Associate the q_vector napi with all the queue[s] on the vector
+ */
+void ice_q_vector_set_napi_queues(struct ice_q_vector *q_vector, bool locked)
+{
+	struct ice_rx_ring *rx_ring;
+	struct ice_tx_ring *tx_ring;
+
+	ice_for_each_rx_ring(rx_ring, q_vector->rx)
+		ice_queue_set_napi(rx_ring->q_index, NETDEV_QUEUE_TYPE_RX,
+				   &q_vector->napi, locked);
+
+	ice_for_each_tx_ring(tx_ring, q_vector->tx)
+		ice_queue_set_napi(tx_ring->q_index, NETDEV_QUEUE_TYPE_TX,
+				   &q_vector->napi, locked);
+}
+
+/**
+ * ice_vsi_set_napi_queues
+ * @vsi: VSI pointer
+ * @locked: is the rtnl_lock already held
+ *
+ * Associate queue[s] with napi for all vectors
+ */
+void ice_vsi_set_napi_queues(struct ice_vsi *vsi, bool locked)
+{
+	int i;
+
+	if (!vsi->netdev)
+		return;
+
+	ice_for_each_q_vector(vsi, i)
+		ice_q_vector_set_napi_queues(vsi->q_vectors[i], locked);
+}
+
 /**
  * ice_vsi_release - Delete a VSI and free its resources
  * @vsi: the VSI being removed
diff --git a/drivers/net/ethernet/intel/ice/ice_lib.h b/drivers/net/ethernet/intel/ice/ice_lib.h
index f24f5d1e6f9c..71bd27244941 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.h
+++ b/drivers/net/ethernet/intel/ice/ice_lib.h
@@ -91,6 +91,10 @@ void ice_vsi_cfg_netdev_tc(struct ice_vsi *vsi, u8 ena_tc);
 struct ice_vsi *
 ice_vsi_setup(struct ice_pf *pf, struct ice_vsi_cfg_params *params);
 
+void ice_q_vector_set_napi_queues(struct ice_q_vector *q_vector, bool locked);
+
+void ice_vsi_set_napi_queues(struct ice_vsi *vsi, bool locked);
+
 int ice_vsi_release(struct ice_vsi *vsi);
 
 void ice_vsi_close(struct ice_vsi *vsi);
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 6607fa6fe556..126da447e168 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -3375,9 +3375,11 @@ static void ice_napi_add(struct ice_vsi *vsi)
 	if (!vsi->netdev)
 		return;
 
-	ice_for_each_q_vector(vsi, v_idx)
+	ice_for_each_q_vector(vsi, v_idx) {
 		netif_napi_add(vsi->netdev, &vsi->q_vectors[v_idx]->napi,
 			       ice_napi_poll);
+		ice_q_vector_set_napi_queues(vsi->q_vectors[v_idx], false);
+	}
 }
 
 /**


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [net-next PATCH v8 04/10] netdev-genl: Add netlink framework functions for queue
  2023-11-17  1:16 [net-next PATCH v8 00/10] Introduce queue and NAPI support in netdev-genl (Was: Introduce NAPI queues support) Amritha Nambiar
                   ` (2 preceding siblings ...)
  2023-11-17  1:16 ` [net-next PATCH v8 03/10] ice: Add support in the driver for associating queue with napi Amritha Nambiar
@ 2023-11-17  1:16 ` Amritha Nambiar
  2023-11-17  1:17 ` [net-next PATCH v8 05/10] netdev-genl: spec: Extend netdev netlink spec in YAML for NAPI Amritha Nambiar
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 21+ messages in thread
From: Amritha Nambiar @ 2023-11-17  1:16 UTC (permalink / raw)
  To: netdev, kuba, pabeni; +Cc: sridhar.samudrala, amritha.nambiar

Implement the netdev netlink framework functions for
exposing queue information.

Signed-off-by: Amritha Nambiar <amritha.nambiar@intel.com>
Reviewed-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
---
 net/core/netdev-genl.c |  181 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 178 insertions(+), 3 deletions(-)

diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
index 336c608e6a6b..fa64a4378166 100644
--- a/net/core/netdev-genl.c
+++ b/net/core/netdev-genl.c
@@ -6,9 +6,23 @@
 #include <net/net_namespace.h>
 #include <net/sock.h>
 #include <net/xdp.h>
+#include <net/netdev_rx_queue.h>
 
 #include "netdev-genl-gen.h"
 
+struct netdev_nl_dump_ctx {
+	unsigned long	ifindex;
+	unsigned int	rxq_idx;
+	unsigned int	txq_idx;
+};
+
+static struct netdev_nl_dump_ctx *netdev_dump_ctx(struct netlink_callback *cb)
+{
+	NL_ASSERT_DUMP_CTX_FITS(struct netdev_nl_dump_ctx);
+
+	return (struct netdev_nl_dump_ctx *)cb->ctx;
+}
+
 static int
 netdev_nl_dev_fill(struct net_device *netdev, struct sk_buff *rsp,
 		   const struct genl_info *info)
@@ -111,12 +125,13 @@ int netdev_nl_dev_get_doit(struct sk_buff *skb, struct genl_info *info)
 
 int netdev_nl_dev_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
 {
+	struct netdev_nl_dump_ctx *ctx = netdev_dump_ctx(cb);
 	struct net *net = sock_net(skb->sk);
 	struct net_device *netdev;
 	int err = 0;
 
 	rtnl_lock();
-	for_each_netdev_dump(net, netdev, cb->args[0]) {
+	for_each_netdev_dump(net, netdev, ctx->ifindex) {
 		err = netdev_nl_dev_fill(netdev, skb, genl_info_dump(cb));
 		if (err < 0)
 			break;
@@ -129,14 +144,174 @@ int netdev_nl_dev_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
 	return skb->len;
 }
 
+static int
+netdev_nl_queue_fill_one(struct sk_buff *rsp, struct net_device *netdev,
+			 u32 q_idx, u32 q_type, const struct genl_info *info)
+{
+	struct netdev_rx_queue *rxq;
+	struct netdev_queue *txq;
+	void *hdr;
+
+	hdr = genlmsg_iput(rsp, info);
+	if (!hdr)
+		return -EMSGSIZE;
+
+	if (nla_put_u32(rsp, NETDEV_A_QUEUE_ID, q_idx) ||
+	    nla_put_u32(rsp, NETDEV_A_QUEUE_TYPE, q_type) ||
+	    nla_put_u32(rsp, NETDEV_A_QUEUE_IFINDEX, netdev->ifindex))
+		goto nla_put_failure;
+
+	switch (q_type) {
+	case NETDEV_QUEUE_TYPE_RX:
+		rxq = __netif_get_rx_queue(netdev, q_idx);
+		if (rxq->napi && nla_put_u32(rsp, NETDEV_A_QUEUE_NAPI_ID,
+					     rxq->napi->napi_id))
+			goto nla_put_failure;
+		break;
+	case NETDEV_QUEUE_TYPE_TX:
+		txq = netdev_get_tx_queue(netdev, q_idx);
+		if (txq->napi && nla_put_u32(rsp, NETDEV_A_QUEUE_NAPI_ID,
+					     txq->napi->napi_id))
+			goto nla_put_failure;
+	}
+
+	genlmsg_end(rsp, hdr);
+
+	return 0;
+
+nla_put_failure:
+	genlmsg_cancel(rsp, hdr);
+	return -EMSGSIZE;
+}
+
+static int netdev_nl_queue_validate(struct net_device *netdev, u32 q_id,
+				    u32 q_type)
+{
+	switch (q_type) {
+	case NETDEV_QUEUE_TYPE_RX:
+		if (q_id >= netdev->real_num_rx_queues)
+			return -EINVAL;
+		return 0;
+	case NETDEV_QUEUE_TYPE_TX:
+		if (q_id >= netdev->real_num_tx_queues)
+			return -EINVAL;
+	}
+	return 0;
+}
+
+static int
+netdev_nl_queue_fill(struct sk_buff *rsp, struct net_device *netdev, u32 q_idx,
+		     u32 q_type, const struct genl_info *info)
+{
+	int err;
+
+	err = netdev_nl_queue_validate(netdev, q_idx, q_type);
+	if (err)
+		return err;
+
+	return netdev_nl_queue_fill_one(rsp, netdev, q_idx, q_type, info);
+}
+
 int netdev_nl_queue_get_doit(struct sk_buff *skb, struct genl_info *info)
 {
-	return -EOPNOTSUPP;
+	u32 q_id, q_type, ifindex;
+	struct net_device *netdev;
+	struct sk_buff *rsp;
+	int err;
+
+	if (GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_ID) ||
+	    GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_TYPE) ||
+	    GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_IFINDEX))
+		return -EINVAL;
+
+	q_id = nla_get_u32(info->attrs[NETDEV_A_QUEUE_ID]);
+	q_type = nla_get_u32(info->attrs[NETDEV_A_QUEUE_TYPE]);
+	ifindex = nla_get_u32(info->attrs[NETDEV_A_QUEUE_IFINDEX]);
+
+	rsp = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
+	if (!rsp)
+		return -ENOMEM;
+
+	rtnl_lock();
+
+	netdev = __dev_get_by_index(genl_info_net(info), ifindex);
+	if (netdev)
+		err = netdev_nl_queue_fill(rsp, netdev, q_id, q_type, info);
+	else
+		err = -ENODEV;
+
+	rtnl_unlock();
+
+	if (err)
+		goto err_free_msg;
+
+	return genlmsg_reply(rsp, info);
+
+err_free_msg:
+	nlmsg_free(rsp);
+	return err;
+}
+
+static int
+netdev_nl_queue_dump_one(struct net_device *netdev, struct sk_buff *rsp,
+			 const struct genl_info *info,
+			 struct netdev_nl_dump_ctx *ctx)
+{
+	int err = 0;
+	int i;
+
+	for (i = ctx->rxq_idx; i < netdev->real_num_rx_queues;) {
+		err = netdev_nl_queue_fill_one(rsp, netdev, i,
+					       NETDEV_QUEUE_TYPE_RX, info);
+		if (err)
+			return err;
+		ctx->rxq_idx = i++;
+	}
+	for (i = ctx->txq_idx; i < netdev->real_num_tx_queues;) {
+		err = netdev_nl_queue_fill_one(rsp, netdev, i,
+					       NETDEV_QUEUE_TYPE_TX, info);
+		if (err)
+			return err;
+		ctx->txq_idx = i++;
+	}
+
+	return err;
 }
 
 int netdev_nl_queue_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
 {
-	return -EOPNOTSUPP;
+	struct netdev_nl_dump_ctx *ctx = netdev_dump_ctx(cb);
+	const struct genl_info *info = genl_info_dump(cb);
+	struct net *net = sock_net(skb->sk);
+	struct net_device *netdev;
+	u32 ifindex = 0;
+	int err = 0;
+
+	if (info->attrs[NETDEV_A_QUEUE_IFINDEX])
+		ifindex = nla_get_u32(info->attrs[NETDEV_A_QUEUE_IFINDEX]);
+
+	rtnl_lock();
+	if (ifindex) {
+		netdev = __dev_get_by_index(net, ifindex);
+		if (netdev)
+			err = netdev_nl_queue_dump_one(netdev, skb, info, ctx);
+		else
+			err = -ENODEV;
+	} else {
+		for_each_netdev_dump(net, netdev, ctx->ifindex) {
+			err = netdev_nl_queue_dump_one(netdev, skb, info, ctx);
+			if (err < 0)
+				break;
+			ctx->rxq_idx = 0;
+			ctx->txq_idx = 0;
+		}
+	}
+	rtnl_unlock();
+
+	if (err != -EMSGSIZE)
+		return err;
+
+	return skb->len;
 }
 
 static int netdev_genl_netdevice_event(struct notifier_block *nb,


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [net-next PATCH v8 05/10] netdev-genl: spec: Extend netdev netlink spec in YAML for NAPI
  2023-11-17  1:16 [net-next PATCH v8 00/10] Introduce queue and NAPI support in netdev-genl (Was: Introduce NAPI queues support) Amritha Nambiar
                   ` (3 preceding siblings ...)
  2023-11-17  1:16 ` [net-next PATCH v8 04/10] netdev-genl: Add netlink framework functions for queue Amritha Nambiar
@ 2023-11-17  1:17 ` Amritha Nambiar
  2023-11-17  1:17 ` [net-next PATCH v8 06/10] netdev-genl: Add netlink framework functions for napi Amritha Nambiar
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 21+ messages in thread
From: Amritha Nambiar @ 2023-11-17  1:17 UTC (permalink / raw)
  To: netdev, kuba, pabeni; +Cc: sridhar.samudrala, amritha.nambiar

Add support in netlink spec(netdev.yaml) for napi related information.
Add code generated from the spec.

Signed-off-by: Amritha Nambiar <amritha.nambiar@intel.com>
Reviewed-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
---
 Documentation/netlink/specs/netdev.yaml |   30 ++++++++
 include/uapi/linux/netdev.h             |    9 ++
 net/core/netdev-genl-gen.c              |   24 ++++++
 net/core/netdev-genl-gen.h              |    2 +
 net/core/netdev-genl.c                  |   10 +++
 tools/include/uapi/linux/netdev.h       |    9 ++
 tools/net/ynl/generated/netdev-user.c   |  124 +++++++++++++++++++++++++++++++
 tools/net/ynl/generated/netdev-user.h   |   75 +++++++++++++++++++
 8 files changed, 283 insertions(+)

diff --git a/Documentation/netlink/specs/netdev.yaml b/Documentation/netlink/specs/netdev.yaml
index f9382b7705ab..d68ab270618e 100644
--- a/Documentation/netlink/specs/netdev.yaml
+++ b/Documentation/netlink/specs/netdev.yaml
@@ -91,6 +91,19 @@ attribute-sets:
         type: u64
         enum: xdp-rx-metadata
 
+  -
+    name: napi
+    attributes:
+      -
+        name: ifindex
+        doc: ifindex of the netdevice to which NAPI instance belongs.
+        type: u32
+        checks:
+          min: 1
+      -
+        name: id
+        doc: ID of the NAPI instance.
+        type: u32
   -
     name: queue
     attributes:
@@ -172,6 +185,23 @@ operations:
           attributes:
             - ifindex
         reply: *queue-get-op
+    -
+      name: napi-get
+      doc: Get information about NAPI instances configured on the system.
+      attribute-set: napi
+      do:
+        request:
+          attributes:
+            - id
+        reply: &napi-get-op
+          attributes:
+            - id
+            - ifindex
+      dump:
+        request:
+          attributes:
+            - ifindex
+        reply: *napi-get-op
 
 mcast-groups:
   list:
diff --git a/include/uapi/linux/netdev.h b/include/uapi/linux/netdev.h
index c440017241f0..ee590cbb9782 100644
--- a/include/uapi/linux/netdev.h
+++ b/include/uapi/linux/netdev.h
@@ -69,6 +69,14 @@ enum {
 	NETDEV_A_DEV_MAX = (__NETDEV_A_DEV_MAX - 1)
 };
 
+enum {
+	NETDEV_A_NAPI_IFINDEX = 1,
+	NETDEV_A_NAPI_ID,
+
+	__NETDEV_A_NAPI_MAX,
+	NETDEV_A_NAPI_MAX = (__NETDEV_A_NAPI_MAX - 1)
+};
+
 enum {
 	NETDEV_A_QUEUE_ID = 1,
 	NETDEV_A_QUEUE_IFINDEX,
@@ -85,6 +93,7 @@ enum {
 	NETDEV_CMD_DEV_DEL_NTF,
 	NETDEV_CMD_DEV_CHANGE_NTF,
 	NETDEV_CMD_QUEUE_GET,
+	NETDEV_CMD_NAPI_GET,
 
 	__NETDEV_CMD_MAX,
 	NETDEV_CMD_MAX = (__NETDEV_CMD_MAX - 1)
diff --git a/net/core/netdev-genl-gen.c b/net/core/netdev-genl-gen.c
index 97ee7ebbad57..0dc618a99304 100644
--- a/net/core/netdev-genl-gen.c
+++ b/net/core/netdev-genl-gen.c
@@ -27,6 +27,16 @@ static const struct nla_policy netdev_queue_get_dump_nl_policy[NETDEV_A_QUEUE_IF
 	[NETDEV_A_QUEUE_IFINDEX] = NLA_POLICY_MIN(NLA_U32, 1),
 };
 
+/* NETDEV_CMD_NAPI_GET - do */
+static const struct nla_policy netdev_napi_get_do_nl_policy[NETDEV_A_NAPI_ID + 1] = {
+	[NETDEV_A_NAPI_ID] = { .type = NLA_U32, },
+};
+
+/* NETDEV_CMD_NAPI_GET - dump */
+static const struct nla_policy netdev_napi_get_dump_nl_policy[NETDEV_A_NAPI_IFINDEX + 1] = {
+	[NETDEV_A_NAPI_IFINDEX] = NLA_POLICY_MIN(NLA_U32, 1),
+};
+
 /* Ops table for netdev */
 static const struct genl_split_ops netdev_nl_ops[] = {
 	{
@@ -55,6 +65,20 @@ static const struct genl_split_ops netdev_nl_ops[] = {
 		.maxattr	= NETDEV_A_QUEUE_IFINDEX,
 		.flags		= GENL_CMD_CAP_DUMP,
 	},
+	{
+		.cmd		= NETDEV_CMD_NAPI_GET,
+		.doit		= netdev_nl_napi_get_doit,
+		.policy		= netdev_napi_get_do_nl_policy,
+		.maxattr	= NETDEV_A_NAPI_ID,
+		.flags		= GENL_CMD_CAP_DO,
+	},
+	{
+		.cmd		= NETDEV_CMD_NAPI_GET,
+		.dumpit		= netdev_nl_napi_get_dumpit,
+		.policy		= netdev_napi_get_dump_nl_policy,
+		.maxattr	= NETDEV_A_NAPI_IFINDEX,
+		.flags		= GENL_CMD_CAP_DUMP,
+	},
 };
 
 static const struct genl_multicast_group netdev_nl_mcgrps[] = {
diff --git a/net/core/netdev-genl-gen.h b/net/core/netdev-genl-gen.h
index 263c94f77bad..ffc94956d1f5 100644
--- a/net/core/netdev-genl-gen.h
+++ b/net/core/netdev-genl-gen.h
@@ -16,6 +16,8 @@ int netdev_nl_dev_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb);
 int netdev_nl_queue_get_doit(struct sk_buff *skb, struct genl_info *info);
 int netdev_nl_queue_get_dumpit(struct sk_buff *skb,
 			       struct netlink_callback *cb);
+int netdev_nl_napi_get_doit(struct sk_buff *skb, struct genl_info *info);
+int netdev_nl_napi_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb);
 
 enum {
 	NETDEV_NLGRP_MGMT,
diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
index fa64a4378166..9386058146a1 100644
--- a/net/core/netdev-genl.c
+++ b/net/core/netdev-genl.c
@@ -144,6 +144,16 @@ int netdev_nl_dev_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
 	return skb->len;
 }
 
+int netdev_nl_napi_get_doit(struct sk_buff *skb, struct genl_info *info)
+{
+	return -EOPNOTSUPP;
+}
+
+int netdev_nl_napi_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
+{
+	return -EOPNOTSUPP;
+}
+
 static int
 netdev_nl_queue_fill_one(struct sk_buff *rsp, struct net_device *netdev,
 			 u32 q_idx, u32 q_type, const struct genl_info *info)
diff --git a/tools/include/uapi/linux/netdev.h b/tools/include/uapi/linux/netdev.h
index c440017241f0..ee590cbb9782 100644
--- a/tools/include/uapi/linux/netdev.h
+++ b/tools/include/uapi/linux/netdev.h
@@ -69,6 +69,14 @@ enum {
 	NETDEV_A_DEV_MAX = (__NETDEV_A_DEV_MAX - 1)
 };
 
+enum {
+	NETDEV_A_NAPI_IFINDEX = 1,
+	NETDEV_A_NAPI_ID,
+
+	__NETDEV_A_NAPI_MAX,
+	NETDEV_A_NAPI_MAX = (__NETDEV_A_NAPI_MAX - 1)
+};
+
 enum {
 	NETDEV_A_QUEUE_ID = 1,
 	NETDEV_A_QUEUE_IFINDEX,
@@ -85,6 +93,7 @@ enum {
 	NETDEV_CMD_DEV_DEL_NTF,
 	NETDEV_CMD_DEV_CHANGE_NTF,
 	NETDEV_CMD_QUEUE_GET,
+	NETDEV_CMD_NAPI_GET,
 
 	__NETDEV_CMD_MAX,
 	NETDEV_CMD_MAX = (__NETDEV_CMD_MAX - 1)
diff --git a/tools/net/ynl/generated/netdev-user.c b/tools/net/ynl/generated/netdev-user.c
index 87845c707b95..a49265f4f4d9 100644
--- a/tools/net/ynl/generated/netdev-user.c
+++ b/tools/net/ynl/generated/netdev-user.c
@@ -19,6 +19,7 @@ static const char * const netdev_op_strmap[] = {
 	[NETDEV_CMD_DEV_DEL_NTF] = "dev-del-ntf",
 	[NETDEV_CMD_DEV_CHANGE_NTF] = "dev-change-ntf",
 	[NETDEV_CMD_QUEUE_GET] = "queue-get",
+	[NETDEV_CMD_NAPI_GET] = "napi-get",
 };
 
 const char *netdev_op_str(int op)
@@ -97,6 +98,16 @@ struct ynl_policy_nest netdev_queue_nest = {
 	.table = netdev_queue_policy,
 };
 
+struct ynl_policy_attr netdev_napi_policy[NETDEV_A_NAPI_MAX + 1] = {
+	[NETDEV_A_NAPI_IFINDEX] = { .name = "ifindex", .type = YNL_PT_U32, },
+	[NETDEV_A_NAPI_ID] = { .name = "id", .type = YNL_PT_U32, },
+};
+
+struct ynl_policy_nest netdev_napi_nest = {
+	.max_attr = NETDEV_A_NAPI_MAX,
+	.table = netdev_napi_policy,
+};
+
 /* Common nested types */
 /* ============== NETDEV_CMD_DEV_GET ============== */
 /* NETDEV_CMD_DEV_GET - do */
@@ -350,6 +361,119 @@ netdev_queue_get_dump(struct ynl_sock *ys,
 	return NULL;
 }
 
+/* ============== NETDEV_CMD_NAPI_GET ============== */
+/* NETDEV_CMD_NAPI_GET - do */
+void netdev_napi_get_req_free(struct netdev_napi_get_req *req)
+{
+	free(req);
+}
+
+void netdev_napi_get_rsp_free(struct netdev_napi_get_rsp *rsp)
+{
+	free(rsp);
+}
+
+int netdev_napi_get_rsp_parse(const struct nlmsghdr *nlh, void *data)
+{
+	struct ynl_parse_arg *yarg = data;
+	struct netdev_napi_get_rsp *dst;
+	const struct nlattr *attr;
+
+	dst = yarg->data;
+
+	mnl_attr_for_each(attr, nlh, sizeof(struct genlmsghdr)) {
+		unsigned int type = mnl_attr_get_type(attr);
+
+		if (type == NETDEV_A_NAPI_ID) {
+			if (ynl_attr_validate(yarg, attr))
+				return MNL_CB_ERROR;
+			dst->_present.id = 1;
+			dst->id = mnl_attr_get_u32(attr);
+		} else if (type == NETDEV_A_NAPI_IFINDEX) {
+			if (ynl_attr_validate(yarg, attr))
+				return MNL_CB_ERROR;
+			dst->_present.ifindex = 1;
+			dst->ifindex = mnl_attr_get_u32(attr);
+		}
+	}
+
+	return MNL_CB_OK;
+}
+
+struct netdev_napi_get_rsp *
+netdev_napi_get(struct ynl_sock *ys, struct netdev_napi_get_req *req)
+{
+	struct ynl_req_state yrs = { .yarg = { .ys = ys, }, };
+	struct netdev_napi_get_rsp *rsp;
+	struct nlmsghdr *nlh;
+	int err;
+
+	nlh = ynl_gemsg_start_req(ys, ys->family_id, NETDEV_CMD_NAPI_GET, 1);
+	ys->req_policy = &netdev_napi_nest;
+	yrs.yarg.rsp_policy = &netdev_napi_nest;
+
+	if (req->_present.id)
+		mnl_attr_put_u32(nlh, NETDEV_A_NAPI_ID, req->id);
+
+	rsp = calloc(1, sizeof(*rsp));
+	yrs.yarg.data = rsp;
+	yrs.cb = netdev_napi_get_rsp_parse;
+	yrs.rsp_cmd = NETDEV_CMD_NAPI_GET;
+
+	err = ynl_exec(ys, nlh, &yrs);
+	if (err < 0)
+		goto err_free;
+
+	return rsp;
+
+err_free:
+	netdev_napi_get_rsp_free(rsp);
+	return NULL;
+}
+
+/* NETDEV_CMD_NAPI_GET - dump */
+void netdev_napi_get_list_free(struct netdev_napi_get_list *rsp)
+{
+	struct netdev_napi_get_list *next = rsp;
+
+	while ((void *)next != YNL_LIST_END) {
+		rsp = next;
+		next = rsp->next;
+
+		free(rsp);
+	}
+}
+
+struct netdev_napi_get_list *
+netdev_napi_get_dump(struct ynl_sock *ys, struct netdev_napi_get_req_dump *req)
+{
+	struct ynl_dump_state yds = {};
+	struct nlmsghdr *nlh;
+	int err;
+
+	yds.ys = ys;
+	yds.alloc_sz = sizeof(struct netdev_napi_get_list);
+	yds.cb = netdev_napi_get_rsp_parse;
+	yds.rsp_cmd = NETDEV_CMD_NAPI_GET;
+	yds.rsp_policy = &netdev_napi_nest;
+
+	nlh = ynl_gemsg_start_dump(ys, ys->family_id, NETDEV_CMD_NAPI_GET, 1);
+	ys->req_policy = &netdev_napi_nest;
+
+	if (req->_present.ifindex)
+		mnl_attr_put_u32(nlh, NETDEV_A_NAPI_IFINDEX, req->ifindex);
+
+	err = ynl_exec_dump(ys, nlh, &yds);
+	if (err < 0)
+		goto free_list;
+
+	return yds.first;
+
+free_list:
+	netdev_napi_get_list_free(yds.first);
+	return NULL;
+}
+
 static const struct ynl_ntf_info netdev_ntf_info[] =  {
 	[NETDEV_CMD_DEV_ADD_NTF] =  {
 		.alloc_sz	= sizeof(struct netdev_dev_get_ntf),
diff --git a/tools/net/ynl/generated/netdev-user.h b/tools/net/ynl/generated/netdev-user.h
index 50d214e8819d..33ba75905ee1 100644
--- a/tools/net/ynl/generated/netdev-user.h
+++ b/tools/net/ynl/generated/netdev-user.h
@@ -186,4 +186,79 @@ struct netdev_queue_get_list *
 netdev_queue_get_dump(struct ynl_sock *ys,
 		      struct netdev_queue_get_req_dump *req);
 
+/* ============== NETDEV_CMD_NAPI_GET ============== */
+/* NETDEV_CMD_NAPI_GET - do */
+struct netdev_napi_get_req {
+	struct {
+		__u32 id:1;
+	} _present;
+
+	__u32 id;
+};
+
+static inline struct netdev_napi_get_req *netdev_napi_get_req_alloc(void)
+{
+	return calloc(1, sizeof(struct netdev_napi_get_req));
+}
+void netdev_napi_get_req_free(struct netdev_napi_get_req *req);
+
+static inline void
+netdev_napi_get_req_set_id(struct netdev_napi_get_req *req, __u32 id)
+{
+	req->_present.id = 1;
+	req->id = id;
+}
+
+struct netdev_napi_get_rsp {
+	struct {
+		__u32 id:1;
+		__u32 ifindex:1;
+	} _present;
+
+	__u32 id;
+	__u32 ifindex;
+};
+
+void netdev_napi_get_rsp_free(struct netdev_napi_get_rsp *rsp);
+
+/*
+ * Get information about NAPI instances configured on the system.
+ */
+struct netdev_napi_get_rsp *
+netdev_napi_get(struct ynl_sock *ys, struct netdev_napi_get_req *req);
+
+/* NETDEV_CMD_NAPI_GET - dump */
+struct netdev_napi_get_req_dump {
+	struct {
+		__u32 ifindex:1;
+	} _present;
+
+	__u32 ifindex;
+};
+
+static inline struct netdev_napi_get_req_dump *
+netdev_napi_get_req_dump_alloc(void)
+{
+	return calloc(1, sizeof(struct netdev_napi_get_req_dump));
+}
+void netdev_napi_get_req_dump_free(struct netdev_napi_get_req_dump *req);
+
+static inline void
+netdev_napi_get_req_dump_set_ifindex(struct netdev_napi_get_req_dump *req,
+				     __u32 ifindex)
+{
+	req->_present.ifindex = 1;
+	req->ifindex = ifindex;
+}
+
+struct netdev_napi_get_list {
+	struct netdev_napi_get_list *next;
+	struct netdev_napi_get_rsp obj __attribute__((aligned(8)));
+};
+
+void netdev_napi_get_list_free(struct netdev_napi_get_list *rsp);
+
+struct netdev_napi_get_list *
+netdev_napi_get_dump(struct ynl_sock *ys, struct netdev_napi_get_req_dump *req);
+
 #endif /* _LINUX_NETDEV_GEN_H */


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [net-next PATCH v8 06/10] netdev-genl: Add netlink framework functions for napi
  2023-11-17  1:16 [net-next PATCH v8 00/10] Introduce queue and NAPI support in netdev-genl (Was: Introduce NAPI queues support) Amritha Nambiar
                   ` (4 preceding siblings ...)
  2023-11-17  1:17 ` [net-next PATCH v8 05/10] netdev-genl: spec: Extend netdev netlink spec in YAML for NAPI Amritha Nambiar
@ 2023-11-17  1:17 ` Amritha Nambiar
  2023-11-17  1:17 ` [net-next PATCH v8 07/10] netdev-genl: spec: Add irq in netdev netlink YAML spec Amritha Nambiar
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 21+ messages in thread
From: Amritha Nambiar @ 2023-11-17  1:17 UTC (permalink / raw)
  To: netdev, kuba, pabeni; +Cc: sridhar.samudrala, amritha.nambiar

Implement the netdev netlink framework functions for
napi support. The netdev structure tracks all the napi
instances and napi fields. The napi instances and associated
parameters can be retrieved this way.

Signed-off-by: Amritha Nambiar <amritha.nambiar@intel.com>
Reviewed-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
---
 net/core/dev.c         |    3 -
 net/core/dev.h         |    2 +
 net/core/netdev-genl.c |  116 +++++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 117 insertions(+), 4 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index cdbc916d0208..fcbfecdf5e5b 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -165,7 +165,6 @@ static int netif_rx_internal(struct sk_buff *skb);
 static int call_netdevice_notifiers_extack(unsigned long val,
 					   struct net_device *dev,
 					   struct netlink_ext_ack *extack);
-static struct napi_struct *napi_by_id(unsigned int napi_id);
 
 /*
  * The @dev_base_head list is protected by @dev_base_lock and the rtnl
@@ -6139,7 +6138,7 @@ bool napi_complete_done(struct napi_struct *n, int work_done)
 EXPORT_SYMBOL(napi_complete_done);
 
 /* must be called under rcu_read_lock(), as we dont take a reference */
-static struct napi_struct *napi_by_id(unsigned int napi_id)
+struct napi_struct *napi_by_id(unsigned int napi_id)
 {
 	unsigned int hash = napi_id % HASH_SIZE(napi_hash);
 	struct napi_struct *napi;
diff --git a/net/core/dev.h b/net/core/dev.h
index 5aa45f0fd4ae..7795b8ad841d 100644
--- a/net/core/dev.h
+++ b/net/core/dev.h
@@ -145,4 +145,6 @@ void xdp_do_check_flushed(struct napi_struct *napi);
 #else
 static inline void xdp_do_check_flushed(struct napi_struct *napi) { }
 #endif
+
+struct napi_struct *napi_by_id(unsigned int napi_id);
 #endif
diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
index 9386058146a1..fda6d06995e0 100644
--- a/net/core/netdev-genl.c
+++ b/net/core/netdev-genl.c
@@ -7,13 +7,16 @@
 #include <net/sock.h>
 #include <net/xdp.h>
 #include <net/netdev_rx_queue.h>
+#include <net/busy_poll.h>
 
 #include "netdev-genl-gen.h"
+#include "dev.h"
 
 struct netdev_nl_dump_ctx {
 	unsigned long	ifindex;
 	unsigned int	rxq_idx;
 	unsigned int	txq_idx;
+	unsigned int	napi_id;
 };
 
 static struct netdev_nl_dump_ctx *netdev_dump_ctx(struct netlink_callback *cb)
@@ -144,14 +147,123 @@ int netdev_nl_dev_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
 	return skb->len;
 }
 
+static int
+netdev_nl_napi_fill_one(struct sk_buff *rsp, struct napi_struct *napi,
+			const struct genl_info *info)
+{
+	void *hdr;
+
+	if (WARN_ON_ONCE(!napi->dev))
+		return -EINVAL;
+
+	hdr = genlmsg_iput(rsp, info);
+	if (!hdr)
+		return -EMSGSIZE;
+
+	if (napi->napi_id >= MIN_NAPI_ID &&
+	    nla_put_u32(rsp, NETDEV_A_NAPI_ID, napi->napi_id))
+		goto nla_put_failure;
+
+	if (nla_put_u32(rsp, NETDEV_A_NAPI_IFINDEX, napi->dev->ifindex))
+		goto nla_put_failure;
+
+	genlmsg_end(rsp, hdr);
+	return 0;
+
+nla_put_failure:
+	genlmsg_cancel(rsp, hdr);
+	return -EMSGSIZE;
+}
+
 int netdev_nl_napi_get_doit(struct sk_buff *skb, struct genl_info *info)
 {
-	return -EOPNOTSUPP;
+	struct napi_struct *napi;
+	struct sk_buff *rsp;
+	u32 napi_id;
+	int err;
+
+	if (GENL_REQ_ATTR_CHECK(info, NETDEV_A_NAPI_ID))
+		return -EINVAL;
+
+	napi_id = nla_get_u32(info->attrs[NETDEV_A_NAPI_ID]);
+
+	rsp = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
+	if (!rsp)
+		return -ENOMEM;
+
+	rtnl_lock();
+
+	napi = napi_by_id(napi_id);
+	if (napi)
+		err = netdev_nl_napi_fill_one(rsp, napi, info);
+	else
+		err = -EINVAL;
+
+	rtnl_unlock();
+
+	if (err)
+		goto err_free_msg;
+
+	return genlmsg_reply(rsp, info);
+
+err_free_msg:
+	nlmsg_free(rsp);
+	return err;
+}
+
+static int
+netdev_nl_napi_dump_one(struct net_device *netdev, struct sk_buff *rsp,
+			const struct genl_info *info,
+			struct netdev_nl_dump_ctx *ctx)
+{
+	struct napi_struct *napi;
+	int err = 0;
+
+	list_for_each_entry(napi, &netdev->napi_list, dev_list) {
+		if (ctx->napi_id && napi->napi_id >= ctx->napi_id)
+			continue;
+
+		err = netdev_nl_napi_fill_one(rsp, napi, info);
+		if (err)
+			return err;
+		ctx->napi_id = napi->napi_id;
+	}
+	return err;
 }
 
 int netdev_nl_napi_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
 {
-	return -EOPNOTSUPP;
+	struct netdev_nl_dump_ctx *ctx = netdev_dump_ctx(cb);
+	const struct genl_info *info = genl_info_dump(cb);
+	struct net *net = sock_net(skb->sk);
+	struct net_device *netdev;
+	u32 ifindex = 0;
+	int err = 0;
+
+	if (info->attrs[NETDEV_A_NAPI_IFINDEX])
+		ifindex = nla_get_u32(info->attrs[NETDEV_A_NAPI_IFINDEX]);
+
+	rtnl_lock();
+	if (ifindex) {
+		netdev = __dev_get_by_index(net, ifindex);
+		if (netdev)
+			err = netdev_nl_napi_dump_one(netdev, skb, info, ctx);
+		else
+			err = -ENODEV;
+	} else {
+		for_each_netdev_dump(net, netdev, ctx->ifindex) {
+			err = netdev_nl_napi_dump_one(netdev, skb, info, ctx);
+			if (err < 0)
+				break;
+			ctx->napi_id = 0;
+		}
+	}
+	rtnl_unlock();
+
+	if (err != -EMSGSIZE)
+		return err;
+
+	return skb->len;
 }
 
 static int


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [net-next PATCH v8 07/10] netdev-genl: spec: Add irq in netdev netlink YAML spec
  2023-11-17  1:16 [net-next PATCH v8 00/10] Introduce queue and NAPI support in netdev-genl (Was: Introduce NAPI queues support) Amritha Nambiar
                   ` (5 preceding siblings ...)
  2023-11-17  1:17 ` [net-next PATCH v8 06/10] netdev-genl: Add netlink framework functions for napi Amritha Nambiar
@ 2023-11-17  1:17 ` Amritha Nambiar
  2023-11-17  1:17 ` [net-next PATCH v8 08/10] net: Add NAPI IRQ support Amritha Nambiar
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 21+ messages in thread
From: Amritha Nambiar @ 2023-11-17  1:17 UTC (permalink / raw)
  To: netdev, kuba, pabeni; +Cc: sridhar.samudrala, amritha.nambiar

Add support in netlink spec(netdev.yaml) for interrupt number
among the NAPI attributes. Add code generated from the spec.

Signed-off-by: Amritha Nambiar <amritha.nambiar@intel.com>
Reviewed-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
---
 Documentation/netlink/specs/netdev.yaml |    5 +++++
 include/uapi/linux/netdev.h             |    1 +
 tools/include/uapi/linux/netdev.h       |    1 +
 tools/net/ynl/generated/netdev-user.c   |    6 ++++++
 tools/net/ynl/generated/netdev-user.h   |    2 ++
 5 files changed, 15 insertions(+)

diff --git a/Documentation/netlink/specs/netdev.yaml b/Documentation/netlink/specs/netdev.yaml
index d68ab270618e..4161f302352d 100644
--- a/Documentation/netlink/specs/netdev.yaml
+++ b/Documentation/netlink/specs/netdev.yaml
@@ -104,6 +104,10 @@ attribute-sets:
         name: id
         doc: ID of the NAPI instance.
         type: u32
+      -
+        name: irq
+        doc: The associated interrupt vector number for the napi
+        type: u32
   -
     name: queue
     attributes:
@@ -197,6 +201,7 @@ operations:
           attributes:
             - id
             - ifindex
+            - irq
       dump:
         request:
           attributes:
diff --git a/include/uapi/linux/netdev.h b/include/uapi/linux/netdev.h
index ee590cbb9782..10ddb7cfde07 100644
--- a/include/uapi/linux/netdev.h
+++ b/include/uapi/linux/netdev.h
@@ -72,6 +72,7 @@ enum {
 enum {
 	NETDEV_A_NAPI_IFINDEX = 1,
 	NETDEV_A_NAPI_ID,
+	NETDEV_A_NAPI_IRQ,
 
 	__NETDEV_A_NAPI_MAX,
 	NETDEV_A_NAPI_MAX = (__NETDEV_A_NAPI_MAX - 1)
diff --git a/tools/include/uapi/linux/netdev.h b/tools/include/uapi/linux/netdev.h
index ee590cbb9782..10ddb7cfde07 100644
--- a/tools/include/uapi/linux/netdev.h
+++ b/tools/include/uapi/linux/netdev.h
@@ -72,6 +72,7 @@ enum {
 enum {
 	NETDEV_A_NAPI_IFINDEX = 1,
 	NETDEV_A_NAPI_ID,
+	NETDEV_A_NAPI_IRQ,
 
 	__NETDEV_A_NAPI_MAX,
 	NETDEV_A_NAPI_MAX = (__NETDEV_A_NAPI_MAX - 1)
diff --git a/tools/net/ynl/generated/netdev-user.c b/tools/net/ynl/generated/netdev-user.c
index a49265f4f4d9..2bd57420b548 100644
--- a/tools/net/ynl/generated/netdev-user.c
+++ b/tools/net/ynl/generated/netdev-user.c
@@ -101,6 +101,7 @@ struct ynl_policy_nest netdev_queue_nest = {
 struct ynl_policy_attr netdev_napi_policy[NETDEV_A_NAPI_MAX + 1] = {
 	[NETDEV_A_NAPI_IFINDEX] = { .name = "ifindex", .type = YNL_PT_U32, },
 	[NETDEV_A_NAPI_ID] = { .name = "id", .type = YNL_PT_U32, },
+	[NETDEV_A_NAPI_IRQ] = { .name = "irq", .type = YNL_PT_U32, },
 };
 
 struct ynl_policy_nest netdev_napi_nest = {
@@ -394,6 +395,11 @@ int netdev_napi_get_rsp_parse(const struct nlmsghdr *nlh, void *data)
 				return MNL_CB_ERROR;
 			dst->_present.ifindex = 1;
 			dst->ifindex = mnl_attr_get_u32(attr);
+		} else if (type == NETDEV_A_NAPI_IRQ) {
+			if (ynl_attr_validate(yarg, attr))
+				return MNL_CB_ERROR;
+			dst->_present.irq = 1;
+			dst->irq = mnl_attr_get_u32(attr);
 		}
 	}
 
diff --git a/tools/net/ynl/generated/netdev-user.h b/tools/net/ynl/generated/netdev-user.h
index 33ba75905ee1..de7205b63ccf 100644
--- a/tools/net/ynl/generated/netdev-user.h
+++ b/tools/net/ynl/generated/netdev-user.h
@@ -213,10 +213,12 @@ struct netdev_napi_get_rsp {
 	struct {
 		__u32 id:1;
 		__u32 ifindex:1;
+		__u32 irq:1;
 	} _present;
 
 	__u32 id;
 	__u32 ifindex;
+	__u32 irq;
 };
 
 void netdev_napi_get_rsp_free(struct netdev_napi_get_rsp *rsp);


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [net-next PATCH v8 08/10] net: Add NAPI IRQ support
  2023-11-17  1:16 [net-next PATCH v8 00/10] Introduce queue and NAPI support in netdev-genl (Was: Introduce NAPI queues support) Amritha Nambiar
                   ` (6 preceding siblings ...)
  2023-11-17  1:17 ` [net-next PATCH v8 07/10] netdev-genl: spec: Add irq in netdev netlink YAML spec Amritha Nambiar
@ 2023-11-17  1:17 ` Amritha Nambiar
  2023-11-17  1:17 ` [net-next PATCH v8 09/10] netdev-genl: spec: Add PID in netdev netlink YAML spec Amritha Nambiar
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 21+ messages in thread
From: Amritha Nambiar @ 2023-11-17  1:17 UTC (permalink / raw)
  To: netdev, kuba, pabeni; +Cc: sridhar.samudrala, amritha.nambiar

Add support to associate the interrupt vector number for a
NAPI instance.

Signed-off-by: Amritha Nambiar <amritha.nambiar@intel.com>
Reviewed-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_lib.c |    2 ++
 include/linux/netdevice.h                |    6 ++++++
 net/core/dev.c                           |    1 +
 net/core/netdev-genl.c                   |    4 ++++
 4 files changed, 13 insertions(+)

diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
index e2c24853376f..9662fbfef366 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_lib.c
@@ -2968,6 +2968,8 @@ void ice_q_vector_set_napi_queues(struct ice_q_vector *q_vector, bool locked)
 	ice_for_each_tx_ring(tx_ring, q_vector->tx)
 		ice_queue_set_napi(tx_ring->q_index, NETDEV_QUEUE_TYPE_TX,
 				   &q_vector->napi, locked);
+	/* Also set the interrupt number for the NAPI */
+	netif_napi_set_irq(&q_vector->napi, q_vector->irq.virq);
 }
 
 /**
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 8c8010e78240..c347ef0ebb54 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -382,6 +382,7 @@ struct napi_struct {
 	/* control-path-only fields follow */
 	struct list_head	dev_list;
 	struct hlist_node	napi_hash_node;
+	int			irq;
 };
 
 enum {
@@ -2650,6 +2651,11 @@ void __netif_queue_set_napi(unsigned int queue_index,
 			    enum netdev_queue_type type,
 			    struct napi_struct *napi);
 
+static inline void netif_napi_set_irq(struct napi_struct *napi, int irq)
+{
+	napi->irq = irq;
+}
+
 /* Default NAPI poll() weight
  * Device drivers are strongly advised to not use bigger value
  */
diff --git a/net/core/dev.c b/net/core/dev.c
index fcbfecdf5e5b..99ca59e18abf 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6479,6 +6479,7 @@ void netif_napi_add_weight(struct net_device *dev, struct napi_struct *napi,
 	 */
 	if (dev->threaded && napi_kthread_create(napi))
 		dev->threaded = 0;
+	netif_napi_set_irq(napi, -1);
 }
 EXPORT_SYMBOL(netif_napi_add_weight);
 
diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
index fda6d06995e0..d4d418c0e67e 100644
--- a/net/core/netdev-genl.c
+++ b/net/core/netdev-genl.c
@@ -167,7 +167,11 @@ netdev_nl_napi_fill_one(struct sk_buff *rsp, struct napi_struct *napi,
 	if (nla_put_u32(rsp, NETDEV_A_NAPI_IFINDEX, napi->dev->ifindex))
 		goto nla_put_failure;
 
+	if (napi->irq >= 0 && nla_put_u32(rsp, NETDEV_A_NAPI_IRQ, napi->irq))
+		goto nla_put_failure;
+
 	genlmsg_end(rsp, hdr);
+
 	return 0;
 
 nla_put_failure:


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [net-next PATCH v8 09/10] netdev-genl: spec: Add PID in netdev netlink YAML spec
  2023-11-17  1:16 [net-next PATCH v8 00/10] Introduce queue and NAPI support in netdev-genl (Was: Introduce NAPI queues support) Amritha Nambiar
                   ` (7 preceding siblings ...)
  2023-11-17  1:17 ` [net-next PATCH v8 08/10] net: Add NAPI IRQ support Amritha Nambiar
@ 2023-11-17  1:17 ` Amritha Nambiar
  2023-11-17  1:17 ` [net-next PATCH v8 10/10] netdev-genl: Add PID for the NAPI thread Amritha Nambiar
  2023-11-20 23:56 ` [PATCH 11/10] eth: bnxt: link NAPI instances to queues and IRQs Jakub Kicinski
  10 siblings, 0 replies; 21+ messages in thread
From: Amritha Nambiar @ 2023-11-17  1:17 UTC (permalink / raw)
  To: netdev, kuba, pabeni; +Cc: sridhar.samudrala, amritha.nambiar

Add support in netlink spec(netdev.yaml) for PID of the
NAPI thread. Add code generated from the spec.

Signed-off-by: Amritha Nambiar <amritha.nambiar@intel.com>
Reviewed-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
---
 Documentation/netlink/specs/netdev.yaml |    7 +++++++
 include/uapi/linux/netdev.h             |    1 +
 tools/include/uapi/linux/netdev.h       |    1 +
 tools/net/ynl/generated/netdev-user.c   |    6 ++++++
 tools/net/ynl/generated/netdev-user.h   |    2 ++
 5 files changed, 17 insertions(+)

diff --git a/Documentation/netlink/specs/netdev.yaml b/Documentation/netlink/specs/netdev.yaml
index 4161f302352d..382022a0d039 100644
--- a/Documentation/netlink/specs/netdev.yaml
+++ b/Documentation/netlink/specs/netdev.yaml
@@ -108,6 +108,12 @@ attribute-sets:
         name: irq
         doc: The associated interrupt vector number for the napi
         type: u32
+      -
+        name: pid
+        doc: PID of the napi thread, if NAPI is configured to operate in
+             threaded mode. If NAPI is not in threaded mode (i.e. uses normal
+             softirq context), the attribute will be absent.
+        type: u32
   -
     name: queue
     attributes:
@@ -202,6 +208,7 @@ operations:
             - id
             - ifindex
             - irq
+            - pid
       dump:
         request:
           attributes:
diff --git a/include/uapi/linux/netdev.h b/include/uapi/linux/netdev.h
index 10ddb7cfde07..c4be7fadfdf9 100644
--- a/include/uapi/linux/netdev.h
+++ b/include/uapi/linux/netdev.h
@@ -73,6 +73,7 @@ enum {
 	NETDEV_A_NAPI_IFINDEX = 1,
 	NETDEV_A_NAPI_ID,
 	NETDEV_A_NAPI_IRQ,
+	NETDEV_A_NAPI_PID,
 
 	__NETDEV_A_NAPI_MAX,
 	NETDEV_A_NAPI_MAX = (__NETDEV_A_NAPI_MAX - 1)
diff --git a/tools/include/uapi/linux/netdev.h b/tools/include/uapi/linux/netdev.h
index 10ddb7cfde07..c4be7fadfdf9 100644
--- a/tools/include/uapi/linux/netdev.h
+++ b/tools/include/uapi/linux/netdev.h
@@ -73,6 +73,7 @@ enum {
 	NETDEV_A_NAPI_IFINDEX = 1,
 	NETDEV_A_NAPI_ID,
 	NETDEV_A_NAPI_IRQ,
+	NETDEV_A_NAPI_PID,
 
 	__NETDEV_A_NAPI_MAX,
 	NETDEV_A_NAPI_MAX = (__NETDEV_A_NAPI_MAX - 1)
diff --git a/tools/net/ynl/generated/netdev-user.c b/tools/net/ynl/generated/netdev-user.c
index 2bd57420b548..5f3ca6ef9aa6 100644
--- a/tools/net/ynl/generated/netdev-user.c
+++ b/tools/net/ynl/generated/netdev-user.c
@@ -102,6 +102,7 @@ struct ynl_policy_attr netdev_napi_policy[NETDEV_A_NAPI_MAX + 1] = {
 	[NETDEV_A_NAPI_IFINDEX] = { .name = "ifindex", .type = YNL_PT_U32, },
 	[NETDEV_A_NAPI_ID] = { .name = "id", .type = YNL_PT_U32, },
 	[NETDEV_A_NAPI_IRQ] = { .name = "irq", .type = YNL_PT_U32, },
+	[NETDEV_A_NAPI_PID] = { .name = "pid", .type = YNL_PT_U32, },
 };
 
 struct ynl_policy_nest netdev_napi_nest = {
@@ -400,6 +401,11 @@ int netdev_napi_get_rsp_parse(const struct nlmsghdr *nlh, void *data)
 				return MNL_CB_ERROR;
 			dst->_present.irq = 1;
 			dst->irq = mnl_attr_get_u32(attr);
+		} else if (type == NETDEV_A_NAPI_PID) {
+			if (ynl_attr_validate(yarg, attr))
+				return MNL_CB_ERROR;
+			dst->_present.pid = 1;
+			dst->pid = mnl_attr_get_u32(attr);
 		}
 	}
 
diff --git a/tools/net/ynl/generated/netdev-user.h b/tools/net/ynl/generated/netdev-user.h
index de7205b63ccf..8337735389bf 100644
--- a/tools/net/ynl/generated/netdev-user.h
+++ b/tools/net/ynl/generated/netdev-user.h
@@ -214,11 +214,13 @@ struct netdev_napi_get_rsp {
 		__u32 id:1;
 		__u32 ifindex:1;
 		__u32 irq:1;
+		__u32 pid:1;
 	} _present;
 
 	__u32 id;
 	__u32 ifindex;
 	__u32 irq;
+	__u32 pid;
 };
 
 void netdev_napi_get_rsp_free(struct netdev_napi_get_rsp *rsp);


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [net-next PATCH v8 10/10] netdev-genl: Add PID for the NAPI thread
  2023-11-17  1:16 [net-next PATCH v8 00/10] Introduce queue and NAPI support in netdev-genl (Was: Introduce NAPI queues support) Amritha Nambiar
                   ` (8 preceding siblings ...)
  2023-11-17  1:17 ` [net-next PATCH v8 09/10] netdev-genl: spec: Add PID in netdev netlink YAML spec Amritha Nambiar
@ 2023-11-17  1:17 ` Amritha Nambiar
  2023-11-20 23:56 ` [PATCH 11/10] eth: bnxt: link NAPI instances to queues and IRQs Jakub Kicinski
  10 siblings, 0 replies; 21+ messages in thread
From: Amritha Nambiar @ 2023-11-17  1:17 UTC (permalink / raw)
  To: netdev, kuba, pabeni; +Cc: sridhar.samudrala, amritha.nambiar

In the threaded NAPI mode, expose the PID of the NAPI thread.

Signed-off-by: Amritha Nambiar <amritha.nambiar@intel.com>
Reviewed-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
---
 net/core/netdev-genl.c |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
index d4d418c0e67e..4f22b9ff6315 100644
--- a/net/core/netdev-genl.c
+++ b/net/core/netdev-genl.c
@@ -152,6 +152,7 @@ netdev_nl_napi_fill_one(struct sk_buff *rsp, struct napi_struct *napi,
 			const struct genl_info *info)
 {
 	void *hdr;
+	pid_t pid;
 
 	if (WARN_ON_ONCE(!napi->dev))
 		return -EINVAL;
@@ -170,6 +171,12 @@ netdev_nl_napi_fill_one(struct sk_buff *rsp, struct napi_struct *napi,
 	if (napi->irq >= 0 && nla_put_u32(rsp, NETDEV_A_NAPI_IRQ, napi->irq))
 		goto nla_put_failure;
 
+	if (napi->thread) {
+		pid = task_pid_nr(napi->thread);
+		if (nla_put_u32(rsp, NETDEV_A_NAPI_PID, pid))
+			goto nla_put_failure;
+	}
+
 	genlmsg_end(rsp, hdr);
 
 	return 0;


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [net-next PATCH v8 02/10] net: Add queue and napi association
  2023-11-17  1:16 ` [net-next PATCH v8 02/10] net: Add queue and napi association Amritha Nambiar
@ 2023-11-20 23:54   ` Jakub Kicinski
  2023-11-21 21:26     ` Nambiar, Amritha
  0 siblings, 1 reply; 21+ messages in thread
From: Jakub Kicinski @ 2023-11-20 23:54 UTC (permalink / raw)
  To: Amritha Nambiar; +Cc: netdev, pabeni, sridhar.samudrala

On Thu, 16 Nov 2023 17:16:48 -0800 Amritha Nambiar wrote:
> Add the napi pointer in netdev queue for tracking the napi
> instance for each queue. This achieves the queue<->napi mapping.

I took the series for a spin. I'll send a bnxt patch in a separate
reply, please add it to the series.

Three high level comments:

 - the netif_queue_set_napi() vs __netif_queue_set_napi() gave me pause;
   developers may be used to calling netif_*() functions from open/stop
   handlers, without worrying about locking.
   I think that netif_queue_set_napi() should assume rtnl_lock was
   already taken, as it will be in 90% of cases. A rare driver which
   does not hold it should take it locally for now.

 - drivers don't set real_num_*_queues to 0 when they go down,
   currently. So even tho we don't list any disabled queues when
   device is UP, we list queues when device is down.
   I mean:

   $ ifup eth0
   $ ethtool -L eth0 combined 4
   $ ./get-queues my-device
   ... will list 4 rx and 4 rx queues ...
   $ ethtool -L eth0 combined 2
   $ ./get-queues my-device
   ... will list 2 rx and 2 rx queues ...
   $ ifdown eth0
   $ ./get-queues my-device
   ... will list 2 rx and 2 rx queues ...
   ... even tho practically speaking there are no active queues ...

   I think we should skip listing queue and NAPI info of devices which
   are DOWN. Do you have any use case which would need looking at those?

 - We need a way to detach queues form NAPI. This is sort-of related to
   the above, maybe its not as crucial once we skip DOWN devices but
   nonetheless for symmetry we should let the driver clear the NAPI
   pointer. NAPIs may be allocated dynamically, the queue::napi pointer
   may get stale.

I hacked together the following for my local testing, feel free to fold
appropriate parts into your patches:

diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
index 1a0603b3529d..2ed7a3aeec40 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_lib.c
@@ -2948,10 +2948,11 @@ static void
 ice_queue_set_napi(unsigned int queue_index, enum netdev_queue_type type,
 		   struct napi_struct *napi, bool locked)
 {
-	if (locked)
-		__netif_queue_set_napi(queue_index, type, napi);
-	else
-		netif_queue_set_napi(queue_index, type, napi);
+	if (!locked)
+		rtnl_lock();
+	netif_queue_set_napi(napi->dev, queue_index, type, napi);
+	if (!locked)
+		rtnl_unlock();
 }
 
 /**
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index dbc4ea74b8d6..e09a039a092a 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2644,13 +2644,10 @@ static inline void *netdev_priv(const struct net_device *dev)
  */
 #define SET_NETDEV_DEVTYPE(net, devtype)	((net)->dev.type = (devtype))
 
-void netif_queue_set_napi(unsigned int queue_index, enum netdev_queue_type type,
+void netif_queue_set_napi(struct net_device *dev, unsigned int queue_index,
+			  enum netdev_queue_type type,
 			  struct napi_struct *napi);
 
-void __netif_queue_set_napi(unsigned int queue_index,
-			    enum netdev_queue_type type,
-			    struct napi_struct *napi);
-
 static inline void netif_napi_set_irq(struct napi_struct *napi, int irq)
 {
 	napi->irq = irq;
diff --git a/net/core/dev.c b/net/core/dev.c
index 99ca59e18abf..bb93240c69b9 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6400,25 +6400,27 @@ int dev_set_threaded(struct net_device *dev, bool threaded)
 EXPORT_SYMBOL(dev_set_threaded);
 
 /**
- * __netif_queue_set_napi - Associate queue with the napi
+ * netif_queue_set_napi - Associate queue with the napi
+ * @dev: device to which NAPI and queue belong
  * @queue_index: Index of queue
  * @type: queue type as RX or TX
- * @napi: NAPI context
+ * @napi: NAPI context, pass NULL to clear previously set NAPI
  *
  * Set queue with its corresponding napi context. This should be done after
  * registering the NAPI handler for the queue-vector and the queues have been
  * mapped to the corresponding interrupt vector.
  */
-void __netif_queue_set_napi(unsigned int queue_index,
-			    enum netdev_queue_type type,
-			    struct napi_struct *napi)
+void netif_queue_set_napi(struct net_device *dev, unsigned int queue_index,
+			  enum netdev_queue_type type,
+			  struct napi_struct *napi)
 {
-	struct net_device *dev = napi->dev;
 	struct netdev_rx_queue *rxq;
 	struct netdev_queue *txq;
 
-	if (WARN_ON_ONCE(!dev))
+	if (WARN_ON_ONCE(napi && !napi->dev))
 		return;
+	if (dev->reg_state >= NETREG_REGISTERED)
+		ASSERT_RTNL();
 
 	switch (type) {
 	case NETDEV_QUEUE_TYPE_RX:
@@ -6433,15 +6435,6 @@ void __netif_queue_set_napi(unsigned int queue_index,
 		return;
 	}
 }
-EXPORT_SYMBOL(__netif_queue_set_napi);
-
-void netif_queue_set_napi(unsigned int queue_index, enum netdev_queue_type type,
-			  struct napi_struct *napi)
-{
-	rtnl_lock();
-	__netif_queue_set_napi(queue_index, type, napi);
-	rtnl_unlock();
-}
 EXPORT_SYMBOL(netif_queue_set_napi);
 
 void netif_napi_add_weight(struct net_device *dev, struct napi_struct *napi,
-- 
2.42.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 11/10] eth: bnxt: link NAPI instances to queues and IRQs
  2023-11-17  1:16 [net-next PATCH v8 00/10] Introduce queue and NAPI support in netdev-genl (Was: Introduce NAPI queues support) Amritha Nambiar
                   ` (9 preceding siblings ...)
  2023-11-17  1:17 ` [net-next PATCH v8 10/10] netdev-genl: Add PID for the NAPI thread Amritha Nambiar
@ 2023-11-20 23:56 ` Jakub Kicinski
  2023-11-21 21:31   ` Nambiar, Amritha
  10 siblings, 1 reply; 21+ messages in thread
From: Jakub Kicinski @ 2023-11-20 23:56 UTC (permalink / raw)
  To: amritha.nambiar; +Cc: michael.chan, netdev, Jakub Kicinski

Make bnxt compatible with the newly added netlink queue GET APIs.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index e6ac1bd21bb3..ee4f4fc38bb5 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -3835,6 +3835,9 @@ static int bnxt_init_one_rx_ring(struct bnxt *bp, int ring_nr)
 	ring = &rxr->rx_ring_struct;
 	bnxt_init_rxbd_pages(ring, type);
 
+	netif_queue_set_napi(bp->dev, ring_nr, NETDEV_QUEUE_TYPE_RX,
+			     &rxr->bnapi->napi);
+
 	if (BNXT_RX_PAGE_MODE(bp) && bp->xdp_prog) {
 		bpf_prog_add(bp->xdp_prog, 1);
 		rxr->xdp_prog = bp->xdp_prog;
@@ -3911,6 +3914,9 @@ static int bnxt_init_tx_rings(struct bnxt *bp)
 		struct bnxt_ring_struct *ring = &txr->tx_ring_struct;
 
 		ring->fw_ring_id = INVALID_HW_RING_ID;
+
+		netif_queue_set_napi(bp->dev, i, NETDEV_QUEUE_TYPE_TX,
+				     &txr->bnapi->napi);
 	}
 
 	return 0;
@@ -9536,6 +9542,7 @@ static int bnxt_request_irq(struct bnxt *bp)
 		if (rc)
 			break;
 
+		netif_napi_set_irq(&bp->bnapi[i]->napi, irq->vector);
 		irq->requested = 1;
 
 		if (zalloc_cpumask_var(&irq->cpu_mask, GFP_KERNEL)) {
@@ -9563,6 +9570,11 @@ static void bnxt_del_napi(struct bnxt *bp)
 	if (!bp->bnapi)
 		return;
 
+	for (i = 0; i < bp->rx_nr_rings; i++)
+		netif_queue_set_napi(bp->dev, i, NETDEV_QUEUE_TYPE_RX, NULL);
+	for (i = 0; i < bp->tx_nr_rings; i++)
+		netif_queue_set_napi(bp->dev, i, NETDEV_QUEUE_TYPE_TX, NULL);
+
 	for (i = 0; i < bp->cp_nr_rings; i++) {
 		struct bnxt_napi *bnapi = bp->bnapi[i];
 
-- 
2.42.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [net-next PATCH v8 02/10] net: Add queue and napi association
  2023-11-20 23:54   ` Jakub Kicinski
@ 2023-11-21 21:26     ` Nambiar, Amritha
  2023-11-21 22:22       ` Jakub Kicinski
  0 siblings, 1 reply; 21+ messages in thread
From: Nambiar, Amritha @ 2023-11-21 21:26 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: netdev, pabeni, sridhar.samudrala

On 11/20/2023 3:54 PM, Jakub Kicinski wrote:
> On Thu, 16 Nov 2023 17:16:48 -0800 Amritha Nambiar wrote:
>> Add the napi pointer in netdev queue for tracking the napi
>> instance for each queue. This achieves the queue<->napi mapping.
> 
> I took the series for a spin. I'll send a bnxt patch in a separate
> reply, please add it to the series.
> 

Sure, will add the bnxt patch to v9.

> Three high level comments:
> 
>   - the netif_queue_set_napi() vs __netif_queue_set_napi() gave me pause;
>     developers may be used to calling netif_*() functions from open/stop
>     handlers, without worrying about locking.
>     I think that netif_queue_set_napi() should assume rtnl_lock was
>     already taken, as it will be in 90% of cases. A rare driver which
>     does not hold it should take it locally for now.
> 

Okay, will make these changes in v9.

>   - drivers don't set real_num_*_queues to 0 when they go down,
>     currently. So even tho we don't list any disabled queues when
>     device is UP, we list queues when device is down.
>     I mean:
> 
>     $ ifup eth0
>     $ ethtool -L eth0 combined 4
>     $ ./get-queues my-device
>     ... will list 4 rx and 4 rx queues ...
>     $ ethtool -L eth0 combined 2
>     $ ./get-queues my-device
>     ... will list 2 rx and 2 rx queues ...
>     $ ifdown eth0
>     $ ./get-queues my-device
>     ... will list 2 rx and 2 rx queues ...
>     ... even tho practically speaking there are no active queues ...
> 
>     I think we should skip listing queue and NAPI info of devices which
>     are DOWN. Do you have any use case which would need looking at those?
> 

So, currently, 'ethtool --show-channels' and 'ps -aef | grep napi' would 
list all the queues and NAPIs if the device is DOWN. I think what you 
are pointing at is:
<ifdown and ./get-queues> should show something similar to <ethtool -L 
eth0 combined 0 (0 is not valid... but almost to that effect) and 
./get-queues>.

But, 'ethtool -L' actually deletes the queues vs 'device DOWN' which 
only disables or makes the queues inactive.

Maybe as a follow-up patch, would it be better to have an additional 
parameter called 'state' for 'queues-get' and 'napi-get' that indicates 
the queues or NAPIs as active/inactive. The queue/NAPI state would be 
inherited from the device state. This way we can still list the 
queues/NAPIs when the device is down, set/update parameter 
configurations and then bring UP the device (in case where we stop 
traffic and tune parameters).

Also, if in future, we have the interface to tune parameters per-queue 
without full reset (of all queues or the device itself, as the hardware 
supports this), the 'state' would report this for specific queue as 
active/inactive. Maybe:
'queue-set' can set 'state = active' for a single queue '{"ifindex": 12, 
"id": 0, "type": 0}' and start a queue.

>   - We need a way to detach queues form NAPI. This is sort-of related to
>     the above, maybe its not as crucial once we skip DOWN devices but
>     nonetheless for symmetry we should let the driver clear the NAPI
>     pointer. NAPIs may be allocated dynamically, the queue::napi pointer
>     may get stale.

Okay, will handle this in v9.

> 
> I hacked together the following for my local testing, feel free to fold
> appropriate parts into your patches:
> 

Sure, thank you!

> diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
> index 1a0603b3529d..2ed7a3aeec40 100644
> --- a/drivers/net/ethernet/intel/ice/ice_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_lib.c
> @@ -2948,10 +2948,11 @@ static void
>   ice_queue_set_napi(unsigned int queue_index, enum netdev_queue_type type,
>   		   struct napi_struct *napi, bool locked)
>   {
> -	if (locked)
> -		__netif_queue_set_napi(queue_index, type, napi);
> -	else
> -		netif_queue_set_napi(queue_index, type, napi);
> +	if (!locked)
> +		rtnl_lock();
> +	netif_queue_set_napi(napi->dev, queue_index, type, napi);
> +	if (!locked)
> +		rtnl_unlock();
>   }
>   
>   /**
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index dbc4ea74b8d6..e09a039a092a 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -2644,13 +2644,10 @@ static inline void *netdev_priv(const struct net_device *dev)
>    */
>   #define SET_NETDEV_DEVTYPE(net, devtype)	((net)->dev.type = (devtype))
>   
> -void netif_queue_set_napi(unsigned int queue_index, enum netdev_queue_type type,
> +void netif_queue_set_napi(struct net_device *dev, unsigned int queue_index,
> +			  enum netdev_queue_type type,
>   			  struct napi_struct *napi);
>   
> -void __netif_queue_set_napi(unsigned int queue_index,
> -			    enum netdev_queue_type type,
> -			    struct napi_struct *napi);
> -
>   static inline void netif_napi_set_irq(struct napi_struct *napi, int irq)
>   {
>   	napi->irq = irq;
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 99ca59e18abf..bb93240c69b9 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -6400,25 +6400,27 @@ int dev_set_threaded(struct net_device *dev, bool threaded)
>   EXPORT_SYMBOL(dev_set_threaded);
>   
>   /**
> - * __netif_queue_set_napi - Associate queue with the napi
> + * netif_queue_set_napi - Associate queue with the napi
> + * @dev: device to which NAPI and queue belong
>    * @queue_index: Index of queue
>    * @type: queue type as RX or TX
> - * @napi: NAPI context
> + * @napi: NAPI context, pass NULL to clear previously set NAPI
>    *
>    * Set queue with its corresponding napi context. This should be done after
>    * registering the NAPI handler for the queue-vector and the queues have been
>    * mapped to the corresponding interrupt vector.
>    */
> -void __netif_queue_set_napi(unsigned int queue_index,
> -			    enum netdev_queue_type type,
> -			    struct napi_struct *napi)
> +void netif_queue_set_napi(struct net_device *dev, unsigned int queue_index,
> +			  enum netdev_queue_type type,
> +			  struct napi_struct *napi)
>   {
> -	struct net_device *dev = napi->dev;
>   	struct netdev_rx_queue *rxq;
>   	struct netdev_queue *txq;
>   
> -	if (WARN_ON_ONCE(!dev))
> +	if (WARN_ON_ONCE(napi && !napi->dev))
>   		return;
> +	if (dev->reg_state >= NETREG_REGISTERED)
> +		ASSERT_RTNL();
>   
>   	switch (type) {
>   	case NETDEV_QUEUE_TYPE_RX:
> @@ -6433,15 +6435,6 @@ void __netif_queue_set_napi(unsigned int queue_index,
>   		return;
>   	}
>   }
> -EXPORT_SYMBOL(__netif_queue_set_napi);
> -
> -void netif_queue_set_napi(unsigned int queue_index, enum netdev_queue_type type,
> -			  struct napi_struct *napi)
> -{
> -	rtnl_lock();
> -	__netif_queue_set_napi(queue_index, type, napi);
> -	rtnl_unlock();
> -}
>   EXPORT_SYMBOL(netif_queue_set_napi);
>   
>   void netif_napi_add_weight(struct net_device *dev, struct napi_struct *napi,

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 11/10] eth: bnxt: link NAPI instances to queues and IRQs
  2023-11-20 23:56 ` [PATCH 11/10] eth: bnxt: link NAPI instances to queues and IRQs Jakub Kicinski
@ 2023-11-21 21:31   ` Nambiar, Amritha
  0 siblings, 0 replies; 21+ messages in thread
From: Nambiar, Amritha @ 2023-11-21 21:31 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: michael.chan, netdev

On 11/20/2023 3:56 PM, Jakub Kicinski wrote:
> Make bnxt compatible with the newly added netlink queue GET APIs.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Thanks. Will add this as patch-11 to v9 of my series
(Introduce queue and NAPI support in netdev-genl).

> ---
>   drivers/net/ethernet/broadcom/bnxt/bnxt.c | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index e6ac1bd21bb3..ee4f4fc38bb5 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> @@ -3835,6 +3835,9 @@ static int bnxt_init_one_rx_ring(struct bnxt *bp, int ring_nr)
>   	ring = &rxr->rx_ring_struct;
>   	bnxt_init_rxbd_pages(ring, type);
>   
> +	netif_queue_set_napi(bp->dev, ring_nr, NETDEV_QUEUE_TYPE_RX,
> +			     &rxr->bnapi->napi);
> +
>   	if (BNXT_RX_PAGE_MODE(bp) && bp->xdp_prog) {
>   		bpf_prog_add(bp->xdp_prog, 1);
>   		rxr->xdp_prog = bp->xdp_prog;
> @@ -3911,6 +3914,9 @@ static int bnxt_init_tx_rings(struct bnxt *bp)
>   		struct bnxt_ring_struct *ring = &txr->tx_ring_struct;
>   
>   		ring->fw_ring_id = INVALID_HW_RING_ID;
> +
> +		netif_queue_set_napi(bp->dev, i, NETDEV_QUEUE_TYPE_TX,
> +				     &txr->bnapi->napi);
>   	}
>   
>   	return 0;
> @@ -9536,6 +9542,7 @@ static int bnxt_request_irq(struct bnxt *bp)
>   		if (rc)
>   			break;
>   
> +		netif_napi_set_irq(&bp->bnapi[i]->napi, irq->vector);
>   		irq->requested = 1;
>   
>   		if (zalloc_cpumask_var(&irq->cpu_mask, GFP_KERNEL)) {
> @@ -9563,6 +9570,11 @@ static void bnxt_del_napi(struct bnxt *bp)
>   	if (!bp->bnapi)
>   		return;
>   
> +	for (i = 0; i < bp->rx_nr_rings; i++)
> +		netif_queue_set_napi(bp->dev, i, NETDEV_QUEUE_TYPE_RX, NULL);
> +	for (i = 0; i < bp->tx_nr_rings; i++)
> +		netif_queue_set_napi(bp->dev, i, NETDEV_QUEUE_TYPE_TX, NULL);
> +
>   	for (i = 0; i < bp->cp_nr_rings; i++) {
>   		struct bnxt_napi *bnapi = bp->bnapi[i];
>   

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [net-next PATCH v8 02/10] net: Add queue and napi association
  2023-11-21 21:26     ` Nambiar, Amritha
@ 2023-11-21 22:22       ` Jakub Kicinski
  2023-11-22  0:08         ` Nambiar, Amritha
  0 siblings, 1 reply; 21+ messages in thread
From: Jakub Kicinski @ 2023-11-21 22:22 UTC (permalink / raw)
  To: Nambiar, Amritha; +Cc: netdev, pabeni, sridhar.samudrala

On Tue, 21 Nov 2023 13:26:27 -0800 Nambiar, Amritha wrote:
> So, currently, 'ethtool --show-channels' and 'ps -aef | grep napi' would 
> list all the queues and NAPIs if the device is DOWN. I think what you 
> are pointing at is:
> <ifdown and ./get-queues> should show something similar to <ethtool -L 
> eth0 combined 0 (0 is not valid... but almost to that effect) and 
> ./get-queues>.
> 
> But, 'ethtool -L' actually deletes the queues vs 'device DOWN' which 
> only disables or makes the queues inactive.
> 
> Maybe as a follow-up patch, would it be better to have an additional 
> parameter called 'state' for 'queues-get' and 'napi-get' that indicates 
> the queues or NAPIs as active/inactive. The queue/NAPI state would be 
> inherited from the device state. This way we can still list the 
> queues/NAPIs when the device is down, set/update parameter 
> configurations and then bring UP the device (in case where we stop 
> traffic and tune parameters).
> 
> Also, if in future, we have the interface to tune parameters per-queue 
> without full reset (of all queues or the device itself, as the hardware 
> supports this), the 'state' would report this for specific queue as 
> active/inactive. Maybe:
> 'queue-set' can set 'state = active' for a single queue '{"ifindex": 12, 
> "id": 0, "type": 0}' and start a queue.

To reiterate - the thing I find odd about the current situation is that
we hide the queues if they get disabled by lowering ethtool -L, but we
don't hide them when the entire interface is down. When the entire
interface is down there should be no queues, right?

Differently put - what logic that'd make sense to the user do we apply
when trying to decide if the queue is visible? < real_num_queues is
an implementation detail.

We can list all the queues, always, too. No preference. I just want to
make sure that the rules are clear and not very dependent on current
implementation and not different driver to driver.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [net-next PATCH v8 02/10] net: Add queue and napi association
  2023-11-21 22:22       ` Jakub Kicinski
@ 2023-11-22  0:08         ` Nambiar, Amritha
  2023-11-22  1:15           ` Jakub Kicinski
  0 siblings, 1 reply; 21+ messages in thread
From: Nambiar, Amritha @ 2023-11-22  0:08 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: netdev, pabeni, sridhar.samudrala

On 11/21/2023 2:22 PM, Jakub Kicinski wrote:
> On Tue, 21 Nov 2023 13:26:27 -0800 Nambiar, Amritha wrote:
>> So, currently, 'ethtool --show-channels' and 'ps -aef | grep napi' would
>> list all the queues and NAPIs if the device is DOWN. I think what you
>> are pointing at is:
>> <ifdown and ./get-queues> should show something similar to <ethtool -L
>> eth0 combined 0 (0 is not valid... but almost to that effect) and
>> ./get-queues>.
>>
>> But, 'ethtool -L' actually deletes the queues vs 'device DOWN' which
>> only disables or makes the queues inactive.
>>
>> Maybe as a follow-up patch, would it be better to have an additional
>> parameter called 'state' for 'queues-get' and 'napi-get' that indicates
>> the queues or NAPIs as active/inactive. The queue/NAPI state would be
>> inherited from the device state. This way we can still list the
>> queues/NAPIs when the device is down, set/update parameter
>> configurations and then bring UP the device (in case where we stop
>> traffic and tune parameters).
>>
>> Also, if in future, we have the interface to tune parameters per-queue
>> without full reset (of all queues or the device itself, as the hardware
>> supports this), the 'state' would report this for specific queue as
>> active/inactive. Maybe:
>> 'queue-set' can set 'state = active' for a single queue '{"ifindex": 12,
>> "id": 0, "type": 0}' and start a queue.
> 
> To reiterate - the thing I find odd about the current situation is that
> we hide the queues if they get disabled by lowering ethtool -L, but we
> don't hide them when the entire interface is down. When the entire
> interface is down there should be no queues, right?
> 

"When the entire interface is down there should be no queues" - 
currently, 'ethtool --show-channels' reports all the available queues 
when interface is DOWN (for all drivers, as drivers don't set 
real_num_queues to 0). Is this incorrect?

> Differently put - what logic that'd make sense to the user do we apply
> when trying to decide if the queue is visible? < real_num_queues is
> an implementation detail.
> 
> We can list all the queues, always, too. No preference. I just want to
> make sure that the rules are clear and not very dependent on current
> implementation and not different driver to driver.

I think currently, the queue dump results when the device is down aligns 
for both APIs (netdev-genl queue-get and ethtool show-channels) for all 
the drivers. If we decide to NOT show queues/NAPIs (with netdev-genl) 
when the device is down, the user would see conflicting results, the 
dump results with netdev-genl APIs would be different from what 'ethtool 
--show-channels' and 'ps -aef | grep napi' reports.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [net-next PATCH v8 02/10] net: Add queue and napi association
  2023-11-22  0:08         ` Nambiar, Amritha
@ 2023-11-22  1:15           ` Jakub Kicinski
  2023-11-22 21:28             ` Nambiar, Amritha
  0 siblings, 1 reply; 21+ messages in thread
From: Jakub Kicinski @ 2023-11-22  1:15 UTC (permalink / raw)
  To: Nambiar, Amritha, Willem de Bruijn; +Cc: netdev, pabeni, sridhar.samudrala

On Tue, 21 Nov 2023 16:08:07 -0800 Nambiar, Amritha wrote:
> > To reiterate - the thing I find odd about the current situation is that
> > we hide the queues if they get disabled by lowering ethtool -L, but we
> > don't hide them when the entire interface is down. When the entire
> > interface is down there should be no queues, right?
> 
> "When the entire interface is down there should be no queues" - 
> currently, 'ethtool --show-channels' reports all the available queues 
> when interface is DOWN

That's not the same. ethtool -l shows the configuration not 
the instantiated objects. ethtool -a will also show you the
pause settings even when cable is not plugged in.
sysfs objects of the queues are still exposed for devices which 
are down, that's true. But again, that's to expose the config.

> > Differently put - what logic that'd make sense to the user do we apply
> > when trying to decide if the queue is visible? < real_num_queues is
> > an implementation detail.
> > 
> > We can list all the queues, always, too. No preference. I just want to
> > make sure that the rules are clear and not very dependent on current
> > implementation and not different driver to driver.  
> 
> I think currently, the queue dump results when the device is down aligns 
> for both APIs (netdev-genl queue-get and ethtool show-channels) for all 
> the drivers. If we decide to NOT show queues/NAPIs (with netdev-genl) 
> when the device is down, the user would see conflicting results, the 
> dump results with netdev-genl APIs would be different from what 'ethtool 
> --show-channels' and 'ps -aef | grep napi' reports.

We should make the distinction between configuration and state of
instantiated objects clear before we get too far. Say we support
setting ring length for a specific queue. Global setting is 512,
queue X wants 256. How do we remove the override for queue X?
By setting it to 512? What if we want 512, and the default shifts
to something else? We'll need an explicit "reset" command.

I think it may be cleaner to keep queue-get as state of queues,
and configuration / settings / rules completely separate.

Am I wrong? Willem?

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [net-next PATCH v8 02/10] net: Add queue and napi association
  2023-11-22  1:15           ` Jakub Kicinski
@ 2023-11-22 21:28             ` Nambiar, Amritha
  2023-11-22 22:00               ` Jakub Kicinski
  0 siblings, 1 reply; 21+ messages in thread
From: Nambiar, Amritha @ 2023-11-22 21:28 UTC (permalink / raw)
  To: Jakub Kicinski, Willem de Bruijn; +Cc: netdev, pabeni, sridhar.samudrala

On 11/21/2023 5:15 PM, Jakub Kicinski wrote:
> On Tue, 21 Nov 2023 16:08:07 -0800 Nambiar, Amritha wrote:
>>> To reiterate - the thing I find odd about the current situation is that
>>> we hide the queues if they get disabled by lowering ethtool -L, but we
>>> don't hide them when the entire interface is down. When the entire
>>> interface is down there should be no queues, right?
>>
>> "When the entire interface is down there should be no queues" -
>> currently, 'ethtool --show-channels' reports all the available queues
>> when interface is DOWN
> 
> That's not the same. ethtool -l shows the configuration not
> the instantiated objects. ethtool -a will also show you the
> pause settings even when cable is not plugged in.
> sysfs objects of the queues are still exposed for devices which
> are down, that's true. But again, that's to expose the config.
> 
>>> Differently put - what logic that'd make sense to the user do we apply
>>> when trying to decide if the queue is visible? < real_num_queues is
>>> an implementation detail.
>>>
>>> We can list all the queues, always, too. No preference. I just want to
>>> make sure that the rules are clear and not very dependent on current
>>> implementation and not different driver to driver.
>>
>> I think currently, the queue dump results when the device is down aligns
>> for both APIs (netdev-genl queue-get and ethtool show-channels) for all
>> the drivers. If we decide to NOT show queues/NAPIs (with netdev-genl)
>> when the device is down, the user would see conflicting results, the
>> dump results with netdev-genl APIs would be different from what 'ethtool
>> --show-channels' and 'ps -aef | grep napi' reports.
> 
> We should make the distinction between configuration and state of
> instantiated objects clear before we get too far. Say we support
> setting ring length for a specific queue. Global setting is 512,
> queue X wants 256. How do we remove the override for queue X?
> By setting it to 512? What if we want 512, and the default shifts
> to something else? We'll need an explicit "reset" command.
> 
> I think it may be cleaner to keep queue-get as state of queues,
> and configuration / settings / rules completely separate.
> 
> Am I wrong? Willem?
> 

So, the instantiated netdev objects and their states are more aligned to 
the netdev-level than the actual configuration in the hardware.

WRT to showing queues when the device is down, I see your point, the 
hardware has those queues available, but when the interface is down, 
those queues are not valid at the netdev-level and need not be reported 
to the user. So, it is worth showing no results.
Also, my concern about showing the queues when the device is down, to do 
the user-configuration and then bring up the device, does not hold 
strong, as configuring when the device is up would also need a reset to 
make the updates in the hardware.

Trying to understand this distinction bit more:
So, netdev-genl queue-get shows state of queue objects as reported from 
the netdev level. Now, unless there's the queue-set command changing the 
configuration, the state of queue-objects would match the hardware 
configurations.
When the user changes the configuration with a queue-set command:
- queue-get would report the new updates (as obtained from the netdev).
- The updates would not be reflected in the hardware till a reset is 
issued. At this point, ethtool or others would report the older 
configuration (before reset).
- After reset, the state of queue objects from queue-get would match the 
actual hardware configuration.

I agree, an explicit "reset" user-command would be great. This way all 
the set operations for the netdev objects (queue, NAPI, page pool etc.) 
would stay at the netdev level without needing ndo_op for each, and then 
the "reset" command can trigger the ndo callback and actuate the 
hardware changes.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [net-next PATCH v8 02/10] net: Add queue and napi association
  2023-11-22 21:28             ` Nambiar, Amritha
@ 2023-11-22 22:00               ` Jakub Kicinski
  2023-11-23  0:56                 ` Nambiar, Amritha
  0 siblings, 1 reply; 21+ messages in thread
From: Jakub Kicinski @ 2023-11-22 22:00 UTC (permalink / raw)
  To: Nambiar, Amritha; +Cc: Willem de Bruijn, netdev, pabeni, sridhar.samudrala

On Wed, 22 Nov 2023 13:28:19 -0800 Nambiar, Amritha wrote:
> Trying to understand this distinction bit more:
> So, netdev-genl queue-get shows state of queue objects as reported from 
> the netdev level. Now, unless there's the queue-set command changing the 
> configuration, the state of queue-objects would match the hardware 
> configurations.
> When the user changes the configuration with a queue-set command:
> - queue-get would report the new updates (as obtained from the netdev).
> - The updates would not be reflected in the hardware till a reset is 
> issued. At this point, ethtool or others would report the older 
> configuration (before reset).
> - After reset, the state of queue objects from queue-get would match the 
> actual hardware configuration.
> 
> I agree, an explicit "reset" user-command would be great. This way all 
> the set operations for the netdev objects (queue, NAPI, page pool etc.) 
> would stay at the netdev level without needing ndo_op for each, and then 
> the "reset" command can trigger the ndo callback and actuate the 
> hardware changes.

How the changes are applied is a separate topic. I was only talking
about the fact that if the settings are controllable both at the device
level and queue level - the queue state is a result of combining device
settings with queue settings.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [net-next PATCH v8 02/10] net: Add queue and napi association
  2023-11-22 22:00               ` Jakub Kicinski
@ 2023-11-23  0:56                 ` Nambiar, Amritha
  0 siblings, 0 replies; 21+ messages in thread
From: Nambiar, Amritha @ 2023-11-23  0:56 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: Willem de Bruijn, netdev, pabeni, sridhar.samudrala

On 11/22/2023 2:00 PM, Jakub Kicinski wrote:
> On Wed, 22 Nov 2023 13:28:19 -0800 Nambiar, Amritha wrote:
>> Trying to understand this distinction bit more:
>> So, netdev-genl queue-get shows state of queue objects as reported from
>> the netdev level. Now, unless there's the queue-set command changing the
>> configuration, the state of queue-objects would match the hardware
>> configurations.
>> When the user changes the configuration with a queue-set command:
>> - queue-get would report the new updates (as obtained from the netdev).
>> - The updates would not be reflected in the hardware till a reset is
>> issued. At this point, ethtool or others would report the older
>> configuration (before reset).
>> - After reset, the state of queue objects from queue-get would match the
>> actual hardware configuration.
>>
>> I agree, an explicit "reset" user-command would be great. This way all
>> the set operations for the netdev objects (queue, NAPI, page pool etc.)
>> would stay at the netdev level without needing ndo_op for each, and then
>> the "reset" command can trigger the ndo callback and actuate the
>> hardware changes.
> 
> How the changes are applied is a separate topic. I was only talking
> about the fact that if the settings are controllable both at the device
> level and queue level - the queue state is a result of combining device
> settings with queue settings.

Okay, makes sense. Will fix in v9 and skip listing queues and NAPIs of 
devices which are DOWN.

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2023-11-23  0:57 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-17  1:16 [net-next PATCH v8 00/10] Introduce queue and NAPI support in netdev-genl (Was: Introduce NAPI queues support) Amritha Nambiar
2023-11-17  1:16 ` [net-next PATCH v8 01/10] netdev-genl: spec: Extend netdev netlink spec in YAML for queue Amritha Nambiar
2023-11-17  1:16 ` [net-next PATCH v8 02/10] net: Add queue and napi association Amritha Nambiar
2023-11-20 23:54   ` Jakub Kicinski
2023-11-21 21:26     ` Nambiar, Amritha
2023-11-21 22:22       ` Jakub Kicinski
2023-11-22  0:08         ` Nambiar, Amritha
2023-11-22  1:15           ` Jakub Kicinski
2023-11-22 21:28             ` Nambiar, Amritha
2023-11-22 22:00               ` Jakub Kicinski
2023-11-23  0:56                 ` Nambiar, Amritha
2023-11-17  1:16 ` [net-next PATCH v8 03/10] ice: Add support in the driver for associating queue with napi Amritha Nambiar
2023-11-17  1:16 ` [net-next PATCH v8 04/10] netdev-genl: Add netlink framework functions for queue Amritha Nambiar
2023-11-17  1:17 ` [net-next PATCH v8 05/10] netdev-genl: spec: Extend netdev netlink spec in YAML for NAPI Amritha Nambiar
2023-11-17  1:17 ` [net-next PATCH v8 06/10] netdev-genl: Add netlink framework functions for napi Amritha Nambiar
2023-11-17  1:17 ` [net-next PATCH v8 07/10] netdev-genl: spec: Add irq in netdev netlink YAML spec Amritha Nambiar
2023-11-17  1:17 ` [net-next PATCH v8 08/10] net: Add NAPI IRQ support Amritha Nambiar
2023-11-17  1:17 ` [net-next PATCH v8 09/10] netdev-genl: spec: Add PID in netdev netlink YAML spec Amritha Nambiar
2023-11-17  1:17 ` [net-next PATCH v8 10/10] netdev-genl: Add PID for the NAPI thread Amritha Nambiar
2023-11-20 23:56 ` [PATCH 11/10] eth: bnxt: link NAPI instances to queues and IRQs Jakub Kicinski
2023-11-21 21:31   ` Nambiar, Amritha

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).