From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56275) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1avh77-0002wW-0c for qemu-devel@nongnu.org; Thu, 28 Apr 2016 04:15:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1avh72-00075p-6l for qemu-devel@nongnu.org; Thu, 28 Apr 2016 04:15:56 -0400 Received: from mx1.redhat.com ([209.132.183.28]:59223) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1avh71-00075l-VH for qemu-devel@nongnu.org; Thu, 28 Apr 2016 04:15:52 -0400 References: <1460977906-25218-1-git-send-email-zhangchen.fnst@cn.fujitsu.com> <1460977906-25218-5-git-send-email-zhangchen.fnst@cn.fujitsu.com> From: Jason Wang Message-ID: <5721C6B1.3030600@redhat.com> Date: Thu, 28 Apr 2016 16:15:45 +0800 MIME-Version: 1.0 In-Reply-To: <1460977906-25218-5-git-send-email-zhangchen.fnst@cn.fujitsu.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC PATCH V3 4/4] colo-compare: add TCP, UDP, ICMP packet comparison List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Zhang Chen , qemu devel Cc: Li Zhijian , Gui jianfeng , "eddie.dong" , zhanghailiang , "Dr. David Alan Gilbert" , Yang Hongyang On 04/18/2016 07:11 PM, Zhang Chen wrote: > Signed-off-by: Zhang Chen > Signed-off-by: Li Zhijian > Signed-off-by: Wen Congyang > --- > net/colo-compare.c | 158 +++++++++++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 154 insertions(+), 4 deletions(-) I'm not a tcp expert, this patch may need some guys who are good at TCP to review. > diff --git a/net/colo-compare.c b/net/colo-compare.c > index 4b5a2d4..3dad461 100644 > --- a/net/colo-compare.c > +++ b/net/colo-compare.c > @@ -385,9 +385,148 @@ 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; > + ptrdiff_t offset; > + > + trace_colo_compare_main("compare tcp"); > + ptcp = (struct tcphdr *)ppkt->transport_layer; > + stcp = (struct tcphdr *)spkt->transport_layer; > + > + /* Initial is compare the whole packet */ > + offset = 12; /* Hack! Skip virtio header */ So this won't work for e1000 I believe? > + > + if (ptcp->th_flags == stcp->th_flags && > + ((ptcp->th_flags & (TH_ACK | TH_SYN)) == (TH_ACK | TH_SYN))) { > + /* This is the syn/ack response from the guest to an incoming > + * connection; the secondary won't have matched the sequence number > + * Note: We should probably compare the IP level? > + * Note hack: This already has the virtio offset > + */ > + offset = sizeof(ptcp->th_ack) + (void *)&ptcp->th_ack - ppkt->data; > + } > + /* Note - we want to compare everything as long as it's not the syn/ack? */ > + assert(offset > 0); > + assert(spkt->size > offset); > + > + /* 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 This is very tricky I believe, I wonder this will work. Would it be easier if we reassembly the packet? And I believe we should check for the length before all the other examinations? > + */ > + 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 + offset, spkt->data + offset, > + (spkt->size - offset)); > + > + if (res && DEBUG_TCP_COMPARE) { > + sdebug = strdup(inet_ntoa(ppkt->ip->ip_src)); > + ddebug = strdup(inet_ntoa(ppkt->ip->ip_dst)); > + fprintf(stderr, "%s: src/dst: %s/%s offset=%zd p: seq/ack=%u/%u" > + " s: seq/ack=%u/%u res=%d flags=%x/%x\n", __func__, > + sdebug, ddebug, offset, > + ntohl(ptcp->th_seq), ntohl(ptcp->th_ack), > + ntohl(stcp->th_seq), ntohl(stcp->th_ack), > + res, ptcp->th_flags, stcp->th_flags); > + if (res && (ptcp->th_seq == stcp->th_seq)) { > + trace_colo_compare_with_int("Primary len", ppkt->size); > + colo_dump_packet(ppkt); > + trace_colo_compare_with_int("Secondary len", spkt->size); > + colo_dump_packet(spkt); > + } > + 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 = 1; > + > + trace_colo_compare_main("compare udp"); > + ret = colo_packet_compare(ppkt, spkt); > + > + if (ret) { > + trace_colo_compare_main("primary udp"); > + colo_dump_packet(ppkt); > + trace_colo_compare_main("secondary udp"); > + colo_dump_packet(spkt); > + } > + > + 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; > + struct icmp *icmp_ppkt, *icmp_spkt; > + > + trace_colo_compare_main("compare icmp"); > + network_length = (ppkt->ip->ip_hl & 0x0F) * 4; > + icmp_ppkt = (struct icmp *)(ppkt->data + network_length + ETH_HLEN); > + icmp_spkt = (struct icmp *)(spkt->data + network_length + ETH_HLEN); > + > + if ((icmp_ppkt->icmp_type == icmp_spkt->icmp_type) && > + (icmp_ppkt->icmp_code == icmp_spkt->icmp_code)) { > + if (icmp_ppkt->icmp_type == ICMP_REDIRECT) { > + if (icmp_ppkt->icmp_gwaddr.s_addr != > + icmp_spkt->icmp_gwaddr.s_addr) { > + trace_colo_compare_main("icmp_gwaddr.s_addr not same"); > + trace_colo_compare_with_char("ppkt s_addr", > + inet_ntoa(icmp_ppkt->icmp_gwaddr)); > + trace_colo_compare_with_char("spkt s_addr", > + inet_ntoa(icmp_spkt->icmp_gwaddr)); > + return -1; > + } > + } else if ((icmp_ppkt->icmp_type == ICMP_UNREACH) && > + (icmp_ppkt->icmp_type == ICMP_UNREACH_NEEDFRAG)) { > + if (icmp_ppkt->icmp_nextmtu != icmp_spkt->icmp_nextmtu) { > + trace_colo_compare_main("icmp_nextmtu not same"); > + trace_colo_compare_with_int("ppkt s_addr", > + icmp_ppkt->icmp_nextmtu); > + trace_colo_compare_with_int("spkt s_addr", > + icmp_spkt->icmp_nextmtu); > + return -1; > + } > + } > + } else { > + return -1; > + } > + > + 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"); > return colo_packet_compare(ppkt, spkt); > } > > @@ -406,8 +545,19 @@ 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_head(&conn->primary_list); > - result = g_queue_find_custom(&conn->secondary_list, > - pkt, (GCompareFunc)colo_packet_compare_all); > + if (conn->ip_proto == IPPROTO_TCP) { > + result = g_queue_find_custom(&conn->secondary_list, > + pkt, (GCompareFunc)colo_packet_compare_tcp); > + } else if (conn->ip_proto == IPPROTO_UDP) { > + result = g_queue_find_custom(&conn->secondary_list, > + pkt, (GCompareFunc)colo_packet_compare_udp); > + } else if (conn->ip_proto == IPPROTO_ICMP) { > + result = g_queue_find_custom(&conn->secondary_list, > + pkt, (GCompareFunc)colo_packet_compare_icmp); > + } else { > + result = g_queue_find_custom(&conn->secondary_list, > + pkt, (GCompareFunc)colo_packet_compare_other); > + } > A switch...case looks better. > if (result) { > ret = compare_chr_send(pkt->s->chr_out, pkt->data, pkt->size);