Re: [2/8] lib/igt_drm_netlink: add get_error_counter support
"Koppuravuri, Ravi Kishore" <[email protected]>
| Newsgroups | org.freedesktop.lists.igt-dev |
|---|---|
| Message-ID | <[email protected]> |
Hi Soham, On 14-08-2026 19:39, Purkait, Soham wrote: > Hi Ravi, > > On 29-07-2026 17:49, Ravi Kishore Koppuravuri wrote: >> Add netlink request/response handling for DRM_RAS_CMD_GET_ERROR_COUNTER. >> >> Signed-off-by: Ravi Kishore Koppuravuri >> <[email protected]> >> --- >> lib/igt_drm_netlink.c | 179 +++++++++++++++++++++++++++++++++++++++++- >> lib/igt_drm_netlink.h | 11 +++ >> 2 files changed, 188 insertions(+), 2 deletions(-) >> >> diff --git a/lib/igt_drm_netlink.c b/lib/igt_drm_netlink.c >> index 1c07bb2db..4d543275d 100644 >> --- a/lib/igt_drm_netlink.c >> +++ b/lib/igt_drm_netlink.c >> @@ -5,6 +5,7 @@ >> #include <stdbool.h> >> #include <stdint.h> >> +#include <errno.h> >> #include <stdio.h> >> #include <stdlib.h> >> #include <string.h> >> @@ -17,6 +18,146 @@ >> #include "igt_core.h" >> #include "igt_drm_netlink.h" >> +static int ras_command_cb(struct nl_msg *msg, void *arg) >> +{ >> + struct app_context *ctx = arg; >> + struct nlmsghdr *nlh; >> + struct genlmsghdr *gnlh; >> + int ret; >> + >> + nlh = nlmsg_hdr(msg); >> + gnlh = nlmsg_data(nlh); >> + >> + switch (gnlh->cmd) { >> + case DRM_RAS_CMD_GET_ERROR_COUNTER: { >> + struct nlattr *attrs[DRM_RAS_A_ERROR_COUNTER_ATTRS_MAX + 1]; >> + >> + ret = genlmsg_parse(nlh, 0, attrs, >> + DRM_RAS_A_ERROR_COUNTER_ATTRS_MAX, NULL); >> + if (ret < 0) >> + return NL_SKIP; >> + >> + if (!attrs[DRM_RAS_A_ERROR_COUNTER_ATTRS_ERROR_VALUE]) >> + return NL_SKIP; >> + >> + ctx->error_value = >> nla_get_u32(attrs[DRM_RAS_A_ERROR_COUNTER_ATTRS_ERROR_VALUE]); >> + break; >> + } >> + default: >> + return NL_SKIP; >> + } >> + >> + return NL_OK; >> +} >> + >> +static int send_and_recv_nl_msg(struct app_context *ctx, >> + struct nl_cb *cb, >> + struct nl_msg *msg) >> +{ >> + int ret; >> + >> + ret = nl_send_auto(ctx->sock, msg); >> + nlmsg_free(msg); >> + if (ret < 0) { >> + nl_cb_put(cb); >> + return ret; >> + } >> + >> + ret = nl_recvmsgs(ctx->sock, cb); > Is it blocking ? if so, is there any timeout ? nl_recvmsgs receives messages from netlink socket and processes the message using the callbacks registered (cb). with libnl library, default mode is blocking until it receives atleast 1 netlink message or an error. >> + nl_cb_put(cb); > Why the callback is being removed on the fly ? Here callbacks are getting registered per-operation and once the message is processed using the registered callbacks, there is no use with cb object and so releasing the respective callback object immediately after processing. >> + >> + return ret; >> +} >> + >> +static int send_command(struct app_context *ctx, uint8_t cmd) >> +{ >> + struct nl_cb *cb; >> + struct nl_msg *msg; >> + void *msg_head; >> + int ret; >> + >> + msg = nlmsg_alloc(); >> + if (!msg) >> + return -ENOMEM; >> + >> + msg_head = genlmsg_put(msg, >> + NL_AUTO_PORT, >> + NL_AUTO_SEQ, >> + ctx->family_id, >> + 0, >> + NLM_F_REQUEST | NLM_F_ACK, >> + cmd, >> + DRM_RAS_FAMILY_VERSION); >> + if (!msg_head) { >> + nlmsg_free(msg); >> + return -ENOMEM; >> + } >> + >> + switch (cmd) { >> + case DRM_RAS_CMD_GET_ERROR_COUNTER: >> + ret = nla_put_u32(msg, >> + DRM_RAS_A_ERROR_COUNTER_ATTRS_NODE_ID, >> + ctx->node_id); >> + if (ret < 0) { >> + nlmsg_free(msg); >> + return ret; >> + } >> + >> + ret = nla_put_u32(msg, >> + DRM_RAS_A_ERROR_COUNTER_ATTRS_ERROR_ID, >> + ctx->error_id); >> + if (ret < 0) { >> + nlmsg_free(msg); >> + return ret; >> + } >> + break; >> + default: >> + nlmsg_free(msg); >> + return -EOPNOTSUPP; >> + } >> + >> + cb = nl_cb_alloc(NL_CB_DEFAULT); >> + if (!cb) { >> + nlmsg_free(msg); >> + return -ENOMEM; >> + } >> + >> + ret = nl_cb_set(cb, NL_CB_VALID, NL_CB_CUSTOM, ras_command_cb, >> ctx); > > The callback could have been set during initialization to avoid > setting and removing this callback on the fly. > > Thanks, > Soham It is possible to register all the callbacks as part of initialization and release the callbacks during cleanup. That enables the persistent callback object and may lead to concerns with reusing the callback object. As the callback object (using nl_cb_alloc()) is lightweight, and per-operation callback registration helps to keep the callback behavior local to the operation, I have opted this approach. Thanks, Ravi Kishore K. > >> + if (ret < 0) { >> + nl_cb_put(cb); >> + nlmsg_free(msg); >> + return ret; >> + } >> + >> + return send_and_recv_nl_msg(ctx, cb, msg); >> +} >> + >> +int init_app_context(struct app_context *ctx) >> +{ >> + if (!ctx) >> + return -EINVAL; >> + >> + ctx->sock = NULL; >> + ctx->node_id = UINT32_MAX; >> + ctx->error_id = UINT32_MAX; >> + ctx->error_value = 0; >> + ctx->family_id = -1; >> + >> + return 0; >> +} >> + >> +void cleanup_app_context(struct app_context *ctx) >> +{ >> + if (!ctx) >> + return; >> + >> + ctx->sock = NULL; >> + ctx->node_id = UINT32_MAX; >> + ctx->error_id = UINT32_MAX; >> + ctx->error_value = 0; >> + ctx->family_id = -1; >> +} >> + >> void cleanup_nl_socket(struct app_context *ctx) >> { >> if (!ctx || !ctx->sock) >> @@ -24,14 +165,20 @@ void cleanup_nl_socket(struct app_context *ctx) >> nl_close(ctx->sock); >> nl_socket_free(ctx->sock); >> - ctx->sock = NULL; >> - ctx->family_id = -1; >> + >> + cleanup_app_context(ctx); >> igt_debug("Cleaned up netlink socket.\n"); >> } >> int init_nl_socket(struct app_context *ctx) >> { >> + int ret; >> + >> + ret = init_app_context(ctx); >> + if (ret < 0) >> + return ret; >> + >> ctx->sock = nl_socket_alloc(); >> if (!ctx->sock) >> return -1; >> @@ -58,3 +205,31 @@ int init_nl_socket(struct app_context *ctx) >> DRM_RAS_FAMILY_NAME, ctx->family_id); >> return 0; >> } >> + >> +int get_error_counter(struct app_context *ctx) >> +{ >> + int ret; >> + >> + if (!ctx || !ctx->sock || ctx->family_id < 0) >> + return -EINVAL; >> + >> + if (ctx->node_id == UINT32_MAX || >> + ctx->error_id == UINT32_MAX || >> + ctx->error_id == 0) { >> + igt_warn("Invalid node_id (%u) or error_id (%u) provided. " >> + "node_id should be >= 0 and error_id should be >= 1.\n", >> + ctx->node_id, ctx->error_id); >> + return -EINVAL; >> + } >> + >> + ctx->error_value = 0; >> + >> + ret = send_command(ctx, DRM_RAS_CMD_GET_ERROR_COUNTER); >> + if (ret < 0) >> + return ret; >> + >> + igt_debug("Retrieved error counter: node_id=%u error_id=%u >> value=%u\n", >> + ctx->node_id, ctx->error_id, ctx->error_value); >> + >> + return 0; >> +} >> diff --git a/lib/igt_drm_netlink.h b/lib/igt_drm_netlink.h >> index c20d5452b..e539bc030 100644 >> --- a/lib/igt_drm_netlink.h >> +++ b/lib/igt_drm_netlink.h >> @@ -9,17 +9,28 @@ >> #include <stdbool.h> >> #include <stdint.h> >> +#include <linux/genetlink.h> >> + >> +#include <netlink/attr.h> >> +#include <netlink/handlers.h> >> +#include <netlink/msg.h> >> #include <netlink/netlink.h> >> #include <drm-uapi/drm_ras.h> >> struct app_context { >> struct nl_sock *sock; >> + uint32_t node_id; >> + uint32_t error_id; >> + uint32_t error_value; >> int family_id; >> }; >> +int init_app_context(struct app_context *ctx); >> +void cleanup_app_context(struct app_context *ctx); >> void cleanup_nl_socket(struct app_context *ctx); >> int init_nl_socket(struct app_context *ctx); >> +int get_error_counter(struct app_context *ctx); >> #endif /* IGT_DRM_NETLINK_H */