From: Jiri Pirko <jiri@resnulli.us>
To: Jacob Keller <jacob.e.keller@intel.com>
Cc: netdev@vger.kernel.org, Jakub Kicinski <kuba@kernel.org>
Subject: Re: [PATCH 00/10] implement DEVLINK_CMD_REGION_NEW
Date: Wed, 25 Mar 2020 14:49:28 +0100 [thread overview]
Message-ID: <20200325134928.GV11304@nanopsycho.orion> (raw)
In-Reply-To: <20200324223445.2077900-1-jacob.e.keller@intel.com>
Tue, Mar 24, 2020 at 11:34:35PM CET, jacob.e.keller@intel.com wrote:
>This series adds support for the DEVLINK_CMD_REGION_NEW operation, used to
>enable userspace requesting a snapshot of a region on demand.
>
>This can be useful to enable adding regions for a driver for which there is
>no trigger to create snapshots. By making this a core part of devlink, there
>is no need for the drivers to use a separate channel such as debugfs.
>
>The primary intent for this kind of region is to expose device information
>that might be useful for diagnostics and information gathering.
>
>The first few patches refactor regions to support a new ops structure for
>extending the available operations that regions can perform. This includes
>converting the destructor into an op from a function argument.
>
>Next, patches refactor the snapshot id allocation to use an xarray which
>tracks the number of current snapshots using a given id. This is done so
>that id lifetime can be determined, and ids can be released when no longer
>in use.
>
>Without this change, snapshot ids remain used forever, until the snapshot_id
>count rolled over UINT_MAX.
>
>Finally, code to enable the previously unused DEVLINK_CMD_REGION_NEW is
>added. This code enforces that the snapshot id is always provided, unlike
>previous revisions of this series.
>
>Finally, a patch is added to enable using this new command via the .snapshot
>callback in both netdevsim and the ice driver.
>
>For the ice driver, a new "nvm-flash" region is added, which will enable
>read access to the NVM flash contents. The intention for this is to allow
>diagnostics tools to gather information about the device. By using a
>snapshot and gathering the NVM contents all at once, the contents can be
>atomic.
>
>General changes since the v2 RFC:
>* Use an xarray instead of IDRs
>* Rebase onto net-next now that the initial ice devlink patches landed.
>
>Patch specific changes:
>* devlink: add functions to take snapshot while locked
>
> Split this into two patches, so that an explanation of why the
> devlink_region_snapshot_id_get is still extracted, even though only one
> caller will remain.
It is much more convenient for reviewing purposes to have the changelog
per-patch.
>
>* devlink: track snapshot ids using an IDR and refcounts
>
> Convert to using an xarray storing the total number of snapshots directly,
> rather than an IDR with a refcount structure. This significantly
> simplifies the code, and avoids the complication of a NULL refcount.
>
>* devlink: implement DEVLINK_CMD_REGION_NEW
>
> As suggested by Jiri, remove the ability for DEVLINK_CMD_REGION_NEW to
> dynamically generate IDs. Instead, always require a snapshot id. This
> aligns with DEVLINK_CMD_REGION_DEL, and helps reduce confusion.
>
> Refactor this patch to use the xarray instead of the IDR, as in the
> previous patch.
>
> Clean up and remove unnecessary new lines on NL_SET_ERR_MSG_MOD
>
>* ice: add a devlink region to dump shadow RAM contents
>
> Remove the code for immediate region read, as this will be worked on in a
> separate series following this one.
>
>Jacob Keller (10):
> devlink: prepare to support region operations
> devlink: convert snapshot destructor callback to region op
> devlink: trivial: fix tab in function documentation
> devlink: add function to take snapshot while locked
> devlink: extract snapshot id allocation to helper function
> devlink: convert snapshot id getter to return an error
> devlink: track snapshot id usage count using an xarray
> devlink: implement DEVLINK_CMD_REGION_NEW
> netdevsim: support taking immediate snapshot via devlink
> ice: add a devlink region for dumping NVM contents
>
> .../networking/devlink/devlink-region.rst | 8 +
> Documentation/networking/devlink/ice.rst | 26 ++
> drivers/net/ethernet/intel/ice/ice.h | 2 +
> drivers/net/ethernet/intel/ice/ice_devlink.c | 99 +++++
> drivers/net/ethernet/intel/ice/ice_devlink.h | 3 +
> drivers/net/ethernet/intel/ice/ice_main.c | 4 +
> drivers/net/ethernet/mellanox/mlx4/crdump.c | 32 +-
> drivers/net/netdevsim/dev.c | 41 +-
> include/net/devlink.h | 31 +-
> net/core/devlink.c | 354 +++++++++++++++---
> .../drivers/net/netdevsim/devlink.sh | 15 +
> 11 files changed, 538 insertions(+), 77 deletions(-)
>
>--
>2.24.1
>
next prev parent reply other threads:[~2020-03-25 13:49 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-24 22:34 [PATCH 00/10] implement DEVLINK_CMD_REGION_NEW Jacob Keller
2020-03-24 22:34 ` [PATCH 01/10] devlink: prepare to support region operations Jacob Keller
2020-03-24 22:34 ` [PATCH 02/10] devlink: convert snapshot destructor callback to region op Jacob Keller
2020-03-24 22:34 ` [PATCH 03/10] devlink: trivial: fix tab in function documentation Jacob Keller
2020-03-24 22:34 ` [PATCH 04/10] devlink: add function to take snapshot while locked Jacob Keller
2020-03-25 17:56 ` Jakub Kicinski
2020-03-24 22:34 ` [PATCH 05/10] devlink: extract snapshot id allocation to helper function Jacob Keller
2020-03-25 14:08 ` Jiri Pirko
2020-03-24 22:34 ` [PATCH 06/10] devlink: convert snapshot id getter to return an error Jacob Keller
2020-03-25 18:04 ` Jakub Kicinski
2020-03-25 19:13 ` Jiri Pirko
2020-03-26 1:33 ` Jacob Keller
2020-03-24 22:34 ` [PATCH] devlink: track snapshot id usage count using an xarray Jacob Keller
2020-03-25 16:08 ` Jiri Pirko
2020-03-26 1:16 ` Jacob Keller
2020-03-26 1:43 ` Jacob Keller
2020-03-24 22:34 ` [PATCH 08/10] devlink: implement DEVLINK_CMD_REGION_NEW Jacob Keller
2020-03-25 16:46 ` Jiri Pirko
2020-03-25 17:18 ` Jakub Kicinski
2020-03-25 17:20 ` Jiri Pirko
2020-03-25 17:46 ` Jakub Kicinski
2020-03-25 18:41 ` Jiri Pirko
2020-03-26 1:30 ` Jacob Keller
2020-03-24 22:34 ` [PATCH 09/10] netdevsim: support taking immediate snapshot via devlink Jacob Keller
2020-03-25 16:50 ` Jiri Pirko
2020-03-24 22:34 ` [PATCH 10/10] ice: add a devlink region for dumping NVM contents Jacob Keller
2020-03-25 17:18 ` Jiri Pirko
2020-03-25 13:49 ` Jiri Pirko [this message]
2020-03-25 13:50 ` [PATCH 00/10] implement DEVLINK_CMD_REGION_NEW Jiri Pirko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200325134928.GV11304@nanopsycho.orion \
--to=jiri@resnulli.us \
--cc=jacob.e.keller@intel.com \
--cc=kuba@kernel.org \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).