| Message ID | 20221221132517.2699698-1-steen.hegelund@microchip.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add support for two classes of VCAP rules | expand |
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
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
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
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(-)