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

["Wiles, Keith" <keith.wiles@intel.com>]

> On Sep 19, 2017, at 1:05 AM, Yang, Zhiyong <zhiyong.yang@intel.com> wrote:
> 
> 
> 
>> -----Original Message-----
>> From: Yigit, Ferruh
>> Sent: Wednesday, September 13, 2017 9:33 PM
>> To: Thomas Monjalon <thomas@monjalon.net>
>> Cc: Yang, Zhiyong <zhiyong.yang@intel.com>; dev@dpdk.org; Doherty, Declan
>> <declan.doherty@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>;
>> hemant.agrawal@nxp.com; Hunt, David <david.hunt@intel.com>; Richardson,
>> Bruce <bruce.richardson@intel.com>; Ananyev, Konstantin
>> <konstantin.ananyev@intel.com>
>> Subject: Re: [PATCH v3 1/4] ethdev: increase port_id range
>> 
>> On 9/13/2017 1:18 PM, Thomas Monjalon wrote:
>>> 13/09/2017 13:56, Ferruh Yigit:
>>>> On 9/13/2017 3:26 AM, Yang, Zhiyong wrote:
>>>>> From: Yigit, Ferruh
>>>>>> 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>
>>>>>> 
>>>>>> <...>
>>>>>> 
>>>>>>> @@ -283,7 +283,7 @@ enum dcb_mode_enable  #define
>>>>>>> MAX_RX_QUEUE_STATS_MAPPINGS 4096 /* MAX_PORT of 32 @ 128
>>>>>>> rx_queues/port */
>>>>>>> 
>>>>>>> struct queue_stats_mappings {
>>>>>>> -	uint8_t port_id;
>>>>>>> +	uint16_t port_id;
>>>>>> 
>>>>>> Can this be "portid_t port_id;" ? For testpmd, portid_t can be used
>>>>>> for all port_id declarations.
>>>>>> 
>>>>> 
>>>>> Ferruh, the suggestion has been discussed in the following thread.
>>>>> Most of people agree on The basic type uint16_t. :).  Your suggestion was
>> my preference  previously.
>>>>> At last, I make this decision to use uint16_t.  You know, whatever I
>>>>> use, some ones will stand out and Say the other is better.  :)
>>>>> http://www.dpdk.org/dev/patchwork/patch/23208/
>>>> 
>>>> This discussion was whole dpdk, my comment is for testpmd only.
>>>> 
>>>> Testpmd already defines "portid_t" and uses it in many places [1]. I
>>>> am saying why keep using "uint16_t" in some places in testpmd? Lets
>>>> switch all to "portid_t" while we are touching them all.
>>>> 
>>>> [1]
>>>>  -typedef uint8_t  portid_t;
>>>>  +typedef uint16_t portid_t;
>>> 
>>> Or the reverse, we can drop portid_t from testpmd, especially if it is
>>> not used everywhere in testpmd.
>>> Note: this typedef hides the size of the port, which may be important
>>> when optimizing code.
>> 
>> No strong opinion about keeping "uint16_t" or "portid_t", "portid_t" is already in
>> use, not sure if worth the effort to remove it.
>> 
>> But I am for unifying the storage type used, one or other.
> 
> Think again. If basic type can bring the benefit as Thomas said, we may consider to use uint16_t instead of portid_t in testpmd. 

We have already decided to use uint16_t for the port ID type and we should convert testpmd to use that typedef instead of the one it is using. I know this changes testpmd a bit, but it is also very low risk to testpmd.

> 
> Zhiyong

Regards,
Keith