Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1658435 > unrolled thread

[PATCH net] net: stmmac: fix completely hung TX when using TSO

Started byNiklas Cassel <niklas.cassel@axis.com>
First post2017-06-06 09:30 +0200
Last post2017-06-06 22:30 +0200
Articles 6 — 5 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH net] net: stmmac: fix completely hung TX when using TSO Niklas Cassel <niklas.cassel@axis.com> - 2017-06-06 09:30 +0200
    Re: [PATCH net] net: stmmac: fix completely hung TX when using TSO Giuseppe CAVALLARO <peppe.cavallaro@st.com> - 2017-06-06 10:10 +0200
      Re: [PATCH net] net: stmmac: fix completely hung TX when using TSO Alexandre Torgue <alexandre.torgue@st.com> - 2017-06-06 11:20 +0200
      Re: [PATCH net] net: stmmac: fix completely hung TX when using TSO Niklas Cassel <niklas.cassel@axis.com> - 2017-06-06 12:10 +0200
    Re: [PATCH net] net: stmmac: fix completely hung TX when using TSO Florian Fainelli <f.fainelli@gmail.com> - 2017-06-06 16:40 +0200
    Re: [PATCH net] net: stmmac: fix completely hung TX when using TSO David Miller <davem@davemloft.net> - 2017-06-06 22:30 +0200

#1658435 — [PATCH net] net: stmmac: fix completely hung TX when using TSO

FromNiklas Cassel <niklas.cassel@axis.com>
Date2017-06-06 09:30 +0200
Subject[PATCH net] net: stmmac: fix completely hung TX when using TSO
Message-ID<tPh6a-5ES-11@gated-at.bofh.it>
stmmac_tso_allocator can fail to set the Last Descriptor bit
on a descriptor that actually was the last descriptor.

This happens when the buffer of the last descriptor ends
up having a size of exactly TSO_MAX_BUFF_SIZE.

When the IP eventually reaches the next last descriptor,
which actually has the bit set, the DMA will hang.

When the DMA hangs, we get a tx timeout, however,
since stmmac does not do a complete reset of the IP
in stmmac_tx_timeout, we end up in a state with
completely hung TX.

Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 68a188e74c54..440bea049a7f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2723,7 +2723,7 @@ static void stmmac_tso_allocator(struct stmmac_priv *priv, unsigned int des,
 
 		priv->hw->desc->prepare_tso_tx_desc(desc, 0, buff_size,
 			0, 1,
-			(last_segment) && (buff_size < TSO_MAX_BUFF_SIZE),
+			(last_segment) && (tmp_len <= TSO_MAX_BUFF_SIZE),
 			0, 0);
 
 		tmp_len -= TSO_MAX_BUFF_SIZE;
-- 
2.11.0

[toc] | [next] | [standalone]

#1658469

FromGiuseppe CAVALLARO <peppe.cavallaro@st.com>
Date2017-06-06 10:10 +0200
Message-ID<tPhIS-68H-7@gated-at.bofh.it>
In reply to#1658435
Hi Niklas

I get the point and I acked the patch but Alex, please, can you confirm
that this issue has never seen on your boxes where the TSO has been
fully tested? The initial development (commit f748be531) introduces
the following:
     (last_segment) && (buff_size < TSO_MAX_BUFF_SIZE),
...

On 6/6/2017 9:25 AM, Niklas Cassel wrote:
> stmmac_tso_allocator can fail to set the Last Descriptor bit
> on a descriptor that actually was the last descriptor.
>
> This happens when the buffer of the last descriptor ends
> up having a size of exactly TSO_MAX_BUFF_SIZE.
>
> When the IP eventually reaches the next last descriptor,
> which actually has the bit set, the DMA will hang.
>
> When the DMA hangs, we get a tx timeout, however,
> since stmmac does not do a complete reset of the IP
> in stmmac_tx_timeout, we end up in a state with
> completely hung TX.
>
> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>

Acked-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>

> ---
>   drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 68a188e74c54..440bea049a7f 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -2723,7 +2723,7 @@ static void stmmac_tso_allocator(struct stmmac_priv *priv, unsigned int des,
>   
>   		priv->hw->desc->prepare_tso_tx_desc(desc, 0, buff_size,
>   			0, 1,
> -			(last_segment) && (buff_size < TSO_MAX_BUFF_SIZE),
> +			(last_segment) && (tmp_len <= TSO_MAX_BUFF_SIZE),
>   			0, 0);
>   
>   		tmp_len -= TSO_MAX_BUFF_SIZE;

Regards
Peppe

[toc] | [prev] | [next] | [standalone]

#1658511

FromAlexandre Torgue <alexandre.torgue@st.com>
Date2017-06-06 11:20 +0200
Message-ID<tPiOC-6Mu-5@gated-at.bofh.it>
In reply to#1658469
Hi Guys,

On 06/06/2017 10:00 AM, Giuseppe CAVALLARO wrote:
> Hi Niklas
>
> I get the point and I acked the patch but Alex, please, can you confirm
> that this issue has never seen on your boxes where the TSO has been
> fully tested? The initial development (commit f748be531) introduces
> the following:
>     (last_segment) && (buff_size < TSO_MAX_BUFF_SIZE),

I don't remember to have seen this kind of issue in the past but for 
sure I agree with this patch.

Acked-by: Alexandre TORGUE <alexandre.torgue@st.com>

> ...
>
> On 6/6/2017 9:25 AM, Niklas Cassel wrote:
>> stmmac_tso_allocator can fail to set the Last Descriptor bit
>> on a descriptor that actually was the last descriptor.
>>
>> This happens when the buffer of the last descriptor ends
>> up having a size of exactly TSO_MAX_BUFF_SIZE.
>>
>> When the IP eventually reaches the next last descriptor,
>> which actually has the bit set, the DMA will hang.
>>
>> When the DMA hangs, we get a tx timeout, however,
>> since stmmac does not do a complete reset of the IP
>> in stmmac_tx_timeout, we end up in a state with
>> completely hung TX.
>>
>> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
>
> Acked-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
>
>> ---
>>   drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> index 68a188e74c54..440bea049a7f 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> @@ -2723,7 +2723,7 @@ static void stmmac_tso_allocator(struct
>> stmmac_priv *priv, unsigned int des,
>>             priv->hw->desc->prepare_tso_tx_desc(desc, 0, buff_size,
>>               0, 1,
>> -            (last_segment) && (buff_size < TSO_MAX_BUFF_SIZE),
>> +            (last_segment) && (tmp_len <= TSO_MAX_BUFF_SIZE),
>>               0, 0);
>>             tmp_len -= TSO_MAX_BUFF_SIZE;
>
> Regards
> Peppe
>
>

[toc] | [prev] | [next] | [standalone]

#1658571

FromNiklas Cassel <niklas.cassel@axis.com>
Date2017-06-06 12:10 +0200
Message-ID<tPjB0-7ik-13@gated-at.bofh.it>
In reply to#1658469
On 06/06/2017 10:00 AM, Giuseppe CAVALLARO wrote:
> Hi Niklas

Hello Peppe, Alex

> 
> I get the point and I acked the patch but Alex, please, can you confirm
> that this issue has never seen on your boxes where the TSO has been
> fully tested? The initial development (commit f748be531) introduces
> the following:
>     (last_segment) && (buff_size < TSO_MAX_BUFF_SIZE),
> ...

Since you need a packet where the txbuff size of last descriptor
has the exact size of 16K-1, depending on your workload,
it is possible to run your system for a very long time without
triggering the bug.

We noticed the problem since a test in our test suite fetches
the syslog. The syslog usually has a size that is not constant,
and the size usually increases with time.
During some of the test runs, far from all of them, we would
eventually end up with a packet of the exact size that triggers
the bug.

It should be possible to trigger the bug more easily if you
know what buffer size to write to your TCP socket, together
with some manual TCP corking.


Regards,
Niklas

> 
> On 6/6/2017 9:25 AM, Niklas Cassel wrote:
>> stmmac_tso_allocator can fail to set the Last Descriptor bit
>> on a descriptor that actually was the last descriptor.
>>
>> This happens when the buffer of the last descriptor ends
>> up having a size of exactly TSO_MAX_BUFF_SIZE.
>>
>> When the IP eventually reaches the next last descriptor,
>> which actually has the bit set, the DMA will hang.
>>
>> When the DMA hangs, we get a tx timeout, however,
>> since stmmac does not do a complete reset of the IP
>> in stmmac_tx_timeout, we end up in a state with
>> completely hung TX.
>>
>> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
> 
> Acked-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> 
>> ---
>>   drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> index 68a188e74c54..440bea049a7f 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> @@ -2723,7 +2723,7 @@ static void stmmac_tso_allocator(struct stmmac_priv *priv, unsigned int des,
>>             priv->hw->desc->prepare_tso_tx_desc(desc, 0, buff_size,
>>               0, 1,
>> -            (last_segment) && (buff_size < TSO_MAX_BUFF_SIZE),
>> +            (last_segment) && (tmp_len <= TSO_MAX_BUFF_SIZE),
>>               0, 0);
>>             tmp_len -= TSO_MAX_BUFF_SIZE;
> 
> Regards
> Peppe
> 
>

[toc] | [prev] | [next] | [standalone]

#1658821

FromFlorian Fainelli <f.fainelli@gmail.com>
Date2017-06-06 16:40 +0200
Message-ID<tPnOi-1Aw-25@gated-at.bofh.it>
In reply to#1658435

On 06/06/2017 12:25 AM, Niklas Cassel wrote:
> stmmac_tso_allocator can fail to set the Last Descriptor bit
> on a descriptor that actually was the last descriptor.
> 
> This happens when the buffer of the last descriptor ends
> up having a size of exactly TSO_MAX_BUFF_SIZE.
> 
> When the IP eventually reaches the next last descriptor,
> which actually has the bit set, the DMA will hang.
> 
> When the DMA hangs, we get a tx timeout, however,
> since stmmac does not do a complete reset of the IP
> in stmmac_tx_timeout, we end up in a state with
> completely hung TX.
> 
> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>

This should have:

Fixes: f748be531d70 ("stmmac: support new GMAC4")

right?

> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 68a188e74c54..440bea049a7f 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -2723,7 +2723,7 @@ static void stmmac_tso_allocator(struct stmmac_priv *priv, unsigned int des,
>  
>  		priv->hw->desc->prepare_tso_tx_desc(desc, 0, buff_size,
>  			0, 1,
> -			(last_segment) && (buff_size < TSO_MAX_BUFF_SIZE),
> +			(last_segment) && (tmp_len <= TSO_MAX_BUFF_SIZE),
>  			0, 0);
>  
>  		tmp_len -= TSO_MAX_BUFF_SIZE;
> 

-- 
Florian

[toc] | [prev] | [next] | [standalone]

#1659176

FromDavid Miller <davem@davemloft.net>
Date2017-06-06 22:30 +0200
Message-ID<tPth0-5bW-9@gated-at.bofh.it>
In reply to#1658435
From: Niklas Cassel <niklas.cassel@axis.com>
Date: Tue, 6 Jun 2017 09:25:00 +0200

> stmmac_tso_allocator can fail to set the Last Descriptor bit
> on a descriptor that actually was the last descriptor.
> 
> This happens when the buffer of the last descriptor ends
> up having a size of exactly TSO_MAX_BUFF_SIZE.
> 
> When the IP eventually reaches the next last descriptor,
> which actually has the bit set, the DMA will hang.
> 
> When the DMA hangs, we get a tx timeout, however,
> since stmmac does not do a complete reset of the IP
> in stmmac_tx_timeout, we end up in a state with
> completely hung TX.
> 
> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>

Applied and queued up for -stable, thank you.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web