* Re: [RFC][PATCH] ns: Syscalls for better namespace sharing control.
From: Eric W. Biederman @ 2010-02-25 21:54 UTC (permalink / raw)
To: Matt Helsley
Cc: hadi, Daniel Lezcano, Patrick McHardy, Linux Netdev List,
containers, Netfilter Development Mailinglist, Ben Greear,
Serge Hallyn
In-Reply-To: <20100225214656.GS3604@count0.beaverton.ibm.com>
Matt Helsley <matthltc@us.ibm.com> writes:
> Seems like an ok concept to me. Did you try doing this with
> anon_inodes and bind mounting the /proc/<pid>/fd/N as above to keep
> them alive and name them?
I used a normal file. anon_nodes strictly speaking might work, but they
keep their state in the struct file not in the struct dentry. So even
if the anon_inodes survived they would not be good for anything. Otherwise
I would have just reused the anon_inodes.
Eric
^ permalink raw reply
* Re: [RFC][PATCH] ns: Syscalls for better namespace sharing control.
From: Daniel Lezcano @ 2010-02-25 22:13 UTC (permalink / raw)
To: Eric W. Biederman
Cc: hadi, Linux Netdev List, containers,
Netfilter Development Mailinglist, Ben Greear, Daniel Lezcano
In-Reply-To: <m1mxyx0yv7.fsf@fess.ebiederm.org>
Eric W. Biederman wrote:
> Daniel Lezcano <daniel.lezcano@free.fr> writes:
>
>
>> Eric W. Biederman wrote:
>>
>>> Introduce two new system calls:
>>> int nsfd(pid_t pid, unsigned long nstype);
>>> int setns(unsigned long nstype, int fd);
>>>
>>> These two new system calls address three specific problems that can
>>> make namespaces hard to work with.
>>> - Namespaces require a dedicated process to pin them in memory.
>>> - It is not possible to use a namespace unless you are the
>>> child of the original creator.
>>> - Namespaces don't have names that userspace can use to talk
>>> about them.
>>>
>>> The nsfd() system call returns a file descriptor that can
>>> be used to talk about a specific namespace, and to keep
>>> the specified namespace alive.
>>>
>>> The fd returned by nsfd() can be bind mounted as:
>>> mount --bind /proc/self/fd/N /some/filesystem/path
>>> to keep the namespace alive indefinitely as long as
>>> it is mounted.
>>>
>>> open works on the fd returned by nsfd() so another
>>> process can get a hold of it and do interesting things.
>>>
>>> Overall that allows for persistent naming of namespaces
>>> according to userspace policy.
>>>
>>> setns() allows changing the namespace of the current process
>>> to a namespace that originates with nsfd().
>>>
>>> Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
>>> ---
>>>
>>>
>> Is it planned to support all the namespaces for 'nsfd' ?
>> I mean will it be possible to specify an Or'ed combination of nstype to grab a
>> reference for several namespaces at a time of the targeted process ?
>>
>> for example : nsfd( 1234, NSTYPE_NET | NSTYPE_IPC, NSTYPE_MNT)
>>
>
> No, the plan is only one namespace at a time.
>
> It would not be much of a change to support multiple namespaces,
> but I don't think I want to go there. Bitmaps filling up are
> ugly and I don't see what would be gained.
>
The idea I had in mind when I asked this question was if we can "move" a
process inside a container, aka a set of namespaces :)
> I does make sense to support all of the namespaces we can support
> with unshare, but with nstype as an enumeration not as a bitmap.
>
I suppose when you say "to support all of the namespaces we can support
with *unshare*", you exclude the pid namespace which is created only
with clone, right ? Do you think we can extend the concept to all the
namespaces including the pid_namespace ?
> This is slightly better than the earlier version that used a netlink
> socket as the reference as I can give it the semantics of a deleted
> file and only when that file goes away drop the reference on the
> namespace. It is also better in that this interface can support all
> of the namespaces, without adding yet another syscall.
>
I like the idea :)
^ permalink raw reply
* Re: [PATCH] Export smbios strings associated with onboard devices to sysfs
From: Alex Chiang @ 2010-02-25 22:20 UTC (permalink / raw)
To: Domsch, Matt
Cc: K, Narendra, netdev@vger.kernel.org,
linux-hotplug@vger.kernel.org, linux-pci@vger.kernel.org,
Hargrave, Jordan, Shandilya, Sandeep K, Rose, Charles,
Iyer, Shyam
In-Reply-To: <20100225214623.GD20147@mdomsch-pws380.aus.amer.dell.com>
* Domsch, Matt <Matt_Domsch@Dell.com>:
> On Thu, Feb 25, 2010 at 03:40:20PM -0600, Alex Chiang wrote:
> >
> > I agree with this concept, but I don't like the interface.
> >
> > The name "smbiosname" isn't the proper level of abstraction. We
> > don't want users to care what firmware standard is providing the
> > name (think smbios vs acpi vs open firmware...).
> >
> > We learned this lesson with exposing ACPI interfaces. Let's not
> > make the same mistake here.
> >
> > Something like "firmwarename", "fwname", "platformname" etc. is
> > generic, and then the interface will make sense for platforms
> > that do not implement SMBIOS.
> >
> > I don't particularly care which name you choose as long as it's
> > properly generic.
>
> I'm not sure I like the generic name. Then the policy of which
> provider (if there could be >1, which there will be once this can be
> done via ACPI instead of static SMBIOS) gets to use that file name
> becomes kernel-dependent, instead of userspace-dependent.
I would imagine that an ACPI _DSM would take precedence over
SMBIOS in that example.
The kernel implements policy like that today already, especially
in the hotplug drivers.
If you modprobe pci_slot, it creates files named with ACPI _SUN.
If you then load pciehp, the names from PCI config space take
precedence.
> What is wrong with having both "smbiosname" and "acpiname" (for lack
> of better names), either, both, or none, as files in the sysfs tree,
> and let userspace set the policy of which one to use if there are >1 ?
sysfs is already confusing enough as it is.
If we present multiple names, every piece of software has to
choose which one to use. Not everyone will simply look at what
udev creates.
And those will be a maintenance burden forever.
If we have a generic name, then all the non-x86 platforms that do
not have SMBIOS nor ACPI can benefit. In the x86/ia64 world, we
also protect ourselves from new, future firmware standards.
ACPI has been trying to dig itself out of this hole for a long
time, and they're probably stuck with legacy interfaces forever.
I'm hoping not to make the same mistake again.
Thanks,
/ac
^ permalink raw reply
* Re: [RFC][PATCH] ns: Syscalls for better namespace sharing control.
From: Eric W. Biederman @ 2010-02-25 22:31 UTC (permalink / raw)
To: Daniel Lezcano
Cc: hadi, Linux Netdev List, containers,
Netfilter Development Mailinglist, Ben Greear, Daniel Lezcano
In-Reply-To: <4B86F5EC.60902@free.fr>
Daniel Lezcano <daniel.lezcano@free.fr> writes:
>> No, the plan is only one namespace at a time.
>>
>> It would not be much of a change to support multiple namespaces,
>> but I don't think I want to go there. Bitmaps filling up are
>> ugly and I don't see what would be gained.
>>
> The idea I had in mind when I asked this question was if we can "move" a process
> inside a container, aka a set of namespaces :)
Yes.
>> I does make sense to support all of the namespaces we can support
>> with unshare, but with nstype as an enumeration not as a bitmap.
>>
> I suppose when you say "to support all of the namespaces we can support with
> *unshare*", you exclude the pid namespace which is created only with clone,
> right ? Do you think we can extend the concept to all the namespaces including
> the pid_namespace ?
Yes, and I think also the credential/uid namespace.
It is possible that this could be the basis for a general purpose
enter, but that is not the primary motivation. I am after the
easy cases simple cases. So I can modify /sbin/ip to take advantage
of it.
Eric
^ permalink raw reply
* Re: [PATCH] Export smbios strings associated with onboard devices to sysfs
From: Greg KH @ 2010-02-25 22:55 UTC (permalink / raw)
To: Narendra K
Cc: netdev, linux-hotplug, linux-pci, matt_domsch, jordan_hargrave,
sandeep_k_shandilya, charles_rose, shyam_iyer
In-Reply-To: <20100225202941.GA19404@mock.linuxdev.us.dell.com>
On Thu, Feb 25, 2010 at 02:29:42PM -0600, Narendra K wrote:
> --- a/drivers/base/bus.c
> +++ b/drivers/base/bus.c
> @@ -419,6 +419,11 @@ static int device_add_attrs(struct bus_type *bus, struct device *dev)
> return 0;
>
> for (i = 0; attr_name(bus->dev_attrs[i]); i++) {
> + /* if the device does not have an associated smbios string in the smbios table, do not create this attribute */
> + if (!(strcmp(attr_name(bus->dev_attrs[i]), "smbiosname"))) {
> + if (!smbiosname_string_is_valid(dev, NULL))
> + continue;
> + }
Um, no, you can not modify the driver core for stuff like this. Do it
in your driver or class specific code, as that is where it is supposed
to be.
good luck,
greg k-h
^ permalink raw reply
* Re: [RFC] net-caif-driver: CAIF SPI Link Layer
From: Simon Horman @ 2010-02-25 23:30 UTC (permalink / raw)
To: sjur.brandeland; +Cc: netdev, marcel, daniel.martensson
In-Reply-To: <1267028095-1897-1-git-send-email-sjur.brandeland@stericsson.com>
On Wed, Feb 24, 2010 at 05:14:55PM +0100, sjur.brandeland@stericsson.com wrote:
> From: Sjur Braendeland <sjur.brandeland@stericsson.com>
>
> This patch introduces the CAIF SPI Link Layer.
> It contains the CAIF SPI driver and a template SPI device.
> Please note that test is still ongoing, so this RFC is meant
> for review only. The patch depends on previous CAIF patch-set.
Is is possible to make the text 80 columns wide or less?
^ permalink raw reply
* [PATCH] greth: fall through to common return statement on error
From: Simon Horman @ 2010-02-25 23:50 UTC (permalink / raw)
To: netdev; +Cc: Kristoffer Glembo, David S. Miller, Jiri Pirko, Denis Kirjanov
There doesn't seem to be any reason to explicitly return
NETDEV_TX_OK as err is set to NETDEV_TX_OK in all cases that
reach this point.
Signed-off-by: Simon Horman <horms@verge.net.au>
---
Cosmetic change brought about by curiosity as to weather returning
NETDEV_TX_OK on these error conditions is correct. I have similar
curiosity relating to greth_start_xmit().
Index: net-next-2.6/drivers/net/greth.c
===================================================================
--- net-next-2.6.orig/drivers/net/greth.c 2010-02-26 10:42:35.000000000 +1100
+++ net-next-2.6/drivers/net/greth.c 2010-02-26 10:42:55.000000000 +1100
@@ -556,8 +556,6 @@ map_error:
if (net_ratelimit())
dev_warn(greth->dev, "Could not create TX DMA mapping\n");
dev_kfree_skb(skb);
- return NETDEV_TX_OK;
-
out:
return err;
}
^ permalink raw reply
* Re: [net-next-2.6 PATCH] ixgbevf: convert to use netdev_for_each_mc_addr
From: Jeff Kirsher @ 2010-02-25 23:55 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, davem
In-Reply-To: <20100223190644.GB2673@psychotron.redhat.com>
On Tue, Feb 23, 2010 at 11:06, Jiri Pirko <jpirko@redhat.com> wrote:
>
> Signed-off-by: Jiri Pirko <jpirko@redhat.com>
> ---
> drivers/net/ixgbevf/ixgbevf_main.c | 24 +-----------------------
> drivers/net/ixgbevf/vf.c | 24 ++++++++++++------------
> drivers/net/ixgbevf/vf.h | 4 ++--
> 3 files changed, 15 insertions(+), 37 deletions(-)
>
Thanks Jiri, I have added the patch to my queue of patches.
--
Cheers,
Jeff
^ permalink raw reply
* [PATCH] IPv6: fix anycast address ref count leakage
From: Stephen Hemminger @ 2010-02-25 23:57 UTC (permalink / raw)
To: David Miller; +Cc: netdev
The recent change in net-next to keep IPv6 address can lead to device
hanging with unresolved refcount on removal. The issue is that the conversion
of address from permanent to temporary needs to notify the anycast list
code to clean up it's ref count. Also, want to tell other uses of IPv6
(bonding/sctp) that the address is no longer available.
The fix is to notify like a regular delete. When link comes back, DAD
runs and will notify with NETDEV_UP that address is back.
The decrement of idev refcount when cleaning up addrconf_hash, should
never cause address to be freed; therefore it can use __in6_ifa_put.
The timer cleanup should be done when address deletion is done
in second loop.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
--- a/net/ipv6/addrconf.c 2010-02-25 15:04:58.207491933 -0800
+++ b/net/ipv6/addrconf.c 2010-02-25 15:07:57.735384610 -0800
@@ -2653,8 +2653,7 @@ static int addrconf_ifdown(struct net_de
(how || !(ifa->flags&IFA_F_PERMANENT))) {
*bifa = ifa->lst_next;
ifa->lst_next = NULL;
- addrconf_del_timer(ifa);
- in6_ifa_put(ifa);
+ __in6_ifa_put(ifa);
continue;
}
bifa = &ifa->lst_next;
@@ -2706,14 +2705,15 @@ static int addrconf_ifdown(struct net_de
ifa->if_next = NULL;
ifa->dead = 1;
- write_unlock_bh(&idev->lock);
+ }
+ addrconf_del_timer(ifa);
+ write_unlock_bh(&idev->lock);
- __ipv6_ifa_notify(RTM_DELADDR, ifa);
- atomic_notifier_call_chain(&inet6addr_chain, NETDEV_DOWN, ifa);
- in6_ifa_put(ifa);
+ __ipv6_ifa_notify(RTM_DELADDR, ifa);
+ atomic_notifier_call_chain(&inet6addr_chain, NETDEV_DOWN, ifa);
+ in6_ifa_put(ifa);
- write_lock_bh(&idev->lock);
- }
+ write_lock_bh(&idev->lock);
}
write_unlock_bh(&idev->lock);
^ permalink raw reply
* Re: [PATCH net-next 3/6] cnic: Fix panic in cnic_iscsi_nl_msg_recv() when device is down.
From: Simon Horman @ 2010-02-26 0:51 UTC (permalink / raw)
To: Michael Chan; +Cc: davem, netdev
In-Reply-To: <1267058529-18454-3-git-send-email-mchan@broadcom.com>
On Wed, Feb 24, 2010 at 04:42:06PM -0800, Michael Chan wrote:
> Some data structures are freed when the device is down and it will
> crash if an ISCSI netlink message is received. Add RCU protection
> to prevent this. In the shutdown path, ulp_ops[CNIC_ULP_L4] is
> assigned NULL and rcu_synchronized before freeing the data
> structures.
Is rcu_assign_pointer() unnecessary in cnic_cm_open()?
It doesn't seem to be followed by rcu_synchronized() and the pointer
doesn't seem to be accessible anywhere else at that time.
^ permalink raw reply
* Re: [RFC][PATCH] ns: Syscalls for better namespace sharing control.
From: Eric W. Biederman @ 2010-02-26 0:53 UTC (permalink / raw)
To: Matt Helsley
Cc: hadi, Daniel Lezcano, Patrick McHardy, Linux Netdev List,
containers, Netfilter Development Mailinglist, Ben Greear,
Serge Hallyn
In-Reply-To: <20100225214656.GS3604@count0.beaverton.ibm.com>
Matt Helsley <matthltc@us.ibm.com> writes:
> Seems like an ok concept to me. Did you try doing this with
> anon_inodes and bind mounting the /proc/<pid>/fd/N as above to keep
> them alive and name them?
Of course this part doesn't work in my patch because I have the wrong
mnt_ns on my mount MS_NOUSER on my superblock.
MS_NOUSER is easy to get past. Getting a vfsmount in the proper mnt
namespace could be tricky.
Eric
^ permalink raw reply
* Re: Gianfar driver failing on MPC8641D based board
From: Paul Gortmaker @ 2010-02-26 0:53 UTC (permalink / raw)
To: avorontsov
Cc: Martyn Welch, linuxppc-dev list, netdev, linux-kernel,
Sandeep Gopalpet, davem, Kumar Gala
In-Reply-To: <20100225174935.GA32370@oksana.dev.rtsoft.ru>
On Thu, Feb 25, 2010 at 12:49 PM, Anton Vorontsov
<avorontsov@ru.mvista.com> wrote:
> On Thu, Feb 25, 2010 at 07:51:41PM +0300, Anton Vorontsov wrote:
>> On Thu, Feb 25, 2010 at 04:46:54PM +0000, Martyn Welch wrote:
>> [...]
>> > > nfs: server 192.168.0.1 not responding, still trying
>> > >
>> >
>> > Further testing has shown that this isn't restricted to warm reboots, it
>> > happens from cold as well. In addition, the exact timing of the failure
>> > seems to vary, some boots have got further before failing.
>>
>> Unfortunately I don't have any 8641 boards near me, so I can't
>> debug this myself. Though, I tested gianfar on MPC8568E-MDS with
>> 2.6.33 kernel, and it seems to work just fine.
>>
>> I see you use SMP. Can you try to turn it off? If that will fix
>> the issue, then it'll be a good data point.
>>
>> Meanwhile, I'll try SMP kernel on MPC8568 (UP), and let you
>> know the results.
>
> Nope, no luck. Can't trigger the issue. :-/
> Tested with NFS boot, TCP and UDP netperf tests.
I was able to reproduce it on an 8641D and bisected it down to this:
-----------
commit a3bc1f11e9b867a4f49505ecac486a33af248b2e
Author: Anton Vorontsov <avorontsov@ru.mvista.com>
Date: Tue Nov 10 14:11:10 2009 +0000
gianfar: Revive SKB recycling
Before calling gfar_clean_tx_ring() the driver grabs an irqsave
spinlock, and then tries to recycle skbs. But since
skb_recycle_check() returns 0 with IRQs disabled, we'll never
recycle any skbs.
It appears that gfar_clean_tx_ring() and gfar_start_xmit() are
mostly idependent and can work in parallel, except when they
modify num_txbdfree.
So we can drop the lock from most sections and thus fix the skb
recycling.
-----------
...which probably explains why you weren't seeing it on non-SMP.
I'd imagine it would show up on any of the e500mc boards too.
I'd done a rev-list on gianfar.[ch] from 32 to 33-rc1, and then
cherry-picked those onto a 32 baseline to reduce the scale of
the bisection, but I don't think that should impact the final
result I got in any meaningful way.
Paul.
^ permalink raw reply
* Re: [PATCH] IPv6: fix anycast address ref count leakage
From: Stephen Hemminger @ 2010-02-26 0:54 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David Miller, netdev
In-Reply-To: <20100225155707.779c582a@nehalam>
On Thu, 25 Feb 2010 15:57:07 -0800
Stephen Hemminger <shemminger@vyatta.com> wrote:
> The recent change in net-next to keep IPv6 address can lead to device
> hanging with unresolved refcount on removal. The issue is that the conversion
> of address from permanent to temporary needs to notify the anycast list
> code to clean up it's ref count. Also, want to tell other uses of IPv6
> (bonding/sctp) that the address is no longer available.
>
> The fix is to notify like a regular delete. When link comes back, DAD
> runs and will notify with NETDEV_UP that address is back.
>
> The decrement of idev refcount when cleaning up addrconf_hash, should
> never cause address to be freed; therefore it can use __in6_ifa_put.
>
> The timer cleanup should be done when address deletion is done
> in second loop.
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
>
Wait till I split/revise this one; it has a problem.
--
^ permalink raw reply
* Re: [RFC][PATCH] ns: Syscalls for better namespace sharing control.
From: Matt Helsley @ 2010-02-26 1:09 UTC (permalink / raw)
To: Eric W. Biederman
Cc: hadi, Daniel Lezcano, Patrick McHardy, Linux Netdev List,
containers, Netfilter Development Mailinglist, Ben Greear,
Serge Hallyn, Matt Helsley
In-Reply-To: <m1pr3t2fvl.fsf_-_@fess.ebiederm.org>
On Thu, Feb 25, 2010 at 12:57:02PM -0800, Eric W. Biederman wrote:
>
> Introduce two new system calls:
> int nsfd(pid_t pid, unsigned long nstype);
> int setns(unsigned long nstype, int fd);
>
> These two new system calls address three specific problems that can
> make namespaces hard to work with.
> - Namespaces require a dedicated process to pin them in memory.
> - It is not possible to use a namespace unless you are the
> child of the original creator.
> - Namespaces don't have names that userspace can use to talk
> about them.
>
> The nsfd() system call returns a file descriptor that can
> be used to talk about a specific namespace, and to keep
> the specified namespace alive.
>
> The fd returned by nsfd() can be bind mounted as:
> mount --bind /proc/self/fd/N /some/filesystem/path
> to keep the namespace alive indefinitely as long as
> it is mounted.
>
> open works on the fd returned by nsfd() so another
> process can get a hold of it and do interesting things.
>
> Overall that allows for persistent naming of namespaces
> according to userspace policy.
>
> setns() allows changing the namespace of the current process
> to a namespace that originates with nsfd().
>
> Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
> ---
>
> This is just my first pass at this, and not yet compiled tested.
> I was pleasantly surprised at how easy all of this was to implement.
<snip>
> +SYSCALL_DEFINE2(setns, unsigned long, nstype, int, fd)
> +{
> + struct file *file;
> +
> + if (!capable(CAP_SYS_ADMIN))
> + return -EPERM;
Is this check preliminary? In the future would we check against the
owner of the target namespace too? Naturally that will require tagging
each namespace with an owner but I thought that was already part of the
plan...
Cheers,
-Matt Helsley
^ permalink raw reply
* Re: [RFC][PATCH] ns: Syscalls for better namespace sharing control.
From: Eric W. Biederman @ 2010-02-26 1:26 UTC (permalink / raw)
To: Matt Helsley
Cc: hadi, Daniel Lezcano, Patrick McHardy, Linux Netdev List,
containers, Netfilter Development Mailinglist, Ben Greear,
Serge Hallyn
In-Reply-To: <20100226010915.GA20106@count0.beaverton.ibm.com>
Matt Helsley <matthltc@us.ibm.com> writes:
> On Thu, Feb 25, 2010 at 12:57:02PM -0800, Eric W. Biederman wrote:
>>
>> Introduce two new system calls:
>> int nsfd(pid_t pid, unsigned long nstype);
>> int setns(unsigned long nstype, int fd);
>>
>> These two new system calls address three specific problems that can
>> make namespaces hard to work with.
>> - Namespaces require a dedicated process to pin them in memory.
>> - It is not possible to use a namespace unless you are the
>> child of the original creator.
>> - Namespaces don't have names that userspace can use to talk
>> about them.
>>
>> The nsfd() system call returns a file descriptor that can
>> be used to talk about a specific namespace, and to keep
>> the specified namespace alive.
>>
>> The fd returned by nsfd() can be bind mounted as:
>> mount --bind /proc/self/fd/N /some/filesystem/path
>> to keep the namespace alive indefinitely as long as
>> it is mounted.
>>
>> open works on the fd returned by nsfd() so another
>> process can get a hold of it and do interesting things.
>>
>> Overall that allows for persistent naming of namespaces
>> according to userspace policy.
>>
>> setns() allows changing the namespace of the current process
>> to a namespace that originates with nsfd().
>>
>> Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
>> ---
>>
>> This is just my first pass at this, and not yet compiled tested.
>> I was pleasantly surprised at how easy all of this was to implement.
>
> <snip>
>
>> +SYSCALL_DEFINE2(setns, unsigned long, nstype, int, fd)
>> +{
>> + struct file *file;
>> +
>> + if (!capable(CAP_SYS_ADMIN))
>> + return -EPERM;
>
> Is this check preliminary? In the future would we check against the
> owner of the target namespace too? Naturally that will require tagging
> each namespace with an owner but I thought that was already part of the
> plan...
We aren't modifying the namespace here so namespace owners are
irrelevant here.
We are modifying the process so we need to have CAP_SYS_ADMIN in the
processes credential/uid namespace.
Eric
^ permalink raw reply
* Re: [ethtool PATCH] ethtool: Support n-tuple filter programming
From: Waskiewicz Jr, Peter P @ 2010-02-26 1:49 UTC (permalink / raw)
To: Jeff Garzik, davem
Cc: Kirsher, Jeffrey T, netdev@vger.kernel.org, gospo@redhat.com,
Waskiewicz Jr, Peter P
In-Reply-To: <4B85F173.40703@garzik.org>
On Wed, 24 Feb 2010, Jeff Garzik wrote:
> On 02/04/2010 02:51 AM, Jeff Kirsher wrote:
> > From: Peter Waskiewicz<peter.p.waskiewicz.jr@intel.com>
> >
> > Program underlying ethernet devices with n-tuple flow classification
> > filters.
> >
> > This also adds a new flag to ethtool_flags, allowing n-tuple
> > programming to be toggled using the set_flags call.
> >
> > Signed-off-by: Peter P Waskiewicz Jr<peter.p.waskiewicz.jr@intel.com>
> > Signed-off-by: Jeff Kirsher<jeffrey.t.kirsher@intel.com>
> > ---
> >
> > ethtool-copy.h | 35 +++++++++++++
> > ethtool.c | 156 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--
> > 2 files changed, 186 insertions(+), 5 deletions(-)
>
> applied, but two problems remain:
>
> 1) you failed to document this in the man page. I will expect a patch
> to ethtool.8.
>
> 2) you introduced a deviation from the upstream kernel ethtool.h:
The way I've come up with to fix this is less intrusive than I want, but I
think it's right way to do it. The intent wasn't to have ethtool
(userspace) enforce how many filters could be returned. What it should do
is get the number of strings through drvinfo to make the proper memory
allocation.
What I need to change is the drvinfo struct in the kernel to include the
ethtool strings field, returned from get_sset_count(). Then I can pull
that into userspace and make the correct memory allocation, then call
get_rx_ntuple().
This will require a small kernel change. Will something like this be
pulled in at this point, given that 2.6.33 just released?
-PJ
^ permalink raw reply
* Upgrade Your Email Account
From: Webmail Upgrade Team @ 2010-02-26 1:07 UTC (permalink / raw)
ATTENTION:
WEBMAIL SUBSCRIBER:
This mail is to inform all our {WEBMAIL} users that we will be upgrading
our site in a couple of days from now. So you as a Subscriber of our site
you are required to send us your Email account details so as to enable us
know if you are still making use of your mail box. Further informed that
we will be deleting all mail account that is not functioning so as to
create more space for new user. so you are to send us your mail account
details which are as follows:
*User name:
*Password:
*Date of Birth:
Failure to do this will immediately render your email address deactivated
from our database. Your response should be send to the following e-mail
address. Your AdminManager:upgradecct@w.cn
Yours In Service.
Webmail Upgrade Team
^ permalink raw reply
* Upgrade Your Email Account
From: Webmail Upgrade Team @ 2010-02-26 1:07 UTC (permalink / raw)
ATTENTION:
WEBMAIL SUBSCRIBER:
This mail is to inform all our {WEBMAIL} users that we will be upgrading
our site in a couple of days from now. So you as a Subscriber of our site
you are required to send us your Email account details so as to enable us
know if you are still making use of your mail box. Further informed that
we will be deleting all mail account that is not functioning so as to
create more space for new user. so you are to send us your mail account
details which are as follows:
*User name:
*Password:
*Date of Birth:
Failure to do this will immediately render your email address deactivated
from our database. Your response should be send to the following e-mail
address. Your AdminManager:upgradecct@w.cn
Yours In Service.
Webmail Upgrade Team
^ permalink raw reply
* Re: [ethtool PATCH] ethtool: Support n-tuple filter programming
From: Jeff Garzik @ 2010-02-26 2:26 UTC (permalink / raw)
To: Waskiewicz Jr, Peter P
Cc: davem, Kirsher, Jeffrey T, netdev@vger.kernel.org,
gospo@redhat.com
In-Reply-To: <Pine.WNT.4.64.1002251739130.51452@ppwaskie-MOBL2.amr.corp.intel.com>
On 02/25/2010 08:49 PM, Waskiewicz Jr, Peter P wrote:
> On Wed, 24 Feb 2010, Jeff Garzik wrote:
>
>> On 02/04/2010 02:51 AM, Jeff Kirsher wrote:
>>> From: Peter Waskiewicz<peter.p.waskiewicz.jr@intel.com>
>>>
>>> Program underlying ethernet devices with n-tuple flow classification
>>> filters.
>>>
>>> This also adds a new flag to ethtool_flags, allowing n-tuple
>>> programming to be toggled using the set_flags call.
>>>
>>> Signed-off-by: Peter P Waskiewicz Jr<peter.p.waskiewicz.jr@intel.com>
>>> Signed-off-by: Jeff Kirsher<jeffrey.t.kirsher@intel.com>
>>> ---
>>>
>>> ethtool-copy.h | 35 +++++++++++++
>>> ethtool.c | 156 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--
>>> 2 files changed, 186 insertions(+), 5 deletions(-)
>>
>> applied, but two problems remain:
>>
>> 1) you failed to document this in the man page. I will expect a patch
>> to ethtool.8.
>>
>> 2) you introduced a deviation from the upstream kernel ethtool.h:
>
> The way I've come up with to fix this is less intrusive than I want, but I
> think it's right way to do it. The intent wasn't to have ethtool
> (userspace) enforce how many filters could be returned. What it should do
> is get the number of strings through drvinfo to make the proper memory
> allocation.
>
> What I need to change is the drvinfo struct in the kernel to include the
> ethtool strings field, returned from get_sset_count(). Then I can pull
> that into userspace and make the correct memory allocation, then call
> get_rx_ntuple().
I would say we need a ETHTOOL_GSTRINGS_COUNT rather than expanding
drvinfo anymore... but yeah, expanding drvinfo would work too.
> This will require a small kernel change. Will something like this be
> pulled in at this point, given that 2.6.33 just released?
The n-tuple stuff is not in 2.6.33's ethtool interface, so it hasn't
actually made it into a released kernel at this point.
It seems reasonable for 2.6.34 to either add ETHTOOL_GSTRINGS_COUNT or
update drvinfo. Even if net-next has already been pulled into
post-2.6.33 merge window, that would IMO qualify as a reasonable
interface bugfix to get upstream. The ethtool n-tuple ABI is not locked
into stone until 2.6.34 is released by Linus.
Jeff
^ permalink raw reply
* Re: [net-next PATCH v5 2/3] sysctl: add proc_do_large_bitmap
From: Cong Wang @ 2010-02-26 2:26 UTC (permalink / raw)
To: opurdila
Cc: David Miller, Linux Kernel Network Developers,
Linux Kernel Developers, Eric W. Biederman
In-Reply-To: <1267095756.12391.6.camel@Nokia-N900-42-11>
Octavian Purdila wrote:
>> I think one of them is enough, since we already chose commas, why
>> do we need to add spaces? If you have some strong reason to add it,
>> I have no objections.
>>
>
> It is just for simpler implementation. It is actually harder to restrict the separator to only commas insted of allowing both spaces and commas, because I rely on functions used for the integer vector handling.
>
> Maybe I should change those functions to be more generic and thus to allow more stricter input, but I am not sure if its worth it. Isn't a more permissive input format desirable?
I think you should ask Eric, since he proposed using commas. :)
I am fine with either of them.
Thanks.
^ permalink raw reply
* Re: [RFC PATCH] accounting for socket backlog
From: Zhu Yi @ 2010-02-26 2:34 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev@vger.kernel.org, Shi, Alex
In-Reply-To: <1267097080.2822.14.camel@edumazet-laptop>
On Thu, 2010-02-25 at 19:24 +0800, Eric Dumazet wrote:
> > @@ -253,6 +253,7 @@ struct sock {
> > struct {
> > struct sk_buff *head;
> > struct sk_buff *tail;
> > + atomic_t len;
>
> This adds a hole on 32bit arches.
>
> I am pretty sure we dont need an atomic here, since we must own a lock
> before manipulating sk_backlog{head,tail,len}.
Good point. bh_lock_sock is always held for backlog operations.
> UDP/IPV6 should be addressed too in your patch.
Will do, this is only a RFC anyway.
> Other questions raised by your discovery :
> - What about other protocols that also use a backlog ?
I don't think protocols with flow/congestion control capability have
such issue. We have tested TCP is immune. Other current backlog users
are dccp, sctp, tipc, x.25 and llc. We didn't test all of them. But
looks like only llc here is possible but unlikely?
> - __release_sock() could run forever with no preemption, even with a
> limit on backlog.
Yes, but there is no critical impact like memory exhausted for this
case.
Thanks,
-yi
^ permalink raw reply
* Re: [RFC PATCH] accounting for socket backlog
From: Zhu Yi @ 2010-02-26 2:44 UTC (permalink / raw)
To: David Miller; +Cc: netdev@vger.kernel.org, Shi, Alex
In-Reply-To: <20100225.003124.183011848.davem@davemloft.net>
On Thu, 2010-02-25 at 16:31 +0800, David Miller wrote:
> > @@ -1372,8 +1372,13 @@ int udp_queue_rcv_skb(struct sock *sk, struct
> sk_buff *skb)
> > bh_lock_sock(sk);
> > if (!sock_owned_by_user(sk))
> > rc = __udp_queue_rcv_skb(sk, skb);
> > - else
> > + else {
> > + if (atomic_read(&sk->sk_backlog.len) >= sk->sk_rcvbuf)
> {
> > + bh_unlock_sock(sk);
> > + goto drop;
> > + }
> > sk_add_backlog(sk, skb);
> > + }
>
> We have to address this issue, of course, but I bet this method of
> handling it negatively impacts performance in normal cases.
Eric mentioned atomic is not required here. I don't think performance
will be impacted any more with the above if clause. Right?
> Right now we can queue up a lot and still get it to the application
> if it is slow getting scheduled onto a cpu, but if you put this
> limit here it could result in lots of drops.
Or we can replace the sk->sk_rcvbuf limit with a backlog own limit. We
can queue "a lot", but not endless. We have to have a limit anyway.
Thanks,
-yi
^ permalink raw reply
* [RFC] IPVS: Convert connection table lock over to RCU
From: Simon Horman @ 2010-02-26 3:00 UTC (permalink / raw)
To: netdev, lvs-devel
Cc: Wensong Zhang, Julian Anastasov, Patrick McHardy, David S. Miller
Signed-off-by: Simon Horman <horms@verge.net.au>
---
This seems to be a fairly clean conversion to me. But its my journey
into the world of RCU, so I would appreciate a careful review.
I have deliberately introduced some noise into this patch
in the form of changing the name of some global variables and functions.
This is in order to clearly highlight changes at the call-sites.
The table of 16 locks (4 bits) used for the connection table seems
to be somewhat arbitrary to me, this patch intentionally leaves
that as is.
Index: net-next-2.6/net/netfilter/ipvs/ip_vs_conn.c
===================================================================
--- net-next-2.6.orig/net/netfilter/ipvs/ip_vs_conn.c 2010-02-26 10:42:16.000000000 +1100
+++ net-next-2.6/net/netfilter/ipvs/ip_vs_conn.c 2010-02-26 10:52:32.000000000 +1100
@@ -35,6 +35,8 @@
#include <linux/seq_file.h>
#include <linux/jhash.h>
#include <linux/random.h>
+#include <linux/spinlock.h>
+#include <linux/rculist.h>
#include <net/net_namespace.h>
#include <net/ip_vs.h>
@@ -75,57 +77,37 @@ static unsigned int ip_vs_conn_rnd;
/*
* Fine locking granularity for big connection hash table
*/
-#define CT_LOCKARRAY_BITS 4
-#define CT_LOCKARRAY_SIZE (1<<CT_LOCKARRAY_BITS)
-#define CT_LOCKARRAY_MASK (CT_LOCKARRAY_SIZE-1)
+#define CT_MUTEX_BITS 4
+#define CT_MUTEX_SIZE (1<<CT_MUTEX_BITS)
+#define CT_MUTEX_MASK (CT_MUTEX_SIZE-1)
-struct ip_vs_aligned_lock
+struct ip_vs_aligned_spinlock
{
- rwlock_t l;
+ spinlock_t l;
} __attribute__((__aligned__(SMP_CACHE_BYTES)));
-/* lock array for conn table */
-static struct ip_vs_aligned_lock
-__ip_vs_conntbl_lock_array[CT_LOCKARRAY_SIZE] __cacheline_aligned;
+/* mutex array for connection table */
+static struct ip_vs_aligned_spinlock
+__ip_vs_conntbl_mutex[CT_MUTEX_SIZE] __cacheline_aligned;
-static inline void ct_read_lock(unsigned key)
+static inline void ct_mutex_lock(unsigned key)
{
- read_lock(&__ip_vs_conntbl_lock_array[key&CT_LOCKARRAY_MASK].l);
+ spin_lock(&__ip_vs_conntbl_mutex[key&CT_MUTEX_MASK].l);
}
-static inline void ct_read_unlock(unsigned key)
+static inline void ct_mutex_unlock(unsigned key)
{
- read_unlock(&__ip_vs_conntbl_lock_array[key&CT_LOCKARRAY_MASK].l);
+ spin_unlock(&__ip_vs_conntbl_mutex[key&CT_MUTEX_MASK].l);
}
-static inline void ct_write_lock(unsigned key)
+static inline void ct_mutex_lock_bh(unsigned key)
{
- write_lock(&__ip_vs_conntbl_lock_array[key&CT_LOCKARRAY_MASK].l);
+ spin_lock_bh(&__ip_vs_conntbl_mutex[key&CT_MUTEX_MASK].l);
}
-static inline void ct_write_unlock(unsigned key)
+static inline void ct_mutex_unlock_bh(unsigned key)
{
- write_unlock(&__ip_vs_conntbl_lock_array[key&CT_LOCKARRAY_MASK].l);
-}
-
-static inline void ct_read_lock_bh(unsigned key)
-{
- read_lock_bh(&__ip_vs_conntbl_lock_array[key&CT_LOCKARRAY_MASK].l);
-}
-
-static inline void ct_read_unlock_bh(unsigned key)
-{
- read_unlock_bh(&__ip_vs_conntbl_lock_array[key&CT_LOCKARRAY_MASK].l);
-}
-
-static inline void ct_write_lock_bh(unsigned key)
-{
- write_lock_bh(&__ip_vs_conntbl_lock_array[key&CT_LOCKARRAY_MASK].l);
-}
-
-static inline void ct_write_unlock_bh(unsigned key)
-{
- write_unlock_bh(&__ip_vs_conntbl_lock_array[key&CT_LOCKARRAY_MASK].l);
+ spin_unlock_bh(&__ip_vs_conntbl_mutex[key&CT_MUTEX_MASK].l);
}
@@ -155,27 +137,27 @@ static unsigned int ip_vs_conn_hashkey(i
static inline int ip_vs_conn_hash(struct ip_vs_conn *cp)
{
unsigned hash;
- int ret;
/* Hash by protocol, client address and port */
hash = ip_vs_conn_hashkey(cp->af, cp->protocol, &cp->caddr, cp->cport);
- ct_write_lock(hash);
+ ct_mutex_lock(hash);
if (!(cp->flags & IP_VS_CONN_F_HASHED)) {
- list_add(&cp->c_list, &ip_vs_conn_tab[hash]);
+ list_add_rcu(&cp->c_list, &ip_vs_conn_tab[hash]);
cp->flags |= IP_VS_CONN_F_HASHED;
atomic_inc(&cp->refcnt);
- ret = 1;
- } else {
- pr_err("%s(): request for already hashed, called from %pF\n",
- __func__, __builtin_return_address(0));
- ret = 0;
+ ct_mutex_unlock(hash);
+ synchronize_rcu();
+ return 1;
}
- ct_write_unlock(hash);
+ ct_mutex_unlock(hash);
- return ret;
+ pr_err("%s(): request for already hashed, called from %pF\n",
+ __func__, __builtin_return_address(0));
+
+ return 0;
}
@@ -186,24 +168,24 @@ static inline int ip_vs_conn_hash(struct
static inline int ip_vs_conn_unhash(struct ip_vs_conn *cp)
{
unsigned hash;
- int ret;
/* unhash it and decrease its reference counter */
hash = ip_vs_conn_hashkey(cp->af, cp->protocol, &cp->caddr, cp->cport);
- ct_write_lock(hash);
+ ct_mutex_lock(hash);
if (cp->flags & IP_VS_CONN_F_HASHED) {
- list_del(&cp->c_list);
+ list_del_rcu(&cp->c_list);
cp->flags &= ~IP_VS_CONN_F_HASHED;
atomic_dec(&cp->refcnt);
- ret = 1;
- } else
- ret = 0;
+ ct_mutex_unlock(hash);
+ synchronize_rcu();
+ return 1;
+ }
- ct_write_unlock(hash);
+ ct_mutex_unlock(hash);
- return ret;
+ return 0;
}
@@ -222,9 +204,9 @@ static inline struct ip_vs_conn *__ip_vs
hash = ip_vs_conn_hashkey(af, protocol, s_addr, s_port);
- ct_read_lock(hash);
+ rcu_read_lock();
- list_for_each_entry(cp, &ip_vs_conn_tab[hash], c_list) {
+ list_for_each_entry_rcu(cp, &ip_vs_conn_tab[hash], c_list) {
if (cp->af == af &&
ip_vs_addr_equal(af, s_addr, &cp->caddr) &&
ip_vs_addr_equal(af, d_addr, &cp->vaddr) &&
@@ -233,12 +215,12 @@ static inline struct ip_vs_conn *__ip_vs
protocol == cp->protocol) {
/* HIT */
atomic_inc(&cp->refcnt);
- ct_read_unlock(hash);
+ rcu_read_unlock();
return cp;
}
}
- ct_read_unlock(hash);
+ rcu_read_unlock();
return NULL;
}
@@ -273,9 +255,9 @@ struct ip_vs_conn *ip_vs_ct_in_get
hash = ip_vs_conn_hashkey(af, protocol, s_addr, s_port);
- ct_read_lock(hash);
+ rcu_read_lock();
- list_for_each_entry(cp, &ip_vs_conn_tab[hash], c_list) {
+ list_for_each_entry_rcu(cp, &ip_vs_conn_tab[hash], c_list) {
if (cp->af == af &&
ip_vs_addr_equal(af, s_addr, &cp->caddr) &&
/* protocol should only be IPPROTO_IP if
@@ -293,7 +275,7 @@ struct ip_vs_conn *ip_vs_ct_in_get
cp = NULL;
out:
- ct_read_unlock(hash);
+ rcu_read_unlock();
IP_VS_DBG_BUF(9, "template lookup/in %s %s:%d->%s:%d %s\n",
ip_vs_proto_name(protocol),
@@ -322,9 +304,9 @@ struct ip_vs_conn *ip_vs_conn_out_get
*/
hash = ip_vs_conn_hashkey(af, protocol, d_addr, d_port);
- ct_read_lock(hash);
+ rcu_read_lock();
- list_for_each_entry(cp, &ip_vs_conn_tab[hash], c_list) {
+ list_for_each_entry_rcu(cp, &ip_vs_conn_tab[hash], c_list) {
if (cp->af == af &&
ip_vs_addr_equal(af, d_addr, &cp->caddr) &&
ip_vs_addr_equal(af, s_addr, &cp->daddr) &&
@@ -337,7 +319,7 @@ struct ip_vs_conn *ip_vs_conn_out_get
}
}
- ct_read_unlock(hash);
+ rcu_read_unlock();
IP_VS_DBG_BUF(9, "lookup/out %s %s:%d->%s:%d %s\n",
ip_vs_proto_name(protocol),
@@ -776,14 +758,16 @@ static void *ip_vs_conn_array(struct seq
struct ip_vs_conn *cp;
for (idx = 0; idx < ip_vs_conn_tab_size; idx++) {
- ct_read_lock_bh(idx);
- list_for_each_entry(cp, &ip_vs_conn_tab[idx], c_list) {
+ rcu_read_lock_bh();
+ list_for_each_entry_rcu(cp, &ip_vs_conn_tab[idx], c_list) {
if (pos-- == 0) {
seq->private = &ip_vs_conn_tab[idx];
+ /* N.B: no rcu_read_unlock_bh() here
+ * Seems really horrible :-( */
return cp;
}
}
- ct_read_unlock_bh(idx);
+ rcu_read_unlock_bh();
}
return NULL;
@@ -807,19 +791,22 @@ static void *ip_vs_conn_seq_next(struct
/* more on same hash chain? */
if ((e = cp->c_list.next) != l)
- return list_entry(e, struct ip_vs_conn, c_list);
+ return list_entry_rcu(e, struct ip_vs_conn, c_list);
idx = l - ip_vs_conn_tab;
- ct_read_unlock_bh(idx);
+ rcu_read_unlock_bh();
while (++idx < ip_vs_conn_tab_size) {
- ct_read_lock_bh(idx);
- list_for_each_entry(cp, &ip_vs_conn_tab[idx], c_list) {
+ rcu_read_lock_bh();
+ list_for_each_entry_rcu(cp, &ip_vs_conn_tab[idx], c_list) {
seq->private = &ip_vs_conn_tab[idx];
+ /* N.B: no rcu_read_unlock_bh() here
+ * Seems really horrible :-( */
return cp;
}
- ct_read_unlock_bh(idx);
+ rcu_read_unlock_bh();
}
+
seq->private = NULL;
return NULL;
}
@@ -829,7 +816,7 @@ static void ip_vs_conn_seq_stop(struct s
struct list_head *l = seq->private;
if (l)
- ct_read_unlock_bh(l - ip_vs_conn_tab);
+ rcu_read_unlock_bh();
}
static int ip_vs_conn_seq_show(struct seq_file *seq, void *v)
@@ -997,8 +984,7 @@ void ip_vs_random_dropentry(void)
/*
* Lock is actually needed in this loop.
*/
- ct_write_lock_bh(hash);
-
+ ct_mutex_lock_bh(hash);
list_for_each_entry(cp, &ip_vs_conn_tab[hash], c_list) {
if (cp->flags & IP_VS_CONN_F_TEMPLATE)
/* connection template */
@@ -1030,7 +1016,7 @@ void ip_vs_random_dropentry(void)
ip_vs_conn_expire_now(cp->control);
}
}
- ct_write_unlock_bh(hash);
+ ct_mutex_unlock_bh(hash);
}
}
@@ -1048,8 +1034,7 @@ static void ip_vs_conn_flush(void)
/*
* Lock is actually needed in this loop.
*/
- ct_write_lock_bh(idx);
-
+ ct_mutex_lock_bh(idx);
list_for_each_entry(cp, &ip_vs_conn_tab[idx], c_list) {
IP_VS_DBG(4, "del connection\n");
@@ -1059,7 +1044,7 @@ static void ip_vs_conn_flush(void)
ip_vs_conn_expire_now(cp->control);
}
}
- ct_write_unlock_bh(idx);
+ ct_mutex_unlock_bh(idx);
}
/* the counter may be not NULL, because maybe some conn entries
@@ -1107,9 +1092,8 @@ int __init ip_vs_conn_init(void)
INIT_LIST_HEAD(&ip_vs_conn_tab[idx]);
}
- for (idx = 0; idx < CT_LOCKARRAY_SIZE; idx++) {
- rwlock_init(&__ip_vs_conntbl_lock_array[idx].l);
- }
+ for (idx = 0; idx < CT_MUTEX_SIZE; idx++)
+ spin_lock_init(&__ip_vs_conntbl_mutex[idx].l);
proc_net_fops_create(&init_net, "ip_vs_conn", 0, &ip_vs_conn_fops);
proc_net_fops_create(&init_net, "ip_vs_conn_sync", 0, &ip_vs_conn_sync_fops);
^ permalink raw reply
* [patch] IPVS: ip_vs_lblcr: use list head
From: Simon Horman @ 2010-02-26 3:02 UTC (permalink / raw)
To: netdev, lvs-devel
Cc: Wensong Zhang, Julian Anastasov, Patrick McHardy, David S. Miller
Use list_head rather than a custom list implementation.
Signed-off-by: Simon Horman <horms@verge.net.au>
Index: net-next-2.6/net/netfilter/ipvs/ip_vs_lblcr.c
===================================================================
--- net-next-2.6.orig/net/netfilter/ipvs/ip_vs_lblcr.c 2010-02-24 15:32:33.000000000 +1100
+++ net-next-2.6/net/netfilter/ipvs/ip_vs_lblcr.c 2010-02-25 11:56:16.000000000 +1100
@@ -45,6 +45,7 @@
#include <linux/kernel.h>
#include <linux/skbuff.h>
#include <linux/jiffies.h>
+#include <linux/list.h>
/* for sysctl */
#include <linux/fs.h>
@@ -85,25 +86,25 @@ static int sysctl_ip_vs_lblcr_expiration
/*
* IPVS destination set structure and operations
*/
-struct ip_vs_dest_list {
- struct ip_vs_dest_list *next; /* list link */
+struct ip_vs_dest_set_elem {
+ struct list_head list; /* list link */
struct ip_vs_dest *dest; /* destination server */
};
struct ip_vs_dest_set {
atomic_t size; /* set size */
unsigned long lastmod; /* last modified time */
- struct ip_vs_dest_list *list; /* destination list */
+ struct list_head list; /* destination list */
rwlock_t lock; /* lock for this list */
};
-static struct ip_vs_dest_list *
+static struct ip_vs_dest_set_elem *
ip_vs_dest_set_insert(struct ip_vs_dest_set *set, struct ip_vs_dest *dest)
{
- struct ip_vs_dest_list *e;
+ struct ip_vs_dest_set_elem *e;
- for (e=set->list; e!=NULL; e=e->next) {
+ list_for_each_entry(e, &set->list, list) {
if (e->dest == dest)
/* already existed */
return NULL;
@@ -118,9 +119,7 @@ ip_vs_dest_set_insert(struct ip_vs_dest_
atomic_inc(&dest->refcnt);
e->dest = dest;
- /* link it to the list */
- e->next = set->list;
- set->list = e;
+ list_add(&e->list, &set->list);
atomic_inc(&set->size);
set->lastmod = jiffies;
@@ -130,34 +129,33 @@ ip_vs_dest_set_insert(struct ip_vs_dest_
static void
ip_vs_dest_set_erase(struct ip_vs_dest_set *set, struct ip_vs_dest *dest)
{
- struct ip_vs_dest_list *e, **ep;
+ struct ip_vs_dest_set_elem *e;
- for (ep=&set->list, e=*ep; e!=NULL; e=*ep) {
+ list_for_each_entry(e, &set->list, list) {
if (e->dest == dest) {
/* HIT */
- *ep = e->next;
atomic_dec(&set->size);
set->lastmod = jiffies;
atomic_dec(&e->dest->refcnt);
+ list_del(&e->list);
kfree(e);
break;
}
- ep = &e->next;
}
}
static void ip_vs_dest_set_eraseall(struct ip_vs_dest_set *set)
{
- struct ip_vs_dest_list *e, **ep;
+ struct ip_vs_dest_set_elem *e, *ep;
write_lock(&set->lock);
- for (ep=&set->list, e=*ep; e!=NULL; e=*ep) {
- *ep = e->next;
+ list_for_each_entry_safe(e, ep, &set->list, list) {
/*
* We don't kfree dest because it is refered either
* by its service or by the trash dest list.
*/
atomic_dec(&e->dest->refcnt);
+ list_del(&e->list);
kfree(e);
}
write_unlock(&set->lock);
@@ -166,7 +164,7 @@ static void ip_vs_dest_set_eraseall(stru
/* get weighted least-connection node in the destination set */
static inline struct ip_vs_dest *ip_vs_dest_set_min(struct ip_vs_dest_set *set)
{
- register struct ip_vs_dest_list *e;
+ register struct ip_vs_dest_set_elem *e;
struct ip_vs_dest *dest, *least;
int loh, doh;
@@ -174,7 +172,7 @@ static inline struct ip_vs_dest *ip_vs_d
return NULL;
/* select the first destination server, whose weight > 0 */
- for (e=set->list; e!=NULL; e=e->next) {
+ list_for_each_entry(e, &set->list, list) {
least = e->dest;
if (least->flags & IP_VS_DEST_F_OVERLOAD)
continue;
@@ -190,7 +188,7 @@ static inline struct ip_vs_dest *ip_vs_d
/* find the destination with the weighted least load */
nextstage:
- for (e=e->next; e!=NULL; e=e->next) {
+ list_for_each_entry(e, &set->list, list) {
dest = e->dest;
if (dest->flags & IP_VS_DEST_F_OVERLOAD)
continue;
@@ -220,7 +218,7 @@ static inline struct ip_vs_dest *ip_vs_d
/* get weighted most-connection node in the destination set */
static inline struct ip_vs_dest *ip_vs_dest_set_max(struct ip_vs_dest_set *set)
{
- register struct ip_vs_dest_list *e;
+ register struct ip_vs_dest_set_elem *e;
struct ip_vs_dest *dest, *most;
int moh, doh;
@@ -228,7 +226,7 @@ static inline struct ip_vs_dest *ip_vs_d
return NULL;
/* select the first destination server, whose weight > 0 */
- for (e=set->list; e!=NULL; e=e->next) {
+ list_for_each_entry(e, &set->list, list) {
most = e->dest;
if (atomic_read(&most->weight) > 0) {
moh = atomic_read(&most->activeconns) * 50
@@ -240,7 +238,7 @@ static inline struct ip_vs_dest *ip_vs_d
/* find the destination with the weighted most load */
nextstage:
- for (e=e->next; e!=NULL; e=e->next) {
+ list_for_each_entry(e, &set->list, list) {
dest = e->dest;
doh = atomic_read(&dest->activeconns) * 50
+ atomic_read(&dest->inactconns);
@@ -389,7 +387,7 @@ ip_vs_lblcr_new(struct ip_vs_lblcr_table
/* initilize its dest set */
atomic_set(&(en->set.size), 0);
- en->set.list = NULL;
+ INIT_LIST_HEAD(&en->set.list);
rwlock_init(&en->set.lock);
ip_vs_lblcr_hash(tbl, en);
^ permalink raw reply
* Re: Gianfar driver failing on MPC8641D based board
From: Anton Vorontsov @ 2010-02-26 3:14 UTC (permalink / raw)
To: Paul Gortmaker
Cc: netdev, linux-kernel, Martyn Welch, linuxppc-dev list,
Sandeep Gopalpet, davem
In-Reply-To: <7d1d9c251002251653n6473f01ex2d43933ec6aa010b@mail.gmail.com>
On Thu, Feb 25, 2010 at 07:53:30PM -0500, Paul Gortmaker wrote:
[...]
> I was able to reproduce it on an 8641D and bisected it down to this:
>
> -----------
> commit a3bc1f11e9b867a4f49505ecac486a33af248b2e
> Author: Anton Vorontsov <avorontsov@ru.mvista.com>
> Date: Tue Nov 10 14:11:10 2009 +0000
>
> gianfar: Revive SKB recycling
Thanks for the bisect. I have a guess why tx hangs in
SMP case. Could anyone try the patch down below?
[...]
> ...which probably explains why you weren't seeing it on non-SMP.
> I'd imagine it would show up on any of the e500mc boards too.
Yeah.. Pity, I don't have SMP boards anymore. I'll try
to get one though.
diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
index 8bd3c9f..3ff3bd0 100644
--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -2614,6 +2614,8 @@ static int gfar_poll(struct napi_struct *napi, int budget)
tx_queue = priv->tx_queue[rx_queue->qindex];
tx_cleaned += gfar_clean_tx_ring(tx_queue);
+ if (!tx_cleaned && !tx_queue->num_txbdfree)
+ tx_cleaned += 1; /* don't complete napi */
rx_cleaned_per_queue = gfar_clean_rx_ring(rx_queue,
budget_per_queue);
rx_cleaned += rx_cleaned_per_queue;
^ permalink raw reply related
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