On Mon, 28 Jul 2025 14:50:12 -0700 Mina Almasry wrote:

On Mon, Jul 28, 2025 at 4:03 AM Pavel Begunkov [off-list ref] wrote:

quoted

Add rx_buf_len to configuration maintained by the core.
Use "three-state" semantics where 0 means "driver default".

What are three states in the semantics here?

- 0 = driver default.
- non-zero means value set by userspace

What is the 3rd state here?

I just mean a value with an explicit default / unset state.
If you have a better name I'm all ears ..

quoted

diff --git a/net/ethtool/common.c b/net/ethtool/common.c
index a87298f659f5..8fdffc77e981 100644
--- a/net/ethtool/common.c
+++ b/net/ethtool/common.c

@@ -832,6 +832,7 @@ void ethtool_ringparam_get_cfg(struct net_device *dev,

        /* Driver gives us current state, we want to return current config */
        kparam->tcp_data_split = dev->cfg->hds_config;
+       kparam->rx_buf_len = dev->cfg->rx_buf_len;  

I'm confused that struct netdev_config is defined in netdev_queues.h,
and is documented to be a queue-related configuration, but doesn't
seem to be actually per queue? This line is grabbing the current
config for this queue from dev->cfg which looks like a shared value.

I don't think rx_buf_len should be a shared value between all the
queues. I strongly think it should a per-queue value. The
devmem/io_uring queues will probably want large rx_buf_len, but normal
queues will want 0 buf len, me thinks.

I presume that question answered itself as you were reading the rest 
of the patches? :)