Conversation
✅ Deploy Preview for aya-rs-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c7f031b83a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
This PR refactors the netlink module to improve safety, performance, and maintainability through four key improvements: using checked accessors for buffer operations, using constants more consistently, advancing through write buffers without manual offset tracking, and avoiding allocations by using iterators instead of vectors.
Changes:
- Introduced constants for netlink header lengths (NLMSG_HDR_LEN, NLMSG_HDR_ALIGN_LEN, NLA_HDR_LEN, NLA_HDR_ALIGN_LEN) for consistent use throughout
- Converted recv() from returning Vec to returning an iterator, avoiding allocations
- Refactored write buffer functions to return (remaining_buffer, bytes_written) tuples, enabling safe buffer advancement
- Changed netlink_find_filter_with_name to accept a socket reference and return an iterator, removing unnecessary unsafe marker
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| aya/src/sys/netlink.rs | Core refactoring: added header length constants, converted recv() to iterator, refactored buffer write functions to return remaining buffers, updated NetlinkMessage and iterator implementations to use checked accessors, made NetlinkSocket pub(crate), and updated tests |
| aya/src/programs/tc.rs | Updated qdisc_detach_program_fast to create NetlinkSocket and pass it to netlink_find_filter_with_name, iterate through results with proper error handling |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Use returned lengths where possible rather than hard coding constants.
Refactor netlink attribute writers to consume and return the remaining buffer instead of passing offsets through helper calls.
tamird
force-pushed
the
netlink-mess
branch
6 times, most recently
from
b28c2e3 to
70c2862
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 70c2862f9a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let f_name = CStr::from_bytes_with_nul(opt.data) | ||
| .map_err(NlAttrError::CStrFromBytesWithNul) | ||
| .map_err(|e| NetlinkError(NetlinkErrorInternal::NlAttrError(e)))?; |
There was a problem hiding this comment.
Avoid failing the entire RTM_GETTFILTER scan on CStr::from_bytes_with_nul errors. The dump includes non-BPF filters whose TCA_OPTIONS payload can reuse attribute id TCA_BPF_NAME with non-string data. This new map_err(...) turns those entries into hard errors, so qdisc_detach_program_fast now aborts instead of finding/detaching matching BPF filters.
Useful? React with 👍 / 👎.
This change is