This is embarrassing, I forgot to remove my debug logs.
I will update the patch and resend it.
On Sat, Sep 11, 2021 at 11:53 AM <joseph.matan...@gmail.com> wrote:
> From: Joseph Matan <joseph.matan...@gmail.com>
>
> The percentile filter can be very useful when running in a non-ptp-aware
> environment.
> For example, if we set a low percentile value (and the filter length is
> large enough),
> we can still get a good estimation of the real delay,
> even if our setup is running under heavy network traffic.
> (when setting the percentile value to 0.50, the median filter is a private
> use-case of the percentile filter)
>
> Signed-off-by: Joseph Matan <joseph.matan...@gmail.com>
> ---
> clock.c | 3 ++-
> config.c | 6 ++++--
> configs/default.cfg | 1 +
> filter.c | 8 +++++---
> filter.h | 8 +++++---
> makefile | 2 +-
> mmedian.c => mpercentile.c | 42 +++++++++++++++++++++-----------------
> mmedian.h => mpercentile.h | 10 ++++-----
> nsm.c | 2 +-
> port.c | 3 ++-
> ptp4l.8 | 6 +++++-
> tsproc.c | 4 ++--
> tsproc.h | 3 ++-
> 13 files changed, 58 insertions(+), 40 deletions(-)
> rename mmedian.c => mpercentile.c (64%)
> rename mmedian.h => mpercentile.h (81%)
>
> diff --git a/clock.c b/clock.c
> index ec70f91..9c4b6cc 100644
> --- a/clock.c
> +++ b/clock.c
> @@ -1171,7 +1171,8 @@ struct clock *clock_create(enum clock_type type,
> struct config *config,
> }
> c->tsproc = tsproc_create(config_get_int(config, NULL,
> "tsproc_mode"),
> config_get_int(config, NULL,
> "delay_filter"),
> - config_get_int(config, NULL,
> "delay_filter_length"));
> + config_get_int(config, NULL,
> "delay_filter_length"),
> + config_get_double(config, NULL,
> "delay_filter_percentile"));
> if (!c->tsproc) {
> pr_err("Failed to create time stamp processor");
> return NULL;
> diff --git a/config.c b/config.c
> index eb8b988..31351f4 100644
> --- a/config.c
> +++ b/config.c
> @@ -158,8 +158,9 @@ static struct config_enum dataset_comp_enu[] = {
> };
>
> static struct config_enum delay_filter_enu[] = {
> - { "moving_average", FILTER_MOVING_AVERAGE },
> - { "moving_median", FILTER_MOVING_MEDIAN },
> + { "moving_average", FILTER_MOVING_AVERAGE },
> + { "moving_median", FILTER_MOVING_MEDIAN },
> + { "moving_percentile", FILTER_MOVING_PERCENTILE },
> { NULL, 0 },
> };
>
> @@ -238,6 +239,7 @@ struct config_item config_tab[] = {
> PORT_ITEM_INT("delayAsymmetry", 0, INT_MIN, INT_MAX),
> PORT_ITEM_ENU("delay_filter", FILTER_MOVING_MEDIAN,
> delay_filter_enu),
> PORT_ITEM_INT("delay_filter_length", 10, 1, INT_MAX),
> + PORT_ITEM_DBL("delay_filter_percentile", 0.5, 0.0, 1.0),
> PORT_ITEM_ENU("delay_mechanism", DM_E2E, delay_mech_enu),
> GLOB_ITEM_INT("dscp_event", 0, 0, 63),
> GLOB_ITEM_INT("dscp_general", 0, 0, 63),
> diff --git a/configs/default.cfg b/configs/default.cfg
> index d615610..29cba1a 100644
> --- a/configs/default.cfg
> +++ b/configs/default.cfg
> @@ -102,6 +102,7 @@ time_stamping hardware
> tsproc_mode filter
> delay_filter moving_median
> delay_filter_length 10
> +delay_filter_percentile 0.5
> egressLatency 0
> ingressLatency 0
> boundary_clock_jbod 0
> diff --git a/filter.c b/filter.c
> index fceb29a..f779280 100644
> --- a/filter.c
> +++ b/filter.c
> @@ -19,15 +19,17 @@
>
> #include "filter_private.h"
> #include "mave.h"
> -#include "mmedian.h"
> +#include "mpercentile.h"
>
> -struct filter *filter_create(enum filter_type type, int length)
> +struct filter *filter_create(enum filter_type type, int length, double
> percentile)
> {
> switch (type) {
> case FILTER_MOVING_AVERAGE:
> return mave_create(length);
> case FILTER_MOVING_MEDIAN:
> - return mmedian_create(length);
> + return mpercentile_create(length, 0.5);
> + case FILTER_MOVING_PERCENTILE:
> + return mpercentile_create(length, percentile);
> default:
> return NULL;
> }
> diff --git a/filter.h b/filter.h
> index 5a196bc..78be629 100644
> --- a/filter.h
> +++ b/filter.h
> @@ -31,15 +31,17 @@ struct filter;
> enum filter_type {
> FILTER_MOVING_AVERAGE,
> FILTER_MOVING_MEDIAN,
> + FILTER_MOVING_PERCENTILE,
> };
>
> /**
> * Create a new instance of a filter.
> - * @param type The type of the filter to create.
> - * @param length The filter's length.
> + * @param type The type of the filter to create.
> + * @param length The filter's length.
> + * @param percentile The percentile of the delay filter samples (valid
> only for FILTER_MOVING_PERCENTILE filter type).
> * @return A pointer to a new filter on success, NULL otherwise.
> */
> -struct filter *filter_create(enum filter_type type, int length);
> +struct filter *filter_create(enum filter_type type, int length, double
> percentile);
>
> /**
> * Destroy an instance of a filter.
> diff --git a/makefile b/makefile
> index 33e7ca0..7de5eab 100644
> --- a/makefile
> +++ b/makefile
> @@ -23,7 +23,7 @@ VER = -DVER=$(version)
> CFLAGS = -Wall $(VER) $(incdefs) $(DEBUG) $(EXTRA_CFLAGS)
> LDLIBS = -lm -lrt -pthread $(EXTRA_LDFLAGS)
> PRG = ptp4l hwstamp_ctl nsm phc2sys phc_ctl pmc timemaster ts2phc
> -FILTERS = filter.o mave.o mmedian.o
> +FILTERS = filter.o mave.o mpercentile.o
> SERVOS = linreg.o ntpshm.o nullf.o pi.o servo.o
> TRANSP = raw.o transport.o udp.o udp6.o uds.o
> TS2PHC = ts2phc.o lstab.o nmea.o serial.o sock.o ts2phc_generic_master.o \
> diff --git a/mmedian.c b/mpercentile.c
> similarity index 64%
> rename from mmedian.c
> rename to mpercentile.c
> index 2383467..cb4010b 100644
> --- a/mmedian.c
> +++ b/mpercentile.c
> @@ -1,5 +1,5 @@
> /**
> - * @file mmedian.c
> + * @file mpercentile.c
> * @note Copyright (C) 2013 Miroslav Lichvar <mlich...@redhat.com>
> *
> * This program is free software; you can redistribute it and/or modify
> @@ -19,36 +19,41 @@
> #include <stdlib.h>
> #include <string.h>
>
> -#include "mmedian.h"
> +#include "mpercentile.h"
> #include "filter_private.h"
> +#include "print.h"
> +#include "math.h"
>
> -struct mmedian {
> +struct mpercentile {
> struct filter filter;
> int cnt;
> int len;
> int index;
> + double percentile;
> + int percentile_index;
> /* Indices sorted by value. */
> int *order;
> /* Values stored in circular buffer. */
> tmv_t *samples;
> };
>
> -static void mmedian_destroy(struct filter *filter)
> +static void mpercentile_destroy(struct filter *filter)
> {
> - struct mmedian *m = container_of(filter, struct mmedian, filter);
> + struct mpercentile *m = container_of(filter, struct mpercentile,
> filter);
> free(m->order);
> free(m->samples);
> free(m);
> }
>
> -static tmv_t mmedian_sample(struct filter *filter, tmv_t sample)
> +static tmv_t mpercentile_sample(struct filter *filter, tmv_t sample)
> {
> - struct mmedian *m = container_of(filter, struct mmedian, filter);
> + struct mpercentile *m = container_of(filter, struct mpercentile,
> filter);
> int i;
>
> m->samples[m->index] = sample;
> if (m->cnt < m->len) {
> m->cnt++;
> + m->percentile_index = (int)round(m->percentile * (m->cnt -
> 1));
> } else {
> /* Remove index of the replaced value from order. */
> for (i = 0; i < m->cnt; i++)
> @@ -69,32 +74,30 @@ static tmv_t mmedian_sample(struct filter *filter,
> tmv_t sample)
>
> m->index = (1 + m->index) % m->len;
>
> - if (m->cnt % 2)
> - return m->samples[m->order[m->cnt / 2]];
> - else
> - return tmv_div(tmv_add(m->samples[m->order[m->cnt / 2 -
> 1]],
> - m->samples[m->order[m->cnt / 2]]),
> 2);
> + /*TODO: deleteMe*/ pr_info("lenght(%d) percentile(%0.2f)
> percentile_index(%d), filtered_delay(%lld)",
> + m->cnt, m->percentile, m->percentile_index,
> tmv_to_nanoseconds(m->samples[m->order[m->percentile_index]]));
> + return m->samples[m->order[m->percentile_index]];
> }
>
> -static void mmedian_reset(struct filter *filter)
> +static void mpercentile_reset(struct filter *filter)
> {
> - struct mmedian *m = container_of(filter, struct mmedian, filter);
> + struct mpercentile *m = container_of(filter, struct mpercentile,
> filter);
> m->cnt = 0;
> m->index = 0;
> }
>
> -struct filter *mmedian_create(int length)
> +struct filter *mpercentile_create(int length, double percentile)
> {
> - struct mmedian *m;
> + struct mpercentile *m;
>
> if (length < 1)
> return NULL;
> m = calloc(1, sizeof(*m));
> if (!m)
> return NULL;
> - m->filter.destroy = mmedian_destroy;
> - m->filter.sample = mmedian_sample;
> - m->filter.reset = mmedian_reset;
> + m->filter.destroy = mpercentile_destroy;
> + m->filter.sample = mpercentile_sample;
> + m->filter.reset = mpercentile_reset;
> m->order = calloc(1, length * sizeof(*m->order));
> if (!m->order) {
> free(m);
> @@ -107,5 +110,6 @@ struct filter *mmedian_create(int length)
> return NULL;
> }
> m->len = length;
> + m->percentile = percentile;
> return &m->filter;
> }
> diff --git a/mmedian.h b/mpercentile.h
> similarity index 81%
> rename from mmedian.h
> rename to mpercentile.h
> index 8fee0dc..193a333 100644
> --- a/mmedian.h
> +++ b/mpercentile.h
> @@ -1,6 +1,6 @@
> /**
> - * @file mmedian.h
> - * @brief Implements a moving median.
> + * @file mpercentile.h
> + * @brief Implements a moving percentile.
> * @note Copyright (C) 2013 Miroslav Lichvar <mlich...@redhat.com>
> *
> * This program is free software; you can redistribute it and/or modify
> @@ -17,11 +17,11 @@
> * with this program; if not, write to the Free Software Foundation, Inc.,
> * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> */
> -#ifndef HAVE_MMEDIAN_H
> -#define HAVE_MMEDIAN_H
> +#ifndef HAVE_MPERCENTILE_H
> +#define HAVE_MPERCENTILE_H
>
> #include "filter.h"
>
> -struct filter *mmedian_create(int length);
> +struct filter *mpercentile_create(int length, double percentile);
>
> #endif
> diff --git a/nsm.c b/nsm.c
> index 5aa925b..3ede6a0 100644
> --- a/nsm.c
> +++ b/nsm.c
> @@ -295,7 +295,7 @@ static int nsm_open(struct nsm *nsm, struct config
> *cfg)
> }
> nsm->port_identity.portNumber = 1;
>
> - nsm->tsproc = tsproc_create(TSPROC_RAW, FILTER_MOVING_AVERAGE, 10);
> + nsm->tsproc = tsproc_create(TSPROC_RAW, FILTER_MOVING_AVERAGE, 10,
> 0);
> if (!nsm->tsproc) {
> pr_err("failed to create time stamp processor");
> goto no_tsproc;
> diff --git a/port.c b/port.c
> index c82bdaf..6dae97b 100644
> --- a/port.c
> +++ b/port.c
> @@ -3200,7 +3200,8 @@ struct port *port_open(const char *phc_device,
>
> p->tsproc = tsproc_create(config_get_int(cfg, p->name,
> "tsproc_mode"),
> config_get_int(cfg, p->name,
> "delay_filter"),
> - config_get_int(cfg, p->name,
> "delay_filter_length"));
> + config_get_int(cfg, p->name,
> "delay_filter_length"),
> + config_get_double(cfg, p->name,
> "delay_filter_percentile"));
> if (!p->tsproc) {
> pr_err("Failed to create time stamp processor");
> goto err_uc_service;
> diff --git a/ptp4l.8 b/ptp4l.8
> index a0779ef..3a3fc13 100644
> --- a/ptp4l.8
> +++ b/ptp4l.8
> @@ -340,13 +340,17 @@ The default is filter.
> .TP
> .B delay_filter
> Select the algorithm used to filter the measured delay and peer delay.
> Possible
> -values are moving_average and moving_median.
> +values are moving_average, moving_median and moving_percentile.
> The default is moving_median.
> .TP
> .B delay_filter_length
> The length of the delay filter in samples.
> The default is 10.
> .TP
> +.B delay_filter_percentile
> +The percentile of the delay filter samples (relevant only when choosing
> the delay_filter as moving_percentile).
> +The default is 0.5 (50%).
> +.TP
> .B egressLatency
> Specifies the difference in nanoseconds between the actual transmission
> time at the reference plane and the reported transmit time stamp. This
> diff --git a/tsproc.c b/tsproc.c
> index a871049..8ace4a6 100644
> --- a/tsproc.c
> +++ b/tsproc.c
> @@ -61,7 +61,7 @@ static int weighting(struct tsproc *tsp)
> }
>
> struct tsproc *tsproc_create(enum tsproc_mode mode,
> - enum filter_type delay_filter, int
> filter_length)
> + enum filter_type delay_filter, int
> filter_length, double percentile)
> {
> struct tsproc *tsp;
>
> @@ -81,7 +81,7 @@ struct tsproc *tsproc_create(enum tsproc_mode mode,
> return NULL;
> }
>
> - tsp->delay_filter = filter_create(delay_filter, filter_length);
> + tsp->delay_filter = filter_create(delay_filter, filter_length,
> percentile);
> if (!tsp->delay_filter) {
> free(tsp);
> return NULL;
> diff --git a/tsproc.h b/tsproc.h
> index fdb35a8..5970705 100644
> --- a/tsproc.h
> +++ b/tsproc.h
> @@ -40,10 +40,11 @@ enum tsproc_mode {
> * @param mode Time stamp processing mode.
> * @param delay_filter Type of the filter that will be applied to delay.
> * @param filter_length Length of the filter.
> + * @param percentile The percentile of the delay filter samples
> (valid only for FILTER_MOVING_PERCENTILE filter type).
> * @return A pointer to a new tsproc on success, NULL
> otherwise.
> */
> struct tsproc *tsproc_create(enum tsproc_mode mode,
> - enum filter_type delay_filter, int
> filter_length);
> + enum filter_type delay_filter, int
> filter_length, double percentile);
>
> /**
> * Destroy a time stamp processor.
> --
> 2.17.1
>
>
_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
