526c8ee0 | 04-Oct-2023 |
Marek Behún <kabel@kernel.org> |
net: dsa: qca8k: fix potential MDIO bus conflict when accessing internal PHYs via management frames
Besides the QCA8337 switch the Turris 1.x device has on it's MDIO bus also Micron ethernet PHY (de
net: dsa: qca8k: fix potential MDIO bus conflict when accessing internal PHYs via management frames
Besides the QCA8337 switch the Turris 1.x device has on it's MDIO bus also Micron ethernet PHY (dedicated to the WAN port).
We've been experiencing a strange behavior of the WAN ethernet interface, wherein the WAN PHY started timing out the MDIO accesses, for example when the interface was brought down and then back up.
Bisecting led to commit 2cd548566384 ("net: dsa: qca8k: add support for phy read/write with mgmt Ethernet"), which added support to access the QCA8337 switch's internal PHYs via management ethernet frames.
Connecting the MDIO bus pins onto an oscilloscope, I was able to see that the MDIO bus was active whenever a request to read/write an internal PHY register was done via an management ethernet frame.
My theory is that when the switch core always communicates with the internal PHYs via the MDIO bus, even when externally we request the access via ethernet. This MDIO bus is the same one via which the switch and internal PHYs are accessible to the board, and the board may have other devices connected on this bus. An ASCII illustration may give more insight:
+---------+ +----| | | | WAN PHY | | +--| | | | +---------+ | | | | +----------------------------------+ | | | QCA8337 | MDC | | | +-------+ | ------o-+--|--------o------------o--| | | MDIO | | | | | PHY 1 |-|--to RJ45 --------o--|---o----+---------o--+--| | | | | | | | +-------+ | | +-------------+ | o--| | | | | MDIO MDC | | | | PHY 2 |-|--to RJ45 eth1 | | | o--+--| | | -----------|-|port0 | | | +-------+ | | | | | o--| | | | | switch core | | | | PHY 3 |-|--to RJ45 | +-------------+ o--+--| | | | | | +-------+ | | | o--| ... | | +----------------------------------+
When we send a request to read an internal PHY register via an ethernet management frame via eth1, the switch core receives the ethernet frame on port 0 and then communicates with the internal PHY via MDIO. At this time, other potential devices, such as the WAN PHY on Turris 1.x, cannot use the MDIO bus, since it may cause a bus conflict.
Fix this issue by locking the MDIO bus even when we are accessing the PHY registers via ethernet management frames.
Fixes: 2cd548566384 ("net: dsa: qca8k: add support for phy read/write with mgmt Ethernet") Signed-off-by: Marek Behún <kabel@kernel.org> Reviewed-by: Christian Marangi <ansuelsmth@gmail.com> Signed-off-by: David S. Miller <davem@davemloft.net>
show more ...
|
01e6f8ad | 30-Jul-2023 |
Christian Marangi <ansuelsmth@gmail.com> |
net: dsa: qca8k: use dsa_for_each macro instead of for loop
Convert for loop to dsa_for_each macro to save some redundant write on unconnected/unused port and tidy things up.
Signed-off-by: Christi
net: dsa: qca8k: use dsa_for_each macro instead of for loop
Convert for loop to dsa_for_each macro to save some redundant write on unconnected/unused port and tidy things up.
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com> Link: https://lore.kernel.org/r/20230730074113.21889-5-ansuelsmth@gmail.com Signed-off-by: Paolo Abeni <pabeni@redhat.com>
show more ...
|
a9108b07 | 30-Jul-2023 |
Christian Marangi <ansuelsmth@gmail.com> |
net: dsa: qca8k: move qca8xxx hol fixup to separate function
Move qca8xxx hol fixup to separate function to tidy things up and to permit using a more efficent loop in future patch.
Signed-off-by: C
net: dsa: qca8k: move qca8xxx hol fixup to separate function
Move qca8xxx hol fixup to separate function to tidy things up and to permit using a more efficent loop in future patch.
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com> Link: https://lore.kernel.org/r/20230730074113.21889-4-ansuelsmth@gmail.com Signed-off-by: Paolo Abeni <pabeni@redhat.com>
show more ...
|
18e8feae | 30-Jul-2023 |
Christian Marangi <ansuelsmth@gmail.com> |
net: dsa: qca8k: limit user ports access to the first CPU port on setup
In preparation for multi-CPU support, set CPU port LOOKUP MEMBER outside the port loop and setup the LOOKUP MEMBER mask for us
net: dsa: qca8k: limit user ports access to the first CPU port on setup
In preparation for multi-CPU support, set CPU port LOOKUP MEMBER outside the port loop and setup the LOOKUP MEMBER mask for user ports only to the first CPU port.
This is to handle flooding condition where every CPU port is set as target and prevent packet duplication for unknown frames from user ports.
Secondary CPU port LOOKUP MEMBER mask will be setup later when port_change_master will be implemented.
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> Reviewed-by: Simon Horman <simon.horman@corigine.com> Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com> Reviewed-by: Vladimir Oltean <olteanv@gmail.com> Link: https://lore.kernel.org/r/20230730074113.21889-3-ansuelsmth@gmail.com Signed-off-by: Paolo Abeni <pabeni@redhat.com>
show more ...
|
dfd739f1 | 23-Jul-2023 |
Christian Marangi <ansuelsmth@gmail.com> |
net: dsa: qca8k: fix mdb add/del case with 0 VID
The qca8k switch doesn't support using 0 as VID and require a default VID to be always set. MDB add/del function doesn't currently handle this and ar
net: dsa: qca8k: fix mdb add/del case with 0 VID
The qca8k switch doesn't support using 0 as VID and require a default VID to be always set. MDB add/del function doesn't currently handle this and are currently setting the default VID.
Fix this by correctly handling this corner case and internally use the default VID for VID 0 case.
Fixes: ba8f870dfa63 ("net: dsa: qca8k: add support for mdb_add/del") Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> Cc: stable@vger.kernel.org Signed-off-by: David S. Miller <davem@davemloft.net>
show more ...
|
ae70dcb9 | 23-Jul-2023 |
Christian Marangi <ansuelsmth@gmail.com> |
net: dsa: qca8k: fix broken search_and_del
On deleting an MDB entry for a port, fdb_search_and_del is used. An FDB entry can't be modified so it needs to be deleted and readded again with the new po
net: dsa: qca8k: fix broken search_and_del
On deleting an MDB entry for a port, fdb_search_and_del is used. An FDB entry can't be modified so it needs to be deleted and readded again with the new portmap (and the port deleted as requested)
We use the SEARCH operator to search the entry to edit by vid and mac address and then we check the aging if we actually found an entry.
Currently the code suffer from a bug where the searched fdb entry is never read again with the found values (if found) resulting in the code always returning -EINVAL as aging was always 0.
Fix this by correctly read the fdb entry after it was searched.
Fixes: ba8f870dfa63 ("net: dsa: qca8k: add support for mdb_add/del") Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> Cc: stable@vger.kernel.org Signed-off-by: David S. Miller <davem@davemloft.net>
show more ...
|
80248d41 | 23-Jul-2023 |
Christian Marangi <ansuelsmth@gmail.com> |
net: dsa: qca8k: fix search_and_insert wrong handling of new rule
On inserting a mdb entry, fdb_search_and_insert is used to add a port to the qca8k target entry in the FDB db.
A FDB entry can't be
net: dsa: qca8k: fix search_and_insert wrong handling of new rule
On inserting a mdb entry, fdb_search_and_insert is used to add a port to the qca8k target entry in the FDB db.
A FDB entry can't be modified so it needs to be removed and insert again with the new values.
To detect if an entry already exist, the SEARCH operation is used and we check the aging of the entry. If the entry is not 0, the entry exist and we proceed to delete it.
Current code have 2 main problem: - The condition to check if the FDB entry exist is wrong and should be the opposite. - When a FDB entry doesn't exist, aging was never actually set to the STATIC value resulting in allocating an invalid entry.
Fix both problem by adding aging support to the function, calling the function with STATIC as aging by default and finally by correct the condition to check if the entry actually exist.
Fixes: ba8f870dfa63 ("net: dsa: qca8k: add support for mdb_add/del") Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> Cc: stable@vger.kernel.org Signed-off-by: David S. Miller <davem@davemloft.net>
show more ...
|