linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v2 0/3] net: stmmac: xgmac: Minor fixes
@ 2025-08-15 16:55 Rohan G Thomas via B4 Relay
  2025-08-15 16:55 ` [PATCH net-next v2 1/3] net: stmmac: xgmac: Do not enable RX FIFO Overflow interrupts Rohan G Thomas via B4 Relay
                   ` (3 more replies)
  0 siblings, 4 replies; 16+ messages in thread
From: Rohan G Thomas via B4 Relay @ 2025-08-15 16:55 UTC (permalink / raw)
  To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Serge Semin,
	Romain Gantois, Jose Abreu, Ong Boon Leong
  Cc: netdev, linux-stm32, linux-arm-kernel, linux-kernel,
	Rohan G Thomas, Matthew Gerlach, Andrew Lunn

This patch series includes following minor fixes for stmmac
dwxgmac driver:

    1. Disable Rx FIFO overflow interrupt for dwxgmac
    2. Correct supported speed modes for dwxgmac
    3. Check for coe-unsupported flag before setting CIC bit of
       Tx Desc3 in the AF_XDP flow

Signed-off-by: Rohan G Thomas <rohan.g.thomas@altera.com>
---
Changes in v2:
- Added Fixes: tags to relevant commits.
- Added a check for synopsys version to enable 10Mbps, 100Mbps support.
- Link to v1: https://lore.kernel.org/r/20250714-xgmac-minor-fixes-v1-0-c34092a88a72@altera.com

---
Rohan G Thomas (3):
      net: stmmac: xgmac: Do not enable RX FIFO Overflow interrupts
      net: stmmac: xgmac: Correct supported speed modes
      net: stmmac: Set CIC bit only for TX queues with COE

 drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c | 13 +++++++++++--
 drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c  |  9 +++++----
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c   |  6 ++++--
 3 files changed, 20 insertions(+), 8 deletions(-)
---
base-commit: 88250d40ed59d2b3c2dff788e9065caa7eb4dba0
change-id: 20250714-xgmac-minor-fixes-40287f221dce

Best regards,
-- 
Rohan G Thomas <rohan.g.thomas@altera.com>




^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH net-next v2 1/3] net: stmmac: xgmac: Do not enable RX FIFO Overflow interrupts
  2025-08-15 16:55 [PATCH net-next v2 0/3] net: stmmac: xgmac: Minor fixes Rohan G Thomas via B4 Relay
@ 2025-08-15 16:55 ` Rohan G Thomas via B4 Relay
  2025-08-15 16:55 ` [PATCH net-next v2 2/3] net: stmmac: xgmac: Correct supported speed modes Rohan G Thomas via B4 Relay
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 16+ messages in thread
From: Rohan G Thomas via B4 Relay @ 2025-08-15 16:55 UTC (permalink / raw)
  To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Serge Semin,
	Romain Gantois, Jose Abreu, Ong Boon Leong
  Cc: netdev, linux-stm32, linux-arm-kernel, linux-kernel,
	Rohan G Thomas, Matthew Gerlach, Andrew Lunn

From: Rohan G Thomas <rohan.g.thomas@altera.com>

Enabling RX FIFO Overflow interrupts is counterproductive
and causes an interrupt storm when RX FIFO overflows.
Disabling this interrupt has no side effect and eliminates
interrupt storms when the RX FIFO overflows.

Commit 8a7cb245cf28 ("net: stmmac: Do not enable RX FIFO
overflow interrupts") disables RX FIFO overflow interrupts
for DWMAC4 IP and removes the corresponding handling of
this interrupt. This patch is doing the same thing for
XGMAC IP.

Fixes: 2142754f8b9c ("net: stmmac: Add MAC related callbacks for XGMAC2")
Signed-off-by: Rohan G Thomas <rohan.g.thomas@altera.com>
Reviewed-by: Matthew Gerlach <matthew.gerlach@altera.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
index 5dcc95bc0ad28b756accf9670c5fa00aa94fcfe3..7201a38842651a865493fce0cefe757d6ae9bafa 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
@@ -203,10 +203,6 @@ static void dwxgmac2_dma_rx_mode(struct stmmac_priv *priv, void __iomem *ioaddr,
 	}
 
 	writel(value, ioaddr + XGMAC_MTL_RXQ_OPMODE(channel));
-
-	/* Enable MTL RX overflow */
-	value = readl(ioaddr + XGMAC_MTL_QINTEN(channel));
-	writel(value | XGMAC_RXOIE, ioaddr + XGMAC_MTL_QINTEN(channel));
 }
 
 static void dwxgmac2_dma_tx_mode(struct stmmac_priv *priv, void __iomem *ioaddr,

-- 
2.32.0




^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH net-next v2 2/3] net: stmmac: xgmac: Correct supported speed modes
  2025-08-15 16:55 [PATCH net-next v2 0/3] net: stmmac: xgmac: Minor fixes Rohan G Thomas via B4 Relay
  2025-08-15 16:55 ` [PATCH net-next v2 1/3] net: stmmac: xgmac: Do not enable RX FIFO Overflow interrupts Rohan G Thomas via B4 Relay
@ 2025-08-15 16:55 ` Rohan G Thomas via B4 Relay
  2025-08-20  1:19   ` Jakub Kicinski
  2025-08-15 16:55 ` [PATCH net-next v2 3/3] net: stmmac: Set CIC bit only for TX queues with COE Rohan G Thomas via B4 Relay
  2025-08-20  1:20 ` [PATCH net-next v2 0/3] net: stmmac: xgmac: Minor fixes Jakub Kicinski
  3 siblings, 1 reply; 16+ messages in thread
From: Rohan G Thomas via B4 Relay @ 2025-08-15 16:55 UTC (permalink / raw)
  To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Serge Semin,
	Romain Gantois, Jose Abreu, Ong Boon Leong
  Cc: netdev, linux-stm32, linux-arm-kernel, linux-kernel,
	Rohan G Thomas, Matthew Gerlach

From: Rohan G Thomas <rohan.g.thomas@altera.com>

Correct supported speed modes as per the XGMAC databook.
Commit 9cb54af214a7 ("net: stmmac: Fix IP-cores specific
MAC capabilities") removes support for 10M, 100M and
1000HD. 1000HD is not supported by XGMAC IP, but it does
support 10M and 100M FD mode for XGMAC version >= 2_20,
and it also supports 10M and 100M HD mode if the HDSEL bit
is set in the MAC_HW_FEATURE0 reg. This commit enables support
for 10M and 100M speed modes for XGMAC IP based on XGMAC
version and MAC capabilities.

Fixes: 9cb54af214a7 ("net: stmmac: Fix IP-cores specific MAC capabilities")
Signed-off-by: Rohan G Thomas <rohan.g.thomas@altera.com>
Reviewed-by: Matthew Gerlach <matthew.gerlach@altera.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c | 13 +++++++++++--
 drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c  |  5 +++++
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
index 6cadf8de4fdfdb18af1a112b883b3d33a53da638..00e929bf280baec7aa8d2a75fc5ceea4a52c9979 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
@@ -49,6 +49,14 @@ static void dwxgmac2_core_init(struct mac_device_info *hw,
 	writel(XGMAC_INT_DEFAULT_EN, ioaddr + XGMAC_INT_EN);
 }
 
+static void dwxgmac2_update_caps(struct stmmac_priv *priv)
+{
+	if (!priv->dma_cap.mbps_10_100)
+		priv->hw->link.caps &= ~(MAC_10 | MAC_100);
+	else if (!priv->dma_cap.half_duplex)
+		priv->hw->link.caps &= ~(MAC_10HD | MAC_100HD);
+}
+
 static void dwxgmac2_set_mac(void __iomem *ioaddr, bool enable)
 {
 	u32 tx = readl(ioaddr + XGMAC_TX_CONFIG);
@@ -1424,6 +1432,7 @@ static void dwxgmac2_set_arp_offload(struct mac_device_info *hw, bool en,
 
 const struct stmmac_ops dwxgmac210_ops = {
 	.core_init = dwxgmac2_core_init,
+	.update_caps = dwxgmac2_update_caps,
 	.set_mac = dwxgmac2_set_mac,
 	.rx_ipc = dwxgmac2_rx_ipc,
 	.rx_queue_enable = dwxgmac2_rx_queue_enable,
@@ -1532,8 +1541,8 @@ int dwxgmac2_setup(struct stmmac_priv *priv)
 		mac->mcast_bits_log2 = ilog2(mac->multicast_filter_bins);
 
 	mac->link.caps = MAC_ASYM_PAUSE | MAC_SYM_PAUSE |
-			 MAC_1000FD | MAC_2500FD | MAC_5000FD |
-			 MAC_10000FD;
+			 MAC_10 | MAC_100 | MAC_1000FD |
+			 MAC_2500FD | MAC_5000FD | MAC_10000FD;
 	mac->link.duplex = 0;
 	mac->link.speed10 = XGMAC_CONFIG_SS_10_MII;
 	mac->link.speed100 = XGMAC_CONFIG_SS_100_MII;
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
index 7201a38842651a865493fce0cefe757d6ae9bafa..18e92b3c6df4200c51ee56f923158e3ae1cf5ef8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
@@ -384,6 +384,9 @@ static int dwxgmac2_get_hw_feature(void __iomem *ioaddr,
 {
 	u32 hw_cap;
 
+	struct stmmac_priv *priv = container_of(dma_cap, struct stmmac_priv,
+						dma_cap);
+
 	/* MAC HW feature 0 */
 	hw_cap = readl(ioaddr + XGMAC_HW_FEATURE0);
 	dma_cap->edma = (hw_cap & XGMAC_HWFEAT_EDMA) >> 31;
@@ -406,6 +409,8 @@ static int dwxgmac2_get_hw_feature(void __iomem *ioaddr,
 	dma_cap->vlhash = (hw_cap & XGMAC_HWFEAT_VLHASH) >> 4;
 	dma_cap->half_duplex = (hw_cap & XGMAC_HWFEAT_HDSEL) >> 3;
 	dma_cap->mbps_1000 = (hw_cap & XGMAC_HWFEAT_GMIISEL) >> 1;
+	if (dma_cap->mbps_1000 && priv->synopsys_id >= DWXGMAC_CORE_2_20)
+		dma_cap->mbps_10_100 = 1;
 
 	/* MAC HW feature 1 */
 	hw_cap = readl(ioaddr + XGMAC_HW_FEATURE1);

-- 
2.32.0




^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH net-next v2 3/3] net: stmmac: Set CIC bit only for TX queues with COE
  2025-08-15 16:55 [PATCH net-next v2 0/3] net: stmmac: xgmac: Minor fixes Rohan G Thomas via B4 Relay
  2025-08-15 16:55 ` [PATCH net-next v2 1/3] net: stmmac: xgmac: Do not enable RX FIFO Overflow interrupts Rohan G Thomas via B4 Relay
  2025-08-15 16:55 ` [PATCH net-next v2 2/3] net: stmmac: xgmac: Correct supported speed modes Rohan G Thomas via B4 Relay
@ 2025-08-15 16:55 ` Rohan G Thomas via B4 Relay
  2025-08-20  1:22   ` Jakub Kicinski
  2025-08-20  1:20 ` [PATCH net-next v2 0/3] net: stmmac: xgmac: Minor fixes Jakub Kicinski
  3 siblings, 1 reply; 16+ messages in thread
From: Rohan G Thomas via B4 Relay @ 2025-08-15 16:55 UTC (permalink / raw)
  To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Serge Semin,
	Romain Gantois, Jose Abreu, Ong Boon Leong
  Cc: netdev, linux-stm32, linux-arm-kernel, linux-kernel,
	Rohan G Thomas, Matthew Gerlach

From: Rohan G Thomas <rohan.g.thomas@altera.com>

Currently, in the AF_XDP transmit paths, the CIC bit of
TX Desc3 is set for all packets. Setting this bit for
packets transmitting through queues that don't support
checksum offloading causes the TX DMA to get stuck after
transmitting some packets. This patch ensures the CIC bit
of TX Desc3 is set only if the TX queue supports checksum
offloading.

Fixes: 132c32ee5bc0 ("net: stmmac: Add TX via XDP zero-copy socket")
Signed-off-by: Rohan G Thomas <rohan.g.thomas@altera.com>
Reviewed-by: Matthew Gerlach <matthew.gerlach@altera.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 9a77390b7f9da4199ad6ac15a2149e2c703900ce..88b7e0aed14428c1884f4c3610c4112e9be8fd59 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2584,6 +2584,7 @@ static bool stmmac_xdp_xmit_zc(struct stmmac_priv *priv, u32 queue, u32 budget)
 	struct netdev_queue *nq = netdev_get_tx_queue(priv->dev, queue);
 	struct stmmac_tx_queue *tx_q = &priv->dma_conf.tx_queue[queue];
 	struct stmmac_txq_stats *txq_stats = &priv->xstats.txq_stats[queue];
+	bool csum = !priv->plat->tx_queues_cfg[queue].coe_unsupported;
 	struct xsk_buff_pool *pool = tx_q->xsk_pool;
 	unsigned int entry = tx_q->cur_tx;
 	struct dma_desc *tx_desc = NULL;
@@ -2671,7 +2672,7 @@ static bool stmmac_xdp_xmit_zc(struct stmmac_priv *priv, u32 queue, u32 budget)
 		}
 
 		stmmac_prepare_tx_desc(priv, tx_desc, 1, xdp_desc.len,
-				       true, priv->mode, true, true,
+				       csum, priv->mode, true, true,
 				       xdp_desc.len);
 
 		stmmac_enable_dma_transmission(priv, priv->ioaddr, queue);
@@ -4983,6 +4984,7 @@ static int stmmac_xdp_xmit_xdpf(struct stmmac_priv *priv, int queue,
 {
 	struct stmmac_txq_stats *txq_stats = &priv->xstats.txq_stats[queue];
 	struct stmmac_tx_queue *tx_q = &priv->dma_conf.tx_queue[queue];
+	bool csum = !priv->plat->tx_queues_cfg[queue].coe_unsupported;
 	unsigned int entry = tx_q->cur_tx;
 	struct dma_desc *tx_desc;
 	dma_addr_t dma_addr;
@@ -5034,7 +5036,7 @@ static int stmmac_xdp_xmit_xdpf(struct stmmac_priv *priv, int queue,
 	stmmac_set_desc_addr(priv, tx_desc, dma_addr);
 
 	stmmac_prepare_tx_desc(priv, tx_desc, 1, xdpf->len,
-			       true, priv->mode, true, true,
+			       csum, priv->mode, true, true,
 			       xdpf->len);
 
 	tx_q->tx_count_frames++;

-- 
2.32.0




^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH net-next v2 2/3] net: stmmac: xgmac: Correct supported speed modes
  2025-08-15 16:55 ` [PATCH net-next v2 2/3] net: stmmac: xgmac: Correct supported speed modes Rohan G Thomas via B4 Relay
@ 2025-08-20  1:19   ` Jakub Kicinski
  2025-08-20  6:57     ` G Thomas, Rohan
  0 siblings, 1 reply; 16+ messages in thread
From: Jakub Kicinski @ 2025-08-20  1:19 UTC (permalink / raw)
  To: Rohan G Thomas via B4 Relay
  Cc: rohan.g.thomas, Andrew Lunn, David S. Miller, Eric Dumazet,
	Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Serge Semin,
	Romain Gantois, Jose Abreu, Ong Boon Leong, netdev, linux-stm32,
	linux-arm-kernel, linux-kernel, Matthew Gerlach

On Sat, 16 Aug 2025 00:55:24 +0800 Rohan G Thomas via B4 Relay wrote:
>  {
>  	u32 hw_cap;
>  
> +	struct stmmac_priv *priv = container_of(dma_cap, struct stmmac_priv,
> +						dma_cap);
> +
>  	/* MAC HW feature 0 */

nit: no empty lines between variable declarations and longest to
shortest, so:

 {
+	struct stmmac_priv *priv = container_of(dma_cap, struct stmmac_priv,
+						dma_cap);
 	u32 hw_cap;


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH net-next v2 0/3] net: stmmac: xgmac: Minor fixes
  2025-08-15 16:55 [PATCH net-next v2 0/3] net: stmmac: xgmac: Minor fixes Rohan G Thomas via B4 Relay
                   ` (2 preceding siblings ...)
  2025-08-15 16:55 ` [PATCH net-next v2 3/3] net: stmmac: Set CIC bit only for TX queues with COE Rohan G Thomas via B4 Relay
@ 2025-08-20  1:20 ` Jakub Kicinski
  2025-08-20  7:17   ` G Thomas, Rohan
  3 siblings, 1 reply; 16+ messages in thread
From: Jakub Kicinski @ 2025-08-20  1:20 UTC (permalink / raw)
  To: Rohan G Thomas via B4 Relay
  Cc: rohan.g.thomas, Andrew Lunn, David S. Miller, Eric Dumazet,
	Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Serge Semin,
	Romain Gantois, Jose Abreu, Ong Boon Leong, netdev, linux-stm32,
	linux-arm-kernel, linux-kernel, Matthew Gerlach, Andrew Lunn

On Sat, 16 Aug 2025 00:55:22 +0800 Rohan G Thomas via B4 Relay wrote:
> Subject: [PATCH net-next v2 0/3] net: stmmac: xgmac: Minor fixes

I left one nit, when you repost please use [PATCH net] rather than
net-next as these will go to the net tree, and 6.17-rc3. Rather
than net-next and therefore 6.18.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH net-next v2 3/3] net: stmmac: Set CIC bit only for TX queues with COE
  2025-08-15 16:55 ` [PATCH net-next v2 3/3] net: stmmac: Set CIC bit only for TX queues with COE Rohan G Thomas via B4 Relay
@ 2025-08-20  1:22   ` Jakub Kicinski
  2025-08-20  7:14     ` G Thomas, Rohan
  0 siblings, 1 reply; 16+ messages in thread
From: Jakub Kicinski @ 2025-08-20  1:22 UTC (permalink / raw)
  To: Rohan G Thomas via B4 Relay
  Cc: rohan.g.thomas, Andrew Lunn, David S. Miller, Eric Dumazet,
	Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Serge Semin,
	Romain Gantois, Jose Abreu, Ong Boon Leong, netdev, linux-stm32,
	linux-arm-kernel, linux-kernel, Matthew Gerlach

On Sat, 16 Aug 2025 00:55:25 +0800 Rohan G Thomas via B4 Relay wrote:
> +	bool csum = !priv->plat->tx_queues_cfg[queue].coe_unsupported;

Hopefully the slight pointer chasing here doesn't impact performance?
XDP itself doesn't support checksum so perhaps we could always pass
false?


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH net-next v2 2/3] net: stmmac: xgmac: Correct supported speed modes
  2025-08-20  1:19   ` Jakub Kicinski
@ 2025-08-20  6:57     ` G Thomas, Rohan
  0 siblings, 0 replies; 16+ messages in thread
From: G Thomas, Rohan @ 2025-08-20  6:57 UTC (permalink / raw)
  To: Jakub Kicinski, Rohan G Thomas via B4 Relay
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni,
	Maxime Coquelin, Alexandre Torgue, Serge Semin, Romain Gantois,
	Jose Abreu, Ong Boon Leong, netdev, linux-stm32, linux-arm-kernel,
	linux-kernel, Matthew Gerlach

Hi Jakub,

Thanks for reviewing the patch.

On 8/20/2025 6:49 AM, Jakub Kicinski wrote:
> On Sat, 16 Aug 2025 00:55:24 +0800 Rohan G Thomas via B4 Relay wrote:
>>   {
>>   	u32 hw_cap;
>>   
>> +	struct stmmac_priv *priv = container_of(dma_cap, struct stmmac_priv,
>> +						dma_cap);
>> +
>>   	/* MAC HW feature 0 */
> 
> nit: no empty lines between variable declarations and longest to
> shortest, so:

Sure, will fix this in the next version.

> 
>   {
> +	struct stmmac_priv *priv = container_of(dma_cap, struct stmmac_priv,
> +						dma_cap);
>   	u32 hw_cap;

Best Regards,
Rohan


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH net-next v2 3/3] net: stmmac: Set CIC bit only for TX queues with COE
  2025-08-20  1:22   ` Jakub Kicinski
@ 2025-08-20  7:14     ` G Thomas, Rohan
  2025-08-20 15:54       ` Jakub Kicinski
  0 siblings, 1 reply; 16+ messages in thread
From: G Thomas, Rohan @ 2025-08-20  7:14 UTC (permalink / raw)
  To: Jakub Kicinski, Rohan G Thomas via B4 Relay
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni,
	Maxime Coquelin, Alexandre Torgue, Serge Semin, Romain Gantois,
	Jose Abreu, Ong Boon Leong, netdev, linux-stm32, linux-arm-kernel,
	linux-kernel, Matthew Gerlach

Hi Jakub,

Thanks for reviewing the patch.

On 8/20/2025 6:52 AM, Jakub Kicinski wrote:
> On Sat, 16 Aug 2025 00:55:25 +0800 Rohan G Thomas via B4 Relay wrote:
>> +	bool csum = !priv->plat->tx_queues_cfg[queue].coe_unsupported;
> 
> Hopefully the slight pointer chasing here doesn't impact performance?
> XDP itself doesn't support checksum so perhaps we could always pass
> false?

I'm not certain whether some XDP applications might be benefiting from
checksum offloading currently, and so passing false unconditionally
could cause regressions.

Best Regards,
Rohan



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH net-next v2 0/3] net: stmmac: xgmac: Minor fixes
  2025-08-20  1:20 ` [PATCH net-next v2 0/3] net: stmmac: xgmac: Minor fixes Jakub Kicinski
@ 2025-08-20  7:17   ` G Thomas, Rohan
  0 siblings, 0 replies; 16+ messages in thread
From: G Thomas, Rohan @ 2025-08-20  7:17 UTC (permalink / raw)
  To: Jakub Kicinski, Rohan G Thomas via B4 Relay
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni,
	Maxime Coquelin, Alexandre Torgue, Serge Semin, Romain Gantois,
	Jose Abreu, Ong Boon Leong, netdev, linux-stm32, linux-arm-kernel,
	linux-kernel, Matthew Gerlach, Andrew Lunn

Hi Jakub,

On 8/20/2025 6:50 AM, Jakub Kicinski wrote:
>   when you repost please use [PATCH net] rather than
> net-next as these will go to the net tree, and 6.17-rc3. Rather
> than net-next and therefore 6.18.

Sure, will take care of it in the next version.

Best Regards,
Rohan


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH net-next v2 3/3] net: stmmac: Set CIC bit only for TX queues with COE
  2025-08-20  7:14     ` G Thomas, Rohan
@ 2025-08-20 15:54       ` Jakub Kicinski
  2025-08-20 15:56         ` Jakub Kicinski
  0 siblings, 1 reply; 16+ messages in thread
From: Jakub Kicinski @ 2025-08-20 15:54 UTC (permalink / raw)
  To: G Thomas, Rohan
  Cc: Rohan G Thomas via B4 Relay, Andrew Lunn, David S. Miller,
	Eric Dumazet, Paolo Abeni, Maxime Coquelin, Alexandre Torgue,
	Serge Semin, Romain Gantois, Jose Abreu, Ong Boon Leong, netdev,
	linux-stm32, linux-arm-kernel, linux-kernel, Matthew Gerlach

On Wed, 20 Aug 2025 12:44:18 +0530 G Thomas, Rohan wrote:
> On 8/20/2025 6:52 AM, Jakub Kicinski wrote:
> > On Sat, 16 Aug 2025 00:55:25 +0800 Rohan G Thomas via B4 Relay wrote:  
> >> +	bool csum = !priv->plat->tx_queues_cfg[queue].coe_unsupported;  
> > 
> > Hopefully the slight pointer chasing here doesn't impact performance?
> > XDP itself doesn't support checksum so perhaps we could always pass
> > false?  
> 
> I'm not certain whether some XDP applications might be benefiting from
> checksum offloading currently

Checksum offload is not supported in real XDP, AFAIK, and in AF_XDP 
the driver must implement a checksum callback which stmmac does not do.
IOW it's not possible to use Tx checksum offload in stmmac today from
XDP.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH net-next v2 3/3] net: stmmac: Set CIC bit only for TX queues with COE
  2025-08-20 15:54       ` Jakub Kicinski
@ 2025-08-20 15:56         ` Jakub Kicinski
  2025-08-21 13:50           ` G Thomas, Rohan
  0 siblings, 1 reply; 16+ messages in thread
From: Jakub Kicinski @ 2025-08-20 15:56 UTC (permalink / raw)
  To: G Thomas, Rohan
  Cc: Rohan G Thomas via B4 Relay, Andrew Lunn, David S. Miller,
	Eric Dumazet, Paolo Abeni, Maxime Coquelin, Alexandre Torgue,
	Serge Semin, Romain Gantois, Jose Abreu, Ong Boon Leong, netdev,
	linux-stm32, linux-arm-kernel, linux-kernel, Matthew Gerlach

On Wed, 20 Aug 2025 08:54:46 -0700 Jakub Kicinski wrote:
> On Wed, 20 Aug 2025 12:44:18 +0530 G Thomas, Rohan wrote:
> > On 8/20/2025 6:52 AM, Jakub Kicinski wrote:  
> > > Hopefully the slight pointer chasing here doesn't impact performance?
> > > XDP itself doesn't support checksum so perhaps we could always pass
> > > false?    
> > 
> > I'm not certain whether some XDP applications might be benefiting from
> > checksum offloading currently  
> 
> Checksum offload is not supported in real XDP, AFAIK, and in AF_XDP 
> the driver must implement a checksum callback which stmmac does not do.
> IOW it's not possible to use Tx checksum offload in stmmac today from
> XDP.

To be clear -- this is just for context. I don't understand the details
of what the CIC bit controls, so the final decision is up to you.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH net-next v2 3/3] net: stmmac: Set CIC bit only for TX queues with COE
  2025-08-20 15:56         ` Jakub Kicinski
@ 2025-08-21 13:50           ` G Thomas, Rohan
  2025-08-21 14:17             ` Jakub Kicinski
  0 siblings, 1 reply; 16+ messages in thread
From: G Thomas, Rohan @ 2025-08-21 13:50 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Rohan G Thomas via B4 Relay, Andrew Lunn, David S. Miller,
	Eric Dumazet, Paolo Abeni, Maxime Coquelin, Alexandre Torgue,
	Serge Semin, Romain Gantois, Jose Abreu, Ong Boon Leong, netdev,
	linux-stm32, linux-arm-kernel, linux-kernel, Matthew Gerlach

Hi Jakub,

On 8/20/2025 9:26 PM, Jakub Kicinski wrote:
> On Wed, 20 Aug 2025 08:54:46 -0700 Jakub Kicinski wrote:
>> On Wed, 20 Aug 2025 12:44:18 +0530 G Thomas, Rohan wrote:
>>> On 8/20/2025 6:52 AM, Jakub Kicinski wrote:
>>>> Hopefully the slight pointer chasing here doesn't impact performance?
>>>> XDP itself doesn't support checksum so perhaps we could always pass
>>>> false?
>>>
>>> I'm not certain whether some XDP applications might be benefiting from
>>> checksum offloading currently
>>
>> Checksum offload is not supported in real XDP, AFAIK, and in AF_XDP
>> the driver must implement a checksum callback which stmmac does not do.
>> IOW it's not possible to use Tx checksum offload in stmmac today from
>> XDP.
> 
> To be clear -- this is just for context. I don't understand the details
> of what the CIC bit controls, so the final decision is up to you.

Currently, in the stmmac driver, even though tmo_request_checksum is not
implemented, checksum offloading is still effectively enabled for AF_XDP
frames, as CIC bit for tx desc are set, which implies checksum
calculation and insertion by hardware for IP packets. So, I'm thinking
it is better to keep it as false only for queues that do not support
COE.

Best Regards,
Rohan


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH net-next v2 3/3] net: stmmac: Set CIC bit only for TX queues with COE
  2025-08-21 13:50           ` G Thomas, Rohan
@ 2025-08-21 14:17             ` Jakub Kicinski
  2025-08-22 12:49               ` G Thomas, Rohan
  0 siblings, 1 reply; 16+ messages in thread
From: Jakub Kicinski @ 2025-08-21 14:17 UTC (permalink / raw)
  To: G Thomas, Rohan
  Cc: Rohan G Thomas via B4 Relay, Andrew Lunn, David S. Miller,
	Eric Dumazet, Paolo Abeni, Maxime Coquelin, Alexandre Torgue,
	Serge Semin, Romain Gantois, Jose Abreu, Ong Boon Leong, netdev,
	linux-stm32, linux-arm-kernel, linux-kernel, Matthew Gerlach

On Thu, 21 Aug 2025 19:20:02 +0530 G Thomas, Rohan wrote:
> > To be clear -- this is just for context. I don't understand the details
> > of what the CIC bit controls, so the final decision is up to you.  
> 
> Currently, in the stmmac driver, even though tmo_request_checksum is not
> implemented, checksum offloading is still effectively enabled for AF_XDP
> frames, as CIC bit for tx desc are set, which implies checksum
> calculation and insertion by hardware for IP packets. So, I'm thinking
> it is better to keep it as false only for queues that do not support
> COE.

Oh, so the device parses the packet and inserts the checksum whether
user asked for it or not? Damn, I guess it may indeed be too late
to fix, but that certainly _not_ how AF_XDP is supposed to work.
The frame should not be modified without user asking for it..


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH net-next v2 3/3] net: stmmac: Set CIC bit only for TX queues with COE
  2025-08-21 14:17             ` Jakub Kicinski
@ 2025-08-22 12:49               ` G Thomas, Rohan
  2025-08-22 14:00                 ` Jakub Kicinski
  0 siblings, 1 reply; 16+ messages in thread
From: G Thomas, Rohan @ 2025-08-22 12:49 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Rohan G Thomas via B4 Relay, Andrew Lunn, David S. Miller,
	Eric Dumazet, Paolo Abeni, Maxime Coquelin, Alexandre Torgue,
	Serge Semin, Romain Gantois, Jose Abreu, Ong Boon Leong, netdev,
	linux-stm32, linux-arm-kernel, linux-kernel, Matthew Gerlach

Hi Jakub,

On 8/21/2025 7:47 PM, Jakub Kicinski wrote:
>> Currently, in the stmmac driver, even though tmo_request_checksum is not
>> implemented, checksum offloading is still effectively enabled for AF_XDP
>> frames, as CIC bit for tx desc are set, which implies checksum
>> calculation and insertion by hardware for IP packets. So, I'm thinking
>> it is better to keep it as false only for queues that do not support
>> COE.
> Oh, so the device parses the packet and inserts the checksum whether
> user asked for it or not? Damn, I guess it may indeed be too late
> to fix, but that certainly_not_ how AF_XDP is supposed to work.
> The frame should not be modified without user asking for it..

Yes, I also agreed. But since not sure, currently any XDP applications
are benefiting from hw checksum, I think it's more reasonable to keep
csum flag as false only for queues that do not support COE, while
maintaining current behavior for queues that do support it. Please let
me know if you think otherwise.

Best Regards,
Rohan


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH net-next v2 3/3] net: stmmac: Set CIC bit only for TX queues with COE
  2025-08-22 12:49               ` G Thomas, Rohan
@ 2025-08-22 14:00                 ` Jakub Kicinski
  0 siblings, 0 replies; 16+ messages in thread
From: Jakub Kicinski @ 2025-08-22 14:00 UTC (permalink / raw)
  To: G Thomas, Rohan
  Cc: Rohan G Thomas via B4 Relay, Andrew Lunn, David S. Miller,
	Eric Dumazet, Paolo Abeni, Maxime Coquelin, Alexandre Torgue,
	Serge Semin, Romain Gantois, Jose Abreu, Ong Boon Leong, netdev,
	linux-stm32, linux-arm-kernel, linux-kernel, Matthew Gerlach

On Fri, 22 Aug 2025 18:19:01 +0530 G Thomas, Rohan wrote:
> On 8/21/2025 7:47 PM, Jakub Kicinski wrote:
> >> Currently, in the stmmac driver, even though tmo_request_checksum is not
> >> implemented, checksum offloading is still effectively enabled for AF_XDP
> >> frames, as CIC bit for tx desc are set, which implies checksum
> >> calculation and insertion by hardware for IP packets. So, I'm thinking
> >> it is better to keep it as false only for queues that do not support
> >> COE.  
> > Oh, so the device parses the packet and inserts the checksum whether
> > user asked for it or not? Damn, I guess it may indeed be too late
> > to fix, but that certainly_not_ how AF_XDP is supposed to work.
> > The frame should not be modified without user asking for it..  
> 
> Yes, I also agreed. But since not sure, currently any XDP applications
> are benefiting from hw checksum, I think it's more reasonable to keep
> csum flag as false only for queues that do not support COE, while
> maintaining current behavior for queues that do support it. Please let
> me know if you think otherwise.

Agreed.


^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2025-08-23  6:36 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-15 16:55 [PATCH net-next v2 0/3] net: stmmac: xgmac: Minor fixes Rohan G Thomas via B4 Relay
2025-08-15 16:55 ` [PATCH net-next v2 1/3] net: stmmac: xgmac: Do not enable RX FIFO Overflow interrupts Rohan G Thomas via B4 Relay
2025-08-15 16:55 ` [PATCH net-next v2 2/3] net: stmmac: xgmac: Correct supported speed modes Rohan G Thomas via B4 Relay
2025-08-20  1:19   ` Jakub Kicinski
2025-08-20  6:57     ` G Thomas, Rohan
2025-08-15 16:55 ` [PATCH net-next v2 3/3] net: stmmac: Set CIC bit only for TX queues with COE Rohan G Thomas via B4 Relay
2025-08-20  1:22   ` Jakub Kicinski
2025-08-20  7:14     ` G Thomas, Rohan
2025-08-20 15:54       ` Jakub Kicinski
2025-08-20 15:56         ` Jakub Kicinski
2025-08-21 13:50           ` G Thomas, Rohan
2025-08-21 14:17             ` Jakub Kicinski
2025-08-22 12:49               ` G Thomas, Rohan
2025-08-22 14:00                 ` Jakub Kicinski
2025-08-20  1:20 ` [PATCH net-next v2 0/3] net: stmmac: xgmac: Minor fixes Jakub Kicinski
2025-08-20  7:17   ` G Thomas, Rohan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).