[net,0/8] Add support for two classes of VCAP rules

Message ID 20221221132517.2699698-1-steen.hegelund@microchip.com
Headers
Series Add support for two classes of VCAP rules |

Message

Steen Hegelund Dec. 21, 2022, 1:25 p.m. UTC
  This adds support for two classes of VCAP rules:

- Permanent rules (added e.g. for PTP support)
- TC user rules (added by the TC userspace tool)

For this to work the VCAP Loopups must be enabled from boot, so that the
"internal" clients like PTP can add rules that are always active.

When the TC tool add a flower filter the VCAP rule corresponding to this
filter will be disabled (kept in memory) until a TC matchall filter creates
a link from chain 0 to the chain (lookup) where the flower filter was
added.

When the flower filter is enabled it will be written to the appropriate
VCAP lookup and become active in HW.

Likewise the flower filter will be disabled if there is no link from chain
0 to the chain of the filter (lookup), and when that happens the
corresponding VCAP rule will be read from the VCAP instance and stored in
memory until it is deleted or enabled again.

Steen Hegelund (8):
  net: microchip: vcap api: Erase VCAP cache before encoding rule
  net: microchip: sparx5: Reset VCAP counter for new rules
  net: microchip: vcap api: Always enable VCAP lookups
  net: microchip: vcap api: Convert multi-word keys/actions when
    encoding
  net: microchip: vcap api: Use src and dst chain id to chain VCAP
    lookups
  net: microchip: vcap api: Check chains when adding a tc flower filter
  net: microchip: vcap api: Add a storage state to a VCAP rule
  net: microchip: vcap api: Enable/Disable rules via chains in VCAP HW

 .../ethernet/microchip/lan966x/lan966x_goto.c |  10 +-
 .../ethernet/microchip/lan966x/lan966x_main.h |   3 +-
 .../microchip/lan966x/lan966x_tc_flower.c     |  30 +-
 .../microchip/lan966x/lan966x_tc_matchall.c   |  16 +-
 .../microchip/lan966x/lan966x_vcap_impl.c     |  24 +-
 .../microchip/sparx5/sparx5_tc_flower.c       |  28 +-
 .../microchip/sparx5/sparx5_tc_matchall.c     |  16 +-
 .../microchip/sparx5/sparx5_vcap_debugfs.c    |   2 +-
 .../microchip/sparx5/sparx5_vcap_impl.c       |  29 +-
 .../net/ethernet/microchip/vcap/vcap_api.c    | 752 +++++++++++++-----
 .../net/ethernet/microchip/vcap/vcap_api.h    |   5 -
 .../ethernet/microchip/vcap/vcap_api_client.h |   8 +-
 .../microchip/vcap/vcap_api_debugfs.c         |  57 +-
 .../microchip/vcap/vcap_api_debugfs_kunit.c   |  10 +-
 .../ethernet/microchip/vcap/vcap_api_kunit.c  |  32 +-
 .../microchip/vcap/vcap_api_private.h         |  12 +-
 16 files changed, 685 insertions(+), 349 deletions(-)
  

Comments

Paolo Abeni Dec. 22, 2022, 2:22 p.m. UTC | #1
Hello,
On Wed, 2022-12-21 at 14:25 +0100, Steen Hegelund wrote:
> This adds support for two classes of VCAP rules:
> 
> - Permanent rules (added e.g. for PTP support)
> - TC user rules (added by the TC userspace tool)
> 
> For this to work the VCAP Loopups must be enabled from boot, so that the
> "internal" clients like PTP can add rules that are always active.
> 
> When the TC tool add a flower filter the VCAP rule corresponding to this
> filter will be disabled (kept in memory) until a TC matchall filter creates
> a link from chain 0 to the chain (lookup) where the flower filter was
> added.
> 
> When the flower filter is enabled it will be written to the appropriate
> VCAP lookup and become active in HW.
> 
> Likewise the flower filter will be disabled if there is no link from chain
> 0 to the chain of the filter (lookup), and when that happens the
> corresponding VCAP rule will be read from the VCAP instance and stored in
> memory until it is deleted or enabled again.

Despite the 'net' target, this looks really like net-next material as
most patches look like large refactor. I see there are a bunch of fixes
in patches 3-8, but quite frankly it's not obvious at all what the
refactors/new features described into the commit messages themself
really fix.

I suggest to move this series to net-next (and thus repost after Jan
2), unless you come-up with some good reasons to keep it in net.

Thanks,

Paolo
  
Steen Hegelund Dec. 22, 2022, 3:02 p.m. UTC | #2
Hi Paolo,

On Thu, 2022-12-22 at 15:22 +0100, Paolo Abeni wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
> 
> Hello,
> On Wed, 2022-12-21 at 14:25 +0100, Steen Hegelund wrote:
> > This adds support for two classes of VCAP rules:
> > 
> > - Permanent rules (added e.g. for PTP support)
> > - TC user rules (added by the TC userspace tool)
> > 
> > For this to work the VCAP Loopups must be enabled from boot, so that the
> > "internal" clients like PTP can add rules that are always active.
> > 
> > When the TC tool add a flower filter the VCAP rule corresponding to this
> > filter will be disabled (kept in memory) until a TC matchall filter creates
> > a link from chain 0 to the chain (lookup) where the flower filter was
> > added.
> > 
> > When the flower filter is enabled it will be written to the appropriate
> > VCAP lookup and become active in HW.
> > 
> > Likewise the flower filter will be disabled if there is no link from chain
> > 0 to the chain of the filter (lookup), and when that happens the
> > corresponding VCAP rule will be read from the VCAP instance and stored in
> > memory until it is deleted or enabled again.
> 
> Despite the 'net' target, this looks really like net-next material as
> most patches look like large refactor. I see there are a bunch of fixes
> in patches 3-8, but quite frankly it's not obvious at all what the
> refactors/new features described into the commit messages themself
> really fix.

Yes the patches 3-8 is the response to Michael Walles observations on LAN966x
and Jakubs Kicinski comment (see link), but the description in the commits may
not be that clear, in the sense that they do not state one-to-one what the
mitigation is.

See https://lore.kernel.org/netdev/20221209150332.79a921fd@kernel.org/

So essentially this makes it possible to have rules that are always in the VCAP
HW (to make the PTP feature work), even before the TC chains have been
established (which was the problem that Michael encountered).

I still think this a net submission, since it fixes the problem that was
observed in the previous netnext window.

But I will rephrase the reasoning in a V2 to hopefully make that more
understandable.

If you still think it is better to post this in the upcoming net-next window, I
am also OK with that.

> 
> I suggest to move this series to net-next (and thus repost after Jan
> 2), unless you come-up with some good reasons to keep it in net.
> 
> Thanks,
> 
> Paolo
> 

BR
Steen
  
Paolo Abeni Dec. 22, 2022, 4:12 p.m. UTC | #3
On Thu, 2022-12-22 at 16:02 +0100, Steen Hegelund wrote:
> On Thu, 2022-12-22 at 15:22 +0100, Paolo Abeni wrote:
> > Despite the 'net' target, this looks really like net-next material as
> > most patches look like large refactor. I see there are a bunch of fixes
> > in patches 3-8, but quite frankly it's not obvious at all what the
> > refactors/new features described into the commit messages themself
> > really fix.
> 
> Yes the patches 3-8 is the response to Michael Walles observations on LAN966x
> and Jakubs Kicinski comment (see link), but the description in the commits may
> not be that clear, in the sense that they do not state one-to-one what the
> mitigation is.
> 
> See https://lore.kernel.org/netdev/20221209150332.79a921fd@kernel.org/
> 
> So essentially this makes it possible to have rules that are always in the VCAP
> HW (to make the PTP feature work), even before the TC chains have been
> established (which was the problem that Michael encountered).
> 
> I still think this a net submission, since it fixes the problem that was
> observed in the previous netnext window.
> 
> But I will rephrase the reasoning in a V2 to hopefully make that more
> understandable.
> 
> If you still think it is better to post this in the upcoming net-next window, I
> am also OK with that.

IMHO this series is quite too invasive for net, especially considering
it will possibly land into the Linus tree with a timeframe promising a
large latency in response to any problem.

If there is any kind of available workaround to address the issue
(comprising disabling h/w offload) I *think* net-next would be a better
option.

Cheers,

Paolo