linux-kernel - Re: [net v2] net: ethernet: mtk_eth_soc: handle dma buffer size soc specific

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 29 May 2024 21:21:54 -0700
From: Florian Fainelli <f.fainelli@...il.com>
To: Bc-bocun Chen (陳柏村)
 <bc-bocun.chen@...iatek.com>, "daniel@...rotopia.org"
 <daniel@...rotopia.org>, "sgoutham@...vell.com" <sgoutham@...vell.com>
Cc: Mark-MC Lee (李明昌) <Mark-MC.Lee@...iatek.com>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
 "linux-mediatek@...ts.infradead.org" <linux-mediatek@...ts.infradead.org>,
 SkyLake Huang (黃啟澤)
 <SkyLake.Huang@...iatek.com>, Sam Shih (史碩三)
 <Sam.Shih@...iatek.com>, "linux@...web.de" <linux@...web.de>,
 "john@...ozen.org" <john@...ozen.org>, "nbd@....name" <nbd@....name>,
 "lorenzo@...nel.org" <lorenzo@...nel.org>,
 "frank-w@...lic-files.de" <frank-w@...lic-files.de>,
 Sean Wang <Sean.Wang@...iatek.com>, "kuba@...nel.org" <kuba@...nel.org>,
 "edumazet@...gle.com" <edumazet@...gle.com>,
 "pabeni@...hat.com" <pabeni@...hat.com>,
 "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
 Steven Liu (劉人豪) <steven.liu@...iatek.com>,
 "matthias.bgg@...il.com" <matthias.bgg@...il.com>,
 "davem@...emloft.net" <davem@...emloft.net>,
 "linux-arm-kernel@...ts.infradead.org"
 <linux-arm-kernel@...ts.infradead.org>,
 "angelogioacchino.delregno@...labora.com"
 <angelogioacchino.delregno@...labora.com>
Subject: Re: [net v2] net: ethernet: mtk_eth_soc: handle dma buffer size soc
 specific



On 5/29/2024 6:46 PM, Bc-bocun Chen (陳柏村) wrote:
> On Wed, 2024-05-29 at 17:50 +0000, Sunil Kovvuri Goutham wrote:
>>   	
>> External email : Please do not click links or open attachments until
>> you have verified the sender or the content.
>>   On Mon, 2024-05-27 at 17:13 +0100, Daniel Golle wrote:
>>>> On Mon, May 27, 2024 at 03:55:55PM GMT, Sunil Kovvuri Goutham
>>> wrote:
>>>>>>
>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Frank Wunderlich <linux@...web.de>
>>>>>>>> Sent: Monday, May 27, 2024 7:52 PM
>>>>>>>> To: Felix Fietkau <nbd@....name>; Sean Wang <
>>>>>>>> sean.wang@...iatek.com>;
>>>>>>>> Mark Lee <Mark-MC.Lee@...iatek.com>; Lorenzo Bianconi
>>>>>>>> <lorenzo@...nel.org>; David S. Miller <
>> davem@...emloft.net>
>>>>> ; Eric
>>>>>>>> Dumazet
>>>>>>>> <edumazet@...gle.com>; Jakub Kicinski <kuba@...nel.org>;
>>>>> Paolo
>>>>>>>> Abeni
>>>>>>>> <pabeni@...hat.com>; Matthias Brugger <
>>>>> matthias.bgg@...il.com>;
>>>>>>>> AngeloGioacchino Del Regno <
>>>>>>>> angelogioacchino.delregno@...labora.com>
>>>>>>>> Cc: Frank Wunderlich <frank-w@...lic-files.de>; John
>>>>> Crispin
>>>>>>>> <john@...ozen.org>; netdev@...r.kernel.org;
>>>>>>>> linux-kernel@...r.kernel.org;
>>>>>>>> linux-arm-kernel@...ts.infradead.org;
>>>>>>>> linux-mediatek@...ts.infradead.org;
>>>>>>>> Daniel Golle <daniel@...rotopia.org>
>>>>>>>> Subject: [net v2] net: ethernet: mtk_eth_soc: handle dma
>>>>> buffer
>>>>>>>> size soc specific
>>>>>>>>
>>>>>>>> From: Frank Wunderlich <frank-w@...lic-files.de>
>>>>>>>>
>>>>>>>> The mainline MTK ethernet driver suffers long time from
>>>>> rarly but
>>>>>>>> annoying tx
>>>>>>>> queue timeouts. We think that this is caused by fixed dma
>>>>> sizes
>>>>>>>> hardcoded for
>>>>>>>> all SoCs.
>>>>>>>>
>>>>>>>> Use the dma-size implementation from SDK in a per SoC
>>>>> manner.
>>>>>>>>
>>>>>>>> Fixes: 656e705243fd ("net-next: mediatek: add support for
>>>>> MT7623
>>>>>>>> ethernet")
>>>>>>>> Suggested-by: Daniel Golle <daniel@...rotopia.org>
>>>>>>>> Signed-off-by: Frank Wunderlich <frank-w@...lic-files.de>
>>>>
>>>>>>
>>>>>> ..............
>>>>>>>>
>>>>>>>> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
>>>>>>>> b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
>>>>>>>> index cae46290a7ae..f1ff1be73926 100644
>>>>>>>> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
>>>>>>>> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
>>>>
>>>>>>
>>>>>> .............
>>>>>>>> @@ -1142,40 +1142,46 @@ static int mtk_init_fq_dma(struct
>>>>> mtk_eth
>>>>>>>> *eth)
>>>>>>>>                              cnt * soc-
>>>>>>>>> tx.desc_size,
>>>>>>>>                              &eth-
>>>>>>>>> phy_scratch_ring,
>>>>>>>>                              GFP_KERNEL);
>>>>
>>>>>>
>>>>>> ..............
>>>>>>>> -  for (i = 0; i < cnt; i++) {
>>>>>>>> -      dma_addr_t addr = dma_addr + i *
>> MTK_QDMA_PAGE_SIZE;
>>>>>>>> -      struct mtk_tx_dma_v2 *txd;
>>>>>>>> +      dma_addr = dma_map_single(eth->dma_dev,
>>>>>>>> +                  eth->scratch_head[j], len *
>>>>>>>> MTK_QDMA_PAGE_SIZE,
>>>>>>>> +                  DMA_FROM_DEVICE);
>>>>>>>>
>>>>
>>>>>>
>>>>>> As per commit msg, the fix is for transmit queue timeouts.
>>>>>> But the DMA buffer changes seems for receive pkts.
>>>>>> Can you please elaborate the connection here.
>>>
>>>>
>>>> *I guess* the memory window used for both, TX and RX DMA
>>> descriptors
>>>> needs to be wisely split to not risk TX queue overruns, depending
>>> on
>>>> the
>>>> SoC speed and without hurting RX performance...
>>>>
>>>> Maybe someone inside MediaTek (I've added to Cc now) and more
>>>> familiar
>>>> with the design can elaborate in more detail.
>>   
>>> We've encountered a transmit queue timeout issue on the MT79888 and
>>> have identified it as being related to the RSS feature.
>>> We suspect this problem arises from a low level of free TX DMADs,
>> the
>>> TX Ring alomost full.
>>> Since RSS is enabled, there are 4 Rx Rings, with each containing
>> 2048
>>> DMADs, totaling 8192 for Rx. In contrast, the Tx Ring has only 2048
>>> DMADs. Tx DMADs will be consumed rapidly during a 10G LAN to 10G WAN
>>> forwarding test, subsequently causing the transmit queue to stop.
>>> Therefore, we reduced the number of Rx DMADs for each ring to
>> balance
>>> both Tx and Rx DMADs, which resolves this issue.
>>
>> Okay, but it’s still not clear why it’s resulting in a transmit
>> timeout.
>> When transmit queue is stopped and after some Tx pkts in the pipeline
>> are flushed out, isn’t
>> Tx queue wakeup not happening ?
>>   
> Yes, the transmit timeout is caused by the Tx queue not waking up.
> The Tx queue stops when the free counter is less than ring->thres, and
> it will wake up once the free counter is greater than ring->thres.
> If the CPU is too late to wake up the Tx queues, it may cause a
> transmit timeout.
> Therefore, we balanced the TX and RX DMADs to improve this error
> situation.

All of this needs to go into the commit message please, thanks!
-- 
Florian