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

Add QCMP support in XDP I/O loop #1084

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Add QCMP support in XDP I/O loop #1084

wants to merge 7 commits into from

Conversation

Jake-Shadle
Copy link
Collaborator

Resolves: #1076

@XAMPPRocky
Copy link
Collaborator

Did you see my comment about having this be a separate ebpf module?

#1076 (comment)

@Jake-Shadle
Copy link
Collaborator Author

We can only have 1 eBPF module loaded on a device at a time, libbpf gets around that by doing tail calls, but we'd have to do that ourselves, I guess I'm not sure why we would want that added complexity? Keeping the code in userspace means the eBPF code gets to stay extremely simple, the more complex it gets the harder it is to maintain since then you need to ensure it keeps passing the validator at all times and makes it harder to share code that already exists in quilkin, like for the QCMP stuff.

@XAMPPRocky
Copy link
Collaborator

XAMPPRocky commented Jan 30, 2025

I guess I'm not sure why we would want that added complexity? Keeping the code in userspace means the eBPF code gets to stay extremely simple, the more complex it gets the harder it is to maintain since then you need to ensure it keeps passing the validator at all times and makes it harder to share code that already exists in quilkin, like for the QCMP stuff.

My thought with QCMP, is that since it is such a simple protocol, it doesn't need to send data to user space at all, the XDP program can send packets directly back. Also this code when combined into a single program needs to handle UDP not being set by the user space program (e.g. the agent mode exposes QCMP but not UDP). And it might be simpler if that logic was in the user space program where it only loads the modules that are requested, and then the module just assumes it is needed.

@Jake-Shadle
Copy link
Collaborator Author

So you're saying you want the agent to run QCMP in eBPF and share that code with the proxy via an eBPF module?

@XAMPPRocky
Copy link
Collaborator

XAMPPRocky commented Jan 30, 2025

So you're saying you want the agent to run QCMP in eBPF and share that code with the proxy via an eBPF module?

No I'm just clarifying that QCMP and UDP should be considered separate services, and thus that you can have either one or both services active at a time. So either we need to put that in one module where we pass that as a setting to the module, or we have them considered to be separate modules and we decide at load time whether to have UDP, QCMP, or UDP+QCMP.

@Jake-Shadle
Copy link
Collaborator Author

Sure, I understand they are separate logically, but in practice with XDP it makes more sense to have them in the same I/O loop because of the thread starvation issue and timing sensitivity of the QCMP packets. Putting that code in eBPF instead of userspace means code duplication, and just makes it harder since the only source of time available to eBPF is nanoseconds since system boot, not unix epoch. I've made it so that if QCMP is disabled it just passes 0 as the port, and it would be trivial to create a different eBPF program for the agent that could only handle QCMP in the future if we felt that made sense.

@XAMPPRocky
Copy link
Collaborator

Okay, the lack of epoch time is a non-starter. So if we have just one XDP module, would you be able to add it so it also doesn't run the UDP service if its port is zero?

Then we can have XDP loaded if either UDP or QCMP are enabled in Service::spawn_services.

How does that sound?

@Jake-Shadle
Copy link
Collaborator Author

That works now, XDP will be loaded if either QCMP or UDP services are enabled, if either are disabled they are set to 0 meaning the eBPF program will ignore packets not sent to the specific port(s) that are enabled.

@quilkin-bot
Copy link
Collaborator

Build Succeeded 🥳

Build Id: cf23b7ae-ed54-48e0-8eae-7704a4054d66

The following development images have been built, and will exist for the next 30 days:

To build this version:

git fetch [email protected]:googleforgames/quilkin.git pull/1084/head:pr_1084 && git checkout pr_1084
cargo build

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

Successfully merging this pull request may close these issues.

Move QCMP to eBPF or XDP
3 participants