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

Rebase to remove PR #41 #57

Closed
wants to merge 21 commits into from
Closed

Conversation

sippejw
Copy link
Contributor

@sippejw sippejw commented Sep 9, 2024

Description

This PR removes #41 from the git history. Effectively adding the QUIC parsing commits back.

Testing

I ran loq_quic for 5 minutes on live traffic:

mempool_0 avail: 8356078, in use: 32529 (0.388%)
Ingress: 0 bps / 0 pps
Good:    49243676552 bps / 4992356 pps
Process: 49243676552 bps / 4992356 pps
Drop: 1023980 pps (inf%)
HW Dropped: 0 pkts (NaN%)
SW Dropped: 265607773 pkts (inf%)
Total Dropped: 265607773 pkts (inf%)
----------------------------------------------
AVERAGE Ingress: 0.000 bps / 0.000 pps
AVERAGE Good:    50812424986.685 bps / 5175306.336 pps
AVERAGE Process: 50812424986.685 bps / 5175306.336 pps
DROPPED: 265607773 pkts (inf%)

Main done. Ran for 300.107478194s
Done. Logged 358182 Quic stream to "quic.jsonl"

FYI, I'm still seeing a large number of drops for long running (>60s) captures.

@thearossman, can you confirm that this still builds on your ARM boxes? This also includes the commits for your upgraded UDP state, I'm pretty sure nothing is broken but you might want to double check.

sippejw and others added 20 commits September 9, 2024 10:54
- UDP performance is bad for connection-level subscriptions, due
  to UDP connections being re-inserted in connection table.
- This is one proposed solution: keep UDP connections around in
  the connection table in a "dropped" state until the timerwheel
  ages them out.

Testing done:
- Ran log_quic and log_dns on both live traffic (anon ips) and pcaps
- Output on live traffic was reasonable; output on pcaps was correct
- Repeated with log_* with connection-level subscriptions (Connection)
Note Retina has now been tested on a non-mlx5 nic.
@thearossman
Copy link
Collaborator

Thank you!

  • This builds on our ARM box
  • The UDP fix LGTM, the commit is the same and I re-ran the basic test (run log quic with a connection-level subscription) to confirm the performance benefit is still there too

It looks like the source tree is a little bit messed up, since there are still a few conflicts?

@sippejw
Copy link
Contributor Author

sippejw commented Sep 9, 2024

Closing this as it was cleaner to fix the merge conflicts locally. Please see PR #58.

@sippejw sippejw closed this Sep 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants