[PATCH net-next 0/3] gve: Improve RX buffer length management

[PATCH net-next 0/3] gve: Improve RX buffer length management

[Joshua Washington]

From: Ankit Garg <nktgrg@google.com>

This patch series improves the management of the RX buffer length for
the DQO queue format in the gve driver. The goal is to make RX buffer
length config more explicit, easy to change, and performant by default.

We accomplish that in three patches:

1.  Currently, the buffer length is implicitly coupled with the header
    split setting, which is an unintuitive and restrictive design. The
    first patch decouples the RX buffer length from the header split
    configuration.

2.  The second patch exposes the `rx_buf_len` parameter to userspace via
    ethtool, allowing user to directly view or modify the RX buffer
    length.

3.  The final patch improves the out-of-the-box RX single stream
    throughput by >10%  by changing the driver's default behavior to
    select the maximum supported RX buffer length advertised by the
    device during initialization.

Ankit Garg (3):
  gve: Decouple header split from RX buffer length
  gve: Allow ethtool to configure rx_buf_len
  gve: Default to max_rx_buffer_size for DQO if device supported

 drivers/net/ethernet/google/gve/gve.h         | 12 +++-
 drivers/net/ethernet/google/gve/gve_adminq.c  |  4 ++
 drivers/net/ethernet/google/gve/gve_ethtool.c | 13 ++++-
 drivers/net/ethernet/google/gve/gve_main.c    | 55 +++++++++++++++----
 4 files changed, 69 insertions(+), 15 deletions(-)

--
2.51.1.814.gb8fa24458f-goog

[PATCH net-next 1/3] gve: Decouple header split from RX buffer length

[Joshua Washington]

From: Ankit Garg <nktgrg@google.com>

Previously, enabling header split via `gve_set_hsplit_config` also
implicitly changed the RX buffer length to 4K (if supported by the
device). This coupled two settings that should be orthogonal; this patch
removes that side effect.

After this change, `gve_set_hsplit_config` only toggles the header
split configuration. The RX buffer length is no longer affected and
must be configured independently.

Signed-off-by: Ankit Garg <nktgrg@google.com>
Reviewed-by: Harshitha Ramamurthy <hramamurthy@google.com>
Reviewed-by: Jordan Rhee <jordanrhee@google.com>
Reviewed-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: Joshua Washington <joshwash@google.com>
---
 drivers/net/ethernet/google/gve/gve.h         |  3 ---
 drivers/net/ethernet/google/gve/gve_ethtool.c |  2 --
 drivers/net/ethernet/google/gve/gve_main.c    | 10 ----------
 3 files changed, 15 deletions(-)

diff --git a/drivers/net/ethernet/google/gve/gve.h b/drivers/net/ethernet/google/gve/gve.h
index cf95ec25b11a..c237d00c5ab3 100644
--- a/drivers/net/ethernet/google/gve/gve.h
+++ b/drivers/net/ethernet/google/gve/gve.h
@@ -59,8 +59,6 @@
 
 #define GVE_DEFAULT_RX_BUFFER_SIZE 2048
 
-#define GVE_MAX_RX_BUFFER_SIZE 4096
-
 #define GVE_XDP_RX_BUFFER_SIZE_DQO 4096
 
 #define GVE_DEFAULT_RX_BUFFER_OFFSET 2048
@@ -1249,7 +1247,6 @@ void gve_rx_free_rings_gqi(struct gve_priv *priv,
 			   struct gve_rx_alloc_rings_cfg *cfg);
 void gve_rx_start_ring_gqi(struct gve_priv *priv, int idx);
 void gve_rx_stop_ring_gqi(struct gve_priv *priv, int idx);
-u16 gve_get_pkt_buf_size(const struct gve_priv *priv, bool enable_hplit);
 bool gve_header_split_supported(const struct gve_priv *priv);
 int gve_set_hsplit_config(struct gve_priv *priv, u8 tcp_data_split,
 			  struct gve_rx_alloc_rings_cfg *rx_alloc_cfg);
diff --git a/drivers/net/ethernet/google/gve/gve_ethtool.c b/drivers/net/ethernet/google/gve/gve_ethtool.c
index b030a84b678c..db6fc855a511 100644
--- a/drivers/net/ethernet/google/gve/gve_ethtool.c
+++ b/drivers/net/ethernet/google/gve/gve_ethtool.c
@@ -606,8 +606,6 @@ static int gve_set_ringparam(struct net_device *netdev,
 	} else {
 		/* Set ring params for the next up */
 		priv->header_split_enabled = rx_alloc_cfg.enable_header_split;
-		priv->rx_cfg.packet_buffer_size =
-			rx_alloc_cfg.packet_buffer_size;
 		priv->tx_desc_cnt = tx_alloc_cfg.ring_size;
 		priv->rx_desc_cnt = rx_alloc_cfg.ring_size;
 	}
diff --git a/drivers/net/ethernet/google/gve/gve_main.c b/drivers/net/ethernet/google/gve/gve_main.c
index 29845e8f3c0d..8d825218965a 100644
--- a/drivers/net/ethernet/google/gve/gve_main.c
+++ b/drivers/net/ethernet/google/gve/gve_main.c
@@ -2041,14 +2041,6 @@ static void gve_tx_timeout(struct net_device *dev, unsigned int txqueue)
 	priv->tx_timeo_cnt++;
 }
 
-u16 gve_get_pkt_buf_size(const struct gve_priv *priv, bool enable_hsplit)
-{
-	if (enable_hsplit && priv->max_rx_buffer_size >= GVE_MAX_RX_BUFFER_SIZE)
-		return GVE_MAX_RX_BUFFER_SIZE;
-	else
-		return GVE_DEFAULT_RX_BUFFER_SIZE;
-}
-
 /* Header split is only supported on DQ RDA queue format. If XDP is enabled,
  * header split is not allowed.
  */
@@ -2080,8 +2072,6 @@ int gve_set_hsplit_config(struct gve_priv *priv, u8 tcp_data_split,
 		return 0;
 
 	rx_alloc_cfg->enable_header_split = enable_hdr_split;
-	rx_alloc_cfg->packet_buffer_size =
-		gve_get_pkt_buf_size(priv, enable_hdr_split);
 
 	return 0;
 }
-- 
2.51.1.814.gb8fa24458f-goog

[PATCH net-next 2/3] gve: Allow ethtool to configure rx_buf_len

[Joshua Washington]

From: Ankit Garg <nktgrg@google.com>

Add support for getting and setting the RX buffer length via the
ethtool ring parameters (`ethtool -g`/`-G`). The driver restricts the
allowed buffer length to 2048 (SZ_2K) or 4096 (SZ_4K).

As XDP is only supported when the `rx_buf_len` is 2048, the driver now
enforces this in two places:
1.  In `gve_xdp_set`, rejecting XDP programs if the current buffer
    length is not 2048.
2.  In `gve_set_rx_buf_len_config`, rejecting buffer length changes if
    XDP is loaded and the new length is not 2048.

Signed-off-by: Ankit Garg <nktgrg@google.com>
Reviewed-by: Harshitha Ramamurthy <hramamurthy@google.com>
Reviewed-by: Jordan Rhee <jordanrhee@google.com>
Reviewed-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: Joshua Washington <joshwash@google.com>
---
 drivers/net/ethernet/google/gve/gve.h         |  9 ++++
 drivers/net/ethernet/google/gve/gve_ethtool.c | 13 +++++-
 drivers/net/ethernet/google/gve/gve_main.c    | 45 +++++++++++++++++++
 3 files changed, 66 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/google/gve/gve.h b/drivers/net/ethernet/google/gve/gve.h
index c237d00c5ab3..a33b44c1eb86 100644
--- a/drivers/net/ethernet/google/gve/gve.h
+++ b/drivers/net/ethernet/google/gve/gve.h
@@ -1167,6 +1167,12 @@ static inline bool gve_is_gqi(struct gve_priv *priv)
 		priv->queue_format == GVE_GQI_QPL_FORMAT;
 }
 
+static inline bool gve_is_dqo(struct gve_priv *priv)
+{
+	return priv->queue_format == GVE_DQO_RDA_FORMAT ||
+	       priv->queue_format == GVE_DQO_QPL_FORMAT;
+}
+
 static inline u32 gve_num_tx_queues(struct gve_priv *priv)
 {
 	return priv->tx_cfg.num_queues + priv->tx_cfg.num_xdp_queues;
@@ -1248,6 +1254,9 @@ void gve_rx_free_rings_gqi(struct gve_priv *priv,
 void gve_rx_start_ring_gqi(struct gve_priv *priv, int idx);
 void gve_rx_stop_ring_gqi(struct gve_priv *priv, int idx);
 bool gve_header_split_supported(const struct gve_priv *priv);
+int gve_set_rx_buf_len_config(struct gve_priv *priv, u32 rx_buf_len,
+			      struct netlink_ext_ack *extack,
+			      struct gve_rx_alloc_rings_cfg *rx_alloc_cfg);
 int gve_set_hsplit_config(struct gve_priv *priv, u8 tcp_data_split,
 			  struct gve_rx_alloc_rings_cfg *rx_alloc_cfg);
 /* rx buffer handling */
diff --git a/drivers/net/ethernet/google/gve/gve_ethtool.c b/drivers/net/ethernet/google/gve/gve_ethtool.c
index db6fc855a511..52500ae8348e 100644
--- a/drivers/net/ethernet/google/gve/gve_ethtool.c
+++ b/drivers/net/ethernet/google/gve/gve_ethtool.c
@@ -529,6 +529,8 @@ static void gve_get_ringparam(struct net_device *netdev,
 	cmd->rx_pending = priv->rx_desc_cnt;
 	cmd->tx_pending = priv->tx_desc_cnt;
 
+	kernel_cmd->rx_buf_len = priv->rx_cfg.packet_buffer_size;
+
 	if (!gve_header_split_supported(priv))
 		kernel_cmd->tcp_data_split = ETHTOOL_TCP_DATA_SPLIT_UNKNOWN;
 	else if (priv->header_split_enabled)
@@ -589,6 +591,12 @@ static int gve_set_ringparam(struct net_device *netdev,
 	int err;
 
 	gve_get_curr_alloc_cfgs(priv, &tx_alloc_cfg, &rx_alloc_cfg);
+
+	err = gve_set_rx_buf_len_config(priv, kernel_cmd->rx_buf_len, extack,
+					&rx_alloc_cfg);
+	if (err)
+		return err;
+
 	err = gve_set_hsplit_config(priv, kernel_cmd->tcp_data_split,
 				    &rx_alloc_cfg);
 	if (err)
@@ -605,6 +613,8 @@ static int gve_set_ringparam(struct net_device *netdev,
 			return err;
 	} else {
 		/* Set ring params for the next up */
+		priv->rx_cfg.packet_buffer_size =
+			rx_alloc_cfg.packet_buffer_size;
 		priv->header_split_enabled = rx_alloc_cfg.enable_header_split;
 		priv->tx_desc_cnt = tx_alloc_cfg.ring_size;
 		priv->rx_desc_cnt = rx_alloc_cfg.ring_size;
@@ -944,7 +954,8 @@ static int gve_get_ts_info(struct net_device *netdev,
 
 const struct ethtool_ops gve_ethtool_ops = {
 	.supported_coalesce_params = ETHTOOL_COALESCE_USECS,
-	.supported_ring_params = ETHTOOL_RING_USE_TCP_DATA_SPLIT,
+	.supported_ring_params = ETHTOOL_RING_USE_TCP_DATA_SPLIT |
+				 ETHTOOL_RING_USE_RX_BUF_LEN,
 	.get_drvinfo = gve_get_drvinfo,
 	.get_strings = gve_get_strings,
 	.get_sset_count = gve_get_sset_count,
diff --git a/drivers/net/ethernet/google/gve/gve_main.c b/drivers/net/ethernet/google/gve/gve_main.c
index 8d825218965a..8009819c73f2 100644
--- a/drivers/net/ethernet/google/gve/gve_main.c
+++ b/drivers/net/ethernet/google/gve/gve_main.c
@@ -1722,6 +1722,13 @@ static int verify_xdp_configuration(struct net_device *dev)
 		return -EOPNOTSUPP;
 	}
 
+	if (priv->rx_cfg.packet_buffer_size != SZ_2K) {
+		netdev_warn(dev,
+			    "XDP is not supported for Rx buf len %d. Set Rx buf len to %d before using XDP.\n",
+			    priv->rx_cfg.packet_buffer_size, SZ_2K);
+		return -EOPNOTSUPP;
+	}
+
 	max_xdp_mtu = priv->rx_cfg.packet_buffer_size - sizeof(struct ethhdr);
 	if (priv->queue_format == GVE_GQI_QPL_FORMAT)
 		max_xdp_mtu -= GVE_RX_PAD;
@@ -2050,6 +2057,44 @@ bool gve_header_split_supported(const struct gve_priv *priv)
 		priv->queue_format == GVE_DQO_RDA_FORMAT && !priv->xdp_prog;
 }
 
+int gve_set_rx_buf_len_config(struct gve_priv *priv, u32 rx_buf_len,
+			      struct netlink_ext_ack *extack,
+			      struct gve_rx_alloc_rings_cfg *rx_alloc_cfg)
+{
+	u32 old_rx_buf_len = rx_alloc_cfg->packet_buffer_size;
+
+	if (rx_buf_len == old_rx_buf_len)
+		return 0;
+
+	if (!gve_is_dqo(priv)) {
+		NL_SET_ERR_MSG_MOD(extack,
+				   "Modifying Rx buf len is only supported with DQO format");
+		return -EOPNOTSUPP;
+	}
+
+	if (priv->xdp_prog && rx_buf_len != SZ_2K) {
+		NL_SET_ERR_MSG_MOD(extack,
+				   "Rx buf len can only be 2048 when XDP is on");
+		return -EINVAL;
+	}
+
+	if (rx_buf_len > priv->max_rx_buffer_size) {
+		NL_SET_ERR_MSG_FMT_MOD(extack,
+				       "Rx buf len exceeds the max supported value of %u",
+				       priv->max_rx_buffer_size);
+		return -EINVAL;
+	}
+
+	if (rx_buf_len != SZ_2K && rx_buf_len != SZ_4K) {
+		NL_SET_ERR_MSG_MOD(extack,
+				   "Rx buf len can only be 2048 or 4096");
+		return -EINVAL;
+	}
+	rx_alloc_cfg->packet_buffer_size = rx_buf_len;
+
+	return 0;
+}
+
 int gve_set_hsplit_config(struct gve_priv *priv, u8 tcp_data_split,
 			  struct gve_rx_alloc_rings_cfg *rx_alloc_cfg)
 {
-- 
2.51.1.814.gb8fa24458f-goog

[PATCH net-next 3/3] gve: Default to max_rx_buffer_size for DQO if device supported

[Joshua Washington]

From: Ankit Garg <nktgrg@google.com>

Change the driver's default behavior to prefer the largest available RX
buffer length supported by the device for DQO format, rather than always
using the hardcoded 2K default.

Previously, the driver would initialize with
`GVE_DEFAULT_RX_BUFFER_SIZE` (2K), even if the device advertised support
for a larger length (e.g., 4K).

Performance observations:
- With LRO disabled, we observed >10% improvement in RX single stream
throughput when MTU >=2048.
- With LRO enabled, we observed >10% improvement in RX single stream
throughput when MTU >=1460.
- No regressions were observed.

Signed-off-by: Ankit Garg <nktgrg@google.com>
Reviewed-by: Harshitha Ramamurthy <hramamurthy@google.com>
Reviewed-by: Jordan Rhee <jordanrhee@google.com>
Reviewed-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: Joshua Washington <joshwash@google.com>
---
 drivers/net/ethernet/google/gve/gve_adminq.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/google/gve/gve_adminq.c b/drivers/net/ethernet/google/gve/gve_adminq.c
index 4f33d094a2ef..b72cc0fa2ba2 100644
--- a/drivers/net/ethernet/google/gve/gve_adminq.c
+++ b/drivers/net/ethernet/google/gve/gve_adminq.c
@@ -987,6 +987,10 @@ static void gve_enable_supported_features(struct gve_priv *priv,
 		dev_info(&priv->pdev->dev,
 			 "BUFFER SIZES device option enabled with max_rx_buffer_size of %u, header_buf_size of %u.\n",
 			 priv->max_rx_buffer_size, priv->header_buf_size);
+		if (gve_is_dqo(priv) &&
+		    priv->max_rx_buffer_size > GVE_DEFAULT_RX_BUFFER_SIZE)
+			priv->rx_cfg.packet_buffer_size =
+				priv->max_rx_buffer_size;
 	}
 
 	/* Read and store ring size ranges given by device */
-- 
2.51.1.814.gb8fa24458f-goog

Re: [PATCH net-next 0/3] gve: Improve RX buffer length management

[Jakub Kicinski]

On Wed, 22 Oct 2025 11:22:22 -0700 Joshua Washington wrote:
> This patch series improves the management of the RX buffer length for
> the DQO queue format in the gve driver. The goal is to make RX buffer
> length config more explicit, easy to change, and performant by default.

I suppose this was done in prep for later buffers for ZC?
It's less urgent given the change of plans in
https://lore.kernel.org/all/20251016184031.66c92962@kernel.org/
but still relevant AFAICT. Just wanted to confirm.. Mina?

Re: [PATCH net-next 2/3] gve: Allow ethtool to configure rx_buf_len

[Jakub Kicinski]

On Wed, 22 Oct 2025 11:22:24 -0700 Joshua Washington wrote:
> +	if (priv->rx_cfg.packet_buffer_size != SZ_2K) {
> +		netdev_warn(dev,
> +			    "XDP is not supported for Rx buf len %d. Set Rx buf len to %d before using XDP.\n",
> +			    priv->rx_cfg.packet_buffer_size, SZ_2K);
> +		return -EOPNOTSUPP;
> +	}

Please plumb extack thru to here. It's inside struct netdev_bpf

>  	max_xdp_mtu = priv->rx_cfg.packet_buffer_size - sizeof(struct ethhdr);
>  	if (priv->queue_format == GVE_GQI_QPL_FORMAT)
>  		max_xdp_mtu -= GVE_RX_PAD;
> @@ -2050,6 +2057,44 @@ bool gve_header_split_supported(const struct gve_priv *priv)
>  		priv->queue_format == GVE_DQO_RDA_FORMAT && !priv->xdp_prog;
>  }
>  
> +int gve_set_rx_buf_len_config(struct gve_priv *priv, u32 rx_buf_len,
> +			      struct netlink_ext_ack *extack,
> +			      struct gve_rx_alloc_rings_cfg *rx_alloc_cfg)
> +{
> +	u32 old_rx_buf_len = rx_alloc_cfg->packet_buffer_size;
> +
> +	if (rx_buf_len == old_rx_buf_len)
> +		return 0;
> +
> +	if (!gve_is_dqo(priv)) {
> +		NL_SET_ERR_MSG_MOD(extack,
> +				   "Modifying Rx buf len is only supported with DQO format");
> +		return -EOPNOTSUPP;
> +	}
> +
> +	if (priv->xdp_prog && rx_buf_len != SZ_2K) {
> +		NL_SET_ERR_MSG_MOD(extack,
> +				   "Rx buf len can only be 2048 when XDP is on");
> +		return -EINVAL;
> +	}
> +
> +	if (rx_buf_len > priv->max_rx_buffer_size) {

This check looks kinda pointless given the check right below against
the exact sizes?

> +		NL_SET_ERR_MSG_FMT_MOD(extack,
> +				       "Rx buf len exceeds the max supported value of %u",
> +				       priv->max_rx_buffer_size);
> +		return -EINVAL;
> +	}
> +
> +	if (rx_buf_len != SZ_2K && rx_buf_len != SZ_4K) {
> +		NL_SET_ERR_MSG_MOD(extack,
> +				   "Rx buf len can only be 2048 or 4096");
> +		return -EINVAL;
> +	}
> +	rx_alloc_cfg->packet_buffer_size = rx_buf_len;
> +
> +	return 0;
> +}

Re: [PATCH net-next 0/3] gve: Improve RX buffer length management

[Mina Almasry]

On Thu, Oct 23, 2025 at 5:12 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Wed, 22 Oct 2025 11:22:22 -0700 Joshua Washington wrote:
> > This patch series improves the management of the RX buffer length for
> > the DQO queue format in the gve driver. The goal is to make RX buffer
> > length config more explicit, easy to change, and performant by default.
>
> I suppose this was done in prep for later buffers for ZC?
> It's less urgent given the change of plans in
> https://lore.kernel.org/all/20251016184031.66c92962@kernel.org/
> but still relevant AFAICT. Just wanted to confirm.. Mina?

This is very likely unrelated to the large ZC buffers and instead
related to another effort we're preparing for. Not really 'urgent' but
as always a bit pressing :D

-- 
Thanks,
Mina

Re: [PATCH net-next 0/3] gve: Improve RX buffer length management

[Ankit Garg]

On Thu, Oct 23, 2025 at 7:41 PM Mina Almasry <almasrymina@google.com> wrote:
>
> On Thu, Oct 23, 2025 at 5:12 PM Jakub Kicinski <kuba@kernel.org> wrote:
> >
> > On Wed, 22 Oct 2025 11:22:22 -0700 Joshua Washington wrote:
> > > This patch series improves the management of the RX buffer length for
> > > the DQO queue format in the gve driver. The goal is to make RX buffer
> > > length config more explicit, easy to change, and performant by default.
> >
> > I suppose this was done in prep for later buffers for ZC?
> > It's less urgent given the change of plans in
> > https://lore.kernel.org/all/20251016184031.66c92962@kernel.org/
> > but still relevant AFAICT. Just wanted to confirm.. Mina?
>
> This is very likely unrelated to the large ZC buffers and instead
> related to another effort we're preparing for. Not really 'urgent' but
> as always a bit pressing :D
>
> --
> Thanks,
> Mina

This is being done in preparation for enabling HW-GRO by default later
on. Our device only coalesces packets till 19 buffers are filled and
with 2k buffer, biggest HW coalesced packet will be limited to 38k bytes.

To enable 64k sized packet, 4K buffers are needed.

Re: [PATCH net-next 2/3] gve: Allow ethtool to configure rx_buf_len

[Ankit Garg]

On Thu, Oct 23, 2025 at 5:14 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Wed, 22 Oct 2025 11:22:24 -0700 Joshua Washington wrote:
> > +     if (priv->rx_cfg.packet_buffer_size != SZ_2K) {
> > +             netdev_warn(dev,
> > +                         "XDP is not supported for Rx buf len %d. Set Rx buf len to %d before using XDP.\n",
> > +                         priv->rx_cfg.packet_buffer_size, SZ_2K);
> > +             return -EOPNOTSUPP;
> > +     }
>
> Please plumb extack thru to here. It's inside struct netdev_bpf
>

Using extack just for this log will make it inconsistent with other
logs in this method. Would it be okay if I send a fast follow patch to
use exstack in this method and others?

> >       max_xdp_mtu = priv->rx_cfg.packet_buffer_size - sizeof(struct ethhdr);
> >       if (priv->queue_format == GVE_GQI_QPL_FORMAT)
> >               max_xdp_mtu -= GVE_RX_PAD;
> > @@ -2050,6 +2057,44 @@ bool gve_header_split_supported(const struct gve_priv *priv)
> >               priv->queue_format == GVE_DQO_RDA_FORMAT && !priv->xdp_prog;
> >  }
> >
> > +int gve_set_rx_buf_len_config(struct gve_priv *priv, u32 rx_buf_len,
> > +                           struct netlink_ext_ack *extack,
> > +                           struct gve_rx_alloc_rings_cfg *rx_alloc_cfg)
> > +{
> > +     u32 old_rx_buf_len = rx_alloc_cfg->packet_buffer_size;
> > +
> > +     if (rx_buf_len == old_rx_buf_len)
> > +             return 0;
> > +
> > +     if (!gve_is_dqo(priv)) {
> > +             NL_SET_ERR_MSG_MOD(extack,
> > +                                "Modifying Rx buf len is only supported with DQO format");
> > +             return -EOPNOTSUPP;
> > +     }
> > +
> > +     if (priv->xdp_prog && rx_buf_len != SZ_2K) {
> > +             NL_SET_ERR_MSG_MOD(extack,
> > +                                "Rx buf len can only be 2048 when XDP is on");
> > +             return -EINVAL;
> > +     }
> > +
> > +     if (rx_buf_len > priv->max_rx_buffer_size) {
>
> This check looks kinda pointless given the check right below against
> the exact sizes?
>

My intent was to code defensively against device accidently advertising
anything in [2k+1,4k) as max buffer size. I will remove this check.

> > +             NL_SET_ERR_MSG_FMT_MOD(extack,
> > +                                    "Rx buf len exceeds the max supported value of %u",
> > +                                    priv->max_rx_buffer_size);
> > +             return -EINVAL;
> > +     }
> > +
> > +     if (rx_buf_len != SZ_2K && rx_buf_len != SZ_4K) {
> > +             NL_SET_ERR_MSG_MOD(extack,
> > +                                "Rx buf len can only be 2048 or 4096");
> > +             return -EINVAL;
> > +     }
> > +     rx_alloc_cfg->packet_buffer_size = rx_buf_len;
> > +
> > +     return 0;
> > +}

Re: [PATCH net-next 2/3] gve: Allow ethtool to configure rx_buf_len

[Jakub Kicinski]

On Fri, 24 Oct 2025 11:17:04 -0700 Ankit Garg wrote:
> > Please plumb extack thru to here. It's inside struct netdev_bpf
> 
> Using extack just for this log will make it inconsistent with other
> logs in this method. Would it be okay if I send a fast follow patch to
> use exstack in this method and others?

Could you make it part of this series, tho?

Re: [PATCH net-next 2/3] gve: Allow ethtool to configure rx_buf_len

[Ankit Garg]

On Fri, Oct 24, 2025 at 1:10 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Fri, 24 Oct 2025 11:17:04 -0700 Ankit Garg wrote:
> > > Please plumb extack thru to here. It's inside struct netdev_bpf
> >
> > Using extack just for this log will make it inconsistent with other
> > logs in this method. Would it be okay if I send a fast follow patch to
> > use exstack in this method and others?
>
> Could you make it part of this series, tho?

Absolutely. Will include in v2.

Re: [PATCH net-next 2/3] gve: Allow ethtool to configure rx_buf_len

[Ankit Garg]

On Fri, Oct 24, 2025 at 11:17 AM Ankit Garg <nktgrg@google.com> wrote:
>
> On Thu, Oct 23, 2025 at 5:14 PM Jakub Kicinski <kuba@kernel.org> wrote:
> >
> > On Wed, 22 Oct 2025 11:22:24 -0700 Joshua Washington wrote:
> > > +     if (priv->rx_cfg.packet_buffer_size != SZ_2K) {
> > > +             netdev_warn(dev,
> > > +                         "XDP is not supported for Rx buf len %d. Set Rx buf len to %d before using XDP.\n",
> > > +                         priv->rx_cfg.packet_buffer_size, SZ_2K);
> > > +             return -EOPNOTSUPP;
> > > +     }
> >
> > Please plumb extack thru to here. It's inside struct netdev_bpf
> >
>
> Using extack just for this log will make it inconsistent with other
> logs in this method. Would it be okay if I send a fast follow patch to
> use exstack in this method and others?
>
> > >       max_xdp_mtu = priv->rx_cfg.packet_buffer_size - sizeof(struct ethhdr);
> > >       if (priv->queue_format == GVE_GQI_QPL_FORMAT)
> > >               max_xdp_mtu -= GVE_RX_PAD;
> > > @@ -2050,6 +2057,44 @@ bool gve_header_split_supported(const struct gve_priv *priv)
> > >               priv->queue_format == GVE_DQO_RDA_FORMAT && !priv->xdp_prog;
> > >  }
> > >
> > > +int gve_set_rx_buf_len_config(struct gve_priv *priv, u32 rx_buf_len,
> > > +                           struct netlink_ext_ack *extack,
> > > +                           struct gve_rx_alloc_rings_cfg *rx_alloc_cfg)
> > > +{
> > > +     u32 old_rx_buf_len = rx_alloc_cfg->packet_buffer_size;
> > > +
> > > +     if (rx_buf_len == old_rx_buf_len)
> > > +             return 0;
> > > +
> > > +     if (!gve_is_dqo(priv)) {
> > > +             NL_SET_ERR_MSG_MOD(extack,
> > > +                                "Modifying Rx buf len is only supported with DQO format");
> > > +             return -EOPNOTSUPP;
> > > +     }
> > > +
> > > +     if (priv->xdp_prog && rx_buf_len != SZ_2K) {
> > > +             NL_SET_ERR_MSG_MOD(extack,
> > > +                                "Rx buf len can only be 2048 when XDP is on");
> > > +             return -EINVAL;
> > > +     }
> > > +
> > > +     if (rx_buf_len > priv->max_rx_buffer_size) {
> >
> > This check looks kinda pointless given the check right below against
> > the exact sizes?
> >
>
> My intent was to code defensively against device accidently advertising
> anything in [2k+1,4k) as max buffer size. I will remove this check.
>

After taking another look, an additional check is still needed to
handle scenario when device doesn't advertise support for 4k buffers.
I reworked this check (and added a comment) in v2 which hopefully
conveys the intent better.


> > > +             NL_SET_ERR_MSG_FMT_MOD(extack,
> > > +                                    "Rx buf len exceeds the max supported value of %u",
> > > +                                    priv->max_rx_buffer_size);
> > > +             return -EINVAL;
> > > +     }
> > > +
> > > +     if (rx_buf_len != SZ_2K && rx_buf_len != SZ_4K) {
> > > +             NL_SET_ERR_MSG_MOD(extack,
> > > +                                "Rx buf len can only be 2048 or 4096");
> > > +             return -EINVAL;
> > > +     }
> > > +     rx_alloc_cfg->packet_buffer_size = rx_buf_len;
> > > +
> > > +     return 0;
> > > +}