From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 53A5B1B2A3 for ; Tue, 13 Feb 2018 15:25:34 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 13 Feb 2018 06:25:33 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.46,507,1511856000"; d="scan'208";a="34339421" Received: from aburakov-mobl.ger.corp.intel.com (HELO [10.237.220.71]) ([10.237.220.71]) by orsmga002.jf.intel.com with ESMTP; 13 Feb 2018 06:25:31 -0800 To: "Van Haaren, Harry" , Thomas Monjalon Cc: "dev@dpdk.org" , "Tan, Jianfeng" References: <46a01b34318bbc799a0911fbb166a592c34ffa66.1518267551.git.anatoly.burakov@intel.com> <7815253.RbYmxIpbrz@xps> From: "Burakov, Anatoly" Message-ID: <8a18715a-c87e-3834-7adc-86b22a4e482d@intel.com> Date: Tue, 13 Feb 2018 14:25:31 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH] eal: fix rte_errno values for IPC API 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: Tue, 13 Feb 2018 14:25:34 -0000 On 13-Feb-18 2:16 PM, Van Haaren, Harry wrote: >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon >> Sent: Tuesday, February 13, 2018 1:51 PM >> To: Burakov, Anatoly >> Cc: dev@dpdk.org; Tan, Jianfeng >> Subject: Re: [dpdk-dev] [PATCH] eal: fix rte_errno values for IPC API >> >>>> rte_errno values should not be negative. >>>> >>>> Fixes: bacaa2754017 ("eal: add channel for multi-process communication") >>>> Fixes: 783b6e54971d ("eal: add synchronous multi-process communication") >>>> Cc: jianfeng.tan@intel.com >>>> Signed-off-by: Anatoly Burakov >>> >>> Reviewed-by: Jianfeng Tan >>> >>> Thanks for fixing this. >> >> Applied, thanks >> >> There are a lot of similar issues: >> >> git grep -l 'rte_errno = -E' | sed 's,[^/]*$,,' | sort -u >> >> drivers/event/opdl/ >> drivers/event/sw/ > >> lib/librte_eventdev/ > > > I just checked the eventdev.h port_link() docs, which indicate negative return values. > Perhaps the header is wrong too - but the PMDs adhere to the library header in this case. > > Is there a requirement for rte_errno to be positive? > It looks to be declared as per-lcore signed int in rte_errno.h +20 > > Either-way, if we want to change the PMDs, we should change the Eventdev APIs, > which means API breakage, and application changes to handle changed return values. > > Sound like more work than it is worth it to me? > To be clear, documentation doesn't *explicitly* spell this out, so it can be interpreted as not having an opinion on this. So i think the first step should be fixing documentation to clearly indicate this is to be expected at least of new code (unless decided against for consistency reasons). However, while docs don't state this outright, i think they imply it both in cases of referring to errno values and returning "-errno" values (which implies sign isn't part of the errno value). Plus, errno values in Linux are always positive, plus rte_strerror() expects a positive value, and (as correctly pointed out by Harry) our custom errno values - so at least our API expects it to be positive, even if it doesn't state it outright. -- Thanks, Anatoly