marvell: prestera: Update prestera driver to v2.8.0 for kernel 5.10

[taraschornyiplv]

The switchdev driver for the Prestera family ASIC was added in Linux 5.10 by commit torvalds/linux@501ef3066c (net: marvell: prestera: Add driver for Prestera family ASIC devices),

The following features are supported by this driver:

- VLAN-aware bridge offloading
- VLAN-unaware bridge offloading
- FDB offloading (learning, ageing)
- Switchport configuration

For the DentOS Arthur release, an out-of-the tree driver was added as a patch for kernel 5.6, which supports additionaly following features:

- IPv4 forwarding
- Static Routing
- Dynamic Routing – BGP
- STP
- VRRP – keepaliveD
- LLDP
- LAG/LACP
- DHCP
- L2/L3 ECMP
- L2/L3/L4 ACL (TC-Flower)
- Control plane Policer
- Phylink support 

With this pull request, in-tree driver is updated to include all features and bug fixes that are present in Arthur release.

NOTE:
Prestera switchdev driver name has been changed to align with
the upstream kernel. The new driver name is prestera instead of
prestera_sw

Signed-off-by: Taras Chornyi taras.chornyi@plvision.eu

[sonoble]

We will need to update the python boot scripts to load the module's new name. I think we can include both during the transition period.

packages/platforms/delta/arm64/tn48m/tn48m/platform-config/r0/src/python/arm64_delta_tn48m_r0/init.py

[taraschornyiplv]

It's not just per-platform script:
we also need to change folder name from prestera_sw to prestera in packages/base/all/vendor-config-onl/src/python/onl/platform/base.py#L226.
Alternatively, we can replace insmod with modprobe and probe only prestera_pci module. because it has a dependency on prestera module both will be loaded

[paulmenzel]

I tried to reviewed, but had to spend several minutes to know what is going on, especially as GitHub is unable to display the big patch file, and so it’s easily missed. It’d be awesome, if you could be much more elaborate. Here is the git patch header from the big commit:

From: Taras Chornyi <taras.chornyi@plvision.eu>
Date: Wed, 10 Feb 2021 10:10:22 +0200
Subject: [PATCH] net: prestera: Update prestera driver

New features:
* Ipv4 offload
* TC offload
* phylink support
* ECMP offload

Co-developed-by: Andrii Savka <andrii.savka@plvision.eu>
Signed-off-by: Andrii Savka <andrii.savka@plvision.eu>
Co-developed-by: Oleksandr Mazur <oleksandr.mazur@plvision.eu>
Signed-off-by: Oleksandr Mazur <oleksandr.mazur@plvision.eu>
Co-developed-by: Serhiy Boiko <serhiy.boiko@plvision.eu>
Signed-off-by: Serhiy Boiko <serhiy.boiko@plvision.eu>
Co-developed-by: Serhiy Pshyk <serhiy.pshyk@plvision.eu>
Signed-off-by: Serhiy Pshyk <serhiy.pshyk@plvision.eu>
Co-developed-by: Volodymyr Mytnyk <volodymyr.mytnyk@plvision.eu>
Signed-off-by: Volodymyr Mytnyk <volodymyr.mytnyk@plvision.eu>
Co-developed-by: Vadym Kochan <vadym.kochan@plvision.eu>
Signed-off-by: Vadym Kochan <vadym.kochan@plvision.eu>
Signed-off-by: Taras Chornyi <taras.chornyi@plvision.eu>
---
 .../net/ethernet/marvell/prestera/Makefile    |   10 +-
 .../net/ethernet/marvell/prestera/prestera.h  |  544 ++-
 .../ethernet/marvell/prestera/prestera_acl.c  |  402 +++
 .../marvell/prestera/prestera_debugfs.c       |  160 +
 .../marvell/prestera/prestera_debugfs.h       |   14 +
 .../marvell/prestera/prestera_devlink.c       |   32 +-
 .../marvell/prestera/prestera_devlink.h       |   18 +-
 .../marvell/prestera/prestera_drv_ver.h       |   23 +
 .../ethernet/marvell/prestera/prestera_dsa.c  |  340 +-
 .../ethernet/marvell/prestera/prestera_dsa.h  |   70 +-
 .../marvell/prestera/prestera_ethtool.c       |  780 -----
 .../marvell/prestera/prestera_ethtool.h       |   11 -
 .../marvell/prestera/prestera_flower.c        |  430 +++
 .../marvell/prestera/prestera_fw_log.c        |  422 +++
 .../marvell/prestera/prestera_fw_log.h        |   15 +
 .../ethernet/marvell/prestera/prestera_hw.c   | 2483 +++++++++----
 .../ethernet/marvell/prestera/prestera_hw.h   |  428 ++-
 .../ethernet/marvell/prestera/prestera_log.c  |  203 ++
 .../ethernet/marvell/prestera/prestera_log.h  |   58 +
 .../ethernet/marvell/prestera/prestera_main.c | 2599 ++++++++++++--
 .../ethernet/marvell/prestera/prestera_pci.c  |  901 +++--
 .../marvell/prestera/prestera_router.c        | 3069 +++++++++++++++++
 .../ethernet/marvell/prestera/prestera_rxtx.c |  878 +----
 .../ethernet/marvell/prestera/prestera_rxtx.h |   35 +-
 .../marvell/prestera/prestera_rxtx_priv.h     |   61 +
 .../marvell/prestera/prestera_rxtx_sdma.c     |  769 +++++
 .../marvell/prestera/prestera_switchdev.c     | 2058 ++++++-----
 .../marvell/prestera/prestera_switchdev.h     |   13 -
 28 files changed, 12625 insertions(+), 4201 deletions(-)

Where can the versions be looked up? Without that Update is misleading. What is v2.8.0?

It’s good to start with some background. Maybe something like:

The switchdev driver for the Prestera family ASIC was added in Linux 5.10 by commit 501ef3066c (net: marvell: prestera: Add driver for Prestera family ASIC devices), and in DentOS the option was currently not enabled for the 5.10 kernel, and the patch just copied from the 5.6 but disabled in the series file.

So, I’d put the commit marvell: prestera: enable Prestera driver (and add Linux 5.10) in there first, and also remove the patch file, as that’s what is in the Linux kernel sources now.

Then, I’d add the updated driver wtih a new patch name, so they are not confused with the file for Linux 5.6. Just use the name created by git format-patch, which takes the git commit message summary.

For the updated driver, give background, to where the version numbers come from. For example, what version is in Linux 5.10 upstream? Also, add the URL, where you sent the updated driver to Linux upstream for review, and when it will be expected in Linux upstream. And lastly, how did you test the new driver and features (something which should be also added to the patch file).

[paulmenzel]

Also, I assume for Linux kernel upstream review you split this up into several commits? For example, why is the ethtool interface (commit a97d3c6939 (net: marvell: prestera: Add ethtool interface support)) removed?

 .../marvell/prestera/prestera_ethtool.c       |  780 -----
 .../marvell/prestera/prestera_ethtool.h       |   11 -

I’d would prefer if you did not add one big patch, but the patch series sent to Linux kernel upstream.

[paulmenzel]

The copyright lines should also be just one line.

[taraschornyiplv]

Not all prestera driver features were upstreamed to the mainline kernel. we are working with the community on upstreaming more features. Latest patch series https://patchwork.kernel.org/project/netdevbpf/cover/20210203165458.28717-1-vadym.kochan@plvision.eu/
in mainline kernel prestera driver version 2.0
with this pull request driver version in kernel sources was updated to version 2.8

[paulmenzel]

Thank you for the update and answers. What did you change in your last push?

So “version” refers to the firmware version? As only mrvl/prestera/mvsw_prestera_fw-v2.0.img is in the linux-firmware repository, where is DentOS getting firmware version 2.8? Why isn’t that documented in the merge/pull request or commits?

So why not add “version 2.5” [net-next,0/7] Marvell Prestera Switchdev misc updates first, and then “version 2.8” second? And why is version 2.8 not submitted for review upstream? Who is supposed to review an almost 17,000 line diffstat?

[paulmenzel]

Does Marvell have a public Linux kernel repository where this version 2.8 patch is part of?

[taraschornyiplv]

version is basically the driver version. The driver itself has a dependency on a minimal FW version. Upstream is several versions behind.
We are upstreaming feature per feature to mainline kernel. Because for DentOS we need all features at once I've updated kernel driver that is currently in 5.10 to the latest codebase.
There is a repo with an out-of-tree driver https://github.com/Marvell-switching/switchdev-prestera

[taraschornyiplv]

as for the firmware image it's already integrated to the main branch with this commit 59ebd04

[paulmenzel]

as for the firmware image it's already integrated to the main branch with this commit 59ebd04

(Too bad the text part in the GitHub notification emails do not contain the URL.)

Ah, would be nice to mention that too.

[paulmenzel]

Without

1. rewritten commit messages and pull/merge request description to add more background and references to the upstream changes,
2. comprehensible commit ordering and patch file changes (first 2.5 then 2.8)
3. test report

I am unable to approve such a big change, which cannot be reviewed because it’s too big (and the GitHub interface is unable to deal with it).

[sonoble]

Marvell will work on a better way to patch the kernel. At this point we are blocked so we are relying on the testing Amazon and Marvell have done.

[paulmenzel]

Yes, I understand that. But the way, how it’s added to DentOS leaves much to be desired, and at least the commit messages and pull/merge request description could have been improved before merging this. :(

[Mickey201]

Hi Paul, we are fully aligned with your wise comments and intentions; And still – we are doing our best to move between these comments to our other commitments. I can assure you we will do our best to address at least most of it – but it will take some time. Regards, Mickey