Skip to content

Improve docs#46

Merged
hulthe merged 24 commits intomainfrom
improve-docs
Jan 9, 2026
Merged

Improve docs#46
hulthe merged 24 commits intomainfrom
improve-docs

Conversation

@hulthe
Copy link
Contributor

@hulthe hulthe commented Jan 7, 2026

I split out the doc improvements from #34 because i want to get merged asap


This change is Reviewable

@hulthe hulthe force-pushed the improve-docs branch 2 times, most recently from 5994d7c to e18094e Compare January 7, 2026 09:44
Copy link
Member

@dlon dlon left a comment

Choose a reason for hiding this comment

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

@dlon reviewed 8 files and all commit messages, and made 8 comments.
Reviewable status: 8 of 19 files reviewed, 8 unresolved discussions (waiting on @hulthe and @MarkusPettersson98).


gotatun/src/packet/wg.rs line 120 at r1 (raw file):
Maybe something like this:

An integer that identifies the WireGuard session for the receiving peer


gotatun/src/packet/wg.rs line 123 at r1 (raw file):
Maybe something like this:

A counter that must be incremented for every data packet to prevent replay attacks.


gotatun/src/packet/wg.rs line 288 at r1 (raw file):
Maybe something like this:

An integer that identifies the WireGuard session for the initiating peer


gotatun/src/packet/wg.rs line 362 at r1 (raw file):
Suggestion:

An integer that identifies the WireGuard session for the responding peer


gotatun/src/packet/wg.rs line 365 at r1 (raw file):
Suggestion:

An integer that identifies the WireGuard session for the initiating peer


gotatun/src/packet/wg.rs line 434 at r1 (raw file):
Suggestion:

An integer that identifies the WireGuard session for the handshake-initiating peer


gotatun/src/packet/wg.rs line 91 at r1 (raw file):

}

/// The first byte of a WireGuard packet. This indentifies its type.

identifies


gotatun/src/packet/ip.rs line 27 at r1 (raw file):

pub struct Ip {
    /// The IP version field. [Read more](IpvxVersion)
    pub header: IpvxVersion,

Should we rename this to version?

Copy link
Contributor Author

@hulthe hulthe left a comment

Choose a reason for hiding this comment

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

@hulthe made 9 comments and resolved 8 discussions.
Reviewable status: 6 of 19 files reviewed, all discussions resolved (waiting on @dlon and @MarkusPettersson98).


gotatun/src/packet/ip.rs line 27 at r1 (raw file):

Previously, dlon (David Lönnhager) wrote…

Should we rename this to version?

I slightly prefer writing packet.header.version over packet.version.version 🤷


gotatun/src/packet/wg.rs line 91 at r1 (raw file):

Previously, dlon (David Lönnhager) wrote…

identifies

Done


gotatun/src/packet/wg.rs line 120 at r1 (raw file):

Previously, dlon (David Lönnhager) wrote…

Maybe something like this:

An integer that identifies the WireGuard session for the receiving peer

Done, thanks.


gotatun/src/packet/wg.rs line 123 at r1 (raw file):

Previously, dlon (David Lönnhager) wrote…

Maybe something like this:

A counter that must be incremented for every data packet to prevent replay attacks.

Done, thanks.


gotatun/src/packet/wg.rs line 288 at r1 (raw file):

Previously, dlon (David Lönnhager) wrote…

Maybe something like this:

An integer that identifies the WireGuard session for the initiating peer

Done, thanks.


gotatun/src/packet/wg.rs line 362 at r1 (raw file):

Previously, dlon (David Lönnhager) wrote…

Suggestion:

An integer that identifies the WireGuard session for the responding peer

Done, thanks.


gotatun/src/packet/wg.rs line 365 at r1 (raw file):

Previously, dlon (David Lönnhager) wrote…

Suggestion:

An integer that identifies the WireGuard session for the initiating peer

Done, thanks.


gotatun/src/packet/wg.rs line 434 at r1 (raw file):

Previously, dlon (David Lönnhager) wrote…

Suggestion:

An integer that identifies the WireGuard session for the handshake-initiating peer

Done, thanks.

@hulthe hulthe requested review from Serock3 and dlon January 7, 2026 12:17
Copy link
Contributor

@MarkusPettersson98 MarkusPettersson98 left a comment

Choose a reason for hiding this comment

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

@MarkusPettersson98 reviewed 6 files and made 6 comments.
Reviewable status: 7 of 19 files reviewed, 5 unresolved discussions (waiting on @dlon, @hulthe, and @Serock3).


gotatun/src/packet/ip.rs line 27 at r1 (raw file):

Previously, hulthe (Joakim Hulthe) wrote…

I slightly prefer writing packet.header.version over packet.version.version 🤷

I'm with @hulthe on this one


gotatun/src/noise/mod.rs line 304 at r2 (raw file):

    }

    /// Pop the first queued packet if it exists and try to encapsulate it

What stance do we want to take on ending doc-comments with punctuation? The std lib does it (e.g. https://doc.rust-lang.org/core/primitive.str.html#method.as_str)

Code quote:

/// Pop the first queued packet if it exists and try to encapsulate it

gotatun/src/lib.rs line 8 at r2 (raw file):

// SPDX-License-Identifier: BSD-3-Clause

//! A library implementation of [WireGuard](https://www.wireguard.com/>).

Link is broken, drop trailing >

Code quote (i):

[WireGuard](https://www.wireguard.com/>)

Code snippet (ii):

[WireGuard](https://www.wireguard.com/)

gotatun/src/packet/mod.rs line 274 at r2 (raw file):

    /// The buffer will be truncated to
    /// - [`Ipv4Header::total_len`]
    /// - or [`Ipv6Header::payload_length`] + [`Ipv6Header::LEN`].

This renders a bit awkwardly. Prefer moving the or out of the list

Code quote (i):

    /// The buffer will be truncated to
    /// - [`Ipv4Header::total_len`]
    /// - or [`Ipv6Header::payload_length`] + [`Ipv6Header::LEN`].

Code snippet (ii):

    /// The buffer will be truncated to either one of these lenghts as appropriate
    /// - [`Ipv4Header::total_len`]
    /// - [`Ipv6Header::payload_length`] + [`Ipv6Header::LEN`].

gotatun/src/packet/mod.rs line 293 at r2 (raw file):

    /// The buffer will be truncated to
    /// - [`Ipv4Header::total_len`]
    /// - or [`Ipv6Header::payload_length`] + [`Ipv6Header::LEN`].

See above comment about or in a list / rendering.

Code quote:

    /// The buffer will be truncated to
    /// - [`Ipv4Header::total_len`]
    /// - or [`Ipv6Header::payload_length`] + [`Ipv6Header::LEN`].

gotatun/src/packet/mod.rs line 357 at r2 (raw file):

    ///
    /// Returns `Packet<Ipv4<Udp>>` if the packet is a valid,
    /// non-fragmented IPv4 UDP packet with no options (IHL == 5).

Should "5" be in backticks because it is a significant value?

Code quote (i):

IHL == 5

Code snippet (ii):

IHL == `5`

Copy link
Contributor

@MarkusPettersson98 MarkusPettersson98 left a comment

Choose a reason for hiding this comment

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

@MarkusPettersson98 reviewed 2 files and made 5 comments.
Reviewable status: 8 of 19 files reviewed, 9 unresolved discussions (waiting on @dlon, @hulthe, and @Serock3).


gotatun/src/packet/wg.rs line 127 at r2 (raw file):

    /// See [whitepaper](https://www.wireguard.com/papers/wireguard.pdf).
    pub counter: little_endian::U64,
}

As is done with WgData, I think the "See whitepaper." suggestion should be moved to the type's doc-string.

Code quote:

/// Header of [`WgData`].
#[derive(FromBytes, IntoBytes, KnownLayout, Unaligned, Immutable)]
#[repr(C)]
pub struct WgDataHeader {
    // INVARIANT: Must be WgPacketType::Data
    packet_type: WgPacketType,
    _reserved_zeros: [u8; 4 - size_of::<WgPacketType>()],

    /// An integer that identifies the WireGuard session for the receiving peer.
    /// See [whitepaper](https://www.wireguard.com/papers/wireguard.pdf).
    pub receiver_idx: little_endian::U32,

    /// A counter that must be incremented for every data packet to prevent replay attacks.
    /// See [whitepaper](https://www.wireguard.com/papers/wireguard.pdf).
    pub counter: little_endian::U64,
}

gotatun/src/packet/wg.rs line 308 at r2 (raw file):

    /// See [whitepaper](https://www.wireguard.com/papers/wireguard.pdf).
    pub mac2: [u8; 16],
}

I think it's enough to link to the whitepaper once, i.e. remove the links from individual fields

Code quote:

/// WireGuard handshake initialization packet.
/// See [whitepaper](https://www.wireguard.com/papers/wireguard.pdf).
#[derive(FromBytes, IntoBytes, KnownLayout, Unaligned, Immutable)]
#[repr(C, packed)]
pub struct WgHandshakeInit {
    // INVARIANT: Must be WgPacketType::HandshakeInit
    packet_type: WgPacketType,
    _reserved_zeros: [u8; 4 - size_of::<WgPacketType>()],

    /// An integer that identifies the WireGuard session for the initiating peer.
    /// See [whitepaper](https://www.wireguard.com/papers/wireguard.pdf).
    pub sender_idx: little_endian::U32,

    /// See [whitepaper](https://www.wireguard.com/papers/wireguard.pdf).
    pub unencrypted_ephemeral: [u8; 32],

    /// See [whitepaper](https://www.wireguard.com/papers/wireguard.pdf).
    pub encrypted_static: [u8; 48],

    /// See [whitepaper](https://www.wireguard.com/papers/wireguard.pdf).
    pub encrypted_timestamp: [u8; 28],

    /// See [whitepaper](https://www.wireguard.com/papers/wireguard.pdf).
    pub mac1: [u8; 16],

    /// See [whitepaper](https://www.wireguard.com/papers/wireguard.pdf).
    pub mac2: [u8; 16],
}

gotatun/src/packet/wg.rs line 383 at r2 (raw file):

    /// See [whitepaper](https://www.wireguard.com/papers/wireguard.pdf).
    pub mac2: [u8; 16],
}

I think it's enough to link to the whitepaper once, i.e. remove the links from individual fields

Code quote:

/// WireGuard handshake response packet.
/// See [whitepaper](https://www.wireguard.com/papers/wireguard.pdf).
#[derive(FromBytes, IntoBytes, KnownLayout, Unaligned, Immutable)]
#[repr(C, packed)]
pub struct WgHandshakeResp {
    // INVARIANT: Must be WgPacketType::HandshakeResp
    packet_type: WgPacketType,
    _reserved_zeros: [u8; 4 - size_of::<WgPacketType>()],

    /// An integer that identifies the WireGuard session for the responding peer.
    /// See [whitepaper](https://www.wireguard.com/papers/wireguard.pdf).
    pub sender_idx: little_endian::U32,

    /// An integer that identifies the WireGuard session for the initiating peer.
    /// See [whitepaper](https://www.wireguard.com/papers/wireguard.pdf).
    pub receiver_idx: little_endian::U32,

    /// See [whitepaper](https://www.wireguard.com/papers/wireguard.pdf).
    pub unencrypted_ephemeral: [u8; 32],

    /// See [whitepaper](https://www.wireguard.com/papers/wireguard.pdf).
    pub encrypted_nothing: [u8; 16],

    /// See [whitepaper](https://www.wireguard.com/papers/wireguard.pdf).
    pub mac1: [u8; 16],

    /// See [whitepaper](https://www.wireguard.com/papers/wireguard.pdf).
    pub mac2: [u8; 16],
}

gotatun/src/packet/wg.rs line 448 at r2 (raw file):

    /// See [whitepaper](https://www.wireguard.com/papers/wireguard.pdf).
    pub encrypted_cookie: [u8; 32],
}

I think it's enough to link to the whitepaper once, i.e. remove the links from individual fields

Code quote:

/// WireGuard cookie reply packet.
/// See [whitepaper](https://www.wireguard.com/papers/wireguard.pdf).
#[derive(FromBytes, IntoBytes, KnownLayout, Unaligned, Immutable)]
#[repr(C, packed)]
pub struct WgCookieReply {
    // INVARIANT: Must be WgPacketType::CookieReply
    packet_type: WgPacketType,
    _reserved_zeros: [u8; 4 - size_of::<WgPacketType>()],

    /// An integer that identifies the WireGuard session for the handshake-initiating peer.
    /// See [whitepaper](https://www.wireguard.com/papers/wireguard.pdf).
    pub receiver_idx: little_endian::U32,

    /// See [whitepaper](https://www.wireguard.com/papers/wireguard.pdf).
    pub nonce: [u8; 24],

    /// See [whitepaper](https://www.wireguard.com/papers/wireguard.pdf).
    pub encrypted_cookie: [u8; 32],
}

gotatun/src/packet/udp.rs line 39 at r2 (raw file):

impl UdpHeader {
    /// Length, in bytes, of a [`UdpHeader`].

⛏️ "Length of a [UdpHeader] (in bytes)." flows a bit smoother imho 👓 🐍

Code quote:

/// Length, in bytes, of a [`UdpHeader`].

Copy link
Contributor

@MarkusPettersson98 MarkusPettersson98 left a comment

Choose a reason for hiding this comment

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

@MarkusPettersson98 reviewed 11 files and all commit messages, and made 4 comments.
Reviewable status: all files reviewed, 13 unresolved discussions (waiting on @dlon, @hulthe, and @Serock3).


gotatun/src/tun/pcap.rs line 27 at r2 (raw file):

/// An implementation of [IpSend] and [IpRecv] which also dumps all packets in the pcap file
/// format to a [Write] (See [PcapStream]).

⛏️ Missing backticks on all of these items

Code quote:

/// An implementation of [IpSend] and [IpRecv] which also dumps all packets in the pcap file
/// format to a [Write] (See [PcapStream]).

gotatun/src/packet/ipv4/mod.rs line 138 at r2 (raw file):

impl Ipv4Header {
    /// Length, in bytes, of an [`Ipv4Header`].

⛏️ "Length of an [Ipv4Header] (in bytes)." flows a bit better imo 🤓

Code quote:

Length, in bytes, of an [`Ipv4Header`].

gotatun/src/packet/ipv6/mod.rs line 55 at r2 (raw file):

    #[bits(4)]
    pub version: u8,
}

Missing punctuation on some of these doc-comments

Code quote:

pub struct Ipv6VersionTrafficFlow {
    /// IPv6 flow label
    #[bits(20)]
    pub flow_label: u32,
    /// IPv6 traffic class
    #[bits(8)]
    pub traffic_class: u8,
    /// IPv6 version. This must be `6`.
    #[bits(4)]
    pub version: u8,
}

gotatun/src/tun/buffer.rs line 32 at r2 (raw file):

    /// lifetime of [Self]. Will panic if the lock is already taken.
    pub fn new<I: IpSend>(capacity: usize, inner: Arc<Mutex<I>>) -> Self {
        let (tx, mut rx) = mpsc::channel::<Packet<Ip>>(capacity);

The remark on when this function may panic could be moved to a dedicated # Panics section.

See e.g. https://doc.rust-lang.org/std/vec/struct.Vec.html#method.insert for inspo

Code quote:

    /// Create a [`BufferedIpSend`] with `capacity`.
    ///
    /// This takes an `Arc<Mutex<I>>` because the inner `I` will be re-used after [Self] is
    /// dropped. We will take the mutex lock when this function is called, and hold onto it for the
    /// lifetime of [Self]. Will panic if the lock is already taken.
    pub fn new<I: IpSend>(capacity: usize, inner: Arc<Mutex<I>>) -> Self {
        let (tx, mut rx) = mpsc::channel::<Packet<Ip>>(capacity);

Copy link
Member

@dlon dlon left a comment

Choose a reason for hiding this comment

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

Nice work 🎉

@dlon reviewed 13 files and all commit messages, and made 4 comments.
Reviewable status: all files reviewed, 14 unresolved discussions (waiting on @hulthe and @Serock3).


gotatun/src/packet/ip.rs line 27 at r1 (raw file):

Previously, MarkusPettersson98 (Markus Pettersson) wrote…

I'm with @hulthe on this one

It's completely fine. Just a bit misleading that header does not contain (most of) the IP header, IMO


gotatun/src/udp/socket/mod.rs line 105 at r2 (raw file):

Returs

Returns


gotatun/src/packet/wg.rs line 30 at r2 (raw file):

}

/// An owned WireGuard [`Packet`] where its [`WgPacketType`] is known. See [`Packet::try_into_wg`].

Nit: "whose" would sound a bit better than "where its", IMO

Copy link
Contributor

@Serock3 Serock3 left a comment

Choose a reason for hiding this comment

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

@Serock3 reviewed 12 files and all commit messages, and made 7 comments.
Reviewable status: all files reviewed, 19 unresolved discussions (waiting on @hulthe and @MarkusPettersson98).


gotatun/src/noise/mod.rs line 304 at r2 (raw file):

Previously, MarkusPettersson98 (Markus Pettersson) wrote…

What stance do we want to take on ending doc-comments with punctuation? The std lib does it (e.g. https://doc.rust-lang.org/core/primitive.str.html#method.as_str)

I think we should have punctuation.


gotatun/src/packet/udp.rs line 39 at r2 (raw file):

Previously, MarkusPettersson98 (Markus Pettersson) wrote…

⛏️ "Length of a [UdpHeader] (in bytes)." flows a bit smoother imho 👓 🐍

Agree, but skip the parenthesis


gotatun/src/packet/udp.rs line 12 at r2 (raw file):

/// A UDP packet.
///
/// This is a dynamically sized zerocopy type, which means you can compose packet types like

Should we add a read more-link to crate::packethere?


gotatun/src/packet/mod.rs line 152 at r2 (raw file):

    }

    fn buf(&self) -> &[u8] {

Should this be called as_bytes instead, to be consistent with into_bytes and buf_mut? One might expect it to return a &ByteMut otherwise, such that you could e.g. check the remaining capacity.


gotatun/src/packet/mod.rs line 167 at r2 (raw file):

    }

    /// Create a `Packet<Y>` from a `&Y`, but re-use the backing buffer of this `Packet<T>`.

Suggestion: Create a Packet<Y> from a &Y by copying its bytes into the backing buffer of this Packet<T>.


gotatun/src/packet/mod.rs line 244 at r2 (raw file):

    }

    /// Get direct mutable access to the backing buffer.

Maybe add a warning about invalidating type invariants?


gotatun/src/packet/wg.rs line 282 at r2 (raw file):

/// WireGuard handshake initialization packet.
/// See [whitepaper](https://www.wireguard.com/papers/wireguard.pdf).

Could we link to the specific subsection of the whitepaper for type definitions, i.e. 5.4.2 for handshake init?

Copy link
Contributor Author

@hulthe hulthe left a comment

Choose a reason for hiding this comment

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

@hulthe made 12 comments and resolved 8 discussions.
Reviewable status: 8 of 19 files reviewed, 11 unresolved discussions (waiting on @dlon, @MarkusPettersson98, and @Serock3).


gotatun/src/lib.rs line 8 at r2 (raw file):

Previously, MarkusPettersson98 (Markus Pettersson) wrote…

Link is broken, drop trailing >

Done.


gotatun/src/noise/mod.rs line 304 at r2 (raw file):

Previously, Serock3 (Sebastian Holmin) wrote…

I think we should have punctuation.

Me too tbh, but I haven't been very diligent with my punctuation.


gotatun/src/packet/mod.rs line 167 at r2 (raw file):

Previously, Serock3 (Sebastian Holmin) wrote…

Suggestion: Create a Packet<Y> from a &Y by copying its bytes into the backing buffer of this Packet<T>.

Sure. Reads a bit nicer.


gotatun/src/packet/mod.rs line 244 at r2 (raw file):

Previously, Serock3 (Sebastian Holmin) wrote…

Maybe add a warning about invalidating type invariants?

This method is only available for Packet<[u8]>, so no type invariants here :)


gotatun/src/packet/mod.rs line 357 at r2 (raw file):

Previously, MarkusPettersson98 (Markus Pettersson) wrote…

Should "5" be in backticks because it is a significant value?

Sure, done.


gotatun/src/packet/udp.rs line 12 at r2 (raw file):

Previously, Serock3 (Sebastian Holmin) wrote…

Should we add a read more-link to crate::packethere?

Good idea. Added to the IP packet types also.


gotatun/src/packet/udp.rs line 39 at r2 (raw file):

Previously, Serock3 (Sebastian Holmin) wrote…

Agree, but skip the parenthesis

Sure


gotatun/src/packet/wg.rs line 30 at r2 (raw file):

Previously, dlon (David Lönnhager) wrote…

Nit: "whose" would sound a bit better of "where its", IMO

👍


gotatun/src/packet/wg.rs line 127 at r2 (raw file):

Previously, MarkusPettersson98 (Markus Pettersson) wrote…

As is done with WgData, I think the "See whitepaper." suggestion should be moved to the type's doc-string.

Aight


gotatun/src/packet/wg.rs line 308 at r2 (raw file):

Previously, MarkusPettersson98 (Markus Pettersson) wrote…

I think it's enough to link to the whitepaper once, i.e. remove the links from individual fields

Yes but then I'd need to actually document each field 😅


gotatun/src/packet/ipv4/mod.rs line 138 at r2 (raw file):

Previously, MarkusPettersson98 (Markus Pettersson) wrote…

⛏️ "Length of an [Ipv4Header] (in bytes)." flows a bit better imo 🤓

Done


gotatun/src/tun/buffer.rs line 32 at r2 (raw file):

Previously, MarkusPettersson98 (Markus Pettersson) wrote…

The remark on when this function may panic could be moved to a dedicated # Panics section.

See e.g. https://doc.rust-lang.org/std/vec/struct.Vec.html#method.insert for inspo

Done.

Copy link
Contributor Author

@hulthe hulthe left a comment

Choose a reason for hiding this comment

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

@hulthe made 1 comment.
Reviewable status: 8 of 19 files reviewed, 11 unresolved discussions (waiting on @dlon, @MarkusPettersson98, and @Serock3).


gotatun/src/packet/mod.rs line 152 at r2 (raw file):

Previously, Serock3 (Sebastian Holmin) wrote…

Should this be called as_bytes instead, to be consistent with into_bytes and buf_mut? One might expect it to return a &ByteMut otherwise, such that you could e.g. check the remaining capacity.

I'm not a fan of buf either, but as_bytes collides with zerocopy::IntoBytes::as_bytes.

Copy link
Contributor Author

@hulthe hulthe left a comment

Choose a reason for hiding this comment

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

@hulthe made 1 comment.
Reviewable status: 8 of 19 files reviewed, 11 unresolved discussions (waiting on @dlon, @MarkusPettersson98, and @Serock3).


gotatun/src/packet/wg.rs line 282 at r2 (raw file):

Previously, Serock3 (Sebastian Holmin) wrote…

Could we link to the specific subsection of the whitepaper for type definitions, i.e. 5.4.2 for handshake init?

Done

Copy link
Contributor

@MarkusPettersson98 MarkusPettersson98 left a comment

Choose a reason for hiding this comment

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

Indeed! 🎉

@MarkusPettersson98 reviewed 11 files and all commit messages, made 3 comments, and resolved 3 discussions.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @hulthe and @Serock3).


gotatun/src/packet/wg.rs line 282 at r2 (raw file):

Previously, hulthe (Joakim Hulthe) wrote…

Done


gotatun/src/packet/wg.rs line 308 at r2 (raw file):

Previously, hulthe (Joakim Hulthe) wrote…

Yes but then I'd need to actually document each field 😅

I get that, but I'd rather signal that they remain to be documented by simply leaving out the current doc-comment. It will stick out more like a sore thumb that way:)

Also, one problem of the current "see whitepaper" call-to-action is that the current field names are not searchable in the whitepaper (except for mac1 and mac2). At the very least, I think we should add the "See section 5.4.2" remark. This applies to WgHandshakeResp and WgCookieReply as well

Copy link
Contributor

@Serock3 Serock3 left a comment

Choose a reason for hiding this comment

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

Very!

@Serock3 made 3 comments and resolved 2 discussions.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @hulthe).


gotatun/src/packet/mod.rs line 152 at r2 (raw file):

Previously, hulthe (Joakim Hulthe) wrote…

I'm not a fan of buf either, but as_bytes collides with zerocopy::IntoBytes::as_bytes.

Does Packet itself implement IntoBytes?


gotatun/src/packet/mod.rs line 244 at r2 (raw file):

Previously, hulthe (Joakim Hulthe) wrote…

This method is only available for Packet<[u8]>, so no type invariants here :)

Ah, right!

Copy link
Contributor

@Serock3 Serock3 left a comment

Choose a reason for hiding this comment

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

@Serock3 reviewed 11 files.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @hulthe).

Copy link
Contributor Author

@hulthe hulthe left a comment

Choose a reason for hiding this comment

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

@hulthe made 1 comment.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @Serock3).


gotatun/src/packet/mod.rs line 152 at r2 (raw file):

Previously, Serock3 (Sebastian Holmin) wrote…

Does Packet itself implement IntoBytes?

No, but it implements Deref into Ipv4 and friends, and they implement IntoBytes.

Copy link
Contributor

@MarkusPettersson98 MarkusPettersson98 left a comment

Choose a reason for hiding this comment

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

@MarkusPettersson98 reviewed 1 file and all commit messages, made 2 comments, and resolved 2 discussions.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @hulthe and @Serock3).


gotatun/src/packet/wg.rs line 308 at r2 (raw file):

Previously, MarkusPettersson98 (Markus Pettersson) wrote…

I get that, but I'd rather signal that they remain to be documented by simply leaving out the current doc-comment. It will stick out more like a sore thumb that way:)

Also, one problem of the current "see whitepaper" call-to-action is that the current field names are not searchable in the whitepaper (except for mac1 and mac2). At the very least, I think we should add the "See section 5.4.2" remark. This applies to WgHandshakeResp and WgCookieReply as well


gotatun/src/packet/wg.rs line 383 at r2 (raw file):

Previously, MarkusPettersson98 (Markus Pettersson) wrote…

I think it's enough to link to the whitepaper once, i.e. remove the links from individual fields

Copy link
Contributor Author

@hulthe hulthe left a comment

Choose a reason for hiding this comment

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

@hulthe partially reviewed 10 files, made 2 comments, and resolved 3 discussions.
Reviewable status: all files reviewed (commit messages unreviewed), 1 unresolved discussion (waiting on @Serock3).


gotatun/src/packet/mod.rs line 274 at r2 (raw file):

Previously, MarkusPettersson98 (Markus Pettersson) wrote…

This renders a bit awkwardly. Prefer moving the or out of the list

N/A anymore


gotatun/src/packet/mod.rs line 293 at r2 (raw file):

Previously, MarkusPettersson98 (Markus Pettersson) wrote…

See above comment about or in a list / rendering.

N/A anymore

Copy link
Contributor Author

@hulthe hulthe left a comment

Choose a reason for hiding this comment

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

@hulthe resolved 1 discussion.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @Serock3).

Copy link
Contributor

@MarkusPettersson98 MarkusPettersson98 left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @Serock3).

Copy link
Contributor

@Serock3 Serock3 left a comment

Choose a reason for hiding this comment

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

@Serock3 reviewed 10 files and all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @hulthe).

@hulthe hulthe merged commit fea1e23 into main Jan 9, 2026
25 checks passed
@hulthe hulthe deleted the improve-docs branch January 9, 2026 18:20
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.

4 participants