From patchwork Wed Nov 30 21:00:52 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Dumazet X-Patchwork-Id: 701198 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3tTXvh0xwrz9t1L for ; Thu, 1 Dec 2016 08:03:44 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="CS1z2ods"; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758731AbcK3VDa (ORCPT ); Wed, 30 Nov 2016 16:03:30 -0500 Received: from mail-pg0-f66.google.com ([74.125.83.66]:34439 "EHLO mail-pg0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757050AbcK3VBy (ORCPT ); Wed, 30 Nov 2016 16:01:54 -0500 Received: by mail-pg0-f66.google.com with SMTP id e9so2334239pgc.1 for ; Wed, 30 Nov 2016 13:01:54 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=message-id:subject:from:to:cc:date:in-reply-to:references :mime-version:content-transfer-encoding; bh=REeyo782Hzqm5nmgfta9K1uXRiWqmnj+SijZu2gmXCs=; b=CS1z2odsuIUGs6v/tVR6bcmSG5/8KuiNhr4KdO7QnVKO3332k0ircRTOYTRjGrIJYa SVWloXnxytogSVf1el+VVWZC4xXHPQeuWVZLJK+JFJyk8ttLONemR5Dz5GfC71xqbrJO QbdvwVKCU8q16n+ZS+x5RwytIKGdwwAjVGkUYfGyk820hMSsxz25F+rba9XOIlZL8sYT x90NClk+qa1ukhw6oXfL4B41w0gkQFwwE4mYhENF6O/1Rjcw2HoG88LlkVY4wIw006rh TOEfzHsLoACqCvmBaPdnzxMETgNkQIBnWWxp1Kev5yupAfGG+eCof3+juKy2D1FlHbDV 0Fgg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:message-id:subject:from:to:cc:date:in-reply-to :references:mime-version:content-transfer-encoding; bh=REeyo782Hzqm5nmgfta9K1uXRiWqmnj+SijZu2gmXCs=; b=hpleGE2BNMDTRin99Gh+1a7YoxBCdhIVNMWrn+7+KuL82IPwICh++zjfrsty9LQ1Bt aUXQdSj7koV7QAtc2VqVgwGpC7qwx84X6/N3AJpCg6ZOtkrFqIA/SqeVuc3YcB7Hkkir bkxeHJvyrtjR++kFGQcl27uwemNCrCuT1U2KBA3UC9/DyQCciDWTC6zT/jBVlltlHX6o XRSoBThQz6NyngPhTtud9lwzIdgi2k40sY1IjGCppO3llEyfeYr2//ayY55hzBCT5x9e vQKcYV/gIzTXZX6bszCoOgJZ9QJl9EotlnVtyRc8N2pUCCgIwjLm5RT1GjQRUac7u0D1 BP5w== X-Gm-Message-State: AKaTC00p87L3gAP7Z5W6ZWbv+BL5lSjmhzMhFzlmhBmXKMqw3JazonOur+O367IQQGbPNA== X-Received: by 10.99.112.66 with SMTP id a2mr62831559pgn.43.1480539713774; Wed, 30 Nov 2016 13:01:53 -0800 (PST) Received: from [172.19.22.200] ([172.19.22.200]) by smtp.googlemail.com with ESMTPSA id n17sm104950456pfg.80.2016.11.30.13.01.53 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 30 Nov 2016 13:01:53 -0800 (PST) Message-ID: <1480539652.18162.205.camel@edumazet-glaptop3.roam.corp.google.com> Subject: Re: Regression: [PATCH] mlx4: give precise rx/tx bytes/packets counters From: Eric Dumazet To: Saeed Mahameed Cc: Jesper Dangaard Brouer , David Miller , netdev , Tariq Toukan Date: Wed, 30 Nov 2016 13:00:52 -0800 In-Reply-To: References: <1480088780.8455.543.camel@edumazet-glaptop3.roam.corp.google.com> <20161130150839.5203ece0@redhat.com> <1480521514.18162.191.camel@edumazet-glaptop3.roam.corp.google.com> <1480527321.18162.196.camel@edumazet-glaptop3.roam.corp.google.com> X-Mailer: Evolution 3.10.4-0ubuntu2 Mime-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Wed, 2016-11-30 at 22:42 +0200, Saeed Mahameed wrote: > On Wed, Nov 30, 2016 at 7:35 PM, Eric Dumazet wrote: > > On Wed, 2016-11-30 at 18:46 +0200, Saeed Mahameed wrote: > > > >> we had/still have the proper stats they are the ones that > >> mlx4_en_fold_software_stats is trying to cache into (they always > >> exist), > >> but the ones that you are trying to read from (the mlx4 rings) are gone ! > >> > >> This bug is totally new and as i warned, this is another symptom of > >> the real root cause (can't sleep while reading stats). > >> > >> Eric what do you suggest ? Keep pre-allocated MAX_RINGS stats and > >> always iterate over all of them to query stats ? > >> what if you have one ring/none/1K ? how would you know how many to query ? > > > > I am suggesting I will fix the bug I introduced. > > > > Do not panic. > > > > > > Not at all, I trust you are the only one who is capable of providing > the best solution. > I am just trying to read your mind :-). > > As i said i like the solution and i want to adapt it to mlx5, so I am > a little bit enthusiastic :) What about the following fix guys ? As a bonus we update the stats right before they are sent to monitors via rtnetlink ;) diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c index 12ea3405f442717478bf0e8882edaf0de77986cb..091b904262bc7932d3edf99cf850affb23b9ce6e 100644 --- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c +++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c @@ -1809,8 +1809,12 @@ void mlx4_en_stop_port(struct net_device *dev, int detach) netif_tx_disable(dev); + spin_lock_bh(&priv->stats_lock); + mlx4_en_fold_software_stats(dev); /* Set port as not active */ priv->port_up = false; + spin_unlock_bh(&priv->stats_lock); + priv->counter_index = MLX4_SINK_COUNTER_INDEX(mdev->dev); /* Promsicuous mode */ diff --git a/drivers/net/ethernet/mellanox/mlx4/en_port.c b/drivers/net/ethernet/mellanox/mlx4/en_port.c index c6c4f1238923e09eced547454b86c68720292859..9166d90e732858610b1407fe85cbf6cbe27f5e0b 100644 --- a/drivers/net/ethernet/mellanox/mlx4/en_port.c +++ b/drivers/net/ethernet/mellanox/mlx4/en_port.c @@ -154,7 +154,7 @@ void mlx4_en_fold_software_stats(struct net_device *dev) unsigned long packets, bytes; int i; - if (mlx4_is_master(mdev->dev)) + if (!priv->port_up || mlx4_is_master(mdev->dev)) return; packets = 0;