Skip to content

Commit

Permalink
Revert "Keep dispatching packets from a socket as long as there are a…
Browse files Browse the repository at this point in the history
…ny."

This reverts commit 51b2f18.

There's no throughput difference so far as I could measure, but
this greatly increases worst-case latency. At some later point
we could perhaps pass a deadline to the poll function, but for now
reverting this is simple enough.
whitequark committed Sep 22, 2017
1 parent 51b2f18 commit 546b367
Showing 1 changed file with 29 additions and 31 deletions.
60 changes: 29 additions & 31 deletions src/iface/ethernet.rs
Original file line number Diff line number Diff line change
@@ -165,38 +165,36 @@ impl<'a, 'b, 'c, DeviceT: Device + 'a> Interface<'a, 'b, 'c, DeviceT> {
let mut limits = self.device.limits();
limits.max_transmission_unit -= EthernetFrame::<&[u8]>::header_len();

'iface: for socket in sockets.iter_mut() {
'socket: loop {
let mut device_result = Ok(());
let socket_result =
match socket {
&mut Socket::Raw(ref mut socket) =>
socket.dispatch(|response| {
device_result = self.dispatch(timestamp, Packet::Raw(response));
device_result
}),
&mut Socket::Udp(ref mut socket) =>
socket.dispatch(|response| {
device_result = self.dispatch(timestamp, Packet::Udp(response));
device_result
}),
&mut Socket::Tcp(ref mut socket) =>
socket.dispatch(timestamp, &limits, |response| {
device_result = self.dispatch(timestamp, Packet::Tcp(response));
device_result
}),
&mut Socket::__Nonexhaustive => unreachable!()
};
match (device_result, socket_result) {
(Err(Error::Exhausted), _) => break 'iface, // nowhere to transmit
(Err(Error::Unaddressable), _) => break 'socket, // no one to transmit to
(Ok(()), Err(Error::Exhausted)) => break 'socket, // nothing to transmit
(_, Err(err)) => {
net_debug!("cannot dispatch egress packet: {}", err);
return Err(err)
}
(_, Ok(())) => ()
for socket in sockets.iter_mut() {
let mut device_result = Ok(());
let socket_result =
match socket {
&mut Socket::Raw(ref mut socket) =>
socket.dispatch(|response| {
device_result = self.dispatch(timestamp, Packet::Raw(response));
device_result
}),
&mut Socket::Udp(ref mut socket) =>
socket.dispatch(|response| {
device_result = self.dispatch(timestamp, Packet::Udp(response));
device_result
}),
&mut Socket::Tcp(ref mut socket) =>
socket.dispatch(timestamp, &limits, |response| {
device_result = self.dispatch(timestamp, Packet::Tcp(response));
device_result
}),
&mut Socket::__Nonexhaustive => unreachable!()
};
match (device_result, socket_result) {
(Err(Error::Unaddressable), _) => break, // no one to transmit to
(Err(Error::Exhausted), _) => break, // nowhere to transmit
(Ok(()), Err(Error::Exhausted)) => (), // nothing to transmit
(Err(err), _) | (_, Err(err)) => {
net_debug!("cannot dispatch egress packet: {}", err);
return Err(err)
}
(Ok(()), Ok(())) => ()
}
}

2 comments on commit 546b367

@dnadlinger
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it make sense to return zero time from poll if there are still packets to transmit, though?

@dnadlinger
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, sorry, didn't realise that this is what you were hinting at in the commit message.

Please sign in to comment.