Re: [Qemu-devel] [PATCH V5 6/9] net/colo-compare.c: Make colo-compare support vnet_hdr_len

[Jason Wang <jasowang@redhat.com>]

On 2017年05月23日 22:20, Zhang Chen wrote:
> We add the vnet_hdr option for colo-compare, default is disable.
> If you use virtio-net-pci net driver, please enable it.
> You can use it for example:
> -object colo-compare,id=comp0,primary_in=compare0-0,secondary_in=compare1,outdev=compare_out0,vnet_hdr=on

This is not accurate since virtio-net-pci is not the only card that uses 
vnet_hdr. E1000E is another one.

>
> COLO-compare can get vnet header length from filter,
> Add vnet_hdr_len to struct packet and output packet with
> the vnet_hdr_len.
>
> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
> ---
>   net/colo-compare.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++-------
>   qemu-options.hx    |  4 +--
>   2 files changed, 69 insertions(+), 11 deletions(-)
>
> diff --git a/net/colo-compare.c b/net/colo-compare.c
> index bf0b856..f89b380 100644
> --- a/net/colo-compare.c
> +++ b/net/colo-compare.c
> @@ -73,6 +73,7 @@ typedef struct CompareState {
>       CharBackend chr_out;
>       SocketReadState pri_rs;
>       SocketReadState sec_rs;
> +    bool vnet_hdr;
>   
>       /* connection list: the connections belonged to this NIC could be found
>        * in this list.
> @@ -97,9 +98,10 @@ enum {
>       SECONDARY_IN,
>   };
>   
> -static int compare_chr_send(CharBackend *out,
> +static int compare_chr_send(CompareState *s,
>                               const uint8_t *buf,
> -                            uint32_t size);
> +                            uint32_t size,
> +                            uint32_t vnet_hdr_len);
>   
>   static gint seq_sorter(Packet *a, Packet *b, gpointer data)
>   {
> @@ -472,7 +474,10 @@ static void colo_compare_connection(void *opaque, void *user_data)
>           }
>   
>           if (result) {
> -            ret = compare_chr_send(&s->chr_out, pkt->data, pkt->size);
> +            ret = compare_chr_send(s,
> +                                   pkt->data,
> +                                   pkt->size,
> +                                   pkt->vnet_hdr_len);

Why not check vnet_hdr support here?  And don't we need strip vnet_hdr 
here? (Since the redirector on destination does not do this)

>               if (ret < 0) {
>                   error_report("colo_send_primary_packet failed");
>               }
> @@ -493,9 +498,10 @@ static void colo_compare_connection(void *opaque, void *user_data)
>       }
>   }
>   
> -static int compare_chr_send(CharBackend *out,
> +static int compare_chr_send(CompareState *s,
>                               const uint8_t *buf,
> -                            uint32_t size)
> +                            uint32_t size,
> +                            uint32_t vnet_hdr_len)
>   {
>       int ret = 0;
>       uint32_t len = htonl(size);
> @@ -504,12 +510,24 @@ static int compare_chr_send(CharBackend *out,
>           return 0;
>       }
>   
> -    ret = qemu_chr_fe_write_all(out, (uint8_t *)&len, sizeof(len));
> +    ret = qemu_chr_fe_write_all(&s->chr_out, (uint8_t *)&len, sizeof(len));
>       if (ret != sizeof(len)) {
>           goto err;
>       }
>   
> -    ret = qemu_chr_fe_write_all(out, (uint8_t *)buf, size);
> +    if (s->vnet_hdr) {
> +        /*
> +         * We send vnet header len make other module(like filter-redirector)
> +         * know how to parse net packet correctly.
> +         */

But redirector does not strip the vnet header, does this really work?

Thanks