From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f42.google.com (mail-wm0-f42.google.com [74.125.82.42]) by dpdk.org (Postfix) with ESMTP id 0D1B42C6E for ; Wed, 1 Feb 2017 11:54:39 +0100 (CET) Received: by mail-wm0-f42.google.com with SMTP id c85so33015938wmi.1 for ; Wed, 01 Feb 2017 02:54:39 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=Z2RP77bxq1j2zJmgL/Udu7hNN1z1eq//3gTXwYJw0M8=; b=gVp+mdq8q5Qvm5u4m80SAOdnNBqp5AFsLWzfmcu3p6rKSn9ZQBbUb78xRoUQ+IkpN5 4FtZZihsxAYk9xUgLaT3I+3PmiwClWn6PtCtBbkJZwx/2SS6Uo28BYd72ttVQL53bXNO qahy83tU4H249/AeywJ31HospG9yQ+zCP9bfas4Q1NvBMcDcXvsufbIT0asStCxzCv7E RsO9z1JXVsPXHt9HYTVrTPXicoUMJp4dn7hAbpw/k1AFME7uH4XuHzNJOuQo6E64eATD HOdomOxnSkWNang+nVSU5dwyZXqNLzTw/GBlp4StU9oQJiiM+/HWAPiyr/J0Iyy/GdPB xqLQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=Z2RP77bxq1j2zJmgL/Udu7hNN1z1eq//3gTXwYJw0M8=; b=HTig2+l9VUAaiayo0k0VWPAd1WcLzlSq62KAyQqoDnsguQKhABUerugK7jgsaHboKB n+3SahD+HI0VUei6ONbgRPOIfQeGxaDcRCODhcp511pbFuHQegr/9uL+lpEqARrXR6QK DZ3hd9+uxN9VL994A2l741ZrFXSeAcLwdKZiYdTLMYFXVrpNCTbDAPdAe4yY07H2JwVX Ue7BiQvJr6ZdMEAF4W1gk+SnAZ3lHctuKWuRIMUUpqx3XGDrvCpJo7FBJ+KSAQn4VGGg kSGfDheYGecSqk4DP0fCncrPgODJv43RFftfDyDrPUSz0s5HY6nZdfClc5NyjF92Xp20 JbKw== X-Gm-Message-State: AIkVDXK/w4F84y5hqfCvBFap4L58PiVhljXoFlkStx/P2ERoJmLKRHqzqN/YFHBNSgp67f46 X-Received: by 10.28.86.214 with SMTP id k205mr2215642wmb.26.1485946478522; Wed, 01 Feb 2017 02:54:38 -0800 (PST) Received: from 6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id g40sm33378284wrg.19.2017.02.01.02.54.37 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 01 Feb 2017 02:54:37 -0800 (PST) Date: Wed, 1 Feb 2017 11:54:30 +0100 From: Adrien Mazarguil To: Thomas Monjalon Cc: Aaron Conole , dev@dpdk.org, Stephen Hemminger , Bruce Richardson Message-ID: <20170201105430.GQ10133@6wind.com> References: <1485529023-5486-1-git-send-email-aconole@redhat.com> <20170127093729.5cef9138@xeon-e3> <2880962.uYJx3WqeFl@xps13> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <2880962.uYJx3WqeFl@xps13> Subject: Re: [dpdk-dev] [PATCH 25/25] rte_eal_init: add info about rte_errno codes X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 01 Feb 2017 10:54:39 -0000 On Mon, Jan 30, 2017 at 09:19:29PM +0100, Thomas Monjalon wrote: > 2017-01-30 13:38, Aaron Conole: > > Stephen Hemminger writes: > > > Bruce Richardson wrote: > > >> On Fri, Jan 27, 2017 at 08:33:46AM -0800, Stephen Hemminger wrote: > > >> > Why use rte_errno? > > >> > Most DPDK calls just return negative value on error which > > >> > corresponds to error number. > > >> > Are you trying to keep ABI compatibility? Doesn't make sense > > >> > because before all these > > >> > errors were panic's no working application is going to care. > > >> > > >> Either will work, but I actually prefer this way. I view using rte_errno > > >> to be better as it can work in just about all cases, including with > > >> functions which return pointers. This allows you to have a standard > > >> method across all functions for returning error codes, and it only > > >> requires a single sentinal value to indicate error, rather than using a > > >> whole range of values. > > > > > > The problem is DPDK is getting more inconsistent on how this is done. > > > As long as error returns are always same as kernel/glibc errno's it really doesn't > > > matter much which way the value is returned from a technical point of view > > > but the inconsistency is sure to be a usability problem and source of errors. > > > > I am using rte_errno here because I assumed it was the preferred > > method. In fact, looking at some recently contributed modules (for > > instance pdump), it seems that folks are using it. > > > > I'm not really sure the purpose of having rte_errno if it isn't used, so > > it'd be helpful to know if there's some consensus on reflecting errors > > via this variable, or on returning error codes. Whichever is the more > > consistent with the way the DPDK project does things, I'm game :). > > I think we can use both return value and rte_errno. > We could try to enforce rte_errno as mandatory everywhere. > > Adrien did the recent rte_flow API. > Please Adrien, could you give your thought? Sure, actually as already pointed out in this thread, both approaches have pros and cons depending on the use-case. Through return value: Pros ---- - Most common approach used in DPPK today. - Used internally by the Linux kernel (negative errno) and in the pthreads library (positive errno). - Avoids the need to access an external, global variable requiring its own thread-local storage. - Inherently thread-safe and reentrant (i.e. safe with signal handlers). - Returned value is also the error code, two facts reported at once. Cons ---- - Difficult to use with functions returning anything other than signed integers with negative values having no other meaning. - The returned value must be assigned to a local variable in order not to discard it and process it later most of the time. - All function calls must be tested for errors. Through rte_errno: Pros ---- - errno-like, well known behavior defined by the C standard and used everywhere in the C library. - Testing return values is not mandatory, e.g. rte_errno can be initialized to zero before calling a group of functions and checking its value afterward (rte_errno is only updated in case of error). - Assigning a local variable to store its value is not necessary as long as another function that may affect rte_errno is not called. Cons ---- - Not fully reentrant, thread-safety is fine for most purposes but signal handlers affecting it still cause undefined behavior (they must at least save and restore its value in case they modify it). - Accessing non-local storage may affect CPU cycle-sensitive functions such as TX/RX burst. My opinion is that rte_errno is best for control path operations while using the return value makes more sense in the data path. The major issue being that function returning anything other than int (e.g. TX/RX burst) cannot describe any kind of error to the application. I went with both in rte_flow (return + rte_errno) mostly due to the return type of a few functions (e.g. rte_flow_create()) and wanted to keep the API consistent while maintaining compatibility with other DPDK APIs. Note there is little overhead for API functions to set rte_errno _and_ return its value, it's mostly free. I think using both is best also because it leaves applications the choice of error-handling method, however if I had to pick one I'd go with rte_errno and standardize on -1 as the default error value (as in the C library). Below are a bunch of use-case examples to illustrate how rte_errno could be convenient to applications. Easily creating many flow rules during init in a all-or-nothing fashion: rte_errno = 0; for (i = 0; i != num; ++i) rule[i] = rte_flow_create(port, ...); if (unlikely(rte_errno)) { rte_flow_flush(port); return -1; } Complete TX packet burst failure with explanation (could also detect partial failures by initializing rte_errno to 0): sent = rte_eth_tx_burst(...); if (unlikely(!sent)) { switch (rte_errno) { case E2BIG: // too many packets in burst ... case EMSGSIZE: // first packet is too large ... case ENOBUFS: // TX queue is full ... } } TX burst functions in PMDs could be modified as follows with minimal impact on their performance and no ABI change: uint16_t sent = 0; int error; // new variable [process burst] if (unlikely([something went wrong])) { // this check already exists error = EPROBLEM; // new assignment goto error; // instead of "return sent" } [process burst] return sent; error: rte_errno = error; return sent; -- Adrien Mazarguil 6WIND