[PATCH net-next v1] bnxt_en: add debugfs file for restarting rx queues

[PATCH net-next v1] bnxt_en: add debugfs file for restarting rx queues

[David Wei]

Add a debugfs file that resets an Rx queue using
netdev_rx_queue_restart(). Useful for testing and debugging.

Signed-off-by: David Wei <dw@davidwei.uk>
---
 .../net/ethernet/broadcom/bnxt/bnxt_debugfs.c | 45 +++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_debugfs.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_debugfs.c
index 127b7015f676..e62a3ff2ffdd 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_debugfs.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_debugfs.c
@@ -10,6 +10,7 @@
 #include <linux/debugfs.h>
 #include <linux/module.h>
 #include <linux/pci.h>
+#include <net/netdev_rx_queue.h>
 #include "bnxt_hsi.h"
 #include <linux/dim.h>
 #include "bnxt.h"
@@ -61,6 +62,48 @@ static const struct file_operations debugfs_dim_fops = {
 	.read = debugfs_dim_read,
 };
 
+static ssize_t debugfs_reset_rx_write(struct file *filep,
+				      const char __user *buffer,
+				      size_t count, loff_t *ppos)
+{
+	unsigned int ring_nr;
+	struct bnxt *bp;
+	char buf[10];
+	ssize_t ret;
+	int rc;
+
+	if (*ppos != 0)
+		return 0;
+
+	if (count >= sizeof(buf))
+		return -ENOSPC;
+
+	ret = copy_from_user(buf, buffer, count);
+	if (ret)
+		return -EFAULT;
+	buf[count] = '\0';
+
+	sscanf(buf, "%u", &ring_nr);
+
+	bp = filep->private_data;
+	if (ring_nr > bp->rx_nr_rings)
+		return -EINVAL;
+
+	netdev_lock(bp->dev);
+	rc = netdev_rx_queue_restart(bp->dev, ring_nr);
+	netdev_unlock(bp->dev);
+	if (rc)
+		return rc;
+
+	return count;
+}
+
+static const struct file_operations debugfs_reset_rx_fops = {
+	.owner = THIS_MODULE,
+	.open = simple_open,
+	.write = debugfs_reset_rx_write,
+};
+
 static void debugfs_dim_ring_init(struct dim *dim, int ring_idx,
 				  struct dentry *dd)
 {
@@ -86,6 +129,8 @@ void bnxt_debug_dev_init(struct bnxt *bp)
 		if (cpr && bp->bnapi[i]->rx_ring)
 			debugfs_dim_ring_init(&cpr->dim, i, dir);
 	}
+
+	debugfs_create_file("reset_rx", 0600, bp->debugfs_pdev, bp, &debugfs_reset_rx_fops);
 }
 
 void bnxt_debug_dev_exit(struct bnxt *bp)
-- 
2.47.1

Re: [PATCH net-next v1] bnxt_en: add debugfs file for restarting rx queues

[Joe Damato]

On Mon, Apr 28, 2025 at 05:06:27PM -0700, David Wei wrote:
> Add a debugfs file that resets an Rx queue using
> netdev_rx_queue_restart(). Useful for testing and debugging.
> 
> Signed-off-by: David Wei <dw@davidwei.uk>
> ---
>  .../net/ethernet/broadcom/bnxt/bnxt_debugfs.c | 45 +++++++++++++++++++
>  1 file changed, 45 insertions(+)
> 
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_debugfs.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_debugfs.c
> index 127b7015f676..e62a3ff2ffdd 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_debugfs.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_debugfs.c

[...]

> +	sscanf(buf, "%u", &ring_nr);

Does sscanf's return value need to be checked to ensure that a match
occurred?

  ret = sscanf(...)
  if (ret != 1)
    return -EINVAL;

or something similar like that?

Re: [PATCH net-next v1] bnxt_en: add debugfs file for restarting rx queues

[David Wei]

On 4/29/25 08:28, Joe Damato wrote:
> On Mon, Apr 28, 2025 at 05:06:27PM -0700, David Wei wrote:
>> Add a debugfs file that resets an Rx queue using
>> netdev_rx_queue_restart(). Useful for testing and debugging.
>>
>> Signed-off-by: David Wei <dw@davidwei.uk>
>> ---
>>   .../net/ethernet/broadcom/bnxt/bnxt_debugfs.c | 45 +++++++++++++++++++
>>   1 file changed, 45 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_debugfs.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_debugfs.c
>> index 127b7015f676..e62a3ff2ffdd 100644
>> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_debugfs.c
>> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_debugfs.c
> 
> [...]
> 
>> +	sscanf(buf, "%u", &ring_nr);
> 
> Does sscanf's return value need to be checked to ensure that a match
> occurred?
> 
>    ret = sscanf(...)
>    if (ret != 1)
>      return -EINVAL;
> 
> or something similar like that?

Yes, that's a good idea, didn't know sscanf() returned a value. I will
add it in v2.

Re: [PATCH net-next v1] bnxt_en: add debugfs file for restarting rx queues

[Jakub Kicinski]

On Mon, 28 Apr 2025 17:06:27 -0700 David Wei wrote:
> Add a debugfs file that resets an Rx queue using
> netdev_rx_queue_restart(). Useful for testing and debugging.

I think we need stronger justification to add reset triggers
in debugfs. If you just need this for development please keep
it in your local tree?

Re: [PATCH net-next v1] bnxt_en: add debugfs file for restarting rx queues

[David Wei]

On 4/29/25 15:18, Jakub Kicinski wrote:
> On Mon, 28 Apr 2025 17:06:27 -0700 David Wei wrote:
>> Add a debugfs file that resets an Rx queue using
>> netdev_rx_queue_restart(). Useful for testing and debugging.
> 
> I think we need stronger justification to add reset triggers
> in debugfs. If you just need this for development please keep
> it in your local tree?

Okay, I thought it would be useful and didn't realise it needed strong
justification. Happy to drop and keep it locally.