netdev - Re: [PATCH] net: added support for 40GbE link.

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]

Date:	Mon, 18 Jun 2012 18:09:36 +0100
From:	Ben Hutchings <bhutchings@...arflare.com>
To:	Parav Pandit <parav.pandit@...lex.com>
CC:	<netdev@...r.kernel.org>
Subject: Re: [PATCH] net: added support for 40GbE link.

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.

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