[PATCH 0/3] ixgbe: request_firmware for configuration parameters

[PATCH 0/3] ixgbe: request_firmware for configuration parameters

[Shannon Nelson]

Most networking dials and knobs can be set using ethtool, ifconfig, ip link
commands, or sysfs entries, all of which can be driven by startup scripts
and other configuration tools.  However, they all depend on having a netdev
already set up, and we have some low-level device functionality that needs
to be sorted out before we start setting up MSI-x and memory allocations.

In order to do early device configuration, most kernel drivers use module
parameters whose settings can be persisted in modprobe.d config files.
However, these can be clumsy to use and manage, difficult to specify port
specific values in a multiport device, and are actively discouraged in
some circles.

In this patchset, the driver uses the existing request_firmware() and
match_token() facilities to grab an ASCII config file from userspace to
find special startup-time configuration information that needs persistence
across reboots.  The configuration strings are formed similar to the
mount options that get passed from /etc/fstab into filesystem modules.

We can assume that the driver and device will come up with sane defaults
that will make the part reasonably useful - we're not solving something
for basic usability.  This capability is for folks that need special
configurations for their virtualization server or cloud configuration or
whatever network server where they've taken the time to tune things more
specifically to their application.

We are using existing facilities so that we don't have to build any
userland utilities, and a config file format that is both humanly and
potentially mechanically editable.

After doing some digging around, it seems that the combination of ASCII
config files stored where request_firmware() can find them, plus the
kind of option configurations used in fstab for mount commands can solve
the need.  Using the lib/parser.c tools seems obvious - we don't want to
add any other parser code to the kernel, and heaven forbid someone tries to
extend any new parser into an XML solution.  Using the request_firmware()
framework also allows the configuration file to get included when building
an initrd image.

In these RFC patches for ixgbe, the configuration data would be found in
the file /lib/firmware/ixgbe.conf, and might look something like this:

	#
	# example ixgbe driver configuration
	#

		# this is a generic setting for the driver
	ixgbe allow_unsupported_sfp,debug=0xf,AtrSampleRate=0

		# msix restricted on one port for testing purposes
	00:1b:21:12:4e:60  nomsix


Signed-off-by: Shannon Nelson <shannon.nelson@intel.com>
Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Cc: David Woodhouse <dwmw2@infradead.org>

---

Shannon Nelson (3):
      ixgbe: add interrupt control parameters
      ixgbe: add additional parameter options
      ixgbe: replace module options with configuration through request_firmware


 drivers/net/ethernet/intel/ixgbe/ixgbe.h      |    4 
 drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c  |    6 
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  305 +++++++++++++++++++++++--
 3 files changed, 289 insertions(+), 26 deletions(-)

-- 
======================================================================
Mr. Shannon Nelson                 LAN Access Division, Intel Corp.
shannon.nelson@intel.com                I don't speak for Intel
(503) 712-7659                    Parents can't afford to be squeamish

[PATCH 1/3] ixgbe: replace module options with configuration through request_firmware

[Shannon Nelson]

Replace the use of module parameters with data read from an ASCII parameter
file found through the request_firmware() framework.

The parameter file is an ASCII data file with lines in the form of
     <label>  <option>=<val>[,<option>=<val>...]
where the <label> specifies the driver name and/or a specific configuration
line to use and the following options define that configuration.  Blank
lines are ignored, as are line comments that start with '#'.

The parameter file is tagged as MODULE_FIRMWARE to be sure it gets
included when pulling the driver into an initrd image.  The possible
config options are tagged using MODULE_INFO to be sure there is still
some discoverability in configuration options.

Signed-off-by: Shannon Nelson <shannon.nelson@intel.com>
Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Cc: David Woodhouse <dwmw2@infradead.org>
---

 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  229 ++++++++++++++++++++++---
 1 files changed, 203 insertions(+), 26 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 20d6764..1670fc7 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -47,13 +47,16 @@
 #include <linux/if_bridge.h>
 #include <linux/prefetch.h>
 #include <scsi/fc/fc_fcoe.h>
+#include <linux/firmware.h>
+#include <linux/parser.h>
 
 #include "ixgbe.h"
 #include "ixgbe_common.h"
 #include "ixgbe_dcb_82599.h"
 #include "ixgbe_sriov.h"
 
-char ixgbe_driver_name[] = "ixgbe";
+#define IXGBE_DRIVER_NAME "ixgbe"
+char ixgbe_driver_name[] = IXGBE_DRIVER_NAME;
 static const char ixgbe_driver_string[] =
 			      "Intel(R) 10 Gigabit PCI Express Network Driver";
 #ifdef IXGBE_FCOE
@@ -127,28 +130,202 @@ static struct notifier_block dca_notifier = {
 };
 #endif
 
-#ifdef CONFIG_PCI_IOV
-static unsigned int max_vfs;
-module_param(max_vfs, uint, 0);
-MODULE_PARM_DESC(max_vfs,
-		 "Maximum number of virtual functions to allocate per physical function - default is zero and maximum value is 63");
-#endif /* CONFIG_PCI_IOV */
-
-static unsigned int allow_unsupported_sfp;
-module_param(allow_unsupported_sfp, uint, 0);
-MODULE_PARM_DESC(allow_unsupported_sfp,
-		 "Allow unsupported and untested SFP+ modules on 82599-based adapters");
-
 #define DEFAULT_MSG_ENABLE (NETIF_MSG_DRV|NETIF_MSG_PROBE|NETIF_MSG_LINK)
-static int debug = -1;
-module_param(debug, int, 0);
-MODULE_PARM_DESC(debug, "Debug level (0=none,...,16=all)");
 
 MODULE_AUTHOR("Intel Corporation, <linux.nics@intel.com>");
 MODULE_DESCRIPTION("Intel(R) 10 Gigabit PCI Express Network Driver");
 MODULE_LICENSE("GPL");
 MODULE_VERSION(DRV_VERSION);
 
+/*
+ * Firmware parameter processing
+ *
+ * This requests a parameter configuration file through the kernel
+ * firmware management service.  The parameter file is an ASCII data
+ * file with lines in the form of
+ *      <label>  <option>=<val>[,<option>=<val>...]
+ * where the <label> specifies a specific configuration to use
+ * and the following options define that configuration.  Blank lines
+ * are ignored, as are line comments that start with '#'.
+ *
+ * For this driver, we use the driver name as the label for the basic
+ * configuration, then use the port MAC address for port specific
+ * override configuration.
+ */
+#define IXGBE_FIRMWARE_FILE        IXGBE_DRIVER_NAME ".conf"
+MODULE_FIRMWARE(IXGBE_FIRMWARE_FILE);
+
+#define xstr(s) str(s)
+#define str(s)  #s
+
+enum {
+	Opt_debug,
+	Opt_allow_unsupported_sfp,
+#ifdef CONFIG_PCI_IOV
+	Opt_max_vfs,
+#endif
+};
+
+static const match_table_t tokens = {
+	{ Opt_debug, "debug=%u" },
+	{ Opt_allow_unsupported_sfp, "allow_unsupported_sfp" },
+#ifdef CONFIG_PCI_IOV
+	{ Opt_max_vfs, "max_vfs=%u" },
+#endif
+
+	/* terminator token */
+	{ 0, NULL },
+};
+MODULE_INFO(fw_option, "debug=N : Debug level (0=none,...,16=all)");
+MODULE_INFO(fw_option, "allow_unsupported_sfp : Allow unsupported and untested SFP+ modules on 82599-based adapters");
+MODULE_INFO(fw_option,
+	    "max_vfs=N : Maximum number of virtual functions per physical function (default=0) - 0 <= N < "
+	    xstr(IXGBE_MAX_VF_FUNCTIONS));
+
+/**
+ * ixgbe_parse_option_line - find ixgbe options
+ * @adapter: pointer to ixgbe_adapter
+ * @config_line: line of option information
+ * @line_len: length of the config line
+ *
+ **/
+static void ixgbe_parse_option_line(struct ixgbe_adapter *adapter,
+				    char *config_line, int line_len)
+{
+	char *p;
+	char *next_option = config_line;
+	substring_t args[MAX_OPT_ARGS];
+	int value;
+
+	if (!config_line)
+		return;
+
+	while ((p = strsep(&next_option, ", \t\n")) != NULL) {
+		int token;
+
+		if (((p - config_line) >= line_len) || *p == '\0' || *p == '#')
+			break;
+
+		/*
+		 * Initialize args struct so we know whether arg was
+		 * found; some options take optional arguments.
+		 */
+		args[0].to = args[0].from = NULL;
+		token = match_token(p, tokens, args);
+		switch (token) {
+
+		case Opt_debug:
+			if (match_int(args, &value))
+				goto parse_error;
+			adapter->msg_enable = netif_msg_init(value,
+							    DEFAULT_MSG_ENABLE);
+			break;
+
+		case Opt_allow_unsupported_sfp:
+			adapter->hw.allow_unsupported_sfp = true;
+			break;
+
+#ifdef CONFIG_PCI_IOV
+		case Opt_max_vfs:
+			if (match_int(args, &value))
+				goto parse_error;
+
+			if (adapter->hw.mac.type != ixgbe_mac_82598EB) {
+				if (value < IXGBE_MAX_VF_FUNCTIONS)
+					adapter->num_vfs = value;
+				else
+					goto parse_error;
+			}
+			break;
+
+#endif
+		default:
+			goto parse_error;
+			break;
+		}
+	}
+
+	return;
+
+parse_error:
+	e_dev_err("options error '%s'\n", p);
+	return;
+}
+
+/**
+ * ixgbe_find_config_line - scan config file for labeled option line
+ * @fw: pointer to firmware data
+ * @label: label to search for
+ *
+ * Returns pointer to the configuration options found, or NULL
+ **/
+static char *ixgbe_find_config_line(const struct firmware *fw,
+				    const char *label)
+{
+	const char *p = fw->data;
+	const char *p_end = fw->data + fw->size;
+	int label_len = strlen(label);
+
+	while (p < p_end) {
+		/* ignore spaces and line comments */
+		p = skip_spaces(p);
+		if (p >= p_end)
+			break;
+		if (*p == '#')
+			goto scan_to_eol;
+
+		/* find tag match? */
+		if (!strncmp(p, label, min_t(int, label_len, (p_end - p)))) {
+
+			/* skip over the tag to find the options */
+			p += label_len;
+			p = skip_spaces(p);
+			if (p >= p_end)
+				break;
+			if (*p != '#')
+				return (char *)p;
+		}
+
+scan_to_eol:
+		while (p < p_end && *p != '\n')
+			p++;
+		if (p < p_end && *p == '\n')
+			p++;
+	}
+
+	return NULL;
+}
+
+/**
+ * ixgbe_check_options - find and check configuration parameters
+ * @adapter: pointer to ixgbe_adapter
+ * @label: the configuration tag to search for
+ **/
+void ixgbe_check_options(struct ixgbe_adapter *adapter, const char *label)
+{
+	char *config_line;
+	char *line_end;
+	int line_len, remaining;
+	int ret;
+	const struct firmware *fw;
+
+	ret = request_firmware(&fw, IXGBE_FIRMWARE_FILE, &adapter->pdev->dev);
+	if (ret)
+		return;
+
+	config_line = ixgbe_find_config_line(fw, label);
+	if (config_line) {
+		remaining = fw->size - (config_line - (char *)fw->data);
+		line_end = strnchr(config_line, remaining, '\n');
+		if (line_end)
+			line_len = line_end - config_line;
+		else
+			line_len = remaining;
+		ixgbe_parse_option_line(adapter, config_line, line_len);
+	}
+	release_firmware(fw);
+}
+
 static void ixgbe_service_event_schedule(struct ixgbe_adapter *adapter)
 {
 	if (!test_bit(__IXGBE_DOWN, &adapter->state) &&
@@ -4572,12 +4749,6 @@ static int ixgbe_sw_init(struct ixgbe_adapter *adapter)
 	hw->fc.disable_fc_autoneg =
 		(ixgbe_device_supports_autoneg_fc(hw) == 0) ? false : true;
 
-#ifdef CONFIG_PCI_IOV
-	/* assign number of SR-IOV VFs */
-	if (hw->mac.type != ixgbe_mac_82598EB)
-		adapter->num_vfs = (max_vfs > 63) ? 0 : max_vfs;
-
-#endif
 	/* enable itr by default in dynamic mode */
 	adapter->rx_itr_setting = 1;
 	adapter->tx_itr_setting = 1;
@@ -7197,6 +7368,7 @@ static int ixgbe_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	u8 part_str[IXGBE_PBANUM_LENGTH];
 	unsigned int indices = num_possible_cpus();
 	unsigned int dcb_max = 0;
+	char mac_str[20];
 #ifdef IXGBE_FCOE
 	u16 device_caps;
 #endif
@@ -7279,7 +7451,7 @@ static int ixgbe_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	adapter->pdev = pdev;
 	hw = &adapter->hw;
 	hw->back = adapter;
-	adapter->msg_enable = netif_msg_init(debug, DEFAULT_MSG_ENABLE);
+	adapter->msg_enable = netif_msg_init(-1, DEFAULT_MSG_ENABLE);
 
 	hw->hw_addr = ioremap(pci_resource_start(pdev, 0),
 			      pci_resource_len(pdev, 0));
@@ -7324,6 +7496,9 @@ static int ixgbe_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (err)
 		goto err_sw_init;
 
+	/* look for generic options in userland config file */
+	ixgbe_check_options(adapter, ixgbe_driver_name);
+
 	/* Make it possible the adapter to be woken up via WOL */
 	switch (adapter->hw.mac.type) {
 	case ixgbe_mac_82599EB:
@@ -7344,12 +7519,14 @@ static int ixgbe_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 			e_crit(probe, "Fan has stopped, replace the adapter\n");
 	}
 
-	if (allow_unsupported_sfp)
-		hw->allow_unsupported_sfp = allow_unsupported_sfp;
-
 	/* reset_hw fills in the perm_addr as well */
 	hw->phy.reset_if_overtemp = true;
 	err = hw->mac.ops.reset_hw(hw);
+
+	/* look for mac specific options in userland config file */
+	snprintf(mac_str, sizeof(mac_str)-1, "%pM", adapter->hw.mac.addr);
+	ixgbe_check_options(adapter, mac_str);
+
 	hw->phy.reset_if_overtemp = false;
 	if (err == IXGBE_ERR_SFP_NOT_PRESENT &&
 	    hw->mac.type == ixgbe_mac_82598EB) {

[PATCH 2/3] ixgbe: add additional parameter options

[Shannon Nelson]

With the new userland ASCII parameter file available, add a few
new parameters for finer control of some driver activity.

dca=off	Disable DCA (Direct Cache Access) activity

FdirPballoc=N
	Flow Director packet buffer allocation control
	1 = 8k hash filters or 2k perfect filters
	2 = 16k hash filters or 4k perfect filters
	3 = 32k hash filters or 8k perfect filters

AtrSampleRate=N
	Set the Software ATR Tx packet sample rate to every Nth packet,
	from 0 (disabled) to 255.

Signed-off-by: Shannon Nelson <shannon.nelson@intel.com>
Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Cc: David Woodhouse <dwmw2@infradead.org>
---

 drivers/net/ethernet/intel/ixgbe/ixgbe.h      |    4 ++
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   63 +++++++++++++++++++++++++
 2 files changed, 66 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index 8e78676..adfcd58 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -111,6 +111,10 @@
 #define IXGBE_TX_FLAGS_VLAN_PRIO_SHIFT  29
 #define IXGBE_TX_FLAGS_VLAN_SHIFT	16
 
+#define IXGBE_MAX_ATR_SAMPLE_RATE	255
+#define IXGBE_MIN_ATR_SAMPLE_RATE	1
+#define IXGBE_ATR_SAMPLE_RATE_OFF	0
+
 #define IXGBE_MAX_VF_MC_ENTRIES         30
 #define IXGBE_MAX_VF_FUNCTIONS          64
 #define IXGBE_MAX_VFTA_ENTRIES          128
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 1670fc7..9a94ca2 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -164,6 +164,11 @@ enum {
 #ifdef CONFIG_PCI_IOV
 	Opt_max_vfs,
 #endif
+#ifdef CONFIG_IXGBE_DCA
+	Opt_no_dca,
+#endif
+	Opt_FdirPballoc,
+	Opt_AtrSampleRate,
 };
 
 static const match_table_t tokens = {
@@ -172,6 +177,11 @@ static const match_table_t tokens = {
 #ifdef CONFIG_PCI_IOV
 	{ Opt_max_vfs, "max_vfs=%u" },
 #endif
+#ifdef CONFIG_IXGBE_DCA
+	{ Opt_no_dca, "nodca" },
+#endif
+	{ Opt_FdirPballoc, "FdirPballoc=%u" },
+	{ Opt_AtrSampleRate, "AtrSampleRate=%u" },
 
 	/* terminator token */
 	{ 0, NULL },
@@ -181,6 +191,9 @@ MODULE_INFO(fw_option, "allow_unsupported_sfp : Allow unsupported and untested S
 MODULE_INFO(fw_option,
 	    "max_vfs=N : Maximum number of virtual functions per physical function (default=0) - 0 <= N < "
 	    xstr(IXGBE_MAX_VF_FUNCTIONS));
+MODULE_INFO(fw_option, "nodca : Disable Direct Cache Access");
+MODULE_INFO(fw_option, "FdirPballoc=N : Flow Director packet buffer allocation level - 1, 2, or 3");
+MODULE_INFO(fw_option, "AtrSampleRate=N : Software ATR Tx packet sample rate, 0-255");
 
 /**
  * ixgbe_parse_option_line - find ixgbe options
@@ -195,7 +208,7 @@ static void ixgbe_parse_option_line(struct ixgbe_adapter *adapter,
 	char *p;
 	char *next_option = config_line;
 	substring_t args[MAX_OPT_ARGS];
-	int value;
+	int value, x;
 
 	if (!config_line)
 		return;
@@ -239,6 +252,54 @@ static void ixgbe_parse_option_line(struct ixgbe_adapter *adapter,
 			break;
 
 #endif
+#ifdef CONFIG_IXGBE_DCA
+		case Opt_no_dca:
+			adapter->flags &= ~(IXGBE_FLAG_DCA_CAPABLE |
+					    IXGBE_FLAG_DCA_ENABLED);
+			break;
+
+#endif
+		case Opt_FdirPballoc:
+			if (match_int(args, &value))
+				goto parse_error;
+
+			x = adapter->fdir_pballoc;
+			if (adapter->hw.mac.type == ixgbe_mac_82598EB
+			    || value == IXGBE_FDIR_PBALLOC_NONE) {
+				adapter->fdir_pballoc = IXGBE_FDIR_PBALLOC_NONE;
+				x = 0;
+			} else if (value == IXGBE_FDIR_PBALLOC_256K) {
+				adapter->fdir_pballoc = IXGBE_FDIR_PBALLOC_256K;
+				x = 256;
+			} else if (value == IXGBE_FDIR_PBALLOC_128K) {
+				adapter->fdir_pballoc = IXGBE_FDIR_PBALLOC_128K;
+				x = 128;
+			} else if (value == IXGBE_FDIR_PBALLOC_64K) {
+				adapter->fdir_pballoc = IXGBE_FDIR_PBALLOC_64K;
+				x = 64;
+			} else {
+				e_dev_err("Bad FdirPballoc value %d\n", value);
+			}
+
+			e_dev_err(
+				"Flow Director will be allocated %dKB of packet buffer\n",
+				x);
+			break;
+
+		case Opt_AtrSampleRate:
+			if (match_int(args, &value))
+				goto parse_error;
+
+			if (adapter->hw.mac.type == ixgbe_mac_82598EB
+			    || value == IXGBE_ATR_SAMPLE_RATE_OFF)
+				adapter->atr_sample_rate = value;
+			else if (value >= IXGBE_MIN_ATR_SAMPLE_RATE
+				   && value <= IXGBE_MAX_ATR_SAMPLE_RATE)
+				adapter->atr_sample_rate = value;
+			else
+				e_dev_err("Bad AtrSampleRate %d\n", value);
+			break;
+
 		default:
 			goto parse_error;
 			break;

[PATCH 3/3] ixgbe: add interrupt control parameters

[Shannon Nelson]

MSIX is used by default in the driver, with MSI and legacy interrupt
handling as fallbacks.  These options allow for controlling which handling
will get used.

nomsi
nomsix
	Disable the use of MSI and/or MSIX interrupt handling

Signed-off-by: Shannon Nelson <shannon.nelson@intel.com>
Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Cc: David Woodhouse <dwmw2@infradead.org>
---

 drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c  |    6 ++++++
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   15 +++++++++++++++
 2 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
index 8c74f73..464d8cf 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
@@ -704,6 +704,9 @@ static void ixgbe_acquire_msix_vectors(struct ixgbe_adapter *adapter,
 {
 	int err, vector_threshold;
 
+	if (!(adapter->flags & IXGBE_FLAG_MSIX_CAPABLE))
+		return;
+
 	/* We'll want at least 2 (vector_threshold):
 	 * 1) TxQ[0] + RxQ[0] handler
 	 * 2) Other (Link Status Change, etc.)
@@ -1107,6 +1110,9 @@ static void ixgbe_set_interrupt_capability(struct ixgbe_adapter *adapter)
 	ixgbe_set_num_queues(adapter);
 	adapter->num_q_vectors = 1;
 
+	if (!(adapter->flags & IXGBE_FLAG_MSI_CAPABLE))
+		return;
+
 	err = pci_enable_msi(adapter->pdev);
 	if (err) {
 		netif_printk(adapter, hw, KERN_DEBUG, adapter->netdev,
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 9a94ca2..baa778e 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -169,6 +169,8 @@ enum {
 #endif
 	Opt_FdirPballoc,
 	Opt_AtrSampleRate,
+	Opt_nomsix,
+	Opt_nomsi,
 };
 
 static const match_table_t tokens = {
@@ -182,6 +184,8 @@ static const match_table_t tokens = {
 #endif
 	{ Opt_FdirPballoc, "FdirPballoc=%u" },
 	{ Opt_AtrSampleRate, "AtrSampleRate=%u" },
+	{ Opt_nomsix, "nomsix" },
+	{ Opt_nomsi, "nomsi" },
 
 	/* terminator token */
 	{ 0, NULL },
@@ -194,6 +198,8 @@ MODULE_INFO(fw_option,
 MODULE_INFO(fw_option, "nodca : Disable Direct Cache Access");
 MODULE_INFO(fw_option, "FdirPballoc=N : Flow Director packet buffer allocation level - 1, 2, or 3");
 MODULE_INFO(fw_option, "AtrSampleRate=N : Software ATR Tx packet sample rate, 0-255");
+MODULE_INFO(fw_option, "nomsix : Disable the use of MSI-X interrupt handling");
+MODULE_INFO(fw_option, "nomsi  : Disable the use of MSI interrupt handling");
 
 /**
  * ixgbe_parse_option_line - find ixgbe options
@@ -300,6 +306,14 @@ static void ixgbe_parse_option_line(struct ixgbe_adapter *adapter,
 				e_dev_err("Bad AtrSampleRate %d\n", value);
 			break;
 
+		case Opt_nomsix:
+			adapter->flags &= ~IXGBE_FLAG_MSIX_CAPABLE;
+			break;
+
+		case Opt_nomsi:
+			adapter->flags &= ~IXGBE_FLAG_MSI_CAPABLE;
+			break;
+
 		default:
 			goto parse_error;
 			break;
@@ -4722,6 +4736,7 @@ static int ixgbe_sw_init(struct ixgbe_adapter *adapter)
 	/* Set capability flags */
 	rss = min_t(int, IXGBE_MAX_RSS_INDICES, num_online_cpus());
 	adapter->ring_feature[RING_F_RSS].limit = rss;
+	adapter->flags |= (IXGBE_FLAG_MSIX_CAPABLE | IXGBE_FLAG_MSI_CAPABLE);
 	switch (hw->mac.type) {
 	case ixgbe_mac_82598EB:
 		if (hw->device_id == IXGBE_DEV_ID_82598AT)

Re: [PATCH 0/3] ixgbe: request_firmware for configuration parameters

[Shannon Nelson]

On Thu, Jan 10, 2013 at 6:02 PM, Shannon Nelson
<shannon.nelson@intel.com> wrote:
[...]
>
> In these RFC patches for ixgbe,
>

Yeah, these should have the "RFC" in the Subject line.  Sorry about that.
sln

-- 
==============================================
Mr. Shannon Nelson         Parents can't afford to be squeamish.

Re: [PATCH 0/3] ixgbe: request_firmware for configuration parameters

[Greg KH]

On Thu, Jan 10, 2013 at 06:02:20PM -0800, Shannon Nelson wrote:
> Most networking dials and knobs can be set using ethtool, ifconfig, ip link
> commands, or sysfs entries, all of which can be driven by startup scripts
> and other configuration tools.  However, they all depend on having a netdev
> already set up, and we have some low-level device functionality that needs
> to be sorted out before we start setting up MSI-x and memory allocations.
> 
> In order to do early device configuration, most kernel drivers use module
> parameters whose settings can be persisted in modprobe.d config files.
> However, these can be clumsy to use and manage, difficult to specify port
> specific values in a multiport device, and are actively discouraged in
> some circles.
> 
> In this patchset, the driver uses the existing request_firmware() and
> match_token() facilities to grab an ASCII config file from userspace to
> find special startup-time configuration information that needs persistence
> across reboots.  The configuration strings are formed similar to the
> mount options that get passed from /etc/fstab into filesystem modules.

Ick, please don't abuse request_firmware() for this type of thing.

What's wrong with configfs?  It sounds like it will fit your need, and
that is what is created for.

greg k-h

Re: [PATCH 0/3] ixgbe: request_firmware for configuration parameters

[Shannon Nelson]

On Fri, Jan 11, 2013 at 10:25 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Thu, Jan 10, 2013 at 06:02:20PM -0800, Shannon Nelson wrote:
>> Most networking dials and knobs can be set using ethtool, ifconfig, ip link
>> commands, or sysfs entries, all of which can be driven by startup scripts
>> and other configuration tools.  However, they all depend on having a netdev
>> already set up, and we have some low-level device functionality that needs
>> to be sorted out before we start setting up MSI-x and memory allocations.

> Ick, please don't abuse request_firmware() for this type of thing.

Yeah, it seemed ugly to me at first as well, but it grew on me as I
realized that it does solve a problem in a rather elegant way.  While
working this up I discussed this with Mr. Woodhouse thinking that as a
firmware tree maintainer he'd have a similar reaction, but he actually
wasn't opposed to it (David, please speak up if I'm misrepresenting
your comments).

> What's wrong with configfs?  It sounds like it will fit your need, and
> that is what is created for.

configfs has similar problems as sysfs - the driver needs to create
the hooks before it has all the info it might need for some hooks,
there is no persistence across reboots, and I don't think it will help
for initrd images.  Additionally, there would need to be some userland
mechanism to notice that the hooks were there and to feed it the
startup info.  Using a file in the firmware path gives us persistence
and a way for the driver to get info before having to set up
filesystem hooks.  It also gives us a way to get special config info
into the boot image.  And the whole mechanism already exists,
including UDEV hooks that can do more fancy stuff if needed.

sln

-- 
==============================================
Mr. Shannon Nelson         Parents can't afford to be squeamish.

Re: [PATCH 0/3] ixgbe: request_firmware for configuration parameters

[Greg KH]

On Fri, Jan 11, 2013 at 11:30:54AM -0800, Shannon Nelson wrote:
> On Fri, Jan 11, 2013 at 10:25 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Thu, Jan 10, 2013 at 06:02:20PM -0800, Shannon Nelson wrote:
> >> Most networking dials and knobs can be set using ethtool, ifconfig, ip link
> >> commands, or sysfs entries, all of which can be driven by startup scripts
> >> and other configuration tools.  However, they all depend on having a netdev
> >> already set up, and we have some low-level device functionality that needs
> >> to be sorted out before we start setting up MSI-x and memory allocations.
> 
> > Ick, please don't abuse request_firmware() for this type of thing.
> 
> Yeah, it seemed ugly to me at first as well, but it grew on me as I
> realized that it does solve a problem in a rather elegant way.  While
> working this up I discussed this with Mr. Woodhouse thinking that as a
> firmware tree maintainer he'd have a similar reaction, but he actually
> wasn't opposed to it (David, please speak up if I'm misrepresenting
> your comments).

David maintains the external firmware tree repo, not the in-kernel
firmware core code (which I used to maintain.)

> > What's wrong with configfs?  It sounds like it will fit your need, and
> > that is what is created for.
> 
> configfs has similar problems as sysfs - the driver needs to create
> the hooks before it has all the info it might need for some hooks,
> there is no persistence across reboots, and I don't think it will help
> for initrd images.  Additionally, there would need to be some userland
> mechanism to notice that the hooks were there and to feed it the
> startup info.  Using a file in the firmware path gives us persistence
> and a way for the driver to get info before having to set up
> filesystem hooks.  It also gives us a way to get special config info
> into the boot image.  And the whole mechanism already exists,
> including UDEV hooks that can do more fancy stuff if needed.

Yes, but you are now starting to use "configuration files" for kernel
drivers, which we have resisted for 20+ years for a variety of good
reasons.  You can't just ignore all of the arguments to not do this all
of a sudden because you feel your driver is somehow "special" here.

All of the above issues you seem to have with sysfs and configfs can be
resolved with userspace code, and having your driver not do anything to
the hardware until it is told to by userspace.

The boot image problem is harder, but I would argue that your driver
better fall-back to some "known good" configuration for that type of
instance, as you will need to do that anyway if your "firmware" file
isn't present in the first place.

So please try configfs again, the idea of loading configuration files
from the filesystem, no matter what the mechanism, into your driver,
isn't ok to do, sorry.

greg k-h

Re: [PATCH 0/3] ixgbe: request_firmware for configuration parameters

[Ali Ayoub]

On 1/11/2013 11:41 AM, Greg KH wrote:
> On Fri, Jan 11, 2013 at 11:30:54AM -0800, Shannon Nelson wrote:
>> On Fri, Jan 11, 2013 at 10:25 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
>>> On Thu, Jan 10, 2013 at 06:02:20PM -0800, Shannon Nelson wrote:
>>>> Most networking dials and knobs can be set using ethtool, ifconfig, ip link
>>>> commands, or sysfs entries, all of which can be driven by startup scripts
>>>> and other configuration tools.  However, they all depend on having a netdev
>>>> already set up, and we have some low-level device functionality that needs
>>>> to be sorted out before we start setting up MSI-x and memory allocations.
>>
>>> Ick, please don't abuse request_firmware() for this type of thing.
>>
>> Yeah, it seemed ugly to me at first as well, but it grew on me as I
>> realized that it does solve a problem in a rather elegant way.  While
>> working this up I discussed this with Mr. Woodhouse thinking that as a
>> firmware tree maintainer he'd have a similar reaction, but he actually
>> wasn't opposed to it (David, please speak up if I'm misrepresenting
>> your comments).
> 
> David maintains the external firmware tree repo, not the in-kernel
> firmware core code (which I used to maintain.)
> 
>>> What's wrong with configfs?  It sounds like it will fit your need, and
>>> that is what is created for.
>>
>> configfs has similar problems as sysfs - the driver needs to create
>> the hooks before it has all the info it might need for some hooks,
>> there is no persistence across reboots, and I don't think it will help
>> for initrd images.  Additionally, there would need to be some userland
>> mechanism to notice that the hooks were there and to feed it the
>> startup info.  Using a file in the firmware path gives us persistence
>> and a way for the driver to get info before having to set up
>> filesystem hooks.  It also gives us a way to get special config info
>> into the boot image.  And the whole mechanism already exists,
>> including UDEV hooks that can do more fancy stuff if needed.
> 
> Yes, but you are now starting to use "configuration files" for kernel
> drivers, which we have resisted for 20+ years for a variety of good
> reasons. You can't just ignore all of the arguments to not do this all
> of a sudden because you feel your driver is somehow "special" here.

Other device drivers of other vendors (not only netdevs) need such a
mechanism as well, I think it's a general requirement for many drivers
that normally need low level configurations for device initialization in
the very first stage of the driver load.

> All of the above issues you seem to have with sysfs and configfs can be
> resolved with userspace code, and having your driver not do anything to
> the hardware until it is told to by userspace.

To tell the driver not to do anything until it's configured by a
userspace code will require a module param for non-default-configs
(which brings us back to the original argument of avoiding module params).

By having userspace code to feed configfs/sysfs nodes, and making it
available in initrd; we will end up having similar mechanism to
request_firmware().

I think this kind of "low level init configuration" can be seen as a
firmware configuration, we can put some limitation on fetching the
config file, or propose a new function such as request_firmware_config()
that uses the same uevent hooks, and leverages the available userspace
tools that already supported in initrd and meant to serve the same
purpose - of feeding the driver the suitable firmware and configuration
to get started.

Ali;

Re: [PATCH 0/3] ixgbe: request_firmware for configuration parameters

[Greg KH]

On Fri, Aug 16, 2013 at 03:14:31PM -0700, Ali Ayoub wrote:
> On 1/11/2013 11:41 AM, Greg KH wrote:

Seriously?  Restarting a thread from over 6 months ago?  Why?

> > On Fri, Jan 11, 2013 at 11:30:54AM -0800, Shannon Nelson wrote:
> >> On Fri, Jan 11, 2013 at 10:25 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> >>> On Thu, Jan 10, 2013 at 06:02:20PM -0800, Shannon Nelson wrote:
> >>>> Most networking dials and knobs can be set using ethtool, ifconfig, ip link
> >>>> commands, or sysfs entries, all of which can be driven by startup scripts
> >>>> and other configuration tools.  However, they all depend on having a netdev
> >>>> already set up, and we have some low-level device functionality that needs
> >>>> to be sorted out before we start setting up MSI-x and memory allocations.
> >>
> >>> Ick, please don't abuse request_firmware() for this type of thing.
> >>
> >> Yeah, it seemed ugly to me at first as well, but it grew on me as I
> >> realized that it does solve a problem in a rather elegant way.  While
> >> working this up I discussed this with Mr. Woodhouse thinking that as a
> >> firmware tree maintainer he'd have a similar reaction, but he actually
> >> wasn't opposed to it (David, please speak up if I'm misrepresenting
> >> your comments).
> > 
> > David maintains the external firmware tree repo, not the in-kernel
> > firmware core code (which I used to maintain.)
> > 
> >>> What's wrong with configfs?  It sounds like it will fit your need, and
> >>> that is what is created for.
> >>
> >> configfs has similar problems as sysfs - the driver needs to create
> >> the hooks before it has all the info it might need for some hooks,
> >> there is no persistence across reboots, and I don't think it will help
> >> for initrd images.  Additionally, there would need to be some userland
> >> mechanism to notice that the hooks were there and to feed it the
> >> startup info.  Using a file in the firmware path gives us persistence
> >> and a way for the driver to get info before having to set up
> >> filesystem hooks.  It also gives us a way to get special config info
> >> into the boot image.  And the whole mechanism already exists,
> >> including UDEV hooks that can do more fancy stuff if needed.
> > 
> > Yes, but you are now starting to use "configuration files" for kernel
> > drivers, which we have resisted for 20+ years for a variety of good
> > reasons. You can't just ignore all of the arguments to not do this all
> > of a sudden because you feel your driver is somehow "special" here.
> 
> Other device drivers of other vendors (not only netdevs) need such a
> mechanism as well,

Specifics please.

> I think it's a general requirement for many drivers that normally need
> low level configurations for device initialization in the very first
> stage of the driver load.

I do not, but feel free to prove me otherwise.

> > All of the above issues you seem to have with sysfs and configfs can be
> > resolved with userspace code, and having your driver not do anything to
> > the hardware until it is told to by userspace.
> 
> To tell the driver not to do anything until it's configured by a
> userspace code will require a module param for non-default-configs
> (which brings us back to the original argument of avoiding module params).

No, never use a module paramater for configuring a driver or a device.
That's not what they are there for.

> By having userspace code to feed configfs/sysfs nodes, and making it
> available in initrd; we will end up having similar mechanism to
> request_firmware().

Close, but not the same.  That's why we created configfs, please use it.

> I think this kind of "low level init configuration" can be seen as a
> firmware configuration, we can put some limitation on fetching the
> config file, or propose a new function such as request_firmware_config()
> that uses the same uevent hooks, and leverages the available userspace
> tools that already supported in initrd and meant to serve the same
> purpose - of feeding the driver the suitable firmware and configuration
> to get started.

Nope, I do not, please don't abuse the interface like this, it's not
going to be allowed at this point in time.

Wait, what "point in time", this was a 6 month old conversation...

greg "what month is this?" k-h

Re: [PATCH 0/3] ixgbe: request_firmware for configuration parameters

[Ali Ayoub]

On 8/16/2013 3:39 PM, Greg KH wrote:
> On Fri, Aug 16, 2013 at 03:14:31PM -0700, Ali Ayoub wrote:
>> On 1/11/2013 11:41 AM, Greg KH wrote:
> 
> Seriously?  Restarting a thread from over 6 months ago?

Yes, it's an old thread, but still relevant for current code.

> Why?

Because currently there is no good alternative for module params for
device drivers that need to have low level configs in init time.
I found Shannon proposal in this thread the closest to what I am looking
for to configure the device driver and yet not to use module params.
Thus, pinged this thread.

>> Other device drivers of other vendors (not only netdevs) need such a
>> mechanism as well,
> 
> Specifics please.
>
>> I think it's a general requirement for many drivers that normally need
>> low level configurations for device initialization in the very first
>> stage of the driver load.
> 
> I do not, but feel free to prove me otherwise.

A driver that claims the PCI device needs some configuration in init
time such as: amount of resources to be allocated in the cache,
interrupt mode, maximum allowed resources to be created for a specific
type, number of event queues, etc. See for example:
/drivers/net/ethernet/mellanox/mlx4/main.c

This driver doesn't necessary register a logical device, it may claim
the PCI device only (e.g. for an HCA), and provide an infrastructure for
upper layer protocols to register a logical device (nic, hba, etc.) on
top of it.

The type of configuration needed varies between vendors, and they are
normally passed in HW initialization stage. So even if we have a tool to
configure the device (such as ethtool for netdevs) it wouldn't be a
replacement for the module param, because some systems requires
non-default (init) configurations, and if we load the driver with the
default, and then use a user-space tool to "adjust" the configuration
we'll have a "glitch" where the driver was loaded with unsuitable
configs for the system.

>>> All of the above issues you seem to have with sysfs and configfs can be
>>> resolved with userspace code, and having your driver not do anything to
>>> the hardware until it is told to by userspace.
>>
>> To tell the driver not to do anything until it's configured by a
>> userspace code will require a module param for non-default-configs
>> (which brings us back to the original argument of avoiding module params).
> 
> No, never use a module paramater for configuring a driver or a device.
> That's not what they are there for.
>
>> By having userspace code to feed configfs/sysfs nodes, and making it
>> available in initrd; we will end up having similar mechanism to
>> request_firmware().
> 
> Close, but not the same.  That's why we created configfs, please use it.

configfs requires from the driver to provide the hooks before the HW is
initialized, while module params allow passing information to the driver
in init time before any driver hooks are ready.

The proposal of changing the driver not to configure the HW until it's
triggered by userspace service through configfs, means that we need:

a. To have driver config to toggle between the mode where the driver
"waits" for configfs, and the "auto" mode where the driver loads with
the default-configs (when using mod params for example, the driver
simply loads with the defaults when there is modprobe config files).

b. To have the userspace mechanism to feed the configs nodes, to store
the configs in a file to keep them persistent between reboots, and make
these userspace services available in initrd.

I don't see how this is better than module params, or
request_firmware_config().

Re: [PATCH 0/3] ixgbe: request_firmware for configuration parameters

[Greg KH]

On Fri, Aug 16, 2013 at 05:18:12PM -0700, Ali Ayoub wrote:
> On 8/16/2013 3:39 PM, Greg KH wrote:
> > On Fri, Aug 16, 2013 at 03:14:31PM -0700, Ali Ayoub wrote:
> >> On 1/11/2013 11:41 AM, Greg KH wrote:
> > 
> > Seriously?  Restarting a thread from over 6 months ago?
> 
> Yes, it's an old thread, but still relevant for current code.

Which specific code are you referring to?

> > Why?
> 
> Because currently there is no good alternative for module params for
> device drivers that need to have low level configs in init time.

I disagree, see below for why module params are broken for this.

> >> Other device drivers of other vendors (not only netdevs) need such a
> >> mechanism as well,
> > 
> > Specifics please.
> >
> >> I think it's a general requirement for many drivers that normally need
> >> low level configurations for device initialization in the very first
> >> stage of the driver load.
> > 
> > I do not, but feel free to prove me otherwise.
> 
> A driver that claims the PCI device needs some configuration in init
> time such as: amount of resources to be allocated in the cache,
> interrupt mode, maximum allowed resources to be created for a specific
> type, number of event queues, etc. See for example:
> /drivers/net/ethernet/mellanox/mlx4/main.c
> 
> This driver doesn't necessary register a logical device, it may claim
> the PCI device only (e.g. for an HCA), and provide an infrastructure for
> upper layer protocols to register a logical device (nic, hba, etc.) on
> top of it.

And how do you handle hundreds of the same card in the same machine?
You can't do that with module paramaters at all, it doesn't scale, or
make any sense.

What about hotplug of new devices after the module is loaded?  That will
not work at all for module parameters either.

That is why we created sysfs, and configfs.

> The type of configuration needed varies between vendors, and they are
> normally passed in HW initialization stage. So even if we have a tool to
> configure the device (such as ethtool for netdevs) it wouldn't be a
> replacement for the module param, because some systems requires
> non-default (init) configurations, and if we load the driver with the
> default, and then use a user-space tool to "adjust" the configuration
> we'll have a "glitch" where the driver was loaded with unsuitable
> configs for the system.

No, just remain in a "unusable" state until you are initialized from
userspace.  No "glitch" at all.

> >>> All of the above issues you seem to have with sysfs and configfs can be
> >>> resolved with userspace code, and having your driver not do anything to
> >>> the hardware until it is told to by userspace.
> >>
> >> To tell the driver not to do anything until it's configured by a
> >> userspace code will require a module param for non-default-configs
> >> (which brings us back to the original argument of avoiding module params).
> > 
> > No, never use a module paramater for configuring a driver or a device.
> > That's not what they are there for.
> >
> >> By having userspace code to feed configfs/sysfs nodes, and making it
> >> available in initrd; we will end up having similar mechanism to
> >> request_firmware().
> > 
> > Close, but not the same.  That's why we created configfs, please use it.
> 
> configfs requires from the driver to provide the hooks before the HW is
> initialized, while module params allow passing information to the driver
> in init time before any driver hooks are ready.

And module params don't work at all for the common usage of hotplug
devices, and multiple numbers of devices that show up _after_ the module
is loaded.

> The proposal of changing the driver not to configure the HW until it's
> triggered by userspace service through configfs, means that we need:
> 
> a. To have driver config to toggle between the mode where the driver
> "waits" for configfs, and the "auto" mode where the driver loads with
> the default-configs (when using mod params for example, the driver
> simply loads with the defaults when there is modprobe config files).
> 
> b. To have the userspace mechanism to feed the configs nodes, to store
> the configs in a file to keep them persistent between reboots, and make
> these userspace services available in initrd.

Does this really need to be in the initrd?  That's only needed for your
boot device, and if you need it there, great, put it there, that's what
dracut is there for, it's a full Linux system in the initrd.

configfs, for complex initializations like you are proposing, works,
that is why it was created, and what it is used for.  Yes, it's "rough"
in places, and a bit complex.  Patches to fix that are always gladly
appreciated.

> I don't see how this is better than module params, or
> request_firmware_config().

request_firmware is for just that, firmware, not configuration values,
sorry.  Use configfs for configuration, that's what it is there for.

thanks,

greg k-h