[<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
Powered by blists - more mailing lists