Re: [Qemu-devel] [RFC PATCH V3 2/4] colo-compare: track connection and enqueue packet

[Zhang Chen <zhangchen.fnst@cn.fujitsu.com>]

On 04/29/2016 10:05 AM, Jason Wang wrote:
> On 04/28/2016 06:25 PM, Zhang Chen wrote:
>>>> +static Packet *packet_new(CompareState *s, const void *data,
>>>> +                              int size, ConnectionKey *key)
>>>> +{
>>>> +    Packet *pkt = g_slice_new(Packet);
>>>> +
>>>> +    pkt->data = g_memdup(data, size);
>>>> +    pkt->size = size;
>>>> +    pkt->s = s;
>>>> +
>>>> +    if (connection_key_init(pkt, key)) {
>>>> +        packet_destroy(pkt, NULL);
>>>> +        pkt = NULL;
>>>> +    }
>>> Can we do connection_key_init() first, this can avoid packet_desctory()
>>> if it fails.
>> Do you mean we should call connection_key_init() first
>> and then call packet_new()?
> Yes, only when connection_key_init() succeed.

OK~ will fix in next.

>>>> +
>>>> +    return pkt;
>>>> +}
>>>> +
>>>> +/*
>>>> + * Return 0 on success, if return -1 means the pkt
>>>> + * is unsupported(arp and ipv6) and will be sent later
>>>> + */
>>>> +static int packet_enqueue(CompareState *s, int mode)
>>>> +{
>>>> +    ConnectionKey key = {{ 0 } };
>>>> +    Packet *pkt = NULL;
>>>> +    Connection *conn;
>>>> +
>>>> +    if (mode == PRIMARY_IN) {
>>>> +        pkt = packet_new(s, s->pri_rs.buf, s->pri_rs.packet_len,
>>>> &key);
>>>> +    } else {
>>>> +        pkt = packet_new(s, s->sec_rs.buf, s->sec_rs.packet_len,
>>>> &key);
>>>> +    }
>>>> +    if (!pkt) {
>>>> +        return -1;
>>>> +    }
>>>> +
>>>> +    conn = connection_get(s, &key);
>>>> +    if (!conn->processing) {
>>>> +        qemu_mutex_lock(&s->conn_list_lock);
>>>> +        g_queue_push_tail(&s->conn_list, conn);
>>>> +        qemu_mutex_unlock(&s->conn_list_lock);
>>>> +        conn->processing = true;
>>>> +    }
>>>> +
>>>> +    qemu_mutex_lock(&conn->list_lock);
>>>> +    if (mode == PRIMARY_IN) {
>>>> +        g_queue_push_tail(&conn->primary_list, pkt);
>>>> +    } else {
>>>> +        g_queue_push_tail(&conn->secondary_list, pkt);
>>>> +    }
>>>> +    qemu_mutex_unlock(&conn->list_lock);
>>>> +
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +static void packet_destroy(void *opaque, void *user_data)
>>>> +{
>>>> +    Packet *pkt = opaque;
>>>> +
>>>> +    g_free(pkt->data);
>>>> +    g_slice_free(Packet, pkt);
>>>> +}
>>>> +
>>>>    static int compare_chr_send(CharDriverState *out,
>>>>                                const uint8_t *buf,
>>>>                                uint32_t size)
>>>> @@ -158,8 +437,10 @@ static void compare_pri_chr_in(void *opaque,
>>>> const uint8_t *buf, int size)
>>>>          ret = compare_chr_fill_rstate(&s->pri_rs, buf, size);
>>>>        if (ret == 1) {
>>>> -        /* FIXME: enqueue to primary packet list */
>>>> -        compare_chr_send(s->chr_out, s->pri_rs.buf,
>>>> s->pri_rs.packet_len);
>>>> +        if (packet_enqueue(s, PRIMARY_IN)) {
>>>> +            trace_colo_compare_main("primary: unsupported packet in");
>>>> +            compare_chr_send(s->chr_out, s->pri_rs.buf,
>>>> s->pri_rs.packet_len);
>>> Looks like if a packet was not recognized by connection_key_init(), it
>>> will be sent directly without comparing it with the packet sent from
>>> secondary? Is this expected?
>> Yes,we will send primary's arp packet to get mac first.
>>
>> Thanks
>> zhangchen
> But what if the packet was not arp?
>
>
> .

rarp packet will be sent, ip packet will be enqueue.


-- 
Thanks
zhangchen