Re: [PATCH v4 18/27] net/iavf: decouple hash uninit from parser uninit

Burakov, Anatoly
2026-02-18T10:32:28Z

On 2/16/2026 6:23 PM, Bruce Richardson wrote:
> On Fri, Feb 13, 2026 at 10:26:29AM +0000, Anatoly Burakov wrote:
>> Currently, parser deinitialization will trigger removal of current RSS
>> configuration. This should not be done as part of parser deinitialization,
>> but should rather be a separate step in dev close flow.
>>
>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
>> ---
>> drivers/net/intel/iavf/iavf_ethdev.c | 4 ++++
>> drivers/net/intel/iavf/iavf_hash.c | 13 +++++++++----
>> drivers/net/intel/iavf/iavf_hash.h | 13 +++++++++++++
>> 3 files changed, 26 insertions(+), 4 deletions(-)
>> create mode 100644 drivers/net/intel/iavf/iavf_hash.h
>>
>> diff --git a/drivers/net/intel/iavf/iavf_ethdev.c b/drivers/net/intel/iavf/iavf_ethdev.c
>> index 802e095174..70eb7e7ec5 100644
>> --- a/drivers/net/intel/iavf/iavf_ethdev.c
>> +++ b/drivers/net/intel/iavf/iavf_ethdev.c
>> @@ -35,6 +35,7 @@
>> #include "iavf_generic_flow.h"
>> #include "rte_pmd_iavf.h"
>> #include "iavf_ipsec_crypto.h"
>> +#include "iavf_hash.h"
>>
>> /* devargs */
>> #define IAVF_PROTO_XTR_ARG "proto_xtr"
>> @@ -2972,6 +2973,9 @@ iavf_dev_close(struct rte_eth_dev *dev)
>> /* free iAVF security device context all related resources */
>> iavf_security_ctx_destroy(adapter);
>>
>> + /* remove RSS configuration */
>> + iavf_hash_uninit(adapter);
>> +
>> iavf_flow_flush(dev, NULL);
>> iavf_flow_uninit(adapter);
>>
>> diff --git a/drivers/net/intel/iavf/iavf_hash.c b/drivers/net/intel/iavf/iavf_hash.c
>> index a40fed7542..d864998402 100644
>> --- a/drivers/net/intel/iavf/iavf_hash.c
>> +++ b/drivers/net/intel/iavf/iavf_hash.c
>> @@ -22,6 +22,7 @@
>> #include "iavf_log.h"
>> #include "iavf.h"
>> #include "iavf_generic_flow.h"
>> +#include "iavf_hash.h"
>>
>> #define IAVF_PHINT_NONE 0
>> #define IAVF_PHINT_GTPU BIT_ULL(0)
>> @@ -77,7 +78,7 @@ static int
>> iavf_hash_destroy(struct iavf_adapter *ad, struct rte_flow *flow,
>> struct rte_flow_error *error);
>> static void
>> -iavf_hash_uninit(struct iavf_adapter *ad);
>> +iavf_hash_uninit_parser(struct iavf_adapter *ad);
>> static void
>> iavf_hash_free(struct rte_flow *flow);
>> static int
>> @@ -680,7 +681,7 @@ static struct iavf_flow_engine iavf_hash_engine = {
>> .init = iavf_hash_init,
>> .create = iavf_hash_create,
>> .destroy = iavf_hash_destroy,
>> - .uninit = iavf_hash_uninit,
>> + .uninit = iavf_hash_uninit_parser,
>> .free = iavf_hash_free,
>> .type = IAVF_FLOW_ENGINE_HASH,
>> };
>> @@ -1641,6 +1642,12 @@ iavf_hash_destroy(__rte_unused struct iavf_adapter *ad,
>> }
>>
>> static void
>> +iavf_hash_uninit_parser(struct iavf_adapter *ad)
>> +{
>> + iavf_unregister_parser(&iavf_hash_parser, ad);
>> +}
>> +
>> +void
>> iavf_hash_uninit(struct iavf_adapter *ad)
>> {
>> struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(ad);
>> @@ -1658,8 +1665,6 @@ iavf_hash_uninit(struct iavf_adapter *ad)
>> rss_conf = &ad->dev_data->dev_conf.rx_adv_conf.rss_conf;
>> if (iavf_rss_hash_set(ad, rss_conf->rss_hf, false))
>> PMD_DRV_LOG(ERR, "fail to delete default RSS");
>> -
>> - iavf_unregister_parser(&iavf_hash_parser, ad);
>> }
>>
>> static void
>> diff --git a/drivers/net/intel/iavf/iavf_hash.h b/drivers/net/intel/iavf/iavf_hash.h
>> new file mode 100644
>> index 0000000000..2348f32673
>> --- /dev/null
>> +++ b/drivers/net/intel/iavf/iavf_hash.h
>> @@ -0,0 +1,13 @@
>> +/* SPDX-License-Identifier: BSD-3-Clause
>> + * Copyright(c) 2025 Intel Corporation
>> + */
>> +
>> +#ifndef _IAVF_HASH_H_
>> +#define _IAVF_HASH_H_
>> +
>> +#include "iavf.h"
>> +
>> +void
>> +iavf_hash_uninit(struct iavf_adapter *ad);
>> +
>> +#endif /* _IAVF_HASH_H_ */
>> --
>
> While its primarily a matter of taste, do we really need to create a new
> header for this? For a single function prototype can it not just go in
> iavf.h directly itself?
>
> Either way:
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
>

This code is not touched further down the line so yes, I can move it to
iavf.h no problem.

--
Thanks,
Anatoly