Re: [PATCH net-next 2/3] net: phy: broadcom: Add support for WAKE_FILTER

From: Andrew Lunn
Date: Wed May 17 2023 - 17:30:07 EST


> > > ethtool -N eth0 flow-type ether dst 01:00:5e:00:00:fb loc 0 action -2
> > > ethtool -n eth0
> > > Total 1 rules
> > >
> > > Filter: 0
> > > Flow Type: Raw Ethernet
> > > Src MAC addr: 00:00:00:00:00:00 mask: FF:FF:FF:FF:FF:FF
> > > Dest MAC addr: 01:00:5E:00:00:FB mask: 00:00:00:00:00:00
> > > Ethertype: 0x0 mask: 0xFFFF
> > > Action: Wake-on-LAN
> > > ethtool -s eth0 wol f
> >
> > What i don't particularly like about this is its not vary
> > discoverable, since it is not part of:
> >
> > wol p|u|m|b|a|g|s|f|d...
> > Sets Wake-on-LAN options. Not all devices support
> > this. The argument to this option is a string of
> > characters specifying which options to enable.
> >
> > p Wake on PHY activity
> > u Wake on unicast messages
> > m Wake on multicast messages
> > b Wake on broadcast messages
> > a Wake on ARP
> > g Wake on MagicPacket™
> > s Enable SecureOn™ password for MagicPacket™
> > f Wake on filter(s)
> > d Disable (wake on nothing). This option
> > clears all previous options.
> >
> > If the PHY hardware is not generic, it only has one action, WoL, it
> > might be better to have this use the standard wol commands. Can it be
> > made to work under the 'f' option?
>
> You actually need both, if you only configure the filter with
> RX_CLS_FLOW_WAKE but forget to set the 'f' bit in wolopts, then the wake-up
> will not occur because the PHY will not have been configured with the
> correct matching mode.

Ah. Please could you extend the man page for ethtool. Maybe make flow
type action -2 reference wol, and wol f reference flow-type?

> I was initially considering that the 'sopass' field could become an union
> since it is exactly the size of a MAC address (6 bytes) and you could do
> something like:
>
> ethtool -s eth0 wol f mac 01:00:5E:00:00:FB

Yes, i was thinking something like that.

> but then we have some intersection with the 'u', 'm' and 'b' options too,
> which are just short hand for specific MAC DAs.

The man page for ethtool say:

sopass xx:yy:zz:aa:bb:cc
Sets the SecureOn™ password. The argument to this option
must be 6 bytes in Ethernet MAC hex format
(xx:yy:zz:aa:bb:cc).

So i don't think it is too much of an API bendage to pass a MAC
address in a union.

I had a quick look at some Marvell switches. They allow an arbitrary
Unicast MAC address to be used to wake a port. So such an extension
could be used for it as well.

And it looks like the Marcell Alaska PHY could implement it as
well. So it would not be limited to just the Broadcom PHYs.

Andrew