Re: [Qemu-devel] [PATCH 2/3] COLO-compare: Optimize colo_packet_compare_common

[Hailiang Zhang <zhang.zhanghailiang@huawei.com>]

On 2017/2/25 11:32, 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.
>
> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
> ---
>   net/colo-compare.c | 28 +++++++++++++++++++++-------
>   1 file changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/net/colo-compare.c b/net/colo-compare.c
> index e75f0ae..9853232 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;
> @@ -237,8 +238,7 @@ 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));
> +    res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN);
>

For tcp packets check, why not ignore the ip headers, just like udp packets check ?
Besides, here, can we compare the checksum stored in headers of tcp and udp
before call colo_packet_compare_common(), which i think will improve the comparing
performance.

Thanks.
Hailiang

>       if (res != 0 && trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
>           trace_colo_compare_pkt_info_src(inet_ntoa(ppkt->ip->ip_src),
> @@ -277,7 +277,14 @@ static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt)
>           return -1;
>       }
>
> -    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.
> +     */
> +    ret = colo_packet_compare_common(ppkt, spkt, network_length + ETH_HLEN);
>
>       if (ret) {
>           trace_colo_compare_udp_miscompare("primary pkt size", ppkt->size);
> @@ -304,7 +311,14 @@ static int colo_packet_compare_icmp(Packet *spkt, Packet *ppkt)
>           return -1;
>       }
>
> -    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",
> @@ -330,7 +344,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)
>