From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 83253C25B76 for ; Sat, 8 Jun 2024 12:43:31 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=CWpB5wQmRaJw7bAZ/tkbe0ZrL9asBDF+bKqHj1qzRzo=; b=R9xfWpjJHPQZWW aPSXotSPhjpmg09b5Jfi4cMqUpEuqhWEwqdWIk2wKwaol6kMbdfGmvWF7Krn+SOju+h0dKjfGKl7y BFaTiEEYggwe8PKsWrSTpUTxZcs8xga1baXLf2xPozeMWy4dN11ov+VbryEM7dV2n1aHu7nw1gZs9 FIMBlYyGpkPbJZXjnsaQdvc9WJ64jEPDmrEfPbuvhqjWd64XmZ2RhqhsoKLFL263MYFlWBA0m7v+s T/FkQLxidpur/gvihyVEq9aebA0mwSoyF+g7w1d7YhrCdQcEgRUa4+2lI9ihi7zzX9JkYRgO3SV4J Wp9B5k01Zc8qzzzyLSyw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sFvPf-0000000HSkd-20Zf; Sat, 08 Jun 2024 12:43:15 +0000 Received: from mail-ej1-x630.google.com ([2a00:1450:4864:20::630]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sFvPc-0000000HSkA-1eVq for linux-arm-kernel@lists.infradead.org; Sat, 08 Jun 2024 12:43:14 +0000 Received: by mail-ej1-x630.google.com with SMTP id a640c23a62f3a-a6e0a499687so147316066b.2 for ; Sat, 08 Jun 2024 05:43:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1717850590; x=1718455390; darn=lists.infradead.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=u2jJpnazhyNnGAHA2J872EkU21KY+hirrOIRfP72bSc=; b=WqPayVOFWfmSk3dS6vmz/WW+Om4HDLP4kiyWKzl2TPivm3+/VuxNuHPULdurGdVEvv ALj5x4bKZLIGPM6je5yVjarNM+EGwpKeaHnK+/4hwqIVPD3B1KOA8oqB9SgDoSSG7sFG GJciX5C+mgbF6q/rwsz/wiRYGeNDVYyXBhYyQTiiBoxBeHzXnH3J11CT8fQ+vzV0SazR EMDpFosV+qe1WBZB1qFCB2MJvBe/TTuQevBdmeAUnd20s8ZA/yWRD3mG3cjYo4ds/LuA hIq4nO1orxfJuhCXtlFVWf/iVxKyhbjP7qPkEVcV4eNMzVgw5jrB6d3fRjQefXHblBqE UE9w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1717850590; x=1718455390; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=u2jJpnazhyNnGAHA2J872EkU21KY+hirrOIRfP72bSc=; b=hH05qvu9BbQSroFBx58pHtkMIyuX7psEwAMXcP86BFj7aCKyYUzu+cbo5errsBnZlJ uTyhA1RX+2GkdljPSCJ+fQZSAr5YtivFD+B1/RzPFYMMWnRlO287EoNAS3GfrYojColZ AXCYrnm3xSo8NKvpLl558JuLXqZDsYczdAkdKTNUdBU7/n3QfJtASLAdCmnBhKYtBGx7 hafWY+4mr6oHaXBwwlYvtIqQ7ZRrsMWwXQWzhN5psOzjbIZYr4H4UGh2nmX3ieFenfAV SN8ACACLTGNFi5HG82pkM0Lq3lraj8Ifevsq1383ZAe3t8sMNlRDA8374X3l0ggMdSO/ /nuQ== X-Forwarded-Encrypted: i=1; AJvYcCUIod5566pC9+QpXSzVPcJrmraC6js2pSXgpGGEsvbTvAyZVEJB61R6sssb3ta4zJeCa2tQDm4mkcAfcTLosMrerHUfI+NHJqptVRdk/Fo0Nvgbo5M= X-Gm-Message-State: AOJu0YydOsszRfnzECj61T0lqvCF40fuTkqh5sdovmbINuM7J786lewL VsS/CnEKsjPqo8WaYShKNIdHpOA4lqGTsG/Br0K8LYrj+GRKjTUf X-Google-Smtp-Source: AGHT+IGDEYouqAB0pPS44aRwekYh9i439dQuEXVnbRKmELsUCwf2icDlFZh9N5imcqNACwiXv9tutg== X-Received: by 2002:a17:906:25c5:b0:a6c:8bc3:aabd with SMTP id a640c23a62f3a-a6cd7d684e9mr310486466b.46.1717850589544; Sat, 08 Jun 2024 05:43:09 -0700 (PDT) Received: from skbuf ([188.25.55.166]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-a6f10d1c044sm10633466b.163.2024.06.08.05.43.08 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 08 Jun 2024 05:43:09 -0700 (PDT) Date: Sat, 8 Jun 2024 15:43:06 +0300 From: Vladimir Oltean To: Xiaolei Wang Cc: linux@armlinux.org.uk, andrew@lunn.ch, alexandre.torgue@foss.st.com, joabreu@synopsys.com, davem@davemloft.net, edumazet@google.com, kuba@kernel.org, pabeni@redhat.com, mcoquelin.stm32@gmail.com, wojciech.drewek@intel.com, netdev@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [net v4 PATCH] net: stmmac: replace priv->speed with the portTransmitRate from the tc-cbs parameters Message-ID: <20240608124306.nh2olzpybffitw6w@skbuf> References: <20240608044557.1380550-1-xiaolei.wang@windriver.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240608044557.1380550-1-xiaolei.wang@windriver.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240608_054312_541517_CAB6FBD8 X-CRM114-Status: GOOD ( 24.58 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Sat, Jun 08, 2024 at 12:45:57PM +0800, Xiaolei Wang wrote: > The current cbs parameter depends on speed after uplinking, > which is not needed and will report a configuration error > if the port is not initially connected. The UAPI exposed by > tc-cbs requires userspace to recalculate the send slope anyway, > because the formula depends on port_transmit_rate (see man tc-cbs), > which is not an invariant from tc's perspective. Therefore, we > use offload->sendslope and offload->idleslope to derive the > original port_transmit_rate from the CBS formula. > > Fixes: 1f705bc61aee ("net: stmmac: Add support for CBS QDISC") > Signed-off-by: Xiaolei Wang > Reviewed-by: Wojciech Drewek > --- > > Change log: > > v1: > https://patchwork.kernel.org/project/linux-arm-kernel/patch/20240528092010.439089-1-xiaolei.wang@windriver.com/ > v2: > Update CBS parameters when speed changes after linking up > https://patchwork.kernel.org/project/linux-arm-kernel/patch/20240530061453.561708-1-xiaolei.wang@windriver.com/ > v3: > replace priv->speed with the portTransmitRate from the tc-cbs parameters suggested by Vladimir Oltean > link: https://patchwork.kernel.org/project/linux-arm-kernel/patch/20240607103327.438455-1-xiaolei.wang@windriver.com/ > v4: > Delete speed_div variable, delete redundant port_transmit_rate_kbps = qopt->idleslope - qopt->sendslope; and update commit log > > .../net/ethernet/stmicro/stmmac/stmmac_tc.c | 20 +++++++------------ > 1 file changed, 7 insertions(+), 13 deletions(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c > index 222540b55480..87af129a6a1d 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c > @@ -344,10 +344,11 @@ static int tc_setup_cbs(struct stmmac_priv *priv, > { > u32 tx_queues_count = priv->plat->tx_queues_to_use; > u32 queue = qopt->queue; > - u32 ptr, speed_div; > + u32 ptr; > u32 mode_to_use; > u64 value; > int ret; > + s64 port_transmit_rate_kbps; > > /* Queue 0 is not AVB capable */ > if (queue <= 0 || queue >= tx_queues_count) > @@ -355,27 +356,20 @@ static int tc_setup_cbs(struct stmmac_priv *priv, > if (!priv->dma_cap.av) > return -EOPNOTSUPP; > > + port_transmit_rate_kbps = qopt->idleslope - qopt->sendslope; > + > /* Port Transmit Rate and Speed Divider */ > - switch (priv->speed) { > + switch (div_s64(port_transmit_rate_kbps, 1000)) { > case SPEED_10000: > - ptr = 32; > - speed_div = 10000000; > - break; > case SPEED_5000: > ptr = 32; > - speed_div = 5000000; > break; > case SPEED_2500: > - ptr = 8; > - speed_div = 2500000; > - break; > case SPEED_1000: > ptr = 8; > - speed_div = 1000000; > break; > case SPEED_100: > ptr = 4; > - speed_div = 100000; > break; > default: > return -EOPNOTSUPP; I have one more request. It is very discouraging for a user to give invalid parameters and receive -EOPNOTSUPP, because this is indicative of other "usual suspects": "did I enable CONFIG_NET_SCH_CBS?" It is unfortunate that struct tc_cbs_qopt_offload does not carry a netlink extack, but I'm also not requesting you to add one here. Instead, please at least change the return code to something like -EINVAL, and print something to the console along the lines of: "Invalid portTransmitRate %lld (idleSlope - sendSlope)\n". _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel