[ovs-dev] Test result. RE: [patch v1] conntrack-tcp: Handle tcp session reuse.

[Darrell Ball]

On Tue, Mar 6, 2018 at 5:23 PM, Yangxiaoliang (Neo) <
david.yangxiaoliang at huawei.com> wrote:

> Hi Darrell,
>
> I have tested VM migration with this patch for several times. And this
> patch can avoid stopping the TCP stream,


Thanks for testing and reporting Neo



> but the issue is that the TCP stream will suspend for  a big number of
> seconds after migration ( for example, more than 100 seconds on 8Gbps). I
> think users will not be satisfied this issue.


Thanks, could you please report:
1/ How you carry out the migration ?
2/ The packets and their timing seen coming the endpoints; the rst packets
are of particular interest.



>
> Can we enlarge the range that is saved by sequence tracking to be more
> permissive to decrease the time.


The problem with enlarging the range is the common case becomes too
permissive and we loose the benefit of sequence tracking.



> Or maybe in the future we will solve this issue completely, for now it's
> not recommended to migrate VM with a big network throughput. Or any other
> idea ?


> Thanks.
>
> -----Original Message-----
> From: ovs-dev-bounces at openvswitch.org [mailto:ovs-dev-bounces@
> openvswitch.org] On Behalf Of Darrell Ball
> Sent: Thursday, March 01, 2018 3:26 PM
> To: dlu998 at gmail.com; dev at openvswitch.org
> Subject: [ovs-dev] [patch v1] conntrack-tcp: Handle tcp session reuse.
>
> Fix tcp sequence tracking for session reuse cases.  This can happen, for
> example by doing VM migration, where sequence tracking needs to be more
> permissive.  The solution is to be more permissive for session restart and
> session start only.  We don't differentiate session start here where we
> could be more strict, although we could, because the gain in protection is
> almost zero and the code modularity would be lessened and code complexity
> increased.
> This issue originates in release 2.7.
>
> Signed-off-by: Darrell Ball <dlu998 at gmail.com>
> ---
>  lib/conntrack-tcp.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/lib/conntrack-tcp.c b/lib/conntrack-tcp.c index
> 04460c3..a0ddd65 100644
> --- a/lib/conntrack-tcp.c
> +++ b/lib/conntrack-tcp.c
> @@ -160,7 +160,6 @@ tcp_conn_update(struct conn *conn_, struct
> conntrack_bucket *ctb,
>      uint16_t win = ntohs(tcp->tcp_winsz);
>      uint32_t ack, end, seq, orig_seq;
>      uint32_t p_len = tcp_payload_length(pkt);
> -    int ackskew;
>
>      if (tcp_invalid_flags(tcp_flags)) {
>          return CT_UPDATE_INVALID;
> @@ -195,11 +194,11 @@ tcp_conn_update(struct conn *conn_, struct
> conntrack_bucket *ctb,
>       */
>
>      orig_seq = seq = ntohl(get_16aligned_be32(&tcp->tcp_seq));
> +    bool check_ackskew = true;
>      if (src->state < CT_DPIF_TCPS_SYN_SENT) {
>          /* First packet from this end. Set its state */
>
>          ack = ntohl(get_16aligned_be32(&tcp->tcp_ack));
> -
>          end = seq + p_len;
>          if (tcp_flags & TCP_SYN) {
>              end++;
> @@ -232,6 +231,7 @@ tcp_conn_update(struct conn *conn_, struct
> conntrack_bucket *ctb,
>          if (src->seqhi == 1
>                  || SEQ_GEQ(end + MAX(1, dst->max_win << dws),
> src->seqhi)) {
>              src->seqhi = end + MAX(1, dst->max_win << dws);
> +            check_ackskew = false;
>          }
>          if (win > src->max_win) {
>              src->max_win = win;
> @@ -265,7 +265,13 @@ tcp_conn_update(struct conn *conn_, struct
> conntrack_bucket *ctb,
>          end = seq;
>      }
>
> -    ackskew = dst->seqlo - ack;
> +    int ackskew;
> +    if (check_ackskew) {
> +        ackskew = dst->seqlo - ack;
> +    } else {
> +        ackskew = 0;
> +    }
> +
>  #define MAXACKWINDOW (0xffff + 1500)    /* 1500 is an arbitrary fudge
> factor */
>      if (SEQ_GEQ(src->seqhi, end)
>          /* Last octet inside other's window space */
> --
> 1.9.1
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>