Re: [dpdk-dev] [PATCH v3 1/4] ethdev: increase port_id range

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 9/13/2017 3:26 AM, Yang, Zhiyong wrote:
> Hi Ferruh,
> 
>> -----Original Message-----
>> From: Yigit, Ferruh
>> Sent: Monday, September 11, 2017 6:22 PM
>> To: Yang, Zhiyong <zhiyong.yang@intel.com>; dev@dpdk.org; Doherty, Declan
>> <declan.doherty@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>
>> Cc: thomas@monjalon.net; hemant.agrawal@nxp.com; Hunt, David
>> <david.hunt@intel.com>
>> Subject: Re: [PATCH v3 1/4] ethdev: increase port_id range
>>
>> On 9/9/2017 3:47 PM, Zhiyong Yang wrote:
>>> Extend port_id definition from uint8_t to uint16_t in lib and drivers
>>> data structures, specifically rte_eth_dev_data.
>>> Modify the APIs, drivers and app using port_id at the same time.
>>>
>>> Fix some checkpatch issues from the original code and remove some
>>> unnecessary cast operations.
>>>
>>> Signed-off-by: Zhiyong Yang <zhiyong.yang@intel.com>
>>

<...>

>>> @@ -1536,17 +1536,12 @@ rte_eth_bond_8023ad_setup_v1708(uint8_t
>> port_id,
>>>  	return 0;
>>>  }
>>>  BIND_DEFAULT_SYMBOL(rte_eth_bond_8023ad_setup, _v1708, 17.08);
>>> -MAP_STATIC_SYMBOL(int rte_eth_bond_8023ad_setup(uint8_t port_id,
>>> +MAP_STATIC_SYMBOL(int rte_eth_bond_8023ad_setup(uint16_t port_id,
>>
>> Hmm, this is tricky!
>> The macro MAP_STATIC_SYMBOL is used for ABI versioning, but changing the
>> port_id storage type breaks the ABI already. ABI versioning can be removed
>> completely. Cc'ed Declan.
>>
> Do you mean that I should remove 
>>> -MAP_STATIC_SYMBOL(int rte_eth_bond_8023ad_setup(uint8_t port_id, ?
> 
>> Which also reminds me that bonding LIBABIVER needs to be updated. This is also
>> required for all i40e, ixgbe and bnxt. Please let me know if you need help here.
>>
> Yes. I'm not clear about it. Need help Ferruh.

There are two bonding APIs [1] using ABI versioning [2], and both have
port_id as parameter. Since we are already breaking ABI, no need to keep
older versions of APIs.

So what needs to be done is, in .c file, remove those APIs and
versioning macros [3], rename latest version of API [6] and update .map
file [4] to remove exposed ABIs [5] from _older_ versions, only keep
latest ones.

I believe ideally these updates should be in this patch, but this patch
is big and no need to make it more confusing, I would suggest making
these changes _before_ this patch, so get rid of unnecessary update in
this patch.


[1]
rte_eth_bond_8023ad_conf_get_v20()
rte_eth_bond_8023ad_conf_get_v1607()
rte_eth_bond_8023ad_setup_v20()
rte_eth_bond_8023ad_setup_v1607()

[2]
http://dpdk.org/doc/guides/contributing/versioning.html

[3]
VERSION_SYMBOL, MAP_STATIC_SYMBOL, BIND_DEFAULT_SYMBOL

[4]
rte_eth_bond_version.map

[5]
rte_eth_bond_8023ad_setup
rte_eth_bond_8023ad_conf_get

[6]
rte_eth_bond_8023ad_conf_get_v1708 -->
	rte_eth_bond_8023ad_conf_get
rte_eth_bond_8023ad_setup_v1708 --->
	rte_eth_bond_8023ad_setup