Re: [dpdk-dev] [PATCH 04/15] latency: switch timestamp to dynamic mbuf field

[Thomas Monjalon <thomas@monjalon.net>]

29/10/2020 11:13, Andrew Rybchenko:
> On 10/29/20 12:27 PM, Thomas Monjalon wrote:
> > The mbuf timestamp is moved to a dynamic field
> > in order to allow removal of the deprecated static field.
> > The related mbuf flag is also replaced with the dynamic one.
> > 
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> 
> [snip]
> 
> > diff --git a/lib/librte_latencystats/rte_latencystats.c b/lib/librte_latencystats/rte_latencystats.c
> > index ba2fff3bcb..a21f6239d9 100644
> > --- a/lib/librte_latencystats/rte_latencystats.c
> > +++ b/lib/librte_latencystats/rte_latencystats.c
> 
> [snip]
> 
> > @@ -204,6 +216,14 @@ int
> >  rte_latencystats_init(uint64_t app_samp_intvl,
> >  		rte_latency_stats_flow_type_fn user_cb)
> >  {
> > +	static const struct rte_mbuf_dynfield timestamp_dynfield_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 timestamp_dynflag_desc = {
> > +		.name = RTE_MBUF_DYNFLAG_RX_TIMESTAMP_NAME,
> > +	};
> 
> I dislike the duplication. If we can't just lookup by name
> which is done after ethdev configure (I guess so), may be
> ethdev should provide an API to register?

That's because it is a separate library.
We don't know whether the feature timestamp is already enabled.
We have the port_id, so we could do something.
But the current behaviour is to use timestamp even if it is disabled
at ethdev level. And I don't want to change the behaviour.

Maybe the right solution is to register a separate field for this lib.
Anyway the time base is not the same.