> -----Original Message----- > From: Adrien Mazarguil [mailto:adrien.mazarg...@6wind.com] > Sent: Monday, September 4, 2017 2:12 PM > To: Yang, Zhiyong <zhiyong.y...@intel.com> > Cc: Yigit, Ferruh <ferruh.yi...@intel.com>; Richardson, Bruce > <bruce.richard...@intel.com>; dev@dpdk.org; tho...@monjalon.net; Wiles, > Keith <keith.wi...@intel.com>; step...@networkplumber.org; Nelio > Laranjeiro <nelio.laranje...@6wind.com> > Subject: Re: [dpdk-dev] [PATCH v2 1/4] ethdev: increase port_id range > > Hi Zhiyong, > > On Mon, Sep 04, 2017 at 09:47:10AM +0000, Yang, Zhiyong wrote: > > Hi, Ferruh, Bruce: > > > > > -----Original Message----- > > > From: Yigit, Ferruh > > > Sent: Monday, September 4, 2017 5:30 PM > > > To: Richardson, Bruce <bruce.richard...@intel.com>; Yang, Zhiyong > > > <zhiyong.y...@intel.com> > > > Cc: dev@dpdk.org; tho...@monjalon.net; Wiles, Keith > > > <keith.wi...@intel.com>; step...@networkplumber.org > > > Subject: Re: [dpdk-dev] [PATCH v2 1/4] ethdev: increase port_id > > > range > > > > > > On 9/4/2017 10:06 AM, Bruce Richardson wrote: > > > > On Mon, Sep 04, 2017 at 01:57:31PM +0800, 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 except > > > >> some drivers such as MLX4 and MLX5 due to fail to compile them in > my server. > > > >> > > > > I think you can change those drivers too - it's not hard to set up > > > > compilation for MLX drivers (instruction in DPDK docs on the > > > > website), and even if you can't compile test them, e.g. dpaa2 > > > > drivers, or other SoC ones, others can do so on your behalf. If > > > > you are going to change drivers, I think you should change all of > them across the board. > > > > > > +1 > > > > OK. I will change them. > > I haven't applied the series yet but I think mlx4 doesn't need any > modification to support the new width. mlx5, on the other hand, at least > uses the following field in its data path: > > unsigned int port_id:8; > > One related question, why not define a new type (like testpmd's portid_t) > part of rte_ethdev.h? (rte_portid_t?) > > I think uint16_t may not last long with virtual ports and all, and when it > becomes necessary, the switch to uint32_t will be painful. A typedef > should also ease the conversion of user applications. > > If you choose to use a typedef, I suggest to do so in a separate patch > first (uint8_t => rte_portid_t) before upgrading rte_portid_t to 16 bits > in the subsequent patch. It means the first patch is large but trivial > while the second one is shorter but deals with the complex changes such as > the one needed for mlx5. > > Thanks.
The downside of hiding the size is that it becomes harder to reason about the layout of key structures like mbuf. Probably not a huge issue, though. A better question would be whether we would see the port id ever needing to increase in size to 32-bits? Even with virtual ports, I find it hard to see us needing more 64k ports in a single application. Regards, /Bruce