diff mbox series

net/e1000: fix port hotplug for multi-process

Message ID 20200429063724.17284-1-alvinx.zhang@intel.com (mailing list archive)
State Accepted, archived
Delegated to: xiaolong ye
Headers
Series net/e1000: fix port hotplug for multi-process |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-nxp-Performance success Performance Testing PASS
ci/travis-robot warning Travis build: failed
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-testing success Testing PASS
ci/Intel-compilation success Compilation OK

Commit Message

Alvin Zhang April 29, 2020, 6:37 a.m. UTC 
  From: Alvin Zhang <alvinx.zhang@intel.com>

Enable detach device on secondary process.

Fixes: b9eee2cb8c29 (e1000: support port hotplug)
Cc: bernard.iremonger@intel.com
Cc: stable@dpdk.org

Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com>
---
 drivers/net/e1000/em_ethdev.c  | 2 +-
 drivers/net/e1000/igb_ethdev.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

Comments

Mei, JianweiX May 8, 2020, 3:32 a.m. UTC | #1 
Tested-by: Mei Jianwei <jianweix.mei@intel.com>

-----Original Message-----
From: Jiang, YuX 
Sent: Thursday, May 7, 2020 4:45 PM
To: Mei, JianweiX <jianweix.mei@intel.com>
Subject: FW: [dpdk-dev] [PATCH] net/e1000: fix port hotplug for multi-process



-----Original Message-----
From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of alvinx.zhang@intel.com
Sent: Wednesday, April 29, 2020 2:37 PM
To: dev@dpdk.org
Cc: Xing, Beilei <beilei.xing@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>
Subject: [dpdk-dev] [PATCH] net/e1000: fix port hotplug for multi-process

From: Alvin Zhang <alvinx.zhang@intel.com>

Enable detach device on secondary process.

Fixes: b9eee2cb8c29 (e1000: support port hotplug)
Cc: bernard.iremonger@intel.com
Cc: stable@dpdk.org

Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com>
---
 drivers/net/e1000/em_ethdev.c  | 2 +-
 drivers/net/e1000/igb_ethdev.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c index 188cda3..902b1cd 100644
--- a/drivers/net/e1000/em_ethdev.c
+++ b/drivers/net/e1000/em_ethdev.c
@@ -321,7 +321,7 @@ static int eth_em_set_mc_addr_list(struct rte_eth_dev *dev,
 	PMD_INIT_FUNC_TRACE();
 
 	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
-		return -EPERM;
+		return 0;
 
 	eth_em_close(eth_dev);
 
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c index 520fba8..a5551e8 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -923,7 +923,7 @@ static int igb_flex_filter_uninit(struct rte_eth_dev *eth_dev)
 	PMD_INIT_FUNC_TRACE();
 
 	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
-		return -EPERM;
+		return 0;
 
 	eth_igb_close(eth_dev);
 
@@ -1044,7 +1044,7 @@ static int igb_flex_filter_uninit(struct rte_eth_dev *eth_dev)
 	PMD_INIT_FUNC_TRACE();
 
 	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
-		return -EPERM;
+		return 0;
 
 	igbvf_dev_close(eth_dev);
 
--
1.8.3.1
Guo, Jia May 12, 2020, 3:03 a.m. UTC | #2 
hi, alvin


On 4/29/2020 2:37 PM, alvinx.zhang@intel.com wrote:
> From: Alvin Zhang <alvinx.zhang@intel.com>
>
> Enable detach device on secondary process.
>
> Fixes: b9eee2cb8c29 (e1000: support port hotplug)
> Cc: bernard.iremonger@intel.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com>
> ---
>   drivers/net/e1000/em_ethdev.c  | 2 +-
>   drivers/net/e1000/igb_ethdev.c | 4 ++--
>   2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
> index 188cda3..902b1cd 100644
> --- a/drivers/net/e1000/em_ethdev.c
> +++ b/drivers/net/e1000/em_ethdev.c
> @@ -321,7 +321,7 @@ static int eth_em_set_mc_addr_list(struct rte_eth_dev *dev,
>   	PMD_INIT_FUNC_TRACE();
>   
>   	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> -		return -EPERM;
> +		return 0;
>   


I guess you mean pass through the process of the RTE_PROC_SECONDARY case 
when detach device, but what about the other case like RTE_PROC_INVALID 
or RTE_PROC_AUTO ?


>   	eth_em_close(eth_dev);
>   
> diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
> index 520fba8..a5551e8 100644
> --- a/drivers/net/e1000/igb_ethdev.c
> +++ b/drivers/net/e1000/igb_ethdev.c
> @@ -923,7 +923,7 @@ static int igb_flex_filter_uninit(struct rte_eth_dev *eth_dev)
>   	PMD_INIT_FUNC_TRACE();
>   
>   	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> -		return -EPERM;
> +		return 0;
>   
>   	eth_igb_close(eth_dev);
>   
> @@ -1044,7 +1044,7 @@ static int igb_flex_filter_uninit(struct rte_eth_dev *eth_dev)
>   	PMD_INIT_FUNC_TRACE();
>   
>   	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> -		return -EPERM;
> +		return 0;
>   
>   	igbvf_dev_close(eth_dev);
>
Alvin Zhang May 12, 2020, 3:44 a.m. UTC | #3 
Hi Jia,

It shouldn't return error for secondary. 'rte_eth_dev_release_port()' has already process type in it, so returning '0' should work better which will cause some process specific variables cleared.
In otherwise, only primary process need to really close the device.

BR,
Alvin

> -----Original Message-----
> From: Guo, Jia
> Sent: Tuesday, May 12, 2020 11:03 AM
> To: Zhang, AlvinX <alvinx.zhang@intel.com>; dev@dpdk.org
> Cc: Xing, Beilei <beilei.xing@intel.com>; Lu, Wenzhuo
> <wenzhuo.lu@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] net/e1000: fix port hotplug for multi-
> process
> 
> hi, alvin
> 
> 
> On 4/29/2020 2:37 PM, alvinx.zhang@intel.com wrote:
> > From: Alvin Zhang <alvinx.zhang@intel.com>
> >
> > Enable detach device on secondary process.
> >
> > Fixes: b9eee2cb8c29 (e1000: support port hotplug)
> > Cc: bernard.iremonger@intel.com
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com>
> > ---
> >   drivers/net/e1000/em_ethdev.c  | 2 +-
> >   drivers/net/e1000/igb_ethdev.c | 4 ++--
> >   2 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/e1000/em_ethdev.c
> > b/drivers/net/e1000/em_ethdev.c index 188cda3..902b1cd 100644
> > --- a/drivers/net/e1000/em_ethdev.c
> > +++ b/drivers/net/e1000/em_ethdev.c
> > @@ -321,7 +321,7 @@ static int eth_em_set_mc_addr_list(struct
> rte_eth_dev *dev,
> >   	PMD_INIT_FUNC_TRACE();
> >
> >   	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> > -		return -EPERM;
> > +		return 0;
> >
> 
> 
> I guess you mean pass through the process of the RTE_PROC_SECONDARY
> case when detach device, but what about the other case like
> RTE_PROC_INVALID or RTE_PROC_AUTO ?
> 
> 
> >   	eth_em_close(eth_dev);
> >
> > diff --git a/drivers/net/e1000/igb_ethdev.c
> > b/drivers/net/e1000/igb_ethdev.c index 520fba8..a5551e8 100644
> > --- a/drivers/net/e1000/igb_ethdev.c
> > +++ b/drivers/net/e1000/igb_ethdev.c
> > @@ -923,7 +923,7 @@ static int igb_flex_filter_uninit(struct rte_eth_dev
> *eth_dev)
> >   	PMD_INIT_FUNC_TRACE();
> >
> >   	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> > -		return -EPERM;
> > +		return 0;
> >
> >   	eth_igb_close(eth_dev);
> >
> > @@ -1044,7 +1044,7 @@ static int igb_flex_filter_uninit(struct rte_eth_dev
> *eth_dev)
> >   	PMD_INIT_FUNC_TRACE();
> >
> >   	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> > -		return -EPERM;
> > +		return 0;
> >
> >   	igbvf_dev_close(eth_dev);
> >
Guo, Jia May 12, 2020, 4:01 a.m. UTC | #4 
hi alvin

On 5/12/2020 11:44 AM, Zhang, AlvinX wrote:
> Hi Jia,
>
> It shouldn't return error for secondary. 'rte_eth_dev_release_port()' has already process type in it, so returning '0' should work better which will cause some process specific variables cleared.
> In otherwise, only primary process need to really close the device.
>
> BR,
> Alvin


Sounds that if driver no need to close, no need to show any useless log 
to user, make sense.


>> -----Original Message-----
>> From: Guo, Jia
>> Sent: Tuesday, May 12, 2020 11:03 AM
>> To: Zhang, AlvinX <alvinx.zhang@intel.com>; dev@dpdk.org
>> Cc: Xing, Beilei <beilei.xing@intel.com>; Lu, Wenzhuo
>> <wenzhuo.lu@intel.com>
>> Subject: Re: [dpdk-dev] [PATCH] net/e1000: fix port hotplug for multi-
>> process
>>
>> hi, alvin
>>
>>
>> On 4/29/2020 2:37 PM, alvinx.zhang@intel.com wrote:
>>> From: Alvin Zhang <alvinx.zhang@intel.com>
>>>
>>> Enable detach device on secondary process.
>>>
>>> Fixes: b9eee2cb8c29 (e1000: support port hotplug)
>>> Cc: bernard.iremonger@intel.com
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com>
>>> ---
>>>    drivers/net/e1000/em_ethdev.c  | 2 +-
>>>    drivers/net/e1000/igb_ethdev.c | 4 ++--
>>>    2 files changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/net/e1000/em_ethdev.c
>>> b/drivers/net/e1000/em_ethdev.c index 188cda3..902b1cd 100644
>>> --- a/drivers/net/e1000/em_ethdev.c
>>> +++ b/drivers/net/e1000/em_ethdev.c
>>> @@ -321,7 +321,7 @@ static int eth_em_set_mc_addr_list(struct
>> rte_eth_dev *dev,
>>>    	PMD_INIT_FUNC_TRACE();
>>>
>>>    	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
>>> -		return -EPERM;
>>> +		return 0;
>>>
>>
>> I guess you mean pass through the process of the RTE_PROC_SECONDARY
>> case when detach device, but what about the other case like
>> RTE_PROC_INVALID or RTE_PROC_AUTO ?
>>
>>
>>>    	eth_em_close(eth_dev);
>>>
>>> diff --git a/drivers/net/e1000/igb_ethdev.c
>>> b/drivers/net/e1000/igb_ethdev.c index 520fba8..a5551e8 100644
>>> --- a/drivers/net/e1000/igb_ethdev.c
>>> +++ b/drivers/net/e1000/igb_ethdev.c
>>> @@ -923,7 +923,7 @@ static int igb_flex_filter_uninit(struct rte_eth_dev
>> *eth_dev)
>>>    	PMD_INIT_FUNC_TRACE();
>>>
>>>    	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
>>> -		return -EPERM;
>>> +		return 0;
>>>
>>>    	eth_igb_close(eth_dev);
>>>
>>> @@ -1044,7 +1044,7 @@ static int igb_flex_filter_uninit(struct rte_eth_dev
>> *eth_dev)
>>>    	PMD_INIT_FUNC_TRACE();
>>>
>>>    	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
>>> -		return -EPERM;
>>> +		return 0;
>>>
>>>    	igbvf_dev_close(eth_dev);


Reviewed-by: Jeff Guo <jia.guo@intel.com>
Xiaolong Ye May 12, 2020, 5:23 a.m. UTC | #5 
On 04/29, alvinx.zhang@intel.com wrote:
>From: Alvin Zhang <alvinx.zhang@intel.com>
Cc: stable@dpdk.org
>
>Enable detach device on secondary process.
>
>Fixes: b9eee2cb8c29 (e1000: support port hotplug)
>Cc: bernard.iremonger@intel.com
>Cc: stable@dpdk.org
>
>Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com>
>---
> drivers/net/e1000/em_ethdev.c  | 2 +-
> drivers/net/e1000/igb_ethdev.c | 4 ++--
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
>diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
>index 188cda3..902b1cd 100644
>--- a/drivers/net/e1000/em_ethdev.c
>+++ b/drivers/net/e1000/em_ethdev.c
>@@ -321,7 +321,7 @@ static int eth_em_set_mc_addr_list(struct rte_eth_dev *dev,
> 	PMD_INIT_FUNC_TRACE();
> 
> 	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
>-		return -EPERM;
>+		return 0;
> 
> 	eth_em_close(eth_dev);
> 
>diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
>index 520fba8..a5551e8 100644
>--- a/drivers/net/e1000/igb_ethdev.c
>+++ b/drivers/net/e1000/igb_ethdev.c
>@@ -923,7 +923,7 @@ static int igb_flex_filter_uninit(struct rte_eth_dev *eth_dev)
> 	PMD_INIT_FUNC_TRACE();
> 
> 	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
>-		return -EPERM;
>+		return 0;
> 
> 	eth_igb_close(eth_dev);
> 
>@@ -1044,7 +1044,7 @@ static int igb_flex_filter_uninit(struct rte_eth_dev *eth_dev)
> 	PMD_INIT_FUNC_TRACE();
> 
> 	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
>-		return -EPERM;
>+		return 0;
> 
> 	igbvf_dev_close(eth_dev);
> 
>-- 
>1.8.3.1
>

Applied to dpdk-next-net-intel, Thanks.
diff mbox series

Patch

diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
index 188cda3..902b1cd 100644
--- a/drivers/net/e1000/em_ethdev.c
+++ b/drivers/net/e1000/em_ethdev.c
@@ -321,7 +321,7 @@  static int eth_em_set_mc_addr_list(struct rte_eth_dev *dev,
 	PMD_INIT_FUNC_TRACE();
 
 	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
-		return -EPERM;
+		return 0;
 
 	eth_em_close(eth_dev);
 
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index 520fba8..a5551e8 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -923,7 +923,7 @@  static int igb_flex_filter_uninit(struct rte_eth_dev *eth_dev)
 	PMD_INIT_FUNC_TRACE();
 
 	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
-		return -EPERM;
+		return 0;
 
 	eth_igb_close(eth_dev);
 
@@ -1044,7 +1044,7 @@  static int igb_flex_filter_uninit(struct rte_eth_dev *eth_dev)
 	PMD_INIT_FUNC_TRACE();
 
 	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
-		return -EPERM;
+		return 0;
 
 	igbvf_dev_close(eth_dev);