Skip to content

Commit

Permalink
fix(conntrack): delete keys in eBPF instead of user space (#831)
Browse files Browse the repository at this point in the history
# Description

Delete TCP connections in `retina_conntrack` map directly in the eBPF
layer instead of relying on the userspace process to delete it later
when the connection is closing and has exceeded its lifetime.

* remove `is_closing` flag from `retina_conntrack` map, update userspace
and bpf program accordingly
* delete connection from `retina_conntrack` map if connection is timed
out or when FIN or RST flags are set
* invoke `bpf_map_delete_elem` in`_ct_should_report_packet` and remove
update `seen_flags` and `last_report`

## Related Issue

fix #807 

## Checklist

- [x] I have read the [contributing
documentation](https://retina.sh/docs/contributing).
- [x] I signed and signed-off the commits (`git commit -S -s ...`). See
[this
documentation](https://docs.github.com/en/authentication/managing-commit-signature-verification/about-commit-signature-verification)
on signing commits.
- [x] I have correctly attributed the author(s) of the code.
- [x] I have tested the changes locally.
- [x] I have followed the project's style guidelines.
- [x] I have updated the documentation, if necessary.
- [x] I have added tests, if applicable.

## Screenshots (if applicable) or Testing Completed

Please add any relevant screenshots or GIFs to showcase the changes
made.

## Additional Notes

Add any additional notes or context about the pull request here.

---

Please refer to the [CONTRIBUTING.md](../CONTRIBUTING.md) file for more
information on how to contribute to this project.

---------

Signed-off-by: Simone Rodigari <[email protected]>
  • Loading branch information
SRodi authored Oct 9, 2024
1 parent 11f190d commit 3983a57
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 21 deletions.
26 changes: 9 additions & 17 deletions pkg/plugin/conntrack/_cprog/conntrack.c
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ struct ct_entry {
*/
__u8 flags_seen_tx_dir;
__u8 flags_seen_rx_dir;
bool is_closing; // is_closing indicates if the connection is closing.
};

struct {
Expand All @@ -72,7 +71,6 @@ struct {
__uint(pinning, LIBBPF_PIN_BY_NAME); // needs pinning so this can be access from other processes .i.e debug cli
} retina_conntrack SEC(".maps");


/**
* Helper function to reverse a key.
* @arg reverse_key The key to store the reversed key.
Expand Down Expand Up @@ -177,7 +175,6 @@ static __always_inline bool _ct_handle_tcp_connection(struct packet *p, struct c
return false;
}
new_value.eviction_time = now + CT_CONNECTION_LIFETIME_TCP;
new_value.is_closing = (p->flags & (TCP_FIN | TCP_RST)) != 0x0;
new_value.traffic_direction = _ct_get_traffic_direction(observation_point);
p->traffic_direction = new_value.traffic_direction;

Expand Down Expand Up @@ -221,12 +218,13 @@ static __always_inline bool _ct_handle_new_connection(struct packet *p, struct c
* @arg protocol The protocol of the packet (TCP or UDP).
* Returns true if the packet should be reported to userspace. False otherwise.
*/
static __always_inline bool _ct_should_report_packet(struct ct_entry *entry, __u8 flags, __u8 direction, __u8 protocol) {
static __always_inline bool _ct_should_report_packet(struct ct_entry *entry, __u8 flags, __u8 direction, struct ct_v4_key *key) {
// Check for null parameters.
if (!entry) {
return false;
}

__u8 protocol = key->proto;
__u64 now = bpf_mono_now();
__u32 eviction_time = READ_ONCE(entry->eviction_time);
__u8 seen_flags;
Expand All @@ -243,15 +241,9 @@ static __always_inline bool _ct_should_report_packet(struct ct_entry *entry, __u

// Check if the connection timed out or if it is a TCP connection and FIN or RST flags are set.
if (now >= eviction_time || (protocol == IPPROTO_TCP && flags & (TCP_FIN | TCP_RST))) {
// The connection is closing or closed. Mark the connection as closing. Update the flags seen and last report time.
WRITE_ONCE(entry->is_closing, true);
if (direction == CT_PACKET_DIR_TX) {
WRITE_ONCE(entry->flags_seen_tx_dir, flags);
WRITE_ONCE(entry->last_report_tx_dir, now);
} else {
WRITE_ONCE(entry->flags_seen_rx_dir, flags);
WRITE_ONCE(entry->last_report_rx_dir, now);
}
// The connection is closing or closed. Delete the connection from the map
bpf_map_delete_elem(&retina_conntrack, key);

return true; // Report the last packet received.
}
// Update the eviction time of the connection.
Expand Down Expand Up @@ -287,7 +279,8 @@ static __always_inline bool _ct_should_report_packet(struct ct_entry *entry, __u
* @arg observation_point The point in the network stack where the packet is observed.
* Returns true if the packet should be report to userspace. False otherwise.
*/
static __always_inline __attribute__((unused)) bool ct_process_packet(struct packet *p, __u8 observation_point) {
static __always_inline __attribute__((unused)) bool ct_process_packet(struct packet *p, __u8 observation_point) {

if (!p) {
return false;
}
Expand All @@ -307,14 +300,13 @@ static __always_inline __attribute__((unused)) bool ct_process_packet(struct pac
// Update the packet accordingly.
p->is_reply = false;
p->traffic_direction = entry->traffic_direction;
return _ct_should_report_packet(entry, p->flags, CT_PACKET_DIR_TX, key.proto);
return _ct_should_report_packet(entry, p->flags, CT_PACKET_DIR_TX, &key);
}

// The connection is not found in the send direction. Check the reply direction by reversing the key.
struct ct_v4_key reverse_key;
__builtin_memset(&reverse_key, 0, sizeof(struct ct_v4_key));
_ct_reverse_key(&reverse_key, &key);

// Lookup the connection in the map based on the reverse key.
entry = bpf_map_lookup_elem(&retina_conntrack, &reverse_key);

Expand All @@ -323,7 +315,7 @@ static __always_inline __attribute__((unused)) bool ct_process_packet(struct pac
// Update the packet accordingly.
p->is_reply = true;
p->traffic_direction = entry->traffic_direction;
return _ct_should_report_packet(entry, p->flags, CT_PACKET_DIR_RX, key.proto);
return _ct_should_report_packet(entry, p->flags, CT_PACKET_DIR_RX, &key);
}

// If the connection is still not found, the connection is new.
Expand Down
2 changes: 1 addition & 1 deletion pkg/plugin/conntrack/conntrack_bpfel_x86.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions pkg/plugin/conntrack/conntrack_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,12 @@ func (ct *Conntrack) Run(ctx context.Context) error {
var noOfCtEntries, entriesDeleted int
// List of keys to be deleted
var keysToDelete []conntrackCtV4Key

iter := ct.ctMap.Iterate()
for iter.Next(&key, &value) {
noOfCtEntries++
// Check if the connection is closing or has expired
if value.IsClosing || ktime.MonotonicOffset.Seconds()+float64(value.EvictionTime) < float64((time.Now().Unix())) {
if ktime.MonotonicOffset.Seconds()+float64(value.EvictionTime) < float64((time.Now().Unix())) {
// Iterating a hash map from which keys are being deleted is not safe.
// So, we store the keys to be deleted in a list and delete them after the iteration.
keyCopy := key // Copy the key to avoid using the same key in the next iteration
Expand All @@ -115,7 +116,6 @@ func (ct *Conntrack) Run(ctx context.Context) error {
zap.String("proto", decodeProto(key.Proto)),
zap.Uint32("eviction_time", value.EvictionTime),
zap.Uint8("traffic_direction", value.TrafficDirection),
zap.Bool("is_closing", value.IsClosing),
zap.String("flags_seen_tx_dir", decodeFlags(value.FlagsSeenTxDir)),
zap.String("flags_seen_rx_dir", decodeFlags(value.FlagsSeenRxDir)),
zap.Uint32("last_reported_tx_dir", value.LastReportTxDir),
Expand Down
2 changes: 1 addition & 1 deletion pkg/plugin/packetparser/packetparser_bpfel_x86.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 3983a57

Please sign in to comment.