Message-ID: <c11cf7d6-4ebc-2d90-0bad-503ac092d821@nvidia.com>
Date:   Sun, 22 Jan 2023 11:33:35 +0200
From:   Paul Blakey <paulb@...dia.com>
To:     Edward Cree <ecree.xilinx@...il.com>,
        Jamal Hadi Salim <jhs@...atatu.com>
Cc:     netdev@...r.kernel.org, Saeed Mahameed <saeedm@...dia.com>,
        Paolo Abeni <pabeni@...hat.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Eric Dumazet <edumazet@...gle.com>,
        Cong Wang <xiyou.wangcong@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Oz Shlomo <ozsh@...dia.com>, Jiri Pirko <jiri@...dia.com>,
        Roi Dayan <roid@...dia.com>, Vlad Buslov <vladbu@...dia.com>
Subject: Re: [PATCH net-next 0/4] net/sched: cls_api: Support hardware miss to
 tc action



On 20/01/2023 13:54, Edward Cree wrote:
> 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.



If I understand correctly, a "action vlan push" which in SW would return 
OK to end classification or CONTINUE to continue to next prio (not sure 
what the default for this action is), then we don't offload such rules, 
as we require a endpoint (goto/drop/mirred), but if we did, I guess we 
would have used this series to point to end of the last run action list 
continuing as software would (returning OK or CONTINUE in the right 
context) or not do the vlan push and jump right before it saving having 
to match again. This might need a small change to allow jumping to the 
end of the action list.

> 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.

We have the skb ext only when hardware already did some stuff (offloaded 
something), so what ever it did hopefully pays for using the skb ext, 
and I think this is the case. And of course we strive for full offload 
so this case would be an exception.

> 
> 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

Of course, we see when users want to set up stateless NAT outside of 
action CT.

Paul.

Powered by blists - more mailing lists