netdev - Re: [PATCH v3 16/16] Documentation: net: octeontx2: Add RVU HW and drivers overview.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 20 Nov 2019 16:41:37 -0800
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     sunil.kovvuri@...il.com
Cc:     netdev@...r.kernel.org, davem@...emloft.net,
        Sunil Goutham <sgoutham@...vell.com>
Subject: Re: [PATCH v3 16/16] Documentation: net: octeontx2: Add RVU HW and
 drivers overview.

On Wed, 20 Nov 2019 23:18:06 +0530, sunil.kovvuri@...il.com wrote:
> From: Sunil Goutham <sgoutham@...vell.com>
> 
> Added high level overview of OcteonTx2 RVU HW and functionality of
> various drivers which will be upstreamed.
> 
> Signed-off-by: Sunil Goutham <sgoutham@...vell.com>

Please double check this renders the way you expect. You may want to
add empty lines before lists.

> diff --git a/Documentation/networking/device_drivers/index.rst b/Documentation/networking/device_drivers/index.rst
> index c1f7f75..8aba64b 100644
> --- a/Documentation/networking/device_drivers/index.rst
> +++ b/Documentation/networking/device_drivers/index.rst
> @@ -22,6 +22,7 @@ Contents:
>     intel/iavf
>     intel/ice
>     google/gve
> +   marvell/octeontx2
>     mellanox/mlx5
>     netronome/nfp
>     pensando/ionic
> diff --git a/Documentation/networking/device_drivers/marvell/octeontx2.rst b/Documentation/networking/device_drivers/marvell/octeontx2.rst
> new file mode 100644
> index 0000000..c8a5150
> --- /dev/null
> +++ b/Documentation/networking/device_drivers/marvell/octeontx2.rst
> @@ -0,0 +1,162 @@
> +.. SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +
> +=============================================
> +Marvell OcteonTx2 RVU Kernel Drivers
> +=============================================

Shouldn't these lines be the length of the text?

> +
> +Copyright (c) 2019 Marvell International Ltd.
> +
> +Contents
> +========
> +
> +- `Overview`_
> +- `Drivers`_
> +- `Basic packet flow`_
> +
> +Overview
> +========
> +Resource virtualization unit (RVU) on Marvell's OcteonTX2 SOC maps HW
> +resources from the network, crypto and other functional blocks into
> +PCI-compatible physical and virtual functions. Each functional block
> +again has multiple local functions (LFs) for provisioning to PCI devices.
> +RVU supports multiple PCIe SRIOV physical functions (PFs) and virtual
> +functions (VFs). PF0 is called the administrative / admin function (AF)
> +and has privileges to provision RVU functional block's LFs to each of the
> +PF/VF.
> +
> +RVU managed networking functional blocks
> + - Network pool or buffer allocator (NPA)
> + - Network interface controller (NIX)
> + - Network parser CAM (NPC)
> + - Schedule/Synchronize/Order unit (SSO)
> + - Loopback interface (LBK)
> +
> +RVU managed non-networking functional blocks
> + - Crypto accelerator (CPT)
> + - Scheduled timers unit (TIM)
> + - Schedule/Synchronize/Order unit (SSO)
> +   Used for both networking and non networking usecases
> +
> +Resource provisioning examples
> + - A PF/VF with NIX-LF & NPA-LF resources works as a pure network device
> + - A PF/VF with CPT-LF resource works as a pure cyrpto offload device.

s/cyrpto/crypto/

> +
> +.. kernel-figure::  resource_virtualization_unit.svg
> +   :alt:	RVU
> +   :align:	center
> +   :figwidth:	60em
> +
> +   RVU HW block connectivity

The diagram isn't really bringing much value if you ask me. The text in
the last section is quite a bit better. Perhaps show packet flow?

> +RVU functional blocks are highly configurable as per software requirements.
> +
> +Firmware setups following stuff before kernel boots
> + - Enables required number of RVU PFs based on number of physical links.
> + - Number of VFs per PF are either static or configurable at compile time.

compile time of the firmware?

> +   Based on config, firmware assigns VFs to each of the PFs.
> + - Also assigns MSIX vectors to each of PF and VFs.
> + - These are not changed after kernel boot.

Can they be changed without FW rebuild?

> +Drivers
> +=======
> +
> +Linux kernel will have multiple drivers registering to different PF and VFs
> +of RVU. Wrt networking there will be 3 flavours of drivers.
> +
> +Admin Function driver
> +---------------------

> +Physical Function driver
> +------------------------

Thanks for the description, I was hoping you'd also provide more info
on how the software componets of the system fit together. Today we only
have an AF driver upstream. Without the PF or VF drivers the card is
pretty much unusable with only the upstream drivers, right?

There is a bunch of cgx_* exports in the AF module, which seem to have
no uses upstream, too (they are only called from rvu_cgx.c which is
compiled into the same module).

We'd like to see you build up a self-sufficient upstream-only solution,
and adding more and more code to the AF driver with unused exports
doesn't really inspire confidence this is the direction.