Re: [dpdk-dev] [PATCH] examples/power: fix wrong core_id with JSON cmds

["Yao, Lei A" <lei.a.yao@intel.com>]

> -----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
> >
> >