From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <stable-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
	by inbox.dpdk.org (Postfix) with ESMTP id A3EE3A0545
	for <public@inbox.dpdk.org>; Thu, 23 Jun 2022 08:41:24 +0200 (CEST)
Received: from [217.70.189.124] (localhost [127.0.0.1])
	by mails.dpdk.org (Postfix) with ESMTP id 9D7C4427ED;
	Thu, 23 Jun 2022 08:41:24 +0200 (CEST)
Received: from out1-smtp.messagingengine.com (out1-smtp.messagingengine.com
 [66.111.4.25]) by mails.dpdk.org (Postfix) with ESMTP id 298534069C;
 Thu, 23 Jun 2022 08:41:23 +0200 (CEST)
Received: from compute3.internal (compute3.nyi.internal [10.202.2.43])
 by mailout.nyi.internal (Postfix) with ESMTP id 07EC35C009B;
 Thu, 23 Jun 2022 02:41:20 -0400 (EDT)
Received: from mailfrontend1 ([10.202.2.162])
 by compute3.internal (MEProxy); Thu, 23 Jun 2022 02:41:20 -0400
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h=
 cc:cc:content-transfer-encoding:content-type:date:date:from:from
 :in-reply-to:in-reply-to:message-id:mime-version:references
 :reply-to:sender:subject:subject:to:to; s=fm2; t=1655966480; x=
 1656052880; bh=UFPCwp9s8pKnaoV9MZHOXWLB6uObvfxePWLsvX0b3f4=; b=b
 WvFXNClpaWJnueFpgYwk+9uIS9Rf5Umm8R8tofhavm7I6EsnwdDskaUleVwrdVCd
 1V7sSrpAPlypFIUuoeyq/EgQ2CZHeH8uNm7bKJY2qCa20PvQSSehTJ0PFEDgLSSr
 MavAnobCEeeynnCMq2rRANYfQWry/f0Sin8d8NJdKd3Wr5RlmPt6zMox2RXYcSTQ
 yyga+Po4El2IuwVLbqSBlLSDvQWmvMrNsT7gaZ95LfspnN2UbjcVgKS/kWYBPUU/
 YV97zPbBExk+z+WHW+YxC/usIrtwo0CCrWEw5gDcD14E+aAltrQHJDG20nB2nXKz
 kSIJXhAZ/jaOruGcqRMhQ==
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=
 messagingengine.com; h=cc:cc:content-transfer-encoding
 :content-type:date:date:feedback-id:feedback-id:from:from
 :in-reply-to:in-reply-to:message-id:mime-version:references
 :reply-to:sender:subject:subject:to:to:x-me-proxy:x-me-proxy
 :x-me-sender:x-me-sender:x-sasl-enc; s=fm2; t=1655966480; x=
 1656052880; bh=UFPCwp9s8pKnaoV9MZHOXWLB6uObvfxePWLsvX0b3f4=; b=C
 ZaZBeOIv5gius0nvpBYSKeq2FD32qDnZKVfcRsKlbpWDvTUjSvtW65zN+r4iY1P8
 zwUAaQr7NVZZIbIAzlgfNE0ln980qWr7PvGasd2TIqbT+rAF5105EMaEai/9529p
 YzcRkrkLoW7haZBrUFOWQd9xt4yVU0IrpJ8qNCfYXIFOPbR2TDETwvTh3PO5SG7F
 x7uoTyMxR8WiMkkZMb+KKoewCNt7UKaWxd7SS7oo2gXDA1Nxwx1TQChpEqHnj1tO
 z1ZhtTnRJQ5YPhPeiy/JWoSO0gwZzGFPOC2OWqCcvUtNRZ8Jo6dLR2okRFECBDo7
 eRiURkAKNPFEeZ6TU5Gew==
X-ME-Sender: <xms:Dwu0YixwdpxjmUbJJlQra5-12LYXi6z8rIFm-MUwUOed5O6JPMVLow>
 <xme:Dwu0YuRmu9_mokwPi_OErj9lLv1x6bxq2oihMG62MwbQmjMStGrR_KZl40dIG7hlk
 WQnOwQZFhQ9H9A9Iw>
X-ME-Received: <xmr:Dwu0YkX_yyqBVnjrvkZpMi6y06aKKP4-48DLRrfQngNH4h-0Pli-RZ0c6ZgurM3SHo_LHR4c08LNB4EWYzi-vj9IQw>
X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvfedrudefiedgudduvdcutefuodetggdotefrod
 ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh
 necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd
 enucfjughrpefhvfevufffkfgjfhgggfgtsehtqhertddttdejnecuhfhrohhmpefvhhho
 mhgrshcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqne
 cuggftrfgrthhtvghrnhepgedttdeljeejgeffkeekkedtjeevtdehvedtkeeivdeuuedv
 ieduvdelveejueejnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilh
 hfrhhomhepthhhohhmrghssehmohhnjhgrlhhonhdrnhgvth
X-ME-Proxy: <xmx:Dwu0Yogas-6P9p0ZSfcU7W4syf0HqQaxjaQomR4kjXPo-ZnleOmOAw>
 <xmx:Dwu0YkBAvC0XeK0eIqF3rXgtiRd-ejidpTzsIRCIaWtWvuhx0wX1eg>
 <xmx:Dwu0YpJ3x9bKp-4oMzxv0ALEs8L39HATcmtfBuwmHwpuv1WNjhsm1Q>
 <xmx:EAu0Yg1DvTg22eTm-WXMeA00uUKOxPMVdQgI-vmKXbA5CxjA-ap4NQ>
Feedback-ID: i47234305:Fastmail
Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu,
 23 Jun 2022 02:41:18 -0400 (EDT)
From: Thomas Monjalon <thomas@monjalon.net>
To: Ke Zhang <ke1x.zhang@intel.com>
Cc: dev@dpdk.org, stable@dpdk.org, xiaoyun.li@intel.com,
 aman.deep.singh@intel.com, yuying.zhang@intel.com,
 Ferruh Yigit <ferruh.yigit@xilinx.com>, bruce.richardson@intel.com,
 david.marchand@redhat.com, qi.z.zhang@intel.com
Subject: Re: [PATCH v2] app/testpmd: fix quit testpmd with vfs and pf
Date: Thu, 23 Jun 2022 08:41:15 +0200
Message-ID: <8178369.aoefvbuG5b@thomas>
In-Reply-To: <4030713.XrmoMso0CX@thomas>
References: <20220322071833.199619-1-ke1x.zhang@intel.com>
 <e4403b4a-dfbf-9ea3-e833-cb9601049cb7@xilinx.com> <4030713.XrmoMso0CX@thomas>
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset="UTF-8"
X-BeenThere: stable@dpdk.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: patches for DPDK stable branches <stable.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/stable>,
 <mailto:stable-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/stable/>
List-Post: <mailto:stable@dpdk.org>
List-Help: <mailto:stable-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/stable>,
 <mailto:stable-request@dpdk.org?subject=subscribe>
Errors-To: stable-bounces@dpdk.org

Adding more about this patch:

testpmd is a test application. It allows you to find some issues
in some scenarios. It seems you found an issue and you decided
to hide it in the test application. That's really wrong!

This kind of issue should be solved in the API or in the driver.
In this case, I believe it should be handled in the driver
which should release dependent ports.


22/06/2022 23:31, Thomas Monjalon:
> Sorry, this patch looks really wrong, I cannot pull it into the main bran=
ch.
> See below for an explanation.
>=20
>=20
> 21/06/2022 12:43, Ferruh Yigit:
> > On 6/21/2022 10:24 AM, Ke Zhang wrote:
> > > When testpmd startups with pf and vfs,this error occurs when quitting,
> > > results in pf is released before vfs ,so the vf would access an
> > > freed heap memory.
> > >=20
> > > The solution is two steps:
> > > 1. Fetch the valid port value from RTE_ETH_FOREACH_DEV.
> > > 2. free the port in reverse order.
> > >=20
> > > Fixes: 08fd782b8454 ("app/testpmd: fix quit to stop all ports before =
close")
> > > Cc: stable@dpdk.org
> > >=20
> > > Signed-off-by: Ke Zhang <ke1x.zhang@intel.com>
> >=20
> > Acked-by: Aman Singh <aman.deep.singh@intel.com>
> > Acked-by: Ferruh Yigit <ferruh.yigit@xilinx.com>
> >=20
> > Applied to dpdk-next-net/main, thanks.
> >=20
> >=20
> > > ---
> > >   app/test-pmd/testpmd.c | 26 +++++++++++++++++++++-----
> > >   1 file changed, 21 insertions(+), 5 deletions(-)
> > >=20
> > > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> > > index 04c39adc21..c01ecf7279 100644
> > > --- a/app/test-pmd/testpmd.c
> > > +++ b/app/test-pmd/testpmd.c
> > > @@ -3492,16 +3492,32 @@ pmd_test_exit(void)
> > >   	}
> > >   #endif
> > >   	if (ports !=3D NULL) {
> > > +		portid_t ethports[RTE_MAX_ETHPORTS];
> > > +		int count =3D 0;
> > > +		int index =3D 0;
> >  >   		no_link_check =3D 1;
> >=20
> > No need to initialize 'index'.
> >=20
> > And better to have an empty line after deceleration block.
> >=20
> > > +
> > > +		/* Fetch the valid port id from port list*/
> > >   		RTE_ETH_FOREACH_DEV(pt_id) {
> > > -			printf("\nStopping port %d...\n", pt_id);
> > > +			ethports[count] =3D pt_id;
> > > +			count++;
> > > +		}
> > > +
> > > +		/*
> > > +		 * Free the port from Reverse order, as general,
> > > +		 * PF port < VF port=EF=BC=8C VF should be free before PF
> > > +		 * be free.
>=20
> No sorry, this is wrong.
> The port ID says nothing.
> If a port is closed before creating a VF,
> it can be VF < PF.
>=20
> > > +		 */
> > > +		for (index =3D count - 1 ; index >=3D 0 ; index--) {
> >=20
> > Space is not needed after ';'.
> >=20
> >=20
> > Since these are minor, I will fix while merging.