* [Qemu-devel] [PATCH] net: split colo_compare_pkt_info into two trace events
@ 2016-10-28 13:25 Alex Bennée
2016-10-31 10:09 ` Peter Maydell
2016-10-31 10:13 ` Zhang Chen
0 siblings, 2 replies; 5+ messages in thread
From: Alex Bennée @ 2016-10-28 13:25 UTC (permalink / raw)
To: peter.maydell
Cc: qemu-devel, Alex Bennée, Zhang Chen, Li Zhijian, Jason Wang
It seems there is a limit to the number of arguments a UST trace event
can take and at 11 the previous trace command broke the build. Split the
trace into a src pkt and dst pkt trace to fix this.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
net/colo-compare.c | 21 +++++++++++----------
net/trace-events | 3 ++-
2 files changed, 13 insertions(+), 11 deletions(-)
diff --git a/net/colo-compare.c b/net/colo-compare.c
index f791383..4ac916a 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -218,16 +218,17 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
(spkt->size - ETH_HLEN));
if (res != 0 && trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
- trace_colo_compare_pkt_info(inet_ntoa(ppkt->ip->ip_src),
- inet_ntoa(ppkt->ip->ip_dst),
- ntohl(ptcp->th_seq),
- ntohl(ptcp->th_ack),
- ntohl(stcp->th_seq),
- ntohl(stcp->th_ack),
- res, ptcp->th_flags,
- stcp->th_flags,
- ppkt->size,
- spkt->size);
+ trace_colo_compare_pkt_info_src(inet_ntoa(ppkt->ip->ip_src),
+ ntohl(stcp->th_seq),
+ ntohl(stcp->th_ack),
+ res, stcp->th_flags,
+ spkt->size);
+
+ trace_colo_compare_pkt_info_dst(inet_ntoa(ppkt->ip->ip_dst),
+ ntohl(ptcp->th_seq),
+ ntohl(ptcp->th_ack),
+ res, ptcp->th_flags,
+ ppkt->size);
qemu_hexdump((char *)ppkt->data, stderr,
"colo-compare ppkt", ppkt->size);
diff --git a/net/trace-events b/net/trace-events
index b1913a6..35198bc 100644
--- a/net/trace-events
+++ b/net/trace-events
@@ -13,7 +13,8 @@ colo_compare_icmp_miscompare(const char *sta, int size) ": %s = %d"
colo_compare_ip_info(int psize, const char *sta, const char *stb, int ssize, const char *stc, const char *std) "ppkt size = %d, ip_src = %s, ip_dst = %s, spkt size = %d, ip_src = %s, ip_dst = %s"
colo_old_packet_check_found(int64_t old_time) "%" PRId64
colo_compare_miscompare(void) ""
-colo_compare_pkt_info(const char *src, const char *dst, uint32_t pseq, uint32_t pack, uint32_t sseq, uint32_t sack, int res, uint32_t pflag, uint32_t sflag, int psize, int ssize) "src/dst: %s/%s p: seq/ack=%u/%u s: seq/ack=%u/%u res=%d flags=%x/%x ppkt_size: %d spkt_size: %d\n"
+colo_compare_pkt_info_src(const char *src, uint32_t sseq, uint32_t sack, int res, uint32_t sflag, int ssize) "src/dst: %s s: seq/ack=%u/%u res=%d flags=%x spkt_size: %d\n"
+colo_compare_pkt_info_dst(const char *dst, uint32_t dseq, uint32_t dack, int res, uint32_t dflag, int dsize) "src/dst: %s d: seq/ack=%u/%u res=%d flags=%x dpkt_size: %d\n"
# net/filter-rewriter.c
colo_filter_rewriter_debug(void) ""
--
2.10.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] net: split colo_compare_pkt_info into two trace events
2016-10-28 13:25 [Qemu-devel] [PATCH] net: split colo_compare_pkt_info into two trace events Alex Bennée
@ 2016-10-31 10:09 ` Peter Maydell
2016-10-31 10:13 ` Zhang Chen
1 sibling, 0 replies; 5+ messages in thread
From: Peter Maydell @ 2016-10-31 10:09 UTC (permalink / raw)
To: Alex Bennée; +Cc: QEMU Developers, Zhang Chen, Li Zhijian, Jason Wang
On 28 October 2016 at 14:25, Alex Bennée <alex.bennee@linaro.org> wrote:
> It seems there is a limit to the number of arguments a UST trace event
> can take and at 11 the previous trace command broke the build. Split the
> trace into a src pkt and dst pkt trace to fix this.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Applied to master as a buildfix, thanks.
(It would probably be a good idea if the trace infrastructure
enforced this limit globally rather than resulting it it
breaking only for UST builds.)
-- PMM
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] net: split colo_compare_pkt_info into two trace events
2016-10-28 13:25 [Qemu-devel] [PATCH] net: split colo_compare_pkt_info into two trace events Alex Bennée
2016-10-31 10:09 ` Peter Maydell
@ 2016-10-31 10:13 ` Zhang Chen
2016-10-31 11:42 ` Alex Bennée
1 sibling, 1 reply; 5+ messages in thread
From: Zhang Chen @ 2016-10-31 10:13 UTC (permalink / raw)
To: Alex Bennée, peter.maydell; +Cc: qemu-devel, Li Zhijian, Jason Wang
On 10/28/2016 09:25 PM, Alex Bennée wrote:
> It seems there is a limit to the number of arguments a UST trace event
> can take and at 11 the previous trace command broke the build. Split the
> trace into a src pkt and dst pkt trace to fix this.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
It looks good for me, but it not the root cause of this bug.
We better fix this in UST trace event codes....
But qemu 2.8 will be released, we need fix this quickly.
So...
Reviewed-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
> ---
> net/colo-compare.c | 21 +++++++++++----------
> net/trace-events | 3 ++-
> 2 files changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/net/colo-compare.c b/net/colo-compare.c
> index f791383..4ac916a 100644
> --- a/net/colo-compare.c
> +++ b/net/colo-compare.c
> @@ -218,16 +218,17 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
> (spkt->size - ETH_HLEN));
>
> if (res != 0 && trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
> - trace_colo_compare_pkt_info(inet_ntoa(ppkt->ip->ip_src),
> - inet_ntoa(ppkt->ip->ip_dst),
> - ntohl(ptcp->th_seq),
> - ntohl(ptcp->th_ack),
> - ntohl(stcp->th_seq),
> - ntohl(stcp->th_ack),
> - res, ptcp->th_flags,
> - stcp->th_flags,
> - ppkt->size,
> - spkt->size);
> + trace_colo_compare_pkt_info_src(inet_ntoa(ppkt->ip->ip_src),
> + ntohl(stcp->th_seq),
> + ntohl(stcp->th_ack),
> + res, stcp->th_flags,
> + spkt->size);
> +
> + trace_colo_compare_pkt_info_dst(inet_ntoa(ppkt->ip->ip_dst),
> + ntohl(ptcp->th_seq),
> + ntohl(ptcp->th_ack),
> + res, ptcp->th_flags,
> + ppkt->size);
>
> qemu_hexdump((char *)ppkt->data, stderr,
> "colo-compare ppkt", ppkt->size);
> diff --git a/net/trace-events b/net/trace-events
> index b1913a6..35198bc 100644
> --- a/net/trace-events
> +++ b/net/trace-events
> @@ -13,7 +13,8 @@ colo_compare_icmp_miscompare(const char *sta, int size) ": %s = %d"
> colo_compare_ip_info(int psize, const char *sta, const char *stb, int ssize, const char *stc, const char *std) "ppkt size = %d, ip_src = %s, ip_dst = %s, spkt size = %d, ip_src = %s, ip_dst = %s"
> colo_old_packet_check_found(int64_t old_time) "%" PRId64
> colo_compare_miscompare(void) ""
> -colo_compare_pkt_info(const char *src, const char *dst, uint32_t pseq, uint32_t pack, uint32_t sseq, uint32_t sack, int res, uint32_t pflag, uint32_t sflag, int psize, int ssize) "src/dst: %s/%s p: seq/ack=%u/%u s: seq/ack=%u/%u res=%d flags=%x/%x ppkt_size: %d spkt_size: %d\n"
> +colo_compare_pkt_info_src(const char *src, uint32_t sseq, uint32_t sack, int res, uint32_t sflag, int ssize) "src/dst: %s s: seq/ack=%u/%u res=%d flags=%x spkt_size: %d\n"
> +colo_compare_pkt_info_dst(const char *dst, uint32_t dseq, uint32_t dack, int res, uint32_t dflag, int dsize) "src/dst: %s d: seq/ack=%u/%u res=%d flags=%x dpkt_size: %d\n"
>
> # net/filter-rewriter.c
> colo_filter_rewriter_debug(void) ""
--
Thanks
zhangchen
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] net: split colo_compare_pkt_info into two trace events
2016-10-31 10:13 ` Zhang Chen
@ 2016-10-31 11:42 ` Alex Bennée
2016-10-31 20:15 ` Eric Blake
0 siblings, 1 reply; 5+ messages in thread
From: Alex Bennée @ 2016-10-31 11:42 UTC (permalink / raw)
To: Zhang Chen; +Cc: peter.maydell, qemu-devel, Li Zhijian, Jason Wang
Zhang Chen <zhangchen.fnst@cn.fujitsu.com> writes:
> On 10/28/2016 09:25 PM, Alex Bennée wrote:
>> It seems there is a limit to the number of arguments a UST trace event
>> can take and at 11 the previous trace command broke the build. Split the
>> trace into a src pkt and dst pkt trace to fix this.
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> It looks good for me, but it not the root cause of this bug.
> We better fix this in UST trace event codes....
I didn't get a chance to dig into the details but yes we need to confirm
if this is a limitation with UST or just the macro headers we generate
for it. That said this is the first time I think we have exceeded 10
parameters for a trace event.
> But qemu 2.8 will be released, we need fix this quickly.
> So...
> Reviewed-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>
>
>> ---
>> net/colo-compare.c | 21 +++++++++++----------
>> net/trace-events | 3 ++-
>> 2 files changed, 13 insertions(+), 11 deletions(-)
>>
>> diff --git a/net/colo-compare.c b/net/colo-compare.c
>> index f791383..4ac916a 100644
>> --- a/net/colo-compare.c
>> +++ b/net/colo-compare.c
>> @@ -218,16 +218,17 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
>> (spkt->size - ETH_HLEN));
>>
>> if (res != 0 && trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
>> - trace_colo_compare_pkt_info(inet_ntoa(ppkt->ip->ip_src),
>> - inet_ntoa(ppkt->ip->ip_dst),
>> - ntohl(ptcp->th_seq),
>> - ntohl(ptcp->th_ack),
>> - ntohl(stcp->th_seq),
>> - ntohl(stcp->th_ack),
>> - res, ptcp->th_flags,
>> - stcp->th_flags,
>> - ppkt->size,
>> - spkt->size);
>> + trace_colo_compare_pkt_info_src(inet_ntoa(ppkt->ip->ip_src),
>> + ntohl(stcp->th_seq),
>> + ntohl(stcp->th_ack),
>> + res, stcp->th_flags,
>> + spkt->size);
>> +
>> + trace_colo_compare_pkt_info_dst(inet_ntoa(ppkt->ip->ip_dst),
>> + ntohl(ptcp->th_seq),
>> + ntohl(ptcp->th_ack),
>> + res, ptcp->th_flags,
>> + ppkt->size);
>>
>> qemu_hexdump((char *)ppkt->data, stderr,
>> "colo-compare ppkt", ppkt->size);
>> diff --git a/net/trace-events b/net/trace-events
>> index b1913a6..35198bc 100644
>> --- a/net/trace-events
>> +++ b/net/trace-events
>> @@ -13,7 +13,8 @@ colo_compare_icmp_miscompare(const char *sta, int size) ": %s = %d"
>> colo_compare_ip_info(int psize, const char *sta, const char *stb, int ssize, const char *stc, const char *std) "ppkt size = %d, ip_src = %s, ip_dst = %s, spkt size = %d, ip_src = %s, ip_dst = %s"
>> colo_old_packet_check_found(int64_t old_time) "%" PRId64
>> colo_compare_miscompare(void) ""
>> -colo_compare_pkt_info(const char *src, const char *dst, uint32_t pseq, uint32_t pack, uint32_t sseq, uint32_t sack, int res, uint32_t pflag, uint32_t sflag, int psize, int ssize) "src/dst: %s/%s p: seq/ack=%u/%u s: seq/ack=%u/%u res=%d flags=%x/%x ppkt_size: %d spkt_size: %d\n"
>> +colo_compare_pkt_info_src(const char *src, uint32_t sseq, uint32_t sack, int res, uint32_t sflag, int ssize) "src/dst: %s s: seq/ack=%u/%u res=%d flags=%x spkt_size: %d\n"
>> +colo_compare_pkt_info_dst(const char *dst, uint32_t dseq, uint32_t dack, int res, uint32_t dflag, int dsize) "src/dst: %s d: seq/ack=%u/%u res=%d flags=%x dpkt_size: %d\n"
>>
>> # net/filter-rewriter.c
>> colo_filter_rewriter_debug(void) ""
--
Alex Bennée
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] net: split colo_compare_pkt_info into two trace events
2016-10-31 11:42 ` Alex Bennée
@ 2016-10-31 20:15 ` Eric Blake
0 siblings, 0 replies; 5+ messages in thread
From: Eric Blake @ 2016-10-31 20:15 UTC (permalink / raw)
To: Alex Bennée, Zhang Chen
Cc: peter.maydell, Jason Wang, qemu-devel, Li Zhijian,
Stefan Hajnoczi
[-- Attachment #1: Type: text/plain, Size: 1172 bytes --]
On 10/31/2016 06:42 AM, Alex Bennée wrote:
>
> Zhang Chen <zhangchen.fnst@cn.fujitsu.com> writes:
>> It looks good for me, but it not the root cause of this bug.
>> We better fix this in UST trace event codes....
>
> I didn't get a chance to dig into the details but yes we need to confirm
> if this is a limitation with UST or just the macro headers we generate
> for it. That said this is the first time I think we have exceeded 10
> parameters for a trace event.
Not the first time; see commit defbaec back in June.
The limit appears to be inherent in UST:
For more information see comment regarding TP_ARGS
in lttng/tracepoint.h:
/*
* TP_ARGS takes tuples of type, argument separated by a comma.
* It can take up to 10 tuples (which means that less than 10 tuples is
* fine too).
* Each tuple is also separated by a comma.
*/
But I agree that fixing the trace generation code to hard-fail on 11
arguments even when UST is not the active trace engine would be a nice
service to developers.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-10-31 20:16 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-10-28 13:25 [Qemu-devel] [PATCH] net: split colo_compare_pkt_info into two trace events Alex Bennée
2016-10-31 10:09 ` Peter Maydell
2016-10-31 10:13 ` Zhang Chen
2016-10-31 11:42 ` Alex Bennée
2016-10-31 20:15 ` Eric Blake
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).