Re: [PATCH] Add support for 82599 Tx->Rx loopback operation.

[Ivan Boule <ivan.boule-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>]

    Hi Qinglai Xiao,

    See my remarks inline prefixed with IB>
    Best regards,
    Ivan

On 09/23/2013 09:16 PM, Qinglai Xiao wrote:
> Signed-off-by: Qinglai Xiao <jigsaw-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>   lib/librte_ether/rte_ethdev.h            |    1 +
>   lib/librte_pmd_ixgbe/ixgbe/ixgbe_82599.c |  122 +++++++++++++++++++++++++++++-
>   lib/librte_pmd_ixgbe/ixgbe/ixgbe_type.h  |    7 ++
>   lib/librte_pmd_ixgbe/ixgbe_ethdev.c      |    8 ++
>   lib/librte_pmd_ixgbe/ixgbe_rxtx.c        |    6 ++
>   5 files changed, 141 insertions(+), 3 deletions(-)
>
> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> index 2d7385f..f474e5b 100644
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -670,6 +670,7 @@ struct rte_eth_conf {
>   	/**< ETH_LINK_SPEED_10[0|00|000], or 0 for autonegotation */
>   	uint16_t link_duplex;
>   	/**< ETH_LINK_[HALF_DUPLEX|FULL_DUPLEX], or 0 for autonegotation */
> +	uint32_t lpbk; /**< Loopback operation. The value depends on ethernet controller. */
>   	struct rte_eth_rxmode rxmode; /**< Port RX configuration. */
>   	struct rte_eth_txmode txmode; /**< Port TX configuration. */
>   	union {
IB> As RX->TX loopback mode is not supported, only introduce the 
configuration of the
TX->RX loopback mode in the DPDK API as follows:

/**
  * A set of flags to identify which loopback mode to use, if any.
  */
enum rte_loopback_mode {
     RTE_LOOPBACK_NONE = 0, /**< No loopback */
     RTE_LOOPBACK_TX_RX,    /**< TX->RX loopback mode */
};

struct rte_eth_conf {
     uint16_t link_speed;
     /**< ETH_LINK_SPEED_10[0|00|000], or 0 for autonegotation */
     uint16_t link_duplex;
     /**< ETH_LINK_[HALF_DUPLEX|FULL_DUPLEX], or 0 for autonegotation */
     enum rte_loopback_mode lpbk_mode; /**< loopback mode. */
     ...
};

> diff --git a/lib/librte_pmd_ixgbe/ixgbe/ixgbe_82599.c b/lib/librte_pmd_ixgbe/ixgbe/ixgbe_82599.c
> index db07789..0416c01 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe/ixgbe_82599.c
> +++ b/lib/librte_pmd_ixgbe/ixgbe/ixgbe_82599.c
> @@ -48,6 +48,17 @@ STATIC s32 ixgbe_read_eeprom_82599(struct ixgbe_hw *hw,
>   STATIC s32 ixgbe_read_eeprom_buffer_82599(struct ixgbe_hw *hw, u16 offset,
>   					  u16 words, u16 *data);
>   
> +
> +STATIC s32 ixgbe_get_link_capabilities_82599_lpbk(struct ixgbe_hw *hw,
> +				      ixgbe_link_speed *speed,
> +				      bool *negotiation);
> +STATIC s32 ixgbe_check_mac_link_82599_lpbk(struct ixgbe_hw *hw, ixgbe_link_speed *speed,
> +				 bool *link_up, bool link_up_wait_to_complete);
> +STATIC s32 ixgbe_setup_mac_link_82599_lpbk(struct ixgbe_hw *hw,
> +			       ixgbe_link_speed speed, bool autoneg,
> +			       bool autoneg_wait_to_complete);
> +
> +
>   void ixgbe_init_mac_link_ops_82599(struct ixgbe_hw *hw)
>   {
>   	struct ixgbe_mac_info *mac = &hw->mac;
> @@ -68,7 +79,10 @@ void ixgbe_init_mac_link_ops_82599(struct ixgbe_hw *hw)
>   		mac->ops.flap_tx_laser = NULL;
>   	}
>   
> -	if (hw->phy.multispeed_fiber) {
> +	if (hw->lpbk == IXGBE_LPBK_82599_TX_RX) {
> +		/* Support for Tx->Rx loopback operation */
> +		mac->ops.setup_link = &ixgbe_setup_mac_link_82599_lpbk;
> +	} else if (hw->phy.multispeed_fiber) {
>   		/* Set up dual speed SFP+ support */
>   		mac->ops.setup_link = &ixgbe_setup_mac_link_multispeed_fiber;
>   	} else {
> @@ -266,8 +280,23 @@ s32 ixgbe_init_ops_82599(struct ixgbe_hw *hw)
>   	mac->ops.set_vlan_anti_spoofing = &ixgbe_set_vlan_anti_spoofing;
>   
>   	/* Link */
> -	mac->ops.get_link_capabilities = &ixgbe_get_link_capabilities_82599;
> -	mac->ops.check_link = &ixgbe_check_mac_link_generic;
> +
> +	/* 82599 has two loopback operations: Tx->Rx and Rx->Tx
> +	 * Only Tx->Rx is supported for now.
> +	 */
> +	switch (hw->lpbk) {
> +	case IXGBE_LPBK_82599_TX_RX:
> +		mac->ops.get_link_capabilities = &ixgbe_get_link_capabilities_82599_lpbk;
> +		mac->ops.check_link = &ixgbe_check_mac_link_82599_lpbk;
> +		break;
> +
> +	case IXGBE_LPBK_82599_NONE: /* FALL THRU */
> +	default:
> +		mac->ops.get_link_capabilities = &ixgbe_get_link_capabilities_82599;
> +		mac->ops.check_link = &ixgbe_check_mac_link_generic;
> +		break;
> +	}
> +
>   	mac->ops.setup_rxpba = &ixgbe_set_rxpba_generic;
>   	ixgbe_init_mac_link_ops_82599(hw);
>   
> @@ -2370,5 +2399,92 @@ reset_pipeline_out:
>   	return ret_val;
>   }
>   
> +/**
> + *  ixgbe_get_link_capabilities_82599_lpbk - Determines link capabilities for Tx->Rx loopback setting
> + *  @hw: pointer to hardware structure
> + *  @speed: pointer to link speed
> + *  @negotiation: always false
> + *
> + *  For Tx->Rx loopback only. Rx->Tx loopback is not supported for now.
> + *
> + *  @speed is always set to IXGBE_LINK_SPEED_10GB_FULL,
> + *  @negotiation is always set to false.
> + **/
> +STATIC s32 ixgbe_get_link_capabilities_82599_lpbk(struct ixgbe_hw *hw,
> +				      ixgbe_link_speed *speed,
> +				      bool *negotiation)
> +{
> +	DEBUGFUNC("ixgbe_get_link_capabilities_82599_lpbk");
> +
> +	*speed = IXGBE_LINK_SPEED_10GB_FULL;
> +	*negotiation = false;
> +
> +	return IXGBE_SUCCESS;
> +}
>   
> +/**
> + *  ixgbe_check_mac_link_82599_lpbk - Determine link and speed status for loopback Tx->Rx setting
> + *  @hw: pointer to hardware structure
> + *  @speed: pointer to link speed
> + *  @link_up: true when link is up
> + *  @link_up_wait_to_complete: bool used to wait for link up or not
> + *
> + *  For Tx->Rx loopback only. Rx->Tx loopback is not supported for now.
> + *
> + *  Regardless of link status (LINKS), always set @linkup to true,
> + *  and @speed to IXGBE_LINK_SPEED_10GB_FULL.
> + **/
> +STATIC s32 ixgbe_check_mac_link_82599_lpbk(struct ixgbe_hw *hw, ixgbe_link_speed *speed,
> +		bool *link_up, bool link_up_wait_to_complete)
> +{
> +	DEBUGFUNC("ixgbe_check_mac_link_82599_lpbk");
> +
> +	*link_up = true;
> +	*speed = IXGBE_LINK_SPEED_10GB_FULL;
> +
> +	return 0;
> +}
> +
> +/**
> + *  ixgbe_setup_mac_link_82599_loopback - Set MAC link speed for Tx->Rx loopback operation.
> + *  @hw: pointer to hardware structure
> + *  @speed: new link speed
> + *  @autoneg: true if autonegotiation enabled
> + *  @autoneg_wait_to_complete: true when waiting for completion is needed
> + *
> + *  For Tx->Rx loopback only. Rx->Tx loopback is not supported for now.
> + *
> + *  @speed, @autoneg and @autoneg_wait_to_complete are ignored.
> + *  Just set AUTOC to IXGBE_AUTOC_LMS_10G_LINK_NO_AN | IXGBE_AUTOC_FLU.
> + **/
> +STATIC s32 ixgbe_setup_mac_link_82599_lpbk(struct ixgbe_hw *hw,
> +			       ixgbe_link_speed speed, bool autoneg,
> +			       bool autoneg_wait_to_complete)
> +{
> +	u32 autoc;
> +	u32 status = IXGBE_SUCCESS;
> +
> +	DEBUGFUNC("ixgbe_setup_mac_link_82599_lpbk");
> +	autoc = IXGBE_AUTOC_LMS_10G_LINK_NO_AN | IXGBE_AUTOC_FLU;
> +
> +	if (ixgbe_verify_lesm_fw_enabled_82599(hw)) {
> +		status = hw->mac.ops.acquire_swfw_sync(hw,
> +				IXGBE_GSSR_MAC_CSR_SM);
> +		if (status != IXGBE_SUCCESS) {
> +			status = IXGBE_ERR_SWFW_SYNC;
> +			goto out;
> +		}
> +	}
> +
> +	/* Restart link */
> +	IXGBE_WRITE_REG(hw, IXGBE_AUTOC, autoc);
> +	ixgbe_reset_pipeline_82599(hw);
> +
> +	hw->mac.ops.release_swfw_sync(hw,
> +			IXGBE_GSSR_MAC_CSR_SM);
> +	msec_delay(50);
> +
> +out:
> +	return status;
> +}
>   
> diff --git a/lib/librte_pmd_ixgbe/ixgbe/ixgbe_type.h b/lib/librte_pmd_ixgbe/ixgbe/ixgbe_type.h
> index 7fffd60..a31c9f7 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe/ixgbe_type.h
> +++ b/lib/librte_pmd_ixgbe/ixgbe/ixgbe_type.h
> @@ -2352,6 +2352,12 @@ enum ixgbe_fdir_pballoc_type {
>   #define FW_CEM_MAX_RETRIES		3
>   #define FW_CEM_RESP_STATUS_SUCCESS	0x1
>   
> +/* Loopback operation types */
> +/* 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_82599_RX_TX		0x2 /* Rx->Tx loopback operation is enabled. */
> +
>   /* Host Interface Command Structures */
>   
>   struct ixgbe_hic_hdr {
> @@ -3150,6 +3156,7 @@ struct ixgbe_hw {
>   	int api_version;
>   	bool force_full_reset;
>   	bool allow_unsupported_sfp;
> +	uint32_t lpbk;
IB> A uint8_t is enough.
>   };
>   
>   #define ixgbe_call_func(hw, func, params, error) \
> diff --git a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
> index 9235f9d..09600bc 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
> +++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
> @@ -1137,6 +1137,14 @@ ixgbe_dev_configure(struct rte_eth_dev *dev)
>   
>   	PMD_INIT_FUNC_TRACE();
>   
> +	if (dev->data->dev_conf.lpbk) {
IB> replace the line above by:
+    if (dev->data->dev_conf.lpbk_mode != RTE_LOOPBACK_NONE) {
> +		struct ixgbe_hw *hw =
> +			IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +
> +		hw->lpbk = dev->data->dev_conf.lpbk;
IB> replace the line above by:
+        /* Only supports TX->RX loopback mode for now. */
+        hw->lpbk = IXGBE_LPBK_82599_TX_RX;
> +	}
> +
> +
>   	/* set flag to update link status after init */
>   	intr->flags |= IXGBE_FLAG_NEED_LINK_UPDATE;
>   
> diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> index 5fba01d..158da0e 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> @@ -3252,6 +3252,12 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
>   	} else
>   		hlreg0 &= ~IXGBE_HLREG0_JUMBOEN;
>   
> +	/*
> +	 * Both Tx->Rx and Rx->Tx requres IXGBE_HLREG0_LPBK to be set.
IB> requres -> requires
> +	 */
> +	if (hw->lpbk)
> +		hlreg0 |= IXGBE_HLREG0_LPBK;
> +
>   	IXGBE_WRITE_REG(hw, IXGBE_HLREG0, hlreg0);
>   
>   	/* Setup RX queues */


-- 
Ivan Boule
6WIND Development Engineer