Mon, Jun 01, 2020 at 08:18:19AM CEST, j...@resnulli.us wrote: >Sun, May 31, 2020 at 09:03:39AM CEST, vasundhara-v.vo...@broadcom.com wrote: >>Live device reset capability allows the users to reset the device in real >>time. For example, after flashing a new firmware image, this feature allows >>a user to initiate the reset immediately from a separate command, to load >>the new firmware without reloading the driver or resetting the system. >> >>When device reset is initiated, services running on the host interfaces >>will momentarily pause and resume once reset is completed, which is very >>similar to momentary network outage. >> >>This patchset adds support for two new generic devlink parameters for >>controlling the live device reset capability and use it in the bnxt_en >>driver. >> >>Users can initiate the reset from a separate command, for example, >>'ethtool --reset ethX all' or 'devlink dev reload' to reset the >>device. >>Where ethX or dev is any PF with administrative privileges. >> >>Patchset also updates firmware spec. to 1.10.1.40. >> >> >>v2->v3: Split the param into two new params "enable_live_dev_reset" and > >Vasundhara, I asked you multiple times for this to be "devlink dev reload" >attribute. I don't recall you telling any argument against it. I belive >that this should not be paramater. This is very tightly related to >reload, could you please have it as an attribute of reload, as I >suggested?
I just wrote the thread to the previous version. I understand now why you need param as you require reboot to activate the feature. However, I don't think it is correct to use enable_live_dev_reset to indicate the live-reset capability to the user. Params serve for configuration only. Could you please move the indication some place else? ("devlink dev info" seems fitting). I think that you can do the indication in a follow-up patchset. But please remove it from this one where you do it with params. > >Thanks! > > >>"allow_live_dev_reset". >>- Expand the documentation of each param and update commit messages >> accordingly. >>- Separated the permanent configuration mode code to another patch and >>rename the callbacks of the "allow_live_dev_reset" parameter accordingly. >> >>v1->v2: Rename param to "allow_fw_live_reset" from "enable_hot_fw_reset". >>- Update documentation files and commit messages with more details of the >> feature. >> >>Vasundhara Volam (6): >> devlink: Add 'enable_live_dev_reset' generic parameter. >> devlink: Add 'allow_live_dev_reset' generic parameter. >> bnxt_en: Use 'enable_live_dev_reset' devlink parameter. >> bnxt_en: Update firmware spec. to 1.10.1.40. >> bnxt_en: Use 'allow_live_dev_reset' devlink parameter. >> bnxt_en: Check if fw_live_reset is allowed before doing ETHTOOL_RESET >> >> Documentation/networking/devlink/bnxt.rst | 4 ++ >> .../networking/devlink/devlink-params.rst | 28 ++++++++++ >> drivers/net/ethernet/broadcom/bnxt/bnxt.c | 28 +++++++++- >> drivers/net/ethernet/broadcom/bnxt/bnxt.h | 2 + >> drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 49 +++++++++++++++++ >> drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h | 1 + >> drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 17 +++--- >> drivers/net/ethernet/broadcom/bnxt/bnxt_hsi.h | 64 >> +++++++++++++--------- >> include/net/devlink.h | 8 +++ >> net/core/devlink.c | 10 ++++ >> 10 files changed, 175 insertions(+), 36 deletions(-) >> >>-- >>1.8.3.1 >>