[PATCH net-next v7 0/8] devlink: Add configuration parameters support for devlink_port

[PATCH net-next v7 0/8] devlink: Add configuration parameters support for devlink_port

[Vasundhara Volam]

This patchset adds support for configuration parameters setting through
devlink_port.  Each device registers supported configuration parameters
table.

The user can retrieve data on these parameters by
"devlink port param show" command and can set new value to a
parameter by "devlink port param set" command.
All configuration modes supported by devlink_dev are supported
by devlink_port also.

Command examples and output:

# devlink port param show
pci/0000:3b:00.0/0:
  name wake-on-lan type generic
    values:
      cmode permanent value false

pci/0000:3b:00.1/1:
  name wake-on-lan type generic
    values:
      cmode permanent value false

pci/0000:af:00.0/0:
  name wake-on-lan type generic
    values:
      cmode permanent value true

# devlink port param show pci/0000:3b:00.0/0 name wake-on-lan
pci/0000:3b:00.0/0:
  name wake-on-lan type generic
    values:
      cmode permanent value false

# devlink port param set pci/0000:3b:00.0/0 name wake-on-lan cmode permanent value true

There is difference of opinion on adding WOL parameter to devlink, between
Jakub Kicinski and Michael Chan.

Quote from Jakud Kicinski:
********
As explained previously I think it's a very bad idea to add existing
configuration options to devlink, just because devlink has the ability
to persist the setting in NVM.  Especially that for WoL you have to get
the link up so you potentially have all link config stuff as well.  And
that n-tuple filters are one of the WoL options, meaning we'd need the
ability to persist n-tuple filters via devlink.

The effort would be far better spent helping with migrating ethtool to
netlink, and allowing persisting there.

I have not heard any reason why devlink is a better fit.  I can imagine
you're just doing it here because it's less effort for you since
ethtool is not yet migrated.
********

Quote from Michael Chan:
********
The devlink's WoL parameter is a persistent WoL parameter stored in the
NIC's NVRAM. It is different from ethtool's WoL parameter in a number of
ways. ethtool WoL is not persistent over AC power cycle and is considered
OS-present WoL. As such, ethtool WoL can use a more sophisticated pattern
including n-tuple with IP address in addition to the more basic types
(e.g. magic packet). Whereas OS-absent power up WoL should only include
magic packet and other simple types. The devlink WoL setting does not have
to match the ethtool WoL setting. The card will autoneg up to the speed
supported by Vaux so no special devlink link setting is needed.
********

v6->v7:
* Remove RFC tag from the patch-set.

v5->v6:
* Replace '-' with '*' in cover letter to avoid cutoff by git.

v4->v5:
* Added quotes from Jakub Kicinski and Michael chan on devlink's WOL
  parameter in the cover letter.

v3->v4:
* Update changes done from v2 to v3 version in individual patch
  descriptions.

v2->v3:
Make following changes as per suggestions from Jiri Pirko and
Michal Kubecek.
* Add a helper __devlink_params_register() with common code used by
  both devlink_params_register() and devlink_port_params_register().
* Define only WOL types used now and define them as bitfield, so that
  mutliple WOL types can be enabled upon power on.
* Modify "wake-on-lan" name to "wake_on_lan" to be symmetric with
  previous definitions.
* Rename DEVLINK_PARAM_WOL_XXX to DEVLINK_PARAM_WAKE_XXX to be
  symmetrical with ethtool WOL definitions.
* Modify bnxt_dl_wol_validate(), to throw error message when user gives
  value other than DEVLINK_PARAM_WAKE_MAGIC or to disable WOL.
* Use netdev_err() instead of netdev_warn(), when devlink_port_register()
  and devlink_port_params_register() returns error. Also, don't log rc
  in this message.

v1->v2:
Make following changes as per suggestions from Jiri Pirko.
* Remove separate enum devlink_port_param_generic_id for port params.
  Instead club it with existing device params. Accordingly refactor
  remaining patchset.
* Move INIT_LIST_HEAD of port param_list to devlink_port_register()
* Add a helper devlink_param_verify() to be used for both
  devlink_params_register() and devlink_port_params_register().
* Add a helper __devlink_params_unregister() for common code in
  devlink_params_unregister() and devlink_port_params_unregister().
* Move DEVLINK_CMD_PORT_PARAM_XXX definitions to the end of the enum.
* Split the patches for devlink_port_param_driverinit_value_get() and
  devlink_port_param_driverinit_value_set() into separate patches.
* define DEVLINK_PARAM_GENERIC_ID_WOL type as u8 and define enum for
  different types of WOL. Accordingly modify bnxt_en patch to validate
  wol type.

Vasundhara Volam (8):
  devlink: Add devlink_param for port register and unregister
  devlink: Add port param get command
  devlink: Add port param set command
  devlink: Add support for driverinit get value for devlink_port
  devlink: Add support for driverinit set value for devlink_port
  devlink: Add devlink notifications support for port params
  devlink: Add a generic wake_on_lan port parameter
  bnxt_en: Add bnxt_en initial port params table and register it

 drivers/net/ethernet/broadcom/bnxt/bnxt.h         |   1 +
 drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c |  43 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h |   1 +
 include/net/devlink.h                             |  57 +++
 include/uapi/linux/devlink.h                      |   5 +
 net/core/devlink.c                                | 464 ++++++++++++++++++----
 6 files changed, 489 insertions(+), 82 deletions(-)

-- 
1.8.3.1

[PATCH net-next v7 1/8] devlink: Add devlink_param for port register and unregister

[Vasundhara Volam]

Add functions to register and unregister for the driver supported
configuration parameters table per port.

v2->v3:
- Add a helper __devlink_params_register() with common code used by
  both devlink_params_register() and devlink_port_params_register().

Cc: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
---
 include/net/devlink.h |  22 +++++++++
 net/core/devlink.c    | 133 +++++++++++++++++++++++++++++++++++---------------
 2 files changed, 117 insertions(+), 38 deletions(-)

diff --git a/include/net/devlink.h b/include/net/devlink.h
index 67f4293..98b8a66 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -48,6 +48,7 @@ struct devlink_port_attrs {
 
 struct devlink_port {
 	struct list_head list;
+	struct list_head param_list;
 	struct devlink *devlink;
 	unsigned index;
 	bool registered;
@@ -574,6 +575,12 @@ int devlink_param_driverinit_value_set(struct devlink *devlink, u32 param_id,
 void devlink_param_value_changed(struct devlink *devlink, u32 param_id);
 void devlink_param_value_str_fill(union devlink_param_value *dst_val,
 				  const char *src);
+int devlink_port_params_register(struct devlink_port *devlink_port,
+				 const struct devlink_param *params,
+				 size_t params_count);
+void devlink_port_params_unregister(struct devlink_port *devlink_port,
+				    const struct devlink_param *params,
+				    size_t params_count);
 struct devlink_region *devlink_region_create(struct devlink *devlink,
 					     const char *region_name,
 					     u32 region_max_snapshots,
@@ -816,6 +823,21 @@ static inline bool devlink_dpipe_table_counter_enabled(struct devlink *devlink,
 {
 }
 
+static inline int
+devlink_port_params_register(struct devlink_port *devlink_port,
+			     const struct devlink_param *params,
+			     size_t params_count)
+{
+	return 0;
+}
+
+static inline void
+devlink_port_params_unregister(struct devlink_port *devlink_port,
+			       const struct devlink_param *params,
+			       size_t params_count)
+{
+}
+
 static inline struct devlink_region *
 devlink_region_create(struct devlink *devlink,
 		      const char *region_name,
diff --git a/net/core/devlink.c b/net/core/devlink.c
index abb0da9..41b60f6 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -3147,12 +3147,12 @@ static int devlink_nl_cmd_param_set_doit(struct sk_buff *skb,
 }
 
 static int devlink_param_register_one(struct devlink *devlink,
+				      struct list_head *param_list,
 				      const struct devlink_param *param)
 {
 	struct devlink_param_item *param_item;
 
-	if (devlink_param_find_by_name(&devlink->param_list,
-				       param->name))
+	if (devlink_param_find_by_name(param_list, param->name))
 		return -EEXIST;
 
 	if (param->supported_cmodes == BIT(DEVLINK_PARAM_CMODE_DRIVERINIT))
@@ -3165,18 +3165,18 @@ static int devlink_param_register_one(struct devlink *devlink,
 		return -ENOMEM;
 	param_item->param = param;
 
-	list_add_tail(&param_item->list, &devlink->param_list);
+	list_add_tail(&param_item->list, param_list);
 	devlink_param_notify(devlink, param_item, DEVLINK_CMD_PARAM_NEW);
 	return 0;
 }
 
 static void devlink_param_unregister_one(struct devlink *devlink,
+					 struct list_head *param_list,
 					 const struct devlink_param *param)
 {
 	struct devlink_param_item *param_item;
 
-	param_item = devlink_param_find_by_name(&devlink->param_list,
-						param->name);
+	param_item = devlink_param_find_by_name(param_list, param->name);
 	WARN_ON(!param_item);
 	devlink_param_notify(devlink, param_item, DEVLINK_CMD_PARAM_DEL);
 	list_del(&param_item->list);
@@ -3954,6 +3954,7 @@ int devlink_port_register(struct devlink *devlink,
 	devlink_port->index = port_index;
 	devlink_port->registered = true;
 	list_add_tail(&devlink_port->list, &devlink->port_list);
+	INIT_LIST_HEAD(&devlink_port->param_list);
 	mutex_unlock(&devlink->lock);
 	devlink_port_notify(devlink_port, DEVLINK_CMD_PORT_NEW);
 	return 0;
@@ -4471,18 +4472,20 @@ void devlink_resource_occ_get_unregister(struct devlink *devlink,
 }
 EXPORT_SYMBOL_GPL(devlink_resource_occ_get_unregister);
 
-/**
- *	devlink_params_register - register configuration parameters
- *
- *	@devlink: devlink
- *	@params: configuration parameters array
- *	@params_count: number of parameters provided
- *
- *	Register the configuration parameters supported by the driver.
- */
-int devlink_params_register(struct devlink *devlink,
-			    const struct devlink_param *params,
-			    size_t params_count)
+static int devlink_param_verify(const struct devlink_param *param)
+{
+	if (!param || !param->name || !param->supported_cmodes)
+		return -EINVAL;
+	if (param->generic)
+		return devlink_param_generic_verify(param);
+	else
+		return devlink_param_driver_verify(param);
+}
+
+static int __devlink_params_register(struct devlink *devlink,
+				     struct list_head *param_list,
+				     const struct devlink_param *params,
+				     size_t params_count)
 {
 	const struct devlink_param *param = params;
 	int i;
@@ -4490,20 +4493,11 @@ int devlink_params_register(struct devlink *devlink,
 
 	mutex_lock(&devlink->lock);
 	for (i = 0; i < params_count; i++, param++) {
-		if (!param || !param->name || !param->supported_cmodes) {
-			err = -EINVAL;
+		err = devlink_param_verify(param);
+		if (err)
 			goto rollback;
-		}
-		if (param->generic) {
-			err = devlink_param_generic_verify(param);
-			if (err)
-				goto rollback;
-		} else {
-			err = devlink_param_driver_verify(param);
-			if (err)
-				goto rollback;
-		}
-		err = devlink_param_register_one(devlink, param);
+
+		err = devlink_param_register_one(devlink, param_list, param);
 		if (err)
 			goto rollback;
 	}
@@ -4515,31 +4509,57 @@ int devlink_params_register(struct devlink *devlink,
 	if (!i)
 		goto unlock;
 	for (param--; i > 0; i--, param--)
-		devlink_param_unregister_one(devlink, param);
+		devlink_param_unregister_one(devlink, param_list, param);
 unlock:
 	mutex_unlock(&devlink->lock);
 	return err;
 }
-EXPORT_SYMBOL_GPL(devlink_params_register);
 
 /**
- *	devlink_params_unregister - unregister configuration parameters
+ *	devlink_params_register - register configuration parameters
+ *
  *	@devlink: devlink
- *	@params: configuration parameters to unregister
+ *	@params: configuration parameters array
  *	@params_count: number of parameters provided
+ *
+ *	Register the configuration parameters supported by the driver.
  */
-void devlink_params_unregister(struct devlink *devlink,
-			       const struct devlink_param *params,
-			       size_t params_count)
+int devlink_params_register(struct devlink *devlink,
+			    const struct devlink_param *params,
+			    size_t params_count)
+{
+	return __devlink_params_register(devlink, &devlink->param_list, params,
+					 params_count);
+}
+EXPORT_SYMBOL_GPL(devlink_params_register);
+
+static void __devlink_params_unregister(struct devlink *devlink,
+					struct list_head *param_list,
+					const struct devlink_param *params,
+					size_t params_count)
 {
 	const struct devlink_param *param = params;
 	int i;
 
 	mutex_lock(&devlink->lock);
 	for (i = 0; i < params_count; i++, param++)
-		devlink_param_unregister_one(devlink, param);
+		devlink_param_unregister_one(devlink, param_list, param);
 	mutex_unlock(&devlink->lock);
 }
+
+/**
+ *	devlink_params_unregister - unregister configuration parameters
+ *	@devlink: devlink
+ *	@params: configuration parameters to unregister
+ *	@params_count: number of parameters provided
+ */
+void devlink_params_unregister(struct devlink *devlink,
+			       const struct devlink_param *params,
+			       size_t params_count)
+{
+	return __devlink_params_unregister(devlink, &devlink->param_list,
+					   params, params_count);
+}
 EXPORT_SYMBOL_GPL(devlink_params_unregister);
 
 /**
@@ -4657,6 +4677,43 @@ void devlink_param_value_str_fill(union devlink_param_value *dst_val,
 EXPORT_SYMBOL_GPL(devlink_param_value_str_fill);
 
 /**
+ *	devlink_port_params_register - register port configuration parameters
+ *
+ *	@devlink_port: devlink port
+ *	@params: configuration parameters array
+ *	@params_count: number of parameters provided
+ *
+ *	Register the configuration parameters supported by the port.
+ */
+int devlink_port_params_register(struct devlink_port *devlink_port,
+				 const struct devlink_param *params,
+				 size_t params_count)
+{
+	return __devlink_params_register(devlink_port->devlink,
+					 &devlink_port->param_list, params,
+					 params_count);
+}
+EXPORT_SYMBOL_GPL(devlink_port_params_register);
+
+/**
+ *	devlink_port_params_unregister - unregister port configuration
+ *	parameters
+ *
+ *	@devlink_port: devlink port
+ *	@params: configuration parameters array
+ *	@params_count: number of parameters provided
+ */
+void devlink_port_params_unregister(struct devlink_port *devlink_port,
+				    const struct devlink_param *params,
+				    size_t params_count)
+{
+	return __devlink_params_unregister(devlink_port->devlink,
+					   &devlink_port->param_list,
+					   params, params_count);
+}
+EXPORT_SYMBOL_GPL(devlink_port_params_unregister);
+
+/**
  *	devlink_region_create - create a new address region
  *
  *	@devlink: devlink
-- 
1.8.3.1

[PATCH net-next v7 2/8] devlink: Add port param get command

[Vasundhara Volam]

Add port param get command which gets data per parameter.
It also has option to dump the parameters data per port.

Cc: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
---
 include/uapi/linux/devlink.h |   2 +
 net/core/devlink.c           | 102 ++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 97 insertions(+), 7 deletions(-)

diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 6e52d36..448973b 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -89,6 +89,8 @@ enum devlink_command {
 	DEVLINK_CMD_REGION_DEL,
 	DEVLINK_CMD_REGION_READ,
 
+	DEVLINK_CMD_PORT_PARAM_GET,	/* can dump */
+
 	/* add new commands above here */
 	__DEVLINK_CMD_MAX,
 	DEVLINK_CMD_MAX = __DEVLINK_CMD_MAX - 1
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 41b60f6..30f0b36 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -2843,6 +2843,7 @@ static int devlink_param_set(struct devlink *devlink,
 }
 
 static int devlink_nl_param_fill(struct sk_buff *msg, struct devlink *devlink,
+				 unsigned int port_index,
 				 struct devlink_param_item *param_item,
 				 enum devlink_command cmd,
 				 u32 portid, u32 seq, int flags)
@@ -2880,6 +2881,11 @@ static int devlink_nl_param_fill(struct sk_buff *msg, struct devlink *devlink,
 
 	if (devlink_nl_put_handle(msg, devlink))
 		goto genlmsg_cancel;
+
+	if (cmd == DEVLINK_CMD_PORT_PARAM_GET)
+		if (nla_put_u32(msg, DEVLINK_ATTR_PORT_INDEX, port_index))
+			goto genlmsg_cancel;
+
 	param_attr = nla_nest_start(msg, DEVLINK_ATTR_PARAM);
 	if (!param_attr)
 		goto genlmsg_cancel;
@@ -2933,7 +2939,7 @@ static void devlink_param_notify(struct devlink *devlink,
 	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
 	if (!msg)
 		return;
-	err = devlink_nl_param_fill(msg, devlink, param_item, cmd, 0, 0, 0);
+	err = devlink_nl_param_fill(msg, devlink, 0, param_item, cmd, 0, 0, 0);
 	if (err) {
 		nlmsg_free(msg);
 		return;
@@ -2962,7 +2968,7 @@ static int devlink_nl_cmd_param_get_dumpit(struct sk_buff *msg,
 				idx++;
 				continue;
 			}
-			err = devlink_nl_param_fill(msg, devlink, param_item,
+			err = devlink_nl_param_fill(msg, devlink, 0, param_item,
 						    DEVLINK_CMD_PARAM_GET,
 						    NETLINK_CB(cb->skb).portid,
 						    cb->nlh->nlmsg_seq,
@@ -3051,7 +3057,7 @@ static int devlink_nl_cmd_param_get_dumpit(struct sk_buff *msg,
 }
 
 static struct devlink_param_item *
-devlink_param_get_from_info(struct devlink *devlink,
+devlink_param_get_from_info(struct list_head *param_list,
 			    struct genl_info *info)
 {
 	char *param_name;
@@ -3060,7 +3066,7 @@ static int devlink_nl_cmd_param_get_dumpit(struct sk_buff *msg,
 		return NULL;
 
 	param_name = nla_data(info->attrs[DEVLINK_ATTR_PARAM_NAME]);
-	return devlink_param_find_by_name(&devlink->param_list, param_name);
+	return devlink_param_find_by_name(param_list, param_name);
 }
 
 static int devlink_nl_cmd_param_get_doit(struct sk_buff *skb,
@@ -3071,7 +3077,7 @@ static int devlink_nl_cmd_param_get_doit(struct sk_buff *skb,
 	struct sk_buff *msg;
 	int err;
 
-	param_item = devlink_param_get_from_info(devlink, info);
+	param_item = devlink_param_get_from_info(&devlink->param_list, info);
 	if (!param_item)
 		return -EINVAL;
 
@@ -3079,7 +3085,7 @@ static int devlink_nl_cmd_param_get_doit(struct sk_buff *skb,
 	if (!msg)
 		return -ENOMEM;
 
-	err = devlink_nl_param_fill(msg, devlink, param_item,
+	err = devlink_nl_param_fill(msg, devlink, 0, param_item,
 				    DEVLINK_CMD_PARAM_GET,
 				    info->snd_portid, info->snd_seq, 0);
 	if (err) {
@@ -3102,7 +3108,7 @@ static int devlink_nl_cmd_param_set_doit(struct sk_buff *skb,
 	union devlink_param_value value;
 	int err = 0;
 
-	param_item = devlink_param_get_from_info(devlink, info);
+	param_item = devlink_param_get_from_info(&devlink->param_list, info);
 	if (!param_item)
 		return -EINVAL;
 	param = param_item->param;
@@ -3183,6 +3189,80 @@ static void devlink_param_unregister_one(struct devlink *devlink,
 	kfree(param_item);
 }
 
+static int devlink_nl_cmd_port_param_get_dumpit(struct sk_buff *msg,
+						struct netlink_callback *cb)
+{
+	struct devlink_param_item *param_item;
+	struct devlink_port *devlink_port;
+	struct devlink *devlink;
+	int start = cb->args[0];
+	int idx = 0;
+	int err;
+
+	mutex_lock(&devlink_mutex);
+	list_for_each_entry(devlink, &devlink_list, list) {
+		if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
+			continue;
+		mutex_lock(&devlink->lock);
+		list_for_each_entry(devlink_port, &devlink->port_list, list) {
+			list_for_each_entry(param_item,
+					    &devlink_port->param_list, list) {
+				if (idx < start) {
+					idx++;
+					continue;
+				}
+				err = devlink_nl_param_fill(msg,
+						devlink_port->devlink,
+						devlink_port->index, param_item,
+						DEVLINK_CMD_PORT_PARAM_GET,
+						NETLINK_CB(cb->skb).portid,
+						cb->nlh->nlmsg_seq,
+						NLM_F_MULTI);
+				if (err) {
+					mutex_unlock(&devlink->lock);
+					goto out;
+				}
+				idx++;
+			}
+		}
+		mutex_unlock(&devlink->lock);
+	}
+out:
+	mutex_unlock(&devlink_mutex);
+
+	cb->args[0] = idx;
+	return msg->len;
+}
+
+static int devlink_nl_cmd_port_param_get_doit(struct sk_buff *skb,
+					      struct genl_info *info)
+{
+	struct devlink_port *devlink_port = info->user_ptr[0];
+	struct devlink_param_item *param_item;
+	struct sk_buff *msg;
+	int err;
+
+	param_item = devlink_param_get_from_info(&devlink_port->param_list,
+						 info);
+	if (!param_item)
+		return -EINVAL;
+
+	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+	if (!msg)
+		return -ENOMEM;
+
+	err = devlink_nl_param_fill(msg, devlink_port->devlink,
+				    devlink_port->index, param_item,
+				    DEVLINK_CMD_PORT_PARAM_GET,
+				    info->snd_portid, info->snd_seq, 0);
+	if (err) {
+		nlmsg_free(msg);
+		return err;
+	}
+
+	return genlmsg_reply(msg, info);
+}
+
 static int devlink_nl_region_snapshot_id_put(struct sk_buff *msg,
 					     struct devlink *devlink,
 					     struct devlink_snapshot *snapshot)
@@ -3821,6 +3901,14 @@ static int devlink_nl_cmd_region_read_dumpit(struct sk_buff *skb,
 		.internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK,
 	},
 	{
+		.cmd = DEVLINK_CMD_PORT_PARAM_GET,
+		.doit = devlink_nl_cmd_port_param_get_doit,
+		.dumpit = devlink_nl_cmd_port_param_get_dumpit,
+		.policy = devlink_nl_policy,
+		.internal_flags = DEVLINK_NL_FLAG_NEED_PORT,
+		/* can be retrieved by unprivileged users */
+	},
+	{
 		.cmd = DEVLINK_CMD_REGION_GET,
 		.doit = devlink_nl_cmd_region_get_doit,
 		.dumpit = devlink_nl_cmd_region_get_dumpit,
-- 
1.8.3.1

[PATCH net-next v7 3/8] devlink: Add port param set command

[Vasundhara Volam]

Add port param set command to set the value for a parameter.
Value can be set to any of the supported configuration modes.

Cc: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
---
 include/uapi/linux/devlink.h |  1 +
 net/core/devlink.c           | 37 ++++++++++++++++++++++++++++++++-----
 2 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 448973b..3658fb2 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -90,6 +90,7 @@ enum devlink_command {
 	DEVLINK_CMD_REGION_READ,
 
 	DEVLINK_CMD_PORT_PARAM_GET,	/* can dump */
+	DEVLINK_CMD_PORT_PARAM_SET,
 
 	/* add new commands above here */
 	__DEVLINK_CMD_MAX,
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 30f0b36..be083a9 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -3096,10 +3096,11 @@ static int devlink_nl_cmd_param_get_doit(struct sk_buff *skb,
 	return genlmsg_reply(msg, info);
 }
 
-static int devlink_nl_cmd_param_set_doit(struct sk_buff *skb,
-					 struct genl_info *info)
+static int __devlink_nl_cmd_param_set_doit(struct devlink *devlink,
+					   struct list_head *param_list,
+					   struct genl_info *info,
+					   enum devlink_command cmd)
 {
-	struct devlink *devlink = info->user_ptr[0];
 	enum devlink_param_type param_type;
 	struct devlink_param_gset_ctx ctx;
 	enum devlink_param_cmode cmode;
@@ -3108,7 +3109,7 @@ static int devlink_nl_cmd_param_set_doit(struct sk_buff *skb,
 	union devlink_param_value value;
 	int err = 0;
 
-	param_item = devlink_param_get_from_info(&devlink->param_list, info);
+	param_item = devlink_param_get_from_info(param_list, info);
 	if (!param_item)
 		return -EINVAL;
 	param = param_item->param;
@@ -3148,10 +3149,19 @@ static int devlink_nl_cmd_param_set_doit(struct sk_buff *skb,
 			return err;
 	}
 
-	devlink_param_notify(devlink, param_item, DEVLINK_CMD_PARAM_NEW);
+	devlink_param_notify(devlink, param_item, cmd);
 	return 0;
 }
 
+static int devlink_nl_cmd_param_set_doit(struct sk_buff *skb,
+					 struct genl_info *info)
+{
+	struct devlink *devlink = info->user_ptr[0];
+
+	return __devlink_nl_cmd_param_set_doit(devlink, &devlink->param_list,
+					       info, DEVLINK_CMD_PARAM_NEW);
+}
+
 static int devlink_param_register_one(struct devlink *devlink,
 				      struct list_head *param_list,
 				      const struct devlink_param *param)
@@ -3263,6 +3273,16 @@ static int devlink_nl_cmd_port_param_get_doit(struct sk_buff *skb,
 	return genlmsg_reply(msg, info);
 }
 
+static int devlink_nl_cmd_port_param_set_doit(struct sk_buff *skb,
+					      struct genl_info *info)
+{
+	struct devlink_port *devlink_port = info->user_ptr[0];
+
+	return __devlink_nl_cmd_param_set_doit(devlink_port->devlink,
+					       &devlink_port->param_list,
+					       info, 0);
+}
+
 static int devlink_nl_region_snapshot_id_put(struct sk_buff *msg,
 					     struct devlink *devlink,
 					     struct devlink_snapshot *snapshot)
@@ -3909,6 +3929,13 @@ static int devlink_nl_cmd_region_read_dumpit(struct sk_buff *skb,
 		/* can be retrieved by unprivileged users */
 	},
 	{
+		.cmd = DEVLINK_CMD_PORT_PARAM_SET,
+		.doit = devlink_nl_cmd_port_param_set_doit,
+		.policy = devlink_nl_policy,
+		.flags = GENL_ADMIN_PERM,
+		.internal_flags = DEVLINK_NL_FLAG_NEED_PORT,
+	},
+	{
 		.cmd = DEVLINK_CMD_REGION_GET,
 		.doit = devlink_nl_cmd_region_get_doit,
 		.dumpit = devlink_nl_cmd_region_get_dumpit,
-- 
1.8.3.1

[PATCH net-next v7 4/8] devlink: Add support for driverinit get value for devlink_port

[Vasundhara Volam]

Add support for "driverinit" configuration mode value for devlink_port
configuration parameters. Add devlink_port_param_driverinit_value_get()
function to help the driver get the value from devlink_port.

Also, move the common code to __devlink_param_driverinit_value_get()
to be used by both device and port params.

Cc: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
---
 include/net/devlink.h |  8 ++++++
 net/core/devlink.c    | 67 ++++++++++++++++++++++++++++++++++++++-------------
 2 files changed, 58 insertions(+), 17 deletions(-)

diff --git a/include/net/devlink.h b/include/net/devlink.h
index 98b8a66..09f3f43 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -838,6 +838,14 @@ static inline bool devlink_dpipe_table_counter_enabled(struct devlink *devlink,
 {
 }
 
+static inline int
+devlink_port_param_driverinit_value_get(struct devlink_port *devlink_port,
+					u32 param_id,
+					union devlink_param_value *init_val)
+{
+	return -EOPNOTSUPP;
+}
+
 static inline struct devlink_region *
 devlink_region_create(struct devlink *devlink,
 		      const char *region_name,
diff --git a/net/core/devlink.c b/net/core/devlink.c
index be083a9..53755ff 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -4677,26 +4677,13 @@ void devlink_params_unregister(struct devlink *devlink,
 }
 EXPORT_SYMBOL_GPL(devlink_params_unregister);
 
-/**
- *	devlink_param_driverinit_value_get - get configuration parameter
- *					     value for driver initializing
- *
- *	@devlink: devlink
- *	@param_id: parameter ID
- *	@init_val: value of parameter in driverinit configuration mode
- *
- *	This function should be used by the driver to get driverinit
- *	configuration for initialization after reload command.
- */
-int devlink_param_driverinit_value_get(struct devlink *devlink, u32 param_id,
-				       union devlink_param_value *init_val)
+static int
+__devlink_param_driverinit_value_get(struct list_head *param_list, u32 param_id,
+				     union devlink_param_value *init_val)
 {
 	struct devlink_param_item *param_item;
 
-	if (!devlink->ops || !devlink->ops->reload)
-		return -EOPNOTSUPP;
-
-	param_item = devlink_param_find_by_id(&devlink->param_list, param_id);
+	param_item = devlink_param_find_by_id(param_list, param_id);
 	if (!param_item)
 		return -EINVAL;
 
@@ -4712,6 +4699,27 @@ int devlink_param_driverinit_value_get(struct devlink *devlink, u32 param_id,
 
 	return 0;
 }
+
+/**
+ *	devlink_param_driverinit_value_get - get configuration parameter
+ *					     value for driver initializing
+ *
+ *	@devlink: devlink
+ *	@param_id: parameter ID
+ *	@init_val: value of parameter in driverinit configuration mode
+ *
+ *	This function should be used by the driver to get driverinit
+ *	configuration for initialization after reload command.
+ */
+int devlink_param_driverinit_value_get(struct devlink *devlink, u32 param_id,
+				       union devlink_param_value *init_val)
+{
+	if (!devlink->ops || !devlink->ops->reload)
+		return -EOPNOTSUPP;
+
+	return __devlink_param_driverinit_value_get(&devlink->param_list,
+						    param_id, init_val);
+}
 EXPORT_SYMBOL_GPL(devlink_param_driverinit_value_get);
 
 /**
@@ -4829,6 +4837,31 @@ void devlink_port_params_unregister(struct devlink_port *devlink_port,
 EXPORT_SYMBOL_GPL(devlink_port_params_unregister);
 
 /**
+ *	devlink_port_param_driverinit_value_get - get configuration parameter
+ *						value for driver initializing
+ *
+ *	@devlink_port: devlink_port
+ *	@param_id: parameter ID
+ *	@init_val: value of parameter in driverinit configuration mode
+ *
+ *	This function should be used by the driver to get driverinit
+ *	configuration for initialization after reload command.
+ */
+int devlink_port_param_driverinit_value_get(struct devlink_port *devlink_port,
+					    u32 param_id,
+					    union devlink_param_value *init_val)
+{
+	struct devlink *devlink = devlink_port->devlink;
+
+	if (!devlink->ops || !devlink->ops->reload)
+		return -EOPNOTSUPP;
+
+	return __devlink_param_driverinit_value_get(&devlink_port->param_list,
+						    param_id, init_val);
+}
+EXPORT_SYMBOL_GPL(devlink_port_param_driverinit_value_get);
+
+/**
  *	devlink_region_create - create a new address region
  *
  *	@devlink: devlink
-- 
1.8.3.1

[PATCH net-next v7 5/8] devlink: Add support for driverinit set value for devlink_port

[Vasundhara Volam]

Add support for "driverinit" configuration mode value for devlink_port
configuration parameters. Add devlink_port_param_driverinit_value_set()
function to help the driver set the value to devlink_port.

Also, move the common code to __devlink_param_driverinit_value_set()
to be used by both device and port params.

Cc: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
---
 include/net/devlink.h | 11 +++++++++
 net/core/devlink.c    | 64 +++++++++++++++++++++++++++++++++++++++------------
 2 files changed, 60 insertions(+), 15 deletions(-)

diff --git a/include/net/devlink.h b/include/net/devlink.h
index 09f3f43..f78cb8d 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -581,6 +581,9 @@ int devlink_port_params_register(struct devlink_port *devlink_port,
 void devlink_port_params_unregister(struct devlink_port *devlink_port,
 				    const struct devlink_param *params,
 				    size_t params_count);
+int devlink_port_param_driverinit_value_set(struct devlink_port *devlink_port,
+					    u32 param_id,
+					    union devlink_param_value init_val);
 struct devlink_region *devlink_region_create(struct devlink *devlink,
 					     const char *region_name,
 					     u32 region_max_snapshots,
@@ -846,6 +849,14 @@ static inline bool devlink_dpipe_table_counter_enabled(struct devlink *devlink,
 	return -EOPNOTSUPP;
 }
 
+static inline int
+devlink_port_param_driverinit_value_set(struct devlink_port *devlink_port,
+					u32 param_id,
+					union devlink_param_value init_val)
+{
+	return -EOPNOTSUPP;
+}
+
 static inline struct devlink_region *
 devlink_region_create(struct devlink *devlink,
 		      const char *region_name,
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 53755ff..8ea5a67 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -4722,24 +4722,15 @@ int devlink_param_driverinit_value_get(struct devlink *devlink, u32 param_id,
 }
 EXPORT_SYMBOL_GPL(devlink_param_driverinit_value_get);
 
-/**
- *	devlink_param_driverinit_value_set - set value of configuration
- *					     parameter for driverinit
- *					     configuration mode
- *
- *	@devlink: devlink
- *	@param_id: parameter ID
- *	@init_val: value of parameter to set for driverinit configuration mode
- *
- *	This function should be used by the driver to set driverinit
- *	configuration mode default value.
- */
-int devlink_param_driverinit_value_set(struct devlink *devlink, u32 param_id,
-				       union devlink_param_value init_val)
+static int
+__devlink_param_driverinit_value_set(struct devlink *devlink,
+				     struct list_head *param_list, u32 param_id,
+				     union devlink_param_value init_val,
+				     enum devlink_command cmd)
 {
 	struct devlink_param_item *param_item;
 
-	param_item = devlink_param_find_by_id(&devlink->param_list, param_id);
+	param_item = devlink_param_find_by_id(param_list, param_id);
 	if (!param_item)
 		return -EINVAL;
 
@@ -4756,6 +4747,27 @@ int devlink_param_driverinit_value_set(struct devlink *devlink, u32 param_id,
 	devlink_param_notify(devlink, param_item, DEVLINK_CMD_PARAM_NEW);
 	return 0;
 }
+
+/**
+ *	devlink_param_driverinit_value_set - set value of configuration
+ *					     parameter for driverinit
+ *					     configuration mode
+ *
+ *	@devlink: devlink
+ *	@param_id: parameter ID
+ *	@init_val: value of parameter to set for driverinit configuration mode
+ *
+ *	This function should be used by the driver to set driverinit
+ *	configuration mode default value.
+ */
+int devlink_param_driverinit_value_set(struct devlink *devlink, u32 param_id,
+				       union devlink_param_value init_val)
+{
+	return __devlink_param_driverinit_value_set(devlink,
+						    &devlink->param_list,
+						    param_id, init_val,
+						    DEVLINK_CMD_PARAM_NEW);
+}
 EXPORT_SYMBOL_GPL(devlink_param_driverinit_value_set);
 
 /**
@@ -4862,6 +4874,28 @@ int devlink_port_param_driverinit_value_get(struct devlink_port *devlink_port,
 EXPORT_SYMBOL_GPL(devlink_port_param_driverinit_value_get);
 
 /**
+ *     devlink_port_param_driverinit_value_set - set value of configuration
+ *                                               parameter for driverinit
+ *                                               configuration mode
+ *
+ *     @devlink_port: devlink_port
+ *     @param_id: parameter ID
+ *     @init_val: value of parameter to set for driverinit configuration mode
+ *
+ *     This function should be used by the driver to set driverinit
+ *     configuration mode default value.
+ */
+int devlink_port_param_driverinit_value_set(struct devlink_port *devlink_port,
+					    u32 param_id,
+					    union devlink_param_value init_val)
+{
+	return __devlink_param_driverinit_value_set(devlink_port->devlink,
+						    &devlink_port->param_list,
+						    param_id, init_val, 0);
+}
+EXPORT_SYMBOL_GPL(devlink_port_param_driverinit_value_set);
+
+/**
  *	devlink_region_create - create a new address region
  *
  *	@devlink: devlink
-- 
1.8.3.1

[PATCH net-next v7 6/8] devlink: Add devlink notifications support for port params

[Vasundhara Volam]

Add notification call for devlink port param set, register and unregister
functions.
Add devlink_port_param_value_changed() function to enable the driver notify
devlink on value change. Driver should use this function after value was
changed on any configuration mode part to driverinit.

Cc: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
---
 include/net/devlink.h        |   8 ++++
 include/uapi/linux/devlink.h |   2 +
 net/core/devlink.c           | 110 +++++++++++++++++++++++++++++++++----------
 3 files changed, 94 insertions(+), 26 deletions(-)

diff --git a/include/net/devlink.h b/include/net/devlink.h
index f78cb8d..db413d3 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -584,6 +584,8 @@ void devlink_port_params_unregister(struct devlink_port *devlink_port,
 int devlink_port_param_driverinit_value_set(struct devlink_port *devlink_port,
 					    u32 param_id,
 					    union devlink_param_value init_val);
+void devlink_port_param_value_changed(struct devlink_port *devlink_port,
+				      u32 param_id);
 struct devlink_region *devlink_region_create(struct devlink *devlink,
 					     const char *region_name,
 					     u32 region_max_snapshots,
@@ -857,6 +859,12 @@ static inline bool devlink_dpipe_table_counter_enabled(struct devlink *devlink,
 	return -EOPNOTSUPP;
 }
 
+static inline void
+devlink_port_param_value_changed(struct devlink_port *devlink_port,
+				 u32 param_id)
+{
+}
+
 static inline struct devlink_region *
 devlink_region_create(struct devlink *devlink,
 		      const char *region_name,
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 3658fb2..61b4447 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -91,6 +91,8 @@ enum devlink_command {
 
 	DEVLINK_CMD_PORT_PARAM_GET,	/* can dump */
 	DEVLINK_CMD_PORT_PARAM_SET,
+	DEVLINK_CMD_PORT_PARAM_NEW,
+	DEVLINK_CMD_PORT_PARAM_DEL,
 
 	/* add new commands above here */
 	__DEVLINK_CMD_MAX,
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 8ea5a67..f891b1c 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -2882,7 +2882,9 @@ static int devlink_nl_param_fill(struct sk_buff *msg, struct devlink *devlink,
 	if (devlink_nl_put_handle(msg, devlink))
 		goto genlmsg_cancel;
 
-	if (cmd == DEVLINK_CMD_PORT_PARAM_GET)
+	if (cmd == DEVLINK_CMD_PORT_PARAM_GET ||
+	    cmd == DEVLINK_CMD_PORT_PARAM_NEW ||
+	    cmd == DEVLINK_CMD_PORT_PARAM_DEL)
 		if (nla_put_u32(msg, DEVLINK_ATTR_PORT_INDEX, port_index))
 			goto genlmsg_cancel;
 
@@ -2928,18 +2930,22 @@ static int devlink_nl_param_fill(struct sk_buff *msg, struct devlink *devlink,
 }
 
 static void devlink_param_notify(struct devlink *devlink,
+				 unsigned int port_index,
 				 struct devlink_param_item *param_item,
 				 enum devlink_command cmd)
 {
 	struct sk_buff *msg;
 	int err;
 
-	WARN_ON(cmd != DEVLINK_CMD_PARAM_NEW && cmd != DEVLINK_CMD_PARAM_DEL);
+	WARN_ON(cmd != DEVLINK_CMD_PARAM_NEW && cmd != DEVLINK_CMD_PARAM_DEL &&
+		cmd != DEVLINK_CMD_PORT_PARAM_NEW &&
+		cmd != DEVLINK_CMD_PORT_PARAM_DEL);
 
 	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
 	if (!msg)
 		return;
-	err = devlink_nl_param_fill(msg, devlink, 0, param_item, cmd, 0, 0, 0);
+	err = devlink_nl_param_fill(msg, devlink, port_index, param_item, cmd,
+				    0, 0, 0);
 	if (err) {
 		nlmsg_free(msg);
 		return;
@@ -3097,6 +3103,7 @@ static int devlink_nl_cmd_param_get_doit(struct sk_buff *skb,
 }
 
 static int __devlink_nl_cmd_param_set_doit(struct devlink *devlink,
+					   unsigned int port_index,
 					   struct list_head *param_list,
 					   struct genl_info *info,
 					   enum devlink_command cmd)
@@ -3149,7 +3156,7 @@ static int __devlink_nl_cmd_param_set_doit(struct devlink *devlink,
 			return err;
 	}
 
-	devlink_param_notify(devlink, param_item, cmd);
+	devlink_param_notify(devlink, port_index, param_item, cmd);
 	return 0;
 }
 
@@ -3158,13 +3165,15 @@ static int devlink_nl_cmd_param_set_doit(struct sk_buff *skb,
 {
 	struct devlink *devlink = info->user_ptr[0];
 
-	return __devlink_nl_cmd_param_set_doit(devlink, &devlink->param_list,
+	return __devlink_nl_cmd_param_set_doit(devlink, 0, &devlink->param_list,
 					       info, DEVLINK_CMD_PARAM_NEW);
 }
 
 static int devlink_param_register_one(struct devlink *devlink,
+				      unsigned int port_index,
 				      struct list_head *param_list,
-				      const struct devlink_param *param)
+				      const struct devlink_param *param,
+				      enum devlink_command cmd)
 {
 	struct devlink_param_item *param_item;
 
@@ -3182,19 +3191,21 @@ static int devlink_param_register_one(struct devlink *devlink,
 	param_item->param = param;
 
 	list_add_tail(&param_item->list, param_list);
-	devlink_param_notify(devlink, param_item, DEVLINK_CMD_PARAM_NEW);
+	devlink_param_notify(devlink, port_index, param_item, cmd);
 	return 0;
 }
 
 static void devlink_param_unregister_one(struct devlink *devlink,
+					 unsigned int port_index,
 					 struct list_head *param_list,
-					 const struct devlink_param *param)
+					 const struct devlink_param *param,
+					 enum devlink_command cmd)
 {
 	struct devlink_param_item *param_item;
 
 	param_item = devlink_param_find_by_name(param_list, param->name);
 	WARN_ON(!param_item);
-	devlink_param_notify(devlink, param_item, DEVLINK_CMD_PARAM_DEL);
+	devlink_param_notify(devlink, port_index, param_item, cmd);
 	list_del(&param_item->list);
 	kfree(param_item);
 }
@@ -3279,8 +3290,9 @@ static int devlink_nl_cmd_port_param_set_doit(struct sk_buff *skb,
 	struct devlink_port *devlink_port = info->user_ptr[0];
 
 	return __devlink_nl_cmd_param_set_doit(devlink_port->devlink,
-					       &devlink_port->param_list,
-					       info, 0);
+					       devlink_port->index,
+					       &devlink_port->param_list, info,
+					       DEVLINK_CMD_PORT_PARAM_NEW);
 }
 
 static int devlink_nl_region_snapshot_id_put(struct sk_buff *msg,
@@ -4598,9 +4610,12 @@ static int devlink_param_verify(const struct devlink_param *param)
 }
 
 static int __devlink_params_register(struct devlink *devlink,
+				     unsigned int port_index,
 				     struct list_head *param_list,
 				     const struct devlink_param *params,
-				     size_t params_count)
+				     size_t params_count,
+				     enum devlink_command reg_cmd,
+				     enum devlink_command unreg_cmd)
 {
 	const struct devlink_param *param = params;
 	int i;
@@ -4612,7 +4627,8 @@ static int __devlink_params_register(struct devlink *devlink,
 		if (err)
 			goto rollback;
 
-		err = devlink_param_register_one(devlink, param_list, param);
+		err = devlink_param_register_one(devlink, port_index,
+						 param_list, param, reg_cmd);
 		if (err)
 			goto rollback;
 	}
@@ -4624,7 +4640,8 @@ static int __devlink_params_register(struct devlink *devlink,
 	if (!i)
 		goto unlock;
 	for (param--; i > 0; i--, param--)
-		devlink_param_unregister_one(devlink, param_list, param);
+		devlink_param_unregister_one(devlink, port_index, param_list,
+					     param, unreg_cmd);
 unlock:
 	mutex_unlock(&devlink->lock);
 	return err;
@@ -4643,22 +4660,27 @@ int devlink_params_register(struct devlink *devlink,
 			    const struct devlink_param *params,
 			    size_t params_count)
 {
-	return __devlink_params_register(devlink, &devlink->param_list, params,
-					 params_count);
+	return __devlink_params_register(devlink, 0, &devlink->param_list,
+					 params, params_count,
+					 DEVLINK_CMD_PARAM_NEW,
+					 DEVLINK_CMD_PARAM_DEL);
 }
 EXPORT_SYMBOL_GPL(devlink_params_register);
 
 static void __devlink_params_unregister(struct devlink *devlink,
+					unsigned int port_index,
 					struct list_head *param_list,
 					const struct devlink_param *params,
-					size_t params_count)
+					size_t params_count,
+					enum devlink_command cmd)
 {
 	const struct devlink_param *param = params;
 	int i;
 
 	mutex_lock(&devlink->lock);
 	for (i = 0; i < params_count; i++, param++)
-		devlink_param_unregister_one(devlink, param_list, param);
+		devlink_param_unregister_one(devlink, 0, param_list, param,
+					     cmd);
 	mutex_unlock(&devlink->lock);
 }
 
@@ -4672,8 +4694,9 @@ void devlink_params_unregister(struct devlink *devlink,
 			       const struct devlink_param *params,
 			       size_t params_count)
 {
-	return __devlink_params_unregister(devlink, &devlink->param_list,
-					   params, params_count);
+	return __devlink_params_unregister(devlink, 0, &devlink->param_list,
+					   params, params_count,
+					   DEVLINK_CMD_PARAM_DEL);
 }
 EXPORT_SYMBOL_GPL(devlink_params_unregister);
 
@@ -4724,6 +4747,7 @@ int devlink_param_driverinit_value_get(struct devlink *devlink, u32 param_id,
 
 static int
 __devlink_param_driverinit_value_set(struct devlink *devlink,
+				     unsigned int port_index,
 				     struct list_head *param_list, u32 param_id,
 				     union devlink_param_value init_val,
 				     enum devlink_command cmd)
@@ -4744,7 +4768,7 @@ int devlink_param_driverinit_value_get(struct devlink *devlink, u32 param_id,
 		param_item->driverinit_value = init_val;
 	param_item->driverinit_value_valid = true;
 
-	devlink_param_notify(devlink, param_item, DEVLINK_CMD_PARAM_NEW);
+	devlink_param_notify(devlink, port_index, param_item, cmd);
 	return 0;
 }
 
@@ -4763,7 +4787,7 @@ int devlink_param_driverinit_value_get(struct devlink *devlink, u32 param_id,
 int devlink_param_driverinit_value_set(struct devlink *devlink, u32 param_id,
 				       union devlink_param_value init_val)
 {
-	return __devlink_param_driverinit_value_set(devlink,
+	return __devlink_param_driverinit_value_set(devlink, 0,
 						    &devlink->param_list,
 						    param_id, init_val,
 						    DEVLINK_CMD_PARAM_NEW);
@@ -4790,7 +4814,7 @@ void devlink_param_value_changed(struct devlink *devlink, u32 param_id)
 	param_item = devlink_param_find_by_id(&devlink->param_list, param_id);
 	WARN_ON(!param_item);
 
-	devlink_param_notify(devlink, param_item, DEVLINK_CMD_PARAM_NEW);
+	devlink_param_notify(devlink, 0, param_item, DEVLINK_CMD_PARAM_NEW);
 }
 EXPORT_SYMBOL_GPL(devlink_param_value_changed);
 
@@ -4825,8 +4849,11 @@ int devlink_port_params_register(struct devlink_port *devlink_port,
 				 size_t params_count)
 {
 	return __devlink_params_register(devlink_port->devlink,
+					 devlink_port->index,
 					 &devlink_port->param_list, params,
-					 params_count);
+					 params_count,
+					 DEVLINK_CMD_PORT_PARAM_NEW,
+					 DEVLINK_CMD_PORT_PARAM_DEL);
 }
 EXPORT_SYMBOL_GPL(devlink_port_params_register);
 
@@ -4843,8 +4870,10 @@ void devlink_port_params_unregister(struct devlink_port *devlink_port,
 				    size_t params_count)
 {
 	return __devlink_params_unregister(devlink_port->devlink,
+					   devlink_port->index,
 					   &devlink_port->param_list,
-					   params, params_count);
+					   params, params_count,
+					   DEVLINK_CMD_PORT_PARAM_DEL);
 }
 EXPORT_SYMBOL_GPL(devlink_port_params_unregister);
 
@@ -4890,12 +4919,41 @@ int devlink_port_param_driverinit_value_set(struct devlink_port *devlink_port,
 					    union devlink_param_value init_val)
 {
 	return __devlink_param_driverinit_value_set(devlink_port->devlink,
+						    devlink_port->index,
 						    &devlink_port->param_list,
-						    param_id, init_val, 0);
+						    param_id, init_val,
+						    DEVLINK_CMD_PORT_PARAM_NEW);
 }
 EXPORT_SYMBOL_GPL(devlink_port_param_driverinit_value_set);
 
 /**
+ *	devlink_port_param_value_changed - notify devlink on a parameter's value
+ *					 change. Should be called by the driver
+ *					 right after the change.
+ *
+ *	@devlink_port: devlink_port
+ *	@param_id: parameter ID
+ *
+ *	This function should be used by the driver to notify devlink on value
+ *	change, excluding driverinit configuration mode.
+ *	For driverinit configuration mode driver should use the function
+ *	devlink_port_param_driverinit_value_set() instead.
+ */
+void devlink_port_param_value_changed(struct devlink_port *devlink_port,
+				      u32 param_id)
+{
+	struct devlink_param_item *param_item;
+
+	param_item = devlink_param_find_by_id(&devlink_port->param_list,
+					      param_id);
+	WARN_ON(!param_item);
+
+	devlink_param_notify(devlink_port->devlink, devlink_port->index,
+			     param_item, DEVLINK_CMD_PORT_PARAM_NEW);
+}
+EXPORT_SYMBOL_GPL(devlink_port_param_value_changed);
+
+/**
  *	devlink_region_create - create a new address region
  *
  *	@devlink: devlink
-- 
1.8.3.1

[PATCH net-next v7 7/8] devlink: Add a generic wake_on_lan port parameter

[Vasundhara Volam]

wake_on_lan - Enables Wake on Lan for this port. If enabled,
the controller asserts a wake pin based on the WOL type.

v2->v3:
- Define only WOL types used now and define them as bitfield, so that
  mutliple WOL types can be enabled upon power on.
- Modify "wake-on-lan" name to "wake_on_lan" to be symmetric with
  previous definitions.
- Rename DEVLINK_PARAM_WOL_XXX to DEVLINK_PARAM_WAKE_XXX to be
  symmetrical with ethtool WOL definitions.

Cc: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
---
 include/net/devlink.h | 8 ++++++++
 net/core/devlink.c    | 5 +++++
 2 files changed, 13 insertions(+)

diff --git a/include/net/devlink.h b/include/net/devlink.h
index db413d3..f2847f8 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -367,12 +367,17 @@ enum devlink_param_generic_id {
 	DEVLINK_PARAM_GENERIC_ID_MSIX_VEC_PER_PF_MAX,
 	DEVLINK_PARAM_GENERIC_ID_MSIX_VEC_PER_PF_MIN,
 	DEVLINK_PARAM_GENERIC_ID_FW_LOAD_POLICY,
+	DEVLINK_PARAM_GENERIC_ID_WOL,
 
 	/* add new param generic ids above here*/
 	__DEVLINK_PARAM_GENERIC_ID_MAX,
 	DEVLINK_PARAM_GENERIC_ID_MAX = __DEVLINK_PARAM_GENERIC_ID_MAX - 1,
 };
 
+enum devlink_param_wol_types {
+	DEVLINK_PARAM_WAKE_MAGIC = (1 << 0),
+};
+
 #define DEVLINK_PARAM_GENERIC_INT_ERR_RESET_NAME "internal_error_reset"
 #define DEVLINK_PARAM_GENERIC_INT_ERR_RESET_TYPE DEVLINK_PARAM_TYPE_BOOL
 
@@ -397,6 +402,9 @@ enum devlink_param_generic_id {
 #define DEVLINK_PARAM_GENERIC_FW_LOAD_POLICY_NAME "fw_load_policy"
 #define DEVLINK_PARAM_GENERIC_FW_LOAD_POLICY_TYPE DEVLINK_PARAM_TYPE_U8
 
+#define DEVLINK_PARAM_GENERIC_WOL_NAME "wake_on_lan"
+#define DEVLINK_PARAM_GENERIC_WOL_TYPE DEVLINK_PARAM_TYPE_U8
+
 #define DEVLINK_PARAM_GENERIC(_id, _cmodes, _get, _set, _validate)	\
 {									\
 	.id = DEVLINK_PARAM_GENERIC_ID_##_id,				\
diff --git a/net/core/devlink.c b/net/core/devlink.c
index f891b1c..9a48a91 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -2697,6 +2697,11 @@ static int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info)
 		.name = DEVLINK_PARAM_GENERIC_FW_LOAD_POLICY_NAME,
 		.type = DEVLINK_PARAM_GENERIC_FW_LOAD_POLICY_TYPE,
 	},
+	{
+		.id = DEVLINK_PARAM_GENERIC_ID_WOL,
+		.name = DEVLINK_PARAM_GENERIC_WOL_NAME,
+		.type = DEVLINK_PARAM_GENERIC_WOL_TYPE,
+	},
 };
 
 static int devlink_param_generic_verify(const struct devlink_param *param)
-- 
1.8.3.1

[PATCH net-next v7 8/8] bnxt_en: Add bnxt_en initial port params table and register it

[Vasundhara Volam]

Register devlink_port with devlink and create initial port params
table for bnxt_en. The table consists of a generic parameter:

wake_on_lan: Enables Wake on Lan for this port when magic packet
is received with this port's MAC address using ACPI pattern.
If enabled, the controller asserts a wake pin upon reception of
WoL packet.  ACPI (Advanced Configuration and Power Interface) is
an industry specification for the efficient handling of power
consumption in desktop and mobile computers.

v2->v3:
- Modify bnxt_dl_wol_validate(), to throw error message when user gives
  value other than DEVLINK_PARAM_WAKE_MAGIC ot to disable WOL.
- Use netdev_err() instead of netdev_warn(), when devlink_port_register()
  and devlink_port_params_register() returns error. Also, don't log rc
  in this message.

Cc: Michael Chan <michael.chan@broadcom.com>
Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.h         |  1 +
 drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 43 ++++++++++++++++++++++-
 drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h |  1 +
 3 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index a451796..5c886a7 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -1609,6 +1609,7 @@ struct bnxt {
 
 	/* devlink interface and vf-rep structs */
 	struct devlink		*dl;
+	struct devlink_port	dl_port;
 	enum devlink_eswitch_mode eswitch_mode;
 	struct bnxt_vf_rep	**vf_reps; /* array of vf-rep ptrs */
 	u16			*cfa_code_map; /* cfa_code -> vf_idx map */
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index 7f56032..a6abfa4 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -37,6 +37,8 @@ enum bnxt_dl_param_id {
 	 NVM_OFF_MSIX_VEC_PER_PF_MIN, BNXT_NVM_SHARED_CFG, 7},
 	{BNXT_DEVLINK_PARAM_ID_GRE_VER_CHECK, NVM_OFF_DIS_GRE_VER_CHECK,
 	 BNXT_NVM_SHARED_CFG, 1},
+
+	{DEVLINK_PARAM_GENERIC_ID_WOL, NVM_OFF_WOL, BNXT_NVM_PORT_CFG, 1},
 };
 
 static int bnxt_hwrm_nvm_req(struct bnxt *bp, u32 param_id, void *msg,
@@ -70,7 +72,8 @@ static int bnxt_hwrm_nvm_req(struct bnxt *bp, u32 param_id, void *msg,
 	bytesize = roundup(nvm_param.num_bits, BITS_PER_BYTE) / BITS_PER_BYTE;
 	switch (bytesize) {
 	case 1:
-		if (nvm_param.num_bits == 1)
+		if (nvm_param.num_bits == 1 &&
+		    nvm_param.id != DEVLINK_PARAM_GENERIC_ID_WOL)
 			buf = &val->vbool;
 		else
 			buf = &val->vu8;
@@ -164,6 +167,17 @@ static int bnxt_dl_msix_validate(struct devlink *dl, u32 id,
 	return 0;
 }
 
+static int bnxt_dl_wol_validate(struct devlink *dl, u32 id,
+				union devlink_param_value val,
+				struct netlink_ext_ack *extack)
+{
+	if (val.vu8 && val.vu8 != DEVLINK_PARAM_WAKE_MAGIC) {
+		NL_SET_ERR_MSG_MOD(extack, "WOL type is not supported");
+		return -EINVAL;
+	}
+	return 0;
+}
+
 static const struct devlink_param bnxt_dl_params[] = {
 	DEVLINK_PARAM_GENERIC(ENABLE_SRIOV,
 			      BIT(DEVLINK_PARAM_CMODE_PERMANENT),
@@ -188,6 +202,12 @@ static int bnxt_dl_msix_validate(struct devlink *dl, u32 id,
 			     NULL),
 };
 
+static const struct devlink_param bnxt_dl_port_params[] = {
+	DEVLINK_PARAM_GENERIC(WOL, BIT(DEVLINK_PARAM_CMODE_PERMANENT),
+			      bnxt_dl_nvm_param_get, bnxt_dl_nvm_param_set,
+			      bnxt_dl_wol_validate),
+};
+
 int bnxt_dl_register(struct bnxt *bp)
 {
 	struct devlink *dl;
@@ -225,8 +245,26 @@ int bnxt_dl_register(struct bnxt *bp)
 		goto err_dl_unreg;
 	}
 
+	rc = devlink_port_register(dl, &bp->dl_port, bp->pf.port_id);
+	if (rc) {
+		netdev_err(bp->dev, "devlink_port_register failed");
+		goto err_dl_param_unreg;
+	}
+	devlink_port_type_eth_set(&bp->dl_port, bp->dev);
+
+	rc = devlink_port_params_register(&bp->dl_port, bnxt_dl_port_params,
+					  ARRAY_SIZE(bnxt_dl_port_params));
+	if (rc) {
+		netdev_err(bp->dev, "devlink_port_params_register failed");
+		goto err_dl_port_unreg;
+	}
 	return 0;
 
+err_dl_port_unreg:
+	devlink_port_unregister(&bp->dl_port);
+err_dl_param_unreg:
+	devlink_params_unregister(dl, bnxt_dl_params,
+				  ARRAY_SIZE(bnxt_dl_params));
 err_dl_unreg:
 	devlink_unregister(dl);
 err_dl_free:
@@ -242,6 +280,9 @@ void bnxt_dl_unregister(struct bnxt *bp)
 	if (!dl)
 		return;
 
+	devlink_port_params_unregister(&bp->dl_port, bnxt_dl_port_params,
+				       ARRAY_SIZE(bnxt_dl_port_params));
+	devlink_port_unregister(&bp->dl_port);
 	devlink_params_unregister(dl, bnxt_dl_params,
 				  ARRAY_SIZE(bnxt_dl_params));
 	devlink_unregister(dl);
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h
index 5b6b2c7..da065ca 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h
@@ -35,6 +35,7 @@ static inline void bnxt_link_bp_to_dl(struct bnxt *bp, struct devlink *dl)
 
 #define NVM_OFF_MSIX_VEC_PER_PF_MAX	108
 #define NVM_OFF_MSIX_VEC_PER_PF_MIN	114
+#define NVM_OFF_WOL			152
 #define NVM_OFF_IGNORE_ARI		164
 #define NVM_OFF_DIS_GRE_VER_CHECK	171
 #define NVM_OFF_ENABLE_SRIOV		401
-- 
1.8.3.1

Re: [PATCH net-next v7 0/8] devlink: Add configuration parameters support for devlink_port

[Michal Kubecek]

On Fri, Jan 18, 2019 at 12:39:37PM +0530, Vasundhara Volam wrote:
> There is difference of opinion on adding WOL parameter to devlink, between
> Jakub Kicinski and Michael Chan.
> 
> Quote from Jakud Kicinski:
> ********
> As explained previously I think it's a very bad idea to add existing
> configuration options to devlink, just because devlink has the ability
> to persist the setting in NVM.  Especially that for WoL you have to get
> the link up so you potentially have all link config stuff as well.  And
> that n-tuple filters are one of the WoL options, meaning we'd need the
> ability to persist n-tuple filters via devlink.
> 
> The effort would be far better spent helping with migrating ethtool to
> netlink, and allowing persisting there.
> 
> I have not heard any reason why devlink is a better fit.  I can imagine
> you're just doing it here because it's less effort for you since
> ethtool is not yet migrated.
> ********
> 
> Quote from Michael Chan:
> ********
> The devlink's WoL parameter is a persistent WoL parameter stored in the
> NIC's NVRAM. It is different from ethtool's WoL parameter in a number of
> ways. ethtool WoL is not persistent over AC power cycle and is considered
> OS-present WoL. As such, ethtool WoL can use a more sophisticated pattern
> including n-tuple with IP address in addition to the more basic types
> (e.g. magic packet). Whereas OS-absent power up WoL should only include
> magic packet and other simple types.

If I understand correctly, it's that way now. I'm not sure there is a
technical reason preventing more complex WoL types in the OS-absent case
in the future. Also, even with traditional ethtool WoL setting, most
NICs only support some of the types (I'm not sure if there is a NIC
which would support all of them.)

> The devlink WoL setting does not have to match the ethtool WoL
> setting.

IMHO this is not really a problem. We can either use an additional flag
telling kernel/driver if we are setting runtime or persistent WoL mask
or we can pass (up to) two bitmaps.

> The card will autoneg up to the speed supported by Vaux so no special
> devlink link setting is needed.
> ********

Like Jakub, I'm not convinced there is a strong technical reason to have
each of the WoL settings handled through a different interface. I don't
say, though, that ethtool is necessarily the right one. If there is
a consensus that it better fits into devlink, I can imagine that both
could be accessible through devlink (for start, in drivers which choose
so, e.g. because they want to implement the persistent setting).

Michal Kubecek

Re: [PATCH net-next v7 0/8] devlink: Add configuration parameters support for devlink_port

[Jakub Kicinski]

On Fri, 18 Jan 2019 15:33:19 +0100, Michal Kubecek wrote:
> On Fri, Jan 18, 2019 at 12:39:37PM +0530, Vasundhara Volam wrote:
> > There is difference of opinion on adding WOL parameter to devlink, between
> > Jakub Kicinski and Michael Chan.
> > 
> > Quote from Jakud Kicinski:
> > ********
> > As explained previously I think it's a very bad idea to add existing
> > configuration options to devlink, just because devlink has the ability
> > to persist the setting in NVM.  Especially that for WoL you have to get
> > the link up so you potentially have all link config stuff as well.  And
> > that n-tuple filters are one of the WoL options, meaning we'd need the
> > ability to persist n-tuple filters via devlink.
> > 
> > The effort would be far better spent helping with migrating ethtool to
> > netlink, and allowing persisting there.
> > 
> > I have not heard any reason why devlink is a better fit.  I can imagine
> > you're just doing it here because it's less effort for you since
> > ethtool is not yet migrated.
> > ********
> > 
> > Quote from Michael Chan:
> > ********
> > The devlink's WoL parameter is a persistent WoL parameter stored in the
> > NIC's NVRAM. It is different from ethtool's WoL parameter in a number of
> > ways. ethtool WoL is not persistent over AC power cycle and is considered
> > OS-present WoL. As such, ethtool WoL can use a more sophisticated pattern
> > including n-tuple with IP address in addition to the more basic types
> > (e.g. magic packet). Whereas OS-absent power up WoL should only include
> > magic packet and other simple types.  
> 
> If I understand correctly, it's that way now. I'm not sure there is a
> technical reason preventing more complex WoL types in the OS-absent case
> in the future. Also, even with traditional ethtool WoL setting, most
> NICs only support some of the types (I'm not sure if there is a NIC
> which would support all of them.)
> 
> > The devlink WoL setting does not have to match the ethtool WoL
> > setting.  
> 
> IMHO this is not really a problem. We can either use an additional flag
> telling kernel/driver if we are setting runtime or persistent WoL mask
> or we can pass (up to) two bitmaps.

I think reusing new netlink ethtool with a special flag would be a nice,
complete solution.  We could also address link settings this way (which
are a pre-requisite for WoL).

I have no strong preference on the mechanism, but for ease of setting
as well as dumping would it be workable to use a nesting, like this:

Run time settings:
  [ETHTOOL_SETTINGS_BLA]
    [ETHTOOL_BLA_VAL_1]
    [ETHTOOL_BLA_VAL_2]
    ...

Persistent:
  [ETHTOOL_PERSISTENT]
    [ETHTOOL_SETTINGS_BLA]
      [ETHTOOL_BLA_VAL_1]
      [ETHTOOL_BLA_VAL_2]

IOW encapsulate settings into a "persistent" attribute?

How does that look to you, Michal?

> > The card will autoneg up to the speed supported by Vaux so no special
> > devlink link setting is needed.
> > ********  
> 
> Like Jakub, I'm not convinced there is a strong technical reason to have
> each of the WoL settings handled through a different interface. I don't
> say, though, that ethtool is necessarily the right one. If there is
> a consensus that it better fits into devlink, I can imagine that both
> could be accessible through devlink (for start, in drivers which choose
> so, e.g. because they want to implement the persistent setting).
> 
> Michal Kubecek

Re: [PATCH net-next v7 1/8] devlink: Add devlink_param for port register and unregister

[Jiri Pirko]

Fri, Jan 18, 2019 at 08:09:38AM CET, vasundhara-v.volam@broadcom.com wrote:
>Add functions to register and unregister for the driver supported
>configuration parameters table per port.
>
>v2->v3:
>- Add a helper __devlink_params_register() with common code used by
>  both devlink_params_register() and devlink_port_params_register().
>

[...]	
	
	
>+
>+static int __devlink_params_register(struct devlink *devlink,
>+				     struct list_head *param_list,
>+				     const struct devlink_param *params,
>+				     size_t params_count)
> {
> 	const struct devlink_param *param = params;
> 	int i;
>@@ -4490,20 +4493,11 @@ int devlink_params_register(struct devlink *devlink,
> 
> 	mutex_lock(&devlink->lock);
> 	for (i = 0; i < params_count; i++, param++) {
>-		if (!param || !param->name || !param->supported_cmodes) {
>-			err = -EINVAL;
>+		err = devlink_param_verify(param);
>+		if (err)
> 			goto rollback;
>-		}
>-		if (param->generic) {
>-			err = devlink_param_generic_verify(param);
>-			if (err)
>-				goto rollback;
>-		} else {
>-			err = devlink_param_driver_verify(param);
>-			if (err)
>-				goto rollback;
>-		}
>-		err = devlink_param_register_one(devlink, param);
>+
>+		err = devlink_param_register_one(devlink, param_list, param);
> 		if (err)
> 			goto rollback;
> 	}
>@@ -4515,31 +4509,57 @@ int devlink_params_register(struct devlink *devlink,
> 	if (!i)
> 		goto unlock;
> 	for (param--; i > 0; i--, param--)
>-		devlink_param_unregister_one(devlink, param);
>+		devlink_param_unregister_one(devlink, param_list, param);
> unlock:
> 	mutex_unlock(&devlink->lock);
> 	return err;
> }
>-EXPORT_SYMBOL_GPL(devlink_params_register);
> 
> /**
>- *	devlink_params_unregister - unregister configuration parameters
>+ *	devlink_params_register - register configuration parameters
>+ *
>  *	@devlink: devlink
>- *	@params: configuration parameters to unregister
>+ *	@params: configuration parameters array
>  *	@params_count: number of parameters provided
>+ *
>+ *	Register the configuration parameters supported by the driver.
>  */
>-void devlink_params_unregister(struct devlink *devlink,
>-			       const struct devlink_param *params,
>-			       size_t params_count)
>+int devlink_params_register(struct devlink *devlink,

The order of the functions should be:
__devlink_params_register
__devlink_params_unregister
devlink_params_register
devlink_params_unregister
devlink_port_params_register
devlink_port_params_unregister

Your order is a bit confusing.

Other than that:
Acked-by: Jiri Pirko <jiri@mellanox.com>


[...]

Re: [PATCH net-next v7 2/8] devlink: Add port param get command

[Jiri Pirko]

Fri, Jan 18, 2019 at 08:09:39AM CET, vasundhara-v.volam@broadcom.com wrote:
>Add port param get command which gets data per parameter.
>It also has option to dump the parameters data per port.
>
>Cc: Jiri Pirko <jiri@mellanox.com>
>Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>

Acked-by: Jiri Pirko <jiri@mellanox.com>

Re: [PATCH net-next v7 1/8] devlink: Add devlink_param for port register and unregister

[Vasundhara Volam]

On Wed, Jan 23, 2019 at 2:04 PM Jiri Pirko <jiri@resnulli.us> wrote:
>
> Fri, Jan 18, 2019 at 08:09:38AM CET, vasundhara-v.volam@broadcom.com wrote:
> >Add functions to register and unregister for the driver supported
> >configuration parameters table per port.
> >
> >v2->v3:
> >- Add a helper __devlink_params_register() with common code used by
> >  both devlink_params_register() and devlink_port_params_register().
> >
>
> [...]
>
>
> >+
> >+static int __devlink_params_register(struct devlink *devlink,
> >+                                   struct list_head *param_list,
> >+                                   const struct devlink_param *params,
> >+                                   size_t params_count)
> > {
> >       const struct devlink_param *param = params;
> >       int i;
> >@@ -4490,20 +4493,11 @@ int devlink_params_register(struct devlink *devlink,
> >
> >       mutex_lock(&devlink->lock);
> >       for (i = 0; i < params_count; i++, param++) {
> >-              if (!param || !param->name || !param->supported_cmodes) {
> >-                      err = -EINVAL;
> >+              err = devlink_param_verify(param);
> >+              if (err)
> >                       goto rollback;
> >-              }
> >-              if (param->generic) {
> >-                      err = devlink_param_generic_verify(param);
> >-                      if (err)
> >-                              goto rollback;
> >-              } else {
> >-                      err = devlink_param_driver_verify(param);
> >-                      if (err)
> >-                              goto rollback;
> >-              }
> >-              err = devlink_param_register_one(devlink, param);
> >+
> >+              err = devlink_param_register_one(devlink, param_list, param);
> >               if (err)
> >                       goto rollback;
> >       }
> >@@ -4515,31 +4509,57 @@ int devlink_params_register(struct devlink *devlink,
> >       if (!i)
> >               goto unlock;
> >       for (param--; i > 0; i--, param--)
> >-              devlink_param_unregister_one(devlink, param);
> >+              devlink_param_unregister_one(devlink, param_list, param);
> > unlock:
> >       mutex_unlock(&devlink->lock);
> >       return err;
> > }
> >-EXPORT_SYMBOL_GPL(devlink_params_register);
> >
> > /**
> >- *    devlink_params_unregister - unregister configuration parameters
> >+ *    devlink_params_register - register configuration parameters
> >+ *
> >  *    @devlink: devlink
> >- *    @params: configuration parameters to unregister
> >+ *    @params: configuration parameters array
> >  *    @params_count: number of parameters provided
> >+ *
> >+ *    Register the configuration parameters supported by the driver.
> >  */
> >-void devlink_params_unregister(struct devlink *devlink,
> >-                             const struct devlink_param *params,
> >-                             size_t params_count)
> >+int devlink_params_register(struct devlink *devlink,
>
> The order of the functions should be:
> __devlink_params_register
> __devlink_params_unregister
> devlink_params_register
> devlink_params_unregister
> devlink_port_params_register
> devlink_port_params_unregister
>
> Your order is a bit confusing.
>
> Other than that:
> Acked-by: Jiri Pirko <jiri@mellanox.com>
Thank you Jiri. I will update the order in the next version.
>
>
> [...]

Re: [PATCH net-next v7 3/8] devlink: Add port param set command

[Jiri Pirko]

Fri, Jan 18, 2019 at 08:09:40AM CET, vasundhara-v.volam@broadcom.com wrote:
>Add port param set command to set the value for a parameter.
>Value can be set to any of the supported configuration modes.
>
>Cc: Jiri Pirko <jiri@mellanox.com>
>Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>

Acked-by: Jiri Pirko <jiri@mellanox.com>

Re: [PATCH net-next v7 4/8] devlink: Add support for driverinit get value for devlink_port

[Jiri Pirko]

Fri, Jan 18, 2019 at 08:09:41AM CET, vasundhara-v.volam@broadcom.com wrote:
>Add support for "driverinit" configuration mode value for devlink_port
>configuration parameters. Add devlink_port_param_driverinit_value_get()
>function to help the driver get the value from devlink_port.
>
>Also, move the common code to __devlink_param_driverinit_value_get()
>to be used by both device and port params.
>
>Cc: Jiri Pirko <jiri@mellanox.com>
>Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
>---
> include/net/devlink.h |  8 ++++++
> net/core/devlink.c    | 67 ++++++++++++++++++++++++++++++++++++++-------------
> 2 files changed, 58 insertions(+), 17 deletions(-)
>
>diff --git a/include/net/devlink.h b/include/net/devlink.h
>index 98b8a66..09f3f43 100644
>--- a/include/net/devlink.h
>+++ b/include/net/devlink.h
>@@ -838,6 +838,14 @@ static inline bool devlink_dpipe_table_counter_enabled(struct devlink *devlink,
> {
> }
> 
>+static inline int
>+devlink_port_param_driverinit_value_get(struct devlink_port *devlink_port,
>+					u32 param_id,
>+					union devlink_param_value *init_val)
>+{
>+	return -EOPNOTSUPP;
>+}


You are missing function declaration in case IS_ENABLED(CONFIG_NET_DEVLINK

Otherwise this looks fine.


>+
> static inline struct devlink_region *
> devlink_region_create(struct devlink *devlink,
> 		      const char *region_name,

[...]

Re: [PATCH net-next v7 4/8] devlink: Add support for driverinit get value for devlink_port

[Vasundhara Volam]

On Wed, Jan 23, 2019 at 4:47 PM Jiri Pirko <jiri@resnulli.us> wrote:
>
> Fri, Jan 18, 2019 at 08:09:41AM CET, vasundhara-v.volam@broadcom.com wrote:
> >Add support for "driverinit" configuration mode value for devlink_port
> >configuration parameters. Add devlink_port_param_driverinit_value_get()
> >function to help the driver get the value from devlink_port.
> >
> >Also, move the common code to __devlink_param_driverinit_value_get()
> >to be used by both device and port params.
> >
> >Cc: Jiri Pirko <jiri@mellanox.com>
> >Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
> >---
> > include/net/devlink.h |  8 ++++++
> > net/core/devlink.c    | 67 ++++++++++++++++++++++++++++++++++++++-------------
> > 2 files changed, 58 insertions(+), 17 deletions(-)
> >
> >diff --git a/include/net/devlink.h b/include/net/devlink.h
> >index 98b8a66..09f3f43 100644
> >--- a/include/net/devlink.h
> >+++ b/include/net/devlink.h
> >@@ -838,6 +838,14 @@ static inline bool devlink_dpipe_table_counter_enabled(struct devlink *devlink,
> > {
> > }
> >
> >+static inline int
> >+devlink_port_param_driverinit_value_get(struct devlink_port *devlink_port,
> >+                                      u32 param_id,
> >+                                      union devlink_param_value *init_val)
> >+{
> >+      return -EOPNOTSUPP;
> >+}
>
>
> You are missing function declaration in case IS_ENABLED(CONFIG_NET_DEVLINK
>
> Otherwise this looks fine.
Thank you for pointing. It is a mistake. I will fix it in next version.
>
>
> >+
> > static inline struct devlink_region *
> > devlink_region_create(struct devlink *devlink,
> >                     const char *region_name,
>
> [...]
>

Re: [PATCH net-next v7 0/8] devlink: Add configuration parameters support for devlink_port

[Vasundhara Volam]

On Fri, Jan 18, 2019 at 8:03 PM Michal Kubecek <mkubecek@suse.cz> wrote:
>
> On Fri, Jan 18, 2019 at 12:39:37PM +0530, Vasundhara Volam wrote:
> > There is difference of opinion on adding WOL parameter to devlink, between
> > Jakub Kicinski and Michael Chan.
> >
> > Quote from Jakud Kicinski:
> > ********
> > As explained previously I think it's a very bad idea to add existing
> > configuration options to devlink, just because devlink has the ability
> > to persist the setting in NVM.  Especially that for WoL you have to get
> > the link up so you potentially have all link config stuff as well.  And
> > that n-tuple filters are one of the WoL options, meaning we'd need the
> > ability to persist n-tuple filters via devlink.
> >
> > The effort would be far better spent helping with migrating ethtool to
> > netlink, and allowing persisting there.
> >
> > I have not heard any reason why devlink is a better fit.  I can imagine
> > you're just doing it here because it's less effort for you since
> > ethtool is not yet migrated.
> > ********
> >
> > Quote from Michael Chan:
> > ********
> > The devlink's WoL parameter is a persistent WoL parameter stored in the
> > NIC's NVRAM. It is different from ethtool's WoL parameter in a number of
> > ways. ethtool WoL is not persistent over AC power cycle and is considered
> > OS-present WoL. As such, ethtool WoL can use a more sophisticated pattern
> > including n-tuple with IP address in addition to the more basic types
> > (e.g. magic packet). Whereas OS-absent power up WoL should only include
> > magic packet and other simple types.
>
> If I understand correctly, it's that way now. I'm not sure there is a
> technical reason preventing more complex WoL types in the OS-absent case
> in the future. Also, even with traditional ethtool WoL setting, most
> NICs only support some of the types (I'm not sure if there is a NIC
> which would support all of them.)
Agreed. This can be extended in future for other types of WoL.

>
> > The devlink WoL setting does not have to match the ethtool WoL
> > setting.
>
> IMHO this is not really a problem. We can either use an additional flag
> telling kernel/driver if we are setting runtime or persistent WoL mask
> or we can pass (up to) two bitmaps.
This already supports as persistent or runtime WoL. All devlink parameters
have this option to be either persistent or runtime which is defined
by the driver
when defining the parameter.

>
> > The card will autoneg up to the speed supported by Vaux so no special
> > devlink link setting is needed.
> > ********
>
> Like Jakub, I'm not convinced there is a strong technical reason to have
> each of the WoL settings handled through a different interface. I don't
> say, though, that ethtool is necessarily the right one. If there is
> a consensus that it better fits into devlink, I can imagine that both
> could be accessible through devlink (for start, in drivers which choose
> so, e.g. because they want to implement the persistent setting).
Devlink supports both persistent and runtime WoL setting. It depends on
driver's use case.

>
> Michal Kubecek

Re: [PATCH net-next v7 0/8] devlink: Add configuration parameters support for devlink_port

[Vasundhara Volam]

On Wed, Jan 23, 2019 at 3:48 AM Jakub Kicinski
<jakub.kicinski@netronome.com> wrote:
>
> On Fri, 18 Jan 2019 15:33:19 +0100, Michal Kubecek wrote:
> > On Fri, Jan 18, 2019 at 12:39:37PM +0530, Vasundhara Volam wrote:
> > > There is difference of opinion on adding WOL parameter to devlink, between
> > > Jakub Kicinski and Michael Chan.
> > >
> > > Quote from Jakud Kicinski:
> > > ********
> > > As explained previously I think it's a very bad idea to add existing
> > > configuration options to devlink, just because devlink has the ability
> > > to persist the setting in NVM.  Especially that for WoL you have to get
> > > the link up so you potentially have all link config stuff as well.  And
> > > that n-tuple filters are one of the WoL options, meaning we'd need the
> > > ability to persist n-tuple filters via devlink.
> > >
> > > The effort would be far better spent helping with migrating ethtool to
> > > netlink, and allowing persisting there.
> > >
> > > I have not heard any reason why devlink is a better fit.  I can imagine
> > > you're just doing it here because it's less effort for you since
> > > ethtool is not yet migrated.
> > > ********
> > >
> > > Quote from Michael Chan:
> > > ********
> > > The devlink's WoL parameter is a persistent WoL parameter stored in the
> > > NIC's NVRAM. It is different from ethtool's WoL parameter in a number of
> > > ways. ethtool WoL is not persistent over AC power cycle and is considered
> > > OS-present WoL. As such, ethtool WoL can use a more sophisticated pattern
> > > including n-tuple with IP address in addition to the more basic types
> > > (e.g. magic packet). Whereas OS-absent power up WoL should only include
> > > magic packet and other simple types.
> >
> > If I understand correctly, it's that way now. I'm not sure there is a
> > technical reason preventing more complex WoL types in the OS-absent case
> > in the future. Also, even with traditional ethtool WoL setting, most
> > NICs only support some of the types (I'm not sure if there is a NIC
> > which would support all of them.)
> >
> > > The devlink WoL setting does not have to match the ethtool WoL
> > > setting.
> >
> > IMHO this is not really a problem. We can either use an additional flag
> > telling kernel/driver if we are setting runtime or persistent WoL mask
> > or we can pass (up to) two bitmaps.
>
> I think reusing new netlink ethtool with a special flag would be a nice,
> complete solution.  We could also address link settings this way (which
> are a pre-requisite for WoL).
>
> I have no strong preference on the mechanism, but for ease of setting
> as well as dumping would it be workable to use a nesting, like this:
>
> Run time settings:
>   [ETHTOOL_SETTINGS_BLA]
>     [ETHTOOL_BLA_VAL_1]
>     [ETHTOOL_BLA_VAL_2]
>     ...
>
> Persistent:
>   [ETHTOOL_PERSISTENT]
>     [ETHTOOL_SETTINGS_BLA]
>       [ETHTOOL_BLA_VAL_1]
>       [ETHTOOL_BLA_VAL_2]
>
> IOW encapsulate settings into a "persistent" attribute?
Not sure if current devlink framework allows to encapsulate additional
settings now.
But we can think of extending it to support this when there is a requirement.

>
> How does that look to you, Michal?
>
> > > The card will autoneg up to the speed supported by Vaux so no special
> > > devlink link setting is needed.
> > > ********
> >
> > Like Jakub, I'm not convinced there is a strong technical reason to have
> > each of the WoL settings handled through a different interface. I don't
> > say, though, that ethtool is necessarily the right one. If there is
> > a consensus that it better fits into devlink, I can imagine that both
> > could be accessible through devlink (for start, in drivers which choose
> > so, e.g. because they want to implement the persistent setting).
> >
> > Michal Kubecek

Re: [PATCH net-next v7 0/8] devlink: Add configuration parameters support for devlink_port

[Jakub Kicinski]

On Thu, 24 Jan 2019 15:16:27 +0530, Vasundhara Volam wrote:
> > > > The devlink WoL setting does not have to match the ethtool WoL
> > > > setting.  
> > >
> > > IMHO this is not really a problem. We can either use an additional flag
> > > telling kernel/driver if we are setting runtime or persistent WoL mask
> > > or we can pass (up to) two bitmaps.  
> >
> > I think reusing new netlink ethtool with a special flag would be a nice,
> > complete solution.  We could also address link settings this way (which
> > are a pre-requisite for WoL).
> >
> > I have no strong preference on the mechanism, but for ease of setting
> > as well as dumping would it be workable to use a nesting, like this:
> >
> > Run time settings:
> >   [ETHTOOL_SETTINGS_BLA]
> >     [ETHTOOL_BLA_VAL_1]
> >     [ETHTOOL_BLA_VAL_2]
> >     ...
> >
> > Persistent:
> >   [ETHTOOL_PERSISTENT]
> >     [ETHTOOL_SETTINGS_BLA]
> >       [ETHTOOL_BLA_VAL_1]
> >       [ETHTOOL_BLA_VAL_2]
> >
> > IOW encapsulate settings into a "persistent" attribute?  
> Not sure if current devlink framework allows to encapsulate additional
> settings now.
> But we can think of extending it to support this when there is a requirement.

To be clear the question was to Michal and about ethtool netlink, where
this configuration belongs.

Re: [PATCH net-next v7 0/8] devlink: Add configuration parameters support for devlink_port

[Michal Kubecek]

On Tue, Jan 22, 2019 at 02:18:42PM -0800, Jakub Kicinski wrote:
> 
> I think reusing new netlink ethtool with a special flag would be a nice,
> complete solution.  We could also address link settings this way (which
> are a pre-requisite for WoL).
> 
> I have no strong preference on the mechanism, but for ease of setting
> as well as dumping would it be workable to use a nesting, like this:
> 
> Run time settings:
>   [ETHTOOL_SETTINGS_BLA]
>     [ETHTOOL_BLA_VAL_1]
>     [ETHTOOL_BLA_VAL_2]
>     ...
> 
> Persistent:
>   [ETHTOOL_PERSISTENT]
>     [ETHTOOL_SETTINGS_BLA]
>       [ETHTOOL_BLA_VAL_1]
>       [ETHTOOL_BLA_VAL_2]
> 
> IOW encapsulate settings into a "persistent" attribute?
> 
> How does that look to you, Michal?

It's certainly one of the options worth considering. What I like is that
this approach would not require knowing (or rather guessing) which
attributes are going to allow persistent settings in the future.

Michal

Re: [PATCH net-next v7 0/8] devlink: Add configuration parameters support for devlink_port

[David Miller]

From: Michal Kubecek <mkubecek@suse.cz>
Date: Mon, 28 Jan 2019 00:07:30 +0100

> What I like is that this approach would not require knowing (or
> rather guessing) which attributes are going to allow persistent
> settings in the future.

+1

Re: [PATCH net-next v7 0/8] devlink: Add configuration parameters support for devlink_port

[Vasundhara Volam]

On Wed, Jan 23, 2019 at 3:48 AM Jakub Kicinski
<jakub.kicinski@netronome.com> wrote:
>
> On Fri, 18 Jan 2019 15:33:19 +0100, Michal Kubecek wrote:
> > On Fri, Jan 18, 2019 at 12:39:37PM +0530, Vasundhara Volam wrote:
> > > There is difference of opinion on adding WOL parameter to devlink, between
> > > Jakub Kicinski and Michael Chan.
> > >
> > > Quote from Jakud Kicinski:
> > > ********
> > > As explained previously I think it's a very bad idea to add existing
> > > configuration options to devlink, just because devlink has the ability
> > > to persist the setting in NVM.  Especially that for WoL you have to get
> > > the link up so you potentially have all link config stuff as well.  And
> > > that n-tuple filters are one of the WoL options, meaning we'd need the
> > > ability to persist n-tuple filters via devlink.
> > >
> > > The effort would be far better spent helping with migrating ethtool to
> > > netlink, and allowing persisting there.
> > >
> > > I have not heard any reason why devlink is a better fit.  I can imagine
> > > you're just doing it here because it's less effort for you since
> > > ethtool is not yet migrated.
> > > ********
> > >
> > > Quote from Michael Chan:
> > > ********
> > > The devlink's WoL parameter is a persistent WoL parameter stored in the
> > > NIC's NVRAM. It is different from ethtool's WoL parameter in a number of
> > > ways. ethtool WoL is not persistent over AC power cycle and is considered
> > > OS-present WoL. As such, ethtool WoL can use a more sophisticated pattern
> > > including n-tuple with IP address in addition to the more basic types
> > > (e.g. magic packet). Whereas OS-absent power up WoL should only include
> > > magic packet and other simple types.
> >
> > If I understand correctly, it's that way now. I'm not sure there is a
> > technical reason preventing more complex WoL types in the OS-absent case
> > in the future. Also, even with traditional ethtool WoL setting, most
> > NICs only support some of the types (I'm not sure if there is a NIC
> > which would support all of them.)
> >
> > > The devlink WoL setting does not have to match the ethtool WoL
> > > setting.
> >
> > IMHO this is not really a problem. We can either use an additional flag
> > telling kernel/driver if we are setting runtime or persistent WoL mask
> > or we can pass (up to) two bitmaps.
Jakub, I will add another two bitmask parameters for WoL named as
wake_on_lan_runtime
and wake_on_lan_persistent. This will give information about what
types are runtime and
what types are persistent, does the device support for any given WoL types.

Does that sound good?
>
> I think reusing new netlink ethtool with a special flag would be a nice,
> complete solution.  We could also address link settings this way (which
> are a pre-requisite for WoL).
>
> I have no strong preference on the mechanism, but for ease of setting
> as well as dumping would it be workable to use a nesting, like this:
>
> Run time settings:
>   [ETHTOOL_SETTINGS_BLA]
>     [ETHTOOL_BLA_VAL_1]
>     [ETHTOOL_BLA_VAL_2]
>     ...
>
> Persistent:
>   [ETHTOOL_PERSISTENT]
>     [ETHTOOL_SETTINGS_BLA]
>       [ETHTOOL_BLA_VAL_1]
>       [ETHTOOL_BLA_VAL_2]
>
> IOW encapsulate settings into a "persistent" attribute?
>
> How does that look to you, Michal?
>
> > > The card will autoneg up to the speed supported by Vaux so no special
> > > devlink link setting is needed.
> > > ********
> >
> > Like Jakub, I'm not convinced there is a strong technical reason to have
> > each of the WoL settings handled through a different interface. I don't
> > say, though, that ethtool is necessarily the right one. If there is
> > a consensus that it better fits into devlink, I can imagine that both
> > could be accessible through devlink (for start, in drivers which choose
> > so, e.g. because they want to implement the persistent setting).
> >
> > Michal Kubecek

Re: [PATCH net-next v7 0/8] devlink: Add configuration parameters support for devlink_port

[Jakub Kicinski]

On Mon, 4 Feb 2019 12:25:13 +0530, Vasundhara Volam wrote:
> > > IMHO this is not really a problem. We can either use an additional flag
> > > telling kernel/driver if we are setting runtime or persistent WoL mask
> > > or we can pass (up to) two bitmaps.  
> Jakub, I will add another two bitmask parameters for WoL named as
> wake_on_lan_runtime
> and wake_on_lan_persistent. This will give information about what
> types are runtime and
> what types are persistent, does the device support for any given WoL types.
> 
> Does that sound good?

No?  We were talking about using the soon-too-come ethtool netlink
API with additional indication that given configuration request is
supposed to be persisted.  Adding more devlink parameters is exactly 
the opposite of what you should be doing.

Re: [PATCH net-next v7 0/8] devlink: Add configuration parameters support for devlink_port

[Vasundhara Volam]

On Tue, Feb 5, 2019 at 8:26 AM Jakub Kicinski
<jakub.kicinski@netronome.com> wrote:
>
> On Mon, 4 Feb 2019 12:25:13 +0530, Vasundhara Volam wrote:
> > > > IMHO this is not really a problem. We can either use an additional flag
> > > > telling kernel/driver if we are setting runtime or persistent WoL mask
> > > > or we can pass (up to) two bitmaps.
> > Jakub, I will add another two bitmask parameters for WoL named as
> > wake_on_lan_runtime
> > and wake_on_lan_persistent. This will give information about what
> > types are runtime and
> > what types are persistent, does the device support for any given WoL types.
> >
> > Does that sound good?
>
> No?  We were talking about using the soon-too-come ethtool netlink
> API with additional indication that given configuration request is
> supposed to be persisted.  Adding more devlink parameters is exactly
> the opposite of what you should be doing.
Okay. So, till then can we have the devlink wake_on_lan parameter or
you want this to be removed? Could you please clarify?

Once ethtool netlink API is available with persisted support, I can remove
this wake_on_lan parameter from devlink. Thanks.

Re: [PATCH net-next v7 0/8] devlink: Add configuration parameters support for devlink_port

[Michal Kubecek]

On Tue, Feb 05, 2019 at 09:53:26AM +0530, Vasundhara Volam wrote:
> On Tue, Feb 5, 2019 at 8:26 AM Jakub Kicinski
> >
> > No?  We were talking about using the soon-too-come ethtool netlink
> > API with additional indication that given configuration request is
> > supposed to be persisted.  Adding more devlink parameters is exactly
> > the opposite of what you should be doing.
> 
> Okay. So, till then can we have the devlink wake_on_lan parameter or
> you want this to be removed? Could you please clarify?
> 
> Once ethtool netlink API is available with persisted support, I can remove
> this wake_on_lan parameter from devlink. Thanks.

Once you provide an interface for userspace and applications start using
it, it's hard to get rid of it. As an extreme example, the legacy ioctl
interface used by ifconfig has been declared obsolete since kernel 2.2.0
(January 1999, i.e. 20 years ago) and we still have to maintain it.

Michal Kubecek

Re: [PATCH net-next v7 0/8] devlink: Add configuration parameters support for devlink_port

[Vasundhara Volam]

On Tue, Feb 5, 2019 at 10:21 PM Michal Kubecek <mkubecek@suse.cz> wrote:
>
> On Tue, Feb 05, 2019 at 09:53:26AM +0530, Vasundhara Volam wrote:
> > On Tue, Feb 5, 2019 at 8:26 AM Jakub Kicinski
> > >
> > > No?  We were talking about using the soon-too-come ethtool netlink
> > > API with additional indication that given configuration request is
> > > supposed to be persisted.  Adding more devlink parameters is exactly
> > > the opposite of what you should be doing.
> >
> > Okay. So, till then can we have the devlink wake_on_lan parameter or
> > you want this to be removed? Could you please clarify?
> >
> > Once ethtool netlink API is available with persisted support, I can remove
> > this wake_on_lan parameter from devlink. Thanks.
>
> Once you provide an interface for userspace and applications start using
> it, it's hard to get rid of it. As an extreme example, the legacy ioctl
> interface used by ifconfig has been declared obsolete since kernel 2.2.0
> (January 1999, i.e. 20 years ago) and we still have to maintain it.
>
Okay Got it. I will revert only the wake_on_lan parameter and send the patch.
We will wait for soon-too-come ethtool netlink API.

Thank you.
> Michal Kubecek