* [PATCH net-next] ipvs: allow connection reuse for unconfirmed conntrack
@ 2020-07-01 15:17 ` Julian Anastasov
0 siblings, 0 replies; 5+ messages in thread
From: Julian Anastasov @ 2020-07-01 15:17 UTC (permalink / raw)
To: Simon Horman; +Cc: lvs-devel, Pablo Neira Ayuso, netfilter-devel, YangYuxi
YangYuxi is reporting that connection reuse
is causing one-second delay when SYN hits
existing connection in TIME_WAIT state.
Such delay was added to give time to expire
both the IPVS connection and the corresponding
conntrack. This was considered a rare case
at that time but it is causing problem for
some environments such as Kubernetes.
As nf_conntrack_tcp_packet() can decide to
release the conntrack in TIME_WAIT state and
to replace it with a fresh NEW conntrack, we
can use this to allow rescheduling just by
tuning our check: if the conntrack is
confirmed we can not schedule it to different
real server and the one-second delay still
applies but if new conntrack was created,
we are free to select new real server without
any delays.
YangYuxi lists some of the problem reports:
- One second connection delay in masquerading mode:
https://marc.info/?t=151683118100004&r=1&w=2
- IPVS low throughput #70747
https://github.com/kubernetes/kubernetes/issues/70747
- Apache Bench can fill up ipvs service proxy in seconds #544
https://github.com/cloudnativelabs/kube-router/issues/544
- Additional 1s latency in `host -> service IP -> pod`
https://github.com/kubernetes/kubernetes/issues/90854
Fixes: f719e3754ee2 ("ipvs: drop first packet to redirect conntrack")
Co-developed-by: YangYuxi <yx.atom1@gmail.com>
Signed-off-by: YangYuxi <yx.atom1@gmail.com>
Signed-off-by: Julian Anastasov <ja@ssi.bg>
---
include/net/ip_vs.h | 10 ++++------
net/netfilter/ipvs/ip_vs_core.c | 12 +++++++-----
2 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
index 83be2d93b407..fe96aa462d05 100644
--- a/include/net/ip_vs.h
+++ b/include/net/ip_vs.h
@@ -1624,18 +1624,16 @@ static inline void ip_vs_conn_drop_conntrack(struct ip_vs_conn *cp)
}
#endif /* CONFIG_IP_VS_NFCT */
-/* Really using conntrack? */
-static inline bool ip_vs_conn_uses_conntrack(struct ip_vs_conn *cp,
- struct sk_buff *skb)
+/* Using old conntrack that can not be redirected to another real server? */
+static inline bool ip_vs_conn_uses_old_conntrack(struct ip_vs_conn *cp,
+ struct sk_buff *skb)
{
#ifdef CONFIG_IP_VS_NFCT
enum ip_conntrack_info ctinfo;
struct nf_conn *ct;
- if (!(cp->flags & IP_VS_CONN_F_NFCT))
- return false;
ct = nf_ct_get(skb, &ctinfo);
- if (ct)
+ if (ct && nf_ct_is_confirmed(ct))
return true;
#endif
return false;
diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
index aa6a603a2425..517f6a2ac15a 100644
--- a/net/netfilter/ipvs/ip_vs_core.c
+++ b/net/netfilter/ipvs/ip_vs_core.c
@@ -2066,14 +2066,14 @@ ip_vs_in(struct netns_ipvs *ipvs, unsigned int hooknum, struct sk_buff *skb, int
conn_reuse_mode = sysctl_conn_reuse_mode(ipvs);
if (conn_reuse_mode && !iph.fragoffs && is_new_conn(skb, &iph) && cp) {
- bool uses_ct = false, resched = false;
+ bool old_ct = false, resched = false;
if (unlikely(sysctl_expire_nodest_conn(ipvs)) && cp->dest &&
unlikely(!atomic_read(&cp->dest->weight))) {
resched = true;
- uses_ct = ip_vs_conn_uses_conntrack(cp, skb);
+ old_ct = ip_vs_conn_uses_old_conntrack(cp, skb);
} else if (is_new_conn_expected(cp, conn_reuse_mode)) {
- uses_ct = ip_vs_conn_uses_conntrack(cp, skb);
+ old_ct = ip_vs_conn_uses_old_conntrack(cp, skb);
if (!atomic_read(&cp->n_control)) {
resched = true;
} else {
@@ -2081,15 +2081,17 @@ ip_vs_in(struct netns_ipvs *ipvs, unsigned int hooknum, struct sk_buff *skb, int
* that uses conntrack while it is still
* referenced by controlled connection(s).
*/
- resched = !uses_ct;
+ resched = !old_ct;
}
}
if (resched) {
+ if (!old_ct)
+ cp->flags &= ~IP_VS_CONN_F_NFCT;
if (!atomic_read(&cp->n_control))
ip_vs_conn_expire_now(cp);
__ip_vs_conn_put(cp);
- if (uses_ct)
+ if (old_ct)
return NF_DROP;
cp = NULL;
}
--
2.26.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH net-next] ipvs: allow connection reuse for unconfirmed conntrack
@ 2020-07-01 15:17 ` Julian Anastasov
0 siblings, 0 replies; 5+ messages in thread
From: Julian Anastasov @ 2020-07-01 15:17 UTC (permalink / raw)
To: Simon Horman; +Cc: lvs-devel, Pablo Neira Ayuso, netfilter-devel, YangYuxi
YangYuxi is reporting that connection reuse
is causing one-second delay when SYN hits
existing connection in TIME_WAIT state.
Such delay was added to give time to expire
both the IPVS connection and the corresponding
conntrack. This was considered a rare case
at that time but it is causing problem for
some environments such as Kubernetes.
As nf_conntrack_tcp_packet() can decide to
release the conntrack in TIME_WAIT state and
to replace it with a fresh NEW conntrack, we
can use this to allow rescheduling just by
tuning our check: if the conntrack is
confirmed we can not schedule it to different
real server and the one-second delay still
applies but if new conntrack was created,
we are free to select new real server without
any delays.
YangYuxi lists some of the problem reports:
- One second connection delay in masquerading mode:
https://marc.info/?t=151683118100004&r=1&w=2
- IPVS low throughput #70747
https://github.com/kubernetes/kubernetes/issues/70747
- Apache Bench can fill up ipvs service proxy in seconds #544
https://github.com/cloudnativelabs/kube-router/issues/544
- Additional 1s latency in `host -> service IP -> pod`
https://github.com/kubernetes/kubernetes/issues/90854
Fixes: f719e3754ee2 ("ipvs: drop first packet to redirect conntrack")
Co-developed-by: YangYuxi <yx.atom1@gmail.com>
Signed-off-by: YangYuxi <yx.atom1@gmail.com>
Signed-off-by: Julian Anastasov <ja@ssi.bg>
---
include/net/ip_vs.h | 10 ++++------
net/netfilter/ipvs/ip_vs_core.c | 12 +++++++-----
2 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
index 83be2d93b407..fe96aa462d05 100644
--- a/include/net/ip_vs.h
+++ b/include/net/ip_vs.h
@@ -1624,18 +1624,16 @@ static inline void ip_vs_conn_drop_conntrack(struct ip_vs_conn *cp)
}
#endif /* CONFIG_IP_VS_NFCT */
-/* Really using conntrack? */
-static inline bool ip_vs_conn_uses_conntrack(struct ip_vs_conn *cp,
- struct sk_buff *skb)
+/* Using old conntrack that can not be redirected to another real server? */
+static inline bool ip_vs_conn_uses_old_conntrack(struct ip_vs_conn *cp,
+ struct sk_buff *skb)
{
#ifdef CONFIG_IP_VS_NFCT
enum ip_conntrack_info ctinfo;
struct nf_conn *ct;
- if (!(cp->flags & IP_VS_CONN_F_NFCT))
- return false;
ct = nf_ct_get(skb, &ctinfo);
- if (ct)
+ if (ct && nf_ct_is_confirmed(ct))
return true;
#endif
return false;
diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
index aa6a603a2425..517f6a2ac15a 100644
--- a/net/netfilter/ipvs/ip_vs_core.c
+++ b/net/netfilter/ipvs/ip_vs_core.c
@@ -2066,14 +2066,14 @@ ip_vs_in(struct netns_ipvs *ipvs, unsigned int hooknum, struct sk_buff *skb, int
conn_reuse_mode = sysctl_conn_reuse_mode(ipvs);
if (conn_reuse_mode && !iph.fragoffs && is_new_conn(skb, &iph) && cp) {
- bool uses_ct = false, resched = false;
+ bool old_ct = false, resched = false;
if (unlikely(sysctl_expire_nodest_conn(ipvs)) && cp->dest &&
unlikely(!atomic_read(&cp->dest->weight))) {
resched = true;
- uses_ct = ip_vs_conn_uses_conntrack(cp, skb);
+ old_ct = ip_vs_conn_uses_old_conntrack(cp, skb);
} else if (is_new_conn_expected(cp, conn_reuse_mode)) {
- uses_ct = ip_vs_conn_uses_conntrack(cp, skb);
+ old_ct = ip_vs_conn_uses_old_conntrack(cp, skb);
if (!atomic_read(&cp->n_control)) {
resched = true;
} else {
@@ -2081,15 +2081,17 @@ ip_vs_in(struct netns_ipvs *ipvs, unsigned int hooknum, struct sk_buff *skb, int
* that uses conntrack while it is still
* referenced by controlled connection(s).
*/
- resched = !uses_ct;
+ resched = !old_ct;
}
}
if (resched) {
+ if (!old_ct)
+ cp->flags &= ~IP_VS_CONN_F_NFCT;
if (!atomic_read(&cp->n_control))
ip_vs_conn_expire_now(cp);
__ip_vs_conn_put(cp);
- if (uses_ct)
+ if (old_ct)
return NF_DROP;
cp = NULL;
}
--
2.26.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] ipvs: allow connection reuse for unconfirmed conntrack
2020-07-01 15:17 ` Julian Anastasov
@ 2020-07-02 9:18 ` Simon Horman
-1 siblings, 0 replies; 5+ messages in thread
From: Simon Horman @ 2020-07-02 9:18 UTC (permalink / raw)
To: Julian Anastasov, Pablo Neira Ayuso; +Cc: lvs-devel, netfilter-devel, YangYuxi
On Wed, Jul 01, 2020 at 06:17:19PM +0300, Julian Anastasov wrote:
> YangYuxi is reporting that connection reuse
> is causing one-second delay when SYN hits
> existing connection in TIME_WAIT state.
> Such delay was added to give time to expire
> both the IPVS connection and the corresponding
> conntrack. This was considered a rare case
> at that time but it is causing problem for
> some environments such as Kubernetes.
>
> As nf_conntrack_tcp_packet() can decide to
> release the conntrack in TIME_WAIT state and
> to replace it with a fresh NEW conntrack, we
> can use this to allow rescheduling just by
> tuning our check: if the conntrack is
> confirmed we can not schedule it to different
> real server and the one-second delay still
> applies but if new conntrack was created,
> we are free to select new real server without
> any delays.
>
> YangYuxi lists some of the problem reports:
>
> - One second connection delay in masquerading mode:
> https://marc.info/?t=151683118100004&r=1&w=2
>
> - IPVS low throughput #70747
> https://github.com/kubernetes/kubernetes/issues/70747
>
> - Apache Bench can fill up ipvs service proxy in seconds #544
> https://github.com/cloudnativelabs/kube-router/issues/544
>
> - Additional 1s latency in `host -> service IP -> pod`
> https://github.com/kubernetes/kubernetes/issues/90854
>
> Fixes: f719e3754ee2 ("ipvs: drop first packet to redirect conntrack")
> Co-developed-by: YangYuxi <yx.atom1@gmail.com>
> Signed-off-by: YangYuxi <yx.atom1@gmail.com>
> Signed-off-by: Julian Anastasov <ja@ssi.bg>
Thanks, this looks good to me.
Reviewed-by: Simon Horman <horms@verge.net.au>
Pablo, could you consider applying this to nf-next?
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] ipvs: allow connection reuse for unconfirmed conntrack
@ 2020-07-02 9:18 ` Simon Horman
0 siblings, 0 replies; 5+ messages in thread
From: Simon Horman @ 2020-07-02 9:18 UTC (permalink / raw)
To: Julian Anastasov, Pablo Neira Ayuso; +Cc: lvs-devel, netfilter-devel, YangYuxi
On Wed, Jul 01, 2020 at 06:17:19PM +0300, Julian Anastasov wrote:
> YangYuxi is reporting that connection reuse
> is causing one-second delay when SYN hits
> existing connection in TIME_WAIT state.
> Such delay was added to give time to expire
> both the IPVS connection and the corresponding
> conntrack. This was considered a rare case
> at that time but it is causing problem for
> some environments such as Kubernetes.
>
> As nf_conntrack_tcp_packet() can decide to
> release the conntrack in TIME_WAIT state and
> to replace it with a fresh NEW conntrack, we
> can use this to allow rescheduling just by
> tuning our check: if the conntrack is
> confirmed we can not schedule it to different
> real server and the one-second delay still
> applies but if new conntrack was created,
> we are free to select new real server without
> any delays.
>
> YangYuxi lists some of the problem reports:
>
> - One second connection delay in masquerading mode:
> https://marc.info/?t=151683118100004&r=1&w=2
>
> - IPVS low throughput #70747
> https://github.com/kubernetes/kubernetes/issues/70747
>
> - Apache Bench can fill up ipvs service proxy in seconds #544
> https://github.com/cloudnativelabs/kube-router/issues/544
>
> - Additional 1s latency in `host -> service IP -> pod`
> https://github.com/kubernetes/kubernetes/issues/90854
>
> Fixes: f719e3754ee2 ("ipvs: drop first packet to redirect conntrack")
> Co-developed-by: YangYuxi <yx.atom1@gmail.com>
> Signed-off-by: YangYuxi <yx.atom1@gmail.com>
> Signed-off-by: Julian Anastasov <ja@ssi.bg>
Thanks, this looks good to me.
Reviewed-by: Simon Horman <horms@verge.net.au>
Pablo, could you consider applying this to nf-next?
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] ipvs: allow connection reuse for unconfirmed conntrack
2020-07-02 9:18 ` Simon Horman
(?)
@ 2020-07-03 23:17 ` Pablo Neira Ayuso
-1 siblings, 0 replies; 5+ messages in thread
From: Pablo Neira Ayuso @ 2020-07-03 23:17 UTC (permalink / raw)
To: Simon Horman; +Cc: Julian Anastasov, lvs-devel, netfilter-devel, YangYuxi
On Thu, Jul 02, 2020 at 11:18:29AM +0200, Simon Horman wrote:
> On Wed, Jul 01, 2020 at 06:17:19PM +0300, Julian Anastasov wrote:
> > YangYuxi is reporting that connection reuse
> > is causing one-second delay when SYN hits
> > existing connection in TIME_WAIT state.
> > Such delay was added to give time to expire
> > both the IPVS connection and the corresponding
> > conntrack. This was considered a rare case
> > at that time but it is causing problem for
> > some environments such as Kubernetes.
> >
> > As nf_conntrack_tcp_packet() can decide to
> > release the conntrack in TIME_WAIT state and
> > to replace it with a fresh NEW conntrack, we
> > can use this to allow rescheduling just by
> > tuning our check: if the conntrack is
> > confirmed we can not schedule it to different
> > real server and the one-second delay still
> > applies but if new conntrack was created,
> > we are free to select new real server without
> > any delays.
> >
> > YangYuxi lists some of the problem reports:
> >
> > - One second connection delay in masquerading mode:
> > https://marc.info/?t=151683118100004&r=1&w=2
> >
> > - IPVS low throughput #70747
> > https://github.com/kubernetes/kubernetes/issues/70747
> >
> > - Apache Bench can fill up ipvs service proxy in seconds #544
> > https://github.com/cloudnativelabs/kube-router/issues/544
> >
> > - Additional 1s latency in `host -> service IP -> pod`
> > https://github.com/kubernetes/kubernetes/issues/90854
> >
> > Fixes: f719e3754ee2 ("ipvs: drop first packet to redirect conntrack")
> > Co-developed-by: YangYuxi <yx.atom1@gmail.com>
> > Signed-off-by: YangYuxi <yx.atom1@gmail.com>
> > Signed-off-by: Julian Anastasov <ja@ssi.bg>
>
> Thanks, this looks good to me.
>
> Reviewed-by: Simon Horman <horms@verge.net.au>
>
> Pablo, could you consider applying this to nf-next?
Applied, thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-07-03 23:18 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-01 15:17 [PATCH net-next] ipvs: allow connection reuse for unconfirmed conntrack Julian Anastasov
2020-07-01 15:17 ` Julian Anastasov
2020-07-02 9:18 ` Simon Horman
2020-07-02 9:18 ` Simon Horman
2020-07-03 23:17 ` Pablo Neira Ayuso
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.