From: Ben Hutchings <bhutchings@solarflare.com>
To: netdev@vger.kernel.org
Cc: linux-net-drivers@solarflare.com,
Naveen Chouksey <nchouksey@solarflare.com>
Subject: Re: [PATCH ethtool] ethtool: Warn if we cannot set the advertising mask for specified speed/duplex
Date: Fri, 18 Feb 2011 20:08:57 +0000 [thread overview]
Message-ID: <1298059737.2570.43.camel@bwh-desktop> (raw)
In-Reply-To: <1298044598.2570.4.camel@bwh-desktop>
On Fri, 2011-02-18 at 15:56 +0000, Ben Hutchings wrote:
> On Thu, 2011-02-17 at 19:06 +0000, Ben Hutchings wrote:
> > When autonegotiation is enabled, drivers must determine link speed and
> > duplex through the autonegotiation process and will generally ignore
> > the speed and duplex specified in struct ethtool_cmd. If the user
> > specifies a recognised combination of speed and duplex, we set the
> > advertising mask to include only the matching mode. Otherwise, we
> > advertise all supported modes. We should really limit the advertised
> > modes separately by speed and duplex, but for now we just warn.
> >
> > Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> > ---
> > This is a longstanding bug in ethtool which people keep getting bitten
> > by. Naveen was just the latest to report it. I have been working on
> > some changes to improve link advertising and reporting, but until those
> > are ready I'm adding this warning.
>
> That patch fixes an additional bug: we will now update the advertising
> mask based on speed/duplex when autoneg is already on. Unfortunately it
> introduces a bug: we will update advertising even if none of autoneg,
> speed or duplex are specified!
>
> Here's a second version which I think will do the right thing in all
> cases, though I haven't tested it yet. I'll include this in ethtool
> 2.6.38 if no-one can spot a flaw.
[...]
> + if (autoneg_wanted == AUTONEG_ENABLE &&
> + advertising_wanted == 0) {
> + ecmd.advertising = ecmd.supported &
> + (ADVERTISED_10baseT_Half |
> + ADVERTISED_10baseT_Full |
> + ADVERTISED_100baseT_Half |
> + ADVERTISED_100baseT_Full |
> + ADVERTISED_1000baseT_Half |
> + ADVERTISED_1000baseT_Full |
> + ADVERTISED_2500baseX_Full |
> + ADVERTISED_10000baseT_Full);
> + } else if (advertising_wanted != -1) {
This condition should be (advertising_wanted > 0). With that last
change, ethtool seems to do the right thing.
Ben.
> + ecmd.advertising = advertising_wanted;
> }
>
> /* Try to perform the update. */
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
prev parent reply other threads:[~2011-02-18 20:09 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-02-17 19:06 [PATCH ethtool] ethtool: Warn if we cannot set the advertising mask for specified speed/duplex Ben Hutchings
2011-02-18 15:56 ` Ben Hutchings
2011-02-18 20:08 ` Ben Hutchings [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1298059737.2570.43.camel@bwh-desktop \
--to=bhutchings@solarflare.com \
--cc=linux-net-drivers@solarflare.com \
--cc=nchouksey@solarflare.com \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).