[PATCH net-next 00/10] net: stmmac: TSO fixes/cleanups

[PATCH net-next 00/10] net: stmmac: TSO fixes/cleanups

[Russell King (Oracle)]

Hot off the press from reading various sources of dwmac information,
this series attempts to fix the buggy hacks that were previously
merged, and clean up the code handling this.

I'm not sure whether "TSO" or "GSO" should be used to describe this
feature - although it primarily handles TCP, dwmac4 appears to also
be able to handle UDP.

In essence, this series adds a .ndo_features_check() method to handle
whether TSO/GSO can be used for a particular skbuff - checking which
queue the skbuff is destined for and whether that has TBS available
which precludes TSO being enabled on that channel.

I'm also adding a check that the header is smaller than 1024 bytes,
as documented in those sources which have TSO support - this is due
to the hardware buffering the header in "TSO memory" which I guess
is limited to 1KiB. I expect this test never to trigger, but if
the headers ever exceed that size, the hardware will likely fail.

I'm also moving the VLAN insertion for TSO packets into core code -
with the addition of .do_Features_check(), this can be done and
unnecessary code removed from the stmmac driver.

I've changed the hardware initialisation to always enable TSO support
on the channels even if the user requests TSO/GSO to be disabled -
this fixes another issue as pointed out by Jakub in a previous review
of the two patches (now patches 5 and 6.)

I'm moving the setup of the GSO features, cleaning those up, and
adding a warning if platform glue requests this to be enabled but the
hardware has no support. Hopefully this will never trigger if everyone
got the STMMAC_FLAG_TSO_EN flag correct.

Also move the "TSO supported" message to the new
stmmac_set_gso_features() function so keep all this TSO stuff together.

 drivers/net/ethernet/stmicro/stmmac/stmmac.h      |   3 +-
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 135 ++++++++++++++--------
 2 files changed, 92 insertions(+), 46 deletions(-)

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

[PATCH net-next 01/10] net: stmmac: fix TSO support when some channels have TBS available

[Russell King (Oracle)]

According to the STM32MP25xx manual, which is dwmac v5.3, TBS (time
based scheduling) is not permitted for channels which have hardware
TSO enabled. Intel's commit 5e6038b88a57 ("net: stmmac: fix TSO and
TBS feature enabling during driver open") concurs with this, but it
is incomplete.

This commit avoids enabling TSO support on the channels which have
TBS available, which, as far as the hardware is concerned, means we
do not set the TSE bit in the DMA channel's transmit control register.

However, the net device's features apply to all queues(channels), which
means these channels may still be handed TSO skbs to transmit, and the
driver will pass them to stmmac_tso_xmit(). This will generate the
descriptors for TSO, even though the channel has the TSE bit clear.

Fix this by checking whether the queue(channel) has TBS available,
and if it does, fall back to software GSO support.

Fixes: 5e6038b88a57 ("net: stmmac: fix TSO and TBS feature enabling during driver open")
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 35 ++++++++++++++++---
 1 file changed, 31 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index ce51b9c22129..2957ee4a43db 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3619,6 +3619,13 @@ static void stmmac_safety_feat_configuration(struct stmmac_priv *priv)
 	}
 }
 
+static bool stmmac_channel_tso_permitted(struct stmmac_priv *priv,
+					 unsigned int chan)
+{
+	/* TSO and TBS cannot co-exist */
+	return !(priv->dma_conf.tx_queue[chan].tbs & STMMAC_TBS_AVAIL);
+}
+
 /**
  * stmmac_hw_setup - setup mac in a usable state.
  *  @dev : pointer to the device structure.
@@ -3707,10 +3714,7 @@ static int stmmac_hw_setup(struct net_device *dev)
 	/* Enable TSO */
 	if (priv->tso) {
 		for (chan = 0; chan < tx_cnt; chan++) {
-			struct stmmac_tx_queue *tx_q = &priv->dma_conf.tx_queue[chan];
-
-			/* TSO and TBS cannot co-exist */
-			if (tx_q->tbs & STMMAC_TBS_AVAIL)
+			if (!stmmac_channel_tso_permitted(priv, chan))
 				continue;
 
 			stmmac_enable_tso(priv, priv->ioaddr, 1, chan);
@@ -4919,6 +4923,28 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
 	return NETDEV_TX_OK;
 }
 
+static netdev_features_t stmmac_features_check(struct sk_buff *skb,
+					       struct net_device *dev,
+					       netdev_features_t features)
+{
+	u16 queue;
+
+	features = vlan_features_check(skb, features);
+
+	if (skb_is_gso(skb)) {
+		/* STM32MP25xx (dwmac v5.3) states "Do not enable time-based
+		 * scheduling for channels on which the TSO feature is
+		 * enabled." If we have a skb for a channel which has TBS
+		 * enabled, fall back to software GSO.
+		 */
+		queue = skb_get_queue_mapping(skb);
+		if (!stmmac_channel_tso_permitted(netdev_priv(dev), queue))
+			features &= ~NETIF_F_GSO_MASK;
+	}
+
+	return features;
+}
+
 static void stmmac_rx_vlan(struct net_device *dev, struct sk_buff *skb)
 {
 	struct vlan_ethhdr *veth = skb_vlan_eth_hdr(skb);
@@ -7220,6 +7246,7 @@ static void stmmac_get_stats64(struct net_device *dev, struct rtnl_link_stats64
 static const struct net_device_ops stmmac_netdev_ops = {
 	.ndo_open = stmmac_open,
 	.ndo_start_xmit = stmmac_xmit,
+	.ndo_features_check = stmmac_features_check,
 	.ndo_stop = stmmac_release,
 	.ndo_change_mtu = stmmac_change_mtu,
 	.ndo_fix_features = stmmac_fix_features,
-- 
2.47.3

[PATCH net-next 02/10] net: stmmac: add TSO check for header length

[Russell King (Oracle)]

According to the STM32MP151 documentation which covers dwmac v4.2, the
hardware TSO feature can handle header lengths up to a maximum of 1023
bytes.

Add a .ndo_features_check() method implementation to check the header
length meets these requirements, otherwise fall back to software GSO.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 2957ee4a43db..e21ca1c70c6d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -4940,6 +4940,14 @@ static netdev_features_t stmmac_features_check(struct sk_buff *skb,
 		queue = skb_get_queue_mapping(skb);
 		if (!stmmac_channel_tso_permitted(netdev_priv(dev), queue))
 			features &= ~NETIF_F_GSO_MASK;
+
+		/* STM32MP151 (dwmac v4.2) and STM32MP25xx (dwmac v5.3) states
+		 * for TDES2 normal (read format) descriptor that the maximum
+		 * header length supported for the TSO feature is 1023 bytes.
+		 * Fall back to software GSO for these skbs.
+		 */
+		if (skb_headlen(skb) > 1023)
+			features &= ~NETIF_F_GSO_MASK;
 	}
 
 	return features;
-- 
2.47.3

[PATCH net-next 03/10] net: stmmac: move TSO VLAN tag insertion to core code

[Russell King (Oracle)]

stmmac_tso_xmit() checks whether the skbuff is trying to offload
vlan tag insertion to hardware, which from the comment in the code
appears to be buggy when the TSO feature is used.

Rather than stmmac_tso_xmit() inserting the VLAN tag, handle this
in stmmac_features_check() which will then use core net code to
handle this. See net/core/dev.c::validate_xmit_skb()

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 21 +++++++------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index e21ca1c70c6d..ed3e9515cf25 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -4419,19 +4419,6 @@ static netdev_tx_t stmmac_tso_xmit(struct sk_buff *skb, struct net_device *dev)
 	u8 proto_hdr_len, hdr;
 	dma_addr_t des;
 
-	/* Always insert VLAN tag to SKB payload for TSO frames.
-	 *
-	 * Never insert VLAN tag by HW, since segments split by
-	 * TSO engine will be un-tagged by mistake.
-	 */
-	if (skb_vlan_tag_present(skb)) {
-		skb = __vlan_hwaccel_push_inside(skb);
-		if (unlikely(!skb)) {
-			priv->xstats.tx_dropped++;
-			return NETDEV_TX_OK;
-		}
-	}
-
 	nfrags = skb_shinfo(skb)->nr_frags;
 	queue = skb_get_queue_mapping(skb);
 
@@ -4932,6 +4919,14 @@ static netdev_features_t stmmac_features_check(struct sk_buff *skb,
 	features = vlan_features_check(skb, features);
 
 	if (skb_is_gso(skb)) {
+		/* Always insert VLAN tag to SKB payload for TSO frames.
+		 *
+		 * Never insert VLAN tag by HW, since segments split by
+		 * TSO engine will be un-tagged by mistake.
+		 */
+		features &= ~(NETIF_F_HW_VLAN_STAG_TX |
+			      NETIF_F_HW_VLAN_CTAG_TX);
+
 		/* STM32MP25xx (dwmac v5.3) states "Do not enable time-based
 		 * scheduling for channels on which the TSO feature is
 		 * enabled." If we have a skb for a channel which has TBS
-- 
2.47.3

[PATCH net-next 04/10] net: stmmac: always enable channel TSO when supported

[Russell King (Oracle)]

Rather than configuring the channels depending on whether GSO/TSO is
currently enabled by the user, always enable if the hardware has
TSO support and the platform wants TSO to be enabled.

This avoids TSO being disabled on a channel over a suspend/resume
when the user has disabled TSO features, and then the hardware
misbehaves when TSO is re-enabled.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index ed3e9515cf25..f500fcc17ce5 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3712,7 +3712,7 @@ static int stmmac_hw_setup(struct net_device *dev)
 	stmmac_set_rings_length(priv);
 
 	/* Enable TSO */
-	if (priv->tso) {
+	if (priv->dma_cap.tsoen && priv->plat->flags & STMMAC_FLAG_TSO_EN) {
 		for (chan = 0; chan < tx_cnt; chan++) {
 			if (!stmmac_channel_tso_permitted(priv, chan))
 				continue;
-- 
2.47.3

[PATCH net-next 05/10] net: stmmac: fix .ndo_fix_features()

[Russell King (Oracle)]

netdev features documentation requires that .ndo_fix_features() is
stateless: it shouldn't modify driver state. Yet, stmmac_fix_features()
does exactly that, changing whether GSO frames are processed by the
driver.

Move this code to stmmac_set_features() instead, which is the correct
place for it. We don't need to check whether TSO is supported; this
is already handled via the setup of netdev->hw_features, and we are
guaranteed that if netdev->hw_features indicates that a feature is
not supported, .ndo_set_features() won't be called with it set.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index f500fcc17ce5..9730535d2dd8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -6102,14 +6102,6 @@ static netdev_features_t stmmac_fix_features(struct net_device *dev,
 	if (priv->plat->bugged_jumbo && (dev->mtu > ETH_DATA_LEN))
 		features &= ~NETIF_F_CSUM_MASK;
 
-	/* Disable tso if asked by ethtool */
-	if ((priv->plat->flags & STMMAC_FLAG_TSO_EN) && (priv->dma_cap.tsoen)) {
-		if (features & NETIF_F_TSO)
-			priv->tso = true;
-		else
-			priv->tso = false;
-	}
-
 	return features;
 }
 
@@ -6136,6 +6128,8 @@ static int stmmac_set_features(struct net_device *netdev,
 			stmmac_enable_sph(priv, priv->ioaddr, sph_en, chan);
 	}
 
+	priv->tso = !!(features & NETIF_F_TSO);
+
 	if (features & NETIF_F_HW_VLAN_CTAG_RX)
 		priv->hw->hw_vlan_en = true;
 	else
-- 
2.47.3

[PATCH net-next 06/10] net: stmmac: simplify GSO/TSO test in stmmac_xmit()

[Russell King (Oracle)]

The test in stmmac_xmit() to see whether we should pass the skbuff to
stmmac_tso_xmit() is more complex than it needs to be. This test can
be simplified by storing the mask of GSO types that we will pass, and
setting it according to the enabled features.

Note that "tso" is a mis-nomer since commit b776620651a1 ("net:
stmmac: Implement UDP Segmentation Offload"). Also note that this
commit controls both via the TSO feature. We preserve this behaviour
in this commit.

Also, this commit unconditionally accessed skb_shinfo(skb)->gso_type
for all frames, even when skb_is_gso() was false. This access is
eliminated.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac.h  |  3 +-
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 28 +++++++++++--------
 2 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
index 919a93a52390..8ba8f03e1ce0 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
@@ -265,8 +265,9 @@ struct stmmac_priv {
 	u32 rx_coal_frames[MTL_MAX_RX_QUEUES];
 
 	int hwts_tx_en;
+	/* skb_shinfo(skb)->gso_type types that we handle */
+	unsigned int gso_enabled_types;
 	bool tx_path_in_lpi_mode;
-	bool tso;
 	bool sph_active;
 	bool sph_capable;
 	u32 sarc_type;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 9730535d2dd8..8991da2f96a9 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -4369,6 +4369,18 @@ static void stmmac_flush_tx_descriptors(struct stmmac_priv *priv, int queue)
 	stmmac_set_queue_tx_tail_ptr(priv, tx_q, queue, tx_q->cur_tx);
 }
 
+static void stmmac_set_gso_types(struct stmmac_priv *priv, bool tso)
+{
+	if (!tso) {
+		priv->gso_enabled_types = 0;
+	} else {
+		/* Manage oversized TCP frames for GMAC4 device */
+		priv->gso_enabled_types = SKB_GSO_TCPV4 | SKB_GSO_TCPV6;
+		if (priv->plat->core_type == DWMAC_CORE_GMAC4)
+			priv->gso_enabled_types |= SKB_GSO_UDP_L4;
+	}
+}
+
 /**
  *  stmmac_tso_xmit - Tx entry point of the driver for oversized frames (TSO)
  *  @skb : the socket buffer
@@ -4671,7 +4683,6 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
 	u32 queue = skb_get_queue_mapping(skb);
 	int nfrags = skb_shinfo(skb)->nr_frags;
 	unsigned int first_entry, tx_packets;
-	int gso = skb_shinfo(skb)->gso_type;
 	struct stmmac_txq_stats *txq_stats;
 	struct dma_desc *desc, *first_desc;
 	struct stmmac_tx_queue *tx_q;
@@ -4683,14 +4694,9 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
 	if (priv->tx_path_in_lpi_mode && priv->eee_sw_timer_en)
 		stmmac_stop_sw_lpi(priv);
 
-	/* Manage oversized TCP frames for GMAC4 device */
-	if (skb_is_gso(skb) && priv->tso) {
-		if (gso & (SKB_GSO_TCPV4 | SKB_GSO_TCPV6))
-			return stmmac_tso_xmit(skb, dev);
-		if (priv->plat->core_type == DWMAC_CORE_GMAC4 &&
-		    (gso & SKB_GSO_UDP_L4))
-			return stmmac_tso_xmit(skb, dev);
-	}
+	if (skb_is_gso(skb) &&
+	    skb_shinfo(skb)->gso_type & priv->gso_enabled_types)
+		return stmmac_tso_xmit(skb, dev);
 
 	if (priv->est && priv->est->enable &&
 	    priv->est->max_sdu[queue]) {
@@ -6128,7 +6134,7 @@ static int stmmac_set_features(struct net_device *netdev,
 			stmmac_enable_sph(priv, priv->ioaddr, sph_en, chan);
 	}
 
-	priv->tso = !!(features & NETIF_F_TSO);
+	stmmac_set_gso_types(priv, features & NETIF_F_TSO);
 
 	if (features & NETIF_F_HW_VLAN_CTAG_RX)
 		priv->hw->hw_vlan_en = true;
@@ -7863,7 +7869,7 @@ static int __stmmac_dvr_probe(struct device *device,
 		ndev->hw_features |= NETIF_F_TSO | NETIF_F_TSO6;
 		if (priv->plat->core_type == DWMAC_CORE_GMAC4)
 			ndev->hw_features |= NETIF_F_GSO_UDP_L4;
-		priv->tso = true;
+		stmmac_set_gso_types(priv, true);
 		dev_info(priv->device, "TSO feature enabled\n");
 	}
 
-- 
2.47.3

[PATCH net-next 07/10] net: stmmac: split out gso features setup

[Russell King (Oracle)]

Move the GSO features setup into a separate function, co-loated with
other GSO/TSO support.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 21 ++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 8991da2f96a9..4442358e8280 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -4381,6 +4381,19 @@ static void stmmac_set_gso_types(struct stmmac_priv *priv, bool tso)
 	}
 }
 
+static void stmmac_set_gso_features(struct net_device *ndev)
+{
+	struct stmmac_priv *priv = netdev_priv(ndev);
+
+	if ((priv->plat->flags & STMMAC_FLAG_TSO_EN) && (priv->dma_cap.tsoen)) {
+		ndev->hw_features |= NETIF_F_TSO | NETIF_F_TSO6;
+		if (priv->plat->core_type == DWMAC_CORE_GMAC4)
+			ndev->hw_features |= NETIF_F_GSO_UDP_L4;
+		stmmac_set_gso_types(priv, true);
+		dev_info(priv->device, "TSO feature enabled\n");
+	}
+}
+
 /**
  *  stmmac_tso_xmit - Tx entry point of the driver for oversized frames (TSO)
  *  @skb : the socket buffer
@@ -7865,13 +7878,7 @@ static int __stmmac_dvr_probe(struct device *device,
 		ndev->hw_features |= NETIF_F_HW_TC;
 	}
 
-	if ((priv->plat->flags & STMMAC_FLAG_TSO_EN) && (priv->dma_cap.tsoen)) {
-		ndev->hw_features |= NETIF_F_TSO | NETIF_F_TSO6;
-		if (priv->plat->core_type == DWMAC_CORE_GMAC4)
-			ndev->hw_features |= NETIF_F_GSO_UDP_L4;
-		stmmac_set_gso_types(priv, true);
-		dev_info(priv->device, "TSO feature enabled\n");
-	}
+	stmmac_set_gso_features(ndev);
 
 	if (priv->dma_cap.sphen &&
 	    !(priv->plat->flags & STMMAC_FLAG_SPH_DISABLE)) {
-- 
2.47.3

[PATCH net-next 08/10] net: stmmac: make stmmac_set_gso_features() more readable

[Russell King (Oracle)]

Make stmmac_set_gso_features() more readable by adding some whitespace
and getting rid of the indentation.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 20 ++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 4442358e8280..3bfa4bbe857f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -4385,13 +4385,19 @@ static void stmmac_set_gso_features(struct net_device *ndev)
 {
 	struct stmmac_priv *priv = netdev_priv(ndev);
 
-	if ((priv->plat->flags & STMMAC_FLAG_TSO_EN) && (priv->dma_cap.tsoen)) {
-		ndev->hw_features |= NETIF_F_TSO | NETIF_F_TSO6;
-		if (priv->plat->core_type == DWMAC_CORE_GMAC4)
-			ndev->hw_features |= NETIF_F_GSO_UDP_L4;
-		stmmac_set_gso_types(priv, true);
-		dev_info(priv->device, "TSO feature enabled\n");
-	}
+	if (!(priv->plat->flags & STMMAC_FLAG_TSO_EN))
+		return;
+
+	if (!priv->dma_cap.tsoen)
+		return;
+
+	ndev->hw_features |= NETIF_F_TSO | NETIF_F_TSO6;
+	if (priv->plat->core_type == DWMAC_CORE_GMAC4)
+		ndev->hw_features |= NETIF_F_GSO_UDP_L4;
+
+	stmmac_set_gso_types(priv, true);
+
+	dev_info(priv->device, "TSO feature enabled\n");
 }
 
 /**
-- 
2.47.3

[PATCH net-next 09/10] net: stmmac: add warning when TSO is requested but unsupported

[Russell King (Oracle)]

Add a warning message if TSO is requested by the platform glue code but
the core wasn't configured for TSO.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 3bfa4bbe857f..c61ce1282368 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -4388,8 +4388,10 @@ static void stmmac_set_gso_features(struct net_device *ndev)
 	if (!(priv->plat->flags & STMMAC_FLAG_TSO_EN))
 		return;
 
-	if (!priv->dma_cap.tsoen)
+	if (!priv->dma_cap.tsoen) {
+		dev_warn(priv->device, "platform requests unsupported TSO\n");
 		return;
+	}
 
 	ndev->hw_features |= NETIF_F_TSO | NETIF_F_TSO6;
 	if (priv->plat->core_type == DWMAC_CORE_GMAC4)
-- 
2.47.3

[PATCH net-next 10/10] net: stmmac: move "TSO supported" message to stmmac_set_gso_features()

[Russell King (Oracle)]

Move the "TSO supported" message to stmmac_set_gso_features() so that
we group all probe-time TSO stuff in one place.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index c61ce1282368..d281793cd0f9 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -4385,6 +4385,9 @@ static void stmmac_set_gso_features(struct net_device *ndev)
 {
 	struct stmmac_priv *priv = netdev_priv(ndev);
 
+	if (priv->dma_cap.tsoen)
+		dev_info(priv->device, "TSO supported\n");
+
 	if (!(priv->plat->flags & STMMAC_FLAG_TSO_EN))
 		return;
 
@@ -7432,9 +7435,6 @@ static int stmmac_hw_init(struct stmmac_priv *priv)
 		devm_pm_set_wake_irq(priv->device, priv->wol_irq);
 	}
 
-	if (priv->dma_cap.tsoen)
-		dev_info(priv->device, "TSO supported\n");
-
 	if (priv->dma_cap.number_rx_queues &&
 	    priv->plat->rx_queues_to_use > priv->dma_cap.number_rx_queues) {
 		dev_warn(priv->device,
-- 
2.47.3

Re: [PATCH net-next 00/10] net: stmmac: TSO fixes/cleanups

[Russell King (Oracle)]

On Sat, Mar 28, 2026 at 09:36:21PM +0000, Russell King (Oracle) wrote:
> Hot off the press from reading various sources of dwmac information,
> this series attempts to fix the buggy hacks that were previously
> merged, and clean up the code handling this.
> 
> I'm not sure whether "TSO" or "GSO" should be used to describe this
> feature - although it primarily handles TCP, dwmac4 appears to also
> be able to handle UDP.
> 
> In essence, this series adds a .ndo_features_check() method to handle
> whether TSO/GSO can be used for a particular skbuff - checking which
> queue the skbuff is destined for and whether that has TBS available
> which precludes TSO being enabled on that channel.
> 
> I'm also adding a check that the header is smaller than 1024 bytes,
> as documented in those sources which have TSO support - this is due
> to the hardware buffering the header in "TSO memory" which I guess
> is limited to 1KiB. I expect this test never to trigger, but if
> the headers ever exceed that size, the hardware will likely fail.
> 
> I'm also moving the VLAN insertion for TSO packets into core code -
> with the addition of .do_Features_check(), this can be done and
> unnecessary code removed from the stmmac driver.
> 
> I've changed the hardware initialisation to always enable TSO support
> on the channels even if the user requests TSO/GSO to be disabled -
> this fixes another issue as pointed out by Jakub in a previous review
> of the two patches (now patches 5 and 6.)
> 
> I'm moving the setup of the GSO features, cleaning those up, and
> adding a warning if platform glue requests this to be enabled but the
> hardware has no support. Hopefully this will never trigger if everyone
> got the STMMAC_FLAG_TSO_EN flag correct.
> 
> Also move the "TSO supported" message to the new
> stmmac_set_gso_features() function so keep all this TSO stuff together.

There are a few more issues that I would like to fix in this driver
in respect of the TSO support.

STM32MP151 and STM32MP25xx both state that when the TSE bit is set
in the channel Tx control register, TxPBL must be >= 4. We don't
check that is the case. Sadly, the documentation doesn't say whether
it's the TxPBL field value or the burst limit itself (there's the PBLx8
bit which multiplies the values in the field by 8.) Given the unknowns
here, I don't have a solution for this yet.

The other restriction is that the MSS[13:0] value must be greater than
the dwmac core's configured data width in bytes, with a recommendation
that it is 64 bytes or more. MSS[13:0] comes from
skb_shinfo(skb)->gso_size. However, the header plus the MSS size must
not exceed 16383 bytes. I think this can be catered for by adding
another test in the new stmmac_features_check() function:

	if (skb_is_gso(skb)) {
		headlen = skb_headlen(skb);
		gso_size = skb_shinfo(skb)->gso_size;
...
		if (headlen > 1023 || gso_size < 64 ||
		    gso_size + headlen > 16383)
			features &= ~NETIF_F_GSO_MASK;

What I haven't worked out yet is whether any of these conditions
just "can't happen" with our networking layers - I suspect the
gso_size < 64 would be one such case.

Next, the hardware has a maximum size for segmentation, which is 256KiB.
I think that needs netif_set_tso_max_size(dev, 256KiB). We're currently
safe, because the default is 64KiB, and I'd worry about whether we'd
overflow the space in the TX descriptor ring.

If anyone has any comments on the above, that would be good. Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next 03/10] net: stmmac: move TSO VLAN tag insertion to core code

[Russell King (Oracle)]

On Sat, Mar 28, 2026 at 09:36:52PM +0000, Russell King (Oracle) wrote:
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index e21ca1c70c6d..ed3e9515cf25 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -4419,19 +4419,6 @@ static netdev_tx_t stmmac_tso_xmit(struct sk_buff *skb, struct net_device *dev)
>  	u8 proto_hdr_len, hdr;
>  	dma_addr_t des;
>  
> -	/* Always insert VLAN tag to SKB payload for TSO frames.
> -	 *
> -	 * Never insert VLAN tag by HW, since segments split by
> -	 * TSO engine will be un-tagged by mistake.
> -	 */
> -	if (skb_vlan_tag_present(skb)) {
> -		skb = __vlan_hwaccel_push_inside(skb);
> -		if (unlikely(!skb)) {
> -			priv->xstats.tx_dropped++;
> -			return NETDEV_TX_OK;
> -		}
> -	}
> -
>  	nfrags = skb_shinfo(skb)->nr_frags;
>  	queue = skb_get_queue_mapping(skb);
>  
> @@ -4932,6 +4919,14 @@ static netdev_features_t stmmac_features_check(struct sk_buff *skb,
>  	features = vlan_features_check(skb, features);
>  
>  	if (skb_is_gso(skb)) {
> +		/* Always insert VLAN tag to SKB payload for TSO frames.
> +		 *
> +		 * Never insert VLAN tag by HW, since segments split by
> +		 * TSO engine will be un-tagged by mistake.
> +		 */
> +		features &= ~(NETIF_F_HW_VLAN_STAG_TX |
> +			      NETIF_F_HW_VLAN_CTAG_TX);
> +

I'm wondering whether this is the correct place to do this. If as a
result of the following tests we fallback to software GSO, then we
will be submitting "normal" frames to the driver to transmit, which
means it can insert the VLAN tag in hardware.

So, I'm thinking this isn't the correct place for the test, but it
should be after the tests that disable NETIF_F_GSO_MASK and only be
masked out when the features mask still contains any of the
NETIF_F_GSO_MASK features. Anyone concur?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next 06/10] net: stmmac: simplify GSO/TSO test in stmmac_xmit()

[Russell King (Oracle)]

On Sat, Mar 28, 2026 at 09:37:07PM +0000, Russell King (Oracle) wrote:
> +static void stmmac_set_gso_types(struct stmmac_priv *priv, bool tso)
> +{
> +	if (!tso) {
> +		priv->gso_enabled_types = 0;
> +	} else {
> +		/* Manage oversized TCP frames for GMAC4 device */
> +		priv->gso_enabled_types = SKB_GSO_TCPV4 | SKB_GSO_TCPV6;
> +		if (priv->plat->core_type == DWMAC_CORE_GMAC4)
> +			priv->gso_enabled_types |= SKB_GSO_UDP_L4;

I've been wondering whether keying all three of these off NETIF_F_TSO
is correct. Shouldn't SKB_GSP_UDP_L4 be dependent on NETIF_F_GSO_UDP_L4?

(The above code doesn't change the current driver behaviour, so this
would be a separate fix.)

> +	}
> +}
> +
>  /**
>   *  stmmac_tso_xmit - Tx entry point of the driver for oversized frames (TSO)
>   *  @skb : the socket buffer
> @@ -4671,7 +4683,6 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
>  	u32 queue = skb_get_queue_mapping(skb);
>  	int nfrags = skb_shinfo(skb)->nr_frags;
>  	unsigned int first_entry, tx_packets;
> -	int gso = skb_shinfo(skb)->gso_type;
>  	struct stmmac_txq_stats *txq_stats;
>  	struct dma_desc *desc, *first_desc;
>  	struct stmmac_tx_queue *tx_q;
> @@ -4683,14 +4694,9 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
>  	if (priv->tx_path_in_lpi_mode && priv->eee_sw_timer_en)
>  		stmmac_stop_sw_lpi(priv);
>  
> -	/* Manage oversized TCP frames for GMAC4 device */
> -	if (skb_is_gso(skb) && priv->tso) {
> -		if (gso & (SKB_GSO_TCPV4 | SKB_GSO_TCPV6))
> -			return stmmac_tso_xmit(skb, dev);
> -		if (priv->plat->core_type == DWMAC_CORE_GMAC4 &&
> -		    (gso & SKB_GSO_UDP_L4))
> -			return stmmac_tso_xmit(skb, dev);
> -	}
> +	if (skb_is_gso(skb) &&
> +	    skb_shinfo(skb)->gso_type & priv->gso_enabled_types)
> +		return stmmac_tso_xmit(skb, dev);

I'm also wondering whether we should check gso_type in our
.ndo_features_check() method rather than here - if we get a GSO skb at
this point for a type that we don't recognise, surely it is wrong to
pass it via the normal skb transmission flow.

Yet more worms in the stmmac can... :/

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next 01/10] net: stmmac: fix TSO support when some channels have TBS available

[Russell King (Oracle)]

On Sat, Mar 28, 2026 at 09:36:41PM +0000, Russell King (Oracle) wrote:
> According to the STM32MP25xx manual, which is dwmac v5.3, TBS (time
> based scheduling) is not permitted for channels which have hardware
> TSO enabled. Intel's commit 5e6038b88a57 ("net: stmmac: fix TSO and
> TBS feature enabling during driver open") concurs with this, but it
> is incomplete.
> 
> This commit avoids enabling TSO support on the channels which have
> TBS available, which, as far as the hardware is concerned, means we
> do not set the TSE bit in the DMA channel's transmit control register.
> 
> However, the net device's features apply to all queues(channels), which
> means these channels may still be handed TSO skbs to transmit, and the
> driver will pass them to stmmac_tso_xmit(). This will generate the
> descriptors for TSO, even though the channel has the TSE bit clear.
> 
> Fix this by checking whether the queue(channel) has TBS available,
> and if it does, fall back to software GSO support.

This is sufficient for the immediate issue of fixing the patch below,
but I think there's another issue that also needs fixing here.

TSO requires the hardware to support checksum offload, and there is
a comment in the driver:

        /* DWMAC IPs can be synthesized to support tx coe only for a few tx
         * queues. In that case, checksum offloading for those queues that don't
         * support tx coe needs to fallback to software checksum calculation.
         *
         * Packets that won't trigger the COE e.g. most DSA-tagged packets will
         * also have to be checksummed in software.
         */

So, it seems at the very least we need to add a check (in a subsequent
patch) for priv->plat->tx_queues_cfg[queue].coe_unsupported to
stmmac_channel_tso_permitted().

I'm also wondering about the stmmac_has_ip_ethertype() thing, which
checks whether the skb can be checksummed by the hardware, and how that
interacts with TSO, and whether that's yet another hole that needs
plugging.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next 01/10] net: stmmac: fix TSO support when some channels have TBS available

[Andrew Lunn]

>          * Packets that won't trigger the COE e.g. most DSA-tagged packets will
>          * also have to be checksummed in software.
>          */

I doubt DSA will work with TSO, it would have to include the DSA
header into the replicated header when doing segmentation.

I thought we had some code in DSA to turn off features which are
likely to cause issues. But i don't see it.

Hardware checksumming has been an issue in the past with DSA. Some MAC
hardware from switch vendors, Marvell, Broadcom etc, understand the
DSA header and so can do the checksum correctly when the DSA tag is
present. MAC hardware from other vendors often get confused so
software checksumming is needed.

	 Andrew