From: Ferruh Yigit <ferruh.yigit@intel.com>
To: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>,
"Guo, Jia" <jia.guo@intel.com>,
"Zhang, Helin" <helin.zhang@intel.com>,
"Wu, Jingjing" <jingjing.wu@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v2 1/2] drivers/i40e: fix X722 macro absence result in compile
Date: Tue, 18 Oct 2016 17:22:29 +0100 [thread overview]
Message-ID: <e857da16-0149-c612-bca7-f668ce7c81ac@intel.com> (raw)
In-Reply-To: <2601191342CEEE43887BDE71AB9772583F0C23DD@irsmsx105.ger.corp.intel.com>
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
next prev parent reply other threads:[~2016-10-18 16:22 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-26 10:51 [dpdk-dev] [PATCH] drivers/i40e: fix the hash filter invalid calculation in X722 Jeff Guo
2016-09-29 6:29 ` Wu, Jingjing
2016-09-29 18:15 ` Ferruh Yigit
2016-09-30 6:05 ` Wu, Jingjing
2016-09-30 9:09 ` Ferruh Yigit
2016-10-16 1:40 ` [dpdk-dev] [PATCH v2 1/2] drivers/i40e: fix X722 macro absence result in compile Jeff Guo
2016-10-16 1:40 ` [dpdk-dev] [PATCH v2 2/2] drivers/i40e: fix the hash filter invalid calculation in X722 Jeff Guo
2016-10-18 16:25 ` Ferruh Yigit
2016-10-20 2:48 ` [dpdk-dev] [PATCH] net/i40e: " Jeff Guo
2016-10-24 9:10 ` Wu, Jingjing
2016-10-25 2:11 ` Guo, Jia
2016-10-25 2:26 ` [dpdk-dev] [PATCH v4] " Jeff Guo
2016-10-25 2:42 ` [dpdk-dev] [PATCH v4] net/i40e: fix hash filter invalid issue " Jeff Guo
2016-10-25 10:22 ` Wu, Jingjing
2016-10-25 12:29 ` Bruce Richardson
2016-10-16 13:31 ` [dpdk-dev] [PATCH v2 1/2] drivers/i40e: fix X722 macro absence result in compile Ananyev, Konstantin
2016-10-17 7:44 ` Guo, Jia
2016-10-17 9:54 ` Ananyev, Konstantin
2016-10-17 10:14 ` Chilikin, Andrey
2016-10-18 16:22 ` Ferruh Yigit [this message]
2016-10-19 6:10 ` Guo, Jia
2016-10-16 1:32 Jeff Guo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e857da16-0149-c612-bca7-f668ce7c81ac@intel.com \
--to=ferruh.yigit@intel.com \
--cc=dev@dpdk.org \
--cc=helin.zhang@intel.com \
--cc=jia.guo@intel.com \
--cc=jingjing.wu@intel.com \
--cc=konstantin.ananyev@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).