diff mbox

[V12,07/10] colo-compare: add TCP, UDP, ICMP packet comparison

Message ID 1471421428-26379-8-git-send-email-zhangchen.fnst@cn.fujitsu.com (mailing list archive)
State New, archived
Headers show

Commit Message

Zhang Chen Aug. 17, 2016, 8:10 a.m. UTC
We add TCP,UDP,ICMP packet comparison to replace
IP packet comparison. This can increase the
accuracy of the package comparison.
Less checkpoint more efficiency.

Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
---
 net/colo-compare.c | 152 +++++++++++++++++++++++++++++++++++++++++++++++++++--
 trace-events       |   4 ++
 2 files changed, 152 insertions(+), 4 deletions(-)

Comments

Jason Wang Aug. 31, 2016, 9:33 a.m. UTC | #1
On 2016年08月17日 16:10, Zhang Chen wrote:
> We add TCP,UDP,ICMP packet comparison to replace
> IP packet comparison. This can increase the
> accuracy of the package comparison.
> Less checkpoint more efficiency.
>
> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> ---
>   net/colo-compare.c | 152 +++++++++++++++++++++++++++++++++++++++++++++++++++--
>   trace-events       |   4 ++
>   2 files changed, 152 insertions(+), 4 deletions(-)
>
> diff --git a/net/colo-compare.c b/net/colo-compare.c
> index b90cf1f..0daefd9 100644
> --- a/net/colo-compare.c
> +++ b/net/colo-compare.c
> @@ -18,6 +18,7 @@
>   #include "qapi/qmp/qerror.h"
>   #include "qapi/error.h"
>   #include "net/net.h"
> +#include "net/eth.h"
>   #include "net/vhost_net.h"
>   #include "qom/object_interfaces.h"
>   #include "qemu/iov.h"
> @@ -179,9 +180,136 @@ static int colo_packet_compare(Packet *ppkt, Packet *spkt)
>       }
>   }
>   
> -static int colo_packet_compare_all(Packet *spkt, Packet *ppkt)
> +/*
> + * called from the compare thread on the primary
> + * for compare tcp packet
> + * compare_tcp copied from Dr. David Alan Gilbert's branch
> + */
> +static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
> +{
> +    struct tcphdr *ptcp, *stcp;
> +    int res;
> +    char *sdebug, *ddebug;
> +
> +    trace_colo_compare_main("compare tcp");
> +    if (ppkt->size != spkt->size) {
> +        if (trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
> +            trace_colo_compare_main("pkt size not same");
> +        }
> +        return -1;
> +    }
> +
> +    ptcp = (struct tcphdr *)ppkt->transport_header;
> +    stcp = (struct tcphdr *)spkt->transport_header;
> +
> +    if (ptcp->th_seq != stcp->th_seq) {
> +        if (trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
> +            trace_colo_compare_main("pkt tcp seq not same");
> +        }
> +        return -1;
> +    }
> +
> +    /*
> +     * The 'identification' field in the IP header is *very* random
> +     * it almost never matches.  Fudge this by ignoring differences in
> +     * unfragmented packets; they'll normally sort themselves out if different
> +     * anyway, and it should recover at the TCP level.
> +     * An alternative would be to get both the primary and secondary to rewrite
> +     * somehow; but that would need some sync traffic to sync the state
> +     */
> +    if (ntohs(ppkt->ip->ip_off) & IP_DF) {
> +        spkt->ip->ip_id = ppkt->ip->ip_id;
> +        /* and the sum will be different if the IDs were different */
> +        spkt->ip->ip_sum = ppkt->ip->ip_sum;
> +    }
> +
> +    res = memcmp(ppkt->data + ETH_HLEN, spkt->data + ETH_HLEN,
> +                (spkt->size - ETH_HLEN));

This may work but I worry about whether or not tagged packet can work 
here. Looks like parse_packet_early() can recognize vlan tag, but 
fill_connection_key() can not. This looks can result queuing wrong 
packets into wrong connection.

> +
> +    if (res != 0 && trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
> +        sdebug = strdup(inet_ntoa(ppkt->ip->ip_src));
> +        ddebug = strdup(inet_ntoa(ppkt->ip->ip_dst));
> +        fprintf(stderr, "%s: src/dst: %s/%s p: seq/ack=%u/%u"
> +        " s: seq/ack=%u/%u res=%d flags=%x/%x\n", __func__,
> +                   sdebug, ddebug,
> +                   ntohl(ptcp->th_seq), ntohl(ptcp->th_ack),
> +                   ntohl(stcp->th_seq), ntohl(stcp->th_ack),
> +                   res, ptcp->th_flags, stcp->th_flags);

I tend not mix using debug logs with tracepoints.

> +
> +        trace_colo_compare_tcp_miscompare("Primary len", ppkt->size);
> +        qemu_hexdump((char *)ppkt->data, stderr, "colo-compare", ppkt->size);
> +        trace_colo_compare_tcp_miscompare("Secondary len", spkt->size);
> +        qemu_hexdump((char *)spkt->data, stderr, "colo-compare", spkt->size);
> +
> +        g_free(sdebug);
> +        g_free(ddebug);
> +    }
> +
> +    return res;
> +}
> +
> +/*
> + * called from the compare thread on the primary
> + * for compare udp packet
> + */
> +static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt)
> +{
> +    int ret;
> +
> +    trace_colo_compare_main("compare udp");
> +    ret = colo_packet_compare(ppkt, spkt);
> +
> +    if (ret) {
> +        trace_colo_compare_udp_miscompare("primary pkt size", ppkt->size);
> +        qemu_hexdump((char *)ppkt->data, stderr, "colo-compare", ppkt->size);
> +        trace_colo_compare_udp_miscompare("Secondary pkt size", spkt->size);
> +        qemu_hexdump((char *)spkt->data, stderr, "colo-compare", spkt->size);
> +    }
> +
> +    return ret;
> +}
> +
> +/*
> + * called from the compare thread on the primary
> + * for compare icmp packet
> + */
> +static int colo_packet_compare_icmp(Packet *spkt, Packet *ppkt)
> +{
> +    int network_length;
> +
> +    trace_colo_compare_main("compare icmp");
> +    network_length = ppkt->ip->ip_hl * 4;
> +    if (ppkt->size != spkt->size ||
> +        ppkt->size < network_length + ETH_HLEN) {
> +        return -1;
> +    }
> +
> +    if (colo_packet_compare(ppkt, spkt)) {
> +        trace_colo_compare_icmp_miscompare("primary pkt size",
> +                                           ppkt->size);
> +        qemu_hexdump((char *)ppkt->data, stderr, "colo-compare",
> +                     ppkt->size);
> +        trace_colo_compare_icmp_miscompare("Secondary pkt size",
> +                                           spkt->size);
> +        qemu_hexdump((char *)spkt->data, stderr, "colo-compare",
> +                     spkt->size);
> +        return -1;
> +    } else {
> +        return 0;
> +    }
> +}
> +
> +/*
> + * called from the compare thread on the primary
> + * for compare other packet
> + */
> +static int colo_packet_compare_other(Packet *spkt, Packet *ppkt)
>   {
> -    trace_colo_compare_main("compare all");
> +    trace_colo_compare_main("compare other");
> +    trace_colo_compare_ip_info(ppkt->size, inet_ntoa(ppkt->ip->ip_src),
> +                               inet_ntoa(ppkt->ip->ip_dst), spkt->size,
> +                               inet_ntoa(spkt->ip->ip_src),
> +                               inet_ntoa(spkt->ip->ip_dst));
>       return colo_packet_compare(ppkt, spkt);
>   }
>   
> @@ -245,8 +373,24 @@ static void colo_compare_connection(void *opaque, void *user_data)
>       while (!g_queue_is_empty(&conn->primary_list) &&
>              !g_queue_is_empty(&conn->secondary_list)) {
>           pkt = g_queue_pop_tail(&conn->primary_list);
> -        result = g_queue_find_custom(&conn->secondary_list,
> -                              pkt, (GCompareFunc)colo_packet_compare_all);
> +        switch (conn->ip_proto) {
> +        case IPPROTO_TCP:
> +            result = g_queue_find_custom(&conn->secondary_list,
> +                     pkt, (GCompareFunc)colo_packet_compare_tcp);
> +            break;
> +        case IPPROTO_UDP:
> +            result = g_queue_find_custom(&conn->secondary_list,
> +                     pkt, (GCompareFunc)colo_packet_compare_udp);
> +            break;
> +        case IPPROTO_ICMP:
> +            result = g_queue_find_custom(&conn->secondary_list,
> +                     pkt, (GCompareFunc)colo_packet_compare_icmp);
> +            break;
> +        default:
> +            result = g_queue_find_custom(&conn->secondary_list,
> +                     pkt, (GCompareFunc)colo_packet_compare_other);
> +            break;
> +        }
>   
>           if (result) {
>               ret = compare_chr_send(s->chr_out, pkt->data, pkt->size);
> diff --git a/trace-events b/trace-events
> index 1537e91..ab22eb2 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -1919,5 +1919,9 @@ aspeed_vic_write(uint64_t offset, unsigned size, uint32_t data) "To 0x%" PRIx64
>   
>   # net/colo-compare.c
>   colo_compare_main(const char *chr) ": %s"
> +colo_compare_tcp_miscompare(const char *sta, int size) ": %s = %d"
> +colo_compare_udp_miscompare(const char *sta, int size) ": %s = %d"
> +colo_compare_icmp_miscompare(const char *sta, int size) ": %s = %d"
>   colo_compare_ip_info(int psize, const char *sta, const char *stb, int ssize, const char *stc, const char *std) "ppkt size = %d, ip_src = %s, ip_dst = %s, spkt size = %d, ip_src = %s, ip_dst = %s"
>   colo_old_packet_check_found(int64_t old_time) "%" PRId64
> +colo_compare_miscompare(void) ""
Zhang Chen Sept. 1, 2016, 5 a.m. UTC | #2
On 08/31/2016 05:33 PM, Jason Wang wrote:
>
>
> On 2016年08月17日 16:10, Zhang Chen wrote:
>> We add TCP,UDP,ICMP packet comparison to replace
>> IP packet comparison. This can increase the
>> accuracy of the package comparison.
>> Less checkpoint more efficiency.
>>
>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>> ---
>>   net/colo-compare.c | 152 
>> +++++++++++++++++++++++++++++++++++++++++++++++++++--
>>   trace-events       |   4 ++
>>   2 files changed, 152 insertions(+), 4 deletions(-)
>>
>> diff --git a/net/colo-compare.c b/net/colo-compare.c
>> index b90cf1f..0daefd9 100644
>> --- a/net/colo-compare.c
>> +++ b/net/colo-compare.c
>> @@ -18,6 +18,7 @@
>>   #include "qapi/qmp/qerror.h"
>>   #include "qapi/error.h"
>>   #include "net/net.h"
>> +#include "net/eth.h"
>>   #include "net/vhost_net.h"
>>   #include "qom/object_interfaces.h"
>>   #include "qemu/iov.h"
>> @@ -179,9 +180,136 @@ static int colo_packet_compare(Packet *ppkt, 
>> Packet *spkt)
>>       }
>>   }
>>   -static int colo_packet_compare_all(Packet *spkt, Packet *ppkt)
>> +/*
>> + * called from the compare thread on the primary
>> + * for compare tcp packet
>> + * compare_tcp copied from Dr. David Alan Gilbert's branch
>> + */
>> +static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
>> +{
>> +    struct tcphdr *ptcp, *stcp;
>> +    int res;
>> +    char *sdebug, *ddebug;
>> +
>> +    trace_colo_compare_main("compare tcp");
>> +    if (ppkt->size != spkt->size) {
>> +        if (trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
>> +            trace_colo_compare_main("pkt size not same");
>> +        }
>> +        return -1;
>> +    }
>> +
>> +    ptcp = (struct tcphdr *)ppkt->transport_header;
>> +    stcp = (struct tcphdr *)spkt->transport_header;
>> +
>> +    if (ptcp->th_seq != stcp->th_seq) {
>> +        if (trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
>> +            trace_colo_compare_main("pkt tcp seq not same");
>> +        }
>> +        return -1;
>> +    }
>> +
>> +    /*
>> +     * The 'identification' field in the IP header is *very* random
>> +     * it almost never matches.  Fudge this by ignoring differences in
>> +     * unfragmented packets; they'll normally sort themselves out if 
>> different
>> +     * anyway, and it should recover at the TCP level.
>> +     * An alternative would be to get both the primary and secondary 
>> to rewrite
>> +     * somehow; but that would need some sync traffic to sync the state
>> +     */
>> +    if (ntohs(ppkt->ip->ip_off) & IP_DF) {
>> +        spkt->ip->ip_id = ppkt->ip->ip_id;
>> +        /* and the sum will be different if the IDs were different */
>> +        spkt->ip->ip_sum = ppkt->ip->ip_sum;
>> +    }
>> +
>> +    res = memcmp(ppkt->data + ETH_HLEN, spkt->data + ETH_HLEN,
>> +                (spkt->size - ETH_HLEN));
>
> This may work but I worry about whether or not tagged packet can work 
> here. Looks like parse_packet_early() can recognize vlan tag, but 
> fill_connection_key() can not. This looks can result queuing wrong 
> packets into wrong connection.

Currently COLO proxy can't support vlan, we will add this feature in the 
future.

>
>> +
>> +    if (res != 0 && 
>> trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
>> +        sdebug = strdup(inet_ntoa(ppkt->ip->ip_src));
>> +        ddebug = strdup(inet_ntoa(ppkt->ip->ip_dst));
>> +        fprintf(stderr, "%s: src/dst: %s/%s p: seq/ack=%u/%u"
>> +        " s: seq/ack=%u/%u res=%d flags=%x/%x\n", __func__,
>> +                   sdebug, ddebug,
>> +                   ntohl(ptcp->th_seq), ntohl(ptcp->th_ack),
>> +                   ntohl(stcp->th_seq), ntohl(stcp->th_ack),
>> +                   res, ptcp->th_flags, stcp->th_flags);
>
> I tend not mix using debug logs with tracepoints.

OK, I will change trace_colo_compare_tcp_miscompare() to fprintf() here.

Thanks
Zhang Chen

>
>> +
>> +        trace_colo_compare_tcp_miscompare("Primary len", ppkt->size);
>> +        qemu_hexdump((char *)ppkt->data, stderr, "colo-compare", 
>> ppkt->size);
>> +        trace_colo_compare_tcp_miscompare("Secondary len", spkt->size);
>> +        qemu_hexdump((char *)spkt->data, stderr, "colo-compare", 
>> spkt->size);
>> +
>> +        g_free(sdebug);
>> +        g_free(ddebug);
>> +    }
>> +
>> +    return res;
>> +}
>> +
>> +/*
>> + * called from the compare thread on the primary
>> + * for compare udp packet
>> + */
>> +static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt)
>> +{
>> +    int ret;
>> +
>> +    trace_colo_compare_main("compare udp");
>> +    ret = colo_packet_compare(ppkt, spkt);
>> +
>> +    if (ret) {
>> +        trace_colo_compare_udp_miscompare("primary pkt size", 
>> ppkt->size);
>> +        qemu_hexdump((char *)ppkt->data, stderr, "colo-compare", 
>> ppkt->size);
>> +        trace_colo_compare_udp_miscompare("Secondary pkt size", 
>> spkt->size);
>> +        qemu_hexdump((char *)spkt->data, stderr, "colo-compare", 
>> spkt->size);
>> +    }
>> +
>> +    return ret;
>> +}
>> +
>> +/*
>> + * called from the compare thread on the primary
>> + * for compare icmp packet
>> + */
>> +static int colo_packet_compare_icmp(Packet *spkt, Packet *ppkt)
>> +{
>> +    int network_length;
>> +
>> +    trace_colo_compare_main("compare icmp");
>> +    network_length = ppkt->ip->ip_hl * 4;
>> +    if (ppkt->size != spkt->size ||
>> +        ppkt->size < network_length + ETH_HLEN) {
>> +        return -1;
>> +    }
>> +
>> +    if (colo_packet_compare(ppkt, spkt)) {
>> +        trace_colo_compare_icmp_miscompare("primary pkt size",
>> +                                           ppkt->size);
>> +        qemu_hexdump((char *)ppkt->data, stderr, "colo-compare",
>> +                     ppkt->size);
>> +        trace_colo_compare_icmp_miscompare("Secondary pkt size",
>> +                                           spkt->size);
>> +        qemu_hexdump((char *)spkt->data, stderr, "colo-compare",
>> +                     spkt->size);
>> +        return -1;
>> +    } else {
>> +        return 0;
>> +    }
>> +}
>> +
>> +/*
>> + * called from the compare thread on the primary
>> + * for compare other packet
>> + */
>> +static int colo_packet_compare_other(Packet *spkt, Packet *ppkt)
>>   {
>> -    trace_colo_compare_main("compare all");
>> +    trace_colo_compare_main("compare other");
>> +    trace_colo_compare_ip_info(ppkt->size, inet_ntoa(ppkt->ip->ip_src),
>> + inet_ntoa(ppkt->ip->ip_dst), spkt->size,
>> + inet_ntoa(spkt->ip->ip_src),
>> + inet_ntoa(spkt->ip->ip_dst));
>>       return colo_packet_compare(ppkt, spkt);
>>   }
>>   @@ -245,8 +373,24 @@ static void colo_compare_connection(void 
>> *opaque, void *user_data)
>>       while (!g_queue_is_empty(&conn->primary_list) &&
>>              !g_queue_is_empty(&conn->secondary_list)) {
>>           pkt = g_queue_pop_tail(&conn->primary_list);
>> -        result = g_queue_find_custom(&conn->secondary_list,
>> -                              pkt, 
>> (GCompareFunc)colo_packet_compare_all);
>> +        switch (conn->ip_proto) {
>> +        case IPPROTO_TCP:
>> +            result = g_queue_find_custom(&conn->secondary_list,
>> +                     pkt, (GCompareFunc)colo_packet_compare_tcp);
>> +            break;
>> +        case IPPROTO_UDP:
>> +            result = g_queue_find_custom(&conn->secondary_list,
>> +                     pkt, (GCompareFunc)colo_packet_compare_udp);
>> +            break;
>> +        case IPPROTO_ICMP:
>> +            result = g_queue_find_custom(&conn->secondary_list,
>> +                     pkt, (GCompareFunc)colo_packet_compare_icmp);
>> +            break;
>> +        default:
>> +            result = g_queue_find_custom(&conn->secondary_list,
>> +                     pkt, (GCompareFunc)colo_packet_compare_other);
>> +            break;
>> +        }
>>             if (result) {
>>               ret = compare_chr_send(s->chr_out, pkt->data, pkt->size);
>> diff --git a/trace-events b/trace-events
>> index 1537e91..ab22eb2 100644
>> --- a/trace-events
>> +++ b/trace-events
>> @@ -1919,5 +1919,9 @@ aspeed_vic_write(uint64_t offset, unsigned 
>> size, uint32_t data) "To 0x%" PRIx64
>>     # net/colo-compare.c
>>   colo_compare_main(const char *chr) ": %s"
>> +colo_compare_tcp_miscompare(const char *sta, int size) ": %s = %d"
>> +colo_compare_udp_miscompare(const char *sta, int size) ": %s = %d"
>> +colo_compare_icmp_miscompare(const char *sta, int size) ": %s = %d"
>>   colo_compare_ip_info(int psize, const char *sta, const char *stb, 
>> int ssize, const char *stc, const char *std) "ppkt size = %d, ip_src 
>> = %s, ip_dst = %s, spkt size = %d, ip_src = %s, ip_dst = %s"
>>   colo_old_packet_check_found(int64_t old_time) "%" PRId64
>> +colo_compare_miscompare(void) ""
>
>
>
> .
>
Jason Wang Sept. 1, 2016, 7:40 a.m. UTC | #3
On 2016年09月01日 13:00, Zhang Chen wrote:
>>> +    /*
>>> +     * The 'identification' field in the IP header is *very* random
>>> +     * it almost never matches.  Fudge this by ignoring differences in
>>> +     * unfragmented packets; they'll normally sort themselves out 
>>> if different
>>> +     * anyway, and it should recover at the TCP level.
>>> +     * An alternative would be to get both the primary and 
>>> secondary to rewrite
>>> +     * somehow; but that would need some sync traffic to sync the 
>>> state
>>> +     */
>>> +    if (ntohs(ppkt->ip->ip_off) & IP_DF) {
>>> +        spkt->ip->ip_id = ppkt->ip->ip_id;
>>> +        /* and the sum will be different if the IDs were different */
>>> +        spkt->ip->ip_sum = ppkt->ip->ip_sum;
>>> +    }
>>> +
>>> +    res = memcmp(ppkt->data + ETH_HLEN, spkt->data + ETH_HLEN,
>>> +                (spkt->size - ETH_HLEN));
>>
>> This may work but I worry about whether or not tagged packet can work 
>> here. Looks like parse_packet_early() can recognize vlan tag, but 
>> fill_connection_key() can not. This looks can result queuing wrong 
>> packets into wrong connection.
>
> Currently COLO proxy can't support vlan, we will add this feature in 
> the future.

Looks like current code can still queue vlan packets, please make sure 
it can't.

Thanks
diff mbox

Patch

diff --git a/net/colo-compare.c b/net/colo-compare.c
index b90cf1f..0daefd9 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -18,6 +18,7 @@ 
 #include "qapi/qmp/qerror.h"
 #include "qapi/error.h"
 #include "net/net.h"
+#include "net/eth.h"
 #include "net/vhost_net.h"
 #include "qom/object_interfaces.h"
 #include "qemu/iov.h"
@@ -179,9 +180,136 @@  static int colo_packet_compare(Packet *ppkt, Packet *spkt)
     }
 }
 
-static int colo_packet_compare_all(Packet *spkt, Packet *ppkt)
+/*
+ * called from the compare thread on the primary
+ * for compare tcp packet
+ * compare_tcp copied from Dr. David Alan Gilbert's branch
+ */
+static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
+{
+    struct tcphdr *ptcp, *stcp;
+    int res;
+    char *sdebug, *ddebug;
+
+    trace_colo_compare_main("compare tcp");
+    if (ppkt->size != spkt->size) {
+        if (trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
+            trace_colo_compare_main("pkt size not same");
+        }
+        return -1;
+    }
+
+    ptcp = (struct tcphdr *)ppkt->transport_header;
+    stcp = (struct tcphdr *)spkt->transport_header;
+
+    if (ptcp->th_seq != stcp->th_seq) {
+        if (trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
+            trace_colo_compare_main("pkt tcp seq not same");
+        }
+        return -1;
+    }
+
+    /*
+     * The 'identification' field in the IP header is *very* random
+     * it almost never matches.  Fudge this by ignoring differences in
+     * unfragmented packets; they'll normally sort themselves out if different
+     * anyway, and it should recover at the TCP level.
+     * An alternative would be to get both the primary and secondary to rewrite
+     * somehow; but that would need some sync traffic to sync the state
+     */
+    if (ntohs(ppkt->ip->ip_off) & IP_DF) {
+        spkt->ip->ip_id = ppkt->ip->ip_id;
+        /* and the sum will be different if the IDs were different */
+        spkt->ip->ip_sum = ppkt->ip->ip_sum;
+    }
+
+    res = memcmp(ppkt->data + ETH_HLEN, spkt->data + ETH_HLEN,
+                (spkt->size - ETH_HLEN));
+
+    if (res != 0 && trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
+        sdebug = strdup(inet_ntoa(ppkt->ip->ip_src));
+        ddebug = strdup(inet_ntoa(ppkt->ip->ip_dst));
+        fprintf(stderr, "%s: src/dst: %s/%s p: seq/ack=%u/%u"
+        " s: seq/ack=%u/%u res=%d flags=%x/%x\n", __func__,
+                   sdebug, ddebug,
+                   ntohl(ptcp->th_seq), ntohl(ptcp->th_ack),
+                   ntohl(stcp->th_seq), ntohl(stcp->th_ack),
+                   res, ptcp->th_flags, stcp->th_flags);
+
+        trace_colo_compare_tcp_miscompare("Primary len", ppkt->size);
+        qemu_hexdump((char *)ppkt->data, stderr, "colo-compare", ppkt->size);
+        trace_colo_compare_tcp_miscompare("Secondary len", spkt->size);
+        qemu_hexdump((char *)spkt->data, stderr, "colo-compare", spkt->size);
+
+        g_free(sdebug);
+        g_free(ddebug);
+    }
+
+    return res;
+}
+
+/*
+ * called from the compare thread on the primary
+ * for compare udp packet
+ */
+static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt)
+{
+    int ret;
+
+    trace_colo_compare_main("compare udp");
+    ret = colo_packet_compare(ppkt, spkt);
+
+    if (ret) {
+        trace_colo_compare_udp_miscompare("primary pkt size", ppkt->size);
+        qemu_hexdump((char *)ppkt->data, stderr, "colo-compare", ppkt->size);
+        trace_colo_compare_udp_miscompare("Secondary pkt size", spkt->size);
+        qemu_hexdump((char *)spkt->data, stderr, "colo-compare", spkt->size);
+    }
+
+    return ret;
+}
+
+/*
+ * called from the compare thread on the primary
+ * for compare icmp packet
+ */
+static int colo_packet_compare_icmp(Packet *spkt, Packet *ppkt)
+{
+    int network_length;
+
+    trace_colo_compare_main("compare icmp");
+    network_length = ppkt->ip->ip_hl * 4;
+    if (ppkt->size != spkt->size ||
+        ppkt->size < network_length + ETH_HLEN) {
+        return -1;
+    }
+
+    if (colo_packet_compare(ppkt, spkt)) {
+        trace_colo_compare_icmp_miscompare("primary pkt size",
+                                           ppkt->size);
+        qemu_hexdump((char *)ppkt->data, stderr, "colo-compare",
+                     ppkt->size);
+        trace_colo_compare_icmp_miscompare("Secondary pkt size",
+                                           spkt->size);
+        qemu_hexdump((char *)spkt->data, stderr, "colo-compare",
+                     spkt->size);
+        return -1;
+    } else {
+        return 0;
+    }
+}
+
+/*
+ * called from the compare thread on the primary
+ * for compare other packet
+ */
+static int colo_packet_compare_other(Packet *spkt, Packet *ppkt)
 {
-    trace_colo_compare_main("compare all");
+    trace_colo_compare_main("compare other");
+    trace_colo_compare_ip_info(ppkt->size, inet_ntoa(ppkt->ip->ip_src),
+                               inet_ntoa(ppkt->ip->ip_dst), spkt->size,
+                               inet_ntoa(spkt->ip->ip_src),
+                               inet_ntoa(spkt->ip->ip_dst));
     return colo_packet_compare(ppkt, spkt);
 }
 
@@ -245,8 +373,24 @@  static void colo_compare_connection(void *opaque, void *user_data)
     while (!g_queue_is_empty(&conn->primary_list) &&
            !g_queue_is_empty(&conn->secondary_list)) {
         pkt = g_queue_pop_tail(&conn->primary_list);
-        result = g_queue_find_custom(&conn->secondary_list,
-                              pkt, (GCompareFunc)colo_packet_compare_all);
+        switch (conn->ip_proto) {
+        case IPPROTO_TCP:
+            result = g_queue_find_custom(&conn->secondary_list,
+                     pkt, (GCompareFunc)colo_packet_compare_tcp);
+            break;
+        case IPPROTO_UDP:
+            result = g_queue_find_custom(&conn->secondary_list,
+                     pkt, (GCompareFunc)colo_packet_compare_udp);
+            break;
+        case IPPROTO_ICMP:
+            result = g_queue_find_custom(&conn->secondary_list,
+                     pkt, (GCompareFunc)colo_packet_compare_icmp);
+            break;
+        default:
+            result = g_queue_find_custom(&conn->secondary_list,
+                     pkt, (GCompareFunc)colo_packet_compare_other);
+            break;
+        }
 
         if (result) {
             ret = compare_chr_send(s->chr_out, pkt->data, pkt->size);
diff --git a/trace-events b/trace-events
index 1537e91..ab22eb2 100644
--- a/trace-events
+++ b/trace-events
@@ -1919,5 +1919,9 @@  aspeed_vic_write(uint64_t offset, unsigned size, uint32_t data) "To 0x%" PRIx64
 
 # net/colo-compare.c
 colo_compare_main(const char *chr) ": %s"
+colo_compare_tcp_miscompare(const char *sta, int size) ": %s = %d"
+colo_compare_udp_miscompare(const char *sta, int size) ": %s = %d"
+colo_compare_icmp_miscompare(const char *sta, int size) ": %s = %d"
 colo_compare_ip_info(int psize, const char *sta, const char *stb, int ssize, const char *stc, const char *std) "ppkt size = %d, ip_src = %s, ip_dst = %s, spkt size = %d, ip_src = %s, ip_dst = %s"
 colo_old_packet_check_found(int64_t old_time) "%" PRId64
+colo_compare_miscompare(void) ""