Received-SPF: pass (google.com: domain of linux-kernel+bounces-195925-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <3d1cf564-c77b-4c8b-95ba-e33658596a93@intel.com>
Date: Thu, 30 May 2024 14:11:19 -0700
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH net-next v6 3/3] net/mlx4: support per-queue statistics
 via netlink
To: Joe Damato <jdamato@fastly.com>, <linux-kernel@vger.kernel.org>,
	<netdev@vger.kernel.org>
CC: <nalramli@fastly.com>, <mkarsten@uwaterloo.ca>, Tariq Toukan
	<tariqt@nvidia.com>, "David S. Miller" <davem@davemloft.net>, Eric Dumazet
	<edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni
	<pabeni@redhat.com>, "open list:MELLANOX MLX4 core VPI driver"
	<linux-rdma@vger.kernel.org>
References: <20240528181139.515070-1-jdamato@fastly.com>
 <20240528181139.515070-4-jdamato@fastly.com>
Content-Language: en-US
From: Jacob Keller <jacob.e.keller@intel.com>
In-Reply-To: <20240528181139.515070-4-jdamato@fastly.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?VlBlYkQvd0tnalArZ0lINDYwbmtvVnNmcUZpeHNMcDZZeVAzWFIwLzlJamJF?=
 =?utf-8?B?WmlXa1QrckxOazJEVUpnYjh0UlVZakdxWURJSlRiSnB2dEhkWm1jNlg0SmZE?=
 =?utf-8?B?Q0pDR2tHL2lWM1NZbXdHRjQxUVEybWFWNkNvYjk0QnZTNTZWTXZONTU3Y1J1?=
 =?utf-8?B?Vm10RWx0alJnTGxsZDIyMWp6ZCsrRVdQMW1QZ2tjVEtYenZROE9RMkFRWDRI?=
 =?utf-8?B?N2ExVHhpSVU3OC8wdms5R3VKeWxkNE1XWkhpSmJ0aXdtSUxIWVoxYTBteFhj?=
 =?utf-8?B?V2RuMUFSdUxmTndoUlJYWmQvRzh2VVozdVpqQjlCR2IwWm1DZzhCZ3JnT0U5?=
 =?utf-8?B?d0pIemtONkZ2UFpqWmI0MkVjZHBwVHF5VmhmU3pCN3RWbURFZ0dvUlcwYllE?=
 =?utf-8?B?Nm9ISDYyZVk2TUhXZ1pDR3Y3cG9lK2M0SkdvZmdnMnhROVorVm1FSG4wR3Yx?=
 =?utf-8?B?cGtXR05xT3JRdkxVRGVpa2RLS2ZQVlFEYVlGYTRiQ3ZnT3FlbVlCWmlKYlJY?=
 =?utf-8?B?cFV4YlJjZHBUUjd6aUZkTVB5MHJuYjJwOE5xNGI4bDBkTkZ3Z0xoM2FiNGk2?=
 =?utf-8?B?K0JHbUM2cndpZXFDUGRMU01YM3pCbWJoTUNSNnZzdnlCTll2SWpPQ2JaeWQw?=
 =?utf-8?B?ZFdwS29ib0dlWlFjelNIbnV1NEVIdE45bXRmaXNXVWdsSkhPNzFkVnNCNndH?=
 =?utf-8?B?eDJQa3J5bDdPdG9hMk1ZNjhERlR0S1NrY1dIQ29VbUlqV1cvZ2k0SWhUbVpw?=
 =?utf-8?B?U2Q5Z3NoWld5MFRORXQyTmFlQUhvTUZHbVpubjJ5ZFZFU01VbzBiQ1ZvcXBs?=
 =?utf-8?B?cGoxQjhoT1hIN3NyN1RNbnRLNDlDZVY0VXhMdTNkOGdVTE4yUnZEVzRZZURq?=
 =?utf-8?B?UFpScHVBbXJaU0lVQkFtaXVoSGpSZmpOUHJzZmdwV0xQRktqVCtPZUxFNTJN?=
 =?utf-8?B?MnlJSGtUaDRtZjBJS0w1YUJpL3FHVDA2UEZyaHZpTmVQQXlrMFg1UWFqN0k0?=
 =?utf-8?B?RlFVQ2tScUNUT0UxRE41dW5LbkFzajhwOGJ4NFpCL0ttWEZ0Vll4QmdIV0Jp?=
 =?utf-8?B?NmFoaWZOWDh1R1d1K0RqSWEwb29jSGs4QlVubm5DUHlrN3FxMEE1b3hlTHkv?=
 =?utf-8?B?aUpxMTFRcXdyNTZkN2xWaUhCZk5FaytQNWR2N25iYzBOYlMzYVhJS2E4QXpR?=
 =?utf-8?B?WDJZWksrK1cxZEYvYitTeUlhTktDVnFzc0RJeUFIbTVWbWFiWjdiWXR3NHRR?=
 =?utf-8?B?MFU3Yi9uM0IwYzYvV05XcG5YNXlDZU9MUHltbVRWVDhOL1ZINXd5eDA5Tk8r?=
 =?utf-8?B?L0wrbFhNWSs0WkRteEhRZzNubTkvdjcxT056cGFYaVI3S1U3eUtZRWpvejF6?=
 =?utf-8?B?YmVZYjA0L2FCOGhhV3pzVlNkN2dhVFpqWmNKVUlOeE8wUzZvTXZJbzRZbnhy?=
 =?utf-8?B?Q1hzMkIyVHM1VDRobi8zR1h0NnlsanpvQUQvODVuci9JNU1ieHNtcEdQTCsy?=
 =?utf-8?B?RmtuemZOdDR2bDNibno0TEF2T1QwaTNhTlhVT0JpNkgyMTRkUURFMy9VTVNz?=
 =?utf-8?B?UnpicXVucXBONzBRQ2hFTGRiR3BCRGtRUml0SlNhRGVFOE1TVEhWS1pHcXVt?=
 =?utf-8?B?eGxMMnpmQ0VpTG9Dc0I2Qk9QTjBZbjBHTXk2OHMrcXJjUzkyaEhiSjJyOW9w?=
 =?utf-8?B?QUY0WlNOemgrY2x6L1liRE5oeEs5OTVmYTFFTm1Fd3V5c3liK0VaWUNCMExu?=
 =?utf-8?B?UnpIV1FhL3pTRzZWZE9KNGl1NEo4TlRVWHZVS050TDV6VS9oSTJmNHlEUSt3?=
 =?utf-8?B?M2ZXcDVvOG5kcVdENHB1MGxRUzFwYnlPcStUeUY3RHE3bGpycDlRZHlqZzhW?=
 =?utf-8?B?MGlpSWd2YmNQOUR1c0hqQzVzMit2RXFCWXZUdXg4Z0tZNWJ6WHNDZmV4cGlO?=
 =?utf-8?B?Wkg3MXdvbEp2ZWUzZ3UzVHBhN0dTcVFkczJFMGlwWnFFNlBKNHU4YlUzUjBY?=
 =?utf-8?B?VlUra1ZaNkNrTSt2UEVDZFc2clAzTHRKWTdGZW9IY2RlZGVueDQydmVLR0Zp?=
 =?utf-8?B?cDVFM0dHOE56azlxOGZ1L1VNZnZ0REhZSFE4cG5pdkUvQU16cE13U3A4aUJF?=
 =?utf-8?B?RkNwU2dyalBiTWJnQ3FzaUZrNlJ1eXZQUnE3eUhFN241MlpCSEtEaGtrTUNM?=
 =?utf-8?B?OVE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 39b81e2f-5ae8-45ee-fe48-08dc80ed0dc3
X-MS-Exchange-CrossTenant-AuthSource: CO1PR11MB5089.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 May 2024 21:11:20.7022
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: S44Pbc7w9RfWMvYz89bRu91Kjusu7R3Lnt3uW0A951eOaqXpWRvLf5UdOO9OxCLyD265iSHdTAagygagfQpuIR7sIbcSZq4+NExzgUpmFTQ=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM3PR11MB8760
X-OriginatorOrg: intel.com


On 5/28/2024 11:11 AM, Joe Damato wrote:
> Make mlx4 compatible with the newly added netlink queue stats API.
> 
> Signed-off-by: Joe Damato <jdamato@fastly.com>
> Tested-by: Martin Karsten <mkarsten@uwaterloo.ca>
> Reviewed-by: Tariq Toukan <tariqt@nvidia.com>
> ---
>  .../net/ethernet/mellanox/mlx4/en_netdev.c    | 73 +++++++++++++++++++
>  1 file changed, 73 insertions(+)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> index 4d2f8c458346..281b34af0bb4 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> @@ -43,6 +43,7 @@
>  #include <net/vxlan.h>
>  #include <net/devlink.h>
>  #include <net/rps.h>
> +#include <net/netdev_queues.h>
>  
>  #include <linux/mlx4/driver.h>
>  #include <linux/mlx4/device.h>
> @@ -3100,6 +3101,77 @@ void mlx4_en_set_stats_bitmap(struct mlx4_dev *dev,
>  	last_i += NUM_PHY_STATS;
>  }
>  
> +static void mlx4_get_queue_stats_rx(struct net_device *dev, int i,
> +				    struct netdev_queue_stats_rx *stats)
> +{
> +	struct mlx4_en_priv *priv = netdev_priv(dev);
> +	const struct mlx4_en_rx_ring *ring;
> +
> +	spin_lock_bh(&priv->stats_lock);
> +
> +	if (!priv->port_up || mlx4_is_master(priv->mdev->dev))
> +		goto out_unlock;
> +
> +	ring = priv->rx_ring[i];
> +	stats->packets = READ_ONCE(ring->packets);
> +	stats->bytes   = READ_ONCE(ring->bytes);
> +	stats->alloc_fail = READ_ONCE(ring->alloc_fail);
> +

Ahh, allocation failures is reported here. Ok. I probably would have
re-ordered patches so that the implementation of alloc_fail came after
and it was a bit more obvious how it would be used.

Either way, I don't think that deserves a resend.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>