netdev - Re: [PATCH net-next v3 4/7] netfilter: flowtable: allow updating offloaded rules asynchronously

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Y8p96knLDtxnRtjz@salvia>
Date:   Fri, 20 Jan 2023 12:41:30 +0100
From:   Pablo Neira Ayuso <pablo@...filter.org>
To:     Vlad Buslov <vladbu@...dia.com>
Cc:     davem@...emloft.net, kuba@...nel.org, pabeni@...hat.com,
        netdev@...r.kernel.org, netfilter-devel@...r.kernel.org,
        jhs@...atatu.com, xiyou.wangcong@...il.com, jiri@...nulli.us,
        ozsh@...dia.com, marcelo.leitner@...il.com,
        simon.horman@...igine.com
Subject: Re: [PATCH net-next v3 4/7] netfilter: flowtable: allow updating
 offloaded rules asynchronously

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.

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.

Thanks.