linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Jiri Pirko <jiri@resnulli.us>
Cc: Moshe Shemesh <moshe@mellanox.com>,
	Jacob Keller <jacob.e.keller@intel.com>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	"David S. Miller" <davem@davemloft.net>,
	Jiri Pirko <jiri@mellanox.com>,
	Vasundhara Volam <vasundhara-v.volam@broadcom.com>
Subject: Re: [PATCH net-next RFC 01/13] devlink: Add reload level option to devlink reload command
Date: Thu, 6 Aug 2020 11:25:30 -0700	[thread overview]
Message-ID: <20200806112530.0588b3ac@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> (raw)
In-Reply-To: <20200805110258.GA2169@nanopsycho>

On Wed, 5 Aug 2020 13:02:58 +0200 Jiri Pirko wrote:
> Tue, Aug 04, 2020 at 10:39:46PM CEST, kuba@kernel.org wrote:
> >AFAIU the per-driver default is needed because we went too low 
> >level with what the action constitutes. We need maintain the higher
> >level actions.
> >
> >The user clearly did not care if FW was reset during devlink reload
> >before this set, so what has changed? The objective user has is to  
> 
> Well for mlxsw, the user is used to this flow:
> devlink dev flash - flash new fw
> devlink dev reload - new fw is activated and reset and driver instances
> are re-created.

Ugh, if the current behavior already implies fw-activation for some
drivers then the default has to probably be "do all the things" :S

> >activate their config / FW / move to different net ns. 
> >
> >Reloading the driver or resetting FW is a low level detail which
> >achieves different things for different implementations. So it's 
> >not a suitable abstraction -> IOW we need the driver default.  
> 
> I'm confused. So you think we need the driver default?

No, I'm talking about the state of this patch set. _In this patchset_ 
we need a driver default because of the unsuitable abstraction.

Better design would not require it.

> >The work flow for the user is:
> >
> >0. download fw to /lib/firmware
> >1. devlink flash $dev $fw
> >2. if live activation is enabled
> >   yes - devlink reload $dev $live-activate
> >   no - report machine has to be drained for reboot
> >
> >fw-reset can't be $live-activate, because as Jake said fw-reset does
> >not activate the new image for Intel. So will we end up per-driver
> >defaults in the kernel space, and user space maintaining a mapping from  
> 
> Well, that is what what is Moshe's proposal. Per-driver kernel default..
> I'm not sure what we are arguing about then :/

The fact that if I do a pure "driver reload" it will active new
firmware for mlxsw but not for mlx5. In this patchset for mlx5 I need
driver reload fw-reset. And for Intel there is no suitable option.

> >a driver to what a "level" of reset implies.
> >
> >I hope this makes things crystal clear. Please explain what problems
> >you're seeing and extensions you're expecting. A list of user scenarios
> >you foresee would be v. useful.  

  reply	other threads:[~2020-08-06 18:41 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-27 11:02 [PATCH net-next RFC 00/13] Add devlink reload level option 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 [this message]
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
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=20200806112530.0588b3ac@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com \
    --to=kuba@kernel.org \
    --cc=davem@davemloft.net \
    --cc=jacob.e.keller@intel.com \
    --cc=jiri@mellanox.com \
    --cc=jiri@resnulli.us \
    --cc=linux-kernel@vger.kernel.org \
    --cc=moshe@mellanox.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).