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