headers
Ben Hutchings | 1 Mar 2011 01:10

Re: [PATCHv2 net-next-2.6] ethtool: Compat handling for struct ethtool_rxnfc

On Mon, 2011-02-28 at 15:51 -0800, Kees Cook wrote:
> Hi Ben,
> 
> On Mon, Feb 28, 2011 at 09:55:41PM +0000, Ben Hutchings wrote:
> > I'm not sure whether more checks on rule_cnt are required for security
> > or whether compat_alloc_user_space() and copy_in_user() can be relied on
> > to limit any buffer overrun to the user process's own memory.  It looks
> > like this is safe on x86.
> 
> I'm less familiar with the compat world, but it looks sane to me. All the
> copy_in_user() calls are calculated based on structure sizes, right? Except
> for the rule_cnt one, which was already bounds-checked for output.
[...]

No, because the native ioctl implementation writes out the actual number
of rules to rxnfc->rule_cnt and we then read that back into rule_cnt
before performing the copy from rxnfc->rule_locs to
compat_rxnfc->rule_locs.  So userland can race with us and modify that
value.

We could use the original value of rule_cnt to calculate the size to
copy; it would just mean copying more than we need to most of the time.

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.
(Continue reading)

Permalink | Reply | 0 comments |
headers
Ben Hutchings | 1 Mar 2011 01:35

Re: [ethtool PATCH 2/2] Add RX packet classification interface

On Tue, 2011-02-22 at 12:52 -0800, Alexander Duyck wrote:
[...]
> >>  <at>  <at>  -408,6 +425,14  <at>  <at>  static int msglvl_changed;
> >>   static u32 msglvl_wanted = 0;
> >>   static u32 msglvl_mask = 0;
> >>
> >> +static int rx_rings_get = 0;
> >> +static int rx_class_rule_get = -1;
> >> +static int rx_class_rule_getall = 0;
> >> +static int rx_class_rule_del = -1;
> >> +static int rx_class_rule_added = 0;
> >> +static struct ethtool_rx_flow_spec rx_rule_fs;
> >> +static u8 rxclass_loc_valid = 0;
> >> +
> >>   static enum {
> >>        ONLINE=0,
> >>        OFFLINE,
> > [...]
> >>  <at>  <at>  -945,6 +974,23  <at>  <at>  static void parse_cmdline(int argc, char **argp)
> >>                                                rxflow_str_to_type(argp[i]);
> >>                                        if (!rx_fhash_get)
> >>                                                show_usage(1);
> >> +                             } else if (!strcmp(argp[i], "rx-rings")) {
> >> +                                     i += 1;
> >> +                                     rx_rings_get = 1;
> >
> > I'm not convinced of the value of a separate rx-rings option/keyword.
> > However it's probably worth displaying the number of rings/queues when
> > showing other flow hashing and steering/filtering information (the -x
> > option does this).
(Continue reading)

Permalink | Reply | 10 comments |
headers
Mikael Abrahamsson | 1 Mar 2011 01:46
Picon
Favicon

Re: txqueuelen has wrong units; should be time

On Mon, 28 Feb 2011, John Heffner wrote:

> Right... while I generally agree that a fixed-length drop-tail queue 
> isn't optimal, isn't this problem what the various AQM schemes try to 
> solve?

I am not an expert on exactly how Linux does this, but for Cisco and for 
instance ATM interfaces, there are two stages of queuing. One is the 
"hardware queue", which is a FIFO queue going into the ATM framer. If one 
wants low CPU usage, then this needs to be high so multiple packets can be 
put there per interrupt. Since AQM is working before this, it also means 
the low-latency-queue will have a higher latency as it ends up behind 
larger packets in the hw queue.

So on what level does the AQM work in Linux? Does it work similarily, that 
txqueuelen is a FIFO queue to the hardware that AQM feeds packets into?

Also, when one uses WRED the thinking is generally to keep the average 
queue len down, but still allow for bursts by dynamically changing the 
drop probability and where it happens. When there is no queuing, allow for 
big queue (so it can fill up if needed), but if the queue is large for 
several seconds, start to apply WRED to bring it down.

There is generally no need at all to constantly buffer > 50 ms of data, 
then it's better to just start selectively dropping it. In time of 
burstyness (perhaps when re-routing traffic) there is need to buffer 
200-500ms of during perhaps 1-2 seconds before things stabilize.

So one queuing scheme and one queue limit isn't going to solve this, there 
need to be some dynamic built into the system for it to work well.
(Continue reading)

Permalink | Reply | 5 comments |
headers
Ben Hutchings | 1 Mar 2011 02:16

pull request: sfc-next-2.6 2011-03-01

The following changes since commit 5b2c4dd2ec12cf0e53b2bd2926f0fe2d1fbb4eda:
  Jiri Pirko (1):
        net: convert bonding to use rx_handler

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/bwh/sfc-next-2.6.git for-davem

Ben Hutchings (6):
      sfc: Read MC firmware version when requested through ethtool
      sfc: Do not read STAT1.FAULT in efx_mdio_check_mmd()
      sfc: Update copyright dates
      sfc: Expose TX push and TSO counters through ethtool statistics
      sfc: Remove configurable FIFO thresholds for pause frame generation
      sfc: Bump version to 3.1

Steve Hodgson (2):
      sfc: Reduce size of efx_rx_buffer by unionising skb and page
      sfc: Reduce size of efx_rx_buffer further by removing data member

 drivers/net/sfc/efx.c           |    2 +-
 drivers/net/sfc/efx.h           |    2 +-
 drivers/net/sfc/ethtool.c       |   27 ++++++-
 drivers/net/sfc/falcon.c        |   22 ++----
 drivers/net/sfc/falcon_boards.c |    2 +-
 drivers/net/sfc/falcon_xmac.c   |    2 +-
 drivers/net/sfc/io.h            |    2 +-
 drivers/net/sfc/mcdi.c          |   23 ++----
 drivers/net/sfc/mcdi.h          |    4 +-
 drivers/net/sfc/mcdi_mac.c      |    2 +-
(Continue reading)

Permalink | Reply | 12 comments |
headers
Stephen Hemminger | 1 Mar 2011 02:17
Favicon

[PATCH] sched: QFQ - quick fair queue scheduler (v2)

This is an implementation of the Quick Fair Queue scheduler developed
by Fabio Checconi. The same algorithm is already implemented in ipfw
in FreeBSD. Fabio had an earlier version developed on Linux, I just
cleaned it up and tested it. All bugs are mine.

This version is still experimental, do not use for production.

Signed-off-by: Stephen Hemminger <shemminger <at> vyatta.com>

---
v2 - Did some debugging, and rebased against net-next tree.

 include/linux/pkt_sched.h |   14 
 net/sched/Kconfig         |   11 
 net/sched/Makefile        |    1 
 net/sched/sch_qfq.c       | 1124 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 1150 insertions(+)

--- a/include/linux/pkt_sched.h	2011-02-28 13:28:57.763177314 -0800
+++ b/include/linux/pkt_sched.h	2011-02-28 13:29:10.466792117 -0800
 <at>  <at>  -588,4 +588,18  <at>  <at>  struct tc_sfb_xstats {

 #define SFB_MAX_PROB 0xFFFF

+/* QFQ */
+enum {
+	TCA_QFQ_WEIGHT,
+	TCA_QFQ_LMAX,
+	__TCA_QFQ_MAX
+};
(Continue reading)

Permalink | Reply | 33 comments |
headers
Ben Hutchings | 1 Mar 2011 02:21

[PATCH net-next-2.6] sfc: Reduce size of efx_rx_buffer by unionising skb and page

From: Steve Hodgson <shodgson <at> solarflare.com>

[bwh: Forward-ported to net-next-2.6.]
Signed-off-by: Ben Hutchings <bhutchings <at> solarflare.com>
---
 drivers/net/sfc/net_driver.h |    8 +++-
 drivers/net/sfc/rx.c         |   96 +++++++++++++++++++----------------------
 2 files changed, 51 insertions(+), 53 deletions(-)

diff --git a/drivers/net/sfc/net_driver.h b/drivers/net/sfc/net_driver.h
index 15b9068..59ff32a 100644
--- a/drivers/net/sfc/net_driver.h
+++ b/drivers/net/sfc/net_driver.h
 <at>  <at>  -216,13 +216,17  <at>  <at>  struct efx_tx_queue {
  *	If both this and skb are %NULL, the buffer slot is currently free.
  *  <at> data: Pointer to ethernet header
  *  <at> len: Buffer length, in bytes.
+ *  <at> is_page: Indicates if  <at> page is valid. If false,  <at> skb is valid.
  */
 struct efx_rx_buffer {
 	dma_addr_t dma_addr;
-	struct sk_buff *skb;
-	struct page *page;
+	union {
+		struct sk_buff *skb;
+		struct page *page;
+	} u;
 	char *data;
 	unsigned int len;
+	bool is_page;
(Continue reading)

Permalink | Reply | 3 comments |
headers
Ben Hutchings | 1 Mar 2011 02:22

[PATCH net-next-2.6 2/8] sfc: Reduce size of efx_rx_buffer further by removing data member

From: Steve Hodgson <shodgson <at> solarflare.com>

Instead calculate the KVA of receive data. It's not like it's a hard sum.

[bwh: Fixed to work with GRO.]
Signed-off-by: Ben Hutchings <bhutchings <at> solarflare.com>
---
 drivers/net/sfc/net_driver.h |    2 -
 drivers/net/sfc/rx.c         |   50 +++++++++++++++++++++++------------------
 2 files changed, 28 insertions(+), 24 deletions(-)

diff --git a/drivers/net/sfc/net_driver.h b/drivers/net/sfc/net_driver.h
index 59ff32a..5b001c1 100644
--- a/drivers/net/sfc/net_driver.h
+++ b/drivers/net/sfc/net_driver.h
 <at>  <at>  -214,7 +214,6  <at>  <at>  struct efx_tx_queue {
  *	If both this and page are %NULL, the buffer slot is currently free.
  *  <at> page: The associated page buffer, if any.
  *	If both this and skb are %NULL, the buffer slot is currently free.
- *  <at> data: Pointer to ethernet header
  *  <at> len: Buffer length, in bytes.
  *  <at> is_page: Indicates if  <at> page is valid. If false,  <at> skb is valid.
  */
 <at>  <at>  -224,7 +223,6  <at>  <at>  struct efx_rx_buffer {
 		struct sk_buff *skb;
 		struct page *page;
 	} u;
-	char *data;
 	unsigned int len;
 	bool is_page;
(Continue reading)

Permalink | Reply | 0 comments |
headers
Ben Hutchings | 1 Mar 2011 02:22

[PATCH net-next-2.6 3/8] sfc: Read MC firmware version when requested through ethtool

We currently make no use of siena_nic_data::fw_{version,build} except
to format the firmware version for ethtool_get_drvinfo().  Since we
only read the version at start of day, this information is incorrect
after an MC firmware update.  Remove the cached version information
and read it via MCDI whenever it is requested.

Signed-off-by: Ben Hutchings <bhutchings <at> solarflare.com>
---
 drivers/net/sfc/ethtool.c |    4 ++--
 drivers/net/sfc/mcdi.c    |   21 ++++++---------------
 drivers/net/sfc/mcdi.h    |    2 +-
 drivers/net/sfc/nic.h     |    6 ------
 drivers/net/sfc/siena.c   |   17 -----------------
 5 files changed, 9 insertions(+), 41 deletions(-)

diff --git a/drivers/net/sfc/ethtool.c b/drivers/net/sfc/ethtool.c
index 272cfe7..3e974b1 100644
--- a/drivers/net/sfc/ethtool.c
+++ b/drivers/net/sfc/ethtool.c
 <at>  <at>  -237,8 +237,8  <at>  <at>  static void efx_ethtool_get_drvinfo(struct net_device *net_dev,
 	strlcpy(info->driver, KBUILD_MODNAME, sizeof(info->driver));
 	strlcpy(info->version, EFX_DRIVER_VERSION, sizeof(info->version));
 	if (efx_nic_rev(efx) >= EFX_REV_SIENA_A0)
-		siena_print_fwver(efx, info->fw_version,
-				  sizeof(info->fw_version));
+		efx_mcdi_print_fwver(efx, info->fw_version,
+				     sizeof(info->fw_version));
 	strlcpy(info->bus_info, pci_name(efx->pci_dev), sizeof(info->bus_info));
 }
(Continue reading)

Permalink | Reply | 0 comments |
headers
Ben Hutchings | 1 Mar 2011 02:22

[PATCH net-next-2.6 4/8] sfc: Do not read STAT1.FAULT in efx_mdio_check_mmd()

This field does not exist in all MMDs we want to check, and all
callers allow it to be set (fault_fatal = 0).

Remove the loopback condition, as STAT2.DEVPRST should be valid
regardless of any fault.

Signed-off-by: Ben Hutchings <bhutchings <at> solarflare.com>
---
 drivers/net/sfc/mdio_10g.c     |   32 +++++---------------------------
 drivers/net/sfc/mdio_10g.h     |    3 +--
 drivers/net/sfc/tenxpress.c    |    2 +-
 drivers/net/sfc/txc43128_phy.c |    2 +-
 4 files changed, 8 insertions(+), 31 deletions(-)

diff --git a/drivers/net/sfc/mdio_10g.c b/drivers/net/sfc/mdio_10g.c
index 56b0266..6e82e5b 100644
--- a/drivers/net/sfc/mdio_10g.c
+++ b/drivers/net/sfc/mdio_10g.c
 <at>  <at>  -51,13 +51,10  <at>  <at>  int efx_mdio_reset_mmd(struct efx_nic *port, int mmd,
 	return spins ? spins : -ETIMEDOUT;
 }

-static int efx_mdio_check_mmd(struct efx_nic *efx, int mmd, int fault_fatal)
+static int efx_mdio_check_mmd(struct efx_nic *efx, int mmd)
 {
 	int status;

-	if (LOOPBACK_INTERNAL(efx))
-		return 0;
-
(Continue reading)

Permalink | Reply | 0 comments |
headers
Ben Hutchings | 1 Mar 2011 02:23

[PATCH net-next-2.6 5/8] sfc: Update copyright dates

Signed-off-by: Ben Hutchings <bhutchings <at> solarflare.com>
---
 drivers/net/sfc/efx.c           |    2 +-
 drivers/net/sfc/efx.h           |    2 +-
 drivers/net/sfc/ethtool.c       |    2 +-
 drivers/net/sfc/falcon.c        |    2 +-
 drivers/net/sfc/falcon_boards.c |    2 +-
 drivers/net/sfc/falcon_xmac.c   |    2 +-
 drivers/net/sfc/io.h            |    2 +-
 drivers/net/sfc/mcdi.c          |    2 +-
 drivers/net/sfc/mcdi.h          |    2 +-
 drivers/net/sfc/mcdi_mac.c      |    2 +-
 drivers/net/sfc/mcdi_pcol.h     |    2 +-
 drivers/net/sfc/mcdi_phy.c      |    2 +-
 drivers/net/sfc/mdio_10g.c      |    2 +-
 drivers/net/sfc/mdio_10g.h      |    2 +-
 drivers/net/sfc/mtd.c           |    2 +-
 drivers/net/sfc/net_driver.h    |    2 +-
 drivers/net/sfc/nic.c           |    2 +-
 drivers/net/sfc/nic.h           |    2 +-
 drivers/net/sfc/phy.h           |    2 +-
 drivers/net/sfc/qt202x_phy.c    |    2 +-
 drivers/net/sfc/regs.h          |    2 +-
 drivers/net/sfc/rx.c            |    2 +-
 drivers/net/sfc/selftest.c      |    2 +-
 drivers/net/sfc/selftest.h      |    2 +-
 drivers/net/sfc/siena.c         |    2 +-
 drivers/net/sfc/spi.h           |    2 +-
 drivers/net/sfc/tenxpress.c     |    2 +-
 drivers/net/sfc/tx.c            |    2 +-
(Continue reading)

Permalink | Reply | 0 comments |

Gmane