| Message ID | 20250107160846.2223263-8-kuba@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: make sure we retain NAPI ordering on netdev->napi_list | expand |
On Tue, Jan 7, 2025 at 8:11 AM Jakub Kicinski <kuba@kernel.org> wrote: > > Support triggering queue reset via debugfs for an upcoming test. > > Reviewed-by: Willem de Bruijn <willemb@google.com> > Acked-by: Stanislav Fomichev <sdf@fomichev.me> > Signed-off-by: Jakub Kicinski <kuba@kernel.org> Reviewed-by: Mina Almasry <almasrymina@google.com> > --- > v2: > - change mode to 0200 > - reorder removal to be inverse of add > - fix the spaces vs tabs > --- > drivers/net/netdevsim/netdev.c | 55 +++++++++++++++++++++++++++++++ > drivers/net/netdevsim/netdevsim.h | 1 + > 2 files changed, 56 insertions(+) > > diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c > index cfb079a34532..d013b6498539 100644 > --- a/drivers/net/netdevsim/netdev.c > +++ b/drivers/net/netdevsim/netdev.c > @@ -20,6 +20,7 @@ > #include <linux/netdevice.h> > #include <linux/slab.h> > #include <net/netdev_queues.h> > +#include <net/netdev_rx_queue.h> > #include <net/page_pool/helpers.h> > #include <net/netlink.h> > #include <net/net_shaper.h> > @@ -29,6 +30,8 @@ > > #include "netdevsim.h" > > +MODULE_IMPORT_NS("NETDEV_INTERNAL"); > + > #define NSIM_RING_SIZE 256 > > static int nsim_napi_rx(struct nsim_rq *rq, struct sk_buff *skb) > @@ -722,6 +725,54 @@ static const struct netdev_queue_mgmt_ops nsim_queue_mgmt_ops = { > .ndo_queue_stop = nsim_queue_stop, > }; > > +static ssize_t > +nsim_qreset_write(struct file *file, const char __user *data, > + size_t count, loff_t *ppos) > +{ > + struct netdevsim *ns = file->private_data; > + unsigned int queue, mode; > + char buf[32]; > + ssize_t ret; > + > + if (count >= sizeof(buf)) > + return -EINVAL; > + if (copy_from_user(buf, data, count)) > + return -EFAULT; > + buf[count] = '\0'; > + > + ret = sscanf(buf, "%u %u", &queue, &mode); > + if (ret != 2) > + return -EINVAL; > + > + rtnl_lock(); > + if (!netif_running(ns->netdev)) { > + ret = -ENETDOWN; > + goto exit_unlock; > + } > + > + if (queue >= ns->netdev->real_num_rx_queues) { > + ret = -EINVAL; > + goto exit_unlock; > + } Is it correct that both these above checks are not inside of netdev_rx_queue_restart() itself? Or should we fix that?
On Tue, 7 Jan 2025 15:00:29 -0800 Mina Almasry wrote: > > + if (queue >= ns->netdev->real_num_rx_queues) { > > + ret = -EINVAL; > > + goto exit_unlock; > > + } > > Is it correct that both these above checks are not inside of > netdev_rx_queue_restart() itself? Or should we fix that? Crossed my mind, too. But I didn't want to start futzing with netdev_rx_queue_restart() based on just two callers. I hope we'll get to a queue API based safe reset sooner rather than later, and the driver needs will dedicate the final shape. More concisely put - a debugfs hook in a test harness is not a strong signal for whether the API is right.
diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c index cfb079a34532..d013b6498539 100644 --- a/drivers/net/netdevsim/netdev.c +++ b/drivers/net/netdevsim/netdev.c @@ -20,6 +20,7 @@ #include <linux/netdevice.h> #include <linux/slab.h> #include <net/netdev_queues.h> +#include <net/netdev_rx_queue.h> #include <net/page_pool/helpers.h> #include <net/netlink.h> #include <net/net_shaper.h> @@ -29,6 +30,8 @@ #include "netdevsim.h" +MODULE_IMPORT_NS("NETDEV_INTERNAL"); + #define NSIM_RING_SIZE 256 static int nsim_napi_rx(struct nsim_rq *rq, struct sk_buff *skb) @@ -722,6 +725,54 @@ static const struct netdev_queue_mgmt_ops nsim_queue_mgmt_ops = { .ndo_queue_stop = nsim_queue_stop, }; +static ssize_t +nsim_qreset_write(struct file *file, const char __user *data, + size_t count, loff_t *ppos) +{ + struct netdevsim *ns = file->private_data; + unsigned int queue, mode; + char buf[32]; + ssize_t ret; + + if (count >= sizeof(buf)) + return -EINVAL; + if (copy_from_user(buf, data, count)) + return -EFAULT; + buf[count] = '\0'; + + ret = sscanf(buf, "%u %u", &queue, &mode); + if (ret != 2) + return -EINVAL; + + rtnl_lock(); + if (!netif_running(ns->netdev)) { + ret = -ENETDOWN; + goto exit_unlock; + } + + if (queue >= ns->netdev->real_num_rx_queues) { + ret = -EINVAL; + goto exit_unlock; + } + + ns->rq_reset_mode = mode; + ret = netdev_rx_queue_restart(ns->netdev, queue); + ns->rq_reset_mode = 0; + if (ret) + goto exit_unlock; + + ret = count; +exit_unlock: + rtnl_unlock(); + return ret; +} + +static const struct file_operations nsim_qreset_fops = { + .open = simple_open, + .write = nsim_qreset_write, + .owner = THIS_MODULE, +}; + static ssize_t nsim_pp_hold_read(struct file *file, char __user *data, size_t count, loff_t *ppos) @@ -934,6 +985,9 @@ nsim_create(struct nsim_dev *nsim_dev, struct nsim_dev_port *nsim_dev_port) ns->pp_dfs = debugfs_create_file("pp_hold", 0600, nsim_dev_port->ddir, ns, &nsim_pp_hold_fops); + ns->qr_dfs = debugfs_create_file("queue_reset", 0200, + nsim_dev_port->ddir, ns, + &nsim_qreset_fops); return ns; @@ -947,6 +1001,7 @@ void nsim_destroy(struct netdevsim *ns) struct net_device *dev = ns->netdev; struct netdevsim *peer; + debugfs_remove(ns->qr_dfs); debugfs_remove(ns->pp_dfs); rtnl_lock(); diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h index 8c50969b1240..a70f62af4c88 100644 --- a/drivers/net/netdevsim/netdevsim.h +++ b/drivers/net/netdevsim/netdevsim.h @@ -136,6 +136,7 @@ struct netdevsim { struct page *page; struct dentry *pp_dfs; + struct dentry *qr_dfs; struct nsim_ethtool ethtool; struct netdevsim __rcu *peer;