From: Pablo Neira Ayuso <pablo@netfilter.org>
To: Paul Blakey <paulb@mellanox.com>
Cc: Oz Shlomo <ozsh@mellanox.com>, Roi Dayan <roid@mellanox.com>,
netdev@vger.kernel.org, Saeed Mahameed <saeedm@mellanox.com>,
netfilter-devel@vger.kernel.org
Subject: Re: [PATCH net] netfilter: flowtable: Add pending bit for offload work
Date: Mon, 11 May 2020 00:14:34 +0200 [thread overview]
Message-ID: <20200510221434.GA11226@salvia> (raw)
In-Reply-To: <1588764279-12166-1-git-send-email-paulb@mellanox.com>
Hi,
On Wed, May 06, 2020 at 02:24:39PM +0300, Paul Blakey wrote:
> Gc step can queue offloaded flow del work or stats work.
> Those work items can race each other and a flow could be freed
> before the stats work is executed and querying it.
> To avoid that, add a pending bit that if a work exists for a flow
> don't queue another work for it.
> This will also avoid adding multiple stats works in case stats work
> didn't complete but gc step started again.
This is happening since the mutex has been removed, right?
Another question below.
> Signed-off-by: Paul Blakey <paulb@mellanox.com>
> Reviewed-by: Roi Dayan <roid@mellanox.com>
> ---
> include/net/netfilter/nf_flow_table.h | 1 +
> net/netfilter/nf_flow_table_offload.c | 8 +++++++-
> 2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/include/net/netfilter/nf_flow_table.h b/include/net/netfilter/nf_flow_table.h
> index 6bf6965..c54a7f7 100644
> --- a/include/net/netfilter/nf_flow_table.h
> +++ b/include/net/netfilter/nf_flow_table.h
> @@ -127,6 +127,7 @@ enum nf_flow_flags {
> NF_FLOW_HW_DYING,
> NF_FLOW_HW_DEAD,
> NF_FLOW_HW_REFRESH,
> + NF_FLOW_HW_PENDING,
> };
>
> enum flow_offload_type {
> diff --git a/net/netfilter/nf_flow_table_offload.c b/net/netfilter/nf_flow_table_offload.c
> index b9d5ecc..731d738 100644
> --- a/net/netfilter/nf_flow_table_offload.c
> +++ b/net/netfilter/nf_flow_table_offload.c
> @@ -817,6 +817,7 @@ static void flow_offload_work_handler(struct work_struct *work)
> WARN_ON_ONCE(1);
> }
>
> + clear_bit(NF_FLOW_HW_PENDING, &offload->flow->flags);
> kfree(offload);
> }
>
> @@ -831,9 +832,14 @@ static void flow_offload_queue_work(struct flow_offload_work *offload)
> {
> struct flow_offload_work *offload;
>
> + if (test_and_set_bit(NF_FLOW_HW_PENDING, &flow->flags))
> + return NULL;
In case of stats, it's fine to lose work.
But how does this work for the deletion case? Does this falls back to
the timeout deletion?
Thanks.
next prev parent reply other threads:[~2020-05-10 22:14 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-06 11:24 [PATCH net] netfilter: flowtable: Add pending bit for offload work Paul Blakey
2020-05-10 22:14 ` Pablo Neira Ayuso [this message]
2020-05-11 8:32 ` Paul Blakey
2020-05-11 11:59 ` Pablo Neira Ayuso
2020-05-11 13:57 ` Roi Dayan
2020-05-11 13:58 ` Roi Dayan
2020-05-11 14:37 ` Pablo Neira Ayuso
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=20200510221434.GA11226@salvia \
--to=pablo@netfilter.org \
--cc=netdev@vger.kernel.org \
--cc=netfilter-devel@vger.kernel.org \
--cc=ozsh@mellanox.com \
--cc=paulb@mellanox.com \
--cc=roid@mellanox.com \
--cc=saeedm@mellanox.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).