improve fake network write() efficiency #1178

chschnell · 2024-11-04T07:11:10Z

Several improvements in fake network code that deal with data flowing towards the guest OS (i.e. package encoding and downstream buffering) and TCP state mechanics.

new: improved support for TCP connection state mechanics, implemented graceful shutdown, fixed FIN flag in generated TCP packet stream (context)
new: member eth_encoder_buf[] in classes FetchNetworkAdapter and WispNetworkAdapter, a buffer allocated once per adapter instance to encode single ethernet frames
new: class GrowableRingbuffer, designed to replace array TCPConnection.send_buffer[] with a growable ringbuffer
new: added internet checksum to synthesized UDP packets (required by mTCP) in function write_udp()
replaced internet checksum calculation with more efficient algorithm from RFC1071 (chapter 4.1)
increased TCP chunk size in TCPConnection.pump() from 500 to 1460 bytes (max. TCP payload size)
added function to efficiently copy an array into a DataView using TypedArray.set(), replaces a bunch of for-loops
added function to efficiently encode a string into a DataView using TextEncoder.encodeInto()
refactored all write_...()-functions to use encoder buffer
added several named constants for ethernet/ipv4/udp/tcp-related offsets
removed several apparently unused "return true" statements

- new: class EthernetPacketEncoder, replaces all dynamically allocated write-buffers with a single, static buffer - new: class Uint8Stream, designed to replace TCPConnection.send_buffer[] with a dynamically growing ringbuffer - new: added internet checksum to synthesized UDP packets (required by mTCP) in function write_udp() - replaced internet checksum calculation with more efficient algorithm from RFC1071 (chapter 4.1) - increased TCP chunk size in TCPConnection.pump() from 500 to 1460 bytes (max. TCP payload size) - added function to efficiently copy an array into a DataView using TypedArray.set(), replaces a bunch of for-loops - added function to efficiently encode a string into a DataView using TextEncoder.encodeInto() - refactored all write_...()-functions to use EthernetPacketEncoder - added several named constants for ethernet/ipv4/udp/tcp-related offsets - removed several apparently unused "return true" statements

TCP FIN flag marks the sender's last package. This patch delays sending FIN in TCPConnection.close() until the send buffer is drained.

copy

Thanks! Overall, this looks like a reasonable set of changes.

I've left some comments.

src/browser/fake_network.js

- Renamed Uint8Stream to GrowableRingbuffer - Minimum capacity now guaranteed to be 16, and maximum (if given) to be greater or equal to minumum - Changed peek(dst_array, length) to peek(dst_array) - Removed misleading method resize(), moved code into write() - Now throws an Exception on capacity overflow error instead of console.error() - Removed debug code that is no longer needed When we have a capacity overflow the situation is as bad as if the Uint8Array constructor had failed with out-of-memory, which throws a RangeError.

- use actual dynamic TCP port range from RFC6335 - throw an Exception when pool of dynamic ports is exhausted

- removed class EthernetPacketEncoder and its global single instance ethernet_encoder - moved encoder buffer ownership to adapter (WispNetworkAdapter and FetchNetworkAdapter) - made view_{setArray,setString,setInetChecksum} global functions - changed prototype of all write_...(spec, ...) functions to uniform write_...(spec, out) - removed function make_packet(spec), added new function adapter_receive(adapter, spec) - replaced all calls of form "A.receive(make_packet(S))" with "adapter_receive(A, S)" (also in WispNetworkAdapter) Note that before this patch function make_packet() was always called in the form: adapter.receive(make_packet(spec)); This patch binds the lifetime and scope of the encoder buffer from class EthernetPacketEncoder to the WispNetworkAdapter/FetchNetworkAdapter instance "adapter".

- removed unnecessary comments around TCP_STATE_... - renamed TCPConnection.send_stream to send_buffer - calculate package length in write_udp() and write_tcp() along the same structure as in all other write_...() functions

src/browser/fake_network.js

For the most part, this patch adds graceful TCP connection shutdown behaviour to class TCPConnection in fake_network.js. Network-intense guest applications like "apt" run noticeably smoother with support for graceful TCP shutdown. Changes in detail: - completed SYN-related state mechanics (TCP connection establishment) - added FIN-related state mechanics (TCP connection shutdown) - reordered and regrouped TCP packet handling steps in TCPConnection.process() - reduced minimum IPv4 packet size from 46 to 40 bytes to reduce noise in logs (why is this even checked) - added explicit detection of TCP Keep-Alive packets to reduce noise in logs - added dbg_log() calls to trace TCP connection state (could be removed if unwanted, or maybe use smth. like LOG_TCP instead of LOG_FETCH?) - removed TCPConnection.seq_history as it was not used anywhere As should be expected, the changes related to TCP connection state only affect class TCPConnection internally and are mostly concentrated in its methods process() and pump(). A few TCP connection states are left unimplemented, reasons: - CLOSING: simultaneous close from both ends (deemed too unlikely) - TIME_WAIT: wait seconds to minutes after sending final ACK before entering CLOSED state (pointless in our specific case and would require a Timer) - LISTEN: are server sockets even supported? Both WISP and FETCH do not support them. Also, TCPConnection.connect() seems not to be called from anywhere (it used to be called from fake_tcp_connect() which was deleted in the previous patcH). As before, fake_network.js does not use any Timers (i.e. setTimeout()), the code is purely driven by outside callers. That means that there are no checks for lost packets (or resends), the internal network is assumed to be ideal (as was the case before). Even though I fixed and removed most of the TODOs I originally found in fake_network.js, I've added several new ones that are left to be discussed. Tested with V86 network providers WispNetworkAdapter and FetchNetworkAdapter, and with V86 network devices ne2k (FreeDOS) and virtio (Debian 12 32-bit).

src/browser/fake_network.js

- removed orphaned function siptolong() - refactored view_setInetChecksum() to calc_inet_checksum() - renamed view_setString() into view_set_string() and simplified it - renamed view_setArray() into view_set_array() - refactored adapter_receive() to make_packet() - bugfix: send FIN after receiving last ACK instead of tagging it to the last packet we send, fixes a corner case when TCPConnection.close() is called with empty send_buffer but with a yet to be acknowledged package in transit. Send a FIN packet in TCPConnection.close() only if both the send_buffer is empty and there is no package in transit, else delay that until we receive the last ACK.

chschnell · 2024-11-11T20:57:51Z

I have a small idea for fetch-based networking that I would first like to check with you before committing.

Currently, the full HTTP response (header and body) from fetch() is cached before being forwarded to the originating HTTP client.

My proposal is to only wait for the HTTP response headers from fetch(), and then to forward the response body asynchronously chunk by chunk to the originating HTTP client using ReadableStream and HTTP Chunked transfer encoding.

To test this, I changed on_data_http() in fetch_network.js based on a ReadableStream usage pattern from MDN, it's pretty straight-forward and it works well with my tests (all HTTP applications support chunked Transfer-Encoding to my knowledge). This would also remove the need for FetchNetworkAdapter.fetch(), I merged its relevant code into on_data_http().

The upside is that this much, much increases both the responsiveness as well as the effective speed of fetch-based networking, all the more with increasing size of the HTTP response.

The downside is that the originating HTTP client never gets to see the Content-Length of the response it is receiving, thus it cannot show any meaningful progress statistics anymore other than about what it has received so far. Any Content-Length must be ignored if any Transfer-Encoding is used, and the Content-Length from the fetch response header can be incorrect (for example, for compressed response bodies). The only way around this seems to be the prefetching like it currently is. I might be wrong, but to my knowledge it's categorically either prefetching or the loss of Content-Length (for any request from the originating HTTP client). I wasn't aware of that until I had finished implementing it.

I would of course wait with this until #1182 is merged, and include any changes from there.

I'm really undecided which side outweighs the other here. What do you think, thumbs up or down?

copy · 2024-11-11T22:20:15Z

I'm really undecided which side outweighs the other here. What do you think, thumbs up or down?

Generally thumbs up, but I'm unsure about the downsides. I wonder if HTTP Chunked transfer encoding is necessary at all. Can't the data be streamed in separate ethernet packets?

and the Content-Length from the fetch response header can be incorrect (for example, for compressed response bodies).

I believe for compressed bodies (via Content-Encoding), Content-Length indicates the length of the compressed body. As far as I know, compression via Transfer-Encoding isn't really used or widely implemented.

chschnell · 2024-11-12T07:20:52Z

EDIT: The gist of the matter is that I cannot determine whether Content-Length from the fetch() response headers is correct, so I cannot use it at all and I have no Content-Length to pass to the originating HTTP client. The only option I see at this point is to use chunked encoding.

If I could rely on Content-Length in the received fetch() response headers then I could just pass the unmodified chunks on to the originating client, passing the very same Content-Length to it.

I build and send the response headers to the originating client when I receive the first response body chunk, but I don't have a Content-Length to set in these response headers (well I do, but it is wrong when compression is used).

In fetch(), my browser negotiates gzip compression with my Apache server and transparently decompresses the response before passing me the decompressed chunks (and passes me the compressed size in Content-Length, which is useless). I never get to know how long the response is going to be myself.

I tried to suppress compression but Accept-Encoding is amongst the forbidden header names in fetch, so I can't set that header in the fetch request.

I wonder, is there any other method besides Chunked transfer encoding that I could use when I don't know the length of the response?

copy · 2024-11-12T16:29:58Z

In fetch(), my browser negotiates gzip compression with my Apache server and transparently decompresses the response before passing me the decompressed chunks (and passes me the compressed size in Content-Length, which is useless). I never get to know how long the response is going to be myself.

I see the problem now. fetch passes you already decompressed chunks, but Content-Length indicates the compressed size on the wire.

I wonder, is there any other method besides Chunked transfer encoding that I could use when I don't know the length of the response?

It seems to be legal to send an http reponse with neither Content-Length nor Transfer-Encoding: chunked, in which case the server closes the TCP connection after writing all data.

chschnell · 2024-11-12T21:39:06Z

I wonder, is there any other method besides Chunked transfer encoding that I could use when I don't know the length of the response?

It seems to be legal to send an http reponse with neither Content-Length nor Transfer-Encoding: chunked, in which case the server closes the TCP connection after writing all data.

I actually do all this to learn and also try out new things, so thank you very much for this information!

It should fit in well since I'm already closing the HTTP connection to the originating client once the transaction is complete (current fetch-based networking can't do that because of the FIN packet that is sent too early).

chschnell · 2024-11-12T21:58:16Z

It works without both Content-Length and chunked encoding! :)

Of course, the originating client still doesn't get to learn the Content-Length of the response body, so it cannot show any meaningfull download progress information besides the total bytes received so far.

See https://en.wikipedia.org/wiki/File:Tcp_state_diagram_fixed_new.svg for a full TCP state diagram, all states (except TIME_WAIT) and state transitions below ESTABLISHED are now properly implemented in class TCPConnection. - fixed misconceptions regarding TCP close states and transitions - fixed incorrect behaviour when guest closes a TCP connection - added state CLOSING to state machine - added support to pass passive close up to WispNetworkAdapter - commented out all calls to dbg_log() that are part of this PR This improves interoperability by further completing the TCP state machine model in fake_network.js.

copy · 2024-11-18T16:30:05Z

src/browser/fake_network.js

        reply.tcp.psh = true;
-        reply.tcp_data = data;
-        this.net.receive(make_packet(reply));
+        reply.tcp_data = data.subarray(0, n_ready);


I believe this line is not necessary

I'm not sure I understand. reply.tcp_data is needed, it's the TCP payload for the packet we create with make_packet() here. It is eventually used further down the call chain in write_tcp().

Sorry, I meant the .subarray() shouldn't be needed.

Well, data has a fixed capacity of 1460 bytes of which only n_ready bytes are used at this point, which can be less than the capacity.

If we remove this .subarray() then write_tcp() would somehow need to know n_ready explicitely together with data. It would need an extra field reply.tcp_data_length next to reply.tcp_data or such.

As it is only called once per generated TCP packet I would just leave it like it is. Also, write_tcp() passes this subarray to view_set_array() which also treats it as an array with implicit size.

copy · 2024-11-18T16:36:51Z

Looks good to merge to me (one more small comment above). @chschnell Any objections? (asking because it's still marked as a draft)

chschnell · 2024-11-18T18:10:37Z

Looks good to merge to me (one more small comment above). @chschnell Any objections? (asking because it's still marked as a draft)

I'm still not sure whether you want the fetch()-improvement or not, I can check in the code any time, it's not that much code and also well-tested.

The question is whether you prefer to have the performance boost or to keep the Content-Length intact.

I tend to the performance boost, but I'm in no position to make that decision.

SuperMaxusa · 2024-11-18T18:43:17Z

The question is whether you prefer to have the performance boost or to keep the Content-Length intact.

How about using the default way for small responses or when the custom x-fetch... header is set (like a fallback)?

copy · 2024-11-18T19:12:59Z

The question is whether you prefer to have the performance boost or to keep the Content-Length intact.

Yeah, let's do proper chunking and give up the Content-Length. (perhaps leave the old path in the code, so it could be toggled if there are any problems).

Optimizes writing multiple buffers to the TCPConnection at once.

- replaced blocking call to fetch() in on_data_http() with async loop, response body chunks are now forwarded as soon as they arrive - commented out old code path in on_data_http() (for fallback) - commented out FetchNetworkAdapter.fetch() (for fallback), merged into on_data_http() - removed code to make type checking happy for data argument in on_data_http(), fixed JSC declaration instead

chschnell · 2024-11-18T20:38:14Z

The question is whether you prefer to have the performance boost or to keep the Content-Length intact.

Yeah, let's do proper chunking and give up the Content-Length. (perhaps leave the old path in the code, so it could be toggled if there are any problems).

Ok, I hope this was the last commit, from my side this PR is ready to go.

I've commented out two blocks of code in fetch_network.js (for fallback), and put a single new one in. Some kind of software switch could be added to toggle between the two implementations, but I tried to keep it simple.

@SuperMaxusa It should be easy to extend the code as it is now to a two-way solution, for example to prefetch the body when it is not too large, or when the originating client's HTTP request contained a specific header. If anyone wants it I could add it in a later PR.

SuperMaxusa · 2024-11-18T21:42:33Z

It should be easy to extend the code as it is now to a two-way solution, for example to prefetch the body when it is not too large, or when the originating client's HTTP request contained a specific header.

Yes, since most HTTP clients can handle these responses without Content-Length (e.g. on KolibriOS, DSL with Dillo and Firefox, Windows 95 with Internet Explorer 4.0, this works properly), let's leave your implementation with ReadableStream.

By the way, in many CI runs, the mocked.example.org test failed (as an exception, this behaviour is allowed), while the test with real example.org is more reliable, and after the recent changes, this test doesn't work. Can we remove this test?

chschnell · 2024-11-19T18:01:33Z

e.g. on KolibriOS, DSL with Dillo and Firefox, Windows 95 with Internet Explorer 4.0, this works properly

@SuperMaxusa Thanks a lot for testing, that is really helpful of you!

By the way, this transfer model (transfer without Content-Length and to use end-of-connection to indicate end-of-response-body) is how HTTP 1.0 used to work, and support for it is still required in HTTP 1.1 (here the relevant section). I guess for these two reasons it is widely support.

chschnell · 2024-11-20T10:31:11Z

It should be easy to extend the code as it is now to a two-way solution, for example to prefetch the body when it is not too large, or when the originating client's HTTP request contained a specific header.

By the way, in many CI runs, the mocked.example.org test failed (as an exception, this behaviour is allowed), while the test with real example.org is more reliable, and after the recent changes, this test doesn't work. Can we remove this test?

Starting test #6: Curl mocked.example.org
    Captured: wget -T 10 -O - mocked.example.org
    Captured: echo -e done\\tmocked.example.org
    Captured: ~% wget -T 10 -O - mocked.example.org
    Captured: Connecting to mocked.example.org (192.168.87.1:80)
Fetch Failed: http://mocked.example.org/
TypeError: fetch failed
    Captured: wget: server returned error: HTTP/1.1 502 Fetch Error
    Captured: ~% echo -e done\\tmocked.example.org
    Captured: done	mocked.example.org
[...]

Unless there's more to it, Test #6 in tests/devices/fetch_network.js fails correctly, it attempts to fetch() from http://mocked.example.org/ which is an invalid DNS name. The next test nr. 7 attempts to fetch() from http://example.org/ wich is a valid URL, and this test succeeds.

I am not sure, but in case you mean test nr. 6, maybe it is intended to just test that a failure in fetch() is handled correctly? Was this test ever meant to succeed?

SuperMaxusa · 2024-11-20T11:26:21Z

it attempts to fetch() from http://mocked.example.org/ which is an invalid DNS name

This test has a custom fetch for which http://mocked.example.org is an existed website, see also #1061 (comment) and 6518770.

When the FetchNetworkAdapter.prototype.fetch has been commented out and replaced by fetch with ReadableStream, it's not called and the test fails.

this transfer model (transfer without Content-Length and to use end-of-connection to indicate end-of-response-body) is how HTTP 1.0 used to work, and support for it is still required in HTTP 1.1

Interesting, I would never have thought this works as HTTP 1.0 compatibility, thanks for that information!

chschnell · 2024-11-20T12:38:27Z

it attempts to fetch() from http://mocked.example.org/ which is an invalid DNS name

This test has a custom fetch for which http://mocked.example.org is an existed website, see also #1061 (comment) and 6518770.

When the FetchNetworkAdapter.prototype.fetch has been commented out and replaced by fetch with ReadableStream, it's not called and the test fails.

Ah I see, thanks for the explanation.

It's true that this test cannot work anymore because I removed FetchNetworkAdapter.fetch(), I wasn't aware of this side-effect.

I would also argue (with my limited oversight) that this test is no longer needed with the changed class layout.

chschnell · 2024-11-20T16:47:48Z

On a second thought, FetchNetworkAdapter.fetch() could stay, it no longer gets called by on_data_http() but exclusively by that test function now.

Perhaps a bit of a stretch, but maybe someone needs the blocking fetch() in the future and then FetchNetworkAdapter.fetch() would be useful to have.

ProgrammerIn-wonderland · 2024-11-21T03:47:46Z

Related note I just noticed since this touches wisp_network. If a connection was closed in wisp and the v86 VM tried sending a TCP packet it crashes the whole thing. It may be worth just adding some extra checks around that code just to make sure everything's behaving fine

This PR might fix it since it touches closing logic, I'm just not sure

Even though this method is currently only used ín a test, keep this function for possible future use.

Even though this method is conceptually correct, it is useless because neither WISP nor fetch() support closing only our half of the TCP connection.

ProgrammerIn-wonderland · 2024-11-21T13:53:18Z

Wisp does support bidirectional stream closure (I think I just screwed up how I handled it in v86), is there some issue you encountered with it? Not sending the voluntary close may cause resource leaks for both the client and the server

chschnell · 2024-11-21T17:21:39Z

Related note I just noticed since this touches wisp_network. If a connection was closed in wisp and the v86 VM tried sending a TCP packet it crashes the whole thing. It may be worth just adding some extra checks around that code just to make sure everything's behaving fine

This PR might fix it since it touches closing logic, I'm just not sure

I did not come across this when writing the PR. I mostly changed fake_network.js, wisp_network.js only minimally where neccessary.

As a test just now, I killed my WISP server (epoxy) while V86 was running without using the network, and after a few seconds it throws an exception in the browser console. Then I restarted the WISP server, and the same V86 instance picked it up just fine, network was ok.

Then I killed the WISP server while having a telnet session open in V86, caused some traffic, restarted the WISP server, tried to exit telnet which took around half a minute (two exceptions or so), but then it came back and I was able to start a new telnet session without problems, stil in the same V86 instance.

It looks stable to me (V86 survived), but maybe I'm not testing right. How can I provoke this situation you mean?

chschnell · 2024-11-21T17:58:01Z

Wisp does support bidirectional stream closure (I think I just screwed up how I handled it in v86), is there some issue you encountered with it? Not sending the voluntary close may cause resource leaks for both the client and the server

I added a WISP frame of type CLOSE with reason 0x02 (voluntary stream closure) to the V86 WISP client just a couple of days ago and removed it again a couple of hours ago after I was told yesterday (in discussion #1175) that this wasn't supported.

I could add it back in easily, I left a comment in fake_network.js in my branch where this would belong (it's where we receive a TCP packet with FIN flag from V86 while in TCP state ESTABLISHED).

The reason I added it was to close only V86's side of the TCP connection (so-called "half-close"), for reasons see here:

TCP Half-Close: a cool feature that is now broken (only sometimes "broken" in the public internet, not in a local network setup, but otherwise a good intro)
Clarify rules around half-closed TCP connections (this specific comment is precisely how I think how TCP half-close should be handled in the context of a proxy, maybe scroll up a bit for more context)

The example from the first link is quite simple (see there for the full explanation):

ssh hostname sort < datafile

This needs TCP half-close in order to work. IIRC then FTP could also be hindered from working without support for half-close.

I'm a bit confused now. Does WISP support TCP half-close? What does CLOSE with reason 0x02 (from the client) exactly do, and is it generally supported by WISP servers?

ProgrammerIn-wonderland · 2024-11-21T18:27:35Z

I've never heard of half close, okay I see, wisp doesn't support that. Does full closing still work? I thought that commit removed full closing and that's why I was concerned

ProgrammerIn-wonderland · 2024-11-21T18:36:38Z

close ~~0x02~~ 0x04 closes the TCP stream entirely, no more receiving or transmitting can be done. The server doesn't really care about the reason. It's there more so as courtesy

Edit: fix wrong packet type

chschnell · 2024-11-21T18:52:13Z

I've never heard of half close, okay I see, wisp doesn't support that. Does full closing still work? I thought that commit removed full closing and that's why I was concerned

close 0x02 closes the TCP stream entirely, no more receiving or transmitting can be done

Only WispServerCpp implements CLOSE/0x02 like that, neither epoxy-tls nor wisp-js do, in my observation. There was no CLOSE/0x02 in V86's WISP client before I added it a couple of days ago.

So in the case of WispServerCpp that commit did remove full closing, but in the case of the other two WISP servers it didn't change anything. I did that because none of them implemented half-close. Now it all comes down to a timeout by the server, or a leaked connection in case that never happens.

Maybe I should put the CLOSE/0x02 better back in, even though it will break certain applications and has no effect with some WISP servers.

ProgrammerIn-wonderland · 2024-11-21T20:19:49Z

Are you saying the other wisp servers don't support closing or don't support half closing? Sorry I think I'm misunderstanding

To my understanding every current wisp server does support full closing of streams from either direction (client and server can both report closed streams I believe)

Also close is 0x04, the reason is 0x02. Just wanted to clarify because I stated it incorrectly above in my own message

r58Playz · 2024-11-21T20:39:26Z

I've never heard of half close, okay I see, wisp doesn't support that. Does full closing still work? I thought that commit removed full closing and that's why I was concerned

close 0x02 closes the TCP stream entirely, no more receiving or transmitting can be done

Only WispServerCpp implements CLOSE/0x02 like that, neither epoxy-tls nor wisp-js do, in my observation. There was no CLOSE/0x02 in V86's WISP client before I added it a couple of days ago.

So in the case of WispServerCpp that commit did remove full closing, but in the case of the other two WISP servers it didn't change anything. I did that because none of them implemented half-close. Now it all comes down to a timeout by the server, or a leaked connection in case that never happens.

Maybe I should put the CLOSE/0x02 better back in, even though it will break certain applications and has no effect with some WISP servers.

@chschnell

The protocol does not specify any special behavior on the server side for specific close reasons. I don't know why WispServerCpp behaves differently here, but wisp-js and epoxy-server are following protocol and closing fully.
Wisp V1 reference Wisp V2 reference

Any CLOSE packets sent from either the server or the client must immediately close the associated stream and TCP socket. The close reason in the payload doesn't affect this behavior, but may provide extra information which is useful for debugging.

On epoxy, you can use RUST_LOG=trace to see that it does close streams (stream uuid f9f5dcdb-5edc-40d2-9ba7-b7686b6cfc19 disconnected for client id "127.0.0.1:46658"):

[2024-11-21T20:32:37Z TRACE epoxy_server] routed "127.0.0.1:46658": Wisp
[2024-11-21T20:32:37Z DEBUG epoxy_server::handle::wisp] new wisp client id "127.0.0.1:46658" connected with extensions [1], downgraded false
[2024-11-21T20:32:37Z TRACE epoxy_server::handle::wisp] sent ping to wisp client id "127.0.0.1:46658"
[2024-11-21T20:32:37Z TRACE epoxy_server::handle::wisp] sent ping to wisp client id "127.0.0.1:46658"
[2024-11-21T20:32:37Z DEBUG epoxy_server::handle::wisp] new stream created for client id "127.0.0.1:46658": (stream uuid f9f5dcdb-5edc-40d2-9ba7-b7686b6cfc19) ConnectPacket { stream_type: Tcp, destination_port: 8000, destination_hostname: "localhost" } ConnectPacket { stream_type: Tcp, destination_port: 8000, destination_hostname: "127.0.0.1" }
[2024-11-21T20:32:37Z DEBUG epoxy_server::handle::wisp] stream uuid f9f5dcdb-5edc-40d2-9ba7-b7686b6cfc19 disconnected for client id "127.0.0.1:46658"
[2024-11-21T20:33:07Z TRACE epoxy_server::handle::wisp] sent ping to wisp client id "127.0.0.1:46658"
[2024-11-21T20:33:37Z TRACE epoxy_server::handle::wisp] sent ping to wisp client id "127.0.0.1:46658"
[2024-11-21T20:33:59Z DEBUG epoxy_server::handle::wisp] wisp client id "127.0.0.1:46658" multiplexor result Ok(())
[2024-11-21T20:33:59Z DEBUG epoxy_server::handle::wisp] shutting down wisp client id "127.0.0.1:46658"
[2024-11-21T20:33:59Z TRACE epoxy_server::handle::wisp] waiting for tasks to close for wisp client id "127.0.0.1:46658"
[2024-11-21T20:33:59Z DEBUG epoxy_server::handle::wisp] wisp client id "127.0.0.1:46658" disconnected

ProgrammerIn-wonderland · 2024-11-21T22:34:59Z

Also wisp server Cpp sends a 0x04 Close packet back after the client sends one. It is the only server to do this, and it actually isn't in the spec. The client is supposed to assume the stream is closed immediately after sending the close packet.

Just wanted to note that on this GitHub thread

r58Playz · 2024-11-21T22:43:14Z

Another note: WispServerCpp doesn't actually close any sockets that it forwards, violating spec, so the remote server never sees the close from the wisp client.
Edit: This can also result in sending packets to sockets that were closed according to the client, emulating half-closed behavior without actually closing anything.

chschnell added 4 commits November 4, 2024 07:57

fixed eslint errors

3c28d30

fixed FIN flag in generated TCP packets

c46e34b

TCP FIN flag marks the sender's last package. This patch delays sending FIN in TCPConnection.close() until the send buffer is drained.

minor fixes

ed7bf7a

copy reviewed Nov 4, 2024

View reviewed changes

chschnell added 4 commits November 5, 2024 21:02

fixed finding unused dynamic TCP port

c1c52b8

- use actual dynamic TCP port range from RFC6335 - throw an Exception when pool of dynamic ports is exhausted

minor cleanups and alignments

d8136c1

- removed unnecessary comments around TCP_STATE_... - renamed TCPConnection.send_stream to send_buffer - calculate package length in write_udp() and write_tcp() along the same structure as in all other write_...() functions

copy reviewed Nov 8, 2024

View reviewed changes

src/browser/fake_network.js Outdated Show resolved Hide resolved

chschnell added 3 commits November 8, 2024 21:24

Merge branch 'copy:master' into fake-network-refactor

bd91c53

remove unused functions

f8af3c6

copy reviewed Nov 11, 2024

View reviewed changes

bugfix: late FIN support in case of passive TCP close was incomplete

51204e2

SuperMaxusa mentioned this pull request Nov 12, 2024

fetch-networking: fix parsing of HTTP headers #1182

Merged

chschnell marked this pull request as ready for review November 16, 2024 16:59

chschnell marked this pull request as draft November 17, 2024 15:44

chschnell added 3 commits November 17, 2024 16:48

added now required call to close() once fetch() is complete

7cbd89c

Merge branch 'copy:master' into fake-network-refactor

c5557a6

copy reviewed Nov 18, 2024

View reviewed changes

chschnell added 2 commits November 18, 2024 20:51

added method TCPConnection.writev([ Uint8Array, ... ])

00ad28d

Optimizes writing multiple buffers to the TCPConnection at once.

chschnell marked this pull request as ready for review November 18, 2024 20:28

chschnell added 2 commits November 19, 2024 19:46

removed/solved a few leftover TODOs

65ceac6

fixed eslint errors

6ad738a

chschnell added 2 commits November 21, 2024 10:40

reactivated FetchNetworkAdapter.fetch()

68b9b2c

Even though this method is currently only used ín a test, keep this function for possible future use.

removed method TCPConnection.on_passive_close()

b06c291

Even though this method is conceptually correct, it is useless because neither WISP nor fetch() support closing only our half of the TCP connection.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

improve fake network write() efficiency #1178

improve fake network write() efficiency #1178

chschnell commented Nov 4, 2024 •

edited

Loading

copy left a comment

chschnell commented Nov 11, 2024

copy commented Nov 11, 2024

chschnell commented Nov 12, 2024 •

edited

Loading

copy commented Nov 12, 2024

chschnell commented Nov 12, 2024

chschnell commented Nov 12, 2024 •

edited

Loading

copy Nov 18, 2024

chschnell Nov 18, 2024

copy Nov 18, 2024

chschnell Nov 18, 2024

copy commented Nov 18, 2024

chschnell commented Nov 18, 2024

SuperMaxusa commented Nov 18, 2024

copy commented Nov 18, 2024

chschnell commented Nov 18, 2024

SuperMaxusa commented Nov 18, 2024 •

edited

Loading

chschnell commented Nov 19, 2024

chschnell commented Nov 20, 2024 •

edited

Loading

SuperMaxusa commented Nov 20, 2024

chschnell commented Nov 20, 2024

chschnell commented Nov 20, 2024

ProgrammerIn-wonderland commented Nov 21, 2024 •

edited

Loading

ProgrammerIn-wonderland commented Nov 21, 2024 •

edited

Loading

chschnell commented Nov 21, 2024

chschnell commented Nov 21, 2024

ProgrammerIn-wonderland commented Nov 21, 2024 •

edited

Loading

ProgrammerIn-wonderland commented Nov 21, 2024 •

edited

Loading

chschnell commented Nov 21, 2024

ProgrammerIn-wonderland commented Nov 21, 2024 •

edited

Loading

r58Playz commented Nov 21, 2024

ProgrammerIn-wonderland commented Nov 21, 2024 •

edited

Loading

r58Playz commented Nov 21, 2024 •

edited

Loading

improve fake network write() efficiency #1178

Are you sure you want to change the base?

improve fake network write() efficiency #1178

Conversation

chschnell commented Nov 4, 2024 • edited Loading

copy left a comment

Choose a reason for hiding this comment

chschnell commented Nov 11, 2024

copy commented Nov 11, 2024

chschnell commented Nov 12, 2024 • edited Loading

copy commented Nov 12, 2024

chschnell commented Nov 12, 2024

chschnell commented Nov 12, 2024 • edited Loading

copy Nov 18, 2024

Choose a reason for hiding this comment

chschnell Nov 18, 2024

Choose a reason for hiding this comment

copy Nov 18, 2024

Choose a reason for hiding this comment

chschnell Nov 18, 2024

Choose a reason for hiding this comment

copy commented Nov 18, 2024

chschnell commented Nov 18, 2024

SuperMaxusa commented Nov 18, 2024

copy commented Nov 18, 2024

chschnell commented Nov 18, 2024

SuperMaxusa commented Nov 18, 2024 • edited Loading

chschnell commented Nov 19, 2024

chschnell commented Nov 20, 2024 • edited Loading

SuperMaxusa commented Nov 20, 2024

chschnell commented Nov 20, 2024

chschnell commented Nov 20, 2024

ProgrammerIn-wonderland commented Nov 21, 2024 • edited Loading

ProgrammerIn-wonderland commented Nov 21, 2024 • edited Loading

chschnell commented Nov 21, 2024

chschnell commented Nov 21, 2024

ProgrammerIn-wonderland commented Nov 21, 2024 • edited Loading

ProgrammerIn-wonderland commented Nov 21, 2024 • edited Loading

chschnell commented Nov 21, 2024

ProgrammerIn-wonderland commented Nov 21, 2024 • edited Loading

r58Playz commented Nov 21, 2024

ProgrammerIn-wonderland commented Nov 21, 2024 • edited Loading

r58Playz commented Nov 21, 2024 • edited Loading

chschnell commented Nov 4, 2024 •

edited

Loading

chschnell commented Nov 12, 2024 •

edited

Loading

chschnell commented Nov 12, 2024 •

edited

Loading

SuperMaxusa commented Nov 18, 2024 •

edited

Loading

chschnell commented Nov 20, 2024 •

edited

Loading

ProgrammerIn-wonderland commented Nov 21, 2024 •

edited

Loading

ProgrammerIn-wonderland commented Nov 21, 2024 •

edited

Loading

ProgrammerIn-wonderland commented Nov 21, 2024 •

edited

Loading

ProgrammerIn-wonderland commented Nov 21, 2024 •

edited

Loading

ProgrammerIn-wonderland commented Nov 21, 2024 •

edited

Loading

ProgrammerIn-wonderland commented Nov 21, 2024 •

edited

Loading

r58Playz commented Nov 21, 2024 •

edited

Loading