From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id D5DC3A00BE; Fri, 11 Feb 2022 10:59:35 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id C8C3341144; Fri, 11 Feb 2022 10:59:35 +0100 (CET) Received: from out1-smtp.messagingengine.com (out1-smtp.messagingengine.com [66.111.4.25]) by mails.dpdk.org (Postfix) with ESMTP id D121240150 for ; Fri, 11 Feb 2022 10:59:33 +0100 (CET) Received: from compute2.internal (compute2.nyi.internal [10.202.2.46]) by mailout.nyi.internal (Postfix) with ESMTP id 3C1075C0091; Fri, 11 Feb 2022 04:59:33 -0500 (EST) Received: from imap48 ([10.202.2.98]) by compute2.internal (MEProxy); Fri, 11 Feb 2022 04:59:33 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=u256.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=fm1; bh=zHP5KGWbj1vXGK F5ubRHFxir8QoUKol+CMNUkC0DZqc=; b=LQ1Nj1YzjDqlXM9EmRvFlGZooFPITP wk+tFSaKlB89laU6ZFPu/pCo2Xw6iyFhCcDhgd5BIgAz5xf2kVexE0VEo0YtW8lj ftjaQOu6A3qgbj/27ymI6y/zIkppId7y5X7gXvdQ5HhoGi3rJjDolRG+Mwm9FxE7 cL7//W/TaxGoau7W8qMsUD2v4OjQCiMhHbdGm8RGQdznTa00vZosSmDzrwvUG7S/ 9S5FJT2HptX/ES7PwHvj59JZ6rKHVdYXsN3Ehyj85eTM2wkAtDcg4DdNdWFDhSR4 djOvLx/chAQacfL8cONFWMco4Ovf6i/4kmesd4L56oFy4QDpWH5WL2UQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; 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:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm2; bh=zHP5KGWbj1vXGKF5ubRHFxir8QoUKol+CMNUkC0DZ qc=; b=POQpHur+zUfX2pjXhJpNp1w0IeJNUQ+TL25AJ2ReOo5cI2YPj4YqI25DX Mb1cxVgiRpIAHk9bIixcQRYmxWs6B4ZJ5Gq1nZvDlYevn50kSqAnYoIrWeZlPISr UIozqQnicai4ZxLA29ZNKOB+w1UoWjPtKblewOqcuTI/I1Ey67BR3TgDai/WQZ/C Tu9TKh1smr6G+27dpOzPIEh7AqxRKDixTZKNlo6Ki98vKpUr0JeXThFnGB/x7BJ3 WQHT3PEiRYCoyBhBnsvkDfxWfUmtelX9V6ayNTEBzMnf7/bHZ+HOmjoGhjXDZy2d uvpVQayvBZuYDxXZGrU0kl4aiCRcg== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvvddrieefgddtlecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefofgggkfgjfhffhffvufgtgfesthhqredtreerjeenucfhrhhomhepifgrtoht rghnpgftihhvvghtuceoghhrihhvvgesuhdvheeirdhnvghtqeenucggtffrrghtthgvrh hnpedvfeefheeukeeuhffhffdtvddutdfgtdektddulefghfefjedtueduiedufeehfeen ucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehgrhhivh gvsehuvdehiedrnhgvth X-ME-Proxy: Received: by mailuser.nyi.internal (Postfix, from userid 501) id BA1C121E006E; Fri, 11 Feb 2022 04:59:32 -0500 (EST) X-Mailer: MessagingEngine.com Webmail Interface User-Agent: Cyrus-JMAP/3.5.0-alpha0-4748-g31a5b5f50e-fm-cal2020-20220204.001-g31a5b5f5 Mime-Version: 1.0 Message-Id: <8ab1f54f-b46b-46cd-8345-2d049708fcf6@www.fastmail.com> In-Reply-To: References: <20220210071052.527-1-madhuker.mythri@oracle.com> <0a3f6aa6-2499-ce40-6dd0-721f70017003@intel.com> <9dc29099-0c73-4070-b017-69de6fc0e687@www.fastmail.com> Date: Fri, 11 Feb 2022 10:58:43 +0100 From: =?UTF-8?Q?Ga=C3=ABtan_Rivet?= To: "madhuker.mythri" , "Ferruh Yigit" , "Stephen Hemminger" , "Long Li" Cc: "dev@dpdk.org" Subject: Re: [External] : Re: Content-Type: text/plain;charset=utf-8 Content-Transfer-Encoding: quoted-printable X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Fri, Feb 11, 2022, at 10:37, Madhuker Mythri wrote: > Hi Gaetan and Ferruh, > >> >>-----Original Message----- >>From: Ga=C3=ABtan Rivet =20 >>Sent: 10 =E0=A4=AB=E0=A4=B0=E0=A4=B5=E0=A4=B0=E0=A5=80 2022 21:39 >>To: Ferruh Yigit ; Madhuker Mythri >>Cc: dev@dpdk.org >>Subject: [External] : Re:=20 >> >>On Thu, Feb 10, 2022, at 16:00, Ferruh Yigit wrote: >>> On 2/10/2022 7:10 AM, madhuker.mythri@oracle.com wrote: >>>> From: Madhuker Mythri >>>>=20 >>>> Failsafe pmd started crashing with global devargs syntax as devargs=20 >>>> is not memset to zero. Access it to in rte_devargs_parse resulted i= n=20 >>>> a crash when called from secondary process. >>>>=20 >>>> Bugzilla Id: 933 >>>>=20 >>>> Signed-off-by: Madhuker Mythri >>>> --- >>>> drivers/net/failsafe/failsafe.c | 1 + >>>> 1 file changed, 1 insertion(+) >>>>=20 >>>> diff --git a/drivers/net/failsafe/failsafe.c=20 >>>> b/drivers/net/failsafe/failsafe.c index 3c754a5f66..aa93cc6000 1006= 44 >>>> --- a/drivers/net/failsafe/failsafe.c >>>> +++ b/drivers/net/failsafe/failsafe.c >>>> @@ -360,6 +360,7 @@ rte_pmd_failsafe_probe(struct rte_vdev_device *= vdev) >>>> if (sdev->devargs.name[0] =3D=3D '\0') >>>> continue; >>>> =20 >>>> + memset(&devargs, 0, sizeof(devargs)); >>>> /* rebuild devargs to be able to get the bus name. */ >>>> ret =3D rte_devargs_parse(&devargs, >>>> sdev->devargs.name); >>> >>> if 'rte_devargs_parse()' requires 'devargs' parameter to be memset,=20 >>> what do you think memset it in the API? >>> This prevents forgotten cases like this. >> >>Hi, >> >>I was looking at it this morning. >>Before the last release, rte_devargs_parse() was only supporting legac= y syntax. >>It never read from the devargs structure, only wrote to it. So it was = safe to use with a non-memset devargs. >> >>The rte_devargs_layer_parse() however is more complex. To allow rte_de= v_iterator_init() to call it without doing memory allocation, it reads p= arts of the devargs to make decisions. >> >>Doing a first call to rte_devargs_layer_parse() as part of rte_devargs= _parse() thus modified the contract it had with the users, that it would= never read from devargs. >> >>It is not possible to completely avoid reading from devargs in rte_dev= args_layer_parse(). >>It is necessary for RTE_DEV_FOREACH() to be safe to interrupt without = having to do iterator cleanup. >> >>This is my current understanding. In that context, yes I think it is p= referrable to do memset() within rte_devargs_parse(). It will restore th= e previous part of the API saying that calling it with non-memset >devar= gs was safe to do. >> >>Thanks, >>-- >>Gaetan Rivet > > Thanks for your comments. > The rte_devargs_parse() is used in other 'netvsc' PMD also in=20 > netvsc_hotadd_callback(). > In this netvsc_hotadd_callback(), it was assigning the devargs with=20 > some other instance pointer(not sure, whether its just a pointer or=20 > with data values) before calling this rte_devargs_parse(), so if we=20 > memset inside this API, then the devargs data values will be nullified=20 > right. > I'm not fully familiar with this parsing functionality. So, please let=20 > me know, doing memset() inside this rte_devargs_parse() is valid or=20 > not, as this is a generic function for all the PMD's. > > Thanks, > Madhuker. Hi Madhuker, I have just checked the code, it does not seem that netvsc relies on val= ues being kept in the devargs structure across calls, so it seems that it wo= uld be safe. It would be better to check with the maintainers however, I'm addi= ng them to this thread. --=20 Gaetan Rivet