Re: [PATCH] netback: allow arbitrary mtu size until frontend connects

[Olaf Hering <olaf@aepfle.de>]

On Mon, Feb 07, Ian Campbell wrote:

> I think for the sake of not fragmenting any more than necessary a
> backport would be preferable to adding new code to the old trees.


Allow arbitrary mtu size until the frontend is connected.  Once the
connection is established, adjust mtu by checking if the backend
supports the 'feature-sg'.  If the backend does support it, keep the
current mtu. Otherwise set it to the default value, which is 1500.

based on two commits from
https://git.kernel.org/?p=linux/kernel/git/jeremy/xen.git

bee2eec2355c4bf4e149a426d5e30527162de566

This allows large MTU to be configured by the VIF hotplug
script. Previously this would fail because at the point the hotplug
script runs the VIF features have most likely not been negotiated with
the frontend and so SG has not yet been enabled. Invert this behaviour
so that SG is assumed present until negotiations prove otherwise and
reduce MTU at that point.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

d532fa93d4eeabbfc0176a6a9a93b0d6ade3f6c4

Make sure that if a feature flag is disabled by ethtool on netback
that we do not gratuitously re-enabled it when we check the frontend
features during ring connection.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
Signed-off-by: Olaf Hering <olaf@aepfle.de>

---
 drivers/xen/netback/common.h    |   12 +++++--
 drivers/xen/netback/interface.c |   64 +++++++++++++++++++++++++++++++++-------
 drivers/xen/netback/netback.c   |    2 -
 drivers/xen/netback/xenbus.c    |   29 +++++++-----------
 4 files changed, 76 insertions(+), 31 deletions(-)

Index: linux-2.6.18-xen.hg/drivers/xen/netback/common.h
===================================================================
--- linux-2.6.18-xen.hg.orig/drivers/xen/netback/common.h
+++ linux-2.6.18-xen.hg/drivers/xen/netback/common.h
@@ -75,8 +75,13 @@ typedef struct netif_st {
 	struct vm_struct *tx_comms_area;
 	struct vm_struct *rx_comms_area;
 
-	/* Set of features that can be turned on in dev->features. */
-	int features;
+	/* Flags that must not be set in dev->features */
+	int features_disabled;
+
+	/* Frontend feature information. */
+	u8 can_sg:1;
+	u8 gso:1;
+	u8 csum:1;
 
 	/* Internal feature information. */
 	u8 can_queue:1;	/* can queue packets for receiver? */
@@ -182,6 +187,7 @@ void netif_accel_init(void);
 
 void netif_disconnect(netif_t *netif);
 
+void netif_set_features(netif_t *netif);
 netif_t *netif_alloc(struct device *parent, domid_t domid, unsigned int handle);
 int netif_map(netif_t *netif, unsigned long tx_ring_ref,
 	      unsigned long rx_ring_ref, unsigned int evtchn);
@@ -214,7 +220,7 @@ static inline int netbk_can_queue(struct
 static inline int netbk_can_sg(struct net_device *dev)
 {
 	netif_t *netif = netdev_priv(dev);
-	return netif->features & NETIF_F_SG;
+	return netif->can_sg;
 }
 
 #endif /* __NETIF__BACKEND__COMMON_H__ */
Index: linux-2.6.18-xen.hg/drivers/xen/netback/interface.c
===================================================================
--- linux-2.6.18-xen.hg.orig/drivers/xen/netback/interface.c
+++ linux-2.6.18-xen.hg/drivers/xen/netback/interface.c
@@ -93,28 +93,69 @@ static int netbk_change_mtu(struct net_d
 	return 0;
 }
 
-static int netbk_set_sg(struct net_device *dev, u32 data)
+void netif_set_features(netif_t *netif)
+{
+	struct net_device *dev = netif->dev;
+	int features = dev->features;
+
+	if (netif->can_sg)
+		features |= NETIF_F_SG;
+	if (netif->gso)
+		features |= NETIF_F_TSO;
+	if (netif->csum)
+		features |= NETIF_F_IP_CSUM;
+
+	features &= ~(netif->features_disabled);
+
+	if (!(features & NETIF_F_SG) && dev->mtu > ETH_DATA_LEN)
+		dev->mtu = ETH_DATA_LEN;
+
+	dev->features = features;
+}
+
+static int netbk_set_tx_csum(struct net_device *dev, u32 data)
 {
+	netif_t *netif = netdev_priv(dev);
 	if (data) {
-		netif_t *netif = netdev_priv(dev);
+		if (!netif->csum)
+			return -ENOSYS;
+		netif->features_disabled &= ~NETIF_F_IP_CSUM;
+	} else {
+		netif->features_disabled |= NETIF_F_IP_CSUM;
+	}
 
-		if (!(netif->features & NETIF_F_SG))
+	netif_set_features(netif);
+	return 0;
+}
+
+static int netbk_set_sg(struct net_device *dev, u32 data)
+{
+	netif_t *netif = netdev_priv(dev);
+	if (data) {
+		if (!netif->can_sg)
 			return -ENOSYS;
+		netif->features_disabled &= ~NETIF_F_SG;
+	} else {
+		netif->features_disabled |= NETIF_F_SG;
 	}
 
-	return ethtool_op_set_sg(dev, data);
+	netif_set_features(netif);
+	return 0;
 }
 
 static int netbk_set_tso(struct net_device *dev, u32 data)
 {
+	netif_t *netif = netdev_priv(dev);
 	if (data) {
-		netif_t *netif = netdev_priv(dev);
-
-		if (!(netif->features & NETIF_F_TSO))
+		if (!netif->gso)
 			return -ENOSYS;
+		netif->features_disabled &= ~NETIF_F_TSO;
+	} else {
+		netif->features_disabled |= NETIF_F_TSO;
 	}
 
-	return ethtool_op_set_tso(dev, data);
+	netif_set_features(netif);
+	return 0;
 }
 
 static void netbk_get_drvinfo(struct net_device *dev,
@@ -164,7 +205,7 @@ static struct ethtool_ops network_ethtoo
 	.get_drvinfo = netbk_get_drvinfo,
 
 	.get_tx_csum = ethtool_op_get_tx_csum,
-	.set_tx_csum = ethtool_op_set_tx_csum,
+	.set_tx_csum = netbk_set_tx_csum,
 	.get_sg = ethtool_op_get_sg,
 	.set_sg = netbk_set_sg,
 	.get_tso = ethtool_op_get_tso,
@@ -196,6 +237,8 @@ netif_t *netif_alloc(struct device *pare
 	memset(netif, 0, sizeof(*netif));
 	netif->domid  = domid;
 	netif->handle = handle;
+	netif->can_sg = 1;
+	netif->csum = 1;
 	atomic_set(&netif->refcnt, 1);
 	init_waitqueue_head(&netif->waiting_to_free);
 	netif->dev = dev;
@@ -215,7 +258,8 @@ netif_t *netif_alloc(struct device *pare
 	dev->open            = net_open;
 	dev->stop            = net_close;
 	dev->change_mtu	     = netbk_change_mtu;
-	dev->features        = NETIF_F_IP_CSUM;
+
+	netif_set_features(netif);
 
 	SET_ETHTOOL_OPS(dev, &network_ethtool_ops);
 
Index: linux-2.6.18-xen.hg/drivers/xen/netback/netback.c
===================================================================
--- linux-2.6.18-xen.hg.orig/drivers/xen/netback/netback.c
+++ linux-2.6.18-xen.hg/drivers/xen/netback/netback.c
@@ -269,7 +269,7 @@ static struct sk_buff *netbk_copy_skb(st
 
 static inline int netbk_max_required_rx_slots(netif_t *netif)
 {
-	if (netif->features & (NETIF_F_SG|NETIF_F_TSO))
+	if (netif->can_sg || netif->gso)
 		return MAX_SKB_FRAGS + 2; /* header + extra_info + frags */
 	return 1; /* all in one */
 }
Index: linux-2.6.18-xen.hg/drivers/xen/netback/xenbus.c
===================================================================
--- linux-2.6.18-xen.hg.orig/drivers/xen/netback/xenbus.c
+++ linux-2.6.18-xen.hg/drivers/xen/netback/xenbus.c
@@ -356,6 +356,7 @@ static void connect(struct backend_info
 
 static int connect_rings(struct backend_info *be)
 {
+	netif_t *netif = be->netif;
 	struct xenbus_device *dev = be->dev;
 	unsigned long tx_ring_ref, rx_ring_ref;
 	unsigned int evtchn, rx_copy;
@@ -386,44 +387,38 @@ static int connect_rings(struct backend_
 				 dev->otherend);
 		return err;
 	}
-	be->netif->copying_receiver = !!rx_copy;
+	netif->copying_receiver = !!rx_copy;
 
-	if (be->netif->dev->tx_queue_len != 0) {
+	if (netif->dev->tx_queue_len != 0) {
 		if (xenbus_scanf(XBT_NIL, dev->otherend,
 				 "feature-rx-notify", "%d", &val) < 0)
 			val = 0;
 		if (val)
-			be->netif->can_queue = 1;
+			netif->can_queue = 1;
 		else
 			/* Must be non-zero for pfifo_fast to work. */
-			be->netif->dev->tx_queue_len = 1;
+			netif->dev->tx_queue_len = 1;
 	}
 
 	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-sg", "%d", &val) < 0)
 		val = 0;
-	if (val) {
-		be->netif->features |= NETIF_F_SG;
-		be->netif->dev->features |= NETIF_F_SG;
-	}
+	netif->can_sg = !!val;
 
 	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-gso-tcpv4", "%d",
 			 &val) < 0)
 		val = 0;
-	if (val) {
-		be->netif->features |= NETIF_F_TSO;
-		be->netif->dev->features |= NETIF_F_TSO;
-	}
+	netif->gso = !!val;
 
 	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-no-csum-offload",
 			 "%d", &val) < 0)
 		val = 0;
-	if (val) {
-		be->netif->features &= ~NETIF_F_IP_CSUM;
-		be->netif->dev->features &= ~NETIF_F_IP_CSUM;
-	}
+	netif->csum = !val;
+
+	/* Set dev->features */
+	netif_set_features(netif);
 
 	/* Map the shared frame, irq etc. */
-	err = netif_map(be->netif, tx_ring_ref, rx_ring_ref, evtchn);
+	err = netif_map(netif, tx_ring_ref, rx_ring_ref, evtchn);
 	if (err) {
 		xenbus_dev_fatal(dev, err,
 				 "mapping shared-frames %lu/%lu port %u",