Date:   Tue, 13 Dec 2016 10:38:17 +0100
From:   Niklas Cassel <niklas.cassel@...s.com>
To:     Giuseppe CAVALLARO <peppe.cavallaro@...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/13/2016 08:22 AM, Giuseppe CAVALLARO wrote:
> 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.
>

This might be a bit off-topic, but:
The patch should not affect any existing code.
All glue drivers uses stmmac_probe_config_dt,
which sets a default value if the property is missing or zero.
The PCI glue driver sets the property explicitly.
Hence, all current code should not be affected.
The error check was added as a sanity check, just in case some
new code is added, which bypasses stmmac_probe_config_dt,
lets say support for GMAC4 in the PCI glue driver.


>
>>
>> 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?

Yes, 4.10a.
Reading the MAC_Version register gives:
0x00004041
Where SNPSVER is bit 7:0, so 0x41.


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

We are also using an ARM platform.
There is no problem with throughput.
The problem is fairness between the streams,
when using for example 3 streams in iperf3.
Did Alex test this? I could not find Alex mail in the netdev archives,
could you link to it?
The problem goes away when disabling TX IRQ coalescing,
which I guess makes sense, since like David Miller said:

"the driver is using non-highres timers
to implement the TX coalescing.  [...]
1 HZ, which is the lowest granularity of non-highres timers in the
kernel, is variable as well as already too large of a delay for
effective TX coalescing."

We are using CONFIG_HZ=100, not CONFIG_HZ=1000.

So if there is a long time before handling interrupts,
I guess that it makes sense that one stream could
get an advantage in the net scheduler.

If I find the time, and if no one beats me to it, I will try to replace
the normal timers with HR timers + a smaller default timeout.


>
>> 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.

I think you misunderstood me, we have a local patch for the synopsys/dwc_eth_qos.c
version, not for stmmac.

I have been using get_maintainer.pl, so you should have been added to all my patches,
but if I send any further patches, I will make sure that you are not excluded.

Powered by blists - more mailing lists