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

sockets-map.h: remove most defines from socket enricher header #2846

Merged
merged 1 commit into from
May 16, 2024

Conversation

alban
Copy link
Member

@alban alban commented May 14, 2024

sockets-map.h can be included in both tracer gadgets (kprobes, etc.) and network gadgets (packet capture). Move most #define to avoid polluting the 'define' namespace of gadgets.

This is a first step to enable gadgets containing both tracers and network programs.

See #2371

This is to avoid the manual copy of sockets-map.h as it was done in the PoC alban/trace-udns#1

Copy link
Member

@mauriciovasquezbernal mauriciovasquezbernal left a comment

Choose a reason for hiding this comment

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

Some comments about missing things.

I'm wondering what a gadget that uses both (networking and tracing) functions will look like. Will it #include <vmlinux.h> and manually define all networking "#defines" as you did on https://github.com/alban/trace-udns/blob/84e3c5f144e5141464134a57a7d6896f94d5f162/program.bpf.c?

err = bpf_skb_load_bytes(
skb, ETH_HLEN + offsetof(struct ipv6hdr, nexthdr),
skb, ETH_HLEN + SE_IPV6_NEXTHDR_OFFSET,

Choose a reason for hiding this comment

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

SE_ETH_LEN? Same in other places.

// Redefine the constants we need but namespaced (SE_) so we don't pollute gadgets.

#define SE_PACKET_HOST 0
#define SE_PACKET_OUTGOING 4

Choose a reason for hiding this comment

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

not used?

@@ -110,7 +82,7 @@ gadget_socket_lookup(const struct __sk_buff *skb)

switch (h_proto) {
case bpf_htons(ETH_P_IP):

Choose a reason for hiding this comment

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

SE_ETH_P_IP

@@ -131,13 +103,13 @@ gadget_socket_lookup(const struct __sk_buff *skb)
break;

case bpf_htons(ETH_P_IPV6):

Choose a reason for hiding this comment

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

missing namespaced version of this one.

include/gadget/sockets-map.h Show resolved Hide resolved
include/gadget/sockets-map.h Show resolved Hide resolved
sockets-map.h can be included in both tracer gadgets (kprobes, etc.) and
network gadgets (packet capture). Move most #define to avoid polluting
the 'define' namespace of gadgets.

This is a first step to enable gadgets containing both tracers and
network programs.

Signed-off-by: Alban Crequy <albancrequy@linux.microsoft.com>
Copy link
Member

@mauriciovasquezbernal mauriciovasquezbernal left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for cleaning this up!

@alban alban merged commit 3b2ce1d into main May 16, 2024
60 checks passed
@alban alban deleted the alban_socketenricher_header branch May 16, 2024 18:53
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.

None yet

2 participants