netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 net-next] ionic: add support for ethtool extended stat link_down_count
@ 2023-06-09  5:50 Shannon Nelson
  2023-06-09 14:45 ` Larysa Zaremba
  2023-06-12  8:40 ` patchwork-bot+netdevbpf
  0 siblings, 2 replies; 5+ messages in thread
From: Shannon Nelson @ 2023-06-09  5:50 UTC (permalink / raw)
  To: netdev, davem, kuba; +Cc: brett.creeley, drivers, Nitya Sunkad, Shannon Nelson

From: Nitya Sunkad <nitya.sunkad@amd.com>

Following the example of 'commit 9a0f830f8026 ("ethtool: linkstate:
add a statistic for PHY down events")', added support for link down
events.

Added callback ionic_get_link_ext_stats to ionic_ethtool.c to support
link_down_count, a property of netdev that gets reported exclusively
on physical link down events.

Run ethtool -I <devname> to display the device link down count.

Signed-off-by: Nitya Sunkad <nitya.sunkad@amd.com>
Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
---
v2: Report link_down_count only on PF, not on VF

 drivers/net/ethernet/pensando/ionic/ionic_ethtool.c | 10 ++++++++++
 drivers/net/ethernet/pensando/ionic/ionic_lif.c     |  1 +
 drivers/net/ethernet/pensando/ionic/ionic_lif.h     |  1 +
 3 files changed, 12 insertions(+)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
index 9b2b96fa36af..3a6b0a9bc241 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
@@ -104,6 +104,15 @@ static void ionic_get_regs(struct net_device *netdev, struct ethtool_regs *regs,
 	memcpy_fromio(p + offset, lif->ionic->idev.dev_cmd_regs->words, size);
 }
 
+static void ionic_get_link_ext_stats(struct net_device *netdev,
+				     struct ethtool_link_ext_stats *stats)
+{
+	struct ionic_lif *lif = netdev_priv(netdev);
+
+	if (lif->ionic->pdev->is_physfn)
+		stats->link_down_events = lif->link_down_count;
+}
+
 static int ionic_get_link_ksettings(struct net_device *netdev,
 				    struct ethtool_link_ksettings *ks)
 {
@@ -1074,6 +1083,7 @@ static const struct ethtool_ops ionic_ethtool_ops = {
 	.get_regs_len		= ionic_get_regs_len,
 	.get_regs		= ionic_get_regs,
 	.get_link		= ethtool_op_get_link,
+	.get_link_ext_stats	= ionic_get_link_ext_stats,
 	.get_link_ksettings	= ionic_get_link_ksettings,
 	.set_link_ksettings	= ionic_set_link_ksettings,
 	.get_coalesce		= ionic_get_coalesce,
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
index 957027e546b3..6ccc1ea91992 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
@@ -168,6 +168,7 @@ static void ionic_link_status_check(struct ionic_lif *lif)
 		}
 	} else {
 		if (netif_carrier_ok(netdev)) {
+			lif->link_down_count++;
 			netdev_info(netdev, "Link down\n");
 			netif_carrier_off(netdev);
 		}
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.h b/drivers/net/ethernet/pensando/ionic/ionic_lif.h
index c9c4c46d5a16..fd2ea670e7d8 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.h
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.h
@@ -201,6 +201,7 @@ struct ionic_lif {
 	u64 hw_features;
 	bool registered;
 	u16 lif_type;
+	unsigned int link_down_count;
 	unsigned int nmcast;
 	unsigned int nucast;
 	unsigned int nvlans;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 net-next] ionic: add support for ethtool extended stat link_down_count
  2023-06-09  5:50 [PATCH v2 net-next] ionic: add support for ethtool extended stat link_down_count Shannon Nelson
@ 2023-06-09 14:45 ` Larysa Zaremba
  2023-06-09 17:54   ` Shannon Nelson
  2023-06-12  8:40 ` patchwork-bot+netdevbpf
  1 sibling, 1 reply; 5+ messages in thread
From: Larysa Zaremba @ 2023-06-09 14:45 UTC (permalink / raw)
  To: Shannon Nelson; +Cc: netdev, davem, kuba, brett.creeley, drivers, Nitya Sunkad

On Thu, Jun 08, 2023 at 10:50:16PM -0700, Shannon Nelson wrote:
> From: Nitya Sunkad <nitya.sunkad@amd.com>
> 
> Following the example of 'commit 9a0f830f8026 ("ethtool: linkstate:
> add a statistic for PHY down events")', added support for link down
> events.
> 
> Added callback ionic_get_link_ext_stats to ionic_ethtool.c to support
> link_down_count, a property of netdev that gets reported exclusively
> on physical link down events.

Commit message hasn't changed since v1, despite the comment about usage of 
"added" vs "add".

> 
> Run ethtool -I <devname> to display the device link down count.
> 
> Signed-off-by: Nitya Sunkad <nitya.sunkad@amd.com>
> Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
> ---
> v2: Report link_down_count only on PF, not on VF
> 
>  drivers/net/ethernet/pensando/ionic/ionic_ethtool.c | 10 ++++++++++
>  drivers/net/ethernet/pensando/ionic/ionic_lif.c     |  1 +
>  drivers/net/ethernet/pensando/ionic/ionic_lif.h     |  1 +
>  3 files changed, 12 insertions(+)
> 
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
> index 9b2b96fa36af..3a6b0a9bc241 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
> @@ -104,6 +104,15 @@ static void ionic_get_regs(struct net_device *netdev, struct ethtool_regs *regs,
>  	memcpy_fromio(p + offset, lif->ionic->idev.dev_cmd_regs->words, size);
>  }
>  
> +static void ionic_get_link_ext_stats(struct net_device *netdev,
> +				     struct ethtool_link_ext_stats *stats)
> +{
> +	struct ionic_lif *lif = netdev_priv(netdev);
> +
> +	if (lif->ionic->pdev->is_physfn)

Maybe

ionic->pdev->device == PCI_DEVICE_ID_PENSANDO_IONIC_ETH_PF

from [0] would be a more reliable way to determine, whether we are dealing with 
a PF?

[0] https://lore.kernel.org/netdev/20191212003344.5571-3-snelson@pensando.io/

> +		stats->link_down_events = lif->link_down_count;
> +}
> +
>  static int ionic_get_link_ksettings(struct net_device *netdev,
>  				    struct ethtool_link_ksettings *ks)
>  {
> @@ -1074,6 +1083,7 @@ static const struct ethtool_ops ionic_ethtool_ops = {
>  	.get_regs_len		= ionic_get_regs_len,
>  	.get_regs		= ionic_get_regs,
>  	.get_link		= ethtool_op_get_link,
> +	.get_link_ext_stats	= ionic_get_link_ext_stats,
>  	.get_link_ksettings	= ionic_get_link_ksettings,
>  	.set_link_ksettings	= ionic_set_link_ksettings,
>  	.get_coalesce		= ionic_get_coalesce,
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> index 957027e546b3..6ccc1ea91992 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> @@ -168,6 +168,7 @@ static void ionic_link_status_check(struct ionic_lif *lif)
>  		}
>  	} else {
>  		if (netif_carrier_ok(netdev)) {
> +			lif->link_down_count++;
>  			netdev_info(netdev, "Link down\n");
>  			netif_carrier_off(netdev);
>  		}
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.h b/drivers/net/ethernet/pensando/ionic/ionic_lif.h
> index c9c4c46d5a16..fd2ea670e7d8 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_lif.h
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.h
> @@ -201,6 +201,7 @@ struct ionic_lif {
>  	u64 hw_features;
>  	bool registered;
>  	u16 lif_type;
> +	unsigned int link_down_count;
>  	unsigned int nmcast;
>  	unsigned int nucast;
>  	unsigned int nvlans;
> -- 
> 2.17.1
> 
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 net-next] ionic: add support for ethtool extended stat link_down_count
  2023-06-09 14:45 ` Larysa Zaremba
@ 2023-06-09 17:54   ` Shannon Nelson
  2023-06-12  8:24     ` Larysa Zaremba
  0 siblings, 1 reply; 5+ messages in thread
From: Shannon Nelson @ 2023-06-09 17:54 UTC (permalink / raw)
  To: Larysa Zaremba; +Cc: netdev, davem, kuba, brett.creeley, drivers, Nitya Sunkad

On 6/9/23 7:45 AM, Larysa Zaremba wrote:
> 
> On Thu, Jun 08, 2023 at 10:50:16PM -0700, Shannon Nelson wrote:
>> From: Nitya Sunkad <nitya.sunkad@amd.com>
>>
>> Following the example of 'commit 9a0f830f8026 ("ethtool: linkstate:
>> add a statistic for PHY down events")', added support for link down
>> events.
>>
>> Added callback ionic_get_link_ext_stats to ionic_ethtool.c to support
>> link_down_count, a property of netdev that gets reported exclusively
>> on physical link down events.
> 
> Commit message hasn't changed since v1, despite the comment about usage of
> "added" vs "add".

Sorry, missed that.  Not sure this is worth another spin by itself.

> 
>>
>> Run ethtool -I <devname> to display the device link down count.
>>
>> Signed-off-by: Nitya Sunkad <nitya.sunkad@amd.com>
>> Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
>> ---
>> v2: Report link_down_count only on PF, not on VF
>>
>>   drivers/net/ethernet/pensando/ionic/ionic_ethtool.c | 10 ++++++++++
>>   drivers/net/ethernet/pensando/ionic/ionic_lif.c     |  1 +
>>   drivers/net/ethernet/pensando/ionic/ionic_lif.h     |  1 +
>>   3 files changed, 12 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
>> index 9b2b96fa36af..3a6b0a9bc241 100644
>> --- a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
>> +++ b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
>> @@ -104,6 +104,15 @@ static void ionic_get_regs(struct net_device *netdev, struct ethtool_regs *regs,
>>        memcpy_fromio(p + offset, lif->ionic->idev.dev_cmd_regs->words, size);
>>   }
>>
>> +static void ionic_get_link_ext_stats(struct net_device *netdev,
>> +                                  struct ethtool_link_ext_stats *stats)
>> +{
>> +     struct ionic_lif *lif = netdev_priv(netdev);
>> +
>> +     if (lif->ionic->pdev->is_physfn)
> 
> Maybe
> 
> ionic->pdev->device == PCI_DEVICE_ID_PENSANDO_IONIC_ETH_PF
> 
> from [0] would be a more reliable way to determine, whether we are dealing with
> a PF?
> 
> [0] https://lore.kernel.org/netdev/20191212003344.5571-3-snelson@pensando.io/

Note that the indicated code was removed from later versions of that 
patchset and never actually made it into the driver.
See commit fbb39807e9ae ('ionic: support sr-iov operations')

Also, using is_physfn will still work with no further changes if we ever 
add another PF device id.

Unless anyone else has a preference, I think this works fine as is.

sln


> 
>> +             stats->link_down_events = lif->link_down_count;
>> +}
>> +
>>   static int ionic_get_link_ksettings(struct net_device *netdev,
>>                                    struct ethtool_link_ksettings *ks)
>>   {
>> @@ -1074,6 +1083,7 @@ static const struct ethtool_ops ionic_ethtool_ops = {
>>        .get_regs_len           = ionic_get_regs_len,
>>        .get_regs               = ionic_get_regs,
>>        .get_link               = ethtool_op_get_link,
>> +     .get_link_ext_stats     = ionic_get_link_ext_stats,
>>        .get_link_ksettings     = ionic_get_link_ksettings,
>>        .set_link_ksettings     = ionic_set_link_ksettings,
>>        .get_coalesce           = ionic_get_coalesce,
>> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
>> index 957027e546b3..6ccc1ea91992 100644
>> --- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
>> +++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
>> @@ -168,6 +168,7 @@ static void ionic_link_status_check(struct ionic_lif *lif)
>>                }
>>        } else {
>>                if (netif_carrier_ok(netdev)) {
>> +                     lif->link_down_count++;
>>                        netdev_info(netdev, "Link down\n");
>>                        netif_carrier_off(netdev);
>>                }
>> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.h b/drivers/net/ethernet/pensando/ionic/ionic_lif.h
>> index c9c4c46d5a16..fd2ea670e7d8 100644
>> --- a/drivers/net/ethernet/pensando/ionic/ionic_lif.h
>> +++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.h
>> @@ -201,6 +201,7 @@ struct ionic_lif {
>>        u64 hw_features;
>>        bool registered;
>>        u16 lif_type;
>> +     unsigned int link_down_count;
>>        unsigned int nmcast;
>>        unsigned int nucast;
>>        unsigned int nvlans;
>> --
>> 2.17.1
>>
>>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 net-next] ionic: add support for ethtool extended stat link_down_count
  2023-06-09 17:54   ` Shannon Nelson
@ 2023-06-12  8:24     ` Larysa Zaremba
  0 siblings, 0 replies; 5+ messages in thread
From: Larysa Zaremba @ 2023-06-12  8:24 UTC (permalink / raw)
  To: Shannon Nelson; +Cc: netdev, davem, kuba, brett.creeley, drivers, Nitya Sunkad

On Fri, Jun 09, 2023 at 10:54:47AM -0700, Shannon Nelson wrote:
> On 6/9/23 7:45 AM, Larysa Zaremba wrote:
> > 
> > On Thu, Jun 08, 2023 at 10:50:16PM -0700, Shannon Nelson wrote:
> > > From: Nitya Sunkad <nitya.sunkad@amd.com>
> > > 
> > > Following the example of 'commit 9a0f830f8026 ("ethtool: linkstate:
> > > add a statistic for PHY down events")', added support for link down
> > > events.
> > > 
> > > Added callback ionic_get_link_ext_stats to ionic_ethtool.c to support
> > > link_down_count, a property of netdev that gets reported exclusively
> > > on physical link down events.
> > 
> > Commit message hasn't changed since v1, despite the comment about usage of
> > "added" vs "add".
> 
> Sorry, missed that.  Not sure this is worth another spin by itself.
> 
> > 
> > > 
> > > Run ethtool -I <devname> to display the device link down count.
> > > 
> > > Signed-off-by: Nitya Sunkad <nitya.sunkad@amd.com>
> > > Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
> > > ---
> > > v2: Report link_down_count only on PF, not on VF
> > > 
> > >   drivers/net/ethernet/pensando/ionic/ionic_ethtool.c | 10 ++++++++++
> > >   drivers/net/ethernet/pensando/ionic/ionic_lif.c     |  1 +
> > >   drivers/net/ethernet/pensando/ionic/ionic_lif.h     |  1 +
> > >   3 files changed, 12 insertions(+)
> > > 
> > > diff --git a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
> > > index 9b2b96fa36af..3a6b0a9bc241 100644
> > > --- a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
> > > +++ b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
> > > @@ -104,6 +104,15 @@ static void ionic_get_regs(struct net_device *netdev, struct ethtool_regs *regs,
> > >        memcpy_fromio(p + offset, lif->ionic->idev.dev_cmd_regs->words, size);
> > >   }
> > > 
> > > +static void ionic_get_link_ext_stats(struct net_device *netdev,
> > > +                                  struct ethtool_link_ext_stats *stats)
> > > +{
> > > +     struct ionic_lif *lif = netdev_priv(netdev);
> > > +
> > > +     if (lif->ionic->pdev->is_physfn)
> > 
> > Maybe
> > 
> > ionic->pdev->device == PCI_DEVICE_ID_PENSANDO_IONIC_ETH_PF
> > 
> > from [0] would be a more reliable way to determine, whether we are dealing with
> > a PF?
> > 
> > [0] https://lore.kernel.org/netdev/20191212003344.5571-3-snelson@pensando.io/
> 
> Note that the indicated code was removed from later versions of that
> patchset and never actually made it into the driver.
> See commit fbb39807e9ae ('ionic: support sr-iov operations')
> 
> Also, using is_physfn will still work with no further changes if we ever add
> another PF device id.
> 
> Unless anyone else has a preference, I think this works fine as is.
> 
> sln
>

If you say so, I won't fight :)

Reviewed-by: Larysa Zaremba <larysa.zaremba@intel.com>

> 
> > 
> > > +             stats->link_down_events = lif->link_down_count;
> > > +}
> > > +
> > >   static int ionic_get_link_ksettings(struct net_device *netdev,
> > >                                    struct ethtool_link_ksettings *ks)
> > >   {
> > > @@ -1074,6 +1083,7 @@ static const struct ethtool_ops ionic_ethtool_ops = {
> > >        .get_regs_len           = ionic_get_regs_len,
> > >        .get_regs               = ionic_get_regs,
> > >        .get_link               = ethtool_op_get_link,
> > > +     .get_link_ext_stats     = ionic_get_link_ext_stats,
> > >        .get_link_ksettings     = ionic_get_link_ksettings,
> > >        .set_link_ksettings     = ionic_set_link_ksettings,
> > >        .get_coalesce           = ionic_get_coalesce,
> > > diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> > > index 957027e546b3..6ccc1ea91992 100644
> > > --- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> > > +++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> > > @@ -168,6 +168,7 @@ static void ionic_link_status_check(struct ionic_lif *lif)
> > >                }
> > >        } else {
> > >                if (netif_carrier_ok(netdev)) {
> > > +                     lif->link_down_count++;
> > >                        netdev_info(netdev, "Link down\n");
> > >                        netif_carrier_off(netdev);
> > >                }
> > > diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.h b/drivers/net/ethernet/pensando/ionic/ionic_lif.h
> > > index c9c4c46d5a16..fd2ea670e7d8 100644
> > > --- a/drivers/net/ethernet/pensando/ionic/ionic_lif.h
> > > +++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.h
> > > @@ -201,6 +201,7 @@ struct ionic_lif {
> > >        u64 hw_features;
> > >        bool registered;
> > >        u16 lif_type;
> > > +     unsigned int link_down_count;
> > >        unsigned int nmcast;
> > >        unsigned int nucast;
> > >        unsigned int nvlans;
> > > --
> > > 2.17.1
> > > 
> > > 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 net-next] ionic: add support for ethtool extended stat link_down_count
  2023-06-09  5:50 [PATCH v2 net-next] ionic: add support for ethtool extended stat link_down_count Shannon Nelson
  2023-06-09 14:45 ` Larysa Zaremba
@ 2023-06-12  8:40 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 5+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-06-12  8:40 UTC (permalink / raw)
  To: Shannon Nelson; +Cc: netdev, davem, kuba, brett.creeley, drivers, nitya.sunkad

Hello:

This patch was applied to netdev/net-next.git (main)
by David S. Miller <davem@davemloft.net>:

On Thu, 8 Jun 2023 22:50:16 -0700 you wrote:
> From: Nitya Sunkad <nitya.sunkad@amd.com>
> 
> Following the example of 'commit 9a0f830f8026 ("ethtool: linkstate:
> add a statistic for PHY down events")', added support for link down
> events.
> 
> Added callback ionic_get_link_ext_stats to ionic_ethtool.c to support
> link_down_count, a property of netdev that gets reported exclusively
> on physical link down events.
> 
> [...]

Here is the summary with links:
  - [v2,net-next] ionic: add support for ethtool extended stat link_down_count
    https://git.kernel.org/netdev/net-next/c/132b4ebfa090

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-06-12  8:40 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-09  5:50 [PATCH v2 net-next] ionic: add support for ethtool extended stat link_down_count Shannon Nelson
2023-06-09 14:45 ` Larysa Zaremba
2023-06-09 17:54   ` Shannon Nelson
2023-06-12  8:24     ` Larysa Zaremba
2023-06-12  8:40 ` patchwork-bot+netdevbpf

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).