* [dpdk-dev] [PATCH] examples/power: fix wrong core_id with JSON cmds
@ 2019-01-07 11:39 David Hunt
2019-01-08 2:02 ` Yao, Lei A
0 siblings, 1 reply; 5+ messages in thread
From: David Hunt @ 2019-01-07 11:39 UTC (permalink / raw)
To: dev; +Cc: david.hunt, lei.a.yao
This patch fixes a bug introduced in the 64-core limtation
enhancement where the core_id is inadvertently converted from
virtual to physical even though it may already be a physical
core_id.
We should be using the core_type field, and only converting via
hypervisor when core_type is set to CORE_TYPE_VIRTUAL
Fixes: 5776b7a371d1 ("examples/power: allow VM to use lcores over 63")
Signed-off-by: David Hunt <david.hunt@intel.com>
---
examples/vm_power_manager/channel_monitor.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/examples/vm_power_manager/channel_monitor.c b/examples/vm_power_manager/channel_monitor.c
index 85622e7cb..1a3a0fa76 100644
--- a/examples/vm_power_manager/channel_monitor.c
+++ b/examples/vm_power_manager/channel_monitor.c
@@ -640,7 +640,10 @@ process_request(struct channel_packet *pkt, struct channel_info *chan_info)
if (pkt->command == CPU_POWER) {
unsigned int core_num;
- core_num = get_pcpu(chan_info, pkt->resource_id);
+ if (pkt->core_type == CORE_TYPE_VIRTUAL)
+ core_num = get_pcpu(chan_info, pkt->resource_id);
+ else
+ core_num = pkt->resource_id;
switch (pkt->unit) {
case(CPU_POWER_SCALE_MIN):
--
2.17.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH] examples/power: fix wrong core_id with JSON cmds
2019-01-07 11:39 [dpdk-dev] [PATCH] examples/power: fix wrong core_id with JSON cmds David Hunt
@ 2019-01-08 2:02 ` Yao, Lei A
2019-01-08 9:20 ` Hunt, David
0 siblings, 1 reply; 5+ messages in thread
From: Yao, Lei A @ 2019-01-08 2:02 UTC (permalink / raw)
To: Hunt, David, dev
> -----Original Message-----
> From: Hunt, David
> Sent: Monday, January 7, 2019 7:39 PM
> To: dev@dpdk.org
> Cc: Hunt, David <david.hunt@intel.com>; Yao, Lei A <lei.a.yao@intel.com>
> Subject: [PATCH] examples/power: fix wrong core_id with JSON cmds
>
> This patch fixes a bug introduced in the 64-core limtation
> enhancement where the core_id is inadvertently converted from
> virtual to physical even though it may already be a physical
> core_id.
>
> We should be using the core_type field, and only converting via
> hypervisor when core_type is set to CORE_TYPE_VIRTUAL
>
> Fixes: 5776b7a371d1 ("examples/power: allow VM to use lcores over 63")
>
> Signed-off-by: David Hunt <david.hunt@intel.com>
> ---
> examples/vm_power_manager/channel_monitor.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/examples/vm_power_manager/channel_monitor.c
> b/examples/vm_power_manager/channel_monitor.c
> index 85622e7cb..1a3a0fa76 100644
> --- a/examples/vm_power_manager/channel_monitor.c
> +++ b/examples/vm_power_manager/channel_monitor.c
> @@ -640,7 +640,10 @@ process_request(struct channel_packet *pkt, struct
> channel_info *chan_info)
> if (pkt->command == CPU_POWER) {
> unsigned int core_num;
>
> - core_num = get_pcpu(chan_info, pkt->resource_id);
> + if (pkt->core_type == CORE_TYPE_VIRTUAL)
> + core_num = get_pcpu(chan_info, pkt->resource_id);
Hi, Dave
Now in DPDK code, only command send from VM(guest_cli sample) will set the
pkt-<core_type to VIRTUAL, if we send JSON command with resource_id, seems
we always won't hit this branch. Because parse_json_to_pkt() will always set the
core_type to CORE_TYPE_PHYSICAL.
If resource_id in instruction format JSON file will always be treated as Pcpu, it's the
same as core_list behavior.
> + else
> + core_num = pkt->resource_id;
>
> switch (pkt->unit) {
> case(CPU_POWER_SCALE_MIN):
> --
> 2.17.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH] examples/power: fix wrong core_id with JSON cmds
2019-01-08 2:02 ` Yao, Lei A
@ 2019-01-08 9:20 ` Hunt, David
2019-01-08 12:47 ` Yao, Lei A
0 siblings, 1 reply; 5+ messages in thread
From: Hunt, David @ 2019-01-08 9:20 UTC (permalink / raw)
To: Yao, Lei A, dev
Hi Lei,
On 8/1/2019 2:02 AM, Yao, Lei A wrote:
>
>> -----Original Message-----
>> From: Hunt, David
>> Sent: Monday, January 7, 2019 7:39 PM
>> To: dev@dpdk.org
>> Cc: Hunt, David <david.hunt@intel.com>; Yao, Lei A <lei.a.yao@intel.com>
>> Subject: [PATCH] examples/power: fix wrong core_id with JSON cmds
>>
>> This patch fixes a bug introduced in the 64-core limtation
>> enhancement where the core_id is inadvertently converted from
>> virtual to physical even though it may already be a physical
>> core_id.
>>
>> We should be using the core_type field, and only converting via
>> hypervisor when core_type is set to CORE_TYPE_VIRTUAL
>>
>> Fixes: 5776b7a371d1 ("examples/power: allow VM to use lcores over 63")
>>
>> Signed-off-by: David Hunt <david.hunt@intel.com>
>> ---
>> examples/vm_power_manager/channel_monitor.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/examples/vm_power_manager/channel_monitor.c
>> b/examples/vm_power_manager/channel_monitor.c
>> index 85622e7cb..1a3a0fa76 100644
>> --- a/examples/vm_power_manager/channel_monitor.c
>> +++ b/examples/vm_power_manager/channel_monitor.c
>> @@ -640,7 +640,10 @@ process_request(struct channel_packet *pkt, struct
>> channel_info *chan_info)
>> if (pkt->command == CPU_POWER) {
>> unsigned int core_num;
>>
>> - core_num = get_pcpu(chan_info, pkt->resource_id);
>> + if (pkt->core_type == CORE_TYPE_VIRTUAL)
>> + core_num = get_pcpu(chan_info, pkt->resource_id);
> Hi, Dave
>
> Now in DPDK code, only command send from VM(guest_cli sample) will set the
> pkt-<core_type to VIRTUAL, if we send JSON command with resource_id, seems
> we always won't hit this branch. Because parse_json_to_pkt() will always set the
> core_type to CORE_TYPE_PHYSICAL.
> If resource_id in instruction format JSON file will always be treated as Pcpu, it's the
> same as core_list behavior.
Yes, that's correct. But I believe it's OK for the moment.
Currently the only way for the guest app to send commands and policies
to the host is via the virtio-serial interface, which takes a different
code path. There is no
way currently for the guest app to send a JSON string to the host, so
that code path will
never be hit. However, now that the JSON functionality is in the host,
we plan to add
that same functionality to the guest, and when we do that we will ensure
that core_type
is handled appropriately.
Regards,
Dave.
>> + else
>> + core_num = pkt->resource_id;
>>
>> switch (pkt->unit) {
>> case(CPU_POWER_SCALE_MIN):
>> --
>> 2.17.1
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH] examples/power: fix wrong core_id with JSON cmds
2019-01-08 9:20 ` Hunt, David
@ 2019-01-08 12:47 ` Yao, Lei A
2019-01-14 23:15 ` Thomas Monjalon
0 siblings, 1 reply; 5+ messages in thread
From: Yao, Lei A @ 2019-01-08 12:47 UTC (permalink / raw)
To: Hunt, David, dev
> -----Original Message-----
> From: Hunt, David
> Sent: Tuesday, January 8, 2019 5:20 PM
> To: Yao, Lei A <lei.a.yao@intel.com>; dev@dpdk.org
> Subject: Re: [PATCH] examples/power: fix wrong core_id with JSON cmds
>
> Hi Lei,
>
> On 8/1/2019 2:02 AM, Yao, Lei A wrote:
> >
> >> -----Original Message-----
> >> From: Hunt, David
> >> Sent: Monday, January 7, 2019 7:39 PM
> >> To: dev@dpdk.org
> >> Cc: Hunt, David <david.hunt@intel.com>; Yao, Lei A <lei.a.yao@intel.com>
> >> Subject: [PATCH] examples/power: fix wrong core_id with JSON cmds
> >>
> >> This patch fixes a bug introduced in the 64-core limtation
> >> enhancement where the core_id is inadvertently converted from
> >> virtual to physical even though it may already be a physical
> >> core_id.
> >>
> >> We should be using the core_type field, and only converting via
> >> hypervisor when core_type is set to CORE_TYPE_VIRTUAL
> >>
> >> Fixes: 5776b7a371d1 ("examples/power: allow VM to use lcores over 63")
> >>
> >> Signed-off-by: David Hunt <david.hunt@intel.com>
Reviewed-by: Lei Yao<lei.a.yao@intel.com>
Tested-by: Lei Yao<lei.a.yao@intel.com>
> >> ---
> >> examples/vm_power_manager/channel_monitor.c | 5 ++++-
> >> 1 file changed, 4 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/examples/vm_power_manager/channel_monitor.c
> >> b/examples/vm_power_manager/channel_monitor.c
> >> index 85622e7cb..1a3a0fa76 100644
> >> --- a/examples/vm_power_manager/channel_monitor.c
> >> +++ b/examples/vm_power_manager/channel_monitor.c
> >> @@ -640,7 +640,10 @@ process_request(struct channel_packet *pkt,
> struct
> >> channel_info *chan_info)
> >> if (pkt->command == CPU_POWER) {
> >> unsigned int core_num;
> >>
> >> - core_num = get_pcpu(chan_info, pkt->resource_id);
> >> + if (pkt->core_type == CORE_TYPE_VIRTUAL)
> >> + core_num = get_pcpu(chan_info, pkt->resource_id);
> > Hi, Dave
> >
> > Now in DPDK code, only command send from VM(guest_cli sample) will set
> the
> > pkt-<core_type to VIRTUAL, if we send JSON command with resource_id,
> seems
> > we always won't hit this branch. Because parse_json_to_pkt() will always
> set the
> > core_type to CORE_TYPE_PHYSICAL.
> > If resource_id in instruction format JSON file will always be treated as Pcpu,
> it's the
> > same as core_list behavior.
>
>
> Yes, that's correct. But I believe it's OK for the moment.
>
> Currently the only way for the guest app to send commands and policies
> to the host is via the virtio-serial interface, which takes a different
> code path. There is no
> way currently for the guest app to send a JSON string to the host, so
> that code path will
> never be hit. However, now that the JSON functionality is in the host,
> we plan to add
> that same functionality to the guest, and when we do that we will ensure
> that core_type
> is handled appropriately.
>
> Regards,
> Dave.
>
> >> + else
> >> + core_num = pkt->resource_id;
> >>
> >> switch (pkt->unit) {
> >> case(CPU_POWER_SCALE_MIN):
> >> --
> >> 2.17.1
> >
> >
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH] examples/power: fix wrong core_id with JSON cmds
2019-01-08 12:47 ` Yao, Lei A
@ 2019-01-14 23:15 ` Thomas Monjalon
0 siblings, 0 replies; 5+ messages in thread
From: Thomas Monjalon @ 2019-01-14 23:15 UTC (permalink / raw)
To: Hunt, David; +Cc: dev, Yao, Lei A
> > >> This patch fixes a bug introduced in the 64-core limtation
> > >> enhancement where the core_id is inadvertently converted from
> > >> virtual to physical even though it may already be a physical
> > >> core_id.
> > >>
> > >> We should be using the core_type field, and only converting via
> > >> hypervisor when core_type is set to CORE_TYPE_VIRTUAL
> > >>
> > >> Fixes: 5776b7a371d1 ("examples/power: allow VM to use lcores over 63")
> > >>
> > >> Signed-off-by: David Hunt <david.hunt@intel.com>
> Reviewed-by: Lei Yao<lei.a.yao@intel.com>
> Tested-by: Lei Yao<lei.a.yao@intel.com>
Applied, thanks
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-01-14 23:15 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-07 11:39 [dpdk-dev] [PATCH] examples/power: fix wrong core_id with JSON cmds David Hunt
2019-01-08 2:02 ` Yao, Lei A
2019-01-08 9:20 ` Hunt, David
2019-01-08 12:47 ` Yao, Lei A
2019-01-14 23:15 ` 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).