On 5 Sep 2024, at 14:32, Aaron Conole wrote:
> After 3d9c1b855a5f ("conntrack: Replace timeout based expiration lists with
> rculists.")
> the conntrack cleanup log reports the number of connections it checked
> rather than the number of connections it cleaned. This patch includes the
> count of connections cleaned during expiration sweeping.
The patch looks good to me, however one comments on the log message itself.
//Eelco
> Reported-by: Cheng Li <[email protected]>
> Suggested-by: Cheng Li <[email protected]>
> Fixes: 3d9c1b855a5f ("conntrack: Replace timeout based expiration lists with
> rculists.")
> Signed-off-by: Aaron Conole <[email protected]>
> ---
> lib/conntrack.c | 18 +++++++++++++++---
> 1 file changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index db44f82374..e90c2ac19f 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -1443,20 +1443,27 @@ conntrack_get_sweep_interval(struct conntrack *ct)
> }
>
> static size_t
> -ct_sweep(struct conntrack *ct, struct rculist *list, long long now)
> +ct_sweep(struct conntrack *ct, struct rculist *list, long long now,
> + size_t *cleaned_count)
> OVS_NO_THREAD_SAFETY_ANALYSIS
> {
> struct conn *conn;
> + size_t cleaned = 0;
> size_t count = 0;
>
> RCULIST_FOR_EACH (conn, node, list) {
> if (conn_expired(conn, now)) {
> conn_clean(ct, conn);
> + cleaned++;
> }
>
> count++;
> }
>
> + if (cleaned_count) {
> + *cleaned_count = cleaned;
> + }
> +
> return count;
> }
>
> @@ -1469,22 +1476,27 @@ conntrack_clean(struct conntrack *ct, long long now)
> long long next_wakeup = now + conntrack_get_sweep_interval(ct);
> unsigned int n_conn_limit, i;
> size_t clean_end, count = 0;
> + size_t total_cleaned = 0;
>
> atomic_read_relaxed(&ct->n_conn_limit, &n_conn_limit);
> clean_end = n_conn_limit / 64;
>
> for (i = ct->next_sweep; i < N_EXP_LISTS; i++) {
> + size_t cleaned;
> +
> if (count > clean_end) {
> next_wakeup = 0;
> break;
> }
>
> - count += ct_sweep(ct, &ct->exp_lists[i], now);
> + count += ct_sweep(ct, &ct->exp_lists[i], now, &cleaned);
> + total_cleaned += cleaned;
> }
>
> ct->next_sweep = (i < N_EXP_LISTS) ? i : 0;
>
> - VLOG_DBG("conntrack cleanup %"PRIuSIZE" entries in %lld msec", count,
> + VLOG_DBG("conntrack cleaned %"PRIuSIZE" / %"PRIuSIZE
> + " entries in %lld msec", total_cleaned, count,
> time_msec() - now);
Can we make the log message a bit more clear? Maybe something like:
“conntrack cleaned %"PRIuSIZE" entries out of %"PRIuSIZE" in %lld msec”
>
> return next_wakeup;
> --
> 2.45.2
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
