* [PATCH net-next 2/4] net/mlx4_core: New init and exit flow for mlx4_core
From: Or Gerlitz @ 2014-09-30 9:03 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Amir Vadai, Jack Morgenstein, Majd Dibbiny, Or Gerlitz
In-Reply-To: <1412067830-30438-1-git-send-email-ogerlitz@mellanox.com>
From: Majd Dibbiny <majd@mellanox.com>
In the new flow, we separate the pci initialization and teardown
from the initialization and teardown of the other resources.
__mlx4_init_one handles the pci resources initialization. It then
calls mlx4_load_one to initialize the remainder of the resources.
When removing a device, mlx4_remove_one is invoked. However, now
mlx4_remove_one calls mlx4_unload_one to free all the resources except the pci
resources. When mlx4_unload_one returns, mlx4_remove_one then frees the
pci resources.
The above separation will allow us to implement 'reset flow' in the future.
It will also enable more EQs for VFs and is a pre-step to the modern API to
enable/disable SRIOV.
Also added nvfs; an integer array of size MLX4_MAX_PORTS + 1; to the mlx4_dev
struct. This new field is used to avoid parsing the num_vfs module parameter
each time the mlx4_restart_one is called.
Signed-off-by: Majd Dibbiny <majd@mellanox.com>
Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx4/main.c | 393 ++++++++++++++++-------------
include/linux/mlx4/device.h | 1 +
2 files changed, 220 insertions(+), 174 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
index 4e9857b..f6c32a9 100644
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -2259,116 +2259,18 @@ static void mlx4_free_ownership(struct mlx4_dev *dev)
iounmap(owner);
}
-static int __mlx4_init_one(struct pci_dev *pdev, int pci_dev_data)
+static int mlx4_load_one(struct pci_dev *pdev, int pci_dev_data,
+ int total_vfs, int *nvfs, struct mlx4_priv *priv)
{
- struct mlx4_priv *priv;
struct mlx4_dev *dev;
+ unsigned sum = 0;
int err;
int port;
- int nvfs[MLX4_MAX_PORTS + 1] = {0, 0, 0};
- int prb_vf[MLX4_MAX_PORTS + 1] = {0, 0, 0};
- const int param_map[MLX4_MAX_PORTS + 1][MLX4_MAX_PORTS + 1] = {
- {2, 0, 0}, {0, 1, 2}, {0, 1, 2} };
- unsigned total_vfs = 0;
- int sriov_initialized = 0;
- unsigned int i;
+ int i;
int existing_vfs = 0;
- pr_info(DRV_NAME ": Initializing %s\n", pci_name(pdev));
-
- err = pci_enable_device(pdev);
- if (err) {
- dev_err(&pdev->dev, "Cannot enable PCI device, aborting\n");
- return err;
- }
-
- /* Due to requirement that all VFs and the PF are *guaranteed* 2 MACS
- * per port, we must limit the number of VFs to 63 (since their are
- * 128 MACs)
- */
- for (i = 0; i < sizeof(nvfs)/sizeof(nvfs[0]) && i < num_vfs_argc;
- total_vfs += nvfs[param_map[num_vfs_argc - 1][i]], i++) {
- nvfs[param_map[num_vfs_argc - 1][i]] = num_vfs[i];
- if (nvfs[i] < 0) {
- dev_err(&pdev->dev, "num_vfs module parameter cannot be negative\n");
- return -EINVAL;
- }
- }
- for (i = 0; i < sizeof(prb_vf)/sizeof(prb_vf[0]) && i < probe_vfs_argc;
- i++) {
- prb_vf[param_map[probe_vfs_argc - 1][i]] = probe_vf[i];
- if (prb_vf[i] < 0 || prb_vf[i] > nvfs[i]) {
- dev_err(&pdev->dev, "probe_vf module parameter cannot be negative or greater than num_vfs\n");
- return -EINVAL;
- }
- }
- if (total_vfs >= MLX4_MAX_NUM_VF) {
- dev_err(&pdev->dev,
- "Requested more VF's (%d) than allowed (%d)\n",
- total_vfs, MLX4_MAX_NUM_VF - 1);
- return -EINVAL;
- }
-
- for (i = 0; i < MLX4_MAX_PORTS; i++) {
- if (nvfs[i] + nvfs[2] >= MLX4_MAX_NUM_VF_P_PORT) {
- dev_err(&pdev->dev,
- "Requested more VF's (%d) for port (%d) than allowed (%d)\n",
- nvfs[i] + nvfs[2], i + 1,
- MLX4_MAX_NUM_VF_P_PORT - 1);
- return -EINVAL;
- }
- }
-
-
- /*
- * Check for BARs.
- */
- if (!(pci_dev_data & MLX4_PCI_DEV_IS_VF) &&
- !(pci_resource_flags(pdev, 0) & IORESOURCE_MEM)) {
- dev_err(&pdev->dev, "Missing DCS, aborting (driver_data: 0x%x, pci_resource_flags(pdev, 0):0x%lx)\n",
- pci_dev_data, pci_resource_flags(pdev, 0));
- err = -ENODEV;
- goto err_disable_pdev;
- }
- if (!(pci_resource_flags(pdev, 2) & IORESOURCE_MEM)) {
- dev_err(&pdev->dev, "Missing UAR, aborting\n");
- err = -ENODEV;
- goto err_disable_pdev;
- }
-
- err = pci_request_regions(pdev, DRV_NAME);
- if (err) {
- dev_err(&pdev->dev, "Couldn't get PCI resources, aborting\n");
- goto err_disable_pdev;
- }
-
- pci_set_master(pdev);
+ dev = &priv->dev;
- err = pci_set_dma_mask(pdev, DMA_BIT_MASK(64));
- if (err) {
- dev_warn(&pdev->dev, "Warning: couldn't set 64-bit PCI DMA mask\n");
- err = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
- if (err) {
- dev_err(&pdev->dev, "Can't set PCI DMA mask, aborting\n");
- goto err_release_regions;
- }
- }
- err = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(64));
- if (err) {
- dev_warn(&pdev->dev, "Warning: couldn't set 64-bit consistent PCI DMA mask\n");
- err = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(32));
- if (err) {
- dev_err(&pdev->dev, "Can't set consistent PCI DMA mask, aborting\n");
- goto err_release_regions;
- }
- }
-
- /* Allow large DMA segments, up to the firmware limit of 1 GB */
- dma_set_max_seg_size(&pdev->dev, 1024 * 1024 * 1024);
-
- dev = pci_get_drvdata(pdev);
- priv = mlx4_priv(dev);
- dev->pdev = pdev;
INIT_LIST_HEAD(&priv->ctx_list);
spin_lock_init(&priv->ctx_lock);
@@ -2382,28 +2284,9 @@ static int __mlx4_init_one(struct pci_dev *pdev, int pci_dev_data)
dev->rev_id = pdev->revision;
dev->numa_node = dev_to_node(&pdev->dev);
+
/* Detect if this device is a virtual function */
if (pci_dev_data & MLX4_PCI_DEV_IS_VF) {
- /* When acting as pf, we normally skip vfs unless explicitly
- * requested to probe them. */
- if (total_vfs) {
- unsigned vfs_offset = 0;
- for (i = 0; i < sizeof(nvfs)/sizeof(nvfs[0]) &&
- vfs_offset + nvfs[i] < extended_func_num(pdev);
- vfs_offset += nvfs[i], i++)
- ;
- if (i == sizeof(nvfs)/sizeof(nvfs[0])) {
- err = -ENODEV;
- goto err_free_dev;
- }
- if ((extended_func_num(pdev) - vfs_offset)
- > prb_vf[i]) {
- mlx4_warn(dev, "Skipping virtual function:%d\n",
- extended_func_num(pdev));
- err = -ENODEV;
- goto err_free_dev;
- }
- }
mlx4_warn(dev, "Detected virtual function - running in slave mode\n");
dev->flags |= MLX4_FLAG_SLAVE;
} else {
@@ -2413,11 +2296,10 @@ static int __mlx4_init_one(struct pci_dev *pdev, int pci_dev_data)
err = mlx4_get_ownership(dev);
if (err) {
if (err < 0)
- goto err_free_dev;
+ return err;
else {
mlx4_warn(dev, "Multiple PFs not yet supported - Skipping PF\n");
- err = -EINVAL;
- goto err_free_dev;
+ return -EINVAL;
}
}
@@ -2429,7 +2311,8 @@ static int __mlx4_init_one(struct pci_dev *pdev, int pci_dev_data)
GFP_KERNEL);
if (NULL == dev->dev_vfs) {
mlx4_err(dev, "Failed to allocate memory for VFs\n");
- err = 0;
+ err = -ENOMEM;
+ goto err_free_own;
} else {
atomic_inc(&pf_loading);
existing_vfs = pci_num_vf(pdev);
@@ -2445,13 +2328,11 @@ static int __mlx4_init_one(struct pci_dev *pdev, int pci_dev_data)
mlx4_err(dev, "Failed to enable SR-IOV, continuing without SR-IOV (err = %d)\n",
err);
atomic_dec(&pf_loading);
- err = 0;
} else {
mlx4_warn(dev, "Running in master mode\n");
dev->flags |= MLX4_FLAG_SRIOV |
MLX4_FLAG_MASTER;
dev->num_vfs = total_vfs;
- sriov_initialized = 1;
}
}
}
@@ -2467,7 +2348,7 @@ static int __mlx4_init_one(struct pci_dev *pdev, int pci_dev_data)
err = mlx4_reset(dev);
if (err) {
mlx4_err(dev, "Failed to reset HCA, aborting\n");
- goto err_rel_own;
+ goto err_sriov;
}
}
@@ -2517,34 +2398,46 @@ slave_start:
/* In master functions, the communication channel must be initialized
* after obtaining its address from fw */
if (mlx4_is_master(dev)) {
- unsigned sum = 0;
- err = mlx4_multi_func_init(dev);
- if (err) {
- mlx4_err(dev, "Failed to init master mfunc interface, aborting\n");
+ int ib_ports = 0;
+
+ mlx4_foreach_port(i, dev, MLX4_PORT_TYPE_IB)
+ ib_ports++;
+
+ if (ib_ports &&
+ (num_vfs_argc > 1 || probe_vfs_argc > 1)) {
+ mlx4_err(dev,
+ "Invalid syntax of num_vfs/probe_vfs with IB port - single port VFs syntax is only supported when all ports are configured as ethernet\n");
+ err = -EINVAL;
+ goto err_close;
+ }
+ if (dev->caps.num_ports < 2 &&
+ num_vfs_argc > 1) {
+ err = -EINVAL;
+ mlx4_err(dev,
+ "Error: Trying to configure VFs on port 2, but HCA has only %d physical ports\n",
+ dev->caps.num_ports);
goto err_close;
}
- if (sriov_initialized) {
- int ib_ports = 0;
- mlx4_foreach_port(i, dev, MLX4_PORT_TYPE_IB)
- ib_ports++;
+ memcpy(dev->nvfs, nvfs, sizeof(dev->nvfs));
- if (ib_ports &&
- (num_vfs_argc > 1 || probe_vfs_argc > 1)) {
- mlx4_err(dev,
- "Invalid syntax of num_vfs/probe_vfs with IB port - single port VFs syntax is only supported when all ports are configured as ethernet\n");
- err = -EINVAL;
- goto err_master_mfunc;
- }
- for (i = 0; i < sizeof(nvfs)/sizeof(nvfs[0]); i++) {
- unsigned j;
- for (j = 0; j < nvfs[i]; ++sum, ++j) {
- dev->dev_vfs[sum].min_port =
- i < 2 ? i + 1 : 1;
- dev->dev_vfs[sum].n_ports = i < 2 ? 1 :
- dev->caps.num_ports;
- }
+ for (i = 0; i < sizeof(dev->nvfs)/sizeof(dev->nvfs[0]); i++) {
+ unsigned j;
+
+ for (j = 0; j < dev->nvfs[i]; ++sum, ++j) {
+ dev->dev_vfs[sum].min_port = i < 2 ? i + 1 : 1;
+ dev->dev_vfs[sum].n_ports = i < 2 ? 1 :
+ dev->caps.num_ports;
}
}
+
+ /* In master functions, the communication channel
+ * must be initialized after obtaining its address from fw
+ */
+ err = mlx4_multi_func_init(dev);
+ if (err) {
+ mlx4_err(dev, "Failed to init master mfunc interface, aborting.\n");
+ goto err_close;
+ }
}
err = mlx4_alloc_eq_table(dev);
@@ -2565,7 +2458,7 @@ slave_start:
if (!mlx4_is_slave(dev)) {
err = mlx4_init_steering(dev);
if (err)
- goto err_free_eq;
+ goto err_disable_msix;
}
err = mlx4_setup_hca(dev);
@@ -2625,6 +2518,10 @@ err_steer:
if (!mlx4_is_slave(dev))
mlx4_clear_steering(dev);
+err_disable_msix:
+ if (dev->flags & MLX4_FLAG_MSI_X)
+ pci_disable_msix(pdev);
+
err_free_eq:
mlx4_free_eq_table(dev);
@@ -2641,9 +2538,6 @@ err_master_mfunc:
}
err_close:
- if (dev->flags & MLX4_FLAG_MSI_X)
- pci_disable_msix(pdev);
-
mlx4_close_hca(dev);
err_mfunc:
@@ -2657,17 +2551,151 @@ err_sriov:
if (dev->flags & MLX4_FLAG_SRIOV && !existing_vfs)
pci_disable_sriov(pdev);
-err_rel_own:
- if (!mlx4_is_slave(dev))
- mlx4_free_ownership(dev);
-
if (mlx4_is_master(dev) && dev->num_vfs)
atomic_dec(&pf_loading);
kfree(priv->dev.dev_vfs);
-err_free_dev:
- kfree(priv);
+err_free_own:
+ if (!mlx4_is_slave(dev))
+ mlx4_free_ownership(dev);
+
+ return err;
+}
+
+static int __mlx4_init_one(struct pci_dev *pdev, int pci_dev_data,
+ struct mlx4_priv *priv)
+{
+ int err;
+ int nvfs[MLX4_MAX_PORTS + 1] = {0, 0, 0};
+ int prb_vf[MLX4_MAX_PORTS + 1] = {0, 0, 0};
+ const int param_map[MLX4_MAX_PORTS + 1][MLX4_MAX_PORTS + 1] = {
+ {2, 0, 0}, {0, 1, 2}, {0, 1, 2} };
+ unsigned total_vfs = 0;
+ unsigned int i;
+
+ pr_info(DRV_NAME ": Initializing %s\n", pci_name(pdev));
+
+ err = pci_enable_device(pdev);
+ if (err) {
+ dev_err(&pdev->dev, "Cannot enable PCI device, aborting\n");
+ return err;
+ }
+
+ /* Due to requirement that all VFs and the PF are *guaranteed* 2 MACS
+ * per port, we must limit the number of VFs to 63 (since their are
+ * 128 MACs)
+ */
+ for (i = 0; i < sizeof(nvfs)/sizeof(nvfs[0]) && i < num_vfs_argc;
+ total_vfs += nvfs[param_map[num_vfs_argc - 1][i]], i++) {
+ nvfs[param_map[num_vfs_argc - 1][i]] = num_vfs[i];
+ if (nvfs[i] < 0) {
+ dev_err(&pdev->dev, "num_vfs module parameter cannot be negative\n");
+ err = -EINVAL;
+ goto err_disable_pdev;
+ }
+ }
+ for (i = 0; i < sizeof(prb_vf)/sizeof(prb_vf[0]) && i < probe_vfs_argc;
+ i++) {
+ prb_vf[param_map[probe_vfs_argc - 1][i]] = probe_vf[i];
+ if (prb_vf[i] < 0 || prb_vf[i] > nvfs[i]) {
+ dev_err(&pdev->dev, "probe_vf module parameter cannot be negative or greater than num_vfs\n");
+ err = -EINVAL;
+ goto err_disable_pdev;
+ }
+ }
+ if (total_vfs >= MLX4_MAX_NUM_VF) {
+ dev_err(&pdev->dev,
+ "Requested more VF's (%d) than allowed (%d)\n",
+ total_vfs, MLX4_MAX_NUM_VF - 1);
+ err = -EINVAL;
+ goto err_disable_pdev;
+ }
+
+ for (i = 0; i < MLX4_MAX_PORTS; i++) {
+ if (nvfs[i] + nvfs[2] >= MLX4_MAX_NUM_VF_P_PORT) {
+ dev_err(&pdev->dev,
+ "Requested more VF's (%d) for port (%d) than allowed (%d)\n",
+ nvfs[i] + nvfs[2], i + 1,
+ MLX4_MAX_NUM_VF_P_PORT - 1);
+ err = -EINVAL;
+ goto err_disable_pdev;
+ }
+ }
+
+ /* Check for BARs. */
+ if (!(pci_dev_data & MLX4_PCI_DEV_IS_VF) &&
+ !(pci_resource_flags(pdev, 0) & IORESOURCE_MEM)) {
+ dev_err(&pdev->dev, "Missing DCS, aborting (driver_data: 0x%x, pci_resource_flags(pdev, 0):0x%lx)\n",
+ pci_dev_data, pci_resource_flags(pdev, 0));
+ err = -ENODEV;
+ goto err_disable_pdev;
+ }
+ if (!(pci_resource_flags(pdev, 2) & IORESOURCE_MEM)) {
+ dev_err(&pdev->dev, "Missing UAR, aborting\n");
+ err = -ENODEV;
+ goto err_disable_pdev;
+ }
+
+ err = pci_request_regions(pdev, DRV_NAME);
+ if (err) {
+ dev_err(&pdev->dev, "Couldn't get PCI resources, aborting\n");
+ goto err_disable_pdev;
+ }
+
+ pci_set_master(pdev);
+
+ err = pci_set_dma_mask(pdev, DMA_BIT_MASK(64));
+ if (err) {
+ dev_warn(&pdev->dev, "Warning: couldn't set 64-bit PCI DMA mask\n");
+ err = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
+ if (err) {
+ dev_err(&pdev->dev, "Can't set PCI DMA mask, aborting\n");
+ goto err_release_regions;
+ }
+ }
+ err = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(64));
+ if (err) {
+ dev_warn(&pdev->dev, "Warning: couldn't set 64-bit consistent PCI DMA mask\n");
+ err = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(32));
+ if (err) {
+ dev_err(&pdev->dev, "Can't set consistent PCI DMA mask, aborting\n");
+ goto err_release_regions;
+ }
+ }
+
+ /* Allow large DMA segments, up to the firmware limit of 1 GB */
+ dma_set_max_seg_size(&pdev->dev, 1024 * 1024 * 1024);
+ /* Detect if this device is a virtual function */
+ if (pci_dev_data & MLX4_PCI_DEV_IS_VF) {
+ /* When acting as pf, we normally skip vfs unless explicitly
+ * requested to probe them.
+ */
+ if (total_vfs) {
+ unsigned vfs_offset = 0;
+
+ for (i = 0; i < sizeof(nvfs)/sizeof(nvfs[0]) &&
+ vfs_offset + nvfs[i] < extended_func_num(pdev);
+ vfs_offset += nvfs[i], i++)
+ ;
+ if (i == sizeof(nvfs)/sizeof(nvfs[0])) {
+ err = -ENODEV;
+ goto err_release_regions;
+ }
+ if ((extended_func_num(pdev) - vfs_offset)
+ > prb_vf[i]) {
+ dev_warn(&pdev->dev, "Skipping virtual function:%d\n",
+ extended_func_num(pdev));
+ err = -ENODEV;
+ goto err_release_regions;
+ }
+ }
+ }
+
+ err = mlx4_load_one(pdev, pci_dev_data, total_vfs, nvfs, priv);
+ if (err)
+ goto err_release_regions;
+ return 0;
err_release_regions:
pci_release_regions(pdev);
@@ -2682,6 +2710,7 @@ static int mlx4_init_one(struct pci_dev *pdev, const struct pci_device_id *id)
{
struct mlx4_priv *priv;
struct mlx4_dev *dev;
+ int ret;
printk_once(KERN_INFO "%s", mlx4_version);
@@ -2690,13 +2719,18 @@ static int mlx4_init_one(struct pci_dev *pdev, const struct pci_device_id *id)
return -ENOMEM;
dev = &priv->dev;
+ dev->pdev = pdev;
pci_set_drvdata(pdev, dev);
priv->pci_dev_data = id->driver_data;
- return __mlx4_init_one(pdev, id->driver_data);
+ ret = __mlx4_init_one(pdev, id->driver_data, priv);
+ if (ret)
+ kfree(priv);
+
+ return ret;
}
-static void __mlx4_remove_one(struct pci_dev *pdev)
+static void mlx4_unload_one(struct pci_dev *pdev)
{
struct mlx4_dev *dev = pci_get_drvdata(pdev);
struct mlx4_priv *priv = mlx4_priv(dev);
@@ -2775,8 +2809,6 @@ static void __mlx4_remove_one(struct pci_dev *pdev)
kfree(dev->caps.qp1_proxy);
kfree(dev->dev_vfs);
- pci_release_regions(pdev);
- pci_disable_device(pdev);
memset(priv, 0, sizeof(*priv));
priv->pci_dev_data = pci_dev_data;
priv->removed = 1;
@@ -2787,7 +2819,9 @@ static void mlx4_remove_one(struct pci_dev *pdev)
struct mlx4_dev *dev = pci_get_drvdata(pdev);
struct mlx4_priv *priv = mlx4_priv(dev);
- __mlx4_remove_one(pdev);
+ mlx4_unload_one(pdev);
+ pci_release_regions(pdev);
+ pci_disable_device(pdev);
kfree(priv);
pci_set_drvdata(pdev, NULL);
}
@@ -2796,11 +2830,22 @@ int mlx4_restart_one(struct pci_dev *pdev)
{
struct mlx4_dev *dev = pci_get_drvdata(pdev);
struct mlx4_priv *priv = mlx4_priv(dev);
- int pci_dev_data;
+ int nvfs[MLX4_MAX_PORTS + 1] = {0, 0, 0};
+ int pci_dev_data, err, total_vfs;
pci_dev_data = priv->pci_dev_data;
- __mlx4_remove_one(pdev);
- return __mlx4_init_one(pdev, pci_dev_data);
+ total_vfs = dev->num_vfs;
+ memcpy(nvfs, dev->nvfs, sizeof(dev->nvfs));
+
+ mlx4_unload_one(pdev);
+ err = mlx4_load_one(pdev, pci_dev_data, total_vfs, nvfs, priv);
+ if (err) {
+ mlx4_err(dev, "%s: ERROR: mlx4_load_one failed, pci_name=%s, err=%d\n",
+ __func__, pci_name(pdev), err);
+ return err;
+ }
+
+ return err;
}
static const struct pci_device_id mlx4_pci_table[] = {
@@ -2854,7 +2899,7 @@ MODULE_DEVICE_TABLE(pci, mlx4_pci_table);
static pci_ers_result_t mlx4_pci_err_detected(struct pci_dev *pdev,
pci_channel_state_t state)
{
- __mlx4_remove_one(pdev);
+ mlx4_unload_one(pdev);
return state == pci_channel_io_perm_failure ?
PCI_ERS_RESULT_DISCONNECT : PCI_ERS_RESULT_NEED_RESET;
@@ -2866,7 +2911,7 @@ static pci_ers_result_t mlx4_pci_slot_reset(struct pci_dev *pdev)
struct mlx4_priv *priv = mlx4_priv(dev);
int ret;
- ret = __mlx4_init_one(pdev, priv->pci_dev_data);
+ ret = __mlx4_init_one(pdev, priv->pci_dev_data, priv);
return ret ? PCI_ERS_RESULT_DISCONNECT : PCI_ERS_RESULT_RECOVERED;
}
@@ -2880,7 +2925,7 @@ static struct pci_driver mlx4_driver = {
.name = DRV_NAME,
.id_table = mlx4_pci_table,
.probe = mlx4_init_one,
- .shutdown = __mlx4_remove_one,
+ .shutdown = mlx4_unload_one,
.remove = mlx4_remove_one,
.err_handler = &mlx4_err_handler,
};
diff --git a/include/linux/mlx4/device.h b/include/linux/mlx4/device.h
index 03b5608..b2f8ab9 100644
--- a/include/linux/mlx4/device.h
+++ b/include/linux/mlx4/device.h
@@ -707,6 +707,7 @@ struct mlx4_dev {
u64 regid_promisc_array[MLX4_MAX_PORTS + 1];
u64 regid_allmulti_array[MLX4_MAX_PORTS + 1];
struct mlx4_vf_dev *dev_vfs;
+ int nvfs[MLX4_MAX_PORTS + 1];
};
struct mlx4_eqe {
--
1.7.1
^ permalink raw reply related
* [PATCH net-next 3/4] net/mlx4_core: Protect QUERY_PORT wrapper from untrusted guests
From: Or Gerlitz @ 2014-09-30 9:03 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Amir Vadai, Jack Morgenstein, Or Gerlitz
In-Reply-To: <1412067830-30438-1-git-send-email-ogerlitz@mellanox.com>
From: Jack Morgenstein <jackm@dev.mellanox.co.il>
The function mlx4_QUERY_PORT_wrapper implements only the
QUERY_PORT "general" case (opcode modifier = 0).
Verify that the opcode modifier is zero, and also that the
input modifier contains only the port number in bits 0..7
(all other bits should be zero).
Signed-off-by: Jack Morgenstein <jackm@dev.mellanox.co.il>
Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx4/fw.c | 9 +++++++--
1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/fw.c b/drivers/net/ethernet/mellanox/mlx4/fw.c
index 13b2e4a..2e88a23 100644
--- a/drivers/net/ethernet/mellanox/mlx4/fw.c
+++ b/drivers/net/ethernet/mellanox/mlx4/fw.c
@@ -982,8 +982,13 @@ int mlx4_QUERY_PORT_wrapper(struct mlx4_dev *dev, int slave,
if (port < 0)
return -EINVAL;
- vhcr->in_modifier = (vhcr->in_modifier & ~0xFF) |
- (port & 0xFF);
+ /* Protect against untrusted guests: enforce that this is the
+ * QUERY_PORT general query.
+ */
+ if (vhcr->op_modifier || vhcr->in_modifier & ~0xFF)
+ return -EINVAL;
+
+ vhcr->in_modifier = port;
err = mlx4_cmd_box(dev, 0, outbox->dma, vhcr->in_modifier, 0,
MLX4_CMD_QUERY_PORT, MLX4_CMD_TIME_CLASS_B,
--
1.7.1
^ permalink raw reply related
* [PATCH net-next 0/4] mlx4_core driver updates
From: Or Gerlitz @ 2014-09-30 9:03 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Amir Vadai, Jack Morgenstein, Or Gerlitz
Hi Dave,
A series from Jack and Co of low-level fixes for the mlx4_core driver
Or.
Jack Morgenstein (3):
net/mlx4_core: Don't disable SRIOV if there are active VFs
net/mlx4_core: Protect QUERY_PORT wrapper from untrusted guests
net/mlx4_core: Deprecate error message at ConnectX-2 cards startup to debug
Majd Dibbiny (1):
net/mlx4_core: New init and exit flow for mlx4_core
drivers/net/ethernet/mellanox/mlx4/cmd.c | 14 +-
drivers/net/ethernet/mellanox/mlx4/fw.c | 9 +-
drivers/net/ethernet/mellanox/mlx4/main.c | 421 ++++++++++++++++-------------
include/linux/mlx4/device.h | 1 +
4 files changed, 260 insertions(+), 185 deletions(-)
^ permalink raw reply
* [PATCH net-next 4/4] net/mlx4_core: Deprecate error message at ConnectX-2 cards startup to debug
From: Or Gerlitz @ 2014-09-30 9:03 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Amir Vadai, Jack Morgenstein, Or Gerlitz
In-Reply-To: <1412067830-30438-1-git-send-email-ogerlitz@mellanox.com>
From: Jack Morgenstein <jackm@dev.mellanox.co.il>
ConnectX2 HCAs have max_mtu=4k and max_vl=8 vls. However, if you specify
a 4K mtu, the max_vl supported for 4K is 4 vls. The driver at startup
attempts to set a 4K mtu using the max_vl value obtained from QUERY_PORT.
Since the max_vl value is 8 vls (which is supported up to 2K mtu size),
the first attempt to set the mtl/vl port value will fail, generating
the following error message in the log:
mlx4_core 0000:06:00.0: command 0xc failed: fw status = 0x40
The driver then tries again, using mtu=4k, vls=4, and this succeeds.
Since we do not want to have this error message always displayed at driver
start when there are ConnectX2 HCAs on the host, we deprecate the error
message for this specific command/input_modifier/opcode_modifier/fw-status
to be debug.
Signed-off-by: Jack Morgenstein <jackm@dev.mellanox.co.il>
Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx4/cmd.c | 14 ++++++++++++--
1 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/cmd.c b/drivers/net/ethernet/mellanox/mlx4/cmd.c
index 02a2e90..436c82f 100644
--- a/drivers/net/ethernet/mellanox/mlx4/cmd.c
+++ b/drivers/net/ethernet/mellanox/mlx4/cmd.c
@@ -580,8 +580,18 @@ static int mlx4_cmd_wait(struct mlx4_dev *dev, u64 in_param, u64 *out_param,
err = context->result;
if (err) {
- mlx4_err(dev, "command 0x%x failed: fw status = 0x%x\n",
- op, context->fw_status);
+ /* Since we do not want to have this error message always
+ * displayed at driver start when there are ConnectX2 HCAs
+ * on the host, we deprecate the error message for this
+ * specific command/input_mod/opcode_mod/fw-status to be debug.
+ */
+ if (op == MLX4_CMD_SET_PORT && in_modifier == 1 &&
+ op_modifier == 0 && context->fw_status == CMD_STAT_BAD_SIZE)
+ mlx4_dbg(dev, "command 0x%x failed: fw status = 0x%x\n",
+ op, context->fw_status);
+ else
+ mlx4_err(dev, "command 0x%x failed: fw status = 0x%x\n",
+ op, context->fw_status);
goto out;
}
--
1.7.1
^ permalink raw reply related
* [PATCH net-next 1/4] net/mlx4_core: Don't disable SRIOV if there are active VFs
From: Or Gerlitz @ 2014-09-30 9:03 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Amir Vadai, Jack Morgenstein, Tal Alon, Or Gerlitz
In-Reply-To: <1412067830-30438-1-git-send-email-ogerlitz@mellanox.com>
From: Jack Morgenstein <jackm@dev.mellanox.co.il>
When unloading the host driver while there are VFs active on VMs,
the PF driver disabled sriov anyway, causing kernel crashes.
We now leave SRIOV enabled, to avoid that.
When the driver is reloaded, __mlx4_init_one is invoked on the PF.
It now checks to see if SRIOV is already enabled on the PF -- and
if so does not enable sriov again.
Signed-off-by: Tal Alon <talal@mellanox.com>
Signed-off-by: Jack Morgenstein <jackm@dev.mellanox.co.il>
Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx4/main.c | 28 +++++++++++++++++++++-------
1 files changed, 21 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
index 1f10023..4e9857b 100644
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -2272,6 +2272,7 @@ static int __mlx4_init_one(struct pci_dev *pdev, int pci_dev_data)
unsigned total_vfs = 0;
int sriov_initialized = 0;
unsigned int i;
+ int existing_vfs = 0;
pr_info(DRV_NAME ": Initializing %s\n", pci_name(pdev));
@@ -2431,7 +2432,15 @@ static int __mlx4_init_one(struct pci_dev *pdev, int pci_dev_data)
err = 0;
} else {
atomic_inc(&pf_loading);
- err = pci_enable_sriov(pdev, total_vfs);
+ existing_vfs = pci_num_vf(pdev);
+ if (existing_vfs) {
+ err = 0;
+ if (existing_vfs != total_vfs)
+ mlx4_err(dev, "SR-IOV was already enabled, but with num_vfs (%d) different than requested (%d)\n",
+ existing_vfs, total_vfs);
+ } else {
+ err = pci_enable_sriov(pdev, total_vfs);
+ }
if (err) {
mlx4_err(dev, "Failed to enable SR-IOV, continuing without SR-IOV (err = %d)\n",
err);
@@ -2645,7 +2654,7 @@ err_cmd:
mlx4_cmd_cleanup(dev);
err_sriov:
- if (dev->flags & MLX4_FLAG_SRIOV)
+ if (dev->flags & MLX4_FLAG_SRIOV && !existing_vfs)
pci_disable_sriov(pdev);
err_rel_own:
@@ -2693,16 +2702,21 @@ static void __mlx4_remove_one(struct pci_dev *pdev)
struct mlx4_priv *priv = mlx4_priv(dev);
int pci_dev_data;
int p;
+ int active_vfs = 0;
if (priv->removed)
return;
pci_dev_data = priv->pci_dev_data;
- /* in SRIOV it is not allowed to unload the pf's
- * driver while there are alive vf's */
- if (mlx4_is_master(dev) && mlx4_how_many_lives_vf(dev))
- pr_warn("Removing PF when there are assigned VF's !!!\n");
+ /* Disabling SR-IOV is not allowed while there are active vf's */
+ if (mlx4_is_master(dev)) {
+ active_vfs = mlx4_how_many_lives_vf(dev);
+ if (active_vfs) {
+ pr_warn("Removing PF when there are active VF's !!\n");
+ pr_warn("Will not disable SR-IOV.\n");
+ }
+ }
mlx4_stop_sense(dev);
mlx4_unregister_device(dev);
@@ -2745,7 +2759,7 @@ static void __mlx4_remove_one(struct pci_dev *pdev)
if (dev->flags & MLX4_FLAG_MSI_X)
pci_disable_msix(pdev);
- if (dev->flags & MLX4_FLAG_SRIOV) {
+ if (dev->flags & MLX4_FLAG_SRIOV && !active_vfs) {
mlx4_warn(dev, "Disabling SR-IOV\n");
pci_disable_sriov(pdev);
dev->num_vfs = 0;
--
1.7.1
^ permalink raw reply related
* Re: [PATCH] bridge: Fix redefined references
From: Pablo Neira Ayuso @ 2014-09-30 9:05 UTC (permalink / raw)
To: David Miller; +Cc: therbert, netdev
In-Reply-To: <20140930.010143.591375062294828360.davem@davemloft.net>
On Tue, Sep 30, 2014 at 01:01:43AM -0400, David Miller wrote:
> From: Tom Herbert <therbert@google.com>
> Date: Mon, 29 Sep 2014 19:16:44 -0700
>
> > Commit 34666d467cbf1e2e3 ("netfilter: bridge: move br_netfilter out of
> > the core") cause compiler errors when CONFIG_BRIDGE_NETFILTER is not
> > defined.
> >
> > net/bridge/br_nf_core.c:77:1: error: expected identifier or '(' before '{' token
> > net/bridge/br_nf_core.c:88:115: error: redefinition of 'br_nf_core_init'
> > In file included from net/bridge/br_nf_core.c:23:0:
> > net/bridge/br_private.h:762:59: note: previous definition of 'br_nf_core_init' was here
> > net/bridge/br_nf_core.c:93:6: error: redefinition of 'br_nf_core_fini'
> > In file included from net/bridge/br_nf_core.c:23:0:
> > net/bridge/br_private.h:763:60: note: previous definition of 'br_nf_core_fini' was here
> >
> > Add #if IS_ENABLED(CONFIG_BRIDGE_NETFILTER) around definitions of
> > the redefined functions.
> >
> > Signed-off-by: Tom Herbert <therbert@google.com>
>
> Pablo please either ACK this or provide an alternative fix ASAP.
>
> This bridging build breakage is impacting a lot of people.
Sorry for the trouble this is causing.
I just sent a patch from Florian to resolve the compilataion breakage
when CONFIG_BRIDGE_NETFILTER=n.
Thanks.
^ permalink raw reply
* [PATCH net-next] netfilter: bridge: build br_nf_core only if required
From: Pablo Neira Ayuso @ 2014-09-30 8:59 UTC (permalink / raw)
To: netfilter-devel
Cc: davem, netdev, Eric Dumazet, Tom Herbert, Florian Westphal
From: Florian Westphal <fw@strlen.de>
Eric reports build failure with
CONFIG_BRIDGE_NETFILTER=n
We insist to build br_nf_core.o unconditionally, but we must only do so
if br_netfilter was enabled, else it fails to build due to
functions being defined to empty stubs (and some structure members
being defined out).
Also, BRIDGE_NETFILTER=y|m makes no sense when BRIDGE=n.
Fixes: 34666d467 (netfilter: bridge: move br_netfilter out of the core)
Reported-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/Kconfig | 2 +-
net/bridge/Makefile | 5 +++--
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/net/Kconfig b/net/Kconfig
index dc5d700..d6b138e 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -177,7 +177,7 @@ config NETFILTER_ADVANCED
config BRIDGE_NETFILTER
tristate "Bridged IP/ARP packets filtering"
- depends on (BRIDGE || BRIDGE=n)
+ depends on BRIDGE
depends on NETFILTER && INET
depends on NETFILTER_ADVANCED
default m
diff --git a/net/bridge/Makefile b/net/bridge/Makefile
index 5e3eac5..fd7ee03 100644
--- a/net/bridge/Makefile
+++ b/net/bridge/Makefile
@@ -6,11 +6,12 @@ obj-$(CONFIG_BRIDGE) += bridge.o
bridge-y := br.o br_device.o br_fdb.o br_forward.o br_if.o br_input.o \
br_ioctl.o br_stp.o br_stp_bpdu.o \
- br_stp_if.o br_stp_timer.o br_netlink.o \
- br_nf_core.o
+ br_stp_if.o br_stp_timer.o br_netlink.o
bridge-$(CONFIG_SYSFS) += br_sysfs_if.o br_sysfs_br.o
+bridge-$(subst m,y,$(CONFIG_BRIDGE_NETFILTER)) += br_nf_core.o
+
obj-$(CONFIG_BRIDGE_NETFILTER) += br_netfilter.o
bridge-$(CONFIG_BRIDGE_IGMP_SNOOPING) += br_multicast.o br_mdb.o
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH 32/34] netfilter: bridge: move br_netfilter out of the core
From: Pablo Neira Ayuso @ 2014-09-30 8:56 UTC (permalink / raw)
To: Florian Westphal; +Cc: Eric Dumazet, netfilter-devel, davem, netdev
In-Reply-To: <20140929231748.GA11709@breakpoint.cc>
On Tue, Sep 30, 2014 at 01:17:48AM +0200, Florian Westphal wrote:
> Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > On Mon, 2014-09-29 at 14:39 +0200, Pablo Neira Ayuso wrote:
> > > Jesper reported that br_netfilter always registers the hooks since
> > > this is part of the bridge core. This harms performance for people that
> > > don't need this.
> > >
> > > This patch modularizes br_netfilter so it can be rmmod'ed, thus,
> > > the hooks can be unregistered. I think the bridge netfilter should have
> > > been a separated module since the beginning, Patrick agreed on that.
> > >
> > > Note that this is breaking compatibility for users that expect that
> > > bridge netfilter is going to be available after explicitly 'modprobe
> > > bridge' or via automatic load through brctl.
> > >
> > > However, the damage can be easily undone by modprobing br_netfilter.
> > > The bridge core also spots a message to provide a clue to people that
> > > didn't notice that this has been deprecated.
> > >
> > > On top of that, the plan is that nftables will not rely on this software
> > > layer, but integrate the connection tracking into the bridge layer to
> > > enable stateful filtering and NAT, which is was bridge netfilter users
> > > seem to require.
> > >
> > > This patch still keeps the fake_dst_ops in the bridge core, since this
> > > is required by when the bridge port is initialized. So we can safely
> > > modprobe/rmmod br_netfilter anytime.
> > >
> > > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> > > Acked-by: Florian Westphal <fw@strlen.de>
> > > ---
> >
> > Hmm... What am I missing here ?
> > # CONFIG_BRIDGE_NETFILTER is not set
>
> Nothing. Our fault. br_nf_core.o should not be built in this case.
>
> > $ make net/bridge/br_nf_core.o
> [..]
> > CC [M] net/bridge/br_nf_core.o
> > net/bridge/br_nf_core.c:77:1: error: expected identifier or ‘(’ before ‘{’ token
> > net/bridge/br_nf_core.c:88:12: error: redefinition of ‘br_nf_core_init’
>
> This patch seems to fix it for me. Pablo, can you double-check?
Thanks Florian, I'll pass this patch to David asap.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [net-next PATCH V5] qdisc: bulk dequeue support for qdiscs with TCQ_F_ONETXQUEUE
From: Jesper Dangaard Brouer @ 2014-09-30 8:53 UTC (permalink / raw)
To: Jesper Dangaard Brouer, netdev, David S. Miller, Tom Herbert,
Eric Dumazet, Hannes Frederic Sowa, Florian Westphal,
Daniel Borkmann
Cc: Jamal Hadi Salim, Alexander Duyck, John Fastabend, Dave Taht,
toke
Based on DaveM's recent API work on dev_hard_start_xmit(), that allows
sending/processing an entire skb list.
This patch implements qdisc bulk dequeue, by allowing multiple packets
to be dequeued in dequeue_skb().
The optimization principle for this is two fold, (1) to amortize
locking cost and (2) avoid expensive tailptr update for notifying HW.
(1) Several packets are dequeued while holding the qdisc root_lock,
amortizing locking cost over several packet. The dequeued SKB list is
processed under the TXQ lock in dev_hard_start_xmit(), thus also
amortizing the cost of the TXQ lock.
(2) Further more, dev_hard_start_xmit() will utilize the skb->xmit_more
API to delay HW tailptr update, which also reduces the cost per
packet.
One restriction of the new API is that every SKB must belong to the
same TXQ. This patch takes the easy way out, by restricting bulk
dequeue to qdisc's with the TCQ_F_ONETXQUEUE flag, that specifies the
qdisc only have attached a single TXQ.
Some detail about the flow; dev_hard_start_xmit() will process the skb
list, and transmit packets individually towards the driver (see
xmit_one()). In case the driver stops midway in the list, the
remaining skb list is returned by dev_hard_start_xmit(). In
sch_direct_xmit() this returned list is requeued by dev_requeue_skb().
To avoid overshooting the HW limits, which results in requeuing, the
patch limits the amount of bytes dequeued, based on the drivers BQL
limits. In-effect bulking will only happen for BQL enabled drivers.
Besides the bytelimit from BQL, also limit bulking to maximum 7
packets to avoid any issues with available descriptor in HW and
any HoL issues measured at 100Mbit/s.
For now, as a conservative approach, don't bulk dequeue GSO and
segmented GSO packets, because testing showed requeuing occuring
with segmented GSO packets.
Jointed work with Hannes, Daniel and Florian.
Testing:
--------
Demonstrating the performance improvement of qdisc dequeue bulking, is
tricky because the effect only "kicks-in" once the qdisc system have a
backlog. Thus, for a backlog to form, we need either 1) to exceed wirespeed
of the link or 2) exceed the capability of the device driver.
For practical use-cases, the measureable effect of this will be a
reduction in CPU usage
01-TCP_STREAM:
--------------
Testing effect for TCP involves disabling TSO and GSO, because TCP
already benefit from bulking, via TSO and especially for GSO segmented
packets. This patch view TSO/GSO as a seperate kind of bulking, and
avoid further bulking of these packet types.
The measured perf diff benefit (at 10Gbit/s) for TCP_STREAM were 4.66%
less CPU used on calls to _raw_spin_lock() (mostly from sch_direct_xmit).
Tool mpstat, while stressing the system with netperf 24x TCP_STREAM, shows:
* Disabled bulking: 8.30% soft 88.75% idle
* Enabled bulking: 7.80% soft 89.36% idle
02-UDP_STREAM
-------------
The measured perf diff benefit for UDP_STREAM were 5.26% less CPU used
on calls to _raw_spin_lock(). UDP_STREAM with packet size -m 1472 (to
avoid sending UDP/IP fragments).
03-trafgen driver test
----------------------
The performance of the 10Gbit/s ixgbe driver is limited due to
updating the HW ring-queue tail-pointer on every packet. As
previously demonstrated with pktgen.
Using trafgen to send RAW frames from userspace (via AF_PACKET), and
forcing it through qdisc path (with option --qdisc-path and -t0),
sending with 12 CPUs.
I can demonstrate this driver layer limitation:
* 12.8 Mpps with no qdisc bulking
* 14.8 Mpps with qdisc bulking (full 10G-wirespeed)
04-netperf-wrapper
------------------
I've designed some tests for the tool "netperf-wrapper" that tries to
measure the Head-of-Line (HoL) blocking effect.
The bulking "budget" limit of 7 packets are choosen, based on testing
with netperf-wrapper. With driver igb at 100Mbit/s and a budget of
max 7 packets I cannot measure any added HoL latency. Increasing this
value to 8 packets, I observed a fluctuating added 0.24ms delay
corrosponding to 3000 bytes at 100Mbit/s.
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
V5:
- Changed "budget" limit to 7 packets (from 8)
- Choosing to use skb->len, because BQL uses that
- Added testing results to patch desc
V4:
- Patch rewritten in the Red Hat Neuchatel office jointed work with
Hannes, Daniel and Florian.
- Conservative approach of only using on BQL enabled drivers
- No user tunable parameter, but limit bulking to 8 packets.
- For now, avoid bulking GSO packets packets.
V3:
- Correct use of BQL
- Some minor adjustments based on feedback.
- Default setting only bulk dequeue 1 extra packet (2 packets).
V2:
- Restruct functions, split out functionality
- Use BQL bytelimit to avoid overshooting driver limits
include/net/sch_generic.h | 16 +++++++++++++++
net/sched/sch_generic.c | 47 +++++++++++++++++++++++++++++++++++++++++++--
2 files changed, 61 insertions(+), 2 deletions(-)
diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index e65b8e0..0654dfa 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -6,6 +6,7 @@
#include <linux/rcupdate.h>
#include <linux/pkt_sched.h>
#include <linux/pkt_cls.h>
+#include <linux/dynamic_queue_limits.h>
#include <net/gen_stats.h>
#include <net/rtnetlink.h>
@@ -111,6 +112,21 @@ static inline void qdisc_run_end(struct Qdisc *qdisc)
qdisc->__state &= ~__QDISC___STATE_RUNNING;
}
+static inline bool qdisc_may_bulk(const struct Qdisc *qdisc)
+{
+ return qdisc->flags & TCQ_F_ONETXQUEUE;
+}
+
+static inline int qdisc_avail_bulklimit(const struct netdev_queue *txq)
+{
+#ifdef CONFIG_BQL
+ /* Non-BQL migrated drivers will return 0, too. */
+ return dql_avail(&txq->dql);
+#else
+ return 0;
+#endif
+}
+
static inline bool qdisc_is_throttled(const struct Qdisc *qdisc)
{
return test_bit(__QDISC_STATE_THROTTLED, &qdisc->state) ? true : false;
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 11b28f6..0fa8f0e 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -56,6 +56,42 @@ static inline int dev_requeue_skb(struct sk_buff *skb, struct Qdisc *q)
return 0;
}
+static struct sk_buff *try_bulk_dequeue_skb(struct Qdisc *q,
+ struct sk_buff *head_skb,
+ int bytelimit)
+{
+ struct sk_buff *skb, *tail_skb = head_skb;
+ int budget = 7; /* For now, conservatively choosen limit */
+
+ while (bytelimit > 0 && --budget > 0) {
+ /* For now, don't bulk dequeue GSO (or GSO segmented) pkts */
+ if (tail_skb->next || skb_is_gso(tail_skb))
+ break;
+
+ skb = q->dequeue(q);
+ if (!skb)
+ break;
+
+ bytelimit -= skb->len; /* covers GSO len */
+ skb = validate_xmit_skb(skb, qdisc_dev(q));
+ if (!skb)
+ break;
+
+ /* "skb" can be a skb list after validate call above
+ * (GSO segmented), but it is okay to append it to
+ * current tail_skb->next, because next round will exit
+ * in-case "tail_skb->next" is a skb list.
+ */
+ tail_skb->next = skb;
+ tail_skb = skb;
+ }
+
+ return head_skb;
+}
+
+/* Note that dequeue_skb can possibly return a SKB list (via skb->next).
+ * A requeued skb (via q->gso_skb) can also be a SKB list.
+ */
static inline struct sk_buff *dequeue_skb(struct Qdisc *q)
{
struct sk_buff *skb = q->gso_skb;
@@ -70,10 +106,17 @@ static inline struct sk_buff *dequeue_skb(struct Qdisc *q)
} else
skb = NULL;
} else {
- if (!(q->flags & TCQ_F_ONETXQUEUE) || !netif_xmit_frozen_or_stopped(txq)) {
+ if (!(q->flags & TCQ_F_ONETXQUEUE) ||
+ !netif_xmit_frozen_or_stopped(txq)) {
+ int bytelimit = qdisc_avail_bulklimit(txq);
+
skb = q->dequeue(q);
- if (skb)
+ if (skb) {
+ bytelimit -= skb->len;
skb = validate_xmit_skb(skb, qdisc_dev(q));
+ }
+ if (skb && qdisc_may_bulk(q))
+ skb = try_bulk_dequeue_skb(q, skb, bytelimit);
}
}
^ permalink raw reply related
* [PATCH net] r8152: fix setting RTL8152_UNPLUG
From: Hayes Wang @ 2014-09-30 8:48 UTC (permalink / raw)
To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
The flag of RTL8152_UNPLUG should only be set when the device is
unplugged, not each time the rtl8152_disconnect() is called.
Otherwise, the device wouldn't be stopped normally.
Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
drivers/net/usb/r8152.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index e039442..f68a4e6 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -3576,7 +3576,11 @@ static void rtl8152_disconnect(struct usb_interface *intf)
usb_set_intfdata(intf, NULL);
if (tp) {
- set_bit(RTL8152_UNPLUG, &tp->flags);
+ struct usb_device *udev = tp->udev;
+
+ if (udev->state == USB_STATE_NOTATTACHED)
+ set_bit(RTL8152_UNPLUG, &tp->flags);
+
tasklet_kill(&tp->tl);
unregister_netdev(tp->netdev);
tp->rtl_ops.unload(tp);
--
1.9.3
^ permalink raw reply related
* [PATCH net 2/2] netxen: Fix bug in Tx completion path.
From: Manish Chopra @ 2014-09-30 7:56 UTC (permalink / raw)
To: davem; +Cc: netdev, umgwanakikbuti, Dept-GELinuxNICDev
In-Reply-To: <1412063796-25959-1-git-send-email-manish.chopra@qlogic.com>
o Driver is not updating sw_consumer while processing Tx completion
when interface is going down. Due to this interface down path gets
stuck forever waiting for NAPI to complete.
Signed-off-by: Manish Chopra <manish.chopra@qlogic.com>
---
.../net/ethernet/qlogic/netxen/netxen_nic_init.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c b/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
index ae4ec7b..5c40683 100644
--- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
+++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
@@ -1794,9 +1794,9 @@ int netxen_process_cmd_ring(struct netxen_adapter *adapter)
break;
}
- if (count && netif_running(netdev)) {
- tx_ring->sw_consumer = sw_consumer;
+ tx_ring->sw_consumer = sw_consumer;
+ if (count && netif_running(netdev)) {
smp_mb();
if (netif_queue_stopped(netdev) && netif_carrier_ok(netdev))
--
1.7.1
^ permalink raw reply related
* [PATCH net 0/2] netxen: Bug fixes.
From: Manish Chopra @ 2014-09-30 7:56 UTC (permalink / raw)
To: davem; +Cc: netdev, umgwanakikbuti, Dept-GELinuxNICDev
Hi David,
This series fixes some TX specific issues.
* Move spin_lock(tx_clean_lock) in down path to fix
atomic sleep bug (Reported by Mike Galbraith).
* Fix hang in interface down while running traffic.
Please consider applying this to 'net'.
Thanks,
Manish
Manish Chopra (2):
netxen: Fix BUG "sleeping function called from invalid context"
netxen: Fix bug in Tx completion path
.../net/ethernet/qlogic/netxen/netxen_nic_init.c | 6 ++++--
.../net/ethernet/qlogic/netxen/netxen_nic_main.c | 2 --
2 files changed, 4 insertions(+), 4 deletions(-)
^ permalink raw reply
* [PATCH net 1/2] netxen: Fix BUG "sleeping function called from invalid context"
From: Manish Chopra @ 2014-09-30 7:56 UTC (permalink / raw)
To: davem; +Cc: netdev, umgwanakikbuti, Dept-GELinuxNICDev
In-Reply-To: <1412063796-25959-1-git-send-email-manish.chopra@qlogic.com>
o __netxen_nic_down() function might sleep while holding spinlock_t(tx_clean_lock).
Acquire this lock for only releasing TX buffers instead of taking it
for whole down path.
Reported-by: Mike Galbraith <umgwanakikbuti@gmail.com>
Signed-off-by: Manish Chopra <manish.chopra@qlogic.com>
---
.../net/ethernet/qlogic/netxen/netxen_nic_init.c | 2 ++
.../net/ethernet/qlogic/netxen/netxen_nic_main.c | 2 --
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c b/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
index 3205861..ae4ec7b 100644
--- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
+++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
@@ -135,6 +135,7 @@ void netxen_release_tx_buffers(struct netxen_adapter *adapter)
int i, j;
struct nx_host_tx_ring *tx_ring = adapter->tx_ring;
+ spin_lock(&adapter->tx_clean_lock);
cmd_buf = tx_ring->cmd_buf_arr;
for (i = 0; i < tx_ring->num_desc; i++) {
buffrag = cmd_buf->frag_array;
@@ -158,6 +159,7 @@ void netxen_release_tx_buffers(struct netxen_adapter *adapter)
}
cmd_buf++;
}
+ spin_unlock(&adapter->tx_clean_lock);
}
void netxen_free_sw_resources(struct netxen_adapter *adapter)
diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
index 1159031..5ec5a2b 100644
--- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
+++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
@@ -1186,7 +1186,6 @@ __netxen_nic_down(struct netxen_adapter *adapter, struct net_device *netdev)
return;
smp_mb();
- spin_lock(&adapter->tx_clean_lock);
netif_carrier_off(netdev);
netif_tx_disable(netdev);
@@ -1204,7 +1203,6 @@ __netxen_nic_down(struct netxen_adapter *adapter, struct net_device *netdev)
netxen_napi_disable(adapter);
netxen_release_tx_buffers(adapter);
- spin_unlock(&adapter->tx_clean_lock);
}
/* Usage: During suspend and firmware recovery module */
--
1.7.1
^ permalink raw reply related
* RE: [PATCH] tun: make sure interface usage can not overflow
From: David Laight @ 2014-09-30 8:20 UTC (permalink / raw)
To: 'Hannes Frederic Sowa', Kees Cook
Cc: linux-kernel@vger.kernel.org, David S. Miller, Jason Wang,
Zhi Yong Wu, Michael S. Tsirkin, Tom Herbert, Masatake YAMATO,
Xi Wang, stephen hemminger, netdev@vger.kernel.org
In-Reply-To: <1412021098.14757.6.camel@localhost>
From: Hannes Frederic
> On Mo, 2014-09-29 at 12:41 -0700, Kees Cook wrote:
> > On Mon, Sep 29, 2014 at 4:04 AM, David Laight <David.Laight@aculab.com> wrote:
> > > From: Kees Cook
> > >> This makes the size argument a const, since it is always populated by
> > >> the caller.
> > >
> > > There is almost no point making parameters 'const.
> > > ('const foo *' makes sense).
> > >
> > >> Additionally double-checks to make sure the copy_from_user
> > >> can never overflow, keeping CONFIG_DEBUG_STRICT_USER_COPY_CHECKS happy:
> > >>
> > >> In function 'copy_from_user',
> > >> inlined from '__tun_chr_ioctl' at drivers/net/tun.c:1871:7:
> > >> ... copy_from_user() buffer size is not provably correct
> > >
> > > If 'ifreq_len' could be too big then you want to error the ioctl, not panic.
> > > If it can't be too big you don't need the check.
> >
> > The ifreq_len comes from the callers, and is the output of "sizeof"
> > which is const. Changing the function parameter to "const" means any
> > changes made in the future where the incoming value isn't const, the
> > compiler will throw a warning.
>
> Hmmm, I think you want something like BUILD_BUG_ON(!
> __builtin_constant_p(var)). const in function argument only ensures that
> the value cannot be modified in the function.
You'd have to do something in the header file - nothing in the function
body can do that check.
I've not got the source handy, but in this case is there any need to
actually pass the size at all?
Is it always the same fixed constant??
David
^ permalink raw reply
* Re: [patch iproute2 1/2] add bridge_slave device support
From: Jiri Pirko @ 2014-09-30 8:05 UTC (permalink / raw)
To: Or Gerlitz
Cc: Linux Netdev List, David Miller, Stephen Hemminger, Scott Feldman,
arvid.brodin, sucheta.chakraborty
In-Reply-To: <CAJ3xEMgLaJtxuerfJ0OXGPZ7Hn0tVBJYkc5Ck6wOGvFAKyctCQ@mail.gmail.com>
Mon, Sep 29, 2014 at 08:20:00PM CEST, gerlitz.or@gmail.com wrote:
>On Fri, Sep 5, 2014 at 4:55 PM, Jiri Pirko <jiri@resnulli.us> wrote:
>> Note this depends on "iproute2: allow to change slave options via
>> type_slave"
>
>OK, but how about sparing few words what this patch does?
Sorry, I thought it is obvious. It allows to use IFLA_INFO_SLAVE_DATA to
get/set bridge port options.
^ permalink raw reply
* Re: [PATCH] xen/xenbus: Use 'void' instead of 'int' for the return of xenbus_switch_state()
From: Chen Gang @ 2014-09-30 8:04 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: David Vrabel, ian.campbell, wei.liu2, boris.ostrovsky, bhelgaas,
jgross, yongjun_wei, mukesh.rathor, xen-devel,
netdev@vger.kernel.org, linux-pci, linux-scsi,
linux-kernel@vger.kernel.org
In-Reply-To: <20140929140229.GA28279@laptop.dumpdata.com>
On 9/29/14 22:02, Konrad Rzeszutek Wilk wrote:
> On Sat, Sep 27, 2014 at 12:36:42AM +0800, Chen Gang wrote:
>> When xenbus_switch_state() fails, it will call xenbus_switch_fatal()
>
> Only on the first depth, not on the subsequent ones (as in if
> the first xenbus_switch_fail fails, it won't try to call
> xenbus_switch_state again and again).
>
Yeah, I guess you want to give more completion for this comment, do not
mean the original comments is incorrect.
[...]
>>
>> Also need be sure that all callers which check the return value must let
>> 'err' be 0.
>
> I am bit uncomfortable with that, that is due to:
>
>
> .. snip..
>> diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
>> index 9c47b89..b5c3d47 100644
>> --- a/drivers/net/xen-netback/xenbus.c
>> +++ b/drivers/net/xen-netback/xenbus.c
>> @@ -337,10 +337,7 @@ static int netback_probe(struct xenbus_device *dev,
>> if (err)
>> pr_debug("Error writing multi-queue-max-queues\n");
>>
>> - err = xenbus_switch_state(dev, XenbusStateInitWait);
>> - if (err)
>> - goto fail;
>> -
>> + xenbus_switch_state(dev, XenbusStateInitWait);
>
> Which if it fails it won't call:
>
> 354 fail:
> 355 pr_debug("failed\n");
> 356 netback_remove(dev);
> 357 return err;
>
Originally, I intended left 'fail' code block for the next patch (which
I thought the next patch would need it).
Hmm... but any way, originally, I really need give additional comments
for it.
[...]
I skip all other contents which have already discussed by maintainers,
if I really miss something, please let me know.
Thanks.
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
^ permalink raw reply
* Re: [PATCH v1 5/5] driver-core: add driver asynchronous probe support
From: Luis R. Rodriguez @ 2014-09-30 7:47 UTC (permalink / raw)
To: Tom Gundersen
Cc: Luis R. Rodriguez, Michal Hocko, Greg KH, Julia Lawall,
Dmitry Torokhov, Takashi Iwai, Tejun Heo, Arjan van de Ven,
Robert Milasan, werner, Oleg Nesterov, hare, Benjamin Poirier,
Santosh Rastapur, pmladek, dbueso, LKML, Tetsuo Handa,
Joseph Salisbury, Kay Sievers, One Thousand Gnomes, Tim Gardner,
Pierre Fersing, Andrew Morton, Nagalakshmi
In-Reply-To: <20140930022751.GA14081@wotan.suse.de>
On Tue, Sep 30, 2014 at 04:27:51AM +0200, Luis R. Rodriguez wrote:
> On Sun, Sep 28, 2014 at 07:07:24PM +0200, Tom Gundersen wrote:
> > On Fri, Sep 26, 2014 at 11:57 PM, Luis R. Rodriguez
> > <mcgrof@do-not-panic.com> wrote:
> > > From: "Luis R. Rodriguez" <mcgrof@suse.com>
> > > 0) Not all drivers are killed, the signal is just sent and
> > > the kill will only be acted upoon if the driver you loaded
> > > happens to have some code path that either uses kthreads (which
> > > as of 786235ee are now killable), or uses some code which checks for
> > > fatal_signal_pending() on the kernel somewhere -- i.e: pci_read_vpd().
> >
> > Shouldn't this be seen as something to be fixed in the kernel?
>
> That's a great question. In practice now after CVE-2012-4398 and its series of
> patches added which enabled OOM to kill things followed by 786235ee to also
> handle OOM on kthreads it seems imperative we strive towards this, in practive
> however if you're getting OOMs on boot you have far more serious issue to be
> concerned over than handling CVE-2012-4398. Another issue is that even if we
> wanted to address this a critical right now on module loading driver error
> paths tend to be pretty buggy and we'd probably end up causing more issues than
> fixing anything if the sigkill that triggered this was an arbitrary timeout,
> specially if the timeout is not properly justified.
<-- snip -->
> So extending the kill onto more drivers *because* of the timeout is probably
> not a good reason as it would probably create more issue than fix anything
> right now.
A bit more on this. Tejun had added devres while trying to convert libata to
use iomap but in that process also help address buggy failure paths on drivers [0].
Even with devres in place and devm functions being available they actually
haven't been popularized until recent kernels [1]. There is even further
research on precicely these sorts of errors, such as "Hector: Detecting
Resource-Release Omission Faults in error-handling code for systems software" [2]
but unfortunately there is no data over time. Another paper is "An approach to
improving the structure of error-handling code in the Linux kernel" [3] which
tries to address moving error handling code in the middle of the function to gotos
to shared code at the end of the function...
So we have buggy error paths on drivers and trusting them unfortunately isn't
a good idea at this point. They should be fixed but saying we should equally
kill all drivers right now would likley introduce more issues than anything.
[0] http://lwn.net/Articles/215861/
[1] http://www.slideshare.net/ennael/kernel-recipes-2013?qid=f0888b85-377b-4b29-95c3-f4e59822f5b3&v=default&b=&from_search=6
See slide 6 on graph usage of devm functions over time
[2] http://coccinelle.lip6.fr/papers/dsn2013.pdf
[3] http://coccinelle.lip6.fr/papers/lctes11.pdf
Luis
^ permalink raw reply
* Re: [PATCH next] r8169: add support for Byte Queue Limits
From: Florian Westphal @ 2014-09-30 7:42 UTC (permalink / raw)
To: Tom Herbert
Cc: Florian Westphal, Linux Netdev List, Francois Romieu, Hayes Wang
In-Reply-To: <CA+mtBx8rpELodiiez8LkKupKBr9ynbQhrmAq8tcD4QKXHNjyaw@mail.gmail.com>
Tom Herbert <therbert@google.com> wrote:
> Watch inflight and limit in the byte_queue_limits for the queue.
> inflight must always go back to zero when link goes idle.
Yes, inflight goes to 0 when link is idle.
Output of
while true; do
for n in inflight limit; do
echo -n $n\ ; cat $n;
done; sleep 1;
done
during netperf run, 100mbit peer:
inflight 0
limit 3028
inflight 6056
limit 4542
[ no changes during test ]
limit 4542
inflight 3028
limit 6122
inflight 0
limit 6122
[ changed cable to 1gbit peer, restart netperf ]
inflight 37850
limit 36336
inflight 33308
limit 31794
inflight 33308
limit 31794
inflight 27252
limit 25738
[ no changes during test ]
inflight 27252
limit 25738
inflight 0
limit 28766
[ change cable to 100mbit peer, restart netperf ]
limit 28766
inflight 27370
limit 28766
inflight 4542
limit 5990
inflight 6056
limit 4542
inflight 6056
limit 4542
inflight 6056
limit 4542
inflight 6056
limit 4542
inflight 6056
limit 4542
inflight 6056
limit 4542
inflight 6056
limit 4542
inflight 6056
limit 4542
inflight 0
[ end of test ]
I think thats what its supposed to look like :-)
^ permalink raw reply
* RE: [PATCH v2] zd1211rw: replace ZD_ASSERT with lockdep_assert_held()
From: Sharma, Sanjeev @ 2014-09-30 7:42 UTC (permalink / raw)
To: Julian Calaby
Cc: Johannes Berg, dsd@gentoo.org, kune@deine-taler.de,
linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org
In-Reply-To: <CAGRGNgWOkzbu7sUxOqpgK_rzw1Xzny4aVFYOO6Rqd09-CpfvJA@mail.gmail.com>
-----Original Message-----
From: Julian Calaby [mailto:julian.calaby@gmail.com]
Sent: Monday, September 15, 2014 11:32 AM
To: Sharma, Sanjeev
Cc: Johannes Berg; dsd@gentoo.org; kune@deine-taler.de; linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org; netdev@vger.kernel.org
Subject: Re: [PATCH v2] zd1211rw: replace ZD_ASSERT with lockdep_assert_held()
Hi Sanjeev,
On Thu, Sep 11, 2014 at 8:36 PM, Sharma, Sanjeev <Sanjeev_Sharma@mentor.com> wrote:
> -----Original Message-----
> From: Johannes Berg [mailto:johannes@sipsolutions.net]
> Sent: Thursday, September 11, 2014 3:42 PM
> To: Sharma, Sanjeev
> Cc: dsd@gentoo.org; kune@deine-taler.de;
> linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org;
> netdev@vger.kernel.org
> Subject: Re: [PATCH v2] zd1211rw: replace ZD_ASSERT with
> lockdep_assert_held()
>
> On Thu, 2014-09-11 at 15:39 +0530, Sanjeev Sharma wrote:
>> on some architecture spin_is_locked() always return false in
>> uniprocessor configuration and therefore it would be advise to
>> replace with lockdep_assert_held().
>>
>> Signed-off-by: Sanjeev Sharma <Sanjeev_Sharma@mentor.com>
>> ---
>> Changes in v2:
>> - corrected the typo
>
>> Now it compiles, but you got the logic wrong.
>
>> +++ b/drivers/net/wireless/zd1211rw/zd_mac.c
>> @@ -235,7 +235,7 @@ void zd_mac_clear(struct zd_mac *mac) {
>> flush_workqueue(zd_workqueue);
>> zd_chip_clear(&mac->chip);
>> - ZD_ASSERT(!spin_is_locked(&mac->lock));
>> + lockdep_assert_held(&mac->lock);
>> ZD_MEMCLEAR(mac, sizeof(struct zd_mac)); }
>
>>Look closely at this again.
>
> I didn't understand where I put wrong logic ?
I find it helps to spell out what code is doing in words.
E.g. the line you're removing is:
ZD_ASSERT(!spin_is_locked(&mac->lock));
So, we'll assert when spin_is_locked(&mac->lock) is false, i.e. when
mac->lock is not spin locked.
This isn't the same as what you're replacing it with.
I feel logic is absolutely correct and if you expand it will looks like ..
+#define lockdep_assert_held(l) do { \
+ WARN_ON(debug_locks && !lockdep_is_held(l)); \
+ } while (0)
Please refer http://lkml.iu.edu/hypermail/linux/kernel/1203.2/00369.html and also see include/linux/lockdep.h for more detail.
Thanks
Sanjeev Sharma
^ permalink raw reply
* Re: [PATCH v1 5/5] driver-core: add driver asynchronous probe support
From: Luis R. Rodriguez @ 2014-09-30 7:21 UTC (permalink / raw)
To: Tejun Heo
Cc: gregkh, Luis R. Rodriguez, dmitry.torokhov, tiwai, arjan, teg,
rmilasan, werner, oleg, hare, bpoirier, santosh, pmladek, dbueso,
linux-kernel, Tetsuo Handa, Joseph Salisbury, Kay Sievers,
One Thousand Gnomes, Tim Gardner, Pierre Fersing, Andrew Morton,
Nagalakshmi Nandigama, Praveen Krishnamoorthy, Sreekanth Reddy,
Abhijit Mahajan, Casey Leedom
In-Reply-To: <20140929212601.GF15925@htj.dyndns.org>
On Mon, Sep 29, 2014 at 05:26:01PM -0400, Tejun Heo wrote:
> Hello, Luis.
>
> On Mon, Sep 29, 2014 at 11:22:08PM +0200, Luis R. Rodriguez wrote:
> > > > + /* For now lets avoid stupid bug reports */
> > > > + if (!strcmp(bus->name, "pci") ||
> > > > + !strcmp(bus->name, "pci_express") ||
> > > > + !strcmp(bus->name, "hid") ||
> > > > + !strcmp(bus->name, "sdio") ||
> > > > + !strcmp(bus->name, "gameport") ||
> > > > + !strcmp(bus->name, "mmc") ||
> > > > + !strcmp(bus->name, "i2c") ||
> > > > + !strcmp(bus->name, "platform") ||
> > > > + !strcmp(bus->name, "usb"))
> > > > + return true;
> > >
> > > Ugh... things like this tend to become permanent. Do we really need
> > > this? And how are we gonna find out what's broken why w/o bug
> > > reports?
> >
> > Yeah... well we have two options, one is have something like this to
> > at least make it generally useful or remove this and let folks who
> > care start fixing async for all modules. The downside to removing
> > this is it makes async probe pretty much useless on most systems
> > right now, it would mean systemd would have to probably consider
> > the list above if they wanted to start using this without expecting
> > systems to not work.
>
> So, I'd much prefer blacklist approach if something like this is a
> necessity. That way, we'd at least know what doesn't work.
For buses? Or do you mean you'd want to wait until we have a decent
list of drivers with the sync probe flag set? If the later it may take
a while to get that list for this to be somewhat useful.
Luis
^ permalink raw reply
* Re: [PATCH v1 5/5] driver-core: add driver asynchronous probe support
From: Luis R. Rodriguez @ 2014-09-30 7:15 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Luis R. Rodriguez, gregkh, tiwai, tj, arjan, teg, rmilasan,
werner, oleg, hare, bpoirier, santosh, pmladek, dbueso,
linux-kernel, Tetsuo Handa, Joseph Salisbury, Kay Sievers,
One Thousand Gnomes, Tim Gardner, Pierre Fersing, Andrew Morton,
Nagalakshmi Nandigama, Praveen Krishnamoorthy, Sreekanth Reddy,
Abhijit Mahajan, Casey Leedom
In-Reply-To: <20140928192247.GA28033@core.coreip.homeip.net>
On Sun, Sep 28, 2014 at 12:22:47PM -0700, Dmitry Torokhov wrote:
> Hi Luis,
>
> On Fri, Sep 26, 2014 at 02:57:17PM -0700, Luis R. Rodriguez wrote:
> > +static bool drv_enable_async_probe(struct device_driver *drv,
> > + struct bus_type *bus)
> > +{
> > + struct module *mod;
> > +
> > + if (!drv->owner || drv->sync_probe)
> > + return false;
>
> This bit is one of the biggest issues I have with the patch set. Why async
> probing is limited to modules only?
Because Tejun wanted to address this separately, so its not that we will restrict
this but we should have non-module solution added as an evolution on top of this,
as a secondary step.
> I mentioned several times that we need
> async probing for built-in drivers and the way you are structuring the flags
> (async by default for modules, possibly opt-out of async for modules, forcibly
> sync for built-in) it is hard to extend the infrastructure for built-in case.
I confess I haven't tried enabling built-in as a secondary step but its just
due to lack of time right now but I don't think impossible and think actually
think it should be fairly trivial. Are there real blockers to do this that
you see as an evolutionary step?
> Also, as far as I can see, you are only considering the case where driver is
> being bound to already registered devices. If you have a module that creates a
> device for a driver that is already loaded and takes long time to probe you
> would still be probing synchronously even if driver/module requested async
> behavior.
Can you provide an example code path hit here? I'll certainly like to address
that as well.
Luis
^ permalink raw reply
* [PATCH net-next v5 2/2] bonding: Simplify the xmit function for modes that use xmit_hash
From: Mahesh Bandewar @ 2014-09-30 6:27 UTC (permalink / raw)
To: Jay Vosburgh, Veaceslav Falico, Andy Gospodarek, David Miller
Cc: netdev, Mahesh Bandewar, Eric Dumazet, Maciej Zenczykowski
Earlier change to use usable slave array for TLB mode had an additional
performance advantage. So extending the same logic to all other modes
that use xmit-hash for slave selection (viz 802.3AD, and XOR modes).
Also consolidating this with the earlier TLB change.
The main idea is to build the usable slaves array in the control path
and use that array for slave selection during xmit operation.
Measured performance in a setup with a bond of 4x1G NICs with 200
instances of netperf for the modes involved (3ad, xor, tlb)
cmd: netperf -t TCP_RR -H <TargetHost> -l 60 -s 5
Mode TPS-Before TPS-After
802.3ad : 468,694 493,101
TLB (lb=0): 392,583 392,965
XOR : 475,696 484,517
Signed-off-by: Mahesh Bandewar <maheshb@google.com>
---
v1:
(a) If bond_update_slave_arr() fails to allocate memory, it will overwrite
the slave that need to be removed.
(b) Freeing of array will assign NULL (to handle bond->down to bond->up
transition gracefully.
(c) Change from pr_debug() to pr_err() if bond_update_slave_arr() returns
failure.
(d) XOR: bond_update_slave_arr() will consider mii-mon, arp-mon cases and
will populate the array even if these parameters are not used.
(e) 3AD: Should handle the ad_agg_selection_logic correctly.
v2:
(a) Removed rcu_read_{un}lock() calls from array manipulation code.
(b) Slave link-events now refresh array for all these modes.
(c) Moved free-array call from bond_close() to bond_uninit().
v3:
(a) Fixed null pointer dereference.
(b) Removed bond->lock lockdep dependency.
v4:
(a) Made to changes to comply with Nikolay's locking changes
(b) Added a work-queue to refresh slave-array when RTNL is not held
(c) Array refresh happens ONLY with RTNL now.
(d) alloc changed from GFP_ATOMIC to GFP_KERNEL
v5:
(a) Consolidated all delayed slave-array updates at one place in
3ad_state_machine_handler()
drivers/net/bonding/bond_3ad.c | 140 ++++++++++++------------------
drivers/net/bonding/bond_alb.c | 51 ++---------
drivers/net/bonding/bond_alb.h | 8 --
drivers/net/bonding/bond_main.c | 185 +++++++++++++++++++++++++++++++++++++---
drivers/net/bonding/bonding.h | 10 +++
5 files changed, 242 insertions(+), 152 deletions(-)
diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index 7e9e522fd476..2110215f3528 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -102,17 +102,20 @@ static const u8 lacpdu_mcast_addr[ETH_ALEN] = MULTICAST_LACPDU_ADDR;
/* ================= main 802.3ad protocol functions ================== */
static int ad_lacpdu_send(struct port *port);
static int ad_marker_send(struct port *port, struct bond_marker *marker);
-static void ad_mux_machine(struct port *port);
+static void ad_mux_machine(struct port *port, bool *update_slave_arr);
static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port);
static void ad_tx_machine(struct port *port);
static void ad_periodic_machine(struct port *port);
-static void ad_port_selection_logic(struct port *port);
-static void ad_agg_selection_logic(struct aggregator *aggregator);
+static void ad_port_selection_logic(struct port *port, bool *update_slave_arr);
+static void ad_agg_selection_logic(struct aggregator *aggregator,
+ bool *update_slave_arr);
static void ad_clear_agg(struct aggregator *aggregator);
static void ad_initialize_agg(struct aggregator *aggregator);
static void ad_initialize_port(struct port *port, int lacp_fast);
-static void ad_enable_collecting_distributing(struct port *port);
-static void ad_disable_collecting_distributing(struct port *port);
+static void ad_enable_collecting_distributing(struct port *port,
+ bool *update_slave_arr);
+static void ad_disable_collecting_distributing(struct port *port,
+ bool *update_slave_arr);
static void ad_marker_info_received(struct bond_marker *marker_info,
struct port *port);
static void ad_marker_response_received(struct bond_marker *marker,
@@ -796,8 +799,9 @@ static int ad_marker_send(struct port *port, struct bond_marker *marker)
/**
* ad_mux_machine - handle a port's mux state machine
* @port: the port we're looking at
+ * @update_slave_arr: Does slave array need update?
*/
-static void ad_mux_machine(struct port *port)
+static void ad_mux_machine(struct port *port, bool *update_slave_arr)
{
mux_states_t last_state;
@@ -901,7 +905,8 @@ static void ad_mux_machine(struct port *port)
switch (port->sm_mux_state) {
case AD_MUX_DETACHED:
port->actor_oper_port_state &= ~AD_STATE_SYNCHRONIZATION;
- ad_disable_collecting_distributing(port);
+ ad_disable_collecting_distributing(port,
+ update_slave_arr);
port->actor_oper_port_state &= ~AD_STATE_COLLECTING;
port->actor_oper_port_state &= ~AD_STATE_DISTRIBUTING;
port->ntt = true;
@@ -913,13 +918,15 @@ static void ad_mux_machine(struct port *port)
port->actor_oper_port_state |= AD_STATE_SYNCHRONIZATION;
port->actor_oper_port_state &= ~AD_STATE_COLLECTING;
port->actor_oper_port_state &= ~AD_STATE_DISTRIBUTING;
- ad_disable_collecting_distributing(port);
+ ad_disable_collecting_distributing(port,
+ update_slave_arr);
port->ntt = true;
break;
case AD_MUX_COLLECTING_DISTRIBUTING:
port->actor_oper_port_state |= AD_STATE_COLLECTING;
port->actor_oper_port_state |= AD_STATE_DISTRIBUTING;
- ad_enable_collecting_distributing(port);
+ ad_enable_collecting_distributing(port,
+ update_slave_arr);
port->ntt = true;
break;
default:
@@ -1187,12 +1194,13 @@ static void ad_periodic_machine(struct port *port)
/**
* ad_port_selection_logic - select aggregation groups
* @port: the port we're looking at
+ * @update_slave_arr: Does slave array need update?
*
* Select aggregation groups, and assign each port for it's aggregetor. The
* selection logic is called in the inititalization (after all the handshkes),
* and after every lacpdu receive (if selected is off).
*/
-static void ad_port_selection_logic(struct port *port)
+static void ad_port_selection_logic(struct port *port, bool *update_slave_arr)
{
struct aggregator *aggregator, *free_aggregator = NULL, *temp_aggregator;
struct port *last_port = NULL, *curr_port;
@@ -1347,7 +1355,7 @@ static void ad_port_selection_logic(struct port *port)
__agg_ports_are_ready(port->aggregator));
aggregator = __get_first_agg(port);
- ad_agg_selection_logic(aggregator);
+ ad_agg_selection_logic(aggregator, update_slave_arr);
}
/* Decide if "agg" is a better choice for the new active aggregator that
@@ -1435,6 +1443,7 @@ static int agg_device_up(const struct aggregator *agg)
/**
* ad_agg_selection_logic - select an aggregation group for a team
* @aggregator: the aggregator we're looking at
+ * @update_slave_arr: Does slave array need update?
*
* It is assumed that only one aggregator may be selected for a team.
*
@@ -1457,7 +1466,8 @@ static int agg_device_up(const struct aggregator *agg)
* __get_active_agg() won't work correctly. This function should be better
* called with the bond itself, and retrieve the first agg from it.
*/
-static void ad_agg_selection_logic(struct aggregator *agg)
+static void ad_agg_selection_logic(struct aggregator *agg,
+ bool *update_slave_arr)
{
struct aggregator *best, *active, *origin;
struct bonding *bond = agg->slave->bond;
@@ -1550,6 +1560,8 @@ static void ad_agg_selection_logic(struct aggregator *agg)
__disable_port(port);
}
}
+ /* Slave array needs update. */
+ *update_slave_arr = true;
}
/* if the selected aggregator is of join individuals
@@ -1678,24 +1690,30 @@ static void ad_initialize_port(struct port *port, int lacp_fast)
/**
* ad_enable_collecting_distributing - enable a port's transmit/receive
* @port: the port we're looking at
+ * @update_slave_arr: Does slave array need update?
*
* Enable @port if it's in an active aggregator
*/
-static void ad_enable_collecting_distributing(struct port *port)
+static void ad_enable_collecting_distributing(struct port *port,
+ bool *update_slave_arr)
{
if (port->aggregator->is_active) {
pr_debug("Enabling port %d(LAG %d)\n",
port->actor_port_number,
port->aggregator->aggregator_identifier);
__enable_port(port);
+ /* Slave array needs update */
+ *update_slave_arr = true;
}
}
/**
* ad_disable_collecting_distributing - disable a port's transmit/receive
* @port: the port we're looking at
+ * @update_slave_arr: Does slave array need update?
*/
-static void ad_disable_collecting_distributing(struct port *port)
+static void ad_disable_collecting_distributing(struct port *port,
+ bool *update_slave_arr)
{
if (port->aggregator &&
!MAC_ADDRESS_EQUAL(&(port->aggregator->partner_system),
@@ -1704,6 +1722,8 @@ static void ad_disable_collecting_distributing(struct port *port)
port->actor_port_number,
port->aggregator->aggregator_identifier);
__disable_port(port);
+ /* Slave array needs an update */
+ *update_slave_arr = true;
}
}
@@ -1868,6 +1888,7 @@ void bond_3ad_unbind_slave(struct slave *slave)
struct bonding *bond = slave->bond;
struct slave *slave_iter;
struct list_head *iter;
+ bool dummy_slave_update; /* Ignore this value as caller updates array */
/* Sync against bond_3ad_state_machine_handler() */
spin_lock_bh(&bond->mode_lock);
@@ -1951,7 +1972,8 @@ void bond_3ad_unbind_slave(struct slave *slave)
ad_clear_agg(aggregator);
if (select_new_active_agg)
- ad_agg_selection_logic(__get_first_agg(port));
+ ad_agg_selection_logic(__get_first_agg(port),
+ &dummy_slave_update);
} else {
netdev_warn(bond->dev, "unbinding aggregator, and could not find a new aggregator for its ports\n");
}
@@ -1966,7 +1988,8 @@ void bond_3ad_unbind_slave(struct slave *slave)
/* select new active aggregator */
temp_aggregator = __get_first_agg(port);
if (temp_aggregator)
- ad_agg_selection_logic(temp_aggregator);
+ ad_agg_selection_logic(temp_aggregator,
+ &dummy_slave_update);
}
}
}
@@ -1996,7 +2019,8 @@ void bond_3ad_unbind_slave(struct slave *slave)
if (select_new_active_agg) {
netdev_info(bond->dev, "Removing an active aggregator\n");
/* select new active aggregator */
- ad_agg_selection_logic(__get_first_agg(port));
+ ad_agg_selection_logic(__get_first_agg(port),
+ &dummy_slave_update);
}
}
break;
@@ -2031,6 +2055,7 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
struct slave *slave;
struct port *port;
bool should_notify_rtnl = BOND_SLAVE_NOTIFY_LATER;
+ bool update_slave_arr = false;
/* Lock to protect data accessed by all (e.g., port->sm_vars) and
* against running with bond_3ad_unbind_slave. ad_rx_machine may run
@@ -2058,7 +2083,7 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
}
aggregator = __get_first_agg(port);
- ad_agg_selection_logic(aggregator);
+ ad_agg_selection_logic(aggregator, &update_slave_arr);
}
bond_3ad_set_carrier(bond);
}
@@ -2074,8 +2099,8 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
ad_rx_machine(NULL, port);
ad_periodic_machine(port);
- ad_port_selection_logic(port);
- ad_mux_machine(port);
+ ad_port_selection_logic(port, &update_slave_arr);
+ ad_mux_machine(port, &update_slave_arr);
ad_tx_machine(port);
/* turn off the BEGIN bit, since we already handled it */
@@ -2093,6 +2118,9 @@ re_arm:
rcu_read_unlock();
spin_unlock_bh(&bond->mode_lock);
+ if (update_slave_arr)
+ bond_slave_arr_work_rearm(bond, 0);
+
if (should_notify_rtnl && rtnl_trylock()) {
bond_slave_state_notify(bond);
rtnl_unlock();
@@ -2283,6 +2311,11 @@ void bond_3ad_handle_link_change(struct slave *slave, char link)
port->sm_vars |= AD_PORT_BEGIN;
spin_unlock_bh(&slave->bond->mode_lock);
+
+ /* RTNL is held and mode_lock is released so it's safe
+ * to update slave_array here.
+ */
+ bond_update_slave_arr(slave->bond, NULL);
}
/**
@@ -2377,73 +2410,6 @@ int bond_3ad_get_active_agg_info(struct bonding *bond, struct ad_info *ad_info)
return ret;
}
-int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev)
-{
- struct bonding *bond = netdev_priv(dev);
- struct slave *slave, *first_ok_slave;
- struct aggregator *agg;
- struct ad_info ad_info;
- struct list_head *iter;
- int slaves_in_agg;
- int slave_agg_no;
- int agg_id;
-
- if (__bond_3ad_get_active_agg_info(bond, &ad_info)) {
- netdev_dbg(dev, "__bond_3ad_get_active_agg_info failed\n");
- goto err_free;
- }
-
- slaves_in_agg = ad_info.ports;
- agg_id = ad_info.aggregator_id;
-
- if (slaves_in_agg == 0) {
- netdev_dbg(dev, "active aggregator is empty\n");
- goto err_free;
- }
-
- slave_agg_no = bond_xmit_hash(bond, skb) % slaves_in_agg;
- first_ok_slave = NULL;
-
- bond_for_each_slave_rcu(bond, slave, iter) {
- agg = SLAVE_AD_INFO(slave)->port.aggregator;
- if (!agg || agg->aggregator_identifier != agg_id)
- continue;
-
- if (slave_agg_no >= 0) {
- if (!first_ok_slave && bond_slave_can_tx(slave))
- first_ok_slave = slave;
- slave_agg_no--;
- continue;
- }
-
- if (bond_slave_can_tx(slave)) {
- bond_dev_queue_xmit(bond, skb, slave->dev);
- goto out;
- }
- }
-
- if (slave_agg_no >= 0) {
- netdev_err(dev, "Couldn't find a slave to tx on for aggregator ID %d\n",
- agg_id);
- goto err_free;
- }
-
- /* we couldn't find any suitable slave after the agg_no, so use the
- * first suitable found, if found.
- */
- if (first_ok_slave)
- bond_dev_queue_xmit(bond, skb, first_ok_slave->dev);
- else
- goto err_free;
-
-out:
- return NETDEV_TX_OK;
-err_free:
- /* no suitable interface, frame not sent */
- dev_kfree_skb_any(skb);
- goto out;
-}
-
int bond_3ad_lacpdu_recv(const struct sk_buff *skb, struct bonding *bond,
struct slave *slave)
{
diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index 615f3bebd019..d2eadab787c5 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -177,7 +177,6 @@ static int tlb_initialize(struct bonding *bond)
static void tlb_deinitialize(struct bonding *bond)
{
struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond));
- struct tlb_up_slave *arr;
spin_lock_bh(&bond->mode_lock);
@@ -185,10 +184,6 @@ static void tlb_deinitialize(struct bonding *bond)
bond_info->tx_hashtbl = NULL;
spin_unlock_bh(&bond->mode_lock);
-
- arr = rtnl_dereference(bond_info->slave_arr);
- if (arr)
- kfree_rcu(arr, rcu);
}
static long long compute_gap(struct slave *slave)
@@ -1336,39 +1331,9 @@ out:
return NETDEV_TX_OK;
}
-static int bond_tlb_update_slave_arr(struct bonding *bond,
- struct slave *skipslave)
-{
- struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond));
- struct slave *tx_slave;
- struct list_head *iter;
- struct tlb_up_slave *new_arr, *old_arr;
-
- new_arr = kzalloc(offsetof(struct tlb_up_slave, arr[bond->slave_cnt]),
- GFP_ATOMIC);
- if (!new_arr)
- return -ENOMEM;
-
- bond_for_each_slave(bond, tx_slave, iter) {
- if (!bond_slave_can_tx(tx_slave))
- continue;
- if (skipslave == tx_slave)
- continue;
- new_arr->arr[new_arr->count++] = tx_slave;
- }
-
- old_arr = rtnl_dereference(bond_info->slave_arr);
- rcu_assign_pointer(bond_info->slave_arr, new_arr);
- if (old_arr)
- kfree_rcu(old_arr, rcu);
-
- return 0;
-}
-
int bond_tlb_xmit(struct sk_buff *skb, struct net_device *bond_dev)
{
struct bonding *bond = netdev_priv(bond_dev);
- struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond));
struct ethhdr *eth_data;
struct slave *tx_slave = NULL;
u32 hash_index;
@@ -1389,12 +1354,14 @@ int bond_tlb_xmit(struct sk_buff *skb, struct net_device *bond_dev)
hash_index & 0xFF,
skb->len);
} else {
- struct tlb_up_slave *slaves;
+ struct bond_up_slave *slaves;
+ unsigned int count;
- slaves = rcu_dereference(bond_info->slave_arr);
- if (slaves && slaves->count)
+ slaves = rcu_dereference(bond->slave_arr);
+ count = slaves ? ACCESS_ONCE(slaves->count) : 0;
+ if (likely(count))
tx_slave = slaves->arr[hash_index %
- slaves->count];
+ count];
}
break;
}
@@ -1641,10 +1608,6 @@ void bond_alb_deinit_slave(struct bonding *bond, struct slave *slave)
rlb_clear_slave(bond, slave);
}
- if (bond_is_nondyn_tlb(bond))
- if (bond_tlb_update_slave_arr(bond, slave))
- pr_err("Failed to build slave-array for TLB mode.\n");
-
}
void bond_alb_handle_link_change(struct bonding *bond, struct slave *slave, char link)
@@ -1669,7 +1632,7 @@ void bond_alb_handle_link_change(struct bonding *bond, struct slave *slave, char
}
if (bond_is_nondyn_tlb(bond)) {
- if (bond_tlb_update_slave_arr(bond, NULL))
+ if (bond_update_slave_arr(bond, NULL))
pr_err("Failed to build slave-array for TLB mode.\n");
}
}
diff --git a/drivers/net/bonding/bond_alb.h b/drivers/net/bonding/bond_alb.h
index 3c6a7ff974d7..1ad473b4ade5 100644
--- a/drivers/net/bonding/bond_alb.h
+++ b/drivers/net/bonding/bond_alb.h
@@ -139,19 +139,11 @@ struct tlb_slave_info {
*/
};
-struct tlb_up_slave {
- unsigned int count;
- struct rcu_head rcu;
- struct slave *arr[0];
-};
-
struct alb_bond_info {
struct tlb_client_info *tx_hashtbl; /* Dynamically allocated */
u32 unbalanced_load;
int tx_rebalance_counter;
int lp_counter;
- /* -------- non-dynamic tlb mode only ---------*/
- struct tlb_up_slave __rcu *slave_arr; /* Up slaves */
/* -------- rlb parameters -------- */
int rlb_enabled;
struct rlb_client_info *rx_hashtbl; /* Receive hash table */
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index c2adc2755ff6..fd4766861d6e 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -210,6 +210,7 @@ static int bond_init(struct net_device *bond_dev);
static void bond_uninit(struct net_device *bond_dev);
static struct rtnl_link_stats64 *bond_get_stats(struct net_device *bond_dev,
struct rtnl_link_stats64 *stats);
+static void bond_slave_arr_handler(struct work_struct *work);
/*---------------------------- General routines -----------------------------*/
@@ -1551,6 +1552,9 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
unblock_netpoll_tx();
}
+ if (bond_mode_uses_xmit_hash(bond))
+ bond_update_slave_arr(bond, NULL);
+
netdev_info(bond_dev, "Enslaving %s as %s interface with %s link\n",
slave_dev->name,
bond_is_active_slave(new_slave) ? "an active" : "a backup",
@@ -1668,6 +1672,9 @@ static int __bond_release_one(struct net_device *bond_dev,
if (BOND_MODE(bond) == BOND_MODE_8023AD)
bond_3ad_unbind_slave(slave);
+ if (bond_mode_uses_xmit_hash(bond))
+ bond_update_slave_arr(bond, slave);
+
netdev_info(bond_dev, "Releasing %s interface %s\n",
bond_is_active_slave(slave) ? "active" : "backup",
slave_dev->name);
@@ -1970,6 +1977,9 @@ static void bond_miimon_commit(struct bonding *bond)
bond_alb_handle_link_change(bond, slave,
BOND_LINK_UP);
+ if (BOND_MODE(bond) == BOND_MODE_XOR)
+ bond_update_slave_arr(bond, NULL);
+
if (!bond->curr_active_slave || slave == primary)
goto do_failover;
@@ -1997,6 +2007,9 @@ static void bond_miimon_commit(struct bonding *bond)
bond_alb_handle_link_change(bond, slave,
BOND_LINK_DOWN);
+ if (BOND_MODE(bond) == BOND_MODE_XOR)
+ bond_update_slave_arr(bond, NULL);
+
if (slave == rcu_access_pointer(bond->curr_active_slave))
goto do_failover;
@@ -2453,6 +2466,8 @@ static void bond_loadbalance_arp_mon(struct work_struct *work)
if (slave_state_changed) {
bond_slave_state_change(bond);
+ if (BOND_MODE(bond) == BOND_MODE_XOR)
+ bond_update_slave_arr(bond, NULL);
} else if (do_failover) {
block_netpoll_tx();
bond_select_active_slave(bond);
@@ -2829,8 +2844,20 @@ static int bond_slave_netdev_event(unsigned long event,
if (old_duplex != slave->duplex)
bond_3ad_adapter_duplex_changed(slave);
}
+ /* Refresh slave-array if applicable!
+ * If the setup does not use miimon or arpmon (mode-specific!),
+ * then these events will not cause the slave-array to be
+ * refreshed. This will cause xmit to use a slave that is not
+ * usable. Avoid such situation by refeshing the array at these
+ * events. If these (miimon/arpmon) parameters are configured
+ * then array gets refreshed twice and that should be fine!
+ */
+ if (bond_mode_uses_xmit_hash(bond))
+ bond_update_slave_arr(bond, NULL);
break;
case NETDEV_DOWN:
+ if (bond_mode_uses_xmit_hash(bond))
+ bond_update_slave_arr(bond, NULL);
break;
case NETDEV_CHANGEMTU:
/* TODO: Should slaves be allowed to
@@ -3010,6 +3037,7 @@ static void bond_work_init_all(struct bonding *bond)
else
INIT_DELAYED_WORK(&bond->arp_work, bond_loadbalance_arp_mon);
INIT_DELAYED_WORK(&bond->ad_work, bond_3ad_state_machine_handler);
+ INIT_DELAYED_WORK(&bond->slave_arr_work, bond_slave_arr_handler);
}
static void bond_work_cancel_all(struct bonding *bond)
@@ -3019,6 +3047,7 @@ static void bond_work_cancel_all(struct bonding *bond)
cancel_delayed_work_sync(&bond->alb_work);
cancel_delayed_work_sync(&bond->ad_work);
cancel_delayed_work_sync(&bond->mcast_work);
+ cancel_delayed_work_sync(&bond->slave_arr_work);
}
static int bond_open(struct net_device *bond_dev)
@@ -3068,6 +3097,9 @@ static int bond_open(struct net_device *bond_dev)
bond_3ad_initiate_agg_selection(bond, 1);
}
+ if (bond_mode_uses_xmit_hash(bond))
+ bond_update_slave_arr(bond, NULL);
+
return 0;
}
@@ -3573,20 +3605,141 @@ static int bond_xmit_activebackup(struct sk_buff *skb, struct net_device *bond_d
return NETDEV_TX_OK;
}
-/* In bond_xmit_xor() , we determine the output device by using a pre-
- * determined xmit_hash_policy(), If the selected device is not enabled,
- * find the next active slave.
+/* Use this to update slave_array when (a) it's not appropriate to update
+ * slave_array right away (note that update_slave_array() may sleep)
+ * and / or (b) RTNL is not held.
*/
-static int bond_xmit_xor(struct sk_buff *skb, struct net_device *bond_dev)
+void bond_slave_arr_work_rearm(struct bonding *bond, unsigned long delay)
{
- struct bonding *bond = netdev_priv(bond_dev);
- int slave_cnt = ACCESS_ONCE(bond->slave_cnt);
+ queue_delayed_work(bond->wq, &bond->slave_arr_work, delay);
+}
- if (likely(slave_cnt))
- bond_xmit_slave_id(bond, skb,
- bond_xmit_hash(bond, skb) % slave_cnt);
- else
+/* Slave array work handler. Holds only RTNL */
+static void bond_slave_arr_handler(struct work_struct *work)
+{
+ struct bonding *bond = container_of(work, struct bonding,
+ slave_arr_work.work);
+ int ret;
+
+ if (!rtnl_trylock())
+ goto err;
+
+ ret = bond_update_slave_arr(bond, NULL);
+ rtnl_unlock();
+ if (ret) {
+ pr_warn_ratelimited("Failed to update slave array from WT\n");
+ goto err;
+ }
+ return;
+
+err:
+ bond_slave_arr_work_rearm(bond, 1);
+}
+
+/* Build the usable slaves array in control path for modes that use xmit-hash
+ * to determine the slave interface -
+ * (a) BOND_MODE_8023AD
+ * (b) BOND_MODE_XOR
+ * (c) BOND_MODE_TLB && tlb_dynamic_lb == 0
+ *
+ * The caller is expected to hold RTNL only and NO other lock!
+ */
+int bond_update_slave_arr(struct bonding *bond, struct slave *skipslave)
+{
+ struct slave *slave;
+ struct list_head *iter;
+ struct bond_up_slave *new_arr, *old_arr;
+ int slaves_in_agg;
+ int agg_id = 0;
+ int ret = 0;
+
+#ifdef CONFIG_LOCKDEP
+ WARN_ON(lockdep_is_held(&bond->mode_lock));
+#endif
+
+ new_arr = kzalloc(offsetof(struct bond_up_slave, arr[bond->slave_cnt]),
+ GFP_KERNEL);
+ if (!new_arr) {
+ ret = -ENOMEM;
+ pr_err("Failed to build slave-array.\n");
+ goto out;
+ }
+ if (BOND_MODE(bond) == BOND_MODE_8023AD) {
+ struct ad_info ad_info;
+
+ if (bond_3ad_get_active_agg_info(bond, &ad_info)) {
+ pr_debug("bond_3ad_get_active_agg_info failed\n");
+ kfree_rcu(new_arr, rcu);
+ ret = -EINVAL;
+ goto out;
+ }
+ slaves_in_agg = ad_info.ports;
+ agg_id = ad_info.aggregator_id;
+ }
+ bond_for_each_slave(bond, slave, iter) {
+ if (BOND_MODE(bond) == BOND_MODE_8023AD) {
+ struct aggregator *agg;
+
+ agg = SLAVE_AD_INFO(slave)->port.aggregator;
+ if (!agg || agg->aggregator_identifier != agg_id)
+ continue;
+ }
+ if (!bond_slave_can_tx(slave))
+ continue;
+ if (skipslave == slave)
+ continue;
+ new_arr->arr[new_arr->count++] = slave;
+ }
+
+ old_arr = rtnl_dereference(bond->slave_arr);
+ rcu_assign_pointer(bond->slave_arr, new_arr);
+ if (old_arr)
+ kfree_rcu(old_arr, rcu);
+out:
+ if (ret != 0 && skipslave) {
+ int idx;
+
+ /* Rare situation where caller has asked to skip a specific
+ * slave but allocation failed (most likely!). BTW this is
+ * only possible when the call is initiated from
+ * __bond_release_one(). In this situation; overwrite the
+ * skipslave entry in the array with the last entry from the
+ * array to avoid a situation where the xmit path may choose
+ * this to-be-skipped slave to send a packet out.
+ */
+ old_arr = rtnl_dereference(bond->slave_arr);
+ for (idx = 0; idx < old_arr->count; idx++) {
+ if (skipslave == old_arr->arr[idx]) {
+ old_arr->arr[idx] =
+ old_arr->arr[old_arr->count-1];
+ old_arr->count--;
+ break;
+ }
+ }
+ }
+ return ret;
+}
+
+/* Use this Xmit function for 3AD as well as XOR modes. The current
+ * usable slave array is formed in the control path. The xmit function
+ * just calculates hash and sends the packet out.
+ */
+int bond_3ad_xor_xmit(struct sk_buff *skb, struct net_device *dev)
+{
+ struct bonding *bond = netdev_priv(dev);
+ struct slave *slave;
+ struct bond_up_slave *slaves;
+ unsigned int count;
+
+ slaves = rcu_dereference(bond->slave_arr);
+ count = slaves ? ACCESS_ONCE(slaves->count) : 0;
+ if (likely(count)) {
+ slave = slaves->arr[bond_xmit_hash(bond, skb) % count];
+ bond_dev_queue_xmit(bond, skb, slave->dev);
+ } else {
dev_kfree_skb_any(skb);
+ atomic_long_inc(&dev->tx_dropped);
+ }
return NETDEV_TX_OK;
}
@@ -3682,12 +3835,11 @@ static netdev_tx_t __bond_start_xmit(struct sk_buff *skb, struct net_device *dev
return bond_xmit_roundrobin(skb, dev);
case BOND_MODE_ACTIVEBACKUP:
return bond_xmit_activebackup(skb, dev);
+ case BOND_MODE_8023AD:
case BOND_MODE_XOR:
- return bond_xmit_xor(skb, dev);
+ return bond_3ad_xor_xmit(skb, dev);
case BOND_MODE_BROADCAST:
return bond_xmit_broadcast(skb, dev);
- case BOND_MODE_8023AD:
- return bond_3ad_xmit_xor(skb, dev);
case BOND_MODE_ALB:
return bond_alb_xmit(skb, dev);
case BOND_MODE_TLB:
@@ -3861,6 +4013,7 @@ static void bond_uninit(struct net_device *bond_dev)
struct bonding *bond = netdev_priv(bond_dev);
struct list_head *iter;
struct slave *slave;
+ struct bond_up_slave *arr;
bond_netpoll_cleanup(bond_dev);
@@ -3869,6 +4022,12 @@ static void bond_uninit(struct net_device *bond_dev)
__bond_release_one(bond_dev, slave->dev, true);
netdev_info(bond_dev, "Released all slaves\n");
+ arr = rtnl_dereference(bond->slave_arr);
+ if (arr) {
+ kfree_rcu(arr, rcu);
+ RCU_INIT_POINTER(bond->slave_arr, NULL);
+ }
+
list_del(&bond->bond_list);
bond_debug_unregister(bond);
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 5b022da9cad2..10920f0686e2 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -179,6 +179,12 @@ struct slave {
struct rtnl_link_stats64 slave_stats;
};
+struct bond_up_slave {
+ unsigned int count;
+ struct rcu_head rcu;
+ struct slave *arr[0];
+};
+
/*
* Link pseudo-state only used internally by monitors
*/
@@ -193,6 +199,7 @@ struct bonding {
struct slave __rcu *curr_active_slave;
struct slave __rcu *current_arp_slave;
struct slave __rcu *primary_slave;
+ struct bond_up_slave __rcu *slave_arr; /* Array of usable slaves */
bool force_primary;
s32 slave_cnt; /* never change this value outside the attach/detach wrappers */
int (*recv_probe)(const struct sk_buff *, struct bonding *,
@@ -222,6 +229,7 @@ struct bonding {
struct delayed_work alb_work;
struct delayed_work ad_work;
struct delayed_work mcast_work;
+ struct delayed_work slave_arr_work;
#ifdef CONFIG_DEBUG_FS
/* debugging support via debugfs */
struct dentry *debug_dir;
@@ -534,6 +542,8 @@ const char *bond_slave_link_status(s8 link);
struct bond_vlan_tag *bond_verify_device_path(struct net_device *start_dev,
struct net_device *end_dev,
int level);
+int bond_update_slave_arr(struct bonding *bond, struct slave *skipslave);
+void bond_slave_arr_work_rearm(struct bonding *bond, unsigned long delay);
#ifdef CONFIG_PROC_FS
void bond_create_proc_entry(struct bonding *bond);
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related
* [PATCH net-next v5 1/2] bonding: display xmit_hash_policy for non-dynamic-tlb mode
From: Mahesh Bandewar @ 2014-09-30 6:26 UTC (permalink / raw)
To: Jay Vosburgh, Veaceslav Falico, Andy Gospodarek, David Miller
Cc: netdev, Mahesh Bandewar, Eric Dumazet, Maciej Zenczykowski
It's a trivial fix to display xmit_hash_policy for this new TLB mode
since it uses transmit-hash-poilicy as part of bonding-master info
(/proc/net/bonding/<bonding-interface).
Signed-off-by: Mahesh Bandewar <maheshb@google.com>
Reviewed-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
v1
Rebase
v2
Added bond_mode_uses_xmit_hash() inline function
v3
Rebase
v4
Rebase
v5
Rebase
drivers/net/bonding/bond_procfs.c | 3 +--
drivers/net/bonding/bonding.h | 7 +++++++
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/net/bonding/bond_procfs.c b/drivers/net/bonding/bond_procfs.c
index bb09d0442aa8..a3948f8d1e53 100644
--- a/drivers/net/bonding/bond_procfs.c
+++ b/drivers/net/bonding/bond_procfs.c
@@ -73,8 +73,7 @@ static void bond_info_show_master(struct seq_file *seq)
seq_printf(seq, "\n");
- if (BOND_MODE(bond) == BOND_MODE_XOR ||
- BOND_MODE(bond) == BOND_MODE_8023AD) {
+ if (bond_mode_uses_xmit_hash(bond)) {
optval = bond_opt_get_val(BOND_OPT_XMIT_HASH,
bond->params.xmit_policy);
seq_printf(seq, "Transmit Hash Policy: %s (%d)\n",
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 57917e63b4e6..5b022da9cad2 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -274,6 +274,13 @@ static inline bool bond_is_nondyn_tlb(const struct bonding *bond)
(bond->params.tlb_dynamic_lb == 0);
}
+static inline bool bond_mode_uses_xmit_hash(const struct bonding *bond)
+{
+ return (BOND_MODE(bond) == BOND_MODE_8023AD ||
+ BOND_MODE(bond) == BOND_MODE_XOR ||
+ bond_is_nondyn_tlb(bond));
+}
+
static inline bool bond_mode_uses_arp(int mode)
{
return mode != BOND_MODE_8023AD && mode != BOND_MODE_TLB &&
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related
* [PATCH iproute2] tests: Check existing of /proc/config.gz before use it
From: Vadim Kochan @ 2014-09-30 6:15 UTC (permalink / raw)
To: netdev; +Cc: Vadim Kochan
Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
---
testsuite/Makefile | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/testsuite/Makefile b/testsuite/Makefile
index b4ab15e..d1bf359 100644
--- a/testsuite/Makefile
+++ b/testsuite/Makefile
@@ -6,7 +6,10 @@ RESULTS_DIR := results
TESTS := $(patsubst tests/%,%,$(wildcard tests/*.t))
IPVERS := $(filter-out iproute2/Makefile,$(wildcard iproute2/*))
-KENV := $(shell cat /proc/config.gz | gunzip | grep ^CONFIG)
+
+ifneq (,$(wildcard /proc/config.gz))
+ KENV := $(shell cat /proc/config.gz | gunzip | grep ^CONFIG)
+endif
.PHONY: compile listtests alltests configure $(TESTS)
--
2.1.0
^ permalink raw reply related
* [PATCH] net: ethernet : stmicro: fixed power suspend and resume failure in stmmac driver
From: hliang1025 @ 2014-09-30 5:55 UTC (permalink / raw)
To: peppe.cavallaro; +Cc: netdev, linux-kernel, adi-linux-docs, Hao Liang
From: Hao Liang <hliang1025@gmail.com>
This is the fix for a power management issue caused by suspend and resume function in stmmac_main.c.
After enable CONFIG_DEBUG_ATOMIC_SLEEP which enable sleep-inside atomic section checking, power
managemet can not work normally. Board couldn't wakeup successfully after suspend. Command
"echo mem > /sys/power/state" suspend the board.
In suspend and resume function of stmmac driver, there are some sleep-inside function in atomic section
created by spin lock. These functions will causes system warnings and wakeup issue when enable
CONFIG_DEBUG_ATOMIC_SLEEP.
This bug was fixed by:
* replace some sleep function with non-sleep function
clk_disable_unprepare -> clk_disable ...
* decrease the atomic area created by spin lock function. The original atomic area in resume function is
too large.
Signed-off-by: Hao Liang <hliang1025@gmail.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 6e6ee22..2effbfe 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2924,9 +2924,8 @@ int stmmac_suspend(struct net_device *ndev)
if (priv->phydev)
phy_stop(priv->phydev);
- spin_lock_irqsave(&priv->lock, flags);
-
netif_device_detach(ndev);
+
netif_stop_queue(ndev);
napi_disable(&priv->napi);
@@ -2935,6 +2934,8 @@ int stmmac_suspend(struct net_device *ndev)
priv->hw->dma->stop_tx(priv->ioaddr);
priv->hw->dma->stop_rx(priv->ioaddr);
+ spin_lock_irqsave(&priv->lock, flags);
+
stmmac_clear_descriptors(priv);
/* Enable Power down mode by programming the PMT regs */
@@ -2945,7 +2946,7 @@ int stmmac_suspend(struct net_device *ndev)
stmmac_set_mac(priv->ioaddr, false);
pinctrl_pm_select_sleep_state(priv->device);
/* Disable clock in case of PWM is off */
- clk_disable_unprepare(priv->stmmac_clk);
+ clk_disable(priv->stmmac_clk);
}
spin_unlock_irqrestore(&priv->lock, flags);
@@ -2977,12 +2978,14 @@ int stmmac_resume(struct net_device *ndev)
} else {
pinctrl_pm_select_default_state(priv->device);
/* enable the clk prevously disabled */
- clk_prepare_enable(priv->stmmac_clk);
+ clk_enable(priv->stmmac_clk);
/* reset the phy so that it's ready */
if (priv->mii)
stmmac_mdio_reset(priv->mii);
}
+ spin_unlock_irqrestore(&priv->lock, flags);
+
netif_device_attach(ndev);
stmmac_hw_setup(ndev);
@@ -2991,8 +2994,6 @@ int stmmac_resume(struct net_device *ndev)
netif_start_queue(ndev);
- spin_unlock_irqrestore(&priv->lock, flags);
-
if (priv->phydev)
phy_start(priv->phydev);
--
1.7.9.5
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox