* Re: [PATCH] xfrm: cache bundle lookup results in flow cache
From: Timo Teräs @ 2010-03-19 7:27 UTC (permalink / raw)
To: Herbert Xu; +Cc: netdev
In-Reply-To: <4BA317CE.4050503@iki.fi>
Timo Teräs wrote:
> Herbert Xu wrote:
>> On Fri, Mar 19, 2010 at 07:48:57AM +0200, Timo Teräs wrote:
>>> But it always matches. The caching happens using the inner
>>> flow. Inner flow always matches with the same bundle unless
>>> the bundle expires or goes stale. What happens is that I get
>>> multiple cache entries per-inner flow each referencing to the
>>> same bundle.
>>
>> Sorry for being slow, but if it always matches, doesn't that mean
>> you'll only have a single bundle in the policy bundle list? IOW
>> why do we need this at all?
>
> No. The bundle created for specific flow, matches always later
> that flow.
Just figured that's it's easier to explain with an example.
We have SPD:
10.1.0.0/16 - 10.2.0.0/16 tunnel
1.2.3.4 - 4.3.2.1
Now we get n+1 clients to connect to server in 10.2.0.1.
They each get separate bundle, since the xfrm_dst will be
created and search using flow id's like:
src 10.1.x.x dst 10.2.0.1
So there's one xfrm_policy and xfrm_state, but n+1
xfrm_dst's.
Since the flow cache caches the result of lookups on the
inner flow "10.1.x.x->10.2.0.1" basis, it always returns
matching valid bundle in O(1) time unless the xfrm_dst
expired.
Currently it's looked up with O(n) search in find_bundle.
Same thing happens with wildcard transport mode SPD's.
E.g. SPD:
0.0.0.0/0 - 0.0.0.0/0 proto gre, transport
We are talking with gre to n+1 tunnel destinations.
We get n+1 xfrm_dst's in that xfrm_policy. Flow cache
works on inner flow using flows like:
src 1.2.3.4 dst 4.3.2.1 proto gre
And can keep in cache the right policy always, and
the bundle to use as long as it stays valid.
Hopefully this explains why I think the patch is
useful.
- Timo
^ permalink raw reply
* Re: [PATCH] xfrm: cache bundle lookup results in flow cache
From: Herbert Xu @ 2010-03-19 7:20 UTC (permalink / raw)
To: Timo Teras; +Cc: netdev
In-Reply-To: <1268655610-7845-1-git-send-email-timo.teras@iki.fi>
On Mon, Mar 15, 2010 at 02:20:10PM +0200, Timo Teras wrote:
>
> - policy = flow_cache_lookup(net, fl, dst_orig->ops->family,
> - dir, xfrm_policy_lookup);
> - err = PTR_ERR(policy);
> - if (IS_ERR(policy)) {
> - XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTPOLERROR);
> - goto dropdst;
> + fce = flow_cache_lookup(&net->xfrm.flow_cache,
> + fl, family, dir);
> + if (fce == NULL)
> + goto no_cache;
> +
> + xf = container_of(fce, struct xfrm_flow_cache_entry, fce);
> + xfrm_flow_cache_entry_validate(&net->xfrm.flow_cache, fce);
This doesn't work.
The flow cache operates without locking as it is a per-cpu cache.
To make this work you must ensure that you stay on the same CPU
or use some other form of synchronoisation if you write to the
object returned.
AFAICS there is no synchronisation here and you're writing to fce.
So you'll need to disable preemption around the bit that touches
fce.
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: Herbert Xu @ 2010-03-19 7:17 UTC (permalink / raw)
To: Timo Teräs; +Cc: netdev
In-Reply-To: <4BA317CE.4050503@iki.fi>
On Fri, Mar 19, 2010 at 08:21:02AM +0200, Timo Teräs wrote:
>
>> Or have I misread your patch? You *are* proposing to cache the last
>> used bundle in the policy, right?
>
> Yes and no. The bundle used is cached on per-flow basis.
> The flow cache can have lot of entries each referring to
> same policy but separate bundle.
OK so I did misread your patch.
In fact it is already doing exactly what I was suggesting, I'll
review your patch again with this new insignt :)
> But keeping policy and bundle in cache is still a win. If we
> kill the xfrm_dst due to GC, we will also lose the policy
> the flow matched. We might need to kill xfrm_dst due to the
> inside dst going old, but the flow cache would still give
> hit with policy info (but no bundle) the next a packet comes
> in using the same flow.
We were in complete agreement all along :)
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
* AW: [Uclinux-dist-devel] [PATCH] can: bfin_can: switch to common Blackfin can header
From: Hartkopp, Oliver (K-EFFI/P) @ 2010-03-19 6:56 UTC (permalink / raw)
To: Mike Frysinger, David Miller
Cc: socketcan-core, netdev, uclinux-dist-devel,
Thuermann, Urs, Dr. (K-EFFI/I)
In-Reply-To: <8bd0f97a1003182335l3c121d87v9369145852c9545d@mail.gmail.com>
Hi Mike,
your patch is just a cleanup and definitely not mission-critical.
Please wait until your needed environment emerges in Dave's net-2.6 or
net-next-2.6 tree until you post your patch for the specific tree then.
This makes it much easier - at least for Dave.
Thanks,
Oliver
-----Ursprüngliche Nachricht-----
Von: Mike Frysinger [mailto:vapier.adi@gmail.com]
Gesendet: Freitag, 19. März 2010 07:35
An: David Miller
Cc: socketcan-core@lists.berlios.de; netdev@vger.kernel.org; uclinux-dist-devel@blackfin.uclinux.org; Hartkopp, Oliver (K-EFFI/P); Thuermann, Urs, Dr. (K-EFFI/I)
Betreff: Re: [Uclinux-dist-devel] [PATCH] can: bfin_can: switch to common Blackfin can header
On Wed, Mar 17, 2010 at 01:51, David Miller wrote:
> From: Mike Frysinger <vapier.adi@gmail.com>
>> if the next tree is merged into linux-next, then the header is already there
>
> One of the highest priority trees in -next is net-next-2.6,
> because so much other stuff ends up depending upon it.
>
> Therefore I keep it building independently, so no matter what Stephen
> Rothwell does the build isn't likely to break.
>
> Just get your stuff merged properly to Linus then resubmit
> your patch.
Linus has pulled my tree, so there shouldnt be any reason to not merge
this patch now
-mike
^ permalink raw reply
* [PATCH net-next-2.6] net: snmp mib cleanup
From: Eric Dumazet @ 2010-03-19 6:36 UTC (permalink / raw)
To: David Miller; +Cc: netdev
There is no point to align or pad mibs to cache lines, they are per cpu
allocated with a 8 bytes alignment anyway.
This wastes space for no gain. This patch removes __SNMP_MIB_ALIGN__
Since SNMP mibs contain "unsigned long" fields only, we can relax the
allocation alignment from "unsigned long long" to "unsigned long"
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
include/net/sctp/sctp.h | 2 +-
include/net/snmp.h | 29 +++++++----------------------
net/dccp/dccp.h | 2 +-
net/ipv4/af_inet.c | 4 ++--
4 files changed, 11 insertions(+), 26 deletions(-)
diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
index 78740ec..5915155 100644
--- a/include/net/sctp/sctp.h
+++ b/include/net/sctp/sctp.h
@@ -268,7 +268,7 @@ enum {
#define SCTP_MIB_MAX __SCTP_MIB_MAX
struct sctp_mib {
unsigned long mibs[SCTP_MIB_MAX];
-} __SNMP_MIB_ALIGN__;
+};
/* Print debugging messages. */
diff --git a/include/net/snmp.h b/include/net/snmp.h
index 692ee00..884fdbb 100644
--- a/include/net/snmp.h
+++ b/include/net/snmp.h
@@ -52,26 +52,11 @@ struct snmp_mib {
* count on the 20Gb/s + networks people expect in a few years time!
*/
-/*
- * The rule for padding:
- * Best is power of two because then the right structure can be found by a
- * simple shift. The structure should be always cache line aligned.
- * gcc needs n=alignto(cachelinesize, popcnt(sizeof(bla_mib))) shift/add
- * instructions to emulate multiply in case it is not power-of-two.
- * Currently n is always <=3 for all sizes so simple cache line alignment
- * is enough.
- *
- * The best solution would be a global CPU local area , especially on 64
- * and 128byte cacheline machine it makes a *lot* of sense -AK
- */
-
-#define __SNMP_MIB_ALIGN__ ____cacheline_aligned
-
/* IPstats */
#define IPSTATS_MIB_MAX __IPSTATS_MIB_MAX
struct ipstats_mib {
unsigned long mibs[IPSTATS_MIB_MAX];
-} __SNMP_MIB_ALIGN__;
+};
/* ICMP */
#define ICMP_MIB_DUMMY __ICMP_MIB_MAX
@@ -79,36 +64,36 @@ struct ipstats_mib {
struct icmp_mib {
unsigned long mibs[ICMP_MIB_MAX];
-} __SNMP_MIB_ALIGN__;
+};
#define ICMPMSG_MIB_MAX __ICMPMSG_MIB_MAX
struct icmpmsg_mib {
unsigned long mibs[ICMPMSG_MIB_MAX];
-} __SNMP_MIB_ALIGN__;
+};
/* ICMP6 (IPv6-ICMP) */
#define ICMP6_MIB_MAX __ICMP6_MIB_MAX
struct icmpv6_mib {
unsigned long mibs[ICMP6_MIB_MAX];
-} __SNMP_MIB_ALIGN__;
+};
#define ICMP6MSG_MIB_MAX __ICMP6MSG_MIB_MAX
struct icmpv6msg_mib {
unsigned long mibs[ICMP6MSG_MIB_MAX];
-} __SNMP_MIB_ALIGN__;
+};
/* TCP */
#define TCP_MIB_MAX __TCP_MIB_MAX
struct tcp_mib {
unsigned long mibs[TCP_MIB_MAX];
-} __SNMP_MIB_ALIGN__;
+};
/* UDP */
#define UDP_MIB_MAX __UDP_MIB_MAX
struct udp_mib {
unsigned long mibs[UDP_MIB_MAX];
-} __SNMP_MIB_ALIGN__;
+};
/* Linux */
#define LINUX_MIB_MAX __LINUX_MIB_MAX
diff --git a/net/dccp/dccp.h b/net/dccp/dccp.h
index 5ef32c2..53f8e12 100644
--- a/net/dccp/dccp.h
+++ b/net/dccp/dccp.h
@@ -189,7 +189,7 @@ enum {
#define DCCP_MIB_MAX __DCCP_MIB_MAX
struct dccp_mib {
unsigned long mibs[DCCP_MIB_MAX];
-} __SNMP_MIB_ALIGN__;
+};
DECLARE_SNMP_STAT(struct dccp_mib, dccp_statistics);
#define DCCP_INC_STATS(field) SNMP_INC_STATS(dccp_statistics, field)
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 33b7dff..55e1190 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1401,10 +1401,10 @@ EXPORT_SYMBOL_GPL(snmp_fold_field);
int snmp_mib_init(void __percpu *ptr[2], size_t mibsize)
{
BUG_ON(ptr == NULL);
- ptr[0] = __alloc_percpu(mibsize, __alignof__(unsigned long long));
+ ptr[0] = __alloc_percpu(mibsize, __alignof__(unsigned long));
if (!ptr[0])
goto err0;
- ptr[1] = __alloc_percpu(mibsize, __alignof__(unsigned long long));
+ ptr[1] = __alloc_percpu(mibsize, __alignof__(unsigned long));
if (!ptr[1])
goto err1;
return 0;
^ permalink raw reply related
* Re: [Uclinux-dist-devel] [PATCH] can: bfin_can: switch to common Blackfin can header
From: Mike Frysinger @ 2010-03-19 6:35 UTC (permalink / raw)
To: David Miller
Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
netdev-u79uwXL29TY76Z2rM5mHXA,
urs.thuermann-l29pVbxQd1IUtdQbppsyvg,
oliver.hartkopp-l29pVbxQd1IUtdQbppsyvg,
uclinux-dist-devel-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b
In-Reply-To: <20100316.225150.213941689.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
On Wed, Mar 17, 2010 at 01:51, David Miller wrote:
> From: Mike Frysinger <vapier.adi-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> if the next tree is merged into linux-next, then the header is already there
>
> One of the highest priority trees in -next is net-next-2.6,
> because so much other stuff ends up depending upon it.
>
> Therefore I keep it building independently, so no matter what Stephen
> Rothwell does the build isn't likely to break.
>
> Just get your stuff merged properly to Linus then resubmit
> your patch.
Linus has pulled my tree, so there shouldnt be any reason to not merge
this patch now
-mike
^ permalink raw reply
* [PATCH net-next-2.6] ipv6: Reduce timer events for addrconf_verify().
From: YOSHIFUJI Hideaki @ 2010-03-18 9:01 UTC (permalink / raw)
To: davem; +Cc: yoshfuji, netdev, stephen.hemminger
This patch reduces timer events while keeping accuracy by rounding
our timer and/or batching several address validations in addrconf_verify().
addrconf_verify() is called at earliest timeout among interface addresses'
timeouts, but at maximum ADDR_CHECK_FREQUENCY (120 secs).
In most cases, all of timeouts of interface addresses are long enough
(e.g. several hours or days vs 2 minutes), this timer is usually called
every ADDR_CHECK_FREQUENCY, and it is okay to be lazy.
(Note this timer could be eliminated if all code paths which modifies
variables related to timeouts call us manually, but it is another story.)
However, in other least but important cases, we try keeping accuracy.
When the real interface address timeout is coming, and the timeout
is just before the rounded timeout, we accept some error.
When a timeout has been reached, we also try batching other several
events in very near future.
Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
net/ipv6/addrconf.c | 27 +++++++++++++++++++++++----
1 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 8d41abc..b716083 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -99,6 +99,10 @@
#define INFINITY_LIFE_TIME 0xFFFFFFFF
#define TIME_DELTA(a,b) ((unsigned long)((long)(a) - (long)(b)))
+#define ADDRCONF_TIMER_FUZZ_MINUS (HZ > 50 ? HZ/50 : 1)
+#define ADDRCONF_TIMER_FUZZ (HZ / 4)
+#define ADDRCONF_TIMER_FUZZ_MAX (HZ)
+
#ifdef CONFIG_SYSCTL
static void addrconf_sysctl_register(struct inet6_dev *idev);
static void addrconf_sysctl_unregister(struct inet6_dev *idev);
@@ -3124,12 +3128,12 @@ int ipv6_chk_home_addr(struct net *net, struct in6_addr *addr)
static void addrconf_verify(unsigned long foo)
{
struct inet6_ifaddr *ifp;
- unsigned long now, next;
+ unsigned long now, next, next_sec, next_sched;
int i;
spin_lock_bh(&addrconf_verify_lock);
now = jiffies;
- next = now + ADDR_CHECK_FREQUENCY;
+ next = round_jiffies_up(now + ADDR_CHECK_FREQUENCY);
del_timer(&addr_chk_timer);
@@ -3147,7 +3151,8 @@ restart:
continue;
spin_lock(&ifp->lock);
- age = (now - ifp->tstamp) / HZ;
+ /* We try to batch several events at once. */
+ age = (now - ifp->tstamp + ADDRCONF_TIMER_FUZZ_MINUS) / HZ;
#ifdef CONFIG_IPV6_PRIVACY
regen_advance = ifp->idev->cnf.regen_max_retry *
@@ -3222,7 +3227,21 @@ restart:
read_unlock(&addrconf_hash_lock);
}
- addr_chk_timer.expires = time_before(next, jiffies + HZ) ? jiffies + HZ : next;
+ next_sec = round_jiffies_up(next);
+ next_sched = next;
+
+ /* If rounded timeout is accurate enough, accept it. */
+ if (time_before(next_sec, next + ADDRCONF_TIMER_FUZZ))
+ next_sched = next_sec;
+
+ /* And minimum interval is ADDRCONF_TIMER_FUZZ_MAX. */
+ if (time_before(next_sched, jiffies + ADDRCONF_TIMER_FUZZ_MAX))
+ next_sched = jiffies + ADDRCONF_TIMER_FUZZ_MAX;
+
+ ADBG((KERN_DEBUG "now = %lu, schedule = %lu, rounded schedule = %lu => %lu\n",
+ now, next, next_sec, next_sched));
+
+ addr_chk_timer.expires = next_sched;
add_timer(&addr_chk_timer);
spin_unlock_bh(&addrconf_verify_lock);
}
--
1.5.6.5
^ permalink raw reply related
* Re: [iproute2] iproute2 question
From: Stephen Hemminger @ 2010-03-19 6:27 UTC (permalink / raw)
To: thomas yang; +Cc: hadi, netdev
In-Reply-To: <f4f837ab1003182253x12727855k118f7d67115daf23@mail.gmail.com>
On Fri, 19 Mar 2010 13:53:23 +0800
thomas yang <lampsu@gmail.com> wrote:
> 2010/3/18 Stephen Hemminger <shemminger@vyatta.com>:
> > On Thu, 18 Mar 2010 17:02:16 +0800
> > thomas yang <lampsu@gmail.com> wrote:
> >
> >> Hi,
> >>
> >> Linux router R (has some interfaces) use the table "main" as default
> >> routing table to route packets. If some link connected to R failed,
> >> I want to use another table "backup_tbl" to route packets; if the
> >> broken link repaired , go back to use table "main" . How to do this?
> >
> > That's now it works. Use a real routing daemon.
> >
>
> I want to reduce the packet loss that happens while routers converge
> after a topology change due to a failure, use precalculated backup
> table to do rapid failure repair (repair faster than routing daemon,
> fast reroute).
> I do static routing on my linux routers. When some link failed,
> I want to use backup tables to route packets. Can iproute2 do this?
You could probably kludge something with multiple route tables
and modifying a single 'ip rule'.
Something like:
http://www.linuxhorizon.ro/iproute2.html
^ permalink raw reply
* Re: [PATCH] xfrm: cache bundle lookup results in flow cache
From: Timo Teräs @ 2010-03-19 6:21 UTC (permalink / raw)
To: Herbert Xu; +Cc: netdev
In-Reply-To: <20100319060322.GA22319@gondor.apana.org.au>
Herbert Xu wrote:
> On Fri, Mar 19, 2010 at 07:48:57AM +0200, Timo Teräs wrote:
>> But it always matches. The caching happens using the inner
>> flow. Inner flow always matches with the same bundle unless
>> the bundle expires or goes stale. What happens is that I get
>> multiple cache entries per-inner flow each referencing to the
>> same bundle.
>
> Sorry for being slow, but if it always matches, doesn't that mean
> you'll only have a single bundle in the policy bundle list? IOW
> why do we need this at all?
No. The bundle created for specific flow, matches always later
that flow.
With transport mode wildcard policy, e.g. single policy saying
encrypt traffic with protocol X to all IP-address, you get a
separate bundle per-public IP destination. Bundle matches only
that specific IP since it gets a separate xfrm_state. But you
can talk to all the hosts in internet using same policy, so
you can end up with a whole lot of valid bundles in the same
policy.
I'm not sure how this works in tunnel mode. It might be that
single bundle can be reused for all packets. But I think the same
applies to tunnel mode. Since afinfo->fill_dst() puts the
inner flow to bundle xfrm_dst->u.rt.fl, which is later compared
against the inner flow by afinfo->find_bundle(). I think this
implies that for each flow traveling inside tunnel, it gets it's
separate xfrm_dst, so again you end up with a whole lot of
valid bundles in the same policy.
> Or have I misread your patch? You *are* proposing to cache the last
> used bundle in the policy, right?
Yes and no. The bundle used is cached on per-flow basis.
The flow cache can have lot of entries each referring to
same policy but separate bundle.
>> True. But if we go and prune a bundle due to it being bad or
>> needing garbage collection we need to invalidate all bundles
>> pointers, and we cannot access the back-pointer. Alternatively
>
> Why can't you access the back-pointer? You should always have
> a reference held on the policy, either explicit or implicit.
>
>> we need to keep xfrm_dst references again in the flow cache
>> requiring an expensive iteration of all flow cache entries
>> whenever a xfrm_dst needs to be deleted (which happens often).
>
> So does the IPv4 routing cache. I think what this reflects is
> just that the IPsec garbage collection mechanism is broken.
>
> There is no point in doing a GC on every dst_alloc if we know
> that it isn't going to go below the threshold. It should gain
> a minimum GC interval like IPv4. Or perhaps we can move the
> minimum GC interval check into the dst core.
Yes, I reported xfrm_dst GC being broke in the earlier mail.
But keeping policy and bundle in cache is still a win. If we
kill the xfrm_dst due to GC, we will also lose the policy
the flow matched. We might need to kill xfrm_dst due to the
inside dst going old, but the flow cache would still give
hit with policy info (but no bundle) the next a packet comes
in using the same flow.
- Timo
^ permalink raw reply
* Re: [PATCH net-next-2.6] net: speedup netdev_set_master()
From: David Miller @ 2010-03-19 6:11 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1268977513.2894.223.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 19 Mar 2010 06:45:13 +0100
> Le jeudi 18 mars 2010 à 22:36 -0700, David Miller a écrit :
>> It was not an issue of related, but rather "B won't apply cleanly
>> without A". Even line offsets can make "git apply" reject.
>> --
>
> I see, I can resubmit later if you prefer, once net-2.6 fix is pulled in
> net-next, there is no hurry :)
No need, just let me know about this stuff in the future, that's
all :-)
^ permalink raw reply
* Re: [PATCH] xfrm: cache bundle lookup results in flow cache
From: Herbert Xu @ 2010-03-19 6:03 UTC (permalink / raw)
To: Timo Teräs; +Cc: netdev
In-Reply-To: <4BA31049.7030005@iki.fi>
On Fri, Mar 19, 2010 at 07:48:57AM +0200, Timo Teräs wrote:
>
> But it always matches. The caching happens using the inner
> flow. Inner flow always matches with the same bundle unless
> the bundle expires or goes stale. What happens is that I get
> multiple cache entries per-inner flow each referencing to the
> same bundle.
Sorry for being slow, but if it always matches, doesn't that mean
you'll only have a single bundle in the policy bundle list? IOW
why do we need this at all?
Or have I misread your patch? You *are* proposing to cache the last
used bundle in the policy, right?
> True. But if we go and prune a bundle due to it being bad or
> needing garbage collection we need to invalidate all bundles
> pointers, and we cannot access the back-pointer. Alternatively
Why can't you access the back-pointer? You should always have
a reference held on the policy, either explicit or implicit.
> we need to keep xfrm_dst references again in the flow cache
> requiring an expensive iteration of all flow cache entries
> whenever a xfrm_dst needs to be deleted (which happens often).
So does the IPv4 routing cache. I think what this reflects is
just that the IPsec garbage collection mechanism is broken.
There is no point in doing a GC on every dst_alloc if we know
that it isn't going to go below the threshold. It should gain
a minimum GC interval like IPv4. Or perhaps we can move the
minimum GC interval check into the dst core.
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
* [PATCH] ipv4: check rt_genid in dst_check
From: Timo Teras @ 2010-03-19 5:54 UTC (permalink / raw)
To: netdev; +Cc: Timo Teras
Xfrm_dst keeps a reference to ipv4 rtable entries on each
cached bundle. The only way to renew xfrm_dst when the underlying
route has changed, is to implement dst_check for this. This is
what ipv6 side does too.
The problems started after 87c1e12b5eeb7b30b4b41291bef8e0b41fc3dde9
which fixed a bug causing xfrm_dst to not get reused, until that all
lookups always generated new xfrm_dst with new route reference
and path mtu worked. But after the fix, the old routes started
to get reused even after they were expired causing pmtu to break
(well it would occationally work if the rtable gc had run recently
and marked the route obsolete causing dst_check to get called).
Signed-off-by: Timo Teras <timo.teras@iki.fi>
Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
---
net/ipv4/route.c | 14 ++++++++++----
1 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index a770df2..8c9d06e 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1441,7 +1441,7 @@ void ip_rt_redirect(__be32 old_gw, __be32 daddr, __be32 new_gw,
dev_hold(rt->u.dst.dev);
if (rt->idev)
in_dev_hold(rt->idev);
- rt->u.dst.obsolete = 0;
+ rt->u.dst.obsolete = -1;
rt->u.dst.lastuse = jiffies;
rt->u.dst.path = &rt->u.dst;
rt->u.dst.neighbour = NULL;
@@ -1506,7 +1506,7 @@ static struct dst_entry *ipv4_negative_advice(struct dst_entry *dst)
struct dst_entry *ret = dst;
if (rt) {
- if (dst->obsolete) {
+ if (dst->obsolete > 0) {
ip_rt_put(rt);
ret = NULL;
} else if ((rt->rt_flags & RTCF_REDIRECTED) ||
@@ -1726,7 +1726,9 @@ static void ip_rt_update_pmtu(struct dst_entry *dst, u32 mtu)
static struct dst_entry *ipv4_dst_check(struct dst_entry *dst, u32 cookie)
{
- return NULL;
+ if (dst && rt_is_expired((struct rtable *)dst))
+ return NULL;
+ return dst;
}
static void ipv4_dst_destroy(struct dst_entry *dst)
@@ -1888,7 +1890,8 @@ static int ip_route_input_mc(struct sk_buff *skb, __be32 daddr, __be32 saddr,
if (!rth)
goto e_nobufs;
- rth->u.dst.output= ip_rt_bug;
+ rth->u.dst.output = ip_rt_bug;
+ rth->u.dst.obsolete = -1;
atomic_set(&rth->u.dst.__refcnt, 1);
rth->u.dst.flags= DST_HOST;
@@ -2054,6 +2057,7 @@ static int __mkroute_input(struct sk_buff *skb,
rth->fl.oif = 0;
rth->rt_spec_dst= spec_dst;
+ rth->u.dst.obsolete = -1;
rth->u.dst.input = ip_forward;
rth->u.dst.output = ip_output;
rth->rt_genid = rt_genid(dev_net(rth->u.dst.dev));
@@ -2218,6 +2222,7 @@ local_input:
goto e_nobufs;
rth->u.dst.output= ip_rt_bug;
+ rth->u.dst.obsolete = -1;
rth->rt_genid = rt_genid(net);
atomic_set(&rth->u.dst.__refcnt, 1);
@@ -2444,6 +2449,7 @@ static int __mkroute_output(struct rtable **result,
rth->rt_spec_dst= fl->fl4_src;
rth->u.dst.output=ip_output;
+ rth->u.dst.obsolete = -1;
rth->rt_genid = rt_genid(dev_net(dev_out));
RT_CACHE_STAT_INC(out_slow_tot);
--
1.6.3.3
^ permalink raw reply related
* Re: [iproute2] iproute2 question
From: thomas yang @ 2010-03-19 5:53 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: hadi, netdev
In-Reply-To: <20100318085919.05328127@nehalam>
2010/3/18 Stephen Hemminger <shemminger@vyatta.com>:
> On Thu, 18 Mar 2010 17:02:16 +0800
> thomas yang <lampsu@gmail.com> wrote:
>
>> Hi,
>>
>> Linux router R (has some interfaces) use the table "main" as default
>> routing table to route packets. If some link connected to R failed,
>> I want to use another table "backup_tbl" to route packets; if the
>> broken link repaired , go back to use table "main" . How to do this?
>
> That's now it works. Use a real routing daemon.
>
I want to reduce the packet loss that happens while routers converge
after a topology change due to a failure, use precalculated backup
table to do rapid failure repair (repair faster than routing daemon,
fast reroute).
I do static routing on my linux routers. When some link failed,
I want to use backup tables to route packets. Can iproute2 do this?
^ permalink raw reply
* Re: [PATCH] xfrm: cache bundle lookup results in flow cache
From: Timo Teräs @ 2010-03-19 5:48 UTC (permalink / raw)
To: Herbert Xu; +Cc: netdev
In-Reply-To: <20100319003130.GA20227@gondor.apana.org.au>
Herbert Xu wrote:
> On Thu, Mar 18, 2010 at 09:30:58PM +0200, Timo Teräs wrote:
>> Now, these created xfrm_dst gets cached in policy->bundles
>> with ref count zero and not deleted until garbage collection
>> is required. That's how xfrm_find_bundle tries to reuse them
>> (but it does O(n) lookup).
>
> Yes but the way you're caching it in the policy means that it
> only helps if two consecutive packets happen to match the same
> bundle. If your traffic mix was such that each packet required
> a different bundle, then we're back to where we started.
>
> That's why I was asking for you to directly cache the xfrm_dst
> objects in the flow cache.
But it always matches. The caching happens using the inner
flow. Inner flow always matches with the same bundle unless
the bundle expires or goes stale. What happens is that I get
multiple cache entries per-inner flow each referencing to the
same bundle.
And this is even more useful with the gre+esp. No matter what
I send inside gre (with private IPs), it ends up being routed
to more limited set of outer public IP parties. The bundle
lookup happens with the public-IP flow, so the speed up works
even better.
>> Actually no. As the pmtu case showed, it's more likely that
>> xfrm_dst needs to be regenerated, but the policy stays the
>> same since policy db isn't touched that often. If we keep
>> them separately we can almost most of the time avoid doing
>> policy lookup which is also O(n). Also the currently cache
>> entry validation is needs to check policy's bundles_genid
>> before allowing touching of xfrm_dst. Otherwise we would have
>> to keep global bundle_genid, and we'd lose the parent pointer
>> on cache miss.
> A back-pointer does not require an O(n) lookup.
True. But if we go and prune a bundle due to it being bad or
needing garbage collection we need to invalidate all bundles
pointers, and we cannot access the back-pointer. Alternatively
we need to keep xfrm_dst references again in the flow cache
requiring an expensive iteration of all flow cache entries
whenever a xfrm_dst needs to be deleted (which happens often).
- Timo
^ permalink raw reply
* Re: [PATCH net-next-2.6] net: speedup netdev_set_master()
From: Eric Dumazet @ 2010-03-19 5:45 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20100318.223658.241427584.davem@davemloft.net>
Le jeudi 18 mars 2010 à 22:36 -0700, David Miller a écrit :
> It was not an issue of related, but rather "B won't apply cleanly
> without A". Even line offsets can make "git apply" reject.
> --
I see, I can resubmit later if you prefer, once net-2.6 fix is pulled in
net-next, there is no hurry :)
^ permalink raw reply
* RE: [PATCH] netxen: The driver doesn't work on NX_P3_B1 so cause probe to fail.
From: Amit Salecha @ 2010-03-19 5:36 UTC (permalink / raw)
To: David Miller, ebiederm@xmission.com; +Cc: netdev@vger.kernel.org, Ameen Rahman
In-Reply-To: <20100318.222126.27823381.davem@davemloft.net>
We are working on this problem, will let you know our decision by end of day.
-Thanks
-----Original Message-----
From: David Miller [mailto:davem@davemloft.net]
Sent: Friday, March 19, 2010 10:51 AM
To: ebiederm@xmission.com
Cc: Amit Salecha; netdev@vger.kernel.org; Ameen Rahman
Subject: Re: [PATCH] netxen: The driver doesn't work on NX_P3_B1 so cause probe to fail.
From: ebiederm@xmission.com (Eric W. Biederman)
Date: Thu, 18 Mar 2010 02:43:39 -0700
> Amit Salecha <amit.salecha@qlogic.com> writes:
>
>> Sorry for all the problem you faced.
>>
>> But you shouldn't add support of device which is not supported.
>> Netxen is now owned by Qlogic. You should first contact Qlogic to solve your problem.
>> Qlogic will take needed action based on problem.
>
> I'm not adding support. I am sending a patch removing support for cards
> that do not work with the current driver and have not worked since 2.6.31.
You qlogic folks better resolve this _FAST_ or else I'll
make an executive decision about how to handle this and
I guarentee I'll make a decision that you will not like.
Thanks. :-)
^ permalink raw reply
* Re: [PATCH net-next-2.6] net: speedup netdev_set_master()
From: David Miller @ 2010-03-19 5:36 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1268976491.2894.221.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 19 Mar 2010 06:28:11 +0100
> Le jeudi 18 mars 2010 à 22:22 -0700, David Miller a écrit :
>> From: Eric Dumazet <eric.dumazet@gmail.com>
>> Date: Fri, 19 Mar 2010 00:37:40 +0100
>>
>> > We currently force a synchronize_net() in netdev_set_master()
>> >
>> > This seems necessary only when a slave had a master and we dismantle it.
>> >
>> > In the other case ("ifenslave bond0 ethO"), we dont need this long
>> > delay.
>> >
>> > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
>>
>> Depends upon the net-2.6 race fix?
>>
>> Thanks for telling me :-/
>
> I believe it's not related. It's net-next-2.6 material, for sure :)
It was not an issue of related, but rather "B won't apply cleanly
without A". Even line offsets can make "git apply" reject.
^ permalink raw reply
* Re: [PATCH net-next-2.6] net: speedup netdev_set_master()
From: Eric Dumazet @ 2010-03-19 5:28 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20100318.222226.191403090.davem@davemloft.net>
Le jeudi 18 mars 2010 à 22:22 -0700, David Miller a écrit :
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Fri, 19 Mar 2010 00:37:40 +0100
>
> > We currently force a synchronize_net() in netdev_set_master()
> >
> > This seems necessary only when a slave had a master and we dismantle it.
> >
> > In the other case ("ifenslave bond0 ethO"), we dont need this long
> > delay.
> >
> > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
>
> Depends upon the net-2.6 race fix?
>
> Thanks for telling me :-/
I believe it's not related. It's net-next-2.6 material, for sure :)
But yes, I discovered the race fix while studying netdev_set_master()
Thanks David
^ permalink raw reply
* Re: [PATCH net-next-2.6] net: speedup netdev_set_master()
From: David Miller @ 2010-03-19 5:22 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1268955460.2894.207.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 19 Mar 2010 00:37:40 +0100
> We currently force a synchronize_net() in netdev_set_master()
>
> This seems necessary only when a slave had a master and we dismantle it.
>
> In the other case ("ifenslave bond0 ethO"), we dont need this long
> delay.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Depends upon the net-2.6 race fix?
Thanks for telling me :-/
^ permalink raw reply
* Re: [PATCH] netxen: The driver doesn't work on NX_P3_B1 so cause probe to fail.
From: David Miller @ 2010-03-19 5:21 UTC (permalink / raw)
To: ebiederm; +Cc: amit.salecha, netdev, ameen.rahman
In-Reply-To: <m11vfic6d0.fsf@fess.ebiederm.org>
From: ebiederm@xmission.com (Eric W. Biederman)
Date: Thu, 18 Mar 2010 02:43:39 -0700
> Amit Salecha <amit.salecha@qlogic.com> writes:
>
>> Sorry for all the problem you faced.
>>
>> But you shouldn't add support of device which is not supported.
>> Netxen is now owned by Qlogic. You should first contact Qlogic to solve your problem.
>> Qlogic will take needed action based on problem.
>
> I'm not adding support. I am sending a patch removing support for cards
> that do not work with the current driver and have not worked since 2.6.31.
You qlogic folks better resolve this _FAST_ or else I'll
make an executive decision about how to handle this and
I guarentee I'll make a decision that you will not like.
Thanks. :-)
^ permalink raw reply
* Re: [PATCH] smsc95xx: Fix tx checksum offload for small packets
From: David Miller @ 2010-03-19 5:19 UTC (permalink / raw)
To: steve.glendinning; +Cc: netdev
In-Reply-To: <1268940010-19394-1-git-send-email-steve.glendinning@smsc.com>
From: Steve Glendinning <steve.glendinning@smsc.com>
Date: Thu, 18 Mar 2010 19:20:10 +0000
> TX checksum offload does not work properly when transmitting
> UDP packets with 0, 1 or 2 bytes of data. This patch works
> around the problem by calculating checksums for these packets
> in the driver.
>
> Signed-off-by: Steve Glendinning <steve.glendinning@smsc.com>
Applied, but I had to fix up trailing whitespace on one of the
patch's lines. Please check for and fix up such trivial matters
before I ever have to see the patch in the future, thanks.
^ permalink raw reply
* Re: [PATCH] ipv4: check rt_genid in dst_check
From: Herbert Xu @ 2010-03-19 5:18 UTC (permalink / raw)
To: David Miller; +Cc: timo.teras, netdev
In-Reply-To: <20100318.221649.02256374.davem@davemloft.net>
On Thu, Mar 18, 2010 at 10:16:49PM -0700, David Miller wrote:
> From: Herbert Xu <herbert@gondor.apana.org.au>
> Date: Thu, 18 Mar 2010 20:11:46 +0800
>
> > On Thu, Mar 18, 2010 at 01:48:22PM +0200, Timo Teras wrote:
> >> @@ -1726,7 +1726,9 @@ static void ip_rt_update_pmtu(struct dst_entry *dst, u32 mtu)
> >>
> >> static struct dst_entry *ipv4_dst_check(struct dst_entry *dst, u32 cookie)
> >> {
> >> - return NULL;
> >> + if (dst && dst->dev && rt_is_expired((struct rtable *) dst))
^
While you're at it, please delete this space to preemptively
stop anyone from sending a checkpatch patch :)
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] ipv4: check rt_genid in dst_check
From: David Miller @ 2010-03-19 5:16 UTC (permalink / raw)
To: herbert; +Cc: timo.teras, netdev
In-Reply-To: <20100318121146.GA13307@gondor.apana.org.au>
From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Thu, 18 Mar 2010 20:11:46 +0800
> On Thu, Mar 18, 2010 at 01:48:22PM +0200, Timo Teras wrote:
>> @@ -1726,7 +1726,9 @@ static void ip_rt_update_pmtu(struct dst_entry *dst, u32 mtu)
>>
>> static struct dst_entry *ipv4_dst_check(struct dst_entry *dst, u32 cookie)
>> {
>> - return NULL;
>> + if (dst && dst->dev && rt_is_expired((struct rtable *) dst))
>> + return NULL;
>> + return dst;
>> }
>
> Can dst->dev ever be NULL? I'm pretty sure that we disallow that
> from ever happening through the use of the loopback device. A
> quick grep also fails to find any other dst->dev NULL checks in
> this file.
Timo please respin with the NULL check removed, it just creates
confusion if one spot has the NULL check and not only is it
not needed, but also no other spots make this check.
Please remember to add in Herbert's ACK when reposting.
Thanks.
^ permalink raw reply
* Re: [net-2.6 PATCH] ixgbe: Fix 82599 KX4 Wake on LAN issue after an improper system shutdown
From: David Miller @ 2010-03-19 5:14 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, gospo, mallikarjuna.chilakala
In-Reply-To: <20100319011656.9385.44064.stgit@localhost.localdomain>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Thu, 18 Mar 2010 18:16:56 -0700
> From: Mallikarjuna R Chilakala <mallikarjuna.chilakala@intel.com>
>
> Advanced Power Management is disabled for 82599 KX4 connections by
> clearing GRC.APME bit, causing it to not wake the system from an
> improper system shutdown. By default GRC.APME is enabled and software
> is not supposed to clear these settings during adapter probe.
>
> Signed-off-by: Mallikarjuna R Chilakala <mallikarjuna.chilakala@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Applied.
^ permalink raw reply
* Re: [net-2.6 PATCH] ixgbe: Fix 82599 multispeed fiber link issues due to Tx laser flapping
From: David Miller @ 2010-03-19 5:14 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, gospo, mallikarjuna.chilakala
In-Reply-To: <20100319003422.8981.79416.stgit@localhost.localdomain>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Thu, 18 Mar 2010 17:34:52 -0700
> From: Mallikarjuna R Chilakala <mallikarjuna.chilakala@intel.com>
>
> Fix 82599 link issues during driver load and unload test using multi-speed
> 10G & 1G fiber modules. When connected back to back sometime 82599 multispeed
> fiber modules would link at 1G speed instead of 10G highest speed, due to a
> race condition in autotry process involving Tx laser flapping. Move autotry
> autoneg-37 tx laser flapping process from multispeed module init setup
> to driver unload. This will alert the link partner to restart its
> autotry process when it tries to establish the link with the link partner
>
> Signed-off-by: Mallikarjuna R Chilakala <mallikarjuna.chilakala@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Applied.
^ 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