Re: [Qemu-devel] [PATCH V2 2/3] COLO-compare: Optimize compare_common and compare_tcp

[Zhang Chen]

On 02/28/2017 01:47 PM, Jason Wang wrote:

On 2017年02月28日 13:18, Zhang Chen wrote:

Add offset args for colo_packet_compare_common, optimize
colo_packet_compare_icmp() and colo_packet_compare_udp()
just compare the IP payload. Before compare all tcp packet,
we compare tcp checksum firstly, this function can get
better performance.

Signed-off-by: Zhang Chen <zhangchen.f...@cn.fujitsu.com>
---
  net/colo-compare.c | 42 ++++++++++++++++++++++++++++--------------
  1 file changed, 28 insertions(+), 14 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 602a758..61fcdf2 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -180,7 +180,7 @@ static int packet_enqueue(CompareState *s, int mode)
   * return:    0  means packet same
   *            > 0 || < 0 means packet different
   */
-static int colo_packet_compare_common(Packet *ppkt, Packet *spkt)

+static int colo_packet_compare_common(Packet *ppkt, Packet *spkt, int offset)

  {

trace_colo_compare_ip_info(ppkt->size, inet_ntoa(ppkt->ip->ip_src),

inet_ntoa(ppkt->ip->ip_dst), spkt->size,

@@ -188,7 +188,8 @@ static int colo_packet_compare_common(Packet *ppkt, Packet *spkt)

inet_ntoa(spkt->ip->ip_dst));
        if (ppkt->size == spkt->size) {
-        return memcmp(ppkt->data, spkt->data, spkt->size);
+        return memcmp(ppkt->data + offset, spkt->data + offset,
+                      spkt->size - offset);
      } else {
          trace_colo_compare_main("Net packet size are not the same");
          return -1;

@@ -207,13 +208,6 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)

        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;

@@ -231,8 +225,11 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)

          spkt->ip->ip_sum = ppkt->ip->ip_sum;
      }
  -    res = memcmp(ppkt->data + ETH_HLEN, spkt->data + ETH_HLEN,
-                (spkt->size - ETH_HLEN));
+    if (ptcp->th_sum == stcp->th_sum) {
+        res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN);
+    } else {
+        res = -1;
+    }

if (res != 0 && trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {

trace_colo_compare_pkt_info_src(inet_ntoa(ppkt->ip->ip_src),

@@ -263,10 +260,18 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)

  static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt)
  {
      int ret;
+    int network_length = ppkt->ip->ip_hl * 4;

network_header_length looks better.

OK, I will rename it.

        trace_colo_compare_main("compare udp");
  -    ret = colo_packet_compare_common(ppkt, spkt);
+    /*
+     * Because of ppkt and spkt are both in the same connection,
+     * The ppkt's src ip, dst ip, src port, dst port, ip_proto all are

+ * same with spkt. In addition, IP header's Identification is a random

+     * field, we can handle it in IP fragmentation function later.
+     * So we just compare the ip payload here.
+     */

I'm afraid there're some other fields were ignored e.g TOS,TTL, and options. We'd better explain this in the comment too.

OK, How about this?

+    /*
+     * Because of ppkt and spkt are both in the same connection,
+     * The ppkt's src ip, dst ip, src port, dst port, ip_proto all are
+     * same with spkt. In addition, IP header's Identification is a random
+     * field, we can handle it in IP fragmentation function later.
+     * So we just compare the ip payload here. we ignored all IP header
+     * include other field like TOS,TTL,IP Checksum.
+     */

Thanks
Zhang Chen

+ ret = colo_packet_compare_common(ppkt, spkt, network_length + ETH_HLEN);

        if (ret) {

trace_colo_compare_udp_miscompare("primary pkt size", ppkt->size); @@ -284,9 +289,18 @@ static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt)

   */
  static int colo_packet_compare_icmp(Packet *spkt, Packet *ppkt)
  {
+    int network_length = ppkt->ip->ip_hl * 4;
+
      trace_colo_compare_main("compare icmp");
  -    if (colo_packet_compare_common(ppkt, spkt)) {
+    /*
+     * Because of ppkt and spkt are both in the same connection,
+     * The ppkt's src ip, dst ip, src port, dst port, ip_proto all are

+ * same with spkt. In addition, IP header's Identification is a random

+     * field, we can handle it in IP fragmentation function later.
+     * So we just compare the ip payload here.
+     */

+ if (colo_packet_compare_common(ppkt, spkt, network_length + ETH_HLEN)) {

          trace_colo_compare_icmp_miscompare("primary pkt size",
                                             ppkt->size);
          qemu_hexdump((char *)ppkt->data, stderr, "colo-compare",

@@ -312,7 +326,7 @@ static int colo_packet_compare_other(Packet *spkt, Packet *ppkt)

inet_ntoa(ppkt->ip->ip_dst), spkt->size,
inet_ntoa(spkt->ip->ip_src),
inet_ntoa(spkt->ip->ip_dst));
-    return colo_packet_compare_common(ppkt, spkt);
+    return colo_packet_compare_common(ppkt, spkt, 0);
  }

static int colo_old_packet_check_one(Packet *pkt, int64_t *check_time)

.

--
Thanks
Zhang Chen