patches for DPDK stable branches
 help / color / mirror / Atom feed
* [dpdk-stable] [PATCH] examples/vm_power: fix app to respect max cpus
@ 2018-10-31 11:50 David Hunt
  2018-11-04 20:49 ` [dpdk-stable] [dpdk-dev] " Thomas Monjalon
  0 siblings, 1 reply; 2+ messages in thread
From: David Hunt @ 2018-10-31 11:50 UTC (permalink / raw)
  To: dev, david.hunt; +Cc: lei.a.yao, stable

The vm_power_manager app was not respecting the POWER_MGR_MAX_CPUS
during initialisation, so if there were more CPUs than this value (64),
it would lead to buffer overruns of there were more then 64 cores in
the system.

Added in a check during init and un-init to only initialise up to
lcore_id 63.

This raises the question as to why not simply increase the value of
POWER_MGR_MAX_CPUS. Well, it's not that simple, as many of the APIs take
a uint64_t as a parameter for the core mask, and this will not work for
cores greater than 63. So some work needs to be done in the future to
remove this limitation. For now we'll fix the memory corruption.

Also, the patch that this fixes says "allow greater than 64 cores" but
that's not across the entire application, it's only for the out-of-band
monitoring. I'll add a notice for an API change in the next release to
clean this up, i.e. depricate any API calls that use masks.

Fixes: 6453b9284b64 ("examples/vm_power: allow greater than 64 cores")

Signed-off-by: David Hunt <david.hunt@intel.com>
---
 examples/vm_power_manager/power_manager.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/examples/vm_power_manager/power_manager.c b/examples/vm_power_manager/power_manager.c
index b7769c3c3..f9e8c0abd 100644
--- a/examples/vm_power_manager/power_manager.c
+++ b/examples/vm_power_manager/power_manager.c
@@ -95,6 +95,7 @@ power_manager_init(void)
 	unsigned int i, num_cpus = 0, num_freqs = 0;
 	int ret = 0;
 	struct core_info *ci;
+	unsigned int max_core_num;
 
 	rte_power_set_env(PM_ENV_ACPI_CPUFREQ);
 
@@ -105,7 +106,12 @@ power_manager_init(void)
 		return -1;
 	}
 
-	for (i = 0; i < ci->core_count; i++) {
+	if (ci->core_count > POWER_MGR_MAX_CPUS)
+		max_core_num = POWER_MGR_MAX_CPUS;
+	else
+		max_core_num = ci->core_count;
+
+	for (i = 0; i < max_core_num; i++) {
 		if (ci->cd[i].global_enabled_cpus) {
 			if (rte_power_init(i) < 0)
 				RTE_LOG(ERR, POWER_MANAGER,
@@ -165,6 +171,7 @@ power_manager_exit(void)
 	unsigned int i;
 	int ret = 0;
 	struct core_info *ci;
+	unsigned int max_core_num;
 
 	ci = get_core_info();
 	if (!ci) {
@@ -173,7 +180,12 @@ power_manager_exit(void)
 		return -1;
 	}
 
-	for (i = 0; i < ci->core_count; i++) {
+	if (ci->core_count > POWER_MGR_MAX_CPUS)
+		max_core_num = POWER_MGR_MAX_CPUS;
+	else
+		max_core_num = ci->core_count;
+
+	for (i = 0; i < max_core_num; i++) {
 		if (ci->cd[i].global_enabled_cpus) {
 			if (rte_power_exit(i) < 0) {
 				RTE_LOG(ERR, POWER_MANAGER, "Unable to shutdown power manager "
-- 
2.17.1

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH] examples/vm_power: fix app to respect max cpus
  2018-10-31 11:50 [dpdk-stable] [PATCH] examples/vm_power: fix app to respect max cpus David Hunt
@ 2018-11-04 20:49 ` Thomas Monjalon
  0 siblings, 0 replies; 2+ messages in thread
From: Thomas Monjalon @ 2018-11-04 20:49 UTC (permalink / raw)
  To: David Hunt; +Cc: dev, lei.a.yao, stable

31/10/2018 12:50, David Hunt:
> The vm_power_manager app was not respecting the POWER_MGR_MAX_CPUS
> during initialisation, so if there were more CPUs than this value (64),
> it would lead to buffer overruns of there were more then 64 cores in
> the system.
> 
> Added in a check during init and un-init to only initialise up to
> lcore_id 63.
> 
> This raises the question as to why not simply increase the value of
> POWER_MGR_MAX_CPUS. Well, it's not that simple, as many of the APIs take
> a uint64_t as a parameter for the core mask, and this will not work for
> cores greater than 63. So some work needs to be done in the future to
> remove this limitation. For now we'll fix the memory corruption.
> 
> Also, the patch that this fixes says "allow greater than 64 cores" but
> that's not across the entire application, it's only for the out-of-band
> monitoring. I'll add a notice for an API change in the next release to
> clean this up, i.e. depricate any API calls that use masks.
> 
> Fixes: 6453b9284b64 ("examples/vm_power: allow greater than 64 cores")
> 
> Signed-off-by: David Hunt <david.hunt@intel.com>

+ Cc: stable@dpdk.org

Applied, thanks

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2018-11-04 20:49 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-31 11:50 [dpdk-stable] [PATCH] examples/vm_power: fix app to respect max cpus David Hunt
2018-11-04 20:49 ` [dpdk-stable] [dpdk-dev] " Thomas Monjalon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).