Re: [PATCH nf v3 1/1] ipvs: stop estimator after disabled calc phase

Julian Anastasov <[email protected]> Thu, 30 Jul 2026 17:44:04 +0300 (EEST)
Newsgroups org.kernel.vger.lvs-devel,org.kernel.vger.netdev,org.kernel.vger.netfilter-devel
Message-ID <[email protected]>
	Hello,

On Wed, 29 Jul 2026, Zhiling Zou wrote:

> IPVS estimator kthread 0 starts with zeroed chain and tick limits until
> its initial calculation phase completes. If network namespace teardown
> clears ipvs->enable during that phase, ip_vs_est_calc_phase() can return
> without installing positive limits.
> 
> The kthread can then continue into its main loop and drain
> est_temp_list with zero chain_max, tick_max and est_max_count values.
> Each enqueue consumes one available tick row, but est_count never
> reaches the zero est_max_count value. After all rows are consumed, the
> row lookup returns IPVS_EST_NTICKS and ip_vs_enqueue_estimator() writes
> past the ticks and tick_len arrays.
> 
> Exit kthread 0 after the calculation phase if the kthread is stopping or
> IPVS has been disabled. That keeps temporary estimators from being
> drained after the limits failed to initialize.
> 
> Estimator kthreads can now self-exit before teardown or reload stops
> kd->task. Keep an extra task reference after creation and release it
> with kthread_stop_put(), so kd->task remains valid until the stop paths
> consume that reference.
> 
> Fixes: 705dd3444081 ("ipvs: use kthreads for stats estimation")
> Cc: [email protected]
> Reported-by: Vega <[email protected]>
> Signed-off-by: Zhiling Zou <[email protected]>

	Looks good to me, thanks!

Acked-by: Julian Anastasov <[email protected]>

> ---
> changes in v3:
> - Keep an extra task_struct reference for estimator kthreads and
>   release it with kthread_stop_put(), as suggested by Julian
>   Anastasov, so self-exiting kthreads do not leave kd->task dangling.
> - v2 Link: https://lore.kernel.org/all/03a7ee4873f9dfc51add97df5b67bf0b0d793dbe.1785211268.git.zhilinz@nebusec.ai
> 
> changes in v2:
> - Exit the estimator kthread after the calculation phase if IPVS
>   has been disabled or the kthread is stopping, as suggested by
>   Julian Anastasov.
> - Drop the later enqueue and drain guards from v1.
> - v1 Link: https://lore.kernel.org/all/6359e0031f0ee7cc969bae3483308de362d3c709.1784878022.git.zhilinz@nebusec.ai
> 
>  net/netfilter/ipvs/ip_vs_est.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/net/netfilter/ipvs/ip_vs_est.c b/net/netfilter/ipvs/ip_vs_est.c
> index ab09f51829512..05a216a47b45e 100644
> --- a/net/netfilter/ipvs/ip_vs_est.c
> +++ b/net/netfilter/ipvs/ip_vs_est.c
> @@ -191,8 +191,11 @@ static int ip_vs_estimation_kthread(void *data)
>  		}
>  
>  		/* kthread 0 will handle the calc phase */
> -		if (ipvs->est_calc_phase)
> +		if (ipvs->est_calc_phase) {
>  			ip_vs_est_calc_phase(ipvs);
> +			if (kthread_should_stop() || !READ_ONCE(ipvs->enable))
> +				return 0;
> +		}
>  	}
>  
>  	while (1) {
> @@ -270,6 +273,7 @@ int ip_vs_est_kthread_start(struct netns_ipvs *ipvs,
>  		kd->task = NULL;
>  		goto out;
>  	}
> +	get_task_struct(kd->task);
>  
>  	set_user_nice(kd->task, sysctl_est_nice(ipvs));
>  	if (sysctl_est_preferred_cpulist(ipvs))
> @@ -286,7 +290,7 @@ void ip_vs_est_kthread_stop(struct ip_vs_est_kt_data *kd)
>  {
>  	if (kd->task) {
>  		pr_info("stopping estimator thread %d...\n", kd->id);
> -		kthread_stop(kd->task);
> +		kthread_stop_put(kd->task);
>  		kd->task = NULL;
>  	}
>  }
> @@ -526,7 +530,7 @@ static void ip_vs_est_kthread_destroy(struct ip_vs_est_kt_data *kd)
>  	if (kd) {
>  		if (kd->task) {
>  			pr_info("stop unused estimator thread %d...\n", kd->id);
> -			kthread_stop(kd->task);
> +			kthread_stop_put(kd->task);
>  		}
>  		ip_vs_stats_free(kd->calc_stats);
>  		kfree(kd);
> -- 
> 2.43.0

Regards

--
Julian Anastasov <[email protected]>