* [net-next PATCH v5 01/10] netdev-genl: spec: Extend netdev netlink spec in YAML for queue
2023-10-19 0:06 [net-next PATCH v5 00/10] Introduce queue and NAPI support in netdev-genl (Was: Introduce NAPI queues support) Amritha Nambiar
@ 2023-10-19 0:06 ` Amritha Nambiar
2023-10-19 2:12 ` kernel test robot
2023-10-19 0:06 ` [net-next PATCH v5 02/10] net: Add queue and napi association Amritha Nambiar
` (8 subsequent siblings)
9 siblings, 1 reply; 24+ messages in thread
From: Amritha Nambiar @ 2023-10-19 0:06 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 | 48 ++++++++++
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 | 100 ++++++++++++++++++++
8 files changed, 372 insertions(+)
diff --git a/Documentation/netlink/specs/netdev.yaml b/Documentation/netlink/specs/netdev.yaml
index 14511b13f305..56d45995edfa 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,29 @@ attribute-sets:
type: u64
enum: xdp-rx-metadata
+ -
+ name: queue
+ attributes:
+ -
+ name: queue-id
+ doc: queue index
+ type: u32
+ -
+ name: ifindex
+ doc: netdev ifindex
+ type: u32
+ checks:
+ min: 1
+ -
+ name: queue-type
+ doc: queue type as rx, tx
+ type: u32
+ enum: queue-type
+ -
+ name: napi-id
+ doc: napi id
+ type: u32
+
operations:
list:
-
@@ -120,6 +147,27 @@ operations:
doc: Notification about device configuration being changed.
notify: dev-get
mcgrp: mgmt
+ -
+ name: queue-get
+ doc: queue information
+ attribute-set: queue
+ do:
+ request:
+ attributes:
+ - ifindex
+ - queue-type
+ - queue-id
+ reply: &queue-get-op
+ attributes:
+ - queue-id
+ - queue-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..a3997efb6786 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_QUEUE_ID = 1,
+ NETDEV_A_QUEUE_IFINDEX,
+ NETDEV_A_QUEUE_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..85d556a051db 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_QUEUE_TYPE + 1] = {
+ [NETDEV_A_QUEUE_IFINDEX] = NLA_POLICY_MIN(NLA_U32, 1),
+ [NETDEV_A_QUEUE_QUEUE_TYPE] = NLA_POLICY_MAX(NLA_U32, 1),
+ [NETDEV_A_QUEUE_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_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..a3997efb6786 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_QUEUE_ID = 1,
+ NETDEV_A_QUEUE_IFINDEX,
+ NETDEV_A_QUEUE_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..9d6f96d80ba1 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_QUEUE_ID] = { .name = "queue-id", .type = YNL_PT_U32, },
+ [NETDEV_A_QUEUE_IFINDEX] = { .name = "ifindex", .type = YNL_PT_U32, },
+ [NETDEV_A_QUEUE_QUEUE_TYPE] = { .name = "queue-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_QUEUE_ID) {
+ if (ynl_attr_validate(yarg, attr))
+ return MNL_CB_ERROR;
+ dst->_present.queue_id = 1;
+ dst->queue_id = mnl_attr_get_u32(attr);
+ } else if (type == NETDEV_A_QUEUE_QUEUE_TYPE) {
+ if (ynl_attr_validate(yarg, attr))
+ return MNL_CB_ERROR;
+ dst->_present.queue_type = 1;
+ dst->queue_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.queue_type)
+ mnl_attr_put_u32(nlh, NETDEV_A_QUEUE_QUEUE_TYPE, req->queue_type);
+ if (req->_present.queue_id)
+ mnl_attr_put_u32(nlh, NETDEV_A_QUEUE_QUEUE_ID, req->queue_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 b4351ff34595..bb01368a0595 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,103 @@ 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 queue_type:1;
+ __u32 queue_id:1;
+ } _present;
+
+ __u32 ifindex;
+ enum netdev_queue_type queue_type;
+ __u32 queue_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_queue_type(struct netdev_queue_get_req *req,
+ enum netdev_queue_type queue_type)
+{
+ req->_present.queue_type = 1;
+ req->queue_type = queue_type;
+}
+static inline void
+netdev_queue_get_req_set_queue_id(struct netdev_queue_get_req *req,
+ __u32 queue_id)
+{
+ req->_present.queue_id = 1;
+ req->queue_id = queue_id;
+}
+
+struct netdev_queue_get_rsp {
+ struct {
+ __u32 queue_id:1;
+ __u32 queue_type:1;
+ __u32 napi_id:1;
+ __u32 ifindex:1;
+ } _present;
+
+ __u32 queue_id;
+ enum netdev_queue_type queue_type;
+ __u32 napi_id;
+ __u32 ifindex;
+};
+
+void netdev_queue_get_rsp_free(struct netdev_queue_get_rsp *rsp);
+
+/*
+ * queue information
+ */
+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] 24+ messages in thread* Re: [net-next PATCH v5 01/10] netdev-genl: spec: Extend netdev netlink spec in YAML for queue
2023-10-19 0:06 ` [net-next PATCH v5 01/10] netdev-genl: spec: Extend netdev netlink spec in YAML for queue Amritha Nambiar
@ 2023-10-19 2:12 ` kernel test robot
2023-10-20 20:26 ` Nambiar, Amritha
0 siblings, 1 reply; 24+ messages in thread
From: kernel test robot @ 2023-10-19 2:12 UTC (permalink / raw)
To: Amritha Nambiar, netdev, kuba, pabeni; +Cc: oe-kbuild-all
Hi Amritha,
kernel test robot noticed the following build warnings:
[auto build test WARNING on net-next/main]
url: https://github.com/intel-lab-lkp/linux/commits/Amritha-Nambiar/netdev-genl-spec-Extend-netdev-netlink-spec-in-YAML-for-queue/20231019-082941
base: net-next/main
patch link: https://lore.kernel.org/r/169767396671.6692.9945461089943525792.stgit%40anambiarhost.jf.intel.com
patch subject: [net-next PATCH v5 01/10] netdev-genl: spec: Extend netdev netlink spec in YAML for queue
reproduce: (https://download.01.org/0day-ci/archive/20231019/202310190900.9Dzgkbev-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310190900.9Dzgkbev-lkp@intel.com/
# many are suggestions rather than must-fix
WARNING:SPACING: space prohibited between function name and open parenthesis '('
#547: FILE: tools/net/ynl/generated/netdev-user.h:181:
+ struct netdev_queue_get_rsp obj __attribute__ ((aligned (8)));
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [net-next PATCH v5 01/10] netdev-genl: spec: Extend netdev netlink spec in YAML for queue
2023-10-19 2:12 ` kernel test robot
@ 2023-10-20 20:26 ` Nambiar, Amritha
2023-10-20 22:05 ` Jakub Kicinski
0 siblings, 1 reply; 24+ messages in thread
From: Nambiar, Amritha @ 2023-10-20 20:26 UTC (permalink / raw)
To: kernel test robot, netdev, kuba, pabeni; +Cc: oe-kbuild-all
On 10/18/2023 7:12 PM, kernel test robot wrote:
> Hi Amritha,
>
> kernel test robot noticed the following build warnings:
>
> [auto build test WARNING on net-next/main]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Amritha-Nambiar/netdev-genl-spec-Extend-netdev-netlink-spec-in-YAML-for-queue/20231019-082941
> base: net-next/main
> patch link: https://lore.kernel.org/r/169767396671.6692.9945461089943525792.stgit%40anambiarhost.jf.intel.com
> patch subject: [net-next PATCH v5 01/10] netdev-genl: spec: Extend netdev netlink spec in YAML for queue
> reproduce: (https://download.01.org/0day-ci/archive/20231019/202310190900.9Dzgkbev-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202310190900.9Dzgkbev-lkp@intel.com/
>
> # many are suggestions rather than must-fix
>
> WARNING:SPACING: space prohibited between function name and open parenthesis '('
> #547: FILE: tools/net/ynl/generated/netdev-user.h:181:
> + struct netdev_queue_get_rsp obj __attribute__ ((aligned (8)));
>
Looks like the series is in "Changes Requested" state following these
warnings. Is there any recommendation for warnings on generated code? I
see similar issues on existing code in the generated file.
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [net-next PATCH v5 01/10] netdev-genl: spec: Extend netdev netlink spec in YAML for queue
2023-10-20 20:26 ` Nambiar, Amritha
@ 2023-10-20 22:05 ` Jakub Kicinski
2023-10-21 0:37 ` Nambiar, Amritha
2023-10-21 1:53 ` Philip Li
0 siblings, 2 replies; 24+ messages in thread
From: Jakub Kicinski @ 2023-10-20 22:05 UTC (permalink / raw)
To: Nambiar, Amritha, oe-kbuild-all; +Cc: kernel test robot, netdev, pabeni
On Fri, 20 Oct 2023 13:26:38 -0700 Nambiar, Amritha wrote:
> > WARNING:SPACING: space prohibited between function name and open parenthesis '('
> > #547: FILE: tools/net/ynl/generated/netdev-user.h:181:
> > + struct netdev_queue_get_rsp obj __attribute__ ((aligned (8)));
>
> Looks like the series is in "Changes Requested" state following these
> warnings. Is there any recommendation for warnings on generated code? I
> see similar issues on existing code in the generated file.
Yes, ignore this one. I'll post a change to the codegen.
The warning on patch 3 is legit, right?
kernel test bot folks, please be careful with the checkpatch warnings.
Some of them are bogus. TBH I'm not sure how much value running
checkpatch in the bot adds. It's really trivial to run for the
maintainers when applying patches.
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [net-next PATCH v5 01/10] netdev-genl: spec: Extend netdev netlink spec in YAML for queue
2023-10-20 22:05 ` Jakub Kicinski
@ 2023-10-21 0:37 ` Nambiar, Amritha
2023-10-21 1:53 ` Philip Li
1 sibling, 0 replies; 24+ messages in thread
From: Nambiar, Amritha @ 2023-10-21 0:37 UTC (permalink / raw)
To: Jakub Kicinski, oe-kbuild-all; +Cc: kernel test robot, netdev, pabeni
On 10/20/2023 3:05 PM, Jakub Kicinski wrote:
> On Fri, 20 Oct 2023 13:26:38 -0700 Nambiar, Amritha wrote:
>>> WARNING:SPACING: space prohibited between function name and open parenthesis '('
>>> #547: FILE: tools/net/ynl/generated/netdev-user.h:181:
>>> + struct netdev_queue_get_rsp obj __attribute__ ((aligned (8)));
>>
>> Looks like the series is in "Changes Requested" state following these
>> warnings. Is there any recommendation for warnings on generated code? I
>> see similar issues on existing code in the generated file.
>
> Yes, ignore this one. I'll post a change to the codegen.
> The warning on patch 3 is legit, right?
>
Thanks for the fix to codegen.
Yes, the warning on patch 3 is legit. I'll fix it up in the next version
together with addressing other review feedback on this v5 series.
> kernel test bot folks, please be careful with the checkpatch warnings.
> Some of them are bogus. TBH I'm not sure how much value running
> checkpatch in the bot adds. It's really trivial to run for the
> maintainers when applying patches.
>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [net-next PATCH v5 01/10] netdev-genl: spec: Extend netdev netlink spec in YAML for queue
2023-10-20 22:05 ` Jakub Kicinski
2023-10-21 0:37 ` Nambiar, Amritha
@ 2023-10-21 1:53 ` Philip Li
2023-10-23 14:52 ` Jakub Kicinski
1 sibling, 1 reply; 24+ messages in thread
From: Philip Li @ 2023-10-21 1:53 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Nambiar, Amritha, oe-kbuild-all, kernel test robot, netdev,
pabeni
On Fri, Oct 20, 2023 at 03:05:57PM -0700, Jakub Kicinski wrote:
> On Fri, 20 Oct 2023 13:26:38 -0700 Nambiar, Amritha wrote:
> > > WARNING:SPACING: space prohibited between function name and open parenthesis '('
> > > #547: FILE: tools/net/ynl/generated/netdev-user.h:181:
> > > + struct netdev_queue_get_rsp obj __attribute__ ((aligned (8)));
> >
> > Looks like the series is in "Changes Requested" state following these
> > warnings. Is there any recommendation for warnings on generated code? I
> > see similar issues on existing code in the generated file.
>
> Yes, ignore this one. I'll post a change to the codegen.
> The warning on patch 3 is legit, right?
>
> kernel test bot folks, please be careful with the checkpatch warnings.
Thanks Jakub, we will be very careful and continuously monitor the
reports and feedbacks to filter unnecessary warnings/errors from
checkpatch.
> Some of them are bogus. TBH I'm not sure how much value running
> checkpatch in the bot adds. It's really trivial to run for the
It is found there're quite some checkpatch related fix commits on
mainline. Thus the bot wants to extend the coverage and do shift
left testing on developer repos and mailing list patches.
But as you mentioned above, we will take furture care to the output
of checkpatch to be conservative for the reporting.
> maintainers when applying patches.
>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [net-next PATCH v5 01/10] netdev-genl: spec: Extend netdev netlink spec in YAML for queue
2023-10-21 1:53 ` Philip Li
@ 2023-10-23 14:52 ` Jakub Kicinski
2023-10-23 19:09 ` Nambiar, Amritha
2023-10-24 1:02 ` Philip Li
0 siblings, 2 replies; 24+ messages in thread
From: Jakub Kicinski @ 2023-10-23 14:52 UTC (permalink / raw)
To: Philip Li
Cc: Nambiar, Amritha, oe-kbuild-all, kernel test robot, netdev,
pabeni
On Sat, 21 Oct 2023 09:53:03 +0800 Philip Li wrote:
> > Some of them are bogus. TBH I'm not sure how much value running
> > checkpatch in the bot adds. It's really trivial to run for the
>
> It is found there're quite some checkpatch related fix commits on
> mainline.
Those changes are mostly for old code, aren't they?
It'd be useful to do some analysis of how long ago the mis-formatted
code has been introduced. Because if new code doesn't get fixes
there's no point testing new patches..
> Thus the bot wants to extend the coverage and do shift
> left testing on developer repos and mailing list patches.
I understand and appreciate the effort.
I think that false positive has about a 100x the negative effect of a
true positive. If more than 1% of checkpatch warnings are ignored, we
should *not* report them to the list. Currently in networking we fully
trust the build bot and as soon as a patch set gets a reply from you it
gets auto-dropped from our review queue.
It'd be quite bad if we have to double check the reports.
Speaking of false positive rate - we disabled some checks in our own
use of checkpatch:
https://github.com/kuba-moo/nipa/blob/master/tests/patch/checkpatch/checkpatch.sh#L6-L12
and we still get about 26% false positive rate! (Count by looking at
checks that failed and were ignored, because patch was merged anyway).
A lot of those may be line length related (we still prefer 80 char
limit) but even without that - checkpatch false positives a lot.
And the maintainer is not very receptive to improvements for false
positives:
https://lore.kernel.org/all/20231013172739.1113964-1-kuba@kernel.org/
> But as you mentioned above, we will take furture care to the output
> of checkpatch to be conservative for the reporting.
FWIW the most issues that "get through" in networking are issues
in documentation (warnings for make htmldocs) :(
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [net-next PATCH v5 01/10] netdev-genl: spec: Extend netdev netlink spec in YAML for queue
2023-10-23 14:52 ` Jakub Kicinski
@ 2023-10-23 19:09 ` Nambiar, Amritha
2023-10-23 22:39 ` Jakub Kicinski
2023-10-24 1:02 ` Philip Li
1 sibling, 1 reply; 24+ messages in thread
From: Nambiar, Amritha @ 2023-10-23 19:09 UTC (permalink / raw)
To: Jakub Kicinski, Philip Li
Cc: oe-kbuild-all, kernel test robot, netdev, pabeni
On 10/23/2023 7:52 AM, Jakub Kicinski wrote:
> On Sat, 21 Oct 2023 09:53:03 +0800 Philip Li wrote:
>>> Some of them are bogus. TBH I'm not sure how much value running
>>> checkpatch in the bot adds. It's really trivial to run for the
>>
>> It is found there're quite some checkpatch related fix commits on
>> mainline.
>
> Those changes are mostly for old code, aren't they?
> It'd be useful to do some analysis of how long ago the mis-formatted
> code has been introduced. Because if new code doesn't get fixes
> there's no point testing new patches..
>
>> Thus the bot wants to extend the coverage and do shift
>> left testing on developer repos and mailing list patches.
>
> I understand and appreciate the effort.
>
> I think that false positive has about a 100x the negative effect of a
> true positive. If more than 1% of checkpatch warnings are ignored, we
> should *not* report them to the list. Currently in networking we fully
> trust the build bot and as soon as a patch set gets a reply from you it
> gets auto-dropped from our review queue.
Hi Jakub,
Just checking if this series is dropped from the review queue because of
the build bot warnings...
should I submit a v6 with the single line fix for the warning (legit) on
patch-3, or
should I wait for more feedback (if there is a chance this v5 series
would still be reviewed) and address them all together submitting v6.
> It'd be quite bad if we have to double check the reports.
>
> Speaking of false positive rate - we disabled some checks in our own
> use of checkpatch:
> https://github.com/kuba-moo/nipa/blob/master/tests/patch/checkpatch/checkpatch.sh#L6-L12
> and we still get about 26% false positive rate! (Count by looking at
> checks that failed and were ignored, because patch was merged anyway).
> A lot of those may be line length related (we still prefer 80 char
> limit) but even without that - checkpatch false positives a lot.
>
> And the maintainer is not very receptive to improvements for false
> positives:
> https://lore.kernel.org/all/20231013172739.1113964-1-kuba@kernel.org/
>
>> But as you mentioned above, we will take furture care to the output
>> of checkpatch to be conservative for the reporting.
>
> FWIW the most issues that "get through" in networking are issues
> in documentation (warnings for make htmldocs) :(
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [net-next PATCH v5 01/10] netdev-genl: spec: Extend netdev netlink spec in YAML for queue
2023-10-23 19:09 ` Nambiar, Amritha
@ 2023-10-23 22:39 ` Jakub Kicinski
0 siblings, 0 replies; 24+ messages in thread
From: Jakub Kicinski @ 2023-10-23 22:39 UTC (permalink / raw)
To: Nambiar, Amritha
Cc: Philip Li, oe-kbuild-all, kernel test robot, netdev, pabeni
On Mon, 23 Oct 2023 12:09:33 -0700 Nambiar, Amritha wrote:
> Just checking if this series is dropped from the review queue because of
> the build bot warnings...
> should I submit a v6 with the single line fix for the warning (legit) on
> patch-3, or
> should I wait for more feedback (if there is a chance this v5 series
> would still be reviewed) and address them all together submitting v6.
Repost please, I almost never review stuff that doesn't build cleanly.
I'll have to re-review before applying, anyway.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [net-next PATCH v5 01/10] netdev-genl: spec: Extend netdev netlink spec in YAML for queue
2023-10-23 14:52 ` Jakub Kicinski
2023-10-23 19:09 ` Nambiar, Amritha
@ 2023-10-24 1:02 ` Philip Li
2023-10-24 1:44 ` Jakub Kicinski
1 sibling, 1 reply; 24+ messages in thread
From: Philip Li @ 2023-10-24 1:02 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Nambiar, Amritha, oe-kbuild-all, kernel test robot, netdev,
pabeni
On Mon, Oct 23, 2023 at 07:52:21AM -0700, Jakub Kicinski wrote:
> On Sat, 21 Oct 2023 09:53:03 +0800 Philip Li wrote:
> > > Some of them are bogus. TBH I'm not sure how much value running
> > > checkpatch in the bot adds. It's really trivial to run for the
> >
> > It is found there're quite some checkpatch related fix commits on
> > mainline.
>
> Those changes are mostly for old code, aren't they?
Got it, we don't have the statistics data for this yet. I think this
is helpful for us to understand the status in depth. We will think of
this to gather some data.
> It'd be useful to do some analysis of how long ago the mis-formatted
> code has been introduced. Because if new code doesn't get fixes
> there's no point testing new patches..
>
> > Thus the bot wants to extend the coverage and do shift
> > left testing on developer repos and mailing list patches.
>
> I understand and appreciate the effort.
>
> I think that false positive has about a 100x the negative effect of a
> true positive. If more than 1% of checkpatch warnings are ignored, we
> should *not* report them to the list. Currently in networking we fully
> trust the build bot and as soon as a patch set gets a reply from you it
> gets auto-dropped from our review queue.
Thanks for the trust. Sorry I didn't notice the false checkpatch report leads
to trouble. From below info, may i understand networking already runs own
checkpatch? Also consider the checkpatch reports from bot still contains quite
some false ones, probably we can pause the checkpatch reporting for network
side if it doesn't add much value and causes trouble?
> It'd be quite bad if we have to double check the reports.
Got it, We are aware of keeping the reports in high quality to make
it fully useful. Usually for static analysis tool like sparse/smatch/cocci,
we maintain both high confidence pattern and low confidence one, which is
continously improved per developer feedback and our own analysis. For low
confidence one, it need manual check to be sent out.
Since we fully enable checkpatch not long ago, it causes various negative
reports. Sorry about this.
>
> Speaking of false positive rate - we disabled some checks in our own
> use of checkpatch:
> https://github.com/kuba-moo/nipa/blob/master/tests/patch/checkpatch/checkpatch.sh#L6-L12
> and we still get about 26% false positive rate! (Count by looking at
> checks that failed and were ignored, because patch was merged anyway).
> A lot of those may be line length related (we still prefer 80 char
> limit) but even without that - checkpatch false positives a lot.
Thanks for the great info, this is very helpful to us to learn from
this. We will adopt some practices here and continue improving the
reports.
>
> And the maintainer is not very receptive to improvements for false
> positives:
> https://lore.kernel.org/all/20231013172739.1113964-1-kuba@kernel.org/
I see. We got this pattern as well, what we do now is to maintain the pattern
internally to avoid unnecessary reports (some are extracted below). I'm looking
for publishing these patterns later, which may get more inputs to filter out
unnecessary reports.
== part of low confidence patterns of checkpatch in bot ==
__func__ should be used instead of gcc specific __FUNCTION__
line over 80 characters
LINUX_VERSION_CODE should be avoided, code should be for the version to which it is merged
Missing commit description - Add an appropriate one
please write a help paragraph that fully describes the config symbol
Possible repeated word: 'Google'
Possible unwrapped commit description \(prefer a maximum 75 chars per line\)
>
> > But as you mentioned above, we will take furture care to the output
> > of checkpatch to be conservative for the reporting.
>
> FWIW the most issues that "get through" in networking are issues
> in documentation (warnings for make htmldocs) :(
Do you suggest that warnings for make htmldocs or kernel-doc warning when building
with W=1 can be ignored and no need to send them to networking side?
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [net-next PATCH v5 01/10] netdev-genl: spec: Extend netdev netlink spec in YAML for queue
2023-10-24 1:02 ` Philip Li
@ 2023-10-24 1:44 ` Jakub Kicinski
2023-10-24 2:00 ` Philip Li
0 siblings, 1 reply; 24+ messages in thread
From: Jakub Kicinski @ 2023-10-24 1:44 UTC (permalink / raw)
To: Philip Li
Cc: Nambiar, Amritha, oe-kbuild-all, kernel test robot, netdev,
pabeni
On Tue, 24 Oct 2023 09:02:46 +0800 Philip Li wrote:
> > I understand and appreciate the effort.
> >
> > I think that false positive has about a 100x the negative effect of a
> > true positive. If more than 1% of checkpatch warnings are ignored, we
> > should *not* report them to the list. Currently in networking we fully
> > trust the build bot and as soon as a patch set gets a reply from you it
> > gets auto-dropped from our review queue.
>
> Thanks for the trust. Sorry I didn't notice the false checkpatch report leads
> to trouble. From below info, may i understand networking already runs own
> checkpatch? Also consider the checkpatch reports from bot still contains quite
> some false ones, probably we can pause the checkpatch reporting for network
> side if it doesn't add much value and causes trouble?
Yes, correct, we already run checkpatch --strict on all patches.
If you have the ability to selectively disable checkpatch for net/ and
drivers/net, and/or patches which CC netdev@vger, that'd be great!
FWIW we have a simple dashboard reporting which checks in our own
local build fail the most: https://netdev.bots.linux.dev/checks.html
Not sure if it's of any interest to you, but that's where I got the
false positive rate I mentioned previously.
> > And the maintainer is not very receptive to improvements for false
> > positives:
> > https://lore.kernel.org/all/20231013172739.1113964-1-kuba@kernel.org/
>
> I see. We got this pattern as well, what we do now is to maintain the pattern
> internally to avoid unnecessary reports (some are extracted below). I'm looking
> for publishing these patterns later, which may get more inputs to filter out
> unnecessary reports.
>
> == part of low confidence patterns of checkpatch in bot ==
Interesting!
> __func__ should be used instead of gcc specific __FUNCTION__
This one I don't see failing often.
> line over 80 characters
This one happens a lot, yes.
> LINUX_VERSION_CODE should be avoided, code should be for the version to which it is merged
This is very rare upstream.
> Missing commit description - Add an appropriate one
Should be rare upstream..
> please write a help paragraph that fully describes the config symbol
This check I think is semi-broken in checkpatch.
Sometimes it just doesn't recognize the help even if symbol has it.
So yes, we see if false-positive as well.
> Possible repeated word: 'Google'
Yes! :)
> Possible unwrapped commit description \(prefer a maximum 75 chars per line\)
This one indeed has a lot of false positives. It should check if
*majority* of the commit message lines (excluding tags) are too long,
not any single line. Because one line can be a crash dump or a commit
reference, and be longer for legit reasons..
Every now and then I feel like we should fork checkpatch or start a new
tool which would report only high-confidence problems.
> > > But as you mentioned above, we will take furture care to the output
> > > of checkpatch to be conservative for the reporting.
> >
> > FWIW the most issues that "get through" in networking are issues
> > in documentation (warnings for make htmldocs) :(
>
> Do you suggest that warnings for make htmldocs or kernel-doc warning when building
> with W=1 can be ignored and no need to send them to networking side?
No, no, the opposite! Documentation is one part we currently don't test,
even tho we should.
Do you run make htmldocs as part of kernel build bot? As you allude to -
W=1 checks kdoc already, and scripts/kernel-doc can be used to validate
headers even more easily. But to validate the ReST files under
Documentation/ one has to actually run make htmldocs (or perhaps some
other docs target), not just a normal build.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [net-next PATCH v5 01/10] netdev-genl: spec: Extend netdev netlink spec in YAML for queue
2023-10-24 1:44 ` Jakub Kicinski
@ 2023-10-24 2:00 ` Philip Li
0 siblings, 0 replies; 24+ messages in thread
From: Philip Li @ 2023-10-24 2:00 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Nambiar, Amritha, oe-kbuild-all, kernel test robot, netdev,
pabeni
On Mon, Oct 23, 2023 at 06:44:11PM -0700, Jakub Kicinski wrote:
> On Tue, 24 Oct 2023 09:02:46 +0800 Philip Li wrote:
> > > I understand and appreciate the effort.
> > >
> > > I think that false positive has about a 100x the negative effect of a
> > > true positive. If more than 1% of checkpatch warnings are ignored, we
> > > should *not* report them to the list. Currently in networking we fully
> > > trust the build bot and as soon as a patch set gets a reply from you it
> > > gets auto-dropped from our review queue.
> >
> > Thanks for the trust. Sorry I didn't notice the false checkpatch report leads
> > to trouble. From below info, may i understand networking already runs own
> > checkpatch? Also consider the checkpatch reports from bot still contains quite
> > some false ones, probably we can pause the checkpatch reporting for network
> > side if it doesn't add much value and causes trouble?
>
> Yes, correct, we already run checkpatch --strict on all patches.
>
> If you have the ability to selectively disable checkpatch for net/ and
> drivers/net, and/or patches which CC netdev@vger, that'd be great!
Got it, thanks for the detail info, we will pause the reports for these
places. Before that, i will also pause the overall checkpatch check until
we have resolved this for networking side.
>
>
> FWIW we have a simple dashboard reporting which checks in our own
> local build fail the most: https://netdev.bots.linux.dev/checks.html
> Not sure if it's of any interest to you, but that's where I got the
> false positive rate I mentioned previously.
This is very advanced and clear! Thanks for sharing, let me dig into
it to learn from the dashboard.
>
> > > And the maintainer is not very receptive to improvements for false
> > > positives:
> > > https://lore.kernel.org/all/20231013172739.1113964-1-kuba@kernel.org/
> >
> > I see. We got this pattern as well, what we do now is to maintain the pattern
> > internally to avoid unnecessary reports (some are extracted below). I'm looking
> > for publishing these patterns later, which may get more inputs to filter out
> > unnecessary reports.
> >
> > == part of low confidence patterns of checkpatch in bot ==
>
> Interesting!
>
> > __func__ should be used instead of gcc specific __FUNCTION__
>
> This one I don't see failing often.
>
> > line over 80 characters
>
> This one happens a lot, yes.
>
> > LINUX_VERSION_CODE should be avoided, code should be for the version to which it is merged
>
> This is very rare upstream.
>
> > Missing commit description - Add an appropriate one
>
> Should be rare upstream..
>
> > please write a help paragraph that fully describes the config symbol
>
> This check I think is semi-broken in checkpatch.
> Sometimes it just doesn't recognize the help even if symbol has it.
> So yes, we see if false-positive as well.
>
> > Possible repeated word: 'Google'
>
> Yes! :)
>
> > Possible unwrapped commit description \(prefer a maximum 75 chars per line\)
>
> This one indeed has a lot of false positives. It should check if
> *majority* of the commit message lines (excluding tags) are too long,
> not any single line. Because one line can be a crash dump or a commit
> reference, and be longer for legit reasons..
Thanks for all above comments/analysis/experience, which brings a lot insights.
>
> Every now and then I feel like we should fork checkpatch or start a new
> tool which would report only high-confidence problems.
:)
>
> > > > But as you mentioned above, we will take furture care to the output
> > > > of checkpatch to be conservative for the reporting.
> > >
> > > FWIW the most issues that "get through" in networking are issues
> > > in documentation (warnings for make htmldocs) :(
> >
> > Do you suggest that warnings for make htmldocs or kernel-doc warning when building
> > with W=1 can be ignored and no need to send them to networking side?
>
> No, no, the opposite! Documentation is one part we currently don't test,
> even tho we should.
>
> Do you run make htmldocs as part of kernel build bot? As you allude to -
yes, the bot runs make htmldocs check as part of various checks such as
includecheck, dtcheck, etc.
We will continue doing this for networking side.
> W=1 checks kdoc already, and scripts/kernel-doc can be used to validate
> headers even more easily. But to validate the ReST files under
> Documentation/ one has to actually run make htmldocs (or perhaps some
> other docs target), not just a normal build.
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [net-next PATCH v5 02/10] net: Add queue and napi association
2023-10-19 0:06 [net-next PATCH v5 00/10] Introduce queue and NAPI support in netdev-genl (Was: Introduce NAPI queues support) Amritha Nambiar
2023-10-19 0:06 ` [net-next PATCH v5 01/10] netdev-genl: spec: Extend netdev netlink spec in YAML for queue Amritha Nambiar
@ 2023-10-19 0:06 ` Amritha Nambiar
2023-10-19 0:06 ` [net-next PATCH v5 03/10] ice: Add support in the driver for associating queue with napi Amritha Nambiar
` (7 subsequent siblings)
9 siblings, 0 replies; 24+ messages in thread
From: Amritha Nambiar @ 2023-10-19 0:06 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 1c7681263d30..875933f86f41 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -642,6 +642,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
*/
@@ -2612,6 +2616,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 97e7b9833db9..e8add4fcf53f 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6403,6 +6403,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] 24+ messages in thread* [net-next PATCH v5 03/10] ice: Add support in the driver for associating queue with napi
2023-10-19 0:06 [net-next PATCH v5 00/10] Introduce queue and NAPI support in netdev-genl (Was: Introduce NAPI queues support) Amritha Nambiar
2023-10-19 0:06 ` [net-next PATCH v5 01/10] netdev-genl: spec: Extend netdev netlink spec in YAML for queue Amritha Nambiar
2023-10-19 0:06 ` [net-next PATCH v5 02/10] net: Add queue and napi association Amritha Nambiar
@ 2023-10-19 0:06 ` Amritha Nambiar
2023-10-19 5:18 ` kernel test robot
2023-10-19 0:06 ` [net-next PATCH v5 04/10] netdev-genl: Add netlink framework functions for queue Amritha Nambiar
` (6 subsequent siblings)
9 siblings, 1 reply; 24+ messages in thread
From: Amritha Nambiar @ 2023-10-19 0:06 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 | 61 +++++++++++++++++++++++++++++
drivers/net/ethernet/intel/ice/ice_lib.h | 4 ++
drivers/net/ethernet/intel/ice/ice_main.c | 4 +-
3 files changed, 68 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
index 1f45f0c3963d..97ca8f9f77a2 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_lib.c
@@ -2448,6 +2448,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)) {
@@ -2927,6 +2931,63 @@ 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
+ */
+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 0dd7f23395b0..ae40550ba35c 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -3374,9 +3374,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] 24+ messages in thread* Re: [net-next PATCH v5 03/10] ice: Add support in the driver for associating queue with napi
2023-10-19 0:06 ` [net-next PATCH v5 03/10] ice: Add support in the driver for associating queue with napi Amritha Nambiar
@ 2023-10-19 5:18 ` kernel test robot
0 siblings, 0 replies; 24+ messages in thread
From: kernel test robot @ 2023-10-19 5:18 UTC (permalink / raw)
To: Amritha Nambiar, netdev, kuba, pabeni
Cc: oe-kbuild-all, sridhar.samudrala, amritha.nambiar
Hi Amritha,
kernel test robot noticed the following build warnings:
[auto build test WARNING on net-next/main]
url: https://github.com/intel-lab-lkp/linux/commits/Amritha-Nambiar/netdev-genl-spec-Extend-netdev-netlink-spec-in-YAML-for-queue/20231019-082941
base: net-next/main
patch link: https://lore.kernel.org/r/169767397753.6692.15797121214738496388.stgit%40anambiarhost.jf.intel.com
patch subject: [net-next PATCH v5 03/10] ice: Add support in the driver for associating queue with napi
config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20231019/202310191339.JfORBqdK-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231019/202310191339.JfORBqdK-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310191339.JfORBqdK-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/net/ethernet/intel/ice/ice_lib.c:2943:6: warning: no previous prototype for 'ice_queue_set_napi' [-Wmissing-prototypes]
2943 | void ice_queue_set_napi(unsigned int queue_index, enum netdev_queue_type type,
| ^~~~~~~~~~~~~~~~~~
vim +/ice_queue_set_napi +2943 drivers/net/ethernet/intel/ice/ice_lib.c
2933
2934 /**
2935 * ice_queue_set_napi - Set the napi instance for the queue
2936 * @queue_index: Index of queue
2937 * @type: queue type as RX or TX
2938 * @napi: NAPI context
2939 * @locked: is the rtnl_lock already held
2940 *
2941 * Set the napi instance for the queue
2942 */
> 2943 void ice_queue_set_napi(unsigned int queue_index, enum netdev_queue_type type,
2944 struct napi_struct *napi, bool locked)
2945 {
2946 if (locked)
2947 __netif_queue_set_napi(queue_index, type, napi);
2948 else
2949 netif_queue_set_napi(queue_index, type, napi);
2950 }
2951
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 24+ messages in thread
* [net-next PATCH v5 04/10] netdev-genl: Add netlink framework functions for queue
2023-10-19 0:06 [net-next PATCH v5 00/10] Introduce queue and NAPI support in netdev-genl (Was: Introduce NAPI queues support) Amritha Nambiar
` (2 preceding siblings ...)
2023-10-19 0:06 ` [net-next PATCH v5 03/10] ice: Add support in the driver for associating queue with napi Amritha Nambiar
@ 2023-10-19 0:06 ` Amritha Nambiar
2023-10-19 0:06 ` [net-next PATCH v5 05/10] netdev-genl: spec: Extend netdev netlink spec in YAML for NAPI Amritha Nambiar
` (5 subsequent siblings)
9 siblings, 0 replies; 24+ messages in thread
From: Amritha Nambiar @ 2023-10-19 0:06 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..a3e308bf74d5 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_QUEUE_ID, q_idx) ||
+ nla_put_u32(rsp, NETDEV_A_QUEUE_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_QUEUE_ID) ||
+ GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_QUEUE_TYPE) ||
+ GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_IFINDEX))
+ return -EINVAL;
+
+ q_id = nla_get_u32(info->attrs[NETDEV_A_QUEUE_QUEUE_ID]);
+ q_type = nla_get_u32(info->attrs[NETDEV_A_QUEUE_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] 24+ messages in thread* [net-next PATCH v5 05/10] netdev-genl: spec: Extend netdev netlink spec in YAML for NAPI
2023-10-19 0:06 [net-next PATCH v5 00/10] Introduce queue and NAPI support in netdev-genl (Was: Introduce NAPI queues support) Amritha Nambiar
` (3 preceding siblings ...)
2023-10-19 0:06 ` [net-next PATCH v5 04/10] netdev-genl: Add netlink framework functions for queue Amritha Nambiar
@ 2023-10-19 0:06 ` Amritha Nambiar
2023-10-19 2:12 ` kernel test robot
2023-10-19 0:06 ` [net-next PATCH v5 06/10] netdev-genl: Add netlink framework functions for napi Amritha Nambiar
` (4 subsequent siblings)
9 siblings, 1 reply; 24+ messages in thread
From: Amritha Nambiar @ 2023-10-19 0:06 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 56d45995edfa..b6c99189a8de 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: netdev ifindex
+ type: u32
+ checks:
+ min: 1
+ -
+ name: napi-id
+ doc: napi id
+ type: u32
-
name: queue
attributes:
@@ -168,6 +181,23 @@ operations:
attributes:
- ifindex
reply: *queue-get-op
+ -
+ name: napi-get
+ doc: napi information such as napi-id
+ attribute-set: napi
+ do:
+ request:
+ attributes:
+ - napi-id
+ reply: &napi-get-op
+ attributes:
+ - napi-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 a3997efb6786..02e6594d0666 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_NAPI_ID,
+
+ __NETDEV_A_NAPI_MAX,
+ NETDEV_A_NAPI_MAX = (__NETDEV_A_NAPI_MAX - 1)
+};
+
enum {
NETDEV_A_QUEUE_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 85d556a051db..77a340f6c285 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_NAPI_ID + 1] = {
+ [NETDEV_A_NAPI_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_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 a3e308bf74d5..27d7f9dff228 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 a3997efb6786..02e6594d0666 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_NAPI_ID,
+
+ __NETDEV_A_NAPI_MAX,
+ NETDEV_A_NAPI_MAX = (__NETDEV_A_NAPI_MAX - 1)
+};
+
enum {
NETDEV_A_QUEUE_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 9d6f96d80ba1..32d8972f1569 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_NAPI_ID] = { .name = "napi-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_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_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.napi_id)
+ mnl_attr_put_u32(nlh, NETDEV_A_NAPI_NAPI_ID, req->napi_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 bb01368a0595..88ea6b3d76d4 100644
--- a/tools/net/ynl/generated/netdev-user.h
+++ b/tools/net/ynl/generated/netdev-user.h
@@ -187,4 +187,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 napi_id:1;
+ } _present;
+
+ __u32 napi_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_napi_id(struct netdev_napi_get_req *req, __u32 napi_id)
+{
+ req->_present.napi_id = 1;
+ req->napi_id = napi_id;
+}
+
+struct netdev_napi_get_rsp {
+ struct {
+ __u32 napi_id:1;
+ __u32 ifindex:1;
+ } _present;
+
+ __u32 napi_id;
+ __u32 ifindex;
+};
+
+void netdev_napi_get_rsp_free(struct netdev_napi_get_rsp *rsp);
+
+/*
+ * napi information such as napi-id
+ */
+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] 24+ messages in thread* [net-next PATCH v5 06/10] netdev-genl: Add netlink framework functions for napi
2023-10-19 0:06 [net-next PATCH v5 00/10] Introduce queue and NAPI support in netdev-genl (Was: Introduce NAPI queues support) Amritha Nambiar
` (4 preceding siblings ...)
2023-10-19 0:06 ` [net-next PATCH v5 05/10] netdev-genl: spec: Extend netdev netlink spec in YAML for NAPI Amritha Nambiar
@ 2023-10-19 0:06 ` Amritha Nambiar
2023-10-19 0:06 ` [net-next PATCH v5 07/10] netdev-genl: spec: Add irq in netdev netlink YAML spec Amritha Nambiar
` (3 subsequent siblings)
9 siblings, 0 replies; 24+ messages in thread
From: Amritha Nambiar @ 2023-10-19 0:06 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 e8add4fcf53f..1b0f40921efc 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
@@ -6142,7 +6141,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 e075e198092c..20ca1e74a60c 100644
--- a/net/core/dev.h
+++ b/net/core/dev.h
@@ -136,4 +136,6 @@ static inline void netif_set_gro_ipv4_max_size(struct net_device *dev,
}
int rps_cpumask_housekeeping(struct cpumask *mask);
+
+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 27d7f9dff228..d6ef7fc093d0 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_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_NAPI_ID))
+ return -EINVAL;
+
+ napi_id = nla_get_u32(info->attrs[NETDEV_A_NAPI_NAPI_ID]);
+
+ rsp = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
+ if (!rsp)
+ return -ENOMEM;
+
+ rcu_read_lock();
+
+ napi = napi_by_id(napi_id);
+ if (napi)
+ err = netdev_nl_napi_fill_one(rsp, napi, info);
+ else
+ err = -EINVAL;
+
+ rcu_read_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] 24+ messages in thread* [net-next PATCH v5 07/10] netdev-genl: spec: Add irq in netdev netlink YAML spec
2023-10-19 0:06 [net-next PATCH v5 00/10] Introduce queue and NAPI support in netdev-genl (Was: Introduce NAPI queues support) Amritha Nambiar
` (5 preceding siblings ...)
2023-10-19 0:06 ` [net-next PATCH v5 06/10] netdev-genl: Add netlink framework functions for napi Amritha Nambiar
@ 2023-10-19 0:06 ` Amritha Nambiar
2023-10-19 0:06 ` [net-next PATCH v5 08/10] net: Add NAPI IRQ support Amritha Nambiar
` (2 subsequent siblings)
9 siblings, 0 replies; 24+ messages in thread
From: Amritha Nambiar @ 2023-10-19 0:06 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 b6c99189a8de..2faa69d5752f 100644
--- a/Documentation/netlink/specs/netdev.yaml
+++ b/Documentation/netlink/specs/netdev.yaml
@@ -104,6 +104,10 @@ attribute-sets:
name: napi-id
doc: napi id
type: u32
+ -
+ name: irq
+ doc: The associated interrupt vector number for the napi
+ type: u32
-
name: queue
attributes:
@@ -193,6 +197,7 @@ operations:
attributes:
- napi-id
- ifindex
+ - irq
dump:
request:
attributes:
diff --git a/include/uapi/linux/netdev.h b/include/uapi/linux/netdev.h
index 02e6594d0666..d461915aa514 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_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 02e6594d0666..d461915aa514 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_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 32d8972f1569..a3060531b42e 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_NAPI_ID] = { .name = "napi-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 88ea6b3d76d4..1f8251e8ed41 100644
--- a/tools/net/ynl/generated/netdev-user.h
+++ b/tools/net/ynl/generated/netdev-user.h
@@ -214,10 +214,12 @@ struct netdev_napi_get_rsp {
struct {
__u32 napi_id:1;
__u32 ifindex:1;
+ __u32 irq:1;
} _present;
__u32 napi_id;
__u32 ifindex;
+ __u32 irq;
};
void netdev_napi_get_rsp_free(struct netdev_napi_get_rsp *rsp);
^ permalink raw reply related [flat|nested] 24+ messages in thread* [net-next PATCH v5 08/10] net: Add NAPI IRQ support
2023-10-19 0:06 [net-next PATCH v5 00/10] Introduce queue and NAPI support in netdev-genl (Was: Introduce NAPI queues support) Amritha Nambiar
` (6 preceding siblings ...)
2023-10-19 0:06 ` [net-next PATCH v5 07/10] netdev-genl: spec: Add irq in netdev netlink YAML spec Amritha Nambiar
@ 2023-10-19 0:06 ` Amritha Nambiar
2023-10-19 0:06 ` [net-next PATCH v5 09/10] netdev-genl: spec: Add PID in netdev netlink YAML spec Amritha Nambiar
2023-10-19 0:06 ` [net-next PATCH v5 10/10] netdev-genl: Add PID for the NAPI thread Amritha Nambiar
9 siblings, 0 replies; 24+ messages in thread
From: Amritha Nambiar @ 2023-10-19 0:06 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 97ca8f9f77a2..752d1616c894 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 875933f86f41..4adf5d6e3558 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 {
@@ -2623,6 +2624,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 1b0f40921efc..242a7cde577b 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6482,6 +6482,7 @@ void netif_napi_add_weight(struct net_device *dev, struct napi_struct *napi,
*/
if (dev->threaded && napi_kthread_create(napi))
dev->threaded = 0;
+ napi->irq = -1;
}
EXPORT_SYMBOL(netif_napi_add_weight);
diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
index d6ef7fc093d0..ad4b1ee0a2d1 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] 24+ messages in thread* [net-next PATCH v5 09/10] netdev-genl: spec: Add PID in netdev netlink YAML spec
2023-10-19 0:06 [net-next PATCH v5 00/10] Introduce queue and NAPI support in netdev-genl (Was: Introduce NAPI queues support) Amritha Nambiar
` (7 preceding siblings ...)
2023-10-19 0:06 ` [net-next PATCH v5 08/10] net: Add NAPI IRQ support Amritha Nambiar
@ 2023-10-19 0:06 ` Amritha Nambiar
2023-10-19 0:06 ` [net-next PATCH v5 10/10] netdev-genl: Add PID for the NAPI thread Amritha Nambiar
9 siblings, 0 replies; 24+ messages in thread
From: Amritha Nambiar @ 2023-10-19 0:06 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 | 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 2faa69d5752f..9ddb1d0b37dc 100644
--- a/Documentation/netlink/specs/netdev.yaml
+++ b/Documentation/netlink/specs/netdev.yaml
@@ -108,6 +108,10 @@ attribute-sets:
name: irq
doc: The associated interrupt vector number for the napi
type: u32
+ -
+ name: pid
+ doc: PID of the napi thread
+ type: s32
-
name: queue
attributes:
@@ -198,6 +202,7 @@ operations:
- napi-id
- ifindex
- irq
+ - pid
dump:
request:
attributes:
diff --git a/include/uapi/linux/netdev.h b/include/uapi/linux/netdev.h
index d461915aa514..a13fb55b2dde 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_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 d461915aa514..a13fb55b2dde 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_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 a3060531b42e..ca9a68f6bd72 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_NAPI_ID] = { .name = "napi-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 1f8251e8ed41..9aa35f66daad 100644
--- a/tools/net/ynl/generated/netdev-user.h
+++ b/tools/net/ynl/generated/netdev-user.h
@@ -215,11 +215,13 @@ struct netdev_napi_get_rsp {
__u32 napi_id:1;
__u32 ifindex:1;
__u32 irq:1;
+ __u32 pid:1;
} _present;
__u32 napi_id;
__u32 ifindex;
__u32 irq;
+ __s32 pid;
};
void netdev_napi_get_rsp_free(struct netdev_napi_get_rsp *rsp);
^ permalink raw reply related [flat|nested] 24+ messages in thread* [net-next PATCH v5 10/10] netdev-genl: Add PID for the NAPI thread
2023-10-19 0:06 [net-next PATCH v5 00/10] Introduce queue and NAPI support in netdev-genl (Was: Introduce NAPI queues support) Amritha Nambiar
` (8 preceding siblings ...)
2023-10-19 0:06 ` [net-next PATCH v5 09/10] netdev-genl: spec: Add PID in netdev netlink YAML spec Amritha Nambiar
@ 2023-10-19 0:06 ` Amritha Nambiar
9 siblings, 0 replies; 24+ messages in thread
From: Amritha Nambiar @ 2023-10-19 0:06 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 ad4b1ee0a2d1..e05fbdac2a58 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_s32(rsp, NETDEV_A_NAPI_PID, pid))
+ goto nla_put_failure;
+ }
+
genlmsg_end(rsp, hdr);
return 0;
^ permalink raw reply related [flat|nested] 24+ messages in thread