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

[Jason Wang]

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.

>
>
>>
>>> +
>>> +    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?