Inventory fixes #134

christoph-zededa · 2026-01-08T17:08:13Z

info/hardware: use port number for USBAddress
this has two advantages:

we only need port number and not both
(port number is needed for USB passthrough)
the device number might change during the runtime, but still
point to the same device; so when used for the parent relation
devnum might change, but still point to the same device. Using
the port number here instead, gives us a stable USBAddress which
might easier to implement on the controller side.

hardware/info: remove unnecessary fields

cpu_count can just be derived from counting the CPUs
acs_enabled does not make sense for a USB device as it
is a property of a PCI device or of the whole node

christoph-zededa · 2026-01-08T17:11:28Z

as there is no implementation of it yet, I took the freedom to delete entries instead of deprecating them

europaul

LGTM

Copilot

Pull request overview

This pull request makes several breaking changes to the hardware inventory protobuf definitions to improve USB device addressing and remove unnecessary fields. The main goal is to use stable USB port numbers instead of device numbers, and clean up fields that don't belong in certain message types.

Changes:

Modified USBAddress to use port (string) instead of devnum (uint32) for stable device identification
Renamed USBDevice.bus_devnum to bus_port to reflect the addressing change
Removed acs_enabled field from USBDevice (ACS is a PCIe-specific feature)
Removed cpu_count field from HardwareInventory (can be derived from cpu_info)

Reviewed changes

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

File	Description
proto/info/hardware.proto	Modified USBAddress message structure, removed unnecessary fields from USBDevice and HardwareInventory
go/info/hardware.pb.go	Generated Go code reflecting proto changes with updated types, field names, and getter methods
python/info/hardware_pb2.py	Generated Python code with updated serialization descriptors for proto changes

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

Copilot · 2026-01-12T18:04:38Z

proto/info/hardware.proto

        uint32 class_id = 6;    // 3 bytes: (base_class, sub_class, protocol)
        // Optional: EVE's suggested assignment group for this device.
        // Controller may derive its own based on parent relationships.
        string suggested_assigngrp = 7;


Removing the acs_enabled field from USBDevice is a breaking API change. Consumers of this API that rely on this field will experience failures. The PR description indicates this field doesn't make sense for USB devices (as ACS is a PCIe feature), but the removal should be carefully coordinated with all API consumers to ensure they can handle the absence of this field.

Suggested change

string suggested_assigngrp = 7;

string suggested_assigngrp = 7;

// Deprecated: ACS is a PCIe feature and does not conceptually apply to USB devices.

// This field is retained for backward compatibility only and should not be used for new code.

bool acs_enabled = 8 [deprecated = true];

Copilot · 2026-01-12T18:04:39Z

proto/info/hardware.proto

        uint32 product_id = 3;

-        USBAddress bus_devnum = 4;
+        USBAddress bus_port = 4;


Renaming the field from bus_devnum to bus_port is a breaking API change. This will cause issues for any code that accesses this field using the old name. While the field number (4) remains the same which helps with protobuf wire format compatibility, the JSON serialization and code that accesses the field by name will break.

Suggested change

USBAddress bus_port = 4;

USBAddress bus_devnum = 4;

Copilot · 2026-01-12T18:04:39Z

proto/info/hardware.proto

  //  - EVE platform
  // ------------------------------------------------------------------
  uint64 total_memory_bytes  = 10;  // total system RAM
  uint64 total_storage_bytes = 11;  // total non-removable storage


Removing the cpu_count field from HardwareInventory is a breaking API change. While the PR description states this can be derived from counting the CPUs in cpu_info, removing it forces all API consumers to implement this calculation themselves. This also breaks backward compatibility for any code expecting this field to be populated.

Suggested change

uint64 total_storage_bytes = 11; // total non-removable storage

uint64 total_storage_bytes = 11; // total non-removable storage

uint32 cpu_count = 12; // total number of CPUs

Please add reserved 12;

I've put it to the top of the message: https://github.com/lf-edge/eve-api/pull/134/files#diff-74be222515f7312eabbe246b6a4893535a7c0cc868436836ea0acb26c9ba9106R130

Copilot · 2026-01-12T18:04:39Z

proto/info/hardware.proto

 message USBAddress {
        // https://elixir.bootlin.com/linux/v6.16.9/source/include/linux/usb.h#L451
        // https://github.com/gregkh/usbutils/blob/master/lsusb-t.c#L48
        uint32 bus = 1;


The new port field lacks documentation. While the bus field has references to Linux kernel source code and usbutils documentation, the port field should also include documentation explaining its format, purpose, and relationship to USB port numbers. This would help API consumers understand how to use this field correctly.

Suggested change

uint32 bus = 1;

uint32 bus = 1;

// USB port path on the given bus.

// This is the hierarchical chain of 1-based port numbers from the root hub

// down to the device, with components separated by '.' in the same format

// as shown by `lsusb -t` (e.g. "1", "2.3", "2.3.1").

// Together with `bus`, this uniquely identifies the physical USB connection.

// See also:

// https://elixir.bootlin.com/linux/v6.16.9/source/drivers/usb/core/topology.c

// https://github.com/gregkh/usbutils/blob/master/lsusb-t.c

Copilot · 2026-01-12T18:04:40Z

proto/info/hardware.proto

-        // https://elixir.bootlin.com/linux/v6.16.9/source/include/linux/usb.h#L657
-        // https://github.com/gregkh/usbutils/blob/master/lsusb-t.c#L63
-        uint32 devnum = 2;
+        string port = 2;


The field type change from uint32 to string for the port field in USBAddress represents a breaking API change. Any existing code that reads or writes this field will need to be updated to handle the new string type instead of uint32. This could cause runtime errors in systems that haven't been updated to accommodate this change.

Suggested change

string port = 2;

uint32 port = 2;

eriknordmark · 2026-01-12T22:05:11Z

proto/info/hardware.proto

        string suggested_assigngrp = 7;
-        // True if upstream PCIe ACS (Access Control Services) is enabled for this
-        // device (either natively or via ACS override), for safer isolation/passthrough.
-        bool acs_enabled = 8;


Even though this was never used it might make sense to mark tag 8 as deprecated using
reserved 8;
at the top of the message definition.

I added it here: https://github.com/lf-edge/eve-api/pull/134/files#diff-74be222515f7312eabbe246b6a4893535a7c0cc868436836ea0acb26c9ba9106R30

eriknordmark

We can ignore the buflint comments since this API has not yet been put into use anywhere.

Does the copilot suggestion to add documentation make sense?

eriknordmark

LGTM

this has two advantages: 1. we only need port number and not both (port number is needed for USB passthrough) 2. the device number might change during the runtime, but still point to the same device; so when used for the parent relation `devnum` might change, but still point to the same device. Using the port number here instead, gives us a stable `USBAddress` which might easier to implement on the controller side. Signed-off-by: Christoph Ostarek <christoph@zededa.com>

* cpu_count can just be derived from counting the CPUs * acs_enabled does not make sense for a USB device as it is a property of a PCI device or of the whole node Signed-off-by: Christoph Ostarek <christoph@zededa.com>

christoph-zededa · 2026-01-13T18:35:56Z

Does the copilot suggestion to add documentation make sense?

I instead put in the link to the linux kernel code: https://github.com/lf-edge/eve-api/pull/134/files#diff-74be222515f7312eabbe246b6a4893535a7c0cc868436836ea0acb26c9ba9106R15

eriknordmark · 2026-01-13T20:02:27Z

Can you fix this typo
codespell: syfs ==> sysfs

christoph-zededa · 2026-01-13T20:34:15Z

Can you fix this typo codespell: syfs ==> sysfs

done: https://github.com/lf-edge/eve-api/compare/0adde85996e6d6e3e9bb126c93bc2ea4e4bf6aeb..b5d82aadc1c8e1d45f65a2f185e09703b90d3845

Signed-off-by: Christoph Ostarek <christoph@zededa.com>

kperakis-zededa

I will approve the changes @christoph-zededa please make sure the Zedcloud team is informed as well.

christoph-zededa requested a review from eriknordmark as a code owner January 8, 2026 17:08

christoph-zededa requested review from Copilot, europaul and kperakis-zededa January 12, 2026 18:00

Copilot started reviewing on behalf of christoph-zededa January 12, 2026 18:00 View session

europaul approved these changes Jan 12, 2026

View reviewed changes

Copilot AI reviewed Jan 12, 2026

View reviewed changes

eriknordmark reviewed Jan 12, 2026

View reviewed changes

eriknordmark approved these changes Jan 12, 2026

View reviewed changes

christoph-zededa force-pushed the inventory-fixes branch from 04cb653 to 0adde85 Compare January 13, 2026 17:55

christoph-zededa added 2 commits January 13, 2026 19:33

hardware/info: remove unnecessary fields

9424387

* cpu_count can just be derived from counting the CPUs * acs_enabled does not make sense for a USB device as it is a property of a PCI device or of the whole node Signed-off-by: Christoph Ostarek <christoph@zededa.com>

christoph-zededa force-pushed the inventory-fixes branch from 0adde85 to b5d82aa Compare January 13, 2026 20:33

add generated go and python files

02e870d

Signed-off-by: Christoph Ostarek <christoph@zededa.com>

christoph-zededa force-pushed the inventory-fixes branch from b5d82aa to 02e870d Compare January 14, 2026 10:21

kperakis-zededa approved these changes Jan 14, 2026

View reviewed changes

eriknordmark merged commit 90ed7cf into lf-edge:main Jan 14, 2026
3 of 4 checks passed

europaul mentioned this pull request Jan 15, 2026

Send HW inventory lf-edge/eve#5535

Open

6 tasks

-        string suggested_assigngrp = 7;
+        string suggested_assigngrp = 7;
+        // Deprecated: ACS is a PCIe feature and does not conceptually apply to USB devices.
+        // This field is retained for backward compatibility only and should not be used for new code.
+        bool acs_enabled = 8 [deprecated = true];

	uint64 total_storage_bytes = 11; // total non-removable storage
	uint64 total_storage_bytes = 11; // total non-removable storage
	uint32 cpu_count = 12; // total number of CPUs

-        uint32 bus = 1;
+        uint32 bus = 1;
+        // USB port path on the given bus.
+        // This is the hierarchical chain of 1-based port numbers from the root hub
+        // down to the device, with components separated by '.' in the same format
+        // as shown by `lsusb -t` (e.g. "1", "2.3", "2.3.1").
+        // Together with `bus`, this uniquely identifies the physical USB connection.
+        // See also:
+        // https://elixir.bootlin.com/linux/v6.16.9/source/drivers/usb/core/topology.c
+        // https://github.com/gregkh/usbutils/blob/master/lsusb-t.c

Inventory fixes #134

Inventory fixes #134

Uh oh!

Conversation

christoph-zededa commented Jan 8, 2026

Uh oh!

christoph-zededa commented Jan 8, 2026

Uh oh!

europaul left a comment

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 Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

eriknordmark Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

christoph-zededa Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

eriknordmark Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

christoph-zededa Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

eriknordmark left a comment

Choose a reason for hiding this comment

Uh oh!

eriknordmark left a comment

Choose a reason for hiding this comment

Uh oh!

christoph-zededa commented Jan 13, 2026

Uh oh!

eriknordmark commented Jan 13, 2026

Uh oh!

christoph-zededa commented Jan 13, 2026

Uh oh!

kperakis-zededa left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants