netdev - Re: [PATCH net-next 0/8] devlink: support direct read from region

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] [day] [month] [year] [list]

Message-ID: <68042dc7-0025-48bc-07e9-7051494ba2e5@intel.com>
Date:   Wed, 23 Nov 2022 12:51:39 -0800
From:   Jacob Keller <jacob.e.keller@...el.com>
To:     <netdev@...r.kernel.org>
CC:     Jiri Pirko <jiri@...dia.com>, Jakub Kicinski <kuba@...nel.org>
Subject: Re: [PATCH net-next 0/8] devlink: support direct read from region



On 11/17/2022 2:07 PM, Jacob Keller wrote:
> A long time ago when initially implementing devlink regions in ice I
> proposed the ability to allow reading from a region without taking a
> snapshot [1]. I eventually dropped this work from the original series due to
> size. Then I eventually lost track of submitting this follow up.
> 
> This can be useful when interacting with some region that has some
> definitive "contents" from which snapshots are made. For example the ice
> driver has regions representing the contents of the device flash.
> 
> If userspace wants to read the contents today, it must first take a snapshot
> and then read from that snapshot. This makes sense if you want to read a
> large portion of data or you want to be sure reads are consistently from the
> same recording of the flash.
> 
> However if user space only wants to read a small chunk, it must first
> generate a snapshot of the entire contents, perform a read from the
> snapshot, and then delete the snapshot after reading.
> 
> For such a use case, a direct read from the region makes more sense. This
> can be achieved by allowing the devlink region read command to work without
> a snapshot. Instead the portion to be read can be forwarded directly to the
> driver via a new .read callback.
> 
> This avoids the need to read the entire region contents into memory first
> and avoids the software overhead of creating a snapshot and then deleting
> it.
> 
> This series implements such behavior and hooks up the ice NVM and shadow RAM
> regions to allow it.
> 
> [1] https://lore.kernel.org/netdev/20200130225913.1671982-1-jacob.e.keller@intel.com/
> 
> Cc: Jiri Pirko <jiri@...dia.com>
> Cc: Jakub Kicinski <kuba@...nel.org>
> 

There was some discussion about GENL_REQ_ATTR_CHECK, but this doesn't 
work for dumpits. While I was investigating this, I also saw a bunch of 
uses of NL_SET_ERR_MSG_MOD in net/core/devlink.c, so I have a separate 
patch to switch those to NL_SET_ERR_MSG, which I'll probably include in 
a series cleaning up the extended error messages.

> Jacob Keller (8):
>    devlink: find snapshot in devlink_nl_cmd_region_read_dumpit
>    devlink: use min_t to calculate data_size
>    devlink: report extended error message in region_read_dumpit
>    devlink: remove unnecessary parameter from chunk_fill function
>    devlink: refactor region_read_snapshot_fill to use a callback function
>    devlink: support directly reading from region memory
>    ice: use same function to snapshot both NVM and Shadow RAM
>    ice: implement direct read for NVM and Shadow RAM regions
> 
>   .../networking/devlink/devlink-region.rst     |   8 ++
>   drivers/net/ethernet/intel/ice/ice_devlink.c  | 112 +++++++++-------
>   include/net/devlink.h                         |  16 +++
>   net/core/devlink.c                            | 121 +++++++++++++-----
>   4 files changed, 180 insertions(+), 77 deletions(-)
> 
> 
> base-commit: b4b221bd79a1c698d9653e3ae2c3cb61cdc9aee7