diff mbox

[dpdk-dev,v1] doc/eth: update document for functional limitation

Message ID 1527069959-37765-1-git-send-email-vipin.varghese@intel.com (mailing list archive)
State Changes Requested, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Varghese, Vipin May 23, 2018, 10:05 a.m. UTC 
  ETH APIs dev_attach and dev_dettach can be exercised from primary
process only. Secondary process only maps the resource and does
not have access to internal device list. Hence updating documentation
to reflect the same.

Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>
---
 lib/librte_ethdev/rte_ethdev.h | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Thomas Monjalon May 24, 2018, 5:06 p.m. UTC | #1 
23/05/2018 12:05, Vipin Varghese:
> --- a/lib/librte_ethdev/rte_ethdev.h
> +++ b/lib/librte_ethdev/rte_ethdev.h
> @@ -1479,6 +1479,9 @@ int rte_eth_dev_attach(const char *devargs, uint16_t *port_id);
>   * This function must be called when the device is in the
>   * closed state.
>   *
> + * Note:
> + * - Currently supported for primary process only.

I think it should be possible to attach a device in secondary process.
If it is a recent limitation, it should be in "known issues" of the release notes.
Qi Zhang May 25, 2018, 12:44 a.m. UTC | #2 
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Friday, May 25, 2018 1:06 AM
> To: Varghese, Vipin <vipin.varghese@intel.com>
> Cc: dev@dpdk.org; Zhang, Qi Z <qi.z.zhang@intel.com>; Kovacevic, Marko
> <marko.kovacevic@intel.com>; Jain, Deepak K <deepak.k.jain@intel.com>;
> Yigit, Ferruh <ferruh.yigit@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v1] doc/eth: update document for functional
> limitation
> 
> 23/05/2018 12:05, Vipin Varghese:
> > --- a/lib/librte_ethdev/rte_ethdev.h
> > +++ b/lib/librte_ethdev/rte_ethdev.h
> > @@ -1479,6 +1479,9 @@ int rte_eth_dev_attach(const char *devargs,
> uint16_t *port_id);
> >   * This function must be called when the device is in the
> >   * closed state.
> >   *
> > + * Note:
> > + * - Currently supported for primary process only.
> 
> I think it should be possible to attach a device in secondary process.
> If it is a recent limitation, it should be in "known issues" of the release notes.

For PCI device, we can only attached a device that is already resource mapped in primary process (for example, a device not in secondary process' white list but in primary process's)
And we should not detached a device in secondary process, that will mess primary process and cause it can't be attached again.

For vdev, I think we still can attached/detach a new device which does not exist in primary process.

> 
>
Ferruh Yigit May 25, 2018, 8:34 a.m. UTC | #3 
On 5/25/2018 1:44 AM, Zhang, Qi Z wrote:
> 
> 
>> -----Original Message-----
>> From: Thomas Monjalon [mailto:thomas@monjalon.net]
>> Sent: Friday, May 25, 2018 1:06 AM
>> To: Varghese, Vipin <vipin.varghese@intel.com>
>> Cc: dev@dpdk.org; Zhang, Qi Z <qi.z.zhang@intel.com>; Kovacevic, Marko
>> <marko.kovacevic@intel.com>; Jain, Deepak K <deepak.k.jain@intel.com>;
>> Yigit, Ferruh <ferruh.yigit@intel.com>
>> Subject: Re: [dpdk-dev] [PATCH v1] doc/eth: update document for functional
>> limitation
>>
>> 23/05/2018 12:05, Vipin Varghese:
>>> --- a/lib/librte_ethdev/rte_ethdev.h
>>> +++ b/lib/librte_ethdev/rte_ethdev.h
>>> @@ -1479,6 +1479,9 @@ int rte_eth_dev_attach(const char *devargs,
>> uint16_t *port_id);
>>>   * This function must be called when the device is in the
>>>   * closed state.
>>>   *
>>> + * Note:
>>> + * - Currently supported for primary process only.
>>
>> I think it should be possible to attach a device in secondary process.
>> If it is a recent limitation, it should be in "known issues" of the release notes.
> 
> For PCI device, we can only attached a device that is already resource mapped in primary process (for example, a device not in secondary process' white list but in primary process's)
> And we should not detached a device in secondary process, that will mess primary process and cause it can't be attached again.
> 
> For vdev, I think we still can attached/detach a new device which does not exist in primary process.

For vdev it was possible to attach a new device in secondary, but it seems
primary process checks has been added to virtual PMDs probe() function by [1]
which seems breaking this capability, can you please check it?


[1]
Fixes: ee27edbe0c10 ("drivers/net: share vdev data to secondary process")

> 
>>
>>
>
Qi Zhang May 25, 2018, 9:08 a.m. UTC | #4 
> -----Original Message-----

> From: Yigit, Ferruh

> Sent: Friday, May 25, 2018 4:35 PM

> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Thomas Monjalon

> <thomas@monjalon.net>; Varghese, Vipin <vipin.varghese@intel.com>

> Cc: dev@dpdk.org; Kovacevic, Marko <marko.kovacevic@intel.com>; Jain,

> Deepak K <deepak.k.jain@intel.com>

> Subject: Re: [dpdk-dev] [PATCH v1] doc/eth: update document for functional

> limitation

> 

> On 5/25/2018 1:44 AM, Zhang, Qi Z wrote:

> >

> >

> >> -----Original Message-----

> >> From: Thomas Monjalon [mailto:thomas@monjalon.net]

> >> Sent: Friday, May 25, 2018 1:06 AM

> >> To: Varghese, Vipin <vipin.varghese@intel.com>

> >> Cc: dev@dpdk.org; Zhang, Qi Z <qi.z.zhang@intel.com>; Kovacevic,

> >> Marko <marko.kovacevic@intel.com>; Jain, Deepak K

> >> <deepak.k.jain@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>

> >> Subject: Re: [dpdk-dev] [PATCH v1] doc/eth: update document for

> >> functional limitation

> >>

> >> 23/05/2018 12:05, Vipin Varghese:

> >>> --- a/lib/librte_ethdev/rte_ethdev.h

> >>> +++ b/lib/librte_ethdev/rte_ethdev.h

> >>> @@ -1479,6 +1479,9 @@ int rte_eth_dev_attach(const char *devargs,

> >> uint16_t *port_id);

> >>>   * This function must be called when the device is in the

> >>>   * closed state.

> >>>   *

> >>> + * Note:

> >>> + * - Currently supported for primary process only.

> >>

> >> I think it should be possible to attach a device in secondary process.

> >> If it is a recent limitation, it should be in "known issues" of the release

> notes.

> >

> > For PCI device, we can only attached a device that is already resource

> > mapped in primary process (for example, a device not in secondary process'

> white list but in primary process's) And we should not detached a device in

> secondary process, that will mess primary process and cause it can't be

> attached again.

> >

> > For vdev, I think we still can attached/detach a new device which does not

> exist in primary process.

> 

> For vdev it was possible to attach a new device in secondary, but it seems

> primary process checks has been added to virtual PMDs probe() function by [1]

> which seems breaking this capability, can you please check it?


Yes, attach vdev "net_af_packet,iface=eth0" on secondary process will create a private device.
> 

> 

> [1]

> Fixes: ee27edbe0c10 ("drivers/net: share vdev data to secondary process")

> 

> >

> >>

> >>

> >
Ferruh Yigit May 25, 2018, 9:12 a.m. UTC | #5 
On 5/25/2018 10:08 AM, Zhang, Qi Z wrote:
> 
> 
>> -----Original Message-----
>> From: Yigit, Ferruh
>> Sent: Friday, May 25, 2018 4:35 PM
>> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Thomas Monjalon
>> <thomas@monjalon.net>; Varghese, Vipin <vipin.varghese@intel.com>
>> Cc: dev@dpdk.org; Kovacevic, Marko <marko.kovacevic@intel.com>; Jain,
>> Deepak K <deepak.k.jain@intel.com>
>> Subject: Re: [dpdk-dev] [PATCH v1] doc/eth: update document for functional
>> limitation
>>
>> On 5/25/2018 1:44 AM, Zhang, Qi Z wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Thomas Monjalon [mailto:thomas@monjalon.net]
>>>> Sent: Friday, May 25, 2018 1:06 AM
>>>> To: Varghese, Vipin <vipin.varghese@intel.com>
>>>> Cc: dev@dpdk.org; Zhang, Qi Z <qi.z.zhang@intel.com>; Kovacevic,
>>>> Marko <marko.kovacevic@intel.com>; Jain, Deepak K
>>>> <deepak.k.jain@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>
>>>> Subject: Re: [dpdk-dev] [PATCH v1] doc/eth: update document for
>>>> functional limitation
>>>>
>>>> 23/05/2018 12:05, Vipin Varghese:
>>>>> --- a/lib/librte_ethdev/rte_ethdev.h
>>>>> +++ b/lib/librte_ethdev/rte_ethdev.h
>>>>> @@ -1479,6 +1479,9 @@ int rte_eth_dev_attach(const char *devargs,
>>>> uint16_t *port_id);
>>>>>   * This function must be called when the device is in the
>>>>>   * closed state.
>>>>>   *
>>>>> + * Note:
>>>>> + * - Currently supported for primary process only.
>>>>
>>>> I think it should be possible to attach a device in secondary process.
>>>> If it is a recent limitation, it should be in "known issues" of the release
>> notes.
>>>
>>> For PCI device, we can only attached a device that is already resource
>>> mapped in primary process (for example, a device not in secondary process'
>> white list but in primary process's) And we should not detached a device in
>> secondary process, that will mess primary process and cause it can't be
>> attached again.
>>>
>>> For vdev, I think we still can attached/detach a new device which does not
>> exist in primary process.
>>
>> For vdev it was possible to attach a new device in secondary, but it seems
>> primary process checks has been added to virtual PMDs probe() function by [1]
>> which seems breaking this capability, can you please check it?
> 
> Yes, attach vdev "net_af_packet,iface=eth0" on secondary process will create a private device.

Yep, Qi kindly explained me that it is still possible create vdevs using
rte_eth_dev_attach() on secondary process.

So this patch is not exactly correct.

>>
>>
>> [1]
>> Fixes: ee27edbe0c10 ("drivers/net: share vdev data to secondary process")
>>
>>>
>>>>
>>>>
>>>
>
diff mbox

Patch

diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index 36e3984..6afcf9f 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -1479,6 +1479,9 @@  int rte_eth_dev_attach(const char *devargs, uint16_t *port_id);
  * This function must be called when the device is in the
  * closed state.
  *
+ * Note:
+ * - Currently supported for primary process only.
+ *
  * @param port_id
  *   The port identifier of the device to detach.
  * @param devname
@@ -1493,6 +1496,9 @@  int rte_eth_dev_detach(uint16_t port_id, char *devname);
  * Convert a numerical speed in Mbps to a bitmap flag that can be used in
  * the bitmap link_speeds of the struct rte_eth_conf
  *
+ * Note:
+ * - Currently supported for primary process only.
+ *
  * @param speed
  *   Numerical speed value in Mbps
  * @param duplex