Re: [dpdk-dev] [PATCH v4 1/2] librte_ether: add internal callback functions

[Thomas Monjalon <thomas.monjalon@6wind.com>]

2016-10-05 17:04, Iremonger, Bernard:
> > > --- a/lib/librte_ether/rte_ethdev.c
> > > +++ b/lib/librte_ether/rte_ethdev.c
> > > @@ -2510,6 +2510,20 @@ void
> > >  _rte_eth_dev_callback_process(struct rte_eth_dev *dev,
> > >  	enum rte_eth_event_type event)
> > >  {
> > > +	return _rte_eth_dev_callback_process_generic(dev, event, NULL); }
> > > +
> > > +void
> > > +_rte_eth_dev_callback_process_vf(struct rte_eth_dev *dev,
> > > +	enum rte_eth_event_type event, void *param) {
> > > +	return _rte_eth_dev_callback_process_generic(dev, event, param);
> > }
> > 
> > This function is just adding a parameter, compared to the legacy
> > _rte_eth_dev_callback_process.
> > Why calling it process_vf?
> 
> The parameter is just being added for the VF event, the handling of the other events is unchanged.
> 
> > And by the way, why not just replacing the legacy function?
> > As it is a driver interface, there is no ABI restriction.
> 
> I thought there would be an ABI issue if the legacy function is replaced.
> The _rte_eth_dev_callback_process is exported in DPDK 2.2 and used in the following PMD's, lib and app:
> 
> app/test/virtual_pmd
> drivers/net/e1000
> drivers/net/ixgbe
> drivers/net/mlx5
> drivers/net/vhost
> drivers/net/virtio
> lib/librte_ether
> 
>  Adding a parameter to _rte_eth_dev_callback_process()  will impact all of the above.
> Will this cause an ABI issue?

No because ABI is for applications (Application Binary Interface).
Here you are just changing the driver interface. And we have no commitment
to maintain the compatibility of this interface for external drivers.

> > > --- a/lib/librte_ether/rte_ethdev.h
> > > +++ b/lib/librte_ether/rte_ethdev.h
> > > @@ -3026,6 +3026,7 @@ enum rte_eth_event_type {
> > >  				/**< queue state event (enabled/disabled)
> > */
> > >  	RTE_ETH_EVENT_INTR_RESET,
> > >  			/**< reset interrupt event, sent to VF on PF reset */
> > > +	RTE_ETH_EVENT_VF_MBOX,  /**< PF mailbox processing callback */
> > >  	RTE_ETH_EVENT_MAX       /**< max value of this enum */
> > >  };
> > 
> > Either we choose to have a "generic" VF event well documented, or it is just
> > a specific event with a tip on where to find the doc.
> > Here we need at least to know how to handle the argument.
> 
> It is a specific event for VF to PF messages, details on the function and arguments are in the rte_ethdev.h file.

No I think it is only explained in the ixgbe code.