On Sun, Dec 18, 2022 at 05:53:25PM +0100, Marco Pfatschbacher wrote: > > On Sun, Dec 18, 2022 at 02:00:24PM +0100, Theo Buehler wrote: > > This is the remaining bit of mpf's recent netcat diff. The commit log > > shows that it was bumped to 64k in the past, but that was promptly > > reverted due to concerns of buffer bloat caused by atomicio blocking > > traffic in the other direction. > > Thanks for taking care of this. > I should've done some research myself before suggesting this. > > For reference: > > revision 1.121 > date: 2014/06/10 16:35:42; author: tedu; state: Exp; lines: +2 -2; > stick with 16k buffers for a little while to avoid bufferbloat. > atomicio writing out 64k in one direction will cause traffic in the > other direction to stall until it's complete. discussion with deraadt > ---------------------------- > revision 1.120 > date: 2014/06/10 16:23:07; author: tedu; state: Exp; lines: +3 -3; > increase buffer size to 64k, and actually use it. ok deraadt > from John-Mark Gurney > > > I don't know if things are different enough 8 years later that this can > > be reconsidered. Not my area, just throwing it out there so it doesn't > > get lost. > > I can only assume that the concern was about talking to a server > that would block while trying to send a response after only reading a part of > the data. > I can't think of a real world scenario where this is a problem right now. > > But since my use case (sending a huge UDP packet) also depends on bumping > the socket buffer size with -O, we could also make the BUFSIZE conditional on > the provided socket buffer size, or give this a new option. >
The problem described is that atomicio() stalls until all of 64k is written. With a small socketbuffer size this requires the receiver to read the data and so calling atomicio() with a big buffer can stall nc. What confuses me is that atomicio() is not used in the main readwrite() loop. There nc polls on both fds and then read/write depending on return values. atomicio() is only used by atelnet() and socks_connect() which do not depend on BUFSIZE. I assume this was modified when TLS support was added but I did not investigate further. -- :wq Claudio
