From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id 53A892BDC for ; Tue, 18 Oct 2016 18:22:33 +0200 (CEST) Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by orsmga102.jf.intel.com with ESMTP; 18 Oct 2016 09:22:31 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,362,1473145200"; d="scan'208";a="20849602" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.237.220.83]) ([10.237.220.83]) by fmsmga006.fm.intel.com with ESMTP; 18 Oct 2016 09:22:30 -0700 To: "Ananyev, Konstantin" , "Guo, Jia" , "Zhang, Helin" , "Wu, Jingjing" References: <1474887098-115474-1-git-send-email-jia.guo@intel.com> <1476582005-110811-1-git-send-email-jia.guo@intel.com> <2601191342CEEE43887BDE71AB9772583F0C1FF9@irsmsx105.ger.corp.intel.com> <2601191342CEEE43887BDE71AB9772583F0C23DD@irsmsx105.ger.corp.intel.com> Cc: "dev@dpdk.org" From: Ferruh Yigit Message-ID: Date: Tue, 18 Oct 2016 17:22:29 +0100 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <2601191342CEEE43887BDE71AB9772583F0C23DD@irsmsx105.ger.corp.intel.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v2 1/2] drivers/i40e: fix X722 macro absence result in compile 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: Tue, 18 Oct 2016 16:22:33 -0000 On 10/17/2016 10:54 AM, Ananyev, Konstantin wrote: > Hi Jeff, > >> >> hi, Konstantin >> Thanks your constructive suggestion. I don't think your question is >> silly and we also think about the code style simply and effective, but >> may be i would interpret the reason why we do that. >> >> 1) Sure, user definitely can choose to define the macro or not when >> building dpdk i40e PMD, but i don't think it is >> necessary to invoke a ret_config option to let up layer user freedom use >> it, because only the older version i40e driver does not support X722, >> the newer version i40e driver will always support X722, so the macro >> will be default hard code in the makefile. and we will use mac.type to >> distinguish the difference register configure in run time. So we may >> consider the macro just like a flag that highlight the difference of the >> shared code between X710 and X722, that would benify the X710/X722 pmd >> development but hardly no use to exposure to the up layer user. >> >> 2) i think the answer also could find from above. But i think if we >> develop go to a certain stage in the future, mute the macro or use >> script to remove them like the way from hw driver, for support all >> device types maybe not a bad idea, right? > > Sorry, but I still didn't get it. > If i40e driver will always support X722 then why do we need that macro at all? > Why just not to remove it completely then? > Same about run-time vs build-time choice: > If let say i40e_get_rss_key() has to behave in a different way, why not to create > i40e_get_rss_key_x722() and use it when hw mactype is x7222? > Or at least inside i40e_get_rss_key() do something like: > if (hw->mac.type == I40E_MAC_X722) {...} else {...} > ? > Why instead you have to pollute whole i40e code with all these #ifdef x7222/#else ...? > Obviously that looks pretty ugly and hard to maintain. It is not possible to remove "#ifdef x7222" from shared code, but what about removing it from DPDK piece of the code, and code as it is always defined? If this is OK, this patch is not more required. And the removing #ifdef work can be done in another patch later. > Konstantin