* [Qemu-devel] [PATCH 1/3] COLO-compare: Add minimum packet size check and some fix
2017-02-25 3:32 [Qemu-devel] [PATCH 0/3] COLO-compare: Optimize the code and fix some bug Zhang Chen
@ 2017-02-25 3:32 ` Zhang Chen
2017-02-25 6:43 ` Hailiang Zhang
2017-02-25 3:32 ` [Qemu-devel] [PATCH 1/3] COLO-compare: Add " Zhang Chen
` (2 subsequent siblings)
3 siblings, 1 reply; 18+ messages in thread
From: Zhang Chen @ 2017-02-25 3:32 UTC (permalink / raw)
To: qemu devel; +Cc: Jason Wang, Zhang Chen, Li Zhijian, eddie . dong, bian naimeng
Add packet minimum size check in colo_packet_compare_udp()
and colo_packet_compare_udp() like colo_packet_compare_icmp(),
rename function colo_packet_compare() to colo_packet_compare_common()
that we will reuse it later.
Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
---
net/colo-compare.c | 30 ++++++++++++++++++++++--------
1 file changed, 22 insertions(+), 8 deletions(-)
diff --git a/net/colo-compare.c b/net/colo-compare.c
index 300f017..e75f0ae 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -180,7 +180,7 @@ static int packet_enqueue(CompareState *s, int mode)
* return: 0 means packet same
* > 0 || < 0 means packet different
*/
-static int colo_packet_compare(Packet *ppkt, Packet *spkt)
+static int colo_packet_compare_common(Packet *ppkt, Packet *spkt)
{
trace_colo_compare_ip_info(ppkt->size, inet_ntoa(ppkt->ip->ip_src),
inet_ntoa(ppkt->ip->ip_dst), spkt->size,
@@ -190,6 +190,7 @@ static int colo_packet_compare(Packet *ppkt, Packet *spkt)
if (ppkt->size == spkt->size) {
return memcmp(ppkt->data, spkt->data, spkt->size);
} else {
+ trace_colo_compare_main("Net packet size are not the same");
return -1;
}
}
@@ -202,9 +203,10 @@ static int colo_packet_compare(Packet *ppkt, Packet *spkt)
static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
{
struct tcphdr *ptcp, *stcp;
- int res;
+ int res, network_length;
trace_colo_compare_main("compare tcp");
+
if (ppkt->size != spkt->size) {
if (trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
trace_colo_compare_main("pkt size not same");
@@ -212,6 +214,12 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
return -1;
}
+ network_length = ppkt->ip->ip_hl * 4;
+ if (ppkt->size < network_length + ETH_HLEN) {
+ trace_colo_compare_main("tcp packet size error");
+ return -1;
+ }
+
ptcp = (struct tcphdr *)ppkt->transport_header;
stcp = (struct tcphdr *)spkt->transport_header;
@@ -260,10 +268,16 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
*/
static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt)
{
- int ret;
+ int ret, network_length;
trace_colo_compare_main("compare udp");
- ret = colo_packet_compare(ppkt, spkt);
+ network_length = ppkt->ip->ip_hl * 4;
+ if (ppkt->size < network_length + ETH_HLEN) {
+ trace_colo_compare_main("udp packet size error");
+ return -1;
+ }
+
+ ret = colo_packet_compare_common(ppkt, spkt);
if (ret) {
trace_colo_compare_udp_miscompare("primary pkt size", ppkt->size);
@@ -285,12 +299,12 @@ static int colo_packet_compare_icmp(Packet *spkt, Packet *ppkt)
trace_colo_compare_main("compare icmp");
network_length = ppkt->ip->ip_hl * 4;
- if (ppkt->size != spkt->size ||
- ppkt->size < network_length + ETH_HLEN) {
+ if (ppkt->size < network_length + ETH_HLEN) {
+ trace_colo_compare_main("icmp packet size error");
return -1;
}
- if (colo_packet_compare(ppkt, spkt)) {
+ if (colo_packet_compare_common(ppkt, spkt)) {
trace_colo_compare_icmp_miscompare("primary pkt size",
ppkt->size);
qemu_hexdump((char *)ppkt->data, stderr, "colo-compare",
@@ -316,7 +330,7 @@ static int colo_packet_compare_other(Packet *spkt, Packet *ppkt)
inet_ntoa(ppkt->ip->ip_dst), spkt->size,
inet_ntoa(spkt->ip->ip_src),
inet_ntoa(spkt->ip->ip_dst));
- return colo_packet_compare(ppkt, spkt);
+ return colo_packet_compare_common(ppkt, spkt);
}
static int colo_old_packet_check_one(Packet *pkt, int64_t *check_time)
--
2.7.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [PATCH 1/3] COLO-compare: Add minimum packet size check and some fix
2017-02-25 3:32 ` [Qemu-devel] [PATCH 1/3] COLO-compare: Add minimum packet size check and some fix Zhang Chen
@ 2017-02-25 6:43 ` Hailiang Zhang
2017-02-27 6:39 ` Zhang Chen
0 siblings, 1 reply; 18+ messages in thread
From: Hailiang Zhang @ 2017-02-25 6:43 UTC (permalink / raw)
To: qemu-devel; +Cc: xuquan8
Hi,
On 2017/2/25 11:32, Zhang Chen wrote:
> Add packet minimum size check in colo_packet_compare_udp()
> and colo_packet_compare_udp() like colo_packet_compare_icmp(),
> rename function colo_packet_compare() to colo_packet_compare_common()
> that we will reuse it later.
>
> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
> ---
> net/colo-compare.c | 30 ++++++++++++++++++++++--------
> 1 file changed, 22 insertions(+), 8 deletions(-)
>
> diff --git a/net/colo-compare.c b/net/colo-compare.c
> index 300f017..e75f0ae 100644
> --- a/net/colo-compare.c
> +++ b/net/colo-compare.c
> @@ -180,7 +180,7 @@ static int packet_enqueue(CompareState *s, int mode)
> * return: 0 means packet same
> * > 0 || < 0 means packet different
> */
> -static int colo_packet_compare(Packet *ppkt, Packet *spkt)
> +static int colo_packet_compare_common(Packet *ppkt, Packet *spkt)
> {
> trace_colo_compare_ip_info(ppkt->size, inet_ntoa(ppkt->ip->ip_src),
> inet_ntoa(ppkt->ip->ip_dst), spkt->size,
> @@ -190,6 +190,7 @@ static int colo_packet_compare(Packet *ppkt, Packet *spkt)
> if (ppkt->size == spkt->size) {
> return memcmp(ppkt->data, spkt->data, spkt->size);
> } else {
> + trace_colo_compare_main("Net packet size are not the same");
> return -1;
> }
> }
> @@ -202,9 +203,10 @@ static int colo_packet_compare(Packet *ppkt, Packet *spkt)
> static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
> {
> struct tcphdr *ptcp, *stcp;
> - int res;
> + int res, network_length;
>
> trace_colo_compare_main("compare tcp");
> +
> if (ppkt->size != spkt->size) {
> if (trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
> trace_colo_compare_main("pkt size not same");
> @@ -212,6 +214,12 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
> return -1;
> }
>
> + network_length = ppkt->ip->ip_hl * 4;
> + if (ppkt->size < network_length + ETH_HLEN) {
I think the check here is useless, because you have such check in
parse_packet_early() which is been called before these helpers.
And what check you need to add is, to check if the packet's size
>= packet's length been record in ip header.
Like:
+++ b/net/colo.c
@@ -78,6 +78,12 @@ int parse_packet_early(Packet *pkt)
trace_colo_proxy_main("pkt->size < network_header + network_length");
return 1;
}
+
+ if (pkt->size < ETH_HLEN + ntohs(pkt->ip->ip_len)) {
+ fprintf(stderr, "pkt->size %d < pkt expect total len %ld\n", pkt->size,
+ pkt_MAChdr_len + ntohs(pkt->ip->ip_len));
+ return -1;
+ }
> + trace_colo_compare_main("tcp packet size error");
> + return -1;
> + }
> +
> ptcp = (struct tcphdr *)ppkt->transport_header;
> stcp = (struct tcphdr *)spkt->transport_header;
>
> @@ -260,10 +268,16 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
> */
> static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt)
> {
> - int ret;
> + int ret, network_length;
>
> trace_colo_compare_main("compare udp");
> - ret = colo_packet_compare(ppkt, spkt);
> + network_length = ppkt->ip->ip_hl * 4;
> + if (ppkt->size < network_length + ETH_HLEN) {
> + trace_colo_compare_main("udp packet size error");
> + return -1;
> + }
> +
> + ret = colo_packet_compare_common(ppkt, spkt);
>
> if (ret) {
> trace_colo_compare_udp_miscompare("primary pkt size", ppkt->size);
> @@ -285,12 +299,12 @@ static int colo_packet_compare_icmp(Packet *spkt, Packet *ppkt)
>
> trace_colo_compare_main("compare icmp");
> network_length = ppkt->ip->ip_hl * 4;
> - if (ppkt->size != spkt->size ||
> - ppkt->size < network_length + ETH_HLEN) {
> + if (ppkt->size < network_length + ETH_HLEN) {
> + trace_colo_compare_main("icmp packet size error");
> return -1;
> }
>
> - if (colo_packet_compare(ppkt, spkt)) {
> + if (colo_packet_compare_common(ppkt, spkt)) {
> trace_colo_compare_icmp_miscompare("primary pkt size",
> ppkt->size);
> qemu_hexdump((char *)ppkt->data, stderr, "colo-compare",
> @@ -316,7 +330,7 @@ static int colo_packet_compare_other(Packet *spkt, Packet *ppkt)
> inet_ntoa(ppkt->ip->ip_dst), spkt->size,
> inet_ntoa(spkt->ip->ip_src),
> inet_ntoa(spkt->ip->ip_dst));
> - return colo_packet_compare(ppkt, spkt);
> + return colo_packet_compare_common(ppkt, spkt);
> }
>
> static int colo_old_packet_check_one(Packet *pkt, int64_t *check_time)
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [PATCH 1/3] COLO-compare: Add minimum packet size check and some fix
2017-02-25 6:43 ` Hailiang Zhang
@ 2017-02-27 6:39 ` Zhang Chen
0 siblings, 0 replies; 18+ messages in thread
From: Zhang Chen @ 2017-02-27 6:39 UTC (permalink / raw)
To: Hailiang Zhang, qemu-devel; +Cc: zhangchen.fnst, xuquan8
On 02/25/2017 02:43 PM, Hailiang Zhang wrote:
> Hi,
>
> On 2017/2/25 11:32, Zhang Chen wrote:
>> Add packet minimum size check in colo_packet_compare_udp()
>> and colo_packet_compare_udp() like colo_packet_compare_icmp(),
>> rename function colo_packet_compare() to colo_packet_compare_common()
>> that we will reuse it later.
>>
>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>> ---
>> net/colo-compare.c | 30 ++++++++++++++++++++++--------
>> 1 file changed, 22 insertions(+), 8 deletions(-)
>>
>> diff --git a/net/colo-compare.c b/net/colo-compare.c
>> index 300f017..e75f0ae 100644
>> --- a/net/colo-compare.c
>> +++ b/net/colo-compare.c
>> @@ -180,7 +180,7 @@ static int packet_enqueue(CompareState *s, int mode)
>> * return: 0 means packet same
>> * > 0 || < 0 means packet different
>> */
>> -static int colo_packet_compare(Packet *ppkt, Packet *spkt)
>> +static int colo_packet_compare_common(Packet *ppkt, Packet *spkt)
>> {
>> trace_colo_compare_ip_info(ppkt->size,
>> inet_ntoa(ppkt->ip->ip_src),
>> inet_ntoa(ppkt->ip->ip_dst), spkt->size,
>> @@ -190,6 +190,7 @@ static int colo_packet_compare(Packet *ppkt,
>> Packet *spkt)
>> if (ppkt->size == spkt->size) {
>> return memcmp(ppkt->data, spkt->data, spkt->size);
>> } else {
>> + trace_colo_compare_main("Net packet size are not the same");
>> return -1;
>> }
>> }
>> @@ -202,9 +203,10 @@ static int colo_packet_compare(Packet *ppkt,
>> Packet *spkt)
>> static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
>> {
>> struct tcphdr *ptcp, *stcp;
>> - int res;
>> + int res, network_length;
>>
>> trace_colo_compare_main("compare tcp");
>> +
>> if (ppkt->size != spkt->size) {
>> if (trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
>> trace_colo_compare_main("pkt size not same");
>> @@ -212,6 +214,12 @@ static int colo_packet_compare_tcp(Packet *spkt,
>> Packet *ppkt)
>> return -1;
>> }
>>
>> + network_length = ppkt->ip->ip_hl * 4;
>> + if (ppkt->size < network_length + ETH_HLEN) {
>
> I think the check here is useless, because you have such check in
> parse_packet_early() which is been called before these helpers.
> And what check you need to add is, to check if the packet's size
>> = packet's length been record in ip header.
>
> Like:
> +++ b/net/colo.c
> @@ -78,6 +78,12 @@ int parse_packet_early(Packet *pkt)
> trace_colo_proxy_main("pkt->size < network_header +
> network_length");
> return 1;
> }
> +
> + if (pkt->size < ETH_HLEN + ntohs(pkt->ip->ip_len)) {
> + fprintf(stderr, "pkt->size %d < pkt expect total len %ld\n",
> pkt->size,
> + pkt_MAChdr_len + ntohs(pkt->ip->ip_len));
> + return -1;
> + }
This check we also have done in parse_packet_early()
network_length = pkt->ip->ip_hl * 4;
if (pkt->size < l2hdr_len + network_length) {
trace_colo_proxy_main("pkt->size < network_header +
network_length");
return 1;
}
So, maybe I need remove my before change and the compare_icmp() check.
Thanks
Zhang Chen
>
>
>> + trace_colo_compare_main("tcp packet size error");
>> + return -1;
>> + }
>> +
>> ptcp = (struct tcphdr *)ppkt->transport_header;
>> stcp = (struct tcphdr *)spkt->transport_header;
>>
>> @@ -260,10 +268,16 @@ static int colo_packet_compare_tcp(Packet
>> *spkt, Packet *ppkt)
>> */
>> static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt)
>> {
>> - int ret;
>> + int ret, network_length;
>>
>> trace_colo_compare_main("compare udp");
>> - ret = colo_packet_compare(ppkt, spkt);
>> + network_length = ppkt->ip->ip_hl * 4;
>> + if (ppkt->size < network_length + ETH_HLEN) {
>> + trace_colo_compare_main("udp packet size error");
>> + return -1;
>> + }
>> +
>> + ret = colo_packet_compare_common(ppkt, spkt);
>>
>> if (ret) {
>> trace_colo_compare_udp_miscompare("primary pkt size",
>> ppkt->size);
>> @@ -285,12 +299,12 @@ static int colo_packet_compare_icmp(Packet
>> *spkt, Packet *ppkt)
>>
>> trace_colo_compare_main("compare icmp");
>> network_length = ppkt->ip->ip_hl * 4;
>> - if (ppkt->size != spkt->size ||
>> - ppkt->size < network_length + ETH_HLEN) {
>> + if (ppkt->size < network_length + ETH_HLEN) {
>> + trace_colo_compare_main("icmp packet size error");
>> return -1;
>> }
>>
>> - if (colo_packet_compare(ppkt, spkt)) {
>> + if (colo_packet_compare_common(ppkt, spkt)) {
>> trace_colo_compare_icmp_miscompare("primary pkt size",
>> ppkt->size);
>> qemu_hexdump((char *)ppkt->data, stderr, "colo-compare",
>> @@ -316,7 +330,7 @@ static int colo_packet_compare_other(Packet
>> *spkt, Packet *ppkt)
>> inet_ntoa(ppkt->ip->ip_dst), spkt->size,
>> inet_ntoa(spkt->ip->ip_src),
>> inet_ntoa(spkt->ip->ip_dst));
>> - return colo_packet_compare(ppkt, spkt);
>> + return colo_packet_compare_common(ppkt, spkt);
>> }
>>
>> static int colo_old_packet_check_one(Packet *pkt, int64_t *check_time)
>>
>
>
>
>
> .
>
--
Thanks
Zhang Chen
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH 1/3] COLO-compare: Add packet size check and some fix
2017-02-25 3:32 [Qemu-devel] [PATCH 0/3] COLO-compare: Optimize the code and fix some bug Zhang Chen
2017-02-25 3:32 ` [Qemu-devel] [PATCH 1/3] COLO-compare: Add minimum packet size check and some fix Zhang Chen
@ 2017-02-25 3:32 ` Zhang Chen
2017-02-25 3:43 ` Zhang Chen
2017-02-25 3:32 ` [Qemu-devel] [PATCH 2/3] COLO-compare: Optimize colo_packet_compare_common Zhang Chen
2017-02-25 3:32 ` [Qemu-devel] [PATCH 3/3] COLO-compare: Make qemu_hexdump() print data when trace-event ON Zhang Chen
3 siblings, 1 reply; 18+ messages in thread
From: Zhang Chen @ 2017-02-25 3:32 UTC (permalink / raw)
To: qemu devel; +Cc: Jason Wang, Zhang Chen, Li Zhijian, eddie . dong, bian naimeng
Add packet size check in colo_packet_compare_udp()
and colo_packet_compare_udp(), rename function colo_packet_compare()
to colo_packet_compare_common() that we will reuse it later.
Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
---
net/colo-compare.c | 30 ++++++++++++++++++++++--------
1 file changed, 22 insertions(+), 8 deletions(-)
diff --git a/net/colo-compare.c b/net/colo-compare.c
index 300f017..e75f0ae 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -180,7 +180,7 @@ static int packet_enqueue(CompareState *s, int mode)
* return: 0 means packet same
* > 0 || < 0 means packet different
*/
-static int colo_packet_compare(Packet *ppkt, Packet *spkt)
+static int colo_packet_compare_common(Packet *ppkt, Packet *spkt)
{
trace_colo_compare_ip_info(ppkt->size, inet_ntoa(ppkt->ip->ip_src),
inet_ntoa(ppkt->ip->ip_dst), spkt->size,
@@ -190,6 +190,7 @@ static int colo_packet_compare(Packet *ppkt, Packet *spkt)
if (ppkt->size == spkt->size) {
return memcmp(ppkt->data, spkt->data, spkt->size);
} else {
+ trace_colo_compare_main("Net packet size are not the same");
return -1;
}
}
@@ -202,9 +203,10 @@ static int colo_packet_compare(Packet *ppkt, Packet *spkt)
static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
{
struct tcphdr *ptcp, *stcp;
- int res;
+ int res, network_length;
trace_colo_compare_main("compare tcp");
+
if (ppkt->size != spkt->size) {
if (trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
trace_colo_compare_main("pkt size not same");
@@ -212,6 +214,12 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
return -1;
}
+ network_length = ppkt->ip->ip_hl * 4;
+ if (ppkt->size < network_length + ETH_HLEN) {
+ trace_colo_compare_main("tcp packet size error");
+ return -1;
+ }
+
ptcp = (struct tcphdr *)ppkt->transport_header;
stcp = (struct tcphdr *)spkt->transport_header;
@@ -260,10 +268,16 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
*/
static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt)
{
- int ret;
+ int ret, network_length;
trace_colo_compare_main("compare udp");
- ret = colo_packet_compare(ppkt, spkt);
+ network_length = ppkt->ip->ip_hl * 4;
+ if (ppkt->size < network_length + ETH_HLEN) {
+ trace_colo_compare_main("udp packet size error");
+ return -1;
+ }
+
+ ret = colo_packet_compare_common(ppkt, spkt);
if (ret) {
trace_colo_compare_udp_miscompare("primary pkt size", ppkt->size);
@@ -285,12 +299,12 @@ static int colo_packet_compare_icmp(Packet *spkt, Packet *ppkt)
trace_colo_compare_main("compare icmp");
network_length = ppkt->ip->ip_hl * 4;
- if (ppkt->size != spkt->size ||
- ppkt->size < network_length + ETH_HLEN) {
+ if (ppkt->size < network_length + ETH_HLEN) {
+ trace_colo_compare_main("icmp packet size error");
return -1;
}
- if (colo_packet_compare(ppkt, spkt)) {
+ if (colo_packet_compare_common(ppkt, spkt)) {
trace_colo_compare_icmp_miscompare("primary pkt size",
ppkt->size);
qemu_hexdump((char *)ppkt->data, stderr, "colo-compare",
@@ -316,7 +330,7 @@ static int colo_packet_compare_other(Packet *spkt, Packet *ppkt)
inet_ntoa(ppkt->ip->ip_dst), spkt->size,
inet_ntoa(spkt->ip->ip_src),
inet_ntoa(spkt->ip->ip_dst));
- return colo_packet_compare(ppkt, spkt);
+ return colo_packet_compare_common(ppkt, spkt);
}
static int colo_old_packet_check_one(Packet *pkt, int64_t *check_time)
--
2.7.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [PATCH 1/3] COLO-compare: Add packet size check and some fix
2017-02-25 3:32 ` [Qemu-devel] [PATCH 1/3] COLO-compare: Add " Zhang Chen
@ 2017-02-25 3:43 ` Zhang Chen
2017-02-28 3:22 ` Jason Wang
0 siblings, 1 reply; 18+ messages in thread
From: Zhang Chen @ 2017-02-25 3:43 UTC (permalink / raw)
To: qemu devel
Cc: zhangchen.fnst, Jason Wang, Li Zhijian, eddie . dong, bian naimeng
Sorry, This patch has been renamed.
please ignore this patch.
Thanks
Zhang Chen
On 02/25/2017 11:32 AM, Zhang Chen wrote:
> Add packet size check in colo_packet_compare_udp()
> and colo_packet_compare_udp(), rename function colo_packet_compare()
> to colo_packet_compare_common() that we will reuse it later.
>
> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
> ---
> net/colo-compare.c | 30 ++++++++++++++++++++++--------
> 1 file changed, 22 insertions(+), 8 deletions(-)
>
> diff --git a/net/colo-compare.c b/net/colo-compare.c
> index 300f017..e75f0ae 100644
> --- a/net/colo-compare.c
> +++ b/net/colo-compare.c
> @@ -180,7 +180,7 @@ static int packet_enqueue(CompareState *s, int mode)
> * return: 0 means packet same
> * > 0 || < 0 means packet different
> */
> -static int colo_packet_compare(Packet *ppkt, Packet *spkt)
> +static int colo_packet_compare_common(Packet *ppkt, Packet *spkt)
> {
> trace_colo_compare_ip_info(ppkt->size, inet_ntoa(ppkt->ip->ip_src),
> inet_ntoa(ppkt->ip->ip_dst), spkt->size,
> @@ -190,6 +190,7 @@ static int colo_packet_compare(Packet *ppkt, Packet *spkt)
> if (ppkt->size == spkt->size) {
> return memcmp(ppkt->data, spkt->data, spkt->size);
> } else {
> + trace_colo_compare_main("Net packet size are not the same");
> return -1;
> }
> }
> @@ -202,9 +203,10 @@ static int colo_packet_compare(Packet *ppkt, Packet *spkt)
> static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
> {
> struct tcphdr *ptcp, *stcp;
> - int res;
> + int res, network_length;
>
> trace_colo_compare_main("compare tcp");
> +
> if (ppkt->size != spkt->size) {
> if (trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
> trace_colo_compare_main("pkt size not same");
> @@ -212,6 +214,12 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
> return -1;
> }
>
> + network_length = ppkt->ip->ip_hl * 4;
> + if (ppkt->size < network_length + ETH_HLEN) {
> + trace_colo_compare_main("tcp packet size error");
> + return -1;
> + }
> +
> ptcp = (struct tcphdr *)ppkt->transport_header;
> stcp = (struct tcphdr *)spkt->transport_header;
>
> @@ -260,10 +268,16 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
> */
> static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt)
> {
> - int ret;
> + int ret, network_length;
>
> trace_colo_compare_main("compare udp");
> - ret = colo_packet_compare(ppkt, spkt);
> + network_length = ppkt->ip->ip_hl * 4;
> + if (ppkt->size < network_length + ETH_HLEN) {
> + trace_colo_compare_main("udp packet size error");
> + return -1;
> + }
> +
> + ret = colo_packet_compare_common(ppkt, spkt);
>
> if (ret) {
> trace_colo_compare_udp_miscompare("primary pkt size", ppkt->size);
> @@ -285,12 +299,12 @@ static int colo_packet_compare_icmp(Packet *spkt, Packet *ppkt)
>
> trace_colo_compare_main("compare icmp");
> network_length = ppkt->ip->ip_hl * 4;
> - if (ppkt->size != spkt->size ||
> - ppkt->size < network_length + ETH_HLEN) {
> + if (ppkt->size < network_length + ETH_HLEN) {
> + trace_colo_compare_main("icmp packet size error");
> return -1;
> }
>
> - if (colo_packet_compare(ppkt, spkt)) {
> + if (colo_packet_compare_common(ppkt, spkt)) {
> trace_colo_compare_icmp_miscompare("primary pkt size",
> ppkt->size);
> qemu_hexdump((char *)ppkt->data, stderr, "colo-compare",
> @@ -316,7 +330,7 @@ static int colo_packet_compare_other(Packet *spkt, Packet *ppkt)
> inet_ntoa(ppkt->ip->ip_dst), spkt->size,
> inet_ntoa(spkt->ip->ip_src),
> inet_ntoa(spkt->ip->ip_dst));
> - return colo_packet_compare(ppkt, spkt);
> + return colo_packet_compare_common(ppkt, spkt);
> }
>
> static int colo_old_packet_check_one(Packet *pkt, int64_t *check_time)
--
Thanks
Zhang Chen
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [PATCH 1/3] COLO-compare: Add packet size check and some fix
2017-02-25 3:43 ` Zhang Chen
@ 2017-02-28 3:22 ` Jason Wang
2017-02-28 3:27 ` Zhang Chen
0 siblings, 1 reply; 18+ messages in thread
From: Jason Wang @ 2017-02-28 3:22 UTC (permalink / raw)
To: Zhang Chen, qemu devel; +Cc: Li Zhijian, eddie . dong, bian naimeng
On 2017年02月25日 11:43, Zhang Chen wrote:
> Sorry, This patch has been renamed.
>
> please ignore this patch.
>
>
> Thanks
>
> Zhang Chen
Want to repost to just withdraw this series (patch 2 does not apply)?
Thanks
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] COLO-compare: Add packet size check and some fix
2017-02-28 3:22 ` Jason Wang
@ 2017-02-28 3:27 ` Zhang Chen
0 siblings, 0 replies; 18+ messages in thread
From: Zhang Chen @ 2017-02-28 3:27 UTC (permalink / raw)
To: Jason Wang, qemu devel
Cc: zhangchen.fnst, Li Zhijian, eddie . dong, bian naimeng
On 02/28/2017 11:22 AM, Jason Wang wrote:
>
>
> On 2017年02月25日 11:43, Zhang Chen wrote:
>> Sorry, This patch has been renamed.
>>
>> please ignore this patch.
>>
>>
>> Thanks
>>
>> Zhang Chen
>
> Want to repost to just withdraw this series (patch 2 does not apply)?
>
I have repost this patch, and will send V2 about this series later.
[Qemu-devel] [PATCH 1/3] COLO-compare: Add minimum packet size check and
some fix
Thanks
Zhang Chen
> Thanks
>
>
>
--
Thanks
Zhang Chen
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH 2/3] COLO-compare: Optimize colo_packet_compare_common
2017-02-25 3:32 [Qemu-devel] [PATCH 0/3] COLO-compare: Optimize the code and fix some bug Zhang Chen
2017-02-25 3:32 ` [Qemu-devel] [PATCH 1/3] COLO-compare: Add minimum packet size check and some fix Zhang Chen
2017-02-25 3:32 ` [Qemu-devel] [PATCH 1/3] COLO-compare: Add " Zhang Chen
@ 2017-02-25 3:32 ` Zhang Chen
2017-02-25 6:58 ` Hailiang Zhang
2017-02-25 7:26 ` Hailiang Zhang
2017-02-25 3:32 ` [Qemu-devel] [PATCH 3/3] COLO-compare: Make qemu_hexdump() print data when trace-event ON Zhang Chen
3 siblings, 2 replies; 18+ messages in thread
From: Zhang Chen @ 2017-02-25 3:32 UTC (permalink / raw)
To: qemu devel; +Cc: Jason Wang, Zhang Chen, Li Zhijian, eddie . dong, bian naimeng
Add offset args for colo_packet_compare_common, optimize
colo_packet_compare_icmp() and colo_packet_compare_udp()
just compare the IP payload.
Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
---
net/colo-compare.c | 28 +++++++++++++++++++++-------
1 file changed, 21 insertions(+), 7 deletions(-)
diff --git a/net/colo-compare.c b/net/colo-compare.c
index e75f0ae..9853232 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -180,7 +180,7 @@ static int packet_enqueue(CompareState *s, int mode)
* return: 0 means packet same
* > 0 || < 0 means packet different
*/
-static int colo_packet_compare_common(Packet *ppkt, Packet *spkt)
+static int colo_packet_compare_common(Packet *ppkt, Packet *spkt, int offset)
{
trace_colo_compare_ip_info(ppkt->size, inet_ntoa(ppkt->ip->ip_src),
inet_ntoa(ppkt->ip->ip_dst), spkt->size,
@@ -188,7 +188,8 @@ static int colo_packet_compare_common(Packet *ppkt, Packet *spkt)
inet_ntoa(spkt->ip->ip_dst));
if (ppkt->size == spkt->size) {
- return memcmp(ppkt->data, spkt->data, spkt->size);
+ return memcmp(ppkt->data + offset, spkt->data + offset,
+ spkt->size - offset);
} else {
trace_colo_compare_main("Net packet size are not the same");
return -1;
@@ -237,8 +238,7 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
spkt->ip->ip_sum = ppkt->ip->ip_sum;
}
- res = memcmp(ppkt->data + ETH_HLEN, spkt->data + ETH_HLEN,
- (spkt->size - ETH_HLEN));
+ res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN);
if (res != 0 && trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
trace_colo_compare_pkt_info_src(inet_ntoa(ppkt->ip->ip_src),
@@ -277,7 +277,14 @@ static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt)
return -1;
}
- ret = colo_packet_compare_common(ppkt, spkt);
+ /*
+ * Because of ppkt and spkt are both in the same connection,
+ * The ppkt's src ip, dst ip, src port, dst port, ip_proto all are
+ * same with spkt. In addition, IP header's Identification is a random
+ * field, we can handle it in IP fragmentation function later.
+ * So we just compare the ip payload here.
+ */
+ ret = colo_packet_compare_common(ppkt, spkt, network_length + ETH_HLEN);
if (ret) {
trace_colo_compare_udp_miscompare("primary pkt size", ppkt->size);
@@ -304,7 +311,14 @@ static int colo_packet_compare_icmp(Packet *spkt, Packet *ppkt)
return -1;
}
- if (colo_packet_compare_common(ppkt, spkt)) {
+ /*
+ * Because of ppkt and spkt are both in the same connection,
+ * The ppkt's src ip, dst ip, src port, dst port, ip_proto all are
+ * same with spkt. In addition, IP header's Identification is a random
+ * field, we can handle it in IP fragmentation function later.
+ * So we just compare the ip payload here.
+ */
+ if (colo_packet_compare_common(ppkt, spkt, network_length + ETH_HLEN)) {
trace_colo_compare_icmp_miscompare("primary pkt size",
ppkt->size);
qemu_hexdump((char *)ppkt->data, stderr, "colo-compare",
@@ -330,7 +344,7 @@ static int colo_packet_compare_other(Packet *spkt, Packet *ppkt)
inet_ntoa(ppkt->ip->ip_dst), spkt->size,
inet_ntoa(spkt->ip->ip_src),
inet_ntoa(spkt->ip->ip_dst));
- return colo_packet_compare_common(ppkt, spkt);
+ return colo_packet_compare_common(ppkt, spkt, 0);
}
static int colo_old_packet_check_one(Packet *pkt, int64_t *check_time)
--
2.7.4
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [PATCH 2/3] COLO-compare: Optimize colo_packet_compare_common
2017-02-25 3:32 ` [Qemu-devel] [PATCH 2/3] COLO-compare: Optimize colo_packet_compare_common Zhang Chen
@ 2017-02-25 6:58 ` Hailiang Zhang
2017-02-27 7:03 ` Zhang Chen
2017-02-25 7:26 ` Hailiang Zhang
1 sibling, 1 reply; 18+ messages in thread
From: Hailiang Zhang @ 2017-02-25 6:58 UTC (permalink / raw)
To: qemu-devel; +Cc: xuquan8
On 2017/2/25 11:32, Zhang Chen wrote:
> Add offset args for colo_packet_compare_common, optimize
> colo_packet_compare_icmp() and colo_packet_compare_udp()
> just compare the IP payload.
>
> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
> ---
> net/colo-compare.c | 28 +++++++++++++++++++++-------
> 1 file changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/net/colo-compare.c b/net/colo-compare.c
> index e75f0ae..9853232 100644
> --- a/net/colo-compare.c
> +++ b/net/colo-compare.c
> @@ -180,7 +180,7 @@ static int packet_enqueue(CompareState *s, int mode)
> * return: 0 means packet same
> * > 0 || < 0 means packet different
> */
> -static int colo_packet_compare_common(Packet *ppkt, Packet *spkt)
> +static int colo_packet_compare_common(Packet *ppkt, Packet *spkt, int offset)
> {
> trace_colo_compare_ip_info(ppkt->size, inet_ntoa(ppkt->ip->ip_src),
> inet_ntoa(ppkt->ip->ip_dst), spkt->size,
> @@ -188,7 +188,8 @@ static int colo_packet_compare_common(Packet *ppkt, Packet *spkt)
> inet_ntoa(spkt->ip->ip_dst));
>
> if (ppkt->size == spkt->size) {
> - return memcmp(ppkt->data, spkt->data, spkt->size);
> + return memcmp(ppkt->data + offset, spkt->data + offset,
> + spkt->size - offset);
> } else {
> trace_colo_compare_main("Net packet size are not the same");
> return -1;
> @@ -237,8 +238,7 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
> spkt->ip->ip_sum = ppkt->ip->ip_sum;
> }
>
> - res = memcmp(ppkt->data + ETH_HLEN, spkt->data + ETH_HLEN,
> - (spkt->size - ETH_HLEN));
> + res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN);
>
For tcp packets check, why not ignore the ip headers, just like udp packets check ?
Besides, here, can we compare the checksum stored in headers of tcp and udp
before call colo_packet_compare_common(), which i think will improve the comparing
performance.
Thanks.
Hailiang
> if (res != 0 && trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
> trace_colo_compare_pkt_info_src(inet_ntoa(ppkt->ip->ip_src),
> @@ -277,7 +277,14 @@ static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt)
> return -1;
> }
>
> - ret = colo_packet_compare_common(ppkt, spkt);
> + /*
> + * Because of ppkt and spkt are both in the same connection,
> + * The ppkt's src ip, dst ip, src port, dst port, ip_proto all are
> + * same with spkt. In addition, IP header's Identification is a random
> + * field, we can handle it in IP fragmentation function later.
> + * So we just compare the ip payload here.
> + */
> + ret = colo_packet_compare_common(ppkt, spkt, network_length + ETH_HLEN);
>
> if (ret) {
> trace_colo_compare_udp_miscompare("primary pkt size", ppkt->size);
> @@ -304,7 +311,14 @@ static int colo_packet_compare_icmp(Packet *spkt, Packet *ppkt)
> return -1;
> }
>
> - if (colo_packet_compare_common(ppkt, spkt)) {
> + /*
> + * Because of ppkt and spkt are both in the same connection,
> + * The ppkt's src ip, dst ip, src port, dst port, ip_proto all are
> + * same with spkt. In addition, IP header's Identification is a random
> + * field, we can handle it in IP fragmentation function later.
> + * So we just compare the ip payload here.
> + */
> + if (colo_packet_compare_common(ppkt, spkt, network_length + ETH_HLEN)) {
> trace_colo_compare_icmp_miscompare("primary pkt size",
> ppkt->size);
> qemu_hexdump((char *)ppkt->data, stderr, "colo-compare",
> @@ -330,7 +344,7 @@ static int colo_packet_compare_other(Packet *spkt, Packet *ppkt)
> inet_ntoa(ppkt->ip->ip_dst), spkt->size,
> inet_ntoa(spkt->ip->ip_src),
> inet_ntoa(spkt->ip->ip_dst));
> - return colo_packet_compare_common(ppkt, spkt);
> + return colo_packet_compare_common(ppkt, spkt, 0);
> }
>
> static int colo_old_packet_check_one(Packet *pkt, int64_t *check_time)
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [PATCH 2/3] COLO-compare: Optimize colo_packet_compare_common
2017-02-25 6:58 ` Hailiang Zhang
@ 2017-02-27 7:03 ` Zhang Chen
2017-02-27 7:28 ` Hailiang Zhang
0 siblings, 1 reply; 18+ messages in thread
From: Zhang Chen @ 2017-02-27 7:03 UTC (permalink / raw)
To: Hailiang Zhang, qemu-devel; +Cc: zhangchen.fnst, xuquan8
On 02/25/2017 02:58 PM, Hailiang Zhang wrote:
> On 2017/2/25 11:32, Zhang Chen wrote:
>> Add offset args for colo_packet_compare_common, optimize
>> colo_packet_compare_icmp() and colo_packet_compare_udp()
>> just compare the IP payload.
>>
>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>> ---
>> net/colo-compare.c | 28 +++++++++++++++++++++-------
>> 1 file changed, 21 insertions(+), 7 deletions(-)
>>
>> diff --git a/net/colo-compare.c b/net/colo-compare.c
>> index e75f0ae..9853232 100644
>> --- a/net/colo-compare.c
>> +++ b/net/colo-compare.c
>> @@ -180,7 +180,7 @@ static int packet_enqueue(CompareState *s, int mode)
>> * return: 0 means packet same
>> * > 0 || < 0 means packet different
>> */
>> -static int colo_packet_compare_common(Packet *ppkt, Packet *spkt)
>> +static int colo_packet_compare_common(Packet *ppkt, Packet *spkt,
>> int offset)
>> {
>> trace_colo_compare_ip_info(ppkt->size,
>> inet_ntoa(ppkt->ip->ip_src),
>> inet_ntoa(ppkt->ip->ip_dst), spkt->size,
>> @@ -188,7 +188,8 @@ static int colo_packet_compare_common(Packet
>> *ppkt, Packet *spkt)
>> inet_ntoa(spkt->ip->ip_dst));
>>
>> if (ppkt->size == spkt->size) {
>> - return memcmp(ppkt->data, spkt->data, spkt->size);
>> + return memcmp(ppkt->data + offset, spkt->data + offset,
>> + spkt->size - offset);
>> } else {
>> trace_colo_compare_main("Net packet size are not the same");
>> return -1;
>> @@ -237,8 +238,7 @@ static int colo_packet_compare_tcp(Packet *spkt,
>> Packet *ppkt)
>> spkt->ip->ip_sum = ppkt->ip->ip_sum;
>> }
>>
>> - res = memcmp(ppkt->data + ETH_HLEN, spkt->data + ETH_HLEN,
>> - (spkt->size - ETH_HLEN));
>> + res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN);
>>
>
> For tcp packets check, why not ignore the ip headers, just like udp
> packets check ?
> Besides, here, can we compare the checksum stored in headers of tcp
> and udp
> before call colo_packet_compare_common(), which i think will improve
> the comparing
> performance.
That's another way to compare the packet suggested by Dr. David Alan
Gilbert,
It makes two packets IP header be same firstly, then compare all IP packet.
This way can tell people why we ignore IP header explicitly in other
packets check like udp.
For performance, If we ignore the IP header that will reduce at least 20
byte not to be compared.
So, ignore ip header have better comparing performance.
Thanks
Zhang Chen
>
> Thanks.
> Hailiang
>
>> if (res != 0 &&
>> trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
>> trace_colo_compare_pkt_info_src(inet_ntoa(ppkt->ip->ip_src),
>> @@ -277,7 +277,14 @@ static int colo_packet_compare_udp(Packet *spkt,
>> Packet *ppkt)
>> return -1;
>> }
>>
>> - ret = colo_packet_compare_common(ppkt, spkt);
>> + /*
>> + * Because of ppkt and spkt are both in the same connection,
>> + * The ppkt's src ip, dst ip, src port, dst port, ip_proto all are
>> + * same with spkt. In addition, IP header's Identification is a
>> random
>> + * field, we can handle it in IP fragmentation function later.
>> + * So we just compare the ip payload here.
>> + */
>> + ret = colo_packet_compare_common(ppkt, spkt, network_length +
>> ETH_HLEN);
>>
>> if (ret) {
>> trace_colo_compare_udp_miscompare("primary pkt size",
>> ppkt->size);
>> @@ -304,7 +311,14 @@ static int colo_packet_compare_icmp(Packet
>> *spkt, Packet *ppkt)
>> return -1;
>> }
>>
>> - if (colo_packet_compare_common(ppkt, spkt)) {
>> + /*
>> + * Because of ppkt and spkt are both in the same connection,
>> + * The ppkt's src ip, dst ip, src port, dst port, ip_proto all are
>> + * same with spkt. In addition, IP header's Identification is a
>> random
>> + * field, we can handle it in IP fragmentation function later.
>> + * So we just compare the ip payload here.
>> + */
>> + if (colo_packet_compare_common(ppkt, spkt, network_length +
>> ETH_HLEN)) {
>> trace_colo_compare_icmp_miscompare("primary pkt size",
>> ppkt->size);
>> qemu_hexdump((char *)ppkt->data, stderr, "colo-compare",
>> @@ -330,7 +344,7 @@ static int colo_packet_compare_other(Packet
>> *spkt, Packet *ppkt)
>> inet_ntoa(ppkt->ip->ip_dst), spkt->size,
>> inet_ntoa(spkt->ip->ip_src),
>> inet_ntoa(spkt->ip->ip_dst));
>> - return colo_packet_compare_common(ppkt, spkt);
>> + return colo_packet_compare_common(ppkt, spkt, 0);
>> }
>>
>> static int colo_old_packet_check_one(Packet *pkt, int64_t *check_time)
>>
>
>
>
>
> .
>
--
Thanks
Zhang Chen
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [PATCH 2/3] COLO-compare: Optimize colo_packet_compare_common
2017-02-27 7:03 ` Zhang Chen
@ 2017-02-27 7:28 ` Hailiang Zhang
2017-02-27 7:34 ` Zhang Chen
0 siblings, 1 reply; 18+ messages in thread
From: Hailiang Zhang @ 2017-02-27 7:28 UTC (permalink / raw)
To: Zhang Chen, qemu-devel; +Cc: xuquan8
On 2017/2/27 15:03, Zhang Chen wrote:
>
>
> On 02/25/2017 02:58 PM, Hailiang Zhang wrote:
>> On 2017/2/25 11:32, Zhang Chen wrote:
>>> Add offset args for colo_packet_compare_common, optimize
>>> colo_packet_compare_icmp() and colo_packet_compare_udp()
>>> just compare the IP payload.
>>>
>>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>>> ---
>>> net/colo-compare.c | 28 +++++++++++++++++++++-------
>>> 1 file changed, 21 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/net/colo-compare.c b/net/colo-compare.c
>>> index e75f0ae..9853232 100644
>>> --- a/net/colo-compare.c
>>> +++ b/net/colo-compare.c
>>> @@ -180,7 +180,7 @@ static int packet_enqueue(CompareState *s, int mode)
>>> * return: 0 means packet same
>>> * > 0 || < 0 means packet different
>>> */
>>> -static int colo_packet_compare_common(Packet *ppkt, Packet *spkt)
>>> +static int colo_packet_compare_common(Packet *ppkt, Packet *spkt,
>>> int offset)
>>> {
>>> trace_colo_compare_ip_info(ppkt->size,
>>> inet_ntoa(ppkt->ip->ip_src),
>>> inet_ntoa(ppkt->ip->ip_dst), spkt->size,
>>> @@ -188,7 +188,8 @@ static int colo_packet_compare_common(Packet
>>> *ppkt, Packet *spkt)
>>> inet_ntoa(spkt->ip->ip_dst));
>>>
>>> if (ppkt->size == spkt->size) {
>>> - return memcmp(ppkt->data, spkt->data, spkt->size);
>>> + return memcmp(ppkt->data + offset, spkt->data + offset,
>>> + spkt->size - offset);
>>> } else {
>>> trace_colo_compare_main("Net packet size are not the same");
>>> return -1;
>>> @@ -237,8 +238,7 @@ static int colo_packet_compare_tcp(Packet *spkt,
>>> Packet *ppkt)
>>> spkt->ip->ip_sum = ppkt->ip->ip_sum;
>>> }
>>>
>>> - res = memcmp(ppkt->data + ETH_HLEN, spkt->data + ETH_HLEN,
>>> - (spkt->size - ETH_HLEN));
>>> + res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN);
>>>
>>
>> For tcp packets check, why not ignore the ip headers, just like udp
>> packets check ?
>> Besides, here, can we compare the checksum stored in headers of tcp
>> and udp
>> before call colo_packet_compare_common(), which i think will improve
>> the comparing
>> performance.
>
> That's another way to compare the packet suggested by Dr. David Alan
> Gilbert,
> It makes two packets IP header be same firstly, then compare all IP packet.
> This way can tell people why we ignore IP header explicitly in other
> packets check like udp.
> For performance, If we ignore the IP header that will reduce at least 20
> byte not to be compared.
> So, ignore ip header have better comparing performance.
>
OK, here, i think we can re-use the checksum value stored in headers of tcp
or udp, comparing it first before comparing the complete payload of tcp or udp
packets is another way to improve the performance, it is only 2 bytes.
Thanks.
> Thanks
> Zhang Chen
>
>
>>
>> Thanks.
>> Hailiang
>>
>>> if (res != 0 &&
>>> trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
>>> trace_colo_compare_pkt_info_src(inet_ntoa(ppkt->ip->ip_src),
>>> @@ -277,7 +277,14 @@ static int colo_packet_compare_udp(Packet *spkt,
>>> Packet *ppkt)
>>> return -1;
>>> }
>>>
>>> - ret = colo_packet_compare_common(ppkt, spkt);
>>> + /*
>>> + * Because of ppkt and spkt are both in the same connection,
>>> + * The ppkt's src ip, dst ip, src port, dst port, ip_proto all are
>>> + * same with spkt. In addition, IP header's Identification is a
>>> random
>>> + * field, we can handle it in IP fragmentation function later.
>>> + * So we just compare the ip payload here.
>>> + */
>>> + ret = colo_packet_compare_common(ppkt, spkt, network_length +
>>> ETH_HLEN);
>>>
>>> if (ret) {
>>> trace_colo_compare_udp_miscompare("primary pkt size",
>>> ppkt->size);
>>> @@ -304,7 +311,14 @@ static int colo_packet_compare_icmp(Packet
>>> *spkt, Packet *ppkt)
>>> return -1;
>>> }
>>>
>>> - if (colo_packet_compare_common(ppkt, spkt)) {
>>> + /*
>>> + * Because of ppkt and spkt are both in the same connection,
>>> + * The ppkt's src ip, dst ip, src port, dst port, ip_proto all are
>>> + * same with spkt. In addition, IP header's Identification is a
>>> random
>>> + * field, we can handle it in IP fragmentation function later.
>>> + * So we just compare the ip payload here.
>>> + */
>>> + if (colo_packet_compare_common(ppkt, spkt, network_length +
>>> ETH_HLEN)) {
>>> trace_colo_compare_icmp_miscompare("primary pkt size",
>>> ppkt->size);
>>> qemu_hexdump((char *)ppkt->data, stderr, "colo-compare",
>>> @@ -330,7 +344,7 @@ static int colo_packet_compare_other(Packet
>>> *spkt, Packet *ppkt)
>>> inet_ntoa(ppkt->ip->ip_dst), spkt->size,
>>> inet_ntoa(spkt->ip->ip_src),
>>> inet_ntoa(spkt->ip->ip_dst));
>>> - return colo_packet_compare_common(ppkt, spkt);
>>> + return colo_packet_compare_common(ppkt, spkt, 0);
>>> }
>>>
>>> static int colo_old_packet_check_one(Packet *pkt, int64_t *check_time)
>>>
>>
>>
>>
>>
>> .
>>
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [PATCH 2/3] COLO-compare: Optimize colo_packet_compare_common
2017-02-27 7:28 ` Hailiang Zhang
@ 2017-02-27 7:34 ` Zhang Chen
2017-02-27 8:43 ` Hailiang Zhang
0 siblings, 1 reply; 18+ messages in thread
From: Zhang Chen @ 2017-02-27 7:34 UTC (permalink / raw)
To: Hailiang Zhang, qemu-devel; +Cc: zhangchen.fnst, xuquan8
On 02/27/2017 03:28 PM, Hailiang Zhang wrote:
> On 2017/2/27 15:03, Zhang Chen wrote:
>>
>>
>> On 02/25/2017 02:58 PM, Hailiang Zhang wrote:
>>> On 2017/2/25 11:32, Zhang Chen wrote:
>>>> Add offset args for colo_packet_compare_common, optimize
>>>> colo_packet_compare_icmp() and colo_packet_compare_udp()
>>>> just compare the IP payload.
>>>>
>>>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>>>> ---
>>>> net/colo-compare.c | 28 +++++++++++++++++++++-------
>>>> 1 file changed, 21 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/net/colo-compare.c b/net/colo-compare.c
>>>> index e75f0ae..9853232 100644
>>>> --- a/net/colo-compare.c
>>>> +++ b/net/colo-compare.c
>>>> @@ -180,7 +180,7 @@ static int packet_enqueue(CompareState *s, int
>>>> mode)
>>>> * return: 0 means packet same
>>>> * > 0 || < 0 means packet different
>>>> */
>>>> -static int colo_packet_compare_common(Packet *ppkt, Packet *spkt)
>>>> +static int colo_packet_compare_common(Packet *ppkt, Packet *spkt,
>>>> int offset)
>>>> {
>>>> trace_colo_compare_ip_info(ppkt->size,
>>>> inet_ntoa(ppkt->ip->ip_src),
>>>> inet_ntoa(ppkt->ip->ip_dst), spkt->size,
>>>> @@ -188,7 +188,8 @@ static int colo_packet_compare_common(Packet
>>>> *ppkt, Packet *spkt)
>>>> inet_ntoa(spkt->ip->ip_dst));
>>>>
>>>> if (ppkt->size == spkt->size) {
>>>> - return memcmp(ppkt->data, spkt->data, spkt->size);
>>>> + return memcmp(ppkt->data + offset, spkt->data + offset,
>>>> + spkt->size - offset);
>>>> } else {
>>>> trace_colo_compare_main("Net packet size are not the
>>>> same");
>>>> return -1;
>>>> @@ -237,8 +238,7 @@ static int colo_packet_compare_tcp(Packet *spkt,
>>>> Packet *ppkt)
>>>> spkt->ip->ip_sum = ppkt->ip->ip_sum;
>>>> }
>>>>
>>>> - res = memcmp(ppkt->data + ETH_HLEN, spkt->data + ETH_HLEN,
>>>> - (spkt->size - ETH_HLEN));
>>>> + res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN);
>>>>
>>>
>>> For tcp packets check, why not ignore the ip headers, just like udp
>>> packets check ?
>>> Besides, here, can we compare the checksum stored in headers of tcp
>>> and udp
>>> before call colo_packet_compare_common(), which i think will improve
>>> the comparing
>>> performance.
>>
>> That's another way to compare the packet suggested by Dr. David Alan
>> Gilbert,
>> It makes two packets IP header be same firstly, then compare all IP
>> packet.
>> This way can tell people why we ignore IP header explicitly in other
>> packets check like udp.
>> For performance, If we ignore the IP header that will reduce at least 20
>> byte not to be compared.
>> So, ignore ip header have better comparing performance.
>>
>
> OK, here, i think we can re-use the checksum value stored in headers
> of tcp
> or udp, comparing it first before comparing the complete payload of
> tcp or udp
> packets is another way to improve the performance, it is only 2 bytes.
No, The IP header's checksum are always different, Because the IP
header's ID field
are always different.
Thanks
Zhang Chen
>
> Thanks.
>
>> Thanks
>> Zhang Chen
>>
>>
>>>
>>> Thanks.
>>> Hailiang
>>>
>>>> if (res != 0 &&
>>>> trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
>>>> trace_colo_compare_pkt_info_src(inet_ntoa(ppkt->ip->ip_src),
>>>> @@ -277,7 +277,14 @@ static int colo_packet_compare_udp(Packet *spkt,
>>>> Packet *ppkt)
>>>> return -1;
>>>> }
>>>>
>>>> - ret = colo_packet_compare_common(ppkt, spkt);
>>>> + /*
>>>> + * Because of ppkt and spkt are both in the same connection,
>>>> + * The ppkt's src ip, dst ip, src port, dst port, ip_proto all
>>>> are
>>>> + * same with spkt. In addition, IP header's Identification is a
>>>> random
>>>> + * field, we can handle it in IP fragmentation function later.
>>>> + * So we just compare the ip payload here.
>>>> + */
>>>> + ret = colo_packet_compare_common(ppkt, spkt, network_length +
>>>> ETH_HLEN);
>>>>
>>>> if (ret) {
>>>> trace_colo_compare_udp_miscompare("primary pkt size",
>>>> ppkt->size);
>>>> @@ -304,7 +311,14 @@ static int colo_packet_compare_icmp(Packet
>>>> *spkt, Packet *ppkt)
>>>> return -1;
>>>> }
>>>>
>>>> - if (colo_packet_compare_common(ppkt, spkt)) {
>>>> + /*
>>>> + * Because of ppkt and spkt are both in the same connection,
>>>> + * The ppkt's src ip, dst ip, src port, dst port, ip_proto all
>>>> are
>>>> + * same with spkt. In addition, IP header's Identification is a
>>>> random
>>>> + * field, we can handle it in IP fragmentation function later.
>>>> + * So we just compare the ip payload here.
>>>> + */
>>>> + if (colo_packet_compare_common(ppkt, spkt, network_length +
>>>> ETH_HLEN)) {
>>>> trace_colo_compare_icmp_miscompare("primary pkt size",
>>>> ppkt->size);
>>>> qemu_hexdump((char *)ppkt->data, stderr, "colo-compare",
>>>> @@ -330,7 +344,7 @@ static int colo_packet_compare_other(Packet
>>>> *spkt, Packet *ppkt)
>>>> inet_ntoa(ppkt->ip->ip_dst), spkt->size,
>>>> inet_ntoa(spkt->ip->ip_src),
>>>> inet_ntoa(spkt->ip->ip_dst));
>>>> - return colo_packet_compare_common(ppkt, spkt);
>>>> + return colo_packet_compare_common(ppkt, spkt, 0);
>>>> }
>>>>
>>>> static int colo_old_packet_check_one(Packet *pkt, int64_t
>>>> *check_time)
>>>>
>>>
>>>
>>>
>>>
>>> .
>>>
>>
>
>
>
> .
>
--
Thanks
Zhang Chen
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [PATCH 2/3] COLO-compare: Optimize colo_packet_compare_common
2017-02-27 7:34 ` Zhang Chen
@ 2017-02-27 8:43 ` Hailiang Zhang
2017-02-27 9:02 ` Zhang Chen
0 siblings, 1 reply; 18+ messages in thread
From: Hailiang Zhang @ 2017-02-27 8:43 UTC (permalink / raw)
To: Zhang Chen, qemu-devel; +Cc: xuquan8
On 2017/2/27 15:34, Zhang Chen wrote:
>
>
> On 02/27/2017 03:28 PM, Hailiang Zhang wrote:
>> On 2017/2/27 15:03, Zhang Chen wrote:
>>>
>>>
>>> On 02/25/2017 02:58 PM, Hailiang Zhang wrote:
>>>> On 2017/2/25 11:32, Zhang Chen wrote:
>>>>> Add offset args for colo_packet_compare_common, optimize
>>>>> colo_packet_compare_icmp() and colo_packet_compare_udp()
>>>>> just compare the IP payload.
>>>>>
>>>>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>>>>> ---
>>>>> net/colo-compare.c | 28 +++++++++++++++++++++-------
>>>>> 1 file changed, 21 insertions(+), 7 deletions(-)
>>>>>
>>>>> diff --git a/net/colo-compare.c b/net/colo-compare.c
>>>>> index e75f0ae..9853232 100644
>>>>> --- a/net/colo-compare.c
>>>>> +++ b/net/colo-compare.c
>>>>> @@ -180,7 +180,7 @@ static int packet_enqueue(CompareState *s, int
>>>>> mode)
>>>>> * return: 0 means packet same
>>>>> * > 0 || < 0 means packet different
>>>>> */
>>>>> -static int colo_packet_compare_common(Packet *ppkt, Packet *spkt)
>>>>> +static int colo_packet_compare_common(Packet *ppkt, Packet *spkt,
>>>>> int offset)
>>>>> {
>>>>> trace_colo_compare_ip_info(ppkt->size,
>>>>> inet_ntoa(ppkt->ip->ip_src),
>>>>> inet_ntoa(ppkt->ip->ip_dst), spkt->size,
>>>>> @@ -188,7 +188,8 @@ static int colo_packet_compare_common(Packet
>>>>> *ppkt, Packet *spkt)
>>>>> inet_ntoa(spkt->ip->ip_dst));
>>>>>
>>>>> if (ppkt->size == spkt->size) {
>>>>> - return memcmp(ppkt->data, spkt->data, spkt->size);
>>>>> + return memcmp(ppkt->data + offset, spkt->data + offset,
>>>>> + spkt->size - offset);
>>>>> } else {
>>>>> trace_colo_compare_main("Net packet size are not the
>>>>> same");
>>>>> return -1;
>>>>> @@ -237,8 +238,7 @@ static int colo_packet_compare_tcp(Packet *spkt,
>>>>> Packet *ppkt)
>>>>> spkt->ip->ip_sum = ppkt->ip->ip_sum;
>>>>> }
>>>>>
>>>>> - res = memcmp(ppkt->data + ETH_HLEN, spkt->data + ETH_HLEN,
>>>>> - (spkt->size - ETH_HLEN));
>>>>> + res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN);
>>>>>
>>>>
>>>> For tcp packets check, why not ignore the ip headers, just like udp
>>>> packets check ?
>>>> Besides, here, can we compare the checksum stored in headers of tcp
>>>> and udp
>>>> before call colo_packet_compare_common(), which i think will improve
>>>> the comparing
>>>> performance.
>>>
>>> That's another way to compare the packet suggested by Dr. David Alan
>>> Gilbert,
>>> It makes two packets IP header be same firstly, then compare all IP
>>> packet.
>>> This way can tell people why we ignore IP header explicitly in other
>>> packets check like udp.
>>> For performance, If we ignore the IP header that will reduce at least 20
>>> byte not to be compared.
>>> So, ignore ip header have better comparing performance.
>>>
>>
>> OK, here, i think we can re-use the checksum value stored in headers
>> of tcp
>> or udp, comparing it first before comparing the complete payload of
>> tcp or udp
>> packets is another way to improve the performance, it is only 2 bytes.
>
> No, The IP header's checksum are always different, Because the IP
> header's ID field
> are always different.
>
Not checksum in ip header， i mean checksum in tcp header or udp header.
> Thanks
> Zhang Chen
>
>>
>> Thanks.
>>
>>> Thanks
>>> Zhang Chen
>>>
>>>
>>>>
>>>> Thanks.
>>>> Hailiang
>>>>
>>>>> if (res != 0 &&
>>>>> trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
>>>>> trace_colo_compare_pkt_info_src(inet_ntoa(ppkt->ip->ip_src),
>>>>> @@ -277,7 +277,14 @@ static int colo_packet_compare_udp(Packet *spkt,
>>>>> Packet *ppkt)
>>>>> return -1;
>>>>> }
>>>>>
>>>>> - ret = colo_packet_compare_common(ppkt, spkt);
>>>>> + /*
>>>>> + * Because of ppkt and spkt are both in the same connection,
>>>>> + * The ppkt's src ip, dst ip, src port, dst port, ip_proto all
>>>>> are
>>>>> + * same with spkt. In addition, IP header's Identification is a
>>>>> random
>>>>> + * field, we can handle it in IP fragmentation function later.
>>>>> + * So we just compare the ip payload here.
>>>>> + */
>>>>> + ret = colo_packet_compare_common(ppkt, spkt, network_length +
>>>>> ETH_HLEN);
>>>>>
>>>>> if (ret) {
>>>>> trace_colo_compare_udp_miscompare("primary pkt size",
>>>>> ppkt->size);
>>>>> @@ -304,7 +311,14 @@ static int colo_packet_compare_icmp(Packet
>>>>> *spkt, Packet *ppkt)
>>>>> return -1;
>>>>> }
>>>>>
>>>>> - if (colo_packet_compare_common(ppkt, spkt)) {
>>>>> + /*
>>>>> + * Because of ppkt and spkt are both in the same connection,
>>>>> + * The ppkt's src ip, dst ip, src port, dst port, ip_proto all
>>>>> are
>>>>> + * same with spkt. In addition, IP header's Identification is a
>>>>> random
>>>>> + * field, we can handle it in IP fragmentation function later.
>>>>> + * So we just compare the ip payload here.
>>>>> + */
>>>>> + if (colo_packet_compare_common(ppkt, spkt, network_length +
>>>>> ETH_HLEN)) {
>>>>> trace_colo_compare_icmp_miscompare("primary pkt size",
>>>>> ppkt->size);
>>>>> qemu_hexdump((char *)ppkt->data, stderr, "colo-compare",
>>>>> @@ -330,7 +344,7 @@ static int colo_packet_compare_other(Packet
>>>>> *spkt, Packet *ppkt)
>>>>> inet_ntoa(ppkt->ip->ip_dst), spkt->size,
>>>>> inet_ntoa(spkt->ip->ip_src),
>>>>> inet_ntoa(spkt->ip->ip_dst));
>>>>> - return colo_packet_compare_common(ppkt, spkt);
>>>>> + return colo_packet_compare_common(ppkt, spkt, 0);
>>>>> }
>>>>>
>>>>> static int colo_old_packet_check_one(Packet *pkt, int64_t
>>>>> *check_time)
>>>>>
>>>>
>>>>
>>>>
>>>>
>>>> .
>>>>
>>>
>>
>>
>>
>> .
>>
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [PATCH 2/3] COLO-compare: Optimize colo_packet_compare_common
2017-02-27 8:43 ` Hailiang Zhang
@ 2017-02-27 9:02 ` Zhang Chen
0 siblings, 0 replies; 18+ messages in thread
From: Zhang Chen @ 2017-02-27 9:02 UTC (permalink / raw)
To: Hailiang Zhang, qemu-devel; +Cc: zhangchen.fnst, xuquan8
On 02/27/2017 04:43 PM, Hailiang Zhang wrote:
> On 2017/2/27 15:34, Zhang Chen wrote:
>>
>>
>> On 02/27/2017 03:28 PM, Hailiang Zhang wrote:
>>> On 2017/2/27 15:03, Zhang Chen wrote:
>>>>
>>>>
>>>> On 02/25/2017 02:58 PM, Hailiang Zhang wrote:
>>>>> On 2017/2/25 11:32, Zhang Chen wrote:
>>>>>> Add offset args for colo_packet_compare_common, optimize
>>>>>> colo_packet_compare_icmp() and colo_packet_compare_udp()
>>>>>> just compare the IP payload.
>>>>>>
>>>>>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>>>>>> ---
>>>>>> net/colo-compare.c | 28 +++++++++++++++++++++-------
>>>>>> 1 file changed, 21 insertions(+), 7 deletions(-)
>>>>>>
>>>>>> diff --git a/net/colo-compare.c b/net/colo-compare.c
>>>>>> index e75f0ae..9853232 100644
>>>>>> --- a/net/colo-compare.c
>>>>>> +++ b/net/colo-compare.c
>>>>>> @@ -180,7 +180,7 @@ static int packet_enqueue(CompareState *s, int
>>>>>> mode)
>>>>>> * return: 0 means packet same
>>>>>> * > 0 || < 0 means packet different
>>>>>> */
>>>>>> -static int colo_packet_compare_common(Packet *ppkt, Packet *spkt)
>>>>>> +static int colo_packet_compare_common(Packet *ppkt, Packet *spkt,
>>>>>> int offset)
>>>>>> {
>>>>>> trace_colo_compare_ip_info(ppkt->size,
>>>>>> inet_ntoa(ppkt->ip->ip_src),
>>>>>> inet_ntoa(ppkt->ip->ip_dst), spkt->size,
>>>>>> @@ -188,7 +188,8 @@ static int colo_packet_compare_common(Packet
>>>>>> *ppkt, Packet *spkt)
>>>>>> inet_ntoa(spkt->ip->ip_dst));
>>>>>>
>>>>>> if (ppkt->size == spkt->size) {
>>>>>> - return memcmp(ppkt->data, spkt->data, spkt->size);
>>>>>> + return memcmp(ppkt->data + offset, spkt->data + offset,
>>>>>> + spkt->size - offset);
>>>>>> } else {
>>>>>> trace_colo_compare_main("Net packet size are not the
>>>>>> same");
>>>>>> return -1;
>>>>>> @@ -237,8 +238,7 @@ static int colo_packet_compare_tcp(Packet *spkt,
>>>>>> Packet *ppkt)
>>>>>> spkt->ip->ip_sum = ppkt->ip->ip_sum;
>>>>>> }
>>>>>>
>>>>>> - res = memcmp(ppkt->data + ETH_HLEN, spkt->data + ETH_HLEN,
>>>>>> - (spkt->size - ETH_HLEN));
>>>>>> + res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN);
>>>>>>
>>>>>
>>>>> For tcp packets check, why not ignore the ip headers, just like udp
>>>>> packets check ?
>>>>> Besides, here, can we compare the checksum stored in headers of tcp
>>>>> and udp
>>>>> before call colo_packet_compare_common(), which i think will improve
>>>>> the comparing
>>>>> performance.
>>>>
>>>> That's another way to compare the packet suggested by Dr. David Alan
>>>> Gilbert,
>>>> It makes two packets IP header be same firstly, then compare all IP
>>>> packet.
>>>> This way can tell people why we ignore IP header explicitly in other
>>>> packets check like udp.
>>>> For performance, If we ignore the IP header that will reduce at
>>>> least 20
>>>> byte not to be compared.
>>>> So, ignore ip header have better comparing performance.
>>>>
>>>
>>> OK, here, i think we can re-use the checksum value stored in headers
>>> of tcp
>>> or udp, comparing it first before comparing the complete payload of
>>> tcp or udp
>>> packets is another way to improve the performance, it is only 2 bytes.
>>
>> No, The IP header's checksum are always different, Because the IP
>> header's ID field
>> are always different.
>>
>
> Not checksum in ip header， i mean checksum in tcp header or udp header.
TCP header can do it, but UDP header's checksum is optional.
Thanks
Zhang Chen
>
>
>> Thanks
>> Zhang Chen
>>
>>>
>>> Thanks.
>>>
>>>> Thanks
>>>> Zhang Chen
>>>>
>>>>
>>>>>
>>>>> Thanks.
>>>>> Hailiang
>>>>>
>>>>>> if (res != 0 &&
>>>>>> trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
>>>>>> trace_colo_compare_pkt_info_src(inet_ntoa(ppkt->ip->ip_src),
>>>>>> @@ -277,7 +277,14 @@ static int colo_packet_compare_udp(Packet
>>>>>> *spkt,
>>>>>> Packet *ppkt)
>>>>>> return -1;
>>>>>> }
>>>>>>
>>>>>> - ret = colo_packet_compare_common(ppkt, spkt);
>>>>>> + /*
>>>>>> + * Because of ppkt and spkt are both in the same connection,
>>>>>> + * The ppkt's src ip, dst ip, src port, dst port, ip_proto all
>>>>>> are
>>>>>> + * same with spkt. In addition, IP header's Identification is a
>>>>>> random
>>>>>> + * field, we can handle it in IP fragmentation function later.
>>>>>> + * So we just compare the ip payload here.
>>>>>> + */
>>>>>> + ret = colo_packet_compare_common(ppkt, spkt, network_length +
>>>>>> ETH_HLEN);
>>>>>>
>>>>>> if (ret) {
>>>>>> trace_colo_compare_udp_miscompare("primary pkt size",
>>>>>> ppkt->size);
>>>>>> @@ -304,7 +311,14 @@ static int colo_packet_compare_icmp(Packet
>>>>>> *spkt, Packet *ppkt)
>>>>>> return -1;
>>>>>> }
>>>>>>
>>>>>> - if (colo_packet_compare_common(ppkt, spkt)) {
>>>>>> + /*
>>>>>> + * Because of ppkt and spkt are both in the same connection,
>>>>>> + * The ppkt's src ip, dst ip, src port, dst port, ip_proto all
>>>>>> are
>>>>>> + * same with spkt. In addition, IP header's Identification is a
>>>>>> random
>>>>>> + * field, we can handle it in IP fragmentation function later.
>>>>>> + * So we just compare the ip payload here.
>>>>>> + */
>>>>>> + if (colo_packet_compare_common(ppkt, spkt, network_length +
>>>>>> ETH_HLEN)) {
>>>>>> trace_colo_compare_icmp_miscompare("primary pkt size",
>>>>>> ppkt->size);
>>>>>> qemu_hexdump((char *)ppkt->data, stderr, "colo-compare",
>>>>>> @@ -330,7 +344,7 @@ static int colo_packet_compare_other(Packet
>>>>>> *spkt, Packet *ppkt)
>>>>>> inet_ntoa(ppkt->ip->ip_dst), spkt->size,
>>>>>> inet_ntoa(spkt->ip->ip_src),
>>>>>> inet_ntoa(spkt->ip->ip_dst));
>>>>>> - return colo_packet_compare_common(ppkt, spkt);
>>>>>> + return colo_packet_compare_common(ppkt, spkt, 0);
>>>>>> }
>>>>>>
>>>>>> static int colo_old_packet_check_one(Packet *pkt, int64_t
>>>>>> *check_time)
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> .
>>>>>
>>>>
>>>
>>>
>>>
>>> .
>>>
>>
>
>
>
> .
>
--
Thanks
Zhang Chen
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH 2/3] COLO-compare: Optimize colo_packet_compare_common
2017-02-25 3:32 ` [Qemu-devel] [PATCH 2/3] COLO-compare: Optimize colo_packet_compare_common Zhang Chen
2017-02-25 6:58 ` Hailiang Zhang
@ 2017-02-25 7:26 ` Hailiang Zhang
2017-02-27 7:06 ` Zhang Chen
1 sibling, 1 reply; 18+ messages in thread
From: Hailiang Zhang @ 2017-02-25 7:26 UTC (permalink / raw)
To: qemu-devel; +Cc: xuquan8
On 2017/2/25 11:32, Zhang Chen wrote:
> Add offset args for colo_packet_compare_common, optimize
> colo_packet_compare_icmp() and colo_packet_compare_udp()
> just compare the IP payload.
>
> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
> ---
> net/colo-compare.c | 28 +++++++++++++++++++++-------
> 1 file changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/net/colo-compare.c b/net/colo-compare.c
> index e75f0ae..9853232 100644
> --- a/net/colo-compare.c
> +++ b/net/colo-compare.c
> @@ -180,7 +180,7 @@ static int packet_enqueue(CompareState *s, int mode)
> * return: 0 means packet same
> * > 0 || < 0 means packet different
> */
> -static int colo_packet_compare_common(Packet *ppkt, Packet *spkt)
> +static int colo_packet_compare_common(Packet *ppkt, Packet *spkt, int offset)
> {
> trace_colo_compare_ip_info(ppkt->size, inet_ntoa(ppkt->ip->ip_src),
> inet_ntoa(ppkt->ip->ip_dst), spkt->size,
> @@ -188,7 +188,8 @@ static int colo_packet_compare_common(Packet *ppkt, Packet *spkt)
> inet_ntoa(spkt->ip->ip_dst));
>
> if (ppkt->size == spkt->size) {
This check is useless, because we have done such checks in every caller.
> - return memcmp(ppkt->data, spkt->data, spkt->size);
> + return memcmp(ppkt->data + offset, spkt->data + offset,
> + spkt->size - offset);
> } else {
> trace_colo_compare_main("Net packet size are not the same");
> return -1;
> @@ -237,8 +238,7 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
> spkt->ip->ip_sum = ppkt->ip->ip_sum;
> }
>
> - res = memcmp(ppkt->data + ETH_HLEN, spkt->data + ETH_HLEN,
> - (spkt->size - ETH_HLEN));
> + res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN);
>
> if (res != 0 && trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
> trace_colo_compare_pkt_info_src(inet_ntoa(ppkt->ip->ip_src),
> @@ -277,7 +277,14 @@ static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt)
> return -1;
> }
>
> - ret = colo_packet_compare_common(ppkt, spkt);
> + /*
> + * Because of ppkt and spkt are both in the same connection,
> + * The ppkt's src ip, dst ip, src port, dst port, ip_proto all are
> + * same with spkt. In addition, IP header's Identification is a random
> + * field, we can handle it in IP fragmentation function later.
> + * So we just compare the ip payload here.
> + */
> + ret = colo_packet_compare_common(ppkt, spkt, network_length + ETH_HLEN);
>
> if (ret) {
> trace_colo_compare_udp_miscompare("primary pkt size", ppkt->size);
> @@ -304,7 +311,14 @@ static int colo_packet_compare_icmp(Packet *spkt, Packet *ppkt)
> return -1;
> }
>
> - if (colo_packet_compare_common(ppkt, spkt)) {
> + /*
> + * Because of ppkt and spkt are both in the same connection,
> + * The ppkt's src ip, dst ip, src port, dst port, ip_proto all are
> + * same with spkt. In addition, IP header's Identification is a random
> + * field, we can handle it in IP fragmentation function later.
> + * So we just compare the ip payload here.
> + */
> + if (colo_packet_compare_common(ppkt, spkt, network_length + ETH_HLEN)) {
> trace_colo_compare_icmp_miscompare("primary pkt size",
> ppkt->size);
> qemu_hexdump((char *)ppkt->data, stderr, "colo-compare",
> @@ -330,7 +344,7 @@ static int colo_packet_compare_other(Packet *spkt, Packet *ppkt)
> inet_ntoa(ppkt->ip->ip_dst), spkt->size,
> inet_ntoa(spkt->ip->ip_src),
> inet_ntoa(spkt->ip->ip_dst));
> - return colo_packet_compare_common(ppkt, spkt);
> + return colo_packet_compare_common(ppkt, spkt, 0);
> }
>
> static int colo_old_packet_check_one(Packet *pkt, int64_t *check_time)
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [PATCH 2/3] COLO-compare: Optimize colo_packet_compare_common
2017-02-25 7:26 ` Hailiang Zhang
@ 2017-02-27 7:06 ` Zhang Chen
0 siblings, 0 replies; 18+ messages in thread
From: Zhang Chen @ 2017-02-27 7:06 UTC (permalink / raw)
To: Hailiang Zhang, qemu-devel; +Cc: zhangchen.fnst, xuquan8
On 02/25/2017 03:26 PM, Hailiang Zhang wrote:
> On 2017/2/25 11:32, Zhang Chen wrote:
>> Add offset args for colo_packet_compare_common, optimize
>> colo_packet_compare_icmp() and colo_packet_compare_udp()
>> just compare the IP payload.
>>
>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>> ---
>> net/colo-compare.c | 28 +++++++++++++++++++++-------
>> 1 file changed, 21 insertions(+), 7 deletions(-)
>>
>> diff --git a/net/colo-compare.c b/net/colo-compare.c
>> index e75f0ae..9853232 100644
>> --- a/net/colo-compare.c
>> +++ b/net/colo-compare.c
>> @@ -180,7 +180,7 @@ static int packet_enqueue(CompareState *s, int mode)
>> * return: 0 means packet same
>> * > 0 || < 0 means packet different
>> */
>> -static int colo_packet_compare_common(Packet *ppkt, Packet *spkt)
>> +static int colo_packet_compare_common(Packet *ppkt, Packet *spkt,
>> int offset)
>> {
>> trace_colo_compare_ip_info(ppkt->size,
>> inet_ntoa(ppkt->ip->ip_src),
>> inet_ntoa(ppkt->ip->ip_dst), spkt->size,
>> @@ -188,7 +188,8 @@ static int colo_packet_compare_common(Packet
>> *ppkt, Packet *spkt)
>> inet_ntoa(spkt->ip->ip_dst));
>>
>> if (ppkt->size == spkt->size) {
>
> This check is useless, because we have done such checks in every caller.
Oh, I forgot it, not in good condition.
I will remove it in next version.
Thanks
Zhang Chen
>
>> - return memcmp(ppkt->data, spkt->data, spkt->size);
>> + return memcmp(ppkt->data + offset, spkt->data + offset,
>> + spkt->size - offset);
>> } else {
>> trace_colo_compare_main("Net packet size are not the same");
>> return -1;
>> @@ -237,8 +238,7 @@ static int colo_packet_compare_tcp(Packet *spkt,
>> Packet *ppkt)
>> spkt->ip->ip_sum = ppkt->ip->ip_sum;
>> }
>>
>> - res = memcmp(ppkt->data + ETH_HLEN, spkt->data + ETH_HLEN,
>> - (spkt->size - ETH_HLEN));
>> + res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN);
>>
>> if (res != 0 &&
>> trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
>> trace_colo_compare_pkt_info_src(inet_ntoa(ppkt->ip->ip_src),
>> @@ -277,7 +277,14 @@ static int colo_packet_compare_udp(Packet *spkt,
>> Packet *ppkt)
>> return -1;
>> }
>>
>> - ret = colo_packet_compare_common(ppkt, spkt);
>> + /*
>> + * Because of ppkt and spkt are both in the same connection,
>> + * The ppkt's src ip, dst ip, src port, dst port, ip_proto all are
>> + * same with spkt. In addition, IP header's Identification is a
>> random
>> + * field, we can handle it in IP fragmentation function later.
>> + * So we just compare the ip payload here.
>> + */
>> + ret = colo_packet_compare_common(ppkt, spkt, network_length +
>> ETH_HLEN);
>>
>> if (ret) {
>> trace_colo_compare_udp_miscompare("primary pkt size",
>> ppkt->size);
>> @@ -304,7 +311,14 @@ static int colo_packet_compare_icmp(Packet
>> *spkt, Packet *ppkt)
>> return -1;
>> }
>>
>> - if (colo_packet_compare_common(ppkt, spkt)) {
>> + /*
>> + * Because of ppkt and spkt are both in the same connection,
>> + * The ppkt's src ip, dst ip, src port, dst port, ip_proto all are
>> + * same with spkt. In addition, IP header's Identification is a
>> random
>> + * field, we can handle it in IP fragmentation function later.
>> + * So we just compare the ip payload here.
>> + */
>> + if (colo_packet_compare_common(ppkt, spkt, network_length +
>> ETH_HLEN)) {
>> trace_colo_compare_icmp_miscompare("primary pkt size",
>> ppkt->size);
>> qemu_hexdump((char *)ppkt->data, stderr, "colo-compare",
>> @@ -330,7 +344,7 @@ static int colo_packet_compare_other(Packet
>> *spkt, Packet *ppkt)
>> inet_ntoa(ppkt->ip->ip_dst), spkt->size,
>> inet_ntoa(spkt->ip->ip_src),
>> inet_ntoa(spkt->ip->ip_dst));
>> - return colo_packet_compare_common(ppkt, spkt);
>> + return colo_packet_compare_common(ppkt, spkt, 0);
>> }
>>
>> static int colo_old_packet_check_one(Packet *pkt, int64_t *check_time)
>>
>
>
>
>
> .
>
--
Thanks
Zhang Chen
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH 3/3] COLO-compare: Make qemu_hexdump() print data when trace-event ON
2017-02-25 3:32 [Qemu-devel] [PATCH 0/3] COLO-compare: Optimize the code and fix some bug Zhang Chen
` (2 preceding siblings ...)
2017-02-25 3:32 ` [Qemu-devel] [PATCH 2/3] COLO-compare: Optimize colo_packet_compare_common Zhang Chen
@ 2017-02-25 3:32 ` Zhang Chen
3 siblings, 0 replies; 18+ messages in thread
From: Zhang Chen @ 2017-02-25 3:32 UTC (permalink / raw)
To: qemu devel; +Cc: Jason Wang, Zhang Chen, Li Zhijian, eddie . dong, bian naimeng
Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
---
net/colo-compare.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/net/colo-compare.c b/net/colo-compare.c
index 9853232..5ffc4a3 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -288,9 +288,13 @@ static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt)
if (ret) {
trace_colo_compare_udp_miscompare("primary pkt size", ppkt->size);
- qemu_hexdump((char *)ppkt->data, stderr, "colo-compare", ppkt->size);
trace_colo_compare_udp_miscompare("Secondary pkt size", spkt->size);
- qemu_hexdump((char *)spkt->data, stderr, "colo-compare", spkt->size);
+ if (trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
+ qemu_hexdump((char *)ppkt->data, stderr, "colo-compare",
+ ppkt->size);
+ qemu_hexdump((char *)spkt->data, stderr, "colo-compare",
+ spkt->size);
+ }
}
return ret;
@@ -321,12 +325,14 @@ static int colo_packet_compare_icmp(Packet *spkt, Packet *ppkt)
if (colo_packet_compare_common(ppkt, spkt, network_length + ETH_HLEN)) {
trace_colo_compare_icmp_miscompare("primary pkt size",
ppkt->size);
- qemu_hexdump((char *)ppkt->data, stderr, "colo-compare",
- ppkt->size);
trace_colo_compare_icmp_miscompare("Secondary pkt size",
spkt->size);
- qemu_hexdump((char *)spkt->data, stderr, "colo-compare",
- spkt->size);
+ if (trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
+ qemu_hexdump((char *)ppkt->data, stderr, "colo-compare",
+ ppkt->size);
+ qemu_hexdump((char *)spkt->data, stderr, "colo-compare",
+ spkt->size);
+ }
return -1;
} else {
return 0;
--
2.7.4
^ permalink raw reply related [flat|nested] 18+ messages in thread