Re: [Qemu-devel] [RFC PATCH 6/9] net/colo-proxy: add packet forward function

[Zhang Chen]

Hi,Dave

On 12/01/2015 11:50 PM, Dr. David Alan Gilbert wrote:
> * Zhang Chen (address@hidden) wrote:
> > From: zhangchen <address@hidden>
> >
> > The packet recv by primary forward to secondary
> > The packet send by secondary forward to primary
> >
> > Signed-off-by: zhangchen <address@hidden>
> > ---
> >   net/colo-proxy.c | 118 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
> >   1 file changed, 114 insertions(+), 4 deletions(-)
> >
> > diff --git a/net/colo-proxy.c b/net/colo-proxy.c
> > index ece5661..08a852f 100644
> > --- a/net/colo-proxy.c
> > +++ b/net/colo-proxy.c
> > @@ -26,6 +26,110 @@ static char *mode;
> >   static bool colo_do_checkpoint;
> >   
> >   /*
> > + * Packets to be sent by colo forward to
> > + * another colo
> > + * return:          >= 0        success
> > + *                  < 0        failed
> > + */
> > +static ssize_t colo_forward2another(NetFilterState *nf,
> > +                                         NetClientState *sender,
> > +                                         unsigned flags,
> > +                                         const struct iovec *iov,
> > +                                         int iovcnt,
> > +                                         NetPacketSent *sent_cb,
> > +                                         mode_type mode)
> > +{
> > +    ColoProxyState *s = FILTER_COLO_PROXY(nf);
> > +    ssize_t ret = 0;
> > +    ssize_t size = 0;
> > +    struct iovec sizeiov = {
> > +        .iov_base = &size,
> > +        .iov_len = 8
> > +    };
> If you always want 'size' to be 8 bytes then use an int64_t
> (or uint64_t probably if you are sending a length).

In next version I will change it to sizeof(size)

> > +    size = iov_size(iov, iovcnt);
> > +    if (!size) {
> > +        return 0;
> > +    }
> > +
> > +    if (mode == COLO_PRIMARY_MODE) {
> > +        qemu_net_queue_send_iov(s->incoming_queue, sender, flags,
> > +                           iov, iovcnt, NULL);
> > +    }
> > +    ret = iov_send(s->sockfd, &sizeiov, 8, 0, 8);
> > +    if (ret < 0) {
> > +        return ret;
> > +    }
> > +    ret = iov_send(s->sockfd, iov, iovcnt, 0, size);
> > +    return ret;
> > +}
> > +
> > +/*
> > + * recv and handle colo secondary
> > + * forward packets in colo primary
> > + */
> > +static void colo_proxy_primary_forward_handler(NetFilterState *nf)
> > +{
> > +    ColoProxyState *s = FILTER_COLO_PROXY(nf);
> > +    ssize_t len = 0;
> > +    ssize_t ret = 0;
> > +    struct iovec sizeiov = {
> > +        .iov_base = &len,
> > +        .iov_len = 8
> > +    };
> > +    if (s->sockfd < 0) {
> > +        printf("secondary forward disconnected\n");
> > +        return;
> > +    }
> > +    iov_recv(s->sockfd, &sizeiov, 8, 0, 8);
> Check the return value of iov_recv.

I will fix it in next version

> > +    DEBUG("primary_forward_handler recv lensbuf lens=%zu\n", len);
> > +
> > +    if (len > 0) {
> > +        char *recvbuf;
> > +        recvbuf = g_malloc0(len);
> You should check the value of 'len' received off the wire - if things
> go wrong the value could be massive and try and allocate a huge amount
> of memory - checking it would also be a good check for something going
> wrong.

Thanks
I consider that when we send packet size so big,we can split it to send
I will try to fix it in next version

> > +        struct iovec iov = {
> > +            .iov_base = recvbuf,
> > +            .iov_len = len
> > +        };
> > +        iov_recv(s->sockfd, &iov, len, 0, len);
> > +        DEBUG("primary_forward_handler primary recvbuf=%s\n", recvbuf);
> > +        ret = colo_enqueue_secondary_packet(nf, recvbuf, len);
> > +        if (ret) {
> > +            DEBUG("colo_enqueue_secondary_packet succese\n");
> > +        } else {
> > +            DEBUG("colo_enqueue_secondary_packet failed\n");
> > +        }
> > +        g_free(recvbuf);
> > +    }
> > +}
> > +
> > +/*
> > + * recv and handle colo primary
> > + * forward packets in colo secondary
> > + */
> > +static void colo_proxy_secondary_forward_handler(NetFilterState *nf)
> > +{
> > +    ColoProxyState *s = FILTER_COLO_PROXY(nf);
> > +    ssize_t len = 0;
> > +    struct iovec sizeiov = {
> > +        .iov_base = &len,
> > +        .iov_len = 8
> > +    };
> > +    iov_recv(s->sockfd, &sizeiov, 8, 0, 8);
> > +    if (len > 0) {
> > +        char *buf;
> > +        buf = g_malloc0(len);
> > +        struct iovec iov = {
> > +            .iov_base = buf,
> > +            .iov_len = len
> > +        };
> > +        iov_recv(s->sockfd, &iov, len, 0, len);
> This code is very similar - factor it out and share it with the primary?
>
> Dave

I will fix it in next version

Thanks for review
zhangchen

> > +        qemu_net_queue_send(s->incoming_queue, nf->netdev,
> > +                    0, (const uint8_t *)buf, len, NULL);
> > +        g_free(buf);
> > +    }
> > +}
> > +
> > +/*
> >    * colo primary handle host's normal send and
> >    * recv packets to primary guest
> >    * return:          >= 0      success
> > @@ -63,7 +167,8 @@ static ssize_t colo_proxy_primary_handler(NetFilterState *nf,
> >       if (direction == NET_FILTER_DIRECTION_RX) {
> >           /* TODO: enqueue_primary_packet */
> >       } else {
> > -        /* TODO: forward packets to another */
> > +        ret = colo_forward2another(nf, sender, flags, iov, iovcnt,
> > +                    sent_cb, COLO_PRIMARY_MODE);
> >       }
> >   
> >       return ret;
> > @@ -107,7 +212,8 @@ static ssize_t colo_proxy_secondary_handler(NetFilterState 
> > *nf,
> >                               iovcnt, NULL);
> >               return 1;
> >           } else {
> > -        /* TODO: forward packets to another */
> > +            ret = colo_forward2another(nf, sender, flags, iov, iovcnt,
> > +                        sent_cb, COLO_SECONDARY_MODE);
> >           }
> >   
> >       } else {
> > @@ -178,7 +284,9 @@ static void colo_accept_incoming(ColoProxyState *s)
> >           return;
> >       }
> >       s->sockfd = acceptsock;
> > -    /* TODO: handle the packets that primary forward */
> > +    qemu_set_fd_handler(s->sockfd,
> > +                (IOHandler *)colo_proxy_secondary_forward_handler, NULL,
> > +                (void *)s);
> >       return;
> >   }
> >   
> > @@ -211,7 +319,9 @@ static ssize_t colo_proxy_primary_setup(NetFilterState *nf)
> >       }
> >       DEBUG("colo proxy connect success\n");
> >       s->sockfd = sock;
> > -   /* TODO: handle the packets that secondary forward */
> > +    qemu_set_fd_handler(s->sockfd,
> > +                (IOHandler *)colo_proxy_primary_forward_handler,
> > +                NULL, (void *)s);
> >       g_free(s->addr);
> >       return 1;
> >   }
> > --
> > 1.9.1
> --
> Dr. David Alan Gilbert / address@hidden / Manchester, UK
>
>
> .