From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-we0-f169.google.com (mail-we0-f169.google.com [74.125.82.169]) by dpdk.org (Postfix) with ESMTP id 12F41B62 for ; Mon, 23 Mar 2015 16:17:18 +0100 (CET) Received: by wegp1 with SMTP id p1so140493902weg.1 for ; Mon, 23 Mar 2015 08:17:18 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:organization :user-agent:in-reply-to:references:mime-version :content-transfer-encoding:content-type; bh=WuvNfHttO/kCl/p9llmimyiwwhNSr0OWuBYbv+s91ho=; b=EY+QGcxcymMW8SM25l5+Y69MjWfwXnM9+OCpXlMj0zXzE6igJWPe5FG5oZNHii1lAi 3dvS4rnwwsmwpmksvgJVTrar1wvj/ytS/D3q81iJQphYO/KrFBtZ8Jq+i2jjpppy5EdN OE0VU7AtM2Ln6048W4sc0CAHTex9HT/IY+mI+xGeSAbvUyiwITqj+SBmJ/jszU/qD+No vXggWy1xMCgqdmxGec7/3E+dKaqhuEPDGVytTb1sQtTSTmI1QIRptBq7K+3ikhOxdkaz 2w4uRxWTxONPBYhQb2crdr3fUJZRrzGq2XOscJScH53kgEZVQ4A8m03FNgybyHukiryZ prdQ== X-Gm-Message-State: ALoCoQlhdH3wisth4cvAabVENm7EWELb8OL8KmMLT4yT9fGr4sdVI/U/EN4pwsHgKtxYo13nL2lk X-Received: by 10.194.61.12 with SMTP id l12mr185281863wjr.139.1427123837947; Mon, 23 Mar 2015 08:17:17 -0700 (PDT) Received: from xps13.localnet (136-92-190-109.dsl.ovh.fr. [109.190.92.136]) by mx.google.com with ESMTPSA id ge8sm1806406wjc.32.2015.03.23.08.17.16 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 23 Mar 2015 08:17:16 -0700 (PDT) From: Thomas Monjalon To: Neil Horman , "Mcnamara, John" Date: Mon, 23 Mar 2015 16:16:36 +0100 Message-ID: <2572007.3HCLxGSxJD@xps13> Organization: 6WIND User-Agent: KMail/4.14.4 (Linux/3.18.4-1-ARCH; KDE/4.14.4; x86_64; ; ) In-Reply-To: <20150313231526.GA14641@neilslaptop.think-freely.org> References: <1426179268-22164-1-git-send-email-john.mcnamara@intel.com> <20150313231526.GA14641@neilslaptop.think-freely.org> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH] ethdev: additional parameter in RX callback X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 23 Mar 2015 15:17:18 -0000 2015-03-13 19:15, Neil Horman: > On Fri, Mar 13, 2015 at 06:28:31PM +0000, Mcnamara, John wrote: > > From: Neil Horman [mailto:nhorman@tuxdriver.com] > > > > > > > Is encoding the information in the array really a better solution here? > > > The cb->param already exists for passing in user defined information to > > > the callback. The proposed patch merely transmits the parent function > > > arguments to the enclosed callback. > > > > > > > The cb->param can't be used here, because its opaque to the internals of > > > the DPDK. rte_eth_rx_burst doesn't (and can't) know where in the cb- > > > >params pointer to store that information. Thats why you added an > > > additional parameter in the first place, isn't it? > > > > Yes. That is correct. > > > Then why did you suggest doing so? > > > > My point is that using > > > an array terminator keeps us out of this habbit of just adding parameters > > > to communicate more information (as thats an ABI breaking method, and not > > > particularly scalable if there is more information to be transmitted in > > > the future). Using a context sensitive API set goes beyond even that, and > > > allows to retrieve arbitrary information form callbacks as needed in an > > > ABI safe manner > > > > Again I can agree with this in the general case, but it isn't necessary, > > in this case, to encode the information in the array since it is already > > local to and available in the function. It seems artificial, at this point, > > to implement an array terminator solution to protect an API that, > > effectively, hasn't been published yet. > > > You indicate that you agree an alternate solution is preferable in the general > case, so as to provide an API that is extensible in a way that isn't subject to > ABI breakage, correct? If so, why do assert that its not necessecary in this > specific case? If you feel you need to add information so that callbacks can be > more flexible (in this case specifying the size of a passed in array), why > immediately shoehorn another parmeter in place, and break the consistency > between rx and tx callbacks, when you don't have to? I don't care if you break > ABI today (although to call it unpublished I think is disingenuous, as lots of > testing and development has already taken place with the ABI as it currently > stands). I care, as I noted above about not getting into the habbit of just > assuming a change like this requires that you invaliate ABI somehow. You don't > have to, you can create an API that is fairly invariant to it here if you like. > The question in my mind is, why don't you? I think John is saying that the API of rte_eth_rx_burst() already includes the nb_pkts parameter. So it's natural to push it to the callback. I also think Neil is saying that this parameter is useless in the callback and in rte_eth_rx_burst() if the array was null terminated. In any case, having a mix (null termination + parameter in rte_eth_rx_burst) is not acceptable. Moreover, I wonder how efficient are the compiler optimizations in each loop case (index and null termination). As the API was using an integer count, my opinion is to keep it and push it to the callback for 2.0. If null termination is validated to be better, it could be a later rework. Is there something I'm missing? Thoughts?