From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by dpdk.org (Postfix) with ESMTP id A356F2904 for ; Wed, 5 Oct 2016 19:07:03 +0200 (CEST) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga105.jf.intel.com with ESMTP; 05 Oct 2016 10:07:02 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,449,1473145200"; d="scan'208";a="176707839" Received: from irsmsx106.ger.corp.intel.com ([163.33.3.31]) by fmsmga004.fm.intel.com with ESMTP; 05 Oct 2016 10:06:58 -0700 Received: from irsmsx156.ger.corp.intel.com (10.108.20.68) by IRSMSX106.ger.corp.intel.com (163.33.3.31) with Microsoft SMTP Server (TLS) id 14.3.248.2; Wed, 5 Oct 2016 18:04:49 +0100 Received: from irsmsx108.ger.corp.intel.com ([169.254.11.164]) by IRSMSX156.ger.corp.intel.com ([169.254.3.80]) with mapi id 14.03.0248.002; Wed, 5 Oct 2016 18:04:49 +0100 From: "Iremonger, Bernard" To: Thomas Monjalon CC: "dev@dpdk.org" , "Shah, Rahul R" , "Lu, Wenzhuo" , "az5157@att.com" , "jerin.jacob@caviumnetworks.com" Thread-Topic: [dpdk-dev] [PATCH v4 1/2] librte_ether: add internal callback functions Thread-Index: AQHSHk7ubfKzVk/Tr0KaDnv9gWU0+6CZ+JmAgAAUhAA= Date: Wed, 5 Oct 2016 17:04:49 +0000 Message-ID: <8CEF83825BEC744B83065625E567D7C21A08F52C@IRSMSX108.ger.corp.intel.com> References: <1475250308-5498-1-git-send-email-bernard.iremonger@intel.com> <1475592734-22355-2-git-send-email-bernard.iremonger@intel.com> <2148835.WQXRyfBhcc@xps13> In-Reply-To: <2148835.WQXRyfBhcc@xps13> Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiN2RlMmRkNmItMjdiNy00MTkxLWE4ZjAtMjVjMGRjMGE1N2NkIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6IkdlVjQxcFk3YzRGMllYcmJYQ002UzF1UEVVZXllYkkzOXlTVENnOEpwVE09In0= x-ctpclassification: CTP_IC x-originating-ip: [163.33.239.181] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v4 1/2] librte_ether: add internal callback functions X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 05 Oct 2016 17:07:04 -0000 Hi Thomas, > Subject: Re: [dpdk-dev] [PATCH v4 1/2] librte_ether: add internal callbac= k > functions >=20 > 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. >=20 > I have some difficulties to parse this explanation. > Please could you reword it and precise the direction of the message and t= he > use case context? I will reword the explanation and add use case context. =20 > > --- 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); > } >=20 > 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 oth= er 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 f= ollowing 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? > > + > > +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 */ > > }; >=20 > Either we choose to have a "generic" VF event well documented, or it is j= ust > 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 a= rguments are in the rte_ethdev.h file. =20 > > +/** > > + * @internal Executes all the user application registered callbacks. U= sed > 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); >=20 > This is really an internal function and should not be exported at all. Both new functions are internal I will make them static and remove them fr= om the map file. When the functions are made static, should the function declarations be mov= ed from rte_ethdev.h to rte_ethdev.c ? Thanks for the review. Regards, Bernard.