<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 Alexander, </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> Alexander Kozyrev <akozyrev@nvidia.com><br>
<b>Sent:</b> Tuesday, November 18, 2025 1:35 AM<br>
<b>To:</b> Tummala, Sivaprasad <Sivaprasad.Tummala@amd.com>; Dariusz Sosnowski <dsosnowski@nvidia.com>; Slava 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>; NBU-Contact-Thomas Monjalon (EXTERNAL) <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>
<table style="direction: ltr; width: 100%; box-sizing: border-box; border-collapse: collapse; border-spacing: 0px;" cellpadding="0" cellspacing="0" align="left">
<tbody>
<tr>
<td style="direction: ltr; background-color: rgb(255, 185, 0); padding: 5pt 2pt;">
</td>
<td style="direction: ltr; background-color: rgb(255, 248, 229); padding: 5pt 4pt 5pt 12pt; width: 100%;">
<div style="direction: ltr; color: rgb(34, 34, 34);"><b>Caution:</b> This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.</div>
</td>
</tr>
</tbody>
</table>
<div style="direction: ltr;"><br>
</div>
<div style="direction: ltr; font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);" class="elementToProof">
>>> 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="direction: ltr; 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="direction: ltr; font-size: 11pt; color: black;" class="elementToProof">
>ACK! Will update this in v2.</div>
<div style="direction: ltr; 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="direction: ltr; font-size: 11pt;" class="elementToProof">>><a data-auth="NotApplicable" class="x_OWAAutoLink" id="OWA2e54c5ce-15bf-9840-085a-4ab66eeeeded" 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="direction: ltr; font-family: Verdana, Geneva, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);" class="elementToProof">
></div>
<div style="direction: ltr; font-size: 11pt; color: rgb(0, 0, 0);" class="elementToProof">
>Sorry for the late reply. I think we should check opcode in both cases.</div>
<div style="direction: ltr; font-size: 11pt; color: rgb(0, 0, 0);" class="elementToProof">
>mlx5_monitor_callback can be updated with the opcode check for enhanced CQE compression layout,</div>
<div style="direction: ltr; font-size: 11pt; color: rgb(0, 0, 0);" class="elementToProof">
>instead of having 2 separate callback functions. Could you please prepare a follow-up patch for that?</div>
<div style="direction: ltr; font-family: Verdana, Geneva, sans-serif; font-size: 9pt; color: rgb(0, 0, 0);" class="elementToProof">
Ok, I can extend this patch to also cover for enhanced CQE compression case as well. </div>
<div style="direction: ltr; font-family: Verdana, Geneva, sans-serif; font-size: 9pt; color: rgb(0, 0, 0);" class="elementToProof">
Right now, the new call back was added to avoid additional checks in the older callback function. </div>
<div style="direction: ltr; font-family: Verdana, Geneva, sans-serif; font-size: 9pt; color: rgb(0, 0, 0);" class="elementToProof">
I can rework on this as needed. </div>
<div style="direction: ltr; font-family: Verdana, Geneva, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);" class="elementToProof">
>></div>
<div style="direction: ltr; font-family: Verdana, Geneva, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);" class="elementToProof">
>>> } 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>
>></div>
</body>
</html>