[v2] net/ixgbe: fix QoS performance drop issue

Message ID 20191121073205.65910-1-shougangx.wang@intel.com (mailing list archive)
State Accepted, archived
Delegated to: xiaolong ye
Headers
Series [v2] net/ixgbe: fix QoS performance drop issue |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-compilation success Compile Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/Intel-compilation fail Compilation issues
ci/travis-robot success Travis build: passed

Commit Message

Shougang Wang Nov. 21, 2019, 7:32 a.m. UTC
  Currently macsec offload will be enabled all the time
when device start. It will cause QoS sample application
performance drop issue. This patch add check before this
feature enabled.

Fixes: 50556c88104c ("net/ixgbe: fix MACsec setting")

Signed-off-by: Shougang Wang <shougangx.wang@intel.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c  | 14 ++++++++------
 drivers/net/ixgbe/ixgbe_ethdev.h  |  1 +
 drivers/net/ixgbe/rte_pmd_ixgbe.c |  1 +
 3 files changed, 10 insertions(+), 6 deletions(-)
  

Comments

Xiaolong Ye Nov. 21, 2019, 8:53 a.m. UTC | #1
On 11/21, Shougang Wang wrote:
>Currently macsec offload will be enabled all the time
>when device start. It will cause QoS sample application
>performance drop issue. This patch add check before this
>feature enabled.
>
>Fixes: 50556c88104c ("net/ixgbe: fix MACsec setting")
>
>Signed-off-by: Shougang Wang <shougangx.wang@intel.com>
>---
> drivers/net/ixgbe/ixgbe_ethdev.c  | 14 ++++++++------
> drivers/net/ixgbe/ixgbe_ethdev.h  |  1 +
> drivers/net/ixgbe/rte_pmd_ixgbe.c |  1 +
> 3 files changed, 10 insertions(+), 6 deletions(-)
>
>diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
>index 8c1caac18..aeb82d1c8 100644
>--- a/drivers/net/ixgbe/ixgbe_ethdev.c
>+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
>@@ -1095,6 +1095,8 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev, void *init_params __rte_unused)
> 
> 	PMD_INIT_FUNC_TRACE();
> 
>+	ixgbe_dev_macsec_setting_reset(eth_dev);
>+
> 	eth_dev->dev_ops = &ixgbe_eth_dev_ops;
> 	eth_dev->rx_pkt_burst = &ixgbe_recv_pkts;
> 	eth_dev->tx_pkt_burst = &ixgbe_xmit_pkts;
>@@ -2545,7 +2547,7 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
> 	uint32_t *link_speeds;
> 	struct ixgbe_tm_conf *tm_conf =
> 		IXGBE_DEV_PRIVATE_TO_TM_CONF(dev->data->dev_private);
>-	struct ixgbe_macsec_setting *macsec_ctrl =
>+	struct ixgbe_macsec_setting *macsec_setting =
> 		IXGBE_DEV_PRIVATE_TO_MACSEC_SETTING(dev->data->dev_private);
> 
> 	PMD_INIT_FUNC_TRACE();
>@@ -2799,8 +2801,9 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
> 	 */
> 	ixgbe_dev_link_update(dev, 0);
> 
>-	/* setup the macsec ctrl register */
>-	ixgbe_dev_macsec_register_enable(dev, macsec_ctrl);
>+	/* setup the macsec setting register */
>+	if (macsec_setting->offload_en)
>+		ixgbe_dev_macsec_register_enable(dev, macsec_setting);
> 
> 	return 0;
> 
>@@ -2833,9 +2836,6 @@ ixgbe_dev_stop(struct rte_eth_dev *dev)
> 
> 	PMD_INIT_FUNC_TRACE();
> 
>-	/* disable mecsec register */
>-	ixgbe_dev_macsec_register_disable(dev);
>-
> 	rte_eal_alarm_cancel(ixgbe_dev_setup_link_alarm_handler, dev);
> 
> 	/* disable interrupts */
>@@ -8843,6 +8843,7 @@ ixgbe_dev_macsec_setting_save(struct rte_eth_dev *dev,
> 	struct ixgbe_macsec_setting *macsec =
> 		IXGBE_DEV_PRIVATE_TO_MACSEC_SETTING(dev->data->dev_private);
> 
>+	macsec->offload_en = macsec_setting->offload_en;
> 	macsec->encrypt_en = macsec_setting->encrypt_en;
> 	macsec->replayprotect_en = macsec_setting->replayprotect_en;
> }
>@@ -8853,6 +8854,7 @@ ixgbe_dev_macsec_setting_reset(struct rte_eth_dev *dev)
> 	struct ixgbe_macsec_setting *macsec =
> 		IXGBE_DEV_PRIVATE_TO_MACSEC_SETTING(dev->data->dev_private);
> 
>+	macsec->offload_en = 0;
> 	macsec->encrypt_en = 0;
> 	macsec->replayprotect_en = 0;
> }
>diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
>index 5da6923a1..76a1b9d18 100644
>--- a/drivers/net/ixgbe/ixgbe_ethdev.h
>+++ b/drivers/net/ixgbe/ixgbe_ethdev.h
>@@ -366,6 +366,7 @@ struct rte_flow {
> };
> 
> struct ixgbe_macsec_setting {
>+	uint8_t offload_en;
> 	uint8_t encrypt_en;
> 	uint8_t replayprotect_en;
> };
>diff --git a/drivers/net/ixgbe/rte_pmd_ixgbe.c b/drivers/net/ixgbe/rte_pmd_ixgbe.c
>index 073fe1e23..8bcaded6e 100644
>--- a/drivers/net/ixgbe/rte_pmd_ixgbe.c
>+++ b/drivers/net/ixgbe/rte_pmd_ixgbe.c
>@@ -522,6 +522,7 @@ rte_pmd_ixgbe_macsec_enable(uint16_t port, uint8_t en, uint8_t rp)
> 
> 	dev = &rte_eth_devices[port];
> 
>+	macsec_setting.offload_en = 1;
> 	macsec_setting.encrypt_en = en;
> 	macsec_setting.replayprotect_en = rp;
> 
>-- 
>2.17.1
>

Reviewed-by: Xiaolong Ye <xiaolong.ye@intel.com>
  
Xiaolong Ye Nov. 21, 2019, 2:32 p.m. UTC | #2
On 11/21, Shougang Wang wrote:
>Currently macsec offload will be enabled all the time
>when device start. It will cause QoS sample application
>performance drop issue. This patch add check before this
>feature enabled.
>
>Fixes: 50556c88104c ("net/ixgbe: fix MACsec setting")
>
>Signed-off-by: Shougang Wang <shougangx.wang@intel.com>
>---
> drivers/net/ixgbe/ixgbe_ethdev.c  | 14 ++++++++------
> drivers/net/ixgbe/ixgbe_ethdev.h  |  1 +
> drivers/net/ixgbe/rte_pmd_ixgbe.c |  1 +
> 3 files changed, 10 insertions(+), 6 deletions(-)
>
>diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
>index 8c1caac18..aeb82d1c8 100644
>--- a/drivers/net/ixgbe/ixgbe_ethdev.c
>+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
>@@ -1095,6 +1095,8 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev, void *init_params __rte_unused)
> 
> 	PMD_INIT_FUNC_TRACE();
> 
>+	ixgbe_dev_macsec_setting_reset(eth_dev);
>+
> 	eth_dev->dev_ops = &ixgbe_eth_dev_ops;
> 	eth_dev->rx_pkt_burst = &ixgbe_recv_pkts;
> 	eth_dev->tx_pkt_burst = &ixgbe_xmit_pkts;
>@@ -2545,7 +2547,7 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
> 	uint32_t *link_speeds;
> 	struct ixgbe_tm_conf *tm_conf =
> 		IXGBE_DEV_PRIVATE_TO_TM_CONF(dev->data->dev_private);
>-	struct ixgbe_macsec_setting *macsec_ctrl =
>+	struct ixgbe_macsec_setting *macsec_setting =
> 		IXGBE_DEV_PRIVATE_TO_MACSEC_SETTING(dev->data->dev_private);
> 
> 	PMD_INIT_FUNC_TRACE();
>@@ -2799,8 +2801,9 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
> 	 */
> 	ixgbe_dev_link_update(dev, 0);
> 
>-	/* setup the macsec ctrl register */
>-	ixgbe_dev_macsec_register_enable(dev, macsec_ctrl);
>+	/* setup the macsec setting register */
>+	if (macsec_setting->offload_en)
>+		ixgbe_dev_macsec_register_enable(dev, macsec_setting);
> 
> 	return 0;
> 
>@@ -2833,9 +2836,6 @@ ixgbe_dev_stop(struct rte_eth_dev *dev)
> 
> 	PMD_INIT_FUNC_TRACE();
> 
>-	/* disable mecsec register */
>-	ixgbe_dev_macsec_register_disable(dev);
>-
> 	rte_eal_alarm_cancel(ixgbe_dev_setup_link_alarm_handler, dev);
> 
> 	/* disable interrupts */
>@@ -8843,6 +8843,7 @@ ixgbe_dev_macsec_setting_save(struct rte_eth_dev *dev,
> 	struct ixgbe_macsec_setting *macsec =
> 		IXGBE_DEV_PRIVATE_TO_MACSEC_SETTING(dev->data->dev_private);
> 
>+	macsec->offload_en = macsec_setting->offload_en;
> 	macsec->encrypt_en = macsec_setting->encrypt_en;
> 	macsec->replayprotect_en = macsec_setting->replayprotect_en;
> }
>@@ -8853,6 +8854,7 @@ ixgbe_dev_macsec_setting_reset(struct rte_eth_dev *dev)
> 	struct ixgbe_macsec_setting *macsec =
> 		IXGBE_DEV_PRIVATE_TO_MACSEC_SETTING(dev->data->dev_private);
> 
>+	macsec->offload_en = 0;
> 	macsec->encrypt_en = 0;
> 	macsec->replayprotect_en = 0;
> }
>diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
>index 5da6923a1..76a1b9d18 100644
>--- a/drivers/net/ixgbe/ixgbe_ethdev.h
>+++ b/drivers/net/ixgbe/ixgbe_ethdev.h
>@@ -366,6 +366,7 @@ struct rte_flow {
> };
> 
> struct ixgbe_macsec_setting {
>+	uint8_t offload_en;
> 	uint8_t encrypt_en;
> 	uint8_t replayprotect_en;
> };
>diff --git a/drivers/net/ixgbe/rte_pmd_ixgbe.c b/drivers/net/ixgbe/rte_pmd_ixgbe.c
>index 073fe1e23..8bcaded6e 100644
>--- a/drivers/net/ixgbe/rte_pmd_ixgbe.c
>+++ b/drivers/net/ixgbe/rte_pmd_ixgbe.c
>@@ -522,6 +522,7 @@ rte_pmd_ixgbe_macsec_enable(uint16_t port, uint8_t en, uint8_t rp)
> 
> 	dev = &rte_eth_devices[port];
> 
>+	macsec_setting.offload_en = 1;
> 	macsec_setting.encrypt_en = en;
> 	macsec_setting.replayprotect_en = rp;
> 
>-- 
>2.17.1
>

Applied to dpdk-next-net-intel, Thanks.
  

Patch

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 8c1caac18..aeb82d1c8 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -1095,6 +1095,8 @@  eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev, void *init_params __rte_unused)
 
 	PMD_INIT_FUNC_TRACE();
 
+	ixgbe_dev_macsec_setting_reset(eth_dev);
+
 	eth_dev->dev_ops = &ixgbe_eth_dev_ops;
 	eth_dev->rx_pkt_burst = &ixgbe_recv_pkts;
 	eth_dev->tx_pkt_burst = &ixgbe_xmit_pkts;
@@ -2545,7 +2547,7 @@  ixgbe_dev_start(struct rte_eth_dev *dev)
 	uint32_t *link_speeds;
 	struct ixgbe_tm_conf *tm_conf =
 		IXGBE_DEV_PRIVATE_TO_TM_CONF(dev->data->dev_private);
-	struct ixgbe_macsec_setting *macsec_ctrl =
+	struct ixgbe_macsec_setting *macsec_setting =
 		IXGBE_DEV_PRIVATE_TO_MACSEC_SETTING(dev->data->dev_private);
 
 	PMD_INIT_FUNC_TRACE();
@@ -2799,8 +2801,9 @@  ixgbe_dev_start(struct rte_eth_dev *dev)
 	 */
 	ixgbe_dev_link_update(dev, 0);
 
-	/* setup the macsec ctrl register */
-	ixgbe_dev_macsec_register_enable(dev, macsec_ctrl);
+	/* setup the macsec setting register */
+	if (macsec_setting->offload_en)
+		ixgbe_dev_macsec_register_enable(dev, macsec_setting);
 
 	return 0;
 
@@ -2833,9 +2836,6 @@  ixgbe_dev_stop(struct rte_eth_dev *dev)
 
 	PMD_INIT_FUNC_TRACE();
 
-	/* disable mecsec register */
-	ixgbe_dev_macsec_register_disable(dev);
-
 	rte_eal_alarm_cancel(ixgbe_dev_setup_link_alarm_handler, dev);
 
 	/* disable interrupts */
@@ -8843,6 +8843,7 @@  ixgbe_dev_macsec_setting_save(struct rte_eth_dev *dev,
 	struct ixgbe_macsec_setting *macsec =
 		IXGBE_DEV_PRIVATE_TO_MACSEC_SETTING(dev->data->dev_private);
 
+	macsec->offload_en = macsec_setting->offload_en;
 	macsec->encrypt_en = macsec_setting->encrypt_en;
 	macsec->replayprotect_en = macsec_setting->replayprotect_en;
 }
@@ -8853,6 +8854,7 @@  ixgbe_dev_macsec_setting_reset(struct rte_eth_dev *dev)
 	struct ixgbe_macsec_setting *macsec =
 		IXGBE_DEV_PRIVATE_TO_MACSEC_SETTING(dev->data->dev_private);
 
+	macsec->offload_en = 0;
 	macsec->encrypt_en = 0;
 	macsec->replayprotect_en = 0;
 }
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
index 5da6923a1..76a1b9d18 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.h
+++ b/drivers/net/ixgbe/ixgbe_ethdev.h
@@ -366,6 +366,7 @@  struct rte_flow {
 };
 
 struct ixgbe_macsec_setting {
+	uint8_t offload_en;
 	uint8_t encrypt_en;
 	uint8_t replayprotect_en;
 };
diff --git a/drivers/net/ixgbe/rte_pmd_ixgbe.c b/drivers/net/ixgbe/rte_pmd_ixgbe.c
index 073fe1e23..8bcaded6e 100644
--- a/drivers/net/ixgbe/rte_pmd_ixgbe.c
+++ b/drivers/net/ixgbe/rte_pmd_ixgbe.c
@@ -522,6 +522,7 @@  rte_pmd_ixgbe_macsec_enable(uint16_t port, uint8_t en, uint8_t rp)
 
 	dev = &rte_eth_devices[port];
 
+	macsec_setting.offload_en = 1;
 	macsec_setting.encrypt_en = en;
 	macsec_setting.replayprotect_en = rp;