Helin, Thanks for comment. Any device that enabled LSC needs this fix, otherwise they all need to call link_update separately. We cannot assume that only Bond enables LSC interrupt.
The fix will not be used for all PMDs, as it explicitly checks if (dev->data->dev_conf.intr_conf.lsc != 0). Therefore, for hardware NIC (e.g. 82599) that disabled lsc by default, the link_update callback will not be executed. Please let me know if you have other concerns. Thanks, Jia On 2/3/15, 12:35 AM, "Zhang, Helin" <helin.zhang at intel.com> wrote: > > >> -----Original Message----- >> From: Jia Yu [mailto:jyu at vmware.com] >> Sent: Tuesday, February 3, 2015 4:00 PM >> To: Zhang, Helin >> Cc: dev at dpdk.org; Thomas Monjalon >> Subject: Re: [dpdk-dev] [PATCH 1/2] rte_ethdev: update link status >>(speed, >> duplex, link_up) after rte_eth_dev_start >> >> My answer to Helin?s comments: >> >> This patch is needed for bond slave devices or other devices, when LSC >> interrupt is enabled. >> 1. slave_configure() -> slave_eth_dev->?.lsc = 1 >> >> 2. rte_eth_link_get() reads dev_link from eth_dev, when lsc interrupt >>is >> enabled. However, the dev_link on eth_dev has not be initialized and >>showed >> link down state. This patch initializes the device?s dev_link at >> rte_eth_dev_start time. >So the link update is for bond only? But your code changes is in >rte_ethdev, it will >be used for all PMDs. For hardware NIC (e.g. 82599), this link update is >not needed >at all. It just need to wait the link status change event. So can those >code changes >be put in librte_pmd_bond, but not in librte_ether? > >Regards, >Helin > >> >> Please let me know if you have further questions/comments. >> >> Thanks, >> Jia >> >> On 1/30/15, 2:28 AM, "Thomas Monjalon" <thomas.monjalon at 6wind.com> >> wrote: >> >> >Jia, any news on this patchset? >> > >> >2014-11-12 03:57, Zhang, Helin: >> >> Hi Jia >> >> >> >> > -----Original Message----- >> >> > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Jia Yu >> >> > Sent: Saturday, November 8, 2014 1:32 AM >> >> > To: dev at dpdk.org >> >> > Subject: [dpdk-dev] [PATCH 1/2] rte_ethdev: update link status >> >>(speed, duplex, >> >> > link_up) after rte_eth_dev_start >> >> > >> >> > Since LSR interrupt is disabled by pmd drivers, link status in >> >>rte_eth_device is >> >> > always down. >> >> If LSC interrupt is disabled by default, it will poll the link status >> >>during the initialization or in dev_start, and then the link status >> >>should he correct. If I am not wrong. >> >> >> >> > Bond slave_configure() enables LSR interrupt on devices to get >> >>notification if link >> >> > status changes. However, the LSC interrupt at device start time is >> >>still lost. >> >> Before enabling interrupt for LSC, the link status should be polled. >> >>So after the port startup, the link status should be there. >> >> >> >> > >> >> > In this fix, call link_update to read link status from hardware >> >>register at device >> >> > start time. >> >> Could you help to explain this code changes a bit more? Why we need >>it? >> >> >> >> > >> >> > Issue: >> >> > Change-Id: Ib57a1c9114f922485c7b0f4338bfe7b3d3f87d65 >> >> > Signed-off-by: Jia Yu <jyu at vmware.com> >> >> > --- >> >> > lib/librte_ether/rte_ethdev.c | 4 ++++ >> >> > 1 file changed, 4 insertions(+) >> >> > >> >> > diff --git a/lib/librte_ether/rte_ethdev.c >> >>b/lib/librte_ether/rte_ethdev.c index >> >> > ff1c769..6c01b02 100644 >> >> > --- a/lib/librte_ether/rte_ethdev.c >> >> > +++ b/lib/librte_ether/rte_ethdev.c >> >> > @@ -869,6 +869,10 @@ rte_eth_dev_start(uint8_t port_id) >> >> > >> >> > rte_eth_dev_config_restore(port_id); >> >> > >> >> > + if (dev->data->dev_conf.intr_conf.lsc != 0) { >> >> > + FUNC_PTR_OR_ERR_RET(*dev->dev_ops->link_update, >> -ENOTSUP); >> >> > + (*dev->dev_ops->link_update)(dev, 0); >> >> > + } >> >> > return 0; >> >> > } >> >> > >> >> > -- >> >> > 1.9.1 >> >> >> >> Regards, >> >> Helin >> > >
