[PATCH net-next V1 0/3] net/mlx4_core: Allow setting init-time device specific parameters

[PATCH net-next V1 0/3] net/mlx4_core: Allow setting init-time device specific parameters

[Amir Vadai]

Hi Dave,

This patchset adds support to enabling configuring _ConnectX-3 specific_
hardware tunables. Hadar took the RFC [1] (the V0) and adapted it according to
Greg K-H comments. And also according to previous similar discussions with
Intel [1] and [2]. I understand and agree with your approach that user
shouldn't care which hardware is at the bottom of the network stack. but, we're
talking here on an HCA that could be both Ethernet and RDMA device - so, the
usual networking API's might not suitable for all its tunables. Also, customers
are paying for a very sophisticated piece of hardware, and we would like to
enable power user to tweak it in some situtations. Of course the default mode
should be used in 99% of the use cases.

Below is Hadar text that gets into the details.

Thanks,
Amir.


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 netdevice 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).

We introduce a new mlx4_core infrastructure using configfs to allow the
configuration of mlx4 device. The goal is to set low-level device
functionality which is specific to Mellanox ConnectX device
hardware/firmware, needs to be sorted when the device is initialized and
before the upper layer protocol (Ethernet or Infiniband) is set.

Lately Intel suggested to configure NPAR BW configuration through
configfs[2]. The idea was rejected since those are networking
parameters that could be used in the future by other devices or vendors.

This patchset is a continuation to the RFC already sent by us[3].
We follow Greg K-H suggestion and implemented configfs for mlx4_core use
only.

In this patchset we export to configfs 2 parameters dmfs mode and port
type. Both parameters must be set at initialization stage since port
type determine which upper layer protocol to use and dmfs mode must be
set before Ethernet or IPoIB interfaces are up. Until now, the user was
able to change dmfs mode and port type (in SR-IOV) only through module
parameter, before mlx4_core was loaded. Using configfs allows setting
those parameters after mlx4_core is loaded and also set different
configuration per pci device. In the future, new low-level and specific
device configurations for mlx4_core will be configured through configfs.

Hadar.

Changes from V0:
- Remove devconf generic module
- Move configfs configuration tree from drivers/devconf to mlx4
private location.

[1] - https://lkml.org/lkml/2013/1/10/606
[2] - https://lkml.org/lkml/2015/3/3/724
[3] - http://marc.info/?l=linux-netdev&m=142074512422335&w=2

Signed-off-by: Hadar Hen Zion <hadarh@mellanox.com>
Signed-off-by: Amir Vadai <amirv@mellanox.com>

Hadar Hen Zion (3):
  net/mlx4_core: Add configfs entries for setting device specific
    parameters
  net/mlx4_core: Set port_type value according to configfs configuration
  net/mlx4_core: Set DMFS mode according to configfs configuration

 drivers/net/ethernet/mellanox/mlx4/Kconfig  |   8 +
 drivers/net/ethernet/mellanox/mlx4/Makefile |   2 +-
 drivers/net/ethernet/mellanox/mlx4/catas.c  |   2 +-
 drivers/net/ethernet/mellanox/mlx4/conf.c   | 532 ++++++++++++++++++++++++++++
 drivers/net/ethernet/mellanox/mlx4/main.c   |  76 +++-
 drivers/net/ethernet/mellanox/mlx4/mlx4.h   |  47 ++-
 include/linux/mlx4/device.h                 |  12 +
 7 files changed, 659 insertions(+), 20 deletions(-)
 create mode 100644 drivers/net/ethernet/mellanox/mlx4/conf.c

-- 
1.9.3

[PATCH net-next V1 1/3] net/mlx4_core: Add configfs entries for setting device specific parameters

[Amir Vadai]

From: Hadar Hen Zion <hadarh@mellanox.com>

Introducing a new mlx4_core infrastructure using the configfs to allow
set and commit specific mlx4_core low-level device configurations.

The initialization in this code creates "mlx4_core" entry in the
configfs system. After that, the user needs to use mkdir to create
configurations for specific pci device; for example "mkdir 0000:04:00.0".
This code will verify that such device exists and that
it is owned by mlx4_core.

Configfs Sub-tree Example:
-------------------------
$ tree  /sys/kernel/config/mlx4_core/
/sys/kernel/config/mlx4_core/
├── 0000:04:00.0
│   ├── commit
│   ├── dmfs_mode
│   └── ports
│       ├── 1
│       │   └── type
│       └── 2
│           └── type
└── 0000:24:00.0
    ├── commit
    ├── dmfs_mode
    └── ports
    	├── 1
    	│   └── type
    	└── 2
    	    └── type

In this patch we export the 'commit' control, a control that accepts
only a value of 1 and triggers a device restart that causes the
settings to be permanently committed.

In the next coming patches we'll export low-level device
controls of mlx4_core that will be configured through the configfs.

Signed-off-by: Hadar Hen Zion <hadarh@mellanox.com>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/Kconfig  |   8 +
 drivers/net/ethernet/mellanox/mlx4/Makefile |   2 +-
 drivers/net/ethernet/mellanox/mlx4/catas.c  |   2 +-
 drivers/net/ethernet/mellanox/mlx4/conf.c   | 283 ++++++++++++++++++++++++++++
 drivers/net/ethernet/mellanox/mlx4/main.c   |  40 +++-
 drivers/net/ethernet/mellanox/mlx4/mlx4.h   |  23 ++-
 6 files changed, 345 insertions(+), 13 deletions(-)
 create mode 100644 drivers/net/ethernet/mellanox/mlx4/conf.c

diff --git a/drivers/net/ethernet/mellanox/mlx4/Kconfig b/drivers/net/ethernet/mellanox/mlx4/Kconfig
index 1486ce9..16eb897 100644
--- a/drivers/net/ethernet/mellanox/mlx4/Kconfig
+++ b/drivers/net/ethernet/mellanox/mlx4/Kconfig
@@ -35,6 +35,14 @@ config MLX4_CORE
 	depends on PCI
 	default n
 
+config MLX4_CONFIGFS_FS
+	bool "Config File System Support (configfs)"
+	default y
+	depends on MLX4_CORE && CONFIGFS_FS && !(MLX4_CORE=y && CONFIGFS_FS=m)
+	---help---
+	Provides support for the configfs file system for driver configuration.
+
+
 config MLX4_DEBUG
 	bool "Verbose debugging output" if (MLX4_CORE && EXPERT)
 	depends on MLX4_CORE
diff --git a/drivers/net/ethernet/mellanox/mlx4/Makefile b/drivers/net/ethernet/mellanox/mlx4/Makefile
index 3e9c70f..1995873 100644
--- a/drivers/net/ethernet/mellanox/mlx4/Makefile
+++ b/drivers/net/ethernet/mellanox/mlx4/Makefile
@@ -1,7 +1,7 @@
 obj-$(CONFIG_MLX4_CORE)		+= mlx4_core.o
 
 mlx4_core-y :=	alloc.o catas.o cmd.o cq.o eq.o fw.o icm.o intf.o main.o mcg.o \
-		mr.o pd.o port.o profile.o qp.o reset.o sense.o srq.o resource_tracker.o
+		mr.o pd.o port.o profile.o qp.o reset.o sense.o srq.o resource_tracker.o conf.o
 
 obj-$(CONFIG_MLX4_EN)               += mlx4_en.o
 
diff --git a/drivers/net/ethernet/mellanox/mlx4/catas.c b/drivers/net/ethernet/mellanox/mlx4/catas.c
index 715de8a..e0ceb29 100644
--- a/drivers/net/ethernet/mellanox/mlx4/catas.c
+++ b/drivers/net/ethernet/mellanox/mlx4/catas.c
@@ -205,7 +205,7 @@ static void mlx4_handle_error_state(struct mlx4_dev_persistent *persist)
 	mutex_lock(&persist->interface_state_mutex);
 	if (persist->interface_state & MLX4_INTERFACE_STATE_UP &&
 	    !(persist->interface_state & MLX4_INTERFACE_STATE_DELETION)) {
-		err = mlx4_restart_one(persist->pdev);
+		err = mlx4_restart_one(persist->pdev, 1);
 		mlx4_info(persist->dev, "mlx4_restart_one was ended, ret=%d\n",
 			  err);
 	}
diff --git a/drivers/net/ethernet/mellanox/mlx4/conf.c b/drivers/net/ethernet/mellanox/mlx4/conf.c
new file mode 100644
index 0000000..abbe18a
--- /dev/null
+++ b/drivers/net/ethernet/mellanox/mlx4/conf.c
@@ -0,0 +1,283 @@
+/*
+ * 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/pci.h>
+#include <linux/string.h>
+
+#include <linux/configfs.h>
+#include <linux/mlx4/device.h>
+
+#include "mlx4.h"
+
+#if IS_ENABLED(CONFIG_MLX4_CONFIGFS_FS)
+
+/**
+ * configfs entries for mlx4_core
+ *
+ * This file adds code for configfs support for mlx4_core driver. This sets
+ * up a filesystem under /sys/kernel/config in which configuration changes
+ * can be made for the driver's pci devices.
+ *
+ * The initialization of this code creates the "mlx4_core" entry in the configfs
+ * system.  After that, the user needs to use mkdir to create configurations
+ * for specific pci device; for example "mkdir 0000:04:00.0". This code will
+ * verify that such a device exists and that it is owned by mlx4_core.
+ *
+ **/
+
+#define MLX4_CFG_CONFIGFS_ATTR(struct_name, name)			\
+	static struct struct_name##_attribute struct_name##_##name =	\
+		__CONFIGFS_ATTR(name,  S_IRUGO | S_IWUSR,		\
+				struct_name##_##name##_show,		\
+				struct_name##_##name##_store)
+
+/* Defines item_ops for 'struct_in'
+ */
+#define MLX4_CONFIG_ITEM_TYPE(struct_in)				\
+									\
+static struct configfs_item_operations struct_in##_item_ops = {		\
+	.release                = struct_in##_release,			\
+	.show_attribute         = struct_in##_attr_show,		\
+	.store_attribute        = struct_in##_attr_store,		\
+};									\
+									\
+static struct config_item_type struct_in##_item_type = {		\
+	.ct_item_ops	= &struct_in##_item_ops,			\
+	.ct_attrs	= struct_in##_attrs,				\
+	.ct_owner	= THIS_MODULE,					\
+}
+
+static ssize_t pdev_config_commit_store(struct pdev_config *cfg,
+					const char *page, size_t len)
+{
+	int err;
+	unsigned long res;
+	int active_vfs = 0;
+	struct mlx4_dev_persistent *persist = pci_get_drvdata(cfg->pdev);
+	struct mlx4_dev *dev = persist->dev;
+	char *pdev_name = cfg->group.cg_item.ci_name;
+
+	err = kstrtoul(page, 10, &res);
+	if (err)
+		return err;
+
+	if ((res != 1) && (res != 0)) {
+		pr_err("mlx4_core %s: Illegal value for commit: %lu, can't apply configurations.\n",
+		       pdev_name, res);
+		return -EINVAL;
+	}
+	if (res) {
+		if (mlx4_is_master(dev) && dev->flags & MLX4_FLAG_SRIOV)
+			active_vfs = mlx4_how_many_lives_vf(dev);
+		if (active_vfs) {
+			pr_warn("Can't restart device %s, unload active VFs before committing your changes.\n",
+				pdev_name);
+			return -EINVAL;
+
+		} else {
+			pr_warn("Restart device %s and allow setting of pre-load configurations.\n",
+				pdev_name);
+			err = mlx4_restart_one(cfg->pdev, 0);
+		}
+	}
+	cfg->commit = res;
+
+	return len;
+}
+
+static ssize_t pdev_config_commit_show(struct pdev_config *pdev_cfg, char *page)
+{
+		return sprintf(page, "%d\n", pdev_cfg->commit);
+}
+
+CONFIGFS_ATTR_STRUCT(pdev_config);
+CONFIGFS_ATTR_OPS(pdev_config);
+MLX4_CFG_CONFIGFS_ATTR(pdev_config, commit);
+
+static void pdev_config_release(struct config_item *item)
+{
+	struct pdev_config *pdev_cfg = to_pdev_config(item);
+
+	pci_dev_put(pdev_cfg->pdev);
+	kfree(pdev_cfg);
+}
+
+static struct configfs_attribute *pdev_config_attrs[] = {
+	&pdev_config_commit.attr,
+	NULL,
+};
+
+MLX4_CONFIG_ITEM_TYPE(pdev_config);
+
+static struct pci_dev *find_pdev_by_name(const char *name)
+{
+	struct pci_dev *pdev;
+	char *pdev_name;
+	char *tmp_p;
+	unsigned long int domain;
+	unsigned long int bus;
+	unsigned long int dev;
+	unsigned long int func;
+	int err;
+
+	tmp_p = kzalloc(sizeof(*name), GFP_KERNEL);
+	if (!tmp_p)
+		return ERR_PTR(-ENOMEM);
+	strcpy(tmp_p, name);
+	pdev_name = tmp_p;
+
+	err = kstrtoul(strsep(&pdev_name, ":"), 16, &domain);
+	if (err)
+		goto format_err;
+
+	err = kstrtoul(strsep(&pdev_name, ":"), 16, &bus);
+	if (err)
+		goto format_err;
+
+	err = kstrtoul(strsep(&pdev_name, "."), 16, &dev);
+	if (err)
+		goto format_err;
+
+	err = kstrtoul(pdev_name, 16, &func);
+	if (err)
+		goto format_err;
+
+	pdev = pci_get_domain_bus_and_slot(domain, bus, (dev << 3) | func);
+	if (!pdev) {
+		pr_err("mlx4_core: Couldn't find pci device: %s\n", name);
+		err = -EINVAL;
+		goto out_err;
+	}
+	if (pdev->is_virtfn) {
+		pr_err("mlx4_core: Couldn't set configuration for a virtual function. bdf name %s\n",
+		       name);
+		err = -EINVAL;
+		pci_dev_put(pdev);
+		goto out_err;
+	}
+	if (strcmp(pdev->driver->name, DRV_NAME)) {
+		pr_err("mlx4_core: pci device %s is not mlx4 device. Can't set configurations.\n",
+		       name);
+		err = -EINVAL;
+		pci_dev_put(pdev);
+		goto out_err;
+	}
+
+	kfree(tmp_p);
+	return pdev;
+
+format_err:
+	pr_err("mlx4_core: Wrong pci device format: %s, use: wwww:xx:yy.x, domain:bus:device.function\n",
+	       name);
+out_err:
+	kfree(tmp_p);
+	return ERR_PTR(err);
+}
+
+static struct config_group *mlx4_set_config(struct config_group *group,
+					    const char *name)
+{
+	struct pdev_config *pdev_config;
+	struct pci_dev *pdev;
+	int err;
+
+	pdev = find_pdev_by_name(name);
+	if (IS_ERR(pdev))
+		return ERR_PTR(PTR_ERR(pdev));
+
+	pdev_config = kzalloc(sizeof(*pdev_config), GFP_KERNEL);
+	if (!pdev_config) {
+		err = -ENOMEM;
+		goto out_err;
+	}
+
+	config_group_init_type_name(&pdev_config->group, name,
+				    &pdev_config_item_type);
+	pdev_config->pdev = pdev;
+	return &pdev_config->group;
+
+out_err:
+	pci_dev_put(pdev);
+	return ERR_PTR(err);
+}
+
+static struct config_group *device_driver_make(struct config_group *group,
+					       const char *name)
+{
+	return mlx4_set_config(group, name);
+}
+
+static struct configfs_group_operations mlx4_group_ops = {
+	.make_group	= device_driver_make,
+};
+
+static struct config_item_type mlx4_type = {
+	.ct_group_ops	= &mlx4_group_ops,
+	.ct_owner	= THIS_MODULE,
+};
+
+static struct configfs_subsystem mlx4_subsys = {
+	.su_group = {
+		.cg_item = {
+			.ci_namebuf = DRV_NAME,
+			.ci_type = &mlx4_type,
+		},
+	},
+};
+
+int mlx4_configfs_init(void)
+{
+	int ret;
+	struct configfs_subsystem *subsys = &mlx4_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);
+		return ret;
+	}
+	return 0;
+}
+
+void mlx4_configfs_exit(void)
+{
+	configfs_unregister_subsystem(&mlx4_subsys);
+}
+
+MODULE_LICENSE("GPL");
+
+#endif /* IS_ENABLED(CONFIG_MLX4_CONFIGFS_FS) */
diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
index 7e48722..c81217e 100644
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -613,7 +613,7 @@ static void mlx4_check_pcie_caps(struct mlx4_dev *dev)
 }
 
 /*The function checks if there are live vf, return the num of them*/
-static int mlx4_how_many_lives_vf(struct mlx4_dev *dev)
+int mlx4_how_many_lives_vf(struct mlx4_dev *dev)
 {
 	struct mlx4_priv *priv = mlx4_priv(dev);
 	struct mlx4_slave_state *s_state;
@@ -3449,7 +3449,7 @@ static int restore_current_port_types(struct mlx4_dev *dev,
 	return err;
 }
 
-int mlx4_restart_one(struct pci_dev *pdev)
+int mlx4_restart_one(struct pci_dev *pdev, int restore)
 {
 	struct mlx4_dev_persistent *persist = pci_get_drvdata(pdev);
 	struct mlx4_dev	 *dev  = persist->dev;
@@ -3469,12 +3469,14 @@ int mlx4_restart_one(struct pci_dev *pdev)
 		return err;
 	}
 
-	err = restore_current_port_types(dev, dev->persist->curr_port_type,
-					 dev->persist->curr_port_poss_type);
-	if (err)
-		mlx4_err(dev, "could not restore original port types (%d)\n",
-			 err);
-
+	if (restore) {
+		err = restore_current_port_types(dev,
+						 dev->persist->curr_port_type,
+						 dev->persist->curr_port_poss_type);
+		if (err)
+			mlx4_err(dev, "could not restore original port types (%d)\n",
+				 err);
+	}
 	return err;
 }
 
@@ -3668,15 +3670,33 @@ static int __init mlx4_init(void)
 	if (!mlx4_wq)
 		return -ENOMEM;
 
+#if IS_ENABLED(CONFIG_MLX4_CONFIGFS_FS)
+	ret = mlx4_configfs_init();
+	if (ret)
+		goto out_wq;
+#endif
+
 	ret = pci_register_driver(&mlx4_driver);
 	if (ret < 0)
-		destroy_workqueue(mlx4_wq);
-	return ret < 0 ? ret : 0;
+		goto out_unregister;
+
+	return 0;
+
+out_unregister:
+#if IS_ENABLED(CONFIG_MLX4_CONFIGFS_FS)
+	mlx4_configfs_exit();
+out_wq:
+#endif
+	destroy_workqueue(mlx4_wq);
+	return ret;
 }
 
 static void __exit mlx4_cleanup(void)
 {
 	pci_unregister_driver(&mlx4_driver);
+#if IS_ENABLED(CONFIG_MLX4_CONFIGFS_FS)
+	mlx4_configfs_exit();
+#endif
 	destroy_workqueue(mlx4_wq);
 }
 
diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4.h b/drivers/net/ethernet/mellanox/mlx4/mlx4.h
index 0b16db0..ee43c0a 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mlx4.h
+++ b/drivers/net/ethernet/mellanox/mlx4/mlx4.h
@@ -45,6 +45,7 @@
 #include <linux/workqueue.h>
 #include <linux/interrupt.h>
 #include <linux/spinlock.h>
+#include <linux/configfs.h>
 
 #include <linux/mlx4/device.h>
 #include <linux/mlx4/driver.h>
@@ -903,6 +904,8 @@ static inline struct mlx4_priv *mlx4_priv(struct mlx4_dev *dev)
 
 extern struct workqueue_struct *mlx4_wq;
 
+int mlx4_how_many_lives_vf(struct mlx4_dev *dev);
+
 u32 mlx4_bitmap_alloc(struct mlx4_bitmap *bitmap);
 void mlx4_bitmap_free(struct mlx4_bitmap *bitmap, u32 obj, int use_rr);
 u32 mlx4_bitmap_alloc_range(struct mlx4_bitmap *bitmap, int cnt,
@@ -1007,7 +1010,7 @@ void mlx4_start_catas_poll(struct mlx4_dev *dev);
 void mlx4_stop_catas_poll(struct mlx4_dev *dev);
 int mlx4_catas_init(struct mlx4_dev *dev);
 void mlx4_catas_end(struct mlx4_dev *dev);
-int mlx4_restart_one(struct pci_dev *pdev);
+int mlx4_restart_one(struct pci_dev *pdev, int restore);
 int mlx4_register_device(struct mlx4_dev *dev);
 void mlx4_unregister_device(struct mlx4_dev *dev);
 void mlx4_dispatch_event(struct mlx4_dev *dev, enum mlx4_dev_event type,
@@ -1437,4 +1440,22 @@ u32 mlx4_zone_free_entries_unique(struct mlx4_zone_allocator *zones, u32 obj, u3
 /* Returns a pointer to mlx4_bitmap that was attached to <zones> with <uid> */
 struct mlx4_bitmap *mlx4_zone_get_bitmap(struct mlx4_zone_allocator *zones, u32 uid);
 
+#if IS_ENABLED(CONFIG_MLX4_CONFIGFS_FS)
+struct pdev_config {
+	struct config_group group;
+	struct pci_dev *pdev;
+	int commit;
+};
+
+static inline struct pdev_config *to_pdev_config(struct config_item *item)
+{
+	return item ? container_of(to_config_group(item),
+				   struct pdev_config, group) : NULL;
+}
+
+int mlx4_configfs_init(void);
+void mlx4_configfs_exit(void);
+int mlx4_conf_get_config(struct mlx4_dev *dev, struct pci_dev *pdev);
+#endif /* CONFIG_MLX4_CONFIGFS_FS */
+
 #endif /* MLX4_H */
-- 
1.9.3

[PATCH net-next V1 2/3] net/mlx4_core: Set port_type value according to configfs configuration

[Amir Vadai]

From: Hadar Hen Zion <hadarh@mellanox.com>

port_type_array is a module parameter which sets mlx4_core ports type.
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 mlx4_core configfs infrastructure allows setting different
ports type per device. Port type attribute can take 2 strings "ib" or
"eth".

In case the configfs configuration 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/conf.c | 203 ++++++++++++++++++++++++++++++
 drivers/net/ethernet/mellanox/mlx4/main.c |  11 +-
 drivers/net/ethernet/mellanox/mlx4/mlx4.h |  23 ++++
 include/linux/mlx4/device.h               |   5 +
 4 files changed, 241 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/conf.c b/drivers/net/ethernet/mellanox/mlx4/conf.c
index abbe18a..678979a 100644
--- a/drivers/net/ethernet/mellanox/mlx4/conf.c
+++ b/drivers/net/ethernet/mellanox/mlx4/conf.c
@@ -57,6 +57,33 @@
  *
  **/
 
+/* Defines item_ops and group_ops for 'struct_in'
+ * Creates new group type 'new_memeber'
+ */
+#define MLX4_CONFIG_GROUPS(struct_in, new_member)			\
+	static struct config_group *					\
+	struct_in##_make_group(struct config_group *group,		\
+				const char *name)			\
+	{								\
+	struct new_member *new;						\
+									\
+	if (new_member##_verify_name(to_##struct_in(&group->cg_item), name))\
+		return ERR_PTR(-EINVAL);				\
+									\
+	new = kzalloc(sizeof(*new), GFP_KERNEL);			\
+	if (!new)							\
+		return ERR_PTR(-ENOMEM);				\
+									\
+	config_group_init_type_name(&new->group, name,			\
+				    &new_member##_item_type);		\
+									\
+	return &new->group;						\
+}									\
+									\
+static struct configfs_group_operations struct_in##_group_ops = {	\
+	.make_group     = &struct_in##_make_group,			\
+}									\
+
 #define MLX4_CFG_CONFIGFS_ATTR(struct_name, name)			\
 	static struct struct_name##_attribute struct_name##_##name =	\
 		__CONFIGFS_ATTR(name,  S_IRUGO | S_IWUSR,		\
@@ -75,10 +102,139 @@ static struct configfs_item_operations struct_in##_item_ops = {		\
 									\
 static struct config_item_type struct_in##_item_type = {		\
 	.ct_item_ops	= &struct_in##_item_ops,			\
+	.ct_group_ops   = &struct_in##_group_ops,			\
 	.ct_attrs	= struct_in##_attrs,				\
 	.ct_owner	= THIS_MODULE,					\
 }
 
+static ssize_t port_config_type_store(struct port_config *port_cfg,
+				      const char *page, size_t len)
+{
+	struct config_item *item = port_cfg->group.cg_item.ci_parent->ci_parent;
+	struct pdev_config *pdev_cfg = to_pdev_config(item);
+	struct mlx4_dev_persistent *persist = pci_get_drvdata(pdev_cfg->pdev);
+	struct mlx4_dev *dev = persist->dev;
+	unsigned long port;
+	int err;
+
+	err = kstrtoul(port_cfg->group.cg_item.ci_name, 10, &port);
+	if (err)
+		return -EINVAL;
+
+	if (!strcmp(page, "ib\n")) {
+		if (dev->caps.supported_type[port] & MLX4_PORT_TYPE_IB)
+			port_cfg->type = MLX4_PORT_TYPE_IB;
+		else
+			goto out_err;
+	} else if (!strcmp(page, "eth\n")) {
+		if (dev->caps.supported_type[port] & MLX4_PORT_TYPE_ETH)
+			port_cfg->type = MLX4_PORT_TYPE_ETH;
+		else
+			goto out_err;
+	} else {
+		pr_err("mlx4_core %s: Unsupported port type: %s, use 'ib' or 'eth'\n",
+		       item->ci_name, page);
+		return -EINVAL;
+	}
+	return len;
+
+out_err:
+	pr_err("mlx4_core %s: port type %s isn't supported.\n",
+	       item->ci_name, page);
+	return -EINVAL;
+}
+
+static ssize_t port_config_type_show(struct port_config *port_cfg, char *page)
+{
+	if (port_cfg->type == MLX4_PORT_TYPE_IB)
+		return sprintf(page, "%s\n", "ib");
+	else if (port_cfg->type == MLX4_PORT_TYPE_ETH)
+		return sprintf(page, "%s\n", "eth");
+	else if (port_cfg->type == MLX4_PORT_TYPE_AUTO)
+		return sprintf(page, "%s\n", "auto");
+	else if (port_cfg->type == MLX4_PORT_TYPE_NONE)
+		return sprintf(page, "%s\n", "port type wasn't set");
+	else
+		return sprintf(page, "%s\n", "unsupported port type");
+}
+
+CONFIGFS_ATTR_STRUCT(port_config);
+CONFIGFS_ATTR_OPS(port_config);
+MLX4_CFG_CONFIGFS_ATTR(port_config, type);
+
+static struct configfs_group_operations port_config_group_ops = {
+	NULL,
+};
+
+static struct configfs_attribute *port_config_attrs[] = {
+	&port_config_type.attr,
+	NULL,
+};
+
+static void port_config_release(struct config_item *item)
+{
+	kfree(to_port_config(item));
+}
+
+MLX4_CONFIG_ITEM_TYPE(port_config);
+
+static int port_config_verify_name(struct ports_config *ports_cfg,
+				   const char *name)
+{
+	struct config_item *pdev_item;
+	struct pdev_config *pdev_cfg;
+	struct mlx4_dev_persistent *persist;
+
+	pdev_item = ports_cfg->group.cg_item.ci_parent;
+
+	pdev_cfg = to_pdev_config(pdev_item);
+	persist = pci_get_drvdata(pdev_cfg->pdev);
+
+	if (!strcmp(name, "1")) {
+		return 0;
+	} else if (!strcmp(name, "2")) {
+		if (persist->dev->caps.num_ports == MLX4_MAX_PORTS)
+			return 0;
+		if (persist->dev->caps.num_ports != MLX4_MAX_PORTS)
+			pr_err("mlx4_core %s: Invalid directory name: %s, device has only one port\n",
+			       pdev_cfg->group.cg_item.ci_name, name);
+	} else {
+		pr_err("mlx4_core %s: Invalid directory name: %s, Directory name should be '1' or '2'.\n",
+		       pdev_cfg->group.cg_item.ci_name, name);
+	}
+	return -EINVAL;
+}
+
+MLX4_CONFIG_GROUPS(ports_config, port_config);
+
+static void ports_config_release(struct config_item *item)
+{
+	kfree(to_ports_config(item));
+}
+
+static struct configfs_item_operations ports_config_item_ops = {
+	.release		= ports_config_release,
+};
+
+static struct config_item_type ports_config_item_type = {
+	.ct_item_ops    = &ports_config_item_ops,
+	.ct_group_ops   = &ports_config_group_ops,
+	.ct_owner       = THIS_MODULE,
+};
+
+static int ports_config_verify_name(struct pdev_config *pdev_cfg,
+				    const char *name)
+{
+	char *pdev_name = pdev_cfg->group.cg_item.ci_name;
+
+	if (strcmp(name, MLX4_CONFIGFS_PORTS)) {
+		pr_err("mlx4_core %s: Invalid directory name: %s, directory name should be %s.\n",
+		       pdev_name, name, MLX4_CONFIGFS_PORTS);
+		return -EINVAL;
+	}
+	return 0;
+}
+
 static ssize_t pdev_config_commit_store(struct pdev_config *cfg,
 					const char *page, size_t len)
 {
@@ -139,6 +295,7 @@ static struct configfs_attribute *pdev_config_attrs[] = {
 	NULL,
 };
 
+MLX4_CONFIG_GROUPS(pdev_config, ports_config);
 MLX4_CONFIG_ITEM_TYPE(pdev_config);
 
 static struct pci_dev *find_pdev_by_name(const char *name)
@@ -233,6 +390,18 @@ out_err:
 	return ERR_PTR(err);
 }
 
+static struct config_group *mlx4_get_config_group(struct config_group *group,
+						  const char *name)
+{
+	struct config_item *item = NULL;
+
+	mutex_lock(&group->cg_subsys->su_mutex);
+	item = config_group_find_item(group, name);
+	mutex_unlock(&group->cg_subsys->su_mutex);
+
+	return to_config_group(item);
+}
+
 static struct config_group *device_driver_make(struct config_group *group,
 					       const char *name)
 {
@@ -257,6 +426,40 @@ static struct configfs_subsystem mlx4_subsys = {
 	},
 };
 
+int mlx4_conf_get_config(struct mlx4_dev *dev, struct pci_dev *pdev)
+{
+#define PORT_NAME_SIZE 2
+	struct config_group *pdev_group, *ports_group, *port_group;
+	char port_num[PORT_NAME_SIZE];
+	struct port_config *port_cfg;
+	int i;
+	struct mlx4_conf *mlx4_config = &dev->persist->mlx4_config;
+
+	pdev_group = mlx4_get_config_group(&mlx4_subsys.su_group,
+					   pci_name(pdev));
+	if (!pdev_group)
+		return -ENOENT;
+
+	ports_group = mlx4_get_config_group(pdev_group, MLX4_CONFIGFS_PORTS);
+	if (!ports_group) {
+		config_item_put(&pdev_group->cg_item);
+		return 0;
+	}
+
+	for (i = 1; i < (MLX4_MAX_PORTS + 1); i++) {
+		snprintf(port_num, PORT_NAME_SIZE, "%d", i);
+		port_group = mlx4_get_config_group(ports_group, port_num);
+		if (port_group) {
+			port_cfg = to_port_config(&port_group->cg_item);
+			mlx4_config->port_type[i] = port_cfg->type;
+			config_item_put(&port_group->cg_item);
+		}
+	}
+	config_item_put(&ports_group->cg_item);
+	config_item_put(&pdev_group->cg_item);
+	return 0;
+}
+
 int mlx4_configfs_init(void)
 {
 	int ret;
diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
index c81217e..b3a42c8 100644
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -415,7 +415,10 @@ 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)
+				if (dev->persist->mlx4_config.port_type[i])
+					dev->caps.port_type[i] =
+						dev->persist->mlx4_config.port_type[i];
+				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
@@ -2735,6 +2738,12 @@ static int mlx4_load_one(struct pci_dev *pdev, int pci_dev_data,
 	dev->rev_id = pdev->revision;
 	dev->numa_node = dev_to_node(&pdev->dev);
 
+#if IS_ENABLED(CONFIG_MLX4_CONFIGFS_FS)
+	err = mlx4_conf_get_config(dev, pdev);
+	if (!err)
+		mlx4_info(dev, "Using device configuration from configfs.\n");
+#endif
+
 	/* Detect if this device is a virtual function */
 	if (pci_dev_data & MLX4_PCI_DEV_IS_VF) {
 		mlx4_warn(dev, "Detected virtual function - running in slave mode\n");
diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4.h b/drivers/net/ethernet/mellanox/mlx4/mlx4.h
index ee43c0a..aad3db4 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mlx4.h
+++ b/drivers/net/ethernet/mellanox/mlx4/mlx4.h
@@ -1441,6 +1441,17 @@ u32 mlx4_zone_free_entries_unique(struct mlx4_zone_allocator *zones, u32 obj, u3
 struct mlx4_bitmap *mlx4_zone_get_bitmap(struct mlx4_zone_allocator *zones, u32 uid);
 
 #if IS_ENABLED(CONFIG_MLX4_CONFIGFS_FS)
+#define MLX4_CONFIGFS_PORTS "ports"
+
+struct port_config {
+	struct config_group group;
+	int type;
+};
+
+struct ports_config {
+	struct config_group group;
+};
+
 struct pdev_config {
 	struct config_group group;
 	struct pci_dev *pdev;
@@ -1453,6 +1464,18 @@ static inline struct pdev_config *to_pdev_config(struct config_item *item)
 				   struct pdev_config, group) : NULL;
 }
 
+static inline struct ports_config *to_ports_config(struct config_item *item)
+{
+	return item ? container_of(to_config_group(item),
+				   struct ports_config, group) : NULL;
+}
+
+static inline struct port_config *to_port_config(struct config_item *item)
+{
+	return item ? container_of(to_config_group(item),
+				   struct port_config, group) : NULL;
+}
+
 int mlx4_configfs_init(void);
 void mlx4_configfs_exit(void);
 int mlx4_conf_get_config(struct mlx4_dev *dev, struct pci_dev *pdev);
diff --git a/include/linux/mlx4/device.h b/include/linux/mlx4/device.h
index 1cc5482..e86ccc1 100644
--- a/include/linux/mlx4/device.h
+++ b/include/linux/mlx4/device.h
@@ -776,6 +776,10 @@ struct mlx4_vf_dev {
 	u8			n_ports;
 };
 
+struct mlx4_conf {
+	int port_type[MLX4_MAX_PORTS + 1];
+};
+
 struct mlx4_dev_persistent {
 	struct pci_dev	       *pdev;
 	struct mlx4_dev	       *dev;
@@ -789,6 +793,7 @@ struct mlx4_dev_persistent {
 	u8		state;
 	struct mutex	interface_state_mutex; /* protect SW state */
 	u8	interface_state;
+	struct mlx4_conf	mlx4_config;
 };
 
 struct mlx4_dev {
-- 
1.9.3

[PATCH net-next V1 3/3] net/mlx4_core: Set DMFS mode according to configfs configuration

[Amir Vadai]

From: Hadar Hen Zion <hadarh@mellanox.com>

Today, in order to enable Device Managed Flow steering (DMFS), we use
log_num_mgm_entry_size parameter. If the value of the parameter is not
positive, we treat the absolute value of log_num_mgm_entry_size as a bit
field. DMFS mode will be determine according to which bits were set.

Using the new mlx4_core configfs infrastructure for configuring DMFS
mode is more "user friendly", allows us to set different DMFS mode per
device and easily add new and advanced DMFS modes.
Currently, dmfs_mode attribute can take 3 strings: "disable", "dmfs" or
"A0_static".

In case the configfs configuration is missing, the DMFS mode of each
device will be set as before, according to log_num_mgm_entry_size value.

Signed-off-by: Hadar Hen Zion <hadarh@mellanox.com>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/conf.c | 46 +++++++++++++++++++++++++++++++
 drivers/net/ethernet/mellanox/mlx4/main.c | 25 +++++++++++++----
 drivers/net/ethernet/mellanox/mlx4/mlx4.h |  1 +
 include/linux/mlx4/device.h               |  7 +++++
 4 files changed, 73 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/conf.c b/drivers/net/ethernet/mellanox/mlx4/conf.c
index 678979a..437a0c1 100644
--- a/drivers/net/ethernet/mellanox/mlx4/conf.c
+++ b/drivers/net/ethernet/mellanox/mlx4/conf.c
@@ -235,6 +235,46 @@ static int ports_config_verify_name(struct pdev_config *pdev_cfg,
 	return 0;
 }
 
+static ssize_t pdev_config_dmfs_mode_store(struct pdev_config *pdev_cfg,
+					   const char *page, size_t len)
+{
+	struct mlx4_dev_persistent *persist = pci_get_drvdata(pdev_cfg->pdev);
+	struct mlx4_dev *dev = persist->dev;
+	char *pdev_name = pdev_cfg->group.cg_item.ci_name;
+
+	if (!(dev->caps.flags2 & MLX4_DEV_CAP_FLAG2_FS_EN)) {
+		pr_err("mlx4_core %s: DMFS isn't supported by the device.\n",
+		       pdev_name);
+		return -EINVAL;
+	}
+
+	if (!strcmp(page, "disable\n")) {
+		pdev_cfg->dmfs_mode = MLX4_DMFS_MODE_DISABLE;
+	} else if (!strcmp(page, "dmfs\n")) {
+		pdev_cfg->dmfs_mode = MLX4_DMFS_MODE_ETH_IPOIB;
+	} else if (!strcmp(page, "A0_static\n")) {
+		pdev_cfg->dmfs_mode = MLX4_DMFS_MODE_STATIC_A0;
+	} else {
+		pr_err("mlx4_core %s: Unsupported DMFS mode: %s, use 'disable', 'dmfs' or 'A0_static'\n",
+		       pdev_name, page);
+		return -EINVAL;
+	}
+	return len;
+}
+
+static ssize_t pdev_config_dmfs_mode_show(struct pdev_config *pdev_cfg,
+					  char *page)
+{
+	if (pdev_cfg->dmfs_mode == MLX4_DMFS_MODE_DISABLE)
+		return sprintf(page, "%s\n", "disable");
+	else if (pdev_cfg->dmfs_mode == MLX4_DMFS_MODE_ETH_IPOIB)
+		return sprintf(page, "%s\n", "dmfs");
+	else if (pdev_cfg->dmfs_mode == MLX4_DMFS_MODE_STATIC_A0)
+		return sprintf(page, "%s\n", "A0_static");
+	else
+		return sprintf(page, "%s\n", "unsupported dmfs mode\n");
+}
+
 static ssize_t pdev_config_commit_store(struct pdev_config *cfg,
 					const char *page, size_t len)
 {
@@ -280,6 +320,7 @@ static ssize_t pdev_config_commit_show(struct pdev_config *pdev_cfg, char *page)
 
 CONFIGFS_ATTR_STRUCT(pdev_config);
 CONFIGFS_ATTR_OPS(pdev_config);
+MLX4_CFG_CONFIGFS_ATTR(pdev_config, dmfs_mode);
 MLX4_CFG_CONFIGFS_ATTR(pdev_config, commit);
 
 static void pdev_config_release(struct config_item *item)
@@ -291,6 +332,7 @@ static void pdev_config_release(struct config_item *item)
 }
 
 static struct configfs_attribute *pdev_config_attrs[] = {
+	&pdev_config_dmfs_mode.attr,
 	&pdev_config_commit.attr,
 	NULL,
 };
@@ -432,6 +474,7 @@ int mlx4_conf_get_config(struct mlx4_dev *dev, struct pci_dev *pdev)
 	struct config_group *pdev_group, *ports_group, *port_group;
 	char port_num[PORT_NAME_SIZE];
 	struct port_config *port_cfg;
+	struct pdev_config *pdev_cfg;
 	int i;
 	struct mlx4_conf *mlx4_config = &dev->persist->mlx4_config;
 
@@ -440,6 +483,9 @@ int mlx4_conf_get_config(struct mlx4_dev *dev, struct pci_dev *pdev)
 	if (!pdev_group)
 		return -ENOENT;
 
+	pdev_cfg = to_pdev_config(&pdev_group->cg_item);
+	mlx4_config->dmfs_mode = pdev_cfg->dmfs_mode;
+
 	ports_group = mlx4_get_config_group(pdev_group, MLX4_CONFIGFS_PORTS);
 	if (!ports_group) {
 		config_item_put(&pdev_group->cg_item);
diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
index b3a42c8..cb40f0e 100644
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -1842,7 +1842,15 @@ static const char *dmfs_high_rate_steering_mode_str(int dmfs_high_steer_mode)
 static void choose_steering_mode(struct mlx4_dev *dev,
 				 struct mlx4_dev_cap *dev_cap)
 {
-	if (mlx4_log_num_mgm_entry_size <= 0) {
+	if (dev->persist->mlx4_config.dmfs_mode == MLX4_DMFS_MODE_STATIC_A0) {
+		if (dev->caps.dmfs_high_steer_mode ==
+		    MLX4_STEERING_DMFS_A0_NOT_SUPPORTED)
+			mlx4_err(dev, "DMFS high rate mode not supported\n");
+		else
+			dev->caps.dmfs_high_steer_mode =
+				MLX4_STEERING_DMFS_A0_STATIC;
+
+	} else if (mlx4_log_num_mgm_entry_size <= 0) {
 		if ((-mlx4_log_num_mgm_entry_size) & MLX4_DMFS_A0_STEERING) {
 			if (dev->caps.dmfs_high_steer_mode ==
 			    MLX4_STEERING_DMFS_A0_NOT_SUPPORTED)
@@ -1853,7 +1861,8 @@ static void choose_steering_mode(struct mlx4_dev *dev,
 		}
 	}
 
-	if (mlx4_log_num_mgm_entry_size <= 0 &&
+	if ((mlx4_log_num_mgm_entry_size <= 0 ||
+	     dev->persist->mlx4_config.dmfs_mode) &&
 	    dev_cap->flags2 & MLX4_DEV_CAP_FLAG2_FS_EN &&
 	    (!mlx4_is_mfunc(dev) ||
 	     (dev_cap->fs_max_num_qp_per_entry >=
@@ -1886,10 +1895,14 @@ static void choose_steering_mode(struct mlx4_dev *dev,
 			MLX4_DEFAULT_MGM_LOG_ENTRY_SIZE;
 		dev->caps.num_qp_per_mgm = mlx4_get_qp_per_mgm(dev);
 	}
-	mlx4_dbg(dev, "Steering mode is: %s, oper_log_mgm_entry_size = %d, modparam log_num_mgm_entry_size = %d\n",
-		 mlx4_steering_mode_str(dev->caps.steering_mode),
-		 dev->oper_log_mgm_entry_size,
-		 mlx4_log_num_mgm_entry_size);
+
+	mlx4_info(dev, "Steering mode is: %s %s, oper_log_mgm_entry_size = %d, modparam log_num_mgm_entry_size = %d\n",
+		  mlx4_steering_mode_str(dev->caps.steering_mode),
+		  (dev->caps.steering_mode == MLX4_STEERING_MODE_DEVICE_MANAGED) ?
+		  dmfs_high_rate_steering_mode_str(dev->caps.dmfs_high_steer_mode) : "",
+		  dev->oper_log_mgm_entry_size,
+		  mlx4_log_num_mgm_entry_size);
+
 }
 
 static void choose_tunnel_offload_mode(struct mlx4_dev *dev,
diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4.h b/drivers/net/ethernet/mellanox/mlx4/mlx4.h
index aad3db4..2ef902d 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mlx4.h
+++ b/drivers/net/ethernet/mellanox/mlx4/mlx4.h
@@ -1455,6 +1455,7 @@ struct ports_config {
 struct pdev_config {
 	struct config_group group;
 	struct pci_dev *pdev;
+	int dmfs_mode;
 	int commit;
 };
 
diff --git a/include/linux/mlx4/device.h b/include/linux/mlx4/device.h
index e86ccc1..f152039 100644
--- a/include/linux/mlx4/device.h
+++ b/include/linux/mlx4/device.h
@@ -126,6 +126,12 @@ enum {
 	MLX4_STEERING_DMFS_A0_NOT_SUPPORTED
 };
 
+enum {
+	MLX4_DMFS_MODE_DISABLE,
+	MLX4_DMFS_MODE_ETH_IPOIB,
+	MLX4_DMFS_MODE_STATIC_A0,
+};
+
 static inline const char *mlx4_steering_mode_str(int steering_mode)
 {
 	switch (steering_mode) {
@@ -777,6 +783,7 @@ struct mlx4_vf_dev {
 };
 
 struct mlx4_conf {
+	int dmfs_mode;
 	int port_type[MLX4_MAX_PORTS + 1];
 };
 
-- 
1.9.3

Re: [PATCH net-next V1 0/3] net/mlx4_core: Allow setting init-time device specific parameters

[David Miller]

From: Amir Vadai <amirv@mellanox.com>
Date: Wed, 11 Mar 2015 11:08:50 +0200

> Also, customers are paying for a very sophisticated piece of
> hardware, and we would like to enable power user to tweak it in some
> situtations. Of course the default mode should be used in 99% of the
> use cases.

How much money someone pays for your hardware has nothing to do with
the standards by which we design userspace interfaces to configure
these devices.

These textual interfaces are arbitrary, and you are choosing it only
because you cannot come up with a more reasonable scheme,

I'm not applying these changes.

Re: [PATCH net-next V1 0/3] net/mlx4_core: Allow setting init-time device specific parameters

[Hadar Hen Zion]

On Wed, Mar 11, 2015 at 7:14 PM, David Miller <davem@davemloft.net> wrote:
>
> From: Amir Vadai <amirv@mellanox.com>
> Date: Wed, 11 Mar 2015 11:08:50 +0200
>
> > Also, customers are paying for a very sophisticated piece of
> > hardware, and we would like to enable power user to tweak it in some
> > situtations. Of course the default mode should be used in 99% of the
> > use cases.
>
> How much money someone pays for your hardware has nothing to do with
> the standards by which we design userspace interfaces to configure
> these devices.
>
> These textual interfaces are arbitrary, and you are choosing it only
> because you cannot come up with a more reasonable scheme,
>
> I'm not applying these changes.
> --

In previous conversations Greg suggested us to use configfs.

Is this case a misuse of configfs?  maybe configfs should be deprecated... Greg?

Is a scheme based on netlink will be acceptable by you?

Thanks,

Hadar and Amir

Re: [PATCH net-next V1 0/3] net/mlx4_core: Allow setting init-time device specific parameters

[Greg KH]

On Thu, Mar 12, 2015 at 10:07:35AM +0200, Hadar Hen Zion wrote:
> On Wed, Mar 11, 2015 at 7:14 PM, David Miller <davem@davemloft.net> wrote:
> >
> > From: Amir Vadai <amirv@mellanox.com>
> > Date: Wed, 11 Mar 2015 11:08:50 +0200
> >
> > > Also, customers are paying for a very sophisticated piece of
> > > hardware, and we would like to enable power user to tweak it in some
> > > situtations. Of course the default mode should be used in 99% of the
> > > use cases.
> >
> > How much money someone pays for your hardware has nothing to do with
> > the standards by which we design userspace interfaces to configure
> > these devices.
> >
> > These textual interfaces are arbitrary, and you are choosing it only
> > because you cannot come up with a more reasonable scheme,
> >
> > I'm not applying these changes.
> > --
> 
> In previous conversations Greg suggested us to use configfs.
> 
> Is this case a misuse of configfs?  maybe configfs should be deprecated... Greg?

No, this has nothing to do with the validity of configfs, please re-read
what David said to you.

greg k-h

Re: [PATCH net-next V1 0/3] net/mlx4_core: Allow setting init-time device specific parameters

[Or Gerlitz]

On 3/12/2015 11:00 AM, Greg KH wrote:
> On Thu, Mar 12, 2015 at 10:07:35AM +0200, Hadar Hen Zion wrote:
>> On Wed, Mar 11, 2015 at 7:14 PM, David Miller <davem@davemloft.net> wrote:
>>> From: Amir Vadai <amirv@mellanox.com>
>>> Date: Wed, 11 Mar 2015 11:08:50 +0200
>>>
>>>> Also, customers are paying for a very sophisticated piece of hardware, and we would like to enable power user to tweak it in some situtations. Of course the default mode should be used in 99% of the use cases.
>>> How much money someone pays for your hardware has nothing to do with the standards by which we design userspace interfaces to configure these devices.
>>>
>>> These textual interfaces are arbitrary, and you are choosing it only because you cannot come up with a more reasonable scheme,
>>>
>>> I'm not applying these changes.
>>> ---
>> In previous conversations Greg suggested us to use configfs.
>>
>> Is this case a misuse of configfs? maybe configfs should be deprecated... Greg?
> No, this has nothing to do with the validity of configfs, please re-read what David said to you.

Greg,

So.. M2-- reading Dave's words I understand that he's against textual 
interfacessuch as configfs and prefers others. This brings two questions 
to the table (1) do we need to deprecate these textual interfaces / 
configfs and avoid using them for configuration purposes with new code? 
(2) does programmable interface such as netlink is the way to go for our 
purposes here (and elsewhere)?

Or.

Re: [PATCH net-next V1 0/3] net/mlx4_core: Allow setting init-time device specific parameters

[Greg KH]

On Thu, Mar 12, 2015 at 02:02:05PM +0200, Or Gerlitz wrote:
> On 3/12/2015 11:00 AM, Greg KH wrote:
> >On Thu, Mar 12, 2015 at 10:07:35AM +0200, Hadar Hen Zion wrote:
> >>On Wed, Mar 11, 2015 at 7:14 PM, David Miller <davem@davemloft.net> wrote:
> >>>From: Amir Vadai <amirv@mellanox.com>
> >>>Date: Wed, 11 Mar 2015 11:08:50 +0200
> >>>
> >>>>Also, customers are paying for a very sophisticated piece of hardware, and we would like to enable power user to tweak it in some situtations. Of course the default mode should be used in 99% of the use cases.
> >>>How much money someone pays for your hardware has nothing to do with the standards by which we design userspace interfaces to configure these devices.
> >>>
> >>>These textual interfaces are arbitrary, and you are choosing it only because you cannot come up with a more reasonable scheme,
> >>>
> >>>I'm not applying these changes.
> >>>---
> >>In previous conversations Greg suggested us to use configfs.
> >>
> >>Is this case a misuse of configfs? maybe configfs should be deprecated... Greg?
> >No, this has nothing to do with the validity of configfs, please re-read what David said to you.
> 
> Greg,
> 
> So.. M2-- reading Dave's words I understand that he's against textual
> interfacessuch as configfs and prefers others. This brings two questions to
> the table (1) do we need to deprecate these textual interfaces / configfs
> and avoid using them for configuration purposes with new code?

No, configfs works quite well for a number of other kernel subsystems,
so please leave them alone here.

> (2) does
> programmable interface such as netlink is the way to go for our purposes
> here (and elsewhere)?

I don't know, that's up to the network developers, but you had better
not abuse the firmware download inteface, that's what I strongly
objected to originally.

good luck,

greg k-h

Re: [PATCH net-next V1 0/3] net/mlx4_core: Allow setting init-time device specific parameters

[David Miller]

From: Hadar Hen Zion <hadarh@dev.mellanox.co.il>
Date: Thu, 12 Mar 2015 10:07:35 +0200

> On Wed, Mar 11, 2015 at 7:14 PM, David Miller <davem@davemloft.net> wrote:
>>
>> From: Amir Vadai <amirv@mellanox.com>
>> Date: Wed, 11 Mar 2015 11:08:50 +0200
>>
>> > Also, customers are paying for a very sophisticated piece of
>> > hardware, and we would like to enable power user to tweak it in some
>> > situtations. Of course the default mode should be used in 99% of the
>> > use cases.
>>
>> How much money someone pays for your hardware has nothing to do with
>> the standards by which we design userspace interfaces to configure
>> these devices.
>>
>> These textual interfaces are arbitrary, and you are choosing it only
>> because you cannot come up with a more reasonable scheme,
>>
>> I'm not applying these changes.
>> --
> 
> In previous conversations Greg suggested us to use configfs.
> 
> Is this case a misuse of configfs?  maybe configfs should be deprecated... Greg?
> 
> Is a scheme based on netlink will be acceptable by you?

A portable, well typed, interface that other vendors could use if their
hardware had similar features would be acceptable.

Re: [PATCH net-next V1 0/3] net/mlx4_core: Allow setting init-time device specific parameters

[David Miller]

From: Or Gerlitz <ogerlitz@mellanox.com>
Date: Thu, 12 Mar 2015 14:02:05 +0200

> So.. M2-- reading Dave's words I understand that he's against textual
> interfacessuch as configfs and prefers others. This brings two
> questions to the table (1) do we need to deprecate these textual
> interfaces / configfs and avoid using them for configuration purposes
> with new code? (2) does programmable interface such as netlink is the
> way to go for our purposes here (and elsewhere)?

Do not try to hijack a discussion about what interfaces you should be
using for your use case into a discussion about deprecating configfs.

Re: [PATCH net-next V1 0/3] net/mlx4_core: Allow setting init-time device specific parameters

[Hadar Hen Zion]

On Thu, Mar 12, 2015 at 8:05 PM, David Miller <davem@davemloft.net> wrote:
> From: Hadar Hen Zion <hadarh@dev.mellanox.co.il>
> Date: Thu, 12 Mar 2015 10:07:35 +0200
>
>> On Wed, Mar 11, 2015 at 7:14 PM, David Miller <davem@davemloft.net> wrote:
>>>
>>> From: Amir Vadai <amirv@mellanox.com>
>>> Date: Wed, 11 Mar 2015 11:08:50 +0200
>>>
>>> > Also, customers are paying for a very sophisticated piece of
>>> > hardware, and we would like to enable power user to tweak it in some
>>> > situtations. Of course the default mode should be used in 99% of the
>>> > use cases.
>>>
>>> How much money someone pays for your hardware has nothing to do with
>>> the standards by which we design userspace interfaces to configure
>>> these devices.
>>>
>>> These textual interfaces are arbitrary, and you are choosing it only
>>> because you cannot come up with a more reasonable scheme,
>>>
>>> I'm not applying these changes.
>>> --
>>
>> In previous conversations Greg suggested us to use configfs.
>>
>> Is this case a misuse of configfs?  maybe configfs should be deprecated... Greg?
>>
>> Is a scheme based on netlink will be acceptable by you?
>
> A portable, well typed, interface that other vendors could use if their
> hardware had similar features would be acceptable.

Do you mean an interface implementation that reminds the ethtool
private flags scheme but implemented through netlink, allows
configuration without a netdevice and more flexibility?

Re: [PATCH net-next V1 0/3] net/mlx4_core: Allow setting init-time device specific parameters

[David Miller]

From: Hadar Hen Zion <hadarh@dev.mellanox.co.il>
Date: Thu, 12 Mar 2015 20:59:26 +0200

> On Thu, Mar 12, 2015 at 8:05 PM, David Miller <davem@davemloft.net> wrote:
>> From: Hadar Hen Zion <hadarh@dev.mellanox.co.il>
>> Date: Thu, 12 Mar 2015 10:07:35 +0200
>>
>>> On Wed, Mar 11, 2015 at 7:14 PM, David Miller <davem@davemloft.net> wrote:
>>>>
>>>> From: Amir Vadai <amirv@mellanox.com>
>>>> Date: Wed, 11 Mar 2015 11:08:50 +0200
>>>>
>>>> > Also, customers are paying for a very sophisticated piece of
>>>> > hardware, and we would like to enable power user to tweak it in some
>>>> > situtations. Of course the default mode should be used in 99% of the
>>>> > use cases.
>>>>
>>>> How much money someone pays for your hardware has nothing to do with
>>>> the standards by which we design userspace interfaces to configure
>>>> these devices.
>>>>
>>>> These textual interfaces are arbitrary, and you are choosing it only
>>>> because you cannot come up with a more reasonable scheme,
>>>>
>>>> I'm not applying these changes.
>>>> --
>>>
>>> In previous conversations Greg suggested us to use configfs.
>>>
>>> Is this case a misuse of configfs?  maybe configfs should be deprecated... Greg?
>>>
>>> Is a scheme based on netlink will be acceptable by you?
>>
>> A portable, well typed, interface that other vendors could use if their
>> hardware had similar features would be acceptable.
> 
> Do you mean an interface implementation that reminds the ethtool
> private flags scheme but implemented through netlink, allows
> configuration without a netdevice and more flexibility?

I mean exactly what I said, neither more nor less.

Re: [PATCH net-next V1 0/3] net/mlx4_core: Allow setting init-time device specific parameters

[Or Gerlitz]

On Thu, Mar 12, 2015 at 9:31 PM, David Miller <davem@redhat.com> wrote:
> From: Hadar Hen Zion <hadarh@dev.mellanox.co.il>
>> On Thu, Mar 12, 2015 at 8:05 PM, David Miller <davem@davemloft.net> wrote:
>>> From: Hadar Hen Zion <hadarh@dev.mellanox.co.il>
>>>> On Wed, Mar 11, 2015 at 7:14 PM, David Miller <davem@davemloft.net>

[...]
>>>>> These textual interfaces are arbitrary, and you are choosing it only
>>>>> because you cannot come up with a more reasonable scheme,

[...]
>>>> Is a scheme based on netlink will be acceptable by you?

>>> A portable, well typed, interface that other vendors could use if their
>>> hardware had similar features would be acceptable.

>> Do you mean an interface implementation that reminds the ethtool
>> private flags scheme but implemented through netlink, allows
>> configuration without a netdevice and more flexibility?

> I mean exactly what I said, neither more nor less.

Hey Dave, we're trying to see we understand... already two
implementations in a row weren't accepted, so now before doing the 3rd
one, we want to better align.

So, it has to be non-textual, portable, well typed and one that can be
used by other vendors too. We are talking that could be used for
multiple vendor driver to configure few proprietary HW initialization
params. So how about a scheme based on key/value pairs carried over
netlink messages?

Or.