From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?UTF-8?B?6LS+5a2m5rab?= Subject: Re: [PATCH] kni: fix igb and ixgbe kni ethtool get_link op Date: Sun, 10 May 2015 14:26:09 +0800 Message-ID: <554EFA01.1000902@sinogrid.com> References: <1428074329-20469-1-git-send-email-jiaxt@sinogrid.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable Cc: "dev@dpdk.org" To: "Zhang, Helin" Return-path: Received: from mail-pd0-f179.google.com (mail-pd0-f179.google.com [209.85.192.179]) by dpdk.org (Postfix) with ESMTP id 56A077F1C for ; Sun, 10 May 2015 08:26:41 +0200 (CEST) Received: by pdbnk13 with SMTP id nk13so121217351pdb.0 for ; Sat, 09 May 2015 23:26:40 -0700 (PDT) In-Reply-To: List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Helin, No matter the cable is plugged or not, the return value of link detected=20 is yes when I run ethtool on a kni interface. But the return values of speed and duplex are correct. So I just copy=20 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@dpdk.org] On Behalf Of Shelton Chia >> Sent: Friday, April 3, 2015 11:19 PM >> To: dev@dpdk.org >> Subject: [dpdk-dev] [PATCH] kni: fix igb and ixgbe kni ethtool get_lin= k op >> >> igb and ixgbe's link detected always return yes, fix get_link func ref= er 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 co= de changes? Thanks! >> Signed-off-by: Shelton Chia >> --- >> .../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 =3D netdev_priv(netdev); >> - struct e1000_mac_info *mac =3D &adapter->hw.mac; >> + struct e1000_hw *hw =3D &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 =3D 1; >> + status =3D 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 *net= dev, >> struct ethtool_regs *regs, >> regs_buff[1128] =3D IXGBE_READ_REG(hw, IXGBE_MFLCN); } >> >> +static u32 ixgbe_get_link(struct net_device *netdev) { >> + struct ixgbe_adapter *adapter =3D netdev_priv(netdev); >> + struct ixgbe_hw *hw =3D &adapter->hw; >> + u32 link_speed =3D 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 ()', mor= e 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 =3D adapter->link_speed; >> + link_up =3D 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 =3D netdev_priv(netdev); @@ -2838,7 >> +2862,7 @@ struct ethtool_ops ixgbe_ethtool_ops =3D { >> .get_wol =3D ixgbe_get_wol, >> .set_wol =3D ixgbe_set_wol, >> .nway_reset =3D ixgbe_nway_reset, >> - .get_link =3D ethtool_op_get_link, >> + .get_link =3D ixgbe_get_link, >> .get_eeprom_len =3D ixgbe_get_eeprom_len, >> .get_eeprom =3D ixgbe_get_eeprom, >> .set_eeprom =3D ixgbe_set_eeprom, >> -- >> 2.3.5 --=20 =E8=B4=BE=E5=AD=A6=E6=B6=9B =E4=BF=A1=E8=AF=BA=E7=91=9E=E5=BE=97=E8=A5=BF=E5=AE=89=E7=A0=94=E5=8F=91=E4= =B8=AD=E5=BF=83