> 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?
...
>
> > + 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?
