* [PATCH net-next 11/22] sfc: Add AER and EEH support for Siena
From: Ben Hutchings @ 2013-03-11 19:55 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers, scrum-linux
In-Reply-To: <1363030400.2608.37.camel@bwh-desktop.uk.solarflarecom.com>
From: Alexandre Rames <arames@solarflare.com>
The Linux side of EEH is triggered by MMIO reads, but this
driver's data path does not issue any MMIO reads (except in
legacy interrupt mode). Therefore add a monitor function
to poll EEH periodically.
When preparing to reset the device based on our own error
detection, also poll EEH and defer to its recovery mechanism
if appropriate.
[bwh: Use a separate condition for the initial link poll; fix some
style errors]
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/ethernet/sfc/efx.c | 207 +++++++++++++++++++++++++++++----
drivers/net/ethernet/sfc/enum.h | 12 ++-
drivers/net/ethernet/sfc/net_driver.h | 1 +
drivers/net/ethernet/sfc/siena.c | 22 ++++-
4 files changed, 216 insertions(+), 26 deletions(-)
diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
index 11a8108..5e1ddc5 100644
--- a/drivers/net/ethernet/sfc/efx.c
+++ b/drivers/net/ethernet/sfc/efx.c
@@ -21,7 +21,9 @@
#include <linux/ethtool.h>
#include <linux/topology.h>
#include <linux/gfp.h>
+#include <linux/pci.h>
#include <linux/cpu_rmap.h>
+#include <linux/aer.h>
#include "net_driver.h"
#include "efx.h"
#include "nic.h"
@@ -71,17 +73,19 @@ const char *const efx_loopback_mode_names[] = {
const unsigned int efx_reset_type_max = RESET_TYPE_MAX;
const char *const efx_reset_type_names[] = {
- [RESET_TYPE_INVISIBLE] = "INVISIBLE",
- [RESET_TYPE_ALL] = "ALL",
- [RESET_TYPE_WORLD] = "WORLD",
- [RESET_TYPE_DISABLE] = "DISABLE",
- [RESET_TYPE_TX_WATCHDOG] = "TX_WATCHDOG",
- [RESET_TYPE_INT_ERROR] = "INT_ERROR",
- [RESET_TYPE_RX_RECOVERY] = "RX_RECOVERY",
- [RESET_TYPE_RX_DESC_FETCH] = "RX_DESC_FETCH",
- [RESET_TYPE_TX_DESC_FETCH] = "TX_DESC_FETCH",
- [RESET_TYPE_TX_SKIP] = "TX_SKIP",
- [RESET_TYPE_MC_FAILURE] = "MC_FAILURE",
+ [RESET_TYPE_INVISIBLE] = "INVISIBLE",
+ [RESET_TYPE_ALL] = "ALL",
+ [RESET_TYPE_RECOVER_OR_ALL] = "RECOVER_OR_ALL",
+ [RESET_TYPE_WORLD] = "WORLD",
+ [RESET_TYPE_RECOVER_OR_DISABLE] = "RECOVER_OR_DISABLE",
+ [RESET_TYPE_DISABLE] = "DISABLE",
+ [RESET_TYPE_TX_WATCHDOG] = "TX_WATCHDOG",
+ [RESET_TYPE_INT_ERROR] = "INT_ERROR",
+ [RESET_TYPE_RX_RECOVERY] = "RX_RECOVERY",
+ [RESET_TYPE_RX_DESC_FETCH] = "RX_DESC_FETCH",
+ [RESET_TYPE_TX_DESC_FETCH] = "TX_DESC_FETCH",
+ [RESET_TYPE_TX_SKIP] = "TX_SKIP",
+ [RESET_TYPE_MC_FAILURE] = "MC_FAILURE",
};
#define EFX_MAX_MTU (9 * 1024)
@@ -117,9 +121,12 @@ MODULE_PARM_DESC(separate_tx_channels,
static int napi_weight = 64;
/* This is the time (in jiffies) between invocations of the hardware
- * monitor. On Falcon-based NICs, this will:
+ * monitor.
+ * On Falcon-based NICs, this will:
* - Check the on-board hardware monitor;
* - Poll the link state and reconfigure the hardware as necessary.
+ * On Siena-based NICs for power systems with EEH support, this will give EEH a
+ * chance to start.
*/
static unsigned int efx_monitor_interval = 1 * HZ;
@@ -203,13 +210,14 @@ static void efx_stop_all(struct efx_nic *efx);
#define EFX_ASSERT_RESET_SERIALISED(efx) \
do { \
if ((efx->state == STATE_READY) || \
+ (efx->state == STATE_RECOVERY) || \
(efx->state == STATE_DISABLED)) \
ASSERT_RTNL(); \
} while (0)
static int efx_check_disabled(struct efx_nic *efx)
{
- if (efx->state == STATE_DISABLED) {
+ if (efx->state == STATE_DISABLED || efx->state == STATE_RECOVERY) {
netif_err(efx, drv, efx->net_dev,
"device is disabled due to earlier errors\n");
return -EIO;
@@ -677,7 +685,7 @@ static void efx_stop_datapath(struct efx_nic *efx)
BUG_ON(efx->port_enabled);
/* Only perform flush if dma is enabled */
- if (dev->is_busmaster) {
+ if (dev->is_busmaster && efx->state != STATE_RECOVERY) {
rc = efx_nic_flush_queues(efx);
if (rc && EFX_WORKAROUND_7803(efx)) {
@@ -1590,13 +1598,15 @@ static void efx_start_all(struct efx_nic *efx)
efx_start_port(efx);
efx_start_datapath(efx);
- /* Start the hardware monitor if there is one. Otherwise (we're link
- * event driven), we have to poll the PHY because after an event queue
- * flush, we could have a missed a link state change */
- if (efx->type->monitor != NULL) {
+ /* Start the hardware monitor if there is one */
+ if (efx->type->monitor != NULL)
queue_delayed_work(efx->workqueue, &efx->monitor_work,
efx_monitor_interval);
- } else {
+
+ /* If link state detection is normally event-driven, we have
+ * to poll now because we could have missed a change
+ */
+ if (efx_nic_rev(efx) >= EFX_REV_SIENA_A0) {
mutex_lock(&efx->mac_lock);
if (efx->phy_op->poll(efx))
efx_link_status_changed(efx);
@@ -2303,7 +2313,9 @@ int efx_reset(struct efx_nic *efx, enum reset_type method)
out:
/* Leave device stopped if necessary */
- disabled = rc || method == RESET_TYPE_DISABLE;
+ disabled = rc ||
+ method == RESET_TYPE_DISABLE ||
+ method == RESET_TYPE_RECOVER_OR_DISABLE;
rc2 = efx_reset_up(efx, method, !disabled);
if (rc2) {
disabled = true;
@@ -2322,13 +2334,48 @@ out:
return rc;
}
+/* Try recovery mechanisms.
+ * For now only EEH is supported.
+ * Returns 0 if the recovery mechanisms are unsuccessful.
+ * Returns a non-zero value otherwise.
+ */
+static int efx_try_recovery(struct efx_nic *efx)
+{
+#ifdef CONFIG_EEH
+ /* A PCI error can occur and not be seen by EEH because nothing
+ * happens on the PCI bus. In this case the driver may fail and
+ * schedule a 'recover or reset', leading to this recovery handler.
+ * Manually call the eeh failure check function.
+ */
+ struct eeh_dev *eehdev =
+ of_node_to_eeh_dev(pci_device_to_OF_node(efx->pci_dev));
+
+ if (eeh_dev_check_failure(eehdev)) {
+ /* The EEH mechanisms will handle the error and reset the
+ * device if necessary.
+ */
+ return 1;
+ }
+#endif
+ return 0;
+}
+
/* The worker thread exists so that code that cannot sleep can
* schedule a reset for later.
*/
static void efx_reset_work(struct work_struct *data)
{
struct efx_nic *efx = container_of(data, struct efx_nic, reset_work);
- unsigned long pending = ACCESS_ONCE(efx->reset_pending);
+ unsigned long pending;
+ enum reset_type method;
+
+ pending = ACCESS_ONCE(efx->reset_pending);
+ method = fls(pending) - 1;
+
+ if ((method == RESET_TYPE_RECOVER_OR_DISABLE ||
+ method == RESET_TYPE_RECOVER_OR_ALL) &&
+ efx_try_recovery(efx))
+ return;
if (!pending)
return;
@@ -2340,7 +2387,7 @@ static void efx_reset_work(struct work_struct *data)
* it cannot change again.
*/
if (efx->state == STATE_READY)
- (void)efx_reset(efx, fls(pending) - 1);
+ (void)efx_reset(efx, method);
rtnl_unlock();
}
@@ -2349,11 +2396,20 @@ void efx_schedule_reset(struct efx_nic *efx, enum reset_type type)
{
enum reset_type method;
+ if (efx->state == STATE_RECOVERY) {
+ netif_dbg(efx, drv, efx->net_dev,
+ "recovering: skip scheduling %s reset\n",
+ RESET_TYPE(type));
+ return;
+ }
+
switch (type) {
case RESET_TYPE_INVISIBLE:
case RESET_TYPE_ALL:
+ case RESET_TYPE_RECOVER_OR_ALL:
case RESET_TYPE_WORLD:
case RESET_TYPE_DISABLE:
+ case RESET_TYPE_RECOVER_OR_DISABLE:
method = type;
netif_dbg(efx, drv, efx->net_dev, "scheduling %s reset\n",
RESET_TYPE(method));
@@ -2563,6 +2619,8 @@ static void efx_pci_remove(struct pci_dev *pci_dev)
efx_fini_struct(efx);
pci_set_drvdata(pci_dev, NULL);
free_netdev(efx->net_dev);
+
+ pci_disable_pcie_error_reporting(pci_dev);
};
/* NIC VPD information
@@ -2735,6 +2793,11 @@ static int efx_pci_probe(struct pci_dev *pci_dev,
netif_warn(efx, probe, efx->net_dev,
"failed to create MTDs (%d)\n", rc);
+ rc = pci_enable_pcie_error_reporting(pci_dev);
+ if (rc && rc != -EINVAL)
+ netif_warn(efx, probe, efx->net_dev,
+ "pci_enable_pcie_error_reporting failed (%d)\n", rc);
+
return 0;
fail4:
@@ -2859,12 +2922,112 @@ static const struct dev_pm_ops efx_pm_ops = {
.restore = efx_pm_resume,
};
+/* A PCI error affecting this device was detected.
+ * At this point MMIO and DMA may be disabled.
+ * Stop the software path and request a slot reset.
+ */
+pci_ers_result_t efx_io_error_detected(struct pci_dev *pdev,
+ enum pci_channel_state state)
+{
+ pci_ers_result_t status = PCI_ERS_RESULT_RECOVERED;
+ struct efx_nic *efx = pci_get_drvdata(pdev);
+
+ if (state == pci_channel_io_perm_failure)
+ return PCI_ERS_RESULT_DISCONNECT;
+
+ rtnl_lock();
+
+ if (efx->state != STATE_DISABLED) {
+ efx->state = STATE_RECOVERY;
+ efx->reset_pending = 0;
+
+ efx_device_detach_sync(efx);
+
+ efx_stop_all(efx);
+ efx_stop_interrupts(efx, false);
+
+ status = PCI_ERS_RESULT_NEED_RESET;
+ } else {
+ /* If the interface is disabled we don't want to do anything
+ * with it.
+ */
+ status = PCI_ERS_RESULT_RECOVERED;
+ }
+
+ rtnl_unlock();
+
+ pci_disable_device(pdev);
+
+ return status;
+}
+
+/* Fake a successfull reset, which will be performed later in efx_io_resume. */
+pci_ers_result_t efx_io_slot_reset(struct pci_dev *pdev)
+{
+ struct efx_nic *efx = pci_get_drvdata(pdev);
+ pci_ers_result_t status = PCI_ERS_RESULT_RECOVERED;
+ int rc;
+
+ if (pci_enable_device(pdev)) {
+ netif_err(efx, hw, efx->net_dev,
+ "Cannot re-enable PCI device after reset.\n");
+ status = PCI_ERS_RESULT_DISCONNECT;
+ }
+
+ rc = pci_cleanup_aer_uncorrect_error_status(pdev);
+ if (rc) {
+ netif_err(efx, hw, efx->net_dev,
+ "pci_cleanup_aer_uncorrect_error_status failed (%d)\n", rc);
+ /* Non-fatal error. Continue. */
+ }
+
+ return status;
+}
+
+/* Perform the actual reset and resume I/O operations. */
+static void efx_io_resume(struct pci_dev *pdev)
+{
+ struct efx_nic *efx = pci_get_drvdata(pdev);
+ int rc;
+
+ rtnl_lock();
+
+ if (efx->state == STATE_DISABLED)
+ goto out;
+
+ rc = efx_reset(efx, RESET_TYPE_ALL);
+ if (rc) {
+ netif_err(efx, hw, efx->net_dev,
+ "efx_reset failed after PCI error (%d)\n", rc);
+ } else {
+ efx->state = STATE_READY;
+ netif_dbg(efx, hw, efx->net_dev,
+ "Done resetting and resuming IO after PCI error.\n");
+ }
+
+out:
+ rtnl_unlock();
+}
+
+/* For simplicity and reliability, we always require a slot reset and try to
+ * reset the hardware when a pci error affecting the device is detected.
+ * We leave both the link_reset and mmio_enabled callback unimplemented:
+ * with our request for slot reset the mmio_enabled callback will never be
+ * called, and the link_reset callback is not used by AER or EEH mechanisms.
+ */
+static struct pci_error_handlers efx_err_handlers = {
+ .error_detected = efx_io_error_detected,
+ .slot_reset = efx_io_slot_reset,
+ .resume = efx_io_resume,
+};
+
static struct pci_driver efx_pci_driver = {
.name = KBUILD_MODNAME,
.id_table = efx_pci_table,
.probe = efx_pci_probe,
.remove = efx_pci_remove,
.driver.pm = &efx_pm_ops,
+ .err_handler = &efx_err_handlers,
};
/**************************************************************************
diff --git a/drivers/net/ethernet/sfc/enum.h b/drivers/net/ethernet/sfc/enum.h
index 182dbe2..ab8fb58 100644
--- a/drivers/net/ethernet/sfc/enum.h
+++ b/drivers/net/ethernet/sfc/enum.h
@@ -137,8 +137,12 @@ enum efx_loopback_mode {
* Reset methods are numbered in order of increasing scope.
*
* @RESET_TYPE_INVISIBLE: Reset datapath and MAC (Falcon only)
+ * @RESET_TYPE_RECOVER_OR_ALL: Try to recover. Apply RESET_TYPE_ALL
+ * if unsuccessful.
* @RESET_TYPE_ALL: Reset datapath, MAC and PHY
* @RESET_TYPE_WORLD: Reset as much as possible
+ * @RESET_TYPE_RECOVER_OR_DISABLE: Try to recover. Apply RESET_TYPE_DISABLE if
+ * unsuccessful.
* @RESET_TYPE_DISABLE: Reset datapath, MAC and PHY; leave NIC disabled
* @RESET_TYPE_TX_WATCHDOG: reset due to TX watchdog
* @RESET_TYPE_INT_ERROR: reset due to internal error
@@ -150,9 +154,11 @@ enum efx_loopback_mode {
*/
enum reset_type {
RESET_TYPE_INVISIBLE = 0,
- RESET_TYPE_ALL = 1,
- RESET_TYPE_WORLD = 2,
- RESET_TYPE_DISABLE = 3,
+ RESET_TYPE_RECOVER_OR_ALL = 1,
+ RESET_TYPE_ALL = 2,
+ RESET_TYPE_WORLD = 3,
+ RESET_TYPE_RECOVER_OR_DISABLE = 4,
+ RESET_TYPE_DISABLE = 5,
RESET_TYPE_MAX_METHOD,
RESET_TYPE_TX_WATCHDOG,
RESET_TYPE_INT_ERROR,
diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
index c83fe09..9e90081 100644
--- a/drivers/net/ethernet/sfc/net_driver.h
+++ b/drivers/net/ethernet/sfc/net_driver.h
@@ -429,6 +429,7 @@ enum nic_state {
STATE_UNINIT = 0, /* device being probed/removed or is frozen */
STATE_READY = 1, /* hardware ready and netdev registered */
STATE_DISABLED = 2, /* device disabled due to hardware errors */
+ STATE_RECOVERY = 3, /* device recovering from PCI error */
};
/*
diff --git a/drivers/net/ethernet/sfc/siena.c b/drivers/net/ethernet/sfc/siena.c
index ba40f67..e07ff0d 100644
--- a/drivers/net/ethernet/sfc/siena.c
+++ b/drivers/net/ethernet/sfc/siena.c
@@ -202,7 +202,7 @@ out:
static enum reset_type siena_map_reset_reason(enum reset_type reason)
{
- return RESET_TYPE_ALL;
+ return RESET_TYPE_RECOVER_OR_ALL;
}
static int siena_map_reset_flags(u32 *flags)
@@ -245,6 +245,22 @@ static int siena_reset_hw(struct efx_nic *efx, enum reset_type method)
return efx_mcdi_reset_port(efx);
}
+#ifdef CONFIG_EEH
+/* When a PCI device is isolated from the bus, a subsequent MMIO read is
+ * required for the kernel EEH mechanisms to notice. As the Solarflare driver
+ * was written to minimise MMIO read (for latency) then a periodic call to check
+ * the EEH status of the device is required so that device recovery can happen
+ * in a timely fashion.
+ */
+static void siena_monitor(struct efx_nic *efx)
+{
+ struct eeh_dev *eehdev =
+ of_node_to_eeh_dev(pci_device_to_OF_node(efx->pci_dev));
+
+ eeh_dev_check_failure(eehdev);
+}
+#endif
+
static int siena_probe_nvconfig(struct efx_nic *efx)
{
u32 caps = 0;
@@ -665,7 +681,11 @@ const struct efx_nic_type siena_a0_nic_type = {
.init = siena_init_nic,
.dimension_resources = siena_dimension_resources,
.fini = efx_port_dummy_op_void,
+#ifdef CONFIG_EEH
+ .monitor = siena_monitor,
+#else
.monitor = NULL,
+#endif
.map_reset_reason = siena_map_reset_reason,
.map_reset_flags = siena_map_reset_flags,
.reset = siena_reset_hw,
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply related
* [PATCH net-next 10/22] sfc: Disable RSS when using SR-IOV and only 1 RX queue on the PF
From: Ben Hutchings @ 2013-03-11 19:55 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers, scrum-linux
In-Reply-To: <1363030400.2608.37.camel@bwh-desktop.uk.solarflarecom.com>
On Siena, VFs share RSS configuration with the PF. We attempted to
support configurations where the PF only uses 1 RX queue and VFs use
multiple RX queues, by (1) setting up RSS for the number of RX queues
per VF (2) disabling RSS in the PF's RX default filters.
Unfortunately commit cd2d5b529cdb ('sfc: Add SR-IOV back-end support
for SFC9000 family') only included (1). This is (2).
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/ethernet/sfc/filter.c | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/sfc/filter.c b/drivers/net/ethernet/sfc/filter.c
index 0de8daf..61b4408 100644
--- a/drivers/net/ethernet/sfc/filter.c
+++ b/drivers/net/ethernet/sfc/filter.c
@@ -414,8 +414,12 @@ static void efx_filter_reset_rx_def(struct efx_nic *efx, unsigned filter_idx)
struct efx_filter_table *table = &state->table[EFX_FILTER_TABLE_RX_DEF];
struct efx_filter_spec *spec = &table->spec[filter_idx];
+ /* If there's only one channel then disable RSS for non VF
+ * traffic, thereby allowing VFs to use RSS when the PF can't.
+ */
efx_filter_init_rx(spec, EFX_FILTER_PRI_MANUAL,
- EFX_FILTER_FLAG_RX_RSS, 0);
+ efx->n_rx_channels > 1 ? EFX_FILTER_FLAG_RX_RSS : 0,
+ 0);
spec->type = EFX_FILTER_UC_DEF + filter_idx;
table->used_bitmap[0] |= 1 << filter_idx;
}
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply related
* [PATCH net-next 09/22] sfc: Fix replacement detection in efx_filter_insert_filter()
From: Ben Hutchings @ 2013-03-11 19:55 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers, scrum-linux
In-Reply-To: <1363030400.2608.37.camel@bwh-desktop.uk.solarflarecom.com>
efx_filter_insert_filter() uses the first table entry in the hash chain
that either has the same match values or is empty. This means that
replacement doesn't always work correctly:
1. Insert filter F1 with match values M1, hashing to H1, at first
possible entry E1.
2. Insert filter F2 with match values M2, hashing to H1, at second
possible entry E2.
3. Remove filter F1.
4. Insert filter F3 with match values M2, hashing to H1, at first
possible entry E1.
F3 should have either replaced F2 or been rejected (depending on
priority and the replace_equal parameter).
Instead, search for both a matching filter that the inserted filter
would replace, and an available insertion point, up to the applicable
maximum search depths. If we insert at lower depth than a replaced
filter, clear the replaced filter.
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/ethernet/sfc/filter.c | 89 +++++++++++++++++++++++++++----------
1 files changed, 66 insertions(+), 23 deletions(-)
diff --git a/drivers/net/ethernet/sfc/filter.c b/drivers/net/ethernet/sfc/filter.c
index 769560a..0de8daf 100644
--- a/drivers/net/ethernet/sfc/filter.c
+++ b/drivers/net/ethernet/sfc/filter.c
@@ -66,6 +66,10 @@ struct efx_filter_state {
#endif
};
+static void efx_filter_table_clear_entry(struct efx_nic *efx,
+ struct efx_filter_table *table,
+ unsigned int filter_idx);
+
/* The filter hash function is LFSR polynomial x^16 + x^3 + 1 of a 32-bit
* key derived from the n-tuple. The initial LFSR state is 0xffff. */
static u16 efx_filter_hash(u32 key)
@@ -626,9 +630,9 @@ s32 efx_filter_insert_filter(struct efx_nic *efx, struct efx_filter_spec *spec,
{
struct efx_filter_state *state = efx->filter_state;
struct efx_filter_table *table = efx_filter_spec_table(state, spec);
- struct efx_filter_spec *saved_spec;
efx_oword_t filter;
- unsigned int filter_idx, depth = 0;
+ int rep_index, ins_index;
+ unsigned int depth = 0;
int rc;
if (!table || table->size == 0)
@@ -643,44 +647,74 @@ s32 efx_filter_insert_filter(struct efx_nic *efx, struct efx_filter_spec *spec,
BUILD_BUG_ON(EFX_FILTER_INDEX_UC_DEF != 0);
BUILD_BUG_ON(EFX_FILTER_INDEX_MC_DEF !=
EFX_FILTER_MC_DEF - EFX_FILTER_UC_DEF);
- filter_idx = spec->type - EFX_FILTER_INDEX_UC_DEF;
+ rep_index = spec->type - EFX_FILTER_INDEX_UC_DEF;
+ ins_index = rep_index;
spin_lock_bh(&state->lock);
} else {
+ /* Search concurrently for
+ * (1) a filter to be replaced (rep_index): any filter
+ * with the same match values, up to the current
+ * search depth for this type, and
+ * (2) the insertion point (ins_index): (1) or any
+ * free slot before it or up to the maximum search
+ * depth for this priority
+ * We fail if we cannot find (2).
+ *
+ * We can stop once either
+ * (a) we find (1), in which case we have definitely
+ * found (2) as well; or
+ * (b) we have searched exhaustively for (1), and have
+ * either found (2) or searched exhaustively for it
+ */
u32 key = efx_filter_build(&filter, spec);
unsigned int hash = efx_filter_hash(key);
unsigned int incr = efx_filter_increment(key);
- unsigned int depth_max =
+ unsigned int max_rep_depth = table->search_depth[spec->type];
+ unsigned int max_ins_depth =
spec->priority <= EFX_FILTER_PRI_HINT ?
FILTER_CTL_SRCH_HINT_MAX : FILTER_CTL_SRCH_MAX;
+ unsigned int i = hash & (table->size - 1);
- filter_idx = hash & (table->size - 1);
+ ins_index = -1;
depth = 1;
spin_lock_bh(&state->lock);
for (;;) {
- if (!test_bit(filter_idx, table->used_bitmap) ||
- efx_filter_equal(spec, &table->spec[filter_idx]))
+ if (!test_bit(i, table->used_bitmap)) {
+ if (ins_index < 0)
+ ins_index = i;
+ } else if (efx_filter_equal(spec, &table->spec[i])) {
+ /* Case (a) */
+ if (ins_index < 0)
+ ins_index = i;
+ rep_index = i;
break;
+ }
- /* Return failure if we reached the maximum search
- * depth
- */
- if (depth == depth_max) {
- rc = -EBUSY;
- goto out;
+ if (depth >= max_rep_depth &&
+ (ins_index >= 0 || depth >= max_ins_depth)) {
+ /* Case (b) */
+ if (ins_index < 0) {
+ rc = -EBUSY;
+ goto out;
+ }
+ rep_index = -1;
+ break;
}
- filter_idx = (filter_idx + incr) & (table->size - 1);
+ i = (i + incr) & (table->size - 1);
++depth;
}
}
- saved_spec = &table->spec[filter_idx];
+ /* If we found a filter to be replaced, check whether we
+ * should do so
+ */
+ if (rep_index >= 0) {
+ struct efx_filter_spec *saved_spec = &table->spec[rep_index];
- if (test_bit(filter_idx, table->used_bitmap)) {
- /* Should we replace the existing filter? */
if (spec->priority == saved_spec->priority && !replace_equal) {
rc = -EEXIST;
goto out;
@@ -689,11 +723,14 @@ s32 efx_filter_insert_filter(struct efx_nic *efx, struct efx_filter_spec *spec,
rc = -EPERM;
goto out;
}
- } else {
- __set_bit(filter_idx, table->used_bitmap);
+ }
+
+ /* Insert the filter */
+ if (ins_index != rep_index) {
+ __set_bit(ins_index, table->used_bitmap);
++table->used;
}
- *saved_spec = *spec;
+ table->spec[ins_index] = *spec;
if (table->id == EFX_FILTER_TABLE_RX_DEF) {
efx_filter_push_rx_config(efx);
@@ -707,13 +744,19 @@ s32 efx_filter_insert_filter(struct efx_nic *efx, struct efx_filter_spec *spec,
}
efx_writeo(efx, &filter,
- table->offset + table->step * filter_idx);
+ table->offset + table->step * ins_index);
+
+ /* If we were able to replace a filter by inserting
+ * at a lower depth, clear the replaced filter
+ */
+ if (ins_index != rep_index && rep_index >= 0)
+ efx_filter_table_clear_entry(efx, table, rep_index);
}
netif_vdbg(efx, hw, efx->net_dev,
"%s: filter type %d index %d rxq %u set",
- __func__, spec->type, filter_idx, spec->dmaq_id);
- rc = efx_filter_make_id(spec, filter_idx);
+ __func__, spec->type, ins_index, spec->dmaq_id);
+ rc = efx_filter_make_id(spec, ins_index);
out:
spin_unlock_bh(&state->lock);
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply related
* [PATCH net-next 08/22] sfc: Merge efx_filter_search() into efx_filter_insert()
From: Ben Hutchings @ 2013-03-11 19:55 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers, scrum-linux
In-Reply-To: <1363030400.2608.37.camel@bwh-desktop.uk.solarflarecom.com>
efx_filter_search() is only called from efx_filter_insert(), and
neither function is very long. The following bug fix requires a more
sophisticated search with a third result, which is going to be easier
to implement as part of the same function.
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/ethernet/sfc/filter.c | 60 +++++++++++++++----------------------
1 files changed, 24 insertions(+), 36 deletions(-)
diff --git a/drivers/net/ethernet/sfc/filter.c b/drivers/net/ethernet/sfc/filter.c
index 8d83d98..769560a 100644
--- a/drivers/net/ethernet/sfc/filter.c
+++ b/drivers/net/ethernet/sfc/filter.c
@@ -513,37 +513,6 @@ static bool efx_filter_equal(const struct efx_filter_spec *left,
return true;
}
-static int efx_filter_search(struct efx_filter_table *table,
- struct efx_filter_spec *spec, u32 key,
- unsigned int *depth_required)
-{
- unsigned hash, incr, filter_idx, depth, depth_max;
-
- hash = efx_filter_hash(key);
- incr = efx_filter_increment(key);
-
- filter_idx = hash & (table->size - 1);
- depth = 1;
- depth_max = (spec->priority <= EFX_FILTER_PRI_HINT ?
- FILTER_CTL_SRCH_HINT_MAX : FILTER_CTL_SRCH_MAX);
-
- for (;;) {
- /* Return success if entry is unused or matches this spec */
- if (!test_bit(filter_idx, table->used_bitmap) ||
- efx_filter_equal(spec, &table->spec[filter_idx])) {
- *depth_required = depth;
- return filter_idx;
- }
-
- /* Return failure if we reached the maximum search depth */
- if (depth == depth_max)
- return -EBUSY;
-
- filter_idx = (filter_idx + incr) & (table->size - 1);
- ++depth;
- }
-}
-
/*
* Construct/deconstruct external filter IDs. At least the RX filter
* IDs must be ordered by matching priority, for RX NFC semantics.
@@ -679,14 +648,33 @@ s32 efx_filter_insert_filter(struct efx_nic *efx, struct efx_filter_spec *spec,
spin_lock_bh(&state->lock);
} else {
u32 key = efx_filter_build(&filter, spec);
+ unsigned int hash = efx_filter_hash(key);
+ unsigned int incr = efx_filter_increment(key);
+ unsigned int depth_max =
+ spec->priority <= EFX_FILTER_PRI_HINT ?
+ FILTER_CTL_SRCH_HINT_MAX : FILTER_CTL_SRCH_MAX;
+
+ filter_idx = hash & (table->size - 1);
+ depth = 1;
spin_lock_bh(&state->lock);
- rc = efx_filter_search(table, spec, key, &depth);
- if (rc < 0)
- goto out;
- filter_idx = rc;
- BUG_ON(filter_idx >= table->size);
+ for (;;) {
+ if (!test_bit(filter_idx, table->used_bitmap) ||
+ efx_filter_equal(spec, &table->spec[filter_idx]))
+ break;
+
+ /* Return failure if we reached the maximum search
+ * depth
+ */
+ if (depth == depth_max) {
+ rc = -EBUSY;
+ goto out;
+ }
+
+ filter_idx = (filter_idx + incr) & (table->size - 1);
+ ++depth;
+ }
}
saved_spec = &table->spec[filter_idx];
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply related
* [PATCH net-next 07/22] sfc: Don't use efx_filter_{build,hash,increment}() for default MAC filters
From: Ben Hutchings @ 2013-03-11 19:54 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers, scrum-linux
In-Reply-To: <1363030400.2608.37.camel@bwh-desktop.uk.solarflarecom.com>
These functions happen to work for default MAC filters: they generate
an initial index of 1/0 for unicast/multicast respectively and an
increment of 1 for either, so a search succeeds at depth 2. But this
is a matter of luck rather than design, and it really won't work well
with the bug fix we're about to do.
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/ethernet/sfc/filter.c | 35 +++++++++++++++++++----------------
1 files changed, 19 insertions(+), 16 deletions(-)
diff --git a/drivers/net/ethernet/sfc/filter.c b/drivers/net/ethernet/sfc/filter.c
index 3d94ed7..8d83d98 100644
--- a/drivers/net/ethernet/sfc/filter.c
+++ b/drivers/net/ethernet/sfc/filter.c
@@ -463,13 +463,6 @@ static u32 efx_filter_build(efx_oword_t *filter, struct efx_filter_spec *spec)
break;
}
- case EFX_FILTER_TABLE_RX_DEF:
- /* One filter spec per type */
- BUILD_BUG_ON(EFX_FILTER_INDEX_UC_DEF != 0);
- BUILD_BUG_ON(EFX_FILTER_INDEX_MC_DEF !=
- EFX_FILTER_MC_DEF - EFX_FILTER_UC_DEF);
- return spec->type - EFX_FILTER_UC_DEF;
-
case EFX_FILTER_TABLE_RX_MAC: {
bool is_wild = spec->type == EFX_FILTER_MAC_WILD;
EFX_POPULATE_OWORD_7(
@@ -667,25 +660,35 @@ s32 efx_filter_insert_filter(struct efx_nic *efx, struct efx_filter_spec *spec,
struct efx_filter_spec *saved_spec;
efx_oword_t filter;
unsigned int filter_idx, depth = 0;
- u32 key;
int rc;
if (!table || table->size == 0)
return -EINVAL;
- key = efx_filter_build(&filter, spec);
-
netif_vdbg(efx, hw, efx->net_dev,
"%s: type %d search_depth=%d", __func__, spec->type,
table->search_depth[spec->type]);
- spin_lock_bh(&state->lock);
+ if (table->id == EFX_FILTER_TABLE_RX_DEF) {
+ /* One filter spec per type */
+ BUILD_BUG_ON(EFX_FILTER_INDEX_UC_DEF != 0);
+ BUILD_BUG_ON(EFX_FILTER_INDEX_MC_DEF !=
+ EFX_FILTER_MC_DEF - EFX_FILTER_UC_DEF);
+ filter_idx = spec->type - EFX_FILTER_INDEX_UC_DEF;
+
+ spin_lock_bh(&state->lock);
+ } else {
+ u32 key = efx_filter_build(&filter, spec);
+
+ spin_lock_bh(&state->lock);
+
+ rc = efx_filter_search(table, spec, key, &depth);
+ if (rc < 0)
+ goto out;
+ filter_idx = rc;
+ BUG_ON(filter_idx >= table->size);
+ }
- rc = efx_filter_search(table, spec, key, &depth);
- if (rc < 0)
- goto out;
- filter_idx = rc;
- BUG_ON(filter_idx >= table->size);
saved_spec = &table->spec[filter_idx];
if (test_bit(filter_idx, table->used_bitmap)) {
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply related
* [PATCH net-next 06/22] sfc: Remove redundant parameter to efx_filter_search()
From: Ben Hutchings @ 2013-03-11 19:54 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers, scrum-linux
In-Reply-To: <1363030400.2608.37.camel@bwh-desktop.uk.solarflarecom.com>
The 'for_insert' parameter is redundant since there are no longer
any other operations that need to search based on a filter spec.
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/ethernet/sfc/filter.c | 21 ++++++++-------------
1 files changed, 8 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/sfc/filter.c b/drivers/net/ethernet/sfc/filter.c
index 2fdd3a5..3d94ed7 100644
--- a/drivers/net/ethernet/sfc/filter.c
+++ b/drivers/net/ethernet/sfc/filter.c
@@ -522,7 +522,7 @@ static bool efx_filter_equal(const struct efx_filter_spec *left,
static int efx_filter_search(struct efx_filter_table *table,
struct efx_filter_spec *spec, u32 key,
- bool for_insert, unsigned int *depth_required)
+ unsigned int *depth_required)
{
unsigned hash, incr, filter_idx, depth, depth_max;
@@ -531,25 +531,20 @@ static int efx_filter_search(struct efx_filter_table *table,
filter_idx = hash & (table->size - 1);
depth = 1;
- depth_max = (for_insert ?
- (spec->priority <= EFX_FILTER_PRI_HINT ?
- FILTER_CTL_SRCH_HINT_MAX : FILTER_CTL_SRCH_MAX) :
- table->search_depth[spec->type]);
+ depth_max = (spec->priority <= EFX_FILTER_PRI_HINT ?
+ FILTER_CTL_SRCH_HINT_MAX : FILTER_CTL_SRCH_MAX);
for (;;) {
- /* Return success if entry is used and matches this spec
- * or entry is unused and we are trying to insert.
- */
- if (test_bit(filter_idx, table->used_bitmap) ?
- efx_filter_equal(spec, &table->spec[filter_idx]) :
- for_insert) {
+ /* Return success if entry is unused or matches this spec */
+ if (!test_bit(filter_idx, table->used_bitmap) ||
+ efx_filter_equal(spec, &table->spec[filter_idx])) {
*depth_required = depth;
return filter_idx;
}
/* Return failure if we reached the maximum search depth */
if (depth == depth_max)
- return for_insert ? -EBUSY : -ENOENT;
+ return -EBUSY;
filter_idx = (filter_idx + incr) & (table->size - 1);
++depth;
@@ -686,7 +681,7 @@ s32 efx_filter_insert_filter(struct efx_nic *efx, struct efx_filter_spec *spec,
spin_lock_bh(&state->lock);
- rc = efx_filter_search(table, spec, key, true, &depth);
+ rc = efx_filter_search(table, spec, key, &depth);
if (rc < 0)
goto out;
filter_idx = rc;
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply related
* [PATCH net-next 05/22] sfc: More sensible semantics for efx_filter_insert_filter() replace flag
From: Ben Hutchings @ 2013-03-11 19:54 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers, scrum-linux
In-Reply-To: <1363030400.2608.37.camel@bwh-desktop.uk.solarflarecom.com>
The 'replace' flag to efx_filter_insert_filter() controls whether the
new filter may replace *any* filter, and is checked even before
priority comparison. But lower-priority filters should never
block insertion of higher-priority filters.
Change the priority checking so that lower-priority filters are
replaced regardless of the value of the flag, and rename the
flag to 'replace_equal'.
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/ethernet/sfc/filter.c | 16 ++++++++++++----
1 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/sfc/filter.c b/drivers/net/ethernet/sfc/filter.c
index 8af42cd..2fdd3a5 100644
--- a/drivers/net/ethernet/sfc/filter.c
+++ b/drivers/net/ethernet/sfc/filter.c
@@ -650,14 +650,22 @@ u32 efx_filter_get_rx_id_limit(struct efx_nic *efx)
* efx_filter_insert_filter - add or replace a filter
* @efx: NIC in which to insert the filter
* @spec: Specification for the filter
- * @replace: Flag for whether the specified filter may replace a filter
- * with an identical match expression and equal or lower priority
+ * @replace_equal: Flag for whether the specified filter may replace an
+ * existing filter with equal priority
*
* On success, return the filter ID.
* On failure, return a negative error code.
+ *
+ * If an existing filter has equal match values to the new filter
+ * spec, then the new filter might replace it, depending on the
+ * relative priorities. If the existing filter has lower priority, or
+ * if @replace_equal is set and it has equal priority, then it is
+ * replaced. Otherwise the function fails, returning -%EPERM if
+ * the existing filter has higher priority or -%EEXIST if it has
+ * equal priority.
*/
s32 efx_filter_insert_filter(struct efx_nic *efx, struct efx_filter_spec *spec,
- bool replace)
+ bool replace_equal)
{
struct efx_filter_state *state = efx->filter_state;
struct efx_filter_table *table = efx_filter_spec_table(state, spec);
@@ -687,7 +695,7 @@ s32 efx_filter_insert_filter(struct efx_nic *efx, struct efx_filter_spec *spec,
if (test_bit(filter_idx, table->used_bitmap)) {
/* Should we replace the existing filter? */
- if (!replace) {
+ if (spec->priority == saved_spec->priority && !replace_equal) {
rc = -EEXIST;
goto out;
}
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply related
* [PATCH net-next 04/22] sfc: Remove rx_alloc_method SKB
From: Ben Hutchings @ 2013-03-11 19:54 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers, scrum-linux
In-Reply-To: <1363030400.2608.37.camel@bwh-desktop.uk.solarflarecom.com>
From: Alexandre Rames <arames@solarflare.com>
[bwh: Remove more dead code, and make efx_ptp_rx() pull the data it
needs into the header area.]
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/ethernet/sfc/efx.c | 8 +-
drivers/net/ethernet/sfc/efx.h | 1 -
drivers/net/ethernet/sfc/net_driver.h | 23 +--
drivers/net/ethernet/sfc/ptp.c | 4 +-
drivers/net/ethernet/sfc/rx.c | 330 ++++++++++-----------------------
5 files changed, 101 insertions(+), 265 deletions(-)
diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
index 0bc0099..11a8108 100644
--- a/drivers/net/ethernet/sfc/efx.c
+++ b/drivers/net/ethernet/sfc/efx.c
@@ -247,10 +247,8 @@ static int efx_process_channel(struct efx_channel *channel, int budget)
__efx_rx_packet(channel, channel->rx_pkt);
channel->rx_pkt = NULL;
}
- if (rx_queue->enabled) {
- efx_rx_strategy(channel);
+ if (rx_queue->enabled)
efx_fast_push_rx_descriptors(rx_queue);
- }
}
return spent;
@@ -655,16 +653,12 @@ static void efx_start_datapath(struct efx_nic *efx)
efx_for_each_channel_tx_queue(tx_queue, channel)
efx_init_tx_queue(tx_queue);
- /* The rx buffer allocation strategy is MTU dependent */
- efx_rx_strategy(channel);
-
efx_for_each_channel_rx_queue(rx_queue, channel) {
efx_init_rx_queue(rx_queue);
efx_nic_generate_fill_event(rx_queue);
}
WARN_ON(channel->rx_pkt != NULL);
- efx_rx_strategy(channel);
}
if (netif_device_present(efx->net_dev))
diff --git a/drivers/net/ethernet/sfc/efx.h b/drivers/net/ethernet/sfc/efx.h
index d2f790d..64c555e 100644
--- a/drivers/net/ethernet/sfc/efx.h
+++ b/drivers/net/ethernet/sfc/efx.h
@@ -37,7 +37,6 @@ extern int efx_probe_rx_queue(struct efx_rx_queue *rx_queue);
extern void efx_remove_rx_queue(struct efx_rx_queue *rx_queue);
extern void efx_init_rx_queue(struct efx_rx_queue *rx_queue);
extern void efx_fini_rx_queue(struct efx_rx_queue *rx_queue);
-extern void efx_rx_strategy(struct efx_channel *channel);
extern void efx_fast_push_rx_descriptors(struct efx_rx_queue *rx_queue);
extern void efx_rx_slow_fill(unsigned long context);
extern void __efx_rx_packet(struct efx_channel *channel,
diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
index cdcf510..c83fe09 100644
--- a/drivers/net/ethernet/sfc/net_driver.h
+++ b/drivers/net/ethernet/sfc/net_driver.h
@@ -206,25 +206,19 @@ struct efx_tx_queue {
/**
* struct efx_rx_buffer - An Efx RX data buffer
* @dma_addr: DMA base address of the buffer
- * @skb: The associated socket buffer. Valid iff !(@flags & %EFX_RX_BUF_PAGE).
+ * @page: The associated page buffer.
* Will be %NULL if the buffer slot is currently free.
- * @page: The associated page buffer. Valif iff @flags & %EFX_RX_BUF_PAGE.
- * Will be %NULL if the buffer slot is currently free.
- * @page_offset: Offset within page. Valid iff @flags & %EFX_RX_BUF_PAGE.
+ * @page_offset: Offset within page
* @len: Buffer length, in bytes.
* @flags: Flags for buffer and packet state.
*/
struct efx_rx_buffer {
dma_addr_t dma_addr;
- union {
- struct sk_buff *skb;
- struct page *page;
- } u;
+ struct page *page;
u16 page_offset;
u16 len;
u16 flags;
};
-#define EFX_RX_BUF_PAGE 0x0001
#define EFX_RX_PKT_CSUMMED 0x0002
#define EFX_RX_PKT_DISCARD 0x0004
@@ -266,8 +260,6 @@ struct efx_rx_page_state {
* @min_fill: RX descriptor minimum non-zero fill level.
* This records the minimum fill level observed when a ring
* refill was triggered.
- * @alloc_page_count: RX allocation strategy counter.
- * @alloc_skb_count: RX allocation strategy counter.
* @slow_fill: Timer used to defer efx_nic_generate_fill_event().
*/
struct efx_rx_queue {
@@ -286,8 +278,6 @@ struct efx_rx_queue {
unsigned int fast_fill_trigger;
unsigned int min_fill;
unsigned int min_overfill;
- unsigned int alloc_page_count;
- unsigned int alloc_skb_count;
struct timer_list slow_fill;
unsigned int slow_fill_count;
};
@@ -336,10 +326,6 @@ enum efx_rx_alloc_method {
* @event_test_cpu: Last CPU to handle interrupt or test event for this channel
* @irq_count: Number of IRQs since last adaptive moderation decision
* @irq_mod_score: IRQ moderation score
- * @rx_alloc_level: Watermark based heuristic counter for pushing descriptors
- * and diagnostic counters
- * @rx_alloc_push_pages: RX allocation method currently in use for pushing
- * descriptors
* @n_rx_tobe_disc: Count of RX_TOBE_DISC errors
* @n_rx_ip_hdr_chksum_err: Count of RX IP header checksum errors
* @n_rx_tcp_udp_chksum_err: Count of RX TCP and UDP checksum errors
@@ -371,9 +357,6 @@ struct efx_channel {
unsigned int rfs_filters_added;
#endif
- int rx_alloc_level;
- int rx_alloc_push_pages;
-
unsigned n_rx_tobe_disc;
unsigned n_rx_ip_hdr_chksum_err;
unsigned n_rx_tcp_udp_chksum_err;
diff --git a/drivers/net/ethernet/sfc/ptp.c b/drivers/net/ethernet/sfc/ptp.c
index d1858c0..07f6baa 100644
--- a/drivers/net/ethernet/sfc/ptp.c
+++ b/drivers/net/ethernet/sfc/ptp.c
@@ -1000,7 +1000,7 @@ static bool efx_ptp_rx(struct efx_channel *channel, struct sk_buff *skb)
/* Correct version? */
if (ptp->mode == MC_CMD_PTP_MODE_V1) {
- if (skb->len < PTP_V1_MIN_LENGTH) {
+ if (!pskb_may_pull(skb, PTP_V1_MIN_LENGTH)) {
return false;
}
version = ntohs(*(__be16 *)&skb->data[PTP_V1_VERSION_OFFSET]);
@@ -1014,7 +1014,7 @@ static bool efx_ptp_rx(struct efx_channel *channel, struct sk_buff *skb)
match_data_012 = skb->data + PTP_V1_UUID_OFFSET;
match_data_345 = skb->data + PTP_V1_UUID_OFFSET + 3;
} else {
- if (skb->len < PTP_V2_MIN_LENGTH) {
+ if (!pskb_may_pull(skb, PTP_V2_MIN_LENGTH)) {
return false;
}
version = skb->data[PTP_V2_VERSION_OFFSET];
diff --git a/drivers/net/ethernet/sfc/rx.c b/drivers/net/ethernet/sfc/rx.c
index f31c23e..e7aa28e 100644
--- a/drivers/net/ethernet/sfc/rx.c
+++ b/drivers/net/ethernet/sfc/rx.c
@@ -33,46 +33,6 @@
/* Size of buffer allocated for skb header area. */
#define EFX_SKB_HEADERS 64u
-/*
- * rx_alloc_method - RX buffer allocation method
- *
- * This driver supports two methods for allocating and using RX buffers:
- * each RX buffer may be backed by an skb or by an order-n page.
- *
- * When GRO is in use then the second method has a lower overhead,
- * since we don't have to allocate then free skbs on reassembled frames.
- *
- * Values:
- * - RX_ALLOC_METHOD_AUTO = 0
- * - RX_ALLOC_METHOD_SKB = 1
- * - RX_ALLOC_METHOD_PAGE = 2
- *
- * The heuristic for %RX_ALLOC_METHOD_AUTO is a simple hysteresis count
- * controlled by the parameters below.
- *
- * - Since pushing and popping descriptors are separated by the rx_queue
- * size, so the watermarks should be ~rxd_size.
- * - The performance win by using page-based allocation for GRO is less
- * than the performance hit of using page-based allocation of non-GRO,
- * so the watermarks should reflect this.
- *
- * Per channel we maintain a single variable, updated by each channel:
- *
- * rx_alloc_level += (gro_performed ? RX_ALLOC_FACTOR_GRO :
- * RX_ALLOC_FACTOR_SKB)
- * Per NAPI poll interval, we constrain rx_alloc_level to 0..MAX (which
- * limits the hysteresis), and update the allocation strategy:
- *
- * rx_alloc_method = (rx_alloc_level > RX_ALLOC_LEVEL_GRO ?
- * RX_ALLOC_METHOD_PAGE : RX_ALLOC_METHOD_SKB)
- */
-static int rx_alloc_method = RX_ALLOC_METHOD_AUTO;
-
-#define RX_ALLOC_LEVEL_GRO 0x2000
-#define RX_ALLOC_LEVEL_MAX 0x3000
-#define RX_ALLOC_FACTOR_GRO 1
-#define RX_ALLOC_FACTOR_SKB (-2)
-
/* This is the percentage fill level below which new RX descriptors
* will be added to the RX descriptor ring.
*/
@@ -99,10 +59,7 @@ static inline unsigned int efx_rx_buf_size(struct efx_nic *efx)
static u8 *efx_rx_buf_eh(struct efx_nic *efx, struct efx_rx_buffer *buf)
{
- if (buf->flags & EFX_RX_BUF_PAGE)
- return page_address(buf->u.page) + efx_rx_buf_offset(efx, buf);
- else
- return (u8 *)buf->u.skb->data + efx->type->rx_buffer_hash_size;
+ return page_address(buf->page) + efx_rx_buf_offset(efx, buf);
}
static inline u32 efx_rx_buf_hash(const u8 *eh)
@@ -120,56 +77,7 @@ static inline u32 efx_rx_buf_hash(const u8 *eh)
}
/**
- * efx_init_rx_buffers_skb - create EFX_RX_BATCH skb-based RX buffers
- *
- * @rx_queue: Efx RX queue
- *
- * This allocates EFX_RX_BATCH skbs, maps them for DMA, and populates a
- * struct efx_rx_buffer for each one. Return a negative error code or 0
- * on success. May fail having only inserted fewer than EFX_RX_BATCH
- * buffers.
- */
-static int efx_init_rx_buffers_skb(struct efx_rx_queue *rx_queue)
-{
- struct efx_nic *efx = rx_queue->efx;
- struct net_device *net_dev = efx->net_dev;
- struct efx_rx_buffer *rx_buf;
- struct sk_buff *skb;
- int skb_len = efx->rx_buffer_len;
- unsigned index, count;
-
- for (count = 0; count < EFX_RX_BATCH; ++count) {
- index = rx_queue->added_count & rx_queue->ptr_mask;
- rx_buf = efx_rx_buffer(rx_queue, index);
-
- rx_buf->u.skb = skb = netdev_alloc_skb(net_dev, skb_len);
- if (unlikely(!skb))
- return -ENOMEM;
-
- /* Adjust the SKB for padding */
- skb_reserve(skb, NET_IP_ALIGN);
- rx_buf->len = skb_len - NET_IP_ALIGN;
- rx_buf->flags = 0;
-
- rx_buf->dma_addr = dma_map_single(&efx->pci_dev->dev,
- skb->data, rx_buf->len,
- DMA_FROM_DEVICE);
- if (unlikely(dma_mapping_error(&efx->pci_dev->dev,
- rx_buf->dma_addr))) {
- dev_kfree_skb_any(skb);
- rx_buf->u.skb = NULL;
- return -EIO;
- }
-
- ++rx_queue->added_count;
- ++rx_queue->alloc_skb_count;
- }
-
- return 0;
-}
-
-/**
- * efx_init_rx_buffers_page - create EFX_RX_BATCH page-based RX buffers
+ * efx_init_rx_buffers - create EFX_RX_BATCH page-based RX buffers
*
* @rx_queue: Efx RX queue
*
@@ -178,7 +86,7 @@ static int efx_init_rx_buffers_skb(struct efx_rx_queue *rx_queue)
* code or 0 on success. If a single page can be split between two buffers,
* then the page will either be inserted fully, or not at at all.
*/
-static int efx_init_rx_buffers_page(struct efx_rx_queue *rx_queue)
+static int efx_init_rx_buffers(struct efx_rx_queue *rx_queue)
{
struct efx_nic *efx = rx_queue->efx;
struct efx_rx_buffer *rx_buf;
@@ -214,12 +122,11 @@ static int efx_init_rx_buffers_page(struct efx_rx_queue *rx_queue)
index = rx_queue->added_count & rx_queue->ptr_mask;
rx_buf = efx_rx_buffer(rx_queue, index);
rx_buf->dma_addr = dma_addr + EFX_PAGE_IP_ALIGN;
- rx_buf->u.page = page;
+ rx_buf->page = page;
rx_buf->page_offset = page_offset + EFX_PAGE_IP_ALIGN;
rx_buf->len = efx->rx_buffer_len - EFX_PAGE_IP_ALIGN;
- rx_buf->flags = EFX_RX_BUF_PAGE;
+ rx_buf->flags = 0;
++rx_queue->added_count;
- ++rx_queue->alloc_page_count;
++state->refcnt;
if ((~count & 1) && (efx->rx_buffer_len <= EFX_RX_HALF_PAGE)) {
@@ -239,10 +146,10 @@ static void efx_unmap_rx_buffer(struct efx_nic *efx,
struct efx_rx_buffer *rx_buf,
unsigned int used_len)
{
- if ((rx_buf->flags & EFX_RX_BUF_PAGE) && rx_buf->u.page) {
+ if (rx_buf->page) {
struct efx_rx_page_state *state;
- state = page_address(rx_buf->u.page);
+ state = page_address(rx_buf->page);
if (--state->refcnt == 0) {
dma_unmap_page(&efx->pci_dev->dev,
state->dma_addr,
@@ -253,21 +160,15 @@ static void efx_unmap_rx_buffer(struct efx_nic *efx,
rx_buf->dma_addr, used_len,
DMA_FROM_DEVICE);
}
- } else if (!(rx_buf->flags & EFX_RX_BUF_PAGE) && rx_buf->u.skb) {
- dma_unmap_single(&efx->pci_dev->dev, rx_buf->dma_addr,
- rx_buf->len, DMA_FROM_DEVICE);
}
}
static void efx_free_rx_buffer(struct efx_nic *efx,
struct efx_rx_buffer *rx_buf)
{
- if ((rx_buf->flags & EFX_RX_BUF_PAGE) && rx_buf->u.page) {
- __free_pages(rx_buf->u.page, efx->rx_buffer_order);
- rx_buf->u.page = NULL;
- } else if (!(rx_buf->flags & EFX_RX_BUF_PAGE) && rx_buf->u.skb) {
- dev_kfree_skb_any(rx_buf->u.skb);
- rx_buf->u.skb = NULL;
+ if (rx_buf->page) {
+ __free_pages(rx_buf->page, efx->rx_buffer_order);
+ rx_buf->page = NULL;
}
}
@@ -283,7 +184,7 @@ static void efx_fini_rx_buffer(struct efx_rx_queue *rx_queue,
static void efx_resurrect_rx_buffer(struct efx_rx_queue *rx_queue,
struct efx_rx_buffer *rx_buf)
{
- struct efx_rx_page_state *state = page_address(rx_buf->u.page);
+ struct efx_rx_page_state *state = page_address(rx_buf->page);
struct efx_rx_buffer *new_buf;
unsigned fill_level, index;
@@ -298,14 +199,13 @@ static void efx_resurrect_rx_buffer(struct efx_rx_queue *rx_queue,
}
++state->refcnt;
- get_page(rx_buf->u.page);
+ get_page(rx_buf->page);
index = rx_queue->added_count & rx_queue->ptr_mask;
new_buf = efx_rx_buffer(rx_queue, index);
new_buf->dma_addr = rx_buf->dma_addr ^ (PAGE_SIZE >> 1);
- new_buf->u.page = rx_buf->u.page;
+ new_buf->page = rx_buf->page;
new_buf->len = rx_buf->len;
- new_buf->flags = EFX_RX_BUF_PAGE;
++rx_queue->added_count;
}
@@ -319,18 +219,17 @@ static void efx_recycle_rx_buffer(struct efx_channel *channel,
struct efx_rx_buffer *new_buf;
unsigned index;
- rx_buf->flags &= EFX_RX_BUF_PAGE;
+ rx_buf->flags = 0;
- if ((rx_buf->flags & EFX_RX_BUF_PAGE) &&
- efx->rx_buffer_len <= EFX_RX_HALF_PAGE &&
- page_count(rx_buf->u.page) == 1)
+ if (efx->rx_buffer_len <= EFX_RX_HALF_PAGE &&
+ page_count(rx_buf->page) == 1)
efx_resurrect_rx_buffer(rx_queue, rx_buf);
index = rx_queue->added_count & rx_queue->ptr_mask;
new_buf = efx_rx_buffer(rx_queue, index);
memcpy(new_buf, rx_buf, sizeof(*new_buf));
- rx_buf->u.page = NULL;
+ rx_buf->page = NULL;
++rx_queue->added_count;
}
@@ -348,7 +247,6 @@ static void efx_recycle_rx_buffer(struct efx_channel *channel,
*/
void efx_fast_push_rx_descriptors(struct efx_rx_queue *rx_queue)
{
- struct efx_channel *channel = efx_rx_queue_channel(rx_queue);
unsigned fill_level;
int space, rc = 0;
@@ -369,16 +267,13 @@ void efx_fast_push_rx_descriptors(struct efx_rx_queue *rx_queue)
netif_vdbg(rx_queue->efx, rx_status, rx_queue->efx->net_dev,
"RX queue %d fast-filling descriptor ring from"
- " level %d to level %d using %s allocation\n",
+ " level %d to level %d\n",
efx_rx_queue_index(rx_queue), fill_level,
- rx_queue->max_fill,
- channel->rx_alloc_push_pages ? "page" : "skb");
+ rx_queue->max_fill);
+
do {
- if (channel->rx_alloc_push_pages)
- rc = efx_init_rx_buffers_page(rx_queue);
- else
- rc = efx_init_rx_buffers_skb(rx_queue);
+ rc = efx_init_rx_buffers(rx_queue);
if (unlikely(rc)) {
/* Ensure that we don't leave the rx queue empty */
if (rx_queue->added_count == rx_queue->removed_count)
@@ -408,7 +303,7 @@ void efx_rx_slow_fill(unsigned long context)
static void efx_rx_packet__check_len(struct efx_rx_queue *rx_queue,
struct efx_rx_buffer *rx_buf,
- int len, bool *leak_packet)
+ int len)
{
struct efx_nic *efx = rx_queue->efx;
unsigned max_len = rx_buf->len - efx->type->rx_buffer_padding;
@@ -428,11 +323,6 @@ static void efx_rx_packet__check_len(struct efx_rx_queue *rx_queue,
"RX event (0x%x > 0x%x+0x%x). Leaking\n",
efx_rx_queue_index(rx_queue), len, max_len,
efx->type->rx_buffer_padding);
- /* If this buffer was skb-allocated, then the meta
- * data at the end of the skb will be trashed. So
- * we have no choice but to leak the fragment.
- */
- *leak_packet = !(rx_buf->flags & EFX_RX_BUF_PAGE);
efx_schedule_reset(efx, RESET_TYPE_RX_RECOVERY);
} else {
if (net_ratelimit())
@@ -454,51 +344,78 @@ static void efx_rx_packet_gro(struct efx_channel *channel,
{
struct napi_struct *napi = &channel->napi_str;
gro_result_t gro_result;
+ struct efx_nic *efx = channel->efx;
+ struct page *page = rx_buf->page;
+ struct sk_buff *skb;
- if (rx_buf->flags & EFX_RX_BUF_PAGE) {
- struct efx_nic *efx = channel->efx;
- struct page *page = rx_buf->u.page;
- struct sk_buff *skb;
-
- rx_buf->u.page = NULL;
+ rx_buf->page = NULL;
- skb = napi_get_frags(napi);
- if (!skb) {
- put_page(page);
- return;
- }
+ skb = napi_get_frags(napi);
+ if (!skb) {
+ put_page(page);
+ return;
+ }
- if (efx->net_dev->features & NETIF_F_RXHASH)
- skb->rxhash = efx_rx_buf_hash(eh);
+ if (efx->net_dev->features & NETIF_F_RXHASH)
+ skb->rxhash = efx_rx_buf_hash(eh);
- skb_fill_page_desc(skb, 0, page,
- efx_rx_buf_offset(efx, rx_buf), rx_buf->len);
+ skb_fill_page_desc(skb, 0, page,
+ efx_rx_buf_offset(efx, rx_buf), rx_buf->len);
- skb->len = rx_buf->len;
- skb->data_len = rx_buf->len;
- skb->truesize += rx_buf->len;
- skb->ip_summed = ((rx_buf->flags & EFX_RX_PKT_CSUMMED) ?
- CHECKSUM_UNNECESSARY : CHECKSUM_NONE);
+ skb->len = rx_buf->len;
+ skb->data_len = rx_buf->len;
+ skb->truesize += rx_buf->len;
+ skb->ip_summed = ((rx_buf->flags & EFX_RX_PKT_CSUMMED) ?
+ CHECKSUM_UNNECESSARY : CHECKSUM_NONE);
- skb_record_rx_queue(skb, channel->rx_queue.core_index);
+ skb_record_rx_queue(skb, channel->rx_queue.core_index);
gro_result = napi_gro_frags(napi);
- } else {
- struct sk_buff *skb = rx_buf->u.skb;
- EFX_BUG_ON_PARANOID(!(rx_buf->flags & EFX_RX_PKT_CSUMMED));
- rx_buf->u.skb = NULL;
- skb->ip_summed = CHECKSUM_UNNECESSARY;
+ if (gro_result != GRO_DROP)
+ channel->irq_mod_score += 2;
+}
- gro_result = napi_gro_receive(napi, skb);
- }
+/* Allocate and construct an SKB around a struct page.*/
+static struct sk_buff *efx_rx_mk_skb(struct efx_channel *channel,
+ struct efx_rx_buffer *rx_buf,
+ u8 *eh, int hdr_len)
+{
+ struct efx_nic *efx = channel->efx;
+ struct sk_buff *skb;
- if (gro_result == GRO_NORMAL) {
- channel->rx_alloc_level += RX_ALLOC_FACTOR_SKB;
- } else if (gro_result != GRO_DROP) {
- channel->rx_alloc_level += RX_ALLOC_FACTOR_GRO;
- channel->irq_mod_score += 2;
+ /* Allocate an SKB to store the headers */
+ skb = netdev_alloc_skb(efx->net_dev, hdr_len + EFX_PAGE_SKB_ALIGN);
+ if (unlikely(skb == NULL))
+ return NULL;
+
+ EFX_BUG_ON_PARANOID(rx_buf->len < hdr_len);
+
+ skb_reserve(skb, EFX_PAGE_SKB_ALIGN);
+
+ skb->len = rx_buf->len;
+ skb->truesize = rx_buf->len + sizeof(struct sk_buff);
+ memcpy(skb->data, eh, hdr_len);
+ skb->tail += hdr_len;
+
+ /* Append the remaining page onto the frag list */
+ if (rx_buf->len > hdr_len) {
+ skb->data_len = skb->len - hdr_len;
+ skb_fill_page_desc(skb, 0, rx_buf->page,
+ efx_rx_buf_offset(efx, rx_buf) + hdr_len,
+ skb->data_len);
+ } else {
+ __free_pages(rx_buf->page, efx->rx_buffer_order);
+ skb->data_len = 0;
}
+
+ /* Ownership has transferred from the rx_buf to skb */
+ rx_buf->page = NULL;
+
+ /* Move past the ethernet header */
+ skb->protocol = eth_type_trans(skb, efx->net_dev);
+
+ return skb;
}
void efx_rx_packet(struct efx_rx_queue *rx_queue, unsigned int index,
@@ -507,7 +424,6 @@ void efx_rx_packet(struct efx_rx_queue *rx_queue, unsigned int index,
struct efx_nic *efx = rx_queue->efx;
struct efx_channel *channel = efx_rx_queue_channel(rx_queue);
struct efx_rx_buffer *rx_buf;
- bool leak_packet = false;
rx_buf = efx_rx_buffer(rx_queue, index);
rx_buf->flags |= flags;
@@ -519,7 +435,7 @@ void efx_rx_packet(struct efx_rx_queue *rx_queue, unsigned int index,
rx_queue->removed_count++;
/* Validate the length encoded in the event vs the descriptor pushed */
- efx_rx_packet__check_len(rx_queue, rx_buf, len, &leak_packet);
+ efx_rx_packet__check_len(rx_queue, rx_buf, len);
netif_vdbg(efx, rx_status, efx->net_dev,
"RX queue %d received id %x at %llx+%x %s%s\n",
@@ -530,10 +446,7 @@ void efx_rx_packet(struct efx_rx_queue *rx_queue, unsigned int index,
/* Discard packet, if instructed to do so */
if (unlikely(rx_buf->flags & EFX_RX_PKT_DISCARD)) {
- if (unlikely(leak_packet))
- channel->n_skbuff_leaks++;
- else
- efx_recycle_rx_buffer(channel, rx_buf);
+ efx_recycle_rx_buffer(channel, rx_buf);
/* Don't hold off the previous receive */
rx_buf = NULL;
@@ -560,31 +473,28 @@ out:
channel->rx_pkt = rx_buf;
}
-static void efx_rx_deliver(struct efx_channel *channel,
+static void efx_rx_deliver(struct efx_channel *channel, u8 *eh,
struct efx_rx_buffer *rx_buf)
{
struct sk_buff *skb;
+ u16 hdr_len = min_t(u16, rx_buf->len, EFX_SKB_HEADERS);
- /* We now own the SKB */
- skb = rx_buf->u.skb;
- rx_buf->u.skb = NULL;
+ skb = efx_rx_mk_skb(channel, rx_buf, eh, hdr_len);
+ if (unlikely(skb == NULL)) {
+ efx_free_rx_buffer(channel->efx, rx_buf);
+ return;
+ }
+ skb_record_rx_queue(skb, channel->rx_queue.core_index);
/* Set the SKB flags */
skb_checksum_none_assert(skb);
- /* Record the rx_queue */
- skb_record_rx_queue(skb, channel->rx_queue.core_index);
-
if (channel->type->receive_skb)
if (channel->type->receive_skb(channel, skb))
- goto handled;
+ return;
/* Pass the packet up */
netif_receive_skb(skb);
-
-handled:
- /* Update allocation strategy method */
- channel->rx_alloc_level += RX_ALLOC_FACTOR_SKB;
}
/* Handle a received packet. Second half: Touches packet payload. */
@@ -602,60 +512,13 @@ void __efx_rx_packet(struct efx_channel *channel, struct efx_rx_buffer *rx_buf)
return;
}
- if (!(rx_buf->flags & EFX_RX_BUF_PAGE)) {
- struct sk_buff *skb = rx_buf->u.skb;
-
- prefetch(skb_shinfo(skb));
-
- skb_reserve(skb, efx->type->rx_buffer_hash_size);
- skb_put(skb, rx_buf->len);
-
- if (efx->net_dev->features & NETIF_F_RXHASH)
- skb->rxhash = efx_rx_buf_hash(eh);
-
- /* Move past the ethernet header. rx_buf->data still points
- * at the ethernet header */
- skb->protocol = eth_type_trans(skb, efx->net_dev);
-
- skb_record_rx_queue(skb, channel->rx_queue.core_index);
- }
-
if (unlikely(!(efx->net_dev->features & NETIF_F_RXCSUM)))
rx_buf->flags &= ~EFX_RX_PKT_CSUMMED;
- if (likely(rx_buf->flags & (EFX_RX_BUF_PAGE | EFX_RX_PKT_CSUMMED)) &&
- !channel->type->receive_skb)
+ if (!channel->type->receive_skb)
efx_rx_packet_gro(channel, rx_buf, eh);
else
- efx_rx_deliver(channel, rx_buf);
-}
-
-void efx_rx_strategy(struct efx_channel *channel)
-{
- enum efx_rx_alloc_method method = rx_alloc_method;
-
- if (channel->type->receive_skb) {
- channel->rx_alloc_push_pages = false;
- return;
- }
-
- /* Only makes sense to use page based allocation if GRO is enabled */
- if (!(channel->efx->net_dev->features & NETIF_F_GRO)) {
- method = RX_ALLOC_METHOD_SKB;
- } else if (method == RX_ALLOC_METHOD_AUTO) {
- /* Constrain the rx_alloc_level */
- if (channel->rx_alloc_level < 0)
- channel->rx_alloc_level = 0;
- else if (channel->rx_alloc_level > RX_ALLOC_LEVEL_MAX)
- channel->rx_alloc_level = RX_ALLOC_LEVEL_MAX;
-
- /* Decide on the allocation method */
- method = ((channel->rx_alloc_level > RX_ALLOC_LEVEL_GRO) ?
- RX_ALLOC_METHOD_PAGE : RX_ALLOC_METHOD_SKB);
- }
-
- /* Push the option */
- channel->rx_alloc_push_pages = (method == RX_ALLOC_METHOD_PAGE);
+ efx_rx_deliver(channel, eh, rx_buf);
}
int efx_probe_rx_queue(struct efx_rx_queue *rx_queue)
@@ -756,9 +619,6 @@ void efx_remove_rx_queue(struct efx_rx_queue *rx_queue)
}
-module_param(rx_alloc_method, int, 0644);
-MODULE_PARM_DESC(rx_alloc_method, "Allocation method used for RX buffers");
-
module_param(rx_refill_threshold, uint, 0444);
MODULE_PARM_DESC(rx_refill_threshold,
"RX descriptor ring refill threshold (%)");
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply related
* [PATCH net-next 03/22] sfc: tidy up PTP synchronize function efx_ptp_process_times()
From: Ben Hutchings @ 2013-03-11 19:53 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers, scrum-linux, Laurence Evans
In-Reply-To: <1363030400.2608.37.camel@bwh-desktop.uk.solarflarecom.com>
From: Laurence Evans <levans@solarflare.com>
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/ethernet/sfc/ptp.c | 35 +++++++----------------------------
1 files changed, 7 insertions(+), 28 deletions(-)
diff --git a/drivers/net/ethernet/sfc/ptp.c b/drivers/net/ethernet/sfc/ptp.c
index 2b40cbd..d1858c0 100644
--- a/drivers/net/ethernet/sfc/ptp.c
+++ b/drivers/net/ethernet/sfc/ptp.c
@@ -432,13 +432,10 @@ static int efx_ptp_process_times(struct efx_nic *efx, u8 *synch_buf,
unsigned number_readings = (response_length /
MC_CMD_PTP_OUT_SYNCHRONIZE_TIMESET_LEN);
unsigned i;
- unsigned min;
- unsigned min_set = 0;
unsigned total;
unsigned ngood = 0;
unsigned last_good = 0;
struct efx_ptp_data *ptp = efx->ptp_data;
- bool min_valid = false;
u32 last_sec;
u32 start_sec;
struct timespec delta;
@@ -446,35 +443,17 @@ static int efx_ptp_process_times(struct efx_nic *efx, u8 *synch_buf,
if (number_readings == 0)
return -EAGAIN;
- /* Find minimum value in this set of results, discarding clearly
- * erroneous results.
+ /* Read the set of results and increment stats for any results that
+ * appera to be erroneous.
*/
for (i = 0; i < number_readings; i++) {
efx_ptp_read_timeset(synch_buf, &ptp->timeset[i]);
synch_buf += MC_CMD_PTP_OUT_SYNCHRONIZE_TIMESET_LEN;
- if (ptp->timeset[i].window > SYNCHRONISATION_GRANULARITY_NS) {
- if (min_valid) {
- if (ptp->timeset[i].window < min_set)
- min_set = ptp->timeset[i].window;
- } else {
- min_valid = true;
- min_set = ptp->timeset[i].window;
- }
- }
- }
-
- if (min_valid) {
- if (ptp->base_sync_valid && (min_set > ptp->base_sync_ns))
- min = ptp->base_sync_ns;
- else
- min = min_set;
- } else {
- min = SYNCHRONISATION_GRANULARITY_NS;
}
- /* Discard excessively long synchronise durations. The MC times
- * when it finishes reading the host time so the corrected window
- * time should be fairly constant for a given platform.
+ /* Find the last good host-MC synchronization result. The MC times
+ * when it finishes reading the host time so the corrected window time
+ * should be fairly constant for a given platform.
*/
total = 0;
for (i = 0; i < number_readings; i++)
@@ -492,8 +471,8 @@ static int efx_ptp_process_times(struct efx_nic *efx, u8 *synch_buf,
if (ngood == 0) {
netif_warn(efx, drv, efx->net_dev,
- "PTP no suitable synchronisations %dns %dns\n",
- ptp->base_sync_ns, min_set);
+ "PTP no suitable synchronisations %dns\n",
+ ptp->base_sync_ns);
return -EAGAIN;
}
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply related
* [PATCH net-next 02/22] sfc: PTP changes to support improved UUID filtering mode
From: Ben Hutchings @ 2013-03-11 19:53 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers, scrum-linux, Laurence Evans
In-Reply-To: <1363030400.2608.37.camel@bwh-desktop.uk.solarflarecom.com>
From: Laurence Evans <levans@solarflare.com>
There is a long-standing problem with the packet-timestamp matching in
the driver. When a PTP packet is received by the MC, the FPGA
timestamps the packet and the MC sends the timestamp and 6 bytes of
the UUID to the driver. The driver then matches the timestamp against
received packets using the same 6 bytes of UUID.
The problem comes from the choice of which 6 bytes to use. The PTP
spec is slightly contradictory and misleading in one of the two places
where the UUIDs are discussed. From section 7.2.2.2 of the spec, a
PTPD2 UUID can be either a EUI-64 or a EUI-64 constructed from a
EUI-48. The typical ethernet based implementation uses a EUI-64
constructed from a EUI-48. This works by taking the first 3 bytes of
the MAC address of the NIC being used for PTP (the OUI), then
inserting 0xFF, 0xFE, then taking the last 3 bytes of the MAC address
giving
MAC[0], MAC[1], MAC[2], 0xFF, 0xFE, MAC[3], MAC[4], MAC[5]
The current MC firmware and driver discard the first two bytes of this
UUID and packets are matched against timestamps using bytes 2 to 7 so
there is a small risk that in a deployment of Solarflare PTP NICs used
with other vendors NICs, that a PTP packet could be matched against
the wrong timestamp. This applies to all other organisations whose
third byte of the OUI is 0x53. It's a long list but I notice that it
includes Cisco.
The necessary modifications to use bytes 0-2 and 5-7 of the UUID to
match against are quite small but introduce incompatibility between
older version of the firmware and driver.
When PTP is enabled via SO_TIMESTAMPING specifying PTP V2, the driver
will try to enable PTP in the firmware using the enhanced mode
(above). If the firmware returns an error, the driver will enable PTP
in the firmware using the old mode.
[bwh: Fix some style errors; remove private ioctl bits]
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/ethernet/sfc/mcdi_pcol.h | 1 +
drivers/net/ethernet/sfc/ptp.c | 61 +++++++++++++++++++++++++---------
2 files changed, 46 insertions(+), 16 deletions(-)
diff --git a/drivers/net/ethernet/sfc/mcdi_pcol.h b/drivers/net/ethernet/sfc/mcdi_pcol.h
index 9d426d0..c5c9747 100644
--- a/drivers/net/ethernet/sfc/mcdi_pcol.h
+++ b/drivers/net/ethernet/sfc/mcdi_pcol.h
@@ -553,6 +553,7 @@
#define MC_CMD_PTP_MODE_V1_VLAN 0x1 /* enum */
#define MC_CMD_PTP_MODE_V2 0x2 /* enum */
#define MC_CMD_PTP_MODE_V2_VLAN 0x3 /* enum */
+#define MC_CMD_PTP_MODE_V2_ENHANCED 0x4 /* enum */
/* MC_CMD_PTP_IN_DISABLE msgrequest */
#define MC_CMD_PTP_IN_DISABLE_LEN 8
diff --git a/drivers/net/ethernet/sfc/ptp.c b/drivers/net/ethernet/sfc/ptp.c
index faf4baf..2b40cbd 100644
--- a/drivers/net/ethernet/sfc/ptp.c
+++ b/drivers/net/ethernet/sfc/ptp.c
@@ -99,6 +99,9 @@
#define PTP_V2_VERSION_LENGTH 1
#define PTP_V2_VERSION_OFFSET 29
+#define PTP_V2_UUID_LENGTH 8
+#define PTP_V2_UUID_OFFSET 48
+
/* Although PTP V2 UUIDs are comprised a ClockIdentity (8) and PortNumber (2),
* the MC only captures the last six bytes of the clock identity. These values
* reflect those, not the ones used in the standard. The standard permits
@@ -1011,7 +1014,7 @@ static bool efx_ptp_rx(struct efx_channel *channel, struct sk_buff *skb)
struct efx_nic *efx = channel->efx;
struct efx_ptp_data *ptp = efx->ptp_data;
struct efx_ptp_match *match = (struct efx_ptp_match *)skb->cb;
- u8 *data;
+ u8 *match_data_012, *match_data_345;
unsigned int version;
match->expiry = jiffies + msecs_to_jiffies(PKT_EVENT_LIFETIME_MS);
@@ -1025,21 +1028,35 @@ static bool efx_ptp_rx(struct efx_channel *channel, struct sk_buff *skb)
if (version != PTP_VERSION_V1) {
return false;
}
+
+ /* PTP V1 uses all six bytes of the UUID to match the packet
+ * to the timestamp
+ */
+ match_data_012 = skb->data + PTP_V1_UUID_OFFSET;
+ match_data_345 = skb->data + PTP_V1_UUID_OFFSET + 3;
} else {
if (skb->len < PTP_V2_MIN_LENGTH) {
return false;
}
version = skb->data[PTP_V2_VERSION_OFFSET];
-
- BUG_ON(ptp->mode != MC_CMD_PTP_MODE_V2);
- BUILD_BUG_ON(PTP_V1_UUID_OFFSET != PTP_V2_MC_UUID_OFFSET);
- BUILD_BUG_ON(PTP_V1_UUID_LENGTH != PTP_V2_MC_UUID_LENGTH);
- BUILD_BUG_ON(PTP_V1_SEQUENCE_OFFSET != PTP_V2_SEQUENCE_OFFSET);
- BUILD_BUG_ON(PTP_V1_SEQUENCE_LENGTH != PTP_V2_SEQUENCE_LENGTH);
-
if ((version & PTP_VERSION_V2_MASK) != PTP_VERSION_V2) {
return false;
}
+
+ /* The original V2 implementation uses bytes 2-7 of
+ * the UUID to match the packet to the timestamp. This
+ * discards two of the bytes of the MAC address used
+ * to create the UUID (SF bug 33070). The PTP V2
+ * enhanced mode fixes this issue and uses bytes 0-2
+ * and byte 5-7 of the UUID.
+ */
+ match_data_345 = skb->data + PTP_V2_UUID_OFFSET + 5;
+ if (ptp->mode == MC_CMD_PTP_MODE_V2) {
+ match_data_012 = skb->data + PTP_V2_UUID_OFFSET + 2;
+ } else {
+ match_data_012 = skb->data + PTP_V2_UUID_OFFSET + 0;
+ BUG_ON(ptp->mode != MC_CMD_PTP_MODE_V2_ENHANCED);
+ }
}
/* Does this packet require timestamping? */
@@ -1052,14 +1069,19 @@ static bool efx_ptp_rx(struct efx_channel *channel, struct sk_buff *skb)
timestamps = skb_hwtstamps(skb);
memset(timestamps, 0, sizeof(*timestamps));
+ /* We expect the sequence number to be in the same position in
+ * the packet for PTP V1 and V2
+ */
+ BUILD_BUG_ON(PTP_V1_SEQUENCE_OFFSET != PTP_V2_SEQUENCE_OFFSET);
+ BUILD_BUG_ON(PTP_V1_SEQUENCE_LENGTH != PTP_V2_SEQUENCE_LENGTH);
+
/* Extract UUID/Sequence information */
- data = skb->data + PTP_V1_UUID_OFFSET;
- match->words[0] = (data[0] |
- (data[1] << 8) |
- (data[2] << 16) |
- (data[3] << 24));
- match->words[1] = (data[4] |
- (data[5] << 8) |
+ match->words[0] = (match_data_012[0] |
+ (match_data_012[1] << 8) |
+ (match_data_012[2] << 16) |
+ (match_data_345[0] << 24));
+ match->words[1] = (match_data_345[1] |
+ (match_data_345[2] << 8) |
(skb->data[PTP_V1_SEQUENCE_OFFSET +
PTP_V1_SEQUENCE_LENGTH - 1] <<
16));
@@ -1165,7 +1187,7 @@ static int efx_ptp_ts_init(struct efx_nic *efx, struct hwtstamp_config *init)
* timestamped
*/
init->rx_filter = HWTSTAMP_FILTER_PTP_V2_L4_EVENT;
- new_mode = MC_CMD_PTP_MODE_V2;
+ new_mode = MC_CMD_PTP_MODE_V2_ENHANCED;
enable_wanted = true;
break;
case HWTSTAMP_FILTER_PTP_V2_EVENT:
@@ -1184,7 +1206,14 @@ static int efx_ptp_ts_init(struct efx_nic *efx, struct hwtstamp_config *init)
if (init->tx_type != HWTSTAMP_TX_OFF)
enable_wanted = true;
+ /* Old versions of the firmware do not support the improved
+ * UUID filtering option (SF bug 33070). If the firmware does
+ * not accept the enhanced mode, fall back to the standard PTP
+ * v2 UUID filtering.
+ */
rc = efx_ptp_change_mode(efx, enable_wanted, new_mode);
+ if ((rc != 0) && (new_mode == MC_CMD_PTP_MODE_V2_ENHANCED))
+ rc = efx_ptp_change_mode(efx, enable_wanted, MC_CMD_PTP_MODE_V2);
if (rc != 0)
return rc;
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply related
* [PATCH net-next 01/22] sfc: Allow efx_channel_type::receive_skb() to reject a packet
From: Ben Hutchings @ 2013-03-11 19:53 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers, scrum-linux, Laurence Evans
In-Reply-To: <1363030400.2608.37.camel@bwh-desktop.uk.solarflarecom.com>
Instead of having efx_ptp_rx() call netif_receive_skb() for an invalid
PTP packet, make it return false for rejected packets and have
efx_rx_deliver() pass them up.
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/ethernet/sfc/net_driver.h | 2 +-
drivers/net/ethernet/sfc/ptp.c | 16 +++++++---------
drivers/net/ethernet/sfc/rx.c | 10 ++++++----
3 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
index 0a90abd..cdcf510 100644
--- a/drivers/net/ethernet/sfc/net_driver.h
+++ b/drivers/net/ethernet/sfc/net_driver.h
@@ -410,7 +410,7 @@ struct efx_channel_type {
void (*post_remove)(struct efx_channel *);
void (*get_name)(struct efx_channel *, char *buf, size_t len);
struct efx_channel *(*copy)(const struct efx_channel *);
- void (*receive_skb)(struct efx_channel *, struct sk_buff *);
+ bool (*receive_skb)(struct efx_channel *, struct sk_buff *);
bool keep_eventq;
};
diff --git a/drivers/net/ethernet/sfc/ptp.c b/drivers/net/ethernet/sfc/ptp.c
index 3f93624..faf4baf 100644
--- a/drivers/net/ethernet/sfc/ptp.c
+++ b/drivers/net/ethernet/sfc/ptp.c
@@ -1006,7 +1006,7 @@ bool efx_ptp_is_ptp_tx(struct efx_nic *efx, struct sk_buff *skb)
* the receive timestamp from the MC - this will probably occur after the
* packet arrival because of the processing in the MC.
*/
-static void efx_ptp_rx(struct efx_channel *channel, struct sk_buff *skb)
+static bool efx_ptp_rx(struct efx_channel *channel, struct sk_buff *skb)
{
struct efx_nic *efx = channel->efx;
struct efx_ptp_data *ptp = efx->ptp_data;
@@ -1019,18 +1019,15 @@ static void efx_ptp_rx(struct efx_channel *channel, struct sk_buff *skb)
/* Correct version? */
if (ptp->mode == MC_CMD_PTP_MODE_V1) {
if (skb->len < PTP_V1_MIN_LENGTH) {
- netif_receive_skb(skb);
- return;
+ return false;
}
version = ntohs(*(__be16 *)&skb->data[PTP_V1_VERSION_OFFSET]);
if (version != PTP_VERSION_V1) {
- netif_receive_skb(skb);
- return;
+ return false;
}
} else {
if (skb->len < PTP_V2_MIN_LENGTH) {
- netif_receive_skb(skb);
- return;
+ return false;
}
version = skb->data[PTP_V2_VERSION_OFFSET];
@@ -1041,8 +1038,7 @@ static void efx_ptp_rx(struct efx_channel *channel, struct sk_buff *skb)
BUILD_BUG_ON(PTP_V1_SEQUENCE_LENGTH != PTP_V2_SEQUENCE_LENGTH);
if ((version & PTP_VERSION_V2_MASK) != PTP_VERSION_V2) {
- netif_receive_skb(skb);
- return;
+ return false;
}
}
@@ -1073,6 +1069,8 @@ static void efx_ptp_rx(struct efx_channel *channel, struct sk_buff *skb)
skb_queue_tail(&ptp->rxq, skb);
queue_work(ptp->workwq, &ptp->work);
+
+ return true;
}
/* Transmit a PTP packet. This has to be transmitted by the MC
diff --git a/drivers/net/ethernet/sfc/rx.c b/drivers/net/ethernet/sfc/rx.c
index bb579a6..f31c23e 100644
--- a/drivers/net/ethernet/sfc/rx.c
+++ b/drivers/net/ethernet/sfc/rx.c
@@ -575,12 +575,14 @@ static void efx_rx_deliver(struct efx_channel *channel,
/* Record the rx_queue */
skb_record_rx_queue(skb, channel->rx_queue.core_index);
- /* Pass the packet up */
if (channel->type->receive_skb)
- channel->type->receive_skb(channel, skb);
- else
- netif_receive_skb(skb);
+ if (channel->type->receive_skb(channel, skb))
+ goto handled;
+
+ /* Pass the packet up */
+ netif_receive_skb(skb);
+handled:
/* Update allocation strategy method */
channel->rx_alloc_level += RX_ALLOC_FACTOR_SKB;
}
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply related
* Re: [PATCH linux-next v2] SUNRPC: rpcrdma_register_default_external: Dynamically allocate ib_phys_buf
From: Myklebust, Trond @ 2013-03-11 19:48 UTC (permalink / raw)
To: J. Bruce Fields
Cc: Tim Gardner, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
David S. Miller, Tom Tucker, Haggai Eran, Or Gerlitz,
Shani Michaeli, linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20130311191545.GA642-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
On Mon, 2013-03-11 at 15:15 -0400, J. Bruce Fields wrote:
> On Mon, Mar 11, 2013 at 12:51:44PM -0600, Tim Gardner wrote:
> > On 03/11/2013 12:14 PM, J. Bruce Fields wrote:
> > <snip>
> > >>
> > >> v2 - Move the array of 'struct ib_phys_buf' objects into struct rpcrdma_req
> > >> and pass this request down through rpcrdma_register_external() and
> > >> rpcrdma_register_default_external(). This is less overhead then using
> > >> kmalloc() and requires no extra error checking as the allocation burden is
> > >> shifted to the transport client.
> > >
> > > Oh good--so that works, and the req is the right place to put this? How
> > > are you testing this?
> > >
> > > (Just want to make it clear: I'm *not* an expert on the rdma code, so my
> > > suggestion to put this in the rpcrdma_req was a suggestion for something
> > > to look into, not a claim that it's correct.)
> > >
> >
> > Just compile tested so far. Incidentally, I've been through the call stack:
> >
> > call_transmit
> > xprt_transmit
> > xprt->ops->send_request(task)
> > xprt_rdma_send_request
> > rpcrdma_marshal_req
> > rpcrdma_create_chunks
> > rpcrdma_register_external
> > rpcrdma_register_default_external
> >
> > It appears that the context for kmalloc() should be fine unless there is
> > a spinlock held around call_transmit() (which seems unlikely).
>
> Right, though I think it shouldn't be GFP_KERNEL--looks like writes
> could wait on it.
Nothing inside the RPC client should be using anything heavier than
GFP_NOWAIT (unless done at setup).
> In any case, the embedding-in-rpcrdma_req solution does look cleaner if
> that's correct (e.g. if we can be sure there won't be two simultaneous
> users of that array).
Putting it in the rpcrdma_req means that you have one copy per transport
slot. Why not rather put it in the rpcrdma_xprt?
AFAICS you only need this array at transmit time for registering memory
for RDMA, at which time the transport XPRT_LOCK guarantees that nobody
else is competing for these resources.
--
Trond Myklebust
Linux NFS client maintainer
NetApp
Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org
www.netapp.com
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH RFC] unix: account skb memory to receiving socket's sk_rmem_alloc on sending
From: Hannes Frederic Sowa @ 2013-03-11 19:37 UTC (permalink / raw)
To: Eric Dumazet, netdev, yannick, xiyou.wangcong, davem
In-Reply-To: <20130310044043.GB2709@order.stressinduktion.org>
On Sun, Mar 10, 2013 at 05:40:43AM +0100, Hannes Frederic Sowa wrote:
> On Sun, Mar 10, 2013 at 05:31:01AM +0100, Eric Dumazet wrote:
> > Do you have a user test program ?
>
> I used a couple of perl scripts. I'll bring them in shape and will post them
> here, hopefully tomorrow.
Did not have enough time yesterday, so now here they are. I just
copy and pasted them together (the code is ugly, sorry). Last test
(test_send_multiple) should fail on current kernels and should work on
kernels with this patch applied (please increase max_dgram_qlen on as
it should not be the limiting factor):
https://gist.github.com/hannes/5136858
or
git clone https://gist.github.com/5136858.git
While doing this I added a testcase for poll on self-connected sockets. I
did report a writeable socket here, while in fact it is not. I fixed
this case by removing the 'if (unix_peer(other) != sk)' check in
unix_dgram_poll because it is not necessary anymore.
Two other changes are rather cosmetic: I replaced
atomic_read(&sk->sk_{r,w}mem_alloc) with sk_{r,w}mem_alloc_get.
The thing I am a bit nervous about is the handling of unix_write_space. I
disabled the call in sock_wfree (by setting SOCK_USE_WRITE_QUEUE) because
I am unsure if we can call sk_wake_async without the socket lock, which
would interfere with the unix_state spinlocks. Otherwise I think it is called
later on without socket lock, too. So I am tempted to remove this hunk from
the patch because otherwise sleeping processes in sock_alloc_send_pskb would
not be woken up.
[PATCH net-next RFC] unix: account skb memory to receiving socket's sk_rmem_alloc on sending
In case of unix datagram sockets, skb memory was only accounted in the
sending socket's sk_wmem_alloc. Hence, if one receiver would stop to
receive frames on its socket, the sending socket's send buffer space
could get exhausted and the socket would block sending datagrams to
other destionations, too.
This patch places the refcounted peer's unix address for AF_UNIX
SOCK_DGRAM sockets into the skb's UNIXCB. So a reference from the skb
to the receiving struct sock can be set and so enables to do proper skb
destructor handling for rmem and wmem. Buffer memory is then accounted
to the receiving socket. If the socket rmem is exhausted the normal
blocking and timeout behaviour kicks in.
Resource exhausion protection for unix dgram sockets is now based
only on sockets rmem checking. Unix dgram sockets do not rely on
sk_max_ack_backlog anymore. The controls for this are
/proc/sys/net/core/{r,w}mem_{default,max}.
This patch also changes the reporting of unix dgram rqueue size, as it
now reports not only the size of the first fragment but the amount of
readable memory for the socket.
Based on the patches from Yannick Koehler and Cong Wang.
Reported-by: Yannick Koehler <yannick@koehler.name>
CC: Yannick Koehler <yannick@koehler.name>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: David Miller <davem@davemloft.net>
Cc: Cong Wang <xiyou.wangcong@gmail.com>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
include/net/af_unix.h | 1 +
net/unix/af_unix.c | 81 ++++++++++++++++++++++++++++++++++++++-------------
net/unix/diag.c | 4 ++-
3 files changed, 65 insertions(+), 21 deletions(-)
diff --git a/include/net/af_unix.h b/include/net/af_unix.h
index 0a996a3..3855fcc 100644
--- a/include/net/af_unix.h
+++ b/include/net/af_unix.h
@@ -31,6 +31,7 @@ struct unix_skb_parms {
struct pid *pid; /* Skb credentials */
const struct cred *cred;
struct scm_fp_list *fp; /* Passed files */
+ struct unix_address *peer_address; /* only used for dgram */
#ifdef CONFIG_SECURITY_NETWORK
u32 secid; /* Security ID */
#endif
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 51be64f..7caf111 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -184,6 +184,12 @@ static inline int unix_recvq_full(struct sock const *sk)
return skb_queue_len(&sk->sk_receive_queue) > sk->sk_max_ack_backlog;
}
+static inline bool unix_rmem_full(struct sock const *sk,
+ struct sk_buff const *skb)
+{
+ return sk_rmem_alloc_get(sk) + skb->truesize > sk->sk_rcvbuf;
+}
+
struct sock *unix_peer_get(struct sock *s)
{
struct sock *peer;
@@ -316,7 +322,12 @@ found:
static inline int unix_writable(struct sock *sk)
{
- return (atomic_read(&sk->sk_wmem_alloc) << 2) <= sk->sk_sndbuf;
+ return (sk_wmem_alloc_get(sk) << 2) <= sk->sk_sndbuf;
+}
+
+static inline bool unix_other_writable(struct sock *sk)
+{
+ return (sk_rmem_alloc_get(sk) << 2) <= sk->sk_rcvbuf;
}
static void unix_write_space(struct sock *sk)
@@ -635,6 +646,8 @@ static struct sock *unix_create1(struct net *net, struct socket *sock)
goto out;
sock_init_data(sock, sk);
+ if (sk->sk_type == SOCK_DGRAM || sk->sk_type == SOCK_SEQPACKET)
+ sock_set_flag(sk, SOCK_USE_WRITE_QUEUE);
lockdep_set_class(&sk->sk_receive_queue.lock,
&af_unix_sk_receive_queue_lock_key);
@@ -1032,14 +1045,18 @@ out:
static long unix_wait_for_peer(struct sock *other, long timeo)
{
struct unix_sock *u = unix_sk(other);
- int sched;
+ bool sched;
DEFINE_WAIT(wait);
prepare_to_wait_exclusive(&u->peer_wait, &wait, TASK_INTERRUPTIBLE);
sched = !sock_flag(other, SOCK_DEAD) &&
- !(other->sk_shutdown & RCV_SHUTDOWN) &&
- unix_recvq_full(other);
+ !(other->sk_shutdown & RCV_SHUTDOWN);
+
+ if (other->sk_type == SOCK_DGRAM || other->sk_type == SOCK_SEQPACKET)
+ sched = sched && unix_other_writable(other);
+ else
+ sched = sched && unix_recvq_full(other);
unix_state_unlock(other);
@@ -1336,7 +1353,7 @@ static void unix_detach_fds(struct scm_cookie *scm, struct sk_buff *skb)
unix_notinflight(scm->fp->fp[i]);
}
-static void unix_destruct_scm(struct sk_buff *skb)
+static inline void __unix_skb_destruct(struct sk_buff *skb)
{
struct scm_cookie scm;
memset(&scm, 0, sizeof(scm));
@@ -1348,6 +1365,19 @@ static void unix_destruct_scm(struct sk_buff *skb)
/* Alas, it calls VFS */
/* So fscking what? fput() had been SMP-safe since the last Summer */
scm_destroy(&scm);
+ if (UNIXCB(skb).peer_address)
+ unix_release_addr(UNIXCB(skb).peer_address);
+}
+
+static void unix_skb_destruct_r(struct sk_buff *skb)
+{
+ __unix_skb_destruct(skb);
+ sock_rfree(skb);
+}
+
+static void unix_skb_destruct_w(struct sk_buff *skb)
+{
+ __unix_skb_destruct(skb);
sock_wfree(skb);
}
@@ -1398,7 +1428,7 @@ static int unix_scm_to_skb(struct scm_cookie *scm, struct sk_buff *skb, bool sen
if (scm->fp && send_fds)
err = unix_attach_fds(scm, skb);
- skb->destructor = unix_destruct_scm;
+ skb->destructor = unix_skb_destruct_w;
return err;
}
@@ -1420,6 +1450,15 @@ static void maybe_add_creds(struct sk_buff *skb, const struct socket *sock,
}
}
+static void unix_skb_set_owner_r(struct sk_buff *skb, struct sock *oldsk,
+ struct sock *newsk)
+{
+ sock_wfree(skb);
+ skb->sk = newsk;
+ skb->destructor = unix_skb_destruct_r;
+ atomic_add(skb->truesize, &newsk->sk_rmem_alloc);
+}
+
/*
* Send AF_UNIX data.
*/
@@ -1484,6 +1523,11 @@ static int unix_dgram_sendmsg(struct kiocb *kiocb, struct socket *sock,
if (skb == NULL)
goto out;
+ if (u->addr) {
+ UNIXCB(skb).peer_address = u->addr;
+ atomic_inc(&UNIXCB(skb).peer_address->refcnt);
+ }
+
err = unix_scm_to_skb(siocb->scm, skb, true);
if (err < 0)
goto out_free;
@@ -1559,7 +1603,7 @@ restart:
goto out_unlock;
}
- if (unix_peer(other) != sk && unix_recvq_full(other)) {
+ if (unix_rmem_full(other, skb)) {
if (!timeo) {
err = -EAGAIN;
goto out_unlock;
@@ -1577,6 +1621,7 @@ restart:
if (sock_flag(other, SOCK_RCVTSTAMP))
__net_timestamp(skb);
maybe_add_creds(skb, sock, other);
+ unix_skb_set_owner_r(skb, sk, other);
skb_queue_tail(&other->sk_receive_queue, skb);
if (max_level > unix_sk(other)->recursion_level)
unix_sk(other)->recursion_level = max_level;
@@ -1749,14 +1794,12 @@ static int unix_seqpacket_recvmsg(struct kiocb *iocb, struct socket *sock,
return unix_dgram_recvmsg(iocb, sock, msg, size, flags);
}
-static void unix_copy_addr(struct msghdr *msg, struct sock *sk)
+static void unix_copy_addr(struct msghdr *msg, struct unix_address *ua)
{
- struct unix_sock *u = unix_sk(sk);
-
msg->msg_namelen = 0;
- if (u->addr) {
- msg->msg_namelen = u->addr->len;
- memcpy(msg->msg_name, u->addr->name, u->addr->len);
+ if (ua) {
+ msg->msg_namelen = ua->len;
+ memcpy(msg->msg_name, ua->name, ua->len);
}
}
@@ -1802,7 +1845,7 @@ static int unix_dgram_recvmsg(struct kiocb *iocb, struct socket *sock,
POLLOUT | POLLWRNORM | POLLWRBAND);
if (msg->msg_name)
- unix_copy_addr(msg, skb->sk);
+ unix_copy_addr(msg, UNIXCB(skb).peer_address);
if (size > skb->len - skip)
size = skb->len - skip;
@@ -2002,7 +2045,7 @@ again:
/* Copy address just once */
if (sunaddr) {
- unix_copy_addr(msg, skb->sk);
+ unix_copy_addr(msg, unix_sk(skb->sk)->addr);
sunaddr = NULL;
}
@@ -2223,11 +2266,9 @@ static unsigned int unix_dgram_poll(struct file *file, struct socket *sock,
writable = unix_writable(sk);
other = unix_peer_get(sk);
if (other) {
- if (unix_peer(other) != sk) {
- sock_poll_wait(file, &unix_sk(other)->peer_wait, wait);
- if (unix_recvq_full(other))
- writable = 0;
- }
+ sock_poll_wait(file, &unix_sk(other)->peer_wait, wait);
+ if (!unix_other_writable(other))
+ writable = 0;
sock_put(other);
}
diff --git a/net/unix/diag.c b/net/unix/diag.c
index d591091..41a38ec 100644
--- a/net/unix/diag.c
+++ b/net/unix/diag.c
@@ -102,7 +102,9 @@ static int sk_diag_show_rqlen(struct sock *sk, struct sk_buff *nlskb)
rql.udiag_rqueue = sk->sk_receive_queue.qlen;
rql.udiag_wqueue = sk->sk_max_ack_backlog;
} else {
- rql.udiag_rqueue = (u32) unix_inq_len(sk);
+ rql.udiag_rqueue = (u32) (sk->sk_type == SOCK_DGRAM ?
+ sk_rmem_alloc_get(sk) :
+ unix_inq_len(sk));
rql.udiag_wqueue = (u32) unix_outq_len(sk);
}
--
1.8.1.4
^ permalink raw reply related
* Pull request: sfc-next 2013-03-11
From: Ben Hutchings @ 2013-03-11 19:33 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers, scrum-linux, Laurence Evans
[-- Attachment #1: Type: text/plain, Size: 3447 bytes --]
The following changes since commit 6906f4ed6f85b2d72fd944e15da6a905fdde8b40:
htb: add HTB_DIRECT_QLEN attribute (2013-03-06 15:40:53 -0500)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/bwh/sfc-next.git for-davem
(commit 1648a23fa159e5c433aac06dc5e0d9db36146016)
1. Merge sfc changes (only) accepted for 3.9.
2. PTP improvements from Laurence Evans.
3. Overhaul of RX buffer management:
- Always allocate pages, and enable scattering where possible
- Fit as many buffers as will fit into a page, rather than limiting to 2
- Introduce recycle rings to reduce the need for IOMMU mapping and
unmapping
4. PCI error recovery (AER and EEH) implementation.
5. Fix a bug in RX filter replacement.
6. Fix configuration with 1 RX queue in the PF and multiple RX queues in
VFs.
Ben.
Alexandre Rames (2):
sfc: Remove rx_alloc_method SKB
sfc: Add AER and EEH support for Siena
Ben Hutchings (19):
sfc: Disable soft interrupt handling during efx_device_detach_sync()
sfc: Correct efx_rx_buffer::page_offset when EFX_PAGE_IP_ALIGN != 0
Merge branch 'sfc-3.9' into master
sfc: Allow efx_channel_type::receive_skb() to reject a packet
sfc: More sensible semantics for efx_filter_insert_filter() replace flag
sfc: Remove redundant parameter to efx_filter_search()
sfc: Don't use efx_filter_{build,hash,increment}() for default MAC filters
sfc: Merge efx_filter_search() into efx_filter_insert()
sfc: Fix replacement detection in efx_filter_insert_filter()
sfc: Disable RSS when using SR-IOV and only 1 RX queue on the PF
sfc: Document current usage of efx_rx_buffer::len and efx_nic::rx_buffer_len
sfc: Properly distinguish RX buffer and DMA lengths
sfc: Make RX queue descriptor counts unsigned for consistency
sfc: Wrap __efx_rx_packet() with efx_rx_flush_packet()
sfc: Replace efx_rx_buf_eh() with simpler efx_rx_buf_va()
sfc: Explicitly prefetch RX hash prefix, not just Ethernet heade
sfc: Update RX buffer address together with length
sfc: Enable RX DMA scattering where possible
sfc: Replace efx_rx_is_last_buffer() with a flag
Daniel Pieczko (2):
sfc: reuse pages to avoid DMA mapping/unmapping costs
sfc: allocate more RX buffers per page
Laurence Evans (2):
sfc: PTP changes to support improved UUID filtering mode
sfc: tidy up PTP synchronize function efx_ptp_process_times()
drivers/net/ethernet/sfc/efx.c | 268 ++++++++++--
drivers/net/ethernet/sfc/efx.h | 18 +-
drivers/net/ethernet/sfc/enum.h | 12 +-
drivers/net/ethernet/sfc/ethtool.c | 4 +-
drivers/net/ethernet/sfc/falcon.c | 17 +-
drivers/net/ethernet/sfc/filter.c | 249 ++++++++---
drivers/net/ethernet/sfc/mcdi_pcol.h | 1 +
drivers/net/ethernet/sfc/net_driver.h | 97 +++--
drivers/net/ethernet/sfc/nic.c | 90 +++-
drivers/net/ethernet/sfc/ptp.c | 116 +++---
drivers/net/ethernet/sfc/rx.c | 793 +++++++++++++++++----------------
drivers/net/ethernet/sfc/siena.c | 25 +-
12 files changed, 1072 insertions(+), 618 deletions(-)
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: [PATCH linux-next v2] SUNRPC: rpcrdma_register_default_external: Dynamically allocate ib_phys_buf
From: J. Bruce Fields @ 2013-03-11 19:15 UTC (permalink / raw)
To: Tim Gardner
Cc: linux-kernel, Trond Myklebust, David S. Miller, Tom Tucker,
Haggai Eran, Or Gerlitz, Shani Michaeli, linux-nfs, netdev
In-Reply-To: <513E27C0.8050108@canonical.com>
On Mon, Mar 11, 2013 at 12:51:44PM -0600, Tim Gardner wrote:
> On 03/11/2013 12:14 PM, J. Bruce Fields wrote:
> <snip>
> >>
> >> v2 - Move the array of 'struct ib_phys_buf' objects into struct rpcrdma_req
> >> and pass this request down through rpcrdma_register_external() and
> >> rpcrdma_register_default_external(). This is less overhead then using
> >> kmalloc() and requires no extra error checking as the allocation burden is
> >> shifted to the transport client.
> >
> > Oh good--so that works, and the req is the right place to put this? How
> > are you testing this?
> >
> > (Just want to make it clear: I'm *not* an expert on the rdma code, so my
> > suggestion to put this in the rpcrdma_req was a suggestion for something
> > to look into, not a claim that it's correct.)
> >
>
> Just compile tested so far. Incidentally, I've been through the call stack:
>
> call_transmit
> xprt_transmit
> xprt->ops->send_request(task)
> xprt_rdma_send_request
> rpcrdma_marshal_req
> rpcrdma_create_chunks
> rpcrdma_register_external
> rpcrdma_register_default_external
>
> It appears that the context for kmalloc() should be fine unless there is
> a spinlock held around call_transmit() (which seems unlikely).
Right, though I think it shouldn't be GFP_KERNEL--looks like writes
could wait on it.
In any case, the embedding-in-rpcrdma_req solution does look cleaner if
that's correct (e.g. if we can be sure there won't be two simultaneous
users of that array).
--b.
^ permalink raw reply
* Re: [net-next.git 4/9] stmmac: add the support for PTP hw clock driver
From: Richard Cochran @ 2013-03-11 18:55 UTC (permalink / raw)
To: Rayagond K; +Cc: Giuseppe CAVALLARO, netdev, bh74.an
In-Reply-To: <CAJ3bTp7n-Y6TYrhDMe58fALtX5O34qifO9khWYXpo-OwnrRKiQ@mail.gmail.com>
On Sun, Mar 10, 2013 at 08:37:24PM +0530, Rayagond K wrote:
> On Sun, Mar 10, 2013 at 5:40 PM, Richard Cochran <richardcochran@gmail.com>wrote:
> > Don't you need locking here to protect against concurrent callers of
> > config_addend?
> >
>
> Yes we need locking here too, will add it.
>
> But I can see no locking in this function in other two driver'e
> ppt_ixp46x.c and gianfar_ptp.c. So I just followed these two driver.
Those two drivers don't need locking, because setting the addend
register is a single 32 bit write operation in both cases.
In your driver, setting the addend involves a sequence of writes and
reads, in addition to polling a status bit, so you do need to
serialize access to those registers.
Thanks,
Richard
^ permalink raw reply
* Re: [PATCH linux-next v2] SUNRPC: rpcrdma_register_default_external: Dynamically allocate ib_phys_buf
From: Tim Gardner @ 2013-03-11 18:51 UTC (permalink / raw)
To: J. Bruce Fields
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Trond Myklebust,
David S. Miller, Tom Tucker, Haggai Eran, Or Gerlitz,
Shani Michaeli, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20130311181450.GC30618-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
On 03/11/2013 12:14 PM, J. Bruce Fields wrote:
<snip>
>>
>> v2 - Move the array of 'struct ib_phys_buf' objects into struct rpcrdma_req
>> and pass this request down through rpcrdma_register_external() and
>> rpcrdma_register_default_external(). This is less overhead then using
>> kmalloc() and requires no extra error checking as the allocation burden is
>> shifted to the transport client.
>
> Oh good--so that works, and the req is the right place to put this? How
> are you testing this?
>
> (Just want to make it clear: I'm *not* an expert on the rdma code, so my
> suggestion to put this in the rpcrdma_req was a suggestion for something
> to look into, not a claim that it's correct.)
>
Just compile tested so far. Incidentally, I've been through the call stack:
call_transmit
xprt_transmit
xprt->ops->send_request(task)
xprt_rdma_send_request
rpcrdma_marshal_req
rpcrdma_create_chunks
rpcrdma_register_external
rpcrdma_register_default_external
It appears that the context for kmalloc() should be fine unless there is
a spinlock held around call_transmit() (which seems unlikely).
rtg
--
Tim Gardner tim.gardner-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH net-next 2/2] gianfar: Bundle rx buffer allocation
From: Paul Gortmaker @ 2013-03-11 18:16 UTC (permalink / raw)
To: Claudiu Manoil; +Cc: netdev, David S. Miller
In-Reply-To: <1363020877-30223-2-git-send-email-claudiu.manoil@freescale.com>
On Mon, Mar 11, 2013 at 12:54 PM, Claudiu Manoil
<claudiu.manoil@freescale.com> wrote:
> Use a more common and efficient pattern for rx buffer processing.
> Instead of allocating one new buffer (skb) and processing (cleanning up)
> the "current" buffer for each iteration, use a more efficient
> approach that bundles the allocation of several rx buffers at a time.
> This way the 2 rx jobs (buffer processing & buffer allocation) are
> more localized along processing iterations.
Aside from it being "more common and efficient", it would be nice
to have concrete underlying reasons for doing this -- i.e. what sort
of performance gains are achieved, what sort of additional memory
pressure can we expect on the older 83xx boards with < 512MB?
>
> So, for each Rx ring we have the next_to_clean index which points to
> the first received buffer and gets updated by the buffer cleanup
> routine. The next_to_alloc index is updated during buffer allocation.
> Once a given number of Rx buffers have been cleaned up/ processed,
> rx allocation is being invoked to refill the cleaned-up BDs with new
> buffers to accommodate the subsequent Rx packets. This is how the
> bundling effect is achieved.
>
> The number of unused BDs to be refilled with new allocated buffers
> can be computed based on the next_to_clean and next_to_alloc indexes.
> The implementation follows a commonly used pattern in other drivers too.
> gfar_alloc_rx_buff() is now the rx buffer allocation routine, the only
> method that updates the next_to_alloc index of a given ring. It tries
> (on a best effort basis) to refill/ alloc the requested number of
> buffers, and updates next_to_alloc to reflect the actual number of allocated
> buffers. While it has work to do, the rx cleanup routine requests buffer
> allocations for a fixed number of buffers (bundle), and when there's
> nothing left to clean it makes a final request to refill the remaining
> unused BDs with new buffers before exiting.
>
> Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
> ---
> drivers/net/ethernet/freescale/gianfar.c | 126 +++++++++++++++---------------
> drivers/net/ethernet/freescale/gianfar.h | 14 +++-
> 2 files changed, 73 insertions(+), 67 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
> index af91196..f117da6 100644
> --- a/drivers/net/ethernet/freescale/gianfar.c
> +++ b/drivers/net/ethernet/freescale/gianfar.c
> @@ -110,7 +110,8 @@ static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev);
> static void gfar_reset_task(struct work_struct *work);
> static void gfar_timeout(struct net_device *dev);
> static int gfar_close(struct net_device *dev);
> -static struct sk_buff *gfar_alloc_skb(struct net_device *dev);
> +static void gfar_alloc_rx_buff(struct gfar_priv_rx_q *rx_queue,
> + int alloc_cnt);
> static void gfar_new_rxbdp(struct gfar_priv_rx_q *rx_queue, struct rxbd8 *bdp,
> struct sk_buff *skb);
> static int gfar_set_mac_address(struct net_device *dev);
> @@ -163,13 +164,12 @@ static void gfar_init_rxbdp(struct gfar_priv_rx_q *rx_queue, struct rxbd8 *bdp,
> bdp->lstatus = lstatus;
> }
>
> -static int gfar_init_bds(struct net_device *ndev)
> +static void gfar_init_bds(struct net_device *ndev)
> {
> struct gfar_private *priv = netdev_priv(ndev);
> struct gfar_priv_tx_q *tx_queue = NULL;
> struct gfar_priv_rx_q *rx_queue = NULL;
> struct txbd8 *txbdp;
> - struct rxbd8 *rxbdp;
> int i, j;
>
> for (i = 0; i < priv->num_tx_queues; i++) {
> @@ -196,33 +196,10 @@ static int gfar_init_bds(struct net_device *ndev)
>
> for (i = 0; i < priv->num_rx_queues; i++) {
> rx_queue = priv->rx_queue[i];
> - rx_queue->cur_rx = rx_queue->rx_bd_base;
> - rx_queue->skb_currx = 0;
> - rxbdp = rx_queue->rx_bd_base;
> -
> - for (j = 0; j < rx_queue->rx_ring_size; j++) {
> - struct sk_buff *skb = rx_queue->rx_skbuff[j];
> -
> - if (skb) {
> - gfar_init_rxbdp(rx_queue, rxbdp,
> - rxbdp->bufPtr);
> - } else {
> - skb = gfar_alloc_skb(ndev);
> - if (!skb) {
> - netdev_err(ndev, "Can't allocate RX buffers\n");
> - return -ENOMEM;
> - }
> - rx_queue->rx_skbuff[j] = skb;
> -
> - gfar_new_rxbdp(rx_queue, rxbdp, skb);
> - }
> -
> - rxbdp++;
> - }
> -
> + rx_queue->next_to_clean = rx_queue->next_to_alloc = 0;
> + gfar_alloc_rx_buff(rx_queue, GFAR_RXBD_UNUSED(rx_queue));
> }
>
> - return 0;
> }
>
> static int gfar_alloc_skb_resources(struct net_device *ndev)
> @@ -301,8 +278,7 @@ static int gfar_alloc_skb_resources(struct net_device *ndev)
> rx_queue->rx_skbuff[j] = NULL;
> }
>
> - if (gfar_init_bds(ndev))
> - goto cleanup;
> + gfar_init_bds(ndev);
>
> return 0;
>
> @@ -1366,10 +1342,7 @@ static int gfar_restore(struct device *dev)
> return 0;
> }
>
> - if (gfar_init_bds(ndev)) {
> - free_skb_resources(priv);
> - return -ENOMEM;
> - }
> + gfar_init_bds(ndev);
You just removed the only instance where gfar_restore could return
anything other than zero, hence the return value is now pointless.
>
> init_registers(ndev);
> gfar_set_mac_address(ndev);
> @@ -2612,18 +2585,46 @@ static void gfar_new_rxbdp(struct gfar_priv_rx_q *rx_queue, struct rxbd8 *bdp,
> gfar_init_rxbdp(rx_queue, bdp, buf);
> }
>
> -static struct sk_buff *gfar_alloc_skb(struct net_device *dev)
> +/* Best effort Rx buff allocation routine. Updates the next_to_alloc
> + * index of the given ring with the # of completed skb allocations.
> + */
> +static void gfar_alloc_rx_buff(struct gfar_priv_rx_q *rx_queue,
> + int alloc_cnt)
> {
> + struct net_device *dev = rx_queue->dev;
> struct gfar_private *priv = netdev_priv(dev);
> - struct sk_buff *skb;
> + struct rxbd8 *bdp, *base;
> + unsigned int i;
>
> - skb = netdev_alloc_skb(dev, priv->rx_buffer_size + RXBUF_ALIGNMENT);
> - if (!skb)
> - return NULL;
> + i = rx_queue->next_to_alloc;
> + base = rx_queue->rx_bd_base;
> + bdp = &rx_queue->rx_bd_base[i];
>
> - gfar_align_skb(skb);
> + while (alloc_cnt--) {
> + struct sk_buff *skb;
>
> - return skb;
> + skb = netdev_alloc_skb(dev,
> + priv->rx_buffer_size + RXBUF_ALIGNMENT);
> + if (unlikely(!skb))
> + break; /* try again with the next alloc request */
> +
> + gfar_align_skb(skb);
> +
> + rx_queue->rx_skbuff[i] = skb;
> +
> + /* Setup the new RxBD */
> + gfar_new_rxbdp(rx_queue, bdp, skb);
> +
> + /* Update to the next pointer */
> + bdp = next_bd(bdp, base, rx_queue->rx_ring_size);
> +
> + if (unlikely(++i == rx_queue->rx_ring_size))
> + i = 0;
> + }
> +
> + rx_queue->next_to_alloc = i;
> +
> + return;
> }
>
> static inline void count_errors(unsigned short status, struct net_device *dev)
> @@ -2746,24 +2747,26 @@ int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue, int rx_work_limit)
> struct sk_buff *skb;
> int pkt_len;
> int amount_pull;
> - int howmany = 0;
> + unsigned int i;
> + int howmany = 0, cleaned_cnt = 0;
This could simply be "cleaned". It is self evident it is a
counter, just as it is for "howmany".
> struct gfar_private *priv = netdev_priv(dev);
>
> + i = rx_queue->next_to_clean;
> /* Get the first full descriptor */
> - bdp = rx_queue->cur_rx;
> base = rx_queue->rx_bd_base;
> + bdp = &rx_queue->rx_bd_base[i];
>
> amount_pull = priv->uses_rxfcb ? GMAC_FCB_LEN : 0;
>
> while (!((bdp->status & RXBD_EMPTY) || (--rx_work_limit < 0))) {
> - struct sk_buff *newskb;
> -
> rmb();
>
> - /* Add another skb for the future */
> - newskb = gfar_alloc_skb(dev);
> + skb = rx_queue->rx_skbuff[i];
> + rx_queue->rx_skbuff[i] = NULL;
>
> - skb = rx_queue->rx_skbuff[rx_queue->skb_currx];
> + cleaned_cnt++;
> + if (unlikely(++i == rx_queue->rx_ring_size))
> + i = 0;
>
> dma_unmap_single(priv->dev, bdp->bufPtr,
> priv->rx_buffer_size, DMA_FROM_DEVICE);
> @@ -2772,15 +2775,13 @@ int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue, int rx_work_limit)
> bdp->length > priv->rx_buffer_size))
> bdp->status = RXBD_LARGE;
>
> - /* We drop the frame if we failed to allocate a new buffer */
> - if (unlikely(!newskb || !(bdp->status & RXBD_LAST) ||
> + if (unlikely(!(bdp->status & RXBD_LAST) ||
> bdp->status & RXBD_ERR)) {
> count_errors(bdp->status, dev);
>
> - if (unlikely(!newskb))
> - newskb = skb;
> - else if (skb)
> - dev_kfree_skb(skb);
> + /* discard faulty buffer */
Unclear to me what you mean by "faulty" here -- in the old instance
it was clear an allocation failure happened. But now gfar_alloc_rx_buf()
is void, and I can't immediately see what prevents allocations from
being retried forever...
> + dev_kfree_skb(skb);
> +
> } else {
> /* Increment the number of packets */
> rx_queue->stats.rx_packets++;
> @@ -2803,21 +2804,20 @@ int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue, int rx_work_limit)
>
> }
>
> - rx_queue->rx_skbuff[rx_queue->skb_currx] = newskb;
> -
> - /* Setup the new bdp */
> - gfar_new_rxbdp(rx_queue, bdp, newskb);
> + if (unlikely(cleaned_cnt >= GFAR_RX_BUFF_ALLOC)) {
> + gfar_alloc_rx_buff(rx_queue, cleaned_cnt);
> + cleaned_cnt = 0;
> + }
>
> /* Update to the next pointer */
> bdp = next_bd(bdp, base, rx_queue->rx_ring_size);
>
> - /* update to point at the next skb */
> - rx_queue->skb_currx = (rx_queue->skb_currx + 1) &
> - RX_RING_MOD_MASK(rx_queue->rx_ring_size);
> }
> + rx_queue->next_to_clean = i;
>
> - /* Update the current rxbd pointer to be the next one */
> - rx_queue->cur_rx = bdp;
> + cleaned_cnt = GFAR_RXBD_UNUSED(rx_queue);
> + if (cleaned_cnt)
> + gfar_alloc_rx_buff(rx_queue, cleaned_cnt);
>
> return howmany;
> }
> diff --git a/drivers/net/ethernet/freescale/gianfar.h b/drivers/net/ethernet/freescale/gianfar.h
> index 63a28d2..9adc1ce 100644
> --- a/drivers/net/ethernet/freescale/gianfar.h
> +++ b/drivers/net/ethernet/freescale/gianfar.h
> @@ -93,6 +93,9 @@ extern const char gfar_driver_version[];
> #define DEFAULT_TX_RING_SIZE 256
> #define DEFAULT_RX_RING_SIZE 256
>
> +/* Rx buffer allocation bundle size */
> +#define GFAR_RX_BUFF_ALLOC 16
Choice of 16 was made because of .... ?
Any need to ever tune this?
> +
> #define GFAR_RX_MAX_RING_SIZE 256
> #define GFAR_TX_MAX_RING_SIZE 256
>
> @@ -964,9 +967,9 @@ struct rx_q_stats {
> * struct gfar_priv_rx_q - per rx queue structure
> * @rxlock: per queue rx spin lock
> * @rx_skbuff: skb pointers
> - * @skb_currx: currently use skb pointer
> * @rx_bd_base: First rx buffer descriptor
> - * @cur_rx: Next free rx ring entry
> + * @next_to_alloc: index of the next buffer to be alloc'd
> + * @next_to_clean: index of the next buffer to be cleaned
> * @qindex: index of this queue
> * @dev: back pointer to the dev structure
> * @rx_ring_size: Rx ring size
> @@ -979,11 +982,11 @@ struct gfar_priv_rx_q {
> struct sk_buff ** rx_skbuff;
> dma_addr_t rx_bd_dma_base;
> struct rxbd8 *rx_bd_base;
> - struct rxbd8 *cur_rx;
> + u16 next_to_clean;
> + u16 next_to_alloc;
> struct net_device *dev;
> struct gfar_priv_grp *grp;
> struct rx_q_stats stats;
> - u16 skb_currx;
> u16 qindex;
> unsigned int rx_ring_size;
> /* RX Coalescing values */
> @@ -991,6 +994,9 @@ struct gfar_priv_rx_q {
> unsigned long rxic;
> };
>
> +#define GFAR_RXBD_UNUSED(Q) ((((Q)->next_to_clean > (Q)->next_to_alloc) ? \
> + 0 : (Q)->rx_ring_size) + (Q)->next_to_clean - (Q)->next_to_alloc - 1)
> +
Does this really need to be a macro vs a more readable static inline,
preferably with some comments on what it does and what criteria it
is using and why?
Part of my trouble in understanding this patch is probably due to not
being clear what GFAR_RXBD_UNUSED is telling us.
Also, what boards were tested? I'm out of office until Wed, so I won't
be able to test on my sbc8548 until at least then.
Thanks,
Paul.
--
> enum gfar_irqinfo_id {
> GFAR_TX = 0,
> GFAR_RX = 1,
> --
> 1.7.7.4
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH linux-next v2] SUNRPC: rpcrdma_register_default_external: Dynamically allocate ib_phys_buf
From: J. Bruce Fields @ 2013-03-11 18:14 UTC (permalink / raw)
To: Tim Gardner
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Trond Myklebust,
David S. Miller, Tom Tucker, Haggai Eran, Or Gerlitz,
Shani Michaeli, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1363023447-22453-1-git-send-email-tim.gardner-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
On Mon, Mar 11, 2013 at 11:37:27AM -0600, Tim Gardner wrote:
> rpcrdma_register_default_external() is several frames into the call stack which
> goes deeper yet. You run the risk of stack corruption by declaring such a large
> automatic variable, so move the array of 'struct ib_phys_buf' objects into the
> requestor structure 'struct rpcrdma_req' (which is dynamically allocated) in
> order to silence the frame-larger-than warning.
>
> net/sunrpc/xprtrdma/verbs.c: In function 'rpcrdma_register_default_external':
> net/sunrpc/xprtrdma/verbs.c:1774:1: warning: the frame size of 1056 bytes is larger than 1024 bytes [-Wframe-larger-than=]
>
> gcc version 4.6.3
>
> Cc: Trond Myklebust <Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org>
> Cc: "J. Bruce Fields" <bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
> Cc: "David S. Miller" <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
> Cc: Tom Tucker <tom-/Yg/VP3ZvrM@public.gmane.org>
> Cc: Haggai Eran <haggaie-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Cc: Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Cc: Shani Michaeli <shanim-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Cc: linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>
> Signed-off-by: Tim Gardner <tim.gardner-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
> ---
>
> v1 - Use kmalloc() to dynamically allocate and free the array of 'struct
> ib_phys_buf' objects
>
> v2 - Move the array of 'struct ib_phys_buf' objects into struct rpcrdma_req
> and pass this request down through rpcrdma_register_external() and
> rpcrdma_register_default_external(). This is less overhead then using
> kmalloc() and requires no extra error checking as the allocation burden is
> shifted to the transport client.
Oh good--so that works, and the req is the right place to put this? How
are you testing this?
(Just want to make it clear: I'm *not* an expert on the rdma code, so my
suggestion to put this in the rpcrdma_req was a suggestion for something
to look into, not a claim that it's correct.)
--b.
>
> net/sunrpc/xprtrdma/rpc_rdma.c | 2 +-
> net/sunrpc/xprtrdma/verbs.c | 11 ++++++-----
> net/sunrpc/xprtrdma/xprt_rdma.h | 3 ++-
> 3 files changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/net/sunrpc/xprtrdma/rpc_rdma.c b/net/sunrpc/xprtrdma/rpc_rdma.c
> index e03725b..c89448b 100644
> --- a/net/sunrpc/xprtrdma/rpc_rdma.c
> +++ b/net/sunrpc/xprtrdma/rpc_rdma.c
> @@ -203,7 +203,7 @@ rpcrdma_create_chunks(struct rpc_rqst *rqst, struct xdr_buf *target,
>
> do {
> /* bind/register the memory, then build chunk from result. */
> - int n = rpcrdma_register_external(seg, nsegs,
> + int n = rpcrdma_register_external(req, seg, nsegs,
> cur_wchunk != NULL, r_xprt);
> if (n <= 0)
> goto out;
> diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c
> index 93726560..5b439ed 100644
> --- a/net/sunrpc/xprtrdma/verbs.c
> +++ b/net/sunrpc/xprtrdma/verbs.c
> @@ -1730,13 +1730,14 @@ rpcrdma_deregister_memwin_external(struct rpcrdma_mr_seg *seg,
> }
>
> static int
> -rpcrdma_register_default_external(struct rpcrdma_mr_seg *seg,
> - int *nsegs, int writing, struct rpcrdma_ia *ia)
> +rpcrdma_register_default_external(struct rpcrdma_req *req,
> + struct rpcrdma_mr_seg *seg, int *nsegs, int writing,
> + struct rpcrdma_ia *ia)
> {
> int mem_priv = (writing ? IB_ACCESS_REMOTE_WRITE :
> IB_ACCESS_REMOTE_READ);
> struct rpcrdma_mr_seg *seg1 = seg;
> - struct ib_phys_buf ipb[RPCRDMA_MAX_DATA_SEGS];
> + struct ib_phys_buf *ipb = req->rl_ipb;
> int len, i, rc = 0;
>
> if (*nsegs > RPCRDMA_MAX_DATA_SEGS)
> @@ -1791,7 +1792,7 @@ rpcrdma_deregister_default_external(struct rpcrdma_mr_seg *seg,
> }
>
> int
> -rpcrdma_register_external(struct rpcrdma_mr_seg *seg,
> +rpcrdma_register_external(struct rpcrdma_req *req, struct rpcrdma_mr_seg *seg,
> int nsegs, int writing, struct rpcrdma_xprt *r_xprt)
> {
> struct rpcrdma_ia *ia = &r_xprt->rx_ia;
> @@ -1827,7 +1828,7 @@ rpcrdma_register_external(struct rpcrdma_mr_seg *seg,
>
> /* Default registration each time */
> default:
> - rc = rpcrdma_register_default_external(seg, &nsegs, writing, ia);
> + rc = rpcrdma_register_default_external(req, seg, &nsegs, writing, ia);
> break;
> }
> if (rc)
> diff --git a/net/sunrpc/xprtrdma/xprt_rdma.h b/net/sunrpc/xprtrdma/xprt_rdma.h
> index cc1445d..b10ed34 100644
> --- a/net/sunrpc/xprtrdma/xprt_rdma.h
> +++ b/net/sunrpc/xprtrdma/xprt_rdma.h
> @@ -192,6 +192,7 @@ struct rpcrdma_req {
> struct ib_sge rl_send_iov[4]; /* for active requests */
> struct ib_sge rl_iov; /* for posting */
> struct ib_mr *rl_handle; /* handle for mem in rl_iov */
> + struct ib_phys_buf rl_ipb[RPCRDMA_MAX_DATA_SEGS]; /* temp work array */
> char rl_base[MAX_RPCRDMAHDR]; /* start of actual buffer */
> __u32 rl_xdr_buf[0]; /* start of returned rpc rq_buffer */
> };
> @@ -327,7 +328,7 @@ int rpcrdma_register_internal(struct rpcrdma_ia *, void *, int,
> int rpcrdma_deregister_internal(struct rpcrdma_ia *,
> struct ib_mr *, struct ib_sge *);
>
> -int rpcrdma_register_external(struct rpcrdma_mr_seg *,
> +int rpcrdma_register_external(struct rpcrdma_req *, struct rpcrdma_mr_seg *,
> int, int, struct rpcrdma_xprt *);
> int rpcrdma_deregister_external(struct rpcrdma_mr_seg *,
> struct rpcrdma_xprt *, void *);
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] netconsole: release the spinlock before __netpoll_cleanup()
From: Neil Horman @ 2013-03-11 17:58 UTC (permalink / raw)
To: Veaceslav Falico; +Cc: Veaceslav Falico, David Miller, netdev, amwang
In-Reply-To: <20130311113914.GE1806@redhat.com>
On Mon, Mar 11, 2013 at 12:39:14PM +0100, Veaceslav Falico wrote:
> On Mon, Mar 11, 2013 at 07:30:24AM -0400, Neil Horman wrote:
> >On Mon, Mar 11, 2013 at 11:08:02AM +0100, Veaceslav Falico wrote:
> >>On Sun, Mar 10, 2013 at 4:25 PM, Veaceslav Falico <vfalico@redhat.com> wrote:
> >>> On Thu, Mar 07, 2013 at 04:14:38PM -0500, David Miller wrote:
> >>>>
> >>...snip...
> >>>> Quite a bit of email corruption of this patch.
> >>>
> >>>
> >>> Sorry, somehow messed it.
> >>>
> >>>
> >>>>
> >>>> Also, this code block is probably too deeply indented to be sane,
> >>>> consider creating a small helper function to call instead.
> >>>
> >>>
> >>> It gets quite ugly if I try to move it to another function. However, maybe
> >>> something like that will work - it's effectively the same code, just that
> >>> I've moved the long part out of the if () { } block. Looks a lot more
> >>> readable, though one line still breaks 80chars limit. I've reworked the
> >>> subject/commit message too.
> >>>
> >>> Subject: [PATCH] netconsole: don't call __netpoll_cleanup() while atomic
> >>>
> >>> __netpoll_cleanup() is called in netconsole_netdev_event() while holding a
> >>> spinlock. Release/acquire the spinlock before/after it and restart the
> >>>
> >>> loop.
> >>>
> >>> Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
> >>> ---
> >>> drivers/net/netconsole.c | 22 +++++++++++++++-------
> >>> 1 files changed, 15 insertions(+), 7 deletions(-)
> >>>
> >>> diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
> >>> index 37add21..38eaa8c 100644
> >>>
> >>> --- a/drivers/net/netconsole.c
> >>> +++ b/drivers/net/netconsole.c
> >>> @@ -666,6 +666,7 @@ static int netconsole_netdev_event(struct notifier_block
> >>> *this,
> >>> goto done;
> >>> spin_lock_irqsave(&target_list_lock, flags);
> >>> +restart:
> >>> list_for_each_entry(nt, &target_list, list) {
> >>> netconsole_target_get(nt);
> >>> if (nt->np.dev == dev) {
> >>> @@ -679,14 +680,21 @@ static int netconsole_netdev_event(struct
> >>> notifier_block *this,
> >>> /*
> >>>
> >>> * rtnl_lock already held
> >>> */
> >>> - if (nt->np.dev) {
> >>> - __netpoll_cleanup(&nt->np);
> >>> - dev_put(nt->np.dev);
> >>> - nt->np.dev = NULL;
> >>> + if (!nt->np.dev) {
> >>> + nt->enabled = 0;
> >>> + stopped = true;
> >>> + break;
> >>> }
> >>> - nt->enabled = 0;
> >>> - stopped = true;
> >>> - break;
> >>> + /*
> >>> + * we might sleep in __netpoll_cleanup()
> >>> + */
> >>> + spin_unlock_irqrestore(&target_list_lock,
> >>> flags);
> >>> + __netpoll_cleanup(&nt->np);
> >>> + spin_lock_irqsave(&target_list_lock, flags);
> >>> + dev_put(nt->np.dev);
> >>> + nt->np.dev = NULL;
> >>>
> >>> + netconsole_target_put(nt);
> >>> + goto restart;
> >>> }
> >>> }
> >>> netconsole_target_put(nt);
> >>> --
> >>> 1.7.1
> >>
> >>Self-NAK this patch, I've triggered another kernel panic with it. Will
> >>send another one shortly. Basicly, the whole if (!nt->np.dev) is not
> >>needed and nt->enabled=0 should always be set, otherwise we
> >>end up with nt->np.dev == NULL and nt->enabled == 1, thus
> >>triggering panics in places like write_msg(), where it verifies only
> >>if the nt->enabled is true.
> >>
> >Yup, I think you want to make the nt->enabled and stopped statements
> >unconditional, and precedede the whole block with a if(!nt->np.dev) { continue;}
> >statement.
>
> I don't see why the statement is needed at all, if we verify it beforehand:
>
> 669 list_for_each_entry(nt, &target_list, list) {
> 670 netconsole_target_get(nt);
> 671 if (nt->np.dev == dev) {
>
> so that effectively it can be null only in case the dev, delivered via
> netconsole_netdev_event() is null, which is a bug by itself. Or am I
> missing something?
>
Honestly, I'm not sure. After I sent this, I took a look at the history to
refresh my memory, and I think your right, its not needed.
Neil
> >Neil
> >
> >>--
> >>Best regards,
> >>Veaceslav Falico
> >>
>
^ permalink raw reply
* Re: [PATCH] 8139too: send NETDEV_CHANGE manually when autoneg is disabled
From: Veaceslav Falico @ 2013-03-11 17:58 UTC (permalink / raw)
To: Ben Hutchings
Cc: David Miller, netdev, wfp5p, jasowang, junchangwang, greearb,
ivecera
In-Reply-To: <1362683433.2936.41.camel@bwh-desktop.uk.solarflarecom.com>
On Thu, Mar 07, 2013 at 07:10:33PM +0000, Ben Hutchings wrote:
>On Thu, 2013-03-07 at 19:38 +0100, Veaceslav Falico wrote:
>> On Thu, Mar 07, 2013 at 04:54:21PM +0000, Ben Hutchings wrote:
>> >On Thu, 2013-03-07 at 17:35 +0100, Veaceslav Falico wrote:
>> >> On Thu, Mar 07, 2013 at 03:52:35PM +0000, Ben Hutchings wrote:
>> >> >On Thu, 2013-03-07 at 11:27 +0100, Veaceslav Falico wrote:
>[...]
>> >> >> Silently changing speed can break things a bit - bonding relies on
>> >> >> interface speeds for 802.3ad/alb/tlb/active-backup iirc, bridge relies on
>> >> >> stp port cost etc. and they all get it via NETDEV_ notifications. So
>> >> >> without them, they would end up with outdated data, per example (eth2 being
>> >> >> 8139too):
>> >> >[...]
>> >> >
>> >> >Yes, I get it. But on real hardware, changing speed/duplex is always
>> >> >going to break the link if it's up. The link change notification should
>> >> >result in kernel and userland notifications via linkwatch_do_dev().
>> >> >
>> >> >(If you're testing against QEMU rather than real hardware, there could
>> >> >be a bug in the emulation of link interrupts.)
>> >>
>> >> It's real hardware:
>> >>
>> >> [ 4.339413] 8139cp 0000:10:09.0: This (id 10ec:8139 rev 10) is not an 8139C+ compatible chip, use 8139too
>> >> [ 4.348210] 8139too: 8139too Fast Ethernet driver 0.9.28
>> >> [ 4.350017] 8139too 0000:10:09.0 eth2: RealTek RTL8139 at 0xffffc90004574100, 00:14:d1:1f:b9:49, IRQ 21
>> >[...]
>> >
>> >OK. But it's generally not enough to send a 'something changed'
>> >notification; the driver actually has to update the link state. MAybe
>> >in your test you reconfigure the other end of the link too, or it's
>> >smart enough to do parallel detect. But in general, changing the link
>> >speed is going to change the link state, and the TX scheduler needs to
>> >enabled or disabled accordingly.
>>
>> Sorry, I think I didn't explain it clearly. The driver *does* notify
>> about link changes even when it has autonegotiation disabled.
>>
>> The only thing that doesn't work is the notification when someone changes
>> it via ethtool. In the same moment.
>
>The link *should* go down (assuming it was up before) because you likely
>have a speed mismatch and won't be able to pass traffic. I don't know
>whether this is possible for the hardware to detect immediately; maybe a
>PHY reset will ensure that it is detected.
Sorry for late response - I've had a feeling that I was writing something
utterly dumb. My feeling was correct. I've looked through it and came up
with the next patch, though I have a feeling... :)
Anyway, you were right - the hardware does everything needed, and the
driver also. The normal way (with autoneg on) would be for the driver to
call mii_check_media() when it got the interrupt, which would verify if
state/duplex has changed and report it.
However, with autoneg off, and thus mii->force_duplex == 1,
mii_check_media() just returns 'nothing changed' back, without doing
anything else. This way, any media change while in autoneg off (like
up/down notifications) are completely hidden to the rest of the kernel.
I've modified the mii_check_media() to not verify for ->force_media and to
get all the media settings from mii_ethtool_gset(), which would take care
of whether autoneg is on or off and return the correct values. Tested,
works ok - it sees when the cable is dis/connected, notifies bonding of
speed/duplex changes, when enslaved.
Subject: [PATCH] mii: make mii_check_media() work with ->force_media == 1
mii_check_media() does nothing when ->force_media is set, and thus might
miss media change events reported by the drivers if the autonegotiation is
off.
Make mii_check_media() use mii_ethtool_gset() for getting media settings,
which cleans the code a lot and works with both autoneg off and on. This
allows us to catch link notifications even ->force_media on.
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
drivers/net/mii.c | 45 ++++++++++++---------------------------------
1 files changed, 12 insertions(+), 33 deletions(-)
diff --git a/drivers/net/mii.c b/drivers/net/mii.c
index 4a99c39..2447487 100644
--- a/drivers/net/mii.c
+++ b/drivers/net/mii.c
@@ -308,19 +308,14 @@ void mii_check_link (struct mii_if_info *mii)
* @init_media: OK to save duplex mode in @mii
*
* Returns 1 if the duplex mode changed, 0 if not.
- * If the media type is forced, always returns 0.
*/
unsigned int mii_check_media (struct mii_if_info *mii,
unsigned int ok_to_print,
unsigned int init_media)
{
unsigned int old_carrier, new_carrier;
- int advertise, lpa, media, duplex;
- int lpa2 = 0;
-
- /* if forced media, go no further */
- if (mii->force_media)
- return 0; /* duplex did not change */
+ int duplex;
+ struct ethtool_cmd ecmd = { .cmd = ETHTOOL_GSET };
/* check current and old link status */
old_carrier = netif_carrier_ok(mii->dev) ? 1 : 0;
@@ -345,37 +340,21 @@ unsigned int mii_check_media (struct mii_if_info *mii,
*/
netif_carrier_on(mii->dev);
- /* get MII advertise and LPA values */
- if ((!init_media) && (mii->advertising))
- advertise = mii->advertising;
- else {
- advertise = mii->mdio_read(mii->dev, mii->phy_id, MII_ADVERTISE);
- mii->advertising = advertise;
- }
- lpa = mii->mdio_read(mii->dev, mii->phy_id, MII_LPA);
- if (mii->supports_gmii)
- lpa2 = mii->mdio_read(mii->dev, mii->phy_id, MII_STAT1000);
+ /*
+ * save the previous state of the duplex, mii_ethtool_gset()
+ * modifies it
+ */
+ duplex = mii->full_duplex;
- /* figure out media and duplex from advertise and LPA values */
- media = mii_nway_result(lpa & advertise);
- duplex = (media & ADVERTISE_FULL) ? 1 : 0;
- if (lpa2 & LPA_1000FULL)
- duplex = 1;
+ mii_ethtool_gset(mii, &ecmd);
if (ok_to_print)
netdev_info(mii->dev, "link up, %uMbps, %s-duplex, lpa 0x%04X\n",
- lpa2 & (LPA_1000FULL | LPA_1000HALF) ? 1000 :
- media & (ADVERTISE_100FULL | ADVERTISE_100HALF) ?
- 100 : 10,
- duplex ? "full" : "half",
- lpa);
-
- if ((init_media) || (mii->full_duplex != duplex)) {
- mii->full_duplex = duplex;
- return 1; /* duplex changed */
- }
+ ethtool_cmd_speed(&ecmd),
+ ecmd.duplex == DUPLEX_FULL ? "full" : "half",
+ ecmd.lp_advertising);
- return 0; /* duplex did not change */
+ return (init_media || (mii->full_duplex != duplex)) ? 1 : 0;
}
/**
--
1.7.1
>
>[...]
>> Other drivers, after setting autoneg off, usually ifdown/ifup the interface
>> manually, and thus everything gets updated. That is another way to fix this
>> driver - to issue ->ndo_stop(); ->ndo_open(); (as netxen_nic/qlcnic/etc
>> do), but I think that just issuing a notification that we've changed the
>> state, without flapping, is better.
>
>The link *should* flap, though doing a full stop/start is likely
>unnecessary.
>
>Ben.
>
>--
>Ben Hutchings, Staff Engineer, Solarflare
>Not speaking for my employer; that's the marketing department's job.
>They asked us to note that Solarflare product names are trademarked.
>
^ permalink raw reply related
* [PATCH linux-next v2] SUNRPC: rpcrdma_register_default_external: Dynamically allocate ib_phys_buf
From: Tim Gardner @ 2013-03-11 17:37 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Tim Gardner, Trond Myklebust, J. Bruce Fields, David S. Miller,
Tom Tucker, Haggai Eran, Or Gerlitz, Shani Michaeli,
linux-nfs-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20130310202838.GL31448-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
rpcrdma_register_default_external() is several frames into the call stack which
goes deeper yet. You run the risk of stack corruption by declaring such a large
automatic variable, so move the array of 'struct ib_phys_buf' objects into the
requestor structure 'struct rpcrdma_req' (which is dynamically allocated) in
order to silence the frame-larger-than warning.
net/sunrpc/xprtrdma/verbs.c: In function 'rpcrdma_register_default_external':
net/sunrpc/xprtrdma/verbs.c:1774:1: warning: the frame size of 1056 bytes is larger than 1024 bytes [-Wframe-larger-than=]
gcc version 4.6.3
Cc: Trond Myklebust <Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org>
Cc: "J. Bruce Fields" <bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
Cc: "David S. Miller" <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
Cc: Tom Tucker <tom-/Yg/VP3ZvrM@public.gmane.org>
Cc: Haggai Eran <haggaie-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Cc: Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Cc: Shani Michaeli <shanim-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Cc: linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Signed-off-by: Tim Gardner <tim.gardner-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
---
v1 - Use kmalloc() to dynamically allocate and free the array of 'struct
ib_phys_buf' objects
v2 - Move the array of 'struct ib_phys_buf' objects into struct rpcrdma_req
and pass this request down through rpcrdma_register_external() and
rpcrdma_register_default_external(). This is less overhead then using
kmalloc() and requires no extra error checking as the allocation burden is
shifted to the transport client.
net/sunrpc/xprtrdma/rpc_rdma.c | 2 +-
net/sunrpc/xprtrdma/verbs.c | 11 ++++++-----
net/sunrpc/xprtrdma/xprt_rdma.h | 3 ++-
3 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/net/sunrpc/xprtrdma/rpc_rdma.c b/net/sunrpc/xprtrdma/rpc_rdma.c
index e03725b..c89448b 100644
--- a/net/sunrpc/xprtrdma/rpc_rdma.c
+++ b/net/sunrpc/xprtrdma/rpc_rdma.c
@@ -203,7 +203,7 @@ rpcrdma_create_chunks(struct rpc_rqst *rqst, struct xdr_buf *target,
do {
/* bind/register the memory, then build chunk from result. */
- int n = rpcrdma_register_external(seg, nsegs,
+ int n = rpcrdma_register_external(req, seg, nsegs,
cur_wchunk != NULL, r_xprt);
if (n <= 0)
goto out;
diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c
index 93726560..5b439ed 100644
--- a/net/sunrpc/xprtrdma/verbs.c
+++ b/net/sunrpc/xprtrdma/verbs.c
@@ -1730,13 +1730,14 @@ rpcrdma_deregister_memwin_external(struct rpcrdma_mr_seg *seg,
}
static int
-rpcrdma_register_default_external(struct rpcrdma_mr_seg *seg,
- int *nsegs, int writing, struct rpcrdma_ia *ia)
+rpcrdma_register_default_external(struct rpcrdma_req *req,
+ struct rpcrdma_mr_seg *seg, int *nsegs, int writing,
+ struct rpcrdma_ia *ia)
{
int mem_priv = (writing ? IB_ACCESS_REMOTE_WRITE :
IB_ACCESS_REMOTE_READ);
struct rpcrdma_mr_seg *seg1 = seg;
- struct ib_phys_buf ipb[RPCRDMA_MAX_DATA_SEGS];
+ struct ib_phys_buf *ipb = req->rl_ipb;
int len, i, rc = 0;
if (*nsegs > RPCRDMA_MAX_DATA_SEGS)
@@ -1791,7 +1792,7 @@ rpcrdma_deregister_default_external(struct rpcrdma_mr_seg *seg,
}
int
-rpcrdma_register_external(struct rpcrdma_mr_seg *seg,
+rpcrdma_register_external(struct rpcrdma_req *req, struct rpcrdma_mr_seg *seg,
int nsegs, int writing, struct rpcrdma_xprt *r_xprt)
{
struct rpcrdma_ia *ia = &r_xprt->rx_ia;
@@ -1827,7 +1828,7 @@ rpcrdma_register_external(struct rpcrdma_mr_seg *seg,
/* Default registration each time */
default:
- rc = rpcrdma_register_default_external(seg, &nsegs, writing, ia);
+ rc = rpcrdma_register_default_external(req, seg, &nsegs, writing, ia);
break;
}
if (rc)
diff --git a/net/sunrpc/xprtrdma/xprt_rdma.h b/net/sunrpc/xprtrdma/xprt_rdma.h
index cc1445d..b10ed34 100644
--- a/net/sunrpc/xprtrdma/xprt_rdma.h
+++ b/net/sunrpc/xprtrdma/xprt_rdma.h
@@ -192,6 +192,7 @@ struct rpcrdma_req {
struct ib_sge rl_send_iov[4]; /* for active requests */
struct ib_sge rl_iov; /* for posting */
struct ib_mr *rl_handle; /* handle for mem in rl_iov */
+ struct ib_phys_buf rl_ipb[RPCRDMA_MAX_DATA_SEGS]; /* temp work array */
char rl_base[MAX_RPCRDMAHDR]; /* start of actual buffer */
__u32 rl_xdr_buf[0]; /* start of returned rpc rq_buffer */
};
@@ -327,7 +328,7 @@ int rpcrdma_register_internal(struct rpcrdma_ia *, void *, int,
int rpcrdma_deregister_internal(struct rpcrdma_ia *,
struct ib_mr *, struct ib_sge *);
-int rpcrdma_register_external(struct rpcrdma_mr_seg *,
+int rpcrdma_register_external(struct rpcrdma_req *, struct rpcrdma_mr_seg *,
int, int, struct rpcrdma_xprt *);
int rpcrdma_deregister_external(struct rpcrdma_mr_seg *,
struct rpcrdma_xprt *, void *);
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 1/2] fec: Remove unused pci header
From: Fabio Estevam @ 2013-03-11 17:32 UTC (permalink / raw)
To: davem; +Cc: s.hauer, netdev, Fabio Estevam
PCI header is not needed, so get rid of it.
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
drivers/net/ethernet/freescale/fec.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/ethernet/freescale/fec.c b/drivers/net/ethernet/freescale/fec.c
index 069a155..89c27e6 100644
--- a/drivers/net/ethernet/freescale/fec.c
+++ b/drivers/net/ethernet/freescale/fec.c
@@ -29,7 +29,6 @@
#include <linux/ioport.h>
#include <linux/slab.h>
#include <linux/interrupt.h>
-#include <linux/pci.h>
#include <linux/init.h>
#include <linux/delay.h>
#include <linux/netdevice.h>
--
1.7.9.5
^ permalink raw reply related
* [PATCH 2/2] fec: Use devm_request_and_ioremap()
From: Fabio Estevam @ 2013-03-11 17:32 UTC (permalink / raw)
To: davem; +Cc: s.hauer, netdev, Fabio Estevam
In-Reply-To: <1363023175-17597-1-git-send-email-fabio.estevam@freescale.com>
Using devm_request_and_ioremap() can make the code cleaner and simpler.
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
drivers/net/ethernet/freescale/fec.c | 21 +++------------------
1 file changed, 3 insertions(+), 18 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec.c b/drivers/net/ethernet/freescale/fec.c
index 89c27e6..559b3d9 100644
--- a/drivers/net/ethernet/freescale/fec.c
+++ b/drivers/net/ethernet/freescale/fec.c
@@ -1733,16 +1733,10 @@ fec_probe(struct platform_device *pdev)
if (!r)
return -ENXIO;
- r = request_mem_region(r->start, resource_size(r), pdev->name);
- if (!r)
- return -EBUSY;
-
/* Init network device */
ndev = alloc_etherdev(sizeof(struct fec_enet_private));
- if (!ndev) {
- ret = -ENOMEM;
- goto failed_alloc_etherdev;
- }
+ if (!ndev)
+ return -ENOMEM;
SET_NETDEV_DEV(ndev, &pdev->dev);
@@ -1754,7 +1748,7 @@ fec_probe(struct platform_device *pdev)
(pdev->id_entry->driver_data & FEC_QUIRK_HAS_GBIT))
fep->pause_flag |= FEC_PAUSE_FLAG_AUTONEG;
- fep->hwp = ioremap(r->start, resource_size(r));
+ fep->hwp = devm_request_and_ioremap(&pdev->dev, r);
fep->pdev = pdev;
fep->dev_id = dev_id++;
@@ -1876,11 +1870,8 @@ failed_regulator:
clk_disable_unprepare(fep->clk_ptp);
failed_pin:
failed_clk:
- iounmap(fep->hwp);
failed_ioremap:
free_netdev(ndev);
-failed_alloc_etherdev:
- release_mem_region(r->start, resource_size(r));
return ret;
}
@@ -1890,7 +1881,6 @@ fec_drv_remove(struct platform_device *pdev)
{
struct net_device *ndev = platform_get_drvdata(pdev);
struct fec_enet_private *fep = netdev_priv(ndev);
- struct resource *r;
int i;
unregister_netdev(ndev);
@@ -1906,13 +1896,8 @@ fec_drv_remove(struct platform_device *pdev)
if (irq > 0)
free_irq(irq, ndev);
}
- iounmap(fep->hwp);
free_netdev(ndev);
- r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- BUG_ON(!r);
- release_mem_region(r->start, resource_size(r));
-
platform_set_drvdata(pdev, NULL);
return 0;
--
1.7.9.5
^ permalink raw reply related
* [PATCH net-next 1/2] gianfar: Remove superfluous gfar_new_skb()
From: Claudiu Manoil @ 2013-03-11 17:10 UTC (permalink / raw)
To: netdev; +Cc: Paul Gortmaker, David S. Miller
gfar_new_skb() does nothing more than to call the static
gfar_alloc_skb() routine.
Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
---
drivers/net/ethernet/freescale/gianfar.c | 11 +++--------
1 files changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index d2c5441..af91196 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -110,7 +110,7 @@ static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev);
static void gfar_reset_task(struct work_struct *work);
static void gfar_timeout(struct net_device *dev);
static int gfar_close(struct net_device *dev);
-struct sk_buff *gfar_new_skb(struct net_device *dev);
+static struct sk_buff *gfar_alloc_skb(struct net_device *dev);
static void gfar_new_rxbdp(struct gfar_priv_rx_q *rx_queue, struct rxbd8 *bdp,
struct sk_buff *skb);
static int gfar_set_mac_address(struct net_device *dev);
@@ -207,7 +207,7 @@ static int gfar_init_bds(struct net_device *ndev)
gfar_init_rxbdp(rx_queue, rxbdp,
rxbdp->bufPtr);
} else {
- skb = gfar_new_skb(ndev);
+ skb = gfar_alloc_skb(ndev);
if (!skb) {
netdev_err(ndev, "Can't allocate RX buffers\n");
return -ENOMEM;
@@ -2626,11 +2626,6 @@ static struct sk_buff *gfar_alloc_skb(struct net_device *dev)
return skb;
}
-struct sk_buff *gfar_new_skb(struct net_device *dev)
-{
- return gfar_alloc_skb(dev);
-}
-
static inline void count_errors(unsigned short status, struct net_device *dev)
{
struct gfar_private *priv = netdev_priv(dev);
@@ -2766,7 +2761,7 @@ int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue, int rx_work_limit)
rmb();
/* Add another skb for the future */
- newskb = gfar_new_skb(dev);
+ newskb = gfar_alloc_skb(dev);
skb = rx_queue->rx_skbuff[rx_queue->skb_currx];
--
1.7.7.4
^ 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