> -----Original Message----- > From: Yigit, Ferruh > Sent: Wednesday, September 13, 2017 9:33 PM > To: Thomas Monjalon <tho...@monjalon.net> > Cc: Yang, Zhiyong <zhiyong.y...@intel.com>; dev@dpdk.org; Doherty, Declan > <declan.dohe...@intel.com>; Lu, Wenzhuo <wenzhuo...@intel.com>; > hemant.agra...@nxp.com; Hunt, David <david.h...@intel.com>; Richardson, > Bruce <bruce.richard...@intel.com>; Ananyev, Konstantin > <konstantin.anan...@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.y...@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. Zhiyong