Re: [2/8] lib/igt_drm_netlink: add get_error_counter support

["Purkait, Soham" <soham.purkait@intel.com>]

Hi Ravi,

On 19-08-2026 13:20, Koppuravuri, Ravi Kishore wrote:
> 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 
>>> <ravi.kishore.koppuravuri@intel.com>
>>> ---
>>>   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.
As this is being used by send_command, it will block the process for 
ever if no timeout is used.
Isn't it possible to set this nl_socket in non blocking mode ?
>
>>> +    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.

Imho setting the call back only once make it more clean and in this case 
you could also use conditional wait if required.

thanks,
Soham

>
> 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 */