fix(iroh): Re-batch received relay datagram batches in case they exceed max_receive_segments
#3414
Conversation
|
Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh/pr/3414/docs/iroh/ Last updated: 2025-07-29T13:51:06Z |
matheus23
force-pushed
the
matheus23/fragment-datagrams
branch
from
1ba8ebc to
1cae9a2
Compare
| /// will result in making `self` empty and returning essentially a clone of `self`. | ||
| /// | ||
| /// Calling this on an empty datagram batch (i.e. one where `contents.is_empty()`) will return `None`. | ||
| pub fn take_segments(&mut self, num_segments: usize) -> Option<Datagrams> { |
| segment_size = ?dm.datagrams.segment_size, | ||
| "dropping received datagram: quinn buffer too small" | ||
| ); | ||
| break; |
There was a problem hiding this comment.
One, in my opinion large, downside of your approach is that you have to plumb the signal of how large this an be all the way through to the ActiveRelayActor. I find all that plumbing really unfortunate and complex.
Have you considered having an Option<Datagrams> on the RelayTransport itself? I think that way you could store the split off ones here locally. When you get called next you first consume that and then continue reading off the channel. Would that not be a lot simpler?
There was a problem hiding this comment.
Didn't consider that.
But thinking about how I'd do this, I'm somewhat afraid of messing up the waking logic in that case.
I'd prefer not to have to mess with wakers :S
There was a problem hiding this comment.
FWIW I think this is very similar to the ipv6_reported: Arc<AtomicBool> we already use. I tried to move the max_receive_segments declarations close to that if possible.
There was a problem hiding this comment.
Do you really need to mess with wakers? When you receive something that doesn't fit you will return a Poll::Ready and no waker is expected to be installed. Next time it gets called it again returns Poll::Ready because there's something in the Option. When the Option is empty you poll the channel and things behave as before.
There was a problem hiding this comment.
Yeah, I think you're right. I'll look into following up this PR with a little bit of a cleanup.
Description
Fixes a panic in
RelayTransport::poll_recvwith the new relay code.Since #3389 we forward the whole GSO batch via the relay. This then gets read on the other receiving endpoint, which tries to fill its
buf_out, which might not be big enough, because its size is initialized withmax_receive_segmentstaken into account, which might be different from the sending endpoint'smax_transmit_segments.So in case we get a batch that's too big, we use a
DatagramReBatcherto split up our datagram batch into chunk sizes we can consume.Change checklist