netdev - Re: [PATCH net-next v3 2/7] netfilter: flowtable: fixup UDP timeout depending on ct state

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] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 24 Jan 2023 09:08:34 +0200
From:   Vlad Buslov <vladbu@...dia.com>
To:     Pablo Neira Ayuso <pablo@...filter.org>
CC:     <davem@...emloft.net>, <kuba@...nel.org>, <pabeni@...hat.com>,
        <netdev@...r.kernel.org>, <netfilter-devel@...r.kernel.org>,
        <jhs@...atatu.com>, <xiyou.wangcong@...il.com>, <jiri@...nulli.us>,
        <ozsh@...dia.com>, <marcelo.leitner@...il.com>,
        <simon.horman@...igine.com>
Subject: Re: [PATCH net-next v3 2/7] netfilter: flowtable: fixup UDP timeout
 depending on ct state


On Fri 20 Jan 2023 at 12:57, Pablo Neira Ayuso <pablo@...filter.org> wrote:
> On Thu, Jan 19, 2023 at 08:50:59PM +0100, Vlad Buslov wrote:
>> Currently flow_offload_fixup_ct() function assumes that only replied UDP
>> connections can be offloaded and hardcodes UDP_CT_REPLIED timeout value. To
>> enable UDP NEW connection offload in following patches extract the actual
>> connections state from ct->status and set the timeout according to it.
>> 
>> Signed-off-by: Vlad Buslov <vladbu@...dia.com>
>> ---
>>  net/netfilter/nf_flow_table_core.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>> 
>> diff --git a/net/netfilter/nf_flow_table_core.c b/net/netfilter/nf_flow_table_core.c
>> index 81c26a96c30b..04bd0ed4d2ae 100644
>> --- a/net/netfilter/nf_flow_table_core.c
>> +++ b/net/netfilter/nf_flow_table_core.c
>> @@ -193,8 +193,11 @@ static void flow_offload_fixup_ct(struct nf_conn *ct)
>>  		timeout -= tn->offload_timeout;
>>  	} else if (l4num == IPPROTO_UDP) {
>>  		struct nf_udp_net *tn = nf_udp_pernet(net);
>> +		enum udp_conntrack state =
>> +			test_bit(IPS_SEEN_REPLY_BIT, &ct->status) ?
>> +			UDP_CT_REPLIED : UDP_CT_UNREPLIED;
>>  
>> -		timeout = tn->timeouts[UDP_CT_REPLIED];
>> +		timeout = tn->timeouts[state];
>>  		timeout -= tn->offload_timeout;
>
> For netfilter's flowtable (not talking about act_ct), this is a
> "problem" because the flowtable path update with ct->status flags.
> In other words, for netfilter's flowtable UDP_CT_UNREPLIED timeout
> will be always used for UDP traffic if it is offloaded and no traffic
> from the classic path was seen.

Hmm, I didn't consider that netfilter might not update the status. Will
try to decouple the teardown timeout calculation and allow flow_table
users to specify their own.

>
> If packets go via hardware offload, the host does not see packets in
> the reply direction (unless hardware counters are used to set on
> IPS_SEEN_REPLY_BIT?).
>
> Then, there is also IPS_ASSURED: Netfilter's flowtable assumes that
> TCP flows are only offloaded to hardware if IPS_ASSURED.