| Message ID | 20190505003207.1353-15-saeedm@mellanox.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net-next,01/15] net/mlx5: Move all devlink related functions calls to devlink.c | expand |
Sun, May 05, 2019 at 02:33:33AM CEST, saeedm@mellanox.com wrote: >From: Moshe Shemesh <moshe@mellanox.com> > >Add support of dump callback for mlx5 FW fatal reporter. >The FW fatal dump use cr-dump functionality to gather cr-space data for >debug. The cr-dump uses vsc interface which is valid even if the FW >command interface is not functional, which is the case in most FW fatal >errors. >The cr-dump is stored as a memory region snapshot to ease read by >address. > >Command example and output: >$ devlink health dump show pci/0000:82:00.0 reporter fw_fatal >devlink_region_name: cr-space snapshot_id: 1 > >$ devlink region read pci/0000:82:00.0/cr-space snapshot 1 address 983064 length 8 >00000000000f0018 e1 03 00 00 fb ae a9 3f > >Signed-off-by: Moshe Shemesh <moshe@mellanox.com> >Signed-off-by: Saeed Mahameed <saeedm@mellanox.com> >--- > .../net/ethernet/mellanox/mlx5/core/health.c | 39 +++++++++++++++++++ > 1 file changed, 39 insertions(+) > >diff --git a/drivers/net/ethernet/mellanox/mlx5/core/health.c b/drivers/net/ethernet/mellanox/mlx5/core/health.c >index e64f0e32cd67..5271c88ef64c 100644 >--- a/drivers/net/ethernet/mellanox/mlx5/core/health.c >+++ b/drivers/net/ethernet/mellanox/mlx5/core/health.c >@@ -547,9 +547,48 @@ mlx5_fw_fatal_reporter_recover(struct devlink_health_reporter *reporter, > return mlx5_health_care(dev); > } > >+static int >+mlx5_fw_fatal_reporter_dump(struct devlink_health_reporter *reporter, >+ struct devlink_fmsg *fmsg, void *priv_ctx) >+{ >+ struct mlx5_core_dev *dev = devlink_health_reporter_priv(reporter); >+ char crdump_region[20]; >+ u32 snapshot_id; >+ int err; >+ >+ if (!mlx5_core_is_pf(dev)) { >+ mlx5_core_err(dev, "Only PF is permitted run FW fatal dump\n"); >+ return -EPERM; >+ } >+ >+ err = mlx5_crdump_collect(dev, crdump_region, &snapshot_id); >+ if (err) >+ return err; >+ >+ if (priv_ctx) { >+ struct mlx5_fw_reporter_ctx *fw_reporter_ctx = priv_ctx; >+ >+ err = mlx5_fw_reporter_ctx_pairs_put(fmsg, fw_reporter_ctx); >+ if (err) >+ return err; >+ } >+ >+ err = devlink_fmsg_string_pair_put(fmsg, "devlink_region_name", >+ crdump_region); Oh come on. You cannot be serious :/ Please do proper linkage to region and snapshot in devlink core. >+ if (err) >+ return err; >+ >+ err = devlink_fmsg_u32_pair_put(fmsg, "snapshot_id", snapshot_id); >+ if (err) >+ return err; >+ >+ return 0; >+} >+ > static const struct devlink_health_reporter_ops mlx5_fw_fatal_reporter_ops = { > .name = "fw_fatal", > .recover = mlx5_fw_fatal_reporter_recover, >+ .dump = mlx5_fw_fatal_reporter_dump, > }; > > #define MLX5_REPORTER_FW_GRACEFUL_PERIOD 1200000 >-- >2.20.1 >
On 5/5/2019 6:52 PM, Jiri Pirko wrote: > Sun, May 05, 2019 at 02:33:33AM CEST, saeedm@mellanox.com wrote: >> From: Moshe Shemesh <moshe@mellanox.com> >> >> Add support of dump callback for mlx5 FW fatal reporter. >> The FW fatal dump use cr-dump functionality to gather cr-space data for >> debug. The cr-dump uses vsc interface which is valid even if the FW >> command interface is not functional, which is the case in most FW fatal >> errors. >> The cr-dump is stored as a memory region snapshot to ease read by >> address. >> >> Command example and output: >> $ devlink health dump show pci/0000:82:00.0 reporter fw_fatal >> devlink_region_name: cr-space snapshot_id: 1 >> >> $ devlink region read pci/0000:82:00.0/cr-space snapshot 1 address 983064 length 8 >> 00000000000f0018 e1 03 00 00 fb ae a9 3f >> >> Signed-off-by: Moshe Shemesh <moshe@mellanox.com> >> Signed-off-by: Saeed Mahameed <saeedm@mellanox.com> >> --- >> .../net/ethernet/mellanox/mlx5/core/health.c | 39 +++++++++++++++++++ >> 1 file changed, 39 insertions(+) >> >> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/health.c b/drivers/net/ethernet/mellanox/mlx5/core/health.c >> index e64f0e32cd67..5271c88ef64c 100644 >> --- a/drivers/net/ethernet/mellanox/mlx5/core/health.c >> +++ b/drivers/net/ethernet/mellanox/mlx5/core/health.c >> @@ -547,9 +547,48 @@ mlx5_fw_fatal_reporter_recover(struct devlink_health_reporter *reporter, >> return mlx5_health_care(dev); >> } >> >> +static int >> +mlx5_fw_fatal_reporter_dump(struct devlink_health_reporter *reporter, >> + struct devlink_fmsg *fmsg, void *priv_ctx) >> +{ >> + struct mlx5_core_dev *dev = devlink_health_reporter_priv(reporter); >> + char crdump_region[20]; >> + u32 snapshot_id; >> + int err; >> + >> + if (!mlx5_core_is_pf(dev)) { >> + mlx5_core_err(dev, "Only PF is permitted run FW fatal dump\n"); >> + return -EPERM; >> + } >> + >> + err = mlx5_crdump_collect(dev, crdump_region, &snapshot_id); >> + if (err) >> + return err; >> + >> + if (priv_ctx) { >> + struct mlx5_fw_reporter_ctx *fw_reporter_ctx = priv_ctx; >> + >> + err = mlx5_fw_reporter_ctx_pairs_put(fmsg, fw_reporter_ctx); >> + if (err) >> + return err; >> + } >> + >> + err = devlink_fmsg_string_pair_put(fmsg, "devlink_region_name", >> + crdump_region); > > Oh come on. You cannot be serious :/ Please do proper linkage to region > and snapshot in devlink core. > Not sure I understand what you mean, as I wrote in the commit message, the region snapshot added value here, is that user can read data by offset. > > >> + if (err) >> + return err; >> + >> + err = devlink_fmsg_u32_pair_put(fmsg, "snapshot_id", snapshot_id); >> + if (err) >> + return err; >> + >> + return 0; >> +} >> + >> static const struct devlink_health_reporter_ops mlx5_fw_fatal_reporter_ops = { >> .name = "fw_fatal", >> .recover = mlx5_fw_fatal_reporter_recover, >> + .dump = mlx5_fw_fatal_reporter_dump, >> }; >> >> #define MLX5_REPORTER_FW_GRACEFUL_PERIOD 1200000 >> -- >> 2.20.1 >>
Mon, May 06, 2019 at 12:54:00PM CEST, moshe@mellanox.com wrote: > > >On 5/5/2019 6:52 PM, Jiri Pirko wrote: >> Sun, May 05, 2019 at 02:33:33AM CEST, saeedm@mellanox.com wrote: >>> From: Moshe Shemesh <moshe@mellanox.com> >>> >>> Add support of dump callback for mlx5 FW fatal reporter. >>> The FW fatal dump use cr-dump functionality to gather cr-space data for >>> debug. The cr-dump uses vsc interface which is valid even if the FW >>> command interface is not functional, which is the case in most FW fatal >>> errors. >>> The cr-dump is stored as a memory region snapshot to ease read by >>> address. >>> >>> Command example and output: >>> $ devlink health dump show pci/0000:82:00.0 reporter fw_fatal >>> devlink_region_name: cr-space snapshot_id: 1 >>> >>> $ devlink region read pci/0000:82:00.0/cr-space snapshot 1 address 983064 length 8 >>> 00000000000f0018 e1 03 00 00 fb ae a9 3f >>> >>> Signed-off-by: Moshe Shemesh <moshe@mellanox.com> >>> Signed-off-by: Saeed Mahameed <saeedm@mellanox.com> >>> --- >>> .../net/ethernet/mellanox/mlx5/core/health.c | 39 +++++++++++++++++++ >>> 1 file changed, 39 insertions(+) >>> >>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/health.c b/drivers/net/ethernet/mellanox/mlx5/core/health.c >>> index e64f0e32cd67..5271c88ef64c 100644 >>> --- a/drivers/net/ethernet/mellanox/mlx5/core/health.c >>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/health.c >>> @@ -547,9 +547,48 @@ mlx5_fw_fatal_reporter_recover(struct devlink_health_reporter *reporter, >>> return mlx5_health_care(dev); >>> } >>> >>> +static int >>> +mlx5_fw_fatal_reporter_dump(struct devlink_health_reporter *reporter, >>> + struct devlink_fmsg *fmsg, void *priv_ctx) >>> +{ >>> + struct mlx5_core_dev *dev = devlink_health_reporter_priv(reporter); >>> + char crdump_region[20]; >>> + u32 snapshot_id; >>> + int err; >>> + >>> + if (!mlx5_core_is_pf(dev)) { >>> + mlx5_core_err(dev, "Only PF is permitted run FW fatal dump\n"); >>> + return -EPERM; >>> + } >>> + >>> + err = mlx5_crdump_collect(dev, crdump_region, &snapshot_id); >>> + if (err) >>> + return err; >>> + >>> + if (priv_ctx) { >>> + struct mlx5_fw_reporter_ctx *fw_reporter_ctx = priv_ctx; >>> + >>> + err = mlx5_fw_reporter_ctx_pairs_put(fmsg, fw_reporter_ctx); >>> + if (err) >>> + return err; >>> + } >>> + >>> + err = devlink_fmsg_string_pair_put(fmsg, "devlink_region_name", >>> + crdump_region); >> >> Oh come on. You cannot be serious :/ Please do proper linkage to region >> and snapshot in devlink core. >> > >Not sure I understand what you mean, as I wrote in the commit message, >the region snapshot added value here, is that user can read data by offset. If there is a region/snapshot affiliated with a reporter, it should be linked together in devlink core. Not in the driver by exposing arbitrary strings... > >> >> >>> + if (err) >>> + return err; >>> + >>> + err = devlink_fmsg_u32_pair_put(fmsg, "snapshot_id", snapshot_id); >>> + if (err) >>> + return err; >>> + >>> + return 0; >>> +} >>> + >>> static const struct devlink_health_reporter_ops mlx5_fw_fatal_reporter_ops = { >>> .name = "fw_fatal", >>> .recover = mlx5_fw_fatal_reporter_recover, >>> + .dump = mlx5_fw_fatal_reporter_dump, >>> }; >>> >>> #define MLX5_REPORTER_FW_GRACEFUL_PERIOD 1200000 >>> -- >>> 2.20.1 >>>
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/health.c b/drivers/net/ethernet/mellanox/mlx5/core/health.c index e64f0e32cd67..5271c88ef64c 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/health.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/health.c @@ -547,9 +547,48 @@ mlx5_fw_fatal_reporter_recover(struct devlink_health_reporter *reporter, return mlx5_health_care(dev); } +static int +mlx5_fw_fatal_reporter_dump(struct devlink_health_reporter *reporter, + struct devlink_fmsg *fmsg, void *priv_ctx) +{ + struct mlx5_core_dev *dev = devlink_health_reporter_priv(reporter); + char crdump_region[20]; + u32 snapshot_id; + int err; + + if (!mlx5_core_is_pf(dev)) { + mlx5_core_err(dev, "Only PF is permitted run FW fatal dump\n"); + return -EPERM; + } + + err = mlx5_crdump_collect(dev, crdump_region, &snapshot_id); + if (err) + return err; + + if (priv_ctx) { + struct mlx5_fw_reporter_ctx *fw_reporter_ctx = priv_ctx; + + err = mlx5_fw_reporter_ctx_pairs_put(fmsg, fw_reporter_ctx); + if (err) + return err; + } + + err = devlink_fmsg_string_pair_put(fmsg, "devlink_region_name", + crdump_region); + if (err) + return err; + + err = devlink_fmsg_u32_pair_put(fmsg, "snapshot_id", snapshot_id); + if (err) + return err; + + return 0; +} + static const struct devlink_health_reporter_ops mlx5_fw_fatal_reporter_ops = { .name = "fw_fatal", .recover = mlx5_fw_fatal_reporter_recover, + .dump = mlx5_fw_fatal_reporter_dump, }; #define MLX5_REPORTER_FW_GRACEFUL_PERIOD 1200000