|WARNING| [v4] dts: resolve trex driver binding issue

[dpdklab at iol.unh.edu]

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

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-10-29

# DPDK Patch Review

## ERRORS

### Error 1: TRex TG not torn down before re-setup
**File:** `dts/framework/test_run.py`  
**Lines:** 512-524

The code checks `other_tg.is_setup` before calling `other_tg.teardown()`, but it does not check `required_tg.is_setup` before calling `required_tg.setup()`. This creates a logic error: if `required_tg` is already set up (e.g., from a previous test suite of the same type), calling `setup()` again without tearing it down first could leave stale state or fail.

**Quoted code:**
```python
if other_tg and other_tg.is_setup:
    other_tg.teardown()

if required_tg and not required_tg.is_setup:
    required_tg.setup(ctx.topology)
```

**Why it matters:** If two functional test suites run consecutively, the second suite would skip `setup()` because `func_tg.is_setup` is already true from the first suite. This violates the stated goal of separating TG setup by suite.

**Suggested fix:**
```python
if other_tg and other_tg.is_setup:
    other_tg.teardown()

if required_tg:
    if required_tg.is_setup:
        required_tg.teardown()
    required_tg.setup(ctx.topology)
```

### Error 2: Cached lshw data deleted without existence check
**File:** `dts/framework/testbed_model/linux_session.py`  
**Lines:** 281, 291

The patch adds `if self._lshw_net_info:` checks before `del self._lshw_net_info`. This pattern is incorrect: `del` requires the attribute to exist, but `if self._lshw_net_info:` tests the attribute's truthiness (whether it is non-empty), not whether it exists. An empty list/dict would pass the existence check but fail the truthiness test, causing `del` to be skipped when it should run.

**Quoted code:**
```python
if self._lshw_net_info:
    del self._lshw_net_info
```

**Why it matters:** If `_lshw_net_info` is bound to an empty container, the `del` is skipped and stale data persists. This is a correctness bug even though the likelihood depends on whether lshw can return empty results.

**Suggested fix:**
```python
if hasattr(self, '_lshw_net_info'):
    del self._lshw_net_info
```

or use a try/except:
```python
try:
    del self._lshw_net_info
except AttributeError:
    pass
```

## WARNINGS

### Warning 1: Missing release notes
**File:** N/A (expected `doc/guides/rel_notes/release_XX_XX.rst`)

This patch changes user-visible DTS behavior (test suite execution flow and TG binding) and adds new API (`test_suite_type`, `func_suite`, `perf_suite` decorators). The guidelines require release notes for such changes.

**Suggested fix:**  
Add a section to the current release notes file under "DTS" describing:
- Test suite type decorators (`@func_suite`, `@perf_suite`)
- Ability to run functional and performance suites in a single DTS run
- TRex TG setup/teardown now per-suite instead of once per execution

### Warning 2: New API not marked experimental
**File:** `dts/api/test_suite.py`  
**Lines:** 97, 304-305

The patch introduces new public API elements: `test_suite_type` class variable, `TestSuiteType` enum, `make_decorator()` method, and `func_suite`/`perf_suite` decorators. These are used across all test suites, making them part of the API contract. However, they are not marked `__rte_experimental`.

**Why it matters:** New API must be marked experimental for at least one release cycle per DPDK policy.

**Suggested fix:**  
Since this is Python API (no C ABI), document the experimental status in docstrings:
```python
class TestSuiteType(Enum):
    """Enum to specify test suite type.
    
    .. note::
       Experimental API. May change in future releases.
    """
```

## INFO

### Info 1: TG teardown in TRex not implemented (pre-existing)
**File:** `dts/framework/testbed_model/traffic_generator/trex.py`  
**Line:** Not touched by patch

The patch adds `teardown()` calls to TRex TG, but the TRex class does not appear to implement a `teardown()` method that reverses the binding done in `setup()` (line 113: `topology.configure_ports("tg", "dpdk")`). This means TG ports remain bound to DPDK even after teardown.

**Pre-existing issue.** The patch assumes `teardown()` exists and works. If it doesn't, that's a separate bug not introduced here.

---

## FINAL CHECK COMPLETE

All findings verified against the evidence rule and severity guidelines.

Review-Result: ERROR