Re: [PATCH net-next v5 01/10] devlink: Add permanent config parameter get/set operations

[Jakub Kicinski <kubakici@wp.pl>]

On Mon, 30 Oct 2017 18:03:01 +0100, Jiri Pirko wrote:
> >+	cfgparam_attr = nla_nest_start(msg, DEVLINK_ATTR_PERM_CONFIG);
> >+	/* Update restart reqd - if any param needs restart, should be set */
> >+	if (need_restart) {  
> 
> You should propagate this out so the caller would fill it to the
> message. This is a global thing, not per-param, shoult not be nested.

Right, I think Jiri has already asked for this but I feel like we
should provide the ability to query running/pending configurations
independently.  I don't see it in this patch set?

I'm not sure what the status of the reconfig trigger patches for mlxsw
is, but we actually may need 3 config sets:
 - current/runtime configurable, 
 - requiring soft reset of the device/driver reinit;
 - requiring hard reset/set on boot.

Secondly, IMHO calling set/get parameters "permanent" is a bit
backwards.  One device may not be able to change max VF counts or MSIX
allocation without full reinit of PCIe blocks, but for others soft
reset is more than enough.  Port splitting is another example.  For 
NICs port splitting at runtime is usually not a priority in HW/FW
development, so some form of reset is generally required, while
switches can split a port at runtime.  IOW we should define parameters
without assigning them to config sets in the ABI itself.  And also we
should make it in a way which will allow existing parameters to be
reused in permanent/sort reset required/runtime modes.

Does that make sense?