RE: [PATCH net-next v2] virtio-net: enable NETIF_F_GRO_HW only if GRO-related offloads are supported

[Zhud <zhud@hygon.cn>]

> Thanks! Yes something to improve:
> 
> On Mon, Mar 16, 2026 at 03:21:52PM +0800, Di Zhu wrote:
> > Although VIRTIO_NET_F_CTRL_GUEST_OFFLOADS is negotiated, which
> > indicates the device supports dynamic control of guest offloads, it
> > does not necessarily mean the device supports specific hardware GRO features.
> >
> > If none of the features defined in GUEST_OFFLOAD_GRO_HW_MASK (such as
> > TSO4, TSO6, or UFO) are present in vi->guest_offloads_capable, the
> > device effectively lacks the hardware capability to perform GRO.
> 
> So what is the user-visible problem this is trying to address?

A key concern is that once a user enables NETIF_F_GRO_HW via ethtool, 
they might manually disable software GRO (ethtool -K eth0 gro off) assuming the 
hardware is now handling the aggregation.

Secondly, while we haven't encountered a specific hardware failure yet, 
enabling a hardware offload feature that the DPU does not physically support 
introduces the risk of undefined hardware behavior

> >
> > So, making NETIF_F_GRO_HW conditional on these feature bits ensures
> > the stack does not enable an unsupported hardware offload configuration.
> 
> I guess the assumption is that without this, something enables such a config? Which
> stack is this and what happens then?
> 

Sorry for the confusion, let me clarify the intent.
The 'stack' here refers to the ethtool interface and the netset (ioctl/netlink) path. 

> 
> > Fixes: a02e8964eaf9 ("virtio-net: ethtool configurable LRO")
> > Signed-off-by: Di Zhu <zhud@hygon.cn>
> 
> judging by this, has something to do with LRO?
> 
> > ---
> > /* 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 |
> > @@ -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;
> > +
> >  	rtnl_unlock();
> >
> >  	err = virtnet_cpu_notif_add(vi);
> > --
> > 2.34.1
> >
>