* Read Mail
From: Andrew Lu Sang @ 2010-06-06 10:40 UTC (permalink / raw)
My Dear Beloved,
I have an obscured business suggestion for you of $24,500,000USD.I will update you with further information
as soon as I receive your complete name and phone number.Sincerely,Andrew
^ permalink raw reply
* [PATCH] tcp: Fix slowness in read /proc/net/tcp
From: Tom Herbert @ 2010-06-07 3:27 UTC (permalink / raw)
To: davem; +Cc: netdev
This patch address a serious performance issue in reading the
TCP sockets table (/proc/net/tcp).
Reading the full table is done by a number of sequential read
operations. At each read operation, a seek is done to find the
last socket that was previously read. This seek operation requires
that the sockets in the table need to be counted up to the current
file position, and to count each of these requires taking a lock for
each non-empty bucket. The whole algorithm is O(n^2).
The fix is to cache the last bucket value, offset within the bucket,
and the file position returned by the last read operation. On the
next sequential read, the bucket and offset are used to find the
last read socket immediately without needing ot scan the previous
buckets the table. This algorithm t read the whole table is O(n).
The improvement offered by this patch is easily show by performing
cat'ing /proc/net/tcp on a machine with a lot of connections. With
about 182K connections in the table, I see the following:
- Without patch
time cat /proc/net/tcp > /dev/null
real 1m56.729s
user 0m0.214s
sys 1m56.344s
- With patch
time cat /proc/net/tcp > /dev/null
real 0m0.894s
user 0m0.290s
sys 0m0.594s
Signed-off-by: Tom Herbert <therbert@google.com>
---
diff --git a/include/net/tcp.h b/include/net/tcp.h
index a144914..5731664 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -1413,7 +1413,8 @@ struct tcp_iter_state {
sa_family_t family;
enum tcp_seq_states state;
struct sock *syn_wait_sk;
- int bucket, sbucket, num, uid;
+ int bucket, offset, sbucket, num, uid;
+ loff_t last_pos;
};
extern int tcp_proc_register(struct net *net, struct tcp_seq_afinfo *afinfo);
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 202cf09..4af6f20 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1977,6 +1977,11 @@ static inline struct inet_timewait_sock *tw_next(struct inet_timewait_sock *tw)
hlist_nulls_entry(tw->tw_node.next, typeof(*tw), tw_node) : NULL;
}
+/*
+ * Get next listener socket follow cur. If cur is NULL, get first socket
+ * starting from bucket given in st->bucket; when st->bucket is zero the
+ * very first socket in the hash table is returned.
+ */
static void *listening_get_next(struct seq_file *seq, void *cur)
{
struct inet_connection_sock *icsk;
@@ -1987,14 +1992,15 @@ static void *listening_get_next(struct seq_file *seq, void *cur)
struct net *net = seq_file_net(seq);
if (!sk) {
- st->bucket = 0;
- ilb = &tcp_hashinfo.listening_hash[0];
+ ilb = &tcp_hashinfo.listening_hash[st->bucket];
spin_lock_bh(&ilb->lock);
sk = sk_nulls_head(&ilb->head);
+ st->offset = 0;
goto get_sk;
}
ilb = &tcp_hashinfo.listening_hash[st->bucket];
++st->num;
+ ++st->offset;
if (st->state == TCP_SEQ_STATE_OPENREQ) {
struct request_sock *req = cur;
@@ -2009,6 +2015,7 @@ static void *listening_get_next(struct seq_file *seq, void *cur)
}
req = req->dl_next;
}
+ st->offset = 0;
if (++st->sbucket >= icsk->icsk_accept_queue.listen_opt->nr_table_entries)
break;
get_req:
@@ -2044,6 +2051,7 @@ start_req:
read_unlock_bh(&icsk->icsk_accept_queue.syn_wait_lock);
}
spin_unlock_bh(&ilb->lock);
+ st->offset = 0;
if (++st->bucket < INET_LHTABLE_SIZE) {
ilb = &tcp_hashinfo.listening_hash[st->bucket];
spin_lock_bh(&ilb->lock);
@@ -2057,7 +2065,12 @@ out:
static void *listening_get_idx(struct seq_file *seq, loff_t *pos)
{
- void *rc = listening_get_next(seq, NULL);
+ struct tcp_iter_state *st = seq->private;
+ void *rc;
+
+ st->bucket = 0;
+ st->offset = 0;
+ rc = listening_get_next(seq, NULL);
while (rc && *pos) {
rc = listening_get_next(seq, rc);
@@ -2072,13 +2085,18 @@ static inline int empty_bucket(struct tcp_iter_state *st)
hlist_nulls_empty(&tcp_hashinfo.ehash[st->bucket].twchain);
}
+/*
+ * Get first established socket starting from bucket given in st->bucket.
+ * If st->bucket is zero, the very first socket in the hash is returned.
+ */
static void *established_get_first(struct seq_file *seq)
{
struct tcp_iter_state *st = seq->private;
struct net *net = seq_file_net(seq);
void *rc = NULL;
- for (st->bucket = 0; st->bucket <= tcp_hashinfo.ehash_mask; ++st->bucket) {
+ st->offset = 0;
+ for (; st->bucket <= tcp_hashinfo.ehash_mask; ++st->bucket) {
struct sock *sk;
struct hlist_nulls_node *node;
struct inet_timewait_sock *tw;
@@ -2123,6 +2141,7 @@ static void *established_get_next(struct seq_file *seq, void *cur)
struct net *net = seq_file_net(seq);
++st->num;
+ ++st->offset;
if (st->state == TCP_SEQ_STATE_TIME_WAIT) {
tw = cur;
@@ -2139,6 +2158,7 @@ get_tw:
st->state = TCP_SEQ_STATE_ESTABLISHED;
/* Look for next non empty bucket */
+ st->offset = 0;
while (++st->bucket <= tcp_hashinfo.ehash_mask &&
empty_bucket(st))
;
@@ -2166,7 +2186,11 @@ out:
static void *established_get_idx(struct seq_file *seq, loff_t pos)
{
- void *rc = established_get_first(seq);
+ struct tcp_iter_state *st = seq->private;
+ void *rc;
+
+ st->bucket = 0;
+ rc = established_get_first(seq);
while (rc && pos) {
rc = established_get_next(seq, rc);
@@ -2191,24 +2215,72 @@ static void *tcp_get_idx(struct seq_file *seq, loff_t pos)
return rc;
}
+static void *tcp_seek_last_pos(struct seq_file *seq)
+{
+ struct tcp_iter_state *st = seq->private;
+ int offset = st->offset;
+ int orig_num = st->num;
+ void *rc = NULL;
+
+ switch (st->state) {
+ case TCP_SEQ_STATE_OPENREQ:
+ case TCP_SEQ_STATE_LISTENING:
+ if (st->bucket >= INET_LHTABLE_SIZE)
+ break;
+ st->state = TCP_SEQ_STATE_LISTENING;
+ rc = listening_get_next(seq, NULL);
+ while (offset-- && rc)
+ rc = listening_get_next(seq, rc);
+ if (rc)
+ break;
+ st->bucket = 0;
+ /* Fallthrough */
+ case TCP_SEQ_STATE_ESTABLISHED:
+ case TCP_SEQ_STATE_TIME_WAIT:
+ st->state = TCP_SEQ_STATE_ESTABLISHED;
+ if (st->bucket > tcp_hashinfo.ehash_mask)
+ break;
+ rc = established_get_first(seq);
+ while (offset-- && rc)
+ rc = established_get_next(seq, rc);
+ }
+
+ st->num = orig_num;
+
+ return rc;
+}
+
static void *tcp_seq_start(struct seq_file *seq, loff_t *pos)
{
struct tcp_iter_state *st = seq->private;
+ void *rc;
+
+ if (*pos && *pos == st->last_pos) {
+ rc = tcp_seek_last_pos(seq);
+ if (rc)
+ goto out;
+ }
+
st->state = TCP_SEQ_STATE_LISTENING;
st->num = 0;
- return *pos ? tcp_get_idx(seq, *pos - 1) : SEQ_START_TOKEN;
+ st->bucket = 0;
+ st->offset = 0;
+ rc = *pos ? tcp_get_idx(seq, *pos - 1) : SEQ_START_TOKEN;
+
+out:
+ st->last_pos = *pos;
+ return rc;
}
static void *tcp_seq_next(struct seq_file *seq, void *v, loff_t *pos)
{
+ struct tcp_iter_state *st = seq->private;
void *rc = NULL;
- struct tcp_iter_state *st;
if (v == SEQ_START_TOKEN) {
rc = tcp_get_idx(seq, 0);
goto out;
}
- st = seq->private;
switch (st->state) {
case TCP_SEQ_STATE_OPENREQ:
@@ -2216,6 +2288,8 @@ static void *tcp_seq_next(struct seq_file *seq, void *v, loff_t *pos)
rc = listening_get_next(seq, v);
if (!rc) {
st->state = TCP_SEQ_STATE_ESTABLISHED;
+ st->bucket = 0;
+ st->offset = 0;
rc = established_get_first(seq);
}
break;
@@ -2226,6 +2300,7 @@ static void *tcp_seq_next(struct seq_file *seq, void *v, loff_t *pos)
}
out:
++*pos;
+ st->last_pos = *pos;
return rc;
}
@@ -2264,6 +2339,7 @@ static int tcp_seq_open(struct inode *inode, struct file *file)
s = ((struct seq_file *)file->private_data)->private;
s->family = afinfo->family;
+ s->last_pos = 0;
return 0;
}
^ permalink raw reply related
* Re: [PATCH net-next 00/11] tg3: Bugfixes and 5719 support
From: Matt Carlson @ 2010-06-07 3:59 UTC (permalink / raw)
To: David Miller; +Cc: Matthew Carlson, netdev@vger.kernel.org, andy@greyhouse.net
In-Reply-To: <20100606.180029.42789560.davem@davemloft.net>
On Sun, Jun 06, 2010 at 06:00:29PM -0700, David Miller wrote:
> From: "Matt Carlson" <mcarlson@broadcom.com>
> Date: Sat, 5 Jun 2010 20:24:29 -0700
>
> > This patchset adds some bugfixes and adds 5719 device support.
>
> All applied to net-next-2.6 but there are two things I'm very disappointed
> with in this series:
>
> 1) Naming register bits things like "TX_MBUF_FIX" isn't descriptive,
> and I suspect the actual bit name used in your programming manuals
> is very different and would be more helpful to someone reading the
> code and trying to understand exactly what that bit does.
Actually, the programming manual describes this register just as
tersely.
> How does it change the chips internal MBUF handling behavior? Does
> it insert a delay in accesses or state changes? Does it change
> the MBUF arbitration? What the heck does that bit do exactly?
The programming manual says this bit prevents a tx state machine lockup
due to tx mbuf corruption. The conditions under which the tx mbufs get
corrupted is complicated, but the net effect of this bit is that the
RDMA engine acts a little more conservatively.
> 2) Removing register definitions is something we really shouldn't do.
> Just because you're not using the register any more in the driver,
> doesn't mean you should remove it's definition from tg3.h
>
> What if some other developer wants to play with that register and
> use it for some other purpose or experiment?
The problem is that register definitions can change from asic rev to
asic rev. Someone interested in playing with their hardware would have
to know more about their chip before it could even be considered safe to
use these bits. Rather than have a bit do something other than appears
advertised, it would be safer to just remove it from view.
The patch that removed the register definition exists precisely because
that register definition was changing. The motivating reason for the
patch was to make the code cleaner and more maintainable, but anyone
attempting to use that definition on a non-5717 device would be using it
incorrectly.
> You really have to handle situations like #1 and #2 better especially
> since you guys do not public post the full PDF hardware programming
> manuals of your chips online for other developers to use.
>
> I wouldn't, therefore, impose these rules on Intel and their drivers
> because they do public the programming manuals for their networking
> chips.
Understood.
^ permalink raw reply
* Re: [PATCH] tcp: Fix slowness in read /proc/net/tcp
From: Eric Dumazet @ 2010-06-07 7:19 UTC (permalink / raw)
To: Tom Herbert; +Cc: davem, netdev, Yakov Lerner
In-Reply-To: <alpine.DEB.1.00.1006062020180.24064@pokey.mtv.corp.google.com>
Le dimanche 06 juin 2010 à 20:27 -0700, Tom Herbert a écrit :
> This patch address a serious performance issue in reading the
> TCP sockets table (/proc/net/tcp).
>
> Reading the full table is done by a number of sequential read
> operations. At each read operation, a seek is done to find the
> last socket that was previously read. This seek operation requires
> that the sockets in the table need to be counted up to the current
> file position, and to count each of these requires taking a lock for
> each non-empty bucket. The whole algorithm is O(n^2).
>
> The fix is to cache the last bucket value, offset within the bucket,
> and the file position returned by the last read operation. On the
> next sequential read, the bucket and offset are used to find the
> last read socket immediately without needing ot scan the previous
> buckets the table. This algorithm t read the whole table is O(n).
>
> The improvement offered by this patch is easily show by performing
> cat'ing /proc/net/tcp on a machine with a lot of connections. With
> about 182K connections in the table, I see the following:
>
> - Without patch
> time cat /proc/net/tcp > /dev/null
>
> real 1m56.729s
> user 0m0.214s
> sys 1m56.344s
>
> - With patch
> time cat /proc/net/tcp > /dev/null
>
> real 0m0.894s
> user 0m0.290s
> sys 0m0.594s
>
> Signed-off-by: Tom Herbert <therbert@google.com>
> ---
This problem raises every year, (last attempt from Yakov Lerner :
http://kerneltrap.org/mailarchive/linux-netdev/2009/9/26/6256119 )
And finally, someone motivated enough to use /proc/net/tcp found the
right answer ;)
Most netdev people tend to push inet_diag (netlink) interface instead of
old /proc/net/tcp, but it seems old interface will still be used in
2030, so :
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
BTW, another problem of /proc/net/tcp is the buffer size used by netstat
utility : 1024 bytes instead of PAGE_SIZE, making O(N^2) behavior even
more palpable.
^ permalink raw reply
* Re: [PATCH net-next 00/11] tg3: Bugfixes and 5719 support
From: David Miller @ 2010-06-07 7:28 UTC (permalink / raw)
To: mcarlson; +Cc: netdev, andy
In-Reply-To: <20100607035934.GB11599@mcarlson.broadcom.com>
From: "Matt Carlson" <mcarlson@broadcom.com>
Date: Sun, 6 Jun 2010 20:59:34 -0700
> On Sun, Jun 06, 2010 at 06:00:29PM -0700, David Miller wrote:
> The programming manual says this bit prevents a tx state machine lockup
> due to tx mbuf corruption. The conditions under which the tx mbufs get
> corrupted is complicated, but the net effect of this bit is that the
> RDMA engine acts a little more conservatively.
That last phrase is a good description and could have been used to
better name the register bit macro in question. You're basically
confirming that you named the register bit too tersely.
> The problem is that register definitions can change from asic rev to
> asic rev.
Frankly, this kind of justification really ticks me off.
How the heck does that make a difference? Describe which chip revs
the register bit is valid for in a comment for crying out loud.
Document your hardware properly, not selectively or where you see
it fit to do so.
It's bad enough that you guys don't publish your hardware specs.
As a consequence as much knowledge as possible must go into the
driver sources. Any piece of information you take away is bad.
There are tons of chip register bits in this driver already which are
only valid on certain hardware revs. In fact, there are many. Yet
they are all still there in tg3.h whether they are used by the driver
or not. In fact, if I recall correctly, the DMA burst size controls
are pretty much different on every single rev of the chip.
Documenting where for which chips a register bit is valid is
pervasively done elsewhere, and is nothing new. Your driver and your
hardware is not special.
^ permalink raw reply
* Re: [PATCH] [ath5k][leds] Ability to disable leds support. If leds support enabled do not force mac802.11 leds layer selection.
From: Dmytro Milinevskyy @ 2010-06-07 7:39 UTC (permalink / raw)
To: Pavel Roskin
Cc: ath5k-devel, Jiri Slaby, Nick Kossifidis, Luis R. Rodriguez,
Bob Copeland, John W. Linville, GeunSik Lim, Greg Kroah-Hartman,
Lukas Turek, Mark Hindley, Johannes Berg, Jiri Kosina, Kalle Valo,
Keng-Yu Lin, Luca Verdesca, Shahar Or, linux-wireless, netdev,
linux-kernel
In-Reply-To: <1275668535.17251.20.camel@mj>
Pavel, thank you for the response here.
I agree with all your comments and will adjust the patch according to them.
I'm new to the submitting patches into the community and I appreciate
telling criticism so that in future I will not cause that much
troubles.
Regards,
-- Dima
On Fri, Jun 4, 2010 at 7:22 PM, Pavel Roskin <proski@gnu.org> wrote:
> On Fri, 2010-06-04 at 16:43 +0300, Dmytro Milinevskyy wrote:
>> Hi!
>>
>> Here is the patch to disable ath5k leds support on build stage.
>> However if the leds support was enabled do not force selection of 802.11 leds layer.
>> Depency on LEDS_CLASS is kept.
>>
>> Suggestion given by Pavel Roskin and Bob Copeland applied.
>
> It's great that you did it. The patch is much clearer now. That makes
> smaller issues visible. Please don't be discouraged by the criticism,
> you are on the right track.
>
> First of all, your patch doesn't apply cleanly to the current
> wireless-testing because of formatting changes in Makefile. Please
> update.
>
>> +config ATH5K_LEDS
>> + tristate "Atheros 5xxx wireless cards LEDs support"
>> + depends on ATH5K
>> + select NEW_LEDS
>> + select LEDS_CLASS
>> + ---help---
>> + Atheros 5xxx LED support.
>
> "tristate" is wrong here. "tristate" would allow users select "m",
> which is wrong, since LED support is not a separate module. I think you
> want "bool" here.
>
>> +#ifdef CONFIG_ATH5K_LEDS
>> /*
>> * State for LED triggers
>> */
>> @@ -95,6 +96,7 @@ struct ath5k_led
>> struct ath5k_softc *sc; /* driver state */
>> struct led_classdev led_dev; /* led classdev */
>> };
>> +#endif
>
> This shouldn't be needed. I'll rather see a structure that is not used
> in some cases than an extra pair of preprocessor conditionals.
>
>> diff --git a/drivers/net/wireless/ath/ath5k/gpio.c b/drivers/net/wireless/ath/ath5k/gpio.c
>> index 64a27e7..9e757b3 100644
>> --- a/drivers/net/wireless/ath/ath5k/gpio.c
>> +++ b/drivers/net/wireless/ath/ath5k/gpio.c
>> @@ -25,6 +25,7 @@
>> #include "debug.h"
>> #include "base.h"
>>
>> +#ifdef CONFIG_ATH5K_LEDS
>> /*
>> * Set led state
>> */
>> @@ -76,6 +77,7 @@ void ath5k_hw_set_ledstate(struct ath5k_hw *ah, unsigned int state)
>> else
>> AR5K_REG_ENABLE_BITS(ah, AR5K_PCICFG, led_5210);
>> }
>> +#endif
>
> I would just move that function to led.c (and don't forget to include
> reg.h). The Makefile should take care of the rest.
>
> --
> Regards,
> Pavel Roskin
>
^ permalink raw reply
* Re: [RFC PATCH v7 01/19] Add a new structure for skb buffer from external.
From: Andi Kleen @ 2010-06-07 7:51 UTC (permalink / raw)
To: Stephen Hemminger
Cc: xiaohui.xin, netdev, kvm, linux-kernel, mst, mingo, davem,
herbert, jdike
In-Reply-To: <20100606161348.427822fb@nehalam>
Stephen Hemminger <shemminger@vyatta.com> writes:
> Still not sure this is a good idea for a couple of reasons:
>
> 1. We already have lots of special cases with skb's (frags and fraglist),
> and skb's travel through a lot of different parts of the kernel. So any
> new change like this creates lots of exposed points for new bugs. Look
> at cases like MD5 TCP and netfilter, and forwarding these SKB's to ipsec
> and ppp and ...
>
> 2. SKB's can have infinite lifetime in the kernel. If these buffers come from
> a fixed size pool in an external device, they can easily all get tied up
> if you have a slow listener. What happens then?
3. If they come from an internal pool what happens when the kernel runs
low on memory? How is that pool balanced against other kernel
memory users?
-Andi
--
ak@linux.intel.com -- Speaking for myself only.
^ permalink raw reply
* Re: [PATCH] tcp: Fix slowness in read /proc/net/tcp
From: David Miller @ 2010-06-07 7:55 UTC (permalink / raw)
To: eric.dumazet; +Cc: therbert, netdev, iler.ml
In-Reply-To: <1275895167.2545.8.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 07 Jun 2010 09:19:27 +0200
> Le dimanche 06 juin 2010 à 20:27 -0700, Tom Herbert a écrit :
>> Signed-off-by: Tom Herbert <therbert@google.com>
>> ---
>
> This problem raises every year, (last attempt from Yakov Lerner :
> http://kerneltrap.org/mailarchive/linux-netdev/2009/9/26/6256119 )
>
> And finally, someone motivated enough to use /proc/net/tcp found the
> right answer ;)
>
> Most netdev people tend to push inet_diag (netlink) interface instead of
> old /proc/net/tcp, but it seems old interface will still be used in
> 2030, so :
>
> Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
Indeed this is the best attempt of this I've seen so far,
applied to net-next-2.6, thanks Tom.
> BTW, another problem of /proc/net/tcp is the buffer size used by netstat
> utility : 1024 bytes instead of PAGE_SIZE, making O(N^2) behavior even
> more palpable.
Probably cap it at 8K like we do for netlink atomic reads, because
there are all sorts of crazy PAGE_SIZE configurations possible out
there, even as high as 512K.
^ permalink raw reply
* Re: [PATCH] asix: check packet size against mtu+ETH_HLEN instead of ETH_FRAME_LEN
From: David Miller @ 2010-06-07 7:57 UTC (permalink / raw)
To: jussi.kivilinna; +Cc: netdev, dhollis
In-Reply-To: <20100606233531.23585.10292.stgit@fate.lan>
From: Jussi Kivilinna <jussi.kivilinna@mbnet.fi>
Date: Mon, 07 Jun 2010 02:35:31 +0300
> Driver checks received packet is too large in asix_rx_fixup() and fails if it is. Problem is
> that MTU might be set larger than 1500 and asix fails to work correctly with VLAN tagged
> packets. The check should be 'dev->net->mtu + ETH_HLEN' instead.
>
> Tested with AX88772.
>
> Signed-off-by: Jussi Kivilinna <jussi.kivilinna@mbnet.fi>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next-2.6] net-caif: Added missing lock validator constants
From: David Miller @ 2010-06-07 8:01 UTC (permalink / raw)
To: alex.lorca; +Cc: sjur.brandeland, netdev
In-Reply-To: <1275761704-8758-1-git-send-email-alex.lorca@gmail.com>
From: Alex Lorca <alex.lorca@gmail.com>
Date: Sat, 5 Jun 2010 18:15:04 +0000
> CAIF is using "xxx-AF_MAX" strings for the lock validator. It should use
> its own strings.
>
> Signed-off-by: Alex Lorca <alex.lorca@gmail.com>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH net-next-2.6] raw: avoid two atomics in xmit
From: David Miller @ 2010-06-07 8:09 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1275639837.2482.17.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 04 Jun 2010 10:23:57 +0200
> Avoid two atomic ops per raw_send_hdrinc() call
>
> Avoid two atomic ops per raw6_send_hdrinc() call
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied.
I'm starting to feel like we now use the implicit skb dst ref stuff
more than the usual cloning, which is great! :-)
^ permalink raw reply
* Re: [PATCH net-next-2.6] net sched: make pedit check for clones instead
From: David Miller @ 2010-06-07 8:09 UTC (permalink / raw)
To: herbert; +Cc: hadi, jpirko, netdev
In-Reply-To: <20100604130511.GA10925@gondor.apana.org.au>
From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Fri, 4 Jun 2010 23:05:11 +1000
> On Fri, Jun 04, 2010 at 08:43:06AM -0400, jamal wrote:
>> And here's the second one. I noticed Changli's changes are not yet in
>> net-next. I will wait for that change to propagate and then send the
>> fix to pedit that i discussed with Herbert.
>
> Thanks Jamal!
>
> BTW, I think this patch should go in first (before the one that
> removes the OK2MUNGE setting) to be on the safe side.
I applied them this way to net-next-2.6, thanks!
^ permalink raw reply
* Re: [PATCH] 8139too: fix buffer overrun in rtl8139_init_board
From: David Miller @ 2010-06-07 8:14 UTC (permalink / raw)
To: dkirjanov; +Cc: jeff, netdev
In-Reply-To: <20100605094549.GA11559@hera.kernel.org>
From: Denis Kirjanov <dkirjanov@hera.kernel.org>
Date: Sat, 5 Jun 2010 09:45:49 +0000
> Fix rtl_chip_info buffer overrun when we can't identify the chip.
> (i = ARRAY_SIZE (rtl_chip_info) in this case)
>
> Signed-off-by: Denis Kirjanov <dkirjanov@kernel.org>
Applied, thanks!
^ permalink raw reply
* Re: [RFC PATCH v7 01/19] Add a new structure for skb buffer from external.
From: Mitchell Erblich @ 2010-06-07 8:17 UTC (permalink / raw)
To: Andi Kleen
Cc: Stephen Hemminger, xiaohui.xin, netdev, kvm, linux-kernel, mst,
mingo, davem, herbert, jdike
In-Reply-To: <87pr03gu1c.fsf@basil.nowhere.org>
On Jun 7, 2010, at 12:51 AM, Andi Kleen wrote:
> Stephen Hemminger <shemminger@vyatta.com> writes:
>
>> Still not sure this is a good idea for a couple of reasons:
>>
>> 1. We already have lots of special cases with skb's (frags and fraglist),
>> and skb's travel through a lot of different parts of the kernel. So any
>> new change like this creates lots of exposed points for new bugs. Look
>> at cases like MD5 TCP and netfilter, and forwarding these SKB's to ipsec
>> and ppp and ...
>>
>> 2. SKB's can have infinite lifetime in the kernel. If these buffers come from
>> a fixed size pool in an external device, they can easily all get tied up
>> if you have a slow listener. What happens then?
>
> 3. If they come from an internal pool what happens when the kernel runs
> low on memory? How is that pool balanced against other kernel
> memory users?
>
> -Andi
>
> --
> ak@linux.intel.com -- Speaking for myself only.
In general,
When an internal pool is created/used, their SHOULD be a reason.
Maybe, to keep allocation latency to a min, OR ...
Now IMO,
internal pool objects should have a ref count and
if that count is 0, then under memory pressure and/or num
of objects are above a high water mark, then they are freed,
OR if there is a last reference age field, then the object is to be
cleaned if dirty, then freed,
Else, the pool is allowed to grow if the number of objects in the
pool is below a set max (max COULD equal Infinity).
Mitchell Erblich
^ permalink raw reply
* Re: [PATCH] phylib: Add support for the LXT973 phy.
From: David Miller @ 2010-06-07 8:18 UTC (permalink / raw)
To: richardcochran; +Cc: afleming, netdev
In-Reply-To: <20100605140017.GA2750@riccoc20.at.omicron.at>
From: Richard Cochran <richardcochran@gmail.com>
Date: Sat, 5 Jun 2010 16:00:17 +0200
> On Wed, Jun 02, 2010 at 02:32:11PM -0500, Andy Fleming wrote:
>> Yeah, I was clearly not thinking clearly. dev_flags will be
>> overwritten, and is not meant for this. I believe, what we should do
>> is add a "port" field to the PHY device, and if PCR_FIBER_SELECT is
>> set, then set the port field to PORT_FIBRE. I'm not entirely clear on
>> the semantics of that field in the ethtool cmd, but it seems like the
>> right idea.
>
> Here is another try. Is that more like it?
I think this is overkill.
One, and only one, PHY driver wants to maintain a boolean state and
we're adding a full 32-bit flags member to the generic PHY device
struct to accomodate this?
Andy if you have ideas to use that state via ethtool or whatever in
the future, you come back to us when the future arrives and you've
implemented the code in the generic PHY lib code to do that stuff.
As it stands right now, that code doesn't exist so accomodating it is
just silly.
I also think worrying about the phy_dev->priv pointer being misused in
the future inside of this driver is a completely bogus argument by any
measure.
We have many cases all over the kernel tree, in drivers and elsewhere,
where we encode integer states in what are normally pointer values
when we need to maintain a small piece of state and don't need to do a
full blown memory allocation to allocate a piece of extra memory to
hold that state.
Richard, please just resubmit your original patch and I will apply it.
Thanks.
^ permalink raw reply
* Re: [PATCH] greth: Remove unnecessary memset of napi member in netdev private data
From: David Miller @ 2010-06-07 8:21 UTC (permalink / raw)
To: tklauser; +Cc: kristoffer, netdev
In-Reply-To: <1275632661-16406-1-git-send-email-tklauser@distanz.ch>
From: Tobias Klauser <tklauser@distanz.ch>
Date: Fri, 4 Jun 2010 08:24:21 +0200
> The memory for the private data is allocated using kzalloc in
> alloc_etherdev (or alloc_netdev_mq respectively) so there is no need to
> set the napi member to 0 explicitely.
>
> Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
Applied.
^ permalink raw reply
* Re: [PATCH] htb: remove two unnecessary assignments
From: David Miller @ 2010-06-07 8:21 UTC (permalink / raw)
To: xiaosuo; +Cc: hadi, netdev
In-Reply-To: <1275651213-3343-1-git-send-email-xiaosuo@gmail.com>
From: Changli Gao <xiaosuo@gmail.com>
Date: Fri, 4 Jun 2010 19:33:33 +0800
> remove two unnecessary assignments
>
> we don't need to assign NULL when initialize structure objects.
>
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
Applied.
^ permalink raw reply
* Re: [RFC] act_cpu: redirect skb receiving to a special CPU.
From: Andi Kleen @ 2010-06-07 8:43 UTC (permalink / raw)
To: hadi
Cc: Changli Gao, Eric Dumazet, David S. Miller, Tom Herbert,
Linux Netdev List
In-Reply-To: <1275746045.3490.60.camel@bigi>
jamal <hadi@cyberus.ca> writes:
>
> I would look at it as "messaging of remote CPU" which may not result
> in an IRQ. I am pretty sure if you tried hard you could use HT in AMD
> hardware - the remote cpu may have an IRQ triggered but it wont be as
> expensive as IPI.
It's unlikely you'll find any way on x86 to do an IPI that is cheaper
than an standard IPI. That is unless you dedicate the receiver to poll
or monitor.
On recent higher end Intel CPUs X2APIC IPIs will be somewhat cheaper
than classical APIC IPIs.
But for a normal "IPI user" like networking it looks all the same,
it's hidden by the architecture code.
-Andi
--
ak@linux.intel.com -- Speaking for myself only.
^ permalink raw reply
* Re: [PATCH] tcp: Fix slowness in read /proc/net/tcp
From: Andi Kleen @ 2010-06-07 8:49 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Tom Herbert, davem, netdev, Yakov Lerner
In-Reply-To: <1275895167.2545.8.camel@edumazet-laptop>
Eric Dumazet <eric.dumazet@gmail.com> writes:
>
> BTW, another problem of /proc/net/tcp is the buffer size used by netstat
> utility : 1024 bytes instead of PAGE_SIZE, making O(N^2) behavior even
> more palpable.
That would be easily fixable in netstat.
But I'm not sure net-tools is still maintained as a separate package,
afaik the standard procedure is to submit a patch to a big distribution
and the others take it from there.
-Andi
--
ak@linux.intel.com -- Speaking for myself only.
^ permalink raw reply
* Re: [Patch 1/2] s2io: add dynamic LRO disable support
From: Cong Wang @ 2010-06-07 9:01 UTC (permalink / raw)
To: Ramkrishna Vepa
Cc: Michal Schmidt, netdev@vger.kernel.org, herbert.xu@redhat.com,
nhorman@redhat.com, sgruszka@redhat.com, davem@davemloft.net
In-Reply-To: <FCA91A92EE52B041906A0358FC28FCC38EF129DB8F@FRE1EXCH02.hq.exar.com>
On 06/05/10 16:53, Ramkrishna Vepa wrote:
>> +static int s2io_ethtool_set_flags(struct net_device *dev, u32 data)
>> +{
>> + struct s2io_nic *sp = netdev_priv(dev);
>> + int rc = 0;
>> + int changed = 0;
>> +
>> + if (data& ETH_FLAG_LRO) {
>> + if (lro_enable) {
>> + if (!(dev->features& NETIF_F_LRO)) {
>> + dev->features |= NETIF_F_LRO;
>> + changed = 1;
>> + }
>> + } else
>> + rc = -EINVAL;
>> + } else if (dev->features& NETIF_F_LRO) {
>> + dev->features&= ~NETIF_F_LRO;
>> + changed = 1;
>> + }
>> +
>> + if (changed&& netif_running(dev)) {
>> + s2io_stop_all_tx_queue(sp);
>> + s2io_card_down(sp);
>> + sp->lro = dev->features& NETIF_F_LRO;
>> + rc = s2io_card_up(sp);
> In s2io_card_up, update ring->lro too as it is used in the fast path -
> struct ring_info *ring =&mac_control->rings[i];
>
> ring->mtu = dev->mtu;
> + ring->lro = sp->lro;
>
Yeah, I missed this important part.
>> + s2io_start_all_tx_queue(sp);
>
> The following line in init_shared_mem() is redundant and can be removed.
> - ring->lro = lro_enable;
>
Okay.
I will send an updated version soon.
Thanks a lot!
^ permalink raw reply
* Re: [Patch] infiniband: check local reserved ports
From: Cong Wang @ 2010-06-07 9:04 UTC (permalink / raw)
To: Roland Dreier
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA, Tetsuo Handa,
davem-fT/PcQaiUtIeIZ0/mPfg9Q, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
sean.hefty-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <ada1vcmpyvu.fsf-BjVyx320WGW9gfZ95n9DRSW4+XlvGpQz@public.gmane.org>
On 06/05/10 00:04, Roland Dreier wrote:
> > > Should this inet_is_reserved_local_port() test apply to all the "port
> > > spaces" that this code is handling? I honestly am ignorant of the
> > > intended semantics of the new local_reserved_ports stuff, hence my question.
>
> > Yes, but I only found this case, is there any else?
>
> My question was more in the other direction: should this test apply to
> all the "port spaces" handled here? From looking at the code, it
> appears the answer is yes -- it seems that putting a port in
> local_reserved_ports reserves that port for IPv4 and IPv6, UDP, TCP,
> SCTP, DCCP, everything, so we should probably reserve all RDMA CM ports too.
Yes.
So this patch looks good for you? :)
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" 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] virtio_net: indicate oom when addbuf returns failure
From: Michael S. Tsirkin @ 2010-06-07 9:15 UTC (permalink / raw)
To: Herbert Xu; +Cc: Bruce Rogers, netdev, Shirley Ma, virtualization, stable
In-Reply-To: <20100606222441.GA5992@gondor.apana.org.au>
On Mon, Jun 07, 2010 at 08:24:41AM +1000, Herbert Xu wrote:
> On Sun, Jun 06, 2010 at 11:13:00PM +0300, Michael S. Tsirkin wrote:
> >
> > Actually this code looks strange:
> > Note that add_buf inicates out of memory
> > condition with a positive return value, and ring full
> > (which is not an error!) with -ENOSPC.
>
> Indeed, this ultimately came from
>
> commit 9ab86bbcf8be755256f0a5e994e0b38af6b4d399
> Author: Shirley Ma <mashirle@us.ibm.com>
> Date: Fri Jan 29 03:20:04 2010 +0000
>
> virtio_net: Defer skb allocation in receive path Date: Wed, 13 Jan 2010 12:53:38 -0800
>
> (Greg, please don't apply this even though I've just given you
> the upstream commit ID that you were asking for :)
>
> where it confuses a memory allocation error with an add_buf failure.
>
> > Possibly the right thing to do is to
> > 1. handle ENOMEM specially
> > 2. fix add_buf to return ENOMEM on error
>
> I think we should make it so that only a memory allocation error
> is returned as before. There is no need for returning the add_buf
> error unless add_buf is now doing an allocation itself that needs
> to be retried.
That's what my patch did, right? Ack it?
> Thanks,
> --
> Visit Openswan at http://www.openswan.org/
> Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* in_dev->refcnt and dev->refcnt
From: ratheesh k @ 2010-06-07 9:20 UTC (permalink / raw)
To: netdev, linux-net
Is there any relation between in_dev->refcnt and dev->refcnt ? . My
question is : if put or get on one variable affects other ?
-Ratheesh
^ permalink raw reply
* Re: in_dev->refcnt and dev->refcnt
From: Eric Dumazet @ 2010-06-07 9:30 UTC (permalink / raw)
To: ratheesh k; +Cc: netdev, linux-net
In-Reply-To: <AANLkTinijGlJRnqhcEbXhc84MXc62gbshxvTa6JeXo2K@mail.gmail.com>
Le lundi 07 juin 2010 à 14:50 +0530, ratheesh k a écrit :
> Is there any relation between in_dev->refcnt and dev->refcnt ? . My
> question is : if put or get on one variable affects other ?
>
No, there is no relation between them.
You can increase / decrease one refcount or the other, using separate
API.
^ permalink raw reply
* RE: [PATCH] r8169: fix random mdio_write failures
From: hayeswang @ 2010-06-07 9:26 UTC (permalink / raw)
To: 'Francois Romieu', 'Timo Teräs'; +Cc: netdev, davem
In-Reply-To: <20100605124103.GA3213@electric-eye.fr.zoreil.com>
Our hardware engineer suggests that check the completed indication
per 100 micro seconds. And it needs 20 micro seconds delay after the
completed indication for the next command.
Best Regards,
Hayes
-----Original Message-----
From: Francois Romieu [mailto:romieu@fr.zoreil.com]
Sent: Saturday, June 05, 2010 8:41 PM
To: Timo Teräs
Cc: netdev@vger.kernel.org; Edward Hsu; Hayeswang; davem@davemloft.net
Subject: Re: [PATCH] r8169: fix random mdio_write failures
Timo Teräs <timo.teras@iki.fi> :
[...]
> diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c index
> 217e709..03a8318 100644
> --- a/drivers/net/r8169.c
> +++ b/drivers/net/r8169.c
> @@ -559,6 +559,11 @@ static void mdio_write(void __iomem *ioaddr, int
reg_addr, int value)
> break;
> udelay(25);
> }
> + /*
> + * Some configurations require a small delay even after the write
> + * completed indication or the next write might fail.
> + */
> + udelay(25);
Acked-off-by: Francois Romieu <romieu@fr.zoreil.com>
Good work.
I wonder if increasing the in-loop delay as well would help the write
succeed faster (or slower ?).
--
Ueimor
------Please consider the environment before printing this e-mail.
^ 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