* [dpdk-dev] [PATCH] kni: fix igb and ixgbe kni ethtool get_link op
@ 2015-04-03 15:18 Shelton Chia
2015-04-28 10:12 ` Thomas Monjalon
2015-05-04 8:55 ` Zhang, Helin
0 siblings, 2 replies; 5+ messages in thread
From: Shelton Chia @ 2015-04-03 15:18 UTC (permalink / raw)
To: dev
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.
Signed-off-by: Shelton Chia <jiaxt@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);
- 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);
+ } 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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [dpdk-dev] [PATCH] kni: fix igb and ixgbe kni ethtool get_link op
2015-04-03 15:18 [dpdk-dev] [PATCH] kni: fix igb and ixgbe kni ethtool get_link op Shelton Chia
@ 2015-04-28 10:12 ` Thomas Monjalon
2015-05-04 8:55 ` Zhang, Helin
1 sibling, 0 replies; 5+ messages in thread
From: Thomas Monjalon @ 2015-04-28 10:12 UTC (permalink / raw)
To: Helin Zhang; +Cc: dev
Helin,
Do you have time to check this patch, please?
2015-04-03 23:18, Shelton Chia:
> 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.
>
> Signed-off-by: Shelton Chia <jiaxt@sinogrid.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH] kni: fix igb and ixgbe kni ethtool get_link op
2015-04-03 15:18 [dpdk-dev] [PATCH] kni: fix igb and ixgbe kni ethtool get_link op Shelton Chia
2015-04-28 10:12 ` Thomas Monjalon
@ 2015-05-04 8:55 ` Zhang, Helin
2015-05-10 6:26 ` 贾学涛
2015-06-15 0:56 ` Zhang, Helin
1 sibling, 2 replies; 5+ messages in thread
From: Zhang, Helin @ 2015-05-04 8:55 UTC (permalink / raw)
To: Shelton Chia, dev
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_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@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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [dpdk-dev] [PATCH] kni: fix igb and ixgbe kni ethtool get_link op
2015-05-04 8:55 ` Zhang, Helin
@ 2015-05-10 6:26 ` 贾学涛
2015-06-15 0:56 ` Zhang, Helin
1 sibling, 0 replies; 5+ messages in thread
From: 贾学涛 @ 2015-05-10 6:26 UTC (permalink / raw)
To: Zhang, Helin; +Cc: dev
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@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_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@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
--
贾学涛
信诺瑞得西安研发中心
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [dpdk-dev] [PATCH] kni: fix igb and ixgbe kni ethtool get_link op
2015-05-04 8:55 ` Zhang, Helin
2015-05-10 6:26 ` 贾学涛
@ 2015-06-15 0:56 ` Zhang, Helin
1 sibling, 0 replies; 5+ messages in thread
From: Zhang, Helin @ 2015-06-15 0:56 UTC (permalink / raw)
To: Shelton Chia, thomas.monjalon; +Cc: dev
Any response to my comments? Did I miss anything?
- Helin
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Zhang, Helin
> Sent: Monday, May 4, 2015 4:56 PM
> To: Shelton Chia; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] kni: fix igb and ixgbe kni ethtool get_link op
>
> 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_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@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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-06-15 0:56 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-03 15:18 [dpdk-dev] [PATCH] kni: fix igb and ixgbe kni ethtool get_link op Shelton Chia
2015-04-28 10:12 ` Thomas Monjalon
2015-05-04 8:55 ` Zhang, Helin
2015-05-10 6:26 ` 贾学涛
2015-06-15 0:56 ` Zhang, Helin
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).