* [PATCH] net: fix problem in reading sock TX queue
From: Tom Herbert @ 2010-07-15 3:48 UTC (permalink / raw)
To: davem, netdev
Fix problem in reading the tx_queue recorded in a socket. In
dev_pick_tx, the TX queue is read by doing a check with
sk_tx_queue_recorded on the socket, followed by a sk_tx_queue_get.
The problem is that there is not mutual exclusion across these
calls in the socket so it it is possible that the queue in the
sock can be invalidated after sk_tx_queue_recorded is called so
that sk_tx_queue get returns -1, which sets 65535 in queue_index
and thus dev_pick_tx returns 65536 which is a bogus queue and
can cause crash in dev_queue_xmit.
We fix this by only calling sk_tx_queue_get which does the proper
checks. The interface is that sk_tx_queue_get returns the TX queue
if the sock argument is non-NULL and TX queue is recorded, else it
returns -1. sk_tx_queue_recorded is no longer used so it can be
completely removed.
Signed-off-by: Tom Herbert <therbert@google.com>
---
diff --git a/include/net/sock.h b/include/net/sock.h
index 3100e71..a441c9c 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1226,12 +1226,7 @@ static inline void sk_tx_queue_clear(struct sock *sk)
static inline int sk_tx_queue_get(const struct sock *sk)
{
- return sk->sk_tx_queue_mapping;
-}
-
-static inline bool sk_tx_queue_recorded(const struct sock *sk)
-{
- return (sk && sk->sk_tx_queue_mapping >= 0);
+ return sk ? sk->sk_tx_queue_mapping : -1;
}
static inline void sk_set_socket(struct sock *sk, struct socket *sock)
diff --git a/net/core/dev.c b/net/core/dev.c
index e2b9fa2..f071252 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2053,12 +2053,11 @@ static inline u16 dev_cap_txqueue(struct net_device *dev, u16 queue_index)
static struct netdev_queue *dev_pick_tx(struct net_device *dev,
struct sk_buff *skb)
{
- u16 queue_index;
+ int queue_index;
struct sock *sk = skb->sk;
- if (sk_tx_queue_recorded(sk)) {
- queue_index = sk_tx_queue_get(sk);
- } else {
+ queue_index = sk_tx_queue_get(sk);
+ if (queue_index < 0) {
const struct net_device_ops *ops = dev->netdev_ops;
if (ops->ndo_select_queue) {
^ permalink raw reply related
* RE: Splice status
From: Ofer Heifetz @ 2010-07-15 3:47 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Changli Gao, Jens Axboe, netdev@vger.kernel.org
In-Reply-To: <1279030308.2634.349.camel@edumazet-laptop>
Hi,
I managed to get splice use up to 64K which look to me as a samba limitation (smb.conf SO_RCVBUF limitation I think) but still do not get any performance improvement using splice, the write numbers for splice are in about the same as for regular read/write though refraining from copy_to_user and copy_from_user.
-Ofer
-----Original Message-----
From: Eric Dumazet [mailto:eric.dumazet@gmail.com]
Sent: Tuesday, July 13, 2010 5:12 PM
To: Ofer Heifetz
Cc: Changli Gao; Jens Axboe; netdev@vger.kernel.org
Subject: RE: Splice status
Le mardi 13 juillet 2010 à 14:41 +0300, Ofer Heifetz a écrit :
> Hi,
>
> I wanted to let you know that I have been testing Samba splice on Marvell 6282 SoC on 2.6.35_rc3 and noticed that it gave worst performance than not using it and also noticed that on re-writing file the iowait is high.
>
> iometer using 2G file (file is created before test)
>
> Splice write cpu% iow%
> -----------------------
> No 58 98 0
> Yes 14 100 48
>
> iozone using 2G file (file created during test)
>
> Splice write cpu% iow% re-write cpu% iow%
> -------------------------------------------
> No 35 85 4 58.2 70 0
> Yes 33 85 4 15.7 100 58
>
> Any clue why splice introduces a high iowait?
> I noticed samba uses up to 16K per splice syscall, changing the samba to try more did not help, so I guess it is a kernel limitation.
>
splice(socket -> pipe) provides partial buffers (depending on the MTU)
With typical MTU=1500 and tcp timestamps, each network frame contains
1448 bytes of payload, partially filling one page (of 4096 bytes)
When doing the splice(pipe -> file), kernel has to coalesce partial
data, but amount of written data per syscall() is small (about 20
Kbytes)
Without splice(), the write() syscall provides more data, and vfs
overhead is smaller as buffer size is a power of two.
Samba uses a 128 KBytes TRANSFER_BUF_SIZE in its default_sys_recvfile()
implementation, it easily outperforms splice() implementation.
You could try extending pipe size (fcntl(fd, F_SETPIPE_SZ, 256)), maybe
it will be a bit better. (and ask 256*4096 bytes to splice())
I tried this and got about 256Kbytes per splice() call...
# perf report
# Events: 13K
#
# Overhead Command Shared Object Symbol
# ........ .............. ................. ......
#
8.69% splice-fromnet [kernel.kallsyms] [k] memcpy
3.82% splice-fromnet [kernel.kallsyms] [k] kunmap_atomic
3.51% splice-fromnet [kernel.kallsyms] [k] __block_prepare_write
2.79% splice-fromnet [kernel.kallsyms] [k] __skb_splice_bits
2.58% splice-fromnet [kernel.kallsyms] [k] ext3_mark_iloc_dirty
2.45% splice-fromnet [kernel.kallsyms] [k] do_get_write_access
2.04% splice-fromnet [kernel.kallsyms] [k] __find_get_block
1.89% splice-fromnet [kernel.kallsyms] [k] _raw_spin_lock
1.83% splice-fromnet [kernel.kallsyms] [k] journal_add_journal_head
1.46% splice-fromnet [bnx2x] [k] bnx2x_rx_int
1.46% splice-fromnet [kernel.kallsyms] [k] kfree
1.42% splice-fromnet [kernel.kallsyms] [k] journal_put_journal_head
1.29% splice-fromnet [kernel.kallsyms] [k] __ext3_get_inode_loc
1.26% splice-fromnet [kernel.kallsyms] [k] journal_dirty_metadata
1.25% splice-fromnet [kernel.kallsyms] [k] page_address
1.20% splice-fromnet [kernel.kallsyms] [k] journal_cancel_revoke
1.15% splice-fromnet [kernel.kallsyms] [k] tcp_read_sock
1.09% splice-fromnet [kernel.kallsyms] [k] unlock_buffer
1.09% splice-fromnet [kernel.kallsyms] [k] pipe_to_file
1.05% splice-fromnet [kernel.kallsyms] [k] radix_tree_lookup_element
1.04% splice-fromnet [kernel.kallsyms] [k] kmap_atomic_prot
1.04% splice-fromnet [kernel.kallsyms] [k] kmem_cache_free
1.03% splice-fromnet [kernel.kallsyms] [k] kmem_cache_alloc
1.01% splice-fromnet [bnx2x] [k] bnx2x_poll
^ permalink raw reply
* Re: Raise initial congestion window size / speedup slow start?
From: Bill Fink @ 2010-07-15 3:49 UTC (permalink / raw)
To: Hagen Paul Pfeifer
Cc: David Miller, rick.jones2, lists, davidsen, linux-kernel, netdev
In-Reply-To: <20100714221301.GI6682@nuttenaction>
On Thu, 15 Jul 2010, Hagen Paul Pfeifer wrote:
> * David Miller | 2010-07-14 14:55:47 [-0700]:
>
> >Although section 3 of RFC 5681 is a great text, it does not say at all
> >that increasing the initial CWND would lead to fairness issues.
>
> Because it is only one side of the medal, probing conservative the available
> link capacity in conjunction with n simultaneous probing TCP/SCTP/DCCP
> instances is another.
>
> >To be honest, I think google's proposal holds a lot of weight. If
> >over time link sizes and speeds are increasing (they are) then nudging
> >the initial CWND every so often is a legitimate proposal. Were
> >someone to claim that utilization is lower than it could be because of
> >the currenttly specified initial CWND, I would have no problem
> >believing them.
> >
> >And I'm happy to make Linux use an increased value once it has
> >traction in the standardization community.
>
> Currently I know no working link capacity probing approach, without active
> network feedback, to conservatively probing the available link capacity with a
> high CWND. I am curious about any future trends.
A long, long time ago, I suggested a Path BW Discovery mechanism
to the IETF, analogous to the Path MTU Discovery mechanism, but
it didn't get any traction. Such information could be extremely
useful to TCP endpoints, to determine a maximum window size to
use, to effectively rate limit a much stronger sender from
overpowering a much weaker receiver (for example 10-GigE -> GigE),
resulting in abominable performance across large RTT paths
(as low as 12 Mbps), even in the absence of any real network
contention.
-Bill
^ permalink raw reply
* Re: [PATCH] net: fix problem in reading sock TX queue
From: David Miller @ 2010-07-15 3:50 UTC (permalink / raw)
To: therbert; +Cc: netdev
In-Reply-To: <alpine.DEB.1.00.1007142025430.22782@pokey.mtv.corp.google.com>
From: Tom Herbert <therbert@google.com>
Date: Wed, 14 Jul 2010 20:48:08 -0700 (PDT)
> Fix problem in reading the tx_queue recorded in a socket. In
> dev_pick_tx, the TX queue is read by doing a check with
> sk_tx_queue_recorded on the socket, followed by a sk_tx_queue_get.
> The problem is that there is not mutual exclusion across these
> calls in the socket so it it is possible that the queue in the
> sock can be invalidated after sk_tx_queue_recorded is called so
> that sk_tx_queue get returns -1, which sets 65535 in queue_index
> and thus dev_pick_tx returns 65536 which is a bogus queue and
> can cause crash in dev_queue_xmit.
>
> We fix this by only calling sk_tx_queue_get which does the proper
> checks. The interface is that sk_tx_queue_get returns the TX queue
> if the sock argument is non-NULL and TX queue is recorded, else it
> returns -1. sk_tx_queue_recorded is no longer used so it can be
> completely removed.
>
> Signed-off-by: Tom Herbert <therbert@google.com>
Applied, thanks Tom!
^ permalink raw reply
* Re: [PATCH] fec: use interrupt for MDIO completion indication
From: Baruch Siach @ 2010-07-15 4:09 UTC (permalink / raw)
To: Bryan Wu; +Cc: netdev, linux-arm-kernel, Sascha Hauer, Greg Ungerer,
Wolfram Sang
In-Reply-To: <4C3E812C.10303@canonical.com>
Hi Bryan,
On Thu, Jul 15, 2010 at 11:31:56AM +0800, Bryan Wu wrote:
> Thanks for this patch, we tested on our i.MX51 board with Ubuntu. It works
> fine.
>
> Wolfram, you can pick up this, too. -;)
Dave has already applied this patch to his net-next tree.
baruch
> On 07/12/2010 03:12 PM, Baruch Siach wrote:
> > With the move to phylib (commit e6b043d) I was seeing sporadic "MDIO write
> > timeout" messages. Measure of the actual time spent showed latency times of
> > more than 1600us.
> >
> > This patch uses the MII event indication of the FEC hardware to detect
> > completion of MDIO transactions.
> >
> > Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> > ---
> > drivers/net/fec.c | 55 ++++++++++++++++++++++++----------------------------
> > 1 files changed, 25 insertions(+), 30 deletions(-)
> >
--
~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch@tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
^ permalink raw reply
* Re: Raise initial congestion window size / speedup slow start?
From: Tom Herbert @ 2010-07-15 4:12 UTC (permalink / raw)
To: Hagen Paul Pfeifer
Cc: Rick Jones, Ed W, David Miller, davidsen, linux-kernel, netdev,
Jerry Chu, Nandita Dukkipati
In-Reply-To: <20100714203919.GD6682@nuttenaction>
On Wed, Jul 14, 2010 at 1:39 PM, Hagen Paul Pfeifer <hagen@jauu.net> wrote:
> * Rick Jones | 2010-07-14 13:17:24 [-0700]:
>
>>There is an effort under way, lead by some folks at Google and
>>including some others, to get the RFC's enhanced in support of the
>>concept of larger initial congestion windows. Some of the discussion
>>may be in the "tcpm" mailing list (assuming I've not gotten my
>>mailing lists confused). There may be some previous discussion of
>>that work in the netdev archives as well.
>
> tcpm is the right mailing list but there is currently no effort to develop
> this topic. Why? Because is not a standardization issue, rather it is a
> technical issue. You cannot rise the initial CWND and expect a fair behavior.
> This was discussed several times and is documented in several documents and
> RFCs.
>
> RFC 5681 Section 3.1. Google employees should start with Section 3. This topic
> pop's of every two months in netdev and until now I _never_ read a
> consolidated contribution.
>
There is an Internet draft
(http://datatracker.ietf.org/doc/draft-hkchu-tcpm-initcwnd/) on
raising the default Initial Congestion window to 10 segments, as well
as a SIGCOMM paper (http://ccr.sigcomm.org/online/?q=node/621). We
presented this proposal and data supporting it at Anaheim IETF, and
will be following up in Netherlands with more data including some of
which should further address fairness questions.
In terms of Linux implementation, setting ICW via ip route is
sufficient support on the server side. There is also a proposed patch
which could allow applications to set ICW themselves (in hopes that
application can reduce number of simultaneous connections). On the
client side we can now adjust the receive window to advertise larger
initial windows. Among current implementations, Linux advertises the
smallest default receive window of major OSes, so it turns out Linux
clients won't get lower latency benefits currently (so we'll probably
ask to raise the default some day :-)).
Tom
> Partial local issues can already be "fixed" via route specific ip options -
> see initcwnd.
>
> HGN
>
>
>
>
>
>
> --
> 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: Raise initial congestion window size / speedup slow start?
From: H.K. Jerry Chu @ 2010-07-15 4:51 UTC (permalink / raw)
To: David Miller; +Cc: davidsen, lists, linux-kernel, netdev
In-Reply-To: <20100714.111553.104052157.davem@davemloft.net>
On Wed, Jul 14, 2010 at 11:15 AM, David Miller <davem@davemloft.net> wrote:
> From: Bill Davidsen <davidsen@tmr.com>
> Date: Wed, 14 Jul 2010 11:21:15 -0400
>
>> You may have to go into /proc/sys/net/core and crank up the
>> rmem_* settings, depending on your distribution.
>
> You should never, ever, have to touch the various networking sysctl
> values to get good performance in any normal setup. If you do, it's a
> bug, report it so we can fix it.
Agreed, except there are indeed bugs in the code today in that the
code in various places assumes initcwnd as per RFC3390. So when
initcwnd is raised, that actual value may be limited unnecessarily by
the initial wmem/sk_sndbuf.
Will try to find time to submit a patch.
Jerry
>
> I cringe every time someone says to do this, so please do me a favor
> and don't spread this further. :-)
>
> For one thing, TCP dynamically adjusts the socket buffer sizes based
> upon the behavior of traffic on the connection.
>
> And the TCP memory limit sysctls (not the core socket ones) are sized
> based upon available memory. They are there to protect you from
> situations such as having so much memory dedicated to socket buffers
> that there is none left to do other things effectively. It's a
> protective limit, rather than a setting meant to increase or improve
> performance. So like the others, leave these alone too.
> --
> 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: Raise initial congestion window size / speedup slow start?
From: H.K. Jerry Chu @ 2010-07-15 5:09 UTC (permalink / raw)
To: Hagen Paul Pfeifer
Cc: Rick Jones, Ed W, David Miller, davidsen, linux-kernel, netdev
In-Reply-To: <20100714203919.GD6682@nuttenaction>
On Wed, Jul 14, 2010 at 1:39 PM, Hagen Paul Pfeifer <hagen@jauu.net> wrote:
> * Rick Jones | 2010-07-14 13:17:24 [-0700]:
>
>>There is an effort under way, lead by some folks at Google and
>>including some others, to get the RFC's enhanced in support of the
>>concept of larger initial congestion windows. Some of the discussion
>>may be in the "tcpm" mailing list (assuming I've not gotten my
>>mailing lists confused). There may be some previous discussion of
>>that work in the netdev archives as well.
>
> tcpm is the right mailing list but there is currently no effort to develop
> this topic. Why? Because is not a standardization issue, rather it is a
Please don't mislead. Raising the initcwnd is actively being pursued at IETF
right now. If not here, where else? It is following the same path where initcwnd
was first raised in late 90' through rfc2414/rfc3390.
IETF is not a standard organization just for protocol lawyers to play
word games.
It is responsible for solving real technical issues as well.
Jerry
> technical issue. You cannot rise the initial CWND and expect a fair behavior.
> This was discussed several times and is documented in several documents and
> RFCs.
>
> RFC 5681 Section 3.1. Google employees should start with Section 3. This topic
> pop's of every two months in netdev and until now I _never_ read a
> consolidated contribution.
>
> Partial local issues can already be "fixed" via route specific ip options -
> see initcwnd.
>
> HGN
>
>
>
>
>
>
> --
> 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 net-next-2.6] xfrm: cleanup of xfrm_input.c. (resend)
From: Rami Rosen @ 2010-07-15 5:21 UTC (permalink / raw)
To: David Miller; +Cc: netdev
[-- Attachment #1: Type: text/plain, Size: 997 bytes --]
Hi,
The patch removes unneeded inclusion of header files
(linux/module.h, linux/netdevice.h, net/dst.h and net/ip.h) and adds inclusion
of linux/skbuff.h instead, in net/xfrm/xfrm_input.c.
Regards,
Rami Rosen
On Thu, Jul 15, 2010 at 3:59 AM, David Miller <davem@davemloft.net> wrote:
> From: Rami Rosen <ramirose@gmail.com>
> Date: Wed, 14 Jul 2010 11:18:41 +0300
>
>> Hi,
>> The patch removes unneeded inclusion of header files
>> (linux/module.h, linux/netdevice.h, net/dst.h and net/ip.h)
>> in net/xfrm/xfrm_input.c
>>
>> Regards,
>> Rami Rosen
>>
>> Signed-off-by: Rami Rosen <ramirose@gmail.com>
>
> If you do this, I also want to see you add includes for things like
> linux/skbuff.h since data structures such as "struct sk_buff"
> are used in this file.
>
> Otherwise, this is how we end up with obscure build failures on
> some configurations and not others, either now or in the future
> when a similar change is made to some header file.
>
>
[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 464 bytes --]
diff --git a/net/xfrm/xfrm_input.c b/net/xfrm/xfrm_input.c
index 45f1c98..c87aec8 100644
--- a/net/xfrm/xfrm_input.c
+++ b/net/xfrm/xfrm_input.c
@@ -6,12 +6,9 @@
* Split up af-specific portion
*
*/
-
+
+#include <linux/skbuff.h>
#include <linux/slab.h>
-#include <linux/module.h>
-#include <linux/netdevice.h>
-#include <net/dst.h>
-#include <net/ip.h>
#include <net/xfrm.h>
static struct kmem_cache *secpath_cachep __read_mostly;
^ permalink raw reply related
* RE: [PATCHv3 NEXT 0/5]qlcnic: aer state
From: Amit Salecha @ 2010-07-15 5:24 UTC (permalink / raw)
To: David Miller; +Cc: netdev@vger.kernel.org, Ameen Rahman
In-Reply-To: <20100714.103147.193705183.davem@davemloft.net>
Thanks.
-----Original Message-----
From: David Miller [mailto:davem@davemloft.net]
Sent: Wednesday, July 14, 2010 11:02 PM
To: Amit Salecha
Cc: netdev@vger.kernel.org; Ameen Rahman
Subject: Re: [PATCHv3 NEXT 0/5]qlcnic: aer state
From: amit.salecha@qlogic.com
Date: Tue, 13 Jul 2010 23:33:30 -0700
> I was in under impression that using space after tab is illegal.
What's undesirable is using spaces instead of tabs for fresh
code-block lines like:
foo();
Also, it is undesirable to have spaces mixed into the middle of a
sequence of tab characters. The spaces, when used to align expression
continuation lines or lists of function arguments, should be at the
end.
So in cases like:
if (foo &&
bar)
The second line should be a tab character, then the spaces to make
the alignment happen.
^ permalink raw reply
* Re: Raise initial congestion window size / speedup slow start?
From: H.K. Jerry Chu @ 2010-07-15 5:29 UTC (permalink / raw)
To: Bill Fink
Cc: Hagen Paul Pfeifer, David Miller, rick.jones2, lists, davidsen,
linux-kernel, netdev
In-Reply-To: <20100714234917.924f420d.billfink@mindspring.com>
On Wed, Jul 14, 2010 at 8:49 PM, Bill Fink <billfink@mindspring.com> wrote:
> On Thu, 15 Jul 2010, Hagen Paul Pfeifer wrote:
>
>> * David Miller | 2010-07-14 14:55:47 [-0700]:
>>
>> >Although section 3 of RFC 5681 is a great text, it does not say at all
>> >that increasing the initial CWND would lead to fairness issues.
>>
>> Because it is only one side of the medal, probing conservative the available
>> link capacity in conjunction with n simultaneous probing TCP/SCTP/DCCP
>> instances is another.
>>
>> >To be honest, I think google's proposal holds a lot of weight. If
>> >over time link sizes and speeds are increasing (they are) then nudging
>> >the initial CWND every so often is a legitimate proposal. Were
>> >someone to claim that utilization is lower than it could be because of
>> >the currenttly specified initial CWND, I would have no problem
>> >believing them.
>> >
>> >And I'm happy to make Linux use an increased value once it has
>> >traction in the standardization community.
>>
>> Currently I know no working link capacity probing approach, without active
>> network feedback, to conservatively probing the available link capacity with a
>> high CWND. I am curious about any future trends.
>
> A long, long time ago, I suggested a Path BW Discovery mechanism
> to the IETF, analogous to the Path MTU Discovery mechanism, but
> it didn't get any traction. Such information could be extremely
> useful to TCP endpoints, to determine a maximum window size to
> use, to effectively rate limit a much stronger sender from
> overpowering a much weaker receiver (for example 10-GigE -> GigE),
> resulting in abominable performance across large RTT paths
> (as low as 12 Mbps), even in the absence of any real network
> contention.
Unfortunately that is not going to help initcwnd (unless one can invent a
PBWD protocol from just 3WHS), and the web is dominated by short-lived
connections so the small initcwnd becomes a choke point.
Jerry
>
> -Bill
> --
> 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 repost] sched: export sched_set/getaffinity to modules
From: Sridhar Samudrala @ 2010-07-15 5:29 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Michael S. Tsirkin, Peter Zijlstra, Tejun Heo, Ingo Molnar,
netdev, lkml, kvm@vger.kernel.org, Andrew Morton, Dmitri Vorobiev,
Jiri Kosina, Thomas Gleixner, Andi Kleen
In-Reply-To: <20100715000506.GB27258@redhat.com>
On 7/14/2010 5:05 PM, Oleg Nesterov wrote:
> On 07/14, Sridhar Samudrala wrote:
>
>> OK. So we want to create a thread that is a child of kthreadd, but inherits the cgroup/cpumask
>> from the caller. How about an exported kthread function kthread_create_in_current_cg()
>> that does this?
>>
> Well. I must admit, this looks a bit strange to me ;)
>
> Instead of exporting sched_xxxaffinity() we export the new function
> which calls them. And I don't think this new helper is very useful
> in general. May be I am wrong...
>
If we agree on exporting sched_xxxaffinity() functions, we don't need
this new kthread function and we
can do the same in vhost as the original patch did.
Thanks
Sridhar
^ permalink raw reply
* Re: [PATCH net-next-2.6] xfrm: cleanup of xfrm_input.c. (resend)
From: David Miller @ 2010-07-15 5:42 UTC (permalink / raw)
To: ramirose; +Cc: netdev
In-Reply-To: <AANLkTim7fVRtycf2B07iI7U4chv2NqD267dvunZJYqPj@mail.gmail.com>
From: Rami Rosen <ramirose@gmail.com>
Date: Thu, 15 Jul 2010 08:21:03 +0300
> Hi,
> The patch removes unneeded inclusion of header files
> (linux/module.h, linux/netdevice.h, net/dst.h and net/ip.h) and adds inclusion
> of linux/skbuff.h instead, in net/xfrm/xfrm_input.c.
Rami, please don't process my feedback like an automaton. :-/
I specifically mentioned linux/skbuff.h because that happened to be
the very first obvious header dependency I found in the xfrm_input.c
file.
There are almost certainly others, and I'd like for you to take the
time to skim over the file and figure out what those might be.
Also, when submitting a new version of a patch, do not hijack the
existing thread. Instead, make a full fresh submission with a
plain fresh subject line, commit log message, signoff, and patch.
^ permalink raw reply
* Re: [5/5] Remove REDWOOD_5 and REDWOOD_6 config options and conditional code
From: Milton Miller @ 2010-07-15 7:42 UTC (permalink / raw)
To: Christian Dietrich
Cc: Josh Boyer, Matt Porter, Benjamin Herrenschmidt, Paul Mackerras,
Solomon Peachy, David Woodhouse, Mike Frysinger, Jiri Kosina,
Artem Bityutskiy, Alexander Kurz, John Linn, David S. Miller,
Randy Dunlap, Florian Fainelli, linuxppc-dev, linux-kernel,
linux-mtd, netdev, vamos-dev
In-Reply-To: <4f07b3092cafbbba37d61d367cc7484e24d18d2a.1279116162.git.qy03fugy@stud.informatik.uni-erlangen.de>
On Wed, 14 Jul 2010 about 04:05:05 -0000, Christian Dietrich wrote:
>
> The config options for REDWOOD_[56] were commented out in the powerpc
> Kconfig. The ifdefs referencing this options therefore are dead and all
> references to this can be removed (Also dependencies in other KConfig
> files).
>
> Signed-off-by: Christian Dietrich <qy03fugy@stud.informatik.uni-erlangen.de>
>
> ---
> arch/powerpc/platforms/40x/Kconfig | 16 -------------
> drivers/mtd/maps/Kconfig | 2 +-
> drivers/mtd/maps/redwood.c | 43 ------------------------------------
> drivers/net/Kconfig | 2 +-
> 4 files changed, 2 insertions(+), 61 deletions(-)
> diff --git a/drivers/mtd/maps/Kconfig b/drivers/mtd/maps/Kconfig
> index f22bc9f..b5ebb72 100644
> --- a/drivers/mtd/maps/Kconfig
> +++ b/drivers/mtd/maps/Kconfig
> @@ -321,7 +321,7 @@ config MTD_CFI_FLAGADM
>
> config MTD_REDWOOD
> tristate "CFI Flash devices mapped on IBM Redwood"
> - depends on MTD_CFI && ( REDWOOD_4 || REDWOOD_5 || REDWOOD_6 )
> + depends on MTD_CFI && REDWOOD_4
> help
> This enables access routines for the flash chips on the IBM
> Redwood board. If you have one of these boards and would like to
REDWOOD_4 does not appear to be in the tree either so this mapping driver
should be deleted if the patch is otherwise acceptable. Besides we
would express the info contained in this simple map driver the device tree
using physmap_of.
milton
^ permalink raw reply
* Re: Raise initial congestion window size / speedup slow start?
From: Ed W @ 2010-07-15 7:48 UTC (permalink / raw)
To: Tom Herbert
Cc: Hagen Paul Pfeifer, Rick Jones, David Miller, davidsen,
linux-kernel, netdev, Jerry Chu, Nandita Dukkipati
In-Reply-To: <AANLkTikTyZGQWWIBzYSQWpUK30xxbMXLbJXeHahWnlIi@mail.gmail.com>
On 15/07/2010 05:12, Tom Herbert wrote:
> There is an Internet draft
> (http://datatracker.ietf.org/doc/draft-hkchu-tcpm-initcwnd/) on
> raising the default Initial Congestion window to 10 segments, as well
> as a SIGCOMM paper (http://ccr.sigcomm.org/online/?q=node/621).
>
You guys have obviously done a lot of work on this, however, it seems
that there is a case for introducing some heuristics into the choice of
init cwnd as well as offering the option to go larger? An initial size
of 10 packets is just another magic number that obviously works with the
median bandwidth delay product on today's networks - can we not do
better still?
Seems like a bunch of clever folks have already suggested tweaks to the
steady stage congestion avoidance, but so far everyone is afraid to
touch the early stage heuristics?
Also would you guys not benefit from wider deployment of ECN? Can you
not help find some ways that deployment could be increased? At present
there are big warnings all over the option that it causes some problems,
but there is no quantification of how much and really whether this
warning is still appropriate?
Ed W
^ permalink raw reply
* Re: [PATCH 01/11] Removing dead RT2800PCI_SOC
From: Bartlomiej Zolnierkiewicz @ 2010-07-15 8:41 UTC (permalink / raw)
To: Felix Fietkau
Cc: John W. Linville, Ivo Van Doorn, Christoph Egger,
Gertjan van Wingerde, Helmut Schaa,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
users-poMEt7QlJxcwIE2E9O76wjtx2kNaKg5H,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
vamos-dev-DSeRVwK2yFR7NQr57o4jwX20dTPRyWU8FLXUG6abMr4,
Luis Correia
In-Reply-To: <4C3DCD5C.1080705-p3rKhJxN3npAfugRpC6u6w@public.gmane.org>
On Wednesday 14 July 2010 04:44:44 pm Felix Fietkau wrote:
> On 2010-07-14 3:15 PM, John W. Linville wrote:
> > On Wed, Jul 14, 2010 at 02:52:14PM +0200, Ivo Van Doorn wrote:
> >> On Wed, Jul 14, 2010 at 2:46 PM, Luis Correia <luis.f.correia-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >> > On Wed, Jul 14, 2010 at 13:39, Christoph Egger <siccegge-n+aIp8eCc/CzQB+pC5nmwQ@public.gmane.org> wrote:
> >> >> While RT2800PCI_SOC exists in Kconfig, it depends on either
> >> >> RALINK_RT288X or RALINK_RT305X which are both not available in Kconfig
> >> >> so all Code depending on that can't ever be selected and, if there's
> >> >> no plan to add these options, should be cleaned up
> >> >>
> >> >> Signed-off-by: Christoph Egger <siccegge-n+aIp8eCc/CzQB+pC5nmwQ@public.gmane.org>
> >> >
> >> > NAK,
> >> >
> >> > this is not dead code, it is needed for the Ralink System-on-Chip
> >> > Platform devices.
> >> >
> >> > While I can't fix Kconfig errors and the current KConfig file may be
> >> > wrong, this code cannot and will not be deleted.
> >>
> >> When the config option was introduced, the config options RALINK_RT288X and
> >> RALINK_RT305X were supposed to be merged as well soon after by somebody (Felix?)
> >>
> >> But since testing is done on SoC boards by Helmut and Felix, I assume the code
> >> isn't dead but actually in use.
> >
> > Perhaps Helmut and Felix can send us the missing code?
> The missing code is a MIPS platform port, which is currently being
> maintained in OpenWrt, but is not ready for upstream submission yet.
> I'm not working on this code at the moment, but I think it will be
> submitted once it's ready.
People are using automatic scripts to catch unused config options nowadays
so the issue is quite likely to come back again sooner or later..
Would it be possible to improve situation somehow till the missing parts
get merged? Maybe by adding a tiny comment documenting RT2800PCI_SOC
situation to Kconfig (if the config option itself really cannot be removed)
until all code is ready etc.?
I bet that Christoph would be willing to update his patch if you ask him
nicely..
Thanks,
--
Bartlomiej Zolnierkiewicz
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" 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] netfilter: xtables: userspace notification target
From: Patrick McHardy @ 2010-07-15 9:05 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: Luciano Coelho, Changli Gao, Samuel Ortiz, David S. Miller,
netdev@vger.kernel.org, netfilter-devel@vger.kernel.org
In-Reply-To: <4C3DE715.8070502@netfilter.org>
Am 14.07.2010 18:34, schrieb Pablo Neira Ayuso:
> Hi Luciano,
>
> On 14/07/10 14:22, Luciano Coelho wrote:
>> On Wed, 2010-07-14 at 13:48 +0200, ext Patrick McHardy wrote:
>>> If you're using connection tracking, you can use conntrack marks
>>> to avoid sending more than a single message:
>>>
>>> iptables ... -m connmark --mark 0x1/0x1 -j RETURN
>>> iptables ... -j NFLOG ...
>>> iptables ... -j CONNMARK --set-mark 0x1/0x1
>>
>> Cool, thanks.
>>
>> It seems that there are lots of possibilities to get this to work, but
>> this is starting to get quite complex. I would still prefer having the
>> NFNOTIF module included, since we would be able to do what we want in a
>> very simple way. It's also probably much more efficient that using
>> several rules, which would increase the CPU usage considerably (in our
>> device we are already reaching the limit of a reasonable CPU resource
>> usage with high throughput WLAN connections).
Its hard to believe that a connmark match filtering out notifications
would require more CPU time than doing the same in a new target module.
>> While I agree that it is possible to achieve the NFNOTIF functionality
>> with existing modules, I still think there is a "niche" for such module,
>> because it is very simple, has a very clear purpose and would make the
>> ruleset simpler and more efficient.
>>
>> Does this make any sense?
>
> I don't think that the NFNOTIF infrastructure fulfill the policy for
> inclusion. It seems to me like something quite specific for your needs.
> It is simple, yes, but we already have this feature into the kernel. I
> don't think that this will reduce CPU usage considerably with regards to
> the NFLOG way.
>
> I would still prefer adding the once-per-matching notification feature
> to NFLOG than these extra lines in the kernel, Patrick?
I agree with Pablo.
^ permalink raw reply
* Re: [PATCH] netfilter: xtables: userspace notification target
From: Luciano Coelho @ 2010-07-15 9:18 UTC (permalink / raw)
To: ext Patrick McHardy
Cc: Pablo Neira Ayuso, Changli Gao, Samuel Ortiz, David S. Miller,
netdev@vger.kernel.org, netfilter-devel@vger.kernel.org
In-Reply-To: <4C3ECF6D.50409@trash.net>
On Thu, 2010-07-15 at 11:05 +0200, ext Patrick McHardy wrote:
> Am 14.07.2010 18:34, schrieb Pablo Neira Ayuso:
> >> It seems that there are lots of possibilities to get this to work, but
> >> this is starting to get quite complex. I would still prefer having the
> >> NFNOTIF module included, since we would be able to do what we want in a
> >> very simple way. It's also probably much more efficient that using
> >> several rules, which would increase the CPU usage considerably (in our
> >> device we are already reaching the limit of a reasonable CPU resource
> >> usage with high throughput WLAN connections).
>
> Its hard to believe that a connmark match filtering out notifications
> would require more CPU time than doing the same in a new target module.
Okay, you have convinced me. :) I studied connmark a bit more and now I
realize that it won't take more CPU. In the solution with connmark that
you proposed the packets coming from a connection that is already marked
will be quickly returned to normal processing, so it will be fairly
efficient and certainly not more CPU hungry than the NFNOTIF.
> >> While I agree that it is possible to achieve the NFNOTIF functionality
> >> with existing modules, I still think there is a "niche" for such module,
> >> because it is very simple, has a very clear purpose and would make the
> >> ruleset simpler and more efficient.
> >>
> >> Does this make any sense?
> >
> > I don't think that the NFNOTIF infrastructure fulfill the policy for
> > inclusion. It seems to me like something quite specific for your needs.
> > It is simple, yes, but we already have this feature into the kernel. I
> > don't think that this will reduce CPU usage considerably with regards to
> > the NFLOG way.
> >
> > I would still prefer adding the once-per-matching notification feature
> > to NFLOG than these extra lines in the kernel, Patrick?
>
> I agree with Pablo.
I have to admit that you're right here again. I think that it will not
be necessary to make this change in the NFLOG, since the connmark
solution is actually pretty clear too. If needed, I'll make this simple
change in the NFLOG module and submit.
Thanks for your help.
--
Cheers,
Luca.
^ permalink raw reply
* Re: [PATCHv2] netfilter: add CHECKSUM target
From: Patrick McHardy @ 2010-07-15 9:36 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: David S. Miller, Jan Engelhardt, Randy Dunlap, netfilter-devel,
netfilter, coreteam, linux-kernel, netdev, kvm, herbert
In-Reply-To: <20100711150623.GA7420@redhat.com>
Am 11.07.2010 17:06, schrieb Michael S. Tsirkin:
> +#ifndef _IPT_CHECKSUM_TARGET_H
> +#define _IPT_CHECKSUM_TARGET_H
> +
> +#define XT_CHECKSUM_OP_FILL 0x01 /* fill in checksum in IP header */
> +
> +struct xt_CHECKSUM_info {
> + u_int8_t operation; /* bitset of operations */
Please use __u8 in public header files.
> +};
> +
> +#endif /* _IPT_CHECKSUM_TARGET_H */
> diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
> index 8593a77..1cf4852 100644
> --- a/net/netfilter/Kconfig
> +++ b/net/netfilter/Kconfig
> @@ -294,7 +294,7 @@ endif # NF_CONNTRACK
> config NETFILTER_TPROXY
> tristate "Transparent proxying support (EXPERIMENTAL)"
> depends on EXPERIMENTAL
> - depends on IP_NF_MANGLE
> + depends on IP_NF_MANGLE || IP6_NF_MANGLE
This does not seem to belong into this patch.
> depends on NETFILTER_ADVANCED
> help
> This option enables transparent proxying support, that is,
> @@ -347,6 +347,21 @@ config NETFILTER_XT_CONNMARK
>
> comment "Xtables targets"
>
> +config NETFILTER_XT_TARGET_CHECKSUM
> + tristate "CHECKSUM target support"
> + depends on NETFILTER_ADVANCED
> + ---help---
> + This option adds a `CHECKSUM' target, which can be used in the iptables mangle
> + table.
You should add a dependency on the mangle table then.
> +
> + You can use this target to compute and fill in the checksum in
> + a packet that lacks a checksum. This is particularly useful,
> + if you need to work around old applications such as dhcp clients,
> + that do not work well with checksum offloads, but don't want to disable
> + checksum offload in your device.
> +
> + To compile it as a module, choose M here. If unsure, say N.
> +
> config NETFILTER_XT_TARGET_CLASSIFY
> tristate '"CLASSIFY" target support'
> depends on NETFILTER_ADVANCED
> diff --git a/net/netfilter/Makefile b/net/netfilter/Makefile
> index 14e3a8f..8eb541d 100644
> --- a/net/netfilter/Makefile
> +++ b/net/netfilter/Makefile
> @@ -45,6 +45,7 @@ obj-$(CONFIG_NETFILTER_XT_MARK) += xt_mark.o
> obj-$(CONFIG_NETFILTER_XT_CONNMARK) += xt_connmark.o
>
> # targets
> +obj-$(CONFIG_NETFILTER_XT_TARGET_CHECKSUM) += xt_CHECKSUM.o
> obj-$(CONFIG_NETFILTER_XT_TARGET_CLASSIFY) += xt_CLASSIFY.o
> obj-$(CONFIG_NETFILTER_XT_TARGET_CONNSECMARK) += xt_CONNSECMARK.o
> obj-$(CONFIG_NETFILTER_XT_TARGET_CT) += xt_CT.o
> diff --git a/net/netfilter/xt_CHECKSUM.c b/net/netfilter/xt_CHECKSUM.c
> new file mode 100644
> index 0000000..0fee1a7
> --- /dev/null
> +++ b/net/netfilter/xt_CHECKSUM.c
> @@ -0,0 +1,72 @@
> +/* iptables module for the packet checksum mangling
> + *
> + * (C) 2002 by Harald Welte <laforge@netfilter.org>
> + * (C) 2010 Red Hat, Inc.
> + *
> + * Author: Michael S. Tsirkin <mst@redhat.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> +*/
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +#include <linux/in.h>
Do you really need in.h?
> +#include <linux/module.h>
> +#include <linux/skbuff.h>
> +#include <linux/netdevice.h>
> +
> +#include <linux/netfilter/x_tables.h>
> +#include <linux/netfilter/xt_CHECKSUM.h>
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Michael S. Tsirkin <mst@redhat.com>");
> +MODULE_DESCRIPTION("Xtables: checksum modification");
> +MODULE_ALIAS("ipt_CHECKSUM");
> +MODULE_ALIAS("ip6t_CHECKSUM");
> +
> +static unsigned int
> +checksum_tg(struct sk_buff *skb, const struct xt_action_param *par)
> +{
> + if (skb->ip_summed == CHECKSUM_PARTIAL)
> + skb_checksum_help(skb);
> +
> + return XT_CONTINUE;
> +}
> +
> +static int checksum_tg_check(const struct xt_tgchk_param *par)
> +{
> + const struct xt_CHECKSUM_info *einfo = par->targinfo;
> +
> + if (einfo->operation & ~XT_CHECKSUM_OP_FILL) {
> + pr_info("unsupported CHECKSUM operation %x\n", einfo->operation);
> + return -EINVAL;
> + }
> + if (!einfo->operation) {
> + pr_info("no CHECKSUM operation enabled\n");
> + return -EINVAL;
> + }
> + return 0;
> +}
> +
> +static struct xt_target checksum_tg_reg __read_mostly = {
> + .name = "CHECKSUM",
> + .family = NFPROTO_UNSPEC,
> + .target = checksum_tg,
> + .targetsize = sizeof(struct xt_CHECKSUM_info),
> + .table = "mangle",
> + .checkentry = checksum_tg_check,
> + .me = THIS_MODULE,
> +};
> +
> +static int __init checksum_tg_init(void)
> +{
> + return xt_register_target(&checksum_tg_reg);
> +}
> +
> +static void __exit checksum_tg_exit(void)
> +{
> + xt_unregister_target(&checksum_tg_reg);
> +}
> +
> +module_init(checksum_tg_init);
> +module_exit(checksum_tg_exit);
^ permalink raw reply
* Re: [PATCH] CAN: Add Flexcan CAN controller driver
From: Wolfgang Grandegger @ 2010-07-15 9:37 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1279144811-12251-1-git-send-email-mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Hi Marc,
On 07/15/2010 12:00 AM, Marc Kleine-Budde wrote:
> From: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>
> This core is found on some Freescale SoCs and also some Coldfire
> SoCs. Support for Coldfire is missing though at the moment as
> they have an older revision of the core which does not have RX FIFO
> support.
>
> Signed-off-by: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> Signed-off-by: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> ---
>
> Changes to prev version:
> * The is now GPLv2 (only) as no one complained.
>
> The patch applies to current net-next-2.6/master.
> If there aren't any objections please consider applying this patch.
> Wolfgang, can I an Acked-by?
I already had a look to the previous version and I realized that
accessing reg_esr is racy. I will dig out my notes and come up with a
full review later today or tomorrow.
Thanks,
Wolfgang.
^ permalink raw reply
* Re: [PATCHv3] extensions: libxt_CHECKSUM extension
From: Patrick McHardy @ 2010-07-15 9:39 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: David S. Miller, Jan Engelhardt, Randy Dunlap, netfilter-devel,
netfilter, coreteam, linux-kernel, netdev, kvm, herbert
In-Reply-To: <20100711151453.GA7563@redhat.com>
Am 11.07.2010 17:14, schrieb Michael S. Tsirkin:
> diff --git a/extensions/libxt_CHECKSUM.c b/extensions/libxt_CHECKSUM.c
> new file mode 100644
> index 0000000..00fbd8f
> --- /dev/null
> +++ b/extensions/libxt_CHECKSUM.c
> @@ -0,0 +1,99 @@
> +/* Shared library add-on to xtables for CHECKSUM
> + *
> + * (C) 2002 by Harald Welte <laforge@gnumonks.org>
> + * (C) 2010 by Red Hat, Inc
> + * Author: Michael S. Tsirkin <mst@redhat.com>
> + *
> + * This program is distributed under the terms of GNU GPL v2, 1991
> + *
> + * libxt_CHECKSUM.c borrowed some bits from libipt_ECN.c
> + *
> + * $Id$
Please no CVS IDs.
> + */
> +#include <stdio.h>
> +#include <string.h>
> +#include <stdlib.h>
> +#include <getopt.h>
> +
> +#include <xtables.h>
> +#include <linux/netfilter/xt_CHECKSUM.h>
> +
> +static void CHECKSUM_help(void)
> +{
> + printf(
> +"CHECKSUM target options\n"
> +" --checksum-fill Fill in packet checksum.\n");
> +}
> +
> +static const struct option CHECKSUM_opts[] = {
> + { "checksum-fill", 0, NULL, 'F' },
> + { .name = NULL }
> +};
> +
> +static int CHECKSUM_parse(int c, char **argv, int invert, unsigned int *flags,
> + const void *entry, struct xt_entry_target **target)
> +{
> + struct xt_CHECKSUM_info *einfo
> + = (struct xt_CHECKSUM_info *)(*target)->data;
> +
> + switch (c) {
> + case 'F':
> + if (*flags)
> + xtables_error(PARAMETER_PROBLEM,
> + "CHECKSUM target: Only use --checksum-fill ONCE!");
There is a helper function called xtables_param_act for checking double
arguments and printing a standarized error message.
> + einfo->operation = XT_CHECKSUM_OP_FILL;
> + *flags |= XT_CHECKSUM_OP_FILL;
> + break;
> + default:
> + return 0;
> + }
> +
> + return 1;
> +}
> +
^ permalink raw reply
* Re: [PATCH] CAN: Add Flexcan CAN controller driver
From: Marc Kleine-Budde @ 2010-07-15 10:06 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4C3ED6F5.7040606-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 1646 bytes --]
Hey Wolfgang,
Wolfgang Grandegger wrote:
>> This core is found on some Freescale SoCs and also some Coldfire
>> SoCs. Support for Coldfire is missing though at the moment as
>> they have an older revision of the core which does not have RX FIFO
>> support.
>>
>> Signed-off-by: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>> Signed-off-by: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>> ---
>>
>> Changes to prev version:
>> * The is now GPLv2 (only) as no one complained.
>>
>> The patch applies to current net-next-2.6/master.
>> If there aren't any objections please consider applying this patch.
>> Wolfgang, can I an Acked-by?
>
> I already had a look to the previous version and I realized that
> accessing reg_esr is racy. I will dig out my notes and come up with a
> full review later today or tomorrow.
Let me have a look....
I think I should remove the read reg_esr in "flexcan_irq_err()" and use
the value read in "flexcan_irq()"
Without the fix, there's a race window that we loose some error bits
from the interrupt handler to the napi function, because the error bits
are cleared on read.
Regarding the upcoming improvement of the can error frames upon state
changes, I'd first like to get this driver merged and then discuss a
solution for the error frames.
cheers, Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 260 bytes --]
[-- Attachment #2: Type: text/plain, Size: 188 bytes --]
_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core
^ permalink raw reply
* Re: several messages
From: Jan Engelhardt @ 2010-07-15 10:17 UTC (permalink / raw)
To: Patrick McHardy
Cc: Michael S. Tsirkin, David S. Miller, Randy Dunlap,
netfilter-devel, netfilter, coreteam, linux-kernel, netdev, kvm,
herbert
In-Reply-To: <4C3ED764.3060301@trash.net>
On Thursday 2010-07-15 11:36, Patrick McHardy wrote:
>> +struct xt_CHECKSUM_info {
>> + u_int8_t operation; /* bitset of operations */
>
>Please use __u8 in public header files.
>
>> +#include <linux/in.h>
>
>Do you really need in.h?
>
>> + * $Id$
>
>Please no CVS IDs.
>
>> + switch (c) {
>> + case 'F':
>> + if (*flags)
>> + xtables_error(PARAMETER_PROBLEM,
>> + "CHECKSUM target: Only use --checksum-fill ONCE!");
>
>There is a helper function called xtables_param_act for checking double
>arguments and printing a standarized error message.
I took care of these for Xt-a.
^ permalink raw reply
* Re: [PATCH] CAN: Add Flexcan CAN controller driver
From: Wolfgang Grandegger @ 2010-07-15 10:21 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4C3EDDB1.5040109-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
On 07/15/2010 12:06 PM, Marc Kleine-Budde wrote:
> Hey Wolfgang,
>
> Wolfgang Grandegger wrote:
>>> This core is found on some Freescale SoCs and also some Coldfire
>>> SoCs. Support for Coldfire is missing though at the moment as
>>> they have an older revision of the core which does not have RX FIFO
>>> support.
>>>
>>> Signed-off-by: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>>> Signed-off-by: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>>> ---
>>>
>>> Changes to prev version:
>>> * The is now GPLv2 (only) as no one complained.
>>>
>>> The patch applies to current net-next-2.6/master.
>>> If there aren't any objections please consider applying this patch.
>>> Wolfgang, can I an Acked-by?
>>
>> I already had a look to the previous version and I realized that
>> accessing reg_esr is racy. I will dig out my notes and come up with a
>> full review later today or tomorrow.
>
> Let me have a look....
>
> I think I should remove the read reg_esr in "flexcan_irq_err()" and use
> the value read in "flexcan_irq()"
>
> Without the fix, there's a race window that we loose some error bits
> from the interrupt handler to the napi function, because the error bits
> are cleared on read.
Right, that is my concern. reg_esr should only be read and handled
*once*. Currently it's read *3* times! I would read it in the ISR and
handle it in the NAPI poll function, including state change
notification. But I need a closer look...
> Regarding the upcoming improvement of the can error frames upon state
> changes, I'd first like to get this driver merged and then discuss a
> solution for the error frames.
Fine for me. I'm currently preparing a RFC patch for SJA1000 and MSCAN.
Wolfgang.
^ permalink raw reply
* Re: Raise initial congestion window size / speedup slow start?
From: Alan Cox @ 2010-07-15 10:33 UTC (permalink / raw)
To: Hagen Paul Pfeifer
Cc: David Miller, rick.jones2, lists, davidsen, linux-kernel, netdev
In-Reply-To: <20100714221301.GI6682@nuttenaction>
On Thu, 15 Jul 2010 00:13:01 +0200
Hagen Paul Pfeifer <hagen@jauu.net> wrote:
> * David Miller | 2010-07-14 14:55:47 [-0700]:
>
> >Although section 3 of RFC 5681 is a great text, it does not say at all
> >that increasing the initial CWND would lead to fairness issues.
>
> Because it is only one side of the medal, probing conservative the available
> link capacity in conjunction with n simultaneous probing TCP/SCTP/DCCP
> instances is another.
>
> >To be honest, I think google's proposal holds a lot of weight. If
> >over time link sizes and speeds are increasing (they are) then nudging
> >the initial CWND every so often is a legitimate proposal. Were
> >someone to claim that utilization is lower than it could be because of
> >the currenttly specified initial CWND, I would have no problem
> >believing them.
> >
> >And I'm happy to make Linux use an increased value once it has
> >traction in the standardization community.
>
> Currently I know no working link capacity probing approach, without active
> network feedback, to conservatively probing the available link capacity with a
> high CWND. I am curious about any future trends.
Given perfect information from the network nodes you still need to
traverse the network each direction and then return an answer which means
with a 0.5sec end to end time as in the original posting causality itself
demands 1.5 seconds to get an answer which is itself incomplete and
obsolete.
Causality isn't showing any signs of going away soon.
^ permalink raw reply
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