[dpdk-dev] [PATCH] ethdev: add field for device data per process

[dpdk-dev] [PATCH] ethdev: add field for device data per process

[Alejandro Lucero]

Primary and secondary processes share a per-device private data. With
current design it is not possible to have data per-device per-process.
This is required for handling properly the CPP interface inside the NFP
PMD with multiprocess support.

There is also at least another PMD driver, tap, with similar
requirements for per-process device data.

Signed-off-by: Alejandro Lucero <alejandro.lucero@netronome.com>
---
 lib/librte_ethdev/rte_ethdev_core.h | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/lib/librte_ethdev/rte_ethdev_core.h b/lib/librte_ethdev/rte_ethdev_core.h
index 33d12b3..9f29889 100644
--- a/lib/librte_ethdev/rte_ethdev_core.h
+++ b/lib/librte_ethdev/rte_ethdev_core.h
@@ -539,7 +539,13 @@ struct rte_eth_dev {
 	eth_rx_burst_t rx_pkt_burst; /**< Pointer to PMD receive function. */
 	eth_tx_burst_t tx_pkt_burst; /**< Pointer to PMD transmit function. */
 	eth_tx_prep_t tx_pkt_prepare; /**< Pointer to PMD transmit prepare function. */
-	struct rte_eth_dev_data *data;  /**< Pointer to device data */
+	/**
+	 * Next two fields are per-device data but *data is shared between
+	 * primary and secondary processes and *process_private is per-process
+	 * private.
+	 */
+	struct rte_eth_dev_data *data;  /**< Pointer to device data. */
+	void *process_private; /**< Pointer to per-process device data. */
 	const struct eth_dev_ops *dev_ops; /**< Functions exported by PMD */
 	struct rte_device *device; /**< Backing device */
 	struct rte_intr_handle *intr_handle; /**< Device interrupt handle */
-- 
1.9.1

Re: [dpdk-dev] [PATCH] ethdev: add field for device data per process

[Thomas Monjalon]

+ Cc more people

27/09/2018 13:26, Alejandro Lucero:
> Primary and secondary processes share a per-device private data. With
> current design it is not possible to have data per-device per-process.
> This is required for handling properly the CPP interface inside the NFP
> PMD with multiprocess support.
> 
> There is also at least another PMD driver, tap, with similar
> requirements for per-process device data.

Yes, it is required to fix tap PMD for multi-process usage.

I am in favor of accepting this change in 18.11.

[...]
> @@ -539,7 +539,13 @@ struct rte_eth_dev {
>  	eth_rx_burst_t rx_pkt_burst; /**< Pointer to PMD receive function. */
>  	eth_tx_burst_t tx_pkt_burst; /**< Pointer to PMD transmit function. */
>  	eth_tx_prep_t tx_pkt_prepare; /**< Pointer to PMD transmit prepare function. */
> -	struct rte_eth_dev_data *data;  /**< Pointer to device data */
> +	/**
> +	 * Next two fields are per-device data but *data is shared between

All fields in rte_eth_dev are per-device.

> +	 * primary and secondary processes and *process_private is per-process
> +	 * private.
> +	 */
> +	struct rte_eth_dev_data *data;  /**< Pointer to device data. */
> +	void *process_private; /**< Pointer to per-process device data. */

We could explain here that this memory is allocated by the PMD.

Re: [dpdk-dev] [PATCH] ethdev: add field for device data per process

[Ferruh Yigit]

On 10/3/2018 9:44 PM, Thomas Monjalon wrote:
> + Cc more people
> 
> 27/09/2018 13:26, Alejandro Lucero:
>> Primary and secondary processes share a per-device private data. With
>> current design it is not possible to have data per-device per-process.
>> This is required for handling properly the CPP interface inside the NFP
>> PMD with multiprocess support.
>>
>> There is also at least another PMD driver, tap, with similar
>> requirements for per-process device data.
> 
> Yes, it is required to fix tap PMD for multi-process usage.
> 
> I am in favor of accepting this change in 18.11.

+1

> 
> [...]
>> @@ -539,7 +539,13 @@ struct rte_eth_dev {
>>  	eth_rx_burst_t rx_pkt_burst; /**< Pointer to PMD receive function. */
>>  	eth_tx_burst_t tx_pkt_burst; /**< Pointer to PMD transmit function. */
>>  	eth_tx_prep_t tx_pkt_prepare; /**< Pointer to PMD transmit prepare function. */
>> -	struct rte_eth_dev_data *data;  /**< Pointer to device data */
>> +	/**
>> +	 * Next two fields are per-device data but *data is shared between
> 
> All fields in rte_eth_dev are per-device.
> 
>> +	 * primary and secondary processes and *process_private is per-process
>> +	 * private.
>> +	 */
>> +	struct rte_eth_dev_data *data;  /**< Pointer to device data. */
>> +	void *process_private; /**< Pointer to per-process device data. */
> 
> We could explain here that this memory is allocated by the PMD.
> 
> 

Re: [dpdk-dev] [PATCH] ethdev: add field for device data per process

[Ferruh Yigit]

On 10/3/2018 9:44 PM, Thomas Monjalon wrote:
> + Cc more people
> 
> 27/09/2018 13:26, Alejandro Lucero:
>> Primary and secondary processes share a per-device private data. With
>> current design it is not possible to have data per-device per-process.
>> This is required for handling properly the CPP interface inside the NFP
>> PMD with multiprocess support.
>>
>> There is also at least another PMD driver, tap, with similar
>> requirements for per-process device data.
> 
> Yes, it is required to fix tap PMD for multi-process usage.
> 
> I am in favor of accepting this change in 18.11.
> 
> [...]
>> @@ -539,7 +539,13 @@ struct rte_eth_dev {
>>  	eth_rx_burst_t rx_pkt_burst; /**< Pointer to PMD receive function. */
>>  	eth_tx_burst_t tx_pkt_burst; /**< Pointer to PMD transmit function. */
>>  	eth_tx_prep_t tx_pkt_prepare; /**< Pointer to PMD transmit prepare function. */
>> -	struct rte_eth_dev_data *data;  /**< Pointer to device data */
>> +	/**
>> +	 * Next two fields are per-device data but *data is shared between
> 
> All fields in rte_eth_dev are per-device.
> 
>> +	 * primary and secondary processes and *process_private is per-process
>> +	 * private.
>> +	 */
>> +	struct rte_eth_dev_data *data;  /**< Pointer to device data. */
>> +	void *process_private; /**< Pointer to per-process device data. */
> 
> We could explain here that this memory is allocated by the PMD.

Will there be new version?

Are we agree on name?

Is LIBABIVER increase should be done in this patch, or will there be other patch
already doing it?

Re: [dpdk-dev] [PATCH] ethdev: add field for device data per process

[Alejandro Lucero]

On Fri, Oct 5, 2018 at 2:01 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:

> On 10/3/2018 9:44 PM, Thomas Monjalon wrote:
> > + Cc more people
> >
> > 27/09/2018 13:26, Alejandro Lucero:
> >> Primary and secondary processes share a per-device private data. With
> >> current design it is not possible to have data per-device per-process.
> >> This is required for handling properly the CPP interface inside the NFP
> >> PMD with multiprocess support.
> >>
> >> There is also at least another PMD driver, tap, with similar
> >> requirements for per-process device data.
> >
> > Yes, it is required to fix tap PMD for multi-process usage.
> >
> > I am in favor of accepting this change in 18.11.
> >
> > [...]
> >> @@ -539,7 +539,13 @@ struct rte_eth_dev {
> >>      eth_rx_burst_t rx_pkt_burst; /**< Pointer to PMD receive function.
> */
> >>      eth_tx_burst_t tx_pkt_burst; /**< Pointer to PMD transmit
> function. */
> >>      eth_tx_prep_t tx_pkt_prepare; /**< Pointer to PMD transmit prepare
> function. */
> >> -    struct rte_eth_dev_data *data;  /**< Pointer to device data */
> >> +    /**
> >> +     * Next two fields are per-device data but *data is shared between
> >
> > All fields in rte_eth_dev are per-device.
> >
> >> +     * primary and secondary processes and *process_private is
> per-process
> >> +     * private.
> >> +     */
> >> +    struct rte_eth_dev_data *data;  /**< Pointer to device data. */
> >> +    void *process_private; /**< Pointer to per-process device data. */
> >
> > We could explain here that this memory is allocated by the PMD.
>
> Will there be new version?
>
> Are we agree on name?
>
> Is LIBABIVER increase should be done in this patch, or will there be other
> patch
> already doing it?
>

I'm not familiar with LIBABIVER but just tell me to send it again with that
change if you consider that is the right thing to do.
About the name, I will let other to tell.

Thanks

Re: [dpdk-dev] [PATCH] ethdev: add field for device data per process

[Ferruh Yigit]

On 10/5/2018 2:17 PM, Alejandro Lucero wrote:
> On Fri, Oct 5, 2018 at 2:01 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> 
>> On 10/3/2018 9:44 PM, Thomas Monjalon wrote:
>>> + Cc more people
>>>
>>> 27/09/2018 13:26, Alejandro Lucero:
>>>> Primary and secondary processes share a per-device private data. With
>>>> current design it is not possible to have data per-device per-process.
>>>> This is required for handling properly the CPP interface inside the NFP
>>>> PMD with multiprocess support.
>>>>
>>>> There is also at least another PMD driver, tap, with similar
>>>> requirements for per-process device data.
>>>
>>> Yes, it is required to fix tap PMD for multi-process usage.
>>>
>>> I am in favor of accepting this change in 18.11.
>>>
>>> [...]
>>>> @@ -539,7 +539,13 @@ struct rte_eth_dev {
>>>>      eth_rx_burst_t rx_pkt_burst; /**< Pointer to PMD receive function.
>> */
>>>>      eth_tx_burst_t tx_pkt_burst; /**< Pointer to PMD transmit
>> function. */
>>>>      eth_tx_prep_t tx_pkt_prepare; /**< Pointer to PMD transmit prepare
>> function. */
>>>> -    struct rte_eth_dev_data *data;  /**< Pointer to device data */
>>>> +    /**
>>>> +     * Next two fields are per-device data but *data is shared between
>>>
>>> All fields in rte_eth_dev are per-device.
>>>
>>>> +     * primary and secondary processes and *process_private is
>> per-process
>>>> +     * private.
>>>> +     */
>>>> +    struct rte_eth_dev_data *data;  /**< Pointer to device data. */
>>>> +    void *process_private; /**< Pointer to per-process device data. */
>>>
>>> We could explain here that this memory is allocated by the PMD.
>>
>> Will there be new version?
>>
>> Are we agree on name?
>>
>> Is LIBABIVER increase should be done in this patch, or will there be other
>> patch
>> already doing it?
>>
> 
> I'm not familiar with LIBABIVER but just tell me to send it again with that
> change if you consider that is the right thing to do.

ABI breakage process:
- Increase LIBABIVER in library Makefile/meson.build
- Update lib in release notes "Shared Library Versions" section, with a "+" to
  to indicate change
- Remove deprecation notice (seems not applies to this one)

Thomas mentioned there is another patch breaking the ABI for ethdev, I wonder
which patch will do the above process.

> About the name, I will let other to tell.
> 
> Thanks
> 

Re: [dpdk-dev] [PATCH] ethdev: add field for device data per process

[Thomas Monjalon]

05/10/2018 15:26, Ferruh Yigit:
> On 10/5/2018 2:17 PM, Alejandro Lucero wrote:
> > On Fri, Oct 5, 2018 at 2:01 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> >> Will there be new version?
> >>
> >> Are we agree on name?
> >>
> >> Is LIBABIVER increase should be done in this patch, or will there be other
> >> patch
> >> already doing it?
> >>
> > 
> > I'm not familiar with LIBABIVER but just tell me to send it again with that
> > change if you consider that is the right thing to do.
> 
> ABI breakage process:
> - Increase LIBABIVER in library Makefile/meson.build
> - Update lib in release notes "Shared Library Versions" section, with a "+" to
>   to indicate change
> - Remove deprecation notice (seems not applies to this one)
> 
> Thomas mentioned there is another patch breaking the ABI for ethdev, I wonder
> which patch will do the above process.

There will be a patch to remove the attach/detach function.
But the patch for data per process will probably be applied first.
Please do the LIBABIVER bump as described by Ferruh.