Thread (46 messages) flat view 46 messages, 5 authors, 2014-06-25
STALE4457d

[PATCH RFC 21/30] net: fec: quiesce packet processing before changing features

From: Russell King <hidden>
Date: 2014-06-20 12:13:30
Also in: linux-arm-kernel
Subsystem: freescale imx / mxc fec driver, networking drivers, the rest · Maintainers: Wei Fang, Andrew Lunn, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds

Changing the features (receive checksumming) requires the hardware to be
reprogrammed, and also changes the checks in the receive packet
processing.

The current implementation has a race - fec_set_features() changes the
flags which alter the receive packet processing while the adapter is
active, and potentially receiving frames.  Only after we've modified
the software flag do we shutdown and reconfigure the hardware.

This can lead to packets being received and marked with a valid checksum
(via CHECKSUM_UNNECESSARY) when the hardware checksum validation has not
yet been enabled.

We must quiesce the device, then change the software configuration for
this feature, and then resume the device if it was previously running.

The resulting code structure also allows us to add other configuration
features in this path without having to quiesce and resume the network
interface and device.

Signed-off-by: Russell King <redacted>
---
 drivers/net/ethernet/freescale/fec_main.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index cc2fab7333fe..466268fffadd 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -2347,12 +2347,21 @@ static void fec_poll_controller(struct net_device *dev)
 }
 #endif
 
+#define FEATURES_NEED_QUIESCE NETIF_F_RXCSUM
+
 static int fec_set_features(struct net_device *netdev,
 	netdev_features_t features)
 {
 	struct fec_enet_private *fep = netdev_priv(netdev);
 	netdev_features_t changed = features ^ netdev->features;
 
+	/* Quiesce the device if necessary */
+	if (netif_running(netdev) && changed & FEATURES_NEED_QUIESCE) {
+		napi_disable(&fep->napi);
+		netif_tx_lock_bh(netdev);
+		fec_stop(netdev);
+	}
+
 	netdev->features = features;
 
 	/* Receive checksum has been changed */
@@ -2361,16 +2370,14 @@ static int fec_set_features(struct net_device *netdev,
 			fep->csum_flags |= FLAG_RX_CSUM_ENABLED;
 		else
 			fep->csum_flags &= ~FLAG_RX_CSUM_ENABLED;
+	}
 
-		if (netif_running(netdev)) {
-			napi_disable(&fep->napi);
-			netif_tx_lock_bh(netdev);
-			fec_stop(netdev);
-			fec_restart(netdev, fep->phy_dev->duplex);
-			netif_wake_queue(netdev);
-			netif_tx_unlock_bh(netdev);
-			napi_enable(&fep->napi);
-		}
+	/* Resume the device after updates */
+	if (netif_running(netdev) && changed & FEATURES_NEED_QUIESCE) {
+		fec_restart(netdev, fep->phy_dev->duplex);
+		netif_wake_queue(netdev);
+		netif_tx_unlock_bh(netdev);
+		napi_enable(&fep->napi);
 	}
 
 	return 0;
-- 
1.8.3.1
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help