• Vladimir Oltean's avatar
    net: dsa: hold rtnl_lock in dsa_switch_setup_tag_protocol · 1951b3f1
    Vladimir Oltean authored
    It was a documented fact that ds->ops->change_tag_protocol() offered
    rtnetlink mutex protection to the switch driver, since there was an
    ASSERT_RTNL right before the call in dsa_switch_change_tag_proto()
    (initiated from sysfs).
    
    The blamed commit introduced another call path for
    ds->ops->change_tag_protocol() which does not hold the rtnl_mutex.
    This is:
    
    dsa_tree_setup
    -> dsa_tree_setup_switches
       -> dsa_switch_setup
          -> dsa_switch_setup_tag_protocol
             -> ds->ops->change_tag_protocol()
       -> dsa_port_setup
          -> dsa_slave_create
             -> register_netdevice(slave_dev)
    -> dsa_tree_setup_master
       -> dsa_master_setup
          -> dev->dsa_ptr = cpu_dp
    
    The reason why the rtnl_mutex is held in the sysfs call path is to
    ensure that, once the master and all the DSA interfaces are down (which
    is required so that no packets flow), they remain down during the
    tagging protocol change.
    
    The above calling order illustrates the fact that it should not be risky
    to change the initial tagging protocol to the one specified in the
    device tree at the given time:
    
    - packets cannot enter the dsa_switch_rcv() packet type handler since
      netdev_uses_dsa() for the master will not yet return true, since
      dev->dsa_ptr has not yet been populated
    
    - packets cannot enter the dsa_slave_xmit() function because no DSA
      interface has yet been registered
    
    So from the DSA core's perspective, holding the rtnl_mutex is indeed not
    necessary.
    
    Yet, drivers may need to do things which need rtnl_mutex protection. For
    example:
    
    felix_set_tag_protocol
    -> felix_setup_tag_8021q
       -> dsa_tag_8021q_register
          -> dsa_tag_8021q_setup
             -> dsa_tag_8021q_port_setup
                -> vlan_vid_add
                   -> ASSERT_RTNL
    
    These drivers do not really have a choice to take the rtnl_mutex
    themselves, since in the sysfs case, the rtnl_mutex is already held.
    
    Fixes: deff7107 ("net: dsa: Allow default tag protocol to be overridden from DT")
    Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
    Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
    1951b3f1
dsa2.c 37.7 KB