Create the necessary macros extract SIFTR data when the kenrel module is loaded and enabled. Add proper translators to the relevant D module.
Brief demo script showing the various values that can be read via the new SIFTR statically defined tracepoint (SDT).
Details
Details
Diff Detail
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Passed - Unit
No Test Coverage
Event Timeline
Comment Actions
It might make more sense to define a siftr provider rather than adding a single siftr probe to the tcp provider, especially if more siftr probes might be added later.
If you keep this probe under the tcp provider, could you also document it in dtrace-tcp.4?
| cddl/lib/libdtrace/tcp.d | ||
|---|---|---|
| 248 | The version bindings reflect the DTrace version at the time the definition was added. The current version is 1.12.1. | |
| 259 | Extra newline. | |
| 261 | Should this be struct siftrinfo? | |
| 289 | Stray space in "siftrinfo_t ;". | |
| sys/netinet/in_kdtrace.c | ||
| 116 | This was sorted by probe name. | |
| sys/netinet/in_kdtrace.h | ||
| 56 | This list is sorted by probe name. | |
| sys/netinet/siftr.c | ||
| 97 | sys/sdt.h can be grouped with the other sys/ includes above, and in_kdtrace.h can be included using the standard include path, so #include <netinet/in_kdtrace.h> should work. | |
Comment Actions
Quick comment. It only makes sense to do this in one place in siftr, so, indeed, a provider is unnecessary. I'll address the inlines in a moment
Comment Actions
I'll see about the diff2 in a minute; sorry that PB doesn't allow me to see the combined current version to only give you one feedback.
| cddl/lib/libdtrace/tcp.d | ||
|---|---|---|
| 246 | Comments end in a dot. | |
| share/dtrace/siftr | ||
| 2 | Usually we try to /*- in the first line before the Copyright to help automated tools. | |
| 29 | dot at end of sentence. | |
| 37 | I keep wondering if you want to add the word "port" to this printout? | |
| 37 | Port numbers, like most of the values following are unsigned but not printed as such. Also size does matter (for ulong/uint64). | |
| 42 | Extra space at the end? | |
| 52 | Print flags in hex with leading 0? | |
| 65 | Not sure what these are but they smell like hex to me as well? Might be worth checking and considering. | |
| share/dtrace/siftr | ||
|---|---|---|
| 37 | "The D compiler performs this processing for your printf calls automatically, so size prefixes are not required. Although size prefixes are provided for C compatibility, their use is explicitly discouraged in D programs because they bind your code to a particular data model when using derived types." | |
| 37 | I'd rather not, this is already wordy and is only to serve as an example not as the actual log replacement. | |
| 65 | For now I'll leave them as is but they can be updated later as we choose. | |