Skip to content

ocp: Fix Parsing of Statistics Snapshot FIFO Event #2777

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 22, 2025
Merged

ocp: Fix Parsing of Statistics Snapshot FIFO Event #2777

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 71 additions & 23 deletions plugins/ocp/ocp-telemetry-decode.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1027,17 +1027,22 @@ int parse_event_fifo(unsigned int fifo_num, unsigned char *pfifo_start,
if (pevent_descriptor->debug_event_class_type == RESERVED_CLASS_TYPE)
break;

if (pevent_descriptor != NULL && pevent_descriptor->event_data_size >= 0) {
__u8 *pevent_specific_data = NULL;
__u16 event_id = 0;
char description_str[256] = "";
unsigned int data_size = 0;

if (pevent_descriptor != NULL &&
pevent_descriptor->event_data_size >= 0 &&
pevent_descriptor->debug_event_class_type !=
STATISTIC_SNAPSHOT_CLASS_TYPE) {
event_des_size = sizeof(struct nvme_ocp_telemetry_event_descriptor);
/* Data is present in the form of DWORDS,
* So multiplying with sizeof(DWORD)
*/
unsigned int data_size = pevent_descriptor->event_data_size *
data_size = pevent_descriptor->event_data_size *
SIZE_OF_DWORD;

__u8 *pevent_specific_data = NULL;
__u16 event_id = 0;
char description_str[256] = "";

if (pevent_descriptor != NULL && pevent_descriptor->event_data_size > 0)
pevent_specific_data = (__u8 *)pevent_descriptor + event_des_size;

Expand Down Expand Up @@ -1128,18 +1133,6 @@ int parse_event_fifo(unsigned int fifo_num, unsigned char *pfifo_start,
pevent_fifos_object,
fp);
break;
case STATISTIC_SNAPSHOT_CLASS_TYPE: {
struct nvme_ocp_statistic_snapshot_evt_class_format
*pStaticSnapshotEvent =
(struct nvme_ocp_statistic_snapshot_evt_class_format *)
pevent_specific_data;
struct nvme_ocp_telemetry_statistic_descriptor *pstatistic_entry =
(struct nvme_ocp_telemetry_statistic_descriptor *)
(&pStaticSnapshotEvent->statisticDescriptorData);

parse_statistic(pstatistic_entry, pevent_descriptor_obj, fp);
break;
}
case RESERVED_CLASS_TYPE:
default:
break;
Expand All @@ -1154,11 +1147,67 @@ int parse_event_fifo(unsigned int fifo_num, unsigned char *pfifo_start,
else
printf(STR_LINE2);
}
} else
} else if ((pevent_descriptor != NULL) &&
(pevent_descriptor->debug_event_class_type ==
STATISTIC_SNAPSHOT_CLASS_TYPE)) {
Copy link
Contributor

@brandon-paupore-sndk brandon-paupore-sndk Apr 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might want to check for event_data_size here, as it's checked above.

Copy link
Contributor Author

@jeff-lien-sndk jeff-lien-sndk Apr 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no event_data_size field in a statistic snapshot event class; it's reserved in that event class. Therefore, it can't be checked in this case.

Copy link
Contributor

@brandon-paupore-sndk brandon-paupore-sndk Apr 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could check stat_data_size instead, which should be the statistic snapshot equivalent. A size of 0 is allowed by the spec, so it'd be good to handle that gracefully here.

Copy link
Contributor

@brandon-paupore-sndk brandon-paupore-sndk Apr 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, this is already being checked below before parsing and after having confirmed that it is a statistic snapshot. LGTM then.

parse_ocp_telemetry_string_log(0, event_id,
pevent_descriptor->debug_event_class_type, EVENT_STRING,
description_str);

struct json_object *pevent_descriptor_obj =
((pevent_fifos_object != NULL) ? json_create_object() : NULL);

if (pevent_descriptor_obj != NULL) {
json_add_formatted_u32_str(pevent_descriptor_obj,
STR_DBG_EVENT_CLASS_TYPE,
pevent_descriptor->debug_event_class_type);
json_object_add_value_string(pevent_descriptor_obj,
STR_EVENT_STRING, description_str);
} else {
if (fp) {
fprintf(fp, "%s: 0x%x\n", STR_DBG_EVENT_CLASS_TYPE,
pevent_descriptor->debug_event_class_type);
fprintf(fp, "%s: %s\n", STR_EVENT_STRING, description_str);
} else {
printf("%s: 0x%x\n", STR_DBG_EVENT_CLASS_TYPE,
pevent_descriptor->debug_event_class_type);
printf("%s: %s\n", STR_EVENT_STRING, description_str);
}
}

struct nvme_ocp_statistic_snapshot_evt_class_format
*pStaticSnapshotEvent =
(struct nvme_ocp_statistic_snapshot_evt_class_format *)
pevent_descriptor;

event_des_size =
sizeof(struct nvme_ocp_statistic_snapshot_evt_class_format);
data_size =
(le16_to_cpu((unsigned int)pStaticSnapshotEvent->stat_data_size) *
SIZE_OF_DWORD);

if (pStaticSnapshotEvent != NULL &&
pStaticSnapshotEvent->stat_data_size > 0) {
__u8 *pstatistic_entry =
(__u8 *)pStaticSnapshotEvent +
sizeof(struct nvme_ocp_telemetry_event_descriptor);

parse_statistic(
(struct nvme_ocp_telemetry_statistic_descriptor *)
pstatistic_entry,
pevent_descriptor_obj,
fp);
}
} else {
if (fp)
fprintf(fp, "Unknown or null event class %p\n", pevent_descriptor);
else
printf("Unknown or null event class %p\n", pevent_descriptor);

break;
}

offset_to_move += (pevent_descriptor->event_data_size * SIZE_OF_DWORD +
event_des_size);
offset_to_move += (data_size + event_des_size);
}

if (pevent_fifos_object != NULL && pevent_fifo_array != NULL)
Expand Down Expand Up @@ -1243,11 +1292,10 @@ int parse_statistic(struct nvme_ocp_telemetry_statistic_descriptor *pstatistic_e
return -1;
}

if (pstatistic_entry->statistic_id == STATISTICS_RESERVED_ID)
if (le16_to_cpu(pstatistic_entry->statistic_id) == STATISTICS_RESERVED_ID)
/* End of statistics entries, return -1 to stop processing the buffer */
return -1;


unsigned int data_size = pstatistic_entry->statistic_data_size * SIZE_OF_DWORD;
__u8 *pdata = (__u8 *)pstatistic_entry +
sizeof(struct nvme_ocp_telemetry_statistic_descriptor);
Expand Down
8 changes: 7 additions & 1 deletion plugins/ocp/ocp-telemetry-decode.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1102,7 +1102,13 @@ struct __packed nvme_ocp_common_dbg_evt_class_vu_data

struct __packed nvme_ocp_statistic_snapshot_evt_class_format
{
struct nvme_ocp_telemetry_statistic_descriptor statisticDescriptorData; // Bytes 11:10
__u8 debug_event_class_type; // Byte 0
__u8 reserved1[3]; // Bytes 3:1
__le16 stat_id; // Bytes 5:4
__u8 stat_info; // Byte 6
__u8 namespace_info; // Byte 7
__le16 stat_data_size; // Bytes 9:8
__le16 nsid; // Bytes 11:10
};

struct __packed nvme_ocp_statistics_identifier_string_table
Expand Down