* Re: [PATCH] act_mirred: don't go back.
From: jamal @ 2009-11-09 10:54 UTC (permalink / raw)
To: Changli Gao; +Cc: Stephen Hemminger, David S. Miller, netdev
In-Reply-To: <412e6f7f0911090031j16ab9c73o47f4d4d3ffd9be3a@mail.gmail.com>
On Mon, 2009-11-09 at 16:31 +0800, Changli Gao wrote:
> code cleanup! :)
I dont really see it as a cleanup to be honest. The code
is not less ugly even after the change;->
cheers,
jamal
^ permalink raw reply
* Re: [PATCH] act_mirred: don't go back.
From: Changli Gao @ 2009-11-09 11:06 UTC (permalink / raw)
To: hadi; +Cc: Stephen Hemminger, David S. Miller, netdev
In-Reply-To: <1257764086.6246.1.camel@bigi>
On Mon, Nov 9, 2009 at 6:54 PM, jamal <hadi@cyberus.ca> wrote:
> On Mon, 2009-11-09 at 16:31 +0800, Changli Gao wrote:
>
>> code cleanup! :)
>
> I dont really see it as a cleanup to be honest. The code
> is not less ugly even after the change;->
>
Maybe I should remove goto, I'll try later.
--
Regards，
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [PATCH] check the return value of ndo_select_queue()
From: Changli Gao @ 2009-11-09 11:29 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S. Miller, netdev
In-Reply-To: <4AF7E8C2.90707@gmail.com>
2009/11/9 Eric Dumazet <eric.dumazet@gmail.com>:
> Eric Dumazet a écrit :
>>
>> Yes, but this is a _very_ unlikely condition (a bug IMHO), so you could use :
>>
>> if (unlikely(queue_index >= dev->real_num_tx_queues)) {
>> ...
>> }
>>
>> (This unlikely() clause is implied in WARN... macros)
>>
>
> Also, you want to trigger this message only once, or ratelimit it at least
>
>
How about this version:
u16 queue_index;
u16 (*ndo_select_queue)(struct net_device*, struct sk_buff*);
unsigned int real_num_tx_queues = dev->real_num_tx_queues;
if (real_num_tx_queues == 1) {
queue_index = 0;
} else if ((ndo_select_queue = dev->netdev_ops->ndo_select_queue)) {
queue_index = ndo_select_queue(dev, skb);
if (unlikely(queue_index >= real_num_tx_queues)) {
if (net_ratelimit())
WARN(1, "%s selects TX queue %d, "
"but real number of TX queues is %d\n",
dev->name, queue_index,
real_num_tx_queues);
queue_index = 0;
}
} else {
queue_index = skb_tx_hash(dev, skb);
}
skb_set_queue_mapping(skb, queue_index);
return netdev_get_tx_queue(dev, queue_index);
--
Regards，
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: query: net-next section mismatch(es)
From: William Allen Simpson @ 2009-11-09 11:36 UTC (permalink / raw)
To: Andi Kleen; +Cc: David Miller, netdev
In-Reply-To: <87k4y056q2.fsf@basil.nowhere.org>
Andi Kleen wrote:
> You should report those to the respective maintainers of the code that throws
> the warning (see MAINTAINERS) and to linux-kernel. This is not really related
> to networking.
>
Thanks. I'll do that. I reported it here, as it was a fairly significant
change to net-next, and I assumed it was probably due to some recent problem
importing/merging some other branch into this one. But that's just a guess,
so it's phrased as a query....
^ permalink raw reply
* Re: [PATCH 1/8] udp: add a counter into udp_hslot
From: Eric Dumazet @ 2009-11-09 11:42 UTC (permalink / raw)
To: Andi Kleen
Cc: David S. Miller, Linux Netdev List, Lucian Adrian Grijincu,
Octavian Purdila
In-Reply-To: <87fx8o56eq.fsf@basil.nowhere.org>
Andi Kleen a écrit :
> Eric Dumazet <eric.dumazet@gmail.com> writes:
>>
>> +/**
>> + * struct udp_hslot - UDP hash slot
>> + *
>> + * @head: head of list of sockets
>> + * @count: number of sockets in 'head' list
>> + * @lock: spinlock protecting changes to head/count
>> + */
>> struct udp_hslot {
>> struct hlist_nulls_head head;
>> + int count;
>
> Do you really need an int? On 64bit it's free due to the alignment,
> but on 32bit x86 it's costly and you blow up the table considerably,
> increasing cache misses.
Even a short (16 bits) might be too small for IXIACOM :)
On 32bit x86, size of hash table is 512 slots max.
(one slot per 2MB of LOWMEM, rounded to power of two)
You are speaking of <= 4096 bytes overhead :)
>
> Again it would be nicer if that was a separate smaller table together
> with the spinlock.
Nice for space, not nice for fast path, because this means additional
cache miss to get the spinlock (multicast rx still needs to take spinlock),
and some guys want really fast (low latency) multicast rx.
>
> In theory could also put a short counter into the low level alignment
> bits of the pointer and perhaps convert the spinlock to a bitlock?
> Then all could collapse into a single pointer.
>
Not enough bits in low level alignment unfortunatly. We only could give a
hint (one bit is enough) of possibly long chain, but not allowing precise
choice of shortest chain.
Once multicast is converted to RCU, then we wont need one spinlock per slot
(it wont be used in fast path, only at bind()/close() time)
and yes, we can use a separate small array to contain hashed spinlocks,
or even a single spinlock for CONFIG_BASE_SMALL :)
^ permalink raw reply
* Re: [PATCHv8 3/3] vhost_net: a kernel-level virtio server
From: Michael S. Tsirkin @ 2009-11-09 11:55 UTC (permalink / raw)
To: Rusty Russell
Cc: netdev, virtualization, kvm, linux-kernel, mingo, linux-mm, akpm,
hpa, gregory.haskins, s.hetze, Daniel Walker, Eric Dumazet
In-Reply-To: <200911091647.29655.rusty@rustcorp.com.au>
On Mon, Nov 09, 2009 at 04:47:29PM +1030, Rusty Russell wrote:
> Actually, this looks wrong to me:
>
> + case VHOST_SET_VRING_BASE:
> ...
> + vq->avail_idx = vq->last_avail_idx = s.num;
>
> The last_avail_idx is part of the state of the driver. It needs to be saved
> and restored over susp/resume. The only reason it's not in the ring itself
> is because I figured the other side doesn't need to see it (which is true, but
> missed debugging opportunities as well as man-in-the-middle issues like this
> one). I had a patch which put this field at the end of the ring, I might
> resurrect it to avoid this problem. This is backwards compatible with all
> implementations. See patch at end.
>
> I would drop avail_idx altogether: get_user is basically free, and simplifies
> a lot. As most state is in the ring, all you need is an ioctl to save/restore
> the last_avail_idx.
I remembered another reason for caching head in avail_idx. Basically,
avail index could change between when I poll for descriptors and when I
want to notify guest.
So we could have:
- poll descriptors until empty
- notify
detects not empty so does not notify
And the way to solve it would be to return flag from
notify telling us to restart the polling loop.
But, this will be more code, on data path, than
what happens today where I simply keep state
from descriptor polling and use that to notify.
I also suspect that somehow this race in practice can not create
deadlocks ... but I prefer to avoid it, these things are very tricky: if
I see an empty ring, and stop processing descriptors, I want to trigger
notify on empty.
So if we want to avoid keeping "empty" state, IMO the best way would be
to pass a flag to vhost_signal that tells it that ring is empty.
Makes sense?
--
MST
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH 43/75] spider-net: declare MODULE_FIRMWARE
From: Jens Osterkamp @ 2009-11-09 11:58 UTC (permalink / raw)
To: Ben Hutchings; +Cc: David Miller, Ishizaki Kou, Jens Osterkamp, netdev, arnd
In-Reply-To: <1257630907.15927.445.camel@localhost>
On Saturday 07 November 2009, Ben Hutchings wrote:
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> ---
> drivers/net/spider_net.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/spider_net.c b/drivers/net/spider_net.c
> index 90e663f..782910c 100644
> --- a/drivers/net/spider_net.c
> +++ b/drivers/net/spider_net.c
> @@ -57,6 +57,7 @@ MODULE_AUTHOR("Utz Bacher <utz.bacher@de.ibm.com> and
> Jens Osterkamp " \ MODULE_DESCRIPTION("Spider Southbridge Gigabit Ethernet
> driver"); MODULE_LICENSE("GPL");
> MODULE_VERSION(VERSION);
> +MODULE_FIRMWARE(SPIDER_NET_FIRMWARE_NAME);
>
> static int rx_descriptors = SPIDER_NET_RX_DESCRIPTORS_DEFAULT;
> static int tx_descriptors = SPIDER_NET_TX_DESCRIPTORS_DEFAULT;
In spider_net, the filesystem is checked for a valid firmware first, if it
does not exist, the firmware is taken from the device tree (which is the
default case).
Do you expect this behaviour to cause problems together with MODULE_FIRMWARE ?
--
Best regards,
Jens Osterkamp
--------------------------------------------------------------------------------
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martin Jetter
Geschäftsführung: Erich Baier
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
^ permalink raw reply
* Re: [PATCH 1/8] udp: add a counter into udp_hslot
From: Andi Kleen @ 2009-11-09 12:10 UTC (permalink / raw)
To: Eric Dumazet
Cc: Andi Kleen, David S. Miller, Linux Netdev List,
Lucian Adrian Grijincu, Octavian Purdila
In-Reply-To: <4AF80040.6060501@gmail.com>
> > Do you really need an int? On 64bit it's free due to the alignment,
> > but on 32bit x86 it's costly and you blow up the table considerably,
> > increasing cache misses.
>
> Even a short (16 bits) might be too small for IXIACOM :)
True, but see below.
>
> On 32bit x86, size of hash table is 512 slots max.
> (one slot per 2MB of LOWMEM, rounded to power of two)
>
> You are speaking of <= 4096 bytes overhead :)
Well it's cache line overhead too. 32bit systems have often
small caches (i.e. Atom)
>
> >
> > Again it would be nicer if that was a separate smaller table together
> > with the spinlock.
>
> Nice for space, not nice for fast path, because this means additional
> cache miss to get the spinlock (multicast rx still needs to take spinlock),
> and some guys want really fast (low latency) multicast rx.
When the spinlock use is mostly local it should be in cache
(that's the nice thing about small tables, they don't drop out of cache)
>
> >
> > In theory could also put a short counter into the low level alignment
> > bits of the pointer and perhaps convert the spinlock to a bitlock?
> > Then all could collapse into a single pointer.
> >
>
> Not enough bits in low level alignment unfortunatly. We only could give a
> hint (one bit is enough) of possibly long chain, but not allowing precise
> choice of shortest chain.
Do we really need a precise answer here? I would assume an approximate
answer would be good enough using a saturating counter.
e.g. if both have >N just round robin.
Ok it would be tricky to decrement that again on unbind, but I assume just
continuing to RR later wouldn't be too bad.
The question is just if there are enough bits even for that.
> Once multicast is converted to RCU, then we wont need one spinlock per slot
> (it wont be used in fast path, only at bind()/close() time)
> and yes, we can use a separate small array to contain hashed spinlocks,
> or even a single spinlock for CONFIG_BASE_SMALL :)
Or a bit spinlock in the bucket low pointer bits.
With that (and the saturating counter) even the 64bit table could be shortened to half.
-Andi
--
ak@linux.intel.com -- Speaking for myself only.
^ permalink raw reply
* [PATCH net-next] Phonet: put sockets in a hash table
From: Rémi Denis-Courmont @ 2009-11-09 12:17 UTC (permalink / raw)
To: netdev; +Cc: Rémi Denis-Courmont
From: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
---
include/net/phonet/phonet.h | 1 +
net/phonet/af_phonet.c | 1 +
net/phonet/socket.c | 79 +++++++++++++++++++++++++++++-------------
3 files changed, 56 insertions(+), 25 deletions(-)
diff --git a/include/net/phonet/phonet.h b/include/net/phonet/phonet.h
index fdb05fa..7b11407 100644
--- a/include/net/phonet/phonet.h
+++ b/include/net/phonet/phonet.h
@@ -46,6 +46,7 @@ static inline struct pn_sock *pn_sk(struct sock *sk)
extern const struct proto_ops phonet_dgram_ops;
+void pn_sock_init(void);
struct sock *pn_find_sock_by_sa(struct net *net, const struct sockaddr_pn *sa);
void pn_deliver_sock_broadcast(struct net *net, struct sk_buff *skb);
void phonet_get_local_port_range(int *min, int *max);
diff --git a/net/phonet/af_phonet.c b/net/phonet/af_phonet.c
index 3bd1be6..8d3a55b 100644
--- a/net/phonet/af_phonet.c
+++ b/net/phonet/af_phonet.c
@@ -481,6 +481,7 @@ static int __init phonet_init(void)
if (err)
return err;
+ pn_sock_init();
err = sock_register(&phonet_proto_family);
if (err) {
printk(KERN_ALERT
diff --git a/net/phonet/socket.c b/net/phonet/socket.c
index 0412beb..4112b6e 100644
--- a/net/phonet/socket.c
+++ b/net/phonet/socket.c
@@ -45,13 +45,28 @@ static int pn_socket_release(struct socket *sock)
return 0;
}
+#define PN_HASHSIZE 16
+#define PN_HASHMASK (PN_HASHSIZE-1)
+
+
static struct {
- struct hlist_head hlist;
+ struct hlist_head hlist[PN_HASHSIZE];
spinlock_t lock;
-} pnsocks = {
- .hlist = HLIST_HEAD_INIT,
- .lock = __SPIN_LOCK_UNLOCKED(pnsocks.lock),
-};
+} pnsocks;
+
+void __init pn_sock_init(void)
+{
+ unsigned i;
+
+ for (i = 0; i < PN_HASHSIZE; i++)
+ INIT_HLIST_HEAD(pnsocks.hlist + i);
+ spin_lock_init(&pnsocks.lock);
+}
+
+static struct hlist_head *pn_hash_list(u16 obj)
+{
+ return pnsocks.hlist + (obj & PN_HASHMASK);
+}
/*
* Find address based on socket address, match only certain fields.
@@ -64,10 +79,11 @@ struct sock *pn_find_sock_by_sa(struct net *net, const struct sockaddr_pn *spn)
struct sock *rval = NULL;
u16 obj = pn_sockaddr_get_object(spn);
u8 res = spn->spn_resource;
+ struct hlist_head *hlist = pn_hash_list(obj);
spin_lock_bh(&pnsocks.lock);
- sk_for_each(sknode, node, &pnsocks.hlist) {
+ sk_for_each(sknode, node, hlist) {
struct pn_sock *pn = pn_sk(sknode);
BUG_ON(!pn->sobject); /* unbound socket */
@@ -99,31 +115,39 @@ struct sock *pn_find_sock_by_sa(struct net *net, const struct sockaddr_pn *spn)
/* Deliver a broadcast packet (only in bottom-half) */
void pn_deliver_sock_broadcast(struct net *net, struct sk_buff *skb)
{
- struct hlist_node *node;
- struct sock *sknode;
+ struct hlist_head *hlist = pnsocks.hlist;
+ unsigned h;
spin_lock(&pnsocks.lock);
- sk_for_each(sknode, node, &pnsocks.hlist) {
- struct sk_buff *clone;
+ for (h = 0; h < PN_HASHSIZE; h++) {
+ struct hlist_node *node;
+ struct sock *sknode;
- if (!net_eq(sock_net(sknode), net))
- continue;
- if (!sock_flag(sknode, SOCK_BROADCAST))
- continue;
+ sk_for_each(sknode, node, hlist) {
+ struct sk_buff *clone;
- clone = skb_clone(skb, GFP_ATOMIC);
- if (clone) {
- sock_hold(sknode);
- sk_receive_skb(sknode, clone, 0);
+ if (!net_eq(sock_net(sknode), net))
+ continue;
+ if (!sock_flag(sknode, SOCK_BROADCAST))
+ continue;
+
+ clone = skb_clone(skb, GFP_ATOMIC);
+ if (clone) {
+ sock_hold(sknode);
+ sk_receive_skb(sknode, clone, 0);
+ }
}
+ hlist++;
}
spin_unlock(&pnsocks.lock);
}
void pn_sock_hash(struct sock *sk)
{
+ struct hlist_head *hlist = pn_hash_list(pn_sk(sk)->sobject);
+
spin_lock_bh(&pnsocks.lock);
- sk_add_node(sk, &pnsocks.hlist);
+ sk_add_node(sk, hlist);
spin_unlock_bh(&pnsocks.lock);
}
EXPORT_SYMBOL(pn_sock_hash);
@@ -439,15 +463,20 @@ EXPORT_SYMBOL(pn_sock_get_port);
static struct sock *pn_sock_get_idx(struct seq_file *seq, loff_t pos)
{
struct net *net = seq_file_net(seq);
+ struct hlist_head *hlist = pnsocks.hlist;
struct hlist_node *node;
struct sock *sknode;
+ unsigned h;
- sk_for_each(sknode, node, &pnsocks.hlist) {
- if (!net_eq(net, sock_net(sknode)))
- continue;
- if (!pos)
- return sknode;
- pos--;
+ for (h = 0; h < PN_HASHSIZE; h++) {
+ sk_for_each(sknode, node, hlist) {
+ if (!net_eq(net, sock_net(sknode)))
+ continue;
+ if (!pos)
+ return sknode;
+ pos--;
+ }
+ hlist++;
}
return NULL;
}
--
1.6.3.3
^ permalink raw reply related
* Re: [PATCH] act_mirred: don't go back.
From: Changli Gao @ 2009-11-09 12:33 UTC (permalink / raw)
To: hadi; +Cc: Stephen Hemminger, David S. Miller, netdev
In-Reply-To: <1257764086.6246.1.camel@bigi>
On Mon, Nov 9, 2009 at 6:54 PM, jamal <hadi@cyberus.ca> wrote:
> On Mon, 2009-11-09 at 16:31 +0800, Changli Gao wrote:
>
>> code cleanup! :)
>
> I dont really see it as a cleanup to be honest. The code
> is not less ugly even after the change;->
>
static int tcf_mirred(struct sk_buff *skb, struct tc_action *a,
struct tcf_result *res)
{
struct net_device *dev;
struct sk_buff *skb2;
u32 at;
struct tcf_mirred *m = a->priv;
int retval, err = 1;
spin_lock(&m->tcf_lock);
m->tcf_tm.lastuse = jiffies;
if (m->tcfm_eaction != TCA_EGRESS_MIRROR &&
m->tcfm_eaction != TCA_EGRESS_REDIR) {
if (net_ratelimit())
printk("tcf_mirred unknown action %d\n",
m->tcfm_eaction);
goto out;
}
dev = m->tcfm_dev;
if (!(dev->flags&IFF_UP) ) {
if (net_ratelimit())
printk("mirred to Houston: device %s is gone!\n",
dev->name);
goto out;
}
skb2 = skb_act_clone(skb, GFP_ATOMIC);
if (skb2 == NULL)
goto out;
m->tcf_bstats.bytes += qdisc_pkt_len(skb2);
m->tcf_bstats.packets++;
at = G_TC_AT(skb->tc_verd);
if (!(at & AT_EGRESS)) {
if (m->tcfm_ok_push)
skb_push(skb2, skb2->dev->hard_header_len);
}
/* mirror is always swallowed */
if (m->tcfm_eaction != TCA_EGRESS_MIRROR)
skb2->tc_verd = SET_TC_FROM(skb2->tc_verd, at);
skb2->dev = dev;
skb2->iif = skb->dev->ifindex;
dev_queue_xmit(skb2);
err = 0;
out:
if (err) {
m->tcf_qstats.overlimits++;
m->tcf_bstats.bytes += qdisc_pkt_len(skb);
m->tcf_bstats.packets++;
/* should we be asking for packet to be dropped?
* may make sense for redirect case only
*/
retval = TC_ACT_SHOT;
} else {
retval = m->tcf_action;
}
spin_unlock(&m->tcf_lock);
return retval;
}
How about this version.
1. move skb_act_clone() after all the necessary checks, and it can
eliminate unnecessary skb_act_clone() if tcfm_eaction isn't correct.
2. there is one exit of the critical section.
3. jump forward instead of backward.
--
Regards，
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [RFC] netlink: add socket destruction notification
From: Patrick McHardy @ 2009-11-09 12:59 UTC (permalink / raw)
To: Johannes Berg; +Cc: netdev, Jouni Malinen, Thomas Graf
In-Reply-To: <1257762132.29454.161.camel@johannes.local>
Johannes Berg wrote:
> On Fri, 2009-11-06 at 16:37 +0100, Patrick McHardy wrote:
>
>>>> This seems pretty similar to the NETLINK_URELEASE notifier invoked
>>>> in netlink_release(). Wouldn't that one work as well?
>>> Hmm, it does seem similar, thanks for pointing it out. What exactly does
>>> the condition
>>> if (nlk->pid && !nlk->subscriptions) {
>>>
>>> mean though?
>> nlk->pid is non-zero for bound sockets, which is basically any
>> non-kernel socket which has either sent a message or explicitly
>> called bind(). nlk->subscriptions is zero for sockets not bound
>> to multicast groups.
>>
>> So effectively it invokes the notifier for all bound unicast
>> userspace sockets. Not sure why it doesn't invoke the notifier
>> for sockets that are used for both unicast and multicast
>> reception. If that is a problem I think the second condition
>> could be removed.
>
> Thanks for the explanation. I think we'd need the second condition
> removed, I don't see a reason to force a socket to not also have
> multicast RX if it's used for any of the purposes we're looking at this
> for. Guess we need to audit the callees to determine whether that's ok.
I've already done that. Its currently only used by netfilter
for which this change also makes sense.
> Can you quickly explain the difference between release and destruct?
release is called when the socket is closed, destruct is called
once all references are gone. I think with the synchonous processing
done nowadays they shouldn't make any difference, but release
should be fine in either case.
^ permalink raw reply
* Re: [RFC] netlink: add socket destruction notification
From: Johannes Berg @ 2009-11-09 13:03 UTC (permalink / raw)
To: Patrick McHardy; +Cc: netdev, Jouni Malinen, Thomas Graf
In-Reply-To: <4AF8122F.9060807@trash.net>
[-- Attachment #1: Type: text/plain, Size: 1239 bytes --]
On Mon, 2009-11-09 at 13:59 +0100, Patrick McHardy wrote:
> > Thanks for the explanation. I think we'd need the second condition
> > removed, I don't see a reason to force a socket to not also have
> > multicast RX if it's used for any of the purposes we're looking at this
> > for. Guess we need to audit the callees to determine whether that's ok.
>
> I've already done that. Its currently only used by netfilter
> for which this change also makes sense.
Cool, I arrived at that conclusion too, it seemed that it would
currently be somewhat strangely broken if you could add multicast groups
to those sockets used there. Not sure if you can though.
> > Can you quickly explain the difference between release and destruct?
>
> release is called when the socket is closed, destruct is called
> once all references are gone. I think with the synchonous processing
> done nowadays they shouldn't make any difference, but release
> should be fine in either case.
Ok, cool, thanks. Do you want me to send the change removing the
multicast check, or would you want to do that since you audited all the
netlink callers?
Also, it's called URELEASE for unicast -- should we rename it to just
RELEASE?
johannes
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [RFC] netlink: add socket destruction notification
From: Patrick McHardy @ 2009-11-09 13:19 UTC (permalink / raw)
To: Johannes Berg; +Cc: netdev, Jouni Malinen, Thomas Graf
In-Reply-To: <1257771787.29454.173.camel@johannes.local>
Johannes Berg wrote:
> On Mon, 2009-11-09 at 13:59 +0100, Patrick McHardy wrote:
>
>>> Thanks for the explanation. I think we'd need the second condition
>>> removed, I don't see a reason to force a socket to not also have
>>> multicast RX if it's used for any of the purposes we're looking at this
>>> for. Guess we need to audit the callees to determine whether that's ok.
>> I've already done that. Its currently only used by netfilter
>> for which this change also makes sense.
>
> Cool, I arrived at that conclusion too, it seemed that it would
> currently be somewhat strangely broken if you could add multicast groups
> to those sockets used there. Not sure if you can though.
I don't see anything preventing it.
>>> Can you quickly explain the difference between release and destruct?
>> release is called when the socket is closed, destruct is called
>> once all references are gone. I think with the synchonous processing
>> done nowadays they shouldn't make any difference, but release
>> should be fine in either case.
>
> Ok, cool, thanks. Do you want me to send the change removing the
> multicast check, or would you want to do that since you audited all the
> netlink callers?
Please go ahead.
> Also, it's called URELEASE for unicast -- should we rename it to just
> RELEASE?
I think URELEASE is still fine since won't necessarily get called
for sockets that are used for pure multicast reception when using
setsockopt to bind to groups. I also have a cleanup patch removing
unneccessary nlk->pid checks from netfilter which would clash with
a rename :)
^ permalink raw reply
* Re: [PATCH RFC] gianfar: Make polling safe with IRQs disabled
From: Anton Vorontsov @ 2009-11-09 13:32 UTC (permalink / raw)
To: David Miller; +Cc: afleming, jason.wessel, netdev, linuxppc-dev
In-Reply-To: <20091108.010533.212571001.davem@davemloft.net>
On Sun, Nov 08, 2009 at 01:05:33AM -0800, David Miller wrote:
...
> > When using KGDBoE, gianfar driver spits 'Interrupt problem' messages,
> > which appears to be a legitimate warning, i.e. we may end up calling
> > netif_receive_skb() or vlan_hwaccel_receive_skb() with IRQs disabled.
> >
> > This patch reworks the RX path so that if netpoll is enabled (the
> > only case when the driver don't know from what context the polling
> > may be called), we check whether IRQs are disabled, and if so we
> > fall back to safe variants of skb receiving functions.
>
> This is bogus, I'll tell you why.
>
> When you go into netif_receive_skb() we have a special check,
> "if (netpoll_receive_skb(..." that takes care of all of the
> details concerning doing a ->poll() from IRQ disabled context
> via netpoll.
>
> So this code you're adding should not be necessary.
>
> Or, explain to me why no other driver needs special logic in their
> ->poll() handler like this and does not run into any kinds of netpoll
> problems :-)
Hm, I was confused by the following note:
/**
* netif_receive_skb - process receive buffer from network
* @skb: buffer to process
...
* This function may only be called from softirq context and interrupts
* should be enabled.
Looking into the code though, I can indeed see that there
are netpoll checks, and __netpoll_rx() is actually called with
irqs disabled. So, in the end it appears that we should just
remove the 'Interrupt problem' message.
Thanks!
--
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2
^ permalink raw reply
* Re: [PATCH RFC] gianfar: Do not call skb recycling with disabled IRQs
From: Anton Vorontsov @ 2009-11-09 13:41 UTC (permalink / raw)
To: David Miller
Cc: jdl, linuxppc-dev, jason.wessel, afleming, netdev, buytenh,
shemminger
In-Reply-To: <20091108.010848.116517157.davem@davemloft.net>
On Sun, Nov 08, 2009 at 01:08:48AM -0800, David Miller wrote:
> From: Anton Vorontsov <avorontsov@ru.mvista.com>
> Date: Thu, 5 Nov 2009 19:57:38 +0300
>
> > But that basically means that with skb recycling we can't safely
> > use KGDBoE, though we can add something like this:
>
> Please stop adding special logic only to your driver to handle these
> things.
>
> Either it's a non-issue, or it's going to potentially be an issue for
> everyone using skb_recycle_check() in a NAPI driver, right?
>
> So why not add the "in_interrupt()" or whatever check to
> skb_recycle_check() and if the context is unsuitable return false (0)
> ok?
I think the check is needed, yes. But if we add just that check,
then we'll never do skb recycling in the tx path (since gianfar
always grabs irqsave spinlock during tx ring cleanup, so the check
will always return false).
So we must add the check to keep polling with disabled IRQs safe,
but we also should get rid of irqsave spinlock in the gianfar
driver (to make the skb recycling actually work in most cases).
Thanks!
--
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2
^ permalink raw reply
* [PATCH net-next] Phonet: allocate and copy for pipe TX without sock lock
From: Rémi Denis-Courmont @ 2009-11-09 14:06 UTC (permalink / raw)
To: netdev; +Cc: Rémi Denis-Courmont
From: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
---
net/phonet/pep.c | 29 ++++++++++++-----------------
1 files changed, 12 insertions(+), 17 deletions(-)
diff --git a/net/phonet/pep.c b/net/phonet/pep.c
index cbaa1d6..bdc17bd 100644
--- a/net/phonet/pep.c
+++ b/net/phonet/pep.c
@@ -843,7 +843,7 @@ static int pep_sendmsg(struct kiocb *iocb, struct sock *sk,
struct msghdr *msg, size_t len)
{
struct pep_sock *pn = pep_sk(sk);
- struct sk_buff *skb = NULL;
+ struct sk_buff *skb;
long timeo;
int flags = msg->msg_flags;
int err, done;
@@ -851,6 +851,16 @@ static int pep_sendmsg(struct kiocb *iocb, struct sock *sk,
if (msg->msg_flags & MSG_OOB || !(msg->msg_flags & MSG_EOR))
return -EOPNOTSUPP;
+ skb = sock_alloc_send_skb(sk, MAX_PNPIPE_HEADER + len,
+ flags & MSG_DONTWAIT, &err);
+ if (!skb)
+ return -ENOBUFS;
+
+ skb_reserve(skb, MAX_PHONET_HEADER + 3);
+ err = memcpy_fromiovec(skb_put(skb, len), msg->msg_iov, len);
+ if (err < 0)
+ goto outfree;
+
lock_sock(sk);
timeo = sock_sndtimeo(sk, flags & MSG_DONTWAIT);
if ((1 << sk->sk_state) & (TCPF_LISTEN|TCPF_CLOSE)) {
@@ -894,28 +904,13 @@ disabled:
goto disabled;
}
- if (!skb) {
- skb = sock_alloc_send_skb(sk, MAX_PNPIPE_HEADER + len,
- flags & MSG_DONTWAIT, &err);
- if (skb == NULL)
- goto out;
- skb_reserve(skb, MAX_PHONET_HEADER + 3);
-
- if (sk->sk_state != TCP_ESTABLISHED ||
- !atomic_read(&pn->tx_credits))
- goto disabled; /* sock_alloc_send_skb might sleep */
- }
-
- err = memcpy_fromiovec(skb_put(skb, len), msg->msg_iov, len);
- if (err < 0)
- goto out;
-
err = pipe_skb_send(sk, skb);
if (err >= 0)
err = len; /* success! */
skb = NULL;
out:
release_sock(sk);
+outfree:
kfree_skb(skb);
return err;
}
--
1.6.3.3
^ permalink raw reply related
* RE: PATCH: Network Device Naming mechanism and policy
From: Narendra_K @ 2009-11-09 14:41 UTC (permalink / raw)
To: md, Matt_Domsch
Cc: bryan, dannf, bhutchings, netdev, linux-hotplug, Jordan_Hargrave,
Charles_Rose, Sandeep_K_Shandilya
In-Reply-To: <20091106223524.GA27121@bongo.bofh.it>
>> > As a distribution developer I highly value solutions like
>this which
>> > do not require patching every application which deals with
>interface
>> > names and then teaching users about aliases which only
>work in some
>> > places and are unknown to the kernel.
>> Fair enough - but would you object if we changed the naming scheme
>> from eth%d to something else?
>I suppose that this would depend on what else. :-) Since you
>want radical changes I recommend that you design the new
>persistent naming infrastructure in a way that will allow root
>to choose to use the classic naming scheme, or many users will
>scream a lot and at least some distributions will do it anyway.
>I also expect that providing choice at the beginning of
>development may lead to more acceptance later if and when the
>new scheme will have proved itself to be superior (at least in
>some situations).
>You have tought about this for a long time and if so far you
>have not found a solution which is widely considered superior
>then I doubt that one will appear soon. Providing your
>favourite naming scheme as an optional add on will immediately
>benefit those who like it and greatly reduce opposition from
>those who do not.
In that way, I suppose char device node solution fits the scheme
perfectly. It doesn't change or interfere with the kernel's default
naming scheme (ethN) in any way. Also, the applications continue to work
the way they did and in addition to supporting traditional names, they
would also support pathnames. Whether all the user space applications
need to be patched can be discussed and debated. But, we can patch
applications like, installers and firewall code, which when don't see
determinism ("eth0 mapping to integrated port 1"), fail and cause very
high impact could be patched. Since users are already familiar with
pathnames like /dev/disk/by-id{label, uuid}, I suppose it might not be
very difficult to get used to pathnames like
/dev/netdev/by-chassis-label/Embedded_NIC_1. Would that be acceptable ?
With regards,
Narendra K
^ permalink raw reply
* Re: [PATCH 2/2] net/compat_ioctl: support SIOCWANDEV
From: Krzysztof Halasa @ 2009-11-09 14:48 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: David Miller, Daniel Walker, linux-kernel, hch, netdev
In-Reply-To: <200911082239.24698.arnd@arndb.de>
Arnd Bergmann <arnd@arndb.de> writes:
> This adds compat_ioctl support for SIOCWANDEV, which has
> always been missing.
It looks good, can't test at the moment.
--
Krzysztof Halasa
^ permalink raw reply
* Re: [PATCH 1/3] net: TCP thin-stream detection
From: Andreas Petlund @ 2009-11-09 15:24 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: Arnd Hannemann, William Allen Simpson, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, shemminger@vyatta.com,
davem@davemloft.net
In-Reply-To: <alpine.DEB.2.00.0911051536200.7024@wel-95.cs.helsinki.fi>
Ilpo Järvinen wrote:
> On Thu, 5 Nov 2009, Andreas Petlund wrote:
>
>> Arnd Hannemann wrote:
>>> One example: Consider standard NewReno non-SACK enabled flow:
>>> For some reasons two data packets get reordered.
>>> The TCP sender will produce a dupACK and an ACK.
>>> The dupACK will trigger (because of your logic) a spurious retransmit.
>>> The spurious retransmit will trigger a dupACK.
>>> This dupACK will again trigger a spurious retransmit.
>>> And this game will continue, unless a packet is dropped by coincidence.
>> Such an effect will be extremely rare. It will depend on the application
>> producing an extremely even flow of packets with just the right
>> interarrival time, and also on reordering of data (which also will
>> happen very seldom when the number of packets in flight are so low).
>> Even though it can happen, the data flow will progress (with spurious
>> retransmissions). The effect will stop as soon as the application sends
>> more than 4 segments in an RTT (which will disable the thin-stream
>> modifications) or less than 1 (which will cause all segments to be
>> successfully ACKed), or if, as you say, a packet is dropped.
>
> I'd simply workaround this problem by requiring SACK to be enabled for
> such a connection. This is reinforced by the fact that small windowed
> transfers want it certainly to be on anyway to get the best out of ACK
> flow even if there were some ACK losses.
>
Thanks. I will revise the patches based on all the feedback I have gotten
and get back to the list with a new version when I have done some more
testing.
Best regards,
Andreas
^ permalink raw reply
* [PATCH net-next-2.6] udp: bind() optimisation
From: Eric Dumazet @ 2009-11-09 15:26 UTC (permalink / raw)
To: David S. Miller
Cc: Linux Netdev List, Lucian Adrian Grijincu, Octavian Purdila
UDP bind() can be O(N^2) in some pathological cases.
Thanks to secondary hash tables, we can make it O(N)
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
include/linux/udp.h | 6 +++
include/net/udp.h | 3 +
net/ipv4/udp.c | 73 +++++++++++++++++++++++++++++++++++++-----
net/ipv6/udp.c | 14 ++++----
4 files changed, 80 insertions(+), 16 deletions(-)
diff --git a/include/linux/udp.h b/include/linux/udp.h
index 59f0ddf..03f72a2 100644
--- a/include/linux/udp.h
+++ b/include/linux/udp.h
@@ -88,6 +88,12 @@ static inline struct udp_sock *udp_sk(const struct sock *sk)
return (struct udp_sock *)sk;
}
+#define udp_portaddr_for_each_entry(__sk, node, list) \
+ hlist_nulls_for_each_entry(__sk, node, list, __sk_common.skc_portaddr_node)
+
+#define udp_portaddr_for_each_entry_rcu(__sk, node, list) \
+ hlist_nulls_for_each_entry_rcu(__sk, node, list, __sk_common.skc_portaddr_node)
+
#define IS_UDPLITE(__sk) (udp_sk(__sk)->pcflag)
#endif
diff --git a/include/net/udp.h b/include/net/udp.h
index af41850..5348d80 100644
--- a/include/net/udp.h
+++ b/include/net/udp.h
@@ -158,7 +158,8 @@ static inline void udp_lib_close(struct sock *sk, long timeout)
}
extern int udp_lib_get_port(struct sock *sk, unsigned short snum,
- int (*)(const struct sock*,const struct sock*));
+ int (*)(const struct sock *,const struct sock *),
+ unsigned int hash2_nulladdr);
/* net/ipv4/udp.c */
extern int udp_get_port(struct sock *sk, unsigned short snum,
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index d73e917..1eaf575 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -152,16 +152,49 @@ static int udp_lib_lport_inuse(struct net *net, __u16 num,
return 0;
}
+/*
+ * Note: we still hold spinlock of primary hash chain, so no other writer
+ * can insert/delete a socket with local_port == num
+ */
+static int udp_lib_lport_inuse2(struct net *net, __u16 num,
+ struct udp_hslot *hslot2,
+ struct sock *sk,
+ int (*saddr_comp)(const struct sock *sk1,
+ const struct sock *sk2))
+{
+ struct sock *sk2;
+ struct hlist_nulls_node *node;
+ int res = 0;
+
+ spin_lock(&hslot2->lock);
+ udp_portaddr_for_each_entry(sk2, node, &hslot2->head)
+ if (net_eq(sock_net(sk2), net) &&
+ sk2 != sk &&
+ (udp_sk(sk2)->udp_port_hash == num) &&
+ (!sk2->sk_reuse || !sk->sk_reuse) &&
+ (!sk2->sk_bound_dev_if || !sk->sk_bound_dev_if
+ || sk2->sk_bound_dev_if == sk->sk_bound_dev_if) &&
+ (*saddr_comp)(sk, sk2)) {
+ res = 1;
+ break;
+ }
+ spin_unlock(&hslot2->lock);
+ return res;
+}
+
/**
* udp_lib_get_port - UDP/-Lite port lookup for IPv4 and IPv6
*
* @sk: socket struct in question
* @snum: port number to look up
* @saddr_comp: AF-dependent comparison of bound local IP addresses
+ * @hash2_nulladdr: AF-dependant hash value in secondary hash chains,
+ * with NULL address
*/
int udp_lib_get_port(struct sock *sk, unsigned short snum,
int (*saddr_comp)(const struct sock *sk1,
- const struct sock *sk2))
+ const struct sock *sk2),
+ unsigned int hash2_nulladdr)
{
struct udp_hslot *hslot, *hslot2;
struct udp_table *udptable = sk->sk_prot->h.udp_table;
@@ -210,6 +243,30 @@ int udp_lib_get_port(struct sock *sk, unsigned short snum,
} else {
hslot = udp_hashslot(udptable, net, snum);
spin_lock_bh(&hslot->lock);
+ if (hslot->count > 10) {
+ int exist;
+ unsigned int slot2 = udp_sk(sk)->udp_portaddr_hash ^ snum;
+
+ slot2 &= udptable->mask;
+ hash2_nulladdr &= udptable->mask;
+
+ hslot2 = udp_hashslot2(udptable, slot2);
+ if (hslot->count < hslot2->count)
+ goto scan_primary_hash;
+
+ exist = udp_lib_lport_inuse2(net, snum, hslot2,
+ sk, saddr_comp);
+ if (!exist && (hash2_nulladdr != slot2)) {
+ hslot2 = udp_hashslot2(udptable, hash2_nulladdr);
+ exist = udp_lib_lport_inuse2(net, snum, hslot2,
+ sk, saddr_comp);
+ }
+ if (exist)
+ goto fail_unlock;
+ else
+ goto found;
+ }
+scan_primary_hash:
if (udp_lib_lport_inuse(net, snum, hslot, NULL, sk,
saddr_comp, 0))
goto fail_unlock;
@@ -255,12 +312,14 @@ static unsigned int udp4_portaddr_hash(struct net *net, __be32 saddr,
int udp_v4_get_port(struct sock *sk, unsigned short snum)
{
+ unsigned int hash2_nulladdr =
+ udp4_portaddr_hash(sock_net(sk), INADDR_ANY, snum);
+ unsigned int hash2_partial =
+ udp4_portaddr_hash(sock_net(sk), inet_sk(sk)->inet_rcv_saddr, 0);
+
/* precompute partial secondary hash */
- udp_sk(sk)->udp_portaddr_hash =
- udp4_portaddr_hash(sock_net(sk),
- inet_sk(sk)->inet_rcv_saddr,
- 0);
- return udp_lib_get_port(sk, snum, ipv4_rcv_saddr_equal);
+ udp_sk(sk)->udp_portaddr_hash = hash2_partial;
+ return udp_lib_get_port(sk, snum, ipv4_rcv_saddr_equal, hash2_nulladdr);
}
static inline int compute_score(struct sock *sk, struct net *net, __be32 saddr,
@@ -336,8 +395,6 @@ static inline int compute_score2(struct sock *sk, struct net *net,
return score;
}
-#define udp_portaddr_for_each_entry_rcu(__sk, node, list) \
- hlist_nulls_for_each_entry_rcu(__sk, node, list, __sk_common.skc_portaddr_node)
/* called with read_rcu_lock() */
static struct sock *udp4_lib_lookup2(struct net *net,
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 2915e1d..f4c85b2 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -100,12 +100,14 @@ static unsigned int udp6_portaddr_hash(struct net *net,
int udp_v6_get_port(struct sock *sk, unsigned short snum)
{
+ unsigned int hash2_nulladdr =
+ udp6_portaddr_hash(sock_net(sk), &in6addr_any, snum);
+ unsigned int hash2_partial =
+ udp6_portaddr_hash(sock_net(sk), &inet6_sk(sk)->rcv_saddr, 0);
+
/* precompute partial secondary hash */
- udp_sk(sk)->udp_portaddr_hash =
- udp6_portaddr_hash(sock_net(sk),
- &inet6_sk(sk)->rcv_saddr,
- 0);
- return udp_lib_get_port(sk, snum, ipv6_rcv_saddr_equal);
+ udp_sk(sk)->udp_portaddr_hash = hash2_partial;
+ return udp_lib_get_port(sk, snum, ipv6_rcv_saddr_equal, hash2_nulladdr);
}
static inline int compute_score(struct sock *sk, struct net *net,
@@ -181,8 +183,6 @@ static inline int compute_score2(struct sock *sk, struct net *net,
return score;
}
-#define udp_portaddr_for_each_entry_rcu(__sk, node, list) \
- hlist_nulls_for_each_entry_rcu(__sk, node, list, __sk_common.skc_portaddr_node)
/* called with read_rcu_lock() */
static struct sock *udp6_lib_lookup2(struct net *net,
^ permalink raw reply related
* Re: [PATCH] xfrm: SAD entries do not expire correctly after suspend-resume
From: Herbert Xu @ 2009-11-09 15:39 UTC (permalink / raw)
To: Yury Polyanskiy; +Cc: netdev, davem, peterz, yoshfuji, tglx, mingo
In-Reply-To: <20091108211249.2ecdfd38@penta.localdomain>
Yury Polyanskiy <ypolyans@princeton.edu> wrote:
>
> This fixes the following bug in the current implementation of
> net/xfrm: SAD entries timeouts do not count the time spent by the machine
> in the suspended state. This leads to the connectivity problems because
> after resuming local machine thinks that the SAD entry is still valid, while
> it has already been expired on the remote server.
>
> The cause of this is very simple: the timeouts in the net/xfrm are bound to
> the old mod_timer() timers. This patch reassigns them to the
> CLOCK_REALTIME hrtimer.
>
> I have been using this version of the patch for a few months on my
> machines without any problems. Also run a few stress tests w/o any
> issues.
>
> This version of the patch uses tasklet_hrtimer by Peter Zijlstra
> (commit 9ba5f0).
>
> This patch is against 2.6.31.4. Please CC me.
>
> Signed-off-by: Yury Polyanskiy <polyanskiy@gmail.com>
Thanks for the patch.
However, I have some reservations as to whether this is the ideal
situation. Unless I'm mistaken, this patch may cause IPsec SAs
to expire if the system clock was out of sync prior to IPsec startup
and is subsequently resynced by ntpdate or similar.
For example, it's quite common for clocks to be out-of-sync by
10 hours in Australia due to time zone issues with BIOS clocks.
So potentially ntpdate could move the clock forward by 10 hours
or more on bootup thus causing IPsec SAs to expire prematurely
with this patch.
This shouldn't really be a problem in itself except that there
are some dodgy IPsec gateways out there that refuse to reestablish
IPsec SAs if the interval between two successive connections is
too small. This could render the SA inoperable for hours.
So the upshot of all this is that we definitely want the effect
of this patch for suspend/resume, but it would be great if we can
avoid it for settimeofday(2).
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH 0/8 net-next-2.6] udp: optimisations
From: Octavian Purdila @ 2009-11-09 15:38 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, lgrijincu
In-Reply-To: <4AF7AA94.6090909@gmail.com>
On Monday 09 November 2009 07:37:24 you wrote:
> David Miller a écrit :
> > Looks great, all applied, thanks Eric.
>
> Thanks David, I'll make the remaining patches too.
>
> > I would even go so far as to say that the cutoff to the second hash
> > table should be even lower than 10, like maybe 4 or 5.
>
> Probably, but we want to avoid the secondary way if possible,
> as this path might have to traverse two different chains.
>
> (total of three cache line accesses to only take a look at chains
> head/count)
>
> Maybe we can change the heuristic to take into account this like that :
>
> if (hslot->count > 4) {
> ...
> if (hslot->count < hslot2->count * 2)
> goto begin_primary_hash_lookup;
>
> This is tuning, and needs benchmarking.
>
Eric, thanks a lot !
Lucian is currently testing it on our setup and once we iron out some IPv6
issues we are seeing he will follow up with some numbers.
^ permalink raw reply
* Re: Libertas related kernel crash
From: Daniel Mack @ 2009-11-09 15:53 UTC (permalink / raw)
To: netdev, libertas-dev; +Cc: linux-kernel, Michael Hirsch
In-Reply-To: <20091105120549.GQ14091@buzzloop.caiaq.de>
On Thu, Nov 05, 2009 at 01:05:49PM +0100, Daniel Mack wrote:
> On an ARM (PXA300) embdedded platform with a libertas chip connected via
> SDIO, we happen to see the kernel Ooops below once in a while.
>
> Any pointer on where to dig?
Some more input on this. Oopses similar to the one below are likely
triggered when switching from Ad-hoc to managed mode multiple times in a
row, and something seems corrupt the memory badly. I've searched for the
obvious (double frees, out-of-bound writes etc), but I couldn't find
anything yet. It is, however, related to the wireless core and/or the
libertas driver.
Any ideas, anyone?
Daniel
> [ 2659.715112] Unable to handle kernel NULL pointer dereference at virtual address 00000001
> [ 2659.723164] pgd = c5ca8000
> [ 2659.725846] [00000001] *pgd=a5cbc031, *pte=00000000, *ppte=00000000
> [ 2659.732062] Internal error: Oops: 13 [#4]
> [ 2659.736041] last sysfs file: /sys/devices/platform/pxa2xx-mci.0/mmc_host/mmc0/mmc0:0001/mmc0:0001:1/net/wlan0/address
> [ 2659.746573] Modules linked in: eeti_ts libertas_sdio pxamci ds2760_battery w1_ds2760 wire
> [ 2659.754698] CPU: 0 Tainted: G D (2.6.32-rc6 #1)
> [ 2659.760255] PC is at kmem_cache_alloc+0x30/0x90
> [ 2659.764774] LR is at inet_bind_bucket_create+0x18/0x5c
> [ 2659.769876] pc : [<c00a28d8>] lr : [<c02a4b24>] psr: 20000093
> [ 2659.769887] sp : c5c27d88 ip : c78ae4a0 fp : 00006e49
> [ 2659.781279] r10: 00008928 r9 : c04fcfac r8 : c0532148
> [ 2659.786466] r7 : 00000020 r6 : 00000020 r5 : 60000013 r4 : 00000001
> [ 2659.792945] r3 : 00000000 r2 : c78ae4a0 r1 : 00000020 r0 : c04d42e4
> [ 2659.799427] Flags: nzCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment user
> [ 2659.806602] Control: 0000397f Table: a5ca8018 DAC: 00000015
> [ 2659.812304] Process p0-renderer (pid: 1527, stack limit = 0xc5c26278)
> [ 2659.818697] Stack: (0xc5c27d88 to 0xc5c28000)
> [ 2659.823029] 7d80: c5cc5300 c5cb1000 c5c18800 c78ae4a0 00008928 00008928
> [ 2659.831155] 7da0: 00000001 c02a4b24 c2388c2e 00000000 c7eb6200 c02a4c8c c02a4200 c2388c2d
> [ 2659.839287] 7dc0: c04fcfac 00000000 0000ee48 00008000 fb0ca8c0 c7eb6200 c04fcfac 00000000
> [ 2659.847412] 7de0: 000038e5 fb0ca8c0 fb0ca8c0 c7eb6200 00000000 c02a4e28 c02a40e4 00000000
> [ 2659.855543] 7e00: 00000000 00000000 00000000 c02b99d4 00000001 00000000 c5c27f08 00000000
> [ 2659.863669] 7e20: c5c7b500 000005a8 00000000 00000000 000200da c5c7b500 c0069a74 c5c27e3c
> [ 2659.871802] 7e40: c5c27e3c c0049fdc be96a8fc c0275d70 c5c27e88 00000006 00000005 00000001
> [ 2659.879933] 7e60: 00000000 00000000 00000000 fb0ca8c0 1d0ca8c0 00000000 00000000 00000000
> [ 2659.888057] 7e80: 00000000 00000000 00000000 00000000 00000006 38e50000 00000000 c7ff6e00
> [ 2659.896183] 7ea0: 000063f8 c5c27f08 00000010 c7eb6200 c5c27f08 c00440c4 c5c26000 c76cd2c0
> [ 2659.904314] 7ec0: 00000802 c02c4f70 0000c1ff 00000001 00000000 00000000 00000000 00045108
> [ 2659.912441] 7ee0: 00000010 c76cd2c0 00045108 00000010 c5c27f08 c00440c4 c5c26000 40343800
> [ 2659.920573] 7f00: 00044440 c0276888 38e50002 fb0ca8c0 00000000 00000000 000063f8 00000000
> [ 2659.928698] 7f20: 00000000 be96a858 c5c27f48 000450c0 000000c5 c00440c4 c5c26000 40343800
> [ 2659.936829] 7f40: 00044440 c00a9864 000063f8 00000000 00000005 c047c1ff 00000001 00000000
> [ 2659.944952] 7f60: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000003
> [ 2659.953078] 7f80: 00000000 c7b5aa00 00000000 00000000 00000000 00046cb8 00045108 40370ef8
> [ 2659.961203] 7fa0: 0000011b c0043f40 00046cb8 00045108 0000000a 00045108 00000010 0000001c
> [ 2659.969328] 7fc0: 00046cb8 00045108 40370ef8 0000011b 00000000 00043178 40343800 00044440
> [ 2659.977451] 7fe0: 40371050 be96a968 4035b3c8 40790638 60000010 0000000a 00000000 00000000
> [ 2659.985599] [<c00a28d8>] (kmem_cache_alloc+0x30/0x90) from [<c02a4b24>] (inet_bind_bucket_create+0x18/0x5c)
> [ 2659.995296] [<c02a4b24>] (inet_bind_bucket_create+0x18/0x5c) from [<c02a4c8c>] (__inet_hash_connect+0x124/0x280)
> [ 2660.005415] [<c02a4c8c>] (__inet_hash_connect+0x124/0x280) from [<c02a4e28>] (inet_hash_connect+0x40/0x50)
> [ 2660.015034] [<c02a4e28>] (inet_hash_connect+0x40/0x50) from [<c02b99d4>] (tcp_v4_connect+0x2a8/0x420)
> [ 2660.024211] [<c02b99d4>] (tcp_v4_connect+0x2a8/0x420) from [<c02c4f70>] (inet_stream_connect+0xac/0x26c)
> [ 2660.033653] [<c02c4f70>] (inet_stream_connect+0xac/0x26c) from [<c0276888>] (sys_connect+0x6c/0x90)
> [ 2660.042662] [<c0276888>] (sys_connect+0x6c/0x90) from [<c0043f40>] (ret_fast_syscall+0x0/0x28)
> [ 2660.051228] Code: e5904080 e5907090 e3540000 1590308c (17943103) [ 2660.057440] ---[ end trace 416b23b4578ffa42 ]---
> [ 2660.062021] Kernel panic - not syncing: Fatal exception in interrupt
> [ 2660.068400] [<c00486cc>] (unwind_backtrace+0x0/0xdc) from [<c0351758>] (panic+0x34/0x120)
> [ 2660.076555] [<c0351758>] (panic+0x34/0x120) from [<c004758c>] (die+0x14c/0x178)
> [ 2660.083823] [<c004758c>] (die+0x14c/0x178) from [<c0049900>] (__do_kernel_fault+0x68/0x80)
> [ 2660.092060] [<c0049900>] (__do_kernel_fault+0x68/0x80) from [<c004b678>] (do_alignment+0x59c/0x700)
> [ 2660.101067] [<c004b678>] (do_alignment+0x59c/0x700) from [<c00432c8>] (do_DataAbort+0x34/0x94)
> [ 2660.109649] [<c00432c8>] (do_DataAbort+0x34/0x94) from [<c0043acc>] (__dabt_svc+0x4c/0x60)
> [ 2660.117874] Exception stack(0xc5c27d40 to 0xc5c27d88)
> [ 2660.122898] 7d40: c04d42e4 00000020 c78ae4a0 00000000 00000001 60000013 00000020 00000020
> [ 2660.131046] 7d60: c0532148 c04fcfac 00008928 00006e49 c78ae4a0 c5c27d88 c02a4b24 c00a28d8
> [ 2660.139189] 7d80: 20000093 ffffffff
> [ 2660.142673] [<c0043acc>] (__dabt_svc+0x4c/0x60) from [<c00a28d8>] (kmem_cache_alloc+0x30/0x90)
> [ 2660.151265] [<c00a28d8>] (kmem_cache_alloc+0x30/0x90) from [<c02a4b24>] (inet_bind_bucket_create+0x18/0x5c)
> [ 2660.160998] [<c02a4b24>] (inet_bind_bucket_create+0x18/0x5c) from [<c02a4c8c>] (__inet_hash_connect+0x124/0x280)
> [ 2660.171148] [<c02a4c8c>] (__inet_hash_connect+0x124/0x280) from [<c02a4e28>] (inet_hash_connect+0x40/0x50)
> [ 2660.180780] [<c02a4e28>] (inet_hash_connect+0x40/0x50) from [<c02b99d4>] (tcp_v4_connect+0x2a8/0x420)
> [ 2660.189971] [<c02b99d4>] (tcp_v4_connect+0x2a8/0x420) from [<c02c4f70>] (inet_stream_connect+0xac/0x26c)
> [ 2660.199424] [<c02c4f70>] (inet_stream_connect+0xac/0x26c) from [<c0276888>] (sys_connect+0x6c/0x90)
> [ 2660.208443] [<c0276888>] (sys_connect+0x6c/0x90) from [<c0043f40>] (ret_fast_syscall+0x0/0x28)
>
^ permalink raw reply
* [net-next-2.6 PATCH v5 0/5 RFC] TCPCT part1: cookie option exchange
From: William Allen Simpson @ 2009-11-09 16:07 UTC (permalink / raw)
To: Linux Kernel Network Developers
I'm torn at the moment, as the net-next tree modules don't compile today.
Although 'make vmlinux' compiles this successfully, it is not tested.
Keep in mind that earlier variants were extensively tested. But at least
this helps move the review comment process along....
Updated to patch cleanly with recent commits.
Changes from previous version:
* swapped conditional tests, requested by Eric and Joe.
* separate patch for constant cleanup, requested by Ilpo.
* added hash generation of Responder Cookie with RCU locking.
* shuffled a few bits to save another byte of tcp socket space.
^ permalink raw reply
* [net-next-2.6 PATCH v5 1/5 RFC] TCPCT part 1a: add request_values parameter for sending SYNACK
From: William Allen Simpson @ 2009-11-09 16:12 UTC (permalink / raw)
To: Linux Kernel Network Developers
In-Reply-To: <4AF83E2D.1050401@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1000 bytes --]
Add optional function parameters associated with sending SYNACK.
These parameters are not needed after sending SYNACK, and are not
used for retransmission. Avoids extending struct tcp_request_sock,
and avoids allocating kernel memory.
Also affects DCCP as it uses common struct request_sock_ops,
but this parameter is currently reserved for future use.
Signed-off-by: William.Allen.Simpson@gmail.com
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
---
include/net/request_sock.h | 8 +++++++-
include/net/tcp.h | 1 +
net/dccp/ipv4.c | 5 +++--
net/dccp/ipv6.c | 5 +++--
net/dccp/minisocks.c | 2 +-
net/ipv4/inet_connection_sock.c | 2 +-
net/ipv4/tcp_ipv4.c | 17 ++++++++++-------
net/ipv4/tcp_minisocks.c | 2 +-
net/ipv4/tcp_output.c | 1 +
net/ipv6/tcp_ipv6.c | 28 +++++++++++++---------------
10 files changed, 41 insertions(+), 30 deletions(-)
[-- Attachment #2: TCPCT+1a5.patch --]
[-- Type: text/plain, Size: 8638 bytes --]
diff --git a/include/net/request_sock.h b/include/net/request_sock.h
index c719084..c9b50eb 100644
--- a/include/net/request_sock.h
+++ b/include/net/request_sock.h
@@ -27,13 +27,19 @@ struct sk_buff;
struct dst_entry;
struct proto;
+/* empty to "strongly type" an otherwise void parameter.
+ */
+struct request_values {
+};
+
struct request_sock_ops {
int family;
int obj_size;
struct kmem_cache *slab;
char *slab_name;
int (*rtx_syn_ack)(struct sock *sk,
- struct request_sock *req);
+ struct request_sock *req,
+ struct request_values *rvp);
void (*send_ack)(struct sock *sk, struct sk_buff *skb,
struct request_sock *req);
void (*send_reset)(struct sock *sk,
diff --git a/include/net/tcp.h b/include/net/tcp.h
index bf20f88..25bf3ba 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -443,6 +443,7 @@ extern int tcp_connect(struct sock *sk);
extern struct sk_buff * tcp_make_synack(struct sock *sk,
struct dst_entry *dst,
+ struct request_values *rvp,
struct request_sock *req);
extern int tcp_disconnect(struct sock *sk, int flags);
diff --git a/net/dccp/ipv4.c b/net/dccp/ipv4.c
index 2423a08..efbcfdc 100644
--- a/net/dccp/ipv4.c
+++ b/net/dccp/ipv4.c
@@ -477,7 +477,8 @@ static struct dst_entry* dccp_v4_route_skb(struct net *net, struct sock *sk,
return &rt->u.dst;
}
-static int dccp_v4_send_response(struct sock *sk, struct request_sock *req)
+static int dccp_v4_send_response(struct sock *sk, struct request_sock *req,
+ struct request_values *rv_unused)
{
int err = -1;
struct sk_buff *skb;
@@ -626,7 +627,7 @@ int dccp_v4_conn_request(struct sock *sk, struct sk_buff *skb)
dreq->dreq_iss = dccp_v4_init_sequence(skb);
dreq->dreq_service = service;
- if (dccp_v4_send_response(sk, req))
+ if (dccp_v4_send_response(sk, req, NULL))
goto drop_and_free;
inet_csk_reqsk_queue_hash_add(sk, req, DCCP_TIMEOUT_INIT);
diff --git a/net/dccp/ipv6.c b/net/dccp/ipv6.c
index 50ea91a..6574215 100644
--- a/net/dccp/ipv6.c
+++ b/net/dccp/ipv6.c
@@ -241,7 +241,8 @@ out:
}
-static int dccp_v6_send_response(struct sock *sk, struct request_sock *req)
+static int dccp_v6_send_response(struct sock *sk, struct request_sock *req,
+ struct request_values *rv_unused)
{
struct inet6_request_sock *ireq6 = inet6_rsk(req);
struct ipv6_pinfo *np = inet6_sk(sk);
@@ -468,7 +469,7 @@ static int dccp_v6_conn_request(struct sock *sk, struct sk_buff *skb)
dreq->dreq_iss = dccp_v6_init_sequence(skb);
dreq->dreq_service = service;
- if (dccp_v6_send_response(sk, req))
+ if (dccp_v6_send_response(sk, req, NULL))
goto drop_and_free;
inet6_csk_reqsk_queue_hash_add(sk, req, DCCP_TIMEOUT_INIT);
diff --git a/net/dccp/minisocks.c b/net/dccp/minisocks.c
index 5ca49ce..af226a0 100644
--- a/net/dccp/minisocks.c
+++ b/net/dccp/minisocks.c
@@ -184,7 +184,7 @@ struct sock *dccp_check_req(struct sock *sk, struct sk_buff *skb,
* counter (backoff, monitored by dccp_response_timer).
*/
req->retrans++;
- req->rsk_ops->rtx_syn_ack(sk, req);
+ req->rsk_ops->rtx_syn_ack(sk, req, NULL);
}
/* Network Duplicate, discard packet */
return NULL;
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index 26fb50e..ad098d6 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -531,7 +531,7 @@ void inet_csk_reqsk_queue_prune(struct sock *parent,
&expire, &resend);
if (!expire &&
(!resend ||
- !req->rsk_ops->rtx_syn_ack(parent, req) ||
+ !req->rsk_ops->rtx_syn_ack(parent, req, NULL) ||
inet_rsk(req)->acked)) {
unsigned long timeo;
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 657ae33..f83ac91 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -743,7 +743,8 @@ static void tcp_v4_reqsk_send_ack(struct sock *sk, struct sk_buff *skb,
* socket.
*/
static int __tcp_v4_send_synack(struct sock *sk, struct request_sock *req,
- struct dst_entry *dst)
+ struct dst_entry *dst,
+ struct request_values *rvp)
{
const struct inet_request_sock *ireq = inet_rsk(req);
int err = -1;
@@ -753,7 +754,7 @@ static int __tcp_v4_send_synack(struct sock *sk, struct request_sock *req,
if (!dst && (dst = inet_csk_route_req(sk, req)) == NULL)
return -1;
- skb = tcp_make_synack(sk, dst, req);
+ skb = tcp_make_synack(sk, dst, rvp, req);
if (skb) {
struct tcphdr *th = tcp_hdr(skb);
@@ -774,9 +775,10 @@ static int __tcp_v4_send_synack(struct sock *sk, struct request_sock *req,
return err;
}
-static int tcp_v4_send_synack(struct sock *sk, struct request_sock *req)
+static int tcp_v4_send_synack(struct sock *sk, struct request_sock *req,
+ struct request_values *rvp)
{
- return __tcp_v4_send_synack(sk, req, NULL);
+ return __tcp_v4_send_synack(sk, req, NULL, rvp);
}
/*
@@ -1211,13 +1213,13 @@ static struct timewait_sock_ops tcp_timewait_sock_ops = {
int tcp_v4_conn_request(struct sock *sk, struct sk_buff *skb)
{
- struct inet_request_sock *ireq;
struct tcp_options_received tmp_opt;
struct request_sock *req;
+ struct inet_request_sock *ireq;
+ struct dst_entry *dst = NULL;
__be32 saddr = ip_hdr(skb)->saddr;
__be32 daddr = ip_hdr(skb)->daddr;
__u32 isn = TCP_SKB_CB(skb)->when;
- struct dst_entry *dst = NULL;
#ifdef CONFIG_SYN_COOKIES
int want_cookie = 0;
#else
@@ -1337,7 +1339,8 @@ int tcp_v4_conn_request(struct sock *sk, struct sk_buff *skb)
}
tcp_rsk(req)->snt_isn = isn;
- if (__tcp_v4_send_synack(sk, req, dst) || want_cookie)
+ if (__tcp_v4_send_synack(sk, req, dst, NULL)
+ || want_cookie)
goto drop_and_free;
inet_csk_reqsk_queue_hash_add(sk, req, TCP_TIMEOUT_INIT);
diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index a9d34e2..fb68bab 100644
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -537,7 +537,7 @@ struct sock *tcp_check_req(struct sock *sk, struct sk_buff *skb,
* Enforce "SYN-ACK" according to figure 8, figure 6
* of RFC793, fixed by RFC1122.
*/
- req->rsk_ops->rtx_syn_ack(sk, req);
+ req->rsk_ops->rtx_syn_ack(sk, req, NULL);
return NULL;
}
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 616c686..784e6ee 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -2224,6 +2224,7 @@ int tcp_send_synack(struct sock *sk)
/* Prepare a SYN-ACK. */
struct sk_buff *tcp_make_synack(struct sock *sk, struct dst_entry *dst,
+ struct request_values *rvp,
struct request_sock *req)
{
struct inet_request_sock *ireq = inet_rsk(req);
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 696a22f..6951827 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -461,7 +461,8 @@ out:
}
-static int tcp_v6_send_synack(struct sock *sk, struct request_sock *req)
+static int tcp_v6_send_synack(struct sock *sk, struct request_sock *req,
+ struct request_values *rvp)
{
struct inet6_request_sock *treq = inet6_rsk(req);
struct ipv6_pinfo *np = inet6_sk(sk);
@@ -499,7 +500,7 @@ static int tcp_v6_send_synack(struct sock *sk, struct request_sock *req)
if ((err = xfrm_lookup(sock_net(sk), &dst, &fl, sk, 0)) < 0)
goto done;
- skb = tcp_make_synack(sk, dst, req);
+ skb = tcp_make_synack(sk, dst, rvp, req);
if (skb) {
struct tcphdr *th = tcp_hdr(skb);
@@ -1161,13 +1162,13 @@ static struct sock *tcp_v6_hnd_req(struct sock *sk,struct sk_buff *skb)
*/
static int tcp_v6_conn_request(struct sock *sk, struct sk_buff *skb)
{
+ struct tcp_options_received tmp_opt;
+ struct request_sock *req;
struct inet6_request_sock *treq;
struct ipv6_pinfo *np = inet6_sk(sk);
- struct tcp_options_received tmp_opt;
struct tcp_sock *tp = tcp_sk(sk);
- struct request_sock *req = NULL;
- __u32 isn = TCP_SKB_CB(skb)->when;
struct dst_entry *dst = __sk_dst_get(sk);
+ __u32 isn = TCP_SKB_CB(skb)->when;
#ifdef CONFIG_SYN_COOKIES
int want_cookie = 0;
#else
@@ -1239,23 +1240,20 @@ static int tcp_v6_conn_request(struct sock *sk, struct sk_buff *skb)
isn = tcp_v6_init_sequence(skb);
}
-
tcp_rsk(req)->snt_isn = isn;
security_inet_conn_request(sk, skb, req);
- if (tcp_v6_send_synack(sk, req))
- goto drop;
+ if (tcp_v6_send_synack(sk, req, NULL)
+ || want_cookie)
+ goto drop_and_free;
- if (!want_cookie) {
- inet6_csk_reqsk_queue_hash_add(sk, req, TCP_TIMEOUT_INIT);
- return 0;
- }
+ inet6_csk_reqsk_queue_hash_add(sk, req, TCP_TIMEOUT_INIT);
+ return 0;
+drop_and_free:
+ reqsk_free(req);
drop:
- if (req)
- reqsk_free(req);
-
return 0; /* don't send reset */
}
--
1.6.3.3
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox