Re: [PATCH net v2] net: sparx5: Add spinlock for frame transmission from CPU

[Horatiu Vultur]

The 02/15/2024 09:33, Horatiu Vultur wrote:
> Both registers used when doing manual injection or fdma injection are
> shared between all the net devices of the switch. It was noticed that
> when having two process which each of them trying to inject frames on
> different ethernet ports, that the HW started to behave strange, by
> sending out more frames then expected. When doing fdma injection it is
> required to set the frame in the DCB and then make sure that the next
> pointer of the last DCB is invalid. But because there is no locks for
> this, then easily this pointer between the DCB can be broken and then it
> would create a loop of DCBs. And that means that the HW will
> continuously transmit these frames in a loop. Until the SW will break
> this loop.
> Therefore to fix this issue, add a spin lock for when accessing the
> registers for manual or fdma injection.
> 
> Signed-off-by: Horatiu Vultur <horatiu.vultur@xxxxxxxxxxxxx>
> Reviewed-by: Daniel Machon <daniel.machon@xxxxxxxxxxxxx>

Argh... I forgot to add the Fixes tag:
Fixes: f3cad2611a77 ("net: sparx5: add hostmode with phylink support")

> ---
> v1->v2:
> - target net instead of net-next
> ---
>  drivers/net/ethernet/microchip/sparx5/sparx5_main.c   | 1 +
>  drivers/net/ethernet/microchip/sparx5/sparx5_main.h   | 1 +
>  drivers/net/ethernet/microchip/sparx5/sparx5_packet.c | 2 ++
>  3 files changed, 4 insertions(+)
> 
> diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.c b/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
> index d1f7fc8b1b71a..3c066b62e6894 100644
> --- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
> +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
> @@ -757,6 +757,7 @@ static int mchp_sparx5_probe(struct platform_device *pdev)
>  	platform_set_drvdata(pdev, sparx5);
>  	sparx5->pdev = pdev;
>  	sparx5->dev = &pdev->dev;
> +	spin_lock_init(&sparx5->tx_lock);
>  
>  	/* Do switch core reset if available */
>  	reset = devm_reset_control_get_optional_shared(&pdev->dev, "switch");
> diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
> index 6f565c0c0c3dc..316fed5f27355 100644
> --- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
> +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
> @@ -280,6 +280,7 @@ struct sparx5 {
>  	int xtr_irq;
>  	/* Frame DMA */
>  	int fdma_irq;
> +	spinlock_t tx_lock; /* lock for frame transmission */
>  	struct sparx5_rx rx;
>  	struct sparx5_tx tx;
>  	/* PTP */
> diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_packet.c b/drivers/net/ethernet/microchip/sparx5/sparx5_packet.c
> index 6db6ac6a3bbc2..ac7e1cffbcecf 100644
> --- a/drivers/net/ethernet/microchip/sparx5/sparx5_packet.c
> +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_packet.c
> @@ -244,10 +244,12 @@ netdev_tx_t sparx5_port_xmit_impl(struct sk_buff *skb, struct net_device *dev)
>  	}
>  
>  	skb_tx_timestamp(skb);
> +	spin_lock(&sparx5->tx_lock);
>  	if (sparx5->fdma_irq > 0)
>  		ret = sparx5_fdma_xmit(sparx5, ifh, skb);
>  	else
>  		ret = sparx5_inject(sparx5, ifh, skb, dev);
> +	spin_unlock(&sparx5->tx_lock);
>  
>  	if (ret == -EBUSY)
>  		goto busy;
> -- 
> 2.34.1
> 

-- 
/Horatiu