From: "Wiles, Keith" <keith.wiles@intel.com>
To: "jozmarti@cisco.com" <jozmarti@cisco.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH] rte_delay_us can be replaced with user function
Date: Tue, 19 Jul 2016 13:17:28 +0000 [thread overview]
Message-ID: <48BAF571-3AFC-4562-BD24-364CC3BD9642@intel.com> (raw)
In-Reply-To: <1468932143-9321-1-git-send-email-jozmarti@cisco.com>
Hi Jozef,
I have two quick comments inline.
> On Jul 19, 2016, at 7:42 AM, jozmarti@cisco.com wrote:
>
> From: Jozef Martiniak <jozmarti@cisco.com>
>
> when running single-core, some drivers tend to call rte_delay_us for a
> long time, and that is causing packet drops.
> Attached patch introduces 2 new functions:
>
> void rte_delay_us_callback_register(void(*userfunc)(unsigned));
> void rte_delay_us_callback_unregister(void);
>
> First one replaces rte_delay_us with userfunc and second one restores
> original rte_delay_us.
> Test user_delay_us is included.
>
> Signed-off-by: Jozef Martiniak <jozmarti@cisco.com>
> ---
> app/test/test_cycles.c | 39 ++++++++++++++++++++++
> lib/librte_eal/common/eal_common_timer.c | 19 +++++++++++
> lib/librte_eal/common/include/generic/rte_cycles.h | 13 ++++++++
> 3 files changed, 71 insertions(+)
>
> diff --git a/app/test/test_cycles.c b/app/test/test_cycles.c
> index f6c043a..2b44a53 100644
> --- a/app/test/test_cycles.c
> +++ b/app/test/test_cycles.c
> @@ -90,3 +90,42 @@ test_cycles(void)
> }
>
> REGISTER_TEST_COMMAND(cycles_autotest, test_cycles);
> +
> +/*
> + * rte_delay_us_callback test
> + *
> + * - check if callback is correctly registered/unregistered
> + *
> + */
> +
> +static int pattern;
> +static void my_rte_delay_us(unsigned us)
> +{
> + pattern += us;
> +}
> +
> +static int
> +test_user_delay_us(void)
> +{
> + pattern = 0;
> +
> + rte_delay_us_callback_register(my_rte_delay_us);
> +
> + rte_delay_us(2);
> + if (pattern != 2)
> + return -1;
> +
> + rte_delay_us(3);
> + if (pattern != 5)
> + return -1;
> +
> + rte_delay_us_callback_unregister();
> +
> + rte_delay_us(3);
> + if (pattern != 5)
> + return -1;
> +
> + return 0;
> +}
> +
> +REGISTER_TEST_COMMAND(user_delay_us, test_user_delay_us);
> diff --git a/lib/librte_eal/common/eal_common_timer.c b/lib/librte_eal/common/eal_common_timer.c
> index c4227cd..a982562 100644
> --- a/lib/librte_eal/common/eal_common_timer.c
> +++ b/lib/librte_eal/common/eal_common_timer.c
> @@ -47,9 +47,18 @@
> /* The frequency of the RDTSC timer resolution */
> static uint64_t eal_tsc_resolution_hz;
>
> +/* User function which replaces rte_delay_us function */
> +static void (*rte_delay_us_override)(unsigned) = NULL;
> +
> void
> rte_delay_us(unsigned us)
> {
> + if (unlikely(rte_delay_us_override != NULL))
> + {
> + rte_delay_us_override(us);
> + return;
> + }
> +
> const uint64_t start = rte_get_timer_cycles();
> const uint64_t ticks = (uint64_t)us * rte_get_timer_hz() / 1E6;
> while ((rte_get_timer_cycles() - start) < ticks)
> @@ -84,3 +93,13 @@ set_tsc_freq(void)
> RTE_LOG(DEBUG, EAL, "TSC frequency is ~%" PRIu64 " KHz\n", freq / 1000);
> eal_tsc_resolution_hz = freq;
> }
> +
> +void rte_delay_us_callback_register(void (*userfunc)(unsigned))
> +{
> + rte_delay_us_override = userfunc;
> +}
> +
> +void rte_delay_us_callback_unregister(void)
> +{
> + rte_delay_us_override = NULL;
> +}
I guess I would have used the rte_delay_us_callback_register(NULL) to unregister, but this is fine.
> diff --git a/lib/librte_eal/common/include/generic/rte_cycles.h b/lib/librte_eal/common/include/generic/rte_cycles.h
> index 8cc21f2..274f798 100644
> --- a/lib/librte_eal/common/include/generic/rte_cycles.h
> +++ b/lib/librte_eal/common/include/generic/rte_cycles.h
> @@ -202,4 +202,17 @@ rte_delay_ms(unsigned ms)
> rte_delay_us(ms * 1000);
> }
>
> +/**
> + * Replace rte_delay_us with user defined function.
> + *
> + * @param userfunc
> + * User function which replaces rte_delay_us.
> + */
> +void rte_delay_us_callback_register(void(*userfunc)(unsigned));
> +
> +/**
> + * Unregister user callback function. Restores original rte_delay_us.
> + */
> +void rte_delay_us_callback_unregister(void);
Just a note we need to add these two new APIs to the map file for ABI checking.
Other then these two comments I would give this one a +1 unless someone else has some comments.
> +
> #endif /* _RTE_CYCLES_H_ */
> --
> 2.1.4
>
next prev parent reply other threads:[~2016-07-19 13:17 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-07-19 12:42 jozmarti
2016-07-19 13:17 ` Wiles, Keith [this message]
2016-07-19 13:21 ` Thomas Monjalon
2016-07-19 13:52 ` Jozef Martiniak -X (jozmarti - PANTHEON TECHNOLOGIES at Cisco)
2016-07-20 12:10 jozmarti
2016-09-13 20:04 ` Thomas Monjalon
2016-09-21 13:12 ` Thomas Monjalon
2016-09-22 8:37 ` Jozef Martiniak -X (jozmarti - PANTHEON TECHNOLOGIES at Cisco)
2016-09-22 15:08 ` Thomas Monjalon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=48BAF571-3AFC-4562-BD24-364CC3BD9642@intel.com \
--to=keith.wiles@intel.com \
--cc=dev@dpdk.org \
--cc=jozmarti@cisco.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).