* RE: [PATCH 11/12] sfc: Only count bad packets in rx_errors
From: Dmitry Kravkov @ 2010-06-08 9:20 UTC (permalink / raw)
To: netdev@vger.kernel.org; +Cc: Ben Hutchings
In-Reply-To: <1275427265.2114.35.camel@achroite.uk.solarflarecom.com>
Hi,
Different drivers include different errors in stats->rx_errors:
bxn2 bnx2x ixgbx sfc dnet atlx
----------------------------------------------------------------------
rx_length rx_length rx_length rx_length rx_length rx_length
rx_over rx_over rx_over
rx_frame rx_frame rx_frame rx_frame rx_frame
rx_crc rx_crc rx_crc rx_crc rx_crc
rx_fifo rx_fifo
rx_missed rx_missed
rx_symbol
unsup_opcd
----------------------------------------------------------------------
Is there any documentation which defines what should this statistic include?
Thanks
Dmitry
-----Original Message-----
From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On Behalf Of Ben Hutchings
Sent: Wednesday, June 02, 2010 12:21 AM
To: David Miller
Cc: netdev@vger.kernel.org; linux-net-drivers@solarflare.com
Subject: [PATCH 11/12] sfc: Only count bad packets in rx_errors
rx_errors is defined as 'bad packets received', but we are currently
including various overflow errors as well.
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/sfc/efx.c | 3 ---
1 files changed, 0 insertions(+), 3 deletions(-)
diff --git a/drivers/net/sfc/efx.c b/drivers/net/sfc/efx.c
index aae3347..26b0cc2 100644
--- a/drivers/net/sfc/efx.c
+++ b/drivers/net/sfc/efx.c
@@ -1518,11 +1518,8 @@ static struct net_device_stats *efx_net_stats(struct net_device *net_dev)
stats->tx_window_errors = mac_stats->tx_late_collision;
stats->rx_errors = (stats->rx_length_errors +
- stats->rx_over_errors +
stats->rx_crc_errors +
stats->rx_frame_errors +
- stats->rx_fifo_errors +
- stats->rx_missed_errors +
mac_stats->rx_symbol_error);
stats->tx_errors = (stats->tx_window_errors +
mac_stats->tx_bad);
--
1.6.2.5
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
--
To unsubscribe from this list: send the line "unsubscribe netdev" 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 related
* Re: [PATCH] netconsole: queue console messages to send later
From: Cong Wang @ 2010-06-08 8:59 UTC (permalink / raw)
To: Flavio Leitner
Cc: David Miller, netdev, fubar, mpm, gospo, nhorman, jmoyer,
shemminger, linux-kernel, bridge, bonding-devel
In-Reply-To: <20100608003707.GA30604@sysclose.org>
Thanks for your fix, Flavio!
On 06/08/10 08:37, Flavio Leitner wrote:
>> There may not be another timer or workqueue able to execute after the
>> printk() we're trying to emit. We may never get to that point.
>
> What if in the netpoll, before we push the skb to the driver, we check
> for a bit saying that it's already pushing another skb. In this case,
> queue the new skb inside of netpoll and soon as the first call returns
> and try to clear the bit, it will send the next skb?
>
> printk("message 1")
> ...
> netconsole called
> netpoll sets the flag bit
> pushes to the bonding driver which does another printk("message 2")
> netconsole called again
> netpoll checks for the flag, queue the message, returns.
> so, bonding can finish up to send the first message
> netpoll is about to return, checks for new queued messages, and pushes them.
> bonding finishes up to send the second message
> ....
>
> No deadlocks, skbs are ordered and still under the same opportunity
> to send something. Does it sound acceptable?
> It's off the top of my head, so probably this idea has some problems.
>
I am not a net expert, I am not sure if this solution really addresses
David's concern, but it makes sense for me.
>
>> Fix the locking in the drivers or layers that cause the issue instead
>> of breaking netconsole.
>
> Someday, somewhere, I know because I did this before, someone will
> use a debugging printk() and will see the entire box hanging with
> absolutely no message in any console because of this problem.
> I'm not saying that fixing driver isn't the right way to go but
> it seems not enough to me.
Well, I think netconsole is not alone, other console drivers could
have the same problem, printk() is not always available in some
situation like this.
Thanks.
^ permalink raw reply
* Re: 2.6.35-rc2-git1 - include/linux/cgroup.h:534 invoked rcu_dereference_check() without protection!
From: Peter Zijlstra @ 2010-06-08 8:40 UTC (permalink / raw)
To: Miles Lane
Cc: paulmck, Vivek Goyal, Eric Paris, Lai Jiangshan, Ingo Molnar,
LKML, nauman, eric.dumazet, netdev, Jens Axboe, Gui Jianfeng,
Li Zefan, Johannes Berg
In-Reply-To: <AANLkTikl13cJNuU32JzRarrUnoh5DYHr-Tbnv9B-UjsG@mail.gmail.com>
On Tue, 2010-06-08 at 00:16 -0400, Miles Lane wrote:
> On Mon, Jun 7, 2010 at 8:19 PM, Paul E. McKenney
> <paulmck@linux.vnet.ibm.com> wrote:
> > On Mon, Jun 07, 2010 at 02:14:25PM -0400, Miles Lane wrote:
> >> Hi All,
> >>
> >> I just reproduced a warning I reported quite a while ago. Is a patch
> >> for this in the pipeline?
> >
> > I proposed a patch, thinking that it was a false positive. Peter Zijlstra
> > pointed out that there was a real race, and proposed an alternative patch,
> > which may be found at http://lkml.org/lkml/2010/4/22/603.
> >
> > Could you please test Peter's patch and let us know if it cures the problem?
> >
Gah, this task_group() stuff is annoying, how about something like the
below which teaches task_group() about the task_rq()->lock rule?
---
include/linux/cgroup.h | 20 +++++++++++----
kernel/sched.c | 61 +++++++++++++++++++++++++----------------------
2 files changed, 46 insertions(+), 35 deletions(-)
diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index 0c62160..1efd212 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -525,13 +525,21 @@ static inline struct cgroup_subsys_state *cgroup_subsys_state(
return cgrp->subsys[subsys_id];
}
-static inline struct cgroup_subsys_state *task_subsys_state(
- struct task_struct *task, int subsys_id)
+/*
+ * function to get the cgroup_subsys_state which allows for extra
+ * rcu_dereference_check() conditions, such as locks used during the
+ * cgroup_subsys::attach() methods.
+ */
+#define task_subsys_state_check(task, subsys_id, __c) \
+ rcu_dereference_check(task->cgroups->subsys[subsys_id], \
+ rcu_read_lock_held() || \
+ lockdep_is_held(&task->alloc_lock) || \
+ cgroup_lock_is_held() || (__c))
+
+static inline struct cgroup_subsys_state *
+task_subsys_state(struct task_struct *task, int subsys_id)
{
- return rcu_dereference_check(task->cgroups->subsys[subsys_id],
- rcu_read_lock_held() ||
- lockdep_is_held(&task->alloc_lock) ||
- cgroup_lock_is_held());
+ return task_subsys_state_check(task, subsys_id, false);
}
static inline struct cgroup* task_cgroup(struct task_struct *task,
diff --git a/kernel/sched.c b/kernel/sched.c
index f8b8996..e01bb45 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -306,32 +306,26 @@ static int init_task_group_load = INIT_TASK_GROUP_LOAD;
*/
struct task_group init_task_group;
-/* return group to which a task belongs */
+/*
+ * Return the group to which this tasks belongs.
+ *
+ * We use task_subsys_state_check() and extend the RCU verification
+ * with lockdep_is_held(&task_rq(p)->lock) because cpu_cgroup_attach()
+ * holds that lock for each task it moves into the cgroup. Therefore
+ * by holding that lock, we pin the task to the current cgroup.
+ */
static inline struct task_group *task_group(struct task_struct *p)
{
- struct task_group *tg;
+ struct cgroup_subsys_state *css;
-#ifdef CONFIG_CGROUP_SCHED
- tg = container_of(task_subsys_state(p, cpu_cgroup_subsys_id),
- struct task_group, css);
-#else
- tg = &init_task_group;
-#endif
- return tg;
+ css = task_subsys_state_check(p, cpu_cgroup_subsys_id,
+ lockdep_is_held(&task_rq(p)->lock));
+ return container_of(css, struct task_group, css);
}
/* Change a task's cfs_rq and parent entity if it moves across CPUs/groups */
static inline void set_task_rq(struct task_struct *p, unsigned int cpu)
{
- /*
- * Strictly speaking this rcu_read_lock() is not needed since the
- * task_group is tied to the cgroup, which in turn can never go away
- * as long as there are tasks attached to it.
- *
- * However since task_group() uses task_subsys_state() which is an
- * rcu_dereference() user, this quiets CONFIG_PROVE_RCU.
- */
- rcu_read_lock();
#ifdef CONFIG_FAIR_GROUP_SCHED
p->se.cfs_rq = task_group(p)->cfs_rq[cpu];
p->se.parent = task_group(p)->se[cpu];
@@ -341,7 +335,6 @@ static inline void set_task_rq(struct task_struct *p, unsigned int cpu)
p->rt.rt_rq = task_group(p)->rt_rq[cpu];
p->rt.parent = task_group(p)->rt_se[cpu];
#endif
- rcu_read_unlock();
}
#else
@@ -4465,16 +4458,6 @@ recheck:
}
if (user) {
-#ifdef CONFIG_RT_GROUP_SCHED
- /*
- * Do not allow realtime tasks into groups that have no runtime
- * assigned.
- */
- if (rt_bandwidth_enabled() && rt_policy(policy) &&
- task_group(p)->rt_bandwidth.rt_runtime == 0)
- return -EPERM;
-#endif
-
retval = security_task_setscheduler(p, policy, param);
if (retval)
return retval;
@@ -4490,6 +4473,26 @@ recheck:
* runqueue lock must be held.
*/
rq = __task_rq_lock(p);
+
+ retval = 0;
+#ifdef CONFIG_RT_GROUP_SCHED
+ if (user) {
+ /*
+ * Do not allow realtime tasks into groups that have no runtime
+ * assigned.
+ */
+ if (rt_bandwidth_enabled() && rt_policy(policy) &&
+ task_group(p)->rt_bandwidth.rt_runtime == 0)
+ retval = -EPERM;
+
+ if (retval) {
+ __task_rq_unlock(rq);
+ raw_spin_unlock_irqrestore(&p->pi_lock, flags);
+ return retval;
+ }
+ }
+#endif
+
/* recheck policy now with rq lock held */
if (unlikely(oldpolicy != -1 && oldpolicy != p->policy)) {
policy = oldpolicy = -1;
^ permalink raw reply related
* Re: [v5 Patch 1/3] netpoll: add generic support for bridge and bonding devices
From: Cong Wang @ 2010-06-08 8:38 UTC (permalink / raw)
To: Andy Gospodarek
Cc: Jay Vosburgh, Flavio Leitner, linux-kernel, Matt Mackall, netdev,
bridge, Andy Gospodarek, Neil Horman, Jeff Moyer,
Stephen Hemminger, bonding-devel, David Miller
In-Reply-To: <20100607130357.GN7497@gospo.rdu.redhat.com>
On 06/07/10 21:03, Andy Gospodarek wrote:
> On Mon, Jun 07, 2010 at 05:57:49PM +0800, Cong Wang wrote:
>> On 06/05/10 03:18, Andy Gospodarek wrote:
>>> On Wed, Jun 02, 2010 at 06:04:45PM +0800, Cong Wang wrote:
>>>> On 06/02/10 02:42, Jay Vosburgh wrote:
>>>>> Cong Wang<amwang@redhat.com> wrote:
>>>>>
>>>>>> On 06/01/10 03:08, Flavio Leitner wrote:
>>>>>>> On Mon, May 31, 2010 at 01:56:52PM +0800, Cong Wang wrote:
>>>>>>>> Hi, Flavio,
>>>>>>>>
>>>>>>>> Please use the attached patch instead, try to see if it solves
>>>>>>>> all your problems.
>>>>>>>
>>>>>>> I tried and it hangs. No backtraces this time.
>>>>>>> The bond_change_active_slave() prints before NETDEV_BONDING_FAILOVER
>>>>>>> notification, so I think it won't work.
>>>>>>
>>>>>> Ah, I thought the same.
>>>>>>
>>>>>>>
>>>>>>> Please, correct if I'm wrong, but when a failover happens with your
>>>>>>> patch applied, the netconsole would be disabled forever even with
>>>>>>> another healthy slave, right?
>>>>>>>
>>>>>>
>>>>>> Yes, this is an easy solution, because bonding has several modes,
>>>>>> it is complex to make netpoll work in different modes.
>>>>>
>>>>> If I understand correctly, the root cause of the problem with
>>>>> netconsole and bonding is that bonding is, ultimately, performing
>>>>> printks with a write lock held, and when netconsole recursively calls
>>>>> into bonding to send the printk over the netconsole, there is a deadlock
>>>>> (when the bonding xmit function attempts to acquire the same lock for
>>>>> read).
>>>>
>>>>
>>>> Yes.
>>>>
>>>>>
>>>>> You're trying to avoid the deadlock by shutting off netconsole
>>>>> (permanently, it looks like) for one problem case: a failover, which
>>>>> does some printks with a write lock held.
>>>>>
>>>>> This doesn't look to me like a complete solution, there are
>>>>> other cases in bonding that will do printk with write locks held. I
>>>>> suspect those will also hang netconsole as things exist today, and won't
>>>>> be affected by your patch below.
>>>>
>>>>
>>>> I can expect that, bonding modes are complex.
>>>>
>>>>>
>>>>> For example:
>>>>>
>>>>> The sysfs functions to set the primary (bonding_store_primary)
>>>>> or active (bonding_store_active_slave) options: a pr_info is called to
>>>>> provide a log message of the results. These could be tested by setting
>>>>> the primary or active options via sysfs, e.g.,
>>>>>
>>>>> echo eth0> /sys/class/net/bond0/bonding/primary
>>>>> echo eth0> /sys/class/net/bond0/bonding/active
>>>>>
>>>>> If the kernel is defined with DEBUG, there are a few pr_debug
>>>>> calls within write_locks (bond_del_vlan, for example).
>>>>>
>>>>> If the slave's underlying device driver's ndo_vlan_rx_register
>>>>> or ndo_vlan_rx_kill_vid functions call printk (and it looks like some do
>>>>> for error cases, e.g., igbvf, ehea, enic), those would also presumably
>>>>> deadlock (because bonding holds its write_lock when calling the ndo_
>>>>> vlan functions).
>>>>>
>>>>> It also appears that (with the patch below) some nominally
>>>>> normal usage patterns will immediately disable netconsole. The one that
>>>>> comes to mind is if the primary= option is set (to "eth1" for this
>>>>> example), but that slave not enslaved first (the slaves are added, say,
>>>>> eth0 then eth1). In that situation, when the primary slave (eth1 here)
>>>>> is added, the first thing that will happen is a failover, and that will
>>>>> disable netconsole.
>>>>>
>>>>
>>>> Thanks for your detailed explanation!
>>>>
>>>> This is why I said bonding is complex. I guess we would have to adjust
>>>> netpoll code for different bonding cases, one solution seems not fix all.
>>>> I am not sure how much work to do, since I am not familiar with bonding
>>>> code. Maybe Andy can help?
>>>>
>>>
>>> Sorry I've been silent until now. This does seem quite similar to a
>>> problem I've previously encountered when dealing with bonding+netpoll on
>>> some old 2.6.9-based kernels. There is no guarantee the methods used
>>> there will apply here, but I'll talk about them anyway.
>>>
>>> As Flavio noticed, recursive calls into the bond transmit routines were
>>> not a good idea. I discovered the same and worked around this issue by
>>> checking to see if we could take the bond->lock for writing before
>>> continuing. If we could not get, I wanted to signal that this should be
>>> queued for transmission later. Based on the flow of netpoll_send_skb
>>> (or possibly for another reason that is escaping me right now) I added
>>> one of these checks in bond_poll_controller too. These aren't the
>>> prettiest fixes, but seemed to work well for me when I did this work in
>>> the past. I realize the differences are not that great compared to some
>>> of the patches posted by Flavio, but I think they are worth trying.
>>
>>
>> Hmm, I still feel like this way is ugly, although it may work.
>> I guess David doesn't like it either.
>>
>
> Notice how I referred to it as a work-around? :)
>
> It certainly isn't a great way to resolve the issue, but I wanted to
> offer my opinon on the issue since you asked.
Sorry for my misunderstanding.
^ permalink raw reply
* [PATCH net-next-2.6] icmp: RCU conversion in icmp_address_reply()
From: Eric Dumazet @ 2010-06-08 8:34 UTC (permalink / raw)
To: David Miller; +Cc: netdev
- rcu_read_lock() already held by caller
- use __in_dev_get_rcu() instead of in_dev_get() / in_dev_put()
- remove goto out;
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
net/ipv4/icmp.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index d65e921..bdb6c71 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -925,6 +925,7 @@ static void icmp_address(struct sk_buff *skb)
/*
* RFC1812 (4.3.3.9). A router SHOULD listen all replies, and complain
* loudly if an inconsistency is found.
+ * called with rcu_read_lock()
*/
static void icmp_address_reply(struct sk_buff *skb)
@@ -935,12 +936,12 @@ static void icmp_address_reply(struct sk_buff *skb)
struct in_ifaddr *ifa;
if (skb->len < 4 || !(rt->rt_flags&RTCF_DIRECTSRC))
- goto out;
+ return;
- in_dev = in_dev_get(dev);
+ in_dev = __in_dev_get_rcu(dev);
if (!in_dev)
- goto out;
- rcu_read_lock();
+ return;
+
if (in_dev->ifa_list &&
IN_DEV_LOG_MARTIANS(in_dev) &&
IN_DEV_FORWARD(in_dev)) {
@@ -958,9 +959,6 @@ static void icmp_address_reply(struct sk_buff *skb)
mp, dev->name, &rt->rt_src);
}
}
- rcu_read_unlock();
- in_dev_put(in_dev);
-out:;
}
static void icmp_discard(struct sk_buff *skb)
^ permalink raw reply related
* Re: [v5 Patch 1/3] netpoll: add generic support for bridge and bonding devices
From: Cong Wang @ 2010-06-08 8:36 UTC (permalink / raw)
To: David Miller
Cc: andy, fubar, fbl, linux-kernel, mpm, netdev, bridge, gospo,
nhorman, jmoyer, shemminger, bonding-devel
In-Reply-To: <20100607.030108.235696592.davem@davemloft.net>
On 06/07/10 18:01, David Miller wrote:
> From: Cong Wang<amwang@redhat.com>
> Date: Mon, 07 Jun 2010 17:57:49 +0800
>
>> Hmm, I still feel like this way is ugly, although it may work.
>> I guess David doesn't like it either.
>
> Of course I don't like it. :-)
>
> I suspect the locking scheme will need to be changed.
>
> Besides, if we're going to hack this up and do write lock attempts in
> the read locking paths, there is no point in using a rwlock any more.
> And I'm personally in disfavor of all rwlock usage anyways (it dirties
> the cacheline for readers just as equally for writers, and if the
> critically protected code path is short enough, that shared cache
> line atomic operation will be the predominant cost).
>
> So I'd say, 1) make this a spinlock and 2) try to use RCU for the
> read path.
>
> That would fix everything.
Yeah, agreed. Even not talking about netconsole, bonding code
does have locking problems, netconsole just makes this problem
clear.
I will try your suggestions above.
Thanks!
^ permalink raw reply
* [PATCH net-2.6] ipv6: fix ICMP6_MIB_OUTERRORS
From: Eric Dumazet @ 2010-06-08 8:24 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In commit 1f8438a85366 (icmp: Account for ICMP out errors), I did a typo
on IPV6 side, using ICMP6_MIB_OUTMSGS instead of ICMP6_MIB_OUTERRORS
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index ce79929..03e62f9 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -483,7 +483,7 @@ route_done:
np->tclass, NULL, &fl, (struct rt6_info*)dst,
MSG_DONTWAIT, np->dontfrag);
if (err) {
- ICMP6_INC_STATS_BH(net, idev, ICMP6_MIB_OUTMSGS);
+ ICMP6_INC_STATS_BH(net, idev, ICMP6_MIB_OUTERRORS);
ip6_flush_pending_frames(sk);
goto out_put;
}
@@ -565,7 +565,7 @@ static void icmpv6_echo_reply(struct sk_buff *skb)
np->dontfrag);
if (err) {
- ICMP6_INC_STATS_BH(net, idev, ICMP6_MIB_OUTMSGS);
+ ICMP6_INC_STATS_BH(net, idev, ICMP6_MIB_OUTERRORS);
ip6_flush_pending_frames(sk);
goto out_put;
}
^ permalink raw reply related
* Re: 2.6.35-rc2-git1 - net/mac80211/sta_info.c:125 invoked rcu_dereference_check() without protection!
From: Johannes Berg @ 2010-06-08 7:26 UTC (permalink / raw)
To: paulmck
Cc: Miles Lane, Vivek Goyal, Eric Paris, Lai Jiangshan, Ingo Molnar,
Peter Zijlstra, LKML, nauman, eric.dumazet, netdev, Jens Axboe,
Gui Jianfeng, Li Zefan
In-Reply-To: <20100607235941.GD2387@linux.vnet.ibm.com>
On Mon, 2010-06-07 at 16:59 -0700, Paul E. McKenney wrote:
> On Mon, Jun 07, 2010 at 02:25:44PM -0400, Miles Lane wrote:
> > [ 43.478812] [ INFO: suspicious rcu_dereference_check() usage. ]
> > [ 43.478815] ---------------------------------------------------
> > [ 43.478820] net/mac80211/sta_info.c:125 invoked
> > rcu_dereference_check() without protection!
> > [ 43.478824]
> > [ 43.478824] other info that might help us debug this:
> > [ 43.478826]
> > [ 43.478829]
> > [ 43.478830] rcu_scheduler_active = 1, debug_locks = 1
> > [ 43.478834] no locks held by NetworkManager/4017.
>
> Hmmm... Johannes's update has been merged, and it requires that callers
> either be in an RCU read-side critical section or hold either the
> ->sta_lock or the ->sta_mtx, and this thread does none of this.
>
> Johannes, any thoughts?
>
> Thanx, Paul
>
> > [ 43.478837] stack backtrace:
> > [ 43.478842] Pid: 4017, comm: NetworkManager Not tainted 2.6.35-rc2-git1 #8
> > [ 43.478846] Call Trace:
> > [ 43.478849] <IRQ> [<ffffffff81064e9c>] lockdep_rcu_dereference+0x9d/0xa5
> > [ 43.478876] [<ffffffffa010cb3c>] sta_info_get_bss+0x71/0x12d [mac80211]
> > [ 43.478889] [<ffffffffa010cc0d>] ieee80211_find_sta+0x15/0x2f [mac80211]
> > [ 43.478902] [<ffffffffa019ae16>] iwlagn_tx_queue_reclaim+0xe7/0x1bb [iwlagn]
iwlwifi wasn't using rcu protection here -- already sent a patch fixing
it. My mistake, I think. Thanks for checking :)
johannes
^ permalink raw reply
* [PATCH v2] net8139: fix a race at the end of NAPI
From: Figo.zhang @ 2010-06-08 7:13 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev
In-Reply-To: <1275979719.1927.4.camel@myhost>
fix a race at the end of NAPI complete processing, it had
better do __napi_complete() first before re-enable interrupt.
in v2, i motify it using vim.
Signed-off-by:Figo.zhang <figo1802@gmail.com>
---
drivers/net/8139cp.c | 2 +-
drivers/net/8139too.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/8139cp.c b/drivers/net/8139cp.c
index 9c14975..284a5f4 100644
--- a/drivers/net/8139cp.c
+++ b/drivers/net/8139cp.c
@@ -598,8 +598,8 @@ rx_next:
goto rx_status_loop;
spin_lock_irqsave(&cp->lock, flags);
- cpw16_f(IntrMask, cp_intr_mask);
__napi_complete(napi);
+ cpw16_f(IntrMask, cp_intr_mask);
spin_unlock_irqrestore(&cp->lock, flags);
}
diff --git a/drivers/net/8139too.c b/drivers/net/8139too.c
index 4ba7293..a7bca8c 100644
--- a/drivers/net/8139too.c
+++ b/drivers/net/8139too.c
@@ -2088,8 +2088,8 @@ static int rtl8139_poll(struct napi_struct *napi, int budget)
* again when we think we are done.
*/
spin_lock_irqsave(&tp->lock, flags);
- RTL_W16_F(IntrMask, rtl8139_intr_mask);
__napi_complete(napi);
+ RTL_W16_F(IntrMask, rtl8139_intr_mask);
spin_unlock_irqrestore(&tp->lock, flags);
}
spin_unlock(&tp->rx_lock);
^ permalink raw reply related
* Re: [PATCH] fix a race at the end of NAPI
From: Eric Dumazet @ 2010-06-08 7:07 UTC (permalink / raw)
To: Figo.zhang; +Cc: David S. Miller, netdev
In-Reply-To: <1275979719.1927.4.camel@myhost>
Le mardi 08 juin 2010 à 14:48 +0800, Figo.zhang a écrit :
> fix a race at the end of NAPI complete processing. it had better do __napi_complete()
> first before re-enable interrupt.
>
> Signed-off-by: Figo.zhang <figo1802@gmail.com>
Patch title is misleading
> ---
> drivers/net/8139cp.c | 2 +-
> drivers/net/8139too.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/8139cp.c b/drivers/net/8139cp.c
> old mode 100644
> new mode 100755
Why do you change file modes ?
> index 9c14975..284a5f4
> --- a/drivers/net/8139cp.c
> +++ b/drivers/net/8139cp.c
> @@ -598,8 +598,8 @@ rx_next:
> goto rx_status_loop;
>
> spin_lock_irqsave(&cp->lock, flags);
> - cpw16_f(IntrMask, cp_intr_mask);
> __napi_complete(napi);
> + cpw16_f(IntrMask, cp_intr_mask);
> spin_unlock_irqrestore(&cp->lock, flags);
> }
>
> diff --git a/drivers/net/8139too.c b/drivers/net/8139too.c
> old mode 100644
> new mode 100755
> index 4ba7293..a7bca8c
> --- a/drivers/net/8139too.c
> +++ b/drivers/net/8139too.c
> @@ -2088,8 +2088,8 @@ static int rtl8139_poll(struct napi_struct *napi, int budget)
> * again when we think we are done.
> */
> spin_lock_irqsave(&tp->lock, flags);
> - RTL_W16_F(IntrMask, rtl8139_intr_mask);
> __napi_complete(napi);
> + RTL_W16_F(IntrMask, rtl8139_intr_mask);
> spin_unlock_irqrestore(&tp->lock, flags);
> }
> spin_unlock(&tp->rx_lock);
>
>
^ permalink raw reply
* [PATCH net-next-2.6] ipv6: mcast: RCU conversions
From: Eric Dumazet @ 2010-06-08 7:05 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Hideaki YOSHIFUJI
- ipv6_sock_mc_join() : doesnt touch dev refcount
- ipv6_sock_mc_drop() : doesnt touch dev/idev refcounts
- ip6_mc_find_dev() becomes ip6_mc_find_dev_rcu() (called from rcu),
and doesnt touch dev/idev refcounts
- ipv6_sock_mc_close() : doesnt touch dev/idev refcounts
- ip6_mc_source() uses ip6_mc_find_dev_rcu()
- ip6_mc_msfilter() uses ip6_mc_find_dev_rcu()
- ip6_mc_msfget() uses ip6_mc_find_dev_rcu()
- ipv6_dev_mc_dec(), ipv6_chk_mcast_addr(),
igmp6_event_query(), igmp6_event_report(),
mld_sendpack(), igmp6_send() dont touch idev refcount
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
net/ipv6/mcast.c | 183 +++++++++++++++++++++------------------------
1 file changed, 87 insertions(+), 96 deletions(-)
diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 8752e80..3e36d15 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -152,18 +152,19 @@ int ipv6_sock_mc_join(struct sock *sk, int ifindex, const struct in6_addr *addr)
mc_lst->next = NULL;
ipv6_addr_copy(&mc_lst->addr, addr);
+ rcu_read_lock();
if (ifindex == 0) {
struct rt6_info *rt;
rt = rt6_lookup(net, addr, NULL, 0, 0);
if (rt) {
dev = rt->rt6i_dev;
- dev_hold(dev);
dst_release(&rt->u.dst);
}
} else
- dev = dev_get_by_index(net, ifindex);
+ dev = dev_get_by_index_rcu(net, ifindex);
if (dev == NULL) {
+ rcu_read_unlock();
sock_kfree_s(sk, mc_lst, sizeof(*mc_lst));
return -ENODEV;
}
@@ -180,8 +181,8 @@ int ipv6_sock_mc_join(struct sock *sk, int ifindex, const struct in6_addr *addr)
err = ipv6_dev_mc_inc(dev, addr);
if (err) {
+ rcu_read_unlock();
sock_kfree_s(sk, mc_lst, sizeof(*mc_lst));
- dev_put(dev);
return err;
}
@@ -190,7 +191,7 @@ int ipv6_sock_mc_join(struct sock *sk, int ifindex, const struct in6_addr *addr)
np->ipv6_mc_list = mc_lst;
write_unlock_bh(&ipv6_sk_mc_lock);
- dev_put(dev);
+ rcu_read_unlock();
return 0;
}
@@ -213,18 +214,17 @@ int ipv6_sock_mc_drop(struct sock *sk, int ifindex, const struct in6_addr *addr)
*lnk = mc_lst->next;
write_unlock_bh(&ipv6_sk_mc_lock);
- dev = dev_get_by_index(net, mc_lst->ifindex);
+ rcu_read_lock();
+ dev = dev_get_by_index_rcu(net, mc_lst->ifindex);
if (dev != NULL) {
- struct inet6_dev *idev = in6_dev_get(dev);
+ struct inet6_dev *idev = __in6_dev_get(dev);
(void) ip6_mc_leave_src(sk, mc_lst, idev);
- if (idev) {
+ if (idev)
__ipv6_dev_mc_dec(idev, &mc_lst->addr);
- in6_dev_put(idev);
- }
- dev_put(dev);
} else
(void) ip6_mc_leave_src(sk, mc_lst, NULL);
+ rcu_read_unlock();
sock_kfree_s(sk, mc_lst, sizeof(*mc_lst));
return 0;
}
@@ -234,43 +234,36 @@ int ipv6_sock_mc_drop(struct sock *sk, int ifindex, const struct in6_addr *addr)
return -EADDRNOTAVAIL;
}
-static struct inet6_dev *ip6_mc_find_dev(struct net *net,
- struct in6_addr *group,
- int ifindex)
+/* called with rcu_read_lock() */
+static struct inet6_dev *ip6_mc_find_dev_rcu(struct net *net,
+ struct in6_addr *group,
+ int ifindex)
{
struct net_device *dev = NULL;
struct inet6_dev *idev = NULL;
if (ifindex == 0) {
- struct rt6_info *rt;
+ struct rt6_info *rt = rt6_lookup(net, group, NULL, 0, 0);
- rt = rt6_lookup(net, group, NULL, 0, 0);
if (rt) {
dev = rt->rt6i_dev;
dev_hold(dev);
dst_release(&rt->u.dst);
}
} else
- dev = dev_get_by_index(net, ifindex);
+ dev = dev_get_by_index_rcu(net, ifindex);
if (!dev)
- goto nodev;
- idev = in6_dev_get(dev);
+ return NULL;
+ idev = __in6_dev_get(dev);
if (!idev)
- goto release;
+ return NULL;;
read_lock_bh(&idev->lock);
- if (idev->dead)
- goto unlock_release;
-
+ if (idev->dead) {
+ read_unlock_bh(&idev->lock);
+ return NULL;
+ }
return idev;
-
-unlock_release:
- read_unlock_bh(&idev->lock);
- in6_dev_put(idev);
-release:
- dev_put(dev);
-nodev:
- return NULL;
}
void ipv6_sock_mc_close(struct sock *sk)
@@ -286,19 +279,17 @@ void ipv6_sock_mc_close(struct sock *sk)
np->ipv6_mc_list = mc_lst->next;
write_unlock_bh(&ipv6_sk_mc_lock);
- dev = dev_get_by_index(net, mc_lst->ifindex);
+ rcu_read_lock();
+ dev = dev_get_by_index_rcu(net, mc_lst->ifindex);
if (dev) {
- struct inet6_dev *idev = in6_dev_get(dev);
+ struct inet6_dev *idev = __in6_dev_get(dev);
(void) ip6_mc_leave_src(sk, mc_lst, idev);
- if (idev) {
+ if (idev)
__ipv6_dev_mc_dec(idev, &mc_lst->addr);
- in6_dev_put(idev);
- }
- dev_put(dev);
} else
(void) ip6_mc_leave_src(sk, mc_lst, NULL);
-
+ rcu_read_unlock();
sock_kfree_s(sk, mc_lst, sizeof(*mc_lst));
write_lock_bh(&ipv6_sk_mc_lock);
@@ -327,14 +318,17 @@ int ip6_mc_source(int add, int omode, struct sock *sk,
if (!ipv6_addr_is_multicast(group))
return -EINVAL;
- idev = ip6_mc_find_dev(net, group, pgsr->gsr_interface);
- if (!idev)
+ rcu_read_lock();
+ idev = ip6_mc_find_dev_rcu(net, group, pgsr->gsr_interface);
+ if (!idev) {
+ rcu_read_unlock();
return -ENODEV;
+ }
dev = idev->dev;
err = -EADDRNOTAVAIL;
- read_lock_bh(&ipv6_sk_mc_lock);
+ read_lock(&ipv6_sk_mc_lock);
for (pmc=inet6->ipv6_mc_list; pmc; pmc=pmc->next) {
if (pgsr->gsr_interface && pmc->ifindex != pgsr->gsr_interface)
continue;
@@ -358,7 +352,7 @@ int ip6_mc_source(int add, int omode, struct sock *sk,
pmc->sfmode = omode;
}
- write_lock_bh(&pmc->sflock);
+ write_lock(&pmc->sflock);
pmclocked = 1;
psl = pmc->sflist;
@@ -433,11 +427,10 @@ int ip6_mc_source(int add, int omode, struct sock *sk,
ip6_mc_add_src(idev, group, omode, 1, source, 1);
done:
if (pmclocked)
- write_unlock_bh(&pmc->sflock);
- read_unlock_bh(&ipv6_sk_mc_lock);
+ write_unlock(&pmc->sflock);
+ read_unlock(&ipv6_sk_mc_lock);
read_unlock_bh(&idev->lock);
- in6_dev_put(idev);
- dev_put(dev);
+ rcu_read_unlock();
if (leavegroup)
return ipv6_sock_mc_drop(sk, pgsr->gsr_interface, group);
return err;
@@ -463,14 +456,17 @@ int ip6_mc_msfilter(struct sock *sk, struct group_filter *gsf)
gsf->gf_fmode != MCAST_EXCLUDE)
return -EINVAL;
- idev = ip6_mc_find_dev(net, group, gsf->gf_interface);
+ rcu_read_lock();
+ idev = ip6_mc_find_dev_rcu(net, group, gsf->gf_interface);
- if (!idev)
+ if (!idev) {
+ rcu_read_unlock();
return -ENODEV;
+ }
dev = idev->dev;
err = 0;
- read_lock_bh(&ipv6_sk_mc_lock);
+ read_lock(&ipv6_sk_mc_lock);
if (gsf->gf_fmode == MCAST_INCLUDE && gsf->gf_numsrc == 0) {
leavegroup = 1;
@@ -512,7 +508,7 @@ int ip6_mc_msfilter(struct sock *sk, struct group_filter *gsf)
(void) ip6_mc_add_src(idev, group, gsf->gf_fmode, 0, NULL, 0);
}
- write_lock_bh(&pmc->sflock);
+ write_lock(&pmc->sflock);
psl = pmc->sflist;
if (psl) {
(void) ip6_mc_del_src(idev, group, pmc->sfmode,
@@ -522,13 +518,12 @@ int ip6_mc_msfilter(struct sock *sk, struct group_filter *gsf)
(void) ip6_mc_del_src(idev, group, pmc->sfmode, 0, NULL, 0);
pmc->sflist = newpsl;
pmc->sfmode = gsf->gf_fmode;
- write_unlock_bh(&pmc->sflock);
+ write_unlock(&pmc->sflock);
err = 0;
done:
- read_unlock_bh(&ipv6_sk_mc_lock);
+ read_unlock(&ipv6_sk_mc_lock);
read_unlock_bh(&idev->lock);
- in6_dev_put(idev);
- dev_put(dev);
+ rcu_read_unlock();
if (leavegroup)
err = ipv6_sock_mc_drop(sk, gsf->gf_interface, group);
return err;
@@ -551,11 +546,13 @@ int ip6_mc_msfget(struct sock *sk, struct group_filter *gsf,
if (!ipv6_addr_is_multicast(group))
return -EINVAL;
- idev = ip6_mc_find_dev(net, group, gsf->gf_interface);
+ rcu_read_lock();
+ idev = ip6_mc_find_dev_rcu(net, group, gsf->gf_interface);
- if (!idev)
+ if (!idev) {
+ rcu_read_unlock();
return -ENODEV;
-
+ }
dev = idev->dev;
err = -EADDRNOTAVAIL;
@@ -577,8 +574,7 @@ int ip6_mc_msfget(struct sock *sk, struct group_filter *gsf,
psl = pmc->sflist;
count = psl ? psl->sl_count : 0;
read_unlock_bh(&idev->lock);
- in6_dev_put(idev);
- dev_put(dev);
+ rcu_read_unlock();
copycount = count < gsf->gf_numsrc ? count : gsf->gf_numsrc;
gsf->gf_numsrc = count;
@@ -604,8 +600,7 @@ int ip6_mc_msfget(struct sock *sk, struct group_filter *gsf,
return 0;
done:
read_unlock_bh(&idev->lock);
- in6_dev_put(idev);
- dev_put(dev);
+ rcu_read_unlock();
return err;
}
@@ -822,6 +817,7 @@ int ipv6_dev_mc_inc(struct net_device *dev, const struct in6_addr *addr)
struct ifmcaddr6 *mc;
struct inet6_dev *idev;
+ /* we need to take a reference on idev */
idev = in6_dev_get(dev);
if (idev == NULL)
@@ -860,7 +856,7 @@ int ipv6_dev_mc_inc(struct net_device *dev, const struct in6_addr *addr)
setup_timer(&mc->mca_timer, igmp6_timer_handler, (unsigned long)mc);
ipv6_addr_copy(&mc->mca_addr, addr);
- mc->idev = idev;
+ mc->idev = idev; /* (reference taken) */
mc->mca_users = 1;
/* mca_stamp should be updated upon changes */
mc->mca_cstamp = mc->mca_tstamp = jiffies;
@@ -915,16 +911,18 @@ int __ipv6_dev_mc_dec(struct inet6_dev *idev, const struct in6_addr *addr)
int ipv6_dev_mc_dec(struct net_device *dev, const struct in6_addr *addr)
{
- struct inet6_dev *idev = in6_dev_get(dev);
+ struct inet6_dev *idev;
int err;
- if (!idev)
- return -ENODEV;
-
- err = __ipv6_dev_mc_dec(idev, addr);
+ rcu_read_lock();
- in6_dev_put(idev);
+ idev = __in6_dev_get(dev);
+ if (!idev)
+ err = -ENODEV;
+ else
+ err = __ipv6_dev_mc_dec(idev, addr);
+ rcu_read_unlock();
return err;
}
@@ -965,7 +963,8 @@ int ipv6_chk_mcast_addr(struct net_device *dev, const struct in6_addr *group,
struct ifmcaddr6 *mc;
int rv = 0;
- idev = in6_dev_get(dev);
+ rcu_read_lock();
+ idev = __in6_dev_get(dev);
if (idev) {
read_lock_bh(&idev->lock);
for (mc = idev->mc_list; mc; mc=mc->next) {
@@ -992,8 +991,8 @@ int ipv6_chk_mcast_addr(struct net_device *dev, const struct in6_addr *group,
rv = 1; /* don't filter unspecified source */
}
read_unlock_bh(&idev->lock);
- in6_dev_put(idev);
}
+ rcu_read_unlock();
return rv;
}
@@ -1104,6 +1103,7 @@ static int mld_marksources(struct ifmcaddr6 *pmc, int nsrcs,
return 1;
}
+/* called with rcu_read_lock() */
int igmp6_event_query(struct sk_buff *skb)
{
struct mld2_query *mlh2 = NULL;
@@ -1127,7 +1127,7 @@ int igmp6_event_query(struct sk_buff *skb)
if (!(ipv6_addr_type(&ipv6_hdr(skb)->saddr) & IPV6_ADDR_LINKLOCAL))
return -EINVAL;
- idev = in6_dev_get(skb->dev);
+ idev = __in6_dev_get(skb->dev);
if (idev == NULL)
return 0;
@@ -1137,10 +1137,8 @@ int igmp6_event_query(struct sk_buff *skb)
group_type = ipv6_addr_type(group);
if (group_type != IPV6_ADDR_ANY &&
- !(group_type&IPV6_ADDR_MULTICAST)) {
- in6_dev_put(idev);
+ !(group_type&IPV6_ADDR_MULTICAST))
return -EINVAL;
- }
if (len == 24) {
int switchback;
@@ -1161,10 +1159,9 @@ int igmp6_event_query(struct sk_buff *skb)
} else if (len >= 28) {
int srcs_offset = sizeof(struct mld2_query) -
sizeof(struct icmp6hdr);
- if (!pskb_may_pull(skb, srcs_offset)) {
- in6_dev_put(idev);
+ if (!pskb_may_pull(skb, srcs_offset))
return -EINVAL;
- }
+
mlh2 = (struct mld2_query *)skb_transport_header(skb);
max_delay = (MLDV2_MRC(ntohs(mlh2->mld2q_mrc))*HZ)/1000;
if (!max_delay)
@@ -1173,28 +1170,23 @@ int igmp6_event_query(struct sk_buff *skb)
if (mlh2->mld2q_qrv)
idev->mc_qrv = mlh2->mld2q_qrv;
if (group_type == IPV6_ADDR_ANY) { /* general query */
- if (mlh2->mld2q_nsrcs) {
- in6_dev_put(idev);
+ if (mlh2->mld2q_nsrcs)
return -EINVAL; /* no sources allowed */
- }
+
mld_gq_start_timer(idev);
- in6_dev_put(idev);
return 0;
}
/* mark sources to include, if group & source-specific */
if (mlh2->mld2q_nsrcs != 0) {
if (!pskb_may_pull(skb, srcs_offset +
- ntohs(mlh2->mld2q_nsrcs) * sizeof(struct in6_addr))) {
- in6_dev_put(idev);
+ ntohs(mlh2->mld2q_nsrcs) * sizeof(struct in6_addr)))
return -EINVAL;
- }
+
mlh2 = (struct mld2_query *)skb_transport_header(skb);
mark = 1;
}
- } else {
- in6_dev_put(idev);
+ } else
return -EINVAL;
- }
read_lock_bh(&idev->lock);
if (group_type == IPV6_ADDR_ANY) {
@@ -1227,12 +1219,11 @@ int igmp6_event_query(struct sk_buff *skb)
}
}
read_unlock_bh(&idev->lock);
- in6_dev_put(idev);
return 0;
}
-
+/* called with rcu_read_lock() */
int igmp6_event_report(struct sk_buff *skb)
{
struct ifmcaddr6 *ma;
@@ -1260,7 +1251,7 @@ int igmp6_event_report(struct sk_buff *skb)
!(addr_type&IPV6_ADDR_LINKLOCAL))
return -EINVAL;
- idev = in6_dev_get(skb->dev);
+ idev = __in6_dev_get(skb->dev);
if (idev == NULL)
return -ENODEV;
@@ -1280,7 +1271,6 @@ int igmp6_event_report(struct sk_buff *skb)
}
}
read_unlock_bh(&idev->lock);
- in6_dev_put(idev);
return 0;
}
@@ -1396,12 +1386,14 @@ static void mld_sendpack(struct sk_buff *skb)
struct mld2_report *pmr =
(struct mld2_report *)skb_transport_header(skb);
int payload_len, mldlen;
- struct inet6_dev *idev = in6_dev_get(skb->dev);
+ struct inet6_dev *idev;
struct net *net = dev_net(skb->dev);
int err;
struct flowi fl;
struct dst_entry *dst;
+ rcu_read_lock();
+ idev = __in6_dev_get(skb->dev);
IP6_UPD_PO_STATS(net, idev, IPSTATS_MIB_OUT, skb->len);
payload_len = (skb->tail - skb->network_header) - sizeof(*pip6);
@@ -1441,8 +1433,7 @@ out:
} else
IP6_INC_STATS_BH(net, idev, IPSTATS_MIB_OUTDISCARDS);
- if (likely(idev != NULL))
- in6_dev_put(idev);
+ rcu_read_unlock();
return;
err_out:
@@ -1779,7 +1770,8 @@ static void igmp6_send(struct in6_addr *addr, struct net_device *dev, int type)
IPPROTO_ICMPV6,
csum_partial(hdr, len, 0));
- idev = in6_dev_get(skb->dev);
+ rcu_read_lock();
+ idev = __in6_dev_get(skb->dev);
dst = icmp6_dst_alloc(skb->dev, NULL, &ipv6_hdr(skb)->daddr);
if (!dst) {
@@ -1806,8 +1798,7 @@ out:
} else
IP6_INC_STATS(net, idev, IPSTATS_MIB_OUTDISCARDS);
- if (likely(idev != NULL))
- in6_dev_put(idev);
+ rcu_read_unlock();
return;
err_out:
^ permalink raw reply related
* [PATCH] fix a race at the end of NAPI
From: Figo.zhang @ 2010-06-08 6:48 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev
fix a race at the end of NAPI complete processing. it had better do __napi_complete()
first before re-enable interrupt.
Signed-off-by: Figo.zhang <figo1802@gmail.com>
---
drivers/net/8139cp.c | 2 +-
drivers/net/8139too.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/8139cp.c b/drivers/net/8139cp.c
old mode 100644
new mode 100755
index 9c14975..284a5f4
--- a/drivers/net/8139cp.c
+++ b/drivers/net/8139cp.c
@@ -598,8 +598,8 @@ rx_next:
goto rx_status_loop;
spin_lock_irqsave(&cp->lock, flags);
- cpw16_f(IntrMask, cp_intr_mask);
__napi_complete(napi);
+ cpw16_f(IntrMask, cp_intr_mask);
spin_unlock_irqrestore(&cp->lock, flags);
}
diff --git a/drivers/net/8139too.c b/drivers/net/8139too.c
old mode 100644
new mode 100755
index 4ba7293..a7bca8c
--- a/drivers/net/8139too.c
+++ b/drivers/net/8139too.c
@@ -2088,8 +2088,8 @@ static int rtl8139_poll(struct napi_struct *napi, int budget)
* again when we think we are done.
*/
spin_lock_irqsave(&tp->lock, flags);
- RTL_W16_F(IntrMask, rtl8139_intr_mask);
__napi_complete(napi);
+ RTL_W16_F(IntrMask, rtl8139_intr_mask);
spin_unlock_irqrestore(&tp->lock, flags);
}
spin_unlock(&tp->rx_lock);
^ permalink raw reply related
* [PATCH] ipvs: Add missing locking during connection table hashing and unhashing
From: Sven Wegener @ 2010-06-08 6:29 UTC (permalink / raw)
To: Patrick McHardy
Cc: Julian Anastasov, Simon Horman, Wensong Zhang, netdev, lvs-devel
In-Reply-To: <20100524235637.GG4794@verge.net.au>
The code that hashes and unhashes connections from the connection table
is missing locking of the connection being modified, which opens up a
race condition and results in memory corruption when this race condition
is hit.
Here is what happens in pretty verbose form:
CPU 0 CPU 1
------------ ------------
An active connection is terminated and
we schedule ip_vs_conn_expire() on this
CPU to expire this connection.
IRQ assignment is changed to this CPU,
but the expire timer stays scheduled on
the other CPU.
New connection from same ip:port comes
in right before the timer expires, we
find the inactive connection in our
connection table and get a reference to
it. We proper lock the connection in
tcp_state_transition() and read the
connection flags in set_tcp_state().
ip_vs_conn_expire() gets called, we
unhash the connection from our
connection table and remove the hashed
flag in ip_vs_conn_unhash(), without
proper locking!
While still holding proper locks we
write the connection flags in
set_tcp_state() and this sets the hashed
flag again.
ip_vs_conn_expire() fails to expire the
connection, because the other CPU has
incremented the reference count. We try
to re-insert the connection into our
connection table, but this fails in
ip_vs_conn_hash(), because the hashed
flag has been set by the other CPU. We
re-schedule execution of
ip_vs_conn_expire(). Now this connection
has the hashed flag set, but isn't
actually hashed in our connection table
and has a dangling list_head.
We drop the reference we held on the
connection and schedule the expire timer
for timeouting the connection on this
CPU. Further packets won't be able to
find this connection in our connection
table.
ip_vs_conn_expire() gets called again,
we think it's already hashed, but the
list_head is dangling and while removing
the connection from our connection table
we write to the memory location where
this list_head points to.
The result will probably be a kernel oops at some other point in time.
Signed-off-by: Sven Wegener <sven.wegener@stealer.net>
Cc: stable@kernel.org
Acked-by: Simon Horman <horms@verge.net.au>
---
net/netfilter/ipvs/ip_vs_conn.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
This race condition is pretty subtle, but it can be triggered remotely.
It needs the IRQ assignment change or another circumstance where packets
coming from the same ip:port for the same service are being processed on
different CPUs. And it involves hitting the exact time at which
ip_vs_conn_expire() gets called. It can be avoided by making sure that
all packets from one connection are always processed on the same CPU and
can be made harder to exploit by changing the connection timeouts to
some custom values.
diff --git a/net/netfilter/ipvs/ip_vs_conn.c b/net/netfilter/ipvs/ip_vs_conn.c
index d8f7e8e..ff04e9e 100644
--- a/net/netfilter/ipvs/ip_vs_conn.c
+++ b/net/netfilter/ipvs/ip_vs_conn.c
@@ -162,6 +162,7 @@ static inline int ip_vs_conn_hash(struct ip_vs_conn *cp)
hash = ip_vs_conn_hashkey(cp->af, cp->protocol, &cp->caddr, cp->cport);
ct_write_lock(hash);
+ spin_lock(&cp->lock);
if (!(cp->flags & IP_VS_CONN_F_HASHED)) {
list_add(&cp->c_list, &ip_vs_conn_tab[hash]);
@@ -174,6 +175,7 @@ static inline int ip_vs_conn_hash(struct ip_vs_conn *cp)
ret = 0;
}
+ spin_unlock(&cp->lock);
ct_write_unlock(hash);
return ret;
@@ -193,6 +195,7 @@ static inline int ip_vs_conn_unhash(struct ip_vs_conn *cp)
hash = ip_vs_conn_hashkey(cp->af, cp->protocol, &cp->caddr, cp->cport);
ct_write_lock(hash);
+ spin_lock(&cp->lock);
if (cp->flags & IP_VS_CONN_F_HASHED) {
list_del(&cp->c_list);
@@ -202,6 +205,7 @@ static inline int ip_vs_conn_unhash(struct ip_vs_conn *cp)
} else
ret = 0;
+ spin_unlock(&cp->lock);
ct_write_unlock(hash);
return ret;
^ permalink raw reply related
* Re: [PATCH] r8169: fix random mdio_write failures
From: Francois Romieu @ 2010-06-08 6:26 UTC (permalink / raw)
To: Timo Teräs; +Cc: hayeswang, netdev, davem
In-Reply-To: <4C0DDDCC.6010500@iki.fi>
Timo Teräs <timo.teras@iki.fi> :
[ok]
> iteration. It sounds unlikely that polling the status register would
> slow down the actual write operation (if that is the case then 100us
> would be desirable).
I would not be that surprized.
> Changing my 25us to 20us would good. The original 25us was just a guess.
> The comment should be probably also updated that those delays are from
> realtek hw specs then.
Yes.
> Would you like me to send a patch?
Of course. Some comment from Hayes regarding mdio_read would be
welcome beforehand though.
--
Ueimor
^ permalink raw reply
* [PATCH] net/irda/sh_irda: Modify clk_get lookups
From: Kuninori Morimoto @ 2010-06-08 6:25 UTC (permalink / raw)
To: Paul Mundt, David S. Miller; +Cc: Magnus, Linux-SH, Linux-Net
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
This patch is v2 of
ARM: mach-shmobile: clock-sh7367: modify IrDA clock
arch/arm/mach-shmobile/board-g3evm.c | 1 +
drivers/net/irda/sh_irda.c | 6 ++----
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/arch/arm/mach-shmobile/board-g3evm.c b/arch/arm/mach-shmobile/board-g3evm.c
index 95ccb94..a552590 100644
--- a/arch/arm/mach-shmobile/board-g3evm.c
+++ b/arch/arm/mach-shmobile/board-g3evm.c
@@ -232,6 +232,7 @@ static struct resource irda_resources[] = {
static struct platform_device irda_device = {
.name = "sh_irda",
+ .id = -1,
.resource = irda_resources,
.num_resources = ARRAY_SIZE(irda_resources),
};
diff --git a/drivers/net/irda/sh_irda.c b/drivers/net/irda/sh_irda.c
index 9a828b0..9db7084 100644
--- a/drivers/net/irda/sh_irda.c
+++ b/drivers/net/irda/sh_irda.c
@@ -748,7 +748,6 @@ static int __devinit sh_irda_probe(struct platform_device *pdev)
struct net_device *ndev;
struct sh_irda_self *self;
struct resource *res;
- char clk_name[8];
unsigned int irq;
int err = -ENOMEM;
@@ -775,10 +774,9 @@ static int __devinit sh_irda_probe(struct platform_device *pdev)
if (err)
goto err_mem_2;
- snprintf(clk_name, sizeof(clk_name), "irda%d", pdev->id);
- self->clk = clk_get(&pdev->dev, clk_name);
+ self->clk = clk_get(&pdev->dev, NULL);
if (IS_ERR(self->clk)) {
- dev_err(&pdev->dev, "cannot get clock \"%s\"\n", clk_name);
+ dev_err(&pdev->dev, "cannot get irda clock\n");
goto err_mem_3;
}
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH] r8169: fix random mdio_write failures
From: Timo Teräs @ 2010-06-08 6:06 UTC (permalink / raw)
To: Francois Romieu; +Cc: hayeswang, netdev, davem
In-Reply-To: <20100607215115.GA6583@electric-eye.fr.zoreil.com>
On 06/08/2010 12:51 AM, Francois Romieu wrote:
> hayeswang <hayeswang@realtek.com> :
>> Our hardware engineer suggests that check the completed indication
>> per 100 micro seconds. And it needs 20 micro seconds delay after the
>> completed indication for the next command.
>
> Should we do the same for mdio_read as well (100 us per iteration +
> an extra 20 us) ?
Well, doing 100us per iteration will increase the latency that the code
notices "write complete" which slows down things. It'll also slightly
decrease bus traffic which is good. But I'd be just fine with 25us per
iteration. It sounds unlikely that polling the status register would
slow down the actual write operation (if that is the case then 100us
would be desirable).
Changing my 25us to 20us would good. The original 25us was just a guess.
The comment should be probably also updated that those delays are from
realtek hw specs then.
Would you like me to send a patch?
- Timo
^ permalink raw reply
* Re: [PATCH net-next-2.6] anycast: Some RCU conversions
From: David Miller @ 2010-06-08 5:57 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, yoshfuji
In-Reply-To: <1275946933.2775.16.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 07 Jun 2010 23:42:13 +0200
> - dev_get_by_flags() changed to dev_get_by_flags_rcu()
>
> - ipv6_sock_ac_join() dont touch dev & idev refcounts
> - ipv6_sock_ac_drop() dont touch dev & idev refcounts
> - ipv6_sock_ac_close() dont touch dev & idev refcounts
> - ipv6_dev_ac_dec() dount touch idev refcount
> - ipv6_chk_acast_addr() dont touch idev refcount
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> CC: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
Looks great, applied, thanks Eric!
^ permalink raw reply
* Re: [PATCH net-2.6] pkt_sched: gen_estimator: add a new lock
From: Eric Dumazet @ 2010-06-08 5:39 UTC (permalink / raw)
To: Changli Gao
Cc: David Miller, netdev, Stephen Hemminger, Jarek Poplawski,
Patrick McHardy
In-Reply-To: <AANLkTik6xx0cxZKezU31K4vYg4Rz0o7ndmN4D9vYSCKX@mail.gmail.com>
Le mardi 08 juin 2010 à 13:20 +0800, Changli Gao a écrit :
> IMO, this bug should be fixed by adding rtnl_lock to xt_RATEEST.c.
> Killing rtnl should be done in separated patches. They are different
> things. Your patch introduces another locks, and it is extra overhead
> for other users.
>
extra overhead, in new/kill estimators ?
Are you kidding ?
RTNL is taken, taking an extra-uncontended spinlock is free.
Nope, I wont add rtnl lock to xt_RATEEST.c
I believe you dont really understood what I patiently explained to you.
Thats becoming rediculous.
^ permalink raw reply
* [RFC PATCH 5/5] perf:add a script shows a process of packet
From: Koki Sanagi @ 2010-06-08 5:31 UTC (permalink / raw)
To: netdev; +Cc: davem, kaneshige.kenji, izumi.taku
In-Reply-To: <4C0DD43F.9090902@jp.fujitsu.com>
This perf script show a time-chart of process of packet.
If you want to use it, install perf and
#perf trace record netdev-times
And if you want a result,
#perf trace report netdev-times
Signed-off-by: Koki Sanagi <sanagi.koki@jp.fujitsu.com>
---
tools/perf/scripts/python/bin/netdev-times-record | 7 +
tools/perf/scripts/python/bin/netdev-times-report | 10 +
tools/perf/scripts/python/netdev-times.py | 451 +++++++++++++++++++++
3 files changed, 468 insertions(+), 0 deletions(-)
diff --git a/tools/perf/scripts/python/bin/netdev-times-record b/tools/perf/scripts/python/bin/netdev-times-record
new file mode 100644
index 0000000..c6a54fd
--- /dev/null
+++ b/tools/perf/scripts/python/bin/netdev-times-record
@@ -0,0 +1,7 @@
+#!/bin/bash
+perf record -c 1 -f -R -a -e net:net_dev_xmit -e net:net_dev_queue \
+ -e net:net_dev_receive -e skb:consume_skb \
+ -e skb:dev_kfree_skb_irq -e napi:napi_poll \
+ -e irq:irq_handler_entry -e irq:irq_handler_exit \
+ -e irq:softirq_entry -e irq:softirq_exit \
+ -e irq:softirq_raise -e skb:skb_copy_datagram_iovec
diff --git a/tools/perf/scripts/python/bin/netdev-times-report b/tools/perf/scripts/python/bin/netdev-times-report
new file mode 100644
index 0000000..5d24c3d
--- /dev/null
+++ b/tools/perf/scripts/python/bin/netdev-times-report
@@ -0,0 +1,10 @@
+#!/bin/bash
+# description: displayi a process of packet and processing time
+# args: [comm]
+if [ $# -gt 0 ] ; then
+ if ! expr match "$1" "-" > /dev/null ; then
+ comm=$1
+ shift
+ fi
+fi
+perf trace $@ -s ~/libexec/perf-core/scripts/python/netdev-times.py $comm
diff --git a/tools/perf/scripts/python/netdev-times.py b/tools/perf/scripts/python/netdev-times.py
new file mode 100644
index 0000000..e7b47c7
--- /dev/null
+++ b/tools/perf/scripts/python/netdev-times.py
@@ -0,0 +1,451 @@
+# Display process of packets and processed time.
+# It helps you to investigate networking.
+
+import os
+import sys
+
+sys.path.append(os.environ['PERF_EXEC_PATH'] + \
+ '/scripts/python/Perf-Trace-Util/lib/Perf/Trace')
+
+from perf_trace_context import *
+from Core import *
+from Util import *
+
+all_event_list = []; # insert all tracepoint event related with this script
+irq_dic = {}; # key is cpu and value is a list which stacks irqs
+ # which raise NET_RX softirq
+net_rx_dic = {}; # key is cpu and value include time of NET_RX softirq-entry
+ # and a list which stacks receive
+receive_hunk_list = []; # a list which include a sequence of receive events
+receive_skb_list = []; # received packet list for matching
+ # skb_copy_datagram_iovec
+
+queue_list = []; # list of packets which pass through dev_queue_xmit
+xmit_list = []; # list of packets which pass through dev_hard_start_xmit
+free_list = []; # list of packets which is freed
+# Calculate a time interval(msec) from src(nsec) to dst(nsec)
+def diff_msec(src, dst):
+ return (dst - src) / 1000000.0
+
+# Display a process of transmitting a packet
+def print_transmit(hunk):
+ print "%7s %5d %6d.%09dsec %12.6fmsec %12.6fmsec" % \
+ (hunk['dev'], hunk['len'],
+ nsecs_secs(hunk['queue_t']),
+ nsecs_nsecs(hunk['queue_t']),
+ diff_msec(hunk['queue_t'], hunk['xmit_t']),
+ diff_msec(hunk['xmit_t'], hunk['free_t']))
+
+# Display a process of received packets and interrputs associated with
+# a NET_RX softirq
+def print_receive(hunk):
+ if 'irq_list' not in hunk.keys() \
+ or len(hunk['irq_list']) == 0:
+ return
+ irq_list = hunk['irq_list']
+ cpu = irq_list[0]['cpu']
+ base_t = irq_list[0]['irq_ent_t']
+ print "%d.%09dsec cpu=%d" % \
+ (nsecs_secs(base_t), nsecs_nsecs(base_t), cpu)
+ for i in range(len(irq_list)):
+ print "irq_entry(+%fmsec,irq=%d:%s)" % \
+ (diff_msec(base_t, irq_list[i]['irq_ent_t']),
+ irq_list[i]['irq'], irq_list[i]['name'])
+
+ print " |------------" \
+ "softirq_raise(+%fmsec)" % \
+ diff_msec(base_t, irq_list[i]['sirq_raise_t'])
+
+ print "irq_exit (+%fmsec) |" % \
+ diff_msec(base_t, irq_list[i]['irq_ext_t'])
+
+ print " |"
+
+ if 'sirq_ent_t' not in hunk.keys():
+ print 'maybe softirq_entry is dropped'
+ return
+ print " " \
+ "softirq_entry(+%fmsec)\n" \
+ " " \
+ " |" % \
+ diff_msec(base_t, hunk['sirq_ent_t'])
+ event_list = hunk['event_list']
+ for i in range(len(event_list)):
+ event = event_list[i]
+ if event['event_name'] == 'napi_poll':
+ print " " \
+ "napi_poll_exit(+%fmsec, %s)" % \
+ (diff_msec(base_t, event['event_t']), event['dev'])
+ print " " \
+ " |"
+ elif 'comm' in event.keys():
+ print " " \
+ " |---netif_receive_skb" \
+ "(+%fmsec,len=%d)\n" \
+ " " \
+ " | |\n" \
+ " " \
+ " | skb_copy_datagram_iovec" \
+ "(+%fmsec, %d:%s)\n" \
+ " " \
+ " |" % \
+ (diff_msec(base_t, event['event_t']),
+ event['len'],
+ diff_msec(base_t, event['comm_t']),
+ event['pid'], event['comm'])
+ else:
+ print " " \
+ " |---netif_receive_skb" \
+ "(+%fmsec,len=%d)\n" \
+ " " \
+ " |" % \
+ (diff_msec(base_t, event['event_t']),
+ event['len'])
+
+ print " " \
+ "softirq_exit(+%fmsec)\n" % \
+ diff_msec(base_t, hunk['sirq_ext_t'])
+
+def trace_end():
+ # order all events in time
+ all_event_list.sort(lambda a,b :cmp(a['time'], b['time']))
+ # process all events
+ for i in range(len(all_event_list)):
+ event = all_event_list[i]
+ event_name = event['event_name']
+ if event_name == 'irq__softirq_exit':
+ handle_irq_softirq_exit(event['event_name'],
+ event['context'], event['common_cpu'],
+ event['common_pid'], event['common_comm'],
+ event['time'], event['vec'])
+ elif event_name == 'irq__softirq_entry':
+ handle_irq_softirq_entry(event['event_name'],
+ event['context'], event['common_cpu'],
+ event['common_pid'], event['common_comm'],
+ event['time'],event['vec'])
+ elif event_name == 'irq__softirq_raise':
+ handle_irq_softirq_raise(event['event_name'],
+ event['context'], event['common_cpu'],
+ event['common_pid'], event['common_comm'],
+ event['time'], event['vec'])
+ elif event_name == 'irq__irq_handler_entry':
+ handle_irq_handler_entry(event['event_name'],
+ event['context'], event['common_cpu'],
+ event['common_pid'], event['common_comm'],
+ event['time'], event['irq'], event['name'])
+ elif event_name == 'irq__irq_handler_exit':
+ handle_irq_handler_exit(event['event_name'],
+ event['context'], event['common_cpu'],
+ event['common_pid'], event['common_comm'],
+ event['time'], event['irq'], event['ret'])
+ elif event_name == 'napi__napi_poll':
+ handle_napi_poll(event['event_name'],
+ event['context'], event['common_cpu'],
+ event['common_pid'], event['common_comm'],
+ event['time'], event['napi'],
+ event['dev_name'])
+ elif event_name == 'net__net_dev_receive':
+ handle_net_dev_receive(event['event_name'],
+ event['context'], event['common_cpu'],
+ event['common_pid'], event['common_comm'],
+ event['time'], event['skbaddr'],
+ event['skblen'], event['name'])
+ elif event_name == 'skb__skb_copy_datagram_iovec':
+ handle_skb_copy_datagram_iovec(event['event_name'],
+ event['context'], event['common_cpu'],
+ event['common_pid'], event['common_comm'],
+ event['time'], event['skbaddr'],
+ event['skblen'])
+ elif event_name == 'net__net_dev_queue':
+ handle_net_dev_queue(event['event_name'],
+ event['context'], event['common_cpu'],
+ event['common_pid'], event['common_comm'],
+ event['time'], event['skbaddr'],
+ event['skblen'], event['name'])
+ elif event_name == 'net__net_dev_xmit':
+ handle_net_dev_xmit(event['event_name'],
+ event['context'], event['common_cpu'],
+ event['common_pid'], event['common_comm'],
+ event['time'], event['skbaddr'],
+ event['skblen'], event['rc'], event['name'])
+ elif event_name == 'skb__dev_kfree_skb_irq':
+ handle_dev_kfree_skb_irq(event['event_name'],
+ event['context'], event['common_cpu'],
+ event['common_pid'], event['common_comm'],
+ event['time'], event['skbaddr'])
+ elif event_name == 'skb__consume_skb':
+ handle_consume_skb(event['event_name'],
+ event['context'], event['common_cpu'],
+ event['common_pid'], event['common_comm'],
+ event['time'], event['skbaddr'])
+ # display receive hunks
+ for i in range(len(receive_hunk_list)):
+ print_receive(receive_hunk_list[i])
+ # display transmit hunks
+ print " dev len dev_queue_xmit|----------|" \
+ "dev_hard_start_xmit|-----|free_skb"
+ print " | |" \
+ " |"
+ for i in range(len(free_list)):
+ print_transmit(free_list[i])
+
+def irq__softirq_exit(event_name, context, common_cpu,
+ common_secs, common_nsecs, common_pid, common_comm,
+ vec):
+ if symbol_str("irq__softirq_entry", "vec", vec) != "NET_RX":
+ return
+ event_data = {'event_name':event_name, 'context':context,
+ 'common_cpu':common_cpu, 'common_pid':common_pid,
+ 'common_comm':common_comm,'time':nsecs(common_secs,
+ common_nsecs),
+ 'vec':vec}
+ all_event_list.append(event_data)
+
+def handle_irq_softirq_exit(event_name, context, common_cpu,
+ common_pid, common_comm, time,
+ vec):
+ rec_data = {'sirq_ext_t':time}
+ if common_cpu in irq_dic.keys():
+ rec_data.update({'irq_list':irq_dic[common_cpu]})
+ del irq_dic[common_cpu]
+ if common_cpu in net_rx_dic.keys():
+ rec_data.update({
+ 'event_list':net_rx_dic[common_cpu]['event_list'],
+ 'sirq_ent_t':net_rx_dic[common_cpu]['sirq_ent_t']})
+ del net_rx_dic[common_cpu]
+ # merge information realted to a NET_RX softirq
+ receive_hunk_list.append(rec_data)
+
+def irq__softirq_entry(event_name, context, common_cpu,
+ common_secs, common_nsecs, common_pid, common_comm,
+ vec):
+ if symbol_str("irq__softirq_entry", "vec", vec) != "NET_RX":
+ return
+ event_data = {'event_name':event_name, 'context':context,
+ 'common_cpu':common_cpu, 'common_pid':common_pid,
+ 'common_comm':common_comm,'time':nsecs(common_secs,
+ common_nsecs),
+ 'vec':vec}
+ all_event_list.append(event_data)
+
+def handle_irq_softirq_entry(event_name, context, common_cpu,
+ common_pid, common_comm, time,
+ vec):
+ net_rx_dic[common_cpu] = {'event_list':[],
+ 'sirq_ent_t':time}
+
+def irq__softirq_raise(event_name, context, common_cpu,
+ common_secs, common_nsecs, common_pid, common_comm,
+ vec):
+ if symbol_str("irq__softirq_entry", "vec", vec) != "NET_RX":
+ return
+ event_data = {'event_name':event_name, 'context':context,
+ 'common_cpu':common_cpu, 'common_pid':common_pid,
+ 'common_comm':common_comm,'time':nsecs(common_secs,
+ common_nsecs),
+ 'vec':vec}
+ all_event_list.append(event_data)
+
+def handle_irq_softirq_raise(event_name, context, common_cpu,
+ common_pid, common_comm, time,
+ vec):
+ if common_cpu not in irq_dic.keys() \
+ or len(irq_dic[common_cpu]) == 0:
+ return
+ irq = irq_dic[common_cpu].pop()
+ # put a time to prev irq on the same cpu
+ irq.update({'sirq_raise_t':time})
+ irq_dic[common_cpu].append(irq)
+
+def irq__irq_handler_entry(event_name, context, common_cpu,
+ common_secs, common_nsecs, common_pid, common_comm,
+ irq, name):
+ event_data = {'event_name':event_name, 'context':context,
+ 'common_cpu':common_cpu, 'common_pid':common_pid,
+ 'common_comm':common_comm,'time':nsecs(common_secs,
+ common_nsecs),
+ 'irq':irq, 'name':name}
+ all_event_list.append(event_data)
+
+def handle_irq_handler_entry(event_name, context, common_cpu,
+ common_pid, common_comm, time,
+ irq, name):
+ if common_cpu not in irq_dic.keys():
+ irq_dic[common_cpu] = []
+ irq_record = {'irq':irq,
+ 'name':name,
+ 'cpu':common_cpu,
+ 'irq_ent_t':time}
+ irq_dic[common_cpu].append(irq_record)
+
+def irq__irq_handler_exit(event_name, context, common_cpu,
+ common_secs, common_nsecs, common_pid, common_comm,
+ irq, ret):
+ event_data = {'event_name':event_name, 'context':context,
+ 'common_cpu':common_cpu, 'common_pid':common_pid,
+ 'common_comm':common_comm,'time':nsecs(common_secs,
+ common_nsecs),
+ 'irq':irq, 'ret':ret}
+ all_event_list.append(event_data)
+
+def handle_irq_handler_exit(event_name, context, common_cpu,
+ common_pid, common_comm, time,
+ irq, ret):
+ if common_cpu not in irq_dic.keys():
+ return
+ irq_record = irq_dic[common_cpu].pop()
+ irq_record.update({'irq_ext_t':time})
+ # if an irq doesn't include NET_RX softirq, drop.
+ if 'sirq_raise_t' in irq_record.keys():
+ irq_dic[common_cpu].append(irq_record)
+
+def napi__napi_poll(event_name, context, common_cpu,
+ common_secs, common_nsecs, common_pid, common_comm,
+ napi, dev_name):
+ event_data = {'event_name':event_name, 'context':context,
+ 'common_cpu':common_cpu, 'common_pid':common_pid,
+ 'common_comm':common_comm,'time':nsecs(common_secs,
+ common_nsecs),
+ 'napi':napi, 'dev_name':dev_name}
+ all_event_list.append(event_data)
+
+def handle_napi_poll(event_name, context, common_cpu,
+ common_pid, common_comm, time,
+ napi, dev_name):
+ if common_cpu in net_rx_dic.keys():
+ event_list = net_rx_dic[common_cpu]['event_list']
+ rec_data = {'event_name':'napi_poll',
+ 'dev':dev_name,
+ 'event_t':time}
+ event_list.append(rec_data)
+
+def net__net_dev_receive(event_name, context, common_cpu,
+ common_secs, common_nsecs, common_pid, common_comm,
+ skbaddr,skblen, name):
+ event_data = {'event_name':event_name, 'context':context,
+ 'common_cpu':common_cpu, 'common_pid':common_pid,
+ 'common_comm':common_comm,'time':nsecs(common_secs,
+ common_nsecs),
+ 'skbaddr':skbaddr, 'skblen':skblen, 'name':name}
+ all_event_list.append(event_data)
+
+def handle_net_dev_receive(event_name, context, common_cpu,
+ ccommon_pid, common_comm, time,
+ skbaddr, skblen, name):
+ if common_cpu in net_rx_dic.keys():
+ rec_data = {'event_name':'netif_receive_skb',
+ 'event_t':time,
+ 'skbaddr':skbaddr,
+ 'len':skblen}
+ event_list = net_rx_dic[common_cpu]['event_list']
+ event_list.append(rec_data)
+ receive_skb_list.insert(0, rec_data)
+
+def skb__skb_copy_datagram_iovec(event_name, context, common_cpu,
+ common_secs, common_nsecs, common_pid, common_comm,
+ skbaddr, skblen):
+ event_data = {'event_name':event_name, 'context':context,
+ 'common_cpu':common_cpu, 'common_pid':common_pid,
+ 'common_comm':common_comm,'time':nsecs(common_secs,
+ common_nsecs),
+ 'skbaddr':skbaddr, 'skblen':skblen}
+ all_event_list.append(event_data)
+
+def handle_skb_copy_datagram_iovec(event_name, context, common_cpu,
+ common_pid, common_comm, time,
+ skbaddr, skblen):
+ for i in range(len(receive_skb_list)):
+ rec_data = receive_skb_list[i]
+ if skbaddr == rec_data['skbaddr'] and \
+ 'comm' not in rec_data.keys():
+ rec_data.update({'comm':common_comm,
+ 'pid':common_pid,
+ 'comm_t':time})
+ del receive_skb_list[i]
+ break
+
+def net__net_dev_queue(event_name, context, common_cpu,
+ common_secs, common_nsecs, common_pid, common_comm,
+ skbaddr, skblen, name):
+ event_data = {'event_name':event_name, 'context':context,
+ 'common_cpu':common_cpu, 'common_pid':common_pid,
+ 'common_comm':common_comm,'time':nsecs(common_secs,
+ common_nsecs),
+ 'skbaddr':skbaddr, 'skblen':skblen, 'name':name}
+ all_event_list.append(event_data)
+
+def handle_net_dev_queue(event_name, context, common_cpu,
+ common_pid, common_comm, time,
+ skbaddr, skblen, name):
+ skb = {'dev':name,
+ 'skbaddr':skbaddr,
+ 'len':skblen,
+ 'queue_t':time}
+ xmit_list.insert(0, skb)
+
+def net__net_dev_xmit(event_name, context, common_cpu,
+ common_secs, common_nsecs, common_pid, common_comm,
+ skbaddr, skblen, rc, name):
+ event_data = {'event_name':event_name, 'context':context,
+ 'common_cpu':common_cpu, 'common_pid':common_pid,
+ 'common_comm':common_comm,'time':nsecs(common_secs,
+ common_nsecs),
+ 'skbaddr':skbaddr, 'skblen':skblen, 'rc':rc, 'name':name}
+ all_event_list.append(event_data)
+
+def handle_net_dev_xmit(event_name, context, common_cpu,
+ common_pid, common_comm, time,
+ skbaddr, skblen, rc, name):
+ if rc == 0: # NETDEV_TX_OK
+ for i in range(len(xmit_list)):
+ skb = xmit_list[i]
+ if skb['skbaddr'] == skbaddr:
+ skb['xmit_t'] = time
+ queue_list.insert(0, skb)
+ del xmit_list[i]
+ break
+
+def free_skb(event_name, context, common_cpu,
+ common_pid, common_comm, time,
+ skbaddr):
+ for i in range(len(queue_list)):
+ skb = queue_list[i]
+ if skb['skbaddr'] ==skbaddr:
+ skb['free_t'] = time
+ free_list.append(skb)
+ del queue_list[i]
+ break
+
+def skb__dev_kfree_skb_irq(event_name, context, common_cpu,
+ common_secs, common_nsecs, common_pid, common_comm,
+ skbaddr):
+ event_data = {'event_name':event_name, 'context':context,
+ 'common_cpu':common_cpu, 'common_pid':common_pid,
+ 'common_comm':common_comm,'time':nsecs(common_secs,
+ common_nsecs),
+ 'skbaddr':skbaddr}
+ all_event_list.append(event_data)
+
+def handle_dev_kfree_skb_irq(event_name, context, common_cpu,
+ common_pid, common_comm, time,
+ skbaddr):
+ free_skb(event_name, context, common_cpu,
+ common_pid, common_comm, time,
+ skbaddr)
+
+def skb__consume_skb(event_name, context, common_cpu,
+ common_secs, common_nsecs, common_pid, common_comm,
+ skbaddr):
+ event_data = {'event_name':event_name, 'context':context,
+ 'common_cpu':common_cpu, 'common_pid':common_pid,
+ 'common_comm':common_comm,'time':nsecs(common_secs,
+ common_nsecs),
+ 'skbaddr':skbaddr}
+ all_event_list.append(event_data)
+
+def handle_consume_skb(event_name, context, common_cpu,
+ common_pid, common_comm, time,
+ skbaddr):
+ free_skb(event_name, context, common_cpu,
+ common_pid, common_comm, time,
+ skbaddr)
^ permalink raw reply related
* [RFC PATCH 4/5] skb: add tracepoints to freeing skb
From: Koki Sanagi @ 2010-06-08 5:30 UTC (permalink / raw)
To: netdev; +Cc: davem, kaneshige.kenji, izumi.taku
In-Reply-To: <4C0DD43F.9090902@jp.fujitsu.com>
This patch adds tracepoint to consume_skb and dev_kfree_skb_irq.
Signed-off-by: Koki Sanagi <sanagi.koki@jp.fujitsu.com>
---
include/trace/events/skb.h | 36 ++++++++++++++++++++++++++++++++++++
net/core/dev.c | 2 ++
net/core/skbuff.c | 1 +
3 files changed, 39 insertions(+), 0 deletions(-)
diff --git a/include/trace/events/skb.h b/include/trace/events/skb.h
index 4b2be6d..6ab5b34 100644
--- a/include/trace/events/skb.h
+++ b/include/trace/events/skb.h
@@ -35,6 +35,42 @@ TRACE_EVENT(kfree_skb,
__entry->skbaddr, __entry->protocol, __entry->location)
);
+TRACE_EVENT(consume_skb,
+
+ TP_PROTO(struct sk_buff *skb),
+
+ TP_ARGS(skb),
+
+ TP_STRUCT__entry(
+ __field( void *, skbaddr )
+ ),
+
+ TP_fast_assign(
+ __entry->skbaddr = skb;
+ ),
+
+ TP_printk("skbaddr=%p",
+ __entry->skbaddr)
+);
+
+TRACE_EVENT(dev_kfree_skb_irq,
+
+ TP_PROTO(struct sk_buff *skb),
+
+ TP_ARGS(skb),
+
+ TP_STRUCT__entry(
+ __field( void *, skbaddr )
+ ),
+
+ TP_fast_assign(
+ __entry->skbaddr = skb;
+ ),
+
+ TP_printk("skbaddr=%p",
+ __entry->skbaddr)
+);
+
TRACE_EVENT(skb_copy_datagram_iovec,
TP_PROTO(const struct sk_buff *skb, int len),
diff --git a/net/core/dev.c b/net/core/dev.c
index f7c731b..e0093c4 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -131,6 +131,7 @@
#include <linux/random.h>
#include <trace/events/napi.h>
#include <trace/events/net.h>
+#include <trace/events/skb.h>
#include <linux/pci.h>
#include "net-sysfs.h"
@@ -1584,6 +1585,7 @@ void dev_kfree_skb_irq(struct sk_buff *skb)
struct softnet_data *sd;
unsigned long flags;
+ trace_dev_kfree_skb_irq(skb);
local_irq_save(flags);
sd = &__get_cpu_var(softnet_data);
skb->next = sd->completion_queue;
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 4e7ac09..008c019 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -466,6 +466,7 @@ void consume_skb(struct sk_buff *skb)
smp_rmb();
else if (likely(!atomic_dec_and_test(&skb->users)))
return;
+ trace_consume_skb(skb);
__kfree_skb(skb);
}
EXPORT_SYMBOL(consume_skb);
^ permalink raw reply related
* [RFC PATCH 3/5] netdev: add tracepoints to netdev layer
From: Koki Sanagi @ 2010-06-08 5:30 UTC (permalink / raw)
To: netdev; +Cc: davem, kaneshige.kenji, izumi.taku
In-Reply-To: <4C0DD43F.9090902@jp.fujitsu.com>
This patch adds tracepoint to dev_queue_xmit, dev_hard_start_xmit and
netif_receive_skb.
Signed-off-by: Koki Sanagi <sanagi.koki@jp.fujitsu.com>
---
include/trace/events/net.h | 84 ++++++++++++++++++++++++++++++++++++++++++++
net/core/dev.c | 5 +++
net/core/net-traces.c | 1 +
3 files changed, 90 insertions(+), 0 deletions(-)
diff --git a/include/trace/events/net.h b/include/trace/events/net.h
new file mode 100644
index 0000000..4f82fb5
--- /dev/null
+++ b/include/trace/events/net.h
@@ -0,0 +1,84 @@
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM net
+
+#if !defined(_TRACE_NET_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_NET_H
+
+#include <linux/skbuff.h>
+#include <linux/netdevice.h>
+#include <linux/tracepoint.h>
+
+#define NO_DEV "(no_device)"
+
+TRACE_EVENT(net_dev_xmit,
+
+ TP_PROTO(struct sk_buff *skb,
+ int rc),
+
+ TP_ARGS(skb, rc),
+
+ TP_STRUCT__entry(
+ __field( void *, skbaddr )
+ __field( unsigned int, len )
+ __field( int, rc )
+ __string( name, skb->dev->name )
+ ),
+
+ TP_fast_assign(
+ __entry->skbaddr = skb;
+ __entry->len = skb->len;
+ __entry->rc = rc;
+ __assign_str(name, skb->dev->name);
+ ),
+
+ TP_printk("dev=%s skbaddr=%p len=%u rc=%d",
+ __get_str(name), __entry->skbaddr, __entry->len, __entry->rc)
+);
+
+TRACE_EVENT(net_dev_queue,
+
+ TP_PROTO(struct sk_buff *skb),
+
+ TP_ARGS(skb),
+
+ TP_STRUCT__entry(
+ __field( void *, skbaddr )
+ __field( unsigned int, len )
+ __string( name, skb->dev->name )
+ ),
+
+ TP_fast_assign(
+ __entry->skbaddr = skb;
+ __entry->len = skb->len;
+ __assign_str(name, skb->dev->name);
+ ),
+
+ TP_printk("dev=%s skbaddr=%p len=%u",
+ __get_str(name), __entry->skbaddr, __entry->len)
+);
+
+TRACE_EVENT(net_dev_receive,
+
+ TP_PROTO(struct sk_buff *skb),
+
+ TP_ARGS(skb),
+
+ TP_STRUCT__entry(
+ __field( void *, skbaddr )
+ __field( unsigned int, len )
+ __string( name, skb->dev->name )
+ ),
+
+ TP_fast_assign(
+ __entry->skbaddr = skb;
+ __entry->len = skb->len;
+ __assign_str(name, skb->dev->name);
+ ),
+
+ TP_printk("dev=%s skbaddr=%p len=%u",
+ __get_str(name), __entry->skbaddr, __entry->len)
+);
+#endif /* _TRACE_NET_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
diff --git a/net/core/dev.c b/net/core/dev.c
index ec01a59..f7c731b 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -130,6 +130,7 @@
#include <linux/jhash.h>
#include <linux/random.h>
#include <trace/events/napi.h>
+#include <trace/events/net.h>
#include <linux/pci.h>
#include "net-sysfs.h"
@@ -1926,6 +1927,7 @@ int dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
}
rc = ops->ndo_start_xmit(skb, dev);
+ trace_net_dev_xmit(skb, rc);
if (rc == NETDEV_TX_OK)
txq_trans_update(txq);
return rc;
@@ -1946,6 +1948,7 @@ gso:
skb_dst_drop(nskb);
rc = ops->ndo_start_xmit(nskb, dev);
+ trace_net_dev_xmit(nskb, rc);
if (unlikely(rc != NETDEV_TX_OK)) {
if (rc & ~NETDEV_TX_MASK)
goto out_kfree_gso_skb;
@@ -2159,6 +2162,7 @@ int dev_queue_xmit(struct sk_buff *skb)
}
gso:
+ trace_net_dev_queue(skb);
/* Disable soft irqs for various locks below. Also
* stops preemption for RCU.
*/
@@ -2934,6 +2938,7 @@ int netif_receive_skb(struct sk_buff *skb)
if (netdev_tstamp_prequeue)
net_timestamp_check(skb);
+ trace_net_dev_receive(skb);
#ifdef CONFIG_RPS
{
struct rps_dev_flow voidflow, *rflow = &voidflow;
diff --git a/net/core/net-traces.c b/net/core/net-traces.c
index afa6380..7f1bb2a 100644
--- a/net/core/net-traces.c
+++ b/net/core/net-traces.c
@@ -26,6 +26,7 @@
#define CREATE_TRACE_POINTS
#include <trace/events/skb.h>
+#include <trace/events/net.h>
#include <trace/events/napi.h>
EXPORT_TRACEPOINT_SYMBOL_GPL(kfree_skb);
^ permalink raw reply related
* [RFC PATCH 2/5] napi: convert trace_napi_poll to TRACE_EVENT
From: Koki Sanagi @ 2010-06-08 5:28 UTC (permalink / raw)
To: netdev; +Cc: davem, kaneshige.kenji, izumi.taku, nhorman
In-Reply-To: <4C0DD43F.9090902@jp.fujitsu.com>
This patch converts trace_napi_poll from DECLARE_EVENT to TRACE_EVENT.
This is a same patch Neil Horman submitted.
http://marc.info/?l=linux-kernel&m=125978157926853&w=2
Signed-off-by: Koki Sanagi <sanagi.koki@jp.fujitsu.com>
---
include/trace/events/napi.h | 23 +++++++++++++++++++++--
1 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/include/trace/events/napi.h b/include/trace/events/napi.h
index 188deca..512a057 100644
--- a/include/trace/events/napi.h
+++ b/include/trace/events/napi.h
@@ -6,10 +6,29 @@
#include <linux/netdevice.h>
#include <linux/tracepoint.h>
+#include <linux/ftrace.h>
+
+#define NO_DEV "(no_device)"
+
+TRACE_EVENT(napi_poll,
-DECLARE_TRACE(napi_poll,
TP_PROTO(struct napi_struct *napi),
- TP_ARGS(napi));
+
+ TP_ARGS(napi),
+
+ TP_STRUCT__entry(
+ __field( struct napi_struct *, napi)
+ __string( dev_name, napi->dev ? napi->dev->name : NO_DEV)
+ ),
+
+ TP_fast_assign(
+ __entry->napi = napi;
+ __assign_str(dev_name, napi->dev ? napi->dev->name : NO_DEV);
+ ),
+
+ TP_printk("napi poll on napi struct %p for device %s",
+ __entry->napi, __get_str(dev_name))
+);
#endif /* _TRACE_NAPI_H_ */
^ permalink raw reply related
* Re: [RFC PATCH v7 01/19] Add a new structure for skb buffer from external.
From: Herbert Xu @ 2010-06-08 5:27 UTC (permalink / raw)
To: Stephen Hemminger
Cc: xiaohui.xin, netdev, kvm, linux-kernel, mst, mingo, davem, jdike
In-Reply-To: <20100606161348.427822fb@nehalam>
On Sun, Jun 06, 2010 at 04:13:48PM -0700, Stephen Hemminger wrote:
> Still not sure this is a good idea for a couple of reasons:
>
> 1. We already have lots of special cases with skb's (frags and fraglist),
> and skb's travel through a lot of different parts of the kernel. So any
> new change like this creates lots of exposed points for new bugs. Look
> at cases like MD5 TCP and netfilter, and forwarding these SKB's to ipsec
> and ppp and ...
>
> 2. SKB's can have infinite lifetime in the kernel. If these buffers come from
> a fixed size pool in an external device, they can easily all get tied up
> if you have a slow listener. What happens then?
I agree with Stephen on this.
FWIW I don't think we even need the external pages concept in
order to implement zero-copy receive (which I gather is the intent
here).
Here is one way to do it, simply construct a completely non-linear
packet in the driver, as you would if you were using the GRO frags
interface (grep for napi_gro_frags under drivers/net for examples).
This way you can transfer the entire contents of the packet without
copying through to the other side, provided that the host stack does
not modify the packet.
If the host side did modify the packet then we have to incur the
memory cost anyway.
IOW I think the only feature provided by the external pages
construct is allowing the skb->head area to be shared without
copying. I'm claiming that this can be done by simply making
skb->head empty.
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
* [RFC PATCH 1/5] irq: add tracepoint to softirq_raise
From: Koki Sanagi @ 2010-06-08 5:27 UTC (permalink / raw)
To: netdev; +Cc: davem, kaneshige.kenji, izumi.taku, laijs
In-Reply-To: <4C0DD43F.9090902@jp.fujitsu.com>
This patch adds tracepoint to softirq_raise.
This is a same patch Lai Jiangshan submitted.
http://marc.info/?l=linux-kernel&m=126026122728732&w=2
Signed-off-by: Koki Sanagi <sanagi.koki@jp.fujitsu.com>
---
include/linux/interrupt.h | 8 +++++++-
include/trace/events/irq.h | 34 +++++++++++++++++++++++++++++++---
2 files changed, 38 insertions(+), 4 deletions(-)
diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index c233113..1cb5726 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -18,6 +18,7 @@
#include <asm/atomic.h>
#include <asm/ptrace.h>
#include <asm/system.h>
+#include <trace/events/irq.h>
/*
* These correspond to the IORESOURCE_IRQ_* defines in
@@ -402,7 +403,12 @@ asmlinkage void do_softirq(void);
asmlinkage void __do_softirq(void);
extern void open_softirq(int nr, void (*action)(struct softirq_action *));
extern void softirq_init(void);
-#define __raise_softirq_irqoff(nr) do { or_softirq_pending(1UL << (nr)); } while (0)
+static inline void __raise_softirq_irqoff(unsigned int nr)
+{
+ trace_softirq_raise(nr);
+ or_softirq_pending(1UL << nr);
+}
+
extern void raise_softirq_irqoff(unsigned int nr);
extern void raise_softirq(unsigned int nr);
extern void wakeup_softirqd(void);
diff --git a/include/trace/events/irq.h b/include/trace/events/irq.h
index 0e4cfb6..7cb7435 100644
--- a/include/trace/events/irq.h
+++ b/include/trace/events/irq.h
@@ -5,7 +5,9 @@
#define _TRACE_IRQ_H
#include <linux/tracepoint.h>
-#include <linux/interrupt.h>
+
+struct irqaction;
+struct softirq_action;
#define softirq_name(sirq) { sirq##_SOFTIRQ, #sirq }
#define show_softirq_name(val) \
@@ -82,6 +84,32 @@ TRACE_EVENT(irq_handler_exit,
__entry->irq, __entry->ret ? "handled" : "unhandled")
);
+/**
+ * softirq_raise - called immediately when a softirq is raised
+ * @nr: softirq vector number
+ *
+ * Tracepoint for tracing when softirq action is raised.
+ * Also, when used in combination with the softirq_entry tracepoint
+ * we can determine the softirq raise latency.
+ */
+TRACE_EVENT(softirq_raise,
+
+ TP_PROTO(unsigned int nr),
+
+ TP_ARGS(nr),
+
+ TP_STRUCT__entry(
+ __field( unsigned int, vec )
+ ),
+
+ TP_fast_assign(
+ __entry->vec = nr;
+ ),
+
+ TP_printk("vec=%d [action=%s]", __entry->vec,
+ show_softirq_name(__entry->vec))
+);
+
DECLARE_EVENT_CLASS(softirq,
TP_PROTO(struct softirq_action *h, struct softirq_action *vec),
@@ -89,11 +117,11 @@ DECLARE_EVENT_CLASS(softirq,
TP_ARGS(h, vec),
TP_STRUCT__entry(
- __field( int, vec )
+ __field( unsigned int, vec )
),
TP_fast_assign(
- __entry->vec = (int)(h - vec);
+ __entry->vec = (unsigned int)(h - vec);
),
TP_printk("vec=%d [action=%s]", __entry->vec,
^ permalink raw reply related
* [RFC PATCH 0/5] netdev: show a process of packets
From: Koki Sanagi @ 2010-06-08 5:25 UTC (permalink / raw)
To: netdev; +Cc: davem, kaneshige.kenji, izumi.taku
These patch-set adds tracepoints to show us a process of packets.
Using these tracepoints and existing points, we can get the time when
packet passes through some points in transmit or receive sequence.
For example, this is an output of perf script which is attached by patch 5/5.
79074.756672832sec cpu=1
irq_entry(+0.000000msec,irq=77:eth3)
|------------softirq_raise(+0.001277msec)
irq_exit (+0.002278msec) |
|
softirq_entry(+0.003562msec)
|
|---netif_receive_skb(+0.006279msec,len=100)
| |
| skb_copy_datagram_iovec(+0.038778msec, 2285:sshd)
|
napi_poll_exit(+0.017160msec, eth3)
|
softirq_exit(+0.018248msec)
The above is a receive side. Like this, it can show receive sequence from
interrupt(irq_entry) to application(skb_copy_datagram_iovec). There are 8
tracepoints in this side. All events except for skb_copy_datagram_iovec can be
associated with each other by CPU number. skb_copy_datagram_iovec can be
associated with netif_receive_skb by skbaddr.
This script shows one NET_RX softirq and events related to it. All relative
time bases on first irq_entry which raise NET_RX softirq.
dev len dev_queue_xmit|----------|dev_hard_start_xmit|-----|free_skb
| | |
eth3 114 79044.417123332sec 0.005242msec 0.103843msec
eth3 114 79044.580090422sec 0.002306msec 0.103632msec
eth3 114 79044.719078251sec 0.002288msec 0.104093msec
The above is a transmit side. There are three tracepoints in this side.
Point1 is before putting a packet to Qdisc. point2 is after ndo_start_xmit in
dev_hard_start_xmit. It indicates finishing putting a packet to driver.
point3 is in consume_skb and dev_kfree_skb_irq. It indicates freeing a
transmitted packet.
Values of this script are, from left, device name, length of a packet, a time of
point1, an interval time between point1 and point2 and an interval time between
point2 and point3.
These times are useful to analyze a performance or to detect a point where
packet delays. For example,
- NET_RX softirq calling is late.
- Application is late to take a packet.
- It takes much time to put a transmitting packet to driver
(It may be caused by packed queue)
And also, these tracepoint help us to investigate a network driver's trouble
from memory dump because ftrace records it to memory. And ftrace is so light
even if always trace on. So, in a case investigating a problem which doesn't
reproduce, it is useful.
Thanks,
Koki Sanagi.
^ 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