[dpdk-dev] [PATCH v6 1/4] lib/librte_ether: support device reset

[Lu, Wenzhuo]

Hi Jerin,


> -----Original Message-----
> From: Jerin Jacob [mailto:jerin.jacob at caviumnetworks.com]
> Sent: Wednesday, June 22, 2016 2:10 PM
> To: Lu, Wenzhuo
> Cc: Ananyev, Konstantin; Stephen Hemminger; dev at dpdk.org; Richardson,
> Bruce; Chen, Jing D; Liang, Cunming; Wu, Jingjing; Zhang, Helin;
> thomas.monjalon at 6wind.com
> Subject: Re: [dpdk-dev] [PATCH v6 1/4] lib/librte_ether: support device reset
> 
> On Wed, Jun 22, 2016 at 05:05:14AM +0000, Lu, Wenzhuo wrote:
> >
> >
> > > -----Original Message-----
> > > From: Jerin Jacob [mailto:jerin.jacob at caviumnetworks.com]
> > > Sent: Wednesday, June 22, 2016 12:15 PM
> > > To: Lu, Wenzhuo
> > > Cc: Ananyev, Konstantin; Stephen Hemminger; dev at dpdk.org;
> > > Richardson, Bruce; Chen, Jing D; Liang, Cunming; Wu, Jingjing;
> > > Zhang, Helin; thomas.monjalon at 6wind.com
> > > Subject: Re: [dpdk-dev] [PATCH v6 1/4] lib/librte_ether: support
> > > device reset
> > >
> > > On Wed, Jun 22, 2016 at 03:32:16AM +0000, Lu, Wenzhuo wrote:
> > > > Lost here. I think these RTE_ETH_EVENTs are used to connect the
> > > > APP call
> > > back functions with the events.
> > > > Actually I want the APP to register a callback function
> > > > reset_event_callback for
> > > the reset event. Like this,
> > > > 		/* register reset interrupt callback */
> > > > 		rte_eth_dev_callback_register(portid,
> > > > 			RTE_ETH_EVENT_INTR_RESET, reset_event_callback,
> > > NULL); And when the
> > > > VF driver finds PF link down/up, it  should  use
> > > _rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_RESET) to run
> > > into the callback which is provided by APP. Means reset_event_callback here.
> > >
> > > me too. Their is existing RTE_ETH_EVENT_INTR_RESET event to notify
> > > the PF reset.I guess it is not for the PF link change or it isfor
> > > generic VF reset request initiated by PF for everything.
> > I think this event is for device reset not only for PF but also can for VF. I think
> we can use this event when the driver want the APP to reset the device. The PF
> link down/up caused VF reset is one of the cases.
> 
> Then please correct description for the RTE_ETH_EVENT_INTR_RESET in
> lib/librte_ether/rte_ethdev.h "/**< reset interrupt event, sent to VF on PF reset
> */"
> 
> >
> > >
> > > file: lib/librte_ether/rte_ethdev.h
> > >         RTE_ETH_EVENT_INTR_RESET,
> > > 		/**< reset interrupt event, sent to VF on PF reset */
> > >
> > > ^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > >
> > > if application need to call rte_ethdev_reset() on
> > > RTE_ETH_EVENT_INTR_RESET event then please mention it commit log or
> API description.
> > Good suggestion. I'll try to find where's the good place to add more
> explanation.
> 
> I guess then reset API can be changed to rte_ethdev_process_reset_intr() or
> similar to reflect the use case(API called by application on reset event from PF)
> 
> The PMDs were PF does not generate the RTE_ETH_EVENT_INTR_RESET to VF
> then VF's reset PMD callback shall be a 'nop'
> 
> Jerin
But I don't think it's appropriate to bind the RTE_ETH_EVENTs with the APIs. This patch set provide a reset API to reset the device. Don't mean this reset API only can be used when the APP hit the event RTE_ETH_EVENT_INTR_RESET. I can add some comments to suggest the user to call the reset API at that time. But I think APP can call the reset API anytime when it thinks it's necessary. So I don't like the name *process_reset_intr*, it hints that this API is only for the INTR_RESET event.

> 
> > >
> >