Re: [PATCH net 1/1] net: stmmac: Prevent DSA tags from breaking COE
From: Vladimir Oltean <olteanv@gmail.com>
Date: 2023-12-19 12:20:38
Also in:
linux-arm-kernel, stable
Hi Romain, On Mon, Dec 18, 2023 at 05:23:23PM +0100, Romain Gantois wrote:
Some stmmac cores have Checksum Offload Engines that cannot handle DSA tags
properly. These cores find the IP/TCP headers on their own and end up
computing an incorrect checksum when a DSA tag is inserted between the MAC
header and IP header.
Add an additional check on stmmac link up so that COE is deactivated
when the stmmac device is used as a DSA conduit.
Add a new dma_feature flag to allow cores to signal that their COEs can't
handle DSA tags on TX.
Fixes: 6b2c6e4a938f ("net: stmmac: propagate feature flags to vlan")
Cc: stable@vger.kernel.org
Reported-by: Richard Tresidder <redacted>
Closes: https://lore.kernel.org/netdev/e5c6c75f-2dfa-4e50-a1fb-6bf4cdb617c2@electromag.com.au/ (local)
Reported-by: Romain Gantois <romain.gantois@bootlin.com>
Closes: https://lore.kernel.org/netdev/c57283ed-6b9b-b0e6-ee12-5655c1c54495@bootlin.com/ (local)
Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>
---DSA_TAG_PROTO_LAN9303, DSA_TAG_PROTO_SJA1105 and DSA_TAG_PROTO_SJA1110 construct tags with ETH_P_8021Q as EtherType. Do you still think it would be correct to say that all DSA tags break COE on the stmmac, as this patch assumes? The NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM convention is not about statically checking whether the interface using DSA, but about looking at each packet before deciding whether to use the offload engine or to call skb_checksum_help(). You can experiment with any tagging protocol on the stmmac driver, and thus with the controller's response to any kind of traffic, even if the port is not attached to a hardware switch. You need to enable the CONFIG_NET_DSA_LOOP option, edit the return value of dsa_loop_get_protocol() and the "netdev" field of dsa_loop_pdata. The packets need to be analyzed on the link partner with a packet analysis tool, since there is no switch to strip the DSA tag.
quoted hunk ↗ jump to hunk
drivers/net/ethernet/stmicro/stmmac/common.h | 1 + .../net/ethernet/stmicro/stmmac/dwmac1000_dma.c | 1 + .../net/ethernet/stmicro/stmmac/stmmac_main.c | 16 +++++++++++++++- 3 files changed, 17 insertions(+), 1 deletion(-)diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c index daf79cdbd3ec..50686cdc3320 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c@@ -264,6 +264,7 @@ static int dwmac1000_get_hw_feature(void __iomem *ioaddr, dma_cap->number_tx_channel = (hw_cap & DMA_HW_FEAT_TXCHCNT) >> 22; /* Alternate (enhanced) DESC mode */ dma_cap->enh_desc = (hw_cap & DMA_HW_FEAT_ENHDESSEL) >> 24; + dma_cap->dsa_breaks_tx_coe = 1; return 0; }diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index a9b6b383e863..733348c65e04 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c@@ -42,6 +42,7 @@ #include <net/page_pool/helpers.h> #include <net/pkt_cls.h> #include <net/xdp_sock_drv.h> +#include <net/dsa.h> #include "stmmac_ptp.h" #include "stmmac.h" #include "stmmac_xdp.h"@@ -993,8 +994,11 @@ static void stmmac_mac_link_up(struct phylink_config *config, int speed, int duplex, bool tx_pause, bool rx_pause) { - struct stmmac_priv *priv = netdev_priv(to_net_dev(config->dev)); + struct net_device *ndev = to_net_dev(config->dev); + struct stmmac_priv *priv = netdev_priv(ndev); + unsigned int tx_queue_cnt; u32 old_ctrl, ctrl; + int queue; if ((priv->plat->flags & STMMAC_FLAG_SERDES_UP_AFTER_PHY_LINKUP) && priv->plat->serdes_powerup)@@ -1102,6 +1106,16 @@ static void stmmac_mac_link_up(struct phylink_config *config, if (priv->plat->flags & STMMAC_FLAG_HWTSTAMP_CORRECT_LATENCY) stmmac_hwtstamp_correct_latency(priv, priv); + + /* DSA tags break COEs on some cores. Disable TX checksum + * offloading on those cores if the netdevice is a DSA conduit. + */ + if (priv->dma_cap.dsa_breaks_tx_coe && netdev_uses_dsa(ndev)) { + tx_queue_cnt = priv->plat->tx_queues_to_use; + for (queue = 0; queue < tx_queue_cnt; queue++) + priv->plat->tx_queues_cfg[queue].coe_unsupported = true; + } +
The DSA switch driver can load after stmmac_mac_link_up() runs. This implementation is racy anyway.
}
static const struct phylink_mac_ops stmmac_phylink_mac_ops = {
--
2.43.0
pw-bot: cr