From: Shahar Shitrit <shshitrit@nvidia.com>
Enable configuration of the error burst period — a time window
starting from the first error recovery, during which the reporter
allows recovery attempts for each reported error.
This feature is helpful when a single underlying issue causes
multiple errors, as it delays the start of the grace period
to allow sufficient time for recovering all related errors.
For example, if multiple TX queues time out simultaneously,
a sufficient error burst period could allow all affected TX
queues to be recovered within that window. Without this period,
only the first TX queue that reports a timeout will undergo
recovery, while the remaining TX queues will be blocked once
the grace period begins.
Configuration example:
$ devlink health set pci/0000:00:09.0 reporter tx error_burst_period 500
Configuration example with ynl:
./tools/net/ynl/pyynl/cli.py \
--spec Documentation/netlink/specs/devlink.yaml \
--do health-reporter-set --json '{
"bus-name": "auxiliary",
"dev-name": "mlx5_core.eth.0",
"port-index": 65535,
"health-reporter-name": "tx",
"health-reporter-error-burst-period": 500
}'
Signed-off-by: Shahar Shitrit <shshitrit@nvidia.com>
Reviewed-by: Carolina Jubran <cjubran@nvidia.com>
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
---
Documentation/netlink/specs/devlink.yaml | 6 ++++
.../networking/devlink/devlink-health.rst | 2 +-
include/uapi/linux/devlink.h | 2 ++
net/devlink/health.c | 30 +++++++++++++++++--
net/devlink/netlink_gen.c | 5 ++--
5 files changed, 39 insertions(+), 6 deletions(-)
diff --git a/Documentation/netlink/specs/devlink.yaml b/Documentation/netlink/specs/devlink.yaml
index bb87111d5e16..0e81640dd3b2 100644
--- a/Documentation/netlink/specs/devlink.yaml
+++ b/Documentation/netlink/specs/devlink.yaml
@@ -853,6 +853,9 @@ attribute-sets:
type: nest
multi-attr: true
nested-attributes: dl-rate-tc-bws
+ -
+ name: health-reporter-error-burst-period
+ type: u64
-
name: dl-dev-stats
subset-of: devlink
@@ -1216,6 +1219,8 @@ attribute-sets:
name: health-reporter-dump-ts-ns
-
name: health-reporter-auto-dump
+ -
+ name: health-reporter-error-burst-period
-
name: dl-attr-stats
@@ -1961,6 +1966,7 @@ operations:
- health-reporter-graceful-period
- health-reporter-auto-recover
- health-reporter-auto-dump
+ - health-reporter-error-burst-period
-
name: health-reporter-recover
diff --git a/Documentation/networking/devlink/devlink-health.rst b/Documentation/networking/devlink/devlink-health.rst
index e0b8cfed610a..2279a4370003 100644
--- a/Documentation/networking/devlink/devlink-health.rst
+++ b/Documentation/networking/devlink/devlink-health.rst
@@ -50,7 +50,7 @@ Once an error is reported, devlink health will perform the following actions:
* Auto recovery attempt is being done. Depends on:
- Auto-recovery configuration
- - Grace period vs. time passed since last recover
+ - Grace period (and error burst period) vs. time passed since last recover
Devlink formatted message
=========================
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 9fcb25a0f447..458915c22990 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -636,6 +636,8 @@ enum devlink_attr {
DEVLINK_ATTR_RATE_TC_BWS, /* nested */
+ DEVLINK_ATTR_HEALTH_REPORTER_ERR_BURST_PERIOD, /* u64 */
+
/* Add new attributes above here, update the spec in
* Documentation/netlink/specs/devlink.yaml and re-generate
* net/devlink/netlink_gen.c.
diff --git a/net/devlink/health.c b/net/devlink/health.c
index c4a028e37277..d01eb4eaf89c 100644
--- a/net/devlink/health.c
+++ b/net/devlink/health.c
@@ -113,7 +113,9 @@ __devlink_health_reporter_create(struct devlink *devlink,
{
struct devlink_health_reporter *reporter;
- if (WARN_ON(ops->default_graceful_period && !ops->recover))
+ if (WARN_ON(ops->default_error_burst_period &&
+ !ops->default_graceful_period) ||
+ WARN_ON(ops->default_graceful_period && !ops->recover))
return ERR_PTR(-EINVAL);
reporter = kzalloc(sizeof(*reporter), GFP_KERNEL);
@@ -293,6 +295,11 @@ devlink_nl_health_reporter_fill(struct sk_buff *msg,
devlink_nl_put_u64(msg, DEVLINK_ATTR_HEALTH_REPORTER_GRACEFUL_PERIOD,
reporter->graceful_period))
goto reporter_nest_cancel;
+ if (reporter->ops->recover &&
+ devlink_nl_put_u64(msg,
+ DEVLINK_ATTR_HEALTH_REPORTER_ERR_BURST_PERIOD,
+ reporter->error_burst_period))
+ goto reporter_nest_cancel;
if (reporter->ops->recover &&
nla_put_u8(msg, DEVLINK_ATTR_HEALTH_REPORTER_AUTO_RECOVER,
reporter->auto_recover))
@@ -458,16 +465,33 @@ int devlink_nl_health_reporter_set_doit(struct sk_buff *skb,
if (!reporter->ops->recover &&
(info->attrs[DEVLINK_ATTR_HEALTH_REPORTER_GRACEFUL_PERIOD] ||
- info->attrs[DEVLINK_ATTR_HEALTH_REPORTER_AUTO_RECOVER]))
+ info->attrs[DEVLINK_ATTR_HEALTH_REPORTER_AUTO_RECOVER] ||
+ info->attrs[DEVLINK_ATTR_HEALTH_REPORTER_ERR_BURST_PERIOD]))
return -EOPNOTSUPP;
if (!reporter->ops->dump &&
info->attrs[DEVLINK_ATTR_HEALTH_REPORTER_AUTO_DUMP])
return -EOPNOTSUPP;
- if (info->attrs[DEVLINK_ATTR_HEALTH_REPORTER_GRACEFUL_PERIOD])
+ if (info->attrs[DEVLINK_ATTR_HEALTH_REPORTER_GRACEFUL_PERIOD]) {
reporter->graceful_period =
nla_get_u64(info->attrs[DEVLINK_ATTR_HEALTH_REPORTER_GRACEFUL_PERIOD]);
+ if (!reporter->graceful_period)
+ reporter->error_burst_period = 0;
+ }
+
+ if (info->attrs[DEVLINK_ATTR_HEALTH_REPORTER_ERR_BURST_PERIOD]) {
+ u64 burst_period =
+ nla_get_u64(info->attrs[DEVLINK_ATTR_HEALTH_REPORTER_ERR_BURST_PERIOD]);
+
+ if (!reporter->graceful_period && burst_period) {
+ NL_SET_ERR_MSG_MOD(info->extack,
+ "Cannot set error burst period without a grace period.");
+ return -EINVAL;
+ }
+
+ reporter->error_burst_period = burst_period;
+ }
if (info->attrs[DEVLINK_ATTR_HEALTH_REPORTER_AUTO_RECOVER])
reporter->auto_recover =
diff --git a/net/devlink/netlink_gen.c b/net/devlink/netlink_gen.c
index d97c326a9045..a02da5a0002f 100644
--- a/net/devlink/netlink_gen.c
+++ b/net/devlink/netlink_gen.c
@@ -389,7 +389,7 @@ static const struct nla_policy devlink_health_reporter_get_dump_nl_policy[DEVLIN
};
/* DEVLINK_CMD_HEALTH_REPORTER_SET - do */
-static const struct nla_policy devlink_health_reporter_set_nl_policy[DEVLINK_ATTR_HEALTH_REPORTER_AUTO_DUMP + 1] = {
+static const struct nla_policy devlink_health_reporter_set_nl_policy[DEVLINK_ATTR_HEALTH_REPORTER_ERR_BURST_PERIOD + 1] = {
[DEVLINK_ATTR_BUS_NAME] = { .type = NLA_NUL_STRING, },
[DEVLINK_ATTR_DEV_NAME] = { .type = NLA_NUL_STRING, },
[DEVLINK_ATTR_PORT_INDEX] = { .type = NLA_U32, },
@@ -397,6 +397,7 @@ static const struct nla_policy devlink_health_reporter_set_nl_policy[DEVLINK_ATT
[DEVLINK_ATTR_HEALTH_REPORTER_GRACEFUL_PERIOD] = { .type = NLA_U64, },
[DEVLINK_ATTR_HEALTH_REPORTER_AUTO_RECOVER] = { .type = NLA_U8, },
[DEVLINK_ATTR_HEALTH_REPORTER_AUTO_DUMP] = { .type = NLA_U8, },
+ [DEVLINK_ATTR_HEALTH_REPORTER_ERR_BURST_PERIOD] = { .type = NLA_U64, },
};
/* DEVLINK_CMD_HEALTH_REPORTER_RECOVER - do */
@@ -1032,7 +1033,7 @@ const struct genl_split_ops devlink_nl_ops[74] = {
.doit = devlink_nl_health_reporter_set_doit,
.post_doit = devlink_nl_post_doit,
.policy = devlink_health_reporter_set_nl_policy,
- .maxattr = DEVLINK_ATTR_HEALTH_REPORTER_AUTO_DUMP,
+ .maxattr = DEVLINK_ATTR_HEALTH_REPORTER_ERR_BURST_PERIOD,
.flags = GENL_ADMIN_PERM | GENL_CMD_CAP_DO,
},
{
--
2.31.1
On Wed, 13 Aug 2025 21:55:48 +0300 Tariq Toukan wrote: > diff --git a/Documentation/netlink/specs/devlink.yaml b/Documentation/netlink/specs/devlink.yaml > index bb87111d5e16..0e81640dd3b2 100644 > --- a/Documentation/netlink/specs/devlink.yaml > +++ b/Documentation/netlink/specs/devlink.yaml > @@ -853,6 +853,9 @@ attribute-sets: > type: nest > multi-attr: true > nested-attributes: dl-rate-tc-bws > + - > + name: health-reporter-error-burst-period the "graceful-period" does not have the word "error" in it. Why is it necessary to include it in this parameter? What else would be bursting in an error reporter if not errors? > + type: u64 could you add a doc: here? > - > name: dl-dev-stats > subset-of: devlink > + DEVLINK_ATTR_HEALTH_REPORTER_ERR_BURST_PERIOD, /* u64 */ The _ERR here won't work if you used _ERROR in YAML. The naming in the YAML spec must match the C naming exactly. -- pw-bot: cr
On 15/08/2025 22:26, Jakub Kicinski wrote: > On Wed, 13 Aug 2025 21:55:48 +0300 Tariq Toukan wrote: >> diff --git a/Documentation/netlink/specs/devlink.yaml b/Documentation/netlink/specs/devlink.yaml >> index bb87111d5e16..0e81640dd3b2 100644 >> --- a/Documentation/netlink/specs/devlink.yaml >> +++ b/Documentation/netlink/specs/devlink.yaml >> @@ -853,6 +853,9 @@ attribute-sets: >> type: nest >> multi-attr: true >> nested-attributes: dl-rate-tc-bws >> + - >> + name: health-reporter-error-burst-period > > the "graceful-period" does not have the word "error" > in it. Why is it necessary to include it in this parameter? > What else would be bursting in an error reporter if not errors? I see. Would you suggest renaming it to "burst period" through the entire series? for example in devlink.h: default_error_burst_period --> default_burst_period > >> + type: u64 > > could you add a doc: here? > >> - >> name: dl-dev-stats >> subset-of: devlink > >> + DEVLINK_ATTR_HEALTH_REPORTER_ERR_BURST_PERIOD, /* u64 */ > > The _ERR here won't work if you used _ERROR in YAML. > The naming in the YAML spec must match the C naming exactly.
On Sun, 17 Aug 2025 19:08:47 +0300 Shahar Shitrit wrote: > On 15/08/2025 22:26, Jakub Kicinski wrote: > > On Wed, 13 Aug 2025 21:55:48 +0300 Tariq Toukan wrote: > >> diff --git a/Documentation/netlink/specs/devlink.yaml b/Documentation/netlink/specs/devlink.yaml > >> index bb87111d5e16..0e81640dd3b2 100644 > >> --- a/Documentation/netlink/specs/devlink.yaml > >> +++ b/Documentation/netlink/specs/devlink.yaml > >> @@ -853,6 +853,9 @@ attribute-sets: > >> type: nest > >> multi-attr: true > >> nested-attributes: dl-rate-tc-bws > >> + - > >> + name: health-reporter-error-burst-period > > > > the "graceful-period" does not have the word "error" > > in it. Why is it necessary to include it in this parameter? > > What else would be bursting in an error reporter if not errors? > > I see. Would you suggest renaming it to "burst period" through the > entire series? > for example in devlink.h: > default_error_burst_period --> default_burst_period Yes, AFAICT it won't result in any loss of clarity.
© 2016 - 2025 Red Hat, Inc.