| Message ID | 20230516231713.2882879-3-florian.fainelli@broadcom.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | WAKE_FILTER for Broadcom PHY | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/apply | fail | Patch does not apply to net-next |
On Tue, May 16, 2023 at 04:17:12PM -0700, Florian Fainelli wrote: > Since the PHY is capable of matching any arbitrary Ethernet MAC > destination as a programmable wake-up pattern, add support for doing > that using the WAKE_FILTER and ethtool::rxnfc API. Are there other actions the PHY can perform? For a MAC based filter, i expect there are other actions, like drop, queue selection, etc. So using the generic RXNFC API makes some sense. > 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? The API to the PHY driver could then be made much more narrow, and you would not need the comment this is supposed to only be used for WoL. Andrew
On 5/17/2023 5:49 AM, Andrew Lunn wrote: > On Tue, May 16, 2023 at 04:17:12PM -0700, Florian Fainelli wrote: >> Since the PHY is capable of matching any arbitrary Ethernet MAC >> destination as a programmable wake-up pattern, add support for doing >> that using the WAKE_FILTER and ethtool::rxnfc API. > > Are there other actions the PHY can perform? Not really, it can match on a custom Ethernet MAC DA and that is pretty much it, unless you use the WAKE_MAGIC or WAKE_MAGICSECURE where it can match either or. > > For a MAC based filter, i expect there are other actions, like drop, > queue selection, etc. So using the generic RXNFC API makes some sense. > >> 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. This is how I originally designed it for SYSTEMPORT and this was copied over to GENET and now to this PHY driver. > > The API to the PHY driver could then be made much more narrow, and you > would not need the comment this is supposed to only be used for WoL. 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 but then we have some intersection with the 'u', 'm' and 'b' options too, which are just short hand for specific MAC DAs. This felt a bit like bending the framework for one specific PHY that supports that use case, so I felt like RXNFC, although we only use a very narrow space could be a better fit in case a more capable PHY came along in the future.
> > > 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
diff --git a/drivers/net/phy/bcm-phy-lib.c b/drivers/net/phy/bcm-phy-lib.c index 5603d0a9ce96..546c21ce9775 100644 --- a/drivers/net/phy/bcm-phy-lib.c +++ b/drivers/net/phy/bcm-phy-lib.c @@ -822,7 +822,8 @@ EXPORT_SYMBOL_GPL(bcm_phy_cable_test_get_status_rdb); WAKE_MCAST | \ WAKE_BCAST | \ WAKE_MAGIC | \ - WAKE_MAGICSECURE) + WAKE_MAGICSECURE | \ + WAKE_FILTER) int bcm_phy_set_wol(struct phy_device *phydev, struct ethtool_wolinfo *wol) { @@ -876,6 +877,12 @@ int bcm_phy_set_wol(struct phy_device *phydev, struct ethtool_wolinfo *wol) ctl &= ~BCM54XX_WOL_DIR_PKT_EN; ctl &= ~(BCM54XX_WOL_SECKEY_OPT_MASK << BCM54XX_WOL_SECKEY_OPT_SHIFT); + /* For WAKE_FILTER, we have already programmed the desired MAC DA + * and associated mask by the time we get there. + */ + if (wol->wolopts & WAKE_FILTER) + goto program_ctl; + /* When using WAKE_MAGIC, we program the magic pattern filter to match * the device's MAC address and we accept any MAC DA in the Ethernet * frame. @@ -930,6 +937,7 @@ int bcm_phy_set_wol(struct phy_device *phydev, struct ethtool_wolinfo *wol) return ret; } +program_ctl: if (wol->wolopts & WAKE_MAGICSECURE) { ctl |= BCM54XX_WOL_SECKEY_OPT_6B << BCM54XX_WOL_SECKEY_OPT_SHIFT; @@ -1034,6 +1042,143 @@ irqreturn_t bcm_phy_wol_isr(int irq, void *dev_id) } EXPORT_SYMBOL_GPL(bcm_phy_wol_isr); +static int bcm_phy_get_rule(struct phy_device *phydev, + struct ethtool_rxnfc *nfc, + int loc) +{ + u8 da[ETH_ALEN]; + unsigned int i; + int ret; + + if (loc != 0) + return -EINVAL; + + memset(nfc, 0, sizeof(*nfc)); + nfc->flow_type = ETHER_FLOW; + nfc->fs.flow_type = ETHER_FLOW; + + for (i = 0; i < sizeof(da) / 2; i++) { + ret = bcm_phy_read_exp(phydev, + BCM54XX_WOL_MPD_DATA2(2 - i)); + if (ret < 0) + return ret; + + da[i * 2] = ret >> 8; + da[i * 2 + 1] = ret & 0xff; + } + ether_addr_copy(nfc->fs.h_u.ether_spec.h_dest, da); + + for (i = 0; i < sizeof(da) / 2; i++) { + ret = bcm_phy_read_exp(phydev, + BCM54XX_WOL_MASK(2 - i)); + if (ret < 0) + return ret; + + da[i * 2] = ret >> 8; + da[i * 2 + 1] = ret & 0xff; + } + ether_addr_copy(nfc->fs.m_u.ether_spec.h_dest, da); + + nfc->fs.ring_cookie = RX_CLS_FLOW_WAKE; + nfc->fs.location = 0; + + return 0; +} + +static int bcm_phy_set_rule(struct phy_device *phydev, + struct ethtool_rxnfc *nfc) +{ + int ret = -EOPNOTSUPP; + unsigned int i; + const u8 *da; + + /* We support only matching on the MAC DA, reject anything else */ + if (nfc->fs.ring_cookie != RX_CLS_FLOW_WAKE || + nfc->fs.location != 0 || + nfc->fs.flow_type != ETHER_FLOW || + nfc->fs.h_u.ether_spec.h_proto || + !is_zero_ether_addr(nfc->fs.h_u.ether_spec.h_source) || + nfc->fs.m_u.ether_spec.h_proto || + !is_zero_ether_addr(nfc->fs.m_u.ether_spec.h_source)) + return ret; + + da = nfc->fs.h_u.ether_spec.h_dest; + for (i = 0; i < ETH_ALEN / 2; i++) { + ret = bcm_phy_write_exp(phydev, + BCM54XX_WOL_MPD_DATA2(2 - i), + da[i * 2] << 8 | da[i * 2 + 1]); + if (ret < 0) + return ret; + } + + da = nfc->fs.m_u.ether_spec.h_dest; + for (i = 0; i < ETH_ALEN / 2; i++) { + ret = bcm_phy_write_exp(phydev, + BCM54XX_WOL_MASK(2 - i), + da[i * 2] << 8 | da[i * 2 + 1]); + if (ret < 0) + return ret; + } + + return 0; +} + +int bcm_phy_get_rxnfc(struct phy_device *phydev, + struct ethtool_rxnfc *cmd, u32 *rule_locs) +{ + int err = 0; + + switch (cmd->cmd) { + case ETHTOOL_GRXCLSRLCNT: + cmd->rule_cnt = 1; + cmd->data = 1 | RX_CLS_LOC_SPECIAL; + break; + case ETHTOOL_GRXCLSRULE: + err = bcm_phy_get_rule(phydev, cmd, cmd->fs.location); + break; + case ETHTOOL_GRXCLSRLALL: + rule_locs[0] = 0; + cmd->rule_cnt = 1; + cmd->data = 1; + break; + default: + err = -EOPNOTSUPP; + break; + } + + return err; +} +EXPORT_SYMBOL_GPL(bcm_phy_get_rxnfc); + +int bcm_phy_set_rxnfc(struct phy_device *phydev, + struct ethtool_rxnfc *cmd, + bool *installed) +{ + int err = 0; + + switch (cmd->cmd) { + case ETHTOOL_SRXCLSRLINS: + err = bcm_phy_set_rule(phydev, cmd); + if (err) + return err; + + *installed = true; + break; + case ETHTOOL_SRXCLSRLDEL: + if (cmd->fs.location != 0) + return err; + + *installed = false; + break; + default: + err = -EOPNOTSUPP; + break; + } + + return err; +} +EXPORT_SYMBOL_GPL(bcm_phy_set_rxnfc); + MODULE_DESCRIPTION("Broadcom PHY Library"); MODULE_LICENSE("GPL v2"); MODULE_AUTHOR("Broadcom Corporation"); diff --git a/drivers/net/phy/bcm-phy-lib.h b/drivers/net/phy/bcm-phy-lib.h index 2f30ce0cab0e..4881ea34e99c 100644 --- a/drivers/net/phy/bcm-phy-lib.h +++ b/drivers/net/phy/bcm-phy-lib.h @@ -118,4 +118,10 @@ int bcm_phy_set_wol(struct phy_device *phydev, struct ethtool_wolinfo *wol); void bcm_phy_get_wol(struct phy_device *phydev, struct ethtool_wolinfo *wol); irqreturn_t bcm_phy_wol_isr(int irq, void *dev_id); +int bcm_phy_get_rxnfc(struct phy_device *phydev, + struct ethtool_rxnfc *nfc, u32 *rule_locs); +int bcm_phy_set_rxnfc(struct phy_device *phydev, + struct ethtool_rxnfc *nfc, + bool *installed); + #endif /* _LINUX_BCM_PHY_LIB_H */ diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c index 822c8b01dc53..b4a8aba7d5ef 100644 --- a/drivers/net/phy/broadcom.c +++ b/drivers/net/phy/broadcom.c @@ -36,6 +36,7 @@ struct bcm54xx_phy_priv { struct bcm_ptp_private *ptp; int wake_irq; bool wake_irq_enabled; + bool wake_filter_installed; }; static bool bcm54xx_phy_can_wakeup(struct phy_device *phydev) @@ -860,6 +861,8 @@ static int brcm_fet_suspend(struct phy_device *phydev) static void bcm54xx_phy_get_wol(struct phy_device *phydev, struct ethtool_wolinfo *wol) { + struct bcm54xx_phy_priv *priv = phydev->priv; + /* We cannot wake-up if we do not have a dedicated PHY interrupt line * or an out of band GPIO descriptor for wake-up. Zeroing * wol->supported allows the caller (MAC driver) to play through and @@ -871,6 +874,8 @@ static void bcm54xx_phy_get_wol(struct phy_device *phydev, } bcm_phy_get_wol(phydev, wol); + if (priv->wake_filter_installed) + wol->wolopts |= WAKE_FILTER; } static int bcm54xx_phy_set_wol(struct phy_device *phydev, @@ -893,6 +898,14 @@ static int bcm54xx_phy_set_wol(struct phy_device *phydev, return 0; } +static int bcm54xx_phy_set_rxnfc(struct phy_device *phydev, + struct ethtool_rxnfc *cmd) +{ + struct bcm54xx_phy_priv *priv = phydev->priv; + + return bcm_phy_set_rxnfc(phydev, cmd, &priv->wake_filter_installed); +} + static int bcm54xx_phy_probe(struct phy_device *phydev) { struct bcm54xx_phy_priv *priv; @@ -1031,6 +1044,8 @@ static struct phy_driver broadcom_drivers[] = { .resume = bcm54xx_resume, .get_wol = bcm54xx_phy_get_wol, .set_wol = bcm54xx_phy_set_wol, + .get_rxnfc = bcm_phy_get_rxnfc, + .set_rxnfc = bcm54xx_phy_set_rxnfc, }, { .phy_id = PHY_ID_BCM5461, .phy_id_mask = 0xfffffff0,
Since the PHY is capable of matching any arbitrary Ethernet MAC destination as a programmable wake-up pattern, add support for doing that using the WAKE_FILTER and ethtool::rxnfc API. For instance, in order to wake-up from the Ethernet MAC address corresponding to the IPv4 multicast IP address of 224.0.0.1, one could do: 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 Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com> --- drivers/net/phy/bcm-phy-lib.c | 147 +++++++++++++++++++++++++++++++++- drivers/net/phy/bcm-phy-lib.h | 6 ++ drivers/net/phy/broadcom.c | 15 ++++ 3 files changed, 167 insertions(+), 1 deletion(-)