Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ADOPTERS.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,4 @@ To add your organization to this list, feel free to PR the additions to this pag

| **Organization** | **Reference or Description of Usage** |
|------------------|---------------------------------------|
| | |
| [Philips](https://www.philips.com) | Using Akri to dynamically discover and manage mice, keyboards, and USB storage devices at the edge. |
26 changes: 13 additions & 13 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ members = [
resolver = "2"

[workspace.package]
version = "0.13.22"
version = "0.13.23"
edition = "2024"
license = "Apache-2.0"
homepage = "https://docs.akri.sh/"
Expand Down
4 changes: 2 additions & 2 deletions deployment/helm/Chart.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ type: application
# This is the chart version. This version number should be incremented each time you make changes
# to the chart and its templates, including the app version.
# Versions are expected to follow Semantic Versioning (https://semver.org/)
version: 0.13.22
version: 0.13.23

# This is the version number of the application being deployed. This version number should be
# incremented each time you make changes to the application. Versions are not expected to
# follow Semantic Versioning. They should reflect the version the application is using.
appVersion: 0.13.22
appVersion: 0.13.23
171 changes: 171 additions & 0 deletions discovery-handlers/udev/src/device_utils.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,171 @@
fn transform_resource_name(resource_name: &str) -> String {
resource_name
.chars()
.map(|c| match c {
'.' | '/' => '_',
other => other.to_ascii_uppercase(),
Comment thread
koendelaat marked this conversation as resolved.
})
.collect()
}

pub fn to_usb_resource_env_var(resource_name: &str) -> String {
format!(
"{}_{}",
super::USB_RESOURCE_PREFIX,
transform_resource_name(resource_name)
)
}

pub fn to_pci_resource_env_var(resource_name: &str) -> String {
format!(
"{}_{}",
super::PCI_RESOURCE_PREFIX,
transform_resource_name(resource_name)
)
}

pub fn extract_usb_address(devnode: &str) -> Option<(String, String)> {
if !devnode.starts_with("/dev/bus/usb/") {
return None;
}

let parts: Vec<&str> = devnode.split('/').collect();
if parts.len() >= 2 {
let bus = parts[parts.len() - 2];
let device = parts[parts.len() - 1];

if let (Ok(bus_num), Ok(dev_num)) = (bus.parse::<u32>(), device.parse::<u32>()) {
return Some((bus_num.to_string(), dev_num.to_string()));
}
}

None
}

fn is_pci_address(s: &str) -> bool {
let parts: Vec<&str> = s.splitn(3, ':').collect();
if parts.len() != 3 {
return false;
}
let slot_func: Vec<&str> = parts[2].splitn(2, '.').collect();
if slot_func.len() != 2 {
return false;
}
[parts[0], parts[1], slot_func[0], slot_func[1]]
.iter()
.all(|seg| !seg.is_empty() && seg.chars().all(|c| c.is_ascii_hexdigit()))
}

pub fn extract_pci_address(sysfs_path: &str) -> Option<String> {
sysfs_path
.split('/')
.filter(|s| is_pci_address(s))
.next_back()
.map(|s| s.to_string())
}

pub fn read_iommu_group(devpath: &str) -> Option<String> {
let full_path = format!("/sys{devpath}");
let iommu_link = std::path::Path::new(&full_path).join("iommu_group");
std::fs::read_link(&iommu_link).ok().and_then(|target| {
target
.file_name()
.and_then(|n| n.to_str())
.map(|s| s.to_string())
})
Comment on lines +67 to +75
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

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

read_iommu_group uses std::fs::read_link from within the discovery handler's Tokio task. This is blocking I/O and can stall the async runtime under load. Consider switching to tokio::fs::read_link or wrapping the filesystem read in tokio::task::spawn_blocking (and handling errors accordingly).

Copilot uses AI. Check for mistakes.
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.

The concern is noted, but std::fs::read_link on a sysfs path is a kernel memory read (microsecond-level) rather than true disk I/O. The same tokio task already performs heavier synchronous blocking calls (udev_enumerator::create_enumerator, do_parse_and_find) — this is the established pattern in this handler. Given the 10-second polling interval, there is no practical runtime stall risk. If the entire discovery loop is ever refactored to use spawn_blocking, read_iommu_group would be included at that time.

}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn test_to_usb_resource_env_var() {
assert_eq!(
to_usb_resource_env_var("akri.sh/udev-usb-keyboard"),
"USB_RESOURCE_AKRI_SH_UDEV-USB-KEYBOARD"
);
assert_eq!(
to_usb_resource_env_var("akri.sh/udev-usb-mouse"),
"USB_RESOURCE_AKRI_SH_UDEV-USB-MOUSE"
);
assert_eq!(
to_usb_resource_env_var("example.com/my-device"),
"USB_RESOURCE_EXAMPLE_COM_MY-DEVICE"
);
}

#[test]
fn test_to_pci_resource_env_var() {
assert_eq!(
to_pci_resource_env_var("example.com/my-gpu"),
"PCI_RESOURCE_EXAMPLE_COM_MY-GPU"
);
assert_eq!(
to_pci_resource_env_var("akri.sh/udev-gpu-t400e"),
"PCI_RESOURCE_AKRI_SH_UDEV-GPU-T400E"
);
assert_eq!(
to_pci_resource_env_var("vendor.io/device"),
"PCI_RESOURCE_VENDOR_IO_DEVICE"
);
}

#[test]
fn test_extract_pci_address() {
assert_eq!(
extract_pci_address("/sys/devices/pci0000:00/0000:00:01.0/0000:01:00.0"),
Some("0000:01:00.0".to_string())
);
assert_eq!(
extract_pci_address("/sys/devices/pci0000:00/0000:03:00.0"),
Some("0000:03:00.0".to_string())
);
assert_eq!(extract_pci_address("/dev/bus/usb/001/010"), None);
assert_eq!(extract_pci_address("/dev/video0"), None);
}

#[test]
fn test_extract_usb_address_valid() {
assert_eq!(
extract_usb_address("/dev/bus/usb/001/010"),
Some(("1".to_string(), "10".to_string()))
);

assert_eq!(
extract_usb_address("/dev/bus/usb/002/005"),
Some(("2".to_string(), "5".to_string()))
);

assert_eq!(
extract_usb_address("/dev/bus/usb/003/127"),
Some(("3".to_string(), "127".to_string()))
);
}

#[test]
fn test_extract_usb_address_invalid() {
assert_eq!(extract_usb_address("/dev/video0"), None);
assert_eq!(extract_usb_address("/dev/sda"), None);
assert_eq!(extract_usb_address("/dev/ttyUSB0"), None);

assert_eq!(extract_usb_address("/dev/bus/usb/"), None);
assert_eq!(extract_usb_address("/dev/bus/usb/abc/def"), None);

assert_eq!(extract_usb_address(""), None);
assert_eq!(extract_usb_address("/"), None);
}

#[test]
fn test_extract_usb_address_edge_cases() {
assert_eq!(
extract_usb_address("/dev/bus/usb/1/5"),
Some(("1".to_string(), "5".to_string()))
);

assert_eq!(
extract_usb_address("/dev/bus/usb/001/001"),
Some(("1".to_string(), "1".to_string()))
);
}
}
53 changes: 53 additions & 0 deletions discovery-handlers/udev/src/discovery_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,19 @@ impl DiscoveryHandler for DiscoveryHandlerImpl {
let discovery_handler_config: UdevDiscoveryDetails =
deserialize_discovery_details(&discover_request.discovery_details)
.map_err(|e| tonic::Status::new(tonic::Code::InvalidArgument, format!("{e}")))?;
let device_plugin_resource_name: Option<String> = discover_request
.discovery_properties
.get(super::DEVICE_PLUGIN_RESOURCE_PROPERTY_KEY)
.and_then(|b| b.vec.as_ref())
.and_then(|v| std::str::from_utf8(v).ok())
.map(|s| s.trim().to_string());
let vfio_passthrough: bool = discover_request
.discovery_properties
.get(super::VFIO_PASSTHROUGH_PROPERTY_KEY)
.and_then(|b| b.vec.as_ref())
.and_then(|v| std::str::from_utf8(v).ok())
.map(|s| s.trim().eq_ignore_ascii_case("true"))
.unwrap_or(false);
let mut previously_discovered_devices: Vec<Device> = Vec::new();
tokio::spawn(async move {
let udev_rules = discovery_handler_config.udev_rules.clone();
Expand Down Expand Up @@ -130,6 +143,16 @@ impl DiscoveryHandler for DiscoveryHandlerImpl {
super::UDEV_DEVNODE_LABEL_ID.to_string() + &property_suffix,
devnode.clone(),
);

if let Some(ref resource_name) = device_plugin_resource_name {
if let Some((bus, device)) = super::device_utils::extract_usb_address(&devnode) {
let env_var = super::device_utils::to_usb_resource_env_var(resource_name);
let value = format!("{bus}:{device}");
trace!("discover - USB resource: {env_var}={value} path={devnode}");
properties.insert(env_var + &property_suffix, value);
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

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

properties.insert(env_var + &property_suffix, value) appends the group-recursive index suffix to the USB_RESOURCE_<RESOURCE> env var name. KubeVirt expects a specific key format (USB_RESOURCE_<RESOURCE_NAME>) without additional suffixes, so this can prevent it from finding the variable when groupRecursive is enabled. Consider setting the USB resource env var without property_suffix (and, if multiple USB devnodes are present, decide deterministically which value wins and/or log that multiple were found).

Suggested change
properties.insert(env_var + &property_suffix, value);
if let Some(existing_value) = properties.get(&env_var) {
trace!(
"discover - USB resource already set: {env_var}={existing_value}, ignoring additional value {value} path={devnode}"
);
} else {
properties.insert(env_var, value);
}

Copilot uses AI. Check for mistakes.
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.

The concern is technically valid in the abstract, but USB passthrough always uses groupRecursive: false (the default). In that case property_suffix is an empty string, so the key is exactly USB_RESOURCE_ as KubeVirt expects. The groupRecursive: true mode is intended for multi-node device groups (e.g. a camera with multiple video nodes), which is a different use case and would not apply here.

}
}

device_specs.push(DeviceSpec {
container_path: devnode.clone(),
host_path: devnode,
Expand All @@ -141,6 +164,36 @@ impl DiscoveryHandler for DiscoveryHandlerImpl {
//id is the sysfs path of the most top level device so we only need this one
properties.insert(super::UDEV_DEVPATH_LABEL_ID.to_string(), id.clone());

let is_usb_device = id.split('/').any(|s| s.starts_with("usb"));
if let Some(ref resource_name) = device_plugin_resource_name {
if !is_usb_device {
if let Some(pci_addr) = super::device_utils::extract_pci_address(&id) {
let env_var = super::device_utils::to_pci_resource_env_var(resource_name);
trace!("discover - PCI resource: {env_var}={pci_addr} path={id}");
properties.insert(env_var, pci_addr);

if vfio_passthrough {
if let Some(iommu_group) = super::device_utils::read_iommu_group(&id) {
let vfio_group = format!("/dev/vfio/{iommu_group}");
trace!("discover - PCI VFIO group: {vfio_group} path={id}");
device_specs.push(DeviceSpec {
container_path: vfio_group.clone(),
host_path: vfio_group,
permissions: "rwm".to_string(),
});
device_specs.push(DeviceSpec {
container_path: "/dev/vfio/vfio".to_string(),
host_path: "/dev/vfio/vfio".to_string(),
permissions: "rwm".to_string(),
});
} else {
trace!("discover - PCI device {id} has no IOMMU group (not vfio-pci bound?), skipping vfio DeviceSpec");
}
}
}
}
}

// TODO: use device spec
Device {
id,
Expand Down
Loading
Loading