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@xxxxxxxxxxxxxxx
> Reported-by: Richard Tresidder <rtresidd@xxxxxxxxxxxxxxxxx>
> Closes: https://lore.kernel.org/netdev/e5c6c75f-2dfa-4e50-a1fb-6bf4cdb617c2@xxxxxxxxxxxxxxxxx/
> Reported-by: Romain Gantois <romain.gantois@xxxxxxxxxxx>
> Closes: https://lore.kernel.org/netdev/c57283ed-6b9b-b0e6-ee12-5655c1c54495@xxxxxxxxxxx/
> Signed-off-by: Romain Gantois <romain.gantois@xxxxxxxxxxx>
> ---
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.
> 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
