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]>