[Intel-wired-lan] [PATCH v1] igc: Increase timeout value for Speed 100/1000/2500

[Intel-wired-lan] [PATCH v1] igc: Increase timeout value for Speed 100/1000/2500

[Muhammad Husaini Zulkifli]

As the cycle time is set to maximum of 1s, the TX Hang timeout need to
be increase to avoid possible TX Hangs caused by using long Qbv cycles.

Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
---
 drivers/net/ethernet/intel/igc/igc_main.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index c8abd7fb70e5..99fb5641297d 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -5322,7 +5322,9 @@ static void igc_watchdog_task(struct work_struct *work)
 				adapter->tx_timeout_factor = 14;
 				break;
 			case SPEED_100:
-				/* maybe add some timeout factor ? */
+			case SPEED_1000:
+			case SPEED_2500:
+				adapter->tx_timeout_factor = 7;
 				break;
 			}
 
-- 
2.17.1

[Intel-wired-lan] [PATCH v1] igc: Increase timeout value for Speed 100/1000/2500

[Sasha Neftin]

On 7/10/2021 02:45, Muhammad Husaini Zulkifli wrote:
> As the cycle time is set to maximum of 1s, the TX Hang timeout need to
> be increase to avoid possible TX Hangs caused by using long Qbv cycles.
>
> Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
> ---
>   drivers/net/ethernet/intel/igc/igc_main.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
> index c8abd7fb70e5..99fb5641297d 100644
> --- a/drivers/net/ethernet/intel/igc/igc_main.c
> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> @@ -5322,7 +5322,9 @@ static void igc_watchdog_task(struct work_struct *work)
>   				adapter->tx_timeout_factor = 14;
>   				break;
>   			case SPEED_100:
> -				/* maybe add some timeout factor ? */
> +			case SPEED_1000:
> +			case SPEED_2500:
> +				adapter->tx_timeout_factor = 7;
>   				break;
>   			}
>   

Acked-by: Sasha Neftin <sasha.neftin@intel.com>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20210713/c45a2a80/attachment.html>

[Intel-wired-lan] [PATCH v1] igc: Increase timeout value for Speed 100/1000/2500

[Zulkifli, Muhammad Husaini]

Thanks sasha and Vinicius!!

From: Neftin, Sasha <sasha.neftin@intel.com>
Sent: Wednesday, July 14, 2021 12:44 AM
To: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>; intel-wired-lan at osuosl.org; Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Gomes, Vinicius <vinicius.gomes@intel.com>
Subject: Re: [Intel-wired-lan] [PATCH v1] igc: Increase timeout value for Speed 100/1000/2500

On 7/10/2021 02:45, Muhammad Husaini Zulkifli wrote:

As the cycle time is set to maximum of 1s, the TX Hang timeout need to

be increase to avoid possible TX Hangs caused by using long Qbv cycles.



Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com><mailto:muhammad.husaini.zulkifli@intel.com>

---

 drivers/net/ethernet/intel/igc/igc_main.c | 4 +++-

 1 file changed, 3 insertions(+), 1 deletion(-)



diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c

index c8abd7fb70e5..99fb5641297d 100644

--- a/drivers/net/ethernet/intel/igc/igc_main.c

+++ b/drivers/net/ethernet/intel/igc/igc_main.c

@@ -5322,7 +5322,9 @@ static void igc_watchdog_task(struct work_struct *work)

                               adapter->tx_timeout_factor = 14;

                               break;

                        case SPEED_100:

-                              /* maybe add some timeout factor ? */

+                       case SPEED_1000:

+                       case SPEED_2500:

+                              adapter->tx_timeout_factor = 7;

                               break;

                        }



Acked-by: Sasha Neftin <sasha.neftin@intel.com><mailto:sasha.neftin@intel.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20210714/c309837c/attachment-0001.html>

[Intel-wired-lan] [PATCH v1] igc: Increase timeout value for Speed 100/1000/2500

[Paul Menzel]

Dear Muhammad,


Am 10.07.21 um 01:45 schrieb Muhammad Husaini Zulkifli:
> As the cycle time is set to maximum of 1s, the TX Hang timeout need to
> be increase to avoid possible TX Hangs caused by using long Qbv cycles.

s/increase/increased/

Do you have a way how to reproduce the issue?

> Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
> ---
>   drivers/net/ethernet/intel/igc/igc_main.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
> index c8abd7fb70e5..99fb5641297d 100644
> --- a/drivers/net/ethernet/intel/igc/igc_main.c
> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> @@ -5322,7 +5322,9 @@ static void igc_watchdog_task(struct work_struct *work)
>   				adapter->tx_timeout_factor = 14;
>   				break;
>   			case SPEED_100:
> -				/* maybe add some timeout factor ? */
> +			case SPEED_1000:
> +			case SPEED_2500:
> +				adapter->tx_timeout_factor = 7;

Please mention in the commit message, how the timeout factor of 7 was 
determined. Why not any other number.

>   				break;
>   			}


Kind regards,

Paul

[Intel-wired-lan] [PATCH v1] igc: Increase timeout value for Speed 100/1000/2500

[Zulkifli, Muhammad Husaini]

Hi Paul,

Thanks for the comment. 
Replied inline

>-----Original Message-----
>From: Paul Menzel <pmenzel@molgen.mpg.de>
>Sent: Wednesday, July 14, 2021 2:24 PM
>To: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>
>Cc: intel-wired-lan at osuosl.org
>Subject: Re: [Intel-wired-lan] [PATCH v1] igc: Increase timeout value for
>Speed 100/1000/2500
>
>Dear Muhammad,
>
>
>Am 10.07.21 um 01:45 schrieb Muhammad Husaini Zulkifli:
>> As the cycle time is set to maximum of 1s, the TX Hang timeout need to
>> be increase to avoid possible TX Hangs caused by using long Qbv cycles.
>
>s/increase/increased/
>
>Do you have a way how to reproduce the issue?
>
>> Signed-off-by: Muhammad Husaini Zulkifli
>> <muhammad.husaini.zulkifli@intel.com>
>> ---
>>   drivers/net/ethernet/intel/igc/igc_main.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c
>> b/drivers/net/ethernet/intel/igc/igc_main.c
>> index c8abd7fb70e5..99fb5641297d 100644
>> --- a/drivers/net/ethernet/intel/igc/igc_main.c
>> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
>> @@ -5322,7 +5322,9 @@ static void igc_watchdog_task(struct work_struct
>*work)
>>   				adapter->tx_timeout_factor = 14;
>>   				break;
>>   			case SPEED_100:
>> -				/* maybe add some timeout factor ? */
>> +			case SPEED_1000:
>> +			case SPEED_2500:
>> +				adapter->tx_timeout_factor = 7;
>
>Please mention in the commit message, how the timeout factor of 7 was
>determined. Why not any other number.

There is no dedicated number specific in data sheet for this timeout factor.
This time out factor was determined during the debugging  to solve the "tx hang" issues that
we were seeing in some cases especially during Scheduled packet Transmission(etf).

>
>>   				break;
>>   			}
>
>
>Kind regards,
>
>Paul

[Intel-wired-lan] [PATCH v1] igc: Increase timeout value for Speed 100/1000/2500

[Paul Menzel]

Dear Muhammad,


Thank you for your response.

Am 17.07.21 um 16:16 schrieb Zulkifli, Muhammad Husaini:

>> -----Original Message-----
>> From: Paul Menzel <pmenzel@molgen.mpg.de>
>> Sent: Wednesday, July 14, 2021 2:24 PM
>> To: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>
>> Cc: intel-wired-lan at osuosl.org
>> Subject: Re: [Intel-wired-lan] [PATCH v1] igc: Increase timeout value for Speed 100/1000/2500

>> Am 10.07.21 um 01:45 schrieb Muhammad Husaini Zulkifli:
>>> As the cycle time is set to maximum of 1s, the TX Hang timeout need to
>>> be increase to avoid possible TX Hangs caused by using long Qbv cycles.
>>
>> s/increase/increased/
>>
>> Do you have a way how to reproduce the issue?
>>
>>> Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
>>> ---
>>>    drivers/net/ethernet/intel/igc/igc_main.c | 4 +++-
>>>    1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c
>>> b/drivers/net/ethernet/intel/igc/igc_main.c
>>> index c8abd7fb70e5..99fb5641297d 100644
>>> --- a/drivers/net/ethernet/intel/igc/igc_main.c
>>> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
>>> @@ -5322,7 +5322,9 @@ static void igc_watchdog_task(struct work_struct
>> *work)
>>>    				adapter->tx_timeout_factor = 14;
>>>    				break;
>>>    			case SPEED_100:
>>> -				/* maybe add some timeout factor ? */
>>> +			case SPEED_1000:
>>> +			case SPEED_2500:
>>> +				adapter->tx_timeout_factor = 7;
>>
>> Please mention in the commit message, how the timeout factor of 7 was
>> determined. Why not any other number.
> 
> There is no dedicated number specific in data sheet for this timeout factor.
> This time out factor was determined during the debugging  to solve the "tx hang" issues that
> we were seeing in some cases especially during Scheduled packet Transmission(etf).

Thank you for the clarification. Please mention that in the commit 
message of v2.

>>>    				break;
>>>    			}


Kind regards,

Paul