netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Florian Westphal <fw@strlen.de>
To: Roi Dayan <roid@nvidia.com>
Cc: Florian Westphal <fw@strlen.de>,
	Pablo Neira Ayuso <pablo@netfilter.org>,
	netdev@vger.kernel.org, Paul Blakey <paulb@nvidia.com>,
	Oz Shlomo <ozsh@nvidia.com>
Subject: Re: [PATCH net 1/1] netfilter: conntrack: Check offload bit on table dump
Date: Wed, 3 Feb 2021 13:50:35 +0100	[thread overview]
Message-ID: <20210203125035.GC16570@breakpoint.cc> (raw)
In-Reply-To: <a908ac8f-1fb4-1427-520d-3a702ecb7597@nvidia.com>

Roi Dayan <roid@nvidia.com> wrote:
> > Do you think rhashtable_insert_fast() in flow_offload_add() blocks for
> > dozens of seconds?
> 
> I'm not sure. but its not only that but also the time to be in
> established state as only then we offload.

That makes it even more weird.  Timeout for established is even larger.
In case of TCP, its days... so I don't understand at all :/

> > Thats about the only thing I can see between 'offload bit gets set'
> > and 'timeout is extended' in flow_offload_add() that could at least
> > spend *some* time.
> > 
> > > We hit this issue before more easily and pushed this fix
> > > 
> > > 4203b19c2796 netfilter: flowtable: Set offload timeout when adding flow
> > 
> > This fix makes sense to me.
> 
> I just noted we didn't test correctly Pablo's suggestion instead of
> to check the bit and extend the timeout in ctnetlink_dump_table() and
> ct_seq_show() like GC does.

Ok.  Extending it there makes sense, but I still don't understand
why newly offloaded flows hit this problem.

Flow offload should never see a 'about to expire' ct entry.
The 'extend timeout from gc' is more to make sure GC doesn't reap
long-lived entries that have been offloaded aeons ago, not 'prevent
new flows from getting zapped...'

  reply	other threads:[~2021-02-03 12:53 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-28  7:40 [PATCH net 1/1] netfilter: conntrack: Check offload bit on table dump Roi Dayan
2021-01-30 12:01 ` Pablo Neira Ayuso
     [not found]   ` <3a29e9b5-7bf8-5c00-3ede-738f9b4725bf@nvidia.com>
     [not found]     ` <997cbda4-acd1-a000-1408-269bc5c3abf3@nvidia.com>
2021-02-01  3:08       ` Pablo Neira Ayuso
2021-02-01  7:53         ` Roi Dayan
2021-02-01 11:50           ` Florian Westphal
2021-02-01 15:04             ` Roi Dayan
2021-02-01 15:25               ` Florian Westphal
2021-02-02 17:08                 ` Roi Dayan
2021-02-03 12:50                   ` Florian Westphal [this message]
2021-02-07  8:38                     ` Roi Dayan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210203125035.GC16570@breakpoint.cc \
    --to=fw@strlen.de \
    --cc=netdev@vger.kernel.org \
    --cc=ozsh@nvidia.com \
    --cc=pablo@netfilter.org \
    --cc=paulb@nvidia.com \
    --cc=roid@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).