* Re: [PATCH] tipc: fix endianness on tipc subscriber messages
From: Neil Horman @ 2010-03-08 21:09 UTC (permalink / raw)
To: Stephens, Allan; +Cc: davem, netdev
In-Reply-To: <29C1DC0826876849BDD9F1C67ABA29430728D116@ala-mail09.corp.ad.wrs.com>
On Mon, Mar 08, 2010 at 12:48:38PM -0800, Stephens, Allan wrote:
> Hi there:
>
> There are a couple of problems with this patch that need to be resolved
> before it can be applied to the upstream kernel.
>
> 1) Neil's replacement of the htohl() call with the equivalent
> htonl()/ntohl() calls, while well intentioned, was done without
> understanding why the htohl() calls were put there in the first place.
I'd love to hear what those reasons are. You're formating on-the-wire data to
an endianness defined by the receiving entity. I'm hard pressed to imagine a
reason why thats sane.
> In addition, the TIPC specification that he used to justify some of his
> decisions is out-dated, and doesn't reflect the latest version of the
> TIPC protocol. (I'll discuss this further in a follow-up email.)
>
Please, point out a more recent spec. The one I referenced is the most recent
publically available spec that I can find. although again, I'm hard pressed to
believe that the spec has changed to include a requirement to send data in
receiver byte order.
> 2) Neil's alteration of the memcpy() in the subscription cancelation
> routine is simply wrong. The pieces of the data structure that he
> claims are local are not local, and must be checked to ensure that the
> cancellation is done properly.
>
The origional memcmp checks sizeof(tipc_subscr) bytes of each entry in the
array. tipc_subscr is defined as:
struct tipc_subscr {
struct tipc_name_seq seq; /* name sequence of interest */
__u32 timeout; /* subscription duration (in ms) */
__u32 filter; /* bitmask of filter options */
char usr_handle[8]; /* available for subscriber use */
};
In subscr_subscribe (from which we also call subscr_cancel), we allocate new
entries for that list, only fillingout the seq.type, seq.lower, seq.upper,
timeout and filter. So if anyone locally changes usr_handle (which I see
several places that do), the memcmp won't match up properly. Additionally, I'm
hard pressed to believe (and this would be supported by the section of the spec
that I referenced), timeout, and filter also don't bear on the uniqueness of the
subscriber Id. Of course a subsequent version of the spec might have changed
that, But if it is, please point it out to me.
> I'm also surprised to see that this patch was immediately applied to
> net-2.6. First, there was no opportunity given for people to comment on
> the patch. Secondly, I would have expected this patch to be applied to
> net-next-2.6, since the functionality being changed here (at least the
> first part of it) is more like a feature enhancement than a bug fix. Am
> I misunderstanding the process being followed here? If so, please
> explain ...
>
I'll let David Comment of this, since the destination tree is his final
decision.
Neil
> Regards,
> Al
>
> > -----Original Message-----
> > From: Neil Horman [mailto:nhorman@tuxdriver.com]
> > Sent: Monday, March 08, 2010 3:03 PM
> > To: netdev@vger.kernel.org
> > Cc: Stephens, Allan; davem@davemloft.net; nhorman@tuxdriver.com
> > Subject: [PATCH] tipc: fix endianness on tipc subscriber messages
> >
> > Remove htohl implementation from tipc
> >
> > I was working on forward porting the downstream commits for
> > TIPC and ran accross this one:
> > http://tipc.cslab.ericsson.net/cgi-bin/gitweb.cgi?p=people/all
> > an/tipc.git;a=commitdiff;h=894279b9437b63cbb02405ad5b8e033b51e4e31e
> >
> > I was going to just take it, when I looked closer and noted
> > what it was doing.
> > This is basically a routine to byte swap fields of data in
> > sent/received packets for tipc, dependent upon the receivers
> > guessed endianness of the peer when a connection is
> > established. Asside from just seeming silly to me, it
> > appears to violate the latest RFC draft for tipc:
> > http://tipc.sourceforge.net/doc/draft-spec-tipc-02.txt
> > Which, according to section 4.2 and 4.3.3, requires that all
> > fields of all commands be sent in network byte order. So
> > instead of just taking this patch, instead I'm removing the
> > htohl function and replacing the calls with calls to ntohl in
> > the rx path and htonl in the send path.
> >
> > As part of this fix, I'm also changing the subscr_cancel
> > function, which searches the list of subscribers, using a
> > memcmp of the entire subscriber list, for the entry to tear
> > down. unfortunately it memcmps the entire tipc_subscr
> > structure which has several bits that are private to the
> > local side, so nothing will ever match. section 5.2 of the
> > draft spec indicates the <type,upper,lower> tuple should
> > uniquely identify a subscriber, so convert subscr_cancel to
> > just match on those fields (properly endian swapped).
> >
> > I've tested this using the tipc test suite, and its passed
> > without issue.
> >
> > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> > CC: Allan Stephens <allan.stephens@windriver.com>
> >
> >
> > subscr.c | 57
> > ++++++++++++++++++++++-----------------------------------
> > subscr.h | 2 --
> > 2 files changed, 22 insertions(+), 37 deletions(-)
> >
> > diff --git a/net/tipc/subscr.c b/net/tipc/subscr.c index
> > ac91f0d..ff123e5 100644
> > --- a/net/tipc/subscr.c
> > +++ b/net/tipc/subscr.c
> > @@ -76,19 +76,6 @@ struct top_srv {
> > static struct top_srv topsrv = { 0 };
> >
> > /**
> > - * htohl - convert value to endianness used by destination
> > - * @in: value to convert
> > - * @swap: non-zero if endianness must be reversed
> > - *
> > - * Returns converted value
> > - */
> > -
> > -static u32 htohl(u32 in, int swap)
> > -{
> > - return swap ? swab32(in) : in;
> > -}
> > -
> > -/**
> > * subscr_send_event - send a message containing a
> > tipc_event to the subscriber
> > *
> > * Note: Must not hold subscriber's server port lock, since
> > tipc_send() will @@ -107,11 +94,11 @@ static void
> > subscr_send_event(struct subscription *sub,
> > msg_sect.iov_base = (void *)&sub->evt;
> > msg_sect.iov_len = sizeof(struct tipc_event);
> >
> > - sub->evt.event = htohl(event, sub->swap);
> > - sub->evt.found_lower = htohl(found_lower, sub->swap);
> > - sub->evt.found_upper = htohl(found_upper, sub->swap);
> > - sub->evt.port.ref = htohl(port_ref, sub->swap);
> > - sub->evt.port.node = htohl(node, sub->swap);
> > + sub->evt.event = htonl(event);
> > + sub->evt.found_lower = htonl(found_lower);
> > + sub->evt.found_upper = htonl(found_upper);
> > + sub->evt.port.ref = htonl(port_ref);
> > + sub->evt.port.node = htonl(node);
> > tipc_send(sub->server_ref, 1, &msg_sect); }
> >
> > @@ -287,16 +274,23 @@ static void subscr_cancel(struct
> > tipc_subscr *s, {
> > struct subscription *sub;
> > struct subscription *sub_temp;
> > + __u32 type, lower, upper;
> > int found = 0;
> >
> > /* Find first matching subscription, exit if not found */
> >
> > + type = ntohl(s->seq.type);
> > + lower = ntohl(s->seq.lower);
> > + upper = ntohl(s->seq.upper);
> > +
> > list_for_each_entry_safe(sub, sub_temp,
> > &subscriber->subscription_list,
> > subscription_list) {
> > - if (!memcmp(s, &sub->evt.s, sizeof(struct
> > tipc_subscr))) {
> > - found = 1;
> > - break;
> > - }
> > + if ((type == sub->seq.type) &&
> > + (lower == sub->seq.lower) &&
> > + (upper == sub->seq.upper)) {
> > + found = 1;
> > + break;
> > + }
> > }
> > if (!found)
> > return;
> > @@ -325,16 +319,10 @@ static struct subscription
> > *subscr_subscribe(struct tipc_subscr *s,
> > struct subscriber
> > *subscriber) {
> > struct subscription *sub;
> > - int swap;
> > -
> > - /* Determine subscriber's endianness */
> > -
> > - swap = !(s->filter & (TIPC_SUB_PORTS | TIPC_SUB_SERVICE));
> >
> > /* Detect & process a subscription cancellation request */
> >
> > - if (s->filter & htohl(TIPC_SUB_CANCEL, swap)) {
> > - s->filter &= ~htohl(TIPC_SUB_CANCEL, swap);
> > + if (ntohl(s->filter) & TIPC_SUB_CANCEL) {
> > subscr_cancel(s, subscriber);
> > return NULL;
> > }
> > @@ -359,11 +347,11 @@ static struct subscription
> > *subscr_subscribe(struct tipc_subscr *s,
> >
> > /* Initialize subscription object */
> >
> > - sub->seq.type = htohl(s->seq.type, swap);
> > - sub->seq.lower = htohl(s->seq.lower, swap);
> > - sub->seq.upper = htohl(s->seq.upper, swap);
> > - sub->timeout = htohl(s->timeout, swap);
> > - sub->filter = htohl(s->filter, swap);
> > + sub->seq.type = ntohl(s->seq.type);
> > + sub->seq.lower = ntohl(s->seq.lower);
> > + sub->seq.upper = ntohl(s->seq.upper);
> > + sub->timeout = ntohl(s->timeout);
> > + sub->filter = ntohl(s->filter);
> > if ((!(sub->filter & TIPC_SUB_PORTS) ==
> > !(sub->filter & TIPC_SUB_SERVICE)) ||
> > (sub->seq.lower > sub->seq.upper)) { @@ -376,7
> > +364,6 @@ static struct subscription *subscr_subscribe(struct
> > tipc_subscr *s,
> > INIT_LIST_HEAD(&sub->nameseq_list);
> > list_add(&sub->subscription_list,
> > &subscriber->subscription_list);
> > sub->server_ref = subscriber->port_ref;
> > - sub->swap = swap;
> > memcpy(&sub->evt.s, s, sizeof(struct tipc_subscr));
> > atomic_inc(&topsrv.subscription_count);
> > if (sub->timeout != TIPC_WAIT_FOREVER) { diff --git
> > a/net/tipc/subscr.h b/net/tipc/subscr.h index 45d89bf..c20f496 100644
> > --- a/net/tipc/subscr.h
> > +++ b/net/tipc/subscr.h
> > @@ -53,7 +53,6 @@ typedef void (*tipc_subscr_event) (struct
> > subscription *sub,
> > * @nameseq_list: adjacent subscriptions in name sequence's
> > subscription list
> > * @subscription_list: adjacent subscriptions in
> > subscriber's subscription list
> > * @server_ref: object reference of server port associated
> > with subscription
> > - * @swap: indicates if subscriber uses opposite endianness
> > in its messages
> > * @evt: template for events generated by subscription
> > */
> >
> > @@ -66,7 +65,6 @@ struct subscription {
> > struct list_head nameseq_list;
> > struct list_head subscription_list;
> > u32 server_ref;
> > - int swap;
> > struct tipc_event evt;
> > };
> >
> >
>
^ permalink raw reply
* Re: [RFC][PATCH] ns: Syscalls for better namespace sharing control.
From: Daniel Lezcano @ 2010-03-08 21:12 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Pavel Emelyanov, Sukadev Bhattiprolu, Serge Hallyn,
Linux Netdev List, containers, Netfilter Development Mailinglist,
Ben Greear
In-Reply-To: <m1sk8ar15b.fsf@fess.ebiederm.org>
Eric W. Biederman wrote:
> Daniel Lezcano <daniel.lezcano@free.fr> writes:
>
>
>> Eric W. Biederman wrote:
>>
>>> Daniel Lezcano <daniel.lezcano@free.fr> writes:
>>>
>>>
>>>
>>>> Eric W. Biederman wrote:
>>>>
>>>>
>>>>> Daniel Lezcano <daniel.lezcano@free.fr> writes:
>>>>>
>>>>>
>>>>>
>>>>>> Eric W. Biederman wrote:
>>>>>>
>>>>>>
>>>>>>> I have take an snapshot of my development tree and placed it at.
>>>>>>>
>>>>>>>
>>>>>>> git://git.kernel.org/pub/scm/linux/people/ebiederm/linux-2.6.33-nsfd-v5.git
>>>>>>>
>>>>>>>
>>>>>> Hi Eric,
>>>>>>
>>>>>> thanks for the pointer.
>>>>>>
>>>>>> I tried to boot the kernel under qemu and I got this oops:
>>>>>>
>>>>>>
>>>>> I am clearly running an old userspace on my test machine. No udev.
>>>>> It looks like udev has a long standing netlink misfeature, where
>>>>> it does not initializing NETLINK_CB....
>>>>>
>>>>>
>>>>> >From 8d85e3ab88718eda3d94cf8e1be14b69dae2b8f1 Mon Sep 17 00:00:00 2001
>>>>> From: Eric W. Biederman <ebiederm@xmission.com>
>>>>> Date: Mon, 8 Mar 2010 09:25:20 -0800
>>>>> Subject: [PATCH] kobject_uevent: Use the netlink allocator helper...
>>>>>
>>>>> Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
>>>>>
>>>>>
>>>> Thanks.
>>>>
>>>> I was able to boot but I have the following warning:
>>>>
>>>>
>>> Thanks for the bug report.
>>>
>>>
>> Thanks to you for the patchset :)
>>
>>
>>> For the moment you might want to drop:
>>> af_netlink: Allow credentials to work across namespaces.
>>> af_netlink: Debugging in case I have missed something.
>>>
>>> Although I am curious if you hit my debugging messages in
>>> netlink recv.
>>>
>>>
>> No, it does not appear (looked for "missing NETLINK_CB proto").
>>
>>
>>> I guess if the goal is to test my nsfd bits you can drop everything
>>> starting with my 'scm: Reorder scm_cookie.' commit. The rest is what
>>> it takes to get get uids, gid and pids translated when the cross
>>> namespaces on an af_unix of an af_netlink socket.
>>>
>>> At least in the af_netlink case it appears clear I am have missed
>>> something.
>>>
>>> This is a warning that netlink throws when the packet accounting messed
>>> up. So it sounds like you are exercising another path that I failed
>>> to exercise and fix.
>>>
>>>
>> I will look forward if I find more clues for this warning.
>>
>> In the meantime was able to enter the container with the ugly following
>> program:
>>
>> #include <unistd.h>
>> #include <stdlib.h>
>> #include <stdio.h>
>> #include <syscall.h>
>> #include <sys/types.h>
>> #include <sys/stat.h>
>> #include <fcntl.h>
>> #include <sys/param.h>
>>
>> #define __NR_setns 300
>>
>> int setns(int nstype, int fd)
>> {
>> return syscall (__NR_setns, nstype, fd);
>> }
>>
>> int main(int argc, char *argv[])
>> {
>> char path[MAXPATHLEN];
>> char *ns[] = { "pid", "mnt", "net", "pid", "uts" };
>> const int size = sizeof(ns) / sizeof(char *);
>> int fd[size];
>> int i;
>>
>> if (argc != 3) {
>> fprintf(stderr, "mynsenter <pid> <command>\n");
>> exit(1);
>> }
>>
>> for (i = 0; i < size; i++) {
>> sprintf(path, "/proc/%s/ns/%s", argv[1], ns[i]);
>>
>> fd[i] = open(path, O_RDONLY);
>> if (fd[i] < 0) {
>> perror("open");
>> return -1;
>> }
>>
>> }
>>
>> for (i = 0; i < size; i++) {
>>
>> if (setns(0, fd[i])) {
>> perror("setns");
>> return -1;
>> }
>> }
>>
>> execve(argv[2], &argv[2], NULL);
>> perror("execve");
>>
>> return 0;
>> }
>>
>> At the fist glance, no problem :)
>>
>
> No fork() so your processes is completely in the pid namespace?
>
What I do is to attach "/bin/sh" to the container with this program.
The container is a VPS running busybox with the full isolation.
echo $$ gives the real pid.
All the forked processes appears in the pid namespace, they are visible
through /proc with the virtual pid.
I am not able to change to the /proc/self directory (I assume this is
normal).
^ permalink raw reply
* Re: [PATCH]: tipc: Fix oops on send prior to entering networked mode (v2)
From: Neil Horman @ 2010-03-08 21:13 UTC (permalink / raw)
To: David Miller; +Cc: netdev, jon.maloy, allan.stephens, tipc-discussion
In-Reply-To: <20100308.121942.67584963.davem@davemloft.net>
On Mon, Mar 08, 2010 at 12:19:42PM -0800, David Miller wrote:
>
> Doesn't apply to net-2.6, Neil please respin.
>
Yes, sorry David, didn't mean to reply so quickly before. I just looked at your
net-2.6 tree, and d0021b252eaf65ca07ed14f0d66425dd9ccab9a6 is in there just
fine. Not sure if I'm missing something or not, but as far as I can see you've
already apply the patch properly.
Regards
Neil
^ permalink raw reply
* Re: [RFC][PATCH] ns: Syscalls for better namespace sharing control.
From: Eric W. Biederman @ 2010-03-08 21:25 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Pavel Emelyanov, Sukadev Bhattiprolu, Serge Hallyn,
Linux Netdev List, containers, Netfilter Development Mailinglist,
Ben Greear
In-Reply-To: <4B956852.7050804@free.fr>
Daniel Lezcano <daniel.lezcano@free.fr> writes:
> Eric W. Biederman wrote:
>> Daniel Lezcano <daniel.lezcano@free.fr> writes:
>>
>>
>>> Eric W. Biederman wrote:
>>>
>>>> Daniel Lezcano <daniel.lezcano@free.fr> writes:
>>>>
>>>>
>>>>> Eric W. Biederman wrote:
>>>>>
>>>>>> Daniel Lezcano <daniel.lezcano@free.fr> writes:
>>>>>>
>>>>>>
>>>>>>> Eric W. Biederman wrote:
>>>>>>>
>>>>>>>> I have take an snapshot of my development tree and placed it at.
>>>>>>>>
>>>>>>>>
>>>>>>>> git://git.kernel.org/pub/scm/linux/people/ebiederm/linux-2.6.33-nsfd-v5.git
>>>>>>>>
>>>>>>> Hi Eric,
>>>>>>>
>>>>>>> thanks for the pointer.
>>>>>>>
>>>>>>> I tried to boot the kernel under qemu and I got this oops:
>>>>>>>
>>>>>> I am clearly running an old userspace on my test machine. No udev.
>>>>>> It looks like udev has a long standing netlink misfeature, where
>>>>>> it does not initializing NETLINK_CB....
>>>>>>
>>>>>>
>>>>>> >From 8d85e3ab88718eda3d94cf8e1be14b69dae2b8f1 Mon Sep 17 00:00:00 2001
>>>>>> From: Eric W. Biederman <ebiederm@xmission.com>
>>>>>> Date: Mon, 8 Mar 2010 09:25:20 -0800
>>>>>> Subject: [PATCH] kobject_uevent: Use the netlink allocator helper...
>>>>>>
>>>>>> Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
>>>>>>
>>>>> Thanks.
>>>>>
>>>>> I was able to boot but I have the following warning:
>>>>>
>>>> Thanks for the bug report.
>>>>
>>> Thanks to you for the patchset :)
>>>
>>>
>>>> For the moment you might want to drop:
>>>> af_netlink: Allow credentials to work across namespaces.
>>>> af_netlink: Debugging in case I have missed something.
>>>>
>>>> Although I am curious if you hit my debugging messages in
>>>> netlink recv.
>>>>
>>> No, it does not appear (looked for "missing NETLINK_CB proto").
>>>
>>>
>>>> I guess if the goal is to test my nsfd bits you can drop everything
>>>> starting with my 'scm: Reorder scm_cookie.' commit. The rest is what
>>>> it takes to get get uids, gid and pids translated when the cross
>>>> namespaces on an af_unix of an af_netlink socket.
>>>>
>>>> At least in the af_netlink case it appears clear I am have missed
>>>> something.
>>>>
>>>> This is a warning that netlink throws when the packet accounting messed
>>>> up. So it sounds like you are exercising another path that I failed
>>>> to exercise and fix.
>>>>
>>> I will look forward if I find more clues for this warning.
>>>
>>> In the meantime was able to enter the container with the ugly following
>>> program:
>>>
>>> #include <unistd.h>
>>> #include <stdlib.h>
>>> #include <stdio.h>
>>> #include <syscall.h>
>>> #include <sys/types.h>
>>> #include <sys/stat.h>
>>> #include <fcntl.h>
>>> #include <sys/param.h>
>>>
>>> #define __NR_setns 300
>>>
>>> int setns(int nstype, int fd)
>>> {
>>> return syscall (__NR_setns, nstype, fd);
>>> }
>>>
>>> int main(int argc, char *argv[])
>>> {
>>> char path[MAXPATHLEN];
>>> char *ns[] = { "pid", "mnt", "net", "pid", "uts" };
>>> const int size = sizeof(ns) / sizeof(char *);
>>> int fd[size];
>>> int i;
>>>
>>> if (argc != 3) {
>>> fprintf(stderr, "mynsenter <pid> <command>\n");
>>> exit(1);
>>> }
>>>
>>> for (i = 0; i < size; i++) {
>>> sprintf(path, "/proc/%s/ns/%s", argv[1], ns[i]);
>>>
>>> fd[i] = open(path, O_RDONLY);
>>> if (fd[i] < 0) {
>>> perror("open");
>>> return -1;
>>> }
>>>
>>> }
>>>
>>> for (i = 0; i < size; i++) {
>>>
>>> if (setns(0, fd[i])) {
>>> perror("setns");
>>> return -1;
>>> }
>>> }
>>>
>>> execve(argv[2], &argv[2], NULL);
>>> perror("execve");
>>>
>>> return 0;
>>> }
>>>
>>> At the fist glance, no problem :)
>>>
>>
>> No fork() so your processes is completely in the pid namespace?
>>
> What I do is to attach "/bin/sh" to the container with this program.
> The container is a VPS running busybox with the full isolation.
>
> echo $$ gives the real pid.
> All the forked processes appears in the pid namespace, they are visible through
> /proc with the virtual pid.
> I am not able to change to the /proc/self directory (I assume this is normal).
I guess my meaning is I was expecting.
child = fork();
if (child == 0) {
execve(...);
}
waitpid(child);
This puts /bin/sh in the container as well.
I'm not certain about the /proc/self thing I have never encountered that.
But I guess if your pid is outside of the pid namespace of that instance
of proc /proc/self will be a broken symlink.
Eric
^ permalink raw reply
* Re: [PATCH]: tipc: Fix oops on send prior to entering networked mode (v2)
From: David Miller @ 2010-03-08 21:26 UTC (permalink / raw)
To: nhorman; +Cc: netdev, jon.maloy, allan.stephens, tipc-discussion
In-Reply-To: <20100308211308.GI23634@hmsreliant.think-freely.org>
From: Neil Horman <nhorman@tuxdriver.com>
Date: Mon, 8 Mar 2010 16:13:08 -0500
> On Mon, Mar 08, 2010 at 12:19:42PM -0800, David Miller wrote:
>>
>> Doesn't apply to net-2.6, Neil please respin.
>>
> Yes, sorry David, didn't mean to reply so quickly before. I just looked at your
> net-2.6 tree, and d0021b252eaf65ca07ed14f0d66425dd9ccab9a6 is in there just
> fine. Not sure if I'm missing something or not, but as far as I can see you've
> already apply the patch properly.
Perfect.
^ permalink raw reply
* [PATCH] netlink: convert DIY reader/writer to mutex and RCU
From: Stephen Hemminger @ 2010-03-08 21:32 UTC (permalink / raw)
To: David Miller; +Cc: netdev
The netlink table locking was open coded of reader/writer
sleeping lock. Change to using mutex and RCU which makes
code clearer, shorter, and simpler.
The genetlink code is simplified because it is allowable
to acquire mutex with RCU lock.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
---
include/linux/netlink.h | 3
include/net/sock.h | 2
net/netlink/af_netlink.c | 146 ++++++++++++++---------------------------------
net/netlink/genetlink.c | 4 -
4 files changed, 47 insertions(+), 108 deletions(-)
--- a/net/netlink/af_netlink.c 2010-03-08 09:07:15.104547875 -0800
+++ b/net/netlink/af_netlink.c 2010-03-08 11:02:01.263297712 -0800
@@ -128,15 +128,11 @@ struct netlink_table {
};
static struct netlink_table *nl_table;
-
-static DECLARE_WAIT_QUEUE_HEAD(nl_table_wait);
+static DEFINE_MUTEX(nltable_mutex);
static int netlink_dump(struct sock *sk);
static void netlink_destroy_callback(struct netlink_callback *cb);
-static DEFINE_RWLOCK(nl_table_lock);
-static atomic_t nl_table_users = ATOMIC_INIT(0);
-
static ATOMIC_NOTIFIER_HEAD(netlink_chain);
static u32 netlink_group_mask(u32 group)
@@ -171,61 +167,6 @@ static void netlink_sock_destruct(struct
WARN_ON(nlk_sk(sk)->groups);
}
-/* This lock without WQ_FLAG_EXCLUSIVE is good on UP and it is _very_ bad on
- * SMP. Look, when several writers sleep and reader wakes them up, all but one
- * immediately hit write lock and grab all the cpus. Exclusive sleep solves
- * this, _but_ remember, it adds useless work on UP machines.
- */
-
-void netlink_table_grab(void)
- __acquires(nl_table_lock)
-{
- might_sleep();
-
- write_lock_irq(&nl_table_lock);
-
- if (atomic_read(&nl_table_users)) {
- DECLARE_WAITQUEUE(wait, current);
-
- add_wait_queue_exclusive(&nl_table_wait, &wait);
- for (;;) {
- set_current_state(TASK_UNINTERRUPTIBLE);
- if (atomic_read(&nl_table_users) == 0)
- break;
- write_unlock_irq(&nl_table_lock);
- schedule();
- write_lock_irq(&nl_table_lock);
- }
-
- __set_current_state(TASK_RUNNING);
- remove_wait_queue(&nl_table_wait, &wait);
- }
-}
-
-void netlink_table_ungrab(void)
- __releases(nl_table_lock)
-{
- write_unlock_irq(&nl_table_lock);
- wake_up(&nl_table_wait);
-}
-
-static inline void
-netlink_lock_table(void)
-{
- /* read_lock() synchronizes us to netlink_table_grab */
-
- read_lock(&nl_table_lock);
- atomic_inc(&nl_table_users);
- read_unlock(&nl_table_lock);
-}
-
-static inline void
-netlink_unlock_table(void)
-{
- if (atomic_dec_and_test(&nl_table_users))
- wake_up(&nl_table_wait);
-}
-
static inline struct sock *netlink_lookup(struct net *net, int protocol,
u32 pid)
{
@@ -234,9 +175,9 @@ static inline struct sock *netlink_looku
struct sock *sk;
struct hlist_node *node;
- read_lock(&nl_table_lock);
+ rcu_read_lock();
head = nl_pid_hashfn(hash, pid);
- sk_for_each(sk, node, head) {
+ sk_for_each_rcu(sk, node, head) {
if (net_eq(sock_net(sk), net) && (nlk_sk(sk)->pid == pid)) {
sock_hold(sk);
goto found;
@@ -244,7 +185,7 @@ static inline struct sock *netlink_looku
}
sk = NULL;
found:
- read_unlock(&nl_table_lock);
+ rcu_read_unlock();
return sk;
}
@@ -353,7 +294,7 @@ static int netlink_insert(struct sock *s
struct hlist_node *node;
int len;
- netlink_table_grab();
+ mutex_lock(&nltable_mutex);
head = nl_pid_hashfn(hash, pid);
len = 0;
sk_for_each(osk, node, head) {
@@ -380,18 +321,18 @@ static int netlink_insert(struct sock *s
err = 0;
err:
- netlink_table_ungrab();
+ mutex_unlock(&nltable_mutex);
return err;
}
static void netlink_remove(struct sock *sk)
{
- netlink_table_grab();
+ mutex_lock(&nltable_mutex);
if (sk_del_node_init(sk))
nl_table[sk->sk_protocol].hash.entries--;
if (nlk_sk(sk)->subscriptions)
__sk_del_bind_node(sk);
- netlink_table_ungrab();
+ mutex_unlock(&nltable_mutex);
}
static struct proto netlink_proto = {
@@ -444,12 +385,14 @@ static int netlink_create(struct net *ne
if (protocol < 0 || protocol >= MAX_LINKS)
return -EPROTONOSUPPORT;
- netlink_lock_table();
+ mutex_lock(&nltable_mutex);
#ifdef CONFIG_MODULES
if (!nl_table[protocol].registered) {
- netlink_unlock_table();
+ mutex_unlock(&nltable_mutex);
+
request_module("net-pf-%d-proto-%d", PF_NETLINK, protocol);
- netlink_lock_table();
+
+ mutex_lock(&nltable_mutex);
}
#endif
if (nl_table[protocol].registered &&
@@ -458,7 +401,7 @@ static int netlink_create(struct net *ne
else
err = -EPROTONOSUPPORT;
cb_mutex = nl_table[protocol].cb_mutex;
- netlink_unlock_table();
+ mutex_unlock(&nltable_mutex);
if (err < 0)
goto out;
@@ -515,7 +458,7 @@ static int netlink_release(struct socket
module_put(nlk->module);
- netlink_table_grab();
+ mutex_lock(&nltable_mutex);
if (netlink_is_kernel(sk)) {
BUG_ON(nl_table[sk->sk_protocol].registered == 0);
if (--nl_table[sk->sk_protocol].registered == 0) {
@@ -525,7 +468,7 @@ static int netlink_release(struct socket
}
} else if (nlk->subscriptions)
netlink_update_listeners(sk);
- netlink_table_ungrab();
+ mutex_unlock(&nltable_mutex);
kfree(nlk->groups);
nlk->groups = NULL;
@@ -533,6 +476,8 @@ static int netlink_release(struct socket
local_bh_disable();
sock_prot_inuse_add(sock_net(sk), &netlink_proto, -1);
local_bh_enable();
+
+ synchronize_rcu();
sock_put(sk);
return 0;
}
@@ -551,7 +496,7 @@ static int netlink_autobind(struct socke
retry:
cond_resched();
- netlink_table_grab();
+ mutex_lock(&nltable_mutex);
head = nl_pid_hashfn(hash, pid);
sk_for_each(osk, node, head) {
if (!net_eq(sock_net(osk), net))
@@ -561,11 +506,11 @@ retry:
pid = rover--;
if (rover > -4097)
rover = -4097;
- netlink_table_ungrab();
+ mutex_unlock(&nltable_mutex);
goto retry;
}
}
- netlink_table_ungrab();
+ mutex_unlock(&nltable_mutex);
err = netlink_insert(sk, net, pid);
if (err == -EADDRINUSE)
@@ -603,7 +548,7 @@ static int netlink_realloc_groups(struct
unsigned long *new_groups;
int err = 0;
- netlink_table_grab();
+ mutex_lock(&nltable_mutex);
groups = nl_table[sk->sk_protocol].groups;
if (!nl_table[sk->sk_protocol].registered) {
@@ -625,7 +570,7 @@ static int netlink_realloc_groups(struct
nlk->groups = new_groups;
nlk->ngroups = groups;
out_unlock:
- netlink_table_ungrab();
+ mutex_unlock(&nltable_mutex);
return err;
}
@@ -664,13 +609,13 @@ static int netlink_bind(struct socket *s
if (!nladdr->nl_groups && (nlk->groups == NULL || !(u32)nlk->groups[0]))
return 0;
- netlink_table_grab();
+ mutex_lock(&nltable_mutex);
netlink_update_subscriptions(sk, nlk->subscriptions +
hweight32(nladdr->nl_groups) -
hweight32(nlk->groups[0]));
nlk->groups[0] = (nlk->groups[0] & ~0xffffffffUL) | nladdr->nl_groups;
netlink_update_listeners(sk);
- netlink_table_ungrab();
+ mutex_unlock(&nltable_mutex);
return 0;
}
@@ -1059,15 +1004,12 @@ int netlink_broadcast(struct sock *ssk,
/* While we sleep in clone, do not allow to change socket list */
- netlink_lock_table();
-
- sk_for_each_bound(sk, node, &nl_table[ssk->sk_protocol].mc_list)
+ rcu_read_lock();
+ sk_for_each_bound_rcu(sk, node, &nl_table[ssk->sk_protocol].mc_list)
do_one_broadcast(sk, &info);
+ rcu_read_unlock();
kfree_skb(skb);
-
- netlink_unlock_table();
-
kfree_skb(info.skb2);
if (info.delivery_failure)
@@ -1129,12 +1071,12 @@ void netlink_set_err(struct sock *ssk, u
/* sk->sk_err wants a positive error value */
info.code = -code;
- read_lock(&nl_table_lock);
+ rcu_read_lock();
- sk_for_each_bound(sk, node, &nl_table[ssk->sk_protocol].mc_list)
+ sk_for_each_bound_rcu(sk, node, &nl_table[ssk->sk_protocol].mc_list)
do_one_set_err(sk, &info);
- read_unlock(&nl_table_lock);
+ rcu_read_unlock();
}
EXPORT_SYMBOL(netlink_set_err);
@@ -1187,10 +1129,10 @@ static int netlink_setsockopt(struct soc
return err;
if (!val || val - 1 >= nlk->ngroups)
return -EINVAL;
- netlink_table_grab();
+ mutex_lock(&nltable_mutex);
netlink_update_socket_mc(nlk, val,
optname == NETLINK_ADD_MEMBERSHIP);
- netlink_table_ungrab();
+ mutex_unlock(&nltable_mutex);
err = 0;
break;
}
@@ -1515,7 +1457,7 @@ netlink_kernel_create(struct net *net, i
nlk = nlk_sk(sk);
nlk->flags |= NETLINK_KERNEL_SOCKET;
- netlink_table_grab();
+ mutex_lock(&nltable_mutex);
if (!nl_table[unit].registered) {
nl_table[unit].groups = groups;
nl_table[unit].listeners = listeners;
@@ -1526,7 +1468,7 @@ netlink_kernel_create(struct net *net, i
kfree(listeners);
nl_table[unit].registered++;
}
- netlink_table_ungrab();
+ mutex_unlock(&nltable_mutex);
return sk;
out_sock_release:
@@ -1557,7 +1499,7 @@ static void netlink_free_old_listeners(s
kfree(lrh->ptr);
}
-int __netlink_change_ngroups(struct sock *sk, unsigned int groups)
+static int __netlink_change_ngroups(struct sock *sk, unsigned int groups)
{
unsigned long *listeners, *old = NULL;
struct listeners_rcu_head *old_rcu_head;
@@ -1608,14 +1550,14 @@ int netlink_change_ngroups(struct sock *
{
int err;
- netlink_table_grab();
+ mutex_lock(&nltable_mutex);
err = __netlink_change_ngroups(sk, groups);
- netlink_table_ungrab();
+ mutex_unlock(&nltable_mutex);
return err;
}
-void __netlink_clear_multicast_users(struct sock *ksk, unsigned int group)
+static void __netlink_clear_multicast_users(struct sock *ksk, unsigned int group)
{
struct sock *sk;
struct hlist_node *node;
@@ -1635,9 +1577,9 @@ void __netlink_clear_multicast_users(str
*/
void netlink_clear_multicast_users(struct sock *ksk, unsigned int group)
{
- netlink_table_grab();
+ mutex_lock(&nltable_mutex);
__netlink_clear_multicast_users(ksk, group);
- netlink_table_ungrab();
+ mutex_unlock(&nltable_mutex);
}
void netlink_set_nonroot(int protocol, unsigned int flags)
@@ -1902,7 +1844,7 @@ static struct sock *netlink_seq_socket_i
struct nl_pid_hash *hash = &nl_table[i].hash;
for (j = 0; j <= hash->mask; j++) {
- sk_for_each(s, node, &hash->table[j]) {
+ sk_for_each_rcu(s, node, &hash->table[j]) {
if (sock_net(s) != seq_file_net(seq))
continue;
if (off == pos) {
@@ -1918,9 +1860,9 @@ static struct sock *netlink_seq_socket_i
}
static void *netlink_seq_start(struct seq_file *seq, loff_t *pos)
- __acquires(nl_table_lock)
+ __acquires(RCU)
{
- read_lock(&nl_table_lock);
+ rcu_read_lock();
return *pos ? netlink_seq_socket_idx(seq, *pos - 1) : SEQ_START_TOKEN;
}
@@ -1967,9 +1909,9 @@ static void *netlink_seq_next(struct seq
}
static void netlink_seq_stop(struct seq_file *seq, void *v)
- __releases(nl_table_lock)
+ __releases(RCU)
{
- read_unlock(&nl_table_lock);
+ rcu_read_unlock();
}
--- a/include/net/sock.h 2010-03-08 10:52:34.113924084 -0800
+++ b/include/net/sock.h 2010-03-08 10:52:50.222986495 -0800
@@ -507,6 +507,8 @@ static __inline__ void sk_add_bind_node(
hlist_for_each_entry_safe(__sk, node, tmp, list, sk_node)
#define sk_for_each_bound(__sk, node, list) \
hlist_for_each_entry(__sk, node, list, sk_bind_node)
+#define sk_for_each_bound_rcu(__sk, node, list) \
+ hlist_for_each_entry_rcu(__sk, node, list, sk_bind_node)
/* Sock flags */
enum sock_flags {
--- a/include/linux/netlink.h 2010-03-08 10:56:35.314235687 -0800
+++ b/include/linux/netlink.h 2010-03-08 11:04:33.414547616 -0800
@@ -170,18 +170,13 @@ struct netlink_skb_parms {
#define NETLINK_CREDS(skb) (&NETLINK_CB((skb)).creds)
-extern void netlink_table_grab(void);
-extern void netlink_table_ungrab(void);
-
extern struct sock *netlink_kernel_create(struct net *net,
int unit,unsigned int groups,
void (*input)(struct sk_buff *skb),
struct mutex *cb_mutex,
struct module *module);
extern void netlink_kernel_release(struct sock *sk);
-extern int __netlink_change_ngroups(struct sock *sk, unsigned int groups);
extern int netlink_change_ngroups(struct sock *sk, unsigned int groups);
-extern void __netlink_clear_multicast_users(struct sock *sk, unsigned int group);
extern void netlink_clear_multicast_users(struct sock *sk, unsigned int group);
extern void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err);
extern int netlink_has_listeners(struct sock *sk, unsigned int group);
--- a/net/netlink/genetlink.c 2010-03-08 10:56:02.194547526 -0800
+++ b/net/netlink/genetlink.c 2010-03-08 11:01:05.865177057 -0800
@@ -168,10 +168,9 @@ int genl_register_mc_group(struct genl_f
if (family->netnsok) {
struct net *net;
- netlink_table_grab();
rcu_read_lock();
for_each_net_rcu(net) {
- err = __netlink_change_ngroups(net->genl_sock,
+ err = netlink_change_ngroups(net->genl_sock,
mc_groups_longs * BITS_PER_LONG);
if (err) {
/*
@@ -181,12 +180,10 @@ int genl_register_mc_group(struct genl_f
* increased on some sockets which is ok.
*/
rcu_read_unlock();
- netlink_table_ungrab();
goto out;
}
}
rcu_read_unlock();
- netlink_table_ungrab();
} else {
err = netlink_change_ngroups(init_net.genl_sock,
mc_groups_longs * BITS_PER_LONG);
@@ -212,12 +209,10 @@ static void __genl_unregister_mc_group(s
struct net *net;
BUG_ON(grp->family != family);
- netlink_table_grab();
rcu_read_lock();
for_each_net_rcu(net)
- __netlink_clear_multicast_users(net->genl_sock, grp->id);
+ netlink_clear_multicast_users(net->genl_sock, grp->id);
rcu_read_unlock();
- netlink_table_ungrab();
clear_bit(grp->id, mc_groups);
list_del(&grp->list);
^ permalink raw reply
* Re: [net-next-2.6 PATCH] bonding: refuse to change bond type if it's used
From: Jiri Pirko @ 2010-03-08 21:35 UTC (permalink / raw)
To: David Miller; +Cc: shemminger, netdev, fubar, bonding-devel
In-Reply-To: <20100308.121632.232613075.davem@davemloft.net>
Mon, Mar 08, 2010 at 09:16:32PM CET, davem@davemloft.net wrote:
>From: Stephen Hemminger <shemminger@linux-foundation.org>
>Date: Mon, 8 Mar 2010 10:24:48 -0800
>
>> On Mon, 8 Mar 2010 18:54:06 +0100
>> Jiri Pirko <jpirko@redhat.com> wrote:
>>
>>> It's not desirable to be able to change the type of net_device in bond device if
>>> it's in use by bridge, or vlan, or so. At the moment, there is possible for
>>> example to have INFINIBAND bond type in bridge (by adding bond with eth type to
>>> a bridge first and then enslave INFINIBAND device).
>>
>> Rather than building lots of back pointer dependencies, why not
>> have another netdevice notifier that allows other subsystems to
>> see the type change and reject it if they care? That way the code
>> would be more modular and expandable.
>
>Agreed.
Fair enough, I will rework this.
Thanks a lot guys for looking at this.
Jirka
^ permalink raw reply
* Re: [RFC][PATCH] ns: Syscalls for better namespace sharing control.
From: Serge E. Hallyn @ 2010-03-08 21:49 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Daniel Lezcano, Pavel Emelyanov, Sukadev Bhattiprolu,
Linux Netdev List, containers, Netfilter Development Mailinglist,
Ben Greear
In-Reply-To: <m1lje2qzf4.fsf@fess.ebiederm.org>
Quoting Eric W. Biederman (ebiederm@xmission.com):
> Daniel Lezcano <daniel.lezcano@free.fr> writes:
> I guess my meaning is I was expecting.
> child = fork();
> if (child == 0) {
> execve(...);
> }
> waitpid(child);
>
> This puts /bin/sh in the container as well.
>
> I'm not certain about the /proc/self thing I have never encountered that.
> But I guess if your pid is outside of the pid namespace of that instance
> of proc /proc/self will be a broken symlink.
>
> Eric
Hmm, worse than a broken symlink, will it be a wrong symlink if just
the right pid is created in the container?
-serge
^ permalink raw reply
* [PATCH] netlink: convert DIY reader/writer to mutex and RCU (v2)
From: Stephen Hemminger @ 2010-03-08 22:00 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David Miller, netdev
In-Reply-To: <20100308133211.4f157e2d@nehalam>
The netlink table locking was open coded version of reader/writer
sleeping lock. Change to using mutex and RCU which makes
code clearer, shorter, and simpler.
Could use sk_list nulls but then would have to have kmem_cache
for netlink handles and that seems like unnecessary bloat.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
---
v1 -> v2 do RCU correctly...
* use spinlock not mutex (not safe to sleep in normal RCU)
* use _rcu variants of add/delete
include/linux/netlink.h | 5 -
include/net/sock.h | 12 +++
net/netlink/af_netlink.c | 161 +++++++++++++++--------------------------------
net/netlink/genetlink.c | 9 --
4 files changed, 66 insertions(+), 121 deletions(-)
--- a/net/netlink/af_netlink.c 2010-03-08 09:07:15.104547875 -0800
+++ b/net/netlink/af_netlink.c 2010-03-08 13:43:46.026097658 -0800
@@ -128,15 +128,11 @@ struct netlink_table {
};
static struct netlink_table *nl_table;
-
-static DECLARE_WAIT_QUEUE_HEAD(nl_table_wait);
+static DEFINE_SPINLOCK(nltable_lock);
static int netlink_dump(struct sock *sk);
static void netlink_destroy_callback(struct netlink_callback *cb);
-static DEFINE_RWLOCK(nl_table_lock);
-static atomic_t nl_table_users = ATOMIC_INIT(0);
-
static ATOMIC_NOTIFIER_HEAD(netlink_chain);
static u32 netlink_group_mask(u32 group)
@@ -171,61 +167,6 @@ static void netlink_sock_destruct(struct
WARN_ON(nlk_sk(sk)->groups);
}
-/* This lock without WQ_FLAG_EXCLUSIVE is good on UP and it is _very_ bad on
- * SMP. Look, when several writers sleep and reader wakes them up, all but one
- * immediately hit write lock and grab all the cpus. Exclusive sleep solves
- * this, _but_ remember, it adds useless work on UP machines.
- */
-
-void netlink_table_grab(void)
- __acquires(nl_table_lock)
-{
- might_sleep();
-
- write_lock_irq(&nl_table_lock);
-
- if (atomic_read(&nl_table_users)) {
- DECLARE_WAITQUEUE(wait, current);
-
- add_wait_queue_exclusive(&nl_table_wait, &wait);
- for (;;) {
- set_current_state(TASK_UNINTERRUPTIBLE);
- if (atomic_read(&nl_table_users) == 0)
- break;
- write_unlock_irq(&nl_table_lock);
- schedule();
- write_lock_irq(&nl_table_lock);
- }
-
- __set_current_state(TASK_RUNNING);
- remove_wait_queue(&nl_table_wait, &wait);
- }
-}
-
-void netlink_table_ungrab(void)
- __releases(nl_table_lock)
-{
- write_unlock_irq(&nl_table_lock);
- wake_up(&nl_table_wait);
-}
-
-static inline void
-netlink_lock_table(void)
-{
- /* read_lock() synchronizes us to netlink_table_grab */
-
- read_lock(&nl_table_lock);
- atomic_inc(&nl_table_users);
- read_unlock(&nl_table_lock);
-}
-
-static inline void
-netlink_unlock_table(void)
-{
- if (atomic_dec_and_test(&nl_table_users))
- wake_up(&nl_table_wait);
-}
-
static inline struct sock *netlink_lookup(struct net *net, int protocol,
u32 pid)
{
@@ -234,9 +175,9 @@ static inline struct sock *netlink_looku
struct sock *sk;
struct hlist_node *node;
- read_lock(&nl_table_lock);
+ rcu_read_lock();
head = nl_pid_hashfn(hash, pid);
- sk_for_each(sk, node, head) {
+ sk_for_each_rcu(sk, node, head) {
if (net_eq(sock_net(sk), net) && (nlk_sk(sk)->pid == pid)) {
sock_hold(sk);
goto found;
@@ -244,7 +185,7 @@ static inline struct sock *netlink_looku
}
sk = NULL;
found:
- read_unlock(&nl_table_lock);
+ rcu_read_unlock();
return sk;
}
@@ -299,7 +240,8 @@ static int nl_pid_hash_rehash(struct nl_
struct hlist_node *node, *tmp;
sk_for_each_safe(sk, node, tmp, &otable[i])
- __sk_add_node(sk, nl_pid_hashfn(hash, nlk_sk(sk)->pid));
+ __sk_add_node_rcu(sk,
+ nl_pid_hashfn(hash, nlk_sk(sk)->pid));
}
nl_pid_hash_free(otable, osize);
@@ -353,7 +295,7 @@ static int netlink_insert(struct sock *s
struct hlist_node *node;
int len;
- netlink_table_grab();
+ spin_lock(&nltable_lock);
head = nl_pid_hashfn(hash, pid);
len = 0;
sk_for_each(osk, node, head) {
@@ -376,22 +318,22 @@ static int netlink_insert(struct sock *s
head = nl_pid_hashfn(hash, pid);
hash->entries++;
nlk_sk(sk)->pid = pid;
- sk_add_node(sk, head);
+ sk_add_node_rcu(sk, head);
err = 0;
err:
- netlink_table_ungrab();
+ spin_unlock(&nltable_lock);
return err;
}
static void netlink_remove(struct sock *sk)
{
- netlink_table_grab();
+ spin_lock(&nltable_lock);
if (sk_del_node_init(sk))
nl_table[sk->sk_protocol].hash.entries--;
if (nlk_sk(sk)->subscriptions)
- __sk_del_bind_node(sk);
- netlink_table_ungrab();
+ __sk_del_bind_node_rcu(sk);
+ spin_unlock(&nltable_lock);
}
static struct proto netlink_proto = {
@@ -444,12 +386,14 @@ static int netlink_create(struct net *ne
if (protocol < 0 || protocol >= MAX_LINKS)
return -EPROTONOSUPPORT;
- netlink_lock_table();
+ spin_lock(&nltable_lock);
#ifdef CONFIG_MODULES
if (!nl_table[protocol].registered) {
- netlink_unlock_table();
+ spin_unlock(&nltable_lock);
+
request_module("net-pf-%d-proto-%d", PF_NETLINK, protocol);
- netlink_lock_table();
+
+ spin_lock(&nltable_lock);
}
#endif
if (nl_table[protocol].registered &&
@@ -458,7 +402,7 @@ static int netlink_create(struct net *ne
else
err = -EPROTONOSUPPORT;
cb_mutex = nl_table[protocol].cb_mutex;
- netlink_unlock_table();
+ spin_unlock(&nltable_lock);
if (err < 0)
goto out;
@@ -515,7 +459,7 @@ static int netlink_release(struct socket
module_put(nlk->module);
- netlink_table_grab();
+ spin_lock(&nltable_lock);
if (netlink_is_kernel(sk)) {
BUG_ON(nl_table[sk->sk_protocol].registered == 0);
if (--nl_table[sk->sk_protocol].registered == 0) {
@@ -525,7 +469,7 @@ static int netlink_release(struct socket
}
} else if (nlk->subscriptions)
netlink_update_listeners(sk);
- netlink_table_ungrab();
+ spin_unlock(&nltable_lock);
kfree(nlk->groups);
nlk->groups = NULL;
@@ -533,6 +477,8 @@ static int netlink_release(struct socket
local_bh_disable();
sock_prot_inuse_add(sock_net(sk), &netlink_proto, -1);
local_bh_enable();
+
+ synchronize_rcu();
sock_put(sk);
return 0;
}
@@ -551,7 +497,7 @@ static int netlink_autobind(struct socke
retry:
cond_resched();
- netlink_table_grab();
+ spin_lock(&nltable_lock);
head = nl_pid_hashfn(hash, pid);
sk_for_each(osk, node, head) {
if (!net_eq(sock_net(osk), net))
@@ -561,11 +507,11 @@ retry:
pid = rover--;
if (rover > -4097)
rover = -4097;
- netlink_table_ungrab();
+ spin_unlock(&nltable_lock);
goto retry;
}
}
- netlink_table_ungrab();
+ spin_unlock(&nltable_lock);
err = netlink_insert(sk, net, pid);
if (err == -EADDRINUSE)
@@ -590,9 +536,9 @@ netlink_update_subscriptions(struct sock
struct netlink_sock *nlk = nlk_sk(sk);
if (nlk->subscriptions && !subscriptions)
- __sk_del_bind_node(sk);
+ __sk_del_bind_node_rcu(sk);
else if (!nlk->subscriptions && subscriptions)
- sk_add_bind_node(sk, &nl_table[sk->sk_protocol].mc_list);
+ sk_add_bind_node_rcu(sk, &nl_table[sk->sk_protocol].mc_list);
nlk->subscriptions = subscriptions;
}
@@ -603,7 +549,7 @@ static int netlink_realloc_groups(struct
unsigned long *new_groups;
int err = 0;
- netlink_table_grab();
+ spin_lock(&nltable_lock);
groups = nl_table[sk->sk_protocol].groups;
if (!nl_table[sk->sk_protocol].registered) {
@@ -625,7 +571,7 @@ static int netlink_realloc_groups(struct
nlk->groups = new_groups;
nlk->ngroups = groups;
out_unlock:
- netlink_table_ungrab();
+ spin_unlock(&nltable_lock);
return err;
}
@@ -664,13 +610,13 @@ static int netlink_bind(struct socket *s
if (!nladdr->nl_groups && (nlk->groups == NULL || !(u32)nlk->groups[0]))
return 0;
- netlink_table_grab();
+ spin_lock(&nltable_lock);
netlink_update_subscriptions(sk, nlk->subscriptions +
hweight32(nladdr->nl_groups) -
hweight32(nlk->groups[0]));
nlk->groups[0] = (nlk->groups[0] & ~0xffffffffUL) | nladdr->nl_groups;
netlink_update_listeners(sk);
- netlink_table_ungrab();
+ spin_unlock(&nltable_lock);
return 0;
}
@@ -1059,15 +1005,12 @@ int netlink_broadcast(struct sock *ssk,
/* While we sleep in clone, do not allow to change socket list */
- netlink_lock_table();
-
- sk_for_each_bound(sk, node, &nl_table[ssk->sk_protocol].mc_list)
+ rcu_read_lock();
+ sk_for_each_bound_rcu(sk, node, &nl_table[ssk->sk_protocol].mc_list)
do_one_broadcast(sk, &info);
+ rcu_read_unlock();
kfree_skb(skb);
-
- netlink_unlock_table();
-
kfree_skb(info.skb2);
if (info.delivery_failure)
@@ -1129,12 +1072,12 @@ void netlink_set_err(struct sock *ssk, u
/* sk->sk_err wants a positive error value */
info.code = -code;
- read_lock(&nl_table_lock);
+ rcu_read_lock();
- sk_for_each_bound(sk, node, &nl_table[ssk->sk_protocol].mc_list)
+ sk_for_each_bound_rcu(sk, node, &nl_table[ssk->sk_protocol].mc_list)
do_one_set_err(sk, &info);
- read_unlock(&nl_table_lock);
+ rcu_read_unlock();
}
EXPORT_SYMBOL(netlink_set_err);
@@ -1187,10 +1130,10 @@ static int netlink_setsockopt(struct soc
return err;
if (!val || val - 1 >= nlk->ngroups)
return -EINVAL;
- netlink_table_grab();
+ spin_lock(&nltable_lock);
netlink_update_socket_mc(nlk, val,
optname == NETLINK_ADD_MEMBERSHIP);
- netlink_table_ungrab();
+ spin_unlock(&nltable_lock);
err = 0;
break;
}
@@ -1515,7 +1458,7 @@ netlink_kernel_create(struct net *net, i
nlk = nlk_sk(sk);
nlk->flags |= NETLINK_KERNEL_SOCKET;
- netlink_table_grab();
+ spin_lock(&nltable_lock);
if (!nl_table[unit].registered) {
nl_table[unit].groups = groups;
nl_table[unit].listeners = listeners;
@@ -1526,7 +1469,7 @@ netlink_kernel_create(struct net *net, i
kfree(listeners);
nl_table[unit].registered++;
}
- netlink_table_ungrab();
+ spin_unlock(&nltable_lock);
return sk;
out_sock_release:
@@ -1557,7 +1500,7 @@ static void netlink_free_old_listeners(s
kfree(lrh->ptr);
}
-int __netlink_change_ngroups(struct sock *sk, unsigned int groups)
+static int __netlink_change_ngroups(struct sock *sk, unsigned int groups)
{
unsigned long *listeners, *old = NULL;
struct listeners_rcu_head *old_rcu_head;
@@ -1608,14 +1551,14 @@ int netlink_change_ngroups(struct sock *
{
int err;
- netlink_table_grab();
+ spin_lock(&nltable_lock);
err = __netlink_change_ngroups(sk, groups);
- netlink_table_ungrab();
+ spin_unlock(&nltable_lock);
return err;
}
-void __netlink_clear_multicast_users(struct sock *ksk, unsigned int group)
+static void __netlink_clear_multicast_users(struct sock *ksk, unsigned int group)
{
struct sock *sk;
struct hlist_node *node;
@@ -1635,9 +1578,9 @@ void __netlink_clear_multicast_users(str
*/
void netlink_clear_multicast_users(struct sock *ksk, unsigned int group)
{
- netlink_table_grab();
+ spin_lock(&nltable_lock);
__netlink_clear_multicast_users(ksk, group);
- netlink_table_ungrab();
+ spin_unlock(&nltable_lock);
}
void netlink_set_nonroot(int protocol, unsigned int flags)
@@ -1902,7 +1845,7 @@ static struct sock *netlink_seq_socket_i
struct nl_pid_hash *hash = &nl_table[i].hash;
for (j = 0; j <= hash->mask; j++) {
- sk_for_each(s, node, &hash->table[j]) {
+ sk_for_each_rcu(s, node, &hash->table[j]) {
if (sock_net(s) != seq_file_net(seq))
continue;
if (off == pos) {
@@ -1918,9 +1861,9 @@ static struct sock *netlink_seq_socket_i
}
static void *netlink_seq_start(struct seq_file *seq, loff_t *pos)
- __acquires(nl_table_lock)
+ __acquires(RCU)
{
- read_lock(&nl_table_lock);
+ rcu_read_lock();
return *pos ? netlink_seq_socket_idx(seq, *pos - 1) : SEQ_START_TOKEN;
}
@@ -1967,9 +1910,9 @@ static void *netlink_seq_next(struct seq
}
static void netlink_seq_stop(struct seq_file *seq, void *v)
- __releases(nl_table_lock)
+ __releases(RCU)
{
- read_unlock(&nl_table_lock);
+ rcu_read_unlock();
}
--- a/include/net/sock.h 2010-03-08 10:52:34.113924084 -0800
+++ b/include/net/sock.h 2010-03-08 13:54:27.815939404 -0800
@@ -451,6 +451,10 @@ static __inline__ void __sk_add_node(str
{
hlist_add_head(&sk->sk_node, list);
}
+static __inline__ void __sk_add_node_rcu(struct sock *sk, struct hlist_head *list)
+{
+ hlist_add_head_rcu(&sk->sk_node, list);
+}
static __inline__ void sk_add_node(struct sock *sk, struct hlist_head *list)
{
@@ -479,12 +483,18 @@ static __inline__ void __sk_del_bind_nod
{
__hlist_del(&sk->sk_bind_node);
}
+#define __sk_del_bind_node_rcu(sk) __sk_del_bind_node(sk)
static __inline__ void sk_add_bind_node(struct sock *sk,
struct hlist_head *list)
{
hlist_add_head(&sk->sk_bind_node, list);
}
+static __inline__ void sk_add_bind_node_rcu(struct sock *sk,
+ struct hlist_head *list)
+{
+ hlist_add_head_rcu(&sk->sk_bind_node, list);
+}
#define sk_for_each(__sk, node, list) \
hlist_for_each_entry(__sk, node, list, sk_node)
@@ -507,6 +517,8 @@ static __inline__ void sk_add_bind_node(
hlist_for_each_entry_safe(__sk, node, tmp, list, sk_node)
#define sk_for_each_bound(__sk, node, list) \
hlist_for_each_entry(__sk, node, list, sk_bind_node)
+#define sk_for_each_bound_rcu(__sk, node, list) \
+ hlist_for_each_entry_rcu(__sk, node, list, sk_bind_node)
/* Sock flags */
enum sock_flags {
--- a/include/linux/netlink.h 2010-03-08 10:56:35.314235687 -0800
+++ b/include/linux/netlink.h 2010-03-08 11:04:33.414547616 -0800
@@ -170,18 +170,13 @@ struct netlink_skb_parms {
#define NETLINK_CREDS(skb) (&NETLINK_CB((skb)).creds)
-extern void netlink_table_grab(void);
-extern void netlink_table_ungrab(void);
-
extern struct sock *netlink_kernel_create(struct net *net,
int unit,unsigned int groups,
void (*input)(struct sk_buff *skb),
struct mutex *cb_mutex,
struct module *module);
extern void netlink_kernel_release(struct sock *sk);
-extern int __netlink_change_ngroups(struct sock *sk, unsigned int groups);
extern int netlink_change_ngroups(struct sock *sk, unsigned int groups);
-extern void __netlink_clear_multicast_users(struct sock *sk, unsigned int group);
extern void netlink_clear_multicast_users(struct sock *sk, unsigned int group);
extern void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err);
extern int netlink_has_listeners(struct sock *sk, unsigned int group);
--- a/net/netlink/genetlink.c 2010-03-08 10:56:02.194547526 -0800
+++ b/net/netlink/genetlink.c 2010-03-08 11:01:05.865177057 -0800
@@ -168,10 +168,9 @@ int genl_register_mc_group(struct genl_f
if (family->netnsok) {
struct net *net;
- netlink_table_grab();
rcu_read_lock();
for_each_net_rcu(net) {
- err = __netlink_change_ngroups(net->genl_sock,
+ err = netlink_change_ngroups(net->genl_sock,
mc_groups_longs * BITS_PER_LONG);
if (err) {
/*
@@ -181,12 +180,10 @@ int genl_register_mc_group(struct genl_f
* increased on some sockets which is ok.
*/
rcu_read_unlock();
- netlink_table_ungrab();
goto out;
}
}
rcu_read_unlock();
- netlink_table_ungrab();
} else {
err = netlink_change_ngroups(init_net.genl_sock,
mc_groups_longs * BITS_PER_LONG);
@@ -212,12 +209,10 @@ static void __genl_unregister_mc_group(s
struct net *net;
BUG_ON(grp->family != family);
- netlink_table_grab();
rcu_read_lock();
for_each_net_rcu(net)
- __netlink_clear_multicast_users(net->genl_sock, grp->id);
+ netlink_clear_multicast_users(net->genl_sock, grp->id);
rcu_read_unlock();
- netlink_table_ungrab();
clear_bit(grp->id, mc_groups);
list_del(&grp->list);
^ permalink raw reply
* Re: [RFC][PATCH] ns: Syscalls for better namespace sharing control.
From: Eric W. Biederman @ 2010-03-08 22:24 UTC (permalink / raw)
To: Serge E. Hallyn
Cc: Daniel Lezcano, Pavel Emelyanov, Sukadev Bhattiprolu,
Linux Netdev List, containers, Netfilter Development Mailinglist,
Ben Greear
In-Reply-To: <20100308214945.GA26617@us.ibm.com>
"Serge E. Hallyn" <serue@us.ibm.com> writes:
> Quoting Eric W. Biederman (ebiederm@xmission.com):
>> Daniel Lezcano <daniel.lezcano@free.fr> writes:
>> I guess my meaning is I was expecting.
>> child = fork();
>> if (child == 0) {
>> execve(...);
>> }
>> waitpid(child);
>>
>> This puts /bin/sh in the container as well.
>>
>> I'm not certain about the /proc/self thing I have never encountered that.
>> But I guess if your pid is outside of the pid namespace of that instance
>> of proc /proc/self will be a broken symlink.
>>
>> Eric
>
> Hmm, worse than a broken symlink, will it be a wrong symlink if just
> the right pid is created in the container?
It won't happen. readlink and followlink are both based on
task_tgid_nr_ns(current, ns_of_proc).
Which fails if your process is not known in that pid namespace.
Eric
^ permalink raw reply
* RE: [PATCH] tipc: fix endianness on tipc subscriber messages
From: Stephens, Allan @ 2010-03-08 22:32 UTC (permalink / raw)
To: Neil Horman; +Cc: davem, netdev
In-Reply-To: <20100308210957.GH23634@hmsreliant.think-freely.org>
Neil wrote:
> > There are a couple of problems with this patch that need to be
> > resolved before it can be applied to the upstream kernel.
> >
> > 1) Neil's replacement of the htohl() call with the equivalent
> > htonl()/ntohl() calls, while well intentioned, was done without
> > understanding why the htohl() calls were put there in the
> first place.
> I'd love to hear what those reasons are. You're formating
> on-the-wire data to an endianness defined by the receiving
> entity. I'm hard pressed to imagine a reason why thats sane.
When TIPC evolved from using ioctl() as the API to the topology service
to a message-based interface it was felt that it would be desirable to
allow the fields in those messages to be formatted in host-byte order,
since that was what existing applications were already doing. I presume
that the idea was to make the migration process as painless as possible,
and to avoid problems with future applications that didn't take
endianness into account.
However, since it was theoretically possible for an application to send
a message to a topology service on another node (whose endianness was
unknown), the node that received the message needed to be able to
recognize if a message contents had the "wrong" endianness and do the
necessary conversion. That is what was implemented back in TIPC 1.5,
and this API still exists today which is why the htohl() conversions are
done the way they are. (I can't say I'm a fan of the htohl() name ...
it would be nice to have something more meaningful.)
The difficulty with the patch you've made is that it will break existing
TIPC applications that adhere to the existing API (i.e. they don't do
endianness conversion on the messages they exchange with the topology
server) if the application is running on an architecture whose host byte
order isn't the same as network byte order.
> > In addition, the TIPC specification that he used to justify some of
> > his decisions is out-dated, and doesn't reflect the latest
> version of
> > the TIPC protocol. (I'll discuss this further in a
> follow-up email.)
> >
> Please, point out a more recent spec. The one I referenced
> is the most recent publically available spec that I can find.
> although again, I'm hard pressed to believe that the spec has
> changed to include a requirement to send data in receiver byte order.
The TIPC spec you're referencing was created a number of years ago when
Ericsson was endeavouring to get TIPC accepted by the IETF. That effort
was unsuccessful, and when that happened Ericsson decided not to pursue
things further and let the spec become stale. The document was kept on
the SourceForge website out of historical interest, and its description
was updated to state the spec wasn't a reliable indication of the
current state of the code. The TIPC working group has continued
development of TIPC by letting the code "do the talking".
> > 2) Neil's alteration of the memcpy() in the subscription
> cancelation
> > routine is simply wrong. The pieces of the data structure that he
> > claims are local are not local, and must be checked to
> ensure that the
> > cancellation is done properly.
> >
>
> The origional memcmp checks sizeof(tipc_subscr) bytes of each
> entry in the array. tipc_subscr is defined as:
> struct tipc_subscr {
> struct tipc_name_seq seq; /* name sequence of
> interest */
> __u32 timeout; /* subscription
> duration (in ms) */
> __u32 filter; /* bitmask of filter
> options */
> char usr_handle[8]; /* available for
> subscriber use */
> };
>
> In subscr_subscribe (from which we also call subscr_cancel),
> we allocate new entries for that list, only fillingout the
> seq.type, seq.lower, seq.upper, timeout and filter. So if
> anyone locally changes usr_handle (which I see several places
> that do), the memcmp won't match up properly. Additionally,
> I'm hard pressed to believe (and this would be supported by
> the section of the spec that I referenced), timeout, and
> filter also don't bear on the uniqueness of the subscriber
> Id. Of course a subsequent version of the spec might have
> changed that, But if it is, please point it out to me.
When a subscription request message is received, a new subscription
object is created (struct subscription). This routine copies over the
sequence, timeout, and filter fields from the original message, and
converts them to host byte order so they can be easily referenced later;
it doesn't copy over the usr_handle info because that is only meaningful
to the application that sent the request. However, in addition, the
subscription object also copies the subscription request message (struct
tipc_subscr), which contains all of these fields in the sender's byte
order, including the usr_handle; this is done by the memcpy near the end
of subscr_subscribe().
Now when subscr_cancel() is invoked to try cancelling an existing
subscription, the caller passes in the cancellation message structure
(struct tipc_subscr) with the cancellation request bit forced off. The
routine can then do a memcmp() against the copy of the original
subscription request (also struct tipc_subscr), ensuring that a match is
declared only if all of the fields specified in the cancellation message
match the fields in the original request.
Regards,
Al
^ permalink raw reply
* Re: [PATCH] tipc: fix endianness on tipc subscriber messages
From: David Miller @ 2010-03-08 23:38 UTC (permalink / raw)
To: allan.stephens; +Cc: nhorman, netdev
In-Reply-To: <20100308.125429.14323494.davem@davemloft.net>
Allan, I don't think you're being reasonable at all.
You tell Neil he's breaking things, and that his code isn't following
the current spec.
Neil asks where the spec is.
And you tell him your out-of-tree code is the spec.
That's absolutely rediculious. It is totally unreasonable to require
Neil to unravel the mess that is the out-of-tree TIPC implementation
in order to figure out what the current protocol "spec" is.
You're doing a lot of "oh crap, don't do this, you'll break this or
that." But frankly, you really don't care what's in the upstream
kernel. If you did, your stuff wouldn't be out of tree.
Therefore I take every piece of feedback you give to Neil's
patches with a grain of salt, at best.
^ permalink raw reply
* Re: [PATCH] vsprintf.c: Use noinline_for_stack
From: Joe Perches @ 2010-03-08 23:39 UTC (permalink / raw)
To: Linus Torvalds
Cc: Andrew Morton, Nick Andrew, Linux Kernel Mailing List,
Greg Kroah-Hartman, netdev, Bjorn Helgaas, Stephen Rothwell
In-Reply-To: <1267929215.1937.18.camel@Joe-Laptop.home>
On Sat, 2010-03-06 at 18:33 -0800, Joe Perches wrote:
> On Sat, 2010-03-06 at 18:03 -0800, Linus Torvalds wrote:
> > A few noinlines might be appropriate.
> Mark static functions with noinline_for_stack
It's fine by me that the vsnprintf recursion and
(pr|dev|netdev)_<level> text reduction patches didn't make .34.
I'd like to know what you think necessary to get them into .35.
Perhaps it'd be useful if they could go into -next as-is for a
bit of wider testing.
http://patchwork.kernel.org/patch/83940/
http://patchwork.kernel.org/patch/83724/
http://patchwork.kernel.org/patch/83726/
http://patchwork.kernel.org/patch/83725/
^ permalink raw reply
* [PATCH] ar9170: fix for driver-core ABI change
From: Stephen Rothwell @ 2010-03-08 23:42 UTC (permalink / raw)
To: David Miller, netdev; +Cc: John W. Linville, LKML, linux-next
Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
drivers/net/wireless/ath/ar9170/usb.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
Hi Dave,
I have been carrying this patch in linux-next for some time as a fixup
for the driver-core tree. It is now needed in net-2.6 (i.e. my
net-current) (since you merged the wireless tree) *after* you merge with
Linus' tree i.e. the down()/up() are present in the net-2.6 tree and the
changes to the driver core to use mutexes are in Linus' tree.
diff --git a/drivers/net/wireless/ath/ar9170/usb.c b/drivers/net/wireless/ath/ar9170/usb.c
index 4e30197..7fc1034 100644
--- a/drivers/net/wireless/ath/ar9170/usb.c
+++ b/drivers/net/wireless/ath/ar9170/usb.c
@@ -41,6 +41,7 @@
#include <linux/usb.h>
#include <linux/firmware.h>
#include <linux/etherdevice.h>
+#include <linux/mutex.h>
#include <net/mac80211.h>
#include "ar9170.h"
#include "cmd.h"
@@ -726,10 +727,10 @@ static void ar9170_usb_firmware_failed(struct ar9170_usb *aru)
/* unbind anything failed */
if (parent)
- down(&parent->sem);
+ device_lock(parent);
device_release_driver(&aru->udev->dev);
if (parent)
- up(&parent->sem);
+ device_unlock(parent);
}
static void ar9170_usb_firmware_finish(const struct firmware *fw, void *context)
--
1.7.0
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/
^ permalink raw reply related
* [PATCH] net/netfilter/ipvs/ip_vs_ftp.c: Remove use of NIPQUAD
From: Joe Perches @ 2010-03-09 0:31 UTC (permalink / raw)
To: Wensong Zhang, Simon Horman, Julian Anastasov, Patrick McHardy,
David S. Miller
Cc: netdev, lvs-devel, LKML
NIPQUAD has very few uses left.
Remove this use and make the code have the identical form of the only
other use of "%u,%u,%u,%u,%u,%u" in net/ipv4/netfilter/nf_nat_ftp.c
Signed-off-by: Joe Perches <joe@perches.com>
---
net/netfilter/ipvs/ip_vs_ftp.c | 10 ++++++++--
1 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/net/netfilter/ipvs/ip_vs_ftp.c
b/net/netfilter/ipvs/ip_vs_ftp.c
index 73f38ea..9f63283 100644
--- a/net/netfilter/ipvs/ip_vs_ftp.c
+++ b/net/netfilter/ipvs/ip_vs_ftp.c
@@ -208,8 +208,14 @@ static int ip_vs_ftp_out(struct ip_vs_app *app,
struct ip_vs_conn *cp,
*/
from.ip = n_cp->vaddr.ip;
port = n_cp->vport;
- sprintf(buf, "%u,%u,%u,%u,%u,%u", NIPQUAD(from.ip),
- (ntohs(port)>>8)&255, ntohs(port)&255);
+ snprintf(buf, sizeof(buf), "%u,%u,%u,%u,%u,%u",
+ ((unsigned char *)&from.ip)[0],
+ ((unsigned char *)&from.ip)[1],
+ ((unsigned char *)&from.ip)[2],
+ ((unsigned char *)&from.ip)[3],
+ ntohs(port) >> 8,
+ ntohs(port) & 0xFF);
+
buf_len = strlen(buf);
/*
^ permalink raw reply related
* Re: [PATCH] tipc: fix endianness on tipc subscriber messages
From: Neil Horman @ 2010-03-09 1:45 UTC (permalink / raw)
To: Stephens, Allan; +Cc: davem, netdev
In-Reply-To: <29C1DC0826876849BDD9F1C67ABA29430728D22A@ala-mail09.corp.ad.wrs.com>
On Mon, Mar 08, 2010 at 02:32:38PM -0800, Stephens, Allan wrote:
> Neil wrote:
>
> > > There are a couple of problems with this patch that need to be
> > > resolved before it can be applied to the upstream kernel.
> > >
> > > 1) Neil's replacement of the htohl() call with the equivalent
> > > htonl()/ntohl() calls, while well intentioned, was done without
> > > understanding why the htohl() calls were put there in the
> > first place.
> > I'd love to hear what those reasons are. You're formating
> > on-the-wire data to an endianness defined by the receiving
> > entity. I'm hard pressed to imagine a reason why thats sane.
>
> When TIPC evolved from using ioctl() as the API to the topology service
> to a message-based interface it was felt that it would be desirable to
> allow the fields in those messages to be formatted in host-byte order,
> since that was what existing applications were already doing. I presume
> that the idea was to make the migration process as painless as possible,
> and to avoid problems with future applications that didn't take
> endianness into account.
>
So you made a decision to make broken applications continue to work by breaking
the protocol stack, and in so doing violated the specification. Thats wrong.
The right thing to do is fix the stack to conform to the spec, identify the
broken applications, then fix them to conform to the specification.
> However, since it was theoretically possible for an application to send
> a message to a topology service on another node (whose endianness was
> unknown), the node that received the message needed to be able to
> recognize if a message contents had the "wrong" endianness and do the
> necessary conversion. That is what was implemented back in TIPC 1.5,
Thats a big mistake. The right thing to do is to make it clear to the
applications that they need to send everything in network byte order, and assume
everything receive is in network byte order. Then the application doesn't have
to care what the receivers byte order is. Anything less than that is completely
broken. Supporting backwards compatibility with broken applications by further
breaking things is just more broken.
> and this API still exists today which is why the htohl() conversions are
> done the way they are. (I can't say I'm a fan of the htohl() name ...
> it would be nice to have something more meaningful.)
>
htohl is perfectly meaningful, its just wrong.
> The difficulty with the patch you've made is that it will break existing
> TIPC applications that adhere to the existing API (i.e. they don't do
> endianness conversion on the messages they exchange with the topology
> server) if the application is running on an architecture whose host byte
> order isn't the same as network byte order.
>
Well, thats broken, but the fix isn't to allow them to continue to support
munging byte order for them, its to make them fix their own brokenness. You may
not be able to break the API for your customers, but upstream can. I won't
speak for Dave or anyone else on the list, but for me, convincing me this isn't
needed will require a technical argument, not a business one. Show me that this
code doesn't violate the latest draft RFC as published.
>
> > > In addition, the TIPC specification that he used to justify some of
> > > his decisions is out-dated, and doesn't reflect the latest
> > version of
> > > the TIPC protocol. (I'll discuss this further in a
> > follow-up email.)
> > >
> > Please, point out a more recent spec. The one I referenced
> > is the most recent publically available spec that I can find.
> > although again, I'm hard pressed to believe that the spec has
> > changed to include a requirement to send data in receiver byte order.
>
> The TIPC spec you're referencing was created a number of years ago when
> Ericsson was endeavouring to get TIPC accepted by the IETF. That effort
> was unsuccessful, and when that happened Ericsson decided not to pursue
> things further and let the spec become stale. The document was kept on
> the SourceForge website out of historical interest, and its description
> was updated to state the spec wasn't a reliable indication of the
> current state of the code. The TIPC working group has continued
> development of TIPC by letting the code "do the talking".
>
Setting asside how unacceptable I find that, I find a flaw in your logic here.
you assert the code defines the specification, yet from your statements above,
we can't change the behavior of the code (read: we can't change the
specification, because using applications have already assumed the code behaves
the way it currently does). As such, it would seem that we can do little more
with tipc than maintain exact bug-for-bug compatibility, in perpituity, and
thats unacceptable. Either we:
1) Fix the bugs we find, using the latest published specification
2) Drop tipc from the kernel, since we don't really know how it should work
Seriously, if you're tree is the definitive source on how tipc works, drop it
from upstream, maintain it yourself, and save others the grief of having to fix
anything with it.
>
> > > 2) Neil's alteration of the memcpy() in the subscription
> > cancelation
> > > routine is simply wrong. The pieces of the data structure that he
> > > claims are local are not local, and must be checked to
> > ensure that the
> > > cancellation is done properly.
> > >
> >
> > The origional memcmp checks sizeof(tipc_subscr) bytes of each
> > entry in the array. tipc_subscr is defined as:
> > struct tipc_subscr {
> > struct tipc_name_seq seq; /* name sequence of
> > interest */
> > __u32 timeout; /* subscription
> > duration (in ms) */
> > __u32 filter; /* bitmask of filter
> > options */
> > char usr_handle[8]; /* available for
> > subscriber use */
> > };
> >
> > In subscr_subscribe (from which we also call subscr_cancel),
> > we allocate new entries for that list, only fillingout the
> > seq.type, seq.lower, seq.upper, timeout and filter. So if
> > anyone locally changes usr_handle (which I see several places
> > that do), the memcmp won't match up properly. Additionally,
> > I'm hard pressed to believe (and this would be supported by
> > the section of the spec that I referenced), timeout, and
> > filter also don't bear on the uniqueness of the subscriber
> > Id. Of course a subsequent version of the spec might have
> > changed that, But if it is, please point it out to me.
>
> When a subscription request message is received, a new subscription
> object is created (struct subscription). This routine copies over the
> sequence, timeout, and filter fields from the original message, and
> converts them to host byte order so they can be easily referenced later;
> it doesn't copy over the usr_handle info because that is only meaningful
> to the application that sent the request. However, in addition, the
> subscription object also copies the subscription request message (struct
> tipc_subscr), which contains all of these fields in the sender's byte
> order, including the usr_handle; this is done by the memcpy near the end
> of subscr_subscribe().
>
> Now when subscr_cancel() is invoked to try cancelling an existing
> subscription, the caller passes in the cancellation message structure
> (struct tipc_subscr) with the cancellation request bit forced off. The
> routine can then do a memcmp() against the copy of the original
> subscription request (also struct tipc_subscr), ensuring that a match is
> declared only if all of the fields specified in the cancellation message
> match the fields in the original request.
>
Yes, I see that, but where in the spec (section 5.2 IIRC defines the subscriber
data), include the timeout, filter and user handle as components of the
^ permalink raw reply
* Re: [PATCH] ar9170: fix for driver-core ABI change
From: Stephen Rothwell @ 2010-03-09 1:52 UTC (permalink / raw)
To: David Miller, netdev; +Cc: John W. Linville, LKML, linux-next
In-Reply-To: <20100309104221.aedc5e7e.sfr@canb.auug.org.au>
[-- Attachment #1: Type: text/plain, Size: 965 bytes --]
Hi Dave,
On Tue, 9 Mar 2010 10:42:21 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
> drivers/net/wireless/ath/ar9170/usb.c | 5 +++--
> 1 files changed, 3 insertions(+), 2 deletions(-)
>
> I have been carrying this patch in linux-next for some time as a fixup
> for the driver-core tree. It is now needed in net-2.6 (i.e. my
> net-current) (since you merged the wireless tree) *after* you merge with
> Linus' tree i.e. the down()/up() are present in the net-2.6 tree and the
> changes to the driver core to use mutexes are in Linus' tree.
Greg tells me that the semaphore to mutex change didn't go into Linus'
tree. However, this patch can still be applied as the device_lock/unlock
primitives did go in. I guess that the include of mutex.h is not needed.
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/
[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: KS8695: possible NAPI issue
From: figo zhang @ 2010-03-09 1:50 UTC (permalink / raw)
To: Yegor Yefremov; +Cc: Dick Hollenbeck, netdev, zealcook
In-Reply-To: <f69abfc31003080611xde6fe26v150dd1fa90b82928@mail.gmail.com>
2010/3/8 Yegor Yefremov <yegorslists@googlemail.com>:
> This is ping afterwards:
>
> debian:~# ping -c 1 192.168.1.38
>
> PING 192.168.1.38 (192.168.1.38) 56(84) bytes of data.
>
>
> tx ram addr = c371c202 , data len = 62
>
> dst:00:18:f3:fe:84:84 src:00:04:d9:80:94:cd type: 0x0800
=> the print info is not clear, it had better just keep "print_mem()",
and remove other printk.
>
> 0xc371c20e : 45 00
>
> 0xc371c212 : 00 54 00 00 40 00 40 01 b6 f0 c0 a8 01 42 c0 a8
>
> 0xc371c222 : 01 26 08 00 65 51 1a 07 00 01 92 00 95 4b 5f 57
>
> 0xc371c232 : 07 00 08 09 0a 0b 0c 0d 0e 0f 10 11 12 13 14 15
>
> 0xc371c242 : 16 17 18 19 1a 1b 1c 1d 1e 1f 20 21 22 23 24 25
>
> 0xc371c252 : 26 27 28 29 2a 2b 2c 2d 2e 2f 30 31 32 33 34 35
>
> 0xc371c262 : 36 37
>
>
>
> tx ram addr = c371c202 , data len = 2a
>
> dst:00:18:f3:fe:84:84 src:00:04:d9:80:94:cd type: 0x0806
>
> 0xc371c20e : 00 01
>
> 0xc371c212 : 08 00 06 04 00 01 00 04 d9 80 94 cd c0 a8 01 42
=> it is a send arp packet, "c0 a8 01 42" is your curr ip addr
192.168.1.66, tell the other side it's ip addr.
>
> 0xc371c222 : 00 00 00 00 00 00 c0 a8 01 26
>
>
>
> rx ram addr = c35d2020 , data len = 3c
>
> dst:00:04:d9:80:94:cd src:00:18:f3:fe:84:84 type: 0x0806
>
> 0xc35d202c : 00 01
>
> 0xc35d2030 : 08 00 06 04 00 02 00 18 f3 fe 84 84 c0 a8 01 26
=> it is a reply arp packet, "c0 a8 01 26" (192.168.1.38 ) is other
side ip addr. so the arp
request is finished.
>
> 0xc35d2040 : 00 04 d9 80 94 cd c0 a8 01 42 00 00 00 00 00 00
>
> 0xc35d2050 : 00 00 00 00 00 00 00 00 00 00 00 00
>
=> so , it should still send "IMCP" packet, you using "ping -c 1", it
send one IMCP packet , and wait for reply. at this point, it have not
send packet?
^ permalink raw reply
* [PATCH] gaiconf: /etc/gai.conf configuration helper.
From: YOSHIFUJI Hideaki @ 2010-03-09 3:12 UTC (permalink / raw)
To: stephen.hemminger, netdev; +Cc: yoshfuji
This tool reads /etc/gai.conf, configuration for getaddrinfo(3), and
set up kernel parameter.
Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
examples/gaiconf | 134 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 134 insertions(+), 0 deletions(-)
create mode 100644 examples/gaiconf
diff --git a/examples/gaiconf b/examples/gaiconf
new file mode 100644
index 0000000..d75292b
--- /dev/null
+++ b/examples/gaiconf
@@ -0,0 +1,134 @@
+#!/bin/sh
+
+#
+# Setup address label from /etc/gai.conf
+#
+# Written by YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>, 2010.
+#
+
+IP=ip
+DEFAULT_GAICONF=/etc/gai.conf
+verbose=
+debug=
+
+function run ()
+{
+ if [ x"$verbose" != x"" ]; then
+ echo "$@"
+ fi
+ if [ x"$debug" = x"" ]; then
+ "$@"
+ fi
+}
+
+function do_load_config ()
+{
+ file=$1; shift
+ flush=1
+ cat $file | while read command prefix label; do
+ if [ x"$command" = x"#label" ]; then
+ if [ ${flush} = 1 ]; then
+ run ${IP} -6 addrlabel flush
+ flush=0
+ fi
+ run ${IP} -6 addrlabel add prefix $prefix label $label
+ fi
+ done
+}
+
+function do_list_config ()
+{
+ ${IP} -6 addrlabel list | while read p pfx l lbl; do
+ echo label ${pfx} ${lbl}
+ done
+}
+
+function help ()
+{
+ echo "Usage: $0 [-v] {--list | --config [ ${DEFAULT_GAICONF} ] | --default}"
+ exit 1
+}
+
+TEMP=`getopt -o c::dlv -l config::,default,list,verbose -n gaiconf -- "$@"`
+
+if [ $? != 0 ]; then
+ echo "Terminating..." >&2
+ exit 1
+fi
+
+TEMPFILE=`mktemp`
+
+eval set -- "$TEMP"
+
+while true ; do
+ case "$1" in
+ -c|--config)
+ if [ x"$cmd" != x"" ]; then
+ help
+ fi
+ case "$2" in
+ "") gai_conf="${DEFAULT_GAICONF}"
+ shift 2
+ ;;
+ *) gai_conf="$2"
+ shift 2
+ esac
+ cmd=config
+ ;;
+ -d|--default)
+ if [ x"$cmd" != x"" ]; then
+ help
+ fi
+ gai_conf=${TEMPFILE}
+ cmd=config
+ ;;
+ -l|--list)
+ if [ x"$cmd" != x"" ]; then
+ help
+ fi
+ cmd=list
+ shift
+ ;;
+ -v)
+ verbose=1
+ shift
+ ;;
+ --)
+ shift;
+ break
+ ;;
+ *)
+ echo "Internal error!" >&2
+ exit 1
+ ;;
+ esac
+done
+
+case "$cmd" in
+ config)
+ if [ x"$gai_conf" = x"${TEMPFILE}" ]; then
+ sed -e 's/^[[:space:]]*//' <<END_OF_DEFAULT >${TEMPFILE}
+ label ::1/128 0
+ label ::/0 1
+ label 2002::/16 2
+ label ::/96 3
+ label ::ffff:0:0/96 4
+ label fec0::/10 5
+ label fc00::/7 6
+ label 2001:0::/32 7
+END_OF_DEFAULT
+ fi
+ do_load_config "$gai_conf"
+ ;;
+ list)
+ do_list_config
+ ;;
+ *)
+ help
+ ;;
+esac
+
+rm -f "${TEMPFILE}"
+
+exit 0
+
--
1.5.6.5
^ permalink raw reply related
* Re: [PATCH 6/13] bridge: Add core IGMP snooping support
From: Paul E. McKenney @ 2010-03-09 3:15 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: Herbert Xu, David S. Miller, netdev, Stephen Hemminger
In-Reply-To: <201003081950.48384.arnd@arndb.de>
On Mon, Mar 08, 2010 at 07:50:48PM +0100, Arnd Bergmann wrote:
> On Sunday 07 March 2010, Paul E. McKenney wrote:
> > On Sun, Mar 07, 2010 at 10:45:00AM +0800, Herbert Xu wrote:
> > > On Sat, Mar 06, 2010 at 11:00:00AM -0800, Paul E. McKenney wrote:
> >
> > OK, just re-checked your patch, and it looks OK.
> >
> > Also adding Arnd to CC.
> >
> > Arnd, would it be reasonable to extend your RCU-sparse changes to have
> > four different pointer namespaces, one for each flavor of RCU? (RCU,
> > RCU-bh, RCU-sched, and SRCU)? Always a fan of making the computer do
> > the auditing where reasonable. ;-)
>
> Yes, I guess that would be possible. I'd still leave out the rculist
> from any annotations for now, as this would get even more complex then.
Understood!
> One consequence will be the need for new rcu_assign_pointer{,_bh,_sched}
> macros that check the address space of the first argument, otherwise
> you'd be able to stick anything in there, including non-__rcu pointers.
>
> I've also found a few places (less than a handful) that use RCU to
> protect per-CPU data. Not sure how to deal with that, because now
> this also has its own named address space (__percpu), and it's probably
> a bit too much to introduce all combinations of
> {s,}rcu_{assign_pointer,dereference}{,_bh,_sched}{,_const}{,_percpu},
> so I'm ignoring them for now.
Ouch!!!
> > This could potentially catch the mismatched call_rcu()s, at least if the
> > rcu_head could be labeled.
>
> I haven't labeled the rcu_head at all so far, and I'm not sure if that's
> necessary. What I've been thinking about is replacing typical code like
>
> /* this is called with the writer-side lock held */
> void foo_assign(struct foo *foo, struct bar *newbar)
> {
> struct bar *bar = rcu_dereference_const(foo->bar); /* I just had to add
> this dereference */
> rcu_assign_pointer(foo->bar, newbar);
> if (bar)
> call_rcu(&bar->rcu, bar_destructor);
> }
>
> with the shorter
>
> void foo_assign(struct foo *foo, struct bar *newbar)
> {
> struct bar *bar = rcu_exchange(foo->bar, newbar);
> if (bar)
> call_rcu(&bar->rcu, bar_destructor);
> }
>
> Now we could combine this to
>
> void foo_assign(struct foo *foo, struct bar *newbar)
> {
> rcu_exchange_call(foo->bar, newbar, rcu, bar_destructor);
> }
>
> #define rcu_exchange_call(ptr, new, member, func) \
> ({ \
> typeof(new) old = rcu_exchange((ptr),(new)); \
> if (old) \
> call_rcu(&(old)->member, (func)); \
> old; \
> })
>
> and make appropriate versions of all the above rcu methods for this.
> With some extra macro magic, this could even become type safe and
> accept a function that takes a typeof(ptr) argument instead of the
> rcu_head.
This approach does look promising! And probably a lot simpler than
attempting to label the rcu_head structure. I am not yet convinced
about the typesafe function taking typeof(ptr), but possibly I am
suffering a failure of C-preprocessor imagination?
Thanx, Paul
^ permalink raw reply
* [2.6.34-rc1] CPU stalls when closing TCP sockets.
From: Tetsuo Handa @ 2010-03-09 3:19 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel
Hello.
Below program sometimes triggers CPU stall at (8).
----- test program -----
#include <stdio.h>
#include <unistd.h>
#include <sys/socket.h>
#include <sys/types.h>
#include <arpa/inet.h>
int main(int argc, char *argv[])
{
struct sockaddr_in saddr = { };
struct sockaddr_in caddr = { };
socklen_t size = sizeof(saddr);
int fd1 = socket(PF_INET, SOCK_STREAM, 0);
int fd2 = socket(PF_INET, SOCK_STREAM, 0);
int fd3 = EOF;
saddr.sin_family = AF_INET;
saddr.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
saddr.sin_port = htons(0);
fprintf(stderr, "(1) Binding TCP 127.0.0.1 0 = ");
fprintf(stderr, "%d\n", bind(fd1, (struct sockaddr *) &saddr,
sizeof(saddr)));
fprintf(stderr, "(2) Getting server name = ");
fprintf(stderr, "%d\n", getsockname(fd1, (struct sockaddr *) &saddr,
&size));
fprintf(stderr, "(3) Listening TCP 127.0.0.1 %d = ",
ntohs(saddr.sin_port));
fprintf(stderr, "%d\n", listen(fd1, 5));
fprintf(stderr, "(4) Connecting TCP 127.0.0.1 %d = ",
ntohs(saddr.sin_port));
fprintf(stderr, "%d\n", connect(fd2, (struct sockaddr *) &saddr,
sizeof(saddr)));
fprintf(stderr, "(5) Getting client name = ");
fprintf(stderr, "%d\n", getsockname(fd2, (struct sockaddr *) &caddr,
&size));
fprintf(stderr, "(6) Accepting TCP 127.0.0.1 %d = ",
ntohs(caddr.sin_port));
fd3 = accept(fd1, (struct sockaddr *) &caddr, &size);
fprintf(stderr, "%d\n", fd3);
fprintf(stderr, "(7) Closing %d = ", fd3);
fprintf(stderr, "%d\n", close(fd3));
fprintf(stderr, "(8) Closing %d = ", fd2);
fprintf(stderr, "%d\n", close(fd2));
fprintf(stderr, "(9) Closing %d = ", fd1);
fprintf(stderr, "%d\n", close(fd1));
return 0;
}
----- test program -----
[ 59.024080] INFO: RCU detected CPU 0 stall (t=2500 jiffies)
[ 59.024080] sending NMI to all CPUs:
[ 59.024080] NMI backtrace for cpu 0
[ 59.024080] Modules linked in: ipv6 pcnet32
[ 59.024080]
[ 59.024080] Pid: 2720, comm: a.out Not tainted 2.6.34-rc1 #2 440BX Desktop Reference Platform/VMware Virtual Platform
[ 59.024080] EIP: 0060:[<c101e719>] EFLAGS: 00000046 CPU: 0
[ 59.024080] EIP is at default_send_IPI_mask_logical+0xa9/0x140
[ 59.024080] EAX: fffff000 EBX: 01000000 ECX: 00000000 EDX: 00000c00
[ 59.024080] ESI: 00000c00 EDI: 00000046 EBP: c2400efc ESP: c2400ee0
[ 59.024080] DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
[ 59.024080] Process a.out (pid: 2720, ti=c2400000 task=debb60e0 task.ti=df324000)
[ 59.024080] Stack:
[ 59.024080] 0000000d be1bfc96 0000000d 00000002 00000000 c1464980 c1464980 c2400f08
[ 59.024080] <0> c101e8d1 00000000 c2400f18 c101f1d7 c13c1569 c1464980 c2400f34 c108d637
[ 59.024080] <0> c13d8620 00000000 000009c4 00000000 c1464980 c2400f44 c108d6b8 c2545b00
[ 59.024080] Call Trace:
[ 59.024080] [<c101e8d1>] ? default_send_IPI_all+0x81/0x90
[ 59.024080] [<c101f1d7>] ? arch_trigger_all_cpu_backtrace+0x37/0x60
[ 59.024080] [<c108d637>] ? print_cpu_stall+0x47/0x90
[ 59.024080] [<c108d6b8>] ? check_cpu_stall+0x38/0x60
[ 59.024080] [<c108e665>] ? __rcu_pending+0x25/0xb0
[ 59.024080] [<c108e71e>] ? rcu_pending+0x2e/0x70
[ 59.024080] [<c108e04b>] ? rcu_check_callbacks+0x1b/0xb0
[ 59.024080] [<c1053b6d>] ? update_process_times+0x3d/0x70
[ 59.024080] [<c106ed1a>] ? tick_periodic+0x2a/0x80
[ 59.024080] [<c106ed8e>] ? tick_handle_periodic+0x1e/0x80
[ 59.024080] [<c1005e83>] ? timer_interrupt+0x23/0x50
[ 59.024080] [<c108a125>] ? handle_IRQ_event+0x45/0x130
[ 59.024080] [<c108c098>] ? handle_level_irq+0x88/0x100
[ 59.024080] [<c108c010>] ? handle_level_irq+0x0/0x100
[ 59.024080] <IRQ>
[ 59.024080] [<c12e969a>] ? do_IRQ+0x4a/0xc0
[ 59.024080] [<c100324e>] ? common_interrupt+0x2e/0x34
[ 59.024080] [<c12a3c84>] ? tcp_v4_rcv+0x154/0x670
[ 59.024080] [<c12873ab>] ? ip_local_deliver_finish+0xcb/0x1b0
[ 59.024080] [<c1287316>] ? ip_local_deliver_finish+0x36/0x1b0
[ 59.024080] [<c12874c5>] ? ip_local_deliver+0x35/0x40
[ 59.024080] [<c12875db>] ? ip_rcv_finish+0x10b/0x2d0
[ 59.024080] [<c1287906>] ? ip_rcv+0x166/0x240
[ 59.024080] [<c1269d83>] ? netif_receive_skb+0x273/0x300
[ 59.024080] [<c1269c1d>] ? netif_receive_skb+0x10d/0x300
[ 59.024080] [<c1074f06>] ? trace_hardirqs_on_caller+0xa6/0x160
[ 59.024080] [<c126a6d2>] ? process_backlog+0x82/0xb0
[ 59.024080] [<c126aa0f>] ? net_rx_action+0x11f/0x190
[ 59.024080] [<c104acd0>] ? __do_softirq+0x50/0x1c0
[ 59.024080] [<c104ad2b>] ? __do_softirq+0xab/0x1c0
[ 59.024080] [<c1269629>] ? dev_queue_xmit+0x209/0x370
[ 59.024080] [<c104ac80>] ? __do_softirq+0x0/0x1c0
[ 59.024080] <IRQ>
[ 59.024080] [<c1269629>] ? dev_queue_xmit+0x209/0x370
[ 59.024080] [<c104ab6e>] ? local_bh_enable+0x8e/0xd0
[ 59.024080] [<c1269629>] ? dev_queue_xmit+0x209/0x370
[ 59.024080] [<c126950e>] ? dev_queue_xmit+0xee/0x370
[ 59.024080] [<c1289f7f>] ? ip_finish_output+0x10f/0x270
[ 59.024080] [<c128a271>] ? ip_output+0x51/0x60
[ 59.024080] [<c1289c5d>] ? ip_local_out+0x1d/0x20
[ 59.024080] [<c128a48f>] ? ip_queue_xmit+0x20f/0x3e0
[ 59.024080] [<c1066dd5>] ? sched_clock_local+0x15/0x170
[ 59.024080] [<c10272f6>] ? __change_page_attr+0x106/0x1b0
[ 59.024080] [<c10274df>] ? __change_page_attr_set_clr+0x2f/0x70
[ 59.024080] [<c12a2a82>] ? tcp_v4_send_check+0x52/0xe0
[ 59.024080] [<c129df10>] ? tcp_transmit_skb+0x2d0/0x420
[ 59.024080] [<c10db72a>] ? poison_obj+0x2a/0x50
[ 59.024080] [<c10ddc15>] ? __kmalloc_track_caller+0x185/0x1e0
[ 59.024080] [<c129f3af>] ? tcp_write_xmit+0x13f/0x290
[ 59.024080] [<c129f536>] ? __tcp_push_pending_frames+0x36/0xf0
[ 59.024080] [<c12a0044>] ? tcp_send_fin+0x54/0xc0
[ 59.024080] [<c12938b4>] ? tcp_close+0x2f4/0x370
[ 59.024080] [<c107509b>] ? trace_hardirqs_off+0xb/0x10
[ 59.024080] [<c12b15fe>] ? inet_release+0x4e/0x60
[ 59.024080] [<c12598d6>] ? sock_release+0x66/0x80
[ 59.024080] [<c125a677>] ? sock_close+0x17/0x30
[ 59.024080] [<c10e3dee>] ? __fput+0x16e/0x1b0
[ 59.024080] [<c10e3bee>] ? fput+0x1e/0x20
[ 59.024080] [<c10e23ff>] ? filp_close+0x3f/0x70
[ 59.024080] [<c10e2498>] ? sys_close+0x68/0xb0
[ 59.024080] [<c12e8de1>] ? syscall_call+0x7/0xb
[ 59.024080] Code: f6 c4 10 75 ed c1 e3 18 89 9a 10 c3 ff ff 8b 55 f0 a1 8c 01 46 c1 09 f2 81 ce 00 04 00 00 83 7d f0 02 0f 44 d6 89 90 00 c3 ff ff <89> f8 c1 e8 09 83 f0 01 a8 01 74 0f 57 9d e8 64 69 05 00 83 c4
[ 59.024080] Call Trace:
[ 59.024080] [<c101e8d1>] default_send_IPI_all+0x81/0x90
[ 59.024080] [<c101f1d7>] arch_trigger_all_cpu_backtrace+0x37/0x60
[ 59.024080] [<c108d637>] print_cpu_stall+0x47/0x90
[ 59.024080] [<c108d6b8>] check_cpu_stall+0x38/0x60
[ 59.024080] [<c108e665>] __rcu_pending+0x25/0xb0
[ 59.024080] [<c108e71e>] rcu_pending+0x2e/0x70
[ 59.024080] [<c108e04b>] rcu_check_callbacks+0x1b/0xb0
[ 59.024080] [<c1053b6d>] update_process_times+0x3d/0x70
[ 59.024080] [<c106ed1a>] tick_periodic+0x2a/0x80
[ 59.024080] [<c106ed8e>] tick_handle_periodic+0x1e/0x80
[ 59.024080] [<c1005e83>] timer_interrupt+0x23/0x50
[ 59.024080] [<c108a125>] handle_IRQ_event+0x45/0x130
[ 59.024080] [<c108c098>] handle_level_irq+0x88/0x100
[ 59.024080] [<c108c010>] ? handle_level_irq+0x0/0x100
[ 59.024080] <IRQ> [<c12e969a>] ? do_IRQ+0x4a/0xc0
[ 59.024080] [<c100324e>] ? common_interrupt+0x2e/0x34
[ 59.024080] [<c12a3c84>] ? tcp_v4_rcv+0x154/0x670
[ 59.024080] [<c12873ab>] ? ip_local_deliver_finish+0xcb/0x1b0
[ 59.024080] [<c1287316>] ? ip_local_deliver_finish+0x36/0x1b0
[ 59.024080] [<c12874c5>] ? ip_local_deliver+0x35/0x40
[ 59.024080] [<c12875db>] ? ip_rcv_finish+0x10b/0x2d0
[ 59.024080] [<c1287906>] ? ip_rcv+0x166/0x240
[ 59.024080] [<c1269d83>] ? netif_receive_skb+0x273/0x300
[ 59.024080] [<c1269c1d>] ? netif_receive_skb+0x10d/0x300
[ 59.024080] [<c1074f06>] ? trace_hardirqs_on_caller+0xa6/0x160
[ 59.024080] [<c126a6d2>] ? process_backlog+0x82/0xb0
[ 59.024080] [<c126aa0f>] ? net_rx_action+0x11f/0x190
[ 59.024080] [<c104acd0>] ? __do_softirq+0x50/0x1c0
[ 59.024080] [<c104ad2b>] ? __do_softirq+0xab/0x1c0
[ 59.024080] [<c1269629>] ? dev_queue_xmit+0x209/0x370
[ 59.024080] [<c104ac80>] ? __do_softirq+0x0/0x1c0
[ 59.024080] <IRQ> [<c1269629>] ? dev_queue_xmit+0x209/0x370
[ 59.024080] [<c104ab6e>] ? local_bh_enable+0x8e/0xd0
[ 59.024080] [<c1269629>] ? dev_queue_xmit+0x209/0x370
[ 59.024080] [<c126950e>] ? dev_queue_xmit+0xee/0x370
[ 59.024080] [<c1289f7f>] ? ip_finish_output+0x10f/0x270
[ 59.024080] [<c128a271>] ? ip_output+0x51/0x60
[ 59.024080] [<c1289c5d>] ? ip_local_out+0x1d/0x20
[ 59.024080] [<c128a48f>] ? ip_queue_xmit+0x20f/0x3e0
[ 59.024080] [<c1066dd5>] ? sched_clock_local+0x15/0x170
[ 59.024080] [<c10272f6>] ? __change_page_attr+0x106/0x1b0
[ 59.024080] [<c10274df>] ? __change_page_attr_set_clr+0x2f/0x70
[ 59.024080] [<c12a2a82>] ? tcp_v4_send_check+0x52/0xe0
[ 59.024080] [<c129df10>] ? tcp_transmit_skb+0x2d0/0x420
[ 59.024080] [<c10db72a>] ? poison_obj+0x2a/0x50
[ 59.024080] [<c10ddc15>] ? __kmalloc_track_caller+0x185/0x1e0
[ 59.024080] [<c129f3af>] ? tcp_write_xmit+0x13f/0x290
[ 59.024080] [<c129f536>] ? __tcp_push_pending_frames+0x36/0xf0
[ 59.024080] [<c12a0044>] ? tcp_send_fin+0x54/0xc0
[ 59.024080] [<c12938b4>] ? tcp_close+0x2f4/0x370
[ 59.024080] [<c107509b>] ? trace_hardirqs_off+0xb/0x10
[ 59.024080] [<c12b15fe>] ? inet_release+0x4e/0x60
[ 59.024080] [<c12598d6>] ? sock_release+0x66/0x80
[ 59.024080] [<c125a677>] ? sock_close+0x17/0x30
[ 59.024080] [<c10e3dee>] ? __fput+0x16e/0x1b0
[ 59.024080] [<c10e3bee>] ? fput+0x1e/0x20
[ 59.024080] [<c10e23ff>] ? filp_close+0x3f/0x70
[ 59.024080] [<c10e2498>] ? sys_close+0x68/0xb0
[ 59.024080] [<c12e8de1>] ? syscall_call+0x7/0xb
[ 59.024080] Pid: 2720, comm: a.out Not tainted 2.6.34-rc1 #2
[ 59.024080] Call Trace:
[ 59.024080] [<c100a997>] ? show_regs+0x27/0x30
[ 59.024080] [<c101ef39>] nmi_watchdog_tick+0x189/0x240
[ 59.024080] [<c1003bdf>] default_do_nmi+0x5f/0x180
[ 59.024080] [<c1003d89>] do_nmi+0x89/0xa0
[ 59.024080] [<c12e9541>] nmi_stack_correct+0x28/0x2d
[ 59.024080] [<c101e719>] ? default_send_IPI_mask_logical+0xa9/0x140
[ 59.024080] [<c101e8d1>] default_send_IPI_all+0x81/0x90
[ 59.024080] [<c101f1d7>] arch_trigger_all_cpu_backtrace+0x37/0x60
[ 59.024080] [<c108d637>] print_cpu_stall+0x47/0x90
[ 59.024080] [<c108d6b8>] check_cpu_stall+0x38/0x60
[ 59.024080] [<c108e665>] __rcu_pending+0x25/0xb0
[ 59.024080] [<c108e71e>] rcu_pending+0x2e/0x70
[ 59.024080] [<c108e04b>] rcu_check_callbacks+0x1b/0xb0
[ 59.024080] [<c1053b6d>] update_process_times+0x3d/0x70
[ 59.024080] [<c106ed1a>] tick_periodic+0x2a/0x80
[ 59.024080] [<c106ed8e>] tick_handle_periodic+0x1e/0x80
[ 59.024080] [<c1005e83>] timer_interrupt+0x23/0x50
[ 59.024080] [<c108a125>] handle_IRQ_event+0x45/0x130
[ 59.024080] [<c108c098>] handle_level_irq+0x88/0x100
[ 59.024080] [<c108c010>] ? handle_level_irq+0x0/0x100
[ 59.024080] <IRQ> [<c12e969a>] ? do_IRQ+0x4a/0xc0
[ 59.024080] [<c100324e>] ? common_interrupt+0x2e/0x34
[ 59.024080] [<c12a3c84>] ? tcp_v4_rcv+0x154/0x670
[ 59.024080] [<c12873ab>] ? ip_local_deliver_finish+0xcb/0x1b0
[ 59.024080] [<c1287316>] ? ip_local_deliver_finish+0x36/0x1b0
[ 59.024080] [<c12874c5>] ? ip_local_deliver+0x35/0x40
[ 59.024080] [<c12875db>] ? ip_rcv_finish+0x10b/0x2d0
[ 59.024080] [<c1287906>] ? ip_rcv+0x166/0x240
[ 59.024080] [<c1269d83>] ? netif_receive_skb+0x273/0x300
[ 59.024080] [<c1269c1d>] ? netif_receive_skb+0x10d/0x300
[ 59.024080] [<c1074f06>] ? trace_hardirqs_on_caller+0xa6/0x160
[ 59.024080] [<c126a6d2>] ? process_backlog+0x82/0xb0
[ 59.024080] [<c126aa0f>] ? net_rx_action+0x11f/0x190
[ 59.024080] [<c104acd0>] ? __do_softirq+0x50/0x1c0
[ 59.024080] [<c104ad2b>] ? __do_softirq+0xab/0x1c0
[ 59.024080] [<c1269629>] ? dev_queue_xmit+0x209/0x370
[ 59.024080] [<c104ac80>] ? __do_softirq+0x0/0x1c0
[ 59.024080] <IRQ> [<c1269629>] ? dev_queue_xmit+0x209/0x370
[ 59.024080] [<c104ab6e>] ? local_bh_enable+0x8e/0xd0
[ 59.024080] [<c1269629>] ? dev_queue_xmit+0x209/0x370
[ 59.024080] [<c126950e>] ? dev_queue_xmit+0xee/0x370
[ 59.024080] [<c1289f7f>] ? ip_finish_output+0x10f/0x270
[ 59.024080] [<c128a271>] ? ip_output+0x51/0x60
[ 59.024080] [<c1289c5d>] ? ip_local_out+0x1d/0x20
[ 59.024080] [<c128a48f>] ? ip_queue_xmit+0x20f/0x3e0
[ 59.024080] [<c1066dd5>] ? sched_clock_local+0x15/0x170
[ 59.024080] [<c10272f6>] ? __change_page_attr+0x106/0x1b0
[ 59.024080] [<c10274df>] ? __change_page_attr_set_clr+0x2f/0x70
[ 59.024080] [<c12a2a82>] ? tcp_v4_send_check+0x52/0xe0
[ 59.024080] [<c129df10>] ? tcp_transmit_skb+0x2d0/0x420
[ 59.024080] [<c10db72a>] ? poison_obj+0x2a/0x50
[ 59.024080] [<c10ddc15>] ? __kmalloc_track_caller+0x185/0x1e0
[ 59.024080] [<c129f3af>] ? tcp_write_xmit+0x13f/0x290
[ 59.024080] [<c129f536>] ? __tcp_push_pending_frames+0x36/0xf0
[ 59.024080] [<c12a0044>] ? tcp_send_fin+0x54/0xc0
[ 59.024080] [<c12938b4>] ? tcp_close+0x2f4/0x370
[ 59.024080] [<c107509b>] ? trace_hardirqs_off+0xb/0x10
[ 59.024080] [<c12b15fe>] ? inet_release+0x4e/0x60
[ 59.024080] [<c12598d6>] ? sock_release+0x66/0x80
[ 59.024080] [<c125a677>] ? sock_close+0x17/0x30
[ 59.024080] [<c10e3dee>] ? __fput+0x16e/0x1b0
[ 59.024080] [<c10e3bee>] ? fput+0x1e/0x20
[ 59.024080] [<c10e23ff>] ? filp_close+0x3f/0x70
[ 59.024080] [<c10e2498>] ? sys_close+0x68/0xb0
[ 59.024080] [<c12e8de1>] ? syscall_call+0x7/0xb
Full log is at http://I-love.SAKURA.ne.jp/tmp/dmesg-2.6.34-rc1.txt
Config is at http://I-love.SAKURA.ne.jp/tmp/config-2.6.34-rc1
Regards.
^ permalink raw reply
* Re: [PATCH] net/netfilter/ipvs/ip_vs_ftp.c: Remove use of NIPQUAD
From: Simon Horman @ 2010-03-09 4:08 UTC (permalink / raw)
To: Joe Perches
Cc: Wensong Zhang, Julian Anastasov, Patrick McHardy, David S. Miller,
netdev, lvs-devel, LKML
In-Reply-To: <1268094699.1617.5.camel@Joe-Laptop.home>
On Mon, Mar 08, 2010 at 04:31:39PM -0800, Joe Perches wrote:
> NIPQUAD has very few uses left.
>
> Remove this use and make the code have the identical form of the only
> other use of "%u,%u,%u,%u,%u,%u" in net/ipv4/netfilter/nf_nat_ftp.c
>
> Signed-off-by: Joe Perches <joe@perches.com>
Acked-by: Simon Horman <horms@verge.net.au>
> ---
> net/netfilter/ipvs/ip_vs_ftp.c | 10 ++++++++--
> 1 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/net/netfilter/ipvs/ip_vs_ftp.c
> b/net/netfilter/ipvs/ip_vs_ftp.c
> index 73f38ea..9f63283 100644
> --- a/net/netfilter/ipvs/ip_vs_ftp.c
> +++ b/net/netfilter/ipvs/ip_vs_ftp.c
> @@ -208,8 +208,14 @@ static int ip_vs_ftp_out(struct ip_vs_app *app,
> struct ip_vs_conn *cp,
> */
> from.ip = n_cp->vaddr.ip;
> port = n_cp->vport;
> - sprintf(buf, "%u,%u,%u,%u,%u,%u", NIPQUAD(from.ip),
> - (ntohs(port)>>8)&255, ntohs(port)&255);
> + snprintf(buf, sizeof(buf), "%u,%u,%u,%u,%u,%u",
> + ((unsigned char *)&from.ip)[0],
> + ((unsigned char *)&from.ip)[1],
> + ((unsigned char *)&from.ip)[2],
> + ((unsigned char *)&from.ip)[3],
> + ntohs(port) >> 8,
> + ntohs(port) & 0xFF);
> +
> buf_len = strlen(buf);
>
> /*
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe lvs-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [2.6.34-rc1] CPU stalls when closing TCP sockets.
From: Eric Dumazet @ 2010-03-09 6:37 UTC (permalink / raw)
To: Tetsuo Handa; +Cc: netdev, linux-kernel, Paul E. McKenney
In-Reply-To: <201003090319.o293J6WF026448@www262.sakura.ne.jp>
Le mardi 09 mars 2010 à 12:19 +0900, Tetsuo Handa a écrit :
> Hello.
>
> Below program sometimes triggers CPU stall at (8).
>
> ----- test program -----
> #include <stdio.h>
> #include <unistd.h>
> #include <sys/socket.h>
> #include <sys/types.h>
> #include <arpa/inet.h>
>
> int main(int argc, char *argv[])
> {
> struct sockaddr_in saddr = { };
> struct sockaddr_in caddr = { };
> socklen_t size = sizeof(saddr);
> int fd1 = socket(PF_INET, SOCK_STREAM, 0);
> int fd2 = socket(PF_INET, SOCK_STREAM, 0);
> int fd3 = EOF;
> saddr.sin_family = AF_INET;
> saddr.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
> saddr.sin_port = htons(0);
> fprintf(stderr, "(1) Binding TCP 127.0.0.1 0 = ");
> fprintf(stderr, "%d\n", bind(fd1, (struct sockaddr *) &saddr,
> sizeof(saddr)));
> fprintf(stderr, "(2) Getting server name = ");
> fprintf(stderr, "%d\n", getsockname(fd1, (struct sockaddr *) &saddr,
> &size));
> fprintf(stderr, "(3) Listening TCP 127.0.0.1 %d = ",
> ntohs(saddr.sin_port));
> fprintf(stderr, "%d\n", listen(fd1, 5));
> fprintf(stderr, "(4) Connecting TCP 127.0.0.1 %d = ",
> ntohs(saddr.sin_port));
> fprintf(stderr, "%d\n", connect(fd2, (struct sockaddr *) &saddr,
> sizeof(saddr)));
> fprintf(stderr, "(5) Getting client name = ");
> fprintf(stderr, "%d\n", getsockname(fd2, (struct sockaddr *) &caddr,
> &size));
> fprintf(stderr, "(6) Accepting TCP 127.0.0.1 %d = ",
> ntohs(caddr.sin_port));
> fd3 = accept(fd1, (struct sockaddr *) &caddr, &size);
> fprintf(stderr, "%d\n", fd3);
> fprintf(stderr, "(7) Closing %d = ", fd3);
> fprintf(stderr, "%d\n", close(fd3));
> fprintf(stderr, "(8) Closing %d = ", fd2);
> fprintf(stderr, "%d\n", close(fd2));
> fprintf(stderr, "(9) Closing %d = ", fd1);
> fprintf(stderr, "%d\n", close(fd1));
> return 0;
> }
> ----- test program -----
>
> [ 59.024080] INFO: RCU detected CPU 0 stall (t=2500 jiffies)
> [ 59.024080] sending NMI to all CPUs:
> [ 59.024080] NMI backtrace for cpu 0
> [ 59.024080] Modules linked in: ipv6 pcnet32
> [ 59.024080]
> [ 59.024080] Pid: 2720, comm: a.out Not tainted 2.6.34-rc1 #2 440BX Desktop Reference Platform/VMware Virtual Platform
> [ 59.024080] EIP: 0060:[<c101e719>] EFLAGS: 00000046 CPU: 0
> [ 59.024080] EIP is at default_send_IPI_mask_logical+0xa9/0x140
> [ 59.024080] EAX: fffff000 EBX: 01000000 ECX: 00000000 EDX: 00000c00
> [ 59.024080] ESI: 00000c00 EDI: 00000046 EBP: c2400efc ESP: c2400ee0
> [ 59.024080] DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
> [ 59.024080] Process a.out (pid: 2720, ti=c2400000 task=debb60e0 task.ti=df324000)
> [ 59.024080] Stack:
> [ 59.024080] 0000000d be1bfc96 0000000d 00000002 00000000 c1464980 c1464980 c2400f08
> [ 59.024080] <0> c101e8d1 00000000 c2400f18 c101f1d7 c13c1569 c1464980 c2400f34 c108d637
> [ 59.024080] <0> c13d8620 00000000 000009c4 00000000 c1464980 c2400f44 c108d6b8 c2545b00
> [ 59.024080] Call Trace:
> [ 59.024080] [<c101e8d1>] ? default_send_IPI_all+0x81/0x90
> [ 59.024080] [<c101f1d7>] ? arch_trigger_all_cpu_backtrace+0x37/0x60
> [ 59.024080] [<c108d637>] ? print_cpu_stall+0x47/0x90
> [ 59.024080] [<c108d6b8>] ? check_cpu_stall+0x38/0x60
> [ 59.024080] [<c108e665>] ? __rcu_pending+0x25/0xb0
> [ 59.024080] [<c108e71e>] ? rcu_pending+0x2e/0x70
> [ 59.024080] [<c108e04b>] ? rcu_check_callbacks+0x1b/0xb0
> [ 59.024080] [<c1053b6d>] ? update_process_times+0x3d/0x70
> [ 59.024080] [<c106ed1a>] ? tick_periodic+0x2a/0x80
> [ 59.024080] [<c106ed8e>] ? tick_handle_periodic+0x1e/0x80
> [ 59.024080] [<c1005e83>] ? timer_interrupt+0x23/0x50
> [ 59.024080] [<c108a125>] ? handle_IRQ_event+0x45/0x130
> [ 59.024080] [<c108c098>] ? handle_level_irq+0x88/0x100
> [ 59.024080] [<c108c010>] ? handle_level_irq+0x0/0x100
> [ 59.024080] <IRQ>
> [ 59.024080] [<c12e969a>] ? do_IRQ+0x4a/0xc0
> [ 59.024080] [<c100324e>] ? common_interrupt+0x2e/0x34
> [ 59.024080] [<c12a3c84>] ? tcp_v4_rcv+0x154/0x670
> [ 59.024080] [<c12873ab>] ? ip_local_deliver_finish+0xcb/0x1b0
> [ 59.024080] [<c1287316>] ? ip_local_deliver_finish+0x36/0x1b0
> [ 59.024080] [<c12874c5>] ? ip_local_deliver+0x35/0x40
> [ 59.024080] [<c12875db>] ? ip_rcv_finish+0x10b/0x2d0
> [ 59.024080] [<c1287906>] ? ip_rcv+0x166/0x240
> [ 59.024080] [<c1269d83>] ? netif_receive_skb+0x273/0x300
> [ 59.024080] [<c1269c1d>] ? netif_receive_skb+0x10d/0x300
> [ 59.024080] [<c1074f06>] ? trace_hardirqs_on_caller+0xa6/0x160
> [ 59.024080] [<c126a6d2>] ? process_backlog+0x82/0xb0
> [ 59.024080] [<c126aa0f>] ? net_rx_action+0x11f/0x190
> [ 59.024080] [<c104acd0>] ? __do_softirq+0x50/0x1c0
> [ 59.024080] [<c104ad2b>] ? __do_softirq+0xab/0x1c0
> [ 59.024080] [<c1269629>] ? dev_queue_xmit+0x209/0x370
> [ 59.024080] [<c104ac80>] ? __do_softirq+0x0/0x1c0
> [ 59.024080] <IRQ>
> [ 59.024080] [<c1269629>] ? dev_queue_xmit+0x209/0x370
> [ 59.024080] [<c104ab6e>] ? local_bh_enable+0x8e/0xd0
> [ 59.024080] [<c1269629>] ? dev_queue_xmit+0x209/0x370
> [ 59.024080] [<c126950e>] ? dev_queue_xmit+0xee/0x370
> [ 59.024080] [<c1289f7f>] ? ip_finish_output+0x10f/0x270
> [ 59.024080] [<c128a271>] ? ip_output+0x51/0x60
> [ 59.024080] [<c1289c5d>] ? ip_local_out+0x1d/0x20
> [ 59.024080] [<c128a48f>] ? ip_queue_xmit+0x20f/0x3e0
> [ 59.024080] [<c1066dd5>] ? sched_clock_local+0x15/0x170
> [ 59.024080] [<c10272f6>] ? __change_page_attr+0x106/0x1b0
> [ 59.024080] [<c10274df>] ? __change_page_attr_set_clr+0x2f/0x70
> [ 59.024080] [<c12a2a82>] ? tcp_v4_send_check+0x52/0xe0
> [ 59.024080] [<c129df10>] ? tcp_transmit_skb+0x2d0/0x420
> [ 59.024080] [<c10db72a>] ? poison_obj+0x2a/0x50
> [ 59.024080] [<c10ddc15>] ? __kmalloc_track_caller+0x185/0x1e0
> [ 59.024080] [<c129f3af>] ? tcp_write_xmit+0x13f/0x290
> [ 59.024080] [<c129f536>] ? __tcp_push_pending_frames+0x36/0xf0
> [ 59.024080] [<c12a0044>] ? tcp_send_fin+0x54/0xc0
> [ 59.024080] [<c12938b4>] ? tcp_close+0x2f4/0x370
> [ 59.024080] [<c107509b>] ? trace_hardirqs_off+0xb/0x10
> [ 59.024080] [<c12b15fe>] ? inet_release+0x4e/0x60
> [ 59.024080] [<c12598d6>] ? sock_release+0x66/0x80
> [ 59.024080] [<c125a677>] ? sock_close+0x17/0x30
> [ 59.024080] [<c10e3dee>] ? __fput+0x16e/0x1b0
> [ 59.024080] [<c10e3bee>] ? fput+0x1e/0x20
> [ 59.024080] [<c10e23ff>] ? filp_close+0x3f/0x70
> [ 59.024080] [<c10e2498>] ? sys_close+0x68/0xb0
> [ 59.024080] [<c12e8de1>] ? syscall_call+0x7/0xb
> [ 59.024080] Code: f6 c4 10 75 ed c1 e3 18 89 9a 10 c3 ff ff 8b 55 f0 a1 8c 01 46 c1 09 f2 81 ce 00 04 00 00 83 7d f0 02 0f 44 d6 89 90 00 c3 ff ff <89> f8 c1 e8 09 83 f0 01 a8 01 74 0f 57 9d e8 64 69 05 00 83 c4
> [ 59.024080] Call Trace:
> [ 59.024080] [<c101e8d1>] default_send_IPI_all+0x81/0x90
> [ 59.024080] [<c101f1d7>] arch_trigger_all_cpu_backtrace+0x37/0x60
> [ 59.024080] [<c108d637>] print_cpu_stall+0x47/0x90
> [ 59.024080] [<c108d6b8>] check_cpu_stall+0x38/0x60
> [ 59.024080] [<c108e665>] __rcu_pending+0x25/0xb0
> [ 59.024080] [<c108e71e>] rcu_pending+0x2e/0x70
> [ 59.024080] [<c108e04b>] rcu_check_callbacks+0x1b/0xb0
> [ 59.024080] [<c1053b6d>] update_process_times+0x3d/0x70
> [ 59.024080] [<c106ed1a>] tick_periodic+0x2a/0x80
> [ 59.024080] [<c106ed8e>] tick_handle_periodic+0x1e/0x80
> [ 59.024080] [<c1005e83>] timer_interrupt+0x23/0x50
> [ 59.024080] [<c108a125>] handle_IRQ_event+0x45/0x130
> [ 59.024080] [<c108c098>] handle_level_irq+0x88/0x100
> [ 59.024080] [<c108c010>] ? handle_level_irq+0x0/0x100
> [ 59.024080] <IRQ> [<c12e969a>] ? do_IRQ+0x4a/0xc0
> [ 59.024080] [<c100324e>] ? common_interrupt+0x2e/0x34
> [ 59.024080] [<c12a3c84>] ? tcp_v4_rcv+0x154/0x670
> [ 59.024080] [<c12873ab>] ? ip_local_deliver_finish+0xcb/0x1b0
> [ 59.024080] [<c1287316>] ? ip_local_deliver_finish+0x36/0x1b0
> [ 59.024080] [<c12874c5>] ? ip_local_deliver+0x35/0x40
> [ 59.024080] [<c12875db>] ? ip_rcv_finish+0x10b/0x2d0
> [ 59.024080] [<c1287906>] ? ip_rcv+0x166/0x240
> [ 59.024080] [<c1269d83>] ? netif_receive_skb+0x273/0x300
> [ 59.024080] [<c1269c1d>] ? netif_receive_skb+0x10d/0x300
> [ 59.024080] [<c1074f06>] ? trace_hardirqs_on_caller+0xa6/0x160
> [ 59.024080] [<c126a6d2>] ? process_backlog+0x82/0xb0
> [ 59.024080] [<c126aa0f>] ? net_rx_action+0x11f/0x190
> [ 59.024080] [<c104acd0>] ? __do_softirq+0x50/0x1c0
> [ 59.024080] [<c104ad2b>] ? __do_softirq+0xab/0x1c0
> [ 59.024080] [<c1269629>] ? dev_queue_xmit+0x209/0x370
> [ 59.024080] [<c104ac80>] ? __do_softirq+0x0/0x1c0
> [ 59.024080] <IRQ> [<c1269629>] ? dev_queue_xmit+0x209/0x370
> [ 59.024080] [<c104ab6e>] ? local_bh_enable+0x8e/0xd0
> [ 59.024080] [<c1269629>] ? dev_queue_xmit+0x209/0x370
> [ 59.024080] [<c126950e>] ? dev_queue_xmit+0xee/0x370
> [ 59.024080] [<c1289f7f>] ? ip_finish_output+0x10f/0x270
> [ 59.024080] [<c128a271>] ? ip_output+0x51/0x60
> [ 59.024080] [<c1289c5d>] ? ip_local_out+0x1d/0x20
> [ 59.024080] [<c128a48f>] ? ip_queue_xmit+0x20f/0x3e0
> [ 59.024080] [<c1066dd5>] ? sched_clock_local+0x15/0x170
> [ 59.024080] [<c10272f6>] ? __change_page_attr+0x106/0x1b0
> [ 59.024080] [<c10274df>] ? __change_page_attr_set_clr+0x2f/0x70
> [ 59.024080] [<c12a2a82>] ? tcp_v4_send_check+0x52/0xe0
> [ 59.024080] [<c129df10>] ? tcp_transmit_skb+0x2d0/0x420
> [ 59.024080] [<c10db72a>] ? poison_obj+0x2a/0x50
> [ 59.024080] [<c10ddc15>] ? __kmalloc_track_caller+0x185/0x1e0
> [ 59.024080] [<c129f3af>] ? tcp_write_xmit+0x13f/0x290
> [ 59.024080] [<c129f536>] ? __tcp_push_pending_frames+0x36/0xf0
> [ 59.024080] [<c12a0044>] ? tcp_send_fin+0x54/0xc0
> [ 59.024080] [<c12938b4>] ? tcp_close+0x2f4/0x370
> [ 59.024080] [<c107509b>] ? trace_hardirqs_off+0xb/0x10
> [ 59.024080] [<c12b15fe>] ? inet_release+0x4e/0x60
> [ 59.024080] [<c12598d6>] ? sock_release+0x66/0x80
> [ 59.024080] [<c125a677>] ? sock_close+0x17/0x30
> [ 59.024080] [<c10e3dee>] ? __fput+0x16e/0x1b0
> [ 59.024080] [<c10e3bee>] ? fput+0x1e/0x20
> [ 59.024080] [<c10e23ff>] ? filp_close+0x3f/0x70
> [ 59.024080] [<c10e2498>] ? sys_close+0x68/0xb0
> [ 59.024080] [<c12e8de1>] ? syscall_call+0x7/0xb
> [ 59.024080] Pid: 2720, comm: a.out Not tainted 2.6.34-rc1 #2
> [ 59.024080] Call Trace:
> [ 59.024080] [<c100a997>] ? show_regs+0x27/0x30
> [ 59.024080] [<c101ef39>] nmi_watchdog_tick+0x189/0x240
> [ 59.024080] [<c1003bdf>] default_do_nmi+0x5f/0x180
> [ 59.024080] [<c1003d89>] do_nmi+0x89/0xa0
> [ 59.024080] [<c12e9541>] nmi_stack_correct+0x28/0x2d
> [ 59.024080] [<c101e719>] ? default_send_IPI_mask_logical+0xa9/0x140
> [ 59.024080] [<c101e8d1>] default_send_IPI_all+0x81/0x90
> [ 59.024080] [<c101f1d7>] arch_trigger_all_cpu_backtrace+0x37/0x60
> [ 59.024080] [<c108d637>] print_cpu_stall+0x47/0x90
> [ 59.024080] [<c108d6b8>] check_cpu_stall+0x38/0x60
> [ 59.024080] [<c108e665>] __rcu_pending+0x25/0xb0
> [ 59.024080] [<c108e71e>] rcu_pending+0x2e/0x70
> [ 59.024080] [<c108e04b>] rcu_check_callbacks+0x1b/0xb0
> [ 59.024080] [<c1053b6d>] update_process_times+0x3d/0x70
> [ 59.024080] [<c106ed1a>] tick_periodic+0x2a/0x80
> [ 59.024080] [<c106ed8e>] tick_handle_periodic+0x1e/0x80
> [ 59.024080] [<c1005e83>] timer_interrupt+0x23/0x50
> [ 59.024080] [<c108a125>] handle_IRQ_event+0x45/0x130
> [ 59.024080] [<c108c098>] handle_level_irq+0x88/0x100
> [ 59.024080] [<c108c010>] ? handle_level_irq+0x0/0x100
> [ 59.024080] <IRQ> [<c12e969a>] ? do_IRQ+0x4a/0xc0
> [ 59.024080] [<c100324e>] ? common_interrupt+0x2e/0x34
> [ 59.024080] [<c12a3c84>] ? tcp_v4_rcv+0x154/0x670
> [ 59.024080] [<c12873ab>] ? ip_local_deliver_finish+0xcb/0x1b0
> [ 59.024080] [<c1287316>] ? ip_local_deliver_finish+0x36/0x1b0
> [ 59.024080] [<c12874c5>] ? ip_local_deliver+0x35/0x40
> [ 59.024080] [<c12875db>] ? ip_rcv_finish+0x10b/0x2d0
> [ 59.024080] [<c1287906>] ? ip_rcv+0x166/0x240
> [ 59.024080] [<c1269d83>] ? netif_receive_skb+0x273/0x300
> [ 59.024080] [<c1269c1d>] ? netif_receive_skb+0x10d/0x300
> [ 59.024080] [<c1074f06>] ? trace_hardirqs_on_caller+0xa6/0x160
> [ 59.024080] [<c126a6d2>] ? process_backlog+0x82/0xb0
> [ 59.024080] [<c126aa0f>] ? net_rx_action+0x11f/0x190
> [ 59.024080] [<c104acd0>] ? __do_softirq+0x50/0x1c0
> [ 59.024080] [<c104ad2b>] ? __do_softirq+0xab/0x1c0
> [ 59.024080] [<c1269629>] ? dev_queue_xmit+0x209/0x370
> [ 59.024080] [<c104ac80>] ? __do_softirq+0x0/0x1c0
> [ 59.024080] <IRQ> [<c1269629>] ? dev_queue_xmit+0x209/0x370
> [ 59.024080] [<c104ab6e>] ? local_bh_enable+0x8e/0xd0
> [ 59.024080] [<c1269629>] ? dev_queue_xmit+0x209/0x370
> [ 59.024080] [<c126950e>] ? dev_queue_xmit+0xee/0x370
> [ 59.024080] [<c1289f7f>] ? ip_finish_output+0x10f/0x270
> [ 59.024080] [<c128a271>] ? ip_output+0x51/0x60
> [ 59.024080] [<c1289c5d>] ? ip_local_out+0x1d/0x20
> [ 59.024080] [<c128a48f>] ? ip_queue_xmit+0x20f/0x3e0
> [ 59.024080] [<c1066dd5>] ? sched_clock_local+0x15/0x170
> [ 59.024080] [<c10272f6>] ? __change_page_attr+0x106/0x1b0
> [ 59.024080] [<c10274df>] ? __change_page_attr_set_clr+0x2f/0x70
> [ 59.024080] [<c12a2a82>] ? tcp_v4_send_check+0x52/0xe0
> [ 59.024080] [<c129df10>] ? tcp_transmit_skb+0x2d0/0x420
> [ 59.024080] [<c10db72a>] ? poison_obj+0x2a/0x50
> [ 59.024080] [<c10ddc15>] ? __kmalloc_track_caller+0x185/0x1e0
> [ 59.024080] [<c129f3af>] ? tcp_write_xmit+0x13f/0x290
> [ 59.024080] [<c129f536>] ? __tcp_push_pending_frames+0x36/0xf0
> [ 59.024080] [<c12a0044>] ? tcp_send_fin+0x54/0xc0
> [ 59.024080] [<c12938b4>] ? tcp_close+0x2f4/0x370
> [ 59.024080] [<c107509b>] ? trace_hardirqs_off+0xb/0x10
> [ 59.024080] [<c12b15fe>] ? inet_release+0x4e/0x60
> [ 59.024080] [<c12598d6>] ? sock_release+0x66/0x80
> [ 59.024080] [<c125a677>] ? sock_close+0x17/0x30
> [ 59.024080] [<c10e3dee>] ? __fput+0x16e/0x1b0
> [ 59.024080] [<c10e3bee>] ? fput+0x1e/0x20
> [ 59.024080] [<c10e23ff>] ? filp_close+0x3f/0x70
> [ 59.024080] [<c10e2498>] ? sys_close+0x68/0xb0
> [ 59.024080] [<c12e8de1>] ? syscall_call+0x7/0xb
>
> Full log is at http://I-love.SAKURA.ne.jp/tmp/dmesg-2.6.34-rc1.txt
> Config is at http://I-love.SAKURA.ne.jp/tmp/config-2.6.34-rc1
Thanks for the report and program.
Nothing comes to my mind, I'll try to reproduce this here.
Is 2.6.33 OK ?
Thanks
^ permalink raw reply
* [patch 4/5] [PATCH] qeth: no recovery after layer mismatch (z/VM NICs)
From: frank.blaschka @ 2010-03-09 6:36 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, Ursula Braun
In-Reply-To: <20100309063652.706791000@de.ibm.com>
[-- Attachment #1: 605-qeth-layer-mismatch.diff --]
[-- Type: text/plain, Size: 2885 bytes --]
From: Ursula Braun <ursula.braun@de.ibm.com>
Depending on their definition in z/VM, virtual devices for z/VM
VSWITCH or GuestLAN must be configured either in layer2 or in
layer3 mode. If qeth detects a layer mismatch, device activation
fails. Trying to recover from this error cannot help; thus
scheduling a recovery should be avoided.
In addition, since recovery is forbidden during online setting of
a qeth device, existence of its network device is guaranteed for all
dev_close() calls in qeth. The corresponding checks can be removed.
Signed-off-by: Ursula Braun <ursula.braun@de.ibm.com>
Signed-off-by: Frank Blaschka <frank.blaschka@de.ibm.com>
---
drivers/s390/net/qeth_core_main.c | 3 ++-
drivers/s390/net/qeth_l2_main.c | 16 ++++++----------
drivers/s390/net/qeth_l3_main.c | 16 ++++++----------
3 files changed, 14 insertions(+), 21 deletions(-)
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -537,7 +537,8 @@ static void qeth_send_control_data_cb(st
dev_err(&card->gdev->dev,
"The qeth device is not configured "
"for the OSI layer required by z/VM\n");
- qeth_schedule_recovery(card);
+ else
+ qeth_schedule_recovery(card);
goto out;
}
--- a/drivers/s390/net/qeth_l2_main.c
+++ b/drivers/s390/net/qeth_l2_main.c
@@ -1071,11 +1071,9 @@ static int qeth_l2_recover(void *ptr)
dev_info(&card->gdev->dev,
"Device successfully recovered!\n");
else {
- if (card->dev) {
- rtnl_lock();
- dev_close(card->dev);
- rtnl_unlock();
- }
+ rtnl_lock();
+ dev_close(card->dev);
+ rtnl_unlock();
dev_warn(&card->gdev->dev, "The qeth device driver "
"failed to recover an error on the device\n");
}
@@ -1129,11 +1127,9 @@ static int qeth_l2_pm_resume(struct ccwg
if (card->state == CARD_STATE_RECOVER) {
rc = __qeth_l2_set_online(card->gdev, 1);
if (rc) {
- if (card->dev) {
- rtnl_lock();
- dev_close(card->dev);
- rtnl_unlock();
- }
+ rtnl_lock();
+ dev_close(card->dev);
+ rtnl_unlock();
}
} else
rc = __qeth_l2_set_online(card->gdev, 0);
--- a/drivers/s390/net/qeth_l3_main.c
+++ b/drivers/s390/net/qeth_l3_main.c
@@ -2219,11 +2219,9 @@ static int qeth_l3_stop_card(struct qeth
if (recovery_mode)
qeth_l3_stop(card->dev);
else {
- if (card->dev) {
- rtnl_lock();
- dev_close(card->dev);
- rtnl_unlock();
- }
+ rtnl_lock();
+ dev_close(card->dev);
+ rtnl_unlock();
}
if (!card->use_hard_stop) {
rc = qeth_send_stoplan(card);
@@ -3536,11 +3534,9 @@ static int qeth_l3_pm_resume(struct ccwg
if (card->state == CARD_STATE_RECOVER) {
rc = __qeth_l3_set_online(card->gdev, 1);
if (rc) {
- if (card->dev) {
- rtnl_lock();
- dev_close(card->dev);
- rtnl_unlock();
- }
+ rtnl_lock();
+ dev_close(card->dev);
+ rtnl_unlock();
}
} else
rc = __qeth_l3_set_online(card->gdev, 0);
^ permalink raw reply
* [patch 1/5] [PATCH] qeth: enable kmsg hash processing in qeth_core_sys.c
From: frank.blaschka @ 2010-03-09 6:36 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, Ursula Braun
In-Reply-To: <20100309063652.706791000@de.ibm.com>
[-- Attachment #1: 601-qeth-kmsg.diff --]
[-- Type: text/plain, Size: 654 bytes --]
From: Ursula Braun <ursula.braun@de.ibm.com>
provide qeth kmsg definitions to enable hash string generation for
kernel message created with dev_err().
Signed-off-by: Ursula Braun <ursula.braun@de.ibm.com>
Signed-off-by: Frank Blaschka <frank.blaschka@de.ibm.com>
---
drivers/s390/net/qeth_core_sys.c | 3 +++
1 file changed, 3 insertions(+)
--- a/drivers/s390/net/qeth_core_sys.c
+++ b/drivers/s390/net/qeth_core_sys.c
@@ -8,6 +8,9 @@
* Frank Blaschka <frank.blaschka@de.ibm.com>
*/
+#define KMSG_COMPONENT "qeth"
+#define pr_fmt(fmt) KMSG_COMPONENT ": " fmt
+
#include <linux/list.h>
#include <linux/rwsem.h>
#include <asm/ebcdic.h>
^ 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