ACK/Cmnt: [SRU][J:linux-bluefield][PATCH v3 00/11] Support hardware miss to tc action

[Tim Gardner]

On 6/6/23 5:01 PM, William Tu wrote:
> Currently tc miss interface only supports resuming from a specific tc chain.
> If a packet modification is done before a missable action such as CT and
> there is a miss in CT after it, this may cause a miss match when resuming
> re-executing the same chain in software, and wrong packet count.
>   This use case for example is a stateless (static) nat.
> 
> * brief explanation of fixes
> Add support for missing to a specific action instance, and support
> of per action hardware stats to update what was actually done in hardware.
> 
> BugLink: https://bugs.launchpad.net/bugs/2012571
> 
> v2->v3:
> - remove mention of "linux-next" from the cherry-pick line of commit
9b7c68b3911a
> - include 3 fixes for commit 80cd22c35c90 ("net/sched: cls_api: Support
hardware miss to tc action")
> commit 2cc8a008d62f ("net/sched: cls_api: Initialize
miss_cookie_node when action miss is not used")

> commit 338469d677e5 ("net/sched: clear actions pointer in miss
cookie init fail")

> commit 37e1f3acc339 ("net/sched: cls_api: Move call to
tcf_exts_miss_cookie_base_destroy()")

> - Add:
> commit dfd2f0eb2347 ("net/sched: flower: fix
fl_change() error recovery path")
> and makes 593e1c66f994 ("net/sched: flower: fix error handler on replace") backported
> 
> v1->v2:
> add two more patches from Vlad, show below
> 
> 
> Eric Dumazet (1):
>    net/sched: flower: fix fl_change() error recovery path
> 
> Ivan Vecera (1):
>    net/sched: cls_api: Initialize miss_cookie_node when action miss is
>      not used
> 
> Nathan Chancellor (1):
>    net/sched: cls_api: Move call to tcf_exts_miss_cookie_base_destroy()
> 
> Paul Blakey (5):
>    UBUNTU: SAUCE: net/sched: Provide act to offload action
>    net/sched: cls_api: Support hardware miss to tc action
>    net/sched: flower: Move filter handle initialization earlier
>    net/sched: flower: Support hardware miss to tc action
>    netfilter: ctnetlink: Support offloaded conntrack entry deletion
> 
> Pedro Tammela (1):
>    net/sched: clear actions pointer in miss cookie init fail
> 
> Vlad Buslov (2):
>    net/sched: flower: fix filter idr initialization
>    net/sched: flower: fix error handler on replace
> 
>   include/linux/skbuff.h               |   6 +-
>   include/net/flow_offload.h           |   2 +
>   include/net/pkt_cls.h                |  30 ++--
>   include/net/sch_generic.h            |   2 +
>   net/netfilter/nf_conntrack_netlink.c |   8 -
>   net/openvswitch/flow.c               |   3 +-
>   net/sched/cls_api.c                  | 220 +++++++++++++++++++++++++--
>   net/sched/cls_flower.c               |  81 ++++++----
>   8 files changed, 293 insertions(+), 59 deletions(-)
> 
Acked-by: Tim Gardner <tim.gardner at canonical.com>

1) Isn't it a little weird to have your reviewed-by signature in the 
same patch as your s-o-b ? See "UBUNTU: SAUCE: net/sched: Provide act to 
offload action"

2) Sometimes the chain of provenance is a little confusing. "net/sched: 
flower: fix filter idr initialization" does not have the 'cherry picked' 
line in the right place, which makes me suspect it was inserted 
manually. Similarly for "net/sched: flower: fix error handler on 
replace". If its going to be a clean cherry-pick, then always get it 
from the upstream kernel, and use 'git cherry-pick -s -x'. Patches with 
Paul Blakely in the chain of provenance after the cherry pick line add 
no value.

3) "netfilter: ctnetlink: Support offloaded conntrack entry deletion" 
has a different Buglink then the rest of the patches. That is going to 
look strange in the changelog.

I'll leave it up to Bart as to whether he NAKs this.

rtg
-- 
-----------
Tim Gardner
Canonical, Inc