Bpftrace: Improvements on printing bitflags

Created on 8 Nov 2019  路  6Comments  路  Source: iovisor/bpftrace

I often find myself writing very repetitive custom code to format flags. For example, file permissions or open[at]() arguments.

It would be neat to have some good way to deal with these. A printf binary formatter would be an obvious answer. However, it's still annoying to have to count the bits.

One thing I thought about was adding a flags function, taking inspiration from the ls -l output:

flags("drwxrwx", 0b0100101) -> "-r--r-x"

Unfortunately, this does not really work for non-contigous flags, which are very frequent. One letter per bit might also be very limiting. It also doesn't allow you to use #included flags.

One way to get around this would be varargs:

flags("rw+c", arg2, O_RDONLY, O_WRONLY, O_RDWR, O_CREAT)

However, this will get very long very fast.
Ideally, tracepoint bitflags would be documented in the events/*/*/format files, but that info isn't there.

Hardcoding globs for #defines might also be an option: defining open: <fcntl.h> O_* might allow you to do flags("open", arg2) -> "RDONLY,CREAT"

I'm curious if there's any better ideas.

Most helpful comment

    printf("%s", args->flags & O_RDONLY ? "r" : "-");

That's going to turn into a lot of BPF instructions, plus a large perf buffer when we're only using one character. I'm tempted to come up with special syntax that can be more efficiently compiled, but then that's baking implementation details into the language abstraction -- which we should avoid.

I can imagine telling people to code this instead:

    printf("%c", args->flags & O_RDONLY ? "r" : "-");

Seems reasonable, right? And now the implementation can see it's a printf() of a single character, and then special-case it: avoiding a perf buffer and instead just passing the character value out. So I get my more efficient implementation by having a suggested best practices rather than changing the language.

All 6 comments

What's the problem with non-contiguous flags? Is it a case where something is true if, say, bits 3 and 7 are true?

Just to add my own examples for consideration; from the book:

https://github.com/brendangregg/bpf-perf-tools-book/blob/master/originals/Ch10_Networking/tcpnagle.bt :

{
    printf("Tracing TCP nagle and xmit delays. Hit Ctrl-C to end.\n");
    // from include/net/tcp.h; add more combinations if needed:
    @flags[0x0] = "ON";
    @flags[0x1] = "OFF";
    @flags[0x2] = "CORK";
    @flags[0x3] = "OFF|CORK";
    @flags[0x4] = "PUSH";
    @flags[0x5] = "OFF|PUSH";
}

and (BCC) https://github.com/brendangregg/bpf-perf-tools-book/blob/master/originals/Ch07_Memory/mmapsnoop.py :

    # map flags
    s = "S" if event.flags & MAP_SHARED else "-"
    p = "P" if event.flags & MAP_PRIVATE else "-"
    f = "F" if event.flags & MAP_FIXED else "-"
    a = "A" if event.flags & MAP_ANON else "-"
    flags = s + p + f + a

Using your first syntax and tcpnagle.bt, I could have done:

flags("pcxo", 5) -> "p-x-"

Where x=off and the rest are their first character. Not bad. (Although tcpnagle.bt is fine as-is; I guess this makes more sense with other flag sets.)

What's the problem with non-contiguous flags? Is it a case where something is true if, say, bits 3 and 7 are true?

I was concerned about the case where flags are very far apart. For example, to continue with the fcntl example, we have:

#define O_RDONLY        00
#define O_WRONLY        01
#define O_RDWR        02
[...]
# define O_ASYNC 020000

You'd have to define values for all the dozens of bits in between.

[examples]

I like your ternary + addition construction. This is what I thought of for bpftrace:

    printf("%s", args->flags & O_RDONLY ? "r" : "-");
    printf("%s", args->flags & O_WRONLY ? "w" : "-");
    printf("%s", args->flags & O_RDWR   ? "+" : "-");

I guess this makes more sense with other flag sets

I think it also makes the most sense with shorter programs. Just wanting to print some flags turning a oneliner into a 20+ line program is what made me search for better alternatives here.

    printf("%s", args->flags & O_RDONLY ? "r" : "-");

That's going to turn into a lot of BPF instructions, plus a large perf buffer when we're only using one character. I'm tempted to come up with special syntax that can be more efficiently compiled, but then that's baking implementation details into the language abstraction -- which we should avoid.

I can imagine telling people to code this instead:

    printf("%c", args->flags & O_RDONLY ? "r" : "-");

Seems reasonable, right? And now the implementation can see it's a printf() of a single character, and then special-case it: avoiding a perf buffer and instead just passing the character value out. So I get my more efficient implementation by having a suggested best practices rather than changing the language.

I just stumbled upon this problem (printing bitfields) for clone(2), which has a lot of flags. Whatever I do, this will be cumbersome. I think it would be good to mention this best-practice in the docs though.

Also, I wouldn't be able to use the "%c" optimization, as the bitfields are printed as strings, not characters. In my case:

    $flags = args->clone_flags;
    printf("%s", $flags & CLONE_CHILD_CLEARTID ? "CLONE_CHILD_CLEARTID|" : "");
    printf("%s", $flags & CLONE_CHILD_SETTID ? "CLONE_CHILD_SETTID|" : "");
    printf("%s", $flags & CLONE_FILES ? "CLONE_FILES|" : "");
    printf("%s", $flags & CLONE_FS ? "CLONE_FS|" : "");
    printf("%s", $flags & CLONE_IO ? "CLONE_IO|" : "");
    printf("%s", $flags & CLONE_NEWCGROUP ? "CLONE_NEWCGROUP|" : "");
    printf("%s", $flags & CLONE_NEWIPC ? "CLONE_NEWIPC|" : "");
    printf("%s", $flags & CLONE_NEWNET ? "CLONE_NEWNET|" : "");
    printf("%s", $flags & CLONE_NEWNS ? "CLONE_NEWNS|" : "");
    printf("%s", $flags & CLONE_NEWPID ? "CLONE_NEWPID|" : "");
    printf("%s", $flags & CLONE_NEWUSER ? "CLONE_NEWUSER|" : "");
    printf("%s", $flags & CLONE_NEWUTS ? "CLONE_NEWUTS|" : "");
    printf("%s", $flags & CLONE_PARENT ? "CLONE_PARENT|" : "");
    printf("%s", $flags & CLONE_PARENT_SETTID ? "CLONE_PARENT_SETTID|" : "");
    printf("%s", $flags & CLONE_PTRACE ? "CLONE_PTRACE|" : "");
    printf("%s", $flags & CLONE_SETTLS ? "CLONE_SETTLS|" : "");
    printf("%s", $flags & CLONE_SIGHAND ? "CLONE_SIGHAND|" : "");
    printf("%s", $flags & CLONE_SYSVSEM ? "CLONE_SYSVSEM|" : "");
    printf("%s", $flags & CLONE_THREAD ? "CLONE_THREAD|" : "");
    printf("%s", $flags & CLONE_UNTRACED ? "CLONE_UNTRACED|" : "");
    printf("%s", $flags & CLONE_VFORK ? "CLONE_VFORK|" : "");
    printf("%s", $flags & CLONE_VM ? "CLONE_VM|" : "");

In that case, from the bpfrtrace program point of view, the efficient program is

#include <linux/sched.h>

tracepoint:syscalls:sys_enter_clone {
    $flags = args->clone_flags;
    if ($flags & CLONE_CHILD_CLEARTID) { printf("CLONE_CHILD_CLEARTID|"); }
    if ($flags & CLONE_CHILD_SETTID)   { printf("CLONE_CHILD_SETTID|");   }
    if ($flags & CLONE_FILES)          { printf("CLONE_FILES|");          }
    if ($flags & CLONE_FS)             { printf("CLONE_FS|");             }
    if ($flags & CLONE_IO)             { printf("CLONE_IO|");             }
    if ($flags & CLONE_NEWCGROUP)      { printf("CLONE_NEWCGROUP|");      }
    if ($flags & CLONE_NEWIPC)         { printf("CLONE_NEWIPC|");         }
    if ($flags & CLONE_NEWNET)         { printf("CLONE_NEWNET|");         }
    if ($flags & CLONE_NEWNS)          { printf("CLONE_NEWNS|");          }
    if ($flags & CLONE_NEWPID)         { printf("CLONE_NEWPID|");         }
    if ($flags & CLONE_NEWUSER)        { printf("CLONE_NEWUSER|");        }
    if ($flags & CLONE_NEWUTS)         { printf("CLONE_NEWUTS|");         }
    if ($flags & CLONE_PARENT)         { printf("CLONE_PARENT|");         }
    if ($flags & CLONE_PARENT_SETTID)  { printf("CLONE_PARENT_SETTID|");  }
    if ($flags & CLONE_PTRACE)         { printf("CLONE_PTRACE|");         }
    if ($flags & CLONE_SETTLS)         { printf("CLONE_SETTLS|");         }
    if ($flags & CLONE_SIGHAND)        { printf("CLONE_SIGHAND|");        }
    if ($flags & CLONE_SYSVSEM)        { printf("CLONE_SYSVSEM|");        }
    if ($flags & CLONE_THREAD)         { printf("CLONE_THREAD|");         }
    if ($flags & CLONE_UNTRACED)       { printf("CLONE_UNTRACED|");       }
    if ($flags & CLONE_VFORK)          { printf("CLONE_VFORK|");          }
    if ($flags & CLONE_VM)             { printf("CLONE_VM|");             }
    printf("\n");
}

because the program does not need to construct strings (the program just send the id for printf.)
In my environment, this is 530 insns. On the otherhand, the following is 1923 insns.

#include <linux/sched.h>

tracepoint:syscalls:sys_enter_clone {
    $flags = args->clone_flags;
    printf("%s", $flags & CLONE_CHILD_CLEARTID ? "CLONE_CHILD_CLEARTID|" : "");
    printf("%s", $flags & CLONE_CHILD_SETTID ? "CLONE_CHILD_SETTID|" : "");
    printf("%s", $flags & CLONE_FILES ? "CLONE_FILES|" : "");
    printf("%s", $flags & CLONE_FS ? "CLONE_FS|" : "");
    printf("%s", $flags & CLONE_IO ? "CLONE_IO|" : "");
    printf("%s", $flags & CLONE_NEWCGROUP ? "CLONE_NEWCGROUP|" : "");
    printf("%s", $flags & CLONE_NEWIPC ? "CLONE_NEWIPC|" : "");
    printf("%s", $flags & CLONE_NEWNET ? "CLONE_NEWNET|" : "");
    printf("%s", $flags & CLONE_NEWNS ? "CLONE_NEWNS|" : "");
    printf("%s", $flags & CLONE_NEWPID ? "CLONE_NEWPID|" : "");
    printf("%s", $flags & CLONE_NEWUSER ? "CLONE_NEWUSER|" : "");
    printf("%s", $flags & CLONE_NEWUTS ? "CLONE_NEWUTS|" : "");
    printf("%s", $flags & CLONE_PARENT ? "CLONE_PARENT|" : "");
    printf("%s", $flags & CLONE_PARENT_SETTID ? "CLONE_PARENT_SETTID|" : "");
    printf("%s", $flags & CLONE_PTRACE ? "CLONE_PTRACE|" : "");
    printf("%s", $flags & CLONE_SETTLS ? "CLONE_SETTLS|" : "");
    printf("%s", $flags & CLONE_SIGHAND ? "CLONE_SIGHAND|" : "");
    printf("%s", $flags & CLONE_SYSVSEM ? "CLONE_SYSVSEM|" : "");
    printf("%s", $flags & CLONE_THREAD ? "CLONE_THREAD|" : "");
    printf("%s", $flags & CLONE_UNTRACED ? "CLONE_UNTRACED|" : "");
    printf("%s", $flags & CLONE_VFORK ? "CLONE_VFORK|" : "");
    printf("%s", $flags & CLONE_VM ? "CLONE_VM|" : "");
    printf("\n");
}

Thanks for discovering the most efficient way to write this. I did give this some thought but decided to follow Brendan's pattern as the volume is low in my case and I was just experimenting. I will use your version for the "production" version.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

brendangregg picture brendangregg  路  7Comments

Brother-Lal picture Brother-Lal  路  6Comments

danobi picture danobi  路  8Comments

shish picture shish  路  5Comments

danobi picture danobi  路  8Comments