[v2] app/testpmd: reduce memory consumption
Checks
Commit Message
Following [1], testpmd memory consumption has skyrocketted.
The rte_port structure has gotten quite fat.
struct rte_port {
[...]
struct rte_eth_rxconf rx_conf[65536]; /* 266280 3145728 */
/* --- cacheline 53312 boundary (3411968 bytes) was 40 bytes ago --- */
struct rte_eth_txconf tx_conf[65536]; /* 3412008 3670016 */
/* --- cacheline 110656 boundary (7081984 bytes) was 40 bytes ago --- */
[...]
/* size: 8654936, cachelines: 135234, members: 31 */
[...]
testpmd handles RTE_MAX_ETHPORTS ports (32 by default) which means that it
needs ~256MB just for this internal representation.
The reason is that a testpmd rte_port (the name is quite confusing, as
it is a local type) maintains configurations for all queues of a port.
But where you would expect testpmd to use RTE_MAX_QUEUES_PER_PORT as the
maximum queue count, the rte_port uses MAX_QUEUE_ID set to 64k.
Prefer the ethdev maximum value.
After this patch:
struct rte_port {
[...]
struct rte_eth_rxconf rx_conf[1025]; /* 8240 49200 */
/* --- cacheline 897 boundary (57408 bytes) was 32 bytes ago --- */
struct rte_eth_txconf tx_conf[1025]; /* 57440 57400 */
/* --- cacheline 1794 boundary (114816 bytes) was 24 bytes ago --- */
[...]
/* size: 139488, cachelines: 2180, members: 31 */
[...]
With this, we can ask for less memory in test-null.sh.
[1]: https://git.dpdk.org/dpdk/commit/?id=436b3a6b6e62
Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>
Acked-by: Thomas Monjalon <thomas@monjalon.net>
---
Changelog since v1:
- updated test-null.sh
---
app/test-pmd/testpmd.c | 6 +++---
app/test-pmd/testpmd.h | 16 +++++++---------
devtools/test-null.sh | 2 +-
3 files changed, 11 insertions(+), 13 deletions(-)
Comments
On 11/22/2019 10:43 AM, David Marchand wrote:
> diff --git a/devtools/test-null.sh b/devtools/test-null.sh
> index 9f9a459f76..6e5b1ad529 100755
> --- a/devtools/test-null.sh
> +++ b/devtools/test-null.sh
> @@ -25,6 +25,6 @@ else
> fi
>
> (sleep 1 && echo stop) |
> -$testpmd -c $coremask --no-huge -m 150 \
> +$testpmd -c $coremask --no-huge -m 20 \
> $libs --vdev net_null1 --vdev net_null2 $eal_options -- \
> --no-mlockall --total-num-mbufs=2048 $testpmd_options -ia
What do you think to separate this part, and go with first version of the patchset?
And I am not sure if we should update at all, what is the benefit?
Also script fails after update, because of the additional physical devices and
their memory requirement, it is possible to make it run with additional testpmd
argument but fails by default.
22/11/2019 13:24, Ferruh Yigit:
> On 11/22/2019 10:43 AM, David Marchand wrote:
> > diff --git a/devtools/test-null.sh b/devtools/test-null.sh
> > index 9f9a459f76..6e5b1ad529 100755
> > --- a/devtools/test-null.sh
> > +++ b/devtools/test-null.sh
> > @@ -25,6 +25,6 @@ else
> > fi
> >
> > (sleep 1 && echo stop) |
> > -$testpmd -c $coremask --no-huge -m 150 \
> > +$testpmd -c $coremask --no-huge -m 20 \
> > $libs --vdev net_null1 --vdev net_null2 $eal_options -- \
> > --no-mlockall --total-num-mbufs=2048 $testpmd_options -ia
>
> What do you think to separate this part, and go with first version of the patchset?
>
> And I am not sure if we should update at all, what is the benefit?
The benefit it to test that there is no anormal memory needs.
> Also script fails after update, because of the additional physical devices and
> their memory requirement, it is possible to make it run with additional testpmd
> argument but fails by default.
This test is supposed to be for null devices only.
Why are you trying to add physical devices?
Oh wait, we miss an option -w 0 to be in whitelist mode?
On 11/22/2019 1:12 PM, Thomas Monjalon wrote:
> 22/11/2019 13:24, Ferruh Yigit:
>> On 11/22/2019 10:43 AM, David Marchand wrote:
>>> diff --git a/devtools/test-null.sh b/devtools/test-null.sh
>>> index 9f9a459f76..6e5b1ad529 100755
>>> --- a/devtools/test-null.sh
>>> +++ b/devtools/test-null.sh
>>> @@ -25,6 +25,6 @@ else
>>> fi
>>>
>>> (sleep 1 && echo stop) |
>>> -$testpmd -c $coremask --no-huge -m 150 \
>>> +$testpmd -c $coremask --no-huge -m 20 \
>>> $libs --vdev net_null1 --vdev net_null2 $eal_options -- \
>>> --no-mlockall --total-num-mbufs=2048 $testpmd_options -ia
>>
>> What do you think to separate this part, and go with first version of the patchset?
>>
>> And I am not sure if we should update at all, what is the benefit?
Fair enough.
>
> The benefit it to test that there is no anormal memory needs.
>
>> Also script fails after update, because of the additional physical devices and
>> their memory requirement, it is possible to make it run with additional testpmd
>> argument but fails by default.
>
> This test is supposed to be for null devices only.
> Why are you trying to add physical devices?
> Oh wait, we miss an option -w 0 to be in whitelist mode?
>
I am not trying to add physical devices, they are already there in my
environment, yes adding "-w" can solve it.
On 11/22/2019 10:43 AM, David Marchand wrote:
> Following [1], testpmd memory consumption has skyrocketted.
> The rte_port structure has gotten quite fat.
>
> struct rte_port {
> [...]
> struct rte_eth_rxconf rx_conf[65536]; /* 266280 3145728 */
> /* --- cacheline 53312 boundary (3411968 bytes) was 40 bytes ago --- */
> struct rte_eth_txconf tx_conf[65536]; /* 3412008 3670016 */
> /* --- cacheline 110656 boundary (7081984 bytes) was 40 bytes ago --- */
> [...]
> /* size: 8654936, cachelines: 135234, members: 31 */
> [...]
>
> testpmd handles RTE_MAX_ETHPORTS ports (32 by default) which means that it
> needs ~256MB just for this internal representation.
>
> The reason is that a testpmd rte_port (the name is quite confusing, as
> it is a local type) maintains configurations for all queues of a port.
> But where you would expect testpmd to use RTE_MAX_QUEUES_PER_PORT as the
> maximum queue count, the rte_port uses MAX_QUEUE_ID set to 64k.
>
> Prefer the ethdev maximum value.
>
> After this patch:
> struct rte_port {
> [...]
> struct rte_eth_rxconf rx_conf[1025]; /* 8240 49200 */
> /* --- cacheline 897 boundary (57408 bytes) was 32 bytes ago --- */
> struct rte_eth_txconf tx_conf[1025]; /* 57440 57400 */
> /* --- cacheline 1794 boundary (114816 bytes) was 24 bytes ago --- */
> [...]
> /* size: 139488, cachelines: 2180, members: 31 */
> [...]
>
> With this, we can ask for less memory in test-null.sh.
>
> [1]: https://git.dpdk.org/dpdk/commit/?id=436b3a6b6e62
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>
> Acked-by: Thomas Monjalon <thomas@monjalon.net>
Applied to dpdk-next-net/master, thanks.
@@ -979,7 +979,7 @@ check_socket_id(const unsigned int socket_id)
queueid_t
get_allowed_max_nb_rxq(portid_t *pid)
{
- queueid_t allowed_max_rxq = MAX_QUEUE_ID;
+ queueid_t allowed_max_rxq = RTE_MAX_QUEUES_PER_PORT;
bool max_rxq_valid = false;
portid_t pi;
struct rte_eth_dev_info dev_info;
@@ -1029,7 +1029,7 @@ check_nb_rxq(queueid_t rxq)
queueid_t
get_allowed_max_nb_txq(portid_t *pid)
{
- queueid_t allowed_max_txq = MAX_QUEUE_ID;
+ queueid_t allowed_max_txq = RTE_MAX_QUEUES_PER_PORT;
bool max_txq_valid = false;
portid_t pi;
struct rte_eth_dev_info dev_info;
@@ -1079,7 +1079,7 @@ check_nb_txq(queueid_t txq)
queueid_t
get_allowed_max_nb_hairpinq(portid_t *pid)
{
- queueid_t allowed_max_hairpinq = MAX_QUEUE_ID;
+ queueid_t allowed_max_hairpinq = RTE_MAX_QUEUES_PER_PORT;
portid_t pi;
struct rte_eth_hairpin_cap cap;
@@ -58,8 +58,6 @@ typedef uint16_t portid_t;
typedef uint16_t queueid_t;
typedef uint16_t streamid_t;
-#define MAX_QUEUE_ID ((1 << (sizeof(queueid_t) * 8)) - 1)
-
#if defined RTE_LIBRTE_PMD_SOFTNIC
#define SOFTNIC 1
#else
@@ -179,22 +177,22 @@ struct rte_port {
uint8_t need_reconfig_queues; /**< need reconfiguring queues or not */
uint8_t rss_flag; /**< enable rss or not */
uint8_t dcb_flag; /**< enable dcb */
- uint16_t nb_rx_desc[MAX_QUEUE_ID+1]; /**< per queue rx desc number */
- uint16_t nb_tx_desc[MAX_QUEUE_ID+1]; /**< per queue tx desc number */
- struct rte_eth_rxconf rx_conf[MAX_QUEUE_ID+1]; /**< per queue rx configuration */
- struct rte_eth_txconf tx_conf[MAX_QUEUE_ID+1]; /**< per queue tx configuration */
+ uint16_t nb_rx_desc[RTE_MAX_QUEUES_PER_PORT+1]; /**< per queue rx desc number */
+ uint16_t nb_tx_desc[RTE_MAX_QUEUES_PER_PORT+1]; /**< per queue tx desc number */
+ struct rte_eth_rxconf rx_conf[RTE_MAX_QUEUES_PER_PORT+1]; /**< per queue rx configuration */
+ struct rte_eth_txconf tx_conf[RTE_MAX_QUEUES_PER_PORT+1]; /**< per queue tx configuration */
struct rte_ether_addr *mc_addr_pool; /**< pool of multicast addrs */
uint32_t mc_addr_nb; /**< nb. of addr. in mc_addr_pool */
uint8_t slave_flag; /**< bonding slave port */
struct port_flow *flow_list; /**< Associated flows. */
- const struct rte_eth_rxtx_callback *rx_dump_cb[MAX_QUEUE_ID+1];
- const struct rte_eth_rxtx_callback *tx_dump_cb[MAX_QUEUE_ID+1];
+ const struct rte_eth_rxtx_callback *rx_dump_cb[RTE_MAX_QUEUES_PER_PORT+1];
+ const struct rte_eth_rxtx_callback *tx_dump_cb[RTE_MAX_QUEUES_PER_PORT+1];
#ifdef SOFTNIC
struct softnic_port softport; /**< softnic params */
#endif
/**< metadata value to insert in Tx packets. */
uint32_t tx_metadata;
- const struct rte_eth_rxtx_callback *tx_set_md_cb[MAX_QUEUE_ID+1];
+ const struct rte_eth_rxtx_callback *tx_set_md_cb[RTE_MAX_QUEUES_PER_PORT+1];
};
/**
@@ -25,6 +25,6 @@ else
fi
(sleep 1 && echo stop) |
-$testpmd -c $coremask --no-huge -m 150 \
+$testpmd -c $coremask --no-huge -m 20 \
$libs --vdev net_null1 --vdev net_null2 $eal_options -- \
--no-mlockall --total-num-mbufs=2048 $testpmd_options -ia