Hi Williams: > -----Original Message----- > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Chas Williams > Sent: Saturday, March 31, 2018 1:22 AM > To: Zhang, Helin <helin.zh...@intel.com> > Cc: dev@dpdk.org; Lu, Wenzhuo <wenzhuo...@intel.com>; Ananyev, > Konstantin <konstantin.anan...@intel.com>; Charles (Chas) Williams > <ch...@att.com> > Subject: Re: [dpdk-dev] [PATCH] net/ixgbe: update data->eth_link status on > start > > On Fri, Mar 30, 2018 at 12:30 PM, Zhang, Helin <helin.zh...@intel.com> > wrote: > > > > > >> -----Original Message----- > >> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Chas Williams > >> Sent: Wednesday, February 14, 2018 6:56 AM > >> To: dev@dpdk.org > >> Cc: Lu, Wenzhuo; Ananyev, Konstantin; Charles (Chas) Williams > >> Subject: [dpdk-dev] [PATCH] net/ixgbe: update data->eth_link status > >> on start > >> > >> From: "Charles (Chas) Williams" <ch...@att.com> > >> > >> dev->data->eth_link isn't updated until the first interrupt. If a > >> link is carrier down, then this interrupt may never happen. Before > >> we finished starting the PMD, call ixgbe_dev_link_update() to ensure we > have a valid status. > >> > >> Signed-off-by: Chas Williams <ch...@att.com> > >> --- > >> drivers/net/ixgbe/ixgbe_ethdev.c | 4 ++++ > >> 1 file changed, 4 insertions(+) > >> > >> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c > >> b/drivers/net/ixgbe/ixgbe_ethdev.c > >> index 37eb668..27d29dc 100644 > >> --- a/drivers/net/ixgbe/ixgbe_ethdev.c > >> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c > >> @@ -2666,6 +2666,8 @@ ixgbe_dev_start(struct rte_eth_dev *dev) > >> if (err) > >> goto error; > >> > >> + ixgbe_dev_link_update(dev, 0); > > It is called in rte_eth_dev_start() if lsc is not enabled, and it is > > not needed here to avoid any duplication. > > BTW, did you see any issue or just check the code? Thanks! > > Yes, I see an issue with bonding. If LSC is enabled, then link_status isn't > set > until the first interrupt to update the link status. If a link is down, this > interrupt may never happen result in link_status being somewhat undefined.
Is your issue only happened on VF? For PF, I saw ixgbe_check_link is called at ixgbe_dev_start, so link status is assume be updated. If you think it is just missed in VF, can you implemented this in the similar way as PF? Regards Qi > > > > > /Helin > > > >> + > >> skip_link_setup: > >> > >> if (rte_intr_allow_others(intr_handle)) { @@ -5033,6 +5035,8 @@ > >> ixgbevf_dev_start(struct rte_eth_dev *dev) > >> > >> ixgbevf_dev_rxtx_start(dev); > >> > >> + ixgbevf_dev_link_update(dev, 0); > >> + > >> /* check and configure queue intr-vector mapping */ > >> if (rte_intr_cap_multiple(intr_handle) && > >> dev->data->dev_conf.intr_conf.rxq) { > >> -- > >> 2.9.5 > >