Skip to content
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.

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

plugins/ocp: Added OCP 2.6 telemetry support #2692

Merged

Conversation

VigneshwaranSaravana
Copy link
Contributor

Updated the Telemetry Structure and Enum to support the OCP 2.6 spec

Reviewed-by: Karthik Balan karthik.b82@samsung.com
Reviewed-by: Arunpandian J arun.j@samsung.com

@igaw
Copy link
Collaborator

igaw commented Feb 7, 2025

There is a build error and also checkpatch complains about style issues.

 FAILED: nvme.p/plugins_ocp_ocp-telemetry-decode.c.o 
/usr/bin/arm-linux-gnueabihf-gcc -Invme.p -I. -I.. -Iccan -I../ccan -Isubprojects/libnvme/src -I../subprojects/libnvme/src -fdiagnostics-color=always -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Werror -std=gnu99 -O3 -fomit-frame-pointer -D_GNU_SOURCE -include config.h -MD -MQ nvme.p/plugins_ocp_ocp-telemetry-decode.c.o -MF nvme.p/plugins_ocp_ocp-telemetry-decode.c.o.d -o nvme.p/plugins_ocp_ocp-telemetry-decode.c.o -c ../plugins/ocp/ocp-telemetry-decode.c
../plugins/ocp/ocp-telemetry-decode.c: In function ‘print_telemetry_fifo_event’:
../plugins/ocp/ocp-telemetry-decode.c:142:50: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 2 has type ‘uint64_t’ {aka ‘long long unsigned int’} [-Werror=format=]
  142 |                         printf("  AEM   : 0x%016lx\n",
      |                                             ~~~~~^
      |                                                  |
      |                                                  long unsigned int
      |                                             %016llx
  143 |                                         le64_to_cpu(aem));
      |                                         ~~~~~~~~~~~~~~~~
      |                                         |
      |                                         uint64_t {aka long long unsigned int}

@VigneshwaranSaravana VigneshwaranSaravana force-pushed the plugin/ocp_2_6_telemetry branch 3 times, most recently from b02b0be to bca0df8 Compare February 11, 2025 12:05
@igaw
Copy link
Collaborator

igaw commented Feb 11, 2025

You need to use PRIu64 and friends to get this working:

printf("Estimated offset : %"PRIu64"\n", le64_to_cpu(stats->est_offset))

@VigneshwaranSaravana VigneshwaranSaravana force-pushed the plugin/ocp_2_6_telemetry branch 4 times, most recently from 121752f to d1c150c Compare February 11, 2025 13:07
@VigneshwaranSaravana
Copy link
Contributor Author

@igaw - I have fixed the issue. Please check

@@ -126,6 +132,19 @@ void print_telemetry_fifo_event(__u8 class_type,

printf(" CSTS Reg Data : 0x%08x\n",
le32_to_cpu(csts_reg_data));
} else if (id == OOB_COMMAND) {
printf(" Cmd Op Code : 0x%02x\n", data[0]);
__u16 status = *(__u16 *)(data + 1);
Copy link
Collaborator

Choose a reason for hiding this comment

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

You need to use the same memcpy trick as above, this likely to work on platforms which enforce data alignment.

The rest looks good.

@igaw
Copy link
Collaborator

igaw commented Feb 17, 2025

I've just updated the cross build containers, thus there are a bunch of new bugs. I'm fixing them up right now.

Updated the Telemetry Structure and Enum to support the OCP 2.6 spec

Reviewed-by: Karthik Balan <karthik.b82@samsung.com>
Reviewed-by: Arunpandian J <arun.j@samsung.com>
Signed-off-by: Vigneshwaran Saravanan/Vigneshwaran Saravanan <s.vignesh@samsung.com>
@igaw igaw force-pushed the plugin/ocp_2_6_telemetry branch from 097e703 to 77b2bd3 Compare February 17, 2025 14:33
@igaw igaw merged commit 68c6ad7 into linux-nvme:master Feb 17, 2025
16 of 17 checks passed
@igaw
Copy link
Collaborator

igaw commented Feb 17, 2025

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants