From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out-178.mta1.migadu.com (out-178.mta1.migadu.com [95.215.58.178]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id E2C1415E96 for ; Wed, 30 Jul 2025 11:32:53 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=95.215.58.178 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1753875175; cv=none; b=MOqnncQpvEUsIQWppNFQTk0Sgg0x9us8ptRT4y1/7rZmewpPR9+ltp7JPoyAxGxsuElL+cIcIUbAsUNrwPnvCIiV6vOwGIMyBuT7k0YmaocYcU5TxXqGqXhLnOTxbz5pTd+AklD4933faRuwcxlkKzRxEOPnohF5YxwMEls99CQ= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1753875175; c=relaxed/simple; bh=nsC0IZElzNqZiVyzWCZWUL4M9nsnN0tzZgv/WMalr04=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=EfG1QdGQbDZYneJgogssoz2Kf7cO02KKHJsZX9v1X/l7ktkaaEGcfeTJ7hICd7xXQ7Y6XKnii4ABNZZ+fAORRsggLXF8/84qiQi+3Tg1aayC2Iq/iS/qQH4gbbi3TCd/f8WcHh0nqPCobMIjtE+uduJLQ1lNm8wleBDnHnogizU= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=bukVWX11; arc=none smtp.client-ip=95.215.58.178 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="bukVWX11" Message-ID: <8b22e9d3-e4d2-4726-9622-28881b2cd406@linux.dev> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1753875171; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=uHW3kJohF557YGobfQDpyiyx5O/H1BpxwxCIIzE1A1k=; b=bukVWX11iXapIW2wm/IWiowp//jiNbLbLJLMmIY9LeEDVHA1rcLhzj4rcYxfFIO1n2CefM vgUA28+Glk97rQsmJSHzdjLQq/Sa/4vUonVYXMUm27DYo6za5u5zR6RydbMBDLouMeuebL vxEv4qDovEnshA2Q1SJmunSYt9JQcto= Date: Wed, 30 Jul 2025 12:32:43 +0100 Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Subject: Re: [RFC PATCH] ethtool: add FEC bins histogramm report To: Gal Pressman , Andrew Lunn , Michael Chan , Pavan Chebbi , Tariq Toukan , intel-wired-lan@lists.osuosl.org, Donald Hunter , Jakub Kicinski Cc: Paolo Abeni , Simon Horman , netdev@vger.kernel.org References: <20250729102354.771859-1-vadfed@meta.com> <041f79a2-5f96-4427-b0e2-6a159fbec84a@nvidia.com> <1129bf26-273e-4685-a0b8-ed8b0e4050f3@linux.dev> <3e84a20e-87ea-413c-9e9d-950605a55bf6@nvidia.com> Content-Language: en-US X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Vadim Fedorenko In-Reply-To: <3e84a20e-87ea-413c-9e9d-950605a55bf6@nvidia.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Migadu-Flow: FLOW_OUT On 30/07/2025 11:42, Gal Pressman wrote: > On 30/07/2025 12:29, Vadim Fedorenko wrote: >> On 30/07/2025 06:54, Gal Pressman wrote: >>> On 29/07/2025 13:23, Vadim Fedorenko wrote: >>>> diff --git a/drivers/net/netdevsim/ethtool.c b/drivers/net/netdevsim/ >>>> ethtool.c >>>> index f631d90c428ac..7257de9ea2f44 100644 >>>> --- a/drivers/net/netdevsim/ethtool.c >>>> +++ b/drivers/net/netdevsim/ethtool.c >>>> @@ -164,12 +164,25 @@ nsim_set_fecparam(struct net_device *dev, >>>> struct ethtool_fecparam *fecparam) >>>>       ns->ethtool.fec.active_fec = 1 << (fls(fec) - 1); >>>>       return 0; >>>>   } >>>> +static const struct ethtool_fec_hist_range netdevsim_fec_ranges[] = { >>>> +    {  0,  0}, >>>> +    {  1,  3}, >>>> +    {  4,  7}, >>>> +    { -1, -1} >>>> +}; >>> >>> The driver-facing API works nicely when the ranges are allocated as >>> static arrays, but I expect most drivers will need to allocate it >>> dynamically as the ranges will be queried from the device. >>> In that case, we need to define who is responsible of freeing the ranges >>> array. >> >> Well, the ranges will not change during link operation, unless the type >> of FEC is changed. You may either have static array of FEC ranges per >> supported FEC types. Or query it on link-up event and reuse it on every >> call for FEC stats. In this case it's pure driver's responsibility to >> manage memory allocations. There is definitely no need to re-query >> ranges on every single call for stats. > > This is just adding unnecessary complexity to the drivers, trying to > figure out the right lifetime for this array. > It will be much simpler if the core passes an array for the drivers to > fill. That way both static and dynamic ranges would work the same. There is no need to pass huge pre-allocated array for drivers which doesn't have support for the histogram. The core doesn't have this info about the driver. And again - there is no need to refill ranges array every time as it will not change. I believe it's pure driver area of knowledge and responsibility.