Skip to content

Commit dd43c8f

Browse files
committed
iface: make poll() process all packets, add fine-grained poll functions.
This makes `.poll()` behave the same as before #954. Users affected by DoS concerns can use the finer-grained egress-only and single-packet-ingress-only fns.
1 parent 3acd511 commit dd43c8f

File tree

4 files changed

+145
-74
lines changed

4 files changed

+145
-74
lines changed

src/iface/interface/ipv4.rs

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,28 +7,23 @@ impl Interface {
77
/// processed or emitted, and thus, whether the readiness of any socket might
88
/// have changed.
99
#[cfg(feature = "proto-ipv4-fragmentation")]
10-
pub(super) fn ipv4_egress<D>(&mut self, device: &mut D) -> bool
11-
where
12-
D: Device + ?Sized,
13-
{
10+
pub(super) fn ipv4_egress(&mut self, device: &mut (impl Device + ?Sized)) {
1411
// Reset the buffer when we transmitted everything.
1512
if self.fragmenter.finished() {
1613
self.fragmenter.reset();
1714
}
1815

1916
if self.fragmenter.is_empty() {
20-
return false;
17+
return;
2118
}
2219

2320
let pkt = &self.fragmenter;
2421
if pkt.packet_len > pkt.sent_bytes {
2522
if let Some(tx_token) = device.transmit(self.inner.now) {
2623
self.inner
2724
.dispatch_ipv4_frag(tx_token, &mut self.fragmenter);
28-
return true;
2925
}
3026
}
31-
false
3227
}
3328
}
3429

src/iface/interface/mod.rs

Lines changed: 140 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,44 @@ macro_rules! check {
6565
}
6666
use check;
6767

68+
/// Result returned by [`Interface::poll`].
69+
///
70+
/// This contains information on whether socket states might have changed.
71+
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
72+
#[cfg_attr(feature = "defmt", derive(defmt::Format))]
73+
pub enum PollResult {
74+
/// Socket state is guaranteed to not have changed.
75+
None,
76+
/// You should check the state of sockets again for received data or completion of operations.
77+
SocketStateChanged,
78+
}
79+
80+
/// Result returned by [`Interface::poll_ingress_single`].
81+
///
82+
/// This contains information on whether a packet was processed or not,
83+
/// and whether it might've affected socket states.
84+
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
85+
#[cfg_attr(feature = "defmt", derive(defmt::Format))]
86+
pub enum PollIngressSingleResult {
87+
/// No packet was processed. You don't need to call [`Interface::poll_ingress_single`]
88+
/// again, until more packets arrive.
89+
///
90+
/// Socket state is guaranteed to not have changed.
91+
None,
92+
/// A packet was processed.
93+
///
94+
/// There may be more packets in the device's RX queue, so you should call [`Interface::poll_ingress_single`] again.
95+
///
96+
/// Socket state is guaranteed to not have changed.
97+
PacketProcessed,
98+
/// A packet was processed, which might have caused socket state to change.
99+
///
100+
/// There may be more packets in the device's RX queue, so you should call [`Interface::poll_ingress_single`] again.
101+
///
102+
/// You should check the state of sockets again for received data or completion of operations.
103+
SocketStateChanged,
104+
}
105+
68106
/// A network interface.
69107
///
70108
/// The network interface logically owns a number of other data structures; to avoid
@@ -150,10 +188,7 @@ impl Interface {
150188
/// # Panics
151189
/// This function panics if the [`Config::hardware_address`] does not match
152190
/// the medium of the device.
153-
pub fn new<D>(config: Config, device: &mut D, now: Instant) -> Self
154-
where
155-
D: Device + ?Sized,
156-
{
191+
pub fn new(config: Config, device: &mut (impl Device + ?Sized), now: Instant) -> Self {
157192
let caps = device.capabilities();
158193
assert_eq!(
159194
config.hardware_addr.medium(),
@@ -375,59 +410,107 @@ impl Interface {
375410
self.fragments.reassembly_timeout = timeout;
376411
}
377412

378-
/// Transmit packets queued in the given sockets, and receive packets queued
413+
/// Transmit packets queued in the sockets, and receive packets queued
379414
/// in the device.
380415
///
381-
/// This function returns a boolean value indicating whether any packets were
382-
/// processed or emitted, and thus, whether the readiness of any socket might
383-
/// have changed.
416+
/// This function returns a value indicating whether the state of any socket
417+
/// might have changed.
418+
///
419+
/// ## DoS warning
384420
///
385-
/// # Note
386-
/// This function performs a bounded amount of work per call to avoid
387-
/// starving other tasks of CPU time. If it returns true, there may still be
388-
/// packets to be received or transmitted. Depending on system design,
389-
/// calling this function in a loop may cause a denial of service if
390-
/// packets cannot be processed faster than they arrive.
391-
pub fn poll<D>(
421+
/// This function processes all packets in the device's queue. This can
422+
/// be an unbounded amount of work if packets arrive faster than they're
423+
/// processed.
424+
///
425+
/// If this is a concern for your application (i.e. your environment doesn't
426+
/// have preemptive scheduling, or `poll()` is called from a main loop where
427+
/// other important things are processed), you may use the lower-level methods
428+
/// [`poll_egress()`](Self::poll_egress) and [`poll_ingress_single()`](Self::poll_ingress_single).
429+
/// This allows you to insert yields or process other events between processing
430+
/// individual ingress packets.
431+
pub fn poll(
392432
&mut self,
393433
timestamp: Instant,
394-
device: &mut D,
434+
device: &mut (impl Device + ?Sized),
395435
sockets: &mut SocketSet<'_>,
396-
) -> bool
397-
where
398-
D: Device + ?Sized,
399-
{
436+
) -> PollResult {
400437
self.inner.now = timestamp;
401438

439+
let mut res = PollResult::None;
440+
402441
#[cfg(feature = "_proto-fragmentation")]
403442
self.fragments.assembler.remove_expired(timestamp);
404443

444+
// Process ingress while there's packets available.
445+
loop {
446+
match self.socket_ingress(device, sockets) {
447+
PollIngressSingleResult::None => break,
448+
PollIngressSingleResult::PacketProcessed => {}
449+
PollIngressSingleResult::SocketStateChanged => res = PollResult::SocketStateChanged,
450+
}
451+
}
452+
453+
// Process egress.
454+
match self.poll_egress(timestamp, device, sockets) {
455+
PollResult::None => {}
456+
PollResult::SocketStateChanged => res = PollResult::SocketStateChanged,
457+
}
458+
459+
res
460+
}
461+
462+
/// Transmit packets queued in the sockets.
463+
///
464+
/// This function returns a value indicating whether the state of any socket
465+
/// might have changed.
466+
///
467+
/// This is guaranteed to always perform a bounded amount of work.
468+
pub fn poll_egress(
469+
&mut self,
470+
timestamp: Instant,
471+
device: &mut (impl Device + ?Sized),
472+
sockets: &mut SocketSet<'_>,
473+
) -> PollResult {
474+
self.inner.now = timestamp;
475+
405476
match self.inner.caps.medium {
406477
#[cfg(feature = "medium-ieee802154")]
407-
Medium::Ieee802154 =>
408-
{
478+
Medium::Ieee802154 => {
409479
#[cfg(feature = "proto-sixlowpan-fragmentation")]
410-
if self.sixlowpan_egress(device) {
411-
return true;
412-
}
480+
self.sixlowpan_egress(device);
413481
}
414482
#[cfg(any(feature = "medium-ethernet", feature = "medium-ip"))]
415-
_ =>
416-
{
483+
_ => {
417484
#[cfg(feature = "proto-ipv4-fragmentation")]
418-
if self.ipv4_egress(device) {
419-
return true;
420-
}
485+
self.ipv4_egress(device);
421486
}
422487
}
423488

424-
let mut readiness_may_have_changed = self.socket_ingress(device, sockets);
425-
readiness_may_have_changed |= self.socket_egress(device, sockets);
426-
427489
#[cfg(feature = "multicast")]
428490
self.multicast_egress(device);
429491

430-
readiness_may_have_changed
492+
self.socket_egress(device, sockets)
493+
}
494+
495+
/// Process one incoming packet queued in the device.
496+
///
497+
/// Returns a value indicating:
498+
/// - whether a packet was processed, in which case you have to call this method again in case there's more packets queued.
499+
/// - whether the state of any socket might have changed.
500+
///
501+
/// Since it processes at most one packet, this is guaranteed to always perform a bounded amount of work.
502+
pub fn poll_ingress_single(
503+
&mut self,
504+
timestamp: Instant,
505+
device: &mut (impl Device + ?Sized),
506+
sockets: &mut SocketSet<'_>,
507+
) -> PollIngressSingleResult {
508+
self.inner.now = timestamp;
509+
510+
#[cfg(feature = "_proto-fragmentation")]
511+
self.fragments.assembler.remove_expired(timestamp);
512+
513+
self.socket_ingress(device, sockets)
431514
}
432515

433516
/// Return a _soft deadline_ for calling [poll] the next time.
@@ -480,20 +563,19 @@ impl Interface {
480563
}
481564
}
482565

483-
fn socket_ingress<D>(&mut self, device: &mut D, sockets: &mut SocketSet<'_>) -> bool
484-
where
485-
D: Device + ?Sized,
486-
{
487-
let mut processed_any = false;
488-
566+
fn socket_ingress(
567+
&mut self,
568+
device: &mut (impl Device + ?Sized),
569+
sockets: &mut SocketSet<'_>,
570+
) -> PollIngressSingleResult {
489571
let Some((rx_token, tx_token)) = device.receive(self.inner.now) else {
490-
return processed_any;
572+
return PollIngressSingleResult::None;
491573
};
492574

493575
let rx_meta = rx_token.meta();
494576
rx_token.consume(|frame| {
495577
if frame.is_empty() {
496-
return;
578+
return PollIngressSingleResult::PacketProcessed;
497579
}
498580

499581
match self.inner.caps.medium {
@@ -543,24 +625,30 @@ impl Interface {
543625
}
544626
}
545627
}
546-
processed_any = true;
547-
});
548628

549-
processed_any
629+
// TODO: Propagate the PollIngressSingleResult from deeper.
630+
// There's many received packets that we process but can't cause sockets
631+
// to change state. For example IP fragments, multicast stuff, ICMP pings
632+
// if they dont't match any raw socket...
633+
// We should return `PacketProcessed` for these to save the user from
634+
// doing useless socket polls.
635+
PollIngressSingleResult::SocketStateChanged
636+
})
550637
}
551638

552-
fn socket_egress<D>(&mut self, device: &mut D, sockets: &mut SocketSet<'_>) -> bool
553-
where
554-
D: Device + ?Sized,
555-
{
639+
fn socket_egress(
640+
&mut self,
641+
device: &mut (impl Device + ?Sized),
642+
sockets: &mut SocketSet<'_>,
643+
) -> PollResult {
556644
let _caps = device.capabilities();
557645

558646
enum EgressError {
559647
Exhausted,
560648
Dispatch,
561649
}
562650

563-
let mut emitted_any = false;
651+
let mut result = PollResult::None;
564652
for item in sockets.items_mut() {
565653
if !item
566654
.meta
@@ -581,7 +669,7 @@ impl Interface {
581669
.dispatch_ip(t, meta, response, &mut self.fragmenter)
582670
.map_err(|_| EgressError::Dispatch)?;
583671

584-
emitted_any = true;
672+
result = PollResult::SocketStateChanged;
585673

586674
Ok(())
587675
};
@@ -663,7 +751,7 @@ impl Interface {
663751
Ok(()) => {}
664752
}
665753
}
666-
emitted_any
754+
result
667755
}
668756
}
669757

src/iface/interface/multicast.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -145,10 +145,7 @@ impl Interface {
145145
/// - Send join/leave packets according to the multicast group state.
146146
/// - Depending on `igmp_report_state` and the therein contained
147147
/// timeouts, send IGMP membership reports.
148-
pub(crate) fn multicast_egress<D>(&mut self, device: &mut D)
149-
where
150-
D: Device + ?Sized,
151-
{
148+
pub(crate) fn multicast_egress(&mut self, device: &mut (impl Device + ?Sized)) {
152149
// Process multicast joins.
153150
while let Some((&addr, _)) = self
154151
.inner

src/iface/interface/sixlowpan.rs

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,33 +7,24 @@ pub(crate) const MAX_DECOMPRESSED_LEN: usize = 1500;
77

88
impl Interface {
99
/// Process fragments that still need to be sent for 6LoWPAN packets.
10-
///
11-
/// This function returns a boolean value indicating whether any packets were
12-
/// processed or emitted, and thus, whether the readiness of any socket might
13-
/// have changed.
1410
#[cfg(feature = "proto-sixlowpan-fragmentation")]
15-
pub(super) fn sixlowpan_egress<D>(&mut self, device: &mut D) -> bool
16-
where
17-
D: Device + ?Sized,
18-
{
11+
pub(super) fn sixlowpan_egress(&mut self, device: &mut (impl Device + ?Sized)) {
1912
// Reset the buffer when we transmitted everything.
2013
if self.fragmenter.finished() {
2114
self.fragmenter.reset();
2215
}
2316

2417
if self.fragmenter.is_empty() {
25-
return false;
18+
return;
2619
}
2720

2821
let pkt = &self.fragmenter;
2922
if pkt.packet_len > pkt.sent_bytes {
3023
if let Some(tx_token) = device.transmit(self.inner.now) {
3124
self.inner
3225
.dispatch_ieee802154_frag(tx_token, &mut self.fragmenter);
33-
return true;
3426
}
3527
}
36-
false
3728
}
3829

3930
/// Get the 6LoWPAN address contexts.

0 commit comments

Comments
 (0)