Re: [dpdk-dev] [PATCH v5 03/15] ethdev: register mbuf field and flags for timestamp

[Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>]

On 10/30/20 8:29 PM, Thomas Monjalon wrote:
> During port configure or queue setup, the offload flags
> DEV_RX_OFFLOAD_TIMESTAMP and DEV_TX_OFFLOAD_SEND_ON_TIMESTAMP
> trigger the registration of the related mbuf field and flags.
>
> Previously, the Tx timestamp field and flag were registered in testpmd,
> as described in mlx5 guide.
> For the general usage of Rx and Tx timestamps,
> managing registrations inside ethdev is simpler and properly documented.
>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>

A nit below, but regardless of outcome:
Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>

[snip]

> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> index b12bb3854d..7c9aadb461 100644
> --- a/lib/librte_ethdev/rte_ethdev.c
> +++ b/lib/librte_ethdev/rte_ethdev.c
> @@ -31,6 +31,7 @@
>   #include <rte_mempool.h>
>   #include <rte_malloc.h>
>   #include <rte_mbuf.h>
> +#include <rte_mbuf_dyn.h>
>   #include <rte_errno.h>
>   #include <rte_spinlock.h>
>   #include <rte_string_fns.h>
> @@ -1232,6 +1233,59 @@ eth_dev_check_lro_pkt_size(uint16_t port_id, uint32_t config_size,
>   	return ret;
>   }
>   
> +static inline int
> +eth_dev_timestamp_mbuf_register(uint64_t rx_offloads, uint64_t tx_offloads)
> +{
> +	static const struct rte_mbuf_dynfield field_desc = {
> +		.name = RTE_MBUF_DYNFIELD_TIMESTAMP_NAME,
> +		.size = sizeof(rte_mbuf_timestamp_t),
> +		.align = __alignof__(rte_mbuf_timestamp_t),
> +	};
> +	static const struct rte_mbuf_dynflag rx_flag_desc = {
> +		.name = RTE_MBUF_DYNFLAG_RX_TIMESTAMP_NAME,
> +	};
> +	static const struct rte_mbuf_dynflag tx_flag_desc = {
> +		.name = RTE_MBUF_DYNFLAG_TX_TIMESTAMP_NAME,
> +	};
> +	static bool done_rx, done_tx;

Are these static fields remain by intention? May be I misunderstand
the summary of the previous discussion.

> +	bool todo_rx, todo_tx;
> +	int offset;
> +
> +	todo_rx = (rx_offloads & DEV_RX_OFFLOAD_TIMESTAMP) != 0
> +		&& !done_rx;
> +	todo_tx = (tx_offloads & DEV_TX_OFFLOAD_SEND_ON_TIMESTAMP) != 0
> +		&& !done_tx;
> +
> +	if (todo_rx || todo_tx) {
> +		offset = rte_mbuf_dynfield_register(&field_desc);
> +		if (offset < 0) {
> +			RTE_ETHDEV_LOG(ERR,
> +					"Failed to register mbuf field for timestamp\n");
> +			return -rte_errno;
> +		}
> +	}
> +	if (todo_rx) {
> +		offset = rte_mbuf_dynflag_register(&rx_flag_desc);
> +		if (offset < 0) {
> +			RTE_ETHDEV_LOG(ERR,
> +					"Failed to register mbuf flag for Rx timestamp\n");
> +			return -rte_errno;
> +		}
> +		done_rx = true;
> +	}
> +	if (todo_tx) {
> +		offset = rte_mbuf_dynflag_register(&tx_flag_desc);
> +		if (offset < 0) {
> +			RTE_ETHDEV_LOG(ERR,
> +					"Failed to register mbuf flag for Tx timestamp\n");
> +			return -rte_errno;
> +		}
> +		done_tx = true;
> +	}
> +
> +	return 0;
> +}
> +
>   /*
>    * Validate offloads that are requested through rte_eth_dev_configure against
>    * the offloads successfully set by the ethernet device.

[snip]