Re: [dpdk-dev] [PATCH v2] app/test: enhance power manager unit tests

2018-04-24 Thread Pattan, Reshma
Hi Thomas,

I sent it few mins back. Can you check  and apply

Thanks,
Reshma

> -Original Message-
> From: Thomas Monjalon [mailto:tho...@monjalon.net]
> Sent: Tuesday, April 24, 2018 11:34 PM
> To: Pattan, Reshma 
> Cc: dev@dpdk.org; Hunt, David ; Richardson, Bruce
> ; Parthasarathy, JananeeX M
> 
> Subject: Re: [dpdk-dev] [PATCH v2] app/test: enhance power manager unit
> tests
> 
> 24/04/2018 14:51, Pattan, Reshma:
> > From: Hunt, David
> > > On 24/4/2018 12:23 PM, Pattan, Reshma wrote:
> > > > From: Richardson, Bruce
> > > >> On Mon, Apr 23, 2018 at 11:04:27PM +0200, Thomas Monjalon wrote:
> > > >>> 11/04/2018 16:14, Reshma Pattan:
> > > >>>> Unit Testcases are added for power_acpi_cpu_freq,
> > > power_kvm_vm_test
> > > >>>> to improve coverage
> > > >>>>
> > > >>>> Signed-off-by: Jananee Parthasarathy
> > > >>>> 
> > > >>>> Acked-by: David Hunt 
> > > >>> Applied, thanks
> > > >>>
> > > >> Sadly, this patch seems to break shared library builds. If you
> > > >> try doing "make test-build" with shared libraries on it will
> > > >> fail, or if you do a meson build using shared libraries you will get 
> > > >> the
> same result.
> > > >>
> > > >> The root cause is that the function guest_channel_host_connect()
> > > >> is a private function and so is not listed in the shared library
> > > >> map file, preventing the test app from linking.
> > > >>
> > > > Any action from my side required? Let me know.
> > >
> > > Reshma,
> > >  Looking at this, I think this particular unit test needs to be
> > > removed. The way it is at the moment, it's "faking" the connect,
> > > then any commands that are sent to the dummy host are only really to
> > > test to see if the API breaks, which is going to be captured by
> > > compilation tests anyway. I don't see the value of this unit test
> > > unless you have the full host setup underneath is, in which case it's no
> longer a unit test.
> > > Also, we don't want to make these functions public, as they are only
> > > of use to the library internally, and there is no use for them
> > > publicly (unless a guest wants to fake a connection to a non-existent 
> > > host).
> > >
> > > What do you think?
> >
> > Fine, we are reverting the changes and will send the patch soon.
> 
> Where is the patch?
> I will revert it myself.
> 



Re: [dpdk-dev] [PATCH v2] app/test: enhance power manager unit tests

2018-04-24 Thread Thomas Monjalon
24/04/2018 14:51, Pattan, Reshma:
> From: Hunt, David
> > On 24/4/2018 12:23 PM, Pattan, Reshma wrote:
> > > From: Richardson, Bruce
> > >> On Mon, Apr 23, 2018 at 11:04:27PM +0200, Thomas Monjalon wrote:
> > >>> 11/04/2018 16:14, Reshma Pattan:
> >  Unit Testcases are added for power_acpi_cpu_freq,
> > power_kvm_vm_test
> >  to improve coverage
> > 
> >  Signed-off-by: Jananee Parthasarathy
> >  
> >  Acked-by: David Hunt 
> > >>> Applied, thanks
> > >>>
> > >> Sadly, this patch seems to break shared library builds. If you try
> > >> doing "make test-build" with shared libraries on it will fail, or if
> > >> you do a meson build using shared libraries you will get the same result.
> > >>
> > >> The root cause is that the function guest_channel_host_connect() is a
> > >> private function and so is not listed in the shared library map file,
> > >> preventing the test app from linking.
> > >>
> > > Any action from my side required? Let me know.
> > 
> > Reshma,
> >  Looking at this, I think this particular unit test needs to be 
> > removed. The
> > way it is at the moment, it's "faking" the connect, then any commands that
> > are sent to the dummy host are only really to test to see if the API breaks,
> > which is going to be captured by compilation tests anyway. I don't see the
> > value of this unit test unless you have the full host setup underneath is, 
> > in
> > which case it's no longer a unit test.
> > Also, we don't want to make these functions public, as they are only of use 
> > to
> > the library internally, and there is no use for them publicly (unless a 
> > guest
> > wants to fake a connection to a non-existent host).
> > 
> > What do you think?
> 
> Fine, we are reverting the changes and will send the patch soon.

Where is the patch?
I will revert it myself.




Re: [dpdk-dev] [PATCH v2] app/test: enhance power manager unit tests

2018-04-24 Thread Pattan, Reshma
Hi,

> -Original Message-
> From: Hunt, David
> Sent: Tuesday, April 24, 2018 1:10 PM
> To: Pattan, Reshma ; Richardson, Bruce
> ; Thomas Monjalon 
> Cc: Parthasarathy, JananeeX M ;
> dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2] app/test: enhance power manager unit
> tests
> 
> 
> On 24/4/2018 12:23 PM, Pattan, Reshma wrote:
> > Hi,
> >
> >> -Original Message-
> >> From: Richardson, Bruce
> >> Sent: Tuesday, April 24, 2018 11:59 AM
> >> To: Thomas Monjalon 
> >> Cc: Parthasarathy, JananeeX M ;
> >> dev@dpdk.org; Pattan, Reshma ; Hunt, David
> >> 
> >> Subject: Re: [dpdk-dev] [PATCH v2] app/test: enhance power manager
> >> unit tests
> >>
> >> On Mon, Apr 23, 2018 at 11:04:27PM +0200, Thomas Monjalon wrote:
> >>> 11/04/2018 16:14, Reshma Pattan:
> >>>> Unit Testcases are added for power_acpi_cpu_freq,
> power_kvm_vm_test
> >>>> to improve coverage
> >>>>
> >>>> Signed-off-by: Jananee Parthasarathy
> >>>> 
> >>>> Acked-by: David Hunt 
> >>> Applied, thanks
> >>>
> >> Sadly, this patch seems to break shared library builds. If you try
> >> doing "make test-build" with shared libraries on it will fail, or if
> >> you do a meson build using shared libraries you will get the same result.
> >>
> >> The root cause is that the function guest_channel_host_connect() is a
> >> private function and so is not listed in the shared library map file,
> >> preventing the test app from linking.
> >>
> > Any action from my side required? Let me know.
> 
> Reshma,
>      Looking at this, I think this particular unit test needs to be removed. 
> The
> way it is at the moment, it's "faking" the connect, then any commands that
> are sent to the dummy host are only really to test to see if the API breaks,
> which is going to be captured by compilation tests anyway. I don't see the
> value of this unit test unless you have the full host setup underneath is, in
> which case it's no longer a unit test.
> Also, we don't want to make these functions public, as they are only of use to
> the library internally, and there is no use for them publicly (unless a guest
> wants to fake a connection to a non-existent host).
> 
> What do you think?

Fine, we are reverting the changes and will send the patch soon.

Thanks,
Reshma




Re: [dpdk-dev] [PATCH v2] app/test: enhance power manager unit tests

2018-04-24 Thread Hunt, David


On 24/4/2018 12:23 PM, Pattan, Reshma wrote:

Hi,


-Original Message-
From: Richardson, Bruce
Sent: Tuesday, April 24, 2018 11:59 AM
To: Thomas Monjalon 
Cc: Parthasarathy, JananeeX M ;
dev@dpdk.org; Pattan, Reshma ; Hunt, David

Subject: Re: [dpdk-dev] [PATCH v2] app/test: enhance power manager unit
tests

On Mon, Apr 23, 2018 at 11:04:27PM +0200, Thomas Monjalon wrote:

11/04/2018 16:14, Reshma Pattan:

Unit Testcases are added for power_acpi_cpu_freq, power_kvm_vm_test
to improve coverage

Signed-off-by: Jananee Parthasarathy

Acked-by: David Hunt 

Applied, thanks


Sadly, this patch seems to break shared library builds. If you try doing "make
test-build" with shared libraries on it will fail, or if you do a meson build 
using
shared libraries you will get the same result.

The root cause is that the function guest_channel_host_connect() is a private
function and so is not listed in the shared library map file, preventing the 
test
app from linking.


Any action from my side required? Let me know.


Reshma,
    Looking at this, I think this particular unit test needs to be 
removed. The way it is at the moment, it's "faking" the connect, then 
any commands that are sent to the dummy host are only really to test to 
see if the API breaks, which is going to be captured by compilation 
tests anyway. I don't see the value of this unit test unless you have 
the full host setup underneath is, in which case it's no longer a unit 
test.
Also, we don't want to make these functions public, as they are only of 
use to the library internally, and there is no use for them publicly 
(unless a guest wants to fake a connection to a non-existent host).


What do you think?
Dave.





Re: [dpdk-dev] [PATCH v2] app/test: enhance power manager unit tests

2018-04-24 Thread Pattan, Reshma
Hi,

> -Original Message-
> From: Richardson, Bruce
> Sent: Tuesday, April 24, 2018 11:59 AM
> To: Thomas Monjalon 
> Cc: Parthasarathy, JananeeX M ;
> dev@dpdk.org; Pattan, Reshma ; Hunt, David
> 
> Subject: Re: [dpdk-dev] [PATCH v2] app/test: enhance power manager unit
> tests
> 
> On Mon, Apr 23, 2018 at 11:04:27PM +0200, Thomas Monjalon wrote:
> > 11/04/2018 16:14, Reshma Pattan:
> > > Unit Testcases are added for power_acpi_cpu_freq, power_kvm_vm_test
> > > to improve coverage
> > >
> > > Signed-off-by: Jananee Parthasarathy
> > > 
> > > Acked-by: David Hunt 
> >
> > Applied, thanks
> >
> Sadly, this patch seems to break shared library builds. If you try doing "make
> test-build" with shared libraries on it will fail, or if you do a meson build 
> using
> shared libraries you will get the same result.
> 
> The root cause is that the function guest_channel_host_connect() is a private
> function and so is not listed in the shared library map file, preventing the 
> test
> app from linking.
> 

Any action from my side required? Let me know.

> Regards,
> /Bruce


Re: [dpdk-dev] [PATCH v2] app/test: enhance power manager unit tests

2018-04-24 Thread Bruce Richardson
On Mon, Apr 23, 2018 at 11:04:27PM +0200, Thomas Monjalon wrote:
> 11/04/2018 16:14, Reshma Pattan:
> > Unit Testcases are added for power_acpi_cpu_freq,
> > power_kvm_vm_test to improve coverage
> > 
> > Signed-off-by: Jananee Parthasarathy 
> > Acked-by: David Hunt 
> 
> Applied, thanks
> 
Sadly, this patch seems to break shared library builds. If you try doing
"make test-build" with shared libraries on it will fail, or if you do a
meson build using shared libraries you will get the same result.

The root cause is that the function guest_channel_host_connect() is a
private function and so is not listed in the shared library map file,
preventing the test app from linking.

Regards,
/Bruce


Re: [dpdk-dev] [PATCH v2] app/test: enhance power manager unit tests

2018-04-23 Thread Thomas Monjalon
11/04/2018 16:14, Reshma Pattan:
> Unit Testcases are added for power_acpi_cpu_freq,
> power_kvm_vm_test to improve coverage
> 
> Signed-off-by: Jananee Parthasarathy 
> Acked-by: David Hunt 

Applied, thanks





[dpdk-dev] [PATCH v2] app/test: enhance power manager unit tests

2018-04-11 Thread Reshma Pattan
Unit Testcases are added for power_acpi_cpu_freq,
power_kvm_vm_test to improve coverage

Signed-off-by: Jananee Parthasarathy 
Acked-by: David Hunt 
---
V3: removed unnecessary extern funtion prototypes.
---
 test/test/test_power_acpi_cpufreq.c |  2 +-
 test/test/test_power_kvm_vm.c   | 60 +
 2 files changed, 55 insertions(+), 7 deletions(-)

diff --git a/test/test/test_power_acpi_cpufreq.c 
b/test/test/test_power_acpi_cpufreq.c
index 3bfd033..8da2dcc 100644
--- a/test/test/test_power_acpi_cpufreq.c
+++ b/test/test/test_power_acpi_cpufreq.c
@@ -27,7 +27,7 @@
 #define TEST_POWER_FREQS_NUM_MAX ((unsigned)RTE_MAX_LCORE_FREQS)
 
 #define TEST_POWER_SYSFILE_CUR_FREQ \
-   "/sys/devices/system/cpu/cpu%u/cpufreq/scaling_cur_freq"
+   "/sys/devices/system/cpu/cpu%u/cpufreq/cpuinfo_cur_freq"
 
 static uint32_t total_freq_num;
 static uint32_t freqs[TEST_POWER_FREQS_NUM_MAX];
diff --git a/test/test/test_power_kvm_vm.c b/test/test/test_power_kvm_vm.c
index 91b31c4..2ac7491 100644
--- a/test/test/test_power_kvm_vm.c
+++ b/test/test/test_power_kvm_vm.c
@@ -25,12 +25,17 @@
 #define TEST_POWER_VM_LCORE_ID0U
 #define TEST_POWER_VM_LCORE_OUT_OF_BOUNDS (RTE_MAX_LCORE+1)
 #define TEST_POWER_VM_LCORE_INVALID   1U
+#define TEMP_POWER_MANAGER_FILE_PATH  "/tmp/testpm"
+
+int guest_channel_host_connect(const char *path, unsigned int lcore_id);
 
 static int
 test_power_kvm_vm(void)
 {
int ret;
enum power_management_env env;
+   char fPath[PATH_MAX];
+   FILE *fPtr = NULL;
 
ret = rte_power_set_env(PM_ENV_KVM_VM);
if (ret != 0) {
@@ -95,12 +100,31 @@
/* Test initialisation of a valid lcore */
ret = rte_power_init(TEST_POWER_VM_LCORE_ID);
if (ret < 0) {
-   printf("Cannot initialise power management for lcore %u, this "
-   "may occur if environment is not configured "
-   "correctly(KVM VM) or operating in another 
valid "
-   "Power management environment\n", 
TEST_POWER_VM_LCORE_ID);
-   rte_power_unset_env();
-   return -1;
+   printf("rte_power_init failed as expected in host\n");
+   /* This test would be successful when run on VM,
+* in order to run in Host itself, temporary file path
+* is created and same is used for further communication
+*/
+
+   snprintf(fPath, PATH_MAX, "%s.%u",
+   TEMP_POWER_MANAGER_FILE_PATH, TEST_POWER_VM_LCORE_ID);
+   fPtr = fopen(fPath, "w");
+   if (fPtr == NULL) {
+   printf(" Unable to create file\n");
+   rte_power_unset_env();
+   return -1;
+   }
+   ret = guest_channel_host_connect(TEMP_POWER_MANAGER_FILE_PATH,
+   TEST_POWER_VM_LCORE_ID);
+   if (ret == 0)
+   printf("guest_channel_host_connect successful\n");
+   else {
+   printf("guest_channel_host_connect failed\n");
+   rte_power_unset_env();
+   fclose(fPtr);
+   remove(fPath);
+   return -1;
+   }
}
 
/* Test initialisation of previously initialised lcore */
@@ -175,6 +199,22 @@
goto fail_all;
}
 
+   /* Test KVM_VM Enable Turbo of valid core */
+   ret = rte_power_freq_enable_turbo(TEST_POWER_VM_LCORE_ID);
+   if (ret == -1) {
+   printf("rte_power_freq_enable_turbo failed on valid lcore"
+   "%u\n", TEST_POWER_VM_LCORE_ID);
+   goto fail_all;
+   }
+
+   /* Test KVM_VM Disable Turbo of valid core */
+   ret = rte_power_freq_disable_turbo(TEST_POWER_VM_LCORE_ID);
+   if (ret == -1) {
+   printf("rte_power_freq_disable_turbo failed on valid lcore"
+   "%u\n", TEST_POWER_VM_LCORE_ID);
+   goto fail_all;
+   }
+
/* Test frequency up of valid lcore */
ret = rte_power_freq_up(TEST_POWER_VM_LCORE_ID);
if (ret != 1) {
@@ -274,10 +314,18 @@
return -1;
}
rte_power_unset_env();
+   if (fPtr != NULL) {
+   fclose(fPtr);
+   remove(fPath);
+   }
return 0;
 fail_all:
rte_power_exit(TEST_POWER_VM_LCORE_ID);
rte_power_unset_env();
+   if (fPtr != NULL) {
+   fclose(fPtr);
+   remove(fPath);
+   }
return -1;
 }
 #endif
-- 
1.7.12.2