DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] doc: announce API change in timer
@ 2020-08-03 11:20 Sarosh Arif
  2020-08-03 16:45 ` Carrillo, Erik G
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Sarosh Arif @ 2020-08-03 11:20 UTC (permalink / raw)
  To: erik.g.carrillo, rsanford; +Cc: dev, Sarosh Arif

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 ``rte_timer_stop_sync``  will
+  have a int return type in order to return with -1 in when this condition
+  occures.
-- 
2.17.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] [PATCH] doc: announce API change in timer
  2020-08-03 11:20 [dpdk-dev] [PATCH] doc: announce API change in timer Sarosh Arif
@ 2020-08-03 16:45 ` Carrillo, Erik G
  2020-08-03 23:16 ` Honnappa Nagarahalli
  2020-08-04  3:51 ` [dpdk-dev] [PATCH v2] " Sarosh Arif
  2 siblings, 0 replies; 7+ messages in thread
From: Carrillo, Erik G @ 2020-08-03 16:45 UTC (permalink / raw)
  To: Sarosh Arif, rsanford; +Cc: dev

> -----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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] [PATCH] doc: announce API change in timer
  2020-08-03 11:20 [dpdk-dev] [PATCH] doc: announce API change in timer Sarosh Arif
  2020-08-03 16:45 ` Carrillo, Erik G
@ 2020-08-03 23:16 ` Honnappa Nagarahalli
  2020-08-04  3:28   ` Sarosh Arif
  2020-08-04  3:51 ` [dpdk-dev] [PATCH v2] " Sarosh Arif
  2 siblings, 1 reply; 7+ messages in thread
From: Honnappa Nagarahalli @ 2020-08-03 23:16 UTC (permalink / raw)
  To: Sarosh Arif, erik.g.carrillo, rsanford; +Cc: dev, nd, Honnappa Nagarahalli, nd

<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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] [PATCH] doc: announce API change in timer
  2020-08-03 23:16 ` Honnappa Nagarahalli
@ 2020-08-04  3:28   ` Sarosh Arif
  2020-08-04 14:24     ` Carrillo, Erik G
  0 siblings, 1 reply; 7+ messages in thread
From: Sarosh Arif @ 2020-08-04  3:28 UTC (permalink / raw)
  To: Honnappa Nagarahalli, erik.g.carrillo; +Cc: rsanford, dev, nd

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
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [dpdk-dev] [PATCH v2] doc: announce API change in timer
  2020-08-03 11:20 [dpdk-dev] [PATCH] doc: announce API change in timer Sarosh Arif
  2020-08-03 16:45 ` Carrillo, Erik G
  2020-08-03 23:16 ` Honnappa Nagarahalli
@ 2020-08-04  3:51 ` Sarosh Arif
  2 siblings, 0 replies; 7+ messages in thread
From: Sarosh Arif @ 2020-08-04  3:51 UTC (permalink / raw)
  To: erik.g.carrillo, rsanford; +Cc: dev, Sarosh Arif

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>
---
v2: rephrase and fix typo
---
 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..227950165 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: 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 be updated
+  with an int return type so that an error code can be returned when this
+  condition occurs.
-- 
2.17.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] [PATCH] doc: announce API change in timer
  2020-08-04  3:28   ` Sarosh Arif
@ 2020-08-04 14:24     ` Carrillo, Erik G
  2020-08-04 20:50       ` Honnappa Nagarahalli
  0 siblings, 1 reply; 7+ messages in thread
From: Carrillo, Erik G @ 2020-08-04 14:24 UTC (permalink / raw)
  To: Sarosh Arif, Honnappa Nagarahalli; +Cc: rsanford, dev, nd



> -----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
> >

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] [PATCH] doc: announce API change in timer
  2020-08-04 14:24     ` Carrillo, Erik G
@ 2020-08-04 20:50       ` Honnappa Nagarahalli
  0 siblings, 0 replies; 7+ messages in thread
From: Honnappa Nagarahalli @ 2020-08-04 20:50 UTC (permalink / raw)
  To: Carrillo, Erik G, Sarosh Arif; +Cc: rsanford, dev, nd, Honnappa Nagarahalli, nd

<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
> > >

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-08-04 20:50 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-03 11:20 [dpdk-dev] [PATCH] doc: announce API change in timer Sarosh Arif
2020-08-03 16:45 ` Carrillo, Erik G
2020-08-03 23:16 ` Honnappa Nagarahalli
2020-08-04  3:28   ` Sarosh Arif
2020-08-04 14:24     ` Carrillo, Erik G
2020-08-04 20:50       ` Honnappa Nagarahalli
2020-08-04  3:51 ` [dpdk-dev] [PATCH v2] " Sarosh Arif

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).