From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id BE4325942 for ; Thu, 28 Aug 2014 16:06:37 +0200 (CEST) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga102.fm.intel.com with ESMTP; 28 Aug 2014 07:10:28 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.04,418,1406617200"; d="scan'208";a="582902777" Received: from fmsmsx103.amr.corp.intel.com ([10.19.9.34]) by fmsmga001.fm.intel.com with ESMTP; 28 Aug 2014 07:08:10 -0700 Received: from fmsmsx157.amr.corp.intel.com (10.18.116.73) by FMSMSX103.amr.corp.intel.com (10.19.9.34) with Microsoft SMTP Server (TLS) id 14.3.195.1; Thu, 28 Aug 2014 07:07:21 -0700 Received: from shsmsx102.ccr.corp.intel.com (10.239.4.154) by FMSMSX157.amr.corp.intel.com (10.18.116.73) with Microsoft SMTP Server (TLS) id 14.3.195.1; Thu, 28 Aug 2014 07:07:19 -0700 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.17]) by shsmsx102.ccr.corp.intel.com ([169.254.2.246]) with mapi id 14.03.0195.001; Thu, 28 Aug 2014 22:07:18 +0800 From: "Wu, Jingjing" To: "Ananyev, Konstantin" , Thomas Monjalon Thread-Topic: [dpdk-dev] [PATCH v2 2/7] ethdev: define new ethdev API rx_classification_filter_ctl Thread-Index: AQHPwrYNUFtTGjRpdkeLRyRFF83fA5vmBwxA Date: Thu, 28 Aug 2014 14:07:17 +0000 Message-ID: <9BB6961774997848B5B42BEC655768F8ADC238@SHSMSX104.ccr.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> <2601191342CEEE43887BDE71AB9772582135F39F@IRSMSX105.ger.corp.intel.com> In-Reply-To: <2601191342CEEE43887BDE71AB9772582135F39F@IRSMSX105.ger.corp.intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] 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 14:06:38 -0000 Hi, Konstantin Thanks a lot. > -----Original Message----- > From: Ananyev, Konstantin > Sent: Thursday, August 28, 2014 7:49 PM > To: Thomas Monjalon; Wu, Jingjing > Cc: dev@dpdk.org > Subject: RE: [dpdk-dev] [PATCH v2 2/7] ethdev: define new ethdev API > rx_classification_filter_ctl >=20 >=20 >=20 > > -----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_classification_filter_ctl > > > > 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. > > > > The approach can be interesting. > > > > > 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. > > > > 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 i= t > > to i40e. > > > > > The API is like below: > > > typedef int (*eth_rx_classification_filter_ctl_t)(struct rte_eth_dev = *dev, > > > enum rte_eth_command cmd, > > > void *arg); > > > Define a head file called rte_i40e.h in lib/librte_pmd_i40e, which co= ntains > > > 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, whi= ch > > > contains the commands' definition linked to cmd parameters above. > > > > Why creating a rte_eth_features.h? Don't you think rte_ethdev.h is a go= od place? >=20 > As I remember the long term idea was > (Jingjing please correct me, if I am wrong here): >=20 > Keep rte_ethdev.h for generic API. > Put features specific for different NICs into rte_eth_features.h > To make things clearer and avoid polluting of rte_ethdev.h. >=20 > Provide API for the upper layer to query list of specific features(comman= ds) supported by the > underlying device. > Something like: > rte_eth_dev_get_rx_filter_supported(port, ...); Yes, in helin's patch "[dpdk-dev] [PATCH v2 2/6] ethdev: add new ops of 'is_command_supported' and 'rx_classification_filter_ctl'", he already defined rte_eth_dev_is_command_supported. It can be used to check whether such command can be supported by the queried port. Actually, some features are based on this architecture, including helin's "Support configuring hash functions" and other non-ready patch. We supposed after any patch of ours is applied, others need to rework then. We are using the same approach. > And ioctl-type API to configure HW specific features: > rte_eth_dev_rx_classification_filter_ctl(port, cmd, cmd_spedific_arg); >=20 > So, instead of application knows in advance "which specific NIC it is usi= ng", > application would query which features/commannds the NIC provides and the= n 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-= pmd. > > > And user can encode these structures and call XXX_ctl API to configur= e > > > their features. > > > > 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? > > > > Example: > > > > 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); > > > > struct i40e_fdir_entry { > > struct fdir_entry generic; > > i40e_fdir_input i40e_input; > > }; > > > > So i40e_input will be handled by the PMD if driver =3D=3D RTE_DRIVER_I4= 0E. > > > > It's just an idea, comments are welcome. > > > > -- > > Thomas