netdev - Re: [PATCH net 1/6] net/mlx4_en: Do not count dropped packets twice

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:	Tue, 16 Feb 2016 10:31:57 -0800
From:	Rick Jones <rick.jones2@....com>
To:	Or Gerlitz <ogerlitz@...lanox.com>,
	"David S. Miller" <davem@...emloft.net>
Cc:	netdev@...r.kernel.org, Eran Ben Elisha <eranbe@...lanox.com>,
	Yishai Hadas <yishaih@...lanox.com>,
	Amir Vadai <amir@...ai.me>,
	Eugenia Emantayev <eugenia@...lanox.com>
Subject: Re: [PATCH net 1/6] net/mlx4_en: Do not count dropped packets twice

On 02/16/2016 07:01 AM, Or Gerlitz wrote:
> From: Amir Vadai <amir@...ai.me>
>
> RdropOvflw counter was mistakenly copied into rx_missed_errors. Because
> of that it was counted twice for the device dropped packets accounting.
>
> Fixes: c27a02cd94d6 ('mlx4_en: Add driver for Mellanox ConnectX 10GbE NIC')
> Signed-off-by: Amir Vadai <amir@...ai.me>
> Signed-off-by: Eugenia Emantayev <eugenia@...lanox.com>
> Signed-off-by: Or Gerlitz <ogerlitz@...lanox.com>
> ---
>   drivers/net/ethernet/mellanox/mlx4/en_port.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_port.c b/drivers/net/ethernet/mellanox/mlx4/en_port.c
> index ee99e67..7b511a5 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_port.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_port.c
> @@ -242,7 +242,7 @@ int mlx4_en_DUMP_ETH_STATS(struct mlx4_en_dev *mdev, u8 port, u8 reset)
>   	stats->rx_crc_errors = be32_to_cpu(mlx4_en_stats->RCRC);
>   	stats->rx_frame_errors = 0;
>   	stats->rx_fifo_errors = be32_to_cpu(mlx4_en_stats->RdropOvflw);
> -	stats->rx_missed_errors = be32_to_cpu(mlx4_en_stats->RdropOvflw);
> +	stats->rx_missed_errors = 0;
>   	stats->tx_aborted_errors = 0;
>   	stats->tx_carrier_errors = 0;
>   	stats->tx_fifo_errors = 0;
>

I'm still not clear on when an Acked-by is appropriate, but given that 
this has been a non-trivial frustration for a long time, a hearty 
endorsement from me.  Perhaps not important enough but it would be nice 
to have it flow back a release or two.

That said, should mlx4_en_stats->RdropOvflw still be going into both 
rx_fifo_errors and rx_over_errors?

         stats->rx_over_errors = be32_to_cpu(mlx4_en_stats->RdropOvflw);
         stats->rx_crc_errors = be32_to_cpu(mlx4_en_stats->RCRC);
         stats->rx_frame_errors = 0;
         stats->rx_fifo_errors = be32_to_cpu(mlx4_en_stats->RdropOvflw);

happy benchmarking,

rick jones