virtio-net: ethtool configurable LRO
Subsystems (3)
PATCH v2 1/1 virtio-net: ethtool configurable LRO
From: Willem de Bruijn <[email protected]>
Virtio-net devices negotiate LRO support with the host. Display the initially negotiated state with ethtool -k.
Also allow configuring it with ethtool -K, reusing the existing virtnet_set_guest_offloads helper that configures LRO for XDP. This is conditional on VIRTIO_NET_F_CTRL_GUEST_OFFLOADS.
Virtio-net negotiates TSO4 and TSO6 separately, but ethtool does not distinguish between the two. Display LRO as on only if any offload is active.
RTNL is held while calling virtnet_set_features, same as on the path from virtnet_xdp_set.
Changes v1 -> v2
- allow ethtool config (-K) only if VIRTIO_NET_F_CTRL_GUEST_OFFLOADS
- show LRO as enabled if any LRO variant is enabled
- do not allow configuration while XDP is active
- differentiate current features from the capable set, to restore on XDP down only those features that were active on XDP up
- move test out of VIRTIO_NET_F_CSUM/TSO branch, which is tx only
--- drivers/net/virtio_net.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+)
drivers/net/virtio_net.c +33 -0
@@ -236,6 +236,7 @@ struct virtnet_info {u32 speed;unsigned long guest_offloads; +unsigned long guest_offloads_capable;/* failover when STANDBY feature enabled */ struct failover *failover;
@@ -2479,6 +2480,31 @@ static int virtnet_get_phys_port_name(struct net_device *dev, char *buf,return 0 ; } +static int virtnet_set_features (struct net_device *dev, +netdev_features_t features) +{ +struct virtnet_info *vi =netdev_priv (dev); +u64 offloads; +int err; + +if ((dev->features ^ features) &NETIF_F_LRO ) { +if (vi->xdp_queue_pairs) +return -EBUSY ; + +if (features &NETIF_F_LRO ) + offloads = vi->guest_offloads_capable; +else + offloads =0 ; + + err =virtnet_set_guest_offloads (vi, offloads); +if (err) +return err; + vi->guest_offloads = offloads; + } + +return 0 ; +} +static const struct net_device_ops virtnet_netdev = { .ndo_open = virtnet_open, .ndo_stop = virtnet_close,
@@ -2493,6 +2519,7 @@ static const struct net_device_ops virtnet_netdev = { .ndo_xdp_xmit = virtnet_xdp_xmit, .ndo_features_check = passthru_features_check, .ndo_get_phys_port_name = virtnet_get_phys_port_name, + .ndo_set_features = virtnet_set_features, };static void virtnet_config_changed_work (struct work_struct *work)
@@ -2951,6 +2978,11 @@ static int virtnet_probe(struct virtio_device *vdev) }if (virtio_has_feature (vdev,VIRTIO_NET_F_GUEST_CSUM )) dev->features |=NETIF_F_RXCSUM ; +if (virtio_has_feature (vdev,VIRTIO_NET_F_GUEST_TSO4 ) || +virtio_has_feature (vdev,VIRTIO_NET_F_GUEST_TSO6 )) + dev->features |=NETIF_F_LRO ; +if (virtio_has_feature (vdev,VIRTIO_NET_F_CTRL_GUEST_OFFLOADS )) + dev->hw_features |=NETIF_F_LRO ; dev->vlan_features = dev->features;
@@ -3080,6 +3112,7 @@ static int virtnet_probe(struct virtio_device *vdev)for (i =0 ; i <ARRAY_SIZE (guest_offloads); i++)if (virtio_has_feature (vi->vdev, guest_offloads[i]))set_bit (guest_offloads[i], &vi->guest_offloads); + vi->guest_offloads_capable = vi->guest_offloads;pr_debug ("virtnet: registered device %s with %d RX and TX vq's\n" , dev->name, max_queue_pairs);
Signature · 1 line
2.20.1.415.g653613c723-goog
Quoted 16 lines · Willem de Bruijn
From: Willem de Bruijn <[email protected]>
Virtio-net devices negotiate LRO support with the host. Display the initially negotiated state with ethtool -k.
Also allow configuring it with ethtool -K, reusing the existing
virtnet_set_guest_offloadshelper that configures LRO for XDP. This is conditional onVIRTIO_NET_F_CTRL_GUEST_OFFLOADS.Virtio-net negotiates TSO4 and TSO6 separately, but ethtool does not distinguish between the two. Display LRO as on only if any offload is active.
RTNL is held while calling
virtnet_set_features, same as on the path fromvirtnet_xdp_set.
Quoted 7 lines
Changes v1 -> v2
- allow ethtool config (-K) only ifVIRTIO_NET_F_CTRL_GUEST_OFFLOADS
- show LRO as enabled if any LRO variant is enabled
- do not allow configuration while XDP is active
- differentiate current features from the capable set, to restore on XDP down only those features that were active on XDP up
- move test out of VIRTIO_NET_F_CSUM/TSO branch, which is tx only
This part shouldn't be in the commit log, right? Should be after "---".
Quoted 1 line
Signed-off-by Willem de Bruijn <[email protected]>
Quoted 78 lines
--- drivers/net/virtio_net.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+)drivers/net/virtio_net.c +33
-0@@ -236,6 +236,7 @@ struct virtnet_info {u32 speed;unsigned long guest_offloads; +unsigned long guest_offloads_capable;/* failover when STANDBY feature enabled */ struct failover *failover;@@ -2479,6 +2480,31 @@ static int virtnet_get_phys_port_name(struct net_device *dev, char *buf,return 0 ; } +static int virtnet_set_features (struct net_device *dev, +netdev_features_t features) +{ +struct virtnet_info *vi =netdev_priv (dev); +u64 offloads; +int err; + +if ((dev->features ^ features) &NETIF_F_LRO ) { +if (vi->xdp_queue_pairs) +return -EBUSY ; + +if (features &NETIF_F_LRO ) + offloads = vi->guest_offloads_capable; +else + offloads =0 ; + + err =virtnet_set_guest_offloads (vi, offloads); +if (err) +return err; + vi->guest_offloads = offloads; + } + +return 0 ; +} +static const struct net_device_ops virtnet_netdev = { .ndo_open = virtnet_open, .ndo_stop = virtnet_close,@@ -2493,6 +2519,7 @@ static const struct net_device_ops virtnet_netdev = { .ndo_xdp_xmit = virtnet_xdp_xmit, .ndo_features_check = passthru_features_check, .ndo_get_phys_port_name = virtnet_get_phys_port_name, + .ndo_set_features = virtnet_set_features, };static void virtnet_config_changed_work (struct work_struct *work)@@ -2951,6 +2978,11 @@ static int virtnet_probe(struct virtio_device *vdev) }if (virtio_has_feature (vdev,VIRTIO_NET_F_GUEST_CSUM )) dev->features |=NETIF_F_RXCSUM ; +if (virtio_has_feature (vdev,VIRTIO_NET_F_GUEST_TSO4 ) || +virtio_has_feature (vdev,VIRTIO_NET_F_GUEST_TSO6 )) + dev->features |=NETIF_F_LRO ; +if (virtio_has_feature (vdev,VIRTIO_NET_F_CTRL_GUEST_OFFLOADS )) + dev->hw_features |=NETIF_F_LRO ; dev->vlan_features = dev->features;@@ -3080,6 +3112,7 @@ static int virtnet_probe(struct virtio_device *vdev)for (i =0 ; i <ARRAY_SIZE (guest_offloads); i++)if (virtio_has_feature (vi->vdev, guest_offloads[i]))set_bit (guest_offloads[i], &vi->guest_offloads); + vi->guest_offloads_capable = vi->guest_offloads;pr_debug ("virtnet: registered device %s with %d RX and TX vq's\n" , dev->name, max_queue_pairs);Signature · 1 line
2.20.1.415.g653613c723-goog
Quoted 26 lines · Michael S. Tsirkin
Quoted 16 lines · Willem de Bruijn
From: Willem de Bruijn <[email protected]>
Virtio-net devices negotiate LRO support with the host. Display the initially negotiated state with ethtool -k.
Also allow configuring it with ethtool -K, reusing the existing
virtnet_set_guest_offloadshelper that configures LRO for XDP. This is conditional onVIRTIO_NET_F_CTRL_GUEST_OFFLOADS.Virtio-net negotiates TSO4 and TSO6 separately, but ethtool does not distinguish between the two. Display LRO as on only if any offload is active.
RTNL is held while calling
virtnet_set_features, same as on the path fromvirtnet_xdp_set.Quoted 7 lines
Changes v1 -> v2
- allow ethtool config (-K) only ifVIRTIO_NET_F_CTRL_GUEST_OFFLOADS
- show LRO as enabled if any LRO variant is enabled
- do not allow configuration while XDP is active
- differentiate current features from the capable set, to restore on XDP down only those features that were active on XDP up
- move test out of VIRTIO_NET_F_CSUM/TSO branch, which is tx only
Quoted 1 line
This part shouldn't be in the commit log, right? Should be after "---".
I believe that David prefers to have this recorded, so that anyone stumbling on the patch later (e.g., through git blame) understands why we made certain choices.
Quoted 3 lines
Quoted 1 line
Signed-off-by Willem de Bruijn <[email protected]>
Thanks for the super fast review!
From: "Michael S. Tsirkin" <[email protected]>
Date: Thu, 20 Dec 2018 17:34:23 -0500
Quoted 25 lines · Willem de Bruijn
Quoted 16 lines
From: Willem de Bruijn <[email protected]>
Virtio-net devices negotiate LRO support with the host. Display the initially negotiated state with ethtool -k.
Also allow configuring it with ethtool -K, reusing the existing
virtnet_set_guest_offloadshelper that configures LRO for XDP. This is conditional onVIRTIO_NET_F_CTRL_GUEST_OFFLOADS.Virtio-net negotiates TSO4 and TSO6 separately, but ethtool does not distinguish between the two. Display LRO as on only if any offload is active.
RTNL is held while calling
virtnet_set_features, same as on the path fromvirtnet_xdp_set.Quoted 7 lines
Changes v1 -> v2
- allow ethtool config (-K) only ifVIRTIO_NET_F_CTRL_GUEST_OFFLOADS
- show LRO as enabled if any LRO variant is enabled
- do not allow configuration while XDP is active
- differentiate current features from the capable set, to restore on XDP down only those features that were active on XDP up
- move test out of VIRTIO_NET_F_CSUM/TSO branch, which is tx only
Quoted 1 line
This part shouldn't be in the commit log, right? Should be after "---".
As Willem stated, I really like to see the changelog in the commit message proper. It is just my taste, because I have on countless times relied on that information when reading over old changes.
And I know it helps other people too.
Quoted 3 lines
Quoted 1 line
Signed-off-by Willem de Bruijn <[email protected]>
Applied, thanks for reviewing.