From: Michael Walle <michael@walle.cc>
To: Steen Hegelund <steen.hegelund@microchip.com>
Cc: "David S . Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
UNGLinuxDriver@microchip.com,
Randy Dunlap <rdunlap@infradead.org>,
Casper Andersson <casper.casan@gmail.com>,
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>,
Dan Carpenter <error27@gmail.com>
Subject: Re: [PATCH net-next v2 0/8] Add support for two classes of VCAP rules
Date: Fri, 06 Jan 2023 15:18:08 +0100 [thread overview]
Message-ID: <01a84b0942f7af86d907ed39f5048b72@walle.cc> (raw)
In-Reply-To: <dc4c4fbf0cb1892dbe45c0ee80d5fafbd5fc36ff.camel@microchip.com>
Hi Steen,
>> > > Wouldn't it make more sense, to fix the regression via net (and
>> > > a Fixes: tag) and then make that stuff work without tc? Maybe
>> > > the fix is just reverting the commits.
>> >
>> > I have discussed this again with Horatiu and I have the following
>> > suggestion of
>> > how to proceed:
>> >
>> > 1) Create a small LAN966x specific patch for net (see below for the two
>> > possible
>> > variants).
>> >
>> > 2) Continue with a net-next V3 without any 'Fixes' tags on top of the
>> > patch in
>> > (1) when it becomes available in net-next.
>>
>> Sounds good.
>>
>> [coming back to this after writing the response below, so see there
>> for more context]
>> When do the patches from net become available in net-next? Only after
>> a
>> merge window? If so, depending on the solution for (1) you'd have two
>> "in-between" kernel versions (v6.2 and v6.3).
>
> According to our own experience the changes in net are usually merged
> into net-
> next the following Thursday: so not too much delay, before we can
> continue.
TIL :)
>> > The LAN966x patch for net (with a Fixes tag) could contain either:
>> >
>> > a) No check on enabled lookup
>> >
>> > Removal of the check for enabled lookups:
>> >
>> > - if (!ANA_VCAP_S2_CFG_ENA_GET(val))
>> > - return -ENOENT;
>> >
>> > This will remove the error that you have seen, but will still
>> > require a
>> > matchall rule to enable the PTP rules. This is compatible with the
>> > TC
>> > framework.
>> >
>> > b) Always enable lookups
>> >
>> > Enable the lookups at startup.
>> > Remove the lookup enable check as above.
>> >
>> > This will make the PTP rules (and any other rules) work even without
>> > the
>> > matchall rule to enable them. It its not ideal, but solves the
>> > problem that
>> > you have been experiencing without the 'TC magic'
>> >
>> > The V3 in net-next will provide the full solution.
>> >
>> > I expect that you might prefer the b) version.
>>
>> I *assume* linuxptp would have worked in my case (no bridge interface)
>> before Horatiu patches. As mentioned before, I haven't really tested
>> it.
>> Does that mean with a) the error is gone and linuxptp is working as
>> before? If so, I'm also fine with a).
>
> Yes this is the result: So I also suggest to go for solution a).
>
> This will still allow LinuxPTP to work (without the error that you have
> seen),
> but the bridged interface PTP support must be enabled with a TC
> matchall rule.
>
>>
>> Honestly, now that there is a good solution in future kernels, I
>> don't care toooo much about that one particular kernel. Other
>> users might disagree though ;)
>>
>> I just want to point out that right now you have some kind of
>> in-between kernel with 6.2:
>>
>> <=6.1 linuxptp working (but not on bridged ports)
>> 6.2 linuxptp working only with tc magic
>> 6.3 linuxptp working
>
> So with the LAN966x patch the second line would change to:
>
> 6.2 linuxptp working. PTP on bridged interfaces: needs TC matchall
> rule
>
>>
>> Therefore, I've raised the question if it's also viable to just
>> revert the former changes for 6.2. The you'd have a clean
>> transition.
>>
>> -michael
>
> TLDR Summary:
>
> 1) LAN966x patch for net to ensure PTP is working without errors
> 2) A V3 net-next VCAP series with the improvements for
> enabled/disable/permanent
> rules (both LAN966x and Sparx5)
>
> I will move forward with this.
Sounds perfect, thanks!
-michael
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
prev parent reply other threads:[~2023-01-06 14:19 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-06 8:53 [PATCH net-next v2 0/8] Add support for two classes of VCAP rules Steen Hegelund
2023-01-06 8:53 ` [PATCH net-next v2 1/8] net: microchip: vcap api: Erase VCAP cache before encoding rule Steen Hegelund
2023-01-06 8:53 ` [PATCH net-next v2 2/8] net: microchip: sparx5: Reset VCAP counter for new rules Steen Hegelund
2023-01-06 8:53 ` [PATCH net-next v2 3/8] net: microchip: vcap api: Always enable VCAP lookups Steen Hegelund
2023-01-06 8:53 ` [PATCH net-next v2 4/8] net: microchip: vcap api: Convert multi-word keys/actions when encoding Steen Hegelund
2023-01-06 8:53 ` [PATCH net-next v2 5/8] net: microchip: vcap api: Use src and dst chain id to chain VCAP lookups Steen Hegelund
2023-01-06 8:53 ` [PATCH net-next v2 6/8] net: microchip: vcap api: Check chains when adding a tc flower filter Steen Hegelund
2023-01-06 8:53 ` [PATCH net-next v2 7/8] net: microchip: vcap api: Add a storage state to a VCAP rule Steen Hegelund
2023-01-06 8:53 ` [PATCH net-next v2 8/8] net: microchip: vcap api: Enable/Disable rules via chains in VCAP HW Steen Hegelund
2023-01-06 8:56 ` [PATCH net-next v2 0/8] Add support for two classes of VCAP rules Dan Carpenter
2023-01-06 9:07 ` Michael Walle
2023-01-06 9:57 ` Steen Hegelund
2023-01-06 10:46 ` Michael Walle
2023-01-06 14:14 ` Steen Hegelund
2023-01-06 14:18 ` Michael Walle [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=01a84b0942f7af86d907ed39f5048b72@walle.cc \
--to=michael@walle.cc \
--cc=UNGLinuxDriver@microchip.com \
--cc=casper.casan@gmail.com \
--cc=daniel.machon@microchip.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=error27@gmail.com \
--cc=horatiu.vultur@microchip.com \
--cc=kuba@kernel.org \
--cc=lars.povlsen@microchip.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=nhuck@google.com \
--cc=pabeni@redhat.com \
--cc=rdunlap@infradead.org \
--cc=rmk+kernel@armlinux.org.uk \
--cc=steen.hegelund@microchip.com \
--cc=wanjiabing@vivo.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox