| Message ID | 20231107143905.1288261-1-ekovalev.off@gmail.com |
|---|---|
| State | Accepted |
| Delegated to: | Dumitru Ceara |
| Headers | show |
| Series | [ovs-dev,1/2] pinctrl: send RST instead of RST_ACK bit for lb hc | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
Thank you for the submission Evgenii, Acked-by: Mark Michelson <mmichels@redhat.com> I have one minor nit below. Whoever merges this change can correct it. On 11/7/23 09:39, Evgenii Kovalev wrote: > Currently an ovn-controller send RST_ACK after receive SYN_ACK in > load balancer health check, but for a windows as load balancer > backend this behavior unexcpected and windows send SYN_ACK like a > tcp retransmission after every RST_ACK, finaly after few reties windows > send RST bit to ovn-controller. In this case the svc_mon status flapping > between online and offline, which triger the ovn-northd for each changes > and increase CPU-usage ovn-northd to 100%. > > This patch fixing this behavior for windows and doesn't affect linux with > send RST instead of RST_ACK after received SYN_ACK. > > In RFC1122 section 4.2.2.13 described the ways for closing a connection: > https://datatracker.ietf.org/doc/html/rfc1122#page-87 > A TCP connection may terminate in two ways: (1) the normal > TCP close sequence using a FIN handshake, and (2) an "abort" > in which one or more RST segments are sent and the > connection state is immediately discarded. > For example, nmap tool use the same logic with RST for scaning ports. > > Signed-off-by: Evgenii Kovalev <ekovalev.off@gmail.com> > --- > controller/pinctrl.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/controller/pinctrl.c b/controller/pinctrl.c > index 3c1cecfde..f98f6d70c 100644 > --- a/controller/pinctrl.c > +++ b/controller/pinctrl.c > @@ -7818,7 +7818,6 @@ pinctrl_handle_tcp_svc_check(struct rconn *swconn, > return false; > } > > - uint32_t tcp_seq = ntohl(get_16aligned_be32(&th->tcp_seq)); > uint32_t tcp_ack = ntohl(get_16aligned_be32(&th->tcp_ack)); > > if (th->tcp_dst != svc_mon->tp_src) { > @@ -7836,9 +7835,9 @@ pinctrl_handle_tcp_svc_check(struct rconn *swconn, > svc_mon->state = SVC_MON_S_ONLINE; > > /* Send RST-ACK packet. */ This comment needs to be updated since we are not sending a RST-ACK any more. > - svc_monitor_send_tcp_health_check__(swconn, svc_mon, TCP_RST | TCP_ACK, > - htonl(tcp_ack + 1), > - htonl(tcp_seq + 1), th->tcp_dst); > + svc_monitor_send_tcp_health_check__(swconn, svc_mon, TCP_RST, > + htonl(tcp_ack), > + htonl(0), th->tcp_dst); > /* Calculate next_send_time. */ > svc_mon->next_send_time = time_msec() + svc_mon->interval; > return true; > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On 11/8/23 18:54, Mark Michelson wrote: > Thank you for the submission Evgenii, > > Acked-by: Mark Michelson <mmichels@redhat.com> > Thanks, I applied this to main and backported it to all stable branches (down to 22.03). > I have one minor nit below. Whoever merges this change can correct it. > Unfortunately I missed this but I posted a patch now for it: https://patchwork.ozlabs.org/project/ovn/patch/20231120194522.1818750-1-dceara@redhat.com/ Regards, Dumitru > On 11/7/23 09:39, Evgenii Kovalev wrote: >> Currently an ovn-controller send RST_ACK after receive SYN_ACK in >> load balancer health check, but for a windows as load balancer >> backend this behavior unexcpected and windows send SYN_ACK like a >> tcp retransmission after every RST_ACK, finaly after few reties windows >> send RST bit to ovn-controller. In this case the svc_mon status flapping >> between online and offline, which triger the ovn-northd for each changes >> and increase CPU-usage ovn-northd to 100%. >> >> This patch fixing this behavior for windows and doesn't affect linux with >> send RST instead of RST_ACK after received SYN_ACK. >> >> In RFC1122 section 4.2.2.13 described the ways for closing a connection: >> https://datatracker.ietf.org/doc/html/rfc1122#page-87 >> A TCP connection may terminate in two ways: (1) the normal >> TCP close sequence using a FIN handshake, and (2) an "abort" >> in which one or more RST segments are sent and the >> connection state is immediately discarded. >> For example, nmap tool use the same logic with RST for scaning ports. >> >> Signed-off-by: Evgenii Kovalev <ekovalev.off@gmail.com> >> --- >> controller/pinctrl.c | 7 +++---- >> 1 file changed, 3 insertions(+), 4 deletions(-) >> >> diff --git a/controller/pinctrl.c b/controller/pinctrl.c >> index 3c1cecfde..f98f6d70c 100644 >> --- a/controller/pinctrl.c >> +++ b/controller/pinctrl.c >> @@ -7818,7 +7818,6 @@ pinctrl_handle_tcp_svc_check(struct rconn *swconn, >> return false; >> } >> - uint32_t tcp_seq = ntohl(get_16aligned_be32(&th->tcp_seq)); >> uint32_t tcp_ack = ntohl(get_16aligned_be32(&th->tcp_ack)); >> if (th->tcp_dst != svc_mon->tp_src) { >> @@ -7836,9 +7835,9 @@ pinctrl_handle_tcp_svc_check(struct rconn *swconn, >> svc_mon->state = SVC_MON_S_ONLINE; >> /* Send RST-ACK packet. */ > > This comment needs to be updated since we are not sending a RST-ACK any > more. > >> - svc_monitor_send_tcp_health_check__(swconn, svc_mon, TCP_RST >> | TCP_ACK, >> - htonl(tcp_ack + 1), >> - htonl(tcp_seq + 1), >> th->tcp_dst); >> + svc_monitor_send_tcp_health_check__(swconn, svc_mon, TCP_RST, >> + htonl(tcp_ack), >> + htonl(0), th->tcp_dst); >> /* Calculate next_send_time. */ >> svc_mon->next_send_time = time_msec() + svc_mon->interval; >> return true; >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
diff --git a/controller/pinctrl.c b/controller/pinctrl.c index 3c1cecfde..f98f6d70c 100644 --- a/controller/pinctrl.c +++ b/controller/pinctrl.c @@ -7818,7 +7818,6 @@ pinctrl_handle_tcp_svc_check(struct rconn *swconn, return false; } - uint32_t tcp_seq = ntohl(get_16aligned_be32(&th->tcp_seq)); uint32_t tcp_ack = ntohl(get_16aligned_be32(&th->tcp_ack)); if (th->tcp_dst != svc_mon->tp_src) { @@ -7836,9 +7835,9 @@ pinctrl_handle_tcp_svc_check(struct rconn *swconn, svc_mon->state = SVC_MON_S_ONLINE; /* Send RST-ACK packet. */ - svc_monitor_send_tcp_health_check__(swconn, svc_mon, TCP_RST | TCP_ACK, - htonl(tcp_ack + 1), - htonl(tcp_seq + 1), th->tcp_dst); + svc_monitor_send_tcp_health_check__(swconn, svc_mon, TCP_RST, + htonl(tcp_ack), + htonl(0), th->tcp_dst); /* Calculate next_send_time. */ svc_mon->next_send_time = time_msec() + svc_mon->interval; return true;
Currently an ovn-controller send RST_ACK after receive SYN_ACK in load balancer health check, but for a windows as load balancer backend this behavior unexcpected and windows send SYN_ACK like a tcp retransmission after every RST_ACK, finaly after few reties windows send RST bit to ovn-controller. In this case the svc_mon status flapping between online and offline, which triger the ovn-northd for each changes and increase CPU-usage ovn-northd to 100%. This patch fixing this behavior for windows and doesn't affect linux with send RST instead of RST_ACK after received SYN_ACK. In RFC1122 section 4.2.2.13 described the ways for closing a connection: https://datatracker.ietf.org/doc/html/rfc1122#page-87 A TCP connection may terminate in two ways: (1) the normal TCP close sequence using a FIN handshake, and (2) an "abort" in which one or more RST segments are sent and the connection state is immediately discarded. For example, nmap tool use the same logic with RST for scaning ports. Signed-off-by: Evgenii Kovalev <ekovalev.off@gmail.com> --- controller/pinctrl.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)