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 L2/L3/L4 inet structs #1638

Merged
merged 4 commits into from
Feb 23, 2023
Merged

Conversation

camshaft
Copy link
Contributor

@camshaft camshaft commented Feb 22, 2023

Description of changes:

When using raw sockets or AF_XDP, the userspace is in charge of complete packet construction and parsing. This includes ethernet, IP, UDP, etc.

This change implements decoding/encoding for

  • Ethernet headers, including well-known ethertype values
  • IP Protocols for well know values
  • IPv4 headers
  • IPv6 headers
  • UDP headers

Call-outs:

Some things are known to be missing at this point and will be added at a later time:

  • Checksum calculation for each of the header types
  • IPv4 Options parsing
  • IPv6 Hop-by-hop/Options parsing

Testing:

For each of the header formats, I've added:

  • Snapshot tests for monotonic byte encodings (i.e. [0, 1, 2, 3, ...])
  • Snapshot tests for filled out byte encodings (i.e. [255, 255, 255, ...])
  • Round trip decoding and encoding tests
  • Getter/setter fuzz tests for the more complicated formats (IPv4 and IPv6)

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

@camshaft camshaft marked this pull request as ready for review February 22, 2023 22:09
quic/s2n-quic-core/src/inet/ethernet.rs Show resolved Hide resolved
quic/s2n-quic-core/src/inet/ipv4.rs Show resolved Hide resolved
}
}

impl Vihl {
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be nice to explicitly call out any opaque boundaries. E.g. Vihl represents Version information in the most significant 4 bits and IHL is the least significant 4 bits. Although this is pretty easily inferred from the RFC diagram, it's nice to give people help when trying to decipher bit operations.

Copy link
Contributor

Choose a reason for hiding this comment

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

Similar thing for the Tos struct.

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 can add some notes

}
}

define_inet_type!(
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm relatively unfamiliar with the ipv4 networking stack, but this type feels particularly dense. It looks like the most significant 3 bits are reserved for reserved, DF and more fragments fields, but the fragment_offset and set_fragment_offset aren't immediately clear to me. I can tell you what bits they return, but I don't really understand why they are used for.

quic/s2n-quic-core/src/inet/ipv6.rs Show resolved Hide resolved
}
}

define_inet_type!(
Copy link
Contributor

Choose a reason for hiding this comment

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

Same thing about opaque boundaries (calling out in a comment that the version field is the 4 most significant bits, etc), especially since the RFC diagram doesn't include the field lengths.

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, nevermind, it apparently does specify lengths. I just counted that flow label was 20 bits long and assumed that wasn't accurate. So just explicitly calling that out would be nice.

@camshaft camshaft merged commit 0c33fec into main Feb 23, 2023
@camshaft camshaft deleted the camshaft/ethernet branch February 23, 2023 19:26
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