| Message ID | 20200109193311.1352330-1-jacob.e.keller@intel.com |
|---|---|
| Headers | show |
| Series | devlink region trigger support | expand |
On 1/9/2020 11:33 AM, 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. > > In this case, using the health API only for the dumping feels incorrect. > Regions make sense when the addressable content is not captured > automatically on error conditions, but only upon request by the devlink API. > > The netdevsim driver is modified to support the new trigger_snapshot > callback as an example of how this can be used. > As mentioned by Jakub on an earlier comment, I wanted to clarify: I implemented this in the netdevsim driver because I wanted to test that the changes actually worked as expected. I'm planning on making use of this in an Intel driver soon, but do not yet have patches ready to send to the list. Thanks, Jake > Jacob Keller (3): > devlink: add callback to trigger region snapshots > devlink: introduce command to trigger region snapshot > netdevsim: support triggering snapshot through devlink > > drivers/net/ethernet/mellanox/mlx4/crdump.c | 4 +- > drivers/net/netdevsim/dev.c | 37 ++++++++++++----- > include/net/devlink.h | 12 ++++-- > include/uapi/linux/devlink.h | 2 + > net/core/devlink.c | 45 +++++++++++++++++++-- > 5 files changed, 80 insertions(+), 20 deletions(-) >
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? > > In this case, using the health API only for the dumping feels incorrect. > Regions make sense when the addressable content is not captured > automatically on error conditions, but only upon request by the devlink API. > > The netdevsim driver is modified to support the new trigger_snapshot > callback as an example of how this can be used. > > Jacob Keller (3): > devlink: add callback to trigger region snapshots > devlink: introduce command to trigger region snapshot > netdevsim: support triggering snapshot through devlink > > drivers/net/ethernet/mellanox/mlx4/crdump.c | 4 +- > drivers/net/netdevsim/dev.c | 37 ++++++++++++----- > include/net/devlink.h | 12 ++++-- > include/uapi/linux/devlink.h | 2 + > net/core/devlink.c | 45 +++++++++++++++++++-- > 5 files changed, 80 insertions(+), 20 deletions(-) >
Thu, Jan 09, 2020 at 08:33:07PM CET, jacob.e.keller@intel.com 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. > >In this case, using the health API only for the dumping feels incorrect. >Regions make sense when the addressable content is not captured >automatically on error conditions, but only upon request by the devlink API. > >The netdevsim driver is modified to support the new trigger_snapshot >callback as an example of how this can be used. I don't think that the netdevsim usecase is enough to merge this in. You need a real-driver user as well. Of course, netdevsim implementation is good to have to, but you have to bundle selftests along with that. > >Jacob Keller (3): > devlink: add callback to trigger region snapshots > devlink: introduce command to trigger region snapshot > netdevsim: support triggering snapshot through devlink > > drivers/net/ethernet/mellanox/mlx4/crdump.c | 4 +- > drivers/net/netdevsim/dev.c | 37 ++++++++++++----- > include/net/devlink.h | 12 ++++-- > include/uapi/linux/devlink.h | 2 + > net/core/devlink.c | 45 +++++++++++++++++++-- > 5 files changed, 80 insertions(+), 20 deletions(-) > >-- >2.25.0.rc1 >
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? https://lore.kernel.org/netdev/20200109224625.1470433-13-jacob.e.keller@intel.com/ Thanks, Jake
On 1/10/2020 1:40 AM, Jiri Pirko wrote: > Thu, Jan 09, 2020 at 08:33:07PM CET, jacob.e.keller@intel.com 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. >> >> In this case, using the health API only for the dumping feels incorrect. >> Regions make sense when the addressable content is not captured >> automatically on error conditions, but only upon request by the devlink API. >> >> The netdevsim driver is modified to support the new trigger_snapshot >> callback as an example of how this can be used. > > I don't think that the netdevsim usecase is enough to merge this in. You > need a real-driver user as well. > Sure. But this direction and implementation seems reasonable? I am making progress on a driver implementation for this, and I am fine holding onto these patches until I am ready to submit the full series to the list with the driver.. Mostly I wanted to make sure the direction was looking good earlier than the time frame for completing that work. Thanks, Jake
On Fri, 10 Jan 2020 09:54:20 -0800, Jacob Keller wrote: > On 1/10/2020 1:40 AM, Jiri Pirko wrote: > > Thu, Jan 09, 2020 at 08:33:07PM CET, jacob.e.keller@intel.com 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. > >> > >> In this case, using the health API only for the dumping feels incorrect. > >> Regions make sense when the addressable content is not captured > >> automatically on error conditions, but only upon request by the devlink API. > >> > >> The netdevsim driver is modified to support the new trigger_snapshot > >> callback as an example of how this can be used. > > > > I don't think that the netdevsim usecase is enough to merge this in. You > > need a real-driver user as well. > > > Sure. But this direction and implementation seems reasonable? > > I am making progress on a driver implementation for this, and I am fine > holding onto these patches until I am ready to submit the full series to > the list with the driver.. > > Mostly I wanted to make sure the direction was looking good earlier than > the time frame for completing that work. As Jiri said, quite hard to tell without seeing the real user. I presume you take some lock to ensure the contents of the snapshot are atomic? Otherwise I wonder if you wouldn't be better served by just allowing region read to operate directly on the data rather then going through the snapshot create -> read -> snapshot remove cycle. Not sure Jiri would agree, it require more code. For the patches themselves we may want to move the callbacks into some ops structure in the region. And I don't think you need to make the trigger command NO_LOCK.
On 1/10/2020 10:47 AM, Jakub Kicinski wrote: > On Fri, 10 Jan 2020 09:54:20 -0800, Jacob Keller wrote: >> Mostly I wanted to make sure the direction was looking good earlier than >> the time frame for completing that work. > > As Jiri said, quite hard to tell without seeing the real user. > Fair. > I presume you take some lock to ensure the contents of the snapshot are > atomic? Otherwise I wonder if you wouldn't be better served by just > allowing region read to operate directly on the data rather then going > through the snapshot create -> read -> snapshot remove cycle. Not sure > Jiri would agree, it require more code. > Right. I saw the original devlink region commit messages indicated possible plans to support writing to regions. There is also the question of handling data that might want to be read sparsely, rather than reading an entire snapshot at once. Hmm. > For the patches themselves we may want to move the callbacks into some > ops structure in the region. And I don't think you need to make the > trigger command NO_LOCK. > So the reason it was made NO_LOCK right now is because the trigger ends up calling devlink_region_snapshot_id_get and devlink_region_snapshot_create which both take the devlink lock. I suppose this could be simplified by having the snapshot callback take similar parameters as the devlink_region_snapshot_create, and assume the devlink core code does such things as generating an id and adding the snapshot to the list all while holding the lock... I will sort out these questions into what I think makes sense for the use case I envision, and will submit the core devlink patches at the same time as the driver changes. Thanks, Jake
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 > > . >
On Sat, 11 Jan 2020 09:51:00 +0800, Yunsheng Lin wrote: > > 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. I think there is a fundamental difference between health API and regions in the fact that health reporters allow for returning structured data from different sources which are associated with an event/error condition. That includes information read from the hardware or driver/software state. Region API (as Jake said) is good for dumping arbitrary addressable content, e.g. registers. I don't see much use for merging the two right now, FWIW...
On 1/12/2020 10:45 PM, Jakub Kicinski wrote: > On Sat, 11 Jan 2020 09:51:00 +0800, Yunsheng Lin wrote: >>> 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. > I think there is a fundamental difference between health API and > regions in the fact that health reporters allow for returning > structured data from different sources which are associated with > an event/error condition. That includes information read from the > hardware or driver/software state. Region API (as Jake said) is good > for dumping arbitrary addressable content, e.g. registers. I don't see > much use for merging the two right now, FWIW... > Totally agree with Jakub, I think health and region are different and each has its usages as mentioned above. Using words such as recovery and health for exposing registers doesn't sound correct. There are future usages I can think of for region if we will add the trigger support as well as the live region read.
On 2020/1/13 5:18, Alex Vesker wrote: > On 1/12/2020 10:45 PM, Jakub Kicinski wrote: >> On Sat, 11 Jan 2020 09:51:00 +0800, Yunsheng Lin wrote: >>>> 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. >> I think there is a fundamental difference between health API and >> regions in the fact that health reporters allow for returning >> structured data from different sources which are associated with >> an event/error condition. That includes information read from the >> hardware or driver/software state. Region API (as Jake said) is good >> for dumping arbitrary addressable content, e.g. registers. I don't see >> much use for merging the two right now, FWIW... The point is that we are beginning to use health API for event/error condition, right? Do we use health API or regions API to dump a arbitrary addressable content when there is an event/error detected? Also we may need to dump both a arbitrary addressable content and the structured data when there is an event/error detected, the health API or regions API can not do both, right? It still seems it is a little confusing when deciding to use health or regions API. >> > Totally agree with Jakub, I think health and region are different and > each has its > usages as mentioned above. Using words such as recovery and health for > exposing > registers doesn't sound correct. There are future usages I can think of > for region if we > will add the trigger support as well as the live region read. health API already has "trigger support" now if region API is merged to health API. I am not sure I understand "live region" here, what is the usecase of live region? > > >
On Mon, 13 Jan 2020 09:39:50 +0800, Yunsheng Lin wrote: > On 2020/1/13 5:18, Alex Vesker wrote: > > On 1/12/2020 10:45 PM, Jakub Kicinski wrote: > >> I think there is a fundamental difference between health API and > >> regions in the fact that health reporters allow for returning > >> structured data from different sources which are associated with > >> an event/error condition. That includes information read from the > >> hardware or driver/software state. Region API (as Jake said) is good > >> for dumping arbitrary addressable content, e.g. registers. I don't see > >> much use for merging the two right now, FWIW... > > The point is that we are beginning to use health API for event/error > condition, right? Do we use health API or regions API to dump a arbitrary > addressable content when there is an event/error detected? > > Also we may need to dump both a arbitrary addressable content and the If the information dumped is pertinent in the context of a health event it's not arbitrary. > structured data when there is an event/error detected, the health API or > regions API can not do both, right? I think for errors and events health API will be more suitable 99% of the time. > It still seems it is a little confusing when deciding to use health or > regions API. > > > Totally agree with Jakub, I think health and region are different and > > each has its > > usages as mentioned above. Using words such as recovery and health for > > exposing > > registers doesn't sound correct. There are future usages I can think of > > for region if we > > will add the trigger support as well as the live region read. > > health API already has "trigger support" now if region API is merged to > health API. > > I am not sure I understand "live region" here, what is the usecase of live > region? Reading registers of a live system without copying them to a snapshot first. Some chips have so many registers it's impractical to group them beyond "registers of IP block X", if that. IMHO that fits nicely with regions, health is grouped by event, so we'd likely want to dump for example one or two registers from the MAC there, while the entire set of MAC registers can be exposed as a region.
Sat, Jan 11, 2020 at 02:51:00AM CET, linyunsheng@huawei.com wrote: >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. It is on the TODO list, yes. For mlx4 usecase the healh reporters are more suitable. > >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. I was thinking about that as well. Will check it out. > >For dpipe, it does not seems flexible enough to dump a table, yes, it provides Why? That is the purpose of the dpipe, but make the hw pipeline visible and show you the content of individual nodes. >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 >> >> . >> >
On 1/13/2020 3:34 AM, Jakub Kicinski wrote: > On Mon, 13 Jan 2020 09:39:50 +0800, Yunsheng Lin wrote: >> I am not sure I understand "live region" here, what is the usecase of live >> region? > > Reading registers of a live system without copying them to a snapshot > first. Some chips have so many registers it's impractical to group them > beyond "registers of IP block X", if that. IMHO that fits nicely with > regions, health is grouped by event, so we'd likely want to dump for > example one or two registers from the MAC there, while the entire set > of MAC registers can be exposed as a region. > Right. I'm actually wondering about this as well. Region snapshots are captured in whole and stored and then returned through the devlink region commands. This could be problematic if you wanted to expose a larger chunk of registers or addressable sections of flash contents, as the size of the contents goes beyond a single page. If we instead focus regions onto the live-read aspect, the API can simply be a request to read a segment of the region. Then, the driver could perform the read of that chunk and report it back.
On 1/13/2020 8:58 AM, Jiri Pirko wrote: > Why? That is the purpose of the dpipe, but make the hw > pipeline visible and show you the content of individual nodes. > I agree. dpipe seems to be focused specifically on dumping nodes of the tables that represent the hardware's pipeline. I think it's unrelated to this discussion about regions vs health API.
Mon, Jan 13, 2020 at 07:16:51PM CET, jacob.e.keller@intel.com wrote: >On 1/13/2020 3:34 AM, Jakub Kicinski wrote: >> On Mon, 13 Jan 2020 09:39:50 +0800, Yunsheng Lin wrote: >>> I am not sure I understand "live region" here, what is the usecase of live >>> region? >> >> Reading registers of a live system without copying them to a snapshot >> first. Some chips have so many registers it's impractical to group them >> beyond "registers of IP block X", if that. IMHO that fits nicely with >> regions, health is grouped by event, so we'd likely want to dump for >> example one or two registers from the MAC there, while the entire set >> of MAC registers can be exposed as a region. >> > >Right. I'm actually wondering about this as well. Region snapshots are >captured in whole and stored and then returned through the devlink >region commands. Well, driver does not have to support snapshots for particular region, only live reading. Yes, this needs to be implemented. > >This could be problematic if you wanted to expose a larger chunk of >registers or addressable sections of flash contents, as the size of the >contents goes beyond a single page. > >If we instead focus regions onto the live-read aspect, the API can >simply be a request to read a segment of the region. Then, the driver >could perform the read of that chunk and report it back. Yep.
Mon, Jan 13, 2020 at 07:22:57PM CET, jacob.e.keller@intel.com wrote: > > >On 1/13/2020 8:58 AM, Jiri Pirko wrote: >> Why? That is the purpose of the dpipe, but make the hw >> pipeline visible and show you the content of individual nodes. >> > >I agree. dpipe seems to be focused specifically on dumping nodes of the >tables that represent the hardware's pipeline. I think it's unrelated to >this discussion about regions vs health API. Nod.
On 2020/1/14 2:22, Jacob Keller wrote: > > > On 1/13/2020 8:58 AM, Jiri Pirko wrote: >> Why? That is the purpose of the dpipe, but make the hw >> pipeline visible and show you the content of individual nodes. >> > > I agree. dpipe seems to be focused specifically on dumping nodes of the > tables that represent the hardware's pipeline. I think it's unrelated to > this discussion about regions vs health API. Sorry for bringing up a not really unrelated question in the thread, For the hns3 hw mac table, it seems the hns3 hw is pretty simple, it mainly contain the port bitmaps of a mac address, then the hw can forward the packet based on the dst mac' port bitamp. It seems a litte hard to match to the dpipe API the last time I tried to use dpipe API to dump that. So maybe it would be good to have the support of table dumping (both structured and binary table) for health API natively, so that we use it to dump some hw table for both driver and user triggering cases. I am not sure if other driver has the above requirement, and if the requirement makes any sense? > >
On 1/14/2020 12:33 AM, Yunsheng Lin wrote: > On 2020/1/14 2:22, Jacob Keller wrote: >> >> >> On 1/13/2020 8:58 AM, Jiri Pirko wrote: >>> Why? That is the purpose of the dpipe, but make the hw >>> pipeline visible and show you the content of individual nodes. >>> >> >> I agree. dpipe seems to be focused specifically on dumping nodes of the >> tables that represent the hardware's pipeline. I think it's unrelated to >> this discussion about regions vs health API. > > Sorry for bringing up a not really unrelated question in the thread, > No problem here :) I've been investigating devlink for our products, and am working towards implementations for several features. Further discussion is definitely welcome! > For the hns3 hw mac table, it seems the hns3 hw is pretty simple, it mainly > contain the port bitmaps of a mac address, then the hw can forward the packet > based on the dst mac' port bitamp. > Right. > It seems a litte hard to match to the dpipe API the last time I tried to > use dpipe API to dump that. dpipe is primarily targeted towards dumping complex hardware pipelines. > > So maybe it would be good to have the support of table dumping (both structured > and binary table) for health API natively, so that we use it to dump some hw > table for both driver and user triggering cases. > Maybe dpipe needs additional mechanism for presenting tables? > I am not sure if other driver has the above requirement, and if the requirement > makes any sense? > I think there's value in the ability to express this kind of contents. Regions could work. In regards to using devlink-health, I do believe the health API should remain focused as the area you use for dumping data gathered specifically in relation to an event such as an error condition. Regions make sense when you want to allow access to a section of addressable contents on demand. A binary table could be dumped via a region, but I'm not 100% sure it would make sense for structured tables. Thanks, Jake
On 2020/1/15 4:04, Jacob Keller wrote: > On 1/14/2020 12:33 AM, Yunsheng Lin wrote: >> On 2020/1/14 2:22, Jacob Keller wrote: >>> >>> >>> On 1/13/2020 8:58 AM, Jiri Pirko wrote: >>>> Why? That is the purpose of the dpipe, but make the hw >>>> pipeline visible and show you the content of individual nodes. >>>> >>> >>> I agree. dpipe seems to be focused specifically on dumping nodes of the >>> tables that represent the hardware's pipeline. I think it's unrelated to >>> this discussion about regions vs health API. >> >> Sorry for bringing up a not really unrelated question in the thread, >> > > No problem here :) I've been investigating devlink for our products, and > am working towards implementations for several features. Further > discussion is definitely welcome! > >> For the hns3 hw mac table, it seems the hns3 hw is pretty simple, it mainly >> contain the port bitmaps of a mac address, then the hw can forward the packet >> based on the dst mac' port bitamp. >> > > Right. > >> It seems a litte hard to match to the dpipe API the last time I tried to >> use dpipe API to dump that. > > dpipe is primarily targeted towards dumping complex hardware pipelines. > >> >> So maybe it would be good to have the support of table dumping (both structured >> and binary table) for health API natively, so that we use it to dump some hw >> table for both driver and user triggering cases. >> > > Maybe dpipe needs additional mechanism for presenting tables? > >> I am not sure if other driver has the above requirement, and if the requirement >> makes any sense? >> > > I think there's value in the ability to express this kind of contents. > Regions could work. > > In regards to using devlink-health, I do believe the health API should > remain focused as the area you use for dumping data gathered > specifically in relation to an event such as an error condition. what if there are requirements to dump the same data for both error/event triggering and user cmd triggering case? If we implement a dump ops of region API for user cmd triggering case, and then implement a similar dump ops of health API for error/event case, does this not seems unnecessary duplication of effort? For user cmd triggering case, it is used to inspect the device' health state mostly, so maybe it makes sense to use health API to dump that too. > > Regions make sense when you want to allow access to a section of > addressable contents on demand. > > A binary table could be dumped via a region, but I'm not 100% sure it > would make sense for structured tables. > > Thanks, > Jake > > . >