|SUCCESS| net/iavf: remove dead code in TM node parameter check

dpdklab at iol.unh.edu dpdklab at iol.unh.edu
Wed Apr 8 15:43:06 CEST 2026


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

_AI Code Review_

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

git sha: 44d0a720a538 ("net/iavf: query QoS capabilities and set queue TC mapping")
Author: Sergei Iashin <yashin.sergey at gmail.com>
Subject: [PATCH] net/iavf: remove dead code in TM node parameter check

The patch removes a dead code path in the iavf_node_param_check()
function where an unreachable conditional check on wfq_weight_mode
pointer existed after a prior check that already rejects any non-NULL
pointer.

> diff --git a/drivers/net/intel/iavf/iavf_tm.c b/drivers/net/intel/iavf/iavf_tm.c
> index 1d12196ba6..efdbb45b62 100644
> --- a/drivers/net/intel/iavf/iavf_tm.c
> +++ b/drivers/net/intel/iavf/iavf_tm.c
> @@ -178,12 +178,6 @@ iavf_node_param_check(struct iavf_info *vf, uint32_t node_id,
>  				RTE_TM_ERROR_TYPE_NODE_PARAMS_N_SP_PRIORITIES;
>  			error->message = "SP priority not supported";
>  			return -EINVAL;
> -		} else if (params->nonleaf.wfq_weight_mode &&
> -			   !(*params->nonleaf.wfq_weight_mode)) {
> -			error->type =
> -				RTE_TM_ERROR_TYPE_NODE_PARAMS_WFQ_WEIGHT_MODE;
> -			error->message = "WFP should be byte mode";
> -			return -EINVAL;
>  		}
>  
>  		return 0;

Does this code leave a potential validation gap? Looking at the broader
context, the earlier check rejects any non-NULL wfq_weight_mode, but
the deleted block was testing for a non-NULL pointer with a value of 0
(false). If the first check should have been validating something else,
removing the second check could now allow invalid configurations
through.

Can you confirm whether the original intent was to:
1. Reject if wfq_weight_mode is non-NULL (current first check)
2. Reject if wfq_weight_mode is non-NULL and value is 0 (deleted check)

If the intended logic was to reject only when the mode is non-NULL AND
set to 0 (packet mode), then the first check at line ~172 may be
incorrect rather than the deleted code being truly dead. Without seeing
the preceding context, does the function now properly validate WFQ mode
settings, or does it reject all WFQ configurations unconditionally?

Also, the deleted error message contains a typo: "WFP should be byte
mode" should likely be "WFQ should be byte mode". While this is being
removed, it suggests possible confusion in the original implementation
logic.


More information about the test-report mailing list