netdev - Re: [PATCH net-next 1/2] net: ethtool: Add support for PHY packet generators

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 17 Feb 2016 21:06:19 +0000
From:	Ben Hutchings <ben@...adent.org.uk>
To:	Andrew Lunn <andrew@...n.ch>, David Miller <davem@...emloft.net>,
	Florian Fainelli <f.fainelli@...il.com>
Cc:	netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next 1/2] net: ethtool: Add support for PHY packet
 generators

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.
[...]
> --- 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?

[...]
> --- 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.
Download attachment "signature.asc" of type "application/pgp-signature" (812 bytes)