On Mon, Oct 26, 2020 at 11:20:04PM +0100, Thomas Monjalon wrote:
> The test worker_loopback used the deprecated mbuf field udata64.
> It is moved to a dynamic field in order to allow removal of udata64.
>
> Signed-off-by: Thomas Monjalon <tho...@monjalon.net>
> Reviewed-by: Andrew Rybchenko <andrew.rybche...@oktetlabs.ru>
> ---
> drivers/event/sw/sw_evdev_selftest.c | 23 ++++++++++++++++++++---
> 1 file changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/event/sw/sw_evdev_selftest.c
> b/drivers/event/sw/sw_evdev_selftest.c
> index 5c7e527917..9af20cecf1 100644
> --- a/drivers/event/sw/sw_evdev_selftest.c
> +++ b/drivers/event/sw/sw_evdev_selftest.c
> @@ -40,6 +40,11 @@ struct test {
> uint32_t service_id;
> };
>
> +static int counter_dynfield_offset;
In general, I wonder if we shouldn't initialize offset to -1.
> +#define COUNTER_FIELD_TYPE uint8_t
Another general comment, I suggest to use a typedef instead of
a define when relevant.
> +#define COUNTER_FIELD(mbuf) (*RTE_MBUF_DYNFIELD(mbuf, \
> + counter_dynfield_offset, COUNTER_FIELD_TYPE *))
> +
I'm not sure this comment applies here, but since it's a simple example,
it's a good place for another general comment. The RTE_MBUF_DYNFIELD()
macro is convenient because it can be used to set or get a value of any
type, but in my opinion it is not always easy to read:
RTE_MBUF_DYNFIELD(m, off, type) = value;
In some situations, having wrappers may make the code more readable:
static inline void mbuf_set_counter(struct rte_mbuf *m, counter_field_t
counter);
static inline counter_field_t mbuf_get_counter(struct rte_mbuf *m);
static inline void mbuf_inc_counter(struct rte_mbuf *m);
> static struct rte_event release_ev;
>
> static inline struct rte_mbuf *
> @@ -2987,8 +2992,8 @@ worker_loopback_worker_fn(void *arg)
> }
>
> ev[i].queue_id = 0;
> - ev[i].mbuf->udata64++;
> - if (ev[i].mbuf->udata64 != 16) {
> + COUNTER_FIELD(ev[i].mbuf)++;
> + if (COUNTER_FIELD(ev[i].mbuf) != 16) {
> ev[i].op = RTE_EVENT_OP_FORWARD;
> enqd = rte_event_enqueue_burst(evdev, port,
> &ev[i], 1);
> @@ -3028,7 +3033,7 @@ worker_loopback_producer_fn(void *arg)
> m = rte_pktmbuf_alloc(t->mbuf_pool);
> } while (m == NULL);
>
> - m->udata64 = 0;
> + COUNTER_FIELD(m) = 0;
>
> struct rte_event ev = {
> .op = RTE_EVENT_OP_NEW,
> @@ -3061,6 +3066,18 @@ worker_loopback(struct test *t, uint8_t
> disable_implicit_release)
> int err;
> int w_lcore, p_lcore;
>
> + static const struct rte_mbuf_dynfield counter_dynfield_desc = {
> + .name = "rte_event_sw_dynfield_selftest_counter",
> + .size = sizeof(COUNTER_FIELD_TYPE),
> + .align = __alignof__(COUNTER_FIELD_TYPE),
> + };
> + counter_dynfield_offset =
> + rte_mbuf_dynfield_register(&counter_dynfield_desc);
> + if (counter_dynfield_offset < 0) {
> + printf("Error registering mbuf field\n");
> + return -rte_errno;
> + }
> +
> if (init(t, 8, 2) < 0 ||
> create_atomic_qids(t, 8) < 0) {
> printf("%d: Error initializing device\n", __LINE__);
> --
> 2.28.0
>
