Liang Mancang <liang...@chinatelecom.cn> writes:
> 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.
of course, if not balanced that could happen.
>> >
>> >> 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.
Thanks.
>> >> 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
