Re: [PATCH v3 2/2] doc: add device hotplug documentation

[Stephen Hemminger <stephen@networkplumber.org>]

On Wed, 25 Feb 2026 11:52:25 +0000
Anatoly Burakov <anatoly.burakov@intel.com> wrote:

> Currently, device hotplug is not documented except in API headers. Add
> documentation for device hotplug, both for user facing side of it, and a
> high level overview of its internal workings.
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
> 
> Notes:
>     v2:
>     - Removed "summary" section
> 
>  doc/guides/prog_guide/dev_args.rst       |   3 +
>  doc/guides/prog_guide/device_hotplug.rst | 286 +++++++++++++++++++++++
>  doc/guides/prog_guide/index.rst          |   1 +
>  3 files changed, 290 insertions(+)
>  create mode 100644 doc/guides/prog_guide/device_hotplug.rst


This is great to see. It would be the flow from kernel to udev/systemd was explained.
If I remember right, mlx does this through another IB mechanism.
Also mention this is Linux only.

AI review is good at finding grammar and other things.


---

**Overall assessment:** This is a solid and welcome addition. Device hotplug has long needed proper documentation beyond the API headers. The structure is logical, the code examples are helpful, and the multi-process synchronization section is particularly well done. A few issues worth addressing:

**Technical issues:**

1. **`device_event_callback` return type mismatch (line 289):** The callback is declared as returning `int`, but `rte_dev_event_cb_fn` is typedef'd as returning `void`. This will cause a compiler warning/error if anyone copies this example verbatim. Should be `void` with no return statement.

2. **Missing error check on `rte_dev_probe()` (line 205):** The "Device Lifecycle Example" calls `rte_dev_probe(devargs)` without checking the return value, while the "Basic Usage" example above it correctly shows error handling. For a reference example demonstrating "proper device lifecycle management," this is inconsistent. Should check the return.

3. **`port_id` type (line 203):** `port_id` is declared as `unsigned` but `RTE_ETH_FOREACH_MATCHING_DEV` expects `uint16_t`. Should be `uint16_t port_id;`.

4. **`rte_eal_hotplug_add()`/`rte_eal_hotplug_remove()` deprecation status (line 179):** Are these functions deprecated or planned for deprecation in favor of `rte_dev_probe()`/`rte_dev_remove()`? If so, worth mentioning. If not, it might still be good to indicate which API is preferred for new code.

**Documentation nits:**

5. **Line 152:** "after the initial EAL initialization has completed" — consider noting that `rte_eal_init()` must have returned successfully, since that's the specific precondition.

6. **Line 224:** "to find newly attached port" → "to find the newly attached port"

7. **Line 232:** "In addition to providing generic hotplug infrastructure to be used in the above described manner" — a bit wordy. Consider: "In addition to the hotplug APIs described above, EAL also offers a device event infrastructure."

8. **Line 269:** "When ``RTE_DEV_EVENT_REMOVE`` event is delivered, the EAL has already released all internal resources associated with the device." — Is this accurate? My understanding is that the uevent notification tells the application the kernel removed the device, but it's still up to the application to call `rte_dev_remove()` to clean up EAL resources. If the EAL had already cleaned up, there'd be nothing for the application to do. Please double-check this claim.

9. **Line 412, nl_groups:** The doc says `nl_groups = 0xffffffff`. This is an implementation detail that could change and may be more confusing than helpful for users reading the prog guide. Consider dropping it or noting it's an implementation note.

**Structural:**

10. **Placement in index.rst:** The new page is added right after `dev_args` under "Device Libraries," which makes sense given the cross-references. Good.

11. **The "Implementation Details" section** (line 308 onward) switches from user-facing guidance to internal architecture. This is useful for contributors but might be better as a separate subsection with a note that it describes internals. Currently the document transitions from "here's how to use it" to "here's how it works internally" without much signaling.

**Minor:**
- Copyright says 2025 (line 142) — should be 2026 given the submission date.