Hello again,
On Sat, Aug 26, 2017 at 12:38 PM, Kristian Evensen
<kristian.even...@gmail.com> wrote:
> Hi,
>
> On Sat, Aug 26, 2017 at 7:43 AM, Mingyu Li <igv...@gmail.com> wrote:
>> Hi.
>>
>> i check the code again. found xmit_more can cause tx timeout. you can
>> reference this.
>> https://www.mail-archive.com/netdev@vger.kernel.org/msg123334.html
>> so the patch should be like this. edit mtk_eth_soc.c
>>
>> tx_num = fe_cal_txd_req(skb);
>> if (unlikely(fe_empty_txd(ring) <= tx_num)) {
>> + if (skb->xmit_more)
>> + fe_reg_w32(ring->tx_next_idx, FE_REG_TX_CTX_IDX0);
>> netif_stop_queue(dev);
>> netif_err(priv, tx_queued, dev,
>> "Tx Ring full when queue awake!\n");
>>
>> but i am not sure. maybe the pause frame cause the problem.
>
> Thanks for the patch. I tested it, but I unfortunately still see the
> error. I also added a print-statement inside the conditional and can
> see that the condition is never hit. I also don't see the "Tx Ring
> full"-message. One difference which I noticed now though, is that I
> don't see the bursty bandwidth pattern I described earlier (32, 0, 32,
> 0, ...). With your patch, it is always 32, 0, crash.
I spent some more time looking into this today and think I might have
been able to solve the issue. My test has been running for ~2 hours
now without any errors (before it would best-case work for 10-15
minutes), and I do not see any performance regressions. Before going
into detail, I should probably point out that I am not very familiar
with driver development, so my observation changes might not be
appropriate/correct :)
I guess our common theory is that something is causing TX interrupts
not to be enabled again. After reading up on NAPI
(https://wiki.linuxfoundation.org/networking/napi), it seems that the
recommended way of using NAPI on clear-on-write devices (like the
RT5350) is to use NAPI for RX and do TX in the interrupt handler. In
the current driver, both TX and RX is handled in the NAPI-callback
fe_poll(). I have modified the driver to split RX and TX, so now
fe_poll() only deals with RX and TX is dealt with in fe_handle_irq().
I have attached the (messy) patch I am currently testing. If this is
an OK solution, I will clean up the patch and submit is to the list. I
will leave my tests running overnight and report back if anything pops
up.
I guess that Johns new driver is the future for mtk_sock_eth, but I
believe that fixing this issue for the current driver is worthwhile.
As things are now, is it possible to DDOS RT5350-based routers running
LEDE 17.01 by just sending the correct type of traffic.
-Kristian
From 98ce0313cd8654fe69028c19b6f08da1d0671d75 Mon Sep 17 00:00:00 2001
From: Kristian Evensen <kristian.even...@gmail.com>
Date: Sat, 26 Aug 2017 16:05:44 +0200
Subject: [PATCH] [FIX] Move TX out of Napi
This is a broken patch only meant for backup.
---
.../drivers/net/ethernet/mtk/mtk_eth_soc.c | 36 +++++++++++++++++-----
1 file changed, 28 insertions(+), 8 deletions(-)
diff --git a/target/linux/ramips/files-4.9/drivers/net/ethernet/mtk/mtk_eth_soc.c b/target/linux/ramips/files-4.9/drivers/net/ethernet/mtk/mtk_eth_soc.c
index 5b354a6563..af865826e8 100644
--- a/target/linux/ramips/files-4.9/drivers/net/ethernet/mtk/mtk_eth_soc.c
+++ b/target/linux/ramips/files-4.9/drivers/net/ethernet/mtk/mtk_eth_soc.c
@@ -684,10 +684,13 @@ static int fe_tx_map_dma(struct sk_buff *skb, struct net_device *dev,
*/
wmb();
if (unlikely(fe_empty_txd(ring) <= ring->tx_thresh)) {
+ pr_info_ratelimited("netif_stop_queue %u %u\n", fe_empty_txd(ring), ring->tx_thresh);
netif_stop_queue(dev);
smp_mb();
- if (unlikely(fe_empty_txd(ring) > ring->tx_thresh))
+ if (unlikely(fe_empty_txd(ring) > ring->tx_thresh)) {
+ pr_info_ratelimited("netif_wake_queue\n");
netif_wake_queue(dev);
+ }
}
if (netif_xmit_stopped(netdev_get_tx_queue(dev, 0)) || !skb->xmit_more)
@@ -781,6 +784,10 @@ static int fe_start_xmit(struct sk_buff *skb, struct net_device *dev)
tx_num = fe_cal_txd_req(skb);
if (unlikely(fe_empty_txd(ring) <= tx_num)) {
+ if (skb->xmit_more) {
+ pr_info_ratelimited("SKB XMIT_MORE\n");
+ fe_reg_w32(ring->tx_next_idx, FE_REG_TX_CTX_IDX0);
+ }
netif_stop_queue(dev);
netif_err(priv, tx_queued, dev,
"Tx Ring full when queue awake!\n");
@@ -967,11 +974,12 @@ static int fe_poll(struct napi_struct *napi, int budget)
status_reg = FE_REG_FE_INT_STATUS;
}
+ /*
if (status & tx_intr) {
fe_reg_w32(tx_intr, FE_REG_FE_INT_STATUS);
tx_done = fe_poll_tx(priv);
status = fe_reg_r32(FE_REG_FE_INT_STATUS);
- }
+ }*/
if (status & rx_intr)
rx_done = fe_poll_rx(napi, budget, priv, rx_intr);
@@ -1042,18 +1050,30 @@ static irqreturn_t fe_handle_irq(int irq, void *dev)
{
struct fe_priv *priv = netdev_priv(dev);
u32 status, int_mask;
+ u32 tx_intr, rx_intr;
+
+ tx_intr = priv->soc->tx_int;
+ rx_intr = priv->soc->rx_int;
status = fe_reg_r32(FE_REG_FE_INT_STATUS);
if (unlikely(!status))
return IRQ_NONE;
- int_mask = (priv->soc->rx_int | priv->soc->tx_int);
+ int_mask = tx_intr | rx_intr;
if (likely(status & int_mask)) {
- if (likely(napi_schedule_prep(&priv->rx_napi))) {
- fe_int_disable(int_mask);
- __napi_schedule(&priv->rx_napi);
- }
+ if (status & rx_intr) {
+ if (likely(napi_schedule_prep(&priv->rx_napi))) {
+ fe_int_disable(rx_intr);
+ __napi_schedule(&priv->rx_napi);
+ }
+ }
+
+ if (status & tx_intr) {
+ fe_reg_w32(tx_intr, FE_REG_FE_INT_STATUS);
+ fe_poll_tx(priv);
+ fe_int_enable(tx_intr);
+ }
} else {
fe_reg_w32(status, FE_REG_FE_INT_STATUS);
}
@@ -1549,7 +1569,7 @@ static int fe_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, netdev);
- netif_info(priv, probe, netdev, "mediatek frame engine at 0x%08lx, irq %d\n",
+ netif_info(priv, probe, netdev, "mediatek frame engine at 0x%08lx, irq %d - patched\n",
netdev->base_addr, netdev->irq);
return 0;
--
2.11.0
_______________________________________________
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev
