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 2F9B4A0C46; Mon, 27 Sep 2021 20:02:23 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id AD478410DC; Mon, 27 Sep 2021 20:02:22 +0200 (CEST) Received: from mail-io1-f45.google.com (mail-io1-f45.google.com [209.85.166.45]) by mails.dpdk.org (Postfix) with ESMTP id 06CAD40E3C for ; Mon, 27 Sep 2021 20:02:21 +0200 (CEST) Received: by mail-io1-f45.google.com with SMTP id p80so23851835iod.10 for ; Mon, 27 Sep 2021 11:02:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=U+wc22Gi0xrcI2uUb8+3fA71bH2sMpT1dkG5OTu2yYw=; b=M0V+2mJE/IuKW/LmABG9gW1nQRqGsmspUD2kGAFMxYTFKsJUgEL+YEjkF3ZdLyasbX d7NqD+ltj7wKKXgue1ajgrJ6pme6RrXgx5Osr1MHfWDaocurEQUhFQf+S45uRFobWYC+ GUHvkLinS9Ds8yuba5KiCUO2VugFOG1zPR0B35/4gjuH7wAwHF2YBsNjN+q31DycquHU e0mP7CxudjK1UCl3d8KTcxJXaXKeT9aFjGOFgK/k1L9Qhui8uQuYLQph7GIYdGm5bsYc xWGZtvANFaPFfT2zBWlEcqfBuoEBU+kgzDup6XYISDbDuNZGvJKBlw0Fjcq1L0hjStQE +FYA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=U+wc22Gi0xrcI2uUb8+3fA71bH2sMpT1dkG5OTu2yYw=; b=ogeaaW/56rGNb5OIpK9jdOXfokeWqEWdOfaPSymVW+iio4UKwyriB/BAwRI4I4lV6G I2a5I5GZBj0oe58nMjKDpU7C7sniVh5uXrOD0B1cGx5lrFEeo7eaUKMg31eNijf4N0QZ ULq2zWuL/7MOfcFHWtAQohMKcfVEts7Ev8nkZih7fToQj2Pj7v8u+5GCwydp6612A/4F i89xTkuElMNdAv2COOKohhPAKvGl/e/2qzQp9QsaKklrdauvO+1NjqNAdl5uiso7lkx1 lk/Sj5kgON/Kl5A258zWS0zudmcKw3rm01QK3AfCj3/7g3oY1PIF6otsbW1UVwTV7zlj GQ4Q== X-Gm-Message-State: AOAM530HD2ztXpnEPW1u3AiPML5hUwMrWxp3Bmw2TRkLMNdxNlxIctYI NdUINpZ3O99jkGkyzZiVeStoHh/zhV7u051syN0= X-Google-Smtp-Source: ABdhPJxmhZDVT0Zd9e4XueKebXORsJPWOcN4To06Wfsq0M3rMmdVo+wWk1MA35PhFSQqKB0Hdw2QJF4RViCg9bVB7wA= X-Received: by 2002:a5d:88d7:: with SMTP id i23mr736959iol.38.1632765741277; Mon, 27 Sep 2021 11:02:21 -0700 (PDT) MIME-Version: 1.0 References: <20210820162834.12544-1-konstantin.ananyev@intel.com> <20210922140953.19388-1-konstantin.ananyev@intel.com> <20210922140953.19388-4-konstantin.ananyev@intel.com> In-Reply-To: From: Jerin Jacob Date: Mon, 27 Sep 2021 23:31:55 +0530 Message-ID: To: "Wang, Haiyue" Cc: "Ananyev, Konstantin" , "dev@dpdk.org" , "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" , "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" , "andrew.rybchenko@oktetlabs.ru" , "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" Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-dev] [RFC v2 3/5] ethdev: copy ethdev 'burst' 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 Thu, Sep 23, 2021 at 11:28 AM Wang, Haiyue wrote: > > > -----Original Message----- > > From: Ananyev, Konstantin > > Sent: Wednesday, September 22, 2021 22:10 > > To: 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; andrew.rybchenko@oktetlabs.ru; > > 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 ; Ananyev, Konstantin > > > > Subject: [RFC v2 3/5] ethdev: copy ethdev 'burst' API into separate structure > > > > Copy public function pointers (rx_pkt_burst(), etc.) and related > > pointers to internal data from rte_eth_dev structure into a separate flat > > array. We can keep it public to still use inline functions for 'fast' calls > > (like rte_eth_rx_burst(), etc.) to avoid/minimize slowdown. > > The intention is to make rte_eth_dev and related structures internal. > > That should allow future possible changes to core eth_dev strcutures > > to be transaprent to the user and help to avoid ABI/API breakages. > > > > Signed-off-by: Konstantin Ananyev > > --- > > lib/ethdev/ethdev_private.c | 53 ++++++++++++++++++++++++++++++++++++ > > lib/ethdev/ethdev_private.h | 7 +++++ > > lib/ethdev/rte_ethdev.c | 17 ++++++++++++ > > lib/ethdev/rte_ethdev_core.h | 45 ++++++++++++++++++++++++++++++ > > 4 files changed, 122 insertions(+) > > > > diff --git a/lib/ethdev/ethdev_private.c b/lib/ethdev/ethdev_private.c > > index 012cf73ca2..a1683da77b 100644 > > --- a/lib/ethdev/ethdev_private.c > > +++ b/lib/ethdev/ethdev_private.c > > @@ -174,3 +174,56 @@ 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"); > > + rte_errno = ENOTSUP; > > + return 0; > > +} > > + > > +static uint16_t > > +dummy_eth_tx_burst(__rte_unused void *txq, > > + __rte_unused struct rte_mbuf **tx_pkts, > > + __rte_unused uint16_t nb_pkts) > > +{ > > + RTE_ETHDEV_LOG(ERR, "tx_pkt_burst for unconfigured port\n"); > > + rte_errno = ENOTSUP; > > + return 0; > > +} > > + > > +void > > +eth_dev_burst_api_reset(struct rte_eth_burst_api *rba) > > +{ > > + static void *dummy_data[RTE_MAX_QUEUES_PER_PORT]; > > + static const struct rte_eth_burst_api dummy_api = { > > + .rx_pkt_burst = dummy_eth_rx_burst, > > + .tx_pkt_burst = dummy_eth_tx_burst, > > + .rxq = {.data = dummy_data, .clbk = dummy_data,}, > > + .txq = {.data = dummy_data, .clbk = dummy_data,}, > > + }; > > + > > + *rba = dummy_api; > > +} > > + > > +void > > +eth_dev_burst_api_setup(struct rte_eth_burst_api *rba, > > + const struct rte_eth_dev *dev) > > +{ > > + rba->rx_pkt_burst = dev->rx_pkt_burst; > > + rba->tx_pkt_burst = dev->tx_pkt_burst; > > + rba->tx_pkt_prepare = dev->tx_pkt_prepare; > > + rba->rx_queue_count = dev->rx_queue_count; > > + rba->rx_descriptor_status = dev->rx_descriptor_status; > > + rba->tx_descriptor_status = dev->tx_descriptor_status; > > + > > + rba->rxq.data = dev->data->rx_queues; > > + rba->rxq.clbk = (void **)(uintptr_t)dev->post_rx_burst_cbs; > > + > > + rba->txq.data = dev->data->tx_queues; > > + rba->txq.clbk = (void **)(uintptr_t)dev->pre_tx_burst_cbs; > > +} > > + > > diff --git a/lib/ethdev/ethdev_private.h b/lib/ethdev/ethdev_private.h > > index 9bb0879538..54921f4860 100644 > > --- a/lib/ethdev/ethdev_private.h > > +++ b/lib/ethdev/ethdev_private.h > > @@ -30,6 +30,13 @@ eth_find_device(const struct rte_eth_dev *_start, rte_eth_cmp_t cmp, > > /* Parse devargs value for representor parameter. */ > > int rte_eth_devargs_parse_representor_ports(char *str, void *data); > > > > +/* reset eth 'burst' API to dummy values */ > > +void eth_dev_burst_api_reset(struct rte_eth_burst_api *rba); > > + > > +/* setup eth 'burst' API to ethdev values */ > > +void eth_dev_burst_api_setup(struct rte_eth_burst_api *rba, > > + const struct rte_eth_dev *dev); > > + > > #ifdef __cplusplus > > } > > #endif > > diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c > > index 424bc260fa..5904bb7bae 100644 > > --- a/lib/ethdev/rte_ethdev.c > > +++ b/lib/ethdev/rte_ethdev.c > > @@ -44,6 +44,9 @@ > > static const char *MZ_RTE_ETH_DEV_DATA = "rte_eth_dev_data"; > > struct rte_eth_dev rte_eth_devices[RTE_MAX_ETHPORTS]; > > > > +/* public 'fast/burst' API */ > > +struct rte_eth_burst_api rte_eth_burst_api[RTE_MAX_ETHPORTS]; > > + > > /* spinlock for eth device callbacks */ > > static rte_spinlock_t eth_dev_cb_lock = RTE_SPINLOCK_INITIALIZER; > > > > @@ -1788,6 +1791,9 @@ rte_eth_dev_start(uint16_t port_id) > > (*dev->dev_ops->link_update)(dev, 0); > > } > > > > + /* expose selection of PMD rx/tx function */ > > + eth_dev_burst_api_setup(rte_eth_burst_api + port_id, dev); > > + > > rte_ethdev_trace_start(port_id); > > return 0; > > } > > @@ -1810,6 +1816,9 @@ rte_eth_dev_stop(uint16_t port_id) > > return 0; > > } > > > > + /* point rx/tx functions to dummy ones */ > > + eth_dev_burst_api_reset(rte_eth_burst_api + port_id); > > + > > dev->data->dev_started = 0; > > ret = (*dev->dev_ops->dev_stop)(dev); > > rte_ethdev_trace_stop(port_id, ret); > > @@ -4568,6 +4577,14 @@ rte_eth_mirror_rule_reset(uint16_t port_id, uint8_t rule_id) > > return eth_err(port_id, (*dev->dev_ops->mirror_rule_reset)(dev, rule_id)); > > } > > > > +RTE_INIT(eth_dev_init_burst_api) > > +{ > > + uint32_t i; > > + > > + for (i = 0; i != RTE_DIM(rte_eth_burst_api); i++) > > + eth_dev_burst_api_reset(rte_eth_burst_api + i); > > +} > > + > > RTE_INIT(eth_dev_init_cb_lists) > > { > > uint16_t i; > > diff --git a/lib/ethdev/rte_ethdev_core.h b/lib/ethdev/rte_ethdev_core.h > > index 00f27c643a..da6de5de43 100644 > > --- a/lib/ethdev/rte_ethdev_core.h > > +++ b/lib/ethdev/rte_ethdev_core.h > > @@ -53,6 +53,51 @@ typedef int (*eth_rx_descriptor_status_t)(void *rxq, uint16_t offset); > > typedef int (*eth_tx_descriptor_status_t)(void *txq, uint16_t offset); > > /**< @internal Check the status of a Tx descriptor */ > > > > +/** > > + * @internal > > + * Structure used to hold opaque pointernals to internal ethdev RX/TXi > > + * queues data. > > + * The main purpose to expose these pointers at all - allow compiler > > + * to fetch this data for 'fast' ethdev inline functions in advance. > > + */ > > +struct rte_ethdev_qdata { > > + void **data; > > + /**< points to array of internal queue data pointers */ > > + void **clbk; > > + /**< points to array of queue callback data pointers */ > > +}; > > + > > +/** > > + * @internal > > + * 'fast' ethdev funcions and related data are hold in a flat array. > > + * one entry per ethdev. > > + */ > > +struct rte_eth_burst_api { > > 'ops' is better ? Like "struct rte_eth_burst_ops". ;-) Since all fastpath APIs are not in bust in nature. IMO, rte_eth_fp_ops or so may be better. > > > + > > + /** first 64B line */ > > + eth_rx_burst_t rx_pkt_burst; > > + /**< PMD receive function. */ > > + eth_tx_burst_t tx_pkt_burst; > > + /**< PMD transmit function. */ > > + eth_tx_prep_t tx_pkt_prepare; > > + /**< PMD transmit prepare function. */ > > + eth_rx_queue_count_t rx_queue_count; > > + /**< Get the number of used RX descriptors. */ > > + eth_rx_descriptor_status_t rx_descriptor_status; > > + /**< Check the status of a Rx descriptor. */ > > + eth_tx_descriptor_status_t tx_descriptor_status; > > + /**< Check the status of a Tx descriptor. */ > > + uintptr_t reserved[2]; > > + > > How about 32 bit system ? Does it need something like : > > __rte_cache_aligned for rxq to make sure 64B line? __rte_cache_aligned_min for 128B CL systems. > > > + /** second 64B line */ > > + struct rte_ethdev_qdata rxq; > > + struct rte_ethdev_qdata txq; > > + uintptr_t reserved2[4]; > > + > > +} __rte_cache_aligned; > > + > > +extern struct rte_eth_burst_api rte_eth_burst_api[RTE_MAX_ETHPORTS]; > > + > > > > /** > > * @internal > > -- > > 2.26.3 >