* [dpdk-dev] [PATCH] Add support for 82599 Tx->Rx loopback operation.
@ 2013-09-23 19:16 Qinglai Xiao
2013-09-25 13:11 ` Ivan Boule
0 siblings, 1 reply; 12+ messages in thread
From: Qinglai Xiao @ 2013-09-23 19:16 UTC (permalink / raw)
To: dev
Signed-off-by: Qinglai Xiao <jigsaw@gmail.com>
---
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 {
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;
};
#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) {
+ struct ixgbe_hw *hw =
+ IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+ hw->lpbk = dev->data->dev_conf.lpbk;
+ }
+
+
/* 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.
+ */
+ if (hw->lpbk)
+ hlreg0 |= IXGBE_HLREG0_LPBK;
+
IXGBE_WRITE_REG(hw, IXGBE_HLREG0, hlreg0);
/* Setup RX queues */
--
1.7.10.4
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [dpdk-dev] [PATCH] Add support for 82599 Tx->Rx loopback operation.
2013-09-23 19:16 [dpdk-dev] [PATCH] Add support for 82599 Tx->Rx loopback operation Qinglai Xiao
@ 2013-09-25 13:11 ` Ivan Boule
2013-09-25 13:56 ` jigsaw
0 siblings, 1 reply; 12+ messages in thread
From: Ivan Boule @ 2013-09-25 13:11 UTC (permalink / raw)
To: Qinglai Xiao; +Cc: dev
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@gmail.com>
> ---
> 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [dpdk-dev] [PATCH] Add support for 82599 Tx->Rx loopback operation.
2013-09-25 13:11 ` Ivan Boule
@ 2013-09-25 13:56 ` jigsaw
2013-09-25 14:12 ` Venkatesan, Venky
2013-09-25 15:04 ` Ivan Boule
0 siblings, 2 replies; 12+ messages in thread
From: jigsaw @ 2013-09-25 13:56 UTC (permalink / raw)
To: Ivan Boule; +Cc: dev
Hi Ivan,
Appreciations for your comments.
I have one question regarding the new field, lpbk_mode, in struct rte_eth_conf.
I was thinking how to define the values for this field, then I had a dilemma.
82599 has only two mode of loopback, either Tx->Rx or Rx->Tx.
But other controller may have different ideas. For instance, I350 can
be set as either MAC, PHY or SGMII loopback mode.
So if we define loopback mode as a enum type in rte_ethdev.h, we then
have to expose the driver level details.
That is, the enum rte_loopback_mode will be sth. like:
enum rte_loopback_mode {
RTE_LOOPBACK_NONE,
RTE_LOOPBACK_82599_TX_RX,
RTE_LOOPBACK_I350_MAC,
RTE_LOOPBACK_I350_PHY,
/* will be more if we add support for other controllers */
};
IMO it doesn't look so nice coz the hardware specific details are
exposed in a higher level API.
However, if we don't expose these details here, like in the patch, the
value is just a integer,
user of the API may get confused, and he/she still has to be aware of
what are possible values for
his/her eth controller.
There may be more subtle problems. It's not clear to me whether the
loopback mode of
certain controller is mutually exclusive. For instance, is it possible
that the Rx-Tx and Tx-Rx can
be activated at the same time for 82599? If so then the lpbk_mode has
to be defined as bitfield.
Having these questions in mind, I decided to expose just a uint32_t in
rte_eth_conf, so that the solution
is open for further changes. I should have stated my thoughts before
sending the patch, and I hope it's not
too late to open the discussion at this moment.
Looking forward to your advice.
thx &
rgds,
-qinglai
On Wed, Sep 25, 2013 at 4:11 PM, Ivan Boule <ivan.boule@6wind.com> wrote:
> 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@gmail.com>
> ---
> 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [dpdk-dev] [PATCH] Add support for 82599 Tx->Rx loopback operation.
2013-09-25 13:56 ` jigsaw
@ 2013-09-25 14:12 ` Venkatesan, Venky
2013-09-25 14:38 ` jigsaw
2013-09-25 14:47 ` Thomas Monjalon
2013-09-25 15:04 ` Ivan Boule
1 sibling, 2 replies; 12+ messages in thread
From: Venkatesan, Venky @ 2013-09-25 14:12 UTC (permalink / raw)
To: jigsaw, Ivan Boule; +Cc: dev
Qinglai/Ivan,
I for one would prefer that the changes not really modify any files in the librte_pmd_ixgbe/ixgbe directory. Those files are derived directly from the BSD driver baseline, and any changes will make future merges of newer code more challenging. The changes should be limited to files in the librte_pmd_ixgbe directory (and ethdev).
Regards,
-Venky
-----Original Message-----
From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of jigsaw
Sent: Wednesday, September 25, 2013 6:56 AM
To: Ivan Boule
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH] Add support for 82599 Tx->Rx loopback operation.
Hi Ivan,
Appreciations for your comments.
I have one question regarding the new field, lpbk_mode, in struct rte_eth_conf.
I was thinking how to define the values for this field, then I had a dilemma.
82599 has only two mode of loopback, either Tx->Rx or Rx->Tx.
But other controller may have different ideas. For instance, I350 can be set as either MAC, PHY or SGMII loopback mode.
So if we define loopback mode as a enum type in rte_ethdev.h, we then have to expose the driver level details.
That is, the enum rte_loopback_mode will be sth. like:
enum rte_loopback_mode {
RTE_LOOPBACK_NONE,
RTE_LOOPBACK_82599_TX_RX,
RTE_LOOPBACK_I350_MAC,
RTE_LOOPBACK_I350_PHY,
/* will be more if we add support for other controllers */ };
IMO it doesn't look so nice coz the hardware specific details are exposed in a higher level API.
However, if we don't expose these details here, like in the patch, the value is just a integer, user of the API may get confused, and he/she still has to be aware of what are possible values for his/her eth controller.
There may be more subtle problems. It's not clear to me whether the loopback mode of certain controller is mutually exclusive. For instance, is it possible that the Rx-Tx and Tx-Rx can be activated at the same time for 82599? If so then the lpbk_mode has to be defined as bitfield.
Having these questions in mind, I decided to expose just a uint32_t in rte_eth_conf, so that the solution is open for further changes. I should have stated my thoughts before sending the patch, and I hope it's not too late to open the discussion at this moment.
Looking forward to your advice.
thx &
rgds,
-qinglai
On Wed, Sep 25, 2013 at 4:11 PM, Ivan Boule <ivan.boule@6wind.com> wrote:
> 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@gmail.com>
> ---
> 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [dpdk-dev] [PATCH] Add support for 82599 Tx->Rx loopback operation.
2013-09-25 14:12 ` Venkatesan, Venky
@ 2013-09-25 14:38 ` jigsaw
2013-09-25 15:47 ` Venkatesan, Venky
2013-09-25 14:47 ` Thomas Monjalon
1 sibling, 1 reply; 12+ messages in thread
From: jigsaw @ 2013-09-25 14:38 UTC (permalink / raw)
To: Venkatesan, Venky; +Cc: dev
Hi Venky,
Thanks for you comments.
>>I for one would prefer that the changes not really modify any files in the librte_pmd_ixgbe/ixgbe directory
With this restriction it is gonna be little bit difficult but still feasible.
The solution would be after calling ixgbe_init_shared_code in
ixgbe_ethdev.c, if the config is for loopback operation, we
immediately replace the callbacks of:
mac->ops.get_link_capabilities
mac->ops.check_link
mac->ops.setup_link
And of course the implementation of these callbacks can be in a new
src file under librte_pmd_ixgbe, thus keep the baseline untouched.
Sounds like a plan?
thx &
rgds,
-qinglai
On Wed, Sep 25, 2013 at 5:12 PM, Venkatesan, Venky
<venky.venkatesan@intel.com> wrote:
> Qinglai/Ivan,
>
> I for one would prefer that the changes not really modify any files in the librte_pmd_ixgbe/ixgbe directory. Those files are derived directly from the BSD driver baseline, and any changes will make future merges of newer code more challenging. The changes should be limited to files in the librte_pmd_ixgbe directory (and ethdev).
>
> Regards,
> -Venky
>
>
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of jigsaw
> Sent: Wednesday, September 25, 2013 6:56 AM
> To: Ivan Boule
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] Add support for 82599 Tx->Rx loopback operation.
>
> Hi Ivan,
>
> Appreciations for your comments.
> I have one question regarding the new field, lpbk_mode, in struct rte_eth_conf.
>
> I was thinking how to define the values for this field, then I had a dilemma.
>
> 82599 has only two mode of loopback, either Tx->Rx or Rx->Tx.
> But other controller may have different ideas. For instance, I350 can be set as either MAC, PHY or SGMII loopback mode.
> So if we define loopback mode as a enum type in rte_ethdev.h, we then have to expose the driver level details.
> That is, the enum rte_loopback_mode will be sth. like:
>
> enum rte_loopback_mode {
> RTE_LOOPBACK_NONE,
> RTE_LOOPBACK_82599_TX_RX,
> RTE_LOOPBACK_I350_MAC,
> RTE_LOOPBACK_I350_PHY,
> /* will be more if we add support for other controllers */ };
>
> IMO it doesn't look so nice coz the hardware specific details are exposed in a higher level API.
>
> However, if we don't expose these details here, like in the patch, the value is just a integer, user of the API may get confused, and he/she still has to be aware of what are possible values for his/her eth controller.
>
> There may be more subtle problems. It's not clear to me whether the loopback mode of certain controller is mutually exclusive. For instance, is it possible that the Rx-Tx and Tx-Rx can be activated at the same time for 82599? If so then the lpbk_mode has to be defined as bitfield.
>
> Having these questions in mind, I decided to expose just a uint32_t in rte_eth_conf, so that the solution is open for further changes. I should have stated my thoughts before sending the patch, and I hope it's not too late to open the discussion at this moment.
>
> Looking forward to your advice.
>
> thx &
> rgds,
> -qinglai
>
>
> On Wed, Sep 25, 2013 at 4:11 PM, Ivan Boule <ivan.boule@6wind.com> wrote:
>> 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@gmail.com>
>> ---
>> 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [dpdk-dev] [PATCH] Add support for 82599 Tx->Rx loopback operation.
2013-09-25 14:12 ` Venkatesan, Venky
2013-09-25 14:38 ` jigsaw
@ 2013-09-25 14:47 ` Thomas Monjalon
2013-09-25 14:56 ` jigsaw
1 sibling, 1 reply; 12+ messages in thread
From: Thomas Monjalon @ 2013-09-25 14:47 UTC (permalink / raw)
To: jigsaw; +Cc: dev
25/09/2013 16:12, Venkatesan, Venky :
> I for one would prefer that the changes not really modify any files in the
> librte_pmd_ixgbe/ixgbe directory. Those files are derived directly from
> the BSD driver baseline, and any changes will make future merges of newer
> code more challenging. The changes should be limited to files in the
> librte_pmd_ixgbe directory (and ethdev).
I agree with Venky.
The poll-mode drivers e1000 and ixgbe are based on drivers located in
subdirectories e1000/ and ixgbe/. And by design, these poll-mode drivers use
the base drivers without modifying it.
So please try to restrict your changes to ixgbe_ethdev.c and ixgbe_rxtx.c.
The coding rules are not written. If needed, we could add a file for it.
Thanks for your patch
--
Thomas
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [dpdk-dev] [PATCH] Add support for 82599 Tx->Rx loopback operation.
2013-09-25 14:47 ` Thomas Monjalon
@ 2013-09-25 14:56 ` jigsaw
0 siblings, 0 replies; 12+ messages in thread
From: jigsaw @ 2013-09-25 14:56 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: dev
Hi Thomas,
The patch can be made without modifying ixgbe/ at all, I will make
another attempt. Please see the proposal in my previous email.
The question is how to keep the lpbk_mode field in struct rte_eth_conf
future-proof. Or if this is overkill to think about possible future
loopback support at this moment?
thx &
rgds,
-qinglai
On Wed, Sep 25, 2013 at 5:47 PM, Thomas Monjalon
<thomas.monjalon@6wind.com> wrote:
> 25/09/2013 16:12, Venkatesan, Venky :
>> I for one would prefer that the changes not really modify any files in the
>> librte_pmd_ixgbe/ixgbe directory. Those files are derived directly from
>> the BSD driver baseline, and any changes will make future merges of newer
>> code more challenging. The changes should be limited to files in the
>> librte_pmd_ixgbe directory (and ethdev).
>
> I agree with Venky.
> The poll-mode drivers e1000 and ixgbe are based on drivers located in
> subdirectories e1000/ and ixgbe/. And by design, these poll-mode drivers use
> the base drivers without modifying it.
> So please try to restrict your changes to ixgbe_ethdev.c and ixgbe_rxtx.c.
>
> The coding rules are not written. If needed, we could add a file for it.
>
> Thanks for your patch
> --
> Thomas
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [dpdk-dev] [PATCH] Add support for 82599 Tx->Rx loopback operation.
2013-09-25 13:56 ` jigsaw
2013-09-25 14:12 ` Venkatesan, Venky
@ 2013-09-25 15:04 ` Ivan Boule
1 sibling, 0 replies; 12+ messages in thread
From: Ivan Boule @ 2013-09-25 15:04 UTC (permalink / raw)
To: jigsaw; +Cc: dev
Hi Qinglai,
I understand your point.
In fact, because loopback modes are only intended to be used in
debug situations by developers that [are supposed to] know what they
do, it is much simpler to use your approach.
Please, can you add into the file librte_ether/rte_ethdev.h comments
that are based on your arguments?
I also agree with your rework proposal to meet Venky's demand to not
change files in the ixgbe sub-directory.
Best regards,
Ivan
On 09/25/2013 03:56 PM, jigsaw wrote:
> Hi Ivan,
>
> Appreciations for your comments.
> I have one question regarding the new field, lpbk_mode, in struct rte_eth_conf.
>
> I was thinking how to define the values for this field, then I had a dilemma.
>
> 82599 has only two mode of loopback, either Tx->Rx or Rx->Tx.
> But other controller may have different ideas. For instance, I350 can
> be set as either MAC, PHY or SGMII loopback mode.
> So if we define loopback mode as a enum type in rte_ethdev.h, we then
> have to expose the driver level details.
> That is, the enum rte_loopback_mode will be sth. like:
>
> enum rte_loopback_mode {
> RTE_LOOPBACK_NONE,
> RTE_LOOPBACK_82599_TX_RX,
> RTE_LOOPBACK_I350_MAC,
> RTE_LOOPBACK_I350_PHY,
> /* will be more if we add support for other controllers */
> };
>
> IMO it doesn't look so nice coz the hardware specific details are
> exposed in a higher level API.
>
> However, if we don't expose these details here, like in the patch, the
> value is just a integer,
> user of the API may get confused, and he/she still has to be aware of
> what are possible values for
> his/her eth controller.
>
> There may be more subtle problems. It's not clear to me whether the
> loopback mode of
> certain controller is mutually exclusive. For instance, is it possible
> that the Rx-Tx and Tx-Rx can
> be activated at the same time for 82599? If so then the lpbk_mode has
> to be defined as bitfield.
>
> Having these questions in mind, I decided to expose just a uint32_t in
> rte_eth_conf, so that the solution
> is open for further changes. I should have stated my thoughts before
> sending the patch, and I hope it's not
> too late to open the discussion at this moment.
>
> Looking forward to your advice.
>
> thx &
> rgds,
> -qinglai
>
>
> On Wed, Sep 25, 2013 at 4:11 PM, Ivan Boule <ivan.boule@6wind.com> wrote:
>> 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@gmail.com>
>> ---
>> 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
--
Ivan Boule
6WIND Development Engineer
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [dpdk-dev] [PATCH] Add support for 82599 Tx->Rx loopback operation.
2013-09-25 14:38 ` jigsaw
@ 2013-09-25 15:47 ` Venkatesan, Venky
2013-09-25 16:59 ` jigsaw
0 siblings, 1 reply; 12+ messages in thread
From: Venkatesan, Venky @ 2013-09-25 15:47 UTC (permalink / raw)
To: jigsaw; +Cc: dev
Thanks a bunch. It does work.
One more suggestion to think about; Once you set the FLU (force link up) bit (bit 0) in the AUTOC (0x42A0) register, the auto-negotiation state will be set to AN_GOOD, the link-up indication should be set regardless. Would you still need the callbacks? You could then merge the setup_link code into something like dev_rxtx_start() [ open to suggestions ].
Regards,
-Venky
-----Original Message-----
From: jigsaw [mailto:jigsaw@gmail.com]
Sent: Wednesday, September 25, 2013 7:39 AM
To: Venkatesan, Venky
Cc: Ivan Boule; dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH] Add support for 82599 Tx->Rx loopback operation.
Hi Venky,
Thanks for you comments.
>>I for one would prefer that the changes not really modify any files in
>>the librte_pmd_ixgbe/ixgbe directory
With this restriction it is gonna be little bit difficult but still feasible.
The solution would be after calling ixgbe_init_shared_code in ixgbe_ethdev.c, if the config is for loopback operation, we immediately replace the callbacks of:
mac->ops.get_link_capabilities
mac->ops.check_link
mac->ops.setup_link
And of course the implementation of these callbacks can be in a new src file under librte_pmd_ixgbe, thus keep the baseline untouched.
Sounds like a plan?
thx &
rgds,
-qinglai
On Wed, Sep 25, 2013 at 5:12 PM, Venkatesan, Venky <venky.venkatesan@intel.com> wrote:
> Qinglai/Ivan,
>
> I for one would prefer that the changes not really modify any files in the librte_pmd_ixgbe/ixgbe directory. Those files are derived directly from the BSD driver baseline, and any changes will make future merges of newer code more challenging. The changes should be limited to files in the librte_pmd_ixgbe directory (and ethdev).
>
> Regards,
> -Venky
>
>
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of jigsaw
> Sent: Wednesday, September 25, 2013 6:56 AM
> To: Ivan Boule
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] Add support for 82599 Tx->Rx loopback operation.
>
> Hi Ivan,
>
> Appreciations for your comments.
> I have one question regarding the new field, lpbk_mode, in struct rte_eth_conf.
>
> I was thinking how to define the values for this field, then I had a dilemma.
>
> 82599 has only two mode of loopback, either Tx->Rx or Rx->Tx.
> But other controller may have different ideas. For instance, I350 can be set as either MAC, PHY or SGMII loopback mode.
> So if we define loopback mode as a enum type in rte_ethdev.h, we then have to expose the driver level details.
> That is, the enum rte_loopback_mode will be sth. like:
>
> enum rte_loopback_mode {
> RTE_LOOPBACK_NONE,
> RTE_LOOPBACK_82599_TX_RX,
> RTE_LOOPBACK_I350_MAC,
> RTE_LOOPBACK_I350_PHY,
> /* will be more if we add support for other controllers */ };
>
> IMO it doesn't look so nice coz the hardware specific details are exposed in a higher level API.
>
> However, if we don't expose these details here, like in the patch, the value is just a integer, user of the API may get confused, and he/she still has to be aware of what are possible values for his/her eth controller.
>
> There may be more subtle problems. It's not clear to me whether the loopback mode of certain controller is mutually exclusive. For instance, is it possible that the Rx-Tx and Tx-Rx can be activated at the same time for 82599? If so then the lpbk_mode has to be defined as bitfield.
>
> Having these questions in mind, I decided to expose just a uint32_t in rte_eth_conf, so that the solution is open for further changes. I should have stated my thoughts before sending the patch, and I hope it's not too late to open the discussion at this moment.
>
> Looking forward to your advice.
>
> thx &
> rgds,
> -qinglai
>
>
> On Wed, Sep 25, 2013 at 4:11 PM, Ivan Boule <ivan.boule@6wind.com> wrote:
>> 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@gmail.com>
>> ---
>> 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [dpdk-dev] [PATCH] Add support for 82599 Tx->Rx loopback operation.
2013-09-25 15:47 ` Venkatesan, Venky
@ 2013-09-25 16:59 ` jigsaw
2013-09-25 17:37 ` Venkatesan, Venky
2013-09-26 7:27 ` Ivan Boule
0 siblings, 2 replies; 12+ messages in thread
From: jigsaw @ 2013-09-25 16:59 UTC (permalink / raw)
To: Venkatesan, Venky; +Cc: dev
Hi Venky,
Good point. Thus we avoid not only replacing all callbacks, but also
adding lpbk field in struct ixgbe_hw.
All loopback-aware operations will be then restricted to function
ixgbe_dev_start and ixgbe_dev_rxtx_start.
The logic of ixgbe_dev_start will be sth. like:
...
...
ixgbe_dev_tx_init(dev);
...
...
if (loopback is configured)
goto skip_setup_link;
...
...
err = ixgbe_setup_link(hw, speed, negotiate, link_up);
if (err)
goto error;
skip_setup_link:
...
...
Meantime, as you suggested, the FLU bit will be set in
ixgbe_dev_rxtx_start, if loopback is configured.
I can send the patch later tomorrow if all agree with this proposal.
thx &
rgds,
-qinglai
On Wed, Sep 25, 2013 at 6:47 PM, Venkatesan, Venky
<venky.venkatesan@intel.com> wrote:
> Thanks a bunch. It does work.
>
> One more suggestion to think about; Once you set the FLU (force link up) bit (bit 0) in the AUTOC (0x42A0) register, the auto-negotiation state will be set to AN_GOOD, the link-up indication should be set regardless. Would you still need the callbacks? You could then merge the setup_link code into something like dev_rxtx_start() [ open to suggestions ].
>
> Regards,
> -Venky
>
> -----Original Message-----
> From: jigsaw [mailto:jigsaw@gmail.com]
> Sent: Wednesday, September 25, 2013 7:39 AM
> To: Venkatesan, Venky
> Cc: Ivan Boule; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] Add support for 82599 Tx->Rx loopback operation.
>
> Hi Venky,
>
> Thanks for you comments.
>
>>>I for one would prefer that the changes not really modify any files in
>>>the librte_pmd_ixgbe/ixgbe directory
>
> With this restriction it is gonna be little bit difficult but still feasible.
>
> The solution would be after calling ixgbe_init_shared_code in ixgbe_ethdev.c, if the config is for loopback operation, we immediately replace the callbacks of:
>
> mac->ops.get_link_capabilities
> mac->ops.check_link
> mac->ops.setup_link
>
> And of course the implementation of these callbacks can be in a new src file under librte_pmd_ixgbe, thus keep the baseline untouched.
>
> Sounds like a plan?
>
> thx &
> rgds,
> -qinglai
>
>
> On Wed, Sep 25, 2013 at 5:12 PM, Venkatesan, Venky <venky.venkatesan@intel.com> wrote:
>> Qinglai/Ivan,
>>
>> I for one would prefer that the changes not really modify any files in the librte_pmd_ixgbe/ixgbe directory. Those files are derived directly from the BSD driver baseline, and any changes will make future merges of newer code more challenging. The changes should be limited to files in the librte_pmd_ixgbe directory (and ethdev).
>>
>> Regards,
>> -Venky
>>
>>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of jigsaw
>> Sent: Wednesday, September 25, 2013 6:56 AM
>> To: Ivan Boule
>> Cc: dev@dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH] Add support for 82599 Tx->Rx loopback operation.
>>
>> Hi Ivan,
>>
>> Appreciations for your comments.
>> I have one question regarding the new field, lpbk_mode, in struct rte_eth_conf.
>>
>> I was thinking how to define the values for this field, then I had a dilemma.
>>
>> 82599 has only two mode of loopback, either Tx->Rx or Rx->Tx.
>> But other controller may have different ideas. For instance, I350 can be set as either MAC, PHY or SGMII loopback mode.
>> So if we define loopback mode as a enum type in rte_ethdev.h, we then have to expose the driver level details.
>> That is, the enum rte_loopback_mode will be sth. like:
>>
>> enum rte_loopback_mode {
>> RTE_LOOPBACK_NONE,
>> RTE_LOOPBACK_82599_TX_RX,
>> RTE_LOOPBACK_I350_MAC,
>> RTE_LOOPBACK_I350_PHY,
>> /* will be more if we add support for other controllers */ };
>>
>> IMO it doesn't look so nice coz the hardware specific details are exposed in a higher level API.
>>
>> However, if we don't expose these details here, like in the patch, the value is just a integer, user of the API may get confused, and he/she still has to be aware of what are possible values for his/her eth controller.
>>
>> There may be more subtle problems. It's not clear to me whether the loopback mode of certain controller is mutually exclusive. For instance, is it possible that the Rx-Tx and Tx-Rx can be activated at the same time for 82599? If so then the lpbk_mode has to be defined as bitfield.
>>
>> Having these questions in mind, I decided to expose just a uint32_t in rte_eth_conf, so that the solution is open for further changes. I should have stated my thoughts before sending the patch, and I hope it's not too late to open the discussion at this moment.
>>
>> Looking forward to your advice.
>>
>> thx &
>> rgds,
>> -qinglai
>>
>>
>> On Wed, Sep 25, 2013 at 4:11 PM, Ivan Boule <ivan.boule@6wind.com> wrote:
>>> 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@gmail.com>
>>> ---
>>> 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [dpdk-dev] [PATCH] Add support for 82599 Tx->Rx loopback operation.
2013-09-25 16:59 ` jigsaw
@ 2013-09-25 17:37 ` Venkatesan, Venky
2013-09-26 7:27 ` Ivan Boule
1 sibling, 0 replies; 12+ messages in thread
From: Venkatesan, Venky @ 2013-09-25 17:37 UTC (permalink / raw)
To: jigsaw; +Cc: dev
That should work perfectly ... :)
Regards,
-Venky
-----Original Message-----
From: jigsaw [mailto:jigsaw@gmail.com]
Sent: Wednesday, September 25, 2013 10:00 AM
To: Venkatesan, Venky
Cc: Ivan Boule; dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH] Add support for 82599 Tx->Rx loopback operation.
Hi Venky,
Good point. Thus we avoid not only replacing all callbacks, but also adding lpbk field in struct ixgbe_hw.
All loopback-aware operations will be then restricted to function ixgbe_dev_start and ixgbe_dev_rxtx_start.
The logic of ixgbe_dev_start will be sth. like:
...
...
ixgbe_dev_tx_init(dev);
...
...
if (loopback is configured)
goto skip_setup_link;
...
...
err = ixgbe_setup_link(hw, speed, negotiate, link_up);
if (err)
goto error;
skip_setup_link:
...
...
Meantime, as you suggested, the FLU bit will be set in ixgbe_dev_rxtx_start, if loopback is configured.
I can send the patch later tomorrow if all agree with this proposal.
thx &
rgds,
-qinglai
On Wed, Sep 25, 2013 at 6:47 PM, Venkatesan, Venky <venky.venkatesan@intel.com> wrote:
> Thanks a bunch. It does work.
>
> One more suggestion to think about; Once you set the FLU (force link up) bit (bit 0) in the AUTOC (0x42A0) register, the auto-negotiation state will be set to AN_GOOD, the link-up indication should be set regardless. Would you still need the callbacks? You could then merge the setup_link code into something like dev_rxtx_start() [ open to suggestions ].
>
> Regards,
> -Venky
>
> -----Original Message-----
> From: jigsaw [mailto:jigsaw@gmail.com]
> Sent: Wednesday, September 25, 2013 7:39 AM
> To: Venkatesan, Venky
> Cc: Ivan Boule; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] Add support for 82599 Tx->Rx loopback operation.
>
> Hi Venky,
>
> Thanks for you comments.
>
>>>I for one would prefer that the changes not really modify any files
>>>in the librte_pmd_ixgbe/ixgbe directory
>
> With this restriction it is gonna be little bit difficult but still feasible.
>
> The solution would be after calling ixgbe_init_shared_code in ixgbe_ethdev.c, if the config is for loopback operation, we immediately replace the callbacks of:
>
> mac->ops.get_link_capabilities
> mac->ops.check_link
> mac->ops.setup_link
>
> And of course the implementation of these callbacks can be in a new src file under librte_pmd_ixgbe, thus keep the baseline untouched.
>
> Sounds like a plan?
>
> thx &
> rgds,
> -qinglai
>
>
> On Wed, Sep 25, 2013 at 5:12 PM, Venkatesan, Venky <venky.venkatesan@intel.com> wrote:
>> Qinglai/Ivan,
>>
>> I for one would prefer that the changes not really modify any files in the librte_pmd_ixgbe/ixgbe directory. Those files are derived directly from the BSD driver baseline, and any changes will make future merges of newer code more challenging. The changes should be limited to files in the librte_pmd_ixgbe directory (and ethdev).
>>
>> Regards,
>> -Venky
>>
>>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of jigsaw
>> Sent: Wednesday, September 25, 2013 6:56 AM
>> To: Ivan Boule
>> Cc: dev@dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH] Add support for 82599 Tx->Rx loopback operation.
>>
>> Hi Ivan,
>>
>> Appreciations for your comments.
>> I have one question regarding the new field, lpbk_mode, in struct rte_eth_conf.
>>
>> I was thinking how to define the values for this field, then I had a dilemma.
>>
>> 82599 has only two mode of loopback, either Tx->Rx or Rx->Tx.
>> But other controller may have different ideas. For instance, I350 can be set as either MAC, PHY or SGMII loopback mode.
>> So if we define loopback mode as a enum type in rte_ethdev.h, we then have to expose the driver level details.
>> That is, the enum rte_loopback_mode will be sth. like:
>>
>> enum rte_loopback_mode {
>> RTE_LOOPBACK_NONE,
>> RTE_LOOPBACK_82599_TX_RX,
>> RTE_LOOPBACK_I350_MAC,
>> RTE_LOOPBACK_I350_PHY,
>> /* will be more if we add support for other controllers */ };
>>
>> IMO it doesn't look so nice coz the hardware specific details are exposed in a higher level API.
>>
>> However, if we don't expose these details here, like in the patch, the value is just a integer, user of the API may get confused, and he/she still has to be aware of what are possible values for his/her eth controller.
>>
>> There may be more subtle problems. It's not clear to me whether the loopback mode of certain controller is mutually exclusive. For instance, is it possible that the Rx-Tx and Tx-Rx can be activated at the same time for 82599? If so then the lpbk_mode has to be defined as bitfield.
>>
>> Having these questions in mind, I decided to expose just a uint32_t in rte_eth_conf, so that the solution is open for further changes. I should have stated my thoughts before sending the patch, and I hope it's not too late to open the discussion at this moment.
>>
>> Looking forward to your advice.
>>
>> thx &
>> rgds,
>> -qinglai
>>
>>
>> On Wed, Sep 25, 2013 at 4:11 PM, Ivan Boule <ivan.boule@6wind.com> wrote:
>>> 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@gmail.com>
>>> ---
>>> 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [dpdk-dev] [PATCH] Add support for 82599 Tx->Rx loopback operation.
2013-09-25 16:59 ` jigsaw
2013-09-25 17:37 ` Venkatesan, Venky
@ 2013-09-26 7:27 ` Ivan Boule
1 sibling, 0 replies; 12+ messages in thread
From: Ivan Boule @ 2013-09-26 7:27 UTC (permalink / raw)
To: jigsaw; +Cc: dev
Hi Qinglai, Venky,
Thanks a lot for all these simplifications toward elegance. I really appreciate
Regards
Ivan
On 09/25/2013 06:59 PM, jigsaw wrote:
> Hi Venky,
>
> Good point. Thus we avoid not only replacing all callbacks, but also
> adding lpbk field in struct ixgbe_hw.
> All loopback-aware operations will be then restricted to function
> ixgbe_dev_start and ixgbe_dev_rxtx_start.
>
> The logic of ixgbe_dev_start will be sth. like:
>
> ...
> ...
> ixgbe_dev_tx_init(dev);
> ...
> ...
> if (loopback is configured)
> goto skip_setup_link;
> ...
> ...
> err = ixgbe_setup_link(hw, speed, negotiate, link_up);
> if (err)
> goto error;
>
> skip_setup_link:
> ...
> ...
>
> Meantime, as you suggested, the FLU bit will be set in
> ixgbe_dev_rxtx_start, if loopback is configured.
>
> I can send the patch later tomorrow if all agree with this proposal.
>
> thx &
> rgds,
> -qinglai
>
>
>
>
> On Wed, Sep 25, 2013 at 6:47 PM, Venkatesan, Venky
> <venky.venkatesan@intel.com> wrote:
>> Thanks a bunch. It does work.
>>
>> One more suggestion to think about; Once you set the FLU (force link up) bit (bit 0) in the AUTOC (0x42A0) register, the auto-negotiation state will be set to AN_GOOD, the link-up indication should be set regardless. Would you still need the callbacks? You could then merge the setup_link code into something like dev_rxtx_start() [ open to suggestions ].
>>
>> Regards,
>> -Venky
>>
>> -----Original Message-----
>> From: jigsaw [mailto:jigsaw@gmail.com]
>> Sent: Wednesday, September 25, 2013 7:39 AM
>> To: Venkatesan, Venky
>> Cc: Ivan Boule; dev@dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH] Add support for 82599 Tx->Rx loopback operation.
>>
>> Hi Venky,
>>
>> Thanks for you comments.
>>
>>>> I for one would prefer that the changes not really modify any files in
>>>> the librte_pmd_ixgbe/ixgbe directory
>> With this restriction it is gonna be little bit difficult but still feasible.
>>
>> The solution would be after calling ixgbe_init_shared_code in ixgbe_ethdev.c, if the config is for loopback operation, we immediately replace the callbacks of:
>>
>> mac->ops.get_link_capabilities
>> mac->ops.check_link
>> mac->ops.setup_link
>>
>> And of course the implementation of these callbacks can be in a new src file under librte_pmd_ixgbe, thus keep the baseline untouched.
>>
>> Sounds like a plan?
>>
>> thx &
>> rgds,
>> -qinglai
>>
>>
>> On Wed, Sep 25, 2013 at 5:12 PM, Venkatesan, Venky <venky.venkatesan@intel.com> wrote:
>>> Qinglai/Ivan,
>>>
>>> I for one would prefer that the changes not really modify any files in the librte_pmd_ixgbe/ixgbe directory. Those files are derived directly from the BSD driver baseline, and any changes will make future merges of newer code more challenging. The changes should be limited to files in the librte_pmd_ixgbe directory (and ethdev).
>>>
>>> Regards,
>>> -Venky
>>>
>>>
>>> -----Original Message-----
>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of jigsaw
>>> Sent: Wednesday, September 25, 2013 6:56 AM
>>> To: Ivan Boule
>>> Cc: dev@dpdk.org
>>> Subject: Re: [dpdk-dev] [PATCH] Add support for 82599 Tx->Rx loopback operation.
>>>
>>> Hi Ivan,
>>>
>>> Appreciations for your comments.
>>> I have one question regarding the new field, lpbk_mode, in struct rte_eth_conf.
>>>
>>> I was thinking how to define the values for this field, then I had a dilemma.
>>>
>>> 82599 has only two mode of loopback, either Tx->Rx or Rx->Tx.
>>> But other controller may have different ideas. For instance, I350 can be set as either MAC, PHY or SGMII loopback mode.
>>> So if we define loopback mode as a enum type in rte_ethdev.h, we then have to expose the driver level details.
>>> That is, the enum rte_loopback_mode will be sth. like:
>>>
>>> enum rte_loopback_mode {
>>> RTE_LOOPBACK_NONE,
>>> RTE_LOOPBACK_82599_TX_RX,
>>> RTE_LOOPBACK_I350_MAC,
>>> RTE_LOOPBACK_I350_PHY,
>>> /* will be more if we add support for other controllers */ };
>>>
>>> IMO it doesn't look so nice coz the hardware specific details are exposed in a higher level API.
>>>
>>> However, if we don't expose these details here, like in the patch, the value is just a integer, user of the API may get confused, and he/she still has to be aware of what are possible values for his/her eth controller.
>>>
>>> There may be more subtle problems. It's not clear to me whether the loopback mode of certain controller is mutually exclusive. For instance, is it possible that the Rx-Tx and Tx-Rx can be activated at the same time for 82599? If so then the lpbk_mode has to be defined as bitfield.
>>>
>>> Having these questions in mind, I decided to expose just a uint32_t in rte_eth_conf, so that the solution is open for further changes. I should have stated my thoughts before sending the patch, and I hope it's not too late to open the discussion at this moment.
>>>
>>> Looking forward to your advice.
>>>
>>> thx &
>>> rgds,
>>> -qinglai
>>>
>>>
>>> On Wed, Sep 25, 2013 at 4:11 PM, Ivan Boule <ivan.boule@6wind.com> wrote:
>>>> 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@gmail.com>
>>>> ---
>>>> 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
--
Ivan Boule
6WIND Development Engineer
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2013-09-26 7:26 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-09-23 19:16 [dpdk-dev] [PATCH] Add support for 82599 Tx->Rx loopback operation Qinglai Xiao
2013-09-25 13:11 ` Ivan Boule
2013-09-25 13:56 ` jigsaw
2013-09-25 14:12 ` Venkatesan, Venky
2013-09-25 14:38 ` jigsaw
2013-09-25 15:47 ` Venkatesan, Venky
2013-09-25 16:59 ` jigsaw
2013-09-25 17:37 ` Venkatesan, Venky
2013-09-26 7:27 ` Ivan Boule
2013-09-25 14:47 ` Thomas Monjalon
2013-09-25 14:56 ` jigsaw
2013-09-25 15:04 ` Ivan Boule
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).