|WARNING| [v5] dts: add support for no link topology

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/166630

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-07-08

# DPDK Patch Review

## Summary
This patch adds support for running DTS with no traffic generator node and no ethdev interfaces. The implementation is generally sound with proper handling of the no-link topology throughout the codebase. However, there are several issues that need to be addressed.

---

## Errors

### 1. Incorrect error message reachability in `node_and_ports_from_id()`
**File:** `dts/framework/testbed_model/topology.py`
**Lines:** 107-113

The error message `f"Invalid node '{node_identifier}' given."` is unreachable after the function logic change. The `match` statement now only handles the `"tg"` case, leaving `"sut"` (which was previously handled by the `match` before this patch) to fall through.

**Current code:**
```python
case "sut":
    return ctx.sut_node, self.sut_ports
case "tg":
    if ctx.tg_node is not None:
        return ctx.tg_node, self.tg_ports
    msg = "node tg does not exist with current topology."
    raise InternalError(msg)
msg = f"Invalid node `{node_identifier}` given."
raise InternalError(msg)
```

**Issue:** If `node_identifier` is `"sut"`, the function returns at line 106. If it is `"tg"` and `ctx.tg_node is None`, it raises at line 108. If it is `"tg"` and `ctx.tg_node is not None`, it returns at line 107. The only way to reach line 112-113 is if `node_identifier` is neither `"sut"` nor `"tg"`, but the match doesn't have a `case _:` to capture that. With the current structure, this code is unreachable.

**Suggested fix:**
```python
match node_identifier:
    case "sut":
        return ctx.sut_node, self.sut_ports
    case "tg":
        if ctx.tg_node is not None:
            return ctx.tg_node, self.tg_ports
        msg = "node tg does not exist with current topology."
        raise InternalError(msg)
    case _:
        msg = f"Invalid node `{node_identifier}` given."
        raise InternalError(msg)
```

---

## Warnings

### 1. Unused variables in `from_port_links()`
**File:** `dts/framework/testbed_model/topology.py`
**Lines:** 75-77

Variables `sut_ports` and `tg_ports` are initialized as empty lists but never used. These appear to be dead code introduced in the refactoring.

**Suggested fix:** Remove the unused variable initializations:
```python
def from_port_links(cls, port_links: Iterator[PortLink]) -> Self:
    type = LinkTopology.NO_LINK

    if port_link := next(port_links, None):
        # ... rest of function
```

### 2. String formatting inconsistency in test output
**File:** `dts/tests/TestSuite_cryptodev_latency.py`
**Lines:** 102, 107

The f-strings use inconsistent spacing around the `*` operator: `{'='*border_len}` vs `{'=' * border_len}`.

**Suggested fix:** While the second usage (line 107) with spaces is now consistent with the first, line 102 should also be updated:
```python
print(f"|\n{'=' * border_len}")  # line 102
```

This change improves readability per Python style guidelines (spaces around operators).

### 3. Missing null check documentation
**File:** `dts/api/test.py`
**Line:** 117

The condition `ctx.tg_node is not None` is added without updating the docstring to explain that TG command history is only shown when a TG node exists.

**Suggested fix:** Update the docstring for the `fail()` function to document this behavior:
```python
def fail(failure_description: str) -> None:
    """Record that a test case failed with a reason.

    Logs the last 10 commands executed on the SUT node.
    If a TG node is configured and the topology is not NO_LINK,
    also logs the last 10 commands executed on the TG node.
    
    Args:
        failure_description: A short description of why the test case failed.
    
    Raises:
        TestCaseVerifyError: Always raised to indicate the test case failed.
    """
```

---

## Code Quality Notes

### 1. Consistent null checking pattern
The patch correctly adds `ctx.tg_node is not None` checks throughout, which is good defensive programming. The pattern is applied consistently in:
- `test.py` (line 117)
- `test_run.py` (lines 202, 358, 465, 467, 469, 471)
- `topology.py` (line 349)

### 2. Configuration validation
The validator changes in `framework/config/__init__.py` correctly handle the case where `port_topology` is empty by skipping TG node validation. The logic at lines 126-131 properly checks `port_topology != []` before validating the TG node.

### 3. Empty iterator handling
The call `Topology.from_port_links(iter([]))` at line 202 in `test_run.py` correctly creates a no-link topology by passing an empty iterator. This is the proper way to signal no ports.

---

## Summary of Required Changes

**Must fix (Errors):**
1. Fix unreachable error message in `node_and_ports_from_id()` by adding `case _:` to the match statement

**Should fix (Warnings):**
1. Remove unused `sut_ports` and `tg_ports` variable initializations in `from_port_links()`
2. Fix string formatting inconsistency in `TestSuite_cryptodev_latency.py` line 102
3. Update docstring for `fail()` function to document TG command history conditional logging