net/ixgbe: add support of loopback for X540/X550

Message ID 20190102160055.30301-1-julien.meunier@nokia.com (mailing list archive)
State Changes Requested, archived
Delegated to: Qi Zhang
Headers
Series net/ixgbe: add support of loopback for X540/X550 |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/mellanox-Performance-Testing success Performance Testing PASS
ci/intel-Performance-Testing success Performance Testing PASS
ci/Intel-compilation success Compilation OK

Commit Message

Julien Meunier Jan. 2, 2019, 4 p.m. UTC
  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(-)
  

Comments

Qi Zhang Jan. 7, 2019, 6:53 a.m. UTC | #1
> -----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
  
Zhao1, Wei Jan. 7, 2019, 10:05 a.m. UTC | #2
> -----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
  
Julien Meunier Jan. 7, 2019, 3:53 p.m. UTC | #3
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
>
  
Julien Meunier Jan. 7, 2019, 4:04 p.m. UTC | #4
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
  
Zhao1, Wei Jan. 8, 2019, 3:10 a.m. UTC | #5
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
  
Zhao1, Wei Jan. 8, 2019, 10:10 a.m. UTC | #6
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
> >
  
Qi Zhang Jan. 8, 2019, 12:39 p.m. UTC | #7
> -----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
> > >
  
Julien Meunier Jan. 8, 2019, 8:18 p.m. UTC | #8
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
>>>>
  
Zhao1, Wei Jan. 9, 2019, 1:30 a.m. UTC | #9
> -----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
> > > >
  

Patch

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 &