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

NETOBSERV-1255 Adding Raw Packet export capability as Packet Capture Agent(PCA) #113

Merged
merged 2 commits into from
Sep 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@
bin/
e2e-logs
ebpf-agent.tar
*.pcap
2 changes: 2 additions & 0 deletions bpf/configs.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,7 @@
volatile const u32 sampling = 0;
volatile const u8 trace_messages = 0;
volatile const u8 enable_rtt = 0;
volatile const u16 pca_port = 0;
volatile const u8 pca_proto = 0;

#endif //__CONFIGS_H__
5 changes: 5 additions & 0 deletions bpf/flows.c
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@
*/
#include "rtt_tracker.h"

/* Defines a Packet Capture Agent (PCA) tracker,
It is enabled by setting env var ENABLE_PCA= true. Is Optional
*/
#include "pca.h"

static inline int flow_monitor(struct __sk_buff *skb, u8 direction) {
// If sampling is defined, will only parse 1 out of "sampling" flows
if (sampling != 0 && (bpf_get_prandom_u32() % sampling) != 0) {
Expand Down
9 changes: 9 additions & 0 deletions bpf/maps_definition.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,14 @@ struct {
__uint(map_flags, BPF_F_NO_PREALLOC);
} flow_sequences SEC(".maps");

//PerfEvent Array for Packet Payloads
struct {
__uint(type, BPF_MAP_TYPE_PERF_EVENT_ARRAY);
__type(key, u32);
__type(value, u32);
__uint(max_entries, 256);
} packet_record SEC(".maps");

Copy link
Contributor

Choose a reason for hiding this comment

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

Indent.

Question: do you know how much space a PERF_EVENT_ARRAY consumes and if it is configurable? can you explictily call it out here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed Indent.

From https://www.spinics.net/lists/xdp-newbies/msg01831.html
"libbpf's bpf_object__create_map() will automatically set max_entries to the maximum configured number of CPUs on the host".

This is specific to PERF_EVENT_ARRAY.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The ValueSize for PERF_EVENT_ARRAY must be 0 or 4 - according to ebpf/map.go .

Copy link
Member

Choose a reason for hiding this comment

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

this is all preallocated? Can't we use here the same BPF_F_NO_PREALLOC as for other map types?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Kernel does not support BPF_F_NO_PREALLOC for LRU type maps.
cilium/ebpf#1072

Copy link
Contributor

Choose a reason for hiding this comment

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

BPF_F_NO_PREALLOC not avaliable for all map types

// DNS tracking flow based hashmap used to correlate query and responses
// to allow calculating latency in ebpf agent directly
struct {
Expand All @@ -39,4 +47,5 @@ struct {
__type(value, u64);
__uint(map_flags, BPF_F_NO_PREALLOC);
} dns_flows SEC(".maps");

#endif //__MAPS_DEFINITION_H__
123 changes: 123 additions & 0 deletions bpf/pca.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
#ifndef __PCA_H__
#define __PCA_H__

#include "utils.h"
#include <string.h>

static int attach_packet_payload(void *data, void *data_end, struct __sk_buff *skb){
payload_meta meta;
u64 flags = BPF_F_CURRENT_CPU;
// Enable the flag to add packet header
// Packet payload follows immediately after the meta struct
u32 packetSize = (u32)(data_end-data);

// Record the current time.
u64 current_time = bpf_ktime_get_ns();

// For packets which are allocated non-linearly struct __sk_buff does not necessarily
// has all data lined up in memory but instead can be part of scatter gather lists.
// This command pulls data from the buffer but incurs data copying penalty.
if (packetSize <= skb->len){
packetSize = skb->len;
if (bpf_skb_pull_data(skb, skb->len)){
return TC_ACT_UNSPEC;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we come to this case for TCP DNS have u tried to capture DNS over TCP and measured the CPU usage ? u can simply do dig google.om +tcp to do DNS over TCP

Copy link
Contributor Author

@shach33 shach33 Aug 25, 2023

Choose a reason for hiding this comment

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

I am not sure what is happening here, but when I experiment, the requests/response generated via dig google.com +tcp are also UDP packets.

Copy link
Contributor

Choose a reason for hiding this comment

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

no that is indeed tcp dns I have used it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Then, I am able to capture those.

};
}
// Set flag's upper 32 bits with the size of the paylaod and the bpf_perf_event_output will
// attach the specified amount of bytes from packet to the perf event
// https://github.com/xdp-project/xdp-tutorial/tree/9b25f0a039179aca1f66cba5492744d9f09662c1/tracing04-xdp-tcpdump
flags |= (u64)packetSize << 32;

meta.if_index = skb->ifindex;
meta.pkt_len = packetSize;
meta.timestamp = current_time;
if (bpf_perf_event_output(skb, &packet_record, flags, &meta, sizeof(meta))){
Copy link
Contributor

Choose a reason for hiding this comment

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

what is the point of checking the return code if either way u return ok ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this was an oversight. Fixed it.

return TC_ACT_OK;
}
return TC_ACT_UNSPEC;
}

static inline bool validate_pca_filter(u8 ipproto, void *ipheaderend, void *data_end){
// If filters: pca_proto and pca_port are not specified, export packet
if (pca_proto == 0 && pca_port == 0)
return true;

//Only export packets with protocol set by ENV var PCA_FILTER
u16 sourcePort, destPort;
if (ipproto != pca_proto) {
return false;
}

if (ipproto == IPPROTO_TCP){
struct tcphdr *tcp_header = ipheaderend;
if ((void *)tcp_header + sizeof(*tcp_header) > data_end) {
return false;
}
sourcePort = tcp_header->source;
destPort = tcp_header->dest;
}
else if (ipproto == IPPROTO_UDP){
struct udphdr *udp_header = ipheaderend;
if ((void *)udp_header + sizeof(*udp_header) > data_end) {
return false;
}
sourcePort = udp_header->source;
destPort = udp_header->dest;
}
Copy link
Contributor

@msherif1234 msherif1234 Aug 24, 2023

Choose a reason for hiding this comment

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

this is not right , we can't assume if its not TCP its UDP, we have SCTP, ICMPv4 and ICMPv6 protocols support

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated. Only checks for tcp/udp, for anything else, we return.

Copy link
Contributor

Choose a reason for hiding this comment

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

sctp will work very similar to ycp and udp unless we see no use case for it ? which isn't true because svc can use sctp protocol taht is why we added it

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 added SCTP. ICMP packets dont really have port associated to them. So, it would require an update to the filter specifications, which I can do as part of an extension, but may be not with the current PR.

else if (ipproto == IPPROTO_SCTP){
struct sctphdr *sctp_header = ipheaderend;
if ((void *)sctp_header + sizeof(*sctp_header) > data_end) {
return false;
}
sourcePort = sctp_header->source;
destPort = sctp_header->dest;
}
else {
return false;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Please add {} here with else to keep the code structure same across all C code.

@shach33

}
u16 pca_port_end = bpf_htons(pca_port);
if (sourcePort == pca_port_end || destPort == pca_port_end){
return true;
}
return false;
}

static inline int export_packet_payload (struct __sk_buff *skb) {
void *data_end = (void *)(long)skb->data_end;
void *data = (void *)(long)skb->data;
struct ethhdr *eth = data;
struct iphdr *ip;

if ((void *)eth + sizeof(*eth) > data_end) {
return TC_ACT_UNSPEC;
Copy link
Contributor

Choose a reason for hiding this comment

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

why is PCA returning TC_ACT_UNSPEC shouldn't it return TC_ACT_PIPE?

If I am gettting confused with them can you document the reference on which one is chosen and why?

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 have only used TCA_ACT_UNSPEC, refering to the flowlogs agent that does the same. I remember discussions about using PIPE instead, but it doesnt look like it has been used.

}

// Only IPv4 and IPv6 packets captured
u16 ethType = bpf_ntohs(eth->h_proto);
if (ethType != ETH_P_IP && ethType != ETH_P_IPV6) {
return TC_ACT_UNSPEC;
}

ip = data + sizeof(*eth);
if ((void *)ip + sizeof(*ip) > data_end) {
return TC_ACT_UNSPEC;
}

if (validate_pca_filter(ip->protocol, (void *)ip + sizeof(*ip), data_end )){
return attach_packet_payload(data, data_end, skb);
}
return TC_ACT_UNSPEC;
}


SEC("tc_pca_ingress")
int ingress_pca_parse (struct __sk_buff *skb) {
return export_packet_payload(skb);
}

SEC("tc_pca_egress")
int egress_pca_parse (struct __sk_buff *skb) {
return export_packet_payload(skb);
}

#endif /* __PCA_H__ */
8 changes: 8 additions & 0 deletions bpf/types.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

#define TC_ACT_OK 0
#define TC_ACT_SHOT 2
#define TC_ACT_UNSPEC -1
#define IP_MAX_LEN 16

#define DISCARD 1
Expand Down Expand Up @@ -156,6 +157,13 @@ typedef struct pkt_info_t {
u64 rtt; // rtt calculated from the flow if possible. else zero
} pkt_info;

// Structure for payload metadata
typedef struct payload_meta_t {
u32 if_index;
u32 pkt_len;
u64 timestamp; // timestamp when packet received by ebpf
} __attribute__((packed)) payload_meta;

// DNS Flow record used as key to correlate DNS query and response
typedef struct dns_flow_id_t {
u16 src_port;
Expand Down
52 changes: 37 additions & 15 deletions cmd/netobserv-ebpf-agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,20 @@ import (
_ "net/http/pprof"
)

func terminateAgent() (ctx context.Context) {

logrus.Infof("push CTRL+C or send SIGTERM to interrupt execution")
ctx, canceler := context.WithCancel(context.Background())
// Subscribe to signals for terminating the program.
go func() {
stopper := make(chan os.Signal, 1)
signal.Notify(stopper, os.Interrupt, syscall.SIGTERM)
<-stopper
canceler()
}()
return ctx
}
Copy link
Contributor

Choose a reason for hiding this comment

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

why did u add this ? for standalone testing ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was to reuse the same code block for terminating both netobserv and PCA.


func main() {
logrus.Infof("starting NetObserv eBPF Agent")
config := agent.Config{}
Expand All @@ -39,22 +53,30 @@ func main() {

logrus.WithField("configuration", fmt.Sprintf("%#v", config)).Debugf("configuration loaded")

flowsAgent, err := agent.FlowsAgent(&config)
if err != nil {
logrus.WithError(err).Fatal("can't instantiate NetObserv eBPF Agent")
}
if config.EnablePCA {
Copy link
Member

Choose a reason for hiding this comment

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

so this means the agent is started either for flow logs or for PCA, but cannot do both at the same time, right?
Do you see a technical limitation for doing so, or is that just a choice and we could change that any time?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. The flows and packets agent are disparate and only one is active at a time. The metric of transfer in flows agent is a flow struct, while with PCA is a packet. We could merge them but that would add multiple unnecessary if checks at every step.

If the team deems this to be a better way to go, I can later create another PR with the merge. As of now, the code is cleanly separated between packet capture and flows agent.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think if we are going to keep agent either PCA or Flow logs we should specify that make a single environment like AGENT_TYPE which can be PCA or FlowLogs

Copy link
Contributor Author

@shach33 shach33 Aug 8, 2023

Choose a reason for hiding this comment

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

We can discuss this in our calls and make a decision then. In this PR, I would suggest to let it stay the way it is.

Copy link
Member

Choose a reason for hiding this comment

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

sounds good

if config.PCAFilters == "" {
logrus.Info("[PCA] NetObserv eBPF Agent instantiated without filters to identify packets. All packets will be captured. This might cause reduced performance.")
}
packetsAgent, err := agent.PacketsAgent(&config)
if err != nil {
shach33 marked this conversation as resolved.
Show resolved Hide resolved
logrus.WithError(err).Fatal("[PCA] can't instantiate NetObserv eBPF Agent")
}

logrus.Infof("push CTRL+C or send SIGTERM to interrupt execution")
ctx, canceler := context.WithCancel(context.Background())
// Subscribe to signals for terminating the program.
go func() {
stopper := make(chan os.Signal, 1)
signal.Notify(stopper, os.Interrupt, syscall.SIGTERM)
<-stopper
canceler()
}()
if err := flowsAgent.Run(ctx); err != nil {
logrus.WithError(err).Fatal("can't start netobserv-ebpf-agent")
ctx := terminateAgent()
if err := packetsAgent.Run(ctx); err != nil {
logrus.WithError(err).Fatal("[PCA] can't start netobserv-ebpf-agent")
}
} else {
flowsAgent, err := agent.FlowsAgent(&config)

if err != nil {
logrus.WithError(err).Fatal("can't instantiate NetObserv eBPF Agent")
}

ctx := terminateAgent()
if err := flowsAgent.Run(ctx); err != nil {
logrus.WithError(err).Fatal("can't start netobserv-ebpf-agent")
}
}
}

Expand Down
4 changes: 4 additions & 0 deletions docs/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,10 @@ The following environment variables are available to configure the NetObserv eBF
If it is not set, profile is disabled.
* `ENABLE_RTT` (default: `false` disabled). If `true` enables RTT calculations for the captured flows in the ebpf agent.
See [docs](./rtt_calculations.md) for more details on this feature.
* `ENABLE_PCA` (default: `false` disabled). If `true` enables Packet Capture Agent.
* `PCA_FILTER` (default: `none`). Works only when `ENABLE_PCA` is set. Accepted format <protocol,portnumber>. Example
`PCA_FILTER=tcp,22`.
* `PCA_SERVER_PORT` (default: 0). Works only when `ENABLE_PCA` is set. Agent opens PCA Server at this port. A collector can connect to it and recieve filtered packets as pcap stream. The filter is set using `PCA_FILTER`.
Copy link
Contributor

Choose a reason for hiding this comment

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

can we rename to PCA_TARGET_PORT and default this port to 9990 so we don't need to explicitly set it at agent start.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also can you add a PCA_TARGET_HOST default to localhost here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because PCA agent is a server, so it didnt seem like there is a need for PCA_TARGET_HOST. I can add it if it is meant for consistency purposes. let me know.

As as as PCA_SERVER_PORT be renamed as PCA_TARGET_PORT, I understand it for consistency again, but in order to make sure it is known that PCA agent is server, I put the SERVER in the name. @jotak I remember we talked about this varaible name, would you prefer SERVER or TARGET in the PORT variable.

Copy link
Member

@jotak jotak Aug 23, 2023

Choose a reason for hiding this comment

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

yeah I vote for "SERVER" here to make it clear that in order to collect pcaps, a client is needed (unlike the netflows that are sent to FLP server)


## Development-only variables

Expand Down
48 changes: 48 additions & 0 deletions examples/packetcapture-dump/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
# packetcapture-client

## How to run

From the root directory of the project:

Build the agent (the flowlogs client that uses ebpf) using:
```bash
make build
```
Build the packetcapture-dump-collector (the client that receives full packets from the agent and writes to a pcap file) using:
```bash
go build -mod vendor -o bin/packetcapture-client examples/packetcapture-dump/client/packetcapture-client.go
```
Start the agent using:
```bash
sudo PCA_SERVER_PORT=9990 ENABLE_PCA=true PCA_FILTER=tcp,22 ./bin/netobserv-ebpf-agent
```

Start the packetcapture-client using: (in a secondary shell)
```bash
./bin/packetcapture-client -outfile=capture.pcap
```

You should see output such as:
```bash
Starting Packet Capture Client.
By default, the read packets are printed on stdout.
To write to a pcap file use flag '-outfile=[filename]'
This creates a file [filename] and writes packets to it.
To view captured packets 'tcpdump -r [filename]'.

07-24-2023 07:58:59.264323 : Received Packet of length 24
07-24-2023 07:59:04.268965 : Received Packet of length 410
07-24-2023 07:59:04.269048 : Received Packet of length 644
07-24-2023 07:59:04.269087 : Received Packet of length 224
07-24-2023 07:59:04.269125 : Received Packet of length 82
07-24-2023 07:59:04.269173 : Received Packet of length 148
...
```

To open pcap file:
```bash
tcpdump -r capture.pcap
```



Loading