Message ID | 20231213181044.103943-1-sumang@marvell.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:7300:3b04:b0:fb:cd0c:d3e with SMTP id c4csp7969448dys; Wed, 13 Dec 2023 10:11:32 -0800 (PST) X-Google-Smtp-Source: AGHT+IFx0+rs/CF0aYauqfRLarehR3ArBK9PrHSMWSkOTBRZSvzKbIO1yMYmTSqokKp1RDlu+QhN X-Received: by 2002:a17:902:ee4d:b0:1d2:eb05:9d02 with SMTP id 13-20020a170902ee4d00b001d2eb059d02mr4423962plo.123.1702491092592; Wed, 13 Dec 2023 10:11:32 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1702491092; cv=none; d=google.com; s=arc-20160816; b=MB9H7vzyZQk7xUuZwi73R53rJjKcZWGZIrmD29D6jDwIjl1YzN+powD97fnY57eHBA 5tlPJNgzZgdA0ujeC5DgpmXpkcXtTtgzkcpNneG+V9KWCTxR3IRXXzWuEy4r8R0ATr0f GEbDzp+RiUwfwDwGj73NKFUAnZah0TYEGqHGUDUEZHfyNuAmB8nM9E8aH2YXEjyap9Ax RumhjJmLTV+A73wcIPXXvJ7hRj1of46zxEi5lDSZar3myYFRG+BFXffomEYQtGf9iFn0 r6zy16ie1uwCuzzZi0yKU0nNFsYDgoQgSZ0tt5cQkYKpUcaqwzwLLDVIWTzJbtn0B4lR 3Y9Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=9Hj0KLnBUeycwyof1iA+0mLw8fTdl+oosoVSFW+ctGw=; fh=pK51JAizWKp9jwq8870a7Zs4DJFrvE+hT0N4i+cGlR0=; b=Djh0bg6b4y8GLMGORcKY16ZpwQ6RdKLuqXjeGeqg547ooYlfvjQXqO3pZK3s5uolVm PlNxA73Lw1MkSM2A1+Sytv+UINq3kG5afrdJqWUFW+iJ5xrc8x+qMutucteLYCxB7Hr7 ILG5tlFw51I4KUmIdIeHnrS2nprmXqq1Jsg/djXkxcyG3UkjQhu7kAFn3TfPsMWZ4FMY bT2v4o6+tVFTMcZApMk54WeX84eVYGudITwVeTEp3td5Xfv0eLTcsX9NDRMEAhvXzBvc rGLfu11T90dmUKB2xuj/mPlpHYW98gSPFSMs36WY4Pb4nWp5QV/jk3LBNvW54wTTw69O 2aPg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@marvell.com header.s=pfpt0220 header.b=YiSd5hZC; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=REJECT dis=NONE) header.from=marvell.com Received: from snail.vger.email (snail.vger.email. [2620:137:e000::3:7]) by mx.google.com with ESMTPS id b2-20020a170902d50200b001d344a1673asi2436270plg.500.2023.12.13.10.11.32 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 13 Dec 2023 10:11:32 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7; Authentication-Results: mx.google.com; dkim=pass header.i=@marvell.com header.s=pfpt0220 header.b=YiSd5hZC; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=REJECT dis=NONE) header.from=marvell.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by snail.vger.email (Postfix) with ESMTP id 17FFC8090FB3; Wed, 13 Dec 2023 10:11:15 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at snail.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1442176AbjLMSLC (ORCPT <rfc822;dexuan.linux@gmail.com> + 99 others); Wed, 13 Dec 2023 13:11:02 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37946 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1442145AbjLMSK7 (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 13 Dec 2023 13:10:59 -0500 Received: from mx0b-0016f401.pphosted.com (mx0a-0016f401.pphosted.com [67.231.148.174]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7820DAF; Wed, 13 Dec 2023 10:11:05 -0800 (PST) Received: from pps.filterd (m0045849.ppops.net [127.0.0.1]) by mx0a-0016f401.pphosted.com (8.17.1.24/8.17.1.24) with ESMTP id 3BDBPTkC007464; Wed, 13 Dec 2023 10:10:53 -0800 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=marvell.com; h= from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding:content-type; s=pfpt0220; bh=9Hj0KLnB Ueycwyof1iA+0mLw8fTdl+oosoVSFW+ctGw=; b=YiSd5hZC0q3ooM0Me1HLmyAk rirR/TQIaDNsdEgmu+3kAY4uAl7dseWWMIzd5xBFE31X8Fw/1MtVFpvu+BgoTaei DeZQh0KfKhc3DrkW1cp6YhlGmmkwIrFjJo8tM50ktSBfgwRRBisc5sAKI0VBS6fy 3Kg8WcDz3+AvLEbVJ0LEggNXDwHnLPxsDOvnGUh8BSBlAQJS/t91SvDsBTfl0MN5 oc3BRAB5YMWCw8DgI0mSpunKn7ToYXMeH/VNnNm5FxbDaALSVjt8Ge2jeGtXmyfp NKVdmKyoKRiqI3xMhLvFLOsk5JnokyzRtYsItCj9WSG80aCwVrJ6EvbycxhXHw== Received: from dc5-exch02.marvell.com ([199.233.59.182]) by mx0a-0016f401.pphosted.com (PPS) with ESMTPS id 3uybqmhtb4-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-SHA384 bits=256 verify=NOT); Wed, 13 Dec 2023 10:10:53 -0800 (PST) Received: from DC5-EXCH01.marvell.com (10.69.176.38) by DC5-EXCH02.marvell.com (10.69.176.39) with Microsoft SMTP Server (TLS) id 15.0.1497.48; Wed, 13 Dec 2023 10:10:51 -0800 Received: from maili.marvell.com (10.69.176.80) by DC5-EXCH01.marvell.com (10.69.176.38) with Microsoft SMTP Server id 15.0.1497.48 via Frontend Transport; Wed, 13 Dec 2023 10:10:51 -0800 Received: from localhost.localdomain (unknown [10.28.36.166]) by maili.marvell.com (Postfix) with ESMTP id A62733F708C; Wed, 13 Dec 2023 10:10:47 -0800 (PST) From: Suman Ghosh <sumang@marvell.com> To: <davem@davemloft.net>, <edumazet@google.com>, <kuba@kernel.org>, <pabeni@redhat.com>, <sgoutham@marvell.com>, <sbhatta@marvell.com>, <jerinj@marvell.com>, <gakula@marvell.com>, <hkelam@marvell.com>, <lcherian@marvell.com>, <netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org> CC: Suman Ghosh <sumang@marvell.com> Subject: [net PATCH] octeontx2-pf: Fix graceful exit during PFC configuration failure Date: Wed, 13 Dec 2023 23:40:44 +0530 Message-ID: <20231213181044.103943-1-sumang@marvell.com> X-Mailer: git-send-email 2.25.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain X-Proofpoint-GUID: VdTMhOCCxuJki73Q6snNOiZI0IHMJ3KY X-Proofpoint-ORIG-GUID: VdTMhOCCxuJki73Q6snNOiZI0IHMJ3KY X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.272,Aquarius:18.0.997,Hydra:6.0.619,FMLib:17.11.176.26 definitions=2023-12-09_02,2023-12-07_01,2023-05-22_02 X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_BLOCKED, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Wed, 13 Dec 2023 10:11:15 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1785191299995016756 X-GMAIL-MSGID: 1785191299995016756 |
Series |
[net] octeontx2-pf: Fix graceful exit during PFC configuration failure
|
|
Commit Message
Suman Ghosh
Dec. 13, 2023, 6:10 p.m. UTC
During PFC configuration failure the code was not handling a graceful
exit. This patch fixes the same and add proper code for a graceful exit.
Fixes: 99c969a83d82 ("octeontx2-pf: Add egress PFC support")
Signed-off-by: Suman Ghosh <sumang@marvell.com>
---
.../ethernet/marvell/octeontx2/nic/otx2_dcbnl.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
Comments
Hello: This patch was applied to netdev/net.git (main) by David S. Miller <davem@davemloft.net>: On Wed, 13 Dec 2023 23:40:44 +0530 you wrote: > During PFC configuration failure the code was not handling a graceful > exit. This patch fixes the same and add proper code for a graceful exit. > > Fixes: 99c969a83d82 ("octeontx2-pf: Add egress PFC support") > Signed-off-by: Suman Ghosh <sumang@marvell.com> > --- > .../ethernet/marvell/octeontx2/nic/otx2_dcbnl.c | 17 ++++++++++++++--- > 1 file changed, 14 insertions(+), 3 deletions(-) Here is the summary with links: - [net] octeontx2-pf: Fix graceful exit during PFC configuration failure https://git.kernel.org/netdev/net/c/8c97ab5448f2 You are awesome, thank you!
On Wed, Dec 13, 2023 at 11:40:44PM +0530, Suman Ghosh wrote: > During PFC configuration failure the code was not handling a graceful > exit. This patch fixes the same and add proper code for a graceful exit. > > Fixes: 99c969a83d82 ("octeontx2-pf: Add egress PFC support") > Signed-off-by: Suman Ghosh <sumang@marvell.com> > --- > .../ethernet/marvell/octeontx2/nic/otx2_dcbnl.c | 17 ++++++++++++++--- > 1 file changed, 14 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_dcbnl.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_dcbnl.c > index bfddbff7bcdf..28fb643d2917 100644 > --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_dcbnl.c > +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_dcbnl.c > @@ -399,9 +399,10 @@ static int otx2_dcbnl_ieee_getpfc(struct net_device *dev, struct ieee_pfc *pfc) > static int otx2_dcbnl_ieee_setpfc(struct net_device *dev, struct ieee_pfc *pfc) > { > struct otx2_nic *pfvf = netdev_priv(dev); > + u8 old_pfc_en; > int err; > > - /* Save PFC configuration to interface */ > + old_pfc_en = pfvf->pfc_en; > pfvf->pfc_en = pfc->pfc_en; > > if (pfvf->hw.tx_queues >= NIX_PF_PFC_PRIO_MAX) > @@ -411,13 +412,17 @@ static int otx2_dcbnl_ieee_setpfc(struct net_device *dev, struct ieee_pfc *pfc) > * supported by the tx queue configuration > */ > err = otx2_check_pfc_config(pfvf); > - if (err) > + if (err) { > + pfvf->pfc_en = old_pfc_en; > return err; Hi Suman, I think that rather than duplicating this logic, it would be appropriate to use a goto label. (OTOH, while not related to this patch, removing the process_pfc label would be a win for readability, IMHO.) > + } > > process_pfc: > err = otx2_config_priority_flow_ctrl(pfvf); > - if (err) > + if (err) { > + pfvf->pfc_en = old_pfc_en; > return err; > + } > > /* Request Per channel Bpids */ > if (pfc->pfc_en) > @@ -425,6 +430,12 @@ static int otx2_dcbnl_ieee_setpfc(struct net_device *dev, struct ieee_pfc *pfc) > > err = otx2_pfc_txschq_update(pfvf); > if (err) { > + if (pfc->pfc_en) > + otx2_nix_config_bp(pfvf, false); > + > + otx2_pfc_txschq_stop(pfvf); > + pfvf->pfc_en = old_pfc_en; > + otx2_config_priority_flow_ctrl(pfvf); > dev_err(pfvf->dev, "%s failed to update TX schedulers\n", __func__); > return err; > } > -- > 2.25.1 > Perhaps I am on the wrong track here, but if 1. otx2_pfc_txschq_stop() was called by otx2_pfc_txschq_update() (or otx2_pfc_txschq_config()) for relevant error cases; and 2. pfc_en was passed as a parameter to otx2_config_priority_flow_ctrl() Then I think that the unwind logic in the if condition above could be expressed as unwind ladder with goto labels whereby the order of unwinding is strictly the reverse of configuration. This is a subjective opinion, but the advantage of this approach is that it tends to lead to more maintainable code and fewer errors in... error paths. (Completely untested!) ... if (err) goto err_pfc_en; ... err = otx2_pfc_txschq_update(pfvf); if (err) goto err_config; return 0; err_config: if (pfc->pfc_en) otx2_nix_config_bp(pfvf, false); otx2_config_priority_flow_ctrl(pfvf, old_pfc_en); err_pfc_en: pfvf->pfc_en = old_pfc_en; return err;
>> err = otx2_check_pfc_config(pfvf); >> - if (err) >> + if (err) { >> + pfvf->pfc_en = old_pfc_en; >> return err; > >Hi Suman, > >I think that rather than duplicating this logic, it would be appropriate >to use a goto label. > >(OTOH, while not related to this patch, removing the process_pfc label >would be a win for readability, IMHO.) [Suman] Sure, I can update that. > >> + } >> >> process_pfc: >> err = otx2_config_priority_flow_ctrl(pfvf); >> - if (err) >> + if (err) { >> + pfvf->pfc_en = old_pfc_en; >> return err; >> + } >> >> /* Request Per channel Bpids */ >> if (pfc->pfc_en) >> @@ -425,6 +430,12 @@ static int otx2_dcbnl_ieee_setpfc(struct >> net_device *dev, struct ieee_pfc *pfc) >> >> err = otx2_pfc_txschq_update(pfvf); >> if (err) { >> + if (pfc->pfc_en) >> + otx2_nix_config_bp(pfvf, false); >> + >> + otx2_pfc_txschq_stop(pfvf); >> + pfvf->pfc_en = old_pfc_en; >> + otx2_config_priority_flow_ctrl(pfvf); >> dev_err(pfvf->dev, "%s failed to update TX schedulers\n", >__func__); >> return err; >> } >> -- >> 2.25.1 >> > >Perhaps I am on the wrong track here, but if >1. otx2_pfc_txschq_stop() was called by otx2_pfc_txschq_update() > (or otx2_pfc_txschq_config()) for relevant error cases; and >2. pfc_en was passed as a parameter to otx2_config_priority_flow_ctrl() > >Then I think that the unwind logic in the if condition above could >be expressed as unwind ladder with goto labels whereby the order >of unwinding is strictly the reverse of configuration. > >This is a subjective opinion, but the advantage of this approach is that >it >tends to lead to more maintainable code and fewer errors in... error >paths. > >(Completely untested!) > > ... > if (err) > goto err_pfc_en; > ... > err = otx2_pfc_txschq_update(pfvf); > if (err) > goto err_config; > > return 0; > >err_config: > if (pfc->pfc_en) > otx2_nix_config_bp(pfvf, false); > otx2_config_priority_flow_ctrl(pfvf, old_pfc_en); >err_pfc_en: > pfvf->pfc_en = old_pfc_en; > > return err; [Suman] Let me think through it. I need to check if some cases will be missed, will update, and push a new patch in that case. Thanks for your feedback, Simon!!
On Sun, Dec 17, 2023 at 06:26:24AM +0000, Suman Ghosh wrote: ... > >Perhaps I am on the wrong track here, but if > >1. otx2_pfc_txschq_stop() was called by otx2_pfc_txschq_update() > > (or otx2_pfc_txschq_config()) for relevant error cases; and > >2. pfc_en was passed as a parameter to otx2_config_priority_flow_ctrl() > > > >Then I think that the unwind logic in the if condition above could > >be expressed as unwind ladder with goto labels whereby the order > >of unwinding is strictly the reverse of configuration. > > > >This is a subjective opinion, but the advantage of this approach is that > >it > >tends to lead to more maintainable code and fewer errors in... error > >paths. > > > >(Completely untested!) > > > > ... > > if (err) > > goto err_pfc_en; > > ... > > err = otx2_pfc_txschq_update(pfvf); > > if (err) > > goto err_config; > > > > return 0; > > > >err_config: > > if (pfc->pfc_en) > > otx2_nix_config_bp(pfvf, false); > > otx2_config_priority_flow_ctrl(pfvf, old_pfc_en); > >err_pfc_en: > > pfvf->pfc_en = old_pfc_en; > > > > return err; > [Suman] Let me think through it. I need to check if some cases will be missed, will update, and push a new patch in that case. Thanks for your feedback, Simon!! Thanks, much appreciated.
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_dcbnl.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_dcbnl.c index bfddbff7bcdf..28fb643d2917 100644 --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_dcbnl.c +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_dcbnl.c @@ -399,9 +399,10 @@ static int otx2_dcbnl_ieee_getpfc(struct net_device *dev, struct ieee_pfc *pfc) static int otx2_dcbnl_ieee_setpfc(struct net_device *dev, struct ieee_pfc *pfc) { struct otx2_nic *pfvf = netdev_priv(dev); + u8 old_pfc_en; int err; - /* Save PFC configuration to interface */ + old_pfc_en = pfvf->pfc_en; pfvf->pfc_en = pfc->pfc_en; if (pfvf->hw.tx_queues >= NIX_PF_PFC_PRIO_MAX) @@ -411,13 +412,17 @@ static int otx2_dcbnl_ieee_setpfc(struct net_device *dev, struct ieee_pfc *pfc) * supported by the tx queue configuration */ err = otx2_check_pfc_config(pfvf); - if (err) + if (err) { + pfvf->pfc_en = old_pfc_en; return err; + } process_pfc: err = otx2_config_priority_flow_ctrl(pfvf); - if (err) + if (err) { + pfvf->pfc_en = old_pfc_en; return err; + } /* Request Per channel Bpids */ if (pfc->pfc_en) @@ -425,6 +430,12 @@ static int otx2_dcbnl_ieee_setpfc(struct net_device *dev, struct ieee_pfc *pfc) err = otx2_pfc_txschq_update(pfvf); if (err) { + if (pfc->pfc_en) + otx2_nix_config_bp(pfvf, false); + + otx2_pfc_txschq_stop(pfvf); + pfvf->pfc_en = old_pfc_en; + otx2_config_priority_flow_ctrl(pfvf); dev_err(pfvf->dev, "%s failed to update TX schedulers\n", __func__); return err; }