diff mbox series

[v4,7/7] ethdev: hide eth dev related structures

Message ID 20211004135603.20593-8-konstantin.ananyev@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers show
Series hide eth dev related structures | expand

Checks

Context Check Description
ci/intel-Testing success Testing PASS
ci/Intel-compilation success Compilation OK
ci/iol-x86_64-compile-testing fail Testing issues
ci/iol-x86_64-unit-testing fail Testing issues
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-spell-check-testing warning Testing issues
ci/checkpatch success coding style OK

Commit Message

Ananyev, Konstantin Oct. 4, 2021, 1:56 p.m. UTC
Move rte_eth_dev, rte_eth_dev_data, rte_eth_rxtx_callback and related
data into private header (ethdev_driver.h).
Few minor changes to keep DPDK building after that.

Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
---
 doc/guides/rel_notes/release_21_11.rst        |   6 +
 drivers/common/octeontx2/otx2_sec_idev.c      |   2 +-
 drivers/crypto/octeontx2/otx2_cryptodev_ops.c |   2 +-
 drivers/net/cxgbe/base/adapter.h              |   2 +-
 drivers/net/dpaa2/dpaa2_ptp.c                 |   2 +-
 drivers/net/netvsc/hn_var.h                   |   1 +
 lib/ethdev/ethdev_driver.h                    | 149 ++++++++++++++++++
 lib/ethdev/rte_ethdev_core.h                  | 143 -----------------
 lib/ethdev/version.map                        |   2 +-
 lib/eventdev/rte_event_eth_rx_adapter.c       |   2 +-
 lib/eventdev/rte_event_eth_tx_adapter.c       |   2 +-
 lib/eventdev/rte_eventdev.c                   |   2 +-
 lib/metrics/rte_metrics_telemetry.c           |   2 +-
 13 files changed, 165 insertions(+), 152 deletions(-)

Comments

David Marchand Oct. 5, 2021, 10:04 a.m. UTC | #1
On Mon, Oct 4, 2021 at 3:59 PM Konstantin Ananyev
<konstantin.ananyev@intel.com> wrote:
>
> Move rte_eth_dev, rte_eth_dev_data, rte_eth_rxtx_callback and related
> data into private header (ethdev_driver.h).
> Few minor changes to keep DPDK building after that.

This change is going to hurt a lot of people :-).
But this is a necessary move.

$ git grep-all -lw rte_eth_devices |grep -v \\.patch$
ANS/ans/ans_main.c
BESS/core/drivers/pmd.cc
dma_ip_drivers/QDMA/DPDK/drivers/net/qdma/qdma_xdebug.c
dma_ip_drivers/QDMA/DPDK/drivers/net/qdma/rte_pmd_qdma.c
dma_ip_drivers/QDMA/DPDK/examples/qdma_testapp/pcierw.c
dma_ip_drivers/QDMA/DPDK/examples/qdma_testapp/testapp.c
FD.io-VPP/src/plugins/dpdk/device/format.c
lagopus/src/dataplane/dpdk/dpdk_io.c
OVS/lib/netdev-dpdk.c
packet-journey/app/kni.c
pktgen-dpdk/app/pktgen-port-cfg.c
pktgen-dpdk/app/pktgen-port-cfg.h
pktgen-dpdk/app/pktgen-stats.c
Trex/src/dpdk_funcs.c
Trex/src/drivers/trex_i40e_fdir.c
Trex/src/drivers/trex_ixgbe_fdir.c
TungstenFabric-vRouter/gdb/vr_dpdk.gdb


I did not check all projects for their uses of rte_eth_devices, but I
did the job for OVS.
If you have cycles to review...
https://patchwork.ozlabs.org/project/openvswitch/patch/20210907082343.16370-1-david.marchand@redhat.com/

One nit:

>
> Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> ---
>  doc/guides/rel_notes/release_21_11.rst        |   6 +
>  drivers/common/octeontx2/otx2_sec_idev.c      |   2 +-
>  drivers/crypto/octeontx2/otx2_cryptodev_ops.c |   2 +-
>  drivers/net/cxgbe/base/adapter.h              |   2 +-
>  drivers/net/dpaa2/dpaa2_ptp.c                 |   2 +-
>  drivers/net/netvsc/hn_var.h                   |   1 +
>  lib/ethdev/ethdev_driver.h                    | 149 ++++++++++++++++++
>  lib/ethdev/rte_ethdev_core.h                  | 143 -----------------
>  lib/ethdev/version.map                        |   2 +-
>  lib/eventdev/rte_event_eth_rx_adapter.c       |   2 +-
>  lib/eventdev/rte_event_eth_tx_adapter.c       |   2 +-
>  lib/eventdev/rte_eventdev.c                   |   2 +-
>  lib/metrics/rte_metrics_telemetry.c           |   2 +-
>  13 files changed, 165 insertions(+), 152 deletions(-)
>
> diff --git a/doc/guides/rel_notes/release_21_11.rst b/doc/guides/rel_notes/release_21_11.rst
> index 6055551443..2944149943 100644
> --- a/doc/guides/rel_notes/release_21_11.rst
> +++ b/doc/guides/rel_notes/release_21_11.rst
> @@ -228,6 +228,12 @@ ABI Changes
>    to user, it still counts as an ABI change, as ``eth_rx_queue_count_t``
>    is used by  public inline function ``rte_eth_rx_queue_count``.
>
> +* ethdev: Made ``rte_eth_dev``, ``rte_eth_dev_data``, ``rte_eth_rxtx_callback``
> +  private data structures. ``rte_eth_devices[]`` can't be accessible directly

accessed*

> +  by user any more. While it is an ABI breakage, this change is intended
> +  to be transparent for both users (no changes in user app is required) and
> +  PMD developers (no changes in PMD is required).
> +
>
>  Known Issues
>  ------------
Ferruh Yigit Oct. 5, 2021, 10:43 a.m. UTC | #2
On 10/5/2021 11:04 AM, David Marchand wrote:
> On Mon, Oct 4, 2021 at 3:59 PM Konstantin Ananyev
> <konstantin.ananyev@intel.com> wrote:
>>
>> Move rte_eth_dev, rte_eth_dev_data, rte_eth_rxtx_callback and related
>> data into private header (ethdev_driver.h).
>> Few minor changes to keep DPDK building after that.
> 
> This change is going to hurt a lot of people :-).
> But this is a necessary move.
> 

+1 that it is necessary move, but I am surprised to see how much 'rte_eth_devices'
is accessed directly.

Do you have any idea/suggestion on how can we reduce the pain for them?

> $ git grep-all -lw rte_eth_devices |grep -v \\.patch$
> ANS/ans/ans_main.c
> BESS/core/drivers/pmd.cc
> dma_ip_drivers/QDMA/DPDK/drivers/net/qdma/qdma_xdebug.c
> dma_ip_drivers/QDMA/DPDK/drivers/net/qdma/rte_pmd_qdma.c
> dma_ip_drivers/QDMA/DPDK/examples/qdma_testapp/pcierw.c
> dma_ip_drivers/QDMA/DPDK/examples/qdma_testapp/testapp.c
> FD.io-VPP/src/plugins/dpdk/device/format.c
> lagopus/src/dataplane/dpdk/dpdk_io.c
> OVS/lib/netdev-dpdk.c
> packet-journey/app/kni.c
> pktgen-dpdk/app/pktgen-port-cfg.c
> pktgen-dpdk/app/pktgen-port-cfg.h
> pktgen-dpdk/app/pktgen-stats.c
> Trex/src/dpdk_funcs.c
> Trex/src/drivers/trex_i40e_fdir.c
> Trex/src/drivers/trex_ixgbe_fdir.c
> TungstenFabric-vRouter/gdb/vr_dpdk.gdb
> 
> 
> I did not check all projects for their uses of rte_eth_devices, but I
> did the job for OVS.
> If you have cycles to review...
> https://patchwork.ozlabs.org/project/openvswitch/patch/20210907082343.16370-1-david.marchand@redhat.com/
> 
> One nit:
> 
>>
>> Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
>> ---
>>   doc/guides/rel_notes/release_21_11.rst        |   6 +
>>   drivers/common/octeontx2/otx2_sec_idev.c      |   2 +-
>>   drivers/crypto/octeontx2/otx2_cryptodev_ops.c |   2 +-
>>   drivers/net/cxgbe/base/adapter.h              |   2 +-
>>   drivers/net/dpaa2/dpaa2_ptp.c                 |   2 +-
>>   drivers/net/netvsc/hn_var.h                   |   1 +
>>   lib/ethdev/ethdev_driver.h                    | 149 ++++++++++++++++++
>>   lib/ethdev/rte_ethdev_core.h                  | 143 -----------------
>>   lib/ethdev/version.map                        |   2 +-
>>   lib/eventdev/rte_event_eth_rx_adapter.c       |   2 +-
>>   lib/eventdev/rte_event_eth_tx_adapter.c       |   2 +-
>>   lib/eventdev/rte_eventdev.c                   |   2 +-
>>   lib/metrics/rte_metrics_telemetry.c           |   2 +-
>>   13 files changed, 165 insertions(+), 152 deletions(-)
>>
>> diff --git a/doc/guides/rel_notes/release_21_11.rst b/doc/guides/rel_notes/release_21_11.rst
>> index 6055551443..2944149943 100644
>> --- a/doc/guides/rel_notes/release_21_11.rst
>> +++ b/doc/guides/rel_notes/release_21_11.rst
>> @@ -228,6 +228,12 @@ ABI Changes
>>     to user, it still counts as an ABI change, as ``eth_rx_queue_count_t``
>>     is used by  public inline function ``rte_eth_rx_queue_count``.
>>
>> +* ethdev: Made ``rte_eth_dev``, ``rte_eth_dev_data``, ``rte_eth_rxtx_callback``
>> +  private data structures. ``rte_eth_devices[]`` can't be accessible directly
> 
> accessed*
> 
>> +  by user any more. While it is an ABI breakage, this change is intended
>> +  to be transparent for both users (no changes in user app is required) and
>> +  PMD developers (no changes in PMD is required).
>> +
>>
>>   Known Issues
>>   ------------
> 
> 
>
David Marchand Oct. 5, 2021, 11:37 a.m. UTC | #3
On Tue, Oct 5, 2021 at 12:43 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> > This change is going to hurt a lot of people :-).
> > But this is a necessary move.
> >
>
> +1 that it is necessary move, but I am surprised to see how much 'rte_eth_devices'
> is accessed directly.
>
> Do you have any idea/suggestion on how can we reduce the pain for them?

From what I see, ethdev iterators are probably something that is not
known enough.
rte_eth_dev_info_get() also fills some other spots.
I don't have a magic answer, people need to look at existing API.


But I just scratched the surface, looking at rte_eth_devices[] accesses.
There might be other rte_eth_dev object dereferences (you get one
point calling rte_eth_dev_info_get) that my grep did not catch.


Details:

>
> > $ git grep-all -lw rte_eth_devices |grep -v \\.patch$
> > ANS/ans/ans_main.c

I think this code is just lagging behind what ethdev currently
provides/does wrt default offload config.
This code probably does not need to dereference rte_eth_devices[] to
query offloads configuration.


> > BESS/core/drivers/pmd.cc

ethdev iterators can replace those accesses.


> > dma_ip_drivers/QDMA/DPDK/drivers/net/qdma/qdma_xdebug.c
> > dma_ip_drivers/QDMA/DPDK/drivers/net/qdma/rte_pmd_qdma.c
> > dma_ip_drivers/QDMA/DPDK/examples/qdma_testapp/pcierw.c
> > dma_ip_drivers/QDMA/DPDK/examples/qdma_testapp/testapp.c

This is a DPDK clone, with an additional driver, so irrelevant.


> > FD.io-VPP/src/plugins/dpdk/device/format.c

rte_eth_rx_burst_mode_get() and rte_eth_tx_burst_mode_get() should do the job.
I wonder if those APIs were introduced in DPDK for VPP.. ?


> > lagopus/src/dataplane/dpdk/dpdk_io.c

Idem, ethdev iterators and rte_eth_dev_info_get() instead of direct
access for dev_flags.


> > OVS/lib/netdev-dpdk.c

For OVS, it was ethdev iterators + rte_eth_dev_info_get() where necessary.


> > packet-journey/app/kni.c

There might be something missing in current ethdev API.
This app wants to know if device is started... but on the other hand,
that's probably something the app tracks itself.


> > pktgen-dpdk/app/pktgen-port-cfg.c
> > pktgen-dpdk/app/pktgen-port-cfg.h
> > pktgen-dpdk/app/pktgen-stats.c

Accesses to offload configuration which I think are unneeded (like ANS).
Direct access for dev_flags, can be replaced with rte_eth_dev_info_get.
Direct access for name, can be replaced with rte_eth_dev_info_get.


> > Trex/src/dpdk_funcs.c
> > Trex/src/drivers/trex_i40e_fdir.c
> > Trex/src/drivers/trex_ixgbe_fdir.c

Here, there is some horror.
Directly casting and accessing hardware:
    struct rte_eth_dev *dev = &rte_eth_devices[repid];
        struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);

        I40E_WRITE_REG(hw, I40E_GLQF_ORT(12), 0x00000062);
        I40E_WRITE_REG(hw, I40E_GLQF_PIT(2), 0x000024A0);
        I40E_WRITE_REG(hw,
I40E_PRTQF_FD_INSET(I40E_FILTER_PCTYPE_NONF_IPV4_UDP, 0), 0);
etc...

This code probably bypasses too much of dpdk API, I stopped at this.


> > TungstenFabric-vRouter/gdb/vr_dpdk.gdb

Mm, interesting, this part displays DPDK internals from gdb.
That's something I have in my todolist for a long time, providing some
common gdb scripts in DPDK...
Thomas Monjalon Oct. 5, 2021, 1:24 p.m. UTC | #4
04/10/2021 15:56, Konstantin Ananyev:
> Move rte_eth_dev, rte_eth_dev_data, rte_eth_rxtx_callback and related
> data into private header (ethdev_driver.h).
[...]
> +/**
> + * @internal
> + * Structure used to hold information about the callbacks to be called for a
> + * queue on RX and TX.
> + */
> +struct rte_eth_rxtx_callback {
> +	struct rte_eth_rxtx_callback *next;
> +	union{
> +		rte_rx_callback_fn rx;
> +		rte_tx_callback_fn tx;
> +	} fn;
> +	void *param;
> +};
> +
> +/**
> + * @internal
> + * The generic data structure associated with each ethernet device.
> + *
> + * Pointers to burst-oriented packet receive and transmit functions are
> + * located at the beginning of the structure, along with the pointer to
> + * where all the data elements for the particular device are stored in shared
> + * memory. This split allows the function pointer and driver data to be per-
> + * process, while the actual configuration data for the device is shared.
> + */
> +struct rte_eth_dev {
> +	eth_rx_burst_t rx_pkt_burst; /**< Pointer to PMD receive function. */
> +	eth_tx_burst_t tx_pkt_burst; /**< Pointer to PMD transmit function. */
> +	eth_tx_prep_t tx_pkt_prepare;
> +	/**< Pointer to 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. */

Why not using the new struct rte_eth_fp_ops?

> +
> +	/**
> +	 * Next two fields are per-device data but *data is shared between
> +	 * primary and secondary processes and *process_private is per-process
> +	 * private. The second one is managed by PMDs if necessary.
> +	 */
> +	struct rte_eth_dev_data *data;  /**< Pointer to device data. */

We should mention that "data" is shared between processes.

> +	void *process_private; /**< Pointer to per-process device data. */
> +	const struct eth_dev_ops *dev_ops; /**< Functions exported by PMD */
> +	struct rte_device *device; /**< Backing device */
> +	struct rte_intr_handle *intr_handle; /**< Device interrupt handle */
> +	/** User application callbacks for NIC interrupts */
> +	struct rte_eth_dev_cb_list link_intr_cbs;
> +	/**
> +	 * User-supplied functions called from rx_burst to post-process
> +	 * received packets before passing them to the user
> +	 */
> +	struct rte_eth_rxtx_callback *post_rx_burst_cbs[RTE_MAX_QUEUES_PER_PORT];
> +	/**
> +	 * User-supplied functions called from tx_burst to pre-process
> +	 * received packets before passing them to the driver for transmission.
> +	 */
> +	struct rte_eth_rxtx_callback *pre_tx_burst_cbs[RTE_MAX_QUEUES_PER_PORT];
> +	enum rte_eth_dev_state state; /**< Flag indicating the port state */
> +	void *security_ctx; /**< Context for security ops */
> +
> +	uint64_t reserved_64s[4]; /**< Reserved for future fields */
> +	void *reserved_ptrs[4];   /**< Reserved for future fields */
> +} __rte_cache_aligned;
> +
> +struct rte_eth_dev_sriov;
> +struct rte_eth_dev_owner;
> +
> +/**
> + * @internal
> + * The data part, with no function pointers, associated with each ethernet
> + * device. This structure is safe to place in shared memory to be common
> + * among different processes in a multi-process configuration.
> + */
> +struct rte_eth_dev_data {
> +	char name[RTE_ETH_NAME_MAX_LEN]; /**< Unique identifier name */
> +
> +	void **rx_queues; /**< Array of pointers to RX queues. */
> +	void **tx_queues; /**< Array of pointers to TX queues. */
> +	uint16_t nb_rx_queues; /**< Number of RX queues. */
> +	uint16_t nb_tx_queues; /**< Number of TX queues. */
> +
> +	struct rte_eth_dev_sriov sriov;    /**< SRIOV data */
> +
> +	void *dev_private;
> +			/**< PMD-specific private data.
> +			 *   @see rte_eth_dev_release_port()
> +			 */
> +
> +	struct rte_eth_link dev_link;   /**< Link-level information & status. */
> +	struct rte_eth_conf dev_conf;   /**< Configuration applied to device. */
> +	uint16_t mtu;                   /**< Maximum Transmission Unit. */
> +	uint32_t min_rx_buf_size;
> +			/**< Common RX buffer size handled by all queues. */
> +
> +	uint64_t rx_mbuf_alloc_failed; /**< RX ring mbuf allocation failures. */
> +	struct rte_ether_addr *mac_addrs;
> +			/**< Device Ethernet link address.
> +			 *   @see rte_eth_dev_release_port()
> +			 */
> +	uint64_t mac_pool_sel[ETH_NUM_RECEIVE_MAC_ADDR];
> +			/**< Bitmap associating MAC addresses to pools. */
> +	struct rte_ether_addr *hash_mac_addrs;
> +			/**< Device Ethernet MAC addresses of hash filtering.
> +			 *   @see rte_eth_dev_release_port()
> +			 */
> +	uint16_t port_id;           /**< Device [external] port identifier. */
> +
> +	__extension__
> +	uint8_t promiscuous   : 1,
> +		/**< RX promiscuous mode ON(1) / OFF(0). */
> +		scattered_rx : 1,
> +		/**< RX of scattered packets is ON(1) / OFF(0) */
> +		all_multicast : 1,
> +		/**< RX all multicast mode ON(1) / OFF(0). */
> +		dev_started : 1,
> +		/**< Device state: STARTED(1) / STOPPED(0). */
> +		lro         : 1,
> +		/**< RX LRO is ON(1) / OFF(0) */
> +		dev_configured : 1;
> +		/**< Indicates whether the device is configured.
> +		 *   CONFIGURED(1) / NOT CONFIGURED(0).
> +		 */
> +	uint8_t rx_queue_state[RTE_MAX_QUEUES_PER_PORT];
> +		/**< Queues state: HAIRPIN(2) / STARTED(1) / STOPPED(0). */
> +	uint8_t tx_queue_state[RTE_MAX_QUEUES_PER_PORT];
> +		/**< Queues state: HAIRPIN(2) / STARTED(1) / STOPPED(0). */
> +	uint32_t dev_flags;             /**< Capabilities. */
> +	int numa_node;                  /**< NUMA node connection. */
> +	struct rte_vlan_filter_conf vlan_filter_conf;
> +			/**< VLAN filter configuration. */
> +	struct rte_eth_dev_owner owner; /**< The port owner. */
> +	uint16_t representor_id;
> +			/**< Switch-specific identifier.
> +			 *   Valid if RTE_ETH_DEV_REPRESENTOR in dev_flags.
> +			 */
> +
> +	pthread_mutex_t flow_ops_mutex; /**< rte_flow ops mutex. */
> +	uint64_t reserved_64s[4]; /**< Reserved for future fields */
> +	void *reserved_ptrs[4];   /**< Reserved for future fields */
> +} __rte_cache_aligned;
> +
> +/**
> + * @internal
> + * The pool of *rte_eth_dev* structures. The size of the pool
> + * is configured at compile-time in the <rte_ethdev.c> file.
> + */
> +extern struct rte_eth_dev rte_eth_devices[];

Later we should add a function to configure the size of this array dynamically
in the early DPDK init stage.
Ananyev, Konstantin Oct. 5, 2021, 3:57 p.m. UTC | #5
> 
> On Tue, Oct 5, 2021 at 12:43 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> > > This change is going to hurt a lot of people :-).
> > > But this is a necessary move.
> > >
> >
> > +1 that it is necessary move, but I am surprised to see how much 'rte_eth_devices'
> > is accessed directly.
> >
> > Do you have any idea/suggestion on how can we reduce the pain for them?
> 
> From what I see, ethdev iterators are probably something that is not
> known enough.
> rte_eth_dev_info_get() also fills some other spots.
> I don't have a magic answer, people need to look at existing API.
> 
> 
> But I just scratched the surface, looking at rte_eth_devices[] accesses.
> There might be other rte_eth_dev object dereferences (you get one
> point calling rte_eth_dev_info_get) that my grep did not catch.
> 

Indeed, that's a quite lot...
Thanks a lot for such detailed list, very interesting.
Wonder should we heads up coming changes to these guys...
Or might be interested persons are already aware (by reading dev@dpdk.org or so).

> Details:
> 
> >
> > > $ git grep-all -lw rte_eth_devices |grep -v \\.patch$
> > > ANS/ans/ans_main.c
> 
> I think this code is just lagging behind what ethdev currently
> provides/does wrt default offload config.
> This code probably does not need to dereference rte_eth_devices[] to
> query offloads configuration.
> 
> 
> > > BESS/core/drivers/pmd.cc
> 
> ethdev iterators can replace those accesses.
> 
> 
> > > dma_ip_drivers/QDMA/DPDK/drivers/net/qdma/qdma_xdebug.c
> > > dma_ip_drivers/QDMA/DPDK/drivers/net/qdma/rte_pmd_qdma.c
> > > dma_ip_drivers/QDMA/DPDK/examples/qdma_testapp/pcierw.c
> > > dma_ip_drivers/QDMA/DPDK/examples/qdma_testapp/testapp.c
> 
> This is a DPDK clone, with an additional driver, so irrelevant.
> 
> 
> > > FD.io-VPP/src/plugins/dpdk/device/format.c
> 
> rte_eth_rx_burst_mode_get() and rte_eth_tx_burst_mode_get() should do the job.
> I wonder if those APIs were introduced in DPDK for VPP.. ?
> 
> 
> > > lagopus/src/dataplane/dpdk/dpdk_io.c
> 
> Idem, ethdev iterators and rte_eth_dev_info_get() instead of direct
> access for dev_flags.
> 
> 
> > > OVS/lib/netdev-dpdk.c
> 
> For OVS, it was ethdev iterators + rte_eth_dev_info_get() where necessary.
> 
> 
> > > packet-journey/app/kni.c
> 
> There might be something missing in current ethdev API.
> This app wants to know if device is started... but on the other hand,
> that's probably something the app tracks itself.
> 
> 
> > > pktgen-dpdk/app/pktgen-port-cfg.c
> > > pktgen-dpdk/app/pktgen-port-cfg.h
> > > pktgen-dpdk/app/pktgen-stats.c
> 
> Accesses to offload configuration which I think are unneeded (like ANS).
> Direct access for dev_flags, can be replaced with rte_eth_dev_info_get.
> Direct access for name, can be replaced with rte_eth_dev_info_get.
> 
> 
> > > Trex/src/dpdk_funcs.c
> > > Trex/src/drivers/trex_i40e_fdir.c
> > > Trex/src/drivers/trex_ixgbe_fdir.c
> 
> Here, there is some horror.
> Directly casting and accessing hardware:
>     struct rte_eth_dev *dev = &rte_eth_devices[repid];
>         struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> 
>         I40E_WRITE_REG(hw, I40E_GLQF_ORT(12), 0x00000062);
>         I40E_WRITE_REG(hw, I40E_GLQF_PIT(2), 0x000024A0);
>         I40E_WRITE_REG(hw,
> I40E_PRTQF_FD_INSET(I40E_FILTER_PCTYPE_NONF_IPV4_UDP, 0), 0);
> etc...
> 
> This code probably bypasses too much of dpdk API, I stopped at this.
> 
> 
> > > TungstenFabric-vRouter/gdb/vr_dpdk.gdb
> 
> Mm, interesting, this part displays DPDK internals from gdb.
> That's something I have in my todolist for a long time, providing some
> common gdb scripts in DPDK...
> 
> 
> --
> David Marchand
Ananyev, Konstantin Oct. 5, 2021, 4:19 p.m. UTC | #6
> 04/10/2021 15:56, Konstantin Ananyev:
> > Move rte_eth_dev, rte_eth_dev_data, rte_eth_rxtx_callback and related
> > data into private header (ethdev_driver.h).
> [...]
> > +/**
> > + * @internal
> > + * Structure used to hold information about the callbacks to be called for a
> > + * queue on RX and TX.
> > + */
> > +struct rte_eth_rxtx_callback {
> > +	struct rte_eth_rxtx_callback *next;
> > +	union{
> > +		rte_rx_callback_fn rx;
> > +		rte_tx_callback_fn tx;
> > +	} fn;
> > +	void *param;
> > +};
> > +
> > +/**
> > + * @internal
> > + * The generic data structure associated with each ethernet device.
> > + *
> > + * Pointers to burst-oriented packet receive and transmit functions are
> > + * located at the beginning of the structure, along with the pointer to
> > + * where all the data elements for the particular device are stored in shared
> > + * memory. This split allows the function pointer and driver data to be per-
> > + * process, while the actual configuration data for the device is shared.
> > + */
> > +struct rte_eth_dev {
> > +	eth_rx_burst_t rx_pkt_burst; /**< Pointer to PMD receive function. */
> > +	eth_tx_burst_t tx_pkt_burst; /**< Pointer to PMD transmit function. */
> > +	eth_tx_prep_t tx_pkt_prepare;
> > +	/**< Pointer to 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. */
> 
> Why not using the new struct rte_eth_fp_ops?

We don't want to change each and every driver for this change.
The idea beyond it:
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.

> 
> > +
> > +	/**
> > +	 * Next two fields are per-device data but *data is shared between
> > +	 * primary and secondary processes and *process_private is per-process
> > +	 * private. The second one is managed by PMDs if necessary.
> > +	 */
> > +	struct rte_eth_dev_data *data;  /**< Pointer to device data. */
> 
> We should mention that "data" is shared between processes.

I think the comment above states exactly that.
In fact, it is just cut and paste from lib/ethdev/rte_ethdev_core.h to 
lib/ethdev/ethdev_driver.h.

> 
> > +	void *process_private; /**< Pointer to per-process device data. */
> > +	const struct eth_dev_ops *dev_ops; /**< Functions exported by PMD */
> > +	struct rte_device *device; /**< Backing device */
> > +	struct rte_intr_handle *intr_handle; /**< Device interrupt handle */
> > +	/** User application callbacks for NIC interrupts */
> > +	struct rte_eth_dev_cb_list link_intr_cbs;
> > +	/**
> > +	 * User-supplied functions called from rx_burst to post-process
> > +	 * received packets before passing them to the user
> > +	 */
> > +	struct rte_eth_rxtx_callback *post_rx_burst_cbs[RTE_MAX_QUEUES_PER_PORT];
> > +	/**
> > +	 * User-supplied functions called from tx_burst to pre-process
> > +	 * received packets before passing them to the driver for transmission.
> > +	 */
> > +	struct rte_eth_rxtx_callback *pre_tx_burst_cbs[RTE_MAX_QUEUES_PER_PORT];
> > +	enum rte_eth_dev_state state; /**< Flag indicating the port state */
> > +	void *security_ctx; /**< Context for security ops */
> > +
> > +	uint64_t reserved_64s[4]; /**< Reserved for future fields */
> > +	void *reserved_ptrs[4];   /**< Reserved for future fields */
> > +} __rte_cache_aligned;
> > +
> > +struct rte_eth_dev_sriov;
> > +struct rte_eth_dev_owner;
> > +
> > +/**
> > + * @internal
> > + * The data part, with no function pointers, associated with each ethernet
> > + * device. This structure is safe to place in shared memory to be common
> > + * among different processes in a multi-process configuration.
> > + */
> > +struct rte_eth_dev_data {
> > +	char name[RTE_ETH_NAME_MAX_LEN]; /**< Unique identifier name */
> > +
> > +	void **rx_queues; /**< Array of pointers to RX queues. */
> > +	void **tx_queues; /**< Array of pointers to TX queues. */
> > +	uint16_t nb_rx_queues; /**< Number of RX queues. */
> > +	uint16_t nb_tx_queues; /**< Number of TX queues. */
> > +
> > +	struct rte_eth_dev_sriov sriov;    /**< SRIOV data */
> > +
> > +	void *dev_private;
> > +			/**< PMD-specific private data.
> > +			 *   @see rte_eth_dev_release_port()
> > +			 */
> > +
> > +	struct rte_eth_link dev_link;   /**< Link-level information & status. */
> > +	struct rte_eth_conf dev_conf;   /**< Configuration applied to device. */
> > +	uint16_t mtu;                   /**< Maximum Transmission Unit. */
> > +	uint32_t min_rx_buf_size;
> > +			/**< Common RX buffer size handled by all queues. */
> > +
> > +	uint64_t rx_mbuf_alloc_failed; /**< RX ring mbuf allocation failures. */
> > +	struct rte_ether_addr *mac_addrs;
> > +			/**< Device Ethernet link address.
> > +			 *   @see rte_eth_dev_release_port()
> > +			 */
> > +	uint64_t mac_pool_sel[ETH_NUM_RECEIVE_MAC_ADDR];
> > +			/**< Bitmap associating MAC addresses to pools. */
> > +	struct rte_ether_addr *hash_mac_addrs;
> > +			/**< Device Ethernet MAC addresses of hash filtering.
> > +			 *   @see rte_eth_dev_release_port()
> > +			 */
> > +	uint16_t port_id;           /**< Device [external] port identifier. */
> > +
> > +	__extension__
> > +	uint8_t promiscuous   : 1,
> > +		/**< RX promiscuous mode ON(1) / OFF(0). */
> > +		scattered_rx : 1,
> > +		/**< RX of scattered packets is ON(1) / OFF(0) */
> > +		all_multicast : 1,
> > +		/**< RX all multicast mode ON(1) / OFF(0). */
> > +		dev_started : 1,
> > +		/**< Device state: STARTED(1) / STOPPED(0). */
> > +		lro         : 1,
> > +		/**< RX LRO is ON(1) / OFF(0) */
> > +		dev_configured : 1;
> > +		/**< Indicates whether the device is configured.
> > +		 *   CONFIGURED(1) / NOT CONFIGURED(0).
> > +		 */
> > +	uint8_t rx_queue_state[RTE_MAX_QUEUES_PER_PORT];
> > +		/**< Queues state: HAIRPIN(2) / STARTED(1) / STOPPED(0). */
> > +	uint8_t tx_queue_state[RTE_MAX_QUEUES_PER_PORT];
> > +		/**< Queues state: HAIRPIN(2) / STARTED(1) / STOPPED(0). */
> > +	uint32_t dev_flags;             /**< Capabilities. */
> > +	int numa_node;                  /**< NUMA node connection. */
> > +	struct rte_vlan_filter_conf vlan_filter_conf;
> > +			/**< VLAN filter configuration. */
> > +	struct rte_eth_dev_owner owner; /**< The port owner. */
> > +	uint16_t representor_id;
> > +			/**< Switch-specific identifier.
> > +			 *   Valid if RTE_ETH_DEV_REPRESENTOR in dev_flags.
> > +			 */
> > +
> > +	pthread_mutex_t flow_ops_mutex; /**< rte_flow ops mutex. */
> > +	uint64_t reserved_64s[4]; /**< Reserved for future fields */
> > +	void *reserved_ptrs[4];   /**< Reserved for future fields */
> > +} __rte_cache_aligned;
> > +
> > +/**
> > + * @internal
> > + * The pool of *rte_eth_dev* structures. The size of the pool
> > + * is configured at compile-time in the <rte_ethdev.c> file.
> > + */
> > +extern struct rte_eth_dev rte_eth_devices[];
> 
> Later we should add a function to configure the size of this array dynamically
> in the early DPDK init stage.

After we will hide rte_eth_devices[] and friends, we should be able to do
with them whatever we want.
But I suppose, that should be a subject of separate patch/discussion,
Probably not in 21.11 timeframe.
Thomas Monjalon Oct. 5, 2021, 4:25 p.m. UTC | #7
05/10/2021 18:19, Ananyev, Konstantin:
> > 04/10/2021 15:56, Konstantin Ananyev:
> > > Move rte_eth_dev, rte_eth_dev_data, rte_eth_rxtx_callback and related
> > > data into private header (ethdev_driver.h).
> > [...]
> > > +/**
> > > + * @internal
> > > + * Structure used to hold information about the callbacks to be called for a
> > > + * queue on RX and TX.
> > > + */
> > > +struct rte_eth_rxtx_callback {
> > > +	struct rte_eth_rxtx_callback *next;
> > > +	union{
> > > +		rte_rx_callback_fn rx;
> > > +		rte_tx_callback_fn tx;
> > > +	} fn;
> > > +	void *param;
> > > +};
> > > +
> > > +/**
> > > + * @internal
> > > + * The generic data structure associated with each ethernet device.
> > > + *
> > > + * Pointers to burst-oriented packet receive and transmit functions are
> > > + * located at the beginning of the structure, along with the pointer to
> > > + * where all the data elements for the particular device are stored in shared
> > > + * memory. This split allows the function pointer and driver data to be per-
> > > + * process, while the actual configuration data for the device is shared.
> > > + */
> > > +struct rte_eth_dev {
> > > +	eth_rx_burst_t rx_pkt_burst; /**< Pointer to PMD receive function. */
> > > +	eth_tx_burst_t tx_pkt_burst; /**< Pointer to PMD transmit function. */
> > > +	eth_tx_prep_t tx_pkt_prepare;
> > > +	/**< Pointer to 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. */
> > 
> > Why not using the new struct rte_eth_fp_ops?
> 
> We don't want to change each and every driver for this change.
> The idea beyond it:
> 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.

OK please add this explanation in the commit log.

> > > +
> > > +	/**
> > > +	 * Next two fields are per-device data but *data is shared between
> > > +	 * primary and secondary processes and *process_private is per-process
> > > +	 * private. The second one is managed by PMDs if necessary.
> > > +	 */
> > > +	struct rte_eth_dev_data *data;  /**< Pointer to device data. */
> > 
> > We should mention that "data" is shared between processes.
> 
> I think the comment above states exactly that.
> In fact, it is just cut and paste from lib/ethdev/rte_ethdev_core.h to 
> lib/ethdev/ethdev_driver.h.

True, but it is confusing, and we cannot have 2 comments for the same field.
The sentence "Next two fields are per-device data" is useless.
Let's comment each field separately.

> > > +	void *process_private; /**< Pointer to per-process device data. */
> > > +	const struct eth_dev_ops *dev_ops; /**< Functions exported by PMD */
> > > +	struct rte_device *device; /**< Backing device */
> > > +	struct rte_intr_handle *intr_handle; /**< Device interrupt handle */
> > > +	/** User application callbacks for NIC interrupts */
> > > +	struct rte_eth_dev_cb_list link_intr_cbs;
> > > +	/**
> > > +	 * User-supplied functions called from rx_burst to post-process
> > > +	 * received packets before passing them to the user
> > > +	 */
> > > +	struct rte_eth_rxtx_callback *post_rx_burst_cbs[RTE_MAX_QUEUES_PER_PORT];
> > > +	/**
> > > +	 * User-supplied functions called from tx_burst to pre-process
> > > +	 * received packets before passing them to the driver for transmission.
> > > +	 */
> > > +	struct rte_eth_rxtx_callback *pre_tx_burst_cbs[RTE_MAX_QUEUES_PER_PORT];
> > > +	enum rte_eth_dev_state state; /**< Flag indicating the port state */
> > > +	void *security_ctx; /**< Context for security ops */
> > > +
> > > +	uint64_t reserved_64s[4]; /**< Reserved for future fields */
> > > +	void *reserved_ptrs[4];   /**< Reserved for future fields */
> > > +} __rte_cache_aligned;
[...]
> > > +extern struct rte_eth_dev rte_eth_devices[];
> > 
> > Later we should add a function to configure the size of this array dynamically
> > in the early DPDK init stage.
> 
> After we will hide rte_eth_devices[] and friends, we should be able to do
> with them whatever we want.
> But I suppose, that should be a subject of separate patch/discussion,
> Probably not in 21.11 timeframe.  

Yes
diff mbox series

Patch

diff --git a/doc/guides/rel_notes/release_21_11.rst b/doc/guides/rel_notes/release_21_11.rst
index 6055551443..2944149943 100644
--- a/doc/guides/rel_notes/release_21_11.rst
+++ b/doc/guides/rel_notes/release_21_11.rst
@@ -228,6 +228,12 @@  ABI Changes
   to user, it still counts as an ABI change, as ``eth_rx_queue_count_t``
   is used by  public inline function ``rte_eth_rx_queue_count``.
 
+* ethdev: Made ``rte_eth_dev``, ``rte_eth_dev_data``, ``rte_eth_rxtx_callback``
+  private data structures. ``rte_eth_devices[]`` can't be accessible directly
+  by user any more. While it is an ABI breakage, this change is intended
+  to be transparent for both users (no changes in user app is required) and
+  PMD developers (no changes in PMD is required).
+
 
 Known Issues
 ------------
diff --git a/drivers/common/octeontx2/otx2_sec_idev.c b/drivers/common/octeontx2/otx2_sec_idev.c
index 6e9643c383..b561b67174 100644
--- a/drivers/common/octeontx2/otx2_sec_idev.c
+++ b/drivers/common/octeontx2/otx2_sec_idev.c
@@ -4,7 +4,7 @@ 
 
 #include <rte_atomic.h>
 #include <rte_bus_pci.h>
-#include <rte_ethdev.h>
+#include <ethdev_driver.h>
 #include <rte_spinlock.h>
 
 #include "otx2_common.h"
diff --git a/drivers/crypto/octeontx2/otx2_cryptodev_ops.c b/drivers/crypto/octeontx2/otx2_cryptodev_ops.c
index 37fad11d91..f0b72e05c2 100644
--- a/drivers/crypto/octeontx2/otx2_cryptodev_ops.c
+++ b/drivers/crypto/octeontx2/otx2_cryptodev_ops.c
@@ -6,7 +6,7 @@ 
 
 #include <cryptodev_pmd.h>
 #include <rte_errno.h>
-#include <rte_ethdev.h>
+#include <ethdev_driver.h>
 #include <rte_event_crypto_adapter.h>
 
 #include "otx2_cryptodev.h"
diff --git a/drivers/net/cxgbe/base/adapter.h b/drivers/net/cxgbe/base/adapter.h
index 01a2a9d147..1c7c8afe16 100644
--- a/drivers/net/cxgbe/base/adapter.h
+++ b/drivers/net/cxgbe/base/adapter.h
@@ -12,7 +12,7 @@ 
 #include <rte_mbuf.h>
 #include <rte_io.h>
 #include <rte_rwlock.h>
-#include <rte_ethdev.h>
+#include <ethdev_driver.h>
 
 #include "../cxgbe_compat.h"
 #include "../cxgbe_ofld.h"
diff --git a/drivers/net/dpaa2/dpaa2_ptp.c b/drivers/net/dpaa2/dpaa2_ptp.c
index 899dd5d442..8d79e39244 100644
--- a/drivers/net/dpaa2/dpaa2_ptp.c
+++ b/drivers/net/dpaa2/dpaa2_ptp.c
@@ -10,7 +10,7 @@ 
 #include <unistd.h>
 #include <stdarg.h>
 
-#include <rte_ethdev.h>
+#include <ethdev_driver.h>
 #include <rte_log.h>
 #include <rte_eth_ctrl.h>
 #include <rte_malloc.h>
diff --git a/drivers/net/netvsc/hn_var.h b/drivers/net/netvsc/hn_var.h
index 2a2bac9338..74e6e6010d 100644
--- a/drivers/net/netvsc/hn_var.h
+++ b/drivers/net/netvsc/hn_var.h
@@ -7,6 +7,7 @@ 
  */
 
 #include <rte_eal_paging.h>
+#include <ethdev_driver.h>
 
 /*
  * Tunable ethdev params
diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
index cc2c75261c..63b04dce32 100644
--- a/lib/ethdev/ethdev_driver.h
+++ b/lib/ethdev/ethdev_driver.h
@@ -17,6 +17,155 @@ 
 
 #include <rte_ethdev.h>
 
+/**
+ * @internal
+ * Structure used to hold information about the callbacks to be called for a
+ * queue on RX and TX.
+ */
+struct rte_eth_rxtx_callback {
+	struct rte_eth_rxtx_callback *next;
+	union{
+		rte_rx_callback_fn rx;
+		rte_tx_callback_fn tx;
+	} fn;
+	void *param;
+};
+
+/**
+ * @internal
+ * The generic data structure associated with each ethernet device.
+ *
+ * Pointers to burst-oriented packet receive and transmit functions are
+ * located at the beginning of the structure, along with the pointer to
+ * where all the data elements for the particular device are stored in shared
+ * memory. This split allows the function pointer and driver data to be per-
+ * process, while the actual configuration data for the device is shared.
+ */
+struct rte_eth_dev {
+	eth_rx_burst_t rx_pkt_burst; /**< Pointer to PMD receive function. */
+	eth_tx_burst_t tx_pkt_burst; /**< Pointer to PMD transmit function. */
+	eth_tx_prep_t tx_pkt_prepare;
+	/**< Pointer to 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. */
+
+	/**
+	 * Next two fields are per-device data but *data is shared between
+	 * primary and secondary processes and *process_private is per-process
+	 * private. The second one is managed by PMDs if necessary.
+	 */
+	struct rte_eth_dev_data *data;  /**< Pointer to device data. */
+	void *process_private; /**< Pointer to per-process device data. */
+	const struct eth_dev_ops *dev_ops; /**< Functions exported by PMD */
+	struct rte_device *device; /**< Backing device */
+	struct rte_intr_handle *intr_handle; /**< Device interrupt handle */
+	/** User application callbacks for NIC interrupts */
+	struct rte_eth_dev_cb_list link_intr_cbs;
+	/**
+	 * User-supplied functions called from rx_burst to post-process
+	 * received packets before passing them to the user
+	 */
+	struct rte_eth_rxtx_callback *post_rx_burst_cbs[RTE_MAX_QUEUES_PER_PORT];
+	/**
+	 * User-supplied functions called from tx_burst to pre-process
+	 * received packets before passing them to the driver for transmission.
+	 */
+	struct rte_eth_rxtx_callback *pre_tx_burst_cbs[RTE_MAX_QUEUES_PER_PORT];
+	enum rte_eth_dev_state state; /**< Flag indicating the port state */
+	void *security_ctx; /**< Context for security ops */
+
+	uint64_t reserved_64s[4]; /**< Reserved for future fields */
+	void *reserved_ptrs[4];   /**< Reserved for future fields */
+} __rte_cache_aligned;
+
+struct rte_eth_dev_sriov;
+struct rte_eth_dev_owner;
+
+/**
+ * @internal
+ * The data part, with no function pointers, associated with each ethernet
+ * device. This structure is safe to place in shared memory to be common
+ * among different processes in a multi-process configuration.
+ */
+struct rte_eth_dev_data {
+	char name[RTE_ETH_NAME_MAX_LEN]; /**< Unique identifier name */
+
+	void **rx_queues; /**< Array of pointers to RX queues. */
+	void **tx_queues; /**< Array of pointers to TX queues. */
+	uint16_t nb_rx_queues; /**< Number of RX queues. */
+	uint16_t nb_tx_queues; /**< Number of TX queues. */
+
+	struct rte_eth_dev_sriov sriov;    /**< SRIOV data */
+
+	void *dev_private;
+			/**< PMD-specific private data.
+			 *   @see rte_eth_dev_release_port()
+			 */
+
+	struct rte_eth_link dev_link;   /**< Link-level information & status. */
+	struct rte_eth_conf dev_conf;   /**< Configuration applied to device. */
+	uint16_t mtu;                   /**< Maximum Transmission Unit. */
+	uint32_t min_rx_buf_size;
+			/**< Common RX buffer size handled by all queues. */
+
+	uint64_t rx_mbuf_alloc_failed; /**< RX ring mbuf allocation failures. */
+	struct rte_ether_addr *mac_addrs;
+			/**< Device Ethernet link address.
+			 *   @see rte_eth_dev_release_port()
+			 */
+	uint64_t mac_pool_sel[ETH_NUM_RECEIVE_MAC_ADDR];
+			/**< Bitmap associating MAC addresses to pools. */
+	struct rte_ether_addr *hash_mac_addrs;
+			/**< Device Ethernet MAC addresses of hash filtering.
+			 *   @see rte_eth_dev_release_port()
+			 */
+	uint16_t port_id;           /**< Device [external] port identifier. */
+
+	__extension__
+	uint8_t promiscuous   : 1,
+		/**< RX promiscuous mode ON(1) / OFF(0). */
+		scattered_rx : 1,
+		/**< RX of scattered packets is ON(1) / OFF(0) */
+		all_multicast : 1,
+		/**< RX all multicast mode ON(1) / OFF(0). */
+		dev_started : 1,
+		/**< Device state: STARTED(1) / STOPPED(0). */
+		lro         : 1,
+		/**< RX LRO is ON(1) / OFF(0) */
+		dev_configured : 1;
+		/**< Indicates whether the device is configured.
+		 *   CONFIGURED(1) / NOT CONFIGURED(0).
+		 */
+	uint8_t rx_queue_state[RTE_MAX_QUEUES_PER_PORT];
+		/**< Queues state: HAIRPIN(2) / STARTED(1) / STOPPED(0). */
+	uint8_t tx_queue_state[RTE_MAX_QUEUES_PER_PORT];
+		/**< Queues state: HAIRPIN(2) / STARTED(1) / STOPPED(0). */
+	uint32_t dev_flags;             /**< Capabilities. */
+	int numa_node;                  /**< NUMA node connection. */
+	struct rte_vlan_filter_conf vlan_filter_conf;
+			/**< VLAN filter configuration. */
+	struct rte_eth_dev_owner owner; /**< The port owner. */
+	uint16_t representor_id;
+			/**< Switch-specific identifier.
+			 *   Valid if RTE_ETH_DEV_REPRESENTOR in dev_flags.
+			 */
+
+	pthread_mutex_t flow_ops_mutex; /**< rte_flow ops mutex. */
+	uint64_t reserved_64s[4]; /**< Reserved for future fields */
+	void *reserved_ptrs[4];   /**< Reserved for future fields */
+} __rte_cache_aligned;
+
+/**
+ * @internal
+ * The pool of *rte_eth_dev* structures. The size of the pool
+ * is configured at compile-time in the <rte_ethdev.c> file.
+ */
+extern struct rte_eth_dev rte_eth_devices[];
+
 /**< @internal Declaration of the hairpin peer queue information structure. */
 struct rte_hairpin_peer_info;
 
diff --git a/lib/ethdev/rte_ethdev_core.h b/lib/ethdev/rte_ethdev_core.h
index 63078e1ef4..2d07db0811 100644
--- a/lib/ethdev/rte_ethdev_core.h
+++ b/lib/ethdev/rte_ethdev_core.h
@@ -95,147 +95,4 @@  struct rte_eth_fp_ops {
 
 extern struct rte_eth_fp_ops rte_eth_fp_ops[RTE_MAX_ETHPORTS];
 
-
-/**
- * @internal
- * Structure used to hold information about the callbacks to be called for a
- * queue on RX and TX.
- */
-struct rte_eth_rxtx_callback {
-	struct rte_eth_rxtx_callback *next;
-	union{
-		rte_rx_callback_fn rx;
-		rte_tx_callback_fn tx;
-	} fn;
-	void *param;
-};
-
-/**
- * @internal
- * The generic data structure associated with each ethernet device.
- *
- * Pointers to burst-oriented packet receive and transmit functions are
- * located at the beginning of the structure, along with the pointer to
- * where all the data elements for the particular device are stored in shared
- * memory. This split allows the function pointer and driver data to be per-
- * process, while the actual configuration data for the device is shared.
- */
-struct rte_eth_dev {
-	eth_rx_burst_t rx_pkt_burst; /**< Pointer to PMD receive function. */
-	eth_tx_burst_t tx_pkt_burst; /**< Pointer to PMD transmit function. */
-	eth_tx_prep_t tx_pkt_prepare; /**< Pointer to 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. */
-
-	/**
-	 * Next two fields are per-device data but *data is shared between
-	 * primary and secondary processes and *process_private is per-process
-	 * private. The second one is managed by PMDs if necessary.
-	 */
-	struct rte_eth_dev_data *data;  /**< Pointer to device data. */
-	void *process_private; /**< Pointer to per-process device data. */
-	const struct eth_dev_ops *dev_ops; /**< Functions exported by PMD */
-	struct rte_device *device; /**< Backing device */
-	struct rte_intr_handle *intr_handle; /**< Device interrupt handle */
-	/** User application callbacks for NIC interrupts */
-	struct rte_eth_dev_cb_list link_intr_cbs;
-	/**
-	 * User-supplied functions called from rx_burst to post-process
-	 * received packets before passing them to the user
-	 */
-	struct rte_eth_rxtx_callback *post_rx_burst_cbs[RTE_MAX_QUEUES_PER_PORT];
-	/**
-	 * User-supplied functions called from tx_burst to pre-process
-	 * received packets before passing them to the driver for transmission.
-	 */
-	struct rte_eth_rxtx_callback *pre_tx_burst_cbs[RTE_MAX_QUEUES_PER_PORT];
-	enum rte_eth_dev_state state; /**< Flag indicating the port state */
-	void *security_ctx; /**< Context for security ops */
-
-	uint64_t reserved_64s[4]; /**< Reserved for future fields */
-	void *reserved_ptrs[4];   /**< Reserved for future fields */
-} __rte_cache_aligned;
-
-struct rte_eth_dev_sriov;
-struct rte_eth_dev_owner;
-
-/**
- * @internal
- * The data part, with no function pointers, associated with each ethernet device.
- *
- * This structure is safe to place in shared memory to be common among different
- * processes in a multi-process configuration.
- */
-struct rte_eth_dev_data {
-	char name[RTE_ETH_NAME_MAX_LEN]; /**< Unique identifier name */
-
-	void **rx_queues; /**< Array of pointers to RX queues. */
-	void **tx_queues; /**< Array of pointers to TX queues. */
-	uint16_t nb_rx_queues; /**< Number of RX queues. */
-	uint16_t nb_tx_queues; /**< Number of TX queues. */
-
-	struct rte_eth_dev_sriov sriov;    /**< SRIOV data */
-
-	void *dev_private;
-			/**< PMD-specific private data.
-			 *   @see rte_eth_dev_release_port()
-			 */
-
-	struct rte_eth_link dev_link;   /**< Link-level information & status. */
-	struct rte_eth_conf dev_conf;   /**< Configuration applied to device. */
-	uint16_t mtu;                   /**< Maximum Transmission Unit. */
-	uint32_t min_rx_buf_size;
-			/**< Common RX buffer size handled by all queues. */
-
-	uint64_t rx_mbuf_alloc_failed; /**< RX ring mbuf allocation failures. */
-	struct rte_ether_addr *mac_addrs;
-			/**< Device Ethernet link address.
-			 *   @see rte_eth_dev_release_port()
-			 */
-	uint64_t mac_pool_sel[ETH_NUM_RECEIVE_MAC_ADDR];
-			/**< Bitmap associating MAC addresses to pools. */
-	struct rte_ether_addr *hash_mac_addrs;
-			/**< Device Ethernet MAC addresses of hash filtering.
-			 *   @see rte_eth_dev_release_port()
-			 */
-	uint16_t port_id;           /**< Device [external] port identifier. */
-
-	__extension__
-	uint8_t promiscuous   : 1, /**< RX promiscuous mode ON(1) / OFF(0). */
-		scattered_rx : 1,  /**< RX of scattered packets is ON(1) / OFF(0) */
-		all_multicast : 1, /**< RX all multicast mode ON(1) / OFF(0). */
-		dev_started : 1,   /**< Device state: STARTED(1) / STOPPED(0). */
-		lro         : 1,   /**< RX LRO is ON(1) / OFF(0) */
-		dev_configured : 1;
-		/**< Indicates whether the device is configured.
-		 *   CONFIGURED(1) / NOT CONFIGURED(0).
-		 */
-	uint8_t rx_queue_state[RTE_MAX_QUEUES_PER_PORT];
-		/**< Queues state: HAIRPIN(2) / STARTED(1) / STOPPED(0). */
-	uint8_t tx_queue_state[RTE_MAX_QUEUES_PER_PORT];
-		/**< Queues state: HAIRPIN(2) / STARTED(1) / STOPPED(0). */
-	uint32_t dev_flags;             /**< Capabilities. */
-	int numa_node;                  /**< NUMA node connection. */
-	struct rte_vlan_filter_conf vlan_filter_conf;
-			/**< VLAN filter configuration. */
-	struct rte_eth_dev_owner owner; /**< The port owner. */
-	uint16_t representor_id;
-			/**< Switch-specific identifier.
-			 *   Valid if RTE_ETH_DEV_REPRESENTOR in dev_flags.
-			 */
-
-	pthread_mutex_t flow_ops_mutex; /**< rte_flow ops mutex. */
-	uint64_t reserved_64s[4]; /**< Reserved for future fields */
-	void *reserved_ptrs[4];   /**< Reserved for future fields */
-} __rte_cache_aligned;
-
-/**
- * @internal
- * The pool of *rte_eth_dev* structures. The size of the pool
- * is configured at compile-time in the <rte_ethdev.c> file.
- */
-extern struct rte_eth_dev rte_eth_devices[];
-
 #endif /* _RTE_ETHDEV_CORE_H_ */
diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
index 0881202381..3dc494a016 100644
--- a/lib/ethdev/version.map
+++ b/lib/ethdev/version.map
@@ -75,7 +75,6 @@  DPDK_22 {
 	rte_eth_dev_udp_tunnel_port_add;
 	rte_eth_dev_udp_tunnel_port_delete;
 	rte_eth_dev_vlan_filter;
-	rte_eth_devices;
 	rte_eth_find_next;
 	rte_eth_find_next_of;
 	rte_eth_find_next_owned_by;
@@ -272,6 +271,7 @@  INTERNAL {
 	rte_eth_dev_release_port;
 	rte_eth_dev_internal_reset;
 	rte_eth_devargs_parse;
+	rte_eth_devices;
 	rte_eth_dma_zone_free;
 	rte_eth_dma_zone_reserve;
 	rte_eth_hairpin_queue_peer_bind;
diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c b/lib/eventdev/rte_event_eth_rx_adapter.c
index 13dfb28401..89c4ca5d40 100644
--- a/lib/eventdev/rte_event_eth_rx_adapter.c
+++ b/lib/eventdev/rte_event_eth_rx_adapter.c
@@ -11,7 +11,7 @@ 
 #include <rte_common.h>
 #include <rte_dev.h>
 #include <rte_errno.h>
-#include <rte_ethdev.h>
+#include <ethdev_driver.h>
 #include <rte_log.h>
 #include <rte_malloc.h>
 #include <rte_service_component.h>
diff --git a/lib/eventdev/rte_event_eth_tx_adapter.c b/lib/eventdev/rte_event_eth_tx_adapter.c
index 18c0359db7..1c06c8707c 100644
--- a/lib/eventdev/rte_event_eth_tx_adapter.c
+++ b/lib/eventdev/rte_event_eth_tx_adapter.c
@@ -3,7 +3,7 @@ 
  */
 #include <rte_spinlock.h>
 #include <rte_service_component.h>
-#include <rte_ethdev.h>
+#include <ethdev_driver.h>
 
 #include "eventdev_pmd.h"
 #include "rte_eventdev_trace.h"
diff --git a/lib/eventdev/rte_eventdev.c b/lib/eventdev/rte_eventdev.c
index e347d6dfd5..ebef5f0906 100644
--- a/lib/eventdev/rte_eventdev.c
+++ b/lib/eventdev/rte_eventdev.c
@@ -29,7 +29,7 @@ 
 #include <rte_common.h>
 #include <rte_malloc.h>
 #include <rte_errno.h>
-#include <rte_ethdev.h>
+#include <ethdev_driver.h>
 #include <rte_cryptodev.h>
 #include <cryptodev_pmd.h>
 #include <rte_telemetry.h>
diff --git a/lib/metrics/rte_metrics_telemetry.c b/lib/metrics/rte_metrics_telemetry.c
index 269f8ef613..5be21b2e86 100644
--- a/lib/metrics/rte_metrics_telemetry.c
+++ b/lib/metrics/rte_metrics_telemetry.c
@@ -2,7 +2,7 @@ 
  * Copyright(c) 2020 Intel Corporation
  */
 
-#include <rte_ethdev.h>
+#include <ethdev_driver.h>
 #include <rte_string_fns.h>
 #ifdef RTE_LIB_TELEMETRY
 #include <telemetry_internal.h>