xdp-tutorial icon indicating copy to clipboard operation
xdp-tutorial copied to clipboard

basic04-pinning-maps: correct solution for assignment 2?

Open alexeldeib opened this issue 4 years ago • 3 comments

Thanks so much for such an incredible resource!

I've been working through the assignments and I was trying to understand how the provided solution implements what the prompt requires for assignment 2.

I was expecting to remove the call to bpf_object__unpin_maps and replace it with something like https://github.com/xdp-project/xdp-tutorial/tree/master/basic04-pinning-maps#reusing-maps-with-libbpf inside the access check:

/* Existing/previous XDP prog might not have cleaned up */
if (access(map_filename, F_OK ) != -1 ) {
	if (verbose)
		printf(" - Unpinning (remove) prev maps in %s/\n",
				pin_dir);

	int map_fd = bpf_obj_get(map_filename);
	struct bpf_object *obj = bpf_object__open(filename);
	struct bpf_map *map = bpf_object__find_map_by_name(obj, map_name);
	err = bpf_map__reuse_fd(map, map_fd);
	if (err) {
		fprintf(stderr, "ERR: reusing bpf map fd");
		return EXIT_FAIL_BPF;
	}

	err = bpf_map__load(obj);
	if (err) {
		fprintf(stderr, "ERR: loading bpf map");
		return EXIT_FAIL_BPF;
	}
}

The solution doesn't see to do this, it only seems to add a toggle to the CLI flags, am I missing something?

alexeldeib avatar May 11 '20 09:05 alexeldeib

Hmm, yeah, you're right that seems to be an oversight. However, libbpf has since introduced automatic re-use of pinned maps, so I guess we should switch this whole assignment over to explaining that. There are a couple of these kinds of tweaks, so taking a full pass through the tutorial and doing a few updates is really needed... It's on my list :)

tohojo avatar May 11 '20 10:05 tohojo

I'd be happy to help update the assignment to reflect that if it's welcome, with the caveat that I'm familiar with BPF but neither an XDP pro nor kernel dev :)

I was looking around for any libbpf docs indicating that change or a patchset, any chance you have some reference material? If not, I can take a stab at it without. I found some tangential patchsets but not this diff yet.

alexeldeib avatar May 11 '20 17:05 alexeldeib

Hmm, only reference is this commit: https://github.com/libbpf/libbpf/commit/e7d860d2fc16759109e0194bc3be8ae3f6c68162

It requires BTF-defined maps, though, which we're not using in the tutorial either since that in turn depends on LLVM9. We haven't decided whether we're OK to depend on LLVM9 for the entire tutorial or if we should be splitting things based on version. So feel free to take a stab at defining something like this, but be prepared to move things around a bit if you do :)

If you do want to try it, you'll also need an updated libbpf; see #126

tohojo avatar May 12 '20 15:05 tohojo