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

[Thomas Monjalon <thomas@monjalon.net>]

29/10/2020 11:08, Andrew Rybchenko:
> On 10/29/20 12:27 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 small note below, other than that
> 
> Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> 
> > +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;
> 
> I think we don't need these static flags. We can just repeat
> registeration request and it will simply lookup and return
> the same offset/flagbit as before.

Absolutely.
I did it as a small optimization in control path.

I hesitated. Given it is only 2 booleans,
do you prefer with or without or no opinion?