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.

Sign up for GitHub

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

Jump to bottom

Add vagrant VM support and XDP_USE_NEED_WAKEUP for advanced03 #70

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Add vagrant VM support and XDP_USE_NEED_WAKEUP for advanced03 #70

wants to merge 4 commits into from

Conversation

chaudron
Copy link
Contributor

@chaudron chaudron commented Sep 3, 2019

vagrant: add vagrant support to build latest bpf-next in VM
advanced03: add support for the XDP_USE_NEED_WAKEUP API

@chaudron
vagrant: add vagrant support to build latest bpf-next in VM
d3c1045 
This adds support for a Vagrant configuration that will build a VM
running fedora29, with the latest bpf-next kernel. In addition, it
will install and build the xdp-tutorials.

Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
@chaudron
advanced03: add support for the XDP_USE_NEED_WAKEUP API
eaf1e96 
Adding this will give improved performance. In my setup, it goes from
6,428kpps to 6,661kpps for 128 bytes packets with the IPv6 ping
packets replier (assignment 3).

Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
@tohojo
Copy link
Member

tohojo commented Sep 3, 2019 via email

@chaudron
Copy link
Contributor Author

chaudron commented Sep 3, 2019

Why fedora 29 instead of 30?

I tried Fedora30 but was giving me some odd errors on kernel install. Rather than spending too much time troubleshooting, I reverted to 29 which I know to be working.

Also, what would it take to turn this into a test runnable from CI? We'd probably need a 'make selftest' kind of target, but what else?

Good question, I have no real experience with CI integration with Vagrant (from GitHub).

@netoptimizer
Copy link
Member

I'll admit upfront: I have zero experience with "vagrant".

Could you explain what is the advantage of using vagrant?

Is this only to get a VM installed?
Or it is also an advantage for CI testing?

I do love the idea of adding make target for: 'make selftests'

@chaudron
Copy link
Contributor Author

chaudron commented Sep 3, 2019

Could you explain what is the advantage of using vagrant?

Vagrant just installs a VM from a base image. This is useful if you want to test your code in a clean environment. If you have the latest includes copied, for example, the build will not fail.

It also helps for people willing to try out the tutorial but don't want to mess with the kernel on their machine.

@tohojo
Copy link
Member

tohojo commented Sep 3, 2019 via email

@chaudron
Copy link
Contributor Author

chaudron commented Sep 3, 2019

Does it also support running the VM and executing a command inside it? That is basically what we need to hook this up to CI...

Yes you can, for example:

vagrant ssh -c "cd xdp-turorial/advanced03-AF_XDP && make"

@tohojo
Copy link
Member

tohojo commented Sep 3, 2019 via email

@chaudron
Copy link
Contributor Author

What distro are you using, works on RHEL7. Some distributions have the vagrant modules as packages.

@tohojo
Copy link
Member

tohojo commented Sep 11, 2019 via email

@chaudron chaudron mentioned this pull request Sep 11, 2019
Closed
@chaudron
Copy link
Contributor Author

@tohojo any reason for not merging this besides your distro having issues with vagrant? Not showing this on RHEL/Fedora.

@chaudron
advanced03: add some basic kernel version checking for AF_XDP support
ce93ed3 
Add some basic version checking support to warn if a kernel version is
not supporting AF_XDP.

Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
@chaudron
advanced03: fix compilation of custom xdp program on older kernels
3f89b0e 
This fixes execution failure when trying to load the custom XDP
program with the tutorial and kernels < 5.3.

Fixes #65

Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
@tohojo
Copy link
Member

tohojo commented Sep 26, 2019 via email

@tohojo
Copy link
Member

tohojo commented Sep 26, 2019

No dice; the Arch packaging for Vagrant plugins appears to be completely broken, and I can't get the vagrant plugin installer itself to do anything other than spit out errors.

Can you make this work without the 'reload' plugin?

@chaudron
Copy link
Contributor Author

Can you make this work without the 'reload' plugin?

I need to restart the machine after the kernel build, did not find a way to do this without a plugin.

@tohojo
Copy link
Member

tohojo commented Sep 26, 2019 via email

@chaudron
Copy link
Contributor Author

Why can't the kernel build not just execute a 'reboot' inside the vm?

As vagrant is doing the provisioning, just calling reboot will fail the provisioning step.

@tohojo
Copy link
Member

tohojo commented Sep 30, 2019 via email

netoptimizer
netoptimizer reviewed Nov 27, 2019
View reviewed changes
advanced03-AF_XDP/af_xdp_kern.c
@@ -18,6 +19,17 @@ struct bpf_map_def SEC("maps") xdp_stats_map = {
.max_entries = 64,
};

#if LINUX_VERSION_CODE < KERNEL_VERSION(5,3,0)
Copy link
Member

Choose a reason for hiding this comment

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

It is not the right approach to check against a kernel version.
(Because distros will backport changes)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I know, but due to the lack of any run-time checks, what would you suggest?

Copy link
Member

Choose a reason for hiding this comment

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

Instead we have to write a bpf probe that can detect this (like bpftool does).
@tohojo have added a configure script in his XDP-tools repo.
In this case, we likely need to confine the check to the advanced03-AF_XDP/ directory, and we can likely add it as part of the Makefile.

Copy link
Member

Choose a reason for hiding this comment

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

The configure script just sets some variables; so we can do that in the top-level and only react to them in advanced03. I've been meaning to port over some of the build stuff I did in xdp-tools to this repo anyway, so this sounds like a way forward :)

tohojo
tohojo reviewed Nov 27, 2019
View reviewed changes
advanced03-AF_XDP/af_xdp_kern.c
#if LINUX_VERSION_CODE < KERNEL_VERSION(5,3,0)

/* Kernel version before 5.3 needed an additional map */
struct bpf_map_def SEC("maps") qidconf_map = {
Copy link
Member

Choose a reason for hiding this comment

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

I don't see this map referred to anywhere; how is it actually used on those old kernels?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Think this is part of the older libbpf installing int

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tohojo tohojo tohojo left review comments

@netoptimizer netoptimizer netoptimizer left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@chaudron @tohojo @netoptimizer