Re: [Qemu-devel] [RFC PATCH v2 09/10] net/colo-proxy: Compare pri pkt to sec pkt

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Zhang Chen (zhangchen.fnst@cn.fujitsu.com) wrote:
> From: zhangchen <zhangchen.fnst@cn.fujitsu.com>
> 
> We will compare packet sent by primary guest
> to secondary guest,if same,send primary packet.
> else we will notify colo to do checkpoint to
> make secondary guset running same as primary
> 
> Signed-off-by: zhangchen <zhangchen.fnst@cn.fujitsu.com>
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> ---
>  net/colo-proxy.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 64 insertions(+)
> 
> diff --git a/net/colo-proxy.c b/net/colo-proxy.c
> index 06bab80..abb289f 100644
> --- a/net/colo-proxy.c
> +++ b/net/colo-proxy.c
> @@ -602,6 +602,70 @@ static void colo_proxy_notify_checkpoint(void)
>      colo_do_checkpoint = true;
>  }
>  
> +/*
> + * The IP packets sent by primary and secondary
> + * will be comparison in here
> + * TODO： support ip fragment
> + * return:    0  means packet same
> + *            > 0 || < 0 means packet different
> + */
> +static int colo_packet_compare(Packet *ppkt, Packet *spkt)
> +{
> +    trace_colo_proxy("colo_packet_compare data   ppkt");
> +    trace_colo_proxy_packet_size(ppkt->size);
> +    trace_colo_proxy_packet_src(inet_ntoa(ppkt->ip->ip_src));
> +    trace_colo_proxy_packet_dst(inet_ntoa(ppkt->ip->ip_dst));
> +    colo_proxy_dump_packet(ppkt);
> +    trace_colo_proxy("colo_packet_compare data   spkt");
> +    trace_colo_proxy_packet_size(spkt->size);
> +    trace_colo_proxy_packet_src(inet_ntoa(spkt->ip->ip_src));
> +    trace_colo_proxy_packet_dst(inet_ntoa(spkt->ip->ip_dst));
> +    colo_proxy_dump_packet(spkt);
> +
> +    if (ppkt->size == spkt->size) {
> +        return memcmp(ppkt->data, spkt->data, spkt->size);
> +    } else {
> +        trace_colo_proxy("colo_packet_compare size not same");
> +        return -1;
> +    }
> +}
> +
> +static void colo_compare_connection(void *opaque, void *user_data)
> +{
> +    Connection *conn = opaque;
> +    Packet *pkt = NULL;
> +    GList *result = NULL;
> +
> +    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);

Are you sure the 'ppkt' and 'spkt' are the right way around in colo_packet_compare?
(Not that in this simple version it makes much difference).
My reading of g_queue_find_custom's man page is that the first parameter
of the compare function comes from the list, which is the secondary.

> +        if (result) {
> +            colo_send_primary_packet(pkt, NULL);
> +            trace_colo_proxy("packet same and release packet");
> +        } else {
> +            g_queue_push_tail(&conn->primary_list, pkt);

You pop the packets off the head of the primary list above,
but push it back to the tail here; why do you reorder?

Dave

> +            trace_colo_proxy("packet different");
> +            colo_proxy_notify_checkpoint();
> +            break;
> +        }
> +    }
> +}
> +
> +static void *colo_proxy_compare_thread(void *opaque)
> +{
> +    COLOProxyState *s = opaque;
> +
> +    while (s->status == COLO_PROXY_RUNNING) {
> +        qemu_event_wait(&s->need_compare_ev);
> +        qemu_event_reset(&s->need_compare_ev);
> +        g_queue_foreach(&s->conn_list, colo_compare_connection, NULL);
> +    }
> +
> +    return NULL;
> +}
> +
>  static void colo_proxy_start_one(NetFilterState *nf,
>                                        void *opaque, Error **errp)
>  {
> -- 
> 1.9.1
> 
> 
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK