netdev - Re: [PATCH net-next] net: sfp: Use smaller chunk size when reading I2C data

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20190531201341.syriememelbklhvo@shell.armlinux.org.uk>
Date:   Fri, 31 May 2019 21:13:41 +0100
From:   Russell King - ARM Linux admin <linux@...linux.org.uk>
To:     Robert Hancock <hancock@...systems.ca>
Cc:     netdev@...r.kernel.org
Subject: Re: [PATCH net-next] net: sfp: Use smaller chunk size when reading
 I2C data

On Fri, May 31, 2019 at 01:18:03PM -0600, Robert Hancock wrote:
> The SFP driver was reading up to 256 bytes of I2C data from the SFP
> module in a single chunk. However, some I2C controllers do not support
> reading that many bytes in a single transaction. Change to use a more
> compatible 16-byte chunk size, since this is not performance critical.

This is the wrong place to fix the problem.  We still end up reading
more than 16 bytes with this approach.  I already have a patch fixing
this the right way, which is in the queue to be sent.

> 
> Signed-off-by: Robert Hancock <hancock@...systems.ca>
> ---
>  drivers/net/phy/sfp.c | 38 ++++++++++++++++++++++++--------------
>  1 file changed, 24 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
> index 6b6c83d..23a40a7 100644
> --- a/drivers/net/phy/sfp.c
> +++ b/drivers/net/phy/sfp.c
> @@ -1651,7 +1651,7 @@ static int sfp_module_info(struct sfp *sfp, struct ethtool_modinfo *modinfo)
>  static int sfp_module_eeprom(struct sfp *sfp, struct ethtool_eeprom *ee,
>  			     u8 *data)
>  {
> -	unsigned int first, last, len;
> +	unsigned int first, last;
>  	int ret;
>  
>  	if (ee->len == 0)
> @@ -1659,26 +1659,36 @@ static int sfp_module_eeprom(struct sfp *sfp, struct ethtool_eeprom *ee,
>  
>  	first = ee->offset;
>  	last = ee->offset + ee->len;
> -	if (first < ETH_MODULE_SFF_8079_LEN) {
> -		len = min_t(unsigned int, last, ETH_MODULE_SFF_8079_LEN);
> -		len -= first;
>  
> -		ret = sfp_read(sfp, false, first, data, len);
> +	while (first < last) {
> +		bool a2;
> +		unsigned int this_addr, len;
> +
> +		if (first < ETH_MODULE_SFF_8079_LEN) {
> +			len = min_t(unsigned int, last,
> +				    ETH_MODULE_SFF_8079_LEN);
> +			len -= first;
> +			a2 = false;
> +			this_addr = first;
> +		} else {
> +			len = min_t(unsigned int, last,
> +				    ETH_MODULE_SFF_8472_LEN);
> +			len -= first;
> +			a2 = true;
> +			this_addr = first - ETH_MODULE_SFF_8079_LEN;
> +		}
> +		/* Some I2C adapters cannot read 256 bytes in a single read.
> +		 * Use a smaller chunk size to ensure we are within limits.
> +		 */
> +		len = min_t(unsigned int, len, 16);
> +
> +		ret = sfp_read(sfp, a2, this_addr, data, len);
>  		if (ret < 0)
>  			return ret;
>  
>  		first += len;
>  		data += len;
>  	}
> -	if (first < ETH_MODULE_SFF_8472_LEN && last > ETH_MODULE_SFF_8079_LEN) {
> -		len = min_t(unsigned int, last, ETH_MODULE_SFF_8472_LEN);
> -		len -= first;
> -		first -= ETH_MODULE_SFF_8079_LEN;
> -
> -		ret = sfp_read(sfp, true, first, data, len);
> -		if (ret < 0)
> -			return ret;
> -	}
>  	return 0;
>  }
>  
> -- 
> 1.8.3.1
> 
> 

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up