doc: announce API change in timer
Checks
Commit Message
If the user tries to reset/stop some other timer in it's callback
function, which is also about to expire, using
rte_timer_reset_sync/rte_timer_stop_sync the application goes into
an infinite loop. This happens because
rte_timer_reset_sync/rte_timer_stop_sync loop until the timer
resets/stops and there is check inside timer_set_config_state which
prevents a running timer from being reset/stopped by not it's own
timer_cb. Therefore timer_set_config_state returns -1 due to which
rte_timer_reset returns -1 and rte_timer_reset_sync goes into an
infinite loop
To to prevent this rte_timer_reset_sync and rte_timer_stop_sync should
have int return types, so that -1 can be returned if the above condition
occurs
Signed-off-by: Sarosh Arif <sarosh.arif@emumba.com>
---
doc/guides/rel_notes/deprecation.rst | 6 ++++++
1 file changed, 6 insertions(+)
Comments
> -----Original Message-----
> From: Sarosh Arif <sarosh.arif@emumba.com>
> Sent: Monday, August 3, 2020 6:21 AM
> To: Carrillo, Erik G <erik.g.carrillo@intel.com>; rsanford@akamai.com
> Cc: dev@dpdk.org; Sarosh Arif <sarosh.arif@emumba.com>
> Subject: [PATCH] doc: announce API change in timer
>
> If the user tries to reset/stop some other timer in it's callback function, which
> is also about to expire, using rte_timer_reset_sync/rte_timer_stop_sync the
> application goes into an infinite loop. This happens because
> rte_timer_reset_sync/rte_timer_stop_sync loop until the timer resets/stops
> and there is check inside timer_set_config_state which prevents a running
> timer from being reset/stopped by not it's own timer_cb. Therefore
> timer_set_config_state returns -1 due to which rte_timer_reset returns -1
> and rte_timer_reset_sync goes into an infinite loop
>
> To to prevent this rte_timer_reset_sync and rte_timer_stop_sync should
> have int return types, so that -1 can be returned if the above condition
> occurs
>
> Signed-off-by: Sarosh Arif <sarosh.arif@emumba.com>
> ---
> doc/guides/rel_notes/deprecation.rst | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/doc/guides/rel_notes/deprecation.rst
> b/doc/guides/rel_notes/deprecation.rst
> index ea4cfa7a4..ed93a707d 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -151,3 +151,9 @@ Deprecation Notices
> Python 2 support will be completely removed in 20.11.
> In 20.08, explicit deprecation warnings will be displayed when running
> scripts with Python 2.
> +
> +* timer: Since timer can get stuck in an infinite loop if the
> +application tries to
> + reset/stop some other timer in it's callback function, which is also
> +about to
> + expire. The function ``rte_timer_stop_sync`` and
It looks like this should be rte_timer_reset_sync. Maybe something like:
timer: Timers can get stuck in an infinite loop if their callback tries to synchronously reset/stop some other timer that is also about to expire. The functions ``rte_timer_reset_sync`` and ``rte_timer_stop_sync`` will updated with an int return type so that an error code can be returned when this condition occurs.
Thanks,
Erik
> +``rte_timer_stop_sync`` will
> + have a int return type in order to return with -1 in when this
> +condition
> + occures.
> --
> 2.17.1
<snip>
>
> If the user tries to reset/stop some other timer in it's callback function, which
Is there any use case for this? Why not just say document that the user is not allowed to reset some other timer in the call back function? How does the user get access to some other timer in the call back function?
Not sure if this was discussed earlier, I might have missed.
> is also about to expire, using rte_timer_reset_sync/rte_timer_stop_sync the
> application goes into an infinite loop. This happens because
> rte_timer_reset_sync/rte_timer_stop_sync loop until the timer resets/stops
> and there is check inside timer_set_config_state which prevents a running
> timer from being reset/stopped by not it's own timer_cb. Therefore
> timer_set_config_state returns -1 due to which rte_timer_reset returns -1 and
> rte_timer_reset_sync goes into an infinite loop
>
> To to prevent this rte_timer_reset_sync and rte_timer_stop_sync should have
> int return types, so that -1 can be returned if the above condition occurs
>
> Signed-off-by: Sarosh Arif <sarosh.arif@emumba.com>
> ---
> doc/guides/rel_notes/deprecation.rst | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/doc/guides/rel_notes/deprecation.rst
> b/doc/guides/rel_notes/deprecation.rst
> index ea4cfa7a4..ed93a707d 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -151,3 +151,9 @@ Deprecation Notices
> Python 2 support will be completely removed in 20.11.
> In 20.08, explicit deprecation warnings will be displayed when running
> scripts with Python 2.
> +
> +* timer: Since timer can get stuck in an infinite loop if the
> +application tries to
> + reset/stop some other timer in it's callback function, which is also
> +about to
> + expire. The function ``rte_timer_stop_sync`` and
> +``rte_timer_stop_sync`` will
> + have a int return type in order to return with -1 in when this
> +condition
> + occures.
> --
> 2.17.1
Thank you Eric, I will fix the mistakes in v2
On Tue, Aug 4, 2020 at 4:16 AM Honnappa Nagarahalli
<Honnappa.Nagarahalli@arm.com> wrote:
>
> <snip>
>
> >
> > If the user tries to reset/stop some other timer in it's callback function, which
> Is there any use case for this? Why not just say document that the user is not allowed to reset some other timer in the call back function? How does the user get access to some other timer in the call back function?
> Not sure if this was discussed earlier, I might have missed.
The issue is more clearly described in bug 491 here is a link:
https://bugs.dpdk.org/show_bug.cgi?id=491
further discussion on this issue was done on the following patch:
https://patches.dpdk.org/patch/73683/
>
> > is also about to expire, using rte_timer_reset_sync/rte_timer_stop_sync the
> > application goes into an infinite loop. This happens because
> > rte_timer_reset_sync/rte_timer_stop_sync loop until the timer resets/stops
> > and there is check inside timer_set_config_state which prevents a running
> > timer from being reset/stopped by not it's own timer_cb. Therefore
> > timer_set_config_state returns -1 due to which rte_timer_reset returns -1 and
> > rte_timer_reset_sync goes into an infinite loop
> >
> > To to prevent this rte_timer_reset_sync and rte_timer_stop_sync should have
> > int return types, so that -1 can be returned if the above condition occurs
> >
> > Signed-off-by: Sarosh Arif <sarosh.arif@emumba.com>
> > ---
> > doc/guides/rel_notes/deprecation.rst | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/doc/guides/rel_notes/deprecation.rst
> > b/doc/guides/rel_notes/deprecation.rst
> > index ea4cfa7a4..ed93a707d 100644
> > --- a/doc/guides/rel_notes/deprecation.rst
> > +++ b/doc/guides/rel_notes/deprecation.rst
> > @@ -151,3 +151,9 @@ Deprecation Notices
> > Python 2 support will be completely removed in 20.11.
> > In 20.08, explicit deprecation warnings will be displayed when running
> > scripts with Python 2.
> > +
> > +* timer: Since timer can get stuck in an infinite loop if the
> > +application tries to
> > + reset/stop some other timer in it's callback function, which is also
> > +about to
> > + expire. The function ``rte_timer_stop_sync`` and
> > +``rte_timer_stop_sync`` will
> > + have a int return type in order to return with -1 in when this
> > +condition
> > + occures.
> > --
> > 2.17.1
>
> -----Original Message-----
> From: Sarosh Arif <sarosh.arif@emumba.com>
> Sent: Monday, August 3, 2020 10:29 PM
> To: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>; Carrillo, Erik G
> <erik.g.carrillo@intel.com>
> Cc: rsanford@akamai.com; dev@dpdk.org; nd <nd@arm.com>
> Subject: Re: [dpdk-dev] [PATCH] doc: announce API change in timer
>
> Thank you Eric, I will fix the mistakes in v2
>
> On Tue, Aug 4, 2020 at 4:16 AM Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com> wrote:
> >
> > <snip>
> >
> > >
> > > If the user tries to reset/stop some other timer in it's callback
> > > function, which
> > Is there any use case for this? Why not just say document that the user is
> not allowed to reset some other timer in the call back function? How does
> the user get access to some other timer in the call back function?
> > Not sure if this was discussed earlier, I might have missed.
> The issue is more clearly described in bug 491 here is a link:
> https://bugs.dpdk.org/show_bug.cgi?id=491
> further discussion on this issue was done on the following patch:
> https://patches.dpdk.org/patch/73683/
>
I like Honnappa's suggestion... we could document that the *_sync functions shouldn't be used inside timer callbacks since there are cases where it could hang. Instead, if doing this was desired, the regular versions could be used there, and the return values could be checked in that case.
> >
> > > is also about to expire, using
> > > rte_timer_reset_sync/rte_timer_stop_sync the application goes into
> > > an infinite loop. This happens because
> > > rte_timer_reset_sync/rte_timer_stop_sync loop until the timer
> > > resets/stops and there is check inside timer_set_config_state which
> > > prevents a running timer from being reset/stopped by not it's own
> > > timer_cb. Therefore timer_set_config_state returns -1 due to which
> > > rte_timer_reset returns -1 and rte_timer_reset_sync goes into an
> > > infinite loop
> > >
> > > To to prevent this rte_timer_reset_sync and rte_timer_stop_sync
> > > should have int return types, so that -1 can be returned if the
> > > above condition occurs
> > >
> > > Signed-off-by: Sarosh Arif <sarosh.arif@emumba.com>
> > > ---
> > > doc/guides/rel_notes/deprecation.rst | 6 ++++++
> > > 1 file changed, 6 insertions(+)
> > >
> > > diff --git a/doc/guides/rel_notes/deprecation.rst
> > > b/doc/guides/rel_notes/deprecation.rst
> > > index ea4cfa7a4..ed93a707d 100644
> > > --- a/doc/guides/rel_notes/deprecation.rst
> > > +++ b/doc/guides/rel_notes/deprecation.rst
> > > @@ -151,3 +151,9 @@ Deprecation Notices
> > > Python 2 support will be completely removed in 20.11.
> > > In 20.08, explicit deprecation warnings will be displayed when running
> > > scripts with Python 2.
> > > +
> > > +* timer: Since timer can get stuck in an infinite loop if the
> > > +application tries to
> > > + reset/stop some other timer in it's callback function, which is
> > > +also about to
> > > + expire. The function ``rte_timer_stop_sync`` and
> > > +``rte_timer_stop_sync`` will
> > > + have a int return type in order to return with -1 in when this
> > > +condition
> > > + occures.
> > > --
> > > 2.17.1
> >
<snip>
[
> > Subject: Re: [dpdk-dev] [PATCH] doc: announce API change in timer
> >
> > Thank you Eric, I will fix the mistakes in v2
> >
> > On Tue, Aug 4, 2020 at 4:16 AM Honnappa Nagarahalli
> > <Honnappa.Nagarahalli@arm.com> wrote:
> > >
> > > <snip>
> > >
> > > >
> > > > If the user tries to reset/stop some other timer in it's callback
> > > > function, which
> > > Is there any use case for this? Why not just say document that the
> > > user is
> > not allowed to reset some other timer in the call back function? How
> > does the user get access to some other timer in the call back function?
> > > Not sure if this was discussed earlier, I might have missed.
> > The issue is more clearly described in bug 491 here is a link:
> > https://bugs.dpdk.org/show_bug.cgi?id=491
> > further discussion on this issue was done on the following patch:
> > https://patches.dpdk.org/patch/73683/
Thanks for the links.
> >
>
> I like Honnappa's suggestion... we could document that the *_sync functions
> shouldn't be used inside timer callbacks since there are cases where it could
> hang. Instead, if doing this was desired, the regular versions could be used
> there, and the return values could be checked in that case.
Agree, non sync functions can be used.
>
> > >
> > > > is also about to expire, using
> > > > rte_timer_reset_sync/rte_timer_stop_sync the application goes into
> > > > an infinite loop. This happens because
> > > > rte_timer_reset_sync/rte_timer_stop_sync loop until the timer
> > > > resets/stops and there is check inside timer_set_config_state
> > > > which prevents a running timer from being reset/stopped by not
> > > > it's own timer_cb. Therefore timer_set_config_state returns -1 due
> > > > to which rte_timer_reset returns -1 and rte_timer_reset_sync goes
> > > > into an infinite loop
> > > >
> > > > To to prevent this rte_timer_reset_sync and rte_timer_stop_sync
> > > > should have int return types, so that -1 can be returned if the
> > > > above condition occurs
> > > >
> > > > Signed-off-by: Sarosh Arif <sarosh.arif@emumba.com>
> > > > ---
> > > > doc/guides/rel_notes/deprecation.rst | 6 ++++++
> > > > 1 file changed, 6 insertions(+)
> > > >
> > > > diff --git a/doc/guides/rel_notes/deprecation.rst
> > > > b/doc/guides/rel_notes/deprecation.rst
> > > > index ea4cfa7a4..ed93a707d 100644
> > > > --- a/doc/guides/rel_notes/deprecation.rst
> > > > +++ b/doc/guides/rel_notes/deprecation.rst
> > > > @@ -151,3 +151,9 @@ Deprecation Notices
> > > > Python 2 support will be completely removed in 20.11.
> > > > In 20.08, explicit deprecation warnings will be displayed when running
> > > > scripts with Python 2.
> > > > +
> > > > +* timer: Since timer can get stuck in an infinite loop if the
> > > > +application tries to
> > > > + reset/stop some other timer in it's callback function, which is
> > > > +also about to
> > > > + expire. The function ``rte_timer_stop_sync`` and
> > > > +``rte_timer_stop_sync`` will
> > > > + have a int return type in order to return with -1 in when this
> > > > +condition
> > > > + occures.
> > > > --
> > > > 2.17.1
> > >
@@ -151,3 +151,9 @@ Deprecation Notices
Python 2 support will be completely removed in 20.11.
In 20.08, explicit deprecation warnings will be displayed when running
scripts with Python 2.
+
+* timer: Since timer can get stuck in an infinite loop if the application tries to
+ reset/stop some other timer in it's callback function, which is also about to
+ expire. The function ``rte_timer_stop_sync`` and ``rte_timer_stop_sync`` will
+ have a int return type in order to return with -1 in when this condition
+ occures.