Hi Jia > -----Original Message----- > From: Jia Yu [mailto:jyu at vmware.com] > Sent: Wednesday, February 4, 2015 2:53 AM > 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 > > 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. I understood. I mean if (lsc == 1), for 82599, it can report lsc event correctly after startup. But if a manually link update is executed, no link change can be found when a lsc event is received in application. Your fix is to solve the problem in bond only, am I correct? My comment is that can we avoid doing that for 82599, Fortville, etc? As some of DPDK customers may not want it if lsc is enabled.
Regards, Helin > > 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 > >> > > >
