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 <konstantin.anan...@intel.com>
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
