Skip to content

Commit b95eb86

Browse files
flubdignifiedquire
andauthored
ref(iroh-net): Improve how STUN probes are run (#1642)
## 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 <[email protected]>
1 parent 57bb691 commit b95eb86

File tree

1 file changed

+41
-31
lines changed

1 file changed

+41
-31
lines changed

iroh-net/src/netcheck/reportgen.rs

+41-31
Original file line numberDiff line numberDiff line change
@@ -792,37 +792,47 @@ async fn run_probe(
792792
let mut result = ProbeReport::new(probe.clone());
793793

794794
match probe {
795-
Probe::StunIpv4 { .. } => {
796-
if let Some(ref sock) = stun_sock4 {
797-
let n = sock.send_to(&req, derp_addr).await;
798-
inc!(NetcheckMetrics, stun_packets_sent_ipv4);
799-
debug!(%derp_addr, send_res=?n, %txid, "sending probe StunIpv4");
800-
// TODO: || neterror.TreatAsLostUDP(err)
801-
if n.is_ok() && n.unwrap() == req.len() {
802-
result.ipv4_can_send = true;
803-
804-
let (delay, addr) = stun_rx
805-
.await
806-
.map_err(|e| ProbeError::Error(e.into(), probe.clone()))?;
807-
result.delay = Some(delay);
808-
result.addr = Some(addr);
809-
}
810-
}
811-
}
812-
Probe::StunIpv6 { .. } => {
813-
if let Some(ref pc6) = stun_sock6 {
814-
let n = pc6.send_to(&req, derp_addr).await;
815-
inc!(NetcheckMetrics, stun_packets_sent_ipv6);
816-
debug!(%derp_addr, snd_res=?n, %txid, "sending probe StunIpv6");
817-
// TODO: || neterror.TreatAsLostUDP(err)
818-
if n.is_ok() && n.unwrap() == req.len() {
819-
result.ipv6_can_send = true;
820-
821-
let (delay, addr) = stun_rx
822-
.await
823-
.map_err(|e| ProbeError::Error(e.into(), probe.clone()))?;
824-
result.delay = Some(delay);
825-
result.addr = Some(addr);
795+
Probe::StunIpv4 { .. } | Probe::StunIpv6 { .. } => {
796+
let maybe_sock = if matches!(probe, Probe::StunIpv4 { .. }) {
797+
stun_sock4.as_ref()
798+
} else {
799+
stun_sock6.as_ref()
800+
};
801+
match maybe_sock {
802+
Some(sock) => match sock.send_to(&req, derp_addr).await {
803+
Ok(n) if n == req.len() => {
804+
debug!(%derp_addr, %txid, "sending probe {}", probe.proto());
805+
806+
if matches!(probe, Probe::StunIpv4 { .. }) {
807+
result.ipv4_can_send = true;
808+
inc!(NetcheckMetrics, stun_packets_sent_ipv4);
809+
} else {
810+
result.ipv6_can_send = true;
811+
inc!(NetcheckMetrics, stun_packets_sent_ipv6);
812+
}
813+
let (delay, addr) = stun_rx
814+
.await
815+
.map_err(|e| ProbeError::Error(e.into(), probe.clone()))?;
816+
result.delay = Some(delay);
817+
result.addr = Some(addr);
818+
}
819+
Ok(n) => {
820+
let err = anyhow!("Failed to send full STUN request: {}", probe.proto());
821+
error!(%derp_addr, sent_len=n, req_len=req.len(), "{err:#}");
822+
return Err(ProbeError::Error(err, probe.clone()));
823+
}
824+
Err(err) => {
825+
let err = anyhow::Error::new(err)
826+
.context(format!("Failed to send STUN request: {}", probe.proto()));
827+
error!(%derp_addr, "{err:#}");
828+
return Err(ProbeError::Error(err, probe.clone()));
829+
}
830+
},
831+
None => {
832+
return Err(ProbeError::AbortSet(
833+
anyhow!("No socket for {}, aborting probeset", probe.proto()),
834+
probe.clone(),
835+
));
826836
}
827837
}
828838
}

0 commit comments

Comments
 (0)