Re: [PATCH net v2] net: pse-pd: don't enable a PI again while disabling it
From: Ross Cawston
Date: Thu Oct 22 2026 - 22:05:20 EST
On Thu, 22 Oct 2026 14:21:12 +0200, Kory Maincent wrote:
> On 10/22/26 12:08, netdev-bot+sashiko@xxxxxxxxxx wrote:
>> - [High] The fix is incomplete: the PI being disabled is still enabled
>> again, and the power budget still leaks, on the priority-eviction path.
>>
>> Pre-existing issues:
>> - [Medium] The priority cut-off in pse_pw_d_retry_power_delivery()
>> (drivers/net/pse-pd/pse_core.c) never takes effect.
>
> Can you take a look at the above issues?
> The other ones are currently in discussion with Kyle due to similar
> report he faced on his series.
Yes, both are right.
The eviction path: v2 leaves it as it was, and its changelog undersold
what is wrong there. Besides powering the PI just turned off again, the
retry in pse_disable_pi_pol() can enable the PI that is making room,
from inside its own allocation, which then requests its budget a second
time; or it turns the other PI off again and recurses. That was already
so before v2, when _pse_pi_disable() retried the domain itself.
I have two patches for it on top of v2, build-tested only so far:
- "net: pse-pd: don't allocate the budget of a powered PI again": power
delivery skips a PI that holds budget and is enabled, and a PI that
holds budget while it is off has it freed and allocated again.
- "net: pse-pd: make room for a PI without recursing": it selects the
PIs of lower priority to turn off before turning any off, turns them
off without a retry, and requests the budget once. If that fails, they
get their budget back and are enabled again. Only the PIs that stay
off are notified as over budget, and the domain is retried once
afterwards.
The prio_max cut-off: the second patch replaces it. The retry goes from
the highest priority (lowest value) to the lowest, the power-up order
that ethtool-netlink.rst documents, so a PI of lower priority no longer
takes the budget that one of higher priority waits for. It drops the
cut-off: with the PIs to turn off selected first, a PI of higher
priority only fails with -ERANGE when turning off all the PIs of lower
priority that hold budget wouldn't make room either. The budget left
over is then of no use to it, and once there is room, it can still turn
off a PI of lower priority that took some. If you'd rather keep the
cut-off as well, with prio_max declared outside the loop, I'll add it.
I'll send v3 as a series of the v2 fix and these two, once I've tested
it on the RTL8239 switch with a budget tight enough for the priorities
to matter. A test on a TPS23881 would be welcome too.
I'll leave the other three to the discussion with Kyle, Cc'd. One note
on the PSE_BUDGET_EVAL_STRAT_DYNAMIC test in pse_set_config_isr(): once
it no longer drops a classification reported with a detection, that
classification can reach the recursion above, so its fix should go in
after the eviction fix, or together with it.
Carlo's net v2 series changes the same functions; whichever goes in
first, the other rebases on it.
pw-bot: cr
Ross