[04/15] latency: switch timestamp to dynamic mbuf field
Checks
Commit Message
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>
---
lib/librte_latencystats/rte_latencystats.c | 48 +++++++++++++++++++---
1 file changed, 43 insertions(+), 5 deletions(-)
Comments
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?
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.
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
<snip>
> rte_mbuf_dynflag_register(×tamp_dynflag_desc);
> + if (timestamp_dynflag_offset < 0) {
> + RTE_LOG(ERR, LATENCY_STATS,
> + "Cannot register mbuf field for timestamp\n");
Field->flag, i.e. field should be changed to flag?
<snip>
29/10/2020 15:20, Pattan, Reshma:
>
> > -----Original Message-----
> > From: Thomas Monjalon <thomas@monjalon.net>
>
>
> <snip>
>
> > rte_mbuf_dynflag_register(×tamp_dynflag_desc);
> > + if (timestamp_dynflag_offset < 0) {
> > + RTE_LOG(ERR, LATENCY_STATS,
> > + "Cannot register mbuf field for timestamp\n");
>
> Field->flag, i.e. field should be changed to flag?
Yes good catch, thanks!
@@ -8,7 +8,9 @@
#include <math.h>
#include <rte_string_fns.h>
+#include <rte_bitops.h>
#include <rte_mbuf.h>
+#include <rte_mbuf_dyn.h>
#include <rte_log.h>
#include <rte_cycles.h>
#include <rte_ethdev.h>
@@ -31,6 +33,16 @@ latencystat_cycles_per_ns(void)
/* Macros for printing using RTE_LOG */
#define RTE_LOGTYPE_LATENCY_STATS RTE_LOGTYPE_USER1
+static uint64_t timestamp_dynflag;
+static int timestamp_dynfield_offset = -1;
+
+static inline rte_mbuf_timestamp_t *
+timestamp_dynfield(struct rte_mbuf *mbuf)
+{
+ return RTE_MBUF_DYNFIELD(mbuf,
+ timestamp_dynfield_offset, rte_mbuf_timestamp_t *);
+}
+
static const char *MZ_RTE_LATENCY_STATS = "rte_latencystats";
static int latency_stats_index;
static uint64_t samp_intvl;
@@ -128,10 +140,10 @@ add_time_stamps(uint16_t pid __rte_unused,
diff_tsc = now - prev_tsc;
timer_tsc += diff_tsc;
- if ((pkts[i]->ol_flags & PKT_RX_TIMESTAMP) == 0
+ if ((pkts[i]->ol_flags & timestamp_dynflag) == 0
&& (timer_tsc >= samp_intvl)) {
- pkts[i]->timestamp = now;
- pkts[i]->ol_flags |= PKT_RX_TIMESTAMP;
+ *timestamp_dynfield(pkts[i]) = now;
+ pkts[i]->ol_flags |= timestamp_dynflag;
timer_tsc = 0;
}
prev_tsc = now;
@@ -161,8 +173,8 @@ calc_latency(uint16_t pid __rte_unused,
now = rte_rdtsc();
for (i = 0; i < nb_pkts; i++) {
- if (pkts[i]->ol_flags & PKT_RX_TIMESTAMP)
- latency[cnt++] = now - pkts[i]->timestamp;
+ if (pkts[i]->ol_flags & timestamp_dynflag)
+ latency[cnt++] = now - *timestamp_dynfield(pkts[i]);
}
rte_spinlock_lock(&glob_stats->lock);
@@ -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,
+ };
unsigned int i;
uint16_t pid;
uint16_t qid;
@@ -211,6 +231,7 @@ rte_latencystats_init(uint64_t app_samp_intvl,
const char *ptr_strings[NUM_LATENCY_STATS] = {0};
const struct rte_memzone *mz = NULL;
const unsigned int flags = 0;
+ int timestamp_dynflag_offset;
int ret;
if (rte_memzone_lookup(MZ_RTE_LATENCY_STATS))
@@ -241,6 +262,23 @@ rte_latencystats_init(uint64_t app_samp_intvl,
return -1;
}
+ /* Register mbuf field and flag for Rx timestamp */
+ timestamp_dynfield_offset =
+ rte_mbuf_dynfield_register(×tamp_dynfield_desc);
+ if (timestamp_dynfield_offset < 0) {
+ RTE_LOG(ERR, LATENCY_STATS,
+ "Cannot register mbuf field for timestamp\n");
+ return -rte_errno;
+ }
+ timestamp_dynflag_offset =
+ rte_mbuf_dynflag_register(×tamp_dynflag_desc);
+ if (timestamp_dynflag_offset < 0) {
+ RTE_LOG(ERR, LATENCY_STATS,
+ "Cannot register mbuf field for timestamp\n");
+ return -rte_errno;
+ }
+ timestamp_dynflag = RTE_BIT64(timestamp_dynflag_offset);
+
/** Register Rx/Tx callbacks */
RTE_ETH_FOREACH_DEV(pid) {
struct rte_eth_dev_info dev_info;