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

Ferruh Yigit ferruh.yigit at intel.com
Tue Oct 18 18:22:29 CEST 2016


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