Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lua Tools for BCC #457

Merged
merged 19 commits into from
Mar 30, 2016
Merged

Lua Tools for BCC #457

merged 19 commits into from
Mar 30, 2016

Conversation

vmg
Copy link
Contributor

@vmg vmg commented Mar 29, 2016

As promised in #455.

This is a code dump of a Lua(JIT) wrapper that we've developed internally at GitHub. Some notes:

  1. The code is Apache 2 licensed, like the rest of BCC.
  2. The code contains a few third party helpers to make writing Lua probes easier. All the dependencies are MIT licensed and 100% compatible with the rest of the wrapper. All due attribution has been added to the corresponding files.
  3. The wrapper has been put in src/lua, so it sits next to the src/python wrapper and the actual implementation.
  4. Some of the existing Python tools have been rewritten to Lua as a sample of how to use the new APIs. They can be found under examples/lua.
  5. The best place to learn how to write a probe using these bindings is memleak.lua. It's a full port of @brendangregg's memleak.py and should have identical behavior.

I'm aware this is a lot of code to go through. I'm not sure what else can I do to help you approve the binding and get it merged on the tree (or whether you trust my dubious Lua expertise, hehe), but I'm here to answer all your questions.

Thanks!

print("%-9s %-6d %s" % {os.date("%H:%M:%S"), tonumber(event.pid), ffi.string(event.str)})
end

b:get_table("events"):open_perf_buffer(print_readline, "struct { uint64_t pid; char str[80]; }")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is (obviously) a port of the existing bashreadline.py, and I'm particularly happy with it. Very little code, and it shows how you can pass a literal C definition for the structure we're getting out of the perf_buffer, and have the binding JIT compile it to native code, so the callback receives an event struct instead of a void pointer.

I think it's a clear win over the Python API!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks great. was worried that this struct {pid, char str[80];} copy-paste will be an issue, but looks like other examples get fields into lua dynamically.

@drzaeus77
Copy link
Collaborator

Will look through this in more detail, but first: where are the test(s)? :) Would be good to have at least one with the initial checkin to test the sanity of things like dependencies, file placement, etc.

@vmg
Copy link
Contributor Author

vmg commented Mar 29, 2016

Good point, hehe. I'm afraid most of the tests are ad-hoc scripts, so I guess my next thing in the TODO list is writing a more-or-less comprehensive suite in Lua that we can put on this PR. I'll get to it sometime this week.

@brendangregg
Copy link
Member

Amazing work! bashreadline does make for a nice short example. BTW memleak.py is actually Sasha's, not mine.

@drzaeus77
Copy link
Collaborator

First shot at running the example didn't work following the README. I'm sure I'll be able to get it working but probably some updates to the doc will be needed.

Running from src/lua:

$ sudo ./bcc-probe ../../examples/lua/task_switch.lua 
luajit: ./bcc-probe:31: module '../../examples/lua/task_switch' not found:
        no field package.preload['../../examples/lua/task_switch']
        no file './//////examples/lua/task_switch/init.lua'
        no file './//////examples/lua/task_switch.lua'
        no file '/usr/share/luajit-2.0.4///////examples/lua/task_switch.lua'
        no file '/usr/local/share/lua/5.1///////examples/lua/task_switch.lua'
        no file '/usr/local/share/lua/5.1///////examples/lua/task_switch/init.lua'
        no file '/usr/share/lua/5.1///////examples/lua/task_switch.lua'
        no file '/usr/share/lua/5.1///////examples/lua/task_switch/init.lua'
        no file './//////examples/lua/task_switch.so'
        no file '/usr/local/lib/lua/5.1///////examples/lua/task_switch.so'
        no file '/usr/lib/lua/5.1///////examples/lua/task_switch.so'
        no file '/usr/local/lib/lua/5.1/loadall.so'
        no file './.so'
        no file '/usr/local/lib/lua/5.1/.so'
        no file '/usr/lib/lua/5.1/.so'
        no file '/usr/local/lib/lua/5.1/loadall.so'
stack traceback:
        [C]: in function 'require'
        ./bcc-probe:31: in main chunk
        [C]: at 0x00404750

and switching to the examples directory:

$ sudo ../../src/lua/bcc-probe bashreadline.lua 
luajit: ../../src/lua/bcc-probe:19: module 'bcc' not found:
        no field package.preload['bcc']
        no file './bcc/init.lua'
        no file './bcc.lua'
        no file '/usr/share/luajit-2.0.4/bcc.lua'
        no file '/usr/local/share/lua/5.1/bcc.lua'
        no file '/usr/local/share/lua/5.1/bcc/init.lua'
        no file '/usr/share/lua/5.1/bcc.lua'
        no file '/usr/share/lua/5.1/bcc/init.lua'
        no file './bcc.so'
        no file '/usr/local/lib/lua/5.1/bcc.so'
        no file '/usr/lib/lua/5.1/bcc.so'
        no file '/usr/local/lib/lua/5.1/loadall.so'
stack traceback:
        [C]: in function 'require'
        ../../src/lua/bcc-probe:19: in main chunk
        [C]: at 0x00404750

@vmg
Copy link
Contributor Author

vmg commented Mar 29, 2016

@drzaeus77: Actually, there's an issue with the examples because the bcc-probe loader is not prepared to load relative paths. 😅

Let me fix this for you. :)

@vmg
Copy link
Contributor Author

vmg commented Mar 29, 2016

@drzaeus77: Fixed. Sorry for wasting your time. Our original deployment had our custom tools as modules. You should now be able to call bcc-probe from src/lua to run probes regardless of location.

Gonna get started with these tests because they're obviously necessary.

@vmg
Copy link
Contributor Author

vmg commented Mar 29, 2016

Amazing work! bashreadline does make for a nice short example. BTW memleak.py is actually Sasha's, not mine.

Thank you! I'd swear I saw memleak on one of your slides. I guess you were just using it an example. Sorry Sasha! :)

@goldshtn
Copy link
Collaborator

This is very impressive. I am barely conversational in Lua, but I can definitely understand the desire to use a JIT-compiled language. I wonder how we are going to move forward, though, with shared functionality such as the USDT* and Tracepoint classes. Is there a way for users of one language to benefit from shared code developed in the other language?

n = n + 1
end

assert(n == kprobe_count, "wtf")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

meaningful error please

@4ast
Copy link
Member

4ast commented Mar 30, 2016

overall looks great. Would be even more awesome to have a static tool that can be built out of luajit-devel and libbcc libraries. Installing libbcc.so and python often gets in the way in production systems. If we can have standalone binary (with only external dependency on kernel headers) then it will go long way.
Please add the tests, so buildbots can make sure that stuff still works.

local ffi = require("ffi")

return function(BPF)
local b = BPF:new{src_file="bashreadline.c", debug=true}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two things:

  • We've found that inlining the C source contents (which you do in memleak.lua) is more portable. I would suggest adding relative path support or inlining everything, since a user needs to be in the same directory to be able to find bashreadline.c here.
  • debug is not a bool, but rather a set of flags. See Minor endian and debug enhancement #456

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking aloud here...if we do truly share the .c file implementation, then duplicating that code in the different bindings could become a bad thing. We should weigh that duplication against tool portability.

Possible solutions to a problem that only just barely exists yet: cmake prepocessing, .c implementation moving to libbcc, git precommit hook to detect out-of-sync files, etc.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found the section on relative include later on in the code, so this is partially answered. If anyone else thinks we should worry about code duplication, let's discuss.

@vmg
Copy link
Contributor Author

vmg commented Mar 30, 2016

Hey all! I've pushed a small batch of tests (ported straight from the Python counterparts) and implemented a bunch of features which were missing to pass those tests. :)

Answering your questions now:

This is very impressive. I am barely conversational in Lua, but I can definitely understand the desire to use a JIT-compiled language. I wonder how we are going to move forward, though, with shared functionality such as the USDT* and Tracepoint classes. Is there a way for users of one language to benefit from shared code developed in the other language?

Thank you! 😍

I'm afraid I don't have a good answer regarding code reuse -- beside the obvious that really important features should be written in C/C++ and available in libbcc. There is no way to automagically reuse Python classes from Lua. I think that's OK. Maybe I've spent too much of my career writing cross-language bindings, but I feel like as long as they're (mostly) thin wrappers over the libbcc code, having diverging and idiomatic implementations is a healthy thing. It keeps the API design in check!

btw, python has the same simple approach to trace_pipe, but it can be used by multiple processes/tracers and can be in binary mode, so would be good to add a check that /sys/kernel/debug/tracing/trace_options has noraw, nobin and context-info set, otherwise regex from line 19 will be confused

Thanks for the pointer. I'll try to get this right in both Python and Lua tomorrow. 👌

Would be even more awesome to have a static tool that can be built out of luajit-devel and libbcc libraries

Totally doable. I've done this plenty of times before for deployments. The resulting binary is rather fat but only depends on libc, so it should be drag-and-droppable anywhere. All it takes is a simple C wrapper, and a small script to compile the Lua bytecode for the wrapper and stick it on the data section of the binary, and a Makefile rule to package it up. Quick and painless!

...Except I gotta port the Makefile rule to CMake. [sweats profusely]

Please add the tests, so buildbots can make sure that stuff still works.

Tests are in place. Now I just gotta learn how to wire them up so they run on CI. Let me tinker with this...

@vmg
Copy link
Contributor Author

vmg commented Mar 30, 2016

OK, I've added the tests to CMake, but they will not run unless luajit can be found on the path.

I'm afraid I don't have access to the buildbot to install luajit. Could anybody take care of that for me? :)

@@ -0,0 +1,6 @@
find_program(LUAJIT luajit)

if(LUAJIT)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@drzaeus77 can you install luajit on buildbots and re-trigger ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

installed on ubuntu:

Setting up libluajit-5.1-common (2.0.2+dfsg-1) ...
Setting up luajit (2.0.2+dfsg-1) ...

installed on fedora:

luajit.x86_64 2.0.3-4.fc22

@4ast
Copy link
Member

4ast commented Mar 30, 2016

@vmg
20/20 Test #20: Unable to find executable: sudo
lua_test_clang ...................***Not Run 0.00 sec

@drzaeus77
Copy link
Collaborator

FYI, sudo is definitely installed on the box, we use and depend on it in
other tests.

On Wed, Mar 30, 2016 at 10:48 AM, 4ast notifications@github.com wrote:

@vmg https://github.com/vmg
20/20 Test #20 #20: Unable to find
executable: sudo
lua_test_clang ...................***Not Run 0.00 sec


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#457 (comment)

@vmg
Copy link
Contributor Author

vmg commented Mar 30, 2016

💥 Green in Ubuntu and Fedora. I've also added some uprobe and dumping tests.

I'm working on a standalone build for the lua tools. It's coming along nicely. Anything else you'd like to see from this first PR before merging? :)

@4ast
Copy link
Member

4ast commented Mar 30, 2016

the last two commits could have been squashed, but ok for the history

@4ast 4ast merged commit a480304 into iovisor:master Mar 30, 2016
@vmg
Copy link
Contributor Author

vmg commented Mar 30, 2016

🎉 🎉 🎉

Exciting! Thanks for the feedback and support!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants