Received-SPF: pass (google.com: domain of linux-kernel+bounces-171709-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Tue, 7 May 2024 17:44:27 +0200
From: Larysa Zaremba <larysa.zaremba@intel.com>
To: Joe Damato <jdamato@fastly.com>
CC: <linux-kernel@vger.kernel.org>, <netdev@vger.kernel.org>,
	<tariqt@nvidia.com>, <saeedm@nvidia.com>, <mkarsten@uwaterloo.ca>,
	<gal@nvidia.com>, <nalramli@fastly.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>
Subject: Re: [PATCH net-next v3 1/3] net/mlx4: Track RX allocation failures
 in a stat
Message-ID: <ZjpMW7KKdtfXv2dd@lzaremba-mobl.ger.corp.intel.com>
References: <20240502212628.381069-1-jdamato@fastly.com>
 <20240502212628.381069-2-jdamato@fastly.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20240502212628.381069-2-jdamato@fastly.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?UxgrjCtwKu4jT5BeWoKQIAwCcmxIbwywQj7buYsb5yHyBTgiC4qnOeZh/c4h?=
 =?us-ascii?Q?ZsQJeW603fZWALIppsFgN8muMYDfU3HSdKPhvsHeX0HKLh4INXHBO7+pKEhg?=
 =?us-ascii?Q?eQ71eqiYcqSPWuRDJcepoEL70S5y0n7A/7CnJNrcf35QmZMGbK34SfyiqKkB?=
 =?us-ascii?Q?kxngsFBiJluj0qdXFeakHFqTXZB1SnV0C1G7j77IkY3TIPeZoHapo95EoBuf?=
 =?us-ascii?Q?DO1S5Nhvs/TdtGCviG0mf/rW2vB0U6sW6uzB/T++XIQbTIedwahg82q5sxkq?=
 =?us-ascii?Q?AYaeA3v+3xQQ9bRj5hGwYaWoE+Svi8Gt+HAtrjG3l10YxY8pcSvaUwMOprzV?=
 =?us-ascii?Q?vnnVhb09vwVxxihW+j+ZRsF2Cycpv/5w7YhXwhlVjPDCb2SV0m7GJWHnCzzN?=
 =?us-ascii?Q?uWs7KT80R/zooCoRCxhkSxj6h8hVtkgQ8X28mkVig66IljiIj5xcOr99RR66?=
 =?us-ascii?Q?mNTv6GUd1+PDHHrShIy7AvoVK6uWzaYHYpw9H1PhLW+kxWnuNX/27tJ/TPTE?=
 =?us-ascii?Q?sVLUWZye+0dYS134ReK1wXWK0yxFRx7S9JOtpmUJZqhx9laITvLnASk0urBa?=
 =?us-ascii?Q?9PfuPxKTYmiyw0GPbWjnupjsXzQ05huI144wMd3cu2RHbP2VfueUR2QklZLO?=
 =?us-ascii?Q?Ww/6SKmwJdQKertJzvAAx4wOi6e1rQ6MkwVlIi4DEAjqq8NLUSIzAFlZq8XF?=
 =?us-ascii?Q?av6UCMPam183/cGTGX25dv1I6nDMSpy3l4Royw+11/x6/n39Zn1dKmxAxFLQ?=
 =?us-ascii?Q?BwCyqHseWDyqwLUPaffWNYp9uzffpb+8VAAi0zVNssrGqbvYE8OQzKGWHCzz?=
 =?us-ascii?Q?pIENQ7HXsGxE14EYREsarEaAHwqXKIYuxiUtWkTZA3UbwZtiHxENuU6S8d1G?=
 =?us-ascii?Q?yuh6n3wCncNF4EIX9DtUxOtRqeYhztbNEG6S1pZ9lBz5Ktq2wr4LLkwwJ0zS?=
 =?us-ascii?Q?3dyJVndgTDEM5D2NuHPi5pX+PoWoW3Y/Ld6kNAomAccLKlO3HqL2R1adl0eZ?=
 =?us-ascii?Q?lRXxO3IlKl6zpRv6w0sC/p+fJqf7zllhRRSu3JJHweE2PDI2fiVt+4BE29Cs?=
 =?us-ascii?Q?NJ3bN4lZiZX1wpkkjxRmRtIUFOt1tfzgSSDmoJPU1GFPt/ry1jFhSN4oCl/j?=
 =?us-ascii?Q?f08WXy4pHn30DFBRWDiWisRjs9abaGeBSK1avFtZEqKQf/AJM45An4YGEmDq?=
 =?us-ascii?Q?EgGrvizmNtNWau2PniF26lkPETcHYhosDdynAYMxD3bbuJLPbN+Y0xx1B67a?=
 =?us-ascii?Q?2V/YR5KN8/xcbDsCng7nYGF0rbXktN98iZKCXU7D+j2piI29Nu8S78CrOc4T?=
 =?us-ascii?Q?yIsCwrCTV2vSafb581acyxoof98ZjMZv72PKApVnQPWkkaJHFlr9d5cAdfWg?=
 =?us-ascii?Q?HVfJ4DuRgugkbS/zzDDXiHVDjrLzqJ6wUs4QaBp73SUp62j4P4MUF1lsW01W?=
 =?us-ascii?Q?5XcFtwiElN/4pcka83g8q99uRX7U2eb5Mka/WOQsQt5n8fbPk0A3zagFtXFd?=
 =?us-ascii?Q?TR9M5Heq/wtF5u1SBGOnovVvz0owfX/6x78e1A46YDLzGSGFU+qUWZvVDP+3?=
 =?us-ascii?Q?Nzl2y3GZYUdHMhpdIZh2iRfG6mjW5mvjc+Ge6ZdU6ey0io+B4e/FdZcCpU8M?=
 =?us-ascii?Q?hA=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 495043be-4800-4089-78aa-08dc6eac9956
X-MS-Exchange-CrossTenant-AuthSource: SN7PR11MB7540.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 May 2024 15:44:36.7453
 (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: In0FJef1N0A/fg/i4gID4UFiHI9dDndex25hXeWzrtabNnXZiL94lGw6RTMS0eZu3DPIBBs+PoP04IdhCHqIFpJ9Xnmbi+mJf2t8XxQZntU=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR11MB6266
X-OriginatorOrg: intel.com

On Thu, May 02, 2024 at 09:26:25PM +0000, 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.
> 
> struct mlx4_en_rx_ring has a dropped field which is tabulated in
> mlx4_en_DUMP_ETH_STATS, but never incremented by the driver.
> 
> This change modifies mlx4_en_alloc_frags to increment mlx4_en_rx_ring's
> dropped field for the -ENOMEM case.
> 
> Signed-off-by: Joe Damato <jdamato@fastly.com>
> Tested-by: Martin Karsten <mkarsten@uwaterloo.ca>
> ---
>  drivers/net/ethernet/mellanox/mlx4/en_rx.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> index 8328df8645d5..573ae10300c7 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->dropped++;
>  				return -ENOMEM;
> +			}

Correct me if I'm wrong, but ring->dropped is added to rx_dropped stats in 
mlx4_en_DUMP_ETH_STATS(). You have already established with Jakub that 
allocation error does not mean dropped packet, but the counter contributes to 
dropped packets stats.

Also, I do not think that using a `dropped` counter for something that does not 
neccessarily result in a dropped packet is plain confusing.

>  			ring->rx_alloc_pages++;
>  		}
>  		rx_desc->data[i].addr = cpu_to_be64(frags->dma +
> -- 
> 2.25.1
> 
>