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 bpf2go example + Devcontainer example #126

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

Conversation

David-VTUK
Copy link

bpf2go enables embedding and interacting with eBPF programs in Go applications. It does this by compiling, loading and managing eBPF programs by generating Go bindings from eBPF object files.

This is a simple example that includes both components:

  • A eBPF kernel application that inspects incoming packets via xdp, recording the protocol value in the IP header and storing it in a map.

  • A Go user space application that loads the eBPF application, reads from the map and outputs information via stdout, translating the IANA protocol number to its respective name.

The README.md for the example contains further information

Additionally, a devcontainers example directory is added to help speed up eBPF development by facilitating repeatable, consistent environments to develop eBPF applications in.

Signed-off-by: David Holder [email protected]

Copy link
Member

@tohojo tohojo left a comment

Choose a reason for hiding this comment

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

Generally looks good, thanks for the contribution! Only a few minor-ish comments on the code, see below.

Also, please squash the second and third commit into the first one when you respin :)

}
defer objs.Close()

ifname := "eno2" // Change this to an interface on your machine.
Copy link
Member

Choose a reason for hiding this comment

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

Couldn't the interface name be given on the command line? That would make it possible to run the example without changing the code...

#include <linux/bpf.h>
#include <bpf/bpf_helpers.h>
#include <linux/if_ether.h>
#include <linux/ip.h>
Copy link
Member

Choose a reason for hiding this comment

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

We generally follow the Linux kernel coding style for the BPF code in this repo. I think we should do this here as well for consistency. So please change the indent to be a single tab character; and a few more comments below:

void *data = (void *)(long)ctx->data;

// Parse Ethernet header
struct ethhdr *eth = data;
Copy link
Member

Choose a reason for hiding this comment

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

Variable declarations should all be at the top of the function.

xdp-bpf2go-example/packetProtocol.c Outdated Show resolved Hide resolved
}

// Check if the packet is an IP packet
if (eth->h_proto != __constant_htons(ETH_P_IP)) {
Copy link
Member

Choose a reason for hiding this comment

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

We use bpf_htons() in other places - that automatically selects the constant version as appropriate.

Also, drop the braces

}

// Parse IP header
struct iphdr *ip = data + sizeof(struct ethhdr);
Copy link
Member

Choose a reason for hiding this comment

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

Variable declaration at the top of the file, please.

}

__u32 key = ip->protocol; // Using IP protocol as the key
__u64 *count = bpf_map_lookup_elem(&protocol_count, &key);
Copy link
Member

Choose a reason for hiding this comment

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

Variable declarations at the top of the file.


__u32 key = ip->protocol; // Using IP protocol as the key
__u64 *count = bpf_map_lookup_elem(&protocol_count, &key);
if (count) {
Copy link
Member

Choose a reason for hiding this comment

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

No need for the braces.

for {
select {
case <-tick:
// log.Print(objs.ProtocolCount)
Copy link
Member

Choose a reason for hiding this comment

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

Leftover debug comment?

xdp-bpf2go-example/main.go Show resolved Hide resolved
Signed-off-by: David-VTUK <[email protected]>

revert README.org to README.md to retain formatting

Signed-off-by: David-VTUK <[email protected]>

Resize diagram image

Signed-off-by: David-VTUK <[email protected]>
Signed-off-by: David-VTUK <[email protected]>
Signed-off-by: David-VTUK <[email protected]>

Remove superfluous braces

Signed-off-by: David-VTUK <[email protected]>
struct ethhdr *eth = data;
struct iphdr *ip = data + sizeof(struct ethhdr);
__u32 key = ip->protocol; // Using IP protocol as the key
__u64 *count = bpf_map_lookup_elem(&protocol_count, &key);
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 think this works? You're dereferencing the ip variable before the range check below, which will be rejected by the verifier.

You can declare the variable without assigning to it at the top of the function (just __u32 key), and then to the assignment key = ip->protocol below the range check at the bottom. Same thing for the assignment to count (and arguably to ip.

Also, please fix the indentation (to be a single tab character per indentation level) as is the custom in the kernel sources.

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

Successfully merging this pull request may close these issues.

2 participants