[PATCH 0/4] Fix possible truncation of frames from /dev/net/tun
These changes started off as part of the series re-introducing EPOLLET for tap event handling. That's now turned out to be of lower priority, but along the way we fixed a bug where we could truncate frames from the kernel tap interface. This is a respin of that patch, plus a few minor preliminary cleanups. Various minor tweaks based on feedback from the original posting as part of the tap EPOLLET series. David Gibson (4): tap: Split out handling of EPOLLIN events tap: Improve handling of EINTR in tap_passt_input() tap: Restructure in tap_pasta_input() tap: Don't risk truncating frames on full buffer in tap_pasta_input() tap.c | 98 +++++++++++++++++++++++++++++++++++------------------------ 1 file changed, 58 insertions(+), 40 deletions(-) -- 2.46.0
Currently, tap_handler_pas{st,ta}() check for EPOLLRDHUP, EPOLLHUP and
EPOLLERR events, then assume anything left is EPOLLIN. We have some future
cases that may want to also handle EPOLLOUT, so in preparation explicitly
handle EPOLLIN, moving the logic to a subfunction.
Signed-off-by: David Gibson
When tap_passt_input() gets an error from recv() it (correctly) does not
print any error message for EINTR, EAGAIN or EWOULDBLOCK. However in all
three cases it returns from the function. That makes sense for EAGAIN and
EWOULDBLOCK, since we then want to wait for the next EPOLLIN event before
trying again. For EINTR, however, it makes more sense to retry immediately
- as it stands we're likely to get a renewer EPOLLIN event immediately in
that case, since we're using level triggered signalling.
So, handle EINTR separately by immediately retrying until we succeed or
get a different type of error.
Signed-off-by: David Gibson
tap_pasta_input() has a rather confusing structure, using two gotos.
Remove these by restructuring the function to have the main loop condition
based on filling our buffer space, with errors or running out of data
treated as the exception, rather than the other way around. This allows
us to handle the EINTR which triggered the 'restart' goto with a continue.
The outer 'redo' was triggered if we completely filled our buffer, to flush
it and do another pass. This one is unnecessary since we don't (yet) use
EPOLLET on the tap device: if there's still more data we'll get another
event and re-enter the loop.
Along the way handle a couple of extra edge cases:
- Check for EWOULDBLOCK as well as EAGAIN for the benefit of any future
ports where those might not have the same value
- Detect EOF on the tap device and exit in that case
Signed-off-by: David Gibson
tap_pasta_input() keeps reading frames from the tap device until the
buffer is full. However, this has an ugly edge case, when we get close
to buffer full, we will provide just the remaining space as a read()
buffer. If this is shorter than the next frame to read, the tap device
will truncate the frame and discard the remainder.
Adjust the code to make sure we always have room for a maximum size frame.
Signed-off-by: David Gibson
On Fri, 6 Sep 2024 21:49:35 +1000
David Gibson
These changes started off as part of the series re-introducing EPOLLET for tap event handling. That's now turned out to be of lower priority, but along the way we fixed a bug where we could truncate frames from the kernel tap interface.
This is a respin of that patch, plus a few minor preliminary cleanups. Various minor tweaks based on feedback from the original posting as part of the tap EPOLLET series.
David Gibson (4): tap: Split out handling of EPOLLIN events tap: Improve handling of EINTR in tap_passt_input() tap: Restructure in tap_pasta_input() tap: Don't risk truncating frames on full buffer in tap_pasta_input()
Applied. -- Stefano
participants (2)
-
David Gibson
-
Stefano Brivio