From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Monjalon Subject: Re: [PATCH v4 1/2] librte_ether: add internal callback functions Date: Wed, 05 Oct 2016 18:10:29 +0200 Message-ID: <2148835.WQXRyfBhcc@xps13> References: <1475250308-5498-1-git-send-email-bernard.iremonger@intel.com> <1475592734-22355-2-git-send-email-bernard.iremonger@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7Bit Cc: dev@dpdk.org, rahul.r.shah@intel.com, wenzhuo.lu@intel.com, az5157@att.com, jerin.jacob@caviumnetworks.com To: Bernard Iremonger Return-path: Received: from mail-lf0-f50.google.com (mail-lf0-f50.google.com [209.85.215.50]) by dpdk.org (Postfix) with ESMTP id 5BA522BFF for ; Wed, 5 Oct 2016 18:10:33 +0200 (CEST) Received: by mail-lf0-f50.google.com with SMTP id b75so85022754lfg.3 for ; Wed, 05 Oct 2016 09:10:33 -0700 (PDT) In-Reply-To: <1475592734-22355-2-git-send-email-bernard.iremonger@intel.com> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 2016-10-04 15:52, Bernard Iremonger: > 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. I have some difficulties to parse this explanation. Please could you reword it and precise the direction of the message and the use case context? > --- 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? And by the way, why not just replacing the legacy function? As it is a driver interface, there is no ABI restriction. > + > +void > +_rte_eth_dev_callback_process_generic(struct rte_eth_dev *dev, > + enum rte_eth_event_type event, void *param) > +{ [...] > --- 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. > +/** > + * @internal Executes all the user application registered callbacks. Used by: > + * _rte_eth_dev_callback_process and _rte_eth_dev_callback_process_vf > + * It is for DPDK internal user only. User application should not call it > + * directly. > + * > + * @param dev > + * Pointer to struct rte_eth_dev. > + * @param event > + * Eth device interrupt event type. > + * > + * @param param > + * parameters to pass back to user application. > + * > + * @return > + * void > + */ > +void > +_rte_eth_dev_callback_process_generic(struct rte_eth_dev *dev, > + enum rte_eth_event_type event, void *param); This is really an internal function and should not be exported at all.