Re: [Qemu-devel] [PATCH V2 1/2] COLO-compare: Optimize tcp compare for option field

[Jason Wang]

On 2017年04月21日 16:22, Zhang Chen wrote:



On 04/21/2017 12:10 PM, Jason Wang wrote:



On 2017年04月21日 11:48, Zhang Chen wrote:



On 04/20/2017 02:43 PM, Jason Wang wrote:



On 2017年04月18日 10:20, Zhang Chen wrote:

In this patch we support packet that have tcp options field.
Add tcp options field check, If the packet have options
field we just skip it and compare tcp payload,
Avoid unnecessary checkpoint, optimize performance.

Signed-off-by: Zhang Chen 
---
  net/colo-compare.c | 27 ++-
  1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index aada04e..049f6f8 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -248,7 +248,32 @@ static int colo_packet_compare_tcp(Packet 
*spkt, Packet *ppkt)

  spkt->ip->ip_sum = ppkt->ip->ip_sum;
  }
  -if (ptcp->th_sum == stcp->th_sum) {
+/*
+ * Check tcp header length for tcp option field.
+ * th_off > 5 means this tcp packet have options field.
+ * The tcp options maybe always different.
+ * for example:
+ * From RFC 7323.
+ * TCP Timestamps option (TSopt):
+ * Kind: 8
+ *
+ * Length: 10 bytes
+ *
+ * +---+---+-+-+
+ *|Kind=8 |  10   |   TS Value (TSval)  |TS Echo Reply 
(TSecr)|

+ * +---+---+-+-+
+ *   1   1  4 4
+ *
+ * In this case the primary guest's timestamp always 
different with

+ * the secondary guest's timestamp. COLO just focus on payload,
+ * so we just need skip this field.h


Probably a good explanation why we can skip this kind of header. 
But it does not explain why we can skip all the rest?


I found tcp options have many kind number to express different meaning,
Here I just give an example for the different options situation,
and this field not the COLO-proxy focus on, COLO just concern the 
payload.
Maybe we will optimize in the feature. Currently we want to make 
COLO full-function

running in qemu upstream.


I see, but the questions are:

- If we see packets with different options (except for the case you 
explain above), does it mean primary and secondary run out of sync?
- What will happen if we compare and ask for synchronization if we 
find options are not exactly the same?





The tcp options begin from the first SYN packet, if client connect to 
guest, the primary guest and secondary guest
just give a responses to the client's options, so I think the 
different options part mostly from guest's running
statues(like the timestamp), and if kind number are not same, the 
payload more likely not same too. finally, after
next checkpoint, all things will be same. If guest connect to client, 
primary guest send a packet and secondary not,

It will trigger old packet scan to do checkpoint.

Thanks
Zhang Chen


Ok this sounds more or less a question of whether or not we want to be 
more relaxed.


Thanks





Thanks



Thanks
Zhang Chen



Thanks


+ */
+if (ptcp->th_off > 5) {
+ptrdiff_t tcp_offset;
+tcp_offset = ppkt->transport_header - (uint8_t *)ppkt->data
+ + (ptcp->th_off * 4);
+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);
  } else {
  res = -1;




.







.

Re: [Qemu-devel] [PATCH V2 1/2] COLO-compare: Optimize tcp compare for option field

[Zhang Chen]

On 04/21/2017 12:10 PM, Jason Wang wrote:



On 2017年04月21日 11:48, Zhang Chen wrote:



On 04/20/2017 02:43 PM, Jason Wang wrote:



On 2017年04月18日 10:20, Zhang Chen wrote:

In this patch we support packet that have tcp options field.
Add tcp options field check, If the packet have options
field we just skip it and compare tcp payload,
Avoid unnecessary checkpoint, optimize performance.

Signed-off-by: Zhang Chen 
---
  net/colo-compare.c | 27 ++-
  1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index aada04e..049f6f8 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -248,7 +248,32 @@ static int colo_packet_compare_tcp(Packet 
*spkt, Packet *ppkt)

  spkt->ip->ip_sum = ppkt->ip->ip_sum;
  }
  -if (ptcp->th_sum == stcp->th_sum) {
+/*
+ * Check tcp header length for tcp option field.
+ * th_off > 5 means this tcp packet have options field.
+ * The tcp options maybe always different.
+ * for example:
+ * From RFC 7323.
+ * TCP Timestamps option (TSopt):
+ * Kind: 8
+ *
+ * Length: 10 bytes
+ *
+ * +---+---+-+-+
+ *|Kind=8 |  10   |   TS Value (TSval)  |TS Echo Reply 
(TSecr)|

+ * +---+---+-+-+
+ *   1   1  4 4
+ *
+ * In this case the primary guest's timestamp always different 
with

+ * the secondary guest's timestamp. COLO just focus on payload,
+ * so we just need skip this field.h


Probably a good explanation why we can skip this kind of header. But 
it does not explain why we can skip all the rest?


I found tcp options have many kind number to express different meaning,
Here I just give an example for the different options situation,
and this field not the COLO-proxy focus on, COLO just concern the 
payload.
Maybe we will optimize in the feature. Currently we want to make COLO 
full-function

running in qemu upstream.


I see, but the questions are:

- If we see packets with different options (except for the case you 
explain above), does it mean primary and secondary run out of sync?
- What will happen if we compare and ask for synchronization if we 
find options are not exactly the same?





The tcp options begin from the first SYN packet, if client connect to 
guest, the primary guest and secondary guest
just give a responses to the client's options, so I think the different 
options part mostly from guest's running
statues(like the timestamp), and if kind number are not same, the 
payload more likely not same too. finally, after
next checkpoint, all things will be same. If guest connect to client, 
primary guest send a packet and secondary not,

It will trigger old packet scan to do checkpoint.

Thanks
Zhang Chen



Thanks



Thanks
Zhang Chen



Thanks


+ */
+if (ptcp->th_off > 5) {
+ptrdiff_t tcp_offset;
+tcp_offset = ppkt->transport_header - (uint8_t *)ppkt->data
+ + (ptcp->th_off * 4);
+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);
  } else {
  res = -1;




.







.



--
Thanks
Zhang Chen

Re: [Qemu-devel] [PATCH V2 1/2] COLO-compare: Optimize tcp compare for option field

[Jason Wang]

On 2017年04月21日 11:48, Zhang Chen wrote:



On 04/20/2017 02:43 PM, Jason Wang wrote:



On 2017年04月18日 10:20, Zhang Chen wrote:

In this patch we support packet that have tcp options field.
Add tcp options field check, If the packet have options
field we just skip it and compare tcp payload,
Avoid unnecessary checkpoint, optimize performance.

Signed-off-by: Zhang Chen 
---
  net/colo-compare.c | 27 ++-
  1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index aada04e..049f6f8 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -248,7 +248,32 @@ static int colo_packet_compare_tcp(Packet 
*spkt, Packet *ppkt)

  spkt->ip->ip_sum = ppkt->ip->ip_sum;
  }
  -if (ptcp->th_sum == stcp->th_sum) {
+/*
+ * Check tcp header length for tcp option field.
+ * th_off > 5 means this tcp packet have options field.
+ * The tcp options maybe always different.
+ * for example:
+ * From RFC 7323.
+ * TCP Timestamps option (TSopt):
+ * Kind: 8
+ *
+ * Length: 10 bytes
+ *
+ * +---+---+-+-+
+ *|Kind=8 |  10   |   TS Value (TSval)  |TS Echo Reply 
(TSecr)|

+ * +---+---+-+-+
+ *   1   1  4 4
+ *
+ * In this case the primary guest's timestamp always different 
with

+ * the secondary guest's timestamp. COLO just focus on payload,
+ * so we just need skip this field.h


Probably a good explanation why we can skip this kind of header. But 
it does not explain why we can skip all the rest?


I found tcp options have many kind number to express different meaning,
Here I just give an example for the different options situation,
and this field not the COLO-proxy focus on, COLO just concern the 
payload.
Maybe we will optimize in the feature. Currently we want to make COLO 
full-function

running in qemu upstream.


I see, but the questions are:

- If we see packets with different options (except for the case you 
explain above), does it mean primary and secondary run out of sync?
- What will happen if we compare and ask for synchronization if we find 
options are not exactly the same?


Thanks



Thanks
Zhang Chen



Thanks


+ */
+if (ptcp->th_off > 5) {
+ptrdiff_t tcp_offset;
+tcp_offset = ppkt->transport_header - (uint8_t *)ppkt->data
+ + (ptcp->th_off * 4);
+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);
  } else {
  res = -1;




.

Re: [Qemu-devel] [PATCH V2 1/2] COLO-compare: Optimize tcp compare for option field

[Zhang Chen]

On 04/20/2017 02:43 PM, Jason Wang wrote:



On 2017年04月18日 10:20, Zhang Chen wrote:

In this patch we support packet that have tcp options field.
Add tcp options field check, If the packet have options
field we just skip it and compare tcp payload,
Avoid unnecessary checkpoint, optimize performance.

Signed-off-by: Zhang Chen 
---
  net/colo-compare.c | 27 ++-
  1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index aada04e..049f6f8 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -248,7 +248,32 @@ static int colo_packet_compare_tcp(Packet *spkt, 
Packet *ppkt)

  spkt->ip->ip_sum = ppkt->ip->ip_sum;
  }
  -if (ptcp->th_sum == stcp->th_sum) {
+/*
+ * Check tcp header length for tcp option field.
+ * th_off > 5 means this tcp packet have options field.
+ * The tcp options maybe always different.
+ * for example:
+ * From RFC 7323.
+ * TCP Timestamps option (TSopt):
+ * Kind: 8
+ *
+ * Length: 10 bytes
+ *
+ * +---+---+-+-+
+ *|Kind=8 |  10   |   TS Value (TSval)  |TS Echo Reply (TSecr)|
+ * +---+---+-+-+
+ *   1   1  4 4
+ *
+ * In this case the primary guest's timestamp always different with
+ * the secondary guest's timestamp. COLO just focus on payload,
+ * so we just need skip this field.h


Probably a good explanation why we can skip this kind of header. But 
it does not explain why we can skip all the rest?


I found tcp options have many kind number to express different meaning,
Here I just give an example for the different options situation,
and this field not the COLO-proxy focus on, COLO just concern the payload.
Maybe we will optimize in the feature. Currently we want to make COLO 
full-function

running in qemu upstream.

Thanks
Zhang Chen



Thanks


+ */
+if (ptcp->th_off > 5) {
+ptrdiff_t tcp_offset;
+tcp_offset = ppkt->transport_header - (uint8_t *)ppkt->data
+ + (ptcp->th_off * 4);
+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);
  } else {
  res = -1;




.



--
Thanks
Zhang Chen

Re: [Qemu-devel] [PATCH V2 1/2] COLO-compare: Optimize tcp compare for option field

[Jason Wang]

On 2017年04月18日 10:20, Zhang Chen wrote:

In this patch we support packet that have tcp options field.
Add tcp options field check, If the packet have options
field we just skip it and compare tcp payload,
Avoid unnecessary checkpoint, optimize performance.

Signed-off-by: Zhang Chen 
---
  net/colo-compare.c | 27 ++-
  1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index aada04e..049f6f8 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -248,7 +248,32 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet 
*ppkt)
  spkt->ip->ip_sum = ppkt->ip->ip_sum;
  }
  
-if (ptcp->th_sum == stcp->th_sum) {

+/*
+ * Check tcp header length for tcp option field.
+ * th_off > 5 means this tcp packet have options field.
+ * The tcp options maybe always different.
+ * for example:
+ * From RFC 7323.
+ * TCP Timestamps option (TSopt):
+ * Kind: 8
+ *
+ * Length: 10 bytes
+ *
+ *+---+---+-+-+
+ *|Kind=8 |  10   |   TS Value (TSval)  |TS Echo Reply (TSecr)|
+ *+---+---+-+-+
+ *   1   1  4 4
+ *
+ * In this case the primary guest's timestamp always different with
+ * the secondary guest's timestamp. COLO just focus on payload,
+ * so we just need skip this field.h


Probably a good explanation why we can skip this kind of header. But it 
does not explain why we can skip all the rest?


Thanks


+ */
+if (ptcp->th_off > 5) {
+ptrdiff_t tcp_offset;
+tcp_offset = ppkt->transport_header - (uint8_t *)ppkt->data
+ + (ptcp->th_off * 4);
+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);
  } else {
  res = -1;

[Qemu-devel] [PATCH V2 1/2] COLO-compare: Optimize tcp compare for option field

[Zhang Chen]

In this patch we support packet that have tcp options field.
Add tcp options field check, If the packet have options
field we just skip it and compare tcp payload,
Avoid unnecessary checkpoint, optimize performance.

Signed-off-by: Zhang Chen 
---
 net/colo-compare.c | 27 ++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index aada04e..049f6f8 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -248,7 +248,32 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet 
*ppkt)
 spkt->ip->ip_sum = ppkt->ip->ip_sum;
 }
 
-if (ptcp->th_sum == stcp->th_sum) {
+/*
+ * Check tcp header length for tcp option field.
+ * th_off > 5 means this tcp packet have options field.
+ * The tcp options maybe always different.
+ * for example:
+ * From RFC 7323.
+ * TCP Timestamps option (TSopt):
+ * Kind: 8
+ *
+ * Length: 10 bytes
+ *
+ *+---+---+-+-+
+ *|Kind=8 |  10   |   TS Value (TSval)  |TS Echo Reply (TSecr)|
+ *+---+---+-+-+
+ *   1   1  4 4
+ *
+ * In this case the primary guest's timestamp always different with
+ * the secondary guest's timestamp. COLO just focus on payload,
+ * so we just need skip this field.
+ */
+if (ptcp->th_off > 5) {
+ptrdiff_t tcp_offset;
+tcp_offset = ppkt->transport_header - (uint8_t *)ppkt->data
+ + (ptcp->th_off * 4);
+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);
 } else {
 res = -1;
-- 
2.7.4