On Tue, Mar 17, 2026 at 07:34:33PM +0800, Di Zhu wrote:

quoted

Negotiating VIRTIO_NET_F_CTRL_GUEST_OFFLOADS indicates the device
allows control over offload support, but the offloads that can be
controlled may have nothing to do with GRO (e.g., if neither
GUEST_TSO4 nor GUEST_TSO6 is supported).

In such a setup, reporting NETIF_F_GRO_HW as available for the device
is too optimistic and misleading to the user.

Improve the situation by masking off NETIF_F_GRO_HW unless the device
possesses actual GRO-related offload capabilities. Out of an abundance
of caution, this does not change the current behaviour for hardware
with just v6 or just v4 GRO: current interfaces do not allow
distinguishing between v6/v4 GRO, so we can't expose them to userspace

precisely.

quoted

Fixes: dbcf24d15388 ("virtio-net: use NETIF_F_GRO_HW instead of
NETIF_F_LRO")
Signed-off-by: Di Zhu <redacted>
---
/* v3 */
  -Update Fixes tag to dbcf24d15388
  -Refine commit message using Maintainer's "too optimistic"
    phrasing to clarify the risk of misleading configurations.

/* v2 */
  -make the modified logic clearer
---
 drivers/net/virtio_net.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index

72d6a9c6a5a2..b233c99925e9 100644

--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c

@@ -6781,8 +6781,6 @@ static int virtnet_probe(struct virtio_device *vdev)
 	if (virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_TSO4) ||
 	    virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_TSO6))
 		dev->features |= NETIF_F_GRO_HW;
-	if (virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS))
-		dev->hw_features |= NETIF_F_GRO_HW;

 	dev->vlan_features = dev->features;
 	dev->xdp_features = NETDEV_XDP_ACT_BASIC |

NETDEV_XDP_ACT_REDIRECT |

quoted

@@ -7058,6 +7056,10 @@ static int virtnet_probe(struct virtio_device *vdev)
 	}
 	vi->guest_offloads_capable = vi->guest_offloads;

+	if (virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS) &&
+	    (vi->guest_offloads_capable & GUEST_OFFLOAD_GRO_HW_MASK))
+		dev->hw_features |= NETIF_F_GRO_HW;
+

Hmm wait a second. Isn't this late? netdev registered, even link is up already.

Documentation/networking/netdev-features.rst says:

 1. netdev->hw_features set contains features whose state may possibly
    be changed (enabled or disabled) for a particular device by user's
    request.  This set should be initialized in ndo_init callback and not
    changed later.



I'd say let's move it up?

"Thanks for the feedback. My initial thought was that performing the modification under 
rtnl_lock after register_netdevice() would be safe from races with ethtool or link-up. However, 
I agree that staying consistent with the documentation and initializing hw_features before the device
becomes globally visible is a much cleaner approach.

Having said that, we already have a bug:

  dev->hw_features &= ~NETIF_F_RXHASH;
is after register, too.

To be honest, I was misled by the existing code where dev->hw_features &= ~NETIF_F_RXHASH; 
is also set after registration.

This is because we take pains to recover from these commands failing.
Except, we completely ignore that in virtnet_set_rxfh ...


Cc relevant contributors to maybe take a look at fixing all that.

quoted

 	rtnl_unlock();

 	err = virtnet_cpu_notif_add(vi);
--
2.34.1