* Re: [PATCH v4 12/20] net/txgbe: fix link stability for 25G NIC
From: Stephen Hemminger @ 2026-05-17 23:49 UTC (permalink / raw)
To: Zaiyu Wang; +Cc: dev, stable, Jiawen Wu
In-Reply-To: <20260511103604.19724-13-zaiyuwang@trustnetic.com>
On Mon, 11 May 2026 18:35:54 +0800
Zaiyu Wang <zaiyuwang@trustnetic.com> wrote:
> The link was previously configured via firmware, but this approach
> resulted in unstable link behavior. To resolve the issue, re-add the
> PHY configuration flow directly into the driver.
>
> Fixes: ead3616f630d ("net/txgbe: support PHY configuration via SW-FW mailbox")
> Cc: stable@dpdk.org
>
> Signed-off-by: Zaiyu Wang <zaiyuwang@trustnetic.com>
> ---
Lots of AI review feedback here:
# Review of Patch 12/20: net/txgbe: fix link stability for 25G NIC
## Errors
### 1. Use-after-free potential in txgbe_e56_rxs_calib_adapt_seq (CORRECTNESS)
**File:** `drivers/net/txgbe/base/txgbe_e56.c`
**Location:** Lines ~1600-1650
The function calls `txgbe_e56_rxs_osc_init_for_temp_track_range(hw, speed)` which can return `TXGBE_ERR_TIMEOUT`. When the timeout path at line ~1115 is taken, it writes to `E56PHY_PMD_CFG_0_ADDR` to disable RX and then returns `-1`. However, the caller `txgbe_e56_rxs_calib_adapt_seq` continues execution and accesses PHY registers without verifying the hardware state is valid. If the initialization timed out, subsequent register accesses may be unreliable.
**Suggested fix:**
```c
status = txgbe_e56_rxs_osc_init_for_temp_track_range(hw, speed);
if (status != 0)
return status; /* Propagate timeout or error immediately */
```
### 2. Resource leak: lock not released on error path (CORRECTNESS)
**File:** `drivers/net/txgbe/base/txgbe_aml.c`
**Location:** Lines 241-247
In `txgbe_setup_phy_link_aml`, `rte_spinlock_lock(&hw->phy_lock)` is acquired at line 241. If `txgbe_set_link_to_amlite` returns `TXGBE_ERR_PHY_INIT_NOT_DONE` at line 246, the code jumps to `out:` at line 264 without releasing the spinlock. The lock is only released on the success path or `TXGBE_ERR_TIMEOUT`.
**Suggested fix:**
```c
rte_spinlock_lock(&hw->phy_lock);
ret_status = txgbe_set_link_to_amlite(hw, speed);
rte_spinlock_unlock(&hw->phy_lock); /* Always release before checking status */
if (ret_status == TXGBE_ERR_PHY_INIT_NOT_DONE)
goto out;
```
### 3. Missing error propagation to caller (CORRECTNESS)
**File:** `drivers/net/txgbe/base/txgbe_aml.c`
**Location:** Lines 280-283
In `txgbe_setup_phy_link_aml`, the function returns `status` which is initialized to 0 and never assigned a failure value, even when `txgbe_set_link_to_amlite` fails or link is not established. The function always returns success regardless of actual outcome.
**Suggested fix:**
```c
if (!link_up) {
*need_reset = true;
return TXGBE_ERR_TIMEOUT; /* Return error when link fails */
}
```
### 4. Double error return value overwrite (CORRECTNESS)
**File:** `drivers/net/txgbe/base/txgbe_e56.c`
**Location:** Lines 1090-1095, 1157-1162, 1197-1202, 1242-1247
In `txgbe_e56_rxs_osc_init_for_temp_track_range`, the pattern:
```c
if (timer++ > PHYINIT_TIMEOUT) {
DEBUGOUT("ERROR: ...");
break;
return -1; /* Unreachable: break exits the loop */
}
```
The `return -1` is unreachable. After `break`, execution continues to the next loop iteration check and then proceeds past the loop. The error is not propagated.
**Suggested fix:**
```c
if (timer++ > PHYINIT_TIMEOUT) {
DEBUGOUT("ERROR: Wait timeout");
return TXGBE_ERR_TIMEOUT; /* Remove break, return immediately */
}
```
### 5. Error code dropped without propagation (CORRECTNESS)
**File:** `drivers/net/txgbe/base/txgbe_e56.c`
**Location:** Lines 1828-1830
In `txgbe_temp_track_seq`:
```c
status = txgbe_e56_get_temp(hw, &temperature);
if (status)
return 0; /* Returns success when get_temp failed */
```
Temperature reading failure is silently converted to success. The caller cannot distinguish between a successful temperature track and a failed temperature read.
**Suggested fix:**
```c
status = txgbe_e56_get_temp(hw, &temperature);
if (status)
return status; /* Propagate the error */
```
### 6. Integer overflow in bit shift (CORRECTNESS)
**File:** `drivers/net/txgbe/base/txgbe_e56.c`
**Location:** Lines 19-31 in `set_fields_e56()`
The function performs `(1 << bit_low)` where `bit_low` can be up to 31 (since bit fields in the code span 0-31). The literal `1` is `int` (signed 32-bit). When `bit_low >= 31`, `1 << 31` invokes undefined behavior (signed overflow).
**Suggested fix:**
```c
if (bit_high == bit_low) {
if (set_value == 0)
*src_data &= ~(1U << bit_low); /* Use 1U for unsigned */
else
*src_data |= (1U << bit_low);
} else {
for (i = bit_low; i <= bit_high; i++)
*src_data &= ~(1U << i);
*src_data |= (set_value << bit_low);
}
```
### 7. Missing bounds check on array size calculation (CORRECTNESS)
**File:** `drivers/net/txgbe/base/txgbe_e56.c`
**Location:** Line 1498 in `txgbe_e56_rx_rd_second_code()`
The code calls `qsort(RXS_BBCDR_SECOND_ORDER_ST, array_size, sizeof(int), ...)` where `array_size = ARRAY_SIZE(RXS_BBCDR_SECOND_ORDER_ST)`. The array has 5 elements, but the calculation `array_size = ARRAY_SIZE(...)` trusts the macro. If N changes in the loop initialization (line 1489: `N = 5`), the array could overflow.
**Suggested fix:**
```c
/* Ensure loop count matches array size */
BUILD_ASSERT(N <= ARRAY_SIZE(RXS_BBCDR_SECOND_ORDER_ST));
```
## Warnings
### 1. Variables declared but may be read before initialization (STYLE)
**File:** `drivers/net/txgbe/base/txgbe_aml.c`
**Location:** Lines 325-327, 355-357
Variables `need_reset` are declared immediately before use, which is acceptable C99 style. However, they are declared in the middle of a conditional block scope, which can reduce clarity when the same variable name is reused in different branches.
**Suggested improvement:**
Declare `need_reset` once at function scope for better readability, though the current usage is technically correct.
### 2. Missing documentation for complex algorithm (STYLE)
**File:** `drivers/net/txgbe/base/txgbe_e56.c`
**Location:** Lines 902-1264 (txgbe_e56_rxs_osc_init_for_temp_track_range)
This 360-line function implements a complex PHY oscillator calibration sequence with numbered steps in comments (1, 2, 3...). The algorithm is critical for 25G link stability but lacks a high-level overview comment explaining the purpose, temperature dependency, and why specific register sequences are needed.
**Suggested improvement:**
Add a function-level Doxygen comment explaining the calibration procedure and referencing the hardware specification section.
### 3. Magic numbers without named constants (STYLE)
**File:** `drivers/net/txgbe/base/txgbe_e56.c`
**Location:** Multiple locations (e.g., lines 505, 732, 1000)
Numerous magic numbers appear in register writes (e.g., `0x68c1`, `0x3321`, `0x973e`, `0xccde` at lines 2281-2284) without explanation. These appear to be RS-FEC configuration magic values but are not documented.
**Suggested improvement:**
Define named constants or add inline comments explaining what these values configure:
```c
#define RS_FEC_CONFIG_VAL1 0x68c1 /* RS-FEC configuration register value */
```
## Info
### 1. Deep nesting in calibration sequence
**File:** `drivers/net/txgbe/base/txgbe_e56.c`
**Location:** Lines 1550-1750 (txgbe_e56_rxs_calib_adapt_seq)
The ADC calibration loop (16 iterations) has 5 levels of nesting. Consider extracting the calibration step into a helper function to improve readability.
### 2. Possible candidate for helper function
**File:** `drivers/net/txgbe/base/txgbe_e56.c`
**Location:** Lines 1950-2050 (repeated CTLE bypass pattern)
The code to override CTLE bypass for multiple lanes (RXS1, RXS2, RXS3) at lines 1999-2030 is repetitive. This pattern could be extracted into a helper function.
---
## Summary
**Critical Issues:** 7 Errors identified, primarily correctness bugs affecting resource management and error handling.
**Style Issues:** 3 Warnings - documentation and code organization improvements.
**Observations:** 2 Info items - code structure suggestions.
The most serious issues are:
1. Lock leak on error path (Error #2)
2. Missing error propagation causing silent failures (Errors #3, #5)
3. Undefined behavior in bit operations (Error #6)
4. Unreachable error returns masking timeouts (Error #4)
============================================================
^ permalink raw reply
* Re: [PATCH v4 03/20] net/ngbe: add missing CDR config for YT PHY
From: Stephen Hemminger @ 2026-05-17 23:37 UTC (permalink / raw)
To: Zaiyu Wang; +Cc: dev, stable, Jiawen Wu
In-Reply-To: <20260511103604.19724-4-zaiyuwang@trustnetic.com>
On Mon, 11 May 2026 18:35:45 +0800
Zaiyu Wang <zaiyuwang@trustnetic.com> wrote:
> According to the PHY vendor, when YT8531S operates in UTP-to-Fiber or
> RGMII-to-Fiber mode with auto-negotiation disabled (Force mode),
> additional CDR (Clock Data Recovery) configuration is required to
> improve link connectivity. Without this config, link may be unstable
> or fail to establish.
>
> Fixes: f1268369403d ("net/ngbe: support autoneg on/off for external PHY SFI mode")
> Cc: stable@dpdk.org
>
> Signed-off-by: Zaiyu Wang <zaiyuwang@trustnetic.com>
> ---
Semi-automated AI review feedback.
# DPDK Patch Review - net/ngbe: add missing CDR config for YT PHY
## Summary
This patch adds Clock Data Recovery (CDR) configuration for YT8531S PHY in Force mode (auto-negotiation disabled). No errors found.
---
## CORRECTNESS BUGS
None identified.
---
## WARNINGS
### 1. Missing error checks on PHY register writes
**File:** `drivers/net/ngbe/base/ngbe_phy_yt.c`
**Lines:** 267-268
The two `ngbe_write_phy_reg_sds_ext_yt()` calls do not check return values. If these PHY register writes fail, the link may be unstable (the exact problem this patch aims to fix), but the function will silently continue.
**Current code:**
```c
/* force mode need to config cdr */
ngbe_write_phy_reg_sds_ext_yt(hw, 0x3, 0, 0x1434);
ngbe_write_phy_reg_sds_ext_yt(hw, 0xe, 0, 0x163);
value = YT_BCR_RESET | YT_BCR_DUPLEX;
```
**Suggested fix:**
```c
/* force mode need to config cdr */
s32 ret;
ret = ngbe_write_phy_reg_sds_ext_yt(hw, 0x3, 0, 0x1434);
if (ret != 0)
return ret;
ret = ngbe_write_phy_reg_sds_ext_yt(hw, 0xe, 0, 0x163);
if (ret != 0)
return ret;
value = YT_BCR_RESET | YT_BCR_DUPLEX;
```
Note: This assumes `ngbe_write_phy_reg_sds_ext_yt()` returns 0 on success, negative on failure (standard DPDK convention). The existing code pattern in the function does not check errors on PHY writes in the auto-negotiation path either (lines 264-265), so this may be intentional design. However, for a fix addressing link stability, propagating write failures would be more robust.
---
## INFO
### 1. Magic numbers in PHY register writes
**File:** `drivers/net/ngbe/base/ngbe_phy_yt.c`
**Lines:** 267-268
The CDR configuration uses literal hex values `0x1434` and `0x163` without named constants or comments explaining their meaning. This is vendor-specific configuration. Consider adding a comment referencing the PHY vendor documentation or datasheet section, or defining named constants if these values are used elsewhere.
Example:
```c
/* CDR config per YT8531S datasheet section X.Y for Force mode */
#define YT_CDR_CONFIG_REG3 0x1434
#define YT_CDR_CONFIG_REGE 0x163
ngbe_write_phy_reg_sds_ext_yt(hw, 0x3, 0, YT_CDR_CONFIG_REG3);
ngbe_write_phy_reg_sds_ext_yt(hw, 0xe, 0, YT_CDR_CONFIG_REGE);
```
This is a style suggestion, not a requirement, and may be deferred if the vendor documentation is not publicly available or these values are unique to this use case.
^ permalink raw reply
* Re: [PATCH v4 14/20] net/txgbe: fix link stability for Amber-Lite backplane mode
From: Stephen Hemminger @ 2026-05-17 23:50 UTC (permalink / raw)
To: Zaiyu Wang; +Cc: dev, stable, Jiawen Wu
In-Reply-To: <20260511103604.19724-15-zaiyuwang@trustnetic.com>
On Mon, 11 May 2026 18:35:56 +0800
Zaiyu Wang <zaiyuwang@trustnetic.com> wrote:
> The link was previously configured via firmware, but this approach
> resulted in unstable link behavior. To resolve the issue, re-add the
> PHY configuration flow directly into the driver.
>
> Fixes: ead3616f630d ("net/txgbe: support PHY configuration via SW-FW mailbox")
> Cc: stable@dpdk.org
>
> Signed-off-by: Zaiyu Wang <zaiyuwang@trustnetic.com>
> ---
Lots of AI feedback here:
# Review of DPDK Patch: net/txgbe: fix link stability for 40G NIC
## Summary
This patch re-adds direct PHY configuration flow to the txgbe driver to fix unstable link behavior on 40G NICs. The previous firmware-based configuration approach was causing link stability issues.
## ERRORS
### 1. Use-after-free risk in `txgbe_setup_phy_link_aml40`
**File:** `drivers/net/txgbe/base/txgbe_aml40.c:157-159`
```c
ret_status = txgbe_set_link_to_amlite(hw, speed);
rte_spinlock_unlock(&hw->phy_lock);
if (ret_status == TXGBE_ERR_TIMEOUT)
hw->link_valid = false;
```
The spinlock is unlocked before checking `ret_status`, but if another thread modifies `hw->link_valid` between lines 158 and 160, the state becomes inconsistent. The assignment to `hw->link_valid` should occur before unlocking:
```c
ret_status = txgbe_set_link_to_amlite(hw, speed);
if (ret_status == TXGBE_ERR_TIMEOUT)
hw->link_valid = false;
rte_spinlock_unlock(&hw->phy_lock);
```
### 2. Missing error propagation in `txgbe_e56_rx_rd_second_code_40g`
**File:** `drivers/net/txgbe/base/txgbe_e56.c:1816`
The function declares `status = 0` and returns `status`, but never assigns a failure value even when qsort is called on potentially invalid data. If the timeout in the preceding while loop is reached (line 1825), the SECOND_CODE array may contain incomplete data, but the function still returns success.
### 3. Missing bounds check before array access
**File:** `drivers/net/txgbe/base/txgbe_e56.c:1831`
```c
median = ((N + 1) / 2) - 1;
*SECOND_CODE = RXS_BBCDR_SECOND_ORDER_ST[median];
```
If `N=5`, `median=2` which is valid. However, this code pattern is repeated multiple times (lines 244, 1831, etc.) with `N` as a constant, so it's safe. Nevertheless, adding `RTE_VERIFY(median < ARRAY_SIZE(RXS_BBCDR_SECOND_ORDER_ST))` would make intent explicit.
**Not flagging this as an error** since `N=5` is a fixed constant throughout.
### 4. Timeout return without cleanup in `txgbe_e56_rxs_calib_adapt_seq_40G`
**File:** `drivers/net/txgbe/base/txgbe_e56.c:2475-2481`
```c
if (timer++ > PHYINIT_TIMEOUT) {
rdata = 0;
addr = E56PHY_PMD_CFG_0_ADDR;
rdata = rd32_ephy(hw, addr);
set_fields_e56(&rdata, E56PHY_PMD_CFG_0_RX_EN_CFG, 0x0);
wr32_ephy(hw, addr, rdata);
return TXGBE_ERR_TIMEOUT;
}
```
The function has already configured many registers in the loop `for (i = 0; i < 4; i++)` (starting line 2393). When a timeout occurs on lane 0-2, the function returns immediately without restoring registers on the lanes that were successfully configured. This leaves the hardware in a partially configured state. The cleanup should disable all lanes, not just the one that timed out.
## WARNINGS
### 1. Hardcoded timeout in multiple locations
**File:** `drivers/net/txgbe/base/txgbe_e56.c` (multiple locations)
The `PHYINIT_TIMEOUT` constant is used consistently, but the delays vary (100µs, 500µs, 1000µs, 10ms). For the 500µs delay case (e.g., line 2478), `PHYINIT_TIMEOUT` iterations result in `PHYINIT_TIMEOUT * 500µs` total wait time. If `PHYINIT_TIMEOUT` is intended to be milliseconds, the timeout duration becomes inconsistent across different polling loops. Consider documenting what the timeout value represents (iterations? milliseconds?) and using consistent delay granularity.
### 2. Potentially unreachable code after loop
**File:** `drivers/net/txgbe/base/txgbe_e56.c:2656`
```c
for (j = 0; j < 16; j++) {
// ... ADC adaptation loop
}
/* g. Repeat #a to #f total 16 times */
```
The comment `/* g. Repeat #a to #f total 16 times */` appears *after* the loop that already runs 16 times. This is documentation only, but could be confusing. The comment should be before the loop or removed.
### 3. Inconsistent use of `msleep` vs `usec_delay`
**File:** `drivers/net/txgbe/base/txgbe_e56.c`
The patch uses `msleep()` for delays >= 10ms (lines 181, 3029) and `usec_delay()` for shorter delays (line 1826). However, line 3029 uses `msleep(10)` for 10ms, while line 2707 uses no delay after setting a register. Consider documenting the rationale for sleep vs busy-wait or using a consistent threshold.
### 4. Variable `bypass_ctle` hardcoded but declared as variable
**File:** `drivers/net/txgbe/base/txgbe_e56.c:2396`
```c
u32 bypass_ctle = true;
```
The variable `bypass_ctle` is declared as `u32` but assigned a boolean value, and it's never modified. Either:
- Change to `const bool bypass_ctle = true;` (preferred)
- Or document why it's a runtime variable despite being hardcoded
### 5. Missing validation of speed parameter in initialization functions
**File:** `drivers/net/txgbe/base/txgbe_e56.c:2206`
```c
if (speed == TXGBE_LINK_SPEED_10GB_FULL || speed == TXGBE_LINK_SPEED_40GB_FULL) {
CMVAR_SEC_LOW_TH = S10G_CMVAR_SEC_LOW_TH;
// ...
} else if (speed == TXGBE_LINK_SPEED_25GB_FULL) {
// ...
} else {
DEBUGOUT("Error Speed\n");
return 0; // Returns success despite error
}
```
The function returns 0 (success) when an invalid speed is passed, but logs "Error Speed". This should return an error code like `-EINVAL` or `TXGBE_ERR_PARAM`.
## INFORMATIONAL
### 1. Large function complexity
The function `txgbe_e56_rxs_calib_adapt_seq_40G` spans ~280 lines with deeply nested loops (3-level nesting). Consider refactoring into smaller helper functions for each calibration stage (ADC offset, ADC gain, interleaver adaptation) to improve readability and maintainability.
### 2. Magic numbers without symbolic constants
**File:** `drivers/net/txgbe/base/txgbe_e56.c:2470`
```c
while (EPHY_XFLD(E56G__PMD_CTRL_FSM_RX_STAT_0, ctrl_fsm_rx0_st) != 0x21 ||
```
The value `0x21` (POWERDN_ST) appears in multiple locations (lines 2470, 3145). This is already defined as `E56PHY_RX_POWERDN_ST` elsewhere. Use the symbolic constant consistently.
### 3. Duplicated initialization sequences
The 40G initialization in `txgbe_e56_cfg_40g` (lines 176-566) and the existing 10G/25G code share many similar register sequences. Consider extracting common configuration patterns into helper functions to reduce code duplication and maintenance burden.
### 4. Temperature check frequency
**File:** `drivers/net/txgbe/base/txgbe_e56.c:2253-2256`
The temperature tracking sequence comment states "must be run before the temperature drifts by >5degC" and recommends running every 100ms. However, the patch doesn't add timer-based periodic execution—it only runs during link setup. If temperature tracking is critical for stability, consider documenting that the caller must invoke this periodically.
---
## Positive Observations
1. The patch correctly adds `hw->link_valid` checks in `txgbe_check_mac_link_aml40` to prevent reporting link up when PHY configuration fails (lines 57-60, 80-81).
2. Error paths in timeout scenarios attempt cleanup by disabling RX (e.g., line 2477).
3. The use of median filtering for SECOND_CODE (lines 1829-1831) reduces noise from asynchronous hardware updates—good defensive programming.
^ permalink raw reply
* Re: [PATCH v4 04/20] net/ngbe: fix VF promiscuous and allmulticast
From: Stephen Hemminger @ 2026-05-17 23:39 UTC (permalink / raw)
To: Zaiyu Wang; +Cc: dev, stable, Jiawen Wu
In-Reply-To: <20260511103604.19724-5-zaiyuwang@trustnetic.com>
On Mon, 11 May 2026 18:35:46 +0800
Zaiyu Wang <zaiyuwang@trustnetic.com> wrote:
> The configuration of allmulti and promiscuous modes conflicts
> together. For instance, if we enable promiscuous mode, then enable and
> disable allmulti, then the promiscuous mode is wrongly disabled.
>
> To resolve this, the following changes are made:
> - do nothing when we set/unset allmulti if promiscuous mode is on
> - restore the proper mode (none or allmulti) when we disable
> promiscuous mode
>
> Fixes: 7744e90805b5 ("net/ngbe: add promiscuous and allmulticast ops for VF device")
> Cc: stable@dpdk.org
>
> Signed-off-by: Zaiyu Wang <zaiyuwang@trustnetic.com>
> ---
Worth mentioning, but IMHO this is case of AI being overly pedantic.
Patch is OK as is.
# Review of DPDK Patch: net/ngbe: fix VF promiscuous and allmulticast
## Summary
This patch fixes a mode conflict issue in the ngbe VF driver between promiscuous and allmulticast modes. The overall approach is correct: it prevents mode conflicts by making allmulticast operations no-ops when promiscuous mode is active, and restores the appropriate mode when disabling promiscuous.
## Errors
None identified.
## Warnings
### 1. Implicit comparison in existing code (line 1226)
**Issue:** The existing code uses `if (dev->data->promiscuous == 1)` which mixes explicit and implicit styles. The patch changes this to `if (dev->data->promiscuous)` which is implicit.
**Why it matters:** DPDK coding standards require explicit comparisons for integers (`!= 0` or `== 0`).
**Suggested fix:**
```c
/* Line 1226 - be consistent with explicit comparison */
if (dev->data->promiscuous != 0)
return 0;
/* Line 1250 - same issue */
if (dev->data->promiscuous != 0)
return 0;
```
Note: While the patch changes `== 1` to implicit, the proper fix is to use `!= 0` since `promiscuous` is an integer field, not a boolean type.
### 2. Error propagation pattern inconsistency
**Issue:** The function `ngbevf_dev_promiscuous_disable()` returns 0 early when allmulticast check causes mode selection, but this early return bypasses the switch statement's error handling for the actual mode update call.
**Context:** This is not a bug in the patch itself—the early returns in `ngbevf_dev_allmulticast_enable()` and `ngbevf_dev_allmulticast_disable()` when promiscuous mode is active are correct (those are intentional no-ops). The mode variable change in `ngbevf_dev_promiscuous_disable()` correctly passes through to the existing switch statement for error handling.
No change needed—this comment is for clarity only.
## Info
### Code structure observation
The logic correctly handles the mode transitions:
- When disabling promiscuous mode, it checks `all_multicast` flag and sets the appropriate target mode (ALLMULTI if enabled, NONE otherwise)
- When enabling/disabling allmulticast while promiscuous is active, it returns early as a no-op (correct, since promiscuous overrides allmulticast)
This is a clean fix for the mode conflict issue described in the commit message.
---
## Final Recommendation
Address the implicit comparison style issue (Warning #1) to comply with DPDK coding standards. Change `if (dev->data->promiscuous)` to `if (dev->data->promiscuous != 0)` in both locations (lines 1226 and 1250).
^ permalink raw reply
* Re: [PATCH v4 16/20] net/txgbe: fix SFP module identification
From: Stephen Hemminger @ 2026-05-17 23:52 UTC (permalink / raw)
To: Zaiyu Wang; +Cc: dev, stable, Jiawen Wu
In-Reply-To: <20260511103604.19724-17-zaiyuwang@trustnetic.com>
On Mon, 11 May 2026 18:35:58 +0800
Zaiyu Wang <zaiyuwang@trustnetic.com> wrote:
> Some optical modules were not correctly recognized due to ambiguous
> classification in the original detection flow. Rework the module
> identification logic to cover all module types. Also narrow the
> I2C lock scope to avoid potential race conditions during module
> access.
>
> Fixes: ab191e6d9189 ("net/txgbe: support new SFP/QSFP modules")
> Cc: stable@dpdk.org
>
> Signed-off-by: Zaiyu Wang <zaiyuwang@trustnetic.com>
> ---
Lots of AI feedback here.
Note: not all this is required; like the request for extra release notes is AI being picky.
# Review of Patch 16/20: net/txgbe: fix SFP module identification
## Summary
This patch refactors SFP/QSFP module identification logic and adjusts I2C locking scope. The changes address module type detection and claim to fix race conditions.
---
## Errors
### 1. Resource leak in `txgbe_identify_sfp_module()` - multiple error paths missing unlock
**Error paths that acquire lock but fail to release on error:**
At line 836, `hw->mac.acquire_swfw_sync()` acquires `TXGBE_MNGSEM_SWPHY`. Several error paths after this point fail to release the lock:
```c
err = hw->phy.read_i2c_eeprom(hw, TXGBE_SFF_IDENTIFIER, &identifier);
if (err != 0) {
/* ... */
hw->mac.release_swfw_sync(hw, TXGBE_MNGSEM_SWPHY); // OK - releases
return TXGBE_ERR_SFP_NOT_PRESENT;
}
if (identifier != TXGBE_SFF_IDENTIFIER_SFP) {
hw->phy.type = txgbe_phy_sfp_unsupported;
hw->mac.release_swfw_sync(hw, TXGBE_MNGSEM_SWPHY); // OK - releases
return TXGBE_ERR_SFP_NOT_SUPPORTED;
}
err = hw->phy.read_i2c_eeprom(hw, ...);
/* Multiple read_i2c_eeprom calls follow */
```
**After line 894**, there are at least **4 read_i2c_eeprom() calls** (lines ~895-930 in the context) that can fail with `goto out;` - but `goto out;` is removed in this patch. If any of these reads fail now, the function returns immediately **without releasing the lock acquired at line 836**.
Example at line 1000:
```c
err = hw->phy.read_i2c_eeprom(hw, TXGBE_SFF_CABLE_SPEC_COMP, &cable_spec);
if (err != 0)
goto out; /* This label is REMOVED - now falls through or returns */
```
The old code had `out:` label that might have handled cleanup. The new code removes this pattern but doesn't add `hw->mac.release_swfw_sync()` calls before each early return.
**Impact:** Lock held indefinitely on I2C read failure, blocking other threads.
**Fix:** Add lock release before every early return after line 836:
```c
err = hw->phy.read_i2c_eeprom(hw, TXGBE_SFF_CABLE_SPEC_COMP, &cable_spec);
if (err != 0) {
hw->mac.release_swfw_sync(hw, TXGBE_MNGSEM_SWPHY);
goto out;
}
```
---
### 2. `txgbe_write_i2c_byte()` removes STOP condition - probable hardware protocol violation
**Line 1299 (old) vs 1299 (new):**
Old code:
```c
wr32(hw, TXGBE_I2CDATA, byte_offset | TXGBE_I2CDATA_STOP);
```
New code:
```c
wr32(hw, TXGBE_I2CDATA, byte_offset);
```
The `TXGBE_I2CDATA_STOP` flag is removed from the byte_offset write but remains only on the data write. This changes the I2C transaction sequence:
- Old: Write byte_offset with STOP, write data with STOP
- New: Write byte_offset (no STOP), write data with WRITE+STOP (via `txgbe_i2c_stop()`)
**Analysis:** I2C writes to EEPROMs typically follow: START → device_addr+W → ACK → byte_offset → ACK → data → ACK → STOP. Removing STOP from the byte_offset phase may be correct if the hardware requires a combined write transaction, **but this is a functional change to I2C timing that is not explained in the commit message** and could break module writes.
**Impact:** Could cause I2C write failures or data corruption on SFP/QSFP EEPROMs.
**Fix:** Either:
1. Verify via hardware datasheet that single STOP at end is correct, document in commit message
2. Restore `TXGBE_I2CDATA_STOP` on byte_offset write
---
### 3. Removed validation logic without replacement - SFP vendor checking deleted
**Lines 1025-1070 (old code, removed):** The old `txgbe_identify_qsfp_module()` read vendor OUI bytes, checked for Intel vendor, and enforced "allow_any_sfp" policy. This entire block is deleted with no equivalent in the new code.
**Impact:**
- Unsupported vendor modules are now silently accepted (security/quality risk)
- `hw->allow_unsupported_sfp` flag is no longer checked (user configuration ignored)
- Warning messages for untested modules removed (users not informed of risks)
**This is a functional regression unless the validation moved elsewhere** (not evident in this patch).
**Fix:** Restore vendor OUI check or document why it's safe to remove.
---
## Warnings
### 1. `txgbe_identify_qsfp_module()` GPIO config runs unconditionally
**Lines 1098-1099:**
```c
/* config GPIO before read i2c */
wr32(hw, TXGBE_GPIODATA, TXGBE_GPIOBIT_1);
```
This GPIO write now runs **even when media_type is not fiber_qsfp** (the media check is after the GPIO write at lines 1108-1111). The old code had this GPIO write inside the `if (hw->mac.type == txgbe_mac_aml40)` block, after the media check.
**Impact:** Unnecessary hardware access, potential side effects on non-QSFP ports.
**Fix:** Move GPIO config inside the `if (hw->mac.type == txgbe_mac_aml40)` block or add media_type check first.
---
### 2. Missing error propagation - `txgbe_read_i2c_sff8636()` page select failure ignored
**Line 1215:**
```c
hw->phy.write_i2c_byte(hw, TXGBE_SFF_QSFP_PAGE_SELECT,
TXGBE_I2C_EEPROM_DEV_ADDR,
page);
```
The return value from `write_i2c_byte()` is discarded. If page select fails, the subsequent read at line 1218 will read from the **wrong page**, producing incorrect module data.
**Fix:**
```c
s32 err = hw->phy.write_i2c_byte(hw, TXGBE_SFF_QSFP_PAGE_SELECT,
TXGBE_I2C_EEPROM_DEV_ADDR, page);
if (err != 0)
return err;
```
---
### 3. Implicit comparison on pointer
**Line 1123 (in `txgbe_identify_qsfp_module`):**
```c
if (err)
return -EBUSY;
```
Should be explicit:
```c
if (err != 0)
return -EBUSY;
```
---
### 4. Release notes likely required
This patch changes module detection logic and affects supported module types. API behavior changes (vendor validation removed, new module type support) typically require release notes under "Fixed Issues" or "Known Issues" if compatibility changes exist.
---
### 5. Function pointer assignments removed but functions still defined
**Lines 2912-2913 (txgbe_hw.c):**
```c
phy->read_i2c_byte_unlocked = txgbe_read_i2c_byte_unlocked;
phy->write_i2c_byte_unlocked = txgbe_write_i2c_byte_unlocked;
```
These assignments are removed, but the commit message doesn't indicate whether the `*_unlocked` functions are deleted or just unused. If the functions remain defined but unreferenced, they are dead code. If other drivers reference `phy->read_i2c_byte_unlocked` as a function pointer, this breaks them.
---
## Info
### 1. Simplified I2C locking approach is clearer
The removal of separate `*_unlocked` variants and the move to caller-managed locking (acquire once, multiple reads, release) is a good pattern that reduces lock churn.
---
### 2. New SFP type handling for 25G/40G modules
The patch adds proper detection for:
- 25G AOC modules (`txgbe_sfp_type_25g_aoc_core0/1`)
- 40G QSFP types with Ethernet compliance byte checks
This appears to address the "ambiguous classification" mentioned in the commit message.
---
### 3. Consistent use of `read_i2c_sff8636()` helper
The new `txgbe_read_i2c_sff8636()` helper centralizes page-select logic for QSFP modules, which is good for maintainability (despite the error handling issue noted above).
---
## Final Check Summary
**Errors (must fix):**
1. Lock leaks in `txgbe_identify_sfp_module()` on I2C read errors after line 836
2. `TXGBE_I2CDATA_STOP` removal in `txgbe_write_i2c_byte()` changes I2C protocol, not explained/verified
3. Vendor OUI validation removed without replacement - security/quality regression
**Warnings (should fix):**
1. GPIO config runs before media_type check
2. Page select error ignored in `txgbe_read_i2c_sff8636()`
3. Implicit comparison `if (err)`
4. Release notes likely needed for behavior changes
5. Unused function pointer assignments removed - check for dead code
^ permalink raw reply
* Re: [PATCH v4 20/20] net/txgbe: fix to enable Tx desc check
From: Stephen Hemminger @ 2026-05-17 23:55 UTC (permalink / raw)
To: Zaiyu Wang; +Cc: dev, stable, Jiawen Wu
In-Reply-To: <20260511103604.19724-21-zaiyuwang@trustnetic.com>
On Mon, 11 May 2026 18:36:02 +0800
Zaiyu Wang <zaiyuwang@trustnetic.com> wrote:
> Now lib security is enabled by default, and cannot be disabled if the
> driver is intended to be used. So Tdm_desc_chk is always unable to enable.
> Remove this restriction, and just enable the corresponding queue check.
>
> Fixes: 0eabdfcd4af4 ("net/txgbe: enable Tx descriptor error interrupt")
> Cc: stable@dpdk.org
>
> Signed-off-by: Zaiyu Wang <zaiyuwang@trustnetic.com>
> ---
Minor AI stuff here:
# Review of DPDK Patch: net/txgbe: fix to enable Tx desc check
## Summary
This patch modifies the Tx descriptor check logic in the txgbe driver to work correctly when security features are enabled. The change moves descriptor check setup from a global per-device operation to a per-queue operation.
## ERRORS
None found.
## WARNINGS
### 1. Bit manipulation should use RTE_BIT32() macro
**File:** drivers/net/txgbe/txgbe_rxtx.c
**Lines:** 4774, 4775
The code uses `BIT()` macro for bitmask generation. DPDK coding standards prefer `RTE_BIT32()` or `RTE_BIT64()` for clarity and portability.
**Current code:**
```c
wr32m(hw, TXGBE_TDM_DESC_CHK(txq->reg_idx / 32),
BIT(txq->reg_idx % 32), BIT(txq->reg_idx % 32));
```
**Suggested fix:**
```c
wr32m(hw, TXGBE_TDM_DESC_CHK(txq->reg_idx / 32),
RTE_BIT32(txq->reg_idx % 32), RTE_BIT32(txq->reg_idx % 32));
```
Verify that `BIT()` is defined in the driver's headers. If it's a legacy macro, consider using the DPDK standard `RTE_BIT32()` for consistency.
### 2. Conditional compilation without corresponding documentation update
**Context:** The patch changes the logic for when descriptor checks are enabled but there's no indication of release notes or documentation updates.
The original code disabled all descriptor checks when `RTE_LIB_SECURITY` was not defined. The new code enables descriptor checks per-queue unless the queue is using IPsec. This is a functional behavior change that should be documented.
**Suggested action:**
- Update release notes to describe the change in Tx descriptor check behavior
- Document that descriptor checks are now enabled per-queue based on IPsec usage rather than globally based on library compilation
## INFO
### 1. Potential logic optimization
The same bitmask value `BIT(txq->reg_idx % 32)` is used twice in the `wr32m()` call. Consider extracting it to a local variable for clarity:
```c
uint32_t queue_bit = BIT(txq->reg_idx % 32);
wr32m(hw, TXGBE_TDM_DESC_CHK(txq->reg_idx / 32), queue_bit, queue_bit);
```
This makes the intent clearer (setting the bit to 1) and slightly improves readability.
### 2. Parentheses around `using_ipsec` unnecessary
**Line:** 4772
```c
if (!(txq->using_ipsec))
```
Could be written as:
```c
if (!txq->using_ipsec)
```
The extra parentheses around `using_ipsec` are unnecessary since `!` has higher precedence than most operators.
---
## OVERALL ASSESSMENT
The patch correctly addresses the issue where descriptor checks could not be enabled when the security library was compiled in. The logic change from global enable/disable to per-queue configuration based on IPsec usage is sound.
**Key improvements made:**
- Moves descriptor check setup into the per-queue loop
- Only disables checks for queues actually using IPsec
- Removes the blanket disable when security library is present
**Correctness:** No bugs detected. The resource management and control flow are correct.
**Style compliance:** Minor issues with macro usage; otherwise compliant.
^ permalink raw reply
* Re: [PATCH v2 0/6] net/gve: add hardware timestamping support
From: Stephen Hemminger @ 2026-05-17 23:15 UTC (permalink / raw)
To: Mark Blasko; +Cc: dev
In-Reply-To: <20260515231936.3296603-1-blasko@google.com>
On Fri, 15 May 2026 23:19:29 +0000
Mark Blasko <blasko@google.com> wrote:
> This patch series introduces support for GVE hardware timestamping on DQO
> queues. To support concurrent access, a mutex lock is introduced to protect
> admin queue operations. A mechanism is then added to periodically synchronize
> the NIC clock via AdminQ, and support is introduced for the read_clock ethdev
> operation. Finally, the RX datapath is updated to reconstruct full 64-bit
> timestamps from the 32-bit values in DQO descriptors.
>
> ---
> v2:
> - Patch 1: Dropped ROBUST mutex attribute.
> - Patch 3: Added adminq timestamp counter reset to gve_adminq_alloc.
> - Patch 4:
> - Removed redundant void* casts.
> - Handled alarm reschedule failures by marking timestamp stale.
> - Added transient error logging on memzone allocation failure.
> - Patch 5: Scoped read_clock ethdev operation strictly to DQO queues.
> - Patch 6:
> - Scoped timestamp offload capability advertisement strictly to
> DQO queues.
> - Predicated capability advertisement directly on memzone
> allocation.
> - Initialized mbuf_timestamp_offset to -1.
> - Added blank line separating release notes.
> ---
>
> Mark Blasko (6):
> net/gve: add thread safety to admin queue
> net/gve: add device option support for HW timestamps
> net/gve: add AdminQ command for NIC timestamps
> net/gve: add periodic NIC clock synchronization
> net/gve: support read clock ethdev op
> net/gve: reconstruct HW timestamps from DQO
>
> .mailmap | 1 +
> doc/guides/nics/features/gve.ini | 1 +
> doc/guides/nics/gve.rst | 20 ++++
> doc/guides/rel_notes/release_26_07.rst | 4 +
> drivers/net/gve/base/gve_adminq.c | 128 +++++++++++++++++----
> drivers/net/gve/base/gve_adminq.h | 29 +++++
> drivers/net/gve/base/gve_desc_dqo.h | 8 +-
> drivers/net/gve/gve_ethdev.c | 149 ++++++++++++++++++++++++-
> drivers/net/gve/gve_ethdev.h | 39 +++++++
> drivers/net/gve/gve_rx_dqo.c | 26 +++++
> 10 files changed, 383 insertions(+), 22 deletions(-)
>
Looks good, once again AI followup still found some things you need to address.
Patch 5: net/gve: support read clock ethdev op
Error: gve_read_clock is defined as static but never assigned to
any eth_dev_ops table. In v1 the patch added
".read_clock = gve_read_clock" to both gve_eth_dev_ops and
gve_eth_dev_ops_dqo. The v2 changelog notes "Scoped read_clock
ethdev operation strictly to DQO queues," which should have left
the assignment in gve_eth_dev_ops_dqo and removed it from
gve_eth_dev_ops. Instead both assignments were dropped, leaving
the function unreferenced. DPDK CI builds with -Dwerror=true, so
-Wunused-function will fail the build, and the read_clock feature
is unreachable at runtime in any case.
Restore the assignment in gve_eth_dev_ops_dqo:
static const struct eth_dev_ops gve_eth_dev_ops_dqo = {
...
.reta_query = gve_rss_reta_query,
.read_clock = gve_read_clock,
};
Warning: (unchanged from v1) gve_read_clock and the periodic
gve_read_nic_clock alarm callback both issue
GVE_ADMINQ_REPORT_NIC_TIMESTAMP into the single shared DMA buffer
priv->nic_ts_report, then read it after gve_adminq_execute_cmd
has released adminq_lock. If gve_read_clock is preempted between
gve_adminq_report_nic_timestamp returning and the be64_to_cpu
read, the alarm callback can memset() and reissue its own
command, so the user thread will read either zero or another
command's response. The simplest fix is for gve_read_clock to
return the cached priv->last_read_nic_timestamp instead of
issuing a fresh adminq command - the 250ms periodic sync keeps
it fresh enough for .read_clock semantics. Once the dev_op
registration is restored this race becomes reachable.
^ permalink raw reply
* Re: [PATCH] devtools: fix SPDX tag check
From: Thomas Monjalon @ 2026-05-17 21:34 UTC (permalink / raw)
To: David Marchand; +Cc: dev, stable, Richardson, Bruce, Hemant Agrawal
In-Reply-To: <CAJFAV8yAP34iRGiGKx0+Ls6_FpSa0EubP3KiDrUEU0fY-tp0FQ@mail.gmail.com>
>> If a file has no SPDX tag and is not filtered out by no_license_list,
>> there will be an error when using its path containing a slash
>> in the sed command delimited with slashes.
>>
>> It is fixed by using the pipe character as sed command delimiter.
>>
>> Fixes: b99a3b8aa989 ("license: standardize SPDX tag")
>> Cc: stable@dpdk.org
>>
>> Reported-by: David Marchand <david.marchand@redhat.com>
>> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
>
> Acked-by: David Marchand <david.marchand@redhat.com>
Applied
^ permalink raw reply
* Re: [PATCH v2] devtools: fix SPDX tag check
From: Thomas Monjalon @ 2026-05-17 20:26 UTC (permalink / raw)
To: Marat Khalili
Cc: dev@dpdk.org, stable@dpdk.org, David Marchand, Bruce Richardson,
Hemant Agrawal
In-Reply-To: <67049e0367f2462bade5c2fe837e8173@huawei.com>
01/05/2026 11:55, Marat Khalili:
> > Sorry I don't understand the need for a sentinel.
> > The script is working fine with an empty files_without_spdx.
>
> It may filter out everything instead of nothing in that case.
OK you're right.
So it becomes complex.
The first solution with sed looks better.
^ permalink raw reply
* [PATCH v2 2/2] dma/dpaa2: fix dpaa2_qdma_remove always returning success
From: Md Shofiqul Islam @ 2026-05-16 11:08 UTC (permalink / raw)
To: dev; +Cc: hemant.agrawal, stable
In-Reply-To: <20260516110828.35701-1-shofiqtest@gmail.com>
dpaa2_qdma_remove() checked the return value of rte_dma_pmd_release()
and logged an error on failure, but then unconditionally returned 0,
hiding the failure from fslmc_bus_unplug() and any higher-level
caller. A device-cleanup failure was silently swallowed.
Return the actual error code so that callers can detect and handle
removal failures correctly.
Fixes: 8caf8427f85a ("dma/dpaa2: introduce driver skeleton")
Bugzilla ID: 1914
Cc: hemant.agrawal@nxp.com
Cc: stable@dpdk.org
Signed-off-by: Md Shofiqul Islam <shofiqtest@gmail.com>
---
drivers/dma/dpaa2/dpaa2_qdma.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/dma/dpaa2/dpaa2_qdma.c b/drivers/dma/dpaa2/dpaa2_qdma.c
index beca464c72..f7d94bb799 100644
--- a/drivers/dma/dpaa2/dpaa2_qdma.c
+++ b/drivers/dma/dpaa2/dpaa2_qdma.c
@@ -1739,7 +1739,7 @@ dpaa2_qdma_remove(struct rte_dpaa2_device *dpaa2_dev)
if (ret)
DPAA2_QDMA_ERR("Device cleanup failed");
- return 0;
+ return ret;
}
static struct rte_dpaa2_driver rte_dpaa2_qdma_pmd;
--
2.51.1
^ permalink raw reply related
* [PATCH v2 1/2] bus/fslmc: fix ignored return value in fslmc_bus_unplug
From: Md Shofiqul Islam @ 2026-05-16 11:08 UTC (permalink / raw)
To: dev; +Cc: hemant.agrawal, stable
In-Reply-To: <20260516110828.35701-1-shofiqtest@gmail.com>
fslmc_bus_unplug() called drv->remove() and discarded the return value,
unconditionally clearing driver references and reporting success even
when the remove callback signalled failure. As a result, callers had
no way to detect or react to removal errors.
Capture the return value and propagate it to the caller. Only clear
the driver references and log successful unplug when the callback
returns zero.
Fixes: b5721f271cbf ("bus/fslmc: support DPNI hotplug")
Bugzilla ID: 1914
Cc: hemant.agrawal@nxp.com
Cc: stable@dpdk.org
Signed-off-by: Md Shofiqul Islam <shofiqtest@gmail.com>
---
drivers/bus/fslmc/fslmc_bus.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/bus/fslmc/fslmc_bus.c b/drivers/bus/fslmc/fslmc_bus.c
index cf881b3eec..57bb61dc35 100644
--- a/drivers/bus/fslmc/fslmc_bus.c
+++ b/drivers/bus/fslmc/fslmc_bus.c
@@ -620,7 +620,9 @@ fslmc_bus_unplug(struct rte_device *rte_dev)
struct rte_dpaa2_driver *drv = dev->driver;
if (drv && drv->remove) {
- drv->remove(dev);
+ int ret = drv->remove(dev);
+ if (ret != 0)
+ return ret;
dev->driver = NULL;
dev->device.driver = NULL;
DPAA2_BUS_INFO("%s Un-Plugged", dev->device.name);
--
2.51.1
^ permalink raw reply related
* [PATCH v2 0/2] dpaa2: fix error propagation in remove path
From: Md Shofiqul Islam @ 2026-05-16 11:08 UTC (permalink / raw)
To: dev; +Cc: hemant.agrawal, stable
This series fixes two bugs in the dpaa2/fslmc device removal path where
error codes were silently discarded, making it impossible for callers to
detect removal failures.
Patch 1 fixes fslmc_bus_unplug() to capture and propagate the return
value of drv->remove() instead of ignoring it.
Patch 2 fixes dpaa2_qdma_remove() to return the actual error code from
rte_dma_pmd_release() instead of unconditionally returning 0.
v2:
- bus/fslmc: use 'if (ret != 0)' style per DPDK coding standard
Md Shofiqul Islam (2):
bus/fslmc: fix ignored return value in fslmc_bus_unplug
dma/dpaa2: fix dpaa2_qdma_remove always returning success
drivers/bus/fslmc/fslmc_bus.c | 4 +++-
drivers/dma/dpaa2/dpaa2_qdma.c | 2 +-
2 files changed, 4 insertions(+), 2 deletions(-)
--
2.51.1
^ permalink raw reply
* Re: [PATCH 1/1] crypto/zsda: Update product name and maintainer
From: Thomas Monjalon @ 2026-05-17 10:01 UTC (permalink / raw)
To: Chengfei Han; +Cc: dev, ran.ming
In-Reply-To: <20260515081212.3565572-2-han.chengfei@zte.com.cn>
15/05/2026 10:12, Chengfei Han:
> Correct product name from '1cf2' to 'Neo X510/X512' in PMD documentation.
> Update maintainer for ZTE Storage Data Accelerator (ZSDA)
>
> Signed-off-by: Chengfei Han <han.chengfei@zte.com.cn>
> ---
> ZTE Storage Data Accelerator (ZSDA)
> -M: Hanxiao Li <li.hanxiao@zte.com.cn>
> +M: Chengfei Han <han.chengfei@zte.com.cn>
> +M: Ming Ran <ran.ming@zte.com.cn>
> F: drivers/common/zsda/
> F: drivers/compress/zsda/
I suppose you wanted to apply this change to the crypto driver as well,
not only for the compress driver.
Applied with assumed change.
^ permalink raw reply
* Re: [PATCH v1 1/2] net/zxdh: add supported zxdh support nics
From: Thomas Monjalon @ 2026-05-17 9:39 UTC (permalink / raw)
To: Junlong Wang; +Cc: stephen, ran.ming, dev
In-Reply-To: <20260514015650.3500578-1-wang.junlong1@zte.com.cn>
14/05/2026 03:56, Junlong Wang:
> update zxdh nics doc.rst, add zxdh support nics.
>
> Signed-off-by: Junlong Wang <wang.junlong1@zte.com.cn>
Series applies with fixes mentioned in the mail thread.
^ permalink raw reply
* Re: [PATCH v1 1/2] net/zxdh: add supported zxdh support nics
From: Thomas Monjalon @ 2026-05-17 9:38 UTC (permalink / raw)
To: Junlong Wang; +Cc: stephen, dev, ran.ming
In-Reply-To: <20260514015650.3500578-1-wang.junlong1@zte.com.cn>
14/05/2026 03:56, Junlong Wang:
> update zxdh nics doc.rst, add zxdh support nics.
There is no such file doc.rst
[...]
> +Supported NICs
> +---------------------------
Underlining is too long
> +
> +- ZXDH E310 25 Gigabit Ethernet Controller
> +- ZXDH E312 100 Gigabit Ethernet Controller
> +- ZXDH E312S 100 Gigabit Ethernet Controller
> +- ZXDH E316 200 Gigabit Ethernet Controller
^ permalink raw reply
* Re: [PATCH v1 2/2] net/zxdh: update zxdh Maintainers eamil
From: Thomas Monjalon @ 2026-05-17 9:36 UTC (permalink / raw)
To: Junlong Wang; +Cc: stephen, dev, ran.ming, dev
In-Reply-To: <20260514015650.3500578-2-wang.junlong1@zte.com.cn>
14/05/2026 03:56, Junlong Wang:
> update zxdh Maintainers eamil.
>
> Signed-off-by: Junlong Wang <wang.junlong1@zte.com.cn>
> ---
> ZTE zxdh
> M: Junlong Wang <wang.junlong1@zte.com.cn>
> -M: Lijie Shan <shan.lijie@zte.com.cn>
> +M: Ming Ran <ran.ming@zte.com.cn>
He must be added in .mailmap.
^ permalink raw reply
* [PATCH] net/ice: improve log messages for DDP loading
From: David Marchand @ 2026-05-16 10:19 UTC (permalink / raw)
To: dev; +Cc: patrick.mahan, Bruce Richardson, Anatoly Burakov
Some nics may not provide a serial number (PCI capability
RTE_PCI_EXT_CAP_ID_DSN).
This results in a confusing ERROR log:
ICE_INIT: ice_dev_init(): Failed to read device serial number
This is confusing as DDP loading does *not* require the serial number to
be present for the port to be functional afterwards.
Besides, after trying various path, if the default DDP is not present on
the runtime system, the port initialisation ends up with a vague error:
ICE_INIT: ice_load_pkg(): failed to search file path
Improve the situation with adjusting the log level when reading the
SN fails, then add more debug context to DDP file loading and end up
with a ERROR log mentioning the expected file.
ICE_INIT: ice_firmware_read(): Cannot read DDP file
/lib/firmware/updates/intel/ice/ddp/ice-b49691ffffe6e69c.pkg
ICE_INIT: ice_firmware_read(): Cannot read DDP file
/lib/firmware/intel/ice/ddp/ice-b49691ffffe6e69c.pkg
ICE_INIT: ice_firmware_read(): Cannot read DDP file
/lib/firmware/updates/intel/ice/ddp/ice.pkg
ICE_INIT: ice_firmware_read(): Cannot read DDP file
/lib/firmware/intel/ice/ddp/ice.pkg
ICE_INIT: ice_load_pkg(): Failed to load default DDP package
/lib/firmware/intel/ice/ddp/ice.pkg
Signed-off-by: David Marchand <david.marchand@redhat.com>
---
drivers/net/intel/ice/ice_ethdev.c | 31 ++++++++++++++++++++----------
1 file changed, 21 insertions(+), 10 deletions(-)
diff --git a/drivers/net/intel/ice/ice_ethdev.c b/drivers/net/intel/ice/ice_ethdev.c
index 0f2e7aee14..e065581ccf 100644
--- a/drivers/net/intel/ice/ice_ethdev.c
+++ b/drivers/net/intel/ice/ice_ethdev.c
@@ -2003,6 +2003,17 @@ static int ice_read_customized_path(char *pkg_file, uint16_t buff_len)
return n;
}
+static int
+ice_firmware_read(const char *file, void *buf, size_t *bufsz)
+{
+ int ret = rte_firmware_read(file, buf, bufsz);
+
+ if (ret < 0)
+ PMD_INIT_LOG(DEBUG, "Cannot read DDP file %s", file);
+
+ return ret;
+}
+
int ice_load_pkg(struct ice_adapter *adapter, bool use_dsn, uint64_t dsn)
{
struct ice_hw *hw = &adapter->hw;
@@ -2016,7 +2027,7 @@ int ice_load_pkg(struct ice_adapter *adapter, bool use_dsn, uint64_t dsn)
/* first read any explicitly referenced DDP file*/
if (adapter->devargs.ddp_filename != NULL) {
strlcpy(pkg_file, adapter->devargs.ddp_filename, sizeof(pkg_file));
- if (rte_firmware_read(pkg_file, &buf, &bufsz) == 0) {
+ if (ice_firmware_read(pkg_file, &buf, &bufsz) == 0) {
goto load_fw;
} else {
PMD_INIT_LOG(ERR, "Cannot load DDP file: %s", pkg_file);
@@ -2032,11 +2043,11 @@ int ice_load_pkg(struct ice_adapter *adapter, bool use_dsn, uint64_t dsn)
if (use_dsn) {
snprintf(pkg_file, RTE_DIM(pkg_file), "%s/%s",
customized_path, opt_ddp_filename);
- if (rte_firmware_read(pkg_file, &buf, &bufsz) == 0)
+ if (ice_firmware_read(pkg_file, &buf, &bufsz) == 0)
goto load_fw;
}
snprintf(pkg_file, RTE_DIM(pkg_file), "%s/%s", customized_path, "ice.pkg");
- if (rte_firmware_read(pkg_file, &buf, &bufsz) == 0)
+ if (ice_firmware_read(pkg_file, &buf, &bufsz) == 0)
goto load_fw;
}
@@ -2046,23 +2057,23 @@ int ice_load_pkg(struct ice_adapter *adapter, bool use_dsn, uint64_t dsn)
strncpy(pkg_file, ICE_PKG_FILE_SEARCH_PATH_UPDATES,
ICE_MAX_PKG_FILENAME_SIZE);
strcat(pkg_file, opt_ddp_filename);
- if (rte_firmware_read(pkg_file, &buf, &bufsz) == 0)
+ if (ice_firmware_read(pkg_file, &buf, &bufsz) == 0)
goto load_fw;
strncpy(pkg_file, ICE_PKG_FILE_SEARCH_PATH_DEFAULT,
ICE_MAX_PKG_FILENAME_SIZE);
strcat(pkg_file, opt_ddp_filename);
- if (rte_firmware_read(pkg_file, &buf, &bufsz) == 0)
+ if (ice_firmware_read(pkg_file, &buf, &bufsz) == 0)
goto load_fw;
no_dsn:
strncpy(pkg_file, ICE_PKG_FILE_UPDATES, ICE_MAX_PKG_FILENAME_SIZE);
- if (rte_firmware_read(pkg_file, &buf, &bufsz) == 0)
+ if (ice_firmware_read(pkg_file, &buf, &bufsz) == 0)
goto load_fw;
strncpy(pkg_file, ICE_PKG_FILE_DEFAULT, ICE_MAX_PKG_FILENAME_SIZE);
- if (rte_firmware_read(pkg_file, &buf, &bufsz) < 0) {
- PMD_INIT_LOG(ERR, "failed to search file path");
+ if (ice_firmware_read(pkg_file, &buf, &bufsz) < 0) {
+ PMD_INIT_LOG(ERR, "Failed to load default DDP package " ICE_PKG_FILE_DEFAULT);
return -1;
}
@@ -2658,13 +2669,13 @@ ice_dev_init(struct rte_eth_dev *dev)
if (pos) {
if (rte_pci_read_config(pci_dev, &dsn_low, 4, pos + 4) < 0 ||
rte_pci_read_config(pci_dev, &dsn_high, 4, pos + 8) < 0) {
- PMD_INIT_LOG(ERR, "Failed to read pci config space");
+ PMD_INIT_LOG(WARNING, "Failed to read pci config space");
} else {
use_dsn = true;
dsn = (uint64_t)dsn_high << 32 | dsn_low;
}
} else {
- PMD_INIT_LOG(ERR, "Failed to read device serial number");
+ PMD_INIT_LOG(INFO, "Failed to read device serial number");
}
ret = ice_load_pkg(pf->adapter, use_dsn, dsn);
--
2.53.0
^ permalink raw reply related
* [PATCH 1/1] crypto/zsda: Update product name and maintainer
From: Chengfei Han @ 2026-05-15 7:27 UTC (permalink / raw)
To: dev; +Cc: ran.ming, Chengfei Han
In-Reply-To: <20260515072725.3563941-1-han.chengfei@zte.com.cn>
[-- Attachment #1.1.1: Type: text/plain, Size: 1826 bytes --]
Correct product name from '1cf2' to 'Neo X510/X512' in PMD documentation.
Update maintainer for ZTE Storage Data Accelerator (ZSDA)
Signed-off-by: Chengfei Han <han.chengfei@zte.com.cn>
---
MAINTAINERS | 3 ++-
doc/guides/compressdevs/zsda.rst | 3 +--
doc/guides/cryptodevs/zsda.rst | 3 +--
3 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 0f5539f851..7f30205b24 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1352,7 +1352,8 @@ F: doc/guides/compressdevs/zlib.rst
F: doc/guides/compressdevs/features/zlib.ini
ZTE Storage Data Accelerator (ZSDA)
-M: Hanxiao Li <li.hanxiao@zte.com.cn>
+M: Chengfei Han <han.chengfei@zte.com.cn>
+M: Ming Ran <ran.ming@zte.com.cn>
F: drivers/common/zsda/
F: drivers/compress/zsda/
F: doc/guides/compressdevs/zsda.rst
diff --git a/doc/guides/compressdevs/zsda.rst b/doc/guides/compressdevs/zsda.rst
index 25b7884535..5b8d4846a0 100644
--- a/doc/guides/compressdevs/zsda.rst
+++ b/doc/guides/compressdevs/zsda.rst
@@ -7,8 +7,7 @@ ZTE Storage Data Accelerator (ZSDA) Poll Mode Driver
The ZSDA compression PMD provides poll mode compression & decompression driver
support for the following hardware accelerator devices:
-* ZTE Processing accelerators 1cf2
-
+- Neo X510/X512
Features
--------
diff --git a/doc/guides/cryptodevs/zsda.rst b/doc/guides/cryptodevs/zsda.rst
index b024f537c1..8885663e9e 100644
--- a/doc/guides/cryptodevs/zsda.rst
+++ b/doc/guides/cryptodevs/zsda.rst
@@ -7,8 +7,7 @@ ZTE Storage Data Accelerator (ZSDA) Poll Mode Driver
The ZSDA crypto PMD provides poll mode Cipher and Hash driver
support for the following hardware accelerator devices:
-* ZTE Processing accelerators 1cf2
-
+- Neo X510/X512
Features
--------
--
2.27.0
[-- Attachment #1.1.2: Type: text/html , Size: 3054 bytes --]
^ permalink raw reply related
* [PATCH 0/1] crypto/zsda: update product name and maintainer
From: Chengfei Han @ 2026-05-15 7:27 UTC (permalink / raw)
To: dev; +Cc: ran.ming, Chengfei Han
[-- Attachment #1.1.1: Type: text/plain, Size: 463 bytes --]
*** BLURB HERE ***
This patch updates the ZSDA PMD driver:
- Correct product name from '1cf2' to 'Neo X510/X512' in documentation
- Update maintainer for ZTE Storage Data Accelerator (ZSDA)
Chengfei Han (1):
crypto/zsda: Update product name and maintainer
MAINTAINERS | 3 ++-
doc/guides/compressdevs/zsda.rst | 3 +--
doc/guides/cryptodevs/zsda.rst | 3 +--
3 files changed, 4 insertions(+), 5 deletions(-)
--
2.27.0
[-- Attachment #1.1.2: Type: text/html , Size: 949 bytes --]
^ permalink raw reply
* [PATCH 2/2] dma/dpaa2: fix dpaa2_qdma_remove always returning success
From: Md Shofiqul Islam @ 2026-05-13 20:37 UTC (permalink / raw)
To: dev; +Cc: hemant.agrawal, sachin.saxena, g.singh, stable
In-Reply-To: <20260513203725.1905-1-shofiqtest@gmail.com>
dpaa2_qdma_remove() checked the return value of rte_dma_pmd_release()
and logged an error on failure, but then unconditionally returned 0,
hiding the failure from fslmc_bus_unplug() and any higher-level
caller. A device-cleanup failure was silently swallowed.
Return the actual error code so that callers can detect and handle
removal failures correctly.
Fixes: 8caf8427f85a ("dma/dpaa2: introduce driver skeleton")
Bugzilla ID: 1914
Cc: hemant.agrawal@nxp.com
Cc: stable@dpdk.org
Signed-off-by: Md Shofiqul Islam <shofiqtest@gmail.com>
---
drivers/dma/dpaa2/dpaa2_qdma.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/dma/dpaa2/dpaa2_qdma.c b/drivers/dma/dpaa2/dpaa2_qdma.c
index beca464c72..f7d94bb799 100644
--- a/drivers/dma/dpaa2/dpaa2_qdma.c
+++ b/drivers/dma/dpaa2/dpaa2_qdma.c
@@ -1739,7 +1739,7 @@ dpaa2_qdma_remove(struct rte_dpaa2_device *dpaa2_dev)
if (ret)
DPAA2_QDMA_ERR("Device cleanup failed");
- return 0;
+ return ret;
}
static struct rte_dpaa2_driver rte_dpaa2_qdma_pmd;
--
2.54.0.windows.1
^ permalink raw reply related
* [PATCH 1/2] bus/fslmc: fix ignored return value in fslmc_bus_unplug
From: Md Shofiqul Islam @ 2026-05-13 20:37 UTC (permalink / raw)
To: dev; +Cc: hemant.agrawal, sachin.saxena, g.singh, stable
In-Reply-To: <20260513203725.1905-1-shofiqtest@gmail.com>
fslmc_bus_unplug() called drv->remove() and discarded the return value,
unconditionally clearing driver references and reporting success even
when the remove callback signalled failure. As a result, callers had
no way to detect or react to removal errors.
Capture the return value and propagate it to the caller. Only clear
the driver references and log successful unplug when the callback
returns zero.
Fixes: b5721f271cbf ("bus/fslmc: support DPNI hotplug")
Bugzilla ID: 1914
Cc: hemant.agrawal@nxp.com
Cc: stable@dpdk.org
Signed-off-by: Md Shofiqul Islam <shofiqtest@gmail.com>
---
drivers/bus/fslmc/fslmc_bus.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/bus/fslmc/fslmc_bus.c b/drivers/bus/fslmc/fslmc_bus.c
index cf881b3eec..9cfd8b10ba 100644
--- a/drivers/bus/fslmc/fslmc_bus.c
+++ b/drivers/bus/fslmc/fslmc_bus.c
@@ -620,7 +620,9 @@ fslmc_bus_unplug(struct rte_device *rte_dev)
struct rte_dpaa2_driver *drv = dev->driver;
if (drv && drv->remove) {
- drv->remove(dev);
+ int ret = drv->remove(dev);
+ if (ret)
+ return ret;
dev->driver = NULL;
dev->device.driver = NULL;
DPAA2_BUS_INFO("%s Un-Plugged", dev->device.name);
--
2.54.0.windows.1
^ permalink raw reply related
* [PATCH 0/2] bus/fslmc, dma/dpaa2: fix remove callback error propagation
From: Md Shofiqul Islam @ 2026-05-13 20:37 UTC (permalink / raw)
To: dev; +Cc: hemant.agrawal, sachin.saxena, g.singh, stable
Two dpaa2 drivers silently discard errors from their remove callbacks,
making it impossible for callers to detect cleanup failures:
- fslmc_bus_unplug() (bus/fslmc) calls drv->remove() and throws away
the return value, reporting success regardless of outcome.
- dpaa2_qdma_remove() (dma/dpaa2) logs a cleanup error but always
returns 0, hiding the failure from its caller.
This series fixes both by propagating the actual return values.
Bugzilla ID: 1914
Md Shofiqul Islam (2):
bus/fslmc: fix ignored return value in fslmc_bus_unplug
dma/dpaa2: fix dpaa2_qdma_remove always returning success
drivers/bus/fslmc/fslmc_bus.c | 4 +++-
drivers/dma/dpaa2/dpaa2_qdma.c | 2 +-
2 files changed, 4 insertions(+), 2 deletions(-)
--
2.54.0.windows.1
^ permalink raw reply
* [PATCH v15 00/11] net/sxe2: fix logic errors and address feedback
From: liujie5 @ 2026-05-16 7:46 UTC (permalink / raw)
To: stephen; +Cc: dev, Jie Liu
In-Reply-To: <20260516025540.2092621-12-liujie5@linkdatatechnology.com>
From: Jie Liu <liujie5@linkdatatechnology.com>
This patch set addresses the feedback received on the v10 submission
for the sxe2 PMD. The primary focus is on fixing vector path selection,
ensuring memory safety during mbuf initialization, and cleaning up
redundant logic in the configuration functions.
v15 Changes:
- Fixed vector Rx burst function being overwritten by scalar selection.
- Refactored Rx/Tx mode set functions to seed flags from caps first,
eliminating tautological checks.
- Added memset for mbuf_def in vector init to avoid uninitialized reads.
- Converted pci_map_addr_info to designated initializers.
- Removed dead Windows-only code in meson.build.
- Added NULL checks for mbuf free for driver-wide consistency.
- Updated burst_mode_get to accurately report AVX paths.
- Adjusted SXE2_ETH_OVERHEAD to match actual VLAN capabilities.
Jie Liu (11):
mailmap: add Jie Liu
doc: add sxe2 guide and release notes
common/sxe2: add sxe2 basic structures
drivers: add base driver skeleton
drivers: add base driver probe skeleton
drivers: support PCI BAR mapping
common/sxe2: add ioctl interface for DMA map and unmap
net/sxe2: support queue setup and control
drivers: add data path for Rx and Tx
net/sxe2: add vectorized Rx and Tx
net/sxe2: implement Tx done cleanup
.mailmap | 1 +
doc/guides/nics/features/sxe2.ini | 29 +
doc/guides/nics/index.rst | 1 +
doc/guides/nics/sxe2.rst | 34 +
doc/guides/rel_notes/release_26_07.rst | 4 +
drivers/common/sxe2/meson.build | 15 +
drivers/common/sxe2/sxe2_common.c | 683 +++++++++++++
drivers/common/sxe2/sxe2_common.h | 85 ++
drivers/common/sxe2/sxe2_common_log.h | 81 ++
drivers/common/sxe2/sxe2_host_regs.h | 707 +++++++++++++
drivers/common/sxe2/sxe2_internal_ver.h | 33 +
drivers/common/sxe2/sxe2_ioctl_chnl.c | 325 ++++++
drivers/common/sxe2/sxe2_ioctl_chnl.h | 130 +++
drivers/common/sxe2/sxe2_ioctl_chnl_func.h | 62 ++
drivers/common/sxe2/sxe2_osal.h | 73 ++
drivers/meson.build | 1 +
drivers/net/meson.build | 1 +
drivers/net/sxe2/meson.build | 32 +
drivers/net/sxe2/sxe2_cmd_chnl.c | 323 ++++++
drivers/net/sxe2/sxe2_cmd_chnl.h | 37 +
drivers/net/sxe2/sxe2_drv_cmd.h | 388 ++++++++
drivers/net/sxe2/sxe2_ethdev.c | 968 ++++++++++++++++++
drivers/net/sxe2/sxe2_ethdev.h | 318 ++++++
drivers/net/sxe2/sxe2_irq.h | 48 +
drivers/net/sxe2/sxe2_queue.c | 66 ++
drivers/net/sxe2/sxe2_queue.h | 195 ++++
drivers/net/sxe2/sxe2_rx.c | 559 +++++++++++
drivers/net/sxe2/sxe2_rx.h | 34 +
drivers/net/sxe2/sxe2_tx.c | 420 ++++++++
drivers/net/sxe2/sxe2_tx.h | 32 +
drivers/net/sxe2/sxe2_txrx.c | 357 +++++++
drivers/net/sxe2/sxe2_txrx.h | 23 +
drivers/net/sxe2/sxe2_txrx_common.h | 540 ++++++++++
drivers/net/sxe2/sxe2_txrx_poll.c | 1045 ++++++++++++++++++++
drivers/net/sxe2/sxe2_txrx_poll.h | 20 +
drivers/net/sxe2/sxe2_txrx_vec.c | 200 ++++
drivers/net/sxe2/sxe2_txrx_vec.h | 73 ++
drivers/net/sxe2/sxe2_txrx_vec_common.h | 235 +++++
drivers/net/sxe2/sxe2_txrx_vec_sse.c | 549 ++++++++++
drivers/net/sxe2/sxe2_vsi.c | 214 ++++
drivers/net/sxe2/sxe2_vsi.h | 204 ++++
41 files changed, 9145 insertions(+)
create mode 100644 doc/guides/nics/features/sxe2.ini
create mode 100644 doc/guides/nics/sxe2.rst
create mode 100644 drivers/common/sxe2/meson.build
create mode 100644 drivers/common/sxe2/sxe2_common.c
create mode 100644 drivers/common/sxe2/sxe2_common.h
create mode 100644 drivers/common/sxe2/sxe2_common_log.h
create mode 100644 drivers/common/sxe2/sxe2_host_regs.h
create mode 100644 drivers/common/sxe2/sxe2_internal_ver.h
create mode 100644 drivers/common/sxe2/sxe2_ioctl_chnl.c
create mode 100644 drivers/common/sxe2/sxe2_ioctl_chnl.h
create mode 100644 drivers/common/sxe2/sxe2_ioctl_chnl_func.h
create mode 100644 drivers/common/sxe2/sxe2_osal.h
create mode 100644 drivers/net/sxe2/meson.build
create mode 100644 drivers/net/sxe2/sxe2_cmd_chnl.c
create mode 100644 drivers/net/sxe2/sxe2_cmd_chnl.h
create mode 100644 drivers/net/sxe2/sxe2_drv_cmd.h
create mode 100644 drivers/net/sxe2/sxe2_ethdev.c
create mode 100644 drivers/net/sxe2/sxe2_ethdev.h
create mode 100644 drivers/net/sxe2/sxe2_irq.h
create mode 100644 drivers/net/sxe2/sxe2_queue.c
create mode 100644 drivers/net/sxe2/sxe2_queue.h
create mode 100644 drivers/net/sxe2/sxe2_rx.c
create mode 100644 drivers/net/sxe2/sxe2_rx.h
create mode 100644 drivers/net/sxe2/sxe2_tx.c
create mode 100644 drivers/net/sxe2/sxe2_tx.h
create mode 100644 drivers/net/sxe2/sxe2_txrx.c
create mode 100644 drivers/net/sxe2/sxe2_txrx.h
create mode 100644 drivers/net/sxe2/sxe2_txrx_common.h
create mode 100644 drivers/net/sxe2/sxe2_txrx_poll.c
create mode 100644 drivers/net/sxe2/sxe2_txrx_poll.h
create mode 100644 drivers/net/sxe2/sxe2_txrx_vec.c
create mode 100644 drivers/net/sxe2/sxe2_txrx_vec.h
create mode 100644 drivers/net/sxe2/sxe2_txrx_vec_common.h
create mode 100644 drivers/net/sxe2/sxe2_txrx_vec_sse.c
create mode 100644 drivers/net/sxe2/sxe2_vsi.c
create mode 100644 drivers/net/sxe2/sxe2_vsi.h
--
2.47.3
^ permalink raw reply
* [PATCH v15 11/11] net/sxe2: implement Tx done cleanup
From: liujie5 @ 2026-05-16 7:46 UTC (permalink / raw)
To: stephen; +Cc: dev, Jie Liu
In-Reply-To: <20260516074618.2343883-1-liujie5@linkdatatechnology.com>
From: Jie Liu <liujie5@linkdatatechnology.com>
This patch implements the 'tx_done_cleanup' ethdev ops in the sxe2
PMD. This interface allows applications to explicitly request the
driver to release mbufs that have been transmitted and are no longer
needed by the hardware.
The implementation iterates through the Tx ring, checking the status
of the descriptors starting from the last cleaned tail. It releases
the corresponding mbufs back to the mempool until either the requested
number of packets are freed or no more completed descriptors are
found.
Signed-off-by: Jie Liu <liujie5@linkdatatechnology.com>
---
drivers/net/sxe2/sxe2_ethdev.c | 1 +
drivers/net/sxe2/sxe2_txrx.h | 1 +
drivers/net/sxe2/sxe2_txrx_poll.c | 101 +++++++++++++++++++++++++++++-
3 files changed, 102 insertions(+), 1 deletion(-)
diff --git a/drivers/net/sxe2/sxe2_ethdev.c b/drivers/net/sxe2/sxe2_ethdev.c
index d1bdc22bd0..8d66e5d8c5 100644
--- a/drivers/net/sxe2/sxe2_ethdev.c
+++ b/drivers/net/sxe2/sxe2_ethdev.c
@@ -290,6 +290,7 @@ static const struct eth_dev_ops sxe2_eth_dev_ops = {
.txq_info_get = sxe2_tx_queue_info_get,
.rx_burst_mode_get = sxe2_rx_burst_mode_get,
.tx_burst_mode_get = sxe2_tx_burst_mode_get,
+ .tx_done_cleanup = sxe2_tx_done_cleanup,
};
struct sxe2_pci_map_bar_info *sxe2_dev_get_bar_info(struct sxe2_adapter *adapter,
diff --git a/drivers/net/sxe2/sxe2_txrx.h b/drivers/net/sxe2/sxe2_txrx.h
index 61c6641e49..6d3d7455c2 100644
--- a/drivers/net/sxe2/sxe2_txrx.h
+++ b/drivers/net/sxe2/sxe2_txrx.h
@@ -12,6 +12,7 @@ int32_t __rte_cold sxe2_tx_simple_batch_support_check(struct rte_eth_dev *dev,
uint32_t *batch_flags);
uint16_t sxe2_tx_pkts_prepare(void *tx_queue,
struct rte_mbuf **tx_pkts, uint16_t nb_pkts);
+int32_t sxe2_tx_done_cleanup(void *txq, uint32_t free_cnt);
void sxe2_tx_mode_func_set(struct rte_eth_dev *dev);
void __rte_cold sxe2_rx_queue_reset(struct sxe2_rx_queue *rxq);
void sxe2_rx_mode_func_set(struct rte_eth_dev *dev);
diff --git a/drivers/net/sxe2/sxe2_txrx_poll.c b/drivers/net/sxe2/sxe2_txrx_poll.c
index dc6a83e380..7302ceb6f5 100644
--- a/drivers/net/sxe2/sxe2_txrx_poll.c
+++ b/drivers/net/sxe2/sxe2_txrx_poll.c
@@ -8,12 +8,13 @@
#include <rte_malloc.h>
#include <rte_memzone.h>
#include <ethdev_driver.h>
-#include <unistd.h>
#include "sxe2_osal.h"
#include "sxe2_txrx_common.h"
+#include "sxe2_txrx_vec_common.h"
#include "sxe2_txrx_poll.h"
#include "sxe2_txrx.h"
+#include "sxe2_txrx_vec.h"
#include "sxe2_queue.h"
#include "sxe2_ethdev.h"
#include "sxe2_common_log.h"
@@ -118,6 +119,104 @@ static inline int32_t sxe2_tx_cleanup(struct sxe2_tx_queue *txq)
return ret;
}
+static int32_t sxe2_tx_done_cleanup_simple(struct sxe2_tx_queue *txq, uint32_t free_cnt)
+{
+ uint32_t free_cnt_align;
+ uint32_t free_cnt_once;
+ uint32_t i;
+
+ if (free_cnt == 0 || free_cnt > txq->ring_depth)
+ free_cnt = txq->ring_depth;
+
+ free_cnt_align = free_cnt - (free_cnt % txq->rs_thresh);
+ for (i = 0; i < free_cnt_align; i += free_cnt_once) {
+ if ((txq->ring_depth - txq->desc_free_num) < txq->rs_thresh)
+ break;
+
+ free_cnt_once = sxe2_tx_bufs_free(txq);
+ if (free_cnt_once == 0)
+ break;
+ }
+
+ return i;
+}
+
+static int32_t sxe2_tx_done_cleanup_normal(struct sxe2_tx_queue *txq, uint32_t free_cnt)
+{
+ struct sxe2_tx_buffer *buffer_ring = txq->buffer_ring;
+ int32_t ret;
+ uint16_t clean_last_idx, clean_idx;
+ uint16_t clean_last, clean_once;
+ uint16_t pkt_cnt, i;
+
+ if (txq->desc_free_num == 0 && sxe2_tx_cleanup(txq) != 0) {
+ ret = 0;
+ goto l_end;
+ }
+
+ if (free_cnt == 0)
+ free_cnt = txq->ring_depth;
+
+ clean_last_idx = txq->next_use;
+ clean_idx = buffer_ring[clean_last_idx].next_id;
+ clean_once = txq->desc_free_num;
+ clean_last = txq->desc_free_num;
+
+ for (pkt_cnt = 0; pkt_cnt < free_cnt;) {
+ for (i = 0; ((i < clean_once) &&
+ (pkt_cnt < free_cnt) &&
+ clean_idx != clean_last_idx); ++i) {
+ if (buffer_ring[clean_idx].mbuf != NULL) {
+ rte_pktmbuf_free_seg(buffer_ring[clean_idx].mbuf);
+ buffer_ring[clean_idx].mbuf = NULL;
+ if (buffer_ring[clean_idx].last_id == clean_idx)
+ pkt_cnt++;
+ }
+ clean_idx = buffer_ring[clean_idx].next_id;
+ }
+
+ if ((txq->rs_thresh > (txq->ring_depth - txq->desc_free_num)) ||
+ clean_idx == clean_last_idx)
+ break;
+
+ if (pkt_cnt < free_cnt) {
+ if (sxe2_tx_cleanup(txq) != 0)
+ break;
+
+ clean_once = txq->desc_free_num - clean_last;
+ clean_last = txq->desc_free_num;
+ }
+ }
+
+ ret = pkt_cnt;
+l_end:
+ return ret;
+}
+
+int32_t sxe2_tx_done_cleanup(void *tx_queue, uint32_t free_cnt)
+{
+ struct sxe2_tx_queue *txq = (struct sxe2_tx_queue *)tx_queue;
+ struct sxe2_adapter *adapter = txq->vsi->adapter;
+ int32_t ret;
+
+ if (txq == NULL) {
+ ret = 0;
+ goto l_end;
+ }
+ if (adapter->q_ctxt.tx_mode_flags & SXE2_TX_MODE_VEC_SET_MASK)
+ ret = -ENOTSUP;
+ else if (adapter->q_ctxt.tx_mode_flags & SXE2_TX_MODE_SIMPLE_BATCH)
+ ret = sxe2_tx_done_cleanup_simple(txq, free_cnt);
+ else
+ ret = sxe2_tx_done_cleanup_normal(txq, free_cnt);
+
+ PMD_LOG_DEBUG(TX, "TX cleanup done desc queue_id=%u free_cnt=%d.",
+ txq->queue_id, ret);
+
+l_end:
+ return ret;
+}
+
static __rte_always_inline uint16_t
sxe2_tx_pkt_data_desc_count(struct rte_mbuf *tx_pkt)
{
--
2.47.3
^ permalink raw reply related
* [PATCH v15 02/11] doc: add sxe2 guide and release notes
From: liujie5 @ 2026-05-16 7:46 UTC (permalink / raw)
To: stephen; +Cc: dev, Jie Liu
In-Reply-To: <20260516074618.2343883-1-liujie5@linkdatatechnology.com>
From: Jie Liu <liujie5@linkdatatechnology.com>
Add a new guide for SXE2 PMD in the nics directory.
The guide contains driver capabilities, prerequisites,
and compilation/usage instructions.
Update the release notes to announce the addition of the
sxe2 network driver.
Signed-off-by: Jie Liu <liujie5@linkdatatechnology.com>
---
doc/guides/nics/features/sxe2.ini | 29 ++++++++++++++++++++++
doc/guides/nics/index.rst | 1 +
doc/guides/nics/sxe2.rst | 34 ++++++++++++++++++++++++++
doc/guides/rel_notes/release_26_07.rst | 4 +++
4 files changed, 68 insertions(+)
create mode 100644 doc/guides/nics/features/sxe2.ini
create mode 100644 doc/guides/nics/sxe2.rst
diff --git a/doc/guides/nics/features/sxe2.ini b/doc/guides/nics/features/sxe2.ini
new file mode 100644
index 0000000000..7e350bab54
--- /dev/null
+++ b/doc/guides/nics/features/sxe2.ini
@@ -0,0 +1,29 @@
+;
+; Supported features of the 'sxe2' network poll mode driver.
+;
+; Refer to default.ini for the full list of available PMD features.
+;
+; A feature with "P" indicates only be supported when non-vector path
+; is selected.
+;
+[Features]
+Fast mbuf free = P
+Free Tx mbuf on demand = Y
+Burst mode info = Y
+Queue start/stop = Y
+Buffer split on Rx = P
+Scattered Rx = Y
+CRC offload = Y
+VLAN offload = Y
+QinQ offload = P
+L3 checksum offload = Y
+L4 checksum offload = Y
+Timestamp offload = P
+Inner L3 checksum = P
+Inner L4 checksum = P
+Rx descriptor status = Y
+Tx descriptor status = Y
+FreeBSD = Y
+Linux = Y
+x86-32 = Y
+x86-64 = Y
diff --git a/doc/guides/nics/index.rst b/doc/guides/nics/index.rst
index cb818284fe..e20be478f8 100644
--- a/doc/guides/nics/index.rst
+++ b/doc/guides/nics/index.rst
@@ -68,6 +68,7 @@ Network Interface Controller Drivers
rnp
sfc_efx
softnic
+ sxe2
tap
thunderx
txgbe
diff --git a/doc/guides/nics/sxe2.rst b/doc/guides/nics/sxe2.rst
new file mode 100644
index 0000000000..7fcf9c085b
--- /dev/null
+++ b/doc/guides/nics/sxe2.rst
@@ -0,0 +1,34 @@
+.. SPDX-License-Identifier: BSD-3-Clause
+ Copyright (C), 2025, Wuxi Stars Micro System Technologies Co., Ltd.
+
+SXE2 Poll Mode Driver
+======================
+
+The sxe2 PMD (**librte_net_sxe2**) provides poll mode driver support for
+10/25/50/100 Gbps Network Adapters.
+The embedded switch, Physical Functions (PF),
+and SR-IOV Virtual Functions (VF) are supported.
+
+Implementation details
+----------------------
+
+The sxe2 PMD is designed to operate alongside the sxe2 kernel network driver.
+For management and control operations, the PMD communicates with the kernel
+driver via ioctl interfaces. These commands are processed by the kernel
+driver and subsequently dispatched to the hardware firmware for execution.
+
+For security and robustness, the driver's data path is optimized to operate
+using virtual addresses (IOVA as VA mode). However, to ensure full
+compatibility in system environments where an IOMMU is absent or disabled,
+the driver also provides an explicit path to support physical addressing
+(IOVA as PA mode).
+
+The hardware is capable of handling the corresponding IOVA addresses (either
+VA or PA) directly, as provided by the DPDK memory subsystem. This ensures
+that DPDK applications can only access memory segments explicitly allocated
+to the current process, preventing unauthorized access to random physical
+memory.
+
+This capability allows the PMD to coexist with kernel network interfaces
+which remain functional, although they stop receiving unicast packets as
+long as they share the same MAC address.
diff --git a/doc/guides/rel_notes/release_26_07.rst b/doc/guides/rel_notes/release_26_07.rst
index f012d47a4b..fa0f0f5cca 100644
--- a/doc/guides/rel_notes/release_26_07.rst
+++ b/doc/guides/rel_notes/release_26_07.rst
@@ -64,6 +64,10 @@ New Features
* ``--auto-probing`` enables the initial bus probing, which is the current default behavior.
+* **Added Linkdata sxe2 ethernet driver.**
+
+ Added network driver for the Linkdata Network Adapters.
+
Removed Items
-------------
--
2.47.3
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox