libdtrace: proper decoding of tcp flags

[rscheff]

Summary

Converting the tcp header flag bitfield
would only show the first encountered flag.
Use D conditionals to convert all possible
combinations.

[markj]

I would like to find a better way to do this. :)

DTrace doesn't really have a good way to do this as far as I can tell. D intentionally has limited control flow. One approach would be to implement the %b formatter for printf(), inspired by print(9)'s implementation. Then, provide the printmask in tcp.d: inline string tcp_flags = "\20\1FIN\2SYN\3RST\4PUSH\5ACK\6URG\7ECE\10CWR\11AE", i.e., the same value as PRINT_TH_FLAGS. D printf handling is implemented in cddl/contrib/opensolaris/lib/libdtrace/common/dt_printf.c; I think it should be pretty easy to add this functionality. I don't have time at the moment but perhaps someone in DTrace might?

Other ideas?

[rscheff]

In D43263#986074, @markj wrote:

I would like to find a better way to do this. :)

DTrace doesn't really have a good way to do this as far as I can tell. D intentionally has limited control flow. One approach would be to implement the %b formatter for printf(), inspired by print(9)'s implementation. Then, provide the printmask in tcp.d: inline string tcp_flags = "\20\1FIN\2SYN\3RST\4PUSH\5ACK\6URG\7ECE\10CWR\11AE", i.e., the same value as PRINT_TH_FLAGS. D printf handling is implemented in cddl/contrib/opensolaris/lib/libdtrace/common/dt_printf.c; I think it should be pretty easy to add this functionality. I don't have time at the moment but perhaps someone in DTrace might?

Other ideas?

As long as this is a one-off, while this looks ugly (and probably excersizes the recursion of inline-conditionals), i'd suggest to keep it like this - as it is also fully backwards compatible. if string concatentation or other ways of string manipulating would be possible in D, that may be a much nicer way (but I don't know D sufficiently for knowing that).

I certainly wouldn't want to modify the d-code compiler for something like this ;)

c-pseudo code:

char[45] s
sprintf(s, "<%s%s%s...>",
flags & TH_FIN ? "FIN" : "",
((flags & TH_FIN) && (flags > TH_FIN)) ? "," : "", 
:
:
return s

Quite frankly, that the values returned in tcp->offset and tcp->flags (maybe because of the th_off/th_x2 bitfield translation) are incorrect, worries me much more - possibly there is some generic issue with accessing bitfields by dtrace... Figuring out the raw data which is handed to dtrace is tricky, but I have no doubt the (struct tcphdr) provided by the TCP_PROBE5() SDTs is correct...

[markj]

In D43263#986075, @rscheff wrote:

In D43263#986074, @markj wrote:

I would like to find a better way to do this. :)

DTrace doesn't really have a good way to do this as far as I can tell. D intentionally has limited control flow. One approach would be to implement the %b formatter for printf(), inspired by print(9)'s implementation. Then, provide the printmask in tcp.d: inline string tcp_flags = "\20\1FIN\2SYN\3RST\4PUSH\5ACK\6URG\7ECE\10CWR\11AE", i.e., the same value as PRINT_TH_FLAGS. D printf handling is implemented in cddl/contrib/opensolaris/lib/libdtrace/common/dt_printf.c; I think it should be pretty easy to add this functionality. I don't have time at the moment but perhaps someone in DTrace might?

Other ideas?

As long as this is a one-off, while this looks ugly (and probably excersizes the recursion of inline-conditionals), i'd suggest to keep it like this - as it is also fully backwards compatible.

Why worry about backward compatibility for something that's undocumented and has been broken since it was added? dtrace_tcp(4) already has an example which prints flags manually, it wasn't updated when tcpflag_string was added. Most likely, no one has really used it.

I would not be happy about committing this patch. It is unmaintainable, and tcp.d is parsed every time dtrace(1) runs.

if string concatentation or other ways of string manipulating would be possible in D, that may be a much nicer way (but I don't know D sufficiently for knowing that).

Yeah, that would be my preference as well, but that would require a fair bit of design work.

I certainly wouldn't want to modify the d-code compiler for something like this ;)

Adding a new printf formatter doesn't require modifying the compiler. All of the new functionality would be implemented in userspace.

c-pseudo code:

char[45] s
sprintf(s, "<%s%s%s...>",
flags & TH_FIN ? "FIN" : "",
((flags & TH_FIN) && (flags > TH_FIN)) ? "," : "", 
:
:
return s

Quite frankly, that the values returned in tcp->offset and tcp->flags (maybe because of the th_off/th_x2 bitfield translation) are incorrect, worries me much more - possibly there is some generic issue with accessing bitfields by dtrace... Figuring out the raw data which is handed to dtrace is tricky, but I have no doubt the (struct tcphdr) provided by the TCP_PROBE5() SDTs is correct...

Yes, this is a ctfconvert bug which I'll look at soon: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=276059