From: "Chilikin, Andrey" <andrey.chilikin@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: Mon, 17 Oct 2016 10:14:27 +0000 [thread overview]
Message-ID: <AAC06825A3B29643AF5372F5E0DDF053644EDBE0@IRSMSX106.ger.corp.intel.com> (raw)
In-Reply-To: <2601191342CEEE43887BDE71AB9772583F0C23DD@irsmsx105.ger.corp.intel.com>
In addition to Konstantin's point - some configuration settings, like RSS input set and PTYPEs, could be firmware dependent and change between fw versions for the same X710/X722 device. Moving mapping tables to the dev private data and initializing it on device start up will make code much cleaner.
Regards,
/Andrey
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ananyev, Konstantin
> Sent: Monday, October 17, 2016 10:55 AM
> To: Guo, Jia <jia.guo@intel.com>; Zhang, Helin <helin.zhang@intel.com>; Wu,
> Jingjing <jingjing.wu@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 1/2] drivers/i40e: fix X722 macro absence
> result in compile
>
> 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.
> Konstantin
>
>
>
> >
> >
> > On 10/16/2016 9:31 PM, Ananyev, Konstantin wrote:
> > > Hi Jeff,
> > >
> > >> -----Original Message-----
> > >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jeff Guo
> > >> Sent: Sunday, October 16, 2016 2:40 AM
> > >> To: Zhang, Helin <helin.zhang@intel.com>; Wu, Jingjing
> > >> <jingjing.wu@intel.com>
> > >> Cc: dev@dpdk.org; Guo, Jia <jia.guo@intel.com>
> > >> Subject: [dpdk-dev] [PATCH v2 1/2] drivers/i40e: fix X722 macro
> > >> absence result in compile
> > >>
> > >> Since some register only be supported by X722 but may not be
> > >> supported by other NICs, so add X722 macro to distinguish that to
> > >> avoid compile error when the X722 macro is undefined.
> > >
> > > Two probably silly questions:
> > > 1) So who will setup X722_SUPPORT macro?
> > > Is that a user responsibility when he is building dpdk i40e PMD?
> > > If so, why it is not a rte_config option?
> > > 2) Why this all has to be build time decision?
> > > Why nor run-time?
> > > Why i40e driver can't support all devices (including x722) and
> > > invoke different config functions (write different registers) based
> > > on device type/id information?
> > > As it does for other device types/ids?
> > >
> > > Konstantin
next prev parent reply other threads:[~2016-10-17 10:14 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 [this message]
2016-10-18 16:22 ` Ferruh Yigit
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=AAC06825A3B29643AF5372F5E0DDF053644EDBE0@IRSMSX106.ger.corp.intel.com \
--to=andrey.chilikin@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).