From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id E56A3A034F; Mon, 11 Oct 2021 19:22:29 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 79794410E6; Mon, 11 Oct 2021 19:22:29 +0200 (CEST) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by mails.dpdk.org (Postfix) with ESMTP id 13DEB410E1 for ; Mon, 11 Oct 2021 19:22:28 +0200 (CEST) Received: from [192.168.1.192] (unknown [188.242.181.57]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by shelob.oktetlabs.ru (Postfix) with ESMTPSA id 1CA177F514; Mon, 11 Oct 2021 20:22:27 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru 1CA177F514 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1633972947; bh=CNbdvPGtSok4ns0sxDiO6L6Qa1JrBiaMUAeYX7PTXZM=; h=Subject:To:Cc:References:From:Date:In-Reply-To; b=Nu0LCpFV4KHGKN0KyHc2V0WSNYc2AYNnEKnIPbt4AWnuzt0/Mkac3sYErTVbj83oz efm+4tPeQWXEr+PjwGm4/8KWuergb9Ny7t67O1ZatKuxkRZD0CniRfk2lIxZXaieFD KjyQnJx3ZaYRVBfuAFosCJjrJlmFq85PlYAZQIwo= To: "Ananyev, Konstantin" , "dev@dpdk.org" Cc: "Li, Xiaoyun" , "anoobj@marvell.com" , "jerinj@marvell.com" , "ndabilpuram@marvell.com" , "adwivedi@marvell.com" , "shepard.siegel@atomicrules.com" , "ed.czeck@atomicrules.com" , "john.miller@atomicrules.com" , "irusskikh@marvell.com" , "ajit.khaparde@broadcom.com" , "somnath.kotur@broadcom.com" , "rahul.lakkireddy@chelsio.com" , "hemant.agrawal@nxp.com" , "sachin.saxena@oss.nxp.com" , "Wang, Haiyue" , "Daley, John" , "hyonkim@cisco.com" , "Zhang, Qi Z" , "Wang, Xiao W" , "humin29@huawei.com" , "yisen.zhuang@huawei.com" , "oulijun@huawei.com" , "Xing, Beilei" , "Wu, Jingjing" , "Yang, Qiming" , "matan@nvidia.com" , "viacheslavo@nvidia.com" , "sthemmin@microsoft.com" , "longli@microsoft.com" , "heinrich.kuhn@corigine.com" , "kirankumark@marvell.com" , "mczekaj@marvell.com" , "jiawenwu@trustnetic.com" , "jianwang@trustnetic.com" , "maxime.coquelin@redhat.com" , "Xia, Chenbo" , "thomas@monjalon.net" , "Yigit, Ferruh" , "mdr@ashroe.eu" , "Jayatheerthan, Jay" References: <20211004135603.20593-1-konstantin.ananyev@intel.com> <20211007112750.25526-1-konstantin.ananyev@intel.com> <20211007112750.25526-5-konstantin.ananyev@intel.com> <37c19f16-ce0d-533f-2861-176ddf416df7@oktetlabs.ru> From: Andrew Rybchenko Message-ID: Date: Mon, 11 Oct 2021 20:22:26 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.14.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v5 4/7] ethdev: copy fast-path API into separate structure X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 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" On 10/11/21 7:52 PM, Ananyev, Konstantin wrote: > > >> On 10/7/21 2:27 PM, Konstantin Ananyev wrote: >>> Copy public function pointers (rx_pkt_burst(), etc.) and related >>> pointers to internal data from rte_eth_dev structure into a >>> separate flat array. That array will remain in a public header. >>> The intention here is to make rte_eth_dev and related structures internal. >>> That should allow future possible changes to core eth_dev structures >>> to be transparent to the user and help to avoid ABI/API breakages. >>> The plan is to keep minimal part of data from rte_eth_dev public, >>> so we still can use inline functions for fast-path calls >>> (like rte_eth_rx_burst(), etc.) to avoid/minimize slowdown. >>> The whole idea beyond this new schema: >>> 1. PMDs keep to setup fast-path function pointers and related data >>> inside rte_eth_dev struct in the same way they did it before. >>> 2. Inside rte_eth_dev_start() and inside rte_eth_dev_probing_finish() >>> (for secondary process) we call eth_dev_fp_ops_setup, which >>> copies these function and data pointers into rte_eth_fp_ops[port_id]. >>> 3. Inside rte_eth_dev_stop() and inside rte_eth_dev_release_port() >>> we call eth_dev_fp_ops_reset(), which resets rte_eth_fp_ops[port_id] >>> into some dummy values. >>> 4. fast-path ethdev API (rte_eth_rx_burst(), etc.) will use that new >>> flat array to call PMD specific functions. >>> That approach should allow us to make rte_eth_devices[] private >>> without introducing regression and help to avoid changes in drivers code. >>> >>> Signed-off-by: Konstantin Ananyev >> >> Overall LGTM, few nits below. >> >>> --- >>> lib/ethdev/ethdev_private.c | 52 ++++++++++++++++++++++++++++++++++ >>> lib/ethdev/ethdev_private.h | 7 +++++ >>> lib/ethdev/rte_ethdev.c | 27 ++++++++++++++++++ >>> lib/ethdev/rte_ethdev_core.h | 55 ++++++++++++++++++++++++++++++++++++ >>> 4 files changed, 141 insertions(+) >>> >>> diff --git a/lib/ethdev/ethdev_private.c b/lib/ethdev/ethdev_private.c >>> index 012cf73ca2..3eeda6e9f9 100644 >>> --- a/lib/ethdev/ethdev_private.c >>> +++ b/lib/ethdev/ethdev_private.c >>> @@ -174,3 +174,55 @@ rte_eth_devargs_parse_representor_ports(char *str, void *data) >>> RTE_LOG(ERR, EAL, "wrong representor format: %s\n", str); >>> return str == NULL ? -1 : 0; >>> } >>> + >>> +static uint16_t >>> +dummy_eth_rx_burst(__rte_unused void *rxq, >>> + __rte_unused struct rte_mbuf **rx_pkts, >>> + __rte_unused uint16_t nb_pkts) >>> +{ >>> + RTE_ETHDEV_LOG(ERR, "rx_pkt_burst for unconfigured port\n"); >> >> May be "unconfigured" -> "stopped" ? Or "non-started" ? > > Yes, it can be configured but not started. > So 'not started' seems like a better wording here. > Another option probably: 'not ready'. > What people think? Taking into account that some PMds would like to set dummy pointers in some specifics conditions, I think "not ready" is the best option here. > > ... > >> >>> + rte_errno = ENOTSUP; >>> + return 0; >>> +} >>> + >>> +struct rte_eth_fp_ops { >>> + >>> + /** >>> + * Rx fast-path functions and related data. >>> + * 64-bit systems: occupies first 64B line >>> + */ >> >> As I understand the above comment is for a group of below >> fields. If so, Doxygen annocation for member groups should >> be used. > > Ok, and how to do it? > See [1] [1] https://www.doxygen.nl/manual/grouping.html#memgroup