| Message ID | 1548172644-30862-1-git-send-email-eranbe@mellanox.com |
|---|---|
| Headers | show |
| Series | Devlink health updates | expand |
Tue, Jan 22, 2019 at 04:57:17PM CET, eranbe@mellanox.com wrote: >This patchset fixes some comments that were received for the devlink >health series, mostly around the devlink health buffers API. > >It offers a new devlink<->driver API for passing health dump and diagnose info. >As part of this patchset, the new API is developed and integrated into the >devlink health and mlx5e TX reporter. >Also, added some helpers together with the new API, which reduce the code >required by the driver to fill dump and diagnose significantly. > >Eventually, it also deletes the old API. > >In addition, it includes some small fixes in the devlink and mlx5e TX reporter. Okay, just leaving, going to review this tomorrow. I would much rather review the patchset from the beginning, not this incremental patchset. It changes a lot of things, deprecating api what was just introduced. Review nightmare :/ Could we do revert, repost? For my health sakes :) Thanks! > > >Eran Ben Elisha (7): > devlink: Add devlink msg API > net/mlx5e: Move driver to use devlink msg API > devlink: move devlink health reporter to use devlink msg API > devlink: Delete depracated health buffers API > devlink: Remove spaces around "=" in the logger print > devlink: Fix use-after-free at reporter destroy > net/mlx5e: Add RTNL lock to TX recover flow > > .../mellanox/mlx5/core/en/reporter_tx.c | 124 +--- > include/net/devlink.h | 79 +-- > include/trace/events/devlink.h | 2 +- > include/uapi/linux/devlink.h | 14 +- > net/core/devlink.c | 633 ++++++++---------- > 5 files changed, 342 insertions(+), 510 deletions(-) > >-- >2.17.1 >
Tue, Jan 22, 2019 at 04:57:17PM CET, eranbe@mellanox.com wrote: >This patchset fixes some comments that were received for the devlink >health series, mostly around the devlink health buffers API. > >It offers a new devlink<->driver API for passing health dump and diagnose info. >As part of this patchset, the new API is developed and integrated into the >devlink health and mlx5e TX reporter. >Also, added some helpers together with the new API, which reduce the code >required by the driver to fill dump and diagnose significantly. > >Eventually, it also deletes the old API. > >In addition, it includes some small fixes in the devlink and mlx5e TX reporter. We are (re-)defining UAPI here. You need to present some examples of devlink tool output, both in json and stdout. Again, much more convenient to be done for the whole patchset, not this "fixup-one" :/ > > >Eran Ben Elisha (7): > devlink: Add devlink msg API > net/mlx5e: Move driver to use devlink msg API > devlink: move devlink health reporter to use devlink msg API > devlink: Delete depracated health buffers API > devlink: Remove spaces around "=" in the logger print > devlink: Fix use-after-free at reporter destroy > net/mlx5e: Add RTNL lock to TX recover flow > > .../mellanox/mlx5/core/en/reporter_tx.c | 124 +--- > include/net/devlink.h | 79 +-- > include/trace/events/devlink.h | 2 +- > include/uapi/linux/devlink.h | 14 +- > net/core/devlink.c | 633 ++++++++---------- > 5 files changed, 342 insertions(+), 510 deletions(-) > >-- >2.17.1 >
On 1/23/2019 1:44 PM, Jiri Pirko wrote: > Tue, Jan 22, 2019 at 04:57:17PM CET, eranbe@mellanox.com wrote: >> This patchset fixes some comments that were received for the devlink >> health series, mostly around the devlink health buffers API. >> >> It offers a new devlink<->driver API for passing health dump and diagnose info. >> As part of this patchset, the new API is developed and integrated into the >> devlink health and mlx5e TX reporter. >> Also, added some helpers together with the new API, which reduce the code >> required by the driver to fill dump and diagnose significantly. >> >> Eventually, it also deletes the old API. >> >> In addition, it includes some small fixes in the devlink and mlx5e TX reporter. > > We are (re-)defining UAPI here. You need to present some examples > of devlink tool output, both in json and stdout. Actually we don't really redefining the section, but only having naming change, DEVLINK_ATTR_HEALTH_BUFFER_OBJECT* to DEVLINK_ATTR_MSG_OBJECT*. It is pretty much the same in UAPI perspective. All examples from original patchset are still 100% true. > Again, much more convenient to be done for the whole patchset, not this > "fixup-one" :/ The core of the change you have asked for it fully implemented in patch 0001 (and will be kept as is if I would do the re-done procedure). Me and Moshe had an internal review yesterday and it doesn't look like a nightmare at all. Please try to give it a real try as I worked a lot in order to meet the DD set here, and if you still find it really hard, let me know. > > >> >> >> Eran Ben Elisha (7): >> devlink: Add devlink msg API >> net/mlx5e: Move driver to use devlink msg API >> devlink: move devlink health reporter to use devlink msg API >> devlink: Delete depracated health buffers API >> devlink: Remove spaces around "=" in the logger print >> devlink: Fix use-after-free at reporter destroy >> net/mlx5e: Add RTNL lock to TX recover flow >> >> .../mellanox/mlx5/core/en/reporter_tx.c | 124 +--- >> include/net/devlink.h | 79 +-- >> include/trace/events/devlink.h | 2 +- >> include/uapi/linux/devlink.h | 14 +- >> net/core/devlink.c | 633 ++++++++---------- >> 5 files changed, 342 insertions(+), 510 deletions(-) >> >> -- >> 2.17.1 >>
From: Jiri Pirko <jiri@resnulli.us> Date: Tue, 22 Jan 2019 17:58:21 +0100 > Tue, Jan 22, 2019 at 04:57:17PM CET, eranbe@mellanox.com wrote: >>This patchset fixes some comments that were received for the devlink >>health series, mostly around the devlink health buffers API. >> >>It offers a new devlink<->driver API for passing health dump and diagnose info. >>As part of this patchset, the new API is developed and integrated into the >>devlink health and mlx5e TX reporter. >>Also, added some helpers together with the new API, which reduce the code >>required by the driver to fill dump and diagnose significantly. >> >>Eventually, it also deletes the old API. >> >>In addition, it includes some small fixes in the devlink and mlx5e TX reporter. > > Okay, just leaving, going to review this tomorrow. I would much rather > review the patchset from the beginning, not this incremental patchset. > It changes a lot of things, deprecating api what was just introduced. > Review nightmare :/ > > Could we do revert, repost? For my health sakes :) Eran are you ok with the revert? I'll do it once I have Eran's confirmation.
On 1/25/2019 8:08 AM, David Miller wrote: > From: Jiri Pirko <jiri@resnulli.us> > Date: Tue, 22 Jan 2019 17:58:21 +0100 > >> Tue, Jan 22, 2019 at 04:57:17PM CET, eranbe@mellanox.com wrote: >>> This patchset fixes some comments that were received for the devlink >>> health series, mostly around the devlink health buffers API. >>> >>> It offers a new devlink<->driver API for passing health dump and diagnose info. >>> As part of this patchset, the new API is developed and integrated into the >>> devlink health and mlx5e TX reporter. >>> Also, added some helpers together with the new API, which reduce the code >>> required by the driver to fill dump and diagnose significantly. >>> >>> Eventually, it also deletes the old API. >>> >>> In addition, it includes some small fixes in the devlink and mlx5e TX reporter. >> >> Okay, just leaving, going to review this tomorrow. I would much rather >> review the patchset from the beginning, not this incremental patchset. >> It changes a lot of things, deprecating api what was just introduced. >> Review nightmare :/ >> >> Could we do revert, repost? For my health sakes :) > > Eran are you ok with the revert? Dave, thanks for your consideration. During the review of this fixes series with Jiri yesterday, we reached to a conclusion that it would be cleaner to revert and re-post it again. I thought I shall submit a revert patchset, but if just remove it, it would be better, I guess. Jiri, I will probably be able to provide a new version with fixed comments from here soon next week. > > I'll do it once I have Eran's confirmation. Note that you will also have to revert ARM compilation fix which was accepted on top. https://patchwork.ozlabs.org/patch/1029047/ Thanks.
Fri, Jan 25, 2019 at 10:16:16AM CET, eranbe@mellanox.com wrote: > > >On 1/25/2019 8:08 AM, David Miller wrote: >> From: Jiri Pirko <jiri@resnulli.us> >> Date: Tue, 22 Jan 2019 17:58:21 +0100 >> >>> Tue, Jan 22, 2019 at 04:57:17PM CET, eranbe@mellanox.com wrote: >>>> This patchset fixes some comments that were received for the devlink >>>> health series, mostly around the devlink health buffers API. >>>> >>>> It offers a new devlink<->driver API for passing health dump and diagnose info. >>>> As part of this patchset, the new API is developed and integrated into the >>>> devlink health and mlx5e TX reporter. >>>> Also, added some helpers together with the new API, which reduce the code >>>> required by the driver to fill dump and diagnose significantly. >>>> >>>> Eventually, it also deletes the old API. >>>> >>>> In addition, it includes some small fixes in the devlink and mlx5e TX reporter. >>> >>> Okay, just leaving, going to review this tomorrow. I would much rather >>> review the patchset from the beginning, not this incremental patchset. >>> It changes a lot of things, deprecating api what was just introduced. >>> Review nightmare :/ >>> >>> Could we do revert, repost? For my health sakes :) >> >> Eran are you ok with the revert? > >Dave, thanks for your consideration. > >During the review of this fixes series with Jiri yesterday, we reached >to a conclusion that it would be cleaner to revert and re-post it again. >I thought I shall submit a revert patchset, but if just remove it, it >would be better, I guess. > >Jiri, >I will probably be able to provide a new version with fixed comments >from here soon next week. Good. Thanks! > >> >> I'll do it once I have Eran's confirmation. > >Note that you will also have to revert ARM compilation fix which was >accepted on top. >https://patchwork.ozlabs.org/patch/1029047/ > >Thanks.
From: Eran Ben Elisha <eranbe@mellanox.com> Date: Fri, 25 Jan 2019 09:16:16 +0000 > On 1/25/2019 8:08 AM, David Miller wrote: >> I'll do it once I have Eran's confirmation. > > Note that you will also have to revert ARM compilation fix which was > accepted on top. > https://patchwork.ozlabs.org/patch/1029047/ Thanks for this heads up. I just pushed out the revert. If I made any mistakes, please send me a fixup. But I am pretty sure I got it right. Thanks!