netdev - Re: [PATCH net-next v2 7/8] netdevsim: add debugfs-triggered queue reset

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHS8izNM-fMV-FZabhJGq6aAdExVGyT3GwUm3rfhnoFpKULiKQ@mail.gmail.com>
Date: Tue, 7 Jan 2025 15:00:29 -0800
From: Mina Almasry <almasrymina@...gle.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: davem@...emloft.net, netdev@...r.kernel.org, edumazet@...gle.com, 
	pabeni@...hat.com, andrew+netdev@...n.ch, horms@...nel.org, 
	willemdebruijn.kernel@...il.com, sdf@...ichev.me, 
	Willem de Bruijn <willemb@...gle.com>
Subject: Re: [PATCH net-next v2 7/8] netdevsim: add debugfs-triggered queue reset

On Tue, Jan 7, 2025 at 8:11 AM Jakub Kicinski <kuba@...nel.org> wrote:
>
> Support triggering queue reset via debugfs for an upcoming test.
>
> Reviewed-by: Willem de Bruijn <willemb@...gle.com>
> Acked-by: Stanislav Fomichev <sdf@...ichev.me>
> Signed-off-by: Jakub Kicinski <kuba@...nel.org>

Reviewed-by: Mina Almasry <almasrymina@...gle.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?

-- 
Thanks,
Mina