DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] net/ixgbe: add support of loopback for X540/X550
@ 2019-01-02 16:00 Julien Meunier
  2019-01-07  6:53 ` Zhang, Qi Z
                   ` (3 more replies)
  0 siblings, 4 replies; 20+ messages in thread
From: Julien Meunier @ 2019-01-02 16:00 UTC (permalink / raw)
  To: konstantin.ananyev, wenzhuo.lu; +Cc: dev

Loopback mode is also supported on X540 and X550 NICs, according to
their datasheet (section 15.2). The way to set it up is a little
different of the 82599.

Signed-off-by: Julien Meunier <julien.meunier@nokia.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 10 ++++++---
 drivers/net/ixgbe/ixgbe_ethdev.h |  5 ++---
 drivers/net/ixgbe/ixgbe_rxtx.c   | 47 ++++++++++++++++++++++++++++++++++------
 3 files changed, 49 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 7493110..7eb3303 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -2652,9 +2652,13 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
 		goto error;
 	}
 
-	/* Skip link setup if loopback mode is enabled for 82599. */
-	if (hw->mac.type == ixgbe_mac_82599EB &&
-			dev->data->dev_conf.lpbk_mode == IXGBE_LPBK_82599_TX_RX)
+	/* Skip link setup if loopback mode is enabled. */
+	if ((hw->mac.type == ixgbe_mac_82599EB ||
+	     hw->mac.type == ixgbe_mac_X540 ||
+	     hw->mac.type == ixgbe_mac_X550 ||
+	     hw->mac.type == ixgbe_mac_X550EM_x ||
+	     hw->mac.type == ixgbe_mac_X550EM_a) &&
+			dev->data->dev_conf.lpbk_mode == IXGBE_LPBK_TX_RX)
 		goto skip_link_setup;
 
 	if (ixgbe_is_sfp(hw) && hw->phy.multispeed_fiber) {
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
index 565c69c..c60a697 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.h
+++ b/drivers/net/ixgbe/ixgbe_ethdev.h
@@ -65,9 +65,8 @@
 #define IXGBE_QUEUE_ITR_INTERVAL_DEFAULT	500 /* 500us */
 
 /* Loopback operation modes */
-/* 82599 specific loopback operation types */
-#define IXGBE_LPBK_82599_NONE   0x0 /* Default value. Loopback is disabled. */
-#define IXGBE_LPBK_82599_TX_RX  0x1 /* Tx->Rx loopback operation is enabled. */
+#define IXGBE_LPBK_NONE   0x0 /* Default value. Loopback is disabled. */
+#define IXGBE_LPBK_TX_RX  0x1 /* Tx->Rx loopback operation is enabled. */
 
 #define IXGBE_MAX_JUMBO_FRAME_SIZE      0x2600 /* Maximum Jumbo frame size. */
 
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index 9a79d18..0ef7fdf 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -4879,10 +4879,14 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
 		hlreg0 &= ~IXGBE_HLREG0_JUMBOEN;
 
 	/*
-	 * If loopback mode is configured for 82599, set LPBK bit.
+	 * If loopback mode is configured, set LPBK bit.
 	 */
-	if (hw->mac.type == ixgbe_mac_82599EB &&
-			dev->data->dev_conf.lpbk_mode == IXGBE_LPBK_82599_TX_RX)
+	if ((hw->mac.type == ixgbe_mac_82599EB ||
+	     hw->mac.type == ixgbe_mac_X540 ||
+	     hw->mac.type == ixgbe_mac_X550 ||
+	     hw->mac.type == ixgbe_mac_X550EM_x ||
+	     hw->mac.type == ixgbe_mac_X550EM_a) &&
+			dev->data->dev_conf.lpbk_mode == IXGBE_LPBK_TX_RX)
 		hlreg0 |= IXGBE_HLREG0_LPBK;
 	else
 		hlreg0 &= ~IXGBE_HLREG0_LPBK;
@@ -5088,6 +5092,29 @@ ixgbe_setup_loopback_link_82599(struct ixgbe_hw *hw)
 	msec_delay(50);
 }
 
+/*
+ * Set up link loopback for X540 / X550 mode Tx->Rx.
+ */
+static inline void __attribute__((cold))
+ixgbe_setup_loopback_link_x540_x550(struct ixgbe_hw *hw)
+{
+	uint32_t macc;
+	PMD_INIT_FUNC_TRACE();
+
+	/* datasheet 15.2.1: MACC.FLU = 1 (force link up) */
+	macc = IXGBE_READ_REG(hw, IXGBE_MACC);
+	macc |= IXGBE_MACC_FLU;
+	IXGBE_WRITE_REG(hw, IXGBE_MACC, macc);
+
+	/* Restart link */
+	IXGBE_WRITE_REG(hw,
+			IXGBE_AUTOC,
+			IXGBE_AUTOC_LMS_10G_LINK_NO_AN | IXGBE_AUTOC_FLU);
+
+	hw->mac.ops.release_swfw_sync(hw, IXGBE_GSSR_MAC_CSR_SM);
+	msec_delay(50);
+}
+
 
 /*
  * Start Transmit and Receive Units.
@@ -5148,10 +5175,16 @@ ixgbe_dev_rxtx_start(struct rte_eth_dev *dev)
 	rxctrl |= IXGBE_RXCTRL_RXEN;
 	hw->mac.ops.enable_rx_dma(hw, rxctrl);
 
-	/* If loopback mode is enabled for 82599, set up the link accordingly */
-	if (hw->mac.type == ixgbe_mac_82599EB &&
-			dev->data->dev_conf.lpbk_mode == IXGBE_LPBK_82599_TX_RX)
-		ixgbe_setup_loopback_link_82599(hw);
+	/* If loopback mode is enabled, set up the link accordingly */
+	if (dev->data->dev_conf.lpbk_mode == IXGBE_LPBK_TX_RX) {
+		if (hw->mac.type == ixgbe_mac_82599EB)
+			ixgbe_setup_loopback_link_82599(hw);
+		else if (hw->mac.type == ixgbe_mac_X540 ||
+		     hw->mac.type == ixgbe_mac_X550 ||
+		     hw->mac.type == ixgbe_mac_X550EM_x ||
+		     hw->mac.type == ixgbe_mac_X550EM_a)
+			ixgbe_setup_loopback_link_x540_x550(hw);
+	}
 
 #ifdef RTE_LIBRTE_SECURITY
 	if ((dev->data->dev_conf.rxmode.offloads &
-- 
2.10.2

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [PATCH] net/ixgbe: add support of loopback for X540/X550
  2019-01-02 16:00 [dpdk-dev] [PATCH] net/ixgbe: add support of loopback for X540/X550 Julien Meunier
@ 2019-01-07  6:53 ` Zhang, Qi Z
  2019-01-07 15:53   ` Meunier, Julien (Nokia - FR/Paris-Saclay)
  2019-01-07 10:05 ` Zhao1, Wei
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 20+ messages in thread
From: Zhang, Qi Z @ 2019-01-07  6:53 UTC (permalink / raw)
  To: Julien Meunier, Ananyev, Konstantin, Lu, Wenzhuo; +Cc: dev



> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Julien Meunier
> Sent: Thursday, January 3, 2019 12:01 AM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Lu, Wenzhuo
> <wenzhuo.lu@intel.com>
> Cc: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH] net/ixgbe: add support of loopback for X540/X550
> 
> Loopback mode is also supported on X540 and X550 NICs, according to their
> datasheet (section 15.2). The way to set it up is a little different of the 82599.

Thanks for enable this.

one question is, Datasheet also mentioned that auto negotiation should be disabled
but I didn't see any related change with it.

Would you share more insight on this?

> 
> Signed-off-by: Julien Meunier <julien.meunier@nokia.com>
> ---
>  drivers/net/ixgbe/ixgbe_ethdev.c | 10 ++++++---
> drivers/net/ixgbe/ixgbe_ethdev.h |  5 ++---
>  drivers/net/ixgbe/ixgbe_rxtx.c   | 47
> ++++++++++++++++++++++++++++++++++------
>  3 files changed, 49 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> b/drivers/net/ixgbe/ixgbe_ethdev.c
> index 7493110..7eb3303 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -2652,9 +2652,13 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
>  		goto error;
>  	}
> 
> -	/* Skip link setup if loopback mode is enabled for 82599. */
> -	if (hw->mac.type == ixgbe_mac_82599EB &&
> -			dev->data->dev_conf.lpbk_mode == IXGBE_LPBK_82599_TX_RX)
> +	/* Skip link setup if loopback mode is enabled. */
> +	if ((hw->mac.type == ixgbe_mac_82599EB ||
> +	     hw->mac.type == ixgbe_mac_X540 ||
> +	     hw->mac.type == ixgbe_mac_X550 ||
> +	     hw->mac.type == ixgbe_mac_X550EM_x ||
> +	     hw->mac.type == ixgbe_mac_X550EM_a) &&
> +			dev->data->dev_conf.lpbk_mode == IXGBE_LPBK_TX_RX)
>  		goto skip_link_setup;
> 
>  	if (ixgbe_is_sfp(hw) && hw->phy.multispeed_fiber) { diff --git
> a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
> index 565c69c..c60a697 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.h
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.h
> @@ -65,9 +65,8 @@
>  #define IXGBE_QUEUE_ITR_INTERVAL_DEFAULT	500 /* 500us */
> 
>  /* Loopback operation modes */
> -/* 82599 specific loopback operation types */
> -#define IXGBE_LPBK_82599_NONE   0x0 /* Default value. Loopback is disabled.
> */
> -#define IXGBE_LPBK_82599_TX_RX  0x1 /* Tx->Rx loopback operation is
> enabled. */
> +#define IXGBE_LPBK_NONE   0x0 /* Default value. Loopback is disabled. */
> +#define IXGBE_LPBK_TX_RX  0x1 /* Tx->Rx loopback operation is enabled.
> +*/
> 
>  #define IXGBE_MAX_JUMBO_FRAME_SIZE      0x2600 /* Maximum Jumbo
> frame size. */
> 
> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
> index 9a79d18..0ef7fdf 100644
> --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> @@ -4879,10 +4879,14 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
>  		hlreg0 &= ~IXGBE_HLREG0_JUMBOEN;
> 
>  	/*
> -	 * If loopback mode is configured for 82599, set LPBK bit.
> +	 * If loopback mode is configured, set LPBK bit.
>  	 */
> -	if (hw->mac.type == ixgbe_mac_82599EB &&
> -			dev->data->dev_conf.lpbk_mode == IXGBE_LPBK_82599_TX_RX)
> +	if ((hw->mac.type == ixgbe_mac_82599EB ||
> +	     hw->mac.type == ixgbe_mac_X540 ||
> +	     hw->mac.type == ixgbe_mac_X550 ||
> +	     hw->mac.type == ixgbe_mac_X550EM_x ||
> +	     hw->mac.type == ixgbe_mac_X550EM_a) &&
> +			dev->data->dev_conf.lpbk_mode == IXGBE_LPBK_TX_RX)
>  		hlreg0 |= IXGBE_HLREG0_LPBK;
>  	else
>  		hlreg0 &= ~IXGBE_HLREG0_LPBK;
> @@ -5088,6 +5092,29 @@ ixgbe_setup_loopback_link_82599(struct ixgbe_hw
> *hw)
>  	msec_delay(50);
>  }
> 
> +/*
> + * Set up link loopback for X540 / X550 mode Tx->Rx.
> + */
> +static inline void __attribute__((cold))
> +ixgbe_setup_loopback_link_x540_x550(struct ixgbe_hw *hw) {
> +	uint32_t macc;
> +	PMD_INIT_FUNC_TRACE();
> +
> +	/* datasheet 15.2.1: MACC.FLU = 1 (force link up) */
> +	macc = IXGBE_READ_REG(hw, IXGBE_MACC);
> +	macc |= IXGBE_MACC_FLU;
> +	IXGBE_WRITE_REG(hw, IXGBE_MACC, macc);
> +
> +	/* Restart link */
> +	IXGBE_WRITE_REG(hw,
> +			IXGBE_AUTOC,
> +			IXGBE_AUTOC_LMS_10G_LINK_NO_AN | IXGBE_AUTOC_FLU);
> +
> +	hw->mac.ops.release_swfw_sync(hw, IXGBE_GSSR_MAC_CSR_SM);
> +	msec_delay(50);
> +}
> +
> 
>  /*
>   * Start Transmit and Receive Units.
> @@ -5148,10 +5175,16 @@ ixgbe_dev_rxtx_start(struct rte_eth_dev *dev)
>  	rxctrl |= IXGBE_RXCTRL_RXEN;
>  	hw->mac.ops.enable_rx_dma(hw, rxctrl);
> 
> -	/* If loopback mode is enabled for 82599, set up the link accordingly */
> -	if (hw->mac.type == ixgbe_mac_82599EB &&
> -			dev->data->dev_conf.lpbk_mode == IXGBE_LPBK_82599_TX_RX)
> -		ixgbe_setup_loopback_link_82599(hw);
> +	/* If loopback mode is enabled, set up the link accordingly */
> +	if (dev->data->dev_conf.lpbk_mode == IXGBE_LPBK_TX_RX) {
> +		if (hw->mac.type == ixgbe_mac_82599EB)
> +			ixgbe_setup_loopback_link_82599(hw);
> +		else if (hw->mac.type == ixgbe_mac_X540 ||
> +		     hw->mac.type == ixgbe_mac_X550 ||
> +		     hw->mac.type == ixgbe_mac_X550EM_x ||
> +		     hw->mac.type == ixgbe_mac_X550EM_a)
> +			ixgbe_setup_loopback_link_x540_x550(hw);
> +	}
> 
>  #ifdef RTE_LIBRTE_SECURITY
>  	if ((dev->data->dev_conf.rxmode.offloads &
> --
> 2.10.2

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [PATCH] net/ixgbe: add support of loopback for X540/X550
  2019-01-02 16:00 [dpdk-dev] [PATCH] net/ixgbe: add support of loopback for X540/X550 Julien Meunier
  2019-01-07  6:53 ` Zhang, Qi Z
@ 2019-01-07 10:05 ` Zhao1, Wei
  2019-01-07 16:04   ` Meunier, Julien (Nokia - FR/Paris-Saclay)
  2019-02-07 17:30 ` [dpdk-dev] [PATCH 1/2] net/ixgbe: do not start on unsupported loopback mode Julien Meunier
  2019-02-20 21:05 ` [dpdk-dev] [PATCH v3 " Julien Meunier
  3 siblings, 1 reply; 20+ messages in thread
From: Zhao1, Wei @ 2019-01-07 10:05 UTC (permalink / raw)
  To: Julien Meunier, Ananyev, Konstantin, Lu, Wenzhuo; +Cc: dev



> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Julien Meunier
> Sent: Thursday, January 3, 2019 12:01 AM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Lu, Wenzhuo
> <wenzhuo.lu@intel.com>
> Cc: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH] net/ixgbe: add support of loopback for
> X540/X550
> 
> Loopback mode is also supported on X540 and X550 NICs, according to their
> datasheet (section 15.2). The way to set it up is a little different of the 82599.
> 
> Signed-off-by: Julien Meunier <julien.meunier@nokia.com>
> ---
>  drivers/net/ixgbe/ixgbe_ethdev.c | 10 ++++++---
> drivers/net/ixgbe/ixgbe_ethdev.h |  5 ++---
>  drivers/net/ixgbe/ixgbe_rxtx.c   | 47
> ++++++++++++++++++++++++++++++++++------
>  3 files changed, 49 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> b/drivers/net/ixgbe/ixgbe_ethdev.c
> index 7493110..7eb3303 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -2652,9 +2652,13 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
>  		goto error;
>  	}
> 
> -	/* Skip link setup if loopback mode is enabled for 82599. */
> -	if (hw->mac.type == ixgbe_mac_82599EB &&
> -			dev->data->dev_conf.lpbk_mode ==
> IXGBE_LPBK_82599_TX_RX)
> +	/* Skip link setup if loopback mode is enabled. */
> +	if ((hw->mac.type == ixgbe_mac_82599EB ||
> +	     hw->mac.type == ixgbe_mac_X540 ||
> +	     hw->mac.type == ixgbe_mac_X550 ||
> +	     hw->mac.type == ixgbe_mac_X550EM_x ||
> +	     hw->mac.type == ixgbe_mac_X550EM_a) &&
> +			dev->data->dev_conf.lpbk_mode ==
> IXGBE_LPBK_TX_RX)
>  		goto skip_link_setup;
> 
>  	if (ixgbe_is_sfp(hw) && hw->phy.multispeed_fiber) { diff --git
> a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
> index 565c69c..c60a697 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.h
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.h
> @@ -65,9 +65,8 @@
>  #define IXGBE_QUEUE_ITR_INTERVAL_DEFAULT	500 /* 500us */
> 
>  /* Loopback operation modes */
> -/* 82599 specific loopback operation types */
> -#define IXGBE_LPBK_82599_NONE   0x0 /* Default value. Loopback is
> disabled. */
> -#define IXGBE_LPBK_82599_TX_RX  0x1 /* Tx->Rx loopback operation is
> enabled. */
> +#define IXGBE_LPBK_NONE   0x0 /* Default value. Loopback is disabled. */
> +#define IXGBE_LPBK_TX_RX  0x1 /* Tx->Rx loopback operation is enabled.
> +*/
> 
>  #define IXGBE_MAX_JUMBO_FRAME_SIZE      0x2600 /* Maximum Jumbo
> frame size. */
> 
> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
> index 9a79d18..0ef7fdf 100644
> --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> @@ -4879,10 +4879,14 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
>  		hlreg0 &= ~IXGBE_HLREG0_JUMBOEN;
> 
>  	/*
> -	 * If loopback mode is configured for 82599, set LPBK bit.
> +	 * If loopback mode is configured, set LPBK bit.
>  	 */
> -	if (hw->mac.type == ixgbe_mac_82599EB &&
> -			dev->data->dev_conf.lpbk_mode ==
> IXGBE_LPBK_82599_TX_RX)
> +	if ((hw->mac.type == ixgbe_mac_82599EB ||
> +	     hw->mac.type == ixgbe_mac_X540 ||
> +	     hw->mac.type == ixgbe_mac_X550 ||
> +	     hw->mac.type == ixgbe_mac_X550EM_x ||
> +	     hw->mac.type == ixgbe_mac_X550EM_a) &&
> +			dev->data->dev_conf.lpbk_mode ==
> IXGBE_LPBK_TX_RX)
>  		hlreg0 |= IXGBE_HLREG0_LPBK;
>  	else
>  		hlreg0 &= ~IXGBE_HLREG0_LPBK;
> @@ -5088,6 +5092,29 @@ ixgbe_setup_loopback_link_82599(struct
> ixgbe_hw *hw)
>  	msec_delay(50);
>  }
> 
> +/*
> + * Set up link loopback for X540 / X550 mode Tx->Rx.
> + */
> +static inline void __attribute__((cold))
> +ixgbe_setup_loopback_link_x540_x550(struct ixgbe_hw *hw) {
> +	uint32_t macc;
> +	PMD_INIT_FUNC_TRACE();
> +
> +	/* datasheet 15.2.1: MACC.FLU = 1 (force link up) */
> +	macc = IXGBE_READ_REG(hw, IXGBE_MACC);
> +	macc |= IXGBE_MACC_FLU;
> +	IXGBE_WRITE_REG(hw, IXGBE_MACC, macc);
> +
> +	/* Restart link */
> +	IXGBE_WRITE_REG(hw,
> +			IXGBE_AUTOC,
> +			IXGBE_AUTOC_LMS_10G_LINK_NO_AN |
> IXGBE_AUTOC_FLU);
> +
> +	hw->mac.ops.release_swfw_sync(hw, IXGBE_GSSR_MAC_CSR_SM);
> +	msec_delay(50);
> +}
> +
> 
>  /*
>   * Start Transmit and Receive Units.
> @@ -5148,10 +5175,16 @@ ixgbe_dev_rxtx_start(struct rte_eth_dev *dev)
>  	rxctrl |= IXGBE_RXCTRL_RXEN;
>  	hw->mac.ops.enable_rx_dma(hw, rxctrl);
> 
> -	/* If loopback mode is enabled for 82599, set up the link accordingly
> */
> -	if (hw->mac.type == ixgbe_mac_82599EB &&
> -			dev->data->dev_conf.lpbk_mode ==
> IXGBE_LPBK_82599_TX_RX)
> -		ixgbe_setup_loopback_link_82599(hw);
> +	/* If loopback mode is enabled, set up the link accordingly */
> +	if (dev->data->dev_conf.lpbk_mode == IXGBE_LPBK_TX_RX) {
> +		if (hw->mac.type == ixgbe_mac_82599EB)
> +			ixgbe_setup_loopback_link_82599(hw);
> +		else if (hw->mac.type == ixgbe_mac_X540 ||
> +		     hw->mac.type == ixgbe_mac_X550 ||
> +		     hw->mac.type == ixgbe_mac_X550EM_x ||
> +		     hw->mac.type == ixgbe_mac_X550EM_a)
> +			ixgbe_setup_loopback_link_x540_x550(hw);
		Else
			Return -1;

SHOULD we add some branch here? 
In case some other NIC with configuration for tx->rx loop but pmd code do not support.

> +	}

> 
>  #ifdef RTE_LIBRTE_SECURITY
>  	if ((dev->data->dev_conf.rxmode.offloads &
> --
> 2.10.2

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [PATCH] net/ixgbe: add support of loopback for X540/X550
  2019-01-07  6:53 ` Zhang, Qi Z
@ 2019-01-07 15:53   ` Meunier, Julien (Nokia - FR/Paris-Saclay)
  2019-01-08 10:10     ` Zhao1, Wei
  0 siblings, 1 reply; 20+ messages in thread
From: Meunier, Julien (Nokia - FR/Paris-Saclay) @ 2019-01-07 15:53 UTC (permalink / raw)
  To: Zhang, Qi Z, Ananyev, Konstantin, Lu, Wenzhuo; +Cc: dev

Hi,

Mmm, you're right. However, as like for the 82599, the pmd skips the 
link configuration, so, it should not enable the autoneg.

During my tests, I had a 10G connectivity, and I didn't notice any 
problem. I used the DPDK application "test", with the test 
pmd_perf_autotest.

Should I need to modify my code to be sure that autoneg is disabled (and 
force it to 10G) ?

Thanks,
Best regards,
Julien Meunier

On 07/01/2019 07:53, Zhang, Qi Z wrote:
> 
> 
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Julien Meunier
>> Sent: Thursday, January 3, 2019 12:01 AM
>> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Lu, Wenzhuo
>> <wenzhuo.lu@intel.com>
>> Cc: dev@dpdk.org
>> Subject: [dpdk-dev] [PATCH] net/ixgbe: add support of loopback for X540/X550
>>
>> Loopback mode is also supported on X540 and X550 NICs, according to their
>> datasheet (section 15.2). The way to set it up is a little different of the 82599.
> 
> Thanks for enable this.
> 
> one question is, Datasheet also mentioned that auto negotiation should be disabled
> but I didn't see any related change with it.
> 
> Would you share more insight on this?
> 
>>
>> Signed-off-by: Julien Meunier <julien.meunier@nokia.com>
>> ---
>>   drivers/net/ixgbe/ixgbe_ethdev.c | 10 ++++++---
>> drivers/net/ixgbe/ixgbe_ethdev.h |  5 ++---
>>   drivers/net/ixgbe/ixgbe_rxtx.c   | 47
>> ++++++++++++++++++++++++++++++++++------
>>   3 files changed, 49 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
>> b/drivers/net/ixgbe/ixgbe_ethdev.c
>> index 7493110..7eb3303 100644
>> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
>> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
>> @@ -2652,9 +2652,13 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
>>   		goto error;
>>   	}
>>
>> -	/* Skip link setup if loopback mode is enabled for 82599. */
>> -	if (hw->mac.type == ixgbe_mac_82599EB &&
>> -			dev->data->dev_conf.lpbk_mode == IXGBE_LPBK_82599_TX_RX)
>> +	/* Skip link setup if loopback mode is enabled. */
>> +	if ((hw->mac.type == ixgbe_mac_82599EB ||
>> +	     hw->mac.type == ixgbe_mac_X540 ||
>> +	     hw->mac.type == ixgbe_mac_X550 ||
>> +	     hw->mac.type == ixgbe_mac_X550EM_x ||
>> +	     hw->mac.type == ixgbe_mac_X550EM_a) &&
>> +			dev->data->dev_conf.lpbk_mode == IXGBE_LPBK_TX_RX)
>>   		goto skip_link_setup;
>>
>>   	if (ixgbe_is_sfp(hw) && hw->phy.multispeed_fiber) { diff --git
>> a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
>> index 565c69c..c60a697 100644
>> --- a/drivers/net/ixgbe/ixgbe_ethdev.h
>> +++ b/drivers/net/ixgbe/ixgbe_ethdev.h
>> @@ -65,9 +65,8 @@
>>   #define IXGBE_QUEUE_ITR_INTERVAL_DEFAULT	500 /* 500us */
>>
>>   /* Loopback operation modes */
>> -/* 82599 specific loopback operation types */
>> -#define IXGBE_LPBK_82599_NONE   0x0 /* Default value. Loopback is disabled.
>> */
>> -#define IXGBE_LPBK_82599_TX_RX  0x1 /* Tx->Rx loopback operation is
>> enabled. */
>> +#define IXGBE_LPBK_NONE   0x0 /* Default value. Loopback is disabled. */
>> +#define IXGBE_LPBK_TX_RX  0x1 /* Tx->Rx loopback operation is enabled.
>> +*/
>>
>>   #define IXGBE_MAX_JUMBO_FRAME_SIZE      0x2600 /* Maximum Jumbo
>> frame size. */
>>
>> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
>> index 9a79d18..0ef7fdf 100644
>> --- a/drivers/net/ixgbe/ixgbe_rxtx.c
>> +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
>> @@ -4879,10 +4879,14 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
>>   		hlreg0 &= ~IXGBE_HLREG0_JUMBOEN;
>>
>>   	/*
>> -	 * If loopback mode is configured for 82599, set LPBK bit.
>> +	 * If loopback mode is configured, set LPBK bit.
>>   	 */
>> -	if (hw->mac.type == ixgbe_mac_82599EB &&
>> -			dev->data->dev_conf.lpbk_mode == IXGBE_LPBK_82599_TX_RX)
>> +	if ((hw->mac.type == ixgbe_mac_82599EB ||
>> +	     hw->mac.type == ixgbe_mac_X540 ||
>> +	     hw->mac.type == ixgbe_mac_X550 ||
>> +	     hw->mac.type == ixgbe_mac_X550EM_x ||
>> +	     hw->mac.type == ixgbe_mac_X550EM_a) &&
>> +			dev->data->dev_conf.lpbk_mode == IXGBE_LPBK_TX_RX)
>>   		hlreg0 |= IXGBE_HLREG0_LPBK;
>>   	else
>>   		hlreg0 &= ~IXGBE_HLREG0_LPBK;
>> @@ -5088,6 +5092,29 @@ ixgbe_setup_loopback_link_82599(struct ixgbe_hw
>> *hw)
>>   	msec_delay(50);
>>   }
>>
>> +/*
>> + * Set up link loopback for X540 / X550 mode Tx->Rx.
>> + */
>> +static inline void __attribute__((cold))
>> +ixgbe_setup_loopback_link_x540_x550(struct ixgbe_hw *hw) {
>> +	uint32_t macc;
>> +	PMD_INIT_FUNC_TRACE();
>> +
>> +	/* datasheet 15.2.1: MACC.FLU = 1 (force link up) */
>> +	macc = IXGBE_READ_REG(hw, IXGBE_MACC);
>> +	macc |= IXGBE_MACC_FLU;
>> +	IXGBE_WRITE_REG(hw, IXGBE_MACC, macc);
>> +
>> +	/* Restart link */
>> +	IXGBE_WRITE_REG(hw,
>> +			IXGBE_AUTOC,
>> +			IXGBE_AUTOC_LMS_10G_LINK_NO_AN | IXGBE_AUTOC_FLU);
>> +
>> +	hw->mac.ops.release_swfw_sync(hw, IXGBE_GSSR_MAC_CSR_SM);
>> +	msec_delay(50);
>> +}
>> +
>>
>>   /*
>>    * Start Transmit and Receive Units.
>> @@ -5148,10 +5175,16 @@ ixgbe_dev_rxtx_start(struct rte_eth_dev *dev)
>>   	rxctrl |= IXGBE_RXCTRL_RXEN;
>>   	hw->mac.ops.enable_rx_dma(hw, rxctrl);
>>
>> -	/* If loopback mode is enabled for 82599, set up the link accordingly */
>> -	if (hw->mac.type == ixgbe_mac_82599EB &&
>> -			dev->data->dev_conf.lpbk_mode == IXGBE_LPBK_82599_TX_RX)
>> -		ixgbe_setup_loopback_link_82599(hw);
>> +	/* If loopback mode is enabled, set up the link accordingly */
>> +	if (dev->data->dev_conf.lpbk_mode == IXGBE_LPBK_TX_RX) {
>> +		if (hw->mac.type == ixgbe_mac_82599EB)
>> +			ixgbe_setup_loopback_link_82599(hw);
>> +		else if (hw->mac.type == ixgbe_mac_X540 ||
>> +		     hw->mac.type == ixgbe_mac_X550 ||
>> +		     hw->mac.type == ixgbe_mac_X550EM_x ||
>> +		     hw->mac.type == ixgbe_mac_X550EM_a)
>> +			ixgbe_setup_loopback_link_x540_x550(hw);
>> +	}
>>
>>   #ifdef RTE_LIBRTE_SECURITY
>>   	if ((dev->data->dev_conf.rxmode.offloads &
>> --
>> 2.10.2
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [PATCH] net/ixgbe: add support of loopback for X540/X550
  2019-01-07 10:05 ` Zhao1, Wei
@ 2019-01-07 16:04   ` Meunier, Julien (Nokia - FR/Paris-Saclay)
  2019-01-08  3:10     ` Zhao1, Wei
  0 siblings, 1 reply; 20+ messages in thread
From: Meunier, Julien (Nokia - FR/Paris-Saclay) @ 2019-01-07 16:04 UTC (permalink / raw)
  To: Zhao1, Wei, Ananyev, Konstantin, Lu, Wenzhuo; +Cc: dev

Inline reply

On 07/01/2019 11:05, Zhao1, Wei wrote:
> 
> 
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Julien Meunier
>> Sent: Thursday, January 3, 2019 12:01 AM
>> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Lu, Wenzhuo
>> <wenzhuo.lu@intel.com>
>> Cc: dev@dpdk.org
>> Subject: [dpdk-dev] [PATCH] net/ixgbe: add support of loopback for
>> X540/X550
>>
>> Loopback mode is also supported on X540 and X550 NICs, according to their
>> datasheet (section 15.2). The way to set it up is a little different of the 82599.
>>
>> Signed-off-by: Julien Meunier <julien.meunier@nokia.com>
[...]

>> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
>> index 9a79d18..0ef7fdf 100644
>> --- a/drivers/net/ixgbe/ixgbe_rxtx.c
>> +++ b/drivers/net/ixgbe/ixgbe_rxtx.c

[...]

>>   /*
>>    * Start Transmit and Receive Units.
>> @@ -5148,10 +5175,16 @@ ixgbe_dev_rxtx_start(struct rte_eth_dev *dev)
>>   	rxctrl |= IXGBE_RXCTRL_RXEN;
>>   	hw->mac.ops.enable_rx_dma(hw, rxctrl);
>>
>> -	/* If loopback mode is enabled for 82599, set up the link accordingly
>> */
>> -	if (hw->mac.type == ixgbe_mac_82599EB &&
>> -			dev->data->dev_conf.lpbk_mode ==
>> IXGBE_LPBK_82599_TX_RX)
>> -		ixgbe_setup_loopback_link_82599(hw);
>> +	/* If loopback mode is enabled, set up the link accordingly */
>> +	if (dev->data->dev_conf.lpbk_mode == IXGBE_LPBK_TX_RX) {
>> +		if (hw->mac.type == ixgbe_mac_82599EB)
>> +			ixgbe_setup_loopback_link_82599(hw);
>> +		else if (hw->mac.type == ixgbe_mac_X540 ||
>> +		     hw->mac.type == ixgbe_mac_X550 ||
>> +		     hw->mac.type == ixgbe_mac_X550EM_x ||
>> +		     hw->mac.type == ixgbe_mac_X550EM_a)
>> +			ixgbe_setup_loopback_link_x540_x550(hw);
> 		Else
> 			Return -1;
> 
> SHOULD we add some branch here?
> In case some other NIC with configuration for tx->rx loop but pmd code do not support.

My patch is iso-functional: if a user sets the lpbk_mode to 0x1 
(IXGBE_LPBK_TX_RX), DPDK setups the TX -> RX loopback. Otherwise, it 
ignores the configuration.

Please remember that lpbk_mode is specific to each PMD.

in rte_ethdev.h:
	Loopback operation mode. By default the value is 0, meaning the
	loopback mode is disabled. Read the datasheet of given ethernet
	controller for details. The possible values of this field are
	defined in implementation of each driver.

Other PMD can implement TX -> RX with other value.

Do I need to add more check around this LPBK ?

> 
>> +	}
> 
>>
>>   #ifdef RTE_LIBRTE_SECURITY
>>   	if ((dev->data->dev_conf.rxmode.offloads &
>> --
>> 2.10.2
> 

Thanks,
Best regards,
Julien Meunier

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [PATCH] net/ixgbe: add support of loopback for X540/X550
  2019-01-07 16:04   ` Meunier, Julien (Nokia - FR/Paris-Saclay)
@ 2019-01-08  3:10     ` Zhao1, Wei
  0 siblings, 0 replies; 20+ messages in thread
From: Zhao1, Wei @ 2019-01-08  3:10 UTC (permalink / raw)
  To: Meunier, Julien (Nokia - FR/Paris-Saclay),
	Ananyev, Konstantin, Lu, Wenzhuo
  Cc: dev

Hi,Meunier, Julien

> -----Original Message-----
> From: Meunier, Julien (Nokia - FR/Paris-Saclay)
> [mailto:julien.meunier@nokia.com]
> Sent: Tuesday, January 8, 2019 12:05 AM
> To: Zhao1, Wei <wei.zhao1@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] net/ixgbe: add support of loopback for
> X540/X550
> 
> Inline reply
> 
> On 07/01/2019 11:05, Zhao1, Wei wrote:
> >
> >
> >> -----Original Message-----
> >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Julien Meunier
> >> Sent: Thursday, January 3, 2019 12:01 AM
> >> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Lu, Wenzhuo
> >> <wenzhuo.lu@intel.com>
> >> Cc: dev@dpdk.org
> >> Subject: [dpdk-dev] [PATCH] net/ixgbe: add support of loopback for
> >> X540/X550
> >>
> >> Loopback mode is also supported on X540 and X550 NICs, according to
> >> their datasheet (section 15.2). The way to set it up is a little different of
> the 82599.
> >>
> >> Signed-off-by: Julien Meunier <julien.meunier@nokia.com>
> [...]
> 
> >> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c
> >> b/drivers/net/ixgbe/ixgbe_rxtx.c index 9a79d18..0ef7fdf 100644
> >> --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> >> +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> 
> [...]
> 
> >>   /*
> >>    * Start Transmit and Receive Units.
> >> @@ -5148,10 +5175,16 @@ ixgbe_dev_rxtx_start(struct rte_eth_dev
> *dev)
> >>   	rxctrl |= IXGBE_RXCTRL_RXEN;
> >>   	hw->mac.ops.enable_rx_dma(hw, rxctrl);
> >>
> >> -	/* If loopback mode is enabled for 82599, set up the link accordingly
> >> */
> >> -	if (hw->mac.type == ixgbe_mac_82599EB &&
> >> -			dev->data->dev_conf.lpbk_mode ==
> >> IXGBE_LPBK_82599_TX_RX)
> >> -		ixgbe_setup_loopback_link_82599(hw);
> >> +	/* If loopback mode is enabled, set up the link accordingly */
> >> +	if (dev->data->dev_conf.lpbk_mode == IXGBE_LPBK_TX_RX) {
> >> +		if (hw->mac.type == ixgbe_mac_82599EB)
> >> +			ixgbe_setup_loopback_link_82599(hw);
> >> +		else if (hw->mac.type == ixgbe_mac_X540 ||
> >> +		     hw->mac.type == ixgbe_mac_X550 ||
> >> +		     hw->mac.type == ixgbe_mac_X550EM_x ||
> >> +		     hw->mac.type == ixgbe_mac_X550EM_a)
> >> +			ixgbe_setup_loopback_link_x540_x550(hw);
> > 		Else
> > 			Return -1;
> >
> > SHOULD we add some branch here?
> > In case some other NIC with configuration for tx->rx loop but pmd code do
> not support.
> 
> My patch is iso-functional: if a user sets the lpbk_mode to 0x1
> (IXGBE_LPBK_TX_RX), DPDK setups the TX -> RX loopback. Otherwise, it
> ignores the configuration.
> 
> Please remember that lpbk_mode is specific to each PMD.
> 
> in rte_ethdev.h:
> 	Loopback operation mode. By default the value is 0, meaning the
> 	loopback mode is disabled. Read the datasheet of given ethernet
> 	controller for details. The possible values of this field are
> 	defined in implementation of each driver.
> 
> Other PMD can implement TX -> RX with other value.
> 
> Do I need to add more check around this LPBK ?

Yes, 
"	Else
		Return -1;
"  
is need 
My concern is  such a case:
If some user is using 82598, and he set lpbk_mode to IXGBE_LPBK_TX_RX, 
In fact Our IXGBE PMD do not support loopback mode for 82598, but we do not return error for that invalid configuration!!
So, we need some check for NIC type and error reminder.


   
> 
> >
> >> +	}
> >
> >>
> >>   #ifdef RTE_LIBRTE_SECURITY
> >>   	if ((dev->data->dev_conf.rxmode.offloads &
> >> --
> >> 2.10.2
> >
> 
> Thanks,
> Best regards,
> Julien Meunier

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [PATCH] net/ixgbe: add support of loopback for X540/X550
  2019-01-07 15:53   ` Meunier, Julien (Nokia - FR/Paris-Saclay)
@ 2019-01-08 10:10     ` Zhao1, Wei
  2019-01-08 12:39       ` Zhang, Qi Z
  0 siblings, 1 reply; 20+ messages in thread
From: Zhao1, Wei @ 2019-01-08 10:10 UTC (permalink / raw)
  To: Meunier, Julien (Nokia - FR/Paris-Saclay),
	Zhang, Qi Z, Ananyev, Konstantin, Lu, Wenzhuo
  Cc: dev

Hi,  Meunier, Julien

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Meunier, Julien
> (Nokia - FR/Paris-Saclay)
> Sent: Monday, January 7, 2019 11:53 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] net/ixgbe: add support of loopback for
> X540/X550
> 
> Hi,
> 
> Mmm, you're right. However, as like for the 82599, the pmd skips the link
> configuration, so, it should not enable the autoneg.
> 
> During my tests, I had a 10G connectivity, and I didn't notice any problem. I
> used the DPDK application "test", with the test pmd_perf_autotest.

It seems you are right, autoneg is done in function of ixgbe_setup_link()->........ ()
which call function  ixgbe_setup_phy_link_speed_generic()->ixgbe_setup_phy_link ()->
ixgbe_setup_phy_link_generic (),  do the work for Auto-Negotiation,  but pmd code skip it.
But the bit C of Address 7.0 is initialized as 1 by default in datasheet,  what pmd code skip is just the 
Process of Auto-Negotiation, because we skip code of bellow In ixgbe_setup_phy_link_generic (), not disable bit 7.0.c.
So, you had better disable 7.0.c bit when mac loopback is enable.

"
	/* Restart PHY auto-negotiation. */
	hw->phy.ops.read_reg(hw, IXGBE_MDIO_AUTO_NEG_CONTROL,
			     IXGBE_MDIO_AUTO_NEG_DEV_TYPE, &autoneg_reg);

	autoneg_reg |= IXGBE_MII_RESTART;

	hw->phy.ops.write_reg(hw, IXGBE_MDIO_AUTO_NEG_CONTROL,
			      IXGBE_MDIO_AUTO_NEG_DEV_TYPE, autoneg_reg);
"


> 
> Should I need to modify my code to be sure that autoneg is disabled (and
> force it to 10G) ?
> 
> Thanks,
> Best regards,
> Julien Meunier
> 
> On 07/01/2019 07:53, Zhang, Qi Z wrote:
> >
> >
> >> -----Original Message-----
> >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Julien Meunier
> >> Sent: Thursday, January 3, 2019 12:01 AM
> >> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Lu, Wenzhuo
> >> <wenzhuo.lu@intel.com>
> >> Cc: dev@dpdk.org
> >> Subject: [dpdk-dev] [PATCH] net/ixgbe: add support of loopback for
> >> X540/X550
> >>
> >> Loopback mode is also supported on X540 and X550 NICs, according to
> >> their datasheet (section 15.2). The way to set it up is a little different of
> the 82599.
> >
> > Thanks for enable this.
> >
> > one question is, Datasheet also mentioned that auto negotiation should
> > be disabled but I didn't see any related change with it.
> >
> > Would you share more insight on this?
> >
> >>
> >> Signed-off-by: Julien Meunier <julien.meunier@nokia.com>
> >> ---
> >>   drivers/net/ixgbe/ixgbe_ethdev.c | 10 ++++++---
> >> drivers/net/ixgbe/ixgbe_ethdev.h |  5 ++---
> >>   drivers/net/ixgbe/ixgbe_rxtx.c   | 47
> >> ++++++++++++++++++++++++++++++++++------
> >>   3 files changed, 49 insertions(+), 13 deletions(-)
> >>
> >> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> >> b/drivers/net/ixgbe/ixgbe_ethdev.c
> >> index 7493110..7eb3303 100644
> >> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> >> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> >> @@ -2652,9 +2652,13 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
> >>   		goto error;
> >>   	}
> >>
> >> -	/* Skip link setup if loopback mode is enabled for 82599. */
> >> -	if (hw->mac.type == ixgbe_mac_82599EB &&
> >> -			dev->data->dev_conf.lpbk_mode ==
> IXGBE_LPBK_82599_TX_RX)
> >> +	/* Skip link setup if loopback mode is enabled. */
> >> +	if ((hw->mac.type == ixgbe_mac_82599EB ||
> >> +	     hw->mac.type == ixgbe_mac_X540 ||
> >> +	     hw->mac.type == ixgbe_mac_X550 ||
> >> +	     hw->mac.type == ixgbe_mac_X550EM_x ||
> >> +	     hw->mac.type == ixgbe_mac_X550EM_a) &&
> >> +			dev->data->dev_conf.lpbk_mode ==
> IXGBE_LPBK_TX_RX)
> >>   		goto skip_link_setup;
> >>
> >>   	if (ixgbe_is_sfp(hw) && hw->phy.multispeed_fiber) { diff --git
> >> a/drivers/net/ixgbe/ixgbe_ethdev.h
> b/drivers/net/ixgbe/ixgbe_ethdev.h
> >> index 565c69c..c60a697 100644
> >> --- a/drivers/net/ixgbe/ixgbe_ethdev.h
> >> +++ b/drivers/net/ixgbe/ixgbe_ethdev.h
> >> @@ -65,9 +65,8 @@
> >>   #define IXGBE_QUEUE_ITR_INTERVAL_DEFAULT	500 /* 500us */
> >>
> >>   /* Loopback operation modes */
> >> -/* 82599 specific loopback operation types */
> >> -#define IXGBE_LPBK_82599_NONE   0x0 /* Default value. Loopback is
> disabled.
> >> */
> >> -#define IXGBE_LPBK_82599_TX_RX  0x1 /* Tx->Rx loopback operation is
> >> enabled. */
> >> +#define IXGBE_LPBK_NONE   0x0 /* Default value. Loopback is disabled.
> */
> >> +#define IXGBE_LPBK_TX_RX  0x1 /* Tx->Rx loopback operation is
> enabled.
> >> +*/
> >>
> >>   #define IXGBE_MAX_JUMBO_FRAME_SIZE      0x2600 /* Maximum
> Jumbo
> >> frame size. */
> >>
> >> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c
> >> b/drivers/net/ixgbe/ixgbe_rxtx.c index 9a79d18..0ef7fdf 100644
> >> --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> >> +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> >> @@ -4879,10 +4879,14 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
> >>   		hlreg0 &= ~IXGBE_HLREG0_JUMBOEN;
> >>
> >>   	/*
> >> -	 * If loopback mode is configured for 82599, set LPBK bit.
> >> +	 * If loopback mode is configured, set LPBK bit.
> >>   	 */
> >> -	if (hw->mac.type == ixgbe_mac_82599EB &&
> >> -			dev->data->dev_conf.lpbk_mode ==
> IXGBE_LPBK_82599_TX_RX)
> >> +	if ((hw->mac.type == ixgbe_mac_82599EB ||
> >> +	     hw->mac.type == ixgbe_mac_X540 ||
> >> +	     hw->mac.type == ixgbe_mac_X550 ||
> >> +	     hw->mac.type == ixgbe_mac_X550EM_x ||
> >> +	     hw->mac.type == ixgbe_mac_X550EM_a) &&
> >> +			dev->data->dev_conf.lpbk_mode ==
> IXGBE_LPBK_TX_RX)
> >>   		hlreg0 |= IXGBE_HLREG0_LPBK;
> >>   	else
> >>   		hlreg0 &= ~IXGBE_HLREG0_LPBK;
> >> @@ -5088,6 +5092,29 @@ ixgbe_setup_loopback_link_82599(struct
> >> ixgbe_hw
> >> *hw)
> >>   	msec_delay(50);
> >>   }
> >>
> >> +/*
> >> + * Set up link loopback for X540 / X550 mode Tx->Rx.
> >> + */
> >> +static inline void __attribute__((cold))
> >> +ixgbe_setup_loopback_link_x540_x550(struct ixgbe_hw *hw) {
> >> +	uint32_t macc;
> >> +	PMD_INIT_FUNC_TRACE();
> >> +
> >> +	/* datasheet 15.2.1: MACC.FLU = 1 (force link up) */
> >> +	macc = IXGBE_READ_REG(hw, IXGBE_MACC);
> >> +	macc |= IXGBE_MACC_FLU;
> >> +	IXGBE_WRITE_REG(hw, IXGBE_MACC, macc);
> >> +
> >> +	/* Restart link */
> >> +	IXGBE_WRITE_REG(hw,
> >> +			IXGBE_AUTOC,
> >> +			IXGBE_AUTOC_LMS_10G_LINK_NO_AN |
> IXGBE_AUTOC_FLU);
> >> +
> >> +	hw->mac.ops.release_swfw_sync(hw, IXGBE_GSSR_MAC_CSR_SM);
> >> +	msec_delay(50);
> >> +}
> >> +
> >>
> >>   /*
> >>    * Start Transmit and Receive Units.
> >> @@ -5148,10 +5175,16 @@ ixgbe_dev_rxtx_start(struct rte_eth_dev
> *dev)
> >>   	rxctrl |= IXGBE_RXCTRL_RXEN;
> >>   	hw->mac.ops.enable_rx_dma(hw, rxctrl);
> >>
> >> -	/* If loopback mode is enabled for 82599, set up the link accordingly
> */
> >> -	if (hw->mac.type == ixgbe_mac_82599EB &&
> >> -			dev->data->dev_conf.lpbk_mode ==
> IXGBE_LPBK_82599_TX_RX)
> >> -		ixgbe_setup_loopback_link_82599(hw);
> >> +	/* If loopback mode is enabled, set up the link accordingly */
> >> +	if (dev->data->dev_conf.lpbk_mode == IXGBE_LPBK_TX_RX) {
> >> +		if (hw->mac.type == ixgbe_mac_82599EB)
> >> +			ixgbe_setup_loopback_link_82599(hw);
> >> +		else if (hw->mac.type == ixgbe_mac_X540 ||
> >> +		     hw->mac.type == ixgbe_mac_X550 ||
> >> +		     hw->mac.type == ixgbe_mac_X550EM_x ||
> >> +		     hw->mac.type == ixgbe_mac_X550EM_a)
> >> +			ixgbe_setup_loopback_link_x540_x550(hw);
> >> +	}
> >>
> >>   #ifdef RTE_LIBRTE_SECURITY
> >>   	if ((dev->data->dev_conf.rxmode.offloads &
> >> --
> >> 2.10.2
> >

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [PATCH] net/ixgbe: add support of loopback for X540/X550
  2019-01-08 10:10     ` Zhao1, Wei
@ 2019-01-08 12:39       ` Zhang, Qi Z
  2019-01-08 20:18         ` Meunier, Julien (Nokia - FR/Paris-Saclay)
  2019-01-09  1:30         ` Zhao1, Wei
  0 siblings, 2 replies; 20+ messages in thread
From: Zhang, Qi Z @ 2019-01-08 12:39 UTC (permalink / raw)
  To: Zhao1, Wei, Meunier, Julien (Nokia - FR/Paris-Saclay),
	Ananyev, Konstantin, Lu, Wenzhuo
  Cc: dev



> -----Original Message-----
> From: Zhao1, Wei
> Sent: Tuesday, January 8, 2019 6:10 PM
> To: Meunier, Julien (Nokia - FR/Paris-Saclay) <julien.meunier@nokia.com>;
> Zhang, Qi Z <qi.z.zhang@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>
> Cc: dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH] net/ixgbe: add support of loopback for
> X540/X550
> 
> Hi,  Meunier, Julien
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Meunier, Julien
> > (Nokia - FR/Paris-Saclay)
> > Sent: Monday, January 7, 2019 11:53 PM
> > To: Zhang, Qi Z <qi.z.zhang@intel.com>; Ananyev, Konstantin
> > <konstantin.ananyev@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>
> > Cc: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH] net/ixgbe: add support of loopback for
> > X540/X550
> >
> > Hi,
> >
> > Mmm, you're right. However, as like for the 82599, the pmd skips the
> > link configuration, so, it should not enable the autoneg.
> >
> > During my tests, I had a 10G connectivity, and I didn't notice any
> > problem. I used the DPDK application "test", with the test pmd_perf_autotest.
> 
> It seems you are right, autoneg is done in function of ixgbe_setup_link()->........ ()
> which call function
> ixgbe_setup_phy_link_speed_generic()->ixgbe_setup_phy_link ()->
> ixgbe_setup_phy_link_generic (),  do the work for Auto-Negotiation,  but pmd
> code skip it.
> But the bit C of Address 7.0 is initialized as 1 by default in datasheet,  what pmd
> code skip is just the Process of Auto-Negotiation, because we skip code of bellow
> In ixgbe_setup_phy_link_generic (), not disable bit 7.0.c.
> So, you had better disable 7.0.c bit when mac loopback is enable.
> 
> "
> 	/* Restart PHY auto-negotiation. */
> 	hw->phy.ops.read_reg(hw, IXGBE_MDIO_AUTO_NEG_CONTROL,
> 			     IXGBE_MDIO_AUTO_NEG_DEV_TYPE, &autoneg_reg);
> 
> 	autoneg_reg |= IXGBE_MII_RESTART;
> 
> 	hw->phy.ops.write_reg(hw, IXGBE_MDIO_AUTO_NEG_CONTROL,
> 			      IXGBE_MDIO_AUTO_NEG_DEV_TYPE, autoneg_reg); "
> 
> 

So, looks like PMD will not touch autoneg bit by default and we should not expected this bit always be on or off, because it can be changed by kernel driver before bind to dpdk.

probably a better solution could be 

during dev_start if loopback is required, we disable the bit to make sure loopback works and remember its original status, and during dev_stop we do roll back if necessary 

what do you think?


> > Should I need to modify my code to be sure that autoneg is disabled
> > (and force it to 10G) ?
> >
> > Thanks,
> > Best regards,
> > Julien Meunier
> >
> > On 07/01/2019 07:53, Zhang, Qi Z wrote:
> > >
> > >
> > >> -----Original Message-----
> > >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Julien Meunier
> > >> Sent: Thursday, January 3, 2019 12:01 AM
> > >> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Lu, Wenzhuo
> > >> <wenzhuo.lu@intel.com>
> > >> Cc: dev@dpdk.org
> > >> Subject: [dpdk-dev] [PATCH] net/ixgbe: add support of loopback for
> > >> X540/X550
> > >>
> > >> Loopback mode is also supported on X540 and X550 NICs, according to
> > >> their datasheet (section 15.2). The way to set it up is a little
> > >> different of
> > the 82599.
> > >
> > > Thanks for enable this.
> > >
> > > one question is, Datasheet also mentioned that auto negotiation
> > > should be disabled but I didn't see any related change with it.
> > >
> > > Would you share more insight on this?
> > >
> > >>
> > >> Signed-off-by: Julien Meunier <julien.meunier@nokia.com>
> > >> ---
> > >>   drivers/net/ixgbe/ixgbe_ethdev.c | 10 ++++++---
> > >> drivers/net/ixgbe/ixgbe_ethdev.h |  5 ++---
> > >>   drivers/net/ixgbe/ixgbe_rxtx.c   | 47
> > >> ++++++++++++++++++++++++++++++++++------
> > >>   3 files changed, 49 insertions(+), 13 deletions(-)
> > >>
> > >> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> > >> b/drivers/net/ixgbe/ixgbe_ethdev.c
> > >> index 7493110..7eb3303 100644
> > >> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> > >> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> > >> @@ -2652,9 +2652,13 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
> > >>   		goto error;
> > >>   	}
> > >>
> > >> -	/* Skip link setup if loopback mode is enabled for 82599. */
> > >> -	if (hw->mac.type == ixgbe_mac_82599EB &&
> > >> -			dev->data->dev_conf.lpbk_mode ==
> > IXGBE_LPBK_82599_TX_RX)
> > >> +	/* Skip link setup if loopback mode is enabled. */
> > >> +	if ((hw->mac.type == ixgbe_mac_82599EB ||
> > >> +	     hw->mac.type == ixgbe_mac_X540 ||
> > >> +	     hw->mac.type == ixgbe_mac_X550 ||
> > >> +	     hw->mac.type == ixgbe_mac_X550EM_x ||
> > >> +	     hw->mac.type == ixgbe_mac_X550EM_a) &&
> > >> +			dev->data->dev_conf.lpbk_mode ==
> > IXGBE_LPBK_TX_RX)
> > >>   		goto skip_link_setup;
> > >>
> > >>   	if (ixgbe_is_sfp(hw) && hw->phy.multispeed_fiber) { diff --git
> > >> a/drivers/net/ixgbe/ixgbe_ethdev.h
> > b/drivers/net/ixgbe/ixgbe_ethdev.h
> > >> index 565c69c..c60a697 100644
> > >> --- a/drivers/net/ixgbe/ixgbe_ethdev.h
> > >> +++ b/drivers/net/ixgbe/ixgbe_ethdev.h
> > >> @@ -65,9 +65,8 @@
> > >>   #define IXGBE_QUEUE_ITR_INTERVAL_DEFAULT	500 /* 500us */
> > >>
> > >>   /* Loopback operation modes */
> > >> -/* 82599 specific loopback operation types */
> > >> -#define IXGBE_LPBK_82599_NONE   0x0 /* Default value. Loopback is
> > disabled.
> > >> */
> > >> -#define IXGBE_LPBK_82599_TX_RX  0x1 /* Tx->Rx loopback operation
> > >> is enabled. */
> > >> +#define IXGBE_LPBK_NONE   0x0 /* Default value. Loopback is disabled.
> > */
> > >> +#define IXGBE_LPBK_TX_RX  0x1 /* Tx->Rx loopback operation is
> > enabled.
> > >> +*/
> > >>
> > >>   #define IXGBE_MAX_JUMBO_FRAME_SIZE      0x2600 /* Maximum
> > Jumbo
> > >> frame size. */
> > >>
> > >> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c
> > >> b/drivers/net/ixgbe/ixgbe_rxtx.c index 9a79d18..0ef7fdf 100644
> > >> --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> > >> +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> > >> @@ -4879,10 +4879,14 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
> > >>   		hlreg0 &= ~IXGBE_HLREG0_JUMBOEN;
> > >>
> > >>   	/*
> > >> -	 * If loopback mode is configured for 82599, set LPBK bit.
> > >> +	 * If loopback mode is configured, set LPBK bit.
> > >>   	 */
> > >> -	if (hw->mac.type == ixgbe_mac_82599EB &&
> > >> -			dev->data->dev_conf.lpbk_mode ==
> > IXGBE_LPBK_82599_TX_RX)
> > >> +	if ((hw->mac.type == ixgbe_mac_82599EB ||
> > >> +	     hw->mac.type == ixgbe_mac_X540 ||
> > >> +	     hw->mac.type == ixgbe_mac_X550 ||
> > >> +	     hw->mac.type == ixgbe_mac_X550EM_x ||
> > >> +	     hw->mac.type == ixgbe_mac_X550EM_a) &&
> > >> +			dev->data->dev_conf.lpbk_mode ==
> > IXGBE_LPBK_TX_RX)
> > >>   		hlreg0 |= IXGBE_HLREG0_LPBK;
> > >>   	else
> > >>   		hlreg0 &= ~IXGBE_HLREG0_LPBK;
> > >> @@ -5088,6 +5092,29 @@ ixgbe_setup_loopback_link_82599(struct
> > >> ixgbe_hw
> > >> *hw)
> > >>   	msec_delay(50);
> > >>   }
> > >>
> > >> +/*
> > >> + * Set up link loopback for X540 / X550 mode Tx->Rx.
> > >> + */
> > >> +static inline void __attribute__((cold))
> > >> +ixgbe_setup_loopback_link_x540_x550(struct ixgbe_hw *hw) {
> > >> +	uint32_t macc;
> > >> +	PMD_INIT_FUNC_TRACE();
> > >> +
> > >> +	/* datasheet 15.2.1: MACC.FLU = 1 (force link up) */
> > >> +	macc = IXGBE_READ_REG(hw, IXGBE_MACC);
> > >> +	macc |= IXGBE_MACC_FLU;
> > >> +	IXGBE_WRITE_REG(hw, IXGBE_MACC, macc);
> > >> +
> > >> +	/* Restart link */
> > >> +	IXGBE_WRITE_REG(hw,
> > >> +			IXGBE_AUTOC,
> > >> +			IXGBE_AUTOC_LMS_10G_LINK_NO_AN |
> > IXGBE_AUTOC_FLU);
> > >> +
> > >> +	hw->mac.ops.release_swfw_sync(hw, IXGBE_GSSR_MAC_CSR_SM);
> > >> +	msec_delay(50);
> > >> +}
> > >> +
> > >>
> > >>   /*
> > >>    * Start Transmit and Receive Units.
> > >> @@ -5148,10 +5175,16 @@ ixgbe_dev_rxtx_start(struct rte_eth_dev
> > *dev)
> > >>   	rxctrl |= IXGBE_RXCTRL_RXEN;
> > >>   	hw->mac.ops.enable_rx_dma(hw, rxctrl);
> > >>
> > >> -	/* If loopback mode is enabled for 82599, set up the link accordingly
> > */
> > >> -	if (hw->mac.type == ixgbe_mac_82599EB &&
> > >> -			dev->data->dev_conf.lpbk_mode ==
> > IXGBE_LPBK_82599_TX_RX)
> > >> -		ixgbe_setup_loopback_link_82599(hw);
> > >> +	/* If loopback mode is enabled, set up the link accordingly */
> > >> +	if (dev->data->dev_conf.lpbk_mode == IXGBE_LPBK_TX_RX) {
> > >> +		if (hw->mac.type == ixgbe_mac_82599EB)
> > >> +			ixgbe_setup_loopback_link_82599(hw);
> > >> +		else if (hw->mac.type == ixgbe_mac_X540 ||
> > >> +		     hw->mac.type == ixgbe_mac_X550 ||
> > >> +		     hw->mac.type == ixgbe_mac_X550EM_x ||
> > >> +		     hw->mac.type == ixgbe_mac_X550EM_a)
> > >> +			ixgbe_setup_loopback_link_x540_x550(hw);
> > >> +	}
> > >>
> > >>   #ifdef RTE_LIBRTE_SECURITY
> > >>   	if ((dev->data->dev_conf.rxmode.offloads &
> > >> --
> > >> 2.10.2
> > >

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [PATCH] net/ixgbe: add support of loopback for X540/X550
  2019-01-08 12:39       ` Zhang, Qi Z
@ 2019-01-08 20:18         ` Meunier, Julien (Nokia - FR/Paris-Saclay)
  2019-01-09  1:30         ` Zhao1, Wei
  1 sibling, 0 replies; 20+ messages in thread
From: Meunier, Julien (Nokia - FR/Paris-Saclay) @ 2019-01-08 20:18 UTC (permalink / raw)
  To: Zhang, Qi Z, Zhao1, Wei, Ananyev, Konstantin, Lu, Wenzhuo; +Cc: dev

Hi Zhang, Qi Z / Zhao1, Wei,

Thanks for your comments.

I will try to submit this week a new pachset based on your 
recommandation (check LPBK feature depending of the model + disable the 
bit in the PHY register)

Best regards,
Julien Meunier

On 08/01/2019 13:39, Zhang, Qi Z wrote:
> 
> 
>> -----Original Message-----
>> From: Zhao1, Wei
>> Sent: Tuesday, January 8, 2019 6:10 PM
>> To: Meunier, Julien (Nokia - FR/Paris-Saclay) <julien.meunier@nokia.com>;
>> Zhang, Qi Z <qi.z.zhang@intel.com>; Ananyev, Konstantin
>> <konstantin.ananyev@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>
>> Cc: dev@dpdk.org
>> Subject: RE: [dpdk-dev] [PATCH] net/ixgbe: add support of loopback for
>> X540/X550
>>
>> Hi,  Meunier, Julien
>>
>>> -----Original Message-----
>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Meunier, Julien
>>> (Nokia - FR/Paris-Saclay)
>>> Sent: Monday, January 7, 2019 11:53 PM
>>> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Ananyev, Konstantin
>>> <konstantin.ananyev@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>
>>> Cc: dev@dpdk.org
>>> Subject: Re: [dpdk-dev] [PATCH] net/ixgbe: add support of loopback for
>>> X540/X550
>>>
>>> Hi,
>>>
>>> Mmm, you're right. However, as like for the 82599, the pmd skips the
>>> link configuration, so, it should not enable the autoneg.
>>>
>>> During my tests, I had a 10G connectivity, and I didn't notice any
>>> problem. I used the DPDK application "test", with the test pmd_perf_autotest.
>>
>> It seems you are right, autoneg is done in function of ixgbe_setup_link()->........ ()
>> which call function
>> ixgbe_setup_phy_link_speed_generic()->ixgbe_setup_phy_link ()->
>> ixgbe_setup_phy_link_generic (),  do the work for Auto-Negotiation,  but pmd
>> code skip it.
>> But the bit C of Address 7.0 is initialized as 1 by default in datasheet,  what pmd
>> code skip is just the Process of Auto-Negotiation, because we skip code of bellow
>> In ixgbe_setup_phy_link_generic (), not disable bit 7.0.c.
>> So, you had better disable 7.0.c bit when mac loopback is enable.
>>
>> "
>> 	/* Restart PHY auto-negotiation. */
>> 	hw->phy.ops.read_reg(hw, IXGBE_MDIO_AUTO_NEG_CONTROL,
>> 			     IXGBE_MDIO_AUTO_NEG_DEV_TYPE, &autoneg_reg);
>>
>> 	autoneg_reg |= IXGBE_MII_RESTART;
>>
>> 	hw->phy.ops.write_reg(hw, IXGBE_MDIO_AUTO_NEG_CONTROL,
>> 			      IXGBE_MDIO_AUTO_NEG_DEV_TYPE, autoneg_reg); "
>>
>>
> 
> So, looks like PMD will not touch autoneg bit by default and we should not expected this bit always be on or off, because it can be changed by kernel driver before bind to dpdk.
> 
> probably a better solution could be
> 
> during dev_start if loopback is required, we disable the bit to make sure loopback works and remember its original status, and during dev_stop we do roll back if necessary
> 
> what do you think?
> 
> 
>>> Should I need to modify my code to be sure that autoneg is disabled
>>> (and force it to 10G) ?
>>>
>>> Thanks,
>>> Best regards,
>>> Julien Meunier
>>>
>>> On 07/01/2019 07:53, Zhang, Qi Z wrote:
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Julien Meunier
>>>>> Sent: Thursday, January 3, 2019 12:01 AM
>>>>> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Lu, Wenzhuo
>>>>> <wenzhuo.lu@intel.com>
>>>>> Cc: dev@dpdk.org
>>>>> Subject: [dpdk-dev] [PATCH] net/ixgbe: add support of loopback for
>>>>> X540/X550
>>>>>
>>>>> Loopback mode is also supported on X540 and X550 NICs, according to
>>>>> their datasheet (section 15.2). The way to set it up is a little
>>>>> different of
>>> the 82599.
>>>>
>>>> Thanks for enable this.
>>>>
>>>> one question is, Datasheet also mentioned that auto negotiation
>>>> should be disabled but I didn't see any related change with it.
>>>>
>>>> Would you share more insight on this?
>>>>
>>>>>
>>>>> Signed-off-by: Julien Meunier <julien.meunier@nokia.com>
>>>>> ---
>>>>>    drivers/net/ixgbe/ixgbe_ethdev.c | 10 ++++++---
>>>>> drivers/net/ixgbe/ixgbe_ethdev.h |  5 ++---
>>>>>    drivers/net/ixgbe/ixgbe_rxtx.c   | 47
>>>>> ++++++++++++++++++++++++++++++++++------
>>>>>    3 files changed, 49 insertions(+), 13 deletions(-)
>>>>>
>>>>> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
>>>>> b/drivers/net/ixgbe/ixgbe_ethdev.c
>>>>> index 7493110..7eb3303 100644
>>>>> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
>>>>> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
>>>>> @@ -2652,9 +2652,13 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
>>>>>    		goto error;
>>>>>    	}
>>>>>
>>>>> -	/* Skip link setup if loopback mode is enabled for 82599. */
>>>>> -	if (hw->mac.type == ixgbe_mac_82599EB &&
>>>>> -			dev->data->dev_conf.lpbk_mode ==
>>> IXGBE_LPBK_82599_TX_RX)
>>>>> +	/* Skip link setup if loopback mode is enabled. */
>>>>> +	if ((hw->mac.type == ixgbe_mac_82599EB ||
>>>>> +	     hw->mac.type == ixgbe_mac_X540 ||
>>>>> +	     hw->mac.type == ixgbe_mac_X550 ||
>>>>> +	     hw->mac.type == ixgbe_mac_X550EM_x ||
>>>>> +	     hw->mac.type == ixgbe_mac_X550EM_a) &&
>>>>> +			dev->data->dev_conf.lpbk_mode ==
>>> IXGBE_LPBK_TX_RX)
>>>>>    		goto skip_link_setup;
>>>>>
>>>>>    	if (ixgbe_is_sfp(hw) && hw->phy.multispeed_fiber) { diff --git
>>>>> a/drivers/net/ixgbe/ixgbe_ethdev.h
>>> b/drivers/net/ixgbe/ixgbe_ethdev.h
>>>>> index 565c69c..c60a697 100644
>>>>> --- a/drivers/net/ixgbe/ixgbe_ethdev.h
>>>>> +++ b/drivers/net/ixgbe/ixgbe_ethdev.h
>>>>> @@ -65,9 +65,8 @@
>>>>>    #define IXGBE_QUEUE_ITR_INTERVAL_DEFAULT	500 /* 500us */
>>>>>
>>>>>    /* Loopback operation modes */
>>>>> -/* 82599 specific loopback operation types */
>>>>> -#define IXGBE_LPBK_82599_NONE   0x0 /* Default value. Loopback is
>>> disabled.
>>>>> */
>>>>> -#define IXGBE_LPBK_82599_TX_RX  0x1 /* Tx->Rx loopback operation
>>>>> is enabled. */
>>>>> +#define IXGBE_LPBK_NONE   0x0 /* Default value. Loopback is disabled.
>>> */
>>>>> +#define IXGBE_LPBK_TX_RX  0x1 /* Tx->Rx loopback operation is
>>> enabled.
>>>>> +*/
>>>>>
>>>>>    #define IXGBE_MAX_JUMBO_FRAME_SIZE      0x2600 /* Maximum
>>> Jumbo
>>>>> frame size. */
>>>>>
>>>>> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c
>>>>> b/drivers/net/ixgbe/ixgbe_rxtx.c index 9a79d18..0ef7fdf 100644
>>>>> --- a/drivers/net/ixgbe/ixgbe_rxtx.c
>>>>> +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
>>>>> @@ -4879,10 +4879,14 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
>>>>>    		hlreg0 &= ~IXGBE_HLREG0_JUMBOEN;
>>>>>
>>>>>    	/*
>>>>> -	 * If loopback mode is configured for 82599, set LPBK bit.
>>>>> +	 * If loopback mode is configured, set LPBK bit.
>>>>>    	 */
>>>>> -	if (hw->mac.type == ixgbe_mac_82599EB &&
>>>>> -			dev->data->dev_conf.lpbk_mode ==
>>> IXGBE_LPBK_82599_TX_RX)
>>>>> +	if ((hw->mac.type == ixgbe_mac_82599EB ||
>>>>> +	     hw->mac.type == ixgbe_mac_X540 ||
>>>>> +	     hw->mac.type == ixgbe_mac_X550 ||
>>>>> +	     hw->mac.type == ixgbe_mac_X550EM_x ||
>>>>> +	     hw->mac.type == ixgbe_mac_X550EM_a) &&
>>>>> +			dev->data->dev_conf.lpbk_mode ==
>>> IXGBE_LPBK_TX_RX)
>>>>>    		hlreg0 |= IXGBE_HLREG0_LPBK;
>>>>>    	else
>>>>>    		hlreg0 &= ~IXGBE_HLREG0_LPBK;
>>>>> @@ -5088,6 +5092,29 @@ ixgbe_setup_loopback_link_82599(struct
>>>>> ixgbe_hw
>>>>> *hw)
>>>>>    	msec_delay(50);
>>>>>    }
>>>>>
>>>>> +/*
>>>>> + * Set up link loopback for X540 / X550 mode Tx->Rx.
>>>>> + */
>>>>> +static inline void __attribute__((cold))
>>>>> +ixgbe_setup_loopback_link_x540_x550(struct ixgbe_hw *hw) {
>>>>> +	uint32_t macc;
>>>>> +	PMD_INIT_FUNC_TRACE();
>>>>> +
>>>>> +	/* datasheet 15.2.1: MACC.FLU = 1 (force link up) */
>>>>> +	macc = IXGBE_READ_REG(hw, IXGBE_MACC);
>>>>> +	macc |= IXGBE_MACC_FLU;
>>>>> +	IXGBE_WRITE_REG(hw, IXGBE_MACC, macc);
>>>>> +
>>>>> +	/* Restart link */
>>>>> +	IXGBE_WRITE_REG(hw,
>>>>> +			IXGBE_AUTOC,
>>>>> +			IXGBE_AUTOC_LMS_10G_LINK_NO_AN |
>>> IXGBE_AUTOC_FLU);
>>>>> +
>>>>> +	hw->mac.ops.release_swfw_sync(hw, IXGBE_GSSR_MAC_CSR_SM);
>>>>> +	msec_delay(50);
>>>>> +}
>>>>> +
>>>>>
>>>>>    /*
>>>>>     * Start Transmit and Receive Units.
>>>>> @@ -5148,10 +5175,16 @@ ixgbe_dev_rxtx_start(struct rte_eth_dev
>>> *dev)
>>>>>    	rxctrl |= IXGBE_RXCTRL_RXEN;
>>>>>    	hw->mac.ops.enable_rx_dma(hw, rxctrl);
>>>>>
>>>>> -	/* If loopback mode is enabled for 82599, set up the link accordingly
>>> */
>>>>> -	if (hw->mac.type == ixgbe_mac_82599EB &&
>>>>> -			dev->data->dev_conf.lpbk_mode ==
>>> IXGBE_LPBK_82599_TX_RX)
>>>>> -		ixgbe_setup_loopback_link_82599(hw);
>>>>> +	/* If loopback mode is enabled, set up the link accordingly */
>>>>> +	if (dev->data->dev_conf.lpbk_mode == IXGBE_LPBK_TX_RX) {
>>>>> +		if (hw->mac.type == ixgbe_mac_82599EB)
>>>>> +			ixgbe_setup_loopback_link_82599(hw);
>>>>> +		else if (hw->mac.type == ixgbe_mac_X540 ||
>>>>> +		     hw->mac.type == ixgbe_mac_X550 ||
>>>>> +		     hw->mac.type == ixgbe_mac_X550EM_x ||
>>>>> +		     hw->mac.type == ixgbe_mac_X550EM_a)
>>>>> +			ixgbe_setup_loopback_link_x540_x550(hw);
>>>>> +	}
>>>>>
>>>>>    #ifdef RTE_LIBRTE_SECURITY
>>>>>    	if ((dev->data->dev_conf.rxmode.offloads &
>>>>> --
>>>>> 2.10.2
>>>>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [PATCH] net/ixgbe: add support of loopback for X540/X550
  2019-01-08 12:39       ` Zhang, Qi Z
  2019-01-08 20:18         ` Meunier, Julien (Nokia - FR/Paris-Saclay)
@ 2019-01-09  1:30         ` Zhao1, Wei
  1 sibling, 0 replies; 20+ messages in thread
From: Zhao1, Wei @ 2019-01-09  1:30 UTC (permalink / raw)
  To: Zhang, Qi Z, Meunier, Julien (Nokia - FR/Paris-Saclay),
	Ananyev, Konstantin, Lu, Wenzhuo
  Cc: dev



> -----Original Message-----
> From: Zhang, Qi Z
> Sent: Tuesday, January 8, 2019 8:39 PM
> To: Zhao1, Wei <wei.zhao1@intel.com>; Meunier, Julien (Nokia - FR/Paris-
> Saclay) <julien.meunier@nokia.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>
> Cc: dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH] net/ixgbe: add support of loopback for
> X540/X550
> 
> 
> 
> > -----Original Message-----
> > From: Zhao1, Wei
> > Sent: Tuesday, January 8, 2019 6:10 PM
> > To: Meunier, Julien (Nokia - FR/Paris-Saclay)
> > <julien.meunier@nokia.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
> > Ananyev, Konstantin <konstantin.ananyev@intel.com>; Lu, Wenzhuo
> > <wenzhuo.lu@intel.com>
> > Cc: dev@dpdk.org
> > Subject: RE: [dpdk-dev] [PATCH] net/ixgbe: add support of loopback for
> > X540/X550
> >
> > Hi,  Meunier, Julien
> >
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Meunier, Julien
> > > (Nokia - FR/Paris-Saclay)
> > > Sent: Monday, January 7, 2019 11:53 PM
> > > To: Zhang, Qi Z <qi.z.zhang@intel.com>; Ananyev, Konstantin
> > > <konstantin.ananyev@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>
> > > Cc: dev@dpdk.org
> > > Subject: Re: [dpdk-dev] [PATCH] net/ixgbe: add support of loopback
> > > for
> > > X540/X550
> > >
> > > Hi,
> > >
> > > Mmm, you're right. However, as like for the 82599, the pmd skips the
> > > link configuration, so, it should not enable the autoneg.
> > >
> > > During my tests, I had a 10G connectivity, and I didn't notice any
> > > problem. I used the DPDK application "test", with the test
> pmd_perf_autotest.
> >
> > It seems you are right, autoneg is done in function of
> > ixgbe_setup_link()->........ () which call function
> > ixgbe_setup_phy_link_speed_generic()->ixgbe_setup_phy_link ()->
> > ixgbe_setup_phy_link_generic (),  do the work for Auto-Negotiation,
> > but pmd code skip it.
> > But the bit C of Address 7.0 is initialized as 1 by default in
> > datasheet,  what pmd code skip is just the Process of
> > Auto-Negotiation, because we skip code of bellow In
> ixgbe_setup_phy_link_generic (), not disable bit 7.0.c.
> > So, you had better disable 7.0.c bit when mac loopback is enable.
> >
> > "
> > 	/* Restart PHY auto-negotiation. */
> > 	hw->phy.ops.read_reg(hw, IXGBE_MDIO_AUTO_NEG_CONTROL,
> > 			     IXGBE_MDIO_AUTO_NEG_DEV_TYPE,
> &autoneg_reg);
> >
> > 	autoneg_reg |= IXGBE_MII_RESTART;
> >
> > 	hw->phy.ops.write_reg(hw, IXGBE_MDIO_AUTO_NEG_CONTROL,
> > 			      IXGBE_MDIO_AUTO_NEG_DEV_TYPE,
> autoneg_reg); "
> >
> >
> 
> So, looks like PMD will not touch autoneg bit by default and we should not
> expected this bit always be on or off, because it can be changed by kernel
> driver before bind to dpdk.
> 
> probably a better solution could be
> 
> during dev_start if loopback is required, we disable the bit to make sure
> loopback works and remember its original status, and during dev_stop we do
> roll back if necessary
> 
> what do you think?

Yes, agree with you.

> 
> 
> > > Should I need to modify my code to be sure that autoneg is disabled
> > > (and force it to 10G) ?
> > >
> > > Thanks,
> > > Best regards,
> > > Julien Meunier
> > >
> > > On 07/01/2019 07:53, Zhang, Qi Z wrote:
> > > >
> > > >
> > > >> -----Original Message-----
> > > >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Julien
> > > >> Meunier
> > > >> Sent: Thursday, January 3, 2019 12:01 AM
> > > >> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Lu,
> > > >> Wenzhuo <wenzhuo.lu@intel.com>
> > > >> Cc: dev@dpdk.org
> > > >> Subject: [dpdk-dev] [PATCH] net/ixgbe: add support of loopback
> > > >> for
> > > >> X540/X550
> > > >>
> > > >> Loopback mode is also supported on X540 and X550 NICs, according
> > > >> to their datasheet (section 15.2). The way to set it up is a
> > > >> little different of
> > > the 82599.
> > > >
> > > > Thanks for enable this.
> > > >
> > > > one question is, Datasheet also mentioned that auto negotiation
> > > > should be disabled but I didn't see any related change with it.
> > > >
> > > > Would you share more insight on this?
> > > >
> > > >>
> > > >> Signed-off-by: Julien Meunier <julien.meunier@nokia.com>
> > > >> ---
> > > >>   drivers/net/ixgbe/ixgbe_ethdev.c | 10 ++++++---
> > > >> drivers/net/ixgbe/ixgbe_ethdev.h |  5 ++---
> > > >>   drivers/net/ixgbe/ixgbe_rxtx.c   | 47
> > > >> ++++++++++++++++++++++++++++++++++------
> > > >>   3 files changed, 49 insertions(+), 13 deletions(-)
> > > >>
> > > >> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> > > >> b/drivers/net/ixgbe/ixgbe_ethdev.c
> > > >> index 7493110..7eb3303 100644
> > > >> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> > > >> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> > > >> @@ -2652,9 +2652,13 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
> > > >>   		goto error;
> > > >>   	}
> > > >>
> > > >> -	/* Skip link setup if loopback mode is enabled for 82599. */
> > > >> -	if (hw->mac.type == ixgbe_mac_82599EB &&
> > > >> -			dev->data->dev_conf.lpbk_mode ==
> > > IXGBE_LPBK_82599_TX_RX)
> > > >> +	/* Skip link setup if loopback mode is enabled. */
> > > >> +	if ((hw->mac.type == ixgbe_mac_82599EB ||
> > > >> +	     hw->mac.type == ixgbe_mac_X540 ||
> > > >> +	     hw->mac.type == ixgbe_mac_X550 ||
> > > >> +	     hw->mac.type == ixgbe_mac_X550EM_x ||
> > > >> +	     hw->mac.type == ixgbe_mac_X550EM_a) &&
> > > >> +			dev->data->dev_conf.lpbk_mode ==
> > > IXGBE_LPBK_TX_RX)
> > > >>   		goto skip_link_setup;
> > > >>
> > > >>   	if (ixgbe_is_sfp(hw) && hw->phy.multispeed_fiber) { diff --
> git
> > > >> a/drivers/net/ixgbe/ixgbe_ethdev.h
> > > b/drivers/net/ixgbe/ixgbe_ethdev.h
> > > >> index 565c69c..c60a697 100644
> > > >> --- a/drivers/net/ixgbe/ixgbe_ethdev.h
> > > >> +++ b/drivers/net/ixgbe/ixgbe_ethdev.h
> > > >> @@ -65,9 +65,8 @@
> > > >>   #define IXGBE_QUEUE_ITR_INTERVAL_DEFAULT	500 /* 500us
> */
> > > >>
> > > >>   /* Loopback operation modes */
> > > >> -/* 82599 specific loopback operation types */
> > > >> -#define IXGBE_LPBK_82599_NONE   0x0 /* Default value. Loopback is
> > > disabled.
> > > >> */
> > > >> -#define IXGBE_LPBK_82599_TX_RX  0x1 /* Tx->Rx loopback
> operation
> > > >> is enabled. */
> > > >> +#define IXGBE_LPBK_NONE   0x0 /* Default value. Loopback is
> disabled.
> > > */
> > > >> +#define IXGBE_LPBK_TX_RX  0x1 /* Tx->Rx loopback operation is
> > > enabled.
> > > >> +*/
> > > >>
> > > >>   #define IXGBE_MAX_JUMBO_FRAME_SIZE      0x2600 /* Maximum
> > > Jumbo
> > > >> frame size. */
> > > >>
> > > >> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c
> > > >> b/drivers/net/ixgbe/ixgbe_rxtx.c index 9a79d18..0ef7fdf 100644
> > > >> --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> > > >> +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> > > >> @@ -4879,10 +4879,14 @@ ixgbe_dev_rx_init(struct rte_eth_dev
> *dev)
> > > >>   		hlreg0 &= ~IXGBE_HLREG0_JUMBOEN;
> > > >>
> > > >>   	/*
> > > >> -	 * If loopback mode is configured for 82599, set LPBK bit.
> > > >> +	 * If loopback mode is configured, set LPBK bit.
> > > >>   	 */
> > > >> -	if (hw->mac.type == ixgbe_mac_82599EB &&
> > > >> -			dev->data->dev_conf.lpbk_mode ==
> > > IXGBE_LPBK_82599_TX_RX)
> > > >> +	if ((hw->mac.type == ixgbe_mac_82599EB ||
> > > >> +	     hw->mac.type == ixgbe_mac_X540 ||
> > > >> +	     hw->mac.type == ixgbe_mac_X550 ||
> > > >> +	     hw->mac.type == ixgbe_mac_X550EM_x ||
> > > >> +	     hw->mac.type == ixgbe_mac_X550EM_a) &&
> > > >> +			dev->data->dev_conf.lpbk_mode ==
> > > IXGBE_LPBK_TX_RX)
> > > >>   		hlreg0 |= IXGBE_HLREG0_LPBK;
> > > >>   	else
> > > >>   		hlreg0 &= ~IXGBE_HLREG0_LPBK; @@ -5088,6
> +5092,29 @@
> > > >> ixgbe_setup_loopback_link_82599(struct
> > > >> ixgbe_hw
> > > >> *hw)
> > > >>   	msec_delay(50);
> > > >>   }
> > > >>
> > > >> +/*
> > > >> + * Set up link loopback for X540 / X550 mode Tx->Rx.
> > > >> + */
> > > >> +static inline void __attribute__((cold))
> > > >> +ixgbe_setup_loopback_link_x540_x550(struct ixgbe_hw *hw) {
> > > >> +	uint32_t macc;
> > > >> +	PMD_INIT_FUNC_TRACE();
> > > >> +
> > > >> +	/* datasheet 15.2.1: MACC.FLU = 1 (force link up) */
> > > >> +	macc = IXGBE_READ_REG(hw, IXGBE_MACC);
> > > >> +	macc |= IXGBE_MACC_FLU;
> > > >> +	IXGBE_WRITE_REG(hw, IXGBE_MACC, macc);
> > > >> +
> > > >> +	/* Restart link */
> > > >> +	IXGBE_WRITE_REG(hw,
> > > >> +			IXGBE_AUTOC,
> > > >> +			IXGBE_AUTOC_LMS_10G_LINK_NO_AN |
> > > IXGBE_AUTOC_FLU);
> > > >> +
> > > >> +	hw->mac.ops.release_swfw_sync(hw,
> IXGBE_GSSR_MAC_CSR_SM);
> > > >> +	msec_delay(50);
> > > >> +}
> > > >> +
> > > >>
> > > >>   /*
> > > >>    * Start Transmit and Receive Units.
> > > >> @@ -5148,10 +5175,16 @@ ixgbe_dev_rxtx_start(struct rte_eth_dev
> > > *dev)
> > > >>   	rxctrl |= IXGBE_RXCTRL_RXEN;
> > > >>   	hw->mac.ops.enable_rx_dma(hw, rxctrl);
> > > >>
> > > >> -	/* If loopback mode is enabled for 82599, set up the link accordingly
> > > */
> > > >> -	if (hw->mac.type == ixgbe_mac_82599EB &&
> > > >> -			dev->data->dev_conf.lpbk_mode ==
> > > IXGBE_LPBK_82599_TX_RX)
> > > >> -		ixgbe_setup_loopback_link_82599(hw);
> > > >> +	/* If loopback mode is enabled, set up the link accordingly */
> > > >> +	if (dev->data->dev_conf.lpbk_mode == IXGBE_LPBK_TX_RX)
> {
> > > >> +		if (hw->mac.type == ixgbe_mac_82599EB)
> > > >> +			ixgbe_setup_loopback_link_82599(hw);
> > > >> +		else if (hw->mac.type == ixgbe_mac_X540 ||
> > > >> +		     hw->mac.type == ixgbe_mac_X550 ||
> > > >> +		     hw->mac.type == ixgbe_mac_X550EM_x ||
> > > >> +		     hw->mac.type == ixgbe_mac_X550EM_a)
> > > >> +			ixgbe_setup_loopback_link_x540_x550(hw);
> > > >> +	}
> > > >>
> > > >>   #ifdef RTE_LIBRTE_SECURITY
> > > >>   	if ((dev->data->dev_conf.rxmode.offloads &
> > > >> --
> > > >> 2.10.2
> > > >

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [dpdk-dev] [PATCH 1/2] net/ixgbe: do not start on unsupported loopback mode
  2019-01-02 16:00 [dpdk-dev] [PATCH] net/ixgbe: add support of loopback for X540/X550 Julien Meunier
  2019-01-07  6:53 ` Zhang, Qi Z
  2019-01-07 10:05 ` Zhao1, Wei
@ 2019-02-07 17:30 ` Julien Meunier
  2019-02-07 17:30   ` [dpdk-dev] [PATCH 2/2] net/ixgbe: add support of loopback for X540/X550 Julien Meunier
  2019-02-12  6:36   ` [dpdk-dev] [PATCH 1/2] net/ixgbe: do not start on unsupported loopback mode Zhao1, Wei
  2019-02-20 21:05 ` [dpdk-dev] [PATCH v3 " Julien Meunier
  3 siblings, 2 replies; 20+ messages in thread
From: Julien Meunier @ 2019-02-07 17:30 UTC (permalink / raw)
  To: konstantin.ananyev, wenzhuo.lu, wei.zhao1, qi.z.zhang; +Cc: dev

Only TX->RX loopback is supported currently on 82599EB. If a user wants
to apply an another loopback configuration (!= IXGBE_LPBK_82599_TX_RX),
ixgbe PMD ignores it and continues the configuration without raising any
error.

Let's robustify this part by checking if the requested loopback mode is
correct for the current device, before starting it. If it is not valid,
PMD will refuse to start.

Signed-off-by: Julien Meunier <julien.meunier@nokia.com>
---
v2:
- factorize code
- check if loopback is really supported
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 13 +++++++++----
 drivers/net/ixgbe/ixgbe_rxtx.c   | 37 +++++++++++++++++++++++++++++--------
 drivers/net/ixgbe/ixgbe_rxtx.h   |  2 ++
 3 files changed, 40 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 7493110..558f60b 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -2652,10 +2652,15 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
 		goto error;
 	}
 
-	/* Skip link setup if loopback mode is enabled for 82599. */
-	if (hw->mac.type == ixgbe_mac_82599EB &&
-			dev->data->dev_conf.lpbk_mode == IXGBE_LPBK_82599_TX_RX)
-		goto skip_link_setup;
+	/* Skip link setup if loopback mode is enabled. */
+	if (dev->data->dev_conf.lpbk_mode != 0) {
+		err = ixgbe_check_supported_loopback_mode(dev);
+		if (err < 0) {
+			PMD_INIT_LOG(ERR, "Unsupported loopback mode");
+			goto error;
+		} else
+			goto skip_link_setup;
+	}
 
 	if (ixgbe_is_sfp(hw) && hw->phy.multispeed_fiber) {
 		err = hw->mac.ops.setup_sfp(hw);
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index 9a79d18..c9a70a8 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -4879,13 +4879,18 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
 		hlreg0 &= ~IXGBE_HLREG0_JUMBOEN;
 
 	/*
-	 * If loopback mode is configured for 82599, set LPBK bit.
+	 * If loopback mode is configured, set LPBK bit.
 	 */
-	if (hw->mac.type == ixgbe_mac_82599EB &&
-			dev->data->dev_conf.lpbk_mode == IXGBE_LPBK_82599_TX_RX)
+	if (dev->data->dev_conf.lpbk_mode != 0) {
+		rc = ixgbe_check_supported_loopback_mode(dev);
+		if (rc < 0) {
+			PMD_INIT_LOG(ERR, "Unsupported loopback mode");
+			return rc;
+		}
 		hlreg0 |= IXGBE_HLREG0_LPBK;
-	else
+	} else {
 		hlreg0 &= ~IXGBE_HLREG0_LPBK;
+	}
 
 	IXGBE_WRITE_REG(hw, IXGBE_HLREG0, hlreg0);
 
@@ -5062,6 +5067,21 @@ ixgbe_dev_tx_init(struct rte_eth_dev *dev)
 }
 
 /*
+ * Check if requested loopback mode is supported
+ */
+int
+ixgbe_check_supported_loopback_mode(struct rte_eth_dev *dev)
+{
+	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+	if (dev->data->dev_conf.lpbk_mode == IXGBE_LPBK_82599_TX_RX)
+		if (hw->mac.type == ixgbe_mac_82599EB)
+			return 0;
+
+	return -ENOTSUP;
+}
+
+/*
  * Set up link for 82599 loopback mode Tx->Rx.
  */
 static inline void __attribute__((cold))
@@ -5148,10 +5168,11 @@ ixgbe_dev_rxtx_start(struct rte_eth_dev *dev)
 	rxctrl |= IXGBE_RXCTRL_RXEN;
 	hw->mac.ops.enable_rx_dma(hw, rxctrl);
 
-	/* If loopback mode is enabled for 82599, set up the link accordingly */
-	if (hw->mac.type == ixgbe_mac_82599EB &&
-			dev->data->dev_conf.lpbk_mode == IXGBE_LPBK_82599_TX_RX)
-		ixgbe_setup_loopback_link_82599(hw);
+	/* If loopback mode is enabled, set up the link accordingly */
+	if (dev->data->dev_conf.lpbk_mode != 0) {
+		if (hw->mac.type == ixgbe_mac_82599EB)
+			ixgbe_setup_loopback_link_82599(hw);
+	}
 
 #ifdef RTE_LIBRTE_SECURITY
 	if ((dev->data->dev_conf.rxmode.offloads &
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.h b/drivers/net/ixgbe/ixgbe_rxtx.h
index 39378f7..2d8011d 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.h
+++ b/drivers/net/ixgbe/ixgbe_rxtx.h
@@ -276,6 +276,8 @@ void ixgbe_set_tx_function(struct rte_eth_dev *dev, struct ixgbe_tx_queue *txq);
  */
 void ixgbe_set_rx_function(struct rte_eth_dev *dev);
 
+int ixgbe_check_supported_loopback_mode(struct rte_eth_dev *dev);
+
 uint16_t ixgbe_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
 		uint16_t nb_pkts);
 uint16_t ixgbe_recv_scattered_pkts_vec(void *rx_queue,
-- 
2.10.2

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [dpdk-dev] [PATCH 2/2] net/ixgbe: add support of loopback for X540/X550
  2019-02-07 17:30 ` [dpdk-dev] [PATCH 1/2] net/ixgbe: do not start on unsupported loopback mode Julien Meunier
@ 2019-02-07 17:30   ` Julien Meunier
  2019-02-12  6:37     ` Zhao1, Wei
  2019-02-12  6:36   ` [dpdk-dev] [PATCH 1/2] net/ixgbe: do not start on unsupported loopback mode Zhao1, Wei
  1 sibling, 1 reply; 20+ messages in thread
From: Julien Meunier @ 2019-02-07 17:30 UTC (permalink / raw)
  To: konstantin.ananyev, wenzhuo.lu, wei.zhao1, qi.z.zhang; +Cc: dev

Loopback mode is also supported on X540 and X550 NICs, according to
their datasheet (section 15.2). The way to set it up is a little
different of the 82599.

Signed-off-by: Julien Meunier <julien.meunier@nokia.com>
---
v2:
- disable / enable autoneg when loopback is requested for X540 / X550
---
 drivers/net/ixgbe/base/ixgbe_type.h |  1 +
 drivers/net/ixgbe/base/ixgbe_x540.c | 26 ++++++++++++++++
 drivers/net/ixgbe/base/ixgbe_x540.h |  2 ++
 drivers/net/ixgbe/base/ixgbe_x550.c | 26 ++++++++++++++++
 drivers/net/ixgbe/base/ixgbe_x550.h |  1 +
 drivers/net/ixgbe/ixgbe_ethdev.h    |  5 ++-
 drivers/net/ixgbe/ixgbe_rxtx.c      | 61 +++++++++++++++++++++++++++++++++++--
 7 files changed, 117 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ixgbe/base/ixgbe_type.h b/drivers/net/ixgbe/base/ixgbe_type.h
index 077b8f0..c4af31e 100644
--- a/drivers/net/ixgbe/base/ixgbe_type.h
+++ b/drivers/net/ixgbe/base/ixgbe_type.h
@@ -1649,6 +1649,7 @@ struct ixgbe_dmac_config {
 #define IXGBE_MII_5GBASE_T_ADVERTISE		0x0800
 #define IXGBE_MII_100BASE_T_ADVERTISE		0x0100 /* full duplex, bit:8 */
 #define IXGBE_MII_100BASE_T_ADVERTISE_HALF	0x0080 /* half duplex, bit:7 */
+#define IXGBE_MII_AUTONEG_ENABLE		0x1000
 #define IXGBE_MII_RESTART			0x200
 #define IXGBE_MII_AUTONEG_COMPLETE		0x20
 #define IXGBE_MII_AUTONEG_LINK_UP		0x04
diff --git a/drivers/net/ixgbe/base/ixgbe_x540.c b/drivers/net/ixgbe/base/ixgbe_x540.c
index f00f0ea..a241d41 100644
--- a/drivers/net/ixgbe/base/ixgbe_x540.c
+++ b/drivers/net/ixgbe/base/ixgbe_x540.c
@@ -1032,3 +1032,29 @@ s32 ixgbe_blink_led_stop_X540(struct ixgbe_hw *hw, u32 index)
 
 	return IXGBE_SUCCESS;
 }
+
+/*
+ *  ixgbe_setup_phy_link_x540 - Enable/disable the autoneg
+ *  @hw: pointer to hardware structure
+ *  enable: enable/disable the autoneg
+ **/
+s32 ixgbe_setup_phy_autoneg_x540(struct ixgbe_hw *hw, bool enable)
+{
+	s32 status = IXGBE_SUCCESS;
+	u16 autoneg_reg = IXGBE_MII_AUTONEG_REG;
+
+	DEBUGFUNC("ixgbe_setup_phy_autoneg_x540");
+
+	hw->phy.ops.read_reg(hw, IXGBE_MDIO_AUTO_NEG_CONTROL,
+			     IXGBE_MDIO_AUTO_NEG_DEV_TYPE, &autoneg_reg);
+
+	if (enable)
+		autoneg_reg |= IXGBE_MII_AUTONEG_ENABLE;
+	else
+		autoneg_reg &= ~IXGBE_MII_AUTONEG_ENABLE;
+
+	hw->phy.ops.write_reg(hw, IXGBE_MDIO_AUTO_NEG_CONTROL,
+			      IXGBE_MDIO_AUTO_NEG_DEV_TYPE, autoneg_reg);
+
+	return status;
+}
diff --git a/drivers/net/ixgbe/base/ixgbe_x540.h b/drivers/net/ixgbe/base/ixgbe_x540.h
index 231dfe5..ef939ce 100644
--- a/drivers/net/ixgbe/base/ixgbe_x540.h
+++ b/drivers/net/ixgbe/base/ixgbe_x540.h
@@ -34,5 +34,7 @@ void ixgbe_init_swfw_sync_X540(struct ixgbe_hw *hw);
 
 s32 ixgbe_blink_led_start_X540(struct ixgbe_hw *hw, u32 index);
 s32 ixgbe_blink_led_stop_X540(struct ixgbe_hw *hw, u32 index);
+
+s32 ixgbe_setup_phy_autoneg_x540(struct ixgbe_hw *hw, bool enable);
 #endif /* _IXGBE_X540_H_ */
 
diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c b/drivers/net/ixgbe/base/ixgbe_x550.c
index a920a14..f4ee188 100644
--- a/drivers/net/ixgbe/base/ixgbe_x550.c
+++ b/drivers/net/ixgbe/base/ixgbe_x550.c
@@ -4652,3 +4652,29 @@ bool ixgbe_fw_recovery_mode_X550(struct ixgbe_hw *hw)
 
 	return !!(fwsm & IXGBE_FWSM_FW_NVM_RECOVERY_MODE);
 }
+
+/*
+ *  ixgbe_setup_phy_link_x550 - Enable/disable the autoneg
+ *  @hw: pointer to hardware structure
+ *  enable: enable/disable the autoneg
+ **/
+s32 ixgbe_setup_phy_autoneg_x550(struct ixgbe_hw *hw, bool enable)
+{
+	s32 status = IXGBE_SUCCESS;
+	u16 autoneg_reg = IXGBE_MII_AUTONEG_REG;
+
+	DEBUGFUNC("ixgbe_setup_phy_autoneg_x550");
+
+	hw->phy.ops.read_reg(hw, IXGBE_MDIO_AUTO_NEG_CONTROL,
+			     IXGBE_MDIO_AUTO_NEG_DEV_TYPE, &autoneg_reg);
+
+	if (enable)
+		autoneg_reg |= IXGBE_MII_AUTONEG_ENABLE;
+	else
+		autoneg_reg &= ~IXGBE_MII_AUTONEG_ENABLE;
+
+	hw->phy.ops.write_reg(hw, IXGBE_MDIO_AUTO_NEG_CONTROL,
+			      IXGBE_MDIO_AUTO_NEG_DEV_TYPE, autoneg_reg);
+
+	return status;
+}
diff --git a/drivers/net/ixgbe/base/ixgbe_x550.h b/drivers/net/ixgbe/base/ixgbe_x550.h
index 3bd98f2..dd4fa18 100644
--- a/drivers/net/ixgbe/base/ixgbe_x550.h
+++ b/drivers/net/ixgbe/base/ixgbe_x550.h
@@ -93,4 +93,5 @@ s32 ixgbe_identify_sfp_module_X550em(struct ixgbe_hw *hw);
 s32 ixgbe_led_on_t_X550em(struct ixgbe_hw *hw, u32 led_idx);
 s32 ixgbe_led_off_t_X550em(struct ixgbe_hw *hw, u32 led_idx);
 bool ixgbe_fw_recovery_mode_X550(struct ixgbe_hw *hw);
+s32 ixgbe_setup_phy_autoneg_x550(struct ixgbe_hw *hw, bool enable);
 #endif /* _IXGBE_X550_H_ */
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
index 565c69c..c60a697 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.h
+++ b/drivers/net/ixgbe/ixgbe_ethdev.h
@@ -65,9 +65,8 @@
 #define IXGBE_QUEUE_ITR_INTERVAL_DEFAULT	500 /* 500us */
 
 /* Loopback operation modes */
-/* 82599 specific loopback operation types */
-#define IXGBE_LPBK_82599_NONE   0x0 /* Default value. Loopback is disabled. */
-#define IXGBE_LPBK_82599_TX_RX  0x1 /* Tx->Rx loopback operation is enabled. */
+#define IXGBE_LPBK_NONE   0x0 /* Default value. Loopback is disabled. */
+#define IXGBE_LPBK_TX_RX  0x1 /* Tx->Rx loopback operation is enabled. */
 
 #define IXGBE_MAX_JUMBO_FRAME_SIZE      0x2600 /* Maximum Jumbo frame size. */
 
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index c9a70a8..1472d13 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -49,6 +49,8 @@
 #include "ixgbe_ethdev.h"
 #include "base/ixgbe_dcb.h"
 #include "base/ixgbe_common.h"
+#include "base/ixgbe_x540.h"
+#include "base/ixgbe_x550.h"
 #include "ixgbe_rxtx.h"
 
 #ifdef RTE_LIBRTE_IEEE1588
@@ -3174,6 +3176,7 @@ ixgbe_dev_clear_queues(struct rte_eth_dev *dev)
 	unsigned i;
 	struct ixgbe_adapter *adapter =
 		(struct ixgbe_adapter *)dev->data->dev_private;
+	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
 	PMD_INIT_FUNC_TRACE();
 
@@ -3194,6 +3197,15 @@ ixgbe_dev_clear_queues(struct rte_eth_dev *dev)
 			ixgbe_reset_rx_queue(adapter, rxq);
 		}
 	}
+	/* If loopback mode was enabled, reconfigure the link accordingly */
+	if (dev->data->dev_conf.lpbk_mode != 0) {
+		if (hw->mac.type == ixgbe_mac_X540)
+			ixgbe_setup_phy_autoneg_x540(hw, true);
+		else if (hw->mac.type == ixgbe_mac_X550 ||
+		     hw->mac.type == ixgbe_mac_X550EM_x ||
+		     hw->mac.type == ixgbe_mac_X550EM_a)
+			ixgbe_setup_phy_autoneg_x550(hw, true);
+	}
 }
 
 void
@@ -5074,8 +5086,12 @@ ixgbe_check_supported_loopback_mode(struct rte_eth_dev *dev)
 {
 	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
-	if (dev->data->dev_conf.lpbk_mode == IXGBE_LPBK_82599_TX_RX)
-		if (hw->mac.type == ixgbe_mac_82599EB)
+	if (dev->data->dev_conf.lpbk_mode == IXGBE_LPBK_TX_RX)
+		if (hw->mac.type == ixgbe_mac_82599EB ||
+		     hw->mac.type == ixgbe_mac_X540 ||
+		     hw->mac.type == ixgbe_mac_X550 ||
+		     hw->mac.type == ixgbe_mac_X550EM_x ||
+		     hw->mac.type == ixgbe_mac_X550EM_a)
 			return 0;
 
 	return -ENOTSUP;
@@ -5108,6 +5124,41 @@ ixgbe_setup_loopback_link_82599(struct ixgbe_hw *hw)
 	msec_delay(50);
 }
 
+/*
+ * Set up link loopback for X540 mode Tx->Rx.
+ */
+static inline void __attribute__((cold))
+ixgbe_setup_loopback_link_x540(struct ixgbe_hw *hw)
+{
+	uint32_t macc;
+	PMD_INIT_FUNC_TRACE();
+
+	/* datasheet 15.2.1: disable AUTONEG (PHY Bit 7.0.C) */
+	ixgbe_setup_phy_autoneg_x540(hw, false);
+
+	/* datasheet 15.2.1: MACC.FLU = 1 (force link up) */
+	macc = IXGBE_READ_REG(hw, IXGBE_MACC);
+	macc |= IXGBE_MACC_FLU;
+	IXGBE_WRITE_REG(hw, IXGBE_MACC, macc);
+}
+
+/*
+ * Set up link loopback for X550 mode Tx->Rx.
+ */
+static inline void __attribute__((cold))
+ixgbe_setup_loopback_link_x550(struct ixgbe_hw *hw)
+{
+	uint32_t macc;
+	PMD_INIT_FUNC_TRACE();
+
+	/* datasheet 15.2.1: disable AUTONEG (PHY Bit 7.0.C) */
+	ixgbe_setup_phy_autoneg_x550(hw, false);
+
+	/* datasheet 15.2.1: MACC.FLU = 1 (force link up) */
+	macc = IXGBE_READ_REG(hw, IXGBE_MACC);
+	macc |= IXGBE_MACC_FLU;
+	IXGBE_WRITE_REG(hw, IXGBE_MACC, macc);
+}
 
 /*
  * Start Transmit and Receive Units.
@@ -5172,6 +5223,12 @@ ixgbe_dev_rxtx_start(struct rte_eth_dev *dev)
 	if (dev->data->dev_conf.lpbk_mode != 0) {
 		if (hw->mac.type == ixgbe_mac_82599EB)
 			ixgbe_setup_loopback_link_82599(hw);
+		else if (hw->mac.type == ixgbe_mac_X540)
+			ixgbe_setup_loopback_link_x540(hw);
+		else if (hw->mac.type == ixgbe_mac_X550 ||
+		     hw->mac.type == ixgbe_mac_X550EM_x ||
+		     hw->mac.type == ixgbe_mac_X550EM_a)
+			ixgbe_setup_loopback_link_x550(hw);
 	}
 
 #ifdef RTE_LIBRTE_SECURITY
-- 
2.10.2

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [PATCH 1/2] net/ixgbe: do not start on unsupported loopback mode
  2019-02-07 17:30 ` [dpdk-dev] [PATCH 1/2] net/ixgbe: do not start on unsupported loopback mode Julien Meunier
  2019-02-07 17:30   ` [dpdk-dev] [PATCH 2/2] net/ixgbe: add support of loopback for X540/X550 Julien Meunier
@ 2019-02-12  6:36   ` Zhao1, Wei
  1 sibling, 0 replies; 20+ messages in thread
From: Zhao1, Wei @ 2019-02-12  6:36 UTC (permalink / raw)
  To: Julien Meunier, Ananyev, Konstantin, Lu, Wenzhuo, Zhang, Qi Z; +Cc: dev


Hi,  Meunier
   Only a little change need for this patch.

Acked-by: Wei Zhao <wei.zhao1@intel.com>

> -----Original Message-----
> From: Julien Meunier [mailto:julien.meunier@nokia.com]
> Sent: Friday, February 8, 2019 1:30 AM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Lu, Wenzhuo
> <wenzhuo.lu@intel.com>; Zhao1, Wei <wei.zhao1@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org
> Subject: [PATCH 1/2] net/ixgbe: do not start on unsupported loopback mode
> 
> Only TX->RX loopback is supported currently on 82599EB. If a user wants to
> apply an another loopback configuration (!= IXGBE_LPBK_82599_TX_RX),
> ixgbe PMD ignores it and continues the configuration without raising any
> error.
> 
> Let's robustify this part by checking if the requested loopback mode is correct
> for the current device, before starting it. If it is not valid, PMD will refuse to
> start.
> 
> Signed-off-by: Julien Meunier <julien.meunier@nokia.com>
> ---
> v2:
> - factorize code
> - check if loopback is really supported
> ---
>  drivers/net/ixgbe/ixgbe_ethdev.c | 13 +++++++++----
>  drivers/net/ixgbe/ixgbe_rxtx.c   | 37 +++++++++++++++++++++++++++++--
> ------
>  drivers/net/ixgbe/ixgbe_rxtx.h   |  2 ++
>  3 files changed, 40 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> b/drivers/net/ixgbe/ixgbe_ethdev.c
> index 7493110..558f60b 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -2652,10 +2652,15 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
>  		goto error;
>  	}
> 
> -	/* Skip link setup if loopback mode is enabled for 82599. */
> -	if (hw->mac.type == ixgbe_mac_82599EB &&
> -			dev->data->dev_conf.lpbk_mode ==
> IXGBE_LPBK_82599_TX_RX)
> -		goto skip_link_setup;
> +	/* Skip link setup if loopback mode is enabled. */
> +	if (dev->data->dev_conf.lpbk_mode != 0) {
> +		err = ixgbe_check_supported_loopback_mode(dev);
> +		if (err < 0) {
> +			PMD_INIT_LOG(ERR, "Unsupported loopback
> mode");
> +			goto error;
> +		} else
> +			goto skip_link_setup;
> +	}
> 
>  	if (ixgbe_is_sfp(hw) && hw->phy.multispeed_fiber) {
>  		err = hw->mac.ops.setup_sfp(hw);
> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
> index 9a79d18..c9a70a8 100644
> --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> @@ -4879,13 +4879,18 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
>  		hlreg0 &= ~IXGBE_HLREG0_JUMBOEN;
> 
>  	/*
> -	 * If loopback mode is configured for 82599, set LPBK bit.
> +	 * If loopback mode is configured, set LPBK bit.
>  	 */
> -	if (hw->mac.type == ixgbe_mac_82599EB &&
> -			dev->data->dev_conf.lpbk_mode ==
> IXGBE_LPBK_82599_TX_RX)
> +	if (dev->data->dev_conf.lpbk_mode != 0) {
> +		rc = ixgbe_check_supported_loopback_mode(dev);
> +		if (rc < 0) {
> +			PMD_INIT_LOG(ERR, "Unsupported loopback
> mode");
> +			return rc;
> +		}
>  		hlreg0 |= IXGBE_HLREG0_LPBK;
> -	else
> +	} else {
>  		hlreg0 &= ~IXGBE_HLREG0_LPBK;
> +	}
> 
>  	IXGBE_WRITE_REG(hw, IXGBE_HLREG0, hlreg0);
> 
> @@ -5062,6 +5067,21 @@ ixgbe_dev_tx_init(struct rte_eth_dev *dev)  }
> 
>  /*
> + * Check if requested loopback mode is supported  */ int
> +ixgbe_check_supported_loopback_mode(struct rte_eth_dev *dev) {
> +	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> +
> +	if (dev->data->dev_conf.lpbk_mode == IXGBE_LPBK_82599_TX_RX)
> +		if (hw->mac.type == ixgbe_mac_82599EB)
> +			return 0;
> +
> +	return -ENOTSUP;
> +}
> +
> +/*
>   * Set up link for 82599 loopback mode Tx->Rx.
>   */
>  static inline void __attribute__((cold)) @@ -5148,10 +5168,11 @@
> ixgbe_dev_rxtx_start(struct rte_eth_dev *dev)
>  	rxctrl |= IXGBE_RXCTRL_RXEN;
>  	hw->mac.ops.enable_rx_dma(hw, rxctrl);
> 
> -	/* If loopback mode is enabled for 82599, set up the link accordingly
> */
> -	if (hw->mac.type == ixgbe_mac_82599EB &&
> -			dev->data->dev_conf.lpbk_mode ==
> IXGBE_LPBK_82599_TX_RX)
> -		ixgbe_setup_loopback_link_82599(hw);
> +	/* If loopback mode is enabled, set up the link accordingly */
> +	if (dev->data->dev_conf.lpbk_mode != 0) {
> +		if (hw->mac.type == ixgbe_mac_82599EB)
> +			ixgbe_setup_loopback_link_82599(hw);
> +	}
> 
>  #ifdef RTE_LIBRTE_SECURITY
>  	if ((dev->data->dev_conf.rxmode.offloads & diff --git
> a/drivers/net/ixgbe/ixgbe_rxtx.h b/drivers/net/ixgbe/ixgbe_rxtx.h index
> 39378f7..2d8011d 100644
> --- a/drivers/net/ixgbe/ixgbe_rxtx.h
> +++ b/drivers/net/ixgbe/ixgbe_rxtx.h
> @@ -276,6 +276,8 @@ void ixgbe_set_tx_function(struct rte_eth_dev *dev,
> struct ixgbe_tx_queue *txq);
>   */
>  void ixgbe_set_rx_function(struct rte_eth_dev *dev);
> 
> +int ixgbe_check_supported_loopback_mode(struct rte_eth_dev *dev);
> +

Space needed?


>  uint16_t ixgbe_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
>  		uint16_t nb_pkts);
>  uint16_t ixgbe_recv_scattered_pkts_vec(void *rx_queue,
> --
> 2.10.2

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [PATCH 2/2] net/ixgbe: add support of loopback for X540/X550
  2019-02-07 17:30   ` [dpdk-dev] [PATCH 2/2] net/ixgbe: add support of loopback for X540/X550 Julien Meunier
@ 2019-02-12  6:37     ` Zhao1, Wei
  2019-02-18 11:20       ` Meunier, Julien (Nokia - FR/Paris-Saclay)
  0 siblings, 1 reply; 20+ messages in thread
From: Zhao1, Wei @ 2019-02-12  6:37 UTC (permalink / raw)
  To: Julien Meunier, Ananyev, Konstantin, Lu, Wenzhuo, Zhang, Qi Z; +Cc: dev

HI,  Meunier && qi

> -----Original Message-----
> From: Julien Meunier [mailto:julien.meunier@nokia.com]
> Sent: Friday, February 8, 2019 1:30 AM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Lu, Wenzhuo
> <wenzhuo.lu@intel.com>; Zhao1, Wei <wei.zhao1@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org
> Subject: [PATCH 2/2] net/ixgbe: add support of loopback for X540/X550
> 
> Loopback mode is also supported on X540 and X550 NICs, according to their
> datasheet (section 15.2). The way to set it up is a little different of the 82599.
> 
> Signed-off-by: Julien Meunier <julien.meunier@nokia.com>
> ---
> v2:
> - disable / enable autoneg when loopback is requested for X540 / X550
> ---
>  drivers/net/ixgbe/base/ixgbe_type.h |  1 +
> drivers/net/ixgbe/base/ixgbe_x540.c | 26 ++++++++++++++++
> drivers/net/ixgbe/base/ixgbe_x540.h |  2 ++
> drivers/net/ixgbe/base/ixgbe_x550.c | 26 ++++++++++++++++
> drivers/net/ixgbe/base/ixgbe_x550.h |  1 +
>  drivers/net/ixgbe/ixgbe_ethdev.h    |  5 ++-
>  drivers/net/ixgbe/ixgbe_rxtx.c      | 61
> +++++++++++++++++++++++++++++++++++--
>  7 files changed, 117 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/ixgbe/base/ixgbe_type.h
> b/drivers/net/ixgbe/base/ixgbe_type.h
> index 077b8f0..c4af31e 100644
> --- a/drivers/net/ixgbe/base/ixgbe_type.h
> +++ b/drivers/net/ixgbe/base/ixgbe_type.h
> @@ -1649,6 +1649,7 @@ struct ixgbe_dmac_config {
>  #define IXGBE_MII_5GBASE_T_ADVERTISE		0x0800
>  #define IXGBE_MII_100BASE_T_ADVERTISE		0x0100 /* full duplex,
> bit:8 */
>  #define IXGBE_MII_100BASE_T_ADVERTISE_HALF	0x0080 /* half duplex,
> bit:7 */
> +#define IXGBE_MII_AUTONEG_ENABLE		0x1000
>  #define IXGBE_MII_RESTART			0x200
>  #define IXGBE_MII_AUTONEG_COMPLETE		0x20
>  #define IXGBE_MII_AUTONEG_LINK_UP		0x04
> diff --git a/drivers/net/ixgbe/base/ixgbe_x540.c
> b/drivers/net/ixgbe/base/ixgbe_x540.c
> index f00f0ea..a241d41 100644
> --- a/drivers/net/ixgbe/base/ixgbe_x540.c
> +++ b/drivers/net/ixgbe/base/ixgbe_x540.c


Is it possible to add these two function of ixgbe_setup_phy_autoneg_x540 and 
ixgbe_setup_phy_autoneg_x550 in a file out of base folder? We need to avoid
change code in that, is that right? @qi
And also, it seems these 2 function do not have much difference, 
can we use one instead of two function?(This point is not so important)



> @@ -1032,3 +1032,29 @@ s32 ixgbe_blink_led_stop_X540(struct ixgbe_hw
> *hw, u32 index)
> 
>  	return IXGBE_SUCCESS;
>  }
> +
> +/*
> + *  ixgbe_setup_phy_link_x540 - Enable/disable the autoneg
> + *  @hw: pointer to hardware structure
> + *  enable: enable/disable the autoneg
> + **/
> +s32 ixgbe_setup_phy_autoneg_x540(struct ixgbe_hw *hw, bool enable) {
> +	s32 status = IXGBE_SUCCESS;
> +	u16 autoneg_reg = IXGBE_MII_AUTONEG_REG;
> +
> +	DEBUGFUNC("ixgbe_setup_phy_autoneg_x540");
> +
> +	hw->phy.ops.read_reg(hw, IXGBE_MDIO_AUTO_NEG_CONTROL,
> +			     IXGBE_MDIO_AUTO_NEG_DEV_TYPE,
> &autoneg_reg);
> +
> +	if (enable)
> +		autoneg_reg |= IXGBE_MII_AUTONEG_ENABLE;
> +	else
> +		autoneg_reg &= ~IXGBE_MII_AUTONEG_ENABLE;
> +
> +	hw->phy.ops.write_reg(hw, IXGBE_MDIO_AUTO_NEG_CONTROL,
> +			      IXGBE_MDIO_AUTO_NEG_DEV_TYPE,
> autoneg_reg);
> +
> +	return status;
> +}
> diff --git a/drivers/net/ixgbe/base/ixgbe_x540.h
> b/drivers/net/ixgbe/base/ixgbe_x540.h
> index 231dfe5..ef939ce 100644
> --- a/drivers/net/ixgbe/base/ixgbe_x540.h
> +++ b/drivers/net/ixgbe/base/ixgbe_x540.h
> @@ -34,5 +34,7 @@ void ixgbe_init_swfw_sync_X540(struct ixgbe_hw *hw);
> 
>  s32 ixgbe_blink_led_start_X540(struct ixgbe_hw *hw, u32 index);
>  s32 ixgbe_blink_led_stop_X540(struct ixgbe_hw *hw, u32 index);
> +
> +s32 ixgbe_setup_phy_autoneg_x540(struct ixgbe_hw *hw, bool enable);

The Space line seems not needed.

>  #endif /* _IXGBE_X540_H_ */
> 
> diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c
> b/drivers/net/ixgbe/base/ixgbe_x550.c
> index a920a14..f4ee188 100644
> --- a/drivers/net/ixgbe/base/ixgbe_x550.c
> +++ b/drivers/net/ixgbe/base/ixgbe_x550.c
> @@ -4652,3 +4652,29 @@ bool ixgbe_fw_recovery_mode_X550(struct
> ixgbe_hw *hw)
> 
>  	return !!(fwsm & IXGBE_FWSM_FW_NVM_RECOVERY_MODE);  }
> +
> +/*
> + *  ixgbe_setup_phy_link_x550 - Enable/disable the autoneg
> + *  @hw: pointer to hardware structure
> + *  enable: enable/disable the autoneg
> + **/
> +s32 ixgbe_setup_phy_autoneg_x550(struct ixgbe_hw *hw, bool enable) {
> +	s32 status = IXGBE_SUCCESS;
> +	u16 autoneg_reg = IXGBE_MII_AUTONEG_REG;
> +
> +	DEBUGFUNC("ixgbe_setup_phy_autoneg_x550");
> +
> +	hw->phy.ops.read_reg(hw, IXGBE_MDIO_AUTO_NEG_CONTROL,
> +			     IXGBE_MDIO_AUTO_NEG_DEV_TYPE,
> &autoneg_reg);
> +
> +	if (enable)
> +		autoneg_reg |= IXGBE_MII_AUTONEG_ENABLE;
> +	else
> +		autoneg_reg &= ~IXGBE_MII_AUTONEG_ENABLE;
> +
> +	hw->phy.ops.write_reg(hw, IXGBE_MDIO_AUTO_NEG_CONTROL,
> +			      IXGBE_MDIO_AUTO_NEG_DEV_TYPE,
> autoneg_reg);
> +
> +	return status;
> +}
> diff --git a/drivers/net/ixgbe/base/ixgbe_x550.h
> b/drivers/net/ixgbe/base/ixgbe_x550.h
> index 3bd98f2..dd4fa18 100644
> --- a/drivers/net/ixgbe/base/ixgbe_x550.h
> +++ b/drivers/net/ixgbe/base/ixgbe_x550.h
> @@ -93,4 +93,5 @@ s32 ixgbe_identify_sfp_module_X550em(struct
> ixgbe_hw *hw);
>  s32 ixgbe_led_on_t_X550em(struct ixgbe_hw *hw, u32 led_idx);
>  s32 ixgbe_led_off_t_X550em(struct ixgbe_hw *hw, u32 led_idx);  bool
> ixgbe_fw_recovery_mode_X550(struct ixgbe_hw *hw);
> +s32 ixgbe_setup_phy_autoneg_x550(struct ixgbe_hw *hw, bool enable);
>  #endif /* _IXGBE_X550_H_ */
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h
> b/drivers/net/ixgbe/ixgbe_ethdev.h
> index 565c69c..c60a697 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.h
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.h
> @@ -65,9 +65,8 @@
>  #define IXGBE_QUEUE_ITR_INTERVAL_DEFAULT	500 /* 500us */
> 
>  /* Loopback operation modes */
> -/* 82599 specific loopback operation types */
> -#define IXGBE_LPBK_82599_NONE   0x0 /* Default value. Loopback is
> disabled. */
> -#define IXGBE_LPBK_82599_TX_RX  0x1 /* Tx->Rx loopback operation is
> enabled. */
> +#define IXGBE_LPBK_NONE   0x0 /* Default value. Loopback is disabled. */
> +#define IXGBE_LPBK_TX_RX  0x1 /* Tx->Rx loopback operation is enabled.
> +*/
> 
>  #define IXGBE_MAX_JUMBO_FRAME_SIZE      0x2600 /* Maximum Jumbo
> frame size. */
> 
> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
> index c9a70a8..1472d13 100644
> --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> @@ -49,6 +49,8 @@
>  #include "ixgbe_ethdev.h"
>  #include "base/ixgbe_dcb.h"
>  #include "base/ixgbe_common.h"
> +#include "base/ixgbe_x540.h"
> +#include "base/ixgbe_x550.h"
>  #include "ixgbe_rxtx.h"
> 
>  #ifdef RTE_LIBRTE_IEEE1588
> @@ -3174,6 +3176,7 @@ ixgbe_dev_clear_queues(struct rte_eth_dev *dev)
>  	unsigned i;
>  	struct ixgbe_adapter *adapter =
>  		(struct ixgbe_adapter *)dev->data->dev_private;
> +	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> 
>  	PMD_INIT_FUNC_TRACE();
> 
> @@ -3194,6 +3197,15 @@ ixgbe_dev_clear_queues(struct rte_eth_dev
> *dev)
>  			ixgbe_reset_rx_queue(adapter, rxq);
>  		}
>  	}
> +	/* If loopback mode was enabled, reconfigure the link accordingly */
> +	if (dev->data->dev_conf.lpbk_mode != 0) {
> +		if (hw->mac.type == ixgbe_mac_X540)
> +			ixgbe_setup_phy_autoneg_x540(hw, true);
> +		else if (hw->mac.type == ixgbe_mac_X550 ||
> +		     hw->mac.type == ixgbe_mac_X550EM_x ||
> +		     hw->mac.type == ixgbe_mac_X550EM_a)
> +			ixgbe_setup_phy_autoneg_x550(hw, true);
> +	}
>  }
> 
>  void
> @@ -5074,8 +5086,12 @@ ixgbe_check_supported_loopback_mode(struct
> rte_eth_dev *dev)  {
>  	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> 
> -	if (dev->data->dev_conf.lpbk_mode == IXGBE_LPBK_82599_TX_RX)
> -		if (hw->mac.type == ixgbe_mac_82599EB)
> +	if (dev->data->dev_conf.lpbk_mode == IXGBE_LPBK_TX_RX)
> +		if (hw->mac.type == ixgbe_mac_82599EB ||
> +		     hw->mac.type == ixgbe_mac_X540 ||
> +		     hw->mac.type == ixgbe_mac_X550 ||
> +		     hw->mac.type == ixgbe_mac_X550EM_x ||
> +		     hw->mac.type == ixgbe_mac_X550EM_a)
>  			return 0;
> 
>  	return -ENOTSUP;
> @@ -5108,6 +5124,41 @@ ixgbe_setup_loopback_link_82599(struct
> ixgbe_hw *hw)
>  	msec_delay(50);
>  }
> 
> +/*
> + * Set up link loopback for X540 mode Tx->Rx.
> + */
> +static inline void __attribute__((cold))
> +ixgbe_setup_loopback_link_x540(struct ixgbe_hw *hw) {
> +	uint32_t macc;
> +	PMD_INIT_FUNC_TRACE();
> +
> +	/* datasheet 15.2.1: disable AUTONEG (PHY Bit 7.0.C) */
> +	ixgbe_setup_phy_autoneg_x540(hw, false);
> +
> +	/* datasheet 15.2.1: MACC.FLU = 1 (force link up) */
> +	macc = IXGBE_READ_REG(hw, IXGBE_MACC);
> +	macc |= IXGBE_MACC_FLU;
> +	IXGBE_WRITE_REG(hw, IXGBE_MACC, macc); }
> +
> +/*
> + * Set up link loopback for X550 mode Tx->Rx.
> + */
> +static inline void __attribute__((cold))
> +ixgbe_setup_loopback_link_x550(struct ixgbe_hw *hw) {
> +	uint32_t macc;
> +	PMD_INIT_FUNC_TRACE();
> +
> +	/* datasheet 15.2.1: disable AUTONEG (PHY Bit 7.0.C) */
> +	ixgbe_setup_phy_autoneg_x550(hw, false);
> +
> +	/* datasheet 15.2.1: MACC.FLU = 1 (force link up) */
> +	macc = IXGBE_READ_REG(hw, IXGBE_MACC);
> +	macc |= IXGBE_MACC_FLU;
> +	IXGBE_WRITE_REG(hw, IXGBE_MACC, macc); }
> 
>  /*
>   * Start Transmit and Receive Units.
> @@ -5172,6 +5223,12 @@ ixgbe_dev_rxtx_start(struct rte_eth_dev *dev)
>  	if (dev->data->dev_conf.lpbk_mode != 0) {
>  		if (hw->mac.type == ixgbe_mac_82599EB)
>  			ixgbe_setup_loopback_link_82599(hw);
> +		else if (hw->mac.type == ixgbe_mac_X540)
> +			ixgbe_setup_loopback_link_x540(hw);
> +		else if (hw->mac.type == ixgbe_mac_X550 ||
> +		     hw->mac.type == ixgbe_mac_X550EM_x ||
> +		     hw->mac.type == ixgbe_mac_X550EM_a)
> +			ixgbe_setup_loopback_link_x550(hw);

The same, why do we have 2 branch for x540 and x550?

>  	}
> 
>  #ifdef RTE_LIBRTE_SECURITY
> --
> 2.10.2

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [PATCH 2/2] net/ixgbe: add support of loopback for X540/X550
  2019-02-12  6:37     ` Zhao1, Wei
@ 2019-02-18 11:20       ` Meunier, Julien (Nokia - FR/Paris-Saclay)
  0 siblings, 0 replies; 20+ messages in thread
From: Meunier, Julien (Nokia - FR/Paris-Saclay) @ 2019-02-18 11:20 UTC (permalink / raw)
  To: Zhao1, Wei, Ananyev, Konstantin, Lu, Wenzhuo, Zhang,  Qi Z; +Cc: dev

Hi all,

Sorry for the delay. See my reply inline.

On 12/02/2019 07:37, Zhao1, Wei wrote:
> HI,  Meunier && qi
> 
> Is it possible to add these two function of ixgbe_setup_phy_autoneg_x540 and
> ixgbe_setup_phy_autoneg_x550 in a file out of base folder? We need to avoid
> change code in that, is that right? @qi
> And also, it seems these 2 function do not have much difference,
> can we use one instead of two function?(This point is not so important)

OK, no problem. I had some doubts when I modified these files.
So, I guess it's OK for you to have direct PHY operations (hw->phy.ops. 
[read|write]_reg) in ixgbe_rxtx.c ?


>>    * Start Transmit and Receive Units.
>> @@ -5172,6 +5223,12 @@ ixgbe_dev_rxtx_start(struct rte_eth_dev *dev)
>>   	if (dev->data->dev_conf.lpbk_mode != 0) {
>>   		if (hw->mac.type == ixgbe_mac_82599EB)
>>   			ixgbe_setup_loopback_link_82599(hw);
>> +		else if (hw->mac.type == ixgbe_mac_X540)
>> +			ixgbe_setup_loopback_link_x540(hw);
>> +		else if (hw->mac.type == ixgbe_mac_X550 ||
>> +		     hw->mac.type == ixgbe_mac_X550EM_x ||
>> +		     hw->mac.type == ixgbe_mac_X550EM_a)
>> +			ixgbe_setup_loopback_link_x550(hw);
> 
> The same, why do we have 2 branch for x540 and x550?
> 

I just wanted to respect the initial rule: 1 model = 1 function. But I 
can merge them in one function as it's the same mechanism in order to 
setup the loopback.

I will update a new patchset today.

Best regards,
Julien Meunier

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [dpdk-dev] [PATCH v3 1/2] net/ixgbe: do not start on unsupported loopback mode
  2019-01-02 16:00 [dpdk-dev] [PATCH] net/ixgbe: add support of loopback for X540/X550 Julien Meunier
                   ` (2 preceding siblings ...)
  2019-02-07 17:30 ` [dpdk-dev] [PATCH 1/2] net/ixgbe: do not start on unsupported loopback mode Julien Meunier
@ 2019-02-20 21:05 ` Julien Meunier
  2019-02-20 21:05   ` [dpdk-dev] [PATCH v3 2/2] net/ixgbe: add support of loopback for X540/X550 Julien Meunier
  2019-03-01  7:45   ` [dpdk-dev] [PATCH v3 1/2] net/ixgbe: do not start on unsupported loopback mode Zhang, Qi Z
  3 siblings, 2 replies; 20+ messages in thread
From: Julien Meunier @ 2019-02-20 21:05 UTC (permalink / raw)
  To: wei.zhao1, qi.z.zhang, Wenzhuo Lu, Konstantin Ananyev; +Cc: dev

Only TX->RX loopback is supported currently on 82599EB. If a user wants
to apply an another loopback configuration (!= IXGBE_LPBK_82599_TX_RX),
ixgbe PMD ignores it and continues the configuration without raising any
error.

Let's robustify this part by checking if the requested loopback mode is
correct for the current device, before starting it. If it is not valid,
PMD will refuse to start.

Signed-off-by: Julien Meunier <julien.meunier@nokia.com>
---
v3:
- code style + checkpatch compliance

v2:
- factorize code
- check if loopback is really supported
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 14 ++++++++++----
 drivers/net/ixgbe/ixgbe_rxtx.c   | 37 +++++++++++++++++++++++++++++--------
 drivers/net/ixgbe/ixgbe_rxtx.h   |  1 +
 3 files changed, 40 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 7493110..4deedb0 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -2652,10 +2652,16 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
 		goto error;
 	}
 
-	/* Skip link setup if loopback mode is enabled for 82599. */
-	if (hw->mac.type == ixgbe_mac_82599EB &&
-			dev->data->dev_conf.lpbk_mode == IXGBE_LPBK_82599_TX_RX)
-		goto skip_link_setup;
+	/* Skip link setup if loopback mode is enabled. */
+	if (dev->data->dev_conf.lpbk_mode != 0) {
+		err = ixgbe_check_supported_loopback_mode(dev);
+		if (err < 0) {
+			PMD_INIT_LOG(ERR, "Unsupported loopback mode");
+			goto error;
+		} else {
+			goto skip_link_setup;
+		}
+	}
 
 	if (ixgbe_is_sfp(hw) && hw->phy.multispeed_fiber) {
 		err = hw->mac.ops.setup_sfp(hw);
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index 9a79d18..c9a70a8 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -4879,13 +4879,18 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
 		hlreg0 &= ~IXGBE_HLREG0_JUMBOEN;
 
 	/*
-	 * If loopback mode is configured for 82599, set LPBK bit.
+	 * If loopback mode is configured, set LPBK bit.
 	 */
-	if (hw->mac.type == ixgbe_mac_82599EB &&
-			dev->data->dev_conf.lpbk_mode == IXGBE_LPBK_82599_TX_RX)
+	if (dev->data->dev_conf.lpbk_mode != 0) {
+		rc = ixgbe_check_supported_loopback_mode(dev);
+		if (rc < 0) {
+			PMD_INIT_LOG(ERR, "Unsupported loopback mode");
+			return rc;
+		}
 		hlreg0 |= IXGBE_HLREG0_LPBK;
-	else
+	} else {
 		hlreg0 &= ~IXGBE_HLREG0_LPBK;
+	}
 
 	IXGBE_WRITE_REG(hw, IXGBE_HLREG0, hlreg0);
 
@@ -5062,6 +5067,21 @@ ixgbe_dev_tx_init(struct rte_eth_dev *dev)
 }
 
 /*
+ * Check if requested loopback mode is supported
+ */
+int
+ixgbe_check_supported_loopback_mode(struct rte_eth_dev *dev)
+{
+	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+	if (dev->data->dev_conf.lpbk_mode == IXGBE_LPBK_82599_TX_RX)
+		if (hw->mac.type == ixgbe_mac_82599EB)
+			return 0;
+
+	return -ENOTSUP;
+}
+
+/*
  * Set up link for 82599 loopback mode Tx->Rx.
  */
 static inline void __attribute__((cold))
@@ -5148,10 +5168,11 @@ ixgbe_dev_rxtx_start(struct rte_eth_dev *dev)
 	rxctrl |= IXGBE_RXCTRL_RXEN;
 	hw->mac.ops.enable_rx_dma(hw, rxctrl);
 
-	/* If loopback mode is enabled for 82599, set up the link accordingly */
-	if (hw->mac.type == ixgbe_mac_82599EB &&
-			dev->data->dev_conf.lpbk_mode == IXGBE_LPBK_82599_TX_RX)
-		ixgbe_setup_loopback_link_82599(hw);
+	/* If loopback mode is enabled, set up the link accordingly */
+	if (dev->data->dev_conf.lpbk_mode != 0) {
+		if (hw->mac.type == ixgbe_mac_82599EB)
+			ixgbe_setup_loopback_link_82599(hw);
+	}
 
 #ifdef RTE_LIBRTE_SECURITY
 	if ((dev->data->dev_conf.rxmode.offloads &
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.h b/drivers/net/ixgbe/ixgbe_rxtx.h
index 39378f7..505d344 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.h
+++ b/drivers/net/ixgbe/ixgbe_rxtx.h
@@ -276,6 +276,7 @@ void ixgbe_set_tx_function(struct rte_eth_dev *dev, struct ixgbe_tx_queue *txq);
  */
 void ixgbe_set_rx_function(struct rte_eth_dev *dev);
 
+int ixgbe_check_supported_loopback_mode(struct rte_eth_dev *dev);
 uint16_t ixgbe_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
 		uint16_t nb_pkts);
 uint16_t ixgbe_recv_scattered_pkts_vec(void *rx_queue,
-- 
2.10.2

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [dpdk-dev] [PATCH v3 2/2] net/ixgbe: add support of loopback for X540/X550
  2019-02-20 21:05 ` [dpdk-dev] [PATCH v3 " Julien Meunier
@ 2019-02-20 21:05   ` Julien Meunier
  2019-02-21  2:30     ` Zhao1, Wei
  2019-03-01  7:45   ` [dpdk-dev] [PATCH v3 1/2] net/ixgbe: do not start on unsupported loopback mode Zhang, Qi Z
  1 sibling, 1 reply; 20+ messages in thread
From: Julien Meunier @ 2019-02-20 21:05 UTC (permalink / raw)
  To: wei.zhao1, qi.z.zhang, Wenzhuo Lu, Konstantin Ananyev; +Cc: dev

Loopback mode is also supported on X540 and X550 NICs, according to
their datasheet (section 15.2). The way to set it up is a little
different of the 82599.

Signed-off-by: Julien Meunier <julien.meunier@nokia.com>
---
v3:
- reorganize and merge common code
- restore MACC_FLU on stop
v2:
- disable / enable autoneg when loopback is requested for X540 / X550
---
 drivers/net/ixgbe/ixgbe_ethdev.h |  7 +++---
 drivers/net/ixgbe/ixgbe_rxtx.c   | 53 ++++++++++++++++++++++++++++++++++++++--
 2 files changed, 55 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
index 565c69c..99a5077 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.h
+++ b/drivers/net/ixgbe/ixgbe_ethdev.h
@@ -65,9 +65,10 @@
 #define IXGBE_QUEUE_ITR_INTERVAL_DEFAULT	500 /* 500us */
 
 /* Loopback operation modes */
-/* 82599 specific loopback operation types */
-#define IXGBE_LPBK_82599_NONE   0x0 /* Default value. Loopback is disabled. */
-#define IXGBE_LPBK_82599_TX_RX  0x1 /* Tx->Rx loopback operation is enabled. */
+#define IXGBE_LPBK_NONE   0x0 /* Default value. Loopback is disabled. */
+#define IXGBE_LPBK_TX_RX  0x1 /* Tx->Rx loopback operation is enabled. */
+/* X540-X550 specific loopback operations */
+#define IXGBE_MII_AUTONEG_ENABLE        0x1000 /* Auto-negociation enable (default = 1) */
 
 #define IXGBE_MAX_JUMBO_FRAME_SIZE      0x2600 /* Maximum Jumbo frame size. */
 
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index c9a70a8..e92a70f 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -3168,12 +3168,44 @@ ixgbe_dev_tx_descriptor_status(void *tx_queue, uint16_t offset)
 	return RTE_ETH_TX_DESC_FULL;
 }
 
+/*
+ * Set up link loopback for X540/X550 mode Tx->Rx.
+ */
+static inline void __attribute__((cold))
+ixgbe_setup_loopback_link_x540_x550(struct ixgbe_hw *hw, bool enable)
+{
+	uint32_t macc;
+	PMD_INIT_FUNC_TRACE();
+
+	u16 autoneg_reg = IXGBE_MII_AUTONEG_REG;
+
+	hw->phy.ops.read_reg(hw, IXGBE_MDIO_AUTO_NEG_CONTROL,
+			     IXGBE_MDIO_AUTO_NEG_DEV_TYPE, &autoneg_reg);
+	macc = IXGBE_READ_REG(hw, IXGBE_MACC);
+
+	if (enable) {
+		/* datasheet 15.2.1: disable AUTONEG (PHY Bit 7.0.C) */
+		autoneg_reg |= IXGBE_MII_AUTONEG_ENABLE;
+		/* datasheet 15.2.1: MACC.FLU = 1 (force link up) */
+		macc |= IXGBE_MACC_FLU;
+	} else {
+		autoneg_reg &= ~IXGBE_MII_AUTONEG_ENABLE;
+		macc &= ~IXGBE_MACC_FLU;
+	}
+
+	hw->phy.ops.write_reg(hw, IXGBE_MDIO_AUTO_NEG_CONTROL,
+			      IXGBE_MDIO_AUTO_NEG_DEV_TYPE, autoneg_reg);
+
+	IXGBE_WRITE_REG(hw, IXGBE_MACC, macc);
+}
+
 void __attribute__((cold))
 ixgbe_dev_clear_queues(struct rte_eth_dev *dev)
 {
 	unsigned i;
 	struct ixgbe_adapter *adapter =
 		(struct ixgbe_adapter *)dev->data->dev_private;
+	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
 	PMD_INIT_FUNC_TRACE();
 
@@ -3194,6 +3226,14 @@ ixgbe_dev_clear_queues(struct rte_eth_dev *dev)
 			ixgbe_reset_rx_queue(adapter, rxq);
 		}
 	}
+	/* If loopback mode was enabled, reconfigure the link accordingly */
+	if (dev->data->dev_conf.lpbk_mode != 0) {
+		if (hw->mac.type == ixgbe_mac_X540 ||
+		     hw->mac.type == ixgbe_mac_X550 ||
+		     hw->mac.type == ixgbe_mac_X550EM_x ||
+		     hw->mac.type == ixgbe_mac_X550EM_a)
+			ixgbe_setup_loopback_link_x540_x550(hw, false);
+	}
 }
 
 void
@@ -5074,8 +5114,12 @@ ixgbe_check_supported_loopback_mode(struct rte_eth_dev *dev)
 {
 	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
-	if (dev->data->dev_conf.lpbk_mode == IXGBE_LPBK_82599_TX_RX)
-		if (hw->mac.type == ixgbe_mac_82599EB)
+	if (dev->data->dev_conf.lpbk_mode == IXGBE_LPBK_TX_RX)
+		if (hw->mac.type == ixgbe_mac_82599EB ||
+		     hw->mac.type == ixgbe_mac_X540 ||
+		     hw->mac.type == ixgbe_mac_X550 ||
+		     hw->mac.type == ixgbe_mac_X550EM_x ||
+		     hw->mac.type == ixgbe_mac_X550EM_a)
 			return 0;
 
 	return -ENOTSUP;
@@ -5172,6 +5216,11 @@ ixgbe_dev_rxtx_start(struct rte_eth_dev *dev)
 	if (dev->data->dev_conf.lpbk_mode != 0) {
 		if (hw->mac.type == ixgbe_mac_82599EB)
 			ixgbe_setup_loopback_link_82599(hw);
+		else if (hw->mac.type == ixgbe_mac_X540 ||
+		     hw->mac.type == ixgbe_mac_X550 ||
+		     hw->mac.type == ixgbe_mac_X550EM_x ||
+		     hw->mac.type == ixgbe_mac_X550EM_a)
+			ixgbe_setup_loopback_link_x540_x550(hw, true);
 	}
 
 #ifdef RTE_LIBRTE_SECURITY
-- 
2.10.2

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [PATCH v3 2/2] net/ixgbe: add support of loopback for X540/X550
  2019-02-20 21:05   ` [dpdk-dev] [PATCH v3 2/2] net/ixgbe: add support of loopback for X540/X550 Julien Meunier
@ 2019-02-21  2:30     ` Zhao1, Wei
  2019-03-01  7:46       ` Zhang, Qi Z
  0 siblings, 1 reply; 20+ messages in thread
From: Zhao1, Wei @ 2019-02-21  2:30 UTC (permalink / raw)
  To: Julien Meunier, Zhang, Qi Z, Lu, Wenzhuo, Ananyev, Konstantin; +Cc: dev

Acked-by: Wei Zhao <wei.zhao1@intel.com>


> -----Original Message-----
> From: Julien Meunier [mailto:julien.meunier@nokia.com]
> Sent: Thursday, February 21, 2019 5:06 AM
> To: Zhao1, Wei <wei.zhao1@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
> Lu, Wenzhuo <wenzhuo.lu@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>
> Cc: dev@dpdk.org
> Subject: [PATCH v3 2/2] net/ixgbe: add support of loopback for X540/X550
> 
> Loopback mode is also supported on X540 and X550 NICs, according to their
> datasheet (section 15.2). The way to set it up is a little different of the 82599.
> 
> Signed-off-by: Julien Meunier <julien.meunier@nokia.com>
> ---
> v3:
> - reorganize and merge common code
> - restore MACC_FLU on stop
> v2:
> - disable / enable autoneg when loopback is requested for X540 / X550
> ---
>  drivers/net/ixgbe/ixgbe_ethdev.h |  7 +++---
>  drivers/net/ixgbe/ixgbe_rxtx.c   | 53
> ++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 55 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h
> b/drivers/net/ixgbe/ixgbe_ethdev.h
> index 565c69c..99a5077 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.h
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.h
> @@ -65,9 +65,10 @@
>  #define IXGBE_QUEUE_ITR_INTERVAL_DEFAULT	500 /* 500us */
> 
>  /* Loopback operation modes */
> -/* 82599 specific loopback operation types */
> -#define IXGBE_LPBK_82599_NONE   0x0 /* Default value. Loopback is
> disabled. */
> -#define IXGBE_LPBK_82599_TX_RX  0x1 /* Tx->Rx loopback operation is
> enabled. */
> +#define IXGBE_LPBK_NONE   0x0 /* Default value. Loopback is disabled. */
> +#define IXGBE_LPBK_TX_RX  0x1 /* Tx->Rx loopback operation is enabled.
> +*/
> +/* X540-X550 specific loopback operations */
> +#define IXGBE_MII_AUTONEG_ENABLE        0x1000 /* Auto-negociation
> enable (default = 1) */
> 
>  #define IXGBE_MAX_JUMBO_FRAME_SIZE      0x2600 /* Maximum Jumbo
> frame size. */
> 
> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
> index c9a70a8..e92a70f 100644
> --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> @@ -3168,12 +3168,44 @@ ixgbe_dev_tx_descriptor_status(void *tx_queue,
> uint16_t offset)
>  	return RTE_ETH_TX_DESC_FULL;
>  }
> 
> +/*
> + * Set up link loopback for X540/X550 mode Tx->Rx.
> + */
> +static inline void __attribute__((cold))
> +ixgbe_setup_loopback_link_x540_x550(struct ixgbe_hw *hw, bool enable)
> {
> +	uint32_t macc;
> +	PMD_INIT_FUNC_TRACE();
> +
> +	u16 autoneg_reg = IXGBE_MII_AUTONEG_REG;
> +
> +	hw->phy.ops.read_reg(hw, IXGBE_MDIO_AUTO_NEG_CONTROL,
> +			     IXGBE_MDIO_AUTO_NEG_DEV_TYPE,
> &autoneg_reg);
> +	macc = IXGBE_READ_REG(hw, IXGBE_MACC);
> +
> +	if (enable) {
> +		/* datasheet 15.2.1: disable AUTONEG (PHY Bit 7.0.C) */
> +		autoneg_reg |= IXGBE_MII_AUTONEG_ENABLE;
> +		/* datasheet 15.2.1: MACC.FLU = 1 (force link up) */
> +		macc |= IXGBE_MACC_FLU;
> +	} else {
> +		autoneg_reg &= ~IXGBE_MII_AUTONEG_ENABLE;
> +		macc &= ~IXGBE_MACC_FLU;
> +	}
> +
> +	hw->phy.ops.write_reg(hw, IXGBE_MDIO_AUTO_NEG_CONTROL,
> +			      IXGBE_MDIO_AUTO_NEG_DEV_TYPE,
> autoneg_reg);
> +
> +	IXGBE_WRITE_REG(hw, IXGBE_MACC, macc); }
> +
>  void __attribute__((cold))
>  ixgbe_dev_clear_queues(struct rte_eth_dev *dev)  {
>  	unsigned i;
>  	struct ixgbe_adapter *adapter =
>  		(struct ixgbe_adapter *)dev->data->dev_private;
> +	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> 
>  	PMD_INIT_FUNC_TRACE();
> 
> @@ -3194,6 +3226,14 @@ ixgbe_dev_clear_queues(struct rte_eth_dev
> *dev)
>  			ixgbe_reset_rx_queue(adapter, rxq);
>  		}
>  	}
> +	/* If loopback mode was enabled, reconfigure the link accordingly */
> +	if (dev->data->dev_conf.lpbk_mode != 0) {
> +		if (hw->mac.type == ixgbe_mac_X540 ||
> +		     hw->mac.type == ixgbe_mac_X550 ||
> +		     hw->mac.type == ixgbe_mac_X550EM_x ||
> +		     hw->mac.type == ixgbe_mac_X550EM_a)
> +			ixgbe_setup_loopback_link_x540_x550(hw, false);
> +	}
>  }
> 
>  void
> @@ -5074,8 +5114,12 @@ ixgbe_check_supported_loopback_mode(struct
> rte_eth_dev *dev)  {
>  	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> 
> -	if (dev->data->dev_conf.lpbk_mode == IXGBE_LPBK_82599_TX_RX)
> -		if (hw->mac.type == ixgbe_mac_82599EB)
> +	if (dev->data->dev_conf.lpbk_mode == IXGBE_LPBK_TX_RX)
> +		if (hw->mac.type == ixgbe_mac_82599EB ||
> +		     hw->mac.type == ixgbe_mac_X540 ||
> +		     hw->mac.type == ixgbe_mac_X550 ||
> +		     hw->mac.type == ixgbe_mac_X550EM_x ||
> +		     hw->mac.type == ixgbe_mac_X550EM_a)
>  			return 0;
> 
>  	return -ENOTSUP;
> @@ -5172,6 +5216,11 @@ ixgbe_dev_rxtx_start(struct rte_eth_dev *dev)
>  	if (dev->data->dev_conf.lpbk_mode != 0) {
>  		if (hw->mac.type == ixgbe_mac_82599EB)
>  			ixgbe_setup_loopback_link_82599(hw);
> +		else if (hw->mac.type == ixgbe_mac_X540 ||
> +		     hw->mac.type == ixgbe_mac_X550 ||
> +		     hw->mac.type == ixgbe_mac_X550EM_x ||
> +		     hw->mac.type == ixgbe_mac_X550EM_a)
> +			ixgbe_setup_loopback_link_x540_x550(hw, true);
>  	}
> 
>  #ifdef RTE_LIBRTE_SECURITY
> --
> 2.10.2

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [PATCH v3 1/2] net/ixgbe: do not start on unsupported loopback mode
  2019-02-20 21:05 ` [dpdk-dev] [PATCH v3 " Julien Meunier
  2019-02-20 21:05   ` [dpdk-dev] [PATCH v3 2/2] net/ixgbe: add support of loopback for X540/X550 Julien Meunier
@ 2019-03-01  7:45   ` Zhang, Qi Z
  1 sibling, 0 replies; 20+ messages in thread
From: Zhang, Qi Z @ 2019-03-01  7:45 UTC (permalink / raw)
  To: Julien Meunier, Zhao1, Wei, Lu, Wenzhuo, Ananyev, Konstantin; +Cc: dev



> -----Original Message-----
> From: Julien Meunier [mailto:julien.meunier@nokia.com]
> Sent: Thursday, February 21, 2019 5:05 AM
> To: Zhao1, Wei <wei.zhao1@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Lu,
> Wenzhuo <wenzhuo.lu@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>
> Cc: dev@dpdk.org
> Subject: [PATCH v3 1/2] net/ixgbe: do not start on unsupported loopback mode
> 
> Only TX->RX loopback is supported currently on 82599EB. If a user wants to apply
> an another loopback configuration (!= IXGBE_LPBK_82599_TX_RX), ixgbe PMD
> ignores it and continues the configuration without raising any error.
> 
> Let's robustify this part by checking if the requested loopback mode is correct for
> the current device, before starting it. If it is not valid, PMD will refuse to start.
> 
> Signed-off-by: Julien Meunier <julien.meunier@nokia.com>

Acked-by: Wei Zhao <wei.zhao1@intel.com>

Applied to dpdk-next-net-intel.

Thanks
Qi

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [PATCH v3 2/2] net/ixgbe: add support of loopback for X540/X550
  2019-02-21  2:30     ` Zhao1, Wei
@ 2019-03-01  7:46       ` Zhang, Qi Z
  0 siblings, 0 replies; 20+ messages in thread
From: Zhang, Qi Z @ 2019-03-01  7:46 UTC (permalink / raw)
  To: Zhao1, Wei, Julien Meunier, Lu, Wenzhuo, Ananyev, Konstantin; +Cc: dev

> -----Original Message-----
> From: Zhao1, Wei
> Sent: Thursday, February 21, 2019 10:31 AM
> To: Julien Meunier <julien.meunier@nokia.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Ananyev,
> Konstantin <konstantin.ananyev@intel.com>
> Cc: dev@dpdk.org
> Subject: RE: [PATCH v3 2/2] net/ixgbe: add support of loopback for X540/X550
> 
> Acked-by: Wei Zhao <wei.zhao1@intel.com>

Applied to dpdk-next-net-intel.

Thanks
Qi

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2019-03-01  7:46 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-02 16:00 [dpdk-dev] [PATCH] net/ixgbe: add support of loopback for X540/X550 Julien Meunier
2019-01-07  6:53 ` Zhang, Qi Z
2019-01-07 15:53   ` Meunier, Julien (Nokia - FR/Paris-Saclay)
2019-01-08 10:10     ` Zhao1, Wei
2019-01-08 12:39       ` Zhang, Qi Z
2019-01-08 20:18         ` Meunier, Julien (Nokia - FR/Paris-Saclay)
2019-01-09  1:30         ` Zhao1, Wei
2019-01-07 10:05 ` Zhao1, Wei
2019-01-07 16:04   ` Meunier, Julien (Nokia - FR/Paris-Saclay)
2019-01-08  3:10     ` Zhao1, Wei
2019-02-07 17:30 ` [dpdk-dev] [PATCH 1/2] net/ixgbe: do not start on unsupported loopback mode Julien Meunier
2019-02-07 17:30   ` [dpdk-dev] [PATCH 2/2] net/ixgbe: add support of loopback for X540/X550 Julien Meunier
2019-02-12  6:37     ` Zhao1, Wei
2019-02-18 11:20       ` Meunier, Julien (Nokia - FR/Paris-Saclay)
2019-02-12  6:36   ` [dpdk-dev] [PATCH 1/2] net/ixgbe: do not start on unsupported loopback mode Zhao1, Wei
2019-02-20 21:05 ` [dpdk-dev] [PATCH v3 " Julien Meunier
2019-02-20 21:05   ` [dpdk-dev] [PATCH v3 2/2] net/ixgbe: add support of loopback for X540/X550 Julien Meunier
2019-02-21  2:30     ` Zhao1, Wei
2019-03-01  7:46       ` Zhang, Qi Z
2019-03-01  7:45   ` [dpdk-dev] [PATCH v3 1/2] net/ixgbe: do not start on unsupported loopback mode Zhang, Qi Z

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).