Re: [PATCH net-next v6 06/12] net: pse-pd: Add support for budget evaluation strategies

[Kyle Swenson]

Hello Kory,

On Tue, Mar 25, 2025 at 04:25:34PM +0100, Kory Maincent wrote:
> On Tue, 25 Mar 2025 06:34:17 +0100
> Oleksij Rempel <o.rempel@xxxxxxxxxxxxxx> wrote:
> 
> > Hi,
> > 
> > On Mon, Mar 24, 2025 at 05:33:18PM +0000, Kyle Swenson wrote:
> > > Hello Kory,
> > > 
> > > On Mon, Mar 24, 2025 at 05:39:07PM +0100, Kory Maincent wrote:  
> > > > Hello Kyle, Oleksij,  
> > > ...  
> > > > 
> > > > Small question on PSE core behavior for PoE users.
> > > > 
> > > > If we want to enable a port but we can't due to over budget.
> > > > Should we :
> > > > - Report an error (or not) and save the enable action from userspace. On
> > > > that case, if enough budget is available later due to priority change or
> > > > port disconnected the PSE core will try automatically to re enable the
> > > > PoE port. The port will then be enabled without any action from the user.
> > > > - Report an error but do nothing. The user will need to rerun the enable
> > > >   command later to try to enable the port again.
> > > > 
> > > > How is it currently managed in PoE poprietary userspace tools?  
> > > 
> > > So in our implementation, we're using the first option you've presented.
> > > That is, we save the enable action from the user and if we can't power
> > > the device due to insufficient budget remaining, we'll indicate that status
> > > to the user.  If enough power budget becomes available later, we'll power up
> > > the device automatically.  
> > 
> > It seems to be similar to administrative UP state - "ip link set dev lan1 up".
> > I'm ok with this behavior.
> 
> Ack I will go for it then, thank you!
> 
> Other question to both of you:
> If we configure manually the current limit for a port. Then we plug a Powered
> Device and we detect (during the classification) a smaller current limit
> supported. Should we change the current limit to the one detected. On that case
> we should not let the user set a power limit greater than the one detected after
> the PD has been plugged.

I don't know that we want to prevent the user from setting a higher
current than a device's classification current because that would
prevent the PD and PSE negotiating a higher current via LLDP.

That said, I'm struggling to think of a use-case where the user would be
setting a current limit before a PD is connected, so maybe we can reset
the current limit when the PD is classified to the classification
result, but also allow it to be adjusted after a PD is powered for the
LLDP negotiation case.

In our implementation, don't really let the user specify something like,
"Only class 3 and lower devices on this port" because we've not seen
customers need this.  We have, however, implemented the LLDP negotiation
support after several requests from customers, but this only makes sense
when a PD is powered at it's initial classification result.  The PD can
then request more power (via LLDP) and then we adjust the current limit
assuming the system has budget available for the request.

> 
> What do you think? Could we let a user burn a PD?

This seems like a very rare case, and if the PD is designed such that
it's reliant on the PSE's current limiting ability then seems like it's
just an accident waiting to happen with any PSE.

Very rarely have we seen a device actually pull more current than it's
classification result allows (except for LLDP negotiation). What's more
likely is a dual-channel 802.3bt device is incorrectly classified as a
single-channel 802.3at device; the device pulls more current than
allocated and gets shut off promptly, but no magic smoke escaped.  

Hope this helps!
Kyle