From 2291982e29b1aa6e628d3072ef533aeb6299945f Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Tue, 29 Nov 2022 16:12:19 +0200 Subject: net: dpaa2-eth: serialize changes to priv->mac with a mutex The dpaa2 architecture permits dynamic connections between objects on the fsl-mc bus, specifically between a DPNI object (represented by a struct net_device) and a DPMAC object (represented by a struct phylink). The DPNI driver is notified when those connections are created/broken through the dpni_irq0_handler_thread() method. To ensure that ethtool operations, as well as netdev up/down operations serialize with the connection/disconnection of the DPNI with a DPMAC, dpni_irq0_handler_thread() takes the rtnl_lock() to block those other operations from taking place. There is code called by dpaa2_mac_connect() which wants to acquire the rtnl_mutex once again, see phylink_create() -> phylink_register_sfp() -> sfp_bus_add_upstream() -> rtnl_lock(). So the strategy doesn't quite work out, even though it's fairly simple. Create a different strategy, where all code paths in the dpaa2-eth driver access priv->mac only while they are holding priv->mac_lock. The phylink instance is not created or connected to the PHY under the priv->mac_lock, but only assigned to priv->mac then. This will eliminate the reliance on the rtnl_mutex. Add lockdep annotations and put comments where holding the lock is not necessary, and priv->mac can be dereferenced freely. Signed-off-by: Vladimir Oltean Reviewed-by: Ioana Ciornei Tested-by: Ioana Ciornei Signed-off-by: Paolo Abeni --- .../net/ethernet/freescale/dpaa2/dpaa2-ethtool.c | 58 ++++++++++++++++++---- 1 file changed, 47 insertions(+), 11 deletions(-) (limited to 'drivers/net/ethernet/freescale/dpaa2/dpaa2-ethtool.c') diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-ethtool.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-ethtool.c index bd87aa9ef686..e80e9388c71f 100644 --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-ethtool.c +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-ethtool.c @@ -85,11 +85,16 @@ static void dpaa2_eth_get_drvinfo(struct net_device *net_dev, static int dpaa2_eth_nway_reset(struct net_device *net_dev) { struct dpaa2_eth_priv *priv = netdev_priv(net_dev); + int err = -EOPNOTSUPP; + + mutex_lock(&priv->mac_lock); if (dpaa2_eth_is_type_phy(priv)) - return phylink_ethtool_nway_reset(priv->mac->phylink); + err = phylink_ethtool_nway_reset(priv->mac->phylink); + + mutex_unlock(&priv->mac_lock); - return -EOPNOTSUPP; + return err; } static int @@ -97,10 +102,18 @@ dpaa2_eth_get_link_ksettings(struct net_device *net_dev, struct ethtool_link_ksettings *link_settings) { struct dpaa2_eth_priv *priv = netdev_priv(net_dev); + int err; - if (dpaa2_eth_is_type_phy(priv)) - return phylink_ethtool_ksettings_get(priv->mac->phylink, - link_settings); + mutex_lock(&priv->mac_lock); + + if (dpaa2_eth_is_type_phy(priv)) { + err = phylink_ethtool_ksettings_get(priv->mac->phylink, + link_settings); + mutex_unlock(&priv->mac_lock); + return err; + } + + mutex_unlock(&priv->mac_lock); link_settings->base.autoneg = AUTONEG_DISABLE; if (!(priv->link_state.options & DPNI_LINK_OPT_HALF_DUPLEX)) @@ -115,11 +128,17 @@ dpaa2_eth_set_link_ksettings(struct net_device *net_dev, const struct ethtool_link_ksettings *link_settings) { struct dpaa2_eth_priv *priv = netdev_priv(net_dev); + int err = -EOPNOTSUPP; - if (!dpaa2_eth_is_type_phy(priv)) - return -EOPNOTSUPP; + mutex_lock(&priv->mac_lock); + + if (dpaa2_eth_is_type_phy(priv)) + err = phylink_ethtool_ksettings_set(priv->mac->phylink, + link_settings); - return phylink_ethtool_ksettings_set(priv->mac->phylink, link_settings); + mutex_unlock(&priv->mac_lock); + + return err; } static void dpaa2_eth_get_pauseparam(struct net_device *net_dev, @@ -128,11 +147,16 @@ static void dpaa2_eth_get_pauseparam(struct net_device *net_dev, struct dpaa2_eth_priv *priv = netdev_priv(net_dev); u64 link_options = priv->link_state.options; + mutex_lock(&priv->mac_lock); + if (dpaa2_eth_is_type_phy(priv)) { phylink_ethtool_get_pauseparam(priv->mac->phylink, pause); + mutex_unlock(&priv->mac_lock); return; } + mutex_unlock(&priv->mac_lock); + pause->rx_pause = dpaa2_eth_rx_pause_enabled(link_options); pause->tx_pause = dpaa2_eth_tx_pause_enabled(link_options); pause->autoneg = AUTONEG_DISABLE; @@ -151,9 +175,17 @@ static int dpaa2_eth_set_pauseparam(struct net_device *net_dev, return -EOPNOTSUPP; } - if (dpaa2_eth_is_type_phy(priv)) - return phylink_ethtool_set_pauseparam(priv->mac->phylink, - pause); + mutex_lock(&priv->mac_lock); + + if (dpaa2_eth_is_type_phy(priv)) { + err = phylink_ethtool_set_pauseparam(priv->mac->phylink, + pause); + mutex_unlock(&priv->mac_lock); + return err; + } + + mutex_unlock(&priv->mac_lock); + if (pause->autoneg) return -EOPNOTSUPP; @@ -309,8 +341,12 @@ static void dpaa2_eth_get_ethtool_stats(struct net_device *net_dev, } *(data + i++) = buf_cnt_total; + mutex_lock(&priv->mac_lock); + if (dpaa2_eth_has_mac(priv)) dpaa2_mac_get_ethtool_stats(priv->mac, data + i); + + mutex_unlock(&priv->mac_lock); } static int dpaa2_eth_prep_eth_rule(struct ethhdr *eth_value, struct ethhdr *eth_mask, -- cgit