Re: [PATCH v2 0/3] devlink region trigger support

From: Yunsheng Lin <linyunsheng@huawei.com>
To: Jacob Keller <jacob.e.keller@intel.com>, <netdev@vger.kernel.org>
Cc: <valex@mellanox.com>, <jiri@resnulli.us>
Subject: Re: [PATCH v2 0/3] devlink region trigger support
Date: Sat, 11 Jan 2020 09:51:00 +0800	[thread overview]
Message-ID: <fe6c0d5e-5705-1118-1a71-80bd0e26a97e@huawei.com> (raw)
In-Reply-To: <1d00deb9-16fc-b2a5-f8f7-5bb8316dbac2@intel.com>

On 2020/1/11 1:52, Jacob Keller wrote:
> On 1/9/2020 8:10 PM, Yunsheng Lin wrote:
>> On 2020/1/10 3:33, Jacob Keller wrote:
>>> This series consists of patches to enable devlink to request a snapshot via
>>> a new DEVLINK_CMD_REGION_TRIGGER_SNAPSHOT.
>>>
>>> A reviewer might notice that the devlink health API already has such support
>>> for handling a similar case. However, the health API does not make sense in
>>> cases where the data is not related to an error condition.
>>
>> Maybe we need to specify the usecases for the region trigger as suggested by
>> Jacob.
>>
>> For example, the orginal usecase is to expose some set of flash/NVM contents.
>> But can it be used to dump the register of the bar space? or some binary
>> table in the hardware to debug some error that is not detected by hw?
>>
> 
> 
> regions can essentially be used to dump arbitrary addressable content. I
> think all of the above are great examples.
> 
> I have a series of patches to update and convert the devlink
> documentation, and I do provide some further detail in the new
> devlink-region.rst file.
> 
> Perhaps you could review that and provide suggestions on what would make
> sense to add there?

For the case of region for mlx4, I am not sure it worths the effort to
document it, because Jiri has mention that there was plan to convert mlx4 to
use "devlink health" api for the above case.

Also, there is dpipe, health and region api:
For health and region, they seems similar to me, and the non-essential
difference is:
1. health can be used used to dump content of tlv style, and can be triggered
   by driver automatically or by user manually.

2. region can be used to dump binary content and can be triggered by driver
   automatically only.

It would be good to merged the above to the same api(perhaps merge the binary
content dumping of region api to health api), then we can resue the same dump
ops for both driver and user triggering case.

For dpipe, it does not seems flexible enough to dump a table, yes, it provides
better visibility, but I am not sure it worth the effort, also, it would be better
to share the same table dump ops for driver and user triggering case.
For hns3 driver, we may have mac, vlan and flow director table that may need to dump
in both driver and user triggering case to debug some complex issues.

So It would be better to be able to dump table(maybe including binary table), binary
content and tlv content for a sinle api, I suppose the health api is the one to do
that? because health api has already supported driver and user triggering case, and
only need to add the table and binary content dumpping.

> 
> https://lore.kernel.org/netdev/20200109224625.1470433-13-jacob.e.keller@intel.com/
> 
> Thanks,
> Jake
> 
> .
> 