print_log2_hist(): check and skip possible paddings

[boat0]

Issue #2154

When a struct S is used as key to a BPF_HISTOGRAM, it is assumed that the second
member of S holds the slot. But when S is converted to python from bpf C,
a padding may be inserted as a second member. This breaks print_log2_hist().

    root@debian:~/bcc/tools# ./softirqs.py -d
    Tracing soft irq event time... Hit Ctrl-C to end.
    ^C
    Traceback (most recent call last):
      File "./softirqs.py", line 144, in <module>
        dist.print_log2_hist(label, "softirq", section_print_fn=vec_to_name)
      File "/usr/local/lib/python2.7/dist-packages/bcc/table.py", line 326, in print_log2_hist
        vals[slot] = v.value
    TypeError: list indices must be integers, not str

Fix it by skipping the possible padding. Future work would be fixing/working
around in the library where the padding is introduced.

[yonghong-song]

[buildbot, test this please]

[jeromemarchand]

Shouldn't be this issue still open given the patch is explicitely a temporary one?
This patch seems very unreliable to me. I'm not going to dive into C standards, but for instance, what guaranty do we have that the padding field is always going to be called '__pad_1'?

Before we come to a proper solution, the format of the key structure should be clearly defined. I couldn't find anything about that in the reference guide. For instance, if the slot field was to be always named 'slot', which seems to be already the case in the tools and examples, then print_log2_hist() could just look for that field.

[jeromemarchand]

Fix it by skipping the possible padding. Future work would be fixing/working
around in the library where the padding is introduced.

The padding is added by the compiler for alignment reason. There is nothing to fix there.

[boat0]

This patch seems very unreliable to me. I'm not going to dive into C standards, but for instance, what guaranty do we have that the padding field is always going to be called '__pad_1'?

bcc/src/cc/json_map_decl_visitor.cc

Line 142 in 51d62d3

result_ += "[\"__pad_" + to_string(F->getFieldIndex()) + "\",\"char\",["

Before we come to a proper solution, the format of the key structure should be clearly defined. I couldn't find anything about that in the reference guide.

I believe help to complete the docs is always welcome :)

[boat0]

Fix it by skipping the possible padding. Future work would be fixing/working
around in the library where the padding is introduced.

The padding is added by the compiler for alignment reason. There is nothing to fix there.

As a matter of fact, the padding is added by libbcc, the C part of BCC itself.

[yonghong-song]

I agree. Do you want to add some documentation about what C=>python key/value structure padding? Although most people should not care about this, but such information is still useful if users reply on particular fields and padding may change their relative position in the structure.

[jeromemarchand]

As a matter of fact, the padding is added by libbcc, the C part of BCC itself.

OK, my bad!

I agree. Do you want to add some documentation about what C=>python key/value structure padding? Although most people should not care about this, but such information is still useful if users reply on particular fields and padding may change their relative position in the structure.

That might be useful too, but I was thinking about documenting the format of the key_type of an histogram when said type is a structure.

[yonghong-song]

That might be useful too, but I was thinking about documenting the format of the key_type of an histogram when said type is a structure.

This is definitely needed. Thanks!