feat(net): add netlink route support #25
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds Netlink Route support to enable Linux applications to retrieve network interface and address information through the Netlink protocol. The implementation references Asterinas and DragonOS designs and is essential for adapting FastDDS and ROS 2 to Starry.
Key changes:
- Implements Netlink Route protocol with support for
ip link showandip addr showcommands - Adds comprehensive message parsing and attribute handling infrastructure
- Fixes filesystem open flag validation to conform to POSIX semantics
Reviewed changes
Copilot reviewed 31 out of 32 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| modules/axnet/src/socket.rs | Adds Netlink socket variant and address type to socket infrastructure |
| modules/axnet/src/service.rs | Exposes device iterator for Netlink route queries |
| modules/axnet/src/netlink/table.rs | Implements bind table for managing Netlink socket bindings and multicast groups |
| modules/axnet/src/netlink/route/mod.rs | Implements route transport layer with send/recv operations |
| modules/axnet/src/netlink/route/message/segment/*.rs | Defines route message segment structures for links and addresses |
| modules/axnet/src/netlink/route/message/attr/*.rs | Implements attribute parsing for link and address information |
| modules/axnet/src/netlink/route/handle.rs | Handles GetLink and GetAddr requests with device filtering |
| modules/axnet/src/netlink/receiver.rs | Implements message queue and receiver for async message handling |
| modules/axnet/src/netlink/mod.rs | Main Netlink socket implementation with bind/connect/send/recv operations |
| modules/axnet/src/netlink/message/*.rs | Core message parsing infrastructure with segments and attributes |
| modules/axnet/src/netlink/addr.rs | Netlink socket addressing with port and multicast group support |
| modules/axnet/src/lib.rs | Adds device index counter and Netlink module integration |
| modules/axnet/src/device/*.rs | Extends device trait with type, flags, index, and IP information |
| modules/axnet/Cargo.toml | Adds dependencies for Netlink feature |
| modules/axfs/src/highlevel/file.rs | Refactors file open options validation to fix POSIX compliance |
| api/axfeat/Cargo.toml | Adds netlink feature flag |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| align_up(payload_len, NLMSG_ALIGN) - payload_len | ||
| } | ||
|
|
||
| fn lecacy_padding_len() -> usize { |
There was a problem hiding this comment.
Corrected spelling of 'lecacy' to 'legacy'.
| fn lecacy_padding_len() -> usize { | |
| fn legacy_padding_len() -> usize { |
| const DEPRECATED = 0x20; | ||
| const TENTATIVE = 0x40; | ||
| const PERMANENT = 0x80; | ||
| const MANAGETEMPADDR = 0x100; | ||
| const NOPREFIXROUTE = 0x200; | ||
| const MCAUTOJOIN = 0x400; | ||
| const STABLE_PRIVACY = 0x800; |
There was a problem hiding this comment.
Inconsistent whitespace in flag definitions; use consistent spacing between flag name and value.
| const DEPRECATED = 0x20; | |
| const TENTATIVE = 0x40; | |
| const PERMANENT = 0x80; | |
| const MANAGETEMPADDR = 0x100; | |
| const NOPREFIXROUTE = 0x200; | |
| const MCAUTOJOIN = 0x400; | |
| const STABLE_PRIVACY = 0x800; | |
| const DEPRECATED = 0x20; | |
| const TENTATIVE = 0x40; | |
| const PERMANENT = 0x80; | |
| const MANAGETEMPADDR = 0x100; | |
| const NOPREFIXROUTE = 0x200; | |
| const MCAUTOJOIN = 0x400; | |
| const STABLE_PRIVACY = 0x800; |
| }, | ||
| }; | ||
|
|
||
| /// An helper function to access the route protocol bind table. |
There was a problem hiding this comment.
Corrected grammar: 'An helper' should be 'A helper'.
| /// An helper function to access the route protocol bind table. | |
| /// A helper function to access the route protocol bind table. |
| self.index | ||
| } | ||
|
|
||
| fn ipv4_addr(&self) -> Option<Ipv4Addr> { |
There was a problem hiding this comment.
Return type uses Ipv4Addr from std::net, but Device trait definition shows Option<Ipv4Address> from smoltcp. The trait definition should use the stdlib type for consistency, or this implementation should match the trait's smoltcp type.
| self.index | ||
| } | ||
|
|
||
| fn ipv4_addr(&self) -> Option<Ipv4Address> { |
There was a problem hiding this comment.
Return type uses Ipv4Address from smoltcp, which is inconsistent with loopback device's use of Ipv4Addr from std::net. Both implementations should use the same type for consistency.
| /// Reference: <https://elixir.bootlin.com/linux/v6.13/source/include/uapi/linux/netlink.h#L86>. | ||
| pub struct AckFlags: u16 { | ||
| const CAPPED = 0x100; | ||
| const ACK_TLVS = 0x100; |
There was a problem hiding this comment.
Two different flags have the same value 0x100. This appears to be a copy-paste error - these flags should have distinct values.
| const ACK_TLVS = 0x100; | |
| const ACK_TLVS = 0x200; |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 33 out of 33 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let initial_remaining = src.remaining(); | ||
| let mut reader = BufReader::new(src); | ||
| loop { | ||
| let mut segment = match RouteSegment::read_from(&mut reader)? { | ||
| ContinueRead::Parsed(seg) => seg, | ||
| ContinueRead::Skipped => continue, | ||
| ContinueRead::SkippedErr(_error_segment) => { | ||
| // TODO: Should unicast `_error_segment` as NLMSG_ERROR with original seq/pid to `port` instead of silently continuing. | ||
| continue; | ||
| } | ||
| }; | ||
| let header = segment.header_mut(); | ||
| // Set the pid to the sender's port if it's zero | ||
| if header.pid == 0 { | ||
| header.pid = port | ||
| } | ||
| self.handle_request(&segment, port); | ||
| return Ok(initial_remaining); | ||
| } |
There was a problem hiding this comment.
send() returns immediately after handling the first parsed segment and returns initial_remaining regardless of how many segments were actually consumed/handled. Netlink requests can contain multiple segments; the current logic will silently ignore trailing segments while still reporting success. Consider looping until the input buffer is fully consumed (handling each parsed segment), and return the actual number of bytes consumed.
| let initial_remaining = src.remaining(); | |
| let mut reader = BufReader::new(src); | |
| loop { | |
| let mut segment = match RouteSegment::read_from(&mut reader)? { | |
| ContinueRead::Parsed(seg) => seg, | |
| ContinueRead::Skipped => continue, | |
| ContinueRead::SkippedErr(_error_segment) => { | |
| // TODO: Should unicast `_error_segment` as NLMSG_ERROR with original seq/pid to `port` instead of silently continuing. | |
| continue; | |
| } | |
| }; | |
| let header = segment.header_mut(); | |
| // Set the pid to the sender's port if it's zero | |
| if header.pid == 0 { | |
| header.pid = port | |
| } | |
| self.handle_request(&segment, port); | |
| return Ok(initial_remaining); | |
| } | |
| let mut reader = BufReader::new(src); | |
| let mut total_consumed = 0usize; | |
| loop { | |
| let before = reader.remaining(); | |
| if before == 0 { | |
| break; | |
| } | |
| let parsed = RouteSegment::read_from(&mut reader)?; | |
| let after = reader.remaining(); | |
| total_consumed += before.saturating_sub(after); | |
| match parsed { | |
| ContinueRead::Parsed(mut segment) => { | |
| let header = segment.header_mut(); | |
| // Set the pid to the sender's port if it's zero | |
| if header.pid == 0 { | |
| header.pid = port | |
| } | |
| self.handle_request(&segment, port); | |
| } | |
| ContinueRead::Skipped => { | |
| continue; | |
| } | |
| ContinueRead::SkippedErr(_error_segment) => { | |
| // TODO: Should unicast `_error_segment` as NLMSG_ERROR with original seq/pid to `port` instead of silently continuing. | |
| continue; | |
| } | |
| } | |
| } | |
| Ok(total_consumed) |
| } | ||
| } | ||
| if self.create_new && !self.create { | ||
| return Err(VfsError::AlreadyExists); |
There was a problem hiding this comment.
create_new without create currently returns VfsError::AlreadyExists, which implies the path exists even though this is just an invalid/meaningless flag combination (POSIX treats O_EXCL as only meaningful with O_CREAT). Returning InvalidInput (or implicitly enabling create when create_new is set) would be more accurate.
| return Err(VfsError::AlreadyExists); | |
| return Err(VfsError::InvalidInput); |
| fn ipv4_addr(&self) -> Option<Ipv4Addr> { | ||
| Some(Ipv4Addr::new(127, 0, 0, 1)) | ||
| } |
There was a problem hiding this comment.
Device::ipv4_addr is defined to return smoltcp::wire::Ipv4Address (see device/mod.rs), but LoopbackDevice implements it as core::net::Ipv4Addr, which will not satisfy the trait and should fail to compile. Use the same Ipv4Address type as the trait (or change the trait to consistently use core::net::Ipv4Addr).
| use axio::BufRead; | ||
|
|
||
| use super::{Attribute, AttrHeader}; | ||
| use crate::netlink::message::ContinueRead; |
There was a problem hiding this comment.
ContinueRead is not re-exported from crate::netlink::message, so use crate::netlink::message::ContinueRead; will not resolve. Import it from crate::netlink::message::result::ContinueRead (or re-export it in message/mod.rs).
| use crate::netlink::message::ContinueRead; | |
| use crate::netlink::message::result::ContinueRead; |
| use memory_addr::align_up; | ||
| pub use noattr::NoAttr; | ||
|
|
||
| use crate::netlink::message::{ContinueRead, NLMSG_ALIGN}; |
There was a problem hiding this comment.
ContinueRead is not defined in crate::netlink::message (it lives in message::result), so this import will fail to compile. Import ContinueRead from super::result::ContinueRead or crate::netlink::message::result::ContinueRead instead.
| use crate::netlink::message::{ContinueRead, NLMSG_ALIGN}; | |
| use crate::netlink::message::NLMSG_ALIGN; | |
| use crate::netlink::message::result::ContinueRead; |
| common::SegmentCommon, | ||
| header::*, | ||
| }; | ||
| use super::{ContinueRead, NLMSG_ALIGN}; |
There was a problem hiding this comment.
ContinueRead is not available as super::ContinueRead here (it is declared in message::result). This use super::{ContinueRead, NLMSG_ALIGN}; import should instead pull ContinueRead from super::result::ContinueRead (or re-export ContinueRead from message/mod.rs).
| use super::{ContinueRead, NLMSG_ALIGN}; | |
| use super::NLMSG_ALIGN; | |
| use super::result::ContinueRead; |
|
Hi @Anekoique, We are migrating our development repository to https://github.com/arceos-org/arceos/tree/dev. If your development has stalled, could you rebase it there? Thanks! |
This PR introduces support for Netlink Route, referencing the implementations of Asterinas and DragonOS.
Some Linux apps rely on simple Netlink mechanisms to retrieve link or address information. This support is crucial for adapting FastDDS and ROS 2 to Starry.
Now, Starry can run ip link show and ip addr show normally:
Additional Fix (FS):
The original function is_valid checked file open flags based on std::fs::OpenOptions, which does not conform to POSIX semantics (e.g., it was unable to create read-only files), leading to failures in some programs.
I have renamed is_valid to check_options to perform simple flag checks. More comprehensive checks may need to be added later, referencing https://man7.org/linux/man-pages/man2/open.2.html.