netdev - Re: [PATCH net 1/1] netfilter: conntrack: Check offload bit on table dump

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Sun, 7 Feb 2021 10:38:17 +0200
From:   Roi Dayan <roid@...dia.com>
To:     Florian Westphal <fw@...len.de>
CC:     Pablo Neira Ayuso <pablo@...filter.org>, <netdev@...r.kernel.org>,
        "Paul Blakey" <paulb@...dia.com>, Oz Shlomo <ozsh@...dia.com>
Subject: Re: [PATCH net 1/1] netfilter: conntrack: Check offload bit on table
 dump



On 2021-02-03 2:50 PM, Florian Westphal wrote:
> Roi Dayan <roid@...dia.com> wrote:
>>> Do you think rhashtable_insert_fast() in flow_offload_add() blocks for
>>> dozens of seconds?
>>
>> I'm not sure. but its not only that but also the time to be in
>> established state as only then we offload.
> 
> That makes it even more weird.  Timeout for established is even larger.
> In case of TCP, its days... so I don't understand at all :/
> 
>>> Thats about the only thing I can see between 'offload bit gets set'
>>> and 'timeout is extended' in flow_offload_add() that could at least
>>> spend *some* time.
>>>
>>>> We hit this issue before more easily and pushed this fix
>>>>
>>>> 4203b19c2796 netfilter: flowtable: Set offload timeout when adding flow
>>>
>>> This fix makes sense to me.
>>
>> I just noted we didn't test correctly Pablo's suggestion instead of
>> to check the bit and extend the timeout in ctnetlink_dump_table() and
>> ct_seq_show() like GC does.
> 
> Ok.  Extending it there makes sense, but I still don't understand
> why newly offloaded flows hit this problem.
> 
> Flow offload should never see a 'about to expire' ct entry.
> The 'extend timeout from gc' is more to make sure GC doesn't reap
> long-lived entries that have been offloaded aeons ago, not 'prevent
> new flows from getting zapped...'
> 

I'll add that an extended timeout from gc is if gc actually iterated
this conn since it was created.
I'll investigate this more and try to do more tests.
thanks for the info