* Re: [PATCH] Avoid extra calculation in ip_route_input_common
From: David Miller @ 2011-12-21 5:57 UTC (permalink / raw)
To: wangyun; +Cc: netdev
In-Reply-To: <4EF16AA3.2070303@linux.vnet.ibm.com>
From: Michael Wang <wangyun@linux.vnet.ibm.com>
Date: Wed, 21 Dec 2011 13:12:03 +0800
> From: Michael Wang <wangyun@linux.vnet.ibm.com>
>
> If previous condition doesn't meet, the later check will be cancelled.
> So we don't need to do all the calculation.
>
> Signed-off-by: Michael Wang <wangyun@linux.vnet.ibm.com>
This is intentional to reduce the number of branch prediction
misses, please don't change this.
Once we read one of these values, the rest are incredibly cheap,
the real cost is if we have tons of real branches here, each
of which can be mispredicted.
^ permalink raw reply
* Re: [PATCH] Avoid extra calculation in ip_route_input_common
From: Michael Wang @ 2011-12-21 6:00 UTC (permalink / raw)
To: Joe Perches; +Cc: Stephen Hemminger, netdev@vger.kernel.org
In-Reply-To: <1324446632.21340.4.camel@joe2Laptop>
On 12/21/2011 01:50 PM, Joe Perches wrote:
> On Wed, 2011-12-21 at 13:39 +0800, Michael Wang wrote:
>> On 12/21/2011 01:23 PM, Joe Perches wrote:
>>> On Wed, 2011-12-21 at 13:12 +0800, Michael Wang wrote:
>>>> From: Michael Wang <wangyun@linux.vnet.ibm.com>
>>>>
>>>> If previous condition doesn't meet, the later check will be cancelled.
>>>> So we don't need to do all the calculation.
> []
>>> commit c0b8c32b1c96afc9b32b717927330025cc1c501e
>>> Author: Stephen Hemminger <shemminger@vyatta.com>
>>> Date: Thu Apr 10 04:00:28 2008 -0700
>>>
>>> IPV4: use xor rather than multiple ands for route compare
>>>
>>> The comparison in ip_route_input is a hot path, by recoding the C
>>> "and" as bit operations, fewer conditional branches get generated
>>> so the code should be faster. Maybe someday Gcc will be smart
>>> enough to do this?
>> This is what confused me, why "fewer conditional branches get generated"
>> will make code faster?
>> In this example, I think the best condition when daddr is different, we
>> only need to go to one branch do compare then quit, won't this be faster?
>
> if (a && b)
> ...
> pseudo-codes to:
> if (!a)
> goto fail;
> if (!b)
> goto fail;
> ...
> fail:
>
> Each of those conditional branches has a cost.
> Combining tests of variables in the same cache lines
> has relatively little cost compared to the conditional
> branches.
>
That make sense :)
> That's the theory anyway.
>
> If you have tests that demonstrate otherwise, please
> provide them.
>
I think the previous patch should have done such test, otherwise they
won't do this change.
Thanks for your reply, that clear my confusion.
Regards,
Michael Wang
> cheers, Joe
>
^ permalink raw reply
* Re: [PATCH net-next V2] tcp: Replace constants with #define macros
From: David Miller @ 2011-12-21 6:03 UTC (permalink / raw)
To: subramanian.vijay; +Cc: netdev, eric.dumazet, ilpo.jarvinen
In-Reply-To: <1324423404-28254-1-git-send-email-subramanian.vijay@gmail.com>
From: Vijay Subramanian <subramanian.vijay@gmail.com>
Date: Tue, 20 Dec 2011 15:23:24 -0800
> to record the state of SACK/FACK and DSACK for better readability and maintenance.
>
> Signed-off-by: Vijay Subramanian <subramanian.vijay@gmail.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] Avoid extra calculation in ip_route_input_common
From: Michael Wang @ 2011-12-21 6:04 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20111221.005758.556186611596801704.davem@davemloft.net>
On 12/21/2011 01:57 PM, David Miller wrote:
> From: Michael Wang <wangyun@linux.vnet.ibm.com>
> Date: Wed, 21 Dec 2011 13:12:03 +0800
>
>> From: Michael Wang <wangyun@linux.vnet.ibm.com>
>>
>> If previous condition doesn't meet, the later check will be cancelled.
>> So we don't need to do all the calculation.
>>
>> Signed-off-by: Michael Wang <wangyun@linux.vnet.ibm.com>
>
> This is intentional to reduce the number of branch prediction
> misses, please don't change this.
>
> Once we read one of these values, the rest are incredibly cheap,
> the real cost is if we have tons of real branches here, each
> of which can be mispredicted.
>
Hi, David
Thanks for your detailed explain, now I know the reason we use this style :)
Thanks,
Michael Wang
^ permalink raw reply
* Re: [PATCH] net: macb: fix build break with !CONFIG_OF
From: David Miller @ 2011-12-21 6:04 UTC (permalink / raw)
To: olof; +Cc: nicolas.ferre, netdev, plagnioj, jamie, linux-arm-kernel
In-Reply-To: <1324416029-28198-1-git-send-email-olof@lixom.net>
From: Olof Johansson <olof@lixom.net>
Date: Tue, 20 Dec 2011 13:20:29 -0800
> This fixes the build of the macb driver that was broken by the DT changes:
>
> drivers/net/ethernet/cadence/macb.c:1503: error: implicit declaration of function 'of_match_ptr'
> drivers/net/ethernet/cadence/macb.c:1503: error: 'macb_dt_ids' undeclared here (not in a function)
> drivers/net/ethernet/cadence/macb.c:1503: error: initializer element is not constant
> drivers/net/ethernet/cadence/macb.c:1503: error: (near initialization for 'macb_driver.driver.of_match_table')
>
> Signed-off-by: Olof Johansson <olof@lixom.net>
> ---
>
> Nicolas, care to ack this so I can apply it on top of the macb DT branch in arm-soc?
Yes, please apply it there.
^ permalink raw reply
* Re: [PATCH V2] IPv6 : add multicast routing verify which net_device is lo
From: David Miller @ 2011-12-21 6:15 UTC (permalink / raw)
To: gaofeng; +Cc: netdev, wangxingtong
In-Reply-To: <1324379424-18055-1-git-send-email-gaofeng@cn.fujitsu.com>
From: Gao feng <gaofeng@cn.fujitsu.com>
Date: Tue, 20 Dec 2011 19:10:24 +0800
> In currently routing subsystem, when we lookup a multicast routing
> to send muticast packets to outside, rt6_device_match will return
> the rt6_info which it's match first. If we add a multicast route on
> loopback devices beforce the others interface, rt6_device_match will
> retrun the rt6_info which rt6i_dev->name is "lo". But, obviously,
> we can't send a muticast packet to outside using loopback devices.
> It case all multicast packets blocking.
>
> Commit 4af04aba93f47699e disabled kernel add multicast route on lo
> automatically. However, we can't surmise the routing-add order or
> interdict add multicast routing on loopback devices in user space.
> The bug still exist. So, i think, more stronger routing subsystem is
> necessary.
>
> Signed-off-by: Wang xingtong <wangxingtong@cn.fujitsu.com>
Ok, this is getting rediculious. I want to revert all of this
stuff.
How about, instead, we stop userland adding explicit addresses to the
loopback device since that's what started behaving differently
recently and causes these problems in the first place?
^ permalink raw reply
* Re: 3.2-rc6+: Reported regressions from 3.0 and 3.1
From: Markus Trippelsdorf @ 2011-12-21 6:45 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Kernel Testers List, Linux SCSI List, Florian Mickler,
Network Development, Linux PM list, Linux Wireless List,
Linux Kernel Mailing List, DRI, Linux ACPI, Andrew Morton,
Linus Torvalds, Maciej Rutecki
In-Reply-To: <201112210039.14100.linux-pm-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org>
On 2011.12.21 at 00:54 +0100, Rafael J. Wysocki wrote:
> Subject : WARNING: at fs/sysfs/sysfs.h:195 (during boot)
> Submitter : Markus Trippelsdorf <markus-xp2qqqlHh3xzoYq+O6RWwA-XMD5yJDbdMReXY1tMh2IBg@public.gmane.org>
> Date : 2011-11-13 19:24
> Message-ID : 20111113192417.GA1659-tLCgZGx+iJ+kxVt8IV0GqQ-XMD5yJDbdMReXY1tMh2IBg@public.gmane.org
> References : http://marc.info/?l=linux-kernel&m=132121231921932&w=2
>
> Subject : WARNING: at mm/slub.c:3357, kernel BUG at mm/slub.c:3413
> Submitter : Markus Trippelsdorf <markus-xp2qqqlHh3xzoYq+O6RWwA-XMD5yJDbdMReXY1tMh2IBg@public.gmane.org>
> Date : 2011-11-18 7:25
> Message-ID : 20111118072519.GA1615-tLCgZGx+iJ+kxVt8IV0GqQ-XMD5yJDbdMReXY1tMh2IBg@public.gmane.org
> References : http://marc.info/?l=linux-kernel&m=132160119031794&w=2
Both entries were caused by the same Radeon kexec writeback bug. This issue
will hopefully be fixed in the next release.
> Subject : Impossible high cpu-time values for migration threads
> Submitter : Markus Trippelsdorf <markus-xp2qqqlHh3xzoYq+O6RWwA-XMD5yJDbdMReXY1tMh2IBg@public.gmane.org>
> Date : 2011-11-17 22:17
> Message-ID : 20111117221731.GA9229-tLCgZGx+iJ+kxVt8IV0GqQ-XMD5yJDbdMReXY1tMh2IBg@public.gmane.org
> References : http://marc.info/?l=linux-kernel&m=132156832124314&w=2
This is a cosmetic issue, but I'm still seeing this after bursts of high
CPU usage:
% ps afx -F
UID PID PPID C SZ RSS PSR STIME TTY STAT TIME CMD
root 2 0 0 0 0 1 Dec20 ? S 0:00 [kthreadd]
root 3 2 0 0 0 0 Dec20 ? S 0:01 \_ [ksoftirqd/0]
root 6 2 0 0 0 0 Dec20 ? S 0:00 \_ [migration/0]
root 7 2 0 0 0 1 Dec20 ? S 0:00 \_ [migration/1]
root 8 2 0 0 0 1 Dec20 ? S 0:05 \_ [kworker/1:0]
root 9 2 0 0 0 1 Dec20 ? S 0:01 \_ [ksoftirqd/1]
root 11 2 0 0 0 2 Dec20 ? S 0:00 \_ [migration/2]
root 13 2 0 0 0 2 Dec20 ? S 0:01 \_ [ksoftirqd/2]
root 14 2 22 0 0 3 Dec20 ? S 192:21 \_ [migration/3]
root 16 2 0 0 0 3 Dec20 ? S 0:01 \_ [ksoftirqd/3]
root 17 2 0 0 0 2 Dec20 ? S< 0:00 \_ [khelper]
...
--
Markus
^ permalink raw reply
* RE: [PATCH 0/2] Tracepoint for tcp retransmission
From: Eric Dumazet @ 2011-12-21 7:05 UTC (permalink / raw)
To: Satoru Moriya
Cc: Hagen Paul Pfeifer, Stephen Hemminger, nhorman@tuxdriver.com,
davem@davemloft.net, tgraf@infradead.org, Seiji Aguchi,
dle-develop@lists.sourceforge.net, netdev@vger.kernel.org
In-Reply-To: <65795E11DBF1E645A09CEC7EAEE94B9CB5AFB01E@USINDEVS02.corp.hds.com>
Le mardi 20 décembre 2011 à 18:40 -0500, Satoru Moriya a écrit :
> We use trace_kfree_skb() which 63e03724b51 uses to detect packet
> drop event. In addition to that, we would like to detect errors
> in TCP layer for better trouble shooting.
>
If you feel we miss a counter in a particular spot, just add it, so that
even a newbie can report "netstat -s" values in a message for analysis.
Not re-sending a frame because of a temporary shortage of memory is not
an error per se, TCP will recover eventually, but keeping track of these
events might be useful (once we have not percpu counters for this, but
regular atomic counters)
^ permalink raw reply
* Re: Kernel-DOS error in arp mechanism – no delete off incomplete arp adresses
From: Eric Dumazet @ 2011-12-21 7:44 UTC (permalink / raw)
To: richard -rw- weinberger, David Miller
Cc: Robert Gladewitz, linux-kernel, netdev
In-Reply-To: <CAFLxGvxjLCyMCPXtpm7a7RaOL4A4=bhCLPKD=FVAc8xOdx_CsQ@mail.gmail.com>
Le samedi 17 décembre 2011 à 14:26 +0100, richard -rw- weinberger a
écrit :
> On Sat, Dec 17, 2011 at 9:27 AM, Robert Gladewitz <gladewitz@gmx.de> wrote:
> > Hello,
> >
> > first i have to say sorry for m y bad english. I try my best to descripe the
> > error.
> >
> > I Use Linux-Routers for internal and external firewall components. For this
> > I Use own kernel configurations und use only the drivers an modules what I
> > need. Other features and modules I deactivated in my kernel versions
> >
> > Since the kernel version 2.6.36 there is some mistake in the ipv4 arp
> > implementation. The the System try to find an unknown system, the send an
> > “who is” and marked the ip address as “incomplete” (German: unvollständig).
> > The thing is, usually linux delete all incomplete and complete entries in
> > some time, but in all kernel versions since 2.6.36 he doas not delete any
> > addresses.
> >
> > In my case, I scan my network-segmens for new devices (Kaspersky, Landesk)
> > and on this process, the router learned a lot of incomplete addresses. I
> > have some class b networks (from the history), and this means the router
> > will be learned mor then 2^16 adresses.
> >
> > Now the kerlen learn a maximum addresses – I know this is defined on
> > gc_thresh1 , gc_thresh2 and gc_thresh3 in the proc system under
> > sys.net.ipv4.neight.default. If the table have the maximum addresses in the
> > table (default=1024), no new host can send traffic packet over this router.
> > This means, we have a classical risk of DOS. In my case, I have only an
> > internal risk, but some providers may have also external risc.
> >
> > I hope, my description help you to find this error. I send also my kernel
> > config, may there is some relation to small configurations in kernel
> >
> > Viele Grüße
> >
> > Robert Gladewitz
> >
>
> CC'ing netdev.
>
Hmm, it seems we removed the ip route garbage collection too soon.
In the old times, we had a mechanism (rt_check_expire()) to remove old
route cache entries, but not anymore, so we can have some slots in IP
route cache referencing neighbours entries for infinite amount of time.
8.0.0.72 dev eth3 ref 2 used 1038/1098/1035 probes 6 FAILED
8.0.0.205 dev eth3 ref 2 used 639/699/636 probes 6 FAILED
8.0.0.82 dev eth3 ref 2 used 1008/1068/1005 probes 6 FAILED
8.0.0.215 dev eth3 ref 2 used 609/669/606 probes 6 FAILED
In 2.6.39, commit 2c8cec5c10bc (ipv4: Cache learned PMTU information in
inetpeer.) removed rt_check_expire() and added this problem.
David, I suggest we add back the garbage collector for current kernels,
we'll remove it when route cache really disappear ?
I'll send a patch today.
^ permalink raw reply
* Re: Kernel-DOS error in arp mechanism =?iso-2022-jp-3?B?GyQoUSN8GyhC?= no delete off incomplete arp adresses
From: David Miller @ 2011-12-21 8:07 UTC (permalink / raw)
To: eric.dumazet; +Cc: richard.weinberger, gladewitz, linux-kernel, netdev
In-Reply-To: <1324453467.2610.20.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 21 Dec 2011 08:44:27 +0100
> David, I suggest we add back the garbage collector for current kernels,
> we'll remove it when route cache really disappear ?
>
> I'll send a patch today.
Yes, it's the best idea.
We can actually remove it again as early as when when route neigh's
are ref-less.
^ permalink raw reply
* RE: [PATCH 2/8] farsync: Fix confusion about DMA address and buffer offset types
From: Kevin Curtis @ 2011-12-21 8:33 UTC (permalink / raw)
To: Ben Hutchings, David Miller; +Cc: netdev, linux-kernel
In-Reply-To: <1324446073.2844.190.camel@deadeye>
Hi,
Thanks for the patches. I will apply them to the master copy of the source code here.
Regards
Kevin Curtis
Linux Development
FarSite Communications Ltd http://www.farsite.com
Winner of The Queen's Award for Enterprise 2009
tel: +44 1256 330461
fax: +44 1256 854931
-----Original Message-----
From: Ben Hutchings [mailto:ben@decadent.org.uk]
Sent: 21 December 2011 05:41
To: David Miller
Cc: Kevin Curtis; netdev@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/8] farsync: Fix confusion about DMA address and buffer offset types
On Tue, 2011-12-20 at 23:46 -0500, David Miller wrote:
> From: Ben Hutchings <ben@decadent.org.uk>
> Date: Wed, 21 Dec 2011 01:32:52 +0000
>
> > - dbg(DBG_RX, "In fst_rx_dma %lx %lx %d\n",
> > - (unsigned long) skb, (unsigned long) mem, len);
> > + dbg(DBG_RX, "In fst_rx_dma %x %x %d\n", (u32)skb, mem, len);
>
> This is more appropriately fixed by using "%p" instead of casting to a
> 32-bit int.
'skb' is the physical address of the data in the skb. Whereas 'mem' is the offset of the corresponding buffer in shared memory.
All completely clear, right? ;-)
Ben.
--
Ben Hutchings
Humans are not rational beings; they are rationalising beings.
^ permalink raw reply
* Re: [PATCH net-next] bnx2: Update driver to use new mips firmware.
From: Eric Dumazet @ 2011-12-21 8:53 UTC (permalink / raw)
To: David Miller; +Cc: mchan, netdev
In-Reply-To: <20111219.154804.255413353396711289.davem@davemloft.net>
Le lundi 19 décembre 2011 à 15:48 -0500, David Miller a écrit :
> From: "Michael Chan" <mchan@broadcom.com>
> Date: Sun, 18 Dec 2011 20:15:09 -0800
>
> > bnx2-mips-06-6.2.3 and bnx2-mips-09-6.2.1.b
> >
> > New firmware fixes iSCSI problems with some LeftHand targets that don't
> > set TTT=0xffffffff for Data-In according to spec. Firmware generates
> > exception warnings for this condition and becomes very slow. This is
> > fixed by suppressing these warnings when using default error mask.
> >
> > Signed-off-by: Michael Chan <mchan@broadcom.com>
>
> Applied.
> --
But... where I am supposed to find this firmware ?
Its _not_ in linux-firmware right now
[ 259.352808] bnx2: Can't load firmware file "bnx2/bnx2-mips-06-6.2.3.fw"
[ 897.157266] bnx2: Can't load firmware file "bnx2/bnx2-mips-06-6.2.3.fw"
[ 1019.944049] bnx2: Can't load firmware file "bnx2/bnx2-mips-06-6.2.3.fw"
^ permalink raw reply
* Re: [PATCH linux-firmware] linux-firmware: bnx2: Update mips firmware to fix iSCSI problems
From: Eric Dumazet @ 2011-12-21 8:55 UTC (permalink / raw)
To: Michael Chan; +Cc: dwmw2, netdev
In-Reply-To: <1324268012-27719-1-git-send-email-mchan@broadcom.com>
Le dimanche 18 décembre 2011 à 20:13 -0800, Michael Chan a écrit :
> New firmware fixes iSCSI problems with some LeftHand targets that don't
> set TTT=0xffffffff for Data-In according to spec. Firmware generates
> exception warnings for this condition and becomes very slow. This is
> fixed by suppressing these warnings when using default error mask.
>
> Signed-off-by: Michael Chan <mchan@broadcom.com>
> ---
> WHENCE | 2 ++
> bnx2/bnx2-mips-06-6.2.3.fw | Bin 0 -> 92824 bytes
> bnx2/bnx2-mips-09-6.2.1b.fw | Bin 0 -> 103904 bytes
> 3 files changed, 2 insertions(+), 0 deletions(-)
> create mode 100644 bnx2/bnx2-mips-06-6.2.3.fw
> create mode 100644 bnx2/bnx2-mips-09-6.2.1b.fw
IMHO, this should be applied _before_ the drivers/net patch.
^ permalink raw reply
* Re: linux-3.0.x regression with ipv4 routes having mtu
From: Steffen Klassert @ 2011-12-21 8:56 UTC (permalink / raw)
To: David Miller; +Cc: timo.teras, netdev
In-Reply-To: <20111220.133542.2144336048061483258.davem@davemloft.net>
On Tue, Dec 20, 2011 at 01:35:42PM -0500, David Miller wrote:
> From: Steffen Klassert <steffen.klassert@secunet.com>
> Date: Tue, 20 Dec 2011 08:18:43 +0100
>
> > On Tue, Dec 20, 2011 at 02:03:55AM -0500, David Miller wrote:
> >> From: Timo Teräs <timo.teras@iki.fi>
> >> Date: Tue, 20 Dec 2011 08:53:09 +0200
> >>
> >> > Or maybe I missed the place where that updated would happen?
> >>
> >> It should happen on every routing cache lookup hit just like we
> >> validate the peer for redirect information.
> >
> > The problem is that we need to do a route cache lookup to
> > validate the peer informations. This does not happen if
> > somebody adds a new route. I tried already to add a pmtu specific
> > generation id and it appears to not solve the problem. We would
> > still need to overwrite the cached value if we add a route with mtu.
>
> Why?
>
> Every use of a routing cache entry does a dst_check() or a routing
> cache lookup.
>
> You can check the generation ID in both locations, and if the
> comparison fails then you force the routing cache entry to be killed
> and the caller performs a lookup. At that time the refreshing of the
> new FIB entry will be realized.
My decscription was a bit missleading, sorry. This seems to be not
even related to pmtu handling. It's just that we don't use the
user configured metrics on the fib entry if we find an inetpeer
with metrics that are not in INETPEER_METRICS_NEW state when we
initialize the routing metrics in rt_init_metrics(). I tried to add
a route with a hoplimit, this has also no effect if the metrics on
the inetpeer are not new.
^ permalink raw reply
* Re: [PATCH V2] IPv6 : add multicast routing verify which net_device is lo
From: Wang Xingtong @ 2011-12-21 9:01 UTC (permalink / raw)
To: David Miller; +Cc: gaofeng, netdev
In-Reply-To: <20111221.011506.1770356582907889299.davem@davemloft.net>
> From: Gao feng <gaofeng@cn.fujitsu.com>
> Date: Tue, 20 Dec 2011 19:10:24 +0800
>
>> In currently routing subsystem, when we lookup a multicast routing
>> to send muticast packets to outside, rt6_device_match will return
>> the rt6_info which it's match first. If we add a multicast route on
>> loopback devices beforce the others interface, rt6_device_match will
>> retrun the rt6_info which rt6i_dev->name is "lo". But, obviously,
>> we can't send a muticast packet to outside using loopback devices.
>> It case all multicast packets blocking.
>>
>> Commit 4af04aba93f47699e disabled kernel add multicast route on lo
>> automatically. However, we can't surmise the routing-add order or
>> interdict add multicast routing on loopback devices in user space.
>> The bug still exist. So, i think, more stronger routing subsystem is
>> necessary.
>>
>> Signed-off-by: Wang xingtong <wangxingtong@cn.fujitsu.com>
>
> Ok, this is getting rediculious. I want to revert all of this
> stuff.
>
> How about, instead, we stop userland adding explicit addresses to the
> loopback device since that's what started behaving differently
> recently and causes these problems in the first place?
OK, David, I reproduce this as following :
1) ip -6 route show | grep ff00
unreachable ff00::/8 dev lo metric 1024 error -101
ff00::/8 dev eth1 metric 1024
2) ip -6 route del ff00::/8 dev eth1
ip -6 route del ff00::/8 dev lo
3) ip -6 route add ff00::/8 dev lo
ip -6 route add ff00::/8 dev eth1
now, if we join to the multicast group with the interface index is
specified as 0, not restrict devices( oif == 0 ), not restrict saddr (
saddr == :: ), rt6_device_match will return rt6_info which
rt6i_dev->name is "lo" all the while, and rt6i_dev->error is
"-ENETUNREACH". we got "ENODEV" at userspace, all multicast packets
will be blocked .
But, in fact, eth1 can be uesd and we missed. This is a bug in routing
system， isn't it？
This patch add multicast routing check-up in rt6_device_match to arrest
it return loopback device when we isn't specified interface and saddr.
thanks,
wang xingtong
^ permalink raw reply
* Re: [PATCH RFC] virtio_net: fix refill related races
From: Michael S. Tsirkin @ 2011-12-21 9:06 UTC (permalink / raw)
To: Rusty Russell; +Cc: Tejun Heo, Amit Shah, netdev, linux-kernel, virtualization
In-Reply-To: <87hb0udd2h.fsf@rustcorp.com.au>
On Wed, Dec 21, 2011 at 10:13:18AM +1030, Rusty Russell wrote:
> On Tue, 20 Dec 2011 21:45:19 +0200, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > On Tue, Dec 20, 2011 at 11:31:54AM -0800, Tejun Heo wrote:
> > > On Tue, Dec 20, 2011 at 09:30:55PM +0200, Michael S. Tsirkin wrote:
> > > > Hmm, in that case it looks like a nasty race could get
> > > > triggered, with try_fill_recv run on multiple CPUs in parallel,
> > > > corrupting the linked list within the vq.
> > > >
> > > > Using the mutex as my patch did will fix that naturally, as well.
> > >
> > > Don't know the code but just use nrt wq. There's even a system one
> > > called system_nrq_wq.
> > >
> > > Thanks.
> >
> > We can, but we need the mutex for other reasons, anyway.
>
> Well, here's the alternate approach. What do you think?
It looks very clean, thanks. Some documentation suggestions below.
Also - Cc stable on this and the block patch?
> Finding two wq issues makes you justifiably cautious, but it almost
> feels like giving up to simply wrap it in a lock. The APIs are designed
> to let us do it without a lock; I was just using them wrong.
One thing I note is that this scheme works because there's a single
entity disabling/enabling napi and the refill thread.
So it's possible that Amit will need to add a lock and track NAPI
state anyway to make suspend work. But we'll see.
> Two patches in one mail. It's gauche, but it's an RFC only (untested).
>
> Cheers,
> Rusty.
>
> virtio_net: set/cancel work on ndo_open/ndo_stop
>
> Michael S. Tsirkin noticed that we could run the refill work after
> ndo_close, which can re-enable napi - we don't disable it until
> virtnet_remove. This is clearly wrong, so move the workqueue control
> to ndo_open and ndo_stop (aka. virtnet_open and virtnet_close).
>
> One subtle point: virtnet_probe() could simply fail if it couldn't
> allocate a receive buffer, but that's less polite in virtnet_open() so
> we schedule a refill as we do in the normal receive path if we run out
> of memory.
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -719,6 +719,10 @@ static int virtnet_open(struct net_devic
> {
> struct virtnet_info *vi = netdev_priv(dev);
>
> + /* Make sure we have some buffersl if oom use wq. */
Typo :)
> + if (!try_fill_recv(vi, GFP_KERNEL))
> + schedule_delayed_work(&vi->refill, 0);
> +
> virtnet_napi_enable(vi);
> return 0;
> }
> @@ -772,6 +776,8 @@ static int virtnet_close(struct net_devi
> {
> struct virtnet_info *vi = netdev_priv(dev);
>
> + /* Make sure refill_work doesn't re-enable napi! */
> + cancel_delayed_work_sync(&vi->refill);
> napi_disable(&vi->napi);
>
> return 0;
> @@ -1082,7 +1088,6 @@ static int virtnet_probe(struct virtio_d
>
> unregister:
> unregister_netdev(dev);
> - cancel_delayed_work_sync(&vi->refill);
> free_vqs:
> vdev->config->del_vqs(vdev);
> free_stats:
> @@ -1121,9 +1126,7 @@ static void __devexit virtnet_remove(str
> /* Stop all the virtqueues. */
> vdev->config->reset(vdev);
>
> -
> unregister_netdev(vi->dev);
> - cancel_delayed_work_sync(&vi->refill);
>
> /* Free unused buffers in both send and recv, if any. */
> free_unused_bufs(vi);
>
>
> virtio_net: use non-reentrant workqueue.
>
> Michael S. Tsirkin also noticed that we could run the refill work
> multiple CPUs: if we kick off a refill on one CPU and then on another,
> they would both manipulate the queue at the same time (they use
> napi_disable to avoid racing against the receive handler itself).
>
> Tejun points out that this is what the WQ_NON_REENTRANT flag is for,
> and that there is a convenient system kthread we can use.
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -501,7 +501,7 @@ static void refill_work(struct work_stru
> /* In theory, this can happen: if we don't get any buffers in
> * we will *never* try to fill again. */
> if (still_empty)
> - schedule_delayed_work(&vi->refill, HZ/2);
> + queue_delayed_work(system_nrt_wq, &vi->refill, HZ/2);
> }
>
> static int virtnet_poll(struct napi_struct *napi, int budget)
> @@ -520,7 +520,7 @@ again:
>
> if (vi->num < vi->max / 2) {
> if (!try_fill_recv(vi, GFP_ATOMIC))
> - schedule_delayed_work(&vi->refill, 0);
> + queue_delayed_work(system_nrt_wq, &vi->refill, 0);
> }
>
> /* Out of packets? */
> @@ -721,7 +721,7 @@ static int virtnet_open(struct net_devic
>
> /* Make sure we have some buffersl if oom use wq. */
> if (!try_fill_recv(vi, GFP_KERNEL))
> - schedule_delayed_work(&vi->refill, 0);
> + queue_delayed_work(&system_nrt_wq, &vi->refill, 0);
>
> virtnet_napi_enable(vi);
> return 0;
Maybe document some rules:
- try_fill_recv must always run from napi or with napi disabled ...
- refill only runs when device is open
--
MST
^ permalink raw reply
* Re: tcp_mtu_probe implementation details
From: Anatoly Sivov @ 2011-12-21 9:50 UTC (permalink / raw)
To: Vijay Subramanian; +Cc: netdev
In-Reply-To: <CAGK4HS93u=3muZu_rkqK7FSacTHUytK5H6_dMNo_uOQY738R_g@mail.gmail.com>
Hello Vijay,
Thank you for your response.
>> The other question is about size_needed variable.
>> It is assigned to value probe_size + (tp->reordering + 1) *
>> tp->mss_cache
>> And that is not clear for me.
>> What is this "(tp->reordering + 1) * tp->mss_cache" addition?
>>
>
> I think the idea is that you want enough bytes in the write_queue so
> that even if the probe is lost, the sender will get an ack even if
> there is reordering in the network. Without sufficient bytes, the
> probe will not be sent. This is what I make of the code but I could be
> wrong.
I believe, I found the explanation of this addition in RFC 4821:
"TCP Fast Retransmit is not robust unless there are
sufficient segments following a probe; that is, the sender SHOULD
have enough data queued and sufficient receiver window to send the
probe plus at least Tcprexmtthresh [RFC2760] additional segments."
However, I'm still confused with magic number 11 in "tp->snd_cwnd < 11"
check.
Thanks.
^ permalink raw reply
* Re: Kernel-DOS error in arp mechanism – no delete off incomplete arp adresses
From: Eric Dumazet @ 2011-12-21 9:51 UTC (permalink / raw)
To: David Miller; +Cc: richard.weinberger, gladewitz, linux-kernel, netdev
In-Reply-To: <20111221.030727.1528369698756365464.davem@davemloft.net>
Le mercredi 21 décembre 2011 à 03:07 -0500, David Miller a écrit :
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Wed, 21 Dec 2011 08:44:27 +0100
>
> > David, I suggest we add back the garbage collector for current kernels,
> > we'll remove it when route cache really disappear ?
> >
> > I'll send a patch today.
>
> Yes, it's the best idea.
>
> We can actually remove it again as early as when when route neigh's
> are ref-less.
Here is the patch I successfully tested in the neighbour stress
situation. This is a stable candidate (2.6.39+)
Thanks !
[PATCH] ipv4: reintroduce route cache garbage collector
Commit 2c8cec5c10b (ipv4: Cache learned PMTU information in inetpeer)
removed IP route cache garbage collector a bit too soon, as this gc was
responsible for expired routes cleanup, releasing their neighbour
reference.
As pointed out by Robert Gladewitz, recent kernels can fill and exhaust
their neighbour cache.
Reintroduce the garbage collection, since we'll have to wait our
neighbour lookups become refcount-less to not depend on this stuff.
Reported-by: Robert Gladewitz <gladewitz@gmx.de>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
net/ipv4/route.c | 107 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 107 insertions(+)
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 46af623..252c512 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -120,6 +120,7 @@
static int ip_rt_max_size;
static int ip_rt_gc_timeout __read_mostly = RT_GC_TIMEOUT;
+static int ip_rt_gc_interval __read_mostly = 60 * HZ;
static int ip_rt_gc_min_interval __read_mostly = HZ / 2;
static int ip_rt_redirect_number __read_mostly = 9;
static int ip_rt_redirect_load __read_mostly = HZ / 50;
@@ -133,6 +134,9 @@ static int ip_rt_min_advmss __read_mostly = 256;
static int rt_chain_length_max __read_mostly = 20;
static int redirect_genid;
+static struct delayed_work expires_work;
+static unsigned long expires_ljiffies;
+
/*
* Interface to generic destination cache.
*/
@@ -830,6 +834,97 @@ static int has_noalias(const struct rtable *head, const struct rtable *rth)
return ONE;
}
+static void rt_check_expire(void)
+{
+ static unsigned int rover;
+ unsigned int i = rover, goal;
+ struct rtable *rth;
+ struct rtable __rcu **rthp;
+ unsigned long samples = 0;
+ unsigned long sum = 0, sum2 = 0;
+ unsigned long delta;
+ u64 mult;
+
+ delta = jiffies - expires_ljiffies;
+ expires_ljiffies = jiffies;
+ mult = ((u64)delta) << rt_hash_log;
+ if (ip_rt_gc_timeout > 1)
+ do_div(mult, ip_rt_gc_timeout);
+ goal = (unsigned int)mult;
+ if (goal > rt_hash_mask)
+ goal = rt_hash_mask + 1;
+ for (; goal > 0; goal--) {
+ unsigned long tmo = ip_rt_gc_timeout;
+ unsigned long length;
+
+ i = (i + 1) & rt_hash_mask;
+ rthp = &rt_hash_table[i].chain;
+
+ if (need_resched())
+ cond_resched();
+
+ samples++;
+
+ if (rcu_dereference_raw(*rthp) == NULL)
+ continue;
+ length = 0;
+ spin_lock_bh(rt_hash_lock_addr(i));
+ while ((rth = rcu_dereference_protected(*rthp,
+ lockdep_is_held(rt_hash_lock_addr(i)))) != NULL) {
+ prefetch(rth->dst.rt_next);
+ if (rt_is_expired(rth)) {
+ *rthp = rth->dst.rt_next;
+ rt_free(rth);
+ continue;
+ }
+ if (rth->dst.expires) {
+ /* Entry is expired even if it is in use */
+ if (time_before_eq(jiffies, rth->dst.expires)) {
+nofree:
+ tmo >>= 1;
+ rthp = &rth->dst.rt_next;
+ /*
+ * We only count entries on
+ * a chain with equal hash inputs once
+ * so that entries for different QOS
+ * levels, and other non-hash input
+ * attributes don't unfairly skew
+ * the length computation
+ */
+ length += has_noalias(rt_hash_table[i].chain, rth);
+ continue;
+ }
+ } else if (!rt_may_expire(rth, tmo, ip_rt_gc_timeout))
+ goto nofree;
+
+ /* Cleanup aged off entries. */
+ *rthp = rth->dst.rt_next;
+ rt_free(rth);
+ }
+ spin_unlock_bh(rt_hash_lock_addr(i));
+ sum += length;
+ sum2 += length*length;
+ }
+ if (samples) {
+ unsigned long avg = sum / samples;
+ unsigned long sd = int_sqrt(sum2 / samples - avg*avg);
+ rt_chain_length_max = max_t(unsigned long,
+ ip_rt_gc_elasticity,
+ (avg + 4*sd) >> FRACT_BITS);
+ }
+ rover = i;
+}
+
+/*
+ * rt_worker_func() is run in process context.
+ * we call rt_check_expire() to scan part of the hash table
+ */
+static void rt_worker_func(struct work_struct *work)
+{
+ rt_check_expire();
+ schedule_delayed_work(&expires_work, ip_rt_gc_interval);
+}
+
/*
* Perturbation of rt_genid by a small quantity [1..256]
* Using 8 bits of shuffling ensure we can call rt_cache_invalidate()
@@ -3179,6 +3274,13 @@ static ctl_table ipv4_route_table[] = {
.proc_handler = proc_dointvec_jiffies,
},
{
+ .procname = "gc_interval",
+ .data = &ip_rt_gc_interval,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec_jiffies,
+ },
+ {
.procname = "redirect_load",
.data = &ip_rt_redirect_load,
.maxlen = sizeof(int),
@@ -3388,6 +3490,11 @@ int __init ip_rt_init(void)
devinet_init();
ip_fib_init();
+ INIT_DELAYED_WORK_DEFERRABLE(&expires_work, rt_worker_func);
+ expires_ljiffies = jiffies;
+ schedule_delayed_work(&expires_work,
+ net_random() % ip_rt_gc_interval + ip_rt_gc_interval);
+
if (ip_rt_proc_init())
printk(KERN_ERR "Unable to create route proc files\n");
#ifdef CONFIG_XFRM
^ permalink raw reply related
* RE: [PATCH 1/1] tcp: Replace constants with #define macros
From: David Laight @ 2011-12-21 9:57 UTC (permalink / raw)
To: David Miller, subramanian.vijay
Cc: netdev, eric.dumazet, ilpo.jarvinen, hkchu
In-Reply-To: <20111220.151809.1704705119739851376.davem@davemloft.net>
> > I assume you are talking about collision with userspace. These
> > definitions are already protected by #ifdef __KERNEL__.
> > That should be enough to protect against collisions with
> userspace I think.
>
> Within the kernel he means, please put a TCP_* prefix onto these
> values.
Given the values are currently:
> > +#define SACK_SEEN (1 << 0) /*1 = peer is SACK capable, */
> > +#define FACK_ENABLED (1 << 1) /*1 = FACK is enabled locally*/
> > +#define DSACK_SEEN (1 << 2) /*1 = DSACK was received from
peer*/
But the structure field member is 'sack_ok', unless it generates
very silly long lines - which in this case it probably doesn't -
it might even be worth prefixing with TCP_SACK_OK_ although
that might be deemed excessive!
David
^ permalink raw reply
* ICMP packets - ll_temac with Microblaze
From: Michal Simek @ 2011-12-21 10:11 UTC (permalink / raw)
To: Eric Dumazet, David Miller, John Williams; +Cc: netdev
Hi Eric and David,
I have found one problem with ll_temac driver and
this commit: net: more accurate skb truesize
sha1: 87fb4b7b533073eeeaed0b6bf7c2328995f6c075
The problem is only with icmp packets from the target. It is sent and driver receive it
but it is not proceed to the application.
The problem I see is that kmalloc_node_track_caller allocate
specific size and then this size is changed by SKB_WITH_OVERHEAD(ksize(data)).
The problem is with netdev_alloc_skb_ip_align which calls __alloc_skb function.
Currently driver uses setting for jumbo frames (9k). When I change it to use mtu (1,5k) then
everything is ok.
Can you give me some hints what can be wrong?
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian
^ permalink raw reply
* Re: ICMP packets - ll_temac with Microblaze
From: Eric Dumazet @ 2011-12-21 10:28 UTC (permalink / raw)
To: monstr; +Cc: David Miller, John Williams, netdev
In-Reply-To: <4EF1B0D9.2010007@monstr.eu>
Le mercredi 21 décembre 2011 à 11:11 +0100, Michal Simek a écrit :
> Hi Eric and David,
>
> I have found one problem with ll_temac driver and
> this commit: net: more accurate skb truesize
> sha1: 87fb4b7b533073eeeaed0b6bf7c2328995f6c075
>
> The problem is only with icmp packets from the target. It is sent and driver receive it
> but it is not proceed to the application.
>
> The problem I see is that kmalloc_node_track_caller allocate
> specific size and then this size is changed by SKB_WITH_OVERHEAD(ksize(data)).
> The problem is with netdev_alloc_skb_ip_align which calls __alloc_skb function.
>
> Currently driver uses setting for jumbo frames (9k). When I change it to use mtu (1,5k) then
> everything is ok.
>
> Can you give me some hints what can be wrong?
>
Is it with SLUB, SLAB or SLOB allocator ?
^ permalink raw reply
* Re: ICMP packets - ll_temac with Microblaze
From: Michal Simek @ 2011-12-21 10:30 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, John Williams, netdev
In-Reply-To: <1324463334.2728.20.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>
Eric Dumazet wrote:
> Le mercredi 21 décembre 2011 à 11:11 +0100, Michal Simek a écrit :
>> Hi Eric and David,
>>
>> I have found one problem with ll_temac driver and
>> this commit: net: more accurate skb truesize
>> sha1: 87fb4b7b533073eeeaed0b6bf7c2328995f6c075
>>
>> The problem is only with icmp packets from the target. It is sent and driver receive it
>> but it is not proceed to the application.
>>
>> The problem I see is that kmalloc_node_track_caller allocate
>> specific size and then this size is changed by SKB_WITH_OVERHEAD(ksize(data)).
>> The problem is with netdev_alloc_skb_ip_align which calls __alloc_skb function.
>>
>> Currently driver uses setting for jumbo frames (9k). When I change it to use mtu (1,5k) then
>> everything is ok.
>>
>> Can you give me some hints what can be wrong?
>>
>
> Is it with SLUB, SLAB or SLOB allocator ?
SLAB.
CONFIG_SLAB=y
# CONFIG_SLUB is not set
# CONFIG_SLOB is not set
Michal
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian
^ permalink raw reply
* Re: ICMP packets - ll_temac with Microblaze
From: Eric Dumazet @ 2011-12-21 10:30 UTC (permalink / raw)
To: monstr; +Cc: David Miller, John Williams, netdev
In-Reply-To: <1324463334.2728.20.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>
Le mercredi 21 décembre 2011 à 11:28 +0100, Eric Dumazet a écrit :
> Le mercredi 21 décembre 2011 à 11:11 +0100, Michal Simek a écrit :
> > Hi Eric and David,
> >
> > I have found one problem with ll_temac driver and
> > this commit: net: more accurate skb truesize
> > sha1: 87fb4b7b533073eeeaed0b6bf7c2328995f6c075
> >
> > The problem is only with icmp packets from the target. It is sent and driver receive it
> > but it is not proceed to the application.
> >
> > The problem I see is that kmalloc_node_track_caller allocate
> > specific size and then this size is changed by SKB_WITH_OVERHEAD(ksize(data)).
> > The problem is with netdev_alloc_skb_ip_align which calls __alloc_skb function.
> >
> > Currently driver uses setting for jumbo frames (9k). When I change it to use mtu (1,5k) then
> > everything is ok.
> >
> > Can you give me some hints what can be wrong?
> >
>
> Is it with SLUB, SLAB or SLOB allocator ?
>
>
(I am referring to commit bc417e30f8d
(net: Add back alignment for size for __alloc_skb)
^ permalink raw reply
* Re: ICMP packets - ll_temac with Microblaze
From: Michal Simek @ 2011-12-21 10:32 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, John Williams, netdev
In-Reply-To: <1324463452.2728.21.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>
Eric Dumazet wrote:
> Le mercredi 21 décembre 2011 à 11:28 +0100, Eric Dumazet a écrit :
>> Le mercredi 21 décembre 2011 à 11:11 +0100, Michal Simek a écrit :
>>> Hi Eric and David,
>>>
>>> I have found one problem with ll_temac driver and
>>> this commit: net: more accurate skb truesize
>>> sha1: 87fb4b7b533073eeeaed0b6bf7c2328995f6c075
>>>
>>> The problem is only with icmp packets from the target. It is sent and driver receive it
>>> but it is not proceed to the application.
>>>
>>> The problem I see is that kmalloc_node_track_caller allocate
>>> specific size and then this size is changed by SKB_WITH_OVERHEAD(ksize(data)).
>>> The problem is with netdev_alloc_skb_ip_align which calls __alloc_skb function.
>>>
>>> Currently driver uses setting for jumbo frames (9k). When I change it to use mtu (1,5k) then
>>> everything is ok.
>>>
>>> Can you give me some hints what can be wrong?
>>>
>> Is it with SLUB, SLAB or SLOB allocator ?
>>
>>
>
> (I am referring to commit bc417e30f8d
> (net: Add back alignment for size for __alloc_skb)
I have seen that commit. Using the latest&greatest version with this patch.
Michal
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian
^ permalink raw reply
* Re: ICMP packets - ll_temac with Microblaze
From: Eric Dumazet @ 2011-12-21 10:37 UTC (permalink / raw)
To: monstr; +Cc: David Miller, John Williams, netdev
In-Reply-To: <4EF1B5BC.5020103@monstr.eu>
Le mercredi 21 décembre 2011 à 11:32 +0100, Michal Simek a écrit :
> Eric Dumazet wrote:
> > Le mercredi 21 décembre 2011 à 11:28 +0100, Eric Dumazet a écrit :
> >> Le mercredi 21 décembre 2011 à 11:11 +0100, Michal Simek a écrit :
> >>> Hi Eric and David,
> >>>
> >>> I have found one problem with ll_temac driver and
> >>> this commit: net: more accurate skb truesize
> >>> sha1: 87fb4b7b533073eeeaed0b6bf7c2328995f6c075
> >>>
> >>> The problem is only with icmp packets from the target. It is sent and driver receive it
> >>> but it is not proceed to the application.
> >>>
> >>> The problem I see is that kmalloc_node_track_caller allocate
> >>> specific size and then this size is changed by SKB_WITH_OVERHEAD(ksize(data)).
> >>> The problem is with netdev_alloc_skb_ip_align which calls __alloc_skb function.
> >>>
> >>> Currently driver uses setting for jumbo frames (9k). When I change it to use mtu (1,5k) then
> >>> everything is ok.
> >>>
> >>> Can you give me some hints what can be wrong?
> >>>
> >> Is it with SLUB, SLAB or SLOB allocator ?
> >>
> >>
> >
> > (I am referring to commit bc417e30f8d
> > (net: Add back alignment for size for __alloc_skb)
>
> I have seen that commit. Using the latest&greatest version with this patch.
>
So after these changes, struct skb_shared_info is located to the very
end of allocated memory.
Maybe there is a problem with MTU=9000, since assuming PAGE_SIZE=4096 on
your machine (is it ?), we have a 16384 bytes block of memory, and
~16000 available in skb head, instead of ~9000
^ 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