-
Notifications
You must be signed in to change notification settings - Fork 186
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
Conversation
c8f0e3a
to
218e602
Compare
There was a problem hiding this 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?
include/gadget/sockets-map.h
Outdated
err = bpf_skb_load_bytes( | ||
skb, ETH_HLEN + offsetof(struct ipv6hdr, nexthdr), | ||
skb, ETH_HLEN + SE_IPV6_NEXTHDR_OFFSET, |
There was a problem hiding this comment.
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.
include/gadget/sockets-map.h
Outdated
// Redefine the constants we need but namespaced (SE_) so we don't pollute gadgets. | ||
|
||
#define SE_PACKET_HOST 0 | ||
#define SE_PACKET_OUTGOING 4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not used?
include/gadget/sockets-map.h
Outdated
@@ -110,7 +82,7 @@ gadget_socket_lookup(const struct __sk_buff *skb) | |||
|
|||
switch (h_proto) { | |||
case bpf_htons(ETH_P_IP): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SE_ETH_P_IP
include/gadget/sockets-map.h
Outdated
@@ -131,13 +103,13 @@ gadget_socket_lookup(const struct __sk_buff *skb) | |||
break; | |||
|
|||
case bpf_htons(ETH_P_IPV6): |
There was a problem hiding this comment.
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.
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>
218e602
to
68b0a43
Compare
There was a problem hiding this 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!
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