* Re: [PATCH] ehea: add kexec support
From: Michael Ellerman @ 2007-10-30 22:50 UTC (permalink / raw)
To: Christoph Raisch
Cc: Jan-Bernd Themann, Jeff Garzik, linux-kernel, linux-ppc,
Marcus Eder, netdev, ossthema, Stefan Roscher, Thomas Q Klein
In-Reply-To: <OF79E7ACE7.5D0488CE-ONC1257384.002E58C0-C1257384.002F8561@de.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 957 bytes --]
On Tue, 2007-10-30 at 09:39 +0100, Christoph Raisch wrote:
>
> Michael Ellerman <michael@ellerman.id.au> wrote on 28.10.2007 23:32:17:
> >
> >
> > How do you plan to support kdump?
> >
>
> When kexec is fully supported kdump should work out of the box
> as for any other ethernet card (if you load the right eth driver).
> There's nothing specific to kdump you have to handle in
> ethernet device drivers.
> Hope I didn't miss anything here...
Perhaps. When we kdump the kernel does not call the reboot notifiers, so
the code Jan-Bernd just added won't get called. So the eHEA resources
won't be freed. When the kdump kernel tries to load the eHEA driver what
will happen?
cheers
--
Michael Ellerman
OzLabs, IBM Australia Development Lab
wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)
We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: recent sysfs changes cause lots of network device errors
From: Greg KH @ 2007-10-30 22:40 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Tejun Heo, David S. Miller, netdev, Kay Sievers
In-Reply-To: <20071030150010.79253f01@freepuppy.rosehill>
On Tue, Oct 30, 2007 at 03:00:10PM -0700, Stephen Hemminger wrote:
> It seems that the network device rename done by distro's
> interacts badly with the code in device_rename() in 2.6.24
> (post -rc1). Network devices create sysfs entries where the sd->s_name is
> just a pointer over to the actual buffer in the netdevice.
>
> Prior to calling device_rename, the code in dev_change_name() updates
> the network device name field to the new name. Then when device_rename
> is called it sees that the new name already exists, and dumps out a bunch
> of sysfs warnings.
>
> I already fixed the obvious case of rename to same name, so that isn't the
> problem.
>
> dev_change_name eth0 -> eth4
> [ 46.029555] sysfs: duplicate filename 'eth4' can not be created
> [ 46.029557] WARNING: at fs/sysfs/dir.c:424 sysfs_add_one()
> [ 46.029559]
> [ 46.029560] Call Trace:
> [ 46.029586] [<ffffffff802e60ff>] sysfs_add_one+0xaf/0xf0
> [ 46.029590] [<ffffffff802e7113>] sysfs_create_link+0xa3/0x140
> [ 46.029597] [<ffffffff803b1bb8>] device_rename+0x1d8/0x230
> [ 46.029603] [<ffffffff8040af45>] dev_change_name+0xe5/0x280
> [ 46.029606] [<ffffffff8040b697>] dev_ioctl+0x2b7/0x540
> [ 46.029612] [<ffffffff803fc32d>] sock_ioctl+0x7d/0x250
> [ 46.029618] [<ffffffff802ab33f>] do_ioctl+0x2f/0xa0
> [ 46.029620] [<ffffffff802ab424>] vfs_ioctl+0x74/0x2d0
> [ 46.029624] [<ffffffff8029bde5>] fd_install+0x25/0x60
> [ 46.029626] [<ffffffff802ab711>] sys_ioctl+0x91/0xb0
> [ 46.029631] [<ffffffff8020bbce>] system_call+0x7e/0x83
>
>
> What is the proper usage mode for the rename code?
> Should the underlying structure get changed first or later?
> Or maybe dev_change_name should just not use device_rename
> at all, and just fix the kobject itself?
>
> device_rename() should be smart enough to:
> 1. not get confused if sysfs entry is already changed
> 2. handle the case of rename to same name correctly.
>
> The device control code needs more regression testing before new patches
> are accepted. I understand there is a strong desire to cleanup and eliminate
> the class device stuff, but before going there you need to create
> regression tests for all usages, and not depend on every subsystem
> maintainer to make changes to keep up with your whims.
Well, this was a bug that no one caught in -mm as we all seem to be
running with CONFIG_SYSFS_DEPRECATED disabled. There was a long
discussion on lkml last week, and here's the patch that should fix it
that is going to Linus in a day or so (it's in my tree, but I'm supposed
to be on vacation right now...)
Let me know if this works for you or not.
thanks,
greg k-h
From: Kay Sievers <kay.sievers@vrfy.org>
Subject: Driver Core: fix bug in device_rename() for SYSFS_DEPRECATED=y
From: Kay Sievers <kay.sievers@vrfy.org>
This should fix the sysfs warnings that renaming network devices is
causing to show up with CONFIG_SYSFS_DEPRECATED=y
The code just shouldn't run if class devices are real directories, it's
an update for the symlink in the class directory. Nobody noticed that as
long as the creation of sysfs files silently failed, and we both missed
it before the merge, because we don't run SYSFS_DEPRECATED=y.
Signed-off-by: Kay Sievers <kay.sievers@vrfy.org>
Cc: Larry Finger <Larry.Finger@lwfinger.net>
Cc: David Miller <davem@davemloft.net>
Cc: Rafael J. Wysocki <rjw@sisk.pl>
Cc: Tejun Heo <htejun@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
---
drivers/base/core.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -1228,18 +1228,18 @@ int device_rename(struct device *dev, ch
sysfs_remove_link(&dev->parent->kobj, old_class_name);
}
}
-#endif
-
+#else
if (dev->class) {
sysfs_remove_link(&dev->class->subsys.kobj, old_device_name);
error = sysfs_create_link(&dev->class->subsys.kobj, &dev->kobj,
dev->bus_id);
if (error) {
- /* Uh... how to unravel this if restoring can fail? */
dev_err(dev, "%s: sysfs_create_symlink failed (%d)\n",
__FUNCTION__, error);
}
}
+#endif
+
out:
put_device(dev);
^ permalink raw reply
* Re: [PATCH 1/2] Convert /proc/net/ipv6_route to seq_file interface
From: David Miller @ 2007-10-30 22:41 UTC (permalink / raw)
To: adobriyan; +Cc: netdev
In-Reply-To: <20071030131147.GA6210@localhost.sw.ru>
From: Alexey Dobriyan <adobriyan@sw.ru>
Date: Tue, 30 Oct 2007 16:11:47 +0300
> One proc_net_create() user less.
>
> Signed-off-by: Alexey Dobriyan <adobriyan@sw.ru>
Please make the corrections suggested in the feedback
for this patch and resubmit.
Thanks!
^ permalink raw reply
* Re: [PATCH 2/2] Remove /proc/net/ip_vs_lblcr
From: David Miller @ 2007-10-30 22:40 UTC (permalink / raw)
To: adobriyan; +Cc: netdev
In-Reply-To: <20071030131257.GB6210@localhost.sw.ru>
From: Alexey Dobriyan <adobriyan@sw.ru>
Date: Tue, 30 Oct 2007 16:12:57 +0300
> It's under CONFIG_IP_VS_LBLCR_DEBUG option which never existed.
>
> Signed-off-by: Alexey Dobriyan <adobriyan@sw.ru>
Applied, thanks.
^ permalink raw reply
* Re: [patch 1/1][IPV6] resend: remove duplicate call to proc_net_remove
From: David Miller @ 2007-10-30 22:39 UTC (permalink / raw)
To: dlezcano; +Cc: ebiederm, containers, netdev
In-Reply-To: <20071030162450.257974469@mai.toulouse-stg.fr.ibm.com>
From: Daniel Lezcano <dlezcano@fr.ibm.com>
Date: Tue, 30 Oct 2007 17:23:45 +0100
> The file /proc/net/if_inet6 is removed twice.
> First time in:
> inet6_exit
> ->addrconf_cleanup
> And followed a few lines after by:
> inet6_exit
> -> if6_proc_exit
>
> Signed-off-by: Daniel Lezcano <dlezcano@fr.ibm.com>
Applied, thanks Daniel.
^ permalink raw reply
* Re: [patch 1/1][NETNS] resend: fix net released by rcu callback
From: David Miller @ 2007-10-30 22:39 UTC (permalink / raw)
To: dlezcano; +Cc: ebiederm, containers, netdev
In-Reply-To: <20071030162305.458123510@mai.toulouse-stg.fr.ibm.com>
From: Daniel Lezcano <dlezcano@fr.ibm.com>
Date: Tue, 30 Oct 2007 17:21:40 +0100
> When a network namespace reference is held by a network subsystem,
> and when this reference is decremented in a rcu update callback, we
> must ensure that there is no more outstanding rcu update before
> trying to free the network namespace.
>
> In the normal case, the rcu_barrier is called when the network namespace
> is exiting in the cleanup_net function.
>
> But when a network namespace creation fails, and the subsystems are
> undone (like the cleanup), the rcu_barrier is missing.
>
> This patch adds the missing rcu_barrier.
>
> Signed-off-by: Daniel Lezcano <dlezcano@fr.ibm.com>
Applied, thanks Daniel.
^ permalink raw reply
* Re: [patch 1/1][CORE] resend - fix free_netdev on register_netdev failure
From: David Miller @ 2007-10-30 22:38 UTC (permalink / raw)
To: dlezcano; +Cc: netdev
In-Reply-To: <20071030162059.010348834@mai.toulouse-stg.fr.ibm.com>
From: Daniel Lezcano <dlezcano@fr.ibm.com>
Date: Tue, 30 Oct 2007 17:19:26 +0100
> Point 1:
> The unregistering of a network device schedule a netdev_run_todo.
> This function calls dev->destructor when it is set and the
> destructor calls free_netdev.
>
> Point 2:
> In the case of an initialization of a network device the usual code
> is:
> * alloc_netdev
> * register_netdev
> -> if this one fails, call free_netdev and exit with error.
>
> Point 3:
> In the register_netdevice function at the later state, when the device
> is at the registered state, a call to the netdevice_notifiers is made.
> If one of the notification falls into an error, a rollback to the
> registered state is done using unregister_netdevice.
>
> Conclusion:
> When a network device fails to register during initialization because
> one network subsystem returned an error during a notification call
> chain, the network device is freed twice because of fact 1 and fact 2.
> The second free_netdev will be done with an invalid pointer.
>
> Proposed solution:
> The following patch move all the code of unregister_netdevice *except*
> the call to net_set_todo, to a new function "rollback_registered".
>
> The following functions are changed in this way:
> * register_netdevice: calls rollback_registered when a notification fails
> * unregister_netdevice: calls rollback_register + net_set_todo, the call
> order to net_set_todo is changed because it is the
> latest now. Since it justs add an element to a list
> that should not break anything.
>
> Signed-off-by: Daniel Lezcano <dlezcano@fr.ibm.com>
Looks good, applied.
^ permalink raw reply
* Re: [patch 1/1][NETNS] resend: fix net released by rcu callback
From: Eric W. Biederman @ 2007-10-30 22:34 UTC (permalink / raw)
To: Daniel Lezcano; +Cc: davem, containers, netdev
In-Reply-To: <4727A57E.501@fr.ibm.com>
Daniel Lezcano <dlezcano@fr.ibm.com> writes:
> Yes, I had this problem when doing ipv6 isolation for netns49. The ipv6
> subsystem creation failed and the different subsystem where rollbacked in the
> setup_net function.
> When the network namespace was about to be freed in free_net function, I had the
> error with an usage refcount different from zero.
> It appears that was coming from core/neighbour.c
>
> neigh_parms_release
> -> neigh_rcu_free_parms
> -> neigh_parms_put
> -> neigh_parms_destroy
> -> release_net
>
> The free_net function was called before rcu callback neigh_rcu_free_parms.
Ok. Cool. It is good to know that this patch was tested. These kinds
of subtle fixes are always more solid when you can actually test them.
Eric
^ permalink raw reply
* Re: [PATCH] remove claim balance_rr won't reorder on many to one
From: Rick Jones @ 2007-10-30 22:12 UTC (permalink / raw)
To: Jay Vosburgh; +Cc: netdev
In-Reply-To: <5242.1193777750@death>
Jay Vosburgh wrote:
> Rick Jones <rick.jones2@hp.com> wrote:
> [...]
>
>>- Note that this out of order delivery occurs when both the
>>- sending and receiving systems are utilizing a multiple
>>- interface bond. Consider a configuration in which a
>>- balance-rr bond feeds into a single higher capacity network
>>- channel (e.g., multiple 100Mb/sec ethernets feeding a single
>>- gigabit ethernet via an etherchannel capable switch). In this
>>- configuration, traffic sent from the multiple 100Mb devices to
>>- a destination connected to the gigabit device will not see
>>- packets out of order. However, traffic sent from the gigabit
>>- device to the multiple 100Mb devices may or may not see
>>- traffic out of order, depending upon the balance policy of the
>>- switch. Many switches do not support any modes that stripe
>>- traffic (instead choosing a port based upon IP or MAC level
>>- addresses); for those devices, traffic flowing from the
>>- gigabit device to the many 100Mb devices will only utilize one
>>- interface.
>
>
> Rather than simply removing this entirely (because I do think
> there is value in discussion of the reordering aspects of balance-rr),
> I'd rather see something that makes the following points:
>
> 1- the worst reordering is balance-rr to balance-rr, back to
> back. The reordering rate here depends upon (a) the number of slaves
> involved and (b) packet reception scheduling behaviors (packet
> coalescing, NAPI, etc), and thus will vary signficantly, but won't be
> better than case #2.
>
> 2- next worst is "balance-rr many slow" to "single fast", with
> the reordering rate generally being substantially lower than case #1 (it
> looked like your test showed about a 1% reordering rate, if I'm reading
> your data correctly).
>
> 3- For the "single fast" to "balance-rr many" case, going
> through a switch configured for etherchannel "may or may not see traffic
> out of order, depending upon the balance policy of the switch. Many
> switches do not support any modes that stripe traffic (instead choosing
> a port based upon IP or MAC level addresses); for those devices, traffic
> flowing from the [single fast] device to the [balance-rr many] devices
> will only utilize one interface."
I have to wonder if the full description of the different versions of being a
little bit pregnant is worth it. Just saying that using balance-rr will result
in reordering seems much more simple to comprehend. Also, since balance-rr is
strictly an outbound policy, does case three even enter into it - as you say,
that will be up to the switch, which will be doing whatever it was told or felt
like doing regardless of balance-rr on the bond in the host.
>
> [...]
>
>> This mode requires the switch to have the appropriate ports
>>- configured for "etherchannel" or "trunking."
>>+ configured for "etherchannel" or "aggregation." N.B. some
>>+ switches might use the term "trunking" for something other
>>+ than link aggregation.
>
>
> If memory serves, Sun uses the term "trunking" to refer to
> "etherchannel" compatible behavior.
I'm not really all that tied to that part of the change - it is there because I
noticed in one of the HP ITRC forums someone talking about a switch (Cisco?)
where trunking meant something with vlans rather than aggregation.
>
> I'm also hearing "aggregation" used to described 802.3ad
> specifically.
>
> Perhaps text of the form:
>
> This mode requires the switch to have the appropriate ports
> configured for "Etherchannel." Some switches use different terms, so
> the configuration may be called "trunking" or "aggregation." Note that
> both of these terms also have other meanings. For example, "trunking"
> is also used to describe a type of switch port, and "aggregation" or
> "link aggregation" is often used to refer to 802.3ad link aggregation,
> which is compatible with bonding's 802.3ad mode, but not balance-rr.
>
> Thoughts?
Even better would be to be able to start to move away from "etherchannel"
towards the de jure standard's terms, whatever the heck they are :)
rick jones
^ permalink raw reply
* recent sysfs changes cause lots of network device errors
From: Stephen Hemminger @ 2007-10-30 22:00 UTC (permalink / raw)
To: Greg KH, Tejun Heo, David S. Miller; +Cc: netdev
It seems that the network device rename done by distro's
interacts badly with the code in device_rename() in 2.6.24
(post -rc1). Network devices create sysfs entries where the sd->s_name is
just a pointer over to the actual buffer in the netdevice.
Prior to calling device_rename, the code in dev_change_name() updates
the network device name field to the new name. Then when device_rename
is called it sees that the new name already exists, and dumps out a bunch
of sysfs warnings.
I already fixed the obvious case of rename to same name, so that isn't the
problem.
dev_change_name eth0 -> eth4
[ 46.029555] sysfs: duplicate filename 'eth4' can not be created
[ 46.029557] WARNING: at fs/sysfs/dir.c:424 sysfs_add_one()
[ 46.029559]
[ 46.029560] Call Trace:
[ 46.029586] [<ffffffff802e60ff>] sysfs_add_one+0xaf/0xf0
[ 46.029590] [<ffffffff802e7113>] sysfs_create_link+0xa3/0x140
[ 46.029597] [<ffffffff803b1bb8>] device_rename+0x1d8/0x230
[ 46.029603] [<ffffffff8040af45>] dev_change_name+0xe5/0x280
[ 46.029606] [<ffffffff8040b697>] dev_ioctl+0x2b7/0x540
[ 46.029612] [<ffffffff803fc32d>] sock_ioctl+0x7d/0x250
[ 46.029618] [<ffffffff802ab33f>] do_ioctl+0x2f/0xa0
[ 46.029620] [<ffffffff802ab424>] vfs_ioctl+0x74/0x2d0
[ 46.029624] [<ffffffff8029bde5>] fd_install+0x25/0x60
[ 46.029626] [<ffffffff802ab711>] sys_ioctl+0x91/0xb0
[ 46.029631] [<ffffffff8020bbce>] system_call+0x7e/0x83
What is the proper usage mode for the rename code?
Should the underlying structure get changed first or later?
Or maybe dev_change_name should just not use device_rename
at all, and just fix the kobject itself?
device_rename() should be smart enough to:
1. not get confused if sysfs entry is already changed
2. handle the case of rename to same name correctly.
The device control code needs more regression testing before new patches
are accepted. I understand there is a strong desire to cleanup and eliminate
the class device stuff, but before going there you need to create
regression tests for all usages, and not depend on every subsystem
maintainer to make changes to keep up with your whims.
--
Stephen Hemminger <shemminger@linux-foundation.org>
^ permalink raw reply
* Re: Bonding in active-backup mode with arp monitoring on Xen
From: Jay Vosburgh @ 2007-10-30 21:48 UTC (permalink / raw)
To: Tsutomu Fujii; +Cc: netdev
In-Reply-To: <47253487.3010909@nb.jp.nec.com>
Tsutomu Fujii <t-fujii@nb.jp.nec.com> wrote:
[...]
>The network structure is the following.
>
>+-------------------------------+
>| virtual I/F(eth0) | <- have IP address.
>+-------------------------------+
>| virtual bridge(xenbr0) |
>+-------------------------------+
>| bonding(bond0) | <- don't have IP address
>+--------------+-+--------------+
>|phy I/F(peth0)| |phy I/F(peth1)|
>+--------------+ +--------------+
[...]
>How about removing bond_has_ip() from the condition for calling
>bond_arp_send_all() to use bonding with xen?
>When I removed bond_has_ip(), the resulting kernel worked fine and
>active-backup mode works with arp monitoring.
I looked at this a bit. I believe that when you remove the
bond_has_ip() test, the ARP probes sent out will be of the form "ARP
who-has A.B.C.D tell 0.0.0.0", which the linux kernel will interpret as
IPv4 Duplicate Address Detection probes (as a special case in
arp_process()), and generate some type of reply. That reply appears to
be enough to keep the ARP monitor happy.
I'm not sure if this is a solution that will work for any peer
(some peers may not reply to an ARP with an IP source of all zeros). At
first glance, there doesn't seem to be much of a downside, but I'll have
to experiment with it a bit to see if the check should be optional or
simply removed entirely.
-J
---
-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
^ permalink raw reply
* Re: [PATCH] net: Saner thash_entries default with much memory
From: David Miller @ 2007-10-30 21:46 UTC (permalink / raw)
To: ak; +Cc: jdelvare, netdev
In-Reply-To: <200710302142.05753.ak@suse.de>
From: Andi Kleen <ak@suse.de>
Date: Tue, 30 Oct 2007 21:42:05 +0100
> I still have my doubts it makes sense to have an own lock for each bucket. It
> would be probably better to just divide the hash value through a factor
> again and then use that to index a smaller lock only table.
Yes, and that's why we do it this way in the routing cache hashes.
^ permalink raw reply
* Re: [patch 1/1][NETNS] resend: fix net released by rcu callback
From: Daniel Lezcano @ 2007-10-30 21:43 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: davem, containers, netdev
In-Reply-To: <m1odegl47t.fsf@ebiederm.dsl.xmission.com>
Eric W. Biederman wrote:
> Daniel Lezcano <dlezcano@fr.ibm.com> writes:
>
>> When a network namespace reference is held by a network subsystem,
>> and when this reference is decremented in a rcu update callback, we
>> must ensure that there is no more outstanding rcu update before
>> trying to free the network namespace.
>>
>> In the normal case, the rcu_barrier is called when the network namespace
>> is exiting in the cleanup_net function.
>>
>> But when a network namespace creation fails, and the subsystems are
>> undone (like the cleanup), the rcu_barrier is missing.
>>
>> This patch adds the missing rcu_barrier.
>
> Looks sane. Did you have any specific failures related to this or was
> this something that was just caught in review?
Yes, I had this problem when doing ipv6 isolation for netns49. The ipv6
subsystem creation failed and the different subsystem where rollbacked
in the setup_net function.
When the network namespace was about to be freed in free_net function, I
had the error with an usage refcount different from zero.
It appears that was coming from core/neighbour.c
neigh_parms_release
-> neigh_rcu_free_parms
-> neigh_parms_put
-> neigh_parms_destroy
-> release_net
The free_net function was called before rcu callback neigh_rcu_free_parms.
^ permalink raw reply
* Re: [PATCH 23/33] netvm: skb processing
From: Peter Zijlstra @ 2007-10-30 21:44 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Linus Torvalds, Andrew Morton, linux-kernel, linux-mm, netdev,
Trond Myklebust
In-Reply-To: <20071030142634.0f00b492@freepuppy.rosehill>
On Tue, 2007-10-30 at 14:26 -0700, Stephen Hemminger wrote:
> On Tue, 30 Oct 2007 17:04:24 +0100
> Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
>
> > In order to make sure emergency packets receive all memory needed to proceed
> > ensure processing of emergency SKBs happens under PF_MEMALLOC.
> >
> > Use the (new) sk_backlog_rcv() wrapper to ensure this for backlog processing.
> >
> > Skip taps, since those are user-space again.
> >
> > Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
> > ---
> > include/net/sock.h | 5 +++++
> > net/core/dev.c | 44 ++++++++++++++++++++++++++++++++++++++------
> > net/core/sock.c | 18 ++++++++++++++++++
> > 3 files changed, 61 insertions(+), 6 deletions(-)
> >
> > Index: linux-2.6/net/core/dev.c
> > ===================================================================
> > --- linux-2.6.orig/net/core/dev.c
> > +++ linux-2.6/net/core/dev.c
> > @@ -1976,10 +1976,23 @@ int netif_receive_skb(struct sk_buff *sk
> > struct net_device *orig_dev;
> > int ret = NET_RX_DROP;
> > __be16 type;
> > + unsigned long pflags = current->flags;
> > +
> > + /* Emergency skb are special, they should
> > + * - be delivered to SOCK_MEMALLOC sockets only
> > + * - stay away from userspace
> > + * - have bounded memory usage
> > + *
> > + * Use PF_MEMALLOC as a poor mans memory pool - the grouping kind.
> > + * This saves us from propagating the allocation context down to all
> > + * allocation sites.
> > + */
> > + if (skb_emergency(skb))
> > + current->flags |= PF_MEMALLOC;
> >
> > /* if we've gotten here through NAPI, check netpoll */
> > if (netpoll_receive_skb(skb))
> > - return NET_RX_DROP;
> > + goto out;
>
> Why the change? doesn't gcc optimize the common exit case anyway?
It needs to unset PF_MEMALLOC at the exit.
> > @@ -2029,19 +2046,31 @@ int netif_receive_skb(struct sk_buff *sk
> >
> > if (ret == TC_ACT_SHOT || (ret == TC_ACT_STOLEN)) {
> > kfree_skb(skb);
> > - goto out;
> > + goto unlock;
> > }
> >
> > skb->tc_verd = 0;
> > ncls:
> > #endif
> >
> > + if (skb_emergency(skb))
> > + switch(skb->protocol) {
> > + case __constant_htons(ETH_P_ARP):
> > + case __constant_htons(ETH_P_IP):
> > + case __constant_htons(ETH_P_IPV6):
> > + case __constant_htons(ETH_P_8021Q):
> > + break;
>
> Indentation is wrong, and hard coding protocol values as spcial case
> seems bad here. What about vlan's, etc?
The other protocols needs analysis on what memory allocations occur
during packet processing, if anything is done that is not yet accounted
for (skb, route cache) then that needs to be added to a reserve, if
there are any paths that could touch user-space, those need to be
handled.
I've started looking at a few others, but its hard and difficult work if
one is not familiar with the protocols.
> > @@ -2063,8 +2093,10 @@ ncls:
> > ret = NET_RX_DROP;
> > }
> >
> > -out:
> > +unlock:
> > rcu_read_unlock();
> > +out:
> > + tsk_restore_flags(current, pflags, PF_MEMALLOC);
> > return ret;
> > }
Its that tsk_restore_flags() there what requires the s/return/goto/
stuff you noted earlier.
> I am still not convinced that this solves the problem well enough
> to be useful. Can you really survive a heavy memory overcommit?
On a machine with mem=128M, I've ran 4 processes of 64M, 2 file backed
with the files on NFS, 2 anonymous. The processes just cycle through the
memory using writes. This is a 100% overcommit.
During these tests I've ran various network loads.
I've shut down the NFS server, waited for say 15 minutes, and restarted
the NFS server, and the machine came back up and continued.
> In other words, can you prove that the added complexity causes the system
> to survive a real test where otherwise it would not?
I've put some statistics in the skb reserve allocations, those are most
definately used. I'm quite certain the machine would lock up solid
without it.
^ permalink raw reply
* Re: [PATCH 23/33] netvm: skb processing
From: Stephen Hemminger @ 2007-10-30 21:26 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: netdev
In-Reply-To: <20071030160914.749995000@chello.nl>
On Tue, 30 Oct 2007 17:04:24 +0100
Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
> In order to make sure emergency packets receive all memory needed to proceed
> ensure processing of emergency SKBs happens under PF_MEMALLOC.
>
> Use the (new) sk_backlog_rcv() wrapper to ensure this for backlog processing.
>
> Skip taps, since those are user-space again.
>
> Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
> ---
> include/net/sock.h | 5 +++++
> net/core/dev.c | 44 ++++++++++++++++++++++++++++++++++++++------
> net/core/sock.c | 18 ++++++++++++++++++
> 3 files changed, 61 insertions(+), 6 deletions(-)
>
> Index: linux-2.6/net/core/dev.c
> ===================================================================
> --- linux-2.6.orig/net/core/dev.c
> +++ linux-2.6/net/core/dev.c
> @@ -1976,10 +1976,23 @@ int netif_receive_skb(struct sk_buff *sk
> struct net_device *orig_dev;
> int ret = NET_RX_DROP;
> __be16 type;
> + unsigned long pflags = current->flags;
> +
> + /* Emergency skb are special, they should
> + * - be delivered to SOCK_MEMALLOC sockets only
> + * - stay away from userspace
> + * - have bounded memory usage
> + *
> + * Use PF_MEMALLOC as a poor mans memory pool - the grouping kind.
> + * This saves us from propagating the allocation context down to all
> + * allocation sites.
> + */
> + if (skb_emergency(skb))
> + current->flags |= PF_MEMALLOC;
>
> /* if we've gotten here through NAPI, check netpoll */
> if (netpoll_receive_skb(skb))
> - return NET_RX_DROP;
> + goto out;
Why the change? doesn't gcc optimize the common exit case anyway?
>
> if (!skb->tstamp.tv64)
> net_timestamp(skb);
> @@ -1990,7 +2003,7 @@ int netif_receive_skb(struct sk_buff *sk
> orig_dev = skb_bond(skb);
>
> if (!orig_dev)
> - return NET_RX_DROP;
> + goto out;
>
> __get_cpu_var(netdev_rx_stat).total++;
>
> @@ -2009,6 +2022,9 @@ int netif_receive_skb(struct sk_buff *sk
> }
> #endif
>
> + if (skb_emergency(skb))
> + goto skip_taps;
> +
> list_for_each_entry_rcu(ptype, &ptype_all, list) {
> if (!ptype->dev || ptype->dev == skb->dev) {
> if (pt_prev)
> @@ -2017,6 +2033,7 @@ int netif_receive_skb(struct sk_buff *sk
> }
> }
>
> +skip_taps:
> #ifdef CONFIG_NET_CLS_ACT
> if (pt_prev) {
> ret = deliver_skb(skb, pt_prev, orig_dev);
> @@ -2029,19 +2046,31 @@ int netif_receive_skb(struct sk_buff *sk
>
> if (ret == TC_ACT_SHOT || (ret == TC_ACT_STOLEN)) {
> kfree_skb(skb);
> - goto out;
> + goto unlock;
> }
>
> skb->tc_verd = 0;
> ncls:
> #endif
>
> + if (skb_emergency(skb))
> + switch(skb->protocol) {
> + case __constant_htons(ETH_P_ARP):
> + case __constant_htons(ETH_P_IP):
> + case __constant_htons(ETH_P_IPV6):
> + case __constant_htons(ETH_P_8021Q):
> + break;
Indentation is wrong, and hard coding protocol values as spcial case
seems bad here. What about vlan's, etc?
> + default:
> + goto drop;
> + }
> +
> skb = handle_bridge(skb, &pt_prev, &ret, orig_dev);
> if (!skb)
> - goto out;
> + goto unlock;
> skb = handle_macvlan(skb, &pt_prev, &ret, orig_dev);
> if (!skb)
> - goto out;
> + goto unlock;
>
> type = skb->protocol;
> list_for_each_entry_rcu(ptype, &ptype_base[ntohs(type)&15], list) {
> @@ -2056,6 +2085,7 @@ ncls:
> if (pt_prev) {
> ret = pt_prev->func(skb, skb->dev, pt_prev, orig_dev);
> } else {
> +drop:
> kfree_skb(skb);
> /* Jamal, now you will not able to escape explaining
> * me how you were going to use this. :-)
> @@ -2063,8 +2093,10 @@ ncls:
> ret = NET_RX_DROP;
> }
>
> -out:
> +unlock:
> rcu_read_unlock();
> +out:
> + tsk_restore_flags(current, pflags, PF_MEMALLOC);
> return ret;
> }
>
> Index: linux-2.6/include/net/sock.h
> ===================================================================
> --- linux-2.6.orig/include/net/sock.h
> +++ linux-2.6/include/net/sock.h
> @@ -523,8 +523,13 @@ static inline void sk_add_backlog(struct
> skb->next = NULL;
> }
>
> +extern int __sk_backlog_rcv(struct sock *sk, struct sk_buff *skb);
> +
> static inline int sk_backlog_rcv(struct sock *sk, struct sk_buff *skb)
> {
> + if (skb_emergency(skb))
> + return __sk_backlog_rcv(sk, skb);
> +
> return sk->sk_backlog_rcv(sk, skb);
> }
>
> Index: linux-2.6/net/core/sock.c
> ===================================================================
> --- linux-2.6.orig/net/core/sock.c
> +++ linux-2.6/net/core/sock.c
> @@ -319,6 +319,24 @@ int sk_clear_memalloc(struct sock *sk)
> }
> EXPORT_SYMBOL_GPL(sk_clear_memalloc);
>
> +#ifdef CONFIG_NETVM
> +int __sk_backlog_rcv(struct sock *sk, struct sk_buff *skb)
> +{
> + int ret;
> + unsigned long pflags = current->flags;
> +
> + /* these should have been dropped before queueing */
> + BUG_ON(!sk_has_memalloc(sk));
> +
> + current->flags |= PF_MEMALLOC;
> + ret = sk->sk_backlog_rcv(sk, skb);
> + tsk_restore_flags(current, pflags, PF_MEMALLOC);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(__sk_backlog_rcv);
> +#endif
> +
> static int sock_set_timeout(long *timeo_p, char __user *optval, int optlen)
> {
> struct timeval tv;
I am still not convinced that this solves the problem well enough
to be useful. Can you really survive a heavy memory overcommit?
In other words, can you prove that the added complexity causes the system
to survive a real test where otherwise it would not?
--
Stephen Hemminger <shemminger@linux-foundation.org>
^ permalink raw reply
* Re: [PATCH] net: Saner thash_entries default with much memory
From: David Miller @ 2007-10-30 21:11 UTC (permalink / raw)
To: jdelvare; +Cc: ak, netdev
In-Reply-To: <200710301418.27643.jdelvare@suse.de>
From: Jean Delvare <jdelvare@suse.de>
Date: Tue, 30 Oct 2007 14:18:27 +0100
> OK, let's go with (512 * 1024) then. Want me to send an updated patch?
Why submit a patch that's already in Linus's tree :-)
^ permalink raw reply
* Re: [PATCH] remove claim balance_rr won't reorder on many to one
From: Jay Vosburgh @ 2007-10-30 20:55 UTC (permalink / raw)
To: Rick Jones; +Cc: netdev
In-Reply-To: <200710301948.MAA04351@tardy.cup.hp.com>
Rick Jones <rick.jones2@hp.com> wrote:
[...]
>- Note that this out of order delivery occurs when both the
>- sending and receiving systems are utilizing a multiple
>- interface bond. Consider a configuration in which a
>- balance-rr bond feeds into a single higher capacity network
>- channel (e.g., multiple 100Mb/sec ethernets feeding a single
>- gigabit ethernet via an etherchannel capable switch). In this
>- configuration, traffic sent from the multiple 100Mb devices to
>- a destination connected to the gigabit device will not see
>- packets out of order. However, traffic sent from the gigabit
>- device to the multiple 100Mb devices may or may not see
>- traffic out of order, depending upon the balance policy of the
>- switch. Many switches do not support any modes that stripe
>- traffic (instead choosing a port based upon IP or MAC level
>- addresses); for those devices, traffic flowing from the
>- gigabit device to the many 100Mb devices will only utilize one
>- interface.
Rather than simply removing this entirely (because I do think
there is value in discussion of the reordering aspects of balance-rr),
I'd rather see something that makes the following points:
1- the worst reordering is balance-rr to balance-rr, back to
back. The reordering rate here depends upon (a) the number of slaves
involved and (b) packet reception scheduling behaviors (packet
coalescing, NAPI, etc), and thus will vary signficantly, but won't be
better than case #2.
2- next worst is "balance-rr many slow" to "single fast", with
the reordering rate generally being substantially lower than case #1 (it
looked like your test showed about a 1% reordering rate, if I'm reading
your data correctly).
3- For the "single fast" to "balance-rr many" case, going
through a switch configured for etherchannel "may or may not see traffic
out of order, depending upon the balance policy of the switch. Many
switches do not support any modes that stripe traffic (instead choosing
a port based upon IP or MAC level addresses); for those devices, traffic
flowing from the [single fast] device to the [balance-rr many] devices
will only utilize one interface."
[...]
> This mode requires the switch to have the appropriate ports
>- configured for "etherchannel" or "trunking."
>+ configured for "etherchannel" or "aggregation." N.B. some
>+ switches might use the term "trunking" for something other
>+ than link aggregation.
If memory serves, Sun uses the term "trunking" to refer to
"etherchannel" compatible behavior.
I'm also hearing "aggregation" used to described 802.3ad
specifically.
Perhaps text of the form:
This mode requires the switch to have the appropriate ports
configured for "Etherchannel." Some switches use different terms, so
the configuration may be called "trunking" or "aggregation." Note that
both of these terms also have other meanings. For example, "trunking"
is also used to describe a type of switch port, and "aggregation" or
"link aggregation" is often used to refer to 802.3ad link aggregation,
which is compatible with bonding's 802.3ad mode, but not balance-rr.
Thoughts?
-J
---
-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
^ permalink raw reply
* Re: [patch 1/1][IPV6] resend: remove duplicate call to proc_net_remove
From: Eric W. Biederman @ 2007-10-30 20:53 UTC (permalink / raw)
To: Daniel Lezcano; +Cc: davem, containers, netdev
In-Reply-To: <20071030162450.257974469@mai.toulouse-stg.fr.ibm.com>
Daniel Lezcano <dlezcano@fr.ibm.com> writes:
> The file /proc/net/if_inet6 is removed twice.
> First time in:
> inet6_exit
> ->addrconf_cleanup
> And followed a few lines after by:
> inet6_exit
> -> if6_proc_exit
>
> Signed-off-by: Daniel Lezcano <dlezcano@fr.ibm.com>
Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>
Looks like a good clean up to me.
> ---
> net/ipv6/addrconf.c | 4 ----
> 1 file changed, 4 deletions(-)
>
> Index: net-2.6/net/ipv6/addrconf.c
> ===================================================================
> --- net-2.6.orig/net/ipv6/addrconf.c
> +++ net-2.6/net/ipv6/addrconf.c
> @@ -4288,8 +4288,4 @@ void __exit addrconf_cleanup(void)
> del_timer(&addr_chk_timer);
>
> rtnl_unlock();
> -
> -#ifdef CONFIG_PROC_FS
> - proc_net_remove(&init_net, "if_inet6");
> -#endif
> }
>
> --
^ permalink raw reply
* Re: [patch 1/1][NETNS] resend: fix net released by rcu callback
From: Eric W. Biederman @ 2007-10-30 20:47 UTC (permalink / raw)
To: Daniel Lezcano; +Cc: davem, containers, netdev
In-Reply-To: <20071030162305.458123510@mai.toulouse-stg.fr.ibm.com>
Daniel Lezcano <dlezcano@fr.ibm.com> writes:
> When a network namespace reference is held by a network subsystem,
> and when this reference is decremented in a rcu update callback, we
> must ensure that there is no more outstanding rcu update before
> trying to free the network namespace.
>
> In the normal case, the rcu_barrier is called when the network namespace
> is exiting in the cleanup_net function.
>
> But when a network namespace creation fails, and the subsystems are
> undone (like the cleanup), the rcu_barrier is missing.
>
> This patch adds the missing rcu_barrier.
Looks sane. Did you have any specific failures related to this or was
this something that was just caught in review?
Eric
^ permalink raw reply
* Re: [2.6 patch] fix drivers/net/wan/lmc/ compilation
From: Roel Kluin @ 2007-10-30 20:45 UTC (permalink / raw)
To: Adrian Bunk; +Cc: Andrew Morton, David S. Miller, netdev, jgarzik, linux-kernel
In-Reply-To: <20071030203136.GQ7227@stusta.de>
Adrian Bunk wrote:
> Documentation/SubmitChecklist, point 1:
>
> <-- snip -->
>
> ...
> CC drivers/net/wan/lmc/lmc_main.o
> /home/bunk/linux/kernel-2.6/git/linux-2.6/drivers/net/wan/lmc/lmc_main.c: In function ‘lmc_ioctl’:
> /home/bunk/linux/kernel-2.6/git/linux-2.6/drivers/net/wan/lmc/lmc_main.c:239: error: expected expression before ‘else’
> ...
> make[5]: *** [drivers/net/wan/lmc/lmc_main.o] Error 1
>
> <-- snip -->
>
> Signed-off-by: Adrian Bunk <bunk@kernel.org>
>
> ---
> d5e92a30491abf073e0a7f4d46b466c7c97f0f61
> diff --git a/drivers/net/wan/lmc/lmc_main.c b/drivers/net/wan/lmc/lmc_main.c
> index 64eb578..37c52e1 100644
> --- a/drivers/net/wan/lmc/lmc_main.c
> +++ b/drivers/net/wan/lmc/lmc_main.c
> @@ -234,7 +234,7 @@ int lmc_ioctl (struct net_device *dev, struct ifreq *ifr, int cmd) /*fold00*/
> sc->lmc_xinfo.Magic1 = 0xDEADBEEF;
>
> if (copy_to_user(ifr->ifr_data, &sc->lmc_xinfo,
> - sizeof(struct lmc_xinfo))) {
> + sizeof(struct lmc_xinfo)))
> ret = -EFAULT;
> else
> ret = 0;
>
I am sorry, my patch broke this and Kristov Provost also noticed this.
See http://lkml.org/lkml/2007/10/30/355
^ permalink raw reply
* Re: [PATCH] net: Saner thash_entries default with much memory
From: Andi Kleen @ 2007-10-30 20:42 UTC (permalink / raw)
To: David Miller; +Cc: jdelvare, netdev
In-Reply-To: <20071030.005758.184980120.davem@davemloft.net>
> Next, machines that service that many sockets typically have them
> mostly with full transmit queues talking to a very slow receiver at
> the other end.
Not sure -- there are likely use cases with lots of idle but connected
sockets.
Also the constraint here is not really how many sockets are served,
but how well the hash function manages to spread them in the table.. I don't
have good data on that.
But still (512 * 1024) sounds reasonable because e.g. in the lots
of idle socket case you're probably fine with the hash chains
having more than one entry worst case because a small working
set will fit in cache and as long as the chains do not end up
very long walking in cache of a short list will be still fast enough.
> So to me (512 * 1024) is a very reasonable limit and (with lockdep
> and spinlock debugging disabled) this makes the EHASH table consume
> 8MB on UP 64-bit and ~12MB on SMP 64-bit systems.
I still have my doubts it makes sense to have an own lock for each bucket. It
would be probably better to just divide the hash value through a factor
again and then use that to index a smaller lock only table.
-Andi
^ permalink raw reply
* [2.6 patch] fix drivers/net/wan/lmc/ compilation
From: Adrian Bunk @ 2007-10-30 20:31 UTC (permalink / raw)
To: Roel Kluin, Andrew Morton, David S. Miller; +Cc: netdev, jgarzik, linux-kernel
Documentation/SubmitChecklist, point 1:
<-- snip -->
...
CC drivers/net/wan/lmc/lmc_main.o
/home/bunk/linux/kernel-2.6/git/linux-2.6/drivers/net/wan/lmc/lmc_main.c: In function ‘lmc_ioctl’:
/home/bunk/linux/kernel-2.6/git/linux-2.6/drivers/net/wan/lmc/lmc_main.c:239: error: expected expression before ‘else’
...
make[5]: *** [drivers/net/wan/lmc/lmc_main.o] Error 1
<-- snip -->
Signed-off-by: Adrian Bunk <bunk@kernel.org>
---
d5e92a30491abf073e0a7f4d46b466c7c97f0f61
diff --git a/drivers/net/wan/lmc/lmc_main.c b/drivers/net/wan/lmc/lmc_main.c
index 64eb578..37c52e1 100644
--- a/drivers/net/wan/lmc/lmc_main.c
+++ b/drivers/net/wan/lmc/lmc_main.c
@@ -234,7 +234,7 @@ int lmc_ioctl (struct net_device *dev, struct ifreq *ifr, int cmd) /*fold00*/
sc->lmc_xinfo.Magic1 = 0xDEADBEEF;
if (copy_to_user(ifr->ifr_data, &sc->lmc_xinfo,
- sizeof(struct lmc_xinfo))) {
+ sizeof(struct lmc_xinfo)))
ret = -EFAULT;
else
ret = 0;
^ permalink raw reply related
* [PATCH] remove claim balance_rr won't reorder on many to one
From: Rick Jones @ 2007-10-30 19:48 UTC (permalink / raw)
To: netdev
Remove the text which suggests that many balance_rr links feeding into
a single uplink will not experience packet reordering.
More up-to-date tests, with 1G links feeding into a switch with a 10G
uplink, using a 2.6.23-rc8 kernel on the system on which the 1G links
were bonded with balance_rr (mode=0) shows that even a many to one
link configuration will experience packet reordering and the attendant
TCP issues involving spurrious retransmissions and the congestion
window. This happens even with a single, simple bulk transfer such as
a netperf TCP_STREAM test. A more complete description of the tests
and results, including tcptrace analysis of packet traces showing the
degree of reordering and such can be found at:
http://marc.info/?l=linux-netdev&m=119101513406349&w=2
Also, note that some switches use the term "trunking" in a context
other than link aggregation.
Signed-off-by: Rick Jones <rick.jones2@hp.com>
---
diff -r 35e54d4beaad Documentation/networking/bonding.txt
--- a/Documentation/networking/bonding.txt Wed Oct 24 05:06:40 2007 +0000
+++ b/Documentation/networking/bonding.txt Mon Oct 29 03:47:19 2007 -0700
@@ -1696,23 +1696,6 @@ balance-rr: This mode is the only mode t
interface's worth of throughput, even after adjusting
tcp_reordering.
- Note that this out of order delivery occurs when both the
- sending and receiving systems are utilizing a multiple
- interface bond. Consider a configuration in which a
- balance-rr bond feeds into a single higher capacity network
- channel (e.g., multiple 100Mb/sec ethernets feeding a single
- gigabit ethernet via an etherchannel capable switch). In this
- configuration, traffic sent from the multiple 100Mb devices to
- a destination connected to the gigabit device will not see
- packets out of order. However, traffic sent from the gigabit
- device to the multiple 100Mb devices may or may not see
- traffic out of order, depending upon the balance policy of the
- switch. Many switches do not support any modes that stripe
- traffic (instead choosing a port based upon IP or MAC level
- addresses); for those devices, traffic flowing from the
- gigabit device to the many 100Mb devices will only utilize one
- interface.
-
If you are utilizing protocols other than TCP/IP, UDP for
example, and your application can tolerate out of order
delivery, then this mode can allow for single stream datagram
@@ -1720,7 +1703,9 @@ balance-rr: This mode is the only mode t
to the bond.
This mode requires the switch to have the appropriate ports
- configured for "etherchannel" or "trunking."
+ configured for "etherchannel" or "aggregation." N.B. some
+ switches might use the term "trunking" for something other
+ than link aggregation.
active-backup: There is not much advantage in this network topology to
the active-backup mode, as the inactive backup devices are all
^ permalink raw reply
* Re: [PATCH] ixgb: fix TX hangs under heavy load
From: Andy Gospodarek @ 2007-10-30 19:28 UTC (permalink / raw)
To: Auke Kok; +Cc: netdev, jesse.brandeburg
In-Reply-To: <20071030182150.25962.48103.stgit@localhost.localdomain>
On Tue, Oct 30, 2007 at 11:21:50AM -0700, Auke Kok wrote:
> A merge error occurred where we merged the wrong block here
> in version 1.0.120. The right condition for frags is slightly
> different then for the skb, so account for the difference properly
> and trim the TSO based size right.
>
> Originally part of a fix reported by IBM to fix TSO hangs on
> pSeries hardware.
>
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
> Cc: Andy Gospodarek <andy@greyhouse.net>
> ---
>
Thanks, Auke and Jesse!
^ permalink raw reply
* Re: [PATCH 2/2] NFS: handle IPv6 addresses in nfs ctl
From: Brian Haley @ 2007-10-30 19:08 UTC (permalink / raw)
To: Aurélien Charbon; +Cc: Neil Brown, Mailing list NFSv4, netdev ML
In-Reply-To: <4727647A.30706@ext.bull.net>
Aurélien Charbon wrote:
> Here is a second missing part of the IPv6 support in NFS server code
> concerning knfd syscall interface.
> It updates write_getfd and write_getfd to accept IPv6 addresses.
>
> Applies on a kernel including ip_map cache modifications
Both patches still have bugs, I think the patch I sent yesterday fixed
them all, so I would recommend using that instead. Of course Neil's
comment possibly trumps all that anyways...
-Brian
^ 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