* [PATCH net-next] ionic: add support for ethtool extended stat link_down_count
@ 2023-06-02 17:32 Shannon Nelson
2023-06-03 6:11 ` Siddharth Vadapalli
2023-06-03 6:47 ` Jakub Kicinski
0 siblings, 2 replies; 6+ messages in thread
From: Shannon Nelson @ 2023-06-02 17:32 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 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 incremented every time
the device link goes down.
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>
---
drivers/net/ethernet/pensando/ionic/ionic_ethtool.c | 9 +++++++++
drivers/net/ethernet/pensando/ionic/ionic_lif.c | 1 +
drivers/net/ethernet/pensando/ionic/ionic_lif.h | 1 +
3 files changed, 11 insertions(+)
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
index 9b2b96fa36af..4c527a06e7d9 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
@@ -104,6 +104,14 @@ 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);
+
+ 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 +1082,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] 6+ messages in thread
* Re: [PATCH net-next] ionic: add support for ethtool extended stat link_down_count
2023-06-02 17:32 [PATCH net-next] ionic: add support for ethtool extended stat link_down_count Shannon Nelson
@ 2023-06-03 6:11 ` Siddharth Vadapalli
2023-06-05 16:31 ` Shannon Nelson
2023-06-03 6:47 ` Jakub Kicinski
1 sibling, 1 reply; 6+ messages in thread
From: Siddharth Vadapalli @ 2023-06-03 6:11 UTC (permalink / raw)
To: Shannon Nelson, netdev, davem, kuba
Cc: brett.creeley, drivers, Nitya Sunkad, s-vadapalli
On 02-06-2023 23:02, Shannon Nelson wrote:
> From: Nitya Sunkad <nitya.sunkad@amd.com>
>
> Following the example of 9a0f830f8026 ("ethtool: linkstate: add a statistic
> for PHY down events"), added support for link down events.
s/added/add.
>
> Added callback ionic_get_link_ext_stats to ionic_ethtool.c to support
s/Added/Add.
> link_down_count, a property of netdev that gets incremented every time
> the device link goes down.
Please use imperative mood when writing commit messages.
Also, I think it is a good practice to Cc all the email IDs generated by
./scripts/get_maintainer.pl.
>
> 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>
Apart from my comments above, the patch looks good to me.
Reviewed-by: Siddharth Vadapalli <s-vadapalli@ti.com>
> ---
> drivers/net/ethernet/pensando/ionic/ionic_ethtool.c | 9 +++++++++
> drivers/net/ethernet/pensando/ionic/ionic_lif.c | 1 +
> drivers/net/ethernet/pensando/ionic/ionic_lif.h | 1 +
> 3 files changed, 11 insertions(+)
>
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
> index 9b2b96fa36af..4c527a06e7d9 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
> @@ -104,6 +104,14 @@ 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);
> +
> + 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 +1082,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;
--
Regards,
Siddharth.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] ionic: add support for ethtool extended stat link_down_count
2023-06-02 17:32 [PATCH net-next] ionic: add support for ethtool extended stat link_down_count Shannon Nelson
2023-06-03 6:11 ` Siddharth Vadapalli
@ 2023-06-03 6:47 ` Jakub Kicinski
2023-06-05 16:26 ` Shannon Nelson
1 sibling, 1 reply; 6+ messages in thread
From: Jakub Kicinski @ 2023-06-03 6:47 UTC (permalink / raw)
To: Shannon Nelson; +Cc: netdev, davem, brett.creeley, drivers, Nitya Sunkad
On Fri, 2 Jun 2023 10:32:52 -0700 Shannon Nelson wrote:
> Following the example of 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 incremented every time
> the device link goes down.
Hm, could you say more about motivation? The ethtool stat is supposed to
come from HW and represent PHY-level flap count. It's used primarily to
find bad cables in a datacenter. Is this also the use case for ionic?
It's unclear to me whether ionic interfaces are seeing an actual PHY or
just some virtual link state of the IPU and in the latter case it's not
really a match.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] ionic: add support for ethtool extended stat link_down_count
2023-06-03 6:47 ` Jakub Kicinski
@ 2023-06-05 16:26 ` Shannon Nelson
2023-06-05 19:05 ` Jakub Kicinski
0 siblings, 1 reply; 6+ messages in thread
From: Shannon Nelson @ 2023-06-05 16:26 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev, davem, brett.creeley, drivers, Nitya Sunkad
On 6/2/23 11:47 PM, Jakub Kicinski wrote:
> On Fri, 2 Jun 2023 10:32:52 -0700 Shannon Nelson wrote:
>> Following the example of 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 incremented every time
>> the device link goes down.
>
> Hm, could you say more about motivation? The ethtool stat is supposed to
> come from HW and represent PHY-level flap count. It's used primarily to
> find bad cables in a datacenter. Is this also the use case for ionic?
> It's unclear to me whether ionic interfaces are seeing an actual PHY or
> just some virtual link state of the IPU and in the latter case it's not
> really a match.
This is a fair question - yes, it is possible that the FW could fake it,
but normally this is a signal from the HW. However, I suppose it would
be best to only have the PF reporting this counter, and not the VFs,
which is currently the case here. I'll have Nitya modify this for PF only.
Thanks,
sln
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] ionic: add support for ethtool extended stat link_down_count
2023-06-03 6:11 ` Siddharth Vadapalli
@ 2023-06-05 16:31 ` Shannon Nelson
0 siblings, 0 replies; 6+ messages in thread
From: Shannon Nelson @ 2023-06-05 16:31 UTC (permalink / raw)
To: Siddharth Vadapalli, netdev, davem, kuba
Cc: brett.creeley, drivers, Nitya Sunkad
On 6/2/23 11:11 PM, Siddharth Vadapalli wrote:
> On 02-06-2023 23:02, Shannon Nelson wrote:
>> From: Nitya Sunkad <nitya.sunkad@amd.com>
>>
>> Following the example of 9a0f830f8026 ("ethtool: linkstate: add a statistic
>> for PHY down events"), added support for link down events.
>
> s/added/add. >
>>
>> Added callback ionic_get_link_ext_stats to ionic_ethtool.c to support
>
> s/Added/Add >
>> link_down_count, a property of netdev that gets incremented every time
>> the device link goes down.
>
> Please use imperative mood when writing commit messages.
We'll get these fixed up in a re-spin.
> Also, I think it is a good practice to Cc all the email IDs generated by
> ./scripts/get_maintainer.pl.
Thanks, I'll keep that in mind.
sln
>
>>
>> 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>
>
> Apart from my comments above, the patch looks good to me.
>
> Reviewed-by: Siddharth Vadapalli <s-vadapalli@ti.com>
>
>> ---
>> drivers/net/ethernet/pensando/ionic/ionic_ethtool.c | 9 +++++++++
>> drivers/net/ethernet/pensando/ionic/ionic_lif.c | 1 +
>> drivers/net/ethernet/pensando/ionic/ionic_lif.h | 1 +
>> 3 files changed, 11 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
>> index 9b2b96fa36af..4c527a06e7d9 100644
>> --- a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
>> +++ b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
>> @@ -104,6 +104,14 @@ 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);
>> +
>> + 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 +1082,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;
>
> --
> Regards,
> Siddharth.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] ionic: add support for ethtool extended stat link_down_count
2023-06-05 16:26 ` Shannon Nelson
@ 2023-06-05 19:05 ` Jakub Kicinski
0 siblings, 0 replies; 6+ messages in thread
From: Jakub Kicinski @ 2023-06-05 19:05 UTC (permalink / raw)
To: Shannon Nelson; +Cc: netdev, davem, brett.creeley, drivers, Nitya Sunkad
On Mon, 5 Jun 2023 09:26:25 -0700 Shannon Nelson wrote:
> > Hm, could you say more about motivation? The ethtool stat is supposed to
> > come from HW and represent PHY-level flap count. It's used primarily to
> > find bad cables in a datacenter. Is this also the use case for ionic?
> > It's unclear to me whether ionic interfaces are seeing an actual PHY or
> > just some virtual link state of the IPU and in the latter case it's not
> > really a match.
>
> This is a fair question - yes, it is possible that the FW could fake it,
> but normally this is a signal from the HW. However, I suppose it would
> be best to only have the PF reporting this counter, and not the VFs,
> which is currently the case here. I'll have Nitya modify this for PF only.
Perfect, thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-06-05 19:05 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-02 17:32 [PATCH net-next] ionic: add support for ethtool extended stat link_down_count Shannon Nelson
2023-06-03 6:11 ` Siddharth Vadapalli
2023-06-05 16:31 ` Shannon Nelson
2023-06-03 6:47 ` Jakub Kicinski
2023-06-05 16:26 ` Shannon Nelson
2023-06-05 19:05 ` Jakub Kicinski
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).