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

feat(s2n-quic-core): add XDP helper modules #1647

Merged
merged 2 commits into from
Mar 3, 2023
Merged

Conversation

camshaft
Copy link
Contributor

@camshaft camshaft commented Mar 2, 2023

Description of changes:

This change adds some XDP utilities to s2n-quic-core:

  • A raw packet decoder, which is also BPF-aware and can be compiled into a BPF program (coming later)
  • A raw packet encoder
  • A new raw path::Handle which now stores the ethernet addresses in addition to IP/port.

Call-outs:

  • I noticed that the EncoderBuffer wasn't being optimized if the compiler knew that a write would be in bounds. So I had to remove the #[forbid(unsafe)] in s2n_codec to ensure it was optimized.
  • I had to add/fix a few small things in various inet modules

Testing:

I added a bolero test that generates a bunch of random paths and packet values and encodes them and ensures they all decode to the exact same input values.

I wrote a bunch of these random packets out to a pcap file and opened them in wireshark to ensure they parsed correctly there:

Screen Shot 2023-03-01 at 8 35 47 PM

Screen Shot 2023-03-01 at 8 38 05 PM

While the pcap approach worked for initial development, longer term, I plan on adding another crate that can be used to check our encodings are correct (especially the checksums).

I've also included benchmarks to get an idea of the overhead of computing headers. As you can see, the checksums definitely slow things down. Note that we can turn off UDP checksums for IPv4 by setting it to 0 but they are required for IPv6. I need to do more research to see if AF_XDP will support checksum offload, which would be ideal.

xdp/encoder/ipv4/1500   time:   [40.017 ns 40.062 ns 40.112 ns]
                        thrpt:  [24.930 Melem/s 24.961 Melem/s 24.989 Melem/s]
xdp/encoder/ipv4/9000   time:   [163.52 ns 163.64 ns 163.78 ns]
                        thrpt:  [6.1057 Melem/s 6.1108 Melem/s 6.1154 Melem/s]
xdp/encoder/ipv4/65536  time:   [1.0920 µs 1.0921 µs 1.0923 µs]
                        thrpt:  [915.51 Kelem/s 915.63 Kelem/s 915.75 Kelem/s]
xdp/encoder/ipv4-no-checksum/1500
                        time:   [16.500 ns 16.503 ns 16.507 ns]
                        thrpt:  [60.579 Melem/s 60.595 Melem/s 60.606 Melem/s]
xdp/encoder/ipv4-no-checksum/9000
                        time:   [15.934 ns 15.938 ns 15.941 ns]
                        thrpt:  [62.732 Melem/s 62.745 Melem/s 62.757 Melem/s]
xdp/encoder/ipv4-no-checksum/65536
                        time:   [15.571 ns 15.575 ns 15.581 ns]
                        thrpt:  [64.182 Melem/s 64.204 Melem/s 64.224 Melem/s]
xdp/encoder/ipv6/1500   time:   [51.229 ns 51.326 ns 51.435 ns]
                        thrpt:  [19.442 Melem/s 19.483 Melem/s 19.520 Melem/s]
xdp/encoder/ipv6/9000   time:   [168.06 ns 168.27 ns 168.47 ns]
                        thrpt:  [5.9357 Melem/s 5.9430 Melem/s 5.9503 Melem/s]
xdp/encoder/ipv6/65536  time:   [1.0588 µs 1.0590 µs 1.0592 µs]
                        thrpt:  [944.10 Kelem/s 944.26 Kelem/s 944.45 Kelem/s]

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@camshaft camshaft force-pushed the camshaft/xdp-core branch 2 times, most recently from 93e3714 to 4083178 Compare March 2, 2023 03:45
@camshaft camshaft marked this pull request as ready for review March 2, 2023 04:03
@camshaft camshaft requested a review from WesleyRosenblum March 2, 2023 23:01
let (header, buffer) = buffer.decode::<&ipv6::Header>()?;
let protocol = header.next_header();

// TODO parse Hop-by-hop/Options headers, for now we'll just forward the packet on to the OS
Copy link
Contributor

Choose a reason for hiding this comment

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

Would we actually use something from the Hop-by-hop/Options headers, or is the TODO saying we should support packets that have these headers

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We won't actually use anything in there but right now we'll bail on parsing if we encounter the hop-by-hop headers. For full compatibility it would be good to handle. I'm mostly trying to get ipv4 support in place ATM so I wanted to put it off for later.

assume!(
buffer.remaining_capacity()
> size_of::<ethernet::Header>()
+ size_of::<ipv6::Header>()
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there an assumption here that ipv6:Header is larger than ipv4:Header?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah. I can do a max between the two to make that clearer

@camshaft camshaft force-pushed the camshaft/xdp-core branch from 4083178 to 311cbd1 Compare March 3, 2023 17:22
@camshaft camshaft enabled auto-merge (squash) March 3, 2023 17:23
@camshaft camshaft requested a review from WesleyRosenblum March 3, 2023 17:23
@camshaft camshaft merged commit d8757ef into main Mar 3, 2023
@camshaft camshaft deleted the camshaft/xdp-core branch March 3, 2023 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants