[PATCH] net/ice: fix get link status timeout

[PATCH] net/ice: fix get link status timeout

[Mingjin Ye]

When hw is just started, it will immediately obtain the link status, and
the longest attempt is 1 second. Some NICs are slow to initialize, which
make it fails to obtain the link status.

The patch fixes this issue by modifying the longest attempt to 5 seconds.

Fixes: cf911d90e366 ("net/ice: support link update")
Cc: stable@dpdk.org

Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
---
 drivers/net/ice/ice_ethdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
index 0bc739daf0..eaa556f45c 100644
--- a/drivers/net/ice/ice_ethdev.c
+++ b/drivers/net/ice/ice_ethdev.c
@@ -3927,7 +3927,7 @@ static int
 ice_link_update(struct rte_eth_dev *dev, int wait_to_complete)
 {
 #define CHECK_INTERVAL 100  /* 100ms */
-#define MAX_REPEAT_TIME 10  /* 1s (10 * 100ms) in total */
+#define MAX_REPEAT_TIME 50  /* 5s (50 * 100ms) in total */
 	struct ice_hw *hw = ICE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct ice_link_status link_status;
 	struct rte_eth_link link, old;
-- 
2.25.1

Re: [PATCH] net/ice: fix get link status timeout

[Thomas Monjalon]

06/02/2023 07:22, Mingjin Ye:
> When hw is just started, it will immediately obtain the link status, and
> the longest attempt is 1 second. Some NICs are slow to initialize, which
> make it fails to obtain the link status.
> 
> The patch fixes this issue by modifying the longest attempt to 5 seconds.

What is the consequence?
In which case, DPDK application would be blocked during 5 seconds?

RE: [PATCH] net/ice: fix get link status timeout

[Ye, MingjinX]

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: 2023年2月6日 15:23
> To: Ye, MingjinX <mingjinx.ye@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org; Zhou, YidingX
> <yidingx.zhou@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Zhang,
> Qi Z <qi.z.zhang@intel.com>; Ye, MingjinX <mingjinx.ye@intel.com>;
> david.marchand@redhat.com; ferruh.yigit@amd.com
> Subject: Re: [PATCH] net/ice: fix get link status timeout
> 
> 06/02/2023 07:22, Mingjin Ye:
> > When hw is just started, it will immediately obtain the link status,
> > and the longest attempt is 1 second. Some NICs are slow to initialize,
> > which make it fails to obtain the link status.
> >
> > The patch fixes this issue by modifying the longest attempt to 5 seconds.
> 
> What is the consequence?
DPDK could not get link status. At this point, the link status obtained through
the pmd API is wrong.

> In which case, DPDK application would be blocked during 5 seconds?
When the dpdk application startup port is used, it will be blocked for up
to 5 seconds to ensure that the connection status can be obtained.
> 

Re: [PATCH] net/ice: fix get link status timeout

[Thomas Monjalon]

06/02/2023 09:14, Ye, MingjinX:
> From: Thomas Monjalon <thomas@monjalon.net>
> > 06/02/2023 07:22, Mingjin Ye:
> > > When hw is just started, it will immediately obtain the link status,
> > > and the longest attempt is 1 second. Some NICs are slow to initialize,
> > > which make it fails to obtain the link status.
> > >
> > > The patch fixes this issue by modifying the longest attempt to 5 seconds.
> > 
> > What is the consequence?
> 
> DPDK could not get link status. At this point, the link status obtained through
> the pmd API is wrong.
> 
> > In which case, DPDK application would be blocked during 5 seconds?
> 
> When the dpdk application startup port is used, it will be blocked for up
> to 5 seconds to ensure that the connection status can be obtained.

I mean what is the consequence of the increase?
For example, if the port is not connected (no wire),
are we going to wait 5 seconds?

I guess it's OK because it won't wait at all if using rte_eth_link_get_nowait.
It may be interesting to note in the commit message.

RE: [PATCH] net/ice: fix get link status timeout

[Ye, MingjinX]

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: 2023年2月6日 17:16
> To: Ye, MingjinX <mingjinx.ye@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org; Zhou, YidingX
> <yidingx.zhou@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Zhang,
> Qi Z <qi.z.zhang@intel.com>; david.marchand@redhat.com;
> ferruh.yigit@amd.com
> Subject: Re: [PATCH] net/ice: fix get link status timeout
> 
> 06/02/2023 09:14, Ye, MingjinX:
> > From: Thomas Monjalon <thomas@monjalon.net>
> > > 06/02/2023 07:22, Mingjin Ye:
> > > > When hw is just started, it will immediately obtain the link
> > > > status, and the longest attempt is 1 second. Some NICs are slow to
> > > > initialize, which make it fails to obtain the link status.
> > > >
> > > > The patch fixes this issue by modifying the longest attempt to 5 seconds.
> > >
> > > What is the consequence?
> >
> > DPDK could not get link status. At this point, the link status
> > obtained through the pmd API is wrong.
> >
> > > In which case, DPDK application would be blocked during 5 seconds?
> >
> > When the dpdk application startup port is used, it will be blocked for
> > up to 5 seconds to ensure that the connection status can be obtained.
> 
> I mean what is the consequence of the increase?
There will be no side effects. 

> For example, if the port is not connected (no wire), are we going to wait 5
> seconds?
This will cause a wait of up to 5 seconds.
> 
> I guess it's OK because it won't wait at all if using rte_eth_link_get_nowait.
> It may be interesting to note in the commit message.
The reason for pushing this patch is that the E823 NIC needs about 2 seconds
to get the link up state when the port is started, so the waiting time needs to
be extended. 
Modified to 5 seconds, consistent with other timeouts of DPDK.
> 

Re: [PATCH] net/ice: fix get link status timeout

[韩爽]

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Monday, February 06, 2023 3:23 PM
> To: Mingjin Ye <mingjinx.ye@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org; yidingx.zhou@intel.com; Qiming Yang
> <qiming.yang@intel.com>; Qi Zhang <qi.z.zhang@intel.com>; Mingjin Ye
> <mingjinx.ye@intel.com>; david.marchand@redhat.com;
> ferruh.yigit@amd.com
> Subject: Re: [PATCH] net/ice: fix get link status timeout
> 
> 06/02/2023 07:22, Mingjin Ye:
> > When hw is just started, it will immediately obtain the link status,
> > and the longest attempt is 1 second. Some NICs are slow to initialize,
> > which make it fails to obtain the link status.
> >
> > The patch fixes this issue by modifying the longest attempt to 5 seconds.
> 
> What is the consequence?
> In which case, DPDK application would be blocked during 5 seconds?
> 

I think ice_link_update with wait_to_complete needs to be optimized when ice_link_update.
Our system integrates dozens or more interfaces(E810), even if wait 1 second, the system starts very slowly.

RE: [PATCH] net/ice: fix get link status timeout

[Ye, MingjinX]

> -----Original Message-----
> From: 韩爽 <hanshuang@qianxin.com>
> Sent: 2023年2月6日 15:57
> To: Thomas Monjalon <thomas@monjalon.net>; Ye, MingjinX
> <mingjinx.ye@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org; Zhou, YidingX
> <yidingx.zhou@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Zhang,
> Qi Z <qi.z.zhang@intel.com>; Ye, MingjinX <mingjinx.ye@intel.com>;
> david.marchand@redhat.com; ferruh.yigit@amd.com
> Subject: Re: [PATCH] net/ice: fix get link status timeout
> 
> 
> 
> > -----Original Message-----
> > From: Thomas Monjalon <thomas@monjalon.net>
> > Sent: Monday, February 06, 2023 3:23 PM
> > To: Mingjin Ye <mingjinx.ye@intel.com>
> > Cc: dev@dpdk.org; stable@dpdk.org; yidingx.zhou@intel.com; Qiming
> Yang
> > <qiming.yang@intel.com>; Qi Zhang <qi.z.zhang@intel.com>; Mingjin Ye
> > <mingjinx.ye@intel.com>; david.marchand@redhat.com;
> > ferruh.yigit@amd.com
> > Subject: Re: [PATCH] net/ice: fix get link status timeout
> >
> > 06/02/2023 07:22, Mingjin Ye:
> > > When hw is just started, it will immediately obtain the link status,
> > > and the longest attempt is 1 second. Some NICs are slow to
> > > initialize, which make it fails to obtain the link status.
> > >
> > > The patch fixes this issue by modifying the longest attempt to 5 seconds.
> >
> > What is the consequence?
> > In which case, DPDK application would be blocked during 5 seconds?
> >
> 
> I think ice_link_update with wait_to_complete needs to be optimized when
> ice_link_update.
> Our system integrates dozens or more interfaces(E810), even if wait 1
> second, the system starts very slowly.

Your proposal is very interesting. We will discuss this in our internal meeting.
Just verified some issues:
----------------------------------------------------------------------------------------------
ice_dev_start
	ice_dev_set_link_up
		ice_link_update(dev, 1)
e810 takes 700ms, e823 takes 1980ms.
-----------------------------------------------------------------------------------------------
ice_dev_start
	ice_dev_set_link_up
		sleep(5)
		ice_link_update(dev, 1)
 e810 and e823 need about 5~8ms.	
-----------------------------------------------------------------------------------------------
The execution timing of ice_link_update needs to be optimized. The root cause is
that the hw  needs time to complete initialization. This involves initializing the hw, 
which can be a lot of work. 
This is not the same issue that the current patch solves.

RE: [PATCH] net/ice: fix get link status timeout

[Yang, Qiming]

Hi, Minjin

> -----Original Message-----
> From: Ye, MingjinX <mingjinx.ye@intel.com>
> Sent: Monday, February 6, 2023 6:52 PM
> To: 韩爽 <hanshuang@qianxin.com>; Thomas Monjalon
> <thomas@monjalon.net>
> Cc: dev@dpdk.org; stable@dpdk.org; Zhou, YidingX
> <yidingx.zhou@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Zhang,
> Qi Z <qi.z.zhang@intel.com>; david.marchand@redhat.com;
> ferruh.yigit@amd.com
> Subject: RE: [PATCH] net/ice: fix get link status timeout
> 
> 
> 
> > -----Original Message-----
> > From: 韩爽 <hanshuang@qianxin.com>
> > Sent: 2023年2月6日 15:57
> > To: Thomas Monjalon <thomas@monjalon.net>; Ye, MingjinX
> > <mingjinx.ye@intel.com>
> > Cc: dev@dpdk.org; stable@dpdk.org; Zhou, YidingX
> > <yidingx.zhou@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Zhang,
> > Qi Z <qi.z.zhang@intel.com>; Ye, MingjinX <mingjinx.ye@intel.com>;
> > david.marchand@redhat.com; ferruh.yigit@amd.com
> > Subject: Re: [PATCH] net/ice: fix get link status timeout
> >
> >
> >
> > > -----Original Message-----
> > > From: Thomas Monjalon <thomas@monjalon.net>
> > > Sent: Monday, February 06, 2023 3:23 PM
> > > To: Mingjin Ye <mingjinx.ye@intel.com>
> > > Cc: dev@dpdk.org; stable@dpdk.org; yidingx.zhou@intel.com; Qiming
> > Yang
> > > <qiming.yang@intel.com>; Qi Zhang <qi.z.zhang@intel.com>; Mingjin Ye
> > > <mingjinx.ye@intel.com>; david.marchand@redhat.com;
> > > ferruh.yigit@amd.com
> > > Subject: Re: [PATCH] net/ice: fix get link status timeout
> > >
> > > 06/02/2023 07:22, Mingjin Ye:
> > > > When hw is just started, it will immediately obtain the link
> > > > status, and the longest attempt is 1 second. Some NICs are slow to
> > > > initialize, which make it fails to obtain the link status.
> > > >
> > > > The patch fixes this issue by modifying the longest attempt to 5 seconds.
> > >
> > > What is the consequence?
> > > In which case, DPDK application would be blocked during 5 seconds?
> > >
> >
> > I think ice_link_update with wait_to_complete needs to be optimized
> > when ice_link_update.
> > Our system integrates dozens or more interfaces(E810), even if wait 1
> > second, the system starts very slowly.
> 
> Your proposal is very interesting. We will discuss this in our internal meeting.
> Just verified some issues:
> ----------------------------------------------------------------------------------------------
> ice_dev_start
> 	ice_dev_set_link_up
> 		ice_link_update(dev, 1)
> e810 takes 700ms, e823 takes 1980ms.
> -----------------------------------------------------------------------------------------------
> ice_dev_start
> 	ice_dev_set_link_up
> 		sleep(5)
> 		ice_link_update(dev, 1)
>  e810 and e823 need about 5~8ms.
> -----------------------------------------------------------------------------------------------
> The execution timing of ice_link_update needs to be optimized. The root
> cause is that the hw  needs time to complete initialization. This involves
> initializing the hw, which can be a lot of work.
> This is not the same issue that the current patch solves.

I think it's a cycle, so even we set the max waiting time to 1s, it will run out when link status result get.
Will not cause system start slowly only when HW meet some errors and can't get link status.
But I also suggest you to use a reasonable time based on the test result.

Re: [PATCH] net/ice: fix get link status timeout

[Shuang Han]

> -----Original Message-----
> From: Mingjin Ye <mingjinx.ye@intel.com>
> Sent: Monday, February 06, 2023 2:23 PM
> To: dev@dpdk.org
> Cc: stable@dpdk.org; yidingx.zhou@intel.com; Mingjin Ye
> <mingjinx.ye@intel.com>; Qiming Yang <qiming.yang@intel.com>; Qi Zhang
> <qi.z.zhang@intel.com>
> Subject: [PATCH] net/ice: fix get link status timeout
>
>
> When hw is just started, it will immediately obtain the link status, and the
> longest attempt is 1 second. Some NICs are slow to initialize, which make it fails
> to obtain the link status.
>
> The patch fixes this issue by modifying the longest attempt to 5 seconds.
>
> Fixes: cf911d90e366 ("net/ice: support link update")
> Cc: stable@dpdk.org
>
> Signed-off-by: Mingjin Ye <mingjinx.ye@intel.com>
> ---
>  drivers/net/ice/ice_ethdev.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c index
> 0bc739daf0..eaa556f45c 100644
> --- a/drivers/net/ice/ice_ethdev.c
> +++ b/drivers/net/ice/ice_ethdev.c
> @@ -3927,7 +3927,7 @@ static int
>  ice_link_update(struct rte_eth_dev *dev, int wait_to_complete)  {  #define
> CHECK_INTERVAL 100  /* 100ms */ -#define MAX_REPEAT_TIME 10  /* 1s (10
> * 100ms) in total */
> +#define MAX_REPEAT_TIME 50  /* 5s (50 * 100ms) in total */
>  struct ice_hw *hw = ICE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
>  struct ice_link_status link_status;
>  struct rte_eth_link link, old;
> --
> 2.25.1

Sorry for my mail client setting with last reply.
I think ice_link_update with wait_to_complete needs to be optimized
when ice_link_update.
Our system integrates dozens or more interfaces(E810), even if wait 1
second, the system starts very slowly.