| Message ID | 169266033666.10199.3744908214828788701.stgit@anambiarhost.jf.intel.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Introduce NAPI queues support | expand |
On Mon, 21 Aug 2023 16:25:36 -0700 Amritha Nambiar wrote: > 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 > queue[s] can be retrieved this way. > > Signed-off-by: Amritha Nambiar <amritha.nambiar@intel.com> > @@ -119,14 +134,158 @@ 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) > +{ > + struct netdev_rx_queue *rx_queue, *rxq; > + struct netdev_queue *tx_queue, *txq; > + unsigned int rx_qid, tx_qid; > + void *hdr; > + > + if (!napi->dev) > + return -EINVAL; WARN_ON_ONCE()? If this can be assumed not to happen. > + hdr = genlmsg_iput(rsp, info); > + if (!hdr) > + return -EMSGSIZE; > + > + if (nla_put_u32(rsp, NETDEV_A_NAPI_NAPI_ID, napi->napi_id)) napi_id can be zero. > + goto nla_put_failure; > + > + if (nla_put_u32(rsp, NETDEV_A_NAPI_IFINDEX, napi->dev->ifindex)) > + goto nla_put_failure; > int netdev_nl_napi_get_doit(struct sk_buff *skb, struct genl_info *info) > { > - return -EOPNOTSUPP; > + struct net_device *netdev; > + 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; > + > + rtnl_lock(); > + > + netdev = dev_get_by_napi_id(napi_id); Why lookup the dev and not the NAPI? > + if (netdev) > + err = netdev_nl_napi_fill(netdev, rsp, info, napi_id); > + 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_napi_dump_one(struct net_device *netdev, struct sk_buff *rsp, > + const struct genl_info *info, int *start) > +{ > + struct napi_struct *napi, *n; > + int err = 0; > + int i = 0; > + > + list_for_each_entry_safe(napi, n, &netdev->napi_list, dev_list) { Why _safe()? I think you need _rcu() instead? > + if (i < *start) { > + i++; > + continue; > + } > + err = netdev_nl_napi_fill_one(rsp, napi, info); > + if (err) > + break; > + *start = ++i; Why count them instead of relying on the IDs? > int netdev_nl_napi_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb) > { > - return -EOPNOTSUPP; > + const struct genl_dumpit_info *info = genl_dumpit_info(cb); You can get genl_info_dump(cb) here, you don't use the genl_dumpit_info AFAICT, only info->info.
On 8/22/2023 5:51 PM, Jakub Kicinski wrote: > On Mon, 21 Aug 2023 16:25:36 -0700 Amritha Nambiar wrote: >> 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 >> queue[s] can be retrieved this way. >> >> Signed-off-by: Amritha Nambiar <amritha.nambiar@intel.com> > >> @@ -119,14 +134,158 @@ 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) >> +{ >> + struct netdev_rx_queue *rx_queue, *rxq; >> + struct netdev_queue *tx_queue, *txq; >> + unsigned int rx_qid, tx_qid; >> + void *hdr; >> + >> + if (!napi->dev) >> + return -EINVAL; > > WARN_ON_ONCE()? If this can be assumed not to happen. Okay. Will fix in v3. > >> + hdr = genlmsg_iput(rsp, info); >> + if (!hdr) >> + return -EMSGSIZE; >> + >> + if (nla_put_u32(rsp, NETDEV_A_NAPI_NAPI_ID, napi->napi_id)) > > napi_id can be zero. Will fix in v3. > >> + goto nla_put_failure; >> + >> + if (nla_put_u32(rsp, NETDEV_A_NAPI_IFINDEX, napi->dev->ifindex)) >> + goto nla_put_failure; > >> int netdev_nl_napi_get_doit(struct sk_buff *skb, struct genl_info *info) >> { >> - return -EOPNOTSUPP; >> + struct net_device *netdev; >> + 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; >> + >> + rtnl_lock(); >> + >> + netdev = dev_get_by_napi_id(napi_id); > > Why lookup the dev and not the NAPI? Agree. Will fix in v3. > >> + if (netdev) >> + err = netdev_nl_napi_fill(netdev, rsp, info, napi_id); >> + 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_napi_dump_one(struct net_device *netdev, struct sk_buff *rsp, >> + const struct genl_info *info, int *start) >> +{ >> + struct napi_struct *napi, *n; >> + int err = 0; >> + int i = 0; >> + >> + list_for_each_entry_safe(napi, n, &netdev->napi_list, dev_list) { > > Why _safe()? I think you need _rcu() instead? Agree. This is called within rtnl_lock. Will fix. > >> + if (i < *start) { >> + i++; >> + continue; >> + } >> + err = netdev_nl_napi_fill_one(rsp, napi, info); >> + if (err) >> + break; >> + *start = ++i; > > Why count them instead of relying on the IDs? Makes sense. Will fix. > >> int netdev_nl_napi_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb) >> { >> - return -EOPNOTSUPP; >> + const struct genl_dumpit_info *info = genl_dumpit_info(cb); > > You can get genl_info_dump(cb) here, you don't use the genl_dumpit_info > AFAICT, only info->info. Will fix in v3.
diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c index b146679d6112..4bc6aa756001 100644 --- a/net/core/netdev-genl.c +++ b/net/core/netdev-genl.c @@ -5,9 +5,23 @@ #include <linux/rtnetlink.h> #include <net/net_namespace.h> #include <net/sock.h> +#include <net/netdev_rx_queue.h> #include "netdev-genl-gen.h" +struct netdev_nl_dump_ctx { + unsigned long ifindex; + int napi_idx; +}; + +static inline 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) @@ -101,12 +115,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; @@ -119,14 +134,158 @@ 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) +{ + struct netdev_rx_queue *rx_queue, *rxq; + struct netdev_queue *tx_queue, *txq; + unsigned int rx_qid, tx_qid; + void *hdr; + + if (!napi->dev) + return -EINVAL; + + hdr = genlmsg_iput(rsp, info); + if (!hdr) + return -EMSGSIZE; + + if (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; + + list_for_each_entry_safe(rx_queue, rxq, &napi->napi_rxq_list, q_list) { + rx_qid = get_netdev_rx_queue_index(rx_queue); + if (nla_put_u32(rsp, NETDEV_A_NAPI_RX_QUEUES, rx_qid)) + goto nla_put_failure; + } + + list_for_each_entry_safe(tx_queue, txq, &napi->napi_txq_list, q_list) { + tx_qid = get_netdev_queue_index(tx_queue); + if (nla_put_u32(rsp, NETDEV_A_NAPI_TX_QUEUES, tx_qid)) + goto nla_put_failure; + } + + genlmsg_end(rsp, hdr); + return 0; + +nla_put_failure: + genlmsg_cancel(rsp, hdr); + return -EMSGSIZE; +} + +static int +netdev_nl_napi_fill(struct net_device *netdev, struct sk_buff *rsp, + const struct genl_info *info, u32 napi_id) +{ + struct napi_struct *napi, *n; + + list_for_each_entry_safe(napi, n, &netdev->napi_list, dev_list) { + if (napi->napi_id == napi_id) + return netdev_nl_napi_fill_one(rsp, napi, info); + } + return -EINVAL; +} + int netdev_nl_napi_get_doit(struct sk_buff *skb, struct genl_info *info) { - return -EOPNOTSUPP; + struct net_device *netdev; + 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; + + rtnl_lock(); + + netdev = dev_get_by_napi_id(napi_id); + if (netdev) + err = netdev_nl_napi_fill(netdev, rsp, info, napi_id); + 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_napi_dump_one(struct net_device *netdev, struct sk_buff *rsp, + const struct genl_info *info, int *start) +{ + struct napi_struct *napi, *n; + int err = 0; + int i = 0; + + list_for_each_entry_safe(napi, n, &netdev->napi_list, dev_list) { + if (i < *start) { + i++; + continue; + } + err = netdev_nl_napi_fill_one(rsp, napi, info); + if (err) + break; + *start = ++i; + } + return err; } int netdev_nl_napi_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb) { - return -EOPNOTSUPP; + const struct genl_dumpit_info *info = genl_dumpit_info(cb); + struct netdev_nl_dump_ctx *ctx = netdev_dump_ctx(cb); + struct net *net = sock_net(skb->sk); + struct net_device *netdev; + int n_idx = ctx->napi_idx; + u32 ifindex = 0; + int err = 0; + + if (info->info.attrs[NETDEV_A_NAPI_IFINDEX]) + ifindex = nla_get_u32(info->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, + genl_info_dump(cb), + &n_idx); + else + err = -ENODEV; + } else { + for_each_netdev_dump(net, netdev, ctx->ifindex) { + err = netdev_nl_napi_dump_one(netdev, skb, + genl_info_dump(cb), + &n_idx); + if (!err) + n_idx = 0; + if (err < 0) + break; + } + } + rtnl_unlock(); + + if (err != -EMSGSIZE) + return err; + + ctx->napi_idx = n_idx; + return skb->len; } static int netdev_genl_netdevice_event(struct notifier_block *nb,
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 queue[s] can be retrieved this way. Signed-off-by: Amritha Nambiar <amritha.nambiar@intel.com> --- net/core/netdev-genl.c | 165 +++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 162 insertions(+), 3 deletions(-)