* Re: [PATCH] net: init ingress queue
From: Jarek Poplawski @ 2010-12-04 13:36 UTC (permalink / raw)
To: Changli Gao
Cc: David S. Miller, Eric Dumazet, Tom Herbert, Jiri Pirko, netdev
In-Reply-To: <1291465901-3189-1-git-send-email-xiaosuo@gmail.com>
Changli Gao wrote:
> The dev field of ingress queue is forgot to initialized, then NULL
> pointer dereference happens in qdisc_alloc().
>
> Move inits of tx queues to netif_alloc_netdev_queues().
>
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
> ---
> v2: factorize the two inits in netdev_init_queues() and move inits of
> tx queues to netif_alloc_netdev_queues().
> net/core/dev.c | 35 +++++++++++++----------------------
> 1 file changed, 13 insertions(+), 22 deletions(-)
> diff --git a/net/core/dev.c b/net/core/dev.c
> index cd24374..4a7fc32 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
I guess you should mention it's net-next problem, because some people
might be unnecessarily concerned (why they don't have such an oops ;-)
Jarek P.
^ permalink raw reply
* Re: [PATCH 1/3] ifb: remove the useless debug stats
From: jamal @ 2010-12-04 14:12 UTC (permalink / raw)
To: Changli Gao; +Cc: netdev
In-Reply-To: <1291442121-3302-1-git-send-email-xiaosuo@gmail.com>
On Sat, 2010-12-04 at 13:55 +0800, Changli Gao wrote:
> These debug stats are not exported, and become useless.
>
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
Given the complexity of ifb, these stats have proved useful over
the years - I would typically ask someone to printk them etc.
If you want me to get me excited (in a good way) you could export these
via link XSTATS ;-> If you dont have time, then it is likely
we are stable enough we can kill them in which case, here you go:
Signed-off-by: Jamal Hadi Salim <hadi@cyberus.ca>
cheers,
jamal
^ permalink raw reply
* Re: [PATCH 2/3] ifb: remove unused macro TX_TIMEOUT
From: jamal @ 2010-12-04 14:13 UTC (permalink / raw)
To: Changli Gao; +Cc: netdev
In-Reply-To: <1291442121-3302-2-git-send-email-xiaosuo@gmail.com>
On Sat, 2010-12-04 at 13:55 +0800, Changli Gao wrote:
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
Signed-off-by: Jamal Hadi Salim <hadi@cyberus.ca>
cheers,
jamal
^ permalink raw reply
* Re: Question about __alloc_skb() speedup
From: Junchang Wang @ 2010-12-04 14:18 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1291373429.2897.96.camel@edumazet-laptop>
On Fri, Dec 03, 2010 at 11:50:29AM +0100, Eric Dumazet wrote:
>
>This is because __alloc_skb() is generic :
>
>We dont know if the skb->data is going to be used right after or not at
>all.
>
>For example, NIC drivers call __alloc_skb() to refill their RX ring
>buffer. There is no gain to prefetch data in this case since the data is
>going to be written by the NIC hardware. The reverse would be needed
>actually : ask to local cpu to evict data from its cache, so that device
>can DMA it faster (less bus transactions)
Thanks for your explanation. Now I understand your patch. :)
>
>By the way, adding prefetchw() right before the "return skb;" is
>probably not very useful.
Did you mean adding prefetchw() right before the "return" instruction
doesn't gain beneficial effect?
My understanding is that what prefetch instructions do is to hint cpu
to hot some cache lines, so this kind of prefetch could also benefit
following functions.
>You can certainly try to add the prefetchw()
>in pktgen itself, since you know for sure you are going to write the
>data.
>
>I dont understand your 10% speedup because pktgen actually uses
>__netdev_alloc_skb(), so it calls skb_reserve(skb, NET_SKB_PAD) : your
>prefetchw is bringing a cache line that wont be used at all by pktgen.
Thanks for corrections. Please check the following code.
>
>I would say 10% sounds highly suspect to me...
>
I repeated the experiment a number of times (>10). The number of 10% was
careless, but I confirmed there's speedup, which fall into (%3, 8%). I
found it hard to give the exact number of speedup because I had to reboot
the system each time I added the prefetch code.
I noticed the hardware prefetch(Hardware Prefetcher and Adjacent Cache
Line Prefetch) was turn on by default. I doubt my faulty code gain benefit
from hardware prefetch. Without those two options, the performance of
both pktgens reduced by 5%-8% and I can hardly see beneficial effect from
my previous code.
I added the prefetchw() in pktgen as follows:
diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 2953b2a..512f1ae 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -2660,6 +2660,7 @@ static struct sk_buff *fill_packet_ipv4(struct net_device *odev,
sprintf(pkt_dev->result, "No memory");
return NULL;
}
+ prefetchw(skb->data);
skb_reserve(skb, datalen);
This time, I can check it without rebooting the system. The performance
gain is 4%-5%(stable). Does 4% worth submitting it to the kernel?
Thanks.
--Junchang
^ permalink raw reply related
* Re: [PATCH 3/3] ifb: move tq from ifb_private
From: jamal @ 2010-12-04 14:18 UTC (permalink / raw)
To: Changli Gao; +Cc: netdev
In-Reply-To: <1291442121-3302-3-git-send-email-xiaosuo@gmail.com>
On Sat, 2010-12-04 at 13:55 +0800, Changli Gao wrote:
> tq is only used in ri_tasklet, so we move it from ifb_private to a
> stack variable of ri_tasklet.
>
> skb_queue_splice_tail_init() is used instead of the open coded and slow
> one.
I like the splice idea but this patch makes me twitch
a little. What test setup did you use to check it?
cheers,
jamal
^ permalink raw reply
* Re: [PATCH 3/3] ifb: move tq from ifb_private
From: jamal @ 2010-12-04 14:28 UTC (permalink / raw)
To: Jarek Poplawski; +Cc: Changli Gao, netdev
In-Reply-To: <4CFA3F01.20109@gmail.com>
On Sat, 2010-12-04 at 14:15 +0100, Jarek Poplawski wrote:
> > @@ -87,7 +83,7 @@ static void ri_tasklet(unsigned long dev)
> > rcu_read_unlock();
> > dev_kfree_skb(skb);
> > stats->tx_dropped++;
> > - break;
> > + continue;
>
> IMHO this line is a bugfix and should be a separate patch (for stable).
Should be separate, yes.
Bug? no.
Improvement, maybe ;->
The idea was to defer processing at the first error. Changli
is changing it to continue despite the error.
The initial goal was to yield whenever possible since we dont maintain
a lot of state.
cheers,
jamal
^ permalink raw reply
* Re: [PATCH 3/3] ifb: move tq from ifb_private
From: jamal @ 2010-12-04 14:42 UTC (permalink / raw)
To: Changli Gao; +Cc: netdev
In-Reply-To: <1291472282.2159.8.camel@mojatatu>
On Sat, 2010-12-04 at 09:18 -0500, jamal wrote:
> I like the splice idea but this patch makes me twitch
> a little. What test setup did you use to check it?
Ok, here's one thing you changed which is important. We do:
-->XXX-->rq-->tq-->XXX-->
rq is controlled by queue limit.
We only load rq to tq if all of tq is empty. If it is not
we dont move things over. Essentially this is a flow
control scheme. We dont want many sources to be overwhelming
us with packets and every time we grab a txqlen number of packets.
For this reason:
I would be comfortable if all you did was to add the splice
after you skb_peek() - i think that would be a good improvement
which is not bound to break anything else.
cheers,
jamal
^ permalink raw reply
* Re: [PATCH 3/3] ifb: move tq from ifb_private
From: Changli Gao @ 2010-12-04 14:20 UTC (permalink / raw)
To: hadi; +Cc: netdev
In-Reply-To: <1291472282.2159.8.camel@mojatatu>
On Sat, Dec 4, 2010 at 10:18 PM, jamal <hadi@cyberus.ca> wrote:
>
> I like the splice idea but this patch makes me twitch
> a little. What test setup did you use to check it?
>
Just a simple test:
tc qdisc add dev eth0 ingress
tc filter add dev eth0 parent ffff: protocol ip basic action mirred
egress redirect dev ifb0
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [PATCH 3/3] ifb: move tq from ifb_private
From: Changli Gao @ 2010-12-04 14:45 UTC (permalink / raw)
To: hadi; +Cc: Jarek Poplawski, netdev
In-Reply-To: <1291472889.2159.18.camel@mojatatu>
On Sat, Dec 4, 2010 at 10:28 PM, jamal <hadi@cyberus.ca> wrote:
> On Sat, 2010-12-04 at 14:15 +0100, Jarek Poplawski wrote:
>
>> > @@ -87,7 +83,7 @@ static void ri_tasklet(unsigned long dev)
>> > rcu_read_unlock();
>> > dev_kfree_skb(skb);
>> > stats->tx_dropped++;
>> > - break;
>> > + continue;
>>
>> IMHO this line is a bugfix and should be a separate patch (for stable).
>
> Should be separate, yes.
> Bug? no.
If we breaks the loop when there are still skbs in tq and no skb in
rq, the skbs will be left in txq until new skbs are enqueued into rq.
In rare cases, no new skb is queued, then these skbs will stay in rq
forever.
- if (__netif_tx_trylock(txq)) {
- if ((skb = skb_peek(&dp->rq)) == NULL) {
- dp->tasklet_pending = 0;
- if (netif_queue_stopped(_dev))
- netif_wake_queue(_dev);
- } else {
- __netif_tx_unlock(txq);
- goto resched;
- }
- __netif_tx_unlock(txq);
- } else {
-resched:
- dp->tasklet_pending = 1;
- tasklet_schedule(&dp->ifb_tasklet);
- }
-
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: Question about __alloc_skb() speedup
From: Eric Dumazet @ 2010-12-04 14:47 UTC (permalink / raw)
To: Junchang Wang; +Cc: netdev
In-Reply-To: <20101204141826.GA5830@Desktop-Junchang>
Le samedi 04 décembre 2010 à 22:18 +0800, Junchang Wang a écrit :
> I added the prefetchw() in pktgen as follows:
>
> diff --git a/net/core/pktgen.c b/net/core/pktgen.c
> index 2953b2a..512f1ae 100644
> --- a/net/core/pktgen.c
> +++ b/net/core/pktgen.c
> @@ -2660,6 +2660,7 @@ static struct sk_buff *fill_packet_ipv4(struct net_device *odev,
> sprintf(pkt_dev->result, "No memory");
> return NULL;
> }
> + prefetchw(skb->data);
>
> skb_reserve(skb, datalen);
>
> This time, I can check it without rebooting the system. The performance
> gain is 4%-5%(stable). Does 4% worth submitting it to the kernel?
Yes I believe so, pktgen being very specific, but I have few questions :
Is it with SLUB or SLAB ?
How many buffers in TX ring on you nic (ethtool -g eth0) ?
What is the datalen value here ? (you prefetch, then advance skb->data)
32 or 64bit kernel ?
How many pps do you get before and after patch ?
Thanks
^ permalink raw reply
* Re: [PATCH 3/3] ifb: move tq from ifb_private
From: jamal @ 2010-12-04 14:48 UTC (permalink / raw)
To: Jarek Poplawski; +Cc: Changli Gao, netdev
In-Reply-To: <1291472889.2159.18.camel@mojatatu>
On Sat, 2010-12-04 at 09:28 -0500, jamal wrote:
> The idea was to defer processing at the first error. Changli
> is changing it to continue despite the error.
> The initial goal was to yield whenever possible since we dont maintain
> a lot of state.
Changli:
Other points we could defer processing is in case of packets
being dropped by dev_queue_xmit and netif_rx
cheers,
jamal
^ permalink raw reply
* Re: Question about __alloc_skb() speedup
From: Eric Dumazet @ 2010-12-04 14:49 UTC (permalink / raw)
To: Junchang Wang; +Cc: netdev
In-Reply-To: <1291474058.2806.96.camel@edumazet-laptop>
Le samedi 04 décembre 2010 à 15:47 +0100, Eric Dumazet a écrit :
> Le samedi 04 décembre 2010 à 22:18 +0800, Junchang Wang a écrit :
>
> > I added the prefetchw() in pktgen as follows:
> >
> > diff --git a/net/core/pktgen.c b/net/core/pktgen.c
> > index 2953b2a..512f1ae 100644
> > --- a/net/core/pktgen.c
> > +++ b/net/core/pktgen.c
> > @@ -2660,6 +2660,7 @@ static struct sk_buff *fill_packet_ipv4(struct net_device *odev,
> > sprintf(pkt_dev->result, "No memory");
> > return NULL;
> > }
> > + prefetchw(skb->data);
> >
> > skb_reserve(skb, datalen);
> >
> > This time, I can check it without rebooting the system. The performance
> > gain is 4%-5%(stable). Does 4% worth submitting it to the kernel?
>
> Yes I believe so, pktgen being very specific, but I have few questions :
>
> Is it with SLUB or SLAB ?
>
> How many buffers in TX ring on you nic (ethtool -g eth0) ?
>
> What is the datalen value here ? (you prefetch, then advance skb->data)
>
> 32 or 64bit kernel ?
>
> How many pps do you get before and after patch ?
>
> Thanks
>
Also, dont forget to include the prefetchw() in fill_packet_ipv6() as
well when submitting your patch.
^ permalink raw reply
* Re: [PATCH 3/3] ifb: move tq from ifb_private
From: Changli Gao @ 2010-12-04 14:50 UTC (permalink / raw)
To: hadi; +Cc: netdev
In-Reply-To: <1291473756.2159.31.camel@mojatatu>
On Sat, Dec 4, 2010 at 10:42 PM, jamal <hadi@cyberus.ca> wrote:
> On Sat, 2010-12-04 at 09:18 -0500, jamal wrote:
>
>> I like the splice idea but this patch makes me twitch
>> a little. What test setup did you use to check it?
>
> Ok, here's one thing you changed which is important. We do:
>
> -->XXX-->rq-->tq-->XXX-->
>
> rq is controlled by queue limit.
> We only load rq to tq if all of tq is empty. If it is not
> we dont move things over. Essentially this is a flow
> control scheme. We dont want many sources to be overwhelming
> us with packets and every time we grab a txqlen number of packets.
> For this reason:
> I would be comfortable if all you did was to add the splice
> after you skb_peek() - i think that would be a good improvement
> which is not bound to break anything else.
>
Maybe you misread my patch. tq is a stack variable in ri_tasklet, and
initialized all the time. ri_tasklet() won't exits until tq is
empty().
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [PATCH 3/3] ifb: move tq from ifb_private
From: jamal @ 2010-12-04 14:55 UTC (permalink / raw)
To: Changli Gao; +Cc: Jarek Poplawski, netdev
In-Reply-To: <AANLkTimy_dK0uDCWqo3AZBKSvod+KnE6Om-CxrbFmPj+@mail.gmail.com>
On Sat, 2010-12-04 at 22:45 +0800, Changli Gao wrote:
>
> If we breaks the loop when there are still skbs in tq and no skb in
> rq, the skbs will be left in txq until new skbs are enqueued into rq.
> In rare cases, no new skb is queued, then these skbs will stay in rq
> forever.
So should we goto resched?
cheers,
jamal
^ permalink raw reply
* Re: [PATCH 3/3] ifb: move tq from ifb_private
From: jamal @ 2010-12-04 14:59 UTC (permalink / raw)
To: Changli Gao; +Cc: netdev
In-Reply-To: <AANLkTikeECFUf0xiug7G+nd5=Sw7p=4Msz6RQ2gdOBzs@mail.gmail.com>
On Sat, 2010-12-04 at 22:50 +0800, Changli Gao wrote:
> Maybe you misread my patch. tq is a stack variable in ri_tasklet, and
> initialized all the time. ri_tasklet() won't exits until tq is
> empty().
in your patch is a variable on the stack.
What i am saying is you should defer processing when there is an
error (note the two other spots i mentioned).
This means you may leave dp->tq non-empty and therefore it
needs to be saved somewhere as it is before your patch.
cheers,
jamal
^ permalink raw reply
* Re: [PATCH 3/3] ifb: move tq from ifb_private
From: Changli Gao @ 2010-12-04 15:01 UTC (permalink / raw)
To: hadi; +Cc: Jarek Poplawski, netdev
In-Reply-To: <1291474504.2159.39.camel@mojatatu>
On Sat, Dec 4, 2010 at 10:55 PM, jamal <hadi@cyberus.ca> wrote:
> On Sat, 2010-12-04 at 22:45 +0800, Changli Gao wrote:
>
>>
>> If we breaks the loop when there are still skbs in tq and no skb in
>> rq, the skbs will be left in txq until new skbs are enqueued into rq.
>> In rare cases, no new skb is queued, then these skbs will stay in rq
>> forever.
>
> So should we goto resched?
>
Only if we can't lock the txq or rq isn't empty, we goto resched. So
it is a bug.
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [PATCH 3/3] ifb: move tq from ifb_private
From: Changli Gao @ 2010-12-04 15:07 UTC (permalink / raw)
To: hadi; +Cc: netdev
In-Reply-To: <1291474793.2159.43.camel@mojatatu>
On Sat, Dec 4, 2010 at 10:59 PM, jamal <hadi@cyberus.ca> wrote:
> On Sat, 2010-12-04 at 22:50 +0800, Changli Gao wrote:
>
>> Maybe you misread my patch. tq is a stack variable in ri_tasklet, and
>> initialized all the time. ri_tasklet() won't exits until tq is
>> empty().
>
> in your patch is a variable on the stack.
> What i am saying is you should defer processing when there is an
> error (note the two other spots i mentioned).
> This means you may leave dp->tq non-empty and therefore it
> needs to be saved somewhere as it is before your patch.
>
I know what you concern now. Thanks. I'll keep the old behavior in v2.
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [PATCH 3/3] ifb: move tq from ifb_private
From: jamal @ 2010-12-04 15:09 UTC (permalink / raw)
To: Changli Gao; +Cc: Jarek Poplawski, netdev
In-Reply-To: <AANLkTi=VESSzNvq_VwkWVU5b13VwnGs_tJn-4mqB28LV@mail.gmail.com>
On Sat, 2010-12-04 at 23:01 +0800, Changli Gao wrote:
> On Sat, Dec 4, 2010 at 10:55 PM, jamal <hadi@cyberus.ca> wrote:
> > On Sat, 2010-12-04 at 22:45 +0800, Changli Gao wrote:
> >
> >>
> >> If we breaks the loop when there are still skbs in tq and no skb in
> >> rq, the skbs will be left in txq until new skbs are enqueued into rq.
> >> In rare cases, no new skb is queued, then these skbs will stay in rq
> >> forever.
> >
> > So should we goto resched?
> >
>
> Only if we can't lock the txq or rq isn't empty, we goto resched. So
> it is a bug.
And to be explicit: Yes, meant to say there is a bug if we break out
in the scenario you described above - the fix is to jump to resched.
Why do we need the lock?
cheers,
jamal
^ permalink raw reply
* Re: [PATCH 3/3] ifb: move tq from ifb_private
From: jamal @ 2010-12-04 15:11 UTC (permalink / raw)
To: Changli Gao; +Cc: netdev
In-Reply-To: <AANLkTi=MFQc4_mYkwiKPzT_NmRRxou18P60ZZo+pYsNp@mail.gmail.com>
On Sat, 2010-12-04 at 23:07 +0800, Changli Gao wrote:
>
> I know what you concern now. Thanks. I'll keep the old behavior in v2.
Ok - thanks Changli.
cheers,
jamal
^ permalink raw reply
* Re: [PATCH 3/3] ifb: move tq from ifb_private
From: Jarek Poplawski @ 2010-12-04 15:40 UTC (permalink / raw)
To: jamal; +Cc: Changli Gao, netdev
In-Reply-To: <1291474127.2159.35.camel@mojatatu>
On Sat, Dec 04, 2010 at 09:48:47AM -0500, jamal wrote:
> On Sat, 2010-12-04 at 09:28 -0500, jamal wrote:
>
> > The idea was to defer processing at the first error. Changli
> > is changing it to continue despite the error.
> > The initial goal was to yield whenever possible since we dont maintain
> > a lot of state.
>
> Changli:
> Other points we could defer processing is in case of packets
> being dropped by dev_queue_xmit and netif_rx
Hmm... But we didn't care until now... ;-) Btw. is it really very
probable (and worth bothering) that this current error of NULL dev
get fixed before we purge this tq queue with deferral one by one?
Cheers,
Jarek P.
^ permalink raw reply
* Re: [PATCH 3/3] ifb: move tq from ifb_private
From: jamal @ 2010-12-04 16:08 UTC (permalink / raw)
To: Jarek Poplawski; +Cc: Changli Gao, netdev
In-Reply-To: <20101204154007.GA1981@del.dom.local>
On Sat, 2010-12-04 at 16:40 +0100, Jarek Poplawski wrote:
> Hmm... But we didn't care until now... ;-)
Well, if Changli didnt post - there would be no discussion ;->
The message was lost in the translation somewhere; events such as
patches sometimes serve as good reminders.
> Btw. is it really very
> probable (and worth bothering) that this current error of NULL dev
> get fixed before we purge this tq queue with deferral one by one?
Indeed - this is working against something buggy. But it has happened
often in the past. And the likelihood of there being a few bad ones
in the train of packets when this occurs is high. But there are many
packets there that wont suffer this sympton - so the only fair scheme is
to check all. Note: a BUG() seems unreasonable and the deferring serves
as a throttling scheme.
What do you have in mind?
cheers,
jamal
^ permalink raw reply
* Re: [PATCH 3/3] ifb: move tq from ifb_private
From: Jarek Poplawski @ 2010-12-04 16:56 UTC (permalink / raw)
To: jamal; +Cc: Changli Gao, netdev
In-Reply-To: <1291478881.2159.70.camel@mojatatu>
On Sat, Dec 04, 2010 at 11:08:01AM -0500, jamal wrote:
> On Sat, 2010-12-04 at 16:40 +0100, Jarek Poplawski wrote:
>
> > Hmm... But we didn't care until now... ;-)
>
> Well, if Changli didnt post - there would be no discussion ;->
> The message was lost in the translation somewhere; events such as
> patches sometimes serve as good reminders.
>
> > Btw. is it really very
> > probable (and worth bothering) that this current error of NULL dev
> > get fixed before we purge this tq queue with deferral one by one?
>
> Indeed - this is working against something buggy. But it has happened
> often in the past. And the likelihood of there being a few bad ones
> in the train of packets when this occurs is high. But there are many
> packets there that wont suffer this sympton - so the only fair scheme is
> to check all. Note: a BUG() seems unreasonable and the deferring serves
> as a throttling scheme.
> What do you have in mind?
I'm simply not convinced this kind of (fast) throttling can properly
fix any of the problems (what about other flows in the queue), while
Changli's patch makes this tasklet simpler and a bit faster.
Cheers,
Jarek P.
^ permalink raw reply
* Re: [PATCH 1/2] vsprintf: Add %pMbt, bluetooth mac address
From: Joe Perches @ 2010-12-04 17:48 UTC (permalink / raw)
To: Michał Mirosław
Cc: Marcel Holtmann, Gustavo F. Padovan, linux-kernel, netdev
In-Reply-To: <AANLkTi=qh0GKp3BdKO9GX1qmM3iJ6J_F3fLqZ8qNAw5v@mail.gmail.com>
On Sat, 2010-12-04 at 12:03 +0100, Michał Mirosław wrote:
> 2010/12/4 Joe Perches <joe@perches.com>:
> > Bluetooth output the MAC address in reverse order.
> > Adding %pMbt allows the batostr function to be removed.
> Just a nitpick:
> You could call it %pMR, as in 'Reverse', so it sounds better when/if
> some other subsystem uses it. It would also be a hint of what is this
> doing instead of where it came from.
I considered that but believe %pMbt is clearer as most
likely no other subsystem will be quite so far (out to
lunch? in left field? north? :) enough to do that again.
If any maintainer wants it changed, it's not any sort
of problem to me, say so and I'll resubmit it.
^ permalink raw reply
* Re: kernel panic with time-stamping in phy devices (monitor mode)
From: Andrew Watts @ 2010-12-04 20:46 UTC (permalink / raw)
To: Richard Cochran, Eric Dumazet; +Cc: netdev, David Miller
In-Reply-To: <1291450622.2806.59.camel@edumazet-laptop>
--- On Sat, 12/4/10, Eric Dumazet wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Subject: Re: kernel panic with time-stamping in phy devices (monitor mode)
> To: "Richard Cochran" <richardcochran@gmail.com>
> Cc: "Andrew Watts" <akwatts@ymail.com>, netdev@vger.kernel.org, "David Miller" <davem@davemloft.net>
> Date: Saturday, December 4, 2010, 8:17 AM
> Le samedi 04 décembre 2010 à 08:57
> +0100, Richard Cochran a écrit :
> > Date: Sat, 4 Dec 2010 08:55:04 +0100
> > From: Richard Cochran <richardcochran@gmail.com>
> > To: Eric Dumazet <eric.dumazet@gmail.com>
> > Cc: Andrew Watts <akwatts@ymail.com>,
> netdev@vger.kernel.org,
> > David Miller <davem@davemloft.net>
> > Subject: Re: kernel panic with time-stamping in phy
> devices (monitor mode)
> > Message-ID: <20101204075503.GA3490@riccoc20.at.omicron.at>
> > References: <252997.92320.qm@web111013.mail.gq1.yahoo.com>
> >
> <1291307884.2871.69.camel@edumazet-laptop>
> > MIME-Version: 1.0
> > Content-Type: text/plain; charset=us-ascii
> > Content-Disposition: inline
> > In-Reply-To:
> <1291307884.2871.69.camel@edumazet-laptop>
> > User-Agent: Mutt/1.5.20 (2009-06-14)
> >
> > Ugh, new kernel code with no users is already causing
> trouble!
> >
> > On Thu, Dec 02, 2010 at 05:38:04PM +0100, Eric Dumazet
> wrote:
> > > Thanks for the report
> > >
> > > Please try following patch.
> >
> > And thank you, Eric, for the quick patch.
> >
> > Can this fix go into 2.6.37, please?
>
> Sure, I'll submit to David today, thanks !
>
Eric, I echo the thanks on the lightning patch.
Impressive turnaround!
There's an open bug report on the kernel's bugzilla
for 2.6.36 (#24102). What is the best way to tie these
together?
~ Andy
^ permalink raw reply
* [PATCH] tcp: Replace time wait bucket msg by counter.
From: Tom Herbert @ 2010-12-04 21:35 UTC (permalink / raw)
To: davem, netdev
Rather than printing the message to the log, use a mib counter to keep
track of the count of occurences of time wait bucket overflow. Reduces
spam in logs.
Signed-off-by: Tom Herbert <therbert@google.com>
---
include/linux/snmp.h | 1 +
net/ipv4/proc.c | 1 +
net/ipv4/tcp_minisocks.c | 2 +-
3 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/include/linux/snmp.h b/include/linux/snmp.h
index ebb0c80..12b2b18 100644
--- a/include/linux/snmp.h
+++ b/include/linux/snmp.h
@@ -230,6 +230,7 @@ enum
LINUX_MIB_TCPMINTTLDROP, /* RFC 5082 */
LINUX_MIB_TCPDEFERACCEPTDROP,
LINUX_MIB_IPRPFILTER, /* IP Reverse Path Filter (rp_filter) */
+ LINUX_MIB_TCPTIMEWAITOVERFLOW, /* TCPTimeWaitOverflow */
__LINUX_MIB_MAX
};
diff --git a/net/ipv4/proc.c b/net/ipv4/proc.c
index 1b48eb1..b14ec7d 100644
--- a/net/ipv4/proc.c
+++ b/net/ipv4/proc.c
@@ -253,6 +253,7 @@ static const struct snmp_mib snmp4_net_list[] = {
SNMP_MIB_ITEM("TCPMinTTLDrop", LINUX_MIB_TCPMINTTLDROP),
SNMP_MIB_ITEM("TCPDeferAcceptDrop", LINUX_MIB_TCPDEFERACCEPTDROP),
SNMP_MIB_ITEM("IPReversePathFilter", LINUX_MIB_IPRPFILTER),
+ SNMP_MIB_ITEM("TCPTimeWaitOverflow", LINUX_MIB_TCPTIMEWAITOVERFLOW),
SNMP_MIB_SENTINEL
};
diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index 43cf901..3052a2b 100644
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -347,7 +347,7 @@ void tcp_time_wait(struct sock *sk, int state, int timeo)
* socket up. We've got bigger problems than
* non-graceful socket closings.
*/
- LIMIT_NETDEBUG(KERN_INFO "TCP: time wait bucket table overflow\n");
+ NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPTIMEWAITOVERFLOW);
}
tcp_update_metrics(sk);
--
1.7.3.1
^ 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