Received-SPF: pass (google.com: domain of linux-kernel+bounces-195926-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: <6cf3a91b-7449-4aec-8995-15088e9e9346@intel.com>
Date: Thu, 30 May 2024 14:12:13 -0700
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH net-next v6 1/3] net/mlx4: Track RX allocation failures in
 a stat
From: Jacob Keller <jacob.e.keller@intel.com>
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-2-jdamato@fastly.com>
 <e1efc17f-e4a9-4952-9ad0-7671b72fd37c@intel.com>
Content-Language: en-US
In-Reply-To: <e1efc17f-e4a9-4952-9ad0-7671b72fd37c@intel.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?enp3SU5Pb1VCMWJiU1NIcEdybExpVWRqVVFpM016aFhSaHVFODR4cjQvNlJa?=
 =?utf-8?B?M1FCWkpzaG5TVVU4QUl2NFBsa0xLeE9aT2g0QWtxZjhUR2ljUXJtalVqTGRU?=
 =?utf-8?B?T1RRYk5CVHg1UDZzZjk0K1pqWXY2SU5zT1hQNEtEOFNvbVc2NjN5LzdyTjk1?=
 =?utf-8?B?NzdmSS9HcHRrMDc1MkhIVnhzVU96S0tqem9jQm5kakNmbWF2b3l0TWtTNUhH?=
 =?utf-8?B?RnN0YXJhRVBmODZ4a3VaL3FkTkdtbXhtL3lrbGJLQ1Z4aUF5MDJJSFdISGJl?=
 =?utf-8?B?ZnFZcjR3YTQ4UWY3SWh4YldyV0tJanpYcTl6cG4vY1hKMklWY25KZnpET256?=
 =?utf-8?B?czE1SHM4NCtzYTh5ZUNrdjVYeG1FZmxqZmlKaUh3WlJ5NWtTbklvWTlhMFh5?=
 =?utf-8?B?ODI4cUdNYnRrdm5xbDZpZ1lteXh0WTZWcnEycVVSSWlIWnk1UXVONUhqbDdQ?=
 =?utf-8?B?VWtGVTJyOWxaQmJXRUxPRURuNU5maG9jTDh3WUZaOWVEbjU4WUlFS3owR2Nu?=
 =?utf-8?B?bGxVTEhNNVhoL01Tb2dlUlh1M0FIY3Nab0FtbHdoelZOYVgrZ2F2dlM1UGhR?=
 =?utf-8?B?WFFwN2pRNGhQbkM5ek0zeW0wU1hpak1vVjJickNFVjY0L1c2cVBsaEk2cUx3?=
 =?utf-8?B?MW9lSnJMeWEwYlltcEZzY0R1M1pkY0pvNHBza3gva1V4Nm5mN21hcG55Vjg0?=
 =?utf-8?B?NURLZGdFc1dXdW8wZmRxWUFhVDdEc1ZEaEliTGxXRUgyZWVkbGYxQXp5TnE5?=
 =?utf-8?B?WVVBL0xZenF5dGVOZDlQU0t0ZzM0Sjhrd0RxT2tGOG43MFFzMC9qRDRNS1R2?=
 =?utf-8?B?WXZvTXNpZktlaWU0SGNXNDVTcUlJSDVOT3p2WFdzc3UvZ3NpN2RGTkQ5cVNB?=
 =?utf-8?B?dFFMUldJSklvNkpkem1hQ3VFeTQycmhJb2gvc2hIV3czWXVLdzVEdGc3RWgw?=
 =?utf-8?B?QW5wMWovSjdNeUh2MitPUCswTVRyRXBmUlB1eUNsMXhtVm1LMy95cWhrUDVj?=
 =?utf-8?B?Mm5hT2E4MnU5anI0KzhvNjZQR3dCWEtTYm5weVB4c2lRWTVzNkVmZGl3OXpq?=
 =?utf-8?B?VUIwL2tSdTArWHZ0Y3JoTlFkLzJ2cGZmdUtGYmw5blFrd0JaUEZ1d3g5SnUz?=
 =?utf-8?B?dFZtSy9LS0NubzZWWUE3YU9Dbnh6dGwwZVR4djI2dnNpSkxUS1AvQ3VVdXd0?=
 =?utf-8?B?NHVlalY4ZlEvV3o3MnlBVEFCZ0VaeHBuZGx6dDhzamJZTUtLU3ZzN3dkNGlZ?=
 =?utf-8?B?cURBeHlaMm12WkNrN25aRGk0eHdHYWdmSElpcVRUbWtXRktkVHNkVGJ2SmpG?=
 =?utf-8?B?aHhkOEhHNlErSHM2TmEvdmt0SEhFRFJVaG9wKytrLy9Vc3RWcVVRSVlQN1Z2?=
 =?utf-8?B?c2h6bXZocGlVRWlINStMSmluKzcvb0dvNW53TWtZMks3K1VmQkoxYWxpNStV?=
 =?utf-8?B?K1Evc3YzRDRXT2hUNjJKOTd6YnU2UFB5Z2JiZTFHUCs5VnA0WE1EdlQzd29Z?=
 =?utf-8?B?Uy8ydjlQTEhtT3NaSmFZUzg2cy9IOGJFUG1Tejc5TnJHL1dJK0pSdS9UcTM2?=
 =?utf-8?B?Tm8wVk40TFNqd0RucVVEZUdta05ZdFRudmp2SzVGc0xRY1ZBd0dBanlXMHVZ?=
 =?utf-8?B?am0xckF1eFAzN3o4em1tWXJmYVZWNWRvTll6M3M4MHZ2aGhva0NNSUlnWUdH?=
 =?utf-8?B?amxiWEtwRTZEcHFTdjNRVk8yeVN2YUo0d3BnUG05bktUU1BZSlBNVDdGMVVW?=
 =?utf-8?B?TnV3WDFZUlhZVUdpUmgxTHNBVmsxWW9kdHBZTDB0aG53Mkg4RUsvc1ErNFpJ?=
 =?utf-8?B?TnZNeFFZb2dOM1FFVnJqdUZ6KzJoOWE2Wk9HWis3ejcrVU5VODhjb3RrQjRm?=
 =?utf-8?B?aU51ZWdJcEV5OVJNTGtKY2ZidTB2KzdXeVpEaERjQkpQYmpEQlR0YTVTKy9Y?=
 =?utf-8?B?bVdPNS82N2pGT3h1VzlEa0FST2dZeEhYV1RXL0lVZEJuV0RzQlcwU1EvbFJI?=
 =?utf-8?B?YkxPS1hVZmFST3JUamJvTWZhQkt0SlhaRFVKZ1hkS014eXoxQWtUTzRnY0lK?=
 =?utf-8?B?WTA3MDRMRmlhcUdzOGQwRk1xcmZzRGtEQ3N3VVEweTYrNmxVWXNGWGpZQUhB?=
 =?utf-8?B?czYvSVRoVGd6dTRIRG1tMEdQRkhOZEZORnN5eDRzSXFGM3pEazV2QnlLaGp4?=
 =?utf-8?B?RFE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 20a6e22f-2260-449f-7e9a-08dc80ed2e30
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:12:15.1063
 (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: AxeGvnnWLFW+UxRDbB+65jW6h9wfyAt6GAVwGpke5L9/Zg6qRsDAQcIlveEnTDmr4gQGBCGzUy3qMtgmcpRBYNpc0qpjoZ1jKSCnB+A7zCA=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM3PR11MB8760
X-OriginatorOrg: intel.com


On 5/30/2024 2:08 PM, Jacob Keller wrote:
> 
> 
> On 5/28/2024 11:11 AM, Joe Damato wrote:
>> mlx4_en_alloc_frags currently returns -ENOMEM when mlx4_alloc_page
>> fails but does not increment a stat field when this occurs.
>>
>> A new field called alloc_fail has been added to struct mlx4_en_rx_ring
>> which is now incremented in mlx4_en_rx_ring when -ENOMEM occurs.
>>
>> Signed-off-by: Joe Damato <jdamato@fastly.com>
>> Tested-by: Martin Karsten <mkarsten@uwaterloo.ca>
>> Reviewed-by: Tariq Toukan <tariqt@nvidia.com>
>> ---
>>  drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 1 +
>>  drivers/net/ethernet/mellanox/mlx4/en_rx.c     | 4 +++-
>>  drivers/net/ethernet/mellanox/mlx4/mlx4_en.h   | 1 +
>>  3 files changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
>> index 4c089cfa027a..4d2f8c458346 100644
>> --- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
>> +++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
>> @@ -2073,6 +2073,7 @@ static void mlx4_en_clear_stats(struct net_device *dev)
>>  		priv->rx_ring[i]->csum_ok = 0;
>>  		priv->rx_ring[i]->csum_none = 0;
>>  		priv->rx_ring[i]->csum_complete = 0;
>> +		priv->rx_ring[i]->alloc_fail = 0;
>>  	}
>>  }
>>  
>> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
>> index 8328df8645d5..15c57e9517e9 100644
>> --- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
>> +++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
>> @@ -82,8 +82,10 @@ static int mlx4_en_alloc_frags(struct mlx4_en_priv *priv,
>>  
>>  	for (i = 0; i < priv->num_frags; i++, frags++) {
>>  		if (!frags->page) {
>> -			if (mlx4_alloc_page(priv, frags, gfp))
>> +			if (mlx4_alloc_page(priv, frags, gfp)) {
>> +				ring->alloc_fail++;
>>  				return -ENOMEM;
>> +			}
>>  			ring->rx_alloc_pages++;
>>  		}
>>  		rx_desc->data[i].addr = cpu_to_be64(frags->dma +
>> diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
>> index efe3f97b874f..cd70df22724b 100644
>> --- a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
>> +++ b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
>> @@ -355,6 +355,7 @@ struct mlx4_en_rx_ring {
>>  	unsigned long xdp_tx;
>>  	unsigned long xdp_tx_full;
>>  	unsigned long dropped;
>> +	unsigned long alloc_fail;
>>  	int hwtstamp_rx_filter;
>>  	cpumask_var_t affinity_mask;
>>  	struct xdp_rxq_info xdp_rxq;
> 
> This patch does not appear to extend either a netdev, ethtool, devlink,
> or any other interface to report this new counter.
> 
> How is a user supposed to obtain this information from the driver?

Ah. Its used in the 3rd patch.

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