login
Search
This series adds Ethernet-level filtering and explicit filter clearing to the ethtool RX n-tuple interface, and implements it in the sfc driver. There is a cleanup patch on the end which is preparation for the following RFC patch series but is worthwhile anyway. Ben. Ben Hutchings (8): ethtool: Define RX n-tuple action to clear a rule ethtool: Add Ethernet MAC-level filtering/steering ethtool: Allocate register dump buffer with vmalloc() sfc: Add filter table management sfc: Implement the ethtool RX n-tuple control functions sfc: Include RX IP filter table in register dump sfc: Set net_device::num_rx_queues once we know the correct value sfc: Clean up and correct comments on efx_monitor() drivers/net/sfc/Makefile | 2 +- drivers/net/sfc/efx.c | 35 ++-- drivers/net/sfc/efx.h | 14 ++ drivers/net/sfc/ethtool.c | 118 +++++++++++- drivers/net/sfc/falcon.c | 2 +- drivers/net/sfc/filter.c | 445 ++++++++++++++++++++++++++++++++++++++++++ drivers/net/sfc/filter.h | 189 ++++++++++++++++++ drivers/net/sfc/net_driver.h | 4 + drivers/net/sfc/nic.c | 5 +- drivers/net/sfc/regs.h | 14 ++ drivers/net/sfc/siena.c | 2 +- include/linux/ethtool.h | 11 +- net/core/ethtool.c | 4 +- 13 files changed, 819 insertions(+), 26 deletions(-) create mode 100644 drivers/net/sfc/filter.c create mode 100644 drivers/net/sfc/filter.h -- 1.7.2.1 -- 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. --
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
include/linux/ethtool.h | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index b67af60..3350870 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -492,11 +492,12 @@ struct ethtool_rx_ntuple_flow_spec {
__u64 data_mask;
__s32 action;
-#define ETHTOOL_RXNTUPLE_ACTION_DROP -1 /* drop packet */
+#define ETHTOOL_RXNTUPLE_ACTION_DROP (-1) /* drop packet */
+#define ETHTOOL_RXNTUPLE_ACTION_CLEAR (-2) /* clear filter */
};
/**
- * struct ethtool_rx_ntuple - command to set RX flow filter
+ * struct ethtool_rx_ntuple - command to set or clear RX flow filter
* @cmd: Command number - %ETHTOOL_SRXNTUPLE
* @fs: Flow filter specification
*/
--
1.7.2.1
--
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.
--
Some NICs have huge register files which exceed the maximum heap allocation size. Signed-off-by: Ben Hutchings <bhutchings@solarflare.com> --- net/core/ethtool.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/net/core/ethtool.c b/net/core/ethtool.c index 91ffce2..dae2fd0 100644 --- a/net/core/ethtool.c +++ b/net/core/ethtool.c @@ -815,7 +815,7 @@ static int ethtool_get_regs(struct net_device *dev, char __user *useraddr) if (regs.len > reglen) regs.len = reglen; - regbuf = kmalloc(reglen, GFP_USER); + regbuf = vmalloc(reglen); if (!regbuf) return -ENOMEM; @@ -830,7 +830,7 @@ static int ethtool_get_regs(struct net_device *dev, char __user *useraddr) ret = 0; out: - kfree(regbuf); + vfree(regbuf); return ret; } -- 1.7.2.1 -- 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. --
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
include/linux/ethtool.h | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 3350870..8a3338c 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -14,6 +14,7 @@
#define _LINUX_ETHTOOL_H
#include <linux/types.h>
+#include <linux/if_ether.h>
/* This should work for both 32 and 64 bit userland. */
struct ethtool_cmd {
@@ -391,6 +392,7 @@ struct ethtool_rx_flow_spec {
struct ethtool_ah_espip4_spec ah_ip4_spec;
struct ethtool_ah_espip4_spec esp_ip4_spec;
struct ethtool_usrip4_spec usr_ip4_spec;
+ struct ethhdr ether_spec;
__u8 hdata[72];
} h_u, m_u;
__u64 ring_cookie;
@@ -483,6 +485,7 @@ struct ethtool_rx_ntuple_flow_spec {
struct ethtool_ah_espip4_spec ah_ip4_spec;
struct ethtool_ah_espip4_spec esp_ip4_spec;
struct ethtool_usrip4_spec usr_ip4_spec;
+ struct ethhdr ether_spec;
__u8 hdata[72];
} h_u, m_u;
@@ -841,7 +844,7 @@ struct ethtool_ops {
#define WAKE_MAGIC (1 << 5)
#define WAKE_MAGICSECURE (1 << 6) /* only meaningful if WAKE_MAGIC */
-/* L3-L4 network traffic flow types */
+/* L2-L4 network traffic flow types */
#define TCP_V4_FLOW 0x01 /* hash or spec (tcp_ip4_spec) */
#define UDP_V4_FLOW 0x02 /* hash or spec (udp_ip4_spec) */
#define SCTP_V4_FLOW 0x03 /* hash or spec (sctp_ip4_spec) */
@@ -857,6 +860,7 @@ struct ethtool_ops {
#define IP_USER_FLOW 0x0d /* spec only (usr_ip4_spec) */
#define IPV4_FLOW 0x10 /* hash only */
#define IPV6_FLOW 0x11 /* hash only */
+#define ETHER_FLOW 0x12 /* spec only (ether_spec) */
/* L3-L4 network traffic flow hash options */
#define RXH_L2DA (1 << 1)
--
1.7.2.1
--
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.
--
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/sfc/Makefile | 2 +-
drivers/net/sfc/efx.c | 11 +
drivers/net/sfc/efx.h | 14 ++
drivers/net/sfc/filter.c | 445 ++++++++++++++++++++++++++++++++++++++++++
drivers/net/sfc/filter.h | 189 ++++++++++++++++++
drivers/net/sfc/net_driver.h | 4 +
drivers/net/sfc/regs.h | 14 ++
7 files changed, 678 insertions(+), 1 deletions(-)
create mode 100644 drivers/net/sfc/filter.c
create mode 100644 drivers/net/sfc/filter.h
diff --git a/drivers/net/sfc/Makefile b/drivers/net/sfc/Makefile
index 1047b19..fd9272b 100644
--- a/drivers/net/sfc/Makefile
+++ b/drivers/net/sfc/Makefile
@@ -1,4 +1,4 @@
-sfc-y += efx.o nic.o falcon.o siena.o tx.o rx.o \
+sfc-y += efx.o nic.o falcon.o siena.o tx.o rx.o filter.o \
falcon_gmac.o falcon_xmac.o mcdi_mac.o \
selftest.o ethtool.o qt202x_phy.o mdio_10g.o \
tenxpress.o falcon_boards.o mcdi.o mcdi_phy.o
diff --git a/drivers/net/sfc/efx.c b/drivers/net/sfc/efx.c
index f702f1f..4a1c93f 100644
--- a/drivers/net/sfc/efx.c
+++ b/drivers/net/sfc/efx.c
@@ -1357,8 +1357,17 @@ static int efx_probe_all(struct efx_nic *efx)
if (rc)
goto fail3;
+ rc = efx_probe_filters(efx);
+ if (rc) {
+ netif_err(efx, probe, efx->net_dev,
+ "failed to create filter tables\n");
+ goto fail4;
+ }
+
return 0;
+ fail4:
+ efx_remove_channels(efx);
fail3:
efx_remove_port(efx);
fail2:
@@ -1489,6 +1498,7 @@ static void efx_stop_all(struct efx_nic *efx)
static void efx_remove_all(struct efx_nic *efx)
{
+ efx_remove_filters(efx);
efx_remove_channels(efx);
efx_remove_port(efx);
efx_remove_nic(efx);
@@ -2002,6 +2012,7 @@ int efx_reset_up(struct efx_nic *efx, enum reset_type method, bool ok)
efx->mac_op->reconfigure(efx);
efx_init_channels(efx);
+ efx_restore_filters(efx);
mutex_unlock(&efx->spi_lock);
mutex_unlock(&efx->mac_lock);
diff --git a/drivers/net/sfc/efx.h ...Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/sfc/ethtool.c | 118 ++++++++++++++++++++++++++++++++++++++++++++-
drivers/net/sfc/falcon.c | 2 +-
drivers/net/sfc/siena.c | 2 +-
3 files changed, 118 insertions(+), 4 deletions(-)
diff --git a/drivers/net/sfc/ethtool.c b/drivers/net/sfc/ethtool.c
index 7f735d8..1b2570c 100644
--- a/drivers/net/sfc/ethtool.c
+++ b/drivers/net/sfc/ethtool.c
@@ -15,6 +15,7 @@
#include "workarounds.h"
#include "selftest.h"
#include "efx.h"
+#include "filter.h"
#include "nic.h"
#include "spi.h"
#include "mdio_10g.h"
@@ -551,9 +552,22 @@ static u32 efx_ethtool_get_rx_csum(struct net_device *net_dev)
static int efx_ethtool_set_flags(struct net_device *net_dev, u32 data)
{
struct efx_nic *efx = netdev_priv(net_dev);
- u32 supported = efx->type->offload_features & ETH_FLAG_RXHASH;
+ u32 supported = (efx->type->offload_features &
+ (ETH_FLAG_RXHASH | ETH_FLAG_NTUPLE));
+ int rc;
+
+ rc = ethtool_op_set_flags(net_dev, data, supported);
+ if (rc)
+ return rc;
+
+ if (!(data & ETH_FLAG_NTUPLE)) {
+ efx_filter_table_clear(efx, EFX_FILTER_TABLE_RX_IP,
+ EFX_FILTER_PRI_MANUAL);
+ efx_filter_table_clear(efx, EFX_FILTER_TABLE_RX_MAC,
+ EFX_FILTER_PRI_MANUAL);
+ }
- return ethtool_op_set_flags(net_dev, data, supported);
+ return 0;
}
static void efx_ethtool_self_test(struct net_device *net_dev,
@@ -955,6 +969,105 @@ efx_ethtool_get_rxnfc(struct net_device *net_dev,
}
}
+static int efx_ethtool_set_rx_ntuple(struct net_device *net_dev,
+ struct ethtool_rx_ntuple *ntuple)
+{
+ struct efx_nic *efx = netdev_priv(net_dev);
+ struct ethtool_tcpip4_spec *ip_entry = &ntuple->fs.h_u.tcp_ip4_spec;
+ struct ethtool_tcpip4_spec *ip_mask = &ntuple->fs.m_u.tcp_ip4_spec;
+ struct ethhdr *mac_entry = &ntuple->fs.h_u.ether_spec;
+ struct ethhdr *mac_mask = &ntuple->fs.m_u.ether_spec;
+ struct efx_filter_spec filter;
+
+ /* Range-check action */
+ if ...For backward compatibility, add it at the end.
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/sfc/nic.c | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/net/sfc/nic.c b/drivers/net/sfc/nic.c
index 6c5c0ce..c4de001 100644
--- a/drivers/net/sfc/nic.c
+++ b/drivers/net/sfc/nic.c
@@ -1849,8 +1849,7 @@ static const struct efx_nic_reg_table efx_nic_reg_tables[] = {
REGISTER_TABLE_BB_CZ(TX_DESC_PTR_TBL),
REGISTER_TABLE_AA(EVQ_PTR_TBL_KER),
REGISTER_TABLE_BB_CZ(EVQ_PTR_TBL),
- /* The register buffer is allocated with slab, so we can't
- * reasonably read all of the buffer table (up to 8MB!).
+ /* We can't reasonably read all of the buffer table (up to 8MB!).
* However this driver will only use a few entries. Reading
* 1K entries allows for some expansion of queue count and
* size before we need to change the version. */
@@ -1858,7 +1857,6 @@ static const struct efx_nic_reg_table efx_nic_reg_tables[] = {
A, A, 8, 1024),
REGISTER_TABLE_DIMENSIONS(BUF_FULL_TBL, FR_BZ_BUF_FULL_TBL,
B, Z, 8, 1024),
- /* RX_FILTER_TBL{0,1} is huge and not used by this driver */
REGISTER_TABLE_CZ(RX_MAC_FILTER_TBL0),
REGISTER_TABLE_BB_CZ(TIMER_TBL),
REGISTER_TABLE_BB_CZ(TX_PACE_TBL),
@@ -1868,6 +1866,7 @@ static const struct efx_nic_reg_table efx_nic_reg_tables[] = {
REGISTER_TABLE_CZ(MC_TREG_SMEM),
/* MSIX_PBA_TABLE is not mapped */
/* SRM_DBG is not mapped (and is redundant with BUF_FLL_TBL) */
+ REGISTER_TABLE_BZ(RX_FILTER_TBL0),
};
size_t efx_nic_get_regs_len(struct efx_nic *efx)
--
1.7.2.1
--
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.
--
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com> --- drivers/net/sfc/efx.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/net/sfc/efx.c b/drivers/net/sfc/efx.c index 4a1c93f..48e2bf4 100644 --- a/drivers/net/sfc/efx.c +++ b/drivers/net/sfc/efx.c @@ -1315,6 +1315,7 @@ static int efx_probe_nic(struct efx_nic *efx) efx_set_channels(efx); efx->net_dev->real_num_tx_queues = efx->n_tx_channels; + efx->net_dev->num_rx_queues = efx->n_rx_channels; /* Initialise the interrupt moderation settings */ efx_init_irq_moderation(efx, tx_irq_mod_usec, rx_irq_mod_usec, true); -- 1.7.2.1 -- 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. --
Interesting, but why ? No other driver does this AFAIK... --
If RPS is enabled there's a separate kobject for each RX queue. Those other drivers probably should be setting it. Oh, but this only exists if CONFIG_RPS is enabled. I think we need an inline function for setting this. Ben. -- 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. --
From: Ben Hutchings <bhutchings@solarflare.com> It's set in the core by alloc_netdev_mq(), you should never have to set this in your driver. And that also takes care of the CONFIG_RPS dependency in one spot, another good argument for drivers never touching this value. I'm not applying this patch. --
That specifies the maximum possible number of queues, but we don't require that the actual number of TX queues (real_num_tx_queues) is the same as the maximum (num_tx_queues) and nor should we assume that of RX queues. I don't think we should require that the maximum numbers of RX Right, but we do need to have some way for drivers to specify the actual number of RX queues. Ben. -- 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. --
From: Ben Hutchings <bhutchings@solarflare.com> Ok, the problem stems merely from the fact that we only specify one "queue count" in alloc_netdev_mq(). We should specify two, one for TX and one for RX. So why not fix that instead of putting hacks into the drivers? :-) --
That still doesn't solve the original problem, since drivers generally won't know how many RX queues they should (or can) create until some time after calling alloc_netdev_mq(). You know very well why we distinguish real_num_tx_queues and num_tx_queues; what's so different about RX queues? Ben. -- 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. --
rx queues were added by Tom for RPS. No function prototypes were changed to provide it as a new parameter. What we need is to extend alloc_netdev_mq() to take a new rx_queue_count parameter. Then allow these txq/rxq parameters to be "unset for the moment", and provide a new function to allocate / populate queues when driver knows what to do, _after_ its alloc_netdev_mq() call. Your one line patch is a work around, not a clean solution. --
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/sfc/efx.c | 23 ++++++++++-------------
1 files changed, 10 insertions(+), 13 deletions(-)
diff --git a/drivers/net/sfc/efx.c b/drivers/net/sfc/efx.c
index 48e2bf4..8a51c41 100644
--- a/drivers/net/sfc/efx.c
+++ b/drivers/net/sfc/efx.c
@@ -124,8 +124,9 @@ MODULE_PARM_DESC(separate_tx_channels,
static int napi_weight = 64;
/* This is the time (in jiffies) between invocations of the hardware
- * monitor, which checks for known hardware bugs and resets the
- * hardware and driver as necessary.
+ * monitor. On Falcon-based NICs, this will:
+ * - Check the on-board hardware monitor;
+ * - Poll the link state and reconfigure the hardware as necessary.
*/
unsigned int efx_monitor_interval = 1 * HZ;
@@ -1546,8 +1547,7 @@ void efx_init_irq_moderation(struct efx_nic *efx, int tx_usecs, int rx_usecs,
*
**************************************************************************/
-/* Run periodically off the general workqueue. Serialised against
- * efx_reconfigure_port via the mac_lock */
+/* Run periodically off the general workqueue */
static void efx_monitor(struct work_struct *data)
{
struct efx_nic *efx = container_of(data, struct efx_nic,
@@ -1560,16 +1560,13 @@ static void efx_monitor(struct work_struct *data)
/* If the mac_lock is already held then it is likely a port
* reconfiguration is already in place, which will likely do
- * most of the work of check_hw() anyway. */
- if (!mutex_trylock(&efx->mac_lock))
- goto out_requeue;
- if (!efx->port_enabled)
- goto out_unlock;
- efx->type->monitor(efx);
+ * most of the work of monitor() anyway. */
+ if (mutex_trylock(&efx->mac_lock)) {
+ if (efx->port_enabled)
+ efx->type->monitor(efx);
+ mutex_unlock(&efx->mac_lock);
+ }
-out_unlock:
- mutex_unlock(&efx->mac_lock);
-out_requeue:
queue_delayed_work(efx->workqueue, &efx->monitor_work,
efx_monitor_interval);
}
--
1.7.2.1
--
Ben ...From: Ben Hutchings <bhutchings@solarflare.com> All applied except patch #7 as noted in the thread for that patch. Thanks! --
From: David Miller <davem@davemloft.net> Ben, just FYI, I had to add "linux/vmalloc.h" includes to both net/core/ethtool.c and drivers/net/sfc/net_driver.h otherwise the build breaks on some architectures. x86 can be a really bad arch to validates builds on because it currently gets vmalloc.h implicitly by some of it's core header files. In particular, asm/io.h :-/ --
Thanks, and sorry for missing this. We have some SPARC systems that I could potentially test on but I think they're limited to running Solaris. Ben. -- 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. --
