Re: [PATCH v2 6/8] mbuf: use 2 bytes for port and nb segments

[Olivier MATZ <olivier.matz@6wind.com>]

On Tue, 11 Jul 2017 13:34:47 +0000, "Wiles, Keith" <keith.wiles@intel.com> wrote:
> Resend because of format problems sorry.
> 
> > On Jul 10, 2017, at 3:15 AM, Morten Brørup <mb@smartsharesystems.com> wrote:
> >   
> >> -----Original Message-----
> >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier Matz
> >> Sent: Monday, July 10, 2017 10:00 AM
> >> Subject: Re: [dpdk-dev] [PATCH v2 6/8] mbuf: use 2 bytes for port and
> >> nb segments
> >> 
> >> Hi,
> >> 
> >> On Tue, 4 Jul 2017 07:54:23 +0000, "Wang, Zhihong"
> >> <zhihong.wang@intel.com> wrote:  
> >>>> -----Original Message-----
> >>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier MATZ
> >>>> Sent: Tuesday, April 18, 2017 9:03 PM
> >>>> To: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> >>>> 
> >>>> Hi Yuanhan,
> >>>> 
> >>>> On Thu, 6 Apr 2017 13:45:23 +0800, Yuanhan Liu
> >>>> <yuanhan.liu@linux.intel.com> wrote:  
> >>>>> Hi Olivier,
> >>>>> 
> >>>>> On Tue, Apr 04, 2017 at 06:28:05PM +0200, Olivier Matz wrote:  
> >>>>>> Change the size of m->port and m->nb_segs to 16 bits.  
> >>>>> 
> >>>>> But all the ethdev APIs are still using 8 bits. 16 bits won't
> >>>>> really take effect without updating those APIs. Any plans?
> >>>>> 
> >>>>> 	--yliu  
> >>>> 
> >>>> Yes, there is some work in ethdev, drivers and in example apps to
> >>>> make the change effective. I think we could define a specific type
> >>>> for a port number, maybe rte_eth_port_num_t. Using this type could
> >>>> be a first step (for 17.08) before switching to 16 bits (17.11?).
> >>>> 
> >>>> I'll do the change and send a rfc.  
> >>> 
> >>> Ping ;) Is this still in your plan?
> >>>   
> >> 
> >> Sorry, I don't think I will have time to work on this issue in the
> >> coming weeks. If you plan to do it, I will be happy to help with
> >> reviews and comments.
> >> 
> >> As I said in a previous message, I think a good first step would be to
> >> introduce a typedef for the port number: rte_eth_port_num_t.
> >> It can still be uint8_t for now, and can be switched to 16 bits in one
> >> step when everyone uses this new type.
> >> 
> >> Olivier  
> > 
> > I think that DPDK follows the Linux tradition of exposing the variable types, as opposed to hiding them behind typedefs. This has the unfortunate consequence that when a variable type changes, it has to be changed everywhere.
> > 
> > Introducing a rte_eth_port_num_t will require changing the same files at the same locations everywhere, so not even as a temporary solution will it be beneficial.  
> 
> I would like to see a much smaller typedef name here, we use it everywhere.
> rte_port_id_t
> port_id_t
> port_num_t
> portid_t
> pid_t
> rte_pid_t
> 
> I do not see why it needs to be rte_eth or even rte_, if we do not put eth in the name then is could be used in crypto or someplace else.

rte_ is required because we want to avoid namespace collision.
For instance, portid_t is too generic, and we would take the risk
that it is also defined for something else in another .h file.

Knowing it is is an ethernet port identifier, I also think eth_ is more
consistent regarding what we already have in rte_ethdev.h.

About num vs id, I have no strong opinion.

*pid_t looks really unclear to me :)


Olivier