From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44439) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bNCQn-0001Nc-Sl for qemu-devel@nongnu.org; Wed, 13 Jul 2016 01:09:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bNCQh-0004S3-Qn for qemu-devel@nongnu.org; Wed, 13 Jul 2016 01:09:56 -0400 Received: from [59.151.112.132] (port=45694 helo=heian.cn.fujitsu.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bNCQf-0004Rm-ES for qemu-devel@nongnu.org; Wed, 13 Jul 2016 01:09:51 -0400 References: <1466681677-30487-1-git-send-email-zhangchen.fnst@cn.fujitsu.com> <1466681677-30487-5-git-send-email-zhangchen.fnst@cn.fujitsu.com> <577F6B8D.2050309@redhat.com> <57836EAB.5090007@cn.fujitsu.com> <5785AD5C.6040903@redhat.com> From: Zhang Chen Message-ID: <5785CD53.90206@cn.fujitsu.com> Date: Wed, 13 Jul 2016 13:10:43 +0800 MIME-Version: 1.0 In-Reply-To: <5785AD5C.6040903@redhat.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [RFC PATCH V5 4/4] colo-compare: add TCP, UDP, ICMP packet comparison List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jason Wang , qemu devel Cc: Li Zhijian , "eddie . dong" , "Dr . David Alan Gilbert" , zhanghailiang On 07/13/2016 10:54 AM, Jason Wang wrote: > > > On 2016年07月11日 18:02, Zhang Chen wrote: >>>> +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 * 4; >>>> + if (ppkt->size != spkt->size || >>>> + ppkt->size < network_length + ETH_HLEN) { >>>> + trace_colo_compare_icmp_miscompare_size(ppkt->size, spkt->size); >>>> + return -1; >>>> + } >>>> + 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_icmp_miscompare_addr("ppkt s_addr", >>>> + inet_ntoa(icmp_ppkt->icmp_gwaddr)); >>>> + trace_colo_compare_icmp_miscompare_addr("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_icmp_miscompare_mtu("ppkt nextmtu", >>>> + icmp_ppkt->icmp_nextmtu); >>>> + trace_colo_compare_icmp_miscompare_mtu("spkt nextmtu", >>>> + icmp_spkt->icmp_nextmtu); >>>> + return -1; >>>> + } >>>> + } >>>> + } else { >>>> + return -1; >>>> + } >>> >>> Why only compare part of icmp packet? >>> >> >> That's include most of situation, increase all part of icmp >> can reduce compare efficiency. >> >> Thanks >> Zhang Chen > > I believe we should cover all instead of "most" of situations. And > looks like icmp packet were all small, so there's probably no need to > do special tricks like this. > > OK, I will fix this in next version. Thanks Zhang Chen > > . > -- Thanks zhangchen