All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jiri Pirko <jiri@resnulli.us>
To: Jakub Kicinski <kuba@kernel.org>
Cc: Vasundhara Volam <vasundhara-v.volam@broadcom.com>,
	David Miller <davem@davemloft.net>,
	Netdev <netdev@vger.kernel.org>,
	Michael Chan <michael.chan@broadcom.com>
Subject: Re: [PATCH v3 net-next 0/6] bnxt_en: Add 'enable_live_dev_reset' and 'allow_live_dev_reset' generic devlink params.
Date: Tue, 2 Jun 2020 08:31:39 +0200	[thread overview]
Message-ID: <20200602063139.GT2282@nanopsycho> (raw)
In-Reply-To: <20200601162416.386937b2@kicinski-fedora-PC1C0HJN.hsd1.ca.comcast.net>

Tue, Jun 02, 2020 at 01:24:16AM CEST, kuba@kernel.org wrote:
>On Mon, 1 Jun 2020 21:01:42 +0530 Vasundhara Volam wrote:
>> > I think that the legacy ethtool should stick with the "ordinary fw reset",
>> > becase that is what user expects. You should add an attribute to
>> > "devlink dev reload" to trigger the "live fw reset"  
>> 
>> Okay.
>> 
>> I am planning to add a type field with "driver-only | fw-reset |
>> live-fw-reset | live-fw-patch" to "devlink dev reload" command.
>> 
>> driver-only - Resets host driver instance of the 'devlink dev'
>> (current behaviour). This will be default, if the user does not
>> provide the type option.
>> fw-reset - Initiate the reset command for the currently running
>> firmware and wait for the driver reload for completing the reset.
>> (This is similar to the legacy "ethtool --reset all" command).
>> live-fw-reset - Resets the currently running firmware and driver entities.
>> live-fw-patch - Loads the currently pending flashed firmware and
>> reloads all driver entities. If no pending flashed firmware, resets
>> currently loaded firmware.
>
>FWIW I'd prefer to extend the ethtool semantics. Ethtool reset has two
>reset "depths" already - single port, entire adapter, we could just add
>"entire sled" here. IOW we'd have reset which can affect only given
>port, then reset which can affect multiple ports, and reset which may
>affect multiple systems.

Hmm, I think that one way or another, we need to implement this in
devlink and have compat fallback from ethtool there (as we have for
other things too).


>
>The mechanism of the reset and whether old or new version of FW is
>activated is a detail, which I believe will be entirely uninteresting 
>to the user. Whether other systems or ports are affected is _very_
>important, OTOH.

Wait. So you say that user is not interested if the reset is fw "live"
or not? There might be all sorts of issues when the reset happens under
working driver instance. I think that user should be able to indicate if
he is willing to take the risk.


  reply	other threads:[~2020-06-02  6:31 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-31  7:03 [PATCH v3 net-next 0/6] bnxt_en: Add 'enable_live_dev_reset' and 'allow_live_dev_reset' generic devlink params Vasundhara Volam
2020-05-31  7:03 ` [PATCH v3 net-next 1/6] devlink: Add 'enable_live_dev_reset' generic parameter Vasundhara Volam
2020-06-01  6:19   ` Jiri Pirko
2020-05-31  7:03 ` [PATCH v3 net-next 2/6] devlink: Add 'allow_live_dev_reset' " Vasundhara Volam
2020-05-31  7:03 ` [PATCH v3 net-next 3/6] bnxt_en: Use 'enable_live_dev_reset' devlink parameter Vasundhara Volam
2020-05-31  7:03 ` [PATCH v3 net-next 4/6] bnxt_en: Update firmware spec. to 1.10.1.40 Vasundhara Volam
2020-05-31  7:03 ` [PATCH v3 net-next 5/6] bnxt_en: Use 'allow_live_dev_reset' devlink parameter Vasundhara Volam
2020-05-31  7:03 ` [PATCH v3 net-next 6/6] bnxt_en: Check if fw_live_reset is allowed before doing ETHTOOL_RESET Vasundhara Volam
2020-06-01  6:18 ` [PATCH v3 net-next 0/6] bnxt_en: Add 'enable_live_dev_reset' and 'allow_live_dev_reset' generic devlink params Jiri Pirko
2020-06-01  6:43   ` Jiri Pirko
2020-06-01  8:58     ` Vasundhara Volam
2020-06-01  9:52       ` Jiri Pirko
2020-06-01 10:08         ` Vasundhara Volam
2020-06-01 10:27           ` Jiri Pirko
2020-06-01 10:04       ` Jiri Pirko
2020-06-01 15:31         ` Vasundhara Volam
2020-06-01 23:24           ` Jakub Kicinski
2020-06-02  6:31             ` Jiri Pirko [this message]
2020-06-06 14:18           ` Vasundhara Volam
2020-06-01  9:58 ` Jiri Pirko
2020-06-01 10:12   ` Vasundhara Volam
2020-06-01 23:20   ` Jakub Kicinski

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=20200602063139.GT2282@nanopsycho \
    --to=jiri@resnulli.us \
    --cc=davem@davemloft.net \
    --cc=kuba@kernel.org \
    --cc=michael.chan@broadcom.com \
    --cc=netdev@vger.kernel.org \
    --cc=vasundhara-v.volam@broadcom.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.