Skip site navigation (1)Skip section navigation (2) 
Date:      Wed, 27 Nov 2013 10:59:08 -0700
From:      "Justin T. Gibbs" <gibbs@scsiguy.com>
To:        net@FreeBSD.org
Cc:        =?iso-8859-1?Q?Roger_Pau_Monn=E9?= <royger@FreeBSD.org>
Subject:   Defaults for if_capenable and detecting user initiated changes
Message-ID:  <0E13D481-9D6D-4B52-A5AD-B671BF3A85AF@scsiguy.com>
next in thread | raw e-mail | index | archive | help 

[-- Attachment #1 --]
Hi net,

I’m reviewing a patch from Roger Pau Monné for the Xen netfront driver.  The goal of the change is to avoid disturbing the user’s settings for the interface just because the backend device has changed or the connection to the backend was reset.  I’ve attached the latest version of the patch.

The current patch leaves the interface settings alone if they can be supported by the newly attached backend.  What would be ideal is to enable capabilities that default to being enabled if they were not explicitly disabled by the user and can be supported by the new backend.  Unfortunately, I don’t think the if_capenable and if_capabilities fields are descriptive enough to deal with an interface whose capabilities can change at runtime.  Just as can be done with link speed, some of these settings need to allow an “auto/default” setting in addition to on or off.  This would allow the user to explicitly disable a capability if needed, but generally allow the system to chose the most optimal settings when they are supported.  Would this be difficult to add?

Thanks,
Justin


[-- Attachment #2 --]
From a660c9e9a3903d72c351e0fb7a890132efff6b7d Mon Sep 17 00:00:00 2001
From: Roger Pau Monne <roger.pau@citrix.com>
Date: Mon, 21 Oct 2013 16:00:15 +0100
Subject: [PATCH] xen-netfront: keep xn options on suspend/resume

Specific xn features should be preserved across migrations when
possible (if the destination host also supports them). Previously if
the user disabled a feature, it would be automatically re-enabled
after resume if the destination host supports it.
---
 sys/dev/xen/netfront/netfront.c |   32 ++++++++++++++++++++++++++------
 1 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/sys/dev/xen/netfront/netfront.c b/sys/dev/xen/netfront/netfront.c
index c0ad393..f5bce33 100644
--- a/sys/dev/xen/netfront/netfront.c
+++ b/sys/dev/xen/netfront/netfront.c
@@ -287,6 +287,8 @@ struct netfront_info {
 	multicall_entry_t	rx_mcl[NET_RX_RING_SIZE+1];
 	mmu_update_t		rx_mmu[NET_RX_RING_SIZE];
 	struct ifmedia		sc_media;
+
+	bool			xn_cold;
 };
 
 #define rx_mbufs xn_cdata.xn_rx_chain
@@ -1963,6 +1965,7 @@ network_connect(struct netfront_info *np)
 	xn_txeof(np);
 	XN_TX_UNLOCK(np);
 	network_alloc_rx_buffers(np);
+	np->xn_cold = false;
 
 	return (0);
 }
@@ -2003,13 +2006,28 @@ xn_configure_features(struct netfront_info *np)
 	int err;
 
 	err = 0;
-#if __FreeBSD_version >= 70000 && (defined(INET) || defined(INET6))
-	if ((np->xn_ifp->if_capenable & IFCAP_LRO) != 0)
-		tcp_lro_free(&np->xn_lro);
+
+	if (np->xn_cold ||
+	    ((np->xn_ifp->if_capenable & np->xn_ifp->if_capabilities)
+	    != np->xn_ifp->if_capenable)) {
+		/*
+		 * Check if current enabled capabilities are available,
+		 * if not switch to default capabilities.
+		 */
+#if __FreeBSD_version >= 700000 && (defined(INET) || defined(INET6))
+		if ((np->xn_ifp->if_capenable & IFCAP_LRO) != 0)
+			tcp_lro_free(&np->xn_lro);
 #endif
-    	np->xn_ifp->if_capenable =
-	    np->xn_ifp->if_capabilities & ~(IFCAP_LRO|IFCAP_TSO4);
-	np->xn_ifp->if_hwassist &= ~CSUM_TSO;
+		np->xn_ifp->if_capenable =
+			np->xn_ifp->if_capabilities & ~(IFCAP_LRO|IFCAP_TSO4);
+		np->xn_ifp->if_hwassist &= ~CSUM_TSO;
+	} else {
+		/*
+		 * What we have currently enabled is supported by the
+		 * new host, no need to change anything.
+		 */
+		return 0;
+	}
 #if __FreeBSD_version >= 700000 && (defined(INET) || defined(INET6))
 	if (xn_enable_lro && (np->xn_ifp->if_capabilities & IFCAP_LRO) != 0) {
 		err = tcp_lro_init(&np->xn_lro);
@@ -2054,6 +2072,8 @@ create_netdev(device_t dev)
 	np->rx_min_target = RX_MIN_TARGET;
 	np->rx_max_target = RX_MAX_TARGET;
 
+	np->xn_cold = true;
+
 	/* Initialise {tx,rx}_skbs to be a free chain containing every entry. */
 	for (i = 0; i <= NET_TX_RING_SIZE; i++) {
 		np->tx_mbufs[i] = (void *) ((u_long) i+1);
-- 
1.7.7.5 (Apple Git-26)

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?0E13D481-9D6D-4B52-A5AD-B671BF3A85AF>