On Tue 23 Apr 2024 at 15:44, Florian Westphal <fw@xxxxxxxxx> 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@xxxxxxxxxx>
> Signed-off-by: Florian Westphal <fw@xxxxxxxxx>
> ---
> 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;
