Re: [dpdk-stable] [dpdk-dev] [PATCH v4] net/ixgbe: fix link status

["Cui, LunyuanX" <lunyuanx.cui@intel.com>]

Hi, xiaolong

> -----Original Message-----
> From: Ye, Xiaolong
> Sent: Tuesday, November 19, 2019 2:28 PM
> To: Cui, LunyuanX <lunyuanx.cui@intel.com>
> Cc: dev@dpdk.org; Lu, Wenzhuo <wenzhuo.lu@intel.com>;
> stable@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v4] net/ixgbe: fix link status
> 
> On 11/18, Lunyuan Cui wrote:
> >The link status for 82599eb got from link status register was not
> >correct. Check the enable/disable flag of tx laser, set the link status
> >down if tx laser disabled. Then, we can get correct status.
> >But after port reset, tx laser register will be reset enable.
> >Link status will always be up. So set tx laser disable when port resets.
> 
> So you call ixgbe_dev_set_link_down to disable tx laser, but
> ixgbe_dev_set_link_down is more than just disable tx laser, will it has some
> side effects?

There are not side effects.
I call ixgbe_dev_set_link_down when port init.
When port starts, It will deal with follows:
	if (hw->mac.ops.get_media_type(hw) == ixgbe_media_type_copper) {
		/* Turn on the copper */
		ixgbe_set_phy_power(hw, true);
	} else {
		/* Turn on the laser */
		ixgbe_enable_tx_laser(hw);
	}

So I think there are not side effects.

> 
> >
> >When hw->mac.autotry_restart is true, whether tx laser is disable or
> >enable, it will be set enable in ixgbe_flap_tx_laser_multispeed_fiber().
> >hw->mac.autotry_restart can be set true in both port init and port start.
> >Because we don't need this treatment before port starts, set
> >hw->mac.autotry_restart false when port init.
> >
> >Fixes: 0408f47ba4d6 ("net/ixgbe: fix busy polling while fiber link
> >update")
> >Cc: stable@dpdk.org
> >
> >Signed-off-by: Lunyuan Cui <lunyuanx.cui@intel.com>
> >---
> >v4:
> >* modifier commit log
> >	Describe the problem in more detail.
> >
> >v3:
> >* Correct countermeasure
> >	Don't delete ixgbe_dev_setup_link_alarm_handler().
> >
> >v2:
> >* modifier commit log
> >	Add a log why I delete ixgbe_dev_setup_link_alarm_handler().
> >---
> > drivers/net/ixgbe/ixgbe_ethdev.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> >diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> >b/drivers/net/ixgbe/ixgbe_ethdev.c
> >index 8c1caac18..260484fbf 100644
> >--- a/drivers/net/ixgbe/ixgbe_ethdev.c
> >+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> >@@ -1188,6 +1188,7 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev,
> void *init_params __rte_unused)
> > 	diag = ixgbe_bypass_init_hw(hw);
> > #else
> > 	diag = ixgbe_init_hw(hw);
> >+	hw->mac.autotry_restart = false;
> > #endif /* RTE_LIBRTE_IXGBE_BYPASS */
> >
> > 	/*
> >@@ -1298,6 +1299,8 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev,
> void *init_params __rte_unused)
> > 	/* enable support intr */
> > 	ixgbe_enable_intr(eth_dev);
> >
> >+	ixgbe_dev_set_link_down(eth_dev);
> >+
> > 	/* initialize filter info */
> > 	memset(filter_info, 0,
> > 	       sizeof(struct ixgbe_filter_info));
> >--
> >2.17.1
> >