Hi Helin, No matter the cable is plugged or not, the return value of link detected is yes when I run ethtool on a kni interface. But the return values of speed and duplex are correct. So I just copy the link detected codes of get_settings op. On 05/04/2015 04:55 PM, Zhang, Helin wrote: > Hi Chia > >> -----Original Message----- >> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Shelton Chia >> Sent: Friday, April 3, 2015 11:19 PM >> To: dev at dpdk.org >> Subject: [dpdk-dev] [PATCH] kni: fix igb and ixgbe kni ethtool get_link op >> >> igb and ixgbe's link detected always return yes, fix get_link func refer to >> get_settings, it works correctly for my i350 and 82599es nic. > Could you help to add more detailed description of why we need these code > changes? Thanks! >> Signed-off-by: Shelton Chia <jiaxt at sinogrid.com> >> --- >> .../linuxapp/kni/ethtool/igb/igb_ethtool.c | 18 ++++++--------- >> .../linuxapp/kni/ethtool/ixgbe/ixgbe_ethtool.c | 26 >> +++++++++++++++++++++- >> 2 files changed, 32 insertions(+), 12 deletions(-) >> >> diff --git a/lib/librte_eal/linuxapp/kni/ethtool/igb/igb_ethtool.c >> b/lib/librte_eal/linuxapp/kni/ethtool/igb/igb_ethtool.c >> index f3c48b2..5457f48 100644 >> --- a/lib/librte_eal/linuxapp/kni/ethtool/igb/igb_ethtool.c >> +++ b/lib/librte_eal/linuxapp/kni/ethtool/igb/igb_ethtool.c >> @@ -383,19 +383,15 @@ static int igb_set_settings(struct net_device >> *netdev, struct ethtool_cmd *ecmd) static u32 igb_get_link(struct >> net_device *netdev) { >> struct igb_adapter *adapter = netdev_priv(netdev); >> - struct e1000_mac_info *mac = &adapter->hw.mac; >> + struct e1000_hw *hw = &adapter->hw; >> + u32 status; >> >> - /* >> - * If the link is not reported up to netdev, interrupts are disabled, >> - * and so the physical link state may have changed since we last >> - * looked. Set get_link_status to make sure that the true link >> - * state is interrogated, rather than pulling a cached and possibly >> - * stale link state from the driver. >> - */ >> - if (!netif_carrier_ok(netdev)) >> - mac->get_link_status = 1; >> + status = E1000_READ_REG(hw, E1000_STATUS); > Can ' check_for_link ' be used for checking the link here? It needs to support > all possible link types. > >> - return igb_has_link(adapter); >> + if (status & E1000_STATUS_LU) >> + return 1; >> + else >> + return 0; >> } >> >> static void igb_get_pauseparam(struct net_device *netdev, diff --git >> a/lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_ethtool.c >> b/lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_ethtool.c >> index 11472bd..184b14f 100644 >> --- a/lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_ethtool.c >> +++ b/lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_ethtool.c >> @@ -801,6 +801,30 @@ static void ixgbe_get_regs(struct net_device *netdev, >> struct ethtool_regs *regs, >> regs_buff[1128] = IXGBE_READ_REG(hw, IXGBE_MFLCN); } >> >> +static u32 ixgbe_get_link(struct net_device *netdev) { >> + struct ixgbe_adapter *adapter = netdev_priv(netdev); >> + struct ixgbe_hw *hw = &adapter->hw; >> + u32 link_speed = 0; >> + bool link_up; >> + >> + if (!in_interrupt()) { >> + hw->mac.ops.check_link(hw, &link_speed, &link_up, false); > As done in kernel driver function ' ixgbe_watchdog_update_link ()', more > checks > may be needed. > > Regards, > Helin > >> + } else { >> + /* >> + * this case is a special workaround for RHEL5 bonding >> + * that calls this routine from interrupt context >> + */ >> + link_speed = adapter->link_speed; >> + link_up = adapter->link_up; >> + } >> + >> + if (link_up) >> + return 1; >> + else >> + return 0; >> +} >> + >> static int ixgbe_get_eeprom_len(struct net_device *netdev) { >> struct ixgbe_adapter *adapter = netdev_priv(netdev); @@ -2838,7 >> +2862,7 @@ struct ethtool_ops ixgbe_ethtool_ops = { >> .get_wol = ixgbe_get_wol, >> .set_wol = ixgbe_set_wol, >> .nway_reset = ixgbe_nway_reset, >> - .get_link = ethtool_op_get_link, >> + .get_link = ixgbe_get_link, >> .get_eeprom_len = ixgbe_get_eeprom_len, >> .get_eeprom = ixgbe_get_eeprom, >> .set_eeprom = ixgbe_set_eeprom, >> -- >> 2.3.5
-- ??? ??????????
