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 B2579B372 for ; Thu, 28 Aug 2014 10:57:23 +0200 (CEST) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga101.jf.intel.com with ESMTP; 28 Aug 2014 02:01:30 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.04,416,1406617200"; d="scan'208";a="594448886" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by orsmga002.jf.intel.com with ESMTP; 28 Aug 2014 02:01:30 -0700 Received: from fmsmsx119.amr.corp.intel.com (10.19.9.28) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.195.1; Thu, 28 Aug 2014 02:01:29 -0700 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by FMSMSX119.amr.corp.intel.com (10.19.9.28) with Microsoft SMTP Server (TLS) id 14.3.195.1; Thu, 28 Aug 2014 02:01:29 -0700 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.17]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.198]) with mapi id 14.03.0195.001; Thu, 28 Aug 2014 17:01:27 +0800 From: "Wu, Jingjing" To: Thomas Monjalon Thread-Topic: [dpdk-dev] [PATCH v2 7/7]app/testpmd: add commands and config functions for i40e flow director support Thread-Index: AQHPwZyuUuNR6DPrqUGFqFfP1yAUpJvj/v+AgAFhSpD//9CngIAAh5tQ Date: Thu, 28 Aug 2014 09:01:26 +0000 Message-ID: <9BB6961774997848B5B42BEC655768F8ADC10D@SHSMSX104.ccr.corp.intel.com> References: <1409105634-29980-1-git-send-email-jingjing.wu@intel.com> <8438692.KHYKcsiDRz@xps13> <9BB6961774997848B5B42BEC655768F8ADBF4E@SHSMSX104.ccr.corp.intel.com> <6406194.6E43HquPHH@xps13> In-Reply-To: <6406194.6E43HquPHH@xps13> 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 7/7]app/testpmd: add commands and config functions for i40e flow director support 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 08:57:24 -0000 Hi, Thomas Thanks for your tips. I have another question: If we use the way 'rx_classification_filter_ctl' works, the specific struct= ures defined in rte_i40e.h will be visible in user's application, such as t= estpmd. I know I shouldn't make commands linked with i40e like what I did before. B= ut will the i40e specific structures become visible be acceptable? > -----Original Message----- > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > Sent: Thursday, August 28, 2014 4:51 PM > To: Wu, Jingjing > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v2 7/7]app/testpmd: add commands and confi= g functions for > i40e flow director support >=20 > 2014-08-28 03:51, Wu, Jingjing: > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > > > 2014-08-27 10:13, Jingjing Wu: > > > > add structure definition to construct programming packet. > > > > > > What is a "programming packet"? > > > > For Fortville, we need to set a flow director filter by sending a > > packet which contains the input set values through the queue > > belonging to flow director. >=20 > OK. To be more clear, some detailed explanations are required in the > commit log. Please try to be very descriptive. > You can think comments like this: > - if comments are absolutely needed to understand the code, you should > put comments in the code (or write the code differently) > - if some feature context can help for the review, you should explain > context and design in the commit log >=20 > > > > +#ifdef RTE_LIBRTE_I40E_PMD > > > > + "i40e_flow_director_filter (port_id) (add|del)" > > > > + " flow (ip4|ip6) src (src_ip_address) dst (dst_ip_address)" > > > > + " flexwords (flexwords_value) (drop|fwd)" > > > > + " queue (queue_id) fd_id (fd_id_value)\n" > > > > + " Add/Del a IP type flow director filter for i40e NIC.\n\n" > > > > + > > > > + "i40e_flow_director_filter (port_id) (add|del)" > > > > + " flow (udp4|tcp4|udp6|tcp6)" > > > > + " src (src_ip_address) (src_port)" > > > > + " dst (dst_ip_address) (dst_port)" > > > > + " flexwords (flexwords_value) (drop|fwd)" > > > > + " queue (queue_id) fd_id (fd_id_value)\n" > > > > + " Add/Del a UDP/TCP type flow director filter for i40e NIC.\= n\n" > > > > + > > > > + "i40e_flush_flow_diretor (port_id)\n" > > > > + " Flush all flow director entries of a device on i40e NIC.\n= \n" > > > > +#endif /* RTE_LIBRTE_I40E_PMD */ > > > > > > I'd really like to stop seeing this kind of thing. > > > We cannot add some ifdef for each PMD in generic code. > > > > > > I stopped reading after that. > > > > > > Sorry, I don't want to be rude but my feeling is that adding such fea= ture > > > with global picture in mind is not easy. I know you want to offer all= i40e > > > capabilities but you should think at future evolutions and how other = drivers > > > will be integrated with yours. > > > > > > > Sorry to make you feel uncomfortable for such code. Just as you say, > > I want to offer more i40e capabilities. I will rework code in testpmd. >=20 > Thanks > -- > Thomas