From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f45.google.com (mail-wm0-f45.google.com [74.125.82.45]) by dpdk.org (Postfix) with ESMTP id 5B5481B199 for ; Wed, 11 Oct 2017 13:56:54 +0200 (CEST) Received: by mail-wm0-f45.google.com with SMTP id f4so4103024wme.0 for ; Wed, 11 Oct 2017 04:56:54 -0700 (PDT) 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=IVjm56/92DKRyX802WeRRA7ITupDZrO4Jy6HYesVU7Q=; b=DxMYNnDR07pAzF8XVvMnC2FQWXtdKf6xHCfr77zaOhfUhDxeCxAC+iFMR4pwHDmKV3 cZhAV0YevIFjpW/2RbOwzVCNcMbhJ2/f4nARRYDAsD6W5pB67YzQYk222ws5OvlD1FZ0 R5AZcwjDweLlhM6NG1idBD/bBya3My6SE1edk331Kuo7wDrJJPvDLyJhIXMWYXiyXPaR gwd/d792f4BU7s80dX4ZXzviA7KmhFLdGz5AiAf73GzS9opo4UqUu03ltf4jY8neuxdO EwHjseJxxVcnmNa0BR3Q750kDb0uj4BLdb+5r3546V/9uyCVZxWu0okz7WeJ1eVuFtJs GKzw== 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=IVjm56/92DKRyX802WeRRA7ITupDZrO4Jy6HYesVU7Q=; b=Xx5bah9oB2+jjiVa+fFoRt7JjkdWNuc/chyhYmdfuVcJnI/3L7wXKspiltPd1SNTYn niIUBRw9b4ofZ2z1o4D8dK0LYa6zlkhbA+pM0JcJDEi/UhlPbw7DTRycyMiJ1/Ok1xOl 8y57935OoxS3gd1xDro3xTrl+sMe+Qh+YoZWsul1Cf/h+WtuLwGnGWw+RY3H9++g3vdZ Tf9WehLdx5HNTMV7U70zTD9k96fACkiORtyYUKz74OGMPxNjWqDn6r/ozP1khZp/p363 OU4qDJH0TVd02Qls21XNyT72FZ+Umme6Cd+bkdfNmGj/VglB7ACOe+tlWmr93FqMsniQ udZw== X-Gm-Message-State: AMCzsaUoh4dtCc5w6e0bV5sxBXFhYwSxf4aF+WRfXQT0JwUOXADuw61Y /qmpq/LbJEJ8iMHgP1CmF8N7xw== X-Google-Smtp-Source: AOwi7QBOPVkRk+A8anieBrKW3chIYk159dPbhj6Y4I8rPXmv7RBOKjiKfCV0yDj250NDRPgw+yTrjQ== X-Received: by 10.28.26.138 with SMTP id a132mr14586044wma.124.1507723014081; Wed, 11 Oct 2017 04:56:54 -0700 (PDT) Received: from 6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id g51sm14079973wra.29.2017.10.11.04.56.53 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 11 Oct 2017 04:56:53 -0700 (PDT) Date: Wed, 11 Oct 2017 13:56:42 +0200 From: Adrien Mazarguil To: Thomas Monjalon Cc: dev@dpdk.org, Ferruh Yigit , Gaetan Rivet , aconole@redhat.com, fbl@redhat.com Message-ID: <20171011115642.GI13551@6wind.com> References: <00db12469380e587ac434bfc5c63795e32cfddef.1507193185.git.adrien.mazarguil@6wind.com> <2075344.22PATxnhkA@xps> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <2075344.22PATxnhkA@xps> Subject: Re: [dpdk-dev] [PATCH v1 1/7] ethdev: expose flow API error helper 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, 11 Oct 2017 11:56:54 -0000 On Wed, Oct 11, 2017 at 11:23:12AM +0200, Thomas Monjalon wrote: > 05/10/2017 11:49, Adrien Mazarguil: > > rte_flow_error_set() is a convenient helper to initialize error objects. > > > > Since there is no fundamental reason to prevent applications from using it, > > expose it through the public interface after modifying its return value > > from positive to negative. This is done for consistency with the rest of > > the public interface. > > > > Documentation is updated accordingly. > > > > Signed-off-by: Adrien Mazarguil > > --- > > --- a/lib/librte_ether/rte_flow.h > > +++ b/lib/librte_ether/rte_flow.h > > /** > > + * Initialize flow error structure. > > + * > > + * @param[out] error > > + * Pointer to flow error structure (may be NULL). > > + * @param code > > + * Related error code (rte_errno). > > + * @param type > > + * Cause field and error types. > > + * @param cause > > + * Object responsible for the error. > > + * @param message > > + * Human-readable error message. > > + * > > + * @return > > + * Negative error code (errno value) and rte_errno is set. > > + */ > > +static inline int > > +rte_flow_error_set(struct rte_flow_error *error, > > + int code, > > + enum rte_flow_error_type type, > > + const void *cause, > > + const char *message) > > When calling this function, the performance is not critical. > So it must not be an inline function. > Please move it to the .c file and add it to the .map file. I'll do it as part of moving this patch into my upcoming series for mlx4, since it relies on the new return value. Now for the record, I believe that static inlines, particularly short ones that are not expected to be modified any time soon (provided they are defined properly that is) are harmless. Yes, that means modifying them, depending on what they do, may cause a global ABI breakage of the underlying library which translates to a major version bump, however exactly the same applies to some macros (in particular the *_MAX ones), structure and other type definitions and so on. What I'm getting at is, making this function part of the .map file won't help all that much with the goal of reaching a stable ABI. Making it a normal function on the other hand successfully prevents PMDs and applications from considering its use in their data plane (flow rules management from the data plane is likely the next step for rte_flow). In my opinion, performance (even if hypothetical) matters more than ABI stability, particularly since the current pattern is for the whole ABI to be broken every other release. I think that with DPDK, ABI stability can only be properly guaranteed in stable/maintenance releases but that effort is vain in the master branch, affected by way too many changes for it to ever become reality. This doesn't mean we shouldn't even try, however we shouldn't restrict ourselves; when the ABI needs to be broken, so be it. (end of rant, thanks for reading) -- Adrien Mazarguil 6WIND