[net-next,0/8] devlink: support direct read from region

Message

Jacob Keller Nov. 17, 2022, 10:07 p.m. UTC

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/[e-mail]/

Cc: Jiri Pirko <[e-mail]>
Cc: Jakub Kicinski <[e-mail]>

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

Comments

Jacob Keller Nov. 23, 2022, 8:51 p.m. UTC | #1

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/[e-mail]/
> 
> Cc: Jiri Pirko <[e-mail]>
> Cc: Jakub Kicinski <[e-mail]>
> 

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