Netdev List
 help / color / mirror / Atom feed
* [PATCH net v2] bnge: add missing ndo_set_features
@ 2026-08-27 16:12 Vikas Gupta
  2026-08-27 16:42 ` Jakub Kicinski
  0 siblings, 1 reply; 2+ messages in thread
From: Vikas Gupta @ 2026-08-27 16:12 UTC (permalink / raw)
  To: davem, edumazet, kuba, pabeni, andrew+netdev, horms
  Cc: netdev, linux-kernel, bhargava.marreddy, rahul-rg.gupta,
	vsrama-krishna.nemani, rajashekar.hudumula, dharmender.garg,
	ajit.khaparde, Vikas Gupta

The driver advertises togglable RX aggregation offloads (LRO and
GRO_HW) in netdev->hw_features but is missing the ndo_set_features
callback, so it is never notified of a feature change.

For example:

  ethtool -K ethX rx-gro-hw off

is accepted - netdev_update_features() clears the bit in dev->features
but the driver never reprograms the hardware, so the TPA engine keeps
aggregating and dev->features no longer reflects the actual hardware
state.

Add ndo_set_features to reprogram TPA (LRO/GRO_HW) and the RX ring
parameters when these features change.

Fixes: c2effd12c96d ("bng_en: Add support for TPA events")
Signed-off-by: Vikas Gupta <vikas.gupta@broadcom.com>
Reviewed-by: Bhargava Chenna Marreddy <bhargava.marreddy@broadcom.com>
---
v2:
 - Sashiko's points addressed.
  Sending FUNC_DRV_IF_CHANGE_REQ_FLAGS_UP twice is safe and no DOWN is
  ever skipped. Reworking the handshake so if_change sits only at the
  ndo_open/ndo_stop boundary is a reasonable cleanup, but it's out of
  scope for this patch and will be handled in a follow-up.

 .../net/ethernet/broadcom/bnge/bnge_netdev.c  | 42 +++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c b/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
index a4288f0258f8..13573a2b78ef 100644
--- a/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
+++ b/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
@@ -3155,6 +3155,9 @@ static void bnge_close_core(struct bnge_net *bn)
 {
 	struct bnge_dev *bd = bn->bd;
 
+	if (!bn->bnapi)
+		return;
+
 	bnge_tx_disable(bn);
 
 	clear_bit(BNGE_STATE_OPEN, &bd->state);
@@ -3267,6 +3270,44 @@ static const struct netdev_stat_ops bnge_stat_ops = {
 	.get_base_stats		= bnge_get_base_stats,
 };
 
+static int bnge_set_features(struct net_device *dev, netdev_features_t features)
+{
+	struct bnge_net *bn = netdev_priv(dev);
+	struct bnge_dev *bd = bn->bd;
+	u32 old_flags = bn->priv_flags;
+	u32 flags = old_flags;
+	int rc;
+
+	flags &= ~BNGE_NET_EN_TPA;
+	if (features & NETIF_F_GRO_HW)
+		flags |= BNGE_NET_EN_GRO;
+	else if (features & NETIF_F_LRO)
+		flags |= BNGE_NET_EN_LRO;
+
+	if (flags == old_flags)
+		return 0;
+
+	if (!netif_running(dev)) {
+		bn->priv_flags = flags;
+		bnge_set_ring_params(bd);
+		return 0;
+	}
+
+	bnge_close_core(bn);
+	bn->priv_flags = flags;
+	bnge_set_ring_params(bd);
+
+	rc = bnge_open_core(bn);
+	if (rc) {
+		netdev_err(dev, "bnge_open_core err: %d\n", rc);
+		bn->priv_flags = old_flags;
+		bnge_set_ring_params(bd);
+		netif_close(dev);
+	}
+
+	return rc;
+}
+
 static const struct net_device_ops bnge_netdev_ops = {
 	.ndo_open		= bnge_open,
 	.ndo_stop		= bnge_close,
@@ -3274,6 +3315,7 @@ static const struct net_device_ops bnge_netdev_ops = {
 	.ndo_get_stats64	= bnge_get_stats64,
 	.ndo_set_rx_mode_async	= bnge_set_rx_mode,
 	.ndo_features_check	= bnge_features_check,
+	.ndo_set_features	= bnge_set_features,
 };
 
 static void bnge_init_mac_addr(struct bnge_dev *bd)
-- 
2.47.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH net v2] bnge: add missing ndo_set_features
  2026-08-27 16:12 [PATCH net v2] bnge: add missing ndo_set_features Vikas Gupta
@ 2026-08-27 16:42 ` Jakub Kicinski
  0 siblings, 0 replies; 2+ messages in thread
From: Jakub Kicinski @ 2026-08-27 16:42 UTC (permalink / raw)
  To: Vikas Gupta
  Cc: davem, edumazet, pabeni, andrew+netdev, horms, netdev,
	linux-kernel, bhargava.marreddy, rahul-rg.gupta,
	vsrama-krishna.nemani, rajashekar.hudumula, dharmender.garg,
	ajit.khaparde

On Thu, 27 Aug 2026 21:42:30 +0530 Vikas Gupta wrote:
> +	bnge_close_core(bn);
> +	bn->priv_flags = flags;
> +	bnge_set_ring_params(bd);
> +
> +	rc = bnge_open_core(bn);
> +	if (rc) {
> +		netdev_err(dev, "bnge_open_core err: %d\n", rc);
> +		bn->priv_flags = old_flags;
> +		bnge_set_ring_params(bd);
> +		netif_close(dev);

We have been requiring NIC reconfiguration to be resilient against
OOM conditions for years now. bnxt is even older so it got
grandfathered in, but bnge has to do it right. So we will reject
any patch that tries to close / reopen and hope for the best.

If you want a net fix - you should reflect current operation
of the driver - meaning make GRO-HW either forced on or forced off.
(Forced on by leaving it in the netdev->features but not in
->hw_features). Or reject changes while the device is UP
-- 
pw-bot: cr

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-08-27 16:42 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-27 16:12 [PATCH net v2] bnge: add missing ndo_set_features Vikas Gupta
2026-08-27 16:42 ` Jakub Kicinski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox