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

[Zhang Chen]

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