1. 24 Oct, 2021 19 commits
    • David S. Miller's avatar
      Merge branch 'dsa-rtnl' · 965e6b26
      David S. Miller authored
      Vladimir Oltean says:
      
      ====================
      Drop rtnl_lock from DSA .port_fdb_{add,del}
      
      As mentioned in the RFC posted 2 months ago:
      https://patchwork.kernel.org/project/netdevbpf/cover/20210824114049.3814660-1-vladimir.oltean@nxp.com/
      
      DSA is transitioning to a driver API where the rtnl_lock is not held
      when calling ds->ops->port_fdb_add() and ds->ops->port_fdb_del().
      Drivers cannot take that lock privately from those callbacks either.
      
      This change is required so that DSA can wait for switchdev FDB work
      items to finish before leaving the bridge. That change will be made in a
      future patch series.
      
      A small selftest is provided with the patch set in the hope that
      concurrency issues uncovered by this series, but not spotted by me by
      code inspection, will be caught.
      
      A status of the existing drivers:
      
      - mv88e6xxx_port_fdb_add() and mv88e6xxx_port_fdb_del() take
        mv88e6xxx_reg_lock() so they should be safe.
      
      - qca8k_fdb_add() and qca8k_fdb_del() take mutex_lock(&priv->reg_mutex)
        so they should be safe.
      
      - hellcreek_fdb_add() and hellcreek_fdb_add() take mutex_lock(&hellcreek->reg_lock)
        so they should be safe.
      
      - ksz9477_port_fdb_add() and ksz9477_port_fdb_del() take mutex_lock(&dev->alu_mutex)
        so they should be safe.
      
      - b53_fdb_add() and b53_fdb_del() did not have locking, so I've added a
        scheme based on my own judgement there (not tested).
      
      - felix_fdb_add() and felix_fdb_del() did not have locking, I've added
        and tested a locking scheme there.
      
      - mt7530_port_fdb_add() and mt7530_port_fdb_del() take
        mutex_lock(&priv->reg_mutex), so they should be safe.
      
      - gswip_port_fdb() did not have locking, so I've added a non-expert
        locking scheme based on my own judgement (not tested).
      
      - lan9303_alr_add_port() and lan9303_alr_del_port() take
        mutex_lock(&chip->alr_mutex) so they should be safe.
      
      - sja1105_fdb_add() and sja1105_fdb_del() did not have locking, I've
        added and tested a locking scheme.
      ====================
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      965e6b26
    • Vladimir Oltean's avatar
      selftests: net: dsa: add a stress test for unlocked FDB operations · edc90d15
      Vladimir Oltean authored
      This test is a bit strange in that it is perhaps more manual than
      others: it does not transmit a clear OK/FAIL verdict, because user space
      does not have synchronous feedback from the kernel. If a hardware access
      fails, it is in deferred context.
      
      Nonetheless, on sja1105 I have used it successfully to find and solve a
      concurrency issue, so it can be used as a starting point for other
      driver maintainers too.
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Reviewed-by: default avatarFlorian Fainelli <f.fainelli@gmail.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      edc90d15
    • Vladimir Oltean's avatar
      selftests: lib: forwarding: allow tests to not require mz and jq · 01674896
      Vladimir Oltean authored
      These programs are useful, but not all selftests require them.
      
      Additionally, on embedded boards without package management (things like
      buildroot), installing mausezahn or jq is not always as trivial as
      downloading a package from the web.
      
      So it is actually a bit annoying to require programs that are not used.
      Introduce options that can be set by scripts to not enforce these
      dependencies. For compatibility, default to "yes".
      
      Cc: Nikolay Aleksandrov <nikolay@nvidia.com>
      Cc: Ido Schimmel <idosch@nvidia.com>
      Cc: Guillaume Nault <gnault@redhat.com>
      Cc: Po-Hsu Lin <po-hsu.lin@canonical.com>
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Reviewed-by: default avatarFlorian Fainelli <f.fainelli@gmail.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      01674896
    • Vladimir Oltean's avatar
      net: dsa: drop rtnl_lock from dsa_slave_switchdev_event_work · 5cdfde49
      Vladimir Oltean authored
      After talking with Ido Schimmel, it became clear that rtnl_lock is not
      actually required for anything that is done inside the
      SWITCHDEV_FDB_{ADD,DEL}_TO_DEVICE deferred work handlers.
      
      The reason why it was probably added by Arkadi Sharshevsky in commit
      c9eb3e0f ("net: dsa: Add support for learning FDB through
      notification") was to offer the same locking/serialization guarantees as
      .ndo_fdb_{add,del} and avoid reworking any drivers.
      
      DSA has implemented .ndo_fdb_add and .ndo_fdb_del until commit
      b117e1e8 ("net: dsa: delete dsa_legacy_fdb_add and
      dsa_legacy_fdb_del") - that is to say, until fairly recently.
      
      But those methods have been deleted, so now we are free to drop the
      rtnl_lock as well.
      
      Note that exposing DSA switch drivers to an unlocked method which was
      previously serialized by the rtnl_mutex is a potentially dangerous
      affair. Driver writers couldn't ensure that their internal locking
      scheme does the right thing even if they wanted.
      
      We could err on the side of paranoia and introduce a switch-wide lock
      inside the DSA framework, but that seems way overreaching. Instead, we
      could check as many drivers for regressions as we can, fix those first,
      then let this change go in once it is assumed to be fairly safe.
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Reviewed-by: default avatarFlorian Fainelli <f.fainelli@gmail.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      5cdfde49
    • Vladimir Oltean's avatar
      net: dsa: introduce locking for the address lists on CPU and DSA ports · d3bd8924
      Vladimir Oltean authored
      Now that the rtnl_mutex is going away for dsa_port_{host_,}fdb_{add,del},
      no one is serializing access to the address lists that DSA keeps for the
      purpose of reference counting on shared ports (CPU and cascade ports).
      
      It can happen for one dsa_switch_do_fdb_del to do list_del on a dp->fdbs
      element while another dsa_switch_do_fdb_{add,del} is traversing dp->fdbs.
      We need to avoid that.
      
      Currently dp->mdbs is not at risk, because dsa_switch_do_mdb_{add,del}
      still runs under the rtnl_mutex. But it would be nice if it would not
      depend on that being the case. So let's introduce a mutex per port (the
      address lists are per port too) and share it between dp->mdbs and
      dp->fdbs.
      
      The place where we put the locking is interesting. It could be tempting
      to put a DSA-level lock which still serializes calls to
      .port_fdb_{add,del}, but it would still not avoid concurrency with other
      driver code paths that are currently under rtnl_mutex (.port_fdb_dump,
      .port_fast_age). So it would add a very false sense of security (and
      adding a global switch-wide lock in DSA to resynchronize with the
      rtnl_lock is also counterproductive and hard).
      
      So the locking is intentionally done only where the dp->fdbs and dp->mdbs
      lists are traversed. That means, from a driver perspective, that
      .port_fdb_add will be called with the dp->addr_lists_lock mutex held on
      the CPU port, but not held on user ports. This is done so that driver
      writers are not encouraged to rely on any guarantee offered by
      dp->addr_lists_lock.
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Reviewed-by: default avatarFlorian Fainelli <f.fainelli@gmail.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      d3bd8924
    • Vladimir Oltean's avatar
      net: dsa: lantiq_gswip: serialize access to the PCE table · 49753a75
      Vladimir Oltean authored
      Looking at the code, the GSWIP switch appears to hold bridging service
      structures (VLANs, FDBs, forwarding rules) in PCE table entries.
      Hardware access to the PCE table is non-atomic, and is comprised of
      several register reads and writes.
      
      These accesses are currently serialized by the rtnl_lock, but DSA is
      changing its driver API and that lock will no longer be held when
      calling ->port_fdb_add() and ->port_fdb_del().
      
      So this driver needs to serialize the access to the PCE table using its
      own locking scheme. This patch adds that.
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Reviewed-by: default avatarFlorian Fainelli <f.fainelli@gmail.com>
      Acked-by: default avatarHauke Mehrtens <hauke@hauke-m.de>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      49753a75
    • Vladimir Oltean's avatar
      net: dsa: b53: serialize access to the ARL table · f239934c
      Vladimir Oltean authored
      The b53 driver performs non-atomic transactions to the ARL table when
      adding, deleting and reading FDB and MDB entries.
      
      Traditionally these were all serialized by the rtnl_lock(), but now it
      is possible that DSA calls ->port_fdb_add and ->port_fdb_del without
      holding that lock.
      
      So the driver must have its own serialization logic. Add a mutex and
      hold it from all entry points (->port_fdb_{add,del,dump},
      ->port_mdb_{add,del}).
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Reviewed-by: default avatarFlorian Fainelli <f.fainelli@gmail.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      f239934c
    • Vladimir Oltean's avatar
      net: mscc: ocelot: serialize access to the MAC table · f2c4bdf6
      Vladimir Oltean authored
      DSA would like to remove the rtnl_lock from its
      SWITCHDEV_FDB_{ADD,DEL}_TO_DEVICE handlers, and the felix driver uses
      the same MAC table functions as ocelot.
      
      This means that the MAC table functions will no longer be implicitly
      serialized with respect to each other by the rtnl_mutex, we need to add
      a dedicated lock in ocelot for the non-atomic operations of selecting a
      MAC table row, reading/writing what we want and polling for completion.
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Reviewed-by: default avatarFlorian Fainelli <f.fainelli@gmail.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      f2c4bdf6
    • Vladimir Oltean's avatar
      net: dsa: sja1105: serialize access to the dynamic config interface · 1681ae16
      Vladimir Oltean authored
      The sja1105 hardware seems as concurrent as can be, but when we create a
      background script that adds/removes a rain of FDB entries without the
      rtnl_mutex taken, then in parallel we do another operation like run
      'bridge fdb show', we can notice these errors popping up:
      
      sja1105 spi2.0: port 2 failed to read back entry for 00:01:02:03:00:40 vid 0: -ENOENT
      sja1105 spi2.0: port 2 failed to add 00:01:02:03:00:40 vid 0 to fdb: -2
      sja1105 spi2.0: port 2 failed to read back entry for 00:01:02:03:00:46 vid 0: -ENOENT
      sja1105 spi2.0: port 2 failed to add 00:01:02:03:00:46 vid 0 to fdb: -2
      
      Luckily what is going on does not require a major rework in the driver.
      The sja1105_dynamic_config_read() function sends multiple SPI buffers to
      the peripheral until the operation completes. We should not do anything
      until the hardware clears the VALID bit.
      
      But since there is no locking (i.e. right now we are implicitly
      serialized by the rtnl_mutex, but if we remove that), it might be
      possible that the process which performs the dynamic config read is
      preempted and another one performs a dynamic config write.
      
      What will happen in that case is that sja1105_dynamic_config_read(),
      when it resumes, expects to see VALIDENT set for the entry it reads
      back. But it won't.
      
      This can be corrected by introducing a mutex for serializing SPI
      accesses to the dynamic config interface which should be atomic with
      respect to each other.
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Reviewed-by: default avatarFlorian Fainelli <f.fainelli@gmail.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      1681ae16
    • Vladimir Oltean's avatar
      net: dsa: sja1105: wait for dynamic config command completion on writes too · 643979cf
      Vladimir Oltean authored
      The hardware manual says that software should attempt a new dynamic
      config access (be it a a write or a read-back) only while the VALID bit
      is cleared. The VALID bit is set by software to 1, and it remains set as
      long as the hardware is still processing the request.
      
      Currently the driver only polls for the command completion only for
      reads, because that's when we need the actual data read back. Writes
      have been more or less "asynchronous", although this has never been an
      observable issue.
      
      This change makes sja1105_dynamic_config_write poll the VALID bit as
      well, to absolutely ensure that a follow-up access to the static config
      finds the VALID bit cleared.
      
      So VALID means "work in progress", while VALIDENT means "entry being
      read is valid". On reads we check the VALIDENT bit too, while on writes
      that bit is not always defined. So we need to factor it out of the loop,
      and make the loop provide back the unpacked command structure, so that
      sja1105_dynamic_config_read can check the VALIDENT bit.
      
      The change also attempts to convert the open-coded loop to use the
      read_poll_timeout macro, since I know this will come up during review.
      It's more code, but hey, it uses read_poll_timeout!
      
      Tested on SJA1105T, SJA1105S, SJA1110A.
      Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      643979cf
    • Sean Anderson's avatar
      net: macb: Use mdio child node for MDIO bus if it exists · 4d98bb0d
      Sean Anderson authored
      This allows explicitly specifying which children are present on the mdio
      bus. Additionally, it allows for non-phy MDIO devices on the bus.
      Signed-off-by: default avatarSean Anderson <sean.anderson@seco.com>
      Reviewed-by: default avatarAndrew Lunn <andrew@lunn.ch>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      4d98bb0d
    • Sean Anderson's avatar
      dt-bindings: net: macb: Add mdio bus child node · 25790844
      Sean Anderson authored
      This adds an optional mdio bus child node. If present, the mac will
      look for PHYs there instead of directly under the top-level node. This
      eliminates any ambiguity about whether child nodes are PHYs, and allows
      the MDIO bus to contain non-PHY devices.
      Signed-off-by: default avatarSean Anderson <sean.anderson@seco.com>
      Reviewed-by: default avatarAndrew Lunn <andrew@lunn.ch>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      25790844
    • Florian Fainelli's avatar
      net: bcmgenet: Add support for 7712 16nm internal EPHY · 3cd92eae
      Florian Fainelli authored
      The 16nm internal EPHY that is present in 7712 is actually a 16nm
      Gigabit PHY which has been forced to operate in 10/100 mode. Its
      controls are therefore via the EXT_GPHY_CTRL registers and not via the
      EXT_EPHY_CTRL which are used for all GENETv5 adapters. Add a match on
      the 7712 compatible string to allow that differentiation to happen.
      
      On previous GENETv4 chips the EXT_CFG_IDDQ_GLOBAL_PWR bit was cleared by
      default, but this is not the case with this chip, so we need to make
      sure we clear it to power on the EPHY.
      Signed-off-by: default avatarFlorian Fainelli <f.fainelli@gmail.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      3cd92eae
    • Florian Fainelli's avatar
      dt-bindings: net: bcmgenet: Document 7712 binding · f4b054d9
      Florian Fainelli authored
      7712 includes a GENETv5 adapter with an on-chip 10/100 16nm Ethernet PHY
      which requires us to document that controller's integration specifically
      for proper driver keying.
      Signed-off-by: default avatarFlorian Fainelli <f.fainelli@gmail.com>
      Reviewed-by: default avatarAndrew Lunn <andrew@lunn.ch>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      f4b054d9
    • Florian Fainelli's avatar
      net: phy: bcm7xxx: Add EPHY entry for 7712 · 218f23e8
      Florian Fainelli authored
      7712 is a 16nm process SoC with a 10/100 integrated Ethernet PHY,
      utilize the recently defined 16nm EPHY macro to configure that PHY.
      Signed-off-by: default avatarFlorian Fainelli <f.fainelli@gmail.com>
      Reviewed-by: default avatarAndrew Lunn <andrew@lunn.ch>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      218f23e8
    • Sean Anderson's avatar
      net: Convert more users of mdiobus_* to mdiodev_* · 65aa371e
      Sean Anderson authored
      This converts users of mdiobus to mdiodev using the following semantic
      patch:
      
      @@
      identifier mdiodev;
      expression regnum;
      @@
      
      - mdiobus_read(mdiodev->bus, mdiodev->addr, regnum)
      + mdiodev_read(mdiodev, regnum)
      
      @@
      identifier mdiodev;
      expression regnum, val;
      @@
      
      - mdiobus_write(mdiodev->bus, mdiodev->addr, regnum, val)
      + mdiodev_write(mdiodev, regnum, val)
      Signed-off-by: default avatarSean Anderson <sean.anderson@seco.com>
      Reviewed-by: default avatarFlorian Fainelli <f.fainelli@gmail.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      65aa371e
    • Sean Anderson's avatar
      net: phylink: Convert some users of mdiobus_* to mdiodev_* · c8fb89a7
      Sean Anderson authored
      This refactors the phylink pcs helper functions to use mdiobus_* instead
      of mdiodev_*.
      Signed-off-by: default avatarSean Anderson <sean.anderson@seco.com>
      Reviewed-by: default avatarRussell King (Oracle) <rmk+kernel@armlinux.org.uk>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      c8fb89a7
    • Sean Anderson's avatar
      net: mdio: Add helper functions for accessing MDIO devices · 0ebecb26
      Sean Anderson authored
      This adds some helpers for accessing non-phy MDIO devices. They are
      analogous to phy_(read|write|modify), except that they take an mdio_device
      and not a phy_device.
      Signed-off-by: default avatarSean Anderson <sean.anderson@seco.com>
      Reviewed-by: default avatarRussell King (Oracle) <rmk+kernel@armlinux.org.uk>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      0ebecb26
    • Kiran Kumar K's avatar
      octeontx2-af: Increase number of reserved entries in KPU · db690aec
      Kiran Kumar K authored
      With current KPU profile, we have 2 reserved entries which can
      be loaded from firmware to parse custom headers. Adding changes
      to increase these reserved entries to 6.
      And also removed KPU entries for unused LTYPEs like
      NPC_LT_LA_IH_8_ETHER, NPC_LT_LA_IH_4_ETHER, NPC_LT_LA_IH_2_ETHER
      Signed-off-by: default avatarKiran Kumar K <kirankumark@marvell.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      db690aec
  2. 22 Oct, 2021 21 commits