Re: [dpdk-dev] [RFC PATCH v2 1/5] librte_ether: add internal callback functions

[Jerin Jacob <jerin.jacob@caviumnetworks.com>]

On Fri, Aug 26, 2016 at 10:10:16AM +0100, Bernard Iremonger wrote:
> add _rte_eth_dev_callback_process_vf function.
> add _rte_eth_dev_callback_process_generic function
> 
> Adding a callback to the user application on VF to PF mailbox message,
> allows passing information to the application controlling the PF
> when a VF mailbox event message is received, such as VF reset.
> 
> Signed-off-by: azelezniak <alexz@att.com>
> Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
> ---
>  lib/librte_ether/rte_ethdev.c          | 17 ++++++++++
>  lib/librte_ether/rte_ethdev.h          | 61 ++++++++++++++++++++++++++++++++++
>  lib/librte_ether/rte_ether_version.map |  7 ++++
>  3 files changed, 85 insertions(+)
> 
> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> index f62a9ec..1388ea3 100644
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -2690,6 +2690,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);
> +}
> +
> +void
> +_rte_eth_dev_callback_process_generic(struct rte_eth_dev *dev,
> +	enum rte_eth_event_type event, void *param)
> +{
>  	struct rte_eth_dev_callback *cb_lst;
>  	struct rte_eth_dev_callback dev_cb;
>  
> @@ -2699,6 +2713,9 @@ _rte_eth_dev_callback_process(struct rte_eth_dev *dev,
>  			continue;
>  		dev_cb = *cb_lst;
>  		cb_lst->active = 1;
> +		if (param != NULL)
> +			dev_cb.cb_arg = (void *) param;
> +
>  		rte_spinlock_unlock(&rte_eth_dev_cb_lock);
>  		dev_cb.cb_fn(dev->data->port_id, dev_cb.event,
>  						dev_cb.cb_arg);
> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> index b0fe033..4fb0b9c 100644
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -3047,9 +3047,27 @@ 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 */
>  };
>  
> +/**
> + * Response sent back to ixgbe driver from user app after callback
> + */
> +enum rte_eth_mb_event_rsp {
> +	RTE_ETH_MB_EVENT_NOOP_ACK,  /**< skip mbox request and ACK */
> +	RTE_ETH_MB_EVENT_NOOP_NACK, /**< skip mbox request and NACK */
> +	RTE_ETH_MB_EVENT_PROCEED,  /**< proceed with mbox request  */
> +	RTE_ETH_MB_EVENT_MAX       /**< max value of this enum */
> +};

Do we really need to define the specifics of PF to VF MBOX communication
in normative ethdev specification?
Each drivers may have different PF to VF MBOX definitions so it may not be
very portable.
What is the use-case here? If its for VF configuration, I think we can
do it as separate 'sync' functions for each functionality so that
PMDs will have room hiding the specifics on MBOX definitions.