DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] eventdev: fix device probe and remove for secondary process
@ 2020-04-27 18:10 pbhagavatula
  2020-05-01 13:26 ` Jerin Jacob
  0 siblings, 1 reply; 8+ messages in thread
From: pbhagavatula @ 2020-04-27 18:10 UTC (permalink / raw)
  To: jerinj, Anatoly Burakov; +Cc: dev, Pavan Nikhilesh

From: Pavan Nikhilesh <pbhagavatula@marvell.com>

When probing event device in secondary process skip reinitializing
the device data structure as it is already done in primary process.

When removing event device in secondary process skip closing the
event device as it should be done by primary process.

Fixes: 322d0345c2bc ("eventdev: implement PMD registration functions")

Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
---
 lib/librte_eventdev/rte_eventdev.c         | 13 ++++++++-----
 lib/librte_eventdev/rte_eventdev_pmd_pci.h |  8 +++++---
 2 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/lib/librte_eventdev/rte_eventdev.c b/lib/librte_eventdev/rte_eventdev.c
index b987e0745..9aca7fbd5 100644
--- a/lib/librte_eventdev/rte_eventdev.c
+++ b/lib/librte_eventdev/rte_eventdev.c
@@ -1364,14 +1364,17 @@ rte_event_pmd_allocate(const char *name, int socket_id)
 
 		eventdev->data = eventdev_data;
 
-		strlcpy(eventdev->data->name, name, RTE_EVENTDEV_NAME_MAX_LEN);
+		if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
 
-		eventdev->data->dev_id = dev_id;
-		eventdev->data->socket_id = socket_id;
-		eventdev->data->dev_started = 0;
+			strlcpy(eventdev->data->name, name,
+				RTE_EVENTDEV_NAME_MAX_LEN);
 
-		eventdev->attached = RTE_EVENTDEV_ATTACHED;
+			eventdev->data->dev_id = dev_id;
+			eventdev->data->socket_id = socket_id;
+			eventdev->data->dev_started = 0;
+		}
 
+		eventdev->attached = RTE_EVENTDEV_ATTACHED;
 		eventdev_globals.nb_devs++;
 	}
 
diff --git a/lib/librte_eventdev/rte_eventdev_pmd_pci.h b/lib/librte_eventdev/rte_eventdev_pmd_pci.h
index 8fb61386f..443cd38c2 100644
--- a/lib/librte_eventdev/rte_eventdev_pmd_pci.h
+++ b/lib/librte_eventdev/rte_eventdev_pmd_pci.h
@@ -112,9 +112,11 @@ rte_event_pmd_pci_remove(struct rte_pci_device *pci_dev,
 	if (eventdev == NULL)
 		return -ENODEV;
 
-	ret = rte_event_dev_close(eventdev->data->dev_id);
-	if (ret < 0)
-		return ret;
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
+		ret = rte_event_dev_close(eventdev->data->dev_id);
+		if (ret < 0)
+			return ret;
+	}
 
 	/* Invoke PMD device un-init function */
 	if (devuninit)
-- 
2.17.1


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

* Re: [dpdk-dev] [PATCH] eventdev: fix device probe and remove for secondary process
  2020-04-27 18:10 [dpdk-dev] [PATCH] eventdev: fix device probe and remove for secondary process pbhagavatula
@ 2020-05-01 13:26 ` Jerin Jacob
  2020-05-02 10:31   ` Jerin Jacob
  0 siblings, 1 reply; 8+ messages in thread
From: Jerin Jacob @ 2020-05-01 13:26 UTC (permalink / raw)
  To: Pavan Nikhilesh; +Cc: Jerin Jacob, Anatoly Burakov, dpdk-dev

On Mon, Apr 27, 2020 at 11:40 PM <pbhagavatula@marvell.com> wrote:
>
> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
>
> When probing event device in secondary process skip reinitializing
> the device data structure as it is already done in primary process.
>
> When removing event device in secondary process skip closing the
> event device as it should be done by primary process.
>
> Fixes: 322d0345c2bc ("eventdev: implement PMD registration functions")

Cc: stable@dpdk.org

>
> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>


Acked-by: Jerin Jacob <jerinj@marvell.com>



> ---
>  lib/librte_eventdev/rte_eventdev.c         | 13 ++++++++-----
>  lib/librte_eventdev/rte_eventdev_pmd_pci.h |  8 +++++---
>  2 files changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/lib/librte_eventdev/rte_eventdev.c b/lib/librte_eventdev/rte_eventdev.c
> index b987e0745..9aca7fbd5 100644
> --- a/lib/librte_eventdev/rte_eventdev.c
> +++ b/lib/librte_eventdev/rte_eventdev.c
> @@ -1364,14 +1364,17 @@ rte_event_pmd_allocate(const char *name, int socket_id)
>
>                 eventdev->data = eventdev_data;
>
> -               strlcpy(eventdev->data->name, name, RTE_EVENTDEV_NAME_MAX_LEN);
> +               if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
>
> -               eventdev->data->dev_id = dev_id;
> -               eventdev->data->socket_id = socket_id;
> -               eventdev->data->dev_started = 0;
> +                       strlcpy(eventdev->data->name, name,
> +                               RTE_EVENTDEV_NAME_MAX_LEN);
>
> -               eventdev->attached = RTE_EVENTDEV_ATTACHED;
> +                       eventdev->data->dev_id = dev_id;
> +                       eventdev->data->socket_id = socket_id;
> +                       eventdev->data->dev_started = 0;
> +               }
>
> +               eventdev->attached = RTE_EVENTDEV_ATTACHED;
>                 eventdev_globals.nb_devs++;
>         }
>
> diff --git a/lib/librte_eventdev/rte_eventdev_pmd_pci.h b/lib/librte_eventdev/rte_eventdev_pmd_pci.h
> index 8fb61386f..443cd38c2 100644
> --- a/lib/librte_eventdev/rte_eventdev_pmd_pci.h
> +++ b/lib/librte_eventdev/rte_eventdev_pmd_pci.h
> @@ -112,9 +112,11 @@ rte_event_pmd_pci_remove(struct rte_pci_device *pci_dev,
>         if (eventdev == NULL)
>                 return -ENODEV;
>
> -       ret = rte_event_dev_close(eventdev->data->dev_id);
> -       if (ret < 0)
> -               return ret;
> +       if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
> +               ret = rte_event_dev_close(eventdev->data->dev_id);
> +               if (ret < 0)
> +                       return ret;
> +       }
>
>         /* Invoke PMD device un-init function */
>         if (devuninit)
> --
> 2.17.1
>

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

* Re: [dpdk-dev] [PATCH] eventdev: fix device probe and remove for secondary process
  2020-05-01 13:26 ` Jerin Jacob
@ 2020-05-02 10:31   ` Jerin Jacob
  2020-05-02 12:07     ` Varghese, Vipin
  0 siblings, 1 reply; 8+ messages in thread
From: Jerin Jacob @ 2020-05-02 10:31 UTC (permalink / raw)
  To: Pavan Nikhilesh; +Cc: Jerin Jacob, Anatoly Burakov, dpdk-dev

On Fri, May 1, 2020 at 6:56 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
>
> On Mon, Apr 27, 2020 at 11:40 PM <pbhagavatula@marvell.com> wrote:
> >
> > From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> >
> > When probing event device in secondary process skip reinitializing
> > the device data structure as it is already done in primary process.
> >
> > When removing event device in secondary process skip closing the
> > event device as it should be done by primary process.
> >
> > Fixes: 322d0345c2bc ("eventdev: implement PMD registration functions")
>
> Cc: stable@dpdk.org
>
> >
> > Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
>
>
> Acked-by: Jerin Jacob <jerinj@marvell.com>


Applied to dpdk-next-eventdev/master. Thanks.


>
>
>
> > ---
> >  lib/librte_eventdev/rte_eventdev.c         | 13 ++++++++-----
> >  lib/librte_eventdev/rte_eventdev_pmd_pci.h |  8 +++++---
> >  2 files changed, 13 insertions(+), 8 deletions(-)
> >
> > diff --git a/lib/librte_eventdev/rte_eventdev.c b/lib/librte_eventdev/rte_eventdev.c
> > index b987e0745..9aca7fbd5 100644
> > --- a/lib/librte_eventdev/rte_eventdev.c
> > +++ b/lib/librte_eventdev/rte_eventdev.c
> > @@ -1364,14 +1364,17 @@ rte_event_pmd_allocate(const char *name, int socket_id)
> >
> >                 eventdev->data = eventdev_data;
> >
> > -               strlcpy(eventdev->data->name, name, RTE_EVENTDEV_NAME_MAX_LEN);
> > +               if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
> >
> > -               eventdev->data->dev_id = dev_id;
> > -               eventdev->data->socket_id = socket_id;
> > -               eventdev->data->dev_started = 0;
> > +                       strlcpy(eventdev->data->name, name,
> > +                               RTE_EVENTDEV_NAME_MAX_LEN);
> >
> > -               eventdev->attached = RTE_EVENTDEV_ATTACHED;
> > +                       eventdev->data->dev_id = dev_id;
> > +                       eventdev->data->socket_id = socket_id;
> > +                       eventdev->data->dev_started = 0;
> > +               }
> >
> > +               eventdev->attached = RTE_EVENTDEV_ATTACHED;
> >                 eventdev_globals.nb_devs++;
> >         }
> >
> > diff --git a/lib/librte_eventdev/rte_eventdev_pmd_pci.h b/lib/librte_eventdev/rte_eventdev_pmd_pci.h
> > index 8fb61386f..443cd38c2 100644
> > --- a/lib/librte_eventdev/rte_eventdev_pmd_pci.h
> > +++ b/lib/librte_eventdev/rte_eventdev_pmd_pci.h
> > @@ -112,9 +112,11 @@ rte_event_pmd_pci_remove(struct rte_pci_device *pci_dev,
> >         if (eventdev == NULL)
> >                 return -ENODEV;
> >
> > -       ret = rte_event_dev_close(eventdev->data->dev_id);
> > -       if (ret < 0)
> > -               return ret;
> > +       if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
> > +               ret = rte_event_dev_close(eventdev->data->dev_id);
> > +               if (ret < 0)
> > +                       return ret;
> > +       }
> >
> >         /* Invoke PMD device un-init function */
> >         if (devuninit)
> > --
> > 2.17.1
> >

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

* Re: [dpdk-dev] [PATCH] eventdev: fix device probe and remove for secondary process
  2020-05-02 10:31   ` Jerin Jacob
@ 2020-05-02 12:07     ` Varghese, Vipin
  2020-05-02 13:04       ` Pavan Nikhilesh Bhagavatula
  0 siblings, 1 reply; 8+ messages in thread
From: Varghese, Vipin @ 2020-05-02 12:07 UTC (permalink / raw)
  To: Jerin Jacob, Pavan Nikhilesh; +Cc: Jerin Jacob, Burakov, Anatoly, dpdk-dev

Hi Pavan,

snipped
> > >
> > > When probing event device in secondary process skip reinitializing
> > > the device data structure as it is already done in primary process.
> > >
> > > When removing event device in secondary process skip closing the
> > > event device as it should be done by primary process.
If primary has crashed or closed before secondary abnormally. Should not close of secondary trigger removal of Eventdev services?

> > >
> > > Fixes: 322d0345c2bc ("eventdev: implement PMD registration
> > > functions")
> >
> > Cc: stable@dpdk.org
> >
> > >
> > > Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> >
> >
> > Acked-by: Jerin Jacob <jerinj@marvell.com>
> 
> 
> Applied to dpdk-next-eventdev/master. Thanks.
> 
> 
> >
> >
snipped

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

* Re: [dpdk-dev] [PATCH] eventdev: fix device probe and remove for secondary process
  2020-05-02 12:07     ` Varghese, Vipin
@ 2020-05-02 13:04       ` Pavan Nikhilesh Bhagavatula
  2020-05-03  1:15         ` Varghese, Vipin
  0 siblings, 1 reply; 8+ messages in thread
From: Pavan Nikhilesh Bhagavatula @ 2020-05-02 13:04 UTC (permalink / raw)
  To: Varghese, Vipin, Jerin Jacob
  Cc: Jerin Jacob Kollanukkaran, Burakov, Anatoly, dpdk-dev

Hi Vipin,

>Hi Pavan,
>
>snipped
>> > >
>> > > When probing event device in secondary process skip reinitializing
>> > > the device data structure as it is already done in primary process.
>> > >
>> > > When removing event device in secondary process skip closing the
>> > > event device as it should be done by primary process.
>If primary has crashed or closed before secondary abnormally. Should
>not close of secondary trigger removal of Eventdev services?

Closing event device on exit of one secondary doesn’t make sense as
there might be systems where there might be one primary and multiple
secondaries and secondaries are spawned/destroyed on demand.

Behavior of secondaries on primary process crash is undefined.


>
>> > >
>> > > Fixes: 322d0345c2bc ("eventdev: implement PMD registration
>> > > functions")
>> >
>> > Cc: stable@dpdk.org
>> >
>> > >
>> > > Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
>> >
>> >
>> > Acked-by: Jerin Jacob <jerinj@marvell.com>
>>
>>
>> Applied to dpdk-next-eventdev/master. Thanks.
>>
>>
>> >
>> >
>snipped

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

* Re: [dpdk-dev] [PATCH] eventdev: fix device probe and remove for secondary process
  2020-05-02 13:04       ` Pavan Nikhilesh Bhagavatula
@ 2020-05-03  1:15         ` Varghese, Vipin
  2020-05-03  9:58           ` Jerin Jacob
  0 siblings, 1 reply; 8+ messages in thread
From: Varghese, Vipin @ 2020-05-03  1:15 UTC (permalink / raw)
  To: Pavan Nikhilesh Bhagavatula, Jerin Jacob
  Cc: Jerin Jacob Kollanukkaran, Burakov, Anatoly, dpdk-dev

Hi Pavan,

Snipped

> >> > >
> >> > > When probing event device in secondary process skip
> >> > > reinitializing the device data structure as it is already done in primary
> process.
> >> > >
> >> > > When removing event device in secondary process skip closing the
> >> > > event device as it should be done by primary process.
> >If primary has crashed or closed before secondary abnormally. Should
> >not close of secondary trigger removal of Eventdev services?
> 
> Closing event device on exit of one secondary doesn’t make sense as there
> might be systems where there might be one primary and multiple secondaries
> and secondaries are spawned/destroyed on demand.
> 
> Behavior of secondaries on primary process crash is undefined.
Absolutely true, there are work scenarios where primary configures ports and Eventdev queues-ports pair. It would be multiple secondaries which process packets via event dev. In such cases, when primary segfaults or crashes it will lead to undefined states.

In such scenarios, would not it be preferer able for all secondaries to subscribe to service function like health check. If the primary is not alive anymore, then gracefully handle inflight events and events to dequeue. If this is right understanding, should not there be option in secondary to gracefully shut down it's worker queue and ports (rather than event device instance)?

snipped

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

* Re: [dpdk-dev] [PATCH] eventdev: fix device probe and remove for secondary process
  2020-05-03  1:15         ` Varghese, Vipin
@ 2020-05-03  9:58           ` Jerin Jacob
  2020-05-03 12:57             ` Varghese, Vipin
  0 siblings, 1 reply; 8+ messages in thread
From: Jerin Jacob @ 2020-05-03  9:58 UTC (permalink / raw)
  To: Varghese, Vipin
  Cc: Pavan Nikhilesh Bhagavatula, Jerin Jacob Kollanukkaran, Burakov,
	Anatoly, dpdk-dev

On Sun, May 3, 2020 at 6:45 AM Varghese, Vipin <vipin.varghese@intel.com> wrote:
>
> Hi Pavan,
>
> Snipped
>
> > >> > >
> > >> > > When probing event device in secondary process skip
> > >> > > reinitializing the device data structure as it is already done in primary
> > process.
> > >> > >
> > >> > > When removing event device in secondary process skip closing the
> > >> > > event device as it should be done by primary process.
> > >If primary has crashed or closed before secondary abnormally. Should
> > >not close of secondary trigger removal of Eventdev services?
> >
> > Closing event device on exit of one secondary doesn’t make sense as there
> > might be systems where there might be one primary and multiple secondaries
> > and secondaries are spawned/destroyed on demand.
> >
> > Behavior of secondaries on primary process crash is undefined.
> Absolutely true, there are work scenarios where primary configures ports and Eventdev queues-ports pair. It would be multiple secondaries which process packets via event dev. In such cases, when primary segfaults or crashes it will lead to undefined states.
>
> In such scenarios, would not it be preferer able for all secondaries to subscribe to service function like health check. If the primary is not alive anymore, then gracefully handle inflight events and events to dequeue. If this is right understanding, should not there be option in secondary to gracefully shut down it's worker queue and ports (rather than event device instance)?

The health check function may not be limited to eventdev, it must
apply for all the subsystems in multiprocess mode if primary dies.
Such features can be designed/agreed based on every subsystem in mind.
For rc2, Let's have this bug fix. New features can be implemented
after an agreement with all stakeholders wrt multi-process semantics
which applicable for all subsystems.


>
> snipped

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

* Re: [dpdk-dev] [PATCH] eventdev: fix device probe and remove for secondary process
  2020-05-03  9:58           ` Jerin Jacob
@ 2020-05-03 12:57             ` Varghese, Vipin
  0 siblings, 0 replies; 8+ messages in thread
From: Varghese, Vipin @ 2020-05-03 12:57 UTC (permalink / raw)
  To: Jerin Jacob
  Cc: Pavan Nikhilesh Bhagavatula, Jerin Jacob Kollanukkaran, Burakov,
	Anatoly, dpdk-dev

Thanks Jerin, agree with you on graceful shutdown in rc2.

> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Sunday, May 3, 2020 3:28 PM
> To: Varghese, Vipin <vipin.varghese@intel.com>
> Cc: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Jerin Jacob
> Kollanukkaran <jerinj@marvell.com>; Burakov, Anatoly
> <anatoly.burakov@intel.com>; dpdk-dev <dev@dpdk.org>
> Subject: Re: [dpdk-dev] [PATCH] eventdev: fix device probe and remove for
> secondary process
> 
> On Sun, May 3, 2020 at 6:45 AM Varghese, Vipin <vipin.varghese@intel.com>
> wrote:
> >
> > Hi Pavan,
> >
> > Snipped
> >
> > > >> > >
> > > >> > > When probing event device in secondary process skip
> > > >> > > reinitializing the device data structure as it is already
> > > >> > > done in primary
> > > process.
> > > >> > >
> > > >> > > When removing event device in secondary process skip closing
> > > >> > > the event device as it should be done by primary process.
> > > >If primary has crashed or closed before secondary abnormally.
> > > >Should not close of secondary trigger removal of Eventdev services?
> > >
> > > Closing event device on exit of one secondary doesn’t make sense as
> > > there might be systems where there might be one primary and multiple
> > > secondaries and secondaries are spawned/destroyed on demand.
> > >
> > > Behavior of secondaries on primary process crash is undefined.
> > Absolutely true, there are work scenarios where primary configures ports
> and Eventdev queues-ports pair. It would be multiple secondaries which
> process packets via event dev. In such cases, when primary segfaults or crashes
> it will lead to undefined states.
> >
> > In such scenarios, would not it be preferer able for all secondaries to
> subscribe to service function like health check. If the primary is not alive
> anymore, then gracefully handle inflight events and events to dequeue. If this
> is right understanding, should not there be option in secondary to gracefully
> shut down it's worker queue and ports (rather than event device instance)?
> 
> The health check function may not be limited to eventdev, it must apply for all
> the subsystems in multiprocess mode if primary dies.
> Such features can be designed/agreed based on every subsystem in mind.
> For rc2, Let's have this bug fix. New features can be implemented after an
> agreement with all stakeholders wrt multi-process semantics which applicable
> for all subsystems.
> 
> 
> >
> > snipped

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

end of thread, other threads:[~2020-05-03 12:57 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-27 18:10 [dpdk-dev] [PATCH] eventdev: fix device probe and remove for secondary process pbhagavatula
2020-05-01 13:26 ` Jerin Jacob
2020-05-02 10:31   ` Jerin Jacob
2020-05-02 12:07     ` Varghese, Vipin
2020-05-02 13:04       ` Pavan Nikhilesh Bhagavatula
2020-05-03  1:15         ` Varghese, Vipin
2020-05-03  9:58           ` Jerin Jacob
2020-05-03 12:57             ` Varghese, Vipin

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