[Qemu-devel] [PATCH V4 00/12] Add COLO-proxy virtio-net support

[Qemu-devel] [PATCH V4 00/12] Add COLO-proxy virtio-net support

[Zhang Chen]

If user use -device virtio-net-pci, virtio-net driver will add a header
to raw net packet that colo-proxy can't handle it. COLO-proxy just
focus on the packet payload, so we skip the virtio-net header to compare
the sent packet that primary guest's to secondary guest's.

V4:
 - Add vnet_hdr option for filter-mirror, filter-redirector,
   filter-rewriter,colo-compare.
 - Use new design to impliment virtio-net support for colo-proxy.
 - Fix codestyle.
 - Remove unused option for filter-rewriter.
 - Add filter-rewriter virtio-net support.
 - Address other comments.


Zhang Chen (12):
  net: Add vnet_hdr_len related arguments in NetClientState
  net/filter-mirror.c: Add new option to enable vnet support for
    filter-mirror
  net/filter-mirror.c: Make filter_mirror_send support vnet support.
  net/filter-mirror.c: Add new option to enable vnet support for
    filter-redirector
  net/net.c: Add vnet_hdr support in SocketReadState
  net/colo-compare.c: Add new option to enable vnet support for
    colo-compare
  net/colo.c: Make vnet_hdr_len as packet property
  net/colo-compare.c: Make colo-compare support vnet_hdr_len
  net/colo.c: Add vnet packet parse feature in colo-proxy
  net/colo-compare.c: Add vnet packet's tcp/udp/icmp compare
  net/filter-rewriter.c: Add new option to enable vnet support for
    filter-rewriter
  net/filter-rewriter.c: Make filter-rewriter support vnet_hdr_len

 include/net/net.h     |  11 +++++-
 net/colo-compare.c    |  99 ++++++++++++++++++++++++++++++++++++++---------
 net/colo.c            |   9 +++--
 net/colo.h            |   4 +-
 net/filter-mirror.c   | 104 +++++++++++++++++++++++++++++++++++++++++++++++---
 net/filter-rewriter.c |  55 +++++++++++++++++++++++++-
 net/net.c             |  38 ++++++++++++++++--
 net/socket.c          |   2 +-
 qemu-options.hx       |  17 +++++----
 9 files changed, 296 insertions(+), 43 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCH V4 01/12] net: Add vnet_hdr_len related arguments in NetClientState

[Zhang Chen]

Add vnet_hdr_len and using_vnet_hdr arguments in NetClientState
that make othermodule get real vnet_hdr_len easily.

Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
---
 include/net/net.h | 2 ++
 net/net.c         | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/include/net/net.h b/include/net/net.h
index 99b28d5..70edfc0 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -100,6 +100,8 @@ struct NetClientState {
     unsigned int queue_index;
     unsigned rxfilter_notify_enabled:1;
     int vring_enable;
+    bool using_vnet_hdr;
+    int vnet_hdr_len;
     QTAILQ_HEAD(NetFilterHead, NetFilterState) filters;
 };
 
diff --git a/net/net.c b/net/net.c
index 0ac3b9e..a00a0c9 100644
--- a/net/net.c
+++ b/net/net.c
@@ -472,6 +472,7 @@ void qemu_using_vnet_hdr(NetClientState *nc, bool enable)
         return;
     }
 
+    nc->using_vnet_hdr = enable;
     nc->info->using_vnet_hdr(nc, enable);
 }
 
@@ -491,6 +492,7 @@ void qemu_set_vnet_hdr_len(NetClientState *nc, int len)
         return;
     }
 
+    nc->vnet_hdr_len = len;
     nc->info->set_vnet_hdr_len(nc, len);
 }
 
-- 
2.7.4

Re: [Qemu-devel] [PATCH V4 01/12] net: Add vnet_hdr_len related arguments in NetClientState

[Philippe Mathieu-Daudé]

Hi Zhang

On 05/11/2017 10:41 PM, Zhang Chen wrote:
> Add vnet_hdr_len and using_vnet_hdr arguments in NetClientState
> that make othermodule get real vnet_hdr_len easily.
>
> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
> ---
>  include/net/net.h | 2 ++
>  net/net.c         | 2 ++
>  2 files changed, 4 insertions(+)
>
> diff --git a/include/net/net.h b/include/net/net.h
> index 99b28d5..70edfc0 100644
> --- a/include/net/net.h
> +++ b/include/net/net.h
> @@ -100,6 +100,8 @@ struct NetClientState {
>      unsigned int queue_index;
>      unsigned rxfilter_notify_enabled:1;
>      int vring_enable;
> +    bool using_vnet_hdr;
> +    int vnet_hdr_len;
>      QTAILQ_HEAD(NetFilterHead, NetFilterState) filters;
>  };
>
> diff --git a/net/net.c b/net/net.c
> index 0ac3b9e..a00a0c9 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -472,6 +472,7 @@ void qemu_using_vnet_hdr(NetClientState *nc, bool enable)
>          return;
>      }
>
> +    nc->using_vnet_hdr = enable;
>      nc->info->using_vnet_hdr(nc, enable);
>  }
>
> @@ -491,6 +492,7 @@ void qemu_set_vnet_hdr_len(NetClientState *nc, int len)
>          return;
>      }
>
> +    nc->vnet_hdr_len = len;
>      nc->info->set_vnet_hdr_len(nc, len);
>  }
>

For what it's worth, now having those fields in NetClientState it is 
possible to remove a deref to NetClientInfo in qemu_has_vnet_hdr() and 
qemu_has_vnet_hdr_len().

Anyway,
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Re: [Qemu-devel] [PATCH V4 01/12] net: Add vnet_hdr_len related arguments in NetClientState

[Jason Wang]

On 2017年05月14日 05:24, Philippe Mathieu-Daudé wrote:
> Hi Zhang
>
> On 05/11/2017 10:41 PM, Zhang Chen wrote:
>> Add vnet_hdr_len and using_vnet_hdr arguments in NetClientState
>> that make othermodule get real vnet_hdr_len easily.
>>
>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>> ---
>>  include/net/net.h | 2 ++
>>  net/net.c         | 2 ++
>>  2 files changed, 4 insertions(+)
>>
>> diff --git a/include/net/net.h b/include/net/net.h
>> index 99b28d5..70edfc0 100644
>> --- a/include/net/net.h
>> +++ b/include/net/net.h
>> @@ -100,6 +100,8 @@ struct NetClientState {
>>      unsigned int queue_index;
>>      unsigned rxfilter_notify_enabled:1;
>>      int vring_enable;
>> +    bool using_vnet_hdr;
>> +    int vnet_hdr_len;
>>      QTAILQ_HEAD(NetFilterHead, NetFilterState) filters;
>>  };
>>
>> diff --git a/net/net.c b/net/net.c
>> index 0ac3b9e..a00a0c9 100644
>> --- a/net/net.c
>> +++ b/net/net.c
>> @@ -472,6 +472,7 @@ void qemu_using_vnet_hdr(NetClientState *nc, bool 
>> enable)
>>          return;
>>      }
>>
>> +    nc->using_vnet_hdr = enable;
>>      nc->info->using_vnet_hdr(nc, enable);
>>  }
>>
>> @@ -491,6 +492,7 @@ void qemu_set_vnet_hdr_len(NetClientState *nc, 
>> int len)
>>          return;
>>      }
>>
>> +    nc->vnet_hdr_len = len;
>>      nc->info->set_vnet_hdr_len(nc, len);
>>  }
>>
>
> For what it's worth, now having those fields in NetClientState it is 
> possible to remove a deref to NetClientInfo in qemu_has_vnet_hdr() and 
> qemu_has_vnet_hdr_len().
>
> Anyway,
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>
>

Yes and this could be done on top with removing private e.g vnet_hdr_len.

Thanks

Re: [Qemu-devel] [PATCH V4 01/12] net: Add vnet_hdr_len related arguments in NetClientState

[Zhang Chen]

On 05/15/2017 11:24 AM, Jason Wang wrote:
>
>
> On 2017年05月14日 05:24, Philippe Mathieu-Daudé wrote:
>> Hi Zhang
>>
>> On 05/11/2017 10:41 PM, Zhang Chen wrote:
>>> Add vnet_hdr_len and using_vnet_hdr arguments in NetClientState
>>> that make othermodule get real vnet_hdr_len easily.
>>>
>>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>>> ---
>>>  include/net/net.h | 2 ++
>>>  net/net.c         | 2 ++
>>>  2 files changed, 4 insertions(+)
>>>
>>> diff --git a/include/net/net.h b/include/net/net.h
>>> index 99b28d5..70edfc0 100644
>>> --- a/include/net/net.h
>>> +++ b/include/net/net.h
>>> @@ -100,6 +100,8 @@ struct NetClientState {
>>>      unsigned int queue_index;
>>>      unsigned rxfilter_notify_enabled:1;
>>>      int vring_enable;
>>> +    bool using_vnet_hdr;
>>> +    int vnet_hdr_len;
>>>      QTAILQ_HEAD(NetFilterHead, NetFilterState) filters;
>>>  };
>>>
>>> diff --git a/net/net.c b/net/net.c
>>> index 0ac3b9e..a00a0c9 100644
>>> --- a/net/net.c
>>> +++ b/net/net.c
>>> @@ -472,6 +472,7 @@ void qemu_using_vnet_hdr(NetClientState *nc, 
>>> bool enable)
>>>          return;
>>>      }
>>>
>>> +    nc->using_vnet_hdr = enable;
>>>      nc->info->using_vnet_hdr(nc, enable);
>>>  }
>>>
>>> @@ -491,6 +492,7 @@ void qemu_set_vnet_hdr_len(NetClientState *nc, 
>>> int len)
>>>          return;
>>>      }
>>>
>>> +    nc->vnet_hdr_len = len;
>>>      nc->info->set_vnet_hdr_len(nc, len);
>>>  }
>>>
>>
>> For what it's worth, now having those fields in NetClientState it is 
>> possible to remove a deref to NetClientInfo in qemu_has_vnet_hdr() 
>> and qemu_has_vnet_hdr_len().
>>
>> Anyway,
>> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>
>>
>
> Yes and this could be done on top with removing private e.g vnet_hdr_len.

Do you means we will remove the qemu_has_vnet_hdr() and 
qemu_has_vnet_hdr_len() in the future?

Thanks
Zhang Chen

>
> Thanks
>
>
> .
>

-- 
Thanks
Zhang Chen

Re: [Qemu-devel] [PATCH V4 01/12] net: Add vnet_hdr_len related arguments in NetClientState

[Jason Wang]

On 2017年05月15日 14:56, Zhang Chen wrote:
>
>
> On 05/15/2017 11:24 AM, Jason Wang wrote:
>>
>>
>> On 2017年05月14日 05:24, Philippe Mathieu-Daudé wrote:
>>> Hi Zhang
>>>
>>> On 05/11/2017 10:41 PM, Zhang Chen wrote:
>>>> Add vnet_hdr_len and using_vnet_hdr arguments in NetClientState
>>>> that make othermodule get real vnet_hdr_len easily.
>>>>
>>>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>>>> ---
>>>>  include/net/net.h | 2 ++
>>>>  net/net.c         | 2 ++
>>>>  2 files changed, 4 insertions(+)
>>>>
>>>> diff --git a/include/net/net.h b/include/net/net.h
>>>> index 99b28d5..70edfc0 100644
>>>> --- a/include/net/net.h
>>>> +++ b/include/net/net.h
>>>> @@ -100,6 +100,8 @@ struct NetClientState {
>>>>      unsigned int queue_index;
>>>>      unsigned rxfilter_notify_enabled:1;
>>>>      int vring_enable;
>>>> +    bool using_vnet_hdr;
>>>> +    int vnet_hdr_len;
>>>>      QTAILQ_HEAD(NetFilterHead, NetFilterState) filters;
>>>>  };
>>>>
>>>> diff --git a/net/net.c b/net/net.c
>>>> index 0ac3b9e..a00a0c9 100644
>>>> --- a/net/net.c
>>>> +++ b/net/net.c
>>>> @@ -472,6 +472,7 @@ void qemu_using_vnet_hdr(NetClientState *nc, 
>>>> bool enable)
>>>>          return;
>>>>      }
>>>>
>>>> +    nc->using_vnet_hdr = enable;
>>>>      nc->info->using_vnet_hdr(nc, enable);
>>>>  }
>>>>
>>>> @@ -491,6 +492,7 @@ void qemu_set_vnet_hdr_len(NetClientState *nc, 
>>>> int len)
>>>>          return;
>>>>      }
>>>>
>>>> +    nc->vnet_hdr_len = len;
>>>>      nc->info->set_vnet_hdr_len(nc, len);
>>>>  }
>>>>
>>>
>>> For what it's worth, now having those fields in NetClientState it is 
>>> possible to remove a deref to NetClientInfo in qemu_has_vnet_hdr() 
>>> and qemu_has_vnet_hdr_len().
>>>
>>> Anyway,
>>> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>
>>>
>>
>> Yes and this could be done on top with removing private e.g 
>> vnet_hdr_len.
>
> Do you means we will remove the qemu_has_vnet_hdr() and 
> qemu_has_vnet_hdr_len() in the future?
>
> Thanks
> Zhang Chen

Yes and e.g both tap and netmap have its private vnet header field. We 
can remove them too.

Thanks

>
>>
>> Thanks
>>
>>
>> .
>>
>

Re: [Qemu-devel] [PATCH V4 01/12] net: Add vnet_hdr_len related arguments in NetClientState

[Zhang Chen]

On 05/15/2017 04:06 PM, Jason Wang wrote:
>
>
> On 2017年05月15日 14:56, Zhang Chen wrote:
>>
>>
>> On 05/15/2017 11:24 AM, Jason Wang wrote:
>>>
>>>
>>> On 2017年05月14日 05:24, Philippe Mathieu-Daudé wrote:
>>>> Hi Zhang
>>>>
>>>> On 05/11/2017 10:41 PM, Zhang Chen wrote:
>>>>> Add vnet_hdr_len and using_vnet_hdr arguments in NetClientState
>>>>> that make othermodule get real vnet_hdr_len easily.
>>>>>
>>>>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>>>>> ---
>>>>>  include/net/net.h | 2 ++
>>>>>  net/net.c         | 2 ++
>>>>>  2 files changed, 4 insertions(+)
>>>>>
>>>>> diff --git a/include/net/net.h b/include/net/net.h
>>>>> index 99b28d5..70edfc0 100644
>>>>> --- a/include/net/net.h
>>>>> +++ b/include/net/net.h
>>>>> @@ -100,6 +100,8 @@ struct NetClientState {
>>>>>      unsigned int queue_index;
>>>>>      unsigned rxfilter_notify_enabled:1;
>>>>>      int vring_enable;
>>>>> +    bool using_vnet_hdr;
>>>>> +    int vnet_hdr_len;
>>>>>      QTAILQ_HEAD(NetFilterHead, NetFilterState) filters;
>>>>>  };
>>>>>
>>>>> diff --git a/net/net.c b/net/net.c
>>>>> index 0ac3b9e..a00a0c9 100644
>>>>> --- a/net/net.c
>>>>> +++ b/net/net.c
>>>>> @@ -472,6 +472,7 @@ void qemu_using_vnet_hdr(NetClientState *nc, 
>>>>> bool enable)
>>>>>          return;
>>>>>      }
>>>>>
>>>>> +    nc->using_vnet_hdr = enable;
>>>>>      nc->info->using_vnet_hdr(nc, enable);
>>>>>  }
>>>>>
>>>>> @@ -491,6 +492,7 @@ void qemu_set_vnet_hdr_len(NetClientState *nc, 
>>>>> int len)
>>>>>          return;
>>>>>      }
>>>>>
>>>>> +    nc->vnet_hdr_len = len;
>>>>>      nc->info->set_vnet_hdr_len(nc, len);
>>>>>  }
>>>>>
>>>>
>>>> For what it's worth, now having those fields in NetClientState it 
>>>> is possible to remove a deref to NetClientInfo in 
>>>> qemu_has_vnet_hdr() and qemu_has_vnet_hdr_len().
>>>>
>>>> Anyway,
>>>> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>>
>>>>
>>>
>>> Yes and this could be done on top with removing private e.g 
>>> vnet_hdr_len.
>>
>> Do you means we will remove the qemu_has_vnet_hdr() and 
>> qemu_has_vnet_hdr_len() in the future?
>>
>> Thanks
>> Zhang Chen
>
> Yes and e.g both tap and netmap have its private vnet header field. We 
> can remove them too.

Maybe I should do this job in this series next version or a independent 
series?

Thanks
Zhang Chen

>
> Thanks
>
>>
>>>
>>> Thanks
>>>
>>>
>>> .
>>>
>>
>
>
>
> .
>

-- 
Thanks
Zhang Chen

Re: [Qemu-devel] [PATCH V4 01/12] net: Add vnet_hdr_len related arguments in NetClientState

[Jason Wang]

On 2017年05月15日 16:14, Zhang Chen wrote:
>> Yes and e.g both tap and netmap have its private vnet header field. 
>> We can remove them too.
>
> Maybe I should do this job in this series next version or a 
> independent series?
>
> Thanks
> Zhang Chen 

Your call.

Thanks

Re: [Qemu-devel] [PATCH V4 01/12] net: Add vnet_hdr_len related arguments in NetClientState

[Zhang Chen]

On 05/16/2017 02:49 PM, Jason Wang wrote:
>
>
> On 2017年05月15日 16:14, Zhang Chen wrote:
>>> Yes and e.g both tap and netmap have its private vnet header field. 
>>> We can remove them too.
>>
>> Maybe I should do this job in this series next version or a 
>> independent series?
>>
>> Thanks
>> Zhang Chen 
>
> Your call.

I will send a independent series after this series have been merged.

Thanks
Zhang Chen

>
> Thanks
>
>
>

-- 
Thanks
Zhang Chen

[Qemu-devel] [PATCH V4 02/12] net/filter-mirror.c: Add new option to enable vnet support for filter-mirror

[Zhang Chen]

We add the vnet_hdr option for filter-mirror, default is disable.
If you use virtio-net-pci net driver, please enable it.
You can use it for example:
-object filter-mirror,id=m0,netdev=hn0,queue=tx,outdev=mirror0,vnet_hdr=on

Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
---
 net/filter-mirror.c | 34 ++++++++++++++++++++++++++++++++++
 qemu-options.hx     |  5 +++--
 2 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/net/filter-mirror.c b/net/filter-mirror.c
index 72fa7c2..3766414 100644
--- a/net/filter-mirror.c
+++ b/net/filter-mirror.c
@@ -38,6 +38,7 @@ typedef struct MirrorState {
     NetFilterState parent_obj;
     char *indev;
     char *outdev;
+    bool vnet_hdr;
     CharBackend chr_in;
     CharBackend chr_out;
     SocketReadState rs;
@@ -308,6 +309,13 @@ static char *filter_mirror_get_outdev(Object *obj, Error **errp)
     return g_strdup(s->outdev);
 }
 
+static char *filter_mirror_get_vnet_hdr(Object *obj, Error **errp)
+{
+    MirrorState *s = FILTER_MIRROR(obj);
+
+    return s->vnet_hdr ? g_strdup("on") : g_strdup("off");
+}
+
 static void
 filter_mirror_set_outdev(Object *obj, const char *value, Error **errp)
 {
@@ -322,6 +330,21 @@ filter_mirror_set_outdev(Object *obj, const char *value, Error **errp)
     }
 }
 
+static void filter_mirror_set_vnet_hdr(Object *obj,
+                                       const char *value,
+                                       Error **errp)
+{
+    MirrorState *s = FILTER_MIRROR(obj);
+
+    if (strcmp(value, "on") && strcmp(value, "off")) {
+        error_setg(errp, "Invalid value for filter-mirror vnet_hdr, "
+                         "should be 'on' or 'off'");
+        return;
+    }
+
+    s->vnet_hdr = !strcmp(value, "on");
+}
+
 static char *filter_redirector_get_outdev(Object *obj, Error **errp)
 {
     MirrorState *s = FILTER_REDIRECTOR(obj);
@@ -340,8 +363,19 @@ filter_redirector_set_outdev(Object *obj, const char *value, Error **errp)
 
 static void filter_mirror_init(Object *obj)
 {
+    MirrorState *s = FILTER_MIRROR(obj);
+
     object_property_add_str(obj, "outdev", filter_mirror_get_outdev,
                             filter_mirror_set_outdev, NULL);
+
+    /*
+     * The vnet_hdr is disabled by default, if you want to enable
+     * this option, you must enable all the option on related modules
+     * (like other filter or colo-compare).
+     */
+    s->vnet_hdr = false;
+    object_property_add_str(obj, "vnet_hdr", filter_mirror_get_vnet_hdr,
+                             filter_mirror_set_vnet_hdr, NULL);
 }
 
 static void filter_redirector_init(Object *obj)
diff --git a/qemu-options.hx b/qemu-options.hx
index 70c0ded..1e08481 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4024,10 +4024,11 @@ queue @var{all|rx|tx} is an option that can be applied to any netfilter.
 @option{tx}: the filter is attached to the transmit queue of the netdev,
              where it will receive packets sent by the netdev.
 
-@item -object filter-mirror,id=@var{id},netdev=@var{netdevid},outdev=@var{chardevid}[,queue=@var{all|rx|tx}]
+@item -object filter-mirror,id=@var{id},netdev=@var{netdevid},outdev=@var{chardevid},vnet_hdr=@var{on|off}[,queue=@var{all|rx|tx}]
 
 filter-mirror on netdev @var{netdevid},mirror net packet to chardev
-@var{chardevid}
+@var{chardevid}, if vnet_hdr = on, filter-mirror will mirror packet
+with vnet_hdr_len.
 
 @item -object filter-redirector,id=@var{id},netdev=@var{netdevid},indev=@var{chardevid},
 outdev=@var{chardevid}[,queue=@var{all|rx|tx}]
-- 
2.7.4

Re: [Qemu-devel] [PATCH V4 02/12] net/filter-mirror.c: Add new option to enable vnet support for filter-mirror

[Hailiang Zhang]

Hi,

On 2017/5/12 9:41, Zhang Chen wrote:
> We add the vnet_hdr option for filter-mirror, default is disable.
> If you use virtio-net-pci net driver, please enable it.
> You can use it for example:
> -object filter-mirror,id=m0,netdev=hn0,queue=tx,outdev=mirror0,vnet_hdr=on

Is there any way to detect whether or not the vNIC using vnet_hdr ?
I don't think it is a good idea to let users to confirm it, especially for users who may not
be so familiar with the vNIC realizing in qemu.


Thanks,
Hailiang

> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
> ---
>   net/filter-mirror.c | 34 ++++++++++++++++++++++++++++++++++
>   qemu-options.hx     |  5 +++--
>   2 files changed, 37 insertions(+), 2 deletions(-)
>
> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
> index 72fa7c2..3766414 100644
> --- a/net/filter-mirror.c
> +++ b/net/filter-mirror.c
> @@ -38,6 +38,7 @@ typedef struct MirrorState {
>       NetFilterState parent_obj;
>       char *indev;
>       char *outdev;
> +    bool vnet_hdr;
>       CharBackend chr_in;
>       CharBackend chr_out;
>       SocketReadState rs;
> @@ -308,6 +309,13 @@ static char *filter_mirror_get_outdev(Object *obj, Error **errp)
>       return g_strdup(s->outdev);
>   }
>   
> +static char *filter_mirror_get_vnet_hdr(Object *obj, Error **errp)
> +{
> +    MirrorState *s = FILTER_MIRROR(obj);
> +
> +    return s->vnet_hdr ? g_strdup("on") : g_strdup("off");
> +}
> +
>   static void
>   filter_mirror_set_outdev(Object *obj, const char *value, Error **errp)
>   {
> @@ -322,6 +330,21 @@ filter_mirror_set_outdev(Object *obj, const char *value, Error **errp)
>       }
>   }
>   
> +static void filter_mirror_set_vnet_hdr(Object *obj,
> +                                       const char *value,
> +                                       Error **errp)
> +{
> +    MirrorState *s = FILTER_MIRROR(obj);
> +
> +    if (strcmp(value, "on") && strcmp(value, "off")) {
> +        error_setg(errp, "Invalid value for filter-mirror vnet_hdr, "
> +                         "should be 'on' or 'off'");
> +        return;
> +    }
> +
> +    s->vnet_hdr = !strcmp(value, "on");
> +}
> +
>   static char *filter_redirector_get_outdev(Object *obj, Error **errp)
>   {
>       MirrorState *s = FILTER_REDIRECTOR(obj);
> @@ -340,8 +363,19 @@ filter_redirector_set_outdev(Object *obj, const char *value, Error **errp)
>   
>   static void filter_mirror_init(Object *obj)
>   {
> +    MirrorState *s = FILTER_MIRROR(obj);
> +
>       object_property_add_str(obj, "outdev", filter_mirror_get_outdev,
>                               filter_mirror_set_outdev, NULL);
> +
> +    /*
> +     * The vnet_hdr is disabled by default, if you want to enable
> +     * this option, you must enable all the option on related modules
> +     * (like other filter or colo-compare).
> +     */
> +    s->vnet_hdr = false;
> +    object_property_add_str(obj, "vnet_hdr", filter_mirror_get_vnet_hdr,
> +                             filter_mirror_set_vnet_hdr, NULL);
>   }
>   
>   static void filter_redirector_init(Object *obj)
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 70c0ded..1e08481 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -4024,10 +4024,11 @@ queue @var{all|rx|tx} is an option that can be applied to any netfilter.
>   @option{tx}: the filter is attached to the transmit queue of the netdev,
>                where it will receive packets sent by the netdev.
>   
> -@item -object filter-mirror,id=@var{id},netdev=@var{netdevid},outdev=@var{chardevid}[,queue=@var{all|rx|tx}]
> +@item -object filter-mirror,id=@var{id},netdev=@var{netdevid},outdev=@var{chardevid},vnet_hdr=@var{on|off}[,queue=@var{all|rx|tx}]
>   
>   filter-mirror on netdev @var{netdevid},mirror net packet to chardev
> -@var{chardevid}
> +@var{chardevid}, if vnet_hdr = on, filter-mirror will mirror packet
> +with vnet_hdr_len.
>   
>   @item -object filter-redirector,id=@var{id},netdev=@var{netdevid},indev=@var{chardevid},
>   outdev=@var{chardevid}[,queue=@var{all|rx|tx}]

Re: [Qemu-devel] [PATCH V4 02/12] net/filter-mirror.c: Add new option to enable vnet support for filter-mirror

[Zhang Chen]

On 05/13/2017 09:49 AM, Hailiang Zhang wrote:
> Hi,
>
> On 2017/5/12 9:41, Zhang Chen wrote:
>> We add the vnet_hdr option for filter-mirror, default is disable.
>> If you use virtio-net-pci net driver, please enable it.
>> You can use it for example:
>> -object 
>> filter-mirror,id=m0,netdev=hn0,queue=tx,outdev=mirror0,vnet_hdr=on
>
> Is there any way to detect whether or not the vNIC using vnet_hdr ?

Yes, we can. but we can't ensure the backward compatibility.
Detail:
https://lists.nongnu.org/archive/html/qemu-devel/2017-05/msg01959.html

> I don't think it is a good idea to let users to confirm it, especially 
> for users who may not
> be so familiar with the vNIC realizing in qemu.
>

If you don't use virtio-net-pci that you need't add this option, default 
is false.

Thanks
Zhang Chen

>
> Thanks,
> Hailiang
>
>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>> ---
>>   net/filter-mirror.c | 34 ++++++++++++++++++++++++++++++++++
>>   qemu-options.hx     |  5 +++--
>>   2 files changed, 37 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
>> index 72fa7c2..3766414 100644
>> --- a/net/filter-mirror.c
>> +++ b/net/filter-mirror.c
>> @@ -38,6 +38,7 @@ typedef struct MirrorState {
>>       NetFilterState parent_obj;
>>       char *indev;
>>       char *outdev;
>> +    bool vnet_hdr;
>>       CharBackend chr_in;
>>       CharBackend chr_out;
>>       SocketReadState rs;
>> @@ -308,6 +309,13 @@ static char *filter_mirror_get_outdev(Object 
>> *obj, Error **errp)
>>       return g_strdup(s->outdev);
>>   }
>>   +static char *filter_mirror_get_vnet_hdr(Object *obj, Error **errp)
>> +{
>> +    MirrorState *s = FILTER_MIRROR(obj);
>> +
>> +    return s->vnet_hdr ? g_strdup("on") : g_strdup("off");
>> +}
>> +
>>   static void
>>   filter_mirror_set_outdev(Object *obj, const char *value, Error **errp)
>>   {
>> @@ -322,6 +330,21 @@ filter_mirror_set_outdev(Object *obj, const char 
>> *value, Error **errp)
>>       }
>>   }
>>   +static void filter_mirror_set_vnet_hdr(Object *obj,
>> +                                       const char *value,
>> +                                       Error **errp)
>> +{
>> +    MirrorState *s = FILTER_MIRROR(obj);
>> +
>> +    if (strcmp(value, "on") && strcmp(value, "off")) {
>> +        error_setg(errp, "Invalid value for filter-mirror vnet_hdr, "
>> +                         "should be 'on' or 'off'");
>> +        return;
>> +    }
>> +
>> +    s->vnet_hdr = !strcmp(value, "on");
>> +}
>> +
>>   static char *filter_redirector_get_outdev(Object *obj, Error **errp)
>>   {
>>       MirrorState *s = FILTER_REDIRECTOR(obj);
>> @@ -340,8 +363,19 @@ filter_redirector_set_outdev(Object *obj, const 
>> char *value, Error **errp)
>>     static void filter_mirror_init(Object *obj)
>>   {
>> +    MirrorState *s = FILTER_MIRROR(obj);
>> +
>>       object_property_add_str(obj, "outdev", filter_mirror_get_outdev,
>>                               filter_mirror_set_outdev, NULL);
>> +
>> +    /*
>> +     * The vnet_hdr is disabled by default, if you want to enable
>> +     * this option, you must enable all the option on related modules
>> +     * (like other filter or colo-compare).
>> +     */
>> +    s->vnet_hdr = false;
>> +    object_property_add_str(obj, "vnet_hdr", 
>> filter_mirror_get_vnet_hdr,
>> +                             filter_mirror_set_vnet_hdr, NULL);
>>   }
>>     static void filter_redirector_init(Object *obj)
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index 70c0ded..1e08481 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -4024,10 +4024,11 @@ queue @var{all|rx|tx} is an option that can 
>> be applied to any netfilter.
>>   @option{tx}: the filter is attached to the transmit queue of the 
>> netdev,
>>                where it will receive packets sent by the netdev.
>>   -@item -object 
>> filter-mirror,id=@var{id},netdev=@var{netdevid},outdev=@var{chardevid}[,queue=@var{all|rx|tx}]
>> +@item -object 
>> filter-mirror,id=@var{id},netdev=@var{netdevid},outdev=@var{chardevid},vnet_hdr=@var{on|off}[,queue=@var{all|rx|tx}]
>>     filter-mirror on netdev @var{netdevid},mirror net packet to chardev
>> -@var{chardevid}
>> +@var{chardevid}, if vnet_hdr = on, filter-mirror will mirror packet
>> +with vnet_hdr_len.
>>     @item -object 
>> filter-redirector,id=@var{id},netdev=@var{netdevid},indev=@var{chardevid},
>>   outdev=@var{chardevid}[,queue=@var{all|rx|tx}]
>
>
>
>
> .
>

-- 
Thanks
Zhang Chen

Re: [Qemu-devel] [PATCH V4 02/12] net/filter-mirror.c: Add new option to enable vnet support for filter-mirror

[Jason Wang]

On 2017年05月12日 09:41, Zhang Chen wrote:
> We add the vnet_hdr option for filter-mirror, default is disable.
> If you use virtio-net-pci net driver, please enable it.
> You can use it for example:
> -object filter-mirror,id=m0,netdev=hn0,queue=tx,outdev=mirror0,vnet_hdr=on
>
> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
> ---
>   net/filter-mirror.c | 34 ++++++++++++++++++++++++++++++++++
>   qemu-options.hx     |  5 +++--
>   2 files changed, 37 insertions(+), 2 deletions(-)
>
> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
> index 72fa7c2..3766414 100644
> --- a/net/filter-mirror.c
> +++ b/net/filter-mirror.c
> @@ -38,6 +38,7 @@ typedef struct MirrorState {
>       NetFilterState parent_obj;
>       char *indev;
>       char *outdev;
> +    bool vnet_hdr;
>       CharBackend chr_in;
>       CharBackend chr_out;
>       SocketReadState rs;
> @@ -308,6 +309,13 @@ static char *filter_mirror_get_outdev(Object *obj, Error **errp)
>       return g_strdup(s->outdev);
>   }
>   
> +static char *filter_mirror_get_vnet_hdr(Object *obj, Error **errp)
> +{
> +    MirrorState *s = FILTER_MIRROR(obj);
> +
> +    return s->vnet_hdr ? g_strdup("on") : g_strdup("off");
> +}
> +
>   static void
>   filter_mirror_set_outdev(Object *obj, const char *value, Error **errp)
>   {
> @@ -322,6 +330,21 @@ filter_mirror_set_outdev(Object *obj, const char *value, Error **errp)
>       }
>   }
>   
> +static void filter_mirror_set_vnet_hdr(Object *obj,
> +                                       const char *value,
> +                                       Error **errp)
> +{
> +    MirrorState *s = FILTER_MIRROR(obj);
> +
> +    if (strcmp(value, "on") && strcmp(value, "off")) {
> +        error_setg(errp, "Invalid value for filter-mirror vnet_hdr, "
> +                         "should be 'on' or 'off'");
> +        return;
> +    }
> +
> +    s->vnet_hdr = !strcmp(value, "on");
> +}
> +
>   static char *filter_redirector_get_outdev(Object *obj, Error **errp)
>   {
>       MirrorState *s = FILTER_REDIRECTOR(obj);
> @@ -340,8 +363,19 @@ filter_redirector_set_outdev(Object *obj, const char *value, Error **errp)
>   
>   static void filter_mirror_init(Object *obj)
>   {
> +    MirrorState *s = FILTER_MIRROR(obj);
> +
>       object_property_add_str(obj, "outdev", filter_mirror_get_outdev,
>                               filter_mirror_set_outdev, NULL);
> +
> +    /*
> +     * The vnet_hdr is disabled by default, if you want to enable
> +     * this option, you must enable all the option on related modules
> +     * (like other filter or colo-compare).
> +     */
> +    s->vnet_hdr = false;
> +    object_property_add_str(obj, "vnet_hdr", filter_mirror_get_vnet_hdr,
> +                             filter_mirror_set_vnet_hdr, NULL);
>   }

We'd better squash this into patch 3 and even after the changes for 
net_fill_rstate().

>   
>   static void filter_redirector_init(Object *obj)
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 70c0ded..1e08481 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -4024,10 +4024,11 @@ queue @var{all|rx|tx} is an option that can be applied to any netfilter.
>   @option{tx}: the filter is attached to the transmit queue of the netdev,
>                where it will receive packets sent by the netdev.
>   
> -@item -object filter-mirror,id=@var{id},netdev=@var{netdevid},outdev=@var{chardevid}[,queue=@var{all|rx|tx}]
> +@item -object filter-mirror,id=@var{id},netdev=@var{netdevid},outdev=@var{chardevid},vnet_hdr=@var{on|off}[,queue=@var{all|rx|tx}]
>   
>   filter-mirror on netdev @var{netdevid},mirror net packet to chardev
> -@var{chardevid}
> +@var{chardevid}, if vnet_hdr = on, filter-mirror will mirror packet
> +with vnet_hdr_len.

As pointed by Eric, we'd better keep the long line here.

Thanks

>   
>   @item -object filter-redirector,id=@var{id},netdev=@var{netdevid},indev=@var{chardevid},
>   outdev=@var{chardevid}[,queue=@var{all|rx|tx}]

Re: [Qemu-devel] [PATCH V4 02/12] net/filter-mirror.c: Add new option to enable vnet support for filter-mirror

[Zhang Chen]

On 05/15/2017 11:26 AM, Jason Wang wrote:
>
>
> On 2017年05月12日 09:41, Zhang Chen wrote:
>> We add the vnet_hdr option for filter-mirror, default is disable.
>> If you use virtio-net-pci net driver, please enable it.
>> You can use it for example:
>> -object 
>> filter-mirror,id=m0,netdev=hn0,queue=tx,outdev=mirror0,vnet_hdr=on
>>
>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>> ---
>>   net/filter-mirror.c | 34 ++++++++++++++++++++++++++++++++++
>>   qemu-options.hx     |  5 +++--
>>   2 files changed, 37 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
>> index 72fa7c2..3766414 100644
>> --- a/net/filter-mirror.c
>> +++ b/net/filter-mirror.c
>> @@ -38,6 +38,7 @@ typedef struct MirrorState {
>>       NetFilterState parent_obj;
>>       char *indev;
>>       char *outdev;
>> +    bool vnet_hdr;
>>       CharBackend chr_in;
>>       CharBackend chr_out;
>>       SocketReadState rs;
>> @@ -308,6 +309,13 @@ static char *filter_mirror_get_outdev(Object 
>> *obj, Error **errp)
>>       return g_strdup(s->outdev);
>>   }
>>   +static char *filter_mirror_get_vnet_hdr(Object *obj, Error **errp)
>> +{
>> +    MirrorState *s = FILTER_MIRROR(obj);
>> +
>> +    return s->vnet_hdr ? g_strdup("on") : g_strdup("off");
>> +}
>> +
>>   static void
>>   filter_mirror_set_outdev(Object *obj, const char *value, Error **errp)
>>   {
>> @@ -322,6 +330,21 @@ filter_mirror_set_outdev(Object *obj, const char 
>> *value, Error **errp)
>>       }
>>   }
>>   +static void filter_mirror_set_vnet_hdr(Object *obj,
>> +                                       const char *value,
>> +                                       Error **errp)
>> +{
>> +    MirrorState *s = FILTER_MIRROR(obj);
>> +
>> +    if (strcmp(value, "on") && strcmp(value, "off")) {
>> +        error_setg(errp, "Invalid value for filter-mirror vnet_hdr, "
>> +                         "should be 'on' or 'off'");
>> +        return;
>> +    }
>> +
>> +    s->vnet_hdr = !strcmp(value, "on");
>> +}
>> +
>>   static char *filter_redirector_get_outdev(Object *obj, Error **errp)
>>   {
>>       MirrorState *s = FILTER_REDIRECTOR(obj);
>> @@ -340,8 +363,19 @@ filter_redirector_set_outdev(Object *obj, const 
>> char *value, Error **errp)
>>     static void filter_mirror_init(Object *obj)
>>   {
>> +    MirrorState *s = FILTER_MIRROR(obj);
>> +
>>       object_property_add_str(obj, "outdev", filter_mirror_get_outdev,
>>                               filter_mirror_set_outdev, NULL);
>> +
>> +    /*
>> +     * The vnet_hdr is disabled by default, if you want to enable
>> +     * this option, you must enable all the option on related modules
>> +     * (like other filter or colo-compare).
>> +     */
>> +    s->vnet_hdr = false;
>> +    object_property_add_str(obj, "vnet_hdr", 
>> filter_mirror_get_vnet_hdr,
>> +                             filter_mirror_set_vnet_hdr, NULL);
>>   }
>
> We'd better squash this into patch 3 and even after the changes for 
> net_fill_rstate().

OK, I will squash it in next version.

>
>>     static void filter_redirector_init(Object *obj)
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index 70c0ded..1e08481 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -4024,10 +4024,11 @@ queue @var{all|rx|tx} is an option that can 
>> be applied to any netfilter.
>>   @option{tx}: the filter is attached to the transmit queue of the 
>> netdev,
>>                where it will receive packets sent by the netdev.
>>   -@item -object 
>> filter-mirror,id=@var{id},netdev=@var{netdevid},outdev=@var{chardevid}[,queue=@var{all|rx|tx}]
>> +@item -object 
>> filter-mirror,id=@var{id},netdev=@var{netdevid},outdev=@var{chardevid},vnet_hdr=@var{on|off}[,queue=@var{all|rx|tx}]
>>     filter-mirror on netdev @var{netdevid},mirror net packet to chardev
>> -@var{chardevid}
>> +@var{chardevid}, if vnet_hdr = on, filter-mirror will mirror packet
>> +with vnet_hdr_len.
>
> As pointed by Eric, we'd better keep the long line here.

OK.

Thanks
Zhang Chen

>
> Thanks
>
>>     @item -object 
>> filter-redirector,id=@var{id},netdev=@var{netdevid},indev=@var{chardevid},
>>   outdev=@var{chardevid}[,queue=@var{all|rx|tx}]
>
>
>
> .
>

-- 
Thanks
Zhang Chen

[Qemu-devel] [PATCH V4 03/12] net/filter-mirror.c: Make filter_mirror_send support vnet support.

[Zhang Chen]

In this patch, if vnet_hdr=on we change the send packet format from
struct {int size; const uint8_t buf[];} to {int size; int vnet_hdr_len; const uint8_t buf[];}.
make other module(like colo-compare) know how to parse net packet correctly.

Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
---
 net/filter-mirror.c | 35 ++++++++++++++++++++++++++++++-----
 1 file changed, 30 insertions(+), 5 deletions(-)

diff --git a/net/filter-mirror.c b/net/filter-mirror.c
index 3766414..64323fc 100644
--- a/net/filter-mirror.c
+++ b/net/filter-mirror.c
@@ -44,10 +44,11 @@ typedef struct MirrorState {
     SocketReadState rs;
 } MirrorState;
 
-static int filter_mirror_send(CharBackend *chr_out,
+static int filter_mirror_send(MirrorState *s,
                               const struct iovec *iov,
                               int iovcnt)
 {
+    NetFilterState *nf = NETFILTER(s);
     int ret = 0;
     ssize_t size = 0;
     uint32_t len = 0;
@@ -59,14 +60,38 @@ static int filter_mirror_send(CharBackend *chr_out,
     }
 
     len = htonl(size);
-    ret = qemu_chr_fe_write_all(chr_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;
     }
 
+    if (s->vnet_hdr) {
+        /*
+         * If vnet_hdr = on, we send vnet header len to make other
+         * module(like colo-compare) know how to parse net
+         * packet correctly.
+         */
+        ssize_t vnet_hdr_len;
+
+        if (nf->netdev->using_vnet_hdr) {
+            vnet_hdr_len = nf->netdev->vnet_hdr_len;
+        } else if (nf->netdev->peer->using_vnet_hdr) {
+            vnet_hdr_len = nf->netdev->peer->vnet_hdr_len;
+        } else {
+            error_report("filter get vnet_hdr_len failed");
+            goto err;
+        }
+
+        len = htonl(vnet_hdr_len);
+        ret = qemu_chr_fe_write_all(&s->chr_out, (uint8_t *)&len, sizeof(len));
+        if (ret != sizeof(len)) {
+            goto err;
+        }
+    }
+
     buf = g_malloc(size);
     iov_to_buf(iov, iovcnt, 0, buf, size);
-    ret = qemu_chr_fe_write_all(chr_out, (uint8_t *)buf, size);
+    ret = qemu_chr_fe_write_all(&s->chr_out, (uint8_t *)buf, size);
     g_free(buf);
     if (ret != size) {
         goto err;
@@ -142,7 +167,7 @@ static ssize_t filter_mirror_receive_iov(NetFilterState *nf,
     MirrorState *s = FILTER_MIRROR(nf);
     int ret;
 
-    ret = filter_mirror_send(&s->chr_out, iov, iovcnt);
+    ret = filter_mirror_send(s, iov, iovcnt);
     if (ret) {
         error_report("filter_mirror_send failed(%s)", strerror(-ret));
     }
@@ -165,7 +190,7 @@ static ssize_t filter_redirector_receive_iov(NetFilterState *nf,
     int ret;
 
     if (qemu_chr_fe_get_driver(&s->chr_out)) {
-        ret = filter_mirror_send(&s->chr_out, iov, iovcnt);
+        ret = filter_mirror_send(s, iov, iovcnt);
         if (ret) {
             error_report("filter_mirror_send failed(%s)", strerror(-ret));
         }
-- 
2.7.4

Re: [Qemu-devel] [PATCH V4 03/12] net/filter-mirror.c: Make filter_mirror_send support vnet support.

[Jason Wang]

On 2017年05月12日 09:41, Zhang Chen wrote:
> In this patch, if vnet_hdr=on we change the send packet format from
> struct {int size; const uint8_t buf[];} to {int size; int vnet_hdr_len; const uint8_t buf[];}.
> make other module(like colo-compare) know how to parse net packet correctly.
>
> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
> ---
>   net/filter-mirror.c | 35 ++++++++++++++++++++++++++++++-----
>   1 file changed, 30 insertions(+), 5 deletions(-)
>
> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
> index 3766414..64323fc 100644
> --- a/net/filter-mirror.c
> +++ b/net/filter-mirror.c
> @@ -44,10 +44,11 @@ typedef struct MirrorState {
>       SocketReadState rs;
>   } MirrorState;
>   
> -static int filter_mirror_send(CharBackend *chr_out,
> +static int filter_mirror_send(MirrorState *s,
>                                 const struct iovec *iov,
>                                 int iovcnt)
>   {
> +    NetFilterState *nf = NETFILTER(s);
>       int ret = 0;
>       ssize_t size = 0;
>       uint32_t len = 0;
> @@ -59,14 +60,38 @@ static int filter_mirror_send(CharBackend *chr_out,
>       }
>   
>       len = htonl(size);
> -    ret = qemu_chr_fe_write_all(chr_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;
>       }
>   
> +    if (s->vnet_hdr) {
> +        /*
> +         * If vnet_hdr = on, we send vnet header len to make other
> +         * module(like colo-compare) know how to parse net
> +         * packet correctly.
> +         */
> +        ssize_t vnet_hdr_len;
> +
> +        if (nf->netdev->using_vnet_hdr) {
> +            vnet_hdr_len = nf->netdev->vnet_hdr_len;
> +        } else if (nf->netdev->peer->using_vnet_hdr) {
> +            vnet_hdr_len = nf->netdev->peer->vnet_hdr_len;

Still questionable here, may need a comment to explain.

> +        } else {
> +            error_report("filter get vnet_hdr_len failed");

Why need error here, could we just use zero?

Thanks

> +            goto err;
> +        }
> +
> +        len = htonl(vnet_hdr_len);
> +        ret = qemu_chr_fe_write_all(&s->chr_out, (uint8_t *)&len, sizeof(len));
> +        if (ret != sizeof(len)) {
> +            goto err;
> +        }
> +    }
> +
>       buf = g_malloc(size);
>       iov_to_buf(iov, iovcnt, 0, buf, size);
> -    ret = qemu_chr_fe_write_all(chr_out, (uint8_t *)buf, size);
> +    ret = qemu_chr_fe_write_all(&s->chr_out, (uint8_t *)buf, size);
>       g_free(buf);
>       if (ret != size) {
>           goto err;
> @@ -142,7 +167,7 @@ static ssize_t filter_mirror_receive_iov(NetFilterState *nf,
>       MirrorState *s = FILTER_MIRROR(nf);
>       int ret;
>   
> -    ret = filter_mirror_send(&s->chr_out, iov, iovcnt);
> +    ret = filter_mirror_send(s, iov, iovcnt);
>       if (ret) {
>           error_report("filter_mirror_send failed(%s)", strerror(-ret));
>       }
> @@ -165,7 +190,7 @@ static ssize_t filter_redirector_receive_iov(NetFilterState *nf,
>       int ret;
>   
>       if (qemu_chr_fe_get_driver(&s->chr_out)) {
> -        ret = filter_mirror_send(&s->chr_out, iov, iovcnt);
> +        ret = filter_mirror_send(s, iov, iovcnt);
>           if (ret) {
>               error_report("filter_mirror_send failed(%s)", strerror(-ret));
>           }

Re: [Qemu-devel] [PATCH V4 03/12] net/filter-mirror.c: Make filter_mirror_send support vnet support.

[Zhang Chen]

On 05/15/2017 11:28 AM, Jason Wang wrote:
>
>
> On 2017年05月12日 09:41, Zhang Chen wrote:
>> In this patch, if vnet_hdr=on we change the send packet format from
>> struct {int size; const uint8_t buf[];} to {int size; int 
>> vnet_hdr_len; const uint8_t buf[];}.
>> make other module(like colo-compare) know how to parse net packet 
>> correctly.
>>
>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>> ---
>>   net/filter-mirror.c | 35 ++++++++++++++++++++++++++++++-----
>>   1 file changed, 30 insertions(+), 5 deletions(-)
>>
>> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
>> index 3766414..64323fc 100644
>> --- a/net/filter-mirror.c
>> +++ b/net/filter-mirror.c
>> @@ -44,10 +44,11 @@ typedef struct MirrorState {
>>       SocketReadState rs;
>>   } MirrorState;
>>   -static int filter_mirror_send(CharBackend *chr_out,
>> +static int filter_mirror_send(MirrorState *s,
>>                                 const struct iovec *iov,
>>                                 int iovcnt)
>>   {
>> +    NetFilterState *nf = NETFILTER(s);
>>       int ret = 0;
>>       ssize_t size = 0;
>>       uint32_t len = 0;
>> @@ -59,14 +60,38 @@ static int filter_mirror_send(CharBackend *chr_out,
>>       }
>>         len = htonl(size);
>> -    ret = qemu_chr_fe_write_all(chr_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;
>>       }
>>   +    if (s->vnet_hdr) {
>> +        /*
>> +         * If vnet_hdr = on, we send vnet header len to make other
>> +         * module(like colo-compare) know how to parse net
>> +         * packet correctly.
>> +         */
>> +        ssize_t vnet_hdr_len;
>> +
>> +        if (nf->netdev->using_vnet_hdr) {
>> +            vnet_hdr_len = nf->netdev->vnet_hdr_len;
>> +        } else if (nf->netdev->peer->using_vnet_hdr) {
>> +            vnet_hdr_len = nf->netdev->peer->vnet_hdr_len;
>
> Still questionable here, may need a comment to explain.
>

OK, I will add comments in next version.

>> +        } else {
>> +            error_report("filter get vnet_hdr_len failed");
>
> Why need error here, could we just use zero?

Yes,we can.

Thanks
Zhang Chen

>
> Thanks
>
>> +            goto err;
>> +        }
>> +
>> +        len = htonl(vnet_hdr_len);
>> +        ret = qemu_chr_fe_write_all(&s->chr_out, (uint8_t *)&len, 
>> sizeof(len));
>> +        if (ret != sizeof(len)) {
>> +            goto err;
>> +        }
>> +    }
>> +
>>       buf = g_malloc(size);
>>       iov_to_buf(iov, iovcnt, 0, buf, size);
>> -    ret = qemu_chr_fe_write_all(chr_out, (uint8_t *)buf, size);
>> +    ret = qemu_chr_fe_write_all(&s->chr_out, (uint8_t *)buf, size);
>>       g_free(buf);
>>       if (ret != size) {
>>           goto err;
>> @@ -142,7 +167,7 @@ static ssize_t 
>> filter_mirror_receive_iov(NetFilterState *nf,
>>       MirrorState *s = FILTER_MIRROR(nf);
>>       int ret;
>>   -    ret = filter_mirror_send(&s->chr_out, iov, iovcnt);
>> +    ret = filter_mirror_send(s, iov, iovcnt);
>>       if (ret) {
>>           error_report("filter_mirror_send failed(%s)", strerror(-ret));
>>       }
>> @@ -165,7 +190,7 @@ static ssize_t 
>> filter_redirector_receive_iov(NetFilterState *nf,
>>       int ret;
>>         if (qemu_chr_fe_get_driver(&s->chr_out)) {
>> -        ret = filter_mirror_send(&s->chr_out, iov, iovcnt);
>> +        ret = filter_mirror_send(s, iov, iovcnt);
>>           if (ret) {
>>               error_report("filter_mirror_send failed(%s)", 
>> strerror(-ret));
>>           }
>
>
>
> .
>

-- 
Thanks
Zhang Chen

[Qemu-devel] [PATCH V4 04/12] net/filter-mirror.c: Add new option to enable vnet support for filter-redirector

[Zhang Chen]

We add the vnet_hdr option for filter-redirector, default is disable.
If you use virtio-net-pci net driver, please enable it.
You can use it for example:
-object filter-redirector,id=r0,netdev=hn0,queue=tx,outdev=red0,vnet_hdr=on

Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
---
 net/filter-mirror.c | 33 +++++++++++++++++++++++++++++++++
 qemu-options.hx     |  5 +++--
 2 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/net/filter-mirror.c b/net/filter-mirror.c
index 64323fc..a65853c 100644
--- a/net/filter-mirror.c
+++ b/net/filter-mirror.c
@@ -377,6 +377,13 @@ static char *filter_redirector_get_outdev(Object *obj, Error **errp)
     return g_strdup(s->outdev);
 }
 
+static char *filter_redirector_get_vnet_hdr(Object *obj, Error **errp)
+{
+    MirrorState *s = FILTER_REDIRECTOR(obj);
+
+    return s->vnet_hdr ? g_strdup("on") : g_strdup("off");
+}
+
 static void
 filter_redirector_set_outdev(Object *obj, const char *value, Error **errp)
 {
@@ -386,6 +393,21 @@ filter_redirector_set_outdev(Object *obj, const char *value, Error **errp)
     s->outdev = g_strdup(value);
 }
 
+static void filter_redirector_set_vnet_hdr(Object *obj,
+                                           const char *value,
+                                           Error **errp)
+{
+    MirrorState *s = FILTER_REDIRECTOR(obj);
+
+    if (strcmp(value, "on") && strcmp(value, "off")) {
+        error_setg(errp, "Invalid value for filter-redirector vnet_hdr, "
+                         "should be 'on' or 'off'");
+        return;
+    }
+
+    s->vnet_hdr = !strcmp(value, "on");
+}
+
 static void filter_mirror_init(Object *obj)
 {
     MirrorState *s = FILTER_MIRROR(obj);
@@ -405,10 +427,21 @@ static void filter_mirror_init(Object *obj)
 
 static void filter_redirector_init(Object *obj)
 {
+    MirrorState *s = FILTER_REDIRECTOR(obj);
+
     object_property_add_str(obj, "indev", filter_redirector_get_indev,
                             filter_redirector_set_indev, NULL);
     object_property_add_str(obj, "outdev", filter_redirector_get_outdev,
                             filter_redirector_set_outdev, NULL);
+
+    /*
+     * The vnet_hdr is disabled by default, if you want to enable
+     * this option, you must enable all the option on related modules
+     * (like other filter or colo-compare).
+     */
+    s->vnet_hdr = false;
+    object_property_add_str(obj, "vnet_hdr", filter_redirector_get_vnet_hdr,
+                            filter_redirector_set_vnet_hdr, NULL);
 }
 
 static void filter_mirror_fini(Object *obj)
diff --git a/qemu-options.hx b/qemu-options.hx
index 1e08481..0f81c22 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4031,10 +4031,11 @@ filter-mirror on netdev @var{netdevid},mirror net packet to chardev
 with vnet_hdr_len.
 
 @item -object filter-redirector,id=@var{id},netdev=@var{netdevid},indev=@var{chardevid},
-outdev=@var{chardevid}[,queue=@var{all|rx|tx}]
+outdev=@var{chardevid},vnet_hdr=@var{on|off}[,queue=@var{all|rx|tx}]
 
 filter-redirector on netdev @var{netdevid},redirect filter's net packet to chardev
-@var{chardevid},and redirect indev's packet to filter.
+@var{chardevid},and redirect indev's packet to filter.if vnet_hdr = on,
+filter-redirector will redirect packet with vnet_hdr_len.
 Create a filter-redirector we need to differ outdev id from indev id, id can not
 be the same. we can just use indev or outdev, but at least one of indev or outdev
 need to be specified.
-- 
2.7.4

Re: [Qemu-devel] [PATCH V4 04/12] net/filter-mirror.c: Add new option to enable vnet support for filter-redirector

[Jason Wang]

On 2017年05月12日 09:41, Zhang Chen wrote:
> We add the vnet_hdr option for filter-redirector, default is disable.
> If you use virtio-net-pci net driver, please enable it.

We need a better commit log here to explain e.g why we need this option.

Thanks

> You can use it for example:
> -object filter-redirector,id=r0,netdev=hn0,queue=tx,outdev=red0,vnet_hdr=on
>
> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
> ---
>   net/filter-mirror.c | 33 +++++++++++++++++++++++++++++++++
>   qemu-options.hx     |  5 +++--
>   2 files changed, 36 insertions(+), 2 deletions(-)
>
> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
> index 64323fc..a65853c 100644
> --- a/net/filter-mirror.c
> +++ b/net/filter-mirror.c
> @@ -377,6 +377,13 @@ static char *filter_redirector_get_outdev(Object *obj, Error **errp)
>       return g_strdup(s->outdev);
>   }
>   
> +static char *filter_redirector_get_vnet_hdr(Object *obj, Error **errp)
> +{
> +    MirrorState *s = FILTER_REDIRECTOR(obj);
> +
> +    return s->vnet_hdr ? g_strdup("on") : g_strdup("off");
> +}
> +
>   static void
>   filter_redirector_set_outdev(Object *obj, const char *value, Error **errp)
>   {
> @@ -386,6 +393,21 @@ filter_redirector_set_outdev(Object *obj, const char *value, Error **errp)
>       s->outdev = g_strdup(value);
>   }
>   
> +static void filter_redirector_set_vnet_hdr(Object *obj,
> +                                           const char *value,
> +                                           Error **errp)
> +{
> +    MirrorState *s = FILTER_REDIRECTOR(obj);
> +
> +    if (strcmp(value, "on") && strcmp(value, "off")) {
> +        error_setg(errp, "Invalid value for filter-redirector vnet_hdr, "
> +                         "should be 'on' or 'off'");
> +        return;
> +    }
> +
> +    s->vnet_hdr = !strcmp(value, "on");
> +}
> +
>   static void filter_mirror_init(Object *obj)
>   {
>       MirrorState *s = FILTER_MIRROR(obj);
> @@ -405,10 +427,21 @@ static void filter_mirror_init(Object *obj)
>   
>   static void filter_redirector_init(Object *obj)
>   {
> +    MirrorState *s = FILTER_REDIRECTOR(obj);
> +
>       object_property_add_str(obj, "indev", filter_redirector_get_indev,
>                               filter_redirector_set_indev, NULL);
>       object_property_add_str(obj, "outdev", filter_redirector_get_outdev,
>                               filter_redirector_set_outdev, NULL);
> +
> +    /*
> +     * The vnet_hdr is disabled by default, if you want to enable
> +     * this option, you must enable all the option on related modules
> +     * (like other filter or colo-compare).
> +     */
> +    s->vnet_hdr = false;
> +    object_property_add_str(obj, "vnet_hdr", filter_redirector_get_vnet_hdr,
> +                            filter_redirector_set_vnet_hdr, NULL);
>   }
>   
>   static void filter_mirror_fini(Object *obj)
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 1e08481..0f81c22 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -4031,10 +4031,11 @@ filter-mirror on netdev @var{netdevid},mirror net packet to chardev
>   with vnet_hdr_len.
>   
>   @item -object filter-redirector,id=@var{id},netdev=@var{netdevid},indev=@var{chardevid},
> -outdev=@var{chardevid}[,queue=@var{all|rx|tx}]
> +outdev=@var{chardevid},vnet_hdr=@var{on|off}[,queue=@var{all|rx|tx}]
>   
>   filter-redirector on netdev @var{netdevid},redirect filter's net packet to chardev
> -@var{chardevid},and redirect indev's packet to filter.
> +@var{chardevid},and redirect indev's packet to filter.if vnet_hdr = on,
> +filter-redirector will redirect packet with vnet_hdr_len.
>   Create a filter-redirector we need to differ outdev id from indev id, id can not
>   be the same. we can just use indev or outdev, but at least one of indev or outdev
>   need to be specified.

Re: [Qemu-devel] [PATCH V4 04/12] net/filter-mirror.c: Add new option to enable vnet support for filter-redirector

[Zhang Chen]

On 05/15/2017 11:31 AM, Jason Wang wrote:
>
>
> On 2017年05月12日 09:41, Zhang Chen wrote:
>> We add the vnet_hdr option for filter-redirector, default is disable.
>> If you use virtio-net-pci net driver, please enable it.
>
> We need a better commit log here to explain e.g why we need this option.

OK. I will more comments in next version.

Thanks
Zhang Chen

>
> Thanks
>
>> You can use it for example:
>> -object 
>> filter-redirector,id=r0,netdev=hn0,queue=tx,outdev=red0,vnet_hdr=on
>>
>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>> ---
>>   net/filter-mirror.c | 33 +++++++++++++++++++++++++++++++++
>>   qemu-options.hx     |  5 +++--
>>   2 files changed, 36 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
>> index 64323fc..a65853c 100644
>> --- a/net/filter-mirror.c
>> +++ b/net/filter-mirror.c
>> @@ -377,6 +377,13 @@ static char *filter_redirector_get_outdev(Object 
>> *obj, Error **errp)
>>       return g_strdup(s->outdev);
>>   }
>>   +static char *filter_redirector_get_vnet_hdr(Object *obj, Error 
>> **errp)
>> +{
>> +    MirrorState *s = FILTER_REDIRECTOR(obj);
>> +
>> +    return s->vnet_hdr ? g_strdup("on") : g_strdup("off");
>> +}
>> +
>>   static void
>>   filter_redirector_set_outdev(Object *obj, const char *value, Error 
>> **errp)
>>   {
>> @@ -386,6 +393,21 @@ filter_redirector_set_outdev(Object *obj, const 
>> char *value, Error **errp)
>>       s->outdev = g_strdup(value);
>>   }
>>   +static void filter_redirector_set_vnet_hdr(Object *obj,
>> +                                           const char *value,
>> +                                           Error **errp)
>> +{
>> +    MirrorState *s = FILTER_REDIRECTOR(obj);
>> +
>> +    if (strcmp(value, "on") && strcmp(value, "off")) {
>> +        error_setg(errp, "Invalid value for filter-redirector 
>> vnet_hdr, "
>> +                         "should be 'on' or 'off'");
>> +        return;
>> +    }
>> +
>> +    s->vnet_hdr = !strcmp(value, "on");
>> +}
>> +
>>   static void filter_mirror_init(Object *obj)
>>   {
>>       MirrorState *s = FILTER_MIRROR(obj);
>> @@ -405,10 +427,21 @@ static void filter_mirror_init(Object *obj)
>>     static void filter_redirector_init(Object *obj)
>>   {
>> +    MirrorState *s = FILTER_REDIRECTOR(obj);
>> +
>>       object_property_add_str(obj, "indev", filter_redirector_get_indev,
>>                               filter_redirector_set_indev, NULL);
>>       object_property_add_str(obj, "outdev", 
>> filter_redirector_get_outdev,
>>                               filter_redirector_set_outdev, NULL);
>> +
>> +    /*
>> +     * The vnet_hdr is disabled by default, if you want to enable
>> +     * this option, you must enable all the option on related modules
>> +     * (like other filter or colo-compare).
>> +     */
>> +    s->vnet_hdr = false;
>> +    object_property_add_str(obj, "vnet_hdr", 
>> filter_redirector_get_vnet_hdr,
>> +                            filter_redirector_set_vnet_hdr, NULL);
>>   }
>>     static void filter_mirror_fini(Object *obj)
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index 1e08481..0f81c22 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -4031,10 +4031,11 @@ filter-mirror on netdev @var{netdevid},mirror 
>> net packet to chardev
>>   with vnet_hdr_len.
>>     @item -object 
>> filter-redirector,id=@var{id},netdev=@var{netdevid},indev=@var{chardevid},
>> -outdev=@var{chardevid}[,queue=@var{all|rx|tx}]
>> +outdev=@var{chardevid},vnet_hdr=@var{on|off}[,queue=@var{all|rx|tx}]
>>     filter-redirector on netdev @var{netdevid},redirect filter's net 
>> packet to chardev
>> -@var{chardevid},and redirect indev's packet to filter.
>> +@var{chardevid},and redirect indev's packet to filter.if vnet_hdr = on,
>> +filter-redirector will redirect packet with vnet_hdr_len.
>>   Create a filter-redirector we need to differ outdev id from indev 
>> id, id can not
>>   be the same. we can just use indev or outdev, but at least one of 
>> indev or outdev
>>   need to be specified.
>
>
>
> .
>

-- 
Thanks
Zhang Chen

[Qemu-devel] [PATCH V4 05/12] net/net.c: Add vnet_hdr support in SocketReadState

[Zhang Chen]

Address Jason Wang's comments add vnet header length to SocketReadState.
We add a flag to dicide whether net_fill_rstate() to read
struct  {int size; int vnet_hdr_len; const uint8_t buf[];} or not.

Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
---
 include/net/net.h   |  9 +++++++--
 net/colo-compare.c  |  4 ++--
 net/filter-mirror.c |  2 +-
 net/net.c           | 36 ++++++++++++++++++++++++++++++++----
 net/socket.c        |  2 +-
 5 files changed, 43 insertions(+), 10 deletions(-)

diff --git a/include/net/net.h b/include/net/net.h
index 70edfc0..0763636 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -113,14 +113,19 @@ typedef struct NICState {
 } NICState;
 
 struct SocketReadState {
-    int state; /* 0 = getting length, 1 = getting data */
+    /* 0 = getting length, 1 = getting vnet header length, 2 = getting data */
+    int state;
     uint32_t index;
     uint32_t packet_len;
+    uint32_t vnet_hdr_len;
     uint8_t buf[NET_BUFSIZE];
     SocketReadStateFinalize *finalize;
 };
 
-int net_fill_rstate(SocketReadState *rs, const uint8_t *buf, int size);
+int net_fill_rstate(SocketReadState *rs,
+                    const uint8_t *buf,
+                    int size,
+                    bool vnet_hdr);
 char *qemu_mac_strdup_printf(const uint8_t *macaddr);
 NetClientState *qemu_find_netdev(const char *id);
 int qemu_find_net_clients_except(const char *id, NetClientState **ncs,
diff --git a/net/colo-compare.c b/net/colo-compare.c
index 4ab80b1..332f57e 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -530,7 +530,7 @@ static void compare_pri_chr_in(void *opaque, const uint8_t *buf, int size)
     CompareState *s = COLO_COMPARE(opaque);
     int ret;
 
-    ret = net_fill_rstate(&s->pri_rs, buf, size);
+    ret = net_fill_rstate(&s->pri_rs, buf, size, false);
     if (ret == -1) {
         qemu_chr_fe_set_handlers(&s->chr_pri_in, NULL, NULL, NULL,
                                  NULL, NULL, true);
@@ -547,7 +547,7 @@ static void compare_sec_chr_in(void *opaque, const uint8_t *buf, int size)
     CompareState *s = COLO_COMPARE(opaque);
     int ret;
 
-    ret = net_fill_rstate(&s->sec_rs, buf, size);
+    ret = net_fill_rstate(&s->sec_rs, buf, size, false);
     if (ret == -1) {
         qemu_chr_fe_set_handlers(&s->chr_sec_in, NULL, NULL, NULL,
                                  NULL, NULL, true);
diff --git a/net/filter-mirror.c b/net/filter-mirror.c
index a65853c..4649416 100644
--- a/net/filter-mirror.c
+++ b/net/filter-mirror.c
@@ -134,7 +134,7 @@ static void redirector_chr_read(void *opaque, const uint8_t *buf, int size)
     MirrorState *s = FILTER_REDIRECTOR(nf);
     int ret;
 
-    ret = net_fill_rstate(&s->rs, buf, size);
+    ret = net_fill_rstate(&s->rs, buf, size, s->vnet_hdr);
 
     if (ret == -1) {
         qemu_chr_fe_set_handlers(&s->chr_in, NULL, NULL, NULL,
diff --git a/net/net.c b/net/net.c
index a00a0c9..a9c97cf 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1618,13 +1618,20 @@ void net_socket_rs_init(SocketReadState *rs,
  * 0: success
  * -1: error occurs
  */
-int net_fill_rstate(SocketReadState *rs, const uint8_t *buf, int size)
+int net_fill_rstate(SocketReadState *rs,
+                    const uint8_t *buf,
+                    int size,
+                    bool vnet_hdr)
 {
     unsigned int l;
 
     while (size > 0) {
-        /* reassemble a packet from the network */
-        switch (rs->state) { /* 0 = getting length, 1 = getting data */
+        /* Reassemble a packet from the network.
+         * 0 = getting length.
+         * 1 = getting vnet header length.
+         * 2 = getting data.
+         */
+        switch (rs->state) {
         case 0:
             l = 4 - rs->index;
             if (l > size) {
@@ -1638,10 +1645,31 @@ int net_fill_rstate(SocketReadState *rs, const uint8_t *buf, int size)
                 /* got length */
                 rs->packet_len = ntohl(*(uint32_t *)rs->buf);
                 rs->index = 0;
-                rs->state = 1;
+                if (vnet_hdr) {
+                    rs->state = 1;
+                } else {
+                    rs->state = 2;
+                    rs->vnet_hdr_len = 0;
+                }
             }
             break;
         case 1:
+            l = 4 - rs->index;
+            if (l > size) {
+                l = size;
+            }
+            memcpy(rs->buf + rs->index, buf, l);
+            buf += l;
+            size -= l;
+            rs->index += l;
+            if (rs->index == 4) {
+                /* got vnet header length */
+                rs->vnet_hdr_len = ntohl(*(uint32_t *)rs->buf);
+                rs->index = 0;
+                rs->state = 2;
+            }
+            break;
+        case 2:
             l = rs->packet_len - rs->index;
             if (l > size) {
                 l = size;
diff --git a/net/socket.c b/net/socket.c
index b8c931e..4e58eff 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -182,7 +182,7 @@ static void net_socket_send(void *opaque)
     }
     buf = buf1;
 
-    ret = net_fill_rstate(&s->rs, buf, size);
+    ret = net_fill_rstate(&s->rs, buf, size, false);
 
     if (ret == -1) {
         goto eoc;
-- 
2.7.4

Re: [Qemu-devel] [PATCH V4 05/12] net/net.c: Add vnet_hdr support in SocketReadState

[Jason Wang]

On 2017年05月12日 09:41, Zhang Chen wrote:
> Address Jason Wang's comments add vnet header length to SocketReadState.

Better to use "Suggested-by" instead of this :).

> We add a flag to dicide whether net_fill_rstate() to read
> struct  {int size; int vnet_hdr_len; const uint8_t buf[];} or not.

Few words to explain the format is better than e.g a struct.

>
> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
> ---
>   include/net/net.h   |  9 +++++++--
>   net/colo-compare.c  |  4 ++--
>   net/filter-mirror.c |  2 +-
>   net/net.c           | 36 ++++++++++++++++++++++++++++++++----
>   net/socket.c        |  2 +-
>   5 files changed, 43 insertions(+), 10 deletions(-)
>
> diff --git a/include/net/net.h b/include/net/net.h
> index 70edfc0..0763636 100644
> --- a/include/net/net.h
> +++ b/include/net/net.h
> @@ -113,14 +113,19 @@ typedef struct NICState {
>   } NICState;
>   
>   struct SocketReadState {
> -    int state; /* 0 = getting length, 1 = getting data */
> +    /* 0 = getting length, 1 = getting vnet header length, 2 = getting data */
> +    int state;
>       uint32_t index;
>       uint32_t packet_len;
> +    uint32_t vnet_hdr_len;
>       uint8_t buf[NET_BUFSIZE];
>       SocketReadStateFinalize *finalize;
>   };
>   
> -int net_fill_rstate(SocketReadState *rs, const uint8_t *buf, int size);
> +int net_fill_rstate(SocketReadState *rs,
> +                    const uint8_t *buf,
> +                    int size,
> +                    bool vnet_hdr);

Why not just move vnet_hdr to SocketReadState?

Thanks

>   char *qemu_mac_strdup_printf(const uint8_t *macaddr);
>   NetClientState *qemu_find_netdev(const char *id);
>   int qemu_find_net_clients_except(const char *id, NetClientState **ncs,
> diff --git a/net/colo-compare.c b/net/colo-compare.c
> index 4ab80b1..332f57e 100644
> --- a/net/colo-compare.c
> +++ b/net/colo-compare.c
> @@ -530,7 +530,7 @@ static void compare_pri_chr_in(void *opaque, const uint8_t *buf, int size)
>       CompareState *s = COLO_COMPARE(opaque);
>       int ret;
>   
> -    ret = net_fill_rstate(&s->pri_rs, buf, size);
> +    ret = net_fill_rstate(&s->pri_rs, buf, size, false);
>       if (ret == -1) {
>           qemu_chr_fe_set_handlers(&s->chr_pri_in, NULL, NULL, NULL,
>                                    NULL, NULL, true);
> @@ -547,7 +547,7 @@ static void compare_sec_chr_in(void *opaque, const uint8_t *buf, int size)
>       CompareState *s = COLO_COMPARE(opaque);
>       int ret;
>   
> -    ret = net_fill_rstate(&s->sec_rs, buf, size);
> +    ret = net_fill_rstate(&s->sec_rs, buf, size, false);
>       if (ret == -1) {
>           qemu_chr_fe_set_handlers(&s->chr_sec_in, NULL, NULL, NULL,
>                                    NULL, NULL, true);
> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
> index a65853c..4649416 100644
> --- a/net/filter-mirror.c
> +++ b/net/filter-mirror.c
> @@ -134,7 +134,7 @@ static void redirector_chr_read(void *opaque, const uint8_t *buf, int size)
>       MirrorState *s = FILTER_REDIRECTOR(nf);
>       int ret;
>   
> -    ret = net_fill_rstate(&s->rs, buf, size);
> +    ret = net_fill_rstate(&s->rs, buf, size, s->vnet_hdr);
>   
>       if (ret == -1) {
>           qemu_chr_fe_set_handlers(&s->chr_in, NULL, NULL, NULL,
> diff --git a/net/net.c b/net/net.c
> index a00a0c9..a9c97cf 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -1618,13 +1618,20 @@ void net_socket_rs_init(SocketReadState *rs,
>    * 0: success
>    * -1: error occurs
>    */
> -int net_fill_rstate(SocketReadState *rs, const uint8_t *buf, int size)
> +int net_fill_rstate(SocketReadState *rs,
> +                    const uint8_t *buf,
> +                    int size,
> +                    bool vnet_hdr)
>   {
>       unsigned int l;
>   
>       while (size > 0) {
> -        /* reassemble a packet from the network */
> -        switch (rs->state) { /* 0 = getting length, 1 = getting data */
> +        /* Reassemble a packet from the network.
> +         * 0 = getting length.
> +         * 1 = getting vnet header length.
> +         * 2 = getting data.
> +         */
> +        switch (rs->state) {
>           case 0:
>               l = 4 - rs->index;
>               if (l > size) {
> @@ -1638,10 +1645,31 @@ int net_fill_rstate(SocketReadState *rs, const uint8_t *buf, int size)
>                   /* got length */
>                   rs->packet_len = ntohl(*(uint32_t *)rs->buf);
>                   rs->index = 0;
> -                rs->state = 1;
> +                if (vnet_hdr) {
> +                    rs->state = 1;
> +                } else {
> +                    rs->state = 2;
> +                    rs->vnet_hdr_len = 0;
> +                }
>               }
>               break;
>           case 1:
> +            l = 4 - rs->index;
> +            if (l > size) {
> +                l = size;
> +            }
> +            memcpy(rs->buf + rs->index, buf, l);
> +            buf += l;
> +            size -= l;
> +            rs->index += l;
> +            if (rs->index == 4) {
> +                /* got vnet header length */
> +                rs->vnet_hdr_len = ntohl(*(uint32_t *)rs->buf);
> +                rs->index = 0;
> +                rs->state = 2;
> +            }
> +            break;
> +        case 2:
>               l = rs->packet_len - rs->index;
>               if (l > size) {
>                   l = size;
> diff --git a/net/socket.c b/net/socket.c
> index b8c931e..4e58eff 100644
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -182,7 +182,7 @@ static void net_socket_send(void *opaque)
>       }
>       buf = buf1;
>   
> -    ret = net_fill_rstate(&s->rs, buf, size);
> +    ret = net_fill_rstate(&s->rs, buf, size, false);
>   
>       if (ret == -1) {
>           goto eoc;

Re: [Qemu-devel] [PATCH V4 05/12] net/net.c: Add vnet_hdr support in SocketReadState

[Zhang Chen]

On 05/15/2017 12:02 PM, Jason Wang wrote:
>
>
> On 2017年05月12日 09:41, Zhang Chen wrote:
>> Address Jason Wang's comments add vnet header length to SocketReadState.
>
> Better to use "Suggested-by" instead of this :).

I got it~

>
>> We add a flag to dicide whether net_fill_rstate() to read
>> struct  {int size; int vnet_hdr_len; const uint8_t buf[];} or not.
>
> Few words to explain the format is better than e.g a struct.

OK, will fix it in next version.

>
>>
>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>> ---
>>   include/net/net.h   |  9 +++++++--
>>   net/colo-compare.c  |  4 ++--
>>   net/filter-mirror.c |  2 +-
>>   net/net.c           | 36 ++++++++++++++++++++++++++++++++----
>>   net/socket.c        |  2 +-
>>   5 files changed, 43 insertions(+), 10 deletions(-)
>>
>> diff --git a/include/net/net.h b/include/net/net.h
>> index 70edfc0..0763636 100644
>> --- a/include/net/net.h
>> +++ b/include/net/net.h
>> @@ -113,14 +113,19 @@ typedef struct NICState {
>>   } NICState;
>>     struct SocketReadState {
>> -    int state; /* 0 = getting length, 1 = getting data */
>> +    /* 0 = getting length, 1 = getting vnet header length, 2 = 
>> getting data */
>> +    int state;
>>       uint32_t index;
>>       uint32_t packet_len;
>> +    uint32_t vnet_hdr_len;
>>       uint8_t buf[NET_BUFSIZE];
>>       SocketReadStateFinalize *finalize;
>>   };
>>   -int net_fill_rstate(SocketReadState *rs, const uint8_t *buf, int 
>> size);
>> +int net_fill_rstate(SocketReadState *rs,
>> +                    const uint8_t *buf,
>> +                    int size,
>> +                    bool vnet_hdr);
>
> Why not just move vnet_hdr to SocketReadState?

Good idea.

Thanks
Zhang Chen

>
> Thanks
>
>>   char *qemu_mac_strdup_printf(const uint8_t *macaddr);
>>   NetClientState *qemu_find_netdev(const char *id);
>>   int qemu_find_net_clients_except(const char *id, NetClientState **ncs,
>> diff --git a/net/colo-compare.c b/net/colo-compare.c
>> index 4ab80b1..332f57e 100644
>> --- a/net/colo-compare.c
>> +++ b/net/colo-compare.c
>> @@ -530,7 +530,7 @@ static void compare_pri_chr_in(void *opaque, 
>> const uint8_t *buf, int size)
>>       CompareState *s = COLO_COMPARE(opaque);
>>       int ret;
>>   -    ret = net_fill_rstate(&s->pri_rs, buf, size);
>> +    ret = net_fill_rstate(&s->pri_rs, buf, size, false);
>>       if (ret == -1) {
>>           qemu_chr_fe_set_handlers(&s->chr_pri_in, NULL, NULL, NULL,
>>                                    NULL, NULL, true);
>> @@ -547,7 +547,7 @@ static void compare_sec_chr_in(void *opaque, 
>> const uint8_t *buf, int size)
>>       CompareState *s = COLO_COMPARE(opaque);
>>       int ret;
>>   -    ret = net_fill_rstate(&s->sec_rs, buf, size);
>> +    ret = net_fill_rstate(&s->sec_rs, buf, size, false);
>>       if (ret == -1) {
>>           qemu_chr_fe_set_handlers(&s->chr_sec_in, NULL, NULL, NULL,
>>                                    NULL, NULL, true);
>> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
>> index a65853c..4649416 100644
>> --- a/net/filter-mirror.c
>> +++ b/net/filter-mirror.c
>> @@ -134,7 +134,7 @@ static void redirector_chr_read(void *opaque, 
>> const uint8_t *buf, int size)
>>       MirrorState *s = FILTER_REDIRECTOR(nf);
>>       int ret;
>>   -    ret = net_fill_rstate(&s->rs, buf, size);
>> +    ret = net_fill_rstate(&s->rs, buf, size, s->vnet_hdr);
>>         if (ret == -1) {
>>           qemu_chr_fe_set_handlers(&s->chr_in, NULL, NULL, NULL,
>> diff --git a/net/net.c b/net/net.c
>> index a00a0c9..a9c97cf 100644
>> --- a/net/net.c
>> +++ b/net/net.c
>> @@ -1618,13 +1618,20 @@ void net_socket_rs_init(SocketReadState *rs,
>>    * 0: success
>>    * -1: error occurs
>>    */
>> -int net_fill_rstate(SocketReadState *rs, const uint8_t *buf, int size)
>> +int net_fill_rstate(SocketReadState *rs,
>> +                    const uint8_t *buf,
>> +                    int size,
>> +                    bool vnet_hdr)
>>   {
>>       unsigned int l;
>>         while (size > 0) {
>> -        /* reassemble a packet from the network */
>> -        switch (rs->state) { /* 0 = getting length, 1 = getting data */
>> +        /* Reassemble a packet from the network.
>> +         * 0 = getting length.
>> +         * 1 = getting vnet header length.
>> +         * 2 = getting data.
>> +         */
>> +        switch (rs->state) {
>>           case 0:
>>               l = 4 - rs->index;
>>               if (l > size) {
>> @@ -1638,10 +1645,31 @@ int net_fill_rstate(SocketReadState *rs, 
>> const uint8_t *buf, int size)
>>                   /* got length */
>>                   rs->packet_len = ntohl(*(uint32_t *)rs->buf);
>>                   rs->index = 0;
>> -                rs->state = 1;
>> +                if (vnet_hdr) {
>> +                    rs->state = 1;
>> +                } else {
>> +                    rs->state = 2;
>> +                    rs->vnet_hdr_len = 0;
>> +                }
>>               }
>>               break;
>>           case 1:
>> +            l = 4 - rs->index;
>> +            if (l > size) {
>> +                l = size;
>> +            }
>> +            memcpy(rs->buf + rs->index, buf, l);
>> +            buf += l;
>> +            size -= l;
>> +            rs->index += l;
>> +            if (rs->index == 4) {
>> +                /* got vnet header length */
>> +                rs->vnet_hdr_len = ntohl(*(uint32_t *)rs->buf);
>> +                rs->index = 0;
>> +                rs->state = 2;
>> +            }
>> +            break;
>> +        case 2:
>>               l = rs->packet_len - rs->index;
>>               if (l > size) {
>>                   l = size;
>> diff --git a/net/socket.c b/net/socket.c
>> index b8c931e..4e58eff 100644
>> --- a/net/socket.c
>> +++ b/net/socket.c
>> @@ -182,7 +182,7 @@ static void net_socket_send(void *opaque)
>>       }
>>       buf = buf1;
>>   -    ret = net_fill_rstate(&s->rs, buf, size);
>> +    ret = net_fill_rstate(&s->rs, buf, size, false);
>>         if (ret == -1) {
>>           goto eoc;
>
>
>
> .
>

-- 
Thanks
Zhang Chen

[Qemu-devel] [PATCH V4 06/12] net/colo-compare.c: Add new option to enable vnet support for colo-compare

[Zhang Chen]

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

Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
---
 net/colo-compare.c | 34 +++++++++++++++++++++++++++++++++-
 qemu-options.hx    |  3 ++-
 2 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 332f57e..99a6912 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.
@@ -642,6 +643,28 @@ static void compare_set_outdev(Object *obj, const char *value, Error **errp)
     s->outdev = g_strdup(value);
 }
 
+static char *compare_get_vnet_hdr(Object *obj, Error **errp)
+{
+    CompareState *s = COLO_COMPARE(obj);
+
+    return s->vnet_hdr ? g_strdup("on") : g_strdup("off");
+}
+
+static void compare_set_vnet_hdr(Object *obj,
+                                 const char *value,
+                                 Error **errp)
+{
+    CompareState *s = COLO_COMPARE(obj);
+
+    if (strcmp(value, "on") && strcmp(value, "off")) {
+        error_setg(errp, "Invalid value for colo-compare vnet_hdr, "
+                         "should be 'on' or 'off'");
+        return;
+    }
+
+    s->vnet_hdr = !strcmp(value, "on");
+}
+
 static void compare_pri_rs_finalize(SocketReadState *pri_rs)
 {
     CompareState *s = container_of(pri_rs, CompareState, pri_rs);
@@ -667,7 +690,6 @@ static void compare_sec_rs_finalize(SocketReadState *sec_rs)
     }
 }
 
-
 /*
  * Return 0 is success.
  * Return 1 is failed.
@@ -775,6 +797,8 @@ static void colo_compare_class_init(ObjectClass *oc, void *data)
 
 static void colo_compare_init(Object *obj)
 {
+    CompareState *s = COLO_COMPARE(obj);
+
     object_property_add_str(obj, "primary_in",
                             compare_get_pri_indev, compare_set_pri_indev,
                             NULL);
@@ -784,6 +808,14 @@ static void colo_compare_init(Object *obj)
     object_property_add_str(obj, "outdev",
                             compare_get_outdev, compare_set_outdev,
                             NULL);
+    /*
+     * The vnet_hdr is disabled by default, if you want to enable
+     * this option, you must enable all the option on related modules
+     * (like other filter or colo-compare).
+     */
+    s->vnet_hdr = false;
+    object_property_add_str(obj, "vnet_hdr", compare_get_vnet_hdr,
+                            compare_set_vnet_hdr, NULL);
 }
 
 static void colo_compare_finalize(Object *obj)
diff --git a/qemu-options.hx b/qemu-options.hx
index 0f81c22..115b83f 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4061,12 +4061,13 @@ The file format is libpcap, so it can be analyzed with tools such as tcpdump
 or Wireshark.
 
 @item -object colo-compare,id=@var{id},primary_in=@var{chardevid},secondary_in=@var{chardevid},
-outdev=@var{chardevid}
+outdev=@var{chardevid},vnet_hdr=@var{on|off}
 
 Colo-compare gets packet from primary_in@var{chardevid} and secondary_in@var{chardevid}, than compare primary packet with
 secondary packet. If the packets are same, we will output primary
 packet to outdev@var{chardevid}, else we will notify colo-frame
 do checkpoint and send primary packet to outdev@var{chardevid}.
+if vnet_hdr = on, colo compare will send/recv packet with vnet_hdr_len.
 
 we must use it with the help of filter-mirror and filter-redirector.
 
-- 
2.7.4

Re: [Qemu-devel] [PATCH V4 06/12] net/colo-compare.c: Add new option to enable vnet support for colo-compare

[Jason Wang]

On 2017年05月12日 09:41, 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
>
> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
> ---
>   net/colo-compare.c | 34 +++++++++++++++++++++++++++++++++-
>   qemu-options.hx    |  3 ++-
>   2 files changed, 35 insertions(+), 2 deletions(-)
>
> diff --git a/net/colo-compare.c b/net/colo-compare.c
> index 332f57e..99a6912 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.
> @@ -642,6 +643,28 @@ static void compare_set_outdev(Object *obj, const char *value, Error **errp)
>       s->outdev = g_strdup(value);
>   }
>   
> +static char *compare_get_vnet_hdr(Object *obj, Error **errp)
> +{
> +    CompareState *s = COLO_COMPARE(obj);
> +
> +    return s->vnet_hdr ? g_strdup("on") : g_strdup("off");
> +}
> +
> +static void compare_set_vnet_hdr(Object *obj,
> +                                 const char *value,
> +                                 Error **errp)
> +{
> +    CompareState *s = COLO_COMPARE(obj);
> +
> +    if (strcmp(value, "on") && strcmp(value, "off")) {
> +        error_setg(errp, "Invalid value for colo-compare vnet_hdr, "
> +                         "should be 'on' or 'off'");
> +        return;
> +    }
> +
> +    s->vnet_hdr = !strcmp(value, "on");
> +}
> +
>   static void compare_pri_rs_finalize(SocketReadState *pri_rs)
>   {
>       CompareState *s = container_of(pri_rs, CompareState, pri_rs);
> @@ -667,7 +690,6 @@ static void compare_sec_rs_finalize(SocketReadState *sec_rs)
>       }
>   }
>   
> -

Unnecessary whitespace change.

>   /*
>    * Return 0 is success.
>    * Return 1 is failed.
> @@ -775,6 +797,8 @@ static void colo_compare_class_init(ObjectClass *oc, void *data)
>   
>   static void colo_compare_init(Object *obj)
>   {
> +    CompareState *s = COLO_COMPARE(obj);
> +
>       object_property_add_str(obj, "primary_in",
>                               compare_get_pri_indev, compare_set_pri_indev,
>                               NULL);
> @@ -784,6 +808,14 @@ static void colo_compare_init(Object *obj)
>       object_property_add_str(obj, "outdev",
>                               compare_get_outdev, compare_set_outdev,
>                               NULL);
> +    /*
> +     * The vnet_hdr is disabled by default, if you want to enable
> +     * this option, you must enable all the option on related modules
> +     * (like other filter or colo-compare).
> +     */
> +    s->vnet_hdr = false;
> +    object_property_add_str(obj, "vnet_hdr", compare_get_vnet_hdr,
> +                            compare_set_vnet_hdr, NULL);
>   }
>   
>   static void colo_compare_finalize(Object *obj)
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 0f81c22..115b83f 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -4061,12 +4061,13 @@ The file format is libpcap, so it can be analyzed with tools such as tcpdump
>   or Wireshark.
>   
>   @item -object colo-compare,id=@var{id},primary_in=@var{chardevid},secondary_in=@var{chardevid},
> -outdev=@var{chardevid}
> +outdev=@var{chardevid},vnet_hdr=@var{on|off}
>   
>   Colo-compare gets packet from primary_in@var{chardevid} and secondary_in@var{chardevid}, than compare primary packet with
>   secondary packet. If the packets are same, we will output primary
>   packet to outdev@var{chardevid}, else we will notify colo-frame
>   do checkpoint and send primary packet to outdev@var{chardevid}.
> +if vnet_hdr = on, colo compare will send/recv packet with vnet_hdr_len.
>   
>   we must use it with the help of filter-mirror and filter-redirector.
>   

Please squash this into its function implementation.

Thanks

Re: [Qemu-devel] [PATCH V4 06/12] net/colo-compare.c: Add new option to enable vnet support for colo-compare

[Zhang Chen]

On 05/15/2017 12:03 PM, Jason Wang wrote:
>
>
> On 2017年05月12日 09:41, 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
>>
>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>> ---
>>   net/colo-compare.c | 34 +++++++++++++++++++++++++++++++++-
>>   qemu-options.hx    |  3 ++-
>>   2 files changed, 35 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/colo-compare.c b/net/colo-compare.c
>> index 332f57e..99a6912 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.
>> @@ -642,6 +643,28 @@ static void compare_set_outdev(Object *obj, 
>> const char *value, Error **errp)
>>       s->outdev = g_strdup(value);
>>   }
>>   +static char *compare_get_vnet_hdr(Object *obj, Error **errp)
>> +{
>> +    CompareState *s = COLO_COMPARE(obj);
>> +
>> +    return s->vnet_hdr ? g_strdup("on") : g_strdup("off");
>> +}
>> +
>> +static void compare_set_vnet_hdr(Object *obj,
>> +                                 const char *value,
>> +                                 Error **errp)
>> +{
>> +    CompareState *s = COLO_COMPARE(obj);
>> +
>> +    if (strcmp(value, "on") && strcmp(value, "off")) {
>> +        error_setg(errp, "Invalid value for colo-compare vnet_hdr, "
>> +                         "should be 'on' or 'off'");
>> +        return;
>> +    }
>> +
>> +    s->vnet_hdr = !strcmp(value, "on");
>> +}
>> +
>>   static void compare_pri_rs_finalize(SocketReadState *pri_rs)
>>   {
>>       CompareState *s = container_of(pri_rs, CompareState, pri_rs);
>> @@ -667,7 +690,6 @@ static void 
>> compare_sec_rs_finalize(SocketReadState *sec_rs)
>>       }
>>   }
>>   -
>
> Unnecessary whitespace change.

I will remove it in next version.

>
>>   /*
>>    * Return 0 is success.
>>    * Return 1 is failed.
>> @@ -775,6 +797,8 @@ static void colo_compare_class_init(ObjectClass 
>> *oc, void *data)
>>     static void colo_compare_init(Object *obj)
>>   {
>> +    CompareState *s = COLO_COMPARE(obj);
>> +
>>       object_property_add_str(obj, "primary_in",
>>                               compare_get_pri_indev, 
>> compare_set_pri_indev,
>>                               NULL);
>> @@ -784,6 +808,14 @@ static void colo_compare_init(Object *obj)
>>       object_property_add_str(obj, "outdev",
>>                               compare_get_outdev, compare_set_outdev,
>>                               NULL);
>> +    /*
>> +     * The vnet_hdr is disabled by default, if you want to enable
>> +     * this option, you must enable all the option on related modules
>> +     * (like other filter or colo-compare).
>> +     */
>> +    s->vnet_hdr = false;
>> +    object_property_add_str(obj, "vnet_hdr", compare_get_vnet_hdr,
>> +                            compare_set_vnet_hdr, NULL);
>>   }
>>     static void colo_compare_finalize(Object *obj)
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index 0f81c22..115b83f 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -4061,12 +4061,13 @@ The file format is libpcap, so it can be 
>> analyzed with tools such as tcpdump
>>   or Wireshark.
>>     @item -object 
>> colo-compare,id=@var{id},primary_in=@var{chardevid},secondary_in=@var{chardevid},
>> -outdev=@var{chardevid}
>> +outdev=@var{chardevid},vnet_hdr=@var{on|off}
>>     Colo-compare gets packet from primary_in@var{chardevid} and 
>> secondary_in@var{chardevid}, than compare primary packet with
>>   secondary packet. If the packets are same, we will output primary
>>   packet to outdev@var{chardevid}, else we will notify colo-frame
>>   do checkpoint and send primary packet to outdev@var{chardevid}.
>> +if vnet_hdr = on, colo compare will send/recv packet with vnet_hdr_len.
>>     we must use it with the help of filter-mirror and filter-redirector.
>
> Please squash this into its function implementation.

OK.
Thanks
Zhang Chen

>
> Thanks
>
>
> .
>

-- 
Thanks
Zhang Chen

[Qemu-devel] [PATCH V4 07/12] net/colo.c: Make vnet_hdr_len as packet property

[Zhang Chen]

We can use this property flush and send packet with vnet_hdr_len.

Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
---
 net/colo-compare.c    | 8 ++++++--
 net/colo.c            | 3 ++-
 net/colo.h            | 4 +++-
 net/filter-rewriter.c | 2 +-
 4 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 99a6912..87a9529 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -122,9 +122,13 @@ static int packet_enqueue(CompareState *s, int mode)
     Connection *conn;
 
     if (mode == PRIMARY_IN) {
-        pkt = packet_new(s->pri_rs.buf, s->pri_rs.packet_len);
+        pkt = packet_new(s->pri_rs.buf,
+                         s->pri_rs.packet_len,
+                         s->pri_rs.vnet_hdr_len);
     } else {
-        pkt = packet_new(s->sec_rs.buf, s->sec_rs.packet_len);
+        pkt = packet_new(s->sec_rs.buf,
+                         s->sec_rs.packet_len,
+                         s->sec_rs.vnet_hdr_len);
     }
 
     if (parse_packet_early(pkt)) {
diff --git a/net/colo.c b/net/colo.c
index 8cc166b..180eaed 100644
--- a/net/colo.c
+++ b/net/colo.c
@@ -153,13 +153,14 @@ void connection_destroy(void *opaque)
     g_slice_free(Connection, conn);
 }
 
-Packet *packet_new(const void *data, int size)
+Packet *packet_new(const void *data, int size, int vnet_hdr_len)
 {
     Packet *pkt = g_slice_new(Packet);
 
     pkt->data = g_memdup(data, size);
     pkt->size = size;
     pkt->creation_ms = qemu_clock_get_ms(QEMU_CLOCK_HOST);
+    pkt->vnet_hdr_len = vnet_hdr_len;
 
     return pkt;
 }
diff --git a/net/colo.h b/net/colo.h
index 7c524f3..caedb0d 100644
--- a/net/colo.h
+++ b/net/colo.h
@@ -43,6 +43,8 @@ typedef struct Packet {
     int size;
     /* Time of packet creation, in wall clock ms */
     int64_t creation_ms;
+    /* Get vnet_hdr_len from filter */
+    uint32_t vnet_hdr_len;
 } Packet;
 
 typedef struct ConnectionKey {
@@ -82,7 +84,7 @@ Connection *connection_get(GHashTable *connection_track_table,
                            ConnectionKey *key,
                            GQueue *conn_list);
 void connection_hashtable_reset(GHashTable *connection_track_table);
-Packet *packet_new(const void *data, int size);
+Packet *packet_new(const void *data, int size, int vnet_hdr_len);
 void packet_destroy(void *opaque, void *user_data);
 
 #endif /* QEMU_COLO_PROXY_H */
diff --git a/net/filter-rewriter.c b/net/filter-rewriter.c
index afa06e8..63256c7 100644
--- a/net/filter-rewriter.c
+++ b/net/filter-rewriter.c
@@ -158,7 +158,7 @@ static ssize_t colo_rewriter_receive_iov(NetFilterState *nf,
     char *buf = g_malloc0(size);
 
     iov_to_buf(iov, iovcnt, 0, buf, size);
-    pkt = packet_new(buf, size);
+    pkt = packet_new(buf, size, 0);
     g_free(buf);
 
     /*
-- 
2.7.4

Re: [Qemu-devel] [PATCH V4 07/12] net/colo.c: Make vnet_hdr_len as packet property

[Jason Wang]

On 2017年05月12日 09:41, Zhang Chen wrote:
> We can use this property flush and send packet with vnet_hdr_len.
>
> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>

Then I think it's not necessary to store vnet_hdr_len in SocketReadState?

Thanks

> ---
>   net/colo-compare.c    | 8 ++++++--
>   net/colo.c            | 3 ++-
>   net/colo.h            | 4 +++-
>   net/filter-rewriter.c | 2 +-
>   4 files changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/net/colo-compare.c b/net/colo-compare.c
> index 99a6912..87a9529 100644
> --- a/net/colo-compare.c
> +++ b/net/colo-compare.c
> @@ -122,9 +122,13 @@ static int packet_enqueue(CompareState *s, int mode)
>       Connection *conn;
>   
>       if (mode == PRIMARY_IN) {
> -        pkt = packet_new(s->pri_rs.buf, s->pri_rs.packet_len);
> +        pkt = packet_new(s->pri_rs.buf,
> +                         s->pri_rs.packet_len,
> +                         s->pri_rs.vnet_hdr_len);
>       } else {
> -        pkt = packet_new(s->sec_rs.buf, s->sec_rs.packet_len);
> +        pkt = packet_new(s->sec_rs.buf,
> +                         s->sec_rs.packet_len,
> +                         s->sec_rs.vnet_hdr_len);
>       }
>   
>       if (parse_packet_early(pkt)) {
> diff --git a/net/colo.c b/net/colo.c
> index 8cc166b..180eaed 100644
> --- a/net/colo.c
> +++ b/net/colo.c
> @@ -153,13 +153,14 @@ void connection_destroy(void *opaque)
>       g_slice_free(Connection, conn);
>   }
>   
> -Packet *packet_new(const void *data, int size)
> +Packet *packet_new(const void *data, int size, int vnet_hdr_len)
>   {
>       Packet *pkt = g_slice_new(Packet);
>   
>       pkt->data = g_memdup(data, size);
>       pkt->size = size;
>       pkt->creation_ms = qemu_clock_get_ms(QEMU_CLOCK_HOST);
> +    pkt->vnet_hdr_len = vnet_hdr_len;
>   
>       return pkt;
>   }
> diff --git a/net/colo.h b/net/colo.h
> index 7c524f3..caedb0d 100644
> --- a/net/colo.h
> +++ b/net/colo.h
> @@ -43,6 +43,8 @@ typedef struct Packet {
>       int size;
>       /* Time of packet creation, in wall clock ms */
>       int64_t creation_ms;
> +    /* Get vnet_hdr_len from filter */
> +    uint32_t vnet_hdr_len;
>   } Packet;
>   
>   typedef struct ConnectionKey {
> @@ -82,7 +84,7 @@ Connection *connection_get(GHashTable *connection_track_table,
>                              ConnectionKey *key,
>                              GQueue *conn_list);
>   void connection_hashtable_reset(GHashTable *connection_track_table);
> -Packet *packet_new(const void *data, int size);
> +Packet *packet_new(const void *data, int size, int vnet_hdr_len);
>   void packet_destroy(void *opaque, void *user_data);
>   
>   #endif /* QEMU_COLO_PROXY_H */
> diff --git a/net/filter-rewriter.c b/net/filter-rewriter.c
> index afa06e8..63256c7 100644
> --- a/net/filter-rewriter.c
> +++ b/net/filter-rewriter.c
> @@ -158,7 +158,7 @@ static ssize_t colo_rewriter_receive_iov(NetFilterState *nf,
>       char *buf = g_malloc0(size);
>   
>       iov_to_buf(iov, iovcnt, 0, buf, size);
> -    pkt = packet_new(buf, size);
> +    pkt = packet_new(buf, size, 0);
>       g_free(buf);
>   
>       /*

Re: [Qemu-devel] [PATCH V4 07/12] net/colo.c: Make vnet_hdr_len as packet property

[Zhang Chen]

On 05/15/2017 12:05 PM, Jason Wang wrote:
>
>
> On 2017年05月12日 09:41, Zhang Chen wrote:
>> We can use this property flush and send packet with vnet_hdr_len.
>>
>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>
> Then I think it's not necessary to store vnet_hdr_len in SocketReadState?

Do you means we keep the patch 05/12 in original?

Thanks
Zhang Chen

>
> Thanks
>
>> ---
>>   net/colo-compare.c    | 8 ++++++--
>>   net/colo.c            | 3 ++-
>>   net/colo.h            | 4 +++-
>>   net/filter-rewriter.c | 2 +-
>>   4 files changed, 12 insertions(+), 5 deletions(-)
>>
>> diff --git a/net/colo-compare.c b/net/colo-compare.c
>> index 99a6912..87a9529 100644
>> --- a/net/colo-compare.c
>> +++ b/net/colo-compare.c
>> @@ -122,9 +122,13 @@ static int packet_enqueue(CompareState *s, int 
>> mode)
>>       Connection *conn;
>>         if (mode == PRIMARY_IN) {
>> -        pkt = packet_new(s->pri_rs.buf, s->pri_rs.packet_len);
>> +        pkt = packet_new(s->pri_rs.buf,
>> +                         s->pri_rs.packet_len,
>> +                         s->pri_rs.vnet_hdr_len);
>>       } else {
>> -        pkt = packet_new(s->sec_rs.buf, s->sec_rs.packet_len);
>> +        pkt = packet_new(s->sec_rs.buf,
>> +                         s->sec_rs.packet_len,
>> +                         s->sec_rs.vnet_hdr_len);
>>       }
>>         if (parse_packet_early(pkt)) {
>> diff --git a/net/colo.c b/net/colo.c
>> index 8cc166b..180eaed 100644
>> --- a/net/colo.c
>> +++ b/net/colo.c
>> @@ -153,13 +153,14 @@ void connection_destroy(void *opaque)
>>       g_slice_free(Connection, conn);
>>   }
>>   -Packet *packet_new(const void *data, int size)
>> +Packet *packet_new(const void *data, int size, int vnet_hdr_len)
>>   {
>>       Packet *pkt = g_slice_new(Packet);
>>         pkt->data = g_memdup(data, size);
>>       pkt->size = size;
>>       pkt->creation_ms = qemu_clock_get_ms(QEMU_CLOCK_HOST);
>> +    pkt->vnet_hdr_len = vnet_hdr_len;
>>         return pkt;
>>   }
>> diff --git a/net/colo.h b/net/colo.h
>> index 7c524f3..caedb0d 100644
>> --- a/net/colo.h
>> +++ b/net/colo.h
>> @@ -43,6 +43,8 @@ typedef struct Packet {
>>       int size;
>>       /* Time of packet creation, in wall clock ms */
>>       int64_t creation_ms;
>> +    /* Get vnet_hdr_len from filter */
>> +    uint32_t vnet_hdr_len;
>>   } Packet;
>>     typedef struct ConnectionKey {
>> @@ -82,7 +84,7 @@ Connection *connection_get(GHashTable 
>> *connection_track_table,
>>                              ConnectionKey *key,
>>                              GQueue *conn_list);
>>   void connection_hashtable_reset(GHashTable *connection_track_table);
>> -Packet *packet_new(const void *data, int size);
>> +Packet *packet_new(const void *data, int size, int vnet_hdr_len);
>>   void packet_destroy(void *opaque, void *user_data);
>>     #endif /* QEMU_COLO_PROXY_H */
>> diff --git a/net/filter-rewriter.c b/net/filter-rewriter.c
>> index afa06e8..63256c7 100644
>> --- a/net/filter-rewriter.c
>> +++ b/net/filter-rewriter.c
>> @@ -158,7 +158,7 @@ static ssize_t 
>> colo_rewriter_receive_iov(NetFilterState *nf,
>>       char *buf = g_malloc0(size);
>>         iov_to_buf(iov, iovcnt, 0, buf, size);
>> -    pkt = packet_new(buf, size);
>> +    pkt = packet_new(buf, size, 0);
>>       g_free(buf);
>>         /*
>
>
>
> .
>

-- 
Thanks
Zhang Chen

Re: [Qemu-devel] [PATCH V4 07/12] net/colo.c: Make vnet_hdr_len as packet property

[Jason Wang]

On 2017年05月15日 16:03, Zhang Chen wrote:
>
>
> On 05/15/2017 12:05 PM, Jason Wang wrote:
>>
>>
>> On 2017年05月12日 09:41, Zhang Chen wrote:
>>> We can use this property flush and send packet with vnet_hdr_len.
>>>
>>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>>
>> Then I think it's not necessary to store vnet_hdr_len in 
>> SocketReadState?
>
> Do you means we keep the patch 05/12 in original?
>
> Thanks
> Zhang Chen
>

I mean we could fetch vnet_hdr_len from the buf directly. Or is there 
any advantage to store it in SocketReadState?

Thanks

Re: [Qemu-devel] [PATCH V4 07/12] net/colo.c: Make vnet_hdr_len as packet property

[Zhang Chen]

On 05/15/2017 04:18 PM, Jason Wang wrote:
>
>
> On 2017年05月15日 16:03, Zhang Chen wrote:
>>
>>
>> On 05/15/2017 12:05 PM, Jason Wang wrote:
>>>
>>>
>>> On 2017年05月12日 09:41, Zhang Chen wrote:
>>>> We can use this property flush and send packet with vnet_hdr_len.
>>>>
>>>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>>>
>>> Then I think it's not necessary to store vnet_hdr_len in 
>>> SocketReadState?
>>
>> Do you means we keep the patch 05/12 in original?
>>
>> Thanks
>> Zhang Chen
>>
>
> I mean we could fetch vnet_hdr_len from the buf directly. Or is there 
> any advantage to store it in SocketReadState?

No, The rs->buf did't have the vnet_hdr_len field.
In the net_fill_rstate(), when case = 2, we override the rs->buf by real 
net packet data,
So, the vnet_hdr_len field and packet_len field didn't be include in the 
last rs->buf.

Thanks
Zhang Chen

>
> Thanks
>
>
>
>

-- 
Thanks
Zhang Chen

[Qemu-devel] [PATCH V4 08/12] net/colo-compare.c: Make colo-compare support vnet_hdr_len

[Zhang Chen]

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 | 45 ++++++++++++++++++++++++++++++++++-----------
 1 file changed, 34 insertions(+), 11 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 87a9529..cb0b04e 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -98,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)
 {
@@ -473,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);
             if (ret < 0) {
                 error_report("colo_send_primary_packet failed");
             }
@@ -494,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);
@@ -505,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.
+         */
+        len = htonl(vnet_hdr_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(&s->chr_out, (uint8_t *)buf, size);
     if (ret != size) {
         goto err;
     }
@@ -535,7 +552,7 @@ static void compare_pri_chr_in(void *opaque, const uint8_t *buf, int size)
     CompareState *s = COLO_COMPARE(opaque);
     int ret;
 
-    ret = net_fill_rstate(&s->pri_rs, buf, size, false);
+    ret = net_fill_rstate(&s->pri_rs, buf, size, s->vnet_hdr);
     if (ret == -1) {
         qemu_chr_fe_set_handlers(&s->chr_pri_in, NULL, NULL, NULL,
                                  NULL, NULL, true);
@@ -552,7 +569,7 @@ static void compare_sec_chr_in(void *opaque, const uint8_t *buf, int size)
     CompareState *s = COLO_COMPARE(opaque);
     int ret;
 
-    ret = net_fill_rstate(&s->sec_rs, buf, size, false);
+    ret = net_fill_rstate(&s->sec_rs, buf, size, s->vnet_hdr);
     if (ret == -1) {
         qemu_chr_fe_set_handlers(&s->chr_sec_in, NULL, NULL, NULL,
                                  NULL, NULL, true);
@@ -675,7 +692,10 @@ static void compare_pri_rs_finalize(SocketReadState *pri_rs)
 
     if (packet_enqueue(s, PRIMARY_IN)) {
         trace_colo_compare_main("primary: unsupported packet in");
-        compare_chr_send(&s->chr_out, pri_rs->buf, pri_rs->packet_len);
+        compare_chr_send(s,
+                         pri_rs->buf,
+                         pri_rs->packet_len,
+                         pri_rs->vnet_hdr_len);
     } else {
         /* compare connection */
         g_queue_foreach(&s->conn_list, colo_compare_connection, s);
@@ -783,7 +803,10 @@ static void colo_flush_packets(void *opaque, void *user_data)
 
     while (!g_queue_is_empty(&conn->primary_list)) {
         pkt = g_queue_pop_head(&conn->primary_list);
-        compare_chr_send(&s->chr_out, pkt->data, pkt->size);
+        compare_chr_send(s,
+                         pkt->data,
+                         pkt->size,
+                         pkt->vnet_hdr_len);
         packet_destroy(pkt, NULL);
     }
     while (!g_queue_is_empty(&conn->secondary_list)) {
-- 
2.7.4

[Qemu-devel] [PATCH V4 09/12] net/colo.c: Add vnet packet parse feature in colo-proxy

[Zhang Chen]

Make colo-compare and filter-rewriter can parse vnet packet.

Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
---
 net/colo.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/colo.c b/net/colo.c
index 180eaed..28ce7c8 100644
--- a/net/colo.c
+++ b/net/colo.c
@@ -43,11 +43,11 @@ int parse_packet_early(Packet *pkt)
 {
     int network_length;
     static const uint8_t vlan[] = {0x81, 0x00};
-    uint8_t *data = pkt->data;
+    uint8_t *data = pkt->data + pkt->vnet_hdr_len;
     uint16_t l3_proto;
     ssize_t l2hdr_len = eth_get_l2_hdr_length(data);
 
-    if (pkt->size < ETH_HLEN) {
+    if (pkt->size < ETH_HLEN + pkt->vnet_hdr_len) {
         trace_colo_proxy_main("pkt->size < ETH_HLEN");
         return 1;
     }
@@ -73,7 +73,7 @@ int parse_packet_early(Packet *pkt)
     }
 
     network_length = pkt->ip->ip_hl * 4;
-    if (pkt->size < l2hdr_len + network_length) {
+    if (pkt->size < l2hdr_len + network_length + pkt->vnet_hdr_len) {
         trace_colo_proxy_main("pkt->size < network_header + network_length");
         return 1;
     }
-- 
2.7.4

[Qemu-devel] [PATCH V4 10/12] net/colo-compare.c: Add vnet packet's tcp/udp/icmp compare

[Zhang Chen]

COLO-Proxy just focus on packet payload, So we skip vnet header.

Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
---
 net/colo-compare.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index cb0b04e..bf565f3 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -188,6 +188,8 @@ static int packet_enqueue(CompareState *s, int mode)
  */
 static int colo_packet_compare_common(Packet *ppkt, Packet *spkt, int offset)
 {
+    int offset_all;
+
     if (trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
         char pri_ip_src[20], pri_ip_dst[20], sec_ip_src[20], sec_ip_dst[20];
 
@@ -201,9 +203,12 @@ static int colo_packet_compare_common(Packet *ppkt, Packet *spkt, int offset)
                                    sec_ip_src, sec_ip_dst);
     }
 
+    offset_all = ppkt->vnet_hdr_len + offset;
+
     if (ppkt->size == spkt->size) {
-        return memcmp(ppkt->data + offset, spkt->data + offset,
-                      spkt->size - offset);
+        return memcmp(ppkt->data + offset_all,
+                      spkt->data + offset_all,
+                      spkt->size - offset_all);
     } else {
         trace_colo_compare_main("Net packet size are not the same");
         return -1;
@@ -261,8 +266,9 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
      */
     if (ptcp->th_off > 5) {
         ptrdiff_t tcp_offset;
+
         tcp_offset = ppkt->transport_header - (uint8_t *)ppkt->data
-                     + (ptcp->th_off * 4);
+                     + (ptcp->th_off * 4) - ppkt->vnet_hdr_len;
         res = colo_packet_compare_common(ppkt, spkt, tcp_offset);
     } else if (ptcp->th_sum == stcp->th_sum) {
         res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN);
-- 
2.7.4

Re: [Qemu-devel] [PATCH V4 10/12] net/colo-compare.c: Add vnet packet's tcp/udp/icmp compare

[Jason Wang]

On 2017年05月12日 09:41, Zhang Chen wrote:
> COLO-Proxy just focus on packet payload, So we skip vnet header.
>
> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
> ---
>   net/colo-compare.c | 12 +++++++++---
>   1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/net/colo-compare.c b/net/colo-compare.c
> index cb0b04e..bf565f3 100644
> --- a/net/colo-compare.c
> +++ b/net/colo-compare.c
> @@ -188,6 +188,8 @@ static int packet_enqueue(CompareState *s, int mode)
>    */
>   static int colo_packet_compare_common(Packet *ppkt, Packet *spkt, int offset)
>   {
> +    int offset_all;
> +
>       if (trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
>           char pri_ip_src[20], pri_ip_dst[20], sec_ip_src[20], sec_ip_dst[20];
>   
> @@ -201,9 +203,12 @@ static int colo_packet_compare_common(Packet *ppkt, Packet *spkt, int offset)
>                                      sec_ip_src, sec_ip_dst);
>       }
>   
> +    offset_all = ppkt->vnet_hdr_len + offset;

How about just keep using offset by increasing it with vnet_hdr_len?

Thanks

> +
>       if (ppkt->size == spkt->size) {
> -        return memcmp(ppkt->data + offset, spkt->data + offset,
> -                      spkt->size - offset);
> +        return memcmp(ppkt->data + offset_all,
> +                      spkt->data + offset_all,
> +                      spkt->size - offset_all);
>       } else {
>           trace_colo_compare_main("Net packet size are not the same");
>           return -1;
> @@ -261,8 +266,9 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
>        */
>       if (ptcp->th_off > 5) {
>           ptrdiff_t tcp_offset;
> +
>           tcp_offset = ppkt->transport_header - (uint8_t *)ppkt->data
> -                     + (ptcp->th_off * 4);
> +                     + (ptcp->th_off * 4) - ppkt->vnet_hdr_len;
>           res = colo_packet_compare_common(ppkt, spkt, tcp_offset);
>       } else if (ptcp->th_sum == stcp->th_sum) {
>           res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN);

Re: [Qemu-devel] [PATCH V4 10/12] net/colo-compare.c: Add vnet packet's tcp/udp/icmp compare

[Zhang Chen]

On 05/15/2017 12:11 PM, Jason Wang wrote:
>
>
> On 2017年05月12日 09:41, Zhang Chen wrote:
>> COLO-Proxy just focus on packet payload, So we skip vnet header.
>>
>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>> ---
>>   net/colo-compare.c | 12 +++++++++---
>>   1 file changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/net/colo-compare.c b/net/colo-compare.c
>> index cb0b04e..bf565f3 100644
>> --- a/net/colo-compare.c
>> +++ b/net/colo-compare.c
>> @@ -188,6 +188,8 @@ static int packet_enqueue(CompareState *s, int mode)
>>    */
>>   static int colo_packet_compare_common(Packet *ppkt, Packet *spkt, 
>> int offset)
>>   {
>> +    int offset_all;
>> +
>>       if (trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
>>           char pri_ip_src[20], pri_ip_dst[20], sec_ip_src[20], 
>> sec_ip_dst[20];
>>   @@ -201,9 +203,12 @@ static int colo_packet_compare_common(Packet 
>> *ppkt, Packet *spkt, int offset)
>>                                      sec_ip_src, sec_ip_dst);
>>       }
>>   +    offset_all = ppkt->vnet_hdr_len + offset;
>
> How about just keep using offset by increasing it with vnet_hdr_len?

OK, I will fix it in next version.

Thanks
Zhang Chen

>
> Thanks
>
>> +
>>       if (ppkt->size == spkt->size) {
>> -        return memcmp(ppkt->data + offset, spkt->data + offset,
>> -                      spkt->size - offset);
>> +        return memcmp(ppkt->data + offset_all,
>> +                      spkt->data + offset_all,
>> +                      spkt->size - offset_all);
>>       } else {
>>           trace_colo_compare_main("Net packet size are not the same");
>>           return -1;
>> @@ -261,8 +266,9 @@ static int colo_packet_compare_tcp(Packet *spkt, 
>> Packet *ppkt)
>>        */
>>       if (ptcp->th_off > 5) {
>>           ptrdiff_t tcp_offset;
>> +
>>           tcp_offset = ppkt->transport_header - (uint8_t *)ppkt->data
>> -                     + (ptcp->th_off * 4);
>> +                     + (ptcp->th_off * 4) - ppkt->vnet_hdr_len;
>>           res = colo_packet_compare_common(ppkt, spkt, tcp_offset);
>>       } else if (ptcp->th_sum == stcp->th_sum) {
>>           res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN);
>
>
>
> .
>

-- 
Thanks
Zhang Chen

[Qemu-devel] [PATCH V4 11/12] net/filter-rewriter.c: Add new option to enable vnet support for filter-rewriter

[Zhang Chen]

We add the vnet_hdr option for filter-rewriter, default is disable.
If you use virtio-net-pci net driver, please enable it.
You can use it for example:
-object filter-rewriter,id=rew0,netdev=hn0,queue=all,vnet_hdr=on

Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
---
 net/filter-rewriter.c | 38 ++++++++++++++++++++++++++++++++++++++
 qemu-options.hx       |  4 ++--
 2 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/net/filter-rewriter.c b/net/filter-rewriter.c
index 63256c7..bc6d12a 100644
--- a/net/filter-rewriter.c
+++ b/net/filter-rewriter.c
@@ -33,6 +33,7 @@ typedef struct RewriterState {
     NetQueue *incoming_queue;
     /* hashtable to save connection */
     GHashTable *connection_track_table;
+    bool vnet_hdr;
 } RewriterState;
 
 static void filter_rewriter_flush(NetFilterState *nf)
@@ -237,6 +238,42 @@ static void colo_rewriter_setup(NetFilterState *nf, Error **errp)
     s->incoming_queue = qemu_new_net_queue(qemu_netfilter_pass_to_next, nf);
 }
 
+static char *filter_rewriter_get_vnet_hdr(Object *obj, Error **errp)
+{
+    RewriterState *s = FILTER_COLO_REWRITER(obj);
+
+    return s->vnet_hdr ? g_strdup("on") : g_strdup("off");
+}
+
+static void filter_rewriter_set_vnet_hdr(Object *obj,
+                                         const char *value,
+                                         Error **errp)
+{
+    RewriterState *s = FILTER_COLO_REWRITER(obj);
+
+    if (strcmp(value, "on") && strcmp(value, "off")) {
+        error_setg(errp, "Invalid value for filter-rewriter vnet_hdr, "
+                         "should be 'on' or 'off'");
+        return;
+    }
+
+    s->vnet_hdr = !strcmp(value, "on");
+}
+
+static void filter_rewriter_init(Object *obj)
+{
+    RewriterState *s = FILTER_COLO_REWRITER(obj);
+
+    /*
+     * The vnet_hdr is disabled by default, if you want to enable
+     * this option, you must enable all the option on related modules
+     * (like other filter or colo-compare).
+     */
+    s->vnet_hdr = false;
+    object_property_add_str(obj, "vnet_hdr", filter_rewriter_get_vnet_hdr,
+                            filter_rewriter_set_vnet_hdr, NULL);
+}
+
 static void colo_rewriter_class_init(ObjectClass *oc, void *data)
 {
     NetFilterClass *nfc = NETFILTER_CLASS(oc);
@@ -250,6 +287,7 @@ static const TypeInfo colo_rewriter_info = {
     .name = TYPE_FILTER_REWRITER,
     .parent = TYPE_NETFILTER,
     .class_init = colo_rewriter_class_init,
+    .instance_init = filter_rewriter_init,
     .instance_size = sizeof(RewriterState),
 };
 
diff --git a/qemu-options.hx b/qemu-options.hx
index 115b83f..d191050 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4040,12 +4040,12 @@ Create a filter-redirector we need to differ outdev id from indev id, id can not
 be the same. we can just use indev or outdev, but at least one of indev or outdev
 need to be specified.
 
-@item -object filter-rewriter,id=@var{id},netdev=@var{netdevid},rewriter-mode=@var{mode}[,queue=@var{all|rx|tx}]
+@item -object filter-rewriter,id=@var{id},netdev=@var{netdevid},rewriter-mode=@var{mode},vnet_hdr=@var{on|off}[,queue=@var{all|rx|tx}]
 
 Filter-rewriter is a part of COLO project.It will rewrite tcp packet to
 secondary from primary to keep secondary tcp connection,and rewrite
 tcp packet to primary from secondary make tcp packet can be handled by
-client.
+client.if vnet_hdr = on, we can parse packet with vnet header.
 
 usage:
 colo secondary:
-- 
2.7.4

Re: [Qemu-devel] [PATCH V4 11/12] net/filter-rewriter.c: Add new option to enable vnet support for filter-rewriter

[Jason Wang]

On 2017年05月12日 09:41, Zhang Chen wrote:
> We add the vnet_hdr option for filter-rewriter, default is disable.
> If you use virtio-net-pci net driver, please enable it.
> You can use it for example:
> -object filter-rewriter,id=rew0,netdev=hn0,queue=all,vnet_hdr=on
>
> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>

As has been pointed out, let's squash this into patch 12.

Thanks

Re: [Qemu-devel] [PATCH V4 11/12] net/filter-rewriter.c: Add new option to enable vnet support for filter-rewriter

[Zhang Chen]

On 05/15/2017 12:12 PM, Jason Wang wrote:
>
>
> On 2017年05月12日 09:41, Zhang Chen wrote:
>> We add the vnet_hdr option for filter-rewriter, default is disable.
>> If you use virtio-net-pci net driver, please enable it.
>> You can use it for example:
>> -object filter-rewriter,id=rew0,netdev=hn0,queue=all,vnet_hdr=on
>>
>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>
> As has been pointed out, let's squash this into patch 12.

OK~

Thanks
Zhang Chen

>
> Thanks
>
>
>
>

-- 
Thanks
Zhang Chen

[Qemu-devel] [PATCH V4 12/12] net/filter-rewriter.c: Make filter-rewriter support vnet_hdr_len

[Zhang Chen]

We get the vnet_hdr_len from NetClientState that make us
parse net packet correctly.

Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
---
 net/filter-rewriter.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/net/filter-rewriter.c b/net/filter-rewriter.c
index bc6d12a..be129c7 100644
--- a/net/filter-rewriter.c
+++ b/net/filter-rewriter.c
@@ -17,6 +17,7 @@
 #include "qemu-common.h"
 #include "qapi/error.h"
 #include "qapi/qmp/qerror.h"
+#include "qemu/error-report.h"
 #include "qapi-visit.h"
 #include "qom/object.h"
 #include "qemu/main-loop.h"
@@ -156,10 +157,24 @@ static ssize_t colo_rewriter_receive_iov(NetFilterState *nf,
     ConnectionKey key;
     Packet *pkt;
     ssize_t size = iov_size(iov, iovcnt);
+    ssize_t vnet_hdr_len = 0;
     char *buf = g_malloc0(size);
 
     iov_to_buf(iov, iovcnt, 0, buf, size);
-    pkt = packet_new(buf, size, 0);
+
+    if (s->vnet_hdr) {
+        if (nf->netdev->using_vnet_hdr) {
+            vnet_hdr_len = nf->netdev->vnet_hdr_len;
+        } else if (nf->netdev->peer->using_vnet_hdr) {
+            vnet_hdr_len = nf->netdev->peer->vnet_hdr_len;
+        } else {
+            error_report("filter-rewriter get vnet_hdr_len failed");
+            /* When error occurred we drop the packet  */
+            return 1;
+        }
+    }
+
+    pkt = packet_new(buf, size, vnet_hdr_len);
     g_free(buf);
 
     /*
-- 
2.7.4