diff mbox series

[net-next] devlink: Add health recover notifications on devlink flows

Message ID 1579446268-26540-1-git-send-email-moshe@mellanox.com
State Accepted
Delegated to: David Miller
Headers show
Series [net-next] devlink: Add health recover notifications on devlink flows | expand

Commit Message

Moshe Shemesh Jan. 19, 2020, 3:04 p.m. UTC
Devlink health recover notifications were added only on driver direct
updates of health_state through devlink_health_reporter_state_update().
Add notifications on updates of health_state by devlink flows of report
and recover.

Fixes: 97ff3bd37fac ("devlink: add devink notification when reporter update health state")
Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
 net/core/devlink.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

David Miller Jan. 20, 2020, 9:50 a.m. UTC | #1
From: Moshe Shemesh <moshe@mellanox.com>
Date: Sun, 19 Jan 2020 17:04:28 +0200

> Devlink health recover notifications were added only on driver direct
> updates of health_state through devlink_health_reporter_state_update().
> Add notifications on updates of health_state by devlink flows of report
> and recover.
> 
> Fixes: 97ff3bd37fac ("devlink: add devink notification when reporter update health state")
> Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
> Acked-by: Jiri Pirko <jiri@mellanox.com>

I really dislike forward declarations and almost all of the time they are
unnecessary.

Could you please just rearrange the code as needed and resubmit?

Thank you.
Moshe Shemesh Jan. 20, 2020, 12:08 p.m. UTC | #2
On 1/20/2020 11:50 AM, David Miller wrote:
> From: Moshe Shemesh <moshe@mellanox.com>
> Date: Sun, 19 Jan 2020 17:04:28 +0200
>
>> Devlink health recover notifications were added only on driver direct
>> updates of health_state through devlink_health_reporter_state_update().
>> Add notifications on updates of health_state by devlink flows of report
>> and recover.
>>
>> Fixes: 97ff3bd37fac ("devlink: add devink notification when reporter update health state")
>> Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
>> Acked-by: Jiri Pirko <jiri@mellanox.com>
> I really dislike forward declarations and almost all of the time they are
> unnecessary.
>
> Could you please just rearrange the code as needed and resubmit?
Sure, I will resubmit.
> Thank you.
Saeed Mahameed Jan. 21, 2020, 8:51 p.m. UTC | #3
On Mon, 2020-01-20 at 10:50 +0100, David Miller wrote:
> From: Moshe Shemesh <moshe@mellanox.com>
> Date: Sun, 19 Jan 2020 17:04:28 +0200
> 
> > Devlink health recover notifications were added only on driver
> direct
> > updates of health_state through
> devlink_health_reporter_state_update().
> > Add notifications on updates of health_state by devlink flows of
> report
> > and recover.
> > 
> > Fixes: 97ff3bd37fac ("devlink: add devink notification when
> reporter update health state")
> > Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
> > Acked-by: Jiri Pirko <jiri@mellanox.com>
> 
> I really dislike forward declarations and almost all of the time they
> are
> unnecessary.
> 

Hi Dave, a question just for educational purposes, i agree regarding
function forward declarations, bottom-up organizing should always be
followed. 

But how about type forward declaration ? for hiding struct details in
specifc c file.. is it a bad practice ?

I use this trick a lot in mlx5, for detail hiding and module separation
.. 

> Could you please just rearrange the code as needed and resubmit?
> 
> Thank you.
diff mbox series

Patch

diff --git a/net/core/devlink.c b/net/core/devlink.c
index b41b2e3..99f2057 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -4851,6 +4851,9 @@  struct devlink_health_reporter *
 }
 EXPORT_SYMBOL_GPL(devlink_health_reporter_recovery_done);
 
+static void devlink_recover_notify(struct devlink_health_reporter *reporter,
+				   enum devlink_command cmd);
+
 static int
 devlink_health_reporter_recover(struct devlink_health_reporter *reporter,
 				void *priv_ctx, struct netlink_ext_ack *extack)
@@ -4869,6 +4872,7 @@  struct devlink_health_reporter *
 
 	devlink_health_reporter_recovery_done(reporter);
 	reporter->health_state = DEVLINK_HEALTH_REPORTER_STATE_HEALTHY;
+	devlink_recover_notify(reporter, DEVLINK_CMD_HEALTH_REPORTER_RECOVER);
 
 	return 0;
 }
@@ -4935,6 +4939,7 @@  int devlink_health_report(struct devlink_health_reporter *reporter,
 	reporter->error_count++;
 	prev_health_state = reporter->health_state;
 	reporter->health_state = DEVLINK_HEALTH_REPORTER_STATE_ERROR;
+	devlink_recover_notify(reporter, DEVLINK_CMD_HEALTH_REPORTER_RECOVER);
 
 	/* abort if the previous error wasn't recovered */
 	if (reporter->auto_recover &&