improve batch_send error handling (#33936)

This commit is contained in:
Jeff Biseda 2023-11-01 02:39:26 -04:00 committed by GitHub
parent 136ab21f34
commit 3f805ad06d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 110 additions and 100 deletions

View File

@ -457,16 +457,17 @@ impl RepairService {
let mut batch_send_repairs_elapsed = Measure::start("batch_send_repairs_elapsed"); let mut batch_send_repairs_elapsed = Measure::start("batch_send_repairs_elapsed");
if !batch.is_empty() { if !batch.is_empty() {
if let Err(SendPktsError::IoError(err, num_failed)) = match batch_send(repair_socket, &batch) {
batch_send(repair_socket, &batch) Ok(()) => (),
{ Err(SendPktsError::IoError(err, num_failed)) => {
error!( error!(
"{} batch_send failed to send {}/{} packets first error {:?}", "{} batch_send failed to send {}/{} packets first error {:?}",
id, id,
num_failed, num_failed,
batch.len(), batch.len(),
err err
); );
}
} }
} }
batch_send_repairs_elapsed.stop(); batch_send_repairs_elapsed.stop();

View File

@ -1221,15 +1221,16 @@ impl ServeRepair {
} }
} }
if !pending_pongs.is_empty() { if !pending_pongs.is_empty() {
if let Err(SendPktsError::IoError(err, num_failed)) = match batch_send(repair_socket, &pending_pongs) {
batch_send(repair_socket, &pending_pongs) Ok(()) => (),
{ Err(SendPktsError::IoError(err, num_failed)) => {
warn!( warn!(
"batch_send failed to send {}/{} packets. First error: {:?}", "batch_send failed to send {}/{} packets. First error: {:?}",
num_failed, num_failed,
pending_pongs.len(), pending_pongs.len(),
err err
); );
}
} }
} }
} }

View File

@ -139,14 +139,16 @@ fn retransmit_to(
.filter(|addr| socket_addr_space.check(addr)) .filter(|addr| socket_addr_space.check(addr))
.collect() .collect()
}; };
if let Err(SendPktsError::IoError(ioerr, num_failed)) = multi_target_send(socket, data, &dests) match multi_target_send(socket, data, &dests) {
{ Ok(()) => (),
error!( Err(SendPktsError::IoError(ioerr, num_failed)) => {
"retransmit_to multi_target_send error: {:?}, {}/{} packets failed", error!(
ioerr, "retransmit_to multi_target_send error: {:?}, {}/{} packets failed",
num_failed, ioerr,
dests.len(), num_failed,
); dests.len(),
);
}
} }
} }

View File

@ -178,16 +178,10 @@ mod tests {
let dest_refs: Vec<_> = vec![&ip4, &ip6, &ip4]; let dest_refs: Vec<_> = vec![&ip4, &ip6, &ip4];
let sender = UdpSocket::bind("0.0.0.0:0").await.expect("bind"); let sender = UdpSocket::bind("0.0.0.0:0").await.expect("bind");
if let Err(SendPktsError::IoError(_, num_failed)) = let res = batch_send(&sender, &packet_refs[..]).await;
batch_send(&sender, &packet_refs[..]).await assert_matches!(res, Err(SendPktsError::IoError(_, /*num_failed*/ 1)));
{ let res = multi_target_send(&sender, &packets[0], &dest_refs).await;
assert_eq!(num_failed, 1); assert_matches!(res, Err(SendPktsError::IoError(_, /*num_failed*/ 1)));
}
if let Err(SendPktsError::IoError(_, num_failed)) =
multi_target_send(&sender, &packets[0], &dest_refs).await
{
assert_eq!(num_failed, 1);
}
} }
#[tokio::test] #[tokio::test]
@ -205,11 +199,12 @@ mod tests {
(&packets[3][..], &ipv4broadcast), (&packets[3][..], &ipv4broadcast),
(&packets[4][..], &ipv4local), (&packets[4][..], &ipv4local),
]; ];
if let Err(SendPktsError::IoError(ioerror, num_failed)) = match batch_send(&sender, &packet_refs[..]).await {
batch_send(&sender, &packet_refs[..]).await Ok(()) => panic!(),
{ Err(SendPktsError::IoError(ioerror, num_failed)) => {
assert_matches!(ioerror.kind(), ErrorKind::PermissionDenied); assert_matches!(ioerror.kind(), ErrorKind::PermissionDenied);
assert_eq!(num_failed, 2); assert_eq!(num_failed, 2);
}
} }
// test leading and trailing failures for batch_send // test leading and trailing failures for batch_send
@ -220,11 +215,12 @@ mod tests {
(&packets[3][..], &ipv4local), (&packets[3][..], &ipv4local),
(&packets[4][..], &ipv4broadcast), (&packets[4][..], &ipv4broadcast),
]; ];
if let Err(SendPktsError::IoError(ioerror, num_failed)) = match batch_send(&sender, &packet_refs[..]).await {
batch_send(&sender, &packet_refs[..]).await Ok(()) => panic!(),
{ Err(SendPktsError::IoError(ioerror, num_failed)) => {
assert_matches!(ioerror.kind(), ErrorKind::PermissionDenied); assert_matches!(ioerror.kind(), ErrorKind::PermissionDenied);
assert_eq!(num_failed, 3); assert_eq!(num_failed, 3);
}
} }
// test consecutive intermediate failures for batch_send // test consecutive intermediate failures for batch_send
@ -235,11 +231,12 @@ mod tests {
(&packets[3][..], &ipv4broadcast), (&packets[3][..], &ipv4broadcast),
(&packets[4][..], &ipv4local), (&packets[4][..], &ipv4local),
]; ];
if let Err(SendPktsError::IoError(ioerror, num_failed)) = match batch_send(&sender, &packet_refs[..]).await {
batch_send(&sender, &packet_refs[..]).await Ok(()) => panic!(),
{ Err(SendPktsError::IoError(ioerror, num_failed)) => {
assert_matches!(ioerror.kind(), ErrorKind::PermissionDenied); assert_matches!(ioerror.kind(), ErrorKind::PermissionDenied);
assert_eq!(num_failed, 2); assert_eq!(num_failed, 2);
}
} }
// test intermediate failures for multi_target_send // test intermediate failures for multi_target_send
@ -250,11 +247,12 @@ mod tests {
&ipv4broadcast, &ipv4broadcast,
&ipv4local, &ipv4local,
]; ];
if let Err(SendPktsError::IoError(ioerror, num_failed)) = match multi_target_send(&sender, &packets[0], &dest_refs).await {
multi_target_send(&sender, &packets[0], &dest_refs).await Ok(()) => panic!(),
{ Err(SendPktsError::IoError(ioerror, num_failed)) => {
assert_matches!(ioerror.kind(), ErrorKind::PermissionDenied); assert_matches!(ioerror.kind(), ErrorKind::PermissionDenied);
assert_eq!(num_failed, 2); assert_eq!(num_failed, 2);
}
} }
// test leading and trailing failures for multi_target_send // test leading and trailing failures for multi_target_send
@ -265,11 +263,12 @@ mod tests {
&ipv4local, &ipv4local,
&ipv4broadcast, &ipv4broadcast,
]; ];
if let Err(SendPktsError::IoError(ioerror, num_failed)) = match multi_target_send(&sender, &packets[0], &dest_refs).await {
multi_target_send(&sender, &packets[0], &dest_refs).await Ok(()) => panic!(),
{ Err(SendPktsError::IoError(ioerror, num_failed)) => {
assert_matches!(ioerror.kind(), ErrorKind::PermissionDenied); assert_matches!(ioerror.kind(), ErrorKind::PermissionDenied);
assert_eq!(num_failed, 3); assert_eq!(num_failed, 3);
}
} }
} }
} }

View File

@ -282,14 +282,10 @@ mod tests {
let dest_refs: Vec<_> = vec![&ip4, &ip6, &ip4]; let dest_refs: Vec<_> = vec![&ip4, &ip6, &ip4];
let sender = UdpSocket::bind("0.0.0.0:0").expect("bind"); let sender = UdpSocket::bind("0.0.0.0:0").expect("bind");
if let Err(SendPktsError::IoError(_, num_failed)) = batch_send(&sender, &packet_refs[..]) { let res = batch_send(&sender, &packet_refs[..]);
assert_eq!(num_failed, 1); assert_matches!(res, Err(SendPktsError::IoError(_, /*num_failed*/ 1)));
} let res = multi_target_send(&sender, &packets[0], &dest_refs);
if let Err(SendPktsError::IoError(_, num_failed)) = assert_matches!(res, Err(SendPktsError::IoError(_, /*num_failed*/ 1)));
multi_target_send(&sender, &packets[0], &dest_refs)
{
assert_eq!(num_failed, 1);
}
} }
#[test] #[test]
@ -307,11 +303,12 @@ mod tests {
(&packets[3][..], &ipv4broadcast), (&packets[3][..], &ipv4broadcast),
(&packets[4][..], &ipv4local), (&packets[4][..], &ipv4local),
]; ];
if let Err(SendPktsError::IoError(ioerror, num_failed)) = match batch_send(&sender, &packet_refs[..]) {
batch_send(&sender, &packet_refs[..]) Ok(()) => panic!(),
{ Err(SendPktsError::IoError(ioerror, num_failed)) => {
assert_matches!(ioerror.kind(), ErrorKind::PermissionDenied); assert_matches!(ioerror.kind(), ErrorKind::PermissionDenied);
assert_eq!(num_failed, 2); assert_eq!(num_failed, 2);
}
} }
// test leading and trailing failures for batch_send // test leading and trailing failures for batch_send
@ -322,11 +319,12 @@ mod tests {
(&packets[3][..], &ipv4local), (&packets[3][..], &ipv4local),
(&packets[4][..], &ipv4broadcast), (&packets[4][..], &ipv4broadcast),
]; ];
if let Err(SendPktsError::IoError(ioerror, num_failed)) = match batch_send(&sender, &packet_refs[..]) {
batch_send(&sender, &packet_refs[..]) Ok(()) => panic!(),
{ Err(SendPktsError::IoError(ioerror, num_failed)) => {
assert_matches!(ioerror.kind(), ErrorKind::PermissionDenied); assert_matches!(ioerror.kind(), ErrorKind::PermissionDenied);
assert_eq!(num_failed, 3); assert_eq!(num_failed, 3);
}
} }
// test consecutive intermediate failures for batch_send // test consecutive intermediate failures for batch_send
@ -337,11 +335,12 @@ mod tests {
(&packets[3][..], &ipv4broadcast), (&packets[3][..], &ipv4broadcast),
(&packets[4][..], &ipv4local), (&packets[4][..], &ipv4local),
]; ];
if let Err(SendPktsError::IoError(ioerror, num_failed)) = match batch_send(&sender, &packet_refs[..]) {
batch_send(&sender, &packet_refs[..]) Ok(()) => panic!(),
{ Err(SendPktsError::IoError(ioerror, num_failed)) => {
assert_matches!(ioerror.kind(), ErrorKind::PermissionDenied); assert_matches!(ioerror.kind(), ErrorKind::PermissionDenied);
assert_eq!(num_failed, 2); assert_eq!(num_failed, 2);
}
} }
// test intermediate failures for multi_target_send // test intermediate failures for multi_target_send
@ -352,11 +351,12 @@ mod tests {
&ipv4broadcast, &ipv4broadcast,
&ipv4local, &ipv4local,
]; ];
if let Err(SendPktsError::IoError(ioerror, num_failed)) = match multi_target_send(&sender, &packets[0], &dest_refs) {
multi_target_send(&sender, &packets[0], &dest_refs) Ok(()) => panic!(),
{ Err(SendPktsError::IoError(ioerror, num_failed)) => {
assert_matches!(ioerror.kind(), ErrorKind::PermissionDenied); assert_matches!(ioerror.kind(), ErrorKind::PermissionDenied);
assert_eq!(num_failed, 2); assert_eq!(num_failed, 2);
}
} }
// test leading and trailing failures for multi_target_send // test leading and trailing failures for multi_target_send
@ -367,11 +367,12 @@ mod tests {
&ipv4local, &ipv4local,
&ipv4broadcast, &ipv4broadcast,
]; ];
if let Err(SendPktsError::IoError(ioerror, num_failed)) = match multi_target_send(&sender, &packets[0], &dest_refs) {
multi_target_send(&sender, &packets[0], &dest_refs) Ok(()) => panic!(),
{ Err(SendPktsError::IoError(ioerror, num_failed)) => {
assert_matches!(ioerror.kind(), ErrorKind::PermissionDenied); assert_matches!(ioerror.kind(), ErrorKind::PermissionDenied);
assert_eq!(num_failed, 3); assert_eq!(num_failed, 3);
}
} }
} }
} }

View File

@ -466,9 +466,12 @@ pub fn broadcast_shreds(
transmit_stats.shred_select += shred_select.as_us(); transmit_stats.shred_select += shred_select.as_us();
let mut send_mmsg_time = Measure::start("send_mmsg"); let mut send_mmsg_time = Measure::start("send_mmsg");
if let Err(SendPktsError::IoError(ioerr, num_failed)) = batch_send(s, &packets[..]) { match batch_send(s, &packets[..]) {
transmit_stats.dropped_packets_udp += num_failed; Ok(()) => (),
result = Err(Error::Io(ioerr)); Err(SendPktsError::IoError(ioerr, num_failed)) => {
transmit_stats.dropped_packets_udp += num_failed;
result = Err(Error::Io(ioerr));
}
} }
send_mmsg_time.stop(); send_mmsg_time.stop();
transmit_stats.send_mmsg_elapsed += send_mmsg_time.as_us(); transmit_stats.send_mmsg_elapsed += send_mmsg_time.as_us();

View File

@ -376,8 +376,11 @@ impl BroadcastRun for BroadcastDuplicatesRun {
.flatten() .flatten()
.collect(); .collect();
if let Err(SendPktsError::IoError(ioerr, _)) = batch_send(sock, &packets) { match batch_send(sock, &packets) {
return Err(Error::Io(ioerr)); Ok(()) => (),
Err(SendPktsError::IoError(ioerr, _)) => {
return Err(Error::Io(ioerr));
}
} }
Ok(()) Ok(())
} }