|SUCCESS| [3/3] net/i40e/base: update version info

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/164153

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-05-19

git sha: 20260519145227.308814-2-ciara.loftus at intel.com
Author: Chinh Cao <chinh.t.cao at intel.com>
Subject: [PATCH 1/3] net/i40e/base: fix integer overflow in NVM timing logic

This patch addresses potential integer overflow issues in NVM timing logic
by renaming timing variables, widening size fields, and introducing a
helper function for wrap-safe timer arithmetic.

> diff --git a/drivers/net/intel/i40e/base/i40e_nvm.c b/drivers/net/intel/i40e/base/i40e_nvm.c
> index 00a207ca81..fd1a987c56 100644
> --- a/drivers/net/intel/i40e/base/i40e_nvm.c
> +++ b/drivers/net/intel/i40e/base/i40e_nvm.c

[ ... ]

> @@ -49,6 +49,20 @@ enum i40e_status_code i40e_init_nvm(struct i40e_hw *hw)
>  	return ret_code;
>  }
>  
> +/**
> + * get_elapsed_time - Compute elapsed hardware timer ticks
> + * @from: The start timer value
> + * @to: The current timer value
> + *
> + * Returns the elapsed time as the difference between the current and start
> + * timer values. The subtraction is wrap-safe for unsigned 32-bit values,
> + * meaning it correctly handles timer wrap-around.
> + **/
> +static u32 get_elapsed_time(u32 from, u32 to)
> +{
> +	return (u32)(to - from); /* wrap-safe */
> +}

Why is this function declared without the i40e_ prefix? The DPDK coding
style requires driver-specific functions to be prefixed with the driver
name to avoid potential namespace conflicts. Should this be named
i40e_get_elapsed_time() instead?

> @@ -61,7 +75,7 @@ enum i40e_status_code i40e_acquire_nvm(struct i40e_hw *hw,
>  				       enum i40e_aq_resource_access_type access)
>  {
>  	enum i40e_status_code ret_code = I40E_SUCCESS;
> -	u32 gtime, timeout;
> +	u32 gtime_start, gtime_current, timeout, elapsed;
>  	u32 time_left = 0;

[ ... ]

> @@ -84,17 +100,20 @@ enum i40e_status_code i40e_acquire_nvm(struct i40e_hw *hw,
>  
>  	if (ret_code && time_left) {
>  		/* Poll until the current NVM owner timeouts */
> -		timeout = I40E_MS_TO_GTIME(I40E_MAX_NVM_TIMEOUT) + gtime;
> -		while ((s32)(gtime - timeout) < 0 && time_left) {
> +		timeout = I40E_MS_TO_GTIME(I40E_MAX_NVM_TIMEOUT);
> +
> +		while (elapsed < timeout && time_left) {
>  			i40e_msec_delay(10);
> -			gtime = rd32(hw, I40E_GLVFGEN_TIMER);
> +			gtime_current = rd32(hw, I40E_GLVFGEN_TIMER);
> +			elapsed = get_elapsed_time(gtime_start, gtime_current);
>  			ret_code = i40e_aq_request_resource(hw,
>  							I40E_NVM_RESOURCE_ID,
>  							access, 0, &time_left,
>  							NULL);
>  			if (ret_code == I40E_SUCCESS) {
>  				hw->nvm.hw_semaphore_timeout =
> -					    I40E_MS_TO_GTIME(time_left) + gtime;
> +					I40E_MS_TO_GTIME(time_left) +
> +					gtime_current;

Does this change properly handle the case where gtime_current wraps
around while the timeout computation doesn't account for it? The old code
stored an absolute deadline (timeout), but the new code compares elapsed
time against a timeout that was computed from I40E_MAX_NVM_TIMEOUT at the
start. If gtime_current wraps, does get_elapsed_time() still correctly
compute elapsed time across the wrap boundary?

[ ... ]

> @@ -516,11 +538,12 @@ i40e_read_nvm_module_data(struct i40e_hw *hw, u8 module_ptr, u16 module_offset,
>   * method. The buffer read is preceded by the NVM ownership take
>   * and followed by the release.
>   **/
> -STATIC enum i40e_status_code i40e_read_nvm_buffer_srctl(struct i40e_hw *hw, u16 offset,
> +STATIC enum i40e_status_code i40e_read_nvm_buffer_srctl(struct i40e_hw *hw, u32 offset,
>  							u16 *words, u16 *data)
>  {
>  	enum i40e_status_code ret_code = I40E_SUCCESS;
> -	u16 index, word;
> +	u32 index;
> +	u16 word;

Does this code properly validate that offset and index remain within
valid hardware register ranges after widening to u32? The hardware may
have addressing constraints that could be violated if offset exceeds u16
range.

> @@ -767,7 +790,7 @@ enum i40e_status_code i40e_calc_nvm_checksum(struct i40e_hw *hw, u16 *checksum)
>  	u16 checksum_local = 0;
>  	u16 vpd_module = 0;
>  	u16 *data;
> -	u16 i = 0;
> +	u32 i = 0;

Does this change ensure that all array indexing using i remains within
bounds? Since data is u16*, can i safely index beyond 64K words if
sr_size is now u32?

> diff --git a/drivers/net/intel/i40e/i40e_ethdev.c b/drivers/net/intel/i40e/i40e_ethdev.c
> index 100a751225..13f3c23fef 100644
> --- a/drivers/net/intel/i40e/i40e_ethdev.c
> +++ b/drivers/net/intel/i40e/i40e_ethdev.c
> @@ -11357,7 +11357,8 @@ static int i40e_get_eeprom(struct rte_eth_dev *dev,
>  {
>  	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
>  	uint16_t *data = eeprom->data;
> -	uint16_t offset, length, cnt_words;
> +	uint32_t offset, length;
> +	uint16_t cnt_words;

Does this function validate offset and length to ensure they don't exceed
hardware capabilities or the actual size of the NVM before performing
operations?

---

git sha: 20260519145227.308814-3-ciara.loftus at intel.com
Author: Deepthi Kavalur <deepthi.kavalur at intel.com>
Subject: [PATCH 2/3] net/i40e/base: fix loop counter width in DCB parsing

This patch widens loop counters from u8 to u32 to prevent truncation when
numapps exceeds 255.

> diff --git a/drivers/net/intel/i40e/base/i40e_dcb.c b/drivers/net/intel/i40e/base/i40e_dcb.c
> index 04322ea034..c1d97fabab 100644
> --- a/drivers/net/intel/i40e/base/i40e_dcb.c
> +++ b/drivers/net/intel/i40e/base/i40e_dcb.c
> @@ -364,7 +364,7 @@ static void i40e_parse_cee_app_tlv(struct i40e_cee_feat_tlv *tlv,
>  {
>  	u16 length, typelength, offset = 0;
>  	struct i40e_cee_app_prio *app;
> -	u8 i;
> +	u32 i;

Does this code validate that numapps doesn't exceed the actual number of
application entries that can fit within the TLV length before iterating?
Can numapps cause an out-of-bounds access if it exceeds the available
data?

> @@ -1169,9 +1169,9 @@ static void i40e_add_ieee_app_pri_tlv(struct i40e_lldp_org_tlv *tlv,
>  				      struct i40e_dcbx_config *dcbcfg)
>  {
>  	u16 typelength, length, offset = 0;
> -	u8 priority, selector, i = 0;
>  	u8 *buf = tlv->tlvinfo;
> -	u32 ouisubtype;
> +	u32 ouisubtype, i = 0;
> +	u8 priority, selector;

Does this code perform bounds checking on i to ensure it doesn't index
beyond the allocated tlvinfo buffer when iterating through dcbcfg->app[]?