* Re: [net-next-2.6 PATCH] ipoib: remove addrlen check for mc addresses
From: Moni Shoua @ 2010-03-23 11:45 UTC (permalink / raw)
To: Eli Cohen; +Cc: Or Gerlitz, Jiri Pirko, netdev, davem, linux-rdma, jgunthorpe
In-Reply-To: <20100323113436.GD12224@mtldesk030.lab.mtl.com>
Eli Cohen wrote:
> On Tue, Mar 23, 2010 at 12:34:13PM +0200, Or Gerlitz wrote:
>> basically, as the subject line suggests, it should be in Dave's net-next tree
>>
> I just need to clone this tree and need the url. Can you give it to me
> from .git/config?
> Thanks.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
maybe your'e looking for this one
http://git.kernel.org/?p=linux/kernel/git/davem/net-next-2.6.git;a=commit;h=32a806c194ea112cfab00f558482dd97bee5e44e
^ permalink raw reply
* Re: [PATCH] rps: make distributing packets fairly among all the online CPUs default
From: Eric Dumazet @ 2010-03-23 11:38 UTC (permalink / raw)
To: Changli Gao; +Cc: David S. Miller, Tom Herbert, netdev, Ben Hutchings
In-Reply-To: <412e6f7f1003230303l69fc142ey1581fff4c3e749be@mail.gmail.com>
Le mardi 23 mars 2010 à 18:03 +0800, Changli Gao a écrit :
>
> Ben Hutchings raised this question, but nobody replied with any message.
>
> I do think distributing packets fairly among all the online CPUs will
> helps most of users. I remember the smp_affinity of IRQ is the
> online_cpu_mask be default.
>
I know _many_ applications that perform better when all network IRQS are
directed to one CPU only. Single threaded UDP server for example, with
fast answer, or routers, or ...
Many sysadmins tuned their machine to meet this requirement. Installing
2.6.35, if RPS enabled by default will make them unhappy, especially if
CONFIG_SYSFS is not set, since they wont be able to change RPS settings.
If RPS was good for all workloads, activating it would make sense, but
this is not the case.
^ permalink raw reply
* Re: [net-next-2.6 PATCH] ipoib: remove addrlen check for mc addresses
From: Eli Cohen @ 2010-03-23 11:34 UTC (permalink / raw)
To: Or Gerlitz; +Cc: Jiri Pirko, netdev, davem, linux-rdma, monis, jgunthorpe
In-Reply-To: <4BA89925.3050602@Voltaire.com>
On Tue, Mar 23, 2010 at 12:34:13PM +0200, Or Gerlitz wrote:
>
> basically, as the subject line suggests, it should be in Dave's net-next tree
>
I just need to clone this tree and need the url. Can you give it to me
from .git/config?
Thanks.
^ permalink raw reply
* Re: [PATCH] rps: make distributing packets fairly among all the online CPUs default
From: Changli Gao @ 2010-03-23 11:27 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S. Miller, Tom Herbert, netdev
In-Reply-To: <1269343422.2983.1.camel@edumazet-laptop>
On Tue, Mar 23, 2010 at 7:23 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le mardi 23 mars 2010 à 17:56 +0800, Changli Gao a écrit :
>> On Tue, Mar 23, 2010 at 3:20 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>> > Le mardi 23 mars 2010 à 14:24 +0800, Changli Gao a écrit :
>> >
>> > You cannot do this like this, these allocations wont be freed.
>> >
>> > Better would be to take a look at net/core/net-sysfs.c
>> >
>>
>> Sorry, but I don't know why these allocations won't be freed.
>>
>>
>
> Hint : This will be freed only if CONFIG_SYSFS is set
>
got it, thanks. But I just wonder if it is right the life cycle of
rps_map is maintained by sysfs rather than netdev itself.
--
Regards，
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [PATCH] rps: make distributing packets fairly among all the online CPUs default
From: Eric Dumazet @ 2010-03-23 11:23 UTC (permalink / raw)
To: Changli Gao; +Cc: David S. Miller, Tom Herbert, netdev
In-Reply-To: <412e6f7f1003230256m109644b0n2988e86a56f1ae1@mail.gmail.com>
Le mardi 23 mars 2010 à 17:56 +0800, Changli Gao a écrit :
> On Tue, Mar 23, 2010 at 3:20 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > Le mardi 23 mars 2010 à 14:24 +0800, Changli Gao a écrit :
> >
> > You cannot do this like this, these allocations wont be freed.
> >
> > Better would be to take a look at net/core/net-sysfs.c
> >
>
> Sorry, but I don't know why these allocations won't be freed.
>
>
Hint : This will be freed only if CONFIG_SYSFS is set
^ permalink raw reply
* Re: [net-next-2.6 PATCH] ipoib: remove addrlen check for mc addresses
From: Moni Shoua @ 2010-03-23 10:46 UTC (permalink / raw)
To: Eli Cohen
Cc: Jiri Pirko, netdev-u79uwXL29TY76Z2rM5mHXA,
davem-fT/PcQaiUtIeIZ0/mPfg9Q, ogerlitz-smomgflXvOZWk0Htik3J/w,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/
In-Reply-To: <20100323103034.GC12224-8YAHvHwT2UEvbXDkjdHOrw/a8Rv0c6iv@public.gmane.org>
> Could you send a link to the git tree where I can find this commit and
> the related fixes?
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
See commit 5e47596bee12597824a3b5b21e20f80b61e58a35 for the fix prior to this one.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [net-next-2.6 PATCH] ipoib: remove addrlen check for mc addresses
From: Or Gerlitz @ 2010-03-23 10:34 UTC (permalink / raw)
To: Eli Cohen
Cc: Jiri Pirko, netdev-u79uwXL29TY76Z2rM5mHXA,
davem-fT/PcQaiUtIeIZ0/mPfg9Q, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
monis-smomgflXvOZWk0Htik3J/w,
jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/
In-Reply-To: <20100323103034.GC12224-8YAHvHwT2UEvbXDkjdHOrw/a8Rv0c6iv@public.gmane.org>
Eli Cohen wrote:
> Could you send a link to the git tree where I can find this commit and
> the related fixes?
basically, as the subject line suggests, it should be in Dave's net-next tree
Or.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [net-next-2.6 PATCH] ipoib: remove addrlen check for mc addresses
From: Eli Cohen @ 2010-03-23 10:30 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
ogerlitz-smomgflXvOZWk0Htik3J/w,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, monis-smomgflXvOZWk0Htik3J/w,
jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/
In-Reply-To: <20100322132138.GC2780-YzwxZg+R7evMbnheQZGK0N5OCZ2W11yPFxja6HXR22MAvxtiuMwx3w@public.gmane.org>
On Mon, Mar 22, 2010 at 02:21:39PM +0100, Jiri Pirko wrote:
> Finally this bit can be removed. Currently, after the bonding driver is
> changed/fixed (32a806c194ea112cfab00f558482dd97bee5e44e net-next-2.6),
Could you send a link to the git tree where I can find this commit and
the related fixes?
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 00/12] l2tp: Introduce L2TPv3 support
From: James Chapman @ 2010-03-23 10:19 UTC (permalink / raw)
To: davem; +Cc: netdev
David Miller wrote:
> > Doesn't even build:
> >
> > net/l2tp/l2tp_ip.c:600: error: ‘compat_udp_setsockopt’ undeclared here (not in a function)
> > net/l2tp/l2tp_ip.c:601: error: ‘compat_udp_getsockopt’ undeclared here (not in a function)
> >
> > You can't do this like this, as the header providing these
> > declarations is private to net/ipv4/
Oops, no-one built this with CONFIG_COMPAT when testing. Will fix.
> > Respin your entire patch set once you've resolved this and
> > also please fix these fundamental whitespace errors emitted
> > by git when I apply your patches:
scripts/checkpatch.pl didn't pick these up. Is there a recommended way
to find these in the future? I'm using stg to manage these patches.
> > Applying: l2tp: Relocate pppol2tp driver to new net/l2tp directory
> > Applying: l2tp: Split pppol2tp driver into separate l2tp and ppp parts
> > /home/davem/src/GIT/net-next-2.6/.git/rebase-apply/patch:118: new blank line at EOF.
> > +
> > /home/davem/src/GIT/net-next-2.6/.git/rebase-apply/patch:133: new blank line at EOF.
> > +
> > warning: 2 lines add whitespace errors.
> > Applying: ppp: Add ppp_dev_name() exported function
> > Applying: l2tp: Add ppp device name to L2TP ppp session data
> > Applying: l2tp: Add L2TPv3 protocol support
> > Applying: l2tp: Update PPP-over-L2TP driver to work over L2TPv3
> > Applying: l2tp: Add L2TPv3 IP encapsulation (no UDP) support
> > /home/davem/src/GIT/net-next-2.6/.git/rebase-apply/patch:61: new blank line at EOF.
> > +
> > warning: 1 line adds whitespace errors.
> > Applying: netlink: Export genl_lock() API for use by modules
> > Applying: l2tp: Add netlink control API for L2TP
> > Applying: l2tp: Add L2TP ethernet pseudowire support
> > /home/davem/src/GIT/net-next-2.6/.git/rebase-apply/patch:25: space before tab in indent.
> > used as a control protocol and for data encapsulation to set
> > /home/davem/src/GIT/net-next-2.6/.git/rebase-apply/patch:26: space before tab in indent.
> > up Pseudowires for transporting layer 2 Packet Data Units
> > /home/davem/src/GIT/net-next-2.6/.git/rebase-apply/patch:27: space before tab in indent.
> > across an IP network [RFC3931].
> > /home/davem/src/GIT/net-next-2.6/.git/rebase-apply/patch:12: new blank line at EOF.
> > +
> > warning: 4 lines add whitespace errors.
> > Applying: l2tp: Add support for static unmanaged L2TPv3 tunnels
> > Applying: l2tp: Update documentation
> >
> > Thanks.
I'm out the office this week. I'll fix the above (unless others beat me
to it) and resend as soon as I'm back.
--
James Chapman
Katalix Systems Ltd
http://www.katalix.com
Catalysts for your Embedded Linux software development
^ permalink raw reply
* Re: [PATCH] xfrm: cache bundle lookup results in flow cache
From: Herbert Xu @ 2010-03-23 9:41 UTC (permalink / raw)
To: Timo Teräs; +Cc: netdev, David S. Miller
In-Reply-To: <4BA88789.7060104@iki.fi>
On Tue, Mar 23, 2010 at 11:19:05AM +0200, Timo Teräs wrote:
>
> Normally, only the getter is called per-packet. So I'm thinking
> to have get() that would check for entry being stale or not,
> and bump up the refcount.
OK if it's just one call per packet it sounds good to me.
Thanks,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH] rps: make distributing packets fairly among all the online CPUs default
From: Changli Gao @ 2010-03-23 10:03 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S. Miller, Tom Herbert, netdev, Ben Hutchings
In-Reply-To: <1269329250.3043.19.camel@edumazet-laptop>
On Tue, Mar 23, 2010 at 3:27 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le mardi 23 mars 2010 à 14:24 +0800, Changli Gao a écrit :
>> make distributing packets fairly among all the online CPUs default.
>>
>> Make distributing packets fairly among all the online CPUs default, then
>> users don't need any explicit configuration to get the benefit of RPS.
>>
>
> Some people want to dedicate a cpu to network interrupts, cache hits
> only, to get low latencies.
> RPS is not a win for all workloads.
>
> I think we should not change default without a knob.
>
> Didnt we discussed all this during last months ?
>
Ben Hutchings raised this question, but nobody replied with any message.
I do think distributing packets fairly among all the online CPUs will
helps most of users. I remember the smp_affinity of IRQ is the
online_cpu_mask be default.
--
Regards，
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [PATCH] rps: make distributing packets fairly among all the online CPUs default
From: Changli Gao @ 2010-03-23 9:56 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S. Miller, Tom Herbert, netdev
In-Reply-To: <1269328852.3043.13.camel@edumazet-laptop>
On Tue, Mar 23, 2010 at 3:20 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le mardi 23 mars 2010 à 14:24 +0800, Changli Gao a écrit :
>
> You cannot do this like this, these allocations wont be freed.
>
> Better would be to take a look at net/core/net-sysfs.c
>
Sorry, but I don't know why these allocations won't be freed.
--
Regards，
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [PATCH] xfrm: cache bundle lookup results in flow cache
From: Timo Teräs @ 2010-03-23 9:19 UTC (permalink / raw)
To: Herbert Xu; +Cc: netdev, David S. Miller
In-Reply-To: <20100323074252.GA27623@gondor.apana.org.au>
Herbert Xu wrote:
> On Tue, Mar 23, 2010 at 09:28:33AM +0200, Timo Teräs wrote:
>> So, should I go ahead and do the virtualization of the
>> getters and putters?
>
> If we're going to have indirect calls per packet then let's at
> least minimise them. I think one should be enough, i.e., just
> add a ->stale() function pointer to be used in flow_cache_lookup.
Normally, only the getter is called per-packet. So I'm thinking
to have get() that would check for entry being stale or not,
and bump up the refcount.
The resolver can just swap the old entry and call appropriate
_put/_get, so we can avoid virtual calls there.
Thinking more about the flushing of the flow cache. It's basically
only needed to flush before the policies are garbage collected.
This is strictly needed so we can do the atomic_dec() without
turning policy's refcount to zero and causing a leak.
I'm now thinking if it'd be worth doing virtual _put(). This
way we would not need flushing at all for in policy garbage
collector (we could kill flow_cache_flush). We could also
call dst_release immediately in the flow cache _put, which
would release the memory faster. This way we would not need
at all any periodic xfrm_dst checker as flow cache is
regenerated regularly.
The code paths that would require calling the virtual put
are:
- randomization of flow cache (every 10 mins currently)
from flow_cache_lookup() with bh disabled
- flow cache going too full, from flow_cache_lookup()
with bh disabled
- cpu notifier
Do you think the virtual _put doing more work would be
too slow?
In that case the plain atomic_dec sounds ok, but we'd then
need to periodically check through the global bundle list
for garbage collection.
Cheers,
Timo
^ permalink raw reply
* RE,
From: FROM CENTRAL BANK @ 2010-03-23 7:53 UTC (permalink / raw)
Very Urgently,
We Conclude Our Meeting Today That $10.7m should be pay to you
as your contract entitlement. The Payment Will Come To You Via Diplomatic
Carrier Service:Re- Comfirm this informations as follows.
Your Full Name,
Home
Address,
Direct Phone No,
Occupation And Age.
Dr. Sanusi A. Lamido
Tel:+234
8067884885
^ permalink raw reply
* Re: [PATCH] xfrm: cache bundle lookup results in flow cache
From: Herbert Xu @ 2010-03-23 7:42 UTC (permalink / raw)
To: Timo Teräs; +Cc: netdev, David S. Miller
In-Reply-To: <4BA86DA1.7090707@iki.fi>
On Tue, Mar 23, 2010 at 09:28:33AM +0200, Timo Teräs wrote:
>
> So, should I go ahead and do the virtualization of the
> getters and putters?
If we're going to have indirect calls per packet then let's at
least minimise them. I think one should be enough, i.e., just
add a ->stale() function pointer to be used in flow_cache_lookup.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH] xfrm: cache bundle lookup results in flow cache
From: Timo Teräs @ 2010-03-23 7:28 UTC (permalink / raw)
To: Herbert Xu; +Cc: netdev, David S. Miller
In-Reply-To: <4BA7B10A.1030302@iki.fi>
Timo Teräs wrote:
> Herbert Xu wrote:
>> On Sun, Mar 21, 2010 at 10:31:23AM +0200, Timo Teräs wrote:
>>>> Ok, we can do that to skip 2. But I think 1 would be still useful.
>>>> It'd probably be good to actually have flow_cache_ops pointer in
>>>> each entry instead of the atomic_t pointer.
>>>>
>>>> The reasoning:
>>>> - we can then have type-based checks that the reference count
>>>> is valid (e.g. policy's refcount must not go to zero, it's bug,
>>>> and we can call dst_release which warns if refcount goes to
>>>> negative); imho it's hack to call atomic_dec instead of the
>>>> real type's xxx_put
>>>> - the flow cache needs to somehow know if the entry is stale so
>>>> it'll try to refresh it atomically; e.g. if there's no
>>>> check for 'stale', the lookup returns stale xfrm_dst. we'd
>>>> then need new api to update the stale entry, or flush it out
>>>> and repeat the lookup. the virtual get could check for it being
>>>> stale (if so release the entry) and then return null for the
>>>> generic code to call the resolver atomically
>>>> - for paranoia we can actually check the type of the object in
>>>> cache via the ops (if needed)
>>
>> The reason I'd prefer to keep the current scheme is to avoid
>> an additional indirect function call on each packet.
>>
>> The way it would work is (we need flow_cache_lookup to return
>> fle instead of the object):
>>
>> fle = flow_cache_lookup
>> xdst = fle->object
>> if (xdst is stale) {
>> flow_cache_mark_obsolete(fle)
>> fle = flow_cache_lookup
>> xdst = fle->object
>> if (xdst is stale)
>> return error
>> }
I've been thinking more about doing this. And I think this
approach is fundamentally racy.
Underlying fle->object can be changed underneath as since
bh are not disabled. This means the refcounting needs to
be done on fle level, and additional measures are needed
to ensure that fle->object is as expected.
Additionally, the second flow_cache_lookup can happen
on another cpu, and could return a stale object that
indeed would need refreshing instead of generating an
error.
Options:
1. return fle and fle->object separately, refcount both
2. call flow_cache_lookup with bh disabled
3. use flow_cache_entry_ops and virtualize put/get
Since 2. was previously said to leak generic code,
it does not sound good.
On 1 vs 3, I'd still choose 3 since:
- adding one more refcount means more atomic calls
which have same (or more) overhead as indirect call
- two refcounts sounds more complicated than one
- exposing fle and touching it without bh's disabled
requires a whole lot of more extra care; doing
3 is simpler
So, should I go ahead and do the virtualization of the
getters and putters?
- Timo
^ permalink raw reply
* Re: [PATCH] rps: make distributing packets fairly among all the online CPUs default
From: Eric Dumazet @ 2010-03-23 7:27 UTC (permalink / raw)
To: xiaosuo; +Cc: David S. Miller, Tom Herbert, netdev
In-Reply-To: <4BA85EA2.1090304@gmail.com>
Le mardi 23 mars 2010 à 14:24 +0800, Changli Gao a écrit :
> make distributing packets fairly among all the online CPUs default.
>
> Make distributing packets fairly among all the online CPUs default, then
> users don't need any explicit configuration to get the benefit of RPS.
>
Some people want to dedicate a cpu to network interrupts, cache hits
only, to get low latencies.
RPS is not a win for all workloads.
I think we should not change default without a knob.
Didnt we discussed all this during last months ?
^ permalink raw reply
* Re: [PATCH] rps: make distributing packets fairly among all the online CPUs default
From: Eric Dumazet @ 2010-03-23 7:20 UTC (permalink / raw)
To: xiaosuo; +Cc: David S. Miller, Tom Herbert, netdev
In-Reply-To: <4BA85EA2.1090304@gmail.com>
Le mardi 23 mars 2010 à 14:24 +0800, Changli Gao a écrit :
> make distributing packets fairly among all the online CPUs default.
>
> Make distributing packets fairly among all the online CPUs default, then
> users don't need any explicit configuration to get the benefit of RPS.
>
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
> ----
> net/core/dev.c | 34 ++++++++++++++++++++++++++++++++--
> 1 file changed, 32 insertions(+), 2 deletions(-)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index c0e2608..a4246f1 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -5234,6 +5234,24 @@ void netif_stacked_transfer_operstate(const struct net_device *rootdev,
> }
> EXPORT_SYMBOL(netif_stacked_transfer_operstate);
>
> +static struct rps_map* alloc_rps_map(void)
> +{
> + struct rps_map *map;
> + int i, cpu;
> +
> + map = kzalloc(max_t(unsigned,
> + RPS_MAP_SIZE(cpumask_weight(cpu_online_mask)),
> + L1_CACHE_BYTES), GFP_KERNEL);
> + if (map == NULL)
> + return NULL;
> + i = 0;
> + for_each_online_cpu(cpu)
> + map->cpus[i++] = cpu;
> + map->len = i;
> +
> + return map;
> +}
> +
> /**
> * register_netdevice - register a network device
> * @dev: device to register
> @@ -5282,7 +5300,13 @@ int register_netdevice(struct net_device *dev)
> ret = -ENOMEM;
> goto out;
> }
> -
> + dev->_rx->rps_map = alloc_rps_map();
> + if (dev->_rx->rps_map == NULL) {
> + kfree(dev->_rx);
> + dev->_rx = NULL;
> + ret = -ENOMEM;
> + goto out;
> + }
> dev->_rx->first = dev->_rx;
> atomic_set(&dev->_rx->count, 1);
> dev->num_rx_queues = 1;
> @@ -5688,8 +5712,12 @@ struct net_device *alloc_netdev_mq(int sizeof_priv, const char *name,
> * Set a pointer to first element in the array which holds the
> * reference count.
> */
> - for (i = 0; i < queue_count; i++)
> + for (i = 0; i < queue_count; i++) {
> rx[i].first = rx;
> + rx[i].rps_map = alloc_rps_map();
> + if (rx[i].rps_map == NULL)
> + goto free_rx;
> + }
>
> dev = PTR_ALIGN(p, NETDEV_ALIGN);
> dev->padded = (char *)dev - (char *)p;
> @@ -5723,6 +5751,8 @@ struct net_device *alloc_netdev_mq(int sizeof_priv, const char *name,
> return dev;
>
> free_rx:
> + for (i = 0; i < queue_count; i++)
> + kfree(rx[i].rps_map);
> kfree(rx);
> free_tx:
> kfree(tx);
>
>
> --
You cannot do this like this, these allocations wont be freed.
Better would be to take a look at net/core/net-sysfs.c
^ permalink raw reply
* Re: [PATCH] rps: memory leak due to forget to release rx queues
From: Changli Gao @ 2010-03-23 7:02 UTC (permalink / raw)
To: Tom Herbert; +Cc: David S. Miller, netdev
In-Reply-To: <65634d661003222344s662000e8ode1b4f49ff73b345@mail.gmail.com>
On Tue, Mar 23, 2010 at 2:44 PM, Tom Herbert <therbert@google.com> wrote:
> 2010/3/22 Changli Gao <xiaosuo@gmail.com>:
>> memory leak due to forget to release rx queues.
>>
>> rx queues is allocated in alloc_netdev_mq(), but they aren't released in
>> free_netdev(), and the field rps_map is also miss.
>>
Thanks for explaining.
--
Regards，
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: tc download always to "default" [solved]
From: Mihamina Rakotomandimby @ 2010-03-23 7:02 UTC (permalink / raw)
To: netdev
In-Reply-To: <20100322143113.GA23778@wolff.to>
> Bruno Wolff III <bruno@wolff.to> :
>> My problem is all the traffic is caught by "classid 1:10", the
>> default class.
>> Nothing is trapped by "classid 1:301" -> "classid 1:426"
>> At a first glance, would you see something wrong in these?
>I took a quick look and didn't see anything obvious. But I haven't
>played with this stuff for a while and my scripts were set up a bit
>differently.
>How are you checking that everything is ending up in the default class?
1°) As I wrote my rules, each IP address has one handler.
I use this script to display UP and DOWN traffic for the handler in $1:
#!/bin/sh
INIF=eth1
OUTIF=ifb0
RATEIN=$(tc -s -d class show dev $INIF \
| egrep -A 5 "class htb 1:$1 " \
| egrep "rate .+ backlog" \
| awk '{print $2}')
RATEOUT=$(tc -s -d class show dev $OUTIF \
| egrep -A 5 "class htb 1:$1 " \
| egrep "rate .+ backlog" \
| awk '{print $2}')
echo $RATEIN
echo $RATEOUT
I launch it like this:
# get-bw.sh $NNN
Where $NNN is the handler.
When I launch it, I see "0bit" for IN traffic, that means nothing is
trapped.
When I launch it with the handler of the default, I see much traffic.
2°) When varying the "default" allocated bandwidth, I see in my charts
it is the one seen.
>I remember there is a command that tells you how much stuff is ending
> up getting handled by the different qdiscs. Is that what you are doing
> or are you just assuming that's what's happening because traffic isn't
> being limited the way you expect?
Anyway, I solved my problem:
The LAN interface is "eth1" but there is a tunnel "tun0" coupled to it
(I use coova chilli access point).
Just to test, I switched to "tun0" and it suddenly worked.
In all the rules I gave, just replacing "eth1" with "tun0" made it!
(What a dumb I am...)
Thanks for everything.
--
Architecte Informatique chez Blueline/Gulfsat:
Administration Systeme, Recherche & Developpement
+261 34 29 155 34 / +261 33 11 207 36
^ permalink raw reply
* Re: [PATCH] rps: memory leak due to forget to release rx queues
From: Tom Herbert @ 2010-03-23 6:44 UTC (permalink / raw)
To: xiaosuo; +Cc: David S. Miller, netdev
In-Reply-To: <4BA85A33.6050004@gmail.com>
2010/3/22 Changli Gao <xiaosuo@gmail.com>:
> memory leak due to forget to release rx queues.
>
> rx queues is allocated in alloc_netdev_mq(), but they aren't released in
> free_netdev(), and the field rps_map is also miss.
>
rx_queues can't be free from free_netdev since they contain kobjects
(only freed when refcnt goes to zero). They should be freed when the
device kobject is freed (netdev_unregister_kobject).
Tom
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
> ----
> net/core/dev.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index c0e2608..51b86e0 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -5746,6 +5746,14 @@ void free_netdev(struct net_device *dev)
>
> release_net(dev_net(dev));
>
> + if (dev->num_rx_queues) {
> + int i;
> +
> + for (i = 0; i < dev->num_rx_queues; i++)
> + kfree(dev->_rx[i].rps_map);
> + kfree(dev->_rx);
> + }
> +
> kfree(dev->_tx);
>
> /* Flush device addresses */
>
>
>
^ permalink raw reply
* [PATCH net-next-2.6 2/2] be2net: fix unmap_single/page() called incorrectly in Tx compl processing
From: Sathya Perla @ 2010-03-23 6:41 UTC (permalink / raw)
To: netdev
The first wrb seen by tx compl processing does not have a dma handle in it.
Currently, pci_unmap_single() is attempted on this wrb and pci_unmap_page() on the
rest. So, pci_unmap_page() gets incorrectly called on the dma hdl of skb->data (that
was mapped using map_single()). This patch fixes this issue.
Signed-off-by: Sathya Perla <sathyap@serverengines.com>
---
drivers/net/benet/be_main.c | 33 ++++++++++++---------------------
1 files changed, 12 insertions(+), 21 deletions(-)
diff --git a/drivers/net/benet/be_main.c b/drivers/net/benet/be_main.c
index 0800c63..174e5f8 100644
--- a/drivers/net/benet/be_main.c
+++ b/drivers/net/benet/be_main.c
@@ -1047,35 +1047,26 @@ static void be_tx_compl_process(struct be_adapter *adapter, u16 last_index)
struct be_eth_wrb *wrb;
struct sk_buff **sent_skbs = adapter->tx_obj.sent_skb_list;
struct sk_buff *sent_skb;
- u64 busaddr;
- u16 cur_index, num_wrbs = 0;
+ u16 cur_index, num_wrbs = 1; /* account for hdr wrb */
+ bool unmap_skb_hdr = true;
- cur_index = txq->tail;
- sent_skb = sent_skbs[cur_index];
+ sent_skb = sent_skbs[txq->tail];
BUG_ON(!sent_skb);
- sent_skbs[cur_index] = NULL;
- wrb = queue_tail_node(txq);
- be_dws_le_to_cpu(wrb, sizeof(*wrb));
- busaddr = ((u64)wrb->frag_pa_hi << 32) | (u64)wrb->frag_pa_lo;
- if (busaddr != 0) {
- pci_unmap_single(adapter->pdev, busaddr,
- wrb->frag_len, PCI_DMA_TODEVICE);
- }
- num_wrbs++;
+ sent_skbs[txq->tail] = NULL;
+
+ /* skip header wrb */
queue_tail_inc(txq);
- while (cur_index != last_index) {
+ do {
cur_index = txq->tail;
wrb = queue_tail_node(txq);
- be_dws_le_to_cpu(wrb, sizeof(*wrb));
- busaddr = ((u64)wrb->frag_pa_hi << 32) | (u64)wrb->frag_pa_lo;
- if (busaddr != 0) {
- pci_unmap_page(adapter->pdev, busaddr,
- wrb->frag_len, PCI_DMA_TODEVICE);
- }
+ unmap_tx_frag(adapter->pdev, wrb, (unmap_skb_hdr &&
+ sent_skb->len > sent_skb->data_len));
+ unmap_skb_hdr = false;
+
num_wrbs++;
queue_tail_inc(txq);
- }
+ } while (cur_index != last_index);
atomic_sub(num_wrbs, &txq->used);
--
1.6.3.3
^ permalink raw reply related
* [PATCH net-next-2.6 1/2] be2net: handle dma mapping errors in Tx path
From: Sathya Perla @ 2010-03-23 6:41 UTC (permalink / raw)
To: netdev
Signed-off-by: Sathya Perla <sathyap@serverengines.com>
---
drivers/net/benet/be_main.c | 41 ++++++++++++++++++++++++++++++++++++++---
1 files changed, 38 insertions(+), 3 deletions(-)
diff --git a/drivers/net/benet/be_main.c b/drivers/net/benet/be_main.c
index 43e8032..0800c63 100644
--- a/drivers/net/benet/be_main.c
+++ b/drivers/net/benet/be_main.c
@@ -386,26 +386,48 @@ static void wrb_fill_hdr(struct be_eth_hdr_wrb *hdr, struct sk_buff *skb,
AMAP_SET_BITS(struct amap_eth_hdr_wrb, len, hdr, len);
}
+static void unmap_tx_frag(struct pci_dev *pdev, struct be_eth_wrb *wrb,
+ bool unmap_single)
+{
+ dma_addr_t dma;
+
+ be_dws_le_to_cpu(wrb, sizeof(*wrb));
+
+ dma = (u64)wrb->frag_pa_hi << 32 | (u64)wrb->frag_pa_lo;
+ if (dma != 0) {
+ if (unmap_single)
+ pci_unmap_single(pdev, dma, wrb->frag_len,
+ PCI_DMA_TODEVICE);
+ else
+ pci_unmap_page(pdev, dma, wrb->frag_len,
+ PCI_DMA_TODEVICE);
+ }
+}
static int make_tx_wrbs(struct be_adapter *adapter,
struct sk_buff *skb, u32 wrb_cnt, bool dummy_wrb)
{
- u64 busaddr;
- u32 i, copied = 0;
+ dma_addr_t busaddr;
+ int i, copied = 0;
struct pci_dev *pdev = adapter->pdev;
struct sk_buff *first_skb = skb;
struct be_queue_info *txq = &adapter->tx_obj.q;
struct be_eth_wrb *wrb;
struct be_eth_hdr_wrb *hdr;
+ bool map_single = false;
+ u16 map_head;
hdr = queue_head_node(txq);
- atomic_add(wrb_cnt, &txq->used);
queue_head_inc(txq);
+ map_head = txq->head;
if (skb->len > skb->data_len) {
int len = skb->len - skb->data_len;
busaddr = pci_map_single(pdev, skb->data, len,
PCI_DMA_TODEVICE);
+ if (pci_dma_mapping_error(pdev, busaddr))
+ goto dma_err;
+ map_single = true;
wrb = queue_head_node(txq);
wrb_fill(wrb, busaddr, len);
be_dws_cpu_to_le(wrb, sizeof(*wrb));
@@ -419,6 +441,8 @@ static int make_tx_wrbs(struct be_adapter *adapter,
busaddr = pci_map_page(pdev, frag->page,
frag->page_offset,
frag->size, PCI_DMA_TODEVICE);
+ if (pci_dma_mapping_error(pdev, busaddr))
+ goto dma_err;
wrb = queue_head_node(txq);
wrb_fill(wrb, busaddr, frag->size);
be_dws_cpu_to_le(wrb, sizeof(*wrb));
@@ -438,6 +462,16 @@ static int make_tx_wrbs(struct be_adapter *adapter,
be_dws_cpu_to_le(hdr, sizeof(*hdr));
return copied;
+dma_err:
+ txq->head = map_head;
+ while (copied) {
+ wrb = queue_head_node(txq);
+ unmap_tx_frag(pdev, wrb, map_single);
+ map_single = false;
+ copied -= wrb->frag_len;
+ queue_head_inc(txq);
+ }
+ return 0;
}
static netdev_tx_t be_xmit(struct sk_buff *skb,
@@ -462,6 +496,7 @@ static netdev_tx_t be_xmit(struct sk_buff *skb,
* *BEFORE* ringing the tx doorbell, so that we serialze the
* tx compls of the current transmit which'll wake up the queue
*/
+ atomic_add(wrb_cnt, &txq->used);
if ((BE_MAX_TX_FRAG_COUNT + atomic_read(&txq->used)) >=
txq->len) {
netif_stop_queue(netdev);
--
1.6.3.3
^ permalink raw reply related
* Re: [PATCH] Fix locking in flush_backlog
From: Eric Dumazet @ 2010-03-23 6:29 UTC (permalink / raw)
To: Tom Herbert; +Cc: davem, netdev
In-Reply-To: <alpine.DEB.1.00.1003222257470.3718@pokey.mtv.corp.google.com>
Le lundi 22 mars 2010 à 23:04 -0700, Tom Herbert a écrit :
> Need to take spinlocks when dequeuing from input_pkt_queue in
> flush_backlog. Also, with the spinlock the backlog queues can
> be flushed directly from netdev_run_todo.
>
> Signed-off-by: Tom Herbert <therbert@google.com>
> ---
> diff --git a/net/core/dev.c b/net/core/dev.c
> index a03aab4..e7db656 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -2765,20 +2765,6 @@ int netif_receive_skb(struct sk_buff *skb)
> }
> EXPORT_SYMBOL(netif_receive_skb);
>
> -/* Network device is going away, flush any packets still pending */
> -static void flush_backlog(void *arg)
> -{
> - struct net_device *dev = arg;
> - struct softnet_data *queue = &__get_cpu_var(softnet_data);
> - struct sk_buff *skb, *tmp;
> -
> - skb_queue_walk_safe(&queue->input_pkt_queue, skb, tmp)
> - if (skb->dev == dev) {
> - __skb_unlink(skb, &queue->input_pkt_queue);
> - kfree_skb(skb);
> - }
> -}
> -
> static int napi_gro_complete(struct sk_buff *skb)
> {
> struct packet_type *ptype;
> @@ -5545,6 +5531,7 @@ void netdev_run_todo(void)
> while (!list_empty(&list)) {
> struct net_device *dev
> = list_first_entry(&list, struct net_device, todo_list);
> + int i;
> list_del(&dev->todo_list);
>
> if (unlikely(dev->reg_state != NETREG_UNREGISTERING)) {
> @@ -5556,7 +5543,22 @@ void netdev_run_todo(void)
>
> dev->reg_state = NETREG_UNREGISTERED;
>
> - on_each_cpu(flush_backlog, dev, 1);
> + /* Flush backlog queues of any pending packets */
> + for_each_online_cpu(i) {
> + struct softnet_data *queue = &per_cpu(softnet_data, i);
> + struct sk_buff *skb, *tmp;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&queue->input_pkt_queue.lock, flags);
> + skb_queue_walk_safe(&queue->input_pkt_queue, skb, tmp)
> + if (skb->dev == dev) {
> + __skb_unlink(skb,
> + &queue->input_pkt_queue);
> + kfree_skb(skb);
> + }
> + spin_unlock_irqrestore(&queue->input_pkt_queue.lock,
> + flags);
> + }
>
> netdev_wait_allrefs(dev);
>
OK (this is a patch for net-next-2.6)
Could you please keep the function, to ease netdev_run_todo() review ?
Thanks
^ permalink raw reply
* [PATCH] rps: make distributing packets fairly among all the online CPUs default
From: Changli Gao @ 2010-03-23 6:24 UTC (permalink / raw)
To: David S. Miller; +Cc: Tom Herbert, xiaosuo, netdev
make distributing packets fairly among all the online CPUs default.
Make distributing packets fairly among all the online CPUs default, then
users don't need any explicit configuration to get the benefit of RPS.
Signed-off-by: Changli Gao <xiaosuo@gmail.com>
----
net/core/dev.c | 34 ++++++++++++++++++++++++++++++++--
1 file changed, 32 insertions(+), 2 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index c0e2608..a4246f1 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5234,6 +5234,24 @@ void netif_stacked_transfer_operstate(const struct net_device *rootdev,
}
EXPORT_SYMBOL(netif_stacked_transfer_operstate);
+static struct rps_map* alloc_rps_map(void)
+{
+ struct rps_map *map;
+ int i, cpu;
+
+ map = kzalloc(max_t(unsigned,
+ RPS_MAP_SIZE(cpumask_weight(cpu_online_mask)),
+ L1_CACHE_BYTES), GFP_KERNEL);
+ if (map == NULL)
+ return NULL;
+ i = 0;
+ for_each_online_cpu(cpu)
+ map->cpus[i++] = cpu;
+ map->len = i;
+
+ return map;
+}
+
/**
* register_netdevice - register a network device
* @dev: device to register
@@ -5282,7 +5300,13 @@ int register_netdevice(struct net_device *dev)
ret = -ENOMEM;
goto out;
}
-
+ dev->_rx->rps_map = alloc_rps_map();
+ if (dev->_rx->rps_map == NULL) {
+ kfree(dev->_rx);
+ dev->_rx = NULL;
+ ret = -ENOMEM;
+ goto out;
+ }
dev->_rx->first = dev->_rx;
atomic_set(&dev->_rx->count, 1);
dev->num_rx_queues = 1;
@@ -5688,8 +5712,12 @@ struct net_device *alloc_netdev_mq(int sizeof_priv, const char *name,
* Set a pointer to first element in the array which holds the
* reference count.
*/
- for (i = 0; i < queue_count; i++)
+ for (i = 0; i < queue_count; i++) {
rx[i].first = rx;
+ rx[i].rps_map = alloc_rps_map();
+ if (rx[i].rps_map == NULL)
+ goto free_rx;
+ }
dev = PTR_ALIGN(p, NETDEV_ALIGN);
dev->padded = (char *)dev - (char *)p;
@@ -5723,6 +5751,8 @@ struct net_device *alloc_netdev_mq(int sizeof_priv, const char *name,
return dev;
free_rx:
+ for (i = 0; i < queue_count; i++)
+ kfree(rx[i].rps_map);
kfree(rx);
free_tx:
kfree(tx);
^ 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