Add github action that checks cargo doc

[hulthe]

• Warn on missing docs when running cargo doc locally
• Deny missing or broken docs when running cargo doc --release
• Run cargo doc --release in CI for various feature combinations
• Tell docs.rs to generate docs for all
  features
• Add more docs
• Fix error enums

---

This change is 

[hulthe]

@hulthe reviewed 5 files and all commit messages, and made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @hulthe).

---

-- commits line 3 at r1:
todo: revert this

[hulthe]

todo: fix all the missing docs 🫠

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

[MarkusPettersson98]

@MarkusPettersson98 reviewed 10 files and all commit messages, and made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dlon, @hulthe, and @Serock3).

---

gotatun/Cargo.toml line 22 at r1 (raw file):

all-features = true
default-target = "x86_64-unknown-linux-gnu"
targets = ["aarch64-apple-darwin", "x86_64-pc-windows-msvc"]

What do you think about adding at least one Android target? It think it is fair, since some code is only compiled for Android (e.g. the android mod in gotatun/src/udp/socket/linux.rs)

Code quote:

[package.metadata.docs.rs]
all-features = true
default-target = "x86_64-unknown-linux-gnu"
targets = ["aarch64-apple-darwin", "x86_64-pc-windows-msvc"]

[dlon]

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

---

gotatun/src/packet/mod.rs line 111 at r7 (raw file):

is

it

---

gotatun/src/packet/mod.rs line 252 at r7 (raw file):

    /// Try to cast this [`Ip`] packet into either an [`Ipv4`] or [`Ipv6`] packet.
    ///
    /// The buffer will be truncated to [`Ipv4Header::total_len`] or [`Ipv6Header::payload_length`].

Not entirely correct in the IPv6 case (missing header length)

[hulthe]

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

---

gotatun/Cargo.toml line 22 at r1 (raw file):

Previously, MarkusPettersson98 (Markus Pettersson) wrote…

What do you think about adding at least one Android target? It think it is fair, since some code is only compiled for Android (e.g. the android mod in gotatun/src/udp/socket/linux.rs)

Good catch! Done

---

gotatun/src/packet/mod.rs line 111 at r7 (raw file):

Previously, dlon (David Lönnhager) wrote…

is

it

Done

[hulthe]

@hulthe made 1 comment and resolved 1 discussion.
Reviewable status: 7 of 10 files reviewed, 1 unresolved discussion (waiting on @dlon, @MarkusPettersson98, and @Serock3).

---

gotatun/src/packet/mod.rs line 252 at r7 (raw file):

Previously, dlon (David Lönnhager) wrote…

Not entirely correct in the IPv6 case (missing header length)

Done

[MarkusPettersson98]

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

---

gotatun/src/packet/ipv4/protocol.rs line 409 at r30 (raw file):

    // This protocol doesn't seem to be documented by IANA.
    #[allow(missing_docs)]
    pub const Iplt: IpNextProtocol = IpNextProtocol(129);

Do we still need the #[allow(missing_docs)]?

Code quote:

    // This protocol doesn't seem to be documented by IANA.
    #[allow(missing_docs)]
    pub const Iplt: IpNextProtocol = IpNextProtocol(129);

---

gotatun/src/packet/udp.rs line 21 at r23 (raw file):

    /// UDP payload. The type of this is `[u8]` by default, but it may be any zerocopy type,
    /// i.e. an `Udp<WgData>`
    pub payload: Payload,

⛏️ This comment is missing a trailing dot (unlike the other field's comment)

Code quote:

    /// UDP payload. The type of this is `[u8]` by default, but it may be any zerocopy type,
    /// i.e. an `Udp<WgData>`
    pub payload: Payload,

---

gotatun/src/packet/udp.rs line 35 at r23 (raw file):

    pub length: big_endian::U16,
    /// Checksum of the UDP packet
    pub checksum: big_endian::U16,

⛏️ This comment is missing a trailing dot (unlike the other fields' comments)

Code quote:

    /// Checksum of the UDP packet
    pub checksum: big_endian::U16,

---

gotatun/src/packet/ipv4/mod.rs line 13 at r28 (raw file):

use super::util::size_must_be;

/// An Ipv4 packet.

⛏️ IPv4*

Code quote:

/// An Ipv4 packet.

---

gotatun/src/packet/ipv4/mod.rs line 30 at r28 (raw file):

    /// IPv4 `ihl` field (Internet Header Length).
    ///
    /// This determines the length in `u32`s of the IPv4 header, including optional fields.

Suggestion: This determines the length as the number of 32-bit (or 4-byte) blocks in the IPv4 header, .. is more instantly obvious to me as to what the intent of the ihl field is.

Very minor though:)

Code quote:

 /// This determines the length in `u32`s of the IPv4 header,

---

gotatun/src/packet/ipv6/mod.rs line 21 at r26 (raw file):

    /// IPv6 payload. The type of this is `[u8]` by default, but it may be any zerocopy type,
    /// e.g. a `Udp<WgData>`
    pub payload: Payload,

⛏️ This comment is missing a trailing dot (unlike the other field's comment)

Code quote:

    /// IPv6 payload. The type of this is `[u8]` by default, but it may be any zerocopy type,
    /// e.g. a `Udp<WgData>`
    pub payload: Payload,

---

gotatun/src/packet/ipv6/mod.rs line 51 at r26 (raw file):

    /// IPv6 traffic class
    #[bits(8)]
    pub traffic_class: u8,

⛏️ These comment are missing a trailing dot (unlike the other version field's comment)

Code quote:

    /// IPv6 flow label
    #[bits(20)]
    pub flow_label: u32,
    /// IPv6 traffic class
    #[bits(8)]
    pub traffic_class: u8,

[hulthe]

@hulthe partially reviewed 16 files, made 8 comments, and resolved 9 discussions.
Reviewable status: 35 of 37 files reviewed, 17 unresolved discussions (waiting on @dlon, @MarkusPettersson98, and @Serock3).

---

gotatun/src/device/uapi/mod.rs line 178 at r38 (raw file):

Previously, hulthe (Joakim Hulthe) wrote…

We could link to DeviceBuilder::with_uapi here

Nvm, it's already specified on the type itself

---

gotatun/src/packet/mod.rs line 8 at r35 (raw file):

Previously, hulthe (Joakim Hulthe) wrote…

were these links broken before?

apparently they were :o

---

gotatun/src/tun/mod.rs line 6 at r38 (raw file):

Previously, hulthe (Joakim Hulthe) wrote…

The traits are defined below. Is the crate::tun:: link really necessary?

apparently they were

[MarkusPettersson98]

So close now! 🚀

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

---

gotatun/src/device/builder.rs line 196 at r38 (raw file):

Previously, hulthe (Joakim Hulthe) wrote…

Returns an error if peer initialization fails

Eeeeh... "Peer initialization" only fails if DAITA fails. Maybe we should specify this.

Preferably we would add this doc if the daita feature is enabled (like was done above for with_ip and the tun feature)

---

gotatun/src/packet/mod.rs line 8 at r35 (raw file):

Previously, hulthe (Joakim Hulthe) wrote…

apparently they were :o

:o

---

gotatun/src/noise/errors.rs line 6 at r43 (raw file):

/// Errors that can occur during WireGuard protocol operations.
#[derive(Debug)]
pub enum WireGuardError {

⛏️ Do these doc-comments warrant a license notice header update? Before this PR, we had not touched this file basically at all iirc

Code quote:

// Copyright (c) 2019 Cloudflare, Inc. All rights reserved.
// SPDX-License-Identifier: BSD-3-Clause

/// Errors that can occur during WireGuard protocol operations.
#[derive(Debug)]
pub enum WireGuardError {

---

gotatun/src/device/uapi/command.rs line 212 at r43 (raw file):

    /// DAITA settings for this peer, if the DAITA feature is enabled.
    #[cfg(feature = "daita-uapi")]

Is it redundant to state ", if the DAITA feature is enabled."? Won't these docs only be visible in that exact case?:)

Code quote:

    /// DAITA settings for this peer, if the DAITA feature is enabled.
    #[cfg(feature = "daita-uapi")]

---

gotatun/src/device/mod.rs line 82 at r43 (raw file):

    #[error("TUN device error: {0}")]
    #[cfg(feature = "tun")]
    TunDevice(#[from] crate::tun::tun_async_device::Error),

Breaking change inc :D

I guess there are some changes to public APIs/types that should be mentioned in the changelog as well.

Code quote:

    /// TUN device error
    #[error("TUN device error: {0}")]
    #[cfg(feature = "tun")]
    TunDevice(#[from] crate::tun::tun_async_device::Error),

---

gotatun/src/device/uapi/mod.rs line 99 at r43 (raw file):

    ///
    /// Returns `Err` if the channel is closed.
    /// Returns a [`Response`] with `errno` set if the request fails.

⛏️ errno will always be set, but it will be non-zero in case of an actually error. I suppose

Same goes for the send function

Code quote:

    /// Returns a [`Response`] with `errno` set if the request fails.

[hulthe]

@hulthe made 3 comments and resolved 4 discussions.
Reviewable status: 34 of 37 files reviewed, 17 unresolved discussions (waiting on @dlon, @MarkusPettersson98, and @Serock3).

---

gotatun/src/device/builder.rs line 196 at r38 (raw file):

Previously, MarkusPettersson98 (Markus Pettersson) wrote…

Preferably we would add this doc if the daita feature is enabled (like was done above for with_ip and the tun feature)

Done.

---

gotatun/src/device/uapi/mod.rs line 99 at r43 (raw file):

Previously, MarkusPettersson98 (Markus Pettersson) wrote…

⛏️ errno will always be set, but it will be non-zero in case of an actually error. I suppose

Same goes for the send function

Done

---

gotatun/src/noise/errors.rs line 6 at r43 (raw file):

Previously, MarkusPettersson98 (Markus Pettersson) wrote…

⛏️ Do these doc-comments warrant a license notice header update? Before this PR, we had not touched this file basically at all iirc

Yes! Done.

[MarkusPettersson98]

🚀

@MarkusPettersson98 reviewed 3 files and all commit messages, and made 1 comment.
Reviewable status: all files reviewed, 17 unresolved discussions (waiting on @hulthe and @Serock3).

[MarkusPettersson98]

@MarkusPettersson98 reviewed 3 files and all commit messages, and resolved 1 discussion.
Reviewable status: all files reviewed, 16 unresolved discussions (waiting on @hulthe and @Serock3).

[Serock3]

@Serock3 partially reviewed 23 files, made 2 comments, and resolved 12 discussions.
Reviewable status: 33 of 38 files reviewed, 5 unresolved discussions (waiting on @dlon, @hulthe, and @MarkusPettersson98).

---

gotatun/src/device/builder.rs line 183 at r47 (raw file):

    }

    /// Specify the `SO_MARK` argument to the [`UdpTransportFactory`].

I guess SO_MARK is the same as fwmark?