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

Requeue packets on network error #352

Closed
kpp opened this issue Mar 28, 2019 · 4 comments
Closed

Requeue packets on network error #352

kpp opened this issue Mar 28, 2019 · 4 comments

Comments

@kpp
Copy link
Member

kpp commented Mar 28, 2019

    // Get a packet from the queue, and try to encapsulate it
    fn send_queued_packet<'a>(&self, dst: &'a mut [u8]) -> TunnResult<'a> {
        if let Some(packet) = self.dequeue_packet() {
            match self.tunnel_to_network(&packet, dst) {
                TunnResult::Err(_) => {
                    // On error, return packet to the queue
                    self.requeue_packet(packet);
                }
                r => return r,
            }
        }
        TunnResult::Done
}

https://github.com/cloudflare/boringtun/blob/master/src/noise/mod.rs#L220

@kurnevsky
Copy link
Member

kurnevsky commented Mar 28, 2019

tunnel_to_network returns error only if there is no session. It seems it's auxiliary method that has nothing to do with socket errors. See: https://github.com/cloudflare/boringtun/blob/e765e12613e8eb33e39e7fb0676ad5797c02c5f6/src/noise/mod.rs#L150

Or do I miss something?

@kpp
Copy link
Member Author

kpp commented Mar 28, 2019

It's an example of a pattern. Instead of dropping a packet we can requeue it to try to send it when we are online again

@kurnevsky
Copy link
Member

Almost all packets expire quite fast. It just doesn't make sense to store them if we can't send them immediately. The only packets that don't expire that come in mind - lossless net crypto packets. But for them we have different mechanism - we store them to a buffer and drop only when other side confirms that they are delivered. So I don't really see where we can apply it.

@kpp
Copy link
Member Author

kpp commented Mar 28, 2019

TCP client - need to reconnect to the server and create a new secure session.
DHT - does not matter. We will bootstrap again
lossless net crypto packets - we have a mechanism to confirm they are delivered

Seems you are right.

@kpp kpp closed this as completed Mar 28, 2019
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

No branches or pull requests

2 participants