* [PATCH] net+crypto: Use vmalloc for zlib inflate buffers.
From: David Miller @ 2011-06-28 4:26 UTC (permalink / raw)
To: netdev; +Cc: eilong, herbert, mjackson220.list
They are 64K and result in order-4 allocations, even with SLUB.
Therefore, just like we always have for the deflate buffers, use
vmalloc.
Reported-by: Martin Jackson <mjackson220.list@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
diff --git a/crypto/deflate.c b/crypto/deflate.c
index b5ccae2..b0165ec 100644
--- a/crypto/deflate.c
+++ b/crypto/deflate.c
@@ -32,7 +32,6 @@
#include <linux/interrupt.h>
#include <linux/mm.h>
#include <linux/net.h>
-#include <linux/slab.h>
#define DEFLATE_DEF_LEVEL Z_DEFAULT_COMPRESSION
#define DEFLATE_DEF_WINBITS 11
@@ -73,7 +72,7 @@ static int deflate_decomp_init(struct deflate_ctx *ctx)
int ret = 0;
struct z_stream_s *stream = &ctx->decomp_stream;
- stream->workspace = kzalloc(zlib_inflate_workspacesize(), GFP_KERNEL);
+ stream->workspace = vzalloc(zlib_inflate_workspacesize());
if (!stream->workspace) {
ret = -ENOMEM;
goto out;
@@ -86,7 +85,7 @@ static int deflate_decomp_init(struct deflate_ctx *ctx)
out:
return ret;
out_free:
- kfree(stream->workspace);
+ vfree(stream->workspace);
goto out;
}
@@ -99,7 +98,7 @@ static void deflate_comp_exit(struct deflate_ctx *ctx)
static void deflate_decomp_exit(struct deflate_ctx *ctx)
{
zlib_inflateEnd(&ctx->decomp_stream);
- kfree(ctx->decomp_stream.workspace);
+ vfree(ctx->decomp_stream.workspace);
}
static int deflate_init(struct crypto_tfm *tfm)
diff --git a/crypto/zlib.c b/crypto/zlib.c
index d11d761..06b62e5 100644
--- a/crypto/zlib.c
+++ b/crypto/zlib.c
@@ -29,7 +29,6 @@
#include <linux/interrupt.h>
#include <linux/mm.h>
#include <linux/net.h>
-#include <linux/slab.h>
#include <crypto/internal/compress.h>
@@ -60,7 +59,7 @@ static void zlib_decomp_exit(struct zlib_ctx *ctx)
if (stream->workspace) {
zlib_inflateEnd(stream);
- kfree(stream->workspace);
+ vfree(stream->workspace);
stream->workspace = NULL;
}
}
@@ -228,13 +227,13 @@ static int zlib_decompress_setup(struct crypto_pcomp *tfm, void *params,
? nla_get_u32(tb[ZLIB_DECOMP_WINDOWBITS])
: DEF_WBITS;
- stream->workspace = kzalloc(zlib_inflate_workspacesize(), GFP_KERNEL);
+ stream->workspace = vzalloc(zlib_inflate_workspacesize());
if (!stream->workspace)
return -ENOMEM;
ret = zlib_inflateInit2(stream, ctx->decomp_windowBits);
if (ret != Z_OK) {
- kfree(stream->workspace);
+ vfree(stream->workspace);
stream->workspace = NULL;
return -EINVAL;
}
diff --git a/drivers/net/bnx2x/bnx2x_main.c b/drivers/net/bnx2x/bnx2x_main.c
index 4b70311..74be989 100644
--- a/drivers/net/bnx2x/bnx2x_main.c
+++ b/drivers/net/bnx2x/bnx2x_main.c
@@ -49,6 +49,7 @@
#include <linux/zlib.h>
#include <linux/io.h>
#include <linux/stringify.h>
+#include <linux/vmalloc.h>
#define BNX2X_MAIN
#include "bnx2x.h"
@@ -4537,8 +4538,7 @@ static int bnx2x_gunzip_init(struct bnx2x *bp)
if (bp->strm == NULL)
goto gunzip_nomem2;
- bp->strm->workspace = kmalloc(zlib_inflate_workspacesize(),
- GFP_KERNEL);
+ bp->strm->workspace = vmalloc(zlib_inflate_workspacesize());
if (bp->strm->workspace == NULL)
goto gunzip_nomem3;
@@ -4562,7 +4562,7 @@ gunzip_nomem1:
static void bnx2x_gunzip_end(struct bnx2x *bp)
{
if (bp->strm) {
- kfree(bp->strm->workspace);
+ vfree(bp->strm->workspace);
kfree(bp->strm);
bp->strm = NULL;
}
diff --git a/drivers/net/ppp_deflate.c b/drivers/net/ppp_deflate.c
index 31e9407..1dbdf82 100644
--- a/drivers/net/ppp_deflate.c
+++ b/drivers/net/ppp_deflate.c
@@ -305,7 +305,7 @@ static void z_decomp_free(void *arg)
if (state) {
zlib_inflateEnd(&state->strm);
- kfree(state->strm.workspace);
+ vfree(state->strm.workspace);
kfree(state);
}
}
@@ -345,8 +345,7 @@ static void *z_decomp_alloc(unsigned char *options, int opt_len)
state->w_size = w_size;
state->strm.next_out = NULL;
- state->strm.workspace = kmalloc(zlib_inflate_workspacesize(),
- GFP_KERNEL|__GFP_REPEAT);
+ state->strm.workspace = vmalloc(zlib_inflate_workspacesize());
if (state->strm.workspace == NULL)
goto out_free;
^ permalink raw reply related
* Re: ppp_deflate + kmalloc
From: David Miller @ 2011-06-28 4:15 UTC (permalink / raw)
To: mjackson220.list; +Cc: carlsonj, paulus, linux-ppp, netdev
In-Reply-To: <BANLkTinMNb65nyo+Y8xhpTP2RC6x06ZJqQ@mail.gmail.com>
From: Martin Jackson <mjackson220.list@gmail.com>
Date: Sun, 26 Jun 2011 23:17:54 +0200
> Thanks for the advice. Hopefully we can indeed remove these obscure
> PPP options from our configuration - I'll look into that.
>
> However, the point still stands that a 4th order kmalloc is likely to
> fail (even our "embedded" device has 256MB RAM and slub allocator,
> after all), and the page allocation code regards anything above order
> 3 as unrealistic and doesn't invoke the OOM to try to satisfy it, so
> my view remains that this should be changed to vmalloc.
PPP is not the only place in the tree where we potentially have
this problem.
crypto/deflate.c: stream->workspace = kzalloc(zlib_inflate_workspacesize(), GFP_KERNEL);
crypto/zlib.c: stream->workspace = kzalloc(zlib_inflate_workspacesize(), GFP_KERNEL);
drivers/net/bnx2x/bnx2x_main.c: bp->strm->workspace = kmalloc(zlib_inflate_workspacesize(),
drivers/net/ppp_deflate.c: state->strm.workspace = kmalloc(zlib_inflate_workspacesize(),
fs/binfmt_flat.c: strm.workspace = kmalloc(zlib_inflate_workspacesize(), GFP_KERNEL);
lib/zlib_inflate/infutil.c: strm->workspace = kmalloc(zlib_inflate_workspacesize(), GFP_KERNEL);
A particularly problematic case is binfmt_flat.c since this is used on
systems which lack an MMU, and therefore for which vmalloc() is not
applicable.
What I'll do for now is transform all of the networking cases, but
someone should think seriously about the remaining cases.
^ permalink raw reply
* Re: [PATCH] net: sh_eth: tidyup compile warning
From: David Miller @ 2011-06-28 4:03 UTC (permalink / raw)
To: kuninori.morimoto.gx; +Cc: netdev, yoshihiro.shimoda.uh
In-Reply-To: <87pqm40yx3.wl%kuninori.morimoto.gx@gmail.com>
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Date: Thu, 23 Jun 2011 19:02:38 -0700 (PDT)
> This patch tidyup below warning
>
> ${LINUX}/drivers/net/sh_eth.c:1773: warning:
> 'mdp' may be used uninitialized in this function
>
> Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Signed-off-by: Kuninori Morimoto <morimoto.kuninori@renesas.com>
Applied, thanks.
^ permalink raw reply
* linux-next: manual merge of the net tree with the wireless-current tree
From: Stephen Rothwell @ 2011-06-28 4:02 UTC (permalink / raw)
To: David Miller, netdev
Cc: linux-next, linux-kernel, Johannes Berg, Wey-Yi Guy,
John W. Linville, Emmanuel Grumbach
Hi all,
Today's linux-next merge of the net tree got a conflict in
drivers/net/wireless/iwlwifi/iwl-tx.c between commits 1107a08a1a3e
("iwlagn: fix cmd queue unmap"), e815407d395e ("iwlagn: map command
buffers BIDI") and 2627c002cbed ("iwlagn: use PCI_DMA_* for pci_*
operations") from the wireless-current tree and commit 795414db8607
("iwlagn: don't use the PCI wrappers for DMA operation") from the net
tree.
I fixed it up (I think - see below) and can carry the fix as necessary.
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
diff --cc drivers/net/wireless/iwlwifi/iwl-tx.c
index 9eee978,fd8aee9..0000000
--- a/drivers/net/wireless/iwlwifi/iwl-tx.c
+++ b/drivers/net/wireless/iwlwifi/iwl-tx.c
@@@ -126,9 -126,8 +126,8 @@@ static inline u8 iwl_tfd_get_num_tbs(st
}
static void iwlagn_unmap_tfd(struct iwl_priv *priv, struct iwl_cmd_meta *meta,
- struct iwl_tfd *tfd)
+ struct iwl_tfd *tfd, enum dma_data_direction dma_dir)
{
- struct pci_dev *dev = priv->pci_dev;
int i;
int num_tbs;
@@@ -150,8 -149,8 +149,8 @@@
/* Unmap chunks, if any. */
for (i = 1; i < num_tbs; i++)
- pci_unmap_single(dev, iwl_tfd_tb_get_addr(tfd, i),
+ dma_unmap_single(priv->bus.dev, iwl_tfd_tb_get_addr(tfd, i),
- iwl_tfd_tb_get_len(tfd, i), DMA_TO_DEVICE);
+ iwl_tfd_tb_get_len(tfd, i), dma_dir);
}
/**
@@@ -167,8 -166,7 +166,8 @@@ void iwlagn_txq_free_tfd(struct iwl_pri
struct iwl_tfd *tfd_tmp = txq->tfds;
int index = txq->q.read_ptr;
- iwlagn_unmap_tfd(priv, &txq->meta[index], &tfd_tmp[index]);
+ iwlagn_unmap_tfd(priv, &txq->meta[index], &tfd_tmp[index],
- PCI_DMA_TODEVICE);
++ DMA_TO_DEVICE);
/* free SKB */
if (txq->txb) {
@@@ -311,8 -309,10 +310,8 @@@ void iwl_cmd_queue_unmap(struct iwl_pri
i = get_cmd_index(q, q->read_ptr);
if (txq->meta[i].flags & CMD_MAPPED) {
- dma_unmap_single(priv->bus.dev,
- dma_unmap_addr(&txq->meta[i], mapping),
- dma_unmap_len(&txq->meta[i], len),
+ iwlagn_unmap_tfd(priv, &txq->meta[i], &txq->tfds[i],
- PCI_DMA_BIDIRECTIONAL);
+ DMA_BIDIRECTIONAL);
txq->meta[i].flags = 0;
}
@@@ -693,12 -698,11 +692,12 @@@ int iwl_enqueue_hcmd(struct iwl_priv *p
continue;
if (!(cmd->dataflags[i] & IWL_HCMD_DFL_NOCOPY))
continue;
- phys_addr = pci_map_single(priv->pci_dev, (void *)cmd->data[i],
- cmd->len[i], PCI_DMA_BIDIRECTIONAL);
- if (pci_dma_mapping_error(priv->pci_dev, phys_addr)) {
+ phys_addr = dma_map_single(priv->bus.dev, (void *)cmd->data[i],
- cmd->len[i], DMA_TO_DEVICE);
++ cmd->len[i], DMA_BIDIRECTIONAL);
+ if (dma_mapping_error(priv->bus.dev, phys_addr)) {
iwlagn_unmap_tfd(priv, out_meta,
- &txq->tfds[q->write_ptr]);
+ &txq->tfds[q->write_ptr],
- PCI_DMA_BIDIRECTIONAL);
++ DMA_BIDIRECTIONAL);
idx = -ENOMEM;
goto out;
}
@@@ -802,7 -806,7 +801,7 @@@ void iwl_tx_cmd_complete(struct iwl_pri
cmd = txq->cmd[cmd_index];
meta = &txq->meta[cmd_index];
- iwlagn_unmap_tfd(priv, meta, &txq->tfds[index], PCI_DMA_BIDIRECTIONAL);
- iwlagn_unmap_tfd(priv, meta, &txq->tfds[index]);
++ iwlagn_unmap_tfd(priv, meta, &txq->tfds[index], DMA_BIDIRECTIONAL);
/* Input error checking is done when commands are added to queue. */
if (meta->flags & CMD_WANT_SKB) {
^ permalink raw reply
* [PATCH 3/9 v3] myri10ge: rework parity error check and cleanup
From: Jon Mason @ 2011-06-28 3:57 UTC (permalink / raw)
To: davem; +Cc: netdev, Andrew Gallatin
In-Reply-To: <1309187108-12715-3-git-send-email-mason@myri.com>
Clean up watchdog reset code:
- move code that checks for stuck slice to a common routine
- unless there is a confirmed h/w fault, verify that a stuck
slice is still stuck in the watchdog worker; if the slice is no
longer stuck, abort the reset.
- this removes an egregious 2000ms pause in the watchdog worker that
was a diagnostic aid (to look for spurious resets) the snuck into
production code.
v3 includes corrections from Joe Perches
Signed-off-by: Jon Mason <mason@myri.com>
---
drivers/net/myri10ge/myri10ge.c | 100 +++++++++++++++++++++++---------------
1 files changed, 60 insertions(+), 40 deletions(-)
diff --git a/drivers/net/myri10ge/myri10ge.c b/drivers/net/myri10ge/myri10ge.c
index 0f0f83d..ca03457 100644
--- a/drivers/net/myri10ge/myri10ge.c
+++ b/drivers/net/myri10ge/myri10ge.c
@@ -193,6 +193,7 @@ struct myri10ge_slice_state {
int watchdog_tx_done;
int watchdog_tx_req;
int watchdog_rx_done;
+ int stuck;
#ifdef CONFIG_MYRI10GE_DCA
int cached_dca_tag;
int cpu;
@@ -3442,6 +3443,42 @@ static u32 myri10ge_read_reboot(struct myri10ge_priv *mgp)
return reboot;
}
+static void
+myri10ge_check_slice(struct myri10ge_slice_state *ss, int *reset_needed,
+ int *busy_slice_cnt, u32 rx_pause_cnt)
+{
+ struct myri10ge_priv *mgp = ss->mgp;
+ int slice = ss - mgp->ss;
+
+ if (ss->tx.req != ss->tx.done &&
+ ss->tx.done == ss->watchdog_tx_done &&
+ ss->watchdog_tx_req != ss->watchdog_tx_done) {
+ /* nic seems like it might be stuck.. */
+ if (rx_pause_cnt != mgp->watchdog_pause) {
+ if (net_ratelimit())
+ netdev_warn(mgp->dev, "slice %d: TX paused, "
+ "check link partner\n", slice);
+ } else {
+ netdev_warn(mgp->dev,
+ "slice %d: TX stuck %d %d %d %d %d %d\n",
+ slice, ss->tx.queue_active, ss->tx.req,
+ ss->tx.done, ss->tx.pkt_start,
+ ss->tx.pkt_done,
+ (int)ntohl(mgp->ss[slice].fw_stats->
+ send_done_count));
+ *reset_needed = 1;
+ ss->stuck = 1;
+ }
+ }
+ if (ss->watchdog_tx_done != ss->tx.done ||
+ ss->watchdog_rx_done != ss->rx_done.cnt) {
+ *busy_slice_cnt += 1;
+ }
+ ss->watchdog_tx_done = ss->tx.done;
+ ss->watchdog_tx_req = ss->tx.req;
+ ss->watchdog_rx_done = ss->rx_done.cnt;
+}
+
/*
* This watchdog is used to check whether the board has suffered
* from a parity error and needs to be recovered.
@@ -3450,10 +3487,12 @@ static void myri10ge_watchdog(struct work_struct *work)
{
struct myri10ge_priv *mgp =
container_of(work, struct myri10ge_priv, watchdog_work);
- struct myri10ge_tx_buf *tx;
- u32 reboot;
+ struct myri10ge_slice_state *ss;
+ u32 reboot, rx_pause_cnt;
int status, rebooted;
int i;
+ int reset_needed = 0;
+ int busy_slice_cnt = 0;
u16 cmd, vendor;
mgp->watchdog_resets++;
@@ -3465,8 +3504,7 @@ static void myri10ge_watchdog(struct work_struct *work)
* For now, just report it */
reboot = myri10ge_read_reboot(mgp);
netdev_err(mgp->dev, "NIC rebooted (0x%x),%s resetting\n",
- reboot,
- myri10ge_reset_recover ? "" : " not");
+ reboot, myri10ge_reset_recover ? "" : " not");
if (myri10ge_reset_recover == 0)
return;
rtnl_lock();
@@ -3498,23 +3536,24 @@ static void myri10ge_watchdog(struct work_struct *work)
return;
}
}
- /* Perhaps it is a software error. Try to reset */
-
- netdev_err(mgp->dev, "device timeout, resetting\n");
+ /* Perhaps it is a software error. See if stuck slice
+ * has recovered, reset if not */
+ rx_pause_cnt = ntohl(mgp->ss[0].fw_stats->dropped_pause);
for (i = 0; i < mgp->num_slices; i++) {
- tx = &mgp->ss[i].tx;
- netdev_err(mgp->dev, "(%d): %d %d %d %d %d %d\n",
- i, tx->queue_active, tx->req,
- tx->done, tx->pkt_start, tx->pkt_done,
- (int)ntohl(mgp->ss[i].fw_stats->
- send_done_count));
- msleep(2000);
- netdev_info(mgp->dev, "(%d): %d %d %d %d %d %d\n",
- i, tx->queue_active, tx->req,
- tx->done, tx->pkt_start, tx->pkt_done,
- (int)ntohl(mgp->ss[i].fw_stats->
- send_done_count));
+ ss = mgp->ss;
+ if (ss->stuck) {
+ myri10ge_check_slice(ss, &reset_needed,
+ &busy_slice_cnt,
+ rx_pause_cnt);
+ ss->stuck = 0;
+ }
}
+ if (!reset_needed) {
+ netdev_dbg(mgp->dev, "not resetting\n");
+ return;
+ }
+
+ netdev_err(mgp->dev, "device timeout, resetting\n");
}
if (!rebooted) {
@@ -3567,27 +3606,8 @@ static void myri10ge_watchdog_timer(unsigned long arg)
myri10ge_fill_thresh)
ss->rx_big.watchdog_needed = 0;
}
-
- if (ss->tx.req != ss->tx.done &&
- ss->tx.done == ss->watchdog_tx_done &&
- ss->watchdog_tx_req != ss->watchdog_tx_done) {
- /* nic seems like it might be stuck.. */
- if (rx_pause_cnt != mgp->watchdog_pause) {
- if (net_ratelimit())
- netdev_err(mgp->dev, "slice %d: TX paused, check link partner\n",
- i);
- } else {
- netdev_warn(mgp->dev, "slice %d stuck:", i);
- reset_needed = 1;
- }
- }
- if (ss->watchdog_tx_done != ss->tx.done ||
- ss->watchdog_rx_done != ss->rx_done.cnt) {
- busy_slice_cnt++;
- }
- ss->watchdog_tx_done = ss->tx.done;
- ss->watchdog_tx_req = ss->tx.req;
- ss->watchdog_rx_done = ss->rx_done.cnt;
+ myri10ge_check_slice(ss, &reset_needed, &busy_slice_cnt,
+ rx_pause_cnt);
}
/* if we've sent or received no traffic, poll the NIC to
* ensure it is still there. Otherwise, we risk not noticing
--
1.7.5.4
^ permalink raw reply related
* Re: [PATCH] vmxnet3: Convert to new vlan model.
From: David Miller @ 2011-06-28 3:57 UTC (permalink / raw)
To: jesse; +Cc: netdev, sbhatewara, pv-drivers
In-Reply-To: <1308870279-30773-1-git-send-email-jesse@nicira.com>
From: Jesse Gross <jesse@nicira.com>
Date: Thu, 23 Jun 2011 16:04:39 -0700
> This converts the vmxnet3 driver to use the new vlan model. In doing so
> it fixes missing tags in tcpdump and failure to do checksum offload when
> tx vlan offload is disabled.
>
> CC: Shreyas Bhatewara <sbhatewara@vmware.com>
> CC: VMware PV-Drivers <pv-drivers@vmware.com>
> Signed-off-by: Jesse Gross <jesse@nicira.com>
Applied, thanks.
^ permalink raw reply
* Re: what's causing "ip_rt_bug"?
From: David Miller @ 2011-06-28 3:55 UTC (permalink / raw)
To: ja; +Cc: mangoo, eric.dumazet, netdev, bazsi, hidden
In-Reply-To: <alpine.LFD.2.00.1106182036001.1425@ja.ssi.bg>
From: Julian Anastasov <ja@ssi.bg>
Date: Sat, 18 Jun 2011 20:53:59 +0300 (EEST)
> Hm, if it happens "sometimes", can it be some
> problem with tproxy and TIME_WAIT sockets? I see that
> tproxy_sk_is_transparent has special treatment for TW
> sockets while ip_route_me_harder is different. As result,
> may be input route is assigned for TW packets.
>
> May be inet_sk_flowi_flags() needs fixing, not
> sure. But following patch is first step to fix this
> problem. I don't have setup to test this patch.
TPROXY has special code to make sure that time-wait sockets
are not assigned to skb->sk, as explained in commit
d503b30bd648b3cb4e5f50b65d27e389960cc6d9, that would cause
all kinds of crashes in nfnetlink_log etc.
Therefore we would see skb->sk==NULL at ip_route_me_harder()
in that case.
> ===========================================================
>
> Avoid creating input routes with ip_route_me_harder.
> It does not work for locally generated packets. Instead,
> restrict sockets to provide valid saddr for output route (or
> unicast saddr for transparent proxy). For other traffic
> allow saddr to be unicast or local but if callers forget
> to check saddr type use 0 for the output route.
>
> The resulting handling should be:
>
> - REJECT TCP:
> - in INPUT we can provide addr_type = RTN_LOCAL but
> better allow rejecting traffic delivered with
> local route (no IP address => use RTN_UNSPEC to
> allow also RTN_UNICAST).
> - FORWARD: RTN_UNSPEC => allow RTN_LOCAL/RTN_UNICAST
> saddr, add fix to ignore RTN_BROADCAST and RTN_MULTICAST
> - OUTPUT: RTN_UNSPEC
>
> - NAT, mangle, ip_queue, nf_ip_reroute: RTN_UNSPEC in LOCAL_OUT
>
> - IPVS:
> - use RTN_LOCAL in LOCAL_OUT and FORWARD after SNAT
> to restrict saddr to be local
>
> Signed-off-by: Julian Anastasov <ja@ssi.bg>
Unless someone gives some negative feedback soon I'm going to
apply this.
^ permalink raw reply
* Re: [RFC][NET-NEXT PATCH 2/2] net: Add GSO to vlan_features initialization
From: Shan Wei @ 2011-06-28 3:47 UTC (permalink / raw)
To: Ben Hutchings
Cc: David Miller, netdev, Eric Dumazet, Michał Mirosław,
therbert@google.com
In-Reply-To: <1308934302.3034.4.camel@bwh-desktop>
Hi Ben:
Ben Hutchings wrote, at 06/25/2011 12:51 AM:
> Have you verified that GSO works correctly for VLAN devices if the
> underlying device does not support VLAN tag insertion?
OK. I will do some test in future days.
Then report test results.
--
Best Regards
-----
Shan Wei
^ permalink raw reply
* Re: [PATCH 2/2] can: bfin_can: auto-calculate accessor sizes
From: David Miller @ 2011-06-28 3:42 UTC (permalink / raw)
To: vapier-aBrp7R+bbdUdnm+yROfE0A
Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
netdev-u79uwXL29TY76Z2rM5mHXA, wg-5Yr1BZd7O62+XT7JhA+gdA,
uclinux-dist-devel-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b
In-Reply-To: <1308925982-14645-2-git-send-email-vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>
From: Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>
Date: Fri, 24 Jun 2011 10:33:02 -0400
> Since we have a struct that defines the sizes of the registers, we don't
> need to explicitly use the 16bit read/write helpers. Let the code figure
> out which size access to make based on the size of the C type.
>
> There should be no functional changes here.
>
> Signed-off-by: Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>
Applied.
^ permalink raw reply
* Re: [PATCH 1/2] can: bfin_can: simplify xmit id1 setup
From: David Miller @ 2011-06-28 3:42 UTC (permalink / raw)
To: kurt.van.dijck-/BeEPy95v10
Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
netdev-u79uwXL29TY76Z2rM5mHXA, vapier-aBrp7R+bbdUdnm+yROfE0A,
wg-5Yr1BZd7O62+XT7JhA+gdA,
uclinux-dist-devel-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b
In-Reply-To: <20110627074355.GA318-MxZ6Iy/zr/UdbCeoMzGj59i2O/JbrIOy@public.gmane.org>
From: Kurt Van Dijck <kurt.van.dijck-/BeEPy95v10@public.gmane.org>
Date: Mon, 27 Jun 2011 09:43:55 +0200
> On Fri, Jun 24, 2011 at 10:33:01AM -0400, Mike Frysinger wrote:
>> If we look closely, the 4 writes to TRANSMIT_CHL.id1 can be collapsed
>> down into much simpler code. So do just that.
>>
>> This also fixes a build failure due to the I/O macros no longer
>> getting pulled in. Their minor (and accidental) usage here gets
>> dropped as part of the unification.
>>
>> Signed-off-by: Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>
>
> Nice cleanup.
> Acked-by: Kurt Van Dijck <kurt.van.dijck-/BeEPy95v10@public.gmane.org>
Applied.
^ permalink raw reply
* Re: [PATCH 2/2] ipv4: Fix IPsec slowpath fragmentation problem
From: David Miller @ 2011-06-28 3:39 UTC (permalink / raw)
To: steffen.klassert; +Cc: eric.dumazet, herbert, netdev
In-Reply-To: <20110622110537.GH6489@secunet.com>
From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Wed, 22 Jun 2011 13:05:37 +0200
> ip_append_data() builds packets based on the mtu from dst_mtu(rt->dst.path).
> On IPsec the effective mtu is lower because we need to add the protocol
> headers and trailers later when we do the IPsec transformations. So after
> the IPsec transformations the packet might be too big, which leads to a
> slowpath fragmentation then. This patch fixes this by building the packets
> based on the lower IPsec mtu from dst_mtu(&rt->dst) and adapts the exthdr
> handling to this.
>
> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
Applied.
^ permalink raw reply
* Re: [PATCH 2/3] ipv4: Fix packet size calculation for IPsec packets in __ip_append_data
From: David Miller @ 2011-06-28 3:39 UTC (permalink / raw)
To: steffen.klassert; +Cc: eric.dumazet, herbert, netdev
In-Reply-To: <20110622110219.GF6489@secunet.com>
From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Wed, 22 Jun 2011 13:02:19 +0200
> In between I found the problem. In ip_setup_cork() we take the mtu on the
> base of dst_mtu(rt->dst.path) and assign it to cork->fragsize which is
> used as the mtu in __ip_append_data(). The path dst_entry is a routing
> dst_entry that does not take the IPsec header and trailer overhead into
> account. So if we build an IPsec packet based on this mtu it might be to
> big after the IPsec transformations are applied. If we take the actual
> IPsec mtu from dst_mtu(&rt->dst) and adapt the exthdr handling to this,
> it works as expected. So I'll send two patches, one that reverts Eric's
> patch and one that fixes the slowpath issue.
Thanks for doing this work Steffen.
> While reading through the code of __ip_append_data() I noticed that we
> might use ip_ufo_append_data() for packets that will be IPsec transformed
> later, is this ok? I don't know how ufo handling works, but I would guess
> that it expects an udp header and not an IPsec header as the packets
> transport header.
Indeed, it could be a real problem.
> The IPsec mtu is 1438 here, so the first packet is too big.
> xfrm4_tunnel_check_size() notices this and sends a ICMP_FRAG_NEEDED
> packet that announces a mtu of 1438 to the original sender of the ping
> packet. Unfortunately the sender is a local address, it's the IPsec
> tunnel entry point. So we update the mtu for this connection to 1438.
> Now, with the next packet xfrm_bundle_ok() notices that the path mtu has
> changed, so it subtracts the IPsec overhead from the mtu a second time
> and we end up with a mtu of 1374. This game goes until we reach a minimal
> mtu of 494.
>
> Unfortunately I don't know how to fix this. Any ideas?
If the generic PMTU handling in net/ipv4/route.c is adjusting the MTU
for the IPSEC path's route, that would be the problem.
In the case of IPSEC encapsulation, tunnels, and similar, only the
tunnel or the XFRM layer should be processing the PMTU notification.
^ permalink raw reply
* Re: [PATCH v2 1/2] ipv4: Fix packet size calculation in __ip_append_data
From: David Miller @ 2011-06-28 3:39 UTC (permalink / raw)
To: steffen.klassert; +Cc: eric.dumazet, herbert, netdev
In-Reply-To: <20110622110437.GG6489@secunet.com>
From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Wed, 22 Jun 2011 13:04:37 +0200
> Git commit 59104f06 (ip: take care of last fragment in ip_append_data)
> added a check to see if we exceed the mtu when we add trailer_len.
> However, the mtu is already subtracted by the trailer length when the
> xfrm transfomation bundles are set up. So IPsec packets with mtu
> size get fragmented, or if the DF bit is set the packets will not
> be send even though they match the mtu perfectly fine. This patch
> actually reverts commit 59104f06.
>
> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
Applied.
^ permalink raw reply
* Re: [PATCH 3/9 v2] myri10ge: rework parity error check and cleanup
From: Jon Mason @ 2011-06-28 3:31 UTC (permalink / raw)
To: Joe Perches; +Cc: davem, netdev, Andrew Gallatin
In-Reply-To: <1309227435.3344.20.camel@Joe-Laptop>
On Mon, Jun 27, 2011 at 9:17 PM, Joe Perches <joe@perches.com> wrote:
> On Mon, 2011-06-27 at 15:54 -0500, Jon Mason wrote:
>> Clean up watchdog reset code:
>> - move code that checks for stuck slice to a common routine
>> - unless there is a confirmed h/w fault, verify that a stuck
>> slice is still stuck in the watchdog worker; if the slice is no
>> longer stuck, abort the reset.
>> - this removes an egregious 2000ms pause in the watchdog worker that
>> was a diagnostic aid (to look for spurious resets) the snuck into
>> production code.
>> v2 includes corrections from Ben Hutchings and Joe Perches
>
> Here's some more trivia:
>
>> diff --git a/drivers/net/myri10ge/myri10ge.c b/drivers/net/myri10ge/myri10ge.c
> []
>> @@ -3442,6 +3443,42 @@ static u32 myri10ge_read_reboot(struct myri10ge_priv *mgp)
>> return reboot;
>> }
>>
>> +static void
>> +myri10ge_check_slice(struct myri10ge_slice_state *ss, int *reset_needed,
>> + int *busy_slice_cnt, u32 rx_pause_cnt)
>> +{
> []
>> + /* nic seems like it might be stuck.. */
>> + if (rx_pause_cnt != mgp->watchdog_pause) {
>> + if (net_ratelimit())
>> + netdev_warn(mgp->dev, "slice %d: TX paused, "
>> + "check link partner\n", slice);
>
> I think this would be better if the format weren't split.
>
> netdev_warn(mgp->dev, "slice %d: TX paused, check link partner\n",
> slice);
> or
> netdev_warn(mgp->dev,
> "slice %d: TX paused, check link partner\n",
> slice);
> or if you really must split it because exceeding 80 columns
> makes you itchy:
> netdev_warn(mgp->dev, "slice %d: "
> "TX paused, check link partner\n",
> slice);
Naa, I prefer it this way.
>> @@ -3465,8 +3504,7 @@ static void myri10ge_watchdog(struct work_struct *work)
>> * For now, just report it */
>> reboot = myri10ge_read_reboot(mgp);
>> netdev_err(mgp->dev, "NIC rebooted (0x%x),%s resetting\n",
>> - reboot,
>> - myri10ge_reset_recover ? "" : " not");
>> + reboot, myri10ge_reset_recover ? " " : " not");
>
> I think this was correct before you changed it.
>
> Maybe:
> reboot, myri10ge_reset_recover ? "" : " not");
Yes, I believe this was the intent.
>
>
>
^ permalink raw reply
* Re: [PATCH 3/9 v2] myri10ge: rework parity error check and cleanup
From: Joe Perches @ 2011-06-28 2:17 UTC (permalink / raw)
To: Jon Mason; +Cc: davem, netdev, Andrew Gallatin
In-Reply-To: <20110627205432.GA18978@myri.com>
On Mon, 2011-06-27 at 15:54 -0500, Jon Mason wrote:
> Clean up watchdog reset code:
> - move code that checks for stuck slice to a common routine
> - unless there is a confirmed h/w fault, verify that a stuck
> slice is still stuck in the watchdog worker; if the slice is no
> longer stuck, abort the reset.
> - this removes an egregious 2000ms pause in the watchdog worker that
> was a diagnostic aid (to look for spurious resets) the snuck into
> production code.
> v2 includes corrections from Ben Hutchings and Joe Perches
Here's some more trivia:
> diff --git a/drivers/net/myri10ge/myri10ge.c b/drivers/net/myri10ge/myri10ge.c
[]
> @@ -3442,6 +3443,42 @@ static u32 myri10ge_read_reboot(struct myri10ge_priv *mgp)
> return reboot;
> }
>
> +static void
> +myri10ge_check_slice(struct myri10ge_slice_state *ss, int *reset_needed,
> + int *busy_slice_cnt, u32 rx_pause_cnt)
> +{
[]
> + /* nic seems like it might be stuck.. */
> + if (rx_pause_cnt != mgp->watchdog_pause) {
> + if (net_ratelimit())
> + netdev_warn(mgp->dev, "slice %d: TX paused, "
> + "check link partner\n", slice);
I think this would be better if the format weren't split.
netdev_warn(mgp->dev, "slice %d: TX paused, check link partner\n",
slice);
or
netdev_warn(mgp->dev,
"slice %d: TX paused, check link partner\n",
slice);
or if you really must split it because exceeding 80 columns
makes you itchy:
netdev_warn(mgp->dev, "slice %d: "
"TX paused, check link partner\n",
slice);
> @@ -3465,8 +3504,7 @@ static void myri10ge_watchdog(struct work_struct *work)
> * For now, just report it */
> reboot = myri10ge_read_reboot(mgp);
> netdev_err(mgp->dev, "NIC rebooted (0x%x),%s resetting\n",
> - reboot,
> - myri10ge_reset_recover ? "" : " not");
> + reboot, myri10ge_reset_recover ? " " : " not");
I think this was correct before you changed it.
Maybe:
reboot, myri10ge_reset_recover ? "" : " not");
^ permalink raw reply
* Re: [PATCH v2] NET: AX88796: Tighten up Kconfig dependencies
From: David Miller @ 2011-06-28 1:38 UTC (permalink / raw)
To: magnus.damm
Cc: ralf, eric.y.miao, linux, ben-linux, lethal, jeff,
linux-arm-kernel, linux-kernel, linux-sh, netdev, linux-mips
In-Reply-To: <BANLkTikDxsOJKpiJs0NpMXbjVOFMHL7RZw@mail.gmail.com>
From: Magnus Damm <magnus.damm@gmail.com>
Date: Tue, 28 Jun 2011 09:40:56 +0900
> As for SH and SH-Mobile ARM, unless explicitly requested we usually
> don't restrict our platform drivers. Allowing them to build on any
> system helps to catch compile errors.
I totally agree with Magnus, drivers should build on as many systems
as possible. Even on those for which the hardware never appears.
Ralf, unless these drivers have unfixable build errors on MIPS I
do not want to add the new restrictions.
^ permalink raw reply
* RE: [PATCH] [net][bna] Fix call trace when interrupts are disabled while sleeping function kzalloc is called
From: Rasesh Mody @ 2011-06-28 0:51 UTC (permalink / raw)
To: Shyam Iyer, netdev@vger.kernel.org
Cc: ddutt@brocadel.com, Shyam Iyer, Jing Huang
In-Reply-To: <1309206092-23064-1-git-send-email-shyam_iyer@dell.com>
>From: Shyam Iyer [mailto:shyam.iyer.t@gmail.com]
>Sent: Monday, June 27, 2011 1:22 PM
>Subject: [PATCH] [net][bna] Fix call trace when interrupts are disabled
>while sleeping function kzalloc is called
>
>The kzalloc sleeps and disabling interrupts(spin_lock_irqsave) causes
>oops like the one.
Hi Shyam,
We are not calling any sleeping function while holding the lock in bnad_mbox_irq_alloc(). How would your patch fix the call trace? Also can you tell which conditions led to this trace.
Thanks,
--Rasesh
^ permalink raw reply
* Re: [PATCH v2] NET: AX88796: Tighten up Kconfig dependencies
From: Magnus Damm @ 2011-06-28 0:40 UTC (permalink / raw)
To: Ralf Baechle
Cc: David S. Miller, Eric Miao, Russell King, Ben Dooks, Paul Mundt,
Jeff Garzik, linux-arm-kernel, linux-kernel, linux-sh, netdev,
linux-mips
In-Reply-To: <20110627111259.GA13620@linux-mips.org>
Hi Ralf,
On Mon, Jun 27, 2011 at 8:13 PM, Ralf Baechle <ralf@linux-mips.org> wrote:
> In def47c5095d53814512bb0c62ec02dfdec769db1 [[netdrvr] Fix dependencies for
> ax88796 ne2k clone driver] the AX88796 driver got restricted to just be
> build for ARM and MIPS on the sole merrit that it was written for some ARM
> sytems and the driver had the misfortune to just build on MIPS, so MIPS was
> throw into the dependency for a good measure. Later
> 8687991a734a67f1638782c968f46fff0f94bb1f [ax88796: add superh to kconfig
> dependencies] added SH but only one in-tree SH system actually has an
> AX88796.
>
> Tighten up dependencies by using an auxilliary config sysmbol
> HAS_NET_AX88796 which is selected only by the platforms that actually
> have or may have an AX88796. This also means the driver won't be built
> anymore for any MIPS platform.
>
> Signed-off-by: Ralf Baechle <ralf@linux-mips.org>
> ---
> v2: fixed Sergei's complaints about the log message
I'm the one who added the SuperH bits a few years ago. Judging by the
text above it seems like you prefer not to build this driver for MIPS.
Which is totally fine with me.
As for SH and SH-Mobile ARM, unless explicitly requested we usually
don't restrict our platform drivers. Allowing them to build on any
system helps to catch compile errors. It also makes it possible to add
board support by simply adding platform data to the board file and
then updating the kconfig. Keeping the amount of code at the bare
minimum makes back porting rather easy too.
I'm not sure if the ax88796 driver does something non-standard to
require special symbols, but usually platform drivers are rather clean
and can be compiled for any architecture or platform. At least in
theory. =)
Cheers,
/ magnus
^ permalink raw reply
* Re: [PATCH 15/19 v2] tg3: remove unnecessary read of PCI_CAP_ID_EXP
From: Matt Carlson, Jon Mason @ 2011-06-27 23:33 UTC (permalink / raw)
Cc: Matthew Carlson, Michael Chan, netdev@vger.kernel.org
In-Reply-To: <20110627225649.GA20786@kudzu.us>
On Mon, Jun 27, 2011 at 03:56:50PM -0700, Jon Mason wrote:
> The PCIE capability offset is saved during PCI bus walking. Use the
> value from pci_dev instead of checking in the driver and saving it off
> the the driver specific structure. It will remove an unnecessary search
> in the PCI configuration space if this value is referenced instead of
> reacquiring it.
>
> v2 of the patch re-adds the PCI_EXPRESS flag and adds comments
> describing why it is necessary.
>
> Signed-off-by: Jon Mason <jdmason@kudzu.us>
> ---
> drivers/net/tg3.c | 25 ++++++++++++++-----------
> drivers/net/tg3.h | 5 +----
> 2 files changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
> index 97cd02d..a555efd 100644
> --- a/drivers/net/tg3.c
> +++ b/drivers/net/tg3.c
> @@ -2679,11 +2679,11 @@ static int tg3_power_down_prepare(struct tg3 *tp)
> u16 lnkctl;
>
> pci_read_config_word(tp->pdev,
> - tp->pcie_cap + PCI_EXP_LNKCTL,
> + tp->pdev->pcie_cap + PCI_EXP_LNKCTL,
Sorry to be a stickler, but can we convert all occurances of
'tp->pdev->pcie_cap' to pci_pcie_cap(tp->pdev)? If the PCI layer is
taking control of that variable, the driver shouldn't be accessing it
directly if it can help it.
> &lnkctl);
> lnkctl |= PCI_EXP_LNKCTL_CLKREQ_EN;
> pci_write_config_word(tp->pdev,
> - tp->pcie_cap + PCI_EXP_LNKCTL,
> + tp->pdev->pcie_cap + PCI_EXP_LNKCTL,
> lnkctl);
> }
>
> @@ -3485,7 +3485,7 @@ relink:
> u16 oldlnkctl, newlnkctl;
>
> pci_read_config_word(tp->pdev,
> - tp->pcie_cap + PCI_EXP_LNKCTL,
> + tp->pdev->pcie_cap + PCI_EXP_LNKCTL,
> &oldlnkctl);
> if (tp->link_config.active_speed == SPEED_100 ||
> tp->link_config.active_speed == SPEED_10)
> @@ -3494,7 +3494,7 @@ relink:
> newlnkctl = oldlnkctl | PCI_EXP_LNKCTL_CLKREQ_EN;
> if (newlnkctl != oldlnkctl)
> pci_write_config_word(tp->pdev,
> - tp->pcie_cap + PCI_EXP_LNKCTL,
> + tp->pdev->pcie_cap + PCI_EXP_LNKCTL,
> newlnkctl);
> }
>
> @@ -7226,7 +7226,7 @@ static int tg3_chip_reset(struct tg3 *tp)
>
> udelay(120);
>
> - if (tg3_flag(tp, PCI_EXPRESS) && tp->pcie_cap) {
> + if (tg3_flag(tp, PCI_EXPRESS) && pci_pcie_cap(tp->pdev)) {
> u16 val16;
>
> if (tp->pci_chip_rev_id == CHIPREV_ID_5750_A0) {
> @@ -7244,7 +7244,7 @@ static int tg3_chip_reset(struct tg3 *tp)
>
> /* Clear the "no snoop" and "relaxed ordering" bits. */
> pci_read_config_word(tp->pdev,
> - tp->pcie_cap + PCI_EXP_DEVCTL,
> + tp->pdev->pcie_cap + PCI_EXP_DEVCTL,
> &val16);
> val16 &= ~(PCI_EXP_DEVCTL_RELAX_EN |
> PCI_EXP_DEVCTL_NOSNOOP_EN);
> @@ -7255,14 +7255,14 @@ static int tg3_chip_reset(struct tg3 *tp)
> if (!tg3_flag(tp, CPMU_PRESENT))
> val16 &= ~PCI_EXP_DEVCTL_PAYLOAD;
> pci_write_config_word(tp->pdev,
> - tp->pcie_cap + PCI_EXP_DEVCTL,
> + tp->pdev->pcie_cap + PCI_EXP_DEVCTL,
> val16);
>
> pcie_set_readrq(tp->pdev, tp->pcie_readrq);
>
> /* Clear error status */
> pci_write_config_word(tp->pdev,
> - tp->pcie_cap + PCI_EXP_DEVSTA,
> + tp->pdev->pcie_cap + PCI_EXP_DEVSTA,
> PCI_EXP_DEVSTA_CED |
> PCI_EXP_DEVSTA_NFED |
> PCI_EXP_DEVSTA_FED |
> @@ -13777,8 +13777,7 @@ static int __devinit tg3_get_invariants(struct tg3 *tp)
> pci_read_config_dword(tp->pdev, TG3PCI_PCISTATE,
> &pci_state_reg);
>
> - tp->pcie_cap = pci_find_capability(tp->pdev, PCI_CAP_ID_EXP);
> - if (tp->pcie_cap != 0) {
> + if (pci_is_pcie(tp->pdev)) {
> u16 lnkctl;
>
> tg3_flag_set(tp, PCI_EXPRESS);
> @@ -13791,7 +13790,7 @@ static int __devinit tg3_get_invariants(struct tg3 *tp)
> pcie_set_readrq(tp->pdev, tp->pcie_readrq);
>
> pci_read_config_word(tp->pdev,
> - tp->pcie_cap + PCI_EXP_LNKCTL,
> + tp->pdev->pcie_cap + PCI_EXP_LNKCTL,
> &lnkctl);
> if (lnkctl & PCI_EXP_LNKCTL_CLKREQ_EN) {
> if (GET_ASIC_REV(tp->pci_chip_rev_id) ==
> @@ -13808,6 +13807,10 @@ static int __devinit tg3_get_invariants(struct tg3 *tp)
> tg3_flag_set(tp, L1PLLPD_EN);
> }
> } else if (GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5785) {
> + /* BCM5785 devices are effectively PCIe devices, and should
> + * follow PCIe codepaths, but do not have a PCIe capabilities
> + * section.
> + */
> tg3_flag_set(tp, PCI_EXPRESS);
> } else if (!tg3_flag(tp, 5705_PLUS) ||
> tg3_flag(tp, 5780_CLASS)) {
> diff --git a/drivers/net/tg3.h b/drivers/net/tg3.h
> index bedc3b4..5f250ae 100644
> --- a/drivers/net/tg3.h
> +++ b/drivers/net/tg3.h
> @@ -2857,7 +2857,7 @@ enum TG3_FLAGS {
> TG3_FLAG_IS_5788,
> TG3_FLAG_MAX_RXPEND_64,
> TG3_FLAG_TSO_CAPABLE,
> - TG3_FLAG_PCI_EXPRESS,
> + TG3_FLAG_PCI_EXPRESS, /* BCM5785 + pci_is_pcie() */
> TG3_FLAG_ASF_NEW_HANDSHAKE,
> TG3_FLAG_HW_AUTONEG,
> TG3_FLAG_IS_NIC,
> @@ -3022,10 +3022,7 @@ struct tg3 {
>
> int pm_cap;
> int msi_cap;
> - union {
> int pcix_cap;
> - int pcie_cap;
> - };
> int pcie_readrq;
>
> struct mii_bus *mdio_bus;
> --
> 1.7.5.4
>
>
^ permalink raw reply
* Re: [PATCH] [net][bna] Fix call trace when interrupts are disabled while sleeping function kzalloc is called
From: David Miller @ 2011-06-27 23:07 UTC (permalink / raw)
To: shyam.iyer.t; +Cc: netdev, rmody, ddutt, shyam_iyer
In-Reply-To: <1309206092-23064-1-git-send-email-shyam_iyer@dell.com>
From: Shyam Iyer <shyam.iyer.t@gmail.com>
Date: Mon, 27 Jun 2011 16:21:32 -0400
> The kzalloc sleeps and disabling interrupts(spin_lock_irqsave)
> causes oops like the one.
What if ->cfg_flags changes while you have dropped the lock?
If the lock doesn't protect those flags, what was it being
taken for in the first place?
^ permalink raw reply
* r8169 : always copying the rx buffer to new skb
From: John Lumby @ 2011-06-27 22:54 UTC (permalink / raw)
To: netdev
Summary of some results since previous posts in April :
Previously I suggested re-introducing the rx_copybreak parameter to provide the option of un-hooking the receive buffer rather than copying it, in order to save the overhead of the memcpy, which shows as the highest tick-count in oprofile. All buffer memcpy'ing is done on CPU0 on my system.
I then found that, without the memcpy, the driver and net stack consume other overhead elsewhere, particularly in too-frequent polling/interrupting.
Eric D pointed out that :
Doing the copy of data and building an exact size skb has benefit of
providing 'right' skb->truesize (might reduce RCVBUF contention and
avoid backlog drops) and already cached data (hot in cpu caches).
Next 'copy' is almost free (L1 cache access)
There was also some discussion off-line about using larger MTU size.
Since then, I have explored some ideas for dealing with the too-frequent polling/interrupting and the cache aspect, with some success on the first and no success on the second. In summary of results:
. With MTU of 1500 and "normal" workload, I see an improvement of between 4% - 6% in throughput, depending on kernel release and kernel .config. Specifically, with the heaviest workload and most tuned kernel .config:
no changes - ~ 1440 Megabits/sec bi-directional
with changes - ~ 1530 Megabits/sec bi-directional
(same .config for each of course)
All 4 of my atom 330's (2 physical x 2SMt per physical) were at 100% on both without and with changes for this workload, but with very different profiles.
These throughput numbers are higher than I reported before, and % improvement lower, because of the tuning to the base system and workload.
. With MTU of 6144, I see a more dramatic effect - the same workload runs at 1725 Megabit/sec on both kernels, (which may be a practical hardware limit on one of the adapters, since it hits exactly this rate almost every time no matter what else I change), but overall CPU utilization drops from ~ 80% without changes to ~60% with changes. I feel this is significant but of course its use limited to networks that can support this segment size everywhere.
Notes on the changes:
Too-frequent polling/interrupting:
These two are highly interrelated by NAPI.
Too-frequent polling:
The NAPI weight is a double-duty parameter, controlling both the dynamic choice between continuing a NAPI polling session versus leaving and resuming interrupts, and also the maximum number of receive buffers to be passed up per poll. It's also not configurable (set to 64). I split it into two numbers, one for each purpose, and made them configurable, and tried tuning them. A good value for the poll/int choice was 16, while the max-size number was best left at 64. This helps a bit, but polling is still too frequent.
I then made an interface up into the softirqd to let the driver tell the softirqd :
"keep my napi session alive but sched_yield to other runnable processes before running another poll"
I added a check to __do_softirq that if the *only* pending softirq is NET_RX_SOFTIRQ and the rx_action routine requested this, then it exits and tells the deamon to yield.
I borrowed a bit in local_softirq_pending for this. This helped a lot for certain workloads. I saw considerable drop in system CPU% on CPU0 and higher user CPU% there.
Too-frequent interrupting:
I made use of the r8169's Interrupt Mitigation feature, setting it to the maximum multiplied by a factor between 0-1 based inversely on tx queue size (large qsize, short delay and vice versa). This also helped a lot. The current driver sets these registers but only once per "up" session, during rtl8169_open of the NIC; But Hayes explained that the regs must be set on each enabling of interrupts. This is the one case where (I think) I corrected a bug present in the current driver. Harmless but not doing what was intended.
The effect of these two changes was to reduce the rate of hardware interrupts down to less than 1/20 of before, and also hold the polling rate down (around 4-5 packets per poll on average on a typical run, sometimes much higher).
memory and caching:
Here I failed to achieve anything. Based on Eric's point about memcpy giving a "free" next copy, I thought possibly memory prefetching might provide something equivalent. Specifically, prefetch the skb and its databuff immediately after un-dma'ing.
For example, with my changes and no memcpy, I see eth_type_trans() high in oprofile tick score on CPU0.
This small function does very little work but is the first (I think) to access a field in the skb->data buffer - the ethernet header. Prefetching ought to do better than memcpy'ing since only one copy of the data will enter L1, not two. But my attempts at this achieved nothing or negative.
Note - the current driver does issue a prefetch of the original buffer prior to the memcpy. But, on my system (atom CPUs), gdb of the object file r8169.o indicates no prefetch instructions are generated, only lea of the address to be prefetched. I tried changing the prefetch call to an asm generated prefetcht0/prefetchnta instruction with disappointing results. I noticed some discussion of memory prefetch in this list earlier and maybe it is not useful.
I tried to explore Eric's other point about skb->truesize but ran out of time researching. I guess my current results are negatively impacted by these memory and skb issues that Eric mentions, but I could not find any answer.
There was a question of how this changed driver handles memory pressure :
Along with the rx_copybreak change, I made the number of rx and tx ring buffers configurable and dynamically replenishable. The changed driver can tolerate occasional or even bursty alloc failures without exposing any effects outside itself, whereas the current driver drops packets. However, under extreme consecutive failures, the changed driver will eventually run too low and stop completely, whereas the current driver will (I assume) stay up. I was unable to cause either of these in my tests. Measurements with concurrent memory hogs confirmed this but did show heavy drop in throughput for the changed driver.
I've tried these changes out on all kernel release levels from 2.6.36 to 3.0-rc3 and see roughly comparable deltas on all, but with slightly different tuning required to hit the optimum, and some variability on all after 2.6.37. 2.6.37 seemed to be slightly the "best". Not sure why although I see some relevant changes to the scheduler between 2.6.37 - 38. There is also a strange effect with the old RTC in 3.0 - I had to remove it from the kernel to get good results, whereas it was a module in 2.6 levels (which I did not load for the tests). I don't need it on my system except for one ancient utility. I also found major impact from iptables and cleared all tables for the tests. That is the one item that would normally be needed in a production setup that I turned off. The overhead of iptables is presumably highly dependent on how many rules in the filter chains. (I have rather a lot in INPUT)
I don't plan to do any more on this but can provide my patch (currently one monolithic one based on DaveM 3.0.0-rc1netnext-110615) and detailed results if anyone wants.
Cheers, John Lumby
^ permalink raw reply
* [PATCH 15/19 v2] tg3: remove unnecessary read of PCI_CAP_ID_EXP
From: Jon Mason @ 2011-06-27 22:56 UTC (permalink / raw)
To: Matt Carlson; +Cc: Michael Chan, netdev
In-Reply-To: <1309196854-16232-1-git-send-email-jdmason@kudzu.us>
The PCIE capability offset is saved during PCI bus walking. Use the
value from pci_dev instead of checking in the driver and saving it off
the the driver specific structure. It will remove an unnecessary search
in the PCI configuration space if this value is referenced instead of
reacquiring it.
v2 of the patch re-adds the PCI_EXPRESS flag and adds comments
describing why it is necessary.
Signed-off-by: Jon Mason <jdmason@kudzu.us>
---
drivers/net/tg3.c | 25 ++++++++++++++-----------
drivers/net/tg3.h | 5 +----
2 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index 97cd02d..a555efd 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -2679,11 +2679,11 @@ static int tg3_power_down_prepare(struct tg3 *tp)
u16 lnkctl;
pci_read_config_word(tp->pdev,
- tp->pcie_cap + PCI_EXP_LNKCTL,
+ tp->pdev->pcie_cap + PCI_EXP_LNKCTL,
&lnkctl);
lnkctl |= PCI_EXP_LNKCTL_CLKREQ_EN;
pci_write_config_word(tp->pdev,
- tp->pcie_cap + PCI_EXP_LNKCTL,
+ tp->pdev->pcie_cap + PCI_EXP_LNKCTL,
lnkctl);
}
@@ -3485,7 +3485,7 @@ relink:
u16 oldlnkctl, newlnkctl;
pci_read_config_word(tp->pdev,
- tp->pcie_cap + PCI_EXP_LNKCTL,
+ tp->pdev->pcie_cap + PCI_EXP_LNKCTL,
&oldlnkctl);
if (tp->link_config.active_speed == SPEED_100 ||
tp->link_config.active_speed == SPEED_10)
@@ -3494,7 +3494,7 @@ relink:
newlnkctl = oldlnkctl | PCI_EXP_LNKCTL_CLKREQ_EN;
if (newlnkctl != oldlnkctl)
pci_write_config_word(tp->pdev,
- tp->pcie_cap + PCI_EXP_LNKCTL,
+ tp->pdev->pcie_cap + PCI_EXP_LNKCTL,
newlnkctl);
}
@@ -7226,7 +7226,7 @@ static int tg3_chip_reset(struct tg3 *tp)
udelay(120);
- if (tg3_flag(tp, PCI_EXPRESS) && tp->pcie_cap) {
+ if (tg3_flag(tp, PCI_EXPRESS) && pci_pcie_cap(tp->pdev)) {
u16 val16;
if (tp->pci_chip_rev_id == CHIPREV_ID_5750_A0) {
@@ -7244,7 +7244,7 @@ static int tg3_chip_reset(struct tg3 *tp)
/* Clear the "no snoop" and "relaxed ordering" bits. */
pci_read_config_word(tp->pdev,
- tp->pcie_cap + PCI_EXP_DEVCTL,
+ tp->pdev->pcie_cap + PCI_EXP_DEVCTL,
&val16);
val16 &= ~(PCI_EXP_DEVCTL_RELAX_EN |
PCI_EXP_DEVCTL_NOSNOOP_EN);
@@ -7255,14 +7255,14 @@ static int tg3_chip_reset(struct tg3 *tp)
if (!tg3_flag(tp, CPMU_PRESENT))
val16 &= ~PCI_EXP_DEVCTL_PAYLOAD;
pci_write_config_word(tp->pdev,
- tp->pcie_cap + PCI_EXP_DEVCTL,
+ tp->pdev->pcie_cap + PCI_EXP_DEVCTL,
val16);
pcie_set_readrq(tp->pdev, tp->pcie_readrq);
/* Clear error status */
pci_write_config_word(tp->pdev,
- tp->pcie_cap + PCI_EXP_DEVSTA,
+ tp->pdev->pcie_cap + PCI_EXP_DEVSTA,
PCI_EXP_DEVSTA_CED |
PCI_EXP_DEVSTA_NFED |
PCI_EXP_DEVSTA_FED |
@@ -13777,8 +13777,7 @@ static int __devinit tg3_get_invariants(struct tg3 *tp)
pci_read_config_dword(tp->pdev, TG3PCI_PCISTATE,
&pci_state_reg);
- tp->pcie_cap = pci_find_capability(tp->pdev, PCI_CAP_ID_EXP);
- if (tp->pcie_cap != 0) {
+ if (pci_is_pcie(tp->pdev)) {
u16 lnkctl;
tg3_flag_set(tp, PCI_EXPRESS);
@@ -13791,7 +13790,7 @@ static int __devinit tg3_get_invariants(struct tg3 *tp)
pcie_set_readrq(tp->pdev, tp->pcie_readrq);
pci_read_config_word(tp->pdev,
- tp->pcie_cap + PCI_EXP_LNKCTL,
+ tp->pdev->pcie_cap + PCI_EXP_LNKCTL,
&lnkctl);
if (lnkctl & PCI_EXP_LNKCTL_CLKREQ_EN) {
if (GET_ASIC_REV(tp->pci_chip_rev_id) ==
@@ -13808,6 +13807,10 @@ static int __devinit tg3_get_invariants(struct tg3 *tp)
tg3_flag_set(tp, L1PLLPD_EN);
}
} else if (GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5785) {
+ /* BCM5785 devices are effectively PCIe devices, and should
+ * follow PCIe codepaths, but do not have a PCIe capabilities
+ * section.
+ */
tg3_flag_set(tp, PCI_EXPRESS);
} else if (!tg3_flag(tp, 5705_PLUS) ||
tg3_flag(tp, 5780_CLASS)) {
diff --git a/drivers/net/tg3.h b/drivers/net/tg3.h
index bedc3b4..5f250ae 100644
--- a/drivers/net/tg3.h
+++ b/drivers/net/tg3.h
@@ -2857,7 +2857,7 @@ enum TG3_FLAGS {
TG3_FLAG_IS_5788,
TG3_FLAG_MAX_RXPEND_64,
TG3_FLAG_TSO_CAPABLE,
- TG3_FLAG_PCI_EXPRESS,
+ TG3_FLAG_PCI_EXPRESS, /* BCM5785 + pci_is_pcie() */
TG3_FLAG_ASF_NEW_HANDSHAKE,
TG3_FLAG_HW_AUTONEG,
TG3_FLAG_IS_NIC,
@@ -3022,10 +3022,7 @@ struct tg3 {
int pm_cap;
int msi_cap;
- union {
int pcix_cap;
- int pcie_cap;
- };
int pcie_readrq;
struct mii_bus *mdio_bus;
--
1.7.5.4
^ permalink raw reply related
* Re: [PATCH V7 2/4 net-next] skbuff: Add userspace zero-copy buffers in skb
From: David Miller @ 2011-06-27 22:54 UTC (permalink / raw)
To: mashirle; +Cc: mst, eric.dumazet, avi, arnd, netdev, kvm, linux-kernel
In-Reply-To: <1309189510.21764.1.camel@localhost.localdomain>
From: Shirley Ma <mashirle@us.ibm.com>
Date: Mon, 27 Jun 2011 08:45:10 -0700
> To support skb zero-copy, a pointer is needed to add to skb share info.
> Do you agree with this approach? If not, do you have any other
> suggestions?
I really can't form an opinion unless I am shown the complete
implementation, what this give us in return, what the impact is, etc.
^ permalink raw reply
* Re: SKB paged fragment lifecycle on receive
From: David Miller @ 2011-06-27 22:49 UTC (permalink / raw)
To: Ian.Campbell; +Cc: netdev, jeremy, xen-devel, eric.dumazet, rusty
In-Reply-To: <1309185724.32717.241.camel@zakaz.uk.xensource.com>
From: Ian Campbell <Ian.Campbell@eu.citrix.com>
Date: Mon, 27 Jun 2011 15:42:04 +0100
> However it seems like this might still have a problem if your SKBs are
> ever cloned. What happens in this case, e.g if a user of AF_PACKET sends
> a broadcast via a device associated with a bridge[1] (where it would be
> flooded)?
You don't need a bridge to get a clone on transmit, the packet
scheduler can do clones. Just grep for skb_clone in the packet
action handlers net/sched/act_*.c
^ permalink raw reply
* [PATCH 7/9 v2] myri10ge: misc style cleanups
From: Jon Mason @ 2011-06-27 20:56 UTC (permalink / raw)
To: davem; +Cc: netdev, Andrew Gallatin
In-Reply-To: <1309187108-12715-7-git-send-email-mason@myri.com>
Miscellaneous white space, style, and other cleanups
v2 includes corrections from Joe Perches
Signed-off-by: Jon Mason <mason@myri.com>
---
drivers/net/myri10ge/myri10ge.c | 32 ++++++++++++++------------------
1 files changed, 14 insertions(+), 18 deletions(-)
diff --git a/drivers/net/myri10ge/myri10ge.c b/drivers/net/myri10ge/myri10ge.c
index b9b80c0..90c8330 100644
--- a/drivers/net/myri10ge/myri10ge.c
+++ b/drivers/net/myri10ge/myri10ge.c
@@ -1086,6 +1086,9 @@ static int myri10ge_toggle_relaxed(struct pci_dev *pdev, int on)
return 0;
err = pci_read_config_word(pdev, cap + PCI_EXP_DEVCTL, &ctl);
+ if (err)
+ return 0;
+
ret = (ctl & PCI_EXP_DEVCTL_RELAX_EN) >> 4;
if (ret != on) {
ctl &= ~PCI_EXP_DEVCTL_RELAX_EN;
@@ -1140,20 +1143,19 @@ static void myri10ge_setup_dca(struct myri10ge_priv *mgp)
mgp->ss[i].cpu = -1;
mgp->ss[i].cached_dca_tag = -1;
myri10ge_update_dca(&mgp->ss[i]);
- }
+ }
}
static void myri10ge_teardown_dca(struct myri10ge_priv *mgp)
{
struct pci_dev *pdev = mgp->pdev;
- int err;
if (!mgp->dca_enabled)
return;
mgp->dca_enabled = 0;
if (mgp->relaxed_order)
myri10ge_toggle_relaxed(pdev, 1);
- err = dca_remove_requester(&pdev->dev);
+ dca_remove_requester(&pdev->dev);
}
static int myri10ge_notify_dca_device(struct device *dev, void *data)
@@ -1314,7 +1316,7 @@ myri10ge_unmap_rx_page(struct pci_dev *pdev,
static inline int
myri10ge_rx_done(struct myri10ge_slice_state *ss, int len, __wsum csum,
- int lro_enabled)
+ bool lro_enabled)
{
struct myri10ge_priv *mgp = ss->mgp;
struct sk_buff *skb;
@@ -1474,11 +1476,9 @@ myri10ge_clean_rx_done(struct myri10ge_slice_state *ss, int budget)
{
struct myri10ge_rx_done *rx_done = &ss->rx_done;
struct myri10ge_priv *mgp = ss->mgp;
-
unsigned long rx_bytes = 0;
unsigned long rx_packets = 0;
unsigned long rx_ok;
-
int idx = rx_done->idx;
int cnt = rx_done->cnt;
int work_done = 0;
@@ -1531,16 +1531,14 @@ static inline void myri10ge_check_statblock(struct myri10ge_priv *mgp)
mgp->link_state = link_up;
if (mgp->link_state == MXGEFW_LINK_UP) {
- if (netif_msg_link(mgp))
- netdev_info(mgp->dev, "link up\n");
+ netif_info(mgp, link, mgp->dev, "link up\n");
netif_carrier_on(mgp->dev);
mgp->link_changes++;
} else {
- if (netif_msg_link(mgp))
- netdev_info(mgp->dev, "link %s\n",
- link_up == MXGEFW_LINK_MYRINET ?
+ netif_info(mgp, link, mgp->dev, "link %s\n",
+ (link_up == MXGEFW_LINK_MYRINET ?
"mismatch (Myrinet detected)" :
- "down");
+ "down"));
netif_carrier_off(mgp->dev);
mgp->link_changes++;
}
@@ -1621,7 +1619,7 @@ static irqreturn_t myri10ge_intr(int irq, void *arg)
if (send_done_count != tx->pkt_done)
myri10ge_tx_done(ss, (int)send_done_count);
if (unlikely(i > myri10ge_max_irq_loops)) {
- netdev_err(mgp->dev, "irq stuck?\n");
+ netdev_warn(mgp->dev, "irq stuck?\n");
stats->valid = 0;
schedule_work(&mgp->watchdog_work);
}
@@ -1785,9 +1783,8 @@ static const char myri10ge_gstrings_slice_stats[][ETH_GSTRING_LEN] = {
"----------- slice ---------",
"tx_pkt_start", "tx_pkt_done", "tx_req", "tx_done",
"rx_small_cnt", "rx_big_cnt",
- "wake_queue", "stop_queue", "tx_linearized", "LRO aggregated",
- "LRO flushed",
- "LRO avg aggr", "LRO no_desc"
+ "wake_queue", "stop_queue", "tx_linearized",
+ "LRO aggregated", "LRO flushed", "LRO avg aggr", "LRO no_desc",
};
#define MYRI10GE_NET_STATS_LEN 21
@@ -3329,7 +3326,6 @@ abort:
/* fall back to using the unaligned firmware */
mgp->tx_boundary = 2048;
set_fw_name(mgp, myri10ge_fw_unaligned, false);
-
}
static void myri10ge_select_firmware(struct myri10ge_priv *mgp)
@@ -3715,8 +3711,8 @@ static void myri10ge_free_slices(struct myri10ge_priv *mgp)
dma_free_coherent(&pdev->dev, bytes,
ss->fw_stats, ss->fw_stats_bus);
ss->fw_stats = NULL;
- netif_napi_del(&ss->napi);
}
+ netif_napi_del(&ss->napi);
}
kfree(mgp->ss);
mgp->ss = NULL;
--
1.7.5.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