Dumping BPF map files (Issue #777) #1044

abhishekvijeev · 2020-04-25T07:55:38Z

Hi,

I have implemented functionality to dump information about BPF map files and also created a template ZDTM static test file for BPF maps. The test currently fails because restore functionality has not yet been implemented. I am currently working on this.

My first goal is to get checkpoint and restore fully working for simple BPF map files, after which I will enhance CRIU to deal with other kinds of BPF files. Kindly let me know whether you would prefer me to prioritize something else.

I await your kind feedback.

Thank you.

Snorch · 2020-04-29T10:22:08Z

Hi @abhishekvijeev!

Let's start with small nits on how you do a pr to criu:

Please add a Siged-off-by line in each commit. (upd: please see https://criu.org/How_to_submit_patches#Sign_your_work)
Each commit should have commit message formated as:

component1[/component2/...]: short commit summary

longer commit description which describes what, why and how exectly you do

Signed-off-by: Name Surname <e-mail>

We want to have pr's containing commits with clean code only, you need to squash all fixup commit to their base and force push the pr. (I mean "Code formatting" commit is a bad practice)
Another thing all related travis tests should pass, it looks like you have some of them failing:

===================== Run zdtm/static/bpf_create_map in h ======================
Start test
./bpf_create_map --pidfile=bpf_create_map.pid --outfile=bpf_create_map.out
Run criu dump
=[log]=> dump/zdtm/static/bpf_create_map/47/1/dump.log
------------------------ grep Error ------------------------
(00.003791) 47 fdinfo 2: pos:              0xe flags:           102001/0
(00.003812) 47 fdinfo 3: pos:                0 flags:           100000/0
(00.003822) Dumping path for 3 fd via self 18 [/home/travis/build/checkpoint-restore/criu/test/zdtm/static]
(00.003841) 47 fdinfo 4: pos:                0 flags:                2/0x1
(00.003868) Error (criu/proc_parse.c:1749): No data left in proc file while parsing bpfmap
(00.003879) Error (criu/proc_parse.c:2114): parse_fdinfo_pid_s: error parsing [map_type:	3] for 19: Success
(00.003887) ----------------------------------------
(00.003898) Error (criu/cr-dump.c:1349): Dump files (pid: 47) failed with -1
(00.004127) 47 was stopped
(00.004215) Unlock network
(00.004220) Unfreezing tasks into 1
(00.004223) 	Unseizing 47 into 1
(00.004235) Error (criu/cr-dump.c:1775): Dumping FAILED.
------------------------ ERROR OVER ------------------------
############## Test zdtm/static/bpf_create_map FAIL at CRIU dump ###############
Send the 9 signal to  47
Wait for zdtm/static/bpf_create_map(47) to die for 0.100000

abhishekvijeev · 2020-04-30T15:49:54Z

Hi @Snorch ,

Thank you very much for the feedback.

I have made the necessary changes to the commits (will keep this in mind henceforth). Kindly let me know if I can explain any of the commit messages more clearly.

Also, the reason that majority of tests were failing was because I had implemented incomplete boiler-plate code for testing c/r of BPF maps. I have removed the test from the PR for now, and will add it once it is complete (I have described below my thoughts on creating tests for BPF maps).
Therefore, these tests pass successfully now. However, 3 of the tests (#4568.13, #4568.14 and 4568.20) for the ARM platform fail with the following error:

criu/proc_parse.c: In function 'parse_bpfmap':
criu/proc_parse.c:1724:32: error: format '%lu' expects argument of type 'long unsigned int *', but argument 3 has type 'uint64_t *' {aka 'long long unsigned int *'} [-Werror=format=]
if (sscanf(str, "map_flags: %lu", &bpf->map_flags) != 1)
~~^ ~~~~~~~~~~~~~~~
%llu
criu/proc_parse.c:1730:30: error: format '%lu' expects argument of type 'long unsigned int *', but argument 3 has type 'uint64_t *' {aka 'long long unsigned int *'} [-Werror=format=]
if (sscanf(str, "memlock: %lu", &bpf->memlock) != 1)
~~^ ~~~~~~~~~~~~~
%llu

I tried changing the format specifier to %llu, but the code didn't compile on my PC (x86_64). Does this test fail because compilers view the data type 'uint64_t' differently for ARM64 and x86_64? If this is the case, how should I resolve this issue?

Currently, this PR accomplishes 2 tasks:
a) Checkpoint - read information about BPF maps from the proc filesystem and dump this into image files
b) Restore - read information back from the image files and create new BPF map files with the same parameters preserved during checkpointing

However, a significant part of the problem remains unsolved. BPF map files typically contain data in the form of key value pairs. CRIU must therefore checkpoint and restore this data as well. According to my current understanding, the data contained within BPF maps is not made available via the proc
filesystem. Therefore, the way I'm thinking of accomplishing this is by iterating through all the key-value pairs stored in a map when it is about to be checkpointed, and saving them in a new image (similar to what is currently done for pipes' data). Kindly let me know your thoughts on this.

Also, BPF maps can be persisted on /sys/fs/bpf/ using the BPF_PIN_FD command argument. In this case, during restoration, instead of creating a new map with the same parameters (the way this PR currently works), CRIU must be able to open the map on the sys filesystem. This is another problem I will solve.

Once I have accomplished this, I will create 2 ZDTM tests that accomplish the following:

a) - Checkpoint a process that has a BPF map with data
- Restore the BPF map and its data
- Check whether the restored map's parameters and data are the same as before checkpointing

b) - Checkpoint a process that has a BPF map persisted on sysfs
- Restore the BPF map and its data
- Check whether the map is the same as the one persisted on sysfs

Thank you,
Abhishek

Snorch · 2020-05-01T12:32:58Z

how should I resolve this issue?

PRIu64 should help to print unit64_t https://en.cppreference.com/w/c/types/integer

iterating through all the key-value pairs stored in a map when it is about to be checkpointed, and saving them in a new image

You may also consider protobuf repeated key-value pair entry.

Thanks,
Pavel.

abhishekvijeev · 2020-05-01T13:50:52Z

Thanks!

avagin · 2020-05-08T07:50:24Z

Could you rebase this PR?

abhishekvijeev · 2020-05-08T12:30:41Z

Sure @avagin , it's done.

abhishekvijeev · 2020-05-08T15:41:13Z

I see that the following RPC test is failing:
https://travis-ci.org/github/checkpoint-restore/criu/jobs/684654477#L17709

I tried reproducing the error on my PC by executing the following command:
"make -C test/others/rpc/ run"

'test-c' fails with the following output (obtained from test/others/rpc/build/output_c):

"build/imgs_c
Fail
build/imgs_c
Fail
Cant connect to socket: Connection refused
build/imgs_c
build/imgs_c
Fail
build/imgs_c
Fail
build/imgs_c
Fail
build/imgs_c
Fail
build/imgs_c
Fail
build/imgs_c
Success
build/imgs_c
Success
Restored"

The socket connection was failing. I therefore re-ran the test as the root user, and it passed. Am I correct in assuming that instructing Travis to execute the test as root user is the correct solution to this problem? If so, how can I ensure that Travis passes it?

Thank you.

avagin · 2020-05-13T16:08:17Z

Could you push the test too? It will help to review these changes.

abhishekvijeev · 2020-05-13T18:10:37Z

The template test I had created earlier for BPF maps?

abhishekvijeev · 2020-05-19T14:52:10Z

Hi @Snorch @avagin ,

I don't think it's possible to checkpoint a BPF map file's data, as Linux's BPF system call API doesn't provide a means to duplicate BPF map data (like 'tee' for pipes). I have explained the problem in this email:
https://lists.openvz.org/pipermail/criu/2020-May/044995.html

It's meaningless to continue work on BPF map files (#777) unless Linux's BPF API evolves to support dumping map data. I mean, I don't see the point in only reading info from the proc filesystem and restoring an identical map without its data. Do let me know if you think otherwise (i.e if you think there is value in only dumping and restoring a map without its data). Else, I shall close this PR.

Thank you.

abhishekvijeev · 2020-06-02T07:24:15Z

Hi @Snorch @avagin ,

Could you kindly let me know whether I am correct in assuming that I've hit a dead end and if so, can I close this PR?

Thank you.

avagin · 2020-06-08T18:58:46Z

@abhishekvijeev This is not a dead end, you hit a new challenge of extending linux API-s. While we were developing CRIU, we met similar problems a few time. Now, you need to discuss a new API for dumping BPF files with the kernel community and implement it.

abhishekvijeev · 2020-06-09T07:36:31Z

Thank you for the response @avagin
Sure, I'll get to work on this.

abhishekvijeev · 2020-08-17T17:31:37Z

Hi,

Travis tests pass now. The Fedora rawhide tests fail, but I see that this has been occurring across CRIU builds over that past couple of days - I therefore assume that this is a temporary problem independent of this PR.

I have not added a CentOS 8 test or upgraded the docker base images because there was no need to. I will be glad to do these if necessary.

Please let me know if I can improve this PR in any way before it's merged.

Thank you.

criu/proc_parse.c

This commit adds protobuf definitions needed to checkpoint and restore BPF map files along with the data they contain Source files added: * bpfmap-file.proto - Stores the meta-data about BPF maps * bpfmap-data.proto - Stores the data (key-value pairs) contained in BPF maps Source files modified: * fdinfo.proto - Added BPF map as a new kind of file descriptor. 'message file_entry' can now hold information about BPF map file descriptors * Makefile - Now generates build artifacts for bpfmap-file.proto and bpfmap-data.proto Signed-off-by: Abhishek Vijeev <[email protected]>

This commit defines constants and includes necessary headers to c/r BPF maps Source files modified: * magic.h - Defining BPFMAP_FILE_MAGIC and BPFMAP_DATA_MAGIC * image-desc.h - Defining CR_FD_BPFMAP_FILE and CR_FD_BPFMAP_DATA * image-desc.c - Create new entries for bpfmap-file and bpfmap-data in CRIU's file descriptor set * protobuf-desc.h - Defining PB_BPFMAP_FILE and PB_BPFMAP_DATA * protobuf-desc.c - Including headers for BPF map protobuf images Signed-off-by: Abhishek Vijeev <[email protected]>

Source files modified: * Makefile.config - Checks whether libbpf is installed on the system. If so, we add -lbpf to LIBS_FEATURES, -DCONFIG_HAS_LIBBPF to FEATURE_DEFINES and set CONFIG_HAS_LIBBPF. This allows us to check for the presence of libbpf before compiling or executing BPF c/r code and ZDTM tests. * Makefile - Set CONFIG_HAS_LIBBPF to clean all files. Signed-off-by: Abhishek Vijeev <[email protected]>

This commit enables CRIU to: (a) identify an anonymous inode as being a BPF map (b) parse information about BPF maps from procfs Source files modified: * files.c - Checks anonymous inodes to see whether they are BPF maps. If so, sets struct fdtype_ops *ops to a structure that knows how to dump BPF maps * proc_parse.c - Function parse_fdinfo_pid_s() now checks whether the current file being processed is a BPF map. If so, it calls a newly defined function parse_bpfmap() which knows how to parse information about BPF maps from procfs Signed-off-by: Abhishek Vijeev <[email protected]>

This commit enables CRIU to dump meta-data about BPF maps files by prividing the structures and functions needed by other parts of the code-base. Source files added: * bpfmap.c - defines new structures and functions: (a) struct fdtype_ops bpfmap_dump_ops: sets up the function handler to dump BPF maps (b) is_bpfmap_link(): checks whether an anonymous inode is a BPF map file (c) dump_one_bpfmap(): parses information for a BPF map file from procfs and dumps it * include/bpfmap.h - structure and function declarations Source files modified: * Makefile.crtools - generates build artifacts for bpfmap.c Signed-off-by: Abhishek Vijeev <[email protected]>

This commit enables CRIU to dump data(key-value) pairs stored in BPF maps Source files modified: * bpfmap.c - Function dump_one_bpfmap_data() reads the map's keys and values into two buffers using bpf_map_lookup_batch() and then writes them out to a protobuf image along with the number of key-value pairs read - Function dump_one_bpfmap() now dumps the data as well before returning * include/bpfmap.h - Includes headers and declares functions needed to dump BPF map data Signed-off-by: Abhishek Vijeev <[email protected]>

This commit enables CRIT to decode the contents of a protobuf image that stores information related to BPF map Signed-off-by: Abhishek Vijeev <[email protected]>

This commit enables CRIU to restore a process' BPF map file descriptors. Source files modified: * bpfmap.c - Structure and function definitions needed to: (a) collect a BPF map's information from its protobuf image (b) create and open a BPF map with the same parameters as when it was dumped (c) add the newly opened BPF map to the process' file descriptor list * include/bpfmap.h - Structure declarations for restoring BPF maps * files.c - Collects a BPF map's file entry during the restoration phase Signed-off-by: Abhishek Vijeev <[email protected]>

This commit restores the data of BPF maps. A hash table (indexed by the map's id) is used to store data objects for multiple BPF map files that a process may have opened. Collisions are resolved with chaining using a linked list. Source files modified: * bpfmap.c - Structure and function definitions needed to: (a) collect the protobuf image containing BPF map data (b) read the BPF map's data from the image and store it in the hash table (c) restore the map's data using bpf_map_update_batch() * include/bpfmap.h - Defines the size of the hash table and maks to be used while indexing into it - Structure and function declarations that are used while restoring BPF map data * cr-restore.c - Collects the protobuf image containing BPF map data during the restoration phase Signed-off-by: Abhishek Vijeev <[email protected]>

This commit adds ZDTM tests for c/r of processes with BPF maps as open files Source files added: * zdtm/static/bpf_hash.c - Tests for c/r of the data and meta-data of BPF map type BPF_MAP_TYPE_HASH * zdtm/static/bpf_array.c - Tests for c/r of the data and meta-data of BPF map type BPF_MAP_TYPE_ARRAY Source files modified: * zdtm/static/Makefile - Generating build artifacts for BPF tests Signed-off-by: Abhishek Vijeev <[email protected]>

Source files modified: * travis/vagrant.sh - Adding libbpf-devel Signed-off-by: Abhishek Vijeev <[email protected]>

This change fixes the error: error: 'security_context_t' is deprecated [-Werror=deprecated-declarations] Source files modified: * lsm.c * net.c Please refer to: SELinuxProject/selinux@9eb9c9327 Signed-off-by: Abhishek Vijeev <[email protected]>

rst0git

Well done!

abhishekvijeev · 2020-08-23T04:12:53Z

Thank you!

avagin · 2020-08-26T03:46:19Z

Applied. Thanks a lot!

abhishekvijeev · 2020-08-26T05:24:56Z

That's great!

Thanks so much to the community for helping me work towards getting this PR merged!

abhishekvijeev force-pushed the bpf_map branch 5 times, most recently from e5703e6 to f41a705 Compare April 30, 2020 15:48

abhishekvijeev force-pushed the bpf_map branch from f41a705 to b84c567 Compare May 3, 2020 10:06

avagin force-pushed the criu-dev branch from 1a20533 to 387b344 Compare May 4, 2020 16:23

avagin mentioned this pull request May 8, 2020

Preliminary set of changes to support dumping BPF map files. A major task is yet to be completed: parsing BPF file info from the proc filesystem in the function parse_fdinfo_pid_s() #1036

Closed

abhishekvijeev force-pushed the bpf_map branch from b84c567 to 21e76f1 Compare May 8, 2020 12:29

abhishekvijeev force-pushed the bpf_map branch from 21e76f1 to c6b6700 Compare May 19, 2020 14:05

abhishekvijeev force-pushed the bpf_map branch from c6b6700 to 45785db Compare May 23, 2020 17:40

abhishekvijeev closed this Jun 8, 2020

avagin reopened this Jun 8, 2020

abhishekvijeev force-pushed the bpf_map branch 2 times, most recently from fca68f1 to b94b87e Compare June 13, 2020 17:47

abhishekvijeev force-pushed the bpf_map branch 2 times, most recently from ea4eed6 to bc0db39 Compare June 19, 2020 11:33

abhishekvijeev force-pushed the bpf_map branch 7 times, most recently from 527d926 to b615e71 Compare August 17, 2020 16:47

jnnks mentioned this pull request Aug 18, 2020

Implement lxc-checkpoint anbox/anbox#1554

Closed

rst0git reviewed Aug 22, 2020

View reviewed changes

criu/proc_parse.c Show resolved Hide resolved

abhishekvijeev added 12 commits August 22, 2020 21:10

crit: add BPF map data decoding

bd4eb5c

This commit enables CRIT to decode the contents of a protobuf image that stores information related to BPF map Signed-off-by: Abhishek Vijeev <[email protected]>

scripts: adding libbpf for Travis tests

88ce1a7

Source files modified: * travis/vagrant.sh - Adding libbpf-devel Signed-off-by: Abhishek Vijeev <[email protected]>

abhishekvijeev force-pushed the bpf_map branch from b615e71 to fb7c634 Compare August 22, 2020 16:50

rst0git approved these changes Aug 22, 2020

View reviewed changes

avagin merged commit 7a576bf into checkpoint-restore:criu-dev Aug 26, 2020

abhishekvijeev deleted the bpf_map branch August 28, 2020 05:28

jnnks mentioned this pull request Sep 7, 2020

Discrepancy lxc-checkpoint vs lxc snapshot lxc/lxc#3537

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dumping BPF map files (Issue #777) #1044

Dumping BPF map files (Issue #777) #1044

abhishekvijeev commented Apr 25, 2020

Snorch commented Apr 29, 2020 •

edited

Loading

abhishekvijeev commented Apr 30, 2020

Snorch commented May 1, 2020 •

edited

Loading

abhishekvijeev commented May 1, 2020

avagin commented May 8, 2020

abhishekvijeev commented May 8, 2020

abhishekvijeev commented May 8, 2020

avagin commented May 13, 2020

abhishekvijeev commented May 13, 2020

abhishekvijeev commented May 19, 2020

abhishekvijeev commented Jun 2, 2020

avagin commented Jun 8, 2020

abhishekvijeev commented Jun 9, 2020

abhishekvijeev commented Aug 17, 2020

rst0git left a comment

abhishekvijeev commented Aug 23, 2020

avagin commented Aug 26, 2020

abhishekvijeev commented Aug 26, 2020

Dumping BPF map files (Issue #777) #1044

Dumping BPF map files (Issue #777) #1044

Conversation

abhishekvijeev commented Apr 25, 2020

Snorch commented Apr 29, 2020 • edited Loading

abhishekvijeev commented Apr 30, 2020

Snorch commented May 1, 2020 • edited Loading

abhishekvijeev commented May 1, 2020

avagin commented May 8, 2020

abhishekvijeev commented May 8, 2020

abhishekvijeev commented May 8, 2020

avagin commented May 13, 2020

abhishekvijeev commented May 13, 2020

abhishekvijeev commented May 19, 2020

abhishekvijeev commented Jun 2, 2020

avagin commented Jun 8, 2020

abhishekvijeev commented Jun 9, 2020

abhishekvijeev commented Aug 17, 2020

rst0git left a comment

Choose a reason for hiding this comment

abhishekvijeev commented Aug 23, 2020

avagin commented Aug 26, 2020

abhishekvijeev commented Aug 26, 2020

Snorch commented Apr 29, 2020 •

edited

Loading

Snorch commented May 1, 2020 •

edited

Loading