[RFC net-next 0/3] devconf: New infrastructure for setting pre-load parameters for a module

[RFC net-next 0/3] devconf: New infrastructure for setting pre-load parameters for a module

[Hadar Hen Zion]

Hi,
    
When configuring a device at an early boot stage, most kernel drivers
use module parameters (the parameters' settings can be determined in
modprobe.d config files).
These parameters are difficult to manage, and one of the reasons is that
module parameters are set per driver and not per device (NICs using the
same driver cannot be set with different configurations).
Furthermore, using other existing configuration tools like ethtool,
ifconfig, ip link commands or sysfs entries is not applicable, since
they all rely on having a netdev already set up.

In the past, 'request_firmware' solution for configuration parameters
was suggested by Shannon Nelson from Intel[1]. The idea was rejected by
Greg KH, who claimed it was abusive of the request_firmware mechanism.
Greg suggested using configfs for device configuration instead (as done
by the USB gadget driver).

As a solution, we introduce a new kernel infrastructure using configfs
to allow the configuration of the device. The goal is to set low-level
device functionality that needs to be sorted out before a module is
loaded.

Configfs Solution:
----------------------
The implemented configfs solution is composed of one generic module
('devconf') that can load configuration modules per driver.
The devconf should be loaded at a very early boot stage, before other
kernel modules are loaded (or compiled as in-tree). After configfs is
mounted, it creates a new directory under configfs, called 'devices'.

In the example presented below, systemd mechanism is being used, but the
configuration can also work with older init systems.

1. A systemd service, scheduled by the OS to run before kernel modules
are loaded, creates a directory under 'devices' with the name of the driver.
The devconf module will try to load a configuration module for this
driver (if exists).

2. Drivers using this mechanism will be made of two parts:
a configuration module and the driver itself.

3. Later on, when the OS loads the driver, it uses the configuration
sub-tree.

To avoid dependencies between the modules, in case the configuration
module does not exist or is not loaded, the driver will use default
values.

Example:
--------
The below mlx4_core configuration file is only an example. Since the
infrastructure is generic, each vendor can choose how to identify its
devices (for example: bdf, mac address, uuid etc.).

If /etc/devconf.d/mlx4_core.conf is as follows (see below), systemd
service will use it to run the following commands (see below).

mlx4_core.conf file:
--------------------
[pdevs]
	[0000:00:08.0]
		dmfs = 0
		[ports]
			[1]
			type = 2
			[2]
			type =2
	[0000:00:04.0]
		dmfs = 0
		[ports]
			[1]
			type = 5
			[2]
			type =1

Systemd commands:
-----------------
$ mkdir /sys/kernel/config/devices/mlx4_core

devconf module will try to load dev_c_mlx4.ko

$ mkdir /sys/kernel/config/devices/mlx4_core/pdevs
$ mkdir /sys/kernel/config/devices/mlx4_core/pdevs/0000:00:08.0
$ mkdir /sys/kernel/config/devices/mlx4_core/pdevs/0000:00:08.0/ports
$ mkdir /sys/kernel/config/devices/mlx4_core/pdevs/0000:00:08.0/ports/1
$ mkdir /sys/kernel/config/devices/mlx4_core/pdevs/0000:00:08.0/ports/2

$ echo 5 > /sys/kernel/config/devices/mlx4_core/pdevs/0000:00:08.0/ports/1/type
$ echo 1 > /sys/kernel/config/devices/mlx4_core/pdevs/0000:00:08.0/ports/2/type
$ echo 0 > /sys/kernel/config/devices/mlx4_core/pdevs/0000:00:08.0/dmfs
[...]

dev_c_mlx4 will reject the above configuration, printing a clear message
to dmesg:
"mlx4_core: Unknown port type '5' for device 0000:00:08.0. Use 0 for
auto, 1 for IB and 2 for ETH"

The driver configuration module validates and prepares configfs sub-tree
for the driver itself.
A configuration sub-tree for a specific module will be created under
/sys/kernel/config/devices/<driver name>, and propagated from a
configuration file on the OS (located under: /etc/devconf.d/).

Configfs Sub-tree Example (for mlx4_core):
------------------------------------------
$ cd /sys/kernel/config/
$ tree devices/
devices/
└── mlx4_core
    └── pdevs
	    ├── 0000:00:04.0
	    │   ├── dmfs
	    │   └── ports
	    │       ├── 1
	    │       │   └── type
	    │       └── 2
	    │		└── type
	    └── 0000:00:08.0
	 	├── dmfs
	 	└── ports
	 	    ├── 1
	 	    │ 	└── type
	 	    └── 2
	 		└── type

The systemd service that populates configfs is part of the sysinit
target. The service is executed after mounting configfs and before udev
load kernel modules.

To use devconf infrastructure, the following should be included:
1. Devconf systemd service
2. Configuration file in the right format under: /etc/devconf.d/

This suggested solution is generic and designed to suite any type of
device. The goal is to make this solution generic enough so all kinds of
drivers are able to use it.
As a start, this RFC is being sent to the netdev mailing list for
feedback, but will eventually be submitted to the LKML mailing list as a
generic kernel infrastructure.

Hadar.

[1] - https://lkml.org/lkml/2013/1/10/606

Hadar Hen Zion (3):
  net/dev_c_mlx4: Support mlx4_core pre-load configuration
  devconf: Add configuration module for setting pre-load parameters
  net/mlx4_core: Set port_type value according to configuration module

 Documentation/filesystems/configfs/devconf.txt |  135 ++++++++++
 drivers/Kconfig                                |    2 +
 drivers/Makefile                               |    1 +
 drivers/devconf/Kconfig                        |    6 +
 drivers/devconf/Makefile                       |    3 +
 drivers/devconf/driver.c                       |  160 ++++++++++++
 drivers/devconf/main.c                         |  103 ++++++++
 drivers/net/ethernet/mellanox/mlx4/Kconfig     |    6 +
 drivers/net/ethernet/mellanox/mlx4/Makefile    |    4 +
 drivers/net/ethernet/mellanox/mlx4/main.c      |   55 ++++-
 drivers/net/ethernet/mellanox/mlx4/mlx4.h      |    1 +
 drivers/net/ethernet/mellanox/mlx4/mlx4_conf.c |  323 ++++++++++++++++++++++++
 include/linux/devconf.h                        |   69 +++++
 13 files changed, 867 insertions(+), 1 deletions(-)
 create mode 100644 Documentation/filesystems/configfs/devconf.txt
 create mode 100644 drivers/devconf/Kconfig
 create mode 100644 drivers/devconf/Makefile
 create mode 100644 drivers/devconf/driver.c
 create mode 100644 drivers/devconf/main.c
 create mode 100644 drivers/net/ethernet/mellanox/mlx4/mlx4_conf.c
 create mode 100644 include/linux/devconf.h

-- 
1.7.8.2

[RFC net-next 1/3] net/dev_c_mlx4: Support mlx4_core pre-load configuration

[Hadar Hen Zion]

Adding a configuration Module to mlx4_core to support setting of
pre-load parameters through configfs.

When dev_c_mlx4 module is loaded, it stores and validates the parameters
sub-tree for mlx4_core driver under 'devices/mlx4_core' directory in configfs.
The parameters sub-tree is a set of directories and files representing
mlx4_core variables that should be set before mlx4_core is loaded.

mlx4_core sub-tree includes directories per device, identified by a
'BDF' name, and under each directory/device its relevant parameters.

Example of optional mlx4_core hierarchy under configfs:
$ cd /sys/kernel/config/
$ tree devices/
devices/
└── mlx4_core
    └── pdevs
            ├── 0000:00:04.0
	    │   ├── dmfs
	    │   └── ports
	    │       ├── 1
	    │       │   └── type
	    │       └── 2
	    │   	└── type
	    └── 0000:00:08.0
	    	├── dmfs
	    	└── ports
	    	    ├── 1
	    	    │   └── type
	    	    └── 2
	    		└── type

systemd service executed by the OS will set a value to those files
according to the user configuration file (/etc/devconf.d/mlx4_core.conf)
before mlx4_core is loaded.
When mlx4_core is loaded, it asks devconf for 'dmfs' and 'type' values,
devconf retrieves the values stored in dev_c_mlx4 and passes them on to
mlx4_core.

Signed-off-by: Hadar Hen Zion <hadarh@mellanox.com>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/Kconfig     |    5 +
 drivers/net/ethernet/mellanox/mlx4/Makefile    |    4 +
 drivers/net/ethernet/mellanox/mlx4/mlx4_conf.c |  323 ++++++++++++++++++++++++
 3 files changed, 332 insertions(+), 0 deletions(-)
 create mode 100644 drivers/net/ethernet/mellanox/mlx4/mlx4_conf.c

diff --git a/drivers/net/ethernet/mellanox/mlx4/Kconfig b/drivers/net/ethernet/mellanox/mlx4/Kconfig
index 1486ce9..fa135d3 100644
--- a/drivers/net/ethernet/mellanox/mlx4/Kconfig
+++ b/drivers/net/ethernet/mellanox/mlx4/Kconfig
@@ -44,3 +44,8 @@ config MLX4_DEBUG
 	  mlx4_core driver.  The output can be turned on via the
 	  debug_level module parameter (which can also be set after
 	  the driver is loaded through sysfs).
+
+config MLX4_DEV_C
+	tristate "mlx4 driver pre-loading configuration module"
+	depends on DEVCONF
+	default m
diff --git a/drivers/net/ethernet/mellanox/mlx4/Makefile b/drivers/net/ethernet/mellanox/mlx4/Makefile
index 3e9c70f..33a1dc8 100644
--- a/drivers/net/ethernet/mellanox/mlx4/Makefile
+++ b/drivers/net/ethernet/mellanox/mlx4/Makefile
@@ -8,3 +8,7 @@ obj-$(CONFIG_MLX4_EN)               += mlx4_en.o
 mlx4_en-y := 	en_main.o en_tx.o en_rx.o en_ethtool.o en_port.o en_cq.o \
 		en_resources.o en_netdev.o en_selftest.o en_clock.o
 mlx4_en-$(CONFIG_MLX4_EN_DCB) += en_dcb_nl.o
+
+obj-$(CONFIG_MLX4_DEV_C)	+= dev_c_mlx4.o
+
+dev_c_mlx4-y := mlx4_conf.o
diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4_conf.c b/drivers/net/ethernet/mellanox/mlx4/mlx4_conf.c
new file mode 100644
index 0000000..c82924b
--- /dev/null
+++ b/drivers/net/ethernet/mellanox/mlx4/mlx4_conf.c
@@ -0,0 +1,323 @@
+/*
+ * Copyright (c) 2015 Mellanox Technologies. All rights reserved.
+ *
+ * This software is available to you under a choice of one of two
+ * licenses.  You may choose to be licensed under the terms of the GNU
+ * General Public License (GPL) Version 2, available from the file
+ * COPYING in the main directory of this source tree, or the
+ * OpenIB.org BSD license below:
+ *
+ *     Redistribution and use in source and binary forms, with or
+ *     without modification, are permitted provided that the following
+ *     conditions are met:
+ *
+ *      - Redistributions of source code must retain the above
+ *        copyright notice, this list of conditions and the following
+ *        disclaimer.
+ *
+ *      - Redistributions in binary form must reproduce the above
+ *        copyright notice, this list of conditions and the following
+ *        disclaimer in the documentation and/or other materials
+ *        provided with the distribution.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+
+#include <linux/configfs.h>
+#include <linux/devconf.h>
+#include <linux/mlx4/device.h>
+
+struct mlx4_port {
+	struct devconf_config_object cobj;
+	int type;
+};
+
+struct mlx4_bdf_config {
+	struct devconf_config_object cobj;
+	int dmfs;
+};
+
+static inline struct mlx4_bdf_config *to_bdf_config(struct config_item *item)
+{
+	struct devconf_config_object *cobj = to_config_obj(item);
+
+	return cobj ? container_of(cobj, struct mlx4_bdf_config, cobj) : NULL;
+}
+
+static inline struct mlx4_port *to_simple_port(struct config_item *item)
+{
+	struct devconf_config_object *cobj = to_config_obj(item);
+
+	return cobj ? container_of(cobj, struct mlx4_port, cobj) : NULL;
+}
+
+static inline struct mlx4_bdf_config
+*cobj_to_bdf_config(struct devconf_config_object *cobj)
+{
+	return cobj ? container_of(cobj, struct mlx4_bdf_config, cobj) : NULL;
+}
+
+static inline struct mlx4_port
+*cobj_to_port(struct devconf_config_object *cobj)
+{
+	return cobj ? container_of(cobj, struct mlx4_port, cobj) : NULL;
+}
+
+static struct configfs_attribute simple_port_attr_type = {
+	.ca_owner = THIS_MODULE,
+	.ca_name = "type",
+	.ca_mode = S_IRUGO | S_IWUSR,
+};
+
+static struct configfs_attribute *simple_port_attrs[] = {
+	&simple_port_attr_type,
+	NULL,
+};
+
+static ssize_t config_attr_show(struct config_item *item,
+				struct configfs_attribute *attr,
+				char *page)
+{
+	struct mlx4_bdf_config *bdf_config;
+	struct mlx4_port *simple_port;
+	ssize_t count = -ENOENT;
+
+	if (strcmp(attr->ca_name, "dmfs")) {
+		bdf_config = to_bdf_config(item);
+		count = sprintf(page, "%d\n", bdf_config->dmfs);
+	} else if (strcmp(attr->ca_name, "type")) {
+		simple_port = to_simple_port(item);
+		count = sprintf(page, "%d\n", simple_port->type);
+	}
+	return count;
+}
+
+static ssize_t config_attr_store(struct config_item *item,
+				 struct configfs_attribute *attr,
+				 const char *page, size_t count)
+{
+	struct mlx4_bdf_config *bdf_config;
+	struct mlx4_port *simple_port;
+	int err;
+	unsigned long res;
+
+	err = kstrtoul(page, 10, &res);
+	if (err)
+		return err;
+	if (strcmp(attr->ca_name, "dmfs")) {
+		bdf_config = to_bdf_config(item);
+		bdf_config->dmfs = res;
+	} else if (strcmp(attr->ca_name, "type")) {
+		simple_port = to_simple_port(item);
+		simple_port->type = res;
+	}
+
+	return count;
+}
+
+static void simple_port_release(struct config_item *item)
+{
+	kfree(to_simple_port(item));
+}
+
+static struct configfs_item_operations simple_port_item_ops = {
+	.release                = simple_port_release,
+	.show_attribute         = config_attr_show,
+	.store_attribute        = config_attr_store,
+};
+
+static struct config_item_type simple_port_config_type = {
+	.ct_item_ops    = &simple_port_item_ops,
+	.ct_attrs       = simple_port_attrs,
+	.ct_owner       = THIS_MODULE,
+};
+
+static struct config_item *simple_port_make_item(struct config_group *group,
+						 const char *name)
+{
+	struct mlx4_port *simple_port;
+
+	simple_port = kzalloc(sizeof(*simple_port), GFP_KERNEL);
+	if (!simple_port)
+		return ERR_PTR(-ENOMEM);
+
+	config_item_init_type_name(&simple_port->cobj.group.cg_item, name,
+				   &simple_port_config_type);
+
+	simple_port->type = 0;
+
+	return &simple_port->cobj.group.cg_item;
+}
+
+static struct configfs_group_operations ports_group_ops = {
+	.make_item = simple_port_make_item,
+};
+
+static void cobj_release(struct config_item *item)
+{
+	kfree(to_config_obj(item));
+}
+
+static struct configfs_item_operations ports_item_ops = {
+	.release = cobj_release,
+};
+
+static struct config_item_type ports_config_type = {
+	.ct_item_ops    = &ports_item_ops,
+	.ct_group_ops   = &ports_group_ops,
+	.ct_owner       = THIS_MODULE,
+};
+
+static struct config_group *bdf_make_group(struct config_group *group,
+					   const char *name)
+{
+	struct devconf_config_object *ports_config;
+
+	ports_config = kzalloc(sizeof(*ports_config), GFP_KERNEL);
+	if (!ports_config)
+		return ERR_PTR(-ENOMEM);
+
+	config_group_init_type_name(&ports_config->group, name,
+				    &ports_config_type);
+
+	return &ports_config->group;
+}
+
+static struct configfs_group_operations bdf_group_ops = {
+	.make_group = bdf_make_group,
+};
+
+static void bdf_config_release(struct config_item *item)
+{
+	kfree(to_bdf_config(item));
+}
+
+static struct configfs_item_operations bdf_item_ops = {
+	.release		= bdf_config_release,
+	.show_attribute		= config_attr_show,
+	.store_attribute	= config_attr_store,
+};
+
+static struct configfs_attribute config_attr_dmfs = {
+	.ca_owner = THIS_MODULE,
+	.ca_name = "dmfs",
+	.ca_mode = S_IRUGO | S_IWUSR,
+};
+
+static struct configfs_attribute *bdf_config_attrs[] = {
+	&config_attr_dmfs,
+	NULL,
+};
+
+static struct config_item_type bdf_config_type = {
+	.ct_item_ops    = &bdf_item_ops,
+	.ct_group_ops   = &bdf_group_ops,
+	.ct_attrs       = bdf_config_attrs,
+	.ct_owner       = THIS_MODULE,
+};
+
+static struct config_group *pdevs_make_group(struct config_group *group,
+					     const char *name)
+{
+	struct mlx4_bdf_config *bdf_config;
+
+	bdf_config = kzalloc(sizeof(*bdf_config), GFP_KERNEL);
+	if (!bdf_config)
+		return ERR_PTR(-ENOMEM);
+
+	config_group_init_type_name(&bdf_config->cobj.group, name,
+				    &bdf_config_type);
+
+	return &bdf_config->cobj.group;
+}
+
+static struct configfs_group_operations pdevs_group_ops = {
+	.make_group = pdevs_make_group,
+};
+
+static struct configfs_item_operations pdevs_item_ops = {
+	.release = cobj_release,
+};
+
+static struct config_item_type pdevs_config_type = {
+	.ct_item_ops    = &pdevs_item_ops,
+	.ct_group_ops   = &pdevs_group_ops,
+	.ct_owner       = THIS_MODULE,
+};
+
+static struct config_group *mlx4_make_group(struct config_group *group,
+					    const char *name)
+{
+	struct devconf_config_object *pdevs_cobj;
+
+	pdevs_cobj = kzalloc(sizeof(*pdevs_cobj), GFP_KERNEL);
+	if (!pdevs_cobj)
+		return ERR_PTR(-ENOMEM);
+
+	config_group_init_type_name(&pdevs_cobj->group, name,
+				    &pdevs_config_type);
+
+	return &pdevs_cobj->group;
+}
+
+static struct configfs_group_operations mlx4_group_ops = {
+	.make_group = mlx4_make_group,
+};
+
+static void mlx4_release(struct config_item *item)
+{
+	struct devconf_config_driver *cdrv = to_config_driver(item);
+
+	devconf_put_config_driver(cdrv);
+}
+
+static struct configfs_item_operations mlx4_item_ops = {
+	.release = mlx4_release,
+};
+
+static struct config_item_type mlx4_config_type = {
+	.ct_item_ops    = &mlx4_item_ops,
+	.ct_group_ops	= &mlx4_group_ops,
+	.ct_owner	= THIS_MODULE,
+};
+
+static int mlx4_get_int_attr(struct devconf_config_object *cobj,
+			     const char *attr_name, int *val)
+{
+	struct mlx4_port *simple_port;
+	struct mlx4_bdf_config *bdf_config;
+
+	if (strcmp(attr_name, "dmfs")) {
+		bdf_config = cobj_to_bdf_config(cobj);
+		*val = bdf_config->dmfs;
+	} else if (strcmp(attr_name, "type")) {
+		simple_port = cobj_to_port(cobj);
+		*val = simple_port->type;
+	} else {
+		return -ENOENT;
+	}
+	return 0;
+}
+
+static void mlx4_set_config_object(struct devconf_config_driver *cdrv,
+				   const char *name)
+{
+	config_group_init_type_name(&cdrv->cobj.group, name, &mlx4_config_type);
+}
+
+DECLARE_DEVCONF_DRIVER_INIT(mlx4_core,
+			    mlx4_set_config_object,
+			    mlx4_get_int_attr);
+
+MODULE_LICENSE("GPL");
-- 
1.7.8.2

[RFC net-next 2/3] devconf: Add configuration module for setting pre-load parameters

[Hadar Hen Zion]

Introducing a new kernel infrastructure using configfs to allow the
configuration of low-level device functionality that needs to be sorted
out before a module is loaded. The suggested solution is generic and is
designed to suite any type of device.

The devconf module presented in this commit gives generic services for a
'dev_c_<driver name>' module. The 'dev_c_<driver name>' module will be
written by each vendor who wishes to implement a pre-load configuration
module for its driver.

Motivation and design for the devconf infrastructure is available under:
Documentation/filesystems/configfs/devconf.txt.

Signed-off-by: Hadar Hen Zion <hadarh@mellanox.com>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 Documentation/filesystems/configfs/devconf.txt |  135 ++++++++++++++++++++
 drivers/Kconfig                                |    2 +
 drivers/Makefile                               |    1 +
 drivers/devconf/Kconfig                        |    6 +
 drivers/devconf/Makefile                       |    3 +
 drivers/devconf/driver.c                       |  160 ++++++++++++++++++++++++
 drivers/devconf/main.c                         |  103 +++++++++++++++
 include/linux/devconf.h                        |   69 ++++++++++
 8 files changed, 479 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/filesystems/configfs/devconf.txt
 create mode 100644 drivers/devconf/Kconfig
 create mode 100644 drivers/devconf/Makefile
 create mode 100644 drivers/devconf/driver.c
 create mode 100644 drivers/devconf/main.c
 create mode 100644 include/linux/devconf.h

diff --git a/Documentation/filesystems/configfs/devconf.txt b/Documentation/filesystems/configfs/devconf.txt
new file mode 100644
index 0000000..a1b8039
--- /dev/null
+++ b/Documentation/filesystems/configfs/devconf.txt
@@ -0,0 +1,135 @@
+Configfs infrastructure for setting pre-load parameters for a module
+====================================================================
+
+When configuring a device at an early boot stage, most kernel drivers use
+module parameters (the parameters' settings can be determined in modprobe.d
+config files.) These parameters are difficult to manage, and one of the
+reasons is that module parameters are set per driver and not per device
+(NICs using the same driver cannot be set with different configurations).
+Furthermore, using other existing configuration tools like ethtool, ifconfig,
+ip link commands or sysfs entries is not applicable, since they all rely on
+having a netdev already set up.
+
+In the past, 'request_firmware' solution for configuration parameters was
+suggested by Shannon Nelson from Intel[1]. The idea was rejected by Greg KH, who
+claimed it was abusive of the request_firmware mechanism. Greg suggested using
+configfs for device configuration instead (as done by the USB gadget driver).
+
+As a solution, we introduce a new kernel infrastructure using configfs to
+allow the configuration of the device. The goal is to set low-level device
+functionality that needs to be sorted out before a module is loaded.
+
+Configfs Solution
+=====================
+The implemented configfs solution is composed of one generic module ('devconf')
+that can load configuration modules per driver. The devconf should be loaded
+at a very early boot stage, before other kernel modules are loaded (or
+compiled as in-tree). After configfs is mounted, it creates a new directory
+under configfs, called 'devices'.
+
+In the example presented below, systemd mechanism is being used, but the
+configuration can also work with older init systems.
+
+1. A systemd service, scheduled by the OS to run before kernel modules
+are loaded, creates a directory under 'devices' with the name of the driver.
+The devconf module will try to load a configuration module for this driver (if
+exists).
+
+2. Drivers using this mechanism will be made of two parts: a
+configuration module and the driver itself.
+
+3. Later on, when the OS loads the driver, it uses the configuration
+sub-tree.
+
+To avoid dependencies between the modules, in case the configuration module
+does not exist or is not loaded, the driver will use default values.
+
+Example
+=======
+The below mlx4_core configuration file is only an example. Since the
+infrastructure is generic, each vendor can choose how to identify its devices
+(for example: bdf, mac address, uuid etc.).
+
+If /etc/devconf.d/mlx4_core.conf is as follows (see below), systemd service
+will use it to run the following commands (see below).
+
+mlx4_core.conf file:
+--------------------
+[pdevs]
+	[0000:00:08.0]
+		dmfs = 0
+		[ports]
+			[1]
+			type = 2
+			[2]
+			type =2
+	[0000:00:04.0]
+		dmfs = 0
+		[ports]
+			[1]
+			type = 5
+			[2]
+			type =1
+
+Systemd commands:
+-----------------
+$ mkdir /sys/kernel/config/devices/mlx4_core
+
+devconf module will try to load dev_c_mlx4.ko
+
+$ mkdir /sys/kernel/config/devices/mlx4_core/pdevs
+$ mkdir /sys/kernel/config/devices/mlx4_core/pdevs/0000:00:08.0/
+$ mkdir /sys/kernel/config/devices/mlx4_core/pdevs/0000:00:08.0/ports
+$ mkdir /sys/kernel/config/devices/mlx4_core/pdevs/0000:00:08.0/ports/1
+$ mkdir /sys/kernel/config/devices/mlx4_core/pdevs/0000:00:08.0/ports/2
+
+$ echo 5 > /sys/kernel/config/devices/mlx4_core/pdevs/0000:00:08.0/ports/1/type
+$ echo 1 > /sys/kernel/config/devices/mlx4_core/pdevs/0000:00:08.0/ports/2/type
+$ echo 0 > /sys/kernel/config/devices/mlx4_core/pdevs/0000:00:08.0/dmfs
+[...]
+
+dev_c_mlx4 will reject the above configuration, printing a clear message to
+dmesg:
+"mlx4_core: Unknown port type '5' for device 0000:00:08.0. Use 0 for auto,
+1 for IB and 2 for ETH"
+
+The driver configuration module validates and prepares configfs sub-tree for
+the driver itself. A configuration sub-tree for a specific module will be
+created under /sys/kernel/config/devices/<driver name>, and propagated from a
+configuration file on the OS (located under: /etc/devconf.d/).
+
+Configfs Sub-tree Example (for mlx4_core):
+------------------------------------------
+$ cd /sys/kernel/config/
+$ tree devices/
+devices/
+ └── mlx4_core
+     └── pdevs
+           ├── 0000:00:04.0
+ 	    │   ├── dmfs
+ 	    │   └── ports
+ 	    │       ├── 1
+ 	    │       │   └── type
+ 	    │       └── 2
+ 	    │   	└── type
+ 	    └── 0000:00:08.0
+ 	    	├── dmfs
+ 	    	└── ports
+ 	    	    ├── 1
+ 	    	    │   └── type
+ 	    	    └── 2
+ 	    		└── type
+
+The systemd service that populates configfs is part of the sysinit target.
+The service is executed after mounting configfs and before udev load kernel
+modules.
+
+To use devconf infrastructure, the following should be included:
+1. Devconf systemd service
+2. Configuration file in the right format under: /etc/devconf.d/
+
+This suggested solution is generic and designed to suite any type of device.
+The goal is to make this solution generic enough so all kinds of drivers
+are able to use it.
+
+[1] - https://lkml.org/lkml/2013/1/10/606
diff --git a/drivers/Kconfig b/drivers/Kconfig
index 694d5a7..5a489e3 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -186,4 +186,6 @@ source "drivers/thunderbolt/Kconfig"
 
 source "drivers/android/Kconfig"
 
+source "drivers/devconf/Kconfig"
+
 endmenu
diff --git a/drivers/Makefile b/drivers/Makefile
index 67d2334..05e0d48 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -163,3 +163,4 @@ obj-$(CONFIG_RAS)		+= ras/
 obj-$(CONFIG_THUNDERBOLT)	+= thunderbolt/
 obj-$(CONFIG_CORESIGHT)		+= coresight/
 obj-$(CONFIG_ANDROID)		+= android/
+obj-$(CONFIG_DEVCONF)		+= devconf/
diff --git a/drivers/devconf/Kconfig b/drivers/devconf/Kconfig
new file mode 100644
index 0000000..3a98617
--- /dev/null
+++ b/drivers/devconf/Kconfig
@@ -0,0 +1,6 @@
+config DEVCONF
+	tristate "Driver configuration module"
+	depends on CONFIGFS_FS
+	default n
+	---help---
+	 Allow Setting pre-load parameters for drivers using configfs file system.
diff --git a/drivers/devconf/Makefile b/drivers/devconf/Makefile
new file mode 100644
index 0000000..f2a5f1f
--- /dev/null
+++ b/drivers/devconf/Makefile
@@ -0,0 +1,3 @@
+obj-$(CONFIG_DEVCONF)  += devconf.o
+
+devconf-y := main.o driver.o
diff --git a/drivers/devconf/driver.c b/drivers/devconf/driver.c
new file mode 100644
index 0000000..230afa6
--- /dev/null
+++ b/drivers/devconf/driver.c
@@ -0,0 +1,160 @@
+/*
+ * Copyright (c) 2015 Mellanox Technologies. All rights reserved.
+ *
+ * This software is available to you under a choice of one of two
+ * licenses.  You may choose to be licensed under the terms of the GNU
+ * General Public License (GPL) Version 2, available from the file
+ * COPYING in the main directory of this source tree, or the
+ * OpenIB.org BSD license below:
+ *
+ *     Redistribution and use in source and binary forms, with or
+ *     without modification, are permitted provided that the following
+ *     conditions are met:
+ *
+ *      - Redistributions of source code must retain the above
+ *        copyright notice, this list of conditions and the following
+ *        disclaimer.
+ *
+ *      - Redistributions in binary form must reproduce the above
+ *        copyright notice, this list of conditions and the following
+ *        disclaimer in the documentation and/or other materials
+ *        provided with the distribution.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/module.h>
+#include <linux/err.h>
+
+#include <linux/devconf.h>
+
+static LIST_HEAD(config_drivers_list);
+static DEFINE_MUTEX(drivers_lock);
+
+static struct devconf_config_driver *find_config_driver(const char *name)
+{
+	struct devconf_config_driver *cd, *tmp;
+
+	cd = ERR_PTR(-ENOENT);
+	mutex_lock(&drivers_lock);
+	list_for_each_entry(tmp, &config_drivers_list, list) {
+		if (strcmp(name, tmp->name))
+			continue;
+		else
+			cd = tmp;
+		if (!try_module_get(cd->mod)) {
+			cd = ERR_PTR(-EBUSY);
+			break;
+		}
+		cd->set_config_object(cd, name);
+		break;
+	}
+	mutex_unlock(&drivers_lock);
+	return cd;
+}
+
+struct devconf_config_driver *devconf_create_config_driver(const char *name)
+{
+	struct devconf_config_driver *cdrv;
+	int ret;
+
+	cdrv = find_config_driver(name);
+	if (!IS_ERR(cdrv))
+		return cdrv;
+	if (PTR_ERR(cdrv) != -ENOENT)
+		return cdrv;
+	ret = request_module("devconf:%s", name);
+	if (ret < 0)
+		return ERR_PTR(ret);
+	return find_config_driver(name);
+}
+
+void devconf_put_config_driver(struct devconf_config_driver *cdrv)
+{
+	struct module *mod;
+
+	if (!cdrv)
+		return;
+
+	mod = cdrv->mod;
+	module_put(mod);
+}
+EXPORT_SYMBOL_GPL(devconf_put_config_driver);
+
+int devconf_device_register(struct devconf_config_driver *newd)
+{
+	struct devconf_config_driver *cd;
+	int ret = -EEXIST;
+
+	mutex_lock(&drivers_lock);
+	list_for_each_entry(cd, &config_drivers_list, list) {
+		if (!strcmp(cd->name, newd->name))
+			goto out;
+	}
+	ret = 0;
+	list_add_tail(&newd->list, &config_drivers_list);
+out:
+	mutex_unlock(&drivers_lock);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(devconf_device_register);
+
+void devconf_device_unregister(struct devconf_config_driver *cd)
+{
+	mutex_lock(&drivers_lock);
+	list_del(&cd->list);
+	mutex_unlock(&drivers_lock);
+}
+EXPORT_SYMBOL_GPL(devconf_device_unregister);
+
+struct devconf_config_driver *devconf_get_config_driver(const char *name)
+{
+	struct devconf_config_driver *cd, *tmp;
+
+	cd = ERR_PTR(-ENOENT);
+	mutex_lock(&drivers_lock);
+	list_for_each_entry(tmp, &config_drivers_list, list) {
+		if (!strcmp(name, tmp->name)) {
+			cd = tmp;
+			break;
+		}
+	}
+	mutex_unlock(&drivers_lock);
+	return cd;
+}
+EXPORT_SYMBOL(devconf_get_config_driver);
+
+struct devconf_config_object
+*devconf_get_config_object(struct config_group *group, const char *name)
+{
+	struct config_item *item;
+	struct devconf_config_object *cobj;
+
+	mutex_lock(&group->cg_subsys->su_mutex);
+	item = config_group_find_item(group, name);
+	mutex_unlock(&group->cg_subsys->su_mutex);
+
+	cobj = to_config_obj(item);
+	return cobj;
+}
+EXPORT_SYMBOL(devconf_get_config_object);
+
+ssize_t devconf_get_int_attr(struct devconf_config_driver *cdrv,
+			     struct devconf_config_object *cobj,
+			     const char *param, int *value)
+{
+	if (cdrv->get_int_attr)
+		return cdrv->get_int_attr(cobj, param, value);
+	else
+		return -ENOSYS;
+}
+EXPORT_SYMBOL(devconf_get_int_attr);
diff --git a/drivers/devconf/main.c b/drivers/devconf/main.c
new file mode 100644
index 0000000..e308d4b
--- /dev/null
+++ b/drivers/devconf/main.c
@@ -0,0 +1,103 @@
+/*
+ * Copyright (c) 2015 Mellanox Technologies. All rights reserved.
+ *
+ * This software is available to you under a choice of one of two
+ * licenses.  You may choose to be licensed under the terms of the GNU
+ * General Public License (GPL) Version 2, available from the file
+ * COPYING in the main directory of this source tree, or the
+ * OpenIB.org BSD license below:
+ *
+ *     Redistribution and use in source and binary forms, with or
+ *     without modification, are permitted provided that the following
+ *     conditions are met:
+ *
+ *      - Redistributions of source code must retain the above
+ *        copyright notice, this list of conditions and the following
+ *        disclaimer.
+ *
+ *      - Redistributions in binary form must reproduce the above
+ *        copyright notice, this list of conditions and the following
+ *        disclaimer in the documentation and/or other materials
+ *        provided with the distribution.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/configfs.h>
+
+#include <linux/devconf.h>
+
+#define MAX_NAME_LEN	40
+
+static struct config_group *device_driver_make(struct config_group *group,
+					       const char *name)
+{
+	struct devconf_config_driver *cdrv;
+
+	cdrv = devconf_create_config_driver(name);
+	if (IS_ERR(cdrv))
+		return ERR_CAST(cdrv);
+
+	return &cdrv->cobj.group;
+}
+
+static struct configfs_group_operations devices_group_ops = {
+	.make_group	= device_driver_make,
+};
+
+static struct config_item_type devices_type = {
+	.ct_group_ops	= &devices_group_ops,
+	.ct_owner	= THIS_MODULE,
+};
+
+static struct configfs_subsystem devices_subsys = {
+	.su_group = {
+		.cg_item = {
+			.ci_namebuf = "devices",
+			.ci_type = &devices_type,
+		},
+	},
+};
+
+static int __init devconf_init(void)
+{
+	int ret;
+	struct configfs_subsystem *subsys = &devices_subsys;
+
+	config_group_init(&subsys->su_group);
+	mutex_init(&subsys->su_mutex);
+	ret = configfs_register_subsystem(subsys);
+	if (ret) {
+		pr_err("Error %d while registering subsystem %s\n",
+		       ret,
+		       subsys->su_group.cg_item.ci_namebuf);
+		goto out_unregister;
+	}
+	return 0;
+
+out_unregister:
+	configfs_unregister_subsystem(subsys);
+
+	return ret;
+}
+
+static void __exit devconf_exit(void)
+{
+	struct configfs_subsystem *subsys = &devices_subsys;
+
+	configfs_unregister_subsystem(subsys);
+}
+
+module_init(devconf_init);
+module_exit(devconf_exit);
+MODULE_LICENSE("GPL");
diff --git a/include/linux/devconf.h b/include/linux/devconf.h
new file mode 100644
index 0000000..ce78662
--- /dev/null
+++ b/include/linux/devconf.h
@@ -0,0 +1,69 @@
+#ifndef	__LINUX_DEVCONF_H
+#define	__LINUX_DEVCONF_H
+#include <linux/configfs.h>
+
+struct devconf_config_object {
+	struct config_group group;
+};
+
+struct devconf_config_driver {
+	struct list_head list;
+	struct devconf_config_object cobj;
+	const char *name;
+	struct module *mod;
+	void (*set_config_object)(struct devconf_config_driver *cdrv,
+				  const char *name);
+	int (*get_int_attr)(struct devconf_config_object *cobj,
+			    const char *attr_name, int *val);
+};
+
+struct devconf_config_driver *devconf_get_config_driver(const char *name);
+
+struct devconf_config_object
+*devconf_get_config_object(struct config_group *group, const char *name);
+
+ssize_t devconf_get_int_attr(struct devconf_config_driver *cdrv,
+			     struct devconf_config_object *cobj,
+			     const char *param, int *value);
+void devconf_device_unregister(struct devconf_config_driver *cd);
+int devconf_device_register(struct devconf_config_driver *new_cd);
+struct devconf_config_driver *devconf_create_config_driver(const char *name);
+void devconf_put_config_driver(struct devconf_config_driver *cdrv);
+
+static inline struct devconf_config_object
+*to_config_obj(struct config_item *item)
+{
+	return item ? container_of(item, struct devconf_config_object,
+				   group.cg_item) : NULL;
+}
+
+static inline struct devconf_config_driver
+*to_config_driver(struct config_item *item)
+{
+	return item ? container_of(item, struct devconf_config_driver,
+				   cobj.group.cg_item) : NULL;
+}
+
+#define DECLARE_DEVCONF_DRIVER(_name, _set_obj, _get_int_attr)		\
+	static struct devconf_config_driver _name ## devconf_driver = {	\
+		.name = __stringify(_name),				\
+		.mod  = THIS_MODULE,					\
+		.set_config_object = _set_obj,				\
+		.get_int_attr = _get_int_attr,				\
+	};								\
+	MODULE_ALIAS("devconf:" __stringify(_name))
+
+#define DECLARE_DEVCONF_DRIVER_INIT(_name, _set_obj, _get_int_attr)	\
+	DECLARE_DEVCONF_DRIVER(_name, _set_obj, _get_int_attr);		\
+	static int __init _name ## mod_init(void)			\
+	{								\
+		return devconf_device_register(&_name ## devconf_driver);\
+	}								\
+	static void __exit _name ## mod_exit(void)			\
+	{								\
+		devconf_device_unregister(&_name ## devconf_driver);	\
+	}								\
+	module_init(_name ## mod_init);					\
+	module_exit(_name ## mod_exit)
+
+#endif
-- 
1.7.8.2

[RFC net-next 3/3] net/mlx4_core: Set port_type value according to configuration module

[Hadar Hen Zion]

port_type_array is a module parameter which sets mlx4_core ports type
under SRIOV.
Since module parameter can't be set per device, few Mellanox NICs using
the same driver, will have to be configured with the same ports type.

Using the new devconf infrastructure allows setting different ports type
per device. In case mlx4_core configuration module is missing, the ports
type of each device will be set as before, according to port_type_array
value.

Signed-off-by: Hadar Hen Zion <hadarh@mellanox.com>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/Kconfig |    1 +
 drivers/net/ethernet/mellanox/mlx4/main.c  |   55 +++++++++++++++++++++++++++-
 drivers/net/ethernet/mellanox/mlx4/mlx4.h  |    1 +
 3 files changed, 56 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/Kconfig b/drivers/net/ethernet/mellanox/mlx4/Kconfig
index fa135d3..ebdfaae 100644
--- a/drivers/net/ethernet/mellanox/mlx4/Kconfig
+++ b/drivers/net/ethernet/mellanox/mlx4/Kconfig
@@ -33,6 +33,7 @@ config MLX4_EN_VXLAN
 config MLX4_CORE
 	tristate
 	depends on PCI
+	select DEVCONF
 	default n
 
 config MLX4_DEBUG
diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
index 943cbd4..c2c1129 100644
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -42,6 +42,7 @@
 #include <linux/io-mapping.h>
 #include <linux/delay.h>
 #include <linux/kmod.h>
+#include <linux/devconf.h>
 
 #include <linux/mlx4/device.h>
 #include <linux/mlx4/doorbell.h>
@@ -55,6 +56,7 @@ MODULE_DESCRIPTION("Mellanox ConnectX HCA low-level driver");
 MODULE_LICENSE("Dual BSD/GPL");
 MODULE_VERSION(DRV_VERSION);
 
+static struct devconf_config_driver *mlx4_conf;
 struct workqueue_struct *mlx4_wq;
 
 #ifdef CONFIG_MLX4_DEBUG
@@ -255,6 +257,36 @@ static void mlx4_enable_cqe_eqe_stride(struct mlx4_dev *dev)
 	}
 }
 
+static int config_port_type(struct mlx4_priv *priv,
+			    int port_num,
+			    int *port_type)
+{
+	struct devconf_config_object *port, *ports;
+	int err;
+	char port_n[sizeof(int)];
+
+	if (!priv->config_obj)
+		return -ENOENT;
+
+	ports = devconf_get_config_object(&priv->config_obj->group, "ports");
+	if (!ports) {
+		pr_err("Fail to get ports configuration\n");
+		return -ENOENT;
+	}
+
+	sprintf(port_n, "%d", port_num);
+	port = devconf_get_config_object(&ports->group, port_n);
+	if (!port)
+		return -ENOENT;
+	err = devconf_get_int_attr(mlx4_conf, port, "type", port_type);
+	if (err < 0) {
+		pr_err("Fail to get port %d type from dev_c_mlx4 configuration module\n",
+		       port_num);
+		return err;
+	}
+	return 0;
+}
+
 static int _mlx4_dev_port(struct mlx4_dev *dev, int port,
 			  struct mlx4_port_cap *port_cap)
 {
@@ -297,6 +329,8 @@ static int mlx4_dev_port(struct mlx4_dev *dev, int port,
 #define MLX4_A0_STEERING_TABLE_SIZE	256
 static int mlx4_dev_cap(struct mlx4_dev *dev, struct mlx4_dev_cap *dev_cap)
 {
+	struct mlx4_priv *priv = mlx4_priv(dev);
+	int port_type;
 	int err;
 	int i;
 
@@ -411,7 +445,11 @@ static int mlx4_dev_cap(struct mlx4_dev *dev, struct mlx4_dev_cap *dev_cap)
 				/* if IB and ETH are supported, we set the port
 				 * type according to user selection of port type;
 				 * if user selected none, take the FW hint */
-				if (port_type_array[i - 1] == MLX4_PORT_TYPE_NONE)
+				err = config_port_type(priv, i, &port_type);
+				if (!err)
+					dev->caps.port_type[i] = port_type;
+				else if (port_type_array[i - 1] ==
+					 MLX4_PORT_TYPE_NONE)
 					dev->caps.port_type[i] = dev->caps.suggested_type[i] ?
 						MLX4_PORT_TYPE_ETH : MLX4_PORT_TYPE_IB;
 				else
@@ -3072,6 +3110,7 @@ static int mlx4_init_one(struct pci_dev *pdev, const struct pci_device_id *id)
 {
 	struct mlx4_priv *priv;
 	struct mlx4_dev *dev;
+	struct devconf_config_object *pdevs;
 	int ret;
 
 	printk_once(KERN_INFO "%s", mlx4_version);
@@ -3085,6 +3124,20 @@ static int mlx4_init_one(struct pci_dev *pdev, const struct pci_device_id *id)
 	pci_set_drvdata(pdev, dev);
 	priv->pci_dev_data = id->driver_data;
 
+	mlx4_conf = devconf_get_config_driver(DRV_NAME);
+	if (IS_ERR(mlx4_conf)) {
+		pr_warn("Configuration driver is missing for "
+			DRV_NAME ", using default vlaues.\n");
+	} else {
+		pdevs = devconf_get_config_object(&mlx4_conf->cobj.group,
+						  "pdevs");
+		if (!pdevs)
+			pr_err("Couldn't find 'pdevs' config object\n");
+		else
+			priv->config_obj =
+				devconf_get_config_object(&pdevs->group,
+							  pci_name(pdev));
+	}
 	ret =  __mlx4_init_one(pdev, id->driver_data, priv);
 	if (ret)
 		kfree(priv);
diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4.h b/drivers/net/ethernet/mellanox/mlx4/mlx4.h
index bdd4eea..81678a5 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mlx4.h
+++ b/drivers/net/ethernet/mellanox/mlx4/mlx4.h
@@ -881,6 +881,7 @@ struct mlx4_priv {
 
 	atomic_t		opreq_count;
 	struct work_struct	opreq_task;
+	struct devconf_config_object	*config_obj;
 };
 
 static inline struct mlx4_priv *mlx4_priv(struct mlx4_dev *dev)
-- 
1.7.8.2

Re: [RFC net-next 0/3] devconf: New infrastructure for setting pre-load parameters for a module

[Greg KH]

On Thu, Jan 08, 2015 at 05:16:57PM +0200, Hadar Hen Zion wrote:
> Hi,
>     
> When configuring a device at an early boot stage, most kernel drivers
> use module parameters (the parameters' settings can be determined in
> modprobe.d config files).

Which is a bad idea, as you have learned :)

> These parameters are difficult to manage, and one of the reasons is that
> module parameters are set per driver and not per device (NICs using the
> same driver cannot be set with different configurations).
> Furthermore, using other existing configuration tools like ethtool,
> ifconfig, ip link commands or sysfs entries is not applicable, since
> they all rely on having a netdev already set up.
> 
> In the past, 'request_firmware' solution for configuration parameters
> was suggested by Shannon Nelson from Intel[1]. The idea was rejected by
> Greg KH, who claimed it was abusive of the request_firmware mechanism.
> Greg suggested using configfs for device configuration instead (as done
> by the USB gadget driver).
> 
> As a solution, we introduce a new kernel infrastructure using configfs
> to allow the configuration of the device. The goal is to set low-level
> device functionality that needs to be sorted out before a module is
> loaded.

Ick, really?  drivers should never need to be configured like this, if
so, then something is wrong, they should "just work" by default.  What
are you needing to "configure" that can't be determined by something
like a device tree?

greg k-h

Re: [RFC net-next 0/3] devconf: New infrastructure for setting pre-load parameters for a module

[Amir Vadai]

On 1/8/2015 6:46 PM, Greg KH wrote:
> On Thu, Jan 08, 2015 at 05:16:57PM +0200, Hadar Hen Zion wrote:
>> Hi,
>>     
>> When configuring a device at an early boot stage, most kernel drivers
>> use module parameters (the parameters' settings can be determined in
>> modprobe.d config files).
> 
> Which is a bad idea, as you have learned :)
> 
>> These parameters are difficult to manage, and one of the reasons is that
>> module parameters are set per driver and not per device (NICs using the
>> same driver cannot be set with different configurations).
>> Furthermore, using other existing configuration tools like ethtool,
>> ifconfig, ip link commands or sysfs entries is not applicable, since
>> they all rely on having a netdev already set up.
>>
>> In the past, 'request_firmware' solution for configuration parameters
>> was suggested by Shannon Nelson from Intel[1]. The idea was rejected by
>> Greg KH, who claimed it was abusive of the request_firmware mechanism.
>> Greg suggested using configfs for device configuration instead (as done
>> by the USB gadget driver).
>>
>> As a solution, we introduce a new kernel infrastructure using configfs
>> to allow the configuration of the device. The goal is to set low-level
>> device functionality that needs to be sorted out before a module is
>> loaded.
> 
> Ick, really?  drivers should never need to be configured like this, if
> so, then something is wrong, they should "just work" by default.  What
> are you needing to "configure" that can't be determined by something
> like a device tree?
Ick indeed - but we can't find anything better.

For example, we have devices that can act as either netdev or as an
Infiniband device.
The driver consists of a core to handle the hardware, and higher layer
drivers - one for Ethernet and one for Infiniband.
Today the selection is done through a module parameter. according to it
the relevant higher level driver is loaded, and the device is
initialized. You don't want to have a default of netdev, and in every
installation that needs Infiniband, a netdev will be created, removed
and only then the Infiniband device will appear.
This is only one example to configuration that needs to be known before
the hardware is initialized, and be persistent across boots.

We can have a 2 stages loading. First load the core, wait for user
input, and only then configure the device and load the right upper layer
driver according to the user input (configfs/sysfs).
Since other vendors seems to need this capability too, we thought it
would be better to make it generic - and this is this what this RFC is
about.

Amir




> 
> greg k-h
> 

Re: [RFC net-next 0/3] devconf: New infrastructure for setting pre-load parameters for a module

[Greg KH]

On Thu, Jan 08, 2015 at 07:11:04PM +0200, Amir Vadai wrote:
> On 1/8/2015 6:46 PM, Greg KH wrote:
> > On Thu, Jan 08, 2015 at 05:16:57PM +0200, Hadar Hen Zion wrote:
> >> Hi,
> >>     
> >> When configuring a device at an early boot stage, most kernel drivers
> >> use module parameters (the parameters' settings can be determined in
> >> modprobe.d config files).
> > 
> > Which is a bad idea, as you have learned :)
> > 
> >> These parameters are difficult to manage, and one of the reasons is that
> >> module parameters are set per driver and not per device (NICs using the
> >> same driver cannot be set with different configurations).
> >> Furthermore, using other existing configuration tools like ethtool,
> >> ifconfig, ip link commands or sysfs entries is not applicable, since
> >> they all rely on having a netdev already set up.
> >>
> >> In the past, 'request_firmware' solution for configuration parameters
> >> was suggested by Shannon Nelson from Intel[1]. The idea was rejected by
> >> Greg KH, who claimed it was abusive of the request_firmware mechanism.
> >> Greg suggested using configfs for device configuration instead (as done
> >> by the USB gadget driver).
> >>
> >> As a solution, we introduce a new kernel infrastructure using configfs
> >> to allow the configuration of the device. The goal is to set low-level
> >> device functionality that needs to be sorted out before a module is
> >> loaded.
> > 
> > Ick, really?  drivers should never need to be configured like this, if
> > so, then something is wrong, they should "just work" by default.  What
> > are you needing to "configure" that can't be determined by something
> > like a device tree?
> Ick indeed - but we can't find anything better.
> 
> For example, we have devices that can act as either netdev or as an
> Infiniband device.
> The driver consists of a core to handle the hardware, and higher layer
> drivers - one for Ethernet and one for Infiniband.
> Today the selection is done through a module parameter. according to it
> the relevant higher level driver is loaded, and the device is
> initialized. You don't want to have a default of netdev, and in every
> installation that needs Infiniband, a netdev will be created, removed
> and only then the Infiniband device will appear.

But that really isn't an issue, right?  Who cares if it happens that
way?

Or just don't do anything until that "higher level" driver is loaded,
either ib or network.

> This is only one example to configuration that needs to be known before
> the hardware is initialized, and be persistent across boots.
> 
> We can have a 2 stages loading. First load the core, wait for user
> input, and only then configure the device and load the right upper layer
> driver according to the user input (configfs/sysfs).

Why not just wait for an "upper layer" driver to be loaded?  It doesn't
make it dynamic, and forces you to have a module manually be loaded, but
would solve this problem.

Or just don't do anything until configfs is set up for your devices,
after the module is loaded.  This allows drivers to be built into the
kernel if people really want that type of configuration to work
properly.

> Since other vendors seems to need this capability too, we thought it
> would be better to make it generic - and this is this what this RFC is
> about.

What other vendors have such horribly designed hardware / kernel modules
that need this type of thing?  :)

thanks,

greg k-h

Re: [RFC net-next 0/3] devconf: New infrastructure for setting pre-load parameters for a module

[Amir Vadai]

On Thu, Jan 8, 2015 at 7:47 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Thu, Jan 08, 2015 at 07:11:04PM +0200, Amir Vadai wrote:
>> On 1/8/2015 6:46 PM, Greg KH wrote:
>> > On Thu, Jan 08, 2015 at 05:16:57PM +0200, Hadar Hen Zion wrote:
>> >> Hi,
>> >>
>> >> When configuring a device at an early boot stage, most kernel drivers
>> >> use module parameters (the parameters' settings can be determined in
>> >> modprobe.d config files).
>> >
>> > Which is a bad idea, as you have learned :)
>> >
>> >> These parameters are difficult to manage, and one of the reasons is that
>> >> module parameters are set per driver and not per device (NICs using the
>> >> same driver cannot be set with different configurations).
>> >> Furthermore, using other existing configuration tools like ethtool,
>> >> ifconfig, ip link commands or sysfs entries is not applicable, since
>> >> they all rely on having a netdev already set up.
>> >>
>> >> In the past, 'request_firmware' solution for configuration parameters
>> >> was suggested by Shannon Nelson from Intel[1]. The idea was rejected by
>> >> Greg KH, who claimed it was abusive of the request_firmware mechanism.
>> >> Greg suggested using configfs for device configuration instead (as done
>> >> by the USB gadget driver).
>> >>
>> >> As a solution, we introduce a new kernel infrastructure using configfs
>> >> to allow the configuration of the device. The goal is to set low-level
>> >> device functionality that needs to be sorted out before a module is
>> >> loaded.
>> >
>> > Ick, really?  drivers should never need to be configured like this, if
>> > so, then something is wrong, they should "just work" by default.  What
>> > are you needing to "configure" that can't be determined by something
>> > like a device tree?
>> Ick indeed - but we can't find anything better.
>>
>> For example, we have devices that can act as either netdev or as an
>> Infiniband device.
>> The driver consists of a core to handle the hardware, and higher layer
>> drivers - one for Ethernet and one for Infiniband.
>> Today the selection is done through a module parameter. according to it
>> the relevant higher level driver is loaded, and the device is
>> initialized. You don't want to have a default of netdev, and in every
>> installation that needs Infiniband, a netdev will be created, removed
>> and only then the Infiniband device will appear.
>
> But that really isn't an issue, right?  Who cares if it happens that
> way?
>
> Or just don't do anything until that "higher level" driver is loaded,
> either ib or network.
>
>> This is only one example to configuration that needs to be known before
>> the hardware is initialized, and be persistent across boots.
>>
>> We can have a 2 stages loading. First load the core, wait for user
>> input, and only then configure the device and load the right upper layer
>> driver according to the user input (configfs/sysfs).
>
> Why not just wait for an "upper layer" driver to be loaded?  It doesn't
> make it dynamic, and forces you to have a module manually be loaded, but
> would solve this problem.
>
> Or just don't do anything until configfs is set up for your devices,
> after the module is loaded.  This allows drivers to be built into the
> kernel if people really want that type of configuration to work
> properly.
Yes this is the idea in this RFC, and we're suggesting a mechanism to
make it general, so other vendors that would like to have a persistent
state for their devices could do it.

>
>> Since other vendors seems to need this capability too, we thought it
>> would be better to make it generic - and this is this what this RFC is
>> about.
>
> What other vendors have such horribly designed hardware / kernel modules
> that need this type of thing?  :)
Intel [1], Atheros [2]. I can dig in the mailing list for some more.

[1] - https://lkml.org/lkml/2013/1/10/606
[2] - http://lists.openwall.net/netdev/2014/06/16/53

I agree that whenever possible the hardware should have a good default
state. But sometimes the user has some useful information that could
be used during initialization and make driver loading process much
more efficient and clean.

Let me know if you prefer us to contribute a generic solution, or to
have a configfs with 2 stages loading for our driver only. I thought
that a generic solution is better, but would be happy to hear what
other vendors and experienced kernel developers have to say.

Thanks,
Amir

>
> thanks,
>
> greg k-h
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC net-next 0/3] devconf: New infrastructure for setting pre-load parameters for a module

[Greg KH]

On Thu, Jan 08, 2015 at 09:14:32PM +0200, Amir Vadai wrote:
> On Thu, Jan 8, 2015 at 7:47 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Thu, Jan 08, 2015 at 07:11:04PM +0200, Amir Vadai wrote:
> >> On 1/8/2015 6:46 PM, Greg KH wrote:
> >> > On Thu, Jan 08, 2015 at 05:16:57PM +0200, Hadar Hen Zion wrote:
> >> >> Hi,
> >> >>
> >> >> When configuring a device at an early boot stage, most kernel drivers
> >> >> use module parameters (the parameters' settings can be determined in
> >> >> modprobe.d config files).
> >> >
> >> > Which is a bad idea, as you have learned :)
> >> >
> >> >> These parameters are difficult to manage, and one of the reasons is that
> >> >> module parameters are set per driver and not per device (NICs using the
> >> >> same driver cannot be set with different configurations).
> >> >> Furthermore, using other existing configuration tools like ethtool,
> >> >> ifconfig, ip link commands or sysfs entries is not applicable, since
> >> >> they all rely on having a netdev already set up.
> >> >>
> >> >> In the past, 'request_firmware' solution for configuration parameters
> >> >> was suggested by Shannon Nelson from Intel[1]. The idea was rejected by
> >> >> Greg KH, who claimed it was abusive of the request_firmware mechanism.
> >> >> Greg suggested using configfs for device configuration instead (as done
> >> >> by the USB gadget driver).
> >> >>
> >> >> As a solution, we introduce a new kernel infrastructure using configfs
> >> >> to allow the configuration of the device. The goal is to set low-level
> >> >> device functionality that needs to be sorted out before a module is
> >> >> loaded.
> >> >
> >> > Ick, really?  drivers should never need to be configured like this, if
> >> > so, then something is wrong, they should "just work" by default.  What
> >> > are you needing to "configure" that can't be determined by something
> >> > like a device tree?
> >> Ick indeed - but we can't find anything better.
> >>
> >> For example, we have devices that can act as either netdev or as an
> >> Infiniband device.
> >> The driver consists of a core to handle the hardware, and higher layer
> >> drivers - one for Ethernet and one for Infiniband.
> >> Today the selection is done through a module parameter. according to it
> >> the relevant higher level driver is loaded, and the device is
> >> initialized. You don't want to have a default of netdev, and in every
> >> installation that needs Infiniband, a netdev will be created, removed
> >> and only then the Infiniband device will appear.
> >
> > But that really isn't an issue, right?  Who cares if it happens that
> > way?
> >
> > Or just don't do anything until that "higher level" driver is loaded,
> > either ib or network.
> >
> >> This is only one example to configuration that needs to be known before
> >> the hardware is initialized, and be persistent across boots.
> >>
> >> We can have a 2 stages loading. First load the core, wait for user
> >> input, and only then configure the device and load the right upper layer
> >> driver according to the user input (configfs/sysfs).
> >
> > Why not just wait for an "upper layer" driver to be loaded?  It doesn't
> > make it dynamic, and forces you to have a module manually be loaded, but
> > would solve this problem.
> >
> > Or just don't do anything until configfs is set up for your devices,
> > after the module is loaded.  This allows drivers to be built into the
> > kernel if people really want that type of configuration to work
> > properly.
> Yes this is the idea in this RFC, and we're suggesting a mechanism to
> make it general, so other vendors that would like to have a persistent
> state for their devices could do it.

I don't think _any_ devices should be doing this, so please don't make
it "generic".  Just use configfs in your module and document it
properly, like other modules that use configfs.

> >> Since other vendors seems to need this capability too, we thought it
> >> would be better to make it generic - and this is this what this RFC is
> >> about.
> >
> > What other vendors have such horribly designed hardware / kernel modules
> > that need this type of thing?  :)
> Intel [1], Atheros [2]. I can dig in the mailing list for some more.
> 
> [1] - https://lkml.org/lkml/2013/1/10/606
> [2] - http://lists.openwall.net/netdev/2014/06/16/53
> 
> I agree that whenever possible the hardware should have a good default
> state. But sometimes the user has some useful information that could
> be used during initialization and make driver loading process much
> more efficient and clean.

But you are talking about loading config info before the driver is
loaded, which goes backwards for how we automatically load modules when
the hardware is found.

> Let me know if you prefer us to contribute a generic solution, or to
> have a configfs with 2 stages loading for our driver only. I thought
> that a generic solution is better, but would be happy to hear what
> other vendors and experienced kernel developers have to say.

A generic solution would be nice, but so far I just see network specific
drivers that can be configured after the driver is loaded, and so should
use the network specific configuration tools like everyone else.

So please just have the module default to a network device and then
configure things from there, after you are loaded, if you need it and
want to do something "special".  Otherwise you are moving backwards for
how autoload drivers work and that is not good.

thanks,

greg k-h

Re: [RFC net-next 0/3] devconf: New infrastructure for setting pre-load parameters for a module

[Amir Vadai]

On Thu, Jan 8, 2015 at 9:25 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Thu, Jan 08, 2015 at 09:14:32PM +0200, Amir Vadai wrote:
>> On Thu, Jan 8, 2015 at 7:47 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
>> > On Thu, Jan 08, 2015 at 07:11:04PM +0200, Amir Vadai wrote:
>> >> On 1/8/2015 6:46 PM, Greg KH wrote:
>> >> > On Thu, Jan 08, 2015 at 05:16:57PM +0200, Hadar Hen Zion wrote:
>> >> >> Hi,
>> >> >>
>> >> >> When configuring a device at an early boot stage, most kernel drivers
>> >> >> use module parameters (the parameters' settings can be determined in
>> >> >> modprobe.d config files).
>> >> >
>> >> > Which is a bad idea, as you have learned :)

[...]

>
> But you are talking about loading config info before the driver is
> loaded, which goes backwards for how we automatically load modules when
> the hardware is found.

I guess you're a busy man and getting tired of my nudges. And it is ok
with us to make it a non-generic solution - so I won't bother you much
more...
I would like to understand the arguments here: Maybe there is a
history that I missed - What are the arguments against having a config
info before the driver is loaded? Assuming, that it will make the
driver loading process simpler and faster.

Thanks,
Amir

Re: [RFC net-next 0/3] devconf: New infrastructure for setting pre-load parameters for a module

[Greg KH]

On Fri, Jan 09, 2015 at 12:28:21AM +0200, Amir Vadai wrote:
> On Thu, Jan 8, 2015 at 9:25 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Thu, Jan 08, 2015 at 09:14:32PM +0200, Amir Vadai wrote:
> >> On Thu, Jan 8, 2015 at 7:47 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> >> > On Thu, Jan 08, 2015 at 07:11:04PM +0200, Amir Vadai wrote:
> >> >> On 1/8/2015 6:46 PM, Greg KH wrote:
> >> >> > On Thu, Jan 08, 2015 at 05:16:57PM +0200, Hadar Hen Zion wrote:
> >> >> >> Hi,
> >> >> >>
> >> >> >> When configuring a device at an early boot stage, most kernel drivers
> >> >> >> use module parameters (the parameters' settings can be determined in
> >> >> >> modprobe.d config files).
> >> >> >
> >> >> > Which is a bad idea, as you have learned :)
> 
> [...]
> 
> >
> > But you are talking about loading config info before the driver is
> > loaded, which goes backwards for how we automatically load modules when
> > the hardware is found.
> 
> I guess you're a busy man and getting tired of my nudges. And it is ok
> with us to make it a non-generic solution - so I won't bother you much
> more...
> I would like to understand the arguments here: Maybe there is a
> history that I missed - What are the arguments against having a config
> info before the driver is loaded? Assuming, that it will make the
> driver loading process simpler and faster.

How can you load any configuration before the module is loaded if your
module is loaded automatically during the initrd?

drivers are loaded on demand, when the hardware is found, don't force
people to configure things manually by running a tool before your module
is loaded, as that is not how the module load process works at all.

greg k-h

Re: [RFC net-next 0/3] devconf: New infrastructure for setting pre-load parameters for a module

[Rob Herring]

Amir Vadai <amirv <at> mellanox.com> writes:
> On Thu, Jan 8, 2015 at 9:25 PM, Greg KH <gregkh <at> linuxfoundation.org> 
wrote:
> > On Thu, Jan 08, 2015 at 09:14:32PM +0200, Amir Vadai wrote:
> >> On Thu, Jan 8, 2015 at 7:47 PM, Greg KH <gregkh <at> 
linuxfoundation.org> wrote:
> >> > On Thu, Jan 08, 2015 at 07:11:04PM +0200, Amir Vadai wrote:
> >> >> On 1/8/2015 6:46 PM, Greg KH wrote:
> >> >> > On Thu, Jan 08, 2015 at 05:16:57PM +0200, Hadar Hen Zion wrote:
> >> >> >> Hi,
> >> >> >>
> >> >> >> When configuring a device at an early boot stage, most kernel 
drivers
> >> >> >> use module parameters (the parameters' settings can be determined 
in
> >> >> >> modprobe.d config files).
> >> >> >
> >> >> > Which is a bad idea, as you have learned :)
> 
> [...]
> 
> >
> > But you are talking about loading config info before the driver is
> > loaded, which goes backwards for how we automatically load modules when
> > the hardware is found.
> 
> I guess you're a busy man and getting tired of my nudges. And it is ok
> with us to make it a non-generic solution - so I won't bother you much
> more...
> I would like to understand the arguments here: Maybe there is a
> history that I missed - What are the arguments against having a config
> info before the driver is loaded? Assuming, that it will make the
> driver loading process simpler and faster.

DeviceTree does exactly this. Even discoverable buses sometimes need 
additional information. DT can be used to add additional properties to PCI 
devices for example. I'm not sure DT is a fit here. As a matter of policy to 
be OS independent, the configuration data should be tied to the hardware and 
not purely software configuration. The example of netdev vs. infiniband would 
fit, but I don't know about other cases. Traditionally, DT would be part of 
the firmware/bootloader rather than coupled to the kernel or distro. It is 
not clear to me in your case who does the configuration and when does it 
change. There is new support making its way into the kernel called DT 
overlays which allows loading additional DT data after or during boot. That 
may be a good fit for your use case.

Rob