On 2021-01-31 12:01 PM, Roi Dayan wrote:


On 2021-01-30 2:01 PM, Pablo Neira Ayuso wrote:
Hi Roi,

On Thu, Jan 28, 2021 at 09:40:52AM +0200, Roi Dayan wrote:
Currently, offloaded flows might be deleted when executing conntrack -L
or cat /proc/net/nf_conntrack while rules being offloaded.
Ct timeout is not maintained for offloaded flows as aging
of offloaded flows are managed by the flow table offload infrastructure.

Don't do garbage collection for offloaded flows when dumping the
entries.

Fixes: 90964016e5d3 ("netfilter: nf_conntrack: add IPS_OFFLOAD status bit")
Signed-off-by: Roi Dayan <r...@nvidia.com>
Reviewed-by: Oz Shlomo <o...@nvidia.com>
---
  include/net/netfilter/nf_conntrack.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/net/netfilter/nf_conntrack.h b/include/net/netfilter/nf_conntrack.h
index 439379ca9ffa..87c85109946a 100644
--- a/include/net/netfilter/nf_conntrack.h
+++ b/include/net/netfilter/nf_conntrack.h
@@ -276,7 +276,7 @@ static inline bool nf_ct_is_expired(const struct nf_conn *ct)
  static inline bool nf_ct_should_gc(const struct nf_conn *ct)
  {
      return nf_ct_is_expired(ct) && nf_ct_is_confirmed(ct) &&
-           !nf_ct_is_dying(ct);
+           !nf_ct_is_dying(ct) && !test_bit(IPS_OFFLOAD_BIT, &ct->status);

The gc_worker() calls nf_ct_offload_timeout() if the flow if
offloaded, so it extends the timeout to skip the garbage collection.

Could you update ctnetlink_dump_table() and ct_seq_show() to extend
the timeout if the flow is offloaded?

Thanks.


sure. i'll submit v2.
thanks


Hi Pablo,

We did more tests with just updating the timeout in the 2 callers
and it's not enough. We reproduce the issue of rules being timed
out just now frim different place.
There is a 3rd caller nf_ct_gc_expired() which being called by 3
other callers:
____nf_conntrack_find()
nf_conntrack_tuple_taken()
early_drop_list()

only early_drop_list() has a check to skip conns with offload bit
but without extending the timeout.
I didnt do a dump but the issue could be from the other 2 calls.

With current commit as is I didn't need to check more callers as I made
sure all callers will skip the non-offload gc.
Instead of updating more callers and there might be more callers
later why current commit is not enough?
We skip offloaded flows and soon gc_worker() will hit and will update
the timeout anyway.

Thanks,
Roi

Reply via email to