<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
<style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style>
</head>
<body dir="ltr">
<div>
<div style="font-family: Calibri; text-align: left; color: rgb(0, 0, 255); margin-left: 5pt; font-size: 10pt;">
[AMD Official Use Only - AMD Internal Distribution Only]</div>
<br>
</div>
<div style="font-family: Verdana, Geneva, sans-serif; font-size: 9pt; color: rgb(0, 0, 0);" class="elementToProof">
Hi Dariusz, </div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<hr style="display: inline-block; width: 98%;">
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<b>From:</b> Dariusz Sosnowski <dsosnowski@nvidia.com><br>
<b>Sent:</b> Friday, November 07, 2025 11:29 PM<br>
<b>To:</b> Tummala, Sivaprasad <Sivaprasad.Tummala@amd.com>; Alexander Kozyrev <akozyrev@nvidia.com>; Viacheslav Ovsiienko <viacheslavo@nvidia.com><br>
<b>Cc:</b> jerinj@marvell.com <jerinj@marvell.com>; kirankumark@marvell.com <kirankumark@marvell.com>; ndabilpuram@marvell.com <ndabilpuram@marvell.com>; yanzhirun_163@163.com <yanzhirun_163@163.com>; david.marchand@redhat.com <david.marchand@redhat.com>; ktraynor@redhat.com
<ktraynor@redhat.com>; thomas@monjalon.net <thomas@monjalon.net>; konstantin.ananyev@huawei.com <konstantin.ananyev@huawei.com>; konstantin.v.ananyev@yandex.ru <konstantin.v.ananyev@yandex.ru>; bruce.richardson@intel.com <bruce.richardson@intel.com>; maxime.coquelin@redhat.com
<maxime.coquelin@redhat.com>; anatoly.burakov@intel.com <anatoly.burakov@intel.com>; aconole@redhat.com <aconole@redhat.com>; dev@dpdk.org <dev@dpdk.org>; stable@dpdk.org <stable@dpdk.org><br>
<b>Subject:</b> Re: [PATCH] net/mlx5: fix spurious CPU wakeups caused by invalid CQE
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-size: 11pt;" class="elementToProof">Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.<br>
<br>
<br>
> Hi,<br>
><br>
> Thank you for the contribution. Please see comments below.<br>
><br>
> On Wed, Oct 15, 2025 at 01:39:57PM +0000, Sivaprasad Tummala wrote:<br>
>> Previously, the PMD used a common monitor callback to determine<br>
>> CQE ownership for power-aware polling. However, when a CQE contained<br>
>> an invalid opcode(MLX5_CQE_INVALID), ownership bit was not reliable.<br>
>> As a result, the monitor condition could falsely indicate CQE<br>
>> availability and cause the CPU to wake up unnecessarily during<br>
>> low traffic periods.<br>
>><br>
>> This resulted in spurious wakeups in monitor-wait mode and reduced<br>
>> the expected power savings, as cores exited the sleep state even<br>
>> when no valid CQEs were available.<br>
>><br>
>> This patch introduces a dedicated callback that skips invalid CQEs<br>
>> and optimizes power efficiency by preventing false wakeups caused<br>
>> by hardware-owned or invalid entries.<br>
>><br>
>> Fixes: a8f0df6bf98d ("net/mlx5: support power monitoring")<br>
>> Cc: akozyrev@nvidia.com<br>
>> Cc: stable@dpdk.org<br>
>><br>
>> Signed-off-by: Sivaprasad Tummala <sivaprasad.tummala@amd.com><br>
>> ---<br>
>> drivers/net/mlx5/mlx5_rx.c | 17 ++++++++++++++++-<br>
>> 1 file changed, 16 insertions(+), 1 deletion(-)<br>
>><br>
>> diff --git a/drivers/net/mlx5/mlx5_rx.c b/drivers/net/mlx5/mlx5_rx.c<br>
>> index 420a03068d..2765b4b730 100644<br>
>> --- a/drivers/net/mlx5/mlx5_rx.c<br>
>> +++ b/drivers/net/mlx5/mlx5_rx.c<br>
>> @@ -295,6 +295,20 @@ mlx5_monitor_callback(const uint64_t value,<br>
>> return (value & m) == v ? -1 : 0;<br>
>> }<br>
>><br>
>> +static int<br>
>> +mlx5_monitor_cqe_own_callback(const uint64_t value,<br>
>> + const uint64_t opaque[RTE_POWER_MONITOR_OPAQUE_SZ])<br>
>> +{<br>
>> + const uint64_t m = opaque[CLB_MSK_IDX];<br>
>> + const uint64_t v = opaque[CLB_VAL_IDX];<br>
>> + const uint64_t match = ((value & m) == v);<br>
><br>
> Could you please rename "match" variable to "sw_owned"?<br>
> This name would better relay the meaning of the checked condition that<br>
> CQE owner bit value signifies that CQE is SW owned.<br>
ACK! Will update this in v2.</div>
<div style="font-size: 11pt;" class="elementToProof">><br>
>> + const uint64_t opcode = MLX5_CQE_OPCODE(value);<br>
>> + const uint64_t valid_op = (opcode ^ MLX5_CQE_INVALID);<br>
><br>
>IMO the usage of bit operations here (although logic is correct) is a bit confusing.<br>
>Could you rewrite it in terms of logical operations so it's easier to<br>
>follow? For example like this:<br>
><br>
> const uint64_t valid_op = opcode != MLX5_CQE_INVALID<br>
><br>
> return (sw_owned && valid_op) ? -1 : 0;<br>
><br>
>This also would properly describe in code the required condition:<br>
>CQE can be parsed by SW if and only if owner bit is "SW owned" and CQE<br>
>opcode is valid.</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);" class="elementToProof">ACK! Will update this in v2.</div>
<div style="font-size: 11pt;" class="elementToProof">><br>
>> +<br>
>> + /* ownership bit is not valid for invalid opcode; CQE is HW owned */<br>
>> + return -(match & valid_op);<br>
>> +}<br>
>> +<br>
>> int mlx5_get_monitor_addr(void *rx_queue, struct rte_power_monitor_cond *pmc)<br>
>> {<br>
>> struct mlx5_rxq_data *rxq = rx_queue;<br>
>> @@ -312,12 +326,13 @@ int mlx5_get_monitor_addr(void *rx_queue, struct rte_power_monitor_cond *pmc)<br>
>> pmc->addr = &cqe->validity_iteration_count;<br>
>> pmc->opaque[CLB_VAL_IDX] = vic;<br>
>> pmc->opaque[CLB_MSK_IDX] = MLX5_CQE_VIC_INIT;<br>
>> + pmc->fn = mlx5_monitor_callback;<br>
><br>
>Alex, Slava: Just to double check - in case of enhanced CQE compression<br>
>layout, should both CQE opcode and vic be checked?<br>
>Right now only vic is checked in power monitor callback for that case.<br>
>In Rx datapath both are checked to determine CQE ownership:</div>
<div style="font-size: 11pt;" class="elementToProof">><a data-auth="NotApplicable" class="OWAAutoLink" id="OWAf645a71b-7320-788e-63a9-a91c9479b8b4" href="https://github.com/DPDK/dpdk/blob/main/drivers/common/mlx5/mlx5_common.h#L277">https://github.com/DPDK/dpdk/blob/main/drivers/common/mlx5/mlx5_common.h#L277</a></div>
<div style="font-size: 11pt;" class="elementToProof">><br>
>> } else {<br>
>> pmc->addr = &cqe->op_own;<br>
>> pmc->opaque[CLB_VAL_IDX] = !!idx;<br>
>> pmc->opaque[CLB_MSK_IDX] = MLX5_CQE_OWNER_MASK;<br>
>> + pmc->fn = mlx5_monitor_cqe_own_callback;<br>
>> }<br>
>> - pmc->fn = mlx5_monitor_callback;<br>
>> pmc->size = sizeof(uint8_t);<br>
>> return 0;<br>
>> }<br>
>> --<br>
>> 2.43.0<br>
>><br>
><br>
>Best regards,<br>
>Dariusz Sosnowski<br>
</div>
<div style="font-size: 11pt;" class="elementToProof"><br>
</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);" class="elementToProof">Thanks & Regards,</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);" class="elementToProof">Sivaprasad</div>
</body>
</html>