[dpdk-dev,1/8] net/qede: fix to disable per-VF Tx switching feature

Message ID 1510043665-8160-2-git-send-email-rasesh.mody@cavium.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

Mody, Rasesh Nov. 7, 2017, 8:34 a.m. UTC
  From: Harish Patil <harish.patil@cavium.com>

Provide a knob to control per-VF Tx switching feature by adding a config
option, CONFIG_RTE_LIBRTE_QEDE_VF_TX_SWITCH. By default, it will be kept
in disabled state for better performance with small sized frames.

Fixes: 2ea6f76aff40 ("qede: add core driver")
Cc: stable@dpdk.org

Signed-off-by: Harish Patil <harish.patil@cavium.com>
---
 config/common_base             |    1 +
 drivers/net/qede/qede_ethdev.c |    9 ++++++++-
 2 files changed, 9 insertions(+), 1 deletion(-)
  

Comments

Thomas Monjalon Nov. 8, 2017, 12:53 a.m. UTC | #1
Hi,

07/11/2017 09:34, Rasesh Mody:
> From: Harish Patil <harish.patil@cavium.com>
> 
> Provide a knob to control per-VF Tx switching feature by adding a config
> option, CONFIG_RTE_LIBRTE_QEDE_VF_TX_SWITCH. By default, it will be kept
> in disabled state for better performance with small sized frames.
> 
> Fixes: 2ea6f76aff40 ("qede: add core driver")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Harish Patil <harish.patil@cavium.com>
> ---
>  config/common_base             |    1 +
>  drivers/net/qede/qede_ethdev.c |    9 ++++++++-
>  2 files changed, 9 insertions(+), 1 deletion(-)
> 
> --- a/config/common_base
> +++ b/config/common_base
> +CONFIG_RTE_LIBRTE_QEDE_VF_TX_SWITCH=n

We should remove all compile-time options because they cannot be used
when DPDK is pre-packaged.
That's why the rule is "NO NEW COMPILE TIME OPTION".

After discussion with Ferruh, this patch is accepted as a hotfix.
But this option is expected to be removed quickly.
I've sent a patch to remove this option in 18.02:
	http://dpdk.org/ml/archives/dev/2017-November/081488.html
It gives you some time to supersede my patch by introducing a
run-time driver option.

Thanks
  
Patil, Harish Nov. 8, 2017, 1:34 a.m. UTC | #2
-----Original Message-----
From: Thomas Monjalon <thomas@monjalon.net>

Date: Tuesday, November 7, 2017 at 5:53 PM
To: "Mody, Rasesh" <Rasesh.Mody@cavium.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>, Harish Patil <Harish.Patil@cavium.com>,
"ferruh.yigit@intel.com" <ferruh.yigit@intel.com>, Dept-Eng DPDK Dev
<Dept-EngDPDKDev@cavium.com>
Subject: Re: [dpdk-dev] [PATCH 1/8] net/qede: fix to disable per-VF Tx
switching feature

>Hi,

>

>07/11/2017 09:34, Rasesh Mody:

>> From: Harish Patil <harish.patil@cavium.com>

>> 

>> Provide a knob to control per-VF Tx switching feature by adding a config

>> option, CONFIG_RTE_LIBRTE_QEDE_VF_TX_SWITCH. By default, it will be kept

>> in disabled state for better performance with small sized frames.

>> 

>> Fixes: 2ea6f76aff40 ("qede: add core driver")

>> Cc: stable@dpdk.org

>> 

>> Signed-off-by: Harish Patil <harish.patil@cavium.com>

>> ---

>>  config/common_base             |    1 +

>>  drivers/net/qede/qede_ethdev.c |    9 ++++++++-

>>  2 files changed, 9 insertions(+), 1 deletion(-)

>> 

>> --- a/config/common_base

>> +++ b/config/common_base

>> +CONFIG_RTE_LIBRTE_QEDE_VF_TX_SWITCH=n

>

>We should remove all compile-time options because they cannot be used

>when DPDK is pre-packaged.

>That's why the rule is "NO NEW COMPILE TIME OPTION".

>

>After discussion with Ferruh, this patch is accepted as a hotfix.

>But this option is expected to be removed quickly.

>I've sent a patch to remove this option in 18.02:

>	http://dpdk.org/ml/archives/dev/2017-November/081488.html

>It gives you some time to supersede my patch by introducing a

>run-time driver option.

>

>Thanks


Hi Thomas,
Sure, thanks, we will address before 18.02-rc1.
Do you have any example on how to add a run-time driver option?
Does it mean to introduce some change in ethdev?
Thanks.

>
  
Thomas Monjalon Nov. 8, 2017, 1:44 a.m. UTC | #3
08/11/2017 02:34, Patil, Harish:
> >> --- a/config/common_base
> >> +++ b/config/common_base
> >> +CONFIG_RTE_LIBRTE_QEDE_VF_TX_SWITCH=n
> >
> >We should remove all compile-time options because they cannot be used
> >when DPDK is pre-packaged.
> >That's why the rule is "NO NEW COMPILE TIME OPTION".
> >
> >After discussion with Ferruh, this patch is accepted as a hotfix.
> >But this option is expected to be removed quickly.
> >I've sent a patch to remove this option in 18.02:
> >	http://dpdk.org/ml/archives/dev/2017-November/081488.html
> >It gives you some time to supersede my patch by introducing a
> >run-time driver option.
> >
> >Thanks
> 
> Hi Thomas,
> Sure, thanks, we will address before 18.02-rc1.
> Do you have any example on how to add a run-time driver option?
> Does it mean to introduce some change in ethdev?

It is done by parsing devargs with rte_kvargs.
Examples:
	http://dpdk.org/commit/447e0d379
	http://dpdk.org/commit/001a520e4
	http://dpdk.org/commit/7958b1310
  
Patil, Harish Nov. 8, 2017, 2:34 a.m. UTC | #4
-----Original Message-----
From: Thomas Monjalon <thomas@monjalon.net>

Date: Tuesday, November 7, 2017 at 6:44 PM
To: Harish Patil <Harish.Patil@cavium.com>
Cc: "ferruh.yigit@intel.com" <ferruh.yigit@intel.com>, "dev@dpdk.org"
<dev@dpdk.org>, Dept-Eng DPDK Dev <Dept-EngDPDKDev@cavium.com>, "Mody,
Rasesh" <Rasesh.Mody@cavium.com>
Subject: Re: [dpdk-dev] [PATCH 1/8] net/qede: fix to disable per-VF Tx
switching feature

>08/11/2017 02:34, Patil, Harish:

>> >> --- a/config/common_base

>> >> +++ b/config/common_base

>> >> +CONFIG_RTE_LIBRTE_QEDE_VF_TX_SWITCH=n

>> >

>> >We should remove all compile-time options because they cannot be used

>> >when DPDK is pre-packaged.

>> >That's why the rule is "NO NEW COMPILE TIME OPTION".

>> >

>> >After discussion with Ferruh, this patch is accepted as a hotfix.

>> >But this option is expected to be removed quickly.

>> >I've sent a patch to remove this option in 18.02:

>> >	http://dpdk.org/ml/archives/dev/2017-November/081488.html

>> >It gives you some time to supersede my patch by introducing a

>> >run-time driver option.

>> >

>> >Thanks

>> 

>> Hi Thomas,

>> Sure, thanks, we will address before 18.02-rc1.

>> Do you have any example on how to add a run-time driver option?

>> Does it mean to introduce some change in ethdev?

>

>It is done by parsing devargs with rte_kvargs.

>Examples:

>	http://dpdk.org/commit/447e0d379

>	http://dpdk.org/commit/001a520e4

>	http://dpdk.org/commit/7958b1310


Thanks Thomas.
>
  

Patch

diff --git a/config/common_base b/config/common_base
index 82ee754..a22d0b1 100644
--- a/config/common_base
+++ b/config/common_base
@@ -410,6 +410,7 @@  CONFIG_RTE_LIBRTE_QEDE_DEBUG_INFO=n
 CONFIG_RTE_LIBRTE_QEDE_DEBUG_DRIVER=n
 CONFIG_RTE_LIBRTE_QEDE_DEBUG_TX=n
 CONFIG_RTE_LIBRTE_QEDE_DEBUG_RX=n
+CONFIG_RTE_LIBRTE_QEDE_VF_TX_SWITCH=n
 #Provides abs path/name of the firmware file.
 #Empty string denotes driver will use default firmware
 CONFIG_RTE_LIBRTE_QEDE_FW=""
diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
index 661d938..c228b06 100644
--- a/drivers/net/qede/qede_ethdev.c
+++ b/drivers/net/qede/qede_ethdev.c
@@ -453,6 +453,12 @@  int qede_activate_vport(struct rte_eth_dev *eth_dev, bool flg)
 	params.update_vport_active_tx_flg = 1;
 	params.vport_active_rx_flg = flg;
 	params.vport_active_tx_flg = flg;
+#ifndef RTE_LIBRTE_QEDE_VF_TX_SWITCH
+	if (IS_VF(edev)) {
+		params.update_tx_switching_flg = 1;
+		params.tx_switching_flg = !flg;
+	}
+#endif
 	for_each_hwfn(edev, i) {
 		p_hwfn = &edev->hwfns[i];
 		params.opaque_fid = p_hwfn->hw_info.opaque_fid;
@@ -463,7 +469,8 @@  int qede_activate_vport(struct rte_eth_dev *eth_dev, bool flg)
 			break;
 		}
 	}
-	DP_INFO(edev, "vport %s\n", flg ? "activated" : "deactivated");
+	DP_INFO(edev, "vport %s VF tx-switch %s\n", flg ? "activated" : "deactivated",
+			params.tx_switching_flg ? "enabled" : "disabled");
 	return rc;
 }