Netdev List
 help / color / mirror / Atom feed
* [PATCH 05/15] batman-adv: kernel doc fix for distributed-arp-table.h
From: Antonio Quartulli @ 2015-01-08 15:15 UTC (permalink / raw)
  To: davem
  Cc: netdev, b.a.t.m.a.n, Martin Hundebøll, Marek Lindner,
	Antonio Quartulli
In-Reply-To: <1420730120-9844-1-git-send-email-antonio@meshcoding.com>

From: Martin Hundebøll <martin@hundeboll.net>

Signed-off-by: Martin Hundebøll <martin@hundeboll.net>
Acked-by: Antonio Quartulli <antonio@meshcoding.com>
Signed-off-by: Marek Lindner <mareklindner@neomailbox.ch>
Signed-off-by: Antonio Quartulli <antonio@meshcoding.com>
---
 net/batman-adv/distributed-arp-table.h | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/net/batman-adv/distributed-arp-table.h b/net/batman-adv/distributed-arp-table.h
index d76e1d0..2fe0764 100644
--- a/net/batman-adv/distributed-arp-table.h
+++ b/net/batman-adv/distributed-arp-table.h
@@ -25,9 +25,7 @@
 
 #include <linux/if_arp.h>
 
-/**
- * BATADV_DAT_ADDR_MAX - maximum address value in the DHT space
- */
+/* BATADV_DAT_ADDR_MAX - maximum address value in the DHT space */
 #define BATADV_DAT_ADDR_MAX ((batadv_dat_addr_t)~(batadv_dat_addr_t)0)
 
 void batadv_dat_status_update(struct net_device *net_dev);
-- 
2.2.1

^ permalink raw reply related

* [PATCH 08/15] batman-adv: checkpatch - No space is necessary after a cast
From: Antonio Quartulli @ 2015-01-08 15:15 UTC (permalink / raw)
  To: davem; +Cc: netdev, b.a.t.m.a.n, Antonio Quartulli, Marek Lindner
In-Reply-To: <1420730120-9844-1-git-send-email-antonio@meshcoding.com>

Signed-off-by: Antonio Quartulli <antonio@meshcoding.com>
Signed-off-by: Marek Lindner <mareklindner@neomailbox.ch>
---
 net/batman-adv/main.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/batman-adv/main.h b/net/batman-adv/main.h
index 2a59b04..18286f8 100644
--- a/net/batman-adv/main.h
+++ b/net/batman-adv/main.h
@@ -312,10 +312,10 @@ static inline bool batadv_has_timed_out(unsigned long timestamp,
  *  - when adding 128 - it is neither a predecessor nor a successor,
  *  - after adding more than 127 to the starting value - it is a successor
  */
-#define batadv_seq_before(x, y) ({typeof(x) _d1 = (x); \
-				 typeof(y) _d2 = (y); \
-				 typeof(x) _dummy = (_d1 - _d2); \
-				 (void) (&_d1 == &_d2); \
+#define batadv_seq_before(x, y) ({typeof(x)_d1 = (x); \
+				 typeof(y)_d2 = (y); \
+				 typeof(x)_dummy = (_d1 - _d2); \
+				 (void)(&_d1 == &_d2); \
 				 _dummy > batadv_smallest_signed_int(_dummy); })
 #define batadv_seq_after(x, y) batadv_seq_before(y, x)
 
-- 
2.2.1

^ permalink raw reply related

* [PATCH 07/15] batman-adv: checkpatch - else is not generally useful after a break or return
From: Antonio Quartulli @ 2015-01-08 15:15 UTC (permalink / raw)
  To: davem; +Cc: netdev, b.a.t.m.a.n, Antonio Quartulli, Marek Lindner
In-Reply-To: <1420730120-9844-1-git-send-email-antonio@meshcoding.com>

Signed-off-by: Antonio Quartulli <antonio@meshcoding.com>
Signed-off-by: Marek Lindner <mareklindner@neomailbox.ch>
---
 net/batman-adv/bitarray.h          | 3 +--
 net/batman-adv/fragmentation.h     | 3 +--
 net/batman-adv/network-coding.c    | 3 +--
 net/batman-adv/translation-table.c | 5 ++---
 4 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/net/batman-adv/bitarray.h b/net/batman-adv/bitarray.h
index cc24073..2acaafe 100644
--- a/net/batman-adv/bitarray.h
+++ b/net/batman-adv/bitarray.h
@@ -29,8 +29,7 @@ static inline int batadv_test_bit(const unsigned long *seq_bits,
 	diff = last_seqno - curr_seqno;
 	if (diff < 0 || diff >= BATADV_TQ_LOCAL_WINDOW_SIZE)
 		return 0;
-	else
-		return test_bit(diff, seq_bits) != 0;
+	return test_bit(diff, seq_bits) != 0;
 }
 
 /* turn corresponding bit on, so we can remember that we got the packet */
diff --git a/net/batman-adv/fragmentation.h b/net/batman-adv/fragmentation.h
index 5d7a0e6..d848cf6 100644
--- a/net/batman-adv/fragmentation.h
+++ b/net/batman-adv/fragmentation.h
@@ -41,8 +41,7 @@ batadv_frag_check_entry(struct batadv_frag_table_entry *frags_entry)
 	if (!hlist_empty(&frags_entry->head) &&
 	    batadv_has_timed_out(frags_entry->timestamp, BATADV_FRAG_TIMEOUT))
 		return true;
-	else
-		return false;
+	return false;
 }
 
 #endif /* _NET_BATMAN_ADV_FRAGMENTATION_H_ */
diff --git a/net/batman-adv/network-coding.c b/net/batman-adv/network-coding.c
index fab47f1..127cc4d 100644
--- a/net/batman-adv/network-coding.c
+++ b/net/batman-adv/network-coding.c
@@ -1212,8 +1212,7 @@ static bool batadv_nc_skb_coding_possible(struct sk_buff *skb,
 {
 	if (BATADV_SKB_CB(skb)->decoded && !batadv_compare_eth(dst, src))
 		return false;
-	else
-		return true;
+	return true;
 }
 
 /**
diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index 5f59e7f..38a804e 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -2769,9 +2769,8 @@ static bool batadv_send_tt_response(struct batadv_priv *bat_priv,
 {
 	if (batadv_is_my_mac(bat_priv, req_dst))
 		return batadv_send_my_tt_response(bat_priv, tt_data, req_src);
-	else
-		return batadv_send_other_tt_response(bat_priv, tt_data,
-						     req_src, req_dst);
+	return batadv_send_other_tt_response(bat_priv, tt_data, req_src,
+					     req_dst);
 }
 
 static void _batadv_tt_update_changes(struct batadv_priv *bat_priv,
-- 
2.2.1

^ permalink raw reply related

* [PATCH 09/15] batman-adv: checkpatch - Please use a blank line after declarations
From: Antonio Quartulli @ 2015-01-08 15:15 UTC (permalink / raw)
  To: davem; +Cc: netdev, b.a.t.m.a.n, Antonio Quartulli, Marek Lindner
In-Reply-To: <1420730120-9844-1-git-send-email-antonio@meshcoding.com>

Signed-off-by: Antonio Quartulli <antonio@meshcoding.com>
Signed-off-by: Marek Lindner <mareklindner@neomailbox.ch>
---
 net/batman-adv/debugfs.c               | 1 +
 net/batman-adv/distributed-arp-table.c | 1 +
 net/batman-adv/gateway_client.c        | 1 +
 net/batman-adv/packet.h                | 1 +
 4 files changed, 4 insertions(+)

diff --git a/net/batman-adv/debugfs.c b/net/batman-adv/debugfs.c
index 1485091..a4972874 100644
--- a/net/batman-adv/debugfs.c
+++ b/net/batman-adv/debugfs.c
@@ -404,6 +404,7 @@ struct batadv_debuginfo batadv_hardif_debuginfo_##_name = {	\
 		.release = single_release,			\
 	},							\
 }
+
 static BATADV_HARDIF_DEBUGINFO(originators, S_IRUGO,
 			       batadv_originators_hardif_open);
 
diff --git a/net/batman-adv/distributed-arp-table.c b/net/batman-adv/distributed-arp-table.c
index b598111..aad022d 100644
--- a/net/batman-adv/distributed-arp-table.c
+++ b/net/batman-adv/distributed-arp-table.c
@@ -1100,6 +1100,7 @@ void batadv_dat_snoop_outgoing_arp_reply(struct batadv_priv *bat_priv,
 	batadv_dat_send_data(bat_priv, skb, ip_src, BATADV_P_DAT_DHT_PUT);
 	batadv_dat_send_data(bat_priv, skb, ip_dst, BATADV_P_DAT_DHT_PUT);
 }
+
 /**
  * batadv_dat_snoop_incoming_arp_reply - snoop the ARP reply and fill the local
  * DAT storage only
diff --git a/net/batman-adv/gateway_client.c b/net/batman-adv/gateway_client.c
index e0bcf9e..27649e8 100644
--- a/net/batman-adv/gateway_client.c
+++ b/net/batman-adv/gateway_client.c
@@ -775,6 +775,7 @@ batadv_gw_dhcp_recipient_get(struct sk_buff *skb, unsigned int *header_len,
 
 	return ret;
 }
+
 /**
  * batadv_gw_out_of_range - check if the dhcp request destination is the best gw
  * @bat_priv: the bat priv with all the soft interface information
diff --git a/net/batman-adv/packet.h b/net/batman-adv/packet.h
index 34e096d..facd1fe 100644
--- a/net/batman-adv/packet.h
+++ b/net/batman-adv/packet.h
@@ -198,6 +198,7 @@ struct batadv_bla_claim_dst {
 	uint8_t type;		/* bla_claimframe */
 	__be16 group;		/* group id */
 };
+
 #pragma pack()
 
 /**
-- 
2.2.1

^ permalink raw reply related

* [RFC net-next 1/3] net/dev_c_mlx4: Support mlx4_core pre-load configuration
From: Hadar Hen Zion @ 2015-01-08 15:16 UTC (permalink / raw)
  To: David S. Miller, gregkh
  Cc: netdev, Amir Vadai, Hadar Har-Zion, Yevgeny Petrilin, Or Gerlitz,
	shannon.nelson, Doug Ledford, greearb
In-Reply-To: <1420730220-20224-1-git-send-email-hadarh@mellanox.com>

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

^ permalink raw reply related

* [RFC net-next 2/3] devconf: Add configuration module for setting pre-load parameters
From: Hadar Hen Zion @ 2015-01-08 15:16 UTC (permalink / raw)
  To: David S. Miller, gregkh
  Cc: netdev, Amir Vadai, Hadar Har-Zion, Yevgeny Petrilin, Or Gerlitz,
	shannon.nelson, Doug Ledford, greearb
In-Reply-To: <1420730220-20224-1-git-send-email-hadarh@mellanox.com>

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

^ permalink raw reply related

* [RFC net-next 3/3] net/mlx4_core: Set port_type value according to configuration module
From: Hadar Hen Zion @ 2015-01-08 15:17 UTC (permalink / raw)
  To: David S. Miller, gregkh
  Cc: netdev, Amir Vadai, Hadar Har-Zion, Yevgeny Petrilin, Or Gerlitz,
	shannon.nelson, Doug Ledford, greearb
In-Reply-To: <1420730220-20224-1-git-send-email-hadarh@mellanox.com>

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

^ permalink raw reply related

* Re: [PATCH v3 4/4] can: kvaser_usb: Add support for the Usbcan-II family
From: Ahmed S. Darwish @ 2015-01-08 15:19 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: Olivier Sobrie, Oliver Hartkopp, Wolfgang Grandegger,
	David S. Miller, Paul Gortmaker, Linux-CAN, netdev, LKML
In-Reply-To: <54AE6FC1.6050007@pengutronix.de>

Hi Marc,

On Thu, Jan 08, 2015 at 12:53:37PM +0100, Marc Kleine-Budde wrote:
> On 01/05/2015 07:31 PM, Ahmed S. Darwish wrote:
> > From: Ahmed S. Darwish <ahmed.darwish@valeo.com>
[...]
> > +/* Kvaser USB CAN dongles are divided into two major families:
> > + * - Leaf: Based on Renesas M32C, running firmware labeled as 'filo'
> > + * - UsbcanII: Based on Renesas M16C, running firmware labeled as 'helios'
> > + */
> > +enum kvaser_usb_family {
> > +	KVASER_LEAF,
> > +	KVASER_USBCAN,
> > +};
> > +
> >  #define MAX_TX_URBS			16
> >  #define MAX_RX_URBS			4
> >  #define START_TIMEOUT			1000 /* msecs */
> > @@ -30,8 +41,9 @@
> >  #define RX_BUFFER_SIZE			3072
> >  #define CAN_USB_CLOCK			8000000
> >  #define MAX_NET_DEVICES			3
> > +#define MAX_USBCAN_NET_DEVICES		2
> >  
> > -/* Kvaser USB devices */
> > +/* Leaf USB devices */
> >  #define KVASER_VENDOR_ID		0x0bfd
> >  #define USB_LEAF_DEVEL_PRODUCT_ID	10
> >  #define USB_LEAF_LITE_PRODUCT_ID	11
> > @@ -55,6 +67,16 @@
> >  #define USB_CAN_R_PRODUCT_ID		39
> >  #define USB_LEAF_LITE_V2_PRODUCT_ID	288
> >  #define USB_MINI_PCIE_HS_PRODUCT_ID	289
> > +#define LEAF_PRODUCT_ID(id)		(id >= USB_LEAF_DEVEL_PRODUCT_ID && \
> > +					 id <= USB_MINI_PCIE_HS_PRODUCT_ID)
> 
> Can you please convert both *_PRODUCT_ID() macros into static inline
> bool functions.
> 

Will do.

[...]
> >  MODULE_DEVICE_TABLE(usb, kvaser_usb_table);
> > @@ -463,7 +631,18 @@ static int kvaser_usb_get_software_info(struct kvaser_usb *dev)
> >  	if (err)
> >  		return err;
> >  
> > -	dev->fw_version = le32_to_cpu(msg.u.softinfo.fw_version);
> > +	switch (dev->family) {
> > +	case KVASER_LEAF:
> > +		dev->fw_version = le32_to_cpu(msg.u.leaf.softinfo.fw_version);
> > +		break;
> > +	case KVASER_USBCAN:
> > +		dev->fw_version = le32_to_cpu(msg.u.usbcan.softinfo.fw_version);
> > +		break;
> > +	default:
> > +		dev_err(dev->udev->dev.parent,
> > +			"Invalid device family (%d)\n", dev->family);
> > +		return -EINVAL;
> 
> The default case should not happen. I think you can remove it.
> 

It's true, it _should_ never happen. But I only add such checks if
the follow-up code critically depends on a certain `dev->family`
behavior. So it's kind of a defensive check against any possible
bug in driver or memory.

What do you think?

> > +	}
> >  
> >  	return 0;
> >  }
> > @@ -484,6 +663,9 @@ static int kvaser_usb_get_card_info(struct kvaser_usb *dev)
> >  	dev->nchannels = msg.u.cardinfo.nchannels;
> >  	if (dev->nchannels > MAX_NET_DEVICES)
> >  		return -EINVAL;
> > +	if (dev->family == KVASER_USBCAN &&
> > +	    dev->nchannels > MAX_USBCAN_NET_DEVICES)
> > +		return -EINVAL;
> 
> Nitpick, as the new "if" also does a test on nchannels, why no extend
> the existing "if" with an "||"?
> 

Will do.

> >  
> >  	return 0;
> >  }
> > @@ -496,8 +678,10 @@ static void kvaser_usb_tx_acknowledge(const struct kvaser_usb *dev,
> >  	struct kvaser_usb_net_priv *priv;
> >  	struct sk_buff *skb;
> >  	struct can_frame *cf;
> > -	u8 channel = msg->u.tx_acknowledge.channel;
> > -	u8 tid = msg->u.tx_acknowledge.tid;
> > +	u8 channel, tid;
> > +
> > +	channel = msg->u.tx_acknowledge_header.channel;
> > +	tid = msg->u.tx_acknowledge_header.tid;
> >  
> >  	if (channel >= dev->nchannels) {
> >  		dev_err(dev->udev->dev.parent,
> > @@ -615,37 +799,80 @@ static void kvaser_usb_unlink_tx_urbs(struct kvaser_usb_net_priv *priv)
> >  		priv->tx_contexts[i].echo_index = MAX_TX_URBS;
> >  }
> >  
> > -static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
> > -				const struct kvaser_msg *msg)
> > +static void kvaser_report_error_event(const struct kvaser_usb *dev,
> > +				      struct kvaser_error_summary *es);
> 
> Please rearange your code that forward declarations are not needed (if
> possible - I haven't checked, though).
> 

I originally did it that way, but it completely messed up with the
patch if I do such rearrangement. A huge block of code gets removed
at top of the patch, and it got added again at the end, making the
actual important lines changed _within_ such big block non-apparent.

Maybe I should do the re-arrangement in a follow-up patch?

> > +
> > +/* Report error to userspace iff the controller's errors counter has
> > + * increased, or we're the only channel seeing the bus error state.
> > + *
> > + * As reported by USBCAN sheets, "the CAN controller has difficulties
> > + * to tell whether an error frame arrived on channel 1 or on channel 2."
> > + * Thus, error counters are compared with their earlier values to
> > + * determine which channel was responsible for the error event.
> 
> Your code doesn't match this comment. You compare the error counters
> against the old values to tell if it's a rx or tx error, the channel
> information from the struct kvaser_error_summary is used directly.
> 

Hmmm, good catch, upon a second look, the code looks a bit deceiving.
The comments, meanwhile, are taken verbatim from the Kvaser sheets:

  http://www.kvaser.com/canlib-webhelp/page_hardware_specific_can_controllers.html
  Archived at http://www.webcitation.org/6VQd87jIA

So, what happens is that the firmware does not tell us whether the
received error event is for ch0 or ch1. But it gives us the (possibly
new) error counters for both of them:

struct usbcan_msg_error_event {
        [..]
        u8 tx_errors_count_ch0;
        u8 rx_errors_count_ch0;
        u8 tx_errors_count_ch1;
        u8 rx_errors_count_ch1;
	[..]
} __packed;

We loop over each channel, and report an error to userspace if extra
errors were spotted for such channel.

kvaser_error_summary is not a firmware command or response. Since
the wire format for an error event differs between Leaf and Usbcan,
it's just our way to summarize an error whether it's from any of
them, and this is where the conflict stems from:

- If it's a Leaf-device, `error_summary->channel' is the excat
channel reported by the firmware
- If it's a Usbcan device, `error_summary->channel' is just a mark
to check the error counters for such a channel and report error
to userspace iff they've increased.

Thus the raison d'etre for `error_summary' struct is to share
the error handling code between Leaf and UsbcanII devices since
it's quite similar.

So to clear up this conflict, I suggest the following error_summary
layout:

struct kvaser_error_summary {
       union {
                struct {
                       u8 channel;
                } leaf;
                struct {
                       u8 possible_channel;
                } usbcan;
       };
       /* Rest of layout is left as-is */
}

This way, it's clear that in case of Usbcan, channel is not final
and we need further work to do. What do you think?

(BTW, any better name than "kvaser_error_summary"? It conflicts a
little bit with the namespace used for the packed wire format
structures "kvaser_*")

> > + */
> > +static void usbcan_report_error_if_applicable(const struct kvaser_usb *dev,
> > +					      struct kvaser_error_summary *es)
> 
> Nitpick: can you please add a "kvaser_" prefix to the all usbcan_* and
> leaf_* functions.
> 

Will do.

> >  {
> > -	struct can_frame *cf;
> > -	struct sk_buff *skb;
> > -	struct net_device_stats *stats;
> >  	struct kvaser_usb_net_priv *priv;
> > -	unsigned int new_state;
> > -	u8 channel, status, txerr, rxerr, error_factor;
> > +	int old_tx_err_count, old_rx_err_count, channel, report_error;
> 
> bool report_error;
> 

Will do.

> > +
> > +	channel = es->channel;
> > +	if (channel >= dev->nchannels) {
> > +		dev_err(dev->udev->dev.parent,
> > +			"Invalid channel number (%d)\n", channel);
> > +		return;
> > +	}
> > +
> > +	priv = dev->nets[channel];
> > +	old_tx_err_count = priv->bec.txerr;
> > +	old_rx_err_count = priv->bec.rxerr;
> 
> Why do you make a copy of priv->bec, AFAICS you can use
> priv->bec.{r,t}xerr directly?
> 

Initially, just for clarity, as I was not used yet to socketCAN
structures ;-) will remove it in the next submission.

> > +
> > +	report_error = 0;
> > +	if (es->txerr > old_tx_err_count) {
> > +		es->usbcan.error_state |= USBCAN_ERROR_STATE_TX_ERROR;
> > +		report_error = 1;
> > +	}
> > +	if (es->rxerr > old_rx_err_count) {
> > +		es->usbcan.error_state |= USBCAN_ERROR_STATE_RX_ERROR;
> > +		report_error = 1;
> > +	}
> > +	if ((es->status & M16C_STATE_BUS_ERROR) &&
> > +	    !(es->usbcan.other_ch_status & M16C_STATE_BUS_ERROR)) {
> > +		es->usbcan.error_state |= USBCAN_ERROR_STATE_BUSERROR;
> > +		report_error = 1;
> > +	}
> > +
> > +	if (report_error)
> > +		kvaser_report_error_event(dev, es);
> > +}
> > +
> > +/* Extract error summary from a Leaf-based device error message */
> > +static void leaf_extract_error_from_msg(const struct kvaser_usb *dev,
> > +					const struct kvaser_msg *msg)
> > +{
> > +	struct kvaser_error_summary es = { 0, };
> 
> IIRC "es = { };" should be sufficient.
> 

Correct, will do.

> >  
> >  	switch (msg->id) {
> >  	case CMD_CAN_ERROR_EVENT:
> > -		channel = msg->u.error_event.channel;
> > -		status =  msg->u.error_event.status;
> > -		txerr = msg->u.error_event.tx_errors_count;
> > -		rxerr = msg->u.error_event.rx_errors_count;
> > -		error_factor = msg->u.error_event.error_factor;
> > +		es.channel = msg->u.leaf.error_event.channel;
> > +		es.status =  msg->u.leaf.error_event.status;
> > +		es.txerr = msg->u.leaf.error_event.tx_errors_count;
> > +		es.rxerr = msg->u.leaf.error_event.rx_errors_count;
> > +		es.leaf.error_factor = msg->u.leaf.error_event.error_factor;
> >  		break;
> > -	case CMD_LOG_MESSAGE:
> > -		channel = msg->u.log_message.channel;
> > -		status = msg->u.log_message.data[0];
> > -		txerr = msg->u.log_message.data[2];
> > -		rxerr = msg->u.log_message.data[3];
> > -		error_factor = msg->u.log_message.data[1];
> > +	case CMD_LEAF_LOG_MESSAGE:
> > +		es.channel = msg->u.leaf.log_message.channel;
> > +		es.status = msg->u.leaf.log_message.data[0];
> > +		es.txerr = msg->u.leaf.log_message.data[2];
> > +		es.rxerr = msg->u.leaf.log_message.data[3];
> > +		es.leaf.error_factor = msg->u.leaf.log_message.data[1];
> >  		break;
> >  	case CMD_CHIP_STATE_EVENT:
> > -		channel = msg->u.chip_state_event.channel;
> > -		status =  msg->u.chip_state_event.status;
> > -		txerr = msg->u.chip_state_event.tx_errors_count;
> > -		rxerr = msg->u.chip_state_event.rx_errors_count;
> > -		error_factor = 0;
> > +		es.channel = msg->u.leaf.chip_state_event.channel;
> > +		es.status =  msg->u.leaf.chip_state_event.status;
> > +		es.txerr = msg->u.leaf.chip_state_event.tx_errors_count;
> > +		es.rxerr = msg->u.leaf.chip_state_event.rx_errors_count;
> > +		es.leaf.error_factor = 0;
> >  		break;
> >  	default:
> >  		dev_err(dev->udev->dev.parent, "Invalid msg id (%d)\n",
> > @@ -653,16 +880,92 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
> >  		return;
> >  	}
> >  
> > -	if (channel >= dev->nchannels) {
> > +	kvaser_report_error_event(dev, &es);
> > +}
> > +
> > +/* Extract error summary from a USBCANII-based device error message */
> > +static void usbcan_extract_error_from_msg(const struct kvaser_usb *dev,
> > +					  const struct kvaser_msg *msg)
> > +{
> > +	struct kvaser_error_summary es = { 0, };
> 
> same here.
> 

Ditto.

> > +
> > +	switch (msg->id) {
> > +	/* Sometimes errors are sent as unsolicited chip state events */
> > +	case CMD_CHIP_STATE_EVENT:
> > +		es.channel = msg->u.usbcan.chip_state_event.channel;
> > +		es.status =  msg->u.usbcan.chip_state_event.status;
> > +		es.txerr = msg->u.usbcan.chip_state_event.tx_errors_count;
> > +		es.rxerr = msg->u.usbcan.chip_state_event.rx_errors_count;
> > +		usbcan_report_error_if_applicable(dev, &es);
> > +		break;
> > +
> > +	case CMD_CAN_ERROR_EVENT:
> > +		es.channel = 0;
> > +		es.status = msg->u.usbcan.error_event.status_ch0;
> > +		es.txerr = msg->u.usbcan.error_event.tx_errors_count_ch0;
> > +		es.rxerr = msg->u.usbcan.error_event.rx_errors_count_ch0;
> > +		es.usbcan.other_ch_status =
> > +			msg->u.usbcan.error_event.status_ch1;
> > +		usbcan_report_error_if_applicable(dev, &es);
> > +
> > +		/* For error events, the USBCAN firmware does not support
> > +		 * more than 2 channels: ch0, and ch1.
> > +		 */
> > +		if (dev->nchannels > 1) {
> > +			es.channel = 1;
> 
> Why is channel == 1 if the device has more than 1 channel?
> 

This is related to the "kvaser_error_summary" discussion above
where "channel" is only a suggestion for checking the error
counters.

If the Usbcan device has only one channel, then there's no need
to check if the "tx_errors_count_ch1", "rx_errors_count_ch1" has
increased or not. Their values are undefined.

> > +			es.status = msg->u.usbcan.error_event.status_ch1;
> > +			es.txerr =
> > +				msg->u.usbcan.error_event.tx_errors_count_ch1;
> > +			es.rxerr =
> > +				msg->u.usbcan.error_event.rx_errors_count_ch1;
> > +			es.usbcan.other_ch_status =
> > +				msg->u.usbcan.error_event.status_ch0;
> > +			usbcan_report_error_if_applicable(dev, &es);
> > +		}
> > +		break;
> > +
> > +	default:
> > +		dev_err(dev->udev->dev.parent, "Invalid msg id (%d)\n",
> > +			msg->id);
> > +	}
> > +}
> > +
> > +static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
> > +				const struct kvaser_msg *msg)
> > +{
> > +	switch (dev->family) {
> > +	case KVASER_LEAF:
> > +		leaf_extract_error_from_msg(dev, msg);
> > +		break;
> > +	case KVASER_USBCAN:
> > +		usbcan_extract_error_from_msg(dev, msg);
> > +		break;
> > +	default:
> should not happen.

Yes. As in above, will wait your input in this regarding the checks
being defensive.

[...]
> > +	case KVASER_USBCAN:
> > +		if (es->usbcan.error_state & USBCAN_ERROR_STATE_TX_ERROR)
> > +			stats->tx_errors++;
> > +		if (es->usbcan.error_state & USBCAN_ERROR_STATE_RX_ERROR)
> > +			stats->rx_errors++;
> > +		if (es->usbcan.error_state & USBCAN_ERROR_STATE_BUSERROR) {
> > +			priv->can.can_stats.bus_error++;
> > +			cf->can_id |= CAN_ERR_BUSERROR;
> > +		}
> > +		break;
> > +	default:
> > +		dev_err(dev->udev->dev.parent,
> > +			"Invalid device family (%d)\n", dev->family);
> > +		goto err;
> 
> should not happen.
> 

Ditto.

> >  	}
> >  
> > -	cf->data[6] = txerr;
> > -	cf->data[7] = rxerr;
> > +	cf->data[6] = es->txerr;
> > +	cf->data[7] = es->rxerr;
> >  
> > -	priv->bec.txerr = txerr;
> > -	priv->bec.rxerr = rxerr;
> > +	priv->bec.txerr = es->txerr;
> > +	priv->bec.rxerr = es->rxerr;
> >  
> >  	priv->can.state = new_state;
> >  
> > @@ -774,6 +1093,11 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
> >  
> >  	stats->rx_packets++;
> >  	stats->rx_bytes += cf->can_dlc;
> > +
> > +	return;
> > +
> > +err:
> > +	dev_kfree_skb(skb);
> >  }
> >  
> >  static void kvaser_usb_rx_can_err(const struct kvaser_usb_net_priv *priv,
> > @@ -783,16 +1107,16 @@ static void kvaser_usb_rx_can_err(const struct kvaser_usb_net_priv *priv,
> >  	struct sk_buff *skb;
> >  	struct net_device_stats *stats = &priv->netdev->stats;
> >  
> > -	if (msg->u.rx_can.flag & (MSG_FLAG_ERROR_FRAME |
> > +	if (msg->u.rx_can_header.flag & (MSG_FLAG_ERROR_FRAME |
> >  					 MSG_FLAG_NERR)) {
> >  		netdev_err(priv->netdev, "Unknow error (flags: 0x%02x)\n",
> > -			   msg->u.rx_can.flag);
> > +			   msg->u.rx_can_header.flag);
> >  
> >  		stats->rx_errors++;
> >  		return;
> >  	}
> >  
> > -	if (msg->u.rx_can.flag & MSG_FLAG_OVERRUN) {
> > +	if (msg->u.rx_can_header.flag & MSG_FLAG_OVERRUN) {
> >  		skb = alloc_can_err_skb(priv->netdev, &cf);
> > 		if (!skb) {
> > 			stats->rx_dropped++;
> > 			return;
> > 		}
> 
> Can you prepare a (seperate) patch that does the stats, even in case of OOM here. Same for kvaser_report_error_event()

Sure.

In kvaser_report_error_event() though, isn't it a little bit tricky?
Specially in fragments as in below:

        switch (dev->family) {
        case KVASER_LEAF:
                if (es->leaf.error_factor) {
                        priv->can.can_stats.bus_error++;
                        stats->rx_errors++;

                        cf->can_id |= CAN_ERR_BUSERROR | CAN_ERR_PROT;
                        [...]
                }
                break;
        case KVASER_USBCAN:
                if (es->usbcan.error_state & USBCAN_ERROR_STATE_TX_ERROR)
                        stats->tx_errors++;
                if (es->usbcan.error_state & USBCAN_ERROR_STATE_RX_ERROR)
                        stats->rx_errors++;
                if (es->usbcan.error_state & USBCAN_ERROR_STATE_BUSERROR) {
                        priv->can.can_stats.bus_error++;
                        cf->can_id |= CAN_ERR_BUSERROR;
                }
                break;
        }

IMHO, there will be some duplication of the above fragment. Once
to handle "stats->*", and once to handle packet-related "cf->*" stuff.
Also in the Usbcan case, it will clutter the error_state checks in
different places.

> 
> > 
> > 		cf->can_id |= CAN_ERR_CRTL;
> > 		cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
> > 
> > 		stats->rx_over_errors++;
> > 		stats->rx_errors++;
> > 
> > 		netif_rx(skb);
> > 
> > 		stats->rx_packets++;
> > 		stats->rx_bytes += cf->can_dlc;
> 
> Another patch would be not to touch cf after netif_rx(), please move the stats handling before calling netif_rx(). Same applies to the kvaser_usb_rx_can_msg() function.
> 

Indeed, these can be totally bogus values after netif_rx().
I'll introduce this as a new patch in a new series.

Thanks for your review!

Regards,
Darwish

^ permalink raw reply

* [RFC net-next 0/3] devconf: New infrastructure for setting pre-load parameters for a module
From: Hadar Hen Zion @ 2015-01-08 15:16 UTC (permalink / raw)
  To: David S. Miller, gregkh
  Cc: netdev, Amir Vadai, Hadar Har-Zion, Yevgeny Petrilin, Or Gerlitz,
	shannon.nelson, Doug Ledford, greearb

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

^ permalink raw reply

* Re: [PATCH net-next v2] tcp: avoid reducing cwnd when ACK+DSACK is received
From: Eric Dumazet @ 2015-01-08 15:19 UTC (permalink / raw)
  To: Sébastien Barré
  Cc: David Miller, netdev, Gregory Detal, Nandita Dukkipati,
	Yuchung Cheng, Neal Cardwell
In-Reply-To: <1420719609-18638-1-git-send-email-sebastien.barre@uclouvain.be>

On Thu, 2015-01-08 at 13:20 +0100, Sébastien Barré wrote:
> When the peer has delayed ack enabled, it may reply to a probe with an
> ACK+D-SACK, with ack value set to tlp_high_seq. In the current code,
> such ACK+DSACK will be missed and only at next, higher ack will the TLP
> episode be considered done. Since the DSACK is not present anymore,
> this will cost a cwnd reduction.
> 
...
>  net/ipv4/tcp_input.c | 30 +++++++++++++-----------------
>  1 file changed, 13 insertions(+), 17 deletions(-)
> 
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index 075ab4d..cf63a29 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -3363,29 +3363,25 @@ static void tcp_replace_ts_recent(struct tcp_sock *tp, u32 seq)
>  static void tcp_process_tlp_ack(struct sock *sk, u32 ack, int flag)
>  {
>  	struct tcp_sock *tp = tcp_sk(sk);
> -	bool is_tlp_dupack = (ack == tp->tlp_high_seq) &&
> -			     !(flag & (FLAG_SND_UNA_ADVANCED |
> -				       FLAG_NOT_DUP | FLAG_DATA_SACKED));
>  

No idea why you removed this bool, it really helped to understand the
code. This makes your patch looks more complex than needed.

>  	/* Mark the end of TLP episode on receiving TLP dupack or when
>  	 * ack is after tlp_high_seq.
> +	 * With delayed acks, we may also get a regular ACK+DSACK, in which
> +	 * case we don't want to reduce the cwnd either.
>  	 */
> -	if (is_tlp_dupack) {
> +	if (((ack == tp->tlp_high_seq) &&
> +	     !(flag & (FLAG_SND_UNA_ADVANCED |
> +		       FLAG_NOT_DUP | FLAG_DATA_SACKED))) ||
> +	    (!before(ack, tp->tlp_high_seq) && (flag & FLAG_DSACKING_ACK))) {
>  		tp->tlp_high_seq = 0;
> -		return;
> -	}

^ permalink raw reply

* Re: [PATCH net-next v2] tcp: avoid reducing cwnd when ACK+DSACK is received
From: Sébastien Barré @ 2015-01-08 15:24 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David Miller, netdev, Gregory Detal, Nandita Dukkipati,
	Yuchung Cheng, Neal Cardwell
In-Reply-To: <1420729656.5947.50.camel@edumazet-glaptop2.roam.corp.google.com>

Hi Eric,

Le 08/01/2015 16:07, Eric Dumazet a écrit :
>
>>   In the current code,
>> such ACK+DSACK will be missed and only at next, higher ack will the TLP
>> episode be considered done. Since the DSACK is not present anymore,
>> this will cost a cwnd reduction.
>>
>> This patch ensures that this scenario does not cause a cwnd reduction, since
>> receiving an ACK+DSACK indicates that both the initial segment and the probe
>> have been received by the peer.
> Do you have at hand a packetdrill test to demonstrate that the patch
> works ?
Not currently, but that would be very good indeed.
I will prepare one and send it.

regards,

Sébastien.
>
> Thanks !
>
>

^ permalink raw reply

* [PATCH 10/15] batman-adv: checkpatch - Please don't use multiple blank lines
From: Antonio Quartulli @ 2015-01-08 15:15 UTC (permalink / raw)
  To: davem; +Cc: netdev, b.a.t.m.a.n, Antonio Quartulli, Marek Lindner
In-Reply-To: <1420730120-9844-1-git-send-email-antonio@meshcoding.com>

Signed-off-by: Antonio Quartulli <antonio@meshcoding.com>
Signed-off-by: Marek Lindner <mareklindner@neomailbox.ch>
---
 net/batman-adv/bat_iv_ogm.c            | 2 --
 net/batman-adv/bitarray.c              | 1 -
 net/batman-adv/bridge_loop_avoidance.c | 7 -------
 net/batman-adv/fragmentation.c         | 1 -
 net/batman-adv/main.c                  | 1 -
 net/batman-adv/originator.c            | 1 -
 net/batman-adv/originator.h            | 1 -
 net/batman-adv/routing.c               | 1 -
 net/batman-adv/soft-interface.c        | 1 -
 net/batman-adv/sysfs.c                 | 1 -
 net/batman-adv/translation-table.c     | 1 -
 11 files changed, 18 deletions(-)

diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c
index 73209f3..4dd7c40 100644
--- a/net/batman-adv/bat_iv_ogm.c
+++ b/net/batman-adv/bat_iv_ogm.c
@@ -26,7 +26,6 @@
 #include "bat_algo.h"
 #include "network-coding.h"
 
-
 /**
  * enum batadv_dup_status - duplicate status
  * @BATADV_NO_DUP: the packet is a duplicate
@@ -1362,7 +1361,6 @@ out:
 	return ret;
 }
 
-
 /**
  * batadv_iv_ogm_process_per_outif - process a batman iv OGM for an outgoing if
  * @skb: the skb containing the OGM
diff --git a/net/batman-adv/bitarray.c b/net/batman-adv/bitarray.c
index 9586750..e3da07a 100644
--- a/net/batman-adv/bitarray.c
+++ b/net/batman-adv/bitarray.c
@@ -29,7 +29,6 @@ static void batadv_bitmap_shift_left(unsigned long *seq_bits, int32_t n)
 	bitmap_shift_left(seq_bits, seq_bits, n, BATADV_TQ_LOCAL_WINDOW_SIZE);
 }
 
-
 /* receive and process one packet within the sequence number window.
  *
  * returns:
diff --git a/net/batman-adv/bridge_loop_avoidance.c b/net/batman-adv/bridge_loop_avoidance.c
index 0f0ca43..4fc6cab 100644
--- a/net/batman-adv/bridge_loop_avoidance.c
+++ b/net/batman-adv/bridge_loop_avoidance.c
@@ -69,7 +69,6 @@ static inline uint32_t batadv_choose_backbone_gw(const void *data,
 	return hash % size;
 }
 
-
 /* compares address and vid of two backbone gws */
 static int batadv_compare_backbone_gw(const struct hlist_node *node,
 				      const void *data2)
@@ -662,7 +661,6 @@ static int batadv_handle_announce(struct batadv_priv *bat_priv,
 	if (unlikely(!backbone_gw))
 		return 1;
 
-
 	/* handle as ANNOUNCE frame */
 	backbone_gw->lasttime = jiffies;
 	crc = ntohs(*((__be16 *)(&an_addr[4])));
@@ -849,7 +847,6 @@ static int batadv_check_claim_group(struct batadv_priv *bat_priv,
 	return 2;
 }
 
-
 /**
  * batadv_bla_process_claim
  * @bat_priv: the bat priv with all the soft interface information
@@ -1347,8 +1344,6 @@ out:
 	return ret;
 }
 
-
-
 /**
  * batadv_bla_is_backbone_gw_orig
  * @bat_priv: the bat priv with all the soft interface information
@@ -1390,7 +1385,6 @@ bool batadv_bla_is_backbone_gw_orig(struct batadv_priv *bat_priv, uint8_t *orig,
 	return false;
 }
 
-
 /**
  * batadv_bla_is_backbone_gw
  * @skb: the frame to be checked
@@ -1480,7 +1474,6 @@ int batadv_bla_rx(struct batadv_priv *bat_priv, struct sk_buff *skb,
 	if (!atomic_read(&bat_priv->bridge_loop_avoidance))
 		goto allow;
 
-
 	if (unlikely(atomic_read(&bat_priv->bla.num_requests)))
 		/* don't allow broadcasts while requests are in flight */
 		if (is_multicast_ether_addr(ethhdr->h_dest) && is_bcast)
diff --git a/net/batman-adv/fragmentation.c b/net/batman-adv/fragmentation.c
index 00f9e14..3d1dcaa 100644
--- a/net/batman-adv/fragmentation.c
+++ b/net/batman-adv/fragmentation.c
@@ -23,7 +23,6 @@
 #include "hard-interface.h"
 #include "soft-interface.h"
 
-
 /**
  * batadv_frag_clear_chain - delete entries in the fragment buffer chain
  * @head: head of chain with entries.
diff --git a/net/batman-adv/main.c b/net/batman-adv/main.c
index 2cdd25a..d4079cd 100644
--- a/net/batman-adv/main.c
+++ b/net/batman-adv/main.c
@@ -41,7 +41,6 @@
 #include "network-coding.h"
 #include "fragmentation.h"
 
-
 /* List manipulations on hardif_list have to be rtnl_lock()'ed,
  * list traversals just rcu-locked
  */
diff --git a/net/batman-adv/originator.c b/net/batman-adv/originator.c
index bea8198..90e805a 100644
--- a/net/batman-adv/originator.c
+++ b/net/batman-adv/originator.c
@@ -797,7 +797,6 @@ batadv_purge_orig_ifinfo(struct batadv_priv *bat_priv,
 	return ifinfo_purged;
 }
 
-
 /**
  * batadv_purge_orig_neighbors - purges neighbors from originator
  * @bat_priv: the bat priv with all the soft interface information
diff --git a/net/batman-adv/originator.h b/net/batman-adv/originator.h
index db3a9ed..aa4a436 100644
--- a/net/batman-adv/originator.h
+++ b/net/batman-adv/originator.h
@@ -70,7 +70,6 @@ batadv_orig_node_vlan_get(struct batadv_orig_node *orig_node,
 			  unsigned short vid);
 void batadv_orig_node_vlan_free_ref(struct batadv_orig_node_vlan *orig_vlan);
 
-
 /* hashfunction to choose an entry in a hash table of given size
  * hash algorithm from http://en.wikipedia.org/wiki/Hash_table
  */
diff --git a/net/batman-adv/routing.c b/net/batman-adv/routing.c
index 6648f32..139d2f6 100644
--- a/net/batman-adv/routing.c
+++ b/net/batman-adv/routing.c
@@ -292,7 +292,6 @@ out:
 	return ret;
 }
 
-
 int batadv_recv_icmp_packet(struct sk_buff *skb,
 			    struct batadv_hard_iface *recv_if)
 {
diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c
index 5467955..5ec31d7 100644
--- a/net/batman-adv/soft-interface.c
+++ b/net/batman-adv/soft-interface.c
@@ -36,7 +36,6 @@
 #include "bridge_loop_avoidance.h"
 #include "network-coding.h"
 
-
 static int batadv_get_settings(struct net_device *dev, struct ethtool_cmd *cmd);
 static void batadv_get_drvinfo(struct net_device *dev,
 			       struct ethtool_drvinfo *info);
diff --git a/net/batman-adv/sysfs.c b/net/batman-adv/sysfs.c
index f40cb04..a75dc12 100644
--- a/net/batman-adv/sysfs.c
+++ b/net/batman-adv/sysfs.c
@@ -151,7 +151,6 @@ ssize_t batadv_show_##_name(struct kobject *kobj,			\
 	static BATADV_ATTR(_name, _mode, batadv_show_##_name,		\
 			   batadv_store_##_name)
 
-
 #define BATADV_ATTR_SIF_STORE_UINT(_name, _min, _max, _post_func)	\
 ssize_t batadv_store_##_name(struct kobject *kobj,			\
 			     struct attribute *attr, char *buff,	\
diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index 38a804e..84e6f01 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -1780,7 +1780,6 @@ static void batadv_tt_global_del(struct batadv_priv *bat_priv,
 		batadv_tt_global_del_roaming(bat_priv, tt_global_entry,
 					     orig_node, message);
 
-
 out:
 	if (tt_global_entry)
 		batadv_tt_global_entry_free_ref(tt_global_entry);
-- 
2.2.1

^ permalink raw reply related

* [PATCH 13/15] batman-adv: fix misspelled words
From: Antonio Quartulli @ 2015-01-08 15:15 UTC (permalink / raw)
  To: davem; +Cc: netdev, b.a.t.m.a.n, Antonio Quartulli, Marek Lindner
In-Reply-To: <1420730120-9844-1-git-send-email-antonio@meshcoding.com>

Reported-by: checkpatch
Signed-off-by: Antonio Quartulli <antonio@meshcoding.com>
Signed-off-by: Marek Lindner <mareklindner@neomailbox.ch>
---
 net/batman-adv/bridge_loop_avoidance.c | 4 ++--
 net/batman-adv/main.c                  | 2 +-
 net/batman-adv/packet.h                | 4 ++--
 net/batman-adv/routing.c               | 2 +-
 net/batman-adv/translation-table.c     | 2 +-
 net/batman-adv/types.h                 | 2 +-
 6 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/net/batman-adv/bridge_loop_avoidance.c b/net/batman-adv/bridge_loop_avoidance.c
index 4fc6cab..ac4b96e 100644
--- a/net/batman-adv/bridge_loop_avoidance.c
+++ b/net/batman-adv/bridge_loop_avoidance.c
@@ -244,7 +244,7 @@ batadv_bla_del_backbone_claims(struct batadv_bla_backbone_gw *backbone_gw)
 		spin_unlock_bh(list_lock);
 	}
 
-	/* all claims gone, intialize CRC */
+	/* all claims gone, initialize CRC */
 	backbone_gw->crc = BATADV_BLA_CRC_INIT;
 }
 
@@ -1328,7 +1328,7 @@ int batadv_bla_check_bcast_duplist(struct batadv_priv *bat_priv,
 		goto out;
 	}
 	/* not found, add a new entry (overwrite the oldest entry)
-	 * and allow it, its the first occurence.
+	 * and allow it, its the first occurrence.
 	 */
 	curr = (bat_priv->bla.bcast_duplist_curr + BATADV_DUPLIST_SIZE - 1);
 	curr %= BATADV_DUPLIST_SIZE;
diff --git a/net/batman-adv/main.c b/net/batman-adv/main.c
index 3bcd847..12fc77b 100644
--- a/net/batman-adv/main.c
+++ b/net/batman-adv/main.c
@@ -798,7 +798,7 @@ void batadv_tvlv_container_register(struct batadv_priv *bat_priv,
 }
 
 /**
- * batadv_tvlv_realloc_packet_buff - reallocate packet buffer to accomodate
+ * batadv_tvlv_realloc_packet_buff - reallocate packet buffer to accommodate
  *  requested packet size
  * @packet_buff: packet buffer
  * @packet_buff_len: packet buffer size
diff --git a/net/batman-adv/packet.h b/net/batman-adv/packet.h
index facd1fe..b81fbbf 100644
--- a/net/batman-adv/packet.h
+++ b/net/batman-adv/packet.h
@@ -377,7 +377,7 @@ struct batadv_frag_packet {
 	uint8_t reserved:4;
 	uint8_t no:4;
 #else
-#error "unknown bitfield endianess"
+#error "unknown bitfield endianness"
 #endif
 	uint8_t dest[ETH_ALEN];
 	uint8_t orig[ETH_ALEN];
@@ -453,7 +453,7 @@ struct batadv_coded_packet {
  * @src: address of the source
  * @dst: address of the destination
  * @tvlv_len: length of tvlv data following the unicast tvlv header
- * @align: 2 bytes to align the header to a 4 byte boundry
+ * @align: 2 bytes to align the header to a 4 byte boundary
  */
 struct batadv_unicast_tvlv_packet {
 	uint8_t  packet_type;
diff --git a/net/batman-adv/routing.c b/net/batman-adv/routing.c
index 139d2f6..da83982 100644
--- a/net/batman-adv/routing.c
+++ b/net/batman-adv/routing.c
@@ -456,7 +456,7 @@ batadv_find_router(struct batadv_priv *bat_priv,
 	 * the last chosen bonding candidate (next_candidate). If no such
 	 * router is found, use the first candidate found (the previously
 	 * chosen bonding candidate might have been the last one in the list).
-	 * If this can't be found either, return the previously choosen
+	 * If this can't be found either, return the previously chosen
 	 * router - obviously there are no other candidates.
 	 */
 	rcu_read_lock();
diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index 84e6f01..07b263a 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -2852,7 +2852,7 @@ static void batadv_tt_update_changes(struct batadv_priv *bat_priv,
 /**
  * batadv_is_my_client - check if a client is served by the local node
  * @bat_priv: the bat priv with all the soft interface information
- * @addr: the mac adress of the client to check
+ * @addr: the mac address of the client to check
  * @vid: VLAN identifier
  *
  * Returns true if the client is served by this node, false otherwise.
diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
index 462a70c..9398c3f 100644
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -968,7 +968,7 @@ struct batadv_tt_orig_list_entry {
 };
 
 /**
- * struct batadv_tt_change_node - structure for tt changes occured
+ * struct batadv_tt_change_node - structure for tt changes occurred
  * @list: list node for batadv_priv_tt::changes_list
  * @change: holds the actual translation table diff data
  */
-- 
2.2.1

^ permalink raw reply related

* [PATCH 11/15] batman-adv: checkpatch - remove unnecessary parentheses
From: Antonio Quartulli @ 2015-01-08 15:15 UTC (permalink / raw)
  To: davem; +Cc: netdev, b.a.t.m.a.n, Antonio Quartulli, Marek Lindner
In-Reply-To: <1420730120-9844-1-git-send-email-antonio@meshcoding.com>

Signed-off-by: Antonio Quartulli <antonio@meshcoding.com>
Signed-off-by: Marek Lindner <mareklindner@neomailbox.ch>
---
 net/batman-adv/bat_iv_ogm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c
index 4dd7c40..00e00e0 100644
--- a/net/batman-adv/bat_iv_ogm.c
+++ b/net/batman-adv/bat_iv_ogm.c
@@ -878,7 +878,7 @@ batadv_iv_ogm_slide_own_bcast_window(struct batadv_hard_iface *hard_iface)
 		hlist_for_each_entry_rcu(orig_node, head, hash_entry) {
 			spin_lock_bh(&orig_node->bat_iv.ogm_cnt_lock);
 			word_index = hard_iface->if_num * BATADV_NUM_WORDS;
-			word = &(orig_node->bat_iv.bcast_own[word_index]);
+			word = &orig_node->bat_iv.bcast_own[word_index];
 
 			batadv_bit_get_packet(bat_priv, word, 1, 0);
 			if_num = hard_iface->if_num;
@@ -1663,7 +1663,7 @@ static void batadv_iv_ogm_process(const struct sk_buff *skb, int ogm_offset,
 			offset = if_num * BATADV_NUM_WORDS;
 
 			spin_lock_bh(&orig_neigh_node->bat_iv.ogm_cnt_lock);
-			word = &(orig_neigh_node->bat_iv.bcast_own[offset]);
+			word = &orig_neigh_node->bat_iv.bcast_own[offset];
 			bit_pos = if_incoming_seqno - 2;
 			bit_pos -= ntohl(ogm_packet->seqno);
 			batadv_set_bit(word, bit_pos);
-- 
2.2.1

^ permalink raw reply related

* [PATCH 12/15] batman-adv: clear control block of received socket buffers
From: Antonio Quartulli @ 2015-01-08 15:15 UTC (permalink / raw)
  To: davem
  Cc: netdev, b.a.t.m.a.n, Martin Hundebøll, Marek Lindner,
	Antonio Quartulli
In-Reply-To: <1420730120-9844-1-git-send-email-antonio@meshcoding.com>

From: Martin Hundebøll <martin@hundeboll.net>

Since other network components (and some drivers) uses the control block
provided in skb's, the network coding feature might wrongly assume that
an SKB has been decoded, and thus not try to code it with another packet
again. This happens for instance when batman-adv is running on a bridge device.

Fix this by clearing the control block for every received SKB.

Introduced by 3c12de9a5c756b23fe7c9ab332474ece1568914c
("batman-adv: network coding - code and transmit packets if possible")
Signed-off-by: Martin Hundebøll <martin@hundeboll.net>
Signed-off-by: Marek Lindner <mareklindner@neomailbox.ch>

Signed-off-by: Antonio Quartulli <antonio@meshcoding.com>
---
 net/batman-adv/main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/batman-adv/main.c b/net/batman-adv/main.c
index d4079cd..3bcd847 100644
--- a/net/batman-adv/main.c
+++ b/net/batman-adv/main.c
@@ -402,6 +402,9 @@ int batadv_batman_skb_recv(struct sk_buff *skb, struct net_device *dev,
 		goto err_free;
 	}
 
+	/* reset control block to avoid left overs from previous users */
+	memset(skb->cb, 0, sizeof(struct batadv_skb_cb));
+
 	/* all receive handlers return whether they received or reused
 	 * the supplied skb. if not, we have to free the skb.
 	 */
-- 
2.2.1

^ permalink raw reply related

* Re: [PATCH net-next v2] tcp: avoid reducing cwnd when ACK+DSACK is received
From: Sébastien Barré @ 2015-01-08 15:39 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David Miller, netdev, Gregory Detal, Nandita Dukkipati,
	Yuchung Cheng, Neal Cardwell
In-Reply-To: <1420730396.5947.55.camel@edumazet-glaptop2.roam.corp.google.com>


Le 08/01/2015 16:19, Eric Dumazet a écrit :
> On Thu, 2015-01-08 at 13:20 +0100, Sébastien Barré wrote:
>> When the peer has delayed ack enabled, it may reply to a probe with an
>> ACK+D-SACK, with ack value set to tlp_high_seq. In the current code,
>> such ACK+DSACK will be missed and only at next, higher ack will the TLP
>> episode be considered done. Since the DSACK is not present anymore,
>> this will cost a cwnd reduction.
>>
> ...
>>   net/ipv4/tcp_input.c | 30 +++++++++++++-----------------
>>   1 file changed, 13 insertions(+), 17 deletions(-)
>>
>> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
>> index 075ab4d..cf63a29 100644
>> --- a/net/ipv4/tcp_input.c
>> +++ b/net/ipv4/tcp_input.c
>> @@ -3363,29 +3363,25 @@ static void tcp_replace_ts_recent(struct tcp_sock *tp, u32 seq)
>>   static void tcp_process_tlp_ack(struct sock *sk, u32 ack, int flag)
>>   {
>>   	struct tcp_sock *tp = tcp_sk(sk);
>> -	bool is_tlp_dupack = (ack == tp->tlp_high_seq) &&
>> -			     !(flag & (FLAG_SND_UNA_ADVANCED |
>> -				       FLAG_NOT_DUP | FLAG_DATA_SACKED));
>>   
> No idea why you removed this bool, it really helped to understand the
> code. This makes your patch looks more complex than needed.
I did not remove it in v1 
(http://www.spinics.net/lists/netdev/msg308653.html)
The removal was a request from Neal 
(http://www.spinics.net/lists/netdev/msg308758.html)

I think he found it special to have part of the logic apart, in a bool, 
and part of it in the if condition.
One possible option is to restore is_tlp_dupack and include the DSACK 
check in it, although this will
not do much more than moving complexity in the bool definition. But 
indeed, that might make
the patch more readable.

What do you and Neal think ?

regards,

Sébastien.
>
>>   	/* Mark the end of TLP episode on receiving TLP dupack or when
>>   	 * ack is after tlp_high_seq.
>> +	 * With delayed acks, we may also get a regular ACK+DSACK, in which
>> +	 * case we don't want to reduce the cwnd either.
>>   	 */
>> -	if (is_tlp_dupack) {
>> +	if (((ack == tp->tlp_high_seq) &&
>> +	     !(flag & (FLAG_SND_UNA_ADVANCED |
>> +		       FLAG_NOT_DUP | FLAG_DATA_SACKED))) ||
>> +	    (!before(ack, tp->tlp_high_seq) && (flag & FLAG_DSACKING_ACK))) {
>>   		tp->tlp_high_seq = 0;
>> -		return;
>> -	}
>

^ permalink raw reply

* Re: [PATCH net-next v2] tcp: avoid reducing cwnd when ACK+DSACK is received
From: Neal Cardwell @ 2015-01-08 15:43 UTC (permalink / raw)
  To: Sébastien Barré
  Cc: Eric Dumazet, David Miller, Netdev, Gregory Detal,
	Nandita Dukkipati, Yuchung Cheng
In-Reply-To: <54AEA11A.1000603@uclouvain.be>

> Le 08/01/2015 16:07, Eric Dumazet a écrit :
>> Do you have at hand a packetdrill test to demonstrate that the patch
>> works ?

I cooked up the packetdrill test below when Sebastien sent out his v1
a few weeks ago. It fails on a kernel without his patch, and passes on
a kernel with his patch.

The code change looks fine to me, but if Eric prefers that the
expression be assigned to a bool before the check, that also sounds
fine to me.

neal


------------
// Establish a connection.
0     socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
+0     setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
+0    bind(3, ..., ...) = 0
+0    listen(3, 1) = 0

+0    < S 0:0(0) win 32792 <mss 1000,sackOK,nop,nop,nop,wscale 7>
+0    > S. 0:0(0) ack 1 <mss 1460,nop,nop,sackOK,nop,wscale 6>
+.020 < . 1:1(0) ack 1 win 257
+0    accept(3, ..., ...) = 4

// Send 1 packet.
+0    write(4, ..., 1000) = 1000
+0    > P. 1:1001(1000) ack 1

// Loss probe retransmission.
// packets_out == 1 => schedule PTO in max(2*RTT, 1.5*RTT + 200ms)
// In this case, this means: 1.5*RTT + 200ms = 230ms
+.230 > P. 1:1001(1000) ack 1
+0    %{ assert tcpi_snd_cwnd == 10 }%

// Receiver ACKs at tlp_high_seq with a DSACK,
// indicating they received the original packet and probe.
+.020 < . 1:1(0) ack 1001 win 257 <sack 1:1001,nop,nop>
+0    %{ assert tcpi_snd_cwnd == 10 }%

// Send another packet.
+0    write(4, ..., 1000) = 1000
+0    > P. 1001:2001(1000) ack 1

// Receiver ACKs above tlp_high_seq, which should end the TLP episode
// if we haven't already. We should not reduce cwnd.
+.020 < . 1:1(0) ack 2001 win 257
+0    %{ assert tcpi_snd_cwnd == 10, tcpi_snd_cwnd }%

^ permalink raw reply

* Re: [PATCH net-next v2] tcp: avoid reducing cwnd when ACK+DSACK is received
From: Neal Cardwell @ 2015-01-08 15:49 UTC (permalink / raw)
  To: Sébastien Barré
  Cc: Eric Dumazet, David Miller, Netdev, Gregory Detal,
	Nandita Dukkipati, Yuchung Cheng
In-Reply-To: <54AEA498.5030202@uclouvain.be>

On Thu, Jan 8, 2015 at 10:39 AM, Sébastien Barré
<sebastien.barre@uclouvain.be> wrote:
> What do you and Neal think ?

My preference is to have the whole expression detecting the case where
the receiver got the probe packet encoded in a single expression. I
don't have a strong feeling about whether it should be stored in a
bool (to reduce the size of the diff) or written directly into the if
() expression (to reduce the size of the code). I'll defer to Eric on
which he thinks is better. :-)

neal

^ permalink raw reply

* Re: [PATCH net-next v2] tcp: avoid reducing cwnd when ACK+DSACK is received
From: Neal Cardwell @ 2015-01-08 15:52 UTC (permalink / raw)
  To: Sébastien Barré
  Cc: Eric Dumazet, David Miller, Netdev, Gregory Detal,
	Nandita Dukkipati, Yuchung Cheng
In-Reply-To: <CADVnQy=A57TUYD=kGtZXvq7-FGSN=vN3D2heaNCnBB=zjujwUg@mail.gmail.com>

On Thu, Jan 8, 2015 at 10:49 AM, Neal Cardwell <ncardwell@google.com> wrote:
> On Thu, Jan 8, 2015 at 10:39 AM, Sébastien Barré
> <sebastien.barre@uclouvain.be> wrote:
>> What do you and Neal think ?
>
> My preference is to have the whole expression detecting the case where
> the receiver got the probe packet encoded in a single expression.

If we do store it in a bool, then "is_tlp_dupack" is no longer quite
accurate (we're expanding the check to include ACKs that are not
dupacks), so I'd suggest a name like "is_probe_rcvd".

neal

^ permalink raw reply

* Re: [PATCH net-next v2] tcp: avoid reducing cwnd when ACK+DSACK is received
From: Eric Dumazet @ 2015-01-08 15:59 UTC (permalink / raw)
  To: Sébastien Barré
  Cc: David Miller, netdev, Gregory Detal, Nandita Dukkipati,
	Yuchung Cheng, Neal Cardwell
In-Reply-To: <54AEA11A.1000603@uclouvain.be>

On Thu, 2015-01-08 at 16:24 +0100, Sébastien Barré wrote:
> Hi Eric,
> 
> Le 08/01/2015 16:07, Eric Dumazet a écrit :
> >
> >>   In the current code,
> >> such ACK+DSACK will be missed and only at next, higher ack will the TLP
> >> episode be considered done. Since the DSACK is not present anymore,
> >> this will cost a cwnd reduction.
> >>
> >> This patch ensures that this scenario does not cause a cwnd reduction, since
> >> receiving an ACK+DSACK indicates that both the initial segment and the probe
> >> have been received by the peer.
> > Do you have at hand a packetdrill test to demonstrate that the patch
> > works ?
> Not currently, but that would be very good indeed.
> I will prepare one and send it.

Here is a skeleton you might adapt :

// TLP test when a sender does not have any new segments for transmission.
// In the absence of ACKs, the sender retransmits an old segment as a probe
// after 2 RTTs.

// Tolerate RTO variations
--tolerance_usecs=15000

// Set up production config.
`../common/defaults.sh
sysctl -q net.ipv4.tcp_early_retrans=3
`

// Establish a connection.
0.000 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
0.000 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
0.000 bind(3, ..., ...) = 0
0.000 listen(3, 1) = 0

0.100  < S 0:0(0) win 32792 <mss 1000,sackOK,nop,nop,nop,wscale 7>
+0     > S. 0:0(0) ack 1 <mss 1460,nop,nop,sackOK,nop,wscale 6>
+0.100 < . 1:1(0) ack 1 win 257
+0     accept(3, ..., ...) = 4

// Send 10 MSS.
+0.100 write(4, ..., 10000) = 10000
+0 > . 1:5001(5000) ack 1
+0 > P. 5001:10001(5000) ack 1

// TLP retransmission in 2 RTTs.
+0.200 > P. 9001:10001(1000) ack 1
+0 %{
assert tcpi_snd_cwnd == 10, 'tcpi_snd_cwnd=%d' % tcpi_snd_cwnd
assert tcpi_unacked == 10
}%

+0.100 < . 1:1(0) ack 10001 win 257 <sack 9001:1001,nop,nop>

+0 %{
assert tcpi_snd_cwnd == 20, 'tcpi_snd_cwnd=%d' % tcpi_snd_cwnd
assert tcpi_unacked == 0, 'tcpi_unacked=%d' % tcpi_unacked
}%

^ permalink raw reply

* Re: [PATCH net-next v2] tcp: avoid reducing cwnd when ACK+DSACK is received
From: Sébastien Barré @ 2015-01-08 16:00 UTC (permalink / raw)
  To: Neal Cardwell
  Cc: Eric Dumazet, David Miller, Netdev, Gregory Detal,
	Nandita Dukkipati, Yuchung Cheng
In-Reply-To: <CADVnQy=kACmKGkakx85BHWg7WppS3-B1yLa-TXvfBKbXtXYJ+A@mail.gmail.com>


Le 08/01/2015 16:43, Neal Cardwell a écrit :
>> Le 08/01/2015 16:07, Eric Dumazet a écrit :
>>> Do you have at hand a packetdrill test to demonstrate that the patch
>>> works ?
> I cooked up the packetdrill test below when Sebastien sent out his v1
> a few weeks ago. It fails on a kernel without his patch, and passes on
> a kernel with his patch.
Thanks a lot for this ! I will include it in our test suite then.
I understand that there is convergence on having a
bool called is_probe_rcvd with the whole logic.
Eric, is this ok for you ?

Thanks,

Sébastien.
>
> The code change looks fine to me, but if Eric prefers that the
> expression be assigned to a bool before the check, that also sounds
> fine to me.
>
> neal
>
>
> ------------
> // Establish a connection.
> 0     socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
> +0     setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
> +0    bind(3, ..., ...) = 0
> +0    listen(3, 1) = 0
>
> +0    < S 0:0(0) win 32792 <mss 1000,sackOK,nop,nop,nop,wscale 7>
> +0    > S. 0:0(0) ack 1 <mss 1460,nop,nop,sackOK,nop,wscale 6>
> +.020 < . 1:1(0) ack 1 win 257
> +0    accept(3, ..., ...) = 4
>
> // Send 1 packet.
> +0    write(4, ..., 1000) = 1000
> +0    > P. 1:1001(1000) ack 1
>
> // Loss probe retransmission.
> // packets_out == 1 => schedule PTO in max(2*RTT, 1.5*RTT + 200ms)
> // In this case, this means: 1.5*RTT + 200ms = 230ms
> +.230 > P. 1:1001(1000) ack 1
> +0    %{ assert tcpi_snd_cwnd == 10 }%
>
> // Receiver ACKs at tlp_high_seq with a DSACK,
> // indicating they received the original packet and probe.
> +.020 < . 1:1(0) ack 1001 win 257 <sack 1:1001,nop,nop>
> +0    %{ assert tcpi_snd_cwnd == 10 }%
>
> // Send another packet.
> +0    write(4, ..., 1000) = 1000
> +0    > P. 1001:2001(1000) ack 1
>
> // Receiver ACKs above tlp_high_seq, which should end the TLP episode
> // if we haven't already. We should not reduce cwnd.
> +.020 < . 1:1(0) ack 2001 win 257
> +0    %{ assert tcpi_snd_cwnd == 10, tcpi_snd_cwnd }%

^ permalink raw reply

* Re: [PATCH net-next v2] tcp: avoid reducing cwnd when ACK+DSACK is received
From: Eric Dumazet @ 2015-01-08 16:25 UTC (permalink / raw)
  To: Neal Cardwell
  Cc: Sébastien Barré, David Miller, Netdev, Gregory Detal,
	Nandita Dukkipati, Yuchung Cheng
In-Reply-To: <CADVnQy=A57TUYD=kGtZXvq7-FGSN=vN3D2heaNCnBB=zjujwUg@mail.gmail.com>

On Thu, 2015-01-08 at 10:49 -0500, Neal Cardwell wrote:
> On Thu, Jan 8, 2015 at 10:39 AM, Sébastien Barré
> <sebastien.barre@uclouvain.be> wrote:
> > What do you and Neal think ?
> 
> My preference is to have the whole expression detecting the case where
> the receiver got the probe packet encoded in a single expression. I
> don't have a strong feeling about whether it should be stored in a
> bool (to reduce the size of the diff) or written directly into the if
> () expression (to reduce the size of the code). I'll defer to Eric on
> which he thinks is better. :-)

There is no shame using helpers with nice names to help understand this
TCP stack. Even if the helper is used exactly once.

In this case, it seems we test 2 different conditions, so this could use
2 helpers with self describing names.

When I see : 

> +     if (((ack == tp->tlp_high_seq) &&
> +          !(flag & (FLAG_SND_UNA_ADVANCED |
> +                    FLAG_NOT_DUP | FLAG_DATA_SACKED))) ||
> +         (!before(ack, tp->tlp_high_seq) && (flag & FLAG_DSACKING_ACK))) {
>               tp->tlp_high_seq = 0;

My brain hurts.

^ permalink raw reply

* [PATCH net-next] packet: make packet too small warning match condition
From: Willem de Bruijn @ 2015-01-08 16:29 UTC (permalink / raw)
  To: netdev; +Cc: davem, jkmalinen, dborkman, Willem de Bruijn

From: Willem de Bruijn <willemb@google.com>

The expression in ll_header_truncated() tests less than or equal, but
the warning prints less than. Update the warning.

Reported-by: Jouni Malinen <jkmalinen@gmail.com>
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
 net/packet/af_packet.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 6880f34..0f02668 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -2102,7 +2102,7 @@ static bool ll_header_truncated(const struct net_device *dev, int len)
 {
 	/* net device doesn't like empty head */
 	if (unlikely(len <= dev->hard_header_len)) {
-		net_warn_ratelimited("%s: packet size is too short (%d < %d)\n",
+		net_warn_ratelimited("%s: packet size is too short (%d <= %d)\n",
 				     current->comm, len, dev->hard_header_len);
 		return true;
 	}
-- 
2.2.0.rc0.207.ga3a616c

^ permalink raw reply related

* Re: [RFC net-next 0/3] devconf: New infrastructure for setting pre-load parameters for a module
From: Greg KH @ 2015-01-08 16:46 UTC (permalink / raw)
  To: Hadar Hen Zion
  Cc: David S. Miller, netdev, Amir Vadai, Yevgeny Petrilin, Or Gerlitz,
	shannon.nelson, Doug Ledford, greearb
In-Reply-To: <1420730220-20224-1-git-send-email-hadarh@mellanox.com>

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

^ permalink raw reply

* Re: [RFC net-next 0/3] devconf: New infrastructure for setting pre-load parameters for a module
From: Amir Vadai @ 2015-01-08 17:11 UTC (permalink / raw)
  To: Greg KH, Hadar Hen Zion
  Cc: David S. Miller, netdev@vger.kernel.org, Yevgeny Petrilin,
	Or Gerlitz, shannon.nelson@intel.com, Doug Ledford,
	greearb@candelatech.com
In-Reply-To: <20150108164618.GA11696@kroah.com>

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
> 

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox