On 17/01/2023 13:40, Jamal Hadi Salim wrote:

I agree that with your patch it will be operationally simpler. I hope other
vendors will be able to use this feature (and the only reason i am saying
this is because you are making core tc changes).

FTR at AMD/sfc we wouldn't use this as our HW has all action execution after
 all match stages in the pipeline (excepting actions that only control match
 behaviour, i.e. ct lookup), so users on ef100 HW (and I'd imagine probably
 some other vendors' products too) would still need to rewrite their rules
 with skbmark.
I mention this because this feature / patch series disconcerts me.  I wasn't
 even really happy about the 'miss to chain' feature, but even more so 'miss
 to action' feels like it makes the TC-driver offload interface more complex
 than it really ought to be.
Especially because the behaviour in some cases is already weird even with a
 fully offloadable rule; consider a match-all filter with 'action vlan push'
 and no further actions (specifically no redirect).  AIUI the HW will push
 the vlan, then deliver to the default destination (e.g. repr if the packet
 came from a VF), at which point TC SW will apply the same rule and perform
 the vlan push again, leading (incorrectly) to a double-tagged packet.
So it's not really about 'miss', there's a more fundamental issue with how
 HW offload and the SW path interact.  And I don't think it's possible to
 guaranteed-remove that issue without a performance cost (skb ext is
 expensive and we don't want it on *every* RX packet), so users will always
 need to consider this sort of thing when writing their TC rules.

TBH doing an IP address pedit before a CT lookup seems like a fairly
 contrived use case to me.  Is there a pressing real-world need for it, or
 are mlx just adding this support because they can?

-ed