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

[Vlad Buslov <vladbu@nvidia.com>]

On Fri 20 Jan 2023 at 12:57, Pablo Neira Ayuso <pablo@netfilter.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@nvidia.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.