Skip to content

Commit

Permalink
ref(iroh-net): Improve how STUN probes are run (#1642)
Browse files Browse the repository at this point in the history
## Description

This improves the branches taken when stun probes need to be run:

- If we don't have an appropriate socket, abort the entire probe set.

- If we didn't send the probe fully, error.

- In case of a plain send error: log it.

## Notes & open questions

This is probably more how this code should look like.

@dignifiedquire feel free to take this over and make any tweaks you
feel like as I'm no supposed to distract myself with this... ;)

## Change checklist

- [x] Self-review.
- [x] Documentation updates if relevant.
- [x] Tests if relevant.

---------

Co-authored-by: dignifiedquire <me@dignifiedquire.com>
  • Loading branch information
flub and dignifiedquire committed Dec 4, 2023
1 parent 57bb691 commit b95eb86
Showing 1 changed file with 41 additions and 31 deletions.
72 changes: 41 additions & 31 deletions iroh-net/src/netcheck/reportgen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -792,37 +792,47 @@ async fn run_probe(
let mut result = ProbeReport::new(probe.clone());

match probe {
Probe::StunIpv4 { .. } => {
if let Some(ref sock) = stun_sock4 {
let n = sock.send_to(&req, derp_addr).await;
inc!(NetcheckMetrics, stun_packets_sent_ipv4);
debug!(%derp_addr, send_res=?n, %txid, "sending probe StunIpv4");
// TODO: || neterror.TreatAsLostUDP(err)
if n.is_ok() && n.unwrap() == req.len() {
result.ipv4_can_send = true;

let (delay, addr) = stun_rx
.await
.map_err(|e| ProbeError::Error(e.into(), probe.clone()))?;
result.delay = Some(delay);
result.addr = Some(addr);
}
}
}
Probe::StunIpv6 { .. } => {
if let Some(ref pc6) = stun_sock6 {
let n = pc6.send_to(&req, derp_addr).await;
inc!(NetcheckMetrics, stun_packets_sent_ipv6);
debug!(%derp_addr, snd_res=?n, %txid, "sending probe StunIpv6");
// TODO: || neterror.TreatAsLostUDP(err)
if n.is_ok() && n.unwrap() == req.len() {
result.ipv6_can_send = true;

let (delay, addr) = stun_rx
.await
.map_err(|e| ProbeError::Error(e.into(), probe.clone()))?;
result.delay = Some(delay);
result.addr = Some(addr);
Probe::StunIpv4 { .. } | Probe::StunIpv6 { .. } => {
let maybe_sock = if matches!(probe, Probe::StunIpv4 { .. }) {
stun_sock4.as_ref()
} else {
stun_sock6.as_ref()
};
match maybe_sock {
Some(sock) => match sock.send_to(&req, derp_addr).await {
Ok(n) if n == req.len() => {
debug!(%derp_addr, %txid, "sending probe {}", probe.proto());

if matches!(probe, Probe::StunIpv4 { .. }) {
result.ipv4_can_send = true;
inc!(NetcheckMetrics, stun_packets_sent_ipv4);
} else {
result.ipv6_can_send = true;
inc!(NetcheckMetrics, stun_packets_sent_ipv6);
}
let (delay, addr) = stun_rx
.await
.map_err(|e| ProbeError::Error(e.into(), probe.clone()))?;
result.delay = Some(delay);
result.addr = Some(addr);
}
Ok(n) => {
let err = anyhow!("Failed to send full STUN request: {}", probe.proto());
error!(%derp_addr, sent_len=n, req_len=req.len(), "{err:#}");
return Err(ProbeError::Error(err, probe.clone()));
}
Err(err) => {
let err = anyhow::Error::new(err)
.context(format!("Failed to send STUN request: {}", probe.proto()));
error!(%derp_addr, "{err:#}");
return Err(ProbeError::Error(err, probe.clone()));
}
},
None => {
return Err(ProbeError::AbortSet(
anyhow!("No socket for {}, aborting probeset", probe.proto()),
probe.clone(),
));
}
}
}
Expand Down

0 comments on commit b95eb86

Please sign in to comment.