From: Ben Hutchings <bhutchings@solarflare.com>
To: Parav Pandit <parav.pandit@emulex.com>
Cc: <netdev@vger.kernel.org>
Subject: Re: [PATCH] net: added support for 40GbE link.
Date: Mon, 18 Jun 2012 18:09:36 +0100 [thread overview]
Message-ID: <1340039376.2913.13.camel@bwh-desktop.uk.solarflarecom.com> (raw)
In-Reply-To: <0c7c97b0-bfe1-4143-a562-2019f86912fc@exht1.ad.emulex.com>
On Mon, 2012-06-18 at 18:14 +0530, Parav Pandit wrote:
> 1. link speed of 40GbE and #4 KR4, CR4, SR4, LR4 modes defined.
> 2. removed code replication for tov calculation for 1G, 10G and
> made is common for 1G, 10G, 40G.
[...]
> @@ -1185,12 +1193,13 @@ struct ethtool_ops {
> * it was forced up into this mode or autonegotiated.
> */
>
> -/* The forced speed, 10Mb, 100Mb, gigabit, 2.5Gb, 10GbE. */
> +/* The forced speed, 10Mb, 100Mb, gigabit, 2.5Gb, 10GbE, 40GbE. */
> #define SPEED_10 10
> #define SPEED_100 100
> #define SPEED_1000 1000
> #define SPEED_2500 2500
> #define SPEED_10000 10000
> +#define SPEED_40000 40000
> #define SPEED_UNKNOWN -1
I don't think there's any need to name all possible link speeds, and it
just encourages the bad practice of ethtool API users checking for
specific values. You may notice there is no SPEED_20000.
> /* Duplex, half or full. */
> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index 8a10d5b..dd0e503 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -542,13 +542,11 @@ static int prb_calc_retire_blk_tmo(struct packet_sock *po,
> rtnl_unlock();
> if (!err) {
> switch (ecmd.speed) {
> - case SPEED_10000:
> - msec = 1;
> - div = 10000/1000;
> - break;
> case SPEED_1000:
> + case SPEED_10000:
> + case SPEED_40000:
> msec = 1;
> - div = 1000/1000;
> + div = ecmd.speed / 1000;
> break;
This function should be fixed properly. Firstly, it must use
ethtool_cmd_speed() rather than directly accessing ecmd.speed.
Secondly, it should allow any speed value rather than checking for
specific values. Then there will be no need to make further changes for
100G or any other new speed.
Ben.
> /*
> * If the link speed is so slow you don't really
--
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.
next prev parent reply other threads:[~2012-06-18 17:09 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-06-18 12:44 [PATCH] net: added support for 40GbE link Parav Pandit
2012-06-18 16:27 ` Rick Jones
2012-06-18 16:56 ` Ben Hutchings
2012-06-19 5:20 ` Parav.Pandit
2012-06-18 17:09 ` Ben Hutchings [this message]
2012-06-19 7:29 ` David Miller
2012-06-19 7:33 ` Parav.Pandit
2012-06-19 7:35 ` David Miller
2012-06-19 7:42 ` Parav.Pandit
2012-06-19 14:11 ` Ben Hutchings
2012-06-27 9:08 ` Parav.Pandit
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=1340039376.2913.13.camel@bwh-desktop.uk.solarflarecom.com \
--to=bhutchings@solarflare.com \
--cc=netdev@vger.kernel.org \
--cc=parav.pandit@emulex.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.