[dpdk-dev] secondary processes and private data

[dpdk-dev] secondary processes and private data

[Alejandro Lucero]

I've a problem when part of device private data needs to be private per
process.

Current multiprocess support shares device private data between primary and
secondaries but it is all dependent on a pointer initialized to the same
memory address by the multiprocess support code. If there is a per-process
data, If a secondary process changes it the primary gets affected, and the
same for additional secondaries which will affect not just the primary but
other previous secondaries.

The solution is to add support for this inside struct rte_eth_dev,
something like

void *secondary_priv_data;

so it is up to the secondaries to use this field if necessary.

NFP PMD creates the required rte_eth_devs specifically, similar to what is
done inside rte_ethdev.c but adding initialization for an interface needed
when calling device ethdev_init function. There are other PMDs doing this
but none has this requirement for per-process private data.

Please, let me know what you think about this change to struct rte_ethdev
or if you have a better idea for solving this problem.

Thanks

Re: [dpdk-dev] secondary processes and private data

[Burakov, Anatoly]

On 25-Sep-18 3:10 PM, Alejandro Lucero wrote:
> I've a problem when part of device private data needs to be private per 
> process.
> 
> Current multiprocess support shares device private data between primary 
> and secondaries but it is all dependent on a pointer initialized to the 
> same memory address by the multiprocess support code. If there is a 
> per-process data, If a secondary process changes it the primary gets 
> affected, and the same for additional secondaries which will affect not 
> just the primary but other previous secondaries.
> 
> The solution is to add support for this inside struct rte_eth_dev, 
> something like
> 
> void *secondary_priv_data;
> 
> so it is up to the secondaries to use this field if necessary.
> 
> NFP PMD creates the required rte_eth_devs specifically, similar to what 
> is done inside rte_ethdev.c but adding initialization for an interface 
> needed when calling device ethdev_init function. There are other PMDs 
> doing this but none has this requirement for per-process private data.
> 
> Please, let me know what you think about this change to struct 
> rte_ethdev or if you have a better idea for solving this problem.
> 
> Thanks
> 

Perhaps "private data" is a bad name for data that is shared across 
primary and secondary processes...

I don't have anything against the idea.

-- 
Thanks,
Anatoly

Re: [dpdk-dev] secondary processes and private data

[Thomas Monjalon]

Hi Alejandro,

25/09/2018 16:10, Alejandro Lucero:
> I've a problem when part of device private data needs to be private per
> process.

It appears we are facing the same issue to support multi-process in tap.

> Current multiprocess support shares device private data between primary and
> secondaries but it is all dependent on a pointer initialized to the same
> memory address by the multiprocess support code. If there is a per-process
> data, If a secondary process changes it the primary gets affected, and the
> same for additional secondaries which will affect not just the primary but
> other previous secondaries.

Yes, the field rte_eth_dev.data.dev_private is private to the device,
but shared between processes.

> The solution is to add support for this inside struct rte_eth_dev,
> something like
> 
> void *secondary_priv_data;
> 
> so it is up to the secondaries to use this field if necessary.

I would say it is not only for secondary process.
What about this name:

	rte_eth_dev.process_private

> NFP PMD creates the required rte_eth_devs specifically, similar to what is
> done inside rte_ethdev.c but adding initialization for an interface needed
> when calling device ethdev_init function. There are other PMDs doing this
> but none has this requirement for per-process private data.

Actually tap has a per-process requirement for its file descriptors.

> Please, let me know what you think about this change to struct rte_ethdev
> or if you have a better idea for solving this problem.

I support the idea, but we need to agree on name bikeshedding :-)

Re: [dpdk-dev] secondary processes and private data

[Alejandro Lucero]

Yes, I agree.

On Wed, Sep 26, 2018 at 2:21 PM Thomas Monjalon <thomas@monjalon.net> wrote:

> Hi Alejandro,
>
> 25/09/2018 16:10, Alejandro Lucero:
> > I've a problem when part of device private data needs to be private per
> > process.
>
> It appears we are facing the same issue to support multi-process in tap.
>
> > Current multiprocess support shares device private data between primary
> and
> > secondaries but it is all dependent on a pointer initialized to the same
> > memory address by the multiprocess support code. If there is a
> per-process
> > data, If a secondary process changes it the primary gets affected, and
> the
> > same for additional secondaries which will affect not just the primary
> but
> > other previous secondaries.
>
> Yes, the field rte_eth_dev.data.dev_private is private to the device,
> but shared between processes.
>
> > The solution is to add support for this inside struct rte_eth_dev,
> > something like
> >
> > void *secondary_priv_data;
> >
> > so it is up to the secondaries to use this field if necessary.
>
> I would say it is not only for secondary process.
> What about this name:
>
>         rte_eth_dev.process_private
>
> > NFP PMD creates the required rte_eth_devs specifically, similar to what
> is
> > done inside rte_ethdev.c but adding initialization for an interface
> needed
> > when calling device ethdev_init function. There are other PMDs doing this
> > but none has this requirement for per-process private data.
>
> Actually tap has a per-process requirement for its file descriptors.
>
> > Please, let me know what you think about this change to struct rte_ethdev
> > or if you have a better idea for solving this problem.
>
> I support the idea, but we need to agree on name bikeshedding :-)
>
>
>

Re: [dpdk-dev] secondary processes and private data

[Stephen Hemminger]

On Wed, 26 Sep 2018 15:21:52 +0200
Thomas Monjalon <thomas@monjalon.net> wrote:

> Hi Alejandro,
> 
> 25/09/2018 16:10, Alejandro Lucero:
> > I've a problem when part of device private data needs to be private per
> > process.  
> 
> It appears we are facing the same issue to support multi-process in tap.
> 
> > Current multiprocess support shares device private data between primary and
> > secondaries but it is all dependent on a pointer initialized to the same
> > memory address by the multiprocess support code. If there is a per-process
> > data, If a secondary process changes it the primary gets affected, and the
> > same for additional secondaries which will affect not just the primary but
> > other previous secondaries.  
> 
> Yes, the field rte_eth_dev.data.dev_private is private to the device,
> but shared between processes.
> 
> > The solution is to add support for this inside struct rte_eth_dev,
> > something like
> > 
> > void *secondary_priv_data;
> > 
> > so it is up to the secondaries to use this field if necessary.  
> 
> I would say it is not only for secondary process.
> What about this name:
> 
> 	rte_eth_dev.process_private
> 
> > NFP PMD creates the required rte_eth_devs specifically, similar to what is
> > done inside rte_ethdev.c but adding initialization for an interface needed
> > when calling device ethdev_init function. There are other PMDs doing this
> > but none has this requirement for per-process private data.  
> 
> Actually tap has a per-process requirement for its file descriptors.
> 
> > Please, let me know what you think about this change to struct rte_ethdev
> > or if you have a better idea for solving this problem.  
> 
> I support the idea, but we need to agree on name bikeshedding :-)

Good idea, as long as it stays contained to DPDK. Don't want additional user API
pointers buried in internal structures (like ethdev).  If application needs device
private data it should manage its own state. 

Re: [dpdk-dev] secondary processes and private data

[Alejandro Lucero]

On Wed, Sep 26, 2018 at 3:33 PM Stephen Hemminger <
stephen@networkplumber.org> wrote:

> On Wed, 26 Sep 2018 15:21:52 +0200
> Thomas Monjalon <thomas@monjalon.net> wrote:
>
> > Hi Alejandro,
> >
> > 25/09/2018 16:10, Alejandro Lucero:
> > > I've a problem when part of device private data needs to be private per
> > > process.
> >
> > It appears we are facing the same issue to support multi-process in tap.
> >
> > > Current multiprocess support shares device private data between
> primary and
> > > secondaries but it is all dependent on a pointer initialized to the
> same
> > > memory address by the multiprocess support code. If there is a
> per-process
> > > data, If a secondary process changes it the primary gets affected, and
> the
> > > same for additional secondaries which will affect not just the primary
> but
> > > other previous secondaries.
> >
> > Yes, the field rte_eth_dev.data.dev_private is private to the device,
> > but shared between processes.
> >
> > > The solution is to add support for this inside struct rte_eth_dev,
> > > something like
> > >
> > > void *secondary_priv_data;
> > >
> > > so it is up to the secondaries to use this field if necessary.
> >
> > I would say it is not only for secondary process.
> > What about this name:
> >
> >       rte_eth_dev.process_private
> >
> > > NFP PMD creates the required rte_eth_devs specifically, similar to
> what is
> > > done inside rte_ethdev.c but adding initialization for an interface
> needed
> > > when calling device ethdev_init function. There are other PMDs doing
> this
> > > but none has this requirement for per-process private data.
> >
> > Actually tap has a per-process requirement for its file descriptors.
> >
> > > Please, let me know what you think about this change to struct
> rte_ethdev
> > > or if you have a better idea for solving this problem.
> >
> > I support the idea, but we need to agree on name bikeshedding :-)
>
> Good idea, as long as it stays contained to DPDK. Don't want additional
> user API
> pointers buried in internal structures (like ethdev).  If application
> needs device
> private data it should manage its own state.
>
>
AFAIK the per-device data is not used by apps but only PMDs, and the new
field should be the same.