| Message ID | 20161028132559.8324-1-alex.bennee@linaro.org |
|---|---|
| State | Accepted |
| Commit | 2dfe5113b11ce0ddb08176ebb54ab7ac4104b413 |
| Headers | show |
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
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
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
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
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) ""
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(-) -- 2.10.1