Re: [PATCH v3 0/3] net/txgbe: fix flow create errors

[Zhang Tengfei <zhtfdev@gmail.com>]

Thanks for the review.

Moving the b_spec check above the allocation is better. I will send a v4
with that change.

The VF status-to-errno conversion will be a separate patch.

Out of curiosity, which AI review tool did you use? Mine did not flag
these Infos.

Thanks

On 9/19/26 00:06, Stephen Hemminger wrote:
> On Thu, 17 Sep 2026 23:32:39 +0800
> Zhang Tengfei <zhtfdev@gmail.com> wrote:
>
>> v3:
>> - reject mask-only FDIR before programming the global mask
>> - log flex offset mismatch
>> - no code change in 1/3 and 2/3
>>
>> v2:
>> - split L2 tunnel add-failure goto out into its own patch
>> - allocate FDIR object before installing the global mask
>> - add Fixes tag for the VF FDIR path
>> - set -EINVAL on remaining FDIR goto out paths with bad ret
>> - use struct assignment instead of rte_memcpy for filter copies
>>
>> 1/3 fix L2 tunnel error on flow create
>> 2/3 fix leak of filters on flow create
>> 3/3 fix FDIR error handling on flow create
>>
>> Zhang Tengfei (3):
>>    net/txgbe: fix L2 tunnel error on flow create
>>    net/txgbe: fix leak of filters on flow create
>>    net/txgbe: fix FDIR error handling on flow create
>>
>>   drivers/net/txgbe/txgbe_flow.c | 266 +++++++++++++++++----------------
>>   1 file changed, 140 insertions(+), 126 deletions(-)
>>
> I applied this to next-net.
>
> AI review had some Info level observations, if you want to send
> a new version, I will replace the one in next-net.
>
> Review: [PATCH v3 0/3] net/txgbe: flow create fixes
> Author: Zhang Tengfei <zhtfdev@gmail.com>
>
> Applies to main, each of the three commits builds with -Dwerror=true.
> Fixes tags resolve; both referenced commits are in released versions,
> so Cc: stable is right.
>
> Patches 1 and 2 are unchanged from v2 apart from commit message
> wording. The v2 warning is addressed: mask-only FDIR rules are now
> rejected before the global input mask is programmed, so a failed
> create no longer leaves mask_added set. With that check in place the
> b_spec block is unconditional, which reads better than the old
> nesting. The memcmp and flex offset paths now set -EINVAL, and the
> flex mismatch has a log message.
>
>
> Patch 3/3: net/txgbe: fix FDIR error handling on flow create
>
> Info: the b_spec check sits after the rte_zmalloc(), so it allocates
>    and immediately frees on the reject path. Moving it above the
>    allocation drops the rte_free() and one level of churn:
>
> 	if (!fdir_rule.b_spec) {
> 		ret = -EINVAL;
> 		goto out;
> 	}
>
> 	fdir_rule_ptr = rte_zmalloc(...);
>
> Info: the VF path still reaches "out" with a base driver status code.
>    txgbevf_fdir_filter_program() propagates TXGBE_ERR_FEATURE_NOT_
>    SUPPORTED (-292) and TXGBE_ERR_NOSUPP from txgbevf_set_fdir(), so
>    rte_flow_error_set(error, -ret) gets 292, not an errno. The PF
>    paths checked out: txgbe_fdir_set_input_mask() and
>    txgbe_fdir_filter_program() return -ENOTSUP, -EINVAL, -ENOMEM or
>    -ETIMEDOUT. Converting the VF codes is a separate patch.
>
> Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>