netdev - Re: [net-next 1/4 (V3)] net: ethtool: add the EEE support

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]

Message-ID: <1334269598.2497.50.camel@bwh-desktop.uk.solarflarecom.com>
Date:	Thu, 12 Apr 2012 23:26:38 +0100
From:	Ben Hutchings <bhutchings@...arflare.com>
To:	Giuseppe CAVALLARO <peppe.cavallaro@...com>
CC:	<netdev@...r.kernel.org>, <rayagond@...avyalabs.com>,
	<davem@...emloft.net>
Subject: Re: [net-next 1/4 (V3)] net: ethtool: add the EEE support

On Fri, 2012-04-06 at 11:29 +0200, Giuseppe CAVALLARO wrote:
> This patch adds two new functions to detect if Energy-Efficient
> Ethernet (EEE) is supported and the way to enable/disable it.
> 
> As Ben said, it is certainly necessary to distinguish:
> 
> a. PHY is advertising EEE from
> b. Link partner is advertising EEE
> or
> c. EEE will be used (= a && b)
> 
> The logic behind this code, is that .get_eee will pass
> to the user-space if the EEE is actually used and available (so point c).
> The .set_eee should used to force the MAC to disable/enable the EEE (if
> actually supported by MAC+PHY).
[...]

What I meant is that userland should be able to find out (a), and,
*separately*, either (b) or (c).  That is, there must be at least 2
separate flags for this.  In fact, I explicitly requested you define
supported/advertising/lp_advertising bitmasks for EEE link modes just
like we have for autonegotiation.  But you've made no substantive
changes in response to my review, aside from dropping the added field in
ethtool_cmd.

What you're submitting just isn't good enough for a generic interface,
as the ethtool API is supposed to be.  It's not even a good interface to
your driver.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html