netdev - Re: [PATCH] Add ethtool to mii advertisment conversion helpers

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] [day] [month] [year] [list]

Message-ID: <1321497950.2885.72.camel@deadeye>
Date:	Thu, 17 Nov 2011 02:45:50 +0000
From:	Ben Hutchings <bhutchings@...arflare.com>
To:	Matt Carlson <mcarlson@...adcom.com>
CC:	"davem@...emloft.net" <davem@...emloft.net>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	Michael Chan <mchan@...adcom.com>
Subject: Re: [PATCH] Add ethtool to mii advertisment conversion helpers

On Wed, 2011-11-16 at 18:06 -0800, Matt Carlson wrote:
> On Wed, Nov 16, 2011 at 05:29:42PM -0800, Ben Hutchings wrote:
> > On Wed, 2011-11-16 at 17:16 -0800, Matt Carlson wrote:
> > > On Wed, Nov 16, 2011 at 04:34:37PM -0800, Ben Hutchings wrote:
> > [...]
> > > > > +#define mii_lpa_to_ethtool_100bt(lpa)	mii_adv_to_ethtool_100bt(lpa)
> > > > 
> > > > Shouldn't this additionally translate LPA_LPACK into ADVERTISED_Autoneg?
> > > 
> > > You mean, like this?
> > > 
> > > static inline u32 mii_lpa_to_ethtool_100bt(u32 lpa)
> > > {
> > > 	u32 result = 0;
> > > 
> > > 	if (lpa & LPA_LPACK)
> > > 		result |= ADVERTISED_Autoneg;
> > > 
> > > 	return result | mii_adv_to_ethtool_100bt(lpa);
> > > }
> > > 
> > > Yes, that looks like a better implementation.
> > 
> > Think so.
> > 
> > And I think the mii_adv_to_ethtool_* functions should add
> > ADVERTISED_Autoneg unconditionally.  But I'm not entirely sure that's
> > right.
> 
> The primary purpose of these functions is to translate the information
> between two representations.  It seems wise to be careful not to add
> from it or take anything away from it.  It is certainly possible to
> have a valid AN advertisement register configuration, but not have
> autoneg enabled.  Keeping the ADVERTISED_Autoneg out could prevent
> misuse.  Do you agree?

I'm not sure what you mean about misuse.  If autoneg is not enabled then
the register contents are not used and I don't think the
mii_adv_to_ethtool functions should be called at all.  But I'm not that
bothered either way.

> > > > Shouldn't there be mii_lpa_to_ethtool_1000X (or
> > > > mii_lpa_to_ethtool_lpa_x)?
> > > 
> > > Yes.  You're right.  Should it just be a preprocessor definition that
> > > points to mii_adv_to_ethtool_1000X()?
> > 
> > I think that would need to handle LPA_LPACK as well.
> 
> I was wondering if that was present in 1000Base-X mode.  I didn't see it
> in my passing glance at a spec.

IEEE 802.3-2005 §37.2.1.6 seems to define it similarly to the TP case.

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