[PATCH net-next RFC 02/13] devlink: Add reload levels data to dev get

2020-07-27 Thread Moshe Shemesh
Expose devlink reload supported levels and driver's default level to the
user through devlink dev get command.

Examples:
$ devlink dev show
pci/:82:00.0:
  reload_levels_info:
default_level driver
supported_levels:
  driver fw_reset fw_live_patch
pci/:82:00.1:
  reload_levels_info:
default_level driver
supported_levels:
  driver fw_reset fw_live_patch

$ devlink dev show -jp
{
"dev": {
"pci/:82:00.0": {
"reload_levels_info": {
"default_level": "driver",
"supported_levels": [ "driver","fw_reset","fw_live_patch" ]
}
},
"pci/:82:00.1": {
"reload_levels_info": {
"default_level": "driver",
"supported_levels": [ "driver","fw_reset","fw_live_patch" ]
}
}
}
}

Signed-off-by: Moshe Shemesh 
---
 include/uapi/linux/devlink.h |  3 +++
 net/core/devlink.c   | 38 +++-
 2 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index fa5f66db5012..249e921ff106 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -476,6 +476,9 @@ enum devlink_attr {
DEVLINK_ATTR_PORT_SPLITTABLE,   /* u8 */
 
DEVLINK_ATTR_RELOAD_LEVEL,  /* u8 */
+   DEVLINK_ATTR_RELOAD_DEFAULT_LEVEL,  /* u8 */
+   DEVLINK_ATTR_RELOAD_SUPPORTED_LEVELS,   /* nested */
+   DEVLINK_ATTR_RELOAD_LEVELS_INFO,/* nested */
 
/* add new attributes above here, update the policy in devlink.c */
 
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 31b367a1612d..f1812fc620d4 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -462,6 +462,11 @@ static int devlink_nl_put_handle(struct sk_buff *msg, 
struct devlink *devlink)
return 0;
 }
 
+static bool devlink_reload_supported(struct devlink *devlink)
+{
+   return devlink->ops->reload_down && devlink->ops->reload_up;
+}
+
 static bool
 devlink_reload_level_is_supported(struct devlink *devlink, enum 
devlink_reload_level level)
 {
@@ -472,7 +477,9 @@ static int devlink_nl_fill(struct sk_buff *msg, struct 
devlink *devlink,
   enum devlink_command cmd, u32 portid,
   u32 seq, int flags)
 {
+   struct nlattr *reload_levels_info, *supported_levels;
void *hdr;
+   int i;
 
hdr = genlmsg_put(msg, portid, seq, &devlink_nl_family, flags, cmd);
if (!hdr)
@@ -483,9 +490,35 @@ static int devlink_nl_fill(struct sk_buff *msg, struct 
devlink *devlink,
if (nla_put_u8(msg, DEVLINK_ATTR_RELOAD_FAILED, devlink->reload_failed))
goto nla_put_failure;
 
+   if (devlink_reload_supported(devlink)) {
+   reload_levels_info = nla_nest_start(msg, 
DEVLINK_ATTR_RELOAD_LEVELS_INFO);
+   if (!reload_levels_info)
+   goto nla_put_failure;
+   if (nla_put_u8(msg, DEVLINK_ATTR_RELOAD_DEFAULT_LEVEL,
+  devlink->ops->default_reload_level))
+   goto reload_levels_info_nest_cancel;
+
+   supported_levels = nla_nest_start(msg, 
DEVLINK_ATTR_RELOAD_SUPPORTED_LEVELS);
+   if (!supported_levels)
+   goto reload_levels_info_nest_cancel;
+
+   for (i = 0; i <= DEVLINK_RELOAD_LEVEL_MAX; i++) {
+   if (!devlink_reload_level_is_supported(devlink, i))
+   continue;
+   if (nla_put_u8(msg, DEVLINK_ATTR_RELOAD_LEVEL, i))
+   goto supported_levels_nest_cancel;
+   }
+   nla_nest_end(msg, supported_levels);
+   nla_nest_end(msg, reload_levels_info);
+   }
+
genlmsg_end(msg, hdr);
return 0;
 
+supported_levels_nest_cancel:
+   nla_nest_cancel(msg, supported_levels);
+reload_levels_info_nest_cancel:
+   nla_nest_cancel(msg, reload_levels_info);
 nla_put_failure:
genlmsg_cancel(msg, hdr);
return -EMSGSIZE;
@@ -2943,11 +2976,6 @@ static void devlink_reload_netns_change(struct devlink 
*devlink,
 DEVLINK_CMD_PARAM_NEW);
 }
 
-static bool devlink_reload_supported(const struct devlink *devlink)
-{
-   return devlink->ops->reload_down && devlink->ops->reload_up;
-}
-
 static void devlink_reload_failed_set(struct devlink *devlink,
  bool reload_failed)
 {
-- 
2.17.1



Re: [PATCH net-next RFC 02/13] devlink: Add reload levels data to dev get

2020-07-29 Thread Jakub Kicinski
On Wed, 29 Jul 2020 17:37:41 +0300 Moshe Shemesh wrote:
> > The fact that the driver supports fw_live_patch, does not necessarily
> > mean that the currently running FW can be live upgraded to the
> > currently flashed one, right?  
> 
> That's correct, though the feature is supported, the firmware gap may 
> not be suitable for live_patch.
> 
> The user will be noted accordingly by extack message.

That's kinda late, because use may have paid the cost of migrating the
workload or otherwise taking precautions - and if live reset fails all
this work is wasted.

While the device most likely knows upfront whether it can be live reset
or not, otherwise I don't see how it could reject the reset reliably.

> > This interface does not appear to be optimal for the purpose.
> >
> > Again, documentation of what can be lost (in terms of configuration and
> > features) upon upgrade is missing.  
> 
> I will clarify in documentation. On live_patch nothing should be lost or 
> re-initialized, that's the "live" thing.

Okay, so FW upgrade cannot be allowed when it'd mean the device gets
de-featured? Also no link loss, correct? What's the expected length of
traffic interruption (order of magnitude)?


Re: [PATCH net-next RFC 02/13] devlink: Add reload levels data to dev get

2020-07-30 Thread Moshe Shemesh



On 7/30/2020 12:11 AM, Jakub Kicinski wrote:

On Wed, 29 Jul 2020 17:37:41 +0300 Moshe Shemesh wrote:

The fact that the driver supports fw_live_patch, does not necessarily
mean that the currently running FW can be live upgraded to the
currently flashed one, right?

That's correct, though the feature is supported, the firmware gap may
not be suitable for live_patch.

The user will be noted accordingly by extack message.

That's kinda late, because use may have paid the cost of migrating the
workload or otherwise taking precautions - and if live reset fails all
this work is wasted.

While the device most likely knows upfront whether it can be live reset
or not, otherwise I don't see how it could reject the reset reliably.



The device knows if the new FW can be updated by live-patch or need 
reset once the new version is stored and it so it can check the gaps.


So once the new FW is stored I can query if it is a change that can do 
by live_patch or need full fw_reset.



This interface does not appear to be optimal for the purpose.

Again, documentation of what can be lost (in terms of configuration and
features) upon upgrade is missing.

I will clarify in documentation. On live_patch nothing should be lost or
re-initialized, that's the "live" thing.

Okay, so FW upgrade cannot be allowed when it'd mean the device gets
de-featured? Also no link loss, correct? What's the expected length of
traffic interruption (order of magnitude)?



That's different between fw_live_patch and fw_reset, that's why I see it 
as different level.


The live_patch is totally live, no link loss, no data interruption at all.

But when the firmware gap for upgrade is not suitable for live patch, 
the user can choose to do full fw reset, that can include link loss 
(depends on device) for few seconds and some configuration which is not 
saved by the driver or was not configured through the driver (some other 
tool) need to re-configure.





Re: [PATCH net-next RFC 02/13] devlink: Add reload levels data to dev get

2020-07-29 Thread Moshe Shemesh



On 7/28/2020 3:58 AM, Jakub Kicinski wrote:

On Mon, 27 Jul 2020 14:02:22 +0300 Moshe Shemesh wrote:

Expose devlink reload supported levels and driver's default level to the
user through devlink dev get command.

Examples:
$ devlink dev show
pci/:82:00.0:
   reload_levels_info:
 default_level driver
 supported_levels:
   driver fw_reset fw_live_patch
pci/:82:00.1:
   reload_levels_info:
 default_level driver
 supported_levels:
   driver fw_reset fw_live_patch

$ devlink dev show -jp
{
 "dev": {
 "pci/:82:00.0": {
 "reload_levels_info": {
 "default_level": "driver",
 "supported_levels": [ "driver","fw_reset","fw_live_patch" ]
 }
 },
 "pci/:82:00.1": {
 "reload_levels_info": {
 "default_level": "driver",
 "supported_levels": [ "driver","fw_reset","fw_live_patch" ]
 }
 }
 }
}

Signed-off-by: Moshe Shemesh 

The fact that the driver supports fw_live_patch, does not necessarily
mean that the currently running FW can be live upgraded to the
currently flashed one, right?



That's correct, though the feature is supported, the firmware gap may 
not be suitable for live_patch.


The user will be noted accordingly by extack message.



This interface does not appear to be optimal for the purpose.

Again, documentation of what can be lost (in terms of configuration and
features) upon upgrade is missing.



I will clarify in documentation. On live_patch nothing should be lost or 
re-initialized, that's the "live" thing.




Re: [PATCH net-next RFC 02/13] devlink: Add reload levels data to dev get

2020-07-27 Thread Jakub Kicinski
On Mon, 27 Jul 2020 14:02:22 +0300 Moshe Shemesh wrote:
> Expose devlink reload supported levels and driver's default level to the
> user through devlink dev get command.
> 
> Examples:
> $ devlink dev show
> pci/:82:00.0:
>   reload_levels_info:
> default_level driver
> supported_levels:
>   driver fw_reset fw_live_patch
> pci/:82:00.1:
>   reload_levels_info:
> default_level driver
> supported_levels:
>   driver fw_reset fw_live_patch
> 
> $ devlink dev show -jp
> {
> "dev": {
> "pci/:82:00.0": {
> "reload_levels_info": {
> "default_level": "driver",
> "supported_levels": [ "driver","fw_reset","fw_live_patch" ]
> }
> },
> "pci/:82:00.1": {
> "reload_levels_info": {
> "default_level": "driver",
> "supported_levels": [ "driver","fw_reset","fw_live_patch" ]
> }
> }
> }
> }
> 
> Signed-off-by: Moshe Shemesh 

The fact that the driver supports fw_live_patch, does not necessarily
mean that the currently running FW can be live upgraded to the
currently flashed one, right? 

This interface does not appear to be optimal for the purpose.

Again, documentation of what can be lost (in terms of configuration and
features) upon upgrade is missing.