[dpdk-dev] [PATCH] Add support for 82599 Tx->Rx loopback operation.
Ivan Boule
ivan.boule at 6wind.com
Wed Sep 25 17:04:15 CEST 2013
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 at 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 at 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
More information about the dev
mailing list