vm: introduce ubpf_unload_code()

[iomartin]

It is currently not possible to run two different programs on the same
VM, as it will fail during the ubpf_load() of the second program.

This function allows unloading a program so that we can reuse the VM.

[coveralls]

Coverage increased (+0.3%) to 96.361% when pulling 0ba603a on iomartin:unload into 0014f29 on iovisor:master.

[sbates130272]

@iomartin this looks good. Can we add a unittest for this? Perhaps test both a failing and passing case.

[jpsamaroo]

LGTM! Just needs some tests of VM reuse.

[jpsamaroo]

Since ext_funcs and ext_func_names aren't freed, maybe name this ubpf_unload_code?

[iomartin]

Sounds good, I'll push a new version soon fixing this and adding vm reuse tests

[iomartin]

@sbates130272 @jpsamaroo I added some tests. I don't really love my approach as it pollutes vm/test.c with some extra options, but I don't see a simpler way.

[iomartin]

@sbates130272 updated

[sbates130272]

Do we have to call this unload_code when its counterpart is called load()?

[sbates130272]

@iomartin I see this was requested. Ignore this.

[jpsamaroo]

I'm not a huge fan of the unload/reload options as-is, because they don't otherwise serve a useful purpose (sometimes I use vm/test.c to actually run real BPF programs).

I feel like if we wanted to do this right, we could move everything into the getopt loop, such that as we incrementally parse the command line, we perform actions like loading, unloading, interpreting programs, JIT'ing code, etc. That would also let us execute multiple different BPF progs in succession, along with whatever actions are necessary to get them loaded and unloaded. We could eventually promote vm/test.c to an bona-fide utility exe, intended for a shell-only workflow.

[iomartin]

I agree that vm/test.c is useful to run real programs. I also think that it serves as an example for new uBPF users and as such it should be kept simple.

With the current implementation nothing changes for existing users. If you omit the -U and -R options, the program will work as it always did. And the code remains quite clear.

If we moved all the initialization to the getopt loop, though, then the code gets quite messy. Plus, it creates a lot of complications. For example: when do you execute the program? As soon as you load it? Then this would require setting memory and jit before loading. Or do we need another option to start the execution?

If there is a more complex use-case, I would say it makes more sense to just write a new program that handles it, instead of trying to shoehorn everything into the test utility program.

[iomartin]

@pchaigno @rlane thoughts? Can we merge this?

[iomartin]

@pchaigno @rlane @Alan-Jowett @jpsamaroo Any issues with this PR? I'd be happy to work on it so that we can merge it.

[sbates130272]

@pchaigno @rlane @jpsamaroo any comments? Some of our other work is blocked by this so input would be appreciated.

[iomartin]

Rebased on latest master

[rlane]

Could you describe the usecase where reusing the ubpf_vm is useful, vs creating a new one?

[niclashedam]

@rlane Efficiency and performance is a great use-case. There is some potentially costly memory allocation calls when creating a new VM. Furthermore, the VM may be stateful by having registered external functions.

[iomartin]

@rlane, the reasons are what @niclashedam described. Creating a new VM would require 1 munmap + 4 frees (for the destroy) + 3 allocs (for the new vm). By reusing a VM, we can cut that down to 1 munmap + 1 free.

And as @niclashedam said, you'd have to re-register all the external functions.

[Alan-Jowett]

Question:
Have you profiled your scenario? Are the extra alloc/frees significantly impacting your performance? I would expect the cost of the mmap/munmap to significantly outweigh the cost of the allocations and frees.

[iomartin]

The mmap/munmap will have to be done regardless of whether I create a new VM or if I reuse the existing one. The only difference between those approaches are in the number of frees and allocs (and the need to re-register external functions).

Is there something inherently wrong about reusing a VM that I am missing?

[niclashedam]

Any news on this? If not, we'd have to fork the project and continue with our own private fork.
This functionality is critical for our research and application of uBPF.

[Alan-Jowett]

LGTM.

[iomartin]

Thanks @Alan-Jowett. Can we merge this now?