Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <e78cd61cdac6c1abb27b16908c241fd4f8310af9.camel@microchip.com>
Subject: Re: [PATCH net-next v2 2/5] net: microchip: sparx5: Adding more tc
 flower keys for the IS2 VCAP
From:   Steen Hegelund <steen.hegelund@microchip.com>
To:     Jakub Kicinski <kuba@kernel.org>
CC:     Casper Andersson <casper.casan@gmail.com>,
        "David S . Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Paolo Abeni <pabeni@redhat.com>,
        <UNGLinuxDriver@microchip.com>,
        Randy Dunlap <rdunlap@infradead.org>,
        Russell King <rmk+kernel@armlinux.org.uk>,
        "Wan Jiabing" <wanjiabing@vivo.com>,
        Nathan Huckleberry <nhuck@google.com>,
        <linux-kernel@vger.kernel.org>, <netdev@vger.kernel.org>,
        <linux-arm-kernel@lists.infradead.org>,
        Daniel Machon <daniel.machon@microchip.com>,
        Horatiu Vultur <horatiu.vultur@microchip.com>,
        Lars Povlsen <lars.povlsen@microchip.com>
Date:   Tue, 1 Nov 2022 20:32:13 +0100
In-Reply-To: <20221101084925.7d8b7641@kernel.org>
References: <20221028144540.3344995-1-steen.hegelund@microchip.com>
         <20221028144540.3344995-3-steen.hegelund@microchip.com>
         <20221031103747.uk76tudphqdo6uto@wse-c0155>
         <51622bfd3fe718139cece38493946c2860ebdf77.camel@microchip.com>
         <20221031184128.1143d51e@kernel.org>
         <741b628857168a6844b6c2e0482beb7df9b56520.camel@microchip.com>
         <20221101084925.7d8b7641@kernel.org>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.36.5-0ubuntu1 
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Precedence: bulk

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@microchip.com/
> 
> 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