[dpdk-dev] [PATCH v2 1/2] drivers/i40e: fix X722 macro absence result in compile

Guo, Jia jia.guo at intel.com
Wed Oct 19 08:10:12 CEST 2016


hi,yigit

   Because of remove "#ifdef x722" will related with some prior patch 
and need related folks to make agreement on that, so we need to make a 
discuss later to get a conclusion. we definitely confidence that we will 
get the better balance base on the code maintenance effective. so just 
ignore this patch and i will rework new patch later. Thanks your great 
review and also appricate konstanitin's suggestion!

On 10/19/2016 12:22 AM, Ferruh Yigit wrote:
> 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
>



More information about the dev mailing list