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 19:19:40 +0200 Message-ID: <3456767.oRusgI458o@xps13> References: <1475250308-5498-1-git-send-email-bernard.iremonger@intel.com> <2148835.WQXRyfBhcc@xps13> <8CEF83825BEC744B83065625E567D7C21A08F52C@IRSMSX108.ger.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7Bit Cc: dev@dpdk.org, "Shah, Rahul R" , "Lu, Wenzhuo" , "az5157@att.com" , "jerin.jacob@caviumnetworks.com" To: "Iremonger, Bernard" Return-path: Received: from mail-wm0-f52.google.com (mail-wm0-f52.google.com [74.125.82.52]) by dpdk.org (Postfix) with ESMTP id 078E02A6C for ; Wed, 5 Oct 2016 19:19:44 +0200 (CEST) Received: by mail-wm0-f52.google.com with SMTP id p138so286097704wmb.1 for ; Wed, 05 Oct 2016 10:19:44 -0700 (PDT) In-Reply-To: <8CEF83825BEC744B83065625E567D7C21A08F52C@IRSMSX108.ger.corp.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-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.