From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:42175)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <jasowang@redhat.com>) id 1avxo8-0005Ry-3n
	for qemu-devel@nongnu.org; Thu, 28 Apr 2016 22:05:29 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <jasowang@redhat.com>) id 1avxo4-0001Nj-U5
	for qemu-devel@nongnu.org; Thu, 28 Apr 2016 22:05:28 -0400
Received: from mx1.redhat.com ([209.132.183.28]:41448)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <jasowang@redhat.com>) id 1avxo4-0001Nb-MV
	for qemu-devel@nongnu.org; Thu, 28 Apr 2016 22:05:24 -0400
References: <1460977906-25218-1-git-send-email-zhangchen.fnst@cn.fujitsu.com>
	<1460977906-25218-3-git-send-email-zhangchen.fnst@cn.fujitsu.com>
	<5721C023.1060306@redhat.com> <5721E51C.7090803@cn.fujitsu.com>
From: Jason Wang <jasowang@redhat.com>
Message-ID: <5722C15E.4090100@redhat.com>
Date: Fri, 29 Apr 2016 10:05:18 +0800
MIME-Version: 1.0
In-Reply-To: <5721E51C.7090803@cn.fujitsu.com>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [RFC PATCH V3 2/4] colo-compare: track connection
 and enqueue packet
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel/>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>, qemu devel <qemu-devel@nongnu.org>
Cc: Li Zhijian <lizhijian@cn.fujitsu.com>, Gui jianfeng <guijianfeng@cn.fujitsu.com>, Wen Congyang <wency@cn.fujitsu.com>, zhanghailiang <zhang.zhanghailiang@huawei.com>, Yang Hongyang <hongyang.yang@easystack.cn>, "eddie.dong" <eddie.dong@intel.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>



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?