* Fwd: KS8851 SPI Driver Integration
From: Abraham Arce @ 2009-11-22 0:16 UTC (permalink / raw)
To: ben; +Cc: netdev
In-Reply-To: <cb8016980911211610g53a958e8m100238cdcb89cff0@mail.gmail.com>
Hi Ben,
I am currently working with ks8851snl driver and facing some issues,
the system integrated with is OMAP4, a ARM Cortex A9 Dual Core. I am
using the following commit as latest
commit 94da210af4978b94cb70318bd1b282a73c50b175
Author: Simon Kagstrom <simon.kagstrom@netinsight.net>
net: fix ks8851 build errors
Fix build errors due to missing Kconfig select of CRC32:
ks8851.c:(.text+0x7d2ee): undefined reference to `crc32_le'
ks8851.c:(.text+0x7d2f5): undefined reference to `bitrev32'
I can summarize the problems as follows:
* Driver won't be able to get ip address via dhcp if SMP/Preemption/L2
Cache are enabled, I have not enabled/disabled one by one to narrow
down the problem...
* Having one single core running, dhcp works, I can ping with small
size packets but cannot mount any nfs directory, 2 different scenarios
listed below, nfs server is up
* Timeout when trying to mount a nfs directory from a system already
booted, if I try to cancel the request then the following messages
will be seen, console access cannot be recovered immediately
rpcbind: server localhost not responding, timed out
CTRL-C
svc: failed to register lockdv1 RPC service (errno 5).
lockd_up: makesock failed, error=-5
* Kernel NULL pointer when trying to mount a nfs directory using
bootargs or after some time of pinging to any other ip
Unable to handle kernel NULL pointer dereference at virtual address 00000054
pgd = c0004000
[00000054] *pgd=00000000
Internal error: Oops: 5 [#1]
Modules linked in:
CPU: 0 Not tainted (2.6.31-00145-g85deb91-dirty #12)
PC is at ks8851_tx_work+0x74/0x19c
LR is at ks8851_tx_work+0x70/0x19c
pc : [<c01b3c04>] lr : [<c01b3c00>] psr: a0000013
sp : cf82ff50 ip : cf82ff50 fp : cf82ff8c
r10: cf85e2e1 r9 : cf85e3a0 r8 : cf85e2c8
r7 : cf85e368 r6 : 00000001 r5 : 00000000 r4 : cf85e2c0
r3 : c03316b8 r2 : 00000001 r1 : 00000000 r0 : c02cbd5d
Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment kernel
Control: 10c53c7d Table: 80004059 DAC: 00000017
Process events/0 (pid: 4, stack limit = 0xcf82e2e8)
Stack: (0xcf82ff50 to 0xcf830000)
ff40: 00000000 0000006e cf99f876 00000001
ff60: c0259ac8 c01b3b90 cf800388 cf800380 cf82e000 00000000 00000000 00000000
ff80: cf82ffc4 cf82ff90 c004e57c c01b3b9c cf82ffc4 00000000 cf822960 c0051e34
ffa0: cf82ffa0 cf82ffa0 cf82ffcc cf825f20 cf800380 c004e474 cf82fff4 cf82ffc8
ffc0: c0051a14 c004e480 00000000 00000000 cf82ffd0 cf82ffd0 00000000 00000000
ffe0: 00000000 00000000 00000000 cf82fff8 c0041de8 c0051998 ffffffff 00000000
Backtrace:
[<c01b3b90>] (ks8851_tx_work+0x0/0x19c) from [<c004e57c>] (worker_thread+0x108/0
x180)
[<c004e474>] (worker_thread+0x0/0x180) from [<c0051a14>] (kthread+0x88/0x90)
r7:c004e474 r6:cf800380 r5:cf825f20 r4:cf82ffcc
[<c005198c>] (kthread+0x0/0x90) from [<c0041de8>] (do_exit+0x0/0x580)
r7:00000000 r6:00000000 r5:00000000 r4:00000000
Code: 0a00000d e5940004 ebffd711 e58d5000 (e5952054)
---[ end trace 1c6069e68868725a ]---
ks8851 spi1.0: ks8851_start_xmit: skb cf91c208, 110@cf99f876
ks8851 spi1.0: ks8851_start_xmit: skb cf8e83c0, 110@cf9c8076
I'll appreciate any advice about these issues...
Best Regards
Abraham
^ permalink raw reply
* Re: Bridge + Conntrack + SKB Recycle: Fragment Reassembly Errors
From: Patrick McHardy @ 2009-11-22 0:21 UTC (permalink / raw)
To: David Miller; +Cc: ben, netdev
In-Reply-To: <20091121.110832.213888237.davem@davemloft.net>
David Miller wrote:
> From: Patrick McHardy <kaber@trash.net>
> Date: Tue, 10 Nov 2009 17:50:38 +0100
>
>> This code in ip_fragment() looks suspicious:
>>
>> if (skb_has_frags(skb)) {
>> ...
>> skb_walk_frags(skb, frag) {
>> ...
>> if (skb->sk) {
>> frag->sk = skb->sk;
>> frag->destructor = sock_wfree;
>> truesizes += frag->truesize;
>> }
>>
>> truesizes is later used to adjust truesize of the head skb.
>> For some reason this is only done when it originated from a
>> local socket.
>
> Well, it shouldn't look _that_ suspicious.
>
> What this code is doing is making sure that after we make all of these
> changes, the truesize of the SKBs referrng to the socket do not
> change.
>
> It's simply making sure that the math works out when all the
> sock_wfree() calls occur later.
>
> If we don't have a socket involved, there is no reason to make
> these adjustments.
That seems to be the assumption. But ip_defrag() uses skb->truesize
as well to make sure the defragmentation memory limits are not exceeded.
In this case what seems to be happening is:
- gianfar with skb recycling enabled receives a number of fragments
on a bridge (~64000b total)
- conntrack defragments the packet using ip_defrag(), which causes
skb->truesize of the head fragment to account for all the fragments
- the packet is refragmented in the bridging code using ip_fragment().
This doesn't re-adjust skb->truesize of the head fragment when the
packet is not associated with a socket
- the head is recycled in gianfar
- another fragment is received and reuses the recycled skb with a
huge truesize
- the defragmentation limits are exceeded due to the huge truesize
So it seems we need to adjust skb->truesize in ip_fragment() since
skb_recycle_check() assumes the skb is linear (and therefore
skb->truesize reflects the linear size). Ben's suggestions of adding
an upper limit based on the requested size to skb_recycle_check()
makes sense to me as well to avoid this problem when recycling large
linear skbs.
^ permalink raw reply
* Re: Bridge + Conntrack + SKB Recycle: Fragment Reassembly Errors
From: Patrick McHardy @ 2009-11-22 0:29 UTC (permalink / raw)
To: ben; +Cc: David Miller, netdev
In-Reply-To: <4B0883FD.2090806@trash.net>
[-- Attachment #1: Type: text/plain, Size: 416 bytes --]
Patrick McHardy wrote:
> So it seems we need to adjust skb->truesize in ip_fragment() since
> skb_recycle_check() assumes the skb is linear (and therefore
> skb->truesize reflects the linear size). Ben's suggestions of adding
> an upper limit based on the requested size to skb_recycle_check()
> makes sense to me as well to avoid this problem when recycling large
> linear skbs.
Ben, please give this patch a try.
[-- Attachment #2: x --]
[-- Type: text/plain, Size: 428 bytes --]
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index 322b408..031989d 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -501,8 +501,8 @@ int ip_fragment(struct sk_buff *skb, int (*output)(struct sk_buff *))
if (skb->sk) {
frag->sk = skb->sk;
frag->destructor = sock_wfree;
- truesizes += frag->truesize;
}
+ truesizes += frag->truesize;
}
/* Everything is OK. Generate! */
^ permalink raw reply related
* Re: [Resend][PATCH] Wireless / ath5k: Simplify suspend and resume callbacks
From: Bob Copeland @ 2009-11-22 3:18 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: ath5k-devel-xDcbHBWguxEUs3QNXV6qNA, John W. Linville,
linux-wireless-u79uwXL29TY76Z2rM5mHXA, LKML, pm list,
netdev-u79uwXL29TY76Z2rM5mHXA, Luis R. Rodriguez
In-Reply-To: <200911212344.24170.rjw-KKrjLPT3xs0@public.gmane.org>
On Sat, Nov 21, 2009 at 11:44:24PM +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw-KKrjLPT3xs0@public.gmane.org>
> Subject: Wireless / ath5k: Simplify suspend and resume callbacks
>
> Simplify the suspend and resume callbacks of ath5k by converting the
> driver to struct dev_pm_ops and allowing the PCI PM core to do the
> PCI-specific suspend/resume handling.
>
> Tested with Atheros Communications Inc. AR2413 802.11bg NIC (rev 01)
> D-Link CardBus.adapter.
>
> Signed-off-by: Rafael J. Wysocki <rjw-KKrjLPT3xs0@public.gmane.org>
Acked-by: Bob Copeland <me-aXfl/3sk2vNUbtYUoyoikg@public.gmane.org>
Thanks for the resend, sorry it got lost in the shuffle.
--
Bob Copeland %% www.bobcopeland.com
--
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: [net-next-2.6 PATCH v7 3/7 RFC] TCPCT part 1c: sysctl_tcp_cookie_size, socket option TCP_COOKIE_TRANSACTIONS
From: William Allen Simpson @ 2009-11-22 4:40 UTC (permalink / raw)
To: David Miller; +Cc: netdev, joe
In-Reply-To: <20091121.111844.54272535.davem@davemloft.net>
David Miller wrote:
> From: William Allen Simpson <william.allen.simpson@gmail.com>
> Date: Sat, 21 Nov 2009 11:51:13 -0500
>
>> This code was based on net/ipv4/tcp_minisocks.c for syncookies.
>> Apparently, default for syncookies has been controversial.
>
> I'll have to change that, it doesn't make any sense. Thanks
> for pointing it out.
>
Once again, for the umpteenth time, I learn that existing code
proves to be a bad example to follow....
>> Would you accept always default to TCP_COOKIE_MAX?
>
> Sure, let's see what breaks.
>
Unlikely to break anything, according to studies of the subject:
Medina, A., Allman, M., and Floyd, S., "Measuring Interactions
Between Transport Protocols and Middleboxes", Proceedings 4th
ACM SIGCOMM/USENIX Conference on Internet Measurement, October
2004. http://www.icsi.berkeley.edu/pubs/networking/tbit-
Aug2004.pdf
TCPCT part 2 is much more likely to break things....
^ permalink raw reply
* Re: [net-next-2.6 PATCH v7 4/7 RFC] TCPCT part 1d: define TCP cookie option, extend existing struct's
From: William Allen Simpson @ 2009-11-22 4:53 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20091120.092515.126988512.davem@davemloft.net>
David Miller wrote:
> From: William Allen Simpson <william.allen.simpson@gmail.com>
> Date: Fri, 20 Nov 2009 09:38:12 -0500
>
>> static inline void tcp_clear_options(struct tcp_options_received *rx_opt)
>> {
>> - rx_opt->tstamp_ok = rx_opt->sack_ok = rx_opt->wscale_ok = rx_opt->snd_wscale = 0;
>> + rx_opt->tstamp_ok = rx_opt->sack_ok = 0;
>> + rx_opt->wscale_ok = rx_opt->snd_wscale = 0;
>> + rx_opt->cookie_plus = 0;
>> }
>>
>
> Why not get the coding style correct wrt. long lines in patch #3 where
> you initially added this function, rather than fixing it here as you
> add the new ->cookie_plus assignment?
>
Documentation/SubmittingPatches at 619:
One significant exception is when moving code from one file to
another -- in this case you should not modify the moved code at all in
the same patch which moves it. This clearly delineates the act of
moving the code and your changes. This greatly aids review of the
actual differences and allows tools to better track the history of
the code itself.
^ permalink raw reply
* Re: [net-next-2.6 PATCH v7 5/7 RFC] TCPCT part 1e: implement socket option TCP_COOKIE_TRANSACTIONS
From: William Allen Simpson @ 2009-11-22 6:25 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20091120.092651.254794724.davem@davemloft.net>
David Miller wrote:
> From: William Allen Simpson <william.allen.simpson@gmail.com>
> Date: Fri, 20 Nov 2009 09:48:12 -0500
>> + if (ctd.tcpct_used > 0
>> + || (tp->cookie_values == NULL
>> + && (sysctl_tcp_cookie_size > 0
>> + || ctd.tcpct_cookie_desired > 0
>> + || ctd.tcpct_s_data_desired > 0))) {
>
> Please fix the conditional coding style, and the alignment of
> the lines, it's not right here.
>
Eliding the repeated assertions, and focusing on this example.
This is not addressed (nor forbidden) in CodingStyle.
As Joe noted earlier with '?' and ':', there are ample examples
throughout the code base of this style, including here and there
among the tcp*.c files. Obviously, this is very easy to read!
Coding style is all about readability and maintainability
using commonly available tools.
However, grep shows that the "||" or "&&" is elsewhere most often
indented 4 for each level (although there is some inconsistency).
Either 1, 2, or 4 distinguishes <condition> indentation from
<statements> indented by tab (8).
In my experience, I've found 1 best, as that lines up variables
and parentheses levels, and it's easy to type. But it certainly
would be easy enough to indent by 2 or 4 instead.
BTW, the 'indent' program turns this into incomprehensible and
unmaintainable garbage. Of course, there are ample examples of
garbage in these tcp*.c files, too.... :-(
Therefore, David, would indentation of 2 or 4 be preferable here?
^ permalink raw reply
* Re: [net-next-2.6 PATCH v7 5/7 RFC] TCPCT part 1e: implement socket option TCP_COOKIE_TRANSACTIONS
From: Joe Perches @ 2009-11-22 7:10 UTC (permalink / raw)
To: William Allen Simpson; +Cc: David Miller, netdev
In-Reply-To: <4B08D967.80003@gmail.com>
On Sun, 2009-11-22 at 01:25 -0500, William Allen Simpson wrote:
> David Miller wrote:
> > From: William Allen Simpson <william.allen.simpson@gmail.com>
> > Date: Fri, 20 Nov 2009 09:48:12 -0500
> >> + if (ctd.tcpct_used > 0
> >> + || (tp->cookie_values == NULL
> >> + && (sysctl_tcp_cookie_size > 0
> >> + || ctd.tcpct_cookie_desired > 0
> >> + || ctd.tcpct_s_data_desired > 0))) {
> > Please fix the conditional coding style, and the alignment of
> > the lines, it's not right here.
> David, would indentation of 2 or 4 be preferable here?
Hi William.
I think the rather significantly majority style, especially
for net/... is to use || and && at the end of the line rather
than the start and it should be used.
Treewide:
$ grep -rP --include=*.[ch] "(\|\||\&\&)\s*$" * | wc -l
34180
$ grep -rP --include=*.[ch] "^\s*(\|\||\&\&)" * | wc -l
7855
net: 3859 to 382 (more than 10:1, so it's the one to follow)
drivers/net: 4610 to 666
Besides, it's the one David wants...
cheers, Joe
^ permalink raw reply
* [PATCH] net: Fix missing kernel-doc notation
From: Jaswinder Singh Rajput @ 2009-11-22 8:19 UTC (permalink / raw)
To: Randy Dunlap, David Miller, Eric Dumazet, netdev, LKML
Fix the following htmldocs warning:
Warning(net/core/dev.c:5378): bad line:
Signed-off-by: Jaswinder Singh Rajput <jaswinderrajput@gmail.com>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: David Miller <davem@davemloft.net>
Cc: Randy Dunlap <randy.dunlap@oracle.com>
---
net/core/dev.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 9977288..bd32d7a 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5375,7 +5375,7 @@ EXPORT_SYMBOL(synchronize_net);
* unregister_netdevice_queue - remove device from the kernel
* @dev: device
* @head: list
-
+ *
* This function shuts down a device interface and removes it
* from the kernel tables.
* If head not NULL, device is queued to be unregistered later.
--
1.6.5.3
^ permalink raw reply related
* [PATCH] mac80211: Fix missing kernel-doc notation
From: Jaswinder Singh Rajput @ 2009-11-22 8:17 UTC (permalink / raw)
To: David Miller, Randy Dunlap, John W. Linville, Johannes Berg,
linux-wireless
Fix the following htmldocs warnings:
Warning(net/mac80211/sta_info.h:322): No description found for parameter 'drv_unblock_wk'
Warning(net/mac80211/sta_info.h:322): No description found for parameter 'drv_unblock_wk'
Signed-off-by: Jaswinder Singh Rajput <jaswinderrajput@gmail.com>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: John W. Linville <linville@tuxdriver.com>
Cc: Randy Dunlap <randy.dunlap@oracle.com>
Cc: David Miller <davem@davemloft.net>
---
net/mac80211/sta_info.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index 4c84c22..b4810f6 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -185,6 +185,7 @@ struct sta_ampdu_mlme {
* @lock: used for locking all fields that require locking, see comments
* in the header file.
* @flaglock: spinlock for flags accesses
+ * @drv_unblock_wk: used for driver PS unblocking
* @listen_interval: listen interval of this station, when we're acting as AP
* @pin_status: used internally for pinning a STA struct into memory
* @flags: STA flags, see &enum ieee80211_sta_info_flags
@@ -225,7 +226,6 @@ struct sta_ampdu_mlme {
* @debugfs: debug filesystem info
* @sta: station information we share with the driver
* @dead: set to true when sta is unlinked
- * @drv_unblock_wk used for driver PS unblocking
*/
struct sta_info {
/* General information, mostly static */
--
1.6.5.3
^ permalink raw reply related
* Re: [PATCHv9 3/3] vhost_net: a kernel-level virtio server
From: Michael S. Tsirkin @ 2009-11-22 10:35 UTC (permalink / raw)
To: Xin, Xiaohui
Cc: netdev@vger.kernel.org, virtualization@lists.linux-foundation.org,
kvm@vger.kernel.org, linux-kernel@vger.kernel.org, mingo@elte.hu,
linux-mm@kvack.org, akpm@linux-foundation.org, hpa@zytor.com,
gregory.haskins@gmail.com, Rusty Russell, s.hetze@linux-ag.com,
Daniel Walker, Eric Dumazet, mashirle
In-Reply-To: <C85CEDA13AB1CF4D9D597824A86D2B901925446AB4@PDSMSX501.ccr.corp.intel.com>
On Wed, Nov 18, 2009 at 01:42:43PM +0800, Xin, Xiaohui wrote:
> Michael,
> >From the http://www.linux-kvm.org/page/VhostNet, we can see netperf with TCP_STREAM can get more than 4GMb/s for the receive side, and more than 5GMb/s for the send side.
> Is it the result from the raw socket or through tap?
> I want to duplicate such performance with vhost on my side. I can only get more than 1GMb/s with following conditions:
> 1) disabled the GRO feature in the host 10G NIC driver
> 2) vi->big_packet in guest is false
> 3) MTU is 1500.
> 4) raw socket, not the tap
> 5) using your vhost git tree
>
> Is that the reasonable result with such conditions or maybe I have made some silly mistakes somewhere I don't know yet?
> May you kindly describe your test environment/conditions in detail to have much better performance in your website (I really need the performance)?
>
> Thanks
> Xiaohui
These results where sent by Shirley Ma (Cc'd).
I think they were with tap, host-to-guest/guest-to-host
> And I have tested the tun support with vhost now, and may you share your /home/mst/ifup script here?
>
These are usually pretty simple, mine looks like this:
#!/bin/sh -x
/sbin/ifconfig tap0 0.0.0.0 up
brctl addif br0 tap0
--
MST
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* [2.6.32-rc8] set_all_monitor_traces() sleeping bug.
From: Tetsuo Handa @ 2009-11-22 13:49 UTC (permalink / raw)
To: nhorman, davem; +Cc: netdev, linux-kernel
In-Reply-To: <200911022100.EAC21893.OMFFJVOSOQHLtF@I-love.SAKURA.ne.jp>
Commit: 4ea7e38696c7e798c47ebbecadfd392f23f814f9
set_all_monitor_traces() calls tracepoint_synchronize_unregister()
with a spinlock held, but tracepoint_synchronize_unregister() may sleep.
static int set_all_monitor_traces(int state)
{
int rc = 0;
struct dm_hw_stat_delta *new_stat = NULL;
struct dm_hw_stat_delta *temp;
spin_lock(&trace_state_lock);
switch (state) {
case TRACE_ON:
rc |= register_trace_kfree_skb(trace_kfree_skb_hit);
rc |= register_trace_napi_poll(trace_napi_poll_hit);
break;
case TRACE_OFF:
rc |= unregister_trace_kfree_skb(trace_kfree_skb_hit);
rc |= unregister_trace_napi_poll(trace_napi_poll_hit);
tracepoint_synchronize_unregister();
/*
* Clean the device list
*/
list_for_each_entry_safe(new_stat, temp, &hw_stats_list, list) {
if (new_stat->dev == NULL) {
list_del_rcu(&new_stat->list);
call_rcu(&new_stat->rcu, free_dm_hw_stat);
}
}
break;
default:
rc = 1;
break;
}
if (!rc)
trace_state = state;
spin_unlock(&trace_state_lock);
if (rc)
return -EINPROGRESS;
return rc;
}
^ permalink raw reply
* Re: large packet loss take2 2.6.31.x
From: Caleb Cushing @ 2009-11-22 19:35 UTC (permalink / raw)
To: Jarek Poplawski; +Cc: Frans Pop, Andi Kleen, linux-kernel, netdev
In-Reply-To: <20091118223851.GA5191@ami.dom.local>
haven't had time to do a test yet. but would it be of any use for you
all for me to throw another nic (it'd be a different driver for sure)
in this box and test that on a problematic kernel? I have some but not
with me.
On Wed, Nov 18, 2009 at 5:38 PM, Jarek Poplawski <jarkao2@gmail.com> wrote:
> On Wed, Nov 18, 2009 at 09:10:34PM +0100, Jarek Poplawski wrote:
>> On Wed, Nov 18, 2009 at 01:21:19PM -0500, Caleb Cushing wrote:
>> > yeah testing it under my known working config first. I'll get back w/ you later.
>>
>> Btw, since dropping at hardware (NIC) level seems more likely to me,
>> could you send 'ethtool eth0', and 'ethtool -S eth0' after such tests
>> (both sides).
>
> Hmm... and 'netstat -s' before and after the test (both sides).
>
> Jarek P.
>
--
Caleb Cushing
http://xenoterracide.blogspot.com
^ permalink raw reply
* Re: [olug] Ping Is Broken
From: Rob Townley @ 2009-11-22 20:10 UTC (permalink / raw)
To: olug4mgm; +Cc: Omaha Linux User Group, Brian Haley, netdev, jarkao2
In-Reply-To: <Pine.LNX.4.64.0910121934360.26068@pakmon.pakint.net>
On Tue, Oct 13, 2009 at 12:50 AM, Matthew G. Marsh
<olug4mgm@paktronix.com> wrote:
>
> </delurk>
>
> On Mon, 12 Oct 2009, Rob Townley wrote:
>
>> On Mon, Oct 12, 2009 at 3:09 AM, Aric Aasgaard <aric@omahax.com> wrote:
>>>
>>> The interface isn't on the network your trying to ping.
>>
>> Maybe i don't understand what you are saying, but eth0's ip address is
>> 4.3.2.8 and when 4.3.2.8 is passed as the interface, it works. When
>> eth0 is passed as the interface name, ping fails. So it isn't a
>> problem with the network or routing rules or routing tables, it is a
>> problem in being able to address a device alias.
>
> Actually - not completely. The main problem is that when you call ping with
> the -I eth0 then you also are invoking your rule 32762 because when the ping
> command goes to use the interface it then runs through the rule set and as
> 32762 is the lowest rule it is invoked and your ping packet goes out through
> eth0 to the default gateway of table nic0:
>
> /sbin/ip route show table nic0
> default via 4.3.2.1 dev eth0
>
> And I suspect that cannot get to the device you want to ping.
>
> On the other hand when you specify -I 4.x.x.x to the ping command then the
> packet goes out from lo0 using that source address and is routed by the
> default rule "from all lookup main" which means it gets routed out the eth1
> interface but with the 4.x address.
>
> Do you see the difference?
>
> What gives this away is the following:
>
> 1: The ping utility is from iputils (Alexey's utility set) and thus behaves
> according to the policy structure in the kernel which _does_ differentiate
> between IP address assignments and mere network interfaces. Remember - the
> IP address DOES NOT belong to an interface and does not even need to be
> bound to such in order to function in a Linux kernel.
>
> 2: When you said "All exhibit the issue that ethX can't be used as an
> interface name when the default gateway isn't reached thru ethX."
>
> That told me that you were slightly confusing the way in which -I works in
> Alexey's ping utility. It is confusing considering that you must understand
> the difference between interface sourcing and ip address sourcing.
>
> In summary -
>
> -I <interface> will use the primary IP address of the named interface AND
> act as though it had been sourced from that interface which triggers any
> relevant rules, routes, etal.
>
> -I <IP addr> merely sets the Source address of the packet but then sources
> the packet from lo0 and this would invoke the default routing
>
> Sense?
>
> Let me know. Thanks!!
>
> P.S. I am considering starting work on a second edition as I now am back
> working in the IT field...
>
> mgm
>
>>> How are you attempting to get from one NIC to the other? Not via the same
>>> switch I hope.
>>
>> Both of these nics in all examples are in the same machine. Some have
>> ip_forward = 1 and some don't. All exhibit the issue that ethX can't
>> be used as an interface name when the default gateway isn't reached
>> thru ethX. Even machines with two nics behind the same switch and
>> therefore have the same default gateway on each interface exhibit a
>> problem referencing the interface by ethX -- however by IP address
>> works just fine. So some are behind the same switch and some use
>> totally different paths to the internet - either way it is broken.
>>
>>>
>>> May I ask what you are trying to do? There might be an easier way.
>>
>> i have a bunch of machines with 2 or more nics (HP and Dell Server
>> Class) that i will use for many types of projects. The easiest way is
>> for ping and tracert to be fixed. it is the first tool in network
>> assurance and debugging and it is very broken. i suspect this would
>> cause problems for users of VPNs, wireless, and other multi nic
>> scenarios.
>>
>>>
>>> As far a multi NIC BSD systems, yeah I got a few pfsense boxes, it is
>>> BSD.
>>> I started using pfsense when I couldn't get multi WAN Linux to work.
>>
>> i wish i would have known to give up 2-3 years ago - i knew i wasn't
>> that stupid as i have only been using ping for 20 years. i also had
>> problems with ping on dd-wrt and as this busybox mailing list shows, i
>> am not the only one. Note, these busybox guys didn't test this too
>> well because their examples never ping an ip on the other side of a
>> wire, but it is the exact same problem.
>> http://lists.busybox.net/pipermail/busybox/2009-July/069972.html
>>
>>>
>>>
>>>
>>> -----Original Message-----
>>> From: olug-bounces@olug.org [mailto:olug-bounces@olug.org] On Behalf Of
>>> Rob
>>> Townley
>>> Sent: Sunday, October 11, 2009 12:32 PM
>>> To: Omaha Linux User Group
>>> Subject: Re: [olug] Ping Is Broken
>>>
>>> On Sat, Oct 10, 2009 at 8:17 PM, Aric Aasgaard <aric@omahax.com> wrote:
>>>>
>>>> Does
>>>> traceroute -i eth0 -I 208.67.222.222
>>>> using ICMP
>>>
>>> eth0 = 4.3.2.8, the default gateway is thru eth1.
>>> tracert -i eth0 -I 208.67.222.222 FAILS
>>> tracert -s 4.3.2.8 -I 208.67.222.222 WORKS
>>> tracert -i eth0 208.67.222.222 FAILS
>>> tracert -s 4.3.2.8 208.67.222.222 WORKS
>>>
>>> There is a still a great deal of heated argument on the netdev list on
>>> how to make network device naming better. Some want to name
>>> interfaces in the order of MAC address, others in the name of bus
>>> enumeration, some think symbolic links will make things more
>>> complicated, others want it. Please test using the following script.
>>> Anybody have a multinic BSD system up that would care to test this.
>>> OSX?
>>>
>>> #!/bin/bash
>>> #The following is a script to test source routing thru the network
>>> interface alias vs ip address.
>>> #Put in your own IP addresses here.
>>> ip0=4.3.2.8
>>> ip1=192.168.168.155
>>> for interface in eth0 $ip0 eth1 $ip1; do echo testing $interface; ping
>>> -I $interface 208.67.222.222; done;
>>>
>>>>
>>>> or using UDP
>>>> traceroute -i eth0 208.67.222.222
>>>>
>>>> Work?
>>>>
>>>> -----Original Message-----
>>>> From: olug-bounces@olug.org [mailto:olug-bounces@olug.org] On Behalf Of
>>>
>>> Rob
>>>>
>>>> Townley
>>>> Sent: Friday, October 09, 2009 11:45 AM
>>>> To: netdev@vger.kernel.org
>>>> Cc: CentOS mailing list; Omaha Linux User Group
>>>> Subject: Re: [olug] Ping Is Broken
>>>>
>>>> ping -I is broken
>>>>
>>>> The following deals with bug in ping that made it very difficult to set
>>>> up
>>>
>>> a
>>>>
>>>> system with two gateways.
>>>>
>>>> Demonstration that *ping -I is broken*. When specifying the source
>>>> interface using -I with an *ethX* alias and that interface is not the
>>>> default gateway
>>>> interface, then ping fails. When specifying the interface as an ip
>>>
>>> address,
>>>>
>>>> ping works. Search for "Destination Host Unreachable" to find the bug.
>>>>
>>>>
>>>> eth*0* = 4.3.2.8 and the default gateway is accessed through a different
>>>> interface eth*1*.
>>>> eth*1* = 192.168.168.155 is used as the device to get to the default
>>>> gateway.
>>>> *FAILS *: ping *-I eth0* 208.67.222.222
>>>> *WORKS*: ping *-I 4.3.2.8* 208.67.222.222
>>>> *WORKS*: ping *-I eth1* 208.67.222.222
>>>> *WORKS*: ping *-I 192.168.168.155* 208.67.222.222
>>>>
>>>> The following are actual results which can be reproduced from an
>>>
>>> up-to-date
>>>>
>>>> Fedora 11 or CentOS 5.3 box. Caused a very very long episode of
>>>
>>> frustration
>>>>
>>>> when setting up multi gatewayed systems.
>>>>
>>>>
>>>> * ping using eth0 *:
>>>>
>>>> ping -c 2 -B -I eth0 208.67.222.222
>>>> PING 208.67.222.222 (208.67.222.222) from 4.3.2.8 eth0: 56(84) bytes of
>>>> data.
>>>> From 4.3.2.8 icmp_seq=1 Destination Host Unreachable
>>>> From 4.3.2.8 icmp_seq=2 Destination Host Unreachable
>>>>
>>>> --- 208.67.222.222 ping statistics ---
>>>> 2 packets transmitted, 0 received, +2 errors, 100% packet loss, time
>>>> 999ms
>>>> , pipe 2
>>>>
>>>> --------------------------------------
>>>> The Following all WORK:
>>>> * ping using 4.3.2.8 *:
>>>>
>>>> ping -c 2 -B -I 4.3.2.8 208.67.222.222
>>>> PING 208.67.222.222 (208.67.222.222) from 4.3.2.8 : 56(84) bytes of
>>>> data.
>>>> 64 bytes from 208.67.222.222: icmp_seq=1 ttl=55 time=562 ms
>>>> 64 bytes from 208.67.222.222: icmp_seq=2 ttl=55 time=642 ms
>>>>
>>>> --- 208.67.222.222 ping statistics ---
>>>> 2 packets transmitted, 2 received, 0% packet loss, time 999ms
>>>> rtt min/avg/max/mdev = 562.546/602.400/642.255/39.862 ms
>>>>
>>>>
>>>> * ping using eth1 *:
>>>>
>>>> ping -c 2 -B -I eth1 208.67.222.222
>>>> PING 208.67.222.222 (208.67.222.222) from 192.168.168.155 eth1: 56(84)
>>>> bytes of data.
>>>> 64 bytes from 208.67.222.222: icmp_seq=1 ttl=54 time=270 ms
>>>> 64 bytes from 208.67.222.222: icmp_seq=2 ttl=54 time=629 ms
>>>>
>>>> --- 208.67.222.222 ping statistics ---
>>>> 2 packets transmitted, 2 received, 0% packet loss, time 1000ms
>>>> rtt min/avg/max/mdev = 270.128/449.766/629.405/179.639 ms
>>>>
>>>>
>>>> * ping using 192.168.168.155 *:
>>>>
>>>> ping -c 2 -B -I 192.168.168.155 208.67.222.222
>>>> PING 208.67.222.222 (208.67.222.222) from 192.168.168.155 : 56(84)
>>>> bytes of data.
>>>> 64 bytes from 208.67.222.222: icmp_seq=1 ttl=54 time=585 ms
>>>> 64 bytes from 208.67.222.222: icmp_seq=2 ttl=54 time=554 ms
>>>>
>>>> --- 208.67.222.222 ping statistics ---
>>>> 2 packets transmitted, 2 received, 0% packet loss, time 999ms
>>>> rtt min/avg/max/mdev = 554.098/569.655/585.212/15.557 ms
>>>>
>>>> My source route policy rules:
>>>>
>>>> /sbin/ip rule show
>>>> 0: from all lookup 255
>>>> 32762: from 4.3.2.8 lookup nic0
>>>> 32763: from 192.168.168.155 lookup nic1
>>>> 32764: from 192.168.168.155 lookup nic1
>>>> 32765: from 4.3.2.8 lookup nic0
>>>> 32766: from all lookup main
>>>> 32767: from all lookup default
>>>>
>>>>
>>>>
>>>> Print out routing tables using /sbin/ip route show table TABLENAME:
>>>> routing table nic0 :
>>>> /sbin/ip route show table nic0
>>>> default via 4.3.2.1 dev eth0
>>>>
>>>> routing table nic1 :
>>>> /sbin/ip route show table nic1
>>>> default via 192.168.168.1 dev eth1
>>>>
>>>> routing table main :
>>>> /sbin/ip route show table main
>>>> 4.3.2.1/27 dev eth0 proto kernel scope link src 4.3.2.8
>>>> 192.168.168.0/24 dev eth1 proto kernel scope link src 192.168.168.155
>>>> 169.254.0.0/16 dev eth1 scope link
>>>> default via 192.168.168.1 dev eth1
>>>>
>>>> routing table default :
>>>> /sbin/ip route show table default
>>>>
>>>>
>>>>
>>>>
>>>> NOTES: cat /etc/iproute2/rt_tables to get your own table names.
>>>>
>>>> ping Maintainer YOSHIFUJI Hideaki / USAGI/WIDE Project
>>>> http://www.skbuff.net/iputils/
>>>> Mailing List netdev@vger.kernel.org
>>>>
>>>> man ping:
>>>> -I interface address
>>>> Set source address to specified interface address.
>>>> Argument may be *numeric IP address or name of device*.
>>>> When pinging IPv6 link-local address this option is
>>>> required.
>>>>
>>>> ping -V returns the latest available on CentOS and Fedora and the
>>>> maintainers website:
>>>> ping utility, iputils-ss020927
>
> --------------------------------------------------
> Matthew G. Marsh
> Special Email Addr for OLUG ;-}
> Phone: (402) 932-7250
> Email: olug4mgm@paktronix.com
> WWW: http://www.paksecured.org
> --------------------------------------------------
Matt,
without iproute2, ping -I ethX works.
leave it to a lawyer to explain away ping failing! (just kidding) it
probably does take legalese, but write that new book soon.
i never forgot your email. i attempted to add additional routes based
on your explanation. It was so long ago that i do not remember the
exact details of the attempts, but there was no success. i was not
sure if you were thinking it would never work as is or that simply
additional routes needed to be added? Regardless, here are some
interesting results that i found using the slax live cd without
iproute installed which demonstrate ping -I ethX works not resulting
in an error. It is not a true test because all interfaces in this vm
are using the same gateway. Have you written your new book yet?
Downloaded the slax livecd iso from slax.org which appears to not have
iproute installed at all, but specifying different ethX interfaces
works fine. There is a iproute module that can be downloaded and
installed, but it isn't. The same VirtualBox config running Fedora 12
Live, ping fails when specifying anything but the default gateway
ethX.
Installed on a VirtualBox vm with 4 virtual bridged network cards.
Linux slax 2.6.27.27 #1 SMP Wed Jul 22 07:27:34 AKDT 2009 i686
Intel(R) Pentium(R) 4 CPU 3.20GHz GenuineIntel GNU/Linux
Linux version 2.6.27.27 (root@darkstar) (gcc version 4.2.4) #1 SMP Wed
Jul 22 07:27:34 AKDT 2009
for ((i=0; i < 4; i++)); do ping -c 1 -I eth${i} 208.67.222.222; done;
PING 208.67.222.222 (208.67.222.222) from 192.168.168.219 eth0: 56(84)
bytes of data.
64 bytes from 208.67.222.222: icmp_seq=1 ttl=49 time=1098 ms
--- 208.67.222.222 ping statistics ---
1 packets transmitted, 1 received, 0% packet loss, time 0ms
rtt min/avg/max/mdev = 1098.078/1098.078/1098.078/0.000 ms
PING 208.67.222.222 (208.67.222.222) from 192.168.168.231 eth1: 56(84)
bytes of data.
64 bytes from 208.67.222.222: icmp_seq=1 ttl=49 time=156 ms
--- 208.67.222.222 ping statistics ---
1 packets transmitted, 1 received, 0% packet loss, time 0ms
rtt min/avg/max/mdev = 156.386/156.386/156.386/0.000 ms
PING 208.67.222.222 (208.67.222.222) from 192.168.168.157 eth2: 56(84)
bytes of data.
64 bytes from 208.67.222.222: icmp_seq=1 ttl=49 time=143 ms
--- 208.67.222.222 ping statistics ---
1 packets transmitted, 1 received, 0% packet loss, time 0ms
rtt min/avg/max/mdev = 143.279/143.279/143.279/0.000 ms
PING 208.67.222.222 (208.67.222.222) from 192.168.168.229 eth3: 56(84)
bytes of data.
64 bytes from 208.67.222.222: icmp_seq=1 ttl=49 time=128 ms
--- 208.67.222.222 ping statistics ---
1 packets transmitted, 1 received, 0% packet loss, time 0ms
rtt min/avg/max/mdev = 128.547/128.547/128.547/0.000 ms
^ permalink raw reply
* Re: large packet loss take2 2.6.31.x
From: Jarek Poplawski @ 2009-11-22 20:50 UTC (permalink / raw)
To: Caleb Cushing; +Cc: Frans Pop, Andi Kleen, linux-kernel, netdev
In-Reply-To: <81bfc67a0911221135y300841f2i8e78417600528cd5@mail.gmail.com>
On Sun, Nov 22, 2009 at 02:35:10PM -0500, Caleb Cushing wrote:
> haven't had time to do a test yet. but would it be of any use for you
> all for me to throw another nic (it'd be a different driver for sure)
> in this box and test that on a problematic kernel? I have some but not
> with me.
Of course it would be useful. Especially if you find new bugs. ;-)
I'm not sure it's the fastest way to diagnose this problem, but if
it's not a problem for you...
Btw, currently I don't consider this dropping means there has to be
a bug. It could be otherwise - a feature... e.g. when a new kernel
can transmit faster (then dropping in some other, slower place can
happen).
Jarek P.
>
> On Wed, Nov 18, 2009 at 5:38 PM, Jarek Poplawski <jarkao2@gmail.com> wrote:
> > On Wed, Nov 18, 2009 at 09:10:34PM +0100, Jarek Poplawski wrote:
> >> On Wed, Nov 18, 2009 at 01:21:19PM -0500, Caleb Cushing wrote:
> >> > yeah testing it under my known working config first. I'll get back w/ you later.
> >>
> >> Btw, since dropping at hardware (NIC) level seems more likely to me,
> >> could you send 'ethtool eth0', and 'ethtool -S eth0' after such tests
> >> (both sides).
> >
> > Hmm... and 'netstat -s' before and after the test (both sides).
> >
> > Jarek P.
> >
>
>
>
> --
> Caleb Cushing
>
> http://xenoterracide.blogspot.com
^ permalink raw reply
* Re: [net-next-2.6 PATCH v7 5/7 RFC] TCPCT part 1e: implement socket option TCP_COOKIE_TRANSACTIONS
From: David Miller @ 2009-11-23 0:31 UTC (permalink / raw)
To: william.allen.simpson; +Cc: netdev
In-Reply-To: <4B08D967.80003@gmail.com>
From: William Allen Simpson <william.allen.simpson@gmail.com>
Date: Sun, 22 Nov 2009 01:25:43 -0500
> Therefore, David, would indentation of 2 or 4 be preferable here?
It should be of the form:
if (x &&
y)
or:
if (x && y)
Fix patches, rather than complaints, for existing cases where things
do not follow this pattern are cetainly welcome.
^ permalink raw reply
* Re: [PATCH 0/1] Defer skb allocation for both mergeable buffers and big packets in virtio_net
From: Rusty Russell @ 2009-11-23 0:53 UTC (permalink / raw)
To: Shirley Ma
Cc: Michael S. Tsirkin, Avi Kivity, netdev, kvm, linux-kernel,
Anthony Liguori
In-Reply-To: <1258697359.7416.14.camel@localhost.localdomain>
On Fri, 20 Nov 2009 04:39:19 pm Shirley Ma wrote:
> Guest virtio_net receives packets from its pre-allocated vring
> buffers, then it delivers these packets to upper layer protocols
> as skb buffs. So it's not necessary to pre-allocate skb for each
> mergable buffer, then frees it when it's useless.
>
> This patch has deferred skb allocation when receiving packets for
> both big packets and mergeable buffers. It reduces skb pre-allocations
> and skb_frees.
>
> Based on Mickael & Avi's suggestion. A destroy function has been created
> to push virtio free buffs to vring for unused pages, and used page private
> to maintain page list.
>
> I didn't touch small packet skb allocation to avoid extra copies for small
> packets.
>
> This patch has tested and measured against 2.6.32-rc5 git. It is built again
> 2.6.32-rc7 kernel. Tests have been done for small packets, big packets and
> mergeable buffers.
>
> The single netperf TCP_STREAM performance improved for host to guest.
> It also reduces UDP packets drop rate.
>
> The netperf laptop results were:
>
> mtu=1500
> netperf -H xxx -l 120
>
> w/o patch w/i patch (two runs)
> guest to host: 3336.84Mb/s 3730.14Mb/s ~ 3582.88Mb/s
>
> host to guest: 3165.10Mb/s 3370.39Mb/s ~ 3407.96Mb/s
Nice!
Is this using mergeable_rx_bufs? Or just big_packets?
I'd like to drop big packet support from our driver, but I don't know
how many kvm hosts will not offer mergable rx bufs yet. Anthony?
Thanks,
Rusty.
^ permalink raw reply
* linux-next: manual merge of the net tree with the net-current tree
From: Stephen Rothwell @ 2009-11-23 1:00 UTC (permalink / raw)
To: David Miller, netdev; +Cc: linux-next, linux-kernel, Amit Kumar Salecha
Hi all,
Today's linux-next merge of the net tree got a conflict in
drivers/net/netxen/netxen_nic_init.c between commit
5d521fd36de4e61ab16a358df54c5babe153396c ("netxen: fix memory
initialization") from the net-current tree and commit
0be367bd5d10634c0836f57a684432fee935d929 ("netxen: reset sequence
changes") from the net tree.
Just context changes. I fixed it up (see below) and can carry the fix as
necessary.
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
diff --cc drivers/net/netxen/netxen_nic_init.c
index 8a09043,6ee27a6..0000000
--- a/drivers/net/netxen/netxen_nic_init.c
+++ b/drivers/net/netxen/netxen_nic_init.c
@@@ -544,9 -528,8 +528,10 @@@ int netxen_pinit_from_rom(struct netxen
continue;
if (off == (ROMUSB_GLB + 0x1c)) /* MS clock */
continue;
+ if ((off & 0x0ff00000) == NETXEN_CRB_DDR_NET)
+ continue;
- if (off == (NETXEN_CRB_PEG_NET_1 + 0x18))
+ if (off == (NETXEN_CRB_PEG_NET_1 + 0x18) &&
+ !NX_IS_REVISION_P3P(adapter->ahw.revision_id))
buf[i].data = 0x1020;
/* skip the function enable register */
if (off == NETXEN_PCIE_REG(PCIE_SETUP_FUNCTION))
^ permalink raw reply
* linux-next: manual merge of the net tree with the net-current tree
From: Stephen Rothwell @ 2009-11-23 1:00 UTC (permalink / raw)
To: David Miller, netdev; +Cc: linux-next, linux-kernel, Eric Dumazet
[-- Attachment #1: Type: text/plain, Size: 541 bytes --]
Hi all,
Today's linux-next merge of the net tree got a conflict in
drivers/ieee802154/fakehard.c between commit
56cf54831fd1be105e89a9df899e3b22442da180 ("ieee802154: dont leak skbs in
ieee802154_fake_xmit()") from the net-current tree and commit
8964be4a9a5ca8cab1219bb046db2f6d1936227c ("net: rename skb->iif to
skb->skb_iif") from the net tree.
The former removed the code modified by the latter, so I used the former.
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/
[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: [PATCH 1/1] Defer skb allocation for both mergeable buffers and big packets in virtio_net
From: Rusty Russell @ 2009-11-23 1:08 UTC (permalink / raw)
To: Shirley Ma
Cc: Eric Dumazet, Michael S. Tsirkin, Avi Kivity, netdev, kvm,
linux-kernel
In-Reply-To: <1258734101.11049.1.camel@localhost.localdomain>
On Sat, 21 Nov 2009 02:51:41 am Shirley Ma wrote:
> Signed-off-by: Shirley Ma (xma@us.ibm.com)
Hi Shirley,
This patch seems like a good idea, but it needs some work. As this is
the first time I've received a patch from you, I will go through it in extra
detail. (As virtio maintainer, it's my responsibility to include this patch,
or not).
> -static void give_a_page(struct virtnet_info *vi, struct page *page)
> +static void give_pages(struct virtnet_info *vi, struct page *page)
> {
> - page->private = (unsigned long)vi->pages;
> + struct page *npage = (struct page *)page->private;
> +
> + if (!npage)
> + page->private = (unsigned long)vi->pages;
> + else {
> + /* give a page list */
> + while (npage) {
> + if (npage->private == (unsigned long)0) {
> + npage->private = (unsigned long)vi->pages;
> + break;
> + }
> + npage = (struct page *)npage->private;
> + }
> + }
> vi->pages = page;
The loops should cover both cases of the if branch. Either way, we want
to find the last page in the list so you can set that ->private pointer to
vi->pages.
Also, the "== (unsigned long)0" is a little verbose.
How about:
struct page *end;
/* Find end of list, sew whole thing into vi->pages. */
for (end = page; end->private; end = (struct page *)end->private);
end->private = (unsigned long)vi->pages;
vi->pages = page;
> +void virtio_free_pages(void *buf)
This is a terrible name; it's specific to virtio_net, plus it should be
static. Just "free_pages" should be sufficient here I think.
> +{
> + struct page *page = (struct page *)buf;
> + struct page *npage;
> +
> + while (page) {
> + npage = (struct page *)page->private;
> + __free_pages(page, 0);
> + page = npage;
> + }
This smells a lot like a for loop to me?
struct page *i, *next;
for (i = buf; next; i = next) {
next = (struct page *)i->private;
__free_pages(i, 0);
}
> +static int set_skb_frags(struct sk_buff *skb, struct page *page,
> + int offset, int len)
A better name might be "skb_add_frag()"?
> +static void receive_skb(struct net_device *dev, void *buf,
> unsigned len)
> {
> struct virtnet_info *vi = netdev_priv(dev);
> - struct skb_vnet_hdr *hdr = skb_vnet_hdr(skb);
> - int err;
> + struct skb_vnet_hdr *hdr;
> + struct sk_buff *skb;
> int i;
>
> if (unlikely(len < sizeof(struct virtio_net_hdr) + ETH_HLEN)) {
> @@ -132,39 +173,71 @@ static void receive_skb(struct net_device *dev, struct sk_buff *skb,
> goto drop;
> }
>
> - if (vi->mergeable_rx_bufs) {
> - unsigned int copy;
> - char *p = page_address(skb_shinfo(skb)->frags[0].page);
> + if (!vi->mergeable_rx_bufs && !vi->big_packets) {
> + skb = (struct sk_buff *)buf;
This cast is unnecessary, but a comment would be nice:
/* Simple case: the pointer is the 1514-byte skb */
> + } else {
And a matching comment here:
/* The pointer is a chain of pages. */
> + struct page *page = (struct page *)buf;
> + int copy, hdr_len, num_buf, offset;
> + char *p;
> +
> + p = page_address(page);
>
> - if (len > PAGE_SIZE)
> - len = PAGE_SIZE;
> - len -= sizeof(struct virtio_net_hdr_mrg_rxbuf);
> + skb = netdev_alloc_skb(vi->dev, GOOD_COPY_LEN + NET_IP_ALIGN);
> + if (unlikely(!skb)) {
> + dev->stats.rx_dropped++;
Does this mean we leak all those pages? Shouldn't we give_pages() here?
> + return;
> + }
> + skb_reserve(skb, NET_IP_ALIGN);
> + hdr = skb_vnet_hdr(skb);
>
> - memcpy(&hdr->mhdr, p, sizeof(hdr->mhdr));
> - p += sizeof(hdr->mhdr);
> + if (vi->mergeable_rx_bufs) {
> + hdr_len = sizeof(hdr->mhdr);
> + memcpy(&hdr->mhdr, p, hdr_len);
> + num_buf = hdr->mhdr.num_buffers;
> + offset = hdr_len;
> + if (len > PAGE_SIZE)
> + len = PAGE_SIZE;
> + } else {
> + /* big packtes 6 bytes alignment between virtio_net
> + * header and data */
> + hdr_len = sizeof(hdr->hdr);
> + memcpy(&hdr->hdr, p, hdr_len);
> + offset = hdr_len + 6;
Really? I can't see where the current driver does this 6 byte offset. There
should be a header, then the packet...
Ah, I see below that you set things up this way. The better way to do this
is to create a new structure to use in various places.
struct padded_vnet_hdr {
struct virtio_net_hdr hdr;
/* This padding makes our packet 16 byte aligned */
char padding[6];
};
However, I question whether making it 16 byte is the right thing: the
ethernet header is 14 bytes long, so don't we want 8 bytes of padding?
> + }
I think you can move the memcpy outside the if, like so:
memcpy(&hdr, p, hdr_len);
> + if (!len)
> + give_pages(vi, page);
> + else {
> + len = set_skb_frags(skb, page, copy + offset, len);
> + /* process big packets */
> + while (len > 0) {
> + page = (struct page *)page->private;
> + if (!page)
> + break;
> + len = set_skb_frags(skb, page, 0, len);
> + }
> + if (page && page->private)
> + give_pages(vi, (struct page *)page->private);
> }
I can't help but think this statement should be one loop somehow. Something
like:
offset += copy;
while (len) {
len = skb_add_frag(skb, page, offset, len);
page = (struct page *)page->private;
offset = 0;
}
if (page)
give_pages(vi, page);
> -static bool try_fill_recv_maxbufs(struct virtnet_info *vi, gfp_t gfp)
> +/* Returns false if we couldn't fill entirely (OOM). */
> +static bool try_fill_recv(struct virtnet_info *vi, gfp_t gfp)
The result of trying to merge all the cases here is messy. I'd split it
inside the loop into: add_recvbuf_small(vi, gfp), add_recvbuf_big(vi, gfp)
and add_recvbuf_mergeable(vi, gfp).
It'll also be easier for me to review each case then, as well as getting
rid of the big packets if we decide to do that later. You could even do
that split as a separate patch, before this one.
> @@ -970,11 +1020,17 @@ static void __devexit virtnet_remove(struct virtio_device *vdev)
> }
> __skb_queue_purge(&vi->send);
>
> - BUG_ON(vi->num != 0);
> -
> unregister_netdev(vi->dev);
> cancel_delayed_work_sync(&vi->refill);
>
> + if (vi->mergeable_rx_bufs || vi->big_packets) {
> + freed = vi->rvq->vq_ops->destroy_buf(vi->rvq,
> + virtio_free_pages);
> + vi->num -= freed;
> + }
> +
> + BUG_ON(vi->num != 0);
> +
destroy_buf should really be called destroy_bufs(). And I don't think you
use the vi->recv skb list in this case at all, so the loop which purges those
should be under an "else {" of this branch.
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index fbd2ecd..aec7fe7 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -334,6 +334,29 @@ static bool vring_enable_cb(struct virtqueue *_vq)
> return true;
> }
>
> +static int vring_destroy_buf(struct virtqueue *_vq, void (*callback)(void *))
This new parameter should be introduced as a separate patch. It should also be
called destroy_bufs. It also returns an unsigned int. I would call the callback
something a little more informative, such as "destroy"; here and in the header.
> + struct vring_virtqueue *vq = to_vvq(_vq);
> + void *ret;
> + unsigned int i;
> + int freed = 0;
> +
> + START_USE(vq);
> +
> + for (i = 0; i < vq->vring.num; i++) {
> + if (vq->data[i]) {
> + /* detach_buf clears data, so grab it now. */
> + ret = vq->data[i];
> + detach_buf(vq, i);
> + callback(ret);
ret is a bad name for this; how about buf?
Overall, the patch looks good. But it would have been nicer if it were
split into several parts: cleanups, new infrastructure, then the actual
allocation change.
Thanks!
Rusty.
^ permalink raw reply
* Re: linux-next: manual merge of the net tree with the net-current tree
From: David Miller @ 2009-11-23 1:19 UTC (permalink / raw)
To: sfr; +Cc: netdev, linux-next, linux-kernel, eric.dumazet
In-Reply-To: <20091123120052.dd388203.sfr@canb.auug.org.au>
From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Mon, 23 Nov 2009 12:00:52 +1100
> Today's linux-next merge of the net tree got a conflict in
> drivers/ieee802154/fakehard.c between commit
> 56cf54831fd1be105e89a9df899e3b22442da180 ("ieee802154: dont leak skbs in
> ieee802154_fake_xmit()") from the net-current tree and commit
> 8964be4a9a5ca8cab1219bb046db2f6d1936227c ("net: rename skb->iif to
> skb->skb_iif") from the net tree.
>
> The former removed the code modified by the latter, so I used the former.
Hmmm... I specifically remember taking core of this conflict when I
merged net-next-2.6 into net-2.6 yesterday or the day before.
^ permalink raw reply
* Re: linux-next: manual merge of the net tree with the net-current tree
From: Stephen Rothwell @ 2009-11-23 2:04 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-next, linux-kernel, eric.dumazet
In-Reply-To: <20091122.171932.74107512.davem@davemloft.net>
[-- Attachment #1: Type: text/plain, Size: 892 bytes --]
Hi Dave,
On Sun, 22 Nov 2009 17:19:32 -0800 (PST) David Miller <davem@davemloft.net> wrote:
>
> Hmmm... I specifically remember taking core of this conflict when I
> merged net-next-2.6 into net-2.6 yesterday or the day before.
These are the trees I picked up from master.kernel.org this morning:
$ git branch -r -v | grep '^ *net'
net-current/master 5d09e53 netxen : fix BOND_MODE_TLB/ALB mode.
net/master e994b7c tcp: Don't make syn cookies initial setting depend on CONFIG_SYSCTL
commit e994b7c901ded7200b525a707c6da71f2cf6d4bb
Author: David S. Miller <davem@davemloft.net>
Date: Sat Nov 21 11:22:25 2009 -0800
Did you push them out? master was down for a while yesterday ...
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/
[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: linux-next: manual merge of the net tree with the net-current tree
From: David Miller @ 2009-11-23 2:11 UTC (permalink / raw)
To: sfr; +Cc: netdev, linux-next, linux-kernel, eric.dumazet
In-Reply-To: <20091123130441.b450278d.sfr@canb.auug.org.au>
From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Mon, 23 Nov 2009 13:04:41 +1100
> Did you push them out? master was down for a while yesterday ...
That's what happened, I couldn't push out yesterday because
master wasn't responding. Let me push it out right now :-)
Thanks!
^ permalink raw reply
* Re: [PATCH] RDMA/addr: Use appropriate locking with for_each_netdev()
From: Roland Dreier @ 2009-11-23 4:18 UTC (permalink / raw)
To: Eric Dumazet
Cc: Sean Hefty, David S. Miller, Linux Netdev List, Roland Dreier,
Hal Rosenstock, linux-rdma
In-Reply-To: <4B063C78.1070502@gmail.com>
> Its possible to get infinite loops or crashes.
> I am not a RDMA user, so I personally dont care :)
I think given that it's not a regression, and I've never heard of anyone
hitting it in practice, we can wait until the merge window.
Thanks!
^ permalink raw reply
* Re: [PATCH] Au1x00: fix crash when trying register_netdev()
From: David Miller @ 2009-11-23 4:41 UTC (permalink / raw)
To: a.beregalov; +Cc: netdev, alohre
In-Reply-To: <1258772855-9141-1-git-send-email-a.beregalov@gmail.com>
From: Alexander Beregalov <a.beregalov@gmail.com>
Date: Sat, 21 Nov 2009 06:07:35 +0300
> Andreas Lohre reported that the driver crashes when trying
> to register_netdev(), he sugessted to move dev->netdev_ops initialization
> before calling register_netdev(), it worked for him.
>
> Reported-by: Andreas Lohre <alohre@gmail.com>
> Signed-off-by: Alexander Beregalov <a.beregalov@gmail.com>
Applied.
> David, please send it to stable@ for 2.6.31
Will do, thanks.
^ 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