netdev - Re: [patch net-next 00/11] mlxsw: Implement dev info and dev flash for line cards

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, 15 Jun 2022 12:13:35 +0300
From:   Ido Schimmel <idosch@...dia.com>
To:     Jiri Pirko <jiri@...nulli.us>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, kuba@...nel.org,
        petrm@...dia.com, pabeni@...hat.com, edumazet@...gle.com,
        mlxsw@...dia.com
Subject: Re: [patch net-next 00/11] mlxsw: Implement dev info and dev flash
 for line cards

On Tue, Jun 14, 2022 at 02:33:15PM +0200, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@...dia.com>
> 
> This patchset implements two features:
> 1) "devlink dev info" is exposed for line card (patches 3-8)
> 2) "devlink dev flash" is implemented for line card gearbox
>    flashing (patch 9)
> 
> For every line card, "a nested" auxiliary device is created which
> allows to bind the features mentioned above (patch 2).

The design choice to use an auxiliary device for each line card needs to
be explained in the cover letter. From what I can tell, the motivation
is to reuse the above devlink uAPI for line cards as opposed to using
the "component" attribute or adding new uAPI. This is achieved by
creating a devlink instance for each line card. The auxiliary device is
needed because each devlink instance is expected to be associated with a
device. Does this constitute proper use of the auxiliary bus?

> 
> The relationship between line card and its auxiliary dev devlink
> is carried over extra line card netlink attribute (patches 1 and 3).
> 
> Examples:
> 
> $ devlink lc show pci/0000:01:00.0 lc 1
> pci/0000:01:00.0:
>   lc 1 state active type 16x100G nested_devlink auxiliary/mlxsw_core.lc.0

Can we try to use the index of the line card as the identifier of the
auxiliary device?

>     supported_types:
>        16x100G
> 
> $ devlink dev show auxiliary/mlxsw_core.lc.0
> auxiliary/mlxsw_core.lc.0

I assume that the auxiliary device cannot outlive line card. Does that
mean that as part of reload of the primary devlink instance the nested
devlink instance is removed? If so, did you check the reload flow with
lockdep to ensure there aren't any problems?

> 
> $ devlink dev info auxiliary/mlxsw_core.lc.0
> auxiliary/mlxsw_core.lc.0:
>   versions:
>       fixed:
>         hw.revision 0
>         fw.psid MT_0000000749
>       running:
>         ini.version 4
>         fw 19.2010.1312
> 
> $ devlink dev flash auxiliary/mlxsw_core.lc.0 file mellanox/fw-AGB-rel-19_2010_1312-022-EVB.mfa2

How is this firmware activated? It is usually done after reload, but I
don't see reload implementation for the line card devlink instance.

> 
> Jiri Pirko (11):
>   devlink: introduce nested devlink entity for line card
>   mlxsw: core_linecards: Introduce per line card auxiliary device
>   mlxsw: core_linecard_dev: Set nested devlink relationship for a line
>     card
>   mlxsw: core_linecards: Expose HW revision and INI version
>   mlxsw: reg: Extend MDDQ by device_info
>   mlxsw: core_linecards: Probe provisioned line cards for devices and
>     expose FW version
>   mlxsw: reg: Add Management DownStream Device Tunneling Register
>   mlxsw: core_linecards: Expose device PSID over device info
>   mlxsw: core_linecards: Implement line card device flashing
>   selftests: mlxsw: Check line card info on provisioned line card
>   selftests: mlxsw: Check line card info on activated line card
> 
>  Documentation/networking/devlink/mlxsw.rst    |  24 ++
>  drivers/net/ethernet/mellanox/mlxsw/Kconfig   |   1 +
>  drivers/net/ethernet/mellanox/mlxsw/Makefile  |   2 +-
>  drivers/net/ethernet/mellanox/mlxsw/core.c    |  44 +-
>  drivers/net/ethernet/mellanox/mlxsw/core.h    |  35 ++
>  .../mellanox/mlxsw/core_linecard_dev.c        | 180 ++++++++
>  .../ethernet/mellanox/mlxsw/core_linecards.c  | 403 ++++++++++++++++++
>  drivers/net/ethernet/mellanox/mlxsw/reg.h     | 173 +++++++-
>  include/net/devlink.h                         |   2 +
>  include/uapi/linux/devlink.h                  |   2 +
>  net/core/devlink.c                            |  42 ++
>  .../drivers/net/mlxsw/devlink_linecard.sh     |  54 +++
>  12 files changed, 948 insertions(+), 14 deletions(-)
>  create mode 100644 drivers/net/ethernet/mellanox/mlxsw/core_linecard_dev.c
> 
> -- 
> 2.35.3
>