netdev - Re: Synopsys Ethernet QoS

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:   Mon, 19 Dec 2016 11:01:32 +0000
From:   Joao Pinto <Joao.Pinto@...opsys.com>
To:     Pavel Machek <pavel@....cz>, Joao Pinto <Joao.Pinto@...opsys.com>
CC:     Niklas Cassel <niklas.cassel@...s.com>,
        Giuseppe CAVALLARO <peppe.cavallaro@...com>,
        Florian Fainelli <f.fainelli@...il.com>,
        "Andy Shevchenko" <andy.shevchenko@...il.com>,
        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>
Subject: Re: Synopsys Ethernet QoS

Às 10:55 AM de 12/19/2016, Joao Pinto escreveu:
> 
> Hi Pavel,
> 
> Às 5:38 PM de 12/17/2016, Pavel Machek escreveu:
>> Hi!
>>
>>>>> 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.
>>>>>
>>>>
>>>> Can you try something like this? Highres timers will be needed, too,
>>>> but this fixes the logic problem.
>>>>
>>>> You'll need to apply it twice as code is copy&pasted.
>>>>
>>>> Best regards,
>>>> 									Pavel
>>>>
>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>>>
>>>>  	 */
>>>>  	priv->tx_count_frames += nfrags + 1;
>>>>  	if (likely(priv->tx_coal_frames > priv->tx_count_frames)) {
>>>> -		mod_timer(&priv->txtimer,
>>>> -			  STMMAC_COAL_TIMER(priv->tx_coal_timer));
>>>> +		if (priv->tx_count_frames == nfrags + 1)
>>>> +			mod_timer(&priv->txtimer,
>>>> +				  STMMAC_COAL_TIMER(priv->tx_coal_timer));
>>>>  	} else {
>>>>  		priv->tx_count_frames = 0;
>>>>  		priv->hw->desc->set_tx_ic(desc);
>>>>
>>>>
>>>
>>> I know that this is completely of topic, but I am facing a dificulty with
>>> stmmac. I have interrupts, mac well configured rx packets being received
>>> successfully, but TX is not working, resulting in Tx errors = Total TX packets.
>>> I have made a lot of debug and my conclusions is that by some reason when using
>>> stmmac after starting tx dma, the hw state machine enters a deadend state
>>> resulting in those errors. Anyone faced this trouble?
>>
>> SMP or UP system? AFAICT the driver gets the memory barriers wrong. It
>> does not fail completely for me, but still fails rather quickly.
> 
> I was able to put my setup receiving and transmiting packets. The problem is
> that because my hw controller supports multi-queues, I had to enable the TX
> queue 0 in order for it to work.
> 
> Now I am facing a new problem. My DUT with stmmac does not answer to ARP
> messages. When I execute a "ping" the other machine in the nework executes a ARP
> message asking "who is the owner of IP XXXX" which my DUT does not reply. I used
> wireshark and the ARP messages are reaching the DUT machine, but for some reason
> it does not reply. Did you ever face this problem when using stmmac?
> 
> When using my internal driver, it works perfectly, so some detail in the stmmac
> I suppose.
> 
> Tahnks.

Forgot to say that I am using a PC with SMP and a PCI link to the controller.

Thanks.

> 
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> index 3e40578..641b03d 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> @@ -2121,11 +2205,11 @@ static netdev_tx_t stmmac_tso_xmit(struct sk_buff *skb, struct net_device *dev)
>>  	if (mss_desc)
>>  		priv->hw->desc->set_tx_owner(mss_desc);
>>  
>> -	/* The own bit must be the latest setting done when prepare the
>> +	/* The own bit must be the latest setting done when preparing the
>>  	 * descriptor and then barrier is needed to make sure that
>>  	 * all is coherent before granting the DMA engine.
>>  	 */
>> -	smp_wmb();
>> +	wmb();
>>  
>>  	if (netif_msg_pktdata(priv)) {
>>  		pr_info("%s: curr=%d dirty=%d f=%d, e=%d, f_p=%p, nfrags %d\n",
>> @@ -2336,9 +2401,9 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
>>  
>>  		/* The own bit must be the latest setting done when prepare the
>>  		 * descriptor and then barrier is needed to make sure that
>> -		 * all is coherent before granting the DMA engine.
>> +		 * all is coherent before granting access to the DMA engine.
>>  		 */
>> -		smp_wmb();
>> +		wmb();
>>  	}
>>  
>>  	netdev_sent_queue(dev, skb->len);
>>
>> Plus I'd suggest... at least (hand-edited). Driver should really be
>> modified to use readl() when accessing memory that changes.
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> index 3e40578..641b03d 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> @@ -1309,6 +1323,8 @@ static void stmmac_tx_clean(struct stmmac_priv *priv)
>>  		status = priv->hw->desc->tx_status(&priv->dev->stats,
>>  						      &priv->xstats, p,
>>  						      priv->ioaddr);
>> +		rmb();
>> +		
>>  		/* Check if the descriptor is owned by the DMA */
>>  		if (unlikely(status & tx_dma_own))
>>  			break;
>>
>> 									Pavel
>>
>