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
>
