From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <venky.venkatesan@intel.com>
Received: from mga14.intel.com (mga14.intel.com [192.55.52.115])
 by dpdk.org (Postfix) with ESMTP id 7B65AB361
 for <dev@dpdk.org>; Wed, 27 Aug 2014 18:50:50 +0200 (CEST)
Received: from fmsmga001.fm.intel.com ([10.253.24.23])
 by fmsmga103.fm.intel.com with ESMTP; 27 Aug 2014 09:46:52 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.04,412,1406617200"; d="scan'208";a="582460994"
Received: from nvenkate-mobl1.amr.corp.intel.com (HELO [10.24.25.27])
 ([10.24.25.27])
 by fmsmga001.fm.intel.com with ESMTP; 27 Aug 2014 09:54:52 -0700
Message-ID: <53FE0D5C.8070102@intel.com>
Date: Wed, 27 Aug 2014 09:54:52 -0700
From: "Venkatesan, Venky" <venky.venkatesan@intel.com>
User-Agent: Mozilla/5.0 (Windows NT 6.2; WOW64;
 rv:24.0) Gecko/20100101 Thunderbird/24.6.0
MIME-Version: 1.0
To: dev@dpdk.org
References: <1409105634-29980-1-git-send-email-jingjing.wu@intel.com>
 <1409105634-29980-8-git-send-email-jingjing.wu@intel.com>
 <8438692.KHYKcsiDRz@xps13>
In-Reply-To: <8438692.KHYKcsiDRz@xps13>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
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 <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Wed, 27 Aug 2014 16:50:51 -0000


On 8/27/2014 7:35 AM, Thomas Monjalon wrote:
> Hi Jingjing,
>
> 2014-08-27 10:13, Jingjing Wu:
>> add structure definition to construct programming packet.
> What is a "programming packet"?
>
>> +#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 feature
> 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.
>
> Thanks
Second that. Any PMD-specifics need to be contained within the PMD 
itself, and not in generic code. Please rework this.
-Venky