Re: [PATCH net-next 1/2] net: ethtool: Add support for PHY packet generators
From: Ben Hutchings <hidden>
Date: 2016-02-17 21:06:39
On Wed, 2016-02-17 at 21:32 +0100, Andrew Lunn wrote:
Some PHY devices contain a simple packet generator. Features vary, but often they can be used to generate packets of different sizes, different contents, with or without errors, and with different inter packet gaps. Add support to the core ethtool code to support this.
[...]
quoted hunk ↗ jump to hunk
--- a/include/uapi/linux/ethtool.h +++ b/include/uapi/linux/ethtool.h@@ -1167,6 +1167,31 @@ struct ethtool_ts_info {__u32 rx_reserved[3]; }; +enum ethtool_phy_pkg_gen_flags { + ETH_PKT_RANDOM = (1 << 0), + ETH_PKT_ERROR = (1 << 1),
What kind of error? CRC error, FEC error, symbol error?
+}; + +/** + * struct ethtool_phy_pkt_get - command to request the phy to generate packets. + * @cmd: command number = %ETHTOOL_PHY_PKT_GEN + * @count: number of packets to generate + * @len: length of generated packets + * @ipg: inter packet gap in bytes.
What if the PHY doesn't allow varying the IPG? Should there be a way to find out what its supported IPG is, or to request the default value? Similarly, should there be a way to find out the minimum/maximum length it supports?
+ * @flags: a bitmask of flags from &enum ethtool_phy_pkg_gen_flags + * + * PHY drivers may not support all of these parameters. If the + * requested parameter value cannot be supported an error should be + * returned.
Should, or must? How does userland tell when the PHY has finished? Should it be possible to cancel this (similar to ETHTOOL_PHYS_ID)? What should happen if the stack tries to send a packet while the PHY is in this mode? Is it discarded? Should the driver indicate carrier-off so that this is obvious? [...]
quoted hunk ↗ jump to hunk
--- a/net/core/ethtool.c +++ b/net/core/ethtool.c@@ -1541,6 +1541,25 @@ static int ethtool_get_phy_stats(struct net_device *dev, void __user *useraddr)return ret; } +static int ethtool_phy_pkt_gen(struct net_device *dev, void __user *useraddr) +{ + struct phy_device *phydev = dev->phydev; + struct ethtool_phy_pkt_gen pkt_gen; + int err; + + if (!phydev || !phydev->drv->pkt_gen) + return -EOPNOTSUPP;
[...]
Why should this be tied to phylib? Nothing else in the ethtool
interface is.
Ben.
--
Ben Hutchings
Lowery's Law:
If it jams, force it. If it breaks, it needed replacing anyway. Attachments
- signature.asc [application/pgp-signature] 811 bytes