From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.tuxdriver.com (charlotte.tuxdriver.com [70.61.120.58]) by dpdk.org (Postfix) with ESMTP id BFED05A98 for ; Fri, 13 Mar 2015 18:31:54 +0100 (CET) Received: from hmsreliant.think-freely.org ([2001:470:8:a08:7aac:c0ff:fec2:933b] helo=localhost) by smtp.tuxdriver.com with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.63) (envelope-from ) id 1YWTR5-0001Os-BE; Fri, 13 Mar 2015 13:31:52 -0400 Date: Fri, 13 Mar 2015 13:31:46 -0400 From: Neil Horman To: "Mcnamara, John" Message-ID: <20150313173146.GJ28191@hmsreliant.think-freely.org> References: <1426179268-22164-1-git-send-email-john.mcnamara@intel.com> <20150312191540.GB15260@hmsreliant.think-freely.org> <20150313094133.GA5056@bricha3-MOBL3> <20150313134514.GC28191@hmsreliant.think-freely.org> <20150313145002.GA11352@bricha3-MOBL3> <20150313150924.GF28191@hmsreliant.think-freely.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) X-Spam-Score: -2.9 (--) X-Spam-Status: No 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: Fri, 13 Mar 2015 17:31:55 -0000 On Fri, Mar 13, 2015 at 04:26:52PM +0000, Mcnamara, John wrote: > > > > -----Original Message----- > > From: Neil Horman [mailto:nhorman@tuxdriver.com] > > Sent: Friday, March 13, 2015 3:09 PM > > To: Richardson, Bruce > > Cc: Mcnamara, John; dev@dpdk.org > > Subject: Re: [dpdk-dev] [PATCH] ethdev: additional parameter in RX > > callback > > > > Plese set asside the ABI issue for a moment. I get that you're trying to > > get it in prior to needing to version it. Thats not the argument. The > > argument is how best to codify the new information you want to express in > > the callback. For this specific case, I think there are better ways to do > > this than to just blindly add a new parameter. > > Hi Neil, > > I think that is good advice is the general case but this is a very specific case. The modified callback is only used in rte_eth_rx_burst(). For context here is the function in its entirety (without #defs). The substantive change (the addition of nb_pkts) is on the line with an asterisk: > > > static inline uint16_t > rte_eth_rx_burst(uint8_t port_id, uint16_t queue_id, > struct rte_mbuf **rx_pkts, const uint16_t nb_pkts) > { > struct rte_eth_dev *dev; > > dev = &rte_eth_devices[port_id]; > > int16_t nb_rx = (*dev->rx_pkt_burst)(dev->data->rx_queues[queue_id], > rx_pkts, nb_pkts); > > struct rte_eth_rxtx_callback *cb = dev->post_rx_burst_cbs[queue_id]; > > if (unlikely(cb != NULL)) { > do { > nb_rx = cb->fn.rx(port_id, queue_id, rx_pkts, nb_rx, > * nb_pkts, cb->param); > cb = cb->next; > } while (cb != NULL); > } > > return nb_rx; > } > Not sure I grok your point here. Why impact does the number of internal callouts for the callback api have on how we structure the API to external consumers? > > Encoding the array size > > implicitly with a terminating marker lets you use this equally well with > > the tx and rx callbacks (should you ever need it on the latter) > > 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? 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 Neil > John > > > > > > >