From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 54382B37B for ; Thu, 28 Aug 2014 13:45:12 +0200 (CEST) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga101.jf.intel.com with ESMTP; 28 Aug 2014 04:48:56 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.04,417,1406617200"; d="scan'208";a="564710453" Received: from irsmsx102.ger.corp.intel.com ([163.33.3.155]) by orsmga001.jf.intel.com with ESMTP; 28 Aug 2014 04:48:55 -0700 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.158]) by IRSMSX102.ger.corp.intel.com ([169.254.2.24]) with mapi id 14.03.0195.001; Thu, 28 Aug 2014 12:48:54 +0100 From: "Ananyev, Konstantin" To: Thomas Monjalon , "Wu, Jingjing" Thread-Topic: [dpdk-dev] [PATCH v2 2/7] ethdev: define new ethdev API rx_classification_filter_ctl Thread-Index: AQHPwq7XQPBC9CDfQk+5QEFc6cZ+yZvl4rzA Date: Thu, 28 Aug 2014 11:48:53 +0000 Message-ID: <2601191342CEEE43887BDE71AB9772582135F39F@IRSMSX105.ger.corp.intel.com> References: <1409105634-29980-1-git-send-email-jingjing.wu@intel.com> <3024593.z48vgEy6Ts@xps13> <9BB6961774997848B5B42BEC655768F8ADBEF0@SHSMSX104.ccr.corp.intel.com> <1793573.SnjKVZ6loZ@xps13> In-Reply-To: <1793573.SnjKVZ6loZ@xps13> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.239.180] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH v2 2/7] ethdev: define new ethdev API rx_classification_filter_ctl 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: Thu, 28 Aug 2014 11:45:12 -0000 > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon > Sent: Thursday, August 28, 2014 11:56 AM > To: Wu, Jingjing > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v2 2/7] ethdev: define new ethdev API rx_c= lassification_filter_ctl >=20 > 2014-08-28 03:30, Wu, Jingjing: > > We want to implement several common API for NIC specific features, > > to avoid creating quite a lot of ops in 'struct eth_dev_ops'. > > The idea came from ioctl. >=20 > The approach can be interesting. >=20 > > Because about flow director feature, there is large gap between i40e > > and ixgbe. The existed flow director APIs looks specific to IXGBE, > > so I choose this new API to implement i40e's flow director feature. >=20 > The API is not OK for you so you create another one. > I'm OK to change APIs but you should remove the old one, or at least, > implement your new API in existing drivers to allow deprecation of the > old API. > I think it would help if you start by doing ixgbe work and then apply it > to i40e. >=20 > > The API is like below: > > typedef int (*eth_rx_classification_filter_ctl_t)(struct rte_eth_dev *d= ev, > > enum rte_eth_command cmd, > > void *arg); > > Define a head file called rte_i40e.h in lib/librte_pmd_i40e, which cont= ains > > the definition about structures specific to i40e, linked to the arg > > parameters above. > > Define a head file called rte_eth_features.h in lib/librte_ether, which > > contains the commands' definition linked to cmd parameters above. >=20 > Why creating a rte_eth_features.h? Don't you think rte_ethdev.h is a good= place? As I remember the long term idea was (Jingjing please correct me, if I am wrong here): Keep rte_ethdev.h for generic API. Put features specific for different NICs into rte_eth_features.h=20 To make things clearer and avoid polluting of rte_ethdev.h. Provide API for the upper layer to query list of specific features(commands= ) supported by the underlying device. Something like:=20 rte_eth_dev_get_rx_filter_supported(port, ...); And ioctl-type API to configure HW specific features:=20 rte_eth_dev_rx_classification_filter_ctl(port, cmd, cmd_spedific_arg); So, instead of application knows in advance "which specific NIC it is using= ", application would query which features/commannds the NIC provides and then = configure them appropriately. >=20 > > And if user want to use i40e specific features, then the head file > > rte_i40e.h need to be included user's application, for example, test-pm= d. > > And user can encode these structures and call XXX_ctl API to configure > > their features. >=20 > OK, but the question is to know what is a specific feature? > I don't think flow director is a specific feature. We shouldn't have > to care if port is i40e or ixgbe to setup flow director. > Is it possible to have a common API and maybe an inheritance of the > common structure with PMD specific fields? >=20 > Example: >=20 > struct fdir_entry { > fdir_input input; > fdir_action action; > enum rte_driver driver; > }; > fdir_entry_generic fdir_entry =3D {.driver =3D RTE_DRIVER_GENERIC}; > filter_ctl(port, FDIR_RULE_ADD, fdir_entry); >=20 > struct i40e_fdir_entry { > struct fdir_entry generic; > i40e_fdir_input i40e_input; > }; >=20 > So i40e_input will be handled by the PMD if driver =3D=3D RTE_DRIVER_I40E= . >=20 > It's just an idea, comments are welcome. >=20 > -- > Thomas