Re: [PATCH 1/8] lib/igt_drm_netlink: Introduce DRM RAS Generic Netlink interface

Hi Ravi,
On 2026-07-29 at 17:49:52 +0530, Ravi Kishore Koppuravuri wrote:
> Introduce DRM RAS Generic Netlink interface as a library
> 
> Signed-off-by: Ravi Kishore Koppuravuri <[email protected]>
> ---
>  include/drm-uapi/drm_ras.h | 50 +++++++++++++++++++++++++++++++
>  lib/igt_drm_netlink.c      | 60 ++++++++++++++++++++++++++++++++++++++
>  lib/igt_drm_netlink.h      | 25 ++++++++++++++++
>  lib/meson.build            |  9 ++++++
>  4 files changed, 144 insertions(+)
>  create mode 100644 include/drm-uapi/drm_ras.h
>  create mode 100644 lib/igt_drm_netlink.c
>  create mode 100644 lib/igt_drm_netlink.h
> 
> diff --git a/include/drm-uapi/drm_ras.h b/include/drm-uapi/drm_ras.h
> new file mode 100644
> index 000000000..218a3ee86
> --- /dev/null
> +++ b/include/drm-uapi/drm_ras.h
> @@ -0,0 +1,50 @@
> +/* SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause) */
> +/* Do not edit directly, auto-generated from: */
> +/*	Documentation/netlink/specs/drm_ras.yaml */
> +/* YNL-GEN uapi header */
> +/* To regenerate run: tools/net/ynl/ynl-regen.sh */
> +
> +#ifndef _UAPI_LINUX_DRM_RAS_H
> +#define _UAPI_LINUX_DRM_RAS_H

Did you use script from kernel, documented in igt?

README.md:    $ make INSTALL_HDR_PATH=<dest-dir> headers_install

imho above define should be simpler, like one in
include/drm-uapi/xe_drm.h

Also, imho add uapi header in a separate patch, so it will be
easily visible with git log.

> +
> +#define DRM_RAS_FAMILY_NAME	"drm-ras"
> +#define DRM_RAS_FAMILY_VERSION	1
> +
> +/*
> + * Type of the node. Currently, only error-counter nodes are supported, which
> + * expose reliability counters for a hardware/software component.
> + */
> +enum drm_ras_node_type {
> +	DRM_RAS_NODE_TYPE_ERROR_COUNTER = 1,
> +};
> +
> +enum {
> +	DRM_RAS_A_NODE_ATTRS_NODE_ID = 1,
> +	DRM_RAS_A_NODE_ATTRS_DEVICE_NAME,
> +	DRM_RAS_A_NODE_ATTRS_NODE_NAME,
> +	DRM_RAS_A_NODE_ATTRS_NODE_TYPE,
> +
> +	__DRM_RAS_A_NODE_ATTRS_MAX,
> +	DRM_RAS_A_NODE_ATTRS_MAX = (__DRM_RAS_A_NODE_ATTRS_MAX - 1)
> +};
> +
> +enum {
> +	DRM_RAS_A_ERROR_COUNTER_ATTRS_NODE_ID = 1,
> +	DRM_RAS_A_ERROR_COUNTER_ATTRS_ERROR_ID,
> +	DRM_RAS_A_ERROR_COUNTER_ATTRS_ERROR_NAME,
> +	DRM_RAS_A_ERROR_COUNTER_ATTRS_ERROR_VALUE,
> +
> +	__DRM_RAS_A_ERROR_COUNTER_ATTRS_MAX,
> +	DRM_RAS_A_ERROR_COUNTER_ATTRS_MAX = (__DRM_RAS_A_ERROR_COUNTER_ATTRS_MAX - 1)
> +};
> +
> +enum {
> +	DRM_RAS_CMD_LIST_NODES = 1,
> +	DRM_RAS_CMD_GET_ERROR_COUNTER,
> +	DRM_RAS_CMD_CLEAR_ERROR_COUNTER,
> +
> +	__DRM_RAS_CMD_MAX,
> +	DRM_RAS_CMD_MAX = (__DRM_RAS_CMD_MAX - 1)
> +};
> +
> +#endif /* _UAPI_LINUX_DRM_RAS_H */
> diff --git a/lib/igt_drm_netlink.c b/lib/igt_drm_netlink.c
> new file mode 100644
> index 000000000..1c07bb2db
> --- /dev/null
> +++ b/lib/igt_drm_netlink.c
> @@ -0,0 +1,60 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2026 Intel Corporation
> + */
> +
> +#include <stdbool.h>
> +#include <stdint.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +
> +#include <netlink/errno.h>
> +#include <netlink/genl/ctrl.h>
> +#include <netlink/genl/genl.h>
> +#include <netlink/netlink.h>
> +
> +#include "igt_core.h"
> +#include "igt_drm_netlink.h"
> +

Please document all new public library functions, here and at
other lib places.

> +void cleanup_nl_socket(struct app_context *ctx)

We usally add igt_ prefix to such lib names, to avoid
confusion with other libs, so here for example:

void igt_cleanup_nl_socket(struct app_context *ctx)

Regards,
Kamil

> +{
> +	if (!ctx || !ctx->sock)
> +		return;
> +
> +	nl_close(ctx->sock);
> +	nl_socket_free(ctx->sock);
> +	ctx->sock = NULL;
> +	ctx->family_id = -1;
> +
> +	igt_debug("Cleaned up netlink socket.\n");
> +}
> +
> +int init_nl_socket(struct app_context *ctx)
> +{
> +	ctx->sock = nl_socket_alloc();
> +	if (!ctx->sock)
> +		return -1;
> +
> +	igt_debug("Socket allocation successful. Connecting to Generic Netlink...\n");
> +	if (genl_connect(ctx->sock) < 0) {
> +		cleanup_nl_socket(ctx);
> +		return -1;
> +	}
> +
> +	igt_debug("Resolving Generic Netlink family '%s'...\n", DRM_RAS_FAMILY_NAME);
> +	ctx->family_id = genl_ctrl_resolve(ctx->sock, DRM_RAS_FAMILY_NAME);
> +	if (ctx->family_id < 0) {
> +		fprintf(stderr,
> +			"Failed to resolve Generic Netlink family '%s': %s. "
> +			"This may mean the running kernel does not expose DRM RAS support.\n",
> +			DRM_RAS_FAMILY_NAME,
> +			nl_geterror(ctx->family_id));
> +		cleanup_nl_socket(ctx);
> +		return -1;
> +	}
> +
> +	igt_debug("Resolved Generic Netlink family '%s' with id %d.\n",
> +		  DRM_RAS_FAMILY_NAME, ctx->family_id);
> +	return 0;
> +}
> diff --git a/lib/igt_drm_netlink.h b/lib/igt_drm_netlink.h
> new file mode 100644
> index 000000000..c20d5452b
> --- /dev/null
> +++ b/lib/igt_drm_netlink.h
> @@ -0,0 +1,25 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2026 Intel Corporation
> + */
> +
> +#ifndef IGT_DRM_NETLINK_H
> +#define IGT_DRM_NETLINK_H
> +
> +#include <stdbool.h>
> +#include <stdint.h>
> +
> +#include <netlink/netlink.h>
> +
> +#include <drm-uapi/drm_ras.h>
> +
> +struct app_context {
> +	struct nl_sock *sock;
> +	int family_id;
> +};
> +
> +void cleanup_nl_socket(struct app_context *ctx);
> +int init_nl_socket(struct app_context *ctx);
> +
> +#endif /* IGT_DRM_NETLINK_H */
> +
> diff --git a/lib/meson.build b/lib/meson.build
> index 4af346b43..79674e032 100644
> --- a/lib/meson.build
> +++ b/lib/meson.build
> @@ -27,6 +27,7 @@ lib_sources = [
>  	'igt_device_scan.c',
>  	'igt_drm_clients.h',
>  	'igt_drm_fdinfo.c',
> +	'igt_drm_netlink.c',
>          'igt_fs.c',
>  	'igt_aux.c',
>  	'igt_dp.c',
> @@ -141,6 +142,13 @@ lib_sources = [
>  	'vendor/uwildmat/uwildmat.c',
>  ]
>  
> +libnl = declare_dependency(dependencies : [
> +	dependency('libnl-3.0', required : true),
> +	dependency('libnl-genl-3.0', required : true),
> +	dependency('libnl-cli-3.0', required : true),
> +	dependency('libnl-utils', required : false),
> +])
> +
>  lib_deps = [
>  	cairo,
>  	glib,
> @@ -148,6 +156,7 @@ lib_deps = [
>  	libdrm,
>  	libdw,
>  	libkmod,
> +	libnl,
>  	libpci,
>  	libudev,
>  	math,
> -- 
> 2.34.1
>