From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 921DEA0353; Tue, 5 Nov 2019 14:02:36 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 190594C8B; Tue, 5 Nov 2019 14:02:36 +0100 (CET) Received: from out2-smtp.messagingengine.com (out2-smtp.messagingengine.com [66.111.4.26]) by dpdk.org (Postfix) with ESMTP id 289DF49DF for ; Tue, 5 Nov 2019 14:02:34 +0100 (CET) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id BF01020D0B; Tue, 5 Nov 2019 08:02:32 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Tue, 05 Nov 2019 08:02:32 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=mesmtp; bh=8lX2q+stY764QqyH6tc6qZBhsBfM/kqFtnvQEewwSpw=; b=eQeNITxqB2zS qUzRLoRd6W/t6GcsY7Mzm9UzICFKIaqk15wii/eCE6JiQ3UBJJTa4E9Em1WXnZKH 9d/vySBqcRZ4gIuXTf5bAzUwb3Vfb7O/6J5Y9O341ra6EVe4wxKCLnMcgi51gtfi IkjLqgXm1CbvFj3ZPSGmup4QhdFPvmg= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; bh=8lX2q+stY764QqyH6tc6qZBhsBfM/kqFtnvQEewwS pw=; b=ZuCVeX2qs8gH+Tw90MJb74sQR4Rr7XjISuoc10WUVjs8Z9p5hM0tK/8is qga8E55bwWrMksHBlR4VWgeqHHkfagBwG+viuTCHosMkE/4hdRl0i7ybc9Xor1aw KsHGr9HTRYyoTKIy1p8zxjUNnklq8pyN8fEzo7Oavq9k7pop/P9L7p7MJRUs2rel uaqamlrH0NZJc5P6MnmuVlA1q5BZvaXlBVNJyZxS266rPZimTnHkVWgBdktVdzAi 7P266/MWLa1AeS2S97AMiGSQH+J4ULD1pb8NsxEJM8RjgsI1+hUG67JHLe+wAKRC vtnqGyteA/Yo2csGOJJWC1RfZe2MQ== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedufedrudduhedggeekucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhephffvufffkfgjfhgggfgtsehtufertddttddvnecuhfhrohhmpefvhhhomhgr shcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqnecuff homhgrihhnpeguphgukhdrohhrghenucfkphepleefrddvfedrvdehuddrvddttdenucfr rghrrghmpehmrghilhhfrhhomhepthhhohhmrghssehmohhnjhgrlhhonhdrnhgvthenuc evlhhushhtvghrufhiiigvpedt X-ME-Proxy: Received: from xps.localnet (200.251.23.93.rev.sfr.net [93.23.251.200]) by mail.messagingengine.com (Postfix) with ESMTPA id B6F9080075; Tue, 5 Nov 2019 08:02:25 -0500 (EST) From: Thomas Monjalon To: Andrew Rybchenko Cc: Ferruh Yigit , Ori Kam , John McNamara , Marko Kovacevic , "dev@dpdk.org" , "jingjing.wu@intel.com" , "stephen@networkplumber.org" , Jerin Jacob Date: Tue, 05 Nov 2019 14:02:22 +0100 Message-ID: <3424559.MG90hyF9n4@xps> In-Reply-To: <39704abe-3a35-8879-0dba-068d09951064@solarflare.com> References: <1569479349-36962-1-git-send-email-orika@mellanox.com> <2045237.JHmPHeCnFR@xps> <39704abe-3a35-8879-0dba-068d09951064@solarflare.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH v7 02/14] ethdev: add support for hairpin queue 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 05/11/2019 13:53, Andrew Rybchenko: > On 11/5/19 3:51 PM, Thomas Monjalon wrote: > > 05/11/2019 13:27, Andrew Rybchenko: > >> On 11/5/19 3:23 PM, Ferruh Yigit wrote: > >>> On 11/5/2019 12:12 PM, Andrew Rybchenko wrote: > >>>> On 11/5/19 3:05 PM, Ferruh Yigit wrote: > >>>>> On 11/5/2019 11:49 AM, Andrew Rybchenko wrote: > >>>>>> On 11/5/19 2:36 PM, Ori Kam wrote: > > >>>>>> From: Ferruh Yigit > >>>>>>>>> /** > >>>>>>>>> + * @internal > >>>>>>>>> + * Check if the selected Rx queue is hairpin queue. > >>>>>>>>> + * > >>>>>>>>> + * @param dev > >>>>>>>>> + * Pointer to the selected device. > >>>>>>>>> + * @param queue_id > >>>>>>>>> + * The selected queue. > >>>>>>>>> + * > >>>>>>>>> + * @return > >>>>>>>>> + * - (1) if the queue is hairpin queue, 0 otherwise. > >>>>>>>>> + */ > >>>>>>>>> +int > >>>>>>>>> +rte_eth_dev_is_rx_hairpin_queue(struct rte_eth_dev *dev, uint16_t > >>>>>>>> queue_id); > >>>>>>>>> + > >>>>>>>>> +/** > >>>>>>>>> + * @internal > >>>>>>>>> + * Check if the selected Tx queue is hairpin queue. > >>>>>>>>> + * > >>>>>>>>> + * @param dev > >>>>>>>>> + * Pointer to the selected device. > >>>>>>>>> + * @param queue_id > >>>>>>>>> + * The selected queue. > >>>>>>>>> + * > >>>>>>>>> + * @return > >>>>>>>>> + * - (1) if the queue is hairpin queue, 0 otherwise. > >>>>>>>>> + */ > >>>>>>>>> +int > >>>>>>>>> +rte_eth_dev_is_tx_hairpin_queue(struct rte_eth_dev *dev, uint16_t > >>>>>>>> queue_id); > > [...] > >>>>>>>> These are causing build error, thanks Jerin for catching, because they are > >>>>>>>> internal and called by a public static inline API, so whoever calls > >>>>>>>> 'rte_eth_rx/tx_burst()' APIs in the shared build, can't find > >>>>>>>> 'rte_eth_dev_is_rx/tx_hairpin_queue()' functions [1], > >>>>>>>> > >>>>>>>> as far as I can see there are two options: > >>>>>>>> 1) Remove these checks > >>>>>>>> 2) Make 'rte_eth_dev_is_rx/tx_hairpin_queue()' public API instead of internal > >>>>>>>> > >>>>>>>> If there is a value to make 'rte_eth_dev_is_rx/tx_hairpin_queue()' public API > >>>>>>>> we > >>>>>>>> should go with (2) else (1). > >>>>>>>> > >>>>>>> I think we can skip the tests, > >>>>>>> But it was Andrew request so we must get is response. > >>>>>>> It was also his empathies that they should be internal. > >>>>>> It is important for me to keep rte_eth_dev_state internal and > >>>>>> few patches ago rte_eth_dev_is_rx_hairpin_queue() was inline. > >>>>> Are you saying you don't want to option to make > >>>>> 'rte_eth_dev_is_rx_hairpin_queue()' static inline because it will force the > >>>>> 'RTE_ETH_QUEUE_STATE_xxx' being public? > >>>> Yes. > >>> +1 > >>> > >>>>>> I'm OK to make the function experimental or keep it internal > >>>>>> (no API/ABI stability requirements) but externally visible (in .map). > >>>>> I think we can't do this, add a function deceleration to the public header file > >>>>> and add it to the .map file but keep it internal. Instead we can make it a > >>>>> proper API and it should be experimental at least first release. > >>>> We have discussed similar thing with Olivier recently [1]. > >>>> > >>>> [1] http://inbox.dpdk.org/dev/20191030142938.bpi4txlrebqfq7uw@platinum/ > >>> Yes we can say they are internal but there won't be anything preventing > >>> applications to use them. > >> > >> That's true, but making it internal says - don't use it. > >> Anyway, I have no strong opinion on experimental vs internal. > >> > >>>>> The question above was do we need this API, or instead should remove the check > >>>>> from rx/tx_burst APIs? > >>>> I think these checks are useful to ensure that these functions > >>>> are not used for hairpin queues. At least to catch it with debug > >>>> enabled. > >>> OK, if so what not make them proper API? Any concern on it? > > > > Why we should not use this API in applications? > > I think the valid question is why application needs the API. > Basically I don't mind, just want to be sure that only required > API is exposed. Because hairpin queues are not standard queues, we may need to distinguish them. I see it as a good helper for applications. Am I missing something obvious?