| Message ID | 20241001235302.57609-7-jdamato@fastly.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Add support for per-NAPI config via netlink | expand |
On Tue, Oct 01, 2024 at 11:52:37PM +0000, Joe Damato wrote: > Add support to set per-NAPI defer_hard_irqs and gro_flush_timeout. > > Signed-off-by: Joe Damato <jdamato@fastly.com> > --- > Documentation/netlink/specs/netdev.yaml | 11 ++++++ > include/uapi/linux/netdev.h | 1 + > net/core/netdev-genl-gen.c | 14 ++++++++ > net/core/netdev-genl-gen.h | 1 + > net/core/netdev-genl.c | 45 +++++++++++++++++++++++++ > tools/include/uapi/linux/netdev.h | 1 + > 6 files changed, 73 insertions(+) [...] > diff --git a/net/core/netdev-genl-gen.c b/net/core/netdev-genl-gen.c > index b28424ae06d5..901c6f65b735 100644 > --- a/net/core/netdev-genl-gen.c > +++ b/net/core/netdev-genl-gen.c > @@ -87,6 +87,13 @@ static const struct nla_policy netdev_bind_rx_nl_policy[NETDEV_A_DMABUF_FD + 1] > [NETDEV_A_DMABUF_QUEUES] = NLA_POLICY_NESTED(netdev_queue_id_nl_policy), > }; > > +/* NETDEV_CMD_NAPI_SET - set */ > +static const struct nla_policy netdev_napi_set_nl_policy[NETDEV_A_NAPI_GRO_FLUSH_TIMEOUT + 1] = { > + [NETDEV_A_NAPI_ID] = { .type = NLA_U32, }, > + [NETDEV_A_NAPI_DEFER_HARD_IRQS] = { .type = NLA_S32 }, Noticed this while re-reading the code; planning on changing this from NLA_S32 to NLA_U32 for v5.
On Tue, 8 Oct 2024 11:20:47 -0700 Joe Damato wrote: > Noticed this while re-reading the code; planning on changing this > from NLA_S32 to NLA_U32 for v5. Make sure you edit the spec, not the output. Looks like there may be a problem here (napi-id vs id in the attributes). Make sure you run: ./tools/net/ynl/ynl-regen.sh -f and the tree is clean afterwards
On Tue, Oct 08, 2024 at 03:19:34PM -0700, Jakub Kicinski wrote: > On Tue, 8 Oct 2024 11:20:47 -0700 Joe Damato wrote: > > Noticed this while re-reading the code; planning on changing this > > from NLA_S32 to NLA_U32 for v5. > > Make sure you edit the spec, not the output. Looks like there may be > a problem here (napi-id vs id in the attributes). I'm not sure I follow this part, sorry if I'm just missing something here. I was referring to NETDEV_A_NAPI_DEFER_HARD_IRQS which in RFCv4 is listed as NLA_S32 (in this patch): static const struct nla_policy netdev_napi_set_nl_policy[NETDEV_A_NAPI_GRO_FLUSH_TIMEOUT + 1] = { [NETDEV_A_NAPI_ID] = { .type = NLA_U32, }, [NETDEV_A_NAPI_DEFER_HARD_IRQS] = { .type = NLA_S32 }, However, in the yaml spec (patch 2/9): + - + name: defer-hard-irqs + doc: The number of consecutive empty polls before IRQ deferral ends + and hardware IRQs are re-enabled. + type: u32 + checks: + max: s32-max So the type is u32 but with a "checks" to match what happens now in sysfs. That's why I mentioned changing NLA_S32 to NLA_U32. Am I missing something? Not sure what you meant by "napi-id vs id" ? > Make sure you run: ./tools/net/ynl/ynl-regen.sh -f > and the tree is clean afterwards OK, will do.
On Tue, 8 Oct 2024 16:00:41 -0700 Joe Damato wrote: > > Make sure you edit the spec, not the output. Looks like there may be > > a problem here (napi-id vs id in the attributes). > > I'm not sure I follow this part, sorry if I'm just missing something > here. > > I was referring to NETDEV_A_NAPI_DEFER_HARD_IRQS which in RFCv4 is > listed as NLA_S32 (in this patch): > > static const struct nla_policy netdev_napi_set_nl_policy[NETDEV_A_NAPI_GRO_FLUSH_TIMEOUT + 1] = { > [NETDEV_A_NAPI_ID] = { .type = NLA_U32, }, > [NETDEV_A_NAPI_DEFER_HARD_IRQS] = { .type = NLA_S32 }, > > However, in the yaml spec (patch 2/9): > > + - > + name: defer-hard-irqs > + doc: The number of consecutive empty polls before IRQ deferral ends > + and hardware IRQs are re-enabled. > + type: u32 > + checks: > + max: s32-max > > So the type is u32 but with a "checks" to match what happens now in > sysfs. > > That's why I mentioned changing NLA_S32 to NLA_U32. > > Am I missing something? YNL will generate the correct code for your - the right type and the right range validation. Run the command below to see. > Not sure what you meant by "napi-id vs id" ? I can't apply the series now, but when it was posted the YNL code generation failed here complaining about napi-id not existing in the attribute set in which it is used. In the napi attribute set the NAPI ID is called just "id", not "napi-id". > > Make sure you run: ./tools/net/ynl/ynl-regen.sh -f > > and the tree is clean afterwards > > OK, will do.
On Tue, Oct 08, 2024 at 04:19:19PM -0700, Jakub Kicinski wrote: > On Tue, 8 Oct 2024 16:00:41 -0700 Joe Damato wrote: > > > Make sure you edit the spec, not the output. Looks like there may be > > > a problem here (napi-id vs id in the attributes). > > > > I'm not sure I follow this part, sorry if I'm just missing something > > here. > > > > I was referring to NETDEV_A_NAPI_DEFER_HARD_IRQS which in RFCv4 is > > listed as NLA_S32 (in this patch): > > > > static const struct nla_policy netdev_napi_set_nl_policy[NETDEV_A_NAPI_GRO_FLUSH_TIMEOUT + 1] = { > > [NETDEV_A_NAPI_ID] = { .type = NLA_U32, }, > > [NETDEV_A_NAPI_DEFER_HARD_IRQS] = { .type = NLA_S32 }, > > > > However, in the yaml spec (patch 2/9): > > > > + - > > + name: defer-hard-irqs > > + doc: The number of consecutive empty polls before IRQ deferral ends > > + and hardware IRQs are re-enabled. > > + type: u32 > > + checks: > > + max: s32-max > > > > So the type is u32 but with a "checks" to match what happens now in > > sysfs. > > > > That's why I mentioned changing NLA_S32 to NLA_U32. > > > > Am I missing something? > > YNL will generate the correct code for your - the right type > and the right range validation. Run the command below to see. > > > Not sure what you meant by "napi-id vs id" ? > > I can't apply the series now, but when it was posted the YNL code > generation failed here complaining about napi-id not existing in > the attribute set in which it is used. In the napi attribute set > the NAPI ID is called just "id", not "napi-id". Ah, I see what you mean now. It should have been obvious, but the -gen* files are, uh, auto-generated ;) And yes, I see now that the attribute set names it "id", so I've fixed it and the command runs clean and I'll include the generated output this time in the v5.
diff --git a/Documentation/netlink/specs/netdev.yaml b/Documentation/netlink/specs/netdev.yaml index bf13613eaa0d..7f8d2489c68c 100644 --- a/Documentation/netlink/specs/netdev.yaml +++ b/Documentation/netlink/specs/netdev.yaml @@ -690,6 +690,17 @@ operations: reply: attributes: - id + - + name: napi-set + doc: Set configurable NAPI instance settings. + attribute-set: napi + flags: [ admin-perm ] + do: + request: + attributes: + - napi-id + - defer-hard-irqs + - gro-flush-timeout kernel-family: headers: [ "linux/list.h"] diff --git a/include/uapi/linux/netdev.h b/include/uapi/linux/netdev.h index cacd33359c76..e3ebb49f60d2 100644 --- a/include/uapi/linux/netdev.h +++ b/include/uapi/linux/netdev.h @@ -201,6 +201,7 @@ enum { NETDEV_CMD_NAPI_GET, NETDEV_CMD_QSTATS_GET, NETDEV_CMD_BIND_RX, + NETDEV_CMD_NAPI_SET, __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 b28424ae06d5..901c6f65b735 100644 --- a/net/core/netdev-genl-gen.c +++ b/net/core/netdev-genl-gen.c @@ -87,6 +87,13 @@ static const struct nla_policy netdev_bind_rx_nl_policy[NETDEV_A_DMABUF_FD + 1] [NETDEV_A_DMABUF_QUEUES] = NLA_POLICY_NESTED(netdev_queue_id_nl_policy), }; +/* NETDEV_CMD_NAPI_SET - set */ +static const struct nla_policy netdev_napi_set_nl_policy[NETDEV_A_NAPI_GRO_FLUSH_TIMEOUT + 1] = { + [NETDEV_A_NAPI_ID] = { .type = NLA_U32, }, + [NETDEV_A_NAPI_DEFER_HARD_IRQS] = { .type = NLA_S32 }, + [NETDEV_A_NAPI_GRO_FLUSH_TIMEOUT] = { .type = NLA_UINT }, +}; + /* Ops table for netdev */ static const struct genl_split_ops netdev_nl_ops[] = { { @@ -171,6 +178,13 @@ static const struct genl_split_ops netdev_nl_ops[] = { .maxattr = NETDEV_A_DMABUF_FD, .flags = GENL_ADMIN_PERM | GENL_CMD_CAP_DO, }, + { + .cmd = NETDEV_CMD_NAPI_SET, + .doit = netdev_nl_napi_set_doit, + .policy = netdev_napi_set_nl_policy, + .maxattr = NETDEV_A_NAPI_GRO_FLUSH_TIMEOUT, + .flags = GENL_ADMIN_PERM | GENL_CMD_CAP_DO, + }, }; 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 8cda334fd042..85e6d7c95ada 100644 --- a/net/core/netdev-genl-gen.h +++ b/net/core/netdev-genl-gen.h @@ -30,6 +30,7 @@ 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); +int netdev_nl_napi_set_doit(struct sk_buff *skb, struct genl_info *info); int netdev_nl_qstats_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb); int netdev_nl_bind_rx_doit(struct sk_buff *skb, struct genl_info *info); diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c index 64e5e4cee60d..59523318d620 100644 --- a/net/core/netdev-genl.c +++ b/net/core/netdev-genl.c @@ -303,6 +303,51 @@ int netdev_nl_napi_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb) return err; } +static int +netdev_nl_napi_set_config(struct napi_struct *napi, struct genl_info *info) +{ + u64 gro_flush_timeout = 0; + u32 defer = 0; + + if (info->attrs[NETDEV_A_NAPI_DEFER_HARD_IRQS]) { + defer = nla_get_u32(info->attrs[NETDEV_A_NAPI_DEFER_HARD_IRQS]); + napi_set_defer_hard_irqs(napi, defer); + } + + if (info->attrs[NETDEV_A_NAPI_GRO_FLUSH_TIMEOUT]) { + gro_flush_timeout = nla_get_uint(info->attrs[NETDEV_A_NAPI_GRO_FLUSH_TIMEOUT]); + napi_set_gro_flush_timeout(napi, gro_flush_timeout); + } + + return 0; +} + +int netdev_nl_napi_set_doit(struct sk_buff *skb, struct genl_info *info) +{ + struct napi_struct *napi; + unsigned int 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]); + + rtnl_lock(); + + napi = napi_by_id(napi_id); + if (napi) { + err = netdev_nl_napi_set_config(napi, info); + } else { + NL_SET_BAD_ATTR(info->extack, info->attrs[NETDEV_A_NAPI_ID]); + err = -ENOENT; + } + + rtnl_unlock(); + + return err; +} + 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 cacd33359c76..e3ebb49f60d2 100644 --- a/tools/include/uapi/linux/netdev.h +++ b/tools/include/uapi/linux/netdev.h @@ -201,6 +201,7 @@ enum { NETDEV_CMD_NAPI_GET, NETDEV_CMD_QSTATS_GET, NETDEV_CMD_BIND_RX, + NETDEV_CMD_NAPI_SET, __NETDEV_CMD_MAX, NETDEV_CMD_MAX = (__NETDEV_CMD_MAX - 1)
Add support to set per-NAPI defer_hard_irqs and gro_flush_timeout. Signed-off-by: Joe Damato <jdamato@fastly.com> --- Documentation/netlink/specs/netdev.yaml | 11 ++++++ include/uapi/linux/netdev.h | 1 + net/core/netdev-genl-gen.c | 14 ++++++++ net/core/netdev-genl-gen.h | 1 + net/core/netdev-genl.c | 45 +++++++++++++++++++++++++ tools/include/uapi/linux/netdev.h | 1 + 6 files changed, 73 insertions(+)