LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
To: Moshe Shemesh <moshe@mellanox.com>
Cc: Jacob Keller <jacob.e.keller@intel.com>,
	"David S. Miller" <davem@davemloft.net>,
	Jiri Pirko <jiri@mellanox.com>, Netdev <netdev@vger.kernel.org>,
	open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH net-next RFC 00/13] Add devlink reload level option
Date: Wed, 12 Aug 2020 15:04:31 +0530
Message-ID: <CAACQVJqgoas2_pqBiphfHfZAdY5ysku=8AzDWf6Vde3j9Bxr8g@mail.gmail.com> (raw)
In-Reply-To: <da7a2f2d-3ff5-0cd1-f166-79d7355f3df0@mellanox.com>

On Wed, Aug 5, 2020 at 1:51 PM Moshe Shemesh <moshe@mellanox.com> wrote:
>
>
> On 8/5/2020 9:55 AM, Vasundhara Volam wrote:
> > On Wed, Aug 5, 2020 at 12:02 PM Moshe Shemesh <moshe@mellanox.com> wrote:
> >>
> >> On 8/4/2020 1:13 PM, Vasundhara Volam wrote:
> >>> On Mon, Aug 3, 2020 at 7:23 PM Moshe Shemesh <moshe@mellanox.com> wrote:
> >>>> On 8/3/2020 3:47 PM, Vasundhara Volam wrote:
> >>>>> On Mon, Aug 3, 2020 at 5:47 PM Moshe Shemesh <moshe@mellanox.com> wrote:
> >>>>>> On 8/3/2020 1:24 PM, Vasundhara Volam wrote:
> >>>>>>> On Tue, Jul 28, 2020 at 10:13 PM Jacob Keller <jacob.e.keller@intel.com> wrote:
> >>>>>>>> On 7/27/2020 10:25 PM, Vasundhara Volam wrote:
> >>>>>>>>> On Mon, Jul 27, 2020 at 4:36 PM Moshe Shemesh <moshe@mellanox.com> wrote:
> >>>>>>>>>> Introduce new option on devlink reload API to enable the user to select the
> >>>>>>>>>> reload level required. Complete support for all levels in mlx5.
> >>>>>>>>>> The following reload levels are supported:
> >>>>>>>>>>       driver: Driver entities re-instantiation only.
> >>>>>>>>>>       fw_reset: Firmware reset and driver entities re-instantiation.
> >>>>>>>>> The Name is a little confusing. I think it should be renamed to
> >>>>>>>>> fw_live_reset (in which both firmware and driver entities are
> >>>>>>>>> re-instantiated).  For only fw_reset, the driver should not undergo
> >>>>>>>>> reset (it requires a driver reload for firmware to undergo reset).
> >>>>>>>>>
> >>>>>>>> So, I think the differentiation here is that "live_patch" doesn't reset
> >>>>>>>> anything.
> >>>>>>> This seems similar to flashing the firmware and does not reset anything.
> >>>>>> The live patch is activating fw change without reset.
> >>>>>>
> >>>>>> It is not suitable for any fw change but fw gaps which don't require reset.
> >>>>>>
> >>>>>> I can query the fw to check if the pending image change is suitable or
> >>>>>> require fw reset.
> >>>>> Okay.
> >>>>>>>>>>       fw_live_patch: Firmware live patching only.
> >>>>>>>>> This level is not clear. Is this similar to flashing??
> >>>>>>>>>
> >>>>>>>>> Also I have a basic query. The reload command is split into
> >>>>>>>>> reload_up/reload_down handlers (Please correct me if this behaviour is
> >>>>>>>>> changed with this patchset). What if the vendor specific driver does
> >>>>>>>>> not support up/down and needs only a single handler to fire a firmware
> >>>>>>>>> reset or firmware live reset command?
> >>>>>>>> In the "reload_down" handler, they would trigger the appropriate reset,
> >>>>>>>> and quiesce anything that needs to be done. Then on reload up, it would
> >>>>>>>> restore and bring up anything quiesced in the first stage.
> >>>>>>> Yes, I got the "reload_down" and "reload_up". Similar to the device
> >>>>>>> "remove" and "re-probe" respectively.
> >>>>>>>
> >>>>>>> But our requirement is a similar "ethtool reset" command, where
> >>>>>>> ethtool calls a single callback in driver and driver just sends a
> >>>>>>> firmware command for doing the reset. Once firmware receives the
> >>>>>>> command, it will initiate the reset of driver and firmware entities
> >>>>>>> asynchronously.
> >>>>>> It is similar to mlx5 case here for fw_reset. The driver triggers the fw
> >>>>>> command to reset and all PFs drivers gets events to handle and do
> >>>>>> re-initialization.  To fit it to the devlink reload_down and reload_up,
> >>>>>> I wait for the event handler to complete and it stops at driver unload
> >>>>>> to have the driver up by devlink reload_up. See patch 8 in this patchset.
> >>>>>>
> >>>>> Yes, I see reload_down is triggering the reset. In our driver, after
> >>>>> triggering the reset through a firmware command, reset is done in
> >>>>> another context as the driver initiates the reset only after receiving
> >>>>> an ASYNC event from the firmware.
> >>>> Same here.
> >>>>
> >>>>> Probably, we have to use reload_down() to send firmware command to
> >>>>> trigger reset and do nothing in reload_up.
> >>>> I had that in previous version, but its wrong to use devlink reload this
> >>>> way, so I added wait with timeout for the event handling to complete
> >>>> before unload_down function ends. See mlx5_fw_wait_fw_reset_done(). Also
> >>>> the event handler stops before load back to have that done by devlink
> >>>> reload_up.
> >>> But "devlink dev reload" will be invoked by the user only on a single
> >>> dev handler and all function drivers will be re-instantiated upon the
> >>> ASYNC event. reload_down and reload_up are invoked only the function
> >>> which the user invoked.
> >>>
> >>> Take an example of a 2-port (PF0 and PF1) adapter on a single host and
> >>> with some VFs loaded on the device. User invokes "devlink dev reload"
> >>> on PF0, ASYNC event is received on 2 PFs and VFs for reset. All the
> >>> function drivers will be re-instantiated including PF0.
> >>>
> >>> If we wait for some time in reload_down() of PF0 and then call load in
> >>> reload_up(), this code will be different from other function drivers.
> >>
> >> I see your point here, but the user run devlink reload command on one
> >> PF, in this case of fw-reset it will influence other PFs, but that's a
> >> result of the fw-reset, the user if asked for params change or namespace
> >> change that was for this PF.
> > Right, if any driver is implementing only fw-reset have to leave
> > reload_up as an empty function.
>
>
> No, its not only up the driver. The netns option is implemented by
> devlink and its running between reload_down and reload_up.
What I mean is, driver will provide a reload_up handler but it will
not do anything and simply return 0.
>
> >>>>>     And returning from reload
> >>>>> does not mean that reset is complete as it is done in another context
> >>>>> and the driver notifies the health reporter once the reset is
> >>>>> complete. devlink framework may have to allow drivers to implement
> >>>>> reload_down only to look more clean or call reload_up only if the
> >>>>> driver notifies the devlink once reset is completed from another
> >>>>> context. Please suggest.

  reply index

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-27 11:02 Moshe Shemesh
2020-07-27 11:02 ` [PATCH net-next RFC 01/13] devlink: Add reload level option to devlink reload command Moshe Shemesh
2020-07-28  0:58   ` Jakub Kicinski
2020-07-28 13:58     ` Jiri Pirko
2020-07-28 16:47       ` Jacob Keller
2020-07-28 18:44         ` Jakub Kicinski
2020-07-28 19:18           ` Jacob Keller
2020-07-28 20:06             ` Jakub Kicinski
2020-07-29 14:54               ` Moshe Shemesh
2020-07-29 21:07                 ` Jakub Kicinski
2020-07-30 12:30                   ` Moshe Shemesh
2020-07-30 23:11                     ` Jakub Kicinski
2020-08-01 21:32                       ` Moshe Shemesh
2020-08-03 14:14                         ` Jiri Pirko
2020-08-03 20:57                           ` Jakub Kicinski
2020-08-04 10:04                             ` Jiri Pirko
2020-08-04 20:39                               ` Jakub Kicinski
2020-08-05 11:02                                 ` Jiri Pirko
2020-08-06 18:25                                   ` Jakub Kicinski
2020-08-06 22:56                                     ` Jacob Keller
2020-08-09 13:21                                     ` Moshe Shemesh
2020-08-10 16:53                                       ` Jakub Kicinski
2020-08-10 17:09                                         ` Jacob Keller
2020-08-10 18:17                                           ` Jakub Kicinski
2020-08-11  5:46                                         ` Jiri Pirko
2020-07-27 11:02 ` [PATCH net-next RFC 02/13] devlink: Add reload levels data to dev get Moshe Shemesh
2020-07-28  0:58   ` Jakub Kicinski
2020-07-29 14:37     ` Moshe Shemesh
2020-07-29 21:11       ` Jakub Kicinski
2020-07-30 12:05         ` Moshe Shemesh
2020-07-27 11:02 ` [PATCH net-next RFC 03/13] net/mlx5: Add functions to set/query MFRL register Moshe Shemesh
2020-07-27 11:02 ` [PATCH net-next RFC 04/13] net/mlx5: Set cap for pci sync for fw update event Moshe Shemesh
2020-07-27 11:02 ` [PATCH net-next RFC 05/13] net/mlx5: Handle sync reset request event Moshe Shemesh
2020-07-27 11:02 ` [PATCH net-next RFC 06/13] net/mlx5: Handle sync reset now event Moshe Shemesh
2020-07-27 11:02 ` [PATCH net-next RFC 07/13] net/mlx5: Handle sync reset abort event Moshe Shemesh
2020-07-27 11:02 ` [PATCH net-next RFC 08/13] net/mlx5: Add support for devlink reload level fw reset Moshe Shemesh
2020-07-27 11:02 ` [PATCH net-next RFC 09/13] devlink: Add enable_remote_dev_reset generic parameter Moshe Shemesh
2020-07-28  0:59   ` Jakub Kicinski
2020-07-29 14:42     ` Moshe Shemesh
2020-07-29 20:57       ` Jakub Kicinski
2020-07-30 12:08         ` Moshe Shemesh
2020-07-27 11:02 ` [PATCH net-next RFC 10/13] net/mlx5: Add devlink param enable_remote_dev_reset support Moshe Shemesh
2020-07-28  0:59   ` Jakub Kicinski
2020-07-27 11:02 ` [PATCH net-next RFC 11/13] net/mlx5: Add support for fw live patch event Moshe Shemesh
2020-07-27 11:02 ` [PATCH net-next RFC 12/13] net/mlx5: Add support for devlink reload level live patch Moshe Shemesh
2020-07-27 11:02 ` [PATCH net-next RFC 13/13] devlink: Add Documentation/networking/devlink/devlink-reload.rst Moshe Shemesh
2020-07-28  5:25 ` [PATCH net-next RFC 00/13] Add devlink reload level option Vasundhara Volam
2020-07-28 16:43   ` Jacob Keller
2020-08-03 10:24     ` Vasundhara Volam
2020-08-03 12:17       ` Moshe Shemesh
2020-08-03 12:47         ` Vasundhara Volam
2020-08-03 13:52           ` Moshe Shemesh
2020-08-04 10:13             ` Vasundhara Volam
2020-08-05  6:32               ` Moshe Shemesh
2020-08-05  6:55                 ` Vasundhara Volam
2020-08-05  8:20                   ` Moshe Shemesh
2020-08-12  9:34                     ` Vasundhara Volam [this message]
2020-07-28 16:37 ` Jacob Keller

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='CAACQVJqgoas2_pqBiphfHfZAdY5ysku=8AzDWf6Vde3j9Bxr8g@mail.gmail.com' \
    --to=vasundhara-v.volam@broadcom.com \
    --cc=davem@davemloft.net \
    --cc=jacob.e.keller@intel.com \
    --cc=jiri@mellanox.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=moshe@mellanox.com \
    --cc=netdev@vger.kernel.org \
    /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

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git
	git clone --mirror https://lore.kernel.org/lkml/9 lkml/git/9.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git