On Thu, Oct 20, 2016 at 11:23:54PM +0300, Michael S. Tsirkin wrote:

On Thu, Oct 20, 2016 at 01:55:21PM -0400, Jarod Wilson wrote:

...

quoted

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index fad84f3..720809f 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c

@@ -1419,17 +1419,6 @@ static const struct ethtool_ops virtnet_ethtool_ops = {
 	.set_settings = virtnet_set_settings,
 };
 
-#define MIN_MTU 68
-#define MAX_MTU 65535
-
-static int virtnet_change_mtu(struct net_device *dev, int new_mtu)
-{
-	if (new_mtu < MIN_MTU || new_mtu > MAX_MTU)
-		return -EINVAL;
-	dev->mtu = new_mtu;
-	return 0;
-}
-
 static const struct net_device_ops virtnet_netdev = {
 	.ndo_open            = virtnet_open,
 	.ndo_stop   	     = virtnet_close,

@@ -1437,7 +1426,6 @@ static const struct net_device_ops virtnet_netdev = {
 	.ndo_validate_addr   = eth_validate_addr,
 	.ndo_set_mac_address = virtnet_set_mac_address,
 	.ndo_set_rx_mode     = virtnet_set_rx_mode,
-	.ndo_change_mtu	     = virtnet_change_mtu,
 	.ndo_get_stats64     = virtnet_stats,
 	.ndo_vlan_rx_add_vid = virtnet_vlan_rx_add_vid,
 	.ndo_vlan_rx_kill_vid = virtnet_vlan_rx_kill_vid,

@@ -1748,6 +1736,9 @@ static bool virtnet_validate_features(struct virtio_device *vdev)
 	return true;
 }
 
+#define MIN_MTU ETH_MIN_MTU
+#define MAX_MTU ETH_MAX_MTU
+

Can we drop these btw?

Bah. Yeah. Should have just used them directly. I didn't add ETH_MAX_MTU
until after doing the virtio_net changes, so I missed that.

quoted

 static int virtnet_probe(struct virtio_device *vdev)
 {
 	int i, err;

@@ -1821,6 +1812,10 @@ static int virtnet_probe(struct virtio_device *vdev)
 
 	dev->vlan_features = dev->features;
 
+	/* MTU range: 68 - 65535 */
+	dev->min_mtu = MIN_MTU;
+	dev->max_mtu = MAX_MTU;
+
 	/* Configuration may specify what MAC to use.  Otherwise random. */
 	if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC))
 		virtio_cread_bytes(vdev,

@@ -1875,8 +1870,10 @@ static int virtnet_probe(struct virtio_device *vdev)
 		mtu = virtio_cread16(vdev,
 				     offsetof(struct virtio_net_config,
 					      mtu));
-		if (virtnet_change_mtu(dev, mtu))
+		if (mtu < dev->min_mtu || mtu > dev->max_mtu)

In fact the > max_mtu branch does not make sense since a 16 bit
value can't exceed MAX_MTU.

Hm. mtu is declared as an int, not sure if there's any sort of type
promotion to be worried about (not an area I know much/anything about).
Certainly something that could be looked into as a minor optimization,
though it's only in a probe path and shouldn't hurt anything, so ... meh?

-- 
Jarod Wilson
jarod@redhat.com