All of lore.kernel.org
 help / color / mirror / Atom feed
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

WARNING: multiple messages have this Message-ID (diff)
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

  reply	other threads:[~2023-01-06 14:18 UTC|newest]

Thread overview: 30+ 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 ` 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   ` 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   ` 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   ` 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   ` 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   ` 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   ` 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   ` 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:53   ` 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  8:56   ` Dan Carpenter
2023-01-06  9:07 ` Michael Walle
2023-01-06  9:07   ` Michael Walle
2023-01-06  9:57   ` Steen Hegelund
2023-01-06  9:57     ` Steen Hegelund
2023-01-06 10:46     ` Michael Walle
2023-01-06 10:46       ` Michael Walle
2023-01-06 14:14       ` Steen Hegelund
2023-01-06 14:14         ` Steen Hegelund
2023-01-06 14:18         ` Michael Walle [this message]
2023-01-06 14:18           ` Michael Walle

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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.