* [PATCH net-next 0/6] net: restore the structure of driver-facing qcfg API
@ 2026-01-22 0:51 Jakub Kicinski
2026-01-22 0:51 ` [PATCH net-next 1/6] eth: bnxt: always set the queue mgmt ops Jakub Kicinski
` (6 more replies)
0 siblings, 7 replies; 17+ messages in thread
From: Jakub Kicinski @ 2026-01-22 0:51 UTC (permalink / raw)
To: davem
Cc: netdev, edumazet, pabeni, andrew+netdev, horms, michael.chan, sdf,
almasrymina, dtatulea, asml.silence, dw, daniel, Jakub Kicinski
The goal of qcfg objects is to let us seamlessly support new use cases
without modifying all the drivers. We want to pull all the logic of
combining configuration supplied via different interfaces into the core
and present the drivers with a flat queue-by-queue configuration.
Additionally we want to separate the current effective configuration
from the user intent (default vs user setting vs memory provider setting).
Restructure the recently added code to re-introduce the pieces that
are missing compared to the old RFC:
https://lore.kernel.org/20250421222827.283737-1-kuba@kernel.org
Namely:
- the netdev_queue_config() helper
- queue config validation callback
I hopefully removed all the more "out there" parts of the RFC.
Jakub Kicinski (6):
eth: bnxt: always set the queue mgmt ops
net: introduce a trivial netdev_queue_config()
net: move mp->rx_page_size validation to __net_mp_open_rxq()
net: use netdev_queue_config() for mp restart
net: add queue config validation callback
eth: bnxt: plug bnxt_validate_qcfg() into qops
net/core/Makefile | 1 +
include/net/netdev_queues.h | 17 ++++-
net/core/dev.h | 5 ++
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 23 ++++---
net/core/dev.c | 17 -----
net/core/netdev_config.c | 78 +++++++++++++++++++++++
net/core/netdev_rx_queue.c | 60 ++++++++++-------
7 files changed, 152 insertions(+), 49 deletions(-)
create mode 100644 net/core/netdev_config.c
--
2.52.0
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH net-next 1/6] eth: bnxt: always set the queue mgmt ops
2026-01-22 0:51 [PATCH net-next 0/6] net: restore the structure of driver-facing qcfg API Jakub Kicinski
@ 2026-01-22 0:51 ` Jakub Kicinski
2026-01-22 8:04 ` Subbaraya Sundeep
2026-01-22 0:51 ` [PATCH net-next 2/6] net: introduce a trivial netdev_queue_config() Jakub Kicinski
` (5 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Jakub Kicinski @ 2026-01-22 0:51 UTC (permalink / raw)
To: davem
Cc: netdev, edumazet, pabeni, andrew+netdev, horms, michael.chan, sdf,
almasrymina, dtatulea, asml.silence, dw, daniel, Jakub Kicinski
Core provides a centralized callback for validating per-queue settings
but the callback is part of the queue management ops. Having the ops
conditionally set complicates the parts of the driver which could
otherwise lean on the core to feed it the correct settings.
Always set the queue ops, but provide no restart-related callbacks if
queue ops are not supported by the device. This should maintain current
behavior, the check in netdev_rx_queue_restart() looks both at op struct
and individual ops.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
--
v1:
- reflow code
- typo fix
rfc: https://lore.kernel.org/20250421222827.283737-15-kuba@kernel.org
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 0e0c88c122f8..0b95100a7c36 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -16314,6 +16314,9 @@ static const struct netdev_queue_mgmt_ops bnxt_queue_mgmt_ops = {
.supported_params = QCFG_RX_PAGE_SIZE,
};
+static const struct netdev_queue_mgmt_ops bnxt_queue_mgmt_ops_unsupp = {
+};
+
static void bnxt_remove_one(struct pci_dev *pdev)
{
struct net_device *dev = pci_get_drvdata(pdev);
@@ -16966,9 +16969,10 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
if (BNXT_SUPPORTS_NTUPLE_VNIC(bp))
bp->rss_cap |= BNXT_RSS_CAP_MULTI_RSS_CTX;
+
+ dev->queue_mgmt_ops = &bnxt_queue_mgmt_ops_unsupp;
if (BNXT_SUPPORTS_QUEUE_API(bp))
dev->queue_mgmt_ops = &bnxt_queue_mgmt_ops;
- dev->request_ops_lock = true;
dev->netmem_tx = true;
rc = register_netdev(dev);
--
2.52.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH net-next 2/6] net: introduce a trivial netdev_queue_config()
2026-01-22 0:51 [PATCH net-next 0/6] net: restore the structure of driver-facing qcfg API Jakub Kicinski
2026-01-22 0:51 ` [PATCH net-next 1/6] eth: bnxt: always set the queue mgmt ops Jakub Kicinski
@ 2026-01-22 0:51 ` Jakub Kicinski
2026-01-22 8:12 ` Subbaraya Sundeep
2026-01-22 0:51 ` [PATCH net-next 3/6] net: move mp->rx_page_size validation to __net_mp_open_rxq() Jakub Kicinski
` (4 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Jakub Kicinski @ 2026-01-22 0:51 UTC (permalink / raw)
To: davem
Cc: netdev, edumazet, pabeni, andrew+netdev, horms, michael.chan, sdf,
almasrymina, dtatulea, asml.silence, dw, daniel, Jakub Kicinski
We may choose to extend or reimplement the logic which renders
the per-queue config. The drivers should not poke directly into
the queue state. Add a helper for drivers to use when they want
to query the config for a specific queue.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
net/core/Makefile | 1 +
include/net/netdev_queues.h | 3 +++
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 6 ++---
net/core/netdev_config.c | 32 +++++++++++++++++++++++
4 files changed, 39 insertions(+), 3 deletions(-)
create mode 100644 net/core/netdev_config.c
diff --git a/net/core/Makefile b/net/core/Makefile
index 9ef2099c5426..d643a5a7fd18 100644
--- a/net/core/Makefile
+++ b/net/core/Makefile
@@ -19,6 +19,7 @@ obj-$(CONFIG_NETDEV_ADDR_LIST_TEST) += dev_addr_lists_test.o
obj-y += net-sysfs.o
obj-y += hotdata.o
+obj-y += netdev_config.o
obj-y += netdev_rx_queue.o
obj-y += netdev_queues.o
obj-$(CONFIG_PAGE_POOL) += page_pool.o page_pool_user.o
diff --git a/include/net/netdev_queues.h b/include/net/netdev_queues.h
index 2ab3eae8e8c3..725bf69ef86c 100644
--- a/include/net/netdev_queues.h
+++ b/include/net/netdev_queues.h
@@ -170,6 +170,9 @@ struct netdev_queue_mgmt_ops {
unsigned int supported_params;
};
+void netdev_queue_config(struct net_device *dev, int rxq,
+ struct netdev_queue_config *qcfg);
+
bool netif_rxq_has_unreadable_mp(struct net_device *dev, int idx);
/**
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 0b95100a7c36..d57e833ce690 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -4326,12 +4326,12 @@ static void bnxt_init_ring_struct(struct bnxt *bp)
for (i = 0; i < bp->cp_nr_rings; i++) {
struct bnxt_napi *bnapi = bp->bnapi[i];
+ struct netdev_queue_config qcfg;
struct bnxt_ring_mem_info *rmem;
struct bnxt_cp_ring_info *cpr;
struct bnxt_rx_ring_info *rxr;
struct bnxt_tx_ring_info *txr;
struct bnxt_ring_struct *ring;
- struct netdev_rx_queue *rxq;
if (!bnapi)
continue;
@@ -4349,8 +4349,8 @@ static void bnxt_init_ring_struct(struct bnxt *bp)
if (!rxr)
goto skip_rx;
- rxq = __netif_get_rx_queue(bp->dev, i);
- rxr->rx_page_size = rxq->qcfg.rx_page_size;
+ netdev_queue_config(bp->dev, i, &qcfg);
+ rxr->rx_page_size = qcfg.rx_page_size;
ring = &rxr->rx_ring_struct;
rmem = &ring->ring_mem;
diff --git a/net/core/netdev_config.c b/net/core/netdev_config.c
new file mode 100644
index 000000000000..562087bd30c8
--- /dev/null
+++ b/net/core/netdev_config.c
@@ -0,0 +1,32 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/netdevice.h>
+#include <net/netdev_queues.h>
+#include <net/netdev_rx_queue.h>
+
+/**
+ * netdev_queue_config() - get configuration for a given queue
+ * @dev: net_device instance
+ * @rxq_idx: index of the queue of interest
+ * @qcfg: queue configuration struct (output)
+ *
+ * Render the configuration for a given queue. This helper should be used
+ * by drivers which support queue configuration to retrieve config for
+ * a particular queue.
+ *
+ * @qcfg is an output parameter and is always fully initialized by this
+ * function. Some values may not be set by the user, drivers may either
+ * deal with the "unset" values in @qcfg, or provide the callback
+ * to populate defaults in queue_management_ops.
+ */
+void netdev_queue_config(struct net_device *dev, int rxq_idx,
+ struct netdev_queue_config *qcfg)
+{
+ struct netdev_queue_config *stored;
+
+ memset(qcfg, 0, sizeof(*qcfg));
+
+ stored = &__netif_get_rx_queue(dev, rxq_idx)->qcfg;
+ qcfg->rx_page_size = stored->rx_page_size;
+}
+EXPORT_SYMBOL(netdev_queue_config);
--
2.52.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH net-next 3/6] net: move mp->rx_page_size validation to __net_mp_open_rxq()
2026-01-22 0:51 [PATCH net-next 0/6] net: restore the structure of driver-facing qcfg API Jakub Kicinski
2026-01-22 0:51 ` [PATCH net-next 1/6] eth: bnxt: always set the queue mgmt ops Jakub Kicinski
2026-01-22 0:51 ` [PATCH net-next 2/6] net: introduce a trivial netdev_queue_config() Jakub Kicinski
@ 2026-01-22 0:51 ` Jakub Kicinski
2026-01-22 0:51 ` [PATCH net-next 4/6] net: use netdev_queue_config() for mp restart Jakub Kicinski
` (3 subsequent siblings)
6 siblings, 0 replies; 17+ messages in thread
From: Jakub Kicinski @ 2026-01-22 0:51 UTC (permalink / raw)
To: davem
Cc: netdev, edumazet, pabeni, andrew+netdev, horms, michael.chan, sdf,
almasrymina, dtatulea, asml.silence, dw, daniel, Jakub Kicinski
Move mp->rx_page_size validation where the rest of MP input
validation lives. No other caller is modifying mp params so
validation logic in queue restarts is out of place.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
net/core/netdev_rx_queue.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/net/core/netdev_rx_queue.c b/net/core/netdev_rx_queue.c
index b81cad90ba2f..03d7531abb3a 100644
--- a/net/core/netdev_rx_queue.c
+++ b/net/core/netdev_rx_queue.c
@@ -39,11 +39,8 @@ int netdev_rx_queue_restart(struct net_device *dev, unsigned int rxq_idx)
if (qops->ndo_default_qcfg)
qops->ndo_default_qcfg(dev, &qcfg);
- if (rxq->mp_params.rx_page_size) {
- if (!(qops->supported_params & QCFG_RX_PAGE_SIZE))
- return -EOPNOTSUPP;
+ if (rxq->mp_params.rx_page_size)
qcfg.rx_page_size = rxq->mp_params.rx_page_size;
- }
new_mem = kvzalloc(qops->ndo_queue_mem_size, GFP_KERNEL);
if (!new_mem)
@@ -115,6 +112,7 @@ int __net_mp_open_rxq(struct net_device *dev, unsigned int rxq_idx,
const struct pp_memory_provider_params *p,
struct netlink_ext_ack *extack)
{
+ const struct netdev_queue_mgmt_ops *qops = dev->queue_mgmt_ops;
struct netdev_rx_queue *rxq;
int ret;
@@ -139,6 +137,10 @@ int __net_mp_open_rxq(struct net_device *dev, unsigned int rxq_idx,
NL_SET_ERR_MSG(extack, "unable to custom memory provider to device with XDP program attached");
return -EEXIST;
}
+ if (p->rx_page_size && !(qops->supported_params & QCFG_RX_PAGE_SIZE)) {
+ NL_SET_ERR_MSG(extack, "device does not support: rx_page_size");
+ return -EOPNOTSUPP;
+ }
rxq = __netif_get_rx_queue(dev, rxq_idx);
if (rxq->mp_params.mp_ops) {
--
2.52.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH net-next 4/6] net: use netdev_queue_config() for mp restart
2026-01-22 0:51 [PATCH net-next 0/6] net: restore the structure of driver-facing qcfg API Jakub Kicinski
` (2 preceding siblings ...)
2026-01-22 0:51 ` [PATCH net-next 3/6] net: move mp->rx_page_size validation to __net_mp_open_rxq() Jakub Kicinski
@ 2026-01-22 0:51 ` Jakub Kicinski
2026-01-22 15:06 ` Dragos Tatulea
2026-01-22 0:51 ` [PATCH net-next 5/6] net: add queue config validation callback Jakub Kicinski
` (2 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Jakub Kicinski @ 2026-01-22 0:51 UTC (permalink / raw)
To: davem
Cc: netdev, edumazet, pabeni, andrew+netdev, horms, michael.chan, sdf,
almasrymina, dtatulea, asml.silence, dw, daniel, Jakub Kicinski
We should follow the prepare/commit approach for queue configuration.
The qcfg struct should be added to dev->cfg rather than directly to
queue objects so that we can clone and discard the pending config
easily.
Remove the qcfg in struct netdev_rx_queue, and switch remaining callers
to netdev_queue_config(). netdev_queue_config() will construct the qcfg
on the fly based on device defaults and state of the queue.
ndo_default_qcfg becomes optional because having the callback itself
does not have any meaningful semantics to us.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
include/net/netdev_queues.h | 4 +++-
net/core/dev.c | 17 ---------------
net/core/netdev_config.c | 12 ++++++++---
net/core/netdev_rx_queue.c | 43 +++++++++++++++++++++----------------
4 files changed, 37 insertions(+), 39 deletions(-)
diff --git a/include/net/netdev_queues.h b/include/net/netdev_queues.h
index 725bf69ef86c..4b23c3697cc9 100644
--- a/include/net/netdev_queues.h
+++ b/include/net/netdev_queues.h
@@ -139,7 +139,9 @@ enum {
* @ndo_queue_get_dma_dev: Get dma device for zero-copy operations to be used
* for this queue. Return NULL on error.
*
- * @ndo_default_qcfg: Populate queue config struct with defaults. Optional.
+ * @ndo_default_qcfg: (Optional) Populate queue config struct with defaults.
+ * Queue config structs are passed to this helper before
+ * the user-requested settings are applied.
*
* @supported_params: Bitmask of supported parameters, see QCFG_*.
*
diff --git a/net/core/dev.c b/net/core/dev.c
index 048ab4409a2c..2661b68f5be3 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -11282,21 +11282,6 @@ static void netdev_free_phy_link_topology(struct net_device *dev)
}
}
-static void init_rx_queue_cfgs(struct net_device *dev)
-{
- const struct netdev_queue_mgmt_ops *qops = dev->queue_mgmt_ops;
- struct netdev_rx_queue *rxq;
- int i;
-
- if (!qops || !qops->ndo_default_qcfg)
- return;
-
- for (i = 0; i < dev->num_rx_queues; i++) {
- rxq = __netif_get_rx_queue(dev, i);
- qops->ndo_default_qcfg(dev, &rxq->qcfg);
- }
-}
-
/**
* register_netdevice() - register a network device
* @dev: device to register
@@ -11342,8 +11327,6 @@ int register_netdevice(struct net_device *dev)
if (!dev->name_node)
goto out;
- init_rx_queue_cfgs(dev);
-
/* Init, if this function is available */
if (dev->netdev_ops->ndo_init) {
ret = dev->netdev_ops->ndo_init(dev);
diff --git a/net/core/netdev_config.c b/net/core/netdev_config.c
index 562087bd30c8..48f763446506 100644
--- a/net/core/netdev_config.c
+++ b/net/core/netdev_config.c
@@ -22,11 +22,17 @@
void netdev_queue_config(struct net_device *dev, int rxq_idx,
struct netdev_queue_config *qcfg)
{
- struct netdev_queue_config *stored;
+ struct pp_memory_provider_params *mpp;
memset(qcfg, 0, sizeof(*qcfg));
- stored = &__netif_get_rx_queue(dev, rxq_idx)->qcfg;
- qcfg->rx_page_size = stored->rx_page_size;
+ /* Get defaults from the driver, in case user config not set */
+ if (dev->queue_mgmt_ops->ndo_default_qcfg)
+ dev->queue_mgmt_ops->ndo_default_qcfg(dev, qcfg);
+
+ /* Apply MP overrides */
+ mpp = &__netif_get_rx_queue(dev, rxq_idx)->mp_params;
+ if (mpp->rx_page_size)
+ qcfg->rx_page_size = mpp->rx_page_size;
}
EXPORT_SYMBOL(netdev_queue_config);
diff --git a/net/core/netdev_rx_queue.c b/net/core/netdev_rx_queue.c
index 03d7531abb3a..72374930699a 100644
--- a/net/core/netdev_rx_queue.c
+++ b/net/core/netdev_rx_queue.c
@@ -18,11 +18,13 @@ bool netif_rxq_has_unreadable_mp(struct net_device *dev, int idx)
}
EXPORT_SYMBOL(netif_rxq_has_unreadable_mp);
-int netdev_rx_queue_restart(struct net_device *dev, unsigned int rxq_idx)
+static int netdev_rx_queue_reconfig(struct net_device *dev,
+ unsigned int rxq_idx,
+ struct netdev_queue_config *qcfg_old,
+ struct netdev_queue_config *qcfg_new)
{
struct netdev_rx_queue *rxq = __netif_get_rx_queue(dev, rxq_idx);
const struct netdev_queue_mgmt_ops *qops = dev->queue_mgmt_ops;
- struct netdev_queue_config qcfg;
void *new_mem, *old_mem;
int err;
@@ -30,18 +32,8 @@ int netdev_rx_queue_restart(struct net_device *dev, unsigned int rxq_idx)
!qops->ndo_queue_mem_alloc || !qops->ndo_queue_start)
return -EOPNOTSUPP;
- if (WARN_ON_ONCE(qops->supported_params && !qops->ndo_default_qcfg))
- return -EINVAL;
-
netdev_assert_locked(dev);
- memset(&qcfg, 0, sizeof(qcfg));
- if (qops->ndo_default_qcfg)
- qops->ndo_default_qcfg(dev, &qcfg);
-
- if (rxq->mp_params.rx_page_size)
- qcfg.rx_page_size = rxq->mp_params.rx_page_size;
-
new_mem = kvzalloc(qops->ndo_queue_mem_size, GFP_KERNEL);
if (!new_mem)
return -ENOMEM;
@@ -52,7 +44,7 @@ int netdev_rx_queue_restart(struct net_device *dev, unsigned int rxq_idx)
goto err_free_new_mem;
}
- err = qops->ndo_queue_mem_alloc(dev, &qcfg, new_mem, rxq_idx);
+ err = qops->ndo_queue_mem_alloc(dev, qcfg_new, new_mem, rxq_idx);
if (err)
goto err_free_old_mem;
@@ -65,7 +57,7 @@ int netdev_rx_queue_restart(struct net_device *dev, unsigned int rxq_idx)
if (err)
goto err_free_new_queue_mem;
- err = qops->ndo_queue_start(dev, &qcfg, new_mem, rxq_idx);
+ err = qops->ndo_queue_start(dev, qcfg_new, new_mem, rxq_idx);
if (err)
goto err_start_queue;
} else {
@@ -77,7 +69,6 @@ int netdev_rx_queue_restart(struct net_device *dev, unsigned int rxq_idx)
kvfree(old_mem);
kvfree(new_mem);
- rxq->qcfg = qcfg;
return 0;
err_start_queue:
@@ -88,7 +79,7 @@ int netdev_rx_queue_restart(struct net_device *dev, unsigned int rxq_idx)
* WARN if we fail to recover the old rx queue, and at least free
* old_mem so we don't also leak that.
*/
- if (qops->ndo_queue_start(dev, &rxq->qcfg, old_mem, rxq_idx)) {
+ if (qops->ndo_queue_start(dev, qcfg_old, old_mem, rxq_idx)) {
WARN(1,
"Failed to restart old queue in error path. RX queue %d may be unhealthy.",
rxq_idx);
@@ -106,6 +97,14 @@ int netdev_rx_queue_restart(struct net_device *dev, unsigned int rxq_idx)
return err;
}
+
+int netdev_rx_queue_restart(struct net_device *dev, unsigned int rxq_idx)
+{
+ struct netdev_queue_config qcfg;
+
+ netdev_queue_config(dev, rxq_idx, &qcfg);
+ return netdev_rx_queue_reconfig(dev, rxq_idx, &qcfg, &qcfg);
+}
EXPORT_SYMBOL_NS_GPL(netdev_rx_queue_restart, "NETDEV_INTERNAL");
int __net_mp_open_rxq(struct net_device *dev, unsigned int rxq_idx,
@@ -113,6 +112,7 @@ int __net_mp_open_rxq(struct net_device *dev, unsigned int rxq_idx,
struct netlink_ext_ack *extack)
{
const struct netdev_queue_mgmt_ops *qops = dev->queue_mgmt_ops;
+ struct netdev_queue_config qcfg[2];
struct netdev_rx_queue *rxq;
int ret;
@@ -154,8 +154,11 @@ int __net_mp_open_rxq(struct net_device *dev, unsigned int rxq_idx,
}
#endif
+ netdev_queue_config(dev, rxq_idx, &qcfg[0]);
rxq->mp_params = *p;
- ret = netdev_rx_queue_restart(dev, rxq_idx);
+ netdev_queue_config(dev, rxq_idx, &qcfg[1]);
+
+ ret = netdev_rx_queue_reconfig(dev, rxq_idx, &qcfg[0], &qcfg[1]);
if (ret)
memset(&rxq->mp_params, 0, sizeof(rxq->mp_params));
@@ -176,6 +179,7 @@ int net_mp_open_rxq(struct net_device *dev, unsigned int rxq_idx,
void __net_mp_close_rxq(struct net_device *dev, unsigned int ifq_idx,
const struct pp_memory_provider_params *old_p)
{
+ struct netdev_queue_config qcfg[2];
struct netdev_rx_queue *rxq;
int err;
@@ -195,8 +199,11 @@ void __net_mp_close_rxq(struct net_device *dev, unsigned int ifq_idx,
rxq->mp_params.mp_priv != old_p->mp_priv))
return;
+ netdev_queue_config(dev, ifq_idx, &qcfg[0]);
memset(&rxq->mp_params, 0, sizeof(rxq->mp_params));
- err = netdev_rx_queue_restart(dev, ifq_idx);
+ netdev_queue_config(dev, ifq_idx, &qcfg[1]);
+
+ err = netdev_rx_queue_reconfig(dev, ifq_idx, &qcfg[0], &qcfg[1]);
WARN_ON(err && err != -ENETDOWN);
}
--
2.52.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH net-next 5/6] net: add queue config validation callback
2026-01-22 0:51 [PATCH net-next 0/6] net: restore the structure of driver-facing qcfg API Jakub Kicinski
` (3 preceding siblings ...)
2026-01-22 0:51 ` [PATCH net-next 4/6] net: use netdev_queue_config() for mp restart Jakub Kicinski
@ 2026-01-22 0:51 ` Jakub Kicinski
2026-01-22 0:51 ` [PATCH net-next 6/6] eth: bnxt: plug bnxt_validate_qcfg() into qops Jakub Kicinski
2026-01-23 21:20 ` [PATCH net-next 0/6] net: restore the structure of driver-facing qcfg API patchwork-bot+netdevbpf
6 siblings, 0 replies; 17+ messages in thread
From: Jakub Kicinski @ 2026-01-22 0:51 UTC (permalink / raw)
To: davem
Cc: netdev, edumazet, pabeni, andrew+netdev, horms, michael.chan, sdf,
almasrymina, dtatulea, asml.silence, dw, daniel, Jakub Kicinski
I imagine (tm) that as the number of per-queue configuration
options grows some of them may conflict for certain drivers.
While the drivers can obviously do all the validation locally
doing so is fairly inconvenient as the config is fed to drivers
piecemeal via different ops (for different params and NIC-wide
vs per-queue).
Add a centralized callback for validating the queue config
in queue ops. The callback gets invoked before memory provider
is installed, and in the future should also be called when ring
params are modified.
The validation is done after each layer of configuration.
Since we can't fail MP un-binding we must make sure that
the config is valid both before and after MP overrides are
applied. This is moot for now since the set of MP and device
configs are disjoint. It will matter significantly in the future,
so adding it now so that we don't forget..
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
include/net/netdev_queues.h | 10 ++++++
net/core/dev.h | 5 +++
net/core/netdev_config.c | 64 ++++++++++++++++++++++++++++++-------
net/core/netdev_rx_queue.c | 11 +++++--
4 files changed, 76 insertions(+), 14 deletions(-)
diff --git a/include/net/netdev_queues.h b/include/net/netdev_queues.h
index 4b23c3697cc9..95ed28212f4e 100644
--- a/include/net/netdev_queues.h
+++ b/include/net/netdev_queues.h
@@ -143,6 +143,13 @@ enum {
* Queue config structs are passed to this helper before
* the user-requested settings are applied.
*
+ * @ndo_validate_qcfg: (Optional) Check if queue config is supported.
+ * Called when configuration affecting a queue may be
+ * changing, either due to NIC-wide config, or config
+ * scoped to the queue at a specified index.
+ * When NIC-wide config is changed the callback will
+ * be invoked for all queues.
+ *
* @supported_params: Bitmask of supported parameters, see QCFG_*.
*
* Note that @ndo_queue_mem_alloc and @ndo_queue_mem_free may be called while
@@ -166,6 +173,9 @@ struct netdev_queue_mgmt_ops {
int idx);
void (*ndo_default_qcfg)(struct net_device *dev,
struct netdev_queue_config *qcfg);
+ int (*ndo_validate_qcfg)(struct net_device *dev,
+ struct netdev_queue_config *qcfg,
+ struct netlink_ext_ack *extack);
struct device * (*ndo_queue_get_dma_dev)(struct net_device *dev,
int idx);
diff --git a/net/core/dev.h b/net/core/dev.h
index da18536cbd35..98793a738f43 100644
--- a/net/core/dev.h
+++ b/net/core/dev.h
@@ -10,6 +10,7 @@
struct net;
struct netlink_ext_ack;
+struct netdev_queue_config;
struct cpumask;
/* Random bits of netdevice that don't need to be exposed */
@@ -91,6 +92,10 @@ extern struct rw_semaphore dev_addr_sem;
extern struct list_head net_todo_list;
void netdev_run_todo(void);
+int netdev_queue_config_validate(struct net_device *dev, int rxq_idx,
+ struct netdev_queue_config *qcfg,
+ struct netlink_ext_ack *extack);
+
/* netdev management, shared between various uAPI entry points */
struct netdev_name_node {
struct hlist_node hlist;
diff --git a/net/core/netdev_config.c b/net/core/netdev_config.c
index 48f763446506..f14af365d5cd 100644
--- a/net/core/netdev_config.c
+++ b/net/core/netdev_config.c
@@ -4,6 +4,50 @@
#include <net/netdev_queues.h>
#include <net/netdev_rx_queue.h>
+#include "dev.h"
+
+static int netdev_nop_validate_qcfg(struct net_device *dev,
+ struct netdev_queue_config *qcfg,
+ struct netlink_ext_ack *extack)
+{
+ return 0;
+}
+
+static int __netdev_queue_config(struct net_device *dev, int rxq_idx,
+ struct netdev_queue_config *qcfg,
+ struct netlink_ext_ack *extack,
+ bool validate)
+{
+ int (*validate_cb)(struct net_device *dev,
+ struct netdev_queue_config *qcfg,
+ struct netlink_ext_ack *extack);
+ struct pp_memory_provider_params *mpp;
+ int err;
+
+ validate_cb = netdev_nop_validate_qcfg;
+ if (validate && dev->queue_mgmt_ops->ndo_validate_qcfg)
+ validate_cb = dev->queue_mgmt_ops->ndo_validate_qcfg;
+
+ memset(qcfg, 0, sizeof(*qcfg));
+
+ /* Get defaults from the driver, in case user config not set */
+ if (dev->queue_mgmt_ops->ndo_default_qcfg)
+ dev->queue_mgmt_ops->ndo_default_qcfg(dev, qcfg);
+ err = validate_cb(dev, qcfg, extack);
+ if (err)
+ return err;
+
+ /* Apply MP overrides */
+ mpp = &__netif_get_rx_queue(dev, rxq_idx)->mp_params;
+ if (mpp->rx_page_size)
+ qcfg->rx_page_size = mpp->rx_page_size;
+ err = validate_cb(dev, qcfg, extack);
+ if (err)
+ return err;
+
+ return 0;
+}
+
/**
* netdev_queue_config() - get configuration for a given queue
* @dev: net_device instance
@@ -22,17 +66,13 @@
void netdev_queue_config(struct net_device *dev, int rxq_idx,
struct netdev_queue_config *qcfg)
{
- struct pp_memory_provider_params *mpp;
-
- memset(qcfg, 0, sizeof(*qcfg));
-
- /* Get defaults from the driver, in case user config not set */
- if (dev->queue_mgmt_ops->ndo_default_qcfg)
- dev->queue_mgmt_ops->ndo_default_qcfg(dev, qcfg);
-
- /* Apply MP overrides */
- mpp = &__netif_get_rx_queue(dev, rxq_idx)->mp_params;
- if (mpp->rx_page_size)
- qcfg->rx_page_size = mpp->rx_page_size;
+ __netdev_queue_config(dev, rxq_idx, qcfg, NULL, false);
}
EXPORT_SYMBOL(netdev_queue_config);
+
+int netdev_queue_config_validate(struct net_device *dev, int rxq_idx,
+ struct netdev_queue_config *qcfg,
+ struct netlink_ext_ack *extack)
+{
+ return __netdev_queue_config(dev, rxq_idx, qcfg, extack, true);
+}
diff --git a/net/core/netdev_rx_queue.c b/net/core/netdev_rx_queue.c
index 72374930699a..668a90658f25 100644
--- a/net/core/netdev_rx_queue.c
+++ b/net/core/netdev_rx_queue.c
@@ -7,6 +7,7 @@
#include <net/netdev_rx_queue.h>
#include <net/page_pool/memory_provider.h>
+#include "dev.h"
#include "page_pool_priv.h"
/* See also page_pool_is_unreadable() */
@@ -156,12 +157,18 @@ int __net_mp_open_rxq(struct net_device *dev, unsigned int rxq_idx,
netdev_queue_config(dev, rxq_idx, &qcfg[0]);
rxq->mp_params = *p;
- netdev_queue_config(dev, rxq_idx, &qcfg[1]);
+ ret = netdev_queue_config_validate(dev, rxq_idx, &qcfg[1], extack);
+ if (ret)
+ goto err_clear_mp;
ret = netdev_rx_queue_reconfig(dev, rxq_idx, &qcfg[0], &qcfg[1]);
if (ret)
- memset(&rxq->mp_params, 0, sizeof(rxq->mp_params));
+ goto err_clear_mp;
+ return 0;
+
+err_clear_mp:
+ memset(&rxq->mp_params, 0, sizeof(rxq->mp_params));
return ret;
}
--
2.52.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH net-next 6/6] eth: bnxt: plug bnxt_validate_qcfg() into qops
2026-01-22 0:51 [PATCH net-next 0/6] net: restore the structure of driver-facing qcfg API Jakub Kicinski
` (4 preceding siblings ...)
2026-01-22 0:51 ` [PATCH net-next 5/6] net: add queue config validation callback Jakub Kicinski
@ 2026-01-22 0:51 ` Jakub Kicinski
2026-01-23 21:20 ` [PATCH net-next 0/6] net: restore the structure of driver-facing qcfg API patchwork-bot+netdevbpf
6 siblings, 0 replies; 17+ messages in thread
From: Jakub Kicinski @ 2026-01-22 0:51 UTC (permalink / raw)
To: davem
Cc: netdev, edumazet, pabeni, andrew+netdev, horms, michael.chan, sdf,
almasrymina, dtatulea, asml.silence, dw, daniel, Jakub Kicinski
Plug bnxt_validate_qcfg() back into qops, where it was in my old RFC.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index d57e833ce690..8fc0720c3057 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -15983,8 +15983,12 @@ static void bnxt_queue_default_qcfg(struct net_device *dev,
qcfg->rx_page_size = BNXT_RX_PAGE_SIZE;
}
-static int bnxt_validate_qcfg(struct bnxt *bp, struct netdev_queue_config *qcfg)
+static int bnxt_validate_qcfg(struct net_device *dev,
+ struct netdev_queue_config *qcfg,
+ struct netlink_ext_ack *extack)
{
+ struct bnxt *bp = netdev_priv(dev);
+
/* Older chips need MSS calc so rx_page_size is not supported */
if (!(bp->flags & BNXT_FLAG_CHIP_P5_PLUS) &&
qcfg->rx_page_size != BNXT_RX_PAGE_SIZE)
@@ -16012,10 +16016,6 @@ static int bnxt_queue_mem_alloc(struct net_device *dev,
if (!bp->rx_ring)
return -ENETDOWN;
- rc = bnxt_validate_qcfg(bp, qcfg);
- if (rc < 0)
- return rc;
-
rxr = &bp->rx_ring[idx];
clone = qmem;
memcpy(clone, rxr, sizeof(*rxr));
@@ -16311,6 +16311,7 @@ static const struct netdev_queue_mgmt_ops bnxt_queue_mgmt_ops = {
.ndo_queue_start = bnxt_queue_start,
.ndo_queue_stop = bnxt_queue_stop,
.ndo_default_qcfg = bnxt_queue_default_qcfg,
+ .ndo_validate_qcfg = bnxt_validate_qcfg,
.supported_params = QCFG_RX_PAGE_SIZE,
};
--
2.52.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 1/6] eth: bnxt: always set the queue mgmt ops
2026-01-22 0:51 ` [PATCH net-next 1/6] eth: bnxt: always set the queue mgmt ops Jakub Kicinski
@ 2026-01-22 8:04 ` Subbaraya Sundeep
2026-01-22 15:36 ` Jakub Kicinski
0 siblings, 1 reply; 17+ messages in thread
From: Subbaraya Sundeep @ 2026-01-22 8:04 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms,
michael.chan, sdf, almasrymina, dtatulea, asml.silence, dw,
daniel
On 2026-01-22 at 06:21:08, Jakub Kicinski (kuba@kernel.org) wrote:
> Core provides a centralized callback for validating per-queue settings
> but the callback is part of the queue management ops. Having the ops
> conditionally set complicates the parts of the driver which could
> otherwise lean on the core to feed it the correct settings.
>
> Always set the queue ops, but provide no restart-related callbacks if
> queue ops are not supported by the device. This should maintain current
> behavior, the check in netdev_rx_queue_restart() looks both at op struct
> and individual ops.
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> --
> v1:
> - reflow code
> - typo fix
> rfc: https://lore.kernel.org/20250421222827.283737-15-kuba@kernel.org
> ---
> drivers/net/ethernet/broadcom/bnxt/bnxt.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index 0e0c88c122f8..0b95100a7c36 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> @@ -16314,6 +16314,9 @@ static const struct netdev_queue_mgmt_ops bnxt_queue_mgmt_ops = {
> .supported_params = QCFG_RX_PAGE_SIZE,
> };
>
> +static const struct netdev_queue_mgmt_ops bnxt_queue_mgmt_ops_unsupp = {
> +};
> +
> static void bnxt_remove_one(struct pci_dev *pdev)
> {
> struct net_device *dev = pci_get_drvdata(pdev);
> @@ -16966,9 +16969,10 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>
> if (BNXT_SUPPORTS_NTUPLE_VNIC(bp))
> bp->rss_cap |= BNXT_RSS_CAP_MULTI_RSS_CTX;
> +
> + dev->queue_mgmt_ops = &bnxt_queue_mgmt_ops_unsupp;
> if (BNXT_SUPPORTS_QUEUE_API(bp))
> dev->queue_mgmt_ops = &bnxt_queue_mgmt_ops;
> - dev->request_ops_lock = true;
Can we also mention that driver with queue_mgmt_ops set makes it as ops locked
driver in Documentation/networking/netdevices.rst ?
Thanks,
Sundeep
> dev->netmem_tx = true;
>
> rc = register_netdev(dev);
> --
> 2.52.0
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 2/6] net: introduce a trivial netdev_queue_config()
2026-01-22 0:51 ` [PATCH net-next 2/6] net: introduce a trivial netdev_queue_config() Jakub Kicinski
@ 2026-01-22 8:12 ` Subbaraya Sundeep
2026-01-22 15:36 ` Jakub Kicinski
0 siblings, 1 reply; 17+ messages in thread
From: Subbaraya Sundeep @ 2026-01-22 8:12 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms,
michael.chan, sdf, almasrymina, dtatulea, asml.silence, dw,
daniel
On 2026-01-22 at 06:21:09, Jakub Kicinski (kuba@kernel.org) wrote:
> We may choose to extend or reimplement the logic which renders
> the per-queue config. The drivers should not poke directly into
> the queue state. Add a helper for drivers to use when they want
> to query the config for a specific queue.
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> net/core/Makefile | 1 +
> include/net/netdev_queues.h | 3 +++
> drivers/net/ethernet/broadcom/bnxt/bnxt.c | 6 ++---
> net/core/netdev_config.c | 32 +++++++++++++++++++++++
> 4 files changed, 39 insertions(+), 3 deletions(-)
> create mode 100644 net/core/netdev_config.c
>
> diff --git a/net/core/Makefile b/net/core/Makefile
> index 9ef2099c5426..d643a5a7fd18 100644
> --- a/net/core/Makefile
> +++ b/net/core/Makefile
> @@ -19,6 +19,7 @@ obj-$(CONFIG_NETDEV_ADDR_LIST_TEST) += dev_addr_lists_test.o
>
> obj-y += net-sysfs.o
> obj-y += hotdata.o
> +obj-y += netdev_config.o
> obj-y += netdev_rx_queue.o
> obj-y += netdev_queues.o
> obj-$(CONFIG_PAGE_POOL) += page_pool.o page_pool_user.o
> diff --git a/include/net/netdev_queues.h b/include/net/netdev_queues.h
> index 2ab3eae8e8c3..725bf69ef86c 100644
> --- a/include/net/netdev_queues.h
> +++ b/include/net/netdev_queues.h
> @@ -170,6 +170,9 @@ struct netdev_queue_mgmt_ops {
> unsigned int supported_params;
> };
>
> +void netdev_queue_config(struct net_device *dev, int rxq,
> + struct netdev_queue_config *qcfg);
> +
Minor nit: make function name something like netdev_get_queue_config
(to avoid same as struct name)
Thanks,
Sundeep
> bool netif_rxq_has_unreadable_mp(struct net_device *dev, int idx);
>
> /**
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index 0b95100a7c36..d57e833ce690 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> @@ -4326,12 +4326,12 @@ static void bnxt_init_ring_struct(struct bnxt *bp)
>
> for (i = 0; i < bp->cp_nr_rings; i++) {
> struct bnxt_napi *bnapi = bp->bnapi[i];
> + struct netdev_queue_config qcfg;
> struct bnxt_ring_mem_info *rmem;
> struct bnxt_cp_ring_info *cpr;
> struct bnxt_rx_ring_info *rxr;
> struct bnxt_tx_ring_info *txr;
> struct bnxt_ring_struct *ring;
> - struct netdev_rx_queue *rxq;
>
> if (!bnapi)
> continue;
> @@ -4349,8 +4349,8 @@ static void bnxt_init_ring_struct(struct bnxt *bp)
> if (!rxr)
> goto skip_rx;
>
> - rxq = __netif_get_rx_queue(bp->dev, i);
> - rxr->rx_page_size = rxq->qcfg.rx_page_size;
> + netdev_queue_config(bp->dev, i, &qcfg);
> + rxr->rx_page_size = qcfg.rx_page_size;
>
> ring = &rxr->rx_ring_struct;
> rmem = &ring->ring_mem;
> diff --git a/net/core/netdev_config.c b/net/core/netdev_config.c
> new file mode 100644
> index 000000000000..562087bd30c8
> --- /dev/null
> +++ b/net/core/netdev_config.c
> @@ -0,0 +1,32 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <linux/netdevice.h>
> +#include <net/netdev_queues.h>
> +#include <net/netdev_rx_queue.h>
> +
> +/**
> + * netdev_queue_config() - get configuration for a given queue
> + * @dev: net_device instance
> + * @rxq_idx: index of the queue of interest
> + * @qcfg: queue configuration struct (output)
> + *
> + * Render the configuration for a given queue. This helper should be used
> + * by drivers which support queue configuration to retrieve config for
> + * a particular queue.
> + *
> + * @qcfg is an output parameter and is always fully initialized by this
> + * function. Some values may not be set by the user, drivers may either
> + * deal with the "unset" values in @qcfg, or provide the callback
> + * to populate defaults in queue_management_ops.
> + */
> +void netdev_queue_config(struct net_device *dev, int rxq_idx,
> + struct netdev_queue_config *qcfg)
> +{
> + struct netdev_queue_config *stored;
> +
> + memset(qcfg, 0, sizeof(*qcfg));
> +
> + stored = &__netif_get_rx_queue(dev, rxq_idx)->qcfg;
> + qcfg->rx_page_size = stored->rx_page_size;
> +}
> +EXPORT_SYMBOL(netdev_queue_config);
> --
> 2.52.0
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 4/6] net: use netdev_queue_config() for mp restart
2026-01-22 0:51 ` [PATCH net-next 4/6] net: use netdev_queue_config() for mp restart Jakub Kicinski
@ 2026-01-22 15:06 ` Dragos Tatulea
2026-01-22 15:46 ` Jakub Kicinski
0 siblings, 1 reply; 17+ messages in thread
From: Dragos Tatulea @ 2026-01-22 15:06 UTC (permalink / raw)
To: Jakub Kicinski, davem
Cc: netdev, edumazet, pabeni, andrew+netdev, horms, michael.chan, sdf,
almasrymina, asml.silence, dw, daniel
On 22.01.26 01:51, Jakub Kicinski wrote:
[...]
> @@ -154,8 +154,11 @@ int __net_mp_open_rxq(struct net_device *dev, unsigned int rxq_idx,
> }
> #endif
>
> + netdev_queue_config(dev, rxq_idx, &qcfg[0]);
> rxq->mp_params = *p;
> - ret = netdev_rx_queue_restart(dev, rxq_idx);
> + netdev_queue_config(dev, rxq_idx, &qcfg[1]);
> +
> + ret = netdev_rx_queue_reconfig(dev, rxq_idx, &qcfg[0], &qcfg[1]);
> if (ret)
> memset(&rxq->mp_params, 0, sizeof(rxq->mp_params));
>
> @@ -176,6 +179,7 @@ int net_mp_open_rxq(struct net_device *dev, unsigned int rxq_idx,
> void __net_mp_close_rxq(struct net_device *dev, unsigned int ifq_idx,
> const struct pp_memory_provider_params *old_p)
> {
> + struct netdev_queue_config qcfg[2];
> struct netdev_rx_queue *rxq;
> int err;
>
> @@ -195,8 +199,11 @@ void __net_mp_close_rxq(struct net_device *dev, unsigned int ifq_idx,
> rxq->mp_params.mp_priv != old_p->mp_priv))
> return;
>
> + netdev_queue_config(dev, ifq_idx, &qcfg[0]);
> memset(&rxq->mp_params, 0, sizeof(rxq->mp_params));
> - err = netdev_rx_queue_restart(dev, ifq_idx);
> + netdev_queue_config(dev, ifq_idx, &qcfg[1]);
> +
Is it ok to assume that on close we always resume to the default?
For now yes but maybe in the future we might want to save qcfg to the
state before mp_open.
With the very first rx-buf-len series it was possible to set a
rx-buf-len via YNL for a normal queue, switch to a MP queue and then
on MP queue close the configuration got switched to the default value
of rx-buf-len instead of what the user had configured. This was
not convenient.
Thanks,
Dragos
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 1/6] eth: bnxt: always set the queue mgmt ops
2026-01-22 8:04 ` Subbaraya Sundeep
@ 2026-01-22 15:36 ` Jakub Kicinski
2026-01-22 16:31 ` Subbaraya Sundeep
0 siblings, 1 reply; 17+ messages in thread
From: Jakub Kicinski @ 2026-01-22 15:36 UTC (permalink / raw)
To: Subbaraya Sundeep
Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms,
michael.chan, sdf, almasrymina, dtatulea, asml.silence, dw,
daniel
On Thu, 22 Jan 2026 13:34:20 +0530 Subbaraya Sundeep wrote:
> > +
> > + dev->queue_mgmt_ops = &bnxt_queue_mgmt_ops_unsupp;
> > if (BNXT_SUPPORTS_QUEUE_API(bp))
> > dev->queue_mgmt_ops = &bnxt_queue_mgmt_ops;
> > - dev->request_ops_lock = true;
>
> Can we also mention that driver with queue_mgmt_ops set makes it as ops locked
> driver in Documentation/networking/netdevices.rst ?
Did you check if it's already documented there before asking me to do
it? I know people don't read docs much but asking someone to document
stuff that's already documented is a bit funny..
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 2/6] net: introduce a trivial netdev_queue_config()
2026-01-22 8:12 ` Subbaraya Sundeep
@ 2026-01-22 15:36 ` Jakub Kicinski
0 siblings, 0 replies; 17+ messages in thread
From: Jakub Kicinski @ 2026-01-22 15:36 UTC (permalink / raw)
To: Subbaraya Sundeep
Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms,
michael.chan, sdf, almasrymina, dtatulea, asml.silence, dw,
daniel
On Thu, 22 Jan 2026 13:42:40 +0530 Subbaraya Sundeep wrote:
> > +void netdev_queue_config(struct net_device *dev, int rxq,
> > + struct netdev_queue_config *qcfg);
> > +
> Minor nit: make function name something like netdev_get_queue_config
> (to avoid same as struct name)
No.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 4/6] net: use netdev_queue_config() for mp restart
2026-01-22 15:06 ` Dragos Tatulea
@ 2026-01-22 15:46 ` Jakub Kicinski
2026-01-22 16:28 ` Dragos Tatulea
0 siblings, 1 reply; 17+ messages in thread
From: Jakub Kicinski @ 2026-01-22 15:46 UTC (permalink / raw)
To: Dragos Tatulea
Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms,
michael.chan, sdf, almasrymina, asml.silence, dw, daniel
On Thu, 22 Jan 2026 16:06:40 +0100 Dragos Tatulea wrote:
> > @@ -195,8 +199,11 @@ void __net_mp_close_rxq(struct net_device *dev, unsigned int ifq_idx,
> > rxq->mp_params.mp_priv != old_p->mp_priv))
> > return;
> >
> > + netdev_queue_config(dev, ifq_idx, &qcfg[0]);
> > memset(&rxq->mp_params, 0, sizeof(rxq->mp_params));
> > - err = netdev_rx_queue_restart(dev, ifq_idx);
> > + netdev_queue_config(dev, ifq_idx, &qcfg[1]);
> > +
> Is it ok to assume that on close we always resume to the default?
> For now yes but maybe in the future we might want to save qcfg to the
> state before mp_open.
When we add the ability to configure the params via Netlink we should
insert another chunk into [__]netdev_queue_config().
netdev_queue_config() should evaluate the config in reverse order of
priority, so:
- get defaults
- get device-level config
- get per-queue config
- get MP overrides
On close we are clearing the MP overrides, since we don't have
per-queue config we revert to defaults (as you say). But once there's
some overlap with device or per-queue configs we'll go back to the next
level of config in order of priority.
Did I understand the question right?
FWIW I think something that'd be a major usability win would be to make
MP presence imply per-queue HDS threshold of 0 automatically. So that'd
probably be first on my list of knobs to extend the "priority" model to.
> With the very first rx-buf-len series it was possible to set a
> rx-buf-len via YNL for a normal queue, switch to a MP queue and then
> on MP queue close the configuration got switched to the default value
> of rx-buf-len instead of what the user had configured. This was
> not convenient.
Yes, not sure IIUC, but the fact that clearing the MP didn't
automatically delete the MP-related setting was the main reason
we ditched the full qcfg for this use case.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 4/6] net: use netdev_queue_config() for mp restart
2026-01-22 15:46 ` Jakub Kicinski
@ 2026-01-22 16:28 ` Dragos Tatulea
2026-01-23 1:41 ` Jakub Kicinski
0 siblings, 1 reply; 17+ messages in thread
From: Dragos Tatulea @ 2026-01-22 16:28 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms,
michael.chan, sdf, almasrymina, asml.silence, dw, daniel
On 22.01.26 16:46, Jakub Kicinski wrote:
> On Thu, 22 Jan 2026 16:06:40 +0100 Dragos Tatulea wrote:
>>> @@ -195,8 +199,11 @@ void __net_mp_close_rxq(struct net_device *dev, unsigned int ifq_idx,
>>> rxq->mp_params.mp_priv != old_p->mp_priv))
>>> return;
>>>
>>> + netdev_queue_config(dev, ifq_idx, &qcfg[0]);
>>> memset(&rxq->mp_params, 0, sizeof(rxq->mp_params));
>>> - err = netdev_rx_queue_restart(dev, ifq_idx);
>>> + netdev_queue_config(dev, ifq_idx, &qcfg[1]);
>>> +
>> Is it ok to assume that on close we always resume to the default?
>> For now yes but maybe in the future we might want to save qcfg to the
>> state before mp_open.
>
> When we add the ability to configure the params via Netlink we should
> insert another chunk into [__]netdev_queue_config().
> netdev_queue_config() should evaluate the config in reverse order of
> priority, so:
> - get defaults
> - get device-level config
> - get per-queue config
> - get MP overrides
>
> On close we are clearing the MP overrides, since we don't have
> per-queue config we revert to defaults (as you say). But once there's
> some overlap with device or per-queue configs we'll go back to the next
> level of config in order of priority.
>
> Did I understand the question right?
>
Yes, that was my point. The order that you mentioned was my concern as
well.
> FWIW I think something that'd be a major usability win would be to make
> MP presence imply per-queue HDS threshold of 0 automatically. So that'd
> probably be first on my list of knobs to extend the "priority" model to.
>
You mean to set it to 0 during MP queue lifetime and then revert to
previous value according to the above priority list. Right?
>> With the very first rx-buf-len series it was possible to set a
>> rx-buf-len via YNL for a normal queue, switch to a MP queue and then
>> on MP queue close the configuration got switched to the default value
>> of rx-buf-len instead of what the user had configured. This was
>> not convenient.
>
> Yes, not sure IIUC, but the fact that clearing the MP didn't
> automatically delete the MP-related setting was the main reason
> we ditched the full qcfg for this use case.
Ack.
Other than that, I reviewed the series and don't have any other
comment. So FWIW:
Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
Thanks,
Dragos
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 1/6] eth: bnxt: always set the queue mgmt ops
2026-01-22 15:36 ` Jakub Kicinski
@ 2026-01-22 16:31 ` Subbaraya Sundeep
0 siblings, 0 replies; 17+ messages in thread
From: Subbaraya Sundeep @ 2026-01-22 16:31 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms,
michael.chan, sdf, almasrymina, dtatulea, asml.silence, dw,
daniel
On 2026-01-22 at 21:06:43, Jakub Kicinski (kuba@kernel.org) wrote:
> On Thu, 22 Jan 2026 13:34:20 +0530 Subbaraya Sundeep wrote:
> > > +
> > > + dev->queue_mgmt_ops = &bnxt_queue_mgmt_ops_unsupp;
> > > if (BNXT_SUPPORTS_QUEUE_API(bp))
> > > dev->queue_mgmt_ops = &bnxt_queue_mgmt_ops;
> > > - dev->request_ops_lock = true;
> >
> > Can we also mention that driver with queue_mgmt_ops set makes it as ops locked
> > driver in Documentation/networking/netdevices.rst ?
>
> Did you check if it's already documented there before asking me to do
> it? I know people don't read docs much but asking someone to document
> stuff that's already documented is a bit funny..
Okay it is there:
"Note that supporting struct netdev_queue_mgmt_ops automatically enables
"ops locking". "
Reviewed-by: Subbaraya Sundeep <sbhatta@marvell.com>
Thanks,
Sundeep
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 4/6] net: use netdev_queue_config() for mp restart
2026-01-22 16:28 ` Dragos Tatulea
@ 2026-01-23 1:41 ` Jakub Kicinski
0 siblings, 0 replies; 17+ messages in thread
From: Jakub Kicinski @ 2026-01-23 1:41 UTC (permalink / raw)
To: Dragos Tatulea
Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms,
michael.chan, sdf, almasrymina, asml.silence, dw, daniel
On Thu, 22 Jan 2026 17:28:02 +0100 Dragos Tatulea wrote:
> > FWIW I think something that'd be a major usability win would be to make
> > MP presence imply per-queue HDS threshold of 0 automatically. So that'd
> > probably be first on my list of knobs to extend the "priority" model to.
>
> You mean to set it to 0 during MP queue lifetime and then revert to
> previous value according to the above priority list. Right?
Exactly. threshold=0 and HDS=on when MP is attached (assuming driver
has the right capabilities in the queue ops struct).
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH net-next 0/6] net: restore the structure of driver-facing qcfg API
2026-01-22 0:51 [PATCH net-next 0/6] net: restore the structure of driver-facing qcfg API Jakub Kicinski
` (5 preceding siblings ...)
2026-01-22 0:51 ` [PATCH net-next 6/6] eth: bnxt: plug bnxt_validate_qcfg() into qops Jakub Kicinski
@ 2026-01-23 21:20 ` patchwork-bot+netdevbpf
6 siblings, 0 replies; 17+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-01-23 21:20 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms,
michael.chan, sdf, almasrymina, dtatulea, asml.silence, dw,
daniel
Hello:
This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Wed, 21 Jan 2026 16:51:07 -0800 you wrote:
> The goal of qcfg objects is to let us seamlessly support new use cases
> without modifying all the drivers. We want to pull all the logic of
> combining configuration supplied via different interfaces into the core
> and present the drivers with a flat queue-by-queue configuration.
> Additionally we want to separate the current effective configuration
> from the user intent (default vs user setting vs memory provider setting).
>
> [...]
Here is the summary with links:
- [net-next,1/6] eth: bnxt: always set the queue mgmt ops
https://git.kernel.org/netdev/net-next/c/1410c7416dc3
- [net-next,2/6] net: introduce a trivial netdev_queue_config()
https://git.kernel.org/netdev/net-next/c/b9ac2c60a3ad
- [net-next,3/6] net: move mp->rx_page_size validation to __net_mp_open_rxq()
https://git.kernel.org/netdev/net-next/c/da7772a2b4ad
- [net-next,4/6] net: use netdev_queue_config() for mp restart
https://git.kernel.org/netdev/net-next/c/fc1a78a25c5e
- [net-next,5/6] net: add queue config validation callback
https://git.kernel.org/netdev/net-next/c/8e3245cb3086
- [net-next,6/6] eth: bnxt: plug bnxt_validate_qcfg() into qops
https://git.kernel.org/netdev/net-next/c/b33006ebb78a
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2026-01-23 21:20 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-22 0:51 [PATCH net-next 0/6] net: restore the structure of driver-facing qcfg API Jakub Kicinski
2026-01-22 0:51 ` [PATCH net-next 1/6] eth: bnxt: always set the queue mgmt ops Jakub Kicinski
2026-01-22 8:04 ` Subbaraya Sundeep
2026-01-22 15:36 ` Jakub Kicinski
2026-01-22 16:31 ` Subbaraya Sundeep
2026-01-22 0:51 ` [PATCH net-next 2/6] net: introduce a trivial netdev_queue_config() Jakub Kicinski
2026-01-22 8:12 ` Subbaraya Sundeep
2026-01-22 15:36 ` Jakub Kicinski
2026-01-22 0:51 ` [PATCH net-next 3/6] net: move mp->rx_page_size validation to __net_mp_open_rxq() Jakub Kicinski
2026-01-22 0:51 ` [PATCH net-next 4/6] net: use netdev_queue_config() for mp restart Jakub Kicinski
2026-01-22 15:06 ` Dragos Tatulea
2026-01-22 15:46 ` Jakub Kicinski
2026-01-22 16:28 ` Dragos Tatulea
2026-01-23 1:41 ` Jakub Kicinski
2026-01-22 0:51 ` [PATCH net-next 5/6] net: add queue config validation callback Jakub Kicinski
2026-01-22 0:51 ` [PATCH net-next 6/6] eth: bnxt: plug bnxt_validate_qcfg() into qops Jakub Kicinski
2026-01-23 21:20 ` [PATCH net-next 0/6] net: restore the structure of driver-facing qcfg API patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox