* [PATCH v2 1/8] net/gve: refactor ethdev for control ops interface
2026-10-05 19:39 ` [PATCH v2 " Joshua Washington
@ 2026-10-05 19:39 ` Joshua Washington
2026-10-05 19:39 ` [PATCH v2 2/8] net/gve: rename doorbell BAR variable to be more generic Joshua Washington
` (8 subsequent siblings)
9 siblings, 0 replies; 22+ messages in thread
From: Joshua Washington @ 2026-10-05 19:39 UTC (permalink / raw)
To: Joshua Washington; +Cc: dev, Mark Blasko
From: Mark Blasko <blasko@google.com>
Abstract control plane operations into a control operations interface.
This decouples the core driver logic from the AQ implementation.
Align AdminQ function parameters to match control operations signatures:
- gve_adminq_configure_device_resources(): read counter array and IRQ
doorbell DMA addresses/counts directly from struct gve_priv (wired to
the get_interrupt_dbs control op).
- gve_adminq_get_ptype_map_dqo(): populate priv->ptype_lut_dqo directly
from struct gve_priv (wired to the get_ptype_map control op).
Move the max_tx_queues and max_rx_queues register reads from
gve_dev_init() into gve_adminq_get_device_properties(), as maximum
queue counts are retrieved via get_device_properties in mailbox mode.
Signed-off-by: Mark Blasko <blasko@google.com>
Signed-off-by: Joshua Washington <joshwash@google.com>
---
drivers/net/gve/base/gve_adminq.c | 18 ++---
drivers/net/gve/base/gve_adminq.h | 9 +--
drivers/net/gve/gve_ethdev.c | 108 +++++++++++++++++++-----------
drivers/net/gve/gve_ethdev.h | 45 +++++++++++++
drivers/net/gve/gve_flow_rule.c | 6 +-
drivers/net/gve/gve_rx.c | 2 +-
drivers/net/gve/gve_rx_dqo.c | 2 +-
drivers/net/gve/gve_tx.c | 2 +-
drivers/net/gve/gve_tx_dqo.c | 2 +-
9 files changed, 130 insertions(+), 64 deletions(-)
diff --git a/drivers/net/gve/base/gve_adminq.c b/drivers/net/gve/base/gve_adminq.c
index 89bf669c24..74b9a0125c 100644
--- a/drivers/net/gve/base/gve_adminq.c
+++ b/drivers/net/gve/base/gve_adminq.c
@@ -664,11 +664,7 @@ int gve_adminq_report_nic_timestamp(struct gve_priv *priv, dma_addr_t nic_ts_rep
* gve arranges the msix vectors so that the management vector is last.
*/
#define GVE_NTFY_BLK_BASE_MSIX_IDX 0
-int gve_adminq_configure_device_resources(struct gve_priv *priv,
- dma_addr_t counter_array_bus_addr,
- u32 num_counters,
- dma_addr_t db_array_bus_addr,
- u32 num_ntfy_blks)
+int gve_adminq_configure_device_resources(struct gve_priv *priv)
{
union gve_adminq_command cmd;
@@ -676,10 +672,10 @@ int gve_adminq_configure_device_resources(struct gve_priv *priv,
cmd.opcode = cpu_to_be32(GVE_ADMINQ_CONFIGURE_DEVICE_RESOURCES);
cmd.configure_device_resources =
(struct gve_adminq_configure_device_resources) {
- .counter_array = cpu_to_be64(counter_array_bus_addr),
- .num_counters = cpu_to_be32(num_counters),
- .irq_db_addr = cpu_to_be64(db_array_bus_addr),
- .num_irq_dbs = cpu_to_be32(num_ntfy_blks),
+ .counter_array = cpu_to_be64(priv->cnt_array_mz->iova),
+ .num_counters = cpu_to_be32(priv->num_event_counters),
+ .irq_db_addr = cpu_to_be64(priv->irq_dbs_mz->iova),
+ .num_irq_dbs = cpu_to_be32(priv->num_ntfy_blks),
.irq_db_stride = cpu_to_be32(sizeof(*priv->irq_dbs)),
.ntfy_blk_msix_base_idx =
cpu_to_be32(GVE_NTFY_BLK_BASE_MSIX_IDX),
@@ -1198,9 +1194,9 @@ int gve_adminq_report_link_speed(struct gve_priv *priv)
return err;
}
-int gve_adminq_get_ptype_map_dqo(struct gve_priv *priv,
- struct gve_ptype_lut *ptype_lut)
+int gve_adminq_get_ptype_map_dqo(struct gve_priv *priv)
{
+ struct gve_ptype_lut *ptype_lut = priv->ptype_lut_dqo;
struct gve_dma_mem ptype_map_dma_mem;
struct gve_ptype_map *ptype_map;
union gve_adminq_command cmd;
diff --git a/drivers/net/gve/base/gve_adminq.h b/drivers/net/gve/base/gve_adminq.h
index 954be39fbf..5002cdf783 100644
--- a/drivers/net/gve/base/gve_adminq.h
+++ b/drivers/net/gve/base/gve_adminq.h
@@ -523,11 +523,7 @@ int gve_adminq_alloc(struct gve_priv *priv);
void gve_adminq_free(struct gve_priv *priv);
void gve_adminq_release(struct gve_priv *priv);
int gve_adminq_describe_device(struct gve_priv *priv);
-int gve_adminq_configure_device_resources(struct gve_priv *priv,
- dma_addr_t counter_array_bus_addr,
- u32 num_counters,
- dma_addr_t db_array_bus_addr,
- u32 num_ntfy_blks);
+int gve_adminq_configure_device_resources(struct gve_priv *priv);
int gve_adminq_deconfigure_device_resources(struct gve_priv *priv);
int gve_adminq_create_tx_queues(struct gve_priv *priv, u32 num_queues);
int gve_adminq_destroy_tx_queues(struct gve_priv *priv, u32 queue_id);
@@ -542,8 +538,7 @@ int gve_adminq_report_stats(struct gve_priv *priv, u64 stats_report_len,
int gve_adminq_report_link_speed(struct gve_priv *priv);
struct gve_ptype_lut;
-int gve_adminq_get_ptype_map_dqo(struct gve_priv *priv,
- struct gve_ptype_lut *ptype_lut);
+int gve_adminq_get_ptype_map_dqo(struct gve_priv *priv);
int gve_adminq_verify_driver_compatibility(struct gve_priv *priv,
u64 driver_info_len,
diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c
index c990920a4d..6704cc40b3 100644
--- a/drivers/net/gve/gve_ethdev.c
+++ b/drivers/net/gve/gve_ethdev.c
@@ -181,7 +181,7 @@ gve_setup_queue_page_list(struct gve_priv *priv, uint16_t queue_id, bool is_rx,
priv->max_registered_pages);
goto cleanup_qpl;
}
- err = gve_adminq_register_page_list(priv, qpl);
+ err = priv->ctrl_ops->register_page_list(priv, qpl);
if (err) {
PMD_DRV_LOG(ERR,
"Failed to register %s qpl for queue %hu.",
@@ -200,7 +200,7 @@ int
gve_teardown_queue_page_list(struct gve_priv *priv,
struct gve_queue_page_list *qpl)
{
- int err = gve_adminq_unregister_page_list(priv, qpl->id);
+ int err = priv->ctrl_ops->unregister_page_list(priv, qpl->id);
if (err) {
PMD_DRV_LOG(CRIT, "Unable to unregister qpl %d!", qpl->id);
return err;
@@ -239,7 +239,7 @@ gve_dev_configure(struct rte_eth_dev *dev)
gve_init_rss_config_from_priv(priv, &update_reta_config);
gve_generate_rss_reta(dev, &update_reta_config);
- err = gve_adminq_configure_rss(priv, &update_reta_config);
+ err = priv->ctrl_ops->configure_rss(priv, &update_reta_config);
if (err)
PMD_DRV_LOG(ERR,
"Could not reconfigure RSS redirection table.");
@@ -270,7 +270,7 @@ gve_link_update(struct rte_eth_dev *dev, __rte_unused int wait_to_complete)
} else {
link.link_status = RTE_ETH_LINK_UP;
PMD_DRV_LOG(DEBUG, "Get link status from hw");
- err = gve_adminq_report_link_speed(priv);
+ err = priv->ctrl_ops->report_link_speed(priv);
if (err) {
PMD_DRV_LOG(ERR, "Failed to get link speed.");
priv->link_speed = RTE_ETH_SPEED_NUM_UNKNOWN;
@@ -366,7 +366,7 @@ gve_start_queues(struct rte_eth_dev *dev)
num_queues = dev->data->nb_tx_queues;
priv->txqs = (struct gve_tx_queue **)dev->data->tx_queues;
- ret = gve_adminq_create_tx_queues(priv, num_queues);
+ ret = priv->ctrl_ops->create_tx_queues(priv, num_queues);
if (ret != 0) {
PMD_DRV_LOG(ERR, "Failed to create %u tx queues.", num_queues);
return ret;
@@ -384,7 +384,7 @@ gve_start_queues(struct rte_eth_dev *dev)
num_queues = dev->data->nb_rx_queues;
priv->rxqs = (struct gve_rx_queue **)dev->data->rx_queues;
- ret = gve_adminq_create_rx_queues(priv, num_queues);
+ ret = priv->ctrl_ops->create_rx_queues(priv, num_queues);
if (ret != 0) {
PMD_DRV_LOG(ERR, "Failed to create %u rx queues.", num_queues);
goto err_tx;
@@ -450,11 +450,11 @@ gve_dev_start(struct rte_eth_dev *dev)
"Failed to allocate region for stats reporting.");
return ret;
}
- ret = gve_adminq_report_stats(priv, priv->stats_report_len,
+ ret = priv->ctrl_ops->setup_stats_report(priv, priv->stats_report_len,
priv->stats_report_mem->iova,
GVE_STATS_REPORT_TIMER_PERIOD);
if (ret != 0) {
- PMD_DRV_LOG(ERR, "gve_adminq_report_stats command failed.");
+ PMD_DRV_LOG(ERR, "setup_stats_report command failed.");
return ret;
}
}
@@ -478,7 +478,7 @@ gve_read_nic_clock(void *arg)
pthread_mutex_lock(&priv->nic_ts_lock);
memset(priv->nic_ts_report, 0, sizeof(struct gve_nic_ts_report));
- err = gve_adminq_report_nic_timestamp(priv, priv->nic_ts_report_mz->iova);
+ err = priv->ctrl_ops->report_nic_timestamp(priv, priv->nic_ts_report_mz->iova);
if (err == 0) {
ts = be64_to_cpu(priv->nic_ts_report->nic_timestamp);
pthread_mutex_unlock(&priv->nic_ts_lock);
@@ -682,7 +682,7 @@ gve_teardown_device_resources(struct gve_priv *priv)
/* Tell device its resources are being freed */
if (gve_get_device_resources_ok(priv)) {
- err = gve_adminq_deconfigure_device_resources(priv);
+ err = priv->ctrl_ops->free_db_resources(priv);
if (err)
PMD_DRV_LOG(ERR,
"Could not deconfigure device resources: err=%d",
@@ -722,7 +722,7 @@ gve_dev_close(struct rte_eth_dev *dev)
gve_free_queues(dev);
gve_teardown_device_resources(priv);
- gve_adminq_free(priv);
+ priv->ctrl_ops->free_ctrl_plane(priv);
pthread_mutex_destroy(&priv->flow_rule_lock);
pthread_mutex_destroy(&priv->nic_ts_lock);
@@ -756,7 +756,7 @@ gve_dev_reset(struct rte_eth_dev *dev)
*/
gve_free_queues(dev);
gve_teardown_device_resources(priv);
- gve_adminq_free(priv);
+ priv->ctrl_ops->free_ctrl_plane(priv);
err = gve_init_priv(priv, true);
if (err != 0) {
@@ -967,7 +967,10 @@ gve_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
return -EBUSY;
}
- err = gve_adminq_set_mtu(priv, mtu);
+ if (priv->ctrl_ops->set_mtu == NULL)
+ return -ENOTSUP;
+
+ err = priv->ctrl_ops->set_mtu(priv, mtu);
if (err) {
PMD_DRV_LOG(ERR, "Failed to set mtu as %u err = %d", mtu, err);
return err;
@@ -1157,7 +1160,7 @@ gve_rss_hash_update(struct rte_eth_dev *dev,
memcpy(gve_rss_conf.indir, priv->rss_config.indir,
gve_rss_conf.indir_size * sizeof(*priv->rss_config.indir));
- err = gve_adminq_configure_rss(priv, &gve_rss_conf);
+ err = priv->ctrl_ops->configure_rss(priv, &gve_rss_conf);
if (!err)
gve_update_priv_rss_config(priv, &gve_rss_conf);
@@ -1233,7 +1236,7 @@ gve_rss_reta_update(struct rte_eth_dev *dev,
table_id++;
}
- err = gve_adminq_configure_rss(priv, &gve_rss_conf);
+ err = priv->ctrl_ops->configure_rss(priv, &gve_rss_conf);
if (err)
PMD_DRV_LOG(ERR, "Problem configuring RSS with device.");
else
@@ -1310,7 +1313,8 @@ gve_read_clock(struct rte_eth_dev *dev, uint64_t *clock)
return -EIO;
pthread_mutex_lock(&priv->nic_ts_lock);
- err = gve_adminq_report_nic_timestamp(priv, priv->nic_ts_report_mz->iova);
+
+ err = priv->ctrl_ops->report_nic_timestamp(priv, priv->nic_ts_report_mz->iova);
if (err != 0) {
pthread_mutex_unlock(&priv->nic_ts_lock);
return err;
@@ -1456,11 +1460,7 @@ gve_setup_device_resources(struct gve_priv *priv)
priv->irq_dbs = (struct gve_irq_db *)mz->addr;
priv->irq_dbs_mz = mz;
- err = gve_adminq_configure_device_resources(priv,
- priv->cnt_array_mz->iova,
- priv->num_event_counters,
- priv->irq_dbs_mz->iova,
- priv->num_ntfy_blks);
+ err = priv->ctrl_ops->get_interrupt_dbs(priv);
if (unlikely(err)) {
PMD_DRV_LOG(ERR, "Could not config device resources: err=%d", err);
goto free_irq_dbs;
@@ -1473,7 +1473,7 @@ gve_setup_device_resources(struct gve_priv *priv)
err = -ENOMEM;
goto free_irq_dbs;
}
- err = gve_adminq_get_ptype_map_dqo(priv, priv->ptype_lut_dqo);
+ err = priv->ctrl_ops->get_ptype_map(priv);
if (unlikely(err)) {
PMD_DRV_LOG(ERR, "Failed to get ptype map: err=%d", err);
goto free_ptype_lut;
@@ -1562,21 +1562,58 @@ gve_stop_dev_status_polling(struct rte_eth_dev *dev)
}
static int
-gve_init_priv(struct gve_priv *priv, bool skip_describe_device)
+gve_adminq_get_device_properties(struct gve_priv *priv)
{
- int num_ntfy;
int err;
- /* Set up the adminq */
- err = gve_adminq_alloc(priv);
+ err = gve_verify_driver_compatibility(priv);
if (err) {
- PMD_DRV_LOG(ERR, "Failed to alloc admin queue: err=%d", err);
+ PMD_DRV_LOG(ERR, "Could not verify driver compatibility: err=%d", err);
return err;
}
- err = gve_verify_driver_compatibility(priv);
+
+ /* Get max queues to alloc etherdev */
+ priv->max_nb_txq = ioread32be(&priv->reg_bar0->max_tx_queues);
+ priv->max_nb_rxq = ioread32be(&priv->reg_bar0->max_rx_queues);
+
+ return gve_adminq_describe_device(priv);
+}
+
+static const struct gve_ctrl_ops gve_adminq_ops = {
+ .init_ctrl_plane = gve_adminq_alloc,
+ .free_ctrl_plane = gve_adminq_free,
+ .get_device_properties = gve_adminq_get_device_properties,
+ .get_ptype_map = gve_adminq_get_ptype_map_dqo,
+ .get_interrupt_dbs = gve_adminq_configure_device_resources,
+ .create_tx_queues = gve_adminq_create_tx_queues,
+ .destroy_tx_queues = gve_adminq_destroy_tx_queues,
+ .create_rx_queues = gve_adminq_create_rx_queues,
+ .destroy_rx_queues = gve_adminq_destroy_rx_queues,
+ .report_link_speed = gve_adminq_report_link_speed,
+ .configure_rss = gve_adminq_configure_rss,
+ .add_flow_rule = gve_adminq_add_flow_rule,
+ .del_flow_rule = gve_adminq_del_flow_rule,
+ .reset_flow_rules = gve_adminq_reset_flow_rules,
+ .free_db_resources = gve_adminq_deconfigure_device_resources,
+ .setup_stats_report = gve_adminq_report_stats,
+ .report_nic_timestamp = gve_adminq_report_nic_timestamp,
+ .set_mtu = gve_adminq_set_mtu,
+ .register_page_list = gve_adminq_register_page_list,
+ .unregister_page_list = gve_adminq_unregister_page_list,
+};
+
+static int
+gve_init_priv(struct gve_priv *priv, bool skip_describe_device)
+{
+ int num_ntfy;
+ int err;
+
+ priv->ctrl_ops = &gve_adminq_ops;
+
+ err = priv->ctrl_ops->init_ctrl_plane(priv);
if (err) {
- PMD_DRV_LOG(ERR, "Could not verify driver compatibility: err=%d", err);
- goto free_adminq;
+ PMD_DRV_LOG(ERR, "Failed to alloc control plane: err=%d", err);
+ return err;
}
if (skip_describe_device)
@@ -1586,7 +1623,7 @@ gve_init_priv(struct gve_priv *priv, bool skip_describe_device)
gve_set_default_ring_size_bounds(priv);
/* Get the initial information we need from the device */
- err = gve_adminq_describe_device(priv);
+ err = priv->ctrl_ops->get_device_properties(priv);
if (err) {
PMD_DRV_LOG(ERR, "Could not get device information: err=%d", err);
goto free_adminq;
@@ -1636,7 +1673,7 @@ gve_init_priv(struct gve_priv *priv, bool skip_describe_device)
if (!err)
return 0;
free_adminq:
- gve_adminq_free(priv);
+ priv->ctrl_ops->free_ctrl_plane(priv);
return err;
}
@@ -1644,7 +1681,6 @@ static int
gve_dev_init(struct rte_eth_dev *eth_dev)
{
struct gve_priv *priv = eth_dev->data->dev_private;
- int max_tx_queues, max_rx_queues;
struct rte_pci_device *pci_dev;
struct gve_registers *reg_bar;
pthread_mutexattr_t mutexattr;
@@ -1679,18 +1715,12 @@ gve_dev_init(struct rte_eth_dev *eth_dev)
}
gve_write_version(®_bar->driver_version);
- /* Get max queues to alloc etherdev */
- max_tx_queues = ioread32be(®_bar->max_tx_queues);
- max_rx_queues = ioread32be(®_bar->max_rx_queues);
priv->reg_bar0 = reg_bar;
priv->db_bar2 = db_bar;
priv->pci_dev = pci_dev;
priv->state_flags = 0x0;
- priv->max_nb_txq = max_tx_queues;
- priv->max_nb_rxq = max_rx_queues;
-
pthread_mutexattr_init(&mutexattr);
pthread_mutexattr_setpshared(&mutexattr, PTHREAD_PROCESS_SHARED);
pthread_mutex_init(&priv->flow_rule_lock, &mutexattr);
diff --git a/drivers/net/gve/gve_ethdev.h b/drivers/net/gve/gve_ethdev.h
index c9a176ff17..f57e81dbc9 100644
--- a/drivers/net/gve/gve_ethdev.h
+++ b/drivers/net/gve/gve_ethdev.h
@@ -272,7 +272,52 @@ struct gve_flow {
extern const struct rte_flow_ops gve_flow_ops;
+struct gve_priv;
+struct gve_flow_rule_params;
+
+struct gve_ctrl_ops {
+ int (*init_ctrl_plane)(struct gve_priv *priv);
+ void (*free_ctrl_plane)(struct gve_priv *priv);
+ int (*get_device_properties)(struct gve_priv *priv);
+ int (*get_ptype_map)(struct gve_priv *priv);
+ int (*get_interrupt_dbs)(struct gve_priv *priv);
+ int (*create_tx_queues)(struct gve_priv *priv, uint32_t num_queues);
+ int (*destroy_tx_queues)(struct gve_priv *priv, uint32_t num_queues);
+ int (*create_rx_queues)(struct gve_priv *priv, uint32_t num_queues);
+ int (*destroy_rx_queues)(struct gve_priv *priv, uint32_t num_queues);
+ int (*report_link_speed)(struct gve_priv *priv);
+ int (*configure_rss)(struct gve_priv *priv,
+ struct gve_rss_config *rss_conf);
+ int (*add_flow_rule)(struct gve_priv *priv,
+ struct gve_flow_rule_params *rule, uint32_t loc);
+ int (*del_flow_rule)(struct gve_priv *priv, uint32_t loc);
+ int (*reset_flow_rules)(struct gve_priv *priv);
+
+ /*
+ * Optional control operations not supported by all control plane modes:
+ * - free_db_resources: AdminQ only
+ * - query_rss: Mailbox only
+ * - setup_stats_report: AdminQ only
+ * - report_nic_timestamp: AdminQ only (Mailbox uses MMIO clock read)
+ * - set_mtu: AdminQ only
+ * - register_page_list / unregister_page_list: AdminQ QPL mode only
+ */
+ int (*free_db_resources)(struct gve_priv *priv);
+ int (*query_rss)(struct gve_priv *priv);
+ int (*setup_stats_report)(struct gve_priv *priv,
+ uint64_t stats_report_len,
+ dma_addr_t stats_report_addr,
+ uint64_t interval_ms);
+ int (*report_nic_timestamp)(struct gve_priv *priv, dma_addr_t dma_addr);
+ int (*set_mtu)(struct gve_priv *priv, uint64_t mtu);
+ int (*register_page_list)(struct gve_priv *priv,
+ struct gve_queue_page_list *qpl);
+ int (*unregister_page_list)(struct gve_priv *priv,
+ uint32_t page_list_id);
+};
+
struct gve_priv {
+ const struct gve_ctrl_ops *ctrl_ops;
struct gve_irq_db *irq_dbs; /* array of num_ntfy_blks */
const struct rte_memzone *irq_dbs_mz;
uint32_t mgmt_msix_idx;
diff --git a/drivers/net/gve/gve_flow_rule.c b/drivers/net/gve/gve_flow_rule.c
index 1266e197ea..5959489791 100644
--- a/drivers/net/gve/gve_flow_rule.c
+++ b/drivers/net/gve/gve_flow_rule.c
@@ -450,7 +450,7 @@ gve_free_flow_rules(struct gve_priv *priv)
int err = 0;
if (!TAILQ_EMPTY(&priv->active_flows)) {
- err = gve_adminq_reset_flow_rules(priv);
+ err = priv->ctrl_ops->reset_flow_rules(priv);
if (err) {
PMD_DRV_LOG(ERR,
"Failed to reset flow rules, internal device err=%d",
@@ -516,7 +516,7 @@ gve_create_flow_rule(struct rte_eth_dev *dev,
goto free_flow_and_unlock;
}
- err = gve_adminq_add_flow_rule(priv, &rule, flow->rule_id);
+ err = priv->ctrl_ops->add_flow_rule(priv, &rule, flow->rule_id);
if (err) {
rte_bitmap_set(priv->avail_flow_rule_bmp, flow->rule_id);
rte_flow_error_set(error, -err,
@@ -588,7 +588,7 @@ gve_destroy_flow_rule(struct rte_eth_dev *dev, struct rte_flow *flow_handle,
goto unlock;
}
- err = gve_adminq_del_flow_rule(priv, flow->rule_id);
+ err = priv->ctrl_ops->del_flow_rule(priv, flow->rule_id);
if (err) {
rte_flow_error_set(error, -err,
RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
diff --git a/drivers/net/gve/gve_rx.c b/drivers/net/gve/gve_rx.c
index d14902c662..2f542c7a66 100644
--- a/drivers/net/gve/gve_rx.c
+++ b/drivers/net/gve/gve_rx.c
@@ -525,7 +525,7 @@ gve_stop_rx_queues(struct rte_eth_dev *dev)
if (!gve_is_gqi(hw))
return gve_stop_rx_queues_dqo(dev);
- err = gve_adminq_destroy_rx_queues(hw, dev->data->nb_rx_queues);
+ err = hw->ctrl_ops->destroy_rx_queues(hw, dev->data->nb_rx_queues);
if (err != 0)
PMD_DRV_LOG(WARNING, "failed to destroy rxqs");
diff --git a/drivers/net/gve/gve_rx_dqo.c b/drivers/net/gve/gve_rx_dqo.c
index 3665d9e4cd..81d7cacaf6 100644
--- a/drivers/net/gve/gve_rx_dqo.c
+++ b/drivers/net/gve/gve_rx_dqo.c
@@ -560,7 +560,7 @@ gve_stop_rx_queues_dqo(struct rte_eth_dev *dev)
uint16_t i;
int err;
- err = gve_adminq_destroy_rx_queues(hw, dev->data->nb_rx_queues);
+ err = hw->ctrl_ops->destroy_rx_queues(hw, dev->data->nb_rx_queues);
if (err != 0)
PMD_DRV_LOG(WARNING, "failed to destroy rxqs");
diff --git a/drivers/net/gve/gve_tx.c b/drivers/net/gve/gve_tx.c
index c0400b07bf..333e82b141 100644
--- a/drivers/net/gve/gve_tx.c
+++ b/drivers/net/gve/gve_tx.c
@@ -732,7 +732,7 @@ gve_stop_tx_queues(struct rte_eth_dev *dev)
if (!gve_is_gqi(hw))
return gve_stop_tx_queues_dqo(dev);
- err = gve_adminq_destroy_tx_queues(hw, dev->data->nb_tx_queues);
+ err = hw->ctrl_ops->destroy_tx_queues(hw, dev->data->nb_tx_queues);
if (err != 0)
PMD_DRV_LOG(WARNING, "failed to destroy txqs");
diff --git a/drivers/net/gve/gve_tx_dqo.c b/drivers/net/gve/gve_tx_dqo.c
index 5c03d3d1a2..a44a20b139 100644
--- a/drivers/net/gve/gve_tx_dqo.c
+++ b/drivers/net/gve/gve_tx_dqo.c
@@ -612,7 +612,7 @@ gve_stop_tx_queues_dqo(struct rte_eth_dev *dev)
uint16_t i;
int err;
- err = gve_adminq_destroy_tx_queues(hw, dev->data->nb_tx_queues);
+ err = hw->ctrl_ops->destroy_tx_queues(hw, dev->data->nb_tx_queues);
if (err != 0)
PMD_DRV_LOG(WARNING, "failed to destroy txqs");
--
2.56.0.rc1.315.gc6ed9934b7-goog
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v2 2/8] net/gve: rename doorbell BAR variable to be more generic
2026-10-05 19:39 ` [PATCH v2 " Joshua Washington
2026-10-05 19:39 ` [PATCH v2 1/8] net/gve: refactor ethdev for control ops interface Joshua Washington
@ 2026-10-05 19:39 ` Joshua Washington
2026-10-05 19:39 ` [PATCH v2 3/8] net/gve: split default queue counts into Tx and Rx Joshua Washington
` (7 subsequent siblings)
9 siblings, 0 replies; 22+ messages in thread
From: Joshua Washington @ 2026-10-05 19:39 UTC (permalink / raw)
To: Joshua Washington; +Cc: dev, Mark Blasko
From: Mark Blasko <blasko@google.com>
Rename db_bar2 to db_bar in struct gve_priv and update queue doorbell
references across the driver. The doorbells will be in BAR 0 in GVE
Mailbox mode.
Signed-off-by: Mark Blasko <blasko@google.com>
Signed-off-by: Joshua Washington <joshwash@google.com>
---
drivers/net/gve/gve_ethdev.c | 2 +-
drivers/net/gve/gve_ethdev.h | 2 +-
drivers/net/gve/gve_rx.c | 4 ++--
drivers/net/gve/gve_rx_dqo.c | 4 ++--
drivers/net/gve/gve_tx.c | 4 ++--
drivers/net/gve/gve_tx_dqo.c | 4 ++--
6 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c
index 6704cc40b3..9cfdd03b9d 100644
--- a/drivers/net/gve/gve_ethdev.c
+++ b/drivers/net/gve/gve_ethdev.c
@@ -1717,7 +1717,7 @@ gve_dev_init(struct rte_eth_dev *eth_dev)
gve_write_version(®_bar->driver_version);
priv->reg_bar0 = reg_bar;
- priv->db_bar2 = db_bar;
+ priv->db_bar = db_bar;
priv->pci_dev = pci_dev;
priv->state_flags = 0x0;
diff --git a/drivers/net/gve/gve_ethdev.h b/drivers/net/gve/gve_ethdev.h
index f57e81dbc9..ef54b75c32 100644
--- a/drivers/net/gve/gve_ethdev.h
+++ b/drivers/net/gve/gve_ethdev.h
@@ -353,7 +353,7 @@ struct gve_priv {
uint32_t num_ntfy_blks; /* spilt between TX and RX so must be even */
struct gve_registers __iomem *reg_bar0; /* see gve_register.h */
- rte_be32_t __iomem *db_bar2; /* "array" of doorbells */
+ rte_be32_t __iomem *db_bar; /* "array" of doorbells */
struct rte_pci_device *pci_dev;
/* Admin queue - see gve_adminq.h*/
diff --git a/drivers/net/gve/gve_rx.c b/drivers/net/gve/gve_rx.c
index 2f542c7a66..ac2d174c67 100644
--- a/drivers/net/gve/gve_rx.c
+++ b/drivers/net/gve/gve_rx.c
@@ -349,7 +349,7 @@ gve_rx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_id,
rxq->is_gqi_qpl = hw->queue_format == GVE_GQI_QPL_FORMAT;
rxq->mpool = pool;
rxq->hw = hw;
- rxq->ntfy_addr = &hw->db_bar2[rte_be_to_cpu_32(hw->irq_dbs[rxq->ntfy_id].id)];
+ rxq->ntfy_addr = &hw->db_bar[rte_be_to_cpu_32(hw->irq_dbs[rxq->ntfy_id].id)];
mbuf_len =
rte_pktmbuf_data_room_size(rxq->mpool) - RTE_PKTMBUF_HEADROOM;
@@ -483,7 +483,7 @@ gve_rx_queue_start(struct rte_eth_dev *dev, uint16_t rx_queue_id)
rxq = dev->data->rx_queues[rx_queue_id];
- rxq->qrx_tail = &hw->db_bar2[rte_be_to_cpu_32(rxq->qres->db_index)];
+ rxq->qrx_tail = &hw->db_bar[rte_be_to_cpu_32(rxq->qres->db_index)];
rte_write32(rte_cpu_to_be_32(GVE_IRQ_MASK), rxq->ntfy_addr);
diff --git a/drivers/net/gve/gve_rx_dqo.c b/drivers/net/gve/gve_rx_dqo.c
index 81d7cacaf6..1fbe875954 100644
--- a/drivers/net/gve/gve_rx_dqo.c
+++ b/drivers/net/gve/gve_rx_dqo.c
@@ -370,7 +370,7 @@ gve_rx_queue_setup_dqo(struct rte_eth_dev *dev, uint16_t queue_id,
rxq->mpool = pool;
rxq->hw = hw;
- rxq->ntfy_addr = &hw->db_bar2[rte_be_to_cpu_32(hw->irq_dbs[rxq->ntfy_id].id)];
+ rxq->ntfy_addr = &hw->db_bar[rte_be_to_cpu_32(hw->irq_dbs[rxq->ntfy_id].id)];
mbuf_len =
rte_pktmbuf_data_room_size(rxq->mpool) - RTE_PKTMBUF_HEADROOM;
@@ -519,7 +519,7 @@ gve_rx_queue_start_dqo(struct rte_eth_dev *dev, uint16_t rx_queue_id)
rxq = dev->data->rx_queues[rx_queue_id];
- rxq->qrx_tail = &hw->db_bar2[rte_be_to_cpu_32(rxq->qres->db_index)];
+ rxq->qrx_tail = &hw->db_bar[rte_be_to_cpu_32(rxq->qres->db_index)];
rte_write32(rte_cpu_to_le_32(GVE_NO_INT_MODE_DQO |
GVE_ITR_NO_UPDATE_DQO),
diff --git a/drivers/net/gve/gve_tx.c b/drivers/net/gve/gve_tx.c
index 333e82b141..6398191b0a 100644
--- a/drivers/net/gve/gve_tx.c
+++ b/drivers/net/gve/gve_tx.c
@@ -606,7 +606,7 @@ gve_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_id, uint16_t nb_desc,
txq->ntfy_id = queue_id;
txq->is_gqi_qpl = hw->queue_format == GVE_GQI_QPL_FORMAT;
txq->hw = hw;
- txq->ntfy_addr = &hw->db_bar2[rte_be_to_cpu_32(hw->irq_dbs[txq->ntfy_id].id)];
+ txq->ntfy_addr = &hw->db_bar[rte_be_to_cpu_32(hw->irq_dbs[txq->ntfy_id].id)];
/* Allocate software ring */
txq->sw_ring = rte_zmalloc_socket("gve tx sw ring",
@@ -694,7 +694,7 @@ gve_tx_queue_start(struct rte_eth_dev *dev, uint16_t tx_queue_id)
txq = dev->data->tx_queues[tx_queue_id];
- txq->qtx_tail = &hw->db_bar2[rte_be_to_cpu_32(txq->qres->db_index)];
+ txq->qtx_tail = &hw->db_bar[rte_be_to_cpu_32(txq->qres->db_index)];
txq->qtx_head =
&hw->cnt_array[rte_be_to_cpu_32(txq->qres->counter_index)];
diff --git a/drivers/net/gve/gve_tx_dqo.c b/drivers/net/gve/gve_tx_dqo.c
index a44a20b139..fe8b883208 100644
--- a/drivers/net/gve/gve_tx_dqo.c
+++ b/drivers/net/gve/gve_tx_dqo.c
@@ -495,7 +495,7 @@ gve_tx_queue_setup_dqo(struct rte_eth_dev *dev, uint16_t queue_id,
txq->port_id = dev->data->port_id;
txq->ntfy_id = queue_id;
txq->hw = hw;
- txq->ntfy_addr = &hw->db_bar2[rte_be_to_cpu_32(hw->irq_dbs[txq->ntfy_id].id)];
+ txq->ntfy_addr = &hw->db_bar[rte_be_to_cpu_32(hw->irq_dbs[txq->ntfy_id].id)];
/* Allocate software ring */
sw_size = nb_desc;
@@ -575,7 +575,7 @@ gve_tx_queue_start_dqo(struct rte_eth_dev *dev, uint16_t tx_queue_id)
txq = dev->data->tx_queues[tx_queue_id];
- txq->qtx_tail = &hw->db_bar2[rte_be_to_cpu_32(txq->qres->db_index)];
+ txq->qtx_tail = &hw->db_bar[rte_be_to_cpu_32(txq->qres->db_index)];
txq->qtx_head =
&hw->cnt_array[rte_be_to_cpu_32(txq->qres->counter_index)];
--
2.56.0.rc1.315.gc6ed9934b7-goog
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v2 3/8] net/gve: split default queue counts into Tx and Rx
2026-10-05 19:39 ` [PATCH v2 " Joshua Washington
2026-10-05 19:39 ` [PATCH v2 1/8] net/gve: refactor ethdev for control ops interface Joshua Washington
2026-10-05 19:39 ` [PATCH v2 2/8] net/gve: rename doorbell BAR variable to be more generic Joshua Washington
@ 2026-10-05 19:39 ` Joshua Washington
2026-10-05 19:39 ` [PATCH v2 4/8] net/gve: clean Tx queue directly in dqo Joshua Washington
` (6 subsequent siblings)
9 siblings, 0 replies; 22+ messages in thread
From: Joshua Washington @ 2026-10-05 19:39 UTC (permalink / raw)
To: Joshua Washington; +Cc: dev, Mark Blasko
From: Mark Blasko <blasko@google.com>
Replace default_num_queues with separate default_tx_num_queues and
default_rx_num_queues fields in struct gve_priv. Update AdminQ device
description and queue initialization logic to use the per-direction
default queue counts.
Signed-off-by: Mark Blasko <blasko@google.com>
Signed-off-by: Joshua Washington <joshwash@google.com>
---
drivers/net/gve/base/gve_adminq.c | 4 ++--
drivers/net/gve/gve_ethdev.c | 8 ++++----
drivers/net/gve/gve_ethdev.h | 3 ++-
3 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/net/gve/base/gve_adminq.c b/drivers/net/gve/base/gve_adminq.c
index 74b9a0125c..4dd095880d 100644
--- a/drivers/net/gve/base/gve_adminq.c
+++ b/drivers/net/gve/base/gve_adminq.c
@@ -1083,8 +1083,8 @@ int gve_adminq_describe_device(struct gve_priv *priv)
PMD_DRV_LOG(INFO, "MAC addr: " RTE_ETHER_ADDR_PRT_FMT,
RTE_ETHER_ADDR_BYTES(&priv->dev_addr));
priv->tx_pages_per_qpl = be16_to_cpu(descriptor->tx_pages_per_qpl);
-
- priv->default_num_queues = be16_to_cpu(descriptor->default_num_queues);
+ priv->default_tx_num_queues = be16_to_cpu(descriptor->default_num_queues);
+ priv->default_rx_num_queues = be16_to_cpu(descriptor->default_num_queues);
gve_enable_supported_features(priv, supported_features_mask,
dev_op_flow_steering, dev_op_modify_ring,
diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c
index 9cfdd03b9d..3ba99fd74c 100644
--- a/drivers/net/gve/gve_ethdev.c
+++ b/drivers/net/gve/gve_ethdev.c
@@ -1652,10 +1652,10 @@ gve_init_priv(struct gve_priv *priv, bool skip_describe_device)
priv->max_nb_txq = RTE_MIN(priv->max_nb_txq, priv->num_ntfy_blks / 2);
priv->max_nb_rxq = RTE_MIN(priv->max_nb_rxq, priv->num_ntfy_blks / 2);
- if (priv->default_num_queues > 0) {
- priv->max_nb_txq = RTE_MIN(priv->default_num_queues, priv->max_nb_txq);
- priv->max_nb_rxq = RTE_MIN(priv->default_num_queues, priv->max_nb_rxq);
- }
+ if (priv->default_tx_num_queues > 0)
+ priv->max_nb_txq = RTE_MIN(priv->default_tx_num_queues, priv->max_nb_txq);
+ if (priv->default_rx_num_queues > 0)
+ priv->max_nb_rxq = RTE_MIN(priv->default_rx_num_queues, priv->max_nb_rxq);
PMD_DRV_LOG(INFO, "Max TX queues %d, Max RX queues %d",
priv->max_nb_txq, priv->max_nb_rxq);
diff --git a/drivers/net/gve/gve_ethdev.h b/drivers/net/gve/gve_ethdev.h
index ef54b75c32..1a0ac87ed8 100644
--- a/drivers/net/gve/gve_ethdev.h
+++ b/drivers/net/gve/gve_ethdev.h
@@ -344,7 +344,8 @@ struct gve_priv {
uint64_t max_registered_pages;
uint64_t num_registered_pages; /* num pages registered with NIC */
- uint16_t default_num_queues; /* default num queues to set up */
+ uint16_t default_tx_num_queues; /* default num tx queues to set up */
+ uint16_t default_rx_num_queues; /* default num rx queues to set up */
enum gve_queue_format queue_format; /* see enum gve_queue_format */
uint8_t enable_rsc;
--
2.56.0.rc1.315.gc6ed9934b7-goog
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v2 4/8] net/gve: clean Tx queue directly in dqo
2026-10-05 19:39 ` [PATCH v2 " Joshua Washington
` (2 preceding siblings ...)
2026-10-05 19:39 ` [PATCH v2 3/8] net/gve: split default queue counts into Tx and Rx Joshua Washington
@ 2026-10-05 19:39 ` Joshua Washington
2026-10-05 19:39 ` [PATCH v2 5/8] net/gve: update RSS config just after successful programming Joshua Washington
` (5 subsequent siblings)
9 siblings, 0 replies; 22+ messages in thread
From: Joshua Washington @ 2026-10-05 19:39 UTC (permalink / raw)
To: Joshua Washington; +Cc: dev, Mark Blasko
From: Mark Blasko <blasko@google.com>
Each Tx completion queue in DPDK is mapped 1:1 to its Tx queue. Remove
the compl_desc id lookup and txqs pointer array, and process descriptor
and packet completions directly on the calling Tx queue.
Signed-off-by: Mark Blasko <blasko@google.com>
Signed-off-by: Joshua Washington <joshwash@google.com>
---
drivers/net/gve/gve_ethdev.h | 1 -
drivers/net/gve/gve_tx_dqo.c | 14 +++++---------
2 files changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/net/gve/gve_ethdev.h b/drivers/net/gve/gve_ethdev.h
index 1a0ac87ed8..9d8939da33 100644
--- a/drivers/net/gve/gve_ethdev.h
+++ b/drivers/net/gve/gve_ethdev.h
@@ -193,7 +193,6 @@ struct gve_tx_queue {
uint16_t sw_size;
uint8_t cur_gen_bit;
uint32_t last_desc_cleaned;
- void **txqs;
uint16_t re_cnt;
/* Only valid for DQO_RDA queue format */
diff --git a/drivers/net/gve/gve_tx_dqo.c b/drivers/net/gve/gve_tx_dqo.c
index fe8b883208..7dbe8db39d 100644
--- a/drivers/net/gve/gve_tx_dqo.c
+++ b/drivers/net/gve/gve_tx_dqo.c
@@ -55,7 +55,6 @@ gve_tx_clean_dqo(struct gve_tx_queue *txq)
{
struct gve_tx_compl_desc *compl_ring;
struct gve_tx_compl_desc *compl_desc;
- struct gve_tx_queue *aim_txq;
struct gve_tx_pkt *pkt;
uint16_t new_tx_head;
uint16_t compl_tag;
@@ -70,15 +69,13 @@ gve_tx_clean_dqo(struct gve_tx_queue *txq)
rte_io_rmb();
- aim_txq = txq->txqs[compl_desc->id];
-
switch (compl_desc->type) {
case GVE_COMPL_TYPE_DQO_DESC:
new_tx_head = rte_le_to_cpu_16(compl_desc->tx_head);
- aim_txq->nb_free +=
- (new_tx_head - aim_txq->last_desc_cleaned)
- & (aim_txq->nb_tx_desc - 1);
- aim_txq->last_desc_cleaned = new_tx_head;
+ txq->nb_free +=
+ (new_tx_head - txq->last_desc_cleaned)
+ & (txq->nb_tx_desc - 1);
+ txq->last_desc_cleaned = new_tx_head;
break;
case GVE_COMPL_TYPE_DQO_REINJECTION:
PMD_DRV_DP_LOG(DEBUG, "GVE_COMPL_TYPE_DQO_REINJECTION !!!");
@@ -93,7 +90,7 @@ gve_tx_clean_dqo(struct gve_tx_queue *txq)
}
/* Free packet.*/
- pkt = &aim_txq->pkt_ring_dqo[compl_tag];
+ pkt = &txq->pkt_ring_dqo[compl_tag];
if (unlikely(!pkt->mbuf)) {
PMD_DRV_DP_LOG(ERR, "No outstanding packet for completion tag %d",
compl_tag);
@@ -534,7 +531,6 @@ gve_tx_queue_setup_dqo(struct rte_eth_dev *dev, uint16_t queue_id,
txq->compl_ring = (struct gve_tx_compl_desc *)mz->addr;
txq->compl_ring_phys_addr = mz->iova;
txq->compl_ring_mz = mz;
- txq->txqs = dev->data->tx_queues;
mz = rte_eth_dma_zone_reserve(dev, "txq_res", queue_id,
sizeof(struct gve_queue_resources),
--
2.56.0.rc1.315.gc6ed9934b7-goog
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v2 5/8] net/gve: update RSS config just after successful programming
2026-10-05 19:39 ` [PATCH v2 " Joshua Washington
` (3 preceding siblings ...)
2026-10-05 19:39 ` [PATCH v2 4/8] net/gve: clean Tx queue directly in dqo Joshua Washington
@ 2026-10-05 19:39 ` Joshua Washington
2026-10-05 19:39 ` [PATCH v2 6/8] net/gve: add RSS cache boolean flag Joshua Washington
` (4 subsequent siblings)
9 siblings, 0 replies; 22+ messages in thread
From: Joshua Washington @ 2026-10-05 19:39 UTC (permalink / raw)
To: Joshua Washington; +Cc: dev
Moving this into the control op allows for better flexibility when
introducing the mailbox op, which will include support for a query
operation.
Signed-off-by: Joshua Washington <joshwash@google.com>
---
drivers/net/gve/base/gve_adminq.c | 5 +++++
drivers/net/gve/gve_ethdev.c | 6 ------
2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/net/gve/base/gve_adminq.c b/drivers/net/gve/base/gve_adminq.c
index 4dd095880d..db3e43c6b0 100644
--- a/drivers/net/gve/base/gve_adminq.c
+++ b/drivers/net/gve/base/gve_adminq.c
@@ -3,9 +3,12 @@
* Copyright (C) 2015-2022 Google, Inc.
*/
+#include <rte_common.h>
+
#include "../gve_ethdev.h"
#include "gve_adminq.h"
#include "gve_register.h"
+#include "gve_rss.h"
#define GVE_MAX_ADMINQ_RELEASE_CHECK 500
#define GVE_ADMINQ_SLEEP_LEN 20
@@ -1275,6 +1278,8 @@ int gve_adminq_configure_rss(struct gve_priv *priv,
};
err = gve_adminq_execute_cmd(priv, &cmd);
+ if (err == 0)
+ gve_update_priv_rss_config(priv, rss_config);
out:
if (indir)
diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c
index 3ba99fd74c..49604f9362 100644
--- a/drivers/net/gve/gve_ethdev.c
+++ b/drivers/net/gve/gve_ethdev.c
@@ -243,8 +243,6 @@ gve_dev_configure(struct rte_eth_dev *dev)
if (err)
PMD_DRV_LOG(ERR,
"Could not reconfigure RSS redirection table.");
- else
- gve_update_priv_rss_config(priv, &update_reta_config);
gve_free_rss_config(&update_reta_config);
return err;
@@ -1161,8 +1159,6 @@ gve_rss_hash_update(struct rte_eth_dev *dev,
gve_rss_conf.indir_size * sizeof(*priv->rss_config.indir));
err = priv->ctrl_ops->configure_rss(priv, &gve_rss_conf);
- if (!err)
- gve_update_priv_rss_config(priv, &gve_rss_conf);
err:
gve_free_rss_config(&gve_rss_conf);
@@ -1239,8 +1235,6 @@ gve_rss_reta_update(struct rte_eth_dev *dev,
err = priv->ctrl_ops->configure_rss(priv, &gve_rss_conf);
if (err)
PMD_DRV_LOG(ERR, "Problem configuring RSS with device.");
- else
- gve_update_priv_rss_config(priv, &gve_rss_conf);
gve_free_rss_config(&gve_rss_conf);
return err;
--
2.56.0.rc1.315.gc6ed9934b7-goog
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v2 6/8] net/gve: add RSS cache boolean flag
2026-10-05 19:39 ` [PATCH v2 " Joshua Washington
` (4 preceding siblings ...)
2026-10-05 19:39 ` [PATCH v2 5/8] net/gve: update RSS config just after successful programming Joshua Washington
@ 2026-10-05 19:39 ` Joshua Washington
2026-10-05 19:39 ` [PATCH v2 7/8] net/gve: fix RSS config memory leak on close Joshua Washington
` (3 subsequent siblings)
9 siblings, 0 replies; 22+ messages in thread
From: Joshua Washington @ 2026-10-05 19:39 UTC (permalink / raw)
To: Joshua Washington; +Cc: dev
Introduce a flag that determines whether the RSS configuration stored in
priv is the latest from the device or not. When sending a control plane
message to configure RSS, the cache will be invalidated. Upon storing
the latest configuration in priv, the cache will be considered clean and
ready to be read.
The RSS configuration should never be read if the cache is dirty.
Signed-off-by: Joshua Washington <joshwash@google.com>
---
v2:
- Remove -ENODATA return for Freebsd compatibility.
---
drivers/net/gve/base/gve_adminq.c | 1 +
drivers/net/gve/gve_ethdev.c | 76 ++++++++++++++++++++++++-------
drivers/net/gve/gve_ethdev.h | 1 +
| 2 +
4 files changed, 64 insertions(+), 16 deletions(-)
diff --git a/drivers/net/gve/base/gve_adminq.c b/drivers/net/gve/base/gve_adminq.c
index db3e43c6b0..60e8544ef4 100644
--- a/drivers/net/gve/base/gve_adminq.c
+++ b/drivers/net/gve/base/gve_adminq.c
@@ -1278,6 +1278,7 @@ int gve_adminq_configure_rss(struct gve_priv *priv,
};
err = gve_adminq_execute_cmd(priv, &cmd);
+ priv->rss_cache_dirty = true;
if (err == 0)
gve_update_priv_rss_config(priv, rss_config);
diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c
index 49604f9362..22f7d30a6a 100644
--- a/drivers/net/gve/gve_ethdev.c
+++ b/drivers/net/gve/gve_ethdev.c
@@ -210,6 +210,28 @@ gve_teardown_queue_page_list(struct gve_priv *priv,
return 0;
}
+static int
+gve_rss_update_cache(struct gve_priv *priv)
+{
+ if (priv->rss_cache_dirty) {
+ int err;
+
+ if (priv->ctrl_ops->query_rss == NULL) {
+ PMD_DRV_LOG(ERR,
+ "No RSS query functionality present in ops table");
+ return -ENOENT;
+ }
+
+ err = priv->ctrl_ops->query_rss(priv);
+ if (err) {
+ PMD_DRV_LOG(ERR, "Failed to query device for RSS info.");
+ return err;
+ }
+ }
+
+ return 0;
+}
+
static int
gve_dev_configure(struct rte_eth_dev *dev)
{
@@ -233,19 +255,24 @@ gve_dev_configure(struct rte_eth_dev *dev)
}
}
- /* Reset RSS RETA in case number of queues changed. */
+ /* Reset RSS RETA in case number of queues changed, but don't fail
+ * configure if the cache cannot be updated.
+ */
if (priv->rss_config.indir) {
- struct gve_rss_config update_reta_config;
- gve_init_rss_config_from_priv(priv, &update_reta_config);
- gve_generate_rss_reta(dev, &update_reta_config);
-
- err = priv->ctrl_ops->configure_rss(priv, &update_reta_config);
- if (err)
- PMD_DRV_LOG(ERR,
- "Could not reconfigure RSS redirection table.");
+ err = gve_rss_update_cache(priv);
+ if (err == 0) {
+ struct gve_rss_config update_reta_config;
+ gve_init_rss_config_from_priv(priv, &update_reta_config);
+ gve_generate_rss_reta(dev, &update_reta_config);
+
+ err = priv->ctrl_ops->configure_rss(priv, &update_reta_config);
+ if (err)
+ PMD_DRV_LOG(ERR,
+ "Could not reconfigure RSS redirection table.");
- gve_free_rss_config(&update_reta_config);
- return err;
+ gve_free_rss_config(&update_reta_config);
+ return err;
+ }
}
return 0;
@@ -1093,7 +1120,6 @@ gve_xstats_get_names(struct rte_eth_dev *dev,
return count;
}
-
static int
gve_rss_hash_update(struct rte_eth_dev *dev,
struct rte_eth_rss_conf *rss_conf)
@@ -1143,6 +1169,10 @@ gve_rss_hash_update(struct rte_eth_dev *dev,
if (err)
return err;
+ err = gve_rss_update_cache(priv);
+ if (err)
+ goto err;
+
gve_rss_conf.alg = GVE_RSS_HASH_TOEPLITZ;
err = gve_update_rss_hash_types(priv, &gve_rss_conf, rss_conf);
if (err)
@@ -1170,6 +1200,7 @@ gve_rss_hash_conf_get(struct rte_eth_dev *dev,
struct rte_eth_rss_conf *rss_conf)
{
struct gve_priv *priv = dev->data->dev_private;
+ int err;
if (!(dev->data->dev_conf.rxmode.offloads &
RTE_ETH_RX_OFFLOAD_RSS_HASH)) {
@@ -1177,6 +1208,9 @@ gve_rss_hash_conf_get(struct rte_eth_dev *dev,
return -ENOTSUP;
}
+ err = gve_rss_update_cache(priv);
+ if (err)
+ return err;
gve_to_rte_rss_hf(priv->rss_config.hash_types, rss_conf);
rss_conf->rss_key_len = priv->rss_config.key_size;
@@ -1202,6 +1236,10 @@ gve_rss_reta_update(struct rte_eth_dev *dev,
int err;
int i;
+ err = gve_rss_update_cache(priv);
+ if (err)
+ return err;
+
/* RSS key must be set before the redirection table can be set. */
if (!priv->rss_config.key || priv->rss_config.key_size == 0) {
PMD_DRV_LOG(ERR, "RSS hash key msut be set before the "
@@ -1246,6 +1284,7 @@ gve_rss_reta_query(struct rte_eth_dev *dev,
{
struct gve_priv *priv = dev->data->dev_private;
int table_id;
+ int err;
int i;
if (!(dev->data->dev_conf.rxmode.offloads &
@@ -1254,10 +1293,13 @@ gve_rss_reta_query(struct rte_eth_dev *dev,
return -ENOTSUP;
}
- /* RSS key must be set before the redirection table can be queried. */
- if (!priv->rss_config.key) {
- PMD_DRV_LOG(ERR, "RSS hash key must be set before the "
- "redirection table can be initialized.");
+ err = gve_rss_update_cache(priv);
+ if (err != 0)
+ return err;
+
+ if (priv->ctrl_ops->query_rss == NULL && priv->rss_config.key == NULL) {
+ /* RSS key must be set before the redirection table can be queried. */
+ PMD_DRV_LOG(ERR, "RSS hash key must be set before the redirection table can be initialized.");
return -ENOTSUP;
}
@@ -1655,6 +1697,8 @@ gve_init_priv(struct gve_priv *priv, bool skip_describe_device)
priv->max_nb_txq, priv->max_nb_rxq);
setup_device:
+ if (priv->ctrl_ops->query_rss != NULL)
+ priv->rss_cache_dirty = true;
if (priv->max_flow_rules) {
err = gve_setup_flow_subsystem(priv);
if (err)
diff --git a/drivers/net/gve/gve_ethdev.h b/drivers/net/gve/gve_ethdev.h
index 9d8939da33..ef93db0c1d 100644
--- a/drivers/net/gve/gve_ethdev.h
+++ b/drivers/net/gve/gve_ethdev.h
@@ -399,6 +399,7 @@ struct gve_priv {
uint16_t stats_end_idx; /* end index of array of stats written by NIC */
struct gve_rss_config rss_config;
+ bool rss_cache_dirty;
struct gve_ptype_lut *ptype_lut_dqo;
/* Flow rule management */
--git a/drivers/net/gve/gve_rss.c b/drivers/net/gve/gve_rss.c
index 67b910ed22..5cd65f2df9 100644
--- a/drivers/net/gve/gve_rss.c
+++ b/drivers/net/gve/gve_rss.c
@@ -120,6 +120,8 @@ gve_update_priv_rss_config(struct gve_priv *priv, struct gve_rss_config *config)
memcpy(priv_config->key, config->key,
config->key_size * sizeof(*priv_config->key));
+ priv->rss_cache_dirty = false;
+
return 0;
}
--
2.56.0.rc1.315.gc6ed9934b7-goog
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v2 7/8] net/gve: fix RSS config memory leak on close
2026-10-05 19:39 ` [PATCH v2 " Joshua Washington
` (5 preceding siblings ...)
2026-10-05 19:39 ` [PATCH v2 6/8] net/gve: add RSS cache boolean flag Joshua Washington
@ 2026-10-05 19:39 ` Joshua Washington
2026-10-05 19:39 ` [PATCH v2 8/8] net/gve: refactor timestamp support to clock read type Joshua Washington
` (2 subsequent siblings)
9 siblings, 0 replies; 22+ messages in thread
From: Joshua Washington @ 2026-10-05 19:39 UTC (permalink / raw)
To: Joshua Washington, Jasper Tran O'Leary; +Cc: dev, stable
From: Jasper Tran O'Leary <jtranoleary@google.com>
Free rss_config resources during device close to avoid leaking
redirection table and hash key memory.
Fixes: 7ba84453bacf ("net/gve: free device resources on close")
Cc: stable@dpdk.org
Signed-off-by: Jasper Tran O'Leary <jtranoleary@google.com>
Signed-off-by: Joshua Washington <joshwash@google.com>
---
drivers/net/gve/gve_ethdev.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c
index 22f7d30a6a..aa8beed3c4 100644
--- a/drivers/net/gve/gve_ethdev.c
+++ b/drivers/net/gve/gve_ethdev.c
@@ -724,6 +724,7 @@ gve_teardown_device_resources(struct gve_priv *priv)
gve_free_ptype_lut_dqo(priv);
gve_free_counter_array(priv);
gve_free_irq_db(priv);
+ gve_free_rss_config(&priv->rss_config);
gve_clear_device_resources_ok(priv);
}
--
2.56.0.rc1.315.gc6ed9934b7-goog
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v2 8/8] net/gve: refactor timestamp support to clock read type
2026-10-05 19:39 ` [PATCH v2 " Joshua Washington
` (6 preceding siblings ...)
2026-10-05 19:39 ` [PATCH v2 7/8] net/gve: fix RSS config memory leak on close Joshua Washington
@ 2026-10-05 19:39 ` Joshua Washington
2026-10-06 14:19 ` [PATCH v2 0/8] gve: precursor control plane changes before bare-metal support Stephen Hemminger
2026-10-06 15:10 ` Stephen Hemminger
9 siblings, 0 replies; 22+ messages in thread
From: Joshua Washington @ 2026-10-05 19:39 UTC (permalink / raw)
To: Joshua Washington; +Cc: dev, Mark Blasko
From: Mark Blasko <blasko@google.com>
Replace the boolean nic_timestamp_supported field with enum
gve_dev_clk_type to represent the device clock read mode
(unsupported, Cmd). Update AdminQ capability negotiation
and ethdev operations to check clk_read_type.
Signed-off-by: Mark Blasko <blasko@google.com>
Signed-off-by: Joshua Washington <joshwash@google.com>
---
drivers/net/gve/base/gve_adminq.c | 5 +++--
drivers/net/gve/gve_ethdev.c | 9 ++++++---
drivers/net/gve/gve_ethdev.h | 7 ++++++-
3 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/net/gve/base/gve_adminq.c b/drivers/net/gve/base/gve_adminq.c
index 60e8544ef4..6fd9323f0c 100644
--- a/drivers/net/gve/base/gve_adminq.c
+++ b/drivers/net/gve/base/gve_adminq.c
@@ -994,9 +994,10 @@ static void gve_enable_supported_features(struct gve_priv *priv,
priv->max_mtu = be16_to_cpu(dev_op_jumbo_frames->max_mtu);
}
if (dev_op_nic_timestamp &&
- (supported_features_mask & GVE_SUP_NIC_TIMESTAMP_MASK)) {
+ (supported_features_mask & GVE_SUP_NIC_TIMESTAMP_MASK) &&
+ !gve_is_gqi(priv)) {
PMD_DRV_LOG(INFO, "NIC TIMESTAMP device option enabled.");
- priv->nic_timestamp_supported = true;
+ priv->clk_read_type = GVE_DEV_CLK_CMD;
}
}
diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c
index aa8beed3c4..b4a5f59816 100644
--- a/drivers/net/gve/gve_ethdev.c
+++ b/drivers/net/gve/gve_ethdev.c
@@ -555,6 +555,9 @@ gve_alloc_nic_ts_report(struct gve_priv *priv)
{
char z_name[RTE_MEMZONE_NAMESIZE];
+ if (priv->clk_read_type == GVE_DEV_CLK_UNSUPPORTED)
+ return -EOPNOTSUPP;
+
snprintf(z_name, sizeof(z_name), "gve_%s_nic_ts_report",
priv->pci_dev->device.name);
priv->nic_ts_report_mz = rte_memzone_reserve_aligned(z_name,
@@ -866,7 +869,7 @@ gve_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
dev_info->min_mtu = RTE_ETHER_MIN_MTU;
dev_info->rx_offload_capa = RTE_ETH_RX_OFFLOAD_RSS_HASH;
- if (!gve_is_gqi(priv) && priv->nic_ts_report_mz)
+ if (priv->clk_read_type != GVE_DEV_CLK_UNSUPPORTED)
dev_info->rx_offload_capa |= RTE_ETH_RX_OFFLOAD_TIMESTAMP;
dev_info->tx_offload_capa =
RTE_ETH_TX_OFFLOAD_MULTI_SEGS |
@@ -1343,7 +1346,7 @@ gve_read_clock(struct rte_eth_dev *dev, uint64_t *clock)
uint64_t ts;
int err;
- if (!priv->nic_timestamp_supported)
+ if (priv->clk_read_type == GVE_DEV_CLK_UNSUPPORTED)
return -EOPNOTSUPP;
if (!priv->nic_ts_report_mz)
@@ -1444,7 +1447,7 @@ gve_setup_nic_timestamp(struct gve_priv *priv)
{
int err;
- if (!priv->nic_timestamp_supported)
+ if (priv->clk_read_type == GVE_DEV_CLK_UNSUPPORTED)
return;
rte_atomic_store_explicit(&priv->nic_ts_read_fails, 0, rte_memory_order_relaxed);
diff --git a/drivers/net/gve/gve_ethdev.h b/drivers/net/gve/gve_ethdev.h
index ef93db0c1d..21454802f9 100644
--- a/drivers/net/gve/gve_ethdev.h
+++ b/drivers/net/gve/gve_ethdev.h
@@ -274,6 +274,11 @@ extern const struct rte_flow_ops gve_flow_ops;
struct gve_priv;
struct gve_flow_rule_params;
+enum gve_dev_clk_type {
+ GVE_DEV_CLK_UNSUPPORTED = 0,
+ GVE_DEV_CLK_CMD,
+};
+
struct gve_ctrl_ops {
int (*init_ctrl_plane)(struct gve_priv *priv);
void (*free_ctrl_plane)(struct gve_priv *priv);
@@ -411,7 +416,7 @@ struct gve_priv {
TAILQ_HEAD(, gve_flow) active_flows;
/* HW Timestamping Fields */
- bool nic_timestamp_supported;
+ enum gve_dev_clk_type clk_read_type;
const struct rte_memzone *nic_ts_report_mz;
struct gve_nic_ts_report *nic_ts_report;
pthread_mutex_t nic_ts_lock;
--
2.56.0.rc1.315.gc6ed9934b7-goog
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v2 0/8] gve: precursor control plane changes before bare-metal support
2026-10-05 19:39 ` [PATCH v2 " Joshua Washington
` (7 preceding siblings ...)
2026-10-05 19:39 ` [PATCH v2 8/8] net/gve: refactor timestamp support to clock read type Joshua Washington
@ 2026-10-06 14:19 ` Stephen Hemminger
2026-10-06 15:10 ` Stephen Hemminger
9 siblings, 0 replies; 22+ messages in thread
From: Stephen Hemminger @ 2026-10-06 14:19 UTC (permalink / raw)
To: Joshua Washington; +Cc: dev
On Mon, 5 Oct 2026 12:39:31 -0700
Joshua Washington <joshwash@google.com> wrote:
> This patch series includes a number of precursor changes to the GVE
> ethdev driver layer in preparation for introducing support for a new
> GVE Mailbox control plane (an alternative to the existing AdminQ control
> plane).
>
> The most major change in this series is the introduction of a new
> control_ops struct that will be used by both AQ and Mailbox for various
> operations which may need to communicate with the device. There are also
> some changes to how RSS and timestamping are handled in ethdev due to
> slight feature differences between the control planes.
>
> ---
Lots of issues found with manual run of AI review.
Review: [PATCH v2 0/8] net/gve control ops rework
Series summary
Patches 1 and 6 add the query_rss op and NULL handling for "optional"
ops, but no op table in this series sets query_rss and the only table
fills every optional op. None of that code can run until the mailbox
backend lands. The abstraction is easier to judge posted together with
its second user.
Errors
[PATCH v2 1/8] net/gve: refactor ethdev for control ops interface
1. Driver compatibility is no longer sent to the device after reset.
gve_verify_driver_compatibility() moved into
gve_adminq_get_device_properties(), which is skipped on reset:
if (skip_describe_device)
goto setup_device;
...
err = priv->ctrl_ops->get_device_properties(priv);
Before this patch it ran right after gve_adminq_alloc() on every
gve_init_priv() call, including gve_dev_reset() ->
gve_init_priv(priv, true). Functional change in a refactor patch,
not mentioned in the commit message. Keep it on the path that runs
on reset, e.g. an AdminQ init_ctrl_plane op that does
gve_adminq_alloc() followed by the compatibility check.
2. Function pointer table stored in shared memory.
priv->ctrl_ops = &gve_adminq_ops;
priv is dev->data->dev_private, shared with secondary processes, and
holds the primary's address of gve_adminq_ops. A secondary installs
dev_ops in gve_dev_init() and returns; gve_link_update() has no
process type check:
err = priv->ctrl_ops->report_link_speed(priv);
GVE has no LSC interrupt, so rte_eth_link_get() from a secondary on
a started port calls link_update and jumps through the primary's
address. That crashes whenever the driver is mapped at a different
address (shared build, PIE with ASLR). Same for read_clock, mtu_set,
RSS and flow ops. Before this patch the secondary called the AdminQ
functions directly. Keep the table process local: store a control
plane mode enum in priv and resolve the ops with an inline helper,
or keep the pointer in eth_dev->process_private and set it in both
primary and secondary init.
[PATCH v2 6/8] net/gve: add RSS cache boolean flag
3. One failed AdminQ RSS command locks out RSS configuration for the
life of the port.
err = gve_adminq_execute_cmd(priv, &cmd);
priv->rss_cache_dirty = true;
if (err == 0)
gve_update_priv_rss_config(priv, rss_config);
On error the flag stays set. gve_adminq_ops has no query_rss, so
gve_rss_update_cache() returns -ENOENT from gve_rss_hash_update(),
gve_rss_hash_conf_get(), gve_rss_reta_update() and
gve_rss_reta_query(), and gve_dev_configure() skips the RETA reset
for the new queue count. Only gve_update_priv_rss_config() clears
the flag, and it is reached only through configure_rss, which every
caller gates on gve_rss_update_cache(). gve_dev_reset() does not
recover: gve_init_priv() touches the flag only when query_rss is
set. The same lockout follows a successful command when
gve_update_priv_rss_config() fails with -ENOMEM; its return value
is ignored since patch 5.
Before this patch a failed AdminQ command left the cached config in
place. AdminQ has no query, so its cache is authoritative. Drop the
dirty write from gve_adminq_configure_rss(), leave it to a backend
that implements query_rss, and propagate the update result:
err = gve_adminq_execute_cmd(priv, &cmd);
if (err == 0)
err = gve_update_priv_rss_config(priv, rss_config);
Warnings
[PATCH v2 6/8] net/gve: add RSS cache boolean flag
4. priv->rss_config is read before the cache is refreshed.
The commit message says the config must not be read while dirty,
but gve_rss_hash_update() checks priv->rss_config.key_size, copies
it into rss_conf->rss_key_len, and sizes the new table from the
cache before refreshing it:
rss_reta_size = priv->rss_config.indir ?
priv->rss_config.indir_size :
GVE_RSS_INDIR_SIZE;
err = gve_init_rss_config(&gve_rss_conf, rss_conf->rss_key_len,
rss_reta_size);
...
err = gve_rss_update_cache(priv);
The later copy then takes its length from the pre-refresh cache and
its source from the post-refresh one:
memcpy(gve_rss_conf.indir, priv->rss_config.indir,
gve_rss_conf.indir_size * sizeof(*priv->rss_config.indir));
gve_dev_configure() likewise tests priv->rss_config.indir before
refreshing. Call gve_rss_update_cache() before the first
priv->rss_config access in both functions.
[PATCH v2 7/8] net/gve: fix RSS config memory leak on close
5. Wrong Fixes tag. priv->rss_config has been allocated by
gve_update_priv_rss_config() since RSS support was added and was
never freed, on close or on remove, before or after 7ba84453bacf.
Use:
Fixes: 63ef54569760 ("net/gve: support RSS configuration update")
6. Stable fix sits behind six refactor patches. It applies to main on
its own; move it to the front of the series.
[PATCH v2 8/8] net/gve: refactor timestamp support to clock read type
7. RTE_ETH_RX_OFFLOAD_TIMESTAMP is now advertised when timestamp setup
failed.
- if (!gve_is_gqi(priv) && priv->nic_ts_report_mz)
+ if (priv->clk_read_type != GVE_DEV_CLK_UNSUPPORTED)
gve_setup_nic_timestamp() leaves nic_ts_report_mz NULL when the
memzone reservation fails and frees it when the sync thread cannot
be created; clk_read_type stays GVE_DEV_CLK_CMD in both cases.
nic_ts_stale stays set, so the Rx path never stamps packets. Before
this patch rte_eth_dev_configure() rejected the offload in that
state. Keep the nic_ts_report_mz test, or set clk_read_type to
GVE_DEV_CLK_UNSUPPORTED on setup failure.
Info
[PATCH v2 1/8] net/gve: refactor ethdev for control ops interface
8. The comment marks free_db_resources, setup_stats_report,
report_nic_timestamp and the page list ops optional, but only the
set_mtu caller checks for NULL. gve_teardown_device_resources()
calls free_db_resources unconditionally. Either check at each call
site or drop the "optional" claim until a backend omits them.
[PATCH v2 6/8] net/gve: add RSS cache boolean flag
9. Pre-existing, not introduced by this patch, but the lines are
re-indented here: gve_dev_configure() ignores the return of
gve_init_rss_config_from_priv(). If the key allocation fails,
update_reta_config.indir is uninitialized stack and
gve_generate_rss_reta() writes through it. If the indir allocation
fails, gve_init_rss_config() frees key without clearing it and
gve_free_rss_config() frees it again.
10. Pre-existing: gve_update_priv_rss_config() assigns rte_realloc()
straight back to the pointer, leaking the old buffer on failure:
priv_config->key = rte_realloc(priv_config->key, key_bytes,
RTE_CACHE_LINE_SIZE);
Same for indir.
[PATCH v2 7/8] net/gve: fix RSS config memory leak on close
11. gve_teardown_device_resources() also runs on gve_dev_reset(). After
a reset key and indir are NULL but key_size, indir_size and
hash_types survive, so gve_rss_hash_conf_get() reports the old
rss_key_len and rss_hf. Zero priv->rss_config after freeing.
[PATCH v2 8/8] net/gve: refactor timestamp support to clock read type
12. The GVE_DEV_CLK_UNSUPPORTED check added to
gve_alloc_nic_ts_report() is dead: its only caller,
gve_setup_nic_timestamp(), already returns on that value.
Review-Result: ERROR
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH v2 0/8] gve: precursor control plane changes before bare-metal support
2026-10-05 19:39 ` [PATCH v2 " Joshua Washington
` (8 preceding siblings ...)
2026-10-06 14:19 ` [PATCH v2 0/8] gve: precursor control plane changes before bare-metal support Stephen Hemminger
@ 2026-10-06 15:10 ` Stephen Hemminger
2026-10-07 20:11 ` Joshua Washington
9 siblings, 1 reply; 22+ messages in thread
From: Stephen Hemminger @ 2026-10-06 15:10 UTC (permalink / raw)
To: Joshua Washington; +Cc: dev
On Mon, 5 Oct 2026 12:39:31 -0700
Joshua Washington <joshwash@google.com> wrote:
> This patch series includes a number of precursor changes to the GVE
> ethdev driver layer in preparation for introducing support for a new
> GVE Mailbox control plane (an alternative to the existing AdminQ control
> plane).
>
> The most major change in this series is the introduction of a new
> control_ops struct that will be used by both AQ and Mailbox for various
> operations which may need to communicate with the device. There are also
> some changes to how RSS and timestamping are handled in ethdev due to
> slight feature differences between the control planes.
>
> ---
Lots of issues found with manual run of AI review.
Review: [PATCH v2 0/8] net/gve control ops rework (bundle 2148)
Series summary
Patches 1 and 6 add the query_rss op and NULL handling for "optional"
ops, but no op table in this series sets query_rss and the only table
fills every optional op. None of that code can run until the mailbox
backend lands. The abstraction is easier to judge posted together with
its second user.
Errors
[PATCH v2 1/8] net/gve: refactor ethdev for control ops interface
1. Driver compatibility is no longer sent to the device after reset.
gve_verify_driver_compatibility() moved into
gve_adminq_get_device_properties(), which is skipped on reset:
if (skip_describe_device)
goto setup_device;
...
err = priv->ctrl_ops->get_device_properties(priv);
Before this patch it ran right after gve_adminq_alloc() on every
gve_init_priv() call, including gve_dev_reset() ->
gve_init_priv(priv, true). Functional change in a refactor patch,
not mentioned in the commit message. Keep it on the path that runs
on reset, e.g. an AdminQ init_ctrl_plane op that does
gve_adminq_alloc() followed by the compatibility check.
2. Function pointer table stored in shared memory.
priv->ctrl_ops = &gve_adminq_ops;
priv is dev->data->dev_private, shared with secondary processes, and
holds the primary's address of gve_adminq_ops. A secondary installs
dev_ops in gve_dev_init() and returns; gve_link_update() has no
process type check:
err = priv->ctrl_ops->report_link_speed(priv);
GVE has no LSC interrupt, so rte_eth_link_get() from a secondary on
a started port calls link_update and jumps through the primary's
address. That crashes whenever the driver is mapped at a different
address (shared build, PIE with ASLR). Same for read_clock, mtu_set,
RSS and flow ops. Before this patch the secondary called the AdminQ
functions directly. Keep the table process local: store a control
plane mode enum in priv and resolve the ops with an inline helper,
or keep the pointer in eth_dev->process_private and set it in both
primary and secondary init.
[PATCH v2 6/8] net/gve: add RSS cache boolean flag
3. One failed AdminQ RSS command locks out RSS configuration for the
life of the port.
err = gve_adminq_execute_cmd(priv, &cmd);
priv->rss_cache_dirty = true;
if (err == 0)
gve_update_priv_rss_config(priv, rss_config);
On error the flag stays set. gve_adminq_ops has no query_rss, so
gve_rss_update_cache() returns -ENOENT from gve_rss_hash_update(),
gve_rss_hash_conf_get(), gve_rss_reta_update() and
gve_rss_reta_query(), and gve_dev_configure() skips the RETA reset
for the new queue count. Only gve_update_priv_rss_config() clears
the flag, and it is reached only through configure_rss, which every
caller gates on gve_rss_update_cache(). gve_dev_reset() does not
recover: gve_init_priv() touches the flag only when query_rss is
set. The same lockout follows a successful command when
gve_update_priv_rss_config() fails with -ENOMEM; its return value
is ignored since patch 5.
Before this patch a failed AdminQ command left the cached config in
place. AdminQ has no query, so its cache is authoritative. Drop the
dirty write from gve_adminq_configure_rss(), leave it to a backend
that implements query_rss, and propagate the update result:
err = gve_adminq_execute_cmd(priv, &cmd);
if (err == 0)
err = gve_update_priv_rss_config(priv, rss_config);
Warnings
[PATCH v2 6/8] net/gve: add RSS cache boolean flag
4. priv->rss_config is read before the cache is refreshed.
The commit message says the config must not be read while dirty,
but gve_rss_hash_update() checks priv->rss_config.key_size, copies
it into rss_conf->rss_key_len, and sizes the new table from the
cache before refreshing it:
rss_reta_size = priv->rss_config.indir ?
priv->rss_config.indir_size :
GVE_RSS_INDIR_SIZE;
err = gve_init_rss_config(&gve_rss_conf, rss_conf->rss_key_len,
rss_reta_size);
...
err = gve_rss_update_cache(priv);
The later copy then takes its length from the pre-refresh cache and
its source from the post-refresh one:
memcpy(gve_rss_conf.indir, priv->rss_config.indir,
gve_rss_conf.indir_size * sizeof(*priv->rss_config.indir));
gve_dev_configure() likewise tests priv->rss_config.indir before
refreshing. Call gve_rss_update_cache() before the first
priv->rss_config access in both functions.
[PATCH v2 7/8] net/gve: fix RSS config memory leak on close
5. Wrong Fixes tag. priv->rss_config has been allocated by
gve_update_priv_rss_config() since RSS support was added and was
never freed, on close or on remove, before or after 7ba84453bacf.
Use:
Fixes: 63ef54569760 ("net/gve: support RSS configuration update")
6. Stable fix sits behind six refactor patches. It applies to main on
its own; move it to the front of the series.
[PATCH v2 8/8] net/gve: refactor timestamp support to clock read type
7. RTE_ETH_RX_OFFLOAD_TIMESTAMP is now advertised when timestamp setup
failed.
- if (!gve_is_gqi(priv) && priv->nic_ts_report_mz)
+ if (priv->clk_read_type != GVE_DEV_CLK_UNSUPPORTED)
gve_setup_nic_timestamp() leaves nic_ts_report_mz NULL when the
memzone reservation fails and frees it when the sync thread cannot
be created; clk_read_type stays GVE_DEV_CLK_CMD in both cases.
nic_ts_stale stays set, so the Rx path never stamps packets. Before
this patch rte_eth_dev_configure() rejected the offload in that
state. Keep the nic_ts_report_mz test, or set clk_read_type to
GVE_DEV_CLK_UNSUPPORTED on setup failure.
Info
[PATCH v2 1/8] net/gve: refactor ethdev for control ops interface
8. The comment marks free_db_resources, setup_stats_report,
report_nic_timestamp and the page list ops optional, but only the
set_mtu caller checks for NULL. gve_teardown_device_resources()
calls free_db_resources unconditionally. Either check at each call
site or drop the "optional" claim until a backend omits them.
[PATCH v2 6/8] net/gve: add RSS cache boolean flag
9. Pre-existing, not introduced by this patch, but the lines are
re-indented here: gve_dev_configure() ignores the return of
gve_init_rss_config_from_priv(). If the key allocation fails,
update_reta_config.indir is uninitialized stack and
gve_generate_rss_reta() writes through it. If the indir allocation
fails, gve_init_rss_config() frees key without clearing it and
gve_free_rss_config() frees it again.
10. Pre-existing: gve_update_priv_rss_config() assigns rte_realloc()
straight back to the pointer, leaking the old buffer on failure:
priv_config->key = rte_realloc(priv_config->key, key_bytes,
RTE_CACHE_LINE_SIZE);
Same for indir.
[PATCH v2 7/8] net/gve: fix RSS config memory leak on close
11. gve_teardown_device_resources() also runs on gve_dev_reset(). After
a reset key and indir are NULL but key_size, indir_size and
hash_types survive, so gve_rss_hash_conf_get() reports the old
rss_key_len and rss_hf. Zero priv->rss_config after freeing.
[PATCH v2 8/8] net/gve: refactor timestamp support to clock read type
12. The GVE_DEV_CLK_UNSUPPORTED check added to
gve_alloc_nic_ts_report() is dead: its only caller,
gve_setup_nic_timestamp(), already returns on that value.
Review-Result: ERROR
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH v2 0/8] gve: precursor control plane changes before bare-metal support
2026-10-06 15:10 ` Stephen Hemminger
@ 2026-10-07 20:11 ` Joshua Washington
0 siblings, 0 replies; 22+ messages in thread
From: Joshua Washington @ 2026-10-07 20:11 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: dev
On Tue, Oct 6, 2026 at 8:10 AM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Mon, 5 Oct 2026 12:39:31 -0700
> Joshua Washington <joshwash@google.com> wrote:
>
> > This patch series includes a number of precursor changes to the GVE
> > ethdev driver layer in preparation for introducing support for a new
> > GVE Mailbox control plane (an alternative to the existing AdminQ control
> > plane).
> >
> > The most major change in this series is the introduction of a new
> > control_ops struct that will be used by both AQ and Mailbox for various
> > operations which may need to communicate with the device. There are also
> > some changes to how RSS and timestamping are handled in ethdev due to
> > slight feature differences between the control planes.
> >
> > ---
>
> Lots of issues found with manual run of AI review.
For now, I've addressed issues marked as Warning or Error. If I see
any "Info" issues that can be trivially fixed, I'll include changes
for those as well, but have not commented on them explicitly.
>
> Review: [PATCH v2 0/8] net/gve control ops rework (bundle 2148)
>
> Series summary
>
> Patches 1 and 6 add the query_rss op and NULL handling for "optional"
> ops, but no op table in this series sets query_rss and the only table
> fills every optional op. None of that code can run until the mailbox
> backend lands. The abstraction is easier to judge posted together with
> its second user.
>
> Errors
>
> [PATCH v2 1/8] net/gve: refactor ethdev for control ops interface
>
> 1. Driver compatibility is no longer sent to the device after reset.
>
> gve_verify_driver_compatibility() moved into
> gve_adminq_get_device_properties(), which is skipped on reset:
>
> if (skip_describe_device)
> goto setup_device;
> ...
> err = priv->ctrl_ops->get_device_properties(priv);
>
> Before this patch it ran right after gve_adminq_alloc() on every
> gve_init_priv() call, including gve_dev_reset() ->
> gve_init_priv(priv, true). Functional change in a refactor patch,
> not mentioned in the commit message. Keep it on the path that runs
> on reset, e.g. an AdminQ init_ctrl_plane op that does
> gve_adminq_alloc() followed by the compatibility check.
This change was intentional. Foregoing
gve_verify_driver_compatibility() on reset should have no negative
effect.
>
> 2. Function pointer table stored in shared memory.
>
> priv->ctrl_ops = &gve_adminq_ops;
>
> priv is dev->data->dev_private, shared with secondary processes, and
> holds the primary's address of gve_adminq_ops. A secondary installs
> dev_ops in gve_dev_init() and returns; gve_link_update() has no
> process type check:
>
> err = priv->ctrl_ops->report_link_speed(priv);
>
> GVE has no LSC interrupt, so rte_eth_link_get() from a secondary on
> a started port calls link_update and jumps through the primary's
> address. That crashes whenever the driver is mapped at a different
> address (shared build, PIE with ASLR). Same for read_clock, mtu_set,
> RSS and flow ops. Before this patch the secondary called the AdminQ
> functions directly. Keep the table process local: store a control
> plane mode enum in priv and resolve the ops with an inline helper,
> or keep the pointer in eth_dev->process_private and set it in both
> primary and secondary init.
Will fix in v3.
>
> [PATCH v2 6/8] net/gve: add RSS cache boolean flag
>
> 3. One failed AdminQ RSS command locks out RSS configuration for the
> life of the port.
>
> err = gve_adminq_execute_cmd(priv, &cmd);
> priv->rss_cache_dirty = true;
> if (err == 0)
> gve_update_priv_rss_config(priv, rss_config);
>
> On error the flag stays set. gve_adminq_ops has no query_rss, so
> gve_rss_update_cache() returns -ENOENT from gve_rss_hash_update(),
> gve_rss_hash_conf_get(), gve_rss_reta_update() and
> gve_rss_reta_query(), and gve_dev_configure() skips the RETA reset
> for the new queue count. Only gve_update_priv_rss_config() clears
> the flag, and it is reached only through configure_rss, which every
> caller gates on gve_rss_update_cache(). gve_dev_reset() does not
> recover: gve_init_priv() touches the flag only when query_rss is
> set. The same lockout follows a successful command when
> gve_update_priv_rss_config() fails with -ENOMEM; its return value
> is ignored since patch 5.
>
> Before this patch a failed AdminQ command left the cached config in
> place. AdminQ has no query, so its cache is authoritative. Drop the
> dirty write from gve_adminq_configure_rss(), leave it to a backend
> that implements query_rss, and propagate the update result:
>
> err = gve_adminq_execute_cmd(priv, &cmd);
> if (err == 0)
> err = gve_update_priv_rss_config(priv, rss_config);
This is a good point. To keep the behavior the same as before the
patch, I will avoid clearing the cache altogether.
>
> Warnings
>
> [PATCH v2 6/8] net/gve: add RSS cache boolean flag
>
> 4. priv->rss_config is read before the cache is refreshed.
>
> The commit message says the config must not be read while dirty,
> but gve_rss_hash_update() checks priv->rss_config.key_size, copies
> it into rss_conf->rss_key_len, and sizes the new table from the
> cache before refreshing it:
>
> rss_reta_size = priv->rss_config.indir ?
> priv->rss_config.indir_size :
> GVE_RSS_INDIR_SIZE;
> err = gve_init_rss_config(&gve_rss_conf, rss_conf->rss_key_len,
> rss_reta_size);
> ...
> err = gve_rss_update_cache(priv);
>
> The later copy then takes its length from the pre-refresh cache and
> its source from the post-refresh one:
>
> memcpy(gve_rss_conf.indir, priv->rss_config.indir,
> gve_rss_conf.indir_size * sizeof(*priv->rss_config.indir));
>
> gve_dev_configure() likewise tests priv->rss_config.indir before
> refreshing. Call gve_rss_update_cache() before the first
> priv->rss_config access in both functions.
GVE only supports RSS reta size of 128 and hash key size of 40. The
important part of priv->rss_config to refresh is the actual data,
which is barred behind a cache update. As for the dev_configure: if
the indirection table is NULL, then RSS was never configured by the
driver. This would mean that device defaults are being used. In that
case, the device automatically scales the RSS reta for the number of
queues, and there is no reason to attempt to update the cache.
>
> [PATCH v2 7/8] net/gve: fix RSS config memory leak on close
>
> 5. Wrong Fixes tag. priv->rss_config has been allocated by
> gve_update_priv_rss_config() since RSS support was added and was
> never freed, on close or on remove, before or after 7ba84453bacf.
> Use:
>
> Fixes: 63ef54569760 ("net/gve: support RSS configuration update")
Will update the Fixes tag.
>
> 6. Stable fix sits behind six refactor patches. It applies to main on
> its own; move it to the front of the series.
Will reorder the patches to have this fix first.
>
> [PATCH v2 8/8] net/gve: refactor timestamp support to clock read type
>
> 7. RTE_ETH_RX_OFFLOAD_TIMESTAMP is now advertised when timestamp setup
> failed.
>
> - if (!gve_is_gqi(priv) && priv->nic_ts_report_mz)
> + if (priv->clk_read_type != GVE_DEV_CLK_UNSUPPORTED)
>
> gve_setup_nic_timestamp() leaves nic_ts_report_mz NULL when the
> memzone reservation fails and frees it when the sync thread cannot
> be created; clk_read_type stays GVE_DEV_CLK_CMD in both cases.
> nic_ts_stale stays set, so the Rx path never stamps packets. Before
> this patch rte_eth_dev_configure() rejected the offload in that
> state. Keep the nic_ts_report_mz test, or set clk_read_type to
> GVE_DEV_CLK_UNSUPPORTED on setup failure.
This was due to a resbase error. Will revert.
>
> Info
>
> [PATCH v2 1/8] net/gve: refactor ethdev for control ops interface
>
> 8. The comment marks free_db_resources, setup_stats_report,
> report_nic_timestamp and the page list ops optional, but only the
> set_mtu caller checks for NULL. gve_teardown_device_resources()
> calls free_db_resources unconditionally. Either check at each call
> site or drop the "optional" claim until a backend omits them.
>
> [PATCH v2 6/8] net/gve: add RSS cache boolean flag
>
> 9. Pre-existing, not introduced by this patch, but the lines are
> re-indented here: gve_dev_configure() ignores the return of
> gve_init_rss_config_from_priv(). If the key allocation fails,
> update_reta_config.indir is uninitialized stack and
> gve_generate_rss_reta() writes through it. If the indir allocation
> fails, gve_init_rss_config() frees key without clearing it and
> gve_free_rss_config() frees it again.
>
> 10. Pre-existing: gve_update_priv_rss_config() assigns rte_realloc()
> straight back to the pointer, leaking the old buffer on failure:
>
> priv_config->key = rte_realloc(priv_config->key, key_bytes,
> RTE_CACHE_LINE_SIZE);
>
> Same for indir.
>
> [PATCH v2 7/8] net/gve: fix RSS config memory leak on close
>
> 11. gve_teardown_device_resources() also runs on gve_dev_reset(). After
> a reset key and indir are NULL but key_size, indir_size and
> hash_types survive, so gve_rss_hash_conf_get() reports the old
> rss_key_len and rss_hf. Zero priv->rss_config after freeing.
>
> [PATCH v2 8/8] net/gve: refactor timestamp support to clock read type
>
> 12. The GVE_DEV_CLK_UNSUPPORTED check added to
> gve_alloc_nic_ts_report() is dead: its only caller,
> gve_setup_nic_timestamp(), already returns on that value.
>
> Review-Result: ERROR
--
Joshua Washington | Software Engineer | joshwash@google.com | (414) 366-4423
^ permalink raw reply [flat|nested] 22+ messages in thread