[dpdk-dev,v2,6/8] mbuf: use 2 bytes for port and nb segments
Checks
Commit Message
Change the size of m->port and m->nb_segs to 16 bits. It is now possible
to reference a port identifier larger than 256 and have a mbuf chain
larger than 256 segments.
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
app/test-pmd/csumonly.c | 4 ++--
.../linuxapp/eal/include/exec-env/rte_kni_common.h | 4 ++--
lib/librte_mbuf/rte_mbuf.h | 12 +++++++-----
3 files changed, 11 insertions(+), 9 deletions(-)
Comments
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
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.
Regards,
Olivier
> -----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>
> Cc: dev@dpdk.org; Ananyev, Konstantin <konstantin.ananyev@intel.com>;
> Richardson, Bruce <bruce.richardson@intel.com>;
> mb@smartsharesystems.com; Chilikin, Andrey <andrey.chilikin@intel.com>;
> jblunck@infradead.org; nelio.laranjeiro@6wind.com;
> arybchenko@solarflare.com; thomas.monjalon@6wind.com;
> jerin.jacob@caviumnetworks.com
> Subject: Re: [dpdk-dev] [PATCH v2 6/8] mbuf: use 2 bytes for port and nb
> segments
>
> 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?
Thanks
Zhihong
>
> Regards,
> Olivier
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>
> > Cc: dev@dpdk.org; Ananyev, Konstantin <konstantin.ananyev@intel.com>;
> > Richardson, Bruce <bruce.richardson@intel.com>;
> > mb@smartsharesystems.com; Chilikin, Andrey <andrey.chilikin@intel.com>;
> > jblunck@infradead.org; nelio.laranjeiro@6wind.com;
> > arybchenko@solarflare.com; thomas.monjalon@6wind.com;
> > jerin.jacob@caviumnetworks.com
> > Subject: Re: [dpdk-dev] [PATCH v2 6/8] mbuf: use 2 bytes for port and nb
> > segments
> >
> > 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
> -----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.
Med venlig hilsen / kind regards
- Morten Brørup
On Jul 10, 2017, at 3:15 AM, Morten Brørup <mb@smartsharesystems.com<mailto: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<mailto: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<mailto:yuanhan.liu@linux.intel.com>>
Hi Yuanhan,
On Thu, 6 Apr 2017 13:45:23 +0800, Yuanhan Liu
<yuanhan.liu@linux.intel.com<mailto: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
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.
Med venlig hilsen / kind regards
- Morten Brørup
Regards,
Keith
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wiles, Keith
> Sent: Tuesday, July 11, 2017 3:26 PM
> To: Morten Brørup
> Cc: Olivier Matz; Wang, Zhihong; Yuanhan Liu; DPDK; Ananyev,
> Konstantin; Richardson, Bruce; Chilikin, Andrey; Jan Blunck;
> nelio.laranjeiro@6wind.com; arybchenko@solarflare.com;
> thomas.monjalon@6wind.com; jerin.jacob@caviumnetworks.com
> Subject: Re: [dpdk-dev] [PATCH v2 6/8] mbuf: use 2 bytes for port and
> nb segments
>
>
> On Jul 10, 2017, at 3:15 AM, Morten Brørup
> <mb@smartsharesystems.com<mailto: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<mailto: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<mailto:yuanhan.liu@linux.intel.com>>
>
> Hi Yuanhan,
>
> On Thu, 6 Apr 2017 13:45:23 +0800, Yuanhan Liu
> <yuanhan.liu@linux.intel.com<mailto: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
>
> 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.
>
>
>
>
> Med venlig hilsen / kind regards
> - Morten Brørup
>
> Regards,
> Keith
What I was trying to communicate with my long argument about type definitions was: When the type changed from 8 bit to 16 bit, the type needs to change from uint8_t to uint16_t everywhere too, including in the ethdev APIs.
Don't start breaking coding conventions here by hiding the type of this variable.
Med venlig hilsen / kind regards
- Morten Brørup
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.
>
>
> Med venlig hilsen / kind regards
> - Morten Brørup
Regards,
Keith
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
11/07/2017 15:30, Morten Brørup:
> Morten Brørup wrote:
> > Olivier Matz wrote:
> > > 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.
> >
> > 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.
[...]
> What I was trying to communicate with my long argument about type definitions was: When the type changed from 8 bit to 16 bit, the type needs to change from uint8_t to uint16_t everywhere too, including in the ethdev APIs.
>
> Don't start breaking coding conventions here by hiding the type of this variable.
So, Morten, you are against the typedef, right?
Because we need to change it everywhere anyway, right?
Note: I have no strong opinion.
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
> Sent: Tuesday, July 11, 2017 5:06 PM
> To: Morten Brørup
> Cc: dev@dpdk.org; Wiles, Keith; Olivier Matz; Wang, Zhihong; Yuanhan
> Liu; Ananyev, Konstantin; Richardson, Bruce; Chilikin, Andrey; Jan
> Blunck; nelio.laranjeiro@6wind.com; arybchenko@solarflare.com;
> jerin.jacob@caviumnetworks.com
> Subject: Re: [dpdk-dev] [PATCH v2 6/8] mbuf: use 2 bytes for port and
> nbsegments
>
> 11/07/2017 15:30, Morten Brørup:
> > Morten Brørup wrote:
> > > Olivier Matz wrote:
> > > > 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.
> > >
> > > 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.
> [...]
> > What I was trying to communicate with my long argument about type
> definitions was: When the type changed from 8 bit to 16 bit, the type
> needs to change from uint8_t to uint16_t everywhere too, including in
> the ethdev APIs.
> >
> > Don't start breaking coding conventions here by hiding the type of
> this variable.
>
> So, Morten, you are against the typedef, right?
> Because we need to change it everywhere anyway, right?
>
> Note: I have no strong opinion.
I'm against the typedef because it would break convention, and I'm a strong proponent of conventions. In other projects, I'm all for typedefs, virtual classes, inheritance etc., but DPDK follows the Linux convention of not hiding simple types.
We need to change it from uint8_t everywhere, regardless what we change it to. (But if we need to change it again sometime in the future, then a typedef will save us next time.)
However, if we change the convention and start hiding simple types, they still need the rte_ prefix regardless if they are popular or obscure types. Even struct rte_mbuf has the rte_ prefix, and I consider that a very popular type. If so, rte_port_t would be a good name for this type.
My preference: Follow convention and change it to uint16_t everywhere.
Med venlig hilsen / kind regards
- Morten Brørup
> On Jul 11, 2017, at 10:23 AM, Morten Brørup <mb@smartsharesystems.com> wrote:
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
>> Sent: Tuesday, July 11, 2017 5:06 PM
>> To: Morten Brørup
>> Cc: dev@dpdk.org; Wiles, Keith; Olivier Matz; Wang, Zhihong; Yuanhan
>> Liu; Ananyev, Konstantin; Richardson, Bruce; Chilikin, Andrey; Jan
>> Blunck; nelio.laranjeiro@6wind.com; arybchenko@solarflare.com;
>> jerin.jacob@caviumnetworks.com
>> Subject: Re: [dpdk-dev] [PATCH v2 6/8] mbuf: use 2 bytes for port and
>> nbsegments
>>
>> 11/07/2017 15:30, Morten Brørup:
>>> Morten Brørup wrote:
>>>> Olivier Matz wrote:
>>>>> 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.
>>>>
>>>> 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.
>> [...]
>>> What I was trying to communicate with my long argument about type
>> definitions was: When the type changed from 8 bit to 16 bit, the type
>> needs to change from uint8_t to uint16_t everywhere too, including in
>> the ethdev APIs.
>>>
>>> Don't start breaking coding conventions here by hiding the type of
>> this variable.
>>
>> So, Morten, you are against the typedef, right?
>> Because we need to change it everywhere anyway, right?
>>
>> Note: I have no strong opinion.
>
> I'm against the typedef because it would break convention, and I'm a strong proponent of conventions. In other projects, I'm all for typedefs, virtual classes, inheritance etc., but DPDK follows the Linux convention of not hiding simple types.
>
> We need to change it from uint8_t everywhere, regardless what we change it to. (But if we need to change it again sometime in the future, then a typedef will save us next time.)
If the number of ports go beyond 64K then I will be the first one (if still alive) to eat this email. :-) The only reason to have more then 2 bytes would be to encode something into the port id value, which I could see, but a very slim chance IMHO.
>
> However, if we change the convention and start hiding simple types, they still need the rte_ prefix regardless if they are popular or obscure types. Even struct rte_mbuf has the rte_ prefix, and I consider that a very popular type. If so, rte_port_t would be a good name for this type.
>
> My preference: Follow convention and change it to uint16_t everywhere.
>
> Med venlig hilsen / kind regards
> - Morten Brørup
>
As we must change the uint8_t to uint16_t, then I would like it to be more descriptive via a typedef. I really do not see us needing to change it again in the near future. The only reason to make it a typedef is to be able to identify from just the prototype of the function that it takes a port ID value, which I am in favor of doing here for that reason.
As for Olivier’s statement about the typedef name I do not see the need for ‘_eth_' to be part of the typedef as it conveys no extra information in the name. Everything port related in DPDK is a ethernet type device or port. If we do add something like fiber channel maybe rte_pid_t is reason to that too, but if it contains ‘_eth_’ it would not.
I would like to see names that are just short enough to convey the information and not be redundant. IMHO rte_pid_t is fine, but if we use some something similar to the length of uint8_t (7) or uint16_t (8) characters then we would not have to also reformat the line more then needed. Using rte_pid_t (pid == port_id) we only extend the length by one (or two) characters and most likely the added byte(s) will not cause more format problems in the code.
Regards,
Keith
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wiles, Keith
> Sent: Tuesday, July 11, 2017 6:48 PM
> To: Morten Brørup
> Cc: Thomas Monjalon; DPDK; Olivier Matz; Wang, Zhihong; Yuanhan Liu;
> Ananyev, Konstantin; Richardson, Bruce; Chilikin, Andrey; Jan Blunck;
> Nélio Laranjeiro; arybchenko@solarflare.com;
> jerin.jacob@caviumnetworks.com
> Subject: Re: [dpdk-dev] [PATCH v2 6/8] mbuf: use 2 bytes for port and
> nbsegments
>
>
> > On Jul 11, 2017, at 10:23 AM, Morten Brørup
> <mb@smartsharesystems.com> wrote:
> >
> >> -----Original Message-----
> >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
> >> Sent: Tuesday, July 11, 2017 5:06 PM
> >> To: Morten Brørup
> >> Cc: dev@dpdk.org; Wiles, Keith; Olivier Matz; Wang, Zhihong; Yuanhan
> >> Liu; Ananyev, Konstantin; Richardson, Bruce; Chilikin, Andrey; Jan
> >> Blunck; nelio.laranjeiro@6wind.com; arybchenko@solarflare.com;
> >> jerin.jacob@caviumnetworks.com
> >> Subject: Re: [dpdk-dev] [PATCH v2 6/8] mbuf: use 2 bytes for port
> and
> >> nbsegments
> >>
> >> 11/07/2017 15:30, Morten Brørup:
> >>> Morten Brørup wrote:
> >>>> Olivier Matz wrote:
> >>>>> 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.
> >>>>
> >>>> 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.
> >> [...]
> >>> What I was trying to communicate with my long argument about type
> >> definitions was: When the type changed from 8 bit to 16 bit, the
> type
> >> needs to change from uint8_t to uint16_t everywhere too, including
> in
> >> the ethdev APIs.
> >>>
> >>> Don't start breaking coding conventions here by hiding the type of
> >> this variable.
> >>
> >> So, Morten, you are against the typedef, right?
> >> Because we need to change it everywhere anyway, right?
> >>
> >> Note: I have no strong opinion.
> >
> > I'm against the typedef because it would break convention, and I'm a
> strong proponent of conventions. In other projects, I'm all for
> typedefs, virtual classes, inheritance etc., but DPDK follows the Linux
> convention of not hiding simple types.
> >
> > We need to change it from uint8_t everywhere, regardless what we
> > change it to. (But if we need to change it again sometime in the
> > future, then a typedef will save us next time.)
>
> If the number of ports go beyond 64K then I will be the first one (if
> still alive) to eat this email. :-) The only reason to have more then 2
> bytes would be to encode something into the port id value, which I
> could see, but a very slim chance IMHO.
>
> >
> > However, if we change the convention and start hiding simple types,
> they still need the rte_ prefix regardless if they are popular or
> obscure types. Even struct rte_mbuf has the rte_ prefix, and I consider
> that a very popular type. If so, rte_port_t would be a good name for
> this type.
> >
> > My preference: Follow convention and change it to uint16_t
> everywhere.
> >
> > Med venlig hilsen / kind regards
> > - Morten Brørup
> >
>
> As we must change the uint8_t to uint16_t, then I would like it to be
> more descriptive via a typedef. I really do not see us needing to
> change it again in the near future. The only reason to make it a
> typedef is to be able to identify from just the prototype of the
> function that it takes a port ID value, which I am in favor of doing
> here for that reason.
That is not a very good reason: When used as a function parameter, the type is only shown in the function declaration, whereas the variable name is shown every time it is used inside the function. So remember to always use meaningful variable names, such as "port" (like in the mbuf structure) or "port_id" (used in other places).
>
> As for Olivier’s statement about the typedef name I do not see the need
> for ‘_eth_' to be part of the typedef as it conveys no extra
> information in the name. Everything port related in DPDK is a ethernet
> type device or port. If we do add something like fiber channel maybe
> rte_pid_t is reason to that too, but if it contains ‘_eth_’ it would
> not.
>
> I would like to see names that are just short enough to convey the
> information and not be redundant. IMHO rte_pid_t is fine, but if we use
> some something similar to the length of uint8_t (7) or uint16_t (8)
> characters then we would not have to also reformat the line more then
> needed. Using rte_pid_t (pid == port_id) we only extend the length by
> one (or two) characters and most likely the added byte(s) will not
> cause more format problems in the code.
I still don't support typedefs for scalar types. I consider it against the coding style, although after reviewing the official DPDK Coding Style documentation (http://dpdk.org/doc/guides/contributing/coding_style.html), I can see that it is not explicitly stated. Please also note that section 1.5.7 of the DPDK Coding Style documentation says that the _t postfix should be avoided. Anyway, if we end up with a typedef, please don't use something resembling pid_t known from POSIX (https://www.gnu.org/software/libc/manual/html_node/Process-Identification.html).
>
> Regards,
> Keith
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Morten Brørup
> Sent: Wednesday, July 12, 2017 3:25 PM
> To: Wiles, Keith <keith.wiles@intel.com>
> Cc: Thomas Monjalon <thomas@monjalon.net>; DPDK <dev@dpdk.org>; Olivier
> Matz <olivier.matz@6wind.com>; Wang, Zhihong <zhihong.wang@intel.com>;
> Yuanhan Liu <yuanhan.liu@linux.intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; Richardson, Bruce
> <bruce.richardson@intel.com>; Chilikin, Andrey <andrey.chilikin@intel.com>;
> Jan Blunck <jblunck@infradead.org>; Nélio Laranjeiro
> <nelio.laranjeiro@6wind.com>; arybchenko@solarflare.com;
> jerin.jacob@caviumnetworks.com
> Subject: Re: [dpdk-dev] [PATCH v2 6/8] mbuf: use 2 bytes for port and
> nbsegments
>
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wiles, Keith
> > Sent: Tuesday, July 11, 2017 6:48 PM
> > To: Morten Brørup
> > Cc: Thomas Monjalon; DPDK; Olivier Matz; Wang, Zhihong; Yuanhan Liu;
> > Ananyev, Konstantin; Richardson, Bruce; Chilikin, Andrey; Jan Blunck;
> > Nélio Laranjeiro; arybchenko@solarflare.com;
> > jerin.jacob@caviumnetworks.com
> > Subject: Re: [dpdk-dev] [PATCH v2 6/8] mbuf: use 2 bytes for port and
> > nbsegments
> >
> >
> > > On Jul 11, 2017, at 10:23 AM, Morten Brørup
> > <mb@smartsharesystems.com> wrote:
> > >
> > >> -----Original Message-----
> > >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas
> > >> Monjalon
> > >> Sent: Tuesday, July 11, 2017 5:06 PM
> > >> To: Morten Brørup
> > >> Cc: dev@dpdk.org; Wiles, Keith; Olivier Matz; Wang, Zhihong;
> > >> Yuanhan Liu; Ananyev, Konstantin; Richardson, Bruce; Chilikin,
> > >> Andrey; Jan Blunck; nelio.laranjeiro@6wind.com;
> > >> arybchenko@solarflare.com; jerin.jacob@caviumnetworks.com
> > >> Subject: Re: [dpdk-dev] [PATCH v2 6/8] mbuf: use 2 bytes for port
> > and
> > >> nbsegments
> > >>
> > >> 11/07/2017 15:30, Morten Brørup:
> > >>> Morten Brørup wrote:
> > >>>> Olivier Matz wrote:
> > >>>>> 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.
> > >>>>
> > >>>> 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.
> > >> [...]
> > >>> What I was trying to communicate with my long argument about type
> > >> definitions was: When the type changed from 8 bit to 16 bit, the
> > type
> > >> needs to change from uint8_t to uint16_t everywhere too, including
> > in
> > >> the ethdev APIs.
> > >>>
> > >>> Don't start breaking coding conventions here by hiding the type of
> > >> this variable.
> > >>
> > >> So, Morten, you are against the typedef, right?
> > >> Because we need to change it everywhere anyway, right?
> > >>
> > >> Note: I have no strong opinion.
> > >
> > > I'm against the typedef because it would break convention, and I'm a
> > strong proponent of conventions. In other projects, I'm all for
> > typedefs, virtual classes, inheritance etc., but DPDK follows the
> > Linux convention of not hiding simple types.
> > >
> > > We need to change it from uint8_t everywhere, regardless what we
> > > change it to. (But if we need to change it again sometime in the
> > > future, then a typedef will save us next time.)
> >
> > If the number of ports go beyond 64K then I will be the first one (if
> > still alive) to eat this email. :-) The only reason to have more then
> > 2 bytes would be to encode something into the port id value, which I
> > could see, but a very slim chance IMHO.
> >
> > >
> > > However, if we change the convention and start hiding simple types,
> > they still need the rte_ prefix regardless if they are popular or
> > obscure types. Even struct rte_mbuf has the rte_ prefix, and I
> > consider that a very popular type. If so, rte_port_t would be a good
> > name for this type.
> > >
> > > My preference: Follow convention and change it to uint16_t
> > everywhere.
> > >
> > > Med venlig hilsen / kind regards
> > > - Morten Brørup
> > >
> >
> > As we must change the uint8_t to uint16_t, then I would like it to be
> > more descriptive via a typedef. I really do not see us needing to
> > change it again in the near future. The only reason to make it a
> > typedef is to be able to identify from just the prototype of the
> > function that it takes a port ID value, which I am in favor of doing
> > here for that reason.
>
> That is not a very good reason: When used as a function parameter, the type is
> only shown in the function declaration, whereas the variable name is shown
> every time it is used inside the function. So remember to always use meaningful
> variable names, such as "port" (like in the mbuf structure) or "port_id" (used in
> other places).
>
> >
> > As for Olivier’s statement about the typedef name I do not see the
> > need for ‘_eth_' to be part of the typedef as it conveys no extra
> > information in the name. Everything port related in DPDK is a ethernet
> > type device or port. If we do add something like fiber channel maybe
> > rte_pid_t is reason to that too, but if it contains ‘_eth_’ it would
> > not.
> >
> > I would like to see names that are just short enough to convey the
> > information and not be redundant. IMHO rte_pid_t is fine, but if we
> > use some something similar to the length of uint8_t (7) or uint16_t
> > (8) characters then we would not have to also reformat the line more
> > then needed. Using rte_pid_t (pid == port_id) we only extend the
> > length by one (or two) characters and most likely the added byte(s)
> > will not cause more format problems in the code.
>
> I still don't support typedefs for scalar types. I consider it against the coding
> style, although after reviewing the official DPDK Coding Style documentation
> (http://dpdk.org/doc/guides/contributing/coding_style.html), I can see that it is
> not explicitly stated. Please also note that section 1.5.7 of the DPDK Coding
> Style documentation says that the _t postfix should be avoided. Anyway, if we
> end up with a typedef, please don't use something resembling pid_t known from
> POSIX (https://www.gnu.org/software/libc/manual/html_node/Process-
> Identification.html).
>
How about rte_dev_id_t?
Thanks
Zhiyong
>
> >
> > Regards,
> > Keith
> -----Original Message-----
> From: Yang, Zhiyong [mailto:zhiyong.yang@intel.com]
> Sent: Wednesday, July 12, 2017 11:02 AM
> To: Morten Brørup; Wiles, Keith
> Cc: Thomas Monjalon; DPDK; Olivier Matz; Wang, Zhihong; Yuanhan Liu;
> Ananyev, Konstantin; Richardson, Bruce; Chilikin, Andrey; Jan Blunck;
> Nélio Laranjeiro; arybchenko@solarflare.com;
> jerin.jacob@caviumnetworks.com
> Subject: RE: [dpdk-dev] [PATCH v2 6/8] mbuf: use 2 bytes for port
> andnbsegments
>
>
>
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Morten Brørup
> > Sent: Wednesday, July 12, 2017 3:25 PM
> > To: Wiles, Keith <keith.wiles@intel.com>
> > Cc: Thomas Monjalon <thomas@monjalon.net>; DPDK <dev@dpdk.org>;
> > Olivier Matz <olivier.matz@6wind.com>; Wang, Zhihong
> > <zhihong.wang@intel.com>; Yuanhan Liu <yuanhan.liu@linux.intel.com>;
> > Ananyev, Konstantin <konstantin.ananyev@intel.com>; Richardson, Bruce
> > <bruce.richardson@intel.com>; Chilikin, Andrey
> > <andrey.chilikin@intel.com>; Jan Blunck <jblunck@infradead.org>;
> Nélio
> > Laranjeiro <nelio.laranjeiro@6wind.com>; arybchenko@solarflare.com;
> > jerin.jacob@caviumnetworks.com
> > Subject: Re: [dpdk-dev] [PATCH v2 6/8] mbuf: use 2 bytes for port and
> > nbsegments
> >
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wiles, Keith
> > > Sent: Tuesday, July 11, 2017 6:48 PM
> > > To: Morten Brørup
> > > Cc: Thomas Monjalon; DPDK; Olivier Matz; Wang, Zhihong; Yuanhan
> Liu;
> > > Ananyev, Konstantin; Richardson, Bruce; Chilikin, Andrey; Jan
> > > Blunck; Nélio Laranjeiro; arybchenko@solarflare.com;
> > > jerin.jacob@caviumnetworks.com
> > > Subject: Re: [dpdk-dev] [PATCH v2 6/8] mbuf: use 2 bytes for port
> > > and nbsegments
> > >
> > >
> > > > On Jul 11, 2017, at 10:23 AM, Morten Brørup
> > > <mb@smartsharesystems.com> wrote:
> > > >
> > > >> -----Original Message-----
> > > >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas
> > > >> Monjalon
> > > >> Sent: Tuesday, July 11, 2017 5:06 PM
> > > >> To: Morten Brørup
> > > >> Cc: dev@dpdk.org; Wiles, Keith; Olivier Matz; Wang, Zhihong;
> > > >> Yuanhan Liu; Ananyev, Konstantin; Richardson, Bruce; Chilikin,
> > > >> Andrey; Jan Blunck; nelio.laranjeiro@6wind.com;
> > > >> arybchenko@solarflare.com; jerin.jacob@caviumnetworks.com
> > > >> Subject: Re: [dpdk-dev] [PATCH v2 6/8] mbuf: use 2 bytes for
> port
> > > and
> > > >> nbsegments
> > > >>
> > > >> 11/07/2017 15:30, Morten Brørup:
> > > >>> Morten Brørup wrote:
> > > >>>> Olivier Matz wrote:
> > > >>>>> 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.
> > > >>>>
> > > >>>> 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.
> > > >> [...]
> > > >>> What I was trying to communicate with my long argument about
> > > >>> type
> > > >> definitions was: When the type changed from 8 bit to 16 bit, the
> > > type
> > > >> needs to change from uint8_t to uint16_t everywhere too,
> > > >> including
> > > in
> > > >> the ethdev APIs.
> > > >>>
> > > >>> Don't start breaking coding conventions here by hiding the type
> > > >>> of
> > > >> this variable.
> > > >>
> > > >> So, Morten, you are against the typedef, right?
> > > >> Because we need to change it everywhere anyway, right?
> > > >>
> > > >> Note: I have no strong opinion.
> > > >
> > > > I'm against the typedef because it would break convention, and
> I'm
> > > > a
> > > strong proponent of conventions. In other projects, I'm all for
> > > typedefs, virtual classes, inheritance etc., but DPDK follows the
> > > Linux convention of not hiding simple types.
> > > >
> > > > We need to change it from uint8_t everywhere, regardless what we
> > > > change it to. (But if we need to change it again sometime in the
> > > > future, then a typedef will save us next time.)
> > >
> > > If the number of ports go beyond 64K then I will be the first one
> > > (if still alive) to eat this email. :-) The only reason to have
> more
> > > then
> > > 2 bytes would be to encode something into the port id value, which
> I
> > > could see, but a very slim chance IMHO.
> > >
> > > >
> > > > However, if we change the convention and start hiding simple
> > > > types,
> > > they still need the rte_ prefix regardless if they are popular or
> > > obscure types. Even struct rte_mbuf has the rte_ prefix, and I
> > > consider that a very popular type. If so, rte_port_t would be a
> good
> > > name for this type.
> > > >
> > > > My preference: Follow convention and change it to uint16_t
> > > everywhere.
> > > >
> > > > Med venlig hilsen / kind regards
> > > > - Morten Brørup
> > > >
> > >
> > > As we must change the uint8_t to uint16_t, then I would like it to
> > > be more descriptive via a typedef. I really do not see us needing
> to
> > > change it again in the near future. The only reason to make it a
> > > typedef is to be able to identify from just the prototype of the
> > > function that it takes a port ID value, which I am in favor of
> doing
> > > here for that reason.
> >
> > That is not a very good reason: When used as a function parameter,
> the
> > type is only shown in the function declaration, whereas the variable
> > name is shown every time it is used inside the function. So remember
> > to always use meaningful variable names, such as "port" (like in the
> > mbuf structure) or "port_id" (used in other places).
> >
> > >
> > > As for Olivier’s statement about the typedef name I do not see the
> > > need for ‘_eth_' to be part of the typedef as it conveys no extra
> > > information in the name. Everything port related in DPDK is a
> > > ethernet type device or port. If we do add something like fiber
> > > channel maybe rte_pid_t is reason to that too, but if it contains
> > > ‘_eth_’ it would not.
> > >
> > > I would like to see names that are just short enough to convey the
> > > information and not be redundant. IMHO rte_pid_t is fine, but if we
> > > use some something similar to the length of uint8_t (7) or uint16_t
> > > (8) characters then we would not have to also reformat the line
> more
> > > then needed. Using rte_pid_t (pid == port_id) we only extend the
> > > length by one (or two) characters and most likely the added byte(s)
> > > will not cause more format problems in the code.
> >
> > I still don't support typedefs for scalar types. I consider it
> against
> > the coding style, although after reviewing the official DPDK Coding
> > Style documentation
> > (http://dpdk.org/doc/guides/contributing/coding_style.html), I can
> see
> > that it is not explicitly stated. Please also note that section 1.5.7
> > of the DPDK Coding Style documentation says that the _t postfix
> should
> > be avoided. Anyway, if we end up with a typedef, please don't use
> > something resembling pid_t known from POSIX
> > (https://www.gnu.org/software/libc/manual/html_node/Process-
> > Identification.html).
> >
>
> How about rte_dev_id_t?
>
> Thanks
> Zhiyong
>
> >
> > >
> > > Regards,
> > > Keith
If the DPDK Coding Style is based on Linux Coding Style, we should avoid typedefs in general, not just for structures. Please read Linus Torvalds' opinions about it: http://yarchive.net/comp/linux/typedefs.html
Perhaps the DPDK Coding Style should be updated to clarify this. (Or if we decide otherwise, to explicitly mention this deviation from the Linux coding style.)
Med venlig hilsen / kind regards
- Morten Brørup
> On Jul 12, 2017, at 2:25 AM, Morten Brørup <mb@smartsharesystems.com> wrote:
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wiles, Keith
>> Sent: Tuesday, July 11, 2017 6:48 PM
>> To: Morten Brørup
>> Cc: Thomas Monjalon; DPDK; Olivier Matz; Wang, Zhihong; Yuanhan Liu;
>> Ananyev, Konstantin; Richardson, Bruce; Chilikin, Andrey; Jan Blunck;
>> Nélio Laranjeiro; arybchenko@solarflare.com;
>> jerin.jacob@caviumnetworks.com
>> Subject: Re: [dpdk-dev] [PATCH v2 6/8] mbuf: use 2 bytes for port and
>> nbsegments
>>
>>
>>> On Jul 11, 2017, at 10:23 AM, Morten Brørup
>> <mb@smartsharesystems.com> wrote:
>>>
>>>> -----Original Message-----
>>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
>>>> Sent: Tuesday, July 11, 2017 5:06 PM
>>>> To: Morten Brørup
>>>> Cc: dev@dpdk.org; Wiles, Keith; Olivier Matz; Wang, Zhihong; Yuanhan
>>>> Liu; Ananyev, Konstantin; Richardson, Bruce; Chilikin, Andrey; Jan
>>>> Blunck; nelio.laranjeiro@6wind.com; arybchenko@solarflare.com;
>>>> jerin.jacob@caviumnetworks.com
>>>> Subject: Re: [dpdk-dev] [PATCH v2 6/8] mbuf: use 2 bytes for port
>> and
>>>> nbsegments
>>>>
>>>> 11/07/2017 15:30, Morten Brørup:
>>>>> Morten Brørup wrote:
>>>>>> Olivier Matz wrote:
>>>>>>> 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.
>>>>>>
>>>>>> 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.
>>>> [...]
>>>>> What I was trying to communicate with my long argument about type
>>>> definitions was: When the type changed from 8 bit to 16 bit, the
>> type
>>>> needs to change from uint8_t to uint16_t everywhere too, including
>> in
>>>> the ethdev APIs.
>>>>>
>>>>> Don't start breaking coding conventions here by hiding the type of
>>>> this variable.
>>>>
>>>> So, Morten, you are against the typedef, right?
>>>> Because we need to change it everywhere anyway, right?
>>>>
>>>> Note: I have no strong opinion.
>>>
>>> I'm against the typedef because it would break convention, and I'm a
>> strong proponent of conventions. In other projects, I'm all for
>> typedefs, virtual classes, inheritance etc., but DPDK follows the Linux
>> convention of not hiding simple types.
>>>
>>> We need to change it from uint8_t everywhere, regardless what we
>>> change it to. (But if we need to change it again sometime in the
>>> future, then a typedef will save us next time.)
>>
>> If the number of ports go beyond 64K then I will be the first one (if
>> still alive) to eat this email. :-) The only reason to have more then 2
>> bytes would be to encode something into the port id value, which I
>> could see, but a very slim chance IMHO.
>>
>>>
>>> However, if we change the convention and start hiding simple types,
>> they still need the rte_ prefix regardless if they are popular or
>> obscure types. Even struct rte_mbuf has the rte_ prefix, and I consider
>> that a very popular type. If so, rte_port_t would be a good name for
>> this type.
>>>
>>> My preference: Follow convention and change it to uint16_t
>> everywhere.
>>>
>>> Med venlig hilsen / kind regards
>>> - Morten Brørup
>>>
>>
>> As we must change the uint8_t to uint16_t, then I would like it to be
>> more descriptive via a typedef. I really do not see us needing to
>> change it again in the near future. The only reason to make it a
>> typedef is to be able to identify from just the prototype of the
>> function that it takes a port ID value, which I am in favor of doing
>> here for that reason.
>
> That is not a very good reason: When used as a function parameter, the type is only shown in the function declaration, whereas the variable name is shown every time it is used inside the function. So remember to always use meaningful variable names, such as "port" (like in the mbuf structure) or "port_id" (used in other places).
I stated in the prototype not docs, which does pop up in some edits and so forth. Using the correct variable names it a given and not the subject of this discussion. Even Torvalds’ states that typedefs are reasonable in some cases, not sure this one falls in to that slot. Typedefs for structures were the main concern from Torvald is what I took from his email and we are not talking about a structure here.
>
>>
>> As for Olivier’s statement about the typedef name I do not see the need
>> for ‘_eth_' to be part of the typedef as it conveys no extra
>> information in the name. Everything port related in DPDK is a ethernet
>> type device or port. If we do add something like fiber channel maybe
>> rte_pid_t is reason to that too, but if it contains ‘_eth_’ it would
>> not.
>>
>> I would like to see names that are just short enough to convey the
>> information and not be redundant. IMHO rte_pid_t is fine, but if we use
>> some something similar to the length of uint8_t (7) or uint16_t (8)
>> characters then we would not have to also reformat the line more then
>> needed. Using rte_pid_t (pid == port_id) we only extend the length by
>> one (or two) characters and most likely the added byte(s) will not
>> cause more format problems in the code.
>
> I still don't support typedefs for scalar types. I consider it against the coding style, although after reviewing the official DPDK Coding Style documentation (http://dpdk.org/doc/guides/contributing/coding_style.html), I can see that it is not explicitly stated. Please also note that section 1.5.7 of the DPDK Coding Style documentation says that the _t postfix should be avoided. Anyway, if we end up with a typedef, please don't use something resembling pid_t known from POSIX (https://www.gnu.org/software/libc/manual/html_node/Process-Identification.html).
Even the Linux uint16_t is a typedef, so even Linux thought using a typedef here was reasonable as it changes from arch to arch. The real question here is does the port id fall into this category. Most likely not in this case unless we think the variable will change in the future or we decide to encode more information to the variable then just a simple port id 0-N.
If we can not determine it falls into this category then we use a scalar typedef like uint16_t, which seems to be the norm here. My point about typedefs mainly was around the name if we have a typedef not to make it 40 characters long or have redundant information in the typedef name.
So it appears the port ID value is not going to change any time soon and we do not need to make it opaque or encode information into the variable, which to me means we need to use the standard typedef from Linux ‘uint16_t’.
Is that agreeable with everyone or do you have a good reason to make it a typedef?
>
>
>>
>> Regards,
>> Keith
Regards,
Keith
On Wed, 12 Jul 2017 11:50:38 +0200
Morten Brørup <mb@smartsharesystems.com> wrote:
> > -----Original Message-----
> > From: Yang, Zhiyong [mailto:zhiyong.yang@intel.com]
> > Sent: Wednesday, July 12, 2017 11:02 AM
> > To: Morten Brørup; Wiles, Keith
> > Cc: Thomas Monjalon; DPDK; Olivier Matz; Wang, Zhihong; Yuanhan Liu;
> > Ananyev, Konstantin; Richardson, Bruce; Chilikin, Andrey; Jan Blunck;
> > Nélio Laranjeiro; arybchenko@solarflare.com;
> > jerin.jacob@caviumnetworks.com
> > Subject: RE: [dpdk-dev] [PATCH v2 6/8] mbuf: use 2 bytes for port
> > andnbsegments
> >
> >
> >
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Morten Brørup
> > > Sent: Wednesday, July 12, 2017 3:25 PM
> > > To: Wiles, Keith <keith.wiles@intel.com>
> > > Cc: Thomas Monjalon <thomas@monjalon.net>; DPDK <dev@dpdk.org>;
> > > Olivier Matz <olivier.matz@6wind.com>; Wang, Zhihong
> > > <zhihong.wang@intel.com>; Yuanhan Liu <yuanhan.liu@linux.intel.com>;
> > > Ananyev, Konstantin <konstantin.ananyev@intel.com>; Richardson, Bruce
> > > <bruce.richardson@intel.com>; Chilikin, Andrey
> > > <andrey.chilikin@intel.com>; Jan Blunck <jblunck@infradead.org>;
> > Nélio
> > > Laranjeiro <nelio.laranjeiro@6wind.com>; arybchenko@solarflare.com;
> > > jerin.jacob@caviumnetworks.com
> > > Subject: Re: [dpdk-dev] [PATCH v2 6/8] mbuf: use 2 bytes for port and
> > > nbsegments
> > >
> > > > -----Original Message-----
> > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wiles, Keith
> > > > Sent: Tuesday, July 11, 2017 6:48 PM
> > > > To: Morten Brørup
> > > > Cc: Thomas Monjalon; DPDK; Olivier Matz; Wang, Zhihong; Yuanhan
> > Liu;
> > > > Ananyev, Konstantin; Richardson, Bruce; Chilikin, Andrey; Jan
> > > > Blunck; Nélio Laranjeiro; arybchenko@solarflare.com;
> > > > jerin.jacob@caviumnetworks.com
> > > > Subject: Re: [dpdk-dev] [PATCH v2 6/8] mbuf: use 2 bytes for port
> > > > and nbsegments
> > > >
> > > >
> > > > > On Jul 11, 2017, at 10:23 AM, Morten Brørup
> > > > <mb@smartsharesystems.com> wrote:
> > > > >
> > > > >> -----Original Message-----
> > > > >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas
> > > > >> Monjalon
> > > > >> Sent: Tuesday, July 11, 2017 5:06 PM
> > > > >> To: Morten Brørup
> > > > >> Cc: dev@dpdk.org; Wiles, Keith; Olivier Matz; Wang, Zhihong;
> > > > >> Yuanhan Liu; Ananyev, Konstantin; Richardson, Bruce; Chilikin,
> > > > >> Andrey; Jan Blunck; nelio.laranjeiro@6wind.com;
> > > > >> arybchenko@solarflare.com; jerin.jacob@caviumnetworks.com
> > > > >> Subject: Re: [dpdk-dev] [PATCH v2 6/8] mbuf: use 2 bytes for
> > port
> > > > and
> > > > >> nbsegments
> > > > >>
> > > > >> 11/07/2017 15:30, Morten Brørup:
> > > > >>> Morten Brørup wrote:
> > > > >>>> Olivier Matz wrote:
> > > > >>>>> 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.
> > > > >>>>
> > > > >>>> 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.
> > > > >> [...]
> > > > >>> What I was trying to communicate with my long argument about
> > > > >>> type
> > > > >> definitions was: When the type changed from 8 bit to 16 bit, the
> > > > type
> > > > >> needs to change from uint8_t to uint16_t everywhere too,
> > > > >> including
> > > > in
> > > > >> the ethdev APIs.
> > > > >>>
> > > > >>> Don't start breaking coding conventions here by hiding the type
> > > > >>> of
> > > > >> this variable.
> > > > >>
> > > > >> So, Morten, you are against the typedef, right?
> > > > >> Because we need to change it everywhere anyway, right?
> > > > >>
> > > > >> Note: I have no strong opinion.
> > > > >
> > > > > I'm against the typedef because it would break convention, and
> > I'm
> > > > > a
> > > > strong proponent of conventions. In other projects, I'm all for
> > > > typedefs, virtual classes, inheritance etc., but DPDK follows the
> > > > Linux convention of not hiding simple types.
> > > > >
> > > > > We need to change it from uint8_t everywhere, regardless what we
> > > > > change it to. (But if we need to change it again sometime in the
> > > > > future, then a typedef will save us next time.)
> > > >
> > > > If the number of ports go beyond 64K then I will be the first one
> > > > (if still alive) to eat this email. :-) The only reason to have
> > more
> > > > then
> > > > 2 bytes would be to encode something into the port id value, which
> > I
> > > > could see, but a very slim chance IMHO.
> > > >
> > > > >
> > > > > However, if we change the convention and start hiding simple
> > > > > types,
> > > > they still need the rte_ prefix regardless if they are popular or
> > > > obscure types. Even struct rte_mbuf has the rte_ prefix, and I
> > > > consider that a very popular type. If so, rte_port_t would be a
> > good
> > > > name for this type.
> > > > >
> > > > > My preference: Follow convention and change it to uint16_t
> > > > everywhere.
> > > > >
> > > > > Med venlig hilsen / kind regards
> > > > > - Morten Brørup
> > > > >
> > > >
> > > > As we must change the uint8_t to uint16_t, then I would like it to
> > > > be more descriptive via a typedef. I really do not see us needing
> > to
> > > > change it again in the near future. The only reason to make it a
> > > > typedef is to be able to identify from just the prototype of the
> > > > function that it takes a port ID value, which I am in favor of
> > doing
> > > > here for that reason.
> > >
> > > That is not a very good reason: When used as a function parameter,
> > the
> > > type is only shown in the function declaration, whereas the variable
> > > name is shown every time it is used inside the function. So remember
> > > to always use meaningful variable names, such as "port" (like in the
> > > mbuf structure) or "port_id" (used in other places).
> > >
> > > >
> > > > As for Olivier’s statement about the typedef name I do not see the
> > > > need for ‘_eth_' to be part of the typedef as it conveys no extra
> > > > information in the name. Everything port related in DPDK is a
> > > > ethernet type device or port. If we do add something like fiber
> > > > channel maybe rte_pid_t is reason to that too, but if it contains
> > > > ‘_eth_’ it would not.
> > > >
> > > > I would like to see names that are just short enough to convey the
> > > > information and not be redundant. IMHO rte_pid_t is fine, but if we
> > > > use some something similar to the length of uint8_t (7) or uint16_t
> > > > (8) characters then we would not have to also reformat the line
> > more
> > > > then needed. Using rte_pid_t (pid == port_id) we only extend the
> > > > length by one (or two) characters and most likely the added byte(s)
> > > > will not cause more format problems in the code.
> > >
> > > I still don't support typedefs for scalar types. I consider it
> > against
> > > the coding style, although after reviewing the official DPDK Coding
> > > Style documentation
> > > (http://dpdk.org/doc/guides/contributing/coding_style.html), I can
> > see
> > > that it is not explicitly stated. Please also note that section 1.5.7
> > > of the DPDK Coding Style documentation says that the _t postfix
> > should
> > > be avoided. Anyway, if we end up with a typedef, please don't use
> > > something resembling pid_t known from POSIX
> > > (https://www.gnu.org/software/libc/manual/html_node/Process-
> > > Identification.html).
> > >
> >
> > How about rte_dev_id_t?
> >
> > Thanks
> > Zhiyong
> >
> > >
> > > >
> > > > Regards,
> > > > Keith
>
> If the DPDK Coding Style is based on Linux Coding Style, we should avoid typedefs in general, not just for structures. Please read Linus Torvalds' opinions about it: http://yarchive.net/comp/linux/typedefs.html
>
> Perhaps the DPDK Coding Style should be updated to clarify this. (Or if we decide otherwise, to explicitly mention this deviation from the Linux coding style.)
It is logical to use typedef's for this kind of scalar type that may need to change.
Names matter, please avoid pid (confuse with posix) and dev (confuse with device id).
I would prefer: rte_portid_t and rte_queueid_t
Longer term, probably rte_eth_devices[] needs to go. Change port id into something
more like ifindex. And use sparse data structure to allow very large number of devices
and non-contiguous lookup. Think of a VPN server where each VPN connection looks
like a DPDK device.
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen Hemminger
> Sent: Wednesday, July 12, 2017 5:36 PM
> To: Morten Brørup
> Cc: Yang, Zhiyong; Wiles, Keith; Thomas Monjalon; DPDK; Olivier Matz;
> Wang, Zhihong; Yuanhan Liu; Ananyev, Konstantin; Richardson, Bruce;
> Chilikin, Andrey; Jan Blunck; Nélio Laranjeiro;
> arybchenko@solarflare.com; jerin.jacob@caviumnetworks.com
> Subject: Re: [dpdk-dev] [PATCH v2 6/8] mbuf: use 2 bytes for port
> andnbsegments
>
> On Wed, 12 Jul 2017 11:50:38 +0200
> Morten Brørup <mb@smartsharesystems.com> wrote:
>
> > > -----Original Message-----
> > > From: Yang, Zhiyong [mailto:zhiyong.yang@intel.com]
> > > Sent: Wednesday, July 12, 2017 11:02 AM
> > > To: Morten Brørup; Wiles, Keith
> > > Cc: Thomas Monjalon; DPDK; Olivier Matz; Wang, Zhihong; Yuanhan
> Liu;
> > > Ananyev, Konstantin; Richardson, Bruce; Chilikin, Andrey; Jan
> > > Blunck; Nélio Laranjeiro; arybchenko@solarflare.com;
> > > jerin.jacob@caviumnetworks.com
> > > Subject: RE: [dpdk-dev] [PATCH v2 6/8] mbuf: use 2 bytes for port
> > > andnbsegments
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Morten
> Brørup
> > > > Sent: Wednesday, July 12, 2017 3:25 PM
> > > > To: Wiles, Keith <keith.wiles@intel.com>
> > > > Cc: Thomas Monjalon <thomas@monjalon.net>; DPDK <dev@dpdk.org>;
> > > > Olivier Matz <olivier.matz@6wind.com>; Wang, Zhihong
> > > > <zhihong.wang@intel.com>; Yuanhan Liu
> > > > <yuanhan.liu@linux.intel.com>; Ananyev, Konstantin
> > > > <konstantin.ananyev@intel.com>; Richardson, Bruce
> > > > <bruce.richardson@intel.com>; Chilikin, Andrey
> > > > <andrey.chilikin@intel.com>; Jan Blunck <jblunck@infradead.org>;
> > > Nélio
> > > > Laranjeiro <nelio.laranjeiro@6wind.com>;
> > > > arybchenko@solarflare.com; jerin.jacob@caviumnetworks.com
> > > > Subject: Re: [dpdk-dev] [PATCH v2 6/8] mbuf: use 2 bytes for port
> > > > and nbsegments
> > > >
> > > > > -----Original Message-----
> > > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wiles,
> > > > > Keith
> > > > > Sent: Tuesday, July 11, 2017 6:48 PM
> > > > > To: Morten Brørup
> > > > > Cc: Thomas Monjalon; DPDK; Olivier Matz; Wang, Zhihong; Yuanhan
> > > Liu;
> > > > > Ananyev, Konstantin; Richardson, Bruce; Chilikin, Andrey; Jan
> > > > > Blunck; Nélio Laranjeiro; arybchenko@solarflare.com;
> > > > > jerin.jacob@caviumnetworks.com
> > > > > Subject: Re: [dpdk-dev] [PATCH v2 6/8] mbuf: use 2 bytes for
> > > > > port and nbsegments
> > > > >
> > > > >
> > > > > > On Jul 11, 2017, at 10:23 AM, Morten Brørup
> > > > > <mb@smartsharesystems.com> wrote:
> > > > > >
> > > > > >> -----Original Message-----
> > > > > >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas
> > > > > >> Monjalon
> > > > > >> Sent: Tuesday, July 11, 2017 5:06 PM
> > > > > >> To: Morten Brørup
> > > > > >> Cc: dev@dpdk.org; Wiles, Keith; Olivier Matz; Wang, Zhihong;
> > > > > >> Yuanhan Liu; Ananyev, Konstantin; Richardson, Bruce;
> > > > > >> Chilikin, Andrey; Jan Blunck; nelio.laranjeiro@6wind.com;
> > > > > >> arybchenko@solarflare.com; jerin.jacob@caviumnetworks.com
> > > > > >> Subject: Re: [dpdk-dev] [PATCH v2 6/8] mbuf: use 2 bytes for
> > > port
> > > > > and
> > > > > >> nbsegments
> > > > > >>
> > > > > >> 11/07/2017 15:30, Morten Brørup:
> > > > > >>> Morten Brørup wrote:
> > > > > >>>> Olivier Matz wrote:
> > > > > >>>>> 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.
> > > > > >>>>
> > > > > >>>> 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.
> > > > > >> [...]
> > > > > >>> What I was trying to communicate with my long argument
> about
> > > > > >>> type
> > > > > >> definitions was: When the type changed from 8 bit to 16 bit,
> > > > > >> the
> > > > > type
> > > > > >> needs to change from uint8_t to uint16_t everywhere too,
> > > > > >> including
> > > > > in
> > > > > >> the ethdev APIs.
> > > > > >>>
> > > > > >>> Don't start breaking coding conventions here by hiding the
> > > > > >>> type of
> > > > > >> this variable.
> > > > > >>
> > > > > >> So, Morten, you are against the typedef, right?
> > > > > >> Because we need to change it everywhere anyway, right?
> > > > > >>
> > > > > >> Note: I have no strong opinion.
> > > > > >
> > > > > > I'm against the typedef because it would break convention,
> and
> > > I'm
> > > > > > a
> > > > > strong proponent of conventions. In other projects, I'm all for
> > > > > typedefs, virtual classes, inheritance etc., but DPDK follows
> > > > > the Linux convention of not hiding simple types.
> > > > > >
> > > > > > We need to change it from uint8_t everywhere, regardless what
> > > > > > we change it to. (But if we need to change it again sometime
> > > > > > in the future, then a typedef will save us next time.)
> > > > >
> > > > > If the number of ports go beyond 64K then I will be the first
> > > > > one (if still alive) to eat this email. :-) The only reason to
> > > > > have
> > > more
> > > > > then
> > > > > 2 bytes would be to encode something into the port id value,
> > > > > which
> > > I
> > > > > could see, but a very slim chance IMHO.
> > > > >
> > > > > >
> > > > > > However, if we change the convention and start hiding simple
> > > > > > types,
> > > > > they still need the rte_ prefix regardless if they are popular
> > > > > or obscure types. Even struct rte_mbuf has the rte_ prefix, and
> > > > > I consider that a very popular type. If so, rte_port_t would be
> > > > > a
> > > good
> > > > > name for this type.
> > > > > >
> > > > > > My preference: Follow convention and change it to uint16_t
> > > > > everywhere.
> > > > > >
> > > > > > Med venlig hilsen / kind regards
> > > > > > - Morten Brørup
> > > > > >
> > > > >
> > > > > As we must change the uint8_t to uint16_t, then I would like it
> > > > > to be more descriptive via a typedef. I really do not see us
> > > > > needing
> > > to
> > > > > change it again in the near future. The only reason to make it
> a
> > > > > typedef is to be able to identify from just the prototype of
> the
> > > > > function that it takes a port ID value, which I am in favor of
> > > doing
> > > > > here for that reason.
> > > >
> > > > That is not a very good reason: When used as a function
> parameter,
> > > the
> > > > type is only shown in the function declaration, whereas the
> > > > variable name is shown every time it is used inside the function.
> > > > So remember to always use meaningful variable names, such as
> > > > "port" (like in the mbuf structure) or "port_id" (used in other
> places).
> > > >
> > > > >
> > > > > As for Olivier’s statement about the typedef name I do not see
> > > > > the need for ‘_eth_' to be part of the typedef as it conveys no
> > > > > extra information in the name. Everything port related in DPDK
> > > > > is a ethernet type device or port. If we do add something like
> > > > > fiber channel maybe rte_pid_t is reason to that too, but if it
> > > > > contains ‘_eth_’ it would not.
> > > > >
> > > > > I would like to see names that are just short enough to convey
> > > > > the information and not be redundant. IMHO rte_pid_t is fine,
> > > > > but if we use some something similar to the length of uint8_t
> > > > > (7) or uint16_t
> > > > > (8) characters then we would not have to also reformat the line
> > > more
> > > > > then needed. Using rte_pid_t (pid == port_id) we only extend
> the
> > > > > length by one (or two) characters and most likely the added
> > > > > byte(s) will not cause more format problems in the code.
> > > >
> > > > I still don't support typedefs for scalar types. I consider it
> > > against
> > > > the coding style, although after reviewing the official DPDK
> > > > Coding Style documentation
> > > > (http://dpdk.org/doc/guides/contributing/coding_style.html), I
> can
> > > see
> > > > that it is not explicitly stated. Please also note that section
> > > > 1.5.7 of the DPDK Coding Style documentation says that the _t
> > > > postfix
> > > should
> > > > be avoided. Anyway, if we end up with a typedef, please don't use
> > > > something resembling pid_t known from POSIX
> > > > (https://www.gnu.org/software/libc/manual/html_node/Process-
> > > > Identification.html).
> > > >
> > >
> > > How about rte_dev_id_t?
> > >
> > > Thanks
> > > Zhiyong
> > >
> > > >
> > > > >
> > > > > Regards,
> > > > > Keith
> >
> > If the DPDK Coding Style is based on Linux Coding Style, we should
> > avoid typedefs in general, not just for structures. Please read Linus
> > Torvalds' opinions about it:
> > http://yarchive.net/comp/linux/typedefs.html
> >
> > Perhaps the DPDK Coding Style should be updated to clarify this. (Or
> > if we decide otherwise, to explicitly mention this deviation from the
> > Linux coding style.)
>
> It is logical to use typedef's for this kind of scalar type that may
> need to change.
> Names matter, please avoid pid (confuse with posix) and dev (confuse
> with device id).
> I would prefer: rte_portid_t and rte_queueid_t
>
> Longer term, probably rte_eth_devices[] needs to go. Change port id
> into something more like ifindex. And use sparse data structure to
> allow very large number of devices and non-contiguous lookup. Think of
> a VPN server where each VPN connection looks like a DPDK device.
We are using a non-contiguous ifindex in our firmware, for virtual interfaces as you mention, so I get your point here! But until DPDK gets there, I suppose the DPDK port id is considered more or less contiguous.
You clearly have a longer track record working with Linus than me, so if you interpret the coding style like that, I will not object anymore - as my objection was based on coding style. And will someone please update the DPDK Coding Style document accordingly...
rte_portid_t is fine with me, but why not just rte_port_t?
PS: uint16_t is a standard C type, not a Linux specific type.
Med venlig hilsen / kind regards
- Morten Brørup
12/07/2017 17:57, Morten Brørup:
> From: Stephen Hemminger
> > Morten Brørup <mb@smartsharesystems.com> wrote:
> > > From: Yang, Zhiyong [mailto:zhiyong.yang@intel.com]
> > > > From: Morten Brørup
> > > > > From: Wiles, Keith
> > > > > > > On Jul 11, 2017, at 10:23 AM, Morten Brørup wrote:
> > > > > > > From: Thomas Monjalon
> > > > > > >> 11/07/2017 15:30, Morten Brørup:
> > > > > > >>> Morten Brørup wrote:
> > > > > > >>>> Olivier Matz wrote:
> > > > > > >>>>> 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.
> > > > > > >>>>
> > > > > > >>>> 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.
> > > > > > >> [...]
> > > > > > >>> What I was trying to communicate with my long argument
> > > > > > >>> about type definitions was:
> > > > > > >>> When the type changed from 8 bit to 16 bit, the type
> > > > > > >>> needs to change from uint8_t to uint16_t everywhere too,
> > > > > > >>> including in the ethdev APIs.
> > > > > > >>>
> > > > > > >>> Don't start breaking coding conventions here by hiding the
> > > > > > >>> type of this variable.
> > > > > > >>
> > > > > > >> So, Morten, you are against the typedef, right?
> > > > > > >> Because we need to change it everywhere anyway, right?
> > > > > > >>
> > > > > > >> Note: I have no strong opinion.
> > > > > > >
> > > > > > > I'm against the typedef because it would break convention,
> > > > > > > and I'm a strong proponent of conventions.
> > > > > > > In other projects, I'm all for typedefs, virtual classes,
> > > > > > > inheritance etc., but DPDK follows the Linux convention
> > > > > > > of not hiding simple types.
> > > > > > >
> > > > > > > We need to change it from uint8_t everywhere, regardless what
> > > > > > > we change it to. (But if we need to change it again sometime
> > > > > > > in the future, then a typedef will save us next time.)
> > > > > >
> > > > > > If the number of ports go beyond 64K then I will be the first
> > > > > > one (if still alive) to eat this email. :-) The only reason to
> > > > > > have more then 2 bytes would be to encode something into the
> > > > > > port id value, which I could see, but a very slim chance IMHO.
> > > > > >
> > > > > > > My preference: Follow convention and change it to uint16_t
> > > > > > > everywhere.
> > > > > >
> > > > > > As we must change the uint8_t to uint16_t, then I would like it
> > > > > > to be more descriptive via a typedef. I really do not see us
> > > > > > needing to change it again in the near future.
> > > > > > The only reason to make it a typedef is to be able to identify
> > > > > > from just the prototype of the function that it takes a port
> > > > > > ID value, which I am in favor of doing here for that reason.
> > > > >
> > > > > That is not a very good reason: When used as a function
> > > > > parameter, the type is only shown in the function declaration,
> > > > > whereas the variable name is shown every time it is used inside
> > > > > the function.
> > > > > So remember to always use meaningful variable names, such as
> > > > > "port" (like in the mbuf structure) or "port_id" (used in other
> > > > > places).
> > > > >
> > > > > I still don't support typedefs for scalar types. I consider it
> > > > > against the coding style, although after reviewing the official
> > > > > DPDK Coding Style documentation
> > > > > (http://dpdk.org/doc/guides/contributing/coding_style.html),
> > > > > I can see that it is not explicitly stated. Please also note
> > > > > that section 1.5.7 of the DPDK Coding Style documentation says
> > > > > that the _t postfix should be avoided. Anyway, if we end up
> > > > > with a typedef, please don't use something resembling pid_t
> > > > > known from POSIX
> > > > > (https://www.gnu.org/software/libc/manual/html_node/Process-
> > > > > Identification.html).
> > > >
> > > > How about rte_dev_id_t?
> > >
> > > If the DPDK Coding Style is based on Linux Coding Style, we should
> > > avoid typedefs in general, not just for structures. Please read Linus
> > > Torvalds' opinions about it:
> > > http://yarchive.net/comp/linux/typedefs.html
> > >
> > > Perhaps the DPDK Coding Style should be updated to clarify this. (Or
> > > if we decide otherwise, to explicitly mention this deviation from the
> > > Linux coding style.)
> >
> > It is logical to use typedef's for this kind of scalar type that may
> > need to change.
> > Names matter, please avoid pid (confuse with posix) and dev (confuse
> > with device id).
> > I would prefer: rte_portid_t and rte_queueid_t
> >
> > Longer term, probably rte_eth_devices[] needs to go. Change port id
> > into something more like ifindex. And use sparse data structure to
> > allow very large number of devices and non-contiguous lookup. Think of
> > a VPN server where each VPN connection looks like a DPDK device.
>
> We are using a non-contiguous ifindex in our firmware, for virtual
> interfaces as you mention, so I get your point here!
> But until DPDK gets there, I suppose the DPDK port id is considered
> more or less contiguous.
>
> You clearly have a longer track record working with Linus than me,
> so if you interpret the coding style like that, I will not object
> anymore - as my objection was based on coding style. And will someone
> please update the DPDK Coding Style document accordingly...
>
> rte_portid_t is fine with me, but why not just rte_port_t?
One problem with opaque typedef is that we don't know how to print them,
except if we have a PRIx macro.
So I suggest to keep with uint16_t (my preference),
or to add a printf format macro.
> On Jul 12, 2017, at 11:23 AM, Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 12/07/2017 17:57, Morten Brørup:
>> From: Stephen Hemminger
>>> Morten Brørup <mb@smartsharesystems.com> wrote:
>>>> From: Yang, Zhiyong [mailto:zhiyong.yang@intel.com]
>>>>> From: Morten Brørup
>>>>>> From: Wiles, Keith
>>>>>>>> On Jul 11, 2017, at 10:23 AM, Morten Brørup wrote:
>>>>>>>> From: Thomas Monjalon
>>>>>>>>> 11/07/2017 15:30, Morten Brørup:
>>>>>>>>>> Morten Brørup wrote:
>>>>>>>>>>> Olivier Matz wrote:
>>>>>>>>>>>> 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.
>>>>>>>>>>>
>>>>>>>>>>> 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.
>>>>>>>>> [...]
>>>>>>>>>> What I was trying to communicate with my long argument
>>>>>>>>>> about type definitions was:
>>>>>>>>>> When the type changed from 8 bit to 16 bit, the type
>>>>>>>>>> needs to change from uint8_t to uint16_t everywhere too,
>>>>>>>>>> including in the ethdev APIs.
>>>>>>>>>>
>>>>>>>>>> Don't start breaking coding conventions here by hiding the
>>>>>>>>>> type of this variable.
>>>>>>>>>
>>>>>>>>> So, Morten, you are against the typedef, right?
>>>>>>>>> Because we need to change it everywhere anyway, right?
>>>>>>>>>
>>>>>>>>> Note: I have no strong opinion.
>>>>>>>>
>>>>>>>> I'm against the typedef because it would break convention,
>>>>>>>> and I'm a strong proponent of conventions.
>>>>>>>> In other projects, I'm all for typedefs, virtual classes,
>>>>>>>> inheritance etc., but DPDK follows the Linux convention
>>>>>>>> of not hiding simple types.
>>>>>>>>
>>>>>>>> We need to change it from uint8_t everywhere, regardless what
>>>>>>>> we change it to. (But if we need to change it again sometime
>>>>>>>> in the future, then a typedef will save us next time.)
>>>>>>>
>>>>>>> If the number of ports go beyond 64K then I will be the first
>>>>>>> one (if still alive) to eat this email. :-) The only reason to
>>>>>>> have more then 2 bytes would be to encode something into the
>>>>>>> port id value, which I could see, but a very slim chance IMHO.
>>>>>>>
>>>>>>>> My preference: Follow convention and change it to uint16_t
>>>>>>>> everywhere.
>>>>>>>
>>>>>>> As we must change the uint8_t to uint16_t, then I would like it
>>>>>>> to be more descriptive via a typedef. I really do not see us
>>>>>>> needing to change it again in the near future.
>>>>>>> The only reason to make it a typedef is to be able to identify
>>>>>>> from just the prototype of the function that it takes a port
>>>>>>> ID value, which I am in favor of doing here for that reason.
>>>>>>
>>>>>> That is not a very good reason: When used as a function
>>>>>> parameter, the type is only shown in the function declaration,
>>>>>> whereas the variable name is shown every time it is used inside
>>>>>> the function.
>>>>>> So remember to always use meaningful variable names, such as
>>>>>> "port" (like in the mbuf structure) or "port_id" (used in other
>>>>>> places).
>>>>>>
>>>>>> I still don't support typedefs for scalar types. I consider it
>>>>>> against the coding style, although after reviewing the official
>>>>>> DPDK Coding Style documentation
>>>>>> (http://dpdk.org/doc/guides/contributing/coding_style.html),
>>>>>> I can see that it is not explicitly stated. Please also note
>>>>>> that section 1.5.7 of the DPDK Coding Style documentation says
>>>>>> that the _t postfix should be avoided. Anyway, if we end up
>>>>>> with a typedef, please don't use something resembling pid_t
>>>>>> known from POSIX
>>>>>> (https://www.gnu.org/software/libc/manual/html_node/Process-
>>>>>> Identification.html).
>>>>>
>>>>> How about rte_dev_id_t?
>>>>
>>>> If the DPDK Coding Style is based on Linux Coding Style, we should
>>>> avoid typedefs in general, not just for structures. Please read Linus
>>>> Torvalds' opinions about it:
>>>> http://yarchive.net/comp/linux/typedefs.html
>>>>
>>>> Perhaps the DPDK Coding Style should be updated to clarify this. (Or
>>>> if we decide otherwise, to explicitly mention this deviation from the
>>>> Linux coding style.)
>>>
>>> It is logical to use typedef's for this kind of scalar type that may
>>> need to change.
>>> Names matter, please avoid pid (confuse with posix) and dev (confuse
>>> with device id).
>>> I would prefer: rte_portid_t and rte_queueid_t
>>>
>>> Longer term, probably rte_eth_devices[] needs to go. Change port id
>>> into something more like ifindex. And use sparse data structure to
>>> allow very large number of devices and non-contiguous lookup. Think of
>>> a VPN server where each VPN connection looks like a DPDK device.
>>
>> We are using a non-contiguous ifindex in our firmware, for virtual
>> interfaces as you mention, so I get your point here!
>> But until DPDK gets there, I suppose the DPDK port id is considered
>> more or less contiguous.
>>
>> You clearly have a longer track record working with Linus than me,
>> so if you interpret the coding style like that, I will not object
>> anymore - as my objection was based on coding style. And will someone
>> please update the DPDK Coding Style document accordingly...
>>
>> rte_portid_t is fine with me, but why not just rte_port_t?
>
> One problem with opaque typedef is that we don't know how to print them,
> except if we have a PRIx macro.
>
> So I suggest to keep with uint16_t (my preference),
> or to add a printf format macro.
As in my previous email I think we have settled on uint16_t for the port and not a new typedef. Unless someone can define a compelling reason to use a new typedef.
Regards,
Keith
On Wed, Jul 12, 2017 at 06:23:38PM +0200, Thomas Monjalon wrote:
> 12/07/2017 17:57, Morten Brørup:
> > From: Stephen Hemminger
> > > Morten Brørup <mb@smartsharesystems.com> wrote:
> > > > From: Yang, Zhiyong [mailto:zhiyong.yang@intel.com]
> > > > > From: Morten Brørup
> > > > > > From: Wiles, Keith
> > > > > > > > On Jul 11, 2017, at 10:23 AM, Morten Brørup wrote:
> > > > > > > > From: Thomas Monjalon
> > > > > > > >> 11/07/2017 15:30, Morten Brørup:
> > > > > > > >>> Morten Brørup wrote:
> > > > > > > >>>> Olivier Matz wrote:
> > > > > > > >>>>> 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.
> > > > > > > >>>>
> > > > > > > >>>> 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.
> > > > > > > >> [...]
> > > > > > > >>> What I was trying to communicate with my long argument
> > > > > > > >>> about type definitions was:
> > > > > > > >>> When the type changed from 8 bit to 16 bit, the type
> > > > > > > >>> needs to change from uint8_t to uint16_t everywhere too,
> > > > > > > >>> including in the ethdev APIs.
> > > > > > > >>>
> > > > > > > >>> Don't start breaking coding conventions here by hiding the
> > > > > > > >>> type of this variable.
> > > > > > > >>
> > > > > > > >> So, Morten, you are against the typedef, right?
> > > > > > > >> Because we need to change it everywhere anyway, right?
> > > > > > > >>
> > > > > > > >> Note: I have no strong opinion.
> > > > > > > >
> > > > > > > > I'm against the typedef because it would break convention,
> > > > > > > > and I'm a strong proponent of conventions.
> > > > > > > > In other projects, I'm all for typedefs, virtual classes,
> > > > > > > > inheritance etc., but DPDK follows the Linux convention
> > > > > > > > of not hiding simple types.
> > > > > > > >
> > > > > > > > We need to change it from uint8_t everywhere, regardless what
> > > > > > > > we change it to. (But if we need to change it again sometime
> > > > > > > > in the future, then a typedef will save us next time.)
> > > > > > >
> > > > > > > If the number of ports go beyond 64K then I will be the first
> > > > > > > one (if still alive) to eat this email. :-) The only reason to
> > > > > > > have more then 2 bytes would be to encode something into the
> > > > > > > port id value, which I could see, but a very slim chance IMHO.
> > > > > > >
> > > > > > > > My preference: Follow convention and change it to uint16_t
> > > > > > > > everywhere.
> > > > > > >
> > > > > > > As we must change the uint8_t to uint16_t, then I would like it
> > > > > > > to be more descriptive via a typedef. I really do not see us
> > > > > > > needing to change it again in the near future.
> > > > > > > The only reason to make it a typedef is to be able to identify
> > > > > > > from just the prototype of the function that it takes a port
> > > > > > > ID value, which I am in favor of doing here for that reason.
> > > > > >
> > > > > > That is not a very good reason: When used as a function
> > > > > > parameter, the type is only shown in the function declaration,
> > > > > > whereas the variable name is shown every time it is used inside
> > > > > > the function.
> > > > > > So remember to always use meaningful variable names, such as
> > > > > > "port" (like in the mbuf structure) or "port_id" (used in other
> > > > > > places).
> > > > > >
> > > > > > I still don't support typedefs for scalar types. I consider it
> > > > > > against the coding style, although after reviewing the official
> > > > > > DPDK Coding Style documentation
> > > > > > (http://dpdk.org/doc/guides/contributing/coding_style.html),
> > > > > > I can see that it is not explicitly stated. Please also note
> > > > > > that section 1.5.7 of the DPDK Coding Style documentation says
> > > > > > that the _t postfix should be avoided. Anyway, if we end up
> > > > > > with a typedef, please don't use something resembling pid_t
> > > > > > known from POSIX
> > > > > > (https://www.gnu.org/software/libc/manual/html_node/Process-
> > > > > > Identification.html).
> > > > >
> > > > > How about rte_dev_id_t?
> > > >
> > > > If the DPDK Coding Style is based on Linux Coding Style, we should
> > > > avoid typedefs in general, not just for structures. Please read Linus
> > > > Torvalds' opinions about it:
> > > > http://yarchive.net/comp/linux/typedefs.html
> > > >
> > > > Perhaps the DPDK Coding Style should be updated to clarify this. (Or
> > > > if we decide otherwise, to explicitly mention this deviation from the
> > > > Linux coding style.)
> > >
> > > It is logical to use typedef's for this kind of scalar type that may
> > > need to change.
> > > Names matter, please avoid pid (confuse with posix) and dev (confuse
> > > with device id).
> > > I would prefer: rte_portid_t and rte_queueid_t
> > >
> > > Longer term, probably rte_eth_devices[] needs to go. Change port id
> > > into something more like ifindex. And use sparse data structure to
> > > allow very large number of devices and non-contiguous lookup. Think of
> > > a VPN server where each VPN connection looks like a DPDK device.
> >
> > We are using a non-contiguous ifindex in our firmware, for virtual
> > interfaces as you mention, so I get your point here!
> > But until DPDK gets there, I suppose the DPDK port id is considered
> > more or less contiguous.
> >
> > You clearly have a longer track record working with Linus than me,
> > so if you interpret the coding style like that, I will not object
> > anymore - as my objection was based on coding style. And will someone
> > please update the DPDK Coding Style document accordingly...
> >
> > rte_portid_t is fine with me, but why not just rte_port_t?
>
> One problem with opaque typedef is that we don't know how to print them,
> except if we have a PRIx macro.
>
> So I suggest to keep with uint16_t (my preference),
> or to add a printf format macro.
+1 for using basic types rather than typedefs.
@@ -583,7 +583,7 @@ pkt_copy_split(const struct rte_mbuf *pkt)
rc = mbuf_copy_split(pkt, md, seglen, nb_seg);
if (rc < 0)
RTE_LOG(ERR, USER1,
- "mbuf_copy_split for %p(len=%u, nb_seg=%hhu) "
+ "mbuf_copy_split for %p(len=%u, nb_seg=%u) "
"into %u segments failed with error code: %d\n",
pkt, pkt->pkt_len, pkt->nb_segs, nb_seg, rc);
@@ -801,7 +801,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
char buf[256];
printf("-----------------\n");
- printf("port=%u, mbuf=%p, pkt_len=%u, nb_segs=%hhu:\n",
+ printf("port=%u, mbuf=%p, pkt_len=%u, nb_segs=%u:\n",
fs->rx_port, m, m->pkt_len, m->nb_segs);
/* dump rx parsed packet info */
rte_get_rx_ol_flag_list(rx_ol_flags, buf, sizeof(buf));
@@ -118,8 +118,8 @@ struct rte_kni_mbuf {
uint64_t buf_physaddr;
uint16_t data_off; /**< Start address of data in segment buffer. */
char pad1[2];
- uint8_t nb_segs; /**< Number of segments. */
- char pad4[3];
+ uint16_t nb_segs; /**< Number of segments. */
+ char pad4[2];
uint64_t ol_flags; /**< Offload features. */
char pad2[4];
uint32_t pkt_len; /**< Total pkt len: sum of all segment data_len. */
@@ -400,12 +400,13 @@ struct rte_mbuf {
void *buf_addr; /**< Virtual address of segment buffer. */
phys_addr_t buf_physaddr; /**< Physical address of segment buffer. */
- /* next 6 bytes are initialised on RX descriptor rearm */
+ /* next 8 bytes are initialised on RX descriptor rearm */
MARKER64 rearm_data;
uint16_t data_off;
/**
- * 16-bit Reference counter.
+ * Reference counter. Its size should at least equal to the size
+ * of port field (16 bits), to support zero-copy broadcast.
* It should only be accessed using the following functions:
* rte_mbuf_refcnt_update(), rte_mbuf_refcnt_read(), and
* rte_mbuf_refcnt_set(). The functionality of these functions (atomic,
@@ -417,9 +418,10 @@ struct rte_mbuf {
rte_atomic16_t refcnt_atomic; /**< Atomically accessed refcnt */
uint16_t refcnt; /**< Non-atomically accessed refcnt */
};
- uint8_t nb_segs; /**< Number of segments. */
- uint8_t port; /**< Input port. */
- uint16_t pad; /**< 2B pad for naturally aligned ol_flags */
+ uint16_t nb_segs; /**< Number of segments. */
+
+ /** Input port (16 bits to support more than 256 virtual ports). */
+ uint16_t port;
uint64_t ol_flags; /**< Offload features. */