Re: [RFC v1 05/22] net: add rx_buf_len to netdev config
From: Mina Almasry <hidden>
Date: 2025-07-28 21:50:25
Also in:
io-uring
On Mon, Jul 28, 2025 at 4:03 AM Pavel Begunkov [off-list ref] wrote:
From: Jakub Kicinski <kuba@kernel.org> 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?
quoted hunk ↗ jump to hunk
Signed-off-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com> --- include/net/netdev_queues.h | 4 ++++ net/ethtool/common.c | 1 + net/ethtool/rings.c | 2 ++ 3 files changed, 7 insertions(+)diff --git a/include/net/netdev_queues.h b/include/net/netdev_queues.h index 81df0794d84c..eb3a5ac823e6 100644 --- a/include/net/netdev_queues.h +++ b/include/net/netdev_queues.h@@ -24,6 +24,10 @@ struct netdev_config { * If "unset" driver is free to decide, and may change its choice * as other parameters change. */ + /** @rx_buf_len: Size of buffers on the Rx ring + * (ETHTOOL_A_RINGS_RX_BUF_LEN). + */ + u32 rx_buf_len; /** @hds_config: HDS enabled (ETHTOOL_A_RINGS_TCP_DATA_SPLIT). */ u8 hds_config;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. -- Thanks, Mina