Skip to content

Dpdk: add a uevent listener helper app - #4661

Open
mcgov (mcgov) wants to merge 1 commit into
mcgov/stack-7-dpdk-installer-rollbackfrom
mcgov/stack-8-dpdk-uevent-app
Open

Dpdk: add a uevent listener helper app#4661
mcgov (mcgov) wants to merge 1 commit into
mcgov/stack-7-dpdk-installer-rollbackfrom
mcgov/stack-8-dpdk-uevent-app

Conversation

@mcgov

Copy link
Copy Markdown
Collaborator

Part 8 of 9 of a stacked series that reworks the DPDK SRIOV hot plug tests. Stacked on #4660, review only the last commit. This adds a single new C source file and changes no Python.

Hot plug tests need to know exactly when the kernel adds or removes a VF, its uverbs node, its ib device and its netdev. DPDK's own rte_dev_event_monitor only parses pci, uio and vfio events that carry PCI_SLOT_NAME, so it drops every vmbus, net and infiniband uevent that matters on Azure.

This helper listens on NETLINK_KOBJECT_UEVENT directly and prints a tagged, single line per event that the tests can match on. It is consumed by the next PR in the stack.

Key Test Cases:
verify_dpdk_sriov_rescind_failover_send_only

Impacted LISA Features:
Sriov, NetworkInterface

Tested Azure Marketplace Images:

  • canonical 0001-com-ubuntu-server-jammy 22_04-lts latest

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR adds a small standalone C helper (azure_uevent_listener.c) under the DPDK test suite that listens directly to NETLINK_KOBJECT_UEVENT and prints one tagged line per kernel uevent relevant to Azure NIC hotplug (net, vmbus, infiniband, pci), enabling higher-level DPDK SR-IOV hotplug tests to reliably match the precise device-add/remove sequence.

Changes:

  • Add a netlink uevent listener that classifies and prints Azure NIC-related uevents with stable, parseable tags.
  • Include optional modes to print all subsystems (-a) and dump raw key/value properties (-v).
Suppressed comments (1)

lisa/microsoft/testsuites/dpdk/uevent_listener/azure_uevent_listener.c:367

  • The -h option is implemented but not documented in usage(), and the usage line doesn’t mention it. This makes CLI help incomplete.
	fprintf(stderr,
		"usage: %s [-a] [-v]\n"
		"  -a  report every subsystem, not just Azure NIC events\n"
		"  -v  dump all uevent properties for each reported event\n",
		argv0);

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

static void
print_event(const struct uevent *ev, const char *tag, bool vf)
{
struct timespec ts;
Comment on lines +20 to +22
* Build:
* gcc -O2 -Wall -Wextra -o azure-hotplug-mon azure_hotplug_mon.c
*
Comment on lines +290 to +295
cm = CMSG_FIRSTHDR(&msg);
if (cm == NULL || cm->cmsg_type != SCM_CREDENTIALS)
return 0;
cred = (struct ucred *)CMSG_DATA(cm);
if (cred->uid != 0)
return 0;
Copilot AI review requested due to automatic review settings August 17, 2026 23:09
@mcgov
mcgov (mcgov) force-pushed the mcgov/stack-8-dpdk-uevent-app branch from c867395 to 7c46c72 Compare August 17, 2026 23:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (3)

lisa/microsoft/testsuites/dpdk/uevent_listener/azure_uevent_listener.c:177

  • struct timespec ts is used in the timestamp (ts.tv_nsec) even if clock_gettime() fails, leaving ts uninitialized and causing undefined behavior in the printed output. Initialize ts so the fallback path is safe.
	struct timespec ts;

lisa/microsoft/testsuites/dpdk/uevent_listener/azure_uevent_listener.c:21

  • The build command in the header comment references a non-existent source file (azure_hotplug_mon.c). Copy/pasting this command will fail because the actual file name is azure_uevent_listener.c.
 * Build:
 *   gcc -O2 -Wall -Wextra -o azure-hotplug-mon azure_hotplug_mon.c

lisa/microsoft/testsuites/dpdk/uevent_listener/azure_uevent_listener.c:158

  • classify() returns PCI_ADD/PCI_REMOVE for non-Azure-VF PCI events. That means the program will print unrelated PCI hotplug events even when -a is not specified, contradicting the comment that non-Azure NIC cases are dropped unless -a is given.
	if (strcmp(ev->subsystem, "pci") == 0) {
		if (vf)
			return add ? "VF_PCI_ADD" : "VF_PCI_REMOVE";
		return add ? "PCI_ADD" : "PCI_REMOVE";
	}

Copilot AI review requested due to automatic review settings August 17, 2026 23:34
@mcgov
mcgov (mcgov) force-pushed the mcgov/stack-8-dpdk-uevent-app branch from 7c46c72 to 5cd523e Compare August 17, 2026 23:34
@mcgov
mcgov (mcgov) force-pushed the mcgov/stack-8-dpdk-uevent-app branch from 5cd523e to 5686a9b Compare August 17, 2026 23:37

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (3)

lisa/microsoft/testsuites/dpdk/uevent_listener/azure_uevent_listener.c:177

  • ts may be uninitialized if clock_gettime() fails, but it is still used in the timestamp print (ts.tv_nsec). Initializing it avoids undefined behavior and keeps output deterministic on failure paths.
	struct timespec ts;

lisa/microsoft/testsuites/dpdk/uevent_listener/azure_uevent_listener.c:295

  • recvmsg() ancillary data can include multiple control messages; relying on CMSG_FIRSTHDR() being SCM_CREDENTIALS may cause all events to be skipped on kernels that prepend other cmsgs (e.g. pktinfo). Iterate cmsgs to find SCM_CREDENTIALS and validate it.
	if (cm == NULL || cm->cmsg_type != SCM_CREDENTIALS)
		return 0;
	cred = (struct ucred *)CMSG_DATA(cm);
	if (cred->uid != 0)
		return 0;

lisa/microsoft/testsuites/dpdk/uevent_listener/azure_uevent_listener.c:22

  • The build command in the header comment refers to azure_hotplug_mon.c, but this file is azure_uevent_listener.c. Updating it prevents copy/paste build failures for anyone following the inline instructions.
 * Build:
 *   gcc -O2 -Wall -Wextra -o azure-hotplug-mon azure_hotplug_mon.c
 *

Copilot AI review requested due to automatic review settings August 17, 2026 23:39

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (2)

lisa/microsoft/testsuites/dpdk/uevent_listener/azure_uevent_listener.c:22

  • The build instruction references azure_hotplug_mon.c, but this PR adds azure_uevent_listener.c. As written, the example command won’t compile on a fresh checkout. Update the command to use the actual source filename (or rename the file to match the documentation).
 * Build:
 *   gcc -O2 -Wall -Wextra -o azure-hotplug-mon azure_hotplug_mon.c
 *

lisa/microsoft/testsuites/dpdk/uevent_listener/azure_uevent_listener.c:177

  • ts is used in the printf even if clock_gettime() fails, but it’s currently uninitialized. Initialize it to avoid undefined behavior in that error path.
	struct timespec ts;

Copilot AI review requested due to automatic review settings August 18, 2026 05:57
@mcgov
mcgov (mcgov) force-pushed the mcgov/stack-8-dpdk-uevent-app branch from 5686a9b to 025ec32 Compare August 18, 2026 05:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (2)

lisa/microsoft/testsuites/dpdk/uevent_listener/azure_uevent_listener.c:22

  • The build instructions reference azure_hotplug_mon.c, but this file doesn’t exist in the repo (this PR adds azure_uevent_listener.c). As written, the example build command will fail. Update the command to compile the actual source file name (or rename the file to match the documented command).
 * Build:
 *   gcc -O2 -Wall -Wextra -o azure-hotplug-mon azure_hotplug_mon.c
 *

lisa/microsoft/testsuites/dpdk/uevent_listener/azure_uevent_listener.c:181

  • ts.tv_nsec is used in the printf even if clock_gettime() fails, leaving ts uninitialized (undefined behavior and can trigger compiler warnings). Initialize ts and only print milliseconds when the timestamp was captured successfully (or default to 0).
	struct timespec ts;
	struct tm tm;
	char when[16] = "??:??:??";

	if (clock_gettime(CLOCK_REALTIME, &ts) == 0 &&

Copilot AI review requested due to automatic review settings August 18, 2026 06:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (3)

lisa/microsoft/testsuites/dpdk/uevent_listener/azure_uevent_listener.c:22

  • The build command references azure_hotplug_mon.c, but this file is named azure_uevent_listener.c in the repo, so the command as written will fail when copy/pasted. Update the filename in the build instructions.
 * Build:
 *   gcc -O2 -Wall -Wextra -o azure-hotplug-mon azure_hotplug_mon.c
 *

lisa/microsoft/testsuites/dpdk/uevent_listener/azure_uevent_listener.c:177

  • ts is used in the printf format even when clock_gettime() fails, which leaves ts.tv_nsec uninitialized and can print garbage timestamps. Initialize ts to a known value before the call.
	struct timespec ts;

lisa/microsoft/testsuites/dpdk/uevent_listener/azure_uevent_listener.c:367

  • -h is supported (and getopt accepts it), but the usage text doesn’t mention it. This makes the CLI help incomplete.
	fprintf(stderr,
		"usage: %s [-a] [-v]\n"
		"  -a  report every subsystem, not just Azure NIC events\n"
		"  -v  dump all uevent properties for each reported event\n",
		argv0);

Copilot AI review requested due to automatic review settings August 19, 2026 18:47
@mcgov
mcgov (mcgov) force-pushed the mcgov/stack-8-dpdk-uevent-app branch from a4f1150 to 5ef3530 Compare August 19, 2026 18:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (4)

lisa/microsoft/testsuites/dpdk/uevent_listener/azure_uevent_listener.c:186

  • Major: print_event() uses ts.tv_nsec in the log line even if clock_gettime() fails, leaving ts uninitialized and resulting in undefined behavior / garbage timestamps.
	struct timespec ts;
	struct tm tm;
	char when[16] = "??:??:??";

	if (clock_gettime(CLOCK_REALTIME, &ts) == 0 &&
	    localtime_r(&ts.tv_sec, &tm) != NULL)
		strftime(when, sizeof(when), "%H:%M:%S", &tm);

	printf("[%s.%03ld] %-18s subsystem=%s", when, ts.tv_nsec / 1000000,
	       tag, ev->subsystem != NULL ? ev->subsystem : "?");

lisa/microsoft/testsuites/dpdk/uevent_listener/azure_uevent_listener.c:295

  • Major: if SO_PASSCRED can't be enabled (you already warn and continue), uev_recv() will drop all messages because it requires SCM_CREDENTIALS. Also, the cmsg check should validate cmsg_level == SOL_SOCKET. Consider treating credentials as an optional extra filter: only enforce uid==0 when credentials are present.
	/* Only the kernel (port id 0, uid 0) is allowed to talk to us. */
	if (snl.nl_pid != 0)
		return 0;

	cm = CMSG_FIRSTHDR(&msg);
	if (cm == NULL || cm->cmsg_type != SCM_CREDENTIALS)
		return 0;
	cred = (struct ucred *)CMSG_DATA(cm);
	if (cred->uid != 0)
		return 0;

lisa/microsoft/testsuites/dpdk/uevent_listener/azure_uevent_listener.c:22

  • Minor: the build instructions reference azure_hotplug_mon.c, but this file is named azure_uevent_listener.c. This can confuse users trying to build the helper.
 * Build:
 *   gcc -O2 -Wall -Wextra -o azure-hotplug-mon azure_hotplug_mon.c
 *

lisa/microsoft/testsuites/dpdk/uevent_listener/azure_uevent_listener.c:367

  • Nit: -h is accepted by getopt, but it isn't shown in the usage text.
	fprintf(stderr,
		"usage: %s [-a] [-v]\n"
		"  -a  report every subsystem, not just Azure NIC events\n"
		"  -v  dump all uevent properties for each reported event\n",
		argv0);

Copilot AI review requested due to automatic review settings August 24, 2026 20:48
@mcgov
mcgov (mcgov) force-pushed the mcgov/stack-8-dpdk-uevent-app branch from 5ef3530 to f121dda Compare August 24, 2026 20:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (3)

lisa/microsoft/testsuites/dpdk/uevent_listener/azure_uevent_listener.c:22

  • Minor: The build instructions reference a different source filename (azure_hotplug_mon.c) than the actual file (azure_uevent_listener.c), which will mislead anyone trying to compile this helper.
 * Build:
 *   gcc -O2 -Wall -Wextra -o azure-hotplug-mon azure_hotplug_mon.c
 *

lisa/microsoft/testsuites/dpdk/uevent_listener/azure_uevent_listener.c:185

  • Major: print_event() uses ts.tv_nsec in the printf even if clock_gettime() fails, leaving ts uninitialized and causing undefined behavior / garbage timestamps. Initialize ts and only derive milliseconds when clock_gettime succeeds.
	if (clock_gettime(CLOCK_REALTIME, &ts) == 0 &&
	    localtime_r(&ts.tv_sec, &tm) != NULL)
		strftime(when, sizeof(when), "%H:%M:%S", &tm);

	printf("[%s.%03ld] %-18s subsystem=%s", when, ts.tv_nsec / 1000000,

lisa/microsoft/testsuites/dpdk/uevent_listener/azure_uevent_listener.c:295

  • Minor: uev_recv() assumes the first control message is SCM_CREDENTIALS without verifying cmsg_level/cmsg_len, and doesn’t search subsequent control messages. It’s more robust to iterate CMSG headers and validate the credential cmsg before trusting it.
	cm = CMSG_FIRSTHDR(&msg);
	if (cm == NULL || cm->cmsg_type != SCM_CREDENTIALS)
		return 0;
	cred = (struct ucred *)CMSG_DATA(cm);
	if (cred->uid != 0)
		return 0;

@LiliDeng
LiliDeng force-pushed the mcgov/stack-8-dpdk-uevent-app branch from f121dda to ebeb6fb Compare August 25, 2026 12:36
Copilot AI review requested due to automatic review settings August 26, 2026 02:03
@LiliDeng
LiliDeng force-pushed the mcgov/stack-8-dpdk-uevent-app branch from ebeb6fb to b607cbe Compare August 26, 2026 02:03

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

lisa/microsoft/testsuites/dpdk/uevent_listener/azure_uevent_listener.c:22

  • Minor: The build instructions reference azure_hotplug_mon.c, which doesn’t exist in this repo. Update the command to use the actual source filename so the helper can be built as documented.
 * Build:
 *   gcc -O2 -Wall -Wextra -o azure-hotplug-mon azure_hotplug_mon.c
 *

lisa/microsoft/testsuites/dpdk/uevent_listener/azure_uevent_listener.c:179

  • Major: ts (and tm) are uninitialized when clock_gettime()/localtime_r() fail, but ts.tv_nsec is still used in the printf, which is undefined behavior. Initialize the structs before use so failure prints a safe timestamp (e.g., 00:00:00.000).
	struct timespec ts;
	struct tm tm;
	char when[16] = "??:??:??";

Comment on lines +226 to +229
/* Credentials let us drop anything not sent by the kernel. */
if (setsockopt(fd, SOL_SOCKET, SO_PASSCRED, &one, sizeof(one)) < 0)
fprintf(stderr, "warning: SO_PASSCRED failed: %s\n",
strerror(errno));
@LiliDeng
LiliDeng force-pushed the mcgov/stack-8-dpdk-uevent-app branch from b607cbe to eebf97a Compare August 26, 2026 02:17

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

Suppressed comments (2)

lisa/microsoft/testsuites/dpdk/uevent_listener/azure_uevent_listener.c:22

  • The build command references azure_hotplug_mon.c, but this file is azure_uevent_listener.c. As-is, the documented build command won’t work when copied verbatim.
 * Build:
 *   gcc -O2 -Wall -Wextra -o azure-hotplug-mon azure_hotplug_mon.c
 *

lisa/microsoft/testsuites/dpdk/uevent_listener/azure_uevent_listener.c:181

  • ts is uninitialized if clock_gettime() or localtime_r() fails, but ts.tv_nsec is still used in the printf(), which is undefined behavior and can print garbage timestamps. Initialize the timespec and only compute milliseconds when the time lookup succeeds.
	struct timespec ts;
	struct tm tm;
	char when[16] = "??:??:??";

	if (clock_gettime(CLOCK_REALTIME, &ts) == 0 &&

@@ -0,0 +1,452 @@
/* SPDX-License-Identifier: BSD-3-Clause
*
* azure_hotplug_mon - watch kernel uevents for NIC hotplug/removal on Azure.
Comment on lines +364 to +367
"usage: %s [-a] [-v]\n"
" -a report every subsystem, not just Azure NIC events\n"
" -v dump all uevent properties for each reported event\n",
argv0);

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (3)

lisa/microsoft/testsuites/dpdk/uevent_listener/azure_uevent_listener.c:22

  • Major: The build instructions reference azure_hotplug_mon.c, but the added source file is azure_uevent_listener.c. This makes the comment block non-actionable for anyone trying to compile the helper from the repo checkout.
 * Build:
 *   gcc -O2 -Wall -Wextra -o azure-hotplug-mon azure_hotplug_mon.c
 *

lisa/microsoft/testsuites/dpdk/uevent_listener/azure_uevent_listener.c:186

  • Major: ts is uninitialized if clock_gettime() fails, but it is always used in the printf for milliseconds (ts.tv_nsec / 1000000). This is undefined behavior and can print garbage or crash under sanitizers.
	struct timespec ts;
	struct tm tm;
	char when[16] = "??:??:??";

	if (clock_gettime(CLOCK_REALTIME, &ts) == 0 &&
	    localtime_r(&ts.tv_sec, &tm) != NULL)
		strftime(when, sizeof(when), "%H:%M:%S", &tm);

	printf("[%s.%03ld] %-18s subsystem=%s", when, ts.tv_nsec / 1000000,
	       tag, ev->subsystem != NULL ? ev->subsystem : "?");

lisa/microsoft/testsuites/dpdk/uevent_listener/azure_uevent_listener.c:367

  • Minor: -h is implemented in getopt() handling, but the usage() output doesn’t mention it. This can confuse callers trying to discover the help flag.
	fprintf(stderr,
		"usage: %s [-a] [-v]\n"
		"  -a  report every subsystem, not just Azure NIC events\n"
		"  -v  dump all uevent properties for each reported event\n",
		argv0);

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Suppressed comments (3)

lisa/microsoft/testsuites/dpdk/uevent_listener/azure_uevent_listener.c:22

  • The build command references a non-existent source filename (azure_hotplug_mon.c). Since the file in-tree is azure_uevent_listener.c, the command as written will fail if copy-pasted.
 * Build:
 *   gcc -O2 -Wall -Wextra -o azure-hotplug-mon azure_hotplug_mon.c
 *

lisa/microsoft/testsuites/dpdk/uevent_listener/azure_uevent_listener.c:177

  • ts is used in the printf even if clock_gettime() fails, which leaves ts.tv_nsec uninitialized and results in undefined output/behavior. Initialize ts (or gate the use of tv_nsec) so printing is always defined.
	struct timespec ts;

lisa/microsoft/testsuites/dpdk/uevent_listener/azure_uevent_listener.c:292

  • The SCM_CREDENTIALS validation checks only cmsg_type. Adding a cmsg_level == SOL_SOCKET check avoids accidentally treating a different control message type as credentials if message layouts change.
	cm = CMSG_FIRSTHDR(&msg);
	if (cm == NULL || cm->cmsg_type != SCM_CREDENTIALS)
		return 0;

Comment on lines +405 to +406
printf("watching kernel uevents (ctrl-c to stop)\n");
fflush(stdout);

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (2)

lisa/microsoft/testsuites/dpdk/uevent_listener/azure_uevent_listener.c:185

  • Major: ts.tv_nsec is used in the printf even if clock_gettime() or localtime_r() fails, leaving ts potentially uninitialized and printing garbage (undefined behavior). Initialize ts/the millisecond field to a safe default before printing.
	if (clock_gettime(CLOCK_REALTIME, &ts) == 0 &&
	    localtime_r(&ts.tv_sec, &tm) != NULL)
		strftime(when, sizeof(when), "%H:%M:%S", &tm);

	printf("[%s.%03ld] %-18s subsystem=%s", when, ts.tv_nsec / 1000000,

lisa/microsoft/testsuites/dpdk/uevent_listener/azure_uevent_listener.c:22

  • Minor: The build instruction references azure_hotplug_mon.c, but this source file is azure_uevent_listener.c. As written, the command won’t work when copy-pasted.
 * Build:
 *   gcc -O2 -Wall -Wextra -o azure-hotplug-mon azure_hotplug_mon.c
 *

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (3)

lisa/microsoft/testsuites/dpdk/uevent_listener/azure_uevent_listener.c:177

  • ts is uninitialized if clock_gettime() fails, but it is still used in the timestamp (ts.tv_nsec) in the printf, which is undefined behavior. Initialize ts to zero so the fallback ??:??:?? path is still safe.
	struct timespec ts;

lisa/microsoft/testsuites/dpdk/uevent_listener/azure_uevent_listener.c:22

  • The build instructions reference azure_hotplug_mon.c, but the file added in this PR is azure_uevent_listener.c. As written, the command won’t work.
 * Build:
 *   gcc -O2 -Wall -Wextra -o azure-hotplug-mon azure_hotplug_mon.c
 *

lisa/microsoft/testsuites/dpdk/uevent_listener/azure_uevent_listener.c:367

  • -h is accepted in getopt("avh"), but it isn’t documented in the usage() output. This makes the help text incomplete.
	fprintf(stderr,
		"usage: %s [-a] [-v]\n"
		"  -a  report every subsystem, not just Azure NIC events\n"
		"  -v  dump all uevent properties for each reported event\n",
		argv0);

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (4)

lisa/microsoft/testsuites/dpdk/uevent_listener/azure_uevent_listener.c:185

  • print_event() uses ts.tv_nsec in the printf even when clock_gettime() fails, leaving ts uninitialized and producing undefined output for the milliseconds field.
	if (clock_gettime(CLOCK_REALTIME, &ts) == 0 &&
	    localtime_r(&ts.tv_sec, &tm) != NULL)
		strftime(when, sizeof(when), "%H:%M:%S", &tm);

	printf("[%s.%03ld] %-18s subsystem=%s", when, ts.tv_nsec / 1000000,

lisa/microsoft/testsuites/dpdk/uevent_listener/azure_uevent_listener.c:295

  • uev_recv() drops the event when no SCM_CREDENTIALS control message is present. That makes the listener silently stop reporting on kernels/configs where SO_PASSCRED isn't supported or credentials aren't attached, even though snl.nl_pid == 0 already filters to kernel-originated uevents.
	if (cm == NULL || cm->cmsg_type != SCM_CREDENTIALS)
		return 0;
	cred = (struct ucred *)CMSG_DATA(cm);
	if (cred->uid != 0)
		return 0;

lisa/microsoft/testsuites/dpdk/uevent_listener/azure_uevent_listener.c:22

  • The build instruction references azure_hotplug_mon.c, but this file is azure_uevent_listener.c. Copy/pasting the command as written will fail.
 * Build:
 *   gcc -O2 -Wall -Wextra -o azure-hotplug-mon azure_hotplug_mon.c
 *

lisa/microsoft/testsuites/dpdk/uevent_listener/azure_uevent_listener.c:367

  • usage() doesn't mention the supported -h flag, and the usage line omits -h, so the help output is incomplete/misleading.
	fprintf(stderr,
		"usage: %s [-a] [-v]\n"
		"  -a  report every subsystem, not just Azure NIC events\n"
		"  -v  dump all uevent properties for each reported event\n",
		argv0);

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (4)

lisa/microsoft/testsuites/dpdk/uevent_listener/azure_uevent_listener.c:21

  • The build example references azure_hotplug_mon.c, but this PR adds azure_uevent_listener.c. As written, the command will fail when copy-pasted.
 *   gcc -O2 -Wall -Wextra -o azure-hotplug-mon azure_hotplug_mon.c

lisa/microsoft/testsuites/dpdk/uevent_listener/azure_uevent_listener.c:177

  • ts.tv_nsec is used in the printf even when clock_gettime() fails, leaving ts uninitialized and producing undefined output.
	struct timespec ts;

lisa/microsoft/testsuites/dpdk/uevent_listener/azure_uevent_listener.c:364

  • usage() documents -a and -v but the program also supports -h. The help output should include -h so users can discover it.
		"usage: %s [-a] [-v]\n"

lisa/microsoft/testsuites/dpdk/uevent_listener/azure_uevent_listener.c:295

  • If SO_PASSCRED can't be enabled (you already print a warning), recvmsg() may not include an SCM_CREDENTIALS control message and this function will silently drop every uevent (returning 0). Consider treating credentials as optional and falling back to the existing snl.nl_pid == 0 kernel-sender check.
	cm = CMSG_FIRSTHDR(&msg);
	if (cm == NULL || cm->cmsg_type != SCM_CREDENTIALS)
		return 0;
	cred = (struct ucred *)CMSG_DATA(cm);
	if (cred->uid != 0)
		return 0;

Hot plug tests need to know exactly when the kernel adds or removes a
VF, its uverbs node, its ib device and its netdev, but DPDK's own
rte_dev_event_monitor only parses pci, uio and vfio events that carry
PCI_SLOT_NAME, so it drops every vmbus, net and infiniband uevent that
matters on Azure.

Add a small helper that listens on NETLINK_KOBJECT_UEVENT directly and
prints a tagged, single line per event that tests can match on.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5d5f58ad-b9df-4420-ad37-22caee78e925
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