2020-01-19 15:05:54

by Moshe Shemesh

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

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 <[email protected]>
Acked-by: Jiri Pirko <[email protected]>
---
net/core/devlink.c | 5 +++++
1 file changed, 5 insertions(+)

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 &&
--
1.8.3.1

2020-01-20 09:51:59

by David Miller

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

From: Moshe Shemesh <[email protected]>
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 <[email protected]>
> Acked-by: Jiri Pirko <[email protected]>

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.

2020-01-20 12:09:51

by Moshe Shemesh

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

On 1/20/2020 11:50 AM, David Miller wrote:
> From: Moshe Shemesh <[email protected]>
> 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 <[email protected]>
>> Acked-by: Jiri Pirko <[email protected]>
> 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.

2020-01-21 20:53:08

by Saeed Mahameed

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

On Mon, 2020-01-20 at 10:50 +0100, David Miller wrote:
> From: Moshe Shemesh <[email protected]>
> 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 <[email protected]>
> > Acked-by: Jiri Pirko <[email protected]>
>
> 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.