Re: [PATCH nf-next] netfilter: conntrack: remove flowtable early-drop test

[Vlad Buslov <vladbu@nvidia.com>]

On Tue 23 Apr 2024 at 15:44, Florian Westphal <fw@strlen.de> wrote:
> Not sure why this special case exists.  Early drop logic
> (which kicks in when conntrack table is full) should be independent
> of flowtable offload and only consider assured bit (i.e., two-way
> traffic was seen).
>
> flowtable entries hold a reference to the conntrack entry (struct
> nf_conn) that has been offloaded. The conntrack use count is not
> decremented until after the entry is free'd.
>
> This change therefore will not result in exceeding the conntrack table
> limit.  It does allow early-drop of tcp flows even when they've been
> offloaded, but only if they have been offloaded before syn-ack was
> received or after at least one peer has sent a fin.
>
> Currently 'fin' packet reception already stops offloading, so this
> should not impact offloading either.
>
> Cc: Vlad Buslov <vladbu@nvidia.com>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
>  Vlad, do you remember why you added this test?

I added it when I introduced UDP NEW connection offload. As far as I
remember the concern was that since at the time early drop algorithm
completely ignored all offloaded connections malicious user could fill
the whole table by just sending a single packet per range of distinct 5
tuples and none of the resulting connections would be early dropped
until they expire.

>
>  For reference, this came in
>  df25455e5a48 ("netfilter: nf_conntrack: allow early drop of offloaded UDP conns")
>  and maybe was just a 'move-it-around' from the check in
>  early_drop_list, which would mean this was there from the
>  beginning.  Doesn't change "i don't understand why this test
>  exists" though :-)
>
>  net/netfilter/nf_conntrack_core.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
> index c63868666bd9..43629e79067d 100644
> --- a/net/netfilter/nf_conntrack_core.c
> +++ b/net/netfilter/nf_conntrack_core.c
> @@ -1440,8 +1440,6 @@ static bool gc_worker_can_early_drop(const struct nf_conn *ct)
>  	const struct nf_conntrack_l4proto *l4proto;
>  	u8 protonum = nf_ct_protonum(ct);
>  
> -	if (test_bit(IPS_OFFLOAD_BIT, &ct->status) && protonum != IPPROTO_UDP)
> -		return false;
>  	if (!test_bit(IPS_ASSURED_BIT, &ct->status))
>  		return true;