Re: [PATCH net-next v2 2/5] net: microchip: sparx5: Adding more tc flower keys for the IS2 VCAP

[Steen Hegelund]

Hi Jacub,

Thanks for the comments.

On Tue, 2022-11-01 at 08:49 -0700, Jakub Kicinski wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Tue, 1 Nov 2022 08:31:16 +0100 Steen Hegelund wrote:
> > > Previous series in this context means previous revision or something
> > > that was already merged?
> > 
> > Casper refers to this series (the first of the VCAP related series) that was merged on Oct 24th:
> > 
> > https://lore.kernel.org/all/20221020130904.1215072-1-steen.hegelund@xxxxxxxxxxxxx/
> 
> Alright, looks like this is only in net-next so no risk of breaking
> existing users.

Yes, this is a new feature.

> 
> That said you should reject filters you can't support with an extack
> message set. Also see below.

That should also be the case.  

I just checked that using an unsupported key, action or chain will result in an extack error
message.

> 
> > > > tc filter add dev eth3 ingress chain 8000000 prio 10 handle 10 \
> > > 
> > > How are you using chains?
> > 
> > The chain ids are referring to the VCAP instances and their lookups.  There are some more
> > details
> > about this in the series I referred to above.
> > 
> > The short version is that this allows you to select where in the frame processing flow your rule
> > will be inserted (using ingress or egress and the chain id).
> > 
> > > I thought you need to offload FLOW_ACTION_GOTO to get to a chain,
> > > and I get no hits on this driver.
> > 
> > I have not yet added the goto action, but one use of that is to chain a filter from one VCAP
> > instance/lookup to another.
> > 
> > The goto action will be added in a soon-to-come series.  I just wanted to avoid a series getting
> > too
> > large, but on the other hand each of them should provide functionality that you can use in
> > practice.
> 
> The behavior of the offload must be the same as the SW implementation.
> It sounds like in your case it very much isn't, as adding rules to
> a magic chain in SW, without the goto will result in the rules being
> unused.

I will add the goto support to my implementation so that it will be consistent with the SW
implementation, adding a check to ensure that there is a goto action when HW offloading.

BR
Steen