[dpdk-dev] net/ixgbe: fix hierarchy commit check

Message ID 1501080853-88038-1-git-send-email-wenzhuo.lu@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers

Checks

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

Commit Message

Wenzhuo Lu July 26, 2017, 2:54 p.m. UTC
  If there's no Traffic Management node added,
not necessary to check if TM is committed.

Fixes: 5713ade69776 ("net/ixgbe: support committing TM hierarchy")

Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Cristian Dumitrescu July 26, 2017, 3:17 p.m. UTC | #1
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wenzhuo Lu
> Sent: Wednesday, July 26, 2017 3:54 PM
> To: dev@dpdk.org
> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>
> Subject: [dpdk-dev] [PATCH] net/ixgbe: fix hierarchy commit check
> 
> If there's no Traffic Management node added,
> not necessary to check if TM is committed.
> 
> Fixes: 5713ade69776 ("net/ixgbe: support committing TM hierarchy")
> 
> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> ---
>  drivers/net/ixgbe/ixgbe_ethdev.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> b/drivers/net/ixgbe/ixgbe_ethdev.c
> index 194058f..e436dca 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -2693,7 +2693,7 @@ static int eth_ixgbevf_pci_remove(struct
> rte_pci_device *pci_dev)
>  	ixgbe_l2_tunnel_conf(dev);
>  	ixgbe_filter_restore(dev);
> 
> -	if (!tm_conf->committed)
> +	if (tm_conf->root && !tm_conf->committed)
>  		PMD_DRV_LOG(WARNING,
>  			    "please call hierarchy_commit() "
>  			    "before starting the port");
> --
> 1.9.3

Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
  
Wiles, Keith July 26, 2017, 3:20 p.m. UTC | #2
> On Jul 26, 2017, at 9:54 AM, Wenzhuo Lu <wenzhuo.lu@intel.com> wrote:

> 

> If there's no Traffic Management node added,

> not necessary to check if TM is committed.

> 

> Fixes: 5713ade69776 ("net/ixgbe: support committing TM hierarchy")

> 

> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>

> ---

> drivers/net/ixgbe/ixgbe_ethdev.c | 2 +-

> 1 file changed, 1 insertion(+), 1 deletion(-)

> 

> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c

> index 194058f..e436dca 100644

> --- a/drivers/net/ixgbe/ixgbe_ethdev.c

> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c

> @@ -2693,7 +2693,7 @@ static int eth_ixgbevf_pci_remove(struct rte_pci_device *pci_dev)

> 	ixgbe_l2_tunnel_conf(dev);

> 	ixgbe_filter_restore(dev);

> 

> -	if (!tm_conf->committed)

> +	if (tm_conf->root && !tm_conf->committed)

> 		PMD_DRV_LOG(WARNING,

> 			    "please call hierarchy_commit() "

> 			    "before starting the port”);


This patch may work on ixgbe, but I am using i40e and the message is still present. I tried to apply your logic above and that seemed to fix the i40e message.

Looks like you need to change all of the drivers that use this logic.

Here is the i40e change:

i40e_dev_start()
-    if (!pf->tm_conf.committed)
+    if (pf->tm_conf.root && !pf->tm_conf.committed)

> -- 

> 1.9.3

> 


Regards,
Keith
  
Wiles, Keith July 26, 2017, 3:21 p.m. UTC | #3
> On Jul 26, 2017, at 10:20 AM, Wiles, Keith <keith.wiles@intel.com> wrote:

> 

>> 

>> On Jul 26, 2017, at 9:54 AM, Wenzhuo Lu <wenzhuo.lu@intel.com> wrote:

>> 

>> If there's no Traffic Management node added,

>> not necessary to check if TM is committed.

>> 

>> Fixes: 5713ade69776 ("net/ixgbe: support committing TM hierarchy")

>> 

>> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>

>> ---

>> drivers/net/ixgbe/ixgbe_ethdev.c | 2 +-

>> 1 file changed, 1 insertion(+), 1 deletion(-)

>> 

>> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c

>> index 194058f..e436dca 100644

>> --- a/drivers/net/ixgbe/ixgbe_ethdev.c

>> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c

>> @@ -2693,7 +2693,7 @@ static int eth_ixgbevf_pci_remove(struct rte_pci_device *pci_dev)

>> 	ixgbe_l2_tunnel_conf(dev);

>> 	ixgbe_filter_restore(dev);

>> 

>> -	if (!tm_conf->committed)

>> +	if (tm_conf->root && !tm_conf->committed)

>> 		PMD_DRV_LOG(WARNING,

>> 			    "please call hierarchy_commit() "

>> 			    "before starting the port”);

> 

> This patch may work on ixgbe, but I am using i40e and the message is still present. I tried to apply your logic above and that seemed to fix the i40e message.

> 

> Looks like you need to change all of the drivers that use this logic.

> 

> Here is the i40e change:

> 

> i40e_dev_start()

> -    if (!pf->tm_conf.committed)

> +    if (pf->tm_conf.root && !pf->tm_conf.committed)


Just saw the new commit.
> 

>> -- 

>> 1.9.3

>> 

> 

> Regards,

> Keith


Regards,
Keith
  
Thomas Monjalon July 31, 2017, 4:53 p.m. UTC | #4
> > If there's no Traffic Management node added,
> > not necessary to check if TM is committed.
> > 
> > Fixes: 5713ade69776 ("net/ixgbe: support committing TM hierarchy")
> > 
> > Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> 
> Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>

Added TM in the title and applied, thanks
  

Patch

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 194058f..e436dca 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -2693,7 +2693,7 @@  static int eth_ixgbevf_pci_remove(struct rte_pci_device *pci_dev)
 	ixgbe_l2_tunnel_conf(dev);
 	ixgbe_filter_restore(dev);
 
-	if (!tm_conf->committed)
+	if (tm_conf->root && !tm_conf->committed)
 		PMD_DRV_LOG(WARNING,
 			    "please call hierarchy_commit() "
 			    "before starting the port");