On Mon, Feb 20, 2023 at 07:38:39PM +0100, Paolo Valerio wrote:
> Paolo Valerio <pvale...@redhat.com> writes:
>
> > Hello Liang,
> >
> > Liang Mancang <liang...@chinatelecom.cn> writes:
> >
> >> when a exp_list contains more than the clean_end's number of nodes,
> >> and these nodes will not expire immediately. Then, every times we
> >> call conntrack_clean, it use the same next_sweep to get exp_list.
> >>
> >
> > Yes, in general, if the previous count exceeds the clean_end, it should
> > not make the sweeper restart from a list just swept, but it should not
> > happen that a single list contains more than n_conn_limit / 64.
> >
> > Did you observe a single exp_list containing more than n_conn_limit / 64
> > entries?
We only select exp_list for a conntrack entry when createing it, but never move
them when update their expires or delete them. So the number of each exp_list
will become unbalanced after long-time running.
> >
> >> Actually, we should add i every times after we call ct_sweep.
> >>
> >> Signed-off-by: Liang Mancang <liang...@chinatelecom.cn>
> >> ---
> >> lib/conntrack.c | 5 +++--
> >> 1 file changed, 3 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/lib/conntrack.c b/lib/conntrack.c
> >> index 524670e45..5029b2cda 100644
> >> --- a/lib/conntrack.c
> >> +++ b/lib/conntrack.c
> >> @@ -1512,12 +1512,13 @@ conntrack_clean(struct conntrack *ct, long long
> >> now)
> >> clean_end = n_conn_limit / 64;
> >>
> >> for (i = ct->next_sweep; i < N_EXP_LISTS; i++) {
> >> - count += ct_sweep(ct, &ct->exp_lists[i], now);
> >> -
> >> if (count > clean_end) {
> >> next_wakeup = 0;
> >> +
> >
> > This new line is not needed, and a Fixes tag could be added:
> >
> > Fixes: 3d9c1b855a5f ("conntrack: Replace timeout based expiration lists
> > with rculists.")
> >
> > The patch LGTM,
> >
>
> Sorry, the last line slipped out. Please consider my question and the
> other comments. I will explicitly tag the patch once we're done.
>
I sent v2 for this.
> >> break;
> >> }
> >> +
> >> + count += ct_sweep(ct, &ct->exp_lists[i], now);
> >> }
> >>
> >> ct->next_sweep = (i < N_EXP_LISTS) ? i : 0;
> >> --
> >> 2.30.0.windows.2
> >>
> >> _______________________________________________
> >> dev mailing list
> >> d...@openvswitch.org
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
