Skip to content

Commit

Permalink
allow packed structure in bpf program in python API (iovisor#2020)
Browse files Browse the repository at this point in the history
Fix issue iovisor#2017.

For python programs, the map data passed from C++ library
is parsed through the key/value desc types which are
generated by C++ json map declaration visitor.

The map declaration visitor visits the map key/value
declaration types and generate a string to represent
the type, which is later used by python to reconstruct
the cttype.

The map declaration already tries to add all the padding
to the type string in order to make sure C++ and python
see the same layout.

This patch further added packed support such that if
C++ json map visitor has applied padding, the python
type reconstructor is free to add _pack_=1 for structure
type since the structure is already packed.

For example, for a type,
  struct t { char a; int b; }
the structure after json visitor will look like
  struct t { char a; char __pad[3]; int b; }

If the type is
  struct t { char a; int b; } __packed;
the structure after json visitor will look like
  struct t { char a; int b; }

In either case, it will be okay to add __packed attribute
for the type generated by json map visitor in order to
match the original declaration.

Thanks Chaitanya for filing the issue and providing the test case!

Signed-off-by: Yonghong Song <[email protected]>
  • Loading branch information
yonghong-song committed Oct 30, 2018
1 parent b81dcb7 commit 9410c86
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 7 deletions.
8 changes: 6 additions & 2 deletions src/cc/json_map_decl_visitor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,12 @@ bool BMapDeclVisitor::VisitRecordDecl(RecordDecl *D) {
result_ += "]";
if (D->isUnion())
result_ += ", \"union\"";
else if (D->isStruct())
result_ += ", \"struct\"";
else if (D->isStruct()) {
if (SkipPadding)
result_ += ", \"struct\"";
else
result_ += ", \"struct_packed\"";
}
result_ += "]";
return true;
}
Expand Down
9 changes: 7 additions & 2 deletions src/lua/bcc/table.lua
Original file line number Diff line number Diff line change
Expand Up @@ -349,8 +349,13 @@ local function _decode_table_type(desc)
table.insert(fields, f)
end

assert(struct == "struct" or struct == "union", "unknown complex type: "..struct)
return string.format("%s { %s }", struct, table.concat(fields, " "))
assert(struct == "struct" or struct == "struct_packed" or struct == "union",
"unknown complex type: "..struct)
if struct == "union" then
return string.format("union { %s }", table.concat(fields, " "))
else
return string.format("struct { %s }", table.concat(fields, " "))
end
end
return _dec(json.parse(json_desc))
end
Expand Down
15 changes: 12 additions & 3 deletions src/python/bcc/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,8 @@ def _decode_table_type(desc):
elif isinstance(t[2], int):
fields.append((t[0], BPF._decode_table_type(t[1]), t[2]))
elif isinstance(t[2], basestring) and (
t[2] == u"union" or t[2] == u"struct"):
t[2] == u"union" or t[2] == u"struct" or
t[2] == u"struct_packed"):
name = t[0]
if name == "":
name = "__anon%d" % len(anon)
Expand All @@ -438,13 +439,21 @@ def _decode_table_type(desc):
else:
raise Exception("Failed to decode type %s" % str(t))
base = ct.Structure
is_packed = False
if len(desc) > 2:
if desc[2] == u"union":
base = ct.Union
elif desc[2] == u"struct":
base = ct.Structure
cls = type(str(desc[0]), (base,), dict(_anonymous_=anon,
_fields_=fields))
elif desc[2] == u"struct_packed":
base = ct.Structure
is_packed = True
if is_packed:
cls = type(str(desc[0]), (base,), dict(_anonymous_=anon, _pack_=1,
_fields_=fields))
else:
cls = type(str(desc[0]), (base,), dict(_anonymous_=anon,
_fields_=fields))
return cls

def get_table(self, name, keytype=None, leaftype=None, reducer=None):
Expand Down
25 changes: 25 additions & 0 deletions tests/python/test_clang.py
Original file line number Diff line number Diff line change
Expand Up @@ -1225,5 +1225,30 @@ def test_arbitrary_increment_simple(self):
b.attach_kprobe(event=b"htab_map_delete_elem", fn_name=b"map_delete")
b.cleanup()

@skipUnless(kernel_version_ge(4,7), "requires kernel >= 4.7")
def test_packed_structure(self):
b = BPF(text=b"""
struct test {
u16 a;
u32 b;
} __packed;
BPF_TABLE("hash", u32, struct test, testing, 2);
TRACEPOINT_PROBE(kmem, kmalloc) {
u32 key = 0;
struct test info, *entry;
entry = testing.lookup(&key);
if (entry == NULL) {
info.a = 10;
info.b = 20;
testing.update(&key, &info);
}
return 0;
}
""")
if len(b["testing"].items()):
st = b["testing"][ct.c_uint(0)]
self.assertEqual(st.a, 10)
self.assertEqual(st.b, 20)

if __name__ == "__main__":
main()

0 comments on commit 9410c86

Please sign in to comment.