All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vlad Buslov <vladbu@nvidia.com>
To: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: <davem@davemloft.net>, <kuba@kernel.org>, <pabeni@redhat.com>,
	<netdev@vger.kernel.org>, <netfilter-devel@vger.kernel.org>,
	<jhs@mojatatu.com>, <xiyou.wangcong@gmail.com>,
	<jiri@resnulli.us>, <ozsh@nvidia.com>,
	<marcelo.leitner@gmail.com>, <simon.horman@corigine.com>
Subject: Re: [PATCH net-next v3 4/7] netfilter: flowtable: allow updating offloaded rules asynchronously
Date: Tue, 24 Jan 2023 09:06:13 +0200	[thread overview]
Message-ID: <871qnke7ga.fsf@nvidia.com> (raw)
In-Reply-To: <Y8p96knLDtxnRtjz@salvia>


On Fri 20 Jan 2023 at 12:41, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> Hi Vlad,
>
> On Thu, Jan 19, 2023 at 08:51:01PM +0100, Vlad Buslov wrote:
>> Following patches in series need to update flowtable rule several times
>> during its lifetime in order to synchronize hardware offload with actual ct
>> status. However, reusing existing 'refresh' logic in act_ct would cause
>> data path to potentially schedule significant amount of spurious tasks in
>> 'add' workqueue since it is executed per-packet. Instead, introduce a new
>> flow 'update' flag and use it to schedule async flow refresh in flowtable
>> gc which will only be executed once per gc iteration.
>
> So the idea is to use a NF_FLOW_HW_UPDATE which triggers the update
> from the garbage collector. I understand the motivation here is to
> avoid adding more work to the workqueue, by simply letting the gc
> thread pick up for the update.
>
> I already proposed in the last year alternative approaches to improve
> the workqueue logic, including cancelation of useless work. For
> example, cancel a flying "add" work if "delete" just arrive and the
> work is still sitting in the queue. Same approach could be use for
> this update logic, ie. cancel an add UDP unidirectional or upgrade it
> to bidirectional if, by the time we see traffic in both directions,
> then work is still sitting in the queue.

Thanks for the suggestion. I'll try to make this work over regular
workqueues without further extending the flow flags and/or putting more
stuff into gc.

>
> I am sorry to say but it seems to me this approach based on flags is
> pushing the existing design to the limit. The flag semantics is
> already overloaded that this just makes the state machine behind the
> flag logic more complicated. I really think we should explore for
> better strategies for the offload work to be processed.

Got it.


  reply	other threads:[~2023-01-24  7:08 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-19 19:50 [PATCH net-next v3 0/7] Allow offloading of UDP NEW connections via act_ct Vlad Buslov
2023-01-19 19:50 ` [PATCH net-next v3 1/7] net: flow_offload: provision conntrack info in ct_metadata Vlad Buslov
2023-01-19 19:50 ` [PATCH net-next v3 2/7] netfilter: flowtable: fixup UDP timeout depending on ct state Vlad Buslov
2023-01-20 11:57   ` Pablo Neira Ayuso
2023-01-24  7:08     ` Vlad Buslov
2023-01-19 19:51 ` [PATCH net-next v3 3/7] netfilter: flowtable: allow unidirectional rules Vlad Buslov
2023-01-19 19:51 ` [PATCH net-next v3 4/7] netfilter: flowtable: allow updating offloaded rules asynchronously Vlad Buslov
2023-01-20 11:41   ` Pablo Neira Ayuso
2023-01-24  7:06     ` Vlad Buslov [this message]
2023-01-24  8:41       ` Pablo Neira Ayuso
2023-01-24  9:19         ` Vlad Buslov
2023-01-24 13:57           ` Vlad Buslov
2023-01-19 19:51 ` [PATCH net-next v3 5/7] net/sched: act_ct: set ctinfo in meta action depending on ct state Vlad Buslov
2023-01-19 19:51 ` [PATCH net-next v3 6/7] net/sched: act_ct: offload UDP NEW connections Vlad Buslov
2023-01-19 19:51 ` [PATCH net-next v3 7/7] netfilter: nf_conntrack: allow early drop of offloaded UDP conns Vlad Buslov
2023-01-19 21:37 ` [PATCH net-next v3 0/7] Allow offloading of UDP NEW connections via act_ct Marcelo Ricardo Leitner
2023-01-20  6:38   ` Vlad Buslov
2023-01-20  6:57     ` Vlad Buslov
2023-01-20 11:30       ` Marcelo Ricardo Leitner

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=871qnke7ga.fsf@nvidia.com \
    --to=vladbu@nvidia.com \
    --cc=davem@davemloft.net \
    --cc=jhs@mojatatu.com \
    --cc=jiri@resnulli.us \
    --cc=kuba@kernel.org \
    --cc=marcelo.leitner@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=ozsh@nvidia.com \
    --cc=pabeni@redhat.com \
    --cc=pablo@netfilter.org \
    --cc=simon.horman@corigine.com \
    --cc=xiyou.wangcong@gmail.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.