> On 10/1/2021 3:02 PM, Konstantin Ananyev wrote:
> > Rework 'fast' burst functions to use rte_eth_fp_ops[].
> > While it is an API/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).
> > One extra thing to note - RX/TX callback invocation will cause extra
> > function call with these changes. That might cause some insignificant
> > slowdown for code-path where RX/TX callbacks are heavily involved.
> >
> > Signed-off-by: Konstantin Ananyev <konstantin.anan...@intel.com>
>
> <...>
>
> > static inline int
> > rte_eth_rx_queue_count(uint16_t port_id, uint16_t queue_id)
> > {
> > - struct rte_eth_dev *dev;
> > + struct rte_eth_fp_ops *p;
> > + void *qd;
> > +
> > + if (port_id >= RTE_MAX_ETHPORTS ||
> > + queue_id >= RTE_MAX_QUEUES_PER_PORT) {
> > + RTE_ETHDEV_LOG(ERR,
> > + "Invalid port_id=%u or queue_id=%u\n",
> > + port_id, queue_id);
> > + return -EINVAL;
> > + }
>
> Should the checkes wrapped with '#ifdef RTE_ETHDEV_DEBUG_RX' like others?
Original rte_eth_rx_queue_count() always have similar checks enabled,
that's why I also kept them 'always on'.
>
> <...>
>
> > +++ b/lib/ethdev/version.map
> > @@ -247,11 +247,16 @@ EXPERIMENTAL {
> > rte_mtr_meter_policy_delete;
> > rte_mtr_meter_policy_update;
> > rte_mtr_meter_policy_validate;
> > +
> > + # added in 21.05
>
> s/21.05/21.11/
>
> > + __rte_eth_rx_epilog;
> > + __rte_eth_tx_prolog;
>
> These are directly called by application and must be part of ABI, but marked
> as
> 'internal' and has '__rte' prefix to highligh it, this may be confusing.
> What about making them proper, non-internal, API?
Hmm not sure what do you suggest here.
We don't want users to call them explicitly.
They are sort of helpers for rte_eth_rx_burst/rte_eth_tx_burst.
So I did what I thought is our usual policy for such semi-internal thigns:
have '@intenal' in comments, but in version.map put them under
EXPERIMETAL/global
section.
What do you think it should be instead?
> > };
> >
> > INTERNAL {
> > global:
> >
> > + rte_eth_fp_ops;
>
> This variable is accessed in inline function, so accessed by application, not
> sure if it suits the 'internal' object definition, internal should be only for
> objects accessed by other parts of DPDK.
> I think this can be added to 'DPDK_22'.
>
> > rte_eth_dev_allocate;
> > rte_eth_dev_allocated;
> > rte_eth_dev_attach_secondary;
> >
