[PATCH 0/2] net/colo-compare.c: Expose "max_queue_size" to users and clean up

[PATCH 0/2] net/colo-compare.c: Expose "max_queue_size" to users and clean up

[Zhang Chen]

From: Zhang Chen <chen.zhang@intel.com>

This series make a way to config COLO "max_queue_size" parameters according to
user's scenarios and environments and do some clean up for descriptions.

Zhang Chen (2):
  net/colo-compare.c: Expose compare "max_queue_size" to users
  qemu-options.hx: Clean up and fix typo for colo-compare

 net/colo-compare.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
 qemu-options.hx    | 33 +++++++++++++++++----------------
 2 files changed, 59 insertions(+), 17 deletions(-)

-- 
2.17.1

[PATCH 1/2] net/colo-compare.c: Expose compare "max_queue_size" to users

[Zhang Chen]

From: Zhang Chen <chen.zhang@intel.com>

This patch allow users to set the "max_queue_size" according
to their environment.

Signed-off-by: Zhang Chen <chen.zhang@intel.com>
---
 net/colo-compare.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
 qemu-options.hx    |  5 +++--
 2 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 10c0239f9d..def60ff2ea 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -54,6 +54,7 @@ static NotifierList colo_compare_notifiers =
 static QemuMutex event_mtx;
 static QemuCond event_complete_cond;
 static int event_unhandled_count;
+static uint32_t max_queue_size;
 
 /*
  *  + CompareState ++
@@ -193,7 +194,7 @@ static void fill_pkt_tcp_info(void *data, uint32_t *max_ack)
  */
 static int colo_insert_packet(GQueue *queue, Packet *pkt, uint32_t *max_ack)
 {
-    if (g_queue_get_length(queue) <= MAX_QUEUE_SIZE) {
+    if (g_queue_get_length(queue) <= max_queue_size) {
         if (pkt->ip->ip_p == IPPROTO_TCP) {
             fill_pkt_tcp_info(pkt, max_ack);
             g_queue_insert_sorted(queue,
@@ -1051,6 +1052,37 @@ out:
     error_propagate(errp, local_err);
 }
 
+static void get_max_queue_size(Object *obj, Visitor *v,
+                               const char *name, void *opaque,
+                               Error **errp)
+{
+    uint32_t value = max_queue_size;
+
+    visit_type_uint32(v, name, &value, errp);
+}
+
+static void set_max_queue_size(Object *obj, Visitor *v,
+                               const char *name, void *opaque,
+                               Error **errp)
+{
+    Error *local_err = NULL;
+    uint32_t value;
+
+    visit_type_uint32(v, name, &value, &local_err);
+    if (local_err) {
+        goto out;
+    }
+    if (!value) {
+        error_setg(&local_err, "Property '%s.%s' requires a positive value",
+                   object_get_typename(obj), name);
+        goto out;
+    }
+    max_queue_size = value;
+
+out:
+    error_propagate(errp, local_err);
+}
+
 static void compare_pri_rs_finalize(SocketReadState *pri_rs)
 {
     CompareState *s = container_of(pri_rs, CompareState, pri_rs);
@@ -1167,6 +1199,11 @@ static void colo_compare_complete(UserCreatable *uc, Error **errp)
         s->expired_scan_cycle = REGULAR_PACKET_CHECK_MS;
     }
 
+    if (!max_queue_size) {
+        /* Set default queue size to 1024 */
+        max_queue_size = MAX_QUEUE_SIZE;
+    }
+
     if (find_and_check_chardev(&chr, s->pri_indev, errp) ||
         !qemu_chr_fe_init(&s->chr_pri_in, chr, errp)) {
         return;
@@ -1270,6 +1307,10 @@ static void colo_compare_init(Object *obj)
                         compare_get_expired_scan_cycle,
                         compare_set_expired_scan_cycle, NULL, NULL, NULL);
 
+    object_property_add(obj, "max_queue_size", "uint32",
+                        get_max_queue_size,
+                        set_max_queue_size, NULL, NULL, NULL);
+
     s->vnet_hdr = false;
     object_property_add_bool(obj, "vnet_hdr_support", compare_get_vnet_hdr,
                              compare_set_vnet_hdr, NULL);
diff --git a/qemu-options.hx b/qemu-options.hx
index 16debd03cb..107ff3e71b 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4615,7 +4615,7 @@ SRST
         stored. The file format is libpcap, so it can be analyzed with
         tools such as tcpdump or Wireshark.
 
-    ``-object colo-compare,id=id,primary_in=chardevid,secondary_in=chardevid,outdev=chardevid,iothread=id[,vnet_hdr_support][,notify_dev=id][,compare_timeout=@var{ms}][,expired_scan_cycle=@var{ms}``
+    ``-object colo-compare,id=id,primary_in=chardevid,secondary_in=chardevid,outdev=chardevid,iothread=id[,vnet_hdr_support][,notify_dev=id][,compare_timeout=@var{ms}][,expired_scan_cycle=@var{ms}][,max_queue_size=@var{size}]``
         Colo-compare gets packet from primary\_inchardevid and
         secondary\_inchardevid, than compare primary packet with
         secondary packet. If the packets are same, we will output
@@ -4627,7 +4627,8 @@ SRST
         vnet\_hdr\_len. Then compare\_timeout=@var{ms} determines the
         maximum delay colo-compare wait for the packet.
         The expired\_scan\_cycle=@var{ms} to set the period of scanning
-        expired primary node network packets.
+        expired primary node network packets. The max\_queue\_size=@var{size}
+        is to set the max compare queue size depend on user environment.
         If you want to use Xen COLO, will need the notify\_dev to
         notify Xen colo-frame to do checkpoint.
 
-- 
2.17.1

[PATCH 2/2] qemu-options.hx: Clean up and fix typo for colo-compare

[Zhang Chen]

From: Zhang Chen <chen.zhang@intel.com>

Fix some typo and optimized some descriptions.

Signed-off-by: Zhang Chen <chen.zhang@intel.com>
---
 qemu-options.hx | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 107ff3e71b..84368f15be 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4616,24 +4616,24 @@ SRST
         tools such as tcpdump or Wireshark.
 
     ``-object colo-compare,id=id,primary_in=chardevid,secondary_in=chardevid,outdev=chardevid,iothread=id[,vnet_hdr_support][,notify_dev=id][,compare_timeout=@var{ms}][,expired_scan_cycle=@var{ms}][,max_queue_size=@var{size}]``
-        Colo-compare gets packet from primary\_inchardevid and
-        secondary\_inchardevid, than compare primary packet with
-        secondary packet. If the packets are same, we will output
-        primary packet to outdevchardevid, else we will notify
-        colo-frame do checkpoint and send primary packet to
-        outdevchardevid. In order to improve efficiency, we need to put
-        the task of comparison in another thread. If it has the
-        vnet\_hdr\_support flag, colo compare will send/recv packet with
-        vnet\_hdr\_len. Then compare\_timeout=@var{ms} determines the
-        maximum delay colo-compare wait for the packet.
-        The expired\_scan\_cycle=@var{ms} to set the period of scanning
-        expired primary node network packets. The max\_queue\_size=@var{size}
-        is to set the max compare queue size depend on user environment.
-        If you want to use Xen COLO, will need the notify\_dev to
+        Colo-compare gets packet from primary\_in chardevid and
+        secondary\_in, then compare whether the payload of primary packet
+        and secondary packet are the same. If same, it will output
+        primary packet to out\_dev, else it will notify COLO-framework to do
+        checkpoint and send primary packet to out\_dev. In order to
+        improve efficiency, we need to put the task of comparison in
+        another iothread. If it has the vnet\_hdr\_support flag,
+        colo compare will send/recv packet with vnet\_hdr\_len.
+        The compare\_timeout=@var{ms} determines the maximum time of the
+        colo-compare hold the packet. The expired\_scan\_cycle=@var{ms}
+        is to set the period of scanning expired primary node network packets.
+        The max\_queue\_size=@var{size} is to set the max compare queue
+        size depend on user environment.
+        If user want to use Xen COLO, need to add the notify\_dev to
         notify Xen colo-frame to do checkpoint.
 
-        we must use it with the help of filter-mirror and
-        filter-redirector.
+        COLO-compare must be used with the help of filter-mirror,
+        filter-redirector and filter-rewriter.
 
         ::
 
-- 
2.17.1

RE: [PATCH 0/2] net/colo-compare.c: Expose "max_queue_size" to users and clean up

[Zhang, Chen]

Hi Jason,

Please review this series when you free.

Thanks
Zhang Chen

> -----Original Message-----
> From: Zhang, Chen <chen.zhang@intel.com>
> Sent: Saturday, April 11, 2020 11:38 AM
> To: Jason Wang <jasowang@redhat.com>; qemu-dev <qemu-
> devel@nongnu.org>
> Cc: Zhang Chen <zhangckid@gmail.com>; Zhang, Chen
> <chen.zhang@intel.com>
> Subject: [PATCH 0/2] net/colo-compare.c: Expose "max_queue_size" to
> users and clean up
> 
> From: Zhang Chen <chen.zhang@intel.com>
> 
> This series make a way to config COLO "max_queue_size" parameters
> according to user's scenarios and environments and do some clean up for
> descriptions.
> 
> Zhang Chen (2):
>   net/colo-compare.c: Expose compare "max_queue_size" to users
>   qemu-options.hx: Clean up and fix typo for colo-compare
> 
>  net/colo-compare.c | 43
> ++++++++++++++++++++++++++++++++++++++++++-
>  qemu-options.hx    | 33 +++++++++++++++++----------------
>  2 files changed, 59 insertions(+), 17 deletions(-)
> 
> --
> 2.17.1

Re: [PATCH 0/2] net/colo-compare.c: Expose "max_queue_size" to users and clean up

[Jason Wang]

On 2020/4/23 下午3:31, Zhang, Chen wrote:
> Hi Jason,
>
> Please review this series when you free.
>
> Thanks
> Zhang Chen
>

Sure.

I wonder maybe it's better e.g you can review and collect the patches 
that looks good and send them to me periodically?

Thanks

RE: [PATCH 0/2] net/colo-compare.c: Expose "max_queue_size" to users and clean up

[Zhang, Chen]

> -----Original Message-----
> From: Jason Wang <jasowang@redhat.com>
> Sent: Thursday, April 23, 2020 4:54 PM
> To: Zhang, Chen <chen.zhang@intel.com>; qemu-dev <qemu-
> devel@nongnu.org>
> Cc: Zhang Chen <zhangckid@gmail.com>
> Subject: Re: [PATCH 0/2] net/colo-compare.c: Expose "max_queue_size" to
> users and clean up
> 
> 
> On 2020/4/23 下午3:31, Zhang, Chen wrote:
> > Hi Jason,
> >
> > Please review this series when you free.
> >
> > Thanks
> > Zhang Chen
> >
> 
> Sure.
> 
> I wonder maybe it's better e.g you can review and collect the patches that
> looks good and send them to me periodically?

OK, I will queue COLO related patch as one series to you.
Do I need send a pull request? or just a big patch set?

Thanks
Zhang Chen

> 
> Thanks

Re: [PATCH 0/2] net/colo-compare.c: Expose "max_queue_size" to users and clean up

[Jason Wang]

On 2020/4/23 下午4:59, Zhang, Chen wrote:
>
>> -----Original Message-----
>> From: Jason Wang <jasowang@redhat.com>
>> Sent: Thursday, April 23, 2020 4:54 PM
>> To: Zhang, Chen <chen.zhang@intel.com>; qemu-dev <qemu-
>> devel@nongnu.org>
>> Cc: Zhang Chen <zhangckid@gmail.com>
>> Subject: Re: [PATCH 0/2] net/colo-compare.c: Expose "max_queue_size" to
>> users and clean up
>>
>>
>> On 2020/4/23 下午3:31, Zhang, Chen wrote:
>>> Hi Jason,
>>>
>>> Please review this series when you free.
>>>
>>> Thanks
>>> Zhang Chen
>>>
>> Sure.
>>
>> I wonder maybe it's better e.g you can review and collect the patches that
>> looks good and send them to me periodically?
> OK, I will queue COLO related patch as one series to you.
> Do I need send a pull request? or just a big patch set?


I prefer big patch set.

Thanks


>
> Thanks
> Zhang Chen
>
>> Thanks

RE: [PATCH 0/2] net/colo-compare.c: Expose "max_queue_size" to users and clean up

[Zhang, Chen]

> -----Original Message-----
> From: Jason Wang <jasowang@redhat.com>
> Sent: Thursday, April 23, 2020 5:07 PM
> To: Zhang, Chen <chen.zhang@intel.com>; qemu-dev <qemu-
> devel@nongnu.org>
> Cc: Zhang Chen <zhangckid@gmail.com>
> Subject: Re: [PATCH 0/2] net/colo-compare.c: Expose "max_queue_size" to
> users and clean up
> 
> 
> On 2020/4/23 下午4:59, Zhang, Chen wrote:
> >
> >> -----Original Message-----
> >> From: Jason Wang <jasowang@redhat.com>
> >> Sent: Thursday, April 23, 2020 4:54 PM
> >> To: Zhang, Chen <chen.zhang@intel.com>; qemu-dev <qemu-
> >> devel@nongnu.org>
> >> Cc: Zhang Chen <zhangckid@gmail.com>
> >> Subject: Re: [PATCH 0/2] net/colo-compare.c: Expose "max_queue_size"
> >> to users and clean up
> >>
> >>
> >> On 2020/4/23 下午3:31, Zhang, Chen wrote:
> >>> Hi Jason,
> >>>
> >>> Please review this series when you free.
> >>>
> >>> Thanks
> >>> Zhang Chen
> >>>
> >> Sure.
> >>
> >> I wonder maybe it's better e.g you can review and collect the patches
> >> that looks good and send them to me periodically?
> > OK, I will queue COLO related patch as one series to you.
> > Do I need send a pull request? or just a big patch set?
> 
> 
> I prefer big patch set.

OK, I got it.

Thanks
Zhang Chen

> 
> Thanks
> 
> 
> >
> > Thanks
> > Zhang Chen
> >
> >> Thanks

Re: [PATCH 0/2] net/colo-compare.c: Expose "max_queue_size" to users and clean up

[Jason Wang]

On 2020/4/11 上午11:38, Zhang Chen wrote:
> From: Zhang Chen <chen.zhang@intel.com>
>
> This series make a way to config COLO "max_queue_size" parameters according to
> user's scenarios and environments and do some clean up for descriptions.
>
> Zhang Chen (2):
>    net/colo-compare.c: Expose compare "max_queue_size" to users
>    qemu-options.hx: Clean up and fix typo for colo-compare
>
>   net/colo-compare.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
>   qemu-options.hx    | 33 +++++++++++++++++----------------
>   2 files changed, 59 insertions(+), 17 deletions(-)


Queued for 5.1.

Thanks

Re: [PATCH 0/2] net/colo-compare.c: Expose "max_queue_size" to users and clean up

[Lukas Straub]

[-- Attachment #1: Type: text/plain, Size: 824 bytes --]

On Fri, 24 Apr 2020 10:19:48 +0800
Jason Wang <jasowang@redhat.com> wrote:

> On 2020/4/11 上午11:38, Zhang Chen wrote:
> > From: Zhang Chen <chen.zhang@intel.com>
> >
> > This series make a way to config COLO "max_queue_size" parameters according to
> > user's scenarios and environments and do some clean up for descriptions.
> >
> > Zhang Chen (2):
> >    net/colo-compare.c: Expose compare "max_queue_size" to users
> >    qemu-options.hx: Clean up and fix typo for colo-compare
> >
> >   net/colo-compare.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
> >   qemu-options.hx    | 33 +++++++++++++++++----------------
> >   2 files changed, 59 insertions(+), 17 deletions(-)  
> 
> 
> Queued for 5.1.
> 
> Thanks
> 
> 

Hi,
It looks like this hasn't been merged.

Regards,
Lukas Straub

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

RE: [PATCH 0/2] net/colo-compare.c: Expose "max_queue_size" to users and clean up

[Zhang, Chen]

> -----Original Message-----
> From: Lukas Straub <lukasstraub2@web.de>
> Sent: Saturday, June 20, 2020 1:47 AM
> To: Jason Wang <jasowang@redhat.com>
> Cc: Zhang, Chen <chen.zhang@intel.com>; qemu-dev <qemu-
> devel@nongnu.org>; Zhang Chen <zhangckid@gmail.com>
> Subject: Re: [PATCH 0/2] net/colo-compare.c: Expose "max_queue_size" to
> users and clean up
> 
> On Fri, 24 Apr 2020 10:19:48 +0800
> Jason Wang <jasowang@redhat.com> wrote:
> 
> > On 2020/4/11 上午11:38, Zhang Chen wrote:
> > > From: Zhang Chen <chen.zhang@intel.com>
> > >
> > > This series make a way to config COLO "max_queue_size" parameters
> > > according to user's scenarios and environments and do some clean up for
> descriptions.
> > >
> > > Zhang Chen (2):
> > >    net/colo-compare.c: Expose compare "max_queue_size" to users
> > >    qemu-options.hx: Clean up and fix typo for colo-compare
> > >
> > >   net/colo-compare.c | 43
> ++++++++++++++++++++++++++++++++++++++++++-
> > >   qemu-options.hx    | 33 +++++++++++++++++----------------
> > >   2 files changed, 59 insertions(+), 17 deletions(-)
> >
> >
> > Queued for 5.1.
> >
> > Thanks
> >
> >
> 
> Hi,
> It looks like this hasn't been merged.

Thanks Lukas.
Hi Jason, I have double checked on upstream, looks missed this series.

Thanks
Zhang Chen

> 
> Regards,
> Lukas Straub

Re: [PATCH 0/2] net/colo-compare.c: Expose "max_queue_size" to users and clean up

[Jason Wang]

On 2020/6/23 上午9:47, Zhang, Chen wrote:
>
>> -----Original Message-----
>> From: Lukas Straub <lukasstraub2@web.de>
>> Sent: Saturday, June 20, 2020 1:47 AM
>> To: Jason Wang <jasowang@redhat.com>
>> Cc: Zhang, Chen <chen.zhang@intel.com>; qemu-dev <qemu-
>> devel@nongnu.org>; Zhang Chen <zhangckid@gmail.com>
>> Subject: Re: [PATCH 0/2] net/colo-compare.c: Expose "max_queue_size" to
>> users and clean up
>>
>> On Fri, 24 Apr 2020 10:19:48 +0800
>> Jason Wang <jasowang@redhat.com> wrote:
>>
>>> On 2020/4/11 上午11:38, Zhang Chen wrote:
>>>> From: Zhang Chen <chen.zhang@intel.com>
>>>>
>>>> This series make a way to config COLO "max_queue_size" parameters
>>>> according to user's scenarios and environments and do some clean up for
>> descriptions.
>>>> Zhang Chen (2):
>>>>     net/colo-compare.c: Expose compare "max_queue_size" to users
>>>>     qemu-options.hx: Clean up and fix typo for colo-compare
>>>>
>>>>    net/colo-compare.c | 43
>> ++++++++++++++++++++++++++++++++++++++++++-
>>>>    qemu-options.hx    | 33 +++++++++++++++++----------------
>>>>    2 files changed, 59 insertions(+), 17 deletions(-)
>>>
>>> Queued for 5.1.
>>>
>>> Thanks
>>>
>>>
>> Hi,
>> It looks like this hasn't been merged.
> Thanks Lukas.
> Hi Jason, I have double checked on upstream, looks missed this series.
>
> Thanks
> Zhang Chen


Right, but unfortunately it can't be applied cleanly on master.

Please send a new version.

Sorry.

Thanks


>
>> Regards,
>> Lukas Straub

RE: [PATCH 0/2] net/colo-compare.c: Expose "max_queue_size" to users and clean up

[Zhang, Chen]

> -----Original Message-----
> From: Jason Wang <jasowang@redhat.com>
> Sent: Tuesday, June 23, 2020 1:54 PM
> To: Zhang, Chen <chen.zhang@intel.com>; Lukas Straub
> <lukasstraub2@web.de>
> Cc: qemu-dev <qemu-devel@nongnu.org>; Zhang Chen
> <zhangckid@gmail.com>
> Subject: Re: [PATCH 0/2] net/colo-compare.c: Expose "max_queue_size" to
> users and clean up
> 
> 
> On 2020/6/23 上午9:47, Zhang, Chen wrote:
> >
> >> -----Original Message-----
> >> From: Lukas Straub <lukasstraub2@web.de>
> >> Sent: Saturday, June 20, 2020 1:47 AM
> >> To: Jason Wang <jasowang@redhat.com>
> >> Cc: Zhang, Chen <chen.zhang@intel.com>; qemu-dev <qemu-
> >> devel@nongnu.org>; Zhang Chen <zhangckid@gmail.com>
> >> Subject: Re: [PATCH 0/2] net/colo-compare.c: Expose "max_queue_size"
> >> to users and clean up
> >>
> >> On Fri, 24 Apr 2020 10:19:48 +0800
> >> Jason Wang <jasowang@redhat.com> wrote:
> >>
> >>> On 2020/4/11 上午11:38, Zhang Chen wrote:
> >>>> From: Zhang Chen <chen.zhang@intel.com>
> >>>>
> >>>> This series make a way to config COLO "max_queue_size" parameters
> >>>> according to user's scenarios and environments and do some clean up
> >>>> for
> >> descriptions.
> >>>> Zhang Chen (2):
> >>>>     net/colo-compare.c: Expose compare "max_queue_size" to users
> >>>>     qemu-options.hx: Clean up and fix typo for colo-compare
> >>>>
> >>>>    net/colo-compare.c | 43
> >> ++++++++++++++++++++++++++++++++++++++++++-
> >>>>    qemu-options.hx    | 33 +++++++++++++++++----------------
> >>>>    2 files changed, 59 insertions(+), 17 deletions(-)
> >>>
> >>> Queued for 5.1.
> >>>
> >>> Thanks
> >>>
> >>>
> >> Hi,
> >> It looks like this hasn't been merged.
> > Thanks Lukas.
> > Hi Jason, I have double checked on upstream, looks missed this series.
> >
> > Thanks
> > Zhang Chen
> 
> 
> Right, but unfortunately it can't be applied cleanly on master.
> 
> Please send a new version.
> 
> Sorry.

It's OK, I have send V2 version of this series, please merge it.

Thanks
Zhang Chen

> 
> Thanks
> 
> 
> >
> >> Regards,
> >> Lukas Straub