feat(agnocastlib): add NodeGraphInterface support by tomiy-0x62 · Pull Request #1241 · autowarefoundation/agnocast

tomiy-0x62 · 2026-04-08T07:39:30Z

Description

add NodeGraphInterface support

Updated docs/agnocast_node_interface_comparison.md.
Implemented agnocat_ioctl_get_node_names() in agnocast_kmod to support the implementation of NodeGraph::get_node_names().
Added agnocast_kunit_get_node_names to kunit_test to verify the new ioctl cmd.

How was this PR tested?

Autoware (required)
bash scripts/test/e2e_test_1to1.bash (required)
bash scripts/test/e2e_test_2to2.bash (required)
kunit tests (required when modifying the kernel module)
bash scripts/test/run_requires_kernel_module_tests.bash (required)
sample application

Notes for reviewers

Version Update Label (Required)

Please add exactly one of the following labels to this PR:

need-major-update: User API breaking changes
need-minor-update: Internal API breaking changes (heaphook/kmod/agnocastlib compatibility)
need-patch-update: Bug fixes and other changes

Important notes:

If you need need-major-update or need-minor-update, please include this in the PR title as well.
- Example: fix(foo)[needs major version update]: bar or feat(baz)[needs minor version update]: qux
After receiving approval from reviewers, add the run-build-test label. The PR can only be merged after the build tests pass.

See CONTRIBUTING.md for detailed versioning rules.

Signed-off-by: tomiy <tomiy@tomiylab.com>

- Replace dummy data with actual publisher and subscriber node names - Ensure node names are unique by removing duplicates Signed-off-by: tomiy <tomiy@tomiylab.com>

Signed-off-by: tomiy <tomiy@tomiylab.com>

Koichi98 · 2026-04-09T10:43:26Z

  // To avoid name conflicts, members of RequestT and ResponseT are given an underscore prefix.
+  /// Request type extending `ServiceT::Request` with internal metadata. Use this in
+  /// `borrow_loaned_request()` return types.
+  // AGNOCAST_PUBLIC


Copilot

Pull request overview

This PR adds initial NodeGraphInterface support to agnocastlib, centered on implementing NodeGraph::get_node_names() backed by a new kernel-module ioctl to retrieve node names.

Changes:

Introduces agnocast::node_interfaces::NodeGraph implementing parts of rclcpp::node_interfaces::NodeGraphInterface (notably get_node_names(), plus existing publisher/subscriber counting).
Adds AGNOCAST_GET_NODE_NAMES_CMD ioctl and kernel implementation to gather unique node names from publisher/subscriber registrations.
Updates docs and adds a KUnit test case for the new ioctl.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
src/agnocastlib/src/node/node_interfaces/node_graph.cpp	New NodeGraph implementation; adds `get_node_names()` calling new ioctl.
src/agnocastlib/src/node/agnocast_node.cpp	Instantiates `NodeGraph` in `agnocast::Node`.
src/agnocastlib/include/agnocast/node/node_interfaces/node_graph.hpp	Declares `NodeGraph` interface implementation.
src/agnocastlib/include/agnocast/node/agnocast_node.hpp	Exposes `get_node_graph_interface()` and stores `node_graph_`.
src/agnocastlib/include/agnocast/agnocast_ioctl.hpp	Adds user-space ioctl arg struct + command macro.
src/agnocastlib/include/agnocast/agnocast_client.hpp	Adds documentation comments for internal Request/Response extensions.
src/agnocastlib/CMakeLists.txt	Adds `node_graph.cpp` to build.
docs/agnocast_node_interface_comparison.md	Documents NodeGraphInterface support level.
agnocast_kmod/Makefile	Adds new KUnit object for get_node_names.
agnocast_kmod/agnocast.h	Adds kernel ioctl arg struct + command macro + function declaration.
agnocast_kmod/agnocast_main.c	Implements ioctl collection logic and adds ioctl dispatch case.
agnocast_kmod/agnocast_kunit/agnocast_kunit_get_node_names.h	Declares new KUnit test.
agnocast_kmod/agnocast_kunit/agnocast_kunit_get_node_names.c	Implements new KUnit test.
agnocast_kmod/agnocast_kunit_main.c	Registers the new KUnit test case.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-09T10:45:35Z

+    struct ioctl_get_node_names_args get_node_names_args;
+    if (copy_from_user(&get_node_names_args, (void __user *)arg, sizeof(get_node_names_args)))
+      return -EFAULT;
+    memset(get_node_names_args.buffer, 0, sizeof(get_node_names_args.buffer));


In the AGNOCAST_GET_NODE_NAMES_CMD handler, memset(get_node_names_args.buffer, 0, sizeof(get_node_names_args.buffer)) is unsafe/incorrect: buffer is a user pointer (must not be directly dereferenced in kernel space) and sizeof(get_node_names_args.buffer) is only the pointer size, not the user buffer length. Consider adding a buffer_size field to the ioctl args and using clear_user()/copy_to_user() to zero at most buffer_size bytes (or just rely on the kernel writer to NUL-terminate) instead of memset() on a user pointer.

Suggested change

memset(get_node_names_args.buffer, 0, sizeof(get_node_names_args.buffer));

Copilot · 2026-04-09T10:45:36Z

+    ret = agnocast_ioctl_get_node_names(&get_node_names_args);
+    if (ret == 0) {
+      if (copy_to_user(
+            (struct get_node_names_args __user *)arg, &get_node_names_args,


The copy_to_user() here casts to struct get_node_names_args __user *, but that type does not exist in this file/header set (the struct is named ioctl_get_node_names_args). This looks like a compile error and also mismatches the preceding copy_from_user() type. Cast to struct ioctl_get_node_names_args __user * (or remove the cast entirely) to keep the ioctl ABI consistent.

Suggested change

(struct get_node_names_args __user *)arg, &get_node_names_args,

(struct ioctl_get_node_names_args __user *)arg, &get_node_names_args,

Copilot · 2026-04-09T10:45:36Z

+  {
+    size_t len = strlen(entry->name) + 1;
+
+    if (copy_to_user(current_user_ptr, entry->name, len)) {
+      ret = -EFAULT;


This loop writes NUL-terminated node names to ioctl_ret->buffer via copy_to_user(), but the ioctl args provide no buffer_size. Without knowing the caller’s allocation, the kernel can write past the end of the user buffer (silent user memory corruption). Consider adding a buffer_size input field (and possibly a required_size output) and returning -ENOSPC when there isn’t enough room for the next name.

Copilot · 2026-04-09T10:45:36Z

+  struct ioctl_get_node_names_args get_node_names_args = {};
+  char buffer[1024];
+  get_node_names_args.buffer = buffer;
+  get_node_names_args.count = 0;
+  if (ioctl(agnocast::agnocast_fd, AGNOCAST_GET_NODE_NAMES_CMD, &get_node_names_args) < 0) {
+    RCLCPP_ERROR(logger, "AGNOCAST_GET_NODE_NAMES_CMD failed: %s", strerror(errno));
+    close(agnocast::agnocast_fd);
+    exit(EXIT_FAILURE);
+  }
+
+  std::vector<std::string> node_names;
+  char * current_ptr = buffer;
+
+  for (int i = 0; i < get_node_names_args.count; ++i) {
+    std::string s(current_ptr);
+    node_names.push_back(s);
+    current_ptr += s.length() + 1;
+  }
+
+  return node_names;


get_node_names() passes a fixed-size stack buffer (1024 bytes) to the ioctl, but the ioctl ABI doesn’t include a buffer length, so the kernel cannot avoid writing past this buffer if there are many/long node names. This can corrupt the stack. Consider extending ioctl_get_node_names_args with a buffer_size field and either (1) preflight to compute required size, or (2) return -ENOSPC with the required size so the caller can allocate accordingly.

Suggested change

struct ioctl_get_node_names_args get_node_names_args = {};

char buffer[1024];

get_node_names_args.buffer = buffer;

get_node_names_args.count = 0;

if (ioctl(agnocast::agnocast_fd, AGNOCAST_GET_NODE_NAMES_CMD, &get_node_names_args) < 0) {

RCLCPP_ERROR(logger, "AGNOCAST_GET_NODE_NAMES_CMD failed: %s", strerror(errno));

close(agnocast::agnocast_fd);

exit(EXIT_FAILURE);

}

std::vector<std::string> node_names;

char * current_ptr = buffer;

for (int i = 0; i < get_node_names_args.count; ++i) {

std::string s(current_ptr);

node_names.push_back(s);

current_ptr += s.length() + 1;

}

return node_names;

throw std::runtime_error(

"NodeGraph::get_node_names is temporarily disabled because the current "

"AGNOCAST_GET_NODE_NAMES_CMD ioctl ABI does not provide a buffer size, "

"so this call cannot be made memory-safe from userspace. Extend "

"ioctl_get_node_names_args with a buffer_size/required_size field and "

"update this method to use a preflight or retry-on-ENOSPC allocation strategy.");

Copilot · 2026-04-09T10:45:37Z

+
+rclcpp::Event::SharedPtr NodeGraph::get_graph_event()
+{
+  throw std::runtime_error("NodeGraph::notify_shutdown is not supported in agnocast. ");


The error message thrown by get_graph_event() says NodeGraph::notify_shutdown instead of NodeGraph::get_graph_event, which is misleading for callers debugging unsupported API usage. Update the string to match the actual method name.

Suggested change

throw std::runtime_error("NodeGraph::notify_shutdown is not supported in agnocast. ");

throw std::runtime_error("NodeGraph::get_graph_event is not supported in agnocast. ");

tomiy-0x62 added 13 commits March 12, 2026 15:53

add NodeGraph

eb47af4

Signed-off-by: tomiy <tomiy@tomiylab.com>

add NodeGraphInerface doc to agnocast_node_interface_comparison.md

1c8b8bd

Signed-off-by: tomiy <tomiy@tomiylab.com>

add get_node_graph_interface() to agnocast::Node

80c2385

Signed-off-by: tomiy <tomiy@tomiylab.com>

add agnocast_ioctl_get_node_names

0c0390c

Signed-off-by: tomiy <tomiy@tomiylab.com>

implement methods of NodeGraph

3416e74

Signed-off-by: tomiy <tomiy@tomiylab.com>

implement AGNOCAST_GET_NODE_NAMES_CMD

d6e551b

Signed-off-by: tomiy <tomiy@tomiylab.com>

Implement agnocast_ioctl_get_node_names to return actual node names

9a197f3

- Replace dummy data with actual publisher and subscriber node names - Ensure node names are unique by removing duplicates Signed-off-by: tomiy <tomiy@tomiylab.com>

add NodeGraph to docs/agnocast_node_interface_comparison.md

7b27f17

Signed-off-by: tomiy <tomiy@tomiylab.com>

Merge branch 'main' into NodeGraph

e54bcdd

Signed-off-by: tomiy <tomiy@tomiylab.com>

add kunit_test for get_node_names

505b56d

Signed-off-by: tomiy <tomiy@tomiylab.com>

Merge branch 'main' into NodeGraph

c8c9cdc

format using clang-format

35033e1

Signed-off-by: tomiy <tomiy@tomiylab.com>

remove temporary code for verification

8fa7756

Signed-off-by: tomiy <tomiy@tomiylab.com>

tomiy-0x62 requested review from Koichi98, atsushi421, kobayu858 and sykwer as code owners April 8, 2026 07:39

Koichi98 requested a review from Copilot April 9, 2026 10:39

Koichi98 assigned tomiy-0x62 Apr 9, 2026

Copilot started reviewing on behalf of Koichi98 April 9, 2026 10:40 View session

Merge branch 'main' into NodeGraph

ff3ddce

Koichi98 reviewed Apr 9, 2026

View reviewed changes

Koichi98 marked this pull request as draft April 9, 2026 10:44

Copilot AI reviewed Apr 9, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(agnocastlib): add NodeGraphInterface support#1241

feat(agnocastlib): add NodeGraphInterface support#1241
tomiy-0x62 wants to merge 14 commits intoautowarefoundation:mainfrom
tomiy-0x62:NodeGraph

tomiy-0x62 commented Apr 8, 2026

Uh oh!

Koichi98 Apr 9, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	(struct get_node_names_args __user *)arg, &get_node_names_args,
	(struct ioctl_get_node_names_args __user *)arg, &get_node_names_args,

	throw std::runtime_error("NodeGraph::notify_shutdown is not supported in agnocast. ");
	throw std::runtime_error("NodeGraph::get_graph_event is not supported in agnocast. ");

Conversation

tomiy-0x62 commented Apr 8, 2026

Description

Related links

How was this PR tested?

Notes for reviewers

Version Update Label (Required)

Uh oh!

Koichi98 Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants