This is a note to let you know that I've just added the patch titled
net: systemport: Add global locking for descriptor lifecycle
to the 4.9-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
net-systemport-add-global-locking-for-descriptor-lifecycle.patch
and it can be found in the queue-4.9 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.
>From 8b8e6e782456f1ce02a7ae914bbd5b1053f0b034 Mon Sep 17 00:00:00 2001
From: Florian Fainelli <f.fainelli@xxxxxxxxx>
Date: Wed, 15 Dec 2021 12:24:49 -0800
Subject: net: systemport: Add global locking for descriptor lifecycle
From: Florian Fainelli <f.fainelli@xxxxxxxxx>
commit 8b8e6e782456f1ce02a7ae914bbd5b1053f0b034 upstream.
The descriptor list is a shared resource across all of the transmit queues, and
the locking mechanism used today only protects concurrency across a given
transmit queue between the transmit and reclaiming. This creates an opportunity
for the SYSTEMPORT hardware to work on corrupted descriptors if we have
multiple producers at once which is the case when using multiple transmit
queues.
This was particularly noticeable when using multiple flows/transmit queues and
it showed up in interesting ways in that UDP packets would get a correct UDP
header checksum being calculated over an incorrect packet length. Similarly TCP
packets would get an equally correct checksum computed by the hardware over an
incorrect packet length.
The SYSTEMPORT hardware maintains an internal descriptor list that it re-arranges
when the driver produces a new descriptor anytime it writes to the
WRITE_PORT_{HI,LO} registers, there is however some delay in the hardware to
re-organize its descriptors and it is possible that concurrent TX queues
eventually break this internal allocation scheme to the point where the
length/status part of the descriptor gets used for an incorrect data buffer.
The fix is to impose a global serialization for all TX queues in the short
section where we are writing to the WRITE_PORT_{HI,LO} registers which solves
the corruption even with multiple concurrent TX queues being used.
Fixes: 80105befdb4b ("net: systemport: add Broadcom SYSTEMPORT Ethernet MAC driver")
Signed-off-by: Florian Fainelli <f.fainelli@xxxxxxxxx>
Link: https://lore.kernel.org/r/20211215202450.4086240-1-f.fainelli@xxxxxxxxx
Signed-off-by: Jakub Kicinski <kuba@xxxxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
---
drivers/net/ethernet/broadcom/bcmsysport.c | 5 +++++
drivers/net/ethernet/broadcom/bcmsysport.h | 1 +
2 files changed, 6 insertions(+)
--- a/drivers/net/ethernet/broadcom/bcmsysport.c
+++ b/drivers/net/ethernet/broadcom/bcmsysport.c
@@ -90,9 +90,13 @@ static inline void tdma_port_write_desc_
struct dma_desc *desc,
unsigned int port)
{
+ unsigned long desc_flags;
+
/* Ports are latched, so write upper address first */
+ spin_lock_irqsave(&priv->desc_lock, desc_flags);
tdma_writel(priv, desc->addr_status_len, TDMA_WRITE_PORT_HI(port));
tdma_writel(priv, desc->addr_lo, TDMA_WRITE_PORT_LO(port));
+ spin_unlock_irqrestore(&priv->desc_lock, desc_flags);
}
/* Ethtool operations */
@@ -1587,6 +1591,7 @@ static int bcm_sysport_open(struct net_d
}
/* Initialize both hardware and software ring */
+ spin_lock_init(&priv->desc_lock);
for (i = 0; i < dev->num_tx_queues; i++) {
ret = bcm_sysport_init_tx_ring(priv, i);
if (ret) {
--- a/drivers/net/ethernet/broadcom/bcmsysport.h
+++ b/drivers/net/ethernet/broadcom/bcmsysport.h
@@ -660,6 +660,7 @@ struct bcm_sysport_priv {
int wol_irq;
/* Transmit rings */
+ spinlock_t desc_lock;
struct bcm_sysport_tx_ring tx_rings[TDMA_NUM_RINGS];
/* Receive queue */
Patches currently in stable-queue which might be from f.fainelli@xxxxxxxxx are
queue-4.9/net-systemport-add-global-locking-for-descriptor-lifecycle.patch
