From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <thomas@monjalon.net>
Received: from out5-smtp.messagingengine.com (out5-smtp.messagingengine.com
 [66.111.4.29]) by dpdk.org (Postfix) with ESMTP id 1C2E85F14
 for <dev@dpdk.org>; Tue, 20 Mar 2018 23:04:58 +0100 (CET)
Received: from compute1.internal (compute1.nyi.internal [10.202.2.41])
 by mailout.nyi.internal (Postfix) with ESMTP id 92E1E20D2A;
 Tue, 20 Mar 2018 18:04:57 -0400 (EDT)
Received: from frontend2 ([10.202.2.161])
 by compute1.internal (MEProxy); Tue, 20 Mar 2018 18:04:57 -0400
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h=
 cc:content-transfer-encoding:content-type:date:from:in-reply-to
 :message-id:mime-version:references:subject:to:x-me-sender
 :x-me-sender:x-sasl-enc; s=mesmtp; bh=lpa9pBQ5iNuDEdTEGTA91xRMiw
 WcX9lIaEPLayw1K6c=; b=QtryzCdK9ttx2UEa4knkIZybt+Ok6WqV2cUlKDxi05
 6QmI3QZL+g6PN4sMEOXUGsPOHye3Lx8vRZzkU/arVLucdhxFrFjp1U49vyBl6t9z
 hHQNOjn98wY1Qf0QXcpV15nqvd+e1559QhhE0yevuQMnqtDhiI60nwovfQfnLHpT
 s=
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=
 messagingengine.com; h=cc:content-transfer-encoding:content-type
 :date:from:in-reply-to:message-id:mime-version:references
 :subject:to:x-me-sender:x-me-sender:x-sasl-enc; s=fm2; bh=lpa9pB
 Q5iNuDEdTEGTA91xRMiwWcX9lIaEPLayw1K6c=; b=gtwYJVmt1zCtVzBg3ocRx2
 QU6ZHewyDj8RggxjAlKEkmSa32z9czab4SocLAH9WHYtT1XeTIlvxTTyxZwei7//
 CnphsOhOloOHkSZ2M+8X0+44PjOLL4Z8DlV91wHiQYvAlPL69fgNqMFpcjZOJpMR
 Q06JelljpQ2d9/uHAFDv3AbudzY8dN5EpsPUrbB/f+r/mXqJF/1muoKsVI9nQPSg
 ZtLbETt2J3SUy7vNVc/fjUf1ZnUSO/NgfUaxeL1gxJVkX5sBVn5uXG4QqmPWNoRN
 5I+V3ehcj4AgWmhztABAppNlpCfC/LYuWZrO0a9rkiJS5itZGUOku7fywmX/ahOQ
 ==
X-ME-Sender: <xms:iYWxWgNI2-6BraW9WIhRMG3h6_G9AUtb3aI7H01VSyd-CqY4hAeuhg>
Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184])
 by mail.messagingengine.com (Postfix) with ESMTPA id 799D0241E3;
 Tue, 20 Mar 2018 18:04:56 -0400 (EDT)
From: Thomas Monjalon <thomas@monjalon.net>
To: Arnon Warshavsky <arnon@qwilt.com>
Cc: anatoly.burakov@intel.com, wenzhuo.lu@intel.com, declan.doherty@intel.com,
 jerin.jacob@caviumnetworks.com, bruce.richardson@intel.com,
 ferruh.yigit@intel.com, dev@dpdk.org
Date: Tue, 20 Mar 2018 23:04:13 +0100
Message-ID: <6503395.k68LoKUDuZ@xps>
In-Reply-To: <1521581285-4709-1-git-send-email-arnon@qwilt.com>
References: <1521581285-4709-1-git-send-email-arnon@qwilt.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 7Bit
Content-Type: text/plain; charset="us-ascii"
Subject: Re: [dpdk-dev] [PATCH] eal: replace rte_panic instances to return
	an error value
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Tue, 20 Mar 2018 22:04:58 -0000

Hi,

20/03/2018 22:28, Arnon Warshavsky:
> The purpose of this patch is to cleanup the library code
> from paths that end up aborting the process,
> and move to checking error values, in order to allow the running process
> perform an orderly teardown or other mitigation of the event.

Thanks for working on this important topic.

> This patch modifies the majority of rte_panic calls under lib and drivers,
> and replaces them with a new variation of rte_panic macro
> that does not abort and returns an error value
> that can be propagated up the call stack.

My feeling is that we could replace most of them by a log + return.
I did not think you would add a new macro. Why you chose this way?

> - Focus was given to the dpdk initialization path
> - Some of the panic calls within drivers were left in place where
>   the call is from within an interrupt or calls that are on the data path,
>   where there is no simple applicative route to propagate
>   the error to temination.
>   These should be handled by the driver maintainers.

Yes, better to let driver maintainers decide if you are not sure.

>   I would like to define a device health state that can be monitored from
>   the side,and this will be an independant patch.

You mean when a device become unusable?

> - No change took place in example and test files

Yes, panic/exit is allowed in applications.

> - No change took place for debug assertions calling panic

Yes, debug assert is a special case.

> - Some previously panicing void functions where changed to return a value,
>   with callers modified accordingly.

If the function is exposed to the application, I think it is an ABI change
and should follow the deprecation process.

> An additional independant patch to devtools/checkpatches.sh
> will be submitted in order to prevent new additions of calls to rte_panic
> under lib and drivers.

Yes please! +1 for an automatic check.

> Keep calm and don't panic.

Sure :)