From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
To: fengchengwen <fengchengwen@huawei.com>,
Konstantin Ananyev <konstantin.ananyev@intel.com>,
dev@dpdk.org
Cc: xiaoyun.li@intel.com, 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, haiyue.wang@intel.com,
johndale@cisco.com, hyonkim@cisco.com, qi.z.zhang@intel.com,
xiao.w.wang@intel.com, humin29@huawei.com,
yisen.zhuang@huawei.com, oulijun@huawei.com,
beilei.xing@intel.com, jingjing.wu@intel.com,
qiming.yang@intel.com, 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,
chenbo.xia@intel.com, thomas@monjalon.net,
ferruh.yigit@intel.com, mdr@ashroe.eu,
jay.jayatheerthan@intel.com
Subject: Re: [dpdk-dev] [PATCH v5 4/7] ethdev: copy fast-path API into separate structure
Date: Mon, 11 Oct 2021 11:35:37 +0300 [thread overview]
Message-ID: <6493c54a-23bb-7f4a-6645-102db6303ab2@oktetlabs.ru> (raw)
In-Reply-To: <4c57bb9d-21d6-0722-92b8-987283bb8fe6@huawei.com>
On 10/9/21 3:05 PM, fengchengwen wrote:
> On 2021/10/7 19:27, 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 <konstantin.ananyev@intel.com>
>> ---
>> 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");
>> + 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_fp_ops_reset(struct rte_eth_fp_ops *fpo)
>
> The port_id parameter is preferable, this will hide rte_eth_fp_ops as much as possible.
Sorry, but I see no point to hide it inside ethdev.
Of course, prototype should be reconsidered if we make
it ethdev-internal API available for drivers.
If so, I agree that the parameter should be port_id.
[snip]
>> diff --git a/lib/ethdev/ethdev_private.h b/lib/ethdev/ethdev_private.h
>> index 3724429577..5721be7bdc 100644
>> --- a/lib/ethdev/ethdev_private.h
>> +++ b/lib/ethdev/ethdev_private.h
>> @@ -26,4 +26,11 @@ 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 fast-path API to dummy values */
>> +void eth_dev_fp_ops_reset(struct rte_eth_fp_ops *fpo);
>> +
>> +/* setup eth fast-path API to ethdev values */
>> +void eth_dev_fp_ops_setup(struct rte_eth_fp_ops *fpo,
>> + const struct rte_eth_dev *dev);
>
> Some drivers control the transmit/receive function during operation. E.g.
> for hns3 driver, when detect reset, primary process will set rx/tx burst to dummy, after
> process reset, primary process will set the correct rx/tx burst. During this process, the
> send and receive threads are still working, but the bursts they call are changed. So:
> 1. it is recommended that trace be deleted from the dummy function.
> 2. public the eth_dev_fp_ops_reset/setup interface for driver usage.
Good point.
[snip]
>> diff --git a/lib/ethdev/rte_ethdev_core.h b/lib/ethdev/rte_ethdev_core.h
>> index 51cd68de94..d5853dff86 100644
>> --- a/lib/ethdev/rte_ethdev_core.h
>> +++ b/lib/ethdev/rte_ethdev_core.h
>> @@ -50,6 +50,61 @@ 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 pointers to internal ethdev Rx/Tx
>> + * queues data.
>> + * The main purpose to expose these pointers at all - allow compiler
>> + * to fetch this data for fast-path 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-path ethdev functions and related data are hold in a flat array.
>> + * One entry per ethdev.
>> + * On 64-bit systems contents of this structure occupy exactly two 64B lines.
>> + * On 32-bit systems contents of this structure fits into one 64B line.
>> + */
>> +struct rte_eth_fp_ops {
>> +
>> + /**
>> + * Rx fast-path functions and related data.
>> + * 64-bit systems: occupies first 64B line
>> + */
>> + eth_rx_burst_t rx_pkt_burst;
>> + /**< PMD receive 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. */
>> + struct rte_ethdev_qdata rxq;
>> + /**< Rx queues data. */
>> + uintptr_t reserved1[3];
>> +
>> + /**
>> + * Tx fast-path functions and related data.
>> + * 64-bit systems: occupies second 64B line
>> + */
>> + eth_tx_burst_t tx_pkt_burst;
>
> Why not place rx_pkt_burst/tx_pkt_burst/rxq /txq to the first cacheline ?
> Other function, e.g. rx_queue_count/descriptor_status are low frequency call functions.
+1 Very good question
If so, tx_pkt_prepare should be on the first cache-line
as well.
>> + /**< PMD transmit function. */
>> + eth_tx_prep_t tx_pkt_prepare;
>> + /**< PMD transmit prepare function. */
>> + eth_tx_descriptor_status_t tx_descriptor_status;
>> + /**< Check the status of a Tx descriptor. */
>> + struct rte_ethdev_qdata txq;
>> + /**< Tx queues data. */
>> + uintptr_t reserved2[3];
>> +
>> +} __rte_cache_aligned;
>> +
>> +extern struct rte_eth_fp_ops rte_eth_fp_ops[RTE_MAX_ETHPORTS];
>> +
>>
>> /**
>> * @internal
>>
next prev parent reply other threads:[~2021-10-11 8:35 UTC|newest]
Thread overview: 112+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-20 16:28 [dpdk-dev] [RFC 0/7] hide eth dev related structures Konstantin Ananyev
2021-08-20 16:28 ` [dpdk-dev] [RFC 1/7] eth: move ethdev 'burst' API into separate structure Konstantin Ananyev
2021-08-20 16:28 ` [dpdk-dev] [RFC 2/7] eth: make drivers to use new API for Rx Konstantin Ananyev
2021-09-06 18:41 ` Ferruh Yigit
2021-09-14 14:28 ` Ananyev, Konstantin
2021-08-20 16:28 ` [dpdk-dev] [RFC 3/7] eth: make drivers to use new API for Tx Konstantin Ananyev
2021-08-20 16:28 ` [dpdk-dev] [RFC 4/7] eth: make drivers to use new API for Tx prepare Konstantin Ananyev
2021-08-20 16:28 ` [dpdk-dev] [RFC 5/7] eth: make drivers to use new API to obtain descriptor status Konstantin Ananyev
2021-08-20 16:28 ` [dpdk-dev] [RFC 6/7] eth: make drivers to use new API for Rx queue count Konstantin Ananyev
2021-08-20 16:28 ` [dpdk-dev] [RFC 7/7] eth: hide eth dev related structures Konstantin Ananyev
2021-08-26 12:37 ` [dpdk-dev] [RFC 0/7] " Jerin Jacob
2021-09-06 18:09 ` Ferruh Yigit
2021-09-14 13:33 ` Ananyev, Konstantin
2021-09-15 9:45 ` Jerin Jacob
2021-09-22 15:08 ` Ananyev, Konstantin
2021-09-27 16:14 ` Jerin Jacob
2021-09-28 9:37 ` Ananyev, Konstantin
2021-09-22 14:09 ` [dpdk-dev] [RFC v2 0/5] " Konstantin Ananyev
2021-09-22 14:09 ` [dpdk-dev] [RFC v2 1/5] ethdev: allocate max space for internal queue array Konstantin Ananyev
2021-09-22 14:09 ` [dpdk-dev] [RFC v2 2/5] ethdev: change input parameters for rx_queue_count Konstantin Ananyev
2021-09-23 5:51 ` Wang, Haiyue
2021-09-22 14:09 ` [dpdk-dev] [RFC v2 3/5] ethdev: copy ethdev 'burst' API into separate structure Konstantin Ananyev
2021-09-23 5:58 ` Wang, Haiyue
2021-09-27 18:01 ` Jerin Jacob
2021-09-28 9:42 ` Ananyev, Konstantin
2021-09-22 14:09 ` [dpdk-dev] [RFC v2 4/5] ethdev: make burst functions to use new flat array Konstantin Ananyev
2021-09-22 14:09 ` [dpdk-dev] [RFC v2 5/5] ethdev: hide eth dev related structures Konstantin Ananyev
2021-10-01 14:02 ` [dpdk-dev] [PATCH v3 0/7] " Konstantin Ananyev
2021-10-01 14:02 ` [dpdk-dev] [PATCH v3 1/7] ethdev: allocate max space for internal queue array Konstantin Ananyev
2021-10-01 16:48 ` Ferruh Yigit
2021-10-01 14:02 ` [dpdk-dev] [PATCH v3 2/7] ethdev: change input parameters for rx_queue_count Konstantin Ananyev
2021-10-01 14:02 ` [dpdk-dev] [PATCH v3 3/7] ethdev: copy ethdev 'fast' API into separate structure Konstantin Ananyev
2021-10-01 14:02 ` [dpdk-dev] [PATCH v3 4/7] ethdev: make burst functions to use new flat array Konstantin Ananyev
2021-10-01 16:46 ` Ferruh Yigit
2021-10-01 17:40 ` Ananyev, Konstantin
2021-10-04 8:46 ` Ferruh Yigit
2021-10-04 9:20 ` Ananyev, Konstantin
2021-10-04 10:13 ` Ferruh Yigit
2021-10-04 11:17 ` Ananyev, Konstantin
2021-10-01 14:02 ` [dpdk-dev] [PATCH v3 5/7] ethdev: add API to retrieve multiple ethernet addresses Konstantin Ananyev
2021-10-01 14:02 ` [dpdk-dev] [PATCH v3 6/7] ethdev: remove legacy Rx descriptor done API Konstantin Ananyev
2021-10-01 14:02 ` [dpdk-dev] [PATCH v3 7/7] ethdev: hide eth dev related structures Konstantin Ananyev
2021-10-01 16:53 ` Ferruh Yigit
2021-10-01 17:04 ` Ferruh Yigit
2021-10-01 17:02 ` [dpdk-dev] [PATCH v3 0/7] " Ferruh Yigit
2021-10-04 13:55 ` [dpdk-dev] [PATCH v4 " Konstantin Ananyev
2021-10-04 13:55 ` [dpdk-dev] [PATCH v4 1/7] ethdev: allocate max space for internal queue array Konstantin Ananyev
2021-10-05 12:09 ` Thomas Monjalon
2021-10-05 16:45 ` Ananyev, Konstantin
2021-10-05 16:49 ` Thomas Monjalon
2021-10-05 12:21 ` Thomas Monjalon
2021-10-04 13:55 ` [dpdk-dev] [PATCH v4 2/7] ethdev: change input parameters for rx_queue_count Konstantin Ananyev
2021-10-04 13:55 ` [dpdk-dev] [PATCH v4 3/7] ethdev: copy ethdev 'fast' API into separate structure Konstantin Ananyev
2021-10-05 13:09 ` Thomas Monjalon
2021-10-05 16:41 ` Ananyev, Konstantin
2021-10-05 16:48 ` Thomas Monjalon
2021-10-05 17:04 ` Ananyev, Konstantin
2021-10-04 13:56 ` [dpdk-dev] [PATCH v4 4/7] ethdev: make burst functions to use new flat array Konstantin Ananyev
2021-10-05 9:54 ` David Marchand
2021-10-05 10:13 ` Ananyev, Konstantin
2021-10-04 13:56 ` [dpdk-dev] [PATCH v4 5/7] ethdev: add API to retrieve multiple ethernet addresses Konstantin Ananyev
2021-10-05 13:13 ` Thomas Monjalon
2021-10-05 16:35 ` Ananyev, Konstantin
2021-10-05 16:45 ` Thomas Monjalon
2021-10-05 17:12 ` Ananyev, Konstantin
2021-10-05 17:41 ` Thomas Monjalon
2021-10-04 13:56 ` [dpdk-dev] [PATCH v4 6/7] ethdev: remove legacy Rx descriptor done API Konstantin Ananyev
2021-10-05 13:14 ` Thomas Monjalon
2021-10-05 16:21 ` Ananyev, Konstantin
2021-10-04 13:56 ` [dpdk-dev] [PATCH v4 7/7] ethdev: hide eth dev related structures Konstantin Ananyev
2021-10-05 10:04 ` David Marchand
2021-10-05 10:43 ` Ferruh Yigit
2021-10-05 11:37 ` David Marchand
2021-10-05 15:57 ` Ananyev, Konstantin
2021-10-05 13:24 ` Thomas Monjalon
2021-10-05 16:19 ` Ananyev, Konstantin
2021-10-05 16:25 ` Thomas Monjalon
2021-10-06 16:42 ` [dpdk-dev] [PATCH v4 0/7] " Ali Alnubani
2021-10-06 17:26 ` Ali Alnubani
2021-10-07 11:27 ` [dpdk-dev] [PATCH v5 " Konstantin Ananyev
2021-10-07 11:27 ` [dpdk-dev] [PATCH v5 1/7] ethdev: remove legacy Rx descriptor done API Konstantin Ananyev
2021-10-07 11:27 ` [dpdk-dev] [PATCH v5 2/7] ethdev: allocate max space for internal queue array Konstantin Ananyev
2021-10-11 9:20 ` Andrew Rybchenko
2021-10-11 16:25 ` Ananyev, Konstantin
2021-10-11 17:15 ` Andrew Rybchenko
2021-10-11 23:06 ` Ananyev, Konstantin
2021-10-12 5:47 ` Andrew Rybchenko
2021-10-07 11:27 ` [dpdk-dev] [PATCH v5 3/7] ethdev: change input parameters for rx_queue_count Konstantin Ananyev
2021-10-11 8:06 ` Andrew Rybchenko
2021-10-12 17:59 ` Hyong Youb Kim (hyonkim)
2021-10-07 11:27 ` [dpdk-dev] [PATCH v5 4/7] ethdev: copy fast-path API into separate structure Konstantin Ananyev
2021-10-09 12:05 ` fengchengwen
2021-10-11 1:18 ` fengchengwen
2021-10-11 8:39 ` Andrew Rybchenko
2021-10-11 15:24 ` Ananyev, Konstantin
2021-10-11 8:35 ` Andrew Rybchenko [this message]
2021-10-11 15:15 ` Ananyev, Konstantin
2021-10-11 8:25 ` Andrew Rybchenko
2021-10-11 16:52 ` Ananyev, Konstantin
2021-10-11 17:22 ` Andrew Rybchenko
2021-10-07 11:27 ` [dpdk-dev] [PATCH v5 5/7] ethdev: make fast-path functions to use new flat array Konstantin Ananyev
2021-10-11 9:02 ` Andrew Rybchenko
2021-10-11 15:47 ` Ananyev, Konstantin
2021-10-11 17:03 ` Andrew Rybchenko
2021-10-07 11:27 ` [dpdk-dev] [PATCH v5 6/7] ethdev: add API to retrieve multiple ethernet addresses Konstantin Ananyev
2021-10-11 9:09 ` Andrew Rybchenko
2021-10-07 11:27 ` [dpdk-dev] [PATCH v5 7/7] ethdev: hide eth dev related structures Konstantin Ananyev
2021-10-11 9:20 ` Andrew Rybchenko
2021-10-11 15:54 ` Ananyev, Konstantin
2021-10-11 17:04 ` Andrew Rybchenko
2021-10-08 18:13 ` [dpdk-dev] [PATCH v5 0/7] " Slava Ovsiienko
2021-10-11 9:22 ` Andrew Rybchenko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=6493c54a-23bb-7f4a-6645-102db6303ab2@oktetlabs.ru \
--to=andrew.rybchenko@oktetlabs.ru \
--cc=adwivedi@marvell.com \
--cc=ajit.khaparde@broadcom.com \
--cc=anoobj@marvell.com \
--cc=beilei.xing@intel.com \
--cc=chenbo.xia@intel.com \
--cc=dev@dpdk.org \
--cc=ed.czeck@atomicrules.com \
--cc=fengchengwen@huawei.com \
--cc=ferruh.yigit@intel.com \
--cc=haiyue.wang@intel.com \
--cc=heinrich.kuhn@corigine.com \
--cc=hemant.agrawal@nxp.com \
--cc=humin29@huawei.com \
--cc=hyonkim@cisco.com \
--cc=irusskikh@marvell.com \
--cc=jay.jayatheerthan@intel.com \
--cc=jerinj@marvell.com \
--cc=jianwang@trustnetic.com \
--cc=jiawenwu@trustnetic.com \
--cc=jingjing.wu@intel.com \
--cc=john.miller@atomicrules.com \
--cc=johndale@cisco.com \
--cc=kirankumark@marvell.com \
--cc=konstantin.ananyev@intel.com \
--cc=longli@microsoft.com \
--cc=matan@nvidia.com \
--cc=maxime.coquelin@redhat.com \
--cc=mczekaj@marvell.com \
--cc=mdr@ashroe.eu \
--cc=ndabilpuram@marvell.com \
--cc=oulijun@huawei.com \
--cc=qi.z.zhang@intel.com \
--cc=qiming.yang@intel.com \
--cc=rahul.lakkireddy@chelsio.com \
--cc=sachin.saxena@oss.nxp.com \
--cc=shepard.siegel@atomicrules.com \
--cc=somnath.kotur@broadcom.com \
--cc=sthemmin@microsoft.com \
--cc=thomas@monjalon.net \
--cc=viacheslavo@nvidia.com \
--cc=xiao.w.wang@intel.com \
--cc=xiaoyun.li@intel.com \
--cc=yisen.zhuang@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).