I'm forwarding this to the pegasus driver developers so they can comment on it. Debian kernel version 2.6.32-9 is closely based on stable kernel 2.6.32.9. -- Ben Hutchings Once a job is fouled up, anything done to improve it makes it worse.

CONGRATULATION YOUR EMAIL HAS WON 950,000 POUNDS --

cleanup patch. Use new __packed annotation in net/ and include/ (except netfilter) Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> --- include/linux/if_ether.h | 2 include/linux/if_fddi.h | 8 +-- include/linux/if_frad.h | 2 include/linux/if_hippi.h | 8 +-- include/linux/if_pppox.h | 8 +-- include/linux/ipv6.h | 4 - include/linux/isdnif.h | 2 include/linux/mISDNif.h | 2 include/linux/nbd.h | 2 ...

Probably no one has used this driver on big-endian systems, since it was setting up descriptor swapping if CONFIG_BIG_ENDIAN is set, which it never is, since that symbol is not mentioned anywhere else in the kernel source. Switch this test to a check for __BIG_ENDIAN so it has a chance at working. Signed-off-by: Roland Dreier <rolandd@cisco.com> --- drivers/net/epic100.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/net/epic100.c ...

Cleanup PHY probing: use helpers from phylib Signed-off-by: Denis Kirjanov <dkirjanov@kernel.org> --- drivers/net/fec.c | 16 +++++----------- 1 files changed, 5 insertions(+), 11 deletions(-) diff --git a/drivers/net/fec.c b/drivers/net/fec.c index edfff92..c107d8e 100644 --- a/drivers/net/fec.c +++ b/drivers/net/fec.c @@ -679,30 +679,24 @@ static int fec_enet_mii_probe(struct net_device *dev) { struct fec_enet_private *fep = netdev_priv(dev); struct phy_device *phy_dev = ...

v2: changed bonding module version, modified to apply on top of changes from previous patch in series, and updated documentation to elaborate on multiqueue awareness that now exists in bonding driver. This patch give the user the ability to control the output slave for round-robin and active-backup bonding. Similar functionality was discussed in the past, but Jay Vosburgh indicated he would rather see a feature like this added to existing modes rather than creating a completely new mode. ...

CONFIG_CHELSIO_T1_COUGAR cannot be set (it appears nowhere in any Kconfig files), and the code it protects could never build (cspi.h was never added to the kernel tree). Therefore it's pretty safe to remove all vestiges of this dead code. Signed-off-by: Roland Dreier <rolandd@cisco.com> --- drivers/net/chelsio/common.h | 1 - drivers/net/chelsio/subr.c | 49 +---------------------------------------- 2 files changed, 2 insertions(+), 48 deletions(-) diff --git ...

commit 5c7fffd0e3b57cb63f50bbd710868f012d67654f ("drivers/net/mac8390.c: Remove useless memcpy casting") removed too many casts, introducing the following warnings: | drivers/net/mac8390.c:248: warning: passing argument 1 of '__builtin_memcpy' makes pointer from integer without a cast | drivers/net/mac8390.c:253: warning: passing argument 1 of 'word_memcpy_tocard' makes pointer from integer without a cast | drivers/net/mac8390.c:255: warning: passing argument 2 of 'word_memcpy_fromcard' makes ...

From: Changli Gao <xiaosuo@gmail.com> The user will find out when he gets warnings in his kernel log messages on platforms where this matters. And, if anything, silently just skipping over things is not acceptable. And imposing a 4-byte alignment could break existing setups that actually work on x86 and powerpc which are platforms that don't have alignment issues. I basically tried to explain to you earlier that I wasn't going to accept patches that try to deal with alignment in any way ...

From: Changli Gao <xiaosuo@gmail.com> Looks good, applied. --

From: Changli Gao <xiaosuo@gmail.com> Please don't do so many things at once Changli. Everything is fine except the get_unaligned() addition. It's intentionally not there, and by adding the get_unaligned() you're going to kill performance as this expands to 4 byte loads per u32 key check on RISC systems. If it's not aligned, that's a path or configuration that will need to be fixed up such that the accesses are aligned. Please respin this patch with the get_unaligned() part ...

While THIS_MODULE is NULL in =y mode, /sys/module/<xyz> can still exist (cf. /sys/module/printk). I just don't know how to get at the kobj for it, but the existence of it must mean it's there somewhere. Might ask sysfs authors. --

Isn't this going to oops when you compile this module as =y? --

Okay, good to know. My initial theory was that /sys/module/xt_IDLETIMER (pointed to by THIS_MODULE) would exist even if it was linked into the kernel itself, but now it's obvious that it doesn't. I'll investigate how printk does that. -- Cheers, Luca. --

On Wed, 2010-06-02 at 20:37 +0200, Coelho Luciano (Nokia-D/Helsinki) Would it be too hacky to force it to be a module (ie. add "depends on m" in Kconfig)? Besides /sys/module/xt_IDLETIMER and /sys/class/net, which we have already discarded, I can't find any other place that would make sense to add the idletimer in the kernel object hierarchy... -- Cheers, Luca. --

Well the netdevs are gone, just the links aren't going away. the yes. Come to think of it, maybe somehow it ends up removing mac80211_hwsim/hwsim0 before wlan0, and thus the link stays around? I guess I could make it print messages about that somehow? johannes --

Yeah though I'm not sure how that would happen? Wouldn't the symlink cause the target kobject to still be referenced, and thus stay around No, not based on find /sys and diffing before/after anyway. johannes --

The references don't affect visibility in sysfs. All of that is manual at the sysfs layer, and there doesn't appear to be a good substitute. Generally the device layer manages to handle all of the details automatically but it It is going to be a little bit before I manage to dig into this deeply. If you want to dig into this look at sysfs_delete_link. instrument it so that you can see if it is called for wlan{0,1,2} and see what ns it is called for. My current hypothesis is something ...

Nah, the unregister_netdev there removes the "hwsim0" device, while the mac80211_hwsim_free() will remove all the others, so the ordering of those two doesn't matter. johannes --

Hmm. I'm also not seeing it with veth, it would seem that ought to be I don't know if that's happening .. just guessing that it might cause such a problem, and maybe some things are deferred somehow? Since netdev destruction can be deferred, but the wifi sysfs destruction isn't. But then that link there should cause the refcount to not go down until the link goes away>? johannes --

No that was the sd, but sd->s_ns is NULL in both cases. johannes --

Ah, so the network devices aren't getting removed? Do you have network namespaces enabled in your kernel or disabled? And this is 2.6.35-rc1, right? Eric, any ideas? thanks, greg k-h --

This patch implements an idletimer Xtables target that can be used to identify when interfaces have been idle for a certain period of time. Timers are identified by labels and are created when a rule is set with a new label. The rules also take a timeout value (in seconds) as an option. If more than one rule uses the same timer label, the timer will be restarted whenever any of the rules get a hit. One entry for each timer is created in sysfs. This attribute contains the timer remaining ...

Hi Jan, Thanks for your prompt review! I'll send v2 with the fixes you suggested. Oops! Fixed in v2. Also added sysfs_remove_file_from_group() if the Yeps, this is a remain from the previous (and ugly) read from ipt_ip. Yes, this is the only reason why I haven't put it under the net class, which would probably look cleaner. In other classes it seems to be common to add misc attributes, but the net class (as of now) only contains interface subclasses, as you ...

This patch adds receiver part to pktgen taking advantages of SMP systems with multiple rx queues: - Creation of new proc file /proc/net/pktgen/pgrx to control and display the receiver. - It uses PER-CPU variable to store the results per each CPU. - Results displayed per CPU and aggregated. - The packet handler is add in the protocols handlers (dev_Add_pack()) - Available statistics: packets and bytes received, work time and rate - Only process pktgen packets - It is possible to select the ...

This patch increases the granularity of the rate generated by pktgen. The previous version of pktgen uses micro seconds (udelay) resolution when it was delayed causing gaps in the rates. It is changed to nanosecond (ndelay). Now any rate is possible. Also it allows to set, the desired rate in Mb/s or packets per second. The documentation has been updated. Signed-off-by: Daniel Turull <daniel.turull@gmail.com> --- diff --git a/Documentation/networking/pktgen.txt ...

Using vmalloc_node(size, numa_node_id()) for temporary storage is not needed. vmalloc(size) is more respectful of user NUMA policy. Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> --- net/ipv4/netfilter/arp_tables.c | 7 +++---- net/ipv4/netfilter/ip_tables.c | 4 ++-- net/ipv6/netfilter/ip6_tables.c | 7 +++---- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c index 1ac01b1..16c0ba0 100644 --- ...

From: "Michael Chan" <mchan@broadcom.com> Applied, thanks! --

From: Anirban Chakraborty <anirban.chakraborty@qlogic.com> Applied. --

Well, I included a comment in the message and left it to your judgement Sorry. Ben. -- Ben Hutchings, Senior Software Engineer, Solarflare Communications Not speaking for my employer; that's the marketing department's job. They asked us to note that Solarflare product names are trademarked. --

From: Ben Hutchings <bhutchings@solarflare.com> ALl applied, thanks Ben. --

For an example, you could take a look at commit 907cdda5205b (tcp: Add SNMP counter for DEFER_ACCEPT) Its pretty straigthforward, and wont conflict with monitoring apps. --

From: Christoph Lameter <cl@linux-foundation.org> Within reason. If you're going to spam the tree with something like 10 or 20 new stat counters getting bumped all over the place, that's not what we're trying to suggest here. Consolidate as much as possible, add new things when absolutely nothing existing fits the bill. --

From: Christoph Lameter <cl@linux-foundation.org> We know, that's why there is a networking tracepoint that allows you to see where all drops occur. :-) --

take a look at http://www.redhat.com/docs/en-US/Red_Hat_Enterprise_Linux/html/SystemTap_Beginners_Gui... --

Where can I find out more about the network tracepoint? --

System tap? Oh no. Also skbs may be freed for legitimate reasons. The point is that the loss detection needs to be usable for a regular mortal. With the counters in /proc/net/snmp you have something that is easy to handle. The approach with systemtap will need lots of work to both get the tracing environment setup (local competence in systemtap) and participation by a developer to figure out what the output means. Then there is also the additional code overhead that you do not want by default ...

I just dont follow you. Your patch has nothing to do with dropped packets because of rp_filter. You inserted a counter increment in a path that is not taken at all by packet delivery. Maybe I missed something really obvious with your patch ? It should only matters for the admin doing following command : ip route get 1.2.3.4 from 192.168.0.1 iif eth0 That is a probe, not a 'packet delivery', this is why I said Have you considered CONFIG_NET_DROP_MONITOR ? This one catches all ...

That stap script was really meant to be a stopgap measure. As mentioned, you can use the debugfs interface to turn tracepoints on and use them anyway you wish. Or, if you want to use the kfree_skb and napi_poll tracepoints in a more formalized way, you can use the dropwatch user space utility: https://fedorahosted.org/dropwatch/ Which includes a man page on usage :) I also recently updated it so that this utility can query /proc/kallsyms to translate program counter values into symbollic ...

rp_filter was setup in the past and it worked. Stephen fixed it in 2.6.31 for multicast and thus suddenly multicast stopped working on secondary interfaces when we moved to 2.6.32. rp_filter having to be off is okay but Looks good which does not say too much given my limited networking knowledge. Reviewed-by: Christoph Lameter <cl@linux-foundation.org> --

The rp_filter is rejecting traffic coming into a NIC for which the kernel has a multicast join list that indicates that this traffic is expected on this NIC. You could consult the MC subscription list to verify that the traffic is coming into the right NIC. In the MC case the user can explicitly specify through which NIC the traffic is expected. See IP_ADD_MEMBERSHIP. --

This has litle to do with MC. We certainly are not going to check MC membership in fib_validate_source() ! Say we have eth0 on 192.168.0.1/24 and eth1 on 192.168.0.2/24 Then we cannot use rp_filter = 1, even with unicast trafic. I really dont understand why you would setup rp_filter in such a situation. This wont work. Now, I agree we should have a counter somewhere to help admins to understand their error ;) Here is patch I am currently testing. I finaly created a new counter, ...

From: Scott Feldman <scofeldm@cisco.com> Applied, thanks Scott. --

From: arno@natisbad.org (Arnaud Ebalard) Looks good, applied to net-next-2.6, thanks. --

From: Stephen Hemminger <shemminger@vyatta.com> Applied. --

From: Dan Carpenter <error27@gmail.com> Applied. --

From: Richard Cochran <richardcochran@gmail.com> Wasn't there another change you were asked to make that got included in the v2 patch? --

In any case, the PHY only supports 100 Mbps when in fiber mode, so the fix is always safe to use. Richard --

From: Richard Cochran <richardcochran@gmail.com> No, I think using dev_flags is absolutely the wrong way to do about this. phy_device->priv "could one day get used for a different purpose"? What in the world are you smoking Andy? It's clearly a private state pointer for the PHY driver to use, full stop. There is absolutely no ambiguity of what this value is and what it is used for and who owns it. The comments in the layout of struct phy_device state this clearly as well. On the other ...

From: jamal <hadi@cyberus.ca> Well for one thing there is all sorts of "stuff" past the end of the valid skb->data area. For example, there is some padding and then there is skb_shared_info(). Furthermore, the kernel allocator can round up the size it uses for SLAB objects which gives even more padding past the end of even skb_shared_info(). Futrhermore, the chance of the page past the page skb->data is in being invalid is very low. You'd have to have invalid memory in the page after ...

Hi Changli, Ok, makes more sense. And thanks for taking time to construct a meaningful example. It is not a common use - but i agree it is a bug. I am suprised we never caught this all this years and wondering why this never crashed in your example? Can we make the fix very simple please? i.e no copy bits, this is the What i meant was if you can tell immediately what the maximum offset is then you dont need to go through for loop making comparison with each key. You could immediately ...

Example, something along lines of: --- diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c index 9627542..dde7a23 100644 --- a/net/sched/cls_u32.c +++ b/net/sched/cls_u32.c @@ -135,6 +135,9 @@ next_knode: for (i = n->sel.nkeys; i>0; i--, key++) { + int toff = key->off+(off2&key->offmask)- 4; + if (unlikely(toff > skb->len)) + /* bailout here - needs some thought */ if ((*(__be32*)(ptr+key->off+(off2&key->offmask))^key->v n = ...

I got it. But it isn't easy for me. -- Regards, Changli Gao(xiaosuo@gmail.com) --

A little worse than skb_copy_bits(). In any case, this change is going to hurt. Dave, can we assume the upper layers(qdiscs in this case) are responsible for any linearizing? Changli, if you have time - can you also audit tcf_pedit() since it follows TheLinuxWay(tm). cheers, jamal --

Thats the part i glossed over - My eyes just saw "it calls cheers, jamal --

From: Herbert Xu <herbert@gondor.apana.org.au> Regardless of whether said configuration is optimal, we should handle this case correctly. So I have applied this patch. --

From: Phil Sutter <phil@nwl.cc> Applied. --

From: Phil Sutter <phil@nwl.cc> Applied. --

The patch is sufficient, but I think it doesn't go far enough. The struct vxge_config in struct vxgedev should be malloc'd as well. struct vxgedev is entirely too big and needs to be smaller, for cache perf and other reasons. Thanks, --

On Wed, 2 Jun 2010 09:52:08 +0200 Rcu assign is not necessary here for because the hook didn't get registered so there is no way for other CPU to see it. -- --

Few nitpicks, I'm going to send patch reflecting this later. Why not to use rcu_assign_pointer here? since this is not hot path, that would be nicer. It should be done for rcu-protected pointers (see rcu_assign_poiter --

Right, I'm not saying it has sense to have it together in the same moment. But you can to this: # ip link add link eth0 type macvlan You can use eth0 + macvlan0. Now you do: # brctl addif br0 eth0 Direct use of eth0 and macvlan0 is not not possible now. # brctl delif br0 eth0 But ok, I hear your arguments, I thought about them myself before I did the patch and I thought that added overhead (which is not too big, I see one more dereference, rx_handler pointer) would be ...

Thats a valid point, but we should use it, and not care of this litle detail. Compiler generates same code anyway, since NULL value is tested by rcu_assign_pointer(). If we dont use rcu_assign_pointer() ourself, Paul or Arnd will put it one day or another :) http://lkml.org/lkml/2010/6/1/290 --

Some year soon, I hope. ;-) Thanx, Paul --

From: Ben Hutchings <bhutchings@solarflare.com> That's perfect, it means we don't have to add explicit settings to drivers which driver only one port. Ie. the vast majority of cases. --

On Wed, 02 Jun 2010 22:34:29 +0100 Another big issue is maintaining ABI compatibility for /proc and ioctl interfaces. So bigger values would only be available through netlink, and most applications using counters don't use netlink. -- --

On Tue, 2010-05-25 at 16:15 -0700, David Miller wrote: My understand of the current situation is as follows; correct me if any of this is wrong: The standard counters in struct net_device_stats have type unsigned long which is the native word size and so can be read and updated automatically. Net drivers can update counters from the data path without any interlocking with their ndo_get_stats implementation or the networking core code which reads them. The values returned by ndo_get_stats ...

From: Eric Dumazet <eric.dumazet@gmail.com> HZ/25 is TCP_DELACK_MIN and TCP_ATO_MIN, but the actual value we use here involves incorporation of various measurements made on the connection (RTO, etc.) There must be a tail-call here at tcp_v4_rcv() or something missed in the backtrace stack scanning logic, because tcp_v4_rcv() and it's main inline tcp_v4_do_rcv() do not modify established state socket timers, and in particular do not modify the delack timer, that I can see. It must be in ...

From: Jiri Pirko <jpirko@redhat.com> Applied. --

Sorry, this was not ment to be a set. I was just posting patches as I went thru the code. Will try to "buffer" it next time. --

From: Jiri Pirko <jpirko@redhat.com> Applied. --

From: Jiri Pirko <jpirko@redhat.com> Applied. --

From: John Fastabend <john.r.fastabend@intel.com> Also applied. --

From: John Fastabend <john.r.fastabend@intel.com> Applied, thanks John. --

From: Eric Dumazet <eric.dumazet@gmail.com> Looks good, applied. --

Thanks David, I realize you did all the necessary changes after qdisc_run_begin/qdisc_is_running/ integration ;) --

__QDISC_STATE_RUNNING is always changed while qdisc lock is held. We can avoid two atomic operations in xmit path, if we move this bit in a new __state container. Location of this __state container is carefully chosen so that fast path only dirties one qdisc cache line. THROTTLED bit could later be moved into this __state location too, to avoid dirtying first qdisc cache line. Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> --- include/net/sch_generic.h | 15 +++++++++++---- 1 ...

From: "Duyck, Alexander H" <alexander.h.duyck@intel.com> Applied, thanks guys. --