-
Notifications
You must be signed in to change notification settings - Fork 512
METRON-822 Improve Fastcapa Performance #509
Conversation
The additional commits are showing up as Github is out-of-sync with Apache. That should clear-up once they sync back up. |
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.
This looks really great; a few minor nits (and I might be talked out of some of them too if you feel strongly the other way). I'm +1 on this; this really enables the pcap ingestion functionality for us.
metron-sensors/fastcapa/src/kafka.c
Outdated
// update queue depth of this kafka connection | ||
kaf_conn_stats[conn_id].depth = rd_kafka_outq_len(rk); | ||
|
||
// TODO this should be handled by a logging lib that can handle faults and rolling the output file |
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.
Can we have a JIRA created on this?
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.
Sure
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.
Created METRON-828 to track this.
metron-sensors/fastcapa/src/args.c
Outdated
printf("[ -b BURST_SIZE ] defined as %d \n", app.burst_size); | ||
|
||
if(app.burst_size < 1 || app.burst_size > MAX_BURST_SIZE) { | ||
printf("Invalid burst size; burst=%u must be in [1, %u]. \n", app.burst_size, MAX_BURST_SIZE); |
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.
Can we send this to stderr?
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.
Actually, same comment on the error comments below.
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.
Sure, makes sense.
As a comment on the JIRA itself, I added some details of a performance test showing the probe operating successfully at ~1.1 Gbps. |
This PR contains significant improvements to the performance and scalability of Fastcapa.
This change has been tested on Cisco UCS hardware with a 10G Cisco VNIC. The probe was able to capture 1 gbps before packets started to drop. Additional performance tuning would push this ceiling much higher, but for my purposes, I just needed to reach 1 gbps. Additional work will proceed in the future to find its true performance ceiling.
To test the change yourself, simply spin-up the virtualized test environment which will deploy and validate that Fastcapa can land packets in Kafka correctly.
Pull Request Checklist
For all changes:
For code changes: