* ip_vs_lblcr logic error causing table flushing
@ 2009-11-05 0:38 Simon Kirby
2009-11-05 1:35 ` Simon Horman
0 siblings, 1 reply; 4+ messages in thread
From: Simon Kirby @ 2009-11-05 0:38 UTC (permalink / raw)
To: lvs-devel, Wensong Zhang
[ Resent with a reasonable subject and to lvs-devel :) ]
Hello!
I was noticing a significant amount of what seems/seemed to be
destination lists with multiple entries with the lblcr LVS algorithm.
While tracking it down, I think I stumbled over a mistake. In
ip_vs_lblcr_full_check(), it appears the time check logic is reversed:
for (i=0, j=tbl->rover; i<IP_VS_LBLCR_TAB_SIZE; i++) {
j = (j + 1) & IP_VS_LBLCR_TAB_MASK;
write_lock(&svc->sched_lock);
list_for_each_entry_safe(en, nxt, &tbl->bucket[j], list) {
if (time_after(en->lastuse+sysctl_ip_vs_lblcr_expiration,
now))
continue;
ip_vs_lblcr_free(en);
atomic_dec(&tbl->entries);
}
write_unlock(&svc->sched_lock);
}
Shouldn't this be "time_before"? It seems that it currently nukes all
recently-used entries every time this function is called, which seems to
be every 30 minutes, rather than removing the not-recently-used ones.
If my reading is correct, this patch should fix it. Am I missing
something?
Cheers,
Simon-
diff --git a/net/netfilter/ipvs/ip_vs_lblcr.c b/net/netfilter/ipvs/ip_vs_lblcr.c
index 715b57f..937743f 100644
--- a/net/netfilter/ipvs/ip_vs_lblcr.c
+++ b/net/netfilter/ipvs/ip_vs_lblcr.c
@@ -432,7 +432,7 @@ static inline void ip_vs_lblcr_full_check(struct ip_vs_service *svc)
write_lock(&svc->sched_lock);
list_for_each_entry_safe(en, nxt, &tbl->bucket[j], list) {
- if (time_after(en->lastuse+sysctl_ip_vs_lblcr_expiration,
+ if (time_before(en->lastuse+sysctl_ip_vs_lblcr_expiration,
now))
continue;
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: ip_vs_lblcr logic error causing table flushing
2009-11-05 0:38 ip_vs_lblcr logic error causing table flushing Simon Kirby
@ 2009-11-05 1:35 ` Simon Horman
2009-11-05 8:37 ` [PATCH] Fix ip_vs_lblcr_full_check to remove unused entries instead of used entries Simon Kirby
0 siblings, 1 reply; 4+ messages in thread
From: Simon Horman @ 2009-11-05 1:35 UTC (permalink / raw)
To: Simon Kirby; +Cc: lvs-devel, Wensong Zhang
On Wed, Nov 04, 2009 at 04:38:01PM -0800, Simon Kirby wrote:
> [ Resent with a reasonable subject and to lvs-devel :) ]
>
> Hello!
>
> I was noticing a significant amount of what seems/seemed to be
> destination lists with multiple entries with the lblcr LVS algorithm.
> While tracking it down, I think I stumbled over a mistake. In
> ip_vs_lblcr_full_check(), it appears the time check logic is reversed:
>
> for (i=0, j=tbl->rover; i<IP_VS_LBLCR_TAB_SIZE; i++) {
> j = (j + 1) & IP_VS_LBLCR_TAB_MASK;
>
> write_lock(&svc->sched_lock);
> list_for_each_entry_safe(en, nxt, &tbl->bucket[j], list) {
> if (time_after(en->lastuse+sysctl_ip_vs_lblcr_expiration,
> now))
> continue;
>
> ip_vs_lblcr_free(en);
> atomic_dec(&tbl->entries);
> }
> write_unlock(&svc->sched_lock);
> }
>
> Shouldn't this be "time_before"? It seems that it currently nukes all
> recently-used entries every time this function is called, which seems to
> be every 30 minutes, rather than removing the not-recently-used ones.
>
> If my reading is correct, this patch should fix it. Am I missing
> something?
>
> Cheers,
>
> Simon-
>
> diff --git a/net/netfilter/ipvs/ip_vs_lblcr.c b/net/netfilter/ipvs/ip_vs_lblcr.c
> index 715b57f..937743f 100644
> --- a/net/netfilter/ipvs/ip_vs_lblcr.c
> +++ b/net/netfilter/ipvs/ip_vs_lblcr.c
> @@ -432,7 +432,7 @@ static inline void ip_vs_lblcr_full_check(struct ip_vs_service *svc)
>
> write_lock(&svc->sched_lock);
> list_for_each_entry_safe(en, nxt, &tbl->bucket[j], list) {
> - if (time_after(en->lastuse+sysctl_ip_vs_lblcr_expiration,
> + if (time_before(en->lastuse+sysctl_ip_vs_lblcr_expiration,
> now))
> continue;
>
Hi Simon,
your analysis seems correct to me. Could you supply a Signed-off-line
with the patch and I'll see about getting it merged.
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] Fix ip_vs_lblcr_full_check to remove unused entries instead of used entries.
2009-11-05 1:35 ` Simon Horman
@ 2009-11-05 8:37 ` Simon Kirby
2009-11-05 9:43 ` Simon Kirby
0 siblings, 1 reply; 4+ messages in thread
From: Simon Kirby @ 2009-11-05 8:37 UTC (permalink / raw)
To: Simon Horman; +Cc: lvs-devel
Tested with real packets -- seems to work.
Signed-off-by: Simon Kirby <sim@hostway.ca>
---
net/netfilter/ipvs/ip_vs_lblcr.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/netfilter/ipvs/ip_vs_lblcr.c b/net/netfilter/ipvs/ip_vs_lblcr.c
index 715b57f..937743f 100644
--- a/net/netfilter/ipvs/ip_vs_lblcr.c
+++ b/net/netfilter/ipvs/ip_vs_lblcr.c
@@ -432,7 +432,7 @@ static inline void ip_vs_lblcr_full_check(struct ip_vs_service *svc)
write_lock(&svc->sched_lock);
list_for_each_entry_safe(en, nxt, &tbl->bucket[j], list) {
- if (time_after(en->lastuse+sysctl_ip_vs_lblcr_expiration,
+ if (time_before(en->lastuse+sysctl_ip_vs_lblcr_expiration,
now))
continue;
--
1.6.5.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix ip_vs_lblcr_full_check to remove unused entries instead of used entries.
2009-11-05 8:37 ` [PATCH] Fix ip_vs_lblcr_full_check to remove unused entries instead of used entries Simon Kirby
@ 2009-11-05 9:43 ` Simon Kirby
0 siblings, 0 replies; 4+ messages in thread
From: Simon Kirby @ 2009-11-05 9:43 UTC (permalink / raw)
To: Simon Horman; +Cc: lvs-devel
D'oh. Don't apply this -- the argument order makes the original correct.
Simon-
On Thu, Nov 05, 2009 at 12:37:16AM -0800, Simon Kirby wrote:
> Tested with real packets -- seems to work.
>
> Signed-off-by: Simon Kirby <sim@hostway.ca>
>
> ---
> net/netfilter/ipvs/ip_vs_lblcr.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/net/netfilter/ipvs/ip_vs_lblcr.c b/net/netfilter/ipvs/ip_vs_lblcr.c
> index 715b57f..937743f 100644
> --- a/net/netfilter/ipvs/ip_vs_lblcr.c
> +++ b/net/netfilter/ipvs/ip_vs_lblcr.c
> @@ -432,7 +432,7 @@ static inline void ip_vs_lblcr_full_check(struct ip_vs_service *svc)
>
> write_lock(&svc->sched_lock);
> list_for_each_entry_safe(en, nxt, &tbl->bucket[j], list) {
> - if (time_after(en->lastuse+sysctl_ip_vs_lblcr_expiration,
> + if (time_before(en->lastuse+sysctl_ip_vs_lblcr_expiration,
> now))
> continue;
>
> --
> 1.6.5.2
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2009-11-05 9:43 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-11-05 0:38 ip_vs_lblcr logic error causing table flushing Simon Kirby
2009-11-05 1:35 ` Simon Horman
2009-11-05 8:37 ` [PATCH] Fix ip_vs_lblcr_full_check to remove unused entries instead of used entries Simon Kirby
2009-11-05 9:43 ` Simon Kirby
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.