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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date: Thu, 15 Feb 2024 09:41:01 +0100
From: Horatiu Vultur <horatiu.vultur@...rochip.com>
To: <davem@...emloft.net>, <edumazet@...gle.com>, <kuba@...nel.org>,
	<pabeni@...hat.com>, <lars.povlsen@...rochip.com>,
	<Steen.Hegelund@...rochip.com>, <daniel.machon@...rochip.com>,
	<UNGLinuxDriver@...rochip.com>
CC: <u.kleine-koenig@...gutronix.de>, <rmk+kernel@...linux.org.uk>,
	<jacob.e.keller@...el.com>, <yuehaibing@...wei.com>,
	<vladimir.oltean@....com>, <netdev@...r.kernel.org>,
	<linux-arm-kernel@...ts.infradead.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net v2] net: sparx5: Add spinlock for frame transmission
 from CPU

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@...rochip.com>
> Reviewed-by: Daniel Machon <daniel.machon@...rochip.com>

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