rte_eth_dev_configure() sets the MTU from rxmode.mtu without calling
mtu_set, and keeps Rx queues that are already set up. Stopping the port
and reconfiguring it with a larger MTU, or without
RTE_ETH_RX_OFFLOAD_SCATTER, therefore skipped both the MTU change check
and the Rx queue setup check. gve also never gave the device that MTU.
Even with the MTU programmed, a GQI device does not limit what it
receives to it: on a network with a larger MTU it still delivers
bigger frames, chained over several Rx buffers. So the configuration
checks alone cannot keep chained mbufs from an application that did
not enable scatter.
Move the configuration check into gve_rx_check_scatter() and call it
from gve_dev_start() as well as gve_dev_mtu_set(), program the
configured MTU into the device at start, and have the GQI Rx path drop
a frame that spans more than one buffer, counted in the Rx errors,
when the port has not enabled scatter. A dropped descriptor's mbuf is
now freed, since the refill replaces it.
Fixes: cffdca824388 ("net/gve: refuse an MTU that needs Rx scatter when it is
off")
Reported-by: Stephen Hemminger <[email protected]>
Signed-off-by: Ryan Wolfe <[email protected]>
---
v2:
- program the configured MTU into the device at start (Stephen)
- on GQI-QPL hardware the device does not enforce the driver MTU on Rx:
8000-byte frames still arrive chained over 4 buffers at MTU 1500, so
also drop and count chained frames in the Rx path when scatter is off,
and free a dropped descriptor's mbuf
- retitled from "net/gve: check the Rx scatter MTU at device start"
- commit message fix and no Cc: stable (Stephen)
Tested on a GQI-QPL gVNIC with testpmd. At MTU 1500 without scatter,
25000 8000-byte and 25000 1000-byte UDP datagrams, interleaved, gave
25000 Rx errors and 25000 packets received, with no nombuf on a
16384-mbuf pool and traffic still flowing afterwards; with scatter on,
the 8042-byte frames arrive as 4-segment mbufs. A small ethdev program
that reconfigures a started-and-stopped port to MTU 8000 without
scatter, keeping its Rx queues, now gets -EINVAL from
rte_eth_dev_start() (it started before).
drivers/net/gve/gve_ethdev.c | 75 +++++++++++++++++++++++++++---------
drivers/net/gve/gve_ethdev.h | 2 +
drivers/net/gve/gve_rx.c | 20 +++++++++-
3 files changed, 76 insertions(+), 21 deletions(-)
diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c
index 7506c46d9a..8ee52fdd47 100644
--- a/drivers/net/gve/gve_ethdev.c
+++ b/drivers/net/gve/gve_ethdev.c
@@ -459,11 +459,38 @@ gve_start_queues(struct rte_eth_dev *dev)
return ret;
}
+/* GQI chains a frame larger than one Rx buffer into a multi-segment
+ * mbuf. No Rx offload is per-queue here, so check the port offloads.
+ */
+static int
+gve_rx_check_scatter(struct rte_eth_dev *dev, uint16_t mtu)
+{
+ struct gve_priv *priv = dev->data->dev_private;
+ uint16_t i;
+
+ if (!gve_is_gqi(priv) ||
+ (dev->data->dev_conf.rxmode.offloads & RTE_ETH_RX_OFFLOAD_SCATTER))
+ return 0;
+
+ for (i = 0; i < dev->data->nb_rx_queues; i++) {
+ struct gve_rx_queue *rxq = dev->data->rx_queues[i];
+
+ if (rxq != NULL && gve_rx_gqi_needs_scatter(mtu,
rxq->rx_buf_len)) {
+ PMD_DRV_LOG(ERR, "MTU %u needs Rx scatter with %u byte
Rx buffers",
+ mtu, rxq->rx_buf_len);
+ return -EINVAL;
+ }
+ }
+
+ return 0;
+}
+
static int
gve_dev_start(struct rte_eth_dev *dev)
{
struct gve_process_private *process_priv = dev->process_private;
- struct gve_priv *priv;
+ struct gve_priv *priv = dev->data->dev_private;
+ uint16_t i;
int ret;
if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
@@ -471,6 +498,31 @@ gve_dev_start(struct rte_eth_dev *dev)
return -EPERM;
}
+ /* rte_eth_dev_configure() sets the MTU and the Rx offloads without
+ * calling mtu_set, and keeps Rx queues already set up, so check them
+ * again and give the device the MTU the check was made against.
+ */
+ ret = gve_rx_check_scatter(dev, dev->data->mtu);
+ if (ret != 0)
+ return ret;
+
+ ret = process_priv->ctrl_ops->set_mtu(priv, dev->data->mtu);
+ if (ret != 0) {
+ PMD_DRV_LOG(ERR, "Failed to set MTU %u: %d", dev->data->mtu,
ret);
+ return ret;
+ }
+
+ /* The device chains frames up to its own MTU, not the one it was
+ * given; the Rx path drops them unless scatter is on.
+ */
+ for (i = 0; i < dev->data->nb_rx_queues; i++) {
+ struct gve_rx_queue *rxq = dev->data->rx_queues[i];
+
+ if (rxq != NULL)
+ rxq->scatter = !!(dev->data->dev_conf.rxmode.offloads &
+ RTE_ETH_RX_OFFLOAD_SCATTER);
+ }
+
ret = gve_start_queues(dev);
if (ret != 0) {
PMD_DRV_LOG(ERR, "Failed to start queues");
@@ -480,7 +532,6 @@ gve_dev_start(struct rte_eth_dev *dev)
dev->data->dev_started = 1;
gve_link_update(dev, 0);
- priv = dev->data->dev_private;
/* No stats available yet for Dqo. */
if (gve_is_gqi(priv)) {
ret = gve_alloc_stats_report(priv,
@@ -1031,23 +1082,9 @@ gve_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
return -EBUSY;
}
- /* GQI chains a frame larger than one Rx buffer into a multi-segment
- * mbuf. No Rx offload is per-queue here, so check the port offloads.
- */
- if (gve_is_gqi(priv) &&
- !(dev->data->dev_conf.rxmode.offloads &
RTE_ETH_RX_OFFLOAD_SCATTER)) {
- for (uint16_t i = 0; i < dev->data->nb_rx_queues; i++) {
- struct gve_rx_queue *rxq = dev->data->rx_queues[i];
-
- if (rxq != NULL &&
- gve_rx_gqi_needs_scatter(mtu, rxq->rx_buf_len)) {
- PMD_DRV_LOG(ERR,
- "MTU %u needs Rx scatter with %u
byte Rx buffers",
- mtu, rxq->rx_buf_len);
- return -EINVAL;
- }
- }
- }
+ err = gve_rx_check_scatter(dev, mtu);
+ if (err != 0)
+ return err;
err = process_priv->ctrl_ops->set_mtu(priv, mtu);
if (err) {
diff --git a/drivers/net/gve/gve_ethdev.h b/drivers/net/gve/gve_ethdev.h
index 26c6a304a6..3a8ddfbbb6 100644
--- a/drivers/net/gve/gve_ethdev.h
+++ b/drivers/net/gve/gve_ethdev.h
@@ -263,6 +263,8 @@ struct gve_rx_queue {
struct rte_mbuf **refill_bufs;
uint8_t is_gqi_qpl;
+ /* the port enabled RTE_ETH_RX_OFFLOAD_SCATTER (GQI) */
+ bool scatter;
bool timestamp_enabled;
};
diff --git a/drivers/net/gve/gve_rx.c b/drivers/net/gve/gve_rx.c
index 4a455b3067..31c187ca83 100644
--- a/drivers/net/gve/gve_rx.c
+++ b/drivers/net/gve/gve_rx.c
@@ -139,12 +139,21 @@ gve_rx(struct gve_rx_queue *rxq, volatile struct
gve_rx_desc *rxd, uint16_t rx_i
struct rte_mbuf *rxe;
if (ctx->drop_pkt)
- goto finish_frag;
+ goto drop_frag;
if (rxd->flags_seq & GVE_RXF_ERR) {
ctx->drop_pkt = true;
rxq->stats.errors++;
- goto finish_frag;
+ goto drop_frag;
+ }
+
+ /* The device chains a frame larger than one Rx buffer whatever MTU
+ * it was given, so drop it unless the application enabled scatter.
+ */
+ if (is_first_frag && !is_last_frag && !rxq->scatter) {
+ ctx->drop_pkt = true;
+ rxq->stats.errors++;
+ goto drop_frag;
}
if (is_first_frag)
@@ -171,6 +180,12 @@ gve_rx(struct gve_rx_queue *rxq, volatile struct
gve_rx_desc *rxd, uint16_t rx_i
}
}
+ goto finish_frag;
+
+drop_frag:
+ /* The refill puts a new mbuf in this slot, so free the old one. */
+ rte_pktmbuf_free(rxq->sw_ring[rx_id]);
+ rxq->sw_ring[rx_id] = NULL;
finish_frag:
ctx->total_frags++;
return is_last_frag;
@@ -375,6 +390,7 @@ gve_rx_queue_setup(struct rte_eth_dev *dev, uint16_t
queue_id,
rxq->ntfy_addr =
&hw->db_bar[rte_be_to_cpu_32(hw->irq_dbs[rxq->ntfy_id].id)];
rxq->rx_buf_len = rx_buf_len;
+ rxq->scatter = !!(offloads & RTE_ETH_RX_OFFLOAD_SCATTER);
/* Allocate software ring */
rxq->sw_ring = rte_zmalloc_socket("gve rx sw ring", sizeof(struct
rte_mbuf *) * nb_desc,
--
2.56.0.257.gf7497c7d27-goog
