Date:   Tue, 13 Dec 2016 08:22:54 +0100
From:   Giuseppe CAVALLARO <peppe.cavallaro@...com>
To:     Niklas Cassel <niklas.cassel@...s.com>,
        Joao Pinto <Joao.Pinto@...opsys.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Andy Shevchenko <andy.shevchenko@...il.com>
CC:     David Miller <davem@...emloft.net>, <larper@...s.com>,
        <rabinv@...s.com>, netdev <netdev@...r.kernel.org>,
        <CARLOS.PALMINHA@...opsys.com>, <Jie.Deng1@...opsys.com>,
        Stephen Warren <swarren@...dia.com>, <pavel@....cz>
Subject: Re: Synopsys Ethernet QoS

On 12/12/2016 5:25 PM, Niklas Cassel wrote:
>
>
> On 12/12/2016 11:19 AM, Joao Pinto wrote:
>> Hi,
>>
>> Às 1:44 AM de 12/10/2016, Florian Fainelli escreveu:
>>> Le 12/09/16 à 16:16, Andy Shevchenko a écrit :
>>>> On Sat, Dec 10, 2016 at 12:52 AM, Florian Fainelli <f.fainelli@...il.com> wrote:
>>>>
>>>>> It's kind of sad that customers of that IP (stmmac, amd-xgbe, sxgbe)
>>>>> did
>>>>> actually pioneer the upstreaming effort, but it is good to see people
>>>>> from Synopsys willing to fix that in the future.
>>>> Wait, you would like to tell that we have more than 2 drivers for the
>>>> same (okay, same vendor) IP?!
>>>> It's better to unify them earlier, than have n+ copies.
>>> Unfortunately that is the case, see this email:
>>>
>>> https://www.mail-archive.com/netdev@vger.kernel.org/msg142796.html
>>>
>>> dwc_eth_qos and stmmac have some overlap. There seems to be work
>>> underway to unify these two to begin with.
>>>
>>>> P.S. Though, I don't see how sxgbe got in the list. First glance on
>>>> the code doesn't show similarities.
>>> Well samsung/sxgbe looks potentially similar to amd/xgbe, but that's
>>> just my cursory look at the code, it may very well be something entirely
>>> different. The descriptor formats just look suspiciously similar.
>>>
>> Thank you for your inputs! Renaming seems to be a hotspot. I agree that maybe
>> instead of renaming (breaking retro-compatibility as David and Florian
>> mentioned), the best is to move stmmac to synopsys/ after merging *qos* and
>> removing it. As Florian mentioned, git is capable of detecting folder restructured.
>>
>> @Rabin Vincent: Hi Rabin. Since Axis is more familiar with the synopsys/*qos*
>> driver would it be possible for you to make an initial analysis of what has to
>> be merged into Stmmac? This way the development would speed-up.
>
> I can answer that question.
>
> I've sent out 12 patches to the stmmac driver
> (all patches are included in the current net-next tree),

ok I have seen these patches applied, I had just a minor concern about
the  failure when DMA configuration is missing.
In these years, I have noticed that, for this kind of HW, default DMA
configuration is usually good to have a driver working. AHB, AXI
parameters can be provided to have a best tuning or to fix know issues
on some platforms. So IMO, we should relax the check with a warning.
Please, consider that, the stmmac also supports very old MAC10/100
versions where the DMA configuration was often never passed.

> with these patches the stmmac driver works properly on Axis hardware
> (we use Synopsys GMAC 4.10a synthesized with multiple TX queues).

perfect and thanks a lot for this effort.

> stmmac's DT binding has also been extended with properties that
> existed in DWC EQoS's DT binding, such as no-pbl-x8, txpbl, rxpbl.
>
> Since we have no problem updating the DTB together with the kernel,
> we will simply move to using the start using the stmmac driver,
> with stmmac's DT binding.
>
> However, I've noticed that NVIDIA has extended the DWC EQoS DT binding,
> I don't how easy it would be for them to switch to stmmac's DT binding.
> (Adding Stephen Warren to CC.)

ok

>
> The reset sequence that Lars Persson was worried about is not an issue
> with the stmmac driver.
>

thx for this check.

>
> There are some performance problems with the stmmac driver though:
>
> When running iperf3 with 3 streams:
> iperf3 -c 192.168.0.90 -P 3 -t 30
> iperf3 -c 192.168.0.90 -P 3 -t 30 -R
>
> I get really bad fairness between the streams.

Can you confirm you are using the 4.xxa version?

This doesn't match with Alex's experiments on ARM platforms.

> This appears to be an issue with how TX IRQ coalescing is implemented in stmmac.
> Disabling TX IRQ coalescing in the stmmac driver makes the problem go away.

this doesn't match with what we had seen but I am happy and open to
review and accept new strategy.

> We have a local patch that implements TX IRQ coalescing in the dwceqos driver,
> and we don't see the same problem.

please, if you have new patch add me on CC and we will review all
together.

> Also netperf TCP_RR and UDP_RR gives really bad results compared to the
> dwceqos driver (without IRQ coalescing).
> 2000 transactions/sec vs 9000 transactions/sec.
> Turning TX IRQ coalescing off and RX interrupt watchdog off in stmmac
> gives the same performance. I guess it's a trade off, low CPU usage
> vs low latency, so I don't know how important TCP_RR/UDP_RR really is.
>
> The best thing would be to get a good working TX IRQ coalesce
> implementation with HR timers in stmmac.

as said, welcome patches.

Basically, the default tuning of coalescence parameters comes from
ST platform experiences. I mean, we tuned to driver to have good
performance and saving CPU on SH4 (UP) and ARM (SMP) systems.
In these years, these default was accepted but, if today we need
to change something welcome effort. On my side, I can try to
perform some bench to see if I have regressions on not.

> Perhaps it should also be investigated if the RX interrupt watchdog
> timeout should have a lower default value.

Do not expect to get many improvements to play with the HW watchdog
due to the poor granularity of the Receive Interrupt Watchdog Timer
Count.

Regards
Peppe

>
>
>
>>
>> Thanks to all.
>>
>> Joao
>
>

Powered by blists - more mailing lists