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

Message-ID: <20111117020613.GA18232@mcarlson.broadcom.com>
Date:	Wed, 16 Nov 2011 18:06:13 -0800
From:	"Matt Carlson" <mcarlson@...adcom.com>
To:	"Ben Hutchings" <bhutchings@...arflare.com>
cc:	"Matthew Carlson" <mcarlson@...adcom.com>,
	"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, 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?

> > > 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.


--
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