The DMA engine in dwmac4 can segment a large TSO packet to several
smaller packets of (max) size Maximum Segment Size (MSS).
The DMA engine fetches and saves the MSS via a context descriptor.
This context decriptor has to be provided to each tx DMA channel.
To ensure that this is done, move struct member mss from stmmac_priv
to stmmac_tx_queue.
stmmac_reset_queues_param() now also resets mss, together with other
queue parameters, so reset of mss value can be removed from
stmmac_resume().
init_dma_tx_desc_rings() now also resets mss, together with other
queue parameters, so reset of mss value can be removed from
stmmac_open().
This fixes tx queue timeouts for dwmac4, with DT property
snps,tx-queues-to-use > 1, when running iperf3 with multiple threads.
Fixes: ce736788e8a9 ("net: stmmac: adding multiple buffers for TX")
Signed-off-by: Niklas Cassel <redacted>
---
drivers/net/ethernet/stmicro/stmmac/stmmac.h | 2 +-
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 13 +++++--------
2 files changed, 6 insertions(+), 9 deletions(-)
@@ -2632,7 +2634,6 @@ static int stmmac_open(struct net_device *dev)priv->dma_buf_sz=STMMAC_ALIGN(buf_sz);priv->rx_copybreak=STMMAC_RX_COPYBREAK;-priv->mss=0;ret=alloc_dma_desc_resources(priv);if(ret<0){
@@ -2872,10 +2873,10 @@ static netdev_tx_t stmmac_tso_xmit(struct sk_buff *skb, struct net_device *dev)mss=skb_shinfo(skb)->gso_size;/* set new MSS value if needed */-if(mss!=priv->mss){+if(mss!=tx_q->mss){mss_desc=tx_q->dma_tx+tx_q->cur_tx;priv->hw->desc->set_mss(mss_desc,mss);-priv->mss=mss;+tx_q->mss=mss;tx_q->cur_tx=STMMAC_GET_ENTRY(tx_q->cur_tx,DMA_TX_SIZE);}
@@ -4481,11 +4483,6 @@ int stmmac_resume(struct device *dev)stmmac_reset_queues_param(priv);-/* reset private mss value to force mss context settings at-*nexttsoxmit(onlyusedforgmac4).-*/-priv->mss=0;-stmmac_clear_descriptors(priv);stmmac_hw_setup(ndev,false);
tx_skbuff is initialized to NULL in init_dma_tx_desc_rings(), which is
called from ndo_open().
stmmac_tx_clean() frees any non-NULL skb, and sets the tx_skbuff
entry to NULL. Hence, there is no need to set skbuff entries to NULL
in stmmac_xmit()/stmmac_tso_xmit(), and doing so falsely gives the
reader the impression that it is needed.
Do not clear tx_skbuff entries in stmmac_xmit()/stmmac_tso_xmit().
Signed-off-by: Niklas Cassel <redacted>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 3 ---
1 file changed, 3 deletions(-)
Looking at dwmac4_tx_queue_routing(), it is obvious that it
sets up rx queue routing.
Rename dwmac4_tx_queue_routing() to dwmac4_rx_queue_routing()
to better match reality.
Fixes: abe80fdc6ee6 ("net: stmmac: RX queue routing configuration")
Signed-off-by: Niklas Cassel <redacted>
---
drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
The current code assumes that a tx_skbuff entry has been cleared
by stmmac_tx_clean() before stmmac_xmit()/stmmac_tso_xmit()
assigns a new skb to that entry. However, since we never check
the current value before overwriting it, it is theoretically
possible that a non-NULL value is overwritten.
Add WARN_ONs to verify that each entry in tx_skbuff is NULL
before it is assigned a new value.
Signed-off-by: Niklas Cassel <redacted>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 5 +++++
1 file changed, 5 insertions(+)
stmmac_mac_config_rx_queues_routing() incorrectly calls rx_queue_prio()
instead of rx_queue_routing().
This looks like a copy paste issue, since
stmmac_mac_config_rx_queues_prio() already calls rx_queue_prio(),
and both stmmac_mac_config_rx_queues_routing() and
stmmac_mac_config_rx_queues_prio() are very similar in structure.
Fixes: abe80fdc6ee6 ("net: stmmac: RX queue routing configuration")
Signed-off-by: Niklas Cassel <redacted>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
@@ -417,8 +417,9 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)dev_warn(&pdev->dev,"snps,phy-addr property is deprecated\n");/* To Configure PHY by using all device-tree supported properties */-if(stmmac_dt_phy(plat,np,&pdev->dev))-returnERR_PTR(-ENODEV);+rc=stmmac_dt_phy(plat,np,&pdev->dev);+if(rc)+returnERR_PTR(rc);of_property_read_u32(np,"tx-fifo-depth",&plat->tx_fifo_size);
The device tree binding for stmmac says:
- Multiple TX Queues parameters: below the list of all the parameters to
configure the multiple TX queues:
- snps,tx-queues-to-use: number of TX queues to be used in the driver
[...]
- For each TX queue
[...]
However, if one specifies snps,tx-queues-to-use = 2,
but omits the queue subnodes, or defines just one queue subnode,
since the driver appears to initialize queues with sane default
values, we will get tx queue timeouts.
This is because the initialization code only initializes
as many queues as it finds subnodes. Potentially leaving
some queues uninitialized.
To avoid hard to debug issues, return an error if the number
of subnodes differ from snps,tx-queues-to-use/snps,rx-queues-to-use.
Signed-off-by: Niklas Cassel <redacted>
---
.../net/ethernet/stmicro/stmmac/stmmac_platform.c | 29 ++++++++++++++++++----
1 file changed, 24 insertions(+), 5 deletions(-)
@@ -135,13 +135,14 @@ static struct stmmac_axi *stmmac_axi_setup(struct platform_device *pdev)*stmmac_mtl_setup-parseDTparametersformultiplequeuesconfiguration*@pdev:platformdevice*/-staticvoidstmmac_mtl_setup(structplatform_device*pdev,-structplat_stmmacenet_data*plat)+staticintstmmac_mtl_setup(structplatform_device*pdev,+structplat_stmmacenet_data*plat){structdevice_node*q_node;structdevice_node*rx_node;structdevice_node*tx_node;u8queue=0;+intret=0;/* For backwards-compatibility with device trees that don't have any*snps,mtl-rx-configorsnps,mtl-tx-configproperties,wefallback
From: Jose Abreu <Jose.Abreu@synopsys.com> Date: 2018-02-19 18:13:14
Hi Niklas,
On 19-02-2018 17:11, Niklas Cassel wrote:
stmmac multi-queue fixes and cleanups
Niklas Cassel (7):
net: stmmac: set MSS for each tx DMA channel
net: stmmac: do not clear tx_skbuff entries in
stmmac_xmit()/stmmac_tso_xmit()
net: stmmac: WARN if tx_skbuff entries are reused before cleared
net: stmmac: rename dwmac4_tx_queue_routing() to match reality
net: stmmac: call correct function in
stmmac_mac_config_rx_queues_routing()
net: stmmac: add error handling in stmmac_mtl_setup()
net: stmmac: honor error code from stmmac_dt_phy()
Thanks! The whole series is:
Reviewed-by: Jose Abreu <redacted>
Best Regards,
Jose Miguel Abreu
From: Giuseppe CAVALLARO <hidden> Date: 2018-02-20 07:31:17
Hi Niklas
I had a look at this series that looks ok for me.
Acked-by: Giuseppe Cavallaro <redacted>
On 2/19/2018 6:11 PM, Niklas Cassel wrote:
quoted hunk
The DMA engine in dwmac4 can segment a large TSO packet to several
smaller packets of (max) size Maximum Segment Size (MSS).
The DMA engine fetches and saves the MSS via a context descriptor.
This context decriptor has to be provided to each tx DMA channel.
To ensure that this is done, move struct member mss from stmmac_priv
to stmmac_tx_queue.
stmmac_reset_queues_param() now also resets mss, together with other
queue parameters, so reset of mss value can be removed from
stmmac_resume().
init_dma_tx_desc_rings() now also resets mss, together with other
queue parameters, so reset of mss value can be removed from
stmmac_open().
This fixes tx queue timeouts for dwmac4, with DT property
snps,tx-queues-to-use > 1, when running iperf3 with multiple threads.
Fixes: ce736788e8a9 ("net: stmmac: adding multiple buffers for TX")
Signed-off-by: Niklas Cassel <redacted>
---
drivers/net/ethernet/stmicro/stmmac/stmmac.h | 2 +-
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 13 +++++--------
2 files changed, 6 insertions(+), 9 deletions(-)
@@ -2632,7 +2634,6 @@ static int stmmac_open(struct net_device *dev)priv->dma_buf_sz=STMMAC_ALIGN(buf_sz);priv->rx_copybreak=STMMAC_RX_COPYBREAK;-priv->mss=0;ret=alloc_dma_desc_resources(priv);if(ret<0){
@@ -2872,10 +2873,10 @@ static netdev_tx_t stmmac_tso_xmit(struct sk_buff *skb, struct net_device *dev)mss=skb_shinfo(skb)->gso_size;/* set new MSS value if needed */-if(mss!=priv->mss){+if(mss!=tx_q->mss){mss_desc=tx_q->dma_tx+tx_q->cur_tx;priv->hw->desc->set_mss(mss_desc,mss);-priv->mss=mss;+tx_q->mss=mss;tx_q->cur_tx=STMMAC_GET_ENTRY(tx_q->cur_tx,DMA_TX_SIZE);}
@@ -4481,11 +4483,6 @@ int stmmac_resume(struct device *dev)stmmac_reset_queues_param(priv);-/* reset private mss value to force mss context settings at-*nexttsoxmit(onlyusedforgmac4).-*/-priv->mss=0;-stmmac_clear_descriptors(priv);stmmac_hw_setup(ndev,false);