Re: [Qemu-devel] [PATCH v2 1/4] colo: remove the incorrect if conditions in colo_compare_packet_tcp()

[Zhang Chen]

This patch should be moved behind the patch with payload comparition.
People needs look your changes to understand why we need this patch.

Thanks
Zhang Chen


On Wed, Dec 6, 2017 at 9:57 AM, Mao Zhongyi <address@hidden>
wrote:

> The th_off field specifies the size of the TCP header, if th_off > 5,
> it only means this tcp packet have options field. Regardless of whether
> the packet carries option fields, it doesn't effect us to obtain the
> length of the header use a general method. So there is no need to add
> the limiting conditions(if (th_off > 5)). In addtion, here we just focus
> on the payload, don't care about the option field. So removed the
> 'if (th_off > 5)' and 'if (ptcp->th_sum == stcp->th_sum)' conditions
> together.
>
> Cc: Zhang Chen <address@hidden>
> Cc: Li Zhijian <address@hidden>
> Cc: Jason Wang <address@hidden>
>
> Signed-off-by: Mao Zhongyi <address@hidden>
> Signed-off-by: Li Zhijian <address@hidden>
> ---
>  net/colo-compare.c | 31 ++++++++++++-------------------
>  1 file changed, 12 insertions(+), 19 deletions(-)
>
> diff --git a/net/colo-compare.c b/net/colo-compare.c
> index 1ce195f..0afb5f0 100644
> --- a/net/colo-compare.c
> +++ b/net/colo-compare.c
> @@ -271,26 +271,19 @@ static int colo_packet_compare_tcp(Packet *spkt,
> Packet *ppkt)
>       * the secondary guest's timestamp. COLO just focus on payload,
>       * so we just need skip this field.
>       */
> -    if (ptcp->th_off > 5) {
> -        ptrdiff_t ptcp_offset, stcp_offset;
> +    ptrdiff_t ptcp_offset, stcp_offset;
>
> -        ptcp_offset = ppkt->transport_header - (uint8_t *)ppkt->data
> -                      + (ptcp->th_off * 4) - ppkt->vnet_hdr_len;
> -        stcp_offset = spkt->transport_header - (uint8_t *)spkt->data
> -                      + (stcp->th_off * 4) - spkt->vnet_hdr_len;
> -
> -        /*
> -         * When network is busy, some tcp options(like sack) will
> unpredictable
> -         * occur in primary side or secondary side. it will make packet
> size
> -         * not same, but the two packet's payload is identical. colo just
> -         * care about packet payload, so we skip the option field.
> -         */
> -        res = colo_packet_compare_common(ppkt, spkt, ptcp_offset,
> stcp_offset);
> -    } else if (ptcp->th_sum == stcp->th_sum) {
> -        res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN, ETH_HLEN);
> -    } else {
> -        res = -1;
> -    }
> +    ptcp_offset = ppkt->transport_header - (uint8_t *)ppkt->data
> +                  + (ptcp->th_off * 4) - ppkt->vnet_hdr_len;
> +    stcp_offset = spkt->transport_header - (uint8_t *)spkt->data
> +                  + (stcp->th_off * 4) - spkt->vnet_hdr_len;
> +    /*
> +     * When network is busy, some tcp options(like sack) will
> unpredictable
> +     * occur in primary side or secondary side. it will make packet size
> +     * not same, but the two packet's payload is identical. colo just
> +     * care about packet payload, so we skip the option field.
> +     */
> +    res = colo_packet_compare_common(ppkt, spkt, ptcp_offset,
> stcp_offset);
>
>      if (res != 0 &&
>          trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE)) {
> --
> 2.9.4
>
>
>
>