[v3,4/7] ethdev: make burst functions to use new flat array
Checks
Commit Message
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.ananyev@intel.com>
---
lib/ethdev/ethdev_private.c | 31 +++++
lib/ethdev/rte_ethdev.h | 244 ++++++++++++++++++++++++++----------
lib/ethdev/version.map | 5 +
3 files changed, 212 insertions(+), 68 deletions(-)
Comments
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.ananyev@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?
<...>
> +++ 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?
> };
>
> 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;
>
> 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.ananyev@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;
> >
On 10/1/2021 6:40 PM, Ananyev, Konstantin wrote:
>
>
>> 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.ananyev@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?
>
Make them public API. (Basically just remove '__' prefix and @internal comment).
This way application can use them to run custom callback(s) (not only the
registered ones), not sure if this can be dangerous though.
We need to trace the ABI for these functions, making them public clarifies it.
Also comment can be updated to describe intended usage instead of marking them
internal, and applications can use these anyway if we mark them internal or not.
>>> };
>>>
>>> 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;
>>>
>
> >>
> >>> 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?
> >
>
> Make them public API. (Basically just remove '__' prefix and @internal comment).
>
> This way application can use them to run custom callback(s) (not only the
> registered ones), not sure if this can be dangerous though.
Hmm, as I said above, I don't want users to call them explicitly.
Do you have any good reason to allow it?
>
> We need to trace the ABI for these functions, making them public clarifies it.
We do have plenty of semi-internal functions right now,
why adding that one will be a problem?
From other side - if we'll declare it public, we will have obligations to support it
in future releases, plus it might encourage users to use it on its own.
To me that sounds like extra headache without any gain in return.
> Also comment can be updated to describe intended usage instead of marking them
> internal, and applications can use these anyway if we mark them internal or not.
On 10/4/2021 10:20 AM, Ananyev, Konstantin wrote:
>
>>>>
>>>>> 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?
>>>
>>
>> Make them public API. (Basically just remove '__' prefix and @internal comment).
>>
>> This way application can use them to run custom callback(s) (not only the
>> registered ones), not sure if this can be dangerous though.
>
> Hmm, as I said above, I don't want users to call them explicitly.
> Do you have any good reason to allow it?
>
Just to get rid of this internal APIs that is exposed to application state.
>>
>> We need to trace the ABI for these functions, making them public clarifies it.
>
> We do have plenty of semi-internal functions right now,
> why adding that one will be a problem?
As far as I remember existing ones are 'static inline' functions, and we don't
have an ABI concern with them. But these are actual functions called by application.
> From other side - if we'll declare it public, we will have obligations to support it
> in future releases, plus it might encourage users to use it on its own.
> To me that sounds like extra headache without any gain in return.
>
If having those two as public API doesn't make sense, I agree with you.
>> Also comment can be updated to describe intended usage instead of marking them
>> internal, and applications can use these anyway if we mark them internal or not.
>
>
> On 10/4/2021 10:20 AM, Ananyev, Konstantin wrote:
> >
> >>>>
> >>>>> 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?
> >>>
> >>
> >> Make them public API. (Basically just remove '__' prefix and @internal comment).
> >>
> >> This way application can use them to run custom callback(s) (not only the
> >> registered ones), not sure if this can be dangerous though.
> >
> > Hmm, as I said above, I don't want users to call them explicitly.
> > Do you have any good reason to allow it?
> >
>
> Just to get rid of this internal APIs that is exposed to application state.
>
> >>
> >> We need to trace the ABI for these functions, making them public clarifies it.
> >
> > We do have plenty of semi-internal functions right now,
> > why adding that one will be a problem?
>
> As far as I remember existing ones are 'static inline' functions, and we don't
> have an ABI concern with them. But these are actual functions called by application.
Not always.
As an example of internal but not static ones:
rte_mempool_check_cookies
rte_mempool_contig_blocks_check_cookies
rte_mempool_op_calc_mem_size_helper
_rte_pktmbuf_read
>
> > From other side - if we'll declare it public, we will have obligations to support it
> > in future releases, plus it might encourage users to use it on its own.
> > To me that sounds like extra headache without any gain in return.
> >
>
> If having those two as public API doesn't make sense, I agree with you.
>
> >> Also comment can be updated to describe intended usage instead of marking them
> >> internal, and applications can use these anyway if we mark them internal or not.
> >
@@ -226,3 +226,34 @@ eth_dev_fp_ops_setup(struct rte_eth_fp_ops *fpo,
fpo->txq.data = dev->data->tx_queues;
fpo->txq.clbk = (void **)(uintptr_t)dev->pre_tx_burst_cbs;
}
+
+uint16_t
+__rte_eth_rx_epilog(uint16_t port_id, uint16_t queue_id,
+ struct rte_mbuf **rx_pkts, uint16_t nb_rx, uint16_t nb_pkts,
+ void *opaque)
+{
+ const struct rte_eth_rxtx_callback *cb = opaque;
+
+ while (cb != NULL) {
+ nb_rx = cb->fn.rx(port_id, queue_id, rx_pkts, nb_rx,
+ nb_pkts, cb->param);
+ cb = cb->next;
+ }
+
+ return nb_rx;
+}
+
+uint16_t
+__rte_eth_tx_prolog(uint16_t port_id, uint16_t queue_id,
+ struct rte_mbuf **tx_pkts, uint16_t nb_pkts, void *opaque)
+{
+ const struct rte_eth_rxtx_callback *cb = opaque;
+
+ while (cb != NULL) {
+ nb_pkts = cb->fn.tx(port_id, queue_id, tx_pkts, nb_pkts,
+ cb->param);
+ cb = cb->next;
+ }
+
+ return nb_pkts;
+}
@@ -4904,6 +4904,34 @@ int rte_eth_representor_info_get(uint16_t port_id,
#include <rte_ethdev_core.h>
+/**
+ * @internal
+ * Helper routine for eth driver rx_burst API.
+ * Should be called at exit from PMD's rte_eth_rx_bulk implementation.
+ * Does necessary post-processing - invokes RX callbacks if any, etc.
+ *
+ * @param port_id
+ * The port identifier of the Ethernet device.
+ * @param queue_id
+ * The index of the receive queue from which to retrieve input packets.
+ * @param rx_pkts
+ * The address of an array of pointers to *rte_mbuf* structures that
+ * have been retrieved from the device.
+ * @param nb_pkts
+ * The number of packets that were retrieved from the device.
+ * @param nb_pkts
+ * The number of elements in *rx_pkts* array.
+ * @param opaque
+ * Opaque pointer of RX queue callback related data.
+ *
+ * @return
+ * The number of packets effectively supplied to the *rx_pkts* array.
+ */
+__rte_experimental
+uint16_t __rte_eth_rx_epilog(uint16_t port_id, uint16_t queue_id,
+ struct rte_mbuf **rx_pkts, uint16_t nb_rx, uint16_t nb_pkts,
+ void *opaque);
+
/**
*
* Retrieve a burst of input packets from a receive queue of an Ethernet
@@ -4995,23 +5023,37 @@ static inline uint16_t
rte_eth_rx_burst(uint16_t port_id, uint16_t queue_id,
struct rte_mbuf **rx_pkts, const uint16_t nb_pkts)
{
- struct rte_eth_dev *dev = &rte_eth_devices[port_id];
uint16_t nb_rx;
+ struct rte_eth_fp_ops *p;
+ void *cb, *qd;
+
+#ifdef RTE_ETHDEV_DEBUG_RX
+ 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 0;
+ }
+#endif
+
+ /* fetch pointer to queue data */
+ p = &rte_eth_fp_ops[port_id];
+ qd = p->rxq.data[queue_id];
#ifdef RTE_ETHDEV_DEBUG_RX
RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, 0);
- RTE_FUNC_PTR_OR_ERR_RET(*dev->rx_pkt_burst, 0);
- if (queue_id >= dev->data->nb_rx_queues) {
- RTE_ETHDEV_LOG(ERR, "Invalid RX queue_id=%u\n", queue_id);
+ if (qd == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Invalid RX queue_id=%u for port_id=%u\n",
+ queue_id, port_id);
return 0;
}
#endif
- nb_rx = (*dev->rx_pkt_burst)(dev->data->rx_queues[queue_id],
- rx_pkts, nb_pkts);
+
+ nb_rx = p->rx_pkt_burst(qd, rx_pkts, nb_pkts);
#ifdef RTE_ETHDEV_RXTX_CALLBACKS
- struct rte_eth_rxtx_callback *cb;
/* __ATOMIC_RELEASE memory order was used when the
* call back was inserted into the list.
@@ -5019,16 +5061,10 @@ rte_eth_rx_burst(uint16_t port_id, uint16_t queue_id,
* cb and cb->fn/cb->next, __ATOMIC_ACQUIRE memory order is
* not required.
*/
- cb = __atomic_load_n(&dev->post_rx_burst_cbs[queue_id],
- __ATOMIC_RELAXED);
-
- if (unlikely(cb != NULL)) {
- do {
- nb_rx = cb->fn.rx(port_id, queue_id, rx_pkts, nb_rx,
- nb_pkts, cb->param);
- cb = cb->next;
- } while (cb != NULL);
- }
+ cb = __atomic_load_n((void **)&p->rxq.clbk[queue_id], __ATOMIC_RELAXED);
+ if (unlikely(cb != NULL))
+ nb_rx = __rte_eth_rx_epilog(port_id, queue_id, rx_pkts, nb_rx,
+ nb_pkts, cb);
#endif
rte_ethdev_trace_rx_burst(port_id, queue_id, (void **)rx_pkts, nb_rx);
@@ -5051,16 +5087,27 @@ rte_eth_rx_burst(uint16_t port_id, uint16_t queue_id,
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;
+ }
+
+ /* fetch pointer to queue data */
+ p = &rte_eth_fp_ops[port_id];
+ qd = p->rxq.data[queue_id];
RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
- dev = &rte_eth_devices[port_id];
- RTE_FUNC_PTR_OR_ERR_RET(*dev->rx_queue_count, -ENOTSUP);
- if (queue_id >= dev->data->nb_rx_queues ||
- dev->data->rx_queues[queue_id] == NULL)
+ RTE_FUNC_PTR_OR_ERR_RET(*p->rx_queue_count, -ENOTSUP);
+ if (qd == NULL)
return -EINVAL;
- return (int)(*dev->rx_queue_count)(dev->data->rx_queues[queue_id]);
+ return (int)(*p->rx_queue_count)(qd);
}
/**
@@ -5133,21 +5180,30 @@ static inline int
rte_eth_rx_descriptor_status(uint16_t port_id, uint16_t queue_id,
uint16_t offset)
{
- struct rte_eth_dev *dev;
- void *rxq;
+ struct rte_eth_fp_ops *p;
+ void *qd;
#ifdef RTE_ETHDEV_DEBUG_RX
- RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+ 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;
+ }
#endif
- dev = &rte_eth_devices[port_id];
+
+ /* fetch pointer to queue data */
+ p = &rte_eth_fp_ops[port_id];
+ qd = p->rxq.data[queue_id];
+
#ifdef RTE_ETHDEV_DEBUG_RX
- if (queue_id >= dev->data->nb_rx_queues)
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+ if (qd == NULL)
return -ENODEV;
#endif
- RTE_FUNC_PTR_OR_ERR_RET(*dev->rx_descriptor_status, -ENOTSUP);
- rxq = dev->data->rx_queues[queue_id];
-
- return (*dev->rx_descriptor_status)(rxq, offset);
+ RTE_FUNC_PTR_OR_ERR_RET(*p->rx_descriptor_status, -ENOTSUP);
+ return (*p->rx_descriptor_status)(qd, offset);
}
/**@{@name Tx hardware descriptor states
@@ -5194,23 +5250,55 @@ rte_eth_rx_descriptor_status(uint16_t port_id, uint16_t queue_id,
static inline int rte_eth_tx_descriptor_status(uint16_t port_id,
uint16_t queue_id, uint16_t offset)
{
- struct rte_eth_dev *dev;
- void *txq;
+ struct rte_eth_fp_ops *p;
+ void *qd;
#ifdef RTE_ETHDEV_DEBUG_TX
- RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+ 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;
+ }
#endif
- dev = &rte_eth_devices[port_id];
+
+ /* fetch pointer to queue data */
+ p = &rte_eth_fp_ops[port_id];
+ qd = p->txq.data[queue_id];
+
#ifdef RTE_ETHDEV_DEBUG_TX
- if (queue_id >= dev->data->nb_tx_queues)
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+ if (qd == NULL)
return -ENODEV;
#endif
- RTE_FUNC_PTR_OR_ERR_RET(*dev->tx_descriptor_status, -ENOTSUP);
- txq = dev->data->tx_queues[queue_id];
-
- return (*dev->tx_descriptor_status)(txq, offset);
+ RTE_FUNC_PTR_OR_ERR_RET(*p->tx_descriptor_status, -ENOTSUP);
+ return (*p->tx_descriptor_status)(qd, offset);
}
+/**
+ * @internal
+ * Helper routine for eth driver tx_burst API.
+ * Should be called before entry PMD's rte_eth_tx_bulk implementation.
+ * Does necessary pre-processing - invokes TX callbacks if any, etc.
+ *
+ * @param port_id
+ * The port identifier of the Ethernet device.
+ * @param queue_id
+ * The index of the transmit queue through which output packets must be
+ * sent.
+ * @param tx_pkts
+ * The address of an array of *nb_pkts* pointers to *rte_mbuf* structures
+ * which contain the output packets.
+ * @param nb_pkts
+ * The maximum number of packets to transmit.
+ * @return
+ * The number of output packets to transmit.
+ */
+__rte_experimental
+uint16_t __rte_eth_tx_prolog(uint16_t port_id, uint16_t queue_id,
+ struct rte_mbuf **tx_pkts, uint16_t nb_pkts, void *opaque);
+
/**
* Send a burst of output packets on a transmit queue of an Ethernet device.
*
@@ -5281,20 +5369,34 @@ static inline uint16_t
rte_eth_tx_burst(uint16_t port_id, uint16_t queue_id,
struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
{
- struct rte_eth_dev *dev = &rte_eth_devices[port_id];
+ struct rte_eth_fp_ops *p;
+ void *cb, *qd;
+
+#ifdef RTE_ETHDEV_DEBUG_TX
+ 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 0;
+ }
+#endif
+
+ /* fetch pointer to queue data */
+ p = &rte_eth_fp_ops[port_id];
+ qd = p->txq.data[queue_id];
#ifdef RTE_ETHDEV_DEBUG_TX
RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, 0);
- RTE_FUNC_PTR_OR_ERR_RET(*dev->tx_pkt_burst, 0);
- if (queue_id >= dev->data->nb_tx_queues) {
- RTE_ETHDEV_LOG(ERR, "Invalid TX queue_id=%u\n", queue_id);
+ if (qd == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Invalid TX queue_id=%u for port_id=%u\n",
+ queue_id, port_id);
return 0;
}
#endif
#ifdef RTE_ETHDEV_RXTX_CALLBACKS
- struct rte_eth_rxtx_callback *cb;
/* __ATOMIC_RELEASE memory order was used when the
* call back was inserted into the list.
@@ -5302,21 +5404,16 @@ rte_eth_tx_burst(uint16_t port_id, uint16_t queue_id,
* cb and cb->fn/cb->next, __ATOMIC_ACQUIRE memory order is
* not required.
*/
- cb = __atomic_load_n(&dev->pre_tx_burst_cbs[queue_id],
- __ATOMIC_RELAXED);
-
- if (unlikely(cb != NULL)) {
- do {
- nb_pkts = cb->fn.tx(port_id, queue_id, tx_pkts, nb_pkts,
- cb->param);
- cb = cb->next;
- } while (cb != NULL);
- }
+ cb = __atomic_load_n((void **)&p->txq.clbk[queue_id], __ATOMIC_RELAXED);
+ if (unlikely(cb != NULL))
+ nb_pkts = __rte_eth_tx_prolog(port_id, queue_id, tx_pkts,
+ nb_pkts, cb);
#endif
- rte_ethdev_trace_tx_burst(port_id, queue_id, (void **)tx_pkts,
- nb_pkts);
- return (*dev->tx_pkt_burst)(dev->data->tx_queues[queue_id], tx_pkts, nb_pkts);
+ nb_pkts = p->tx_pkt_burst(qd, tx_pkts, nb_pkts);
+
+ rte_ethdev_trace_tx_burst(port_id, queue_id, (void **)tx_pkts, nb_pkts);
+ return nb_pkts;
}
/**
@@ -5379,31 +5476,42 @@ static inline uint16_t
rte_eth_tx_prepare(uint16_t port_id, uint16_t queue_id,
struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
{
- struct rte_eth_dev *dev;
+ struct rte_eth_fp_ops *p;
+ void *qd;
#ifdef RTE_ETHDEV_DEBUG_TX
- if (!rte_eth_dev_is_valid_port(port_id)) {
- RTE_ETHDEV_LOG(ERR, "Invalid TX port_id=%u\n", port_id);
+ 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);
rte_errno = ENODEV;
return 0;
}
#endif
- dev = &rte_eth_devices[port_id];
+ /* fetch pointer to queue data */
+ p = &rte_eth_fp_ops[port_id];
+ qd = p->txq.data[queue_id];
#ifdef RTE_ETHDEV_DEBUG_TX
- if (queue_id >= dev->data->nb_tx_queues) {
- RTE_ETHDEV_LOG(ERR, "Invalid TX queue_id=%u\n", queue_id);
+ if (!rte_eth_dev_is_valid_port(port_id)) {
+ RTE_ETHDEV_LOG(ERR, "Invalid TX port_id=%u\n", port_id);
+ rte_errno = ENODEV;
+ return 0;
+ }
+ if (qd == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Invalid TX queue_id=%u for port_id=%u\n",
+ queue_id, port_id);
rte_errno = EINVAL;
return 0;
}
#endif
- if (!dev->tx_pkt_prepare)
+ if (!p->tx_pkt_prepare)
return nb_pkts;
- return (*dev->tx_pkt_prepare)(dev->data->tx_queues[queue_id],
- tx_pkts, nb_pkts);
+ return p->tx_pkt_prepare(qd, tx_pkts, nb_pkts);
}
#else
@@ -247,11 +247,16 @@ EXPERIMENTAL {
rte_mtr_meter_policy_delete;
rte_mtr_meter_policy_update;
rte_mtr_meter_policy_validate;
+
+ # added in 21.05
+ __rte_eth_rx_epilog;
+ __rte_eth_tx_prolog;
};
INTERNAL {
global:
+ rte_eth_fp_ops;
rte_eth_dev_allocate;
rte_eth_dev_allocated;
rte_eth_dev_attach_secondary;