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 486F0A0353; Tue, 5 Nov 2019 13:51:14 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id C9699493D; Tue, 5 Nov 2019 13:51:13 +0100 (CET) Received: from out2-smtp.messagingengine.com (out2-smtp.messagingengine.com [66.111.4.26]) by dpdk.org (Postfix) with ESMTP id 73D4844C7 for ; Tue, 5 Nov 2019 13:51:12 +0100 (CET) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id C2BE421FA7; Tue, 5 Nov 2019 07:51:11 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute1.internal (MEProxy); Tue, 05 Nov 2019 07:51:11 -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=rIxnGiQGz+vEhdnHo0+IT7DaVp56Z7+/lDhrIHGkw4w=; b=gNQj/VeMQVhh w8qqtXyx47vrVHrDC4AZMHQVhxUlaZm4FrlIFKfH4K7qy+hN7iC4yqDRgSbS+uiU /+5ssjxQlRY/6KUZgS7gGirVpk4l6x9O3bYFX7tPwrSbpw0fuPtdTMaqSh/VgSBz 7SWlzCHoTNz9BkboeI2bOk3JsuVFMRU= 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=rIxnGiQGz+vEhdnHo0+IT7DaVp56Z7+/lDhrIHGkw 4w=; b=H8q6iZXJqaQoesb62MKia1qFtmHKCMrjvSx4lmX4ZXmGi8JxNB/t5Rsi+ L1pmzcPWzSw1gwOgUF2ki+jQWWuTS6QqCOiFRWk5IEgcWYVtiwVRYOL+KpuEe1lO OI1BsuRR14i4cZkPWOIM/tWyIe0p0xLOr8hypxHap8oHMD6Cl0VXWh1Go31iCB2p pBdSN2KDo9ZtBgdmmGcMJeZu16QnfTmtvFILdvRgVy+YEpXhheZpBo/x7J0QhD2/ YoCfIgyTa6OeeLq9RK7cs+ygXypz1sWv0YVfPkuGxrfZpLaJ1LOIULUy64a8XBku zu/0x0cqYWoAMU+i17oUj8itBxITA== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedufedrudduhedggeeiucetufdoteggodetrfdotf 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 94C5C306005C; Tue, 5 Nov 2019 07:51:05 -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 13:51:03 +0100 Message-ID: <2045237.JHmPHeCnFR@xps> In-Reply-To: <5bfadec6-d8b9-049d-0c10-983ec2681e5c@solarflare.com> References: <1569479349-36962-1-git-send-email-orika@mellanox.com> <14821917-596c-06b9-e5db-eaa1c1c8ffbe@intel.com> <5bfadec6-d8b9-049d-0c10-983ec2681e5c@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: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?