[dpdk-dev,v2,3/4] common_base: extend RTE_MAX_ETHPORTS from 32 to 1024

Message ID 20170904055734.21354-4-zhiyong.yang@intel.com (mailing list archive)
State Superseded, archived
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Yang, Zhiyong Sept. 4, 2017, 5:57 a.m. UTC
  The reasons to modify RTE_MAX_ETHPORTS is the following.

1. RTE_MAX_ETHPORTS=32 by default has not met user's requirements
with development of virtualization technology. Some vdev users have
to modify the setting before the compiling.

2. port_id have been extended to 16 bits definition. But for many
samples such as testpmd, l3fwd, num of port is still limited to
RTE_MAX_ETHPORTS=32 by default. This may limit usage of 16 bits
port_id.

So, it is necessary to enlarge RTE_MAX_ETHPORTS to more than 256.

Signed-off-by: Zhiyong Yang <zhiyong.yang@intel.com>
---
 config/common_base | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Yao, Lei A Sept. 4, 2017, 7:46 a.m. UTC | #1
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Zhiyong Yang
> Sent: Monday, September 4, 2017 1:58 PM
> To: dev@dpdk.org
> Cc: thomas@monjalon.net; Yigit, Ferruh <ferruh.yigit@intel.com>; Wiles,
> Keith <keith.wiles@intel.com>; stephen@networkplumber.org; Yang,
> Zhiyong <zhiyong.yang@intel.com>
> Subject: [dpdk-dev] [PATCH v2 3/4] common_base: extend
> RTE_MAX_ETHPORTS from 32 to 1024
> 
> The reasons to modify RTE_MAX_ETHPORTS is the following.
> 
> 1. RTE_MAX_ETHPORTS=32 by default has not met user's requirements
> with development of virtualization technology. Some vdev users have
> to modify the setting before the compiling.
> 
> 2. port_id have been extended to 16 bits definition. But for many
> samples such as testpmd, l3fwd, num of port is still limited to
> RTE_MAX_ETHPORTS=32 by default. This may limit usage of 16 bits
> port_id.
> 
> So, it is necessary to enlarge RTE_MAX_ETHPORTS to more than 256.
> 
> Signed-off-by: Zhiyong Yang <zhiyong.yang@intel.com>
> ---
>  config/common_base | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/config/common_base b/config/common_base
> index 5e97a08b6..dccc13e31 100644
> --- a/config/common_base
> +++ b/config/common_base
> @@ -131,7 +131,7 @@ CONFIG_RTE_LIBRTE_KVARGS=y
>  #
>  CONFIG_RTE_LIBRTE_ETHER=y
>  CONFIG_RTE_LIBRTE_ETHDEV_DEBUG=n
> -CONFIG_RTE_MAX_ETHPORTS=32
> +CONFIG_RTE_MAX_ETHPORTS=1024
>  CONFIG_RTE_MAX_QUEUES_PER_PORT=1024
>  CONFIG_RTE_LIBRTE_IEEE1588=n
>  CONFIG_RTE_ETHDEV_QUEUE_STAT_CNTRS=16
> --
> 2.13.3
Hi, Zhiyong

I met one issue for changing CONFIG_RTE_MAX_ETHPORTS to 1024.
One process can only open 1024 file as maximum in common linux distribution, 
after practice, only 1009 socket file can be used for vdev device with testpmd
sample.
  
Yang, Zhiyong Sept. 4, 2017, 7:59 a.m. UTC | #2
Hi,  Lei:

> -----Original Message-----
> From: Yao, Lei A
> Sent: Monday, September 4, 2017 3:46 PM
> To: Yang, Zhiyong <zhiyong.yang@intel.com>; dev@dpdk.org
> Cc: thomas@monjalon.net; Yigit, Ferruh <ferruh.yigit@intel.com>; Wiles, Keith
> <keith.wiles@intel.com>; stephen@networkplumber.org; Yang, Zhiyong
> <zhiyong.yang@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v2 3/4] common_base: extend
> RTE_MAX_ETHPORTS from 32 to 1024
> 
> 
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Zhiyong Yang
> > Sent: Monday, September 4, 2017 1:58 PM
> > To: dev@dpdk.org
> > Cc: thomas@monjalon.net; Yigit, Ferruh <ferruh.yigit@intel.com>;
> > Wiles, Keith <keith.wiles@intel.com>; stephen@networkplumber.org;
> > Yang, Zhiyong <zhiyong.yang@intel.com>
> > Subject: [dpdk-dev] [PATCH v2 3/4] common_base: extend
> > RTE_MAX_ETHPORTS from 32 to 1024
> >
> > The reasons to modify RTE_MAX_ETHPORTS is the following.
> >
> > 1. RTE_MAX_ETHPORTS=32 by default has not met user's requirements with
> > development of virtualization technology. Some vdev users have to
> > modify the setting before the compiling.
> >
> > 2. port_id have been extended to 16 bits definition. But for many
> > samples such as testpmd, l3fwd, num of port is still limited to
> > RTE_MAX_ETHPORTS=32 by default. This may limit usage of 16 bits
> > port_id.
> >
> > So, it is necessary to enlarge RTE_MAX_ETHPORTS to more than 256.
> >
> > Signed-off-by: Zhiyong Yang <zhiyong.yang@intel.com>
> > ---
> >  config/common_base | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/config/common_base b/config/common_base index
> > 5e97a08b6..dccc13e31 100644
> > --- a/config/common_base
> > +++ b/config/common_base
> > @@ -131,7 +131,7 @@ CONFIG_RTE_LIBRTE_KVARGS=y  #
> > CONFIG_RTE_LIBRTE_ETHER=y  CONFIG_RTE_LIBRTE_ETHDEV_DEBUG=n
> > -CONFIG_RTE_MAX_ETHPORTS=32
> > +CONFIG_RTE_MAX_ETHPORTS=1024
> >  CONFIG_RTE_MAX_QUEUES_PER_PORT=1024
> >  CONFIG_RTE_LIBRTE_IEEE1588=n
> >  CONFIG_RTE_ETHDEV_QUEUE_STAT_CNTRS=16
> > --
> > 2.13.3
> Hi, Zhiyong
> 
> I met one issue for changing CONFIG_RTE_MAX_ETHPORTS to 1024.
> One process can only open 1024 file as maximum in common linux distribution,
> after practice, only 1009 socket file can be used for vdev device with testpmd
> sample.

Thanks for your info.  It seems that 1024 is too large and may bring some potential issues.

Thanks
Zhiyong
  
Bruce Richardson Sept. 4, 2017, 9:09 a.m. UTC | #3
On Mon, Sep 04, 2017 at 07:59:29AM +0000, Yang, Zhiyong wrote:
> Hi,  Lei:
> 
> > -----Original Message-----
> > From: Yao, Lei A
> > Sent: Monday, September 4, 2017 3:46 PM
> > To: Yang, Zhiyong <zhiyong.yang@intel.com>; dev@dpdk.org
> > Cc: thomas@monjalon.net; Yigit, Ferruh <ferruh.yigit@intel.com>; Wiles, Keith
> > <keith.wiles@intel.com>; stephen@networkplumber.org; Yang, Zhiyong
> > <zhiyong.yang@intel.com>
> > Subject: RE: [dpdk-dev] [PATCH v2 3/4] common_base: extend
> > RTE_MAX_ETHPORTS from 32 to 1024
> > 
> > 
> > 
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Zhiyong Yang
> > > Sent: Monday, September 4, 2017 1:58 PM
> > > To: dev@dpdk.org
> > > Cc: thomas@monjalon.net; Yigit, Ferruh <ferruh.yigit@intel.com>;
> > > Wiles, Keith <keith.wiles@intel.com>; stephen@networkplumber.org;
> > > Yang, Zhiyong <zhiyong.yang@intel.com>
> > > Subject: [dpdk-dev] [PATCH v2 3/4] common_base: extend
> > > RTE_MAX_ETHPORTS from 32 to 1024
> > >
> > > The reasons to modify RTE_MAX_ETHPORTS is the following.
> > >
> > > 1. RTE_MAX_ETHPORTS=32 by default has not met user's requirements with
> > > development of virtualization technology. Some vdev users have to
> > > modify the setting before the compiling.
> > >
> > > 2. port_id have been extended to 16 bits definition. But for many
> > > samples such as testpmd, l3fwd, num of port is still limited to
> > > RTE_MAX_ETHPORTS=32 by default. This may limit usage of 16 bits
> > > port_id.
> > >
> > > So, it is necessary to enlarge RTE_MAX_ETHPORTS to more than 256.
> > >
> > > Signed-off-by: Zhiyong Yang <zhiyong.yang@intel.com>
> > > ---
> > >  config/common_base | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/config/common_base b/config/common_base index
> > > 5e97a08b6..dccc13e31 100644
> > > --- a/config/common_base
> > > +++ b/config/common_base
> > > @@ -131,7 +131,7 @@ CONFIG_RTE_LIBRTE_KVARGS=y  #
> > > CONFIG_RTE_LIBRTE_ETHER=y  CONFIG_RTE_LIBRTE_ETHDEV_DEBUG=n
> > > -CONFIG_RTE_MAX_ETHPORTS=32
> > > +CONFIG_RTE_MAX_ETHPORTS=1024
> > >  CONFIG_RTE_MAX_QUEUES_PER_PORT=1024
> > >  CONFIG_RTE_LIBRTE_IEEE1588=n
> > >  CONFIG_RTE_ETHDEV_QUEUE_STAT_CNTRS=16
> > > --
> > > 2.13.3
> > Hi, Zhiyong
> > 
> > I met one issue for changing CONFIG_RTE_MAX_ETHPORTS to 1024.
> > One process can only open 1024 file as maximum in common linux distribution,
> > after practice, only 1009 socket file can be used for vdev device with testpmd
> > sample.
> 
> Thanks for your info.  It seems that 1024 is too large and may bring some potential issues.
> 
> Thanks
> Zhiyong
> 

It should be possible to have a dynamically allocated ethdev array,
which would allow use to have a default value - which could be e.g. 32
or 64 as now - while also allowing a run-time parameter to increase that
to thousands if needed.

/Bruce
  
Ananyev, Konstantin Sept. 4, 2017, 9:27 a.m. UTC | #4
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Yao, Lei A
> Sent: Monday, September 4, 2017 8:46 AM
> To: Yang, Zhiyong <zhiyong.yang@intel.com>; dev@dpdk.org
> Cc: thomas@monjalon.net; Yigit, Ferruh <ferruh.yigit@intel.com>; Wiles, Keith <keith.wiles@intel.com>; stephen@networkplumber.org;
> Yang, Zhiyong <zhiyong.yang@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v2 3/4] common_base: extend RTE_MAX_ETHPORTS from 32 to 1024
> 
> 
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Zhiyong Yang
> > Sent: Monday, September 4, 2017 1:58 PM
> > To: dev@dpdk.org
> > Cc: thomas@monjalon.net; Yigit, Ferruh <ferruh.yigit@intel.com>; Wiles,
> > Keith <keith.wiles@intel.com>; stephen@networkplumber.org; Yang,
> > Zhiyong <zhiyong.yang@intel.com>
> > Subject: [dpdk-dev] [PATCH v2 3/4] common_base: extend
> > RTE_MAX_ETHPORTS from 32 to 1024
> >
> > The reasons to modify RTE_MAX_ETHPORTS is the following.
> >
> > 1. RTE_MAX_ETHPORTS=32 by default has not met user's requirements
> > with development of virtualization technology. Some vdev users have
> > to modify the setting before the compiling.
> >
> > 2. port_id have been extended to 16 bits definition. But for many
> > samples such as testpmd, l3fwd, num of port is still limited to
> > RTE_MAX_ETHPORTS=32 by default. This may limit usage of 16 bits
> > port_id.
> >
> > So, it is necessary to enlarge RTE_MAX_ETHPORTS to more than 256.
> >
> > Signed-off-by: Zhiyong Yang <zhiyong.yang@intel.com>
> > ---
> >  config/common_base | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/config/common_base b/config/common_base
> > index 5e97a08b6..dccc13e31 100644
> > --- a/config/common_base
> > +++ b/config/common_base
> > @@ -131,7 +131,7 @@ CONFIG_RTE_LIBRTE_KVARGS=y
> >  #
> >  CONFIG_RTE_LIBRTE_ETHER=y
> >  CONFIG_RTE_LIBRTE_ETHDEV_DEBUG=n
> > -CONFIG_RTE_MAX_ETHPORTS=32
> > +CONFIG_RTE_MAX_ETHPORTS=1024
> >  CONFIG_RTE_MAX_QUEUES_PER_PORT=1024
> >  CONFIG_RTE_LIBRTE_IEEE1588=n
> >  CONFIG_RTE_ETHDEV_QUEUE_STAT_CNTRS=16
> > --
 > 2.13.3
> Hi, Zhiyong
> 
> I met one issue for changing CONFIG_RTE_MAX_ETHPORTS to 1024.
> One process can only open 1024 file as maximum in common linux distribution,
> after practice, only 1009 socket file can be used for vdev device with testpmd
> sample.

ulimit -Sn 10000
?
  
Yang, Zhiyong Sept. 4, 2017, 10:05 a.m. UTC | #5
Hi, Bruce:

> -----Original Message-----
> From: Richardson, Bruce
> Sent: Monday, September 4, 2017 5:09 PM
> To: Yang, Zhiyong <zhiyong.yang@intel.com>
> Cc: Yao, Lei A <lei.a.yao@intel.com>; dev@dpdk.org; thomas@monjalon.net;
> Yigit, Ferruh <ferruh.yigit@intel.com>; Wiles, Keith <keith.wiles@intel.com>;
> stephen@networkplumber.org
> Subject: Re: [dpdk-dev] [PATCH v2 3/4] common_base: extend
> RTE_MAX_ETHPORTS from 32 to 1024
> > > > --- a/config/common_base
> > > > +++ b/config/common_base
> > > > @@ -131,7 +131,7 @@ CONFIG_RTE_LIBRTE_KVARGS=y  #
> > > > CONFIG_RTE_LIBRTE_ETHER=y  CONFIG_RTE_LIBRTE_ETHDEV_DEBUG=n
> > > > -CONFIG_RTE_MAX_ETHPORTS=32
> > > > +CONFIG_RTE_MAX_ETHPORTS=1024
> > > >  CONFIG_RTE_MAX_QUEUES_PER_PORT=1024
> > > >  CONFIG_RTE_LIBRTE_IEEE1588=n
> > > >  CONFIG_RTE_ETHDEV_QUEUE_STAT_CNTRS=16
> > > > --
> > > > 2.13.3
> > > Hi, Zhiyong
> > >
> > > I met one issue for changing CONFIG_RTE_MAX_ETHPORTS to 1024.
> > > One process can only open 1024 file as maximum in common linux
> > > distribution, after practice, only 1009 socket file can be used for
> > > vdev device with testpmd sample.
> >
> > Thanks for your info.  It seems that 1024 is too large and may bring some
> potential issues.
> >
> > Thanks
> > Zhiyong
> >
> 
> It should be possible to have a dynamically allocated ethdev array, which would
> allow use to have a default value - which could be e.g. 32 or 64 as now - while
> also allowing a run-time parameter to increase that to thousands if needed.
> 
> /Bruce

In testpmd,  the following function will be called to validate the port_id. 
So, It is necessary to modify the max port num RTE_MAX_ETHPORTS. 

I think that RTE_MAX_ETHPORTS and  a default value(num of port ) should be different values.
Now dpdk limits the max num to RTE_MAX_ETHPORTS = 32 by default.  
int
rte_eth_dev_is_valid_port(uint16_t port_id)
{
	if (port_id >= RTE_MAX_ETHPORTS ||
	    (rte_eth_devices[port_id].state != RTE_ETH_DEV_ATTACHED &&
	     rte_eth_devices[port_id].state != RTE_ETH_DEV_DEFERRED))
		return 0;
	else
		return 1;
}

Thanks
Zhiyong.
  
Ananyev, Konstantin Sept. 4, 2017, 10:27 a.m. UTC | #6
Hi Zhiong,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Yang, Zhiyong
> Sent: Monday, September 4, 2017 11:05 AM
> To: Richardson, Bruce <bruce.richardson@intel.com>
> Cc: Yao, Lei A <lei.a.yao@intel.com>; dev@dpdk.org; thomas@monjalon.net; Yigit, Ferruh <ferruh.yigit@intel.com>; Wiles, Keith
> <keith.wiles@intel.com>; stephen@networkplumber.org
> Subject: Re: [dpdk-dev] [PATCH v2 3/4] common_base: extend RTE_MAX_ETHPORTS from 32 to 1024
> 
> Hi, Bruce:
> 
> > -----Original Message-----
> > From: Richardson, Bruce
> > Sent: Monday, September 4, 2017 5:09 PM
> > To: Yang, Zhiyong <zhiyong.yang@intel.com>
> > Cc: Yao, Lei A <lei.a.yao@intel.com>; dev@dpdk.org; thomas@monjalon.net;
> > Yigit, Ferruh <ferruh.yigit@intel.com>; Wiles, Keith <keith.wiles@intel.com>;
> > stephen@networkplumber.org
> > Subject: Re: [dpdk-dev] [PATCH v2 3/4] common_base: extend
> > RTE_MAX_ETHPORTS from 32 to 1024
> > > > > --- a/config/common_base
> > > > > +++ b/config/common_base
> > > > > @@ -131,7 +131,7 @@ CONFIG_RTE_LIBRTE_KVARGS=y  #
> > > > > CONFIG_RTE_LIBRTE_ETHER=y  CONFIG_RTE_LIBRTE_ETHDEV_DEBUG=n
> > > > > -CONFIG_RTE_MAX_ETHPORTS=32
> > > > > +CONFIG_RTE_MAX_ETHPORTS=1024
> > > > >  CONFIG_RTE_MAX_QUEUES_PER_PORT=1024
> > > > >  CONFIG_RTE_LIBRTE_IEEE1588=n
> > > > >  CONFIG_RTE_ETHDEV_QUEUE_STAT_CNTRS=16
> > > > > --
> > > > > 2.13.3
> > > > Hi, Zhiyong
> > > >
> > > > I met one issue for changing CONFIG_RTE_MAX_ETHPORTS to 1024.
> > > > One process can only open 1024 file as maximum in common linux
> > > > distribution, after practice, only 1009 socket file can be used for
> > > > vdev device with testpmd sample.
> > >
> > > Thanks for your info.  It seems that 1024 is too large and may bring some
> > potential issues.
> > >
> > > Thanks
> > > Zhiyong
> > >
> >
> > It should be possible to have a dynamically allocated ethdev array, which would
> > allow use to have a default value - which could be e.g. 32 or 64 as now - while
> > also allowing a run-time parameter to increase that to thousands if needed.
> >
> > /Bruce
> 
> In testpmd,  the following function will be called to validate the port_id.
> So, It is necessary to modify the max port num RTE_MAX_ETHPORTS.

There are quite a lot memory allocations (both static an dynamic) inside DPDK libs and sample apps
 that use RTE_MAX_ETHPORTS.
Increasing RTE_MAX_ETHPORTS would increase DPDK memory requirements quite significantly.
Why do you think it is *necessary* to increase default RTE_MAX_ETHPORTS to 1024?
Konstantin

> 
> I think that RTE_MAX_ETHPORTS and  a default value(num of port ) should be different values.
> Now dpdk limits the max num to RTE_MAX_ETHPORTS = 32 by default.
> int
> rte_eth_dev_is_valid_port(uint16_t port_id)
> {
> 	if (port_id >= RTE_MAX_ETHPORTS ||
> 	    (rte_eth_devices[port_id].state != RTE_ETH_DEV_ATTACHED &&
> 	     rte_eth_devices[port_id].state != RTE_ETH_DEV_DEFERRED))
> 		return 0;
> 	else
> 		return 1;
> }
> 
> Thanks
> Zhiyong.
  
Bruce Richardson Sept. 4, 2017, 10:29 a.m. UTC | #7
On Mon, Sep 04, 2017 at 11:05:11AM +0100, Yang, Zhiyong wrote:
> Hi, Bruce:
> 
> > -----Original Message-----
> > From: Richardson, Bruce
> > Sent: Monday, September 4, 2017 5:09 PM
> > To: Yang, Zhiyong <zhiyong.yang@intel.com>
> > Cc: Yao, Lei A <lei.a.yao@intel.com>; dev@dpdk.org; thomas@monjalon.net;
> > Yigit, Ferruh <ferruh.yigit@intel.com>; Wiles, Keith <keith.wiles@intel.com>;
> > stephen@networkplumber.org
> > Subject: Re: [dpdk-dev] [PATCH v2 3/4] common_base: extend
> > RTE_MAX_ETHPORTS from 32 to 1024
> > > > > --- a/config/common_base
> > > > > +++ b/config/common_base
> > > > > @@ -131,7 +131,7 @@ CONFIG_RTE_LIBRTE_KVARGS=y  #
> > > > > CONFIG_RTE_LIBRTE_ETHER=y  CONFIG_RTE_LIBRTE_ETHDEV_DEBUG=n
> > > > > -CONFIG_RTE_MAX_ETHPORTS=32
> > > > > +CONFIG_RTE_MAX_ETHPORTS=1024
> > > > >  CONFIG_RTE_MAX_QUEUES_PER_PORT=1024
> > > > >  CONFIG_RTE_LIBRTE_IEEE1588=n
> > > > >  CONFIG_RTE_ETHDEV_QUEUE_STAT_CNTRS=16
> > > > > --
> > > > > 2.13.3
> > > > Hi, Zhiyong
> > > >
> > > > I met one issue for changing CONFIG_RTE_MAX_ETHPORTS to 1024.
> > > > One process can only open 1024 file as maximum in common linux
> > > > distribution, after practice, only 1009 socket file can be used for
> > > > vdev device with testpmd sample.
> > >
> > > Thanks for your info.  It seems that 1024 is too large and may bring some
> > potential issues.
> > >
> > > Thanks
> > > Zhiyong
> > >
> > 
> > It should be possible to have a dynamically allocated ethdev array, which would
> > allow use to have a default value - which could be e.g. 32 or 64 as now - while
> > also allowing a run-time parameter to increase that to thousands if needed.
> > 
> > /Bruce
> 
> In testpmd,  the following function will be called to validate the port_id. 
> So, It is necessary to modify the max port num RTE_MAX_ETHPORTS. 
> 
> I think that RTE_MAX_ETHPORTS and  a default value(num of port ) should be different values.
> Now dpdk limits the max num to RTE_MAX_ETHPORTS = 32 by default.  
> int
> rte_eth_dev_is_valid_port(uint16_t port_id)
> {
> 	if (port_id >= RTE_MAX_ETHPORTS ||
> 	    (rte_eth_devices[port_id].state != RTE_ETH_DEV_ATTACHED &&
> 	     rte_eth_devices[port_id].state != RTE_ETH_DEV_DEFERRED))
> 		return 0;
> 	else
> 		return 1;
> }
>

If we have a dynamically allocated array size, the port_id can still be
bounds-checks. It's just the check comes from a variable rather than a
compile-time constant. It should not be any slower in practice.

As for an absolute max value, I won't object to having one, but I don't
see it as needed. If we don't use one, we have one automatically at 64k,
i.e. UINT_16_MAX. I fail to see the need to limit it to lower than that
- if a user wants that many ports, let them have them.

/Bruce
  
Yang, Zhiyong Sept. 4, 2017, 2:18 p.m. UTC | #8
Hi, konstantin:

> > > It should be possible to have a dynamically allocated ethdev array,
> > > which would allow use to have a default value - which could be e.g.
> > > 32 or 64 as now - while also allowing a run-time parameter to increase that
> to thousands if needed.
> > >
> > > /Bruce
> >
> > In testpmd,  the following function will be called to validate the port_id.
> > So, It is necessary to modify the max port num RTE_MAX_ETHPORTS.
> 
> There are quite a lot memory allocations (both static an dynamic) inside DPDK
> libs and sample apps  that use RTE_MAX_ETHPORTS.
> Increasing RTE_MAX_ETHPORTS would increase DPDK memory requirements
> quite significantly.
> Why do you think it is *necessary* to increase default RTE_MAX_ETHPORTS to
> 1024?
> Konstantin

One reason is that some users have the requirement of more than 32 ports now.
But it need to reconfigure and recompile dpdk. 
Another reason is that validating team doesn't modify the common_base when
doing regression test. But they need to validate if uint16_t port_id can work.

Bruce suggests to pass parameter and dynamically allocate ethdev array at run time.
It is another method. I'm thinking about it. 

Thanks
Zhiyong
  
Hemant Agrawal Sept. 6, 2017, 8:42 a.m. UTC | #9
On 9/4/2017 3:57 PM, Ananyev, Konstantin wrote:
> Hi Zhiong,
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Yang, Zhiyong
>> Sent: Monday, September 4, 2017 11:05 AM
>> To: Richardson, Bruce <bruce.richardson@intel.com>
>> Cc: Yao, Lei A <lei.a.yao@intel.com>; dev@dpdk.org; thomas@monjalon.net; Yigit, Ferruh <ferruh.yigit@intel.com>; Wiles, Keith
>> <keith.wiles@intel.com>; stephen@networkplumber.org
>> Subject: Re: [dpdk-dev] [PATCH v2 3/4] common_base: extend RTE_MAX_ETHPORTS from 32 to 1024
>>
>> Hi, Bruce:
>>
>>> -----Original Message-----
>>> From: Richardson, Bruce
>>> Sent: Monday, September 4, 2017 5:09 PM
>>> To: Yang, Zhiyong <zhiyong.yang@intel.com>
>>> Cc: Yao, Lei A <lei.a.yao@intel.com>; dev@dpdk.org; thomas@monjalon.net;
>>> Yigit, Ferruh <ferruh.yigit@intel.com>; Wiles, Keith <keith.wiles@intel.com>;
>>> stephen@networkplumber.org
>>> Subject: Re: [dpdk-dev] [PATCH v2 3/4] common_base: extend
>>> RTE_MAX_ETHPORTS from 32 to 1024
>>>>>> --- a/config/common_base
>>>>>> +++ b/config/common_base
>>>>>> @@ -131,7 +131,7 @@ CONFIG_RTE_LIBRTE_KVARGS=y  #
>>>>>> CONFIG_RTE_LIBRTE_ETHER=y  CONFIG_RTE_LIBRTE_ETHDEV_DEBUG=n
>>>>>> -CONFIG_RTE_MAX_ETHPORTS=32
>>>>>> +CONFIG_RTE_MAX_ETHPORTS=1024
>>>>>>  CONFIG_RTE_MAX_QUEUES_PER_PORT=1024
>>>>>>  CONFIG_RTE_LIBRTE_IEEE1588=n
>>>>>>  CONFIG_RTE_ETHDEV_QUEUE_STAT_CNTRS=16
>>>>>> --
>>>>>> 2.13.3
>>>>> Hi, Zhiyong
>>>>>
>>>>> I met one issue for changing CONFIG_RTE_MAX_ETHPORTS to 1024.
>>>>> One process can only open 1024 file as maximum in common linux
>>>>> distribution, after practice, only 1009 socket file can be used for
>>>>> vdev device with testpmd sample.
>>>>
>>>> Thanks for your info.  It seems that 1024 is too large and may bring some
>>> potential issues.
>>>>
>>>> Thanks
>>>> Zhiyong
>>>>
>>>
>>> It should be possible to have a dynamically allocated ethdev array, which would
>>> allow use to have a default value - which could be e.g. 32 or 64 as now - while
>>> also allowing a run-time parameter to increase that to thousands if needed.
>>>
>>> /Bruce
>>
>> In testpmd,  the following function will be called to validate the port_id.
>> So, It is necessary to modify the max port num RTE_MAX_ETHPORTS.
>
> There are quite a lot memory allocations (both static an dynamic) inside DPDK libs and sample apps
>  that use RTE_MAX_ETHPORTS.
> Increasing RTE_MAX_ETHPORTS would increase DPDK memory requirements quite significantly.

It is not a good idea to significantly increase the DPDK memory 
requirement.

Can you not make it with "CONFIG_RTE_MAX_ETHPORTS=1024" for your testing 
configs?

> Why do you think it is *necessary* to increase default RTE_MAX_ETHPORTS to 1024?
> Konstantin
>
>>
>> I think that RTE_MAX_ETHPORTS and  a default value(num of port ) should be different values.
>> Now dpdk limits the max num to RTE_MAX_ETHPORTS = 32 by default.
>> int
>> rte_eth_dev_is_valid_port(uint16_t port_id)
>> {
>> 	if (port_id >= RTE_MAX_ETHPORTS ||
>> 	    (rte_eth_devices[port_id].state != RTE_ETH_DEV_ATTACHED &&
>> 	     rte_eth_devices[port_id].state != RTE_ETH_DEV_DEFERRED))
>> 		return 0;
>> 	else
>> 		return 1;
>> }
>>
>> Thanks
>> Zhiyong.
>
  
Yang, Zhiyong Sept. 6, 2017, 8:52 a.m. UTC | #10
Hi, Hemant:
	This patch will be dropped from the patchset next version.
Thanks
Zhiyong

>> In testpmd,  the following function will be called to validate the port_id.

> >> So, It is necessary to modify the max port num RTE_MAX_ETHPORTS.

> >

> > There are quite a lot memory allocations (both static an dynamic)

> > inside DPDK libs and sample apps  that use RTE_MAX_ETHPORTS.

> > Increasing RTE_MAX_ETHPORTS would increase DPDK memory requirements

> quite significantly.

> 

> It is not a good idea to significantly increase the DPDK memory requirement.

> 

> Can you not make it with "CONFIG_RTE_MAX_ETHPORTS=1024" for your testing

> configs?

>
  

Patch

diff --git a/config/common_base b/config/common_base
index 5e97a08b6..dccc13e31 100644
--- a/config/common_base
+++ b/config/common_base
@@ -131,7 +131,7 @@  CONFIG_RTE_LIBRTE_KVARGS=y
 #
 CONFIG_RTE_LIBRTE_ETHER=y
 CONFIG_RTE_LIBRTE_ETHDEV_DEBUG=n
-CONFIG_RTE_MAX_ETHPORTS=32
+CONFIG_RTE_MAX_ETHPORTS=1024
 CONFIG_RTE_MAX_QUEUES_PER_PORT=1024
 CONFIG_RTE_LIBRTE_IEEE1588=n
 CONFIG_RTE_ETHDEV_QUEUE_STAT_CNTRS=16